QueueFKConstraintValidation() recurses through the partition hierarchy
to queue child constraint validations and to mark child rows as
validated. With a sufficiently deep partition tree, this can result
in a stack-overflow crash. Defend against that as we do elsewhere.
Bug: #19482
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/19482-4cc37cbf52d55235@postgresql.org
Backpatch-through: 18
The original code would leave a shared memory segment unreleased if we
fail partway through initialization. Change the shutdown order so that
we always free it.
Author: Álvaro Herrera <alvherre@kurilemu.de>
Reviewed-by: Antonin Houska <ah@cybertec.at>
Discussion: https://postgr.es/m/agtNn6ZCmdI2KJFn@alvherre.pgsql
pg_get_multixact_stats() uses members_size to report the amount of
storage used by the currently retained multixact members. However,
MultiXactOffsetStorageSize() divided the member count by the number of
members per storage group before multiplying by the group size, so it
was rounding down its result and incorrectly reported zero when there
were few retained members. The calculation is changed to calculate the
same based on the member count.
While on it, this fixes a different issue in the isolation test
multixact-stats. Three fields were defined for checks related to the
oldest offset values, but were not used. The offsets existed in an
older version of the patch than what has been committed. These are
replaced by checks for members_size, checking the new calculation
formula.
Thinkos introduced in 97b101776c.
Author: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/819AC1B2-1A71-4244-B081-3ADD85D1725D@gmail.com
The wording of two error hints is tweaked in this commit:
- Import of extended statistics, where the value of an array element is
not a NULL or a string.
- Online data checksum switch, where a period was missing.
Author: Baji Shaik <baji.pgdev@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/CA+fm-RMrKbyky_+vi5SDdAVnFVjWh7zW3GoDAVnrp5OpDnW6tw@mail.gmail.com
Given a WHERE clause like "int[] @@ query_int" or "query_int ~~ int[]"
where the query_int side is a table column having statistics,
_int_matchsel() exited without remembering to free the statistics
tuple. This would typically lead to warnings about cache refcount
leakage, like
WARNING: resource was not closed: cache pg_statistic (73), tuple 42/12 has count 1
It's been wrong since this code was added, in commit c6fbe6d6f.
Bug: #19492
Reported-by: Man Zeng <zengman@halodbtech.com>
Author: Man Zeng <zengman@halodbtech.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/19492-ddcd0e22399ef85a@postgresql.org
Backpatch-through: 14
Previously, dblink accepted the use_scram_passthrough option on
foreign-data wrappers via ALTER FOREIGN DATA WRAPPER dblink_fdw
OPTIONS, even though the setting had no effect there.
use_scram_passthrough should be only meaningful for foreign servers
and user mappings, so this commit updates dblink to accept the option
only in those contexts.
Backpatch to v18, where use_scram_passthrough was introduced.
Author: Matheus Alcantara <matheusssilv97@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwEJ8rZjmbOvCicyr4vbuLio082bNTde0WNoSWaWr9wVcg@mail.gmail.com
Backpatch-through: 18
Commit 97f6fc10ff changed postgres_fdw so that user-mapping settings
override foreign server settings for use_scram_passthrough. This commit
applies the same behavior to dblink.
Backpatch to v18, where use_scram_passthrough was introduced.
Author: Matheus Alcantara <matheusssilv97@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwEJ8rZjmbOvCicyr4vbuLio082bNTde0WNoSWaWr9wVcg@mail.gmail.com
Backpatch-through: 18
Previously, when use_scram_passthrough was specified on both a foreign server
and a user mapping, the server-level setting took precedence over the
user-mapping setting. This was inconsistent with the usual semantics of
postgres_fdw options, where foreign server options provide shared defaults
and user mapping options override them on a per-user basis.
This commit updates postgres_fdw so that the user-mapping setting takes
precedence when use_scram_passthrough is specified in both places. This
matches the behavior of other connection options such as sslcert and sslkey.
Backpatch to v18, where use_scram_passthrough was introduced. In v18,
this only affects limited configurations that specify conflicting values
at both the foreign server and user-mapping levels. In such cases, users
would naturally expect the user-mapping setting to override the server-level
setting, so changing the behavior should be minimally disruptive.
Also keeping v18 as the only branch with different semantics for
use_scram_passthrough would be unnecessarily confusing, so backpatch
this fix to v18.
Author: Matheus Alcantara <matheusssilv97@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwEJ8rZjmbOvCicyr4vbuLio082bNTde0WNoSWaWr9wVcg@mail.gmail.com
Backpatch-through: 18
The CHECKPOINT reference page still described checkpoints as flushing
all data files, which could be misleading as it depends on the value
of FLUSH_UNLOGGED option. Update the description to make it clearer
that only data files of permanent relations are flushed by default.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/4855807D-F1CA-44E6-9B58-406691832848@gmail.com
Tab completion for CHECKPOINT options contained FLUSH_UNLOGGED, but
the boolean value was not part of the completion. Fix to make this
consistent with other boolean values.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/4855807D-F1CA-44E6-9B58-406691832848@gmail.com
ALTER TABLE ... SPLIT PARTITION allows a DEFAULT partition to be created
as one of the replacement partitions when the parent table does not
already have one. However, it should not allow the degenerate case where
a non-DEFAULT partition keeps exactly the same bound as the split
partition and the command merely adds a DEFAULT partition through the
SPLIT PARTITION path.
Detect that case by comparing the bound of the split partition with the
bound of the only non-DEFAULT replacement partition, and raise an error
when they are the same. Users should add a DEFAULT partition directly
with CREATE TABLE ... PARTITION OF ... DEFAULT or ALTER TABLE ... ATTACH
PARTITION ... DEFAULT instead.
The comparison goes through the partition operator family rather than
byte equality so that values which are binary-different but compare
equal under the partition key's comparator are treated as the same
bound. The corresponding regression test uses a float8 LIST partition
with -0.0 and 0.0 -- they have different bit patterns but are equal
under float8 -- to verify that a datumIsEqual()-based check would let
the degenerate split through while the partsupfunc-based check
correctly rejects it.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
Discussion: https://postgr.es/m/C18878AB-DEB2-4A61-9995-A035DD644B81@gmail.com
The check for the minimum expected bytea size of a MVDependencies object
was using SizeOfItem() for its calculation. This macro uses the number
of attributes in a single dependency.
This minimum size calculation should be based on MinSizeOfItems(), that
computes the minimum expected size as the header plus the
minimally-sized number of dependency items.
Oversight in d08c44f7a4.
Author: Ilia Evdokimov <ilya.evdokimov@tantorlabs.com>
Discussion: https://postgr.es/m/4b8d299d-2505-4c30-bf80-0f697410db35@tantorlabs.com
Backpatch-through: 14
This reverts commit 0d3dba38c7, which was determined to have
fundamental flaws. This restricts REPACK (CONCURRENTLY) so that only
one process can run it concurrently on different tables and even on
different databases; we'll lift that restriction in another way during
the next development cycle.
Reported-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/CAA4eK1Jg21ODQ7fS2fvN5W_S5kDRhAP5inj3XMRQaa=s-GbYhw@mail.gmail.com
When reusing an existing WAL receiver after it has reached
WALRCV_WAITING for new instructions, RequestXLogStreaming() copied
PrimaryConnInfo into WalRcv->conninfo before switching the state to
WALRCV_RESTARTING. At that point ready_to_display could still be true,
so pg_stat_wal_receiver could expose the raw connection string,
including sensitive fields, but it should only show the user-displayable
version of the connection string.
WALRCV_RESTARTING does not establish a new connection. The waiting WAL
receiver reuses its existing connection and only needs a new startpoint
and timeline, so there is no need to copy the raw connection string into
shared memory again. Let's only copy conninfo when launching a new WAL
receiver after WALRCV_STOPPED, not while waiting for instructions.
This commit adds coverage for the case fixed by this commit to the
timeline-switch test by verifying that the WAL receiver conninfo remains
consistent across the jump.
Backpatch all the way down, as this issue is possible since
pg_stat_wal_receiver has been introduced.
Author: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/EF91FF76-1E2B-4F3B-9162-290B4DC517FF@gmail.com
Backpatch-through: 14
Commit a36164e746 added a CONNECTING status for the WAL receiver, but
pg_stat_wal_receiver returned no information while the connection to the
primary was attempted, limiting the usability of the feature in
high-latency environments where the connection attempt to the primary
could take time.
This commit improves the report of the status by splitting the way the
shared memory state of the WAL receiver is filled before and after the
connection to the primary is attempted with walrcv_connect():
- Before the attempt, reset all the connection fields, switch
ready_to_display to true.
- After the attempt, fill in the connection fields.
This change means two spinlock acquisitions instead of one, but at least
monitoring tools can know about the connection attempt before its
completion, enlarging the usability of the feature. This code path is
taken only once when a WAL receiver is spawned, so the extra acquisition
does not matter performance-wise.
Reported-by: Chao Li <li.evan.chao@gmail.com>
Author: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/EF91FF76-1E2B-4F3B-9162-290B4DC517FF@gmail.com
Commit 112faf1378 added custom notice receivers for replication,
postgres_fdw, and dblink so that remote NOTICE, WARNING, and similar
messages are reported via ereport(). However, those notice receivers were
installed only after libpqsrv_connect() and libpqsrv_connect_params()
returned, by which point libpq connection startup had already completed.
As a result, messages emitted during connection establishment could be
missed.
This commit fixes the issue by splitting libpqsrv_connect() and
libpqsrv_connect_params() into separate start and complete phases:
libpqsrv_connect_start(), libpqsrv_connect_params_start(), and
libpqsrv_connect_complete(). This allows callers to perform
per-connection setup, such as installing a notice receiver, after the
connection has been started but before startup completes.
Note that callers of libpqsrv_connect_start() and
libpqsrv_connect_params_start() must still call
libpqsrv_connect_complete(), even if the start function returns NULL, so
that any external FDs reserved during startup are released properly.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Vignesh C <vignesh21@gmail.com>
Reviewed-by: Rafia Sabih <rafia.pghackers@gmail.com>
Discussion: https://postgr.es/m/A2B8B7DE-C119-492F-A9FA-14CF86849777@gmail.com
The documentation states that NOT NULL constraints on partitioned tables
are always inherited by all partitions, and therefore cannot be declared
NO INHERIT. While a check already existed to reject creating such
constraints with NO INHERIT, previously the same check was missing for
ALTER TABLE ... ALTER CONSTRAINT ... NO INHERIT.
This commit adds the missing check so that attempting to set NO INHERIT
on a partitioned NOT NULL constraint now fails.
Backpatch to v18, where ALTER TABLE ... ALTER CONSTRAINT ... [NO] INHERIT
was added.
Author: Andreas Karlsson <andreas@proxel.se>
Reviewed-by: Jim Jones <jim.jones@uni-muenster.de>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/ecc985ad-6ec1-4094-a315-317943ca5f3f@proxel.se
Backpatch-through: 18
ALTER TABLE ... SPLIT PARTITION allows a DEFAULT partition to be created
as one of the replacement partitions when the parent table does not
already have one. However, it should not allow the degenerate case where
a non-DEFAULT partition keeps exactly the same bound as the split
partition and the command merely adds a DEFAULT partition through the
SPLIT PARTITION path.
Detect that case by comparing the bound of the split partition with the
bound of the only non-DEFAULT replacement partition, and raise an error
when they are the same. Users should add a DEFAULT partition directly
with CREATE TABLE ... PARTITION OF ... DEFAULT or ALTER TABLE ... ATTACH
PARTITION ... DEFAULT instead.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
Discussion: https://postgr.es/m/C18878AB-DEB2-4A61-9995-A035DD644B81@gmail.com
Commit 263d1e6dfe changed pg_recvlogical to honor source cluster file
permissions when creating output files. This commit adds tests verifying
that output files are created with mode 0600 when the source cluster is
initialized without group access, and with mode 0640 when group access is
enabled.
Author: Srinath Reddy Sadipiralla <srinath2133@gmail.com>
Author: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwHhpizYzMo3nFP4GkNMueSNMY3QfC-gBN1VTXtuiANDvw@mail.gmail.com
Commit c37b3d08ca attempted to preserve group permissions on pg_recvlogical
output files when group access was enabled on the source cluster. However,
the output files were still created with a fixed S_IRUSR | S_IWUSR mode,
preventing group-read permissions from being applied.
This commit fixes the issue by creating output files with pg_file_create_mode
instead of a hard-coded mode. This allows pg_recvlogical to correctly preserve
group permissions from the source cluster.
Backpatch to all supported branches.
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Srinath Reddy Sadipiralla <srinath2133@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwHhpizYzMo3nFP4GkNMueSNMY3QfC-gBN1VTXtuiANDvw@mail.gmail.com
Backpatch-through: 14
When the launching backend of REPACK (CONCURRENTLY) is terminated via
pg_terminate_backend(), ProcDiePending causes ereport(FATAL) which
bypasses PG_FINALLY blocks. As a result, stop_repack_decoding_worker()
is never called, leaving the decoding worker running indefinitely and
holding its temporary replication slot.
Fix by using PG_ENSURE_ERROR_CLEANUP, which handles both ERROR and
FATAL exits.
Author: Baji Shaik <baji.pgdev@gmail.com>
Reviewed-by: Sami Imseih <samimseih@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Discussion: https://postgr.es/m/CA+fm-RNoPxL2N7db_A0anMXV_aDu6jWj4PNOPtMtBUAPDPvSXQ@mail.gmail.com
The documentation said that the bounds of new partitions should not
overlap and that their combined bounds should equal the bounds of the
split partition. That is misleading when a new DEFAULT partition is
specified, because the explicit partitions may cover only part of the
split partition while the DEFAULT partition covers the rest.
Clarify that new non-DEFAULT partition bounds must not overlap with
other new or existing partitions and must be contained within the bounds
of the split partition. Also state that the combined bounds must exactly
match the split partition only when no new DEFAULT partition is specified.
While here, improve nearby wording about hash-partitioned target tables
and splitting a DEFAULT partition with the same partition name.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
Discussion: https://postgr.es/m/C18878AB-DEB2-4A61-9995-A035DD644B81@gmail.com
When ALTER TABLE ... SPLIT PARTITION specifies a DEFAULT partition, the
explicit partitions do not need to cover the split partition's bound
exactly. They may cover only part of it, with the DEFAULT partition
covering the remaining range.
However, the existing hint said that the combined bounds of the new
partitions must exactly match the bound of the split partition, which is
misleading for this case and inconsistent with the code comment.
Fix the hint to state the actual requirement: explicit partition bounds
must stay within the bounds of the split partition when a DEFAULT
partition is specified.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
Discussion: https://postgr.es/m/C18878AB-DEB2-4A61-9995-A035DD644B81@gmail.com
When splitting a range partition and defining a new DEFAULT partition, the
validation checked the lower bound of the first explicit partition and the
upper bound of explicit partitions only when they were not first. If there
was exactly one explicit non-DEFAULT partition, its upper bound was therefore
not checked.
This could allow the replacement partition to extend beyond the upper bound
of the partition being split, potentially overlapping another existing
partition.
Fix this by checking the upper bound whenever the explicit partition is the
last one. Add a regression test covering the single explicit partition plus
DEFAULT case.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Kirill Reshke <reshkekirill@gmail.com>
Reviewed-by: Zhenwei Shang <a934172442@gmail.com>
Reviewed-by: Dmitry Koval <d.koval@postgrespro.ru>
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
Discussion: https://postgr.es/m/C18878AB-DEB2-4A61-9995-A035DD644B81@gmail.com
When using COPY FROM ... ON_ERROR SET_NULL with a selective column list, the
domain_with_constraint array was incorrectly allocated based on the length of
the target column list. While the array was populated sequentially,
CopyFromTextLikeOneRow attempted to access it using the physical attribute
index (attnum - 1). This mismatch caused out-of-bounds reads when targeting
high-numbered columns, allowing NULL values to bypass NOT NULL domain checks
and be silently inserted.
Fix by allocating the array to match the total number of physical attributes
(num_phys_attrs) and indexing via attnum - 1, bringing it into alignment with
other per-column arrays in BeginCopyFrom.
Author: SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>
Reviewed-by: Jian He <jian.universality@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CAHg+QDdej0c0gWJi2FnbirzhgzyZNPiTwC1P5B_-dSNCzq-91A@mail.gmail.com
The macro for enabling single-copy atomicity on i586+ when using
GCC has been incorrect since 2017 (commit e8fdbd58f) without any
complaints, and getting it to work is non-trivial.
Getting this to work reliably require C11 atomics, which in turn
also bumps the required MSVC version. For now, simply remove the
attempted support which doesn't work anyways.
Author: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Reported-by: Jakub Wartak <jakub.wartak@enterprisedb.com>
Suggested-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CAKZiRmycHOOJyEPc9FUss1_69_U62WoSx32jT7wyES-YkStZKA@mail.gmail.com
Discussion: https://posrgr.es/m/CA+hUKGKFvu3zyvv3aaj5hHs9VtWcjFAmisOwOc7aOZNc5AF3NA@mail.gmail.com
This comment was originally added to RegisterInvalid() in POSTGRES before
Postgres95, and came in via the Postgres95 import. It has been obsolote
for quite some time so remove.
Author: Steven Niu <niushiji@highgo.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/MN2PR15MB30219837B2381AE2518A4C45A7FCA@MN2PR15MB3021.namprd15.prod.outlook.com
ParseVariableDouble missed returning false after logging an error when
the parsed value exceeded max, making the value assigned rather than
rejected. Backpatch down to v18 where this was introduced as part of
the \WATCH_INTERVAL.
Author: Sven Klemm <sven@tigerdata.com>
Co-authored-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/CAMCrgp31p_5SDVi7dwnP39tTW5icQ0MWHA+N4kJdXgkL0PEy8w@mail.gmail.com
Backpatch-through: 18
The error message for incorrect oauth validator configuration was missing
a quote character. OAuth was introduced in v18 but there is no need for a
backpatch since this was introduced in 22f9207aaa.
Author: Jonathan Gonzalez V. <jonathan.abdiel@gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/ff9b84b9e6d5a3fef1f320ee5d63ec7dae722739.camel@gmail.com
This commit addresses some defects with the handling of expressions in
pg_restore_extended_stats() and pg_clear_extended_stats():
- Misleading WARNING for an incorrect number of expressions, where the
number of required expressions was reported as the number of elements
given in input rather than the actual number of expressions expected by
the extstats object definition.
- Incorrect matching of expression names, where a key name was
considered as valid as long as it matched with the prefix of a legit key
name. For example "correlatio" given in input would match with
"correlation", and be considered valid. The consequence of this bug was
a silent discard of the input data, where the operation would be
considered a success. The value associated to the prefixed key was not
inserted in the catalogs, just ignored. pg_dump would not generate such
input data patterns, but a user doing manual stats injection could.
- Missing heap_freetuple() in pg_clear_extended_stats(), for the case
where the extstats object in input does not match with its parent
relation.
Author: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/A7C11B83-7534-4A09-9071-FBD09175CFC8@gmail.com
Previously, REPACK option parsing had two bugs.
First, REPACK (CONCURRENTLY OFF) failed with:
ERROR: unrecognized REPACK option "concurrently"
while CONCURRENTLY ON was accepted correctly.
Second, when the same option was specified multiple times, the last value
specified was not always honored. If any occurrence set the option to ON,
the option was treated as enabled even when the final setting was OFF.
This commit fixes these issues by correctly accepting CONCURRENTLY
regardless of its value, and by making the last specified value take precedence
when an option appears multiple times.
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Discussion: https://postgr.es/m/CAHGQGwGAY4kfDtC4i+hAOX-a3u0yOA6__6EDTQz-ytsDHgh-yQ@mail.gmail.com
The IGNORE NULLS implementation caches whether a window function argument
evaluated to NULL or NOT NULL for a given partition row. That is safe for
ordinary expressions, but not for volatile expressions, where evaluating the
same argument on the same row can produce a different NULL/NOT NULL result
later.
This could produce wrong results in two ways. A row previously cached as
NULL could be skipped even though a later evaluation would return NOT NULL.
Conversely, a row cached as NOT NULL could be chosen as the target row, then
re-evaluated to fetch the actual value and return NULL.
Make the nullness cache conditional per argument. Do not use it for
arguments containing volatile functions or subplans, following the same
conservative approach used for moving window aggregates. Also avoid
re-evaluating non-cacheable partition arguments after the scan has already
found the target row.
Add regression tests covering volatile arguments and subplan arguments with
IGNORE NULLS.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Tatsuo Ishii <ishii@postgresql.org>
Discussion: https://postgr.es/m/42B42506-6972-4266-8422-FB73E61D9DA7@gmail.com
This commit moves the definitions of InjectionPointConditionType and
InjectionPointCondition into a new header local to the test module
injection_points.h, so as these can be shared across more files in the
module. A patch for a bug fix is under discussion, whose proposed test
will benefit from this refactoring.
Backpatch down to where the module exists, as this should be useful for
future bug fixes, even cases unrelated to the thread where this change
has been discussed.
Author: Andrey Borodin <x4mmm@yandex-team.ru>
Author: Vlad Lesin <vladlesin@gmail.com>
Discussion: https://postgr.es/m/d2983796-2603-41b7-a66e-fc8489ddb954@gmail.com
Backpatch-through: 17
Three locations use Assert() to guard against a mismatch between the
number of columns advertised in the RELATION message and the number
actually received in the subsequent INSERT/UPDATE tuple message. Since
these values originate from the publisher, the check must survive into
production builds.
A malicious or buggy publisher can send a RELATION claiming N columns
and an INSERT claiming M < N columns. The subscriber's apply worker
indexes into colvalues[]/colstatus[] using column indices from the
RELATION message's attribute map, causing a heap out-of-bounds read when
the tuple's column array is smaller than expected. We've looked, without
success, for a scenario in which the publisher holds sufficient control
over these out-of-bounds bytes to exploit this or even to reach a
SIGSEGV. Despite not finding one, the code has been fragile. Back-patch
to v14 (all supported versions).
Reported-by: Varik Matevosyan <varikmatevosyan@gmail.com>
Author: Varik Matevosyan <varikmatevosyan@gmail.com>
Discussion: https://postgr.es/m/CA+bBoog3cCogktzfLb9bppUByu-10B3CFp8u=iKXG_OvtAguCw@mail.gmail.com
Backpatch-through: 14
There is now only one caller of ProcessStartupPacket(). Let's simplify
the routine so as the GSS and SSL states are tracked inside it. If
future callers are added, there is less guessing to do.
Suggested-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://postgr.es/m/aga7lCWluyc5zLb5@paquier.xyz
In some cases its necessary to understand whether TSC frequency data was
sourced from CPUID, and which of the registers. Show this debug info at
the end of pg_test_timing, and rework TSC functions to support that.
This would have helped debug the buildfarm report fixed in 7fc36c5db5
and is likely going to aid in any TSC-related issues reported during the
beta period or later.
Additionally, emit a warning if TSC frequency from calibration differs
by more than 10% from the TSC frequency in use, and suggest the use
of timing_clock_source = 'system'.
In passing, add an explicit early return in the output function if the
loop count is zero. This can't happen in practice, but coverity complained
because we unconditionally call output for the fast TSC measurement.
Author: Lukas Fittl <lukas@fittl.com>
Suggested-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Haibo Yan <tristan.yim@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> (coverity fix only)
Discussion: https://postgr.es/m/CAP53Pkw3Gzb+KTF5pu_o7tzbfZ7+qm2m6uDWuGtTJjZpV9yNpg@mail.gmail.com
Commit 28972b6fc ("Add support for importing statistics from remote
servers.") stored the names of local/remote columns for a foreign table
into the buffers of NAMEDATALEN bytes in this structure, without
accounting for the possibility that the remote column name in particular
could be longer than NAMEDATALEN - 1. If it was longer than that, this
would leave it unterminated/truncated in the buffer, invoking undefined
behavior when match_attrmap() processes it, which assumes that it's
fully-contained/terminated in the buffer.
To fix, replace the buffers with char pointers, pstrdup the local/remote
column names, and store the results into the pointers. This commit also
adds a function to clean up the nested data structure.
Per Coverity and Tom Lane.
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Author: Corey Huinker <corey.huinker@gmail.com>
Reviewed-by: Etsuro Fujita <etsuro.fujita@gmail.com>
Discussion: https://postgr.es/m/342868.1776017700%40sss.pgh.pa.us
Previously, the subscription setting retain_dead_tuples didn't cause
ALTER SUBSCRIPTION ... SERVER to check the publisher. And if the
publisher was checked for some other reason, then it would use the old
conninfo.
Fix ALTER SUBSCRIPTION ... SERVER to always check the publisher when
retain_dead_tuples is set, and to use the new connection info, like
ALTER SUBSCRIPTION ... CONNECTION.
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/f13a8e29410bbbf9999290f2c04513a8884fa51c.camel@j-davis.com
These tests have been removed by 906ea101d0, due to some of them being
unstable in the buildfarm with low max_stack_depth values. They are now
reworked so as they should be more portable.
The tests to cover the findoprnd() overflows use a balanced tree to
avoid using too much stack, per a suggestion and an investigation by Tom
Lane.
Note: This is initially applied only on HEAD; a backpatch will follow
should the buildfarm be fine with the situation.
Discussion: https://postgr.es/m/agZc6XecyE7E7fep@paquier.xyz
Backpatch-through: 14
Previously, tab completion for REPACK parenthesized boolean options
(ANALYZE, CONCURRENTLY, and VERBOSE) did not suggest the boolean values
ON and OFF, unlike VACUUM.
This commit fixes the issue by adding ON/OFF completion for those options.
Author: Baji Shaik <baji.pgdev@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CA+fm-RNZpy7MAceR9gSyy833H_uL-fTx0LxO73RnvwEaprpuRA@mail.gmail.com
When an UPDATE statement triggers check_foreign_key() with the
action set to "cascade", it generates more UPDATE statements to
modify the key values in referencing relations. If a new key value
is NULL, SPI_getvalue() returns a NULL pointer, which is
subsequently passed to quote_literal_cstr(), causing a segfault.
To fix, skip quoting when a new key value is NULL and insert an
unquoted NULL keyword instead.
Oversight in commit 260e97733b. While the refint documentation
recommends marking primary key columns NOT NULL, the aforementioned
scenario accidentally worked on platforms where snprintf()
substitutes "(null)" for NULL pointers. Note that for
character-type columns, the old code quoted "(null)" as a string
literal, so this didn't always produce correct results. But it
still seems better to fix this than to reject cases that previously
worked.
Reported-by: Nikita Kalinin <n.kalinin@postgrespro.ru>
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Pierre Forstmann <pierre.forstmann@gmail.com>
Discussion: https://postgr.es/m/19476-bd04ea6241345303%40postgresql.org
Backpatch-through: 14
Commit 4bea91f21f enabled COPY TO on a partitioned table to read
tuples from its partitions and mapped them to the root table's tuple
descriptor before output. However, it incorrectly built the attribute
map from the root table to the partition.
This commit fixes by building the attribute map from the partition to
the root table, ensuring that partition attributes are correctly
mapped to their corresponding root attributes.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/85EA70F3-C3DB-477B-B856-EA569FDAAE7C@gmail.com
Commit b7b0f3f272 ("Use streaming I/O in sequential scans") routed
sequential scans through read_stream_next_buffer(), bypassing the
RELATION_IS_OTHER_TEMP() check in ReadBufferExtended(). As a result,
a superuser can attempt to read or modify temp tables of other
sessions through the read-stream path. When the query plan uses no index,
SELECT/UPDATE/DELETE/MERGE silently see no rows / report zero affected rows,
and COPY produces an empty output -- because the buffer manager has no
visibility into the owning session's local buffers and silently returns
nothing. Any query plan that uses, for instance, a btree index
still errors out via the existing check in ReadBufferExtended(), which
is reached from hio.c and nbtree respectively, but this is incidental.
Fix by enforcing RELATION_IS_OTHER_TEMP() at the three additional
buffer-manager entry points:
- read_stream_begin_impl() rejects the read at stream setup time,
covering sequential and bitmap scans that go through the
read-stream path.
- ReadBuffer_common() becomes the canonical place for the check,
consolidating the existing one previously kept in
ReadBufferExtended(). All ReadBufferExtended() callers go through
ReadBuffer_common(), so the consolidation is behavior-preserving.
- StartReadBuffersImpl() catches direct callers of StartReadBuffers()
that bypass both of the above. This is currently defense-in-depth,
but documents the contract for future code.
The companion test in src/test/modules/test_misc was added in the
preceding commit; this commit updates the assertions for SELECT,
UPDATE, DELETE, MERGE, and COPY (which previously documented the
bug as silent success) to expect the new error.
Author: Jim Jones <jim.jones@uni-muenster.de>
Author: Daniil Davydov <3danissimo@gmail.com>
Co-authored-by: Alexander Korotkov <aekorotkov@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Soumya S Murali <soumyamurali.work@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAJDiXghdFcZ8%3Dnh4G69te7iRr3Q0uFyXxb3ZdG09_GTNZXwH0g%40mail.gmail.com
Backpatch-through: 17
