LLVM's headers produce warnings under some of the warning options we
might like to use. For example, -Wshadow=local reports several
warnings from LLVM's headers when compiling llvmjit_inline.cpp. To
work around that, add the include directories reported by llvm-config
with -isystem rather than -I, which makes the compiler treat them as
system headers and not report warnings from them. In meson, this is a
built-in facility of the dependency() function, in configure we
implement it ourselves.
An alternative solution would have been to use "#pragma GCC
system_header", as is already done elsewhere in the tree. But that
seems less elegant here. Either, we would have to potentially create
a separate wrapper for each LLVM header, or we would have to route all
LLVM includes through a common header, which would break modularity.
Reviewed-by: Peter Geoghegan <pg@bowt.ie>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/84b6f128-91f5-480e-8a9e-7d0e8f538cea%40eisentraut.org
The in-tree getopt_long() moves each non-option to the end of argv,
which might put it right where an option's argument lookup expects
to find the argument. For example, "vacuumdb postgres --jobs"
takes "postgres" as the number of jobs instead of complaining that
--jobs is missing its argument. To fix, stop the argument lookups
at the start of the moved non-options, which we already track to
know when to stop scanning.
Oversight in commit 411b720343.
Author: Sehrope Sarkuni <sehrope@jackdb.com>
Reviewed-by: solai v <solai.cdac@gmail.com>
Discussion: https://postgr.es/m/CAH7T-arxDuVCSkorO%3Dk7%2BM-_JV0JFzMpN_EtKMyD2K0RDqZ2OA%40mail.gmail.com
Backpatch-through: 17
If a long option that accepts an optional argument is given without
one, optind is advanced twice, and the following argument is
skipped. For example, "pg_waldump --stats --limit 5" skips --limit
and interprets "5" as a non-option, so the command complains that
it cannot location WAL file "5". The same path also returns BADARG
when optstring starts with a colon, even though nothing is missing.
To fix, handle optional arguments before the missing-argument code,
which then only needs to deal with required arguments.
This is a bug fix and could be back-patched, but since this issue
went unnoticed for 23 years, I'm not going to bother.
Author: Sehrope Sarkuni <sehrope@jackdb.com>
Reviewed-by: solai v <solai.cdac@gmail.com>
Discussion: https://postgr.es/m/CAH7T-arxDuVCSkorO%3Dk7%2BM-_JV0JFzMpN_EtKMyD2K0RDqZ2OA%40mail.gmail.com
REPACK (CONCURRENTLY) suppresses logical decoding of the changes it
applies to the transient heap. For an update, suppression keeps the
tuple data out of the WAL record, but the record itself is still
written, and decoding turned it into a change carrying neither a new
nor an old tuple. An output plugin that asks for the changes made by
heap rewrites therefore outputs an UPDATE with no data at all,
reported under the name of the table being repacked.
Ignore such records, as decoding already does for inserts. Updates on
catalog relations don't carry the new tuple either, so we ignore them
too. For deletes, REPACK sets XLH_DELETE_NO_LOGICAL instead.
Backpatch to v19, where REPACK (CONCURRENTLY) was introduced.
Reported-by: Thom Brown <thom@linux.com>
Reviewed-by: Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Discussion: https://postgr.es/m/CAA-aLv7L_-dOuHXjLh0Di66dExdOb=uTOzR=jtrqCmV0Wxyd2Q@mail.gmail.com
Backpatch-through: 19
Previously, heap_update() honored TABLE_UPDATE_NO_LOGICAL for the main
tuple but passed a hardcoded 0 to the tuple toaster, so the tuples it
wrote to the TOAST relation were logged for decoding anyway. That was
harmless until commit 28d534e2ae gave updates the flag; the insert
path has propagated its own ever since suppression was introduced for
heap rewrites.
REPACK (CONCURRENTLY), the only user of the flag, relies on it to keep
the changes it applies to the transient heap out of the logical
stream. Logical decoding therefore reassembled the TOAST value and
then dereferenced a new tuple that the suppressed record doesn't
carry, crashing the backend. This is reachable only if an output
plugin asks for the changes made by heap rewrites. In core that is
just test_decoding with include-rewrites.
Fix this by passing HEAP_INSERT_NO_LOGICAL down to the tuple toaster
when TABLE_UPDATE_NO_LOGICAL is set.
Backpatch to v19, where REPACK (CONCURRENTLY) was introduced.
Reported-by: Thom Brown <thom@linux.com>
Author: Antonin Houska <ah@cybertec.at>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-by: Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Discussion: https://postgr.es/m/CAA-aLv7L_-dOuHXjLh0Di66dExdOb=uTOzR=jtrqCmV0Wxyd2Q@mail.gmail.com
Backpatch-through: 19
Several pg_ctl command-line options are used only on Windows, but
pg_ctl accepted them anyway on all platforms but didn't do anything
with them.
This commit changes it to report an error if an option is specified
but the platform doesn't make use of it. The affected options are -e,
-N, -P, -U. The option -S already errored out on non-Windows
platforms; the new behavior of the other options is aligned with that.
The --help output already didn't list these options on non-Windows
builds, so the fact that they were accepted silently was already
inconsistent. The man page already lists them as "Options for
Windows".
Several global variables that were set but then only read on Windows,
are now in #ifdef WIN32 blocks. This avoids -Wunused-but-set-global
warnings that the new clang 23 enables via -Wall.
Reviewed-by: Andreas Karlsson <andreas@proxel.se>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/eb013f9d-2247-444e-8815-9d17b4ce78e7%40eisentraut.org
The new clang 23 has a new warning about set-but-unused
static (internal-linkage) global variables: -Wunused-but-set-global,
which is activated in PostgreSQL builds via -Wall. This triggers a
few warnings in PostgreSQL code. This commit removes several such
variables that were either never used or whose last use was removed
some time ago.
Reviewed-by: Andreas Karlsson <andreas@proxel.se>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/eb013f9d-2247-444e-8815-9d17b4ce78e7%40eisentraut.org
This reverts commit a9c350d9ee.
While that intentionally changed our semantics for CREATE SCHEMA
subcommands, it's being argued that the functionality gain does
not justify potentially-subtle compatibility breakage. The most
useful bit of functionality gain came from commit 404db8f9e
("Execute foreign key constraints in CREATE SCHEMA at the end"),
which we're keeping because it's required by SQL spec and doesn't
seem to create a compatibility hazard by itself.
This is not an exact revert, partly because we're keeping 404db8f9e,
and partly because I kept the API changes that allowed passing down
a ParseState (which allows providing an error cursor for many of the
errors thrown in CREATE SCHEMA).
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/20260904180712.cc.noahmisch@microsoft.com
Backpatch-through: 19
This reverts commit d516974840,
along with parts of commit 049b742daa
("psql: Tighten heuristics for BEGIN/END within CREATE SCHEMA").
While there's nothing particularly wrong with d51697484 in itself,
it depends on a9c350d9e ("Don't try to re-order the subcommands of
CREATE SCHEMA"), and concerns have been raised that the compatibility
impact of that outweighs the benefit of allowing more object types
within CREATE SCHEMA.
It's not possible to revert 049b742da verbatim, because the
CVE-2026-6464 patches 3045a25ba ("Teach psql to skip in-line COPY
... FROM STDIN data after a failure") and cf754f741 ("Save/restore
more lexer state when skipping text due to \if") depend on
infrastructure it added. Instead, rip out just the bits specifically
needed to parse CREATE FUNCTION within CREATE SCHEMA. This results in
lexer code that matches v18-and-earlier as modified by CVE-2026-6464.
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/20260904180712.cc.noahmisch@microsoft.com
Backpatch-through: 19
The replica identity determination was "flawed": it fell back to the
primary key if the marked index was dropped, which is a defensible
choice, but logical decoding has a differing opinion and fails to
provide usable tuple identity data in this case. "Fix" REPACK by
refusing to use the primary key in that case.
This is arguably broken behavior in RelationGetIndexList, but I'll
refrain from changing that at this stage.
Reported-by: Nathan Bossart <nathandbossart@gmail.com>
Author: Matthias van de Meent <boekewurm+postgres@gmail.com>
Author: Ewan Young <kdbase.hack@gmail.com>
Backpatch-through: 19
Discussion: https://postgr.es/m/apCBRKCH8jwKiaSY@nathan
ALTER TABLE ... DETACH PARTITION ... CONCURRENTLY leaves the partition
marked as detach-pending if its wait is interrupted, and only DETACH
PARTITION ... FINALIZE clears that mark. In that state pg_class still says
relispartition while get_partition_ancestors() already reports nothing,
which RelationBuildPublicationDesc() was not ready for. It took
relispartition to mean that the ancestor list is not empty and asked for
its last element, which is an assertion failure, or a NULL pointer
dereference without assertions. CheckCmdReplicaIdentity() needs the
descriptor for every UPDATE and DELETE of a publishable relation and
builds it on first use, so no publication has to exist for this, and a
plain UPDATE crashes the backend.
Treat such a partition as a standalone table, as after the detach is
finalized. It is then published by FOR ALL TABLES publications, but cannot
be added to an EXCEPT clause until the detach completes, which is now
reported with a hint to run DETACH PARTITION ... FINALIZE.
Oversight in fd366065e0, which added the exclusion.
Author: Mikhail Nikalayeu <mihailnikalayeu@gmail.com>
Author: shveta malik <shveta.malik@gmail.com>
Author: Nisha Moond <nisha.moond412@gmail.com>
Reviewed-by: shveta malik <shveta.malik@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Reviewed-by: Nisha Moond <nisha.moond412@gmail.com>
Discussion: https://postgr.es/m/CADzfLwWoFPT%2Ba73%3DA%3DbsNWRMZQ98NpBEMgE%3Dt1FS4O4_%3DQVLfA%40mail.gmail.com
Backpatch-through: 19, where it was introduced
The tab-completion query used for ALTER DATABASE ... RESET referenced
pg_db_role_setting and pg_database without schema qualification, so a
same-named relation earlier in search_path could be used instead of the
system catalogs. This could make psql offer misleading RESET candidates.
Commit dbf5a83d46 schema-qualified the unnest() call in this same query,
but the catalog references were still left unqualified. Fix that by
referring to pg_catalog.pg_db_role_setting and pg_catalog.pg_database
explicitly.
Backpatch to 18, where the tab-completion for ALTER DATABASE ... RESET
was introduced.
Bug: #19523
Reported-by: Zhou Digoal <digoal@126.com>
Author: Vismay Tiwari <vismay.t@gmail.com>
Reviewed-by: Paul Kim <mok03127@gmail.com>
Reviewed-by: Kirill Reshke <reshkekirill@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/19523-424457118202f570@postgresql.org
Discussion: https://postgr.es/m/CALHMmB84qkCgv3QAR78YawgfqQZCxSPkRhppxzU=e6fg8RA+AA@mail.gmail.com
Backpatch-through: 18
The fast path probes using the referenced index's collation. When that
collation differs from the referenced column's, it can reject valid
references or accept invalid ones. Changing the scan key's collation
is not sufficient, since btree navigation must use the ordering under
which the index was built.
Cache index eligibility lazily after locking the referenced table,
reloading the constraint information, and opening the index. Compare
index and column collations using the held relation descriptors, and
fall back to SPI on a mismatch. Also move the btree eligibility check
here, avoiding a lookup of an index that REINDEX CONCURRENTLY could
drop before the referenced-table lock is acquired.
Cache both acceptance and rejection until the constraint information
is reloaded, so subsequent rows need not repeat the comparisons. Check
before probing or buffering any row. If batch entry initialization
rejects the index, remove that entry while preserving any other pending
checks.
Add ICU regression coverage for per-row validation, ordinary inserts,
reordered index columns, both directions of collation mismatch, and
fallback while another foreign key check is pending.
Reported-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/CAA4eK1Lk1DfgiFPL-HccZm-Nm+exg+FcsODH8KPw6BOBuzfHtA@mail.gmail.com
Backpatch-through: 19
AlterPublicationOptions() can use stale publication state when the
publication is modified concurrently while waiting for the publication
lock. In particular, a concurrent ALTER PUBLICATION ... SET ALL TABLES
can change puballtables, causing the validation to make decisions based
on the old value and leads to assertion failure. Likewise,
CheckAlterPublication() could miss a table added concurrently, letting
SET ALL TABLES EXCEPT (...) leave pg_publication_rel with a mixture of
inclusion and exclusion rows.
Fix by acquiring the lock and re-reading the publication tuple in
AlterPublication() before either path runs, and by calling
CheckAlterPublication() only after that.
Also fix 037_except.pl, which left one computed result untested and
silently reused a subscription created by an earlier test.
Author: Vignesh C <vignesh21@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: shveta malik <shveta.malik@gmail.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Discussion: https://postgr.es/m/CALDaNm1r2MkGu6h8zgU1Kj1sX-FcMQ7wGTeLSnhx-5joiyXEvg@mail.gmail.com
Backpatch-through: 19, where it was introduced
This relation option gives the possibility to define the attribute type
that can be used for chunk_id in a TOAST table when it is initially
created. An update of this parameter has no effect if a TOAST table
already exists under the relation, even on rewrites. It cannot be set
for TOAST relations.
This new option can be set only to "oid" as of this commit. This
feature is extracted as a patch of its own to ease the introduction of a
second mode in a follow-up change, related to the introduction of 8-byte
OIDs in external TOAST pointers.
Bump catalog version, as StdRdOptions has changed.
Author: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Greg Burd <greg@burd.me>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Yugo Nagata <nagata@sraoss.co.jp>
Discussion: https://postgr.es/m/af19kUjwjhaoUTLn@paquier.xyz
varatt_external_oid (varatt_external in v19 and older branches) having
no padding is a rule that has been implied for years, as pointers of
this structure may be compared using memcpy().
I doubt that anybody is going to break this rule as varatt_external is
old enough that it could vote in most countries, but this makes varatt.h
more consistent for a follow-up change that introduces a second on-disk
external TOAST pointer, and it's free to have a compile-time rule.
Suggested-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Discussion: https://postgr.es/m/CALj2ACVG9cjxr-0jb8PP_Vb7vJvVGBVM_fQ2srt4CpyURpfYhQ@mail.gmail.com
This commit adds a new SQL function able to set the type of a chunk_id
attribute for a TOAST table across binary upgrades. The value is set
when the creation of a TOAST table is required across upgrades, in the
same fashion as the TOAST relation's oid and relfilenode.
Note that this piece currently works only with a TOAST table's chunk_id
set at OIDOID, because it is currently the only type of on-disk external
TOAST pointer supported. This change is required for a follow-up
feature that aims to introduce support for 8-byte OIDs in external TOAST
pointers.
Taking care of this issue separately is less code churn for the next
changes, and perhaps some of the surrounding discussions with support
for different types of external on-disk TOAST pointers will find this
new facility useful.
Bump catalog version.
Author: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Greg Burd <greg@burd.me>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Yugo Nagata <nagata@sraoss.co.jp>
Discussion: https://postgr.es/m/af19kUjwjhaoUTLn@paquier.xyz
Those changes were intended to avoid dependencies on the global libc
locale settings by basing identifier casefolding on the default
locale. They introduced an unintentional behavior difference for the
builtin provider with locale C and a single-byte encoding.
Instead of fixing that edge case, just revert the changes. The
identifier casefolding behavior has always been based on the global
libc locale, and clients make an effort to use the same rules. Until
we have a plan to change that, provider-specific identifier
casefolding doesn't make sense.
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20260826225510.10.noahmisch@microsoft.com
Backpatch-through: 19
The general nature of this interaction has been understood (at least
by me) since before the original commit, but I failed to explicitly
document it. I also left behind an XXX in the README which I intended
to remove before commit. Fix those things.
Reported-by: Noah Misch <noah@leadboat.com>
Backpatch-through: 19
Discussion: https://postgr.es/m/CA+TgmoYmXy-jiP5qDhqNEiYFEBzQsArO6O2d9E8szNZqi1bePQ@mail.gmail.com
Commit ac58465e06 moved the discussion on clustering from CLUSTER's
refpage to REPACK's, but somehow introduced a mistake regarding when
exactly can the seqscan-and-sort strategy be used with respect to btrees
(spoiler: it was 100% backwards). Fix that, but also restructure and
reword the whole subsection more fully to try and make it more coherent.
Author: Chao Li <li.evan.chao@gmail.com>
Author: Álvaro Herrera <alvherre@kurilemu.de>
Reported-by: Antonin Houska <ah@cybertec.at>
Backpatch-through: 19
Discussion: https://postgr.es/m/69CA7636-0359-42B6-B971-16A036698187@gmail.com
Explain that returning false for TM_SelfModified follows ExecLockRows(),
which skips such tuples. This differs from TM_Updated, which is
unexpected in READ COMMITTED with FIND_LAST_VERSION.
Also remove "fall through to error" from the TM_Updated comment, since
the code calls elog(ERROR) directly.
Reported-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/CAA4eK1Lk1DfgiFPL-HccZm-Nm+exg+FcsODH8KPw6BOBuzfHtA@mail.gmail.com
Backpatch-through: 19
At backend level, PgStat_TableCounts currently stores all the per-table
pending counters in one structure. This is now split into two parts,
with the transactional part of the data moved out of PgStat_TableCounts
into its own structure labelled with an "xact" in PgStat_RelationStatus.
While on it, move truncdropped out of the counters and into
PgStat_RelationStatus so as both structs (existing PgStat_TableCounts
and new PgStat_TableCountsXact) contain only counters. The previous
state was slightly confusing: this flag is a state tracking if a
relation has been truncated at backend-level, and not a counter.
Because truncdropped no longer lives inside PgStat_TableCounts, this
commit teaches the flush callback to treat a pending truncate as
sufficient reason to flush, even when both counter structs are otherwise
all zeros.
This refactoring work is in preparation of a different patch that wants
to introduce the concept of transactional stats data flush. Compared to
all the other stats kinds, relations are an exception as they mix
transactional and non-transactional data.
Author: Sami Imseih <samimseih.pg@gmail.com>
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/aoU32VTkUtpdUewB@paquier.xyz
ri_FastPathCheck() acquired its scan snapshot before opening the
referenced relation. If it then waited for the relation lock in READ
COMMITTED mode, a referenced row committed during the wait would not be
visible to the old snapshot. The check could consequently report a
foreign key violation even though the referenced row existed.
The batched path acquires its scan snapshot after opening the relations,
so it does not have the lock-wait problem. The per-row path remains
reachable during constraint validation and checks re-entered from a
batch flush.
Take the snapshot after opening the referenced relation and reloading
the constraint information. This also agrees with the SPI path, which
acquires the referenced-relation lock before selecting the snapshot
used for the check.
Also make the scan snapshot active for the duration of the check, so
STABLE cast and equality functions use the same snapshot as the index
scan. Otherwise, a STABLE cast can miss rows inserted by earlier AFTER
triggers and cause a valid foreign key to be rejected. Add a
regression test comparing this case with the SPI path.
Finally, also advance the command counter after acquiring the
referenced-relation lock and before reloading the constraint
information, so local invalidations are processed under the lock, as in
the SPI path.
Add an injection-point test that pauses a per-row check before locking
the referenced relation, commits the missing referenced row in another
session, and verifies that validation succeeds. Force per-row
validation by using a role without SELECT privilege on the referenced
table, so the test exercises this path even with batching enabled.
Add isolation-test coverage for the visibility of a referenced row
committed after the referencing transaction has executed an earlier
command. A later command can see such a row in READ COMMITTED, but not
in REPEATABLE READ or SERIALIZABLE.
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/CA+HiwqEhm+_=bs=2wavAJz-UqC+1KebD31++mapJQQGweE8iQQ@mail.gmail.com
Backpatch-through: 19
pg_stat_force_next_flush() has primarily been used in in-core tests to
force cumulative statistics to be flushed at the end of a transaction
without waiting for the PGSTAT_MIN_INTERVAL throttle, to speed up
things.
It is also useful in tests outside the core tree. Spoiler: I have used
it myself for some out-of-core work for the same reason as the in-core
tests, and would have unlikely known about it without my work on the
core code.
Author: Sami Imseih <samimseih.pg@gmail.com>
Discussion: https://postgr.es/m/aqEONVuLhgBiqHW6@paquier.xyz
Previously, readCommandResponse() stored every column with PQgetvalue(),
which returns an empty string for the SQL NULL value. An empty string
is not a valid value in the pgbench expression language, and when
returned by the server it was also indistinguishable from a NULL.
When receiving a value, pgbench now checks for PQgetisnull() and sets a
NULL value with setNullValue() before saving it. This behavior is more
consistent with \set, making the result saved testable with IS NULL.
This is a quality-of-life improvement, so no backpatch is done.
Author: Shinya Kato <shinya11.kato@gmail.com>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: Shihao Zhong <zhong950419@gmail.com>
Discussion: https://postgr.es/m/CAOzEurT9J9QMmpJt_8pWZCziNYCUHOD=c-hoccOVYJLc_VxVRg@mail.gmail.com
Commits 630706ced0 and 9c8de15969 lost some encoding-specific nuances:
in libc with EUC encodings, or in ICU with any non-UTF8 encoding, we
cannot easily classify characters, so we must assume they are
case-varying. Restore these behaviors.
Previously, ILIKE using an index scan could cause wrong results.
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20260826221526.89.noahmisch@microsoft.com
Backpatch-through: 19
pg_stat_progress_cluster translates REPACK into VACUUM FULL when
pg_stat_progress_repack.repack_index_relid is zero, and into CLUSTER
otherwise.
Previously, repack_index_relid was set only when an index scan was used.
When REPACK used a sequential scan and sort to order the table by an index,
the field remained zero even though index ordering had been requested. As a
result, pg_stat_progress_cluster incorrectly reported the command as
VACUUM FULL instead of CLUSTER.
Fix this by setting repack_index_relid to the OID of the requested ordering
index when progress reporting starts, regardless of the scan method. This
allows pg_stat_progress_cluster to correctly report REPACK as CLUSTER when
index ordering is requested.
Also update the documentation to clarify the meaning of the index columns
and how REPACK is translated into CLUSTER or VACUUM FULL.
Suggested-by: Fujii Masao <masao.fujii@gmail.com>
Author: Chao Li <lic@highgo.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
Discussion: https://postgr.es/m/CAN4CZFMzy2V_wvRCBW5K8=wVCk1-C7nq=8otXO=u+s_1KZHqyA@mail.gmail.com
Backpatch-through: 19
Previously, when another session ran REPACK, CLUSTER, or VACUUM FULL and
the user lacked permission to view its progress details,
pg_stat_progress_cluster incorrectly reported the command as CLUSTER
instead of NULL.
pg_stat_progress_cluster is based on pg_stat_progress_repack and
translates REPACK into CLUSTER or VACUUM FULL depending on whether index
ordering is requested. However, this translation defaulted to CLUSTER,
incorrectly converting NULL commands returned for insufficient privileges
into CLUSTER.
Fix this translation to preserve a NULL command when the user lacks
permission to view the progress details.
Bump catalog version.
Author: Zsolt Parragi <zsolt.parragi@percona.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Chao Li <lic@highgo.com>
Discussion: https://postgr.es/m/CAN4CZFMzy2V_wvRCBW5K8=wVCk1-C7nq=8otXO=u+s_1KZHqyA@mail.gmail.com
Backpatch-through: 19
The SQL wrappers of the module relied on stringToNode(), that trusts
its input. When specifying node strings that to not match with the
expected ExtensibleNode "TestExtNode", the module would crash.
This adds a simple set of sanity checks to check the input, ensuring
that only an ExtensibleNode "TestExtNode" goes through. Though far
from critical, it's always nice to make these test modules not crash
when used.
Oversight in 0e944fe357.
Author: Zsolt Parragi <zsolt.parragi@percona.com>
Reviewed-by: Greg Burd <greg@burd.me>
Discussion: https://postgr.es/m/CAN4CZFOTQXMZvuPxweO4Fm7+sjrPJUqU-JvYKETLBxuhqgbPjw@mail.gmail.com
A clause connecting the same two EC members can be requested from the
EC machinery in different contexts. When building parameterized index
paths for a relation, generate_implied_equalities_for_column derives
an indexable clause with parent_ec set, to mark it as redundant with
other join clauses derived from the same EC. Meanwhile, at a join
where the relations of a multi-relation EC member first come together,
generate_join_implied_equalities emits a clause equating the same two
members with parent_ec unset, since that clause is the sole enforcer
of its member there and must not be dropped as redundant. Because
create_join_clause treats parent_ec as part of its cache-lookup key,
such requests can yield two different RestrictInfos for the same
condition, carrying different rinfo_serials.
This breaks the assumption that one condition has exactly one serial
number, which create_nestloop_path relies on to drop join clauses that
are already enforced within a parameterized inner path. As a result,
the same condition could be enforced twice at different plan levels,
wasting execution effort and applying the clause's selectivity twice,
underestimating the join's row count.
To fix, make create_join_clause copy the rinfo_serial from an existing
clause that connects the same two members with the opposite parent_ec
marking, so that the two clauses keep their distinct markings but are
recognized as the same condition.
As a result, a duplicate join filter disappears from one of the
existing regression plans (the "cardinal_number" query in join.sql);
the other plan changes are join-order differences between plans of
essentially equal cost.
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAMbWs4-3yZeBMGQDpmYRPdV-sL_=6H2AN=MoeneOT+VUk28PPg@mail.gmail.com
Backpatch-through: 19
Commit f836b688f ensured that at most one clone of a join clause is
chosen for a parameterized path, so long as the clone variants are
parse-tree distinct. Two ways remained to end up enforcing the same
condition more than once, which wastes execution effort and applies
the clause's selectivity multiple times, underestimating the row
count.
If a commuting outer join nulls no Var referenced by a particular
qual, the clone variants of that qual are parse-tree identical, and
the incompatible_relids test cannot tell them apart. To fix, enforce
just the first surviving variant, identified by rinfo_serial.
The other case is in get_joinrel_parampathinfo, where the
clause-recovery pass for dropped EquivalenceClasses queries the EC
machinery with a context that overlaps the preceding loop's, so the
derived-clause cache can hand back a clause that that loop already
accepted. To fix, skip any clause that is already present in the
collected list.
Also add assertions verifying that the clauses to be enforced at a
join or within a parameterized path contain no duplicate rinfo_serial,
to catch any remaining or future violations. These assertions are
added in master only, since hard prevention of duplicate clauses is a
new goal and other unfixed cases may remain.
Back-patch to v19, as with commit f836b688f.
Author: Richard Guo <guofenglinux@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAMbWs4_Ezb1Lnj7BqcTQSysC6B5UFZxfhib8xr6rqpu5uqr8dA@mail.gmail.com
Backpatch-through: 19
Commit 0ee83dd4a9 allowed postgres_fdw to push down inner joins between
foreign tables and functions in FROM clauses to the remote server.
However, planning such a join with use_remote_estimate enabled could
cause a segmentation fault. For example, the following query crashed
when ft had use_remote_estimate set to true:
EXPLAIN SELECT * FROM generate_series(1, 3) g JOIN ft ON ft.i = g;
The crash occurred because init_func_stub_fpinfo() copied the connection
information and options from the foreign side to the function-side stub,
but omitted the user mapping, leaving it NULL. When the function was the
outer side of the join, the join inherited this NULL mapping and passed it
to GetConnection() when requesting a remote estimate, causing a
segmentation fault.
Fix this by having init_func_stub_fpinfo() also copy the user mapping
from the foreign side.
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Alexander Pyhalov <a.pyhalov@postgrespro.ru>
Discussion: https://postgr.es/m/CAHGQGwGE9xPzCke0C0r4omTkh-hU08aBd2j9oSA0FYmJLCf9jA@mail.gmail.com
This commit adds a new routine called GetNewObjectId8() in varsup.c,
which is able to retrieve a 8-byte OID, based on the next OID retrieved
from the control file. GetNewObjectId() is kept compatible with its
origin, where we still check that the lower 32 bits of the counter do
not wraparound, while handling the FirstNormalObjectId case.
This change is combined with an update of the control file, with
CheckPoint.nextOid enlarged from Oid to Oid8. pg_control_checkpoint(),
pg_resetwal and pg_upgrade are updated to adapt to this change, to make
sure that the value is not truncated due to 32-bit limitations when
used, making an upgrade to a newer version where 8-byte is supported a
transparent operation.
A couple of comments in the xlog redo logic referred to wraparound
issues, which is not the case anymore with this change in place, so
refresh these.
This change is required to be able to support external TOAST pointers
based on Oid8, under discussion, and can be used for other purposes.
Tests are added for pg_upgrade (2^32 value carried across upgrades),
pg_resetwal and pg_control_checkpoint(), hence across all the tools
that need to be covered by this change.
Bump catalog version.
Bump PG_CONTROL_VERSION.
Bump XLOG_PAGE_MAGIC, due to XLOG_NEXTOID.
Author: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Greg Burd <greg@burd.me>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Yugo Nagata <nagata@sraoss.co.jp>
Discussion: https://postgr.es/m/af19kUjwjhaoUTLn@paquier.xyz
Without CHECK_FOR_INTERRUPTS(), backends might not respond to query
cancellations in a timely fashion, especially for JOIN_ORDER()
specifications, where matching against each join can take O(n^2) time
in the length of the join order list in degenerate cases.
Recursive functions should check_stack_depth().
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/CA+TgmoYmXy-jiP5qDhqNEiYFEBzQsArO6O2d9E8szNZqi1bePQ@mail.gmail.com
Backpatch-through: 19
The SIMD path for this command reads ahead via CopyLoadInputBuf()
whenever fewer than sizeof(Vector8) bytes remain in the input
buffer, even if those bytes hold a complete end-of-copy marker.
If the input is a pipe whose writer has sent the marker but not
closed the pipe, that read blocks, and COPY waits for data it will
never use. The scalar loop asks only for the bytes it needs, so
it stops at the marker without reading any further.
To fix, teach CopyLoadInputBuf() to decline a speculative load
when the caller's remaining bytes contain a backslash, which in
text mode might begin such a marker. (CSV mode doesn't treat
\. as special, so it is unaffected.) The SIMD path then hands
those bytes to the scalar loop, as it already does when a load
comes up short. Checking for the marker in the SIMD helper itself
would be more direct, but a call there costs the compiler
registers on every line, which measurably slowed COPY of short
lines in testing.
Oversight in commit e0a3a3fd53.
Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Reviewed-by: Greg Burd <greg@burd.me>
Discussion: https://postgr.es/m/CAN55FZ1qFb4Yo3-MWeQfGQnu1_Ksx4xcFC2m3hyw8a--%2BeHQYQ%40mail.gmail.com
Backpatch-through: 19
Commit 86f7c82cf1 reported the user catalog table restriction as a
detail, but the other checks in check_concurrent_repack_requirements()
were still a mix: some were hints, and the details repeated
"REPACK (CONCURRENTLY)", which the primary message already names.
Report them as details with the same "This operation ..." wording. The
one actionable hint, ALTER TABLE ... REPLICA IDENTITY USING INDEX,
stays.
Backpatch to v19, where REPACK (CONCURRENTLY) was introduced.
Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Suggested-by: Alvaro Herrera <alvherre@kurilemu.de>
Discussion: https://postgr.es/m/apBIFWzHYOaG0auN%40nathan
Backpatch-through: 19
DROP TABLESPACE checked pg_shdepend for dependent objects without
first locking the tablespace. A concurrent command that recorded a
shared dependency on the tablespace right after that check could still
commit, leaving an object whose pg_shdepend entry (or, for a relation,
pg_class.reltablespace) pointed to a tablespace that no longer existed.
Close the race by having DropTableSpace() take an AccessExclusiveLock
on the tablespace before calling checkSharedDependencies(). That
conflicts with the AccessShareLock shdepLockAndCheckObject() takes when
recording a new dependency, so the loser of the race blocks and
rechecks once the winner commits.
That AccessExclusiveLock creates a new deadlock: ALTER TABLESPACE
RENAME/SET and the internal ACL/owner updates in DROP OWNED and
REASSIGN OWNED touched the catalog tuple without locking the
tablespace, risking a lock-order cycle with DROP. Fix by taking an
AccessShareLock first in all four paths, rechecking pg_shdepend after
any wait in the DROP OWNED and REASSIGN OWNED cases. GRANT, REVOKE,
and ALTER TABLESPACE ... OWNER TO already lock the object.
Add isolation tests covering both orderings of the original race
(dependency-first and drop-first) and all four previously-unlocked
update paths.
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Andrew Dunstan <andrew@dunslane.net>
Discussion: https://postgr.es/m/CAJTYsWXjAQFnGKzXsht3XK8KHhyhontwFJw4JAHsUSfs_ptR4g@mail.gmail.com
Backpatch-through: 14
Previously, running REPACK (CONCURRENTLY) on a materialized view failed
with a confusing "has no identity index" error.
REPACK (CONCURRENTLY) replays changes decoded from WAL, but a
materialized view produces none, since it cannot be modified by DML and
REFRESH rewrites it through a transient heap that is not logically
decoded.
Fix this by reporting a clear error for materialized views upfront,
alongside the existing checks for system catalogs and TOAST tables, and
document the limitation.
Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Kiran Kaki <itskkpg@gmail.com>
Backpatch-through: 19
Discussion: https://postgr.es/m/CALj2ACXPvy4_LkQ0nHKgbZjAGK9fkDqRYUvXA3hgwxPm_X7Eng@mail.gmail.com
REPACK (CONCURRENTLY) is not an MVCC-safe operation; it doesn't
preserve the visibility information, which logical decoding needs
because it reads user catalog tables under a historic snapshot.
Disallow REPACK (CONCURRENTLY) on user catalog tables. Removing this
check requires making it MVCC-safe and logical rewrite mappings.
Backpatch to v19, where REPACK (CONCURRENTLY) was introduced.
Reported-by: Nathan Bossart <nathandbossart@gmail.com>
Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Antonin Houska <ah@cybertec.at>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-by: Alvaro Herrera <alvherre@kurilemu.de>
Discussion: https://postgr.es/m/apBIFWzHYOaG0auN%40nathan
Backpatch-through: 19
REPACK (CONCURRENTLY) launches a background worker to decode changes
made while the table is being rewritten. The worker connects as the
table owner but bypassed the LOGIN check only, so CONNECT was still
checked against a role that need not have it, and the command could
fail with "permission denied for database".
Pass BGWORKER_BYPASS_ALLOWCONN as well, as we do for parallel
workers. That is safe because the leader already checked the invoking
user's privileges on the table before starting the worker.
Reported-by: Nathan Bossart <nathandbossart@gmail.com>
Author: Matheus Alcantara <mths.dev@pm.me>
Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Discussion: https://postgr.es/m/apBbzFd_EYAfHV45@nathan
Backpatch-through: 19
