1. 13 Aug, 2015 5 commits
    • Alvaro Herrera's avatar
      Use materialize SRF mode in brin_page_items · 94d626ff
      Alvaro Herrera authored
      This function was using the single-value-per-call mechanism, but the
      code relied on a relcache entry that wasn't kept open across calls.
      This manifested as weird errors in buildfarm during the short time that
      the "brin-1" isolation test lived.
      
      Backpatch to 9.5, where it was introduced.
      94d626ff
    • Heikki Linnakangas's avatar
      Run autoheader to add a few missing #defines to pg_config.h.in. · 36e863bb
      Heikki Linnakangas authored
      These are emitted by the new ax_pthread.m4 script version. They are not
      used for anything in PostgreSQL, but let's keep the generated header file
      up-to-date.
      
      Andres Freund
      36e863bb
    • Michael Meskes's avatar
      Fix declaration of isarray variable. · c396f2b8
      Michael Meskes authored
      Found and fixed by Andres Freund.
      c396f2b8
    • Alvaro Herrera's avatar
      Fix unitialized variables · fcbf4558
      Alvaro Herrera authored
      As complained by clang, reported by Andres Freund.  Brown paper bag bug
      in ccc4c074.
      
      Add some comments, too.
      
      Backpatch to 9.5, like that one.
      fcbf4558
    • Tom Lane's avatar
      Undo mistaken tightening in join_is_legal(). · cfe30a72
      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.
      cfe30a72
  2. 12 Aug, 2015 6 commits
    • Alvaro Herrera's avatar
      Close some holes in BRIN page assignment · ccc4c074
      Alvaro Herrera authored
      In some corner cases, it is possible for the BRIN index relation to be
      extended by brin_getinsertbuffer but the new page not be used
      immediately for anything by its callers; when this happens, the page is
      initialized and the FSM is updated (by brin_getinsertbuffer) with the
      info about that page, but these actions are not WAL-logged.  A later
      index insert/update can use the page, but since the page is already
      initialized, the initialization itself is not WAL-logged then either.
      Replay of this sequence of events causes recovery to fail altogether.
      
      There is a related corner case within brin_getinsertbuffer itself, in
      which we extend the relation to put a new index tuple there, but later
      find out that we cannot do so, and do not return the buffer; the page
      obtained from extension is not even initialized.  The resulting page is
      lost forever.
      
      To fix, shuffle the code so that initialization is not the
      responsibility of brin_getinsertbuffer anymore, in normal cases;
      instead, the initialization is done by its callers (brin_doinsert and
      brin_doupdate) once they're certain that the page is going to be used.
      When either those functions determine that the new page cannot be used,
      before bailing out they initialize the page as an empty regular page,
      enter it in FSM and WAL-log all this.  This way, the page is usable for
      future index insertions, and WAL replay doesn't find trying to insert
      tuples in pages whose initialization didn't make it to the WAL.  The
      same strategy is used in brin_getinsertbuffer when it cannot return the
      new page.
      
      Additionally, add a new step to vacuuming so that all pages of the index
      are scanned; whenever an uninitialized page is found, it is initialized
      as empty and WAL-logged.  This closes the hole that the relation is
      extended but the system crashes before anything is WAL-logged about it.
      We also take this opportunity to update the FSM, in case it has gotten
      out of date.
      
      Thanks to Heikki Linnakangas for finding the problem that kicked some
      additional analysis of BRIN page assignment code.
      
      Backpatch to 9.5, where BRIN was introduced.
      
      Discussion: https://www.postgresql.org/message-id/20150723204810.GY5596@postgresql.org
      ccc4c074
    • Andres Freund's avatar
      Remove duplicated assignment in pg_create_physical_replication_slot. · a4b059fd
      Andres Freund authored
      Reported-By: Gurjeet Singh
      a4b059fd
    • Andres Freund's avatar
      Handle PQresultErrorField(PG_DIAG_SQLSTATE) returning NULL in streamutil.c. · 7685963e
      Andres Freund authored
      In ff27db5d I missed that PQresultErrorField() may return NULL if
      there's no sqlstate associated with an error.
      
      Spotted-By: Coverity
      Reported-By: Michael Paquier
      Discussion: CAB7nPqQ3o10SY6NVdU4pjq85GQTN5tbbkq2gnNUh2fBNU3rKyQ@mail.gmail.com
      Backpatch: 9.5, like ff27db5d
      7685963e
    • Andres Freund's avatar
      Fix two off-by-one errors in bufmgr.c. · d25fbf9f
      Andres Freund authored
      In 4b4b680c I passed a buffer index number (starting from 0) instead of
      a proper Buffer id (which start from 1 for shared buffers) in two
      places.
      
      This wasn't noticed so far as one of those locations isn't compiled at
      all (PrintPinnedBufs) and the other one (InvalidBuffer) requires a
      unlikely, but possible, set of circumstances to trigger a symptom.
      
      To reduce the likelihood of such incidents a bit also convert existing
      open coded mappings from buffer descriptors to buffer ids with
      BufferDescriptorGetBuffer().
      
      Author: Qingqing Zhou
      Reported-By: Qingqing Zhou
      Discussion: CAJjS0u2ai9ooUisKtkV8cuVUtEkMTsbK8c7juNAjv8K11zeCQg@mail.gmail.com
      Backpatch: 9.5 where the private ref count infrastructure was introduced
      d25fbf9f
    • Tom Lane's avatar
      Fix some possible low-memory failures in regexp compilation. · 8a0258c3
      Tom Lane authored
      newnfa() failed to set the regex error state when malloc() fails.
      Several places in regcomp.c failed to check for an error after calling
      subre().  Each of these mistakes could lead to null-pointer-dereference
      crashes in memory-starved backends.
      
      Report and patch by Andreas Seltenreich.  Back-patch to all branches.
      8a0258c3
    • Tom Lane's avatar
      Postpone extParam/allParam calculations until the very end of planning. · 68fa28f7
      Tom Lane authored
      Until now we computed these Param ID sets at the end of subquery_planner,
      but that approach depends on subquery_planner returning a concrete Plan
      tree.  We would like to switch over to returning one or more Paths for a
      subquery, and in that representation the necessary details aren't fully
      fleshed out (not to mention that we don't really want to do this work for
      Paths that end up getting discarded).  Hence, refactor so that we can
      compute the param ID sets at the end of planning, just before
      set_plan_references is run.
      
      The main change necessary to make this work is that we need to capture
      the set of outer-level Param IDs available to the current query level
      before exiting subquery_planner, since the outer levels' plan_params lists
      are transient.  (That's not going to pose a problem for returning Paths,
      since all the work involved in producing that data is part of expression
      preprocessing, which will continue to happen before Paths are produced.)
      On the plus side, this change gets rid of several existing kluges.
      
      Eventually I'd like to get rid of SS_finalize_plan altogether in favor of
      doing this work during set_plan_references, but that will require some
      complex rejiggering because SS_finalize_plan needs to visit subplans and
      initplans before the main plan.  So leave that idea for another day.
      68fa28f7
  3. 11 Aug, 2015 7 commits
    • Alvaro Herrera's avatar
      Don't include rel.h when relcache.h is sufficient · 4901b2f4
      Alvaro Herrera authored
      Trivial change to reduce exposure of rel.h.
      4901b2f4
    • Tom Lane's avatar
      Fix broken markup, and copy-edit a bit. · 750fc78b
      Tom Lane authored
      Fix docs build failure introduced by commit 6fcd8851.
      I failed to resist the temptation to rearrange the description of
      pg_create_physical_replication_slot(), too.
      750fc78b
    • Andrew Dunstan's avatar
      15c3a1b4
    • Andres Freund's avatar
      Allow pg_create_physical_replication_slot() to reserve WAL. · 6fcd8851
      Andres Freund authored
      When creating a physical slot it's often useful to immediately reserve
      the current WAL position instead of only doing after the first feedback
      message arrives. That e.g. allows slots to guarantee that all the WAL
      for a base backup will be available afterwards.
      
      Logical slots already have to reserve WAL during creation, so generalize
      that logic into being usable for both physical and logical slots.
      
      Catversion bump because of the new parameter.
      
      Author: Gurjeet Singh
      Reviewed-By: Andres Freund
      Discussion: CABwTF4Wh_dBCzTU=49pFXR6coR4NW1ynb+vBqT+Po=7fuq5iCw@mail.gmail.com
      6fcd8851
    • Andres Freund's avatar
      Introduce macros determining if a replication slot is physical or logical. · 093d0c83
      Andres Freund authored
      These make the code a bit easier to read, and make it easier to add a
      more explicit notion of a slot's type at some point in the future.
      
      Author: Gurjeet Singh
      Discussion: CABwTF4Wh_dBCzTU=49pFXR6coR4NW1ynb+vBqT+Po=7fuq5iCw@mail.gmail.com
      093d0c83
    • Andres Freund's avatar
      Minor cleanups in slot related code. · 3b425b7c
      Andres Freund authored
      Fix a bunch of typos, and remove two superflous includes.
      
      Author: Gurjeet Singh
      Discussion: CABwTF4Wh_dBCzTU=49pFXR6coR4NW1ynb+vBqT+Po=7fuq5iCw@mail.gmail.com
      Backpatch: 9.4
      3b425b7c
    • Tom Lane's avatar
      Fix privilege dumping from servers too old to have that type of privilege. · b861678f
      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.
      b861678f
  4. 10 Aug, 2015 7 commits
    • Tom Lane's avatar
      Accept alternate spellings of __sparcv7 and __sparcv8. · 1f64ec6f
      Tom Lane authored
      Apparently some versions of gcc prefer __sparc_v7__ and __sparc_v8__.
      Per report from Waldemar Brodkorb.
      1f64ec6f
    • Tom Lane's avatar
      Further mucking with PlaceHolderVar-related restrictions on join order. · 4200a928
      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.
      4200a928
    • Andrew Dunstan's avatar
      Work around an apparent bug in the Msys DTK perl's regex engine. · e7293e32
      Andrew Dunstan authored
      Several versions of the perl that comes with the Msys DTK have been
      found to have a bug that fails to recognize a ' before a multiline $ in
      some circumstances. To work around the problem, use a character class
      for the '. Another solution would have been to use \n instead of $, but
      that would have changed the test semantics very slightly.
      e7293e32
    • Tom Lane's avatar
      Temporarily(?) remove BRIN isolation test. · 6a1e14c6
      Tom Lane authored
      Commit 2834855c added a not-very-carefully-thought-out isolation test
      to check a BRIN index bug fix.  The test depended on the availability
      of the pageinspect contrib module, which meant it did not work in
      several common testing scenarios such as "make check-world".  It's not
      clear whether we want a core test depending on a contrib module like
      that, but in any case, failing to deal with the possibility that the
      module isn't present in the installation-under-test is not acceptable.
      
      Remove that test pending some better solution.
      6a1e14c6
    • Andres Freund's avatar
      Add confirmed_flush column to pg_replication_slots. · 3f811c2d
      Andres Freund authored
      There's no reason not to expose both restart_lsn and confirmed_flush
      since they have rather distinct meanings. The former is the oldest WAL
      still required and valid for both physical and logical slots, whereas
      the latter is the location up to which a logical slot's consumer has
      confirmed receiving data. Most of the time a slot will require older
      WAL (i.e. restart_lsn) than the confirmed
      position (i.e. confirmed_flush_lsn).
      
      Author: Marko Tiikkaja, editorialized by me
      Discussion: 559D110B.1020109@joh.to
      3f811c2d
    • Andres Freund's avatar
      Fix copy & paste mistake in pg_get_replication_slots(). · 5c4b25ac
      Andres Freund authored
      XLogRecPtr was compared with InvalidTransactionId instead of
      InvalidXLogRecPtr. As both are defined to the same value this doesn't
      cause any actual problems, but it's still wrong.
      
      Backpatch: 9.4-master, bug was introduced in 9.4
      5c4b25ac
    • Andres Freund's avatar
      Don't start to stream after pg_receivexlog --create-slot. · 70fd0e14
      Andres Freund authored
      Immediately starting to stream after --create-slot is inconvenient in a
      number of situations (e.g. when configuring a slot for use in
      recovery.conf) and it's easy to just call pg_receivexlog twice in the
      rest of the cases.
      
      Author: Michael Paquier
      Discussion: CAB7nPqQ9qEtuDiKY3OpNzHcz5iUA+DUX9FcN9K8GUkCZvG7+Ew@mail.gmail.com
      Backpatch: 9.5, where the option was introduced
      70fd0e14
  5. 09 Aug, 2015 5 commits
    • Tom Lane's avatar
      Remove gram.y's precedence declaration for OVERLAPS. · 1e3e1ae2
      Tom Lane authored
      The allowed syntax for OVERLAPS, viz "row OVERLAPS row", is sufficiently
      constrained that we don't actually need a precedence declaration for
      OVERLAPS; indeed removing this declaration does not change the generated
      gram.c file at all.  Let's remove it to avoid confusion about whether
      OVERLAPS has precedence or not.  If we ever generalize what we allow for
      OVERLAPS, we might need to put back a precedence declaration for it,
      but we might want some other level than what it has today --- and leaving
      the declaration there would just risk confusion about whether that would
      be an incompatible change.
      
      Likewise, remove OVERLAPS from the documentation's precedence table.
      
      Per discussion with Noah Misch.  Back-patch to 9.5 where we hacked up some
      nearby precedence decisions.
      1e3e1ae2
    • Magnus Hagander's avatar
      Fix typo in LDAP example · 2a330d55
      Magnus Hagander authored
      Reported by William Meitzen
      2a330d55
    • Bruce Momjian's avatar
      d4aeb3de
    • Tatsuo Ishii's avatar
      Fix broken multibyte regression tests. · efc1610b
      Tatsuo Ishii authored
      commit 9043Fe390f4f0b4586cfe59cbd22314b9c3e2957 broke multibyte
      regression tests because the commit removes the warning message when
      temporary hash indexes is created, which has been added by commit
      07af5238.
      
      Back patched to 9.5 stable tree.
      efc1610b
    • Bruce Momjian's avatar
      docs: fix typo in rules.sgml · 08c6178a
      Bruce Momjian authored
      Report by Dean Rasheed
      
      Patch by Dean Rasheed
      
      Backpatch through 9.5
      08c6178a
  6. 08 Aug, 2015 2 commits
  7. 07 Aug, 2015 8 commits
    • Andres Freund's avatar
      Attempt to work around a 32bit xlc compiler bug from a different place. · 5a33650f
      Andres Freund authored
      In de6fd1c8 I moved the the work around from 53f73879 into the aix
      template. The previous location was removed in the former commit, and I
      thought that it would be nice to emit a warning when running configure.
      
      That didn't turn out to work because at the point the template is
      included we don't know whether we're compiling a 32/64 bit binary and
      it's possible to install compilers for both on a 64 bit kernel/OS.
      
      So go back to a less ambitious approach and define
      PG_FORCE_DISABLE_INLINE in port/aix.h, without emitting a warning. We
      could try a more fancy approach, but it doesn't seem worth it.
      
      This requires moving the check for PG_FORCE_DISABLE_INLINE in c.h to
      after including the system headers included from therein which isn't
      perfect, as it seems slightly more robust to include all system headers
      in a similar environment. Oh well.
      
      Discussion: 20150807132000.GC13310@awork2.anarazel.de
      5a33650f
    • Andres Freund's avatar
      Fix bug slowing down pgbench when -P is used. · c2509944
      Andres Freund authored
      A removed check in ba3deeef made all threads but the main one busy-loop
      when -P was used. All threads computed the time to the next time the
      progress report should be printed, but only the main thread did so and
      re-scheduled it only for the future.
      
      Reported-By: Jesper Pedersen
      Discussion: 55C4E190.3050104@redhat.com
      c2509944
    • Tom Lane's avatar
      Further adjustments to PlaceHolderVar removal. · 89db8392
      Tom Lane authored
      A new test case from Andreas Seltenreich showed that we were still a bit
      confused about removing PlaceHolderVars during join removal.  Specifically,
      remove_rel_from_query would remove a PHV that was used only underneath
      the removable join, even if the place where it's used was the join partner
      relation and not the join clause being deleted.  This would lead to a
      "too late to create a new PlaceHolderInfo" error later on.  We can defend
      against that by checking ph_eval_at to see if the PHV could possibly be
      getting used at some partner rel.
      
      Also improve some nearby LATERAL-related logic.  I decided that the check
      on ph_lateral needed to take precedence over the check on ph_needed, in
      case there's a lateral reference underneath the join being considered.
      (That may be impossible, but I'm not convinced of it, and it's easy enough
      to defend against the case.)  Also, I realized that remove_rel_from_query's
      logic for updating LateralJoinInfos is dead code, because we don't build
      those at all until after join removal.
      
      Back-patch to 9.3.  Previous versions didn't have the LATERAL issues, of
      course, and they also didn't attempt to remove PlaceHolderInfos during join
      removal.  (I'm starting to wonder if changing that was really such a great
      idea.)
      89db8392
    • Robert Haas's avatar
      Fix attach-related race condition in shm_mq_send_bytes. · 846f8c94
      Robert Haas authored
      Spotted by Antonin Houska.
      846f8c94
    • Andres Freund's avatar
      Don't include low level locking code from frontend code. · 4eda0a64
      Andres Freund authored
      Some frontend code like e.g. pg_xlogdump or pg_resetxlog, has to use
      backend headers. Unfortunately until now that code includes most of the
      locking code. It's generally not nice to expose such low level details,
      but de6fd1c8 made that a hard problem. We fall back to defining
      'inline' away if the compiler doesn't support it - that can cause linker
      errors like on buildfarm animal pademelon if a inline function
      references backend only code.
      
      To fix that problem separate definitions from lock.h that are required
      from frontend code into lockdefs.h and use it in the relevant
      places. I've only removed the minimal amount of necessary definitions
      for now - it might turn out that we want more for other reasons.
      
      To avoid such details being exposed again put some checks against being
      included from frontend code into atomics.h, lock.h, lwlock.h and
      s_lock.h. It's otherwise fairly easy to indirectly include these
      headers.
      
      Discussion: 20150806070902.GE12214@awork2.anarazel.de
      4eda0a64
    • Andres Freund's avatar
      Address points made in post-commit review of replication origins. · 18e86135
      Andres Freund authored
      Amit reviewed the replication origins patch and made some good
      points. Address them. This fixes typos in error messages, docs and
      comments and adds a missing error check (although in a
      should-never-happen scenario).
      
      Discussion: CAA4eK1JqUBVeWWKwUmBPryFaje4190ug0y-OAUHWQ6tD83V4xg@mail.gmail.com
      Backpatch: 9.5, where replication origins were introduced.
      18e86135
    • Bruce Momjian's avatar
      9.5 release notes: updates from Andres Freund and Jeff Janes · d6a8c943
      Bruce Momjian authored
      Report by Andres Freund and Jeff Janes
      
      Backpatch through 9.5
      d6a8c943
    • Tom Lane's avatar
      Fix old oversight in join removal logic. · bab163e1
      Tom Lane authored
      Commit 9e7e29c7 introduced an Assert that
      join removal didn't reduce the eval_at set of any PlaceHolderVar to empty.
      At first glance it looks like join_is_removable ensures that's true --- but
      actually, the loop in join_is_removable skips PlaceHolderVars that are not
      referenced above the join due to be removed.  So, if we don't want any
      empty eval_at sets, the right thing to do is to delete any now-unreferenced
      PlaceHolderVars from the data structure entirely.
      
      Per fuzz testing by Andreas Seltenreich.  Back-patch to 9.3 where the
      aforesaid Assert was added.
      bab163e1