Michael Paquier [Tue, 14 Jul 2026 23:05:03 +0000 (08:05 +0900)]
Revert "Rename routines for write/read of pgstats file"
This reverts commit ed823da1289, that has made pgstat_write_chunk() and
pgstat_read_chunk() available for public use. These routines do not
have a symmetric API definition across reads and writes, with the write
part returning a void status, deferring an error detection once all the
stats entries have been processed with an ferror(), and the read part
returning a boolean status.
These routines are just tiny wrappers around fread() and fwrite(), and
extensions can just define they own routines instead of relying on the
same facilities as the core pgstat.c. This commit removes their
declaration from the public headers, to reduce the confusion.
test_custom_stats is updated to use its own read/write routines.
Perhaps something better could be designed in the future; trying to do
so for v19 is not feasable during beta.
Reported-by: Peter Eisentraut <peter@eisentraut.org>
Author: Sami Imseih <samimseih@gmail.com>
Discussion: https://postgr.es/m/a4a8e9af-3eaf-4bbf-9b21-21620f3fc434@eisentraut.org
Backpatch-through: 19
postgres_fdw: don't push down non-relabeling ArrayCoerceExpr
Commit 62c3b4cd9ddc taught postgres_fdw to push down ArrayCoerceExpr, but
foreign_expr_walker() only recursed into the input array expression and
never examined elemexpr, the per-element conversion that gives the
coercion its semantics. deparseArrayCoerceExpr() then shipped a bare
"arg::resulttype" cast, or nothing at all for an implicit-format
coercion, leaving the remote server to re-resolve the element conversion
against its own catalogs and session state.
This produced wrong results or remote errors whenever the element
conversion was not a plain relabeling, and it was inconsistent with how
postgres_fdw treats the equivalent scalar coercions. An ArrayCoerceExpr
was shipped even when its elemexpr was a cast function (whose
shippability was never checked), a CoerceViaIO (e.g. float8out or
byteaout, which depend on extra_float_digits / bytea_output that
postgres_fdw sets differently on the remote session), or a
CoerceToDomain (which pushes domain enforcement to the remote catalog).
By contrast, a scalar CoerceViaIO is never shipped, and a scalar cast
function is shipped only when it is shippable.
Restrict pushdown to element coercions that are a plain relabeling, that
is, elemexpr is a RelabelType or a bare CaseTestExpr. Any other element
coercion is now evaluated locally. This keeps the common
binary-coercible case pushed down, including "col = ANY($1)" with a
varchar[]-to-text[] relabeling, which is the case 62c3b4cd9ddc set out to
optimize.
Pushing down shippable element cast functions, to reach parity with the
scalar case, is left out here for simplicity.
Remove unreachable error check in JSON_TABLE plan transform
transformJsonTableNestedColumns() looked up the nested column named by a
JSON_TABLE PLAN node and raised "invalid JSON_TABLE plan clause / PATH
name was %s not found in nested columns list" if none was found. That
lookup cannot fail: validateJsonTableChildPlan() runs first, at every
plan level, and already matches the plan's sibling path names one-to-one
against the nested columns (reporting any uncovered nested path or any
extra or duplicate sibling node). The check has been unreachable since
the PLAN clause code was first written.
Replace it with an Assert documenting the invariant, which also removes
a user-facing message that could never be emitted.
Reported-by: Thom Brown <thom@linux.com>
Discussion: https://postgr.es/m/CAA-aLv5_9%3DzgA_Y7aoFp-%2BQSeh0kx4dfbAas9Wx%3DyrweQSqa6Q%40mail.gmail.com
Avoid redundant re-evaluation of JSON_TABLE nested paths
86ab7f4c721d makes JSON_TABLE with a NESTED PATH re-ran the nested path's
jsonpath expression several times for each parent row. That makes such
queries significantly slower as the nested arrays grew, even without a PLAN
clause.
Two leftovers from the plan/join executor rework were responsible.
JsonTablePlanScanNextRow() still reset and advanced the nested plan
itself, although JsonTablePlanNextRow() now does that; and
JsonTableResetNestedPlan() eagerly called JsonTableResetRowPattern()
(which evaluates the path) in addition to setting the reset flag that
makes JsonTablePlanNextRow() evaluate it again. Together these caused
the nested path to be evaluated multiple times per parent row.
Reduce JsonTablePlanScanNextRow() to advancing its own row pattern
iterator, and have JsonTableResetNestedPlan() only reset the transient
scan state (so a not-yet-advanced sibling still reads as NULL) while
deferring the actual path evaluation to the reset flag. The nested path
is now evaluated exactly once per parent row, as before the PLAN clause
feature; results are unchanged and are covered by the existing tests.
Reported-by: Thom Brown <thom@linux.com>
Discussion: https://postgr.es/m/CAA-aLv5U94KD4C%2BLhAPYcCeGvs1xBMngcS5oEkZHN9YWwXUHsA%40mail.gmail.com
Correct the JSON_TABLE synopsis to place the ON ERROR clause after the
PLAN clause, matching the grammar (the previous order could not be
typed). Replace a dangling reference to json_path_specification with
path_expression, the term the synopsis actually defines. Mark up the
PLAN DEFAULT keywords with <literal> and fix a couple of wording issues.
Also list OUTER before INNER consistently, including in the parent/child
join description, where OUTER is the default.
Author: Thom Brown <thom@linux.com>
Discussion: https://postgr.es/m/CAA-aLv5PGAFmAEpbhKQz8wppoOOTHfo-=LJb2sAB3974-9QtOw@mail.gmail.com
Make JSON_TABLE generated path names avoid collisions
generateJsonTablePathName() produced names of the form
"json_table_path_N" in the same namespace as user-supplied path and
column names, without checking whether the name was already in use.
When an unnamed NESTED path's generated name happened to match a
user-supplied path name that a PLAN clause referenced, two sibling paths
matched the same plan entry and one of them, together with its columns,
was silently dropped from the output.
Bump the counter until the generated name is unused, so a generated name
can no longer coincide with a user-supplied one. The still-uncovered
path is then correctly reported as not found in the plan.
The row pattern (root) path is named before the user-supplied column and
path names are collected, so when it is left unnamed its generated name
could not avoid them either, and a user column or path named like a
generated name, e.g.
SELECT * FROM JSON_TABLE(jsonb '1', '$'
COLUMNS (json_table_path_0 int PATH '$')) jt;
was rejected with a bogus "duplicate JSON_TABLE column or path name"
error. Collect the user-supplied names first and generate the row
pattern path's name afterwards, so that it avoids all of them. An
explicit root path name is still seeded into the namespace, so a column
duplicating it is still correctly rejected.
Reported-by: Thom Brown <thom@linux.com>
Discussion: https://postgr.es/m/CAA-aLv7aZGSExnbjJRw8eKkoXbu34TdoKLLA2gPye3aHjO5OSA@mail.gmail.com
Discussion: https://postgr.es/m/CAA-aLv5U94KD4C%2BLhAPYcCeGvs1xBMngcS5oEkZHN9YWwXUHsA%40mail.gmail.com
Fix JSON_TABLE PLAN deparse to keep parentheses around nested joins
get_json_table_plan() parenthesized the child of a parent/child
(OUTER/INNER) plan only when that child was a sibling (UNION/CROSS)
join, not when it was itself a parent/child join. A plan such as
PLAN (p0 OUTER (p1 INNER p11)) was therefore deparsed as
PLAN (p0 OUTER p1 INNER p11), which does not parse back to the same
plan tree -- a dump/restore hazard. Parenthesize the child whenever it
is not a bare path name, matching the logic already used for the
operands of sibling joins.
Reported-by: Thom Brown <thom@linux.com>
Discussion: https://postgr.es/m/CAA-aLv7aZGSExnbjJRw8eKkoXbu34TdoKLLA2gPye3aHjO5OSA@mail.gmail.com
86ab7f4c721d commit made the table-level ON ERROR clause serve as the default
ON ERROR for columns lacking their own, so that a top-level ERROR ON ERROR
turned per-column evaluation errors into hard errors.
The SQL standard does mandate this cascade, but introducing it should be
a deliberate, separately-documented change, so restore the previous
behavior for now. This also reverts the paired ruleutils.c logic that
deparsed a column's behavior against an ERROR default: that dropped an
explicit ERROR ON EMPTY from a dumped view, and otherwise emitted a
redundant NULL ON EMPTY.
Reported-by: Thom Brown <thom@linux.com>
Discussion: https://postgr.es/m/CAA-aLv7aZGSExnbjJRw8eKkoXbu34TdoKLLA2gPye3aHjO5OSA@mail.gmail.com
Peter Eisentraut [Tue, 14 Jul 2026 08:28:04 +0000 (10:28 +0200)]
Replace __builtin_types_compatible_p with _Generic
_Generic is the C11 standard equivalent (and superset) of
__builtin_types_compatible_p, so by replacing the latter with the
former, we can now rely on this working on all compilers, instead of
previously just with GCC-compatible ones. And we can drop a configure
test.
This affects StaticAssertVariableIsOfType() and
StaticAssertVariableIsOfTypeMacro() and indirectly unconstify() and
unvolatize().
Neither _Generic nor __builtin_types_compatible_p works in C++, so
this does not change that, but this adds an explicit code comment
about that.
There are some subtle behavior changes, but these do not affect cases
that are in use or likely to be useful. _Generic does lvalue
conversion on the controlling expression, which means it drops
top-level qualifiers and converts arrays to pointers.
__builtin_types_compatible_p on the other hand, supports arrays and
just ignores qualifiers. So before, StaticAssertVariableIsOfType(x,
const int) might have worked, but now it does not. (But note that it
would previously have succeeded even if x was a non-const int, so this
usage would always have been dubious.) Also,
StaticAssertVariableIsOfType(y, char[]) would have worked, but now you
need to write char *. (But this is not backward compatible, because
char * would previously not have succeeded.) Similarly, unconstify of
non-pointers, like unconstify(const int, x) would previously have
worked, but this was just by accident and never useful (you can just
assign directly, without a cast), and the C++ implementation rejects
non-pointers anyway. Some comments are added to explain this a bit.
There are no current uses affected by this.
Note that even though we have required C11 since f5e0186f865, we have
not made use of _Generic until now (except in an MSVC-specific case in
commit 59c2f03d1ec). This now raises the effective compiler
requirement on the trailing edge slightly from GCC 4.8 to GCC 4.9.
This in turn means that we effectively drop support for RHEL 7.
Author: Peter Eisentraut <peter@eisentraut.org> Co-authored-by: Thomas Munro <thomas.munro@gmail.com> Co-authored-by: Jelte Fennema-Nio <postgres@jeltef.nl>
Discussion: https://www.postgresql.org/message-id/flat/CA+hUKGL7trhWiJ4qxpksBztMMTWDyPnP1QN+Lq341V7QL775DA@mail.gmail.com
Michael Paquier [Mon, 13 Jul 2026 23:00:46 +0000 (08:00 +0900)]
Add recovery test for missing redo segment with backup_label
This commit adds a test case for early startup where a backup_label file
uses a checkpoint LSN and a redo LSN located in two different segments,
where the segment of the redo LSN is missing. This code has never been
covered, and is complex enough that a test case is going to be useful in
the long-term.
Nitin has proposed a more complex approach than what is added by this
commit, by forcing a reuse of the injection points to produce a split
between redo and checkpoint. This commit relies on the checkpoint and
redo LSNs generated by the first steps of the test, combined with a
generated backup_label, making the whole cheaper.
Author: Nitin Jadhav <nitinjadhavpostgres@gmail.com>
Author: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CAMm1aWZ9Tv=Wrx52_2Ppw+6ULf_twRZuQm=ZWLA_a-kXWykHkQ@mail.gmail.com
Peter Eisentraut [Mon, 13 Jul 2026 09:01:36 +0000 (11:01 +0200)]
Make blkreftable API use size_t/ssize_t consistently
It was using int for the input and output length, where size_t or
ssize_t would be more appropriate. It might not matter in practice,
but it makes the APIs consistent at all levels.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://www.postgresql.org/message-id/flat/f9aab072-0078-49e4-ab93-3b08086a4406@eisentraut.org
Peter Eisentraut [Mon, 13 Jul 2026 09:01:36 +0000 (11:01 +0200)]
Clean up readlink() return type
The return type of readlink() per POSIX is ssize_t, but most existing
callers use int, so fix that. Also fix the return type of the Windows
implementation to match.
In _pglstat64(), we neglected to handle the case where the output
buffer is not large enough and the result would be truncated. This
case actually can't happen, because the Windows pgreadlink()
implementation doesn't ever return that case, but adding this seems
good for consistency with _pgstat64(), which already had this check,
and in case pgreadlink() ever changes in this regard.
Some callers of readlink(), in particular _pglstat64(), assume that
errno == EINVAL means that the file was not a symlink. But Windows
pgreadlink() also sets EINVAL in other cases, in particular if the
buffer was too small. This could result in incorrect behavior, so
pick a different errno. (There might be other cases where EINVAL is
set inappropriately, but they are outside the theme of this patch.)
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://www.postgresql.org/message-id/flat/f9aab072-0078-49e4-ab93-3b08086a4406@eisentraut.org
Peter Eisentraut [Mon, 13 Jul 2026 09:01:36 +0000 (11:01 +0200)]
Some const qualifications added in passing
These are some cases in the vicinity of the patches to improve the
size_t/ssize_t use with POSIX file system APIs. For example, the
input buffers for write operations or the file names passed for error
reporting can be const.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://www.postgresql.org/message-id/flat/f9aab072-0078-49e4-ab93-3b08086a4406@eisentraut.org
Amit Kapila [Mon, 13 Jul 2026 03:57:47 +0000 (09:27 +0530)]
Don't show tables redundantly when their schema is published.
A table published both explicitly and through its schema (FOR TABLES IN
SCHEMA) was listed twice by \dRp+ and \d. Since publishing a schema
publishes its tables in full, the explicit entry's row filter has no
effect; suppress it when the same publication also publishes the table's
schema.
Author: Peter Smith <smithpb2250@gmail.com>
Co-author: Jim Jones <jim.jones@uni-muenster.de> Reviewed-by: Nisha Moond <nisha.moond412@gmail.com> Reviewed-by: vignesh C <vignesh21@gmail.com> Reviewed-by: Shlok Kyal <shlok.kyal.oss@gmail.com>
Discussion: https://postgr.es/m/CAHut%2BPvSOmRrQX%2BVrFYHtFipV9hM%3Dp99FeOwYCzkuU2BOaLu7Q%40mail.gmail.com
Michael Paquier [Mon, 13 Jul 2026 00:19:20 +0000 (09:19 +0900)]
Add recovery/startup test with backup_label and missing checkpoint segment
This test is able to trigger the following failure at the beginning of
recovery, that was not covered yet:
FATAL: could not locate required checkpoint record at %X/%X
Note that the backup used for the node created has its pg_wal/ removed,
which is why the segment expected is missing.
Tomas Vondra [Sat, 11 Jul 2026 13:14:50 +0000 (15:14 +0200)]
Shorten pg_attribute_always_inline to pg_always_inline
The pg_attribute_always_inline macro name is so long it forces pgindent
to format the code in strange ways. Which may incentivize patch authors
to either structure the code in strange ways (e.g. reorder prototypes),
use shorter names, etc. Neither is very desirable for code readability.
This shortens the name by removing the _attribute_ part. It also makes
it more consistent with pg_noinline, which does not have the _attribute_
part either.
Backpatched to all supported branches, to prevent conflicts when
backpatching other fixes. The backbranches however keep both the old and
new macro name, so that existing code keeps working.
Author: Andres Freund <andres@anarazel.de> Reviewed-by: Peter Geoghegan <pg@bowt.ie> Reviewed-by: Tomas Vondra <tomas@vondra.me>
Discussion: https://postgr.es/m/bqqdehahpoa36igpictuqyn2s2mexk3t3ehidh2ffd2slb35e5@rzgksuiszgbg
Backpatch-through: 14
Peter Eisentraut [Sat, 11 Jul 2026 12:38:33 +0000 (14:38 +0200)]
Fix for loop variables used with lengthof
lengthof returns type size_t, but most for loops used int as a loop
variable. Fix that. This avoids possible warnings about
signed/unsigned mismatches under higher warning levels. (The compiler
will likely optimize these loops beyond recognition, so this shouldn't
affect the generated code much.)
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/d639aede-209f-412b-927a-d38d4848b370%40eisentraut.org
Peter Eisentraut [Sat, 11 Jul 2026 12:38:33 +0000 (14:38 +0200)]
Fix for loop variables
A number of for loops used loop variables that did not match the type
of the end condition. This could lead to wraparound or
signed/unsigned mismatches. Probably none of these are a problem in
practice, but it's fragile code.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/d639aede-209f-412b-927a-d38d4848b370%40eisentraut.org
b46e1e54d078de allowed setting the VM while on-access pruning, but it
neglected to update the freespace map. Once the page was all-visible,
vacuum could skip it, leading to stale freespace map values and,
effectively, bloat. Fix it by updating the FSM if we updated the VM.
Andres Freund [Fri, 10 Jul 2026 17:25:08 +0000 (13:25 -0400)]
bufmgr: Fix order of operations in UnlockBufHdrExt
In c75ebc657ffc I (Andres) introduced UnlockBufHdrExt() which can set and
clear bits in the buffer state using CAS. Unfortunately I added bits before
subtracting them, which means that a bit that was both removed and set would
remain unset. Fix the order of operations.
The only known case where that is a problem is that BM_IO_ERROR would not
actually remain set.
It's unfortunately not trivial to add a decent, race-free, test to verify that
BM_IO_ERROR remains set. That's therefore left for the 20 cycle.
Reported-by: Yura Sokolov <y.sokolov@postgrespro.ru>
Discussion: https://postgr.es/m/ab0dcc9e-aba0-44e3-ac23-8d74c48888e6@postgrespro.ru
Backpatch-through: 19, where c75ebc657ffc went in
When doing a whole database repack, we build a list of tables to process
taking a lock on each. But because it's a regular transaction-scoped
lock, it's automatically released immediately after building the list
anyway, which makes it not very useful. (Also, we have three ways to
obtain a list of tables to repack, and only one of them acquired this
lock.) Remove that lock acquisition, as it's useless and inconsistent.
We acquire a lock properly afterwards (and recheck that the table can
still be repacked as indicated), so we don't need to do anything other
than drop that initial lock acquisition and harden the code in
repack_is_permitted_for_relation() against possible concurrent drops.
This is similar to how vacuum does it in get_all_vacuum_rels().
In order for this to work reliably, also change
repack_is_permitted_for_relation() to cope with the possibility of the
table going away partway through. Similarly, in ExecRepack(), be
prepared for what we believed to be a table or matview to now be
something else, and skip it without erroring out, by changing
try_table_open() to try_relation_open() and testing the relkind
separately.
While at it, replace one relation_close() call in get_tables_to_repack()
with table_close() to match the table_open() that opened the catalog.
Fix data checksum processing for temp relations and dropped databases
When building the list of temporary relations to wait for, the code
previously included temporary relations without storage, such as
temporary views, even though they are irrelevant to checksum
processing. As a result, enabling data checksums could wait for a
long-lived session that owned only a temporary view.
This commit fixes the issue by filtering temporary relations with storage
only, matching the existing behavior for non-temporary relations.
Also, when enabling data checksums online, the launcher assigns the
first worker to process shared catalogs and prevents later workers from
doing so. Previously, if that worker's database was dropped after it
had been selected for processing but before checksum processing began,
the worker failed without processing the shared catalogs, yet they were
still marked as processed. As a result, later workers skipped them, and
checksum enabling could complete successfully even though the shared
catalogs had never been processed.
This commit fixes the issue by marking shared catalogs as processed
only after a worker completes successfully.
pg_stat_progress_data_checksums uses -1 as a sentinel value that is
displayed as NULL for progress counters. However, after
pgstat_progress_start_command() initialized all progress counters to
zero, data checksum progress did not reset those counters to -1.
As a result, some counters could incorrectly appear as zero instead
of NULL. For example, workers could report zero database counters,
and the disabling launcher could report zero relation and block counters.
Also, blocks_done was not reset when a worker started processing
a new relation fork. As a result, it could temporarily exceed blocks_total
or report a stale value for an empty relation fork.
Fix this by initializing the data checksum progress counters to -1
when progress reporting starts for both launcher and worker processes.
Also reset blocks_done together with blocks_total when starting each
relation fork.
postgres_fdw: Mark statistics import helpers as static
The set_*_arg helper functions in postgres_fdw.c are declared
static, but their definitions omitted the static keyword. Add it to
make their file-local scope explicit and keep the declarations and
definitions consistent.
Peter Eisentraut [Fri, 10 Jul 2026 08:08:21 +0000 (10:08 +0200)]
Forbid FOR PORTION OF on views with INSTEAD OF triggers
Previously, an attempt to use these features together caused a crash.
Oversight of commit 8e72d914c528.
Tests are added also to show that the check for this should be in the
rewriter, not the parser, as an earlier patch version suggested.
Author: Aleksander Alekseev <aleksander@tigerdata.com>
Author: Paul A. Jungwirth <pj@illuminatedcomputing.com>
Discussion: https://www.postgresql.org/message-id/flat/CAJ7c6TME%2Bix6VRf-2TPnVTsj8qn_hy6sYAOmMhZEivwsu2wS6g%40mail.gmail.com
postgres_fdw: Remove SPI from postgresImportForeignStatistics.
Previously, this function imported remote statistics by executing SQL
functions like pg_restore_relation_stats and pg_restore_attribute_stats
via SPI (in read-write mode). As the SQL functions take a schema name
and a relation name as two separate arguments, rather than a single OID
argument, if the containing schema was concurrently renamed, the
callback function would throw an error like this:
ERROR: schema "foo" does not exist
To fix, 1) provide new interface functions to import remote statistics
that are directly callable from FDWs and take a single OID, and 2)
modify the callback function to use the interface functions instead when
importing remote statistics.
For #1, this commit does a bit of refactoring to
relation_statistics_update and attribute_statistics_update, which are
the workhorse functions for pg_restore_relation_stats and
pg_restore_attribute_stats respectively: since they also take a schema
name and a relation name, separate the guts of them into new functions
so that they take a single OID and are callable not only from the
workhorse functions but from the interface functions introduced by #1.
truncate_query_log() returns NULL when statement truncation is
disabled or the supplied query does not need to be truncated. Therefore,
callers do not need to check log_statement_max_length before calling
it.
Remove the redundant checks and let truncate_query_log() make the
decision in one place. This simplifies the statement and duration
logging code without changing its behavior.
Author: Jim Jones <jim.jones@uni-muenster.de> Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwFOV+7nOdfoO=kfVH=-fRA9aQE1YcHHLYty3nfQ9rQ4RA@mail.gmail.com
log_statement_max_length limits the statement text logged by
log_statement and duration logging. However, the prepared statement
shown in the DETAIL message for EXECUTE was still logged in full,
allowing very large prepared queries to bypass the limit.
Apply the same truncation to the prepared statement text in
errdetail_execute(), making the setting behave consistently across the
main log message and its associated DETAIL output.
Also append an ellipsis to truncated statement text so users can easily
tell when truncation has occurred. With this change, a limit of zero
logs only the ellipsis, indicating that the entire statement text was
truncated.
Finally, avoid scanning the entire query string just to determine
whether truncation is needed. Use strnlen() with sufficient lookahead
for multibyte character handling, then use pg_mbcliplen() to ensure
truncation never splits a multibyte character.
Author: Jim Jones <jim.jones@uni-muenster.de> Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwFOV+7nOdfoO=kfVH=-fRA9aQE1YcHHLYty3nfQ9rQ4RA@mail.gmail.com
The spinlock is unnecessary because the counter is only ever
accessed with WaitEventCustomLock held exclusively. This commit
removes the struct definition and converts WaitEventCustomCounter
to a pointer to an integer in shared memory.
Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/ak8MeTS9QBmz6DKt%40nathan
libpq: Make error checks in the new buffer draining code more robust
Check explicitly for pqsecure_read() returning an error. It shouldn't
fail, and we would've caught it in the check for a short read, but
better to be explicit so that the error message is more informative.
We also shouldn't update 'inEnd' when the read fails, although that
too is just pro forma as we will bail out and close the connection on
error.
Reported-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://www.postgresql.org/message-id/34844e8c-267c-4daf-b1e0-f26059a4a7d3@eisentraut.org
Backpatch-through: 14
ssl: Include limits.h to get INT_MAX when using LibreSSL
When compiling against OpenSSL, the <limits.h> header is indirectly
included via openssl/ossl_typ.h from openssl/conf.h, but the LibreSSL
version of ossl_typ.h does not include <limits.h> which cause compiler
failure due to missing symbol (since ffd080d94fe). Fix by explicitly
including <limits.h>.
Author: Daniel Gustafsson <dgustafsson@postgresql.org>
Discussion: https://www.postgresql.org/message-id/6A9E7815-BD5A-4C31-A515-48159823406B@yesql.se
Backpatch-through: 14
This patch adds the PLAN clauses for JSON_TABLE, which allow the user
to specify how data from nested paths are joined, allowing
considerable freedom in shaping the tabular output of JSON_TABLE.
PLAN DEFAULT allows the user to specify the global strategies when
dealing with sibling or child nested paths. This is often sufficient
to achieve the necessary goal, and is considerably simpler than the
full PLAN clause, which allows the user to specify the strategy to be
used for each named nested path.
Path names may be attached to the row pattern and to each NESTED path
using AS. Unlike the SQL/JSON standard, which requires a name for every
NESTED path when a PLAN clause is present, PostgreSQL does not require
them and generates a name for any path left unnamed; a specific PLAN()
can only reference paths by name, so unnamed paths it must mention are
reported as not covered by the plan.
Author: Nikita Malakhov <n.malakhov@postgrespro.ru> Co-authored-by: Nikita Glukhov <n.gluhov@postgrespro.ru> Co-authored-by: Teodor Sigaev <teodor@sigaev.ru> Co-authored-by: Oleg Bartunov <obartunov@gmail.com> Co-authored-by: Alexander Korotkov <aekorotkov@gmail.com> Co-authored-by: Andrew Dunstan <andrew@dunslane.net> Co-authored-by: Amit Langote <amitlangote09@gmail.com> Co-authored-by: Anton Melnikov <a.melnikov@postgrespro.ru> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com> Reviewed-by: Andrew Alsup <bluesbreaker@gmail.com> Reviewed-by: Erik Rijkers <er@xs4all.nl> Reviewed-by: Zhihong Yu <zyu@yugabyte.com> Reviewed-by: Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com> Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Reviewed-by: Justin Pryzby <pryzby@telsasoft.com> Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de> Reviewed-by: jian he <jian.universality@gmail.com> Reviewed-by: Vladlen Popolitov <v.popolitov@postgrespro.ru>
Discussion: https://postgr.es/m/cd0bb935-0158-78a7-08b5-904886deac4b@postgrespro.ru
Discussion: https://postgr.es/m/20220616233130.rparivafipt6doj3@alap3.anarazel.de
Discussion: https://postgr.es/m/abd9b83b-aa66-f230-3d6d-734817f0995d%40postgresql.org
Discussion: https://postgr.es/m/CA+HiwqE4XTdfb1nW=Ojoy_tQSRhYt-q_kb6i5d4xcKyrLC1Nbg@mail.gmail.com
Discussion: https://postgr.es/m/CAN-LCVP7HXmGu-WcinsHvdKqMGEdv=1Y67H4U58F6Y=Q0M5GyQ@mail.gmail.com
Specifying a locking clause (FOR UPDATE/SHARE) that names a
GRAPH_TABLE alias currently results in an unhelpful "unrecognized RTE
type: 8" error. This commit explicitly prohibits specifying a locking
clause on a GRAPH_TABLE alias, raising a more user-friendly error
instead.
(Locking clause support for GRAPH_TABLE could be added as a separate
feature in the future.)
In transformLockingClause(), there was a comment that listed all the
RTE kinds it did not want to process, but that list was already
outdated about what RTE kinds actually exist. Rather than keeping
that up-to-date, just say "all other".
Michael Paquier [Thu, 9 Jul 2026 05:23:42 +0000 (14:23 +0900)]
Reject incorrect range_bounds_histograms in stats import functions
pg_restore_attribute_stats() and pg_restore_extended_stats()
(expressions) can handle a STATISTIC_KIND_BOUNDS_HISTOGRAM value, but
did not check its shape when importing, especially regarding:
- Empty ranges.
- Unsorted elements.
These properties are respected by ANALYZE in compute_range_stats(), when
computing a histogram for a [multi]range, and by the planner when
reading the data from the stats catalogs. The effects of importing data
with these properties depend on the compilation options:
- A assertion would be hit, if enabled.
- A non-sensical value that would make the load of the stats fail.
- A failure is hit if an empty range is loaded.
While the owner of the table or the one with MAINTAIN rights is
responsible for the stats data inserted, buggy data makes little sense
if their load is going to fail. This commit adds a validation step to
match what compute_range_stats() expects.
This issue would be unlikely hit in practice, so no backpatch is done.
Author: Ewan Young <kdbase.hack@gmail.com>
Discussion: https://postgr.es/m/CAON2xHNM809WLR_g0ymKgU-tWxtbyH1Xvh4fqzRqy9YP2A59pg@mail.gmail.com
Amit Kapila [Thu, 9 Jul 2026 02:39:07 +0000 (08:09 +0530)]
Doc: Clarify sequence synchronization commands.
Explain more accurately how REFRESH SEQUENCES differs from REFRESH
PUBLICATION in ALTER SUBSCRIPTION, and note that CREATE SUBSCRIPTION uses
copy_data = true (the default) to copy initial sequence values.
Author: Peter Smith <smithpb2250@gmail.com>
Author: Amit Kapila <amit.kapila16@gmail.com>
Backpatch-through: 19
Discussion: https://postgr.es/m/CAHut+PtFkGvZNihGRDoghWNKMfJufEpR9+thbG_8qPQ7RyVN4w@mail.gmail.com
Richard Guo [Thu, 9 Jul 2026 00:44:56 +0000 (09:44 +0900)]
Add an enable_groupagg GUC parameter
We've long had enable_hashagg to discourage hashed aggregation, but
there was no equivalent for grouping that works by reading presorted
input. That's handy when investigating planner cost misestimates, and
as an escape hatch when a sorted grouping plan is chosen over a much
cheaper hashed one.
enable_groupagg (on by default) covers the GroupAggregate and Group
nodes, the sort-based Unique step used for DISTINCT and semijoin
unique-ification, and the sorted mode of SetOp. It isn't a hard
switch; it only bumps disabled_nodes, so plans with no other choice
are still produced.
Several regression and module tests had been setting enable_sort off,
and one enable_indexscan off, only to force a hashed plan. Use
enable_groupagg there instead, where that was the real intent. In
union.sql this also lets us test the hashed UNION path, which we had
no way to reach before.
Author: Tatsuro Yamada <yamatattsu@gmail.com> Co-authored-by: Richard Guo <guofenglinux@gmail.com> Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/CAOKkKFvYHSEsFazkrf9bRH14p-H27XMaqbZfRYjS6EHBruvZMQ@mail.gmail.com
David Rowley [Wed, 8 Jul 2026 23:50:18 +0000 (11:50 +1200)]
Tidy up datatype usage in test_random_offset_operations()
I had previously made "seed" a uint64, as that's what pg_prng_seed()
expects to get. However, GetCurrentTimestamp() and PG_GETARG_INT64()
both return int64, so there was a mismatch in signedness.
There are no bugs being fixed here, but it seems cleaner to make the
variable int64 and cast to unsigned when passing to pg_prng_seed(). This
means we no longer have to use the INT64_FORMAT specifier on the unsigned
type to format the seed of the failing test correctly to allow a user to
use the same seed from SQL when trying to recreate any failures manually.
The most important thing to ensure is correct here is that users can
specify the full range of seed values that could be automatically
selected by GetCurrentTimestamp() so that we can recreate any failures
from runs where the seed was auto-selected. This still works as
expected when using the int64 type.
In passing, adjust a comment that was a little misleading.
Reported-by: Peter Eisentraut <peter@eisentraut.org>
Author: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/08fa9f01-373e-4cb4-9650-ad20517e2de3@eisentraut.org
Don't create SPLIT/MERGE partitions as internal relations
The new partitions built for ALTER TABLE ... SPLIT PARTITION and
ALTER TABLE ... MERGE PARTITIONS are created at the explicit request of
the user, just like a plain CREATE TABLE. createPartitionTable() passes
is_internal=true to heap_create_with_catalog(), while createTableConstraints()
does the same to StoreAttrDefault() and AddRelationNewConstraints().
Pass is_internal=false in all these places instead, so that object-access
hooks treat them as user-requested objects. The is_internal flag is intended
for objects created as internal implementation details, such as a transient
heap built during CLUSTER.
While at it, pass 0 rather than PERFORM_DELETION_INTERNAL to the
performDeletionCheck() calls that pre-check the drop eligibility of the
old partitions, to match the subsequent performDeletion(). The flag has
no functional effect on performDeletionCheck(), but change this for code
consistency.
refint was sample code from the pre-built-in-FK era and has long
been documented as superseded by the built-in foreign key
mechanism. Recent fixes (see commits b0b6196386, 8cfbdf8f4d, 260e97733b, 611756948e, 1fbe2066dc, and 1541d91d1c) made it clear
that the code has more issues than its sample-code value justifies.
Dean Rasheed [Wed, 8 Jul 2026 19:46:25 +0000 (20:46 +0100)]
Fix RETURNING OLD with BEFORE UPDATE trigger and concurrent update.
When executing an UPDATE with a RETURNING clause on a table with a
BEFORE UPDATE row trigger, the computation of the OLD values in the
RETURNING list was incorrect if the target tuple was concurrently
updated by another session, at isolation level READ COMMITTED.
The problem was that the trigger code would lock the target tuple,
waiting for the other session to commit, and then fetch the updated
target tuple, but ExecUpdate() would not realise that the target tuple
had changed, and use the outdated target tuple for computing OLD
values. Fix by having ExecUpdate() check the TM_FailureData from
trigger execution and re-fetch the target tuple if necessary.
Re-fetching the target tuple like this is a little inefficient, but
probably negligible compared to the trigger execution and update. A
better long-term fix might be to move the EPQ code out of trigger.c,
and let ExecUpdate() handle it, like ExecMergeMatched() does, but that
would likely mean changing the trigger API, which seems a bit much for
back-patching.
Backpatch to v18, where support for RETURNING OLD/NEW was added.
Move WAIT_FOR_WAL_* wait events from Client to IPC class
WAIT_FOR_WAL_FLUSH, WAIT_FOR_WAL_REPLAY, and WAIT_FOR_WAL_WRITE were
placed in the WaitEventClient class. But WaitEventClient is about
waiting for a socket to become readable or writable, while these events
have other delay sources as well: local fsync and local replay, which
may be disk- or CPU-bound. WaitEventIPC is a better fit, so move them
there.
When changing the expression of a generated column via ALTER TABLE
ALTER COLUMN SET EXPRESSION, objects that depend on the column via
indirect whole-row references (such as CHECK constraints, indexes)
must be handled specially, because technically pg_depend does not
contain such dependencies, see
recordDependencyOnSingleRelExpr->find_expr_references_walker.
This is a fix for commit f80bedd52, "Allow ALTER COLUMN SET EXPRESSION
on virtual columns with CHECK constraints".
Author: jian he <jian.universality@gmail.com> Co-authored-by: Peter Eisentraut <peter@eisentraut.org> Reported-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com> Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com> Reviewed-by: solai v <solai.cdac@gmail.com> Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
Discussion: https://www.postgresql.org/message-id/flat/CAJTYsWXOkyeDVbzymWc9sKrq7Y_MUv6XJXN4H9GfsBOPd3NJ+w@mail.gmail.com
Refactor how some aux processes advertise their ProcNumber
This moves the responsibility of setting the ProcGlobal->walwriterProc
and checkpointerProc fields into InitAuxiliaryProcess. Also switch to
the same pattern to advertise the autovacuum launcher's ProcNumber,
replacing the ad hoc av_launcherpid field in shared memory. This can
easily be extended to other aux processes in the future, if other
processes need to find them.
Switch to pg_atomic_uint32 for the fields. Seems easier to reason
about than volatile pointers. There was some precedence for that, as
were already using pg_atomic_uint32 for the procArrayGroupFirst and
clogGroupFirst fields, which also store ProcNumbers.
Todo: We could also replace WalRecv->procno with this, but that's a
little more code churn so I left that for the future.
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/818bafaf-1e77-4c78-8037-d7120878d87c@iki.fi
Remove sketchy TerminateThread() call on Ctrl-C on Windows
When pg_dump or pg_restore --jobs N is interrupted with Ctrl-C on
Windows, we cancel all queries, but we don't want the cancellations to
be reported as errors to the user in the short time before the whole
process exits. That was previously achieved by calling
TerminateThread() on each worker thread before sending the cancel
message, but that doesn't appear to be 100% safe: the implementations
of write() and the socket calls inside PQcancel() might acquire user
space locks that were held by the terminated threads. (write()
certainly does that.)
Instead of silencing the threads in such a sketchy way this now sets a
volatile flag before sending any cancel requests that tells the
threads to not log errors anymore. (Instead of a volatile, it would be
better to use an atomic operation here, but that has to wait until we
add support for atomics on the frontend.)
Note that this also stops using pg_fatal() and exit to exit() from
workers on failure and instead use pg_log_error combined with
exit_nicely. If a query fails in a worker we want it to kill the
worker not the whole process. On Unix that's currently the same thing,
but on Windows workers are threads.
Author: Jelte Fennema-Nio <postgres@jeltef.nl> Reviewed-by: Bryan Green <dbryan.green@gmail.com> Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://www.postgresql.org/message-id/DJPQS3FYSD4U.3DBTXA6U8IQ0Q@jeltef.nl
David Rowley [Wed, 8 Jul 2026 13:11:42 +0000 (01:11 +1200)]
Introduce bms_offset_members() function
Effectively, a function to bitshift members by the specified number of
bits. We have various fragments of code doing this manually with a
bms_next_member() -> bms_add_member() loop. We can do this more
efficiently in terms of CPU and memory allocation by making a new
Bitmapset and bitshifting in the words of the old set to populate it.
Author: David Rowley <dgrowleyml@gmail.com> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Reviewed-by: Greg Burd <greg@burd.me>
Discussion: https://postgr.es/m/CAApHDvq=eEdw2Qp+aSzSOtTSe+h0fnVQ55CcTNqBkLDYiRZmxw@mail.gmail.com
Add hints for sequence synchronization permission warnings
Sequence synchronization reports insufficient privileges on publisher
and subscriber sequences, but the warnings do not indicate which role
needs which privilege. This makes common configuration mistakes harder
to diagnose.
Add HINT messages for these warnings. Publisher-side warnings suggest
granting SELECT to the role used for the replication connection.
Subscriber-side warnings suggest granting UPDATE to the subscription
owner when run_as_owner is enabled. Otherwise, the worker runs as the
sequence owner, so no useful GRANT hint can be provided.
The documentation previously said that pg_get_sequence_data() returns
a row of NULL values if the sequence does not exist or if the current
user lacks privileges on it. This was incomplete and could be misleading.
A nonexistent relation name is rejected during regclass input conversion,
while the function returns NULLs for a nonexistent relation OID and
several other cases.
This commit clarifies that the function returns NULLs when the specified
relation OID does not exist, the relation is not a sequence, the current
user lacks SELECT privilege on the sequence, the sequence belongs to
another session's temporary schema, or it is an unlogged sequence on
a standby server.
Resolve unknown-type literals in property expressions
When a string literal is provided as a property expression, the data
type of the property was set to "unknown", which may lead to various
failures when the property is used in GRAPH_TABLE or when its data
type is compared against other properties with the same name. To fix
this, call resolveTargetListUnknowns() on the targetlist of new
properties being added to resolve unknown type literals.
Fix replace_property_refs() ignoring the root of expression tree
replace_property_refs() called expression_tree_mutator() with the root
of the expression tree as the input node. But
expression_tree_mutator() does not call the mutator function on the
root node, so the root node remains unchanged. If the root node is a
property reference or a lateral reference -- the two node kinds that
replace_property_refs_mutator() rewrites -- it is returned unchanged.
Modules after the rewriter do not know about property reference nodes,
resulting in "ERROR: unrecognized node type: 63". Since varlevelsup
of lateral references is not incremented, they are not resolved
correctly in the planner, leading to many different symptoms. Fix
this by calling replace_property_refs_mutator() directly from
replace_property_refs(), similar to how other mutator functions do.
The only case when a property reference or a lateral reference can be
the root of a GRAPH_TABLE expression tree is when it is a bare
property reference or a bare lateral reference in the WHERE clause.
The COLUMNS clause is passed to replace_property_refs() as a
targetlist. Every other expression has at least one expression node
covering the property reference or a lateral reference in the
expression tree. That explains why this bug was not seen so far.
Michael Paquier [Wed, 8 Jul 2026 05:34:09 +0000 (14:34 +0900)]
injection_points: Switch wait/wakeup to rely on atomics
This change switches the implementation of wait and wakeups in the
module injection_points to not rely anymore on condition variables,
using a more primitive implementation based on atomics. The former
implementation required a PGPROC, making it impossible to inject waits
in the postmaster or during authentication. A couple of use cases have
popped up for these in the past, where this would have become handy.
The loop in the wait callback that relied on a condition variable is
replaced by an atomic counter, whose check increases over time in an
exponential manner (starts at 10us for quick responsiveness, up to
100ms).
This change may be backpatched at some point depending on how much
testing coverage is wanted. Let's limit ourselves to HEAD for now,
checking things first with the buildfarm.
Creating a wait still requires the SQL interface. We are looking at
expanding that with an alternative implementation, so as early startup
or authentication waits would become possible. This refactoring piece
is mandatory to achieve this goal.
Michael Paquier [Wed, 8 Jul 2026 04:22:50 +0000 (13:22 +0900)]
Fix more Datum conversion inconsistencies
This is a continuation of the work done in ac59a90bef45. The
*GetDatum() macros for output should match with what the SQL functions
use as DatumGet*() in input.
Aleksander has spotted some of the areas patched here, for pageinspect.
I have spotted the rest while digging into the state of the tree.
There is no behavior change after this commit, since all the affected
values are small enough that the signed bit is never used.
Author: Aleksander Alekseev <aleksander@tigerdata.com>
Author: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/afLsqRjVqKK8hhKk@paquier.xyz
doc: Clarify COPY FROM WHERE expression restrictions
Commit aa606b9316a disallowed generated columns in COPY FROM WHERE
expressions, and commit 21c69dc73f9 disallowed system columns.
However, the COPY reference page still mentions only the restriction
on subqueries.
Update the documentation to also list generated columns and system
columns as unsupported in COPY FROM WHERE expressions.
Backpatch the generated-column documentation change to all supported
versions. Backpatch the system-column documentation change to v19,
where that restriction was introduced.
pg_recvlogical: send final feedback on SIGINT/SIGTERM shutdown
Previously, when pg_recvlogical exited due to SIGINT or SIGTERM,
it could terminate without sending final feedback for the last decoded
changes it had already written locally. So, if pg_recvlogical was
restarted afterwards, the server-side logical replication slot could
still point behind those changes, causing them to be sent again.
Make pg_recvlogical send final feedback once more during SIGINT/SIGTERM
shutdown, before sending CopyDone. This gives the server one more chance
to advance the slot far enough to avoid resending already-written data,
so users are less likely to see duplicate decoded output after stopping
and restarting pg_recvlogical.
This remains a best-effort improvement rather than a guarantee. Depending
on when the signal arrives, pg_recvlogical can already have written
decoded output that the server cannot yet safely treat as confirmed, so a
later restart can still receive duplicate data.
Richard Guo [Wed, 8 Jul 2026 03:02:21 +0000 (12:02 +0900)]
Tighten nullingrels checks for outer joins
When fixing up the targetlist and qpqual of an outer join, we must
account for the effects of the outer join. Vars and PHVs appearing
there are logically above the join, so they should have nullingrels
equal to the input Vars/PHVs' nullingrels plus the bit added by the
outer join.
Determining the effects of the outer join can be tricky when the join
has been commuted with another one per outer join identity 3. In this
case, the Vars/PHVs in the join's targetlist and qpqual should have
the same nullingrels that they would if the two joins had been done in
syntactic order. Unfortunately, in setrefs.c, we don't have enough
information to identify what that should be, so we have to use
superset nullingrels matches instead of exact ones.
However, we can tighten the check somewhat. Currently, we check
whether the jointype is JOIN_INNER and use NRM_SUPERSET if it is not.
We can improve this by checking whether the Join node has non-empty
ojrelids and using NRM_SUPERSET only in that case. This allows us to
perform exact matches in more situations.
To support this, we record the outer-join relids in Join plan nodes.
This information can also improve EXPLAIN (RANGE_TABLE) output by
showing which outer-join relids are completed by each Join plan node.
We may discover additional uses for this information in the future.
Author: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/CAMbWs482_DFHzQ079ZPp6c8UvmFdz3Jj+4K8tVRu9g2Bw34NPA@mail.gmail.com
Richard Guo [Wed, 8 Jul 2026 03:01:44 +0000 (12:01 +0900)]
Remove nrm_match parameter from fix_upper_expr
With the changes in the previous commit, we can now use exact
nullingrels matches in all cases when fixing up expressions of
upper-level plan nodes that are not joins. Therefore, we can remove
the nrm_match parameter from fix_upper_expr(), along with the
corresponding field in fix_upper_expr_context.
Author: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/CAMbWs482_DFHzQ079ZPp6c8UvmFdz3Jj+4K8tVRu9g2Bw34NPA@mail.gmail.com
Richard Guo [Wed, 8 Jul 2026 03:00:36 +0000 (12:00 +0900)]
Use exact nullingrels matches for NestLoopParams
We have been using NRM_SUBSET to process NestLoopParams in setrefs.c,
because Vars or PHVs in NestLoopParam expressions may previously have
had nullingrels that were just subsets of those in the Vars or PHVs
actually available from the outer side.
Since 66e9df9f6, identify_current_nestloop_params ensures that any
Vars or PHVs seen in a NestLoopParam expression have nullingrels that
include exactly the outer-join relids that appear in the outer side's
output and can null the respective Var or PHV. As noted in that
commit's message, we can now safely use NRM_EQUAL to process
NestLoopParams in setrefs.c.
This patch makes that change and removes the definition of NRM_SUBSET,
along with all remaining checks for it, since it is no longer used.
Author: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/CAMbWs482_DFHzQ079ZPp6c8UvmFdz3Jj+4K8tVRu9g2Bw34NPA@mail.gmail.com
Richard Guo [Wed, 8 Jul 2026 00:38:31 +0000 (09:38 +0900)]
Propagate stadistinct through GROUP BY/DISTINCT in subqueries and CTEs
Previously, examine_simple_variable() would return early when a
subquery or CTE used GROUP BY or DISTINCT. It could detect uniqueness
for single-column cases, but for multi-column GROUP BY or DISTINCT,
selectivity estimation fell back on 1/DEFAULT_NUM_DISTINCT (1/200).
This produced wildly inaccurate estimates for filters and joins on
such columns, often leading the planner to choose nested loop joins
where hash joins would be far better. This was a significant factor
in poor TPC-DS benchmark performance.
For DISTINCT or GROUP BY key columns that are simple Vars, we now
recurse into the subquery to obtain the base table's stadistinct,
which remains valid after grouping (the set of distinct values is
preserved). However, MCV frequencies, histograms, and correlation
data are not valid because GROUP BY and DISTINCT change the frequency
distribution of key columns. So we strip all stats slots from the
copied stats tuple, causing callers like var_eq_const() to use the
1/ndistinct estimate instead. If stadistinct is stored as a negative
value (a fraction of the base table's row count), we convert it to an
absolute count so it is not misinterpreted relative to the subquery's
output row count.
stanullfrac is adjusted too, since grouping collapses NULLs. For a
single grouping key, at most one NULL group survives, so the null
fraction is 1/(ndistinct+1). For multiple grouping keys the null
fraction depends on the joint distribution of the keys, which we don't
have, so we approximate it as zero; NULLs collapse far more
aggressively than non-NULLs, so the real fraction is well below the
base table's, and erring low keeps estimates on the hash-join-favoring
side.
Non-key columns (e.g., aggregate outputs) continue to get no stats,
same as before.
Author: Richard Guo <guofenglinux@gmail.com> Reviewed-by: wenhui qiu <qiuwenhuifx@gmail.com>
Discussion: https://postgr.es/m/CAMbWs49rWYrecgreDhKsfx3VSDW=qo35s+iAmgGu=wpARrM8_g@mail.gmail.com
Commit dcb00495236 accidentally changed the final expanded query's
condition to > 2 while rewriting the example into SQL operator notation.
The original query and the preceding rewritten forms all use >= 2,
and view expansion should preserve that qualification. This commit
changes the final condition from > 2 to >= 2.
Richard Guo [Tue, 7 Jul 2026 23:46:43 +0000 (08:46 +0900)]
Fix EXPLAIN failure when deparsing SQL/JSON aggregates
If an expression containing an aggregate is evaluated above the plan
node that computes the aggregate, as happens with window functions or
with expressions postponed to above the final sort, setrefs.c replaces
the Aggref or WindowFunc with a Var referencing the lower node's
output. For SQL/JSON aggregates such as JSON_ARRAYAGG and
JSON_OBJECTAGG, deparsing the containing JsonConstructorExpr then
failed with "invalid JsonConstructorExpr underlying node type", since
get_json_agg_constructor() did not expect a Var there.
Fix by resolving the Var back to the underlying Aggref or WindowFunc
and deparsing the constructor as if the aggregate were computed at the
current node. The JsonConstructorExpr retains the RETURNING clause
and the ABSENT/NULL ON NULL and WITH UNIQUE options, and the arguments
come from the resolved aggregate, so the original JSON aggregate
syntax is reproduced in full. This mirrors how get_agg_expr() already
looks through such a Var when deparsing a combining aggregate.
Reported-by: Thom Brown <thom@linux.com>
Author: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/CAA-aLv5QYTaMOk=Qhv6cgwceeHETZV8YJvWZ_rH+yVZCuchATA@mail.gmail.com
Backpatch-through: 16
Deduplicate metapage sanity checks in _bt_gettrueroot()
Replace the metapage sanity checks in _bt_gettrueroot() with a call to
_bt_getmeta(), which does exactly the same checks.
Author: Chao Li <li.evan.chao@gmail.com> Reviewed-by: Neil Chen <carpenter.nail.cz@gmail.com>
Discussion: https://www.postgresql.org/message-id/CAEoWx2nisjqs4iC9o4Hu7-Ab767=cMZZzmhBGb8SaQtMMmVqPQ@mail.gmail.com
GiST index killitems feature misbehaves for single-page GiST index,
i.e. one that has only a root page. This is caused by the GiST scan's
curBlkno variable not being initialized for the first-to-scan page,
which is the root page. Fix this by moving the initializing of
curBlkno into gistScanPage(), where we also set the related curPageLSN
variable.
Commit 377b7ab145 actually added a regression test for this already,
but it merely noted that it's not working and memorized the result
where the items were not killed. Now they are, as the test shows.
This has been broken all along, but since it's just a very minor
performance issue on tiny tables, I didn't bother backpatching it.
Rename register_unlink_segment() to register_unlink_tombstone()
Only "tombstone" files (first segment of main fork) are unlinked after
checkpoints, so rename the function and remove the extra arguments to
make that more clear.
Additionally, add an assertion in mdunlinkfiletag() that the FileTag
only contains expected values.
Author: Matthias van de Meent <boekewurm+postgres@gmail.com> Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://www.postgresql.org/message-id/CAEze2WjfP95SL_Hsu7GzYXLnQyEsT49zOnNvbY_mBLCFiQra1g@mail.gmail.com
Adding a GRANT caused pg_dump to emit a useless REVOKE + GRANT of owner
privileges, as seen in a dump of the regression database:
REVOKE ALL ON PROPERTY GRAPH graph_rls_schema.cabinet FROM nm;
GRANT ALL ON PROPERTY GRAPH graph_rls_schema.cabinet TO nm;
GRANT ALL ON PROPERTY GRAPH graph_rls_schema.cabinet TO PUBLIC;
For normal dumps, this has no functional consequences. For --no-owner
restores, the extra statements may fail or locate unrelated users of the
destination cluster.
The problem was pg_dump assuming NULL relacl implies acldefault('r'),
the default for TABLE. Fix by teaching acldefault() to retrieve the
PROPERTY GRAPH default ACL. So pg_dump can still dump from 19beta1, use
acldefault('g') for v20+ only. For v19, use a hard-coded snapshot of
the v19 default.
information_schema.pg_property_graph_privileges also misused
acldefault('r'), but its "c.prtype IN ('SELECT')" predicate compensated
for it. Switch to the new acldefault('g') for clarity. Bump catversion
since a new view won't work with old binaries. Back-patch to v19, which
introduced PROPERTY GRAPH.
This commit cleans up volatile qualifiers that fit the below
criteria:
* Accesses to shared memory protected by a spinlock or LWLock.
Before commit 0709b7ee72, callers had to use volatile when
accessing spinlock-protected shared memory. Since spinlock
acquire/release became compiler barriers, and because LWLocks
provide the same guarantee, that is no longer necessary. These
either predate that change or were cargo-culted from code that did.
* Pointers used only to find the address of a member. The volatile
qualifier only affects accesses made by dereferencing the pointer,
so it is unnecessary there.
* Accesses to struct members that are marked volatile in the struct
definition. There's no need to mark these pointers volatile,
either.
* Leftovers from removed PG_TRY blocks. These were marked volatile
to protect a value that is modified inside a PG_TRY block, but the
PG_TRY has since been removed.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://postgr.es/m/akQ5eJR1tCCXme8e%40nathan
libpq: Drain all pending bytes from SSL/GSS during pqReadData()
The previous commit strengthened a workaround for a hang when large
messages are split across TLS records/GSS tokens. Because that
workaround is implemented in libpq internals, it can only help us when
libpq itself is polling on the socket. In nonblocking situations,
where the client above libpq is expected to poll, the same bugs can
show up.
As a contrived example, consider a large protocol-2.0 error coming
back from a server during PQconnectPoll(), split in an odd way across
two records:
-- TLS record (8192-byte payload) --
EEEE[...repeated a total of 8192 times]
-- TLS record (8193-byte payload) --
EEEE[...repeated a total of 8192 times]\0
The first record will fill the first half of the libpq receive buffer,
which is 16k long by default. The second record completely fills the
last half with its first 8192 bytes, leaving the terminating NULL in
the OpenSSL buffer. Since we still haven't seen the terminator at our
level, PQconnectPoll() will return PGRES_POLLING_READING, expecting to
come back when the server has sent "the rest" of the data. But there
is nothing left to read from the socket; OpenSSL had to pull all of
the data in the 8193-byte record off of the wire to decrypt it.
A real server would probably not split up the records this way, nor
keep the connection open after sending a fatal connection error. But
servers that regularly use larger TLS records can get the libpq
receive buffer into the same state if DataRows are big enough, as
reported on the list. While the PostgreSQL server doesn't use larger
TLS records like that, other non-PostgreSQL servers that implement the
wire protocol are known to do that, as well as proxies that sit
between the server and the client
This is a layering violation. libpq makes decisions based on data in
the application buffer, above the transport buffer (whether SSL or
GSS), but clients are polling the socket below the transport buffer.
One way to fix this in a backportable way, without changing APIs too
much, is to ensure data never stays in the transport buffer. Then
pqReadData's postconditions will look similar for both raw sockets and
SSL/GSS: any available data is either in the application buffer, or
still on the socket.
Building on the prior commit, make pqReadData() to drain all pending
data from the transport layer into conn->inBuffer, expanding the
buffer as necessary. This is not particularly efficient from an
architectural perspective (the pqsecure_read() implementations take
care to fit their packets into the current buffer, and that effort is
now completely discarded), but it's hopefully easier to reason about
than a full rewrite would be for the back branches.
Author: Jacob Champion <jacob.champion@enterprisedb.com> Reviewed-by: Mark Dilger <mark.dilger@enterprisedb.com> Reviewed-by: solai v <solai.cdac@gmail.com> Reported-by: Lars Kanis <lars@greiz-reinsdorf.de>
Discussion: https://postgr.es/m/2039ac58-d3e0-434b-ac1a-2a987f3b4cb1%40greiz-reinsdorf.de
Backpatch-through: 14
libpq: Extend "read pending" check from SSL to GSS
An extra check for pending bytes in the SSL layer has been part of
pqReadReady() for a very long time (79ff2e96d). But when GSS transport
encryption was added, it didn't receive the same treatment. (As 79ff2e96d notes, "The bug that I fixed in this patch is exceptionally
hard to reproduce reliably.")
Without that check, it's possible to hit a hang in gssencmode, if the
server splits a large libpq message such that the final message in a
streamed response is part of the same wrapped token as the split
message:
If the split message takes up enough memory to nearly fill libpq's
receive buffer, libpq may return from pqReadData() before the later
messages are pulled out of the PqGSSRecvBuffer. Without additional
socket activity from the server, pqReadReady() (via pqSocketCheck())
will never again return true, hanging the connection.
Pull the pending-bytes check into the pqsecure API layer, where both
SSL and GSS now implement it.
Note that this does not fix the root problem! Third party clients of
libpq have no way to call pqsecure_read_is_pending() in their own
polling. This just brings the GSS implementation up to par with the
existing SSL workaround; a broader fix is left to a subsequent commit.
In preparation for the broader fix, this patch already changes the
*_read_pending() functions to return the number of bytes in the buffer
rather than just a boolean. The current callers don't need that, but
the subsequent fix will.
Author: Jacob Champion <jacob.champion@enterprisedb.com>
Discussion: https://postgr.es/m/CAOYmi%2BmpymrgZ76Jre2dx_PwRniS9YZojwH0rZnTuiGHCsj0rA%40mail.gmail.com
Backpatch-through: 14
Replace hardcoded mentions of pg_hosts.conf with GUC
Three error messages were using the default file name pg_hosts.conf
and not the variable backing the GUC, which would make logging be
confusing for users who have renamed the file using the GUC. Fix
by consistently using the HostsFileName variable.
Backpatch down to v19 where serverside SNI was introduced.
pg_dump: check for _beginthreadex() failure in parallel dump
ParallelBackupStart() stored _beginthreadex()'s return value as the
worker's thread handle without checking it. On failure that value is 0,
which would later reach WaitForMultipleObjects() as a null handle, caught
only by an Assert. The fork() path already calls pg_fatal() when it
fails; do the same for _beginthreadex(), as pgbench does.
Author: Bryan Green <dbryan.green@gmail.com>
Discussion: https://www.postgresql.org/message-id/8c712d76-ecf7-4749-a6d8-dddc01f298ec@gmail.com
Backpatch-through: 14
David Rowley [Tue, 7 Jul 2026 11:57:45 +0000 (23:57 +1200)]
Fix COUNT's logic for window run condition support
9d9c02ccd added code to allow the executor to stop early when processing
WindowAgg nodes where a monotonic window function starts producing
values that result in a pushed-down qual no longer matching, and will
never match again due to the window function's monotonic properties.
That commit requires a SupportRequestWFuncMonotonic to exist on the
window function and for it to detect when the function is monotonic. For
COUNT(ANY) and COUNT(*), the support function failed to consider some
cases where the WindowClause used EXCLUDE to exclude certain rows from
being aggregated. Some WindowClause definitions mean we aggregate rows
that come after the current row, and when processing those rows later,
if we EXCLUDE certain rows, the monotonic property can be broken.
Wrongly treating the COUNT(*) or COUNT(ANY) aggregate as monotonic could
lead to rows being filtered that should not be filtered from the result
set.
Another issue was that the support function for the COUNT aggregate
mistakenly thought that a WindowClause without an ORDER BY meant that
the results would be both monotonically increasing and decreasing, but
that's only true when in RANGE mode, where all rows are peers.
It is possible to support various cases that do have an EXCLUDE clause,
but getting the logic correct for the exact set of cases that are valid
is quite complex and would likely better be left for a future project.
Here, we mostly disable run condition pushdown when there is an EXCLUDE
clause unless the clause is for EXCLUDE CURRENT ROW, uses COUNT(*)
(rather than COUNT(ANY)), and the window aggregate has no FILTER clause.
Bug: #19533 Reported-by: Qifan Liu <imchifan@163.com>
Author: Chengpeng Yan <chengpeng_yan@outlook.com>
Author: David Rowley <dgrowleyml@gmail.com> Reviewed-by: Richard Guo <guofenglinux@gmail.com> Reviewed-by: John Naylor <johncnaylorls@gmail.com>
Discussion: https://postgr.es/m/19533-413a1014e5d0e766@postgresql.org
Backpatch-through: 15
This was the dominant style already, but some places used %llu
instead. Since off_t/pgoff_t are signed types, using %lld seems a
better match, and it might handle obscure error conditions with
negative values better.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/20ce62fa-47fc-457b-b504-12f3c1651726%40eisentraut.org
Don't cast pgoff_t to possibly 32-bit types for output
pgoff_t is most likely a 64-bit integer, so casting it to a 32-bit
type for output could lose data. In the cases addressed here, the
files cannot actually get that large, so this is only cosmetic and to
set better examples for the future. (Similar issues that could have
actual practical impact were addressed separately in commit e8f851d6172.)
In one case, the 32-bit size is baked into the protocol, so here we
add an elog and document this discrepancy.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/20ce62fa-47fc-457b-b504-12f3c1651726%40eisentraut.org
Update GROUP BY ALL comments about window functions
When GROUP BY ALL was added in commit ef38a4d9756, the SQL standard
working draft was silent on what to do with window functions. This
has now been fixed in the SQL standard working draft. Update the
documentation and code comments about that.
Also make the documentation more specific that we are only talking
about aggregate functions referring to the same query level, which is
another thing that has been made more precise in the SQL standard
working draft since.
The PostgreSQL implementation was already doing the right thing for
both aspects, so no functionality changes.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/CAHM0NXjz0kDwtzoe-fnHAqPB1qA8_VJN0XAmCgUZ%2BiPnvP5LbA%40mail.gmail.com
Amit Langote [Mon, 6 Jul 2026 23:14:04 +0000 (08:14 +0900)]
Enforce RETURNING typmod on SQL/JSON DEFAULT behavior expressions
transformJsonBehavior() coerced an ON EMPTY / ON ERROR DEFAULT
expression only when its type differed from the RETURNING type's OID.
When the base type matched but the RETURNING type carried a type
modifier (e.g. numeric(4,1) or varchar(3)), the coercion that enforces
the typmod was skipped, so the DEFAULT value could violate the
declared type:
SELECT JSON_VALUE(jsonb '{}', '$.a'
RETURNING numeric(4,1) DEFAULT 99999.999 ON EMPTY);
returned 99999.999, which 99999.999::numeric(4,1) would reject; the
value could even be stored into a numeric(4,1) column, as later
coercions trust its already-correct type label.
Fix by also coercing when the RETURNING type has a typmod, except for
a NULL constant. coerce_to_target_type() is a no-op when the typmod
already matches. The matching-OID short-circuit dates to 74c96699be3.
Reported-by: Ewan Young <kdbase.hack@gmail.com>
Author: Ewan Young <kdbase.hack@gmail.com>
Discussion: https://postgr.es/m/CAON2xHPO9f4cAmyGn1mQ=VqoS7wN5rz4yOiqudxX78zninZpCw@mail.gmail.com
Backpatch-through: 17
Robert Haas [Mon, 6 Jul 2026 19:34:12 +0000 (15:34 -0400)]
Use PG_MODULE_MAGIC_EXT in newly introduced modules
We forgot to use the PG_MODULE_MAGIC_EXT in some newly added modules:
pg_plan_advice, pg_stash_advice and the pgrepack output plugin and
instead used the older PG_MODULE_MAGIC macro.
Author: Andreas Karlsson <andreas@proxel.se>
Discussion: http://postgr.es/m/ad7b910c-d145-4120-994d-2e55c456aa75@proxel.se
Backpatch-through: 19
Tom Lane [Mon, 6 Jul 2026 18:47:58 +0000 (14:47 -0400)]
Fix mishandling of leading '\' in nondeterministic LIKE.
The loop in MatchText() processed a leading '\' without regard to
nondeterministic locales, which is problematic if what the '\'
precedes is an ordinary character that should be subject to
nondeterministic matching. We'd insist on a literal match for it,
which is not right and is not like what happens with a '\' that
follows some ordinary characters. Worse, we'd then advance the text
and pattern pointers by one byte, so that if the escaped character
is multibyte the next loop iteration would take the nondeterministic
code path starting at a point within the character. That could very
possibly cause pg_strncoll() to misbehave.
The fix is quite simple: move the stanza that handles '\' down past
the one that handles nondeterminism. The stanzas for '%' and '_'
are fine where they are, but the '\' stanza is only correct for
deterministic matching. The logic for nondeterministic cases is
already prepared to do the right things with a '\'.
While here, I replaced tests of "locale && !locale->deterministic"
with a boolean local variable, reasoning that those are in the hot
loop paths so saving a branch and indirect fetch is worth the
trouble. I also improved a number of related comments.
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/391592.1783187986@sss.pgh.pa.us
Backpatch-through: 18
Tom Lane [Mon, 6 Jul 2026 18:35:21 +0000 (14:35 -0400)]
Fix LIKE matching with nondeterministic collations and backslashes.
Commit 85b7efa1c added support for LIKE with nondeterministic
collations, but it included a bug in the de-escaping logic for
literal pattern substrings. That unconditionally skipped all
backslashes, but when it encounters '\\' it should emit the second
backslash as a de-escaped character. That led to acting as though
the escaped backslash was not there.
Bug: #19474 Reported-by: Bowen Shi <zxwsbg12138@gmail.com>
Author: Nitin Motiani <nitinmotiani@google.com> Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com> Reviewed-by: Ewan Young <kdbase.hack@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/19474-5b86a95f3d9a7ecb@postgresql.org
Discussion: https://postgr.es/m/CAH5HC94yU+K8Gcdy12M5BS8gwD_SXLSHzc9k5tNk7JDnpBiFMA@mail.gmail.com
Backpatch-through: 18
Tom Lane [Mon, 6 Jul 2026 17:48:42 +0000 (13:48 -0400)]
Make PLy_elog() use pg_integer_constant_p().
This macro is supposed to work like ereport(). But when 59c2f03d1 adjusted ereport() to be more MSVC-friendly,
it missed updating this copy of the logic.
Tom Lane [Mon, 6 Jul 2026 17:06:21 +0000 (13:06 -0400)]
Fix LIKE/regex optimization for indexscan with exact-match pattern.
Commit 85b7efa1c introduced support for LIKE with non-deterministic
collations. By moving some conditionals around, it accidentally broke
the optimization for converting a LIKE or regex exact-match pattern
to an equality indexqual when the index collation doesn't match the
expression collation. That should be allowed if the expression
collation is deterministic. This patch re-introduces the optimization
for that common case.
One important beneficiary of this optimization is the "\d tablename"
command in psql. Without this fix that will do a seqscan on pg_class
instead of an index point lookup.
Reported-by: Andres Freund <andres@anarazel.de>
Author: Jelte Fennema-Nio <postgres@jeltef.nl> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/DHBQIZX8SZVI.ZX614ZMFL645@jeltef.nl
Backpatch-through: 18
Robert Haas [Mon, 6 Jul 2026 16:12:41 +0000 (12:12 -0400)]
Prevent satisfies_hash_partition from crashing with VARIADIC NULL.
Commit f3b0897a1213f46b4d3a99a7f8ef3a4b32e03572 fixed some
related problems, but overlooked this one. That commit first
appeared in PostgreSQL 11, so back-patch to all supported branches.
Backpatch-through: 14
Discussion: http://postgr.es/m/CA+TgmobsvQw3F+KRYT83=N3teh8D2t-oPR=U06QDZJE3viCJRg@mail.gmail.com Reviewed-by: Tender Wang <tndrwang@gmail.com> Reviewed-by: Ewan Young <kdbase.hack@gmail.com>
Remove switch statements in vector8_shift_{left,right}.
In commit ec8719ccbf, I added switch statements with all expected
shift counts to vector8_shift_{left,right} because vshlq_n_u32()
and vshrq_n_u32() require integer literals. But we can use
vshlq_u32() instead for both cases, which does not require an
integer literal, thereby avoiding the need for the switch
statements. This compiles to the same machine code on newer
versions of popular compilers.
Reviewed-by: John Naylor <johncnaylorls@gmail.com>
Discussion: https://postgr.es/m/akWxkA-mszMm57cV%40nathan
Remove apparent support for SECURITY LABEL ON PROPERTY GRAPH
Commit 2f094e7ac69 added a mention of SECURITY LABEL ON PROPERTY GRAPH
to the SECURITY LABEL reference page, and it added support to psql tab
completion. However, security labels on property graphs are not
actually supported (per SecLabelSupportsObjectType()). The syntax
does work, but that is just a result of how gram.y is factored. We
don't document or tab-complete the syntax of SECURITY LABEL for other
object types that are not actually supported, so it was inconsistent
to do this for property graphs. Thus, remove this.
projects / thirdparty / postgresql.git / log
