1. 31 Aug, 2021 2 commits
    • 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
  2. 30 Aug, 2021 5 commits
  3. 28 Aug, 2021 4 commits
  4. 27 Aug, 2021 8 commits
  5. 25 Aug, 2021 9 commits
  6. 24 Aug, 2021 2 commits
    • Tom Lane's avatar
      Fix regexp misbehavior with capturing parens inside "{0}". · 244dd799
      Tom Lane authored
      Regexps like "(.){0}...\1" drew an "invalid backreference number".
      That's not unreasonable on its face, since the capture group will
      never be matched if it's iterated zero times.  However, other engines
      such as Perl's don't complain about this, nor do we throw an error for
      related cases such as "(.)|\1", even though that backref can never
      succeed either.  Also, if the zero-iterations case happens at runtime
      rather than compile time --- say, "(x)*...\1" when there's no "x" to
      be found --- that's not an error, we just deem the backref to not
      match.  Making this even less defensible, no error was thrown for
      nested cases such as "((.)){0}...\2"; and to add insult to injury,
      those cases could result in assertion failures instead.  (It seems
      that nothing especially bad happened in non-assert builds, though.)
      
      Let's just fix it so that no error is thrown and instead the backref
      is deemed to never match, so that compile-time detection of no
      iterations behaves the same as run-time detection.
      
      Per report from Mark Dilger.  This appears to be an aboriginal error
      in Spencer's library, so back-patch to all supported versions.
      
      Pre-v14, it turns out to also be necessary to back-patch one aspect of
      commits cb76fbd7e/00116dee5, namely to create capture-node subREs with
      the begin/end states of their subexpressions, not the current lp/rp
      of the outer parseqatom invocation.  Otherwise delsub complains that
      we're trying to disconnect a state from itself.  This is a bit scary
      but code examination shows that it's safe: in the pre-v14 code, if we
      want to wrap iteration around the subexpression, the first thing we do
      is overwrite the atom's begin/end fields with new states.  So the
      bogus values didn't survive long enough to be used for anything, except
      if no iteration is required, in which case it doesn't matter.
      
      Discussion: https://postgr.es/m/A099E4A8-4377-4C64-A98C-3DEDDC075502@enterprisedb.com
      244dd799
    • Amit Kapila's avatar
      Fix Alter Subscription's Add/Drop Publication behavior. · 5cfcd46e
      Amit Kapila authored
      The current refresh behavior tries to just refresh added/dropped
      publications but that leads to removing wrong tables from subscription. We
      can't refresh just the dropped publication because it is quite possible
      that some of the tables are removed from publication by that time and now
      those will remain as part of the subscription. Also, there is a chance
      that the tables that were part of the publication being dropped are also
      part of another publication, so we can't remove those.
      
      So, we decided that by default, add/drop commands will also act like
      REFRESH PUBLICATION which means they will refresh all the publications. We
      can keep the old behavior for "add publication" but it is better to be
      consistent with "drop publication".
      
      Author: Hou Zhijie
      Reviewed-by: Masahiko Sawada, Amit Kapila
      Backpatch-through: 14, where it was introduced
      Discussion: https://postgr.es/m/OS0PR01MB5716935D4C2CC85A6143073F94EF9@OS0PR01MB5716.jpnprd01.prod.outlook.com
      5cfcd46e
  7. 23 Aug, 2021 3 commits
    • Tom Lane's avatar
      Prevent regexp back-refs from sometimes matching when they shouldn't. · 779557bd
      Tom Lane authored
      The recursion in cdissect() was careless about clearing match data
      for capturing parentheses after rejecting a partial match.  This
      could allow a later back-reference to succeed when by rights it
      should fail for lack of a defined referent.
      
      To fix, think a little more rigorously about what the contract
      between different levels of cdissect's recursion needs to be.
      With the right spec, we can fix this using fewer rather than more
      resets of the match data; the key decision being that a failed
      sub-match is now explicitly responsible for clearing any matches
      it may have set.
      
      There are enough other cross-checks and optimizations in the code
      that it's not especially easy to exhibit this problem; usually, the
      match will fail as-expected.  Plus, regexps that are even potentially
      vulnerable are most likely user errors, since there's just not much
      point in writing a back-ref that doesn't always have a referent.
      These facts perhaps explain why the issue hasn't been detected,
      even though it's almost certainly a couple of decades old.
      
      Discussion: https://postgr.es/m/151435.1629733387@sss.pgh.pa.us
      779557bd
    • Alvaro Herrera's avatar
      Avoid creating archive status ".ready" files too early · e3fb6170
      Alvaro Herrera authored
      WAL records may span multiple segments, but XLogWrite() does not
      wait for the entire record to be written out to disk before
      creating archive status files.  Instead, as soon as the last WAL page of
      the segment is written, the archive status file is created, and the
      archiver may process it.  If PostgreSQL crashes before it is able to
      write and flush the rest of the record (in the next WAL segment), the
      wrong version of the first segment file lingers in the archive, which
      causes operations such as point-in-time restores to fail.
      
      To fix this, keep track of records that span across segments and ensure
      that segments are only marked ready-for-archival once such records have
      been completely written to disk.
      
      This has always been wrong, so backpatch all the way back.
      
      Author: Nathan Bossart <bossartn@amazon.com>
      Reviewed-by: default avatarKyotaro Horiguchi <horikyota.ntt@gmail.com>
      Reviewed-by: default avatarRyo Matsumura <matsumura.ryo@fujitsu.com>
      Reviewed-by: default avatarAndrey Borodin <x4mmm@yandex-team.ru>
      Discussion: https://postgr.es/m/CBDDFA01-6E40-46BB-9F98-9340F4379505@amazon.com
      e3fb6170
    • Michael Paquier's avatar
      Fix backup manifests to generate correct WAL-Ranges across timelines · 65b649fe
      Michael Paquier authored
      In a backup manifest, WAL-Ranges stores the range of WAL that is
      required for the backup to be valid.  pg_verifybackup would then
      internally use pg_waldump for the checks based on this data.
      
      When the timeline where the backup started was more than 1 with a
      history file looked at for the manifest data generation, the calculation
      of the WAL range for the first timeline to check was incorrect.  The
      previous logic used as start LSN the start position of the first
      timeline, but it needs to use the start LSN of the backup.  This would
      cause failures with pg_verifybackup, or any tools making use of the
      backup manifests.
      
      This commit adds a test based on a logic using a self-promoted node,
      making it rather cheap.
      
      Author: Kyotaro Horiguchi
      Discussion: https://postgr.es/m/20210818.143031.1867083699202617521.horikyota.ntt@gmail.com
      Backpatch-through: 13
      65b649fe
  8. 21 Aug, 2021 1 commit
    • Tom Lane's avatar
      Improve error messages about misuse of SELECT INTO. · 8f6a5219
      Tom Lane authored
      Improve two places in plpgsql, and one in spi.c, where an error
      message would confusingly tell you that you couldn't use a SELECT
      query, when what you had written *was* a SELECT query.  The actual
      problem is that you can't use SELECT ... INTO in these contexts,
      but the messages failed to make that apparent.  Special-case
      SELECT INTO to make these errors more helpful.
      
      Also, fix the same spots in plpgsql, as well as several messages
      in exec_eval_expr(), to not quote the entire complained-of query or
      expression in the primary error message.  That behavior very easily
      led to violating our message style guideline about keeping the primary
      error message short and single-line.  Also, since the important part
      of the message was after the inserted text, it could make the real
      problem very hard to see.  We can report the query or expression as
      the first line of errcontext instead.
      
      Per complaint from Roger Mason.  Back-patch to v14, since (a) some
      of these messages are new in v14 and (b) v14's translatable strings
      are still somewhat in flux.  The problem's older than that of
      course, but I'm hesitant to change the behavior further back.
      
      Discussion: https://postgr.es/m/1914708.1629474624@sss.pgh.pa.us
      8f6a5219
  9. 20 Aug, 2021 3 commits
  10. 19 Aug, 2021 3 commits