Replace a static scratch buffer with a local variable, because a
static buffer makes the function not thread-safe. This function is
used in client-code in libpq, so it needs to be thread-safe. It was
until commit b67b57a966, which replaced the implementation with the
one from pgcrypto.
Backpatch to v14, where we switched to the new implementation.
Reviewed-by: Robert Haas, Michael Paquier
Discussion: https://www.postgresql.org/message-id/dfa2015d-ad21-4802-a4cc-3850fc5fff3f@iki.fi
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
This reverts commit 5887dd4894.
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.
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
This removes an inconsistency in the treatment of different datatypes by
the jsonpath timestamp_tz() function. Conversions from data types that
are not timestamp-aware, such as date and timestamp, are now treated
consistently with conversion from those that are such as timestamptz.
Author: David Wheeler
Reviewed-by: Junwang Zhao and Jeevan Chalke
Discussion: https://postgr.es/m/7DE080CE-6D8C-4794-9BD1-7D9699172FAB%40justatheory.com
Backpatch to release 17.
The tests had a race condition if autovacuum was set to off. Instead we
create all the tables we are interested in with autovacuum disabled, so
they are only ever touched when in danger of wraparound.
Discussion: https://postgr.es/m/3e2cbd24-f45e-4b2b-ba83-8149214f0a4d@dunslane.net
Masahiko Sawada (slightly tweaked by me)
Backpatch to release 17 where these tests were introduced.
The problem is that the tool is using the LSN returned by
pg_create_logical_replication_slot() as recovery_target_lsn. This LSN is
ahead of the current WAL position and the recovery waits until the
publisher writes a WAL record to reach the target and ends the recovery.
On idle systems, this wait time is unpredictable and could lead to failure
in promoting the subscriber. To avoid that, insert a harmless WAL record.
Reported-by: Alexander Lakhin and Tom Lane
Diagnosed-by: Hayato Kuroda
Author: Euler Taveira
Reviewed-by: Hayato Kuroda, Amit Kapila
Backpatch-through: 17
Discussion: https://postgr.es/m/2377319.1719766794%40sss.pgh.pa.us
Discussion: https://postgr.es/m/CA+TgmoYcY+Wb67NAwaHT7MvxCSeV86oSc+va9hHKaasE42ukyw@mail.gmail.com
The current method of coercing the boolean result value of
JsonPathExists() to the target type specified for an EXISTS column,
which is to call the type's input function via json_populate_type(),
leads to an error when the target type is integer, because the
integer input function doesn't recognize boolean literal values as
valid.
Instead use the boolean-to-integer cast function for coercion in that
case so that using integer or domains thereof as type for EXISTS
columns works. Note that coercion for ON ERROR values TRUE and FALSE
already works like that because the parser creates a cast expression
including the cast function, but the coercion of the actual result
value is not handled by the parser.
Tests by Jian He.
Reported-by: Jian He <jian.universality@gmail.com>
Author: Jian He <jian.universality@gmail.com>
Author: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Backpatch-through: 17
1. Remove the special case handling when casting the JsonBehavior
expressions to types with typmod, like 86d33987 did for the casting
of SQL/JSON constructor functions.
2. Fix casting for fixed-length character and bit string types by
using assignment-level casts. This is again similar to what
86d33987 did, but for ON ERROR / EMPTY expressions.
3. Use runtime coercion for the boolean ON ERROR constants so that
using fixed-length character string types, for example, for an
EXISTS column doesn't cause a "value too long for type
character(n)" when the parser tries to coerce the default ON ERROR
value "false" to that type, that is, even when clause is not
specified.
4. Simplify the conditions of when to use runtime coercion vs
creating the cast expression in the parser itself. jsonb-valued
expressions are now always coerced at runtime and boolean
expressions too if the target type is a string type for the
reasons mentioned above.
New tests are from a patch that Jian He posted. Outputs of some
existing tests change because the coercion now happens at runtime
instead of at parse time.
Reported-by: Jian He <jian.universality@gmail.com>
Author: Jian He <jian.universality@gmail.com>
Author: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Backpatch-through: 17
The initial implementation in commit 959b38d77 counted one action
per TOC entry (except for some special cases for multi-blob BLOBS
entries). This assumes that TOC entries are all about equally
complex, but it turns out that that assumption doesn't hold up very
well in binary-upgrade mode. For example, even after the previous
commit I was able to cause backend bloat with tables having many
inherited constraints. There may be other cases too. (Since no
serious problems have been reported with --single-transaction mode,
we can conclude that the backend copes well with psql's regular
restore scripts; but before 959b38d77 we never ran binary-upgrade
restores with multi-command transactions.)
To fix, count multi-command TOC entries as N actions, allowing the
transaction size to be scaled down when we hit a complex TOC entry.
Rather than add a SQL parser to pg_restore, approximate "multi
command" by counting semicolons in the TOC entry's defn string.
This will be fooled by semicolons appearing in string literals ---
but the error is in the conservative direction, so it doesn't seem
worth working harder. The biggest risk is with function/procedure
TOC entries, but we can just explicitly skip those.
(This is undoubtedly a hack, and maybe someday we'll be able to
revert it after fixing the backend's bloat issues or rethinking
what pg_dump emits in binary upgrade mode. But that surely isn't
a project for v17.)
Thanks to Alexander Korotkov for the let's-count-semicolons idea.
Per report from Justin Pryzby. Back-patch to v17 where txn_size mode
was introduced.
Discussion: https://postgr.es/m/ZqEND4ZcTDBmcv31@pryzbyj2023
Avoid issuing a separate SQL UPDATE command for each column when
directly manipulating pg_attribute contents in binary upgrade mode.
With the separate updates, we triggered a relcache invalidation with
each update. For a table with N columns, that causes O(N^2) relcache
bloat in txn_size mode because the table's newly-created relcache
entry can't be flushed till end of transaction. Reducing the number
of commands should make it marginally faster as well as avoiding that
problem.
While at it, likewise avoid issuing a separate UPDATE on pg_constraint
for each inherited constraint. This is less exciting, first because
inherited (non-partitioned) constraints are relatively rare, and
second because the backend has a good deal of trouble anyway with
restoring tables containing many such constraints, due to
MergeConstraintsIntoExisting being horribly inefficient. But it seems
more consistent to do it this way here too, and it surely can't hurt.
In passing, fix one place in dumpTableSchema that failed to use ONLY
in ALTER TABLE. That's not a live bug, but it's inconsistent.
Also avoid silently casting away const from string literals.
Per report from Justin Pryzby. Back-patch to v17 where txn_size mode
was introduced.
Discussion: https://postgr.es/m/ZqEND4ZcTDBmcv31@pryzbyj2023
pg_size_pretty(bigint) would return the value in bytes rather than PB
for the smallest-most bigint value. This happened due to an incorrect
assumption that the absolute value of -9223372036854775808 could be
stored inside a signed 64-bit type.
Here we fix that by instead storing that value in an unsigned 64-bit type.
This bug does exist in versions prior to 15 but the code there is
sufficiently different and the bug seems sufficiently non-critical that
it does not seem worth risking backpatching further.
Author: Joseph Koshakow <koshy44@gmail.com>
Discussion: https://postgr.es/m/CAAvxfHdTsMZPWEHUrZ=h3cky9Ccc3Mtx2whUHygY+ABP-mCmUw@mail.gmail.com
Backpatch-through: 15
Commit 453c468737 introduced a use of strerror() into libpq, but that
is not thread-safe. Fix by using strerror_r() instead.
In passing, update some of the code comments added by 453c468737, as
we have learned more about the reason for the change in OpenSSL that
started this.
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: Discussion: https://postgr.es/m/b6fb018b-f05c-4afd-abd3-318c649faf18@highgo.ca
When a standby is promoted, CleanupAfterArchiveRecovery() may decide
to rename the final WAL file from the old timeline by adding ".partial"
to the name. If WAL summarization is enabled and this file is renamed
before its partial contents are summarized, WAL summarization breaks:
the summarizer gets stuck at that point in the WAL stream and just
errors out.
To fix that, first make the startup process wait for WAL summarization
to catch up before renaming the file. Generally, this should be quick,
and if it's not, the user can shut off summarize_wal and try again.
To make this fix work, also teach the WAL summarizer that after a
promotion has occurred, no more WAL can appear on the previous
timeline: previously, the WAL summarizer wouldn't switch to the new
timeline until we actually started writing WAL there, but that meant
that when the startup process was waiting for the WAL summarizer, it
was waiting for an action that the summarizer wasn't yet prepared to
take.
In the process of fixing these bugs, I realized that the logic to wait
for WAL summarization to catch up was spread out in a way that made
it difficult to reuse properly, so this code refactors things to make
it easier.
Finally, add a test case that would have caught this bug and the
previously-fixed bug that WAL summarization sometimes needs to back up
when the timeline changes.
Discussion: https://postgr.es/m/CA+TgmoZGEsZodXC4f=XZNkAeyuDmWTSkpkjCEOcF19Am0mt_OA@mail.gmail.com
Commit 274bbced85 accidentally placed the pg_config.h.in
for SSL_CTX_set_num_tickets on the wrong line wrt where autoheader
places it. Fix by re-arranging and backpatch to the same level as
the original commit.
Reported-by: Marina Polyakova <m.polyakova@postgrespro.ru>
Discussion: https://postgr.es/m/48cebe8c3eaf308bae253b1dbf4e4a75@postgrespro.ru
Backpatch-through: v12
The old code believed that it was not possible to switch timelines
without first replaying all of the WAL from the old timeline, but
that turns out to be false, as demonstrated by an example from Fujii
Masao. As a result, it assumed that summarization would always
continue from the LSN where summarization previously ended. But in
fact, when a timeline switch occurs without replaying all the WAL
from the previous timeline, we can need to back up to an earlier
LSN. Adjust accordingly.
Discussion: https://postgr.es/m/CA+TgmoZGEsZodXC4f=XZNkAeyuDmWTSkpkjCEOcF19Am0mt_OA@mail.gmail.com
Commit 86db52a506 changed the locking of injection points to use only
atomic ops and spinlocks, to make it possible to define injection
points in processes that don't have a PGPROC entry (yet). However, it
didn't work in EXEC_BACKEND mode, because the pointer to shared memory
area was not initialized until the process "attaches" to all the
shared memory structs. To fix, pass the pointer to the child process
along with other global variables that need to be set up early.
Backpatch-through: 17
With sslmode=prefer, the desired behavior is to completely fail the
connection attempt, *not* fall back to a plaintext connection, if the
server responds to the SSLRequest with an error ('E') response instead
of rejecting SSL with an 'N' response. This was broken in commit
05fd30c0e7.
Reported-by: Jacob Champion
Reviewed-by: Michael Paquier
Discussion: https://www.postgresql.org/message-id/CAOYmi%2Bnwvu21mJ4DYKUa98HdfM_KZJi7B1MhyXtnsyOO-PB6Ww%40mail.gmail.com
Backpatch-through: 17
OpenSSL supports two types of session tickets for TLSv1.3, stateless
and stateful. The option we've used only turns off stateless tickets
leaving stateful tickets active. Use the new API introduced in 1.1.1
to disable all types of tickets.
Backpatch to all supported versions.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reported-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20240617173803.6alnafnxpiqvlh3g@awork3.anarazel.de
Backpatch-through: v12
populate_domain() didn't take into account the omit_quotes flag passed
down to json_populate_type() by ExecEvalJsonCoercion() and that led
to incorrect behavior when the RETURNING type is a domain over
jsonb. Fix that by passing the flag by adding a new function
parameter to populate_domain().
Reported-by: Jian He <jian.universality@gmail.com>
Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Backpatch-through: 17
Instead of returning a NULL when the JsonBehavior expression value
could not be coerced to the RETURNING type, throw the error message
informing the user that it is the JsonBehavior expression that caused
the error with the actual coercion error message shown in its DETAIL
line.
Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Backpatch-through: 17
To ensure that the errors of executing a JsonBehavior expression that
is coerced in the parser are caught instead of being thrown directly,
pass ErrorSaveContext to ExecInitExprRec() when initializing it.
Also, add a EEOP_JSONEXPR_COERCION_FINISH step to handle the errors
that are caught that way.
Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Backpatch-through: 17
If you try to take an incremental backup on a standby and there hasn't
been much system activity, it might fail. Document why this happens.
Also add a hint to the error message you get, to make it more likely
that users will understand what has gone wrong.
Laurenz Albe and Robert Haas
Discussion: https://postgr.es/m/5468641ad821dad7aa3b2d65bf843146443a1b68.camel@cybertec.at
Commit d01ce180 invented a new way to find the latest MacPorts version.
By bad luck, a new beta release has just been published, and it seems
to lack some packages we need. Go back to searching for this specific
version for now. We still search with a pattern so that we can find the
package for the running version of macOS, but for now we always look for
2.9.3. The code to do that had been anticipated already in a commented
out line, I just didn't expect to have to use it so soon...
Also include the whole MacPorts installation script in the cache key, so
that changes to the script cause a fresh installation. This should make
it a bit easier to reason about the effect of changes on cached state in
github accounts using CI, when we make adjustments.
Back-patch to 15, like d01ce180.
Discussion: https://postgr.es/m/CA%2BhUKGLqJdv6RcwyZ_0H7khxtLTNJyuK%2BvDFzv3uwYbn8hKH6A%40mail.gmail.com
1. Previously we were using ghcr.io/cirruslabs/macos-XXX-base:latest
images, but Cirrus has started ignoring that and using a particular
image, currently ghcr.io/cirruslabs/macos-runner:sonoma, for github
accounts using free CI resources (as opposed to dedicated runner
machines, as cfbot uses). Let's just ask for that image anyway, to stay
in sync.
2. Instead of hard-coding a MacPorts installation URL, deduce it from
the running macOS version and the available releases. This removes the
need to keep the ci_macports_packages.sh in sync with .cirrus.task.yml,
and to advance the MacPorts version from time to time.
3. Change the cache key we use to cache the whole macports installation
across builds to include the OS major version, to trigger a fresh
installation when appropriate.
Back-patch to 15 where CI began.
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA%2BhUKGLqJdv6RcwyZ_0H7khxtLTNJyuK%2BvDFzv3uwYbn8hKH6A%40mail.gmail.com
Presently, pg_upgrade obtains the number of subscriptions in the
to-be-upgraded cluster by first querying pg_subscription in every
database for the number of subscriptions in only that database.
Then, in count_old_cluster_subscriptions(), it adds all the values
collected in the first step. This is expensive, especially when
there are many databases.
Fortunately, there is a better way to retrieve the subscription
count. Since pg_subscription is a shared catalog, we only need to
connect to a single database and query it once. This commit
modifies pg_upgrade to use that approach, which also allows us to
trim several lines of code. In passing, move the call to
get_db_subscription_count(), which has been renamed to
get_subscription_count(), from get_db_rel_and_slot_infos() to the
dedicated >= v17 section in check_and_dump_old_cluster().
We may be able to make similar improvements to
get_old_cluster_logical_slot_infos(), but that is left as a future
exercise.
Reviewed-by: Michael Paquier, Amit Kapila
Discussion: https://postgr.es/m/ZprQJv_TxccN3tkr%40nathan
Backpatch-through: 17
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
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
clog.c, async.c and predicate.c included some SLRU page numbers still
handled as 4-byte integers, while int64 should be used for this purpose.
These holes have been introduced in 4ed8f0913b, that has introduced
the use of 8-byte integers for SLRU page numbers, still forgot about the
code paths updated by this commit.
Reported-by: Noah Misch
Author: Aleksander Alekseev, Michael Paquier
Discussion: https://postgr.es/m/20240626002747.dc.nmisch@google.com
Backpatch-through: 17
slru.h described incorrectly how SLRU segment names are formatted
depending on the segment number and if long or short segment names are
used. This commit closes the gap with a better description, fitting
with the reality.
Reported-by: Noah Misch
Author: Aleksander Alekseev
Discussion: https://postgr.es/m/20240626002747.dc.nmisch@google.com
Backpatch-through: 17
We were not being clear about which variants of the "direction"
clause are permitted in MOVE. Also, the text seemed to be
written with only the FETCH/MOVE NEXT case in mind, so it
didn't apply very well to other variants.
Also, document that "MOVE count IN cursor" only works if count
is a constant. This is not the whole truth, because some other
cases such as a parenthesized expression will also work, but
we want to push people to use "MOVE FORWARD count" instead.
The constant case is enough to cover what we allow in plain SQL,
and that seems sufficient to claim support for.
Update a comment in pl_gram.y claiming that we don't document
that point.
Per gripe from Philipp Salvisberg.
Discussion: https://postgr.es/m/172155553388.702.7932496598218792085@wrigleys.postgresql.org
This reverts commit 80c34692e8.
This test proved to be unstable on the buildfarm, timing out before the
standby could catch up on 32-bit machines where more rows were required
and failing to reliably trigger multiple index vacuum rounds on 64-bit
machines where fewer rows should be required.
Because the instability is only known to be present on versions of
Postgres with TIDStore used for dead TID storage by vacuum, this is only
being reverted on master and REL_17_STABLE.
As having this coverage may be valuable, there is a discussion on the
thread of possible ways to stabilize the test. If that happens, a fixed
test can be committed again.
Backpatch-through: 17
Reported-by: Tom Lane
Discussion: https://postgr.es/m/614152.1721580711%40sss.pgh.pa.us
After calling ConditionVariableSleep() or ConditionVariableTimedSleep()
one or more times, code is supposed to call ConditionVariableCancelSleep()
to remove itself from the waitlist. This code neglected to do so.
As far as I know, that had no observable consequences, but let's make
the code correct.
Discussion: http://postgr.es/m/CA+TgmoYW8eR+KN6zhVH0sin7QH6AvENqw_bkN-bB4yLYKAnsew@mail.gmail.com
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
Alvaro Herrera