1. 13 Jan, 2015 3 commits
    • Andres Freund's avatar
      Add barriers to the latch code. · 14e8803f
      Andres Freund authored
      Since their introduction latches have required barriers in SetLatch
      and ResetLatch - but when they were introduced there wasn't any
      barrier abstraction. Instead latches were documented to rely on the
      callsites to provide barrier semantics.
      
      Now that the barrier support looks halfway complete, add the necessary
      barriers to both latch implementations.
      
      Also remove a now superflous lock acquisition from syncrep.c and a
      superflous (and insufficient) barrier from freelist.c. There might be
      other cases that can now be simplified, but those are the only ones
      I've seen on a quick scan.
      
      We might want to backpatch this at some later point, but right now the
      barrier infrastructure in the backbranches isn't totally on par with
      master.
      
      Discussion: 20150112154026.GB2092@awork2.anarazel.de
      14e8803f
    • Andres Freund's avatar
      Allow latches to wait for socket writability without waiting for readability. · 4bad60e3
      Andres Freund authored
      So far WaitLatchOrSocket() required to pass in WL_SOCKET_READABLE as
      that solely was used to indicate error conditions, like EOF. Waiting
      for WL_SOCKET_WRITEABLE would have meant to busy wait upon socket
      errors.
      
      Adjust the API to signal errors by returning the socket as readable,
      writable or both, depending on WL_SOCKET_READABLE/WL_SOCKET_WRITEABLE
      being specified.  It would arguably be nicer to return WL_SOCKET_ERROR
      but that's not possible on platforms and would probably also result in
      more complex callsites.
      
      This previously had explicitly been forbidden in e42a21b9, as
      there was no strong use case at that point. We now are looking into
      making FE/BE communication use latches, so changing this makes sense.
      
      There also are some portability concerns because there cases of older
      platforms where select(2) is known to, in violation of POSIX, not
      return a socket as writable after the peer has closed it.  So far the
      platforms where that's the case provide a working poll(2). If we find
      one where that's not the case, we'll need to add a workaround for that
      platform.
      
      Discussion: 20140927191243.GD5423@alap3.anarazel.de
      Reviewed-By: Heikki Linnakangas, Noah Misch
      4bad60e3
    • Heikki Linnakangas's avatar
      Fix typos in comment. · 3dfce376
      Heikki Linnakangas authored
      Plus some tiny wordsmithing of not-quite-typos.
      3dfce376
  2. 12 Jan, 2015 6 commits
    • Tom Lane's avatar
      Fix some functions that were declared static then defined not-static. · 7391e251
      Tom Lane authored
      Per testing with a compiler that whines about this.
      7391e251
    • Tom Lane's avatar
      Avoid unexpected slowdown in vacuum regression test. · 5b3ce2c9
      Tom Lane authored
      I noticed the "vacuum" regression test taking really significantly longer
      than it used to on a slow machine.  Investigation pointed the finger at
      commit e415b469, which added creation of
      an index using an extremely expensive index function.  That function was
      evidently meant to be applied only twice ... but the test re-used an
      existing test table, which up till a couple lines before that had had over
      two thousand rows.  Depending on timing of the concurrent regression tests,
      the intervening VACUUMs might have been unable to remove those
      recently-dead rows, and then the index build would need to create index
      entries for them too, leading to the wrap_do_analyze() function being
      executed 2000+ times not twice.  Avoid this by using a different table
      that is guaranteed to have only the intended two rows in it.
      
      Back-patch to 9.0, like the commit that created the problem.
      5b3ce2c9
    • Alvaro Herrera's avatar
      Tweak heapam's rmgr desc output slightly · d126e1e9
      Alvaro Herrera authored
      Some spaces were missing, and putting the affected tuple offset first in
      the lock cases instead of the locking data makes more sense.
      
      No backpatch since this is cosmetic and surrounding code has changed.
      d126e1e9
    • Alvaro Herrera's avatar
      Fix get_object_address argument type for extension statement · 5c5ffee8
      Alvaro Herrera authored
      Commit 3f88672a neglected to update the AlterExtensionContentsStmt
      production in the grammar to use TypeName to represent types when
      passing objects to get_object_address.
      
      Reported as a pg_upgrade failure by Jeff Janes.
      5c5ffee8
    • Tom Lane's avatar
      Use correct text domain for errcontext() appearing within ereport(). · 1f9bf05e
      Tom Lane authored
      The mechanism added in commit dbdf9679
      for associating the correct translation domain with errcontext strings
      potentially fails in cases where errcontext() is used within an ereport()
      macro.  Such usage was not originally envisioned for errcontext(), but we
      do have a few places that do it.  In this situation, the intended comma
      expression becomes just a couple of arguments to errfinish(), which the
      compiler might choose to evaluate right-to-left.
      
      Fortunately, in such cases the textdomain for the errcontext string must
      be the same as for the surrounding ereport.  So we can fix this by letting
      errstart initialize context_domain along with domain; then it will have
      the correct value no matter which order the calls occur in.  (Note that
      error stack callback functions are not invoked until errfinish, so normal
      usage of errcontext won't affect what happens for errcontext calls within
      the ereport macro.)
      
      In passing, make sure that errcontext calls within the main backend set
      context_domain to something non-NULL.  This isn't a live bug because
      NULL would select the current textdomain() setting which should be the
      right thing anyway --- but it seems better to handle this completely
      consistently with the regular domain field.
      
      Per report from Dmitry Voronin.  Backpatch to 9.3; before that, there
      wasn't any attempt to ensure that errcontext strings were translated
      in an appropriate domain.
      1f9bf05e
    • Stephen Frost's avatar
      Skip dead backends in MinimumActiveBackends · 1bf4a84d
      Stephen Frost authored
      Back in ed0b409d, PGPROC was split and moved to static variables in
      procarray.c, with procs in ProcArrayStruct replaced by an array of
      integers representing process numbers (pgprocnos), with -1 indicating a
      dead process which has yet to be removed.  Access to procArray is
      generally done under ProcArrayLock and therefore most code does not have
      to concern itself with -1 entries.
      
      However, MinimumActiveBackends intentionally does not take
      ProcArrayLock, which means it has to be extra careful when accessing
      procArray.  Prior to ed0b409d, this was handled by checking for a NULL
      in the pointer array, but that check was no longer valid after the
      split.  Coverity pointed out that the check could never happen and so
      it was removed in 5592ebac.  That didn't make anything worse, but it
      didn't fix the issue either.
      
      The correct fix is to check for pgprocno == -1 and skip over that entry
      if it is encountered.
      
      Back-patch to 9.2, since there can be attempts to access the arrays
      prior to their start otherwise.  Note that the changes prior to 9.4 will
      look a bit different due to the change in 5592ebac.
      
      Note that MinimumActiveBackends only returns a bool for heuristic
      purposes and any pre-array accesses are strictly read-only and so there
      is no security implication and the lack of fields complaints indicates
      it's very unlikely to run into issues due to this.
      
      Pointed out by Noah.
      1bf4a84d
  3. 11 Jan, 2015 5 commits
    • Tom Lane's avatar
      Fix portability breakage in pg_dump. · 44096f1c
      Tom Lane authored
      Commit 0eea8047 introduced some overly
      optimistic assumptions about what could be in a local struct variable's
      initializer.  (This might in fact be valid code according to C99, but I've
      got at least one pre-C99 compiler that falls over on those nonconstant
      address expressions.)  There is no reason whatsoever for main()'s workspace
      to not be static, so revert long_options[] to a static and make the
      DumpOptions struct static as well.
      44096f1c
    • Tom Lane's avatar
      Remove configure test for nonstandard variants of getpwuid_r(). · 8883bae3
      Tom Lane authored
      We had code that supposed that some platforms might offer a nonstandard
      version of getpwuid_r() with only four arguments.  However, the 5-argument
      definition has been standardized at least since the Single Unix Spec v2,
      which is our normal reference for what's portable across all Unix-oid
      platforms.  (What's more, this wasn't the only pre-standardization version
      of getpwuid_r(); my old HPUX 10.20 box has still another signature.)
      So let's just get rid of the now-useless configure step.
      8883bae3
    • Tom Lane's avatar
      Fix libpq's behavior when /etc/passwd isn't readable. · 080eabe2
      Tom Lane authored
      Some users run their applications in chroot environments that lack an
      /etc/passwd file.  This means that the current UID's user name and home
      directory are not obtainable.  libpq used to be all right with that,
      so long as the database role name to use was specified explicitly.
      But commit a4c8f143 broke such cases by
      causing any failure of pg_fe_getauthname() to be treated as a hard error.
      In any case it did little to advance its nominal goal of causing errors
      in pg_fe_getauthname() to be reported better.  So revert that and instead
      put some real error-reporting code in place.  This requires changes to the
      APIs of pg_fe_getauthname() and pqGetpwuid(), since the latter had
      departed from the POSIX-specified API of getpwuid_r() in a way that made
      it impossible to distinguish actual lookup errors from "no such user".
      
      To allow such failures to be reported, while not failing if the caller
      supplies a role name, add a second call of pg_fe_getauthname() in
      connectOptions2().  This is a tad ugly, and could perhaps be avoided with
      some refactoring of PQsetdbLogin(), but I'll leave that idea for later.
      (Note that the complained-of misbehavior only occurs in PQsetdbLogin,
      not when using the PQconnect functions, because in the latter we will
      never bother to call pg_fe_getauthname() if the user gives a role name.)
      
      In passing also clean up the Windows-side usage of GetUserName(): the
      recommended buffer size is 257 bytes, the passed buffer length should
      be the buffer size not buffer size less 1, and any error is reported
      by GetLastError() not errno.
      
      Per report from Christoph Berg.  Back-patch to 9.4 where the chroot
      failure case was introduced.  The generally poor reporting of errors
      here is of very long standing, of course, but given the lack of field
      complaints about it we won't risk changing these APIs further back
      (even though they're theoretically internal to libpq).
      080eabe2
    • Andres Freund's avatar
      Provide a generic fallback for pg_compiler_barrier using an extern function. · de6429a8
      Andres Freund authored
      If the compiler/arch combination does not provide compiler barriers,
      provide a fallback. That fallback simply consists out of a function
      call into a externally defined function.  That should guarantee
      compiler barrierer semantics except for compilers that do inter
      translation unit/global optimization - those better provide an actual
      compiler barrier.
      
      Hopefully this fixes Tom's report of linker failures due to
      pg_compiler_barrier_impl not being provided.
      
      I'm not backpatching this commit as it builds on the new atomics
      infrastructure. If we decide an equivalent fix needs to be
      backpatched, I'll do so in a separate commit.
      
      Discussion: 27746.1420930690@sss.pgh.pa.us
      
      Per report from Tom Lane.
      de6429a8
    • Andres Freund's avatar
      Fix alignment of pg_atomic_uint64 variables on some 32bit platforms. · db4ec2ff
      Andres Freund authored
      I failed to recognize that pg_atomic_uint64 wasn't guaranteed to be 8
      byte aligned on some 32bit platforms - which it has to be on some
      platforms to guarantee the desired atomicity and which we assert.
      
      As this is all compiler specific code anyway we can just rely on
      compiler specific tricks to enforce alignment.
      
      I've been unable to find concrete documentation about the version that
      introduce the sunpro alignment support, so that might need additional
      guards.
      
      I've verified that this works with gcc x86 32bit, but I don't have
      access to any other 32bit environment.
      
      Discussion: op.xpsjdkil0sbe7t@vld-kuci
      
      Per report from Vladimir Koković.
      db4ec2ff
  4. 10 Jan, 2015 1 commit
  5. 09 Jan, 2015 3 commits
  6. 08 Jan, 2015 8 commits
    • Stephen Frost's avatar
      Move rowsecurity event trigger test · c219cbfe
      Stephen Frost authored
      The event trigger test for rowsecurity can cause problems for other
      tests which are run in parallel with it.  Instead of running that test
      in the rowsecurity set, move it to the event_trigger set, which runs
      isolated from other tests.
      
      Also reverts 7161b082, which moved rowsecurity into its own test group.
      That's no longer necessary, now that the event trigger test is gone from
      the rowsecurity set of tests.
      
      Pointed out by Tom.
      c219cbfe
    • Andres Freund's avatar
      Remove comment that was intended to have been removed before commit. · f454144a
      Andres Freund authored
      Noticed by Amit Kapila
      f454144a
    • Andres Freund's avatar
      Move comment about sun cc's __machine_rw_barrier being a full barrier. · 93be0950
      Andres Freund authored
      I'd accidentally written the comment besides the read barrier, instead
      of the full barrier, implementation.
      
      Noticed by Oskari Saarenmaa
      93be0950
    • Andres Freund's avatar
      Fix logging of pages skipped due to pins during vacuum. · 17eaae98
      Andres Freund authored
      The new logging introduced in 35192f06 made the incorrect assumption
      that scan_all vacuums would always wait for buffer pins; but they only
      do so if the page actually needs to be frozen.
      
      Fix that inaccuracy by removing the difference in log output based on
      scan_all and just always remove the same message.  I chose to keep the
      split log message from the original commit for now, it seems likely
      that it'll be of use in the future.
      
      Also merge the line about buffer pins in autovacuum's log output into
      the existing "pages: ..." line. It seems odd to have a separate line
      about pins, without the "topic: " prefix others have.
      
      Also rename the new 'pinned_pages' variable to 'pinskipped_pages'
      because it actually tracks the number of pages that could *not* be
      pinned.
      
      Discussion: 20150104005324.GC9626@awork2.anarazel.de
      17eaae98
    • Noah Misch's avatar
      On Darwin, refuse postmaster startup when multithreaded. · 2048e5b8
      Noah Misch authored
      The previous commit introduced its report at LOG level to avoid
      surprises at minor release upgrade time.  Compel users deploying the
      next major release to also deploy the reported workaround.
      2048e5b8
    • Noah Misch's avatar
      On Darwin, detect and report a multithreaded postmaster. · 894459e5
      Noah Misch authored
      Darwin --enable-nls builds use a substitute setlocale() that may start a
      thread.  Buildfarm member orangutan experienced BackendList corruption
      on account of different postmaster threads executing signal handlers
      simultaneously.  Furthermore, a multithreaded postmaster risks undefined
      behavior from sigprocmask() and fork().  Emit LOG messages about the
      problem and its workaround.  Back-patch to 9.0 (all supported versions).
      894459e5
    • Noah Misch's avatar
      Always set the six locale category environment variables in main(). · 6fdba8ce
      Noah Misch authored
      Typical server invocations already achieved that.  Invalid locale
      settings in the initial postmaster environment interfered, as could
      malloc() failure.  Setting "LC_MESSAGES=pt_BR.utf8 LC_ALL=invalid" in
      the postmaster environment will now choose C-locale messages, not
      Brazilian Portuguese messages.  Most localized programs, including all
      PostgreSQL frontend executables, do likewise.  Users are unlikely to
      observe changes involving locale categories other than LC_MESSAGES.
      CheckMyDatabase() ensures that we successfully set LC_COLLATE and
      LC_CTYPE; main() sets the remaining three categories to locale "C",
      which almost cannot fail.  Back-patch to 9.0 (all supported versions).
      6fdba8ce
    • Noah Misch's avatar
      Reject ANALYZE commands during VACUUM FULL or another ANALYZE. · e415b469
      Noah Misch authored
      vacuum()'s static variable handling makes it non-reentrant; an ensuing
      null pointer deference crashed the backend.  Back-patch to 9.0 (all
      supported versions).
      e415b469
  7. 07 Jan, 2015 3 commits
    • Robert Haas's avatar
      docs: Reword CREATE POLICY documentation. · 39f2594b
      Robert Haas authored
      39f2594b
    • Heikki Linnakangas's avatar
      Don't open a WAL segment for writing at end of recovery. · 1e78d81e
      Heikki Linnakangas authored
      Since commit ba94518a, we used XLogFileOpen to open the next segment for
      writing, but if the end-of-recovery happens exactly at a segment boundary,
      the new segment might not exist yet. (Before ba94518a, XLogFileOpen was
      correct, because we would open the previous segment if the switch happened
      at the boundary.)
      
      Instead of trying to create it if necessary, it's simpler to not bother
      opening the segment at all. XLogWrite() will open or create it soon anyway,
      after writing the checkpoint or end-of-recovery record.
      
      Reported by Andres Freund.
      1e78d81e
    • Peter Eisentraut's avatar
      Fix namespace handling in xpath function · 79af9a1d
      Peter Eisentraut authored
      Previously, the xml value resulting from an xpath query would not have
      namespace declarations if the namespace declarations were attached to
      an ancestor element in the input xml value.  That means the output value
      was not correct XML.  Fix that by running the result value through
      xmlCopyNode(), which produces the correct namespace declarations.
      
      Author: Ali Akbar <the.apaan@gmail.com>
      79af9a1d
  8. 06 Jan, 2015 8 commits
    • Andres Freund's avatar
      Correctly handle relcache invalidation corner case during logical decoding. · 3fabed07
      Andres Freund authored
      When using a historic snapshot for logical decoding it can validly
      happen that a relation that's in the relcache isn't visible to that
      historic snapshot.  E.g. if a newly created relation is referenced in
      the query that uses the SQL interface for logical decoding and a
      sinval reset occurs.
      
      The earlier commit that fixed the error handling for that corner case
      already improves the situation as a ERROR is better than hitting an
      assertion... But it's obviously not good enough.  So additionally
      allow that case without an error if a historic snapshot is set up -
      that won't allow an invalid entry to stay in the cache because it's a)
      already marked invalid and will thus be rebuilt during the next access
      b) the syscaches will be reset at the end of decoding.
      
      There might be prettier solutions to handle this case, but all that we
      could think of so far end up being much more complex than this quite
      simple fix.
      
      This fixes the assertion failures reported by the buildfarm (markhor,
      tick, leech) after the introduction of new regression tests in
      89fd41b3. The failure there weren't actually directly caused by
      CLOBBER_CACHE_ALWAYS but the extraordinary long runtimes due to it
      lead to sinval resets triggering the behaviour.
      
      Discussion: 22459.1418656530@sss.pgh.pa.us
      
      Backpatch to 9.4 where logical decoding was introduced.
      3fabed07
    • Andres Freund's avatar
      Improve relcache invalidation handling of currently invisible relations. · 31912d01
      Andres Freund authored
      The corner case where a relcache invalidation tried to rebuild the
      entry for a referenced relation but couldn't find it in the catalog
      wasn't correct.
      
      The code tried to RelationCacheDelete/RelationDestroyRelation the
      entry. That didn't work when assertions are enabled because the latter
      contains an assertion ensuring the refcount is zero. It's also more
      generally a bad idea, because by virtue of being referenced somebody
      might actually look at the entry, which is possible if the error is
      trapped and handled via a subtransaction abort.
      
      Instead just error out, without deleting the entry. As the entry is
      marked invalid, the worst that can happen is that the invalid (and at
      some point unused) entry lingers in the relcache.
      
      Discussion: 22459.1418656530@sss.pgh.pa.us
      
      There should be no way to hit this case < 9.4 where logical decoding
      introduced a bug that can hit this. But since the code for handling
      the corner case is there it should do something halfway sane, so
      backpatch all the the way back.  The logical decoding bug will be
      handled in a separate commit.
      31912d01
    • Bruce Momjian's avatar
      Document that Perl's Tie might add a trailing newline · cb075178
      Bruce Momjian authored
      Report by Stefan Kaltenbrunner
      cb075178
    • Alvaro Herrera's avatar
      Fix thinko in plpython error message · 91539c56
      Alvaro Herrera authored
      91539c56
    • Bruce Momjian's avatar
      29c18d91
    • Bruce Momjian's avatar
      338c10b7
    • Bruce Momjian's avatar
      Update copyright for 2015 · 4baaf863
      Bruce Momjian authored
      Backpatch certain files through 9.0
      4baaf863
    • Tom Lane's avatar
      Fix broken pg_dump code for dumping comments on event triggers. · adfc157d
      Tom Lane authored
      This never worked, I think.  Per report from Marc Munro.
      
      In passing, fix funny spacing in the COMMENT ON command as a result of
      excess space in the "label" string.
      adfc157d
  9. 05 Jan, 2015 2 commits
    • Andres Freund's avatar
      Fix oversight in recent pg_basebackup fix causing pg_receivexlog failures. · 3c9e4cdb
      Andres Freund authored
      A oversight in 2c0a4858 causes 'could not create archive status file
      "...": No such file or directory' errors in pg_receivexlog if the
      target directory doesn't happen to contain a archive_status
      directory. That's due to a stupidly left over 'true' constant instead
      of mark_done being passed down to ProcessXLogDataMsg().
      
      The bug is only present in the master branch, and luckily wasn't
      released.
      
      Spotted by Fujii Masao.
      3c9e4cdb
    • Fujii Masao's avatar
      Fix typo in comment. · 9f1d7313
      Fujii Masao authored
      Report by Amit Kapila
      9f1d7313
  10. 04 Jan, 2015 1 commit
    • Alvaro Herrera's avatar
      Fix thinko in lock mode enum · d5e3d1e9
      Alvaro Herrera authored
      Commit 0e5680f4 contained a thinko
      mixing LOCKMODE with LockTupleMode.  This caused misbehavior in the case
      where a tuple is marked with a multixact with at most a FOR SHARE lock,
      and another transaction tries to acquire a FOR NO KEY EXCLUSIVE lock;
      this case should block but doesn't.
      
      Include a new isolation tester spec file to explicitely try all the
      tuple lock combinations; without the fix it shows the problem:
      
          starting permutation: s1_begin s1_lcksvpt s1_tuplock2 s2_tuplock3 s1_commit
          step s1_begin: BEGIN;
          step s1_lcksvpt: SELECT * FROM multixact_conflict FOR KEY SHARE; SAVEPOINT foo;
          a
      
          1
          step s1_tuplock2: SELECT * FROM multixact_conflict FOR SHARE;
          a
      
          1
          step s2_tuplock3: SELECT * FROM multixact_conflict FOR NO KEY UPDATE;
          a
      
          1
          step s1_commit: COMMIT;
      
      With the fixed code, step s2_tuplock3 blocks until session 1 commits,
      which is the correct behavior.
      
      All other cases behave correctly.
      
      Backpatch to 9.3, like the commit that introduced the problem.
      d5e3d1e9