Skip to content
Snippets Groups Projects
  1. Sep 16, 2015
    • Tom Lane's avatar
      Fix documentation of regular expression character-entry escapes. · 11103c6d
      Tom Lane authored
      The docs claimed that \uhhhh would be interpreted as a Unicode value
      regardless of the database encoding, but it's never been implemented
      that way: \uhhhh and \xhhhh actually mean exactly the same thing, namely
      the character that pg_mb2wchar translates to 0xhhhh.  Moreover we were
      falsely dismissive of the usefulness of Unicode code points above FFFF.
      Fix that.
      
      It's been like this for ages, so back-patch to all supported branches.
      11103c6d
  2. Sep 12, 2015
    • Tom Lane's avatar
      Remove set-but-not-used variable. · 49232d41
      Tom Lane authored
      In branches before 9.3, commit 8703059c caused join_is_legal()'s
      unique_ified variable to become unused, since its only remaining
      use is for LATERAL-related tests which don't exist pre-9.3.
      My compiler didn't complain about that, but Peter's does.
      49232d41
  3. Sep 11, 2015
    • Bruce Momjian's avatar
      pg_dump, pg_upgrade: allow postgres/template1 tablespace moves · befc63e8
      Bruce Momjian authored
      Modify pg_dump to restore postgres/template1 databases to non-default
      tablespaces by switching out of the database to be moved, then switching
      back.
      
      Also, to fix potentially cases where the old/new tablespaces might not
      match, fix pg_upgrade to process new/old tablespaces separately in all
      cases.
      
      Report by Marti Raudsepp
      
      Patch by Marti Raudsepp, me
      
      Backpatch through 9.0
      befc63e8
  4. Sep 10, 2015
    • Tom Lane's avatar
      Fix setrefs.c comment properly. · fe146952
      Tom Lane authored
      The "typo" alleged in commit 1e460d4b was actually a comment that was
      correct when written, but I missed updating it in commit b5282aa8.
      Use a slightly less specific (and hopefully more future-proof) description
      of what is collected.  Back-patch to 9.2 where that commit appeared, and
      revert the comment to its then-entirely-correct state before that.
      fe146952
    • Stephen Frost's avatar
      Fix typo in setrefs.c · f4ea1b35
      Stephen Frost authored
      We're adding OIDs, not TIDs, to invalItems.
      
      Pointed out by Etsuro Fujita.
      
      Back-patch to all supported branches.
      f4ea1b35
    • Tom Lane's avatar
      Fix minor bug in regexp makesearch() function. · 3718015e
      Tom Lane authored
      The list-wrangling here was done wrong, allowing the same state to get
      put into the list twice.  The following loop then would clone it twice.
      The second clone would wind up with no inarcs, so that there was no
      observable misbehavior AFAICT, but a useless state in the finished NFA
      isn't an especially good thing.
      3718015e
  5. Sep 09, 2015
    • Fujii Masao's avatar
      Remove files signaling a standby promotion request at postmaster startup · 67518a14
      Fujii Masao authored
      This commit makes postmaster forcibly remove the files signaling
      a standby promotion request. Otherwise, the existence of those files
      can trigger a promotion too early, whether a user wants that or not.
      
      This removal of files is usually unnecessary because they can exist
      only during a few moments during a standby promotion. However
      there is a race condition: if pg_ctl promote is executed and creates
      the files during a promotion, the files can stay around even after
      the server is brought up to new master. Then, if new standby starts
      by using the backup taken from that master, the files can exist
      at the server startup and should be removed in order to avoid
      an unexpected promotion.
      
      Back-patch to 9.1 where promote signal file was introduced.
      
      Problem reported by Feike Steenbergen.
      Original patch by Michael Paquier, modified by me.
      
      Discussion: 20150528100705.4686.91426@wrigleys.postgresql.org
      67518a14
  6. Sep 08, 2015
  7. Sep 07, 2015
  8. Sep 06, 2015
    • Greg Stark's avatar
      Move DTK_ISODOW DTK_DOW and DTK_DOY to be type UNITS rather than · f4afbe06
      Greg Stark authored
      RESERV. RESERV is meant for tokens like "now" and having them in that
      category throws errors like these when used as an input date:
      
      stark=# SELECT 'doy'::timestamptz;
      ERROR:  unexpected dtype 33 while parsing timestamptz "doy"
      LINE 1: SELECT 'doy'::timestamptz;
                     ^
      stark=# SELECT 'dow'::timestamptz;
      ERROR:  unexpected dtype 32 while parsing timestamptz "dow"
      LINE 1: SELECT 'dow'::timestamptz;
                     ^
      
      Found by LLVM's Libfuzzer
      f4afbe06
  9. Sep 05, 2015
  10. Sep 04, 2015
    • Tom Lane's avatar
      Fix subtransaction cleanup after an outer-subtransaction portal fails. · 39ebb646
      Tom Lane authored
      Formerly, we treated only portals created in the current subtransaction as
      having failed during subtransaction abort.  However, if the error occurred
      while running a portal created in an outer subtransaction (ie, a cursor
      declared before the last savepoint), that has to be considered broken too.
      
      To allow reliable detection of which ones those are, add a bookkeeping
      field to struct Portal that tracks the innermost subtransaction in which
      each portal has actually been executed.  (Without this, we'd end up
      failing portals containing functions that had called the subtransaction,
      thereby breaking plpgsql exception blocks completely.)
      
      In addition, when we fail an outer-subtransaction Portal, transfer its
      resources into the subtransaction's resource owner, so that they're
      released early in cleanup of the subxact.  This fixes a problem reported by
      Jim Nasby in which a function executed in an outer-subtransaction cursor
      could cause an Assert failure or crash by referencing a relation created
      within the inner subtransaction.
      
      The proximate cause of the Assert failure is that AtEOSubXact_RelationCache
      assumed it could blow away a relcache entry without first checking that the
      entry had zero refcount.  That was a bad idea on its own terms, so add such
      a check there, and to the similar coding in AtEOXact_RelationCache.  This
      provides an independent safety measure in case there are still ways to
      provoke the situation despite the Portal-level changes.
      
      This has been broken since subtransactions were invented, so back-patch
      to all supported branches.
      
      Tom Lane and Michael Paquier
      39ebb646
  11. Aug 29, 2015
    • Tom Lane's avatar
      Fix s_lock.h PPC assembly code to be compatible with native AIX assembler. · 472680c5
      Tom Lane authored
      On recent AIX it's necessary to configure gcc to use the native assembler
      (because the GNU assembler hasn't been updated to handle AIX 6+).  This
      caused PG builds to fail with assembler syntax errors, because we'd try
      to compile s_lock.h's gcc asm fragment for PPC, and that assembly code
      relied on GNU-style local labels.  We can't substitute normal labels
      because it would fail in any file containing more than one inlined use of
      tas().  Fortunately, that code is stable enough, and the PPC ISA is simple
      enough, that it doesn't seem like too much of a maintenance burden to just
      hand-code the branch offsets, removing the need for any labels.
      
      Note that the AIX assembler only accepts "$" for the location counter
      pseudo-symbol.  The usual GNU convention is "."; but it appears that all
      versions of gas for PPC also accept "$", so in theory this patch will not
      break any other PPC platforms.
      
      This has been reported by a few people, but Steve Underwood gets the credit
      for being the first to pursue the problem far enough to understand why it
      was failing.  Thanks also to Noah Misch for additional testing.
      472680c5
  12. Aug 27, 2015
  13. Aug 26, 2015
    • Tom Lane's avatar
      Docs: be explicit about datatype matching for lead/lag functions. · 8cffc4f5
      Tom Lane authored
      The default argument, if given, has to be of exactly the same datatype
      as the first argument; but this was not stated in so many words, and
      the error message you get about it might not lead your thought in the
      right direction.  Per bug #13587 from Robert McGehee.
      
      A quick scan says that these are the only two built-in functions with two
      anyelement arguments and no other polymorphic arguments.  There are plenty
      of cases of, eg, anyarray and anyelement, but those seem less likely to
      confuse.  For instance this doesn't seem terribly hard to figure out:
      "function array_remove(integer[], numeric) does not exist".  So I've
      contented myself with fixing these two cases.
      8cffc4f5
  14. Aug 22, 2015
    • Tom Lane's avatar
      Avoid O(N^2) behavior when enlarging SPI tuple table in spi_printtup(). · d951d606
      Tom Lane authored
      For no obvious reason, spi_printtup() was coded to enlarge the tuple
      pointer table by just 256 slots at a time, rather than doubling the size at
      each reallocation, as is our usual habit.  For very large SPI results, this
      makes for O(N^2) time spent in repalloc(), which of course soon comes to
      dominate the runtime.  Use the standard doubling approach instead.
      
      This is a longstanding performance bug, so back-patch to all active
      branches.
      
      Neil Conway
      d951d606
  15. Aug 21, 2015
    • Tom Lane's avatar
      Fix plpython crash when returning string representation of a RECORD result. · dadef8af
      Tom Lane authored
      PLyString_ToComposite() blithely overwrote proc->result.out.d, even though
      for a composite result type the other union variant proc->result.out.r is
      the one that should be valid.  This could result in a crash if out.r had
      in fact been filled in (proc->result.is_rowtype == 1) and then somebody
      later attempted to use that data; as per bug #13579 from Paweł Michalak.
      
      Just to add insult to injury, it didn't work for RECORD results anyway,
      because record_in() would refuse the case.
      
      Fix by doing the I/O function lookup in a local PLyTypeInfo variable,
      as we were doing already in PLyObject_ToComposite().  This is not a great
      technique because any fn_extra data allocated by the input function will
      be leaked permanently (thanks to using TopMemoryContext as fn_mcxt).
      But that's a pre-existing issue that is much less serious than a crash,
      so leave it to be fixed separately.
      
      This bug would be a potential security issue, except that plpython is
      only available to superusers and the crash requires coding the function
      in a way that didn't work before today's patches.
      
      Add regression test cases covering all the supported methods of converting
      composite results.
      
      Back-patch to 9.1 where the faulty coding was introduced.
      dadef8af
    • Tom Lane's avatar
      Allow record_in() and record_recv() to work for transient record types. · 2f1d558b
      Tom Lane authored
      If we have the typmod that identifies a registered record type, there's no
      reason that record_in() should refuse to perform input conversion for it.
      Now, in direct SQL usage, record_in() will always be passed typmod = -1
      with type OID RECORDOID, because no typmodin exists for type RECORD, so the
      case can't arise.  However, some InputFunctionCall users such as PLs may be
      able to supply the right typmod, so we should allow this to support them.
      
      Note: the previous coding and comment here predate commit 59c016aa.
      There has been no case since 8.1 in which the passed type OID wouldn't be
      valid; and if it weren't, this error message wouldn't be apropos anyway.
      Better to let lookup_rowtype_tupdesc complain about it.
      
      Back-patch to 9.1, as this is necessary for my upcoming plpython fix.
      I'm committing it separately just to make it a bit more visible in the
      commit history.
      2f1d558b
  16. Aug 19, 2015
    • Tom Lane's avatar
      Fix a few bogus statement type names in plpgsql error messages. · fb41bf4b
      Tom Lane authored
      plpgsql's error location context messages ("PL/pgSQL function fn-name line
      line-no at stmt-type") would misreport a CONTINUE statement as being an
      EXIT, and misreport a MOVE statement as being a FETCH.  These are clear
      bugs that have been there a long time, so back-patch to all supported
      branches.
      
      In addition, in 9.5 and HEAD, change the description of EXECUTE from
      "EXECUTE statement" to just plain EXECUTE; there seems no good reason why
      this statement type should be described differently from others that have
      a well-defined head keyword.  And distinguish GET STACKED DIAGNOSTICS from
      plain GET DIAGNOSTICS.  These are a bit more of a judgment call, and also
      affect existing regression-test outputs, so I did not back-patch into
      stable branches.
      
      Pavel Stehule and Tom Lane
      fb41bf4b
  17. Aug 15, 2015
    • Tom Lane's avatar
      Improve documentation about MVCC-unsafe utility commands. · ed511659
      Tom Lane authored
      The table-rewriting forms of ALTER TABLE are MVCC-unsafe, in much the same
      way as TRUNCATE, because they replace all rows of the table with newly-made
      rows with a new xmin.  (Ideally, concurrent transactions with old snapshots
      would continue to see the old table contents, but the data is not there
      anymore --- and if it were there, it would be inconsistent with the table's
      updated rowtype, so there would be serious implementation problems to fix.)
      This was nowhere documented though, and the problem was only documented for
      TRUNCATE in a note in the TRUNCATE reference page.  Create a new "Caveats"
      section in the MVCC chapter that can be home to this and other limitations
      on serializable consistency.
      
      In passing, fix a mistaken statement that VACUUM and CLUSTER would reclaim
      space occupied by a dropped column.  They don't reconstruct existing tuples
      so they couldn't do that.
      
      Back-patch to all supported branches.
      ed511659
    • Andres Freund's avatar
      Don't use 'bool' as a struct member name in help_config.c. · 86ec86c2
      Andres Freund authored
      Doing so doesn't work if bool is a macro rather than a typedef.
      
      Although c.h spends some effort to support configurations where bool is
      a preexisting macro, help_config.c has existed this way since
      2003 (b700a6), and there have not been any reports of
      problems. Backpatch anyway since this is as riskless as it gets.
      
      Discussion: 20150812084351.GD8470@awork2.anarazel.de
      Backpatch: 9.0-master
      86ec86c2
  18. Aug 13, 2015
    • Tom Lane's avatar
      Improve regression test case to avoid depending on system catalog stats. · e8485478
      Tom Lane authored
      In commit 95f4e59c I added a regression test case that examined
      the plan of a query on system catalogs.  That isn't a terribly great idea
      because the catalogs tend to change from version to version, or even
      within a version if someone makes an unrelated regression-test change that
      populates the catalogs a bit differently.  Usually I try to make planner
      test cases rely on test tables that have not changed since Berkeley days,
      but I got sloppy in this case because the submitted crasher example queried
      the catalogs and I didn't spend enough time on rewriting it.  But it was a
      problem waiting to happen, as I was rudely reminded when I tried to port
      that patch into Salesforce's Postgres variant :-(.  So spend a little more
      effort and rewrite the query to not use any system catalogs.  I verified
      that this version still provokes the Assert if 95f4e59c's code fix
      is reverted.
      
      I also removed the EXPLAIN output from the test, as it turns out that the
      assertion occurs while considering a plan that isn't the one ultimately
      selected anyway; so there's no value in risking any cross-platform
      variation in that printout.
      
      Back-patch to 9.2, like the previous patch.
      e8485478
    • Michael Meskes's avatar
      Fix declaration of isarray variable. · e8f41777
      Michael Meskes authored
      Found and fixed by Andres Freund.
      e8f41777
    • Tom Lane's avatar
      Undo mistaken tightening in join_is_legal(). · 866197d8
      Tom Lane authored
      One of the changes I made in commit 8703059c turns out not to have
      been such a good idea: we still need the exception in join_is_legal() that
      allows a join if both inputs already overlap the RHS of the special join
      we're checking.  Otherwise we can miss valid plans, and might indeed fail
      to find a plan at all, as in recent report from Andreas Seltenreich.
      
      That code was added way back in commit c1711764, but I failed to
      include a regression test case then; my bad.  Put it back with a better
      explanation, and a test this time.  The logic does end up a bit different
      than before though: I now believe it's appropriate to make this check
      first, thereby allowing such a case whether or not we'd consider the
      previous SJ(s) to commute with this one.  (Presumably, we already decided
      they did; but it was confusing to have this consideration in the middle
      of the code that was handling the other case.)
      
      Back-patch to all active branches, like the previous patch.
      866197d8
  19. Aug 12, 2015
  20. Aug 11, 2015
    • Tom Lane's avatar
      Fix privilege dumping from servers too old to have that type of privilege. · be9ef396
      Tom Lane authored
      pg_dump produced fairly silly GRANT/REVOKE commands when dumping types from
      pre-9.2 servers, and when dumping functions or procedural languages from
      pre-7.3 servers.  Those server versions lack the typacl, proacl, and/or
      lanacl columns respectively, and pg_dump substituted default values that
      were in fact incorrect.  We ended up revoking all the owner's own
      privileges for the object while granting all privileges to PUBLIC.
      Of course the owner would then have those privileges again via PUBLIC, so
      long as she did not try to revoke PUBLIC's privileges; which may explain
      the lack of field reports.  Nonetheless this is pretty silly behavior.
      
      The stakes were raised by my recent patch to make pg_dump dump shell types,
      because 9.2 and up pg_dump would proceed to emit bogus GRANT/REVOKE
      commands for a shell type if dumping from a pre-9.2 server; and the server
      will not accept GRANT/REVOKE commands for a shell type.  (Perhaps it
      should, but that's a topic for another day.)  So the resulting dump script
      wouldn't load without errors.
      
      The right thing to do is to act as though these objects have default
      privileges (null ACL entries), which causes pg_dump to print no
      GRANT/REVOKE commands at all for them.  That fixes the silly results
      and also dodges the problem with shell types.
      
      In passing, modify getProcLangs() to be less creatively different about
      how to handle missing columns when dumping from older server versions.
      Every other data-acquisition function in pg_dump does that by substituting
      appropriate default values in the version-specific SQL commands, and I see
      no reason why this one should march to its own drummer.  Its use of
      "SELECT *" was likewise not conformant with anyplace else, not to mention
      it's not considered good SQL style for production queries.
      
      Back-patch to all supported versions.  Although 9.0 and 9.1 pg_dump don't
      have the issue with typacl, they are more likely than newer versions to be
      used to dump from ancient servers, so we ought to fix the proacl/lanacl
      issues all the way back.
      be9ef396
  21. Aug 10, 2015
    • Tom Lane's avatar
      Accept alternate spellings of __sparcv7 and __sparcv8. · fe460461
      Tom Lane authored
      Apparently some versions of gcc prefer __sparc_v7__ and __sparc_v8__.
      Per report from Waldemar Brodkorb.
      fe460461
    • Tom Lane's avatar
      Further mucking with PlaceHolderVar-related restrictions on join order. · 54cea765
      Tom Lane authored
      Commit 85e5e222 turns out not to have taken
      care of all cases of the partially-evaluatable-PlaceHolderVar problem found
      by Andreas Seltenreich's fuzz testing.  I had set it up to check for risky
      PHVs only in the event that we were making a star-schema-based exception to
      the param_source_rels join ordering heuristic.  However, it turns out that
      the problem can occur even in joins that satisfy the param_source_rels
      heuristic, in which case allow_star_schema_join() isn't consulted.
      Refactor so that we check for risky PHVs whenever the proposed join has
      any remaining parameterization.
      
      Back-patch to 9.2, like the previous patch (except for the regression test
      case, which only works back to 9.3 because it uses LATERAL).
      
      Note that this discovery implies that problems of this sort could've
      occurred in 9.2 and up even before the star-schema patch; though I've not
      tried to prove that experimentally.
      54cea765
  22. Aug 06, 2015
    • Tom Lane's avatar
      Further fixes for degenerate outer join clauses. · 754ece93
      Tom Lane authored
      Further testing revealed that commit f69b4b94 was still a few
      bricks shy of a load: minor tweaking of the previous test cases resulted
      in the same wrong-outer-join-order problem coming back.  After study
      I concluded that my previous changes in make_outerjoininfo() were just
      accidentally masking the problem, and should be reverted in favor of
      forcing syntactic join order whenever an upper outer join's predicate
      doesn't mention a lower outer join's LHS.  This still allows the
      chained-outer-joins style that is the normally optimizable case.
      
      I also tightened things up some more in join_is_legal().  It seems to me
      on review that what's really happening in the exception case where we
      ignore a mismatched special join is that we're allowing the proposed join
      to associate into the RHS of the outer join we're comparing it to.  As
      such, we should *always* insist that the proposed join be a left join,
      which eliminates a bunch of rather dubious argumentation.  The case where
      we weren't enforcing that was the one that was already known buggy anyway
      (it had a violatable Assert before the aforesaid commit) so it hardly
      deserves a lot of deference.
      
      Back-patch to all active branches, like the previous patch.  The added
      regression test case failed in all branches back to 9.1, and I think it's
      only an unrelated change in costing calculations that kept 9.0 from
      choosing a broken plan.
      754ece93
  23. Aug 05, 2015
    • Tom Lane's avatar
      Make real sure we don't reassociate joins into or out of SEMI/ANTI joins. · 08dee567
      Tom Lane authored
      Per the discussion in optimizer/README, it's unsafe to reassociate anything
      into or out of the RHS of a SEMI or ANTI join.  An example from Piotr
      Stefaniak showed that join_is_legal() wasn't sufficiently enforcing this
      rule, so lock it down a little harder.
      
      I couldn't find a reasonably simple example of the optimizer trying to
      do this, so no new regression test.  (Piotr's example involved the random
      search in GEQO accidentally trying an invalid case and triggering a sanity
      check way downstream in clause selectivity estimation, which did not seem
      like a sequence of events that would be useful to memorialize in a
      regression test as-is.)
      
      Back-patch to all active branches.
      08dee567
    • Tom Lane's avatar
      Docs: add an explicit example about controlling overall greediness of REs. · 4eb4e711
      Tom Lane authored
      Per discussion of bug #13538.
      4eb4e711
    • Tom Lane's avatar
      Fix pg_dump to dump shell types. · dae6e460
      Tom Lane authored
      Per discussion, it really ought to do this.  The original choice to
      exclude shell types was probably made in the dark ages before we made
      it harder to accidentally create shell types; but that was in 7.3.
      
      Also, cause the standard regression tests to leave a shell type behind,
      for convenience in testing the case in pg_dump and pg_upgrade.
      
      Back-patch to all supported branches.
      dae6e460
    • Tom Lane's avatar
      Fix bogus "out of memory" reports in tuplestore.c. · b6659a3b
      Tom Lane authored
      The tuplesort/tuplestore memory management logic assumed that the chunk
      allocation overhead for its memtuples array could not increase when
      increasing the array size.  This is and always was true for tuplesort,
      but we (I, I think) blindly copied that logic into tuplestore.c without
      noticing that the assumption failed to hold for the much smaller array
      elements used by tuplestore.  Given rather small work_mem, this could
      result in an improper complaint about "unexpected out-of-memory situation",
      as reported by Brent DeSpain in bug #13530.
      
      The easiest way to fix this is just to increase tuplestore's initial
      array size so that the assumption holds.  Rather than relying on magic
      constants, though, let's export a #define from aset.c that represents
      the safe allocation threshold, and make tuplestore's calculation depend
      on that.
      
      Do the same in tuplesort.c to keep the logic looking parallel, even though
      tuplesort.c isn't actually at risk at present.  This will keep us from
      breaking it if we ever muck with the allocation parameters in aset.c.
      
      Back-patch to all supported versions.  The error message doesn't occur
      pre-9.3, not so much because the problem can't happen as because the
      pre-9.3 tuplestore code neglected to check for it.  (The chance of
      trouble is a great deal larger as of 9.3, though, due to changes in the
      array-size-increasing strategy.)  However, allowing LACKMEM() to become
      true unexpectedly could still result in less-than-desirable behavior,
      so let's patch it all the way back.
      b6659a3b
  24. Aug 04, 2015
    • Tom Lane's avatar
      Fix a PlaceHolderVar-related oversight in star-schema planning patch. · 359016d2
      Tom Lane authored
      In commit b514a746, I changed the planner
      so that it would allow nestloop paths to remain partially parameterized,
      ie the inner relation might need parameters from both the current outer
      relation and some upper-level outer relation.  That's fine so long as we're
      talking about distinct parameters; but the patch also allowed creation of
      nestloop paths for cases where the inner relation's parameter was a
      PlaceHolderVar whose eval_at set included the current outer relation and
      some upper-level one.  That does *not* work.
      
      In principle we could allow such a PlaceHolderVar to be evaluated at the
      lower join node using values passed down from the upper relation along with
      values from the join's own outer relation.  However, nodeNestloop.c only
      supports simple Vars not arbitrary expressions as nestloop parameters.
      createplan.c is also a few bricks shy of being able to handle such cases;
      it misplaces the PlaceHolderVar parameters in the plan tree, which is why
      the visible symptoms of this bug are "plan should not reference subplan's
      variable" and "failed to assign all NestLoopParams to plan nodes" planner
      errors.
      
      Adding the necessary complexity to make this work doesn't seem like it
      would be repaid in significantly better plans, because in cases where such
      a PHV exists, there is probably a corresponding join order constraint that
      would allow a good plan to be found without using the star-schema exception.
      Furthermore, adding complexity to nodeNestloop.c would create a run-time
      penalty even for plans where this whole consideration is irrelevant.
      So let's just reject such paths instead.
      
      Per fuzz testing by Andreas Seltenreich; the added regression test is based
      on his example query.  Back-patch to 9.2, like the previous patch.
      359016d2
Loading