Skip to content

P
Postgres FD Implementation
Switch branch/tag
  1. 12 Aug, 2022 11 commits
  3. 11 Aug, 2022 1 commit
    • Amit Kapila's avatar
      Fix catalog lookup with the wrong snapshot during logical decoding. · 68dcce24
      Amit Kapila authored 
      Previously, we relied on HEAP2_NEW_CID records and XACT_INVALIDATION
records to know if the transaction has modified the catalog, and that
information is not serialized to snapshot. Therefore, after the restart,
if the logical decoding decodes only the commit record of the transaction
that has actually modified a catalog, we will miss adding its XID to the
snapshot. Thus, we will end up looking at catalogs with the wrong
snapshot.

To fix this problem, this changes the snapshot builder so that it
remembers the last-running-xacts list of the decoded RUNNING_XACTS record
after restoring the previously serialized snapshot. Then, we mark the
transaction as containing catalog changes if it's in the list of initial
running transactions and its commit record has XACT_XINFO_HAS_INVALS. To
avoid ABI breakage, we store the array of the initial running transactions
in the static variables InitialRunningXacts and NInitialRunningXacts,
instead of storing those in SnapBuild or ReorderBuffer.

This approach has a false positive; we could end up adding the transaction
that didn't change catalog to the snapshot since we cannot distinguish
whether the transaction has catalog changes only by checking the COMMIT
record. It doesn't have the information on which (sub) transaction has
catalog changes, and XACT_XINFO_HAS_INVALS doesn't necessarily indicate
that the transaction has catalog change. But that won't be a problem since
we use snapshot built during decoding only to read system catalogs.

On the master branch, we took a more future-proof approach by writing
catalog modifying transactions to the serialized snapshot which avoids the
above false positive. But we cannot backpatch it because of a change in
the SnapBuild.

Reported-by: Mike Oh
Author: Masahiko Sawada
Reviewed-by: Amit Kapila, Shi yu, Takamichi Osumi, Kyotaro Horiguchi, Bertrand Drouvot, Ahsan Hadi
Backpatch-through: 10
Discussion: https://postgr.es/m/81D0D8B0-E7C4-4999-B616-1E5004DBDCD2%40amazon.com
      68dcce24
  5. 10 Aug, 2022 1 commit
  7. 08 Aug, 2022 5 commits
    • Tom Lane's avatar
      Stamp 14.5. · 278273cc
      Tom Lane authored
      278273cc
    • Tom Lane's avatar
      Stabilize output of new regression test. · c0d5d52a
      Tom Lane authored 
      Per buildfarm, the output order of \dx+ isn't consistent across
locales.  Apply NO_LOCALE to force C locale.  There might be a
more localized way, but I'm not seeing it offhand, and anyway
there is nothing in this test module that particularly cares
about locales.

Security: CVE-2022-2625
      c0d5d52a
    • Tom Lane's avatar
      Last-minute updates for release notes. · ea2917ca
      Tom Lane authored 
      Security: CVE-2022-2625
      ea2917ca
    • Tom Lane's avatar
      In extensions, don't replace objects not belonging to the extension. · 5721da7e
      Tom Lane authored 
      Previously, if an extension script did CREATE OR REPLACE and there was
an existing object not belonging to the extension, it would overwrite
the object and adopt it into the extension.  This is problematic, first
because the overwrite is probably unintentional, and second because we
didn't change the object's ownership.  Thus a hostile user could create
an object in advance of an expected CREATE EXTENSION command, and would
then have ownership rights on an extension object, which could be
modified for trojan-horse-type attacks.

Hence, forbid CREATE OR REPLACE of an existing object unless it already
belongs to the extension.  (Note that we've always forbidden replacing
an object that belongs to some other extension; only the behavior for
previously-free-standing objects changes here.)

For the same reason, also fail CREATE IF NOT EXISTS when there is
an existing object that doesn't belong to the extension.

Our thanks to Sven Klemm for reporting this problem.

Security: CVE-2022-2625
      5721da7e
    • Alvaro Herrera's avatar
      Translation updates · 9a8df330
      Alvaro Herrera authored 
      Source-Git-URL: ssh://git@git.postgresql.org/pgtranslation/messages.git
Source-Git-Hash: 20d70fc4a9763d5d31afc422be0be0feb0fb0363
      9a8df330
  9. 07 Aug, 2022 2 commits
  11. 06 Aug, 2022 1 commit
    • Alvaro Herrera's avatar
      Improve recently-added test reliability · 9d5c96d9
      Alvaro Herrera authored 
      Commit 59be1c942a47 already tried to make
src/test/recovery/t/033_replay_tsp_drops more reliable, but it wasn't
enough.  Try to improve on that by making this use of a replication slot
to be more like others.  Also, don't drop the slot.

Make a few other stylistic changes while at it.  It's still quite slow,
which is another thing that we need to fix in this script.

Backpatch to all supported branches.

Discussion: https://postgr.es/m/349302.1659191875@sss.pgh.pa.us
      9d5c96d9
  13. 05 Aug, 2022 11 commits
    • Tom Lane's avatar
      First-draft release notes for 14.5. · aab05919
      Tom Lane authored 
      As usual, the release notes for older branches will be made by cutting
these down, but put them up for community review first.

Due to the out-of-cycle release of 14.4, there are a number of commits
that appeared in 14.4 that are not yet shipped in the earlier branches.
This draft repeats those release note entries for convenience in
preparing the older-branch notes later.  They'll be stripped out of
the 14.5 section after that's done.
      aab05919
    • Tom Lane's avatar
      Partially undo commit 94da73281. · b9243cad
      Tom Lane authored 
      On closer inspection, mcv.c isn't as broken for ScalarArrayOpExpr
as I thought.  The Var-on-right issue is real enough, but actually
it does cope fine with a NULL array constant --- I was misled by
an XXX comment suggesting it didn't.  Undo that part of the code
change, and replace the XXX comment with something less misleading.
      b9243cad
    • Tom Lane's avatar
      Fix handling of bare boolean expressions in mcv_get_match_bitmap. · 3fe2fc6b
      Tom Lane authored 
      Since v14, the extended stats machinery will try to estimate for
otherwise-unsupported boolean expressions if they match an expression
available from an extended stats object.  mcv.c did not get the memo
about this, and would spit up with "unknown clause type".  Fortunately
the case is easy to handle, since we can expect the expression yields
boolean.

While here, replace some not-terribly-on-point assertions with
simpler runtime tests for lookup failure.  That seems appropriate
so that we get an elog not a crash if we somehow get to the new
it-should-be-a-bool-expression code with a subexpression that
doesn't match any stats column.

Per report from Danny Shemesh.  Thanks to Justin Pryzby for
preliminary investigation.

Discussion: https://postgr.es/m/CAFZC=QqD6=27wQPOW1pbRa98KPyuyn+7cL_Ay_Ck-roZV84vHg@mail.gmail.com
      3fe2fc6b
    • Tom Lane's avatar
      Fix non-bulletproof ScalarArrayOpExpr code for extended statistics. · ea6c9169
      Tom Lane authored 
      statext_is_compatible_clause_internal() checked that the arguments
of a ScalarArrayOpExpr are one Var and one Const, but it would allow
cases where the Const was on the left.  Subsequent uses of the clause
are not expecting that and would suffer assertion failures or core
dumps.  mcv.c also had not bothered to cope with the case of a NULL
array constant, which seems really unacceptably sloppy of somebody.
(Although our tools failed us there too, since AFAIK neither Coverity
nor any compiler warned of the obvious use-of-uninitialized-variable
condition.)  It seems best to handle that by having
statext_is_compatible_clause_internal() reject it.

Noted while fixing bug #17570.  Back-patch to v13 where the
extended stats code grew some awareness of ScalarArrayOpExpr.
      ea6c9169
    • Alvaro Herrera's avatar
      Backpatch addition of .git-blame-ignore-revs · fff3f333
      Alvaro Herrera authored 
      This makes it more convenient for git config to contain the
blame.ignoreRevsFile setting; otherwise current git versions complain if
the file is not present.

I constructed the file for each branch by scraping the file in branch
master for commits that appear in that branch.  Because a few additional
pgindent commits have been added to the list in master since the list
was first created, this also propagates those to branches 14 and 15
where the file already existed.  Also, some entries appear to have been
made using author-date rather than committer-date in the format string,
so some timestamps are changed.  Also remove bogus whitespace in the
suggested `git log` format string.

Backpatch to all supported branches.

Discussion: https://postgr.es/m/20220711163138.o72evdeus5f5yy5z@alvherre.pgsql
      fff3f333
    • Tom Lane's avatar
      Fix incorrect permissions-checking code for extended statistics. · 7c6ce047
      Tom Lane authored 
      Commit a4d75c86 improved the extended-stats logic to allow extended
stats to be collected on expressions not just bare Vars.  To apply
such stats, we first verify that the user has permissions to read all
columns used in the stats.  (If not, the query will likely fail at
runtime, but the planner ought not do so.)  That had to get extended
to check permissions of columns appearing within such expressions,
but the code for that was completely wrong: it applied pull_varattnos
to the wrong pointer, leading to "unrecognized node type" failures.
Furthermore, although you couldn't get to this because of that bug,
it failed to account for the attnum offset applied by pull_varattnos.

This escaped recognition so far because the code in question is not
reached when the user has whole-table SELECT privilege (which is the
common case), and because only subexpressions not specially handled
by statext_is_compatible_clause_internal() are at risk.

I think a large part of the reason for this bug is under-documentation
of what statext_is_compatible_clause() is doing and what its arguments
are, so do some work on the comments to try to improve that.

Per bug #17570 from Alexander Kozhemyakin.  Patch by Richard Guo;
comments and other cosmetic improvements by me.  (Thanks also to
Japin Li for diagnosis.)  Back-patch to v14 where the bug came in.

Discussion: https://postgr.es/m/17570-f2f2e0f4bccf0965@postgresql.org
      7c6ce047
    • Alvaro Herrera's avatar
      BRIN: mask BRIN_EVACUATE_PAGE for WAL consistency checking · 541f41d4
      Alvaro Herrera authored 
      That bit is unlogged and therefore it's wrong to consider it in WAL page
comparison.

Add a test that tickles the case, as branch testing technology allows.

This has been a problem ever since wal consistency checking was
introduced (commit a507b869 for pg10), so backpatch to all supported
branches.

Author: 王海洋 (Haiyang Wang) <wanghaiyang.001@bytedance.com>
Reviewed-by: default avatarKyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/CACciXAD2UvLMOhc4jX9VvOKt7DtYLr3OYRBhvOZ-jRxtzc_7Jg@mail.gmail.com
Discussion: https://postgr.es/m/CACciXADOfErX9Bx0nzE_SkdfXr6Bbpo5R=v_B6MUTEYW4ya+cg@mail.gmail.com
      541f41d4
    • Noah Misch's avatar
      Add HINT for restartpoint race with KeepFileRestoredFromArchive(). · 8ad6c5db
      Noah Misch authored 
      The five commits ending at cc2c7d65fc27e877c9f407587b0b92d46cd6dd16
closed this race condition for v15+.  For v14 through v10, add a HINT to
discourage studying the cosmetic problem.

Reviewed by Kyotaro Horiguchi and David Steele.

Discussion: https://postgr.es/m/20220731061747.GA3692882@rfd.leadboat.com
      8ad6c5db
    • Alvaro Herrera's avatar
      regress: fix test instability · 6d9481cd
      Alvaro Herrera authored 
      Having additional triggers in a test table made the ORDER BY clauses in
old queries underspecified.  Add another column there for stability.

Per sporadic buildfarm pink.
      6d9481cd
    • Etsuro Fujita's avatar
      postgres_fdw: Disable batch insertion when there are WCO constraints. · 4a9bc2e0
      Etsuro Fujita authored 
      When inserting a view referencing a foreign table that has WITH CHECK
OPTION constraints, in single-insert mode postgres_fdw retrieves the
data that was actually inserted on the remote side so that the WITH
CHECK OPTION constraints are enforced with the data locally, but in
batch-insert mode it cannot currently retrieve the data (except for the
row first inserted through the view), resulting in enforcing the WITH
CHECK OPTION constraints with the data passed from the core (except for
the first-inserted row), which led to incorrect results when inserting
into a view referencing a foreign table in which a remote BEFORE ROW
INSERT trigger changes the rows inserted through the view so that they
violate the view's WITH CHECK OPTION constraint.  Also, the query
inserting into the view caused an assertion failure in assert-enabled
builds.

Fix these by disabling batch insertion when inserting into such a view.

Back-patch to v14 where batch insertion was added.

Discussion: https://postgr.es/m/CAPmGK17LpbTZs4m4a_6THP54UBeK9fHvX8aVVA%2BC6yEZDZwQcg%40mail.gmail.com
      4a9bc2e0
    • Alvaro Herrera's avatar
      Fix ENABLE/DISABLE TRIGGER to handle recursion correctly · 731d514a
      Alvaro Herrera authored 
      Using ATSimpleRecursion() in ATPrepCmd() to do so as bbb927b4 did is
not correct, because ATPrepCmd() can't distinguish between triggers that
may be cloned and those that may not, so would wrongly try to recurse
for the latter category of triggers.

So this commit restores the code in EnableDisableTrigger() that
86f57594 had added to do the recursion, which would do it only for
triggers that may be cloned, that is, row-level triggers.  This also
changes tablecmds.c such that ATExecCmd() is able to pass the value of
ONLY flag down to EnableDisableTrigger() using its new 'recurse'
parameter.

This also fixes what seems like an oversight of 86f57594 that the
recursion to partition triggers would only occur if EnableDisableTrigger()
had actually changed the trigger.  It is more apt to recurse to inspect
partition triggers even if the parent's trigger didn't need to be
changed: only then can we be certain that all descendants share the same
state afterwards.

Backpatch all the way back to 11, like bbb927b4.  Care is taken not
to break ABI compatibility (and that no catversion bump is needed.)
Co-authored-by: default avatarAmit Langote <amitlangote09@gmail.com>
Reviewed-by: default avatarDmitry Koval <d.koval@postgrespro.ru>
Discussion: https://postgr.es/m/CA+HiwqG-cZT3XzGAnEgZQLoQbyfJApVwOTQaCaas1mhpf+4V5A@mail.gmail.com
      731d514a
  15. 04 Aug, 2022 4 commits
  17. 03 Aug, 2022 2 commits
    • Tom Lane's avatar
      Fix incorrect tests for SRFs in relation_can_be_sorted_early(). · 445b9020
      Tom Lane authored 
      Commit fac1b470 thought we could check for set-returning functions
by testing only the top-level node in an expression tree.  This is
wrong in itself, and to make matters worse it encouraged others
to make the same mistake, by exporting tlist.c's special-purpose
IS_SRF_CALL() as a widely-visible macro.  I can't find any evidence
that anyone's taken the bait, but it was only a matter of time.

Use expression_returns_set() instead, and stuff the IS_SRF_CALL()
genie back in its bottle, this time with a warning label.  I also
added a couple of cross-reference comments.

After a fair amount of fooling around, I've despaired of making
a robust test case that exposes the bug reliably, so no test case
here.  (Note that the test case added by fac1b470 is itself
broken, in that it doesn't notice if you remove the code change.
The repro given by the bug submitter currently doesn't fail either
in v15 or HEAD, though I suspect that may indicate an unrelated bug.)

Per bug #17564 from Martijn van Oosterhout.  Back-patch to v13,
as the faulty patch was.

Discussion: https://postgr.es/m/17564-c7472c2f90ef2da3@postgresql.org
      445b9020
    • Tom Lane's avatar
      Reduce test runtime of src/test/modules/snapshot_too_old. · 8eaa4d0f
      Tom Lane authored 
      The sto_using_cursor and sto_using_select tests were coded to exercise
every permutation of their test steps, but AFAICS there is no value in
exercising more than one.  This matters because each permutation costs
about six seconds, thanks to the "pg_sleep(6)".  Perhaps we could
reduce that, but the useless permutations seem worth getting rid of
in any case.  (Note that sto_using_hash_index got it right already.)

While here, clean up some other sloppiness such as an unused table.

This doesn't make too much difference in interactive testing, since the
wasted time is typically masked by parallelization with other tests.
However, the buildfarm runs this as a serial step, which means we can
expect to shave ~40 seconds from every buildfarm run.  That makes it
worth back-patching.

Discussion: https://postgr.es/m/2515192.1659454702@sss.pgh.pa.us
      8eaa4d0f
  19. 02 Aug, 2022 1 commit
    • Tom Lane's avatar
      Be more wary about 32-bit integer overflow in pg_stat_statements. · 17fd203b
      Tom Lane authored 
      We've heard a couple of reports of people having trouble with
multi-gigabyte-sized query-texts files.  It occurred to me that on
32-bit platforms, there could be an issue with integer overflow
of calculations associated with the total query text size.
Address that with several changes:

1. Limit pg_stat_statements.max to INT_MAX / 2 not INT_MAX.
The hashtable code will bound it to that anyway unless "long"
is 64 bits.  We still need overflow guards on its use, but
this helps.

2. Add a check to prevent extending the query-texts file to
more than MaxAllocHugeSize.  If it got that big, qtext_load_file
would certainly fail, so there's not much point in allowing it.
Without this, we'd need to consider whether extent, query_offset,
and related variables shouldn't be off_t not size_t.

3. Adjust the comparisons in need_gc_qtexts() to be done in 64-bit
arithmetic on all platforms.  It appears possible that under duress
those multiplications could overflow 32 bits, yielding a false
conclusion that we need to garbage-collect the texts file, which
could lead to repeatedly garbage-collecting after every hash table
insertion.

Per report from Bruno da Silva.  I'm not convinced that these
issues fully explain his problem; there may be some other bug that's
contributing to the query-texts file becoming so large in the first
place.  But it did get that big, so #2 is a reasonable defense,
and #3 could explain the reported performance difficulties.

(See also commit 8bbe4cbd, which addressed some related bugs.
The second Discussion: link is the thread that led up to that.)

This issue is old, and is primarily a problem for old platforms,
so back-patch.

Discussion: https://postgr.es/m/CAB+Nuk93fL1Q9eLOCotvLP07g7RAv4vbdrkm0cVQohDVMpAb9A@mail.gmail.com
Discussion: https://postgr.es/m/5601D354.5000703@BlueTreble.com
      17fd203b
  21. 01 Aug, 2022 1 commit
    • Tom Lane's avatar
      Check maximum number of columns in function RTEs, too. · d947a8bd
      Tom Lane authored 
      I thought commit fd96d14d9 had plugged all the holes of this sort,
but no, function RTEs could produce oversize tuples too, either
via long coldeflists or just from multiple functions in one RTE.
(I'm pretty sure the other variants of base RTEs aren't a problem,
because they ultimately refer to either a table or a sub-SELECT,
whose widths are enforced elsewhere.  But we explicitly allow join
RTEs to be overwidth, as long as you don't try to form their
tuple result.)

Per further discussion of bug #17561.  As before, patch all branches.

Discussion: https://postgr.es/m/17561-80350151b9ad2ad4@postgresql.org
      d947a8bd