1. 13 Sep, 2021 1 commit
    • Michael Paquier's avatar
      Fix error handling with threads on OOM in ECPG connection logic · cc057fb3
      Michael Paquier authored
      An out-of-memory failure happening when allocating the structures to
      store the connection parameter keywords and values would mess up with
      the set of connections saved, as on failure the pthread mutex would
      still be hold with the new connection object listed but free()'d.
      
      Rather than just unlocking the mutex, which would leave the static list
      of connections into an inconsistent state, move the allocation for the
      structures of the connection parameters before beginning the test
      manipulation.  This ensures that the list of connections and the
      connection mutex remain consistent all the time in this code path.
      
      This error is unlikely going to happen, but this could mess up badly
      with ECPG clients in surprising ways, so backpatch all the way down.
      
      Reported-by: ryancaicse
      Discussion: https://postgr.es/m/17186-b4cfd8f0eb4d1dee@postgresql.org
      Backpatch-through: 9.6
      cc057fb3
  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 7 commits
    • Tomas Vondra's avatar
      Don't print extra parens around expressions in extended stats · 4d1816ec
      Tomas Vondra authored
      The code printing expressions for extended statistics doubled the
      parens, producing results like ((a+1)), which is unnecessary and not
      consistent with how we print expressions elsewhere.
      
      Fixed by tweaking the code to produce just a single set of parens.
      
      Reported by Mark Dilger, fix by me. Backpatch to 14, where support for
      extended statistics on expressions was added.
      
      Reported-by: Mark Dilger
      Discussion: https://postgr.es/m/20210122040101.GF27167%40telsasoft.com
      4d1816ec
    • John Naylor's avatar
      Mark the timestamptz variant of date_bin() as stable · 3eda9fc0
      John Naylor authored
      Previously, it was immutable by lack of marking. This is not
      correct, since the time zone could change.
      
      Bump catversion
      
      Discussion: https://www.postgresql.org/message-id/CAFBsxsG2UHk8mOWL0tca%3D_cg%2B_oA5mVRNLhDF0TBw980iOg5NQ%40mail.gmail.com
      Backpatch to v14, when this function came in
      3eda9fc0
    • Tom Lane's avatar
      In pg_dump, avoid doing per-table queries for RLS policies. · a20a9f26
      Tom Lane authored
      For no particularly good reason, getPolicies() queried pg_policy
      separately for each table.  We can collect all the policies in
      a single query instead, and attach them to the correct TableInfo
      objects using findTableByOid() lookups.  On the regression
      database, this reduces the number of queries substantially, and
      provides a visible savings even when running against a local
      server.
      
      Per complaint from Hubert Depesz Lubaczewski.  Since this is such
      a simple fix and can have a visible performance benefit, back-patch
      to all supported branches.
      
      Discussion: https://postgr.es/m/20210826084430.GA26282@depesz.com
      a20a9f26
    • Tom Lane's avatar
      Cache the results of format_type() queries in pg_dump. · 9407dbbc
      Tom Lane authored
      There's long been a "TODO: there might be some value in caching
      the results" annotation on pg_dump's getFormattedTypeName function;
      but we hadn't gotten around to checking what it was costing us to
      repetitively look up type names.  It turns out that when dumping the
      current regression database, about 10% of the total number of queries
      issued are duplicative format_type() queries.  However, Hubert Depesz
      Lubaczewski reported a not-unusual case where these account for over
      half of the queries issued by pg_dump.  Individually these queries
      aren't expensive, but when network lag is a factor, they add up to a
      problem.  We can very easily add some caching to getFormattedTypeName
      to solve it.
      
      Since this is such a simple fix and can have a visible performance
      benefit, back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/20210826084430.GA26282@depesz.com
      9407dbbc
    • Tomas Vondra's avatar
      Rename the role in stats_ext to have regress_ prefix · 4090ff2a
      Tomas Vondra authored
      Commit 5be8ce82e8 added a new role to the stats_ext regression suite,
      but the role name did not start with regress_ causing failures when
      running with ENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS. Fixed by
      renaming the role to start with the expected regress_ prefix.
      
      Backpatch-through: 10, same as the new regression test
      Discussion: https://postgr.es/m/1F238937-7CC2-4703-A1B1-6DC225B8978A%40enterprisedb.com
      4090ff2a
    • Tomas Vondra's avatar
      Fix lookup error in extended stats ownership check · a371a5ba
      Tomas Vondra authored
      When an ownership check on extended statistics object failed, the code
      was calling aclcheck_error_type to report the failure, which is clearly
      wrong, resulting in cache lookup errors. Fix by calling aclcheck_error.
      
      This issue exists since the introduction of extended statistics, so
      backpatch all the way back to PostgreSQL 10. It went unnoticed because
      there were no tests triggering the error, so add one.
      
      Reported-by: Mark Dilger
      Backpatch-through: 10, where extended stats were introduced
      Discussion: https://postgr.es/m/1F238937-7CC2-4703-A1B1-6DC225B8978A%40enterprisedb.com
      a371a5ba
    • Tom Lane's avatar
      Fix missed lock acquisition while inlining new-style SQL functions. · 983d7033
      Tom Lane authored
      When starting to use a query parsetree loaded from the catalogs,
      we must begin by applying AcquireRewriteLocks(), to obtain the same
      relation locks that the parser would have gotten if the query were
      entered interactively, and to do some other cleanup such as dealing
      with later-dropped columns.  New-style SQL functions are just as
      subject to this rule as other stored parsetrees; however, of the
      places dealing with such functions, only init_sql_fcache had gotten
      the memo.  In particular, if we successfully inlined a new-style
      set-returning SQL function that contained any relation references,
      we'd either get an assertion failure or attempt to use those
      relation(s) sans locks.
      
      I also added AcquireRewriteLocks calls to fmgr_sql_validator and
      print_function_sqlbody.  Desultory experiments didn't demonstrate any
      failures in those, but I suspect that I just didn't try hard enough.
      Certainly we don't expect nearby code paths to operate without locks.
      
      On the same logic of it-ought-to-have-the-same-effects-as-the-old-code,
      call pg_rewrite_query() in fmgr_sql_validator, too.  It's possible
      that neither code path there needs to bother with rewriting, but
      doing the analysis to prove that is beyond my goals for today.
      
      Per bug #17161 from Alexander Lakhin.
      
      Discussion: https://postgr.es/m/17161-048a1cdff8422800@postgresql.org
      983d7033