Commit graph

5870 commits

Author SHA1 Message Date
Tom Lane
76123ded6e Fix improper interactions between session_authorization and role.
The SQL spec mandates that SET SESSION AUTHORIZATION implies
SET ROLE NONE.  We tried to implement that within the lowest-level
functions that manipulate these settings, but that was a bad idea.
In particular, guc.c assumes that it doesn't matter in what order
it applies GUC variable updates, but that was not the case for these
two variables.  This problem, compounded by some hackish attempts to
work around it, led to some security-grade issues:

* Rolling back a transaction that had done SET SESSION AUTHORIZATION
would revert to SET ROLE NONE, even if that had not been the previous
state, so that the effective user ID might now be different from what
it had been.

* The same for SET SESSION AUTHORIZATION in a function SET clause.

* If a parallel worker inspected current_setting('role'), it saw
"none" even when it should see something else.

Also, although the parallel worker startup code intended to cope
with the current role's pg_authid row having disappeared, its
implementation of that was incomplete so it would still fail.

Fix by fully separating the miscinit.c functions that assign
session_authorization from those that assign role.  To implement the
spec's requirement, teach set_config_option itself to perform "SET
ROLE NONE" when it sets session_authorization.  (This is undoubtedly
ugly, but the alternatives seem worse.  In particular, there's no way
to do it within assign_session_authorization without incompatible
changes in the API for GUC assign hooks.)  Also, improve
ParallelWorkerMain to directly set all the relevant user-ID variables
instead of relying on some of them to get set indirectly.  That
allows us to survive not finding the pg_authid row during worker
startup.

In v16 and earlier, this includes back-patching 9987a7bf3 which
fixed a violation of GUC coding rules: SetSessionAuthorization
is not an appropriate place to be throwing errors from.

Security: CVE-2024-10978
2024-11-11 10:29:54 -05:00
Nathan Bossart
952ff31e2a Ensure cached plans are correctly marked as dependent on role.
If a CTE, subquery, sublink, security invoker view, or coercion
projection references a table with row-level security policies, we
neglected to mark the plan as potentially dependent on which role
is executing it.  This could lead to later executions in the same
session returning or hiding rows that should have been hidden or
returned instead.

Reported-by: Wolfgang Walther
Reviewed-by: Noah Misch
Security: CVE-2024-10976
Backpatch-through: 12
2024-11-11 09:00:00 -06:00
Noah Misch
e428cd058f Block environment variable mutations from trusted PL/Perl.
Many process environment variables (e.g. PATH), bypass the containment
expected of a trusted PL.  Hence, trusted PLs must not offer features
that achieve setenv().  Otherwise, an attacker having USAGE privilege on
the language often can achieve arbitrary code execution, even if the
attacker lacks a database server operating system user.

To fix PL/Perl, replace trusted PL/Perl %ENV with a tied hash that just
replaces each modification attempt with a warning.  Sites that reach
these warnings should evaluate the application-specific implications of
proceeding without the environment modification:

  Can the application reasonably proceed without the modification?

    If no, switch to plperlu or another approach.

    If yes, the application should change the code to stop attempting
    environment modifications.  If that's too difficult, add "untie
    %main::ENV" in any code executed before the warning.  For example,
    one might add it to the start of the affected function or even to
    the plperl.on_plperl_init setting.

In passing, link to Perl's guidance about the Perl features behind the
security posture of PL/Perl.

Back-patch to v12 (all supported versions).

Andrew Dunstan and Noah Misch

Security: CVE-2024-10979
2024-11-11 06:23:48 -08:00
Amit Langote
054701a2b7 Disallow partitionwise join when collations don't match
If the collation of any join key column doesn’t match the collation of
the corresponding partition key, partitionwise joins can yield incorrect
results. For example, rows that would match under the join key collation
might be located in different partitions due to the partitioning
collation. In such cases, a partitionwise join would yield different
results from a non-partitionwise join, so disallow it in such cases.

Reported-by: Tender Wang <tndrwang@gmail.com>
Author: Jian He <jian.universality@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Reviewed-by: Junwang Zhao <zhjwpku@gmail.com>
Discussion: https://postgr.es/m/CAHewXNno_HKiQ6PqyLYfuqDtwp7KKHZiH1J7Pqyz0nr+PS2Dwg@mail.gmail.com
Backpatch-through: 12
2024-11-08 17:18:21 +09:00
Amit Langote
ff65f695c0 Disallow partitionwise grouping when collations don't match
If the collation of any grouping column doesn’t match the collation of
the corresponding partition key, partitionwise grouping can yield
incorrect results. For example, rows that would be grouped under the
grouping collation may end up in different partitions under the
partitioning collation. In such cases, full partitionwise grouping
would produce results that differ from those without partitionwise
grouping, so disallowed that.

Partial partitionwise aggregation is still allowed, as the Finalize
step reconciles partition-level aggregates with grouping requirements
across all partitions, ensuring that the final output remains
consistent.

This commit also fixes group_by_has_partkey() by ensuring the
RelabelType node is stripped from grouping expressions when matching
them to partition key expressions to avoid false mismatches.

Bug: #18568
Reported-by: Webbo Han <1105066510@qq.com>
Author: Webbo Han <1105066510@qq.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Reviewed-by: Aleksander Alekseev <aleksander@timescale.com>
Reviewed-by: Jian He <jian.universality@gmail.com>
Discussion: https://postgr.es/m/18568-2a9afb6b9f7e6ed3@postgresql.org
Discussion: https://postgr.es/m/tencent_9D9103CDA420C07768349CC1DFF88465F90A@qq.com
Discussion: https://postgr.es/m/CAHewXNno_HKiQ6PqyLYfuqDtwp7KKHZiH1J7Pqyz0nr+PS2Dwg@mail.gmail.com
Backpatch-through: 12
2024-11-08 16:06:12 +09:00
Peter Eisentraut
ebbfa2ae34 Message style improvement
Backpatch the part of edee0c621d that applies to a90bdd7a44, which
was also backpatched.  That way, the message is consistent in all
branches.
2024-11-08 07:32:14 +01:00
Noah Misch
fe8091c9e3 Revert "For inplace update, send nontransactional invalidations."
This reverts commit 95c5acb3fc (v17) and
counterparts in each other non-master branch.  If released, that commit
would have caused a worst-in-years minor release regression, via
undetected LWLock self-deadlock.  This commit and its self-deadlock fix
warrant more bake time in the master branch.

Reported by Alexander Lakhin.

Discussion: https://postgr.es/m/10ec0bc3-5933-1189-6bb8-5dec4114558e@gmail.com
2024-11-02 09:05:07 -07:00
Noah Misch
0ea9d40a66 For inplace update, send nontransactional invalidations.
The inplace update survives ROLLBACK.  The inval didn't, so another
backend's DDL could then update the row without incorporating the
inplace update.  In the test this fixes, a mix of CREATE INDEX and ALTER
TABLE resulted in a table with an index, yet relhasindex=f.  That is a
source of index corruption.  Back-patch to v12 (all supported versions).
The back branch versions don't change WAL, because those branches just
added end-of-recovery SIResetAll().  All branches change the ABI of
extern function PrepareToInvalidateCacheTuple().  No PGXN extension
calls that, and there's no apparent use case in extensions.

Reviewed by Nitin Motiani and (in earlier versions) Andres Freund.

Discussion: https://postgr.es/m/20240523000548.58.nmisch@google.com
2024-10-25 06:51:08 -07:00
Noah Misch
3e5ea478d8 Stop reading uninitialized memory in heap_inplace_lock().
Stop computing a never-used value.  This removes the read; the read had
no functional implications.  Back-patch to v12, like commit
a07e03fd8f.

Reported by Alexander Lakhin.

Discussion: https://postgr.es/m/6c92f59b-f5bc-e58c-9bdd-d1f21c17c786@gmail.com
2024-10-24 09:16:19 -07:00
Álvaro Herrera
d20194cead
Restructure foreign key handling code for ATTACH/DETACH
... to fix bugs when the referenced table is partitioned.

The catalog representation we chose for foreign keys connecting
partitioned tables (in commit f56f8f8da6) is inconvenient, in the
sense that a standalone table has a different way to represent the
constraint when referencing a partitioned table, than when the same
table becomes a partition (and vice versa).  Because of this, we need to
create additional catalog rows on detach (pg_constraint and pg_trigger),
and remove them on attach.  We were doing some of those things, but not
all of them, leading to missing catalog rows in certain cases.

The worst problem seems to be that we are missing action triggers after
detaching a partition, which means that you could update/delete rows
from the referenced partitioned table that still had referencing rows on
that table, the server failing to throw the required errors.

!!!
Note that this means existing databases with FKs that reference
partitioned tables might have rows that break relational integrity, on
tables that were once partitions on the referencing side of the FK.

Another possible problem is that trying to reattach a table
that had been detached would fail indicating that internal triggers
cannot be found, which from the user's point of view is nonsensical.

In branches 15 and above, we fix this by creating a new helper function
addFkConstraint() which is in charge of creating a standalone
pg_constraint row, and repurposing addFkRecurseReferencing() and
addFkRecurseReferenced() so that they're only the recursive routine for
each side of the FK, and they call addFkConstraint() to create
pg_constraint at each partitioning level and add the necessary triggers.
These new routines can be used during partition creation, partition
attach and detach, and foreign key creation.  This reduces redundant
code and simplifies the flow.

In branches 14 and 13, we have a much simpler fix that consists on
simply removing the constraint on detach.  The reason is that those
branches are missing commit f4566345cf, which reworked the way this
works in a way that we didn't consider back-patchable at the time.

We opted to leave branch 12 alone, because it's different from branch 13
enough that the fix doesn't apply; and because it is going in EOL mode
very soon, patching it now might be worse since there's no way to undo
the damage if it goes wrong.

Existing databases might need to be repaired.

In the future we might want to rethink the catalog representation to
avoid this problem, but for now the code seems to do what's required to
make the constraints operate correctly.

Co-authored-by: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
Co-authored-by: Tender Wang <tndrwang@gmail.com>
Co-authored-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Reported-by: Guillaume Lelarge <guillaume@lelarge.info>
Reported-by: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
Reported-by: Thomas Baehler (SBB CFF FFS) <thomas.baehler2@sbb.ch>
Discussion: https://postgr.es/m/20230420144344.40744130@karst
Discussion: https://postgr.es/m/20230705233028.2f554f73@karst
Discussion: https://postgr.es/m/GVAP278MB02787E7134FD691861635A8BC9032@GVAP278MB0278.CHEP278.PROD.OUTLOOK.COM
Discussion: https://postgr.es/m/18541-628a61bc267cd2d3@postgresql.org
2024-10-22 16:01:18 +02:00
Tom Lane
beab395a42 Fix wrong assertion and poor error messages in "COPY (query) TO".
If the query is rewritten into a NOTIFY command by a DO INSTEAD
rule, we'd get an assertion failure, or in non-assert builds
issue a rather confusing error message.  Improve that.

Also fix a longstanding grammar mistake in a nearby error message.

Per bug #18664 from Alexander Lakhin.  Back-patch to all supported
branches.

Tender Wang and Tom Lane

Discussion: https://postgr.es/m/18664-ffd0ebc2386598df@postgresql.org
2024-10-21 15:08:22 -04:00
Tom Lane
76de4b182c Correctly identify which EC members are computable at a plan node.
find_computable_ec_member() had the wrong mental model of what
its primary caller prepare_sort_from_pathkeys() would do with
the selected EquivalenceClass member expression.  We will not
compute the EC expression in a plan node atop the one returning
the passed-in targetlist; rather, the EC expression will be
computed as an additional column of that targetlist.  So any
Var or quasi-Var used in the given tlist is also available to the
EC expression.  In simple cases this makes no difference because
the given tlist is just a list of Vars or quasi-Vars --- but if
we are considering an appendrel member produced by flattening
a UNION ALL, the tlist may contain expressions, resulting in
failure to match and a "could not find pathkey item to sort"
error.

To fix, we can flatten both the tlist and the EC members with
pull_var_clause(), and then just check for subset-ness, so
that the code is actually shorter than before.

While this bug is quite old, the present patch only works back to
v13.  We could possibly make it work in v12 by back-patching parts
of 375398244.  On the whole though I don't like the risk/reward
ratio of that idea.  v12's final release is next month, meaning
there would be no chance to correct matters if the patch causes a
regression.  Since this failure has escaped notice for 14 years,
it's likely nobody will hit it in the field with v12.

Per bug #18652 from Alexander Lakhin.

Andrei Lepikhov and Tom Lane

Discussion: https://postgr.es/m/18652-deaa782ebcca85d1@postgresql.org
2024-10-12 14:56:08 -04:00
Noah Misch
118dfd1213 Avoid 037_invalid_database.pl hang under debug_discard_caches.
Back-patch to v12 (all supported versions).
2024-09-27 15:29:01 -07:00
Andres Freund
f232d7c682 tests: Restrict pg_locks queries in advisory_locks.sql to current database
Otherwise testing an existing installation can fail, if there are other locks,
e.g. from one of the isolation tests.

This was originally applied as c3315a7da5 in 16~, but it is possible
to see this test fail depending on the concurrent activity for older
active branches.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/20221003234111.4ob7yph6r4g4ywhu@awork3.anarazel.de
Backpatch-through: 12
2024-09-26 13:46:40 +09:00
Noah Misch
14c57cb639 For inplace update durability, make heap_update() callers wait.
The previous commit fixed some ways of losing an inplace update.  It
remained possible to lose one when a backend working toward a
heap_update() copied a tuple into memory just before inplace update of
that tuple.  In catalogs eligible for inplace update, use LOCKTAG_TUPLE
to govern admission to the steps of copying an old tuple, modifying it,
and issuing heap_update().  This includes MERGE commands.  To avoid
changing most of the pg_class DDL, don't require LOCKTAG_TUPLE when
holding a relation lock sufficient to exclude inplace updaters.
Back-patch to v12 (all supported versions).  In v13 and v12, "UPDATE
pg_class" or "UPDATE pg_database" can still lose an inplace update.  The
v14+ UPDATE fix needs commit 86dc90056d,
and it wasn't worth reimplementing that fix without such infrastructure.

Reviewed by Nitin Motiani and (in earlier versions) Heikki Linnakangas.

Discussion: https://postgr.es/m/20231027214946.79.nmisch@google.com
2024-09-24 15:25:24 -07:00
Noah Misch
a8ad1929d2 Fix data loss at inplace update after heap_update().
As previously-added tests demonstrated, heap_inplace_update() could
instead update an unrelated tuple of the same catalog.  It could lose
the update.  Losing relhasindex=t was a source of index corruption.
Inplace-updating commands like VACUUM will now wait for heap_update()
commands like GRANT TABLE and GRANT DATABASE.  That isn't ideal, but a
long-running GRANT already hurts VACUUM progress more just by keeping an
XID running.  The VACUUM will behave like a DELETE or UPDATE waiting for
the uncommitted change.

For implementation details, start at the systable_inplace_update_begin()
header comment and README.tuplock.  Back-patch to v12 (all supported
versions).  In back branches, retain a deprecated heap_inplace_update(),
for extensions.

Reported by Smolkin Grigory.  Reviewed by Nitin Motiani, (in earlier
versions) Heikki Linnakangas, and (in earlier versions) Alexander
Lakhin.

Discussion: https://postgr.es/m/CAMp+ueZQz3yDk7qg42hk6-9gxniYbp-=bG2mgqecErqR5gGGOA@mail.gmail.com
2024-09-24 15:25:24 -07:00
Daniel Gustafsson
e43e71b6cd Drop global objects after completed test
Project policy is to not leave global objects behind after a regress
test run.  This was found as a result of the development of a patch
to make pg_regress detect such leftovers automatically, which in the
end was withdrawn due to issues with parallel runs.

This was originally committed as 936e3fa378, but the issue also exists
in the 12~16 range.

Discussion: https://postgr.es/m/E1phvk7-000VAH-7k@gemulon.postgresql.org
Backpatch-through: 12
2024-09-24 12:10:33 +09:00
Noah Misch
916b8ae475 Don't enter parallel mode when holding interrupts.
Doing so caused the leader to hang in wait_event=ParallelFinish, which
required an immediate shutdown to resolve.  Back-patch to v12 (all
supported versions).

Francesco Degrassi

Discussion: https://postgr.es/m/CAC-SaSzHUKT=vZJ8MPxYdC_URPfax+yoA1hKTcF4ROz_Q6z0_Q@mail.gmail.com
2024-09-17 19:54:26 -07:00
Tom Lane
b28b9b19bb Run regression tests with timezone America/Los_Angeles.
Historically we've used timezone "PST8PDT", but the recent release
2024b of tzdb changes the definition of that zone in a way that
breaks many test cases concerned with dates before 1970.  Although
we've not yet adopted 2024b into our own tree, this is already
problematic for people using --with-system-tzdata if their platform
has already adopted 2024b.  To work with both older and newer
versions of tzdb, switch to using "America/Los_Angeles", accepting
the ensuing changes in regression test results.

Back-patch to all supported branches.

Per report and patch from Wolfgang Walther.

Discussion: https://postgr.es/m/0a997455-5aba-4cf2-a354-d26d8bcbfae6@technowledgy.de
2024-09-14 17:55:03 -04:00
Thomas Munro
d1d0fe1feb Stabilize 039_end_of_wal test.
The first test was sensitive to the insert LSN after setting up the
catalogs, which depended on environmental things like the locales on the
OS and usernames.  Switch to a new WAL file before the first test, as a
simple way to put every computer into the same state.

Back-patch to all supported releases.

Reported-by: Anton Voloshin <a.voloshin@postgrespro.ru>
Reported-by: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Discussion: https://postgr.es/m/b26aeac2-cb6d-4633-a7ea-945baae83dcf%40postgrespro.ru
2024-08-31 14:55:40 +12:00
Peter Eisentraut
7589d5c5b9 Disallow USING clause when altering type of generated column
This does not make sense.  It would write the output of the USING
clause into the converted column, which would violate the generation
expression.  This adds a check to error out if this is specified.

There was a test for this, but that test errored out for a different
reason, so it was not effective.

Reported-by: Jian He <jian.universality@gmail.com>
Reviewed-by: Yugo NAGATA <nagata@sraoss.co.jp>
Discussion: https://www.postgresql.org/message-id/flat/c7083982-69f4-4b14-8315-f9ddb20b9834%40eisentraut.org
2024-08-29 09:02:06 +02:00
Tom Lane
adf9808fa9 Allow adjusting session_authorization and role in parallel workers.
The code intends to allow GUCs to be set within parallel workers
via function SET clauses, but not otherwise.  However, doing so fails
for "session_authorization" and "role", because the assign hooks for
those attempt to set the subsidiary "is_superuser" GUC, and that call
falls foul of the "not otherwise" prohibition.  We can't switch to
using GUC_ACTION_SAVE for this, so instead add a new GUC variable
flag GUC_ALLOW_IN_PARALLEL to mark is_superuser as being safe to set
anyway.  (This is okay because is_superuser has context PGC_INTERNAL
and thus only hard-wired calls can change it.  We'd need more thought
before applying the flag to other GUCs; but maybe there are other
use-cases.)  This isn't the prettiest fix perhaps, but other
alternatives we thought of would be much more invasive.

While here, correct a thinko in commit 059de3ca4: when rejecting
a GUC setting within a parallel worker, we should return 0 not -1
if the ereport doesn't longjmp.  (This seems to have no consequences
right now because no caller cares, but it's inconsistent.)  Improve
the comments to try to forestall future confusion of the same kind.

Despite the lack of field complaints, this seems worth back-patching.
Thanks to Nathan Bossart for the idea to invent a new flag,
and for review.

Discussion: https://postgr.es/m/2833457.1723229039@sss.pgh.pa.us
2024-08-10 15:51:28 -04:00
Tom Lane
3ad35d5022 Fix "failed to find plan for subquery/CTE" errors in EXPLAIN.
To deparse a reference to a field of a RECORD-type output of a
subquery, EXPLAIN normally digs down into the subquery's plan to try
to discover exactly which anonymous RECORD type is meant.  However,
this can fail if the subquery has been optimized out of the plan
altogether on the grounds that no rows could pass the WHERE quals,
which has been possible at least since 3fc6e2d7f.  There isn't
anything remaining in the plan tree that would help us, so fall back
to printing the field name as "fN" for the N'th column of the record.
(This will actually be the right thing some of the time, since it
matches the column names we assign to RowExprs.)

In passing, fix a comment typo in create_projection_plan, which
I noticed while experimenting with an alternative fix for this.

Per bug #18576 from Vasya B.  Back-patch to all supported branches.

Richard Guo and Tom Lane

Discussion: https://postgr.es/m/18576-9feac34e132fea9e@postgresql.org
2024-08-09 11:21:39 -04:00
Alvaro Herrera
2ee02c98dd
Refuse ATTACH of a table referenced by a foreign key
Trying to attach a table as a partition which is already on the
referenced side of a foreign key on the partitioned table that it is
being attached to, leads to strange behavior: we try to clone the
foreign key from the parent to the partition, but this new FK points to
the partition itself, and the mix of pg_constraint rows and triggers
doesn't behave well.

Rather than trying to untangle the mess (which might be possible given
sufficient time), I opted to forbid the ATTACH.  This doesn't seem a
problematic restriction, given that we already fail to create the
foreign key if you do it the other way around, that is, having the
partition first and the FK second.

Backpatch to all supported branches.

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Discussion: https://postgr.es/m/18541-628a61bc267cd2d3@postgresql.org
2024-08-08 19:35:13 -04:00
Noah Misch
382909b635 Teach RPM the package name provided in Perl alias packages.
When commit 1185be3554 introduced
installation of a file containing "use PostgreSQL::Test::Utils", the RPM
Package Manager said "nothing provides perl(PostgreSQL::Test::Utils)".
Discussed on pgsql-packagers.  Back-patch to v12, v13, and v14 only;
newer versions don't have the alias packages.

Reviewed by Andrew Dunstan, Tom Lane, and John Harvey.  Reported by John
Harvey.
2024-08-07 11:43:40 -07:00
Masahiko Sawada
bbc94abf66 Restrict accesses to non-system views and foreign tables during pg_dump.
When pg_dump retrieves the list of database objects and performs the
data dump, there was possibility that objects are replaced with others
of the same name, such as views, and access them. This vulnerability
could result in code execution with superuser privileges during the
pg_dump process.

This issue can arise when dumping data of sequences, foreign
tables (only 13 or later), or tables registered with a WHERE clause in
the extension configuration table.

To address this, pg_dump now utilizes the newly introduced
restrict_nonsystem_relation_kind GUC parameter to restrict the
accesses to non-system views and foreign tables during the dump
process. This new GUC parameter is added to back branches too, but
these changes do not require cluster recreation.

Back-patch to all supported branches.

Reviewed-by: Noah Misch
Security: CVE-2024-7348
Backpatch-through: 12
2024-08-05 06:05:20 -07:00
Tom Lane
33c5baff66 Revert "Allow parallel workers to cope with a newly-created session user ID."
This reverts commit 216201027d.

Some buildfarm animals are failing with "cannot change
"client_encoding" during a parallel operation".  It looks like
assign_client_encoding is unhappy at being asked to roll back a
client_encoding setting after a parallel worker encounters a
failure.  There must be more to it though: why didn't I see this
during local testing?  In any case, it's clear that moving the
RestoreGUCState() call is not as side-effect-free as I thought.
Given that the bug f5f30c22e intended to fix has gone unreported
for years, it's not something that's urgent to fix; I'm not
willing to risk messing with it further with only days to our
next release wrap.
2024-07-31 20:56:11 -04:00
Tom Lane
216201027d Allow parallel workers to cope with a newly-created session user ID.
Parallel workers failed after a sequence like
	BEGIN;
	CREATE USER foo;
	SET SESSION AUTHORIZATION foo;
because check_session_authorization could not see the uncommitted
pg_authid row for "foo".  This is because we ran RestoreGUCState()
in a separate transaction using an ordinary just-created snapshot.
The same disease afflicts any other GUC that requires catalog lookups
and isn't forgiving about the lookups failing.

To fix, postpone RestoreGUCState() into the worker's main transaction
after we've set up a snapshot duplicating the leader's.  This affects
check_transaction_isolation and check_transaction_deferrable, which
think they should only run during transaction start.  Make them
act like check_transaction_read_only, which already knows it should
silently accept the value when InitializingParallelWorker.

Per bug #18545 from Andrey Rachitskiy.  Back-patch to all
supported branches, because this has been wrong for awhile.

Discussion: https://postgr.es/m/18545-feba138862f19aaa@postgresql.org
2024-07-31 18:54:10 -04:00
Alvaro Herrera
ed74309755
Reset relhassubclass upon attaching table as a partition
We don't allow inheritance parents as partitions, and have checks to
prevent this; but if a table _was_ in the past an inheritance parents
and all their children are removed, the pg_class.relhassubclass flag
may remain set, which confuses the partition pruning code (most
obviously, it results in an assertion failure; in production builds it
may be worse.)

Fix by resetting relhassubclass on attach.

Backpatch to all supported versions.

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/18550-d5e047e9a897a889@postgresql.org
2024-07-24 12:38:18 +02:00
Nathan Bossart
6c1b71bc6a Detect integer overflow in array_set_slice().
When provided an empty initial array, array_set_slice() fails to
check for overflow when computing the new array's dimensions.
While such overflows are ordinarily caught by ArrayGetNItems(),
commands with the following form are accepted:

	INSERT INTO t (i[-2147483648:2147483647]) VALUES ('{}');

To fix, perform the hazardous computations using overflow-detecting
arithmetic routines.  As with commit 18b585155a, the added test
cases generate errors that include a platform-dependent value, so
we again use psql's VERBOSITY parameter to suppress printing the
message text.

Reported-by: Alexander Lakhin
Author: Joseph Koshakow
Reviewed-by: Jian He
Discussion: https://postgr.es/m/31ad2cd1-db94-bdb3-f91a-65ffdb4bef95%40gmail.com
Backpatch-through: 12
2024-07-23 21:59:02 -05:00
Tom Lane
461f479485 Correctly check updatability of columns targeted by INSERT...DEFAULT.
If a view has some updatable and some non-updatable columns, we failed
to verify updatability of any columns for which an INSERT or UPDATE
on the view explicitly specifies a DEFAULT item (unless the view has
a declared default for that column, which is rare anyway, and one
would almost certainly not write one for a non-updatable column).
This would lead to an unexpected "attribute number N not found in
view targetlist" error rather than the intended error.

Per bug #18546 from Alexander Lakhin.  This bug is old, so back-patch
to all supported branches.

Discussion: https://postgr.es/m/18546-84a292e759a9361d@postgresql.org
2024-07-20 13:40:15 -04:00
Nathan Bossart
c5321e9651 Add overflow checks to money type.
None of the arithmetic functions for the the money type handle
overflow.  This commit introduces several helper functions with
overflow checking and makes use of them in the money type's
arithmetic functions.

Fixes bug #18240.

Reported-by: Alexander Lakhin
Author: Joseph Koshakow
Discussion: https://postgr.es/m/18240-c5da758d7dc1ecf0%40postgresql.org
Discussion: https://postgr.es/m/CAAvxfHdBPOyEGS7s%2Bxf4iaW0-cgiq25jpYdWBqQqvLtLe_t6tw%40mail.gmail.com
Backpatch-through: 12
2024-07-19 11:52:32 -05:00
Andrew Dunstan
5ee78e3038 Avoid error in recovery test if history file is not yet present
Error was detected when testing use of libpq sessions instead of psql
for polling queries.

Discussion: https://postgr.es/m/e86b6d2d-20d8-4ac9-9a98-165fff7db886@dunslane.net

Backpatch to all live branches
2024-07-17 10:42:23 -04:00
Tom Lane
b020a866a2 Avoid unhelpful internal error for incorrect recursive-WITH queries.
checkWellFormedRecursion would issue "missing recursive reference"
if a WITH RECURSIVE query contained a single self-reference but
that self-reference was inside a top-level WITH, ORDER BY, LIMIT,
etc, rather than inside the second arm of the UNION as expected.
We already intended to throw more-on-point errors for such cases,
but those error checks must be done before examining the UNION arm
in order to have the desired results.  So this patch need only
move some code (and improve the comments).

Per bug #18536 from Alexander Lakhin.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/18536-0a342ec07901203e@postgresql.org
2024-07-14 13:49:46 -04:00
Alvaro Herrera
7898a494fd
Add ORDER BY to new test query
Per buildfarm.
2024-07-12 13:44:19 +02:00
Alvaro Herrera
0574825693
Fix ALTER TABLE DETACH for inconsistent indexes
When a partitioned table has an index that doesn't support a constraint,
but a partition has an equivalent index that does, then a DETACH
operation would misbehave: a crash in assertion-enabled systems (because
we fail to find the constraint in the parent that we expect to), or a
broken coninhcount value (-1) in production systems (because we blindly
believe that we've successfully detached the parent).

While we should reject an ATTACH of a partition with such an index, we
have failed to do so in existing releases, so adding an error in stable
releases might break the (unlikely) existing applications that rely on
this behavior.  At this point I don't even want to reject them in
master, because it'd break pg_upgrade if such databases exist, and there
would be no easy way to fix existing databases without expensive index
rebuilds.

(Later on we could add ALTER TABLE ... ADD CONSTRAINT USING INDEX to
partitioned tables, which would allow the user to fix such patterns.  At
that point we could add more restrictions to prevent the problem from
its root.)

Also, add a test case that leaves one table in this condition, so that
we can verify that pg_upgrade continues to work if we later decide to
change the policy on the master branch.

Backpatch to all supported branches.

Co-authored-by: Tender Wang <tndrwang@gmail.com>
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/18500-62948b6fe5522f56@postgresql.org
2024-07-12 12:54:01 +02:00
Tom Lane
48132587d9 Make our back branches compatible with libxml2 2.13.x.
This back-patches HEAD commits 066e8ac6e, 6082b3d5d, e7192486d,
and 896cd266f into supported branches.  Changes:

* Use xmlAddChildList not xmlAddChild in XMLSERIALIZE
(affects v16 and up only).  This was a flat-out coding mistake
that we got away with due to lax checking in previous versions
of xmlAddChild.

* Use xmlParseInNodeContext not xmlParseBalancedChunkMemory.
This is to dodge a bug in xmlParseBalancedChunkMemory in libxm2
releases 2.13.0-2.13.2.  While that bug is now fixed upstream and
will probably never be seen in any production-oriented distro, it is
currently a problem on some more-bleeding-edge-friendly platforms.

* Suppress "chunk is not well balanced" errors from libxml2,
unless it is the only error.  This eliminates an error-reporting
discrepancy between 2.13 and older releases.  This error is
almost always redundant with previous errors, if not flat-out
inappropriate, which is why 2.13 changed the behavior and why
nobody's likely to miss it.

Erik Wienhold and Tom Lane, per report from Frank Streitzig.

Discussion: https://postgr.es/m/trinity-b0161630-d230-4598-9ebc-7a23acdb37cb-1720186432160@3c-app-gmx-bap25
Discussion: https://postgr.es/m/trinity-361ba18b-541a-4fe7-bc63-655ae3a7d599-1720259822452@3c-app-gmx-bs01
2024-07-10 20:15:52 -04:00
Andrew Dunstan
c774d31f84 Choose ports for test servers less likely to result in conflicts
If we choose ports in the range typically used for ephemeral ports there
is a danger of encountering a port conflict due to a race condition
between the time we choose the port in a range below that typically used
to allocate ephemeral ports, but higher than the range typically used by
well known services.

Author: Jelte Fenema-Nio, with some editing by me.

Discussion: https://postgr.es/m/d6ee8761-39d1-0033-1afb-d5a57ee056f2@gmail.com

Backpatch to all live branches (12 and up)
2024-07-08 11:40:58 -04:00
Andrew Dunstan
94c2386878 Force nodes for SSL tests to start in TCP mode
Currently they are started in unix socket mode in ost cases, and then
converted to run in TCP mode. This can result in port collisions, and
there is no virtue in startng in unix socket mode, so start as we will
be going on.

Discussion: https://postgr.es/m/d6ee8761-39d1-0033-1afb-d5a57ee056f2@gmail.com

Backpatch to all live branches (12 and up).
2024-07-08 11:40:58 -04:00
Dean Rasheed
ece2969266 Fix scale clamping in numeric round() and trunc().
The numeric round() and trunc() functions clamp the scale argument to
the range between +/- NUMERIC_MAX_RESULT_SCALE (2000), which is much
smaller than the actual allowed range of type numeric. As a result,
they return incorrect results when asked to round/truncate more than
2000 digits before or after the decimal point.

Fix by using the correct upper and lower scale limits based on the
actual allowed (and documented) range of type numeric.

While at it, use the new NUMERIC_WEIGHT_MAX constant instead of
SHRT_MAX in all other overflow checks, and fix a comment thinko in
power_var() introduced by e54a758d24 -- the minimum value of
ln_dweight is -NUMERIC_DSCALE_MAX (-16383), not -SHRT_MAX, though this
doesn't affect the point being made in the comment, that the resulting
local_rscale value may exceed NUMERIC_MAX_DISPLAY_SCALE (1000).

Back-patch to all supported branches.

Dean Rasheed, reviewed by Joel Jacobson.

Discussion: https://postgr.es/m/CAEZATCXB%2BrDTuMjhK5ZxcouufigSc-X4tGJCBTMpZ3n%3DxxQuhg%40mail.gmail.com
2024-07-08 17:56:51 +01:00
Heikki Linnakangas
0d80e59a1b Fix missing installation/uninstallation rules for BackgroundPsql.pm
Commit d5fd7865 backported BackgroundPsql perl module module with
helper functions for tests running interactive or background psql
tasks to PG 12 to 15, but did not add installation/uninstallation
rules of the build system, causing problems running TAP tests for the
extensions.

Author: Pavan Deolasee <pavan.deolasee@gmail.com>
Discussion: https://www.postgresql.org/message-id/CABOikdPmRuZrcf_gtgXmQzZ5Tbg9yUJmqXDCAZ2aW%3DWi-PbDyQ%40mail.gmail.com
2024-07-01 19:25:44 +03:00
Noah Misch
ca30e8ba95 Remove configuration-dependent output from new inplace-inval test.
Per buildfarm members prion and trilobite.  Back-patch to v12 (all
supported versions), like commit
0844b39689.

Strategy reviewed by Tom Lane.

Discussion: https://postgr.es/m/20240628051353.a0.nmisch@google.com
2024-06-28 09:33:44 -07:00
Noah Misch
65aae93c45 Improve test coverage for changes to inplace-updated catalogs.
This covers both regular and inplace changes, since bugs arise at their
intersection.  Where marked, these witness extant bugs.  Back-patch to
v12 (all supported versions).

Reviewed (in an earlier version) by Robert Haas.

Discussion: https://postgr.es/m/20240512232923.aa.nmisch@google.com
2024-06-27 19:21:12 -07:00
Heikki Linnakangas
e9c8747ee9 Fix MVCC bug with prepared xact with subxacts on standby
We did not recover the subtransaction IDs of prepared transactions
when starting a hot standby from a shutdown checkpoint. As a result,
such subtransactions were considered as aborted, rather than
in-progress. That would lead to hint bits being set incorrectly, and
the subtransactions suddenly becoming visible to old snapshots when
the prepared transaction was committed.

To fix, update pg_subtrans with prepared transactions's subxids when
starting hot standby from a shutdown checkpoint. The snapshots taken
from that state need to be marked as "suboverflowed", so that we also
check the pg_subtrans.

Backport to all supported versions.

Discussion: https://www.postgresql.org/message-id/6b852e98-2d49-4ca1-9e95-db419a2696e0@iki.fi
2024-06-27 21:08:55 +03:00
Heikki Linnakangas
071e19a36c tests: Trim newline from result returned by BackgroundPsql->query
This went unnoticed, because only a few existing callers of
BackgroundPsql->query used the result, and the ones that did were not
bothered by an extra newline. I noticed because I was about to add a
new test that checks the result.

Backport to all supported versions, since I just backported the
BackgroundPsql facility to all supported versions too.
2024-06-27 21:08:49 +03:00
Heikki Linnakangas
12f327b210 Backport BackgroundPsql perl test module
Backport the new BackgroundPsql modules and the constructor functions,
background_psql() and interactive_psql, to all supported
branches. That makes it easier to backpatch tests that use it.

BackgroundPsql was introduced in version 16. On version 16, this
commit backports just the new timeout argument from master (commit
334f512f45). On older branches, the whole facility. This includes the
change to `use warnings FATAL => 'all'`, which we haven't otherwise
backported, but it seems good to keep the file identical across
branches.

Discussion: https://www.postgresql.org/message-id/b7c64f20-ea01-4f15-9088-0cd6832af149@iki.fi
2024-06-27 19:01:23 +03:00
Tom Lane
9ce8ee9d3e Don't throw an error if a queued AFTER trigger no longer exists.
afterTriggerInvokeEvents and AfterTriggerExecute have always
treated it as an error if the trigger OID mentioned in a queued
after-trigger event can't be found.  However, that fails to
account for the edge case where the trigger's been dropped in
the current transaction since queueing the event.  There seems
no very good reason to disallow that case, so instead silently
do nothing if the trigger OID can't be found.

This does give up a little bit of bug-detection ability, but I don't
recall that these error messages have ever actually revealed a bug,
so it seems mostly theoretical.  Alternatives such as marking
pending events DONE at the time of dropping a trigger would be
complicated and perhaps introduce bugs of their own.

Per bug #18517 from Alexander Lakhin.  Back-patch to all
supported branches.

Discussion: https://postgr.es/m/18517-af2d19882240902c@postgresql.org
2024-06-20 14:21:36 -04:00
Tom Lane
198de79612 Clean out column-level pg_init_privs entries when dropping tables.
DeleteInitPrivs did not get the memo about how, when dropping a
whole object (with subid == 0), you should drop entries relating
to its sub-objects too.  This is visible in the test_pg_dump test
case if one drops the extension at the end: the entry for
	GRANT SELECT(col1) ON regress_pg_dump_table TO public;
was still present in pg_init_privs afterwards, although it was
pointing to a dangling table OID.

Noted while fooling with a fix for REASSIGN OWNED for pg_init_privs
entries.  This bug is aboriginal in the pg_init_privs feature
though, and there seems no reason not to back-patch the fix.
2024-06-14 16:20:35 -04:00
Tom Lane
c7de5a6545 Fix parsing of ignored operators in websearch_to_tsquery().
The manual says clearly that punctuation in the input of
websearch_to_tsquery() is ignored, except for the special cases
of dashes and quotes.  However, this failed for cases like
"(foo bar) or something", or in general an ISOPERATOR character
in front of the "or".  We'd switch back to WAITOPERAND state,
then ignore the operator character while remaining in that state,
and then reach the "or" in WAITOPERAND state which (intentionally)
makes us treat it as data.

The fix is simple enough: if we see an ISOPERATOR character while in
WAITOPERATOR state, we have to skip it while staying in that state.
(We don't need to worry about other punctuation characters: those will
be consumed as though they were words, but then rejected by lexizing.)

In v14 and up (since commit eb086056f) we can simplify the code a bit
more too, because there is no longer a reason for the WAITOPERAND
state to distinguish between quoted and unquoted operands.

Per bug #18479 from Manos Emmanouilidis.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/18479-d9b46e2fc242c33e@postgresql.org
2024-06-13 20:34:43 -04:00
Tom Lane
5e8aa32a9c Fix infer_arbiter_indexes() to not assume resultRelation is 1.
infer_arbiter_indexes failed to renumber varnos in index expressions
or predicates that it got from the catalogs.  This escaped detection
up to now because the stored varnos in such trees will be 1, and an
INSERT's result relation is usually the first rangetable entry,
so that that was fine.  However, in cases such as inserting through
an updatable view, it's not fine, leading to failure to match the
expressions to the query with ensuing "there is no unique or exclusion
constraint matching the ON CONFLICT specification" errors.

Fix by copy-and-paste from get_relation_info().

Per bug #18502 from Michael Wang.  Back-patch to all supported
versions.

Discussion: https://postgr.es/m/18502-545b53f5b81e54e0@postgresql.org
2024-06-11 17:57:46 -04:00