postgresql.git
2 months agoImplement Self-Join Elimination
Alexander Korotkov [Wed, 12 Feb 2025 22:56:03 +0000 (00:56 +0200)]
Implement Self-Join Elimination

The Self-Join Elimination (SJE) feature removes an inner join of a plain
table to itself in the query tree if it is proven that the join can be
replaced with a scan without impacting the query result.  Self-join and
inner relation get replaced with the outer in query, equivalence classes,
and planner info structures.  Also, the inner restrictlist moves to the
outer one with the removal of duplicated clauses.  Thus, this optimization
reduces the length of the range table list (this especially makes sense for
partitioned relations), reduces the number of restriction clauses and,
in turn, selectivity estimations, and potentially improves total planner
prediction for the query.

This feature is dedicated to avoiding redundancy, which can appear after
pull-up transformations or the creation of an EquivalenceClass-derived clause
like the below.

  SELECT * FROM t1 WHERE x IN (SELECT t3.x FROM t1 t3);
  SELECT * FROM t1 WHERE EXISTS (SELECT t3.x FROM t1 t3 WHERE t3.x = t1.x);
  SELECT * FROM t1,t2, t1 t3 WHERE t1.x = t2.x AND t2.x = t3.x;

In the future, we could also reduce redundancy caused by subquery pull-up
after unnecessary outer join removal in cases like the one below.

  SELECT * FROM t1 WHERE x IN
    (SELECT t3.x FROM t1 t3 LEFT JOIN t2 ON t2.x = t1.x);

Also, it can drastically help to join partitioned tables, removing entries
even before their expansion.

The SJE proof is based on innerrel_is_unique() machinery.

We can remove a self-join when for each outer row:

 1. At most, one inner row matches the join clause;
 2. Each matched inner row must be (physically) the same as the outer one;
 3. Inner and outer rows have the same row mark.

In this patch, we use the next approach to identify a self-join:

 1. Collect all merge-joinable join quals which look like a.x = b.x;
 2. Add to the list above the baseretrictinfo of the inner table;
 3. Check innerrel_is_unique() for the qual list.  If it returns false, skip
    this pair of joining tables;
 4. Check uniqueness, proved by the baserestrictinfo clauses. To prove the
    possibility of self-join elimination, the inner and outer clauses must
    match exactly.

The relation replacement procedure is not trivial and is partly combined
with the one used to remove useless left joins.  Tests covering this feature
were added to join.sql.  Some of the existing regression tests changed due
to self-join removal logic.

Discussion: https://postgr.es/m/flat/64486b0b-0404-e39e-322d-0801154901f3%40postgrespro.ru
Author: Andrey Lepikhov <a.lepikhov@postgrespro.ru>
Author: Alexander Kuzmenkov <a.kuzmenkov@postgrespro.ru>
Co-authored-by: Alexander Korotkov <aekorotkov@gmail.com>
Co-authored-by: Alena Rybakina <lena.ribackina@yandex.ru>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Simon Riggs <simon@2ndquadrant.com>
Reviewed-by: Jonathan S. Katz <jkatz@postgresql.org>
Reviewed-by: David Rowley <david.rowley@2ndquadrant.com>
Reviewed-by: Thomas Munro <thomas.munro@enterprisedb.com>
Reviewed-by: Konstantin Knizhnik <k.knizhnik@postgrespro.ru>
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Hywel Carver <hywel@skillerwhale.com>
Reviewed-by: Laurenz Albe <laurenz.albe@cybertec.at>
Reviewed-by: Ronan Dunklau <ronan.dunklau@aiven.io>
Reviewed-by: vignesh C <vignesh21@gmail.com>
Reviewed-by: Zhihong Yu <zyu@yugabyte.com>
Reviewed-by: Greg Stark <stark@mit.edu>
Reviewed-by: Jaime Casanova <jcasanov@systemguards.com.ec>
Reviewed-by: Michał Kłeczek <michal@kleczek.org>
Reviewed-by: Alena Rybakina <lena.ribackina@yandex.ru>
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
2 months agoRevert: Get rid of WALBufMappingLock
Alexander Korotkov [Mon, 17 Feb 2025 10:35:28 +0000 (12:35 +0200)]
Revert: Get rid of WALBufMappingLock

This commit reverts 6a2275b895.  Buildfarm failure on batta spots some
concurrency issue, which requires further investigation.

2 months agoFix an oversight in cbc127917 to handle MERGE correctly
Amit Langote [Mon, 17 Feb 2025 07:12:03 +0000 (16:12 +0900)]
Fix an oversight in cbc127917 to handle MERGE correctly

ExecInitModifyTable() forgot to trim MERGE-related lists to exclude
entries for result relations pruned during initial pruning, so fix
that.

While at it, make the function's use of the pruned resultRelations
list, rather than ModifyTable.resultRelations, more consistent.

Reported-by: Alexander Lakhin <exclusion@gmail.com> (via sqlsmith)
Reviewed-by: Junwang Zhao <zhjwpku@gmail.com>
Discussion: https://postgr.es/m/e72c94d9-e5f9-4753-9bc1-69d72bd54b8a@gmail.com

2 months agoAdd information about WAL buffers full to VACUUM/ANALYZE (VERBOSE)
Michael Paquier [Mon, 17 Feb 2025 06:09:51 +0000 (15:09 +0900)]
Add information about WAL buffers full to VACUUM/ANALYZE (VERBOSE)

This commit adds the information about the number of times WAL buffers
have been full to the logs generated by VACUUM/ANALYZE (VERBOSE) and in
the logs generated by autovacuum, complementing the existing information
stored by WalUsage.

This is the last part of the backend code where the value of
wal_buffers_full can be reported, similarly to all the other fields of
WalUsage.  320545bfcfee and ce5bcc4a9f26 have done the same for EXPLAIN
and pgss.

Author: Bertrand Drouvot
Reviewed-by: Ilia Evdokimov
Discussion: https://postgr.es/m/Z6SOha5YFFgvpwQY@ip-10-97-1-34.eu-west-3.compute.internal

2 months agoAdd information about WAL buffers being full to EXPLAIN (WAL)
Michael Paquier [Mon, 17 Feb 2025 05:50:33 +0000 (14:50 +0900)]
Add information about WAL buffers being full to EXPLAIN (WAL)

This is similar to ce5bcc4a9f26, relying on the addition of
wal_buffers_full to WalUsage.  This time, the information is added to
the output generated by EXPLAIN (WAL).

Author: Bertrand Drouvot
Reviewed-by: Ilia Evdokimov
Discussion: https://postgr.es/m/Z6SOha5YFFgvpwQY@ip-10-97-1-34.eu-west-3.compute.internal

2 months agopg_stat_statements: Add wal_buffers_full
Michael Paquier [Mon, 17 Feb 2025 04:55:17 +0000 (13:55 +0900)]
pg_stat_statements: Add wal_buffers_full

wal_buffers_full tracks the number of times WAL buffers become full,
giving hints to be able to tune the GUC wal_buffers.

Up to now, this information was only available in pg_stat_wal.  With
this field available in WalUsage since eaf502747bac, exposing it in
pg_stat_statements is straight-forward, and it offers more granularity
at query level.

pg_stat_statements does not need a version bump as one has been done in
commit cf54a2c00254 for this development cycle.

Author: Bertrand Drouvot
Reviewed-by: Ilia Evdokimov
Discussion: https://postgr.es/m/Z6SOha5YFFgvpwQY@ip-10-97-1-34.eu-west-3.compute.internal

2 months agoMove wal_buffers_full from PgStat_PendingWalStats to WalUsage
Michael Paquier [Mon, 17 Feb 2025 04:14:28 +0000 (13:14 +0900)]
Move wal_buffers_full from PgStat_PendingWalStats to WalUsage

wal_buffers_full has been introduced in pg_stat_wal in 8d9a935965f, as
some information providing metrics for the tuning of the GUC
wal_buffers.  WalUsage has been introduced before that in df3b181499.

Moving this field is proving to be beneficial for several reasons:
- This information can now be made available in more layers, providing
more granularity than just pg_stat_wal, on a per-query basis: EXPLAIN,
pgss and VACUUM/ANALYZE logs.
- A patch is under discussion to provide statistics for WAL at backend
level, and this move simplifies a bit the handling of pending
statistics.  The remaining data in PgStat_PendingWalStats now relates to
write/sync counters and times, with equivalents present in pg_stat_io,
that backend statistics are able to already track.  So this should cut
all the dependencies between PgStat_PendingWalStats and WAL stats at
backend level.

As of this change, wal_buffers_full only shows in pg_stat_wal.

Author: Bertrand Drouvot
Reviewed-by: Ilia Evdokimov
Discussion: https://postgr.es/m/Z6SOha5YFFgvpwQY@ip-10-97-1-34.eu-west-3.compute.internal

2 months agoGet rid of WALBufMappingLock
Alexander Korotkov [Mon, 17 Feb 2025 02:19:01 +0000 (04:19 +0200)]
Get rid of WALBufMappingLock

Allow multiple backends to initialize WAL buffers concurrently.  This way
`MemSet((char *) NewPage, 0, XLOG_BLCKSZ);` can run in parallel without
taking a single LWLock in exclusive mode.

The new algorithm works as follows:
 * reserve a page for initialization using XLogCtl->InitializeReserved,
 * ensure the page is written out,
 * once the page is initialized, try to advance XLogCtl->InitializedUpTo and
   signal to waiters using XLogCtl->InitializedUpToCondVar condition
   variable,
 * repeat previous steps until we reserve initialization up to the target
   WAL position,
 * wait until concurrent initialization finishes using a
   XLogCtl->InitializedUpToCondVar.

Now, multiple backends can, in parallel, concurrently reserve pages,
initialize them, and advance XLogCtl->InitializedUpTo to point to the latest
initialized page.

Author: Yura Sokolov <y.sokolov@postgrespro.ru>
Co-authored-by: Alexander Korotkov <aekorotkov@gmail.com>
Reviewed-by: Pavel Borisov <pashkin.elfe@gmail.com>
2 months agoAdjust tuples estimate for appendrels
Richard Guo [Mon, 17 Feb 2025 02:13:15 +0000 (11:13 +0900)]
Adjust tuples estimate for appendrels

In set_append_rel_size(), we currently set rel->tuples to rel->rows
for an appendrel.  Generally, rel->tuples is the raw number of tuples
in the relation and rel->rows is the estimated number of tuples after
the relation's restriction clauses have been applied.  Although an
appendrel itself doesn't directly enforce any quals today, its child
relations may.  Therefore, setting rel->tuples equal to rel->rows for
an appendrel isn't always appropriate.

Doing so can lead to issues in cost estimates in some cases.  For
instance, when estimating the number of distinct values from an
appendrel, we would not be able to adjust the estimate based on the
restriction selectivity.

This patch addresses this by setting an appendrel's tuples to the
total number of tuples accumulated from each live child, which better
aligns with reality.

This is arguably a bug, but nobody has complained about that until
now, so no back-patch.

Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Reviewed-by: Alena Rybakina <a.rybakina@postgrespro.ru>
Discussion: https://postgr.es/m/CAMbWs4_TG_+kVn6fjG-5GYzzukrNK57=g9eUo4gsrUG26OFawg@mail.gmail.com

2 months agoIn fmtIdEnc(), handle failure of enlargePQExpBuffer().
Tom Lane [Sun, 16 Feb 2025 17:46:35 +0000 (12:46 -0500)]
In fmtIdEnc(), handle failure of enlargePQExpBuffer().

Coverity complained that we weren't doing that, and it's right.

This fix just makes fmtIdEnc() honor the general convention that OOM
causes a PQExpBuffer to become marked "broken", without any immediate
error.  In the pretty-unlikely case that we actually did hit OOM here,
the end result would be to return an empty string to the caller,
probably resulting in invalid SQL syntax in an issued command (if
nothing else went wrong, which is even more unlikely).  It's tempting
to throw an "out of memory" error if the buffer becomes broken, but
there's not a lot of point in doing that only here and not in hundreds
of other PQExpBuffer-using places in pg_dump and similar callers.
The whole issue could do with some non-time-crunched redesign, perhaps.

This is a followup to the fixes for CVE-2025-1094, and should be
included if cherry-picking those fixes.

2 months agoMake escaping functions retain trailing bytes of an invalid character.
Tom Lane [Sat, 15 Feb 2025 21:20:21 +0000 (16:20 -0500)]
Make escaping functions retain trailing bytes of an invalid character.

Instead of dropping the trailing byte(s) of an invalid or incomplete
multibyte character, replace only the first byte with a known-invalid
sequence, and process the rest normally.  This seems less likely to
confuse incautious callers than the behavior adopted in 5dc1e42b4.

While we're at it, adjust PQescapeStringInternal to produce at most
one bleat about invalid multibyte characters per string.  This
matches the behavior of PQescapeInternal, and avoids the risk of
producing tons of repetitive junk if a long string is simply given
in the wrong encoding.

This is a followup to the fixes for CVE-2025-1094, and should be
included if cherry-picking those fixes.

Author: Andres Freund <andres@anarazel.de>
Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us>
Reported-by: Jeff Davis <pgsql@j-davis.com>
Discussion: https://postgr.es/m/20250215012712.45@rfd.leadboat.com
Backpatch-through: 13

2 months agoFix explicit valgrind interaction in read_stream.c.
Thomas Munro [Sat, 15 Feb 2025 00:13:19 +0000 (13:13 +1300)]
Fix explicit valgrind interaction in read_stream.c.

By calling wipe_mem() on per-buffer data memory that has been released,
we are also telling Valgrind that the memory is "noaccess".  We need to
set it to "undefined" before giving it to the registered callback to
fill in, when a slot is reused.

As discovered by build farm animal skink when the VACUUM streamification
patches landed (the first users of per-buffer data).

Pushing to master only for now, to clear the error on skink.  It's also
possible that external code might discover the per-buffer data feature
in v17, and reasonable to expect Valgrind not to produce spurious
memcheck reports, but the back-patch is deferred until after the
imminent minor release is out of the way.

Reviewed-by: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Tested-by: Melanie Plageman <melanieplageman@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKG%2Bg6aXpi2FEHqeLOzE%2BxYw%3DOV%2B-N5jhOEnnV%2BF0USM9xA%40mail.gmail.com

2 months agoFix PQescapeLiteral()/PQescapeIdentifier() length handling
Andres Freund [Fri, 14 Feb 2025 22:44:28 +0000 (17:44 -0500)]
Fix PQescapeLiteral()/PQescapeIdentifier() length handling

In 5dc1e42b4fa I fixed bugs in various escape functions, unfortunately as part
of that I introduced a new bug in PQescapeLiteral()/PQescapeIdentifier(). The
bug is that I made PQescapeInternal() just use strlen(), rather than taking
the specified input length into account.

That's bad, because it can lead to including input that wasn't intended to be
included (in case len is shorter than null termination of the string) and
because it can lead to reading invalid memory if the input string is not null
terminated.

Expand test_escape to this kind of bug:

a) for escape functions with length support, append data that should not be
   escaped and check that it is not

b) add valgrind requests to detect access of bytes that should not be touched

Author: Tom Lane <tgl@sss.pgh.pa.us>
Author: Andres Freund <andres@anarazel.de
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/Z64jD3u46gObCo1p@pryzbyj2023
Backpatch: 13

2 months agoAdd delay time to VACUUM/ANALYZE (VERBOSE) and autovacuum logs.
Nathan Bossart [Fri, 14 Feb 2025 20:53:28 +0000 (14:53 -0600)]
Add delay time to VACUUM/ANALYZE (VERBOSE) and autovacuum logs.

Commit bb8dff9995 added this information to the
pg_stat_progress_vacuum and pg_stat_progress_analyze system views.
This commit adds the same information to the output of VACUUM and
ANALYZE with the VERBOSE option and to the autovacuum logs.

Suggested-by: Masahiro Ikeda <ikedamsh@oss.nttdata.com>
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/ZmaXmWDL829fzAVX%40ip-10-97-1-34.eu-west-3.compute.internal

2 months agopgcrypto: Add support for CFB mode in AES encryption
Daniel Gustafsson [Fri, 14 Feb 2025 20:18:37 +0000 (21:18 +0100)]
pgcrypto: Add support for CFB mode in AES encryption

Cipher Feedback Mode, CFB, is a self-synchronizing stream cipher which
is very similar to CBC performed in reverse. Since OpenSSL supports it,
we can easily plug it into the existing cipher selection code without
any need for infrastructure changes.

This patch was simultaneously submitted by Umar Hayat and Vladyslav
Nebozhyn, the latter whom suggested the feauture. The committed patch
is Umar's version.

Author: Umar Hayat <postgresql.wizard@gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/CAPBGcbxo9ASzq14VTpQp3mnUJ5omdgTWUJOvWV0L6nNigWE5jw@mail.gmail.com

2 months agoUse PqMsg_Progress macro in HandleParallelMessage().
Nathan Bossart [Fri, 14 Feb 2025 18:57:13 +0000 (12:57 -0600)]
Use PqMsg_Progress macro in HandleParallelMessage().

Commit a99cc6c6b4 introduced the PqMsg_Progress macro but missed
updating HandleParallelMessage() accordingly.

Backpatch-through: 17

2 months agoUse streaming read I/O in VACUUM's third phase
Melanie Plageman [Fri, 14 Feb 2025 17:57:03 +0000 (12:57 -0500)]
Use streaming read I/O in VACUUM's third phase

Make vacuum's third phase (its second pass over the heap), which reaps
dead items collected in the first phase and marks them as reusable, use
the read stream API. This commit adds a new read stream callback,
vacuum_reap_lp_read_stream_next(), that looks ahead in the TidStore and
returns the next block number to read for vacuum.

Author: Melanie Plageman <melanieplageman@gmail.com>
Co-authored-by: Thomas Munro <thomas.munro@gmail.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKGKN3oy0bN_3yv8hd78a4%2BM1tJC9z7mD8%2Bf%2ByA%2BGeoFUwQ%40mail.gmail.com

2 months agoUse streaming read I/O in VACUUM's first phase
Melanie Plageman [Fri, 14 Feb 2025 17:56:57 +0000 (12:56 -0500)]
Use streaming read I/O in VACUUM's first phase

Make vacuum's first phase, which prunes and freezes tuples and records
dead TIDs, use the read stream API by by converting
heap_vac_scan_next_block() to a read stream callback.

Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/CAAKRu_aLwANZpxHc0tC-6OT0OQT4TftDGkKAO5yigMUOv_Tcsw%40mail.gmail.com

2 months agoConvert heap_vac_scan_next_block() boolean parameters to flags
Melanie Plageman [Fri, 14 Feb 2025 17:56:50 +0000 (12:56 -0500)]
Convert heap_vac_scan_next_block() boolean parameters to flags

The read stream API only allows one piece of extra per block state to be
passed back to the API user (per_buffer_data). lazy_scan_heap() needs
two pieces of per-buffer data: whether or not the block was all-visible
in the visibility map and whether or not it was eagerly scanned.

Convert these two pieces of information to flags so that they can be
populated by heap_vac_scan_next_block() and returned to
lazy_scan_heap(). A future commit will turn heap_vac_scan_next_block()
into the read stream callback for heap phase I vacuuming.

Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/CAAKRu_bmx33jTqATP5GKNFYwAg02a9dDtk4U_ciEjgBHZSVkOQ%40mail.gmail.com

2 months agoDescribe special values in GUC descriptions more consistently.
Nathan Bossart [Fri, 14 Feb 2025 16:44:30 +0000 (10:44 -0600)]
Describe special values in GUC descriptions more consistently.

Many GUCs accept special values like -1 or an empty string to
disable the feature, use a system default, etc.  While the
documentation consistently lists these special values, the GUC
descriptions do not.  Many such descriptions fail to mention the
special values, and those that do vary in phrasing and placement.
This commit aims to bring some consistency to this area by applying
the following rules:

* Special values should be listed at the end of the long
  description.
* Descriptions should use numerals (e.g., "0") instead of words
  (e.g., "zero").
* Special value mentions should be concise and direct (e.g., "0
  disables the timeout.", "An empty string means use the operating
  system setting.").
* Multiple special values should be listed in ascending order.

Of course, there are exceptions, such as
max_pred_locks_per_relation and search_path, whose special values
are too complex to include.  And there are cases like
listen_addresses, where the meaning of an empty string is arguably
too obvious to include.  In those cases, I've refrained from adding
special value information to the GUC description.

Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Reviewed-by: "David G. Johnston" <david.g.johnston@gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/Z6aIy4aywxUZHAo6%40nathan

2 months agoFix assertion on dereferenced object
Daniel Gustafsson [Fri, 14 Feb 2025 10:50:56 +0000 (11:50 +0100)]
Fix assertion on dereferenced object

Commit 27cc7cd2bc8a accidentally placed the assertion ensuring
that the pointer isn't NULL after it had already been accessed.
Fix by moving the pointer dereferencing to after the assertion.
Backpatch to all supported branches.

Author: Dmitry Koval <d.koval@postgrespro.ru>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/1618848d-cdc7-414b-9c03-08cf4bef4408@postgrespro.ru
Backpatch-through: 13

2 months agoRemove obsolete comment.
Thomas Munro [Fri, 14 Feb 2025 00:16:05 +0000 (13:16 +1300)]
Remove obsolete comment.

Commit 755a4c10d19d prevented StartReadBuffers() from crossing md.c
segment boundaries in one operation, but a comment about that
possibility remained.

2 months agoRemove unused parameter from execute_extension_script().
Nathan Bossart [Thu, 13 Feb 2025 22:47:42 +0000 (16:47 -0600)]
Remove unused parameter from execute_extension_script().

This function's schemaOid parameter appears to have never been used
for anything.

Author: Yugo Nagata <nagata@sraoss.co.jp>
Reviewed-by: Fabrízio de Royes Mello <fabriziomello@gmail.com>
Discussion: https://postgr.es/m/20250214010218.550ebe4ec1a7c7811a7fa2bb%40sraoss.co.jp

2 months agoRemove unnecessary (char *) casts [xlog]
Peter Eisentraut [Tue, 11 Feb 2025 21:03:29 +0000 (22:03 +0100)]
Remove unnecessary (char *) casts [xlog]

Remove (char *) casts no longer needed after XLogRegisterData() and
XLogRegisterBufData() argument type change.

Reviewed-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Discussion: https://www.postgresql.org/message-id/flat/fd1fcedb-3492-4fc8-9e3e-74b97f2db6c7%40eisentraut.org

2 months agoXLogRegisterData, XLogRegisterBufData void * argument for binary data
Peter Eisentraut [Thu, 13 Feb 2025 09:33:14 +0000 (10:33 +0100)]
XLogRegisterData, XLogRegisterBufData void * argument for binary data

Change XLogRegisterData() and XLogRegisterBufData() functions to take
void * for binary data instead of char *.  This will remove the need
for numerous casts (done in a separate commit for clarity).

Reviewed-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Discussion: https://www.postgresql.org/message-id/flat/fd1fcedb-3492-4fc8-9e3e-74b97f2db6c7%40eisentraut.org

2 months agoFix MakeTransitionCaptureState() to return a consistent result
Michael Paquier [Thu, 13 Feb 2025 07:30:58 +0000 (16:30 +0900)]
Fix MakeTransitionCaptureState() to return a consistent result

When an UPDATE trigger referencing a new table and a DELETE trigger
referencing an old table are both present, MakeTransitionCaptureState()
returns an inconsistent result for UPDATE commands in its set of flags
and tuplestores holding the TransitionCaptureState for transition
tables.

As proved by the test added here, this issue causes a crash in v14 and
earlier versions (down to 11, actually, older versions do not support
triggers on partitioned tables) during cross-partition updates on a
partitioned table.  v15 and newer versions are safe thanks to
7103ebb7aae8.

This commit fixes the function so that it returns a consistent state
by using portions of the changes made in commit 7103ebb7aae8 for v13 and
v14.  v15 and newer versions are slightly tweaked to match with the
older versions, mainly for consistency across branches.

Author: Kyotaro Horiguchi
Discussion: https://postgr.es/m/20250207.150238.968446820828052276.horikyota.ntt@gmail.com
Backpatch-through: 13

2 months agoRename RBTXN_PREPARE to RBTXN_IS_PREPARE for better clarification.
Masahiko Sawada [Thu, 13 Feb 2025 00:55:00 +0000 (16:55 -0800)]
Rename RBTXN_PREPARE to RBTXN_IS_PREPARE for better clarification.

RBTXN_PREPARE flag and rbtxn_prepared macro could be misinterpreted as
either indicating the transaction type (e.g. a prepared transaction or
a normal transaction) or its currentstate (e.g. skipped or its prepare
message is sent), especially after commit 072ee847ad4 introduced the
RBTXN_SENT_PREPARE flag and the rbtxn_sent_prepare macro.

The RBTXN_PREPARE flag (and its corresponding macro) have been renamed
to RBTXN_IS_PREPARE to explicitly indicate the transaction
type. Therefore, this commit also adds the RBTXN_IS_PREPARE flag to
the transaction that is a prepared transaction and has been skipped,
which previously had only the RBTXN_SKIPPED_PREPARE flag.

Reviewed-by: Amit Kapila, Peter Smith
Discussion: https://postgr.es/m/CAA4eK1KgNmBsG%3D155E7QQ6TX9RoWnM4z5Z20SvsbwxSe_QXYsg%40mail.gmail.com

2 months agoSkip logical decoding of already-aborted transactions.
Masahiko Sawada [Thu, 13 Feb 2025 00:31:34 +0000 (16:31 -0800)]
Skip logical decoding of already-aborted transactions.

Previously, transaction aborts were detected concurrently only during
system catalog scans while replaying a transaction in streaming mode.

This commit adds an additional CLOG lookup to check the transaction
status, allowing the logical decoding to skip changes also when it
doesn't touch system catalogs, if the transaction is already
aborted. This optimization enhances logical decoding performance,
especially for large transactions that have already been rolled back,
as it avoids unnecessary disk or network I/O.

To avoid potential slowdowns caused by frequent CLOG lookups for small
transactions (most of which commit), the CLOG lookup is performed only
for large transactions before eviction. The performance benchmark
results showed there is not noticeable performance regression due to
CLOG lookups.

Reviewed-by: Amit Kapila, Peter Smith, Vignesh C, Ajin Cherian
Reviewed-by: Dilip Kumar, Andres Freund
Discussion: https://postgr.es/m/CAD21AoDht9Pz_DFv_R2LqBTBbO4eGrpa9Vojmt5z5sEx3XwD7A@mail.gmail.com

2 months agoRemove unneeded volatile qualifier in fmgr.c.
Nathan Bossart [Wed, 12 Feb 2025 21:45:40 +0000 (15:45 -0600)]
Remove unneeded volatile qualifier in fmgr.c.

Currently, the save_nestlevel variable in fmgr_security_definer()
is marked volatile.  While this may have been necessary when it was
used in a PG_CATCH section (as explained in the comment for PG_TRY
in elog.h), it appears to have been unnecessary since commit
82a47982f3, which removed its use in a PG_CATCH section.

Author: Julien Rouhaud <rjuju123@gmail.com>
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/Z6xbAgXKY2L-3d5Q%40jrouhaud

2 months agoClean up impenetrable logic in pg_basebackup/receivelog.c.
Tom Lane [Wed, 12 Feb 2025 21:07:23 +0000 (16:07 -0500)]
Clean up impenetrable logic in pg_basebackup/receivelog.c.

Coverity complained about possible double free of HandleCopyStream's
"copybuf".  AFAICS it's mistaken, but it is easy to see why it's
confused, because management of that buffer is impossibly confusing.
It's unreasonable that HandleEndOfCopyStream frees the buffer in some
cases but not others, updates the caller's state for that in no case,
and has not a single comment about how complicated that makes things.

Let's put all the responsibility for freeing copybuf in the actual
owner of that variable, HandleCopyStream.  This results in one more
PQfreemem call than before, but the logic is far easier to follow,
both for humans and machines.

Since this isn't (quite) actually broken, no back-patch.

2 months agoFix minor memory leaks in pg_dump.
Tom Lane [Wed, 12 Feb 2025 20:46:31 +0000 (15:46 -0500)]
Fix minor memory leaks in pg_dump.

Coverity reported the two oversights in getPublicationTables.
Valgrind found the one in determineNotNullFlags.

The mistakes in getPublicationTables seem too minor to be worth
back-patching.  determineNotNullFlags could be run enough times
to matter, but that code is new in v18.  So, no back-patch.

2 months agoci: Collect core files on NetBSD and OpenBSD
Andres Freund [Wed, 12 Feb 2025 14:40:20 +0000 (09:40 -0500)]
ci: Collect core files on NetBSD and OpenBSD

Support for NetBSD and OpenBSD operating systems have been added to CI in the
prior commit. Now add support for collect core files and generating backtraces
using for all core files.

Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/CAN55FZ32ySyYa06k9MFd+VY5vHhUyBpvgmJUZae5PihjzaurVg@mail.gmail.com

2 months agoci: Test NetBSD and OpenBSD
Andres Freund [Wed, 12 Feb 2025 14:40:07 +0000 (09:40 -0500)]
ci: Test NetBSD and OpenBSD

NetBSD and OpenBSD Postgres CI images are now generated [1], but aren't yet
utilized for Postgres' CI. This commit adds CI support for them.

For now the tasks will be manually triggered, to save on CI credits.

[1] https://github.com/anarazel/pg-vm-images

Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/CAN55FZ32ySyYa06k9MFd+VY5vHhUyBpvgmJUZae5PihjzaurVg@mail.gmail.com

2 months agomeson: Fix failure to detect bsd_auth.h presence
Andres Freund [Wed, 12 Feb 2025 13:15:53 +0000 (08:15 -0500)]
meson: Fix failure to detect bsd_auth.h presence

bsd_auth.h file needs to be included after 'sys/types.h', as documented in
https://man.openbsd.org/authenticate.3

The reason a similar looking stanza works for autoconf is that autoconf
automatically adds AC_INCLUDES_DEFAULT, which in turn includes sys/types.h.

Backpatch to all versions with meson support.

Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/637haqqyhg2wlz7q6wq25m2qupe67g7f2uupngzui64zypy4x2@ysr2xnmynmu4
Backpatch-through: 16

2 months agoFix issue in recovery test 041_checkpoint_at_promote
Michael Paquier [Wed, 12 Feb 2025 08:58:25 +0000 (17:58 +0900)]
Fix issue in recovery test 041_checkpoint_at_promote

The phase of the test waiting for a restartpoint to complete was not
working as intended, due to a log_contains() call incorrectly
written.

The problem reported by the author could be simply reproduced by
removing the injection_points_wakeup() call: the test succeeds rather
than waiting for the restartpoint completion.  In most cases, the
restartpoint completion is fast enough that the test offered the wanted
coverage.  On slow machines, it could have become unreliable.

Oversight in 6782709df81f.

Author: Nitin Jadhav
Discussion: https://postgr.es/m/CAMm1aWa_6u+o52r7h7G6pX-oWD0Qraf0ee17Ma50qxGS0B_Rzg@mail.gmail.com
Backpatch-through: 17

2 months agoFix some inconsistencies with memory freeing in pg_createsubscriber
Michael Paquier [Wed, 12 Feb 2025 08:11:43 +0000 (17:11 +0900)]
Fix some inconsistencies with memory freeing in pg_createsubscriber

The correct function documented to free the memory allocated for the
result returned by PQescapeIdentifier() and PQescapeLiteral() is
PQfreemem().  pg_createsubscriber.c relied on pg_free() instead, which
is not incorrect as both do a free() internally, but inconsistent with
the documentation.

While on it, this commit fixes a small memory leak introduced by
4867f8a555ce, as the code of pg_createsubscriber makes this effort.

Author: Ranier Vilela
Reviewed-by: Euler Taveira
Discussion: https://postgr.es/m/CAEudQAp=AW5dJXrGLbC_aZg_9nOo=42W7uLDRONFQE-gcgnkgQ@mail.gmail.com
Backpatch-through: 17

2 months agoRemove unnecessary (char *) casts [checksum]
Peter Eisentraut [Wed, 12 Feb 2025 07:59:48 +0000 (08:59 +0100)]
Remove unnecessary (char *) casts [checksum]

Remove some (char *) casts related to uses of the pg_checksum_page()
function.  These casts are useless, because everything involved
already has the right type.  Moreover, these casts actually silently
discarded a const qualifier.  The declaration of a higher-level
function needs to be adjusted to fix that.

Reviewed-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Discussion: https://www.postgresql.org/message-id/flat/fd1fcedb-3492-4fc8-9e3e-74b97f2db6c7%40eisentraut.org

2 months agoRemove unnecessary (char *) casts [mem]
Peter Eisentraut [Wed, 12 Feb 2025 07:50:13 +0000 (08:50 +0100)]
Remove unnecessary (char *) casts [mem]

Remove (char *) casts around memory functions such as memcmp(),
memcpy(), or memset() where the cast is useless.  Since these
functions don't take char * arguments anyway, these casts are at best
complicated casts to (void *), about which see commit 7f798aca1d5.

Reviewed-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Discussion: https://www.postgresql.org/message-id/flat/fd1fcedb-3492-4fc8-9e3e-74b97f2db6c7%40eisentraut.org

2 months agoRemove unnecessary (char *) casts [string]
Peter Eisentraut [Wed, 12 Feb 2025 07:49:18 +0000 (08:49 +0100)]
Remove unnecessary (char *) casts [string]

Remove (char *) casts around string functions where the arguments or
result already have the right type and the cast is useless (or worse,
potentially casts away a qualifier, but this doesn't appear to be the
case here).

Reviewed-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Discussion: https://www.postgresql.org/message-id/flat/fd1fcedb-3492-4fc8-9e3e-74b97f2db6c7%40eisentraut.org

2 months agoDoc: Fix punctuation errors
John Naylor [Wed, 12 Feb 2025 06:12:51 +0000 (13:12 +0700)]
Doc: Fix punctuation errors

Author: 斉藤登 <noborusai@gmail.com>
Reviewed-by: David G. Johnston <david.g.johnston@gmail.com>
Discussion: https://postgr.es/m/CAAM3qnL6i-BSu5rB2+KiHLjMCOXiQEiPMBvEj7F1CgUzZMooLA@mail.gmail.com
Backpatch-through: 13

2 months agoAdd cost-based vacuum delay time to progress views.
Nathan Bossart [Tue, 11 Feb 2025 22:38:14 +0000 (16:38 -0600)]
Add cost-based vacuum delay time to progress views.

This commit adds the amount of time spent sleeping due to
cost-based delay to the pg_stat_progress_vacuum and
pg_stat_progress_analyze system views.  A new configuration
parameter named track_cost_delay_timing, which is off by default,
controls whether this information is gathered.  For vacuum, the
reported value includes the sleep time of any associated parallel
workers.  However, parallel workers only report their sleep time
once per second to avoid overloading the leader process.

Bumps catversion.

Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Co-authored-by: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Sami Imseih <samimseih@gmail.com>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-by: Masahiro Ikeda <ikedamsh@oss.nttdata.com>
Reviewed-by: Dilip Kumar <dilipbalaut@gmail.com>
Reviewed-by: Sergei Kornilov <sk@zsrv.org>
Discussion: https://postgr.es/m/ZmaXmWDL829fzAVX%40ip-10-97-1-34.eu-west-3.compute.internal

2 months agoAdd is_analyze parameter to vacuum_delay_point().
Nathan Bossart [Tue, 11 Feb 2025 22:38:14 +0000 (16:38 -0600)]
Add is_analyze parameter to vacuum_delay_point().

This function is used in both vacuum and analyze code paths, and a
follow-up commit will require distinguishing between the two.  This
commit forces callers to specify whether they are in a vacuum or
analyze path, but it does not use that information for anything
yet.

Author: Nathan Bossart <nathandbossart@gmail.com>
Co-authored-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/ZmaXmWDL829fzAVX%40ip-10-97-1-34.eu-west-3.compute.internal

2 months agoLimit pgbench COPY FREEZE to ordinary relations
Melanie Plageman [Tue, 11 Feb 2025 21:51:28 +0000 (16:51 -0500)]
Limit pgbench COPY FREEZE to ordinary relations

pgbench client-side data generation uses COPY FREEZE to load data for most
tables. COPY FREEZE isn't supported for partitioned tables and since pgbench
only supports partitioning pgbench_accounts, pgbench used a hard-coded check to
skip COPY FREEZE and use plain COPY for a partitioned pgbench_accounts.

If the user has manually partitioned one of the other pgbench tables, this
causes client-side data generation to error out with:

ERROR:  cannot perform COPY FREEZE on a partitioned table

Fix this by limiting COPY FREEZE to ordinary tables (RELKIND_RELATION).

Author: Sergey Tatarintsev <s.tatarintsev@postgrespro.ru>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/flat/97f55fca-8a7b-4da8-b413-7d1c57010676%40postgrespro.ru

2 months agoInjection points for hash aggregation.
Jeff Davis [Tue, 11 Feb 2025 19:26:25 +0000 (11:26 -0800)]
Injection points for hash aggregation.

Requires adding a guard against shift-by-32. Previously, that was
impossible because the number of partitions was always greater than 1,
but a new injection point can force the number of partitions to 1.

Discussion: https://postgr.es/m/ff4e59305e5d689e03cd256a736348d3e7958f8f.camel@j-davis.com

2 months agoEagerly scan all-visible pages to amortize aggressive vacuum
Melanie Plageman [Tue, 11 Feb 2025 18:52:19 +0000 (13:52 -0500)]
Eagerly scan all-visible pages to amortize aggressive vacuum

Aggressive vacuums must scan every unfrozen tuple in order to advance
the relfrozenxid/relminmxid. Because data is often vacuumed before it is
old enough to require freezing, relations may build up a large backlog
of pages that are set all-visible but not all-frozen in the visibility
map. When an aggressive vacuum is triggered, all of these pages must be
scanned. These pages have often been evicted from shared buffers and
even from the kernel buffer cache. Thus, aggressive vacuums often incur
large amounts of extra I/O at the expense of foreground workloads.

To amortize the cost of aggressive vacuums, eagerly scan some
all-visible but not all-frozen pages during normal vacuums.

All-visible pages that are eagerly scanned and set all-frozen in the
visibility map are counted as successful eager freezes and those not
frozen are counted as failed eager freezes.

If too many eager scans fail in a row, eager scanning is temporarily
suspended until a later portion of the relation. The number of failures
tolerated is configurable globally and per table.

To effectively amortize aggressive vacuums, we cap the number of
successes as well. Capping eager freeze successes also limits the amount
of potentially wasted work if these pages are modified again before the
next aggressive vacuum. Once we reach the maximum number of blocks
successfully eager frozen, eager scanning is disabled for the remainder
of the vacuum of the relation.

Original design idea from Robert Haas, with enhancements from
Andres Freund, Tomas Vondra, and me

Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Robert Treat <rob@xzilla.net>
Reviewed-by: Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/flat/CAAKRu_ZF_KCzZuOrPrOqjGVe8iRVWEAJSpzMgRQs%3D5-v84cXUg%40mail.gmail.com

2 months agoconfig: Rename "Asynchronous Behavior" to "I/O"
Andres Freund [Tue, 11 Feb 2025 16:39:20 +0000 (11:39 -0500)]
config: Rename "Asynchronous Behavior" to "I/O"

"I/O" seems more descriptive than "Asynchronous Behavior", given that some of
the GUCs in the section don't relate to anything asynchronous.

Most other abbreviations in the config sections are un-abbreviated, but
"Input/Output" seems less likely to be helpful than just IO or I/O.

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

2 months agoconfig: Split "Worker Processes" out of "Asynchronous Behavior"
Andres Freund [Tue, 11 Feb 2025 16:39:19 +0000 (11:39 -0500)]
config: Split "Worker Processes" out of "Asynchronous Behavior"

Having all the worker related GUCs in the same section as IO controlling GUCs
doesn't really make sense. Create a separate section for them.

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

2 months agoAllow extension functions to participate in in-place updates.
Tom Lane [Tue, 11 Feb 2025 17:49:34 +0000 (12:49 -0500)]
Allow extension functions to participate in in-place updates.

Commit 1dc5ebc90 allowed PL/pgSQL to perform in-place updates
of expanded-object variables that are being updated with
assignments like "x := f(x, ...)".  However this was allowed
only for a hard-wired list of functions f(), since we need to
be sure that f() will not modify the variable if it fails.
It was always envisioned that we should make that extensible,
but at the time we didn't have a good way to do so.  Since
then we've invented the idea of "support functions" to allow
attaching specialized optimization knowledge to functions,
and that is a perfect mechanism for doing this.

Hence, adjust PL/pgSQL to use a support function request instead
of hard-wired logic to decide if in-place update is safe.
Preserve the previous optimizations by creating support functions
for the three functions that were previously hard-wired.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
Reviewed-by: Pavel Borisov <pashkin.elfe@gmail.com>
Discussion: https://postgr.es/m/CACxu=vJaKFNsYxooSnW1wEgsAO5u_v1XYBacfVJ14wgJV_PYeg@mail.gmail.com

2 months agoImplement new optimization rule for updates of expanded variables.
Tom Lane [Tue, 11 Feb 2025 17:34:59 +0000 (12:34 -0500)]
Implement new optimization rule for updates of expanded variables.

If a read/write expanded variable is declared locally to the
assignment statement that is updating it, and it is referenced
exactly once in the assignment RHS, then we can optimize the
operation as a direct update of the expanded value, whether
or not the function(s) operating on it can be trusted not to
modify the value before throwing an error.  This works because
if an error does get thrown, we no longer care what value the
variable has.

In cases where that doesn't work, fall back to the previous
rule that checks for safety of the top-level function.

In any case, postpone determination of whether these optimizations
are feasible until we are executing a Param referencing the target
variable and that variable holds a R/W expanded object.  While the
previous incarnation of exec_check_rw_parameter was pretty cheap,
this is a bit less so, and our plan to invoke support functions
will make it even less so.  So avoiding the check for variables
where it couldn't be useful should be a win.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
Reviewed-by: Pavel Borisov <pashkin.elfe@gmail.com>
Discussion: https://postgr.es/m/CACxu=vJaKFNsYxooSnW1wEgsAO5u_v1XYBacfVJ14wgJV_PYeg@mail.gmail.com

2 months agoDetect whether plpgsql assignment targets are "local" variables.
Tom Lane [Tue, 11 Feb 2025 17:27:15 +0000 (12:27 -0500)]
Detect whether plpgsql assignment targets are "local" variables.

Mark whether the target of a potentially optimizable assignment
is "local", in the sense of being declared inside any exception
block that could trap an error thrown from the assignment.
(This implies that we needn't preserve the variable's value
in case of an error.  This patch doesn't do anything with the
knowledge, but the next one will.)

Normally, this requires a post-parsing scan of the function's
parse tree, since we don't know while parsing a BEGIN ...
construct whether we will find EXCEPTION at its end.  However,
if there are no BEGIN ... EXCEPTION blocks in the function at
all, then all assignments are local, even those to variables
representing function arguments.  We optimize that common case
by initializing the target_is_local flags to "true", and fixing
them up with a post-scan only if we found EXCEPTION.

Note that variables' default-value expressions are never interesting
for expanded-variable optimization, since they couldn't contain a
reference to the target variable anyway.  But the code is set up
to compute their target_param and target_is_local correctly anyway,
for consistency and in case someone thinks of a use for that data.

I added a bit of plpgsql_dumptree support to help verify that this
code sets the flags as expected.  I also added a plpgsql_dumptree
call in plpgsql_compile_inline.  It was at best an oversight that
"#option dump" didn't work in a DO block; now it does.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
Reviewed-by: Pavel Borisov <pashkin.elfe@gmail.com>
Discussion: https://postgr.es/m/CACxu=vJaKFNsYxooSnW1wEgsAO5u_v1XYBacfVJ14wgJV_PYeg@mail.gmail.com

2 months agoPreliminary refactoring of plpgsql expression construction.
Tom Lane [Tue, 11 Feb 2025 17:20:05 +0000 (12:20 -0500)]
Preliminary refactoring of plpgsql expression construction.

This short and boring patch simply moves the responsibility for
initializing PLpgSQL_expr.target_param into plpgsql parsing,
rather than doing it at first execution of the expr as before.
This doesn't save anything in terms of runtime, since the work was
trivial and done only once per expr anyway.  But it makes the info
available during parsing, which will be useful for the next step.

Likewise set PLpgSQL_expr.func during parsing.  According to the
comments, this was once impossible; but it's certainly possible
since we invented the plpgsql_curr_compile variable.  Again, this
saves little runtime, but it seems far cleaner conceptually.

While at it, I reordered stuff in struct PLpgSQL_expr to make it
clearer which fields are filled when, and merged some duplicative
code in pl_gram.y.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
Reviewed-by: Pavel Borisov <pashkin.elfe@gmail.com>
Discussion: https://postgr.es/m/CACxu=vJaKFNsYxooSnW1wEgsAO5u_v1XYBacfVJ14wgJV_PYeg@mail.gmail.com

2 months agoRefactor pl_funcs.c to provide a usage-independent tree walker.
Tom Lane [Tue, 11 Feb 2025 17:14:12 +0000 (12:14 -0500)]
Refactor pl_funcs.c to provide a usage-independent tree walker.

We haven't done this up to now because there was only one use-case,
namely plpgsql_free_function_memory's search for expressions to clean
up.  However an upcoming patch has another need for walking plpgsql
functions' statement trees, so let's create sharable tree-walker
infrastructure in the same style as expression_tree_walker().

This patch actually makes the code shorter, although that's
mainly down to having used a more compact coding style.  (I didn't
write a separate subroutine for each statement type, and I made
use of some newer notations like foreach_ptr.)

Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
Reviewed-by: Pavel Borisov <pashkin.elfe@gmail.com>
Discussion: https://postgr.es/m/CACxu=vJaKFNsYxooSnW1wEgsAO5u_v1XYBacfVJ14wgJV_PYeg@mail.gmail.com

2 months agoReplace AssertMacro() with Assert() when not in macro
Peter Eisentraut [Tue, 11 Feb 2025 10:12:05 +0000 (11:12 +0100)]
Replace AssertMacro() with Assert() when not in macro

This was forgotten to be changed in commit 9c727360bcc.

2 months agoFix indentation of comment in plannodes.h
Michael Paquier [Mon, 10 Feb 2025 22:40:03 +0000 (07:40 +0900)]
Fix indentation of comment in plannodes.h

Oversight in commit 3d17d7d7fb7a.  Worth noting that pgindent was fine
as-is.

Author: Sami Imseih
Discussion: https://postgr.es/m/CAA5RZ0t80hP2aTv97QtEJy39GkxKmDBVDiTBApfiuTa4O=TEWQ@mail.gmail.com

2 months agoAdapt appendPsqlMetaConnect() to the new fmtId() encoding expectations.
Tom Lane [Mon, 10 Feb 2025 21:30:03 +0000 (16:30 -0500)]
Adapt appendPsqlMetaConnect() to the new fmtId() encoding expectations.

We need to tell fmtId() what encoding to assume, but this function
doesn't know that.  Fortunately we can fix that without changing the
function's API, because we can just use SQL_ASCII.  That's because
database names in connection requests are effectively binary not text:
no encoding-aware processing will happen on them.

This fixes XversionUpgrade failures seen in the buildfarm.  The
alternative of having pg_upgrade use setFmtEncoding() is unappetizing,
given that it's connecting to multiple databases that may have
different encodings.

Andres Freund, Noah Misch, Tom Lane

Security: CVE-2025-1094

2 months agoLock table in ShareUpdateExclusive when importing index stats.
Jeff Davis [Mon, 10 Feb 2025 20:25:24 +0000 (12:25 -0800)]
Lock table in ShareUpdateExclusive when importing index stats.

Follow locking behavior of ANALYZE when importing statistics. In
particular, when importing index statistics, the table must be locked
in ShareUpdateExclusive mode. Fixes bug reportd by Jian He.

ANALYZE doesn't update statistics on partitioned indexes, and the
locking requirements are slightly different for in-place updates on
partitioned indexes versus normal indexes. To be conservative, lock
both the partitioned table and the partitioned index in
ShareUpdateExclusive mode when importing stats for a partitioned
index.

Author: Corey Huinker
Reported-by: Jian He
Reviewed-by: Michael Paquier
Discussion: https://www.postgresql.org/message-id/CACJufxGreTY7qsCV8%2BBkuv0p5SXGTScgh%3DD%2BDq6%3D%2B_%3DXTp7FWg%40mail.gmail.com

2 months agoFix type in test_escape test
Andres Freund [Mon, 10 Feb 2025 17:09:23 +0000 (12:09 -0500)]
Fix type in test_escape test

On machines where char is unsigned this could lead to option parsing looping
endlessly. It's also too narrow a type on other hardware.

Found via Tom Lane's monitoring of the buildfarm.

Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Security: CVE-2025-1094
Backpatch-through: 13

2 months agodocs: EUC_TW can be up to four bytes wide, not three
Andres Freund [Mon, 10 Feb 2025 15:03:37 +0000 (10:03 -0500)]
docs: EUC_TW can be up to four bytes wide, not three

Backpatch-through: 13
Security: CVE-2025-1094

2 months agoAdd test of various escape functions
Andres Freund [Mon, 10 Feb 2025 15:03:37 +0000 (10:03 -0500)]
Add test of various escape functions

As highlighted by the prior commit, writing correct escape functions is less
trivial than one might hope.

This test module tries to verify that different escaping functions behave
reasonably. It e.g. tests:

- Invalidly encoded input to an escape function leads to invalidly encoded
  output

- Trailing incomplete multi-byte characters are handled sensibly

- Escaped strings are parsed as single statement by psql's parser (which
  derives from the backend parser)

There are further tests that would be good to add. But even in the current
state it was rather useful for writing the fix in the prior commit.

Reviewed-by: Noah Misch <noah@leadboat.com>
Backpatch-through: 13
Security: CVE-2025-1094

2 months agoFix handling of invalidly encoded data in escaping functions
Andres Freund [Mon, 10 Feb 2025 15:03:37 +0000 (10:03 -0500)]
Fix handling of invalidly encoded data in escaping functions

Previously invalidly encoded input to various escaping functions could lead to
the escaped string getting incorrectly parsed by psql.  To be safe, escaping
functions need to ensure that neither invalid nor incomplete multi-byte
characters can be used to "escape" from being quoted.

Functions which can report errors now return an error in more cases than
before. Functions that cannot report errors now replace invalid input bytes
with a byte sequence that cannot be used to escape the quotes and that is
guaranteed to error out when a query is sent to the server.

The following functions are fixed by this commit:
- PQescapeLiteral()
- PQescapeIdentifier()
- PQescapeString()
- PQescapeStringConn()
- fmtId()
- appendStringLiteral()

Reported-by: Stephen Fewer <stephen_fewer@rapid7.com>
Reviewed-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Backpatch-through: 13
Security: CVE-2025-1094

2 months agoSpecify the encoding of input to fmtId()
Andres Freund [Mon, 10 Feb 2025 15:03:37 +0000 (10:03 -0500)]
Specify the encoding of input to fmtId()

This commit adds fmtIdEnc() and fmtQualifiedIdEnc(), which allow to specify
the encoding as an explicit argument.  Additionally setFmtEncoding() is
provided, which defines the encoding when no explicit encoding is provided, to
avoid breaking all code using fmtId().

All users of fmtId()/fmtQualifiedId() are either converted to the explicit
version or a call to setFmtEncoding() has been added.

This commit does not yet utilize the now well-defined encoding, that will
happen in a subsequent commit.

Reviewed-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Backpatch-through: 13
Security: CVE-2025-1094

2 months agoAdd pg_encoding_set_invalid()
Andres Freund [Mon, 10 Feb 2025 15:03:37 +0000 (10:03 -0500)]
Add pg_encoding_set_invalid()

There are cases where we cannot / do not want to error out for invalidly
encoded input. In such cases it can be useful to replace e.g. an incomplete
multi-byte characters with bytes that will trigger an error when getting
validated as part of a larger string.

Unfortunately, until now, for some encoding no such sequence existed. For
those encodings this commit removes one previously accepted input combination
- we consider that to be ok, as the chosen bytes are outside of the valid
ranges for the encodings, we just previously failed to detect that.

As we cannot add a new field to pg_wchar_table without breaking ABI, this is
implemented "in-line" in the newly added function.

Author: Noah Misch <noah@leadboat.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Backpatch-through: 13
Security: CVE-2025-1094

2 months agoReformat node comments in plannodes.h
Michael Paquier [Mon, 10 Feb 2025 00:58:25 +0000 (09:58 +0900)]
Reformat node comments in plannodes.h

This is similar to d575051b9af9 but this time for the comments in
plannodes.h to avoid long lines, which is useful if adding per-field
annotations with pg_node_attr() to these planner structures.

Some patches are under discussion to add such properties to planner
fields, which is something that may or may not happen, and this change
makes future proposals easier to work on and review, which being more
consistent in style with the parse nodes.

Author: Sami Imseih
Discussion: https://postgr.es/m/Z5xTb5iBHVGns35R@paquier.xyz

2 months agoCache NO ACTION foreign keys separately from RESTRICT foreign keys
Peter Eisentraut [Sun, 9 Feb 2025 12:43:56 +0000 (13:43 +0100)]
Cache NO ACTION foreign keys separately from RESTRICT foreign keys

Now that we generate different SQL for temporal NO ACTION vs RESTRICT
foreign keys, we should cache their query plans with different keys.
Since the key also includes the constraint oid, this shouldn't be
necessary, but we have been seeing build farm failures that suggest we
might be sometimes using a cached NO ACTION plan to implement a RESTRICT
constraint.

Author: Paul A. Jungwirth <pj@illuminatedcomputing.com>
Discussion: https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com

2 months agoMake TLS write functions' buffer arguments pointers const
Peter Eisentraut [Sun, 9 Feb 2025 11:10:25 +0000 (12:10 +0100)]
Make TLS write functions' buffer arguments pointers const

This also makes it match the equivalent APIs in libpq.

Author: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Discussion: https://www.postgresql.org/message-id/flat/fd1fcedb-3492-4fc8-9e3e-74b97f2db6c7%40eisentraut.org

2 months agoRefactor TAP test code for file comparisons into new routine in Utils.pm
Michael Paquier [Sun, 9 Feb 2025 07:52:33 +0000 (16:52 +0900)]
Refactor TAP test code for file comparisons into new routine in Utils.pm

This unifies the output used should any differences be found in the
files provided, information that 027_stream_regress did not show on
failures.  TAP tests of pg_combinebackup and pg_upgrade now rely on the
refactored routine, reducing the dependency to the diff command.  The
callers of this routine can optionally specify a custom line-comparison
function.

There are a couple of tests that still use directly a diff command:
001_pg_bsd_indent, 017_shm and test_json_parser's 003.  These rely on
different properties and are left out for now.

Extracted from a larger patch by the same author.

Author: Ashutosh Bapat
Discussion: https://postgr.es/m/Z6RQS-tMzGYjlA-H@paquier.xyz

2 months agoPDF docs build: avoid spurious "warn" in build logs.
Tom Lane [Sat, 8 Feb 2025 03:12:38 +0000 (22:12 -0500)]
PDF docs build: avoid spurious "warn" in build logs.

Improve on e4c886519 so that the string "warn" appears in
the output when there's a problem, and not when there isn't.
This should silence noise I've been seeing in my buildfarm
warning scraper.

2 months agoFix pgbench performance issue induced by commit af35fe501.
Tom Lane [Fri, 7 Feb 2025 18:41:42 +0000 (13:41 -0500)]
Fix pgbench performance issue induced by commit af35fe501.

Commit af35fe501 caused "pgbench -i" to emit a '\r' character
for each data row loaded (when stderr is a terminal).
That's effectively invisible on-screen, but it causes the
connected terminal program to consume a lot of cycles.
It's even worse if you're connected over ssh, as the data
then has to pass through the ssh tunnel.

Simplest fix is to move the added logic inside the if-tests
that check whether to print a progress line.  We could do
it another way that avoids duplicating these few lines,
but on the whole this seems the most transparent way to
write it.

Like the previous commit, back-patch to all supported versions.

Reported-by: Andres Freund <andres@anarazel.de>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Discussion: https://postgr.es/m/4k4drkh7bcmdezq6zbkhp25mnrzpswqi2o75d5uv2eeg3aq6q7@b7kqdmzzwzgb
Backpatch-through: 13

2 months agoDoc: clarify behavior of timestamptz input some more.
Tom Lane [Fri, 7 Feb 2025 17:40:41 +0000 (12:40 -0500)]
Doc: clarify behavior of timestamptz input some more.

Try to make it absolutely plain that we don't retain the
originally specified time zone, only the UTC timestamp.

While at it, make glossary entries for "UTC" and "GMT".

Author: Robert Treat <rob@xzilla.net>
Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/173796426022.1064.9135167366862649513@wrigleys.postgresql.org
Backpatch-through: 13

2 months agoAllow non-btree speculative insertion indexes
Peter Eisentraut [Fri, 7 Feb 2025 10:17:25 +0000 (11:17 +0100)]
Allow non-btree speculative insertion indexes

Previously, only btrees were supported as the arbiter index for
speculative insertion because there was no way to get the equality
strategy number for other index methods.  We have this now (commit
c09e5a6a016), so we can support this.

At the moment, only btree supports unique indexes, so this does not
change anything in practice, but it would allow another index method
that has amcanunique to be supported.

Co-authored-by: Mark Dilger <mark.dilger@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/E72EAA49-354D-4C2E-8EB9-255197F55330@enterprisedb.com

2 months agoSupport non-btree indexes for foreign keys
Peter Eisentraut [Fri, 7 Feb 2025 10:17:25 +0000 (11:17 +0100)]
Support non-btree indexes for foreign keys

Previously, only btrees were supported as the referenced unique index
for foreign keys because there was no way to get the equality strategy
number for other index methods.  We have this now (commit
c09e5a6a016), so we can support this.  In fact, this is now just a
special case of the existing generalized "period" foreign key
support, since that already knows how to lookup equality strategy
numbers.

Note that this does not change the requirement that the referenced
index needs to be unique, and at the moment, only btree supports that,
so this does not change anything in practice, but it would allow
another index method that has amcanunique to be supported.

Co-authored-by: Mark Dilger <mark.dilger@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/E72EAA49-354D-4C2E-8EB9-255197F55330@enterprisedb.com

2 months agoVirtual generated columns
Peter Eisentraut [Fri, 7 Feb 2025 08:09:34 +0000 (09:09 +0100)]
Virtual generated columns

This adds a new variant of generated columns that are computed on read
(like a view, unlike the existing stored generated columns, which are
computed on write, like a materialized view).

The syntax for the column definition is

    ... GENERATED ALWAYS AS (...) VIRTUAL

and VIRTUAL is also optional.  VIRTUAL is the default rather than
STORED to match various other SQL products.  (The SQL standard makes
no specification about this, but it also doesn't know about VIRTUAL or
STORED.)  (Also, virtual views are the default, rather than
materialized views.)

Virtual generated columns are stored in tuples as null values.  (A
very early version of this patch had the ambition to not store them at
all.  But so much stuff breaks or gets confused if you have tuples
where a column in the middle is completely missing.  This is a
compromise, and it still saves space over being forced to use stored
generated columns.  If we ever find a way to improve this, a bit of
pg_upgrade cleverness could allow for upgrades to a newer scheme.)

The capabilities and restrictions of virtual generated columns are
mostly the same as for stored generated columns.  In some cases, this
patch keeps virtual generated columns more restricted than they might
technically need to be, to keep the two kinds consistent.  Some of
that could maybe be relaxed later after separate careful
considerations.

Some functionality that is currently not supported, but could possibly
be added as incremental features, some easier than others:

- index on or using a virtual column
- hence also no unique constraints on virtual columns
- extended statistics on virtual columns
- foreign-key constraints on virtual columns
- not-null constraints on virtual columns (check constraints are supported)
- ALTER TABLE / DROP EXPRESSION
- virtual column cannot have domain type
- virtual columns are not supported in logical replication

The tests in generated_virtual.sql have been copied over from
generated_stored.sql with the keyword replaced.  This way we can make
sure the behavior is mostly aligned, and the differences can be
visible.  Some tests for currently not supported features are
currently commented out.

Reviewed-by: Jian He <jian.universality@gmail.com>
Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Tested-by: Shlok Kyal <shlok.kyal.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/a368248e-69e4-40be-9c07-6c3b5880b0a6@eisentraut.org

2 months agoTrack unpruned relids to avoid processing pruned relations
Amit Langote [Fri, 7 Feb 2025 08:15:09 +0000 (17:15 +0900)]
Track unpruned relids to avoid processing pruned relations

This commit introduces changes to track unpruned relations explicitly,
making it possible for top-level plan nodes, such as ModifyTable and
LockRows, to avoid processing partitions pruned during initial
pruning.  Scan-level nodes, such as Append and MergeAppend, already
avoid the unnecessary processing by accessing partition pruning
results directly via part_prune_index. In contrast, top-level nodes
cannot access pruning results directly and need to determine which
partitions remain unpruned.

To address this, this commit introduces a new bitmapset field,
es_unpruned_relids, which the executor uses to track the set of
unpruned relations.  This field is referenced during plan
initialization to skip initializing certain nodes for pruned
partitions. It is initialized with PlannedStmt.unprunableRelids,
a new field that the planner populates with RT indexes of relations
that cannot be pruned during runtime pruning. These include relations
not subject to partition pruning and those required for execution
regardless of pruning.

PlannedStmt.unprunableRelids is computed during set_plan_refs() by
removing the RT indexes of runtime-prunable relations, identified
from PartitionPruneInfos, from the full set of relation RT indexes.
ExecDoInitialPruning() then updates es_unpruned_relids by adding
partitions that survive initial pruning.

To support this, PartitionedRelPruneInfo and PartitionedRelPruningData
now include a leafpart_rti_map[] array that maps partition indexes to
their corresponding RT indexes. The former is used in set_plan_refs()
when constructing unprunableRelids, while the latter is used in
ExecDoInitialPruning() to convert partition indexes returned by
get_matching_partitions() into RT indexes, which are then added to
es_unpruned_relids.

These changes make it possible for ModifyTable and LockRows nodes to
process only relations that remain unpruned after initial pruning.
ExecInitModifyTable() trims lists, such as resultRelations,
withCheckOptionLists, returningLists, and updateColnosLists, to
consider only unpruned partitions. It also creates ResultRelInfo
structs only for these partitions. Similarly, child RowMarks for
pruned relations are skipped.

By avoiding unnecessary initialization of structures for pruned
partitions, these changes improve the performance of updates and
deletes on partitioned tables during initial runtime pruning.

Due to ExecInitModifyTable() changes as described above, EXPLAIN on a
plan for UPDATE and DELETE that uses runtime initial pruning no longer
lists partitions pruned during initial pruning.

Reviewed-by: Robert Haas <robertmhaas@gmail.com> (earlier versions)
Reviewed-by: Tomas Vondra <tomas@vondra.me>
Discussion: https://postgr.es/m/CA+HiwqFGkMSge6TgC9KQzde0ohpAycLQuV7ooitEEpbKB0O_mg@mail.gmail.com

2 months agoinjection_points: Tweak more permutation in isolation test "basic"
Michael Paquier [Fri, 7 Feb 2025 04:58:22 +0000 (13:58 +0900)]
injection_points: Tweak more permutation in isolation test "basic"

The CI has reported that using a marker to force the output of the
detach step to happen after the wait step was not enough, as
isolationtester has managed to report the detach step as waiting before
the wait step finishes in some runs.

src/test/isolation/README tells that there is a more drastic method to
enforce the ordering of the output: an empty step positioned just after
the wait step can force the wait step to complete before the detach step
begins.  This method has been able to pass 10 runs in the CI here, while
HEAD seems to fail 15~20% of the time in the CF bot.

Discussion: https://postgr.es/m/Z6WO8FbqK_FHmrzC@paquier.xyz

2 months agoMove SQL tests of pg_stat_io for WAL data to recovery test 029_stats_restart
Michael Paquier [Fri, 7 Feb 2025 00:42:31 +0000 (09:42 +0900)]
Move SQL tests of pg_stat_io for WAL data to recovery test 029_stats_restart

Three tests in the main regression test suite are proving to not be
portable across multiple runs on a deployed cluster as stats of
pg_stat_io are reset.  Problems happen for tests on:
- Writes of WAL in the init context, when creating a WAL segment.
- Syncs of WAL in the init context, when creating a WAL segment.
- Reads of WAL in the normal context, requiring a WAL record to be read.
For a `make check`, this could rely on the checkpoint record read by the
startup process when starting the cluster, something that is not going
to work for a deployed node.

Two of the three tests are moved to the recovery TAP test
029_stats_restart, where we already check the consistency of stats
data.  The test for syncs is dropped as TAP can run with fsync=off.  The
other two are checked with some data from a freshly-initialized cluster.

Per discussion with Tom Lane, Bertrand Drouvot and Nazir Bilal Yavuz.

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

2 months agoDisallow COPY FREEZE on foreign tables.
Nathan Bossart [Thu, 6 Feb 2025 21:23:40 +0000 (15:23 -0600)]
Disallow COPY FREEZE on foreign tables.

This didn't actually work: the COPY succeeds, but the FREEZE
optimization isn't applied.  There doesn't seem to be an easy way
to support FREEZE on foreign tables, so let's follow the precedent
established by commit 5c9a5513a3 by raising an error early.  This
is arguably a bug fix, but due to the lack of reports, the minimal
discussion on the mailing list, and the potential to break existing
scripts, I am not back-patching it for now.

Author: Sami Imseih <samimseih@gmail.com>
Reviewed-by: Zhang Mingli <zmlpostgres@gmail.com>
Discussion: https://postgr.es/m/CAA5RZ0ujeNgKpE3OrLtR%3DeJGa5LkGMekFzQTwjgw%3DrzaLufQLQ%40mail.gmail.com

2 months agolibpq: Handle asynchronous actions during SASL
Daniel Gustafsson [Thu, 6 Feb 2025 21:19:21 +0000 (22:19 +0100)]
libpq: Handle asynchronous actions during SASL

This adds the ability for a SASL mechanism to signal PQconnectPoll()
that some arbitrary work, external to the Postgres connection, is
required for authentication to continue.  There is no consumer for
this capability as part of this commit, it is infrastructure which
is required for future work on supporting the OAUTHBEARER mechanism.

To ensure that threads are not blocked waiting for the SASL mechanism
to make long-running calls, the mechanism communicates with the top-
level client via the "altsock": a file or socket descriptor, opaque to
this layer of libpq, which is signaled when work is ready to be done
again.  The altsock temporarily replaces the regular connection
descriptor, so existing PQsocket() clients should continue to operate
correctly using their existing polling implementations.

For a mechanism to use this it should set an authentication callback,
conn->async_auth(), and a cleanup callback, conn->cleanup_async_auth(),
and return SASL_ASYNC during the exchange.  It should then assign
conn->altsock during the first call to async_auth().  When the cleanup
callback is called, either because authentication has succeeded or
because the connection is being dropped, the altsock must be released
and disconnected from the PGconn object.

This was extracted from the larger OAUTHBEARER patchset which has
been developed, and reviewed by many, over several years and it is
thus likely that some reviewer credit of much earlier versions has
been accidentally omitted.

Author: Jacob Champion <jacob.champion@enterprisedb.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Antonin Houska <ah@cybertec.at>
Discussion: https://postgr.es/m/CAOYmi+kJqzo6XsR9TEhvVfeVNQ-TyFM5LATypm9yoQVYk=4Wrw@mail.gmail.com

2 months agoRemove support for linking with libeay32 and ssleay32
Daniel Gustafsson [Thu, 6 Feb 2025 19:26:46 +0000 (20:26 +0100)]
Remove support for linking with libeay32 and ssleay32

The OpenSSL project stopped using the eay names back in 2016
on platforms other than Microsoft Windows, and version 1.1.0
removed the names from Windows as well. Since we now require
OpenSSL 1.1.1 we can remove support for using the eay names
from our tree as well.

Author: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/3C445F8E-D43E-4970-9CD9-A54882197714@yesql.se
Discussion: https://postgr.es/m/CAHrt6656W9OnFomQTHBGYDcM5CKZ7hcgzFt8L+N0ezBZfcN3zA@mail.gmail.com

3 months agoFix autovacuum_vacuum_max_threshold's GUC description.
Nathan Bossart [Thu, 6 Feb 2025 17:59:12 +0000 (11:59 -0600)]
Fix autovacuum_vacuum_max_threshold's GUC description.

Most GUCs that accept a special value to disable the feature
mention it in their GUC description.  This commit adds that
information to autovacuum_vacuum_max_threshold's description.

Oversight in commit 306dc520b9.

3 months agopgcrypto: Remove static storage class from variables
Daniel Gustafsson [Thu, 6 Feb 2025 14:13:40 +0000 (15:13 +0100)]
pgcrypto: Remove static storage class from variables

Variables p, sp and ep were labeled with static storage class
but are all assigned before use so they cannot carry any data
across calls.  Fix by removing the static label.

Also while in there, make the magic variable const as it will
never change.

Author: Japin Li <japinli@hotmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/ME0P300MB0445096B67ACE8CE25772F00B6F72@ME0P300MB0445.AUSP300.PROD.OUTLOOK.COM

3 months agoinjection_points: Re-enable permutation in isolation test "basic"
Michael Paquier [Thu, 6 Feb 2025 01:39:41 +0000 (10:39 +0900)]
injection_points: Re-enable permutation in isolation test "basic"

This test has been disabled in 9f00edc22888 due to an instable expected
output, where it would be possible for the wait step to report its
result after the detach step is done.  The expected output was ordered
so as the detach would always report last.

Isolation test permutations have the option to use markers to control
the ordering for cases like this one, as documented in
src/test/isolation/README.  The permutation is enabled once again, this
time with a marker added so as the detach step reports only once the
wait step has finished, ensuring a correct output ordering.

Reviewed-by: Bertrand Drouvot
Discussion: https://postgr.es/m/Z6MBZTX5EqQ6A8Zc@paquier.xyz

3 months agoIntroduce autovacuum_vacuum_max_threshold.
Nathan Bossart [Wed, 5 Feb 2025 21:48:18 +0000 (15:48 -0600)]
Introduce autovacuum_vacuum_max_threshold.

One way autovacuum chooses tables to vacuum is by comparing the
number of updated or deleted tuples with a value calculated using
autovacuum_vacuum_threshold and autovacuum_vacuum_scale_factor.
The threshold specifies the base value for comparison, and the
scale factor specifies the fraction of the table size to add to it.
This strategy ensures that smaller tables are vacuumed after fewer
updates/deletes than larger tables, which is reasonable in many
cases but can result in infrequent vacuums on very large tables.
This is undesirable for a couple of reasons, such as very large
tables incurring a huge amount of bloat between vacuums.

This new parameter provides a way to set a limit on the value
calculated with autovacuum_vacuum_threshold and
autovacuum_vacuum_scale_factor so that very large tables are
vacuumed more frequently.  By default, it is set to 100,000,000
tuples, but it can be disabled by setting it to -1.  It can also be
adjusted for individual tables by changing storage parameters.

Author: Nathan Bossart <nathandbossart@gmail.com>
Co-authored-by: Frédéric Yhuel <frederic.yhuel@dalibo.com>
Reviewed-by: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Reviewed-by: Laurenz Albe <laurenz.albe@cybertec.at>
Reviewed-by: Michael Banck <mbanck@gmx.net>
Reviewed-by: Joe Conway <mail@joeconway.com>
Reviewed-by: Sami Imseih <samimseih@gmail.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: wenhui qiu <qiuwenhuifx@gmail.com>
Reviewed-by: Vinícius Abrahão <vinnix.bsd@gmail.com>
Reviewed-by: Robert Treat <rob@xzilla.net>
Reviewed-by: Alena Rybakina <a.rybakina@postgrespro.ru>
Discussion: https://postgr.es/m/956435f8-3b2f-47a6-8756-8c54ded61802%40dalibo.com

3 months agoShow more-intuitive titles for psql commands \dt, \di, etc.
Tom Lane [Wed, 5 Feb 2025 17:45:58 +0000 (12:45 -0500)]
Show more-intuitive titles for psql commands \dt, \di, etc.

If exactly one relation type is requested in a command of the \dtisv
family, say "tables", "indexes", etc instead of "relations".  This
should cover the majority of actual uses, without creating a huge
number of new translatable strings.  The error messages for no
matching relations are adjusted as well.

In passing, invent "pg_log_error_internal()" to be used for frontend
error messages that don't seem to need translation, analogously to
errmsg_internal() in the backend.  The implementation is a bit cheesy,
being just a macro to prevent xgettext from recognizing a trigger
keyword.  This won't avoid a useless gettext lookup cycle at runtime
--- but surely we don't care about an extra microsecond or two in
what's supposed to be a can't-happen case.  I (tgl) also made
"pg_fatal_internal()", though it's not used in this patch.

Author: Greg Sabino Mullane <htamfids@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAKAnmm+7o93fQV-RFkGaN1QnP-0D4d3JTykD+cLueqjDMKdfag@mail.gmail.com

3 months agodoc: Update links which returned 404
Daniel Gustafsson [Wed, 5 Feb 2025 12:58:40 +0000 (13:58 +0100)]
doc: Update links which returned 404

Two links in the isn module documentation were pointing to tools
which had been moved, resulting in 404 error responses.  Update
to the new URLs for the tools.  The link to the Sequoia 2000 page
in the history section was no longer working, and since the page
is no longer available online update our link to point at the
paper instead which is on a stable URL.

These links exist in all versions of the documentation so backpatch
to all supported branches.

Author: Daniel Gustafsson <daniel@yesql.se>
Reported-by: charukiewicz@protonmail.com
Discussion: https://postgr.es/m/173679670185.705.8565555804465055355@wrigleys.postgresql.org
Backpatch-through: 13

3 months agoAvoid updating inactive_since for invalid replication slots.
Amit Kapila [Wed, 5 Feb 2025 03:26:14 +0000 (08:56 +0530)]
Avoid updating inactive_since for invalid replication slots.

It is possible for the inactive_since value of an invalid replication slot
to be updated multiple times, which is unexpected behavior like during the
release of the slot or at the time of restart. This is harmless because
invalid slots are not allowed to be accessed but it is not prudent to
update invalid slots. We are planning to invalidate slots due to other
reasons like idle time and it will look odd that the slot's inactive_since
displays the recent time in this field after invalidated due to idle time.
So, this patch ensures that the inactive_since field of slots is not
updated for invalid slots.

In the passing, ensure to use the same inactive_since time for all the
slots at restart while restoring them from the disk.

Author: Nisha Moond <nisha.moond412@gmail.com>
Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Vignesh C <vignesh21@gmail.com>
Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Reviewed-by: Hou Zhijie <houzj.fnst@fujitsu.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/CABdArM7QdifQ_MHmMA=Cc4v8+MeckkwKncm2Nn6tX9wSCQ-+iw@mail.gmail.com

3 months agomeson: ci: ensure tests are built before running them
Andres Freund [Tue, 4 Feb 2025 22:45:56 +0000 (17:45 -0500)]
meson: ci: ensure tests are built before running them

Meson 1.7 stopped building all the dependencies of tests as part of the
default build target. But it does breaks CI because we only built the default
target before running the test, and ran the tests with --no-rebuild.

The simplest fix would be to remove --no-rebuild from MTEST_ARGS, but it seems
better to explicitly build the test dependencies, so compiler warnings /
errors are visible as part of the build step.

Discussion: https://postgr.es/m/CAGECzQSvM3iSDmjF+=Kof5an6jN8UbkP_4cKKT9w6GZavmb5yQ@mail.gmail.com
Backpatch: 16-, where meson was added

3 months agomeson: Add missing dependencies for libpq tests
Andres Freund [Tue, 4 Feb 2025 22:45:56 +0000 (17:45 -0500)]
meson: Add missing dependencies for libpq tests

The missing dependency was, e.g., visible when doing
  ninja clean && ninja meson-test-prereq && meson test --no-rebuild --suite setup --suite libpq

This is a bit more complicated than other related fixes, because until now
libpq's tests depended on 'frontend_code', which includes a dependency on
fe_utils, which in turns on libpq. That in turn required
src/interfaces/libpq/test to be entered from the top-level, not from
libpq/meson.build.  Because of that the test definitions in libpq/meson.build
could not declare a dependency on the binaries defined in
libpq/test/meson.build.

To fix this, this commit creates frontend_no_fe_utils_code, which allows us to
recurse into libpq/test from withing libpq/meson.build.

Apply this to all branches with meson support, as part of an effort to fix
incorrect test dependencies that can lead to test failures.

Discussion: https://postgr.es/m/CAGECzQSvM3iSDmjF+=Kof5an6jN8UbkP_4cKKT9w6GZavmb5yQ@mail.gmail.com
Discussion: https://postgr.es/m/bdba588f-69a9-4f3e-9b95-62d07210a32e@eisentraut.org
Backpatch: 16-, where meson support was added

3 months agomeson: Add missing dependencies to libpq_pipeline test
Andres Freund [Tue, 4 Feb 2025 22:45:56 +0000 (17:45 -0500)]
meson: Add missing dependencies to libpq_pipeline test

The missing dependency was, e.g., visible when doing
  ninja clean && ninja meson-test-prereq && meson test --no-rebuild --suite setup --suite libpq_pipeline

Apply this to all branches with meson support, as part of an effort to fix
incorrect test dependencies that can lead to test failures.

Discussion: https://postgr.es/m/CAGECzQSvM3iSDmjF+=Kof5an6jN8UbkP_4cKKT9w6GZavmb5yQ@mail.gmail.com
Discussion: https://postgr.es/m/bdba588f-69a9-4f3e-9b95-62d07210a32e@eisentraut.org
Backpatch: 16-, where meson support was added

3 months agomeson: Add test dependencies for test_json_parser
Andres Freund [Tue, 4 Feb 2025 22:45:56 +0000 (17:45 -0500)]
meson: Add test dependencies for test_json_parser

This is required to ensure correct test dependencies, previously
the test binaries would not necessarily be built.

The missing dependency was, e.g., visible when doing
  ninja clean && ninja meson-test-prereq && m test --no-rebuild --suite setup --suite test_json_parser

Apply this to all branches with meson support, as part of an effort to fix
incorrect test dependencies that can lead to test failures.

Author: Peter Eisentraut <peter@eisentraut.org>
Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CAGECzQSvM3iSDmjF+=Kof5an6jN8UbkP_4cKKT9w6GZavmb5yQ@mail.gmail.com
Discussion: https://postgr.es/m/bdba588f-69a9-4f3e-9b95-62d07210a32e@eisentraut.org
Backpatch: 17-, where test_json_parser was added

3 months agomeson: Add pg_regress_ecpg to ecpg test dependencies
Andres Freund [Tue, 4 Feb 2025 22:45:56 +0000 (17:45 -0500)]
meson: Add pg_regress_ecpg to ecpg test dependencies

This is required to ensure correct test dependencies, previously
pg_regress_ecpg would not necessarily be built.

The missing dependency was, e.g., visible when doing
  ninja clean && ninja meson-test-prereq && meson test --no-rebuild --suite setup --suite ecpg

Apply this to all branches with meson support, as part of an effort to fix
incorrect test dependencies that can lead to test failures.

Discussion: https://postgr.es/m/CAGECzQSvM3iSDmjF+=Kof5an6jN8UbkP_4cKKT9w6GZavmb5yQ@mail.gmail.com
Discussion: https://postgr.es/m/bdba588f-69a9-4f3e-9b95-62d07210a32e@eisentraut.org
Backpatch: 16-, where meson support was added

3 months agomeson: Improve dependencies for tmp_install test target
Andres Freund [Tue, 4 Feb 2025 22:45:55 +0000 (17:45 -0500)]
meson: Improve dependencies for tmp_install test target

The missing dependency was, e.g., visible when doing
  ninja clean && ninja meson-test-prereq && meson test --no-rebuild --suite setup --suite cube
because meson (and thus its internal meson-test-prereq target) did not know
about a lot of the required targets.

Previously tmp_install did not actually depend on the relevant files being
built. That was mostly not visible, because "meson test" currently uses the
'default' targets as a test's dependency if no dependency is specified.
However, there are plans to narrow that on the meson side, to make it quicker
to run tests.

Apply this to all branches with meson support, as part of an effort to fix
incorrect test dependencies that can lead to test failures.

Discussion: https://postgr.es/m/CAGECzQSvM3iSDmjF+=Kof5an6jN8UbkP_4cKKT9w6GZavmb5yQ@mail.gmail.com
Discussion: https://postgr.es/m/bdba588f-69a9-4f3e-9b95-62d07210a32e@eisentraut.org
Backpatch: 16-, where meson support was added

3 months agomeson: Narrow dependencies for 'install-quiet' target
Andres Freund [Tue, 4 Feb 2025 22:45:55 +0000 (17:45 -0500)]
meson: Narrow dependencies for 'install-quiet' target

Previously test dependencies, which are not actually installed, were
unnecessarily built.

Apply this to all branches with meson support, as part of an effort to fix
incorrect test dependencies that can lead to test failures.

Discussion: https://postgr.es/m/CAGECzQSvM3iSDmjF+=Kof5an6jN8UbkP_4cKKT9w6GZavmb5yQ@mail.gmail.com
Discussion: https://postgr.es/m/bdba588f-69a9-4f3e-9b95-62d07210a32e@eisentraut.org
Backpatch: 16-, where meson support was added

3 months agopg_controldata: Fix possible errors on corrupted pg_control
Alexander Korotkov [Tue, 4 Feb 2025 22:15:17 +0000 (00:15 +0200)]
pg_controldata: Fix possible errors on corrupted pg_control

Protect against malformed timestamps.  Also protect against negative WalSegSz
as it triggers division by zero:

((0x100000000UL) / (WalSegSz)) can turn into zero in

XLogFileName(xlogfilename, ControlFile->checkPointCopy.ThisTimeLineID,
             segno, WalSegSz);

because if WalSegSz is -1 then by arithmetic rules in C we get
0x100000000UL / 0xFFFFFFFFFFFFFFFFUL == 0.

Author: Ilyasov Ian <ianilyasov@outlook.com>
Author: Anton Voloshin <a.voloshin@postgrespro.ru>
Backpatch-through: 13

3 months agoAllow usage of match_orclause_to_indexcol() for joins
Alexander Korotkov [Tue, 4 Feb 2025 21:21:49 +0000 (23:21 +0200)]
Allow usage of match_orclause_to_indexcol() for joins

This commit allows transformation of OR-clauses into SAOP's for index scans
within nested loop joins.  That required the following changes.

 1. Make match_orclause_to_indexcol() and group_similar_or_args() understand
    const-ness in the same way as match_opclause_to_indexcol().  This
    generally makes our approach more uniform.
 2. Make match_join_clauses_to_index() pass OR-clauses to
    match_clause_to_index().
 3. Also switch match_join_clauses_to_index() to use list_append_unique_ptr()
    for adding clauses to *joinorclauses.  That avoids possible duplicates
    when processing the same clauses with different indexes.  Previously such
    duplicates were elimited in match_clause_to_index(), but now
    group_similar_or_args() each time generates distinct copies of grouped
    OR clauses.

Discussion: https://postgr.es/m/CAPpHfdv%2BjtNwofg-p5z86jLYZUTt6tR17Wy00ta0dL%3DwHQN3ZA%40mail.gmail.com
Reviewed-by: Andrei Lepikhov <lepihov@gmail.com>
Reviewed-by: Alena Rybakina <a.rybakina@postgrespro.ru>
Reviewed-by: Pavel Borisov <pashkin.elfe@gmail.com>
3 months agoRevise the header comment for match_clause_to_indexcol()
Alexander Korotkov [Tue, 4 Feb 2025 21:18:47 +0000 (23:18 +0200)]
Revise the header comment for match_clause_to_indexcol()

Since d4378c0005e6, match_clause_to_indexcol() doesn't always return NULL
for an OR clause.  This commit reflects that in the function header comment.

Reported-by: Pavel Borisov <pashkin.elfe@gmail.com>
3 months agovacuumdb: Add missing PQfinish() calls to vacuum_one_database().
Nathan Bossart [Tue, 4 Feb 2025 19:26:57 +0000 (13:26 -0600)]
vacuumdb: Add missing PQfinish() calls to vacuum_one_database().

A few of the version checks in vacuum_one_database() do not call
PQfinish() before exiting.  This precedent was unintentionally
established in commit 00d1e88d36, and while it's probably not too
problematic, it seems better to properly close the connection.

Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/Z6JAwqN1I8ljTuXp%40nathan
Backpatch-through: 13

3 months agosepgsql: update TAP test to use fat comma style
Peter Eisentraut [Tue, 4 Feb 2025 14:51:42 +0000 (15:51 +0100)]
sepgsql: update TAP test to use fat comma style

Adopt the style introduced by commit ce1b0f9da03 to this new test
file.

Author: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Discussion: https://www.postgresql.org/message-id/87y0yv2har.fsf@wibble.ilmari.org

3 months agoAdd data for WAL in pg_stat_io and backend statistics
Michael Paquier [Tue, 4 Feb 2025 07:50:00 +0000 (16:50 +0900)]
Add data for WAL in pg_stat_io and backend statistics

This commit adds WAL IO stats to both pg_stat_io view and per-backend IO
statistics (pg_stat_get_backend_io()).  This change is possible since
f92c854cf406, as WAL IO is not counted in blocks in some code paths
where its stats data is measured (like WAL read in xlogreader.c).

IOContext gains IOCONTEXT_INIT and IOObject IOOBJECT_WAL, with the
following combinations allowed:
- IOOBJECT_WAL/IOCONTEXT_NORMAL is used to track I/O operations done on
already-created WAL segments.
- IOOBJECT_WAL/IOCONTEXT_INIT is used for tracking I/O operations done
when initializing WAL segments.

The core changes are done in pg_stat_io.c, backend statistics inherit
them.  Backend statistics and pg_stat_io are now available for the WAL
writer, the WAL receiver and the WAL summarizer processes.

I/O timing data is controlled by the GUC track_io_timing, like the
existing data of pg_stat_io for consistency.  The timings related to
IOOBJECT_WAL show up if the GUC is enabled (disabled by default).

Bump pgstats file version, due to the additions in IOObject and
IOContext, impacting the amount of data written for the fixed-numbered
IO stats kind in the pgstats file.

Author: Nazir Bilal Yavuz
Reviewed-by: Bertrand Drouvot, Nitin Jadhav, Amit Kapila, Michael
Paquier, Melanie Plageman, Bharath Rupireddy
Discussion: https://postgr.es/m/CAN55FZ3AiQ+ZMxUuXnBpd0Rrh1YhwJ5FudkHg=JU0P+-W8T4Vg@mail.gmail.com

3 months agoIntegrate GistTranslateCompareType() into IndexAmTranslateCompareType()
Peter Eisentraut [Mon, 3 Feb 2025 07:14:27 +0000 (08:14 +0100)]
Integrate GistTranslateCompareType() into IndexAmTranslateCompareType()

This turns GistTranslateCompareType() into a callback function of the
gist index AM instead of a standalone function.  The existing callers
are changed to use IndexAmTranslateCompareType().  This then makes
that code not hardcoded toward gist.

This means in particular that the temporal keys code is now
independent of gist.  Also, this generalizes commit 74edabce7a3, so
other index access methods other than the previously hardcoded ones
could now work as REPLICA IDENTITY in a logical replication
subscriber.

Author: Mark Dilger <mark.dilger@enterprisedb.com>
Co-authored-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://www.postgresql.org/message-id/flat/E72EAA49-354D-4C2E-8EB9-255197F55330@enterprisedb.com

3 months agoFix incorrect range in pg_regress comment.
Tom Lane [Mon, 3 Feb 2025 03:37:13 +0000 (22:37 -0500)]
Fix incorrect range in pg_regress comment.

A comment in pg_regress incorrectly stated that alternative
output files could be named test_{i}.out with 0 < i <= 9.
However, the valid range is actually 0 <= i <= 9.
(The user-facing docs have this right already.)

Author: Ilia Evdokimov <ilya.evdokimov@tantorlabs.com>
Discussion: https://postgr.es/m/6e6c4dea-07a1-4a83-9bb7-77b9b3324c37@tantorlabs.com