postgresql.git
4 weeks agoDoc: add information about partition locking
David Rowley [Wed, 2 Apr 2025 01:02:44 +0000 (14:02 +1300)]
Doc: add information about partition locking

The documentation around locking of partitions for the executor startup
phase of run-time partition pruning wasn't clear about which partitions
were being locked.  Fix that.

Reviewed-by: Tender Wang <tndrwang@gmail.com>
Discussion: https://postgr.es/m/CAApHDvp738G75HfkKcfXaf3a8s%3D6mmtOLh46tMD0D2hAo1UCzA%40mail.gmail.com
Backpatch-through: 13

4 weeks agoaio: Add errcontext for processing I/Os for another backend
Melanie Plageman [Tue, 1 Apr 2025 23:53:07 +0000 (19:53 -0400)]
aio: Add errcontext for processing I/Os for another backend

Push an ErrorContextCallback adding additional detail about the process
performing the I/O and the owner of the I/O when those are not the same.

For io_method worker, this adds context specifying which process owns
the I/O that the I/O worker is processing.

For io_method io_uring, this adds context only when a backend is
*completing* I/O for another backend. It specifies the pid of the owning
process.

Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/rdml3fpukrqnas7qc5uimtl2fyytrnu6ymc2vjf2zuflbsjuul%40hyizyjsexwmm

4 weeks agoFix planner's failure to identify multiple hashable ScalarArrayOpExprs
David Rowley [Tue, 1 Apr 2025 22:56:29 +0000 (11:56 +1300)]
Fix planner's failure to identify multiple hashable ScalarArrayOpExprs

50e17ad28 (v14) and 29f45e299 (v15) made it so the planner could identify
IN and NOT IN clauses which have Const lists as right-hand arguments and
when an appropriate hash function is available for the data types, mark
the ScalarArrayOpExpr as hashable so the executor could execute it more
optimally by building and probing a hash table during expression
evaluation.

These commits both worked correctly when there was only a single
ScalarArrayOpExpr in the given expression being processed by the
planner, but when there were multiple, only the first was checked and any
subsequent ones were not identified, which resulted in less optimal
expression evaluation during query execution for all but the first found
ScalarArrayOpExpr.

Backpatch to 14, where 50e17ad28 was introduced.

Author: David Geier <geidav.pg@gmail.com>
Discussion: https://postgr.es/m/29a76f51-97b0-4c07-87b7-ec8e3b5345c9@gmail.com
Backpatch-through: 14

4 weeks agoIntroduce a SQL-callable function array_sort(anyarray).
Tom Lane [Tue, 1 Apr 2025 22:03:55 +0000 (18:03 -0400)]
Introduce a SQL-callable function array_sort(anyarray).

Create a function that will sort the elements of an array
according to the element type's sort order.  If the array
has more than one dimension, the sub-arrays of the first
dimension are sorted per normal array-comparison rules,
leaving their contents alone.

In support of this, add pg_type.typarray to the set of fields
cached by the typcache.

Author: Junwang Zhao <zhjwpku@gmail.com>
Co-authored-by: Jian He <jian.universality@gmail.com>
Reviewed-by: Aleksander Alekseev <aleksander@timescale.com>
Discussion: https://postgr.es/m/CAEG8a3J41a4dpw_-F94fF-JPRXYxw-GfsgoGotKcjs9LVfEEvw@mail.gmail.com

4 weeks agoFix detection and handling of strchrnul() for macOS 15.4.
Tom Lane [Tue, 1 Apr 2025 20:49:51 +0000 (16:49 -0400)]
Fix detection and handling of strchrnul() for macOS 15.4.

As of 15.4, macOS has strchrnul(), but access to it is blocked behind
a check for MACOSX_DEPLOYMENT_TARGET >= 15.4.  But our does-it-link
configure check finds it, so we try to use it, and fail with the
present default deployment target (namely 15.0).  This accounts for
today's buildfarm failures on indri and sifaka.

This is the identical problem that we faced some years ago when Apple
introduced preadv and pwritev in the same way.  We solved that in
commit f014b1b9b by using AC_CHECK_DECLS instead of AC_CHECK_FUNCS
to check the functions' availability.  So do the same now for
strchrnul().  Interestingly, we already had a workaround for
"the link check doesn't agree with <string.h>" cases with glibc,
which we no longer need since only the header declaration is being
checked.

Testing this revealed that the meson version of this check has never
worked, because it failed to use "-Werror=unguarded-availability-new".
(Apparently nobody's tried to build with meson on macOS versions that
lack preadv/pwritev as standard.)  Adjust that while at it.  Also,
we had never put support for "-Werror=unguarded-availability-new"
into v13, but we need that now.

Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us>
Co-authored-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/385134.1743523038@sss.pgh.pa.us
Backpatch-through: 13

4 weeks agoUse workaround of __builtin_setjmp only on MINGW on MSVCRT
Andrew Dunstan [Tue, 1 Apr 2025 20:24:59 +0000 (16:24 -0400)]
Use workaround of __builtin_setjmp only on MINGW on MSVCRT

MSVCRT is not present Windows/ARM64 and the workaround is not
necessary on any UCRT based toolchain.

Author: Lars Kanis <lars@greiz-reinsdorf.de>

Discussion: https://postgr.es/m/CAHXCYb2OjNHtoGVKyXtXmw4B3bUXwJX6M-Lcp1KcMCRUMLOocA@mail.gmail.com

4 weeks agoaio: Minor comment improvements
Andres Freund [Tue, 1 Apr 2025 20:06:48 +0000 (16:06 -0400)]
aio: Minor comment improvements

Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/usbwzckj7q3jhfx3ann3nrfnukmupbs35axvq5zfyeo6nvrzrm@onjhxs2du4st

4 weeks agoaio: Add README.md explaining higher level design
Andres Freund [Tue, 1 Apr 2025 20:06:48 +0000 (16:06 -0400)]
aio: Add README.md explaining higher level design

Reviewed-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://postgr.es/m/uvrtrknj4kdytuboidbhwclo4gxhswwcpgadptsjvjqcluzmah%40brqs62irg4dt
Discussion: https://postgr.es/m/20210223100344.llw5an2aklengrmn@alap3.anarazel.de
Discussion: https://postgr.es/m/stj36ea6yyhoxtqkhpieia2z4krnam7qyetc57rfezgk4zgapf@gcnactj4z56m

4 weeks agodoc: Adjust some notes about pg_upgrade's file transfer modes.
Nathan Bossart [Tue, 1 Apr 2025 19:37:47 +0000 (14:37 -0500)]
doc: Adjust some notes about pg_upgrade's file transfer modes.

--copy-file-range and --swap were not mentioned in a few places
that discuss the available file transfer modes.  This entire page
would likely benefit from an overhaul, but that's v19 material at
this point.

Oversights in commits d93627bcbe and 626d7236b6.

4 weeks agomd: Add comment & assert to buffer-zeroing path in md[start]readv()
Andres Freund [Tue, 1 Apr 2025 17:50:39 +0000 (13:50 -0400)]
md: Add comment & assert to buffer-zeroing path in md[start]readv()

mdreadv() has a codepath to zero out buffers when a read returns zero bytes,
guarded by a check for zero_damaged_pages || InRecovery.

The InRecovery codepath to zero out buffers in mdreadv() appears to be
unreachable. The only known paths to reach mdreadv()/mdstartreadv() in
recovery are XLogReadBufferExtended(), vm_readbuf(), and fsm_readbuf(), each
of which takes care to extend the relation if necessary. This looks to either
have been the case for a long time, or the code was never reachable.

The zero_damaged_pages path is incomplete, as missing segments are not
created.

Putting blocks into the buffer-pool that do not exist on disk is rather
problematic, as such blocks will, at least initially, not be found by scans
that rely on smgrnblocks(), as they are beyond EOF. It also can cause weird
problems with relation extension, as relation extension does not expect blocks
beyond EOF to exist.

Therefore we would like to remove that path.

mdstartreadv(), which I added in e5fe570b51c, does not implement this zeroing
logic. I had started a discussion about that a while ago (linked below), but
forgot to act on the conclusion of the discussion, namely to disable the
in-memory-zeroing behavior.

We could certainly implement equivalent zeroing logic in mdstartreadv(), but
it would have to be more complicated due to potential differences in the
zero_damaged_pages setting between the definer and completor of IO. Given that
we want to remove the logic, that does not seem worth implementing the
necessary logic.

For now, put an Assert(false) and comments documenting this choice into
mdreadv() and comments documenting the deprecation of the path in mdreadv()
and the non-implementation of it in mdstartreadv().  If we, during testing,
discover that we do need the path, we can implement it at that time.

Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/postgr.es/m/20250330024513.ac.nmisch@google.com
Discussion: https://postgr.es/m/postgr.es/m/3qxxsnciyffyf3wyguiz4besdp5t5uxvv3utg75cbcszojlz7p@uibfzmnukkbd

4 weeks agoaio: Add test_aio module
Andres Freund [Tue, 1 Apr 2025 17:47:46 +0000 (13:47 -0400)]
aio: Add test_aio module

To make the tests possible, a few functions from bufmgr.c/localbuf.c had to be
exported, via buf_internals.h.

Reviewed-by: Noah Misch <noah@leadboat.com>
Co-authored-by: Andres Freund <andres@anarazel.de>
Co-authored-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/uvrtrknj4kdytuboidbhwclo4gxhswwcpgadptsjvjqcluzmah%40brqs62irg4dt

4 weeks agoaio: Add pg_aios view
Andres Freund [Tue, 1 Apr 2025 17:30:33 +0000 (13:30 -0400)]
aio: Add pg_aios view

The new view lists all IO handles that are currently in use and is mainly
useful for PG developers, but may also be useful when tuning PG.

Bumps catversion.

Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/uvrtrknj4kdytuboidbhwclo4gxhswwcpgadptsjvjqcluzmah%40brqs62irg4dt

4 weeks agodocs: Add acronym and glossary entries for I/O and AIO
Andres Freund [Tue, 1 Apr 2025 17:30:33 +0000 (13:30 -0400)]
docs: Add acronym and glossary entries for I/O and AIO

These are fairly basic, but better than nothing.  While there are several
opportunities to link to these entries, this patch does not add any. They will
however be referenced by future patches.

Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20250326183102.92.nmisch@google.com

4 weeks agoVerify roundtrip dump/restore of regression database
Álvaro Herrera [Tue, 1 Apr 2025 16:50:40 +0000 (18:50 +0200)]
Verify roundtrip dump/restore of regression database

Add a test to pg_upgrade's test suite that verifies that
dump-restore-dump of regression database produces equivalent output to
dumping it directly.  This was already being tested by running
pg_upgrade itself, but non-binary-upgrade mode was not being covered.

The regression database has accrued, over time, a sufficient collection
of interesting objects to ensure good coverage, but there hasn't been a
concerted effort to be completely exhaustive, so it is likely still
possible to have more.

This'd belong more naturally in the pg_dump test suite, but we chose to
put it in src/bin/pg_upgrade/t/002_pg_upgrade.pl because we need a run
of the regression tests which is already done here, so this has less
total test runtime impact.  Also, experiments have shown that using
parallel dump/restore is slightly faster, so we use --format=directory -j2.

This test has already reported pg_dump bugs, as fixed in fd41ba93e463,
74563f6b9021d611f8b1587b4694aedf63bf.

Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://www.postgresql.org/message-id/CAExHW5uF5V=Cjecx3_Z=7xfh4rg2Wf61PT+hfquzjBqouRzQJQ@mail.gmail.com

4 weeks agoRemove a stray "pgrminclude" annotation
Peter Eisentraut [Tue, 1 Apr 2025 13:27:28 +0000 (15:27 +0200)]
Remove a stray "pgrminclude" annotation

We don't use those anymore.  Fix for commit 8492feb98f6.

4 weeks agoFix minor C type confusion
Peter Eisentraut [Tue, 1 Apr 2025 13:25:42 +0000 (15:25 +0200)]
Fix minor C type confusion

Returning false instead of NULL gets a compiler error under gcc-14
-std=gnu23, and it appears to have been unintentional.  Fix for commit
8492feb98f6.

4 weeks agoheapam: Only set tuple's block once per page in pagemode
Heikki Linnakangas [Tue, 1 Apr 2025 10:24:27 +0000 (13:24 +0300)]
heapam: Only set tuple's block once per page in pagemode

Due to splitting the block id into two 16 bit integers, BlockIdSet()
is more expensive than one might think.  Doing it once per returned
tuple shows up as a small but reliably reproducible cost.  It's simple
enough to set the block number just once per block in pagemode, so do
so.

Author: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/lxzj26ga6ippdeunz6kuncectr5gfuugmm2ry22qu6hcx6oid6@lzx3sjsqhmt6

4 weeks agoUse function attributes for SSE 4.2 even when targeting that extension
John Naylor [Tue, 1 Apr 2025 05:01:58 +0000 (12:01 +0700)]
Use function attributes for SSE 4.2 even when targeting that extension

On Red Hat 9 systems (or similar), the packaged gcc targets x86-64-v2,
but clang does not. This has caused build failures in the wake of
commit e2809e3a1 when building --with-llvm.

The most expedient fix is to use the same function attributes for
the inlined function as we do for the global function.

Reported-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com> (plus members skimmer and bumblebee)
Diagnosed-by: Tom Lane <tgl@sss.pgh.pa.us>
Tested-by: Todd Cook <cookt@blackduck.com>
Discussion: https://postgr.es/m/CANWCAZZSxs3a1YRKehkgk2OHKbrVn+xZ+AWW8Co2R_f70NqqmA@mail.gmail.com

4 weeks agoFix failing regression test on x86-32 machines
David Rowley [Mon, 31 Mar 2025 21:52:25 +0000 (10:52 +1300)]
Fix failing regression test on x86-32 machines

95d6e9af0 added code to display the tuplestore storage type for
WindowAgg nodes and added a test to ensure the "Disk" storage method was
working correctly by setting work_mem to 64 and running a test which
caused the WindowAgg to go to disk.  Seemingly, the number of rows
chosen there wasn't quite enough for that to happen in x86 32-bit.

Fix this by increasing the number of rows slightly.

I suspect the buildfarm didn't catch this as MEMORY_CONTEXT_CHECKING
builds will use a bit more memory for MemoryChunks to store the
requested_size and also because of the additional space to store the
chunk's sentinel byte.

Reported-by: Christoph Berg <myon@debian.org>
Discussion: https://postgr.es/m/Z-q3ZAM4OhE-4UiI@msg.df7cb.de

4 weeks agoFix accidentally-harmless thinko in psqlscan_test_variable().
Tom Lane [Mon, 31 Mar 2025 16:16:10 +0000 (12:16 -0400)]
Fix accidentally-harmless thinko in psqlscan_test_variable().

This code was passing literal strings to psqlscan_emit,
which is quite contrary to that function's specification:
"If you pass it something that is not part of the yytext
string, you are making a mistake".  It accidentally worked
anyway, even in non-safe_encoding mode.  psqlscan_emit
would compute a garbage "reference" pointer, but would
never dereference that since the passed string is all-ASCII.
So there's no live bug today, but that is a happenstance
outcome of psqlscan_emit's current implementation.

Let's make psqlscan_test_variable do what it's supposed to,
namely append directly to the output buffer.  This is just
future-proofing against possible changes in psqlscan_emit,
so I don't feel a need to back-patch.

4 weeks agodoc: Mention clock synchronization recommendation for hot_standby_feedback
Peter Eisentraut [Mon, 31 Mar 2025 14:54:50 +0000 (16:54 +0200)]
doc: Mention clock synchronization recommendation for hot_standby_feedback

hot_standby_feedback mechanics assume that clocks are synchronized,
but it was not clear from documentation.

Author: Jakub Wartak <jakub.wartak@enterprisedb.com>
Reviewed-by: Euler Taveira <euler@eulerto.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: vignesh C <vignesh21@gmail.com>
Discussion: https://postgr.es/m/CAKZiRmwBcALLrDgCyEhHP1enUxtPMjyNM_d1A2Lng3_6Rf4Qfw%40mail.gmail.com

4 weeks agoInline CRC computation for small fixed-length input on x86
John Naylor [Fri, 28 Feb 2025 09:27:30 +0000 (16:27 +0700)]
Inline CRC computation for small fixed-length input on x86

pg_crc32c.h now has a simplified copy of the loop in pg_crc32c_sse42.c
suitable for inlining where possible.

This may slightly reduce contention for the WAL insertion lock,
but that hasn't been tested. The motivation for this change is avoid
regressing for a future commit that will use a function pointer for
non-constant input in all x86 builds.

While it's technically possible to make a similar change for Arm and
LoongArch, there are some questions about how inlining should work
since those platforms prefer stricter alignment. There are also no
immediate plans to add additional implementations for them.

Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Raghuveer Devulapalli <raghuveer.devulapalli@intel.com>
Discussion: https://postgr.es/m/CANWCAZZEiTzhZcuwTiJ2=opiNpAUn1vuDRu1N02z61AthwRZLA@mail.gmail.com
Discussion: https://postgr.es/m/CANWCAZYRhLHArpyfV4uRK-Rw9N5oV5HMkkKtBehcuTjNOMwCZg@mail.gmail.com

4 weeks agoAdd relallfrozen to pg_dump statistics.
Jeff Davis [Mon, 31 Mar 2025 05:14:06 +0000 (22:14 -0700)]
Add relallfrozen to pg_dump statistics.

Author: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://postgr.es/m/CADkLM=desCuf3dVHasADvdUVRmb-5gO0mhMO5u9nzgv6i7U86Q@mail.gmail.com

4 weeks agoEnable IO concurrency on all systems
Andres Freund [Sun, 30 Mar 2025 23:14:55 +0000 (19:14 -0400)]
Enable IO concurrency on all systems

Previously effective_io_concurrency and maintenance_io_concurrency could not
be set above 0 on machines without fadvise support. AIO enables IO concurrency
without such support, via io_method=worker.

Currently only subsystems using the read stream API will take advantage of
this. Other users of maintenance_io_concurrency (like recovery prefetching)
which leverage OS advice directly will not benefit from this change. In those
cases, maintenance_io_concurrency will have no effect on I/O behavior.

Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/CAAKRu_atGgZePo=_g6T3cNtfMf0QxpvoUh5OUqa_cnPdhLd=gw@mail.gmail.com

4 weeks agoread_stream: Introduce and use optional batchmode support
Andres Freund [Sun, 30 Mar 2025 22:30:36 +0000 (18:30 -0400)]
read_stream: Introduce and use optional batchmode support

Submitting IO in larger batches can be more efficient than doing so
one-by-one, particularly for many small reads. It does, however, require
the ReadStreamBlockNumberCB callback to abide by the restrictions of AIO
batching (c.f. pgaio_enter_batchmode()). Basically, the callback may not:
a) block without first calling pgaio_submit_staged(), unless a
   to-be-waited-on lock cannot be part of a deadlock, e.g. because it is
   never held while waiting for IO.

b) directly or indirectly start another batch pgaio_enter_batchmode()

As this requires care and is nontrivial in some cases, batching is only
used with explicit opt-in.

This patch adds an explicit flag (READ_STREAM_USE_BATCHING) to read_stream and
uses it where appropriate.

There are two cases where batching would likely be beneficial, but where we
aren't using it yet:

1) bitmap heap scans, because the callback reads the VM

   This should soon be solved, because we are planning to remove the use of
   the VM, due to that not being sound.

2) The first phase of heap vacuum

   This could be made to support batchmode, but would require some care.

Reviewed-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/uvrtrknj4kdytuboidbhwclo4gxhswwcpgadptsjvjqcluzmah%40brqs62irg4dt

4 weeks agoaio: Basic read_stream adjustments for real AIO
Andres Freund [Sun, 30 Mar 2025 22:26:44 +0000 (18:26 -0400)]
aio: Basic read_stream adjustments for real AIO

Adapt the read stream logic for real AIO:
- If AIO is enabled, we shouldn't issue advice, but if it isn't, we should
  continue issuing advice
- AIO benefits from reading ahead with direct IO
- If effective_io_concurrency=0, pass READ_BUFFERS_SYNCHRONOUSLY to
  StartReadBuffers() to ensure synchronous IO execution

There are further improvements we should consider:

- While in read_stream_look_ahead(), we can use AIO batch submission mode for
  increased efficiency. That however requires care to avoid deadlocks and thus
  done separately.
- It can be beneficial to defer starting new IOs until we can issue multiple
  IOs at once. That however requires non-trivial heuristics to decide when to
  do so.

Reviewed-by: Noah Misch <noah@leadboat.com>
Co-authored-by: Andres Freund <andres@anarazel.de>
Co-authored-by: Thomas Munro <thomas.munro@gmail.com>
4 weeks agodocs: Reframe track_io_timing related docs as wait time
Andres Freund [Sun, 30 Mar 2025 22:04:40 +0000 (18:04 -0400)]
docs: Reframe track_io_timing related docs as wait time

With AIO it does not make sense anymore to track the time for each individual
IO, as multiple IOs can be in-flight at the same time. Instead we now track
the time spent *waiting* for IOs.

This should be reflected in the docs. While, so far, we only do a subset of
reads, and no other operations, via AIO, describing the GUC and view columns
as measuring IO waits is accurate for synchronous and asynchronous IO.

Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/5dzyoduxlvfg55oqtjyjehez5uoq6hnwgzor4kkybkfdgkj7ag@rbi4gsmzaczk

4 weeks agobufmgr: Use AIO in StartReadBuffers()
Andres Freund [Sun, 30 Mar 2025 22:02:23 +0000 (18:02 -0400)]
bufmgr: Use AIO in StartReadBuffers()

This finally introduces the first actual use of AIO. StartReadBuffers() now
uses the AIO routines to issue IO.

As the implementation of StartReadBuffers() is also used by the functions for
reading individual blocks (StartReadBuffer() and through that
ReadBufferExtended()) this means all buffered read IO passes through the AIO
paths.  However, as those are synchronous reads, actually performing the IO
asynchronously would be rarely beneficial. Instead such IOs are flagged to
always be executed synchronously. This way we don't have to duplicate a fair
bit of code.

When io_method=sync is used, the IO patterns generated after this change are
the same as before, i.e. actual reads are only issued in WaitReadBuffers() and
StartReadBuffers() may issue prefetch requests.  This allows to bypass most of
the actual asynchronicity, which is important to make a change as big as this
less risky.

One thing worth calling out is that, if IO is actually executed
asynchronously, the precise meaning of what track_io_timing is measuring has
changed. Previously it tracked the time for each IO, but that does not make
sense when multiple IOs are executed concurrently. Now it only measures the
time actually spent waiting for IO. A subsequent commit will adjust the docs
for this.

While AIO is now actually used, the logic in read_stream.c will often prevent
using sufficiently many concurrent IOs. That will be addressed in the next
commit.

Reviewed-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Co-authored-by: Andres Freund <andres@anarazel.de>
Co-authored-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/uvrtrknj4kdytuboidbhwclo4gxhswwcpgadptsjvjqcluzmah%40brqs62irg4dt
Discussion: https://postgr.es/m/20210223100344.llw5an2aklengrmn@alap3.anarazel.de
Discussion: https://postgr.es/m/stj36ea6yyhoxtqkhpieia2z4krnam7qyetc57rfezgk4zgapf@gcnactj4z56m

4 weeks agobufmgr: Implement AIO read support
Andres Freund [Sun, 30 Mar 2025 21:28:03 +0000 (17:28 -0400)]
bufmgr: Implement AIO read support

This commit implements the infrastructure to perform asynchronous reads into
the buffer pool.

To do so, it:

- Adds readv AIO callbacks for shared and local buffers

  It may be worth calling out that shared buffer completions may be run in a
  different backend than where the IO started.

- Adds an AIO wait reference to BufferDesc, to allow backends to wait for
  in-progress asynchronous IOs

- Adapts StartBufferIO(), WaitIO(), TerminateBufferIO(), and their localbuf.c
  equivalents, to be able to deal with AIO

- Moves the code to handle BM_PIN_COUNT_WAITER into a helper function, as it
  now also needs to be called on IO completion

As of this commit, nothing issues AIO on shared/local buffers. A future commit
will update StartReadBuffers() to do so.

Buffer reads executed through this infrastructure will report invalid page /
checksum errors / warnings differently than before:

In the error case the error message will cover all the blocks that were
included in the read, rather than just the reporting the first invalid
block. If more than one block is invalid, the error will include information
about the range of the read, the first invalid block and the number of invalid
pages, with a HINT towards the server log for per-block details.

For the warning case (i.e. zero_damaged_buffers) we would previously emit one
warning message for each buffer in a multi-block read. Now there is only a
single warning message for the entire read, again referring to the server log
for more details in case of multiple checksum failures within a single larger
read.

Reviewed-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/uvrtrknj4kdytuboidbhwclo4gxhswwcpgadptsjvjqcluzmah%40brqs62irg4dt
Discussion: https://postgr.es/m/20210223100344.llw5an2aklengrmn@alap3.anarazel.de
Discussion: https://postgr.es/m/stj36ea6yyhoxtqkhpieia2z4krnam7qyetc57rfezgk4zgapf@gcnactj4z56m

4 weeks agoaio: Add WARNING result status
Andres Freund [Sun, 30 Mar 2025 20:10:51 +0000 (16:10 -0400)]
aio: Add WARNING result status

If an IO succeeds, but issues a warning, e.g. due to a page verification
failure with zero_damaged_pages, we want to issue that warning in the context
of the issuer of the IO, not the process that executes the completion (always
the case for worker).

It's already possible for a completion callback to report a custom error
message, we just didn't have a result status that allowed a user of AIO to
know that a warning should be emitted even though the IO request succeeded.

All that's needed for that is a dedicated PGAIO_RS_ value.

Previously there were not enough bits in PgAioResult.id for the new
value. Increase. While at that, add defines for the amount of bits and static
asserts to check that the widths are appropriate.

Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20250329212929.a6.nmisch@google.com

4 weeks agoLet caller of PageIsVerified() control ignore_checksum_failure
Andres Freund [Sun, 30 Mar 2025 20:10:51 +0000 (16:10 -0400)]
Let caller of PageIsVerified() control ignore_checksum_failure

For AIO the completion of a read into shared buffers (i.e. verifying the page
including the checksum, updating the BufferDesc to reflect the IO) can happen
in a different backend than the backend that started the IO. As
ignore_checksum_failure can differ between backends, we need to allow the
caller of PageIsVerified() control whether to ignore checksum failures.

The commit leaves a gap in the PIV_* values, as an upcoming commit, which
depends on this commit, will add PIV_LOG_LOG, which better fits just after
PIV_LOG_WARNING.

Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20250329212929.a6.nmisch@google.com

4 weeks agopgstat: Allow checksum errors to be reported in critical sections
Andres Freund [Sun, 30 Mar 2025 20:10:51 +0000 (16:10 -0400)]
pgstat: Allow checksum errors to be reported in critical sections

For AIO we execute completion callbacks in critical sections (to ensure that
AIO can in the future be used for WAL, which in turn requires that we can call
completion callbacks in critical sections, to get the resources for WAL
io). To report checksum errors a backend now has to call
pgstat_prepare_report_checksum_failure(), before entering a critical section,
which guarantees the relevant pgstats entry is in shared memory, the relevant
DSM segment is mapped into the backend's memory and the address is known via a
PgStat_EntryRef.

Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/wkjj4p2rmkevutkwc6tewoovdqznj6c6nvjmvii4oo5wmbh5sr@retq7d6uqs4j

4 weeks agoAdd errhint_internal()
Andres Freund [Sun, 30 Mar 2025 20:10:51 +0000 (16:10 -0400)]
Add errhint_internal()

We have errmsg_internal(), errdetail_internal(), but not errhint_internal().

Sometimes it is useful to output a hint with already translated format
string (e.g. because there different messages depending on the condition). For
message/detail we do that with the _internal() variants, but we can't do that
with hint today.  It's possible to work around that that by using something
like
  str = psprintf(translated_format, args);
  ereport(...
          errhint("%s", str);
but that's not exactly pretty and makes it harder to avoid memory leaks.

Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/ym3dqpa4xcvoeknewcw63x77vnqdosbqcetjinb2zfoh65k55m@m4ozmwhr6lk6

4 weeks agoRemove incidental md5() function use from test
Tomas Vondra [Sun, 30 Mar 2025 11:22:39 +0000 (13:22 +0200)]
Remove incidental md5() function use from test

Replace md5() with sha256() in tests introduced in 14ffaece0fb5, to
allow test to pass in OpenSSL FIPS mode.

Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/3518736.1743307492@sss.pgh.pa.us

4 weeks agolocalbuf: Track pincount in BufferDesc as well
Andres Freund [Sat, 29 Mar 2025 20:36:51 +0000 (16:36 -0400)]
localbuf: Track pincount in BufferDesc as well

For AIO on temporary table buffers the AIO subsystem needs to be able to
ensure a pin on a buffer while AIO is going on, even if the IO issuing query
errors out. Tracking the buffer in LocalRefCount does not work, as it would
cause CheckForLocalBufferLeaks() to assert out.

Instead, also track the refcount in BufferDesc.state, not just
LocalRefCount. This also makes local buffers behave a bit more akin to shared
buffers.

Note that we still don't need locking, AIO completion callbacks for local
buffers are executed in the issuing session (i.e. nobody else has access to
the BufferDesc).

Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/uvrtrknj4kdytuboidbhwclo4gxhswwcpgadptsjvjqcluzmah%40brqs62irg4dt

4 weeks agoaio, bufmgr: Comment fixes/improvements
Andres Freund [Sat, 29 Mar 2025 18:45:42 +0000 (14:45 -0400)]
aio, bufmgr: Comment fixes/improvements

Some of these comments have been wrong for a while (12f3867f5534), some I
recently introduced (da7226993fd55b454d0e14). This includes an update to a
comment in FlushBuffer(), which will be copied in a future commit.

These changes seem big enough to be worth doing in separate commits.

Suggested-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20250319212530.80.nmisch@google.com

4 weeks agoaio: Implement support for reads in smgr/md/fd
Andres Freund [Sat, 29 Mar 2025 17:38:35 +0000 (13:38 -0400)]
aio: Implement support for reads in smgr/md/fd

This implements the following:

1) An smgr AIO target, for AIO on smgr files. This should be usable not just
   for md.c but also other SMGR implementation if we ever get them.
2) readv support in fd.c, which requires a small bit of infrastructure work in
   fd.c
3) smgr.c and md.c support for readv

There still is nothing performing AIO, but as of this commit it would be
possible.

As part of this change FileGetRawDesc() actually ensures that the file is
opened - previously it was basically not usable. It's used to reopen a file in
IO workers.

Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/uvrtrknj4kdytuboidbhwclo4gxhswwcpgadptsjvjqcluzmah%40brqs62irg4dt
Discussion: https://postgr.es/m/20210223100344.llw5an2aklengrmn@alap3.anarazel.de
Discussion: https://postgr.es/m/stj36ea6yyhoxtqkhpieia2z4krnam7qyetc57rfezgk4zgapf@gcnactj4z56m

4 weeks agoFix mis-attribution of checksum failure stats to the wrong database
Andres Freund [Sat, 29 Mar 2025 17:38:35 +0000 (13:38 -0400)]
Fix mis-attribution of checksum failure stats to the wrong database

Checksum failure stats could be attributed to the wrong database in two cases:

- when a read of a shared relation encountered a checksum error , it would be
  attributed to the current database, instead of the "database" representing
  shared relations

- when using CREATE DATABASE ... STRATEGY WAL_LOG checksum errors in the
  source database would be attributed to the current database

The checksum stats reporting via PageIsVerifiedExtended(PIV_REPORT_STAT) does
not have access to the information about what database a page belongs to.

This fixes the issue by removing PIV_REPORT_STAT and delegating the
responsibility to report stats to the caller, which now can learn about the
number of stats via a new optional argument.

As this changes the signature of PageIsVerifiedExtended() and all callers
should adapt to the new signature, use the occasion to rename the function to
PageIsVerified() and remove the compatibility macro.

We could instead have fixed this by adding information about the database to
the args of PageIsVerified(), but there are soon-to-be-applied patches that
need to separate the stats reporting from the PageIsVerified() call
anyway. Those patches also include testing for the failure paths, something we
inexplicably have not had.

As there is no caller of pgstat_report_checksum_failure() left, remove it.

It'd be possible, but awkward to fix this in the back branches. We considered
doing the work not quite worth it, as mis-attributed stats should still elicit
concern. The emitted error messages do allow to attribute the errors
correctly.

Discussion: https://postgr.es/m/5tyic6epvdlmd6eddgelv47syg2b5cpwffjam54axp25xyq2ga@ptwkinxqo3az
Discussion: https://postgr.es/m/mglpvvbhighzuwudjxzu4br65qqcxsnyvio3nl4fbog3qknwhg@e4gt7npsohuz

4 weeks agoamcheck: Add a GIN index to the CREATE INDEX CONCURRENTLY tests
Tomas Vondra [Sat, 29 Mar 2025 15:46:49 +0000 (16:46 +0100)]
amcheck: Add a GIN index to the CREATE INDEX CONCURRENTLY tests

The existing CREATE INDEX CONCURRENTLY tests checking only B-Tree, but
can be cheaply extended to also check GIN. This helps increasing test
coverage for GIN amcheck, especially related to handling concurrent page
splits and posting list trees.

This already helped to identify several issues during development of the
GIN amcheck support.

Author: Mark Dilger <mark.dilger@enterprisedb.com>
Reviewed-By: Tomas Vondra <tomas.vondra@enterprisedb.com>
Reviewed-By: Kirill Reshke <reshkekirill@gmail.com>
Discussion: https://postgr.es/m/BC221A56-977C-418E-A1B8-9EFC881D80C5%40enterprisedb.com

4 weeks agoamcheck: Add a test with GIN index on JSONB data
Tomas Vondra [Sat, 29 Mar 2025 15:46:34 +0000 (16:46 +0100)]
amcheck: Add a test with GIN index on JSONB data

Extend the existing test of GIN checks to also include an index on JSONB
data, using the jsonb_path_ops opclass. This is a common enough usage of
GIN that it makes sense to have better test coverage for it.

Author: Mark Dilger <mark.dilger@enterprisedb.com>
Reviewed-By: Tomas Vondra <tomas.vondra@enterprisedb.com>
Reviewed-By: Kirill Reshke <reshkekirill@gmail.com>
Discussion: https://postgr.es/m/BC221A56-977C-418E-A1B8-9EFC881D80C5%40enterprisedb.com

4 weeks agoamcheck: Fix indentation in verify_gin.c
Tomas Vondra [Sat, 29 Mar 2025 15:42:13 +0000 (16:42 +0100)]
amcheck: Fix indentation in verify_gin.c

I forgot to reindent the code after a couple last-minute adjustments
just before committing 14ffaece0fb53fed8ddbc46d2b353e1c4834863a.

Discussion: https://postgr.es/m/45AC9B0A-2B45-40EE-B08F-BDCF5739D1E1%40yandex-team.ru

4 weeks agoFix "‘static’ is not at beginning of declaration" warning
Andres Freund [Sat, 29 Mar 2025 14:48:59 +0000 (10:48 -0400)]
Fix "‘static’ is not at beginning of declaration" warning

b98be8a2a2a used "const static" instead of "static const". We normally use the
latter form.

Discussion: https://postgr.es/m/z4mc2hzecahyq3paupfsouhuupmzmgum45md3k5my6bmo7gvn7@z5j26doqamqy

4 weeks agoamcheck: Add gin_index_check() to verify GIN index
Tomas Vondra [Sat, 29 Mar 2025 14:43:55 +0000 (15:43 +0100)]
amcheck: Add gin_index_check() to verify GIN index

Adds a new function, validating two kinds of invariants on a GIN index:

- parent-child consistency: Paths in a GIN graph have to contain
  consistent keys. Tuples on parent pages consistently include tuples
  from child pages; parent tuples do not require any adjustments.

- balanced-tree / graph: Each internal page has at least one downlink,
  and can reference either only leaf pages or only internal pages.

The GIN verification is based on work by Grigory Kryachko, reworked by
Heikki Linnakangas and with various improvements by Andrey Borodin.
Investigation and fixes for multiple bugs by Kirill Reshke.

Author: Grigory Kryachko <GSKryachko@gmail.com>
Author: Heikki Linnakangas <hlinnaka@iki.fi>
Author: Andrey Borodin <amborodin@acm.org>
Reviewed-By: José Villanova <jose.arthur@gmail.com>
Reviewed-By: Aleksander Alekseev <aleksander@timescale.com>
Reviewed-By: Nikolay Samokhvalov <samokhvalov@gmail.com>
Reviewed-By: Andres Freund <andres@anarazel.de>
Reviewed-By: Tomas Vondra <tomas.vondra@enterprisedb.com>
Reviewed-By: Kirill Reshke <reshkekirill@gmail.com>
Reviewed-By: Mark Dilger <mark.dilger@enterprisedb.com>
Reviewed-By: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/45AC9B0A-2B45-40EE-B08F-BDCF5739D1E1%40yandex-team.ru

4 weeks agopgbench: Make set_random_seed() 64-bit everywhere.
Peter Eisentraut [Sat, 29 Mar 2025 14:24:42 +0000 (15:24 +0100)]
pgbench: Make set_random_seed() 64-bit everywhere.

Delete an intermediate variable, a redundant cast, a use of long and a
use of long long.  scanf() the seed directly into a uint64, now that we
can do that with SCNu64 from <inttypes.h>.

The previous coding was from pre-C99 times when %lld might not have been
there, so it read into an unsigned long.  Therefore behavior varied
by OS, and --random-seed would accept either 32 or 64 bit seeds.  Now
it's the same everywhere.

Author: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/b936d2fb-590d-49c3-a615-92c3a88c6c19%40eisentraut.org

4 weeks agoamcheck: Move common routines into a separate module
Tomas Vondra [Sat, 29 Mar 2025 14:14:47 +0000 (15:14 +0100)]
amcheck: Move common routines into a separate module

Before performing checks on an index, we need to take some safety
measures that apply to all index AMs. This includes:

* verifying that the index can be checked - Only selected AMs are
supported by amcheck (right now only B-Tree). The index has to be
valid and not a temporary index from another session.

* changing (and then restoring) user's security context

* obtaining proper locks on the index (and table, if needed)

* discarding GUC changes from the index functions

Until now this was implemented in the B-Tree amcheck module, but it's
something every AM will have to do. So relocate the code into a new
module verify_common for reuse.

The shared steps are implemented by amcheck_lock_relation_and_check(),
receiving the AM-specific verification as a callback. Custom parameters
may be supplied using a pointer.

Author: Andrey Borodin <amborodin@acm.org>
Reviewed-By: José Villanova <jose.arthur@gmail.com>
Reviewed-By: Aleksander Alekseev <aleksander@timescale.com>
Reviewed-By: Nikolay Samokhvalov <samokhvalov@gmail.com>
Reviewed-By: Andres Freund <andres@anarazel.de>
Reviewed-By: Tomas Vondra <tomas@vondra.me>
Reviewed-By: Mark Dilger <mark.dilger@enterprisedb.com>
Reviewed-By: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Kirill Reshke <reshkekirill@gmail.com>
Discussion: https://postgr.es/m/45AC9B0A-2B45-40EE-B08F-BDCF5739D1E1%40yandex-team.ru

4 weeks agoFix grammar in GIN README
Tomas Vondra [Sat, 29 Mar 2025 14:14:09 +0000 (15:14 +0100)]
Fix grammar in GIN README

Author: Kirill Reshke <reshkekirill@gmail.com>
Discussion: https://postgr.es/m/CALdSSPgu9uAhVYojQ0yjG%3Dq5MaqmiSLUJPhz%2B-u7cA6K6Mc9UA%40mail.gmail.com

4 weeks agoFix MERGE with DO NOTHING actions into a partitioned table.
Dean Rasheed [Sat, 29 Mar 2025 09:58:40 +0000 (09:58 +0000)]
Fix MERGE with DO NOTHING actions into a partitioned table.

ExecInitPartitionInfo() duplicates much of the logic in
ExecInitMerge(), except that it failed to handle DO NOTHING
actions. This would cause an "unknown action in MERGE WHEN clause"
error if a MERGE with any DO NOTHING actions attempted to insert into
a partition not already initialised by ExecInitModifyTable().

Bug: #18871
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Author: Tender Wang <tndrwang@gmail.com>
Reviewed-by: Gurjeet Singh <gurjeet@singh.im>
Discussion: https://postgr.es/m/18871-b44e3c96de3bd2e8%40postgresql.org
Backpatch-through: 15

4 weeks agoUse PRI?64 instead of "ll?" in format strings (continued).
Peter Eisentraut [Sat, 29 Mar 2025 09:30:08 +0000 (10:30 +0100)]
Use PRI?64 instead of "ll?" in format strings (continued).

Continuation of work started in commit 15a79c73, after initial trial.

Author: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/b936d2fb-590d-49c3-a615-92c3a88c6c19%40eisentraut.org

4 weeks agoMatview statistics depend on matview data.
Jeff Davis [Fri, 28 Mar 2025 23:12:55 +0000 (16:12 -0700)]
Matview statistics depend on matview data.

REFRESH MATERIALIZED VIEW replaces the storage, which resets
statistics, so statistics must be restored afterward.

If both statistics and data are being dumped for a materialized view,
add a dependency from the former to the latter. Defer the statistics
to SECTION_POST_DATA, and use RESTORE_PASS_POST_ACL.

Reported-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://postgr.es/m/CAExHW5s47kmubpbbRJzSM-Zfe0Tj2O3GBagB7YAyE8rQ-V24Uw@mail.gmail.com

4 weeks agoMake group_similar_or_args() reorder clause list as little as possible
Alexander Korotkov [Fri, 28 Mar 2025 21:36:49 +0000 (23:36 +0200)]
Make group_similar_or_args() reorder clause list as little as possible

Currently, group_similar_or_args() permutes original positions of clauses
independently on whether it manages to find any groups of similar clauses.
While we are not providing any strict warranties on saving the original order
of OR-clauses, it is preferred that the original order be modified as little
as possible.

This commit changes the reordering algorithm of group_similar_or_args() in
the following way.  We reorder each group of similar clauses so that the
first item of the group stays in place, but all the other items are moved
after it.  So, if there are no similar clauses, the order of clauses stays
the same.  When there are some groups, only required reordering happens while
the rest of the clauses remain in their places.

Reported-by: Andrei Lepikhov <lepihov@gmail.com>
Discussion: https://postgr.es/m/3ac7c436-81e1-4191-9caf-b0dd70b51511%40gmail.com
Reviewed-by: Pavel Borisov <pashkin.elfe@gmail.com>
Reviewed-by: Andrei Lepikhov <lepihov@gmail.com>
Reviewed-by: Alena Rybakina <a.rybakina@postgrespro.ru>
4 weeks agoOptimize popcount functions with ARM SVE intrinsics.
Nathan Bossart [Fri, 28 Mar 2025 21:20:20 +0000 (16:20 -0500)]
Optimize popcount functions with ARM SVE intrinsics.

This commit introduces SVE implementations of pg_popcount{32,64}.
Unlike the Neon versions, we need an additional configure-time
check to determine if the compiler supports SVE intrinsics, and we
need a runtime check to determine if the current CPU supports SVE
instructions.  Our testing showed that the SVE implementations are
much faster for larger inputs and are comparable to the status
quo for smaller inputs.

Author: "Devanga.Susmitha@fujitsu.com" <Devanga.Susmitha@fujitsu.com>
Co-authored-by: "Chiranmoy.Bhattacharya@fujitsu.com" <Chiranmoy.Bhattacharya@fujitsu.com>
Co-authored-by: "Malladi, Rama" <ramamalladi@hotmail.com>
Reviewed-by: John Naylor <johncnaylorls@gmail.com>
Reviewed-by: Kirill Reshke <reshkekirill@gmail.com>
Discussion: https://postgr.es/m/010101936e4aaa70-b474ab9e-b9ce-474d-a3ba-a3dc223d295c-000000%40us-west-2.amazonses.com
Discussion: https://postgr.es/m/OSZPR01MB84990A9A02A3515C6E85A65B8B2A2%40OSZPR01MB8499.jpnprd01.prod.outlook.com

4 weeks agoRevert "Tidy up locale thread safety in ECPG library."
Peter Eisentraut [Fri, 28 Mar 2025 20:27:37 +0000 (21:27 +0100)]
Revert "Tidy up locale thread safety in ECPG library."

This reverts commit 8e993bff5326b00ced137c837fce7cd1e0ecae14.

It causes various build failures on the buildfarm, to be investigated.

Discussion: https://postgr.es/m/CWZBBRR6YA8D.8EHMDRGLCKCD%40neon.tech

4 weeks agoOptimize popcount functions with ARM Neon intrinsics.
Nathan Bossart [Fri, 28 Mar 2025 19:49:35 +0000 (14:49 -0500)]
Optimize popcount functions with ARM Neon intrinsics.

This commit introduces Neon implementations of pg_popcount{32,64},
pg_popcount(), and pg_popcount_masked().  As in simd.h, we assume
that all available AArch64 hardware supports Neon, so we don't need
any new configure-time or runtime checks.  Some compilers already
emit Neon instructions for these functions, but our hand-rolled
implementations for pg_popcount() and pg_popcount_masked()
performed better in testing, likely due to better instruction-level
parallelism.

Author: "Chiranmoy.Bhattacharya@fujitsu.com" <Chiranmoy.Bhattacharya@fujitsu.com>
Reviewed-by: John Naylor <johncnaylorls@gmail.com>
Discussion: https://postgr.es/m/010101936e4aaa70-b474ab9e-b9ce-474d-a3ba-a3dc223d295c-000000%40us-west-2.amazonses.com

4 weeks agoFix crash if LockErrorCleanup() is called twice
Heikki Linnakangas [Fri, 28 Mar 2025 18:19:17 +0000 (20:19 +0200)]
Fix crash if LockErrorCleanup() is called twice

The refactoring in commit 3c0fd64fec removed the clearing of
awaitedLock from LockErrorCleanup(). It's still needed, otherwise
LockErrorCleanup() during abort processing will try to update the
LOCALLOCK struct even after the lock has already been released. Put it
back.

Reported-by: Richard Guo <guofenglinux@gmail.com>
Reported-by: Robins Tharakan <tharakan@gmail.com>
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAMbWs4_dNX1SzBmvFdoY-LxJh_4W_BjtVd5i008ihfU-wFF=eg@mail.gmail.com
Discussion: https://www.postgresql.org/message-id/18832-38e5575b1bbd7277@postgresql.org
Discussion: https://www.postgresql.org/message-id/e11a30e5-c0d8-491d-8546-3a1b50c10ad4@gmail.com

4 weeks agoRename TRY_POPCNT_FAST to TRY_POPCNT_X86_64.
Nathan Bossart [Fri, 28 Mar 2025 17:27:47 +0000 (12:27 -0500)]
Rename TRY_POPCNT_FAST to TRY_POPCNT_X86_64.

This macro protects x86_64-specific code, and a subsequent commit
will introduce AArch64-specific versions of that code.  To prevent
confusion, let's rename it to clearly indicate that it's for
x86_64.  We should likely move this code to its own file (perhaps
merging it with the AVX-512 popcount code), but that is left as a
future exercise.

Reviewed-by: "Chiranmoy.Bhattacharya@fujitsu.com" <Chiranmoy.Bhattacharya@fujitsu.com>
Reviewed-by: John Naylor <johncnaylorls@gmail.com>
Discussion: https://postgr.es/m/010101936e4aaa70-b474ab9e-b9ce-474d-a3ba-a3dc223d295c-000000%40us-west-2.amazonses.com

5 weeks agoFix timestamp overflow in UUIDv7 implementation.
Masahiko Sawada [Fri, 28 Mar 2025 16:39:11 +0000 (09:39 -0700)]
Fix timestamp overflow in UUIDv7 implementation.

The uuidv7_interval() function previously converted a shifted
microsecond-precision timestamp (64-bit integer) to another 64-bit
integer representing a timestamp with nanosecond precision. This
conversion caused overflow for dates beyond the year 2262. The
millisecond and sub-millisecond parts were then extracted from this
nanosecond-precision timestamp and stored in UUIDv7 values.

With this commit, the millisecond and sub-millisecond parts are stored
directly into the UUIDv7 value without being converted back to a
nanosecond precision timestamp. Following RFC 9562, the timestamp is
stored as an unsigned integer, enabling support for dates up to the
year 10889.

Reported and fixed by Andrey Borodin, with cosmetic changes and
regression tests by me.

Reported-by: Andrey Borodin <x4mmm@yandex-team.ru>
Author: Andrey Borodin <x4mmm@yandex-team.ru>
Discussion: https://postgr.es/m/96DEC2D9-659A-40E8-B7BA-AF5D162A9E21@yandex-team.ru

5 weeks agoTidy up locale thread safety in ECPG library.
Peter Eisentraut [Fri, 28 Mar 2025 15:15:57 +0000 (16:15 +0100)]
Tidy up locale thread safety in ECPG library.

Remove setlocale() and _configthreadlocal() as fallback strategy on
systems that don't have uselocale(), where ECPG tries to control
LC_NUMERIC formatting on input and output of floating point numbers.  It
was probably broken on some systems (NetBSD), and the code was also
quite messy and complicated, with obsolete configure tests (Windows).
It was also arguably broken, or at least had unstated environmental
requirements, if pgtypeslib code was called directly.

Instead, introduce PG_C_LOCALE to refer to the "C" locale as a locale_t
value.  It maps to the special constant LC_C_LOCALE when defined by libc
(macOS, NetBSD), or otherwise uses a process-lifetime locale_t that is
allocated on first use, just as ECPG previously did itself.  The new
replacement might be more widely useful.  Then change the float parsing
and printing code to pass that to _l() functions where appropriate.

Unfortunately the portability of those functions is a bit complicated.
First, many obvious and useful _l() functions are missing from POSIX,
though most standard libraries define some of them anyway.  Second,
although the thread-safe save/restore technique can be used to replace
the missing ones, Windows and NetBSD refused to implement standard
uselocale().  They might have a point: "wide scope" uselocale() is hard
to combine with other code and error-prone, especially in library code.
Luckily they have the  _l() functions we want so far anyway.  So we have
to be prepared for both ways of doing things:

1.  In ECPG, use strtod_l() for parsing, and supply a port.h replacement
using uselocale() over a limited scope if missing.

2.  Inside our own snprintf.c, use three different approaches to format
floats.  For frontend code, call libc's snprintf_l(), or wrap libc's
snprintf() in uselocale() if it's missing.  For backend code, snprintf.c
can keep assuming that the global locale's LC_NUMERIC is "C" and call
libc's snprintf() without change, for now.

(It might eventually be possible to call our in-tree Ryū routines to
display floats in snprintf.c, given the C-locale-always remit of our
in-tree snprintf(), but this patch doesn't risk changing anything that
complicated.)

Author: Thomas Munro <thomas.munro@gmail.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Tristan Partin <tristan@partin.io>
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://postgr.es/m/CWZBBRR6YA8D.8EHMDRGLCKCD%40neon.tech

5 weeks agoCast result of i64abs() back to int64
Peter Eisentraut [Fri, 28 Mar 2025 13:05:45 +0000 (14:05 +0100)]
Cast result of i64abs() back to int64

Without the cast, the return type could be long or long long,
depending on what int64 is underneath.  This doesn't affect code
correctness, but it could result in format-mismatch warnings when
attempting to printf such values using PRId64.

Reported-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CA+hUKGJc4s+Wyb3EFOQNN9VVK+Qv40r2LK41o9PkS9ThxviTvQ@mail.gmail.com

5 weeks agopg_overexplain: Use PG_MODULE_MAGIC_EXT.
Robert Haas [Fri, 28 Mar 2025 13:16:29 +0000 (09:16 -0400)]
pg_overexplain: Use PG_MODULE_MAGIC_EXT.

I committed this contrib module just after Tom committed
55527368bd07248e91e3d37a782bf66b76f06865; adjust it to match.

Author: Man Zeng <zengman@halodbtech.com>
Discussion: http://postgr.es/m/174313513707.60295.16516085012903412705.pgcf@coridan.postgresql.org

5 weeks agopg_overexplain: Call previous hooks as appropriate.
Robert Haas [Fri, 28 Mar 2025 12:59:33 +0000 (08:59 -0400)]
pg_overexplain: Call previous hooks as appropriate.

It makes no sense to remember the previous values of the hook variables
and then never bother calling those functions. Thanks to Andrei for
spotting my goof.

Author: Andrei Lepikhov <lepihov@gmail.com>
Discussion: http://postgr.es/m/41a344e3-ffb1-4296-8ba7-801f1e9642e5@gmail.com

5 weeks agoAdd support for not-null constraints on virtual generated columns
Peter Eisentraut [Fri, 28 Mar 2025 12:53:37 +0000 (13:53 +0100)]
Add support for not-null constraints on virtual generated columns

This was left out of the original patch for virtual generated columns
(commit 83ea6c54025).

This just involves a bit of extra work in the executor to expand the
generation expressions and run a "IS NOT NULL" test against them.

There is also a bit of work to make sure that not-null constraints are
checked during a table rewrite.

Author: jian he <jian.universality@gmail.com>
Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>
Reviewed-by: Navneet Kumar <thanit3111@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/CACJufxHArQysbDkWFmvK+D1TPHQWWTxWN15cMuUaTYX3xhQXgg@mail.gmail.com

5 weeks agoModernize some code a bit
Peter Eisentraut [Fri, 28 Mar 2025 09:49:15 +0000 (10:49 +0100)]
Modernize some code a bit

Modernize code in ExecRelCheck() and ExecConstraints() a bit,
preparing the way for some new code.

Co-authored-by: jian he <jian.universality@gmail.com>
Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>
Reviewed-by: Navneet Kumar <thanit3111@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/CACJufxHArQysbDkWFmvK+D1TPHQWWTxWN15cMuUaTYX3xhQXgg@mail.gmail.com

5 weeks agoRename a node field for clarity
Peter Eisentraut [Fri, 28 Mar 2025 08:50:01 +0000 (09:50 +0100)]
Rename a node field for clarity

Rename ResultRelInfo.ri_ConstraintExprs to ri_CheckConstraintExprs.
This reflects its specific purpose better and avoids confusion with
adjacent fields with similar but distinct purposes.

Discussion: https://postgr.es/m/CACJufxHArQysbDkWFmvK+D1TPHQWWTxWN15cMuUaTYX3xhQXgg@mail.gmail.com

5 weeks agopg_createsubscriber: Add '--all' option.
Amit Kapila [Fri, 28 Mar 2025 06:56:39 +0000 (12:26 +0530)]
pg_createsubscriber: Add '--all' option.

The '--all' option indicates that the tool queries the source server
(publisher) for all databases and creates subscriptions on the target
server (subscriber) for databases with matching names. Without this user
needs to explicitly specify all databases by using -d option for each
database.

This simplifies converting a physical standby to a logical subscriber,
particularly during upgrades.

The options '--database', '--publication', '--subscription', and
'--replication-slot' cannot be used when '--all' is specified.

Author: Shubham Khanna <khannashubham1197@gmail.com>
Reviewed-by: vignesh C <vignesh21@gmail.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: Euler Taveira <euler@eulerto.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Reviewed-by: Shlok Kyal <shlok.kyal.oss@gmail.com>
Discussion: https://postgr.es/m/CAHv8RjKhA=_h5vAbozzJ1Opnv=KXYQHQ-fJyaMfqfRqPpnC2bA@mail.gmail.com

5 weeks agoUse thread-safe strftime_l() instead of strftime().
Peter Eisentraut [Fri, 28 Mar 2025 06:13:43 +0000 (07:13 +0100)]
Use thread-safe strftime_l() instead of strftime().

This removes some setlocale() calls and a lot of commentary about how
dangerous that is.  strftime_l() is from POSIX 2008, and on Windows we
use _wcsftime_l().

While here, adjust error message for strftime_l() failure: it does not
in practice set errno (even though POSIX says it could), so no %m.

Author: Thomas Munro <thomas.munro@gmail.com>
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/CA%2BhUKGJqVe0%2BPv9dvC9dSums_PXxGo9SWcxYAMBguWJUGbWz-A%40mail.gmail.com

5 weeks agoStablize tests added in 3abe9dc188.
Amit Kapila [Fri, 28 Mar 2025 05:33:05 +0000 (11:03 +0530)]
Stablize tests added in 3abe9dc188.

The problem is that after the ALTER SUBSCRIPTION tap_sub SET PUBLICATION
command, we didn't wait for the new walsender to start on the publisher.
Immediately after ALTER, we performed Insert and expected it to replicate.
However, the replication could start from a point after the INSERT location,
and as the subscription isn't copying initial data, we could miss such an
Insert.

The fix is to wait for connection to be established between publisher and
subscriber before starting DML operations that are expected to replicate.

As per CI.

Reported-by: Andres Freund <andres@anarazel.de>
Author: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Discussion: https://postgr.es/m/CALDaNm2ms1deM5EYNLFEfESv_Kw=Y4AiTB0LP=qGS-UpFwGbPg@mail.gmail.com

5 weeks agoFix guc_malloc calls for consistency and OOM checks
Daniel Gustafsson [Thu, 27 Mar 2025 21:57:34 +0000 (22:57 +0100)]
Fix guc_malloc calls for consistency and OOM checks

check_createrole_self_grant and check_synchronized_standby_slots
were allocating memory on a LOG elevel without checking if the
allocation succeeded or not, which would have led to a segfault
on allocation failure.

On top of that, a number of callsites were using the ERROR level,
relying on erroring out rather than returning false to allow the
GUC machinery handle it gracefully.  Other callsites used WARNING
instead of LOG.  While neither being not wrong, this changes all
check_ functions do it consistently with LOG.

init_custom_variable gets a promoted elevel to FATAL to keep
the guc_malloc error handling in line with the rest of the
error handling in that function which already call FATAL.  If
we encounter an OOM in this callsite there is no graceful
handling to be had, better to error out hard.

Backpatch the fix to check_createrole_self_grant down to v16
and the fix to check_synchronized_standby_slots down to v17
where they were introduced.

Author: Daniel Gustafsson <daniel@yesql.se>
Reported-by: Nikita <pm91.arapov@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Bug: #18845
Discussion: https://postgr.es/m/18845-582c6e10247377ec@postgresql.org
Backpatch-through: 16

5 weeks agoUse streaming read I/O in heap amcheck
Melanie Plageman [Thu, 27 Mar 2025 18:02:40 +0000 (14:02 -0400)]
Use streaming read I/O in heap amcheck

Instead of directly invoking ReadBuffer() for each unskippable block in
the heap relation, verify_heapam() now uses the read stream API to
acquire the next buffer to check for corruption.

Author: Matheus Alcantara <matheusssilv97@gmail.com>
Co-authored-by: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Reviewed-by: Kirill Reshke <reshkekirill@gmail.com>
Reviewed-by: jian he <jian.universality@gmail.com>
Discussion: https://postgr.es/m/flat/CAFY6G8eLyz7%2BsccegZYFj%3D5tAUR-GZ9uEq4Ch5gvwKqUwb_hCA%40mail.gmail.com

5 weeks agoPrevent assertion failure in contrib/pg_freespacemap.
Tom Lane [Thu, 27 Mar 2025 17:20:23 +0000 (13:20 -0400)]
Prevent assertion failure in contrib/pg_freespacemap.

Applying pg_freespacemap() to a relation lacking storage (such as a
view) caused an assertion failure, although there was no ill effect
in non-assert builds.  Add an error check for that case.

Bug: #18866
Reported-by: Robins Tharakan <tharakan@gmail.com>
Author: Tender Wang <tndrwang@gmail.com>
Reviewed-by: Euler Taveira <euler@eulerto.com>
Discussion: https://postgr.es/m/18866-d68926d0f1c72d44@postgresql.org
Backpatch-through: 13

5 weeks agoAvoid mixing designated and non-designated field initializers.
Tom Lane [Thu, 27 Mar 2025 15:06:30 +0000 (11:06 -0400)]
Avoid mixing designated and non-designated field initializers.

As revised by commit 9324c8c58, PG_MODULE_MAGIC constructed a
struct initializer containing both designated fields and a
non-designated "0".  That's okay in C, but not in C++, with
the result that extensions written in C++ failed to compile.
Change it to use only designated field initializers.

Author: Yurii Rashkovskii <yrashk@omnigres.com>
Discussion: https://postgr.es/m/CAG=VW14mctsR543gpzLCuJ9JgJqwa=ptmBfGvxEjs+k8Jf7-Bg@mail.gmail.com

5 weeks agopsql: Fix incorrect equality comparison
Daniel Gustafsson [Thu, 27 Mar 2025 13:09:25 +0000 (14:09 +0100)]
psql: Fix incorrect equality comparison

Commit 1a759c83278 contained an incorrect equality comparison
which was discovered by Coverity.

Reported-by: Ranier Vilela <ranier.vf@gmail.com>
Discussion: https://postgr.es/m/CAEudQApfAWzLo+oSuy2byXktdr7R8KJC_ACT5VV8fontrL35Pw@mail.gmail.com

5 weeks agopg_overexplain: Filter out actual row count from test result.
Robert Haas [Thu, 27 Mar 2025 13:00:46 +0000 (09:00 -0400)]
pg_overexplain: Filter out actual row count from test result.

Per buildfarm, these are not stable. In particular, 1/8 is sometimes
0.12 and sometimes 0.13.

5 weeks agoRemove the query_id_squash_values GUC
Álvaro Herrera [Thu, 27 Mar 2025 12:33:37 +0000 (13:33 +0100)]
Remove the query_id_squash_values GUC

Commit 62d712ecfd94 introduced the capability to calculate the same
queryId for queries with different lengths of constants in a list for an
IN clause.  This behavior was originally enabled with a GUC
query_id_squash_values.  After a discussion about the value of such a
GUC, it was decided to back out of the use of a GUC and make the
squashing behavior the only available option.

Author: Sami Imseih <samimseih@gmail.com>
Discussion: https://postgr.es/m/Z-LZyygkkNyA8-kR@msg.df7cb.de
Discussion: https://postgr.es/m/CA+q6zcVTK-3C-8NWV1oY2NZrvtnMCDqnyYYyk1T7WMUG65MeOQ@mail.gmail.com

5 weeks agoExpand test a bit
Peter Eisentraut [Thu, 27 Mar 2025 11:11:15 +0000 (12:11 +0100)]
Expand test a bit

Make pg_constraint output in inherit test show the convalidated column
as well.  This shows the interaction between convalidated and
conenforced.

This is extracted from a larger patch so that this reformatting isn't
distracting there.

Author: Amul Sul <amul.sul@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/CAAJ_b962c5AcYW9KUt_R_ER5qs3fUGbe4az-SP-vuwPS-w-AGA@mail.gmail.com

5 weeks agoProvide thread-safe pg_localeconv_r().
Peter Eisentraut [Thu, 27 Mar 2025 06:52:22 +0000 (07:52 +0100)]
Provide thread-safe pg_localeconv_r().

This involves four different implementation strategies:

1.  For Windows, we now require _configthreadlocale() to be available
and work (commit f1da075d9a0), and the documentation says that the
object returned by localeconv() is in thread-local memory.

2.  For glibc, we translate to nl_langinfo_l() calls, because it
offers the same information that way as an extension, and that API is
thread-safe.

3.  For macOS/*BSD, use localeconv_l(), which is thread-safe.

4.  For everything else, use uselocale() to set the locale for the
thread, and use a big ugly lock to defend against the returned object
being concurrently clobbered.  In practice this currently means only
Solaris.

The new call is used in pg_locale.c, replacing calls to setlocale() and
localeconv().

Author: Thomas Munro <thomas.munro@gmail.com>
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/CA%2BhUKGJqVe0%2BPv9dvC9dSums_PXxGo9SWcxYAMBguWJUGbWz-A%40mail.gmail.com

5 weeks agoSimplify syntax for ALTER TABLE ALTER CONSTRAINT NO INHERIT
Álvaro Herrera [Thu, 27 Mar 2025 08:24:52 +0000 (09:24 +0100)]
Simplify syntax for ALTER TABLE ALTER CONSTRAINT NO INHERIT

Commit d45597f72fe5 introduced the ability to change a not-null
constraint from NO INHERIT to INHERIT and vice versa, but we included
the SET noise word in the syntax for it.  The SET turns out not to be
necessary and goes against what the SQL standard says for other ALTER
TABLE subcommands, so remove it.

This changes the way this command is processed for constraint types
other than not-null, so there are some error message changes.

Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Suraj Kharage <suraj.kharage@enterprisedb.com>
Discussion: https://postgr.es/m/202503251602.vsxaehsyaoac@alvherre.pgsql

5 weeks agolibpq: Add TAP tests for service files and names
Michael Paquier [Thu, 27 Mar 2025 07:01:38 +0000 (16:01 +0900)]
libpq: Add TAP tests for service files and names

This commit adds a set of regression tests that checks various patterns
with service names and service files, with:
- Service file with no contents, used as default for PGSERVICEFILE to
prevent any lookups at the HOME directory of an environment where the
test is run.
- Service file with valid service name and its section.
- Service file at the root of PGSYSCONFDIR, named pg_service.conf.
- Missing service file.
- Service name defined as a connection parameter or as PGSERVICE.

Note that PGSYSCONFDIR is set to always point at a temporary directory
created by the test, so as we never try to look at SYSCONFDIR.

This set of tests has come up as a useful independent addition while
discussing a patch that adds an equivalent of PGSERVICEFILE as a
connection parameter as there have never been any tests for service
files and service names.  Torsten Foertsch and Ryo Kanbayashi have
provided a basic implementation, that I have expanded to what is
introduced in this commit.

Author: Torsten Foertsch <tfoertsch123@gmail.com>
Author: Ryo Kanbayashi <kanbayashi.dev@gmail.com>
Author: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CAKkG4_nCjx3a_F3gyXHSPWxD8Sd8URaM89wey7fG_9g7KBkOCQ@mail.gmail.com

5 weeks agoOptimize Query jumble
David Rowley [Thu, 27 Mar 2025 05:34:34 +0000 (18:34 +1300)]
Optimize Query jumble

f31aad9b0 adjusted query jumbling so it no longer ignores NULL nodes
during the jumble.  This added some overhead.  Here we tune a few
things to make jumbling faster again.  This makes jumbling perform
similar or even slightly faster than prior to that change.

Author: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CAApHDvreP04nhTKuYsPw0F-YN+4nr4f=L72SPeFb81jfv+2c7w@mail.gmail.com

5 weeks agoFix query jumbling to account for NULL nodes
David Rowley [Thu, 27 Mar 2025 05:23:00 +0000 (18:23 +1300)]
Fix query jumbling to account for NULL nodes

Previously NULL nodes were ignored.  This could cause issues where the
computed query ID could match for queries where fields that are next to
each other in their Node struct where one field was NULL and the other
non-NULL.  For example, the Query struct had distinctClause and sortClause
next to each other.  If someone wrote;

SELECT DISTINCT c1 FROM t;

and then;

SELECT c1 FROM t ORDER BY c1;

these would produce the same query ID since, in the first query, we
ignored the NULL sortClause and appended the jumble bytes for the
distictClause.  In the latter query, since we did nothing for the NULL
distinctClause then jumble the non-NULL sortClause, and since the node
representation stored is the same in both cases, the query IDs were
identical.

Here we fix this by always accounting for NULL nodes by recording that
we saw a NULL in the jumble buffer.  This fixes the issue as the order that
the NULL is recorded isn't the same in the above two queries.

Author: Bykov Ivan <i.bykov@modernsys.ru>
Author: Michael Paquier <michael@paquier.xyz>
Author: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/aafce7966e234372b2ba876c0193f1e9%40localhost.localdomain

5 weeks agodoc: Correct description of values used in FSM for indexes
Michael Paquier [Thu, 27 Mar 2025 01:20:41 +0000 (10:20 +0900)]
doc: Correct description of values used in FSM for indexes

The implementation of FSM for indexes is simpler than heap, where 0 is
used to track if a page is in-use and (BLCKSZ - 1) if a page is free.
One comment in indexfsm.c and one description in the documentation of
pg_freespacemap were incorrect about that.

Author: Alex Friedman <alexf01@gmail.com>
Discussion: https://postgr.es/m/71eef655-c192-453f-ac45-2772fec2cb04@gmail.com
Backpatch-through: 13

5 weeks agoaio: Add io_method=io_uring
Andres Freund [Tue, 18 Mar 2025 18:40:05 +0000 (14:40 -0400)]
aio: Add io_method=io_uring

Performing AIO using io_uring can be considerably faster than
io_method=worker, particularly when lots of small IOs are issued, as
a) the context-switch overhead for worker based AIO becomes more significant
b) the number of IO workers can become limiting

io_uring, however, is linux specific and requires an additional compile-time
dependency (liburing).

This implementation is fairly simple and there are substantial optimization
opportunities.

The description of the existing AIO_IO_COMPLETION wait event is updated to
make the difference between it and the new AIO_IO_URING_EXECUTION clearer.

Reviewed-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Jakub Wartak <jakub.wartak@enterprisedb.com>
Discussion: https://postgr.es/m/uvrtrknj4kdytuboidbhwclo4gxhswwcpgadptsjvjqcluzmah%40brqs62irg4dt
Discussion: https://postgr.es/m/20210223100344.llw5an2aklengrmn@alap3.anarazel.de
Discussion: https://postgr.es/m/stj36ea6yyhoxtqkhpieia2z4krnam7qyetc57rfezgk4zgapf@gcnactj4z56m

5 weeks agoaio: Add liburing dependency
Andres Freund [Tue, 18 Mar 2025 18:40:05 +0000 (14:40 -0400)]
aio: Add liburing dependency

Will be used in a subsequent commit, to implement io_method=io_uring. Kept
separate for easier review.

Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/uvrtrknj4kdytuboidbhwclo4gxhswwcpgadptsjvjqcluzmah%40brqs62irg4dt

5 weeks agodoc: Mention possible ephemeral discrepancies in pg_stat_activity
Michael Paquier [Wed, 26 Mar 2025 23:07:54 +0000 (08:07 +0900)]
doc: Mention possible ephemeral discrepancies in pg_stat_activity

Ephemeral inconsistencies across multiple attributes of pg_stat_activity
can exist as the system is designed to be efficient with a low overhead.
This question is raised by users from time to time based on the data
read in the view, so let's add a note in the docs about this
possibility.

Author: Alex Friedman <alexf01@gmail.com>
Reviewed-by: Sami Imseih <samimseih@gmail.com>
Discussion: https://postgr.es/m/8a275154-a654-44b0-ab37-197802f04c7b@gmail.com

5 weeks agoaio: Rename pgaio_io_prep_* to pgaio_io_start_*
Andres Freund [Wed, 26 Mar 2025 20:10:29 +0000 (16:10 -0400)]
aio: Rename pgaio_io_prep_* to pgaio_io_start_*

The old naming pattern (mirroring liburing's naming) was inconsistent with
the (not yet introduced) callers. It seems better to get rid of the
inconsistency now than to grow more users of the odd naming.

Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20250326001915.bc.nmisch@google.com

5 weeks agoaio: Pass result of local callbacks to ->report_return
Andres Freund [Wed, 26 Mar 2025 20:06:54 +0000 (16:06 -0400)]
aio: Pass result of local callbacks to ->report_return

Otherwise the results of e.g. temp table buffer verification errors will not
reach bufmgr.c. Obviously that's not right. Found while expanding the tests
for invalid buffer contents.

Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20250326001915.bc.nmisch@google.com

5 weeks agoaio: Be more paranoid about interrupts
Andres Freund [Wed, 26 Mar 2025 20:06:54 +0000 (16:06 -0400)]
aio: Be more paranoid about interrupts

As reported by Noah, it's possible, although practically very unlikely, that
interrupts could be processed in between pgaio_io_reopen() and
pgaio_io_perform_synchronously(). Prevent that by explicitly holding
interrupts.

It also seems good to add an assertion to pgaio_io_before_prep() to ensure
that interrupts are held, as otherwise FDs referenced by the IO could be
closed during interrupt processing. All code in the aio series currently runs
the code with interrupts held, but it seems better to be paranoid.

Reviewed-by: Noah Misch <noah@leadboat.com>
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20250324002939.5c.nmisch@google.com

5 weeks agopg_overexplain: SET jit=off when running tests.
Robert Haas [Wed, 26 Mar 2025 19:43:25 +0000 (15:43 -0400)]
pg_overexplain: SET jit=off when running tests.

Per buildfarm.

5 weeks agoFix oversights in commit 8d5ceb113e3f7ddb627bd40b26438a9d2fa05512
Robert Haas [Wed, 26 Mar 2025 18:22:45 +0000 (14:22 -0400)]
Fix oversights in commit 8d5ceb113e3f7ddb627bd40b26438a9d2fa05512

It added bogus whitespace at the end of a line in the documentation.
It should not have done that.

The pg_overexplain tests must SET debug_parallel_query = false,
not just RESET debug_parallel_query, or we get failures on test
machines that make debug_parallel_query = true the defualt.

5 weeks agopg_overexplain: Additional EXPLAIN options for debugging.
Robert Haas [Wed, 26 Mar 2025 17:52:21 +0000 (13:52 -0400)]
pg_overexplain: Additional EXPLAIN options for debugging.

There's a fair amount of information in the Plan and PlanState trees
that isn't printed by any existing EXPLAIN option. This means that,
when working on the planner, it's often necessary to rely on facilities
such as debug_print_plan, which produce excessively voluminous
output. Hence, use the new EXPLAIN extension facilities to implement
EXPLAIN (DEBUG) and EXPLAIN (RANGE_TABLE) as extensions to the core
EXPLAIN facility.

A great deal more could be done here, and the specific choices about
what to print and how are definitely arguable, but this is at least
a starting point for discussion and a jumping-off point for possible
future improvements.

Reviewed-by: Sami Imseih <samimseih@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviweed-by: Andrei Lepikhov <lepihov@gmail.com> (who didn't like it)
Discussion: http://postgr.es/m/CA+TgmoZfvQUBWQ2P8iO30jywhfEAKyNzMZSR+uc2xr9PZBw6eQ@mail.gmail.com

5 weeks agoKeep the decompressed filter in brin_bloom_union
Tomas Vondra [Wed, 26 Mar 2025 15:50:13 +0000 (16:50 +0100)]
Keep the decompressed filter in brin_bloom_union

The brin_bloom_union() function combines two BRIN summaries, by merging
one filter into the other. With bloom, we have to decompress the filters
first, but the function failed to update the summary to store the merged
filter. As a consequence, the index may be missing some of the data, and
return false negatives.

This issue exists since BRIN bloom indexes were introduced in Postgres
14, but at that point the union function was called only when two
sessions happened to summarize a range concurrently, which is rare. It
got much easier to hit in 17, as parallel builds use the union function
to merge summaries built by workers.

Fixed by storing a pointer to the decompressed filter, and freeing the
original one. Free the second filter too, if it was decompressed. The
freeing is not strictly necessary, because the union is called in
short-lived contexts, but it's tidy.

Backpatch to 14, where BRIN bloom indexes were introduced.

Reported by Arseniy Mukhin, investigation and fix by me.

Reported-by: Arseniy Mukhin
Discussion: https://postgr.es/m/18855-1cf1c8bcc22150e6%40postgresql.org
Backpatch-through: 14

5 weeks agoUse PG_MODULE_MAGIC_EXT in our installable shared libraries.
Tom Lane [Wed, 26 Mar 2025 15:11:02 +0000 (11:11 -0400)]
Use PG_MODULE_MAGIC_EXT in our installable shared libraries.

It seems potentially useful to label our shared libraries with version
information, now that a facility exists for retrieving that.  This
patch labels them with the PG_VERSION string.  There was some
discussion about using semantic versioning conventions, but that
doesn't seem terribly helpful for modules with no SQL-level presence;
and for those that do have SQL objects, we typically expect them
to support multiple revisions of the SQL definitions, so it'd still
not be very helpful.

I did not label any of src/test/modules/.  It seems unnecessary since
we don't install those, and besides there ought to be someplace that
still provides test coverage for the original PG_MODULE_MAGIC macro.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/dd4d1b59-d0fe-49d5-b28f-1e463b68fa32@gmail.com

5 weeks agoIntroduce PG_MODULE_MAGIC_EXT macro.
Tom Lane [Wed, 26 Mar 2025 14:59:42 +0000 (10:59 -0400)]
Introduce PG_MODULE_MAGIC_EXT macro.

This macro allows dynamically loaded shared libraries (modules) to
provide a wired-in module name and version, and possibly other
compile-time-constant fields in future.  This information can be
retrieved with the new pg_get_loaded_modules() function.

This feature is expected to be particularly useful for modules
that do not have any exposed SQL functionality and thus are
not associated with a SQL-level extension object.  But even for
modules that do belong to extensions, being able to verify the
actual code version can be useful.

Author: Andrei Lepikhov <lepihov@gmail.com>
Reviewed-by: Yurii Rashkovskii <yrashk@omnigres.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/dd4d1b59-d0fe-49d5-b28f-1e463b68fa32@gmail.com

5 weeks agoMove GSSAPI includes into its own header
Daniel Gustafsson [Wed, 26 Mar 2025 14:31:46 +0000 (15:31 +0100)]
Move GSSAPI includes into its own header

Due to a conflict in macro names on Windows between <wincrypt.h>
and <openssl/ssl.h> these headers need to be included using a
predictable pattern with an undef to handle that. The GSSAPI
header <gssapi.h> does include <wincrypt.h> which cause problems
with compiling PostgreSQL using MSVC when OpenSSL and GSSAPI are
both enabled in the tree. Rather than fixing piecemeal for each
file including gssapi headers, move the the includes and undef
to a new file which should be used to centralize the logic.

This patch is a reworked version of a patch by Imran Zaheer
proposed earlier in the thread. Once this has proven effective
in master we should look at backporting this as the problem
exist at least since v16.

Author: Daniel Gustafsson <daniel@yesql.se>
Co-authored-by: Imran Zaheer <imran.zhir@gmail.com>
Reported-by: Dave Page <dpage@pgadmin.org>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: vignesh C <vignesh21@gmail.com>
Discussion: https://postgr.es/m/20240708173204.3f3xjilglx5wuzx6@awork3.anarazel.de

5 weeks agopsql: Make test robust against locale variations
Daniel Gustafsson [Wed, 26 Mar 2025 12:20:56 +0000 (13:20 +0100)]
psql: Make test robust against locale variations

The test committed in 1a759c83278 was prone to failing when using
locales with a different decimal separator.  Since the test value
isn't the important part, change to using an integer instead.

Author: Daniel Gustafsson <daniel@yesql.se>
Reported-by: Pavel Stehule <pavel.stehule@gmail.com>
Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com>
Discussion: https://postgr.es/m/CAFj8pRDE=7uW7QP4rg-OQLE2i-puYsUUt+eHE-L6_b_J9w=eWg@mail.gmail.com

5 weeks agodblink: SCRAM authentication pass-through
Peter Eisentraut [Wed, 26 Mar 2025 09:05:49 +0000 (10:05 +0100)]
dblink: SCRAM authentication pass-through

This enables SCRAM authentication for dblink (using dblink_fdw) when
connecting to a foreign server without having to store a plain-text
password on user mapping options

This uses the same approach as it was implemented for postgres_fdw in
commit 761c79508e7.  (It also contains the equivalent of the
subsequent fixes 76563f88cfb and d2028e9bbc1.)

Author: Matheus Alcantara <mths.dev@pm.me>
Reviewed-by: Jacob Champion <jacob.champion@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/CAFY6G8ercA1KES%3DE_0__R9QCTR805TTyYr1No8qF8ZxmMg8z2Q%40mail.gmail.com

5 weeks agoAdd support for gamma() and lgamma() functions.
Dean Rasheed [Wed, 26 Mar 2025 09:35:53 +0000 (09:35 +0000)]
Add support for gamma() and lgamma() functions.

These are useful general-purpose math functions which are included in
POSIX and C99, and are commonly included in other math libraries, so
expose them as SQL-callable functions.

Author: Dean Rasheed <dean.a.rasheed@gmail.com>
Reviewed-by: Stepan Neretin <sncfmgg@gmail.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Dmitry Koval <d.koval@postgrespro.ru>
Reviewed-by: Alexandra Wang <alexandra.wang.oss@gmail.com>
Discussion: https://postgr.es/m/CAEZATCXpGyfjXCirFk9au+FvM0y2Ah+2-0WSJx7MO368ysNUPA@mail.gmail.com

5 weeks agoFix integer-overflow problem in scram_SaltedPassword()
Richard Guo [Wed, 26 Mar 2025 08:46:51 +0000 (17:46 +0900)]
Fix integer-overflow problem in scram_SaltedPassword()

Setting the iteration count for SCRAM secret generation to INT_MAX
will cause an infinite loop in scram_SaltedPassword() due to integer
overflow, as the loop uses the "i <= iterations" comparison.  To fix,
use "i < iterations" instead.

Back-patch to v16 where the user-settable GUC scram_iterations has
been added.

Author: Kevin K Biju <kevinkbiju@gmail.com>
Reviewed-by: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CAM45KeEMm8hnxdTOxA98qhfZ9CzGDdgy3mxgJmy0c+2WwjA6Zg@mail.gmail.com

5 weeks agoUse relation name instead of OID in query jumbling for RangeTblEntry
Michael Paquier [Wed, 26 Mar 2025 06:21:05 +0000 (15:21 +0900)]
Use relation name instead of OID in query jumbling for RangeTblEntry

custom_query_jumble (introduced in 5ac462e2b7ac as a node field
attribute) is now assigned to the expanded reference name "eref" of
RangeTblEntry, adding in the query jumble computation the non-qualified
aliased relation name, without the list of column names.  The relation
OID is removed from the query jumbling.

The effects of this change can be seen in the tests added by
3430215fe35f, where pg_stat_statements (PGSS) entries are now grouped
using the relation name, ignoring the relation search_path may point at.
For example, these two relations are different, but are now grouped in a
single PGSS entry as they are assigned the same query ID:
CREATE TABLE foo1.tab (a int);
CREATE TABLE foo2.tab (b int);
SET search_path = 'foo1';
SELECT count(*) FROM tab;
SET search_path = 'foo2';
SELECT count(*) FROM tab;
SELECT count(*) FROM foo1.tab;
SELECT count(*) FROM foo2.tab;
SELECT query, calls FROM pg_stat_statements WHERE query ~ 'FROM tab';
          query           | calls
--------------------------+-------
 SELECT count(*) FROM tab |     4
(1 row)

It is still possible to use an alias in the FROM clause to split these.
This behavior is useful for relations re-created with the same name,
where queries based on such relations would be grouped in the same
PGSS entry.  For permanent schemas, it should not really matter in
practice.  The main benefit is for workloads that use a lot of temporary
relations, which are usually re-created with the same name continuously.
These can be a heavy source of bloat in PGSS depending on the workload.
Such entries can now be grouped together, improving the user experience.

The original idea from Christoph Berg used catalog lookups to find
temporary relations, something that the query jumble has never done, and
it could cause some performance regressions.  The idea to use
RangeTblEntry.eref and the relation name, applying the same rules for
all relations, temporary and not temporary, has been proposed by Tom
Lane.  The documentation additions have been suggested by Sami Imseih.

Author: Michael Paquier <michael@paquier.xyz>
Co-authored-by: Sami Imseih <samimseih@gmail.com>
Reviewed-by: Christoph Berg <myon@debian.org>
Reviewed-by: Lukas Fittl <lukas@fittl.com>
Reviewed-by: Sami Imseih <samimseih@gmail.com>
Discussion: https://postgr.es/m/Z9iWXKGwkm8RAC93@msg.df7cb.de

5 weeks agopostgres_fdw: Fix tests on some Windows variants
Peter Eisentraut [Wed, 26 Mar 2025 05:56:52 +0000 (06:56 +0100)]
postgres_fdw: Fix tests on some Windows variants

The tests introduced by commit 76563f88cfb only work when Unix-domain
sockets are available.  This is optional on Windows, and buildfarm
member drongo runs without them.  To fix, skip the test if Unix-domain
sockets are not enabled.

5 weeks agoAdd pg_dump --with-{schema|data|statistics} options.
Jeff Davis [Wed, 26 Mar 2025 00:36:38 +0000 (17:36 -0700)]
Add pg_dump --with-{schema|data|statistics} options.

By adding the positive variants of options, in addition to the
negative variants that already exist, users can be explicit about what
pg_dump should produce.

Discussion: https://postgr.es/m/bd0513e4b1ea2b2f2d06f02720c6579711cb62a6.camel@j-davis.com
Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>