users/c2main/postgres.git
13 months agoCleanup parallel BRIN index build code
Tomas Vondra [Wed, 17 Apr 2024 16:31:43 +0000 (18:31 +0200)]
Cleanup parallel BRIN index build code

Commit b43757171470 added support for parallel builds of BRIN indexes,
using code similar to BTREE. But there were to be a couple unnecessary
differences, particularly in how the leader waits for the workers, and
merges the results. So remove these, to make the code more similar.

The leader never waited on the workersdonecv condition variable, but
simply called WaitForParallelWorkersToFinish() in _brin_end_parallel()
and then merged the per-worker results. This worked correctly, but it
seems better to do the wait and merge before _brin_end_parallel().

This commit moves the relevant code to _brin_parallel_heapscan/merge(),
which means  _brin_end_parallel() remains responsible only for exiting
the parallel mode and accumulating WAL usage data.

Discussion: https://postgr.es/m/3733d042-71e1-6ae6-5fac-00c12db62db6@enterprisedb.com

13 months agoStabilize test of BRIN parallel create
Tomas Vondra [Wed, 17 Apr 2024 14:14:44 +0000 (16:14 +0200)]
Stabilize test of BRIN parallel create

As explained in 4d916dd876, the test instability is caused by delayed
cleanup of deleted rows. This commit removes the DELETE, stabilizing the
test without accidentally disabling parallel builds.

The intent of the delete however was to produce empty ranges, and test
that the parallel index build populates those correctly. But there's
another way to create empty ranges - partial indexes, which does not
rely on cleanup of deleted rows.

Idea to use partial indexes by Matthias van de Meent, patch by me.

Discussion: https://postgr.es/m/95d9cd43-5a92-407c-b7e4-54cd303630fe%40enterprisedb.com

13 months agoRevert "Stabilize test of BRIN parallel create"
Tomas Vondra [Wed, 17 Apr 2024 14:12:03 +0000 (16:12 +0200)]
Revert "Stabilize test of BRIN parallel create"

This reverts commit 4d916dd876c3. The goal of that commit was to
stabilize a test of parallel BRIN build, but using a TEMPORARY table
disables parallel index builds on that table, making the test useless.

Discussion: https://postgr.es/m/95d9cd43-5a92-407c-b7e4-54cd303630fe%40enterprisedb.com

13 months agomeson: Add some missing LLVM function checks
Peter Eisentraut [Wed, 17 Apr 2024 13:16:39 +0000 (15:16 +0200)]
meson: Add some missing LLVM function checks

The checks for

HAVE_DECL_LLVMCREATEGDBREGISTRATIONLISTENER and
HAVE_DECL_LLVMCREATEPERFJITEVENTLISTENER

are in configure but are missing on the meson side.  This adds those.

Reported-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://www.postgresql.org/message-id/5539b16c-cff7-46d5-9621-c3fb6b549e9e@iki.fi

13 months agoRemove dead code
Peter Eisentraut [Wed, 17 Apr 2024 08:46:43 +0000 (10:46 +0200)]
Remove dead code

The configure check for HAVE_DECL_LLVMORCREGISTERPERF was removed by
e9a9843e138, but some code guarded by it was left.  (That commit
removed the "register" calls but left the "unregister" calls.)  That
code cannot be reached anymore, so remove it.

Reported-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://www.postgresql.org/message-id/5539b16c-cff7-46d5-9621-c3fb6b549e9e@iki.fi

13 months agoAdd missing source file to libpq/nls.mk
Peter Eisentraut [Wed, 17 Apr 2024 07:09:53 +0000 (09:09 +0200)]
Add missing source file to libpq/nls.mk

13 months agodoc: Fix COPY ON_ERROR option syntax synopsis.
Masahiko Sawada [Wed, 17 Apr 2024 07:10:18 +0000 (16:10 +0900)]
doc: Fix COPY ON_ERROR option syntax synopsis.

ON_ERROR option values don't require quotations, which was
inconsistent with the syntax synopsis in the documentation.

Oversight in b725b7eec43.

Author: Atsushi Torikoshi
Reviewed-by: Masahiko Sawada
Discussion: https://postgr.es/m/CAD21AoC%3Dn4xR3%2BKQiqodnfT9chSB62XwZqmMff39H%3Dx9DS4scQ%40mail.gmail.com

13 months agoFix typos with function name in event_trigger.c
Michael Paquier [Wed, 17 Apr 2024 05:56:31 +0000 (14:56 +0900)]
Fix typos with function name in event_trigger.c

Databases exist, contrary to datatabases.

Oversight in e83d1b0c40cc.

Author: Japin Li
Discussion: https://postgr.es/m/ME3P282MB316611A2F7BF43919F695228B6082@ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM

13 months agoDisallow specifying ON_ERROR option without value.
Masahiko Sawada [Wed, 17 Apr 2024 02:31:27 +0000 (11:31 +0900)]
Disallow specifying ON_ERROR option without value.

The ON_ERROR option of the COPY command previously allowed omitting
its value, which was inconsistent with the syntax synopsis in the
documentation and the behavior of other non-boolean COPY options.

This change enforces providing a value for the ON_ERROR option,
ensuring consistency across other non-boolean options and aligning
with the documented syntax.

Author: Atsushi Torikoshi
Reviewed-by: Masahiko Sawada
Discussion: https://postgr.es/m/a9770bf57646d90dedc3d54cf32634b2%40oss.nttdata.com

13 months agoUpdate mmgr's README to mention BumpContext
David Rowley [Tue, 16 Apr 2024 22:49:09 +0000 (10:49 +1200)]
Update mmgr's README to mention BumpContext

Oversight in 29f6a959c.

In passing, since we now have 4 memory context types to choose from,
provide a brief overview of the specialities of each memory context
type.

Reported-by: Amul Sul
Author: Amul Sul, David Rowley
Discussion: https://postgr.es/m/CAAJ_b94U2s9nHh--DEK=sPEZUQ+x7vQJ7529fF8UAH97QJ9NXg@mail.gmail.com

13 months agoPush dedicated BumpBlocks to the tail of the blocks list
David Rowley [Tue, 16 Apr 2024 22:40:31 +0000 (10:40 +1200)]
Push dedicated BumpBlocks to the tail of the blocks list

BumpContext relies on using the head block from its 'blocks' field to
use as the current block to allocate new chunks to.  When we receive an
allocation request larger than allocChunkLimit, we place these chunks on
a new dedicated block and, until now, we pushed the block onto the
*head* of the 'blocks' list.

This behavior caused the previous bump block to no longer be available
for new normal-sized (non-large) allocations and would result in blocks
only being partially filled if a large allocation request arrived before
the block became full.

Here adjust the code to push these dedicated blocks onto the *tail* of
the blocks list so that the head block remains intact and available to
be used by normal allocation request sizes until it becomes full.

In passing, make the elog(ERROR) calls for the unsupported callbacks
consistent.  Likewise for the header comments for those functions.

Discussion: https://postgr.es/m/CAApHDvp9___r-ayJj0nZ6GD3MeCGwGZ0_6ZptWpwj+zqHtmwCw@mail.gmail.com
Discussion: https://postgr.es/m/CAApHDvqerXpzUnuDQfUEi3DZA+9=Ud9WSt3ruxN5b6PcOosx2g@mail.gmail.com

13 months agoMark some new location fields as ParseLoc
Peter Eisentraut [Tue, 16 Apr 2024 18:22:41 +0000 (20:22 +0200)]
Mark some new location fields as ParseLoc

Some new code probably didn't see 605721f819f and continued to use
type int for parse location fields.  Fix those.

13 months agoClean up more indent breakage from 6377e12a5.
Tom Lane [Tue, 16 Apr 2024 17:00:40 +0000 (13:00 -0400)]
Clean up more indent breakage from 6377e12a5.

Per buildfarm member koel.

13 months agoFix assorted bugs in ecpg's macro mechanism.
Tom Lane [Tue, 16 Apr 2024 16:31:32 +0000 (12:31 -0400)]
Fix assorted bugs in ecpg's macro mechanism.

The code associated with EXEC SQL DEFINE was unreadable and full of
bugs, notably:

* It'd attempt to free a non-malloced string if the ecpg program
tries to redefine a macro that was defined on the command line.

* Possible memory stomp if user writes "-D=foo".

* Undef'ing or redefining a macro defined on the command line would
change the state visible to the next file, when multiple files are
specified on the command line.  (While possibly that could have been
an intentional choice, the code clearly intends to revert to the
original macro state; it's just failing to consider this interaction.)

* Missing "break" in defining a new macro meant that redefinition
of an existing name would cause an extra entry to be added to the
definition list.  While not immediately harmful, a subsequent undef
would result in the prior entry becoming visible again.

* The interactions with input buffering are subtle and were entirely
undocumented.

It's not that surprising that we hadn't noticed these bugs,
because there was no test coverage at all of either the -D
command line switch or multiple input files.  This patch adds
such coverage (in a rather hacky way I guess).

In addition to the code bugs, the user documentation was confused
about whether the -D switch defines a C macro or an ecpg one, and
it failed to mention that you can write "-Dsymbol=value".

These problems are old, so back-patch to all supported branches.

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

13 months agoFix nbtree "deduce NOT NULL" scan key comment.
Peter Geoghegan [Tue, 16 Apr 2024 16:04:20 +0000 (12:04 -0400)]
Fix nbtree "deduce NOT NULL" scan key comment.

Oversight in commit c9c0589fda.

13 months agoUndo incorrect typedefs.list change.
Tom Lane [Tue, 16 Apr 2024 15:37:51 +0000 (11:37 -0400)]
Undo incorrect typedefs.list change.

6377e12a5 should not have removed AcquireSampleRowsFunc,
as that's still in use.  Per buildfarm member koel.

13 months agoStabilize test of BRIN parallel create
Tomas Vondra [Tue, 16 Apr 2024 15:31:03 +0000 (17:31 +0200)]
Stabilize test of BRIN parallel create

The test for parallel create of BRIN indexes added by commit 8225c2fd40
happens to be unstable - a background transaction (e.g. auto-analyze)
may hold back global xmin for the initial VACUUM / CREATE INDEX. If the
cleanup happens before the next CREATE INDEX, the indexes will not be
exactly the same.

This is the same issue as e2933a6e11, so fix it the same way by making
the table TEMPORARY, which uses an up-to-date cutoff xmin that is not
held back by other processes.

Reported by Alexander Lakhin, who also suggested the fix.

Author: Alexander Lakhin
Discussion: https://postgr.es/m/b58901cd-a7cc-29c6-e2b1-e3d7317c3c69@gmail.com
Discussion: https://postgr.es/m/2892135.1668976646@sss.pgh.pa.us

13 months agoEnsure generated join clauses for child rels have correct relids.
Tom Lane [Tue, 16 Apr 2024 15:03:43 +0000 (11:03 -0400)]
Ensure generated join clauses for child rels have correct relids.

When building a join clause derived from an EquivalenceClass, if the
clause is to be used with an appendrel child relation then make sure
its clause_relids include the relids of that child relation.
Normally this would be true already because the EquivalenceMember
would be a Var of that relation.  However, if the appendrel represents
a flattened UNION ALL construct then some child EquivalenceMembers
could be constants with no relids.  The resulting under-marked clause
is problematic because it could mislead join_clause_is_movable_into
about where the clause should be evaluated.  We do not have an example
showing incorrect plan generation, but there are existing cases in
the regression tests that will fail the Asserts this patch adds to
get_baserel_parampathinfo.  A similarly wrong conclusion about a
clause being considered by get_joinrel_parampathinfo would lead to
wrong placement of the clause.  (This also squares with the way
that clause_relids is calculated for non-equijoin clauses in
adjust_appendrel_attrs.)

The other reason for wanting these new Asserts is that the previous
blithe assumption that the results of generate_join_implied_equalities
"necessarily satisfy join_clause_is_movable_into" turns out to be
wrong pre-v16.  If it's still wrong it'd be good to find out.

Per bug #18429 from Benoît Ryder.  The bug as filed was fixed by
commit 2489d76c4, but these changes correlate with the fix we
will need to apply in pre-v16 branches.

Discussion: https://postgr.es/m/18429-8982d4a348cc86c6@postgresql.org

13 months agoFix nbtree posting list comment.
Peter Geoghegan [Tue, 16 Apr 2024 15:20:41 +0000 (11:20 -0400)]
Fix nbtree posting list comment.

Oversight in commit 0d861bbb70.

13 months agoFix nbtree page recycling comment.
Peter Geoghegan [Tue, 16 Apr 2024 15:14:36 +0000 (11:14 -0400)]
Fix nbtree page recycling comment.

Oversight in commit e5d8a99903.

13 months agorevert: Generalize relation analyze in table AM interface
Alexander Korotkov [Tue, 16 Apr 2024 10:14:20 +0000 (13:14 +0300)]
revert: Generalize relation analyze in table AM interface

This commit reverts 27bc1772fc and dd1f6b0c17.  Per review by Andres Freund.

Discussion: https://postgr.es/m/20240415201057.khoyxbwwxfgzomeo%40awork3.anarazel.de

13 months agoImprove test coverage in bump.c
David Rowley [Tue, 16 Apr 2024 04:21:31 +0000 (16:21 +1200)]
Improve test coverage in bump.c

There were no callers of BumpAllocLarge() in the regression tests, so
here we add a sort with a tuple large enough to use that path in bump.c.

Also, BumpStats() wasn't being called, so add a test to sysviews.sql to
call pg_backend_memory_contexts() while a bump context exists in the
backend.

Reported-by: Andres Freund
Discussion: https://postgr.es/m/20240414223305.m3i5eju6zylabvln@awork3.anarazel.de

13 months agoAdd missing PGDLLIMPORT markings
Michael Paquier [Tue, 16 Apr 2024 00:38:46 +0000 (09:38 +0900)]
Add missing PGDLLIMPORT markings

All backend-side variables should be marked with PGDLLIMPORT, as per
policy introduced in 8ec569479f.  aafc05de1bf5 has forgotten
MyClientSocket, and 05c3980e7f47 LoadedSSL.

These can be spotted with a command like this one (be careful of not
switching __pg_log_level):
src/tools/mark_pgdllimport.pl $(git ls-files src/include/)

Reported-by: Peter Eisentraut
Discussion: https://postgr.es/m/ZhzkRzrkKhbeQMRm@paquier.xyz

13 months agodocs: Consolidate into new "WAL for Extensions" chapter.
Robert Haas [Mon, 15 Apr 2024 19:46:19 +0000 (15:46 -0400)]
docs: Consolidate into new "WAL for Extensions" chapter.

Previously, we had consecutive, very short chapters called "Generic
WAL" and "Custom WAL Resource Managers," explaining different approaches
to the same problem. Merge them into a single chapter. Explain most
of the differences between the approaches in the chapter's
introductory text, rather than in the individual sections.

Discussion: http://postgr.es/m/46ac50c1-6b2a-404f-a683-b67af6ab56e9@eisentraut.org

13 months agodoc: Note exceptions for SET ROLE's effect on privilege checks.
Nathan Bossart [Mon, 15 Apr 2024 19:03:24 +0000 (14:03 -0500)]
doc: Note exceptions for SET ROLE's effect on privilege checks.

The documentation for SET ROLE states that superusers who switch to
a non-superuser role lose their superuser privileges.  While this
is true for most commands, there are exceptions such as SET ROLE
and SET SESSION AUTHORIZATION, which continue to use the current
session user and the authenticated user, respectively.
Furthermore, the description of this command already describes its
effect, so it is arguably unnecessary to include this special case.
This commit removes the note about the superuser case and adds a
sentence about the aforementioned exceptions to the description.

Co-authored-by: Yurii Rashkovskii
Reviewed-by: Shubham Khanna, Robert Haas, Michael Paquier
Discussion: https://postgr.es/m/CA%2BRLCQysHtME0znk2KUMJN343ksboSRQSU-hCnOjesX6VK300Q%40mail.gmail.com

13 months agoFix type-checking of RECORD-returning functions in FROM, redux.
Tom Lane [Mon, 15 Apr 2024 16:56:56 +0000 (12:56 -0400)]
Fix type-checking of RECORD-returning functions in FROM, redux.

Commit 2ed8f9a01 intended to institute a policy that if a
RangeTblFunction has a coldeflist, then the function return type is
certainly RECORD, and we should use the coldeflist as the source of
truth about what the columns of the record type are.  When the
original function has been folded to a constant, inspection of the
constant might give a different answer.  This situation will lead to
a tuple-type-mismatch error at execution, but up until that point we
need to consistently believe the coldeflist, or we'll have problems
from different bits of code reaching different conclusions.

expandRTE didn't get that memo though, and would try to produce a
tupdesc based on the constant in this situation, leading to an
assertion failure.  (Desultory testing suggests that non-assert
builds often manage to give the expected error, although I also
saw a "cache lookup failed for type 0" error, and it seems at
least possible that a crash could happen.)

Some other callers of get_expr_result_type and get_expr_result_tupdesc
were also being incautious about this.  While none of them seem to
have actual bugs, they're working harder than necessary in this case,
besides which it seems safest to have an explicit policy of not using
those functions on an RTE with a coldeflist.  Adjust the code
accordingly, and add commentary to funcapi.c about this policy.

Also fix an obsolete comment that claimed "get_expr_result_type()
doesn't know how to extract type info from a RECORD constant".
That hasn't been true since commit d57534740.

Per bug #18422 from Alexander Lakhin.
As with the previous commit, back-patch to all supported branches.

Discussion: https://postgr.es/m/18422-89ca86c8eac5246d@postgresql.org

13 months agoATTACH PARTITION: Don't match a PK with a UNIQUE constraint
Alvaro Herrera [Mon, 15 Apr 2024 13:07:47 +0000 (15:07 +0200)]
ATTACH PARTITION: Don't match a PK with a UNIQUE constraint

When matching constraints in AttachPartitionEnsureIndexes() we weren't
testing the constraint type, which could make a UNIQUE key lacking a
not-null constraint incorrectly satisfy a primary key requirement.  Fix
this by testing that the constraint types match.  (Other possible
mismatches are verified by comparing index properties.)

Discussion: https://postgr.es/m/202402051447.wimb4xmtiiyb@alvherre.pgsql

13 months agoGrammar fixes for split/merge partitions code
Alexander Korotkov [Mon, 15 Apr 2024 12:41:37 +0000 (15:41 +0300)]
Grammar fixes for split/merge partitions code

The fixes relate to comments, error messages, and corresponding expected output
of regression tests.

Discussion: https://postgr.es/m/CAMbWs49DDsknxyoycBqiE72VxzL_sYHF6zqL8dSeNehKPJhkKg%40mail.gmail.com
Discussion: https://postgr.es/m/86bfd241-a58c-479a-9a72-2c67a02becf8%40postgrespro.ru
Discussion: https://postgr.es/m/CAHewXNkGMPU50QG7V6Q60JGFORfo8LfYO1_GCkCa0VWbmB-fEw%40mail.gmail.com
Author: Richard Guo, Dmitry Koval, Tender Wang

13 months agoFix propagating attnotnull in multiple inheritance
Alvaro Herrera [Mon, 15 Apr 2024 10:20:56 +0000 (12:20 +0200)]
Fix propagating attnotnull in multiple inheritance

In one of the many strange corner cases of multiple inheritance being
used, commit b0e96f311985 missed a CommandCounterIncrement() call after
updating the attnotnull flag during ALTER TABLE ADD COLUMN, which caused
a catalog tuple to be update attempted twice in the same command, giving
rise to a "tuple already updated by self" error.  Add the missing call
to solve that, and a test case that reproduces the scenario.

As a (perhaps surprising) secondary effect, this CCI addition triggers
another behavior change: when a primary key is added to a parent
partitioned table and the column in an existing partition does not have
a not-null constraint, we no longer error out.  This will probably be a
welcome change by some users, and I think it's unlikely that anybody
will miss the old behavior.

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: http://postgr.es/m/045dec3f-9b3d-aa44-0c99-85f6992306c7@gmail.com

13 months agopsql: Make output of \dD more stable
Peter Eisentraut [Mon, 15 Apr 2024 07:28:48 +0000 (09:28 +0200)]
psql: Make output of \dD more stable

\dD showed domain check constraints in arbitrary order, which can
cause regression test failures, which was exposed by commit
9895b35cb8.  To fix, order the constraints by conname, which matches
what psql does in other queries listing constraints.

13 months agoFix ALTER DOMAIN NOT NULL syntax
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

13 months agoPut back initialization of 'sslmode', to silence Coverity
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.

13 months agoFix unnecessary padding in incremental backups
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.

13 months agoAdd regression test for BRIN parallel builds
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

13 months agoUse the correct PG_DETOAST_DATUM macro in BRIN
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

13 months agoUpdate nbits_set in brin_bloom_union
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

13 months agofreespace: Don't return blocks past the end of the main fork.
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

13 months agoCorrect "improve role option documentation".
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

13 months agoDocument PG_TEST_EXTRA=libpq_encryption and also check 'kerberos'
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.

13 months agoMove libpq encryption negotiation tests
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

13 months agoFix compilation with --with-gssapi --without-openssl
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

13 months agoFix libpq_encryption tests when compiled without SSL support
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

13 months agoDon't allocate large buffer on the stack in pg_verifybackup
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

13 months agoAssorted minor cleanups in the test_json_parser module
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.

13 months agoShrink test file for test_json_parser module
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

13 months agoAdd a TAP test for test_json_parser_perf
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.

13 months agoFix some memory leaks associated with parsing json and manifests
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.

13 months agoFix recently introduced typo in code comment
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

13 months agoFix the review comments and a bug in the slot sync code.
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

13 months agoFix IS [NOT] NULL qual optimization for inheritance tables
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

13 months agoDoc: fix bogus to_date() examples.
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

13 months agoRevert: Implement pg_wal_replay_wait() stored procedure
Alexander Korotkov [Thu, 11 Apr 2024 13:30:32 +0000 (16:30 +0300)]
Revert: Implement pg_wal_replay_wait() stored procedure

This commit reverts 06c418e163e37662f221bf1e65080625f42429e2,
ee79928441, and 74eaf66f98 per review by Heikki Linnakangas.

Discussion: https://postgr.es/m/b155606b-e744-4218-bda5-29379779da1a%40iki.fi

13 months agoRevert: Allow table AM to store complex data structures in rd_amcache
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

13 months agoRevert: Allow table AM tuple_insert() method to return the different slot
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

13 months agoRevert: Allow locking updated tuples in tuple_update() and tuple_delete()
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

13 months agoRevert: Let table AM insertion methods control index insertion
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

13 months agoRevert: Custom reloptions for table AM
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

13 months agomeson: Remove obsolete function test
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.

13 months agopostgres_fdw: Improve comment about handling of asynchronous requests.
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

13 months agoUse correct datatype for xmin variables in slot.c
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

13 months agoRevert indexed and enlargable binary heap implementation.
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

13 months agoReplace binaryheap + index with pairingheap in reorderbuffer.c
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

13 months agoFix grammar.
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

13 months agoFix potential stack overflow in incremental backup.
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

13 months agoFix inconsistency with replay of hash squeeze record for clean buffers
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

13 months agoFix plpgsql's handling of -- comments following expressions.
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

13 months agoDoc: Update ulinks to RFC documents to avoid redirect
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

13 months agoMake GIN tests using injection points concurrent-safe
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

13 months agoFix a test in failover slots regression test.
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

13 months agoFix illegal attribute propagation in LLVM JIT.
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

13 months agoFixup various StringInfo function usages
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
13 months agorevert: Transform OR clauses to ANY expression
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

13 months agoRemove unused BumpBlockIsValid macro
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

13 months agoChecks for ALTER TABLE ... SPLIT/MERGE PARTITIONS ... commands
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

13 months agoDoc: use "an SQL" instead of "a SQL"
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

13 months agodoc: Remove stray comma from list of psql options
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

13 months agoFix incorrect format placeholders
Peter Eisentraut [Tue, 9 Apr 2024 12:33:06 +0000 (14:33 +0200)]
Fix incorrect format placeholders

13 months agoUpdate config.guess and config.sub
Peter Eisentraut [Tue, 9 Apr 2024 12:21:57 +0000 (14:21 +0200)]
Update config.guess and config.sub

13 months agoFix whitespace
Peter Eisentraut [Tue, 9 Apr 2024 09:32:48 +0000 (11:32 +0200)]
Fix whitespace

13 months agoGet rid of anonymous struct
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

13 months agolibpq error message fixes
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

13 months agoFix typo in docs
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

13 months agoAdd missing set_pglocale_pgservice() for pg_walsummary and pg_combinebackup
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

13 months agoinjection_points: Fix race condition with local injection point tests
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

13 months agoIn psql, avoid leaking a PGresult after a query is cancelled.
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.

13 months agoFurther review for re-implementation of psql's FETCH_COUNT feature.
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

13 months agoTeach radix tree to embed values at runtime
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

13 months agoTeach TID store to skip bitmap for small numbers of offsets
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

13 months agoProvide a way block-level table AMs could re-use acquire_sample_rows()
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
13 months agoFix some grammer errors from error messages and codes comments
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

13 months agoFill CommonRdOptions with default values in extract_autovac_opts()
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

13 months agoAdjust wording of trace_connection_negotiation GUC's description
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

13 months agoCustom reloptions for table AM
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
13 months agoFix the intermittent buildfarm failures in 040_standby_failover_slots_sync.
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

13 months agoUse bump context for TID bitmaps stored by vacuum
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

13 months agoJSON_TABLE: Add support for NESTED paths and columns
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

13 months agoFix JsonExpr deparsing to emit QUOTES and WRAPPER correctly
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

13 months agoFix restriction on specifying KEEP QUOTES in JSON_QUERY()
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.

13 months agoFix the wording of or_to_any_transform_limit description
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

13 months agoFix the value of or_to_any_transform_limit in postgresql.conf.sample
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