postgresql.git
15 months agoFix race leading to incorrect conflict cause in InvalidatePossiblyObsoleteSlot()
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

15 months agodoc: Use system-username instead of system-user
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

15 months agoFix incorrect pruning of NULL partition for boolean IS NOT clauses
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

15 months agoFix test race between primary XLOG_RUNNING_XACTS and standby logical slot.
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]

15 months agoBound waits in 035_standby_logical_decoding.pl.
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]

15 months agoDoc: fix typo in SECURITY LABEL synopsis.
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

15 months agoecpg: Fix zero-termination of string generated by intoasc()
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

16 months agoRemove non-existing file from .gitattributes
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

16 months agoDoc: improve a couple of comments in postgresql.conf.sample.
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

16 months agodoc: Remove links to further reading from pgcrypto docs
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

16 months agoFix 'mmap' DSM implementation with allocations larger than 4 GB
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

16 months agoUse a safer outfuncs/readfuncs representation for BitStrings.
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

16 months agoSkip .DS_Store files in server side utils
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

16 months agoFix gai_strerror() thread-safety on Windows.
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

16 months agoRemove race condition in pg_get_expr().
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

16 months agoAvoid concurrent calls to bindtextdomain().
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

16 months agoClean up Windows-specific mutex code in libpq and ecpglib.
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

16 months agoFix wrong logic in TransactionIdInRecentPast()
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

16 months agoFix propagation of persistence to sequences in ALTER TABLE / ADD COLUMN
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

16 months agodoc: Remove superfluous bracket in synopsis
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

16 months agoUpdate comment
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.

16 months agoUpdate PQparameterStatus and ParameterStatus docs
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]

16 months agoStamp 16.2. REL_16_2
Tom Lane [Mon, 5 Feb 2024 21:41:37 +0000 (16:41 -0500)]
Stamp 16.2.

16 months agoLast-minute updates for release notes.
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)

16 months agoTranslation updates
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

16 months agoFix assertion if index is dropped during REFRESH CONCURRENTLY
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
16 months agoRun REFRESH MATERIALIZED VIEW CONCURRENTLY in right security context
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
16 months agoRelease notes for 16.2, 15.6, 14.11, 13.14, 12.18.
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.

16 months agoFix typo in comments
Heikki Linnakangas [Fri, 2 Feb 2024 22:18:21 +0000 (00:18 +0200)]
Fix typo in comments

Backpatch-through: v16

16 months agoTranslate ENOMEM to ERRCODE_OUT_OF_MEMORY in errcode_for_file_access().
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

16 months agoFirst-draft release notes for 16.2.
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.

16 months agoSync PG_VERSION file in CREATE DATABASE.
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]

16 months agoHandle interleavings between CREATE DATABASE steps and base backup.
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]

16 months agoUpdate time zone data files to tzdata release 2024a.
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.

16 months agoAvoid package qualification of $windows_os
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.

16 months agoApply band-aid fix for an oversight in reparameterize_path_by_child.
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

16 months agodoc: improve role option documentation
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

16 months agodoc: remove incorrect grammar for ALTER EVENT TRIGGER
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

16 months agodoc: Fix incorrect openssl option
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

16 months agoFix stats_fetch_consistency with stats for fixed-numbered objects
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

16 months agoFix various issues with ALTER TEXT SEARCH CONFIGURATION
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

16 months agoFix 003_extrafiles.pl test for the Windows
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

16 months agopgcrypto: Fix check for buffer size
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

16 months agoDoc: mention foreign keys can reference unique indexes
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

16 months agoMove is_valid_ascii() to ascii.h.
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

16 months agoFix incompatibilities with libxml2 >= 2.12.0.
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

16 months agoFix locking when fixing an incomplete split of a GIN internal page
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]

16 months agoAdd more LOG messages when starting and ending recovery from a backup
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

16 months agoFix DROP ROLE when specifying duplicated roles
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

16 months agoCompare varnullingrels too in assign_param_for_var().
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

16 months agoDetect Julian-date overflow in timestamp[tz]_pl_interval.
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

16 months agoTrack LLVM 18 changes.
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

16 months agoFix ALTER TABLE .. ADD COLUMN with complex inheritance trees
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

16 months agomeson: portname was set too early
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

16 months agoRevert "libpqwalreceiver: Convert to libpq-be-fe-helpers.h"
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]

16 months agoImprove stability of recovery test 035_standby_logical_decoding
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

16 months agoAbort pgbench if script end is reached with an open pipeline
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]

16 months agoRe-disallow Memoize for parameterized nested loops with join filters
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.

17 months agoFix buildfarm error from commit 5c31669058.
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

17 months agoFix plpgsql to allow new-style SQL CREATE FUNCTION as a SQL command.
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

17 months agoImprove handling of dropped partitioned indexes for REINDEX INDEX
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

17 months agoAdd try_index_open(), conditional variant of index_open()
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

17 months agoseg: Add test "security" in meson.build
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

17 months agoFix incorrect comment on how BackendStatusArray is indexed
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

17 months agoClose socket in case of errors in setting non-blocking
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

17 months agoDecorate WITH with literal markup tags
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

17 months agoDon't test already-referenced pointer for nullness
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.

17 months agoPrevent access to an unpinned buffer in BEFORE ROW UPDATE triggers.
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

17 months agoRe-pgindent catcache.c after previous commit.
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

17 months agoCope with catcache entries becoming stale during detoasting.
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

17 months agoFix memory leak in connection string validation.
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

17 months agoRe-validate connection string in libpqrcv_connect().
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

17 months agoAdded literal tag for RETURNING
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

17 months agopg_regress: Disable autoruns for cmd.exe on Windows
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

17 months agopg_ctl: Disable autoruns for cmd.exe on Windows
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

17 months agoAllow subquery pullup to wrap a PlaceHolderVar in another one.
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

17 months agoFix omission in partitioning limitation documentation
Magnus Hagander [Thu, 11 Jan 2024 13:27:10 +0000 (14:27 +0100)]
Fix omission in partitioning limitation documentation

UNIQUE and PRIMARY KEY constraints can be created on ONLY the
partitioned table.  We already had an example demonstrating that,
but forgot to mention it in the documentation of the limits of
partitioning.

Author: Laurenz Albe
Reviewed-By: shihao zhong, Shubham Khanna, Ashutosh Bapat
Backpatch-through: 12
Discussion: https://postgr.es/m/167299368731.659.16130012959616771853@wrigleys.postgresql.org

17 months agoRestore initdb's old behavior of always setting the lc_xxx GUCs.
Tom Lane [Wed, 10 Jan 2024 23:09:29 +0000 (18:09 -0500)]
Restore initdb's old behavior of always setting the lc_xxx GUCs.

In commit 3e51b278d I (tgl) caused initdb to leave lc_messages and
other lc_xxx GUCs commented-out in the installed postgresql.conf file
if they were going to be set to 'C'.  This was a hack for cosmetic
purposes, and it was buggy because lc_messages' wired-in default is
not 'C' but '' (empty string).  That led to --no-locale not having
the expected effect, since the postmaster would then obtain
lc_messages from its startup environment.

Let's just revert to the prior behavior of always de-commenting the
lc_xxx entries; the argument for changing that longstanding behavior
was weak in the first place.

Also, fix postgresql.conf.sample's erroneous claim that the default
value of lc_messages is 'C'.  I suspect that was what misled me into
making this mistake in the first place.

Report and patch by Kyotaro Horiguchi.  Back-patch to v16 where
the problem was introduced.

Discussion: https://postgr.es/m/20231122.162700.1995154567625541112[email protected]

17 months agoHandle WindowClause.runCondition in tree walker/mutator functions.
Tom Lane [Wed, 10 Jan 2024 18:36:34 +0000 (13:36 -0500)]
Handle WindowClause.runCondition in tree walker/mutator functions.

Commit 9d9c02ccd, which added the notion of a "run condition" for
window functions, neglected to teach nodeFuncs.c to process the new
field.  Remarkably, that doesn't seem to have had any ill effects
before we invented Var.varnullingrels, but now it can cause visible
failures in join-removal scenarios.

I have no faith that there's not reachable problems in v15 too,
so back-patch the code change to v15 where 9d9c02ccd came in.
The test case seems irrelevant to v15, though.

Per bug #18277 from Zuming Jiang.  Diagnosis and patch by
Richard Guo.

Discussion: https://postgr.es/m/18277-089ead83b329a2fd@postgresql.org

17 months agodoc: clarify who owns the initdb-created cluster, by default
Bruce Momjian [Wed, 10 Jan 2024 16:20:15 +0000 (11:20 -0500)]
doc:  clarify who owns the initdb-created cluster, by default

Discussion: https://postgr.es/m/[email protected]

Backpatch-through: 16

17 months agoDoc: fix character_sets view.
Tatsuo Ishii [Tue, 9 Jan 2024 10:46:36 +0000 (19:46 +0900)]
Doc: fix character_sets view.

The note regarding character encoding form in "The Information Schema"
said that LATIN1 character repertoires only use one encoding form
LATIN1. This is not correct because LATIN1 has another encoding form
ISO-2022-JP-2. To fix this, replace LATIN1 with LATIN2, which is not
supported by ISO-2022-JP-2, thus it can be said that LATIN2 only uses
one encoding form.

Back-patch to supported branches.

Author: Tatsuo Ishii
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/flat/20240102.153925.1147403616414525145.t-ishii%40sranhm.sra.co.jp

17 months agoFix indentation in ExecParallelHashIncreaseNumBatches()
Alexander Korotkov [Mon, 8 Jan 2024 17:43:05 +0000 (19:43 +0200)]
Fix indentation in ExecParallelHashIncreaseNumBatches()

Backpatch-through: 12

17 months agoFix integer-overflow problem in intarray's g_int_decompress().
Tom Lane [Sun, 7 Jan 2024 20:19:50 +0000 (15:19 -0500)]
Fix integer-overflow problem in intarray's g_int_decompress().

An array element equal to INT_MAX gave this code indigestion,
causing an infinite loop that surely ended in SIGSEGV.  We fixed
some nearby problems awhile ago (cf 757c5182f) but missed this.

Report and diagnosis by Alexander Lakhin (bug #18273); patch by me

Discussion: https://postgr.es/m/18273-9a832d1da122600c@postgresql.org

17 months agoFix oversized memory allocation in Parallel Hash Join
Alexander Korotkov [Sun, 7 Jan 2024 07:03:55 +0000 (09:03 +0200)]
Fix oversized memory allocation in Parallel Hash Join

During the calculations of the maximum for the number of buckets, take into
account that later we round that to the next power of 2.

Reported-by: Karen Talarico
Bug: #16925
Discussion: https://postgr.es/m/16925-ec96d83529d0d629%40postgresql.org
Author: Thomas Munro, Andrei Lepikhov, Alexander Korotkov
Reviewed-by: Alena Rybakina
Backpatch-through: 12

17 months agoFix corruption of local buffer state during extend of temp relation
Michael Paquier [Fri, 5 Jan 2024 11:10:46 +0000 (20:10 +0900)]
Fix corruption of local buffer state during extend of temp relation

A typo has been introduced by 31966b151e6a when updating the state of a
local buffer when a temporary relation is extended, for the case of a
block included in the relation range extended, when it is already found
in the hash table holding the local buffers.  In this case, BM_VALID
should be cleared, but the buffer state was changed so as BM_VALID
remained while clearing the other flags.

As reported on the thread, it was possible to corrupt the state of the
local buffers on ENOSPC, but the states would be corrupted on any kind
of ERROR during the relation extend (like partial writes or some other
errno).

Reported-by: Alexander Lakhin
Author: Tender Wang
Reviewed-by: Richard Guo, Alexander Lakhin, Michael Paquier
Discussion: https://postgr.es/m/18259-6e256429825dd435@postgresql.org
Backpatch-through: 16

17 months agoFix use of incorrect TupleTableSlot in DISTINCT aggregates
David Rowley [Thu, 4 Jan 2024 07:40:11 +0000 (20:40 +1300)]
Fix use of incorrect TupleTableSlot in DISTINCT aggregates

1349d2790 added code to allow DISTINCT and ORDER BY aggregates to work
more efficiently by using presorted input.  That commit added some code
that made use of the AggState's tmpcontext and adjusted the
ecxt_outertuple and ecxt_innertuple slots before checking if the current
row is distinct from the previously seen row.  That code forgot to set the
TupleTableSlots back to what they were originally, which could result in
errors such as:

ERROR:  attribute 1 of type record has wrong type

This only affects aggregate functions which have multiple arguments when
DISTINCT is used.  For example: string_agg(DISTINCT col, ', ')

Thanks to Tom Lane for identifying the breaking commit.

Bug: #18264
Reported-by: Vojtěch Beneš
Discussion: https://postgr.es/m/18264-e363593d7e9feb7d@postgresql.org
Backpatch-through: 16, where 1349d2790 was added

17 months agoUpdate copyright for 2024
Bruce Momjian [Thu, 4 Jan 2024 01:49:04 +0000 (20:49 -0500)]
Update copyright for 2024

Reported-by: Michael Paquier
Discussion: https://postgr.es/m/[email protected]

Backpatch-through: 12

17 months agoAvoid masking EOF (no-password-supplied) conditions in auth.c.
Tom Lane [Wed, 3 Jan 2024 22:40:38 +0000 (17:40 -0500)]
Avoid masking EOF (no-password-supplied) conditions in auth.c.

CheckPWChallengeAuth() would return STATUS_ERROR if the user does not
exist or has no password assigned, even if the client disconnected
without responding to the password challenge (as libpq often will,
for example).  We should return STATUS_EOF in that case, and the
lower-level functions do, but this code level got it wrong since the
refactoring done in 7ac955b34.  This breaks the intent of not logging
anything for EOF cases (cf. comments in auth_failed()) and might
also confuse users of ClientAuthentication_hook.

Per report from Liu Lang.  Back-patch to all supported versions.

Discussion: https://postgr.es/m/b725238c-539d-cb09-2bff-b5e6cb2c069c@esgyn.cn

17 months agoDoc: Python's control flow construct is try/except not try/catch.
Tom Lane [Wed, 3 Jan 2024 17:22:00 +0000 (12:22 -0500)]
Doc: Python's control flow construct is try/except not try/catch.

Very ancient thinko, dating evidently to 22690719e.
Spotted by gweatherby.

Discussion: https://postgr.es/m/170423637139.1288848.11840082988774620003@wrigleys.postgresql.org

17 months agoIn pg_dump, don't dump a stats object unless dumping underlying table.
Tom Lane [Fri, 29 Dec 2023 15:57:11 +0000 (10:57 -0500)]
In pg_dump, don't dump a stats object unless dumping underlying table.

If the underlying table isn't being dumped, it's useless to dump
an extended statistics object; it'll just cause errors at restore.
We have always applied similar policies to, say, indexes.

(When and if we get cross-table stats objects, it might be profitable
to think a little harder about what to do with them.  But for now
there seems no point in considering a stats object as anything but
an appendage of its table.)

Rian McGuire and Tom Lane, per report from Rian McGuire.
Back-patch to supported branches.

Discussion: https://postgr.es/m/7075d3aa-3f05-44a5-b68f-47dc6a8a0550@buildkite.com

17 months agodoc: Mention AttributeRelationId in FDW validator function description
Michael Paquier [Thu, 28 Dec 2023 11:09:26 +0000 (20:09 +0900)]
doc: Mention AttributeRelationId in FDW validator function description

The documentation has been missing one value in the list of catalog OIDs
that can be given to the validator function of a FDW, as of
AttributeRelationId, when changing the attribute options of a foreign
table.

Author: Ian Lawrence Barwick
Discussion: https://postgr.es/m/CAB8KJ=i16t2yJU_Pq2Z+hnNGWFhagp_bJmzxHZu3ZkOjZm-+rQ@mail.gmail.com
Backpatch-through: 12

17 months agoDoc: specify aclitem syntax more clearly.
Tom Lane [Wed, 27 Dec 2023 18:52:01 +0000 (13:52 -0500)]
Doc: specify aclitem syntax more clearly.

The previous wording here relied solely on an example to explain
aclitem output format.  Add an actual syntax synopsis and
explanation of the elements to make it clearer.

David Johnston and Tom Lane, per gripe from Eugen Konkov.

Discussion: https://postgr.es/m/170326116972.1876499.18357820037829248593@wrigleys.postgresql.org

17 months agoFix failure to verify PGC_[SU_]BACKEND GUCs in pg_file_settings view.
Tom Lane [Tue, 26 Dec 2023 22:57:48 +0000 (17:57 -0500)]
Fix failure to verify PGC_[SU_]BACKEND GUCs in pg_file_settings view.

set_config_option() bails out early if it detects that the option to
be set is PGC_BACKEND or PGC_SU_BACKEND class and we're reading the
config file in a postmaster child; we don't want to apply any new
value in such a case.  That's fine as far as it goes, but it fails
to consider the requirements of the pg_file_settings view: for that,
we need to check validity of the value even though we have no
intention to apply it.  Because we didn't, even very silly values
for affected GUCs would be reported as valid by the view.  There
are only half a dozen such GUCs, which perhaps explains why this
got overlooked for so long.

Fix by continuing when changeVal is false; this parallels the logic
in some other early-exit paths.

Also, the check added by commit 924bcf4f1 to prevent GUC changes in
parallel workers seems a few bricks shy of a load: it's evidently
assuming that ereport(elevel, ...) won't return.  Make sure we
bail out if it does.  The lack of trouble reports suggests that
this is only a latent bug, i.e. parallel workers don't actually
reach here with elevel < ERROR.  (Per the code coverage report,
we never reach here at all in the regression suite.)  But we clearly
don't want to risk proceeding if that does happen.

Per report from Rıdvan Korkmaz.  These are ancient bugs, so back-patch
to all supported branches.

Discussion: https://postgr.es/m/2089235.1703617353@sss.pgh.pa.us

17 months agoFix mistaken file name in plpython's meson recipe.
Tom Lane [Tue, 26 Dec 2023 22:03:02 +0000 (17:03 -0500)]
Fix mistaken file name in plpython's meson recipe.

Brown-paper-bag bug in commit 58c3151bb.  Per buildfarm.

17 months agoHide warnings from Python headers when using gcc-compatible compiler.
Tom Lane [Tue, 26 Dec 2023 21:16:29 +0000 (16:16 -0500)]
Hide warnings from Python headers when using gcc-compatible compiler.

Like commit 388e80132, use "#pragma GCC system_header" to silence
warnings appearing within the Python headers, since newer Python
versions no longer worry about some restrictions we still use like
-Wdeclaration-after-statement.

This patch improves on 388e80132 by inventing a separate wrapper
header file, allowing the pragma to be tightly scoped to just
the Python headers and not other stuff we have laying about in
plpython.h.  I applied the same technique to plperl for the same
reason: the original patch suppressed warnings for a good deal
of our own code, not only the Perl headers.

Like the previous commit, back-patch to supported branches.

Peter Eisentraut and Tom Lane

Discussion: https://postgr.es/m/ae523163-6d2a-4b81-a875-832e48dec502@eisentraut.org

17 months agoDoc: Add missing pgoutput options.
Amit Kapila [Tue, 26 Dec 2023 05:48:05 +0000 (11:18 +0530)]
Doc: Add missing pgoutput options.

We forgot to update the docs while adding new options in pgoutput.

Author: Emre Hasegeli
Reviewed-by: Peter Smith, Amit Kapila
Backpatch-through: 12
Discussion: https://postgr.es/m/CAE2gYzwdwtUbs-tPSV-QBwgTubiyGD2ZGsSnAVsDfAGGLDrGOA%40mail.gmail.com

17 months agoSet readline-relevant ENV vars in interactive_psql(), not caller.
Tom Lane [Sat, 23 Dec 2023 16:50:33 +0000 (11:50 -0500)]
Set readline-relevant ENV vars in interactive_psql(), not caller.

Commit 664d75753 pulled 010_tab_completion.pl's infrastructure for
invoking an interactive psql session out into a generally-useful test
function, but it didn't move enough stuff.  We need to set up various
environment variables that readline will look at, both to ensure
stability of test results and to prevent test actions from cluttering
the calling user's ~/.psql_history.  Expecting calling scripts to
remember to do that is too failure-prone: the other existing caller
001_password.pl did not do it.  Hence, remove those initialization
steps from 010_tab_completion.pl and put them into interactive_psql().
Since interactive_psql was already making a local ENV hash, this has
no effect on calling scripts.

Discussion: https://postgr.es/m/794610.1703182896@sss.pgh.pa.us

17 months agoAvoid trying to fetch metapage of an SPGist partitioned index.
Tom Lane [Thu, 21 Dec 2023 17:43:36 +0000 (12:43 -0500)]
Avoid trying to fetch metapage of an SPGist partitioned index.

This is necessary when spgcanreturn() is invoked on a partitioned
index, and the failure might be reachable in other scenarios as
well.  The rest of what spgGetCache() does is perfectly sensible
for a partitioned index, so we should allow it to go through.

I think the main takeaway from this is that we lack sufficient test
coverage for non-btree partitioned indexes.  Therefore, I added
simple test cases for brin and gin as well as spgist (hash and
gist AMs were covered already in indexing.sql).

Per bug #18256 from Alexander Lakhin.  Although the known test case
only fails since v16 (3c569049b), I've got no faith at all that there
aren't other ways to reach this problem; so back-patch to all
supported branches.

Discussion: https://postgr.es/m/18256-0b0e1b6e4a620f1b@postgresql.org

17 months agoFix BEFORE ROW trigger handling in cross-partition MERGE update.
Dean Rasheed [Thu, 21 Dec 2023 12:53:48 +0000 (12:53 +0000)]
Fix BEFORE ROW trigger handling in cross-partition MERGE update.

Fix a bug during MERGE if a cross-partition update is attempted on a
partitioned table with a BEFORE DELETE ROW trigger that returns NULL,
to prevent the update. This would cause an error to be thrown, or an
assert failure in an assert-enabled build.

This was an oversight in 9321c79c86, which failed to properly
distinguish a DELETE prevented by a trigger from one prevented by a
concurrent update. Fix by having ExecDelete() return the TM_Result
status to ExecCrossPartitionUpdate(), so that it can distinguish the
two cases, and make ExecCrossPartitionUpdate() return the TM_Result
status to ExecUpdateAct(), so that it can return the correct status
from a concurrent update.

In addition, ensure that the command tag is correctly updated by
having ExecMergeMatched() pass canSetTag to ExecUpdateAct(), rather
than passing false, so that it updates the command tag if it does a
cross-partition update, making this code path in ExecMergeMatched()
consistent with ExecUpdate().

Per bug #18238 from Alexander Lakhin. Back-patch to v15, where MERGE
was introduced.

Dean Rasheed, reviewed by Richard Guo and Jian He.

Discussion: https://postgr.es/m/18238-2f2bdc7f720180b9%40postgresql.org

17 months agoFix unchecked return value from strdup
Daniel Gustafsson [Wed, 20 Dec 2023 21:37:28 +0000 (22:37 +0100)]
Fix unchecked return value from strdup

The pg_dump compression was using strdup() instead of pg_strdup()
and failed to check the returned pointer for out-of-memory before
dereferencing it. Fix by using pg_strdup() instead which probably
was the intention here in the original patch.

Backpatch to v16 where pg_dump compression was introduced.

Reviewed-by: Tristan Partin
Reviewed-by: Nathan Bossart
Discussion: https://postgr.es/m/CC661D60-6F4C-474D-B9CF-E789ACA3CEFC@yesql.se
Backpatch-through: 16