Allocate JsonLexContexts on the heap to avoid warnings

The stack allocated JsonLexContexts, in combination with codepaths
using goto, were causing warnings when compiling with LTO enabled
as the optimizer is unable to figure out that is safe.  Rather than
contort the code with workarounds for this simply heap allocate the
structs instead as these are not in any performance critical paths.

Author: Daniel Gustafsson <[email protected]>
Reported-by: Tom Lane <[email protected]>
Reviewed-by: Jacob Champion <[email protected]>
Reviewed-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/2074634.1744839761@sss.pgh.pa.us

psql: Rework TAP routine psql_fails_like() to define WAL sender context

The routine was coded so as a WAL sender was always used, state required
only for one failure test related to START_REPLICATION.  This test is
changed so as a WAL sender is used by passing a replication option to
psql_fails_like(), instead of forcing the use of a WAL sender for all
the tests.

This has come up as useful in the context of a separate bug fix where
we are looking at extending tests for some failure scenarios.  These
tests need to happen in the context of a normal backend, and not a WAL
sender where the extended query protocol cannot be used.

Discussion: https://postgr.es/m/[email protected]

Fix an oversight in 3f28b2fcac.

Commit 3f28b2fcac tried to ensure that the replication origin shouldn't be
advanced in case of an ERROR in the apply worker, so that it can request
the same data again after restart. However, it is possible that an ERROR
was caught and handled by a (say PL/pgSQL) function, and  the apply worker
continues to apply further changes, in which case, we shouldn't reset the
replication origin.

Ensure to reset the origin only when the apply worker exits after an
ERROR.

Commit 3f28b2fcac added new function geterrlevel, which we removed in HEAD
as part of this commit, but kept it in backbranches to avoid breaking any
applications. A separate case can be made to have such a function even for
HEAD.

Reported-by: Shawn McCoy <[email protected]>
Author: Hayato Kuroda <[email protected]>
Reviewed-by: Masahiko Sawada <[email protected]>
Reviewed-by: vignesh C <[email protected]>
Reviewed-by: Amit Kapila <[email protected]>
Backpatch-through: 16, where it was introduced
Discussion: https://postgr.es/m/CALsgZNCGARa2mcYNVTSj9uoPcJo-tPuWUGECReKpNgTpo31_Pw@mail.gmail.com

Remove assertion based on pending_since in pgstat_report_stat()

This assertion, based on pending_since (timestamp used to prevent stats
reports to be too frequent or should a partial flush happen), is reached
when it is found that no data can be flushed but a previous call of
pgstat_report_stat() determined that some stats data has been found as
in need of a flush.  So pending_since is set when some stats data is
pending (in non-force mode) or if report attempts are too frequent, and
reset to 0 once all stats have been flushed.

Since 5cbbe70a9cc6, WAL senders have begun to report their stats on a
periodic basis for IO stats in v16~ and backend stats on HEAD, creating
some friction with the concurrent pgstat_report_stat() calls that can
happen in the context of a WAL sender (shutdown callback doing a final
report or backend-related code paths).  This problem is the cause of
spurious failures in the TAP tests.

In theory, this assertion can be also reached in v15, even if that's
very unlikely.  For example, a process, say a background worker, could
do periodic and direct stats flushes with concurrent calls of
pgstat_report_stat() that could cause conflicting values of
pending_since.  This can be done with WAL or SLRU stats flushes using
pgstat_flush_wal() or pgstat_slru_flush().  HEAD makes this situation
easier to happen with custom cumulative stats.

This commit removes the assertion altogether, per discussion, as it is
more useful to keep the state of things as they are for the WAL sender.
The assertion could use a special state based on for example
am_walsender, but I doubt that this would be meaningful in the long run
based on the other arguments raised while discussing this issue.

Reported-by: Tom Lane <[email protected]>
Reported-by: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/1489124.1744685908@sss.pgh.pa.us
Discussion: https://postgr.es/m/dwrkeszz6czvtkxzr5mqlciy652zau5qqnm3cp5f3p2po74ppk@omg4g3cc6dgq
Backpatch-through: 15

Re-enable SSL connect_fails tests, and fix related race conditions.

Cluster.pm's connect_fails routine has long had the ability to
sniff the postmaster log file for expected messages after a
connection failure.  However, that's always had a race condition:
on some platforms it's possible for psql to exit and the test
script to slurp up the postmaster log before the backend process
has been able to write out its final log messages.  Back in
commit 55828a6b6 we disabled a bunch of tests after discovering
that, and the aim of this patch is to re-enable them.

(The sibling function connect_ok doesn't seem to have a similar
problem, mainly because the messages we look for come out during
the authentication handshake, so that if psql reports successful
connection they should certainly have been emitted already.)

The solution used here is borrowed from 002_connection_limits.pl's
connect_fails_wait routine: set the server's log_min_messages setting
to DEBUG2 so that the postmaster will log child-process exit, and then
wait till we see that log entry before checking for the messages we
are actually interested in.

If a TAP test uses connect_fails' log_like or log_unlike options, and
forgets to set log_min_messages, those connect_fails calls will now
hang until timeout.  Fixing up the existing callers shows that we had
several other TAP tests that were in theory vulnerable to the same
problem.  It's unclear whether the lack of failures is just luck, or
lack of buildfarm coverage, or perhaps there is some obscure timing
effect that only manifests in SSL connections.  In any case, this
change should in principle make those other call sites more robust.
I'm not inclined to back-patch though, unless sometime we observe
an actual failure in one of them.

Reported-by: Andrew Dunstan <[email protected]>
Author: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/984fca80-85a8-4c6f-a5cc-bb860950b435@dunslane.net

Avoid depending on post-UPDATE row order in float4/float8 tests.

While heapam reproduces the insertion order of rows well, updates
can move rows to varying places depending on autovacuum activity.
In most regression tests we've guarded against getting variable
results due to that, but float4.sql and float8.sql had escaped
notice so far because they update tables that are too small for
autovacuum to pay attention to.

With increasing interest in non-heap table AMs, it seems worth
allowing for update behaviors that are not like heapam's.  Hence,
add ORDER BY to stabilize the results in case the updates put
the rows in a different order.  (We'll continue to assume that a
seqscan will reproduce original insertion order, though.  Removing
that assumption would require vastly-more-invasive test changes.)

Author: Pavel Borisov <[email protected]>
Reviewed-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/CALT9ZEExHAnBoBVQzQuWPMKUbapF5-FBO3fdeYG3s2tuWQz1NQ@mail.gmail.com

gen_node_support.pl: improve error message for unclosed struct.

This error message was 'runaway "struct_name"', which isn't all
that clear; I think 'could not find closing brace for "struct_name"'
is better.  Also, provide the location of the struct start using the
script's usual '$file:$lineno' style.

Bug: #18901
Reported-by: Clemens Ruck <[email protected]>
Author: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/18901-424272abe01357e6@postgresql.org

doc: Mention naming convention used by injection points

All the injection points used in the tree have relied on an implied
rule: their names should be made of lower-case characters, with dashes
between the words used.

This commit adds a light mention about that in the docs, encouraging the
practice.

Author: Hayato Kuroda <[email protected]>
Reviewed-by: Aleksander Alekseev <[email protected]>
Discussion: https://postgr.es/m/OSCPR01MB14966E14C1378DEE51FB7B7C5F5B32@OSCPR01MB14966.jpnprd01.prod.outlook.com
Backpatch-through: 17

Doc: reword text explaining the --maintenance-db option

The previous text was a little clumsy.  Here we improve that.

Author: David Rowley <[email protected]>
Reported-by: Noboru Saito <[email protected]>
Reviewed-by: David G. Johnston <[email protected]>
Discussion: https://postgr.es/m/CAAM3qnJtv5YbjpwDfVOYN2gZ9zGSLFM1UGJgptSXmwfifOZJFQ@mail.gmail.com
Backpatch-through: 13

Rename injection point for invalidation messages at end of transaction

This injection point was named "AtEOXact_Inval-with-transInvalInfo", not
respecting the implied naming convention that injection points should
use lower-case characters, with terms separated by dashes.  All the
other points defined in the tree follow this style, so let's be more
consistent.

Author: Hayato Kuroda <[email protected]>
Reviewed-by: Aleksander Alekseev <[email protected]>
Discussion: https://postgr.es/m/OSCPR01MB14966E14C1378DEE51FB7B7C5F5B32@OSCPR01MB14966.jpnprd01.prod.outlook.com
Backpatch-through: 17

Doc: various fixups

* Use <symbol> tags for CONNECTION_* #defines

We were using an inconsistent mix of <literal> and sometimes <function>
tags.

* Use <application> tag for libpq

There was a mix of <literal> and <productname>

Also fix a whitespace issue.

None of these seem critical enough mistakes to backpatch.

Author: Noboru Saito <[email protected]>
Discussion: https://postgr.es/m/CAAM3qnJtv5YbjpwDfVOYN2gZ9zGSLFM1UGJgptSXmwfifOZJFQ@mail.gmail.com

Doc: fix incorrect punctuation

Author: Noboru Saito <[email protected]>
Discussion: https://postgr.es/m/CAAM3qnJtv5YbjpwDfVOYN2gZ9zGSLFM1UGJgptSXmwfifOZJFQ@mail.gmail.com
Backpatch-through: 17

Fix INITCAP() word boundaries for PG_UNICODE_FAST.

Word boundaries are based on whether a character is alphanumeric or
not. For the PG_UNICODE_FAST collation, alphanumeric includes
non-ASCII digits; whereas for the PG_C_UTF8 collation, it only
includes digits 0-9. Pass down the right information from the
pg_locale_t into initcap_wbnext to differentiate the behavior.

Reported-by: Noah Misch <[email protected]>
Reviewed-by: Noah Misch <[email protected]>
Discussion: https://postgr.es/m/20250417135841[email protected]

Use the same cmd_context throughout a walsender's lifetime.

exec_replication_command created a cmd_context to work in and
then deleted it on exit.  This is pretty dangerous because
some replication commands start/finish transactions.  In the
wake of commit 1afe31f03, that could lead to re-selecting a
CurrentMemoryContext that's already been deleted, leading to
hilarity such as a memory context that is its own parent.

To fix, let's make the cmd_context persist across
exec_replication_command calls; instead of deleting it, we'll just
reset it each time.  In this way it retains the same identity and
there's no problem if transaction abort restores it as the working
context.  It probably even saves a few microseconds to do this.

This fix also ensures that exec_replication_command returns to the
caller (PostgresMain) with the same context active that had been
when it was called (probably MessageContext).  The previous
coding could get that wrong too.

Reported-by: Anthonin Bonnefoy <[email protected]>
Author: Anthonin Bonnefoy <[email protected]>
Reviewed-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/CAO6_XqoJA7-_G6t7Uqe5nWF3nj+QBGn4F6Ptp=rUGDr0zo+KvA@mail.gmail.com

MemoryContextCreate: assert parent is valid and different from node.

The case of "node == parent" might seem impossible, since we just
allocated the new node.  But it's possible if parent is a dangling
reference to a recently-deleted context.  In fact, given aset.c's
habit of recycling contexts, it's actually rather likely if that's so.
If we'd had this assertion before, it would have simplified debugging
a recently-identified walsender issue.

Reported-by: Anthonin Bonnefoy <[email protected]>
Author: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/CAO6_XqoJA7-_G6t7Uqe5nWF3nj+QBGn4F6Ptp=rUGDr0zo+KvA@mail.gmail.com

doc: Fix memory context level in pg_log_backend_memory_contexts() example.

Commit d9e03864b6b changed the memory context level numbers shown by
pg_log_backend_memory_contexts() to be 1-based. However, the example in
the documentation was not updated and still used 0-based numbering.

This commit updates the example to match the current 1-based output.

Author: Fujii Masao <[email protected]>
Reviewed-by: David Rowley <[email protected]>
Discussion: https://postgr.es/m/1ad6d388-1b43-400d-bec9-36d52f755f74@oss.nttdata.com

Fix a few more duplicate words in comments

Similar to 84fd3bc14 but these ones were found using a regex that can span
multiple lines.

Author: David Rowley <[email protected]>
Discussion: https://postgr.es/m/CAApHDvrMcr8XD107H3NV=WHgyBcu=sx5+7=WArr-n_cWUqdFXQ@mail.gmail.com

Fix a few duplicate words in comments

These are all new to v18

Author: David Rowley <[email protected]>
Discussion: https://postgr.es/m/CAApHDvrMcr8XD107H3NV=WHgyBcu=sx5+7=WArr-n_cWUqdFXQ@mail.gmail.com

Comment on need to MarkBufferDirty() if omitting DELAY_CHKPT_START.

Blocking checkpoint phase 2 requires MarkBufferDirty() and
BUFFER_LOCK_EXCLUSIVE; neither suffices by itself.  transam/README documents
this, citing SyncOneBuffer().  Update the DELAY_CHKPT_START documentation to
say this.  Expand the heap_inplace_update_and_unlock() comment that cites
XLogSaveBufferForHint() as precedent, since heap_inplace_update_and_unlock()
could have opted not to use DELAY_CHKPT_START.

Commit 8e7e672cdaa6bfec85d4d5dd9be84159df23bb41 added DELAY_CHKPT_START to
heap_inplace_update_and_unlock().  Since commit
bc6bad88572501aecaa2ac5d4bc900ac0fd457d5 reverted it in non-master branches,
no back-patch.

Discussion: https://postgr.es/m/20250406180054[email protected]

Test restartpoints in archive recovery.

v14 commit 1f95181b44c843729caaa688f74babe9403b5850 and its v13
equivalent caused timing-dependent failures in archive recovery, at
restartpoints.  The symptom was "invalid magic number 0000 in log
segment X, offset 0", "unexpected pageaddr X in log segment Y, offset 0"
[X < Y], or an assertion failure.  Commit
3635a0a35aafd3bfa80b7a809bc6e91ccd36606a and predecessors back-patched
v15 changes to fix that.  This test reproduces the problem
probabilistically, typically in less than 1000 iterations of the test.
Hence, buildfarm and CI runs would have surfaced enough failures to get
attention within a day.

Reported-by: Arun Thirupathi <[email protected]>
Discussion: https://postgr.es/m/20250306193013[email protected]
Backpatch-through: 13

Avoid ERROR at ON COMMIT DELETE ROWS after relhassubclass=f.

Commit 7102070329d8147246d2791321f9915c3b5abf31 fixed a similar bug, but
it missed the case of database-wide ANALYZE ("use_own_xacts" mode).
Commit a07e03fd8fa7daf4d1356f7cb501ffe784ea6257 changed consequences
from silent discard of a pg_class stats (relpages et al.) update to
ERROR "tuple to be updated was already modified".  Losing a relpages
update of an ON COMMIT DELETE ROWS table was negligible, but a
COMMIT-time error isn't negligible.  Back-patch to v13 (all supported
versions).

Reported-by: Richard Guo <[email protected]
Reported-by: Robins Tharakan <[email protected]>
Discussion: https://postgr.es/m/CAMbWs4-XwMKMKJ_GT=p3_-_=j9rQSEs1FbDFUnW9zHuKPsPNEQ@mail.gmail.com
Backpatch-through: 13

Fix issue with ORDER BY / DISTINCT aggregates and FILTER

1349d2790 added support so that aggregate functions with an ORDER BY or
DISTINCT clause could make use of presorted inputs to avoid an implicit
sort within nodeAgg.c.  That commit failed to consider that a FILTER
clause may exist that filters rows before the aggregate function
arguments are evaluated.  That can be problematic if an aggregate
argument contains an expression which could error out during evaluation.
It's perfectly valid to want to have a FILTER clause which eliminates
such values, and with the pre-sorted path added in 1349d2790, it was
possible that the planner would produce a plan with a Sort node above
the Aggregate to perform the sort on the aggregate's arguments long before
the Aggregate node would filter out the non-matching values.

Here we fix this by inspecting ORDER BY / DISTINCT aggregate functions
which have a FILTER clause to see if the aggregate's arguments are
anything more complex than a Var or a Const.  Evaluating these isn't
going to cause an error.  If we find any non-Var, non-Const parameters
then the planner will now opt to perform the sort in the Aggregate node
for these aggregates, i.e. disable the presorted aggregate optimization.

An alternative fix would have been to completely disallow the presorted
optimization for Aggrefs with any FILTER clause, but that wasn't done as
that could cause large performance regressions for queries that see
significant gains from 1349d2790 due to presorted results coming in from
an Index Scan.

Backpatch to 16, where 1349d2790 was introduced

Author: David Rowley <[email protected]>
Reported-by: Kaimeh <[email protected]>
Diagnosed-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/CAK-%2BJz9J%3DQ06-M7cDJoPNeYbz5EZDqkjQbJnmRyQyzkbRGsYkA%40mail.gmail.com
Backpatch-through: 16

psql: Split extended query protocol meta-commands in --help=commands

Compared to v17 with only \bind able to do extended query protocol work,
v18 has now a total of 11 meta-commands related to the extended query
protocol.  These were all listed under the "General" section of the
--help=commands output and are specialized, bloating the output
generated.

All these meta-commands are moved into a new section called "Extended
Query Protocol", listed at the end of --help=commands.

This split has been suggested by Noah Misch.

Discussion: https://postgr.es/m/20250415213450[email protected]

psql: Improve descriptions of \\flush[request] in --help

Noah has reported that the current wording was confusing compared to the
description of the underlying libpq routine.  The new wording is from
me.

Reported-by: Noah Misch <[email protected]>
Discussion: https://postgr.es/m/20250415213450[email protected]

psql: Fix incorrect status code returned by \getresults

When an invalid number of results is requested for \getresults, the
status code returned by exec_command_getresults() was PSQL_CMD_SKIP_LINE
and not PSQL_CMD_ERROR.

This led to incorrect behaviors, with ON_ERROR_STOP for example.

Reported-by: Noah Misch <[email protected]>
Discussion: https://postgr.es/m/20250415213450[email protected]

Be more wary of corrupt data in pageinspect's heap_page_items().

The original intent in heap_page_items() was to return nulls, not
throw an error or crash, if an item was sufficiently corrupt that
we couldn't safely extract data from it.  However, commit d6061f83a
utterly missed that memo, and not only put in an un-length-checked
copy of the tuple's data section, but also managed to break the check
on sane nulls-bitmap length.  Either mistake could possibly lead to
a SIGSEGV crash if the tuple is corrupt.

Bug: #18896
Reported-by: Dmitry Kovalenko <[email protected]>
Author: Dmitry Kovalenko <[email protected]>
Reviewed-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/18896-add267b8e06663e3@postgresql.org
Backpatch-through: 13

Fix typos and grammar in the code

The large majority of these have been introduced by recent commits done
in the v18 development cycle.

Author: Alexander Lakhin <[email protected]>
Discussion: https://postgr.es/m/9a7763ab-5252-429d-a943-b28941e0e28b@gmail.com

Rename injection points used in AIO tests

The format of the injection point names used by the AIO code does not
match the existing naming convention used everywhere else in the code,
so let's be consistent.  These points are used in test_aio.

Reviewed-by: Hayato Kuroda <[email protected]>
Discussion: https://postgr.es/m/[email protected]

Make pg_upgrade log message with control file path translatable.

Commit 173c97812ff replaced the hardcoded "global/pg_control" in pg_upgrade
log message with a string literal concatenation of XLOG_CONTROL_FILE macro.
However, this change made the message untranslatable.

This commit fixes the issue by using %s with XLOG_CONTROL_FILE instead of
that literal concatenation, allowing the message to be translated properly.
It also wraps the file path in double quotes for consistency with similar
log messages.

Author: Kyotaro Horiguchi <[email protected]>
Reviewed-by: Masao Fujii <[email protected]>
Discussion: https://postgr.es/m/20250407.155546.2129693791769531891[email protected]

Doc: fix missing comma at the end of a line.

Backpatch to 17, where the line was added.

Reported by Noboru Saito while he was working on translating the file
into Japanese.

Discussion: https://postgr.es/m/20250417.203047.1321297410457834775.ishii%40postgresql.org
Reported-by: Noboru Saito <[email protected]>
Reviewed-by: Daniel Gustafs <[email protected]>
Backpatch-through: 17

Fixup various older misuses of appendPQExpBuffer

Use appendPQExpBufferStr when there are no parameters and
appendPQExpBufferChar when the string length is 1.

Unlike 3fae25cbb, which fixed this issue for code that was new to v18,
this one fixes up instances which exist in the backbranches.  We've
historically tried to maintain this standard and if we're going to
continue doing that, then we won't be doing that selectively based on
when the code was introduced.  Now seems like a good time to flush out the
existing misuses.  Waiting until v19 just prolongs their existence in
terms of released versions that the misuses exist in.

Author: David Rowley <[email protected]>
Discussion: https://postgr.es/m/CAApHDvoARMvPeXTTC0HnpARBHn-WgVstc8XFCyMGOzvgu_1HvQ@mail.gmail.com

Make levels 1-based in pg_log_backend_memory_contexts()

Both pg_get_process_memory_contexts() and pg_backend_memory_contexts
have 1-based levels, whereas pg_log_backend_memory_contexts() was using
0-based levels.  Align these.

This results in slightly saner behavior from MemoryContextStatsDetail()
in regards to the max_level.  Previously it would stop at 1 level before
the maximum requested level rather than at that level.

Reported-by: Atsushi Torikoshi <[email protected]>
Author: Atsushi Torikoshi <[email protected]>
Author: David Rowley <[email protected]
Reviewed-by: Melih Mutlu <[email protected]>
Reviewed-by: Rahila Syed <[email protected]>
Discussion: https://postgr.es/m/395ea5d4fe190480efa95bf533485c70@oss.nttdata.com

Suppress "may be used uninitialized" warnings from older compilers.

The "children" list won't be used until "got_children" has been set
true, but older compilers don't get that; about half a dozen
buildfarm animals are warning about this.  Issue added by 11ff192b5.

While here, improve slightly-shaky grammar in comment.

Discussion: https://postgr.es/m/2057835.1744833309@sss.pgh.pa.us

Portability fix: isdigit() must be passed an unsigned char.

Oversight in commit 40b9c2701, per buildfarm member mamba.

Cache typlens of a SQL function's input arguments.

This gets rid of repetitive get_typlen calls in postquel_sub_params,
which show up as costing a few percent of the runtime in simple test
cases (more with more parameters).

In combination with the preceding patches, this gets us most of the
way back down to the amount of per-call overhead that functions.c
had before commit 0dca5d68d.  There are some more things that could
be done, but this seems like an okay place to stop for v18.

Make SQLFunctionCache long-lived again.

At this point, the only data structures we allocate directly in
fcontext are the SQLFunctionCache struct itself, the ParamListInfo
struct, and the execution_state array, all of which are small and
perfectly capable of being re-used across executions of the same
FmgrInfo.  Hence, let's give them the same lifespan as the FmgrInfo.
This step gets rid of the separate SQLFunctionLink struct and makes
fn_extra point to SQLFunctionCache again.  We also get rid of the
separate fcontext memory context and allocate these items directly
in fn_mcxt.

For notational simplicity, SQLFunctionCache still has an fcontext
field, but it's just a copy of fn_mcxt.

The motivation for this is to allow these structures to live as
long as the FmgrInfo and be re-used across calls, restoring the
original design without its propensity for memory leaks.  This
gets rid of some per-call overhead that we added in 0dca5d68d.

We also make an effort to re-use the JunkFilter and result slot.
Those might need to change if the function definition changes,
so we compromise by rebuilding them if the cached plan changes.

This also moves the tuplestore into fn_mcxt so that it can be
re-used across calls, again undoing a change made in 0dca5d68d.

Split some storage out to separate subcontexts of fcontext.

Put the JunkFilter and its result slot (and thence also
some subsidiary data such as the result tupledesc) into a
separate subcontext "jfcontext".  This doesn't accomplish
a lot at this point, because we make a new JunkFilter each
time through the SQL function.  However, the plan is to make
the fcontext long-lived, and that raises the possibility
that we'll need a new JunkFilter because the plan for the
result-generating query changes.  A separate context makes
it easy to free the obsoleted data when that happens.

Also, instead of always running the sub-executor in fcontext,
make a separate context for it if we're doing lazy eval of
a SRF, and otherwise just run it inside CurrentMemoryContext.

Make functions.c mostly run in a short-lived memory context.

Previously, much of this code ran with CurrentMemoryContext set
to be the function's fcontext, so that we tended to leak a lot of
stuff there.  Commit 0dca5d68d dealt with that by releasing the
fcontext at the completion of each SQL function call, but we'd
like to go back to the previous approach of allowing the fcontext
to be query-lifespan.  To control the leakage problem, rearrange
the code so that we mostly run in the memory context that fmgr_sql
is called in (which we expect to be short-lived).  Notably, this
means that parsing/planning is all done in the short-lived context
and doesn't leak cruft into fcontext.

This patch also fixes the allocation of execution_state records
so that we don't leak them across executions.  I set that up
with a re-usable array that contains at least as many
execution_state structs as we need for the current querytree.
The chain structure is still there, but it's not really doing
much for us, and maybe somebody will be motivated to get rid
of it.  I'm not though.

This incidentally also moves the call of BlessTupleDesc to be
with the code that creates the JunkFilter.  That doesn't make
much difference now, but a later patch will reduce the number
of times the JunkFilter gets made, and we needn't bless the
results any more often than that.

We still leak a fair amount in fcontext, particularly when
executing utility statements, but that's material for a
separate patch step; the point here is only to get rid of
unintentional allocations in fcontext.

Minor performance improvement for SQL-language functions.

Late in the development of commit 0dca5d68d, I added a step to copy
the result tlist we extract from the cached final query, because
I was afraid that that might not last as long as the JunkFilter that
we're passing it off to.  However, that turns out to cost a noticeable
number of cycles, and it's really quite unnecessary because the
JunkFilter will not examine that tlist after it's been created.
(ExecFindJunkAttribute would use it, but we don't use that function
on this JunkFilter.)  Hence, remove the copy step.  For safety,
reset the might-become-dangling jf_targetList pointer to NIL.

In passing, remove DR_sqlfunction.cxt, which we don't use anymore;
it's confusing because it's not entirely clear which context it
ought to point at.

Assert lack of hazardous buffer locks before possible catalog read.

Commit 0bada39c83a150079567a6e97b1a25a198f30ea3 fixed a bug of this kind,
which existed in all branches for six days before detection.  While the
probability of reaching the trouble was low, the disruption was extreme.  No
new backends could start, and service restoration needed an immediate
shutdown.  Hence, add this to catch the next bug like it.

The new check in RelationIdGetRelation() suffices to make autovacuum detect
the bug in commit 243e9b40f1b2dd09d6e5bf91ebf6e822a2cd3704 that led to commit
0bada39.  This also checks in a number of similar places.  It replaces each
Assert(IsTransactionState()) that pertained to a conditional catalog read.

No back-patch for now, but a back-patch of commit 243e9b4 should back-patch
this, too.  A back-patch could omit the src/test/regress changes, since back
branches won't gain new index columns.

Reported-by: Alexander Lakhin <[email protected]>
Discussion: https://postgr.es/m/20250410191830[email protected]
Discussion: https://postgr.es/m/10ec0bc3-5933-1189-6bb8-5dec4114558e@gmail.com

pg_dump: Set private_date pointer to NULL in callback

The end callback for ZStandard compression frees the private_data
but didn't set the pointer to NULL after freeing.  This is not a
bug as the code is right now, since nothing is dereferencing the
pointer upon returning from the callback but it is good practice
to do.

Author: Alexander Kuznetsov <[email protected]>
Reviewed-by: Daniel Gustafsson <[email protected]>
Reviewed-by: Ashutosh Bapat <[email protected]>
Reviewed-by: Michael Paquier <[email protected]>
Reviewed-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/efaee52b-9550-44ca-8633-ea86076b3283@altlinux.org

pg_dump: Fix incorrect archive format shown in error message.

In pg_dump and pg_restore, _allocAH() calls _discoverArchiveFormat() to
determine the archive format when the input format is unknown one.
If the input or discovered format is unrecognized, it reports an error
including the archive format number.

If discovered format is unrecognized, its number should be shown in
the error message. But previously the error message mistakenly showed
the originally requested format number (i.e., unknown one) instead of
the discovered one, due to referencing the wrong variable in the error
message.

This commit corrects the issue by using the appropriate variable in
the error message.

This fix has no practical impact since _discoverArchiveFormat() never
returns an unrecognized format and that error mesasge is actually
never output. Therefore, while the issue exists in back branches,
it's not worth the trouble and buildfarm cycles to back-patch.
So this fix is applied only to the master branch.

Author: Mahendra Singh Thalor <[email protected]>
Reviewed-by: Tom Lane <[email protected]>
Reviewed-by: Fujii Masao <[email protected]>
Discussion: https://postgr.es/m/CAKYtNAqu+N-Ab2Fq6wzNSOm_-0N-BMneanYNV1+6kFDXjva1Eg@mail.gmail.com

Another unintentional behavior change in commit e9931bfb75.

Reported-by: Noah Misch <[email protected]>
Reviewed-by: Noah Misch <[email protected]>
Discussion: https://postgr.es/m/20250412123430[email protected]

Improve comment in regc_pg_locale.c.

Reported-by: Noah Misch <[email protected]>
Reviewed-by: Noah Misch <[email protected]>
Discussion: https://postgr.es/m/20250412123430[email protected]

Fixup various new-to-v18 usages of appendPQExpBuffer

Use appendPQExpBufferStr when there are no parameters and
appendPQExpBufferChar when the string length is 1.

Author: David Rowley <[email protected]>
Discussion: https://postgr.es/m/CAApHDvoARMvPeXTTC0HnpARBHn-WgVstc8XFCyMGOzvgu_1HvQ@mail.gmail.com

Improve comments for estimate_multivariate_ndistinct()

estimate_multivariate_ndistinct() is coded to assume the caller handles
passing it a list of GroupVarInfos with unique 'var' fields over the
entire list.  6bb6a62f3 added code which didn't ensure this and that
could result in estimate_multivariate_ndistinct() erroring out with:

ERROR:  corrupt MVNDistinct entry

This occurred because estimate_multivariate_ndistinct() first searches
for a set of stats that match to at least two of the given GroupVarInfos
and then later assumes that the MVNDistinctItem.items array of the
best matching stats will have an entry for those two columns.  If the
GroupVarInfos List contained a duplicate entry then the same column could
be matched to twice and that could trick the code into thinking we have
>= 2 columns matched in cases where only a single distinct column has been
matched.  This could result in a failure to find the correct
MVNDistinctItem in the stats as the array containing those never
contains an item for single columns.

Here we make it more clear that the function needs a distinct set of
GroupVarInfos and also tidy up a few other comments to make things a bit
easier to follow.

Author: David Rowley <[email protected]>
Discussion: https://postgr.es/m/CAApHDvocZCUhM9W9mJ39d6oQz7ePKoqFnao_347mvC-A7QatcQ@mail.gmail.com

Sync declarations and definitions of two new tablecmds.c functions.

Buildfarm member drongo complained because the definitions of these
functions used "const Oid foo" where the forward declarations just
had "Oid foo".  (I'm a bit surprised that drongo seems to be the only
complainant.)  I chose to fix this by removing the "consts" because
(a) I'm generally not a fan of using const that way, and (b) it was
a minority usage even within these two functions, let alone compared
to the rest of our code base.

Oversight in commit eec0040c4, so no need for back-patch.

Elide not-null constraint checks on child tables during PK creation

We were unnecessarily acquiring AccessExclusiveLock on all child tables
when "ALTER TABLE ONLY sometab ADD PRIMARY KEY" was run on their parent
table, an oversight in commit 14e87ffa5c54.  This caused deadlocks
during pg_restore of partitioned tables.

The reason to acquire the AEL was that we need to verify that child
tables have the involved columns already marked as not-null; but if the
parent table has an inheritable not-null constraint, then all children
must necessarily be in the correct state already, so we can skip the
check, which avoids acquiring the lock.  Reorder the code so that it
works that way.  This doesn't change things in the case where the
constraint doesn't exist, but that case is of lesser importance because
it doesn't occur during parallel pg_restore.

While at it, reword some errmsg() and add errhint() to similar cases in
related but not adjacent code.

Diagnosed-by: Tom Lane <[email protected]>
Reviewed-by: Tender Wang <[email protected]>
Discussion: https://postgr.es/m/67469c1c-38bc-7d94-918a-67033f5dd731@gmx.net
Discussion: https://postgr.es/m/2045026.1743801143@sss.pgh.pa.us
Discussion: https://postgr.es/m/1280408.1744650810@sss.pgh.pa.us

Update pg_config.h.in with libnuma changes

Add macros from autoheader which were accidentally omitted in
commit 65c298f61fc. There is no function change by this as no
code is currently using the missing macro.

Author: Daniel Gustafsson <[email protected]>
Reviewed-by: Jacob Champion <[email protected]>
Discussion: https://postgr.es/m/CF6D7D7F-E1C4-45BE-9019-0F4B4BC7C135@yesql.se

Fix pg_dump --clean with partitioned indexes.

We'd try to drop the partitions of a partitioned index separately,
which is disallowed by the backend, leading to an error during
restore.  While the error is harmless, it causes problems if you
try to use --single-transaction mode.

Fortunately, there seems no need to do a DROP at all, since the
partition will go away silently when we drop either the parent index
or the partition's table.  So just make the DROP conditional on not
being a partition.

Reported-by: jian he <[email protected]>
Author: jian he <[email protected]>
Reviewed-by: Pavel Stehule <[email protected]>
Reviewed-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/CACJufxF0QSdkjFKF4di-JGWN6CSdQYEAhGPmQJJCdkSZtd=oLg@mail.gmail.com
Backpatch-through: 13

pg_restore cleanups

. remove unnecessary oid_string list stuff
. use pg_get_line_buf() instead of open-coding it
. cleaner parsing of map.dat lines

Reverts 2b69afbe50d add new list type simple_oid_string_list to fe-utils/simple_list

Author: Álvaro Herrera <[email protected]>
Author: Andrew Dunstan <[email protected]>

Discussion: https://postgr.es/m/202504141220[email protected]

Fix an incorrect check in get_memoize_path

Memoize typically marks cache entries as complete after fully scanning
the inner side of a join.  However, in the case of unique joins, we
skip to the next outer tuple as soon as the first matching inner tuple
is found, leaving no opportunity to scan the inner side to completion.
To work around that, we mark cache entries as complete after fetching
the first matching inner tuple in unique joins.

This approach is only safe when all of the join's restriction clauses
are parameterized; otherwise, there is no guarantee that reading just
one tuple from the inner side is sufficient.

Currently, we check for this by verifying that the number of clauses
in ppi_clauses is no less than the number of the join's restriction
clauses.  However, this check isn't entirely reliable, as ppi_clauses
includes join clauses available from all outer rels, not just the
current outer rel.  This means the check could pass even if a
restriction clause isn't parameterized, as long as another join
clause, which doesn't belong to the current join, is included in
ppi_clauses.

To fix this, we explicitly check whether each restriction clause of
the current join is present in ppi_clauses.

While we're here, remove the XXX comment from the modified code, as
it's not justified; in certain cases, it's not possible to move a join
clause to the inner side.

This is arguably a bugfix, but no backpatch given the lack of field
reports.

Author: Richard Guo <[email protected]>
Reviewed-by: wenhui qiu <[email protected]>
Reviewed-by: Andrei Lepikhov <[email protected]>
Discussion: https://postgr.es/m/CAMbWs4-8JPouj=wBDj4DhK-WO4+Xdx=A2jbjvvyyTBQneJ1=BQ@mail.gmail.com

doc: Fix typos in documentation

This fixes a set of typos introduced during the v18 development
cycle.

Author: Daniel Gustafsson <[email protected]>
Reviewed-by: Laurenz Albe <[email protected]>
Discussion: https://postgr.es/m/7038B4C5-2742-42B1-A8F0-0FFEAECF02A7@yesql.se

Fix failure for generated column with a not-null domain constraint.

If a GENERATED column is declared to have a domain data type where
the domain's constraints disallow null values, INSERT commands failed
because we built a targetlist that included coercing a null constant
to the domain's type.  The failure occurred even when the generated
value would have been perfectly OK.  This is adjacent to the issues
fixed in 0da39aa76, but we didn't notice for lack of testing a domain
with such a constraint.

We aren't going to use the result of the targetlist entry for the
generated column --- ExecComputeStoredGenerated will overwrite it.
So it's not really necessary that it have the exact datatype of
the generated column.  This patch fixes the problem by changing
the targetlist entry to be a null Const of the domain's base type,
which should be sufficiently legal.  (We do have to tweak
ExecCheckPlanOutput to accept the situation, though.)

This has been broken since we implemented generated columns.
However, this patch only applies easily as far back as v14, partly
because I (tgl) only carried 0da39aa76 back that far, but mostly
because v14 significantly refactored the handling of INSERT/UPDATE
targetlists.  Given the lack of field complaints and the short
remaining support lifetime of v13, I judge the cost-benefit ratio
not good for devising a version that would work in v13.

Reported-by: jian he <[email protected]>
Author: jian he <[email protected]>
Reviewed-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/CACJufxG59tip2+9h=rEv-ykOFjt0cbsPVchhi0RTij8bABBA0Q@mail.gmail.com
Backpatch-through: 14

doc: Fix missing whitespace in pg_restore documentation.

Previously, a space was missing between "<option>--exclude-schema</option>"
and "for" in the pg_restore documentation. This commit fixes the typo by
adding the missing whitespace.

Back-patch to v17 where the typo was added.

Author: Lele Gaifax <[email protected]>
Reviewed-by: Fujii Masao <[email protected]>
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 17

pg_combinebackup: Fix incorrect code documentation

The code comment for parse_oid accidentally used the wrong parameter
when referring to the location of the last backup. Also, while there,
improve sentence wording by removing a superfluous word.

Backpatch to v17 where pg_combinebackup was addedd

Author: Amul Sul <[email protected]>
Reviewed-by: Daniel Gustafsson <[email protected]>
Reviewed-by: Robert Haas <[email protected]>
Discussion: https://postgr.es/m/CAAJ_b95ecWgzcS4K3Dx0E_Yp-SLwK5JBasFgioKMSjhQLw9xvg@mail.gmail.com
Backpatch-through: 17

Fix incorrect format placeholders

BlockNumber is unsigned int.  Fix for commit 14ffaece0fb.

Add more source files to pg_verifybackup/nls.mk

also related to commit 8dfd3129027

Doc: use "an SQL" consistently rather than "a SQL"

Per the precedent set by 04539e73f, adjust article prefixes for "SQL" to
use "an" consistently rather than "a", i.e., "an es-que-ell" rather than
"a sequel".

Both of these are new to v18. Also see b1b13d2b5, d866f0374 and
7bdd489d3.

Mark sslkeylogfile as Debug option

Mark the sslkeylogile option as "D" debug as this truly is a debug
option, and it will allow postgres_fdw et.al to filter it out as
well.  Also update the display length to match that for an ssl key
as they are both filename based inputs.

Author: Daniel Gustafsson <[email protected]>
Reported-by: Jacob Champion <[email protected]>
Discussion: https://postgr.es/m/CAOYmi+=5GyBKpu7bU4D_xkAnYJTj=rMzGaUvHO99-DpNG_YKcw@mail.gmail.com

Make AIO error test more portable

Alpine Linux's C library (musl) spells one error message differently.

Reported-by: Wolfgang Walther

Free memory properly in pg_restore.c

Thinko in commit 39729ec01d2. Mea maxima culpa.

Per Mahendra Singh Thalor <[email protected]>

Doc: do a little copy-editing on Index Storage Parameters list.

Add a paragraph break per suggestion from David G. Johnston.
Use a consistent voice for all the different parameter
descriptions, and fix a couple of grammatical issues.

Reported-by: Igor Korot <[email protected]>
Co-authored-by: "David G. Johnston" <[email protected]>
Co-authored-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/CA+FnnTz=EW1VQRpWB9J+G-NSchrPFcw4nR7d0JqzEK9jWKB35A@mail.gmail.com

Fix GIN's shimTriConsistentFn to not corrupt its input.

Commit 0f21db36d made an assumption that GIN triConsistentFns
would not modify their input entryRes[] arrays.  But in fact,
the "shim" triConsistentFn that we use for opclasses that don't
supply their own did exactly that, potentially leading to wrong
answers from a GIN index search.  Through bad luck, none of the
test cases that we have for such opclasses exposed the bug.

One response to this could be that the assumption of consistency check
functions not modifying entryRes[] arrays is a bad one, but it still
seems reasonable to me.  Notably, shimTriConsistentFn is itself
assuming that with respect to the underlying boolean consistentFn,
so it's sure being self-centered in supposing that it gets to do so.

Fortunately, it's quite simple to fix shimTriConsistentFn to restore
the entry-time state of entryRes[], so let's do that instead.

This issue doesn't affect any core GIN opclasses, since they all
supply their own triConsistentFns.  It does affect contrib modules
btree_gin, hstore, and intarray.

Along the way, I (tgl) noticed that shimTriConsistentFn failed to
pick up on a "recheck" flag returned by its first call to the boolean
consistentFn.  This may be only a latent problem, since it would be
unlikely for a consistentFn to set recheck for the all-false case
and not any other cases.  (Indeed, none of our contrib modules do
that.)  Nonetheless, it's formally wrong.

Reported-by: Vinod Sridharan <[email protected]>
Author: Vinod Sridharan <[email protected]>
Reviewed-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/CAFMdLD7XzsXfi1+DpTqTgrD8XU0i2C99KuF=5VHLWjx4C1pkcg@mail.gmail.com
Backpatch-through: 13

Harmonize function parameter names for Postgres 18.

Make sure that function declarations use names that exactly match the
corresponding names from function definitions in a few places.  These
inconsistencies were all introduced during Postgres 18 development.

This commit was written with help from clang-tidy, by mechanically
applying the same rules as similar clean-up commits (the earliest such
commit was commit 035ce1fe).

Fix instability with WAL fsync test in stats.sql

A backend using wal_sync_method set to "open_sync" or "open_datasync"
would fail the test checking the WAL sync data in pg_stat_io.  These
modes guarantee that a sync is done when WAL is written to disk, and the
data checked by the test is not incremented in this case,
issue_xlog_fsync() doing nothing.

Oversight in commit a051e71e28a1.

Author: Sami Imseih <[email protected]>
Discussion: https://postgr.es/m/CAA5RZ0uxwg3xAi4nvdBMJ-zJQEeyg+RotuU+ebM2F6CKmnvaYA@mail.gmail.com

Fix recently introduced typos

This fixes typos in docs and comments introduced during the v18
development cycle, to keep them from ending up in backbranches.

Author: Jacob Brazeal <[email protected]>
Reviewed-by: Daniel Gustafsson <[email protected]>
Discussion: https://postgr.es/m/CA+COZaCgGua25f2hSrjrDLJcJJAHkwoKgTTqUy-wyL1=64JNjw@mail.gmail.com

Add missing space in pg_restore documentation.

Oversight in commit 1495eff7bd.

Add missing source file to pg_verifybackup/nls.mk

added by commit 8dfd3129027

Add missing source file to pg_dump/nls.mk

added by commit c1da7281060

Add missing source file to pg_upgrade/nls.mk

added by commit 40e2e5e92b7

Add missing PGDLLIMPORT markings

Discussion: https://www.postgresql.org/message-id/flat/25095db5-b595-4b85-9100-d358907c25b5%40eisentraut.org

Fix race with synchronous_standby_names at startup

synchronous_standby_names cannot be reloaded safely by backends, and the
checkpointer is in charge of updating a state in shared memory if the
GUC is enabled in WalSndCtl, to let the backends know if they should
wait or not for a given LSN.  This provides a strict control on the
timing of the waiting queues if the GUC is enabled or disabled, then
reloaded.  The checkpointer is also in charge of waking up the backends
that could be waiting for a LSN when the GUC is disabled.

This logic had a race condition at startup, where it would be possible
for backends to not wait for a LSN even if synchronous_standby_names is
enabled.  This would cause visibility issues with transactions that we
should be waiting for but they were not.  The problem lasts until the
checkpointer does its initial update of the shared memory state when it
loads synchronous_standby_names.

In order to take care of this problem, the shared memory state in
WalSndCtl is extended to detect if it has been initialized by the
checkpointer, and not only check if synchronous_standby_names is
defined.  In WalSndCtlData, sync_standbys_defined is renamed to
sync_standbys_status, a bits8 able to know about two states:
- If the shared memory state has been initialized.  This flag is set by
the checkpointer at startup once, and never removed.
- If synchronous_standby_names is known as defined in the shared memory
state.  This is the same as the previous sync_standbys_defined in
WalSndCtl.

This method gives a way for backends to decide what they should do until
the shared memory area is initialized, and they now ultimately fall back
to a check on the GUC value in this case, which is the best thing that
can be done.

Fortunately, SyncRepUpdateSyncStandbysDefined() is called immediately by
the checkpointer when this process starts, so the window is very narrow.
It is possible to enlarge the problematic window by making the
checkpointer wait at the beginning of SyncRepUpdateSyncStandbysDefined()
with a hardcoded sleep for example, and doing so has showed that a 2PC
visibility test is indeed failing.  On machines slow enough, this bug
would cause spurious failures.

In 17~, we have looked at the possibility of adding an injection point
to have a reproducible test, but as the problematic window happens at
early startup, we would need to invent a way to make an injection point
optionally persistent across restarts when attached, something that
would be fine for this case as it would involve the checkpointer.  This
issue is quite old, and can be reproduced on all the stable branches.

Author: Melnikov Maksim <[email protected]>
Co-authored-by: Michael Paquier <[email protected]>
Discussion: https://postgr.es/m/163fcbec-900b-4b07-beaa-d2ead8634bec@postgrespro.ru
Backpatch-through: 13

Add code comment explaining ins_since_vacuum and aborted inserts

Sami complained that there's a discrepancy between n_mod_since_analyze
and n_ins_since_vacuum, as the former only accounts for committed changes
and the latter tracks committed and aborted inserts.  Nobody seemed
overly concerned that this would cause any concerning issues.  The
repercussions, from what I can tell, are limited to causing an
autovacuum to trigger for inserts sooner than it otherwise might. For
typical ratios of commits to aborts, it's unlikely to ever be noticed.

Fixing things to make it so n_ins_since_vacuum only displays committed
inserts would require an additional field in PgStat_TableCounts, which
does not quite seem worthwhile at this stage.  This commit just adds a
comment with some details to mention that we know about it, which will
hopefully prevent repeat discussions.

Reported-by: Sami Imseih <[email protected]>
Author: David Rowley <[email protected]>
Reviewed-by: Sami Imseih <[email protected]>
Discussion: https://postgr.es/m/CAApHDvpgV3a-R2EGmPOh0L-x3pHbZpM3y4dySWfy+UqUazwDQA@mail.gmail.com

Fix fat fingering in 22cb6d28950

Per Rainier Vilela

Improve various new-to-v18 appendStringInfo calls

Similar to 8461424fd, here we adjust a few new locations which were not
using the most suitable appendStringInfo* function for the intended
purpose.

Author: David Rowley <[email protected]
Discussion: https://postgr.es/m/CAApHDvqJnNjueb=Eoj8K+8n0g7nj_AcPWSiCj5RNV4fDejAfqA@mail.gmail.com

Rename global variable backing DSA area

The global variable backing the DSA area for Memory Context stats
reporting had a too generic name, rename to be more descriptive.
Independently reported by Peter and Laurenz.

Author: Daniel Gustafsson <[email protected]>
Reported-by: Peter Eisentraut <[email protected]>
Reported-by: Laurenz Albe <[email protected]>
Discussion: https://postgr.es/m/d51172bd4e7f4b07a18a0288ca1b1c28a71a5f6a[email protected]
Discussion: https://postgr.es/m/25095db5-b595-4b85-9100-d358907c25b5@eisentraut.org

Fix memory leak in pg_restore.c

Oversight in 1495eff7bdb

Author: Ranier Vilela <[email protected]>

Doc: remove long-obsolete advice about generated constraint names.

It's been twenty years since we generated constraint names that
look like "$N".  So this advice about double-quoting such names
is well past its sell-by date, and now it merely seems confusing.

Reported-by: Yaroslav Saburov <[email protected]>
Author: "David G. Johnston" <[email protected]>
Discussion: https://postgr.es/m/174393459040.678.17810152410419444783@wrigleys.postgresql.org
Backpatch-through: 13

Remove useless check for negative result of ip_addrsize().

By inspection, ip_addrsize() can't return a negative result.
(If it could, we'd have way bigger problems elsewhere.)
So delete useless check in network_send().  Most C compilers
are probably perfectly capable of removing this code by
themselves, but it's confusing/misleading.

Bug: #18889
Reported-by: Daniel Elishakov <[email protected]>
Discussion: https://postgr.es/m/18889-73d4f19e953a629e@postgresql.org

Further cleanup for directory creation on pg_dump/pg_dumpall

Instead of two separate (and different) implementations, refactor to use
a single common routine.

Along the way, remove use of a hardcoded file permissions constant in
favor of the common project setting for directory creation.

Author: Mahendra Singh Thalor <[email protected]>

Discussion: https://postgr.es/m/CAKYtNApihL8X1h7XO-zOjznc8Ca66Aevgvhc9zOTh6DBh2iaeA@mail.gmail.com

Fix data loss in logical replication.

Data loss can happen when the DDLs like ALTER PUBLICATION ... ADD TABLE ...
or ALTER TYPE ...  that don't take a strong lock on table happens
concurrently to DMLs on the tables involved in the DDL. This happens
because logical decoding doesn't distribute invalidations to concurrent
transactions and those transactions use stale cache data to decode the
changes. The problem becomes bigger because we keep using the stale cache
even after those in-progress transactions are finished and skip the
changes required to be sent to the client.

This commit fixes the issue by distributing invalidation messages from
catalog-modifying transactions to all concurrent in-progress transactions.
This allows the necessary rebuild of the catalog cache when decoding new
changes after concurrent DDL.

We observed performance regression primarily during frequent execution of
*publication DDL* statements that modify the published tables. The
regression is minor or nearly nonexistent for DDLs that do not affect the
published tables or occur infrequently, making this a worthwhile cost to
resolve a longstanding data loss issue.

An alternative approach considered was to take a strong lock on each
affected table during publication modification. However, this would only
address issues related to publication DDLs (but not the ALTER TYPE ...)
and require locking every relation in the database for publications
created as FOR ALL TABLES, which is impractical.

The bug exists in all supported branches, but we are backpatching till 14.
The fix for 13 requires somewhat bigger changes than this fix, so the fix
for that branch is still under discussion.

Reported-by: hubert depesz lubaczewski <[email protected]>
Reported-by: Tomas Vondra <[email protected]>
Author: Shlok Kyal <[email protected]>
Author: Hayato Kuroda <[email protected]>
Reviewed-by: Zhijie Hou <[email protected]>
Reviewed-by: Masahiko Sawada <[email protected]>
Reviewed-by: Amit Kapila <[email protected]>
Tested-by: Benoit Lobréau <[email protected]>
Backpatch-through: 14
Discussion: https://postgr.es/m/de52b282-1166-1180-45a2-8d8917ca74c6@enterprisedb.com
Discussion: https://postgr.es/m/CAD21AoAenVqiMjpN-PvGHL1N9DWnHSq673bfgr6phmBUzx=kLQ@mail.gmail.com

Fix incorrect format placeholders

for commits 8f427187db7, 6ee3b91bad2

Update wording in optimizer/README for EquivalenceClasses

d69d45a5a changed how em_is_child members are stored in
EquivalenceClasses.  Children are no longer stored in the ec_members
list.  optimizer/README mentioned that most operations "should ignore
child members", but that felt a little untrue now since child members
are now stored in a separate place, they simply won't be found by the
normal means of looking (a foreach loop over ec_members), and if you don't
find them, there's technically no need to "ignore" them.

Here we tweak the wording slightly to reflect the new storage location
for child members.

Reported-by: Amit Langote <[email protected]>
Author: Amit Langote <[email protected]>
Author: David Rowley <[email protected]>
Discussion: https://postgr.es/m/CA+HiwqE8v=EuAP_3F_A2xn8zWx+nG_etW_Fe_DvKO-Fkx=+DdQ@mail.gmail.com

Cosmetic fixes for pg_createsubscriber's -all option.

Author: Peter Smith <[email protected]>
Reviewed-by: Amit Kapila <[email protected]>
Discussion: https://postgr.es/m/CAHut+PsmSCQ-ENSDQ0YOUcsgzT=GG-E9jyXBvxd51A_dMXH5XA@mail.gmail.com

ci: Check for missing dependencies in meson builds

Extends the Linux and Windows meson builds with a check for missing
dependencies by running

    ninja -t missingdeps

after the build. This highlights unindended dependencies.

Reviewed-by: Andres Freund <[email protected]>
https://postgr.es/m/CALdSSPi5fj0a7UG7Fmw2cUD1uWuckU_e8dJ+6x-bJEokcSXzqA@mail.gmail.com

Cleanup of pg_numa.c

This moves/renames some of the functions defined in pg_numa.c:

* pg_numa_get_pagesize() is renamed to pg_get_shmem_pagesize(), and
  moved to src/backend/storage/ipc/shmem.c. The new name better reflects
  that the page size is not related to NUMA, and it's specifically about
  the page size used for the main shared memory segment.

* move pg_numa_available() to src/backend/storage/ipc/shmem.c, i.e. into
  the backend (which more appropriate for functions callable from SQL).
  While at it, improve the comment to explain what page size it returns.

* remove unnecessary includes from src/port/pg_numa.c, adding
  unnecessary dependencies (src/port should be suitable for frontent).
  These were either leftovers or unnecessary thanks to the other changes
  in this commit.

This eliminates unnecessary dependencies on backend symbols, which we
don't want in src/port.

Reported-by: Kirill Reshke <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
https://postgr.es/m/CALdSSPi5fj0a7UG7Fmw2cUD1uWuckU_e8dJ+6x-bJEokcSXzqA@mail.gmail.com

pg_upgrade: Mention that we preserve database OIDs in a comment.

Oversight in commit aa01051418.

Reported-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/4055696.1744134682%40sss.pgh.pa.us

Fix performance issue in deadlock-parallel isolation test.

With debug_discard_caches = 1, the runtime of this test script
increased by about a factor of 10 after commit 0dca5d68d.  That's
causing some of our buildfarm animals to fail with a timeout.

The reason for the increased time is that now we are re-planning
some intentionally-non-inlineable SQL functions on every execution,
where the previous coding held onto the original plans throughout
the outer query.  The previous behavior was arguably quite buggy,
so I don't think 0dca5d68d deserves blame here.  But we would
like this test script to not take so long.

To fix, instead of forcing a "parallel safe" label via a
non-inlineable SQL function, apply it directly to the advisory-lock
functions by making internal-language aliases for them.  A small
problem is that the advisory-lock functions return void but this
test would really like them to return integer 1.  I cheated here by
declaring the aliases as returning "int".  That's perhaps undue
familiarity with the implementation of PG_RETURN_VOID(), but that
hasn't changed in twenty years and is unlikely to do so in the next
twenty.  That gets us an integer 0 result, and then an inline-able
wrapper to convert that to an integer 1 allows the rest of the script
to remain unchanged.

For me, this reduces the runtime with debug_discard_caches = 1
by about 100x, making the test comfortably faster than before
instead of slower.

Discussion: https://postgr.es/m/136163.1744179562@sss.pgh.pa.us

Fix test races between syscache-update-pruned.spec and autovacuum.

This spec fails ~3% of my Valgrind runs, and the spec has failed on Valgrind
buildfarm member skink at a similar rate.  Two problems contributed to that:

- A competing buffer pin triggered VACUUM's lazy_scan_noprune() path, causing
  "tuples missed: 1 dead from 1 pages not removed due to cleanup lock
  contention".  FREEZE fixes that.

- The spec ran lazy VACUUM immediately after VACUUM FULL.  The spec implicitly
  assumed lazy VACUUM prunes the one tuple that VACUUM FULL made dead.  First
  wait for old snapshots, making that assumption reliable.

This also adds two forms of defense in depth:

- Wait for snapshots using shared catalog pruning rules (VISHORIZON_SHARED).
  This avoids the removable cutoff moving backward when an XID-bearing
  autoanalyze process runs in another database.  That may never happen in this
  test, but it's cheap insurance.

- Use lazy VACUUM option DISABLE_PAGE_SKIPPING.  Commit
  c2dc1a79767a0f947e1145f82eb65dfe4360d25f did this for a related requirement
  in other tests, but I suspect FREEZE is necessary and sufficient in all
  these tests.

Back-patch to v17, where the test first appeared.

Reported-by: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/sv3taq4e6ea4qckimien3nxp3sz4b6cw6sfcy4nhwl52zpur4g@h6i6tohxmizu
Backpatch-through: 17

Update config.guess and config.sub

Fix a few oversights in the longer cancel keys patch

Change MyCancelKeyLength's type from uint8 to int. While it always
fits in a uint8, plain int is less surprising, as there's no
particular reason for it to be uint8.

Fix one ProcSignalInit caller that passed 'false' instead of NULL for
the pointer argument.

Author: Peter Eisentraut <[email protected]>
Discussion: https://www.postgresql.org/message-id/61be9e31-7b7d-49d5-bc11-721800d89d64@eisentraut.org

Perform missed catversion bump

Commit c57971034e69ca renamed an argument for a function but missed
to bump the catversion to reflect this.

Reported-by: David Rowley <[email protected]>
Discussion: https://postgr.es/m/CAApHDvqOega=dPtu3h2C5fJWJEuaGCMDib_sVfhKQqgUNJVmFA@mail.gmail.com

Doc: note that two examples in optimizer/README are oversimplified.

These examples fail to account for join clauses generated by
EquivalenceClasses, but since we haven't mentioned EquivalenceClasses
yet it seems like it'd just add confusion to make them fully accurate.
Instead, parenthetically note that they're oversimplified.

Reported-by: Zeyuan Hu <[email protected]>
Co-authored-by: David Rowley <[email protected]>
Co-authored-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/CACvHWmYFo+60yMqKJajDDvKN5EM41YHrCT3oxukwXmGAqpWvyw@mail.gmail.com

Adjust AdjustUpgrade.pm for commit b1720fe63.

Need to delete the functions we no longer have available from
the dumps to be reloaded from old versions.

Per buildfarm.

Move contrib/spi testing from core regression tests to contrib/spi.

It's weird to have the core regression tests depending on contrib
code, and coverage testing shows that those test queries add nothing
to the core-code coverage of the core tests.  So pull those test bits
out and put them into ordinary test scripts inside contrib/spi/,
making that more like other contrib modules.

Aside from being structurally nicer, anything we can take out of the
core tests (which are executed multiple times per check-world run)
and put into tests executed only once should be a win.  It doesn't
look like this change will buy a whole lot of milliseconds, but a
cycle saved is a cycle earned.

Also, there is some discussion around possibly removing refint and/or
autoinc altogether.  I don't know if that will happen, but we'd
certainly need to decouple them from the core tests to do so.

The tests for autoinc were quite intertwined with the undocumented
"ttdummy" trigger in regress.c.  That made the tests very hard to
understand and contributed nothing to autoinc's testing either.
So I just deleted ttdummy and rewrote the autoinc tests without it.

I realized while doing this that the description of autoinc in
the SGML docs is not a great description of what the function
actually does, so the patch includes some updates to those docs.

Author: Tom Lane <[email protected]>
Reviewed-by: Heikki Linnakangas <[email protected]>
Discussion: https://postgr.es/m/3872677.1744077559@sss.pgh.pa.us

Rename argument in pg_get_process_memory_contexts().

During development the third argument to pg_get_process_memory_contexts
was a retry count, but it was changed to a timeout instead.  The param
name was accidentally left in pg_proc.dat though.  Fix by renaming to
the correct parameter name.

Author: Fujii Masao <[email protected]>
Reviewed-by: Daniel Gustafsson <[email protected]>
Discussion: https://postgr.es/m/3eb40b3e-45c7-426a-b7f8-81f7d05a9b53@oss.nttdata.com

Fix incorrect format placeholder

for commit 749a9e20c97

Prevent 006_transfer_modes.pl from leaving files behind.

This test was leaving files like delete_old_cluster.{sh,bat} in the
source directory for VPATH and meson builds.  To fix, change the
directory to tmp_check before running the test, as was done in
commits 15b6d21553, 8af917be6b, and c462b054ba.

Oversight in commit af0d4901c1.

Reported-by: Andrew Dunstan <[email protected]> (on Discord)
Reviewed-by: Michael Paquier <[email protected]>
Reviewed-by: Andrew Dunstan <[email protected]>
Reviewed-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/Z_RHkG770w3SE0yU%40nathan

ci: Add MBUILD_TARGET for NetBSD and OpenBSD

Commit b2bdb972c0 added MBUILD_TARGET to ensure that meson builds
the tests before running them, this adds MBUILD_TARGET to OpenBSD
and NetBSD builds as well where it was missing.

No backpatching since OpenBSD and NetBSD support does not exist
in the backbranch CI.

Author: Nazir Bilal Yavuz <[email protected]>
Reviewed-by: Daniel Gustafsson <[email protected]>
Discussion: https://postgr.es/m/CAN55FZ2LNnRrtL+cpSdEg44fQcLPq_GjJjfNa0vz+xqEdq=ZHw@mail.gmail.com