common/jsonapi: support libpq as a client

Based on a  by Michael Paquier.

For libpq, use PQExpBuffer instead of StringInfo. This requires us to
track allocation failures so that we can return JSON_OUT_OF_MEMORY as
needed rather than exit()ing.

Author: Jacob Champion <[email protected]>
Co-authored-by: Michael Paquier <[email protected]>
Co-authored-by: Daniel Gustafsson <[email protected]>
Reviewed-by: Peter Eisentraut <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/d1b467a78e0e36ed85a09adf979d04cf124a9d4b[email protected]

Improve assertion in FindReplTupleInLocalRel().

The first part of the assertion verifying that the passed index must be PK
or RI was incorrectly passing index relation instead of heap relation in
GetRelationIdentityOrPK(). The assertion was not failing because the
second part of the assertion which needs to be performed only when remote
relation has REPLICA_IDENTITY_FULL set was also incorrect.

The change is not backed because the current coding doesn't lead to
any failure.

Reported-by: Dilip Kumar
Author: Amit Kapila
Reviewed-by: Vignesh C
Discussion: https://postgr.es/m/CAFiTN-tmguaT1DXbCC+ZomZg-oZLmU6BPhr0po7akQSG6vNJrg@mail.gmail.com

Optimize pg_visibility with read streams.

We've measured 5% performance improvement, and this arranges to benefit
automatically from future optimizations to the read_stream subsystem.
The area lacked test coverage, so close that gap.

Nazir Bilal Yavuz

Discussion: https://postgr.es/m/CAN55FZ1_Ru3XpMgTwsU67FTH2fs_FrRROmb7x6zs+F44QBEiww@mail.gmail.com
Discussion: https://postgr.es/m/CAEudQAozv3wTY5TV2t29JcwPydbmKbiWQkZD42S2OgzdixPMDQ@mail.gmail.com

Use a hash table to de-duplicate column names in ruleutils.c.

Commit 8004953b5 added a hash table to avoid O(N^2) cost in choosing
unique relation aliases while deparsing a view or rule.  It did
nothing about the similar O(N^2) (maybe worse) costs of choosing
unique column aliases within each RTE.  However, that's now
demonstrably a bottleneck when deparsing CHECK constraints for wide
tables, so let's use a similar hash table to handle those.

The extra cost of setting up the hash table will not be repaid unless
the table has many columns.  I've set this up so that we use the brute
force method if there are less than 32 columns.  The exact cutoff is
not too critical, but this value seems good because it results in both
code paths getting exercised by existing regression-test cases.

 by me; thanks to David Rowley for review.

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

Fix some whitespace issues in XMLSERIALIZE(... INDENT).

We must drop whitespace while parsing the input, else libxml2
will include "blank" nodes that interfere with the desired
indentation behavior.  The end result is that we didn't indent
nodes separated by whitespace.

Also, it seems that libxml2 may add a trailing newline when working
in DOCUMENT mode.  This is semantically insignificant, so strip it.

This is in the gray area between being a bug fix and a definition
change.  However, the INDENT option is still pretty new (since v16),
so I think we can get away with changing this in stable branches.
Hence, back- to v16.

Jim Jones

Discussion: https://postgr.es/m/872865a8-548b-48e1-bfcd-4e38e672c1e4@uni-muenster.de

Improve documentation and testing of jsonpath string() for datetimes.

Point out that the output format depends on DateStyle, and test that,
along with testing some cases previously not covered.

In passing, adjust the horology test to verify that the prevailing
DateStyle is 'Postgres, MDY', much as it has long verified the
prevailing TimeZone.  We expect pg_regress to have set these up,
and there are multiple regression tests relying on these settings.

Also make the formatting of entries in table 9.50 more consistent.

David Wheeler (marginal additional hacking by me); review by jian he

Discussion: https://postgr.es/m/56955B33-6959-4FDA-A459-F00363ECDFEE@justatheory.com

Add PG_TEST_PG_COMBINEBACKUP_MODE to CI tasks

The environment variable PG_TEST_PG_COMBINEBACKUP_MODE has been
available since 35a7b288b975, but was not set by any built-in CI tasks.
This commit modifies two of the CI tasks to use the alternative modes,
to exercise the pg_combinebackup code.

The Linux task uses --copy-file-range, macOS uses --clone.

This is not an exhaustive test of combinations. The supported modes
depend on the operating system and filesystem, and it would be nice to
test all supported combinations. Right now we have just one task for
each OS, and it doesn't seem worth adding more just for this.

Reported-by: Peter Eisentraut
Reviewed-by: Peter Eisentraut
Discussion: https://postgr.es/m/48da4a1f-ccd9-4988-9622-24f37b1de2b4%40eisentraut.org

Protect against small overread in SASLprep validation

In case of torn UTF8 in the input data we might end up going
past the end of the string since we don't account for length.
While validation won't be performed on a sequence with a NULL
byte it's better to avoid going past the end to beging with.
Fix by taking the length into consideration.

Author: Jacob Champion <[email protected]>
Reviewed-by: Daniel Gustafsson <[email protected]>
Discussion: https://postgr.es/m/CAOYmi+mTnmM172g=_+Yvc47hzzeAsYPy2C4UBY3HK9p-AXNV0g@mail.gmail.com

Add amgettreeheight index AM API routine

The only current implementation is for btree where it calls
_bt_getrootheight().  Other index types can now also use this to pass
information to their amcostestimate routine.  Previously, btree was
hardcoded and other index types could not hook into the optimizer at
this point.

Author: Mark Dilger <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/E72EAA49-354D-4C2E-8EB9-255197F55330@enterprisedb.com

Mark expressions nullable by grouping sets

When generating window_pathkeys, distinct_pathkeys, or sort_pathkeys,
we failed to realize that the grouping/ordering expressions might be
nullable by grouping sets.  As a result, we may incorrectly deem that
the PathKeys are redundant by EquivalenceClass processing and thus
remove them from the pathkeys list.  That would lead to wrong results
in some cases.

To fix this issue, we mark the grouping expressions nullable by
grouping sets if that is the case.  If the grouping expression is a
Var or PlaceHolderVar or constructed from those, we can just add the
RT index of the RTE_GROUP RTE to the existing nullingrels field(s);
otherwise we have to add a PlaceHolderVar to carry on the nullingrel
bit.

However, we have to manually remove this nullingrel bit from
expressions in various cases where these expressions are logically
below the grouping step, such as when we generate groupClause pathkeys
for grouping sets, or when we generate PathTarget for initial input to
grouping nodes.

Furthermore, in set_upper_references, the targetlist and quals of an
Agg node should have nullingrels that include the effects of the
grouping step, ie they will have nullingrels equal to the input
Vars/PHVs' nullingrels plus the nullingrel bit that references the
grouping RTE.  In order to perform exact nullingrels matches, we also
need to manually remove this nullingrel bit.

Bump catversion because this changes the querytree produced by the
parser.

Thanks to Tom Lane for the idea to invent a new kind of RTE.

Per reports from Geoff Winkless, Tobias Wendorff, Richard Guo from
various threads.

Author: Richard Guo
Reviewed-by: Ashutosh Bapat, Sutou Kouhei
Discussion: https://postgr.es/m/CAMbWs4_dp7e7oTwaiZeBX8+P1rXw4ThkZxh1QG81rhu9Z47VsQ@mail.gmail.com

Introduce an RTE for the grouping step

If there are subqueries in the grouping expressions, each of these
subqueries in the targetlist and HAVING clause is expanded into
distinct SubPlan nodes.  As a result, only one of these SubPlan nodes
would be converted to reference to the grouping key column output by
the Agg node; others would have to get evaluated afresh.  This is not
efficient, and with grouping sets this can cause wrong results issues
in cases where they should go to NULL because they are from the wrong
grouping set.  Furthermore, during re-evaluation, these SubPlan nodes
might use  column values from grouping sets, which is not
correct.

This issue is not limited to subqueries.  For other types of
expressions that are part of grouping items, if they are transformed
into another form during preprocessing, they may fail to match lower
target items.  This can also lead to wrong results with grouping sets.

To fix this issue, we introduce a new kind of RTE representing the
output of the grouping step, with columns that are the Vars or
expressions being grouped on.  In the parser, we replace the grouping
expressions in the targetlist and HAVING clause with Vars referencing
this new RTE, so that the output of the parser directly expresses the
semantic requirement that the grouping expressions be gotten from the
grouping output rather than computed some other way.  In the planner,
we first preprocess all the columns of this new RTE and then replace
any Vars in the targetlist and HAVING clause that reference this new
RTE with the underlying grouping expressions, so that we will have
only one instance of a SubPlan node for each subquery contained in the
grouping expressions.

Bump catversion because this changes the querytree produced by the
parser.

Thanks to Tom Lane for the idea to invent a new kind of RTE.

Per reports from Geoff Winkless, Tobias Wendorff, Richard Guo from
various threads.

Author: Richard Guo
Reviewed-by: Ashutosh Bapat, Sutou Kouhei
Discussion: https://postgr.es/m/CAMbWs4_dp7e7oTwaiZeBX8+P1rXw4ThkZxh1QG81rhu9Z47VsQ@mail.gmail.com

Remove emode argument from XLogFileRead() and XLogFileReadAnyTLI()

This change makes the code slightly easier to reason about, because
there is actually no need to know if a specific caller of one of these
routines should fail hard on a PANIC, or just let it go through with a
DEBUG2.

The only caller of XLogFileReadAnyTLI() used DEBUG2, and XLogFileRead()
has never used its emode.  This can be simplified since 1bb2558046cc
that has introduced XLogFileReadAnyTLI(), splitting both.

Author: Yugo Nagata
Discussion: https://postgr.es/m/20240906201043.a640f3b44e755d4db2b6943e@sraoss.co.jp

Add WAL usage reporting to ANALYZE VERBOSE output.

This change adds WAL usage reporting to the output of ANALYZE VERBOSE
and autoanalyze reports. It aligns the analyze output with VACUUM,
providing consistency. Additionally, it aids in troubleshooting cases
where WAL records are generated during analyze operations.

Author: Anthonin Bonnefoy
Reviewed-by: Masahiko Sawada
Discussion: https://postgr.es/m/CAO6_Xqr__kTTCLkftqS0qSCm-J7_xbRG3Ge2rWhucxQJMJhcRA%40mail.gmail.com

Consistently use PageGetExactFreeSpace() in pgstattuple.

Previously this code used PageGetHeapFreeSpace on heap pages,
and usually used PageGetFreeSpace on index pages (though for some
reason GetHashPageStats used PageGetExactFreeSpace instead).
The difference is that those functions subtract off the size of
a line pointer, and PageGetHeapFreeSpace has some additional
rules about returning zero if adding another line pointer would
require exceeding MaxHeapTuplesPerPage.  Those things make sense
when testing to see if a new tuple can be put on the page, but
they seem pretty strange for pure statistics collection.

Additionally, statapprox_heap had a special rule about counting
a "new" page as being fully available space.  This also seems
strange, because it's not actually usable until VACUUM or some
such process initializes the page.  Moreover, it's inconsistent
with what pgstat_heap does, which is to count such a page as
having zero free space.  So make it work like pgstat_heap, which
as of this  unconditionally calls PageGetExactFreeSpace.

This is more of a definitional change than a bug fix, so no
back-.  The module's documentation doesn't define exactly
what "free space" means either, so we left that as-is.

Frédéric Yhuel, reviewed by Rafia Sabih and Andreas Karlsson.

Discussion: https://postgr.es/m/3a18f843-76f6-4a84-8cca-49537fefa15d@dalibo.com

Don't bother checking the result of SPI_connect[_ext] anymore.

SPI_connect/SPI_connect_ext have not returned any value other than
SPI_OK_CONNECT since commit 1833f1a1c in v10; any errors are thrown
via ereport.  (The most likely failure is out-of-memory, which has
always been thrown that way, so callers had better be prepared for
such errors.)  This makes it somewhat pointless to check these
functions' result, and some callers within our code haven't been
bothering; indeed, the only usage example within spi.sgml doesn't
bother.  So it's likely that the omission has propagated into
extensions too.

Hence, let's standardize on not checking, and document the return
value as historical, while not actually changing these functions'
behavior.  (The original proposal was to change their return type
to "void", but that would needlessly break extensions that are
conforming to the old practice.)  This saves a small amount of
boilerplate code in a lot of places.

Stepan Neretin

Discussion: https://postgr.es/m/CAMaYL5Z9Uk8cD9qGz9QaZ2UBJFOu7jFx5Mwbznz-1tBbPDQZow@mail.gmail.com

Add PQfullProtocolVersion() to surface the precise protocol version.

The existing function PQprotocolVersion() does not include the minor
version of the protocol.  In preparation for pending work that will
bump that number for the first time, add a new function to provide it
to clients that may care, using the (major * 10000 + minor)
convention already used by PQserverVersion().

Jacob Champion based on earlier work by Jelte Fennema-Nio

Discussion: http://postgr.es/m/CAOYmi+mM8+6Swt1k7XsLcichJv8xdhPnuNv7-02zJWsezuDL+g@mail.gmail.com

Fix waits of REINDEX CONCURRENTLY for indexes with predicates or expressions

As introduced by f9900df5f94, a REINDEX CONCURRENTLY job done for an
index with predicates or expressions would set PROC_IN_SAFE_IC in its
MyProc->statusFlags, causing it to be ignored by other concurrent
operations.

Such concurrent index rebuilds should never be ignored, as a predicate
or an expression could call a user-defined function that accesses a
different table than the table where the index is rebuilt.

A test that uses injection points is added, backed down to 17.
Michail has proposed a different test, but I have added something
simpler with more coverage.

Oversight in f9900df5f949.

Author: Michail Nikolaev
Discussion: https://postgr.es/m/CANtu0oj9A3kZVduFTG0vrmGnKB+DCHgEpzOp0qAyOgmks84j0w@mail.gmail.com
Back-through: 14

SQL/JSON: Avoid initializing unnecessary ON ERROR / ON EMPTY steps

When the ON ERROR / ON EMPTY behavior is to return NULL, returning
NULL directly from ExecEvalJsonExprPath() suffices. Therefore, there's
no need to create separate steps to check the error/empty flag or
those to evaluate the the constant NULL expression.  This speeds up
common cases because the default ON ERROR / ON EMPTY behavior for
JSON_QUERY() and JSON_VALUE() is to return NULL.  However, these steps
are necessary if the RETURNING type is a domain, as constraints on the
domain may need to be checked.

Reported-by: Jian He <[email protected]>
Author: Jian He <[email protected]>
Author: Amit Langote <[email protected]>
Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Back-through: 17

Fix order of parameters in a cost_sort call

In label_sort_with_costsize, the cost_sort function is called with the
parameters 'input_disabled_nodes' and 'input_cost' in the wrong order.
This does not cause any plan diffs in the regression tests, because
label_sort_with_costsize is only used to label the Sort node nicely
for EXPLAIN, and cost numbers are not displayed in regression tests.

Oversight in e22253467.  Fixed by passing arguments in the right
order.

Per report from Alexander Lakhin running UBSan.

Author: Alexander Lakhin
Discussion: https://postgr.es/m/a9b7231d-68bc-f117-a07c-96688f3e6aef@gmail.com

Add callbacks to control flush of fixed-numbered stats

This commit adds two callbacks in pgstats to have a better control of
the flush timing of pgstat_report_stat(), whose operation depends on the
three PGSTAT_*_INTERVAL variables:
- have_fixed_pending_cb(), to check if a stats kind has any pending
data waiting for a flush.  This is used as a fast path if there are no
pending statistics to flush, and this check is done for fixed-numbered
statistics only if there are no variable-numbered statistics to flush.
A flush will need to happen if at least one callback reports any pending
data.
- flush_fixed_cb(), to do the actual flush.

These callbacks are currently used by the SLRU, WAL and IO statistics,
generalizing the concept for all stats kinds (builtin and custom).

The SLRU and IO stats relied each on one global variable to determine
whether a flush should happen; these are now local to pgstat_slru.c and
pgstat_io.c, cleaning up a bit how the pending flush states are tracked
in pgstat.c.

pgstat_flush_io() and pgstat_flush_wal() are still required, but we do
not need to check their return result anymore.

Reviewed-by: Bertrand Drouvot, Kyotaro Horiguchi
Discussion: https://postgr.es/m/[email protected]

Avoid core dump after getpwuid_r failure.

Looking up a nonexistent user ID would lead to a null pointer
dereference.  That's unlikely to happen here, but perhaps
not impossible.

Thinko in commit 4d5111b3f, noticed by Coverity.

Update extension lookup routines to use the syscache

The following routines are changed to use the syscache entries added for
pg_extension in 490f869d92e5:
- get_extension_oid()
- get_extension_name()
- get_extension_schema()

A catalog scan is costly and could easily lead to a noticeable
performance impact when called once or more per query, so this is going
to be helpful for developers for extension data lookups.

Author: Andrei Lepikhov
Reviewed-by: Jelte Fennema-Nio
Discussion: https://postgr.es/m/529295b2-6ba9-4dae-acd1-20a9c6fb8f9a@gmail.com

Remove lc_ctype_is_c().

Instead always fetch the locale and look at the ctype_is_c field.

hba.c relies on regexes working for the C locale without needing
catalog access, which worked before due to a special case for
C_COLLATION_OID in lc_ctype_is_c(). Move the special case to
pg_set_regex_collation() now that lc_ctype_is_c() is gone.

Author: Andreas Karlsson
Discussion: https://postgr.es/m/60929555-4709-40a7-b136-bcb44cff5a3c@proxel.se

Fix incorrect pg_stat_io output on 32-bit machines.

pg_stat_get_io() applied TimestampTzGetDatum twice to the
stat_reset_timestamp value.  On 64-bit builds that's harmless because
TimestampTzGetDatum is a no-op, but on 32-bit builds it results in
displaying garbage in the stats_reset column of the pg_stat_io view.

Bug dates to commit a9c70b46d which introduced pg_stat_io, so
back- to v16 where that came in.

Bertrand Drouvot

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

Remove useless unconstify

Digging into the history, this was not necessary even when it was
added, but might have been some time before that.  In any case, there
is no use for this now.

SQL/JSON: Fix default ON ERROR behavior for JSON_TABLE

Use EMPTY ARRAY instead of EMPTY.

This change does not affect the runtime behavior of JSON_TABLE(),
which continues to return an empty relation ON ERROR. It only alters
whether the default ON ERROR behavior is shown in the deparsed output.

Reported-by: Jian He <[email protected]>
Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Back-through: 17

SQL/JSON: Fix JSON_TABLE() column deparsing

The deparsing code in get_json_expr_options() unnecessarily emitted
the default column-specific ON ERROR / EMPTY behavior when the
top-level ON ERROR behavior in JSON_TABLE was set to ERROR. Fix that
by not overriding the column-specific default, determined based on
the column's JsonExprOp in get_json_table_columns(), with
JSON_BEHAVIOR_ERROR when that is the top-level ON ERROR behavior.

Note that this only removes redundancy; the current deparsing output
is not incorrect, just redundant.

Reviewed-by: Jian He <[email protected]>
Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Back-through: 17

Revert recent SQL/JSON related commits

Reverts 68222851d5a8, 565caaa79af, and 3a97460970f, because a few
BF animals didn't like one or all of them.

SQL/JSON: Avoid initializing unnecessary ON ERROR / ON EMPTY steps

When the ON ERROR / ON EMPTY behavior is to return NULL, returning
NULL directly from ExecEvalJsonExprPath() suffices. Therefore, there's
no need to create separate steps to check the error/empty flag or
those to evaluate the the constant NULL expression.  This speeds up
common cases because the default ON ERROR / ON EMPTY behavior for
JSON_QUERY() and JSON_VALUE() is to return NULL.  However, these steps
are necessary if the RETURNING type is a domain, as constraints on the
domain may need to be checked.

Reported-by: Jian He <[email protected]>
Author: Jian He <[email protected]>
Author: Amit Langote <[email protected]>
Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Back-through: 17

SQL/JSON: Fix default ON ERROR behavior for JSON_TABLE

Use EMPTY ARRAY instead of EMPTY.

This change does not affect the runtime behavior of JSON_TABLE(),
which continues to return an empty relation ON ERROR. It only alters
whether the default ON ERROR behavior is shown in the deparsed output.

Reported-by: Jian He <[email protected]>
Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Back-through: 17

SQL/JSON: Fix JSON_TABLE() column deparsing

The deparsing code in get_json_expr_options() unnecessarily emitted
the default column-specific ON ERROR / EMPTY behavior when the
top-level ON ERROR behavior in JSON_TABLE was set to ERROR. Fix that
by not overriding the column-specific default, determined based on
the column's JsonExprOp in get_json_table_columns(), with
JSON_BEHAVIOR_ERROR when that is the top-level ON ERROR behavior.

Note that this only removes redundancy; the current deparsing output
is not incorrect, just redundant.

Reviewed-by: Jian He <[email protected]>
Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Back-through: 17

Update comment about ExprState.escontext

The updated comment provides more helpful guidance by mentioning that
escontext should be set when soft error handling is needed.

Reported-by: Jian He <[email protected]>
Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Back-through: 17

Be more careful with error paths in pg_set_regex_collation().

Set global variables after error paths so that they don't end up in an
inconsistent state.

The inconsistent state doesn't lead to an actual problem, because
after an error, pg_set_regex_collation() will be called again before
the globals are accessed.

Change extracted from  by Andreas Karlsson, though not discussed
explicitly.

Discussion: https://postgr.es/m/60929555-4709-40a7-b136-bcb44cff5a3c@proxel.se

Prevent mis-encoding of "trailing junk after numeric literal" errors.

Since commit 2549f0661, we reject an identifier immediately following
a numeric literal (without separating whitespace), because that risks
ambiguity with hex/octal/binary integers.  However, that  used
token patterns like "{integer}{ident_start}", which is problematic
because {ident_start} matches only a single byte.  If the first
character after the integer is a multibyte character, this ends up
with flex reporting an error message that includes a partial multibyte
character.  That can cause assorted bad-encoding problems downstream,
both in the report to the client and in the postmaster log file.

To fix, use {identifier} not {ident_start} in the "junk" token
patterns, so that they will match complete multibyte characters.
This seems generally better user experience quite aside from the
encoding problem: for "123abc" the error message will now say that
the error appeared at or near "123abc" instead of "123a".

While at it, add some commentary about why these patterns exist
and how they work.

Report and  by Karina Litskevich; review by Pavel Borisov.
Back- to v15 where the problem came in.

Discussion: https://postgr.es/m/CACiT8iZ_diop=0zJ7zuY3BXegJpkKK1Av-PU7xh0EDYHsa5+=g@mail.gmail.com

Fix handling of NULL return value in typarray lookup

Commit 6ebeeae29 accidentally omitted testing the return value from
findTypeByOid which can return NULL.  Fix by adding a check to make
sure that we have a pointer to dereference.

Author: Ranier Vilela <[email protected]>
Reviewed-by: Nathan Bossart <[email protected]>
Reviewed-by: Daniel Gustafsson <[email protected]>
Discussion: https://postgr.es/m/CAEudQAqfMTH8Ya_J6E-NW_y_JyDFDxtQ4V_g6nY_1=0oDbQqdg@mail.gmail.com

Fix misleading error message context

Author: Pavel Stehule <[email protected]>
Reviewed-by: Stepan Neretin <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/CAFj8pRAw+OkVW=FgMKHKyvY3CgtWy3cWdY7XT+S5TJaTttu=oA@mail.gmail.com

Add callback for backend initialization in pgstats

pgstat_initialize() is currently used by the WAL stats as a code path to
take some custom actions when a backend starts.  A callback is added to
generalize the concept so as all stats kinds can do the same, for
builtin and custom kinds, if set.

Reviewed-by: Bertrand Drouvot, Kyotaro Horiguchi
Discussion: https://postgr.es/m/[email protected]

Fix two NULL pointer dereferences when reading custom pgstats from file

There were two spots in pgstat_read_statsfile() where is was possible to
finish with a null-pointer-dereference crash for custom pgstats kinds:
- When reading stats for a fixed-numbered stats entry.
- When reading a variable stats entry with name serialization.
For both cases, these issues were reachable by starting a server after
changing shared_preload_libraries so as the stats written previously
could not be loaded.

The code is changed so as the stats are ignored in this case, like the
other code paths doing similar sanity checks.  Two WARNINGs are added to
be able to debug these issues.  A test is added for the case of
fixed-numbered stats with the module injection_points.

Oversights in 7949d9594582, spotted while looking at a different report.

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

Check availability of module injection_points in TAP tests

This fixes defects with installcheck for TAP tests that expect the
module injection_points to exist in an installation, but the contents of
src/test/modules are not installed by default with installcheck.  This
would cause, for example, failures under installcheck-world for a build
with injection points enabled, when the contents of src/test/modules/
are not installed.

The availability of the module can be done with a scan of
pg_available_extension.  This has been introduced in 2cdcae9da696, and
it is refactored here as a new routine in Cluster.pm.

Tests are changed in different ways depending on what they need:
- The libpq TAP test sets up a node even without injection points, so it
is enough to check that CREATE EXTENSION can be used.  There is no need
for the variable enable_injection_points.
- In test_misc, 006_signal_autovacuum requires a runtime check.
- 041_checkpoint_at_promote in recovery tests and 005_timeouts in
test_misc are updated to use the routine introduced in Cluster.pm.
- test_slru's 001_multixact, injection_points's 001_stats and
modules/gin/ do not require a check as these modules disable
installcheck entirely.

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

Optimize WindowAgg's use of tuplestores

When WindowAgg finished one partition of a PARTITION BY, it previously
would call tuplestore_end() to purge all the stored tuples before again
calling tuplestore_begin_heap() and carefully setting up all of the
tuplestore read pointers exactly as required for the given frameOptions.
Since the frameOptions don't change between partitions, this part does
not make much sense.  For queries that had very few rows per partition,
the overhead of this was very large.

It seems much better to create the tuplestore and the read pointers once
and simply call tuplestore_clear() at the end of each partition.
tuplestore_clear() moves all of the read pointers back to the start
position and deletes all the previously stored tuples.

A simple test query with 1 million partitions and 1 tuple per partition
has been shown to run around 40% faster than without this change.  The
additional effort seems to have mostly been spent in malloc/free.

Making this work required adding a new bool field to WindowAggState
which had the unfortunate effect of being the 9th bool field in a group
resulting in the struct being enlarged.  Here we shuffle the fields
around a little so that the two bool fields for runcondition relating
stuff fit into existing padding.  Also, move the "runcondition" field to
be near those.  This frees up enough space with the other bool fields so
that the newly added one fits into the padding bytes.  This was done to
address a very small but apparent performance regression with queries
containing a large number of rows per partition.

Reviewed-by: Ashutosh Bapat <[email protected]>
Reviewed-by: Tatsuo Ishii <[email protected]>
Discussion: https://postgr.es/m/CAHoyFK9n-QCXKTUWT_xxtXninSMEv%2BgbJN66-y6prM3f4WkEHw%40mail.gmail.com

Speedup WindowAgg code by moving uncommon code out-of-line

The code to calculate the frame offsets is only performed once per scan.
Moving this code out of line gives a small (around 4-5%) speedup when testing
with some CPUs.  Other tested CPUs are indifferent to the change.

Reviewed-by: Ashutosh Bapat <[email protected]>
Reviewed-by: Tatsuo Ishii <[email protected]>
Discussion: https://postgr.es/m/CAApHDvqPgFtwme2Zyf75BpMLwYr2mnUstDyPiP%3DEpudYuQTPPQ%40mail.gmail.com

Remove lc_collate_is_c().

Instead just look up the collation and check collate_is_c field.

Author: Andreas Karlsson
Discussion: https://postgr.es/m/60929555-4709-40a7-b136-bcb44cff5a3c@proxel.se

Remove test-case workarounds for ancient libedit versions.

This reverts some hacks added in d33a81203 and cd69ec66c.
At the time the concern was the already-ancient version of
libedit shipped in Debian 10 (Buster).  That platform is
now two years past EOL, so desupporting it for PG 18 seems
fine.  (Also, if anyone is really hot to keep testing it,
they can use SKIP_READLINE_TESTS to skip this test.)

We might have to reconsider if any animals still in the
buildfarm don't like this, but the best way to find out
is to try it.

Anton Melnikov

Discussion: https://postgr.es/m/CAGRrpzZU48F2oV3d8eDLr=4TU9xFH5Jt9ED+qU1+X91gMH68Sw@mail.gmail.com

Revert "Optimize pg_visibility with read streams."

This reverts commit ed1b1ee59fb3792baa32f669333b75024ef01bcc and its
followup 1c61fd8b527954f0ec522e5e60a11ce82628b681.  They rendered
collect_corrupt_items() unable to detect corruption.

Discussion: https://postgr.es/m/CAN55FZ1_Ru3XpMgTwsU67FTH2fs_FrRROmb7x6zs+F44QBEiww@mail.gmail.com
Discussion: https://postgr.es/m/CAEudQAozv3wTY5TV2t29JcwPydbmKbiWQkZD42S2OgzdixPMDQ@mail.gmail.com

Remove a couple of strerror() calls

Change to using %m in the error message string.  We need to be a bit
careful here to preserve errno until we need to print it.

This change avoids the use of not-thread-safe strerror() and unifies
some error message strings, and maybe makes the code appear more
consistent.

Reviewed-by: Tom Lane <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/daa87d79-c044-46c4-8458-8d77241ed7b0%40eisentraut.org

Unify some error messages to ease work of translators

This commit updates a couple of error messages around control file data,
GUCs and server settings, unifying to the same message where possible.
This reduces the translation burden a bit.

Author: Peter Smith
Discussion: https://postgr.es/m/CAHut+Pv-kSN8SkxSdoHano_wPubqcg5789ejhCDZAcLFceBR-w@mail.gmail.com

Apply more quoting to GUC names in messages

This is a continuation of 17974ec25946.  More quotes are applied to
GUC names in error messages and hints, taking care of what seems to be
all the remaining holes currently in the tree for the GUCs.

Author: Peter Smith
Discussion: https://postgr.es/m/CAHut+Pv-kSN8SkxSdoHano_wPubqcg5789ejhCDZAcLFceBR-w@mail.gmail.com

Collect statistics about conflicts in logical replication.

This commit adds columns in view pg_stat_subscription_stats to show the
number of times a particular conflict type has occurred during the
application of logical replication changes. The following columns are
added:

confl_insert_exists:
        Number of times a row insertion violated a NOT DEFERRABLE unique
        constraint.
confl_update_origin_differs:
        Number of times an update was performed on a row that was
        previously modified by another origin.
confl_update_exists:
        Number of times that the updated value of a row violates a
        NOT DEFERRABLE unique constraint.
confl_update_missing:
        Number of times that the tuple to be updated is missing.
confl_delete_origin_differs:
        Number of times a delete was performed on a row that was
        previously modified by another origin.
confl_delete_missing:
        Number of times that the tuple to be deleted is missing.

The update_origin_differs and delete_origin_differs conflicts can be
detected only when track_commit_timestamp is enabled.

Author: Hou Zhijie
Reviewed-by: Shveta Malik, Peter Smith, Anit Kapila
Discussion: https://postgr.es/m/OS0PR01MB57160A07BD575773045FC214948F2@OS0PR01MB5716.jpnprd01.prod.outlook.com

Avoid unnecessary post-sort projection

When generating paths for the ORDER BY clause, one thing we need to
ensure is that the output paths project the correct final_target.  To
achieve this, in create_ordered_paths, we compare the pathtarget of
each generated path with the given 'target', and add a post-sort
projection step if the two targets do not match.

Currently we perform a simple pointer comparison between the two
targets.  It turns out that this is not sufficient.  Each sorted_path
generated in create_ordered_paths initially projects the correct
target required by the preceding steps of sort.  If it is the same
pointer as sort_input_target, pointer comparison suffices, because
sort_input_target is always identical to final_target when no
post-sort projection is needed.

However, sorted_path's initial pathtarget may not be the same pointer
as sort_input_target, because in apply_scanjoin_target_to_paths, if
the target to be applied has the same expressions as the existing
reltarget, we only inject the sortgroupref info into the existing
pathtargets, rather than create new projection paths.  As a result,
pointer comparison in create_ordered_paths is not reliable.

Instead, we can compare PathTarget.exprs to determine whether a
projection step is needed.  If the expressions match, we can be
confident that a post-sort projection is not required.

It could be argued that this change adds extra check cost each time we
decide whether a post-sort projection is needed.  However, as
explained in apply_scanjoin_target_to_paths, by avoiding the creation
of projection paths, we save effort both immediately and at plan
creation time.  This, I think, justifies the extra check cost.

There are two ensuing plan changes in the regression tests, but they
look reasonable and are exactly what we are fixing here.  So no
additional test cases are added.

No back as this could result in plan changes.

Author: Richard Guo
Reviewed-by: Peter Eisentraut, David Rowley, Tom Lane
Discussion: https://postgr.es/m/CAMbWs48TosSvmnz88663_2yg3hfeOFss-J2PtnENDH6J_rLnRQ@mail.gmail.com

Check the validity of commutators for merge/hash clauses

When creating merge or hash join plans in createplan.c, the merge or
hash clauses may need to get commuted to ensure that the outer var is
on the left and the inner var is on the right if they are not already
in the expected form.  This requires that their operators have
commutators.  Failing to find a commutator at this stage would result
in 'ERROR: could not find commutator for operator xxx', with no
opportunity to select an alternative plan.

Typically, this is not an issue because mergejoinable or hashable
operators are expected to always have valid commutators.  But in some
artificial cases this assumption may not hold true.  Therefore, here
in this  we check the validity of commutators for clauses in the
form "inner op outer" when selecting mergejoin/hash clauses, and
consider a clause unusable for the current pair of outer and inner
relations if it lacks a commutator.

There are not (and should not be) any such operators built into
Postgres that are mergejoinable or hashable but have no commutators;
so we leverage the alias type 'int8alias1' created in equivclass.sql
to build the test case.  This is why the test case is included in
equivclass.sql rather than in join.sql.

Although this is arguably a bug fix, it cannot be reproduced without
installing an incomplete opclass, which is unlikely to happen in
practice, so no back-.

Reported-by: Alexander Pyhalov
Author: Richard Guo
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/c59ec04a2fef94d9ffc35a9b17dfc081@postgrespro.ru

Fix inconsistent LWLock tranche name "CommitTsSLRU"

This term was using an inconsistent casing between the code and the
documentation, using "CommitTsSLRU" in wait_event_names.txt and
"CommitTSSLRU" in the code.

Let's update the term in the code to reflect what's in the
documentation, "CommitTs" being more commonly used, so as
pg_stat_activity shows the same term as the documentation.

Oversight in 53c2a97a9266.

Author: Alexander Lakhin
Discussion: https://postgr.es/m/f7e514cf-2446-21f1-a5d2-8c089a6e2168@gmail.com
Back-through: 17

Avoid installcheck failure in TAP tests using injection_points

These tests depend on the test module injection_points to be installed,
but it may not be available as the contents of src/test/modules/ are not
installed by default.

This commit adds a workaround based on a scan of pg_available_extensions
to check if the extension is available, skipping the test if it is not.
This allows installcheck to work transparently.

There are more tests impacted by this problem on HEAD, but for now this
addresses only the tests that exist on HEAD and v17 as the release is
close by.

Reported-by: Maxim Orlov
Discussion: https://postgr.es/m/CACG=ezZkoT-pFz6a9XnyToiuR-Wg8fGELqHLoyBodr+2h-77qA@mail.gmail.com
Back-through: 17

Remember last collation to speed up collation cache.

This optimization is to avoid a performance regression in an upcoming
 that will remove lc_collate_is_c().

Discussion: https://postgr.es/m/96a559be83329bc66074a3925ebcfa8ceb16dfc5[email protected]
Discussion: https://postgr.es/m/646f662e145ab38cff1c04d475f4448f53fc5042[email protected]
Discussion: https://postgr.es/m/54565933-d82f-4d7c-8f47-288b1b570fd8@eisentraut.org

Simplify makefiles exporting twice enable_injection_points

This is confusing, as it exports twice the same variable.  Oversight in
6782709df81f that has spread in more places afterwards.

Reported-by: Alvaro Herrera, Tom Lane
Discussion: https://postgr.es/m/202408201630[email protected]
Back-through: 17

Standardize "read-ahead advice" terminology.

Commit 6654bb920 added macOS's equivalent of POSIX_FADV_WILLNEED, and
changed some explicit references to posix_fadvise to use this more
general name for the concept.  Update some remaining references.

Reviewed-by: Peter Eisentraut <[email protected]>
Discussion: https://postgr.es/m/0827edec-1317-4917-a186-035eb1e3241d%40eisentraut.org

Fix stack variable scope from previous commit.

The defect came from me, not from that commit's credited author.  Per
buildfarm members olingo and grassquit.

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

Optimize pg_visibility with read streams.

We've measured 5% performance improvement, and this arranges to benefit
automatically from future optimizations to the read_stream subsystem.

Nazir Bilal Yavuz

Discussion: https://postgr.es/m/CAN55FZ1_Ru3XpMgTwsU67FTH2fs_FrRROmb7x6zs+F44QBEiww@mail.gmail.com

Add block_range_read_stream_cb(), to deduplicate code.

This replaces two functions for iterating over all blocks in a range.  A
pending  will use this instead of adding a third.

Nazir Bilal Yavuz

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

Use library functions to edit config in SSL tests

The SSL tests were editing the postgres configuration by directly
reading and writing the files rather than using append_conf() from
the testcode library.

Reviewed-by: Peter Eisentraut <[email protected]>
Discussion: https://postgr.es/m/01F4684C-8C98-4BBE-AB83-AC8D7C746AF8@yesql.se

Test for PG_TEST_EXTRA separately in SSL tests

PG_TEST_EXTRA is an override and should be tested for separately
from any other test as there is no dependency on whether OpenSSL
is available or not.

Reviewed-by: Peter Eisentraut <[email protected]>
Discussion: https://postgr.es/m/01F4684C-8C98-4BBE-AB83-AC8D7C746AF8@yesql.se

Fix typos in code comments and test data

The typos in 005_negotiate_encryption.pl and pg_combinebackup.c
shall be backported to v17 where they were introduced.

Back-through: v17
Discussion: https://postgr.es/m/[email protected]

Add const qualifiers to XLogRegister*() functions

Add const qualifiers to XLogRegisterData() and XLogRegisterBufData().
Several unconstify() calls can be removed.

Reviewed-by: Aleksander Alekseev <[email protected]>
Discussion: https://www.postgresql.org/message-id/dd889784-9ce7-436a-b4f1-52e4a5e577bd@eisentraut.org

Fix typos and grammar in code comments and docs

Author: Alexander Lakhin
Discussion: https://postgr.es/m/f7e514cf-2446-21f1-a5d2-8c089a6e2168@gmail.com

Define PG_TBLSPC_DIR for path pg_tblspc/ in data folder

Similarly to 2065ddf5e34c, this introduces a define for "pg_tblspc".
This makes the style more consistent with the existing PG_STAT_TMP_DIR,
for example.

There is a difference with the other cases with the introduction of
PG_TBLSPC_DIR_SLASH, required in two places for recovery and backups.

Author: Bertrand Drouvot
Reviewed-by: Ashutosh Bapat, Álvaro Herrera, Yugo Nagata, Michael
Paquier
Discussion: https://postgr.es/m/[email protected]

doc: Consistently use result set in documentation

We use "result set" in all other places so let's be consistent
across the entire documentation.

Reported-by: [email protected]
Discussion: https://postgr.es/m/172187924855.915373.15595156724215203822@wrigleys.postgresql.org

Fix rarely-run test for message wording change

fixup for 2e6a8047f0

Reported-by: Nazir Bilal Yavuz <[email protected]>

Only perform pg_strong_random init when required

The random number generator in OpenSSL 1.1.1 was redesigned to provide
fork safety by default, thus removing the need for calling RAND_poll
after forking to ensure that two processes cannot share the same state.
Since we now support 1.1.0 as the minumum version, and 1.1.0 is being
increasingly phased out from production use, only perform the RAND_poll
initialization for installations running 1.1.0 by checking the OpenSSL
version number.

LibreSSL changed random number generator when forking OpenSSL and has
provided fork safety since version 2.0.2.

This removes the overhead of initializing the RNG for strong random
for the vast majority of users for whom it is no longer required.

Reviewed-by: Jacob Champion <[email protected]>
Reviewed-by: Peter Eisentraut <[email protected]>
Reviewed-by: Michael Paquier <[email protected]>
Discussion: https://postgr.es/m/CA+hUKGKh7QrYzu=8yWEUJvXtMVm_CNWH1L_TLWCbZMwbi1XP2Q@mail.gmail.com

Remove support for OpenSSL older than 1.1.0

OpenSSL 1.0.2 has been EOL from the upstream OpenSSL project for
some time, and is no longer the default OpenSSL version with any
vendor which package PostgreSQL. By retiring support for OpenSSL
1.0.2 we can remove a lot of no longer required complexity for
managing state within libcrypto which is now handled by OpenSSL.

Reviewed-by: Jacob Champion <[email protected]>
Reviewed-by: Peter Eisentraut <[email protected]>
Reviewed-by: Michael Paquier <[email protected]>
Discussion: https://postgr.es/m/[email protected]
Discussion: https://postgr.es/m/CA+hUKGKh7QrYzu=8yWEUJvXtMVm_CNWH1L_TLWCbZMwbi1XP2Q@mail.gmail.com

Cache typarray for fast lookups in binary upgrade mode

When upgrading a large schema it adds significant overhead to perform
individual catalog lookups per relation in order to retrieve Oid for
preserving Oid calls. This instead adds the typarray to the TypeInfo
cache which then allows for fast lookups using the existing API. A
35% reduction of pg_dump runtime in binary upgrade mode was observed
with this change.

Reviewed-by: Nathan Bossart <[email protected]>
Discussion: https://postgr.es/m/8F1F1E1D-D17B-4B33-B014-EDBCD15F3F0B@yesql.se

More use of getpwuid_r() directly

Remove src/port/user.c, call getpwuid_r() directly.  This reduces some
complexity and allows better control of the error behavior.  For
example, the old code would in some circumstances silently truncate
the result string, or produce error message strings that the caller
wouldn't use.

src/port/user.c used to be called src/port/thread.c and contained
various portability complications to support thread-safety.  These are
all obsolete, and all but the user-lookup functions have already been
removed.  This  completes this by also removing the user-lookup
functions.

Also convert src/backend/libpq/auth.c to use getpwuid_r() for
thread-safety.

Originally, I tried to be overly correct by using
sysconf(_SC_GETPW_R_SIZE_MAX) to get the buffer size for getpwuid_r(),
but that doesn't work on FreeBSD.  All the OS where I could find the
source code internally use 1024 as the suggested buffer size, so I
just ended up hardcoding that.  The previous code used BUFSIZ, which
is an unrelated constant from stdio.h, so its use seemed
inappropriate.

Reviewed-by: Heikki Linnakangas <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/5f293da9-ceb4-4937-8e52-82c25db8e4d3%40eisentraut.org

Rename enum labels of PG_Locale_Strategy

PG_REGEX_BUILTIN was added in f69319f2f1fb but it did not follow the
same pattern as the previous labels, i.e. PG_LOCALE_*.  In addition to
this, the two libc strategies did not include in the name that they were
related to this library.

The enum labels are renamed as PG_STRATEGY_type[_subtype] to make the
code clearer, in accordance to the library and the functions they rely
on.

Author: Andreas Karlsson
Discussion: https://postgr.es/m/6f81200f-68fd-411e-97a1-d1f291d2e222@proxel.se

Fix unfairness in all-cached parallel seq scan.

Commit b5a9b18c introduced block  infrastructure with a special
fast path for all-cached scans, and commit b7b0f3f2 connected the
infrastructure up to sequential scans.  One of the fast path
micro-optimizations had an unintended consequence: it interfered with
parallel sequential scan's block range allocator (from commit 56788d21),
which has its own ramp-up and ramp-down algorithm when handing out
groups of pages to workers.  A scan of an all-cached table could give
extra blocks to one worker, when others had finished.  In some plans
(probably already very bad plans, such as the one reported by
Alexander), the unfairness could be magnified.

An internal buffer of 16 block numbers is removed, keeping just a single
block buffer for technical reasons.

Back- to 17.

Reported-by: Alexander Lakhin <[email protected]>
Discussion: https://postgr.es/m/63a63690-dd92-c809-0b47-af05459e95d1%40gmail.com

Stabilize 039_end_of_wal test.

The first test was sensitive to the insert LSN after setting up the
catalogs, which depended on environmental things like the locales on the
OS and usernames.  Switch to a new WAL file before the first test, as a
simple way to put every computer into the same state.

Back- to all supported releases.

Reported-by: Anton Voloshin <[email protected]>
Reported-by: Nathan Bossart <[email protected]>
Reviewed-by: Tom Lane <[email protected]>
Reviewed-by: Nathan Bossart <[email protected]>
Discussion: https://postgr.es/m/b26aeac2-cb6d-4633-a7ea-945baae83dcf%40postgrespro.ru

Clarify restrict_nonsystem_relation_kind description.

This change improves the description of the
restrict_nonsystem_relation_kind parameter in guc_table.c and the
documentation for better clarity.

Back to 12, where this GUC parameter was introduced.

Reviewed-by: Peter Eisentraut
Discussion: https://postgr.es/m/6a96f1af-22b4-4a80-8161-1f26606b9ee2%40eisentraut.org
Back-through: 12

Make postgres_fdw's query_cancel test less flaky.

This test occasionally shows

+WARNING:  could not get result of cancel request due to timeout

which appears to be because the cancel request is sometimes unluckily
sent to the remote session between queries, and then it's ignored.

This  tries to make that less probable in three ways:

1. Use a test query that does not involve remote estimates, so that
no EXPLAINs are sent.
2. Make sure that the remote session is ready-to-go (transaction
started, SET commands sent) before we start the timer.
3. Increase the statement_timeout to 100ms, to give the local
session enough time to plan and issue the query.

We might have to go higher than 100ms to make this adequately
stable in the buildfarm, but let's see how it goes.

Back- to v17 where this test was introduced.

Jelte Fennema-Nio and Tom Lane

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

Avoid inserting PlaceHolderVars in cases where pre-v16 PG did not.

Commit 2489d76c4 removed some logic from pullup_replace_vars()
that avoided wrapping a PlaceHolderVar around a pulled-up
subquery output expression if the expression could be proven
to go to NULL anyway (because it contained Vars or PHVs of the
pulled-up relation and did not contain non-strict constructs).
But removing that logic turns out to cause performance regressions
in some cases, because the extra PHV blocks subexpression folding,
and will do so even if outer-join reduction later turns it into a
no-op with no phnullingrels bits.  This can for example prevent
an expression from being matched to an index.

The reason for always adding a PHV was to ensure we had someplace
to put the varnullingrels marker bits of the Var being replaced.
However, it turns out we can optimize in exactly the same cases that
the previous code did, because we can instead attach the needed
varnullingrels bits to the contained Var(s)/PHV(s).

This is not a complete solution --- it would be even better if we
could remove PHVs after reducing them to no-ops.  It doesn't look
practical to back- such an improvement, but this change seems
safe and at least gets rid of the performance-regression cases.

Per complaint from Nikhil Raj.  Back- to v16 where the
problem appeared.

Discussion: https://postgr.es/m/CAG1ps1xvnTZceKK24OUfMKLPvDP2vjT-d+F2AOCWbw_v3KeEgg@mail.gmail.com

Remove one memoize test case added by commit 069d0ff02.

This test case turns out to depend on the assumption that a non-Var
subquery output that's underneath an outer join will always get
wrapped in a PlaceHolderVar.  But that behavior causes performance
regressions in some cases compared to what happened before v16.
The next commit will avoid inserting a PHV in the same cases where
pre-v16 did, and that causes get_memoized_path to not detect that
a memoize plan could be used.

Commit this separately, in hopes that we can restore the test after
making get_memoized_path smarter.  (It's failing to find memoize
plans in adjacent cases where no PHV was ever inserted, so there
is definitely room for improvement there.)

Discussion: https://postgr.es/m/CAG1ps1xvnTZceKK24OUfMKLPvDP2vjT-d+F2AOCWbw_v3KeEgg@mail.gmail.com

Define PG_LOGICAL_DIR for path pg_logical/ in data folder

This is similar to 2065ddf5e34c, but this time for pg_logical/ itself
and its contents, like the paths for snapshots, mappings or origin
checkpoints.

Author: Bertrand Drouvot
Reviewed-by: Ashutosh Bapat, Yugo Nagata, Michael Paquier
Discussion: https://postgr.es/m/[email protected]

Define PG_REPLSLOT_DIR for path pg_replslot/ in data folder

This commit replaces most of the hardcoded values of "pg_replslot" by a
new PG_REPLSLOT_DIR #define.  This makes the style more consistent with
the existing PG_STAT_TMP_DIR, for example.  More places will follow a
similar change.

Author: Bertrand Drouvot
Reviewed-by: Ashutosh Bapat, Yugo Nagata, Michael Paquier
Discussion: https://postgr.es/m/[email protected]

Rename pg_sequence_read_tuple() to pg_get_sequence_data()

This commit removes log_cnt from the tuple returned by the SQL function.
This field is an internal counter that tracks when a WAL record should
be generated for a sequence, and it is reset each time the sequence is
restored or recovered.  It is not necessary to rebuild the sequence DDL
commands for pg_dump and pg_upgrade where this function is used.  The
field can still be queried with a scan of the "table" created
under-the-hood for a sequence.

Issue noticed while hacking on a feature that can rely on this new
function rather than pg_sequence_last_value(), aimed at making sequence
computation more easily pluggable.

Bump catalog version.

Reviewed-by: Nathan Bossart
Discussion: https://postgr.es/m/[email protected]

Fix mis-deparsing of ORDER BY lists when there is a name conflict.

If an ORDER BY item in SELECT is a bare identifier, the parser
first seeks it as an output column name of the SELECT (for SQL92
compatibility).  However, ruleutils.c is expecting the SQL99
interpretation where such a name is an input column name.  So it's
possible to produce an incorrect display of a view in the (admittedly
pretty ill-advised) case where some other column is renamed in the
SELECT output list to match an ORDER BY column.

This can be fixed by table-qualifying such names in the dumped
view text.  To avoid cluttering less-ill-advised queries, we'd
like to do so only when there's an actual name conflict.
That requires passing the current get_query_def call's resultDesc
parameter down to get_variable, so that it can determine what
the output column names are.  In hopes of reducing rather than
increasing notational clutter in ruleutils.c, I moved that value
into the deparse_context struct and removed it from the parameter
lists of get_query_def's other subroutines.

I made a few other cosmetic changes while at it:
* Likewise move the colNamesVisible parameter into deparse_context.
* Rename deparse_context's windowTList field to targetList,
since it's no longer used only in connection with WINDOW clauses.
* Replace the special_exprkind field with a bool inGroupBy,
since that was all it was being used for, and the apparent
flexibility of storing a ParseExprKind proved to be illusory.
(We need a separate varInOrderBy field to make this  work.)
* Remove useless save/restore logic in get_select_query_def.

In principle, this bug is quite old.  However, it seems unreachable
before 1b4d280ea, because before that the presence of "new" and "old"
entries in a view's rangetable caused us to always table-qualify every
Var reference in dumped views.  Hence, back- to v16 where that
came in.

Per bug #18589 from Quynh Tran.

Discussion: https://postgr.es/m/18589-70091cb81db1a3f1@postgresql.org

Message style improvements

Put generated_stored test objects in a schema

This avoids naming conflicts with concurrent tests with similarly
named objects.  Currently, there are none, but a tests for virtual
generated columns are planned to be added.

Reviewed-by: Corey Huinker <[email protected]>
Reviewed-by: Tomasz Rybak <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/a368248e-69e4-40be-9c07-6c3b5880b0a6@eisentraut.org

Rename regress test generated to generated_stored

This makes naming room to have another test file for virtual generated
columns.

Reviewed-by: Corey Huinker <[email protected]>
Reviewed-by: Tomasz Rybak <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/a368248e-69e4-40be-9c07-6c3b5880b0a6@eisentraut.org

Disallow USING clause when altering type of generated column

This does not make sense.  It would write the output of the USING
clause into the converted column, which would violate the generation
expression.  This adds a check to error out if this is specified.

There was a test for this, but that test errored out for a different
reason, so it was not effective.

Reported-by: Jian He <[email protected]>
Reviewed-by: Yugo NAGATA <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/c7083982-69f4-4b14-8315-f9ddb20b9834%40eisentraut.org

Rename some shared memory initialization routines

To make them follow the usual naming convention where
FoobarShmemSize() calculates the amount of shared memory needed by
Foobar subsystem, and FoobarShmemInit() performs the initialization.

I didn't rename CreateLWLocks() and InitShmmeIndex(), because they are
a little special. They need to be called before any of the other
ShmemInit() functions, because they set up the shared memory
bookkeeping itself. I also didn't rename InitProcGlobal(), because
unlike other Shmeminit functions, it's not called by individual
backends.

Reviewed-by: Andreas Karlsson
Discussion: https://www.postgresql.org/message-id/c09694ff-2453-47e5-b26c-32a16cd75ce6@iki.fi

Refactor lock manager initialization to make it a bit less special

Split the shared and local initialization to separate functions, and
follow the common naming conventions. With this, we no longer create
the LockMethodLocalHash hash table in the postmaster process, which
was always pointless.

Reviewed-by: Andreas Karlsson
Discussion: https://www.postgresql.org/message-id/c09694ff-2453-47e5-b26c-32a16cd75ce6@iki.fi

Refactor some code for ALTER TABLE SET LOGGED/UNLOGGED in tablecmds.c

Both sub-commands use the same routine to switch the relpersistence of a
relation, duplicated the same checks, and used a style inconsistent with
access methods and tablespaces.

SET LOGEED/UNLOGGED is refactored to avoid any duplication, setting the
reason why a relation rewrite happens within ATPrepChangePersistence().
This shaves some code.

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

Fixup for prefetching support on macOS

The new code path (commit 6654bb92047) should call FileAccess() first,
like the posix_fadvise() path.

Reported-by: Thomas Munro <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/0827edec-1317-4917-a186-035eb1e3241d%40eisentraut.org

Rename the conflict types for the origin differ cases.

The conflict types 'update_differ' and 'delete_differ' indicate that a row
to be modified was previously altered by another origin. Rename those to
'update_origin_differs' and 'delete_origin_differs' to clarify their
meaning.

Author: Hou Zhijie
Reviewed-by: Shveta Malik, Peter Smith
Discussion: https://postgr.es/m/CAA4eK1+HEKwG_UYt4Zvwh5o_HoCKCjEGesRjJX38xAH3OxuuYA@mail.gmail.com

Doc: Fix the ambiguity in the description of failover slots.

The failover slots ensure a seamless transition of a subscriber after the
standby is promoted. But the docs for it also explain the behavior of
asynchronous replication which can confuse the readers.

Reported-by: Masahiro Ikeda
Back-through: 17
Discussion: https://postgr.es/m/OS3PR01MB6390B660F4198BB9745E0526B18B2@OS3PR01MB6390.jpnprd01.prod.outlook.com

Add prefetching support on macOS

macOS doesn't have posix_fadvise(), but fcntl() with the F_RDADVISE
command does the same thing.

Some related documentation has been generalized to not mention
posix_advise() specifically anymore.

Reviewed-by: Thomas Munro <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/0827edec-1317-4917-a186-035eb1e3241d%40eisentraut.org

Message style improvements

Fix misplaced translator comments

They did not immediately precede the code they were applying to.

Fix identation.

Fix memory counter update in ReorderBuffer.

Commit 5bec1d6bc5e changed the memory usage updates of the
ReorderBufferTXN to zero all at once by subtracting txn->size, rather
than updating it for each change. However, if TOAST reconstruction
data remained in the transaction when freeing it, there were cases
where it further subtracted the memory counter from zero, resulting in
an assertion failure.

This change calculates the memory size for each change and updates the
memory usage to precisely the amount that has been freed.

Back to v17, where this was introducd.

Reviewed-by: Amit Kapila, Shlok Kyal
Discussion: https://postgr.es/m/CAD21AoAqkNUvicgKPT_dXzNoOwpPkVTg0QPPxEcWmzT0moCJ1g%40mail.gmail.com
Back-through: 17

Fix nbtree lookahead overflow bug.

Add bounds checking to nbtree's lookahead/skip-within-a-page mechanism.
Otherwise it's possible for cases with lots of before-array-keys tuples
to overflow an int16 variable, causing the mechanism to generate an out
of bounds page offset number.

Oversight in commit 5bf748b8, which enhanced nbtree ScalarArrayOp
execution.

Reported-By: Alexander Lakhin <[email protected]>
Discussion: https://postgr.es/m/6c68ac42-bbb5-8b24-103e-af0e279c536f@gmail.com
Back: 17-, where nbtree SAOP execution was enhanced.

pg_upgrade: Message style improvements

Fix compiler warning in mul_var_short().

Some compilers (e.g., gcc before version 7) mistakenly think "carry"
might be used uninitialized.

Reported by Tom Lane, per various buildfarm members, e.g. arowana.

Revert: Avoid looping over all type cache entries in TypeCacheRelCallback()

This commit reverts c14d4acb8 as the  design didn't take into account
that TypeCacheEntry could be invalidated during the lookup_type_cache() call.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/1927cba4-177e-5c23-cbcc-d444a850304f%40gmail.com