1. 29 Jul, 2021 4 commits
    • Tom Lane's avatar
      Improve libpq's handling of OOM during error message construction. · 43f1d2ab
      Tom Lane authored
      Commit ffa2e467 changed libpq so that multiple error reports
      occurring during one operation (a connection attempt or query)
      are accumulated in conn->errorMessage, where before new ones
      usually replaced any prior error.  At least in theory, that makes
      us more vulnerable to running out of memory for the errorMessage
      buffer.  If it did happen, the user would be left with just an
      empty-string error report, which is pretty unhelpful.
      
      We can improve this by relying on pqexpbuffer.c's existing "broken
      buffer" convention to track whether we've hit OOM for the current
      operation's error string, and then substituting a constant "out of
      memory" string in the small number of places where the errorMessage
      is read out.
      
      While at it, apply the same method to similar OOM cases in
      pqInternalNotice and pqGetErrorNotice3.
      
      Back-patch to v14 where ffa2e467 came in.  In principle this could
      go back further; but in view of the lack of field reports, the
      hazard seems negligible in older branches.
      
      Discussion: https://postgr.es/m/530153.1627425648@sss.pgh.pa.us
      43f1d2ab
    • Andrew Dunstan's avatar
      Avoid calling TestLib::perl2host on a symlinked directory · c42d3d04
      Andrew Dunstan authored
      Certain versions of msys2/Windows have been observed to resolve symlinks
      in perl2host rather than just follow them. This defeats using a
      symlinked shorter path to a longer path, and makes certain tests fail.
      We therefore call perl2host on the parent directory of the symlink and
      thereafter just use that result.
      
      Apply to release 14 where the problem has been observed.
      c42d3d04
    • Andrew Dunstan's avatar
      Make TestLib::perl2host more consistent and robust · 68011e17
      Andrew Dunstan authored
      Sometimes cygpath has been observed to return a path with a trailing
      slash. That can cause problems, Also, make "cygpath" usage
      consistent with "pwd -W" with respect to the use of forward slashes.
      
      Backpatch to release 14 where the current code was introduced.
      68011e17
    • Michael Paquier's avatar
      Add missing exit() in pg_verifybackup when failing to find pg_waldump · 67445deb
      Michael Paquier authored
      pg_verifybackup needs by default pg_waldump to check after a range of
      WAL segments required for a backup, except if --no-parse-wal is
      specified.  The code checked for the presence of the binary pg_waldump
      in an installation and reported an error, but it forgot to properly
      exit().  This could lead to confusing errors reported.
      
      Reviewed-by: Robert Haas, Fabien Coelho
      Discussion: https://postgr.es/m/YQDMdB+B68yePFeT@paquier.xyz
      Backpatch-through: 13
      67445deb
  2. 28 Jul, 2021 3 commits
  3. 27 Jul, 2021 5 commits
  4. 26 Jul, 2021 5 commits
  5. 25 Jul, 2021 2 commits
    • Tom Lane's avatar
      Get rid of artificial restriction on hash table sizes on Windows. · b154ee63
      Tom Lane authored
      The point of introducing the hash_mem_multiplier GUC was to let users
      reproduce the old behavior of hash aggregation, i.e. that it could use
      more than work_mem at need.  However, the implementation failed to get
      the job done on Win64, where work_mem is clamped to 2GB to protect
      various places that calculate memory sizes using "long int".  As
      written, the same clamp was applied to hash_mem.  This resulted in
      severe performance regressions for queries requiring a bit more than
      2GB for hash aggregation, as they now spill to disk and there's no
      way to stop that.
      
      Getting rid of the work_mem restriction seems like a good idea, but
      it's a big job and could not conceivably be back-patched.  However,
      there's only a fairly small number of places that are concerned with
      the hash_mem value, and it turns out to be possible to remove the
      restriction there without too much code churn or any ABI breaks.
      So, let's do that for now to fix the regression, and leave the
      larger task for another day.
      
      This patch does introduce a bit more infrastructure that should help
      with the larger task, namely pg_bitutils.h support for working with
      size_t values.
      
      Per gripe from Laurent Hasson.  Back-patch to v13 where the
      behavior change came in.
      
      Discussion: https://postgr.es/m/997817.1627074924@sss.pgh.pa.us
      Discussion: https://postgr.es/m/MN2PR15MB25601E80A9B6D1BA6F592B1985E39@MN2PR15MB2560.namprd15.prod.outlook.com
      b154ee63
    • Andres Freund's avatar
      Deduplicate choice of horizon for a relation procarray.c. · 3d0a4636
      Andres Freund authored
      5a1e1d83 was a minimal bug fix for dc7420c2. To avoid future bugs of
      that kind, deduplicate the choice of a relation's horizon into a new helper,
      GlobalVisHorizonKindForRel().
      
      As the code in question was only introduced in dc7420c2 it seems worth
      backpatching this change as well, otherwise 14 will look different from all
      other branches.
      
      A different approach to this was suggested by Matthias van de Meent.
      
      Author: Andres Freund
      Discussion: https://postgr.es/m/20210621122919.2qhu3pfugxxp3cji@alap3.anarazel.de
      Backpatch: 14, like 5a1e1d83
      3d0a4636
  6. 24 Jul, 2021 3 commits
    • Tom Lane's avatar
      Fix check for conflicting session- vs transaction-level locks. · 712ba6b8
      Tom Lane authored
      We have an implementation restriction that PREPARE TRANSACTION can't
      handle cases where both session-lifespan and transaction-lifespan locks
      are held on the same lockable object.  (That's because we'd otherwise
      need to acquire a new PROCLOCK entry during post-prepare cleanup, which
      is an operation that might fail.  The situation can only arise with odd
      usages of advisory locks, so removing the restriction is probably not
      worth the amount of effort it would take.)  AtPrepare_Locks attempted
      to enforce this, but its logic was many bricks shy of a load, because
      it only detected cases where the session and transaction locks had the
      same lockmode.  Locks of different modes on the same object would lead
      to the rather unhelpful message "PANIC: we seem to have dropped a bit
      somewhere".
      
      To fix, build a transient hashtable with one entry per locktag,
      not one per locktag + mode, and use that to detect conflicts.
      
      Per bug #17122 from Alexander Pyhalov.  This bug is ancient,
      so back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/17122-04f3c32098a62233@postgresql.org
      712ba6b8
    • Tom Lane's avatar
      Make printf("%s", NULL) print "(null)" instead of crashing. · 89ad14cd
      Tom Lane authored
      We previously took a hard-line attitude that callers should never print
      a null string pointer, and doing so is worthy of an assertion failure
      or crash.  However, we've long since flushed out any easy-to-find bugs
      of that nature.  What remains is a lot of code that perhaps could fail
      that way in hard-to-reach corner cases.  For example, in something as
      simple as
          ereport(ERROR,
                  (errcode(ERRCODE_UNDEFINED_OBJECT),
                   errmsg("constraint \"%s\" for table \"%s\" does not exist",
                          conname, get_rel_name(relid))));
      one must wonder whether it's completely guaranteed that get_rel_name
      cannot return NULL in this context.  If such a situation did occur,
      the existing policy converts what might be a pretty minor bug into
      a server crash condition.  This is not good for robustness.
      
      Hence, let's follow the lead of glibc and print "(null)" instead
      of failing.  We should, of course, still consider it a bug if that
      behavior is reachable in ordinary use; but crashing seems less
      desirable than not crashing.
      
      This fix works across-the-board in v12 and up, where we always use
      src/port/snprintf.c.  Before that, on most platforms we're at the mercy
      of the local libc, but it appears that Solaris 10 is the only supported
      platform where we'd still get a crash.  Most other platforms such as
      *BSD, macOS, and Solaris 11 have adopted glibc's behavior at some
      point.  (AIX and HPUX just print "" not "(null)", but that's close
      enough.)  I've not checked what Windows' native printf would do, but
      it doesn't matter because we've long used snprintf.c on that platform.
      
      In v12 and up, also const-ify related code so that we're not casting
      away const on the constant string.  This is just neatnik-ism, since
      next to no compilers will warn about that.
      
      Discussion: https://postgr.es/m/17098-b960f3616c861f83@postgresql.org
      89ad14cd
    • Tom Lane's avatar
      Remove configure-time thread safety checking (thread_test.c). · d5e913a8
      Tom Lane authored
      This testing was useful when it was written, nigh twenty years ago,
      but it seems fairly pointless for any platform built in the last
      dozen or more years.  (Compare also the comments at 8a212118.)
      Also we now have reports that the test program itself fails under
      ThreadSanitizer.  Rather than invest effort in fixing it, let's
      just drop it, and assume that the few people who still care
      already know they need to use --disable-thread-safety.
      
      Back-patch into v14, for consistency with 8a212118.
      
      Discussion: https://postgr.es/m/CADhDkKzPSiNvA3Hyq+wSR_icuPmazG0cFe=YnC3U-CFcYLc8Xw@mail.gmail.com
      d5e913a8
  7. 22 Jul, 2021 2 commits
  8. 21 Jul, 2021 4 commits
  9. 20 Jul, 2021 5 commits
    • Bruce Momjian's avatar
      doc: PG 14 relnote adjustments · f8d1333d
      Bruce Momjian authored
      Reported-by: Elena Indrupskaya
      
      Discussion: https://postgr.es/m/38555778-a56b-4aca-2581-e05582fc9bcf@postgrespro.ru
      
      Author: Elena Indrupskaya
      
      Backpatch-through: 14 only
      f8d1333d
    • Tom Lane's avatar
      Fix corner-case uninitialized-variable issues in plpgsql. · 899564e0
      Tom Lane authored
      If an error was raised during our initial attempt to check whether
      a successfully-compiled expression is "simple", subsequent calls of
      exec_stmt_execsql would suppose that stmt->mod_stmt was already computed
      when it had not been.  This could lead to assertion failures in debug
      builds; in production builds the effect would typically be to act as
      if INTO STRICT had been specified even when it had not been.  Of course
      that only matters if the subsequent attempt to execute the expression
      succeeds, so that the problem can only be reached by fixing a failure
      in some referenced, inline-able SQL function and then retrying the
      calling plpgsql function in the same session.
      
      (There might be even-more-obscure ways to change the expression's
      behavior without changing the plpgsql function, but that one seems
      like the only one people would be likely to hit in practice.)
      
      The most foolproof way to fix this would be to arrange for
      exec_prepare_plan to not set expr->plan until we've finished the
      subsidiary simple-expression check.  But it seems hard to do that
      without creating reference-count leak issues.  So settle for documenting
      the hazard in a comment and fixing exec_stmt_execsql to test separately
      for whether it's computed stmt->mod_stmt.  (That adds a test-and-branch
      per execution, but hopefully that's negligible in context.)  In v11 and
      up, also fix exec_stmt_call which had a variant of the same issue.
      
      Per bug #17113 from Alexander Lakhin.  Back-patch to all
      supported branches.
      
      Discussion: https://postgr.es/m/17113-077605ce00e0e7ec@postgresql.org
      899564e0
    • Michael Paquier's avatar
      Fix some issues with WAL segment opening for pg_receivewal --compress · 3a0d2d0c
      Michael Paquier authored
      The logic handling the opening of new WAL segments was fuzzy when using
      --compress if a partial, non-compressed, segment with the same base name
      existed in the repository storing those files.  In this case, using
      --compress would cause the code to first check for the existence and the
      size of a non-compressed segment, followed by the opening of a new
      compressed, partial, segment.  The code was accidentally working
      correctly on most platforms as the buildfarm has proved, except
      bowerbird where gzflush() could fail in this code path.  It is wrong
      anyway to take the code path used pre-padding when creating a new
      partial, non-compressed, segment, so let's fix it.
      
      Note that this issue exists when users mix successive runs of
      pg_receivewal with or without compression, as discovered with the tests
      introduced by ffc9dda.
      
      While on it, this refactors the code so as code paths that need to know
      about the ".gz" suffix are down from four to one in walmethods.c, easing
      a bit the introduction of new compression methods.  This addresses a
      second issue where log messages generated for an unexpected failure
      would not show the compressed segment name involved, which was
      confusing, printing instead the name of the non-compressed equivalent.
      
      Reported-by: Georgios Kokolatos
      Discussion: https://postgr.es/m/YPDLz2x3o1aX2wRh@paquier.xyz
      Backpatch-through: 10
      3a0d2d0c
    • Peter Geoghegan's avatar
      Doc: vacuum_multixact_failsafe_age is multixact-based. · e1cdf617
      Peter Geoghegan authored
      Oversight in commit 1e55e7d1, which added a wraparound failsafe
      mechanism to VACUUM.
      
      Backpatch: 14-, where VACUUM failsafe was introduced.
      e1cdf617
    • Peter Geoghegan's avatar
      vacuumdb: Correct comment about --force-index-cleanup. · 9a3d41a2
      Peter Geoghegan authored
      Commit 3499df0d added a comment that incorrectly suggested that
      --force-index-cleanup did not appear in the same major version as the
      similar --no-index-cleanup option.  In fact, both options are new to
      PostgreSQL 14.
      
      Backpatch: 14-, where both options were introduced.
      9a3d41a2
  10. 19 Jul, 2021 3 commits
  11. 18 Jul, 2021 1 commit
    • Alexander Korotkov's avatar
      Support for unnest(multirange) · 244ad541
      Alexander Korotkov authored
      It has been spotted that multiranges lack of ability to decompose them into
      individual ranges.  Subscription and proper expanded object representation
      require substantial work, and it's too late for v14.  This commit
      provides the implementation of unnest(multirange), which is quite trivial.
      unnest(multirange) is defined as a polymorphic procedure.
      
      Catversion is bumped.
      
      Reported-by: Jonathan S. Katz
      Discussion: https://postgr.es/m/flat/60258efe-bd7e-4886-82e1-196e0cac5433%40postgresql.org
      Author: Alexander Korotkov
      Reviewed-by: Justin Pryzby, Jonathan S. Katz, Zhihong Yu, Tom Lane
      Reviewed-by: Alvaro Herrera
      244ad541
  12. 17 Jul, 2021 2 commits
  13. 16 Jul, 2021 1 commit