1. 15 Jun, 2021 4 commits
  2. 14 Jun, 2021 6 commits
  3. 13 Jun, 2021 3 commits
    • Tom Lane's avatar
      Work around portability issue with newer versions of mktime(). · f807e341
      Tom Lane authored
      Recent glibc versions have made mktime() fail if tm_isdst is
      inconsistent with the prevailing timezone; in particular it fails for
      tm_isdst = 1 when the zone is UTC.  (This seems wildly inconsistent
      with the POSIX-mandated treatment of "incorrect" values for the other
      fields of struct tm, so if you ask me it's a bug, but I bet they'll
      say it's intentional.)  This has been observed to cause cosmetic
      problems when pg_restore'ing an archive created in a different
      timezone.
      
      To fix, do mktime() using the field values from the archive, and if
      that fails try again with tm_isdst = -1.  This will give a result
      that's off by the UTC-offset difference from the original zone, but
      that was true before, too.  It's not terribly critical since we don't
      do anything with the result except possibly print it.  (Someday we
      should flush this entire bit of logic and record a standard-format
      timestamp in the archive instead.  That's not okay for a back-patched
      bug fix, though.)
      
      Also, guard our only other use of mktime() by having initdb's
      build_time_t() set tm_isdst = -1 not 0.  This case could only have
      an issue in zones that are DST year-round; but I think some do exist,
      or could in future.
      
      Per report from Wells Oliver.  Back-patch to all supported
      versions, since any of them might need to run with a newer glibc.
      
      Discussion: https://postgr.es/m/CAOC+FBWDhDHO7G-i1_n_hjRzCnUeFO+H-Czi1y10mFhRWpBrew@mail.gmail.com
      f807e341
    • Andrew Dunstan's avatar
      Further tweaks to stuck_on_old_timeline recovery test · 9d97c340
      Andrew Dunstan authored
      Translate path slashes on target directory path. This was confusing old
      branches, but is applied to all branches for the sake of uniformity.
      Perl is perfectly able to understand paths with forward slashes.
      
      Along the way, restore the previous archive_wait query, for the sake of
      uniformity with other tests, per gripe from Tom Lane.
      9d97c340
    • Michael Paquier's avatar
      Ignore more environment variables in pg_regress.c · a9e0b3b0
      Michael Paquier authored
      This is similar to the work done in 8279f68a for TestLib.pm, where
      environment variables set may cause unwanted failures if using a
      temporary installation with pg_regress.  The list of variables reset is
      adjusted in each stable branch depending on what is supported.
      
      Comments are added to remember that the lists in TestLib.pm and
      pg_regress.c had better be kept in sync.
      
      Reviewed-by: Álvaro Herrera
      Discussion: https://postgr.es/m/YMNR9GYDn+fHlMta@paquier.xyz
      Backpatch-through: 9.6
      a9e0b3b0
  4. 12 Jun, 2021 8 commits
    • Tom Lane's avatar
      Restore robustness of TAP tests that wait for postmaster restart. · f452aaf7
      Tom Lane authored
      Several TAP tests use poll_query_until() to wait for the postmaster
      to restart.  They were checking to see if a trivial query
      (e.g. "SELECT 1") succeeds.  However, that's problematic in the wake
      of commit 11e9caff, because now that we feed said query to psql
      via stdin, we risk IPC::Run whining about a SIGPIPE failure if psql
      quits before reading the query.  Hence, we can't use a nonempty
      query in cases where we need to wait for connection failures to
      stop happening.
      
      Per the precedent of commits c757a3da and 6d41dd04, we can pass
      "undef" as the query in such cases to ensure that IPC::Run has
      nothing to write.  However, then we have to say that the expected
      output is empty, and this exposes a deficiency in poll_query_until:
      if psql fails altogether and returns empty stdout, poll_query_until
      will treat that as a success!  That's because, contrary to its
      documentation, it makes no actual check for psql failure, looking
      neither at the exit status nor at stderr.
      
      To fix that, adjust poll_query_until to insist on empty stderr as
      well as a stdout match.  (I experimented with checking exit status
      instead, but it seems that psql often does exit(1) in cases that we
      need to consider successes.  That might be something to fix someday,
      but it would be a non-back-patchable behavior change.)
      
      Back-patch to v10.  The test cases needing this exist only as far
      back as v11, but it seems wise to keep poll_query_until's behavior
      the same in v10, in case we back-patch another such test case in
      future.  (9.6 does not currently need this change, because in that
      branch poll_query_until can't be told to accept empty stdout as
      a success case.)
      
      Per assorted buildfarm failures, mostly on hoverfly.
      
      Discussion: https://postgr.es/m/CAA4eK1+zM6L4QSA1XMvXY_qqWwdUmqkOS1+hWvL8QcYEBGA1Uw@mail.gmail.com
      f452aaf7
    • Tom Lane's avatar
      Ensure pg_filenode_relation(0, 0) returns NULL. · 1250aad4
      Tom Lane authored
      Previously, a zero value for the relfilenode resulted in
      a confusing error message about "unexpected duplicate".
      This function returns NULL for other invalid relfilenode
      values, so zero should be treated likewise.
      
      It's been like this all along, so back-patch to all supported
      branches.
      
      Justin Pryzby
      
      Discussion: https://postgr.es/m/20210612023324.GT16435@telsasoft.com
      1250aad4
    • Tom Lane's avatar
      Don't use Asserts to check for violations of replication protocol. · fe6a20ce
      Tom Lane authored
      Using an Assert to check the validity of incoming messages is an
      extremely poor decision.  In a debug build, it should not be that easy
      for a broken or malicious remote client to crash the logrep worker.
      The consequences could be even worse in non-debug builds, which will
      fail to make such checks at all, leading to who-knows-what misbehavior.
      Hence, promote every Assert that could possibly be triggered by wrong
      or out-of-order replication messages to a full test-and-ereport.
      
      To avoid bloating the set of messages the translation team has to cope
      with, establish a policy that replication protocol violation error
      reports don't need to be translated.  Hence, all the new messages here
      use errmsg_internal().  A couple of old messages are changed likewise
      for consistency.
      
      Along the way, fix some non-idiomatic or outright wrong uses of
      hash_search().
      
      Most of these mistakes are new with the "streaming replication"
      patch (commit 46482432), but a couple go back a long way.
      Back-patch as appropriate.
      
      Discussion: https://postgr.es/m/1719083.1623351052@sss.pgh.pa.us
      fe6a20ce
    • Andrew Dunstan's avatar
      Fix new recovery test for use under msys · c3652f97
      Andrew Dunstan authored
      Commit caba8f0d wasn't quite right for msys, as demonstrated by
      several buildfarm animals, including jacana and fairywren. We need to
      use the msys perl in the archive command, but call it in such a way that
      Windows will understand the path. Furthermore, inside the copy script we
      need to convert a Windows path to an msys path.
      c3652f97
    • Michael Paquier's avatar
      Simplify some code in getObjectTypeDescription() · b56b83aa
      Michael Paquier authored
      This routine is designed to never return an empty description or NULL,
      providing description fallbacks even if missing objects are accepted,
      but it included a code path where this was considered possible.  All the
      callers of this routine already consider NULL as not possible, so
      change a bit the code to map with the assumptions of the callers, and
      add more comments close to the callers of this routine to outline the
      behavior expected.
      
      This code is new as of 2a10fdc4, so no backpatch is needed.
      
      Discussion: https://postgr.es/m/YMNY6RGPBRCeLmFb@paquier.xyz
      b56b83aa
    • Michael Paquier's avatar
      Improve log pattern detection in recently-added TAP tests · bfd96b7a
      Michael Paquier authored
      ab55d742 has introduced some tests with rows found as missing in logical
      replication subscriptions for partitioned tables, relying on a logic
      with a lookup of the logs generated, scanning the whole file.  This
      commit makes the logic more precise, by scanning the logs only from the
      position before the key queries are run to the position where we check
      for the logs.  This will reduce the risk of issues with log patterns
      overlapping with each other if those tests get more complicated in the
      future.
      
      Per discussion with Tom Lane.
      
      Discussion: https://postgr.es/m/YMP+Gx2S8meYYHW4@paquier.xyz
      Backpatch-through: 13
      bfd96b7a
    • Andres Freund's avatar
      Improve and cleanup ProcArrayAdd(), ProcArrayRemove(). · d8e950d3
      Andres Freund authored
      941697c3 changed ProcArrayAdd()/Remove() substantially. As reported by
      zhanyi, I missed that due to the introduction of the PGPROC->pgxactoff
      ProcArrayRemove() does not need to search for the position in
      procArray->pgprocnos anymore - that's pgxactoff. Remove the search loop.
      
      The removal of the search loop reduces assertion coverage a bit - but we can
      easily do better than before by adding assertions to other loops over
      pgprocnos elements.
      
      Also do a bit of cleanup, mostly by reducing line lengths by introducing local
      helper variables and adding newlines.
      
      Author: zhanyi <w@hidva.com>
      Author: Andres Freund <andres@anarazel.de>
      Discussion: https://postgr.es/m/tencent_5624AA3B116B3D1C31CA9744@qq.com
      d8e950d3
    • Bruce Momjian's avatar
      doc: remove extra right angle bracket in PG 14 relnotes · d120e66f
      Bruce Momjian authored
      Reported-by: Justin Pryzby
      d120e66f
  5. 11 Jun, 2021 11 commits
  6. 10 Jun, 2021 7 commits
    • Tom Lane's avatar
      Reconsider the handling of procedure OUT parameters. · e56bce5d
      Tom Lane authored
      Commit 2453ea14 redefined pg_proc.proargtypes to include the types of
      OUT parameters, for procedures only.  While that had some advantages
      for implementing the SQL-spec behavior of DROP PROCEDURE, it was pretty
      disastrous from a number of other perspectives.  Notably, since the
      primary key of pg_proc is name + proargtypes, this made it possible to
      have multiple procedures with identical names + input arguments and
      differing output argument types.  That would make it impossible to call
      any one of the procedures by writing just NULL (or "?", or any other
      data-type-free notation) for the output argument(s).  The change also
      seems likely to cause grave confusion for client applications that
      examine pg_proc and expect the traditional definition of proargtypes.
      
      Hence, revert the definition of proargtypes to what it was, and
      undo a number of complications that had been added to support that.
      
      To support the SQL-spec behavior of DROP PROCEDURE, when there are
      no argmode markers in the command's parameter list, we perform the
      lookup both ways (that is, matching against both proargtypes and
      proallargtypes), succeeding if we get just one unique match.
      In principle this could result in ambiguous-function failures
      that would not happen when using only one of the two rules.
      However, overloading of procedure names is thought to be a pretty
      rare usage, so this shouldn't cause many problems in practice.
      Postgres-specific code such as pg_dump can defend against any
      possibility of such failures by being careful to specify argmodes
      for all procedure arguments.
      
      This also fixes a few other bugs in the area of CALL statements
      with named parameters, and improves the documentation a little.
      
      catversion bump forced because the representation of procedures
      with OUT arguments changes.
      
      Discussion: https://postgr.es/m/3742981.1621533210@sss.pgh.pa.us
      e56bce5d
    • Tom Lane's avatar
      Rearrange logrep worker's snapshot handling some more. · 3a09d75b
      Tom Lane authored
      It turns out that worker.c's code path for TRUNCATE was also
      careless about establishing a snapshot while executing user-defined
      code, allowing the checks added by commit 84f5c290 to fail when
      a trigger is fired in that context.
      
      We could just wrap Push/PopActiveSnapshot around the truncate call,
      but it seems better to establish a policy of holding a snapshot
      throughout execution of a replication step.  To help with that and
      possible future requirements, replace the previous ensure_transaction
      calls with pairs of begin/end_replication_step calls.
      
      Per report from Mark Dilger.  Back-patch to v11, like the previous
      changes.
      
      Discussion: https://postgr.es/m/B4A3AF82-79ED-4F4C-A4E5-CD2622098972@enterprisedb.com
      3a09d75b
    • Tom Lane's avatar
      Shut down EvalPlanQual machinery when LockRows node reaches the end. · bb4aed46
      Tom Lane authored
      Previously, we left the EPQ sub-executor alone until ExecEndLockRows.
      This caused any buffer pins or other resources that it might hold to
      remain held until ExecutorEnd, which in some code paths means that
      they are held till the Portal is closed.  That can cause user-visible
      problems, such as blocking VACUUM; and it's unlike the behavior of
      ordinary table-scanning nodes, which will have released all buffer
      pins by the time they return an EOF indication.
      
      We can make LockRows work more like other plan nodes by calling
      EvalPlanQualEnd just before returning NULL.  We still need to call it
      in ExecEndLockRows in case the node was not run to completion, but in
      the normal case the second call does nothing and costs little.
      
      Per report from Yura Sokolov.  In principle this is a longstanding
      bug, but in view of the lack of other complaints and the low severity
      of the consequences, I chose not to back-patch.
      
      Discussion: https://postgr.es/m/4aa370cb91ecf2f9885d98b80ad1109c@postgrespro.ru
      bb4aed46
    • Tom Lane's avatar
      Avoid ECPG test failures in some GSS-capable environments. · 9bb5eecc
      Tom Lane authored
      Buildfarm member hamerkop has been reporting that two cases in
      connect/test5.pgc show different error messages than the test expects,
      because since commit ffa2e467 libpq's connection failure messages
      are exposing the fact that a GSS-encrypted connection was attempted
      and failed.  That's pretty interesting information in itself, and
      I certainly don't wish to shoot the messenger, but we need to do
      something to stabilize the ECPG results.
      
      For the second of these two failure cases, we can add the
      gssencmode=disable option to prevent the discrepancy.  However,
      that solution is problematic for the first failure, because the only
      unique thing about that case is that it's testing a completely-omitted
      connection target; there's noplace to add the option without defeating
      the point of the test case.  After some thrashing around with
      alternative fixes that turned out to have undesirable side-effects,
      the most workable answer is just to give up and remove that test case.
      Perhaps we can revert this later, if we figure out why the GSS code
      is misbehaving in hamerkop's environment.
      
      Thanks to Michael Paquier for exploration of alternatives.
      
      Discussion: https://postgr.es/m/YLRZH6CWs9N6Pusy@paquier.xyz
      9bb5eecc
    • Peter Eisentraut's avatar
      Add some const decorations · b29fa951
      Peter Eisentraut authored
      One of these functions is new in PostgreSQL 14; might as well start it
      out right.
      b29fa951
    • Robert Haas's avatar
      Adjust new test case to set wal_keep_size. · 4dcb1d08
      Robert Haas authored
      Per buildfarm member conchuela and Kyotaro Horiguchi, it's possible
      for the WAL segment that the cascading standby needs to be removed
      too quickly. Hopefully this will prevent that.
      
      Kyotaro Horiguchi
      
      Discussion: http://postgr.es/m/20210610.101240.1270925505780628275.horikyota.ntt@gmail.com
      4dcb1d08
    • David Rowley's avatar
  7. 09 Jun, 2021 1 commit
    • Robert Haas's avatar
      Fix corner case failure of new standby to follow new primary. · caba8f0d
      Robert Haas authored
      This only happens if (1) the new standby has no WAL available locally,
      (2) the new standby is starting from the old timeline, (3) the promotion
      happened in the WAL segment from which the new standby is starting,
      (4) the timeline history file for the new timeline is available from
      the archive but the WAL files for are not (i.e. this is a race),
      (5) the WAL files for the new timeline are available via streaming,
      and (6) recovery_target_timeline='latest'.
      
      Commit ee994272 introduced this
      logic and was an improvement over the previous code, but it mishandled
      this case. If recovery_target_timeline='latest' and restore_command is
      set, validateRecoveryParameters() can change recoveryTargetTLI to be
      different from receiveTLI. If streaming is then tried afterward,
      expectedTLEs gets initialized with the history of the wrong timeline.
      It's supposed to be a list of entries explaining how to get to the
      target timeline, but in this case it ends up with a list of entries
      explaining how to get to the new standby's original timeline, which
      isn't right.
      
      Dilip Kumar and Robert Haas, reviewed by Kyotaro Horiguchi.
      
      Discussion: http://postgr.es/m/CAFiTN-sE-jr=LB8jQuxeqikd-Ux+jHiXyh4YDiZMPedgQKup0g@mail.gmail.com
      caba8f0d