Robert Haas [Thu, 14 Mar 2024 12:55:25 +0000 (08:55 -0400)]
Allow a no-wait lock acquisition to succeed in more cases.
We don't determine the position at which a process waiting for a lock
should insert itself into the wait queue until we reach ProcSleep(),
and we may at that point discover that we must insert ourselves ahead
of everyone who wants a conflicting lock, in which case we obtain the
lock immediately. Up until now, a no-wait lock acquisition would fail
in such cases, erroneously claiming that the lock couldn't be obtained
immediately. Fix that by trying ProcSleep even in the no-wait case.
No back-patch for now, because I'm treating this as an improvement to
the existing no-wait feature. It could instead be argued that it's a
bug fix, on the theory that there should never be any case whatsoever
where no-wait fails to obtain a lock that would have been obtained
immediately without no-wait, but I'm reluctant to interpret the
semantics of no-wait that strictly.
Robert Haas and Jingxian Li
Discussion: http://postgr.es/m/CA+TgmobCH-kMXGVpb0BB-iNMdtcNkTvcZ4JBxDJows3kYM+GDg@mail.gmail.com
Alexander Korotkov [Thu, 14 Mar 2024 11:38:44 +0000 (13:38 +0200)]
Fix contrib/pg_visibility/meson.build
I broke that in
e85662df44ff by oversight.
Alexander Korotkov [Thu, 14 Mar 2024 11:10:21 +0000 (13:10 +0200)]
Add TAP tests for timeouts
This commit adds new tests to verify that transaction_timeout,
idle_session_timeout, and idle_in_transaction_session_timeout work as expected.
We introduce new injection points in before throwing a timeout FATAL error
and check these injection points are reached.
Discussion: https://postgr.es/m/CAAhFRxiQsRs2Eq5kCo9nXE3HTugsAAJdSQSmxncivebAxdmBjQ%40mail.gmail.com
Author: Andrey Borodin
Reviewed-by: Alexander Korotkov
Alexander Korotkov [Thu, 14 Mar 2024 11:08:53 +0000 (13:08 +0200)]
Fix false reports in pg_visibility
Currently, pg_visibility computes its xid horizon using the
GetOldestNonRemovableTransactionId(). The problem is that this horizon can
sometimes go backward. That can lead to reporting false errors.
In order to fix that, this commit implements a new function
GetStrictOldestNonRemovableTransactionId(). This function computes the xid
horizon, which would be guaranteed to be newer or equal to any xid horizon
computed before.
We have to do the following to achieve this.
1. Ignore processes xmin's, because they consider connection to other databases
that were ignored before.
2. Ignore KnownAssignedXids, because they are not database-aware. At the same
time, the primary could compute its horizons database-aware.
3. Ignore walsender xmin, because it could go backward if some replication
connections don't use replication slots.
As a result, we're using only currently running xids to compute the horizon.
Surely these would significantly sacrifice accuracy. But we have to do so to
avoid reporting false errors.
Inspired by earlier patch by Daniel Shelepanov and the following discussion
with Robert Haas and Tom Lane.
Discussion: https://postgr.es/m/
1649062270.
289865713%40f403.i.mail.ru
Reviewed-by: Alexander Lakhin, Dmitry Koval
Alvaro Herrera [Thu, 14 Mar 2024 09:23:38 +0000 (10:23 +0100)]
Comment out noisy libpq_pipeline test
libpq_pipeline's new 'cancel' test needs more research; disable it
temporarily to prevent measles in the buildfarm.
Daniel Gustafsson [Thu, 14 Mar 2024 08:23:37 +0000 (09:23 +0100)]
Fix documentation comment for pg_md5_hash
Commit
b69aba74578 added the errstr parameter to pg_md5_hash but
missed updating the synopsis in the documentation comment. The
follow-up commit
587de223f03 added the parameter to the list of
outputs. The returnvalue had been changed from integer to bool
before that but remained in the synopsis. This fixes both.
Author: Tatsuro Yamada <
[email protected]>
Discussion: https://postgr.es/m/TYYPR01MB82313576150CC86084A122CD9E292@TYYPR01MB8231.jpnprd01.prod.outlook.com
Amit Kapila [Thu, 14 Mar 2024 06:42:55 +0000 (12:12 +0530)]
Fix typos in reorderbuffer.c.
Author: Kyotaro Horiguchi
Discussion: https://postgr.es/m/
20240314.132817.
1496502692848380820[email protected]
Jeff Davis [Thu, 14 Mar 2024 06:33:44 +0000 (23:33 -0700)]
Introduce "builtin" collation provider.
New provider for collations, like "libc" or "icu", but without any
external dependency.
Initially, the only locale supported by the builtin provider is "C",
which is identical to the libc provider's "C" locale. The libc
provider's "C" locale has always been treated as a special case that
uses an internal implementation, without using libc at all -- so the
new builtin provider uses the same implementation.
The builtin provider's locale is independent of the server environment
variables LC_COLLATE and LC_CTYPE. Using the builtin provider, the
database collation locale can be "C" while LC_COLLATE and LC_CTYPE are
set to "en_US", which is impossible with the libc provider.
By offering a new builtin provider, it clarifies that the semantics of
a collation using this provider will never depend on libc, and makes
it easier to document the behavior.
Discussion: https://postgr.es/m/
ab925f69-5f9d-f85e-b87c-
bd2a44798659@joeconway.com
Discussion: https://postgr.es/m/
dd9261f4-7a98-4565-93ec-
336c1c110d90@manitou-mail.org
Discussion: https://postgr.es/m/
ff4c2f2f9c8fc7ca27c1c24ae37ecaeaeaff6b53.camel%40j-davis.com
Reviewed-by: Daniel Vérité, Peter Eisentraut, Jeremy Schneider
Peter Eisentraut [Thu, 14 Mar 2024 05:57:16 +0000 (06:57 +0100)]
Put genbki.pl output into src/include/catalog/ directly
With the makefile rules, the output of genbki.pl was written to
src/backend/catalog/, and then the header files were linked to
src/include/catalog/.
This changes it so that the output files are written directly to
src/include/catalog/. This makes the logic simpler, and it also makes
the behavior consistent with the meson build system. Also, the list
of catalog files is now kept in parallel in
src/include/catalog/{meson.build,Makefile}, while before the makefiles
had it in src/backend/catalog/Makefile.
Reviewed-by: Andreas Karlsson <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/
21b74bdc-183d-4dd5-9c27-
9378d178f459@eisentraut.org
Michael Paquier [Thu, 14 Mar 2024 04:19:12 +0000 (13:19 +0900)]
Revert "Add basic TAP tests for the low-level backup method"
This reverts commit
99b4a63bef94. The test is proving to be unstable,
so revert it for now.
One of the failures seen involves the cluster started without the
backup_label, where the archives of the primary are overwritten, causing
recovery failures on Windows. This is simple to fix, but there is
another issue that's creeping behind in the form of an "invalid data in
file" ERROR when parsing the backup_label for the second recovery case,
as an effect of the backup_label data written after retrieving its
contents from pg_backup_stop().
_
Per buildfarm member sidewinder.
Michael Paquier [Thu, 14 Mar 2024 01:49:52 +0000 (10:49 +0900)]
Add basic TAP tests for the low-level backup method
There are currently no tests for the low-level backup method where
pg_backup_start() and pg_backup_stop() are involved while taking a
file-system backup. The tests introduced in this commit rely on a
background psql process to make sure that the backup is taken while the
session doing the SQL start and stop calls remains alive.
Two cases are checked here with the backup taken:
- Recovery without a backup_label, leading to a corrupted state.
- Recovery with a backup_label, with a consistent state reached.
Both cases cross-check some patterns in the logs generated when running
recovery.
Author: David Steele
Discussion: https://postgr.es/m/
f20fcc82-dadb-478d-beb4-
1e2ffb0ace76@pgmasters.net
Alexander Korotkov [Thu, 14 Mar 2024 00:07:27 +0000 (02:07 +0200)]
Improve documentation for pg_stat_checkpointer fields
pg_stat_checkpointer contains statistics for checkpoints and restartpoints.
Before
12915a58eec9 documentation said only about checkpoints implying that
restartpoint is the variation of checkpoint.
12915a58eec9 introduced
new separate statistics fields for restartpoints. This commit explicitly
documents fields that are relevant for both checkpoints and restartpoints.
Reported-by: Magnus Hagander
Discussion: https://postgr.es/m/CABUevExav5-SR0x%2BG9kBUMV0G8XsvSUfuyyqmYBBJi6VHns6sw%40mail.gmail.com
Reviewed-by: Anton A. Melnikov
Nathan Bossart [Wed, 13 Mar 2024 19:49:26 +0000 (14:49 -0500)]
Reintroduce MAINTAIN privilege and pg_maintain predefined role.
Roles with MAINTAIN on a relation may run VACUUM, ANALYZE, REINDEX,
REFRESH MATERIALIZE VIEW, CLUSTER, and LOCK TABLE on the relation.
Roles with privileges of pg_maintain may run those same commands on
all relations.
This was previously committed for v16, but it was reverted in
commit
151c22deee due to concerns about search_path tricks that
could be used to escalate privileges to the table owner. Commits
2af07e2f74,
59825d1639, and
c7ea3f4229 resolved these concerns by
restricting search_path when running maintenance commands.
Bumps catversion.
Reviewed-by: Jeff Davis
Discussion: https://postgr.es/m/
20240305161235.GA3478007%40nathanxps13
Robert Haas [Wed, 13 Mar 2024 19:04:22 +0000 (15:04 -0400)]
Add the system identifier to backup manifests.
Before this patch, if you took a full backup on server A and then
tried to use the backup manifest to take an incremental backup on
server B, it wouldn't know that the manifest was from a different
server and so the incremental backup operation could potentially
complete without error. When you later tried to run pg_combinebackup,
you'd find out that your incremental backup was and always had been
invalid. That's poor timing, because nobody likes finding out about
backup problems only at restore time.
With this patch, you'll get an error when trying to take the (invalid)
incremental backup, which seems a lot nicer.
Amul Sul, revised by me. Review by Michael Paquier.
Discussion: http://postgr.es/m/CA+TgmoYLZzbSAMM3cAjV4Y+iCRZn-bR9H2+Mdz7NdaJFU1Zb5w@mail.gmail.com
Alvaro Herrera [Wed, 13 Mar 2024 18:53:49 +0000 (19:53 +0100)]
Hopefully make libpq_pipeline's new cancel test more reliable
The newly introduced cancel test in libpq_pipeline was flaky. It's not
completely clear why, but one option is that the check for "active" was
actually seeing the active state for the previous query. This change
should address any such race condition by first waiting until the
connection is reported as idle.
Author: Jelte Fennema-Nio <
[email protected]>
Discussion: https://postgr.es/m/CAGECzQRvmUK5-d68A+cm+fgmfht9Dv2uZ28-qq3QiaF6EAZqPQ@mail.gmail.com
Robert Haas [Wed, 13 Mar 2024 16:06:44 +0000 (12:06 -0400)]
Expose new function get_controlfile_by_exact_path().
This works just like get_controlfile(), but expects the path to the
control file rather than the path to the data directory that contains
the control file. This makes more sense in cases where the caller
has already constructed the path to the control file itself.
Amul Sul and Robert Haas, reviewed by Michael Paquier
Peter Eisentraut [Wed, 13 Mar 2024 14:07:00 +0000 (15:07 +0100)]
Make the order of the header file includes consistent
Similar to commit
7e735035f20.
Author: Richard Guo <
[email protected]>
Reviewed-by: Bharath Rupireddy <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/CAMbWs4-WhpCFMbXCjtJ%2BFzmjfPrp7Hw1pk4p%2BZpU95Kh3ofZ1A%40mail.gmail.com
Dean Rasheed [Wed, 13 Mar 2024 13:16:24 +0000 (13:16 +0000)]
doc: Improve a couple of places in the MERGE docs.
In the synopsis, make the syntax for merge_update consistent with the
syntax for a plain UPDATE command. It was missing the optional "ROW"
keyword that can be used in a multi-column assignment, and the option
to assign from a multi-column subquery, both of which have been
supported by MERGE since it was introduced.
In the parameters section for the with_query parameter, mention that
WITH RECURSIVE isn't supported, since this is different from plain
INSERT, UPDATE, and DELETE commands. While at it, move that entry to
the top of the list, for consistency with the other pages.
Back-patch to v15, where MERGE was introduced.
Discussion: https://postgr.es/m/CAEZATCWoQyWkMFfu7JXXQr8dA6%3DgxjhYzgpuBP2oz0QoJTxGWw%40mail.gmail.com
Andres Freund [Wed, 13 Mar 2024 08:40:53 +0000 (01:40 -0700)]
meson: macos: Avoid warnings on Sonoma
Starting with the Sonoma toolchain macos' linker emits warnings when the same
library is linked to twice. That's ill considered, as the same library can be
used by multiple subsidiary libraries. Luckily there's a flag to suppress that
warning.
On Ventura meson's default of -Wl,-undefined,dynamic_lookup caused warnings,
which we suppressed with -Wl,-undefined,error. Unfortunately that causes a
warning on Sonoma, which is absurd, as it's documented linker default. To
avoid that warning, only add -Wl,-undefined,error if it does not trigger
warnings. Luckily dynamic_lookup doesn't trigger a warning on Sonoma anymore.
Discussion: https://postgr.es/m/
20231201040515[email protected]
Backpatch: 16-, where the meson build was added
Peter Eisentraut [Wed, 13 Mar 2024 05:40:32 +0000 (06:40 +0100)]
Fix incorrect format placeholders
Michael Paquier [Wed, 13 Mar 2024 05:19:21 +0000 (14:19 +0900)]
Add tests for more row patterns with COPY FROM .. (ON_ERROR ignore)
While digging into the code of this feature, I got confused by the fact
that a line is skipped when a value cannot be converted to its expected
attribute even if the line has fewer attributes than the target
relation. The tests had a check for the case of an empty line, this
commit a couple more patterns where a line is incomplete, but skipped
because of a conversion error.
Reviewed-by: Bharath Rupireddy
Discussion: https://postgr.es/m/
[email protected]
Amit Kapila [Wed, 13 Mar 2024 03:03:26 +0000 (08:33 +0530)]
Fix a random failure in 038_save_logical_slots_shutdown.pl.
The test ensures that all the WAL on the publisher is sent to the
subscriber before shutdown by comparing the confirmed_flush_lsn of the
associated slot with the shutdown_checkpoint WAL location. But if the
shutdown_checkpoint location falls into a new page in the WAL then the
check won't work. So, ensure that the shutdown_checkpoint WAL record
doesn't fall into a new page.
Reported-by: Bharath Rupireddy
Author: Bharath Rupireddy
Reviewed-by: Vignesh C, Kuroda Hayato, Amit Kapila
Discussion: https://postgr.es/m/CALj2ACVLzH5CN-h9=S26mdRHPuJ9yDLUw70yh4JOiPw03WL0CQ@mail.gmail.com
Thomas Munro [Wed, 13 Mar 2024 01:51:24 +0000 (14:51 +1300)]
ci: Use a RAM disk and more CPUs on FreeBSD.
Run the tests in a RAM disk. It's still a UFS file system and is backed
by 20GB of disk, but this avoids a lot of I/O. Even though we disable
fsync, our tests do a lot of directory manipulations, some of which
force file system meta-data to disk and flush slow device write caches
on UFS. This was a bottleneck preventing effective scaling beyond 2
CPUs.
Now we can use 4 CPUs like on other OSes, for a huge speedup.
Reviewed-by: Maxim Orlov <[email protected]>
Discussion: https://postgr.es/m/CA%2BhUKG%2BFXLcEg1dyTqJjDiNQ8pGom4KrJj4wF38C90thti9dVA%40mail.gmail.com
Michael Paquier [Tue, 12 Mar 2024 22:45:11 +0000 (07:45 +0900)]
Add some asserts based on LWLockHeldByMe() for replication slot statistics
Two assertions checking that ReplicationSlotAllocationLock is acquired
are added to pgstat_create_replslot() and pgstat_drop_replslot(),
corresponding to the routines in charge of the creation and the drop of
replication slot statistics. The code previously relied on this
assumption and documented it in comments, but did not enforce this
policy at runtime.
Reviewed-by: Bertrand Drouvot
Discussion: https://postgr.es/m/
[email protected]
Jeff Davis [Tue, 12 Mar 2024 22:24:03 +0000 (15:24 -0700)]
Fix version check in 002_pg_upgrade.pl.
Commit
f696c0cd5f tried to account for the version in a way that
includes development versions, but it was broken. Fix with suggestion
from Tom Lane.
Discussion: https://postgr.es/m/
1553991.
1710191312@sss.pgh.pa.us
Reported-by: Tom Lane
Tom Lane [Tue, 12 Mar 2024 22:16:10 +0000 (18:16 -0400)]
Fix confusion about the return rowtype of SQL-language procedures.
There is a very ancient hack in check_sql_fn_retval that allows a
single SELECT targetlist entry of composite type to be taken as
supplying all the output columns of a function returning composite.
(This is grotty and fundamentally ambiguous, but it's really hard
to do nested composite-returning functions without it.)
As far as I know, that doesn't cause any problems in ordinary
functions. It's disastrous for procedures however. All procedures
that have any output parameters are labeled with prorettype RECORD,
and the CALL code expects it will get back a record with one column
per output parameter, regardless of whether any of those parameters
is composite. Doing something else leads to an assertion failure
or core dump.
This is simple enough to fix: we just need to not apply that rule
when considering procedures. However, that requires adding another
argument to check_sql_fn_retval, which at least in principle might be
getting called by external callers. Therefore, in the back branches
convert check_sql_fn_retval into an ABI-preserving wrapper around a
new function check_sql_fn_retval_ext.
Per report from Yahor Yuzefovich. This has been broken since we
implemented procedures, so back-patch to all supported branches.
Discussion: https://postgr.es/m/CABz5gWHSjj2df6uG0NRiDhZ_Uz=Y8t0FJP-_SVSsRsnrQT76Gg@mail.gmail.com
David Rowley [Tue, 12 Mar 2024 20:34:11 +0000 (09:34 +1300)]
Fix incorrect filename reference in comment
Author: Cary Huang
Discussion: https://postgr.es/m/
18e34071af0.
dbfc9663424635.
8571906799773344646@highgo.ca
Alvaro Herrera [Tue, 12 Mar 2024 16:32:25 +0000 (17:32 +0100)]
libpq: Add encrypted and non-blocking query cancellation routines
The existing PQcancel API uses blocking IO, which makes PQcancel
impossible to use in an event loop based codebase without blocking the
event loop until the call returns. It also doesn't encrypt the
connection over which the cancel request is sent, even when the original
connection required encryption.
This commit adds a PQcancelConn struct and assorted functions, which
provide a better mechanism of sending cancel requests; in particular all
the encryption used in the original connection are also used in the
cancel connection. The main entry points are:
- PQcancelCreate creates the PQcancelConn based on the original
connection (but does not establish an actual connection).
- PQcancelStart can be used to initiate non-blocking cancel requests,
using encryption if the original connection did so, which must be
pumped using
- PQcancelPoll.
- PQcancelReset puts a PQcancelConn back in state so that it can be
reused to send a new cancel request to the same connection.
- PQcancelBlocking is a simpler-to-use blocking API that still uses
encryption.
Additional functions are
- PQcancelStatus, mimicks PQstatus;
- PQcancelSocket, mimicks PQcancelSocket;
- PQcancelErrorMessage, mimicks PQerrorMessage;
- PQcancelFinish, mimicks PQfinish.
Author: Jelte Fennema-Nio <
[email protected]>
Reviewed-by: Denis Laxalde <[email protected]>
Discussion: https://postgr.es/m/AM5PR83MB0178D3B31CA1B6EC4A8ECC42F7529@AM5PR83MB0178.EURPRD83.prod.outlook.com
Heikki Linnakangas [Tue, 12 Mar 2024 13:31:02 +0000 (15:31 +0200)]
Fix copying SockAddr struct
Valgrind alerted about accessing uninitialized bytes after commit
4945e4ed4a:
==700242== VALGRINDERROR-BEGIN
==700242== Conditional jump or move depends on uninitialised value(s)
==700242== at 0x6D8A2A: getnameinfo_unix (ip.c:253)
==700242== by 0x6D8BD1: pg_getnameinfo_all (ip.c:122)
==700242== by 0x4B3EB6: BackendInitialize (postmaster.c:4266)
==700242== by 0x4B684E: BackendStartup (postmaster.c:4114)
==700242== by 0x4B6986: ServerLoop (postmaster.c:1780)
==700242== by 0x4B80CA: PostmasterMain (postmaster.c:1478)
==700242== by 0x3F7424: main (main.c:197)
==700242== Uninitialised value was created by a stack allocation
==700242== at 0x4B6934: ServerLoop (postmaster.c:1737)
==700242==
==700242== VALGRINDERROR-END
That was because the SockAddr struct was not copied correctly.
Per buildfarm animal "skink".
Heikki Linnakangas [Tue, 12 Mar 2024 11:42:38 +0000 (13:42 +0200)]
Move initialization of the Port struct to the child process
In postmaster, use a more lightweight ClientSocket struct that
encapsulates just the socket itself and the remote endpoint's address
that you get from accept() call. ClientSocket is passed to the child
process, which initializes the bigger Port struct. This makes it more
clear what information postmaster initializes, and what is left to the
child process.
Rename the StreamServerPort and StreamConnection functions to make it
more clear what they do. Remove StreamClose, replacing it with plain
closesocket() calls.
Reviewed-by: Tristan Partin, Andres Freund
Discussion: https://www.postgresql.org/message-id/
7a59b073-5b5b-151e-7ed3-
8b01ff7ce9ef@iki.fi
Heikki Linnakangas [Tue, 12 Mar 2024 11:42:36 +0000 (13:42 +0200)]
Pass CAC as an argument to the backend process
We used to smuggle it to the child process in the Port struct, but it
seems better to pass it down as a separate argument. This paves the
way for the next commit, which moves the initialization of the Port
struct to the backend process, after forking.
Reviewed-by: Tristan Partin, Andres Freund
Discussion: https://www.postgresql.org/message-id/
7a59b073-5b5b-151e-7ed3-
8b01ff7ce9ef@iki.fi
Heikki Linnakangas [Tue, 12 Mar 2024 11:42:28 +0000 (13:42 +0200)]
Set socket options in child process after forking
Try to minimize the work done in the postmaster process for each
accepted connection, so that postmaster can quickly proceed with its
duties. These function calls are very fast so this doesn't make any
measurable performance difference in practice, but it's nice to have
all the socket options initialization code in one place for sake of
readability too. This also paves the way for an upcoming commit that
will move the initialization of the Port struct to the child process.
Discussion: https://www.postgresql.org/message-id/
7a59b073-5b5b-151e-7ed3-
8b01ff7ce9ef@iki.fi
Heikki Linnakangas [Tue, 12 Mar 2024 08:18:32 +0000 (10:18 +0200)]
Disconnect if socket cannot be put into non-blocking mode
Commit
387da18874 moved the code to put socket into non-blocking mode
from socket_set_nonblocking() into the one-time initialization
function, pq_init(). In socket_set_nonblocking(), there indeed was a
risk of recursion on failure like the comment said, but in pq_init(),
ERROR or FATAL is fine. There's even another elog(FATAL) just after
this, if setting FD_CLOEXEC fails.
Note that COMMERROR merely logged the error, it did not close the
connection, so if putting the socket to non-blocking mode failed we
would use the connection anyway. You might not immediately notice,
because most socket operations in a regular backend wait for the
socket to become readable/writable anyway. But e.g. replication will
be quite broken.
Backpatch to all supported versions.
Discussion: https://www.postgresql.org/message-id/
d40a5cd0-2722-40c5-8755-
12e9e811fa3c@iki.fi
Alvaro Herrera [Tue, 12 Mar 2024 08:11:24 +0000 (09:11 +0100)]
libpq: Move pg_cancel to fe-cancel.c
No other files need to access this struct, so there is no need to have
its definition in a header file.
Author: Jelte Fennema-Nio <
[email protected]>
Discussion: https://postgr.es/m/
202403061822[email protected]
Michael Paquier [Tue, 12 Mar 2024 05:22:31 +0000 (14:22 +0900)]
Keep replication slot statistics on invalidation
The code path in charge of invalidating a replication slot includes a
call to pgstat_drop_replslot(), which would result in removing the
statistics of the slot once invalidated. However, there is no need to
remove the statistics of an invalidated slot as one could still be
interested in looking at them to understand the activity of the slot
until its actual removal.
The initial design of the feature committed in
be87200efd used the
approach to drop the slots, which is likely why the statistics were
still removed during the invalidation.
Another problem with this operation is that it was done without holding
ReplicationSlotAllocationLock, leaving it unprotected on concurrent
activity. This part is arguably a bug, but that's a limited problem in
practice so no backpatch is done.
In passing, this commit adds a test to check this behavior. The only
remaining code path where slot statistics are dropped now related to the
slot getting dropped.
Author: Bertrand Drouvot
Discussion: https://postgr.es/m/
[email protected]
Amit Kapila [Tue, 12 Mar 2024 04:55:27 +0000 (10:25 +0530)]
Remove redundant fetch of the recent flush pointer in WalSndWaitForWal.
In WalSndWaitForWal(), we fetch a recent flush pointer both outside the
loop and inside the loop. But we start using RecentFlushPtr only after we
fetch it inside the loop. So we can remove one outside the loop.
Author: Shveta Malik
Reviewed-by: Bertrand Drouvot, Matthias van de Meent, Amit Kapila
Discussion: https://postgr.es/m/CAJpy0uBSCQz1yMD-WiEthzEe23dti2-Kr_pitVb7vAPFbFKm=A@mail.gmail.com
Michael Paquier [Tue, 12 Mar 2024 01:02:54 +0000 (10:02 +0900)]
Use printf's %m format instead of strerror(errno) in more places
Most callers of strerror() are removed from the backend code. The
remaining callers require special handling with a saved errno from a
previous system call. The frontend code still needs strerror() where
error states need to be handled outside of fprintf.
Note that pg_regress is not changed to use %m as the TAP output may
clobber errno, since those functions call fprintf() and friends before
evaluating the format string.
Support for %m in src/port/snprintf.c has been added in
d6c55de1f99a,
hence all the stable branches currently supported include it.
Author: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/
[email protected]
Peter Geoghegan [Mon, 11 Mar 2024 22:07:10 +0000 (18:07 -0400)]
Update obsolete index scan TID comments.
Oversight in commit
c2fe139c20.
Jeff Davis [Mon, 11 Mar 2024 20:57:37 +0000 (13:57 -0700)]
Fix 002_pg_upgrade.pl.
Commit
f696c0cd5f caused a test failure in 002_pg_upgrade.pl, because
an earlier s/// operator caused qr// to no longer match the empty
string. Use qr/^$/ instead, which is a better test anyway, because we
expect the stderr to be empty.
Initially this appeared to be a perl bug, but per discussion, it seems
that it was a misunderstanding of how perl works: an empty pattern
uses the last successful pattern. Given how surprising that behavior
is to perl non-experts, we will need to look for similar problems
elsewhere and eliminate the use of empty patterns throughout the
code. For now, address this one instance to fix the buildfarm.
Discussion: https://postgr.es/m/
0ef325fa06e7a1605c4e119c4ecb637c67e5fb4e[email protected]
Reviewed-by: Tom Lane
Alvaro Herrera [Mon, 11 Mar 2024 20:54:03 +0000 (21:54 +0100)]
Add tests for libpq query cancellation APIs
This is in preparation of making changes and additions to these APIs.
Author: Jelte Fennema-Nio <
[email protected]>
Discussion: https://postgr.es/m/CAGECzQRb21spiiykQ48rzz8w+Hcykz+mB2_hxR65D9Qk6nnw=w@mail.gmail.com
Nathan Bossart [Mon, 11 Mar 2024 20:42:27 +0000 (15:42 -0500)]
reindexdb: Allow specifying objects to process in all databases.
Presently, reindexdb's --table, --schema, --index, and --system
options cannot be used together with --all, i.e., you cannot
specify objects to process in all databases. This commit removes
this unnecessary restriction. Furthermore, it removes the
restriction that --system cannot be used with --table, --schema,
and --index. There is no such restriction for the latter options,
and there is no technical reason to disallow these combinations.
Reviewed-by: Kyotaro Horiguchi, Dean Rasheed
Discussion: https://postgr.es/m/
20230628232402.GA1954626%40nathanxps13
Heikki Linnakangas [Mon, 11 Mar 2024 18:43:04 +0000 (20:43 +0200)]
Remove unneeded vacuum_delay_point from heap_vac_scan_get_next_block
heap_vac_scan_get_next_block() does relatively little work, so there
is no need to call vacuum_delay_point(). A future commit will call
heap_vac_scan_get_next_block() from a callback, and we would like to
avoid calling vacuum_delay_point() in that callback.
Author: Melanie Plageman
Discussion: https://postgr.es/m/CAAKRu_Yf3gvXGcCnqqfoq0Q8LX8UM-e-qbm_B1LeZh60f8WhWA%40mail.gmail.com
Heikki Linnakangas [Mon, 11 Mar 2024 18:43:58 +0000 (20:43 +0200)]
Confine vacuum skip logic to lazy_scan_skip()
Rename lazy_scan_skip() to heap_vac_scan_next_block() and move more
code into the function, so that the caller doesn't need to know about
ranges or skipping anymore. heap_vac_scan_next_block() returns the
next block to process, and the logic for determining that block is all
within the function. This makes the skipping logic easier to
understand, as it's all in the same function, and makes the calling
code easier to understand as it's less cluttered. The state variables
needed to manage the skipping logic are moved to LVRelState.
heap_vac_scan_next_block() now manages its own VM buffer separately
from the caller's vmbuffer variable. The caller's vmbuffer holds the
VM page for the current block its processing, while
heap_vac_scan_next_block() keeps a pin on the VM page for the next
unskippable block. Most of the time they are the same, so we hold two
pins on the same buffer, but it's more convenient to manage them
separately.
For readability inside heap_vac_scan_next_block(), move the logic of
finding the next unskippable block to separate function, and add some
comments.
This refactoring will also help future patches to switch to using a
streaming read interface, and eventually AIO
(https://postgr.es/m/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2gkvA%40mail.gmail.com)
Author: Melanie Plageman, Heikki Linnakangas
Reviewed-by: Andres Freund (older version)
Discussion: https://postgr.es/m/CAAKRu_Yf3gvXGcCnqqfoq0Q8LX8UM-e-qbm_B1LeZh60f8WhWA%40mail.gmail.com
Nathan Bossart [Mon, 11 Mar 2024 18:11:20 +0000 (13:11 -0500)]
clusterdb: Allow specifying tables to process in all databases.
Presently, clusterdb's --table option cannot be used together with
--all, i.e., you cannot specify tables to process in all databases.
This commit removes this unnecessary restriction. In passing,
change the synopsis in the documentation to use "[option...]"
instead of "[--verbose | -v]". There are other general-purpose
options (e.g., --quiet and --echo), but the synopsis currently only
lists --verbose.
Reviewed-by: Kyotaro Horiguchi, Dean Rasheed
Discussion: https://postgr.es/m/
20230628232402.GA1954626%40nathanxps13
Bruce Momjian [Mon, 11 Mar 2024 17:31:13 +0000 (13:31 -0400)]
doc: add missing word "the"
Reported-by: [email protected]
Discussion: https://postgr.es/m/
170993253510.640.
5664117187431542912@wrigleys.postgresql.org
Backpatch-through: 12
Alvaro Herrera [Mon, 11 Mar 2024 16:20:36 +0000 (17:20 +0100)]
Add missing connection statuses to docs
The list of connection statuses that PQstatus might return during an
asynchronous connection attempt was outdated:
1. CONNECTION_SETENV is never returned anymore and is only part of the
enum for backwards compatibility, so remove it from the docs.
2. CONNECTION_CHECK_STANDBY and CONNECTION_GSS_STARTUP were not listed,
so add them.
CONNECTION_NEEDED and CONNECTION_CHECK_TARGET are not listed in the docs
on purpose, since these are internal states that can never be observed
by a caller of PQstatus.
Author: Jelte Fennema-Nio <
[email protected]>
Discussion: https://postgr.es/m/CAGECzQRb21spiiykQ48rzz8w+Hcykz+mB2_hxR65D9Qk6nnw=w@mail.gmail.com
Nathan Bossart [Mon, 11 Mar 2024 15:33:36 +0000 (10:33 -0500)]
vacuumdb: Allow specifying objects to process in all databases.
Presently, vacuumdb's --table, --schema, and --exclude-schema
options cannot be used together with --all, i.e., you cannot
specify tables or schemas to process in all databases. This commit
removes this unnecessary restriction, thus enabling potentially
useful commands like "vacuumdb --all --schema pg_catalog".
Reviewed-by: Kyotaro Horiguchi, Dean Rasheed
Discussion: https://postgr.es/m/
20230628232402.GA1954626%40nathanxps13
Heikki Linnakangas [Mon, 11 Mar 2024 07:28:09 +0000 (09:28 +0200)]
Set all_visible_according_to_vm correctly with DISABLE_PAGE_SKIPPING
It's important for 'all_visible_according_to_vm' to correctly reflect
whether the VM bit is set or not, even when we are not trusting the VM
to skip pages, because contrary to what the comment said,
lazy_scan_prune() relies on it.
If it's incorrectly set to 'false', when the VM bit is in fact set,
lazy_scan_prune() will try to set the VM bit again and dirty the page
unnecessarily. As a result, if you used DISABLE_PAGE_SKIPPING, all
heap pages were dirtied, even if there were no changes. We would also
fail to clear any VM bits that were set incorrectly.
This was broken in commit
980ae17310, so backpatch to v16.
Backpatch-through: 16
Reviewed-by: Melanie Plageman, Peter Geoghegan
Discussion: https://www.postgresql.org/message-id/
3df2b582-dc1c-46b6-99b6-
38eddd1b2784@iki.fi
Heikki Linnakangas [Mon, 11 Mar 2024 07:08:02 +0000 (09:08 +0200)]
Don't destroy SMgrRelations at relcache invalidation
With commit
21d9c3ee4e, SMgrRelations remain valid until end of
transaction (or longer if they're "pinned"). Relcache invalidation can
happen in the middle of a transaction, so we must not destroy them at
relcache invalidation anymore.
This was revealed by failures in the 'constraints' test in buildfarm
animals using -DCLOBBER_CACHE_ALWAYS. That started failing with commit
8af2565248, which was the first commit that started to rely on an
SMgrRelation living until end of transaction.
Diagnosed-by: Tomas Vondra, Thomas Munro
Reviewed-by: Thomas Munro
Discussion: https://www.postgresql.org/message-id/CA%2BhUKGK%2B5DOmLaBp3Z7C4S-Yv6yoROvr1UncjH2S1ZbPT8D%2BZg%40mail.gmail.com
David Rowley [Mon, 11 Mar 2024 05:19:56 +0000 (18:19 +1300)]
Fix incorrect accessing of pfree'd memory in Memoize
For pass-by-reference types, the code added in
0b053e78b, which aimed to
resolve a memory leak, was overly aggressive in resetting the per-tuple
memory context which could result in pfree'd memory being accessed
resulting in failing to find previously cached results in the hash
table.
What was happening was prepare_probe_slot() was switching to the
per-tuple memory context and calling ExecEvalExpr(). ExecEvalExpr() may
have required a memory allocation. Both MemoizeHash_hash() and
MemoizeHash_equal() were aggressively resetting the per-tuple context
and after determining the hash value, the context would have gotten reset
before MemoizeHash_equal() was called. This could have resulted in
MemoizeHash_equal() looking at pfree'd memory.
This is less likely to have caused issues on a production build as some
other allocation would have had to have reused the pfree'd memory to
overwrite it. Otherwise, the original contents would have been intact.
However, this clearly caused issues on MEMORY_CONTEXT_CHECKING builds.
Author: Tender Wang, Andrei Lepikhov
Reported-by: Tender Wang (using SQLancer)
Reviewed-by: Andrei Lepikhov, Richard Guo, David Rowley
Discussion: https://postgr.es/m/CAHewXNnT6N6UJkya0z-jLFzVxcwGfeRQSfhiwA+NyLg-x8iGew@mail.gmail.com
Backpatch-through: 14, where Memoize was added
Amit Kapila [Mon, 11 Mar 2024 04:03:04 +0000 (09:33 +0530)]
Doc: Warn about two_phase when altering a subscription's slot name.
We expect the 'two_phase' and 'failover' properties to match between the
slot on the publisher and a subscription option on the subscriber.
Otherwise, the slot on the publisher may behave differently from what the
subscription's failover option says.
Author: Bertrand Drouvot
Reviewed-by: Peter Smith, Tristen Raab, Amit Kapila
Discussion: https://postgr.es/m/
[email protected]
Michael Paquier [Mon, 11 Mar 2024 01:25:01 +0000 (10:25 +0900)]
Improve consistency of replication slot statistics
The replication slot stats stored in shared memory rely on an internal
index number. Both pgstat_reset_replslot() and pgstat_fetch_replslot()
lacked some LWLock protections with ReplicationSlotControlLock while
operating on these index numbers. This issue could cause these two
functions to potentially operate on incorrect slots when taken in
isolation in the event of slots dropped and/or re-created concurrently.
Note that pg_stat_get_replication_slot() is called once per slot when
querying pg_stat_replication_slots, meaning that the stats are retrieved
across multiple ReplicationSlotControlLock acquisitions. So, while this
commit improves more consistency, it may still be possible that
statistics are not completely consistent for a single scan of
pg_stat_replication_slots under concurrent replication slot drop or
creation activity.
The issue should unlikely be a problem in practice, causing the report
of inconsistent stats or or the stats reset of an incorrect slot, so no
backpatch is done.
Author: Bertrand Drouvot
Reviewed-by: Heikki Linnakangas, Shveta Malik, Michael Paquier
Discussion: https://postgr.es/m/
[email protected]
Michael Paquier [Mon, 11 Mar 2024 00:08:05 +0000 (09:08 +0900)]
Add some checkpoint and redo LSNs to a couple of recovery errors
Two FATALs and one PANIC gain details about the LSNs they fail at:
- When restoring from a backup_label, the FATAL log generated when not
finding the checkpoint record now reports its LSN.
- When restoring from a backup_label, the FATAL log generated when not
finding the redo record referenced by a checkpoint record now shows both
the redo and checkpoint record LSNs.
- When not restoring from a backup_label, the PANIC error generated when
not finding the checkpoint record now reports its LSN.
This information is useful when debugging corruption issues, and these
LSNs may not show up in the logs depending on the level of logging
configured in the backend.
Author: David Steele
Discussion: https://postgr.es/m/
0e90da89-77ca-4ccf-872c-
9626d755e288@pgmasters.net
Michael Paquier [Sun, 10 Mar 2024 23:40:40 +0000 (08:40 +0900)]
Improve support for ExplainOneQuery() hook
There is a hook called ExplainOneQuery_hook that gives modules the
possibility to plug into this code path, but, like utility.c for utility
statement execution, there is no corresponding "standard" routine in
the case of EXPLAIN executed for one Query.
This commit adds a new standard_ExplainOneQuery() in explain.c, which is
able to run explain on a non-utility Query without calling its hook.
Per the feedback received from a couple of hackers, this change gives
the possibility to cut a few hundred lines of code in some of the
popular out-of-core modules as these maintained a copy of
ExplainOneQuery(), adding custom extra information at the beginning or
the end of the EXPLAIN output.
Author: Mats Kindahl
Reviewed-by: Aleksander Alekseev, Jelte Fennema-Nio, Andrei Lepikhov
Discussion: https://postgr.es/m/CA+14427V_B4EAoC_o-iYYucRdMSOTfpuH9k-QbexffY1HYJBiA@mail.gmail.com
David Rowley [Sun, 10 Mar 2024 23:27:11 +0000 (12:27 +1300)]
Fix deparsing of Consts in postgres_fdw ORDER BY
For UNION ALL queries where a union child query contained a foreign
table, if the targetlist of that query contained a constant, and the
top-level query performed an ORDER BY which contained the column for the
constant value, then postgres_fdw would find the EquivalenceMember with
the Const and then try to produce an ORDER BY containing that Const.
This caused problems with INT typed Consts as these could appear to be
requests to order by an ordinal column position rather than the constant
value. This could lead to either an error such as:
ERROR: ORDER BY position <int const> is not in select list
or worse, if the constant value is a valid column, then we could just
sort by the wrong column altogether.
Here we fix this issue by just not including these Consts in the ORDER
BY clause.
In passing, add a new section for testing ORDER BY in the postgres_fdw
tests and move two existing tests which were misplaced in the WHERE
clause testing section into it.
Reported-by: Michał Kłeczek
Reviewed-by: Ashutosh Bapat, Richard Guo
Bug: #18381
Discussion: https://postgr.es/m/
0714C8B8-8D82-4ABB-9F8D-
A0C3657E7B6E%40kleczek.org
Discussion: https://postgr.es/m/18381-
137456acd168bf93%40postgresql.org
Backpatch-through: 12, oldest supported version
Peter Eisentraut [Sun, 10 Mar 2024 06:33:57 +0000 (07:33 +0100)]
Combine headerscheck and cpluspluscheck scripts
They are mostly the same, and it is tedious to maintain two copies of
essentially the same exclude list. headerscheck now has a new option
--cplusplus to select the cpluspluscheck functionality. The top-level
make targets are still the same.
Reviewed-by: Thomas Munro <[email protected]>
Reviewed-by: Michael Paquier <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/
4754a5b0-a32b-4036-a99a-
6de14cf9fd72@eisentraut.org
Jeff Davis [Sat, 9 Mar 2024 22:48:18 +0000 (14:48 -0800)]
Catalog changes preparing for builtin collation provider.
Rename pg_collation.colliculocale to colllocale, and
pg_database.daticulocale to datlocale. These names reflects that the
fields will be useful for the upcoming builtin provider as well, not
just for ICU.
This is purely a rename; no changes to the meaning of the fields.
Discussion: https://postgr.es/m/
ff4c2f2f9c8fc7ca27c1c24ae37ecaeaeaff6b53.camel%40j-davis.com
Reviewed-by: Peter Eisentraut
Joe Conway [Sat, 9 Mar 2024 22:32:32 +0000 (17:32 -0500)]
Make contrib/tablefunc crosstab() also check typmod
contrib/tablefunc connectby() checks both type OID and typmod for
its output columns while crosstab() only checks type OID. Fix that
by makeing the crosstab() check look more like the connectby() check.
Reported-by: Tom Lane
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/flat/18937.
1709676295%40sss.pgh.pa.us
Tom Lane [Sat, 9 Mar 2024 21:20:44 +0000 (16:20 -0500)]
Simplify and merge unwanted-module drop logic in AdjustUpgrade.pm.
In
be7800674 and followups, we failed to notice that there was
already a better way to do it: instead of using DROP DATABASE
IF EXISTS, we can check the list of existing DBs. Also, there
seems no reason not to merge this into the pre-existing code
for getting rid of unwanted module databases.
Discussion: https://postgr.es/m/
1066872.
1710006597@sss.pgh.pa.us
Tom Lane [Sat, 9 Mar 2024 20:48:21 +0000 (15:48 -0500)]
Improve wrong-tuple-type error reports in contrib/tablefunc.
These messages were fairly confusing, and didn't match the
column names used in the SGML docs. Try to improve that.
Also use error codes more specific than ERRCODE_SYNTAX_ERROR.
Patch by me, reviewed by Joe Conway
Discussion: https://postgr.es/m/18937.
1709676295@sss.pgh.pa.us
Jeff Davis [Sat, 9 Mar 2024 19:51:10 +0000 (11:51 -0800)]
Run perltidy on 002_pg_upgrade.pl.
Jeff Davis [Sat, 9 Mar 2024 19:48:52 +0000 (11:48 -0800)]
Fix cross-version pg_upgrade test.
Pass each statement as a separate '-c' arg, so they don't get combined
into a single transaction.
Discussion: https://postgr.es/m/
bca97aecb50b2026b7dbc26604bf31861c819a64[email protected]
Reviewed-by: Tom Lane
Michael Paquier [Sat, 9 Mar 2024 06:44:41 +0000 (15:44 +0900)]
Document units of "timeout" in ConditionVariableTimedSleep()
The timeout is passed down to WaitLatch() as milliseconds.
Author: Shveta Malik
Discussion: https://postgr.es/m/CAJpy0uC=xiBQD1WapgYYvOiytap6ULJaakLd867zZXqu9tYc8w@mail.gmail.com
Jeff Davis [Fri, 8 Mar 2024 23:58:32 +0000 (15:58 -0800)]
Fix type signedness error in commit
5c40364dd6.
Use ssize_t instead of size_t.
Discussion: https://postgr.es/m/
b20d6d97-7338-48ea-ba33-
837a1c8ef98e@iki.fi
Reported-by: Heikki Linnakangas
Daniel Gustafsson [Fri, 8 Mar 2024 21:53:06 +0000 (22:53 +0100)]
Fix errorhandling for reading from a pipe
When reading a line from a pipe failed on no data being read, the
errorhandling was erroneously logging with %m even thoug no error
description is available for %m to print. This flaw accidentally
introduced in
5c7038d70bb.
Reported-by: Peter Eisentraut <[email protected]>
Discussion: https://postgr.es/m/
baa34329-f431-46af-bf74-
1a78fdc90e4f@eisentraut.org
Daniel Gustafsson [Fri, 8 Mar 2024 21:50:20 +0000 (22:50 +0100)]
Replace perror with custom postgres logging
perror() is not used in postgres anymore out of policy, this replaces
the final callsites with the custom postgres logging framework.
Reviewed-by: Tom Lane <[email protected]>
Reviewed-by: Peter Eisentraut <[email protected]>
Discussion: https://postgr.es/m/
89B00F63-40F7-4D82-8353-
DC9CABBAC1D1@yesql.se
Tom Lane [Fri, 8 Mar 2024 17:07:35 +0000 (12:07 -0500)]
Improve WIN32 waiting logic in psql's \watch command.
do_watch had some leftover logic for enabling siglongjmp out of
waiting for input. That's never done anything on Windows (cf.
psql_cancel_callback), and do_watch no longer relies on it for
non-Windows, so let's drop it.
Also, when the user cancels \watch by pressing ^C, the Windows
code would run the query one more time before exiting. That doesn't
seem very desirable, and it's not what happens on other platforms.
Use the "done" flag similarly to non-Windows to avoid the extra query
execution.
Yugo Nagata (with minor fixes by me)
Discussion: https://postgr.es/m/
20240305220552.
85fd4afd6b6b8103bf4fe3d0@sraoss.co.jp
Alvaro Herrera [Fri, 8 Mar 2024 15:32:29 +0000 (16:32 +0100)]
Admit deferrable PKs into rd_pkindex, but flag them as such
... and in particular don't return them as replica identity.
The motivation for this change is letting the primary keys be seen by
code that derives NOT NULL constraints from them, when creating
inheritance children; before this change, if you had a deferrable PK,
pg_dump would not recreate the attnotnull marking properly, because the
column would not be considered as having anything to back said marking
after dropping the throwaway NOT NULL constraint.
The reason we don't want these PKs as replica identities is that
replication can corrupt data, if the uniqueness constraint is
transiently broken.
Reported-by: Amul Sul <[email protected]>
Reviewed-by: Dean Rasheed <[email protected]>
Discussion: https://postgr.es/m/CAAJ_b94QonkgsbDXofakHDnORQNgafd1y3Oa5QXfpQNJyXyQ7A@mail.gmail.com
Alexander Korotkov [Fri, 8 Mar 2024 11:01:36 +0000 (13:01 +0200)]
Avoid recursion in MemoryContext functions
You might run out of stack space with recursion, which is not nice in
functions that might be used e.g. at cleanup after transaction
abort. MemoryContext contains pointer to parent and siblings, so we
can traverse a tree of contexts iteratively, without using
stack. Refactor the functions to do that.
MemoryContextStats() still recurses, but it now has a limit to how
deep it recurses. Once the limit is reached, it prints just a summary
of the rest of the hierarchy, similar to how it summarizes contexts
with lots of children. That seems good anyway, because a context dump
with hundreds of nested contexts isn't very readable.
Report by Egor Chindyaskin and Alexander Lakhin.
Discussion: https://postgr.es/m/
1672760457.
940462079%40f306.i.mail.ru
Author: Heikki Linnakangas
Reviewed-by: Robert Haas, Andres Freund, Alexander Korotkov, Tom Lane
Alexander Korotkov [Fri, 8 Mar 2024 11:01:12 +0000 (13:01 +0200)]
Avoid stack overflow in ShowTransactionStateRec()
The function recurses, but didn't perform stack-depth checks. It's
just a debugging aid, so instead of the usual check_stack_depth()
call, stop the printing if we'd risk stack overflow.
Here's an example of how to test this:
(n=
1000000; printf "BEGIN;"; for ((i=1;i<=$n;i++)); do printf "SAVEPOINT s$i;"; done; printf "SET log_min_messages = 'DEBUG5'; SAVEPOINT sp;") | psql >/dev/null
In the passing, swap building the list of child XIDs and recursing to
parent. That saves memory while recursing, reducing the risk of out of
memory errors with lots of subtransactions. The saving is not very
significant in practice, but this order seems more logical anyway.
Report by Egor Chindyaskin and Alexander Lakhin.
Discussion: https://www.postgresql.org/message-id/
1672760457.
940462079%40f306.i.mail.ru
Author: Heikki Linnakangas
Reviewed-by: Robert Haas, Andres Freund, Alexander Korotkov
Alexander Korotkov [Fri, 8 Mar 2024 11:00:40 +0000 (13:00 +0200)]
Turn tail recursion into iteration in CommitTransactionCommand()
Usually the compiler will optimize away the tail recursion anyway, but
if it doesn't, you can drive the function into stack overflow. For
example:
(n=
1000000; printf "BEGIN;"; for ((i=1;i<=$n;i++)); do printf "SAVEPOINT s$i;"; done; printf "ERROR; COMMIT;") | psql >/dev/null
In order to get better readability and less changes to the existing code the
recursion-replacing loop is implemented as a wrapper function.
Report by Egor Chindyaskin and Alexander Lakhin.
Discussion: https://postgr.es/m/
1672760457.
940462079%40f306.i.mail.ru
Author: Alexander Korotkov, Heikki Linnakangas
John Naylor [Fri, 8 Mar 2024 04:08:01 +0000 (11:08 +0700)]
Revert "Fix link error for test_radixtree module on Windows"
This reverts commit
9552e3ace317ac8bb0a80613c0e5cd6536c96dc8.
I (john) forgot to revert this locally when a more principled
fix was found, which has the same message title.
John Naylor [Fri, 8 Mar 2024 03:57:40 +0000 (10:57 +0700)]
Fix link error for test_radixtree module on Windows
Add PGDLLIMPORT to pg_popcount32/64. In passing, fix a typo.
Diagnosis by Masahiko Sawada, patch by David Rowley
Per buildfarm members drongo and fairywren
Discussion: https://postgr.es/m/CAD21AoAMm1mQd%3Dw4PrfrKK%3DOMP8j8%3D7ntJRPF8%2B%3D10iUuvwiCA%40mail.gmail.com
Discussion: https://postgr.es/m/CAApHDvov7724UrD1Ug0D1eV%2B9Pd_x5VEQmw-6HVG9w1WdCxXPA%40mail.gmail.com
John Naylor [Fri, 8 Mar 2024 03:25:23 +0000 (10:25 +0700)]
Fix link error for test_radixtree module on Windows
Add back "link_with" directive, similar to the one removed by
1f1d73a8b,
but only for Windows, but use the "_shlib" variation.
Diagnosis by Masahiko Sawada, proposed fix adjusted and tested by me
Per buildfarm members drongo and fairywren
Discussion: https://postgr.es/m/CAD21AoAMm1mQd%3Dw4PrfrKK%3DOMP8j8%3D7ntJRPF8%2B%3D10iUuvwiCA%40mail.gmail.com
Amit Kapila [Fri, 8 Mar 2024 02:40:45 +0000 (08:10 +0530)]
Introduce a new GUC 'standby_slot_names'.
This patch provides a way to ensure that physical standbys that are
potential failover candidates have received and flushed changes before
the primary server making them visible to subscribers. Doing so guarantees
that the promoted standby server is not lagging behind the subscribers
when a failover is necessary.
The logical walsender now guarantees that all local changes are sent and
flushed to the standby servers corresponding to the replication slots
specified in 'standby_slot_names' before sending those changes to the
subscriber.
Additionally, the SQL functions pg_logical_slot_get_changes,
pg_logical_slot_peek_changes and pg_replication_slot_advance are modified
to ensure that they process changes for failover slots only after physical
slots specified in 'standby_slot_names' have confirmed WAL receipt for those.
Author: Hou Zhijie and Shveta Malik
Reviewed-by: Masahiko Sawada, Peter Smith, Bertrand Drouvot, Ajin Cherian, Nisha Moond, Amit Kapila
Discussion: https://postgr.es/m/
514f6f2f-6833-4539-39f1-
96cd1e011f23@enterprisedb.com
Tom Lane [Fri, 8 Mar 2024 00:37:51 +0000 (19:37 -0500)]
Cope with a deficiency in OpenSSL 3.x's error reporting.
In OpenSSL 3.0.0 and later, ERR_reason_error_string randomly refuses
to provide a string for error codes representing system errno values
(e.g., "No such file or directory"). There is a poorly-documented way
to extract the errno from the SSL error code in this case, so do that
and apply strerror, rather than falling back to reporting the error
code's numeric value as we were previously doing.
Problem reported by David Zhang, although this is not his proposed
patch; it's instead based on a suggestion from Heikki Linnakangas.
Back-patch to all supported branches, since any of them are likely
to be used with recent OpenSSL.
Discussion: https://postgr.es/m/
b6fb018b-f05c-4afd-abd3-
318c649faf18@highgo.ca
Michael Paquier [Fri, 8 Mar 2024 00:31:52 +0000 (09:31 +0900)]
Add support for DEFAULT in ALTER TABLE .. SET ACCESS METHOD
This option can be used to switch a relation to use the access method
set by default_table_access_method when running the command.
This has come up when discussing the possibility to support setting
pg_class.relam for partitioned tables (left out here as future work),
while being useful on its own for relations with physical storage as
these must have an access method set.
Per suggestion from Justin Pryzby.
Author: Michael Paquier
Reviewed-by: Justin Pryzby
Discussion: https://postgr.es/m/ZeCZ89xAVFeOmrQC@pryzbyj2023
Michael Paquier [Thu, 7 Mar 2024 23:44:13 +0000 (08:44 +0900)]
Update comment of AlterTableCmd->name in parsenodes.h
Since
b0483263dda0, this field can be used to store an access method
name for ALTER TABLE, but access methods were not mentioned in the
field's description.
Issue noticed while working on the area.
Discussion: https://postgr.es/m/
[email protected]
Jeff Davis [Thu, 7 Mar 2024 19:15:06 +0000 (11:15 -0800)]
Unicode case mapping tables and functions.
Implements Unicode simple case mapping, in which all code points map
to exactly one other code point unconditionally.
These tables are generated from UnicodeData.txt, which is already
being used by other infrastructure in src/common/unicode. The tables
are checked into the source tree, so they only need to be regenerated
when we update the Unicode version.
In preparation for the builtin collation provider, and possibly useful
for other callers.
Discussion: https://postgr.es/m/
ff4c2f2f9c8fc7ca27c1c24ae37ecaeaeaff6b53.camel%40j-davis.com
Reviewed-by: Peter Eisentraut, Daniel Verite, Jeremy Schneider
Peter Eisentraut [Thu, 7 Mar 2024 10:50:24 +0000 (11:50 +0100)]
Fix description and grouping of RangeTblEntry.inh
The inh field of RangeTblEntry was doubly confusingly documented.
Some parts of the code insisted that it was only valid for
RTE_RELATION entries, other parts said the field was valid for all
entries. Neither was quite correct. More correctly, the field is
valid for RTE_RELATION entries but is also used in the planner for
RTE_SUBQUERY entries. So it makes more sense to group it with other
fields that are primarily for RTE_RELATION but borrowed by
RTE_SUBQUERY. (The exact position was chosen so that it is next to
relkind for better struct packing, and next to relid, since relid and
inh are sort of the input fields and the others are filled in later.)
Also add documentation for the planner's use at the struct definition.
Discussion: https://www.postgresql.org/message-id/
6c1fbccc-85c8-40d3-b08b-
4f47f2093711@eisentraut.org
John Naylor [Thu, 7 Mar 2024 09:59:25 +0000 (16:59 +0700)]
Blind attempt to fix ODR violations
Remove apparently useless "link_with" directive. Even if this isn't the
root cause, it makes the .build file more like the other test modules.
Reviewed by Masahiko Sawada
Follow-up to
ee1b30f12, per buildfarm members olingo and grassquit.
Discussion: https://postgr.es/m/CANWCAZaJAaO8MimTU%2BY-DZutM6HQLQu%3DK2HyoQULdB3v_6BSCg%40mail.gmail.com
Dean Rasheed [Thu, 7 Mar 2024 09:57:02 +0000 (09:57 +0000)]
Fix handling of self-modified tuples in MERGE.
When an UPDATE or DELETE action in MERGE returns TM_SelfModified,
there are 2 possible causes:
1). The target tuple was already updated or deleted by the current
command. This can happen if the target row joins to more than one
source row, and the SQL standard explicitly says that this must be
an error.
2). The target tuple was already updated or deleted by a later command
in the current transaction. This can happen if the tuple is
modified by a BEFORE trigger or a volatile function used in the
query, and should be an error for the same reason that it is in a
plain UPDATE or DELETE command.
In MERGE's primary error handling block, it failed to check for (2),
causing it to return a misleading error message in such cases.
In the secondary error handling block, following a concurrent update
from another session, it failed to check for (1), causing it to
silently ignore target rows joined to more than one source row,
instead of reporting an error.
Fix this, and add tests for both of these cases.
Per report from Wenjiang Zhang. Back-patch to v15, where MERGE was
introduced.
Discussion: https://postgr.es/m/tencent_41DE0FF443FE14B94A5898D373792109E408%40qq.com
John Naylor [Thu, 7 Mar 2024 07:25:30 +0000 (14:25 +0700)]
Fix incorrect format specifier for int64
Follow-up to
ee1b30f12, per buildfarm member mamba.
Discussion: https://postgr.es/m/CANWCAZYwyRMU%2BOTVOjK%3Dno1hm-W3ZQ5vrSFM1MFAaLtLydvwzA%40mail.gmail.com
John Naylor [Thu, 7 Mar 2024 07:11:49 +0000 (14:11 +0700)]
Fix redefinition of typedefs
Per buildfarm members sifaka and longfin, clang with
-Wtypedef-redefinition warns of duplicate typedefs unless building with
C11. Follow-up to
ee1b30f12.
Masahiko Sawada
Discussion: https://postgr.es/m/CANWCAZauSg%3DLUbBbXhpeQtBuPifmzQNTYS6O8NsoAPz1zL-Txg%40mail.gmail.com
John Naylor [Thu, 7 Mar 2024 05:40:11 +0000 (12:40 +0700)]
Add template for adaptive radix tree
This implements a radix tree data structure based on the design in
"The Adaptive Radix Tree: ARTful Indexing for Main-Memory Databases"
by Viktor Leis, Alfons Kemper, and ThomasNeumann, 2013. The main
technique that makes it adaptive is using several different node types,
each with a different capacity of elements, and a different algorithm
for accessing them. The nodes start small and grow/shrink as needed.
The main advantage over hash tables is efficient sorted iteration and
better memory locality when successive keys are lexicographically
close together. The implementation currently assumes 64-bit integer
keys, and traversing the tree is in general slower than a linear
probing hash table, so this is not a general-purpose associative array.
The paper describes two other techniques not implemented here,
namely "path compression" and "lazy expansion". These can further
reduce memory usage and speed up traversal, but the former would add
significant complexity and the latter requires storing the full key
with the value. We do trivially compress the path when leading bytes
of the key are zeros, however.
For value storage, we use "combined pointer/value slots", as
recommended in the paper. Values of size equal or smaller than the the
platform's pointer type are stored in the array of child pointers in
the last level node, while larger values are each stored in a separate
allocation. This is for now fixed at compile time, but it would be
fairly trivial to allow determining at runtime how variable-length
values are stored.
One innovation in our implementation compared to the ART paper is
decoupling the notion of node "size class" from "kind". The size
classes within a given node kind have the same underlying type, but
a variable capacity for children, so we can introduce additional node
sizes with little additional code.
To enable different use cases to specialize for different value types
and for shared/local memory, we use macro-templatized code generation
in the same manner as simplehash.h and sort_template.h.
Future commits will use this infrastructure for storing TIDs.
Patch by Masahiko Sawada and John Naylor, but a substantial amount of
credit is due to Andres Freund, whose proof-of-concept was a valuable
source of coding idioms and awareness of performance pitfalls, and
who reviewed earlier versions.
Discussion: https://postgr.es/m/CAD21AoAfOZvmfR0j8VmZorZjL7RhTiQdVttNuC4W-Shdc2a-AA%40mail.gmail.com
Michael Paquier [Thu, 7 Mar 2024 00:57:52 +0000 (09:57 +0900)]
Revert "Add recovery TAP test for race condition with slot invalidations"
This reverts commit
08a52ab151ca, due to some sporadic instability in
the test. Getting the test right should require some redesign with a
second injection point, but let's revert it for now to avoid these
issues in the CI as a lot of patches are under discussion in this last
commit fest.
Per buildfarm members hachi and gokiburi.
Discussion: https://postgr.es/m/
[email protected]
Michael Paquier [Wed, 6 Mar 2024 23:30:35 +0000 (08:30 +0900)]
Revert "Fix parallel-safety check of expressions and predicate for index builds"
This reverts commit
eae7be600be7, following a discussion with Tom Lane,
due to concerns that this impacts the decisions made by the planner for
the number of workers spawned based on the inlining and const-folding of
index expressions and predicate for cases that would have worked until
this commit.
Discussion: https://postgr.es/m/162802.
1709746091@sss.pgh.pa.us
Backpatch-through: 12
Jeff Davis [Wed, 6 Mar 2024 20:50:01 +0000 (12:50 -0800)]
Add Unicode property tables.
Provide functions to test for Unicode properties, such as Alphabetic
or Cased. These functions use tables derived from Unicode data files,
similar to the tables for Unicode normalization or general category,
and those tables can be updated with the 'update-unicode' build
target.
Use Unicode properties to provide functions to test for regex
character classes, like 'punct' or 'alnum'.
Infrastructure in preparation for a builtin collation provider, and
may also be useful for other callers.
Discussion: https://postgr.es/m/
ff4c2f2f9c8fc7ca27c1c24ae37ecaeaeaff6b53.camel%40j-davis.com
Reviewed-by: Daniel Verite, Peter Eisentraut, Jeremy Schneider
Tom Lane [Wed, 6 Mar 2024 19:41:13 +0000 (14:41 -0500)]
Fix type-checking of RECORD-returning functions in FROM.
In the corner case where a function returning RECORD has been
simplified to a RECORD constant or an inlined ROW() expression,
ExecInitFunctionScan failed to cross-check the function's result
rowtype against the coldeflist provided by the calling query.
That happened because get_expr_result_type is able to extract a
tupdesc from such expressions, which led ExecInitFunctionScan to
ignore the coldeflist. (Instead, it used the extracted tupdesc
to check the function's output, which of course always succeeds.)
I have not been able to demonstrate any really serious consequences
from this, because if some column of the result is of the wrong
type and is directly referenced by a Var of the calling query,
CheckVarSlotCompatibility will catch it. However, we definitely do
fail to report the case where the function returns more columns than
the coldeflist expects, and in the converse case where it returns
fewer columns, we get an assert failure (but, seemingly, no worse
results in non-assert builds).
To fix, always build the expected tupdesc from the coldeflist if there
is one, and consult get_expr_result_type only when there isn't one.
Also remove the failing Assert, even though it is no longer reached
after this fix. It doesn't seem to be adding anything useful, since
later checking will deal with cases with the wrong number of columns.
The only other place I could find that is doing something similar
is inline_set_returning_function. There's no live bug there because
we cannot be looking at a Const or RowExpr, but for consistency
change that code to agree with ExecInitFunctionScan.
Per report from PetSerAl. After some debate I've concluded that
this should be back-patched. There is a small risk that somebody
has been relying on such a case not throwing an error, but I judge
this outweighed by the risk that I've missed some way in which the
failure to cross-check has worse consequences than sketched above.
Discussion: https://postgr.es/m/CAKygsHSerA1eXsJHR9wft3Gn3wfHQ5RfP8XHBzF70_qcrrRvEg@mail.gmail.com
John Naylor [Wed, 6 Mar 2024 08:54:25 +0000 (15:54 +0700)]
Fix signedness error in
9f225e992 for gcc
The first argument of vshrq_n_s8 needs to be a signed vector type,
but it was passed unsigned. Clang is more lax with conversion, but
gcc needs a cast.
Fix by me, tested by Masahiko Sawada
Per buildfarm members splitfin, batta, widowbird, snakefly, parula,
massasauga
Discussion: https://postgr.es/m/
20240306074106.mg6w4koohdlworbs%40alap3.anarazel.de
Michael Paquier [Wed, 6 Mar 2024 08:23:56 +0000 (17:23 +0900)]
Fix parallel-safety check of expressions and predicate for index builds
As coded, the planner logic that calculates the number of parallel
workers to use for a parallel index build uses expressions and
predicates from the relcache, which are flattened for the planner by
eval_const_expressions().
As reported in the bug, an immutable parallel-unsafe function flattened
in the relcache would become a Const, which would be considered as
parallel-safe, even if the predicate or the expressions including the
function are not safe in parallel workers. Depending on the expressions
or predicate used, this could cause the parallel build to fail.
Tests are included that check parallel index builds with parallel-unsafe
predicate and expressions. Two routines are added to lsyscache.h to be
able to retrieve expressions and predicate of an index from its pg_index
data.
Reported-by: Alexander Lakhin
Author: Tender Wang
Reviewed-by: Jian He, Michael Paquier
Discussion: https://postgr.es/m/CAHewXN=UaAaNn9ruHDH3Os8kxLVmtWqbssnf=dZN_s9=evHUFA@mail.gmail.com
Backpatch-through: 12
John Naylor [Tue, 6 Dec 2022 06:39:41 +0000 (13:39 +0700)]
Move some bitmap logic out of bitmapset.c
Move the logic for selecting appropriate pg_bitutils.h
functions based on word size to bitmapset.h for wider
visibility.
Reviewed (in a previous version) by Tom Lane
Discussion: https://postgr.es/m/CAFBsxsFW2JjTo58jtDB%2B3sZhxMx3t-3evew8%3DAcr%2BGGhC%2BkFaA%40mail.gmail.com
John Naylor [Wed, 6 Mar 2024 07:22:15 +0000 (14:22 +0700)]
Introduce helper SIMD functions for small byte arrays
vector8_min - helper for emulating ">=" semantics
vector8_highbit_mask - used to turn the result of a vector
comparison into a bitmask
Masahiko Sawada
Reviewed by Nathan Bossart, with additional adjustments by me
Discussion: https://postgr.es/m/CAFBsxsHbBm_M22gLBO%2BAZT4mfMq3L_oX3wdKZxjeNnT7fHsYMQ%40mail.gmail.com
Amit Kapila [Wed, 6 Mar 2024 05:46:51 +0000 (11:16 +0530)]
Doc: Improve replication slot synchronization section.
Author: Peter Smith
Reviewed-by: Michael Paquier, Laurenz Albe
Discussion: https://postgr.es/m/CAHut+Ps01fV9bjhj6njCFiHu0W0gu1Jcu0tKo0RYMhHx-hZZ7g@mail.gmail.com
Michael Paquier [Wed, 6 Mar 2024 05:38:25 +0000 (14:38 +0900)]
Add recovery TAP test for race condition with slot invalidations
This commit adds a recovery test to provide coverage for the bug fixed
in
818fefd8fd, using an injection point to wait just after the process
of an active slot is killed. The trick is to give enough time for
effective_xmin and effective_catalog_xmin to advance so as the slot
invalidation robustness can be checked since the active process is
killed without holding its slot's mutex for a short time.
Author: Bertrand Drouvot
Discussion: https://postgr.es/m/
[email protected]
Thomas Munro [Tue, 5 Mar 2024 22:39:50 +0000 (11:39 +1300)]
Add --copy-file-range option to pg_upgrade.
The copy_file_range() system call is available on at least Linux and
FreeBSD, and asks the kernel to use efficient ways to copy ranges of a
file. Options available to the kernel include sharing block ranges
(similar to --clone mode), and pushing down block copies to the storage
layer.
For automated testing, see PG_TEST_PG_UPGRADE_MODE. (Perhaps in a later
commit we could consider setting this mode for one of the CI targets.)
Reviewed-by: Peter Eisentraut <[email protected]>
Discussion: https://postgr.es/m/CA%2BhUKGKe7Hb0-UNih8VD5UNZy5-ojxFb3Pr3xSBBL8qj2M2%3DdQ%40mail.gmail.com
David Rowley [Tue, 5 Mar 2024 21:57:31 +0000 (10:57 +1300)]
Remove surplus trailing semicolon
Author: Richard Guo
Discussion: https://postgr.es/m/CAMbWs4-qjotfa7G=5PEOw4LDDDX58MmTwDdpdoU3Quse_BKv1Q@mail.gmail.com
Nathan Bossart [Tue, 5 Mar 2024 20:15:06 +0000 (14:15 -0600)]
Add a few recent commits to .git-blame-ignore-revs.
Jeff Davis [Tue, 5 Mar 2024 19:16:23 +0000 (11:16 -0800)]
Run pgindent again on the same file.
Apparently, pgindent got confused by the double space. The first time
I ran it, it moved the function name to the next line. The second time
I ran it, it moved the function name back, but without the double
space.
Now the results appear stable.
Jeff Davis [Tue, 5 Mar 2024 18:58:24 +0000 (10:58 -0800)]
Run pgindent for commit
ef4cfdce0e.