Peter Eisentraut [Mon, 15 Apr 2024 06:20:34 +0000 (08:20 +0200)]
Fix ALTER DOMAIN NOT NULL syntax
This addresses a few problems with commit
e5da0fe3c22 ("Catalog domain
not-null constraints").
In CREATE DOMAIN, a NOT NULL constraint looks like
CREATE DOMAIN d1 AS int [ CONSTRAINT conname ] NOT NULL
(Before
e5da0fe3c22, the constraint name was accepted but ignored.)
But in ALTER DOMAIN, a NOT NULL constraint looks like
ALTER DOMAIN d1 ADD [ CONSTRAINT conname ] NOT NULL VALUE
where VALUE is where for a table constraint the column name would be.
(This works as of
e5da0fe3c22. Before
e5da0fe3c22, this syntax
resulted in an internal error.)
But for domains, this latter syntax is confusing and needlessly
inconsistent between CREATE and ALTER. So this changes it to just
ALTER DOMAIN d1 ADD [ CONSTRAINT conname ] NOT NULL
(None of these syntaxes are per SQL standard; we are just living with
the bits of inconsistency that have built up over time.)
In passing, this also changes the psql \dD output to not show not-null
constraints in the column "Check", since it's already shown in the
column "Nullable". This has also been off since
e5da0fe3c22.
Reviewed-by: jian he <jian.universality@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/
9ec24d7b-633d-463a-84c6-
7acff769c9e8%40eisentraut.org
Heikki Linnakangas [Sun, 14 Apr 2024 20:02:43 +0000 (23:02 +0300)]
Put back initialization of 'sslmode', to silence Coverity
Coverity pointed out that the function checks for conn->sslmode !=
NULL, which implies that it might be NULL, but later we access it
without a NULL-check anyway. It doesn't know that it is in fact always
initialized earlier, in conninfo_add_defaults(), and hence the
NULL-check is not necessary. However, there is a lot of distance
between conninfo_add_defaults() and pqConnectOptions2(), so it's not
surprising that it doesn't see that. Put back the initialization code,
as it existed before commit
05fd30c0e7, to silence the warning.
In the long run, I'd like to refactor the libpq options handling and
initalization code. It seems silly to strdup() and copy strings, for
things like sslmode that have a limited set of possible values; it
should be an enum. But that's for another day.
Tomas Vondra [Sun, 14 Apr 2024 18:34:29 +0000 (20:34 +0200)]
Fix unnecessary padding in incremental backups
Commit
10e3226ba13d added padding to incremental backups to ensure the
block data is properly aligned. The code in sendFile() however failed to
consider that the header may be a multiple of BLCKSZ and thus already
aligned, adding a full BLCKSZ of unnecessary padding.
Not only does this make the incremental file a bit larger, but the other
places calculating the amount of padding did realize it's not needed and
did not include it in the formula. This resulted in pg_basebackup
getting confused while parsing the data stream, trying to access files
with invalid filenames (e.g. with binary data etc.) and failing.
Tomas Vondra [Sun, 14 Apr 2024 16:58:30 +0000 (18:58 +0200)]
Add regression test for BRIN parallel builds
Adds a regression test for parallel CREATE INDEX for BRIN indexes, to
improve coverage for BRIN code, particularly code to allow parallel
index builds introduced by
b43757171470.
The test is added to pageinspect, as that allows comparing the index to
one built without parallelism. Another option would be to just build the
index with parallelism and then check it produces correct results. But
checking the index is exactly as if built without parallelism makes
these query checks unnecessary.
Discussion: https://postgr.es/m/
1df00a66-db5a-4e66-809a-
99b386a06d86%40enterprisedb.com
Tomas Vondra [Sun, 14 Apr 2024 16:19:52 +0000 (18:19 +0200)]
Use the correct PG_DETOAST_DATUM macro in BRIN
Commit
6bcda4a721 replaced PG_DETOAST_DATUM with PG_DETOAST_DATUM_PACKED
in two BRIN output functions, for minmax-multi and bloom opclasses. But
this is incorrect - the code is accessing the data through structs that
already include a 4B header, so the detoast needs to match that. But the
PACKED macro may keep the 1B header, which means the struct fields will
point to incorrect data.
Backpatch-through: 16
Discussion: https://postgr.es/m/
1df00a66-db5a-4e66-809a-
99b386a06d86%40enterprisedb.com
Tomas Vondra [Sun, 14 Apr 2024 15:58:59 +0000 (17:58 +0200)]
Update nbits_set in brin_bloom_union
Properly update the number of bits set in the bitmap after merging the
filters in brin_bloom_union.
This is mostly harmless, as the counter is used only in the output
function, which means pageinspect may show incorrect information about
the BRIN summary. The counter does not affect correctness.
Discovered while adding a regression test comparing indexes built with
and without parallelism. The parallel index builds exercise the union
procedure when merging results from workers, which is otherwise very
hard to do in a test. Which is why this went unnoticed until now.
Backpatch through 14, where the BRIN bloom opclasses were introduced.
Backpatch-through: 14
Discussion: https://postgr.es/m/
1df00a66-db5a-4e66-809a-
99b386a06d86%40enterprisedb.com
Noah Misch [Sat, 13 Apr 2024 15:34:20 +0000 (08:34 -0700)]
freespace: Don't return blocks past the end of the main fork.
GetPageWithFreeSpace() callers assume the returned block exists in the
main fork, failing with "could not read block" errors if that doesn't
hold. Make that assumption reliable now. It hadn't been guaranteed,
due to the weak WAL and data ordering of participating components. Most
operations on the fsm fork are not WAL-logged. Relation extension is
not WAL-logged. Hence, an fsm-fork block on disk can reference a
main-fork block that no WAL record has initialized. That could happen
after an OS crash, a replica promote, or a PITR restore. wal_log_hints
makes the trouble easier to hit; a replica promote or PITR ending just
after a relevant fsm-fork FPI_FOR_HINT may yield this broken state. The
v16 RelationAddBlocks() mechanism also makes the trouble easier to hit,
since it bulk-extends even without extension lock waiters. Commit
917dc7d2393ce680dea7a59418be9ff341df3c14 stopped trouble around
truncation, but vectors involving PageIsNew() pages remained.
This implementation adds a RelationGetNumberOfBlocks() call when the
cached relation size doesn't confirm a block exists. We've been unable
to identify a benchmark that slows materially, but this may show up as
additional time in lseek(). An alternative without that overhead would
be a new ReadBufferMode such that ReadBufferExtended() returns NULL
after a 0-byte read, with all other errors handled normally. However,
each GetFreeIndexPage() caller would then need code for the return-NULL
case. Back-patch to v14, due to earlier versions not caching relation
size and the absence of a pre-v16 problem report.
Ronan Dunklau. Reported by Ronan Dunklau.
Discussion: https://postgr.es/m/
1878547.tdWV9SEqCh%40aivenlaptop
Noah Misch [Sat, 13 Apr 2024 14:56:14 +0000 (07:56 -0700)]
Correct "improve role option documentation".
This corrects doc commit
21912e3c0262e2cfe64856e028799d6927862563.
Back-patch to v16, like that one.
Reviewed by David G. Johnston.
Discussion: https://postgr.es/m/
20240331061642.07@rfd.leadboat.com
Heikki Linnakangas [Fri, 12 Apr 2024 16:52:39 +0000 (19:52 +0300)]
Document PG_TEST_EXTRA=libpq_encryption and also check 'kerberos'
In the libpq encryption negotiation tests, don't run the GSSAPI tests
unless PG_TEST_EXTRA='kerberos' is also set. That makes it possible to
still run most of the tests when GSSAPI support is compiled in, but
there's no MIT Kerberos installation.
Heikki Linnakangas [Fri, 12 Apr 2024 16:52:37 +0000 (19:52 +0300)]
Move libpq encryption negotiation tests
The test targets libpq's options, so 'src/test/interfaces/libpq/t' is
a more natural place for it.
While doing this, I noticed that I had missed adding the
libpq_encryption subdir to the Makefile. That's why this commit only
needs to remove it from the meson.build file.
Per Peter Eisentraut's suggestion.
Discussion: https://www.postgresql.org/message-id/
09d4bf5d-d0fa-4c66-a1d7-
5ec757609646@eisentraut.org
Heikki Linnakangas [Fri, 12 Apr 2024 16:52:34 +0000 (19:52 +0300)]
Fix compilation with --with-gssapi --without-openssl
The #define is spelled ENABLE_GSS, not USE_GSS. Introduced in commit
05fd30c0e7, reported by Thomas Munro.
Discussion: https://www.postgresql.org/message-id/CA%2BhUKG%2BHRTtB%2Bx%2BKKKj_cfX6sNhbeGuqmGxjGMwdVPG7YGFP8w@mail.gmail.com
Heikki Linnakangas [Fri, 12 Apr 2024 16:52:28 +0000 (19:52 +0300)]
Fix libpq_encryption tests when compiled without SSL support
It correctly skipped tests involving SSL in the server when SSL
support was not compiled in, but even when SSL is not enabled in the
server and the connection is established without SSL, libpq behaves
differently in many of the test scenarios when libpq is compiled
without SSL support. For example, with sslmode=prefer, if libpq is
compiled with SSL support it will attempt to use SSL, but without SSL
support it will try authenticating in plaintext mode directly. The
expected test output didn't take that into account.
Discussion: https://www.postgresql.org/message-id/CA%2BhUKG%2BHRTtB%2Bx%2BKKKj_cfX6sNhbeGuqmGxjGMwdVPG7YGFP8w@mail.gmail.com
Andrew Dunstan [Fri, 12 Apr 2024 14:52:25 +0000 (10:52 -0400)]
Don't allocate large buffer on the stack in pg_verifybackup
Per complaint from Andres Freund. Follow his suggestion to allocate the
buffer once in the calling routine instead.
Also make a tiny indentation improvement.
Discussion: https://postgr.es/m/
20240411190147.a3yries632olfcgg@awork3.anarazel.de
Andrew Dunstan [Tue, 9 Apr 2024 19:30:48 +0000 (15:30 -0400)]
Assorted minor cleanups in the test_json_parser module
Per gripes from Michael Paquier
Discussion: https://postgr.es/m/ZhTQ6_w1vwOhqTQI@paquier.xyz
Along the way, also clean up a handful of typos in
3311ea86ed and
ea7b4e9a2a, found by Alexander Lakhin, and a couple of stylistic
snafus noted by Daniel Westermann and Daniel Gustafsson.
Andrew Dunstan [Tue, 9 Apr 2024 18:35:08 +0000 (14:35 -0400)]
Shrink test file for test_json_parser module
Also delete live URLs
Jacob Champion
Discussion: https://postgr.es/m/CAOYmi+mtH=V1wZKAOauCd5QqQWr61hnXMJbJ9h-CZXAa1JXd3w@mail.gmail.com
Andrew Dunstan [Tue, 9 Apr 2024 18:23:20 +0000 (14:23 -0400)]
Add a TAP test for test_json_parser_perf
This just makes sure the test can run with a single iteration. A real
performance test would test with many more.
Andrew Dunstan [Tue, 9 Apr 2024 13:07:14 +0000 (09:07 -0400)]
Fix some memory leaks associated with parsing json and manifests
Coverity complained about not freeing some memory associated with
incrementally parsing backup manifests. To fix that, provide and use a new
shutdown function for the JsonManifestParseIncrementalState object, in
line with a suggestion from Tom Lane.
While analysing the problem, I noticed a buglet in freeing memory for
incremental json lexers. To fix that remove a bogus condition on
freeing the memory allocated for them.
David Rowley [Fri, 12 Apr 2024 11:15:52 +0000 (23:15 +1200)]
Fix recently introduced typo in code comment
Reported-by: Richard Guo
Discussion: https://postgr.es/m/CAMbWs49kAsZUsj7-0SBLvE9+uKz0RCqMEmM3NVytc1YvS8sTrQ@mail.gmail.com
Amit Kapila [Fri, 12 Apr 2024 09:33:28 +0000 (15:03 +0530)]
Fix the review comments and a bug in the slot sync code.
Ensure that when updating the catalog_xmin of the synced slots, it is
first written to disk before changing the in-memory value
(effective_catalog_xmin). This is to prevent a scenario where the
in-memory value change triggers a vacuum to remove catalog tuples before
the catalog_xmin is written to disk. In the event of a crash before the
catalog_xmin is persisted, we would not know that some required catalog
tuples have been removed and the synced slot would be invalidated.
Change the sanity check to ensure that remote_slot's confirmed_flush LSN
can't precede the local/synced slot during slot sync. Note that the
restart_lsn of the synced/local slot can be ahead of remote_slot. This can
happen when slot advancing machinery finds a running xacts record after
reaching the consistent state at a later point than the primary where it
serializes the snapshot and updates the restart_lsn.
Make the check to sync slots robust by allowing to sync only when the
confirmed_lsn, restart_lsn, or catalog_xmin of the remote slot is ahead of
the synced/local slot.
Reported-by: Amit Kapila and Shveta Malik
Author: Hou Zhijie, Shveta Malik
Reviewed-by: Amit Kapila, Bertrand Drouvot
Discussion: https://postgr.es/m/OS0PR01MB57162B67D3CB01B2756FBA6D94062@OS0PR01MB5716.jpnprd01.prod.outlook.com
Discussion: https://postgr.es/m/CAJpy0uCSS5zmdyUXhvw41HSdTbRqX1hbYqkOfHNj7qQ+2zn0AQ@mail.gmail.com
David Rowley [Fri, 12 Apr 2024 08:07:53 +0000 (20:07 +1200)]
Fix IS [NOT] NULL qual optimization for inheritance tables
b262ad440 added code to have the planner remove redundant IS NOT NULL
quals and eliminate needless scans for IS NULL quals on tables where the
qual's column has a NOT NULL constraint.
That commit failed to consider that an inheritance parent table could
have differing NOT NULL constraints between the parent and the child.
This caused issues as if we eliminated a qual on the parent, when
applying the quals to child tables in apply_child_basequals(), the qual
might not have been added to the parent's baserestrictinfo.
Here we fix this by not applying the optimization to remove redundant
quals to RelOptInfos belonging to inheritance parents and applying the
optimization again in apply_child_basequals(). Effectively, this means
that the parent and child are considered independently as the parent has
both an inh=true and inh=false RTE and we still apply the optimization
to the RelOptInfo corresponding to the inh=false RTE.
We're able to still apply the optimization in add_base_clause_to_rel()
for partitioned tables as the NULLability of partitions must match that
of their parent. And, if we ever expand restriction_is_always_false()
and restriction_is_always_true() to handle partition constraints then we
can apply the same logic as, even in multi-level partitioned tables,
there's no way to route values to a partition when the qual does not
match the partition qual of the partitioned table's parent partition.
The same is true for CHECK constraints as those must also match between
arent partitioned tables and their partitions.
Author: Richard Guo, David Rowley
Discussion: https://postgr.es/m/CAMbWs4930gQSZmjR7aANzEapdy61gCg6z8dT-kAEYD0sYWKPdQ@mail.gmail.com
Tom Lane [Thu, 11 Apr 2024 15:09:00 +0000 (11:09 -0400)]
Doc: fix bogus to_date() examples.
November doesn't have 31 days. Remarkably, this thinko
has escaped detection since commit
3f1998727.
Noted by Y. Saburov.
Discussion: https://postgr.es/m/
171276122213.681.
531905738590773705@wrigleys.postgresql.org
Alexander Korotkov [Thu, 11 Apr 2024 13:30:32 +0000 (16:30 +0300)]
Revert: Implement pg_wal_replay_wait() stored procedure
This commit reverts
06c418e163,
e37662f221,
bf1e650806,
25f42429e2,
ee79928441, and
74eaf66f98 per review by Heikki Linnakangas.
Discussion: https://postgr.es/m/
b155606b-e744-4218-bda5-
29379779da1a%40iki.fi
Alexander Korotkov [Thu, 11 Apr 2024 12:54:25 +0000 (15:54 +0300)]
Revert: Allow table AM to store complex data structures in rd_amcache
This commit reverts
02eb07ea89 per review by Andres Freund.
Discussion: https://postgr.es/m/
20240410165236.rwyrny7ihi4ddxw4%40awork3.anarazel.de
Alexander Korotkov [Thu, 11 Apr 2024 12:53:26 +0000 (15:53 +0300)]
Revert: Allow table AM tuple_insert() method to return the different slot
This commit reverts
c35a3fb5e0 per review by Andres Freund.
Discussion: https://postgr.es/m/
20240410165236.rwyrny7ihi4ddxw4%40awork3.anarazel.de
Alexander Korotkov [Thu, 11 Apr 2024 12:51:35 +0000 (15:51 +0300)]
Revert: Allow locking updated tuples in tuple_update() and tuple_delete()
This commit reverts
87985cc925 and
818861eb57 per review by Andres Freund.
Discussion: https://postgr.es/m/
20240410165236.rwyrny7ihi4ddxw4%40awork3.anarazel.de
Alexander Korotkov [Thu, 11 Apr 2024 12:47:53 +0000 (15:47 +0300)]
Revert: Let table AM insertion methods control index insertion
This commit reverts
b1484a3f19 per review by Andres Freund.
Discussion: https://postgr.es/m/
20240410165236.rwyrny7ihi4ddxw4%40awork3.anarazel.de
Alexander Korotkov [Thu, 11 Apr 2024 12:46:35 +0000 (15:46 +0300)]
Revert: Custom reloptions for table AM
This commit reverts
9bd99f4c26 and
422041542f per review by Andres Freund.
Discussion: https://postgr.es/m/
20240410165236.rwyrny7ihi4ddxw4%40awork3.anarazel.de
Peter Eisentraut [Thu, 11 Apr 2024 10:44:54 +0000 (12:44 +0200)]
meson: Remove obsolete function test
The test for pstat was removed from configure by
9db300ce6e3 but not
from meson.build. Do that now.
Etsuro Fujita [Thu, 11 Apr 2024 10:25:00 +0000 (19:25 +0900)]
postgres_fdw: Improve comment about handling of asynchronous requests.
We updated this comment in back branches (see commit
f6f61a4bd et al);
let's do so in HEAD as well for consistency.
Discussion: https://postgr.es/m/CAPmGK142V1kqDfjo2H%2Bb54JTn2woVBrisFq%2B%3D9jwXwxr0VvbgA%40mail.gmail.com
Michael Paquier [Thu, 11 Apr 2024 08:19:20 +0000 (17:19 +0900)]
Use correct datatype for xmin variables in slot.c
Two variables storing a slot's effective_xmin and effective_catalog_xmin
were saved as XLogRecPtr, which is incorrect as these should be
TransactionIds.
Oversight in
818fefd8fd44.
Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACVPSB74mrDTFezz-LV3Oi6F3SN71QA0oUHvndzi5dwTNg@mail.gmail.com
Backpatch-through: 16
Masahiko Sawada [Thu, 11 Apr 2024 08:18:05 +0000 (17:18 +0900)]
Revert indexed and enlargable binary heap implementation.
This reverts commit
b840508644 and
bcb14f4abc. These commits were made
for commit
5bec1d6bc5 (Improve eviction algorithm in ReorderBuffer
using max-heap for many subtransactions). However, per discussion,
commit
efb8acc0d0 replaced binary heap + index with pairing heap, and
made these commits unnecessary.
Reported-by: Jeff Davis
Discussion: https://postgr.es/m/
12747c15811d94efcc5cda72d6b35c80d7bf3443.camel%40j-davis.com
Masahiko Sawada [Thu, 11 Apr 2024 08:04:38 +0000 (17:04 +0900)]
Replace binaryheap + index with pairingheap in reorderbuffer.c
A pairing heap can perform the same operations as the binary heap +
index, with as good or better algorithmic complexity, and that's an
existing data structure so that we don't need to invent anything new
compared to v16. This commit makes the new binaryheap functionality
that was added in commits
b840508644 and
bcb14f4abc unnecessary, but
they will be reverted separately.
Remove the optimization to only build and maintain the heap when the
amount of memory used is close to the limit, becuase the bookkeeping
overhead with the pairing heap seems to be small enough that it
doesn't matter in practice.
Reported-by: Jeff Davis
Author: Heikki Linnakangas
Reviewed-by: Michael Paquier, Hayato Kuroda, Masahiko Sawada
Discussion: https://postgr.es/m/
12747c15811d94efcc5cda72d6b35c80d7bf3443.camel%40j-davis.com
Thomas Munro [Thu, 11 Apr 2024 02:35:42 +0000 (14:35 +1200)]
Fix grammar.
Reported-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/ZhdKqj5DwoOzirFv%40paquier.xyz
Thomas Munro [Thu, 11 Apr 2024 01:19:29 +0000 (13:19 +1200)]
Fix potential stack overflow in incremental backup.
The user can set RELSEG_SIZE to a high number at compile time, so we
can't use it to control the size of an array on the stack: it could be
many gigabytes in size. On closer inspection, we don't really need that
intermediate array anyway. Let's just write directly into the output
array, and then perform the absolute->relative adjustment in place.
This fixes new code from commit
dc212340058.
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKG%2B2hZ0sBztPW4mkLfng0qfkNtAHFUfxOMLizJ0BPmi5%2Bg%40mail.gmail.com
Michael Paquier [Thu, 11 Apr 2024 00:20:51 +0000 (09:20 +0900)]
Fix inconsistency with replay of hash squeeze record for clean buffers
aa5edbe379d6 has tweaked _hash_freeovflpage() so as the write buffer's
LSN is updated only when necessary, when REGBUF_NO_CHANGE is not used.
The replay code was not consistent with that, causing the write buffer's
LSN to be updated and its page to be marked as dirty even if the buffer
was registered in a "clean" state. This was possible for the case of a
squeeze record when there are no tuples to add to the write buffer, for
(is_prim_bucket_same_wrt && !is_prev_bucket_same_wrt).
I have performed some validation of this commit with
wal_consistency_checking and a change in WAL that logs REGBUF_NO_CHANGE
to a new BKPIMAGE_*. Thanks to that, it is possible to know at replay
if a buffer was clean when it was registered, then cross-checked the LSN
of the "clean" page copy coming from WAL with the LSN of the block once
the record has been replayed. This eats one bit in bimg_info, which is
not acceptable to be integrated as-is, but it could become handy in the
future. I didn't spot other areas than the one fixed by this commit at
the extent of what the main regression test suite covers.
As this is an oversight in
aa5edbe379d6, no backpatch is required.
Reported-by: Zubeyr Eryilmaz
Author: Hayato Kuroda
Reviewed-by: Amit Kapila, Michael Paquier
Discussion: https://postgr.es/m/ZbyVVG_7eW3YD5-A@paquier.xyz
Tom Lane [Wed, 10 Apr 2024 19:45:58 +0000 (15:45 -0400)]
Fix plpgsql's handling of -- comments following expressions.
Up to now, read_sql_construct() has collected all the source text from
the statement or expression's initial token up to the character just
before the "until" token. It normally tries to strip trailing
whitespace from that, largely for neatness. If there was a "-- text"
comment after the expression, this resulted in removing the newline
that terminates the comment, which creates a hazard if we try to paste
the collected text into a larger SQL construct without inserting a
newline after it. In particular this caused our handling of CASE
constructs to fail if there's a comment after a WHEN expression.
Commit
4adead1d2 noticed a similar problem with cursor arguments,
and worked around it through the rather crude hack of suppressing
the whitespace-trimming behavior for those. Rather than do that
and leave the hazard open for future hackers to trip over, let's
fix it properly. pl_scanner.c already has enough infrastructure
to report the end location of the expression's last token, so
we can copy up to that location and never collect any trailing
whitespace or comment to begin with.
Erik Wienhold and Tom Lane, per report from Michal Bartak.
Back-patch to all supported branches.
Discussion: https://postgr.es/m/CAAVzF_FjRoi8fOVuLCZhQJx6HATQ7MKm=aFOHWZODFnLmjX-xA@mail.gmail.com
Daniel Gustafsson [Wed, 10 Apr 2024 11:53:25 +0000 (13:53 +0200)]
Doc: Update ulinks to RFC documents to avoid redirect
The tools.ietf.org site has been decommissioned and replaced by a
number of sites serving various purposes. Links to RFCs and BCPs
are now 301 redirected to their new respective IETF sites. Since
this serves no purpose and only adds network overhead, update our
links to the new locations.
Backpatch to all supported versions.
Discussion: https://postgr.es/m/
3C1CEA99-FCED-447D-9858-
5A579B4C6687@yesql.se
Backpatch-through: v12
Michael Paquier [Wed, 10 Apr 2024 04:48:13 +0000 (13:48 +0900)]
Make GIN tests using injection points concurrent-safe
f587338dec87 has introduced in the test module injection_points a SQL
function called injection_points_set_local(), that can be used to make
all the injection points linked to the process where they are attached,
discarded automatically if any remain once the process exits.
e2e3b8ae9ed7 has added a NO_INSTALLCHECK to the test module to prevent
the use of installcheck. Now that there is a way to make the test
concurrent-safe, let's use it and remove the installcheck restriction.
Concurrency issues could be easily reproduced by running in a tight
loop a command like this one, in src/test/modules/gin/ (hardcoding
pg_sleep() after attaching injection points enlarges the race window)
and a second test suite like contrib/btree_gin/:
make installcheck USE_MODULE_DB=1
Reviewed-by: Andrey Borodin
Discussion: https://postgr.es/m/ZhNG4Io9uYOgwv3F@paquier.xyz
Amit Kapila [Wed, 10 Apr 2024 03:14:17 +0000 (08:44 +0530)]
Fix a test in failover slots regression test.
Wait for the standby to catch up before syncing the slots with
pg_sync_replication_slots(), otherwise, the logical slot could be ahead
and the sync would fail.
The other way to fix the test is to change it to use slotsync worker and
poll for the sync to get finished but the current approach is better as
this is a predictable way to write the test.
Per buildfarm
Author: Hou Zhijie
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/OS0PR01MB571665359F2F5DCD3ADABC9F94002@OS0PR01MB5716.jpnprd01.prod.outlook.com
Thomas Munro [Tue, 9 Apr 2024 22:46:15 +0000 (10:46 +1200)]
Fix illegal attribute propagation in LLVM JIT.
Commit
72559438 started copying more attributes from AttributeTemplate
to the functions we generate on the fly. In the case of deform
functions, which return void, this meant that "noundef", from
AttributeTemplate's return value (a Datum) was copied to a void type.
Older LLVM releases were OK with that, but LLVM 18 crashes.
Update our llvm_copy_attributes() function to skip copying the attribute
for the return value, if the target function returns void.
Thanks to Dmitry Dolgov for help chasing this down.
Back-patch to all supported releases, like
72559438.
Reported-by: Pavel Stehule <pavel.stehule@gmail.com>
Reviewed-by: Dmitry Dolgov <9erthalion6@gmail.com>
Discussion: https://postgr.es/m/CAFj8pRACpVFr7LMdVYENUkScG5FCYMZDDdSGNU-tch%2Bw98OxYg%40mail.gmail.com
David Rowley [Tue, 9 Apr 2024 23:53:32 +0000 (11:53 +1200)]
Fixup various StringInfo function usages
This adjusts various appendStringInfo* function calls to use a more
appropriate and efficient function with the same behavior. For example,
use appendStringInfoChar() when appending a single character rather than
appendStringInfo() and appendStringInfoString() when no formatting is
required rather than using appendStringInfo().
All adjustments made here are in code that's new to v17, so it makes
sense to fix these now rather than wait a few years and make
backpatching harder.
Discussion: https://postgr.es/m/CAApHDvojY2UvMiO+9_55ArTj10P1LBNJyyoGB+C65BLDNT0GsQ@mail.gmail.com
Reviewed-by: Nathan Bossart, Tom Lane
Alexander Korotkov [Tue, 9 Apr 2024 23:07:34 +0000 (02:07 +0300)]
revert: Transform OR clauses to ANY expression
This commit reverts
72bd38cc99 due to implementation and design issues.
Reported-by: Tom Lane
Discussion: https://postgr.es/m/
3604469.
1712628736%40sss.pgh.pa.us
David Rowley [Tue, 9 Apr 2024 23:10:16 +0000 (11:10 +1200)]
Remove unused BumpBlockIsValid macro
The bump allocator was recently added in
29f6a959c. Our other
allocators have a similar macro to this, but seemingly the version of
the macro for those allocators is only used in places where the chunk
header is decoded. Since the bump allocator has no chunk header, none
of those functions exist for bump therefore macro is unused. Remove it.
Reported-by: Peter Eisentraut
Discussion: https://postgr.es/m/
5f724fb2-96e1-4f36-b65b-
47b337ad432e@eisentraut.org
Alexander Korotkov [Tue, 9 Apr 2024 22:47:00 +0000 (01:47 +0300)]
Checks for ALTER TABLE ... SPLIT/MERGE PARTITIONS ... commands
Check that the target partition actually belongs to the parent table.
Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/
cd842601-cf1a-9806-f7b7-
d2509b93ba61%40gmail.com
Author: Dmitry Koval
David Rowley [Tue, 9 Apr 2024 22:43:31 +0000 (10:43 +1200)]
Doc: use "an SQL" instead of "a SQL"
Although which is correct depends entirely on whether you pronounce SQL
as "ess-que-ell" or "sequel", we have standardized on the former in our
user-facing documentation, so use the correct article according to that
pronunciation.
Discussion: https://postgr.es/m/CAApHDvp3osQwQam+wNTp9BdhP+QfWO6aY6ZTixQQMfM-UArKCw@mail.gmail.com
Daniel Gustafsson [Tue, 9 Apr 2024 21:39:38 +0000 (23:39 +0200)]
doc: Remove stray comma from list of psql options
Back in 7.2 the list of options had short options and long options
on the same line separated by comma, but since 7.3 they are listed
separate lines. The comma on -X was left behind so fix by removing
and backpatching all the way.
Reported-by: y.saburov@gmail.com
Discussion: https://postgr.es/m/
171267154345.684.
7212826057932148541@wrigleys.postgresql.org
Backpatch-through: v12
Peter Eisentraut [Tue, 9 Apr 2024 12:33:06 +0000 (14:33 +0200)]
Fix incorrect format placeholders
Peter Eisentraut [Tue, 9 Apr 2024 12:21:57 +0000 (14:21 +0200)]
Update config.guess and config.sub
Peter Eisentraut [Tue, 9 Apr 2024 09:32:48 +0000 (11:32 +0200)]
Fix whitespace
John Naylor [Tue, 9 Apr 2024 09:16:01 +0000 (16:16 +0700)]
Get rid of anonymous struct
This is a C11 feature, and we require C99. While at it, go the further
step and get rid of the surrounding union (with uintptr_t) entirely,
as there is currently no use case for this file to access the header of
BlocktableEntry as a uintptr_t, and there are no additional alignment
requirements. The least invasive way seems to be to transfer the old
union name to this struct.
Reported by Pavel Borisov and Andres Freund, per buildfarm member mylodon
Reviewed by Pavel Borisov
Discussion: https://postgr.es/m/CALT9ZEH11NYV8AOzKb1bWhCf6J0H=H31f0MgT9xX+HdqvcA1rw@mail.gmail.com
Heikki Linnakangas [Tue, 9 Apr 2024 05:06:31 +0000 (08:06 +0300)]
libpq error message fixes
Remove stray paren, capitalize SSL and ALPN.
Author: Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/
20240409.104613.
1653854506705708036.horikyota.ntt@gmail.com
Heikki Linnakangas [Tue, 9 Apr 2024 05:04:20 +0000 (08:04 +0300)]
Fix typo in docs
Author: Erik Rijkers
Discussion: https://www.postgresql.org/message-id/
0167b1e1-676c-66ba-e857-
3ad7cd84404f@xs4all.nl
Michael Paquier [Tue, 9 Apr 2024 05:01:33 +0000 (14:01 +0900)]
Add missing set_pglocale_pgservice() for pg_walsummary and pg_combinebackup
These calls are required to make both tools work with NLS.
Author: Kyotaro Horiguchi
Discussion: https://postgr.es/m/
20240408.162702.
183779935636035593.horikyota.ntt@gmail.com
Michael Paquier [Tue, 9 Apr 2024 01:31:12 +0000 (10:31 +0900)]
injection_points: Fix race condition with local injection point tests
The module relies on a shmem exit callback to clean up any injection
points linked to a specific process. One of the tests checks for the
case of an injection point name reused in a second connection where the
first connection should clean it up, but it did not count for the fact
that the shmem exit callback of the first connection may not have run
when the second connection begins its work.
The regress library includes a wait_pid() that can be used for this
purpose, instead of a custom wait logic, so let's rely on it to wait for
the first connection to exit before working with the second connection.
The module gains a REGRESS_OPTS to be able to look at the regress
library's dlpath.
This issue could be reproduced with a hardcoded sleep() in the shmem
exit callback, and the CI has been able to trigger it sporadically.
Oversight in
f587338dec87.
Reported-by: Bharath Rupireddy
Reviewed-by: Andrey Borodin
Discussion: https://postgr.es/m/ZhOd3NXAutteokGL@paquier.xyz
Tom Lane [Mon, 8 Apr 2024 21:00:07 +0000 (17:00 -0400)]
In psql, avoid leaking a PGresult after a query is cancelled.
After a query cancel, the tail end of ExecQueryAndProcessResults
took care to clear any not-yet-read PGresults; but it forgot about
the one it has already read. There would only be such a result
when handling a multi-command string made with "\;", so that you'd
have to cancel an earlier command in such a string to reach the
bug at all. Even then, there would only be leakage of a single
PGresult per cancel, so it's not surprising nobody noticed this.
But a leak is a leak.
Noted while re-reviewing
90f517821, but this is independent of that:
it dates to
7844c9918. Back-patch to v15 where that came in.
Tom Lane [Mon, 8 Apr 2024 19:49:10 +0000 (15:49 -0400)]
Further review for re-implementation of psql's FETCH_COUNT feature.
Alexander Lakhin noted an obsolete comment, which led me to revisit
some other important comments in the patch, and that study turned up a
couple of unintended ways in which the chunked-fetch code path didn't
match the normal code path in ExecQueryAndProcessResults. The only
nontrivial problem is that it didn't call PrintQueryStatus, so that
we'd not print the final status result from DML ... RETURNING
commands. To avoid code duplication, move the filter for whether a
result is from RETURNING from PrintQueryResult to PrintQueryStatus.
Discussion: https://postgr.es/m/
0023bea5-79c0-476e-96c8-
dad599cc3ad8@gmail.com
John Naylor [Mon, 8 Apr 2024 11:54:35 +0000 (18:54 +0700)]
Teach radix tree to embed values at runtime
Previously, the decision to store values in leaves or within the child
pointer was made at compile time, with variable length values using
leaves by necessity. This commit allows introspecting the length of
variable length values at runtime for that decision. This requires
the ability to tell whether the last-level child pointer is actually
a value, so we use a pointer tag in the lowest level bit.
Use this in TID store. This entails adding a byte to the header to
reserve space for the tag. Commit
f35bd9bf3 stores up to three offsets
within the header with no bitmap, and now the header can be embedded
as above. This reduces worst-case memory usage when TIDs are sparse.
Reviewed (in an earlier version) by Masahiko Sawada
Discussion: https://postgr.es/m/CANWCAZYw+_KAaUNruhJfE=h6WgtBKeDG32St8vBJBEY82bGVRQ@mail.gmail.com
Discussion: https://postgr.es/m/CAD21AoBci3Hujzijubomo1tdwH3XtQ9F89cTNQ4bsQijOmqnEw@mail.gmail.com
John Naylor [Mon, 8 Apr 2024 11:38:11 +0000 (18:38 +0700)]
Teach TID store to skip bitmap for small numbers of offsets
The header portion of BlocktableEntry has enough padding space for
an array of 3 offsets (1 on 32-bit platforms). Use this space instead
of having a sparse bitmap array. This will take up a constant amount
of space no matter what the offsets are.
Reviewed (in an earlier version) by Masahiko Sawada
Discussion: https://postgr.es/m/CANWCAZYw+_KAaUNruhJfE=h6WgtBKeDG32St8vBJBEY82bGVRQ@mail.gmail.com
Discussion: https://postgr.es/m/CAD21AoBci3Hujzijubomo1tdwH3XtQ9F89cTNQ4bsQijOmqnEw@mail.gmail.com
Alexander Korotkov [Mon, 8 Apr 2024 11:30:30 +0000 (14:30 +0300)]
Provide a way block-level table AMs could re-use acquire_sample_rows()
While keeping API the same, this commit provides a way for block-level table
AMs to re-use existing acquire_sample_rows() by providing custom callbacks
for getting the next block and the next tuple.
Reported-by: Andres Freund
Discussion: https://postgr.es/m/
20240407214001.jgpg5q3yv33ve6y3%40awork3.anarazel.de
Reviewed-by: Pavel Borisov
Alexander Korotkov [Mon, 8 Apr 2024 11:39:28 +0000 (14:39 +0300)]
Fix some grammer errors from error messages and codes comments
Discussion: https://postgr.es/m/CAHewXNkGMPU50QG7V6Q60JGFORfo8LfYO1_GCkCa0VWbmB-fEw%40mail.gmail.com
Author: Tender Wang
Alexander Korotkov [Mon, 8 Apr 2024 09:11:56 +0000 (12:11 +0300)]
Fill CommonRdOptions with default values in extract_autovac_opts()
Reported-by: Thomas Munro
Reported-by: Pavel Borisov
Discussion: https://postgr.es/m/CA%2BhUKGLZzLR50RBvuqOO3MZ%3DF54ETz-rTp1PDX9uDGP_GqyYqA%40mail.gmail.com
Heikki Linnakangas [Mon, 8 Apr 2024 09:14:20 +0000 (12:14 +0300)]
Adjust wording of trace_connection_negotiation GUC's description
We're not very consistent about this across all the GUCs, but the
"Logs ..." phrasing is more common than "Log ...", and is used by the
neighboring "log_connections" and "log_disconnections" GUCs, so switch
to that.
Author: Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/
20240408.154010.
1170771365226258348.horikyota.ntt@gmail.com
Alexander Korotkov [Mon, 8 Apr 2024 08:23:28 +0000 (11:23 +0300)]
Custom reloptions for table AM
Let table AM define custom reloptions for its tables. This allows specifying
AM-specific parameters by the WITH clause when creating a table.
The reloptions, which could be used outside of table AM, are now extracted
into the CommonRdOptions data structure. These options could be by decision
of table AM directly specified by a user or calculated in some way.
The new test module test_tam_options evaluates the ability to set up custom
reloptions and calculate fields of CommonRdOptions on their base.
The code may use some parts from prior work by Hao Wu.
Discussion: https://postgr.es/m/CAPpHfdurb9ycV8udYqM%3Do0sPS66PJ4RCBM1g-bBpvzUfogY0EA%40mail.gmail.com
Discussion: https://postgr.es/m/AMUA1wBBBxfc3tKRLLdU64rb.1.
1683276279979.Hmail.wuhao%40hashdata.cn
Reviewed-by: Reviewed-by: Pavel Borisov, Matthias van de Meent, Jess Davis
Amit Kapila [Mon, 8 Apr 2024 07:51:55 +0000 (13:21 +0530)]
Fix the intermittent buildfarm failures in 040_standby_failover_slots_sync.
It is possible that even if the primary waits for the subscriber to catch
up and then disables the subscription, the XLOG_RUNNING_XACTS record gets
inserted between the two steps by bgwriter and walsender processes it.
This can move the restart_lsn of the corresponding slot in an
unpredictable way which further leads to slot sync failure.
To ensure predictable behaviour, we drop the subscription and manually
create the slot before the test. The other idea we discussed to write a
predictable test is to use injection points to control the bgwriter
logging XLOG_RUNNING_XACTS but that needs more analysis. We can add a
separate test using injection points.
Per buildfarm
Author: Hou Zhijie
Reviewed-by: Amit Kapila, Shveta Malik
Discussion: https://postgr.es/m/CAA4eK1JD8h_XLRsK_o_Xh=5MhTzm+6d4Cb4_uPgFJ2wSQDah=g@mail.gmail.com
John Naylor [Sun, 7 Apr 2024 05:27:34 +0000 (12:27 +0700)]
Use bump context for TID bitmaps stored by vacuum
Vacuum does not pfree individual entries, and only frees the entire
storage space when finished with it. This allows using a bump context,
eliminating the chunk header in each leaf allocation. Most leaf
allocations will be 16 to 32 bytes, so that's a significant savings.
TidStoreCreateLocal gets a boolean parameter to indicate that the
created store is insert-only.
This requires a separate tree context for iteration, since we free
the iteration state after iteration completes.
Discussion: https://postgr.es/m/CANWCAZac%3DpBePg3rhX8nXkUuaLoiAJJLtmnCfZsPEAS4EtJ%3Dkg%40mail.gmail.com
Discussion: https://postgr.es/m/CANWCAZZQFfxvzO8yZHFWtQV+Z2gAMv1ku16Vu7KWmb5kZQyd1w@mail.gmail.com
Amit Langote [Mon, 8 Apr 2024 06:58:58 +0000 (15:58 +0900)]
JSON_TABLE: Add support for NESTED paths and columns
A NESTED path allows to extract data from nested levels of JSON
objects given by the parent path expression, which are projected as
columns specified using a nested COLUMNS clause, just like the parent
COLUMNS clause. Rows comprised from a NESTED columns are "joined"
to the row comprised from the parent columns. If a particular NESTED
path evaluates to 0 rows, then the nested COLUMNS will emit NULLs,
making it an OUTER join.
NESTED columns themselves may include NESTED paths to allow
extracting data from arbitrary nesting levels, which are likewise
joined against the rows at the parent level.
Multiple NESTED paths at a given level are called "sibling" paths
and their rows are combined by UNIONing them, that is, after being
joined against the parent row as described above.
Author: Nikita Glukhov <n.gluhov@postgrespro.ru>
Author: Teodor Sigaev <teodor@sigaev.ru>
Author: Oleg Bartunov <obartunov@gmail.com>
Author: Alexander Korotkov <aekorotkov@gmail.com>
Author: Andrew Dunstan <andrew@dunslane.net>
Author: Amit Langote <amitlangote09@gmail.com>
Author: Jian He <jian.universality@gmail.com>
Reviewers have included (in no particular order):
Andres Freund, Alexander Korotkov, Pavel Stehule, Andrew Alsup,
Erik Rijkers, Zihong Yu, Himanshu Upadhyaya, Daniel Gustafsson,
Justin Pryzby, Álvaro Herrera, Jian He
Discussion: https://postgr.es/m/
cd0bb935-0158-78a7-08b5-
904886deac4b@postgrespro.ru
Discussion: https://postgr.es/m/
20220616233130.rparivafipt6doj3@alap3.anarazel.de
Discussion: https://postgr.es/m/
abd9b83b-aa66-f230-3d6d-
734817f0995d%40postgresql.org
Discussion: https://postgr.es/m/CA+HiwqE4XTdfb1nW=Ojoy_tQSRhYt-q_kb6i5d4xcKyrLC1Nbg@mail.gmail.com
Amit Langote [Mon, 8 Apr 2024 07:02:40 +0000 (16:02 +0900)]
Fix JsonExpr deparsing to emit QUOTES and WRAPPER correctly
Currently, get_json_expr_options() does not emit the default values
for QUOTES (KEEP QUOTES) and WRAPPER (WITHOUT WRAPPER). That causes
the deparsed JSON_TABLE() columns, such as those contained in a a
view's query, to behave differently when executed than the original
definition. That's because the rules encoded in
transformJsonTableColumns() will choose either JSON_VALUE() or
JSON_QUERY() as implementation to execute a given column's path
expression depending on the QUOTES and WRAPPER specificationd and
they have slightly different semantics.
Reported-by: Jian He <jian.universality@gmail.com>
Discussion: https://postgr.es/m/CACJufxEqhqsfrg_p7EMyo5zak3d767iFDL8vz_4%3DZBHpOtrghw%40mail.gmail.com
Amit Langote [Mon, 8 Apr 2024 07:02:29 +0000 (16:02 +0900)]
Fix restriction on specifying KEEP QUOTES in JSON_QUERY()
Currently, transformJsonFuncExpr() enforces some restrictions on
the combinations of QUOTES and WRAPPER clauses that can be specified
in JSON_QUERY(). The intent was to only prevent the useless
combination WITH WRAPPER OMIT QUOTES, but the coding prevented KEEP
QUOTES too, which is not helpful. Fix that.
Alexander Korotkov [Mon, 8 Apr 2024 05:59:27 +0000 (08:59 +0300)]
Fix the wording of or_to_any_transform_limit description
Reported-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/
20240408.144657.
1746688590065601661.horikyota.ntt%40gmail.com
Alexander Korotkov [Mon, 8 Apr 2024 05:51:07 +0000 (08:51 +0300)]
Fix the value of or_to_any_transform_limit in postgresql.conf.sample
Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/ZhM8jH8gsKm5Q-9p%40pryzbyj2023
Andres Freund [Mon, 8 Apr 2024 04:41:00 +0000 (21:41 -0700)]
Remove references to old function name
In
a97bbe1f1df I accidentally referenced heapgetpage(), both in a function
name and a comment. But since
44086b09753 the relevant function is named
heap_prepare_pagescan(). Rename the new function to page_collect_tuples().
Reported-by: Melanie Plageman <melanieplageman@gmail.com>
Reported-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/
20240407172615.cocrsvboqm3ttqe4@awork3.anarazel.de
Discussion: https://postgr.es/m/CAApHDvp4SniHopTrVeKWcEvNXFtdki0utAvO=5R7H6TNhtULRQ@mail.gmail.com
Thomas Munro [Sat, 6 Apr 2024 21:13:17 +0000 (09:13 +1200)]
Add pg_buffercache_evict() function for testing.
When testing buffer pool logic, it is useful to be able to evict
arbitrary blocks. This function can be used in SQL queries over the
pg_buffercache view to set up a wide range of buffer pool states. Of
course, buffer mappings might change concurrently so you might evict a
block other than the one you had in mind, and another session might
bring it back in at any time. That's OK for the intended purpose of
setting up developer testing scenarios, and more complicated interlocking
schemes to give stronger guararantees about that would likely be less
flexible for actual testing work anyway. Superuser-only.
Author: Palak Chaturvedi <chaturvedipalak1911@gmail.com>
Author: Thomas Munro <thomas.munro@gmail.com> (docs, small tweaks)
Reviewed-by: Nitin Jadhav <nitinjadhavpostgres@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Cary Huang <cary.huang@highgo.ca>
Reviewed-by: Cédric Villemain <cedric.villemain+pgsql@abcsql.com>
Reviewed-by: Jim Nasby <jim.nasby@gmail.com>
Reviewed-by: Maxim Orlov <orlovmg@gmail.com>
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Reviewed-by: Melanie Plageman <melanieplageman@gmail.com>
Discussion: https://postgr.es/m/CALfch19pW48ZwWzUoRSpsaV9hqt0UPyaBPC4bOZ4W+
c7FF566A@mail.gmail.com
John Naylor [Mon, 8 Apr 2024 03:40:20 +0000 (10:40 +0700)]
Fix alignment of stack variable
Declare with union similar to PGAlignedBlock.
Report and fix by Andres Freund
Discussion: https://postgr.es/m/
20240407190731.izm3mdazednrsiqk%40awork3.anarazel.de
Masahiko Sawada [Mon, 8 Apr 2024 03:15:10 +0000 (12:15 +0900)]
Add more tab completion support for ALTER DEFAULT PRIVILEGES in psql.
This adds tab completion of "GRANT" and "REVOKE [GRANT OPTION FOR]"
for ALTER DEFAULT PRIVILEGES, and adds "WITH GRANT OPTION" for
ALTER DEFAULT PRIVILEGES ... GRANT ... TO role.
Author: Vignesh C, with cosmetic adjustments by me
Reviewed-by: Shubham Khanna, Masahiko Sawada
Discussion: https://postgr.es/m/CALDaNm1aEdJb-QJi%3DGWStkfj_%2BEDUK_VtDkn%2BTjQ2z7HyU0MBw%40mail.gmail.com
Peter Geoghegan [Mon, 8 Apr 2024 02:13:28 +0000 (22:13 -0400)]
Remove redundant nbtree preprocessing assertions.
One of the assertions was the subject of a false positive complaint from
Coverity, but none of the assertions added much, so get rid of them.
Reported-By: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/
3000247.
1712537309@sss.pgh.pa.us
Andres Freund [Mon, 8 Apr 2024 02:00:11 +0000 (19:00 -0700)]
simplehash: Free collisions array in SH_STAT
While SH_STAT() is only used for debugging, the allocated array can be large,
and therefore should be freed.
It's unclear why coverity started warning now.
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Reported-by: Coverity
Discussion: https://postgr.es/m/
3005248.
1712538233@sss.pgh.pa.us
Backpatch: 12-
Heikki Linnakangas [Mon, 8 Apr 2024 02:03:17 +0000 (05:03 +0300)]
Fix check for 'outlen' return from SSL_select_next_proto()
Fixes compiler warning reported by Andres Freund.
Discusssion: https://www.postgresql.org/message-id/
20240408015055.xsuahullywpfwyvu@awork3.anarazel.de
Heikki Linnakangas [Mon, 8 Apr 2024 01:32:26 +0000 (04:32 +0300)]
Silence perlcritic warnings in new libpq tests
Per buildfarm member 'koel'.
Heikki Linnakangas [Mon, 8 Apr 2024 01:24:51 +0000 (04:24 +0300)]
Send ALPN in TLS handshake, require it in direct SSL connections
libpq now always tries to send ALPN. With the traditional negotiated
SSL connections, the server accepts the ALPN, and refuses the
connection if it's not what we expect, but connecting without ALPN is
still OK. With the new direct SSL connections, ALPN is mandatory.
NOTE: This uses "TBD-pgsql" as the protocol ID. We must register a
proper one with IANA before the release!
Author: Greg Stark, Heikki Linnakangas
Reviewed-by: Matthias van de Meent, Jacob Champion
Heikki Linnakangas [Mon, 8 Apr 2024 01:24:49 +0000 (04:24 +0300)]
Support TLS handshake directly without SSLRequest negotiation
By skipping SSLRequest, you can eliminate one round-trip when
establishing a TLS connection. It is also more friendly to generic TLS
proxies that don't understand the PostgreSQL protocol.
This is disabled by default in libpq, because the direct TLS handshake
will fail with old server versions. It can be enabled with the
sslnegotation=direct option. It will still fall back to the negotiated
TLS handshake if the server rejects the direct attempt, either because
it is an older version or the server doesn't support TLS at all, but
the fallback can be disabled with the sslnegotiation=requiredirect
option.
Author: Greg Stark, Heikki Linnakangas
Reviewed-by: Matthias van de Meent, Jacob Champion
Heikki Linnakangas [Mon, 8 Apr 2024 01:24:46 +0000 (04:24 +0300)]
Refactor libpq state machine for negotiating encryption
This fixes the few corner cases noted in commit
705843d294, as shown
by the changes in the test.
Author: Heikki Linnakangas, Matthias van de Meent
Reviewed-by: Jacob Champion
Thomas Munro [Mon, 8 Apr 2024 01:16:20 +0000 (13:16 +1200)]
Use streaming I/O in ANALYZE.
The ANALYZE command prefetches and reads sample blocks chosen by a
BlockSampler algorithm. Instead of calling [Prefetch|Read]Buffer() for
each block, ANALYZE now uses the streaming API introduced in
b5a9b18cd0.
Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Reviewed-by: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Jakub Wartak <jakub.wartak@enterprisedb.com>
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/flat/CAN55FZ0UhXqk9v3y-zW_fp4-WCp43V8y0A72xPmLkOM%2B6M%2BmJg%40mail.gmail.com
Michael Paquier [Mon, 8 Apr 2024 00:47:50 +0000 (09:47 +0900)]
injection_points: Introduce runtime conditions
This adds a new SQL function injection_points_set_local() that can be
used to force injection points to be run only in the process where they
are attached. This is handy for SQL tests to:
- Detach automatically injection points when the process exits.
- Allow tests with injection points to run concurrently with other test
suites, so as such modules do not have to be marked with
NO_INSTALLCHECK.
Currently, the only condition that can be registered is for a PID.
This could be extended to more kinds later, if required, like database
names/OIDs, roles, or more concepts I did not consider.
Using a single function for SQL scripts is an idea from Heikki
Linnakangas.
Reviewed-by: Andrey Borodin
Discussion: https://postgr.es/m/ZfP7IDs9TvrKe49x@paquier.xyz
Heikki Linnakangas [Sun, 7 Apr 2024 23:49:37 +0000 (02:49 +0300)]
Enhance libpq encryption negotiation tests with new GUC
The new "log_connection_negotiation" server option causes the server
to print messages to the log when it receives a SSLRequest or
GSSENCRequest packet from the client. Together with "log_connections",
it gives a trace of how a connection and encryption is
negotiatated. Use the option in the libpq_encryption test, to verify
in more detail how libpq negotiates encryption with different
gssencmode and sslmode options.
This revealed a couple of cases where libpq retries encryption or
authentication, when it should already know that it cannot succeed. I
marked them with XXX comments in the test tables. They only happen
when the connection was going to fail anyway, and only with rare
combinations of options, so they're not serious.
Discussion: https://www.postgresql.org/message-id/CAEze2Wja8VUoZygCepwUeiCrWa4jP316k0mvJrOW4PFmWP0Tcw@mail.gmail.com
Heikki Linnakangas [Sun, 7 Apr 2024 23:49:35 +0000 (02:49 +0300)]
With gssencmode='require', check credential cache before connecting
Previously, libpq would establish the TCP connection, and then
immediately disconnect if the credentials were not available. The
same thing happened if you tried to use a Unix domain socket with
gssencmode=require. Check those conditions before establishing the TCP
connection.
This is a very minor issue, but my motivation to do this now is that
I'm about to add more detail to the tests for encryption negotiation.
This makes the case of gssencmode=require but no credentials
configured fail at the same stage as with gssencmode=require and
GSSAPI support not compiled at all. That avoids having to deal with
variations in expected output depending on build options.
Discussion: https://www.postgresql.org/message-id/CAEze2Wja8VUoZygCepwUeiCrWa4jP316k0mvJrOW4PFmWP0Tcw@mail.gmail.com
Heikki Linnakangas [Sun, 7 Apr 2024 23:49:32 +0000 (02:49 +0300)]
Add tests for libpq gssencmode and sslmode options
Test all combinations of gssencmode, sslmode, whether the server
supports SSL and/or GSSAPI encryption, and whether they are accepted
by pg_hba.conf. This is in preparation for refactoring that code in
libpq, and for adding a new option for "direct SSL" connections, which
adds another dimension to the logic.
If we add even more options in the future, testing all combinations
will become unwieldy and we'll need to rethink this, but for now an
exhaustive test is nice.
Author: Heikki Linnakangas, Matthias van de Meent
Reviewed-by: Jacob Champion
Discussion: https://www.postgresql.org/message-id/
a3af4070-3556-461d-aec8-
a8d794f94894@iki.fi
Heikki Linnakangas [Sun, 7 Apr 2024 23:49:30 +0000 (02:49 +0300)]
Move Kerberos module
So that we can reuse it in new tests.
Discussion: https://www.postgresql.org/message-id/
a3af4070-3556-461d-aec8-
a8d794f94894@iki.fi
Reviewed-by: Jacob Champion, Matthias van de Meent
Michael Paquier [Sun, 7 Apr 2024 23:45:04 +0000 (08:45 +0900)]
Make GIN test using injection points repeatable
As written, the test would fail when run repeatedly because one of the
injection points attached was not detached. This would not matter if
the test is rewritten to be concurrently safe, but let's be clean and
it is a good practice.
Oversight in
6a1ea02c491d.
Discussion: https://postgr.es/m/ZfP7IDs9TvrKe49x@paquier.xyz
David Rowley [Sun, 7 Apr 2024 23:06:31 +0000 (11:06 +1200)]
Fix incorrect KeeperBlock macro in bump.c
The macro was missing a MAXALIGN around the sizeof(BumpContext) which
would cause problems detecting the keeper block on 32-bit systems that
have a MAXALIGN value of 8.
Thank you to Andres Freund, Tomas Vondra and Tom Lane for investigating
and testing.
Reported-by: Melanie Plageman, Tomas Vondra
Discussion: https://postgr.es/m/CAAKRu_Y6dZjiJEZghgNZp0Gjar1JVq-CH7XGDqExDVHnPgDjuw@mail.gmail.com
Discussion: https://postgr.es/m/
a4a10b89-6ba8-4abd-b449-
019aafff04fc@enterprisedb.com
Alexander Korotkov [Sun, 7 Apr 2024 22:38:37 +0000 (01:38 +0300)]
Fix usage of same ListCell transform_or_to_any()'s in nested loops
Discussion: https://postgr.es/m/CAAKRu_b4SXNW4GAM0bv3e6wcL5ODSXg1ZdRCn6uyLLjSPbveBg%40mail.gmail.com
Author: Melanie Plageman
Alexander Korotkov [Sun, 7 Apr 2024 22:27:28 +0000 (01:27 +0300)]
Transform OR clauses to ANY expression
Replace (expr op C1) OR (expr op C2) ... with expr op ANY(ARRAY[C1, C2, ...])
on the preliminary stage of optimization when we are still working with the
expression tree.
Here Cn is a n-th constant expression, 'expr' is non-constant expression, 'op'
is an operator which returns boolean result and has a commuter (for the case
of reverse order of constant and non-constant parts of the expression,
like 'Cn op expr').
Sometimes it can lead to not optimal plan. This is why there is a
or_to_any_transform_limit GUC. It specifies a threshold value of length of
arguments in an OR expression that triggers the OR-to-ANY transformation.
Generally, more groupable OR arguments mean that transformation will be more
likely to win than to lose.
Discussion: https://postgr.es/m/
567ED6CA.
2040504%40sigaev.ru
Author: Alena Rybakina <lena.ribackina@yandex.ru>
Author: Andrey Lepikhov <a.lepikhov@postgrespro.ru>
Reviewed-by: Peter Geoghegan <pg@bowt.ie>
Reviewed-by: Ranier Vilela <ranier.vf@gmail.com>
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Reviewed-by: Jian He <jian.universality@gmail.com>
Daniel Gustafsson [Sun, 7 Apr 2024 22:24:20 +0000 (00:24 +0200)]
Change debug printing to log filename
When restarting the cluster fails the code introduced in
33774978c78
printed the full log contents to aid debugging. For cases when the
logfile is large this adds unnecessary overhead. Reduce to printing
the logfile path instead.
Reported-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/
20240406214439.2n4zf2w7ukhf7dsy@awork3.anarazel.de
Tom Lane [Sun, 7 Apr 2024 21:16:32 +0000 (17:16 -0400)]
Doc: clarify behavior of boolean options in replication protocol commands.
Same idea as
ec7e053a9, but applying to the walsender commands
described in protocol.sgml.
Peter Smith
Discussion: https://postgr.es/m/CAHut+PvwjZfdGt2R8HTXgSZft=jZKymrS8KUg31pS7zqaaWKKw@mail.gmail.com
Tom Lane [Sun, 7 Apr 2024 21:08:06 +0000 (17:08 -0400)]
Remove useless duplicate call of defGetBoolean().
Seems to be a copy-and-paste error dating to
dc2123400.
Noted while reviewing a related documentation patch.
Tom Lane [Sun, 7 Apr 2024 20:26:47 +0000 (16:26 -0400)]
Doc: show how to get the equivalent of LIMIT for UPDATE/DELETE.
Add examples showing use of a CTE and a self-join to perform
partial UPDATEs and DELETEs.
Corey Huinker, reviewed by Laurenz Albe
Discussion: https://postgr.es/m/CADkLM=caNEQsUwPWnfi2jR4ix99E0EJM_3jtcE-YjnEQC7Rssw@mail.gmail.com
Tom Lane [Sun, 7 Apr 2024 19:36:08 +0000 (15:36 -0400)]
Doc: update documentation about EXCLUDE constraint elements.
What the documentation calls an exclude_element is an index_elem
according to gram.y, and it allows all the same options that
a CREATE INDEX column specification does. The COLLATE patch
neglected to update the CREATE/ALTER TABLE docs about that,
and later the opclass-parameters patch made the same oversight.
Add those options to the syntax synopses, and polish the
associated text a bit.
Back-patch to v13 where opclass parameters came in. We could
update v12 with just the COLLATE omission, but it doesn't quite
seem worth the trouble at this point.
Shihao Zhong, reviewed by Daniel Vérité, Shubham Khanna and myself
Discussion: https://postgr.es/m/CAGRkXqShbVyB8E3gapfdtuwiWTiK=Q67Qb9qwxu=+-w0w46EBA@mail.gmail.com
Alvaro Herrera [Sun, 7 Apr 2024 18:33:45 +0000 (20:33 +0200)]
Use conditional variable to wait for next MultiXact offset
In one multixact.c edge case, we need a mechanism to wait for one
multixact offset to be written before being allowed to read the next
one. We used to handle this case by sleeping for one millisecond and
retrying, but such sleeps have been reported as problematic in
production cases. We can avoid the problem by using a condition
variable: readers sleep on it and then every creator of multixacts
broadcasts into the CV when creation is sufficiently far along.
Author: Kyotaro Horiguchi <horikyotajntt@gmail.com>
Reviewed-by: Andrey Borodin <amborodin@acm.org>
Discussion: https://postgr.es/m/
47A598F4-B4E7-4029-8FEC-
A06A6C3CB4B5@yandex-team.ru
Discussion: https://postgr.es/m/
20200515.090333.
24867479329066911.horikyota.ntt
Peter Geoghegan [Sun, 7 Apr 2024 18:15:54 +0000 (14:15 -0400)]
Avoid extra lookups with nbtree array inequalities.
nbtree index scans with SAOP inequalities (but no SAOP equalities)
performed extra ORDER proc lookups for any remaining equality strategy
scan keys. This could waste cycles, and caused assertion failures.
Keeping around a separate ORDER proc is only necessary for a scan's
non-array/non-SAOP equality scan keys when the scan has at least one
other SAOP equality strategy key (a SAOP inequality shouldn't count).
To fix, replace _bt_preprocess_array_keys_final's assertion with a test
that makes the function return early when the scan has no SAOP equality
scan keys.
Oversight in commit
1b134ca5, which enhanced nbtree ScalarArrayOp
execution.
Reported-By: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/
0539d3d3-a402-0a49-ed5e-
26429dffc4bd@gmail.com
Heikki Linnakangas [Sun, 7 Apr 2024 17:21:27 +0000 (20:21 +0300)]
Don't clobber test exit code at cleanup in LDAP/Kerberors tests
If the test script die()d before running the first test, the whole test
was interpreted as SKIPped rather than failed. The PostgreSQL::Cluster
module got this right.
Backpatch to all supported versions.
Discussion: https://www.postgresql.org/message-id/
fb898a70-3a88-4629-88e9-
f2375020061d@iki.fi
Heikki Linnakangas [Sun, 7 Apr 2024 17:21:21 +0000 (20:21 +0300)]
Improve check in LDAP test to find the OpenLDAP installation
If the OpenLDAP installation directory is not found, set $setup to 0
so that the LDAP tests are skipped. The macOS checks were already
doing that, but the checks on other OS's were not. While we're at it,
improve the error message when the tests are skipped, to specify
whether the OS is supported at all, or if we just didn't find the
installation directory.
This was accidentally "working" without this, i.e. we were skipping
the tests if the OpenLDAP installation was not found, because of a bug
in the LdapServer test module: the END block clobbered the exit code
so if the script die()s before running the first subtest, the whole
test script was marked as SKIPped. The next commit will fix that bug,
but we need to fix the setup code first.
These checks should probably go into configure/meson, but this is
better than nothing and allows fixing the bug in the END block.
Backpatch to all supported versions.
Discussion: https://www.postgresql.org/message-id/
fb898a70-3a88-4629-88e9-
f2375020061d@iki.fi