David Rowley [Mon, 11 Mar 2024 05:20:39 +0000 (18:20 +1300)]
Fix incorrect accessing of pfree'd memory in Memoize
For pass-by-reference types, the code added in
0b053e78b, which aimed to
resolve a memory leak, was overly aggressive in resetting the per-tuple
memory context which could result in pfree'd memory being accessed
resulting in failing to find previously cached results in the hash
table.
What was happening was prepare_probe_slot() was switching to the
per-tuple memory context and calling ExecEvalExpr(). ExecEvalExpr() may
have required a memory allocation. Both MemoizeHash_hash() and
MemoizeHash_equal() were aggressively resetting the per-tuple context
and after determining the hash value, the context would have gotten reset
before MemoizeHash_equal() was called. This could have resulted in
MemoizeHash_equal() looking at pfree'd memory.
This is less likely to have caused issues on a production build as some
other allocation would have had to have reused the pfree'd memory to
overwrite it. Otherwise, the original contents would have been intact.
However, this clearly caused issues on MEMORY_CONTEXT_CHECKING builds.
Author: Tender Wang, Andrei Lepikhov
Reported-by: Tender Wang (using SQLancer)
Reviewed-by: Andrei Lepikhov, Richard Guo, David Rowley
Discussion: https://postgr.es/m/CAHewXNnT6N6UJkya0z-jLFzVxcwGfeRQSfhiwA+NyLg-x8iGew@mail.gmail.com
Backpatch-through: 14, where Memoize was added
Alexander Korotkov [Fri, 16 Feb 2024 14:02:00 +0000 (16:02 +0200)]
Backpatch missing check_stack_depth() to some recursive functions
Backpatch changes from
d57b7cc333,
75bcba6cbd to all supported branches per
proposal of Egor Chindyaskin.
Discussion: https://postgr.es/m/
DE5FD776-A8CD-4378-BCFA-
3BF30F1F6D60%40mail.ru
David Rowley [Sun, 10 Mar 2024 23:27:46 +0000 (12:27 +1300)]
Fix deparsing of Consts in postgres_fdw ORDER BY
For UNION ALL queries where a union child query contained a foreign
table, if the targetlist of that query contained a constant, and the
top-level query performed an ORDER BY which contained the column for the
constant value, then postgres_fdw would find the EquivalenceMember with
the Const and then try to produce an ORDER BY containing that Const.
This caused problems with INT typed Consts as these could appear to be
requests to order by an ordinal column position rather than the constant
value. This could lead to either an error such as:
ERROR: ORDER BY position
is not in select list
or worse, if the constant value is a valid column, then we could just
sort by the wrong column altogether.
Here we fix this issue by just not including these Consts in the ORDER
BY clause.
In passing, add a new section for testing ORDER BY in the postgres_fdw
tests and move two existing tests which were misplaced in the WHERE
clause testing section into it.
Reported-by: Michał Kłeczek
Reviewed-by: Ashutosh Bapat, Richard Guo
Bug: #18381
Discussion: https://postgr.es/m/0714C8B8-8D82-4ABB-9F8D-A0C3657E7B6E%40kleczek.org
Discussion: https://postgr.es/m/18381-137456acd168bf93%40postgresql.org
Backpatch-through: 12, oldest supported version
Tom Lane [Fri, 8 Mar 2024 00:37:51 +0000 (19:37 -0500)]
Cope with a deficiency in OpenSSL 3.x's error reporting.
In OpenSSL 3.0.0 and later, ERR_reason_error_string randomly refuses
to provide a string for error codes representing system errno values
(e.g., "No such file or directory"). There is a poorly-documented way
to extract the errno from the SSL error code in this case, so do that
and apply strerror, rather than falling back to reporting the error
code's numeric value as we were previously doing.
Problem reported by David Zhang, although this is not his proposed
patch; it's instead based on a suggestion from Heikki Linnakangas.
Back-patch to all supported branches, since any of them are likely
to be used with recent OpenSSL.
Discussion: https://postgr.es/m/
b6fb018b-f05c-4afd-abd3-
318c649faf18@highgo.ca
Dean Rasheed [Thu, 7 Mar 2024 09:55:39 +0000 (09:55 +0000)]
Fix handling of self-modified tuples in MERGE.
When an UPDATE or DELETE action in MERGE returns TM_SelfModified,
there are 2 possible causes:
1). The target tuple was already updated or deleted by the current
command. This can happen if the target row joins to more than one
source row, and the SQL standard explicitly says that this must be
an error.
2). The target tuple was already updated or deleted by a later command
in the current transaction. This can happen if the tuple is
modified by a BEFORE trigger or a volatile function used in the
query, and should be an error for the same reason that it is in a
plain UPDATE or DELETE command.
In MERGE's primary error handling block, it failed to check for (2),
causing it to return a misleading error message in such cases.
In the secondary error handling block, following a concurrent update
from another session, it failed to check for (1), causing it to
silently ignore target rows joined to more than one source row,
instead of reporting an error.
Fix this, and add tests for both of these cases.
Per report from Wenjiang Zhang. Back-patch to v15, where MERGE was
introduced.
Discussion: https://postgr.es/m/tencent_41DE0FF443FE14B94A5898D373792109E408%40qq.com
Michael Paquier [Wed, 6 Mar 2024 23:31:00 +0000 (08:31 +0900)]
Revert "Fix parallel-safety check of expressions and predicate for index builds"
This reverts commit
eae7be600be7, following a discussion with Tom Lane,
due to concerns that this impacts the decisions made by the planner for
the number of workers spawned based on the inlining and const-folding of
index expressions and predicate for cases that would have worked until
this commit.
Discussion: https://postgr.es/m/162802.
1709746091@sss.pgh.pa.us
Backpatch-through: 12
Tom Lane [Wed, 6 Mar 2024 19:41:13 +0000 (14:41 -0500)]
Fix type-checking of RECORD-returning functions in FROM.
In the corner case where a function returning RECORD has been
simplified to a RECORD constant or an inlined ROW() expression,
ExecInitFunctionScan failed to cross-check the function's result
rowtype against the coldeflist provided by the calling query.
That happened because get_expr_result_type is able to extract a
tupdesc from such expressions, which led ExecInitFunctionScan to
ignore the coldeflist. (Instead, it used the extracted tupdesc
to check the function's output, which of course always succeeds.)
I have not been able to demonstrate any really serious consequences
from this, because if some column of the result is of the wrong
type and is directly referenced by a Var of the calling query,
CheckVarSlotCompatibility will catch it. However, we definitely do
fail to report the case where the function returns more columns than
the coldeflist expects, and in the converse case where it returns
fewer columns, we get an assert failure (but, seemingly, no worse
results in non-assert builds).
To fix, always build the expected tupdesc from the coldeflist if there
is one, and consult get_expr_result_type only when there isn't one.
Also remove the failing Assert, even though it is no longer reached
after this fix. It doesn't seem to be adding anything useful, since
later checking will deal with cases with the wrong number of columns.
The only other place I could find that is doing something similar
is inline_set_returning_function. There's no live bug there because
we cannot be looking at a Const or RowExpr, but for consistency
change that code to agree with ExecInitFunctionScan.
Per report from PetSerAl. After some debate I've concluded that
this should be back-patched. There is a small risk that somebody
has been relying on such a case not throwing an error, but I judge
this outweighed by the risk that I've missed some way in which the
failure to cross-check has worse consequences than sketched above.
Discussion: https://postgr.es/m/CAKygsHSerA1eXsJHR9wft3Gn3wfHQ5RfP8XHBzF70_qcrrRvEg@mail.gmail.com
Michael Paquier [Wed, 6 Mar 2024 08:24:05 +0000 (17:24 +0900)]
Fix parallel-safety check of expressions and predicate for index builds
As coded, the planner logic that calculates the number of parallel
workers to use for a parallel index build uses expressions and
predicates from the relcache, which are flattened for the planner by
eval_const_expressions().
As reported in the bug, an immutable parallel-unsafe function flattened
in the relcache would become a Const, which would be considered as
parallel-safe, even if the predicate or the expressions including the
function are not safe in parallel workers. Depending on the expressions
or predicate used, this could cause the parallel build to fail.
Tests are included that check parallel index builds with parallel-unsafe
predicate and expressions. Two routines are added to lsyscache.h to be
able to retrieve expressions and predicate of an index from its pg_index
data.
Reported-by: Alexander Lakhin
Author: Tender Wang
Reviewed-by: Jian He, Michael Paquier
Discussion: https://postgr.es/m/CAHewXN=UaAaNn9ruHDH3Os8kxLVmtWqbssnf=dZN_s9=evHUFA@mail.gmail.com
Backpatch-through: 12
Jeff Davis [Tue, 5 Mar 2024 17:25:04 +0000 (09:25 -0800)]
docs: Update HOT update docs for
19d8e2308b
Commit
19d8e2308b changed when the HOT update optimization is possible
but neglected to update the Heap-Only Tuples (HOT) documentation. This
patch updates that documentation accordingly.
Author: Elizabeth Christensen
Backpatch-through: 16
Reviewed-By: Stephen Frost, Alvaro Herrera
Discussion: https://postgr.es/m/CABoUFXRjisr58Ct_3VsFEdQx+fJeQTWTdJnM7XAp=8MUbtoa9A@mail.gmail.com
David Rowley [Tue, 5 Mar 2024 03:17:53 +0000 (16:17 +1300)]
Fix incorrectly reported stats kind in "can't happen" ERROR
The error message(s) were reporting the stats kind of 'f', which is not
correct as that's for the "dependencies" statistics kind.
Reported-by: Horst Reiterer
Reviewed-by: Richard Guo
Discussion: https://postgr.es/m/18375-
ba99383eb9062d6a@postgresql.org
Backpatch-through: 12, where MCV extended stats were added.
Tom Lane [Mon, 4 Mar 2024 17:00:39 +0000 (12:00 -0500)]
Fix initdb's -c option to treat the GUC name case-insensitively.
The backend treats GUC names case-insensitively, so this code should
too. This avoids ending up with a confusing set of redundant entries
in the generated postgresql.conf file.
Per report from Kyotaro Horiguchi. Back-patch to v16 where this
feature was added (in commit
3e51b278d).
Discussion: https://postgr.es/m/
20230928.164904.
2153358973162534034[email protected]
Daniel Gustafsson [Mon, 4 Mar 2024 09:52:19 +0000 (10:52 +0100)]
doc: Fix datatype for postgres_fdw option
The datatype for analyze_sampling had accidentally been set to text
and not string. Backpatch to v16 where analyze_sampling first was
introduced.
Author: Shinya Kato
Reviewed-by: Laurenz Albe
Discussion: https://postgr.es/m/7fd9166b9fda267411793f39986d7f24@oss.nttdata.com
Backpatch-through: v16
Daniel Gustafsson [Thu, 29 Feb 2024 11:19:52 +0000 (12:19 +0100)]
Fix integer underflow in shared memory debugging
dsa_dump would print a large negative number instead of zero for
segment bin 0. Fix by explicitly checking for underflow and add
special case for bin 0. Backpatch to all supported versions.
Author: Ian Ilyasov
Reviewed-by: Robert Haas
Discussion: https://postgr.es/m/GV1P251MB1004E0D09D117D3CECF9256ECD502@GV1P251MB1004.EURP251.PROD.OUTLOOK.COM
Backpatch-through: v12
Tom Lane [Wed, 28 Feb 2024 19:00:30 +0000 (14:00 -0500)]
Fix mis-rounding and overflow hazards in date_bin().
In the case where the target timestamp is before the origin timestamp
and their difference is already an exact multiple of the stride, the
code incorrectly subtracted the stride anyway.
Also detect several integer-overflow cases that previously produced
bogus results. (The submitted patch tried to avoid overflow, but
I'm not convinced it's right, and problematic cases are so far out of
the plausibly-useful range that they don't seem worth sweating over.
Let's just use overflow-detecting arithmetic and throw errors.)
timestamp_bin() and timestamptz_bin() are basically identical and
so had identical bugs. Fix both.
Report and patch by Moaaz Assali, adjusted some by me. Back-patch
to v14 where date_bin() was introduced.
Discussion: https://postgr.es/m/CALkF+nvtuas-2kydG-WfofbRSJpyODAJWun==W-yO5j2R4meqA@mail.gmail.com
Alvaro Herrera [Mon, 26 Feb 2024 17:19:03 +0000 (18:19 +0100)]
Revise MERGE documentation
Add a note about the additional privileges required after the fix in
4989ce72644b (wording per Tom Lane); also change marked-up mentions of
"target_table_name" to be simply "the target table" or the like. Also,
note that "join_condition" is scouted for requisite privileges.
Backpatch to 15.
Discussion: https://postgr.es/m/
202402211653[email protected]
Amit Kapila [Mon, 26 Feb 2024 04:03:57 +0000 (09:33 +0530)]
Back-patch test modifications that were done as part of
b6df0798a5.
This commit fixes the intermittent buildfarm failures in 031_column_list.
I missed to back-patch while committing
b6df0798a5 in the HEAD.
Diagnosed-by: Amit Kapila
Author: Vignesh C
Backpatch-through: 15
Discussion: https://postgr.es/m/
3307255.
1706911634@sss.pgh.pa.us
Tom Lane [Sun, 25 Feb 2024 21:15:07 +0000 (16:15 -0500)]
Promote assertion about !ReindexIsProcessingIndex to runtime error.
When this assertion was installed (in commit
d2f60a3ab), I thought
it was only for catching server logic errors that caused accesses to
catalogs that were undergoing index rebuilds. However, it will also
fire in case of a user-defined index expression that attempts to
access its own table. We occasionally see reports of people trying
to do that, and typically getting unintelligible low-level errors
as a result. We can provide a more on-point message by making this
a regular runtime check.
While at it, adjust the similar error check in
systable_beginscan_ordered to use the same message text. That one
is (probably) not reachable without a coding bug, but we might as
well use a translatable message if we have one.
Per bug #18363 from Alexander Lakhin. Back-patch to all supported
branches.
Discussion: https://postgr.es/m/18363-
e3598a5a572d0699@postgresql.org
Tom Lane [Sun, 25 Feb 2024 20:29:09 +0000 (15:29 -0500)]
Doc: fix minor typos in two ECPG function descriptions.
Noted by Aidar Imamov.
Discussion: https://postgr.es/m/
170869935022.643.
3709087848818148291@wrigleys.postgresql.org
Tom Lane [Fri, 23 Feb 2024 20:21:53 +0000 (15:21 -0500)]
Avoid dangling-pointer problem with partitionwise joins under GEQO.
build_child_join_sjinfo creates a derived SpecialJoinInfo in
the short-lived GEQO context, but afterwards the semi_rhs_exprs
from that may be used in a UniquePath for a child base relation.
This breaks the expectation that all base-relation-level structures
are in the planning-lifespan context, leading to use of a dangling
pointer with probable ensuing crash later on in create_unique_plan.
To fix, copy the expression trees when making a UniquePath.
Per bug #18360 from Alexander Lakhin. This has been broken since
partitionwise joins were added, so back-patch to all supported
branches.
Discussion: https://postgr.es/m/18360-
a23caf3157f34e62@postgresql.org
Peter Eisentraut [Fri, 23 Feb 2024 13:40:25 +0000 (14:40 +0100)]
Fix mistake in SQL features list
Fix for
c9f57541d97: Feature F302-02 was renamed to F305, but that
commit failed to delete the old line.
Reported-by: Satoru Koizumi (小泉 悟)
Discussion: https://www.postgresql.org/message-id/flat/
170866661469.645.
14101429540172934386%40wrigleys.postgresql.org
Alvaro Herrera [Wed, 21 Feb 2024 16:18:52 +0000 (17:18 +0100)]
MERGE ... DO NOTHING: require SELECT privileges
Verify that a user running MERGE with a DO NOTHING clause has
privileges to read the table, even if no columns are referenced. Such
privileges were already required if the ON clause or any of the WHEN
conditions referenced any column at all, so there's no functional change
in practice.
This change fixes an assertion failure in the case where no column is
referenced by the command and the WHEN clauses are all DO NOTHING.
Backpatch to 15, where MERGE was introduced.
Reported-by: Alena Rybakina
Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/
4d65a385-7efa-4436-a825-
0869f89d9d92@postgrespro.ru
Daniel Gustafsson [Wed, 21 Feb 2024 08:22:18 +0000 (09:22 +0100)]
doc: Fix link to pg_ident_file_mappings view
Commit
a2c84990bea7 accidentally used the link for pg_hba_file_rules
when linking to pg_ident_file_mappings. Backpatch to v16 where this
was introduced.
Author: Erik Wienhold
Discussion: https://postgr.es/m/qt5hvgvfi4qzlgml2dfssaut2t2x5nwf7b5l63fklr7fpxwm6g@hle3mtglpm4y
Backpatch-through: v16
Tom Lane [Tue, 20 Feb 2024 19:35:12 +0000 (14:35 -0500)]
Doc: improve explanation of type interval, especially extract().
The explanation of interval's behavior in datatype.sgml wasn't wrong
exactly, but it was unclear, partly because it buried the lede about
there being three internal fields. Rearrange and wordsmith for more
clarity.
The discussion of extract() claimed that input of type date was
handled by casting, but actually there's been a separate SQL function
taking date for a very long time. Also, it was mostly silent about
how interval inputs are handled, but there are several field types
for which it seems useful to be specific.
Improve discussion of justify_days()/justify_hours() too.
In passing, remove vertical space in some groups of examples,
as there was little consistency about whether to have such space
or not. (I only did this within the datetime functions section;
there are some related inconsistencies elsewhere.)
Per discussion of bug #18348 from Michael Bondarenko. There
may be some code changes coming out of that discussion too,
but we likely won't back-patch them. This docs-only patch
seems useful to back-patch, though I only carried it back to
v13 because it didn't apply easily in v12.
Discussion: https://postgr.es/m/18348-
b097a3587dfde8a4@postgresql.org
Tom Lane [Tue, 20 Feb 2024 16:58:28 +0000 (11:58 -0500)]
Doc: correct minor error in back-branch release notes.
Commits
1b2c6b756 et al affected the core BRIN "bloom" opclasses,
not contrib/bloom. This only corrected a bad assertion so it's not
too significant to end users, but since we documented it we should
do so accurately.
Spotted by Takatsuka Haruka.
Discussion: https://postgr.es/m/18353-
926aa99cfe58aa78@postgresql.org
Michael Paquier [Tue, 20 Feb 2024 04:43:56 +0000 (13:43 +0900)]
Fix race leading to incorrect conflict cause in InvalidatePossiblyObsoleteSlot()
The invalidation of an active slot is done in two steps:
- Termination of the backend holding it, if any.
- Report that the slot is obsolete, with a conflict cause depending on
the slot's data.
This can be racy because between these two steps the slot mutex would be
released while doing system calls, which means that the effective_xmin
and effective_catalog_xmin could advance during that time, detecting a
conflict cause different than the one originally wanted before the
process owning a slot is terminated.
Holding the mutex longer is not an option, so this commit changes the
code to record the LSNs stored in the slot during the termination of the
process owning the slot.
Bonus thanks to Alexander Lakhin for the various tests and the analysis.
Author: Bertrand Drouvot
Reviewed-by: Michael Paquier, Bharath Rupireddy
Discussion: https://postgr.es/m/
[email protected]
Backpatch-through: 16
Michael Paquier [Tue, 20 Feb 2024 02:59:10 +0000 (11:59 +0900)]
doc: Use system-username instead of system-user
This inconsistency has been introduced in
efb6f4a4f9b6.
Reported-by: Julien Rouhaud
Author: Bertrand Drouvot
Discussion: https://postgr.es/m/
[email protected]
Backpatch-through: 16
David Rowley [Mon, 19 Feb 2024 23:50:09 +0000 (12:50 +1300)]
Fix incorrect pruning of NULL partition for boolean IS NOT clauses
Partition pruning wrongly assumed that, for a table partitioned on a
boolean column, a clause in the form "boolcol IS NOT false" and "boolcol
IS NOT true" could be inverted to correspondingly become "boolcol IS true"
and "boolcol IS false". These are not equivalent as the NOT version
matches the opposite boolean value *and* NULLs. This incorrect assumption
meant that partition pruning pruned away partitions that could contain
NULL values.
Here we fix this by correctly not pruning partitions which could store
NULLs.
To be affected by this, the table must be partitioned by a NULLable boolean
column and queries would have to contain "boolcol IS NOT false" or "boolcol
IS NOT true". This could result in queries filtering out NULL values
with a LIST partitioned table and "ERROR: invalid strategy number 0"
for RANGE and HASH partitioned tables.
Reported-by: Alexander Lakhin
Bug: #18344
Discussion: https://postgr.es/m/18344-
8d3f00bada6d09c6@postgresql.org
Backpatch-through: 12
Noah Misch [Mon, 19 Feb 2024 20:52:28 +0000 (12:52 -0800)]
Fix test race between primary XLOG_RUNNING_XACTS and standby logical slot.
Before the previous commit, the test could hang until
LOG_SNAPSHOT_INTERVAL_MS (15s), until checkpoint_timeout (300s), or
indefinitely. An indefinite hang was awfully improbable. It entailed
the test reaching checkpoint_timeout before the
DecodingContextFindStartpoint() of a CREATE SUBSCRIPTION, yet after the
preceding WAL record. Back-patch to v16, which introduced the test.
Bertrand Drouvot, reported by Noah Misch.
Discussion: https://postgr.es/m/
20240211010227[email protected]
Noah Misch [Mon, 19 Feb 2024 20:52:07 +0000 (12:52 -0800)]
Bound waits in 035_standby_logical_decoding.pl.
One IPC::Run::start() used an IPC::Run::timer() without checking for
expiration. The other used no timeout or timer. Back-patch to v16,
which introduced the test.
Reviewed by Bertrand Drouvot.
Discussion: https://postgr.es/m/
20240211010227[email protected]
Tom Lane [Mon, 19 Feb 2024 19:17:11 +0000 (14:17 -0500)]
Doc: fix typo in SECURITY LABEL synopsis.
One case missed its trailing "|".
Reported by Tim Needham.
Discussion: https://postgr.es/m/
170833547220.
3279712.
700702770281879175@wrigleys.postgresql.org
Michael Paquier [Mon, 19 Feb 2024 02:38:44 +0000 (11:38 +0900)]
ecpg: Fix zero-termination of string generated by intoasc()
intoasc(), a wrapper for PGTYPESinterval_to_asc that converts an
interval to its textual representation, used a plain memcpy() when
copying its result. This could miss a zero-termination in the result
string, leading to an incorrect result.
The routines in informix.c do not provide the length of their result
buffer, which would allow a replacement of strcpy() to safer strlcpy()
calls, but this requires an ABI breakage and that cannot happen in
back-branches.
Author: Oleg Tselebrovskiy
Reviewed-by: Ashutosh Bapat
Discussion: https://postgr.es/m/
bf47888585149f83b276861a1662f7e4@postgrespro.ru
Backpatch-through: 12
Peter Eisentraut [Fri, 16 Feb 2024 10:39:09 +0000 (11:39 +0100)]
Remove non-existing file from .gitattributes
The file was removed by
ac25173cdbc.
Author: Jelte Fennema-Nio
Discussion: https://www.postgresql.org/message-id/flat/CAGECzQQGzbroAXi%2BYicp3HvcCo4%3Dg84kaOgjuvQ5MW9F0ubOGg%40mail.gmail.com
Tom Lane [Thu, 15 Feb 2024 21:45:03 +0000 (16:45 -0500)]
Doc: improve a couple of comments in postgresql.conf.sample.
Clarify comments associated with max_parallel_workers and
related settings.
Per bug #18343 from Christopher Kline.
Discussion: https://postgr.es/m/18343-
3a5e903d1d3692ab@postgresql.org
Daniel Gustafsson [Wed, 14 Feb 2024 10:05:10 +0000 (11:05 +0100)]
doc: Remove links to further reading from pgcrypto docs
The pgcrypto docs contained a set of links for useful reading and
technical references. These sets of links were however not actively
curated and had stale content and dead links. Rather than investing
time into maintining these, this removes them altogether since there
are lots of resources online which are actively maintained.
Backpatch to all supported versions since these links have been in
the docs for a long time.
Reported-by: Hanefi Onaldi
Reviewed-by: Magnus Hagander
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/
170774255387.
3279713.
2822272755998870925@wrigleys.postgresql.org
Backpatch-through: v12
Heikki Linnakangas [Tue, 13 Feb 2024 19:23:41 +0000 (21:23 +0200)]
Fix 'mmap' DSM implementation with allocations larger than 4 GB
Fixes bug #18341. Backpatch to all supported versions.
Discussion: https://www.postgresql.org/message-id/18341-
ce16599e7fd6228c@postgresql.org
Tom Lane [Tue, 13 Feb 2024 17:18:25 +0000 (12:18 -0500)]
Use a safer outfuncs/readfuncs representation for BitStrings.
For a long time, our outfuncs.c code has supposed that the string
contents of a BitString node could just be printed literally with
no concern for quoting/escaping. Now, that's okay if the string
literal contains only valid binary or hex digits ... but our lexer
doesn't check that, preferring to let bitin() be the sole authority
on what's valid. So we could have raw parse trees that contain
incorrect BitString literals, and that can result in failures when
WRITE_READ_PARSE_PLAN_TREES debugging is enabled.
Fix by using outToken() to print the string field, and debackslash()
to read it. This results in a change in the emitted representation
only in cases that would have failed before, and don't represent valid
SQL in the first place. Between that and the fact that we don't store
raw parse trees in the catalogs, I judge this safe to apply without a
catversion bump.
Per bug #18340 from Alexander Lakhin. Back-patch to v16; before that,
we lacked readfuncs support for BitString nodes, so that the problem
was only cosmetic.
Discussion: https://postgr.es/m/18340-
4aa1ae6ed4121912@postgresql.org
Daniel Gustafsson [Tue, 13 Feb 2024 12:47:12 +0000 (13:47 +0100)]
Skip .DS_Store files in server side utils
The macOS Finder application creates .DS_Store files in directories
when opened, which creates problems for serverside utilities which
expect all files to be PostgreSQL specific files. Skip these files
when encountered in pg_checksums, pg_rewind and pg_basebackup.
This was extracted from a larger patchset for skipping hidden files
and system files, where the concencus was to just skip these. Since
this is equally likely to happen in every version, backpatch to all
supported versions.
Reported-by: Mark Guertin
Reviewed-by: Michael Paquier
Reviewed-by: Tobias Bussmann
Discussion: https://postgr.es/m/
E258CE50-AB0E-455D-8AAD-
BB4FE8F882FB@gmail.com
Backpatch-through: v12
Thomas Munro [Sun, 11 Feb 2024 21:47:57 +0000 (10:47 +1300)]
Fix gai_strerror() thread-safety on Windows.
Commit
5579388d removed code that supplied a fallback implementation of
getaddrinfo(), which was dead code on modern systems. One tiny piece of
the removed code was still doing something useful on Windows, though:
that OS's own gai_strerror()/gai_strerrorA() function returns a pointer
to a static buffer that it overwrites each time, so it's not
thread-safe. In rare circumstances, a multi-threaded client program
could get an incorrect or corrupted error message.
Restore the replacement gai_strerror() function, though now that it's
only for Windows we can put it into a win32-specific file and cut it
down to the errors that Windows documents. The error messages here are
taken from FreeBSD, because Windows' own messages seemed too verbose.
Back-patch to 16.
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/CA%2BhUKGKz%2BF9d2PTiXwfYV7qJw%2BWg2jzACgSDgPizUw7UG%3Di58A%40mail.gmail.com
Tom Lane [Fri, 9 Feb 2024 17:29:41 +0000 (12:29 -0500)]
Remove race condition in pg_get_expr().
Since its introduction, pg_get_expr() has intended to silently
return NULL if called with an invalid relation OID, as can happen
when scanning the catalogs concurrently with relation drops.
However, there is a race condition: we check validity of the OID
at the start, but it could get dropped just afterward, leading to
failures. This is the cause of some intermittent instability we're
seeing in a proposed new test case, and presumably it's a hazard in
the field as well.
We can fix this by AccessShareLock-ing the target relation for the
duration of pg_get_expr(). Since we don't require any permissions
on the target relation, this is semantically a bit undesirable. But
it turns out that the set_relation_column_names() subroutine already
takes a transient AccessShareLock on that relation, and has done since
commit
2ffa740be in 2012. Given the lack of complaints about that, it
seems like there should be no harm in holding the lock a bit longer.
Back-patch to all supported branches.
Discussion: https://postgr.es/m/
31ddcc01-a71b-4e8c-9948-
01d1c47293ca@eisentraut.org
Tom Lane [Fri, 9 Feb 2024 16:21:08 +0000 (11:21 -0500)]
Avoid concurrent calls to bindtextdomain().
We previously supposed that it was okay for different threads to
call bindtextdomain() concurrently (cf. commit
1f655fdc3).
It now emerges that there's at least one gettext implementation
in which that triggers an abort() crash, so let's stop doing that.
Add mutexes guarding libpq's and ecpglib's calls, which are the
only ones that need worry about multithreaded callers.
Note: in libpq, we could perhaps have piggybacked on
default_threadlock() to avoid defining a new mutex variable.
I judge that not terribly safe though, since libpq_gettext could
be called from code that is holding the default mutex. If that
were the first such call in the process, it'd fail. An extra
mutex is cheap insurance against unforeseen interactions.
Per bug #18312 from Christian Maurer. Back-patch to all
supported versions.
Discussion: https://postgr.es/m/18312-
bbbabc8113592b78@postgresql.org
Discussion: https://postgr.es/m/264860.
1707163416@sss.pgh.pa.us
Tom Lane [Fri, 9 Feb 2024 16:11:39 +0000 (11:11 -0500)]
Clean up Windows-specific mutex code in libpq and ecpglib.
Fix pthread-win32.h and pthread-win32.c to provide a more complete
emulation of POSIX pthread mutexes: define PTHREAD_MUTEX_INITIALIZER
and make sure that pthread_mutex_lock() can operate on a mutex
object that's been initialized that way. Then we don't need the
duplicative platform-specific logic in default_threadlock() and
pgtls_init(), which we'd otherwise need yet a third copy of for
an upcoming bug fix.
Also, since default_threadlock() supposes that pthread_mutex_lock()
cannot fail, try to ensure that that's actually true, by getting
rid of the malloc call that was formerly involved in initializing
an emulated mutex. We can define an extra state for the spinlock
field instead.
Also, replace the similar code in ecpglib/misc.c with this version.
While ecpglib's version at least had a POSIX-compliant API, it
also had the potential of failing during mutex init (but here,
because of CreateMutex failure rather than malloc failure). Since
all of misc.c's callers ignore failures, it seems like a wise idea
to avoid failures here too.
A further improvement in this area could be to unify libpq's and
ecpglib's implementations into a src/port/pthread-win32.c file.
But that doesn't seem like a bug fix, so I'll desist for now.
In preparation for the aforementioned bug fix, back-patch to all
supported branches.
Discussion: https://postgr.es/m/264860.
1707163416@sss.pgh.pa.us
Alexander Korotkov [Thu, 8 Feb 2024 10:45:26 +0000 (12:45 +0200)]
Fix wrong logic in TransactionIdInRecentPast()
The TransactionIdInRecentPast() should return false for all the transactions
older than TransamVariables->oldestClogXid. However, the function contains
a bug in comparison FullTransactionId to TransactionID allowing full
transactions between nextXid - 2^32 and oldestClogXid - 2^31.
This commit fixes TransactionIdInRecentPast() by turning the oldestClogXid into
FullTransactionId first, then performing the comparison.
Backpatch to all supported versions.
Reported-by: Egor Chindyaskin
Bug: 18212
Discussion: https://postgr.es/m/18212-
547307f8adf57262%40postgresql.org
Author: Karina Litskevich
Reviewed-by: Kyotaro Horiguchi
Backpatch-through: 12
Peter Eisentraut [Fri, 9 Feb 2024 06:57:31 +0000 (07:57 +0100)]
Fix propagation of persistence to sequences in ALTER TABLE / ADD COLUMN
Fix for
344d62fb9a9: That commit introduced unlogged sequences and
made it so that identity/serial sequences automatically get the
persistence level of their owning table. But this works only for
CREATE TABLE and not for ALTER TABLE / ADD COLUMN. The latter would
always create the sequence as logged (default), independent of the
persistence setting of the table. This is fixed here.
Note: It is allowed to change the persistence of identity sequences
directly using ALTER SEQUENCE. So mistakes in existing databases can
be fixed manually.
Reviewed-by: Ashutosh Bapat
Discussion: https://www.postgresql.org/message-id/flat/
c4b6e2ed-bcdf-4ea7-965f-
e49761094827%40eisentraut.org
Daniel Gustafsson [Thu, 8 Feb 2024 11:19:34 +0000 (12:19 +0100)]
doc: Remove superfluous bracket in synopsis
Commit
9c08aea6a30 accidentally added one too many end brackets
in the synopsis for CREATE DATABASE .. strategy = strat. Fix by
removing. Backpatch to v15 where it was introduced.
Reported-by: [email protected]
Discussion: https://postgr.es/m/
170734160862.
3279712.
810853722572951776@wrigleys.postgresql.org
Backpatch-through: v15
Peter Eisentraut [Thu, 8 Feb 2024 09:18:50 +0000 (10:18 +0100)]
Update comment
The documentation output format htmlhelp is no longer supported, but a
comment still mentioned it.
Alvaro Herrera [Wed, 7 Feb 2024 18:25:07 +0000 (19:25 +0100)]
Update PQparameterStatus and ParameterStatus docs
Cover scram_iterations, which was added in commit
b577743000cd. While
at it, turn the list into a
with 2 columns, which is much
nicer to read.
In master, remove mentions of antediluvian versions before which some
parameters were not reported.
Noticed while investigating a question by Maiquel Grassi.
Backpatch to 16.
Reviewed-by: Daniel Gustafsson
Reviewed-by: Jelte Fennema-Nio
Discussion: https://postgr.es/m/202401301236[email protected]
Tom Lane [Mon, 5 Feb 2024 21:41:37 +0000 (16:41 -0500)]
Stamp 16.2.
Tom Lane [Mon, 5 Feb 2024 16:51:11 +0000 (11:51 -0500)]
Last-minute updates for release notes.
Security: CVE-2024-0985 (not CVE-2023-5869 as claimed in prior commit msg)
Peter Eisentraut [Mon, 5 Feb 2024 13:45:29 +0000 (14:45 +0100)]
Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash:
7465ae7935588bbbafa9aac1c2b8c5863de50cbb
Heikki Linnakangas [Mon, 5 Feb 2024 09:01:30 +0000 (11:01 +0200)]
Fix assertion if index is dropped during REFRESH CONCURRENTLY
When assertions are disabled, the built SQL statement is invalid and
you get a "syntax error". So this isn't a serious problem, but let's
avoid the assertion failure.
Backpatch to all supported versions.
Reviewed-by: Noah Misch
Heikki Linnakangas [Mon, 5 Feb 2024 09:01:23 +0000 (11:01 +0200)]
Run REFRESH MATERIALIZED VIEW CONCURRENTLY in right security context
The internal commands in REFRESH MATERIALIZED VIEW CONCURRENTLY are
correctly executed in SECURITY_RESTRICTED_OPERATION mode, except for
creating the temporary "diff" table, because you cannot create
temporary tables in SRO mode. But creating the temporary "diff" table
is a pretty complex CTAS command that selects from another temporary
table created earlier in the command. If you can cajole that CTAS
command to execute code defined by the table owner, the table owner
can run code with the privileges of the user running the REFRESH
command.
The proof-of-concept reported to the security team relied on CREATE
RULE to convert the internally-built temp table to a view. That's not
possible since commit
b23cd185fd, and I was not able to find a
different way to turn the SELECT on the temp table into code
execution, so as far as I know this is only exploitable in v15 and
below. That's a fiddly assumption though, so apply this patch to
master and all stable versions.
Thanks to Pedro Gallegos for the report.
Security: CVE-2023-5869
Reviewed-by: Noah Misch
Tom Lane [Sun, 4 Feb 2024 19:17:14 +0000 (14:17 -0500)]
Release notes for 16.2, 15.6, 14.11, 13.14, 12.18.
Heikki Linnakangas [Fri, 2 Feb 2024 22:18:21 +0000 (00:18 +0200)]
Fix typo in comments
Backpatch-through: v16
Tom Lane [Fri, 2 Feb 2024 20:34:29 +0000 (15:34 -0500)]
Translate ENOMEM to ERRCODE_OUT_OF_MEMORY in errcode_for_file_access().
Previously you got ERRCODE_INTERNAL_ERROR, which seems inappropriate,
especially given that we're trying to avoid emitting that in reachable
cases.
Alexander Kuzmenkov
Discussion: https://postgr.es/m/CALzhyqzgQph0BY8-hFRRGdHhF8CoqmmDHW9S=hMZ-HMzLxRqDQ@mail.gmail.com
Tom Lane [Fri, 2 Feb 2024 17:52:29 +0000 (12:52 -0500)]
First-draft release notes for 16.2.
As usual, the release notes for other branches will be made by cutting
these down, but put them up for community review first.
Noah Misch [Thu, 1 Feb 2024 21:44:19 +0000 (13:44 -0800)]
Sync PG_VERSION file in CREATE DATABASE.
An OS crash could leave PG_VERSION empty or missing. The same symptom
appeared in a backup by block device snapshot, taken after the next
checkpoint and before the OS flushes the PG_VERSION blocks. Device
snapshots are not a documented backup method, however. Back-patch to
v15, where commit
9c08aea6a3090a396be334cc58c511edab05776a introduced
STRATEGY=WAL_LOG and made it the default.
Discussion: https://postgr.es/m/
20240130195003[email protected]
Noah Misch [Thu, 1 Feb 2024 21:44:19 +0000 (13:44 -0800)]
Handle interleavings between CREATE DATABASE steps and base backup.
Restoring a base backup taken in the middle of CreateDirAndVersionFile()
or write_relmap_file() would lose the function's effects. The symptom
was absence of the database directory, PG_VERSION file, or
pg_filenode.map. If missing the directory, recovery would fail. Either
missing file would not fail recovery but would render the new database
unusable. Fix CreateDirAndVersionFile() with the transam/README "action
first and then write a WAL entry" strategy. That has a side benefit of
moving filesystem mutations out of a critical section, reducing the ways
to PANIC. Fix the write_relmap_file() call with a lock acquisition, so
it interacts with checkpoints like non-CREATE DATABASE calls do.
Back-patch to v15, where commit
9c08aea6a3090a396be334cc58c511edab05776a
introduced STRATEGY=WAL_LOG and made it the default.
Discussion: https://postgr.es/m/
20240130195003[email protected]
Tom Lane [Thu, 1 Feb 2024 20:57:53 +0000 (15:57 -0500)]
Update time zone data files to tzdata release 2024a.
DST law changes in Ittoqqortoormiit, Greenland (America/Scoresbysund),
Kazakhstan (Asia/Almaty and Asia/Qostanay) and Palestine; as well as
updates for the Antarctic stations Casey and Vostok.
Historical corrections for Vietnam, Toronto, and Miquelon.
Andrew Dunstan [Thu, 1 Feb 2024 20:17:41 +0000 (15:17 -0500)]
Avoid package qualification of $windows_os
Further fallout from commit
6ee26c6a4b. To keep code in sync and avoid
issues on older releases with different package names, simply use the
unqualified name like many other places in our code.
Tom Lane [Thu, 1 Feb 2024 17:34:21 +0000 (12:34 -0500)]
Apply band-aid fix for an oversight in reparameterize_path_by_child.
The path we wish to reparameterize is not a standalone object:
in particular, it implicitly references baserestrictinfo clauses
in the associated RelOptInfo, and if it's a SampleScan path then
there is also the TableSampleClause in the RTE to worry about.
Both of those could contain lateral references to the join partner
relation, which would need to be modified to refer to its child.
Since we aren't doing that, affected queries can give wrong answers,
or odd failures such as "variable not found in subplan target list",
or executor crashes. But we can't just summarily modify those
expressions, because they are shared with other paths for the rel.
We'd break things if we modify them and then end up using some
non-partitioned-join path.
In HEAD, we plan to fix this by postponing reparameterization
until create_plan(), when we know that those other paths are
no longer of interest, and then adjusting those expressions along
with the ones in the path itself. That seems like too big a change
for stable branches however. In the back branches, let's just detect
whether any troublesome lateral references actually exist in those
expressions, and fail reparameterization if so. This will result in
not performing a partitioned join in such cases. Given the lack of
field complaints, nobody's likely to miss the optimization.
Report and patch by Richard Guo. Apply to 12-16 only, since
the intended fix for HEAD looks quite different. We're not quite
ready to push the HEAD fix, but with back-branch releases coming
up soon, it seems wise to get this stopgap fix in place there.
Discussion: https://postgr.es/m/CAMbWs496+N=UAjOc=rcD3P7B6oJe4rZw08e_TZRUsWbPxZW3Tw@mail.gmail.com
Bruce Momjian [Thu, 1 Feb 2024 11:11:53 +0000 (06:11 -0500)]
doc: improve role option documentation
Role option management was changed in Postgres 16. This patch improves
the docs around these changes, including CREATE ROLE's INHERIT option,
inheritance handling, and grant's ability to change role options.
Discussion: https://postgr.es/m/
[email protected]
Co-authored-by: David G. Johnston
Backpatch-through: 16
Daniel Gustafsson [Thu, 1 Feb 2024 09:45:37 +0000 (10:45 +0100)]
doc: remove incorrect grammar for ALTER EVENT TRIGGER
The Parameters subsection had an extra TRIGGER in the grammar
for DISABLE/ENABLE which is incorrect. Backpatch down to all
supported versions since it's been like this all along.
Discussion: https://postgr.es/m/
0AFB171E-7E78-4A90-A140-
46AB270212CA@yesql.se
Backpatch-through: v12
Daniel Gustafsson [Thu, 1 Feb 2024 08:36:34 +0000 (09:36 +0100)]
doc: Fix incorrect openssl option
The openssl command for displaying the DN of a client certificate was
using --subject and not the single-dash option -subject. While recent
versions of openssl handles double dash options, earlier does not so
fix by using just -subject (which is per the openssl documentation).
Backpatch to v14 where this was introduced.
Reported-by: [email protected]
Discussion: https://postgr.es/m/
170672168899.666.
10442618407194498217@wrigleys.postgresql.org
Backpatch-through: v14
Michael Paquier [Thu, 1 Feb 2024 08:13:09 +0000 (17:13 +0900)]
Fix stats_fetch_consistency with stats for fixed-numbered objects
This impacts the statistics retrieved in transactions for the following
views when updating the value of stats_fetch_consistency, leading to
behaviors contrary to what is documented since
605994651b6a as an update
of this parameter should discard all statistics snapshot data:
- pg_stat_archiver
- pg_stat_bgwriter
- pg_stat_checkpointer
- pg_stat_io
- pg_stat_slru
- pg_stat_wal
For example, updating stats_fetch_consistency from "snapshot" to "cache"
in a transaction did not re-fetch any fresh data, using data cached from
the time when "snapshot" was in use.
Author: Shinya Kato
Discussion: https://postgr.es/m/
d77fc5190d4dbe1738d77231488e768b@oss.nttdata.com
Backpatch-through: 15
Michael Paquier [Wed, 31 Jan 2024 04:16:43 +0000 (13:16 +0900)]
Fix various issues with ALTER TEXT SEARCH CONFIGURATION
This commit addresses a set of issues when changing token type mappings
in a text search configuration when using duplicated token names:
- ADD MAPPING would fail on insertion because of a constraint failure
after inserting the same mapping.
- ALTER MAPPING with an "overridden" configuration failed with "tuple
already updated by self" when the token mappings are removed.
- DROP MAPPING failed with "tuple already updated by self", like
previously, but in a different code path.
The code is refactored so the token names (with their numbers) are
handled as a List with unique members rather than an array with numbers,
ensuring that no duplicates mess up with the catalog inserts, updates
and deletes. The list is generated by getTokenTypes(), with the same
error handling as previously while duplicated tokens are discarded from
the list used to work on the catalogs.
Regression tests are expanded to cover much more ground for the cases
fixed by this commit, as there was no coverage for the code touched in
this commit. A bit more is done regarding the fact that a token name
not supported by a configuration's parser should result in an error even
if IF EXISTS is used in a DROP MAPPING clause. This is implied in the
code but there was no coverage for that, and it was very easy to miss.
These issues exist since at least their introduction in core with
140d4ebcb46e, so backpatch all the way down.
Reported-by: Alexander Lakhin
Author: Tender Wang, Michael Paquier
Discussion: https://postgr.es/m/18310-
1eb233c5908189c8@postgresql.org
Backpatch-through: 12
Andrew Dunstan [Tue, 30 Jan 2024 22:09:44 +0000 (17:09 -0500)]
Fix 003_extrafiles.pl test for the Windows
File::Find converts backslashes to slashes in the newer Perl versions.
See: https://github.com/Perl/perl5/commit/
414f14df98cb1c9a20f92c5c54948b67c09f072d
So, do the same conversion for Windows before comparing paths. To
support all Perl versions, always convert them on Windows regardless of
the Perl's version.
Author: Nazir Bilal Yavuz
Backpatch to all live branches
Daniel Gustafsson [Tue, 30 Jan 2024 10:15:46 +0000 (11:15 +0100)]
pgcrypto: Fix check for buffer size
The code copying the PGP block into the temp buffer failed to
account for the extra 2 bytes in the buffer which are needed
for the prefix. If the block was oversized, subsequent checks
of the prefix would have exceeded the buffer size. Since the
block sizes are hardcoded in the list of supported ciphers it
can be verified that there is no live bug here. Backpatch all
the way for consistency though, as this bug is old.
Author: Mikhail Gribkov
Discussion: https://postgr.es/m/CAMEv5_uWvcMCMdRFDsJLz2Q8g16HEa9xWyfrkr+FYMMFJhawOw@mail.gmail.com
Backpatch-through: v12
David Rowley [Mon, 29 Jan 2024 21:15:51 +0000 (10:15 +1300)]
Doc: mention foreign keys can reference unique indexes
We seem to have only documented a foreign key can reference the columns of
a primary key or unique constraint. Here we adjust the documentation
to mention columns in a non-partial unique index can be mentioned too.
The header comment for transformFkeyCheckAttrs() also didn't mention
unique indexes, so fix that too. In passing make that header comment
reflect reality in the various other aspects where it deviated from it.
Bug: 18295
Reported-by: Gilles PARC
Author: Laurenz Albe, David Rowley
Discussion: https://www.postgresql.org/message-id/18295-
0ed0fac5c9f7b17b%40postgresql.org
Backpatch-through: 12
Nathan Bossart [Mon, 29 Jan 2024 18:09:03 +0000 (12:09 -0600)]
Move is_valid_ascii() to ascii.h.
This function requires simd.h, which is a rather large dependency
for a widely-used header file like pg_wchar.h. Furthermore, there
is a report of a third-party tool that is struggling to use
pg_wchar.h due to its dependence on simd.h (presumably because
simd.h uses several intrinsics). Moving the function to the much
less popular ascii.h resolves these issues for now.
This commit is back-patched for the benefit of the aforementioned
third-party tool. The simd.h dependency was only added in v16,
but we've opted to back-patch to v15 so that is_valid_ascii() lives
in the same file for all versions where it exists. This could
break existing third-party code that uses the function, but we
couldn't find any examples of such code. It should be possible to
fix any code that this commit breaks by including ascii.h in the
file that uses is_valid_ascii().
Author: Jubilee Young
Reviewed-by: Tom Lane, John Naylor, Andres Freund, Eric Ridge
Discussion: https://postgr.es/m/CAPNHn3oKJJxMsYq%2BqLYzVJOFrUcOr4OF1EC-KtFT-qh8nOOOtQ%40mail.gmail.com
Backpatch-through: 15
Tom Lane [Mon, 29 Jan 2024 17:06:07 +0000 (12:06 -0500)]
Fix incompatibilities with libxml2 >= 2.12.0.
libxml2 changed the required signature of error handler callbacks
to make the passed xmlError struct "const". This is causing build
failures on buildfarm member caiman, and no doubt will start showing
up in the field quite soon. Add a version check to adjust the
declaration of xml_errorHandler() according to LIBXML_VERSION.
2.12.x also produces deprecation warnings for contrib/xml2/xpath.c's
assignment to xmlLoadExtDtdDefaultValue. I see no good reason for
that to still be there, seeing that we disabled external DTDs (at a
lower level) years ago for security reasons. Let's just remove it.
Back-patch to all supported branches, since they might all get built
with newer libxml2 once it gets a bit more popular. (The back
branches produce another deprecation warning about xpath.c's use of
xmlSubstituteEntitiesDefault(). We ought to consider whether to
back-patch all or part of commit
65c5864d7 to silence that. It's
less urgent though, since it won't break the buildfarm.)
Discussion: https://postgr.es/m/
1389505.
1706382262@sss.pgh.pa.us
Heikki Linnakangas [Mon, 29 Jan 2024 11:46:22 +0000 (13:46 +0200)]
Fix locking when fixing an incomplete split of a GIN internal page
ginFinishSplit() expects the caller to hold an exclusive lock on the
buffer, but when finishing an earlier "leftover" incomplete split of
an internal page, the caller held a shared lock. That caused an
assertion failure in MarkBufferDirty(). Without assertions, it could
lead to corruption if two backends tried to complete the split at the
same time.
On master, add a test case using the new injection point facility.
Report and analysis by Fei Changhong. Backpatch the fix to all
supported versions.
Reviewed-by: Fei Changhong, Michael Paquier
Discussion: https://www.postgresql.org/message-id/
[email protected]
Michael Paquier [Thu, 25 Jan 2024 08:07:56 +0000 (17:07 +0900)]
Add more LOG messages when starting and ending recovery from a backup
Three LOG messages are added in the recovery code paths, providing
information that can be useful to track corruption issues depending on
the state of the cluster, telling that:
- Recovery has started from a backup_label.
- Recovery is restarting from a backup start LSN, without a
backup_label.
- Recovery has completed from a backup.
This was originally applied on HEAD as of
1d35f705e191, and there is
consensus that this can be useful for older versions. This applies
cleanly down to 15, so do it down to this version for now (older
versions have heavily refactored the WAL recovery paths, making the
change less straight-forward to do).
Author: Andres Freund
Reviewed-by: David Steele, Laurenz Albe, Michael Paquier
Discussion: https://postgr.es/m/
20231117041811[email protected]
Backpatch-through: 15
Michael Paquier [Sun, 28 Jan 2024 23:06:03 +0000 (08:06 +0900)]
Fix DROP ROLE when specifying duplicated roles
This commit fixes failures with "tuple already updated by self" when
listing twice the same role and in a DROP ROLE query.
This is an oversight in
6566133c5f52, that has introduced a two-phase
logic in DropRole() where dependencies of all the roles to drop are
removed in a first phase, with the roles themselves removed from
pg_authid in a second phase.
The code is simplified to not rely on a List of ObjectAddress built in
the first phase used to remove the pg_authid entries in the second
phase, switching to a list of OIDs. Duplicated OIDs can be simply
avoided in the first phase thanks to that. Using ObjectAddress was not
necessary for the roles as they are not used for anything specific to
dependency.c, building all the ObjectAddress in the List with
AuthIdRelationId as class ID.
In 15 and older versions, where a single phase is used, DROP ROLE with
duplicated role names would fail on "role \"blah\" does not exist" for
the second entry after the CCI() done by the first deletion. This is
not really incorrect, but it does not seem worth changing based on a
lack of complaints.
Reported-by: Alexander Lakhin
Reviewed-by: Tender Wang
Discussion: https://postgr.es/m/18310-
1eb233c5908189c8@postgresql.org
Backpatch-through: 16
Tom Lane [Fri, 26 Jan 2024 20:54:17 +0000 (15:54 -0500)]
Compare varnullingrels too in assign_param_for_var().
Oversight in
2489d76c4. Preliminary analysis suggests that the
problem may be unreachable --- but if we did have instances of
the same column with different varnullingrels, we'd surely need
to treat them as different Params.
Discussion: https://postgr.es/m/412552.
1706203379@sss.pgh.pa.us
Tom Lane [Fri, 26 Jan 2024 18:39:37 +0000 (13:39 -0500)]
Detect Julian-date overflow in timestamp[tz]_pl_interval.
We perform addition of the days field of an interval via
arithmetic on the Julian-date representation of the timestamp's date.
This step is subject to int32 overflow, and we also should not let
the Julian date become very negative, for fear of weird results from
j2date. (In the timestamptz case, allow a Julian date of -1 to pass,
since it might convert back to zero after timezone rotation.)
The additions of the months and microseconds fields could also
overflow, of course. However, I believe we need no additional
checks there; the existing range checks should catch such cases.
The difficulty here is that j2date's magic modular arithmetic could
produce something that looks like it's in-range.
Per bug #18313 from Christian Maurer. This has been wrong for
a long time, so back-patch to all supported branches.
Discussion: https://postgr.es/m/18313-
64d2c8952d81e84b@postgresql.org
Thomas Munro [Wed, 24 Jan 2024 21:37:35 +0000 (10:37 +1300)]
Track LLVM 18 changes.
A function was given a newly standard name from C++20 in LLVM 16. Then
LLVM 18 added a deprecation warning for the old name, and it is about to
ship, so it's time to adjust that.
Back-patch to all supported releases.
Discussion: https://www.postgresql.org/message-id/CA+hUKGLbuVhH6mqS8z+FwAn4=5dHs0bAWmEMZ3B+iYHWKC4-ZA@mail.gmail.com
Michael Paquier [Wed, 24 Jan 2024 05:20:08 +0000 (14:20 +0900)]
Fix ALTER TABLE .. ADD COLUMN with complex inheritance trees
This command, when used to add a column on a parent table with a complex
inheritance tree, tried to update multiple times the same tuple in
pg_attribute for a child table when incrementing attinhcount, causing
failures with "tuple already updated by self" because of a missing
CommandCounterIncrement() between two updates.
This exists for a rather long time, so backpatch all the way down.
Reported-by: Alexander Lakhin
Author: Tender Wang
Reviewed-by: Richard Guo
Discussion: https://postgr.es/m/18297-
b04cd83a55b51e35@postgresql.org
Backpatch-through: 12
Peter Eisentraut [Tue, 23 Jan 2024 11:15:21 +0000 (12:15 +0100)]
meson: portname was set too early
portname is set to host_system, but host_system might still be changed
later in the file. As a consequence, platforms where host_system is
changed later wouldn't find some of their platform-specific files
(driven by portname), and the build would fail. Move the assignment
of portname further down after the last change of host_system (but
before further overriding assignments to portname).
Discussion: https://www.postgresql.org/message-id/flat/CAC7zN94TdsHhY88XkroJzSMx7E%3DBQpV9LKKjNSEnTM04ihoWCA%40mail.gmail.com
Heikki Linnakangas [Tue, 23 Jan 2024 08:38:07 +0000 (10:38 +0200)]
Revert "libpqwalreceiver: Convert to libpq-be-fe-helpers.h"
This reverts commit
728f86fec65537eade8d9e751961782ddb527934.
The signal handling was a few bricks shy of a load in that commit,
which made the walreceiver non-responsive to SIGTERM while it was
waiting for the connection to be established. That prevented a standby
from being promoted.
Since it was non-essential refactoring, let's revert it to make v16
work the same as earlier releases. I reverted it in 'master' too, to
keep the branches in sync. The refactoring was a good idea as such,
but it needs a bit more work. Once we have developed a complete patch
with this issue fixed, let's re-apply that to 'master'.
Reported-by: Kyotaro Horiguchi
Backpatch-through: 16
Discussion: https://www.postgresql.org/message-id/
20231231.200741.
1078989336605759878[email protected]
Michael Paquier [Tue, 23 Jan 2024 05:46:05 +0000 (14:46 +0900)]
Improve stability of recovery test 035_standby_logical_decoding
This commit tweaks a couple of things in 035_standby_logical_decoding to
hopefully stabilize it:
- Autovacuum is now disabled, as it could hold a global xmin with a
transaction.
- Conflicts are generated with command sequences that removed rows (on
catalogs, shared or non-shared, or just plain relations) followed by a
VACUUM. This was unstable because these did not check that the horizon
moved between the SQL commands and the VACUUM. The logic is refactored
as follows, to ensure that VACUUM removes dead rows before testing for
slot invalidation on a standby (idea suggested by Andres Freund):
-- Grab the current horizon.
-- Launch SQL commands removing rows.
-- Check that the snapshot horizon has been updated.
-- Launch VACUUM on the relation whose rows have been removed by the
first step.
Note that there are still some issues because of standby snapshot WAL
records generated by the bgwriter, but this makes the test much more
stable.
Per reports from buildfarm members dikkop and skink, with analysis and
tests from Alexander Lakhin.
While on it, fix a couple of incorrect comments.
Author: Bertrand Drouvot
Reviewed-by: Alexander Lakhin, Michael Paquier
Discussion: https://postgr.es/m/OSZPR01MB6310ED3CEDB531BCEDBC6AF2FD479@OSZPR01MB6310.jpnprd01.prod.outlook.com
Discussion: https://postgr.es/m/
bf67e076-b163-9ba3-4ade-
b9fc51a3a8f6@gmail.com
Backpatch-through: 16
Alvaro Herrera [Mon, 22 Jan 2024 16:48:30 +0000 (17:48 +0100)]
Abort pgbench if script end is reached with an open pipeline
When a pipeline is opened with \startpipeline and not closed, pgbench
will either error on the next transaction with a "already in pipeline
mode" error or successfully end if this was the last transaction --
despite not sending anything that was piped in the pipeline.
Make it an error to reach end of script is reached while there's an
open pipeline.
Backpatch to 14, where pgbench got support for pipelines.
Author: Anthonin Bonnefoy
Reported-by: Michael Paquier
Discussion: https://postgr.es/m/[email protected]
David Rowley [Mon, 22 Jan 2024 09:45:33 +0000 (22:45 +1300)]
Re-disallow Memoize for parameterized nested loops with join filters
This was previously fixed in
9e215378d but got broken again as a result
of
2489d76c4. It seems that commit causes ppi_clauses to contain
duplicate clauses and it's no longer safe to check the list_length of
that list to determine if there are join conditions other than what's
mentioned in ppi_clauses.
Here we adjust the check to count the distinct rinfo_serial mentioned in
ppi_clauses. We expect that extra->restrictlist won't have duplicate
rinfo_serials.
Reported-by: Amadeo Gallardo
Author: Richard Guo
Discussion: https://postgr.es/m/CADFREbW-BLJd7-a5J%2B5wjVumeFG1ByXiSOFzMtkmY_SDWckTxw%40mail.gmail.com
Backpatch-through: 16, where
2489d76c4 was introduced.
Jeff Davis [Thu, 18 Jan 2024 23:00:15 +0000 (15:00 -0800)]
Fix buildfarm error from commit
5c31669058.
Skip test when not using unix domain sockets.
Discussion: https://postgr.es/m/CALDaNm29-8OozsBWo9H6DN_Tb_3yA1QjRJput-KhaN8ncDJtJA@mail.gmail.com
Backpatch-through: 16
Tom Lane [Thu, 18 Jan 2024 21:10:57 +0000 (16:10 -0500)]
Fix plpgsql to allow new-style SQL CREATE FUNCTION as a SQL command.
plpgsql fails on new-style CREATE FUNCTION/PROCEDURE commands within
a routine or DO block, because make_execsql_stmt believes that a
semicolon token always terminates a SQL command. Now, that's actually
been wrong since the day it was written, because CREATE RULE has long
allowed multiple rule actions separated by semicolons. But there are
few enough people using multi-action rules that there was never an
attempt to fix it. New-style SQL functions, though, are popular.
psql has this same problem of "does this semicolon really terminate
the command?". It deals with CREATE RULE by counting parenthesis
nesting depth: a semicolon within parens doesn't end a command.
Commits
e717a9a18 and
029c5ac03 created a similar heuristic to count
matching BEGIN/END pairs (but only within CREATEs, so as not to be
fooled by plain BEGIN). That's survived several releases now without
trouble reports, so let's just absorb those heuristics into plpgsql.
Per report from Samuel Dussault. Back-patch to v14 where new-style
SQL function syntax came in.
Discussion: https://postgr.es/m/YT2PR01MB88552C3E9AD40A6C038774A781722@YT2PR01MB8855.CANPRD01.PROD.OUTLOOK.COM
Michael Paquier [Thu, 18 Jan 2024 07:31:38 +0000 (16:31 +0900)]
Improve handling of dropped partitioned indexes for REINDEX INDEX
A REINDEX INDEX done on a partitioned index builds a list of the indexes
to work on before processing its partitions in individual transactions.
When combined with a DROP of the partitioned index, there was a window
where it was possible to see some unexpected "could not open relation
with OID", synonym of relation lookup error. The code was robust enough
to handle the case where the parent relation is missing, but not the
case where an index would be gone missing.
This is similar to
1d65416661bb.
Support for REINDEX on partitioned relations has been introduced in
a6642b3ae060, so backpatch down to 14.
Author: Fei Changhong
Discussion: https://postgr.es/m/
[email protected]
Backpatch-through: 14
Michael Paquier [Thu, 18 Jan 2024 06:04:31 +0000 (15:04 +0900)]
Add try_index_open(), conditional variant of index_open()
try_index_open() is able to open an index if its relkind fits, except
that it would return NULL instead of generated an error if the relation
does not exist. This new routine will be used by an upcoming patch to
make REINDEX on partitioned relations more robust when an index in a
partition tree is dropped.
Extracted from a larger patch by the same author.
Author: Fei Changhong
Discussion: https://postgr.es/m/
[email protected]
Backpatch-through: 14
Michael Paquier [Thu, 18 Jan 2024 01:12:51 +0000 (10:12 +0900)]
seg: Add test "security" in meson.build
Oversight in
681d9e4621aa where the test has been added.
Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/
[email protected]
Backpatch-through: 16
Heikki Linnakangas [Wed, 17 Jan 2024 13:44:10 +0000 (15:44 +0200)]
Fix incorrect comment on how BackendStatusArray is indexed
The comment was copy-pasted from the call to ProcSignalInit() in
AuxiliaryProcessMain(), which uses a similar scheme of having reserved
slots for aux processes after MaxBackends slots for backends. However,
ProcSignalInit() indexing starts from 1, whereas BackendStatusArray
starts from 0. The code is correct, but the comment was wrong.
Discussion: https://www.postgresql.org/message-id/
f3ecd4cb-85ee-4e54-8278-
5fabfb3a4ed0@iki.fi
Backpatch-through: v14
Daniel Gustafsson [Wed, 17 Jan 2024 10:24:11 +0000 (11:24 +0100)]
Close socket in case of errors in setting non-blocking
If configuring the newly created socket non-blocking fails we
error out and return INVALID_SOCKET, but the socket that had
been created wasn't closed. Fix by issuing closesocket in the
errorpath.
Backpatch to all supported branches.
Author: Ranier Vilela
Discussion: https://postgr.es/m/CAEudQApmU5CrKefH85VbNYE2y8H=-qqEJbg6RAPU65+vCe+89A@mail.gmail.com
Backpatch-through: v12
Daniel Gustafsson [Tue, 16 Jan 2024 12:51:15 +0000 (13:51 +0100)]
Decorate WITH with literal markup tags
One instance of "WITH clause" was not using tags around
WITH, while others were, so add markup to the last one to ensure
consistency. Backpatch to v15 where MERGE was added.
Reported-by: jian he
Discussion: https://postgr.es/m/CACJufxGJKY9ZCPV2WDM6xFsXq9C8r7r3vU6AkScN+p9k6CEpMw@mail.gmail.com
Backpatch-through: v15
Alvaro Herrera [Tue, 16 Jan 2024 11:27:52 +0000 (12:27 +0100)]
Don't test already-referenced pointer for nullness
Commit
b8ba7344e9eb added in PQgetResult a derefence to a pointer
returned by pqPrepareAsyncResult(), before some other code that was
already testing that pointer for nullness. But since commit
618c16707a6d (in Postgres 15), pqPrepareAsyncResult() doesn't ever
return NULL (a statically-allocated result is returned if OOM). So in
branches 15 and up, we can remove the redundant pointer check with no
harm done.
However, in branch 14, pqPrepareAsyncResult() can indeed return NULL if
it runs out of memory. Fix things there by adding a null pointer check
before dereferencing the pointer. This should hint Coverity that the
preexisting check is not redundant but necessary.
Backpatch to 14, like
b8ba7344e9eb.
Per Coverity.
Tom Lane [Sun, 14 Jan 2024 17:38:41 +0000 (12:38 -0500)]
Prevent access to an unpinned buffer in BEFORE ROW UPDATE triggers.
When ExecBRUpdateTriggers switches to a new target tuple as a result
of the EvalPlanQual logic, it must form a new proposed update tuple.
Since commit
86dc90056, that tuple (the result of
ExecGetUpdateNewTuple) has been a virtual tuple that might contain
pointers to by-ref fields of the new target tuple (in "oldslot").
However, immediately after that we materialize oldslot, causing it to
drop its buffer pin, whereupon the by-ref pointers are unsafe to use.
This is a live bug only when the new target tuple is in a different
page than the original target tuple, since we do still hold a pin on
the original one. (Before
86dc90056, there was no bug because the
EPQ plantree would hold a pin on the new target tuple; but now that's
not assured.) To fix, forcibly materialize the new tuple before we
materialize oldslot. This costs nothing since we would have done that
shortly anyway.
The real-world impact of this is probably minimal. A visible failure
could occur if the new target tuple's buffer were recycled for some
other page in the short interval before we materialize newslot within
the trigger-calling loop; but that's quite unlikely given that we'd
just touched that page. There's a larger hazard that some other
process could prune and repack that page within the window. We have
lock on the new target tuple, but that wouldn't prevent it being moved
on the page.
Alexander Lakhin and Tom Lane, per bug #17798 from Alexander Lakhin.
Back-patch to v14 where
86dc90056 came in.
Discussion: https://postgr.es/m/17798-
0907404928dcf0dd@postgresql.org
Tom Lane [Sat, 13 Jan 2024 18:54:11 +0000 (13:54 -0500)]
Re-pgindent catcache.c after previous commit.
Discussion: https://postgr.es/m/
1393953.
1698353013@sss.pgh.pa.us
Discussion: https://postgr.es/m/CAGjhLkOoBEC9mLsnB42d3CO1vcMx71MLSEuigeABbQ8oRdA6gw@mail.gmail.com
Tom Lane [Sat, 13 Jan 2024 18:46:27 +0000 (13:46 -0500)]
Cope with catcache entries becoming stale during detoasting.
We've long had a policy that any toasted fields in a catalog tuple
should be pulled in-line before entering the tuple in a catalog cache.
However, that requires access to the catalog's toast table, and we'll
typically do AcceptInvalidationMessages while opening the toast table.
So it's possible that the catalog tuple is outdated by the time we
finish detoasting it. Since no cache entry exists yet, we can't
mark the entry stale during AcceptInvalidationMessages, and instead
we'll press forward and build an apparently-valid cache entry. The
upshot is that we have a race condition whereby an out-of-date entry
could be made in a backend's catalog cache, and persist there
indefinitely causing indeterminate misbehavior.
To fix, use the existing systable_recheck_tuple code to recheck
whether the catalog tuple is still up-to-date after we finish
detoasting it. If not, loop around and restart the process of
searching the catalog and constructing cache entries from the top.
The case is rare enough that this shouldn't create any meaningful
performance penalty, even in the SearchCatCacheList case where
we need to tear down and reconstruct the whole list.
Indeed, the case is so rare that AFAICT it doesn't occur during
our regression tests, and there doesn't seem to be any easy way
to build a test that would exercise it reliably. To allow
testing of the retry code paths, add logic (in USE_ASSERT_CHECKING
builds only) that randomly pretends that the recheck failed about
one time out of a thousand. This is enough to ensure that we'll
pass through the retry paths during most regression test runs.
By adding an extra level of looping, this commit creates a need
to reindent most of SearchCatCacheMiss and SearchCatCacheList.
I'll do that separately, to allow putting those changes in
.git-blame-ignore-revs.
Patch by me; thanks to Alexander Lakhin for having built a test
case to prove the bug is real, and to Xiaoran Wang for review.
Back-patch to all supported branches.
Discussion: https://postgr.es/m/
1393953.
1698353013@sss.pgh.pa.us
Discussion: https://postgr.es/m/CAGjhLkOoBEC9mLsnB42d3CO1vcMx71MLSEuigeABbQ8oRdA6gw@mail.gmail.com
Jeff Davis [Sat, 13 Jan 2024 05:39:35 +0000 (21:39 -0800)]
Fix memory leak in connection string validation.
Introduced in commit
c3afe8cf5a.
Discussion: https://postgr.es/m/
066a65233d3cb4ea27a9e0778d2f1d0dc764b222[email protected]
Reviewed-by: Nathan Bossart, Tom Lane
Backpatch-through: 16
Jeff Davis [Fri, 12 Jan 2024 21:42:09 +0000 (13:42 -0800)]
Re-validate connection string in libpqrcv_connect().
A superuser may create a subscription with password_required=true, but
which uses a connection string without a password.
Previously, if the owner of such a subscription was changed to a
non-superuser, the non-superuser was able to utilize a password from
another source (like a password file or the PGPASSWORD environment
variable), which should not have been allowed.
This commit adds a step to re-validate the connection string before
connecting.
Reported-by: Jeff Davis
Author: Vignesh C
Reviewed-by: Peter Smith, Robert Haas, Amit Kapila
Discussion: https://www.postgresql.org/message-id/flat/
e5892973ae2a80a1a3e0266806640dae3c428100.camel%40j-davis.com
Backpatch-through: 16
Alvaro Herrera [Fri, 12 Jan 2024 11:44:20 +0000 (12:44 +0100)]
Added literal tag for RETURNING
This is an old mistake (
92e38182d7c8); backpatch all the way back.
Author: Atsushi Torikoshi
Reviewed-by: Ashutosh Bapat
Discussion: https://postgr.es/m/c0aa00b60a16c0ea2a4c5123b013acb9@oss.nttdata.com
Michael Paquier [Fri, 12 Jan 2024 04:59:51 +0000 (13:59 +0900)]
pg_regress: Disable autoruns for cmd.exe on Windows
This is similar to
9886744a361b, to prevent the execution of other
programs due to autorun configurations which could influence the
postmaster startup.
This was originally applied on HEAD as of
83c75ac7fb69 without a
backpatch, but the patch has survived CI and buildfarm cycles. I have
checked that cmd /d exists down to Windows XP, which should make this
change work correctly in the oldest branches still supported.
Discussion: https://postgr.es/m/
20230922.161551.
320043332510268554[email protected]
Backpatch-through: 12
Michael Paquier [Fri, 12 Jan 2024 04:53:07 +0000 (13:53 +0900)]
pg_ctl: Disable autoruns for cmd.exe on Windows
On Windows, cmd.exe is used to launch the postmaster process to ease its
redirection setup. However, cmd.exe may execute other programs at
startup due to autorun configurations, which could influence the
postmaster startup. This patch adds /D flag to the launcher cmd.exe
command line to disable autorun settings written in the registry.
This was originally applied on HEAD as of
9886744a361b without a
backpatch, but the patch has survived CI and buildfarm cycles. I have
checked that cmd /d exists down to Windows XP, which should make this
change work correctly in the oldest branches still supported.
Reported-by: Hayato Kuroda
Author: Kyotaro Horiguchi
Reviewed-by: Robert Haas, Michael Paquier
Discussion: https://postgr.es/m/
20230922.161551.
320043332510268554[email protected]
Backpatch-through: 12
Tom Lane [Thu, 11 Jan 2024 20:28:13 +0000 (15:28 -0500)]
Allow subquery pullup to wrap a PlaceHolderVar in another one.
The code for wrapping subquery output expressions in PlaceHolderVars
believed that if the expression already was a PlaceHolderVar, it was
never necessary to wrap that in another one. That's wrong if the
expression is underneath an outer join and involves a lateral
reference to outside that scope: failing to add an additional PHV
risks evaluating the expression at the wrong place and hence not
forcing it to null when the outer join should do so. This is an
oversight in commit
9e7e29c75, which added logic to forcibly wrap
lateral-reference Vars in PlaceHolderVars, but didn't see that the
adjacent case for PlaceHolderVars needed the same treatment.
The test case we have for this doesn't fail before
4be058fe9, but now
that I see the problem I wonder if it is possible to demonstrate
related errors before that. That's moot though, since all such
branches are out of support.
Per bug #18284 from Holger Reise. Back-patch to all supported
branches.
Discussion: https://postgr.es/m/18284-
47505a20c23647f8@postgresql.org