1. 13 Sep, 2021 7 commits
  2. 11 Sep, 2021 1 commit
    • Tom Lane's avatar
      Make pg_regexec() robust against out-of-range search_start. · b33283cb
      Tom Lane authored
      If search_start is greater than the length of the string, we should just
      return REG_NOMATCH immediately.  (Note that the equality case should
      *not* be rejected, since the pattern might be able to match zero
      characters.)  This guards various internal assumptions that the min of a
      range of string positions is not more than the max.  Violation of those
      assumptions could allow an attempt to fetch string[search_start-1],
      possibly causing a crash.
      
      Jaime Casanova pointed out that this situation is reachable with the
      new regexp_xxx functions that accept a user-specified start position.
      I don't believe it's reachable via any in-core call site in v14 and
      below.  However, extensions could possibly call pg_regexec with an
      out-of-range search_start, so let's back-patch the fix anyway.
      
      Discussion: https://postgr.es/m/20210911180357.GA6870@ahch-to
      b33283cb
  3. 10 Sep, 2021 1 commit
    • Tom Lane's avatar
      Fix some anomalies with NO SCROLL cursors. · d844cd75
      Tom Lane authored
      We have long forbidden fetching backwards from a NO SCROLL cursor,
      but the prohibition didn't extend to cases in which we rewind the
      query altogether and then re-fetch forwards.  I think the reason is
      that this logic was mainly meant to protect plan nodes that can't
      be run in the reverse direction.  However, re-reading the query output
      is problematic if the query is volatile (which includes SELECT FOR
      UPDATE, not just queries with volatile functions): the re-read can
      produce different results, which confuses the cursor navigation logic
      completely.  Another reason for disliking this approach is that some
      code paths will either fetch backwards or rewind-and-fetch-forwards
      depending on the distance to the target row; so that seemingly
      identical use-cases may or may not draw the "cursor can only scan
      forward" error.  Hence, let's clean things up by disallowing rewind
      as well as fetch-backwards in a NO SCROLL cursor.
      
      Ordinarily we'd only make such a definitional change in HEAD, but
      there is a third reason to consider this change now.  Commit ba2c6d6c
      created some new user-visible anomalies for non-scrollable cursors
      WITH HOLD, in that navigation in the cursor result got confused if the
      cursor had been partially read before committing.  The only good way
      to resolve those anomalies is to forbid rewinding such a cursor, which
      allows removal of the incorrect cursor state manipulations that
      ba2c6d6c added to PersistHoldablePortal.
      
      To minimize the behavioral change in the back branches (including
      v14), refuse to rewind a NO SCROLL cursor only when it has a holdStore,
      ie has been held over from a previous transaction due to WITH HOLD.
      This should avoid breaking most applications that have been sloppy
      about whether to declare cursors as scrollable.  We'll enforce the
      prohibition across-the-board beginning in v15.
      
      Back-patch to v11, as ba2c6d6c was.
      
      Discussion: https://postgr.es/m/3712911.1631207435@sss.pgh.pa.us
      d844cd75
  4. 09 Sep, 2021 4 commits
    • Tom Lane's avatar
      Avoid fetching from an already-terminated plan. · b7056c0a
      Tom Lane authored
      Some plan node types don't react well to being called again after
      they've already returned NULL.  PortalRunSelect() has long dealt
      with this by calling the executor with NoMovementScanDirection
      if it sees that we've already run the portal to the end.  However,
      commit ba2c6d6c overlooked this point, so that persisting an
      already-fully-fetched cursor would fail if it had such a plan.
      
      Per report from Tomas Barton.  Back-patch to v11, as the faulty
      commit was.  (I've omitted a test case because the type of plan
      that causes a problem isn't all that stable.)
      
      Discussion: https://postgr.es/m/CAPV2KRjd=ErgVGbvO2Ty20tKTEZZr6cYsYLxgN_W3eAo9pf5sw@mail.gmail.com
      b7056c0a
    • Fujii Masao's avatar
      pgbench: Stop counting skipped transactions as soon as timer is exceeded. · b27d0cd3
      Fujii Masao authored
      When throttling is used, transactions that lag behind schedule by
      more than the latency limit are counted and reported as skipped.
      Previously, there was the case where pgbench counted all skipped
      transactions even if the timer specified in -T option was exceeded.
      This could take a very long time to do that especially when unrealistically
      high rate setting in -R option caused quite a lot of transactions that
      lagged behind schedule. This could prevent pgbench from ending
      immediately, and so pgbench could look like it got stuck to users.
      
      To fix the issue, this commit changes pgbench so that it stops counting
      skipped transactions as soon as the timer is exceeded. The timer can
      make pgbench end soon even when there are lots of skipped transactions
      that have not been counted yet.
      
      Note that there is no guarantee that all skipped transactions are
      counted under -T though there is under -t. This is OK in practice
      because it's very unlikely to happen with realistic setting. Also this is
      not the issue that this commit newly introdues. There used to be
      the case where pgbench ended without counting all skipped
      transactions since before.
      
      Back-patch to v14. Per discussion, we decided not to bother
      back-patch to the stable branches because it's hard to imagine
      the issue happens in practice (with realistic setting).
      
      Author: Yugo Nagata, Fabien COELHO
      Reviewed-by: Greg Sabino Mullane, Fujii Masao
      Discussion: https://postgr.es/m/20210613040151.265ff59d832f835bbcf8b3ba@sraoss.co.jp
      b27d0cd3
    • Tom Lane's avatar
      Check for relation length overrun soon enough. · 7430c774
      Tom Lane authored
      We don't allow relations to exceed 2^32-1 blocks, because block
      numbers are 32 bits and the last possible block number is reserved
      to mean InvalidBlockNumber.  There is a check for this in mdextend,
      but that's really way too late, because the smgr API requires us to
      create a buffer for the block-to-be-added, and we do not want to
      have any buffer with blocknum InvalidBlockNumber.  (Such a case
      can trigger assertions in bufmgr.c, plus I think it might confuse
      ReadBuffer's logic for data-past-EOF later on.)  So put the check
      into ReadBuffer.
      
      Per report from Christoph Berg.  It's been like this forever,
      so back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/YTn1iTkUYBZfcODk@msg.credativ.de
      7430c774
    • Fujii Masao's avatar
      Fix issue with WAL archiving in standby. · b5ec22bf
      Fujii Masao authored
      Previously, walreceiver always closed the currently-opened WAL segment
      and created its archive notification file, after it finished writing
      the current segment up and received any WAL data that should be
      written into the next segment. If walreceiver exited just before
      any WAL data in the next segment arrived at standby, it did not
      create the archive notification file of the current segment
      even though that's known completed. This behavior could cause
      WAL archiving of the segment to be delayed until subsequent
      restartpoints or checkpoints created its notification file.
      
      To fix the issue, this commit changes walreceiver so that it creates
      an archive notification file of a current WAL segment immediately
      if that's known completed before receiving next WAL data.
      
      Back-patch to all supported branches.
      
      Reported-by: Kyotaro Horiguchi
      Author: Fujii Masao
      Reviewed-by: Kyotaro Horiguchi
      Discussion: https://postgr.es/m/20200630.165503.1465894182551545886.horikyota.ntt@gmail.com
      b5ec22bf
  5. 08 Sep, 2021 5 commits
    • Tom Lane's avatar
      Avoid useless malloc/free traffic around getFormattedTypeName(). · 52c300df
      Tom Lane authored
      Coverity complained that one caller of getFormattedTypeName() failed
      to free the returned string.  Which is true, but rather than fixing
      that one, let's get rid of this tedious and error-prone requirement.
      Now that getFormattedTypeName() caches its result, strdup'ing that
      result and expecting the caller to free it accomplishes little except
      to waste cycles.  We do create a leak in the case where getTypes didn't
      make a TypeInfo for the type, but that basically shouldn't ever happen.
      
      Back-patch, as commit 6c450a861 was.  This isn't a particularly
      interesting bug fix, but the API change seems like a hazard for
      future back-patching activity if we don't back-patch it.
      52c300df
    • Tom Lane's avatar
      Fix misleading comments about TOAST access macros. · 67948a43
      Tom Lane authored
      Seems to have been my error in commit aeb1631e.
      Noted by Christoph Berg.
      
      Discussion: https://postgr.es/m/YTeLipdnSOg4NNcI@msg.df7cb.de
      67948a43
    • Tom Lane's avatar
      Fix rewriter to set hasModifyingCTE correctly on rewritten queries. · 03d01d74
      Tom Lane authored
      If we copy data-modifying CTEs from the original query to a replacement
      query (from a DO INSTEAD rule), we must set hasModifyingCTE properly
      in the replacement query.  Failure to do this can cause various
      unpleasantness, such as unsafe usage of parallel plans.  The code also
      neglected to propagate hasRecursive, though that's only cosmetic at
      the moment.
      
      A difficulty arises if the rule action is an INSERT...SELECT.  We
      attach the original query's RTEs and CTEs to the sub-SELECT Query, but
      data-modifying CTEs are only allowed to appear in the topmost Query.
      For the moment, throw an error in such cases.  It would probably be
      possible to avoid this error by attaching the CTEs to the top INSERT
      Query instead; but that would require a bunch of new code to adjust
      ctelevelsup references.  Given the narrowness of the use-case, and
      the need to back-patch this fix, it does not seem worth the trouble
      for now.  We can revisit this if we get field complaints.
      
      Per report from Greg Nancarrow.  Back-patch to all supported branches.
      (The test case added here does not fail before v10, but there are
      plenty of places checking top-level hasModifyingCTE in 9.6, so I have
      no doubt that this code change is necessary there too.)
      
      Greg Nancarrow and Tom Lane
      
      Discussion: https://postgr.es/m/CAJcOf-f68DT=26YAMz_i0+Au3TcLO5oiHY5=fL6Sfuits6r+_w@mail.gmail.com
      Discussion: https://postgr.es/m/CAJcOf-fAdj=nDKMsRhQzndm-O13NY4dL6xGcEvdX5Xvbbi0V7g@mail.gmail.com
      03d01d74
    • Peter Eisentraut's avatar
      Disable anonymous record hash support except in special cases · 054adca6
      Peter Eisentraut authored
      Commit 01e658fa added hash support for row types.  This also added
      support for hashing anonymous record types, using the same approach
      that the type cache uses for comparison support for record types: It
      just reports that it works, but it might fail at run time if a
      component type doesn't actually support the operation.  We get away
      with that for comparison because most types support that.  But some
      types don't support hashing, so the current state can result in
      failures at run time where the planner chooses hashing over sorting,
      whereas that previously worked if only sorting was an option.
      
      We do, however, want the record hashing support for path tracking in
      recursive unions, and the SEARCH and CYCLE clauses built on that.  In
      that case, hashing is the only plan option.  So enable that, this
      commit implements the following approach: The type cache does not
      report that hashing is available for the record type.  This undoes
      that part of 01e658fa.  Instead, callers that require hashing no
      matter what can override that result themselves.  This patch only
      touches the callers to make the aforementioned recursive query cases
      work, namely the parse analysis of unions, as well as the hash_array()
      function.
      Reported-by: default avatarSait Talha Nisanci <sait.nisanci@microsoft.com>
      Bug: #17158
      Discussion: https://www.postgresql.org/message-id/flat/17158-8a2ba823982537a4%40postgresql.org
      054adca6
    • Amit Kapila's avatar
      Invalidate relcache for publications defined for all tables. · 8db27fbc
      Amit Kapila authored
      Updates/Deletes on a relation were allowed even without replica identity
      after we define the publication for all tables. This would later lead to
      an error on subscribers. The reason was that for such publications we were
      not invalidating the relcache and the publication information for
      relations was not getting rebuilt. Similarly, we were not invalidating the
      relcache after dropping of such publications which will prohibit
      Updates/Deletes without replica identity even without any publication.
      
      Author: Vignesh C and Hou Zhijie
      Reviewed-by: Hou Zhijie, Kyotaro Horiguchi, Amit Kapila
      Backpatch-through: 10, where it was introduced
      Discussion: https://postgr.es/m/CALDaNm0pF6zeWqCA8TCe2sDuwFAy8fCqba=nHampCKag-qLixg@mail.gmail.com
      8db27fbc
  6. 07 Sep, 2021 3 commits
  7. 06 Sep, 2021 3 commits
  8. 04 Sep, 2021 4 commits
  9. 03 Sep, 2021 3 commits
    • Tom Lane's avatar
      Disallow creating an ICU collation if the DB encoding won't support it. · 2cc018ba
      Tom Lane authored
      Previously this was allowed, but the collation effectively vanished
      into the ether because of the way lookup_collation() works: you could
      not use the collation, nor even drop it.  Seems better to give an
      error up front than to leave the user wondering why it doesn't work.
      
      (Because this test is in DefineCollation not CreateCollation, it does
      not prevent pg_import_system_collations from creating ICU collations,
      regardless of the initially-chosen encoding.)
      
      Per bug #17170 from Andrew Bille.  Back-patch to v10 where ICU support
      was added.
      
      Discussion: https://postgr.es/m/17170-95845cf3f0a9c36d@postgresql.org
      2cc018ba
    • John Naylor's avatar
      Set the volatility of the timestamptz version of date_bin() back to immutable · 67c33a11
      John Naylor authored
      543f36b43d was too hasty in thinking that the volatility of date_bin()
      had to match date_trunc(), since only the latter references
      session_timezone.
      
      Bump catversion
      
      Per feedback from Aleksander Alekseev
      Backpatch to v14, as the former commit was
      67c33a11
    • Tom Lane's avatar
      Fix portability issue in tests from commit ce773f230. · 08b96a2b
      Tom Lane authored
      Modern POSIX seems to require strtod() to accept "-NaN", but there's
      nothing about NaN in SUSv2, and some of our oldest buildfarm members
      don't like it.  Let's try writing it as -'NaN' instead; that seems
      to produce the same result, at least on Intel hardware.
      
      Per buildfarm.
      08b96a2b
  10. 02 Sep, 2021 3 commits
    • Tom Lane's avatar
      In count_usable_fds(), duplicate stderr not stdin. · 6b54f123
      Tom Lane authored
      We had a complaint that the postmaster fails to start if the invoking
      program closes stdin.  That happens because count_usable_fds expects
      to be able to dup(0), and if it can't, we conclude there are no free
      FDs and go belly-up.  So far as I can find, though, there is no other
      place in the server that touches stdin, and it's not unreasonable to
      expect that a daemon wouldn't use that file.
      
      As a simple improvement, let's dup FD 2 (stderr) instead.  Unlike stdin,
      it *is* reasonable for us to expect that stderr be open; even if we are
      configured not to touch it, common libraries such as libc might try to
      write error messages there.
      
      Per gripe from Mario Emmenlauer.  Given the lack of previous complaints,
      I'm not excited about pushing this into stable branches, but it seems
      OK to squeeze it into v14.
      
      Discussion: https://postgr.es/m/48bafc63-c30f-3962-2ded-f2e985d93e86@emmenlauer.de
      6b54f123
    • Tom Lane's avatar
      Fix float4/float8 hash functions to produce uniform results for NaNs. · 23c6bc58
      Tom Lane authored
      The IEEE 754 standard allows a wide variety of bit patterns for NaNs,
      of which at least two ("NaN" and "-NaN") are pretty easy to produce
      from SQL on most machines.  This is problematic because our btree
      comparison functions deem all NaNs to be equal, but our float hash
      functions know nothing about NaNs and will happily produce varying
      hash codes for them.  That causes unexpected results from queries
      that hash a column containing different NaN values.  It could also
      produce unexpected lookup failures when using a hash index on a
      float column, i.e. "WHERE x = 'NaN'" will not find all the rows
      it should.
      
      To fix, special-case NaN in the float hash functions, not too much
      unlike the existing special case that forces zero and minus zero
      to hash the same.  I arranged for the most vanilla sort of NaN
      (that coming from the C99 NAN constant) to still have the same
      hash code as before, to reduce the risk to existing hash indexes.
      
      I dithered about whether to back-patch this into stable branches,
      but ultimately decided to do so.  It's a clear improvement for
      queries that hash internally.  If there is anybody who has -NaN
      in a hash index, they'd be well advised to re-index after applying
      this patch ... but the misbehavior if they don't will not be much
      worse than the misbehavior they had before.
      
      Per bug #17172 from Ma Liangzhu.
      
      Discussion: https://postgr.es/m/17172-7505bea9e04e230f@postgresql.org
      23c6bc58
    • Michael Paquier's avatar
      doc: Replace some uses of "which" by "that" in parallel.sgml · 2c1981ec
      Michael Paquier authored
      This makes the documentation more accurate grammatically.
      
      Author: Elena Indrupskaya
      Discussion: https://postgr.es/m/1c994b3d-951e-59bb-1ac2-7b9221c0e4cf@postgrespro.ru
      Backpatch-through: 9.6
      2c1981ec
  11. 01 Sep, 2021 5 commits
    • Tom Lane's avatar
      Doc: clarify how triggers relate to transactions. · 95bc40f8
      Tom Lane authored
      Laurenz Albe, per gripe from Nathan Long.
      
      Discussion: https://postgr.es/m/161953360822.695.15805897835151971142@wrigleys.postgresql.org
      95bc40f8
    • Tomas Vondra's avatar
      Identify simple column references in extended statistics · 50ba70a9
      Tomas Vondra authored
      Until now, when defining extended statistics, everything except a plain
      column reference was treated as complex expression. So for example "a"
      was a column reference, but "(a)" would be an expression. In most cases
      this does not matter much, but there were a couple strange consequences.
      For example
      
          CREATE STATISTICS s ON a FROM t;
      
      would fail, because extended stats require at least two columns. But
      
          CREATE STATISTICS s ON (a) FROM t;
      
      would succeed, because that requirement does not apply to expressions.
      Moreover, that statistics object is useless - the optimizer will always
      use the regular statistics collected for attribute "a".
      
      So do a bit more work to identify those expressions referencing a single
      column, and translate them to a simple column reference. Backpatch to
      14, where support for extended statistics on expressions was introduced.
      
      Reported-by: Justin Pryzby
      Backpatch-through: 14
      Discussion: https://postgr.es/m/20210816013255.GS10479%40telsasoft.com
      50ba70a9
    • Fujii Masao's avatar
      pgbench: Fix bug in measurement of disconnection delays. · d760d942
      Fujii Masao authored
      When -C/--connect option is specified, pgbench establishes and closes
      the connection for each transaction. In this case pgbench needs to
      measure the times taken for all those connections and disconnections,
      to include the average connection time in the benchmark result.
      But previously pgbench could not measure those disconnection delays.
      To fix the bug, this commit makes pgbench measure the disconnection
      delays whenever the connection is closed at the end of transaction,
      if -C/--connect option is specified.
      
      Back-patch to v14. Per discussion, we concluded not to back-patch to v13
      or before because changing that behavior in stable branches would
      surprise users rather than providing benefits.
      
      Author: Yugo Nagata
      Reviewed-by: Fabien COELHO, Tatsuo Ishii, Asif Rehman, Fujii Masao
      Discussion: https://postgr.es/m/20210614151155.a393bc7d8fed183e38c9f52a@sraoss.co.jp
      d760d942
    • Amit Kapila's avatar
      Fix the random test failure in 001_rep_changes. · b7ad093d
      Amit Kapila authored
      The check to test whether the subscription workers were restarting after a
      change in the subscription was failing. The reason was that the test was
      assuming the walsender started before it reaches the 'streaming' state and
      the walsender was exiting due to an error before that. Now, the walsender
      was erroring out before reaching the 'streaming' state because it tries to
      acquire the slot before the previous walsender has exited.
      
      In passing, improve the die messages so that it is easier to investigate
      the failures in the future if any.
      
      Reported-by: Michael Paquier, as per buildfarm
      Author: Ajin Cherian
      Reviewed-by: Masahiko Sawada, Amit Kapila
      Backpatch-through: 10, where this test was introduced
      Discussion: https://postgr.es/m/YRnhFxa9bo73wfpV@paquier.xyz
      b7ad093d
    • Peter Geoghegan's avatar
      VACUUM VERBOSE: Don't report "pages removed". · 0d892cf7
      Peter Geoghegan authored
      It doesn't make any sense to report this information, since VACUUM
      VERBOSE reports on heap relation truncation directly.  This was an
      oversight in commit 7ab96cf6, which made VACUUM VERBOSE output a little
      more consistent with nearby autovacuum-specific log output.  Adjust
      comments that describe how this is supposed to work in passing.
      
      Also bring truncation-related VACUUM VERBOSE output in line with the
      convention established for VACUUM VERBOSE output by commit f4f4a649.
      
      Author: Peter Geoghegan <pg@bowt.ie>
      Backpatch: 14-, where VACUUM VERBOSE's output changed.
      0d892cf7
  12. 31 Aug, 2021 1 commit