postgresql.git
8 weeks agoImprove snapmgr.c comment
Heikki Linnakangas [Tue, 11 Mar 2025 21:28:38 +0000 (23:28 +0200)]
Improve snapmgr.c comment

Add more details on the different kinds of snapshots, how to use them,
and how the active snapshot stack works.

Discussion: https://www.postgresql.org/message-id/7c56f180-b9e1-481e-8c1d-efa63de3ecbb@iki.fi

8 weeks agoAssert that a snapshot is active or registered before it's used
Heikki Linnakangas [Tue, 11 Mar 2025 21:20:34 +0000 (23:20 +0200)]
Assert that a snapshot is active or registered before it's used

The comment in GetTransactionSnapshot() said that you "should call
RegisterSnapshot or PushActiveSnapshot on the returned snap if it is
to be used very long". That felt too unclear to me. Make the comment
more strongly worded.

To enforce that rule and to catch potential bugs where a snapshot
might get invalidated while it's still in use, add an assertion to
HeapTupleSatisfiesMVCC() to check that the snapshot is registered or
pushed to active stack. No new bugs were found by this, but it seems
like good future-proofing. It's not a great place for the check;
HeapTupleSatisfiesMVCC() is in fact safe to call with an unregistered
snapshot, and the assertion won't catch other unsafe uses. But it goes
a long way in practice.

Fix a few cases that were playing fast and loose with that and just
assumed that the snapshot cannot be invalidated during a scan. Those
assumptions were not wrong, but they're not performance critical, so
let's drop the excuses and just register the snapshot. These were
false positives found by the new assertion.

Discussion: https://www.postgresql.org/message-id/7c56f180-b9e1-481e-8c1d-efa63de3ecbb@iki.fi

8 weeks agopg_logicalinspect: Fix possible crash when passing a directory path.
Masahiko Sawada [Tue, 11 Mar 2025 16:56:40 +0000 (09:56 -0700)]
pg_logicalinspect: Fix possible crash when passing a directory path.

Previously, pg_logicalinspect functions were too trusting of their
input and blindly passed it to SnapBuildRestoreSnapshot(). If the
input pointed to a directory, the server could a PANIC error while
attempting to fsync_fname() with isdir=false on a directory.

This commit adds validation checks for input filenames and passes the
LSN extracted from the filename to SnapBuildRestoreSnapshot() instead
of the filename itself. It also adds regression tests for various
input patterns and permission checks.

Bug: #18828
Reported-by: Robins Tharakan <tharakan@gmail.com>
Co-authored-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Co-authored-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/18828-0f4701c635064211@postgresql.org

8 weeks agopg_logicalinspect: Stabilize isolation tests.
Masahiko Sawada [Tue, 11 Mar 2025 16:30:00 +0000 (09:30 -0700)]
pg_logicalinspect: Stabilize isolation tests.

The previous isolation tests did not account for the possibility that
the background writer or the checkpointer could write a RUNNING_XACTS
record, which could cause logical decoding to produce more logical
snapshots than expected.

This commit modifies the isolation tests to verify that at least one
logical snapshot contains the expected number of committed or ongoing
catalog-change transactions.

Per buildfarm member skink.

Reported-by: Andres Freund <andres@anarazel.de>
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/5qbxud4pvnvmtuoi7weiizm5hmumxaeohx4vztfhrwlfhyz6rj@buh4435mllwo

8 weeks agoImprove EXPLAIN's display of window functions.
Tom Lane [Tue, 11 Mar 2025 15:19:54 +0000 (11:19 -0400)]
Improve EXPLAIN's display of window functions.

Up to now we just punted on showing the window definitions used
in a plan, with window function calls represented as "OVER (?)".
To improve that, show the window definition implemented by each
WindowAgg plan node, and reference their window names in OVER.
For nameless window clauses generated by "OVER (...)", assign
unique names w1, w2, etc.

In passing, re-order the properties shown for a WindowAgg node
so that the Run Condition (if any) appears after the Window
property and before the Filter (if any).  This seems more
sensible since the Run Condition is associated with the Window
and acts before the Filter.

Thanks to David G. Johnston and Álvaro Herrera for design
suggestions.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/144530.1741469955@sss.pgh.pa.us

8 weeks agonbtree: Make BTMaxItemSize into object-like macro.
Peter Geoghegan [Tue, 11 Mar 2025 14:35:56 +0000 (10:35 -0400)]
nbtree: Make BTMaxItemSize into object-like macro.

Make nbtree's "1/3 of a page limit" BTMaxItemSize function-like macro
(which accepts a "page" argument) into an object-like macro that can be
used from code that doesn't have convenient access to an nbtree page.

Preparation for an upcoming patch that adds skip scan to nbtree.
Parallel index scans that use skip scan will serialize datums (not just
SAOP array subscripts) when scheduling primitive scans.  BTMaxItemSize
will be used by btestimateparallelscan to determine how much DSM to
request.

Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-Wz=H_RG5weNGeUG_TkK87tRBnH9mGCQj6WpM4V4FNWKv2g@mail.gmail.com

8 weeks agoShow index search count in EXPLAIN ANALYZE, take 2.
Peter Geoghegan [Tue, 11 Mar 2025 13:20:50 +0000 (09:20 -0400)]
Show index search count in EXPLAIN ANALYZE, take 2.

Expose the count of index searches/index descents in EXPLAIN ANALYZE's
output for index scan/index-only scan/bitmap index scan nodes.  This
information is particularly useful with scans that use ScalarArrayOp
quals, where the number of index searches can be unpredictable due to
implementation details that interact with physical index characteristics
(at least with nbtree SAOP scans, since Postgres 17 commit 5bf748b8).
The information shown also provides useful context when EXPLAIN ANALYZE
runs a plan with an index scan node that successfully applied the skip
scan optimization (set to be added to nbtree by an upcoming patch).

The instrumentation works by teaching all index AMs to increment a new
nsearches counter whenever a new index search begins.  The counter is
incremented at exactly the same point that index AMs already increment
the pg_stat_*_indexes.idx_scan counter (we're counting the same event,
but at the scan level rather than the relation level).  Parallel queries
have workers copy their local counter struct into shared memory when an
index scan node ends -- even when it isn't a parallel aware scan node.
An earlier version of this patch that only worked with parallel aware
scans became commit 5ead85fb (though that was quickly reverted by commit
d00107cd following "debug_parallel_query=regress" buildfarm failures).

Our approach doesn't match the approach used when tracking other index
scan related costs (e.g., "Rows Removed by Filter:").  It is comparable
to the approach used in similar cases involving costs that are only
readily accessible inside an access method, not from the executor proper
(e.g., "Heap Blocks:" output for a Bitmap Heap Scan, which was recently
enhanced to show per-worker costs by commit 5a1e6df3, using essentially
the same scheme as the one used here).  It is necessary for index AMs to
have direct responsibility for maintaining the new counter, since the
counter might need to be incremented multiple times per amgettuple call
(or per amgetbitmap call).  But it is also necessary for the executor
proper to manage the shared memory now used to transfer each worker's
counter struct to the leader.

Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Robert Haas <robertmhaas@gmail.com>
Reviewed-By: Tomas Vondra <tomas@vondra.me>
Reviewed-By: Masahiro Ikeda <ikedamsh@oss.nttdata.com>
Reviewed-By: Matthias van de Meent <boekewurm+postgres@gmail.com>
Discussion: https://postgr.es/m/CAH2-WzkRqvaqR2CTNqTZP0z6FuL4-3ED6eQB0yx38XBNj1v-4Q@mail.gmail.com
Discussion: https://postgr.es/m/CAH2-Wz=PKR6rB7qbx+Vnd7eqeB5VTcrW=iJvAsTsKbdG+kW_UA@mail.gmail.com

8 weeks agoUpdate nls.mk for newly added file
Peter Eisentraut [Tue, 11 Mar 2025 12:42:03 +0000 (13:42 +0100)]
Update nls.mk for newly added file

Commit f18231e8175 moved some code to a new file, but the new file
wasn't added to nls.mk.

8 weeks agoBRIN: be more strict about required support procs
Álvaro Herrera [Tue, 11 Mar 2025 11:50:35 +0000 (12:50 +0100)]
BRIN: be more strict about required support procs

With improperly defined operator classes, it's possible to get a
Postgres crash because we'd try to invoke a procedure that doesn't
exist.  This is because the code is being a bit too trusting that the
opclass is correctly defined.  Add some ereport(ERROR)s for cases where
mandatory support procedures are not defined, transforming the crashes
into errors.

The particular case that was reported is an incomplete opclass in
PostGIS.

Backpatch all the way down to 13.

Reported-by: Tobias Wendorff <tobias.wendorff@tu-dortmund.de>
Diagnosed-by: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: Tomas Vondra <tomas@vondra.me>
Discussion: https://postgr.es/m/fb6d9a35-6c8e-4869-af80-0a4944a793a4@tu-dortmund.de

8 weeks agoAdd special case fast-paths for strict functions
Daniel Gustafsson [Tue, 11 Mar 2025 11:02:42 +0000 (12:02 +0100)]
Add special case fast-paths for strict functions

Many STRICT function calls will have one or two arguments, in which
case we can speed up checking for NULL input by avoiding setting up
a loop over the arguments. This adds EEOP_FUNCEXPR_STRICT_1 and the
corresponding EEOP_FUNCEXPR_STRICT_2 for functions with one and two
arguments respectively.

Author: Andres Freund <andres@anarazel.de>
Co-authored-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Andreas Karlsson <andreas@proxel.se>
Discussion: https://postgr.es/m/415721CE-7D2E-4B74-B5D9-1950083BA03E@yesql.se
Discussion: https://postgr.es/m/20191023163849.sosqbfs5yenocez3@alap3.anarazel.de

8 weeks agoReplace EEOP_DONE with special steps for return/no return
Daniel Gustafsson [Tue, 11 Mar 2025 11:02:38 +0000 (12:02 +0100)]
Replace EEOP_DONE with special steps for return/no return

Knowing when the side-effects of an expression is the intended result
of the execution, rather than the returnvalue, is important for being
able generate more efficient JITed code. This replaces EEOP_DONE with
two new steps: EEOP_DONE_RETURN and EEOP_DONE_NO_RETURN.  Expressions
which return a value should use the former step; expressions used for
their side-effects which don't return value should use the latter.

Author: Andres Freund <andres@anarazel.de>
Co-authored-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Andreas Karlsson <andreas@proxel.se>
Discussion: https://postgr.es/m/415721CE-7D2E-4B74-B5D9-1950083BA03E@yesql.se
Discussion: https://postgr.es/m/20191023163849.sosqbfs5yenocez3@alap3.anarazel.de

8 weeks agoMove RemoveInheritedConstraint() call slightly earlier
Peter Eisentraut [Tue, 11 Mar 2025 09:43:48 +0000 (10:43 +0100)]
Move RemoveInheritedConstraint() call slightly earlier

This change is harmless and does not affect the existing intended
operation.  It is necessary for a subsequent patch operation (NOT
ENFORCED foreign keys), where we may need to change the child
constraint to enforced.  In this case, we would create the necessary
triggers and queue the constraint for validation, so it is important
to remove any unnecessary constraints before proceeding.

This is a small change that could have been included in the previous
"split tryAttachPartitionForeignKey" refactoring patch (commit
1d26c2d2c4b), but was kept separate to highlight the changes.

Author: Amul Sul <amul.sul@enterprisedb.com>
Reviewed-by: Alexandra Wang <alexandra.wang.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAAJ_b962c5AcYW9KUt_R_ER5qs3fUGbe4az-SP-vuwPS-w-AGA%40mail.gmail.com

8 weeks agorefactor: Split tryAttachPartitionForeignKey()
Peter Eisentraut [Tue, 11 Mar 2025 08:33:36 +0000 (09:33 +0100)]
refactor: Split tryAttachPartitionForeignKey()

Split tryAttachPartitionForeignKey() into three functions:
AttachPartitionForeignKey(), RemoveInheritedConstraint(), and
DropForeignKeyConstraintTriggers(), so they can be reused in some
subsequent patches for the NOT ENFORCED feature.

Author: Amul Sul <amul.sul@enterprisedb.com>
Reviewed-by: Alexandra Wang <alexandra.wang.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAAJ_b962c5AcYW9KUt_R_ER5qs3fUGbe4az-SP-vuwPS-w-AGA%40mail.gmail.com

8 weeks agorefactor: re-add ATExecAlterChildConstr()
Peter Eisentraut [Tue, 11 Mar 2025 07:40:42 +0000 (08:40 +0100)]
refactor: re-add ATExecAlterChildConstr()

ATExecAlterChildConstr() was removed in commit 80d7f990496, but it is
needed in some subsequent patches for the NOT ENFORCED feature, to
recurse over child constraints.  This adds it back in slightly altered
form.

Author: Amul Sul <amul.sul@enterprisedb.com>
Reviewed-by: Alexandra Wang <alexandra.wang.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAAJ_b962c5AcYW9KUt_R_ER5qs3fUGbe4az-SP-vuwPS-w-AGA%40mail.gmail.com

8 weeks agoAdd WAL data to backend statistics
Michael Paquier [Tue, 11 Mar 2025 00:04:11 +0000 (09:04 +0900)]
Add WAL data to backend statistics

This commit adds per-backend WAL statistics, providing the same
information as pg_stat_wal, except that it is now possible to know how
much WAL activity is happening in each backend rather than an overall
aggregate of all the activity.  Like pg_stat_wal, the implementation
relies on pgWalUsage, tracking the difference of activity between two
reports to pgstats.

This data can be retrieved with a new system function called
pg_stat_get_backend_wal(), that returns one tuple based on the PID
provided in input.  Like pg_stat_get_backend_io(), this is useful when
joined with pg_stat_activity to get a live picture of the WAL generated
for each running backend, showing how the activity is [un]balanced.

pgstat_flush_backend() gains a new flag value, able to control the flush
of the WAL stats.

This commit relies mostly on the infrastructure provided by
9aea73fc61d4, that has introduced backend statistics.

Bump catalog version.  A bump of PGSTAT_FILE_FORMAT_ID is not required,
as backend stats do not persist on disk.

Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>
Discussion: https://postgr.es/m/Z3zqc4o09dM/Ezyz@ip-10-97-1-34.eu-west-3.compute.internal

8 weeks agotests: Make postmaster/002_connection_limits deal verbose logs
Andres Freund [Mon, 10 Mar 2025 23:11:32 +0000 (19:11 -0400)]
tests: Make postmaster/002_connection_limits deal verbose logs

When log_error_verbosity=verbose is configured the test would hand (and then
fail), because of the sqlstate being added between log level and message. Make
regex cope.

Reported-by: Andrew Dunstan <andrew@dunslane.net>
Discussion: https://postgr.es/m/c7ba6bd0-3701-43d1-9087-017777fe9cd2%40dunslane.net

8 weeks agoCREATE INDEX: do update index stats if autovacuum=off.
Tom Lane [Mon, 10 Mar 2025 21:49:27 +0000 (17:49 -0400)]
CREATE INDEX: do update index stats if autovacuum=off.

This fixes a thinko from commit d611f8b15.  The intent was to prevent
updating the stats of the pre-existing heap if autovacuum is off,
but it also disabled updating the stats of the just-created index.
There is AFAICS no good reason to do the latter, since there could not
be any pre-existing stats to refrain from overwriting, and the zeroed
stats that are there to begin with are very unlikely to be useful.
Moreover, the change broke our cross-version upgrade tests again.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/1116282.1741374848@sss.pgh.pa.us

8 weeks agoFix a few more redundant calls of GetLatestSnapshot()
Heikki Linnakangas [Mon, 10 Mar 2025 16:54:58 +0000 (18:54 +0200)]
Fix a few more redundant calls of GetLatestSnapshot()

Commit 2367503177 fixed this in RelationFindReplTupleByIndex(), but I
missed two other similar cases.

Per report from Ranier Vilela.

Discussion: https://www.postgresql.org/message-id/CAEudQArUT1dE45WN87F-Gb7XMy_hW6x1DFd3sqdhhxP-RMDa0Q@mail.gmail.com
Backpatch-through: 13

8 weeks agoFix snapshot used in logical replication index lookup
Heikki Linnakangas [Mon, 10 Mar 2025 15:07:38 +0000 (17:07 +0200)]
Fix snapshot used in logical replication index lookup

The function calls GetLatestSnapshot() to acquire a fresh snapshot,
makes it active, and was meant to pass it to table_tuple_lock(), but
instead called GetLatestSnapshot() again to acquire yet another
snapshot. It was harmless because the heap AM and all other known
table AMs ignore the 'snapshot' argument anyway, but let's be tidy.

In the long run, this perhaps should be redesigned so that snapshot
was not needed in the first place. The table AM API uses TID +
snapshot as the unique identifier for the row version, which is
questionable when the row came from an index scan with a Dirty
snapshot. You might lock a different row version when you use a
different snapshot in the table_tuple_lock() call (a fresh MVCC
snapshot) than in the index scan (DirtySnapshot). However, in the heap
AM and other AMs where the TID alone identifies the row version, it
doesn't matter. So for now, just fix the obvious albeit harmless bug.

This has been wrong ever since the table AM API was introduced in
commit 5db6df0c01, so backpatch to all supported versions.

Discussion: https://www.postgresql.org/message-id/83d243d6-ad8d-4307-8b51-2ee5844f6230@iki.fi
Backpatch-through: 13

8 weeks agoDoc: improve description of window function processing.
Tom Lane [Mon, 10 Mar 2025 14:22:08 +0000 (10:22 -0400)]
Doc: improve description of window function processing.

The previous wording talked about a "single pass over the data",
which can be read as promising more than intended (to wit, that only
one WindowAgg plan node will be used).  What we promise is only what
the SQL spec requires, namely that the data not get re-sorted between
window functions with compatible PARTITION BY/ORDER BY clauses.
Adjust the wording in hopes of making this clearer.

Reported-by: Christopher Inokuchi <cinokuchi@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: David G. Johnston <david.g.johnston@gmail.com>
Discussion: https://postgr.es/m/CABde6B5va2wMsnM79u_x=n9KUgfKQje_pbLROEBmA9Ru5XWidw@mail.gmail.com
Backpatch-through: 13

8 weeks agoUse extended stats for precise estimation of bucket size in hash join
Alexander Korotkov [Mon, 10 Mar 2025 11:42:00 +0000 (13:42 +0200)]
Use extended stats for precise estimation of bucket size in hash join

Recognizing the real-life complexity where columns in the table often have
functional dependencies, PostgreSQL's estimation of the number of distinct
values over a set of columns can be underestimated (or much rarely,
overestimated) when dealing with multi-clause JOIN.  In the case of hash
join, it can end up with a small number of predicted hash  buckets and, as
a result, picking non-optimal merge join.

To improve the situation, we introduce one additional stage of bucket size
estimation - having two or more join clauses estimator lookup for extended
statistics and use it for multicolumn estimation.  Clauses are grouped into
lists, each containing expressions referencing the same relation.  The result
of the multicolumn estimation made over such a list is combined with others
according to the caller's logic.  Clauses that are not estimated are returned
to the caller for further estimation.

Discussion: https://postgr.es/m/52257607-57f6-850d-399a-ec33a654457b%40postgrespro.ru
Author: Andrei Lepikhov <lepihov@gmail.com>
Reviewed-by: Andy Fan <zhihui.fan1213@gmail.com>
Reviewed-by: Tomas Vondra <tomas.vondra@enterprisedb.com>
Reviewed-by: Alena Rybakina <lena.ribackina@yandex.ru>
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
8 weeks agoTeach Append to consider tuple_fraction when accumulating subpaths.
Alexander Korotkov [Mon, 10 Mar 2025 11:38:39 +0000 (13:38 +0200)]
Teach Append to consider tuple_fraction when accumulating subpaths.

This change is dedicated to more active usage of IndexScan and parameterized
NestLoop paths in partitioned cases under an Append node, as it already works
with plain tables.  As newly added regression tests demonstrate, it should
provide more smartness to the partitionwise technique.

With an indication of how many tuples are needed, it may be more meaningful
to use the 'fractional branch' subpaths of the Append path list, which are
more optimal for this specific number of tuples.  Planning on a higher level,
if the optimizer needs all the tuples, it will choose non-fractional paths.
In the case when, during execution, Append needs to return fewer tuples than
declared by tuple_fraction, it would not be harmful to use the 'intermediate'
variant of paths.  However, it will earn a considerable profit if a sensible
set of tuples is selected.

The change of the existing regression test demonstrates the positive outcome
of this feature: instead of scanning the whole table, the optimizer prefers
to use a parameterized scan, being aware of the only single tuple the join
has to produce to perform the query.

Discussion: https://www.postgresql.org/message-id/flat/CAN-LCVPxnWB39CUBTgOQ9O7Dd8DrA_tpT1EY3LNVnUuvAX1NjA%40mail.gmail.com
Author: Nikita Malakhov <hukutoc@gmail.com>
Author: Andrei Lepikhov <lepihov@gmail.com>
Reviewed-by: Andy Fan <zhihuifan1213@163.com>
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
8 weeks agoRemove support for temporal RESTRICT foreign keys
Peter Eisentraut [Mon, 10 Mar 2025 10:29:54 +0000 (11:29 +0100)]
Remove support for temporal RESTRICT foreign keys

It isn't clear how these should behave, so let's wait to implement them
until we are sure how to do it.

This feature was initially added by commit 89f908a6d0a, so it hasn't
been released yet.

Author: Paul A. Jungwirth <pj@illuminatedcomputing.com>
Discussion: https://postgr.es/m/e773bc11-4ac1-40de-bb91-814e02f05b6d%40eisentraut.org

8 weeks agoFix incorrect #endif comment
David Rowley [Mon, 10 Mar 2025 00:36:04 +0000 (13:36 +1300)]
Fix incorrect #endif comment

Noticed while reading code in this area.

2 months agoFix incorrect assertion in libpqwalreceiver
Heikki Linnakangas [Sun, 9 Mar 2025 18:40:45 +0000 (20:40 +0200)]
Fix incorrect assertion in libpqwalreceiver

Was supposed to check the length of the array, but was checking its
size in bytes.

Author: Jacob Brazeal <jacob.brazeal@gmail.com>
Discussion: https://www.postgresql.org/message-id/CA%2BCOZaA_9afJxj9ZuO73U5P7WXP%2BZM9NGnZvTDCmBFz0FGP%2BwA@mail.gmail.com

2 months agoFix test name and username used in failed connection attempts
Heikki Linnakangas [Sun, 9 Mar 2025 17:47:55 +0000 (19:47 +0200)]
Fix test name and username used in failed connection attempts

The first failed connection tests the "regular" connections limit, not
the reserved limit.

In the second failed connection, the username doesn't really matter,
but since the previous successful connections used "regress_reserved",
it seems weird to switch back to "regress_regular" for the
expected-to-fail attempt.

Discussion: https://www.postgresql.org/message-id/fd5e9523-78d3-4270-86b2-fd1b1eeb4fc9@iki.fi

2 months agoDon't try to parallelize array_agg() on an anonymous record type.
Tom Lane [Sun, 9 Mar 2025 17:11:20 +0000 (13:11 -0400)]
Don't try to parallelize array_agg() on an anonymous record type.

This doesn't work because record_recv requires the typmod that
identifies the specific record type (in our session) and
array_agg_deserialize has no convenient way to get that information.
The result is an "input of anonymous composite types is not
implemented" error.

We could probably make this work if we had to, but it does not seem
worth the trouble, given that it took this long to get a field report.
Just shut off parallelization, as though record_recv didn't exist.

Oversight in commit 16fd03e95.  Back-patch to v16 where that
came in.

Reported-by: Kirill Zdornyy <kirill@dineserve.com>
Diagnosed-by: Richard Guo <guofenglinux@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/atLI5Kce2ie1zcYjU0w_kjtVaxiYbYGTihrkLDmGZQnRDD4pnXukIATaABbnIj9pUnelC4ESvCXMm4HAyHg-v61XABaKpERj0A2IXzJZM7g=@dineserve.com
Backpatch-through: 16

2 months agodoc: Adjust note about pg_upgrade's --jobs option.
Nathan Bossart [Sat, 8 Mar 2025 20:28:16 +0000 (14:28 -0600)]
doc: Adjust note about pg_upgrade's --jobs option.

Presently, this section lists a couple of parallelized parts of
pg_upgrade and suggests a starting point for setting the --jobs
option.  The list of parallelized tasks is not particularly
actionable, and the phrasing for the --jobs recommendation is
confusing to some readers.

This commit attempts to improve this section by eliminating the
list of parallelized tasks and instead highlighting that --jobs is
most useful for clusters with multiple databases or tablespaces.
Additionally, the recommendation for setting --jobs is simplified
to suggest starting with the number of CPU cores.

Reported-by: Magnus Hagander <magnus@hagander.net>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Magnus Hagander <magnus@hagander.net>
Discussion: https://postgr.es/m/Z8dBn_5iGLNuYiPo%40nathan

2 months agoDon't convert to and from floats in pg_dump.
Jeff Davis [Sat, 8 Mar 2025 19:25:36 +0000 (11:25 -0800)]
Don't convert to and from floats in pg_dump.

Commit 8f427187db improved performance by remembering relation stats
as native types rather than issuing a new query for each relation.

Using native types is fine for integers like relpages; but reltuples
is floating point. The commit controllled for that complexity by using
setlocale(LC_NUMERIC, "C"). After that, Alexander Lakhin found a
problem in pg_strtof(), fixed in 00d61a08c5.

While we aren't aware of any more problems with that approach, it
seems wise to just use a string the whole way for floating point
values, as Corey's original patch did, and get rid of the
setlocale(). Integers are still converted to native types to avoid
wasting memory.

Co-authored-by: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://postgr.es/m/3049348.1740855411@sss.pgh.pa.us
Discussion: https://postgr.es/m/560cca3781740bd69881bb07e26eb8f65b09792c.camel%40j-davis.com

2 months agoClear errno before calling strtol() in spell.c.
Tom Lane [Sat, 8 Mar 2025 16:24:22 +0000 (11:24 -0500)]
Clear errno before calling strtol() in spell.c.

Per POSIX, a caller of strtol() that wishes to check for errors must
set errno to 0 beforehand.  Several places in spell.c neglected that,
so that they risked delivering a false overflow error in case errno
had been ERANGE already.  Given the lack of field reports, this case
may be unreachable at present --- but it's surely trouble waiting to
happen, so fix it.

Author: Jacob Brazeal <jacob.brazeal@gmail.com>
Discussion: https://postgr.es/m/CA+COZaBhsq6EromFm+knMJfzK6nTpG23zJ+K2=nfUQQXcj_xcQ@mail.gmail.com
Backpatch-through: 13

2 months agoMake parallel nbtree index scans use an LWLock.
Peter Geoghegan [Sat, 8 Mar 2025 16:10:14 +0000 (11:10 -0500)]
Make parallel nbtree index scans use an LWLock.

Teach parallel nbtree index scans to use an LWLock (not a spinlock) to
protect the scan's shared descriptor state.

Preparation for an upcoming patch that will add skip scan optimizations
to nbtree.  That patch will create the need to occasionally allocate
memory while the scan descriptor is locked, while copying datums that
were serialized by another backend.

Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Matthias van de Meent <boekewurm+postgres@gmail.com>
Discussion: https://postgr.es/m/CAH2-Wz=PKR6rB7qbx+Vnd7eqeB5VTcrW=iJvAsTsKbdG+kW_UA@mail.gmail.com

2 months agoMake amcanorder independent of amconsistentordering
Peter Eisentraut [Sat, 8 Mar 2025 08:37:06 +0000 (09:37 +0100)]
Make amcanorder independent of amconsistentordering

Follow-up to commit af4002b381d: Make amconsistentordering not depend
on amcanorder.  Although they are related, they are independent
properties.

Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/E1tngY6-0000UL-2n%40gemulon.postgresql.org

2 months agoFix typo
Peter Eisentraut [Sat, 8 Mar 2025 07:06:30 +0000 (08:06 +0100)]
Fix typo

Duplicate assignment in commit af4002b381d should have been a
different field.  (But it didn't affect the outcome.)

Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/E1tngY6-0000UL-2n%40gemulon.postgresql.org

2 months agoUse stricter ordering in regression test query for pg_stat_io
Michael Paquier [Sat, 8 Mar 2025 04:39:57 +0000 (13:39 +0900)]
Use stricter ordering in regression test query for pg_stat_io

The query introduced in 8b532771a099 is proving to have ordering issues
under at least the locale cs_CZ.  This commit updates the query to use a
stricter ordering.

Per reports from buildfarm members hippopotamus and jay.

2 months agoAdd regression test listing all the possible tuples in pg_stat_io
Michael Paquier [Sat, 8 Mar 2025 03:22:41 +0000 (12:22 +0900)]
Add regression test listing all the possible tuples in pg_stat_io

pg_stat_io returns a set of tuples based on a combination of three
properties (BackendType, IOObject and IOContext) and
pgstat_tracks_io_object() to decide if a BackendType should return a
tuple based on a pair made of an IOObject and an IOContext.

This commit adds a regression test to track all the combinations
supported.  This is useful to know which tuples are relevant when adding
a new BackendType to the set or when touching pgstat_tracks_io_object(),
and I have noticed while playing with this area that it is not
complicated to break it without the regression tests noticing a
difference in some cases.

Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/Z8exfAehbVbEKXW5@paquier.xyz

2 months agoImprove check for detection of pending data in backend statistics
Michael Paquier [Sat, 8 Mar 2025 01:56:30 +0000 (10:56 +0900)]
Improve check for detection of pending data in backend statistics

The callback pgstat_backend_have_pending_cb() is used as a way for
pg_stat_report() to detect if there is any pending data for backend
statistics.

It did not include a check based on pgstat_tracks_backend_bktype(), that
discards processes whose backend types do not support backend
statistics.  The logic is not a problem on HEAD, as processes that do
not support backend statistics cannot touch PendingBackendStats, so the
callback would always report that there is no pending data in this case.
However, we would run into trouble once backend statistics include
portions of pending stats that are not always zeroed, like pgWalUsage.

There is no reason for pgstat_backend_have_pending_cb() to not check
for pgstat_tracks_backend_bktype(), anyway, and this pattern is safer in
the long run, so let's update the code to do so.

While on it, this commit adds a proper initialization to
PendingBackendStats.

Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Co-authored-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/Z8l6EMM4ImVoWRkg@ip-10-97-1-34.eu-west-3.compute.internal

2 months agonbtree: refine _bt_readnextpage contract comments.
Peter Geoghegan [Fri, 7 Mar 2025 23:35:13 +0000 (18:35 -0500)]
nbtree: refine _bt_readnextpage contract comments.

Another minor follow-up commit for commit 1bd4bc85, which changed the
_bt_readnextpage contract.

2 months agoAssert that wrapper_handler()'s argument is within expected range.
Nathan Bossart [Fri, 7 Mar 2025 21:23:09 +0000 (15:23 -0600)]
Assert that wrapper_handler()'s argument is within expected range.

pqsignal() already does a similar check, but strange Valgrind
reports have us wondering if wrapper_handler() is somehow getting
called with an invalid signal number.

Reported-by: Tomas Vondra <tomas@vondra.me>
Suggested-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/ace01111-f9ac-4f61-b1b1-8e9379415444%40vondra.me
Backpatch-through: 17

2 months agoInclude column name in build_attrmap_by_position's error reports.
Tom Lane [Fri, 7 Mar 2025 18:24:09 +0000 (13:24 -0500)]
Include column name in build_attrmap_by_position's error reports.

Formerly we only provided the column number, but it's frequently
more useful to mention the column name.  The input tupdesc often
doesn't have useful column names, but the output tupdesc usually
contains user-supplied names, so report that one.

Author: Marcos Pegoraro <marcos@f10.com.br>
Co-authored-by: jian he <jian.universality@gmail.com>
Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us>
Co-authored-by: Erik Wienhold <ewie@ewie.name>
Reviewed-by: Vladlen Popolitov <v.popolitov@postgrespro.ru>
Discussion: https://postgr.es/m/CAB-JLwanky28gjAMdnMh1CjyO1b2zLdr6UOA1-oY9G7PVL9KKQ@mail.gmail.com

2 months agotests: Don't fail due to high default timeout in postmaster/003_start_stop
Andres Freund [Fri, 7 Mar 2025 18:09:16 +0000 (13:09 -0500)]
tests: Don't fail due to high default timeout in postmaster/003_start_stop

Some BF animals use very high timeouts due to their slowness. Unfortunately
postmaster/003_start_stop fails if a high timeout is configured, due to
authentication_timeout having a fairly low max.

As this test is reasonably fast, the easiest fix seems to be to cap the
timeout to 600.

Per buildfarm animal skink.

Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://postgr.es/m/ggflhkciwdyotpoie323chu2c2idpjk5qimrn462encwx2io7s@thmcxl7i6dpw

2 months agotests: Fix race condition in postmaster/002_connection_limits
Andres Freund [Fri, 7 Mar 2025 18:09:16 +0000 (13:09 -0500)]
tests: Fix race condition in postmaster/002_connection_limits

The test occasionally failed due to unexpected connection limit errors being
encountered after having waited for FATAL errors on another connection. These
spurious failures were caused by the the backend reporting FATAL errors to the
client before detaching from the PGPROC entry. Adding a sleep(1) before
proc_exit() makes it easy to reproduce that problem.

To fix the issue, add a helper function that waits for postmaster to notice
the process having exited. For now this is implemented by waiting for the
DEBUG2 message that postmaster logs in that case. That's not the prettiest
fix, but simple. If we notice this problem elsewhere, it might be worthwhile
to make this more general, e.g. by adding an injection point.

Reported-by: Tomas Vondra <tomas@vondra.me>
Diagnosed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Tested-by: Tomas Vondra <tomas@vondra.me>
Discussion: https://postgr.es/m/ggflhkciwdyotpoie323chu2c2idpjk5qimrn462encwx2io7s@thmcxl7i6dpw

2 months agodoc: Add missing decimal places to example rowcount.
Robert Haas [Fri, 7 Mar 2025 14:00:53 +0000 (09:00 -0500)]
doc: Add missing decimal places to example rowcount.

Commit 95dbd827f2edc4d10bebd7e840a0bd6782cf69b7 updated a bunch
of similar cases in the documentation, but missed this one.

Author: Ilia Evdokimov <ilya.evdokimov@tantorlabs.com>
Reviewed-by: Fabrízio de Royes Mello <fabriziomello@gmail.com>
2 months agoImprove possible performance regression
Peter Eisentraut [Fri, 7 Mar 2025 10:20:26 +0000 (11:20 +0100)]
Improve possible performance regression

Commit ce62f2f2a0a introduced calls to GetIndexAmRoutineByAmId() in
lsyscache.c functions.  This call is a bit more expensive than a
simple syscache lookup.  So rearrange the nesting so that we call that
one last and do the cheaper checks first.

Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/E1tngY6-0000UL-2n%40gemulon.postgresql.org

2 months agoRename amcancrosscompare
Peter Eisentraut [Fri, 7 Mar 2025 09:51:53 +0000 (10:51 +0100)]
Rename amcancrosscompare

After more discussion about commit ce62f2f2a0a, rename the index AM
property amcancrosscompare to two separate properties
amconsistentequality and amconsistentordering.  Also improve the
documentation and update some comments that were previously missed.

Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/E1tngY6-0000UL-2n%40gemulon.postgresql.org

2 months agoAllow casting between bytea and integer types.
Dean Rasheed [Fri, 7 Mar 2025 09:31:18 +0000 (09:31 +0000)]
Allow casting between bytea and integer types.

This allows smallint, integer, and bigint values to be cast to and
from bytea. The bytea value is the two's complement representation of
the integer, with the most significant byte first. For example:

  1234::bytea -> \x000004d2
  (-1234)::bytea -> \xfffffb2e

Author: Aleksander Alekseev <aleksander@timescale.com>
Reviewed-by: Joel Jacobson <joel@compiler.org>
Reviewed-by: Yugo Nagata <nagata@sraoss.co.jp>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Discussion: https://postgr.es/m/CAJ7c6TPtOp6%2BkFX5QX3fH1SVr7v65uHr-7yEJ%3DGMGQi5uhGtcA%40mail.gmail.com

2 months agoCREATE INDEX: don't update table stats if autovacuum=off.
Jeff Davis [Fri, 7 Mar 2025 03:36:34 +0000 (19:36 -0800)]
CREATE INDEX: don't update table stats if autovacuum=off.

We previously fixed this for binary upgrade in 71b66171d0, but a
similar problem remained when dumping statistics without data.

Fix by not opportunistically updating table stats during CREATE INDEX
when autovacuum is disabled. For stats to be stable at all, the server
needs to be aware that it should not take every opportunity to update
stats. Per discussion, autovacuum=off is a signal that the user
expects stats to be stable; though if necessary, we could create
a more specific mode in the future.

Reported-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://postgr.es/m/CAExHW5vf9D+8-a5_BEX3y=2y_xY9hiCxV1=C+FnxDvfprWvkng@mail.gmail.com
Discussion: https://postgr.es/m/ca81cbf6e6ea2af838df972801ad4da52640a503.camel%40j-davis.com

2 months agoRevert "vacuumdb: Add option for analyzing only relations missing stats."
John Naylor [Fri, 7 Mar 2025 03:35:21 +0000 (10:35 +0700)]
Revert "vacuumdb: Add option for analyzing only relations missing stats."

This reverts commit 5f8eb25706b62923c53172e453c8a4dedd877a3d, which in
my branch by mistake.

2 months agoDoc: correct aggressive vacuum threshold for multixact members storage
John Naylor [Fri, 7 Mar 2025 03:22:56 +0000 (10:22 +0700)]
Doc: correct aggressive vacuum threshold for multixact members storage

The threshold is two billion members, which was interpreted as 2GB
in the documentation. Fix to reflect that each member takes up five
bytes, which translates to about 10GB. This is not exact, because of
page boundaries. While at it, mention the maximum size 20GB.

This has been wrong since commit c552e171d16e, so backpatch to
version 14.

Author: Alex Friedman <alexf01@gmail.com>
Reviewed-by: Sami Imseih <samimseih@gmail.com>
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/CACbFw60UOk6fCC02KsyT3OfU9Dnuq5roYxdw2aFisiN_p1L0bg@mail.gmail.com
Backpatch-through: 14

2 months agovacuumdb: Add option for analyzing only relations missing stats.
Nathan Bossart [Tue, 4 Feb 2025 21:07:54 +0000 (15:07 -0600)]
vacuumdb: Add option for analyzing only relations missing stats.

This commit adds a new --missing-only option that can be used in
conjunction with --analyze-only and --analyze-in-stages.  When this
option is specified, vacuumdb will generate ANALYZE commands for a
relation if it is missing any statistics it should ordinarily have.
For example, if a table has statistics for one column but not
another, we will analyze the whole table.  A similar principle
applies to extended statistics, expression indexes, and table
inheritance.

Co-authored-by: Corey Huinker <corey.huinker@gmail.com>
Reviewed-by: TODO
Discussion: https://postgr.es/m/Z5O1bpcwDrMgyrYy%40nathan

2 months agoFix race condition in TAP test 007_pre_auth
Michael Paquier [Thu, 6 Mar 2025 23:12:45 +0000 (08:12 +0900)]
Fix race condition in TAP test 007_pre_auth

The authentication test added in c76db55c9085 expects a backend to start
and wait at the injection point "init-pre-auth".  A query is used to
retrieve the PID of the backend waiting at authentication, but its WHERE
clause was too soft, checking only for a backend in a "starting" state.

As proved by the CI, this WHERE clause is not enough.  There is a small
window between the moment when the backend is reported as "starting" in
its backend entry and the moment when it waits in its injection point,
and it was possible for the test to return the PID of a backend process
not yet waiting in the injection point, causing spurious failures.  This
issue is fixed by tweaking the query retrieving the PID of the backend
waiting before authentication so as we check for "init-pre-auth" in its
wait_event.  An extra check based on the backend_type is added, based on
a suggestion by Jacob, to be more cautious.

Error spotted by the CI on Windows, but it could happen anywhere, as
long as the authentication path is slow enough compared to the TAP test.

Reported-by: Andres Freund <andres@anarazel.de>
Author: Jacob Champion <jacob.champion@enterprisedb.com>
Co-authored-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/soexrl7oeyku24bj3czupxmv27ow35u6edymp5y3oyoysbe2kb@r3tgoos2xp2x

2 months agoreindexdb: move PQfinish() calls to the right place
Álvaro Herrera [Thu, 6 Mar 2025 17:14:41 +0000 (18:14 +0100)]
reindexdb: move PQfinish() calls to the right place

get_parallel_object_list() has no business closing a connection it did
not create.  Make things more sensible by closing the connection at the
level where it is created, in reindex_one_database().

Extracted from a larger patch by the same author.  However, the patch as
submitted not only was not described as containing this change, but in
addition it contained a fatal flaw whereby reindexdb would crash and
fail across all of its TAP test, which is why I list myself as
co-author.

Author: Ranier Vilela <ranier.vf@gmail.com>
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/CAEudQArfqr0-s0VVPSEh=0kgOgBJvFNdGW=xSL5rBcr0WDMQYQ@mail.gmail.com

2 months agoFix some performance issues in GIN query startup.
Tom Lane [Thu, 6 Mar 2025 16:54:27 +0000 (11:54 -0500)]
Fix some performance issues in GIN query startup.

If a GIN index search had a lot of search keys (for example,
"jsonbcol ?| array[]" with tens of thousands of array elements),
both ginFillScanKey() and startScanKey() took O(N^2) time.
Worse, those loops were uncancelable for lack of CHECK_FOR_INTERRUPTS.

The problem in ginFillScanKey() is the brute-force search key
de-duplication done in ginFillScanEntry().  The most expedient
solution seems to be to just stop trying to de-duplicate once
there are "too many" search keys.  We could imagine working harder,
say by using a sort-and-unique algorithm instead of brute force
compare-all-the-keys.  But it seems unlikely to be worth the trouble.
There is no correctness issue here, since the code already allowed
duplicate keys if any extra_data is present.

The problem in startScanKey() is the loop that attempts to identify
the first non-required search key.  In the submitted test case, that
vainly tests all the key positions, and each iteration takes O(N)
time.  One part of that is that it's reinitializing the entryRes[]
array from scratch each time, which is entirely unnecessary given
that the triConsistentFn isn't supposed to scribble on its input.
We can easily adjust the array contents incrementally instead.
The other part of it is that the triConsistentFn may itself take
O(N) time (and does in this test case).  This is all extremely
brute force: in simple cases with AND or OR semantics, we could
know without any looping whatever that all or none of the keys
are required.  But GIN opclasses don't have any API for exposing
that knowledge, so at least in the short run there is little to
be done about that.  Put in a CHECK_FOR_INTERRUPTS so that at
least the loop is cancelable.

These two changes together resolve the primary complaint that
the test query doesn't respond promptly to cancel interrupts.
Also, while they don't completely eliminate the O(N^2) behavior,
they do provide quite a nice speedup for mid-sized examples.

Bug: #18831
Reported-by: Niek <niek.brasa@hitachienergy.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/18831-e845ac44ebc5dd36@postgresql.org
Backpatch-through: 13

2 months agoFurther fix for json_strip_nulls documentation
Andrew Dunstan [Thu, 6 Mar 2025 15:24:03 +0000 (10:24 -0500)]
Further fix for json_strip_nulls documentation

Oversight in commit 4603903d294.

Author: Shinoda, Noriyoshi (SXD Japan FSI) <noriyoshi.shinoda@hpe.com>

2 months agoRemove extraneous commas in json{b}_strip_nulls documentation
Andrew Dunstan [Thu, 6 Mar 2025 13:46:15 +0000 (08:46 -0500)]
Remove extraneous commas in json{b}_strip_nulls documentation

Oversight in commit 4603903d294.

Author: Ian Lawrence Barwick <barwick@gmail.com>

2 months agoAvoid invalidating all RelationSyncCache entries on publication change.
Amit Kapila [Thu, 6 Mar 2025 08:49:38 +0000 (14:19 +0530)]
Avoid invalidating all RelationSyncCache entries on publication change.

On change of publication via ALTER PUBLICATION ... SET/ADD/DROP commands,
we were invalidating all the relations present in relation sync cache
maintained by pgoutput. We need to invalidate only the relation entries
that are changed as part of publication DDL.

We have ensured that the publication DDL execution generated the
invalidations required to invalidate impacted relation sync entries in
RelationSyncCache.

This improves the performance by avoiding building the cache entries for
the cases where a publication has many tables but only one of them is
dropped.

Author: Shlok Kyal <shlok.kyal.oss@gmail.com>
Author: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Reviewed-by: Hou Zhijie <houzj.fnst@fujitsu.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/OSCPR01MB14966C09AA201EFFA706576A7F5C92@OSCPR01MB14966.jpnprd01.prod.outlook.com

2 months agoOrganize and deduplicate statistics import tests.
Jeff Davis [Thu, 6 Mar 2025 08:19:22 +0000 (00:19 -0800)]
Organize and deduplicate statistics import tests.

Author: Corey Huinker <corey.huinker@gmail.com>
Reported-by: Melanie Plageman <melanieplageman@gmail.com>
Discussion: https://postgr.es/m/CAAKRu_bWEqUfxhODfJ-XbZC75vq=P6DYOKK6biyey=yM1Ah3Hg@mail.gmail.com
Discussion: https://postgr.es/m/CADkLM=f1n2_Vomq0gKab7xdxDHmJGgn=DE48P8fzQOp3Mrs1Qg@mail.gmail.com

2 months agoAddress stats export review comments.
Jeff Davis [Thu, 6 Mar 2025 08:11:12 +0000 (00:11 -0800)]
Address stats export review comments.

Per discussion, did not use Jian He's patch exactly.

Reported-by: jian he <jian.universality@gmail.com>
Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://postgr.es/m/CACJufxFVq=tq9u1zrHWYSbMi1T07gS9Ff0LJScMco4HZmtZ1xw@mail.gmail.com
Discussion: https://postgr.es/m/CADkLM=f1n2_Vomq0gKab7xdxDHmJGgn=DE48P8fzQOp3Mrs1Qg@mail.gmail.com

2 months agoAddress stats import review comments.
Jeff Davis [Thu, 6 Mar 2025 07:07:25 +0000 (23:07 -0800)]
Address stats import review comments.

Reported-by: jian he <jian.universality@gmail.com>
Discussion: https://postgr.es/m/CACJufxHG9MBQozbJQ4JRBcRbUO+t+sx4qLZX092rS_9b4SR_EA@mail.gmail.com

2 months agoFix compiler warnings about typedef redefinitions
Heikki Linnakangas [Thu, 6 Mar 2025 01:10:22 +0000 (03:10 +0200)]
Fix compiler warnings about typedef redefinitions

Clang with -Wtypedef-redefinition produced warnings:

    src/include/storage/latch.h:122:3: error: redefinition of typedef 'Latch' is a C11 feature [-Werror,-Wtypedef-redefinition]

Per buildfarm

2 months agoAdd more monitoring data for WAL writes in the WAL receiver
Michael Paquier [Thu, 6 Mar 2025 00:39:45 +0000 (09:39 +0900)]
Add more monitoring data for WAL writes in the WAL receiver

This commit adds two improvements related to the monitoring of WAL
writes for the WAL receiver.

First, write counts and timings are now counted in pg_stat_io for the
WAL receiver.  These have been discarded from pg_stat_wal in
ff99918c625a due to performance concerns, related to the fact that we
still relied on an on-disk file for the stats back then, even with
track_wal_io_timing to avoid the overhead of the timestamp calculations.
This implementation is simpler than the original proposal as it is
possible to rely on the APIs of pgstat_io.c to do the job.  Like the
fsync and read data, track_wal_io_timing needs to be enabled to track
the timings.

Second, a wait event is added around the pg_pwrite() call in charge of
the writes, using the exiting WAIT_EVENT_WAL_WRITE.  This is useful as
the WAL receiver data is tracked in pg_stat_activity.

Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/Z8gFnH4o3jBm5BRz@ip-10-97-1-34.eu-west-3.compute.internal

2 months agoSplit WaitEventSet functions to separate source file
Heikki Linnakangas [Wed, 5 Mar 2025 23:26:16 +0000 (01:26 +0200)]
Split WaitEventSet functions to separate source file

latch.c now only contains the Latch related functions, which build on
the WaitEventSet abstraction. Most of the platform-dependent stuff is
now in waiteventset.c.

Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/8a507fb6-df28-49d3-81a5-ede180d7f0fb@iki.fi

2 months agoUse ModifyWaitEvent to update exit_on_postmaster_death
Heikki Linnakangas [Wed, 5 Mar 2025 23:26:12 +0000 (01:26 +0200)]
Use ModifyWaitEvent to update exit_on_postmaster_death

This is in preparation for splitting WaitEventSet related functions to
a separate source file. That will hide the details of WaitEventSet
from WaitLatch, so it must use an exposed function instead of
modifying WaitEventSet->exit_on_postmaster_death directly.

Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/8a507fb6-df28-49d3-81a5-ede180d7f0fb@iki.fi

2 months agoecpg: Fix compiler warning in ecpg build with Meson.
Fujii Masao [Wed, 5 Mar 2025 23:22:30 +0000 (08:22 +0900)]
ecpg: Fix compiler warning in ecpg build with Meson.

Previously, Meson could produce a warning about the use of 'deps' in ecpg:

    WARNING: Project targets '>=0.54' but uses a feature introduced in '0.60.0': list.<plus>. The right-hand operand was not a list.

The right-hand operand of 'deps' should be a list. This commit fixes
the warning by wrapping it with square brackets.

This issue was introduced in commit 28f04984f0c.

Author: Jacob Champion <jacob.champion@enterprisedb.com>
Discussion: https://postgr.es/m/CAOYmi+ks8wO06Ymxduw2h_eQJ_D4_jHGeyMK0P=p5Q3psnEdMA@mail.gmail.com

2 months agoRemove unused ShutdownLatchSupport() function
Heikki Linnakangas [Wed, 5 Mar 2025 21:46:29 +0000 (23:46 +0200)]
Remove unused ShutdownLatchSupport() function

The only caller was removed in commit 80a8f95b3b. I don't foresee
needing it any time soon, and I'm working on some big changes in this
area, so let's remove it out of the way.

Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/8a507fb6-df28-49d3-81a5-ede180d7f0fb@iki.fi

2 months agoci: Remove installation of libcurl
Daniel Gustafsson [Wed, 5 Mar 2025 21:12:20 +0000 (22:12 +0100)]
ci: Remove installation of libcurl

The CI images come with libcurl pre-installed since commit a119426
in the pg-vm-images repository so remove the installation commands
from the Cirrus tasks.  Installation of libcurl packages was added
in the OAuth patchset which introduced the dependency, a backpatch
is thus not applicable.

Author: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/8745B9D8-D897-4302-BD4C-FC18F291ECB7@yesql.se

2 months agoci: Document what makes certain tasks special
Andres Freund [Wed, 5 Mar 2025 18:19:28 +0000 (13:19 -0500)]
ci: Document what makes certain tasks special

To increase coverage without drastically increasing CI resource usage, we have
different CI tasks test different things (e.g. the linux tasks use
sanitizers).  Unfortunately that can create confusing situations where CI
fails on some OS, but not others, without the problem appearing to be platform
dependent.

To, partially, address that, add a comment, prefixed with SPECIAL, to each
task that we use to test in some non-default way.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/321570.1741195755@sss.pgh.pa.us

2 months agoci: freebsd: Specify debug_parallel_query=regress
Andres Freund [Wed, 5 Mar 2025 18:19:28 +0000 (13:19 -0500)]
ci: freebsd: Specify debug_parallel_query=regress

A lot of buildfarm animals run with debug_parallel_query=regress, while CI
didn't test that. That lead to the annoying situation of only noticing related
test instabilities after merging changes upstream.

FreeBSD was chosen because it's a relatively fast task. It also tests
debug_write_read_parse_plan_trees etc, which probably is exercised a bit more
heavily with debug_parallel_query=regress.

Discussion: https://postgr.es/m/zbuk4mlov22yfoktf5ub3lwjw2b7ezwphwolbplthepda42int@h6wpvq7orc44

2 months agoci: Upgrade FreeBSD image
Andres Freund [Wed, 5 Mar 2025 15:33:47 +0000 (10:33 -0500)]
ci: Upgrade FreeBSD image

Upgrade to the current stable version. To avoid needing commits like this in
the future, the CI image name now doesn't contain the OS version number
anymore.

Backpatch to all versions with CI support, we don't want to generate CI images
for multiple FreeBSD versions.

Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/CAN55FZ3_P4JJ6tWZafjf-_XbHgG6DQGXhH-y6Yp78_bwBJjcww@mail.gmail.com
Backpatch-through: 15

2 months agoRevert "Show index search count in EXPLAIN ANALYZE."
Peter Geoghegan [Wed, 5 Mar 2025 15:27:31 +0000 (10:27 -0500)]
Revert "Show index search count in EXPLAIN ANALYZE."

This reverts commit 5ead85fbc81162ab1594f656b036a22e814f96b3.

This commit shows test failures with debug_parallel_query=regress.  The
underlying issue needs to be debugged, so revert for now.

2 months agoAllow json{b}_strip_nulls to remove null array elements
Andrew Dunstan [Wed, 5 Mar 2025 14:50:34 +0000 (09:50 -0500)]
Allow json{b}_strip_nulls to remove null array elements

An additional paramater ("strip_in_arrays") is added to these functions.
It defaults to false. If true, then null array elements are removed as
well as null valued object fields. JSON that just consists of a single
null is not affected.

Author: Florents Tselai <florents.tselai@gmail.com>

Discussion: https://postgr.es/m/4BCECCD5-4F40-4313-9E98-9E16BEB0B01D@gmail.com

2 months agoShow index search count in EXPLAIN ANALYZE.
Peter Geoghegan [Wed, 5 Mar 2025 14:36:48 +0000 (09:36 -0500)]
Show index search count in EXPLAIN ANALYZE.

Expose the count of index searches/index descents in EXPLAIN ANALYZE's
output for index scan nodes.  This information is particularly useful
with scans that use ScalarArrayOp quals, where the number of index scans
isn't predictable in advance (at least not with optimizations like the
one added to nbtree by Postgres 17 commit 5bf748b8).  It will also be
useful when EXPLAIN ANALYZE shows details of an nbtree index scan that
uses skip scan optimizations set to be introduced by an upcoming patch.

The instrumentation works by teaching index AMs to increment a new
nsearches counter whenever a new index search begins.  The counter is
incremented at exactly the same point that index AMs must already
increment the index's pg_stat_*_indexes.idx_scan counter (we're counting
the same event, but at the scan level rather than the relation level).
The new counter is stored in the scan descriptor (IndexScanDescData),
which explain.c reaches by going through the scan node's PlanState.

This approach doesn't match the approach used when tracking other index
scan specific costs (e.g., "Rows Removed by Filter:").  It is similar to
the approach used in other cases where we must track costs that are only
readily accessible inside an access method, and not from the executor
(e.g., "Heap Blocks:" output for a Bitmap Heap Scan).  It is inherently
necessary to maintain a counter that can be incremented multiple times
during a single amgettuple call (or amgetbitmap call), and directly
exposing PlanState.instrument to index access methods seems unappealing.

Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Tomas Vondra <tomas@vondra.me>
Reviewed-By: Robert Haas <robertmhaas@gmail.com>
Reviewed-By: Masahiro Ikeda <ikedamsh@oss.nttdata.com>
Reviewed-By: Matthias van de Meent <boekewurm+postgres@gmail.com>
Discussion: https://postgr.es/m/CAH2-Wz=PKR6rB7qbx+Vnd7eqeB5VTcrW=iJvAsTsKbdG+kW_UA@mail.gmail.com
Discussion: https://postgr.es/m/CAH2-WzkRqvaqR2CTNqTZP0z6FuL4-3ED6eQB0yx38XBNj1v-4Q@mail.gmail.com

2 months agoRename some signal and interrupt handling functions for consistency
Heikki Linnakangas [Wed, 5 Mar 2025 14:22:26 +0000 (16:22 +0200)]
Rename some signal and interrupt handling functions for consistency

The usual pattern for handling a signal is that the signal handler
sets a flag and calls SetLatch(MyLatch), and CHECK_FOR_INTERRUPTS() or
other code that is part of a wait loop calls another function to deal
with it. The naming of the functions involved was a bit inconsistent,
however. CHECK_FOR_INTERRUPTS() calls ProcessInterrupts() to do the
heavy-lifting, but the analogous functions in aux processes were
called HandleMainLoopInterrupts(), HandleStartupProcInterrupts(),
etc. Similarly, most subroutines of ProcessInterrupts() were called
Process*(), but some were called Handle*().

To make things less confusing, rename all the functions that are part
of the overall signal/interrupt handling system but are not executed
in a signal handler to e.g. ProcessSomething(), rather than
HandleSomething(). The "Process" prefix is now consistently used in
the non-signal-handler functions, and the "Handle" prefix in functions
that are part of signal handlers, except for some completely unrelated
functions that clearly have nothing to do with signal or interrupt
handling.

Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Discussion: https://www.postgresql.org/message-id/8a384b26-1499-41f6-be33-64b801fb98b8@iki.fi

2 months agoAdd ALTER TABLE ... ALTER CONSTRAINT ... SET [NO] INHERIT
Álvaro Herrera [Wed, 5 Mar 2025 12:50:22 +0000 (13:50 +0100)]
Add ALTER TABLE ... ALTER CONSTRAINT ... SET [NO] INHERIT

This allows to redefine an existing non-inheritable constraint to be
inheritable, which allows to straighten up situations with NO INHERIT
constraints so that thay can become normal constraints without having to
re-verify existing data.  For existing inheritance children this may
require creating additional constraints, if they don't exist already.

It also allows to do the opposite, if only for symmetry.

Author: Suraj Kharage <suraj.kharage@enterprisedb.com>
Reviewed-by: jian he <jian.universality@gmail.com>
Discussion: https://postgr.es/m/CAF1DzPVfOW6Kk=7SSh7LbneQDJWh=PbJrEC_Wkzc24tHOyQWGg@mail.gmail.com

2 months agoFix some gaps in pg_stat_io with WAL receiver and WAL summarizer
Michael Paquier [Wed, 5 Mar 2025 01:17:39 +0000 (10:17 +0900)]
Fix some gaps in pg_stat_io with WAL receiver and WAL summarizer

The WAL receiver and WAL summarizer processes gain each one a call to
pgstat_report_wal(), to make sure that they report their WAL statistics
to pgstats, gathering data for pg_stat_io.

In the WAL receiver, the stats reports are timed with status updates sent
to the primary, that depend on wal_receiver_status_interval and
wal_receiver_timeout.  This is a conservative choice, but perhaps we
could be more aggressive with the frequency of the stats reports.  An
interesting historical fact is that the WAL receiver does writes and
syncs of WAL, but it has never reported its statistics to pgstats in
pg_stat_wal.

In the WAL summarizer, the stats reports are done each time the process
waits for WAL.

While on it, pg_stat_io is adjusted so as these two processes do not
report any rows when IOObject is not WAL, making the view easier to use
with less rows.

Two tests are added in TAP, checking statistics for the WAL summarizer
and the WAL receiver.  Status updates in the WAL receiver are currently
possible in the recovery test 001_stream_rep.pl.

Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/Z8UKZyVSHUUQJHNb@paquier.xyz

2 months agopsql: Fix memory leak with \gx used within a pipeline
Michael Paquier [Tue, 4 Mar 2025 22:56:03 +0000 (07:56 +0900)]
psql: Fix memory leak with \gx used within a pipeline

While inside a pipeline, \gx is currently forbidden and will make
exec_command_g() exit early.  There was a memory leak in this code path,
so let's fix it.

Author: Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com>
Discussion: https://postgr.es/m/CAO6_XqqFVQjLjZQiL7xdwLpzZEy1ghO_JWvCFPM_OmwF9s7XdA@mail.gmail.com

2 months agoEnforce memory limit during parallel GIN builds
Tomas Vondra [Tue, 4 Mar 2025 19:37:55 +0000 (20:37 +0100)]
Enforce memory limit during parallel GIN builds

Index builds are expected to respect maintenance_work_mem, just like
other maintenance operations. For serial builds this is done simply by
flushing the buffer in ginBuildCallback() into the index. But with
parallel builds it's more complicated, because there are multiple places
that can allocate memory.

ginBuildCallbackParallel() does the same thing as ginBuildCallback(),
except that the accumulated items are written into tuplesort. Then the
entries with the same key get merged - first in the worker, then in the
leader - and the TID lists may get (arbitrarily) long. It's unlikely it
would exceed the memory limit, but it's possible. We address this by
evicting some of the data if the list gets too long.

We can't simply dump the whole in-memory TID list. The GIN index bulk
insert code expects to see TIDs in monotonic order; it may fail if the
TIDs go backwards. If the TID lists overlap, evicting the whole current
TID list would break this (a later entry might add "old" TID values into
the already-written part).

In the workers this is not an issue, because the lists never overlap.
But the leader may see overlapping lists produced by the workers.

We can however derive a safe "horizon" TID - the entries (for a given
key) are sorted by (key, first TID), which means no future list can add
values before the last "first TID" we've seen. This patch tracks the
"frozen" part of the TID list, which we know can't change by merging
additional TID lists. If needed, we can evict this part of the list.

We don't want to do this too often - the smaller lists we evict, the
more expensive it'll be to merge them in the next step (especially in
the leader). Therefore we only trim the list if we have at least 1024
frozen items, and if the whole list is at least 64kB large.

These thresholds are somewhat arbitrary and conservative. We might
calculate the values from maintenance_work_mem, but tests show that does
not really improve anything (time, compression ratio, ...). So we stick
to these conservative values to release memory faster.

Author: Tomas Vondra
Reviewed-by: Matthias van de Meent, Andy Fan, Kirill Reshke
Discussion: https://postgr.es/m/6ab4003f-a8b8-4d75-a67f-f25ad98582dc%40enterprisedb.com

2 months agopg_upgrade: Check for the expected error message in TAP tests.
Masahiko Sawada [Tue, 4 Mar 2025 19:16:12 +0000 (11:16 -0800)]
pg_upgrade: Check for the expected error message in TAP tests.

Since pg_upgrade prints its error messages on stdout, we can't use
command_fails_like() to check if it fails for the right reason. This
commit uses command_checks_all() in pg_upgrade TAP tests to check the
exit status and stdout, enabling proper verification of error
reasons.

Author: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Discussion: https://postgr.es/m/87tt8h1vb7.fsf@wibble.ilmari.org

2 months agoFix ALTER TABLE error message
Álvaro Herrera [Tue, 4 Mar 2025 19:07:30 +0000 (20:07 +0100)]
Fix ALTER TABLE error message

This bogus error message was introduced in 2013 by commit f177cbfe676d,
because of misunderstanding the processCASbits() API; at the time, no
test cases were added that would be affected by this change.  Only in
ca87c415e2fc was one added (along with a couple of typos), with an XXX
note that the error message was bogus.  Fix the whole, add some test
cases.

Backpatch all the way back.

Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Discussion: https://postgr.es/m/202503041822.aobpqke3igvb@alvherre.pgsql

2 months agoRefactor Copy{From|To}GetRoutine() to use pass-by-reference argument.
Masahiko Sawada [Tue, 4 Mar 2025 18:38:41 +0000 (10:38 -0800)]
Refactor Copy{From|To}GetRoutine() to use pass-by-reference argument.

The change improves efficiency by eliminating unnecessary copying of
CopyFormatOptions.

The coverity also complained about inefficiencies caused by
pass-by-value.

Oversight in 7717f6300 and 2e4127b6d.

Reported-by: Junwang Zhao <zhjwpku@gmail.com>
Reported-by: Tom Lane <tgl@sss.pgh.pa.us> (per reports from coverity)
Author: Sutou Kouhei <kou@clear-code.com>
Reviewed-by: Junwang Zhao <zhjwpku@gmail.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAEG8a3L6YCpPksTQMzjD_CvwDEhW3D_t=5md9BvvdOs5k+TA=Q@mail.gmail.com

2 months agoCompress TID lists when writing GIN tuples to disk
Tomas Vondra [Tue, 4 Mar 2025 18:02:04 +0000 (19:02 +0100)]
Compress TID lists when writing GIN tuples to disk

When serializing GIN tuples to tuplesorts during parallel index builds,
we can significantly reduce the amount of data by compressing the TID
lists. The GIN opclasses may produce a lot of data (depending on how
many keys are extracted from each row), and the TID compression is very
efficient and effective.

If the number of distinct keys is high, the first worker pass (reading
data from the table and writing them into a private tuplesort) may not
benefit from the compression very much. It is likely to spill data to
disk before the TID lists get long enough for the compression to help.
The second pass (writing the merged data into the shared tuplesort) is
more likely to benefit from compression.

The compression can be seen as a way to reduce the amount of disk space
needed by the parallel builds, because the data is written twice. First
into the per-worker tuplesorts, then into the shared tuplesort.

Author: Tomas Vondra
Reviewed-by: Matthias van de Meent, Andy Fan, Kirill Reshke
Discussion: https://postgr.es/m/6ab4003f-a8b8-4d75-a67f-f25ad98582dc%40enterprisedb.com

2 months agoAdd .gitignore entry for ecpg test detritus.
Tom Lane [Tue, 4 Mar 2025 17:58:04 +0000 (12:58 -0500)]
Add .gitignore entry for ecpg test detritus.

Oversight in commit 28f04984f.

2 months agoMake FP_LOCK_SLOTS_PER_BACKEND look like a function
Tomas Vondra [Tue, 4 Mar 2025 17:33:09 +0000 (18:33 +0100)]
Make FP_LOCK_SLOTS_PER_BACKEND look like a function

The FP_LOCK_SLOTS_PER_BACKEND macro looks like a constant, but it
depends on the max_locks_per_transaction GUC, and thus can change. This
is non-obvious and confusing, so make it look more like a function by
renaming it to FastPathLockSlotsPerBackend().

While at it, use the macro when initializing fast-path shared memory,
instead of using the formula.

Reported-by: Andres Freund
Discussion: https://postgr.es/m/ffiwtzc6vedo6wb4gbwelon5nefqg675t5c7an2ta7pcz646cg%40qwmkdb3l4ett

2 months agoAdd regression tests for pg_stat_progress_copy.tuples_skipped.
Fujii Masao [Tue, 4 Mar 2025 14:56:49 +0000 (23:56 +0900)]
Add regression tests for pg_stat_progress_copy.tuples_skipped.

This commit adds tests to verify that tuples_skipped in pg_stat_progress_copy
works as expected. While existing tests checked other fields, tuples_skipped
was previously untested.

This improves test coverage and ensures accurate tracking of skipped tuples.

Author: Jian He <jian.universality@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Josef Šimánek <josef.simanek@gmail.com>
Discussion: https://postgr.es/m/CACJufxFazq-bfyhiO0KBojR=yOr84E25Rqf6mHB0Ow0KPidkKw@mail.gmail.com

2 months agoFix outdated comment
Heikki Linnakangas [Tue, 4 Mar 2025 13:33:19 +0000 (15:33 +0200)]
Fix outdated comment

Commit bc971f4025 replaced the latch-setting mechanism that the
comment talked about with a condition variable. And before that,
commit 2258e76f90 moved the code so that the comment got detached from
the loop that it talked about, so move the comment closer to the loop.

2 months agodoc: Expand version compatibility for pg_basebackup features
Daniel Gustafsson [Tue, 4 Mar 2025 11:08:27 +0000 (12:08 +0100)]
doc: Expand version compatibility for pg_basebackup features

This updates the paragraph on backwards compatitibility for server
features to include --incremental which only works on servers with
v17 or newer.  Backpatch down to v17 where incremental backup was
added.

Author: David G. Johnston <David.G.Johnston@Gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/CAKFQuwZYfZyeTkS3g2Ovw84TsxHa796xnf-u5kfgn_auyxZk0Q@mail.gmail.com
Backpatch-through: 17

2 months agoFix accidental use of = instead of ==
Peter Eisentraut [Tue, 4 Mar 2025 08:45:01 +0000 (09:45 +0100)]
Fix accidental use of = instead of ==

Fix for commit 630f9a43cec.  It used = instead of ==.  The result
would be an incorrect error message.

Author: Jacob Brazeal <jacob.brazeal@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/CA%2BCOZaC-JMbhQ4O0Q8V1Bxa0R%2BNex_RN9D6UyuLPiEx_CK4Heg%40mail.gmail.com

2 months agoFix ALTER TABLE ADD VIRTUAL GENERATED COLUMN when table rewrite
Peter Eisentraut [Tue, 4 Mar 2025 08:18:32 +0000 (09:18 +0100)]
Fix ALTER TABLE ADD VIRTUAL GENERATED COLUMN when table rewrite

demo:
CREATE TABLE gtest20a (a int PRIMARY KEY, b int GENERATED ALWAYS AS (a * 2) VIRTUAL);
ALTER TABLE gtest20a ADD COLUMN c float8 DEFAULT RANDOM() CHECK (b < 60);
ERROR:  no generation expression found for column number 2 of table "pg_temp_17306"

In ATRewriteTable, the variable OIDNewHeap (if valid) corresponding
pg_attrdef default expression entry was not populated.  So OIDNewHeap
cannot be used to call expand_generated_columns_in_expr or
build_generation_expression.  Therefore in ATRewriteTable, we can only
use the existing relation to expand the generated expression.

Author: jian he <jian.universality@gmail.com>
Reviewed-by: Srinath Reddy <srinath2133@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CACJufxEJ%3DFoajabWXjszo_yrQeKSxdZ87KJqBW373rSbajKGAA%40mail.gmail.com

2 months agoAvoid NullTest deduction for clone clauses
Richard Guo [Tue, 4 Mar 2025 07:11:03 +0000 (16:11 +0900)]
Avoid NullTest deduction for clone clauses

In commit b262ad440, we introduced an optimization that reduces an IS
NOT NULL qual on a column defined as NOT NULL to constant true, and an
IS NULL qual on a NOT NULL column to constant false, provided we can
prove that the input expression of the NullTest is not nullable by any
outer join.  This deduction happens after we have generated multiple
clones of the same qual condition to cope with commuted-left-join
cases.

However, performing the NullTest deduction for clone clauses can be
unsafe, because we don't have a reliable way to determine if the input
expression of a NullTest is non-nullable: nullingrel bits in clone
clauses may not reflect reality, so we dare not draw conclusions from
clones about whether Vars are guaranteed not-null.

To fix, we check whether the given RestrictInfo is a clone clause in
restriction_is_always_true and restriction_is_always_false, and avoid
performing any reduction if it is.

There are several ensuing plan changes in predicate.out, and we have
to modify the tests to ensure that they continue to test what they are
intended to.  Additionally, this fix causes the test case added in
f00ab1fd1 to no longer trigger the bug that commit fixed, so we also
remove that test case.

Back-patch to v17 where this bug crept in.

Reported-by: Ronald Cruz <cruz@rentec.com>
Diagnosed-by: Tom Lane <tgl@sss.pgh.pa.us>
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/f5320d3d-77af-4ce8-b9c3-4715ff33f213@rentec.com
Backpatch-through: 17

2 months agoecpg: Add TAP test for the ecpg command.
Fujii Masao [Tue, 4 Mar 2025 05:58:46 +0000 (14:58 +0900)]
ecpg: Add TAP test for the ecpg command.

This commit adds a TAP test to verify that the ecpg command correctly
detects unsupported or disallowed statements in input files and reports
the appropriate error or warning messages.

This test helps catch bugs like the one introduced in commit 3d009e45bd,
which broke ecpg's handling of unsupported COPY FROM STDIN statements,
later fixed by commit 94b914f601b.

Author: Ryo Kanbayashi <kanbayashi.dev@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CANOn0EzoMyxA1m-quDS1UeQUq6FNki6+GGiGucgr9tm2R78rKw@mail.gmail.com

2 months agoSplit pgstat_bestart() into three different routines
Michael Paquier [Tue, 4 Mar 2025 05:09:44 +0000 (14:09 +0900)]
Split pgstat_bestart() into three different routines

pgstat_bestart(), used post-authentication to set up a backend entry
in the PgBackendStatus array, so as its data becomes visible in
pg_stat_activity and related catalogs, has its logic divided into three
routines with this commit, called in order at different steps of the
backend initialization:
* pgstat_bestart_initial() sets up the backend entry with a minimal
amount of information, reporting it with a new BackendState called
STATE_STARTING while waiting for backend initialization and client
authentication to complete.  The main benefit that this offers is
observability, so as it is possible to monitor the backend activity
during authentication.  This step happens earlier than in the logic
prior to this commit.  pgstat_beinit() happens earlier as well, before
authentication.
* pgstat_bestart_security() reports the SSL/GSS status of the
connection, once authentication completes.  Auxiliary processes, for
example, do not need to call this step, hence it is optional.  This
step is called after performing authentication, same as previously.
* pgstat_bestart_final() reports the user and database IDs, takes the
entry out of STATE_STARTING, and reports its application_name.  This is
called as the last step of the three, once authentication completes.

An injection point is added, with a test checking that the "starting"
phase of a backend entry is visible in pg_stat_activity.  Some follow-up
patches are planned to take advantage of this refactoring with more
information provided in backend entries during authentication (LDAP
hanging was a problem for the author, initially).

Author: Jacob Champion <jacob.champion@enterprisedb.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CAOYmi+=60deN20WDyCoHCiecgivJxr=98s7s7-C8SkXwrCfHXg@mail.gmail.com

2 months agoAdd more assertions in palloc0() and palloc_extended()
Michael Paquier [Tue, 4 Mar 2025 01:53:10 +0000 (10:53 +0900)]
Add more assertions in palloc0() and palloc_extended()

palloc() includes an assertion checking that an alloc() implementation
never returns NULL for all MemoryContextMethods.

This commit adds a similar assertion in palloc0().  In palloc_extend(),
a different assertion is added, checking that MCXT_ALLOC_NO_OOM is set
when an alloc() routine returns NULL.  These additions can be useful to
catch errors when implementing a new set of MemoryContextMethods
routines.

Author: Andreas Karlsson <andreas@proxel.se>
Discussion: https://postgr.es/m/507e8eba-2035-4a12-a777-98199a66beb8@proxel.se

2 months agodoc: Convert UUID functions list to table format.
Masahiko Sawada [Mon, 3 Mar 2025 23:44:01 +0000 (15:44 -0800)]
doc: Convert UUID functions list to table format.

Convert the list of UUID functions into a table for better
readability. This commit also adds references to the UUID type section
and includes descriptions of different UUID generation algorithm
versions.

Author: Andy Alsup <bluesbreaker@gmail.com>
Reviewed-by: Laurenz Albe <laurenz.albe@cybertec.at>
Discussion: https://postgr.es/m/CADOZ7s7OHag+r6w+BzKw2xgb3fVtAD-pU=_N9-9pSe5W1TB+xQ@mail.gmail.com

2 months agoAllow => syntax for named cursor arguments in plpgsql.
Tom Lane [Mon, 3 Mar 2025 23:00:05 +0000 (18:00 -0500)]
Allow => syntax for named cursor arguments in plpgsql.

We've traditionally accepted "name := value" syntax for
cursor arguments in plpgsql.  But it turns out that the
equivalent statements in Oracle use "name => value".
Since we accept both forms of punctuation for function
arguments, it makes sense to do the same here.

Author: Pavel Stehule <pavel.stehule@gmail.com>
Reviewed-by: Gilles Darold <gilles@darold.net>
Discussion: https://postgr.es/m/CAFj8pRA3d0ARQEMbABa1n6q25AUdNmyO8aGs56XNf9pD4sRMjQ@mail.gmail.com

2 months agoci: Use a RAM disk for NetBSD and OpenBSD.
Thomas Munro [Mon, 3 Mar 2025 22:18:29 +0000 (11:18 +1300)]
ci: Use a RAM disk for NetBSD and OpenBSD.

Put the RAM disk setup for all three *BSD CI tasks into a common script,
replacing the old FreeBSD-specific one from commit 0265e5c1.  This makes
them run 3 times and a bit over 2 times faster, respectively.

NetBSD and FreeBSD now share the same one-liner to mount tmpfs.  OpenBSD
needs a GCP-image specific recipe that knows where to steal an unused
disk partition needed to reserve swap space for an mfs RAM disk, because
its tmpfs is deprecated and currently broken.  The configured size is
enough for our current tests but could potentially need future
expansion.  Thanks to Bilal for the disklabel incantation.

Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKGJJ-XrPhN%2BQA4ZUfYAAXcwOSDty9t0vE9Z8__AdacKnQg%40mail.gmail.com

2 months agoTrigger more frequent autovacuums with relallfrozen
Melanie Plageman [Mon, 3 Mar 2025 19:42:00 +0000 (14:42 -0500)]
Trigger more frequent autovacuums with relallfrozen

Calculate the insert threshold for triggering an autovacuum of a
relation based on the number of unfrozen pages.

By only considering the unfrozen portion of the table when calculating
how many tuples to add to the insert threshold, we can trigger more
frequent vacuums of insert-heavy tables. This increases the chances of
vacuuming those pages when they still reside in shared buffers

This also increases the number of autovacuums triggered by tuples
inserted and not by wraparound risk. We prefer to freeze these pages
during insert-triggered autovacuums, as anti-wraparound vacuums are not
automatically canceled by conflicting lock requests.

We calculate the unfrozen percentage of the table using the recently
added (99f8f3fbbc8f) relallfrozen column of pg_class.

Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Greg Sabino Mullane <htamfids@gmail.com>
Reviewed-by: Robert Treat <rob@xzilla.net>
Reviewed-by: wenhui qiu <qiuwenhuifx@gmail.com>
Discussion: https://postgr.es/m/flat/CAAKRu_aj-P7YyBz_cPNwztz6ohP%2BvWis%3Diz3YcomkB3NpYA--w%40mail.gmail.com

2 months agoSimplify some logic around setting pg_attribute.atthasdef.
Tom Lane [Mon, 3 Mar 2025 18:35:48 +0000 (13:35 -0500)]
Simplify some logic around setting pg_attribute.atthasdef.

DefineRelation was of the opinion that it could usefully pre-fill
atthasdef flags to eliminate work for StoreAttrDefault.  This is not
the case, however: the tupledesc that it's filling is not the one that
InsertPgAttributeTuples will work from.  The tupledesc used there is
made by RelationBuildLocalRelation, which deliberately doesn't copy
atthasdef.  Moreover, if this did happen as the code thinks, it would
be wrong for the case of plain "DEFAULT NULL" clauses, since we detect
and ignore simple-null-Const defaults later on.  Hence, remove the
useless code.

It also emerges that it's not really worth a special-case path in
StoreAttrDefault() for atthasdef already being set, because as far as
we can see that never happens: cases where an existing default gets
updated always do RemoveAttrDefault first, so as to clean up
possibly-no-longer-correct dependency entries.  If it were the case
the code would still work, anyway.

Also remove a nearby comment made moot by 5eaa0e92e.

Author: jian he <jian.universality@gmail.com>
Discussion: https://postgr.es/m/CACJufxHFssPvkP1we7WMhPD_1kwgbG52o=kQgL+TnVoX5LOyCQ@mail.gmail.com

2 months agoRemove now-dead code in StoreAttrDefault().
Tom Lane [Mon, 3 Mar 2025 18:09:20 +0000 (13:09 -0500)]
Remove now-dead code in StoreAttrDefault().

StoreAttrDefault() is no longer responsible for filling
attmissingval, so remove the code for that.

Get rid of RawColumnDefault.missingMode, too, as we no longer
need that to pass information around.

While here, clean up some sloppy coding in StoreAttrDefault(),
such as failure to use XXXGetDatum macros.  These aren't bugs
but they're not good code either.

Reported-by: jian he <jian.universality@gmail.com>
Author: jian he <jian.universality@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CACJufxHFssPvkP1we7WMhPD_1kwgbG52o=kQgL+TnVoX5LOyCQ@mail.gmail.com

2 months agoFix broken handling of domains in atthasmissing logic.
Tom Lane [Mon, 3 Mar 2025 17:43:29 +0000 (12:43 -0500)]
Fix broken handling of domains in atthasmissing logic.

If a domain type has a default, adding a column of that type (without
any explicit DEFAULT clause) failed to install the domain's default
value in existing rows, instead leaving the new column null.  This
is unexpected, and it used to work correctly before v11.  The cause
is confusion in the atthasmissing mechanism about which default value
to install: we'd only consider installing an explicitly-specified
default, and then we'd decide that no table rewrite is needed.

To fix, take the responsibility for filling attmissingval out of
StoreAttrDefault, and instead put it into ATExecAddColumn's existing
logic that derives the correct value to fill the new column with.
Also, centralize the logic that determines the need for
default-related table rewriting there, instead of spreading it over
four or five places.

In the back branches, we'll leave the attmissingval-filling code
in StoreAttrDefault even though it's now dead, for fear that some
extension may be depending on that functionality to exist there.
A separate HEAD-only patch will clean up the now-useless code.

Reported-by: jian he <jian.universality@gmail.com>
Author: jian he <jian.universality@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CACJufxHFssPvkP1we7WMhPD_1kwgbG52o=kQgL+TnVoX5LOyCQ@mail.gmail.com
Backpatch-through: 13

2 months agoAdd relallfrozen to pg_class
Melanie Plageman [Mon, 3 Mar 2025 16:18:05 +0000 (11:18 -0500)]
Add relallfrozen to pg_class

Add relallfrozen, an estimate of the number of pages marked all-frozen
in the visibility map.

pg_class already has relallvisible, an estimate of the number of pages
in the relation marked all-visible in the visibility map. This is used
primarily for planning.

relallfrozen, together with relallvisible, is useful for estimating the
outstanding number of all-visible but not all-frozen pages in the
relation for the purposes of scheduling manual VACUUMs and tuning vacuum
freeze parameters.

A future commit will use relallfrozen to trigger more frequent vacuums
on insert-focused workloads with significant volume of frozen data.

Bump catalog version

Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Robert Treat <rob@xzilla.net>
Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Reviewed-by: Greg Sabino Mullane <htamfids@gmail.com>
Discussion: https://postgr.es/m/flat/CAAKRu_aj-P7YyBz_cPNwztz6ohP%2BvWis%3Diz3YcomkB3NpYA--w%40mail.gmail.com

2 months agoAllow parallel CREATE INDEX for GIN indexes
Tomas Vondra [Mon, 3 Mar 2025 15:53:03 +0000 (16:53 +0100)]
Allow parallel CREATE INDEX for GIN indexes

Allow using parallel workers to build a GIN index, similarly to BTREE
and BRIN. For large tables this may result in significant speedup when
the build is CPU-bound.

The work is divided so that each worker builds index entries on a subset
of the table, determined by the regular parallel scan used to read the
data. Each worker uses a local tuplesort to sort and merge the entries
for the same key. The TID lists do not overlap (for a given key), which
means the merge sort simply concatenates the two lists. The merged
entries are written into a shared tuplesort for the leader.

The leader needs to merge the sorted entries again, before writing them
into the index. But this way a significant part of the work happens in
the workers, and the leader is left with merging fewer large entries,
which is more efficient.

Most of the parallelism infrastructure is a simplified copy of the code
used by BTREE indexes, omitting the parts irrelevant for GIN indexes
(e.g. uniqueness checks).

Original patch by me, with reviews and substantial improvements by
Matthias van de Meent, certainly enough to make him a co-author.

Author: Tomas Vondra, Matthias van de Meent
Reviewed-by: Matthias van de Meent, Andy Fan, Kirill Reshke
Discussion: https://postgr.es/m/6ab4003f-a8b8-4d75-a67f-f25ad98582dc%40enterprisedb.com