postgresql.git
18 months agoLog LLVM library version in configure output.
Thomas Munro [Sun, 22 Oct 2023 01:17:00 +0000 (14:17 +1300)]
Log LLVM library version in configure output.

When scanning build farm results, it's useful to be able to see which
version is in use.  For the Meson build system, this information was
already displayed.

Back-patch to all supported branches.

Discussion: https://postgr.es/m/4022690.1697852728%40sss.pgh.pa.us

18 months agoDodge a compiler bug affecting timetz_zone/timetz_izone.
Tom Lane [Fri, 20 Oct 2023 17:40:15 +0000 (13:40 -0400)]
Dodge a compiler bug affecting timetz_zone/timetz_izone.

This avoids a compiler bug occurring in AIX's xlc, even in pretty
late-model revisions.  Buildfarm testing has now confirmed that
only 64-bit xlc is affected.  Although we are contemplating
dropping support for xlc in v17, it's still supported in the
back branches, so we need this fix.

Back-patch of code changes from HEAD commit 19fa97731.
(The test cases were already back-patched, in 4a427b82c et al.)

Discussion: https://postgr.es/m/CA+hUKGK=DOC+hE-62FKfZy=Ybt5uLkrg3zCZD-jFykM-iPn8yw@mail.gmail.com

18 months agoDoc: update CREATE OPERATOR's statement about => as an operator.
Tom Lane [Fri, 20 Oct 2023 17:01:02 +0000 (13:01 -0400)]
Doc: update CREATE OPERATOR's statement about => as an operator.

This doco said that use of => as an operator "is deprecated".
It's been fully disallowed since 865f14a2d back in 9.5, but
evidently that commit missed updating this statement.
Do so now.

18 months agoImprove pglz_decompress's defenses against corrupt compressed data.
Tom Lane [Thu, 19 Oct 2023 00:43:17 +0000 (20:43 -0400)]
Improve pglz_decompress's defenses against corrupt compressed data.

When processing a match tag, check to see if the claimed "off"
is more than the distance back to the output buffer start.
If it is, then the data is corrupt, and what's more we would
fetch from outside the buffer boundaries and potentially incur
a SIGSEGV.  (Although the odds of that seem relatively low, given
that "off" can't be more than 4K.)

Back-patch to v13; before that, this function wasn't really
trying to protect against bad data.

Report and fix by Flavien Guedez.

Discussion: https://postgr.es/m/01fc0593-e31e-463d-902c-dd43174acee2@oopacity.net

18 months agojit: Changes for LLVM 17.
Thomas Munro [Wed, 18 Oct 2023 09:15:54 +0000 (22:15 +1300)]
jit: Changes for LLVM 17.

Changes required by https://llvm.org/docs/NewPassManager.html.

Back-patch to 12, leaving the final release of 11 unchanged, consistent
with earlier decision not to back-patch LLVM 16 support either.

Author: Dmitry Dolgov <9erthalion6@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKG%2BWXznXCyTgCADd%3DHWkP9Qksa6chd7L%3DGCnZo-MBgg9Lg%40mail.gmail.com

18 months agojit: Supply LLVMGlobalGetValueType() for LLVM < 8.
Thomas Munro [Wed, 18 Oct 2023 14:01:55 +0000 (03:01 +1300)]
jit: Supply LLVMGlobalGetValueType() for LLVM < 8.

Commit 37d5babb used this C API function while adding support for LLVM
16 and opaque pointers, but it's not available in LLVM 7 and older.
Provide it in our own llvmjit_wrap.cpp.  It just calls a C++ function
that pre-dates LLVM 3.9, our minimum target.

Back-patch to 12, like 37d5babb.

Discussion: https://postgr.es/m/CA%2BhUKGKnLnJnWrkr%3D4mSGhE5FuTK55FY15uULR7%3Dzzc%3DwX4Nqw%40mail.gmail.com

18 months agojit: Support opaque pointers in LLVM 16.
Thomas Munro [Wed, 18 Oct 2023 09:09:05 +0000 (22:09 +1300)]
jit: Support opaque pointers in LLVM 16.

Remove use of LLVMGetElementType() and provide the type of all pointers
to LLVMBuildXXX() functions when emitting IR, as required by modern LLVM
versions[1].

 * For LLVM <= 14, we'll still use the old LLVMBuildXXX() functions.
 * For LLVM == 15, we'll continue to do the same, explicitly opting
   out of opaque pointer mode.
 * For LLVM >= 16, we'll use the new LLVMBuildXXX2() functions that take
   the extra type argument.

The difference is hidden behind some new IR emitting wrapper functions
l_load(), l_gep(), l_call() etc.  The change is mostly mechanical,
except that at each site the correct type had to be provided.

In some places we needed to do some extra work to get functions types,
including some new wrappers for C++ APIs that are not yet exposed by in
LLVM's C API, and some new "example" functions in llvmjit_types.c
because it's no longer possible to start from the function pointer type
and ask for the function type.

Back-patch to 12, because it's a little tricker in 11 and we agreed not
to put the latest LLVM support into the upcoming final release of 11.

[1] https://llvm.org/docs/OpaquePointers.html

Reviewed-by: Dmitry Dolgov <9erthalion6@gmail.com>
Reviewed-by: Ronan Dunklau <ronan.dunklau@aiven.io>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA%2BhUKGKNX_%3Df%2B1C4r06WETKTq0G4Z_7q4L4Fxn5WWpMycDj9Fw%40mail.gmail.com

18 months agowindows: msvc: Define STDIN/OUT/ERR_FILENO.
Nathan Bossart [Tue, 17 Oct 2023 21:11:08 +0000 (16:11 -0500)]
windows: msvc: Define STDIN/OUT/ERR_FILENO.

This commit (c290e79cf0) was originally back-patched to v15.
Commit 97550c0711 added a new use of STDERR_FILENO, and it was
back-patched all the way to v11, thus breaking MSVC builds for v11
through v14.  Since STDERR_FILENO is now needed on older versions,
let's back-patch c290e79cf0 down to v11, too.

Here follows the original commit message describing the change:

Because they are not available we've used _fileno(stdin) in some places, but
that doesn't reliably work, because stdin might be closed. This is the
prerequisite of the subsequent commit, fixing a failure introduced in
76e38b37a5.

Author: Andres Freund <andres@anarazel.de>
Reported-By: Sandeep Thakkar <sandeep.thakkar@enterprisedb.com>
Message-Id: 20220520164558.ozb7lm6unakqzezi@alap3.anarazel.de (on pgsql-packagers)
Discussion: https://postgr.es/m/20231017164517.GA613565%40nathanxps13
Backpatch-through: 11

18 months agoBack-patch test cases for timetz_zone/timetz_izone.
Tom Lane [Tue, 17 Oct 2023 17:55:45 +0000 (13:55 -0400)]
Back-patch test cases for timetz_zone/timetz_izone.

Per code coverage reports, we had zero regression test coverage
of these functions.  That came back to bite us, as apparently
that's allowed us to miss discovering misbehavior of this code
with AIX's xlc compiler.  Install relevant portions of the
test cases added in 97957fdba2f047203019fa97731.

(Assuming the expected outcome that the xlc problem does appear
in back branches, a code fix will follow.)

Discussion: https://postgr.es/m/CA+hUKGK=DOC+hE-62FKfZy=Ybt5uLkrg3zCZD-jFykM-iPn8yw@mail.gmail.com

18 months agoAvoid calling proc_exit() in processes forked by system().
Nathan Bossart [Tue, 17 Oct 2023 15:42:17 +0000 (10:42 -0500)]
Avoid calling proc_exit() in processes forked by system().

The SIGTERM handler for the startup process immediately calls
proc_exit() for the duration of the restore_command, i.e., a call
to system().  This system() call forks a new process to execute the
shell command, and this child process inherits the parent's signal
handlers.  If both the parent and child processes receive SIGTERM,
both will attempt to call proc_exit().  This can end badly.  For
example, both processes will try to remove themselves from the
PGPROC shared array.

To fix this problem, this commit adds a check in
StartupProcShutdownHandler() to see whether MyProcPid == getpid().
If they match, this is the parent process, and we can proc_exit()
like before.  If they do not match, this is a child process, and we
just emit a message to STDERR (in a signal safe manner) and
_exit(), thereby skipping any problematic exit callbacks.

This commit also adds checks in proc_exit(), ProcKill(), and
AuxiliaryProcKill() that verify they are not being called within
such child processes.

Suggested-by: Andres Freund
Reviewed-by: Thomas Munro, Andres Freund
Discussion: https://postgr.es/m/Y9nGDSgIm83FHcad%40paquier.xyz
Discussion: https://postgr.es/m/20230223231503.GA743455%40nathanxps13
Backpatch-through: 11

18 months agoEnsure we have a snapshot while dropping ON COMMIT DROP temp tables.
Tom Lane [Mon, 16 Oct 2023 18:06:11 +0000 (14:06 -0400)]
Ensure we have a snapshot while dropping ON COMMIT DROP temp tables.

Dropping a temp table could entail TOAST table access to clean out
toasted catalog entries, such as large pg_constraint.conbin strings
for complex CHECK constraints.  If we did that via ON COMMIT DROP,
we triggered the assertion in init_toast_snapshot(), because
there was no provision for setting up a snapshot for the drop
actions.  Fix that.

(I assume here that the adjacent truncation actions for ON COMMIT
DELETE ROWS don't have a similar problem: it doesn't seem like
nontransactional truncations would need to touch any toasted fields.
If that proves wrong, we could refactor a bit to have the same
snapshot acquisition cover that too.)

The test case added here does not fail before v15, because that
assertion was added in 277692220 which was not back-patched.
However, the race condition the assertion warns of surely
exists further back, so back-patch to all supported branches.

Per report from Richard Guo.

Discussion: https://postgr.es/m/CAMbWs4-x26=_QxxgdJyNbiCDzvtr2WV5ZDso_v-CukKEe6cBZw@mail.gmail.com

18 months agoTry to handle torn reads of pg_control in frontend.
Thomas Munro [Mon, 16 Oct 2023 04:10:13 +0000 (17:10 +1300)]
Try to handle torn reads of pg_control in frontend.

Some of our src/bin tools read the control file without any kind of
interlocking against concurrent writes from the server.  At least ext4
and ntfs can expose partially modified contents when you do that.

For now, we'll try to tolerate this by retrying up to 10 times if the
checksum doesn't match, until we get two reads in a row with the same
bad checksum.  This is not guaranteed to reach the right conclusion, but
it seems very likely to.  Thanks to Tom Lane for this suggestion.

Various ideas for interlocking or atomicity were considered too
complicated, unportable or expensive given the lack of field reports,
but remain open for future reconsideration.

Back-patch as far as 12.  It doesn't seem like a good idea to put a
heuristic change for a very rare problem into the final release of 11.

Reviewed-by: Anton A. Melnikov <aamelnikov@inbox.ru>
Reviewed-by: David Steele <david@pgmasters.net>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/20221123014224.xisi44byq3cf5psi%40awork3.anarazel.de

18 months agoAcquire ControlFileLock in relevant SQL functions.
Thomas Munro [Sun, 15 Oct 2023 21:43:47 +0000 (10:43 +1300)]
Acquire ControlFileLock in relevant SQL functions.

Commit dc7d70ea added functions that read the control file, but didn't
acquire ControlFileLock.  With unlucky timing, file systems that have
weak interlocking like ext4 and ntfs could expose partially overwritten
contents, and the checksum would fail.

Back-patch to all supported releases.

Reviewed-by: David Steele <david@pgmasters.net>
Reviewed-by: Anton A. Melnikov <aamelnikov@inbox.ru>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/20221123014224.xisi44byq3cf5psi%40awork3.anarazel.de

18 months agoDissociate btequalimage() from interval_ops, ending its deduplication.
Noah Misch [Sat, 14 Oct 2023 23:33:51 +0000 (16:33 -0700)]
Dissociate btequalimage() from interval_ops, ending its deduplication.

Under interval_ops, some equal values are distinguishable.  One such
pair is '24:00:00' and '1 day'.  With that being so, btequalimage()
breaches the documented contract for the "equalimage" btree support
function.  This can cause incorrect results from index-only scans.
Users should REINDEX any btree indexes having interval-type columns.
After updating, pg_amcheck will report an error for almost all such
indexes.  This fix makes interval_ops simply omit the support function,
like numeric_ops does.  Back-pack to v13, where btequalimage() first
appeared.  In back branches, for the benefit of old catalog content,
btequalimage() code will return false for type "interval".  Going
forward, back-branch initdb will include the catalog change.

Reviewed by Peter Geoghegan.

Discussion: https://postgr.es/m/20231011013317.22.nmisch@google.com

18 months agoDon't spuriously report FD_SETSIZE exhaustion on Windows.
Noah Misch [Sat, 14 Oct 2023 22:54:46 +0000 (15:54 -0700)]
Don't spuriously report FD_SETSIZE exhaustion on Windows.

Starting on 2023-08-03, this intermittently terminated a "pgbench -C"
test in CI.  It could affect a high-client-count "pgbench" without "-C".
While parallel reindexdb and vacuumdb reach the same problematic check,
sufficient client count and/or connection turnover is less plausible for
them.  Given the lack of examples from the buildfarm or from manual
builds, reproducing this must entail rare operating system
configurations.  Also correct the associated error message, which was
wrong for non-Windows.  Back-patch to v12, where the pgbench check first
appeared.  While v11 vacuumdb has the problematic check, reaching it
with typical vacuumdb usage is implausible.

Reviewed by Thomas Munro.

Discussion: https://postgr.es/m/CA+hUKG+JwvTNdcyJTriy9BbtzF1veSRQ=9M_ZKFn9_LqE7Kp7Q@mail.gmail.com

18 months agoFix runtime partition pruning for HASH partitioned tables
David Rowley [Thu, 12 Oct 2023 12:14:22 +0000 (01:14 +1300)]
Fix runtime partition pruning for HASH partitioned tables

This could only affect HASH partitioned tables with at least 2 partition
key columns.

If partition pruning was delayed until execution and the query contained
an IS NULL qual on one of the partitioned keys, and some subsequent
partitioned key was being compared to a non-Const, then this could result
in a crash due to the incorrect keyno being used to calculate the
stateidx for the expression evaluation code.

Here we fix this by properly skipping partitioned keys which have a
nullkey set.  Effectively, this must be the same as what's going on
inside perform_pruning_base_step().

Sergei Glukhov also provided a patch, but that's not what's being used
here.

Reported-by: Sergei Glukhov
Reviewed-by: tender wang, Sergei Glukhov
Discussion: https://postgr.es/m/d05b26fa-af54-27e1-f693-6c31590802fa@postgrespro.ru
Backpatch-through: 11, where runtime partition pruning was added.

18 months agoDoc: fix grammatical errors for enable_partitionwise_aggregate
David Rowley [Thu, 12 Oct 2023 08:17:04 +0000 (21:17 +1300)]
Doc: fix grammatical errors for enable_partitionwise_aggregate

Author: Andrew Atkinson
Reviewed-by: Ashutosh Bapat
Discussion: https://postgr.es/m/CAG6XLEnC%3DEgq0YHRic2kWWDs4xwQnQ_kBA6qhhzAq1-pO_9Tfw%40mail.gmail.com
Backpatch-through: 11, where enable_partitionwise_aggregate was added

18 months agoFix incorrect step generation in HASH partition pruning
David Rowley [Thu, 12 Oct 2023 06:52:58 +0000 (19:52 +1300)]
Fix incorrect step generation in HASH partition pruning

get_steps_using_prefix_recurse() incorrectly assumed that it could stop
recursive processing of the 'prefix' list when cur_keyno was one before
the step_lastkeyno.  Since hash partition pruning can prune using IS
NULL quals, and these IS NULL quals are not present in the 'prefix'
list, then that logic could cause more levels of recursion than what is
needed and lead to there being no more items in the 'prefix' list to
process.  This would manifest itself as a crash in some code that
expected the 'start' ListCell not to be NULL.

Here we adjust the logic so that instead of stopping recursion at 1 key
before the step_lastkeyno, we just look at the llast(prefix) item and
ensure we only recursively process up until just before whichever the last
key is.  This effectively allows keys to be missing in the 'prefix' list.

This change does mean that step_lastkeyno is no longer needed, so we
remove that from the static functions.  I also spent quite some time
reading this code and testing it to try to convince myself that there
are no other issues.  That resulted in the irresistible temptation of
rewriting some comments, many of which were just not true or inconcise.

Reported-by: Sergei Glukhov
Reviewed-by: Sergei Glukhov, tender wang
Discussion: https://postgr.es/m/2f09ce72-315e-2a33-589a-8519ada8df61@postgrespro.ru
Backpatch-through: 11, where partition pruning was introduced.

18 months agodoc: clarify that SSPI and GSSAPI are interchangeable
Bruce Momjian [Tue, 10 Oct 2023 20:51:08 +0000 (16:51 -0400)]
doc:  clarify that SSPI and GSSAPI are interchangeable

Reported-by: tpo_deb@sourcepole.ch
Discussion: https://postgr.es/m/167846222574.1803490.15815104179136215862@wrigleys.postgresql.org

Backpatch-through: 11

18 months agodoc: foreign servers with pushdown need matching collation
Bruce Momjian [Tue, 10 Oct 2023 20:04:56 +0000 (16:04 -0400)]
doc:  foreign servers with pushdown need matching collation

Reported-by: Pete Storer
Discussion: https://postgr.es/m/BL0PR05MB66283C57D72E321591AE4EB1F3CE9@BL0PR05MB6628.namprd05.prod.outlook.com

Backpatch-through: 11

18 months agodoc: add SSL configuration section reference
Bruce Momjian [Tue, 10 Oct 2023 19:54:28 +0000 (15:54 -0400)]
doc:  add SSL configuration section reference

Reported-by: Steve Atkins
Discussion: https://postgr.es/m/B82E80DD-1452-4175-B19C-564FE46705BA@blighty.com

Backpatch-through: 11

18 months agodoc: document the need to analyze partitioned tables
Bruce Momjian [Tue, 10 Oct 2023 19:14:18 +0000 (15:14 -0400)]
doc:  document the need to analyze partitioned tables

Autovacuum does not do it.

Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/20210913035409.GA10647@telsasoft.com

Backpatch-through: 11

18 months agoFix bug in GenericXLogFinish().
Jeff Davis [Tue, 10 Oct 2023 18:01:13 +0000 (11:01 -0700)]
Fix bug in GenericXLogFinish().

Mark the buffers dirty before writing WAL.

Discussion: https://postgr.es/m/25104133-7df8-cae3-b9a2-1c0aaa1c094a@iki.fi
Reviewed-by: Heikki Linnakangas
Backpatch-through: 11

18 months agoDoc: use CURRENT_USER not USER in plpgsql trigger examples.
Tom Lane [Mon, 9 Oct 2023 15:29:21 +0000 (11:29 -0400)]
Doc: use CURRENT_USER not USER in plpgsql trigger examples.

While these two built-in functions do exactly the same thing,
CURRENT_USER seems preferable to use in documentation examples.
It's easier to look up if the reader is unsure what it is.
Also, this puts these examples in sync with an adjacent example
that already used CURRENT_USER.

Per question from Kirk Parker.

Discussion: https://postgr.es/m/CANwZ8rmN_Eb0h0hoMRS8Feftaik0z89PxVsKg+cP+PctuOq=Qg@mail.gmail.com

18 months agoAvoid memory size overflow when allocating backend activity buffer
Michael Paquier [Tue, 3 Oct 2023 06:37:23 +0000 (15:37 +0900)]
Avoid memory size overflow when allocating backend activity buffer

The code in charge of copying the contents of PgBackendStatus to local
memory could fail on memory allocation because of an overflow on the
amount of memory to use.  The overflow can happen when combining a high
value track_activity_query_size (max at 1MB) with a large
max_connections, when both multiplied get higher than INT32_MAX as both
parameters treated as signed integers.  This could for example trigger
with the following functions, all calling pgstat_read_current_status():
- pg_stat_get_backend_subxact()
- pg_stat_get_backend_idset()
- pg_stat_get_progress_info()
- pg_stat_get_activity()
- pg_stat_get_db_numbackends()

The change to use MemoryContextAllocHuge() has been introduced in
8d0ddccec636, so backpatch down to 12.

Author: Jakub Wartak
Discussion: https://postgr.es/m/CAKZiRmw8QSNVw2qNK-dznsatQqz+9DkCquxP0GHbbv1jMkGHMA@mail.gmail.com
Backpatch-through: 12

19 months agoFail hard on out-of-memory failures in xlogreader.c
Michael Paquier [Tue, 3 Oct 2023 01:25:17 +0000 (10:25 +0900)]
Fail hard on out-of-memory failures in xlogreader.c

This commit changes the WAL reader routines so as a FATAL for the
backend or exit(FAILURE) for the frontend is triggered if an allocation
for a WAL record decode fails in walreader.c, rather than treating this
case as bogus data, which would be equivalent to the end of WAL.  The
key is to avoid palloc_extended(MCXT_ALLOC_NO_OOM) in walreader.c,
relying on plain palloc() calls.

The previous behavior could make WAL replay finish too early than it
should.  For example, crash recovery finishing earlier may corrupt
clusters because not all the WAL available locally was replayed to
ensure a consistent state.  Out-of-memory failures would show up
randomly depending on the memory pressure on the host, but one simple
case would be to generate a large record, then replay this record after
downsizing a host, as Ethan Mertz originally reported.

This relies on bae868caf222, as the WAL reader routines now do the
memory allocation required for a record only once its header has been
fully read and validated, making xl_tot_len trustable.  Making the WAL
reader react differently on out-of-memory or bogus record data would
require ABI changes, so this is the safest choice for stable branches.
Also, it is worth noting that 3f1ce973467a has been using a plain
palloc() in this code for some time now.

Thanks to Noah Misch and Thomas Munro for the discussion.

Like the other commit, backpatch down to 12, leaving out v11 that will
be EOL'd soon.  The behavior of considering a failed allocation as bogus
data comes originally from 0ffe11abd3a0, where the record length
retrieved from its header was not entirely trustable.

Reported-by: Ethan Mertz
Discussion: https://postgr.es/m/ZRKKdI5-RRlta3aF@paquier.xyz
Backpatch-through: 12

19 months agoFix omission of column-level privileges in selective pg_restore.
Tom Lane [Mon, 2 Oct 2023 17:27:51 +0000 (13:27 -0400)]
Fix omission of column-level privileges in selective pg_restore.

In a selective restore, ACLs for a table should be dumped if the
table is selected to be dumped.  However, if the table has both
table-level and column-level ACLs, only the table-level ACL was
restored.  This happened because _tocEntryRequired assumed that
an ACL could have only one dependency (the one on its table),
and punted if there was more than one.  But since commit ea9125304,
column-level ACLs also depend on the table-level ACL if any, to
ensure correct ordering in parallel restores.  To fix, adjust the
logic in _tocEntryRequired to ignore dependencies on ACLs.

I extended a test case in 002_pg_dump.pl so that it purports to
test for this; but in fact the test passes even without the fix.
That's because this bug only manifests during a selective restore,
while the scenarios 002_pg_dump.pl tests include only selective dumps.
Perhaps somebody would like to extend the script so that it can test
scenarios including selective restore, but I'm not touching that.

Euler Taveira and Tom Lane, per report from Kong Man.
Back-patch to all supported branches.

Discussion: https://postgr.es/m/DM4PR11MB73976902DBBA10B1D652F9498B06A@DM4PR11MB7397.namprd11.prod.outlook.com

19 months agoFix datalen calculation in tsvectorrecv().
Tom Lane [Sun, 1 Oct 2023 17:16:47 +0000 (13:16 -0400)]
Fix datalen calculation in tsvectorrecv().

After receiving position data for a lexeme, tsvectorrecv()
advanced its "datalen" value by (npos+1)*sizeof(WordEntry)
where the correct calculation is (npos+1)*sizeof(WordEntryPos).
This accidentally failed to render the constructed tsvector
invalid, but it did result in leaving some wasted space
approximately equal to the space consumed by the position data.
That could have several bad effects:

* Disk space is wasted if the received tsvector is stored into a
  table as-is.

* A legal tsvector could get rejected with "maximum total lexeme
  length exceeded" if the extra space pushes it over the MAXSTRPOS
  limit.

* In edge cases, the finished tsvector could be assigned a length
  larger than the allocated size of its palloc chunk, conceivably
  leading to SIGSEGV when the tsvector gets copied somewhere else.
  The odds of a field failure of this sort seem low, though valgrind
  testing could probably have found this.

While we're here, let's express the calculation as
"sizeof(uint16) + npos * sizeof(WordEntryPos)" to avoid the type
pun implicit in the "npos + 1" formulation.  It's not wrong
given that WordEntryPos had better be 2 bytes to avoid padding
problems, but it seems clearer this way.

Report and patch by Denis Erokhin.  Back-patch to all supported
versions.

Discussion: https://postgr.es/m/009801d9f2d9$f29730c0$d7c59240$@datagile.ru

19 months agoFix briefly showing old progress stats for ANALYZE on inherited tables.
Heikki Linnakangas [Sat, 30 Sep 2023 14:03:50 +0000 (17:03 +0300)]
Fix briefly showing old progress stats for ANALYZE on inherited tables.

ANALYZE on a table with inheritance children analyzes all the child
tables in a loop. When stepping to next child table, it updated the
child rel ID value in the command progress stats, but did not reset
the 'sample_blks_total' and 'sample_blks_scanned' counters.
acquire_sample_rows() updates 'sample_blks_total' as soon as the scan
starts and 'sample_blks_scanned' after processing the first block, but
until then, pg_stat_progress_analyze would display a bogus combination
of the new child table relid with old counter values from the
previously processed child table. Fix by resetting 'sample_blks_total'
and 'sample_blks_scanned' to zero at the same time that
'current_child_table_relid' is updated.

Backpatch to v13, where pg_stat_progress_analyze view was introduced.

Reported-by: Justin Pryzby
Discussion: https://www.postgresql.org/message-id/20230122162345.GP13860%40telsasoft.com

19 months agoRemove environment sensitivity in pl/tcl regression test.
Tom Lane [Sat, 30 Sep 2023 00:20:57 +0000 (20:20 -0400)]
Remove environment sensitivity in pl/tcl regression test.

Add "-gmt 1" to our test invocations of the Tcl "clock" command,
so that they do not consult the timezone environment.  While it
doesn't really matter which timezone is used here, it does
matter that the command not fall over entirely.  We've now
discovered that at least on FreeBSD, "clock scan" will fail if
/etc/localtime is missing.  It seems worth making the test
insensitive to that.

Per Tomas Vondras' buildfarm animal dikkop.  Thanks to
Thomas Munro for the diagnosis.

Discussion: https://postgr.es/m/316d304a-1dcd-cea1-3d6c-27f794727a06@enterprisedb.com

19 months agoSuppress macOS warnings about duplicate libraries in link commands.
Tom Lane [Fri, 29 Sep 2023 18:07:30 +0000 (14:07 -0400)]
Suppress macOS warnings about duplicate libraries in link commands.

As of Xcode 15 (macOS Sonoma), the linker complains about duplicate
references to the same library.  We see warnings about libpgport and
libpgcommon being duplicated in many client executables.  This is a
consequence of the hack introduced in commit 6b7ef076b to list
libpgport before libpq while not removing it from $(LIBS).
(Commit 8396447cd later applied the same rule to libpgcommon.)

The concern in 6b7ef076b was to ensure that the client executable
wouldn't unintentionally depend on pgport functions from libpq.
That concern is obsolete on any platform for which we can do symbol
export control, because if we can then the pgport functions in libpq
won't be exposed anyway.  Hence, we can fix this problem by just
removing libpgport and libpgcommon from $(libpq_pgport), and letting
clients depend on the occurrences in $(LIBS).

In the back branches, do that only on macOS (which we know has
symbol export control).  In HEAD, let's be more aggressive and
remove the extra libraries everywhere.  The only still-supported
platforms that lack export control are MinGW/Cygwin, and it
doesn't seem worth sweating over ABI stability details for those
(or if somebody does care, it'd probably be possible to perform
symbol export control for those too).  As well as being simpler,
this might give some microscopic improvement in build time.

The meson build system is not changed here, as it doesn't have
this particular disease, though it does have some related issues
that we'll fix separately.

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

19 months agoDoc: improve description of dump/restore's --clean and --if-exists.
Tom Lane [Fri, 29 Sep 2023 17:13:54 +0000 (13:13 -0400)]
Doc: improve description of dump/restore's --clean and --if-exists.

Try to make these option descriptions a little clearer for novices.
Per gripe from Attila Gulyás.

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

19 months agodoc: Change statistics function xref to the right target
Daniel Gustafsson [Fri, 29 Sep 2023 13:55:37 +0000 (15:55 +0200)]
doc: Change statistics function xref to the right target

Commit 7d3b7011b added a link to the statistics functions, which at the
time were anchored under the section for statistics views.  aebe989477a
added a separate section for statistics functions, but the link was not
updated to point to the new anchor.  Fix by changing the xref.

Backpatch to all supported branches.

Author: Peter Smith <peter.b.smith@fujitsu.com>
Discussion: https://postgr.es/m/CAHut+Ptr0jKzNNtWnssLq+3jNhbyaBseqf6NPrWHk08mQFRoTg@mail.gmail.com
Backpatch-through: 11

19 months agoFix btmarkpos/btrestrpos array key wraparound bug.
Peter Geoghegan [Thu, 28 Sep 2023 23:29:27 +0000 (16:29 -0700)]
Fix btmarkpos/btrestrpos array key wraparound bug.

nbtree's mark/restore processing failed to correctly handle an edge case
involving array key advancement and related search-type scan key state.
Scans with ScalarArrayScalarArrayOpExpr quals requiring mark/restore
processing (for a merge join) could incorrectly conclude that an
affected array/scan key must not have advanced during the time between
marking and restoring the scan's position.

As a result of all this, array key handling within btrestrpos could skip
a required call to _bt_preprocess_keys().  This confusion allowed later
primitive index scans to overlook tuples matching the true current array
keys.  The scan's search-type scan keys would still have spurious values
corresponding to the final array element(s) -- not values matching the
first/now-current array element(s).

To fix, remember that "array key wraparound" has taken place during the
ongoing btrescan in a flag variable stored in the scan's state, and use
that information at the point where btrestrpos decides if another call
to _bt_preprocess_keys is required.

Oversight in commit 70bc5833, which taught nbtree to handle array keys
during mark/restore processing, but missed this subtlety.  That commit
was itself a bug fix for an issue in commit 9e8da0f7, which taught
nbtree to handle ScalarArrayOpExpr quals natively.

Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-WzkgP3DDRJxw6DgjCxo-cu-DKrvjEv_ArkP2ctBJatDCYg@mail.gmail.com
Backpatch: 11- (all supported branches).

19 months agoFix checking of index expressions in CompareIndexInfo().
Tom Lane [Thu, 28 Sep 2023 18:05:25 +0000 (14:05 -0400)]
Fix checking of index expressions in CompareIndexInfo().

This code was sloppy about comparison of index columns that
are expressions.  It didn't reliably reject cases where one
index has an expression where the other has a plain column,
and it could index off the start of the attmap array, leading
to a Valgrind complaint (though an actual crash seems unlikely).

I'm not sure that the expression-vs-column sloppiness leads
to any visible problem in practice, because the subsequent
comparison of the two expression lists would reject cases
where the indexes have different numbers of expressions
overall.  Maybe we could falsely match indexes having the
same expressions in different column positions, but it'd
require unlucky contents of the word before the attmap array.
It's not too surprising that no problem has been reported
from the field.  Nonetheless, this code is clearly wrong.

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

Discussion: https://postgr.es/m/18135-532f4a755e71e4d2@postgresql.org

19 months agounaccent: Tweak value of PYTHON when building without Python support
Michael Paquier [Wed, 27 Sep 2023 05:41:26 +0000 (14:41 +0900)]
unaccent: Tweak value of PYTHON when building without Python support

As coded, the module's Makefile would fail to set a value for PYTHON as
it checked if the variable is defined.  When compiling without
--with-python, PYTHON is defined and set to an empty value, so the
existing check is not able to do its work.

This commit switches the rule to check if the value is empty rather than
defined, allowing the generation of unaccent.rules even if --with-python
is not used as long as "python" exists.  BISON and FLEX do the same in
pgxs.mk, for instance.

Thinko in f85a485f89e2.

Author: Japin Li
Discussion: https://postgr.es/m/MEYP282MB1669F86C0DC7B4DC48489CB0B6C3A@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
Backpatch-through: 13

19 months agoStop using "-multiply_defined suppress" on macOS.
Tom Lane [Wed, 27 Sep 2023 01:06:21 +0000 (21:06 -0400)]
Stop using "-multiply_defined suppress" on macOS.

We started to use this linker switch in commit 9df308697 of
2004-07-13, which was in the OS X 10.3 era.  Apparently it's been a
no-op since around OS X 10.9.  Apple's most recent toolchain version
actively complains about it, so it's time to get rid of it.

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

19 months agodoc: clarify the effect of concurrent work_mem allocations
Bruce Momjian [Tue, 26 Sep 2023 23:44:21 +0000 (19:44 -0400)]
doc: clarify the effect of concurrent work_mem allocations

Reported-by: Sami Imseih
Discussion: https://postgr.es/m/66590882-F48C-4A25-83E3-73792CF8C51F@amazon.com

Backpatch-through: 11

19 months agodoc: clarify handling of time zones with "time with time zone"
Bruce Momjian [Tue, 26 Sep 2023 23:23:59 +0000 (19:23 -0400)]
doc:  clarify handling of time zones with "time with time zone"

Reported-by: davecramer@postgres.rocks
Discussion: https://postgr.es/m/168451942371.714.9173574930845904336@wrigleys.postgresql.org

Backpatch-through: 11

19 months agodoc: clarify the behavior of unopenable listen_addresses
Bruce Momjian [Tue, 26 Sep 2023 23:02:18 +0000 (19:02 -0400)]
doc:  clarify the behavior of unopenable listen_addresses

Reported-by: Gurjeet Singh
Discussion: https://postgr.es/m/CABwTF4WYPD9ov-kcSq1+J+ZJ5wYDQLXquY6Lu2cvb-Y7pTpSGA@mail.gmail.com

Backpatch-through: 11

19 months agodoc: pg_upgrade, clarify standby servers must remain running
Bruce Momjian [Tue, 26 Sep 2023 22:54:10 +0000 (18:54 -0400)]
doc: pg_upgrade, clarify standby servers must remain running

Also mention that mismatching primary/standby LSNs should never
happen.

Reported-by: Nikolay Samokhvalov
Discussion: https://postgr.es/m/CAM527d8heqkjG5VrvjU3Xjsqxg41ufUyabD9QZccdAxnpbRH-Q@mail.gmail.com

Backpatch-through: 11

19 months agodoc: mention GROUP BY columns can reference target col numbers
Bruce Momjian [Tue, 26 Sep 2023 21:31:06 +0000 (17:31 -0400)]
doc:  mention GROUP BY columns can reference target col numbers

Reported-by: hape <postgres-hape@gmx.de>
Discussion: https://postgr.es/m/168871536004.379168.9352636188330923805@wrigleys.postgresql.org

Backpatch-through: 11

19 months agoFix another bug in parent page splitting during GiST index build.
Heikki Linnakangas [Tue, 26 Sep 2023 11:14:49 +0000 (14:14 +0300)]
Fix another bug in parent page splitting during GiST index build.

Yet another bug in the ilk of commits a7ee7c851 and 741b88435. In
741b88435, we took care to clear the memorized location of the
downlink when we split the parent page, because splitting the parent
page can move the downlink. But we missed that even *updating* a tuple
on the parent can move it, because updating a tuple on a gist page is
implemented as a delete+insert, so the updated tuple gets moved to the
end of the page.

This commit fixes the bug in two different ways (belt and suspenders):

1. Clear the downlink when we update a tuple on the parent page, even
   if it's not split. This the same approach as in commits a7ee7c851
   and 741b88435.

   I also noticed that gistFindCorrectParent did not clear the
   'downlinkoffnum' when it stepped to the right sibling. Fix that
   too, as it seems like a clear bug even though I haven't been able
   to find a test case to hit that.

2. Change gistFindCorrectParent so that it treats 'downlinkoffnum'
   merely as a hint. It now always first checks if the downlink is
   still at that location, and if not, it scans the page like before.
   That's more robust if there are still more cases where we fail to
   clear 'downlinkoffnum' that we haven't yet uncovered. With this,
   it's no longer necessary to meticulously clear 'downlinkoffnum',
   so this makes the previous fixes unnecessary, but I didn't revert
   them because it still seems nice to clear it when we know that the
   downlink has moved.

Also add the test case using the same test data that Alexander
posted. I tried to reduce it to a smaller test, and I also tried to
reproduce this with different test data, but I was not able to, so
let's just include what we have.

Backpatch to v12, like the previous fixes.

Reported-by: Alexander Lakhin
Discussion: https://www.postgresql.org/message-id/18129-caca016eaf0c3702@postgresql.org

19 months agodoc: Tell about "vcregress taptest" for regression tests on Windows
Michael Paquier [Mon, 25 Sep 2023 23:16:46 +0000 (08:16 +0900)]
doc: Tell about "vcregress taptest" for regression tests on Windows

There was no mention of this command in the documentation, and it is
useful to run the TAP tests of a target source directory.

Author: Yugo Nagata
Discussion: https://postgr.es/m/20230925153204.926d685d347ee1c8f527090c@sraoss.co.jp
Backpatch-through: 11

19 months agoFix edge-case for xl_tot_len broken by bae868ca.
Thomas Munro [Mon, 25 Sep 2023 20:07:26 +0000 (09:07 +1300)]
Fix edge-case for xl_tot_len broken by bae868ca.

bae868ca removed a check that was still needed.  If you had an
xl_tot_len at the end of a page that was too small for a record header,
but not big enough to span onto the next page, we'd immediately perform
the CRC check using a bogus large length.  Because of arbitrary coding
differences between the CRC implementations on different platforms,
nothing very bad happened on common modern systems.  On systems using
the _sb8.c fallback we could segfault.

Restore that check, add a new assertion and supply a test for that case.
Back-patch to 12, like bae868ca.

Tested-by: Tom Lane <tgl@sss.pgh.pa.us>
Tested-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKGLCkTT7zYjzOxuLGahBdQ%3DMcF%3Dz5ZvrjSOnW4EDhVjT-g%40mail.gmail.com

19 months agopg_dump: tests: Correct test condition for invalid databases
Andres Freund [Mon, 25 Sep 2023 18:50:02 +0000 (11:50 -0700)]
pg_dump: tests: Correct test condition for invalid databases

For some reason I used not_like = { pg_dumpall_dbprivs => 1, } in the test
condition of one of the tests added in in c66a7d75e65. That doesn't make sense
for two reasons: 1) not_like isn't a valid test condition 2) the database
should not be dumped in any of the tests.  Due to 1), the test achieved its
goal, but clearly the formulation is confusing.  Instead use like => {}, with
a comment explaining why.

Reported-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/3ddf79f2-8b7b-a093-11d2-5c739bc64f86@eisentraut.org
Backpatch: 11-, like c66a7d75e65

19 months agoCollect dependency information for parsed CallStmts.
Tom Lane [Mon, 25 Sep 2023 18:41:57 +0000 (14:41 -0400)]
Collect dependency information for parsed CallStmts.

Parse analysis of a CallStmt will inject mutable information,
for instance the OID of the called procedure, so that subsequent
DDL may create a need to re-parse the CALL.  We failed to detect
this for CALLs in plpgsql routines, because no dependency information
was collected when putting a CallStmt into the plan cache.  That
could lead to misbehavior or strange errors such as "cache lookup
failed".

Before commit ee895a655, the issue would only manifest for CALLs
appearing in atomic contexts, because we re-planned non-atomic
CALLs every time through anyway.

It is now apparent that extract_query_dependencies() probably
needs a special case for every utility statement type for which
stmt_requires_parse_analysis() returns true.  I wanted to add
something like Assert(!stmt_requires_parse_analysis(...)) when
falling out of extract_query_dependencies_walker without doing
anything, but there are API issues as well as a more fundamental
point: stmt_requires_parse_analysis is supposed to be applied to
raw parser output, so it'd be cheating to assume it will give the
correct answer for post-parse-analysis trees.  I contented myself
with adding a comment.

Per bug #18131 from Christian Stork.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/18131-576854e79c5cd264@postgresql.org

19 months agoLimit to_tsvector_byid's initial array allocation to something sane.
Tom Lane [Mon, 25 Sep 2023 15:50:28 +0000 (11:50 -0400)]
Limit to_tsvector_byid's initial array allocation to something sane.

The initial estimate of the number of distinct ParsedWords is just
that: an estimate.  Don't let it exceed what palloc is willing to
allocate.  If in fact we need more entries, we'll eventually fail
trying to enlarge the array.  But if we don't, this allows success on
inputs that currently draw "invalid memory alloc request size".

Per bug #18080 from Uwe Binder.  Back-patch to all supported branches.

Discussion: https://postgr.es/m/18080-d5c5e58fef8c99b7@postgresql.org

19 months agopg_upgrade: check for types removed in pg12
Alvaro Herrera [Mon, 25 Sep 2023 12:34:05 +0000 (14:34 +0200)]
pg_upgrade: check for types removed in pg12

Commit cda6a8d01d39 removed a few datatypes, but didn't update
pg_upgrade --check to throw error if these types are used.  So the users
find that pg_upgrade --check tells them that everything is fine, only to
fail when the real upgrade is attempted.

Reviewed-by: Tristan Partin <tristan@neon.tech>
Reviewed-by: Suraj Kharage <suraj.kharage@enterprisedb.com>
Discussion: https://postgr.es/m/202309201654.ng4ksea25mti@alvherre.pgsql

19 months agoDon't use Perl pack('Q') in 039_end_of_wal.pl.
Thomas Munro [Sat, 23 Sep 2023 02:13:06 +0000 (14:13 +1200)]
Don't use Perl pack('Q') in 039_end_of_wal.pl.

'Q' for 64 bit integers turns out not to work on 32 bit Perl, as
revealed by the build farm.  Use 'II' instead, and deal with endianness.

Back-patch to 12, like bae868ca.

Discussion: https://postgr.es/m/ZQ4r1vHcryBsSi_V%40paquier.xyz

19 months agoDon't trust unvalidated xl_tot_len.
Thomas Munro [Fri, 22 Sep 2023 22:26:24 +0000 (10:26 +1200)]
Don't trust unvalidated xl_tot_len.

xl_tot_len comes first in a WAL record.  Usually we don't trust it to be
the true length until we've validated the record header.  If the record
header was split across two pages, previously we wouldn't do the
validation until after we'd already tried to allocate enough memory to
hold the record, which was bad because it might actually be garbage
bytes from a recycled WAL file, so we could try to allocate a lot of
memory.  Release 15 made it worse.

Since 70b4f82a4b5, we'd at least generate an end-of-WAL condition if the
garbage 4 byte value happened to be > 1GB, but we'd still try to
allocate up to 1GB of memory bogusly otherwise.  That was an
improvement, but unfortunately release 15 tries to allocate another
object before that, so you could get a FATAL error and recovery could
fail.

We can fix both variants of the problem more fundamentally using
pre-existing page-level validation, if we just re-order some logic.

The new order of operations in the split-header case defers all memory
allocation based on xl_tot_len until we've read the following page.  At
that point we know that its first few bytes are not recycled data, by
checking its xlp_pageaddr, and that its xlp_rem_len agrees with
xl_tot_len on the preceding page.  That is strong evidence that
xl_tot_len was truly the start of a record that was logged.

This problem was most likely to occur on a standby, because
walreceiver.c recycles WAL files without zeroing out trailing regions of
each page.  We could fix that too, but it wouldn't protect us from rare
crash scenarios where the trailing zeroes don't make it to disk.

With reliable xl_tot_len validation in place, the ancient policy of
considering malloc failure to indicate corruption at end-of-WAL seems
quite surprising, but changing that is left for later work.

Also included is a new TAP test to exercise various cases of end-of-WAL
detection by writing contrived data into the WAL from Perl.

Back-patch to 12.  We decided not to put this change into the final
release of 11.

Author: Thomas Munro <thomas.munro@gmail.com>
Author: Michael Paquier <michael@paquier.xyz>
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Reviewed-by: Noah Misch <noah@leadboat.com> (the idea, not the code)
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Sergei Kornilov <sk@zsrv.org>
Reviewed-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/17928-aa92416a70ff44a2%40postgresql.org

19 months agoDoc: copy-edit the introductory para for the pg_class catalog.
Tom Lane [Fri, 22 Sep 2023 18:52:36 +0000 (14:52 -0400)]
Doc: copy-edit the introductory para for the pg_class catalog.

The previous wording had a faint archaic whiff to it, and more
importantly used "catalogs" as a verb, which while cutely
self-referential seems likely to provoke confusion in this
particular context.  Also consistently use "kind" not "type" to
refer to the different kinds of relations distinguished by relkind.

Per gripe from Martin Nash.  Back-patch to supported versions.

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

19 months agoFix COMMIT/ROLLBACK AND CHAIN in the presence of subtransactions.
Tom Lane [Fri, 22 Sep 2023 03:11:31 +0000 (23:11 -0400)]
Fix COMMIT/ROLLBACK AND CHAIN in the presence of subtransactions.

In older branches, COMMIT/ROLLBACK AND CHAIN failed to propagate
the current transaction's properties to the new transaction if
there was any open subtransaction (unreleased savepoint).
Instead, some previous transaction's properties would be restored.
This is because the "if (s->chain)" check in CommitTransactionCommand
examined the wrong instance of the "chain" flag and falsely
concluded that it didn't need to save transaction properties.

Our regression tests would have noticed this, except they used
identical transaction properties for multiple tests in a row,
so that the faulty behavior was not distinguishable from correct
behavior.

Commit 12d768e70 fixed the problem in v15 and later, but only rather
accidentally, because I removed the "if (s->chain)" test to avoid a
compiler warning, while not realizing that the warning was flagging a
real bug.

In v14 and before, remove the if-test and save transaction properties
unconditionally; just as in the newer branches, that's not expensive
enough to justify thinking harder.

Add the comment and extra regression test to v15 and later to
forestall any future recurrence, but there's no live bug in those
branches.

Patch by me, per bug #18118 from Liu Xiang.  Back-patch to v12 where
the AND CHAIN feature was added.

Discussion: https://postgr.es/m/18118-4b72fcbb903aace6@postgresql.org

19 months agoUpdate comment about set_join_pathlist_hook().
Etsuro Fujita [Thu, 21 Sep 2023 10:45:07 +0000 (19:45 +0900)]
Update comment about set_join_pathlist_hook().

The comment introduced by commit e7cb7ee14 was a bit too terse, which
could lead to extensions doing different things within the hook function
than we intend to allow.  Extend the comment to explain what they can do
within the hook function.

Back-patch to all supported branches.

In passing, I rephrased a nearby comment that I recently added to the
back branches.

Reviewed by David Rowley and Andrei Lepikhov.

Discussion: https://postgr.es/m/CAPmGK15SBPA1nr3Aqsdm%2BYyS-ay0Ayo2BRYQ8_A2To9eLqwopQ%40mail.gmail.com

19 months agoFix GiST README's explanation of the NSN cross-check.
Heikki Linnakangas [Tue, 19 Sep 2023 08:53:51 +0000 (11:53 +0300)]
Fix GiST README's explanation of the NSN cross-check.

The text got the condition backwards, it's "NSN > LSN", not "NSN < LSN".
While we're at it, expand it a little for clarity.

Reviewed-by: Daniel Gustafsson
Discussion: https://www.postgresql.org/message-id/4cb46e18-e688-524a-0f73-b1f03ed5d6ee@iki.fi

19 months agoFix assertion failure with PL/Python exceptions
Michael Paquier [Mon, 18 Sep 2023 23:31:27 +0000 (08:31 +0900)]
Fix assertion failure with PL/Python exceptions

PLy_elog() was not able to handle correctly cases where a SPI called
failed, which would fill in a DETAIL string able to trigger an
assertion.  We may want to improve this infrastructure so as it is able
to provide any extra detail information provided by an error stack, but
this is left as a future improvement as it could impact existing error
stacks and any applications that depend on them.  For now, the assertion
is removed and a regression test is added to cover the case of a failure
with a detail string.

This problem exists since 2bd78eb8d51c, so backpatch all the way down
with tweaks to the regression tests output added where required.

Author: Alexander Lakhin
Discussion: https://postgr.es/m/18070-ab9c171cbf4ebb0f@postgresql.org
Backpatch-through: 11

19 months agoDon't crash if cursor_to_xmlschema is used on a non-data-returning Portal.
Tom Lane [Mon, 18 Sep 2023 18:27:47 +0000 (14:27 -0400)]
Don't crash if cursor_to_xmlschema is used on a non-data-returning Portal.

cursor_to_xmlschema() assumed that any Portal must have a tupDesc,
which is not so.  Add a defensive check.

It's plausible that this mistake occurred because of the rather
poorly chosen name of the lookup function SPI_cursor_find(),
which in such cases is returning something that isn't very much
like a cursor.  Add some documentation to try to forestall future
errors of the same ilk.

Report and patch by Boyu Yang (docs changes by me).  Back-patch
to all supported branches.

Discussion: https://postgr.es/m/dd343010-c637-434c-a8cb-418f53bda3b8.yangboyu.yby@alibaba-inc.com

19 months agoTrack nesting depth correctly when drilling down into RECORD Vars.
Tom Lane [Fri, 15 Sep 2023 21:01:26 +0000 (17:01 -0400)]
Track nesting depth correctly when drilling down into RECORD Vars.

expandRecordVariable() failed to adjust the parse nesting structure
correctly when recursing to inspect an outer-level Var.  This could
result in assertion failures or core dumps in corner cases.

Likewise, get_name_for_var_field() failed to adjust the deparse
namespace stack correctly when recursing to inspect an outer-level
Var.  In this case the likely result was a "bogus varno" error
while deparsing a view.

Per bug #18077 from Jingzhou Fu.  Back-patch to all supported
branches.

Richard Guo, with some adjustments by me

Discussion: https://postgr.es/m/18077-b9db97c6e0ab45d8@postgresql.org

19 months agoFix get_expr_result_type() to find field names for RECORD Consts.
Tom Lane [Fri, 15 Sep 2023 20:39:27 +0000 (16:39 -0400)]
Fix get_expr_result_type() to find field names for RECORD Consts.

This is a back-patch of commit d57534740 ("Fix EXPLAIN of SEARCH
BREADTH FIRST with a constant initial value") into pre-v14 branches.
At the time I'd thought it was not needed in branches that lack the
SEARCH/CYCLE feature, but that was just a failure of imagination.
It's possible to demonstrate "record type has not been registered"
failures in older branches too, during deparsing of views that contain
references to fields of composite constants.

Back-patch only the code changes, as the test cases added by d57534740
all require SEARCH/CYCLE syntax.  A suitable test case will be added
in the upcoming fix for bug #18077.

Discussion: https://postgr.es/m/17644-3bd1f3036d6d7a16@postgresql.org
Discussion: https://postgr.es/m/3607145.1694803130@sss.pgh.pa.us

19 months agoRevert "Improve error message on snapshot import in snapmgr.c"
Michael Paquier [Thu, 14 Sep 2023 07:00:43 +0000 (16:00 +0900)]
Revert "Improve error message on snapshot import in snapmgr.c"

This reverts commit a0d87bcd9b57, following a remark from Andres Frend
that the new error can be triggered with an incorrect SET TRANSACTION
SNAPSHOT command without being really helpful for the user as it uses
the internal file name.

Discussion: https://postgr.es/m/20230914020724.hlks7vunitvtbbz4@awork3.anarazel.de
Backpatch-through: 11

19 months agoImprove error message on snapshot import in snapmgr.c
Michael Paquier [Thu, 14 Sep 2023 01:30:32 +0000 (10:30 +0900)]
Improve error message on snapshot import in snapmgr.c

When a snapshot file fails to be read in ImportSnapshot(), it would
issue an ERROR as "invalid snapshot identifier" when opening a stream
for it in read-only mode.  This error message is reworded to be the same
as all the other messages used in this case on failure, which is useful
when debugging this area.

Thinko introduced by bb446b689b66 where snapshot imports have been
added.  A backpatch down to 11 is done as this can improve any work
related to snapshot imports in older branches.

Author: Bharath Rupireddy
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/CALj2ACWmr=3KdxDkm8h7Zn1XxBoF6hdzq8WQyMn2y1OL5RYFrg@mail.gmail.com
Backpatch-through: 11

19 months agoFix exception safety bug in typcache.c.
Thomas Munro [Wed, 13 Sep 2023 02:32:24 +0000 (14:32 +1200)]
Fix exception safety bug in typcache.c.

If an out-of-memory error was thrown at an unfortunate time,
ensure_record_cache_typmod_slot_exists() could leak memory and leave
behind a global state that produced an infinite loop on the next call.

Fix by merging RecordCacheArray and RecordIdentifierArray into a single
array.  With only one allocation or re-allocation, there is no
intermediate state.

Back-patch to all supported releases.

Reported-by: "James Pang (chaolpan)" <chaolpan@cisco.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/PH0PR11MB519113E738814BDDA702EDADD6EFA%40PH0PR11MB5191.namprd11.prod.outlook.com

19 months agoFix uninitialized access to InitialRunningXacts during decoding after ERROR.
Amit Kapila [Tue, 12 Sep 2023 04:28:50 +0000 (09:58 +0530)]
Fix uninitialized access to InitialRunningXacts during decoding after ERROR.

The transactions and subtransactions array that was allocated under
snapshot builder memory context and recorded during decoding was not
cleared in case of errors. This can result in an assertion failure if we
attempt to retry logical decoding within the same session. To address this
issue, we register a callback function under the snapshot builder memory
context to clear the recorded transactions and subtransactions array along
with the context.

This problem doesn't exist in PG16 and HEAD as instead of using
InitialRunningXacts, we added the list of transaction IDs and
sub-transaction IDs, that have modified catalogs and are running during
snapshot serialization, to the serialized snapshot (see commit 7f13ac8123).

Author: Hou Zhijie
Reviewed-by: Amit Kapila
Backpatch-through: 11
Discussion: http://postgr.es/m/18055-ab3beed9f4b7b7d6@postgresql.org

19 months agoMake recovery report error message when invalid page header is found.
Fujii Masao [Tue, 5 Oct 2021 15:16:03 +0000 (00:16 +0900)]
Make recovery report error message when invalid page header is found.

Commit 0668719801 changed XLogPageRead() so that it validated the page
header, if invalid page header was found reset the error message and
retried reading the page, to fix the scenario where streaming standby
got stuck at a continuation record. This change hid the error message
about invalid page header, which would make it harder for users to
investigate what the actual issue was found in WAL.

To fix the issue, this commit makes XLogPageRead() report the error
message when invalid page header is found.

When not in standby mode, an invalid page header should cause recovery
to end, not retry reading the page, so XLogPageRead() doesn't need to
validate the page header for the retry. Instead, ReadPageInternal() should
be responsible for the validation in that case. Therefore this commit
changes XLogPageRead() so that if not in standby mode it doesn't validate
the page header for the retry.

This commit has been originally pushed as of 68601985e699 for 15 and
newer versions, but not to the older branches.  A recent investigation
related to WAL replay failures has showed up that the lack of this patch
in 12~14 is an issue, as we want to be able to improve the WAL reader to
make a correct distinction between the end-of-wal and OOM cases when
validating record headers.  REL_11_STABLE is left out as it will be
EOL'd soon.

Reported-by: Yugo Nagata
Author: Yugo Nagata, Kyotaro Horiguchi
Reviewed-by: Ranier Vilela, Fujii Masao
Discussion: https://postgr.es/m/20210718045505.32f463ed6c227111038d8ae4@sraoss.co.jp
Discussion: https://postgr.es/m/17928-aa92416a70ff44a2@postgresql.org
Backpatch-through: 12

19 months agodoc: remove mention of backslash doubling in strings
Bruce Momjian [Fri, 8 Sep 2023 21:25:15 +0000 (17:25 -0400)]
doc:  remove mention of backslash doubling in strings

Reported-by: Laurenz Albe
Discussion: https://postgr.es/m/0b03f91a875fb44182f5bed9e1d404ed6d138066.camel@cybertec.at

Author: Laurenz Albe

Backpatch-through: 11

19 months agopg_basebackup: Generate valid temporary slot names under PQbackendPID()
Michael Paquier [Thu, 7 Sep 2023 05:12:33 +0000 (14:12 +0900)]
pg_basebackup: Generate valid temporary slot names under PQbackendPID()

pgbouncer can cause PQbackendPID() to return negative values due to it
filling be_pid with random bytes (even these days pid_max can only be
set up to 2^22 on 64b machines on Linux, for example, so this cannot
happen with normal PID numbers).  When this happens, pg_basebackup may
generate a temporary slot name that may not be accepted by the parser,
leading to spurious failures, like:
pg_basebackup: error: could not send replication command
ERROR:  replication slot name "pg_basebackup_-1201966863" contains
invalid character

This commit fixes that problem by formatting the result from
PQbackendPID() as an unsigned integer when creating the temporary
replication slot name, so as the invalid character is gone and the
command can be parsed.

Author: Jelte Fennema
Reviewed-by: Daniel Gustafsson, Nishant Sharma
Discussion: https://postgr.es/m/CAGECzQQOGvYfp8ziF4fWQ_o8s2K7ppaoWBQnTmdakn3s-4Z=5g@mail.gmail.com
Backpatch-through: 11

19 months agodoc: mention that to_char() values are rounded
Bruce Momjian [Wed, 6 Sep 2023 20:52:24 +0000 (16:52 -0400)]
doc:  mention that to_char() values are rounded

Reported-by: barsikdacat@gmail.com
Diagnosed-by: Laurenz Albe
Discussion: https://postgr.es/m/168991536429.626.9957835774751337210@wrigleys.postgresql.org

Author: Laurenz Albe

Backpatch-through: 11

19 months agodoc: mention libpq regression tests
Bruce Momjian [Tue, 5 Sep 2023 17:05:27 +0000 (13:05 -0400)]
doc: mention libpq regression tests

Reported-by: Ryo Matsumura
Discussion: https://postgr.es/m/TYCPR01MB11316B3FB56EE54D70BF0CEF6E8E4A@TYCPR01MB11316.jpnprd01.prod.outlook.com

Backpatch-through: 11

19 months agoFix out-of-bound read in gtsvector_picksplit()
Michael Paquier [Mon, 4 Sep 2023 05:55:55 +0000 (14:55 +0900)]
Fix out-of-bound read in gtsvector_picksplit()

This could lead to an imprecise choice when splitting an index page of a
GiST index on a tsvector, deciding which entries should remain on the
old page and which entries should move to a new page.

This is wrong since tsearch2 has been moved into core with commit
140d4ebcb46e, so backpatch all the way down.  This error has been
spotted by valgrind.

Author: Alexander Lakhin
Discussion: https://postgr.es/m/17950-6c80a8d2b94ec695@postgresql.org
Backpatch-through: 11

20 months agopostgres_fdw: Fix test for parameterized foreign scan.
Etsuro Fujita [Wed, 30 Aug 2023 08:15:07 +0000 (17:15 +0900)]
postgres_fdw: Fix test for parameterized foreign scan.

Commit e4106b252 should have updated this test, but did not; back-patch
to all supported branches.

Reviewed by Richard Guo.

Discussion: http://postgr.es/m/CAPmGK15nR0NXLSCKQAcqbZbTzrzd5MozowWnTnGfPkayndF43Q%40mail.gmail.com

20 months agoAvoid possible overflow with ltsGetFreeBlock() in logtape.c
Michael Paquier [Tue, 29 Aug 2023 23:03:54 +0000 (08:03 +0900)]
Avoid possible overflow with ltsGetFreeBlock() in logtape.c

nFreeBlocks, defined as a long, stores the number of free blocks in a
logical tape.  ltsGetFreeBlock() has been using an int to store the
value of nFreeBlocks, which could lead to overflows on platforms where
long and int are not the same size (in short everything except Windows
where long is 4 bytes).

The problematic intermediate variable is switched to be a long instead
of an int.

Issue introduced by c02fdc9223015, so backpatch down to 13.

Author: Ranier vilela
Reviewed-by: Peter Geoghegan, David Rowley
Discussion: https://postgr.es/m/CAEudQApLDWCBR_xmwNjGBrDo+f+S4E87x3s7-+hoaKqYdtC4JQ@mail.gmail.com
Backpatch-through: 13

20 months agoInitialize ListenSocket array earlier.
Heikki Linnakangas [Tue, 29 Aug 2023 06:09:40 +0000 (09:09 +0300)]
Initialize ListenSocket array earlier.

After commit b0bea38705, syslogger prints 63 warnings about failing to
close a listen socket at postmaster startup. That's because the
syslogger process forks before the ListenSockets array is initialized,
so ClosePostmasterPorts() calls "close(0)" 64 times. The first call
succeeds, because fd 0 is stdin.

This has been like this since commit 9a86f03b4e in version 13, which
moved the SysLogger_Start() call to before initializing ListenSockets.
We just didn't notice until commit b0bea38705 added the LOG message.

Reported by Michael Paquier and Jeff Janes.

Author: Michael Paquier
Discussion: https://www.postgresql.org/message-id/ZOvvuQe0rdj2slA9%40paquier.xyz
Discussion: https://www.postgresql.org/message-id/ZO0fgDwVw2SUJiZx@paquier.xyz#482670177eb4eaf4c9f03c1eed963e5f
Backpatch-through: 13

20 months agoAvoid unnecessary plancache revalidation of utility statements.
Tom Lane [Thu, 24 Aug 2023 16:02:40 +0000 (12:02 -0400)]
Avoid unnecessary plancache revalidation of utility statements.

Revalidation of a plancache entry (after a cache invalidation event)
requires acquiring a snapshot.  Normally that is harmless, but not
if the cached statement is one that needs to run without acquiring a
snapshot.  We were already aware of that for TransactionStmts,
but for some reason hadn't extrapolated to the other statements that
PlannedStmtRequiresSnapshot() knows mustn't set a snapshot.  This can
lead to unexpected failures of commands such as SET TRANSACTION
ISOLATION LEVEL.  We can fix it in the same way, by excluding those
command types from revalidation.

However, we can do even better than that: there is no need to
revalidate for any statement type for which parse analysis, rewrite,
and plan steps do nothing interesting, which is nearly all utility
commands.  To mechanize this, invent a parser function
stmt_requires_parse_analysis() that tells whether parse analysis does
anything beyond wrapping a CMD_UTILITY Query around the raw parse
tree.  If that's what it does, then rewrite and plan will just
skip the Query, so that it is not possible for the same raw parse
tree to produce a different plan tree after cache invalidation.

stmt_requires_parse_analysis() is basically equivalent to the
existing function analyze_requires_snapshot(), except that for
obscure reasons that function omits ReturnStmt and CallStmt.
It is unclear whether those were oversights or intentional.
I have not been able to demonstrate a bug from not acquiring a
snapshot while analyzing these commands, but at best it seems mighty
fragile.  It seems safer to acquire a snapshot for parse analysis of
these commands too, which allows making stmt_requires_parse_analysis
and analyze_requires_snapshot equivalent.

In passing this fixes a second bug, which is that ResetPlanCache
would exclude ReturnStmts and CallStmts from revalidation.
That's surely *not* safe, since they contain parsable expressions.

Per bug #18059 from Pavel Kulakov.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/18059-79c692f036b25346@postgresql.org

20 months agodoc: Replace list of drivers and PLs with wiki link
Daniel Gustafsson [Wed, 23 Aug 2023 12:13:07 +0000 (14:13 +0200)]
doc: Replace list of drivers and PLs with wiki link

The list of external language drivers and procedural languages was
never complete or exhaustive, and rather than attempting to manage
it the content has migrated to the wiki.  This replaces the tables
altogether with links to the wiki as we regularly get requests for
adding various projects,  which we reject without any clear policy
for why or how the content should be managed.

The threads linked to below are the most recent discussions about
this, the archives contain many more.

Backpatch to all supported branches since the list on the wiki
applies to all branches.

Author: Jonathan Katz <jkatz@postgresql.org>
Discussion: https://postgr.es/m/169165415312.635.10247434927885764880@wrigleys.postgresql.org
Discussion: https://postgr.es/m/169177958824.635.11087800083040275266@wrigleys.postgresql.org
Backpatch-through: v11

20 months agoCache by-reference missing values in a long lived context
Andrew Dunstan [Tue, 22 Aug 2023 15:57:08 +0000 (11:57 -0400)]
Cache by-reference missing values in a long lived context

Attribute missing values might be needed past the lifetime of the tuple
descriptors from which they are extracted. To avoid possibly using
pointers for by-reference values which might thus be left dangling, we
cache a datumCopy'd version of the datum in the TopMemoryContext. Since
we first search for the value this only needs to be done once per
session for any such value.

Original complaint from Tom Lane, idea for mitigation by Andrew Dunstan,
tweaked by Tom Lane.

Backpatch to version 11 where missing values were introduced.

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

20 months agoRemove test from commit fa2e874946.
Jeff Davis [Thu, 10 Aug 2023 17:16:59 +0000 (10:16 -0700)]
Remove test from commit fa2e874946.

The fix itself is fine, but the test revealed other problems related
to parallel query that are not easily fixable. Remove the test for
now to fix the buildfarm.

Discussion: https://postgr.es/m/88825.1691665432@sss.pgh.pa.us
Backpatch-through: 11

20 months agoRecalculate search_path after ALTER ROLE.
Jeff Davis [Mon, 7 Aug 2023 22:12:09 +0000 (15:12 -0700)]
Recalculate search_path after ALTER ROLE.

Renaming a role can affect the meaning of the special string $user, so
must cause search_path to be recalculated.

Discussion: https://postgr.es/m/186761d32c0255debbdf50b6310b581b9c973e6c.camel@j-davis.com
Reviewed-by: Nathan Bossart, Michael Paquier
Backpatch-through: 11

20 months agoStamp 13.12. REL_13_12
Tom Lane [Mon, 7 Aug 2023 20:11:34 +0000 (16:11 -0400)]
Stamp 13.12.

20 months agoLast-minute updates for release notes.
Tom Lane [Mon, 7 Aug 2023 16:50:15 +0000 (12:50 -0400)]
Last-minute updates for release notes.

Security: CVE-2023-39417, CVE-2023-39418

20 months agoReject substituting extension schemas or owners matching ["$'\].
Noah Misch [Mon, 7 Aug 2023 13:05:56 +0000 (06:05 -0700)]
Reject substituting extension schemas or owners matching ["$'\].

Substituting such values in extension scripts facilitated SQL injection
when @extowner@, @extschema@, or @extschema:...@ appeared inside a
quoting construct (dollar quoting, '', or "").  No bundled extension was
vulnerable.  Vulnerable uses do appear in a documentation example and in
non-bundled extensions.  Hence, the attack prerequisite was an
administrator having installed files of a vulnerable, trusted,
non-bundled extension.  Subject to that prerequisite, this enabled an
attacker having database-level CREATE privilege to execute arbitrary
code as the bootstrap superuser.  By blocking this attack in the core
server, there's no need to modify individual extensions.  Back-patch to
v11 (all supported versions).

Reported by Micah Gate, Valerie Woolard, Tim Carey-Smith, and Christoph
Berg.

Security: CVE-2023-39417

20 months agoTranslation updates
Peter Eisentraut [Mon, 7 Aug 2023 10:30:06 +0000 (12:30 +0200)]
Translation updates

Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: 2cfadec9afd05853f9bf6dd83f6cf7fe96f9f3cf

20 months agoRelease notes for 15.4, 14.9, 13.12, 12.16, 11.21.
Tom Lane [Sat, 5 Aug 2023 20:47:04 +0000 (16:47 -0400)]
Release notes for 15.4, 14.9, 13.12, 12.16, 11.21.

21 months agoDoc: update documentation for creating custom scan paths.
Etsuro Fujita [Thu, 3 Aug 2023 08:45:06 +0000 (17:45 +0900)]
Doc: update documentation for creating custom scan paths.

Commit f49842d1e added a new callback for custom scan paths, but missed
updating the documentation.

Back-patch to all supported branches.

Discussion: https://postgr.es/m/CAPmGK15ODkN%2B%3DhkBCufj1HBW0x5OTb65Xuy7ryXchMdiCMpx_g%40mail.gmail.com

21 months agoUpdate comments on CustomPath struct.
Etsuro Fujita [Thu, 3 Aug 2023 08:15:06 +0000 (17:15 +0900)]
Update comments on CustomPath struct.

Commit e7cb7ee14 allowed custom scan providers to create CustomPath
paths for join relations as well, but missed updating the comments.

Back-patch to all supported branches.

Discussion: https://postgr.es/m/CAPmGK15ODkN%2B%3DhkBCufj1HBW0x5OTb65Xuy7ryXchMdiCMpx_g%40mail.gmail.com

21 months agoFix overly strict Assert in jsonpath code
David Rowley [Tue, 1 Aug 2023 13:41:55 +0000 (01:41 +1200)]
Fix overly strict Assert in jsonpath code

This was failing for queries which try to get the .type() of a
jpiLikeRegex.  For example:

select jsonb_path_query('["string", "string"]',
                        '($[0] like_regex ".{7}").type()');

Reported-by: Alexander Kozhemyakin
Bug: #18035
Discussion: https://postgr.es/m/18035-64af5cdcb5adf2a9@postgresql.org
Backpatch-through: 12, where SQL/JSON path was added.

21 months agoDisallow replacing joins with scans in problematic cases.
Etsuro Fujita [Fri, 28 Jul 2023 06:45:06 +0000 (15:45 +0900)]
Disallow replacing joins with scans in problematic cases.

Commit e7cb7ee14, which introduced the infrastructure for FDWs and
custom scan providers to replace joins with scans, failed to add support
handling of pseudoconstant quals assigned to replaced joins in
createplan.c, leading to an incorrect plan without a gating Result node
when postgres_fdw replaced a join with such a qual.

To fix, we could add the support by 1) modifying the ForeignPath and
CustomPath structs to store the list of RestrictInfo nodes to apply to
the join, as in JoinPaths, if they represent foreign and custom scans
replacing a join with a scan, and by 2) modifying create_scan_plan() in
createplan.c to use that list in that case, instead of the
baserestrictinfo list, to get pseudoconstant quals assigned to the join;
but #1 would cause an ABI break.  So fix by modifying the infrastructure
to just disallow replacing joins with such quals.

Back-patch to all supported branches.

Reported by Nishant Sharma.  Patch by me, reviewed by Nishant Sharma and
Richard Guo.

Discussion: https://postgr.es/m/CADrsxdbcN1vejBaf8a%2BQhrZY5PXL-04mCd4GDu6qm6FigDZd6Q%40mail.gmail.com

21 months agoRaise fixed token-length limit in hba.c.
Tom Lane [Thu, 27 Jul 2023 16:07:48 +0000 (12:07 -0400)]
Raise fixed token-length limit in hba.c.

Historically, hba.c limited tokens in the authentication configuration
files (pg_hba.conf and pg_ident.conf) to less than 256 bytes.  We have
seen a few reports of this limit causing problems; notably, for
moderately-complex LDAP configurations.  Increase the limit to 10240
bytes as a low-risk stop-gap solution.

In v13 and earlier, this also requires raising MAX_LINE, the limit
on overall line length.  I'm hesitant to make this code consume
too much stack space, so I only raised that to 20480 bytes.

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

21 months agoGuard against null plan pointer in CachedPlanIsSimplyValid().
Tom Lane [Thu, 20 Jul 2023 18:23:46 +0000 (14:23 -0400)]
Guard against null plan pointer in CachedPlanIsSimplyValid().

If both the passed-in plan pointer and plansource->gplan are
NULL, CachedPlanIsSimplyValid would think that the plan pointer
is possibly-valid and try to dereference it.  For the one extant
call site in plpgsql, this situation doesn't normally happen
which is why we've not noticed. However, it appears to be possible
if the previous use of the cached plan failed, as per report from
Justin Pryzby.  Add an extra check to prevent crashing.
Back-patch to v13 where this code was added.

Discussion: https://postgr.es/m/ZLlV+STFz1l/WhAQ@telsasoft.com

21 months agoDoc: improve description of IN and row-constructor comparisons.
Tom Lane [Wed, 19 Jul 2023 15:00:34 +0000 (11:00 -0400)]
Doc: improve description of IN and row-constructor comparisons.

IN and NOT IN work fine on records and arrays, so just say that
they accept "expressions" not "scalar expressions".  I think that
that phrasing was meant to say that they don't work on set-returning
expressions, but that's not the common meaning of "scalar".

Revise the description of row-constructor comparisons to make it
perhaps a bit less confusing.  (This partially reverts some
dubious wording changes made by commit f56651519.)

Per gripe from Ilya Nenashev.  Back-patch to supported branches.
In HEAD and v16, also drop a NOTE about pre-8.2 behavior, which
is hopefully no longer of interest to anybody.

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

21 months agoDoc: fix out-of-date example of SPI usage.
Tom Lane [Tue, 18 Jul 2023 15:59:39 +0000 (11:59 -0400)]
Doc: fix out-of-date example of SPI usage.

The "count" argument of SPI_exec() only limits execution when
the query is actually returning rows.  This was not the case
before PG 9.0, so this example was correct when written; but
we missed updating it in commit 2ddc600f8.  Extend the example
to show the behavior both with and without RETURNING.

While here, improve the commentary and markup for the rest
of the example.

David G. Johnston and Tom Lane, per report from Curt Kolovson.
Back-patch to all supported branches.

Discussion: https://postgr.es/m/CANhYJV6HWtgz_qjx_APfK0PAgLUzY-2vjLuj7i_o=TZF1LAQew@mail.gmail.com

21 months agoFix indentation in twophase.c
Michael Paquier [Tue, 18 Jul 2023 05:04:51 +0000 (14:04 +0900)]
Fix indentation in twophase.c

This has been missed in cb0cca1, noticed before buildfarm member koel
has been able to complain while poking at a different patch.  Like the
other commit, backpatch all the way down to limit the odds of merge
conflicts.

Backpatch-through: 11

21 months agoFix recovery of 2PC transaction during crash recovery
Michael Paquier [Tue, 18 Jul 2023 04:44:33 +0000 (13:44 +0900)]
Fix recovery of 2PC transaction during crash recovery

A crash in the middle of a checkpoint with some two-phase state data
already flushed to disk by this checkpoint could cause a follow-up crash
recovery to recover twice the same transaction, once from what has been
found in pg_twophase/ at the beginning of recovery and a second time
when replaying its corresponding record.

This would lead to FATAL failures in the startup process during
recovery, where the same transaction would have a state recovered twice
instead of once:
LOG:  recovering prepared transaction 731 from shared memory
LOG:  recovering prepared transaction 731 from shared memory
FATAL:  lock ExclusiveLock on object 731/0/0 is already held

This issue is fixed by skipping the addition of any 2PC state coming
from a record whose equivalent 2PC state file has already been loaded in
TwoPhaseState at the beginning of recovery by restoreTwoPhaseData(),
which is OK as long as the system has not reached a consistent state.

The timing to get a messed up recovery processing is very racy, and
would very unlikely happen.  The thread that has reported the issue has
demonstrated the bug using injection points to force a PANIC in the
middle of a checkpoint.

Issue introduced in 728bd99, so backpatch all the way down.

Reported-by: "suyu.cmj" <mengjuan.cmj@alibaba-inc.com>
Author: "suyu.cmj" <mengjuan.cmj@alibaba-inc.com>
Author: Michael Paquier
Discussion: https://postgr.es/m/109e6994-b971-48cb-84f6-829646f18b4c.mengjuan.cmj@alibaba-inc.com
Backpatch-through: 11

21 months agoAdd indisreplident to fields refreshed by RelationReloadIndexInfo()
Michael Paquier [Fri, 14 Jul 2023 02:16:10 +0000 (11:16 +0900)]
Add indisreplident to fields refreshed by RelationReloadIndexInfo()

RelationReloadIndexInfo() is a fast-path used for index reloads in the
relation cache, and it has always forgotten about updating
indisreplident, which is something that would happen after an index is
selected for a replica identity.  This can lead to incorrect cache
information provided when executing a command in a transaction context
that updates indisreplident.

None of the code paths currently on HEAD that need to check upon
pg_index.indisreplident fetch its value from the relation cache, always
relying on a fresh copy on the syscache.  Unfortunately, this may not be
the case of out-of-core code, that could see out-of-date value.

Author: Shruthi Gowda
Reviewed-by: Robert Haas, Dilip Kumar, Michael Paquier
Discussion: https://postgr.es/m/CAASxf_PBcxax0wW-3gErUyftZ0XrCs3Lrpuhq4-Z3Fak1DoW7Q@mail.gmail.com
Backpatch-through: 11

21 months agoFix updates of indisvalid for partitioned indexes
Michael Paquier [Fri, 14 Jul 2023 01:13:20 +0000 (10:13 +0900)]
Fix updates of indisvalid for partitioned indexes

indisvalid is switched to true for partitioned indexes when all its
partitions have valid indexes when attaching a new partition, up to the
top-most parent if all its leaves are themselves valid when dealing with
multiple layers of partitions.

The copy of the tuple from pg_index used to switch indisvalid to true
came from the relation cache, which is incorrect.  Particularly, in the
case reported by Shruthi Gowda, executing a series of commands in a
single transaction would cause the validation of partitioned indexes to
use an incorrect version of a pg_index tuple, as indexes are reloaded
after an invalidation request with RelationReloadIndexInfo(), a much
faster version than a full index cache rebuild.  In this case, the
limited information updated in the cache leads to an incorrect version
of the tuple used.  One of the symptoms reported was the following
error, with a replica identity update, for instance:
"ERROR: attempted to update invisible tuple"

This is incorrect since 8b08f7d, so backpatch all the way down.

Reported-by: Shruthi Gowda
Author: Michael Paquier
Reviewed-by: Shruthi Gowda, Dilip Kumar
Discussion: https://postgr.es/m/CAASxf_PBcxax0wW-3gErUyftZ0XrCs3Lrpuhq4-Z3Fak1DoW7Q@mail.gmail.com
Backpatch-through: 11

21 months agoHandle DROP DATABASE getting interrupted
Andres Freund [Thu, 13 Jul 2023 20:03:34 +0000 (13:03 -0700)]
Handle DROP DATABASE getting interrupted

Until now, when DROP DATABASE got interrupted in the wrong moment, the removal
of the pg_database row would also roll back, even though some irreversible
steps have already been taken. E.g. DropDatabaseBuffers() might have thrown
out dirty buffers, or files could have been unlinked. But we continued to
allow connections to such a corrupted database.

To fix this, mark databases invalid with an in-place update, just before
starting to perform irreversible steps. As we can't add a new column in the
back branches, we use pg_database.datconnlimit = -2 for this purpose.

An invalid database cannot be connected to anymore, but can still be
dropped.

Unfortunately we can't easily add output to psql's \l to indicate that some
database is invalid, it doesn't fit in any of the existing columns.

Add tests verifying that a interrupted DROP DATABASE is handled correctly in
the backend and in various tools.

Reported-by: Evgeny Morozov <postgresql3@realityexists.net>
Author: Andres Freund <andres@anarazel.de>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/20230509004637.cgvmfwrbht7xm7p6@awork3.anarazel.de
Discussion: https://postgr.es/m/20230314174521.74jl6ffqsee5mtug@awork3.anarazel.de
Backpatch: 11-, bug present in all supported versions

21 months agoRelease lock after encountering bogs row in vac_truncate_clog()
Andres Freund [Thu, 13 Jul 2023 20:03:34 +0000 (13:03 -0700)]
Release lock after encountering bogs row in vac_truncate_clog()

When vac_truncate_clog() encounters bogus datfrozenxid / datminmxid values, it
returns early. Unfortunately, until now, it did not release
WrapLimitsVacuumLock. If the backend later tries to acquire
WrapLimitsVacuumLock, the session / autovacuum worker hangs in an
uncancellable way. Similarly, other sessions will hang waiting for the
lock. However, if the backend holding the lock exited or errored out for some
reason, the lock was released.

The bug was introduced as a side effect of 566372b3d643.

It is interesting that there are no production reports of this problem. That
is likely due to a mix of bugs leading to bogus values having gotten less
common, process exit releasing locks and instances of hangs being hard to
debug for "normal" users.

Discussion: https://postgr.es/m/20230621221208.vhsqgduwfpzwxnpg@awork3.anarazel.de

21 months agoRemove unnecessary pfree() in g_intbig_compress().
Tom Lane [Thu, 13 Jul 2023 17:07:51 +0000 (13:07 -0400)]
Remove unnecessary pfree() in g_intbig_compress().

GiST compress functions (like all GiST opclass functions) are
supposed to be called in short-lived memory contexts, so that
minor memory leaks in them are not of concern, and indeed
explicit pfree's are likely slightly counterproductive.
But this one in g_intbig_compress() is more than
slightly counterproductive, because it's guarded by
"if (in != DatumGetArrayTypeP(entry->key))" which means
that if this test succeeds, we've detoasted the datum twice.
(And to add insult to injury, the extra detoast result is
leaked.)  Let's just drop the whole stanza, relying on the
GiST temporary context mechanism to clean up in good time.

The analogous bit in g_int_compress() is
       if (r != (ArrayType *) DatumGetPointer(entry->key))
           pfree(r);
which doesn't have the gratuitous-detoast problem so
I left it alone.  Perhaps there is a case for removing
unnecessary pfree's more widely, but I'm not sure if it's
worth the code churn.

The potential extra decompress seems expensive enough to
justify calling this a (minor) performance bug and
back-patching.

Konstantin Knizhnik, Matthias van de Meent, Tom Lane

Discussion: https://postgr.es/m/CAEze2Wi86=DxErfvf+SCB2UKmU2amKOF60BKuJOX=w-RojRn0A@mail.gmail.com

21 months agoBe more rigorous about local variables in PostgresMain().
Tom Lane [Mon, 10 Jul 2023 16:14:34 +0000 (12:14 -0400)]
Be more rigorous about local variables in PostgresMain().

Since PostgresMain calls sigsetjmp, any local variables that are not
marked "volatile" have a risk of unspecified behavior.  In practice
this means that when control returns via longjmp, such variables might
get reset to their values as of the time of sigsetjmp, depending on
whether the compiler chose to put them in registers or on the stack.
We were careful about this for "send_ready_for_query", but not the
other local variables.

In the case of the timeout_enabled flags, resetting them to
their initial "false" states is actually good, since we do
"disable_all_timeouts()" in the longjmp cleanup code path.  If that
does not happen, we risk uselessly calling "disable_timeout()" later,
which is harmless but a little bit expensive.  Let's explicitly reset
these flags so that the behavior is correct and platform-independent.
(This change means that we really don't need the new "volatile"
markings after all, but let's install them anyway since any change
in this logic could re-introduce a problem.)

There is no issue for "firstchar" and "input_message" because those
are explicitly reinitialized each time through the query processing
loop.  To make that clearer, move them to be declared inside the loop.
That leaves us with all the function-lifespan locals except the
sigjmp_buf itself marked as volatile, which seems like a good policy
to have going forward.

Because of the possibility of extra disable_timeout() calls, this
seems worth back-patching.

Sergey Shinderuk and Tom Lane

Discussion: https://postgr.es/m/2eda015b-7dff-47fd-d5e2-f1a9899b90a6@postgrespro.ru

21 months agoFix ALTER EXTENSION SET SCHEMA with objects outside an extension's schema
Michael Paquier [Mon, 10 Jul 2023 00:40:17 +0000 (09:40 +0900)]
Fix ALTER EXTENSION SET SCHEMA with objects outside an extension's schema

As coded, the code would use as a base comparison the namespace OID from
the first object scanned in pg_depend when switching its namespace
dependency entry to the new one, and use it as a base of comparison for
any follow-up checks.  It would also be used as the old namespace OID to
switch *from* for the extension's pg_depend entry.  Hence, if the first
object scanned has a namespace different than the one stored in the
extension, we would finish by:
- Not checking that the extension objects map with the extension's
schema.
- Not switching the extension -> namespace dependency entry to the new
namespace provided by the user, making ALTER EXTENSION ineffective.

This issue exists since this command has been introduced in d9572c4 for
relocatable extension, so backpatch all the way down to 11.  The test
case has been provided by Heikki, that I have tweaked a bit to show the
effects on pg_depend for the extension.

Reported-by: Heikki Linnakangas
Author: Michael Paquier, Heikki Linnakangas
Discussion: https://postgr.es/m/20eea594-a05b-4c31-491b-007b6fceef28@iki.fi
Backpatch-through: 11

21 months agoFix type of iterator variable in SH_START_ITERATE
Andres Freund [Thu, 6 Jul 2023 15:34:17 +0000 (08:34 -0700)]
Fix type of iterator variable in SH_START_ITERATE

Also add comment to make the reasoning behind the Assert() more explicit (per
Tom).

Reported-by: Ranier Vilela
Discussion: https://postgr.es/m/CAEudQAocXNJ6s1VLz+hMamLAQAiewRoW17OJ6-+9GACKfj6iPQ@mail.gmail.com
Backpatch: 11-