1. 09 Jan, 2018 7 commits
    • Peter Eisentraut's avatar
      Fix ssl tests for when tls-server-end-point is not supported · c3d41ccf
      Peter Eisentraut authored
      Add a function to TestLib that allows us to check pg_config.h and then
      decide the expected test outcome based on that.
      
      Author: Michael Paquier <michael.paquier@gmail.com>
      c3d41ccf
    • Tom Lane's avatar
      Fix race condition during replication origin drop. · 8a906204
      Tom Lane authored
      replorigin_drop() misunderstood the API for condition variables: it
      had ConditionVariablePrepareToSleep and ConditionVariableCancelSleep
      inside its test-and-sleep loop, rather than outside the loop as
      intended.  The net effect is a narrow race-condition window wherein,
      if the process using a replication slot releases it immediately after
      replorigin_drop() releases the ReplicationOriginLock, replorigin_drop()
      would get into the condition variable's wait list too late and then
      wait indefinitely for a signal that won't come.
      
      Because there's a different CV for each replication slot, we can't
      just move the ConditionVariablePrepareToSleep call to above the
      test-and-sleep loop.  What we can do, in the wake of commit 13db3b93,
      is drop the ConditionVariablePrepareToSleep call entirely.  This fix
      depends on that commit because (at least in principle) the slot matching
      the target replication origin might move around, so that once in a blue
      moon successive loop iterations might involve different CVs.  We can now
      cope with such a scenario, at the cost of an extra trip through the
      retry loop.
      
      (There are ways we could fix this bug without depending on that commit,
      but they're all a lot more complicated than this way.)
      
      While at it, upgrade the rather skimpy comments in this function.
      
      Back-patch to v10 where this code came in.
      
      Discussion: https://postgr.es/m/19947.1515455433@sss.pgh.pa.us
      8a906204
    • Tom Lane's avatar
      Allow ConditionVariable[PrepareTo]Sleep to auto-switch between CVs. · 13db3b93
      Tom Lane authored
      The original coding here insisted that callers manually cancel any prepared
      sleep for one condition variable before starting a sleep on another one.
      While that's not a huge burden today, it seems like a gotcha that will bite
      us in future if the use of condition variables increases; anything we can
      do to make the use of this API simpler and more robust is attractive.
      Hence, allow these functions to automatically switch their attention to
      a different CV when required.  This is safe for the same reason it was OK
      for commit aced5a92 to let a broadcast operation cancel any prepared CV
      sleep: whenever we return to the other test-and-sleep loop, we will
      automatically re-prepare that CV, paying at most an extra test of that
      loop's exit condition.
      
      Back-patch to v10 where condition variables were introduced.  Ordinarily
      we would probably not back-patch a change like this, but since it does not
      invalidate any coding pattern that was legal before, it seems safe enough.
      Furthermore, there's an open bug in replorigin_drop() for which the
      simplest fix requires this.  Even if we chose to fix that in some more
      complicated way, the hazard would remain that we might back-patch some
      other bug fix that requires this behavior.
      
      Patch by me, reviewed by Thomas Munro.
      
      Discussion: https://postgr.es/m/2437.1515368316@sss.pgh.pa.us
      13db3b93
    • Robert Haas's avatar
      Don't allow VACUUM VERBOSE ANALYZE VERBOSE. · 921059bd
      Robert Haas authored
      There are plans to extend the syntax for ANALYZE, so we need to break
      the link between VacuumStmt and AnalyzeStmt.  But apart from that, the
      syntax above is undocumented and, if discovered by users, might give
      the impression that the VERBOSE option for VACUUM differs from the
      verbose option from ANALYZE, which it does not.
      
      Nathan Bossart, reviewed by Michael Paquier and Masahiko Sawada
      
      Discussion: http://postgr.es/m/D3FC73E2-9B1A-4DB4-8180-55F57D116B4E@amazon.com
      921059bd
    • Teodor Sigaev's avatar
      Improve scripting language in pgbench · bc7fa0c1
      Teodor Sigaev authored
      Added:
       - variable now might contain integer, double, boolean and null values
       - functions ln, exp
       - logical AND/OR/NOT
       - bitwise AND/OR/NOT/XOR
       - bit right/left shift
       - comparison operators
       - IS [NOT] (NULL|TRUE|FALSE)
       - conditional choice (in form of when/case/then)
      
      New operations and functions allow to implement more complicated test scenario.
      
      Author: Fabien Coelho with minor editorization by me
      Reviewed-By: Pavel Stehule, Jeevan Ladhe, me
      Discussion: https://www.postgresql.org/message-id/flat/alpine.DEB.2.10.1604030742390.31618@sto
      bc7fa0c1
    • Robert Haas's avatar
      Fix comment. · 63008b19
      Robert Haas authored
      RELATION_IS_OTHER_TEMP is tested in the caller, not here.
      
      Discussion: http://postgr.es/m/5A5438E4.3090709@lab.ntt.co.jp
      63008b19
    • Bruce Momjian's avatar
      pg_upgrade: prevent check on live cluster from generating error · d25ee300
      Bruce Momjian authored
      Previously an inaccurate but harmless error was generated when running
      --check on a live server before reporting the servers as compatible.
      The fix is to split error reporting and exit control in the exec_prog()
      API.
      
      Reported-by: Daniel Westermann
      
      Backpatch-through: 10
      d25ee300
  2. 08 Jan, 2018 3 commits
    • Tom Lane's avatar
    • Tom Lane's avatar
      Improve error detection capability in proclists. · ea8e1bbc
      Tom Lane authored
      Previously, although the initial state of a proclist_node is expected
      to be next == prev == 0, proclist_delete_offset would reset nodes to
      next == prev == INVALID_PGPROCNO when removing them from a list.
      This is the same state that a node in a singleton list has, so that
      it's impossible to distinguish not-in-a-list from in-a-list.  Change
      proclist_delete_offset to reset removed nodes to next == prev == 0,
      making it possible to distinguish those cases, and then add Asserts
      to the list add and delete functions that the supplied node isn't
      or is in a list at entry.  Also tighten assertions about the node
      being in the particular list (not some other one) where it is possible
      to check that in O(1) time.
      
      In ConditionVariablePrepareToSleep, since we don't expect the process's
      cvWaitLink to already be in a list, remove the more-or-less-useless
      proclist_contains check; we'd rather have proclist_push_tail's new
      assertion fire if that happens.
      
      Improve various comments related to proclists, too.
      
      Patch by me, reviewed by Thomas Munro.  This isn't back-patchable, since
      there could theoretically be inlined copies of proclist_delete_offset in
      third-party modules.  But it's only improving debuggability anyway.
      
      Discussion: https://postgr.es/m/CAEepm=0NWKehYw7NDoUSf8juuKOPRnCyY3vuaSvhrEWsOTAa3w@mail.gmail.com
      ea8e1bbc
    • Tom Lane's avatar
      Back off chattiness in RemovePgTempFiles(). · eeb3c2df
      Tom Lane authored
      In commit 561885db, as part of normalizing RemovePgTempFiles's error
      handling, I removed its behavior of silently ignoring ENOENT failures
      during directory opens.  Thomas Munro points out that this is a bad idea at
      the top level, because we don't create pgsql_tmp directories until needed.
      Thus this coding could produce LOG messages in perfectly normal situations,
      which isn't what I intended.  Restore the suppression of ENOENT logging,
      but only at top level --- it would still be unexpected for a nested temp
      directory to disappear between seeing it in the parent directory and
      opening it.
      
      Discussion: https://postgr.es/m/CAEepm=2y06SehAkTnd5sU_eVqdv5P-=Srt1y5vYNQk6yVDVaPw@mail.gmail.com
      eeb3c2df
  3. 06 Jan, 2018 5 commits
    • Simon Riggs's avatar
      Add TIMELINE to backup_label file · 6271fceb
      Simon Riggs authored
      Allows new test to confirm timelines match
      
      Author: Michael Paquier
      Reviewed-by: David Steele
      6271fceb
    • Simon Riggs's avatar
      Default monitoring roles - errata · 6668a54e
      Simon Riggs authored
      25fff407 introduced
      default monitoring roles. Apply these corrections:
      
      * Allow access to pg_stat_get_wal_senders()
        by role pg_read_all_stats
      
      * Correct comment in pg_stat_get_wal_receiver()
        to show it is no longer superuser-only.
      
      Author: Feike Steenbergen
      Reviewed-by: Michael Paquier
      
      Apply to HEAD, then later backpatch to 10
      6668a54e
    • Tom Lane's avatar
      Remove return values of ConditionVariableSignal/Broadcast. · ccf312a4
      Tom Lane authored
      In the wake of commit aced5a92, the semantics of these results are
      a bit squishy: we can tell whether we signaled some other process(es),
      but we do not know which ones were real waiters versus mere sentinels
      for ConditionVariableBroadcast operations.  It does not help much that
      ConditionVariableBroadcast will attempt to pass on the signal to the
      next real waiter, because (a) there might not be one, and (b) that will
      only happen awhile later, anyway.  So these results could overstate how
      much effect the calls really had.
      
      However, no existing caller of either function pays any attention to its
      result value, so it seems reasonable to just define that as a required
      property of a correct algorithm.  To encourage correctness and save some
      tiny number of cycles, change both functions to return void.
      
      Patch by me, per an observation by Thomas Munro.  No back-patch, since
      if any third parties happen to be using these functions, they might not
      appreciate an API break in a minor release.
      
      Discussion: https://postgr.es/m/CAEepm=0NWKehYw7NDoUSf8juuKOPRnCyY3vuaSvhrEWsOTAa3w@mail.gmail.com
      ccf312a4
    • Tom Lane's avatar
      Reorder steps in ConditionVariablePrepareToSleep for more safety. · 3cac0ec8
      Tom Lane authored
      In the admittedly-very-unlikely case that AddWaitEventToSet fails,
      ConditionVariablePrepareToSleep would error out after already having
      set cv_sleep_target, which is probably bad, and after having already
      set cv_wait_event_set, which is very bad.  Transaction abort might or
      might not clean up cv_sleep_target properly; but there is nothing
      that would be aware that the WaitEventSet wasn't fully constructed,
      so that all future condition variable sleeps would be broken.
      We can easily guard against these hazards with slight restructuring.
      
      Back-patch to v10 where condition_variable.c was introduced.
      
      Discussion: https://postgr.es/m/CAEepm=0NWKehYw7NDoUSf8juuKOPRnCyY3vuaSvhrEWsOTAa3w@mail.gmail.com
      3cac0ec8
    • Tom Lane's avatar
      Rewrite ConditionVariableBroadcast() to avoid live-lock. · aced5a92
      Tom Lane authored
      The original implementation of ConditionVariableBroadcast was, per its
      self-description, "the dumbest way possible".  Thomas Munro found out
      it was a bit too dumb.  An awakened process may immediately re-queue
      itself, if the specific condition it's waiting for is not yet satisfied.
      If this happens before ConditionVariableBroadcast is able to see the wait
      queue as empty, then ConditionVariableBroadcast will re-awaken the same
      process, repeating the cycle.  Given unlucky timing this back-and-forth
      can repeat indefinitely; loops lasting thousands of seconds have been
      seen in testing.
      
      To fix, add our own process to the end of the wait queue to serve as a
      sentinel, and exit the broadcast loop once our process is not there
      anymore.  There are various special considerations described in the
      comments, the principal disadvantage being that wakers can no longer
      be sure whether they awakened a real waiter or just a sentinel.  But in
      practice nobody pays attention to the result of ConditionVariableSignal
      or ConditionVariableBroadcast anyway, so that problem seems hypothetical.
      
      Back-patch to v10 where condition_variable.c was introduced.
      
      Tom Lane and Thomas Munro
      
      Discussion: https://postgr.es/m/CAEepm=0NWKehYw7NDoUSf8juuKOPRnCyY3vuaSvhrEWsOTAa3w@mail.gmail.com
      aced5a92
  4. 05 Jan, 2018 6 commits
  5. 04 Jan, 2018 11 commits
  6. 03 Jan, 2018 8 commits
    • Tom Lane's avatar
      Clean up tupdesc.c for recent changes. · 47c6772e
      Tom Lane authored
      TupleDescCopy needs to have the same effects as CreateTupleDescCopy in
      that, since it doesn't copy constraints, it should clear the per-attribute
      fields associated with them.  Oversight in commit cc5f8136.
      
      Since TupleDescCopy has already established the presumption that it
      can just flat-copy the entire attribute array in one go, propagate
      that approach into CreateTupleDescCopy and CreateTupleDescCopyConstr.
      (I'm suspicious that this would lead to valgrind complaints if we
      had any trailing padding in the struct, but we do not, and anyway
      fixing that seems like a job for a separate commit.)
      
      Add some better comments.
      
      Thomas Munro, reviewed by Vik Fearing, some additional hacking by me
      
      Discussion: https://postgr.es/m/CAEepm=0NvOGZ8B6GbQyQe2C_c2m3LKJ9w=8OMBaYRLgZ_Gw6Nw@mail.gmail.com
      47c6772e
    • Alvaro Herrera's avatar
      Fix typo · bab29698
      Alvaro Herrera authored
      Author: Dagfinn Ilmari Mannsåker
      Discussion: https://postgr.es/m/d8jefpk4jtd.fsf@dalvik.ping.uio.no
      bab29698
    • Alvaro Herrera's avatar
      Revert "Fix isolation test to be less timing-dependent" · 6c8be596
      Alvaro Herrera authored
      This reverts commit 2268e6af.  It turned out that inconsistency in
      the report is still possible, so go back to the simpler formulation of
      the test and instead add an alternate expected output.
      
      Discussion: https://postgr.es/m/20180103193728.ysqpcp2xjnqpiep7@alvherre.pgsql
      6c8be596
    • Andres Freund's avatar
      Rename pg_rewind's copy_file_range() to avoid conflict with new linux syscall. · 3e68686e
      Andres Freund authored
      Upcoming versions of glibc will contain copy_file_range(2), a wrapper
      around a new linux syscall for in-kernel copying of data ranges. This
      conflicts with pg_rewinds function of the same name.
      
      Therefore rename pg_rewinds version. As our version isn't a generic
      copying facility we decided to choose a rewind specific function name.
      
      Per buildfarm animal caiman and subsequent discussion with Tom Lane.
      
      Author: Andres Freund
      Discussion:
          https://postgr.es/m/20180103033425.w7jkljth3e26sduc@alap3.anarazel.de
          https://postgr.es/m/31122.1514951044@sss.pgh.pa.us
      Backpatch: 9.5-, where pg_rewind was introduced
      3e68686e
    • Andrew Dunstan's avatar
      Fix use of config-specific libraries for Windows OpenSSL · 99d5a3ff
      Andrew Dunstan authored
      Commit 614350a3 allowed for an different builds of OpenSSL libraries on
      Windows, but ignored the fact that the alternative builds don't have
      config-specific libraries. This patch fixes the Solution file to ask for
      the correct libraries.
      
      per offline discussions with Leonardo Cecchi and Marco Nenciarini,
      
      Backpatch to all live branches.
      99d5a3ff
    • Alvaro Herrera's avatar
      Make XactLockTableWait work for transactions that are not yet self-locked · 3c27944f
      Alvaro Herrera authored
      XactLockTableWait assumed that its xid argument has already added itself
      to the lock table.  That assumption led to another assumption that if
      locking the xid has succeeded but the xid is reported as still in
      progress, then the input xid must have been a subtransaction.
      
      These assumptions hold true for the original uses of this code in
      locking related to on-disk tuples, but they break down in logical
      replication slot snapshot building -- in particular, when a standby
      snapshot logged contains an xid that's already in ProcArray but not yet
      in the lock table.  This leads to assertion failures that can be
      reproduced all the way back to 9.4, when logical decoding was
      introduced.
      
      To fix, change SubTransGetParent to SubTransGetTopmostTransaction which
      has a slightly different API: it returns the argument Xid if there is no
      parent, and it goes all the way to the top instead of moving up the
      levels one by one.  Also, to avoid busy-waiting, add a 1ms sleep to give
      the other process time to register itself in the lock table.
      
      For consistency, change ConditionalXactLockTableWait the same way.
      
      Author: Petr Jelínek
      Discussion: https://postgr.es/m/1B3E32D8-FCF4-40B4-AEF9-5C0E3AC57969@postgrespro.ru
      Reported-by: Konstantin Knizhnik
      Diagnosed-by: Stas Kelvich, Petr Jelínek
      Reviewed-by: Andres Freund, Robert Haas
      3c27944f
    • Tom Lane's avatar
      Fix some minor errors in new PHJ code. · 6fcde240
      Tom Lane authored
      Correct ExecParallelHashTuplePrealloc's estimate of whether the
      space_allowed limit is exceeded.  Be more consistent about tuples that
      are exactly HASH_CHUNK_THRESHOLD in size (they're "small", not "large").
      Neither of these things explain the current buildfarm unhappiness, but
      they're still bugs.
      
      Thomas Munro, per gripe by me
      
      Discussion: https://postgr.es/m/CAEepm=34PDuR69kfYVhmZPgMdy8pSA-MYbpesEN1SR+2oj3Y+w@mail.gmail.com
      6fcde240
    • Tom Lane's avatar
      Teach eval_const_expressions() to handle some more cases. · 3decd150
      Tom Lane authored
      Add some infrastructure (mostly macros) to make it easier to write
      typical cases for constant-expression simplification.  Add simplification
      processing for ArrayRef, RowExpr, and ScalarArrayOpExpr node types,
      which formerly went unsimplified even if all their inputs were constants.
      Also teach it to simplify FieldSelect from a composite constant.
      Make use of the new infrastructure to reduce the amount of code needed
      for the existing ArrayExpr and ArrayCoerceExpr cases.
      
      One existing test case changes output as a result of the fact that
      RowExpr can now be folded to a constant.  All the new code is exercised
      by existing test cases according to gcov, so I feel no need to add
      additional tests.
      
      Tom Lane, reviewed by Dmitry Dolgov
      
      Discussion: https://postgr.es/m/3be3b82c-e29c-b674-2163-bf47d98817b1@iki.fi
      3decd150