1. 11 Nov, 2013 4 commits
    • Tom Lane's avatar
      Fix failure with whole-row reference to a subquery. · ebefbb5f
      Tom Lane authored
      Simple oversight in commit 1cb108ef ---
      recursively examining a subquery output column is only sane if the
      original Var refers to a single output column.  Found by Kevin Grittner.
      ebefbb5f
    • Tom Lane's avatar
      Fix ruleutils pretty-printing to not generate trailing whitespace. · 0b7e660d
      Tom Lane authored
      The pretty-printing logic in ruleutils.c operates by inserting a newline
      and some indentation whitespace into strings that are already valid SQL.
      This naturally results in leaving some trailing whitespace before the
      newline in many cases; which can be annoying when processing the output
      with other tools, as complained of by Joe Abbate.  We can fix that in
      a pretty localized fashion by deleting any trailing whitespace before
      we append a pretty-printing newline.  In addition, we have to modify the
      code inserted by commit 2f582f76 so that
      we also delete trailing whitespace when transposing items from temporary
      buffers into the main result string, when a temporary item starts with a
      newline.
      
      This results in rather voluminous changes to the regression test results,
      but it's easily verified that they are only removal of trailing whitespace.
      
      Back-patch to 9.3, because the aforementioned commit resulted in many
      more cases of trailing whitespace than had occurred in earlier branches.
      0b7e660d
    • Tom Lane's avatar
      Re-allow duplicate aliases within aliased JOINs. · 648bd05b
      Tom Lane authored
      Although the SQL spec forbids duplicate table aliases, historically
      we've allowed queries like
          SELECT ... FROM tab1 x CROSS JOIN (tab2 x CROSS JOIN tab3 y) z
      on the grounds that the aliased join (z) hides the aliases within it,
      therefore there is no conflict between the two RTEs named "x".  The
      LATERAL patch broke this, on the misguided basis that "x" could be
      ambiguous if tab3 were a LATERAL subquery.  To avoid breaking existing
      queries, it's better to allow this situation and complain only if
      tab3 actually does contain an ambiguous reference.  We need only remove
      the check that was throwing an error, because the column lookup code
      is already prepared to handle ambiguous references.  Per bug #8444.
      648bd05b
    • Magnus Hagander's avatar
      Don't abort pg_basebackup when receiving empty WAL block · 705556a6
      Magnus Hagander authored
      This is a similar fix as c6ec8793aa59d1842082e14b4b4aae7d4bd883fd
      9.2. This should never happen in 9.3 and newer since the special case
      cannot happen there, but this patch synchronizes up the code so there
      is no confusion on why they're different. An empty block is as harmless
      in 9.3 as it was in 9.2, and can safely be ignored.
      705556a6
  2. 10 Nov, 2013 1 commit
  3. 09 Nov, 2013 1 commit
    • Robert Haas's avatar
      Fix ECPG compiler warning. · dca09ac5
      Robert Haas authored
      Commit 9b4d52f2 failed to notice
      that pg_regress_ecpg needed updating.
      
      This patch was independently submitted by both David Rowley
      and Andres Freund.
      dca09ac5
  4. 08 Nov, 2013 7 commits
    • Heikki Linnakangas's avatar
      Fix race condition in GIN posting tree page deletion. · ac4ab97e
      Heikki Linnakangas authored
      If a page is deleted, and reused for something else, just as a search is
      following a rightlink to it from its left sibling, the search would continue
      scanning whatever the new contents of the page are. That could lead to
      incorrect query results, or even something more curious if the page is
      reused for a different kind of a page.
      
      To fix, modify the search algorithm to lock the next page before releasing
      the previous one, and refrain from deleting pages from the leftmost branch
      of the tree.
      
      Add a new Concurrency section to the README, explaining why this works.
      There is a lot more one could say about concurrency in GIN, but that's for
      another patch.
      
      Backpatch to all supported versions.
      ac4ab97e
    • Robert Haas's avatar
      doc: Clarify under what circumstances pg_dump needs superuser access. · 636b868f
      Robert Haas authored
      Inspired by, but different from, a patch from Ivan Lezhnjov IV
      636b868f
    • Robert Haas's avatar
      Fix pg_isolation_regress to work outside its build directory. · 9b4d52f2
      Robert Haas authored
      This makes it possible to, for example, use the isolation tester to
      test a contrib module.
      
      Andres Freund
      9b4d52f2
    • Robert Haas's avatar
      Add the notion of REPLICA IDENTITY for a table. · 07cacba9
      Robert Haas authored
      Pending patches for logical replication will use this to determine
      which columns of a tuple ought to be considered as its candidate key.
      
      Andres Freund, with minor, mostly cosmetic adjustments by me
      07cacba9
    • Tom Lane's avatar
      Make contain_volatile_functions/contain_mutable_functions look into SubLinks. · b97ee66c
      Tom Lane authored
      This change prevents us from doing inappropriate subquery flattening in
      cases such as dangerous functions hidden inside a sub-SELECT in the
      targetlist of another sub-SELECT.  That could result in unexpected behavior
      due to multiple evaluations of a volatile function, as in a recent
      complaint from Etienne Dube.  It's been questionable from the very
      beginning whether these functions should look into subqueries (as noted in
      their comments), and this case seems to provide proof that they should.
      
      Because the new code only descends into SubLinks, not SubPlans or
      InitPlans, the change only affects the planner's behavior during
      prepjointree processing and not later on --- for example, you can still get
      it to use a volatile function in an indexqual if you wrap the function in
      (SELECT ...).  That's a historical behavior, for sure, but it's reasonable
      given that the executor's evaluation rules for subplans don't depend on
      whether there are volatile functions inside them.  In any case, we need to
      constrain the behavioral change as narrowly as we can to make this
      reasonable to back-patch.
      b97ee66c
    • Tom Lane's avatar
      Fix subtly-wrong volatility checking in BeginCopyFrom(). · 060b22a9
      Tom Lane authored
      contain_volatile_functions() is best applied to the output of
      expression_planner(), not its input, so that insertion of function
      default arguments and constant-folding have been done.  (See comments
      at CheckMutability, for instance.)  It's perhaps unlikely that anyone
      will notice a difference in practice, but still we should do it properly.
      
      In passing, change variable type from Node* to Expr* to reduce the net
      number of casts needed.
      
      Noted while perusing uses of contain_volatile_functions().
      060b22a9
    • Tom Lane's avatar
      Make LOCK_PRINT & PROCLOCK_PRINT expand to ((void) 0) when not in use. · 20803d78
      Tom Lane authored
      This avoids warnings from more-anal-than-average compilers, and might
      prevent hidden syntax problems in the future.
      
      Andres Freund
      20803d78
  5. 07 Nov, 2013 5 commits
    • Kevin Grittner's avatar
    • Tom Lane's avatar
      Prevent display of dropped columns in row constraint violation messages. · c28b289b
      Tom Lane authored
      ExecBuildSlotValueDescription() printed "null" for each dropped column in
      a row being complained of by ExecConstraints().  This has some sanity in
      terms of the underlying implementation, but is of course pretty surprising
      to users.  To fix, we must pass the target relation's descriptor to
      ExecBuildSlotValueDescription(), because the slot descriptor it had been
      using doesn't get labeled with attisdropped markers.
      
      Per bug #8408 from Maxim Boguk.  Back-patch to 9.2 where the feature of
      printing row values in NOT NULL and CHECK constraint violation messages
      was introduced.
      
      Michael Paquier and Tom Lane
      c28b289b
    • Tom Lane's avatar
      Fix generation of MergeAppend plans for optimized min/max on expressions. · 5e900bc0
      Tom Lane authored
      Before jamming a desired targetlist into a plan node, one really ought to
      make sure the plan node can handle projections, and insert a buffering
      Result plan node if not.  planagg.c forgot to do this, which is a hangover
      from the days when it only dealt with IndexScan plan types.  MergeAppend
      doesn't project though, not to mention that it gets unhappy if you remove
      its possibly-resjunk sort columns.  The code accidentally failed to fail
      for cases in which the min/max argument was a simple Var, because the new
      targetlist would be equivalent to the original "flat" tlist anyway.
      For any more complex case, it's been broken since 9.1 where we introduced
      the ability to optimize min/max using MergeAppend, as reported by Raphael
      Bauduin.  Fix by duplicating the logic from grouping_planner that decides
      whether we need a Result node.
      
      In 9.2 and 9.1, this requires back-porting the tlist_same_exprs() function
      introduced in commit 4387cf95, else we'd
      uselessly add a Result node in cases that worked before.  It's rather
      tempting to back-patch that whole commit so that we can avoid extra Result
      nodes in mainline cases too; but I'll refrain, since that code hasn't
      really seen all that much field testing yet.
      5e900bc0
    • Heikki Linnakangas's avatar
      Fix setting of right bound at GIN page split. · fde7172d
      Heikki Linnakangas authored
      Broken by my refactoring.
      fde7172d
    • Tom Lane's avatar
      Add #ifdef guards for some POSIX error symbols that Windows doesn't like. · 8dace66e
      Tom Lane authored
      Per buildfarm results.  It looks like the older the Windows version, the
      more errno codes it hasn't got ...
      8dace66e
  6. 06 Nov, 2013 6 commits
    • Tom Lane's avatar
      Be more robust when strerror() doesn't give a useful result. · 8e68816c
      Tom Lane authored
      glibc, at least, is capable of returning "???" instead of anything useful
      if it doesn't like the setting of LC_CTYPE.  If this happens, or in the
      previously-known case of strerror() returning an empty string, try to
      print the C macro name for the error code ("EACCES" etc).  Only if we
      don't have the error code in our compiled-in list of popular error codes
      (which covers most though not quite all of what's called out in the POSIX
      spec) will we fall back to printing a numeric error code.  This should
      simplify debugging.
      
      Note that this functionality is currently only provided for %m in backend
      ereport/elog messages.  That may be sufficient, since we don't fool with the
      locale environment in frontend clients, but it's foreseeable that we might
      want similar code in libpq for instance.
      
      There was some talk of back-patching this, but let's see how the buildfarm
      likes it first.  It seems likely that at least some of the POSIX-defined
      error code symbols don't exist on all platforms.  I don't want to clutter
      the entire list with #ifdefs, but we may need more than are here now.
      
      MauMau, edited by me
      8e68816c
    • Tom Lane's avatar
      Support default arguments and named-argument notation for window functions. · bb45c640
      Tom Lane authored
      These things didn't work because the planner omitted to do the necessary
      preprocessing of a WindowFunc's argument list.  Add the few dozen lines
      of code needed to handle that.
      
      Although this sounds like a feature addition, it's really a bug fix because
      the default-argument case was likely to crash previously, due to lack of
      checking of the number of supplied arguments in the built-in window
      functions.  It's not a security issue because there's no way for a
      non-superuser to create a window function definition with defaults that
      refers to a built-in C function, but nonetheless people might be annoyed
      that it crashes rather than producing a useful error message.  So
      back-patch as far as the patch applies easily, which turns out to be 9.2.
      I'll put a band-aid in earlier versions as a separate patch.
      
      (Note that these features still don't work for aggregates, and fixing that
      case will be harder since we represent aggregate arg lists as target lists
      not bare expression lists.  There's no crash risk though because CREATE
      AGGREGATE doesn't accept defaults, and we reject named-argument notation
      when parsing an aggregate call.)
      bb45c640
    • Kevin Grittner's avatar
      Keep heap open until new heap generated in RMV. · 5829082a
      Kevin Grittner authored
      Early close became apparent when invalidation messages were
      processed in a new location under CLOBBER_CACHE_ALWAYS builds, due
      to additional locking.
      
      Back-patch to 9.3
      5829082a
    • Heikki Linnakangas's avatar
      Fix missing argument and function prototypes. · 0ea53256
      Heikki Linnakangas authored
      Not sure how I missed these in previous commit.
      0ea53256
    • Heikki Linnakangas's avatar
      Misc GIN refactoring. · ecaa4708
      Heikki Linnakangas authored
      Merge the isEnoughSpace and placeToPage functions in the b-tree interface
      into one function that tries to put a tuple on page, and returns false if
      it doesn't fit.
      
      Move createPostingTree function to gindatapage.c, and change its contract
      so that it can be passed more items than fit on the root page. It's in a
      better position than the callers to know how many items fit.
      
      Move ginMergeItemPointers out of gindatapage.c, into a separate file.
      
      These changes make no difference now, but reduce the footprint of Alexander
      Korotkov's upcoming patch to pack item pointers more tightly.
      ecaa4708
    • Tom Lane's avatar
      Improve the error message given for modifying a window with frame clause. · 920c8261
      Tom Lane authored
      For rather inscrutable reasons, SQL:2008 disallows copying-and-modifying a
      window definition that has any explicit framing clause.  The error message
      we gave for this only made sense if the referencing window definition
      itself contains an explicit framing clause, which it might well not.
      Moreover, in the context of an OVER clause it's not exactly obvious that
      "OVER (windowname)" implies copy-and-modify while "OVER windowname" does
      not.  This has led to multiple complaints, eg bug #5199 from Iliya
      Krapchatov.  Change to a hopefully more intelligible error message, and
      in the case where we have just "OVER (windowname)", add a HINT suggesting
      that omitting the parentheses will fix it.  Also improve the related
      documentation.  Back-patch to all supported branches.
      920c8261
  7. 05 Nov, 2013 3 commits
    • Tom Lane's avatar
      Revert commit 0725065b. · d4e6133c
      Tom Lane authored
      The previous commit was intended to make psql show the full path name when
      doing a \s (history save), but it was very badly implemented and would show
      confusing if not outright wrong information in many situations; for
      instance if the path name given to \s is absolute, or if \cd commands
      involving relative paths have been issued.  Consensus seems to be that
      we don't especially need this functionality in \s, and certainly not in \s
      alone.  So revert rather than trying to fix it up.  Per gripe from
      Ian Barwick.
      
      Although the bogus behavior exists in all supported versions, I'm not
      back-patching, because the work created for translators (by change of
      a translatable message) would probably outweigh the value of what is
      after all a mostly-cosmetic change.
      d4e6133c
    • Kevin Grittner's avatar
      Lock relation used to generate fresh data for RMV. · 2636ecf7
      Kevin Grittner authored
      The relation should not be accessible to any other process, but it
      should be locked for consistency.  Since this is not known to
      cause any bug, it will not be back-patch, at least for now.
      
      Per report from Andres Freund
      2636ecf7
    • Tom Lane's avatar
      Fix some obsolete information in src/backend/optimizer/README. · 6331de1d
      Tom Lane authored
      Constant quals aren't handled the same way they used to be.  Also,
      add mention of a couple more major steps in grouping_planner.
      Per complaint a couple months back from Etsuro Fujita.
      6331de1d
  8. 04 Nov, 2013 3 commits
  9. 03 Nov, 2013 4 commits
    • Tom Lane's avatar
      Get rid of more cases of the "must detoast before output function" meme. · e36ce0c7
      Tom Lane authored
      I missed that json.c was doing this too, because for some bizarre reason
      it wasn't doing it adjacent to the output function call.
      e36ce0c7
    • Tom Lane's avatar
      Prevent memory leaks from accumulating across printtup() calls. · b006f4dd
      Tom Lane authored
      Historically, printtup() has assumed that it could prevent memory leakage
      by pfree'ing the string result of each output function and manually
      managing detoasting of toasted values.  This amounts to assuming that
      datatype output functions never leak any memory internally; an assumption
      we've already decided to be bogus elsewhere, for example in COPY OUT.
      range_out in particular is known to leak multiple kilobytes per call, as
      noted in bug #8573 from Godfried Vanluffelen.  While we could go in and fix
      that leak, it wouldn't be very notationally convenient, and in any case
      there have been and undoubtedly will again be other leaks in other output
      functions.  So what seems like the best solution is to run the output
      functions in a temporary memory context that can be reset after each row,
      as we're doing in COPY OUT.  Some quick experimentation suggests this is
      actually a tad faster than the retail pfree's anyway.
      
      This patch fixes all the variants of printtup, except for debugtup()
      which is used in standalone mode.  It doesn't seem worth worrying
      about query-lifespan leaks in standalone mode, and fixing that case
      would be a bit tedious since debugtup() doesn't currently have any
      startup or shutdown functions.
      
      While at it, remove manual detoast management from several other
      output-function call sites that had copied it from printtup().  This
      doesn't make a lot of difference right now, but in view of recent
      discussions about supporting "non-flattened" Datums, we're going to
      want that code gone eventually anyway.
      
      Back-patch to 9.2 where range_out was introduced.  We might eventually
      decide to back-patch this further, but in the absence of known major
      leaks in older output functions, I'll refrain for now.
      b006f4dd
    • Michael Meskes's avatar
    • Kevin Grittner's avatar
      Acquire appropriate locks when rewriting during RMV. · 2a781d57
      Kevin Grittner authored
      Since the query has not been freshly parsed when executing REFRESH
      MATERIALIZED VIEW, locks must be explicitly taken before rewrite.
      
      Backpatch to 9.3.
      
      Andres Freund
      2a781d57
  10. 02 Nov, 2013 2 commits
    • Kevin Grittner's avatar
      Fix subquery reference to non-populated MV in CMV. · be420fa0
      Kevin Grittner authored
      A subquery reference to a matview should be allowed by CREATE
      MATERIALIZED VIEW WITH NO DATA, just like a direct reference is.
      
      Per bug report from Laurent Sartran.
      
      Backpatch to 9.3.
      be420fa0
    • Tom Lane's avatar
      Retry after buffer locking failure during SPGiST index creation. · 24ace405
      Tom Lane authored
      The original coding thought this case was impossible, but it can happen
      if the bgwriter or checkpointer processes decide to write out an index
      page while creation is still proceeding, leading to a bogus "unexpected
      spgdoinsert() failure" error.  Problem reported by Jonathan S. Katz.
      
      Teodor Sigaev
      24ace405
  11. 01 Nov, 2013 4 commits
    • Tom Lane's avatar
      Ensure all files created for a single BufFile have the same resource owner. · bffd1ce9
      Tom Lane authored
      Callers expect that they only have to set the right resource owner when
      creating a BufFile, not during subsequent operations on it.  While we could
      insist this be fixed at the caller level, it seems more sensible for the
      BufFile to take care of it.  Without this, some temp files belonging to
      a BufFile can go away too soon, eg at the end of a subtransaction,
      leading to errors or crashes.
      
      Reported and fixed by Andres Freund.  Back-patch to all active branches.
      bffd1ce9
    • Tom Lane's avatar
      Remove CTimeZone/HasCTZSet, root and branch. · 45f64f1b
      Tom Lane authored
      These variables no longer have any useful purpose, since there's no reason
      to special-case brute force timezones now that we have a valid
      session_timezone setting for them.  Remove the variables, and remove the
      SET/SHOW TIME ZONE code that deals with them.
      
      The user-visible impact of this is that SHOW TIME ZONE will now show a
      POSIX-style zone specification, in the form "<+-offset>-+offset", rather
      than an interval value when a brute-force zone has been set.  While perhaps
      less intuitive, this is a better definition than before because it's
      actually possible to give that string back to SET TIME ZONE and get the
      same behavior, unlike what used to happen.
      
      We did not previously mention the angle-bracket syntax when describing
      POSIX timezone specifications; add some documentation so that people
      can figure out what these strings do.  (There's still quite a lot of
      undocumented functionality there, but anybody who really cares can
      go read the POSIX spec to find out about it.  In practice most people
      seem to prefer Olsen-style city names anyway.)
      45f64f1b
    • Tom Lane's avatar
      Remove internal uses of CTimeZone/HasCTZSet. · 1c8a7f61
      Tom Lane authored
      The only remaining places where we actually look at CTimeZone/HasCTZSet
      are abstime2tm() and timestamp2tm().  Now that session_timezone is always
      valid, we can remove these special cases.  The caller-visible impact of
      this is that these functions now always return a valid zone abbreviation
      if requested, whereas before they'd return a NULL pointer if a brute-force
      timezone was in use.  In the existing code, the only place I can find that
      changes behavior is to_char(), whose TZ format code will now print
      something useful rather than nothing for such zones.  (In the places where
      the returned zone abbreviation is passed to EncodeDateTime, the lack of
      visible change is because we've chosen the abbreviation used for these
      zones to match what EncodeTimezone would have printed.)
      
      It's likely that there is now a fair amount of removable dead code around
      the call sites, namely anything that's meant to cope with getting a NULL
      timezone abbreviation, but I've not made an effort to root that out.
      
      This could be back-patched if we decide we'd like to fix to_char()'s
      behavior in the back branches, but there doesn't seem to be much
      enthusiasm for that at present.
      1c8a7f61
    • Tom Lane's avatar
      Fix some odd behaviors when using a SQL-style simple GMT offset timezone. · 631dc390
      Tom Lane authored
      Formerly, when using a SQL-spec timezone setting with a fixed GMT offset
      (called a "brute force" timezone in the code), the session_timezone
      variable was not updated to match the nominal timezone; rather, all code
      was expected to ignore session_timezone if HasCTZSet was true.  This is
      of course obviously fragile, though a search of the code finds only
      timeofday() failing to honor the rule.  A bigger problem was that
      DetermineTimeZoneOffset() supposed that if its pg_tz parameter was
      pointer-equal to session_timezone, then HasCTZSet should override the
      parameter.  This would cause datetime input containing an explicit zone
      name to be treated as referencing the brute-force zone instead, if the
      zone name happened to match the session timezone that had prevailed
      before installing the brute-force zone setting (as reported in bug #8572).
      The same malady could affect AT TIME ZONE operators.
      
      To fix, set up session_timezone so that it matches the brute-force zone
      specification, which we can do using the POSIX timezone definition syntax
      "<abbrev>offset", and get rid of the bogus lookaside check in
      DetermineTimeZoneOffset().  Aside from fixing the erroneous behavior in
      datetime parsing and AT TIME ZONE, this will cause the timeofday() function
      to print its result in the user-requested time zone rather than some
      previously-set zone.  It might also affect results in third-party
      extensions, if there are any that make use of session_timezone without
      considering HasCTZSet, but in all cases the new behavior should be saner
      than before.
      
      Back-patch to all supported branches.
      631dc390