postgresql.git
9 months agopg_createsubscriber: Remove obsolete comment
Peter Eisentraut [Tue, 30 Jul 2024 10:21:20 +0000 (12:21 +0200)]
pg_createsubscriber: Remove obsolete comment

This comment should have been removed by commit b9639138262.  There is
no replication slot check on the primary anymore.

Author: Euler Taveira <euler@eulerto.com>
Discussion: https://www.postgresql.org/message-id/697d692f-f9d3-41f6-9f0e-29a4fb18e544@app.fastmail.com

9 months agoStabilize xid_wraparound tests
Andrew Dunstan [Tue, 30 Jul 2024 10:17:48 +0000 (06:17 -0400)]
Stabilize xid_wraparound tests

The tests had a race condition if autovacuum was set to off. Instead we
create all the tables we are interested in with autovacuum disabled, so
they are only ever touched when in danger of wraparound.

Discussion: https://postgr.es/m/3e2cbd24-f45e-4b2b-ba83-8149214f0a4d@dunslane.net

Masahiko Sawada (slightly tweaked by me)

Backpatch to release 17 where these tests were introduced.

9 months agopg_createsubscriber: Fix an unpredictable recovery wait time.
Amit Kapila [Tue, 30 Jul 2024 08:31:01 +0000 (14:01 +0530)]
pg_createsubscriber: Fix an unpredictable recovery wait time.

The problem is that the tool is using the LSN returned by
pg_create_logical_replication_slot() as recovery_target_lsn. This LSN is
ahead of the current WAL position and the recovery waits until the
publisher writes a WAL record to reach the target and ends the recovery.
On idle systems, this wait time is unpredictable and could lead to failure
in promoting the subscriber. To avoid that, insert a harmless WAL record.

Reported-by: Alexander Lakhin and Tom Lane
Diagnosed-by: Hayato Kuroda
Author: Euler Taveira
Reviewed-by: Hayato Kuroda, Amit Kapila
Backpatch-through: 17
Discussion: https://postgr.es/m/2377319.1719766794%40sss.pgh.pa.us
Discussion: https://postgr.es/m/CA+TgmoYcY+Wb67NAwaHT7MvxCSeV86oSc+va9hHKaasE42ukyw@mail.gmail.com

9 months agoDisallow setting MAX_PARTITION_BUFFERS to less than 2
David Rowley [Tue, 30 Jul 2024 08:19:59 +0000 (20:19 +1200)]
Disallow setting MAX_PARTITION_BUFFERS to less than 2

Add some comments to mention that this value must be at least 2 and also
add a StaticAssertDecl to cause compilation failure if anyone tries to
build with an invalid value.

The multiInsertBuffers list must have at least two elements due to how the
code in CopyMultiInsertInfoFlush() pushes the current ResultRelInfo's
CopyMultiInsertBuffer to the end of the list.  If the first element is
also the last element, bad things will happen.

Author: Zhang Mingli <avamingli@gmail.com>
Discussion: https://postgr.es/m/CAApHDvpQ6t9ROcqbD-OgqR04Kfq4vQKw79Vo6r5j%2BciHwsSfkA%40mail.gmail.com

9 months agoMake collation not depend on setlocale().
Jeff Davis [Tue, 30 Jul 2024 07:58:06 +0000 (00:58 -0700)]
Make collation not depend on setlocale().

Now that the result of pg_newlocale_from_collation() is always
non-NULL, then we can move the collate_is_c and ctype_is_c flags into
pg_locale_t. That simplifies the logic in lc_collate_is_c() and
lc_ctype_is_c(), removing the dependence on setlocale().

This commit also eliminates the multi-stage initialization of the
collation cache.

As long as we have catalog access, then it's now safe to call
pg_newlocale_from_collation() without checking lc_collate_is_c()
first.

Discussion: https://postgr.es/m/cfd9eb85-c52a-4ec9-a90e-a5e4de56e57d@eisentraut.org
Reviewed-by: Peter Eisentraut, Andreas Karlsson
9 months agoFix partitionwise join with partially-redundant join clauses
Richard Guo [Tue, 30 Jul 2024 06:51:54 +0000 (15:51 +0900)]
Fix partitionwise join with partially-redundant join clauses

To determine if the two relations being joined can use partitionwise
join, we need to verify the existence of equi-join conditions
involving pairs of matching partition keys for all partition keys.
Currently we do that by looking through the join's restriction
clauses.  However, it has been discovered that this approach is
insufficient, because there might be partition keys known equal by a
specific EC, but they do not form a join clause because it happens
that other members of the EC than the partition keys are constrained
to become a join clause.

To address this issue, in addition to examining the join's restriction
clauses, we also check if any partition keys are known equal by ECs,
by leveraging function exprs_known_equal().  To accomplish this, we
enhance exprs_known_equal() to check equality per the semantics of the
opfamily, if provided.

It could be argued that exprs_known_equal() could be called O(N^2)
times, where N is the number of partition key expressions, resulting
in noticeable performance costs if there are a lot of partition key
expressions.  But I think this is not a problem.  The number of a
joinrel's partition key expressions would only be equal to the join
degree, since each base relation within the join contributes only one
partition key expression.  That is to say, it does not scale with the
number of partitions.  A benchmark with a query involving 5-way joins
of partitioned tables, each with 3 partition keys and 1000 partitions,
shows that the planning time is not significantly affected by this
patch (within the margin of error), particularly when compared to the
impact caused by partitionwise join.

Thanks to Tom Lane for the idea of leveraging exprs_known_equal() to
check if partition keys are known equal by ECs.

Author: Richard Guo, Tom Lane
Reviewed-by: Tom Lane, Ashutosh Bapat, Robert Haas
Discussion: https://postgr.es/m/CAN_9JTzo_2F5dKLqXVtDX5V6dwqB0Xk+ihstpKEt3a1LT6X78A@mail.gmail.com

9 months agoRefactor the checks for parameterized partial paths
Richard Guo [Tue, 30 Jul 2024 06:49:44 +0000 (15:49 +0900)]
Refactor the checks for parameterized partial paths

Parameterized partial paths are not supported, and we have several
checks in try_partial_xxx_path functions to enforce this.  For a
partial nestloop join path, we need to ensure that if the inner path
is parameterized, the parameterization is fully satisfied by the
proposed outer path.  For a partial merge/hashjoin join path, we need
to ensure that the inner path is not parameterized.  In all cases, we
need to ensure that the outer path is not parameterized.

However, the comment in try_partial_hashjoin_path does not describe
this correctly.  This patch fixes that.

In addtion, this patch simplifies the checks peformed in
try_partial_hashjoin_path and try_partial_mergejoin_path with the help
of macro PATH_REQ_OUTER, and also adds asserts that the outer path is
not parameterized in try_partial_xxx_path functions.

Author: Richard Guo
Discussion: https://postgr.es/m/CAMbWs48mKJ6g_GnYNa7dnw04MHaMK-jnAEBrMVhTp2uUg3Ut4A@mail.gmail.com

9 months agoShort-circuit sort_inner_and_outer if there are no mergejoin clauses
Richard Guo [Tue, 30 Jul 2024 06:46:39 +0000 (15:46 +0900)]
Short-circuit sort_inner_and_outer if there are no mergejoin clauses

In sort_inner_and_outer, we create mergejoin join paths by explicitly
sorting both relations on each possible ordering of the available
mergejoin clauses.  However, if there are no available mergejoin
clauses, we can skip this process entirely.

This patch introduces a check for mergeclause_list at the beginning of
sort_inner_and_outer and exits the function if it is found to be
empty.  This might help skip all the statements that come before the
call to select_outer_pathkeys_for_merge, including the build of
UniquePaths in the case of JOIN_UNIQUE_OUTER or JOIN_UNIQUE_INNER.

I doubt there's any measurable performance improvement, but throughout
the run of the regression tests, sort_inner_and_outer is called a
total of 44,424 times.  Among these calls, there are 11,064 instances
where mergeclause_list is found to be empty, which accounts for
approximately one-fourth.  I think this suggests that implementing
this shortcut is worthwhile.

Author: Richard Guo
Reviewed-by: Ashutosh Bapat
Discussion: https://postgr.es/m/CAMbWs48RKiZGFEd5A0JtztRY5ZdvVvNiHh0AKeuoz21F+0dVjQ@mail.gmail.com

9 months agoAdd more debugging information when failing to read pgstats files
Michael Paquier [Tue, 30 Jul 2024 06:08:21 +0000 (15:08 +0900)]
Add more debugging information when failing to read pgstats files

This is useful to know which part of a stats file is corrupted when
reading it, adding to the server logs a WARNING with details about what
could not be read before giving up with the remaining data in the file.

Author: Michael Paquier
Reviewed-by: Bertrand Drouvot
Discussion: https://postgr.es/m/Zp8o6_cl0KSgsnvS@paquier.xyz

9 months agoSQL/JSON: Fix casting for integer EXISTS columns in JSON_TABLE
Amit Langote [Tue, 30 Jul 2024 01:12:23 +0000 (10:12 +0900)]
SQL/JSON: Fix casting for integer EXISTS columns in JSON_TABLE

The current method of coercing the boolean result value of
JsonPathExists() to the target type specified for an EXISTS column,
which is to call the type's input function via json_populate_type(),
leads to an error when the target type is integer, because the
integer input function doesn't recognize boolean literal values as
valid.

Instead use the boolean-to-integer cast function for coercion in that
case so that using integer or domains thereof as type for EXISTS
columns works. Note that coercion for ON ERROR values TRUE and FALSE
already works like that because the parser creates a cast expression
including the cast function, but the coercion of the actual result
value is not handled by the parser.

Tests by Jian He.

Reported-by: Jian He <jian.universality@gmail.com>
Author: Jian He <jian.universality@gmail.com>
Author: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Backpatch-through: 17

9 months agoSQL/JSON: Some fixes to JsonBehavior expression casting
Amit Langote [Tue, 30 Jul 2024 01:11:23 +0000 (10:11 +0900)]
SQL/JSON: Some fixes to JsonBehavior expression casting

1. Remove the special case handling when casting the JsonBehavior
   expressions to types with typmod, like 86d33987 did for the casting
   of SQL/JSON constructor functions.

2. Fix casting for fixed-length character and bit string types by
   using assignment-level casts.  This is again similar to what
   86d33987 did, but for ON ERROR / EMPTY expressions.

3. Use runtime coercion for the boolean ON ERROR constants so that
   using fixed-length character string types, for example, for an
   EXISTS column doesn't cause a "value too long for type
   character(n)" when the parser tries to coerce the default ON ERROR
   value "false" to that type, that is, even when clause is not
   specified.

4. Simplify the conditions of when to use runtime coercion vs
   creating the cast expression in the parser itself.  jsonb-valued
   expressions are now always coerced at runtime and boolean
   expressions too if the target type is a string type for the
   reasons mentioned above.

Tests are taken from a patch that Jian He posted.

Reported-by: Jian He <jian.universality@gmail.com>
Author: Jian He <jian.universality@gmail.com>
Author: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Backpatch-through: 17

9 months agoDo not return NULL from pg_newlocale_from_collation().
Jeff Davis [Mon, 29 Jul 2024 22:15:11 +0000 (15:15 -0700)]
Do not return NULL from pg_newlocale_from_collation().

Previously, pg_newlocale_from_collation() returned NULL as a special
case for the DEFAULT_COLLATION_OID if the provider was libc. In that
case the behavior would depend on the last call to setlocale().

Now, consistent with the other providers, it will return a pointer to
default_locale, which is not dependent on setlocale().

Note: for the C and POSIX locales, the locale_t structure within the
pg_locale_t will still be zero, because those locales are implemented
with internal logic and do not use libc at all.

lc_collate_is_c() and lc_ctype_is_c() still depend on setlocale() to
determine the current locale, which will be removed in a subsequent
commit.

Discussion: https://postgr.es/m/cfd9eb85-c52a-4ec9-a90e-a5e4de56e57d@eisentraut.org
Reviewed-by: Peter Eisentraut, Andreas Karlsson
9 months agoDetach syslogger from shared memory
Heikki Linnakangas [Mon, 29 Jul 2024 19:21:34 +0000 (22:21 +0300)]
Detach syslogger from shared memory

Commit aafc05de1b removed the calls to detach from shared memory from
syslogger startup. That was not intentional, so put them back.

Author: Rui Zhao
Reviewed-by: Aleksander Alekseev
Backpatch-through: 17
Discussion: https://www.postgresql.org/message-id/11505016-8cf3-4691-b996-7faed99b7877.xiyuan.zr@alibaba-inc.com

9 months agoRemove dead generators for cyrillic encoding conversion tables
Heikki Linnakangas [Mon, 29 Jul 2024 17:38:19 +0000 (20:38 +0300)]
Remove dead generators for cyrillic encoding conversion tables

These tools were used to read the koi-iso.tab, koi-win.tab, and
koi-alt.tab files, which contained the mappings between the
single-byte cyrillic encodings. However, those data files were removed
in commit 4c3c8c048d, back in 2003. These code generators have been
unused and unusable ever since.

The generated tables live in cyrillic_and_mic.c. There has been one
change to the tables since they were generated in 1999, in commit
f4b7624eb07a. So if we resurrected the original data tables, that
change would need to be taken into account.

So this code is very dead. The tables in cyrillic_and_mic.c, which
were originally generated by these tools, are now the authoritative
source for these mappings.

Reviewed-by: Tom Lane, Aleksander Alekseev
Discussion: https://www.postgresql.org/message-id/flat/a821c3dc-36ec-4cee-8b41-7ccaa17adb18@iki.fi

9 months agoRemove tab completion for CREATE UNLOGGED MATERIALIZED VIEW.
Nathan Bossart [Mon, 29 Jul 2024 16:34:12 +0000 (11:34 -0500)]
Remove tab completion for CREATE UNLOGGED MATERIALIZED VIEW.

Commit 3bf3ab8c56 added support for unlogged materialized views,
but commit 3223b25ff7 reverted that feature before it made it into
a release.  However, the latter commit left the grammar and
tab-completion support intact.  This commit removes the
tab-completion support to prevent psql from recommending bogus
commands.  I've opted to keep the grammar support so that the
server continues to emit a descriptive error when users try to
create unlogged matviews.

Reported-by: Daniel Westermann, px shi
Author: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/ZR0P278MB092093E92263DE16734208A5D2C59%40ZR0P278MB0920.CHEP278.PROD.OUTLOOK.COM
Discussion: https://postgr.es/m/CAAccyY%2BWg1Z-9tNfSwLmuZVgGOwqU5u1OP-RWcoAr2UZGuvN_w%40mail.gmail.com

9 months agoCount individual SQL commands in pg_restore's --transaction-size mode.
Tom Lane [Mon, 29 Jul 2024 16:17:24 +0000 (12:17 -0400)]
Count individual SQL commands in pg_restore's --transaction-size mode.

The initial implementation in commit 959b38d77 counted one action
per TOC entry (except for some special cases for multi-blob BLOBS
entries).  This assumes that TOC entries are all about equally
complex, but it turns out that that assumption doesn't hold up very
well in binary-upgrade mode.  For example, even after the previous
commit I was able to cause backend bloat with tables having many
inherited constraints.  There may be other cases too.  (Since no
serious problems have been reported with --single-transaction mode,
we can conclude that the backend copes well with psql's regular
restore scripts; but before 959b38d77 we never ran binary-upgrade
restores with multi-command transactions.)

To fix, count multi-command TOC entries as N actions, allowing the
transaction size to be scaled down when we hit a complex TOC entry.
Rather than add a SQL parser to pg_restore, approximate "multi
command" by counting semicolons in the TOC entry's defn string.
This will be fooled by semicolons appearing in string literals ---
but the error is in the conservative direction, so it doesn't seem
worth working harder.  The biggest risk is with function/procedure
TOC entries, but we can just explicitly skip those.

(This is undoubtedly a hack, and maybe someday we'll be able to
revert it after fixing the backend's bloat issues or rethinking
what pg_dump emits in binary upgrade mode.  But that surely isn't
a project for v17.)

Thanks to Alexander Korotkov for the let's-count-semicolons idea.

Per report from Justin Pryzby.  Back-patch to v17 where txn_size mode
was introduced.

Discussion: https://postgr.es/m/ZqEND4ZcTDBmcv31@pryzbyj2023

9 months agoReduce number of commands dumpTableSchema emits for binary upgrade.
Tom Lane [Mon, 29 Jul 2024 15:53:49 +0000 (11:53 -0400)]
Reduce number of commands dumpTableSchema emits for binary upgrade.

Avoid issuing a separate SQL UPDATE command for each column when
directly manipulating pg_attribute contents in binary upgrade mode.
With the separate updates, we triggered a relcache invalidation with
each update.  For a table with N columns, that causes O(N^2) relcache
bloat in txn_size mode because the table's newly-created relcache
entry can't be flushed till end of transaction.  Reducing the number
of commands should make it marginally faster as well as avoiding that
problem.

While at it, likewise avoid issuing a separate UPDATE on pg_constraint
for each inherited constraint.  This is less exciting, first because
inherited (non-partitioned) constraints are relatively rare, and
second because the backend has a good deal of trouble anyway with
restoring tables containing many such constraints, due to
MergeConstraintsIntoExisting being horribly inefficient.  But it seems
more consistent to do it this way here too, and it surely can't hurt.

In passing, fix one place in dumpTableSchema that failed to use ONLY
in ALTER TABLE.  That's not a live bug, but it's inconsistent.
Also avoid silently casting away const from string literals.

Per report from Justin Pryzby.  Back-patch to v17 where txn_size mode
was introduced.

Discussion: https://postgr.es/m/ZqEND4ZcTDBmcv31@pryzbyj2023

9 months agoFix double-release of spinlock
Heikki Linnakangas [Mon, 29 Jul 2024 15:17:33 +0000 (18:17 +0300)]
Fix double-release of spinlock

Commit 9d9b9d46f3 added spinlocks to protect the fields in ProcSignal
flags, but in EmitProcSignalBarrier(), the spinlock was released
twice. With most spinlock implementations, releasing a lock that's not
held is not easy to notice, because most of the time it does nothing,
but if the spinlock was concurrently acquired by another process, it
could lead to more serious issues. Fortunately, with the
--disable-spinlocks emulation implementation, it caused more visible
failures.

In the passing, fix a type in comment and add an assertion that the
procNumber passed to SendProcSignal looks valid.

Discussion: https://www.postgresql.org/message-id/b8ce284c-18a2-4a79-afd3-1991a2e7d246@iki.fi

9 months agoFix compiler warning/error about typedef redefinitions
Heikki Linnakangas [Mon, 29 Jul 2024 13:23:30 +0000 (16:23 +0300)]
Fix compiler warning/error about typedef redefinitions

Per buildfarm member 'sifaka':

    procsignal.c:87:3: error: redefinition of typedef 'ProcSignalHeader' is a C11 feature [-Werror,-Wtypedef-redefinition]

9 months agoMove cancel key generation to after forking the backend
Heikki Linnakangas [Mon, 29 Jul 2024 12:37:48 +0000 (15:37 +0300)]
Move cancel key generation to after forking the backend

Move responsibility of generating the cancel key to the backend
process. The cancel key is now generated after forking, and the
backend advertises it in the ProcSignal array. When a cancel request
arrives, the backend handling it scans the ProcSignal array to find
the target pid and cancel key. This is similar to how this previously
worked in the EXEC_BACKEND case with the ShmemBackendArray, just
reusing the ProcSignal array.

One notable change is that we no longer generate cancellation keys for
non-backend processes. We generated them before just to prevent a
malicious user from canceling them; the keys for non-backend processes
were never actually given to anyone. There is now an explicit flag
indicating whether a process has a valid key or not.

I wrote this originally in preparation for supporting longer cancel
keys, but it's a nice cleanup on its own.

Reviewed-by: Jelte Fennema-Nio
Discussion: https://www.postgresql.org/message-id/508d0505-8b7a-4864-a681-e7e5edfe32aa@iki.fi

9 months agoFix outdated comment in smgrtruncate()
Heikki Linnakangas [Mon, 29 Jul 2024 11:23:23 +0000 (14:23 +0300)]
Fix outdated comment in smgrtruncate()

Commit c5315f4f44 replaced smgr_fsm_nblocks and smgr_vm_nblocks with
smgr_cached_nblocks, but forgot to update this comment.

Author: Kirill Reshke
Discussion: https://www.postgresql.org/message-id/CALdSSPh9VA6SDSVjrcmSPEYramf%2BrFisK7GqJo1dtRnD3vddmA@mail.gmail.com

9 months agoReduce memory used by partitionwise joins
Richard Guo [Mon, 29 Jul 2024 02:35:51 +0000 (11:35 +0900)]
Reduce memory used by partitionwise joins

In try_partitionwise_join, we aim to break down the join between two
partitioned relations into joins between matching partitions.  To
achieve this, we iterate through each pair of partitions from the two
joining relations and create child-join relations for them.  With
potentially thousands of partitions, the local objects allocated in
each iteration can accumulate significant memory usage.  Therefore, we
opt to eagerly free these local objects at the end of each iteration.

In line with this approach, this patch frees the bitmap set that
represents the relids of child-join relations at the end of each
iteration.  Additionally, it modifies build_child_join_rel() to reuse
the AppendRelInfo structures generated within each iteration.

Author: Ashutosh Bapat
Reviewed-by: David Christensen, Richard Guo
Discussion: https://postgr.es/m/CAExHW5s4EqY43oB=ne6B2=-xLgrs9ZGeTr1NXwkGFt2j-OmaQQ@mail.gmail.com

9 months agoSimplify create_merge_append_path for clarity
Richard Guo [Mon, 29 Jul 2024 02:33:18 +0000 (11:33 +0900)]
Simplify create_merge_append_path for clarity

We don't currently support parameterized MergeAppend paths: there's
little use for an ordered path on the inside of a nestloop.  Given
this, we can simplify create_merge_append_path by directly setting
param_info to NULL instead of calling get_appendrel_parampathinfo.  We
can also simplify the Assert for child paths a little bit.

This change won't make any measurable difference in performance; it's
just for clarity's sake.

Author: Richard Guo
Reviewed-by: Alena Rybakina, Paul A Jungwirth
Discussion: https://postgr.es/m/CAMbWs4_n1bgH2nACMuGsXZct3KH6PBFS0tPdQsXdstRfyxTunQ@mail.gmail.com

9 months agoRefactor pg_set_regex_collation() for clarity.
Jeff Davis [Sun, 28 Jul 2024 23:55:17 +0000 (16:55 -0700)]
Refactor pg_set_regex_collation() for clarity.

Discussion: https://postgr.es/m/63409030-2746-462e-beac-759bd43032ce@proxel.se
Reviewed-by: Andreas Karlsson
9 months agoAdd missing pointer dereference in pg_backend_memory_contexts view
David Rowley [Sun, 28 Jul 2024 21:53:10 +0000 (09:53 +1200)]
Add missing pointer dereference in pg_backend_memory_contexts view

32d3ed816 moved the logic for setting the context's name and ident into
a reusable function.  I missed adding a pointer dereference after
copying and pasting the code into that function.  The ident parameter is
a pointer to the ident variable in the calling function, so the
dereference is required to correctly determine if the contents of that
variable is NULL or not.

In passing, adjust the if condition to include an == NULL to make it
more clear that it's not checking for == '\0'.

Reported-by: Tom Lane, Coverity
Discussion: https://postgr.es/m/2256588.1722184287@sss.pgh.pa.us

9 months agoFix whitespace in commit 005c6b833f.
Jeff Davis [Sun, 28 Jul 2024 20:34:52 +0000 (13:34 -0700)]
Fix whitespace in commit 005c6b833f.

9 months agoRefactor: make default_locale internal to pg_locale.c.
Jeff Davis [Sun, 28 Jul 2024 20:07:25 +0000 (13:07 -0700)]
Refactor: make default_locale internal to pg_locale.c.

Discussion: https://postgr.es/m/2228884bb1f1a02614b39f71a90c94d2cc8a3a2f.camel@j-davis.com
Reviewed-by: Peter Eisentraut, Andreas Karlsson
9 months agoChange collation cache to use simplehash.h.
Jeff Davis [Sun, 28 Jul 2024 19:39:57 +0000 (12:39 -0700)]
Change collation cache to use simplehash.h.

Speeds up text comparison expressions when using a collation other
than the database default collation. Does not affect larger operations
such as ORDER BY, because the lookup is only done once.

Discussion: https://postgr.es/m/7bb9f018d20a7b30b9a7f6231efab1b5e50c7720.camel@j-davis.com
Reviewed-by: John Naylor, Andreas Karlsson
9 months agoamcheck: Optimize speed of checking for unique constraint violation
Alexander Korotkov [Sun, 28 Jul 2024 10:50:57 +0000 (13:50 +0300)]
amcheck: Optimize speed of checking for unique constraint violation

Currently, when amcheck validates a unique constraint, it visits the heap for
each index tuple.  This commit implements skipping keys, which have only one
non-dedeuplicated index tuple (quite common case for unique indexes). That
gives substantial economy on index checking time.

Reported-by: Noah Misch
Discussion: https://postgr.es/m/20240325020323.fd.nmisch%40google.com
Author: Alexander Korotkov, Pavel Borisov

9 months agoFix incorrect return value for pg_size_pretty(bigint)
David Rowley [Sun, 28 Jul 2024 10:22:52 +0000 (22:22 +1200)]
Fix incorrect return value for pg_size_pretty(bigint)

pg_size_pretty(bigint) would return the value in bytes rather than PB
for the smallest-most bigint value.  This happened due to an incorrect
assumption that the absolute value of -9223372036854775808 could be
stored inside a signed 64-bit type.

Here we fix that by instead storing that value in an unsigned 64-bit type.

This bug does exist in versions prior to 15 but the code there is
sufficiently different and the bug seems sufficiently non-critical that
it does not seem worth risking backpatching further.

Author: Joseph Koshakow <koshy44@gmail.com>
Discussion: https://postgr.es/m/CAAvxfHdTsMZPWEHUrZ=h3cky9Ccc3Mtx2whUHygY+ABP-mCmUw@mail.gmail.com
Backpatch-through: 15

9 months agolibpq: Use strerror_r instead of strerror
Peter Eisentraut [Sun, 28 Jul 2024 07:12:00 +0000 (09:12 +0200)]
libpq: Use strerror_r instead of strerror

Commit 453c4687377 introduced a use of strerror() into libpq, but that
is not thread-safe.  Fix by using strerror_r() instead.

In passing, update some of the code comments added by 453c4687377, as
we have learned more about the reason for the change in OpenSSL that
started this.

Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: Discussion: https://postgr.es/m/b6fb018b-f05c-4afd-abd3-318c649faf18@highgo.ca

9 months agoDoc: fix text's description of regexp_replace's arguments.
Tom Lane [Sat, 27 Jul 2024 19:38:54 +0000 (15:38 -0400)]
Doc: fix text's description of regexp_replace's arguments.

Section 9.7.3 had a syntax synopsis for regexp_replace()
that was different from Table 9.10's, but still wrong.
Update that one too.  Oversight in 580f8727c.

Jian He

Discussion: https://postgr.es/m/CACJufxG3NFKKsh6x4fRLv8h3V-HvN4W5dA=zNKMxsNcDwOKang@mail.gmail.com

9 months agoOptimize escaping of JSON strings
David Rowley [Sat, 27 Jul 2024 11:46:07 +0000 (23:46 +1200)]
Optimize escaping of JSON strings

There were quite a few places where we either had a non-NUL-terminated
string or a text Datum which we needed to call escape_json() on.  Many of
these places required that a temporary string was created due to the fact
that escape_json() needs a NUL-terminated cstring.  For text types, those
first had to be converted to cstring before calling escape_json() on them.

Here we introduce two new functions to make escaping JSON more optimal:

escape_json_text() can be given a text Datum to append onto the given
buffer.  This is more optimal as it foregoes the need to convert the text
Datum into a cstring.  A temporary allocation is only required if the text
Datum needs to be detoasted.

escape_json_with_len() can be used when the length of the cstring is
already known or the given string isn't NUL-terminated.  Having this
allows various places which were creating a temporary NUL-terminated
string to just call escape_json_with_len() without any temporary memory
allocations.

Discussion: https://postgr.es/m/CAApHDvpLXwMZvbCKcdGfU9XQjGCDm7tFpRdTXuB9PVgpNUYfEQ@mail.gmail.com
Reviewed-by: Melih Mutlu, Heikki Linnakangas
9 months agoSupport falling back to non-preferred readline implementation with meson
Heikki Linnakangas [Sat, 27 Jul 2024 10:53:16 +0000 (13:53 +0300)]
Support falling back to non-preferred readline implementation with meson

To build with -Dreadline=enabled one can use either readline or
libedit. The -Dlibedit_preferred flag is supposed to control the order
of names to lookup.  This works fine when either both libraries are
present or -Dreadline is set to auto. However, explicitly enabling
readline with only libedit present, but not setting libedit_preferred,
or alternatively enabling readline with only readline present, but
setting libedit_preferred, too, are both broken. This is because
cc.find_library will throw an error for a not found dependency as soon
as the first required dependency is checked, thus it's impossible to
fallback to the alternative.

Here we only check the second of the two dependencies for
requiredness, thus we only fail when none of the two can be found.

Author: Wolfgang Walther
Reviewed-by: Nazir Bilal Yavuz, Alvaro Herrera, Peter Eisentraut
Reviewed-by: Tristan Partin
Discussion: https://www.postgresql.org/message-id/ca8f37e1-a2c3-40e2-91f6-59c3d3652ad4@technowledgy.de
Backpatch: 16-, where meson support was added

9 months agoSupport absolute bindir/libdir in regression tests with meson
Heikki Linnakangas [Sat, 27 Jul 2024 10:53:14 +0000 (13:53 +0300)]
Support absolute bindir/libdir in regression tests with meson

Passing an absolute bindir/libdir will install the binaries and
libraries to <build>/tmp_install/<bindir> and
<build>/tmp_install/<libdir> respectively.

This path is correctly passed to the regression test suite via
configure/make, but not via meson, yet. This is because the "/"
operator in the following expression throws away the whole left side
when the right side is an absolute path:

    test_install_location / get_option('libdir')

This was already correctly handled for dir_prefix, which is likely
absolute as well. This patch handles both bindir and libdir in the
same way - prefixing absolute paths with the tmp_install path
correctly.

Author: Wolfgang Walther
Reviewed-by: Nazir Bilal Yavuz, Alvaro Herrera, Peter Eisentraut
Reviewed-by: Tristan Partin
Discussion: https://www.postgresql.org/message-id/ca8f37e1-a2c3-40e2-91f6-59c3d3652ad4@technowledgy.de
Backpatch: 16-, where meson support was added

9 months agoFallback to clang in PATH with meson
Heikki Linnakangas [Sat, 27 Jul 2024 10:53:11 +0000 (13:53 +0300)]
Fallback to clang in PATH with meson

Some distributions put clang into a different path than the llvm
binary path.

For example, this is the case on NixOS / nixpkgs, which failed to find
clang with meson before this patch.

Author: Wolfgang Walther
Reviewed-by: Nazir Bilal Yavuz, Alvaro Herrera, Peter Eisentraut
Reviewed-by: Tristan Partin
Discussion: https://www.postgresql.org/message-id/ca8f37e1-a2c3-40e2-91f6-59c3d3652ad4@technowledgy.de
Backpatch: 16-, where meson support was added

9 months agoFallback to uuid for ossp-uuid with meson
Heikki Linnakangas [Sat, 27 Jul 2024 10:53:08 +0000 (13:53 +0300)]
Fallback to uuid for ossp-uuid with meson

The upstream name for the ossp-uuid package / pkg-config file is
"uuid". Many distributions change this to be "ossp-uuid" to not
conflict with e2fsprogs.

This lookup fails on distributions which don't change this name, for
example NixOS / nixpkgs. Both "ossp-uuid" and "uuid" are also checked
in configure.ac.

Author: Wolfgang Walther
Reviewed-by: Nazir Bilal Yavuz, Alvaro Herrera, Peter Eisentraut
Reviewed-by: Tristan Partin
Discussion: https://www.postgresql.org/message-id/ca8f37e1-a2c3-40e2-91f6-59c3d3652ad4@technowledgy.de
Backpatch: 16-, where meson support was added

9 months agoFix more holes with SLRU code in need of int64 for segment numbers
Michael Paquier [Fri, 26 Jul 2024 22:16:52 +0000 (07:16 +0900)]
Fix more holes with SLRU code in need of int64 for segment numbers

This is a continuation of 3937cadfd438, taking care of more areas I have
managed to miss previously.

Reported-by: Noah Misch
Reviewed-by: Noah Misch
Discussion: https://postgr.es/m/20240724130059.1f.nmisch@google.com
Backpatch-through: 17

9 months agoIntroduce num_os_semaphores GUC.
Nathan Bossart [Fri, 26 Jul 2024 20:28:55 +0000 (15:28 -0500)]
Introduce num_os_semaphores GUC.

The documentation for System V IPC parameters provides complicated
formulas to determine the appropriate values for SEMMNI and SEMMNS.
Furthermore, these formulas have often been wrong because folks
forget to update them (e.g., when adding a new auxiliary process).

This commit introduces a new runtime-computed GUC named
num_os_semaphores that reports the number of semaphores needed for
the configured number of allowed connections, worker processes,
etc.  This new GUC allows us to simplify the formulas in the
documentation, and it should help prevent future inaccuracies.
Like the other runtime-computed GUCs, users can view it with
"postgres -C" before starting the server, which is useful for
preconfiguring the necessary operating system resources.

Reviewed-by: Tom Lane, Sami Imseih, Andres Freund, Robert Haas
Discussion: https://postgr.es/m/20240517164452.GA1914161%40nathanxps13

9 months agoWait for WAL summarization to catch up before creating .partial file.
Robert Haas [Fri, 26 Jul 2024 18:50:21 +0000 (14:50 -0400)]
Wait for WAL summarization to catch up before creating .partial file.

When a standby is promoted, CleanupAfterArchiveRecovery() may decide
to rename the final WAL file from the old timeline by adding ".partial"
to the name. If WAL summarization is enabled and this file is renamed
before its partial contents are summarized, WAL summarization breaks:
the summarizer gets stuck at that point in the WAL stream and just
errors out.

To fix that, first make the startup process wait for WAL summarization
to catch up before renaming the file. Generally, this should be quick,
and if it's not, the user can shut off summarize_wal and try again.
To make this fix work, also teach the WAL summarizer that after a
promotion has occurred, no more WAL can appear on the previous
timeline: previously, the WAL summarizer wouldn't switch to the new
timeline until we actually started writing WAL there, but that meant
that when the startup process was waiting for the WAL summarizer, it
was waiting for an action that the summarizer wasn't yet prepared to
take.

In the process of fixing these bugs, I realized that the logic to wait
for WAL summarization to catch up was spread out in a way that made
it difficult to reuse properly, so this code refactors things to make
it easier.

Finally, add a test case that would have caught this bug and the
previously-fixed bug that WAL summarization sometimes needs to back up
when the timeline changes.

Discussion: https://postgr.es/m/CA+TgmoZGEsZodXC4f=XZNkAeyuDmWTSkpkjCEOcF19Am0mt_OA@mail.gmail.com

9 months agopostgres_fdw: Fix bug in connection status check.
Fujii Masao [Fri, 26 Jul 2024 18:05:47 +0000 (03:05 +0900)]
postgres_fdw: Fix bug in connection status check.

The buildfarm member "hake" reported a failure in the regression test
added by commit 857df3cef7, where postgres_fdw_get_connections(true)
returned unexpected results.

The function postgres_fdw_get_connections(true) checks
if a connection is closed by using POLLRDHUP in the requested events
and calling poll(). Previously, the function only considered
POLLRDHUP or 0 as valid returned events. However, poll() can also
return POLLHUP, POLLERR, and/or POLLNVAL. So if any of these events
were returned, postgres_fdw_get_connections(true) would report
incorrect results. postgres_fdw_get_connections(true) failed to
account for these return events.

This commit updates postgres_fdw_get_connections(true) to correctly
report a closed connection when poll() returns not only POLLRDHUP
but also POLLHUP, POLLERR, or POLLNVAL.

Discussion: https://postgr.es/m/fd8f6186-9e1e-4b9a-92c5-e71e3697d381@oss.nttdata.com

9 months agopg_upgrade: Move live_check variable to user_opts.
Nathan Bossart [Fri, 26 Jul 2024 18:37:32 +0000 (13:37 -0500)]
pg_upgrade: Move live_check variable to user_opts.

At the moment, pg_upgrade stores whether it is doing a "live check"
(i.e., the user specified --check and the old server is still
running) in a local variable scoped to main().  This live_check
variable is passed to several functions.  To further complicate
matters, a few call sites provide a hard-coded "false" as the
live_check argument.  Specifically, this is done when calling these
functions for the new cluster, for which any live-check-only paths
won't apply.

This commit moves the live_check variable to the global user_opts
variable, which stores information about the options the user
specified on the command line.  This allows us to remove the
live_check parameter from several functions.  For the functions
with callers that provide a hard-coded "false" as the live_check
argument (e.g., get_control_data()), we verify the given cluster is
the old cluster before taking any live-check-only paths.

This small refactoring effort helps simplify some proposed changes
that would parallelize many of pg_upgrade's once-in-each-database
tasks using libpq's asynchronous APIs.  By removing the live_check
parameter, we can more easily convert the functions to callbacks
for the new parallel system.

Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/20240516211638.GA1688936%40nathanxps13

9 months agoClarify error message and documentation related to typed tables.
Tom Lane [Fri, 26 Jul 2024 16:39:45 +0000 (12:39 -0400)]
Clarify error message and documentation related to typed tables.

We restrict typed tables (those declared as "OF composite_type")
to be based on stand-alone composite types, not composite types
that are the implicitly-created rowtypes of other tables.
But if you tried to do that, you got the very confusing error
message "type foo is not a composite type".  Provide a more specific
message for that case.  Also clarify related documentation in the
CREATE TABLE man page.

Erik Wienhold and David G. Johnston, per complaint from Hannu Krosing.

Discussion: https://postgr.es/m/CAMT0RQRysCb_Amy5CTENSc5GfsvXL1a4qX3mv_hx31_v74P==g@mail.gmail.com

9 months agoFix indentation.
Robert Haas [Fri, 26 Jul 2024 15:59:34 +0000 (11:59 -0400)]
Fix indentation.

9 months agoFix macro placement in pg_config.h.in
Daniel Gustafsson [Fri, 26 Jul 2024 14:25:28 +0000 (16:25 +0200)]
Fix macro placement in pg_config.h.in

Commit 274bbced85383e831dde accidentally placed the pg_config.h.in
for SSL_CTX_set_num_tickets on the wrong line wrt where autoheader
places it.  Fix by re-arranging and backpatch to the same level as
the original commit.

Reported-by: Marina Polyakova <m.polyakova@postgrespro.ru>
Discussion: https://postgr.es/m/48cebe8c3eaf308bae253b1dbf4e4a75@postgrespro.ru
Backpatch-through: v12

9 months agoAllow WAL summarization to back up when timeline changes.
Robert Haas [Fri, 26 Jul 2024 13:50:31 +0000 (09:50 -0400)]
Allow WAL summarization to back up when timeline changes.

The old code believed that it was not possible to switch timelines
without first replaying all of the WAL from the old timeline, but
that turns out to be false, as demonstrated by an example from Fujii
Masao. As a result, it assumed that summarization would always
continue from the LSN where summarization previously ended. But in
fact, when a timeline switch occurs without replaying all the WAL
from the previous timeline, we can need to back up to an earlier
LSN. Adjust accordingly.

Discussion: https://postgr.es/m/CA+TgmoZGEsZodXC4f=XZNkAeyuDmWTSkpkjCEOcF19Am0mt_OA@mail.gmail.com

9 months agopostgres_fdw: Add connection status check to postgres_fdw_get_connections().
Fujii Masao [Fri, 26 Jul 2024 13:16:39 +0000 (22:16 +0900)]
postgres_fdw: Add connection status check to postgres_fdw_get_connections().

This commit extends the postgres_fdw_get_connections() function
to check if connections are closed. This is useful for detecting closed
postgres_fdw connections that could prevent successful transaction
commits. Users can roll back transactions immediately upon detecting
closed connections, avoiding unnecessary processing of failed
transactions.

This feature is available only on systems supporting the non-standard
POLLRDHUP extension to the poll system call, including Linux.

Author: Hayato Kuroda
Reviewed-by: Shinya Kato, Zhihong Yu, Kyotaro Horiguchi, Andres Freund
Reviewed-by: Onder Kalaci, Takamichi Osumi, Vignesh C, Tom Lane, Ted Yu
Reviewed-by: Katsuragi Yuta, Peter Smith, Shubham Khanna, Fujii Masao
Discussion: https://postgr.es/m/TYAPR01MB58662809E678253B90E82CE5F5889@TYAPR01MB5866.jpnprd01.prod.outlook.com

9 months agopostgres_fdw: Add "used_in_xact" column to postgres_fdw_get_connections().
Fujii Masao [Fri, 26 Jul 2024 13:15:51 +0000 (22:15 +0900)]
postgres_fdw: Add "used_in_xact" column to postgres_fdw_get_connections().

This commit extends the postgres_fdw_get_connections() function to
include a new used_in_xact column, indicating whether each connection
is used in the current transaction.

This addition is particularly useful for the upcoming feature that
will check if connections are closed. By using those information,
users can verify if postgres_fdw connections used in a transaction
remain open. If any connection is closed, the transaction cannot
be committed successfully. In this case users can roll back it
immediately without waiting for transaction end.

The SQL API for postgres_fdw_get_connections() is updated by
this commit and may change in the future. To handle compatibility
with older SQL declarations, an API versioning system is introduced,
allowing the function to behave differently based on the API version.

Author: Hayato Kuroda
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/be9382f7-5072-4760-8b3f-31d6dffa8d62@oss.nttdata.com

9 months agopg_createsubscriber: Message style improvements
Peter Eisentraut [Fri, 26 Jul 2024 12:45:13 +0000 (14:45 +0200)]
pg_createsubscriber: Message style improvements

Refactor some messages, improve quoting.

9 months agoAdd tests for errors during SSL or GSSAPI handshake
Heikki Linnakangas [Fri, 26 Jul 2024 12:12:23 +0000 (15:12 +0300)]
Add tests for errors during SSL or GSSAPI handshake

These test that libpq correctly falls back to a plaintext connection
on handshake error, in the "prefer" modes.

Reviewed-by: Michael Paquier
Discussion: https://www.postgresql.org/message-id/CAOYmi%2Bnwvu21mJ4DYKUa98HdfM_KZJi7B1MhyXtnsyOO-PB6Ww%40mail.gmail.com

9 months agoAdd test for early backend startup errors
Heikki Linnakangas [Fri, 26 Jul 2024 12:12:21 +0000 (15:12 +0300)]
Add test for early backend startup errors

The new test tests the libpq fallback behavior on an early error,
which was fixed in the previous commit.

This adds an IS_INJECTION_POINT_ATTACHED() macro, to allow writing
injected test code alongside the normal source code. In principle, the
new test could've been implemented by an extra test module with a
callback that sets the FrontendProtocol global variable, but I think
it's more clear to have the test code right where the injection point
is, because it has pretty intimate knowledge of the surrounding
context it runs in.

Reviewed-by: Michael Paquier
Discussion: https://www.postgresql.org/message-id/CAOYmi%2Bnwvu21mJ4DYKUa98HdfM_KZJi7B1MhyXtnsyOO-PB6Ww%40mail.gmail.com

9 months agoFix using injection points at backend startup in EXEC_BACKEND mode
Heikki Linnakangas [Fri, 26 Jul 2024 11:55:04 +0000 (14:55 +0300)]
Fix using injection points at backend startup in EXEC_BACKEND mode

Commit 86db52a506 changed the locking of injection points to use only
atomic ops and spinlocks, to make it possible to define injection
points in processes that don't have a PGPROC entry (yet). However, it
didn't work in EXEC_BACKEND mode, because the pointer to shared memory
area was not initialized until the process "attaches" to all the
shared memory structs. To fix, pass the pointer to the child process
along with other global variables that need to be set up early.

Backpatch-through: 17

9 months agoFix fallback behavior when server sends an ERROR early at startup
Heikki Linnakangas [Fri, 26 Jul 2024 11:52:08 +0000 (14:52 +0300)]
Fix fallback behavior when server sends an ERROR early at startup

With sslmode=prefer, the desired behavior is to completely fail the
connection attempt, *not* fall back to a plaintext connection, if the
server responds to the SSLRequest with an error ('E') response instead
of rejecting SSL with an 'N' response. This was broken in commit
05fd30c0e7.

Reported-by: Jacob Champion
Reviewed-by: Michael Paquier
Discussion: https://www.postgresql.org/message-id/CAOYmi%2Bnwvu21mJ4DYKUa98HdfM_KZJi7B1MhyXtnsyOO-PB6Ww%40mail.gmail.com
Backpatch-through: 17

9 months agodoc: Enhance documentation for postgres_fdw_get_connections() output columns.
Fujii Masao [Fri, 26 Jul 2024 11:47:05 +0000 (20:47 +0900)]
doc: Enhance documentation for postgres_fdw_get_connections() output columns.

The documentation previously described the output columns of
postgres_fdw_get_connections() in text format, which was manageable
for the original two columns. However, upcoming patches will add
new columns, making text descriptions less readable.

This commit updates the documentation to use a table format,
making it easier for users to understand each output column.

Author: Fujii Masao, Hayato Kuroda
Reviewed-by: Hayato Kuroda
Discussion: https://postgr.es/m/d04aae8d-05f5-42f4-a263-b962334d9f75@oss.nttdata.com

9 months agoDisable all TLS session tickets
Daniel Gustafsson [Fri, 26 Jul 2024 09:09:45 +0000 (11:09 +0200)]
Disable all TLS session tickets

OpenSSL supports two types of session tickets for TLSv1.3, stateless
and stateful. The option we've used only turns off stateless tickets
leaving stateful tickets active. Use the new API introduced in 1.1.1
to disable all types of tickets.

Backpatch to all supported versions.

Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reported-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20240617173803.6alnafnxpiqvlh3g@awork3.anarazel.de
Backpatch-through: v12

9 months agoSQL/JSON: Remove useless code in ExecInitJsonExpr()
Amit Langote [Fri, 26 Jul 2024 07:38:46 +0000 (16:38 +0900)]
SQL/JSON: Remove useless code in ExecInitJsonExpr()

The code was for adding an unconditional JUMP to the next step,
which is unnecessary processing.

Reported-by: Jian He <jian.universality@gmail.com>
Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Backpatch-through: 17

9 months agoSQL/JSON: Respect OMIT QUOTES when RETURNING domains over jsonb
Amit Langote [Fri, 26 Jul 2024 07:08:13 +0000 (16:08 +0900)]
SQL/JSON: Respect OMIT QUOTES when RETURNING domains over jsonb

populate_domain() didn't take into account the omit_quotes flag passed
down to json_populate_type() by ExecEvalJsonCoercion() and that led
to incorrect behavior when the RETURNING type is a domain over
jsonb.  Fix that by passing the flag by adding a new function
parameter to populate_domain().

Reported-by: Jian He <jian.universality@gmail.com>
Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Backpatch-through: 17

9 months agoSQL/JSON: Improve error-handling of JsonBehavior expressions
Amit Langote [Fri, 26 Jul 2024 07:00:16 +0000 (16:00 +0900)]
SQL/JSON: Improve error-handling of JsonBehavior expressions

Instead of returning a NULL when the JsonBehavior expression value
could not be coerced to the RETURNING type, throw the error message
informing the user that it is the JsonBehavior expression that caused
the error with the actual coercion error message shown in its DETAIL
line.

Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Backpatch-through: 17

9 months agoSQL/JSON: Fix error-handling of some JsonBehavior expressions
Amit Langote [Fri, 26 Jul 2024 06:59:27 +0000 (15:59 +0900)]
SQL/JSON: Fix error-handling of some JsonBehavior expressions

To ensure that the errors of executing a JsonBehavior expression that
is coerced in the parser are caught instead of being thrown directly,
pass ErrorSaveContext to ExecInitExprRec() when initializing it.
Also, add a EEOP_JSONEXPR_COERCION_FINISH step to handle the errors
that are caught that way.

Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Backpatch-through: 17

9 months agoDoc: fix misleading syntax synopses for targetlists.
Tom Lane [Thu, 25 Jul 2024 23:52:08 +0000 (19:52 -0400)]
Doc: fix misleading syntax synopses for targetlists.

In the syntax synopses for SELECT, INSERT, UPDATE, etc,
SELECT ... and RETURNING ... targetlists were missing { ... }
braces around an OR (|) operator.  That allows misinterpretation
which could lead to confusion.

David G. Johnston, per gripe from masondeanm@aol.com.

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

9 months agoDoc: update some HTTP links to point to canonical URLs.
Tom Lane [Thu, 25 Jul 2024 20:38:19 +0000 (16:38 -0400)]
Doc: update some HTTP links to point to canonical URLs.

These aren't actually broken at present, but we might as well
avoid redirects.

Joel Jacobson

Discussion: https://postgr.es/m/8ccc96c7-0515-491b-be98-cfacdaeda815@app.fastmail.com

9 months agoDocument restrictions regarding incremental backups and standbys.
Robert Haas [Thu, 25 Jul 2024 19:45:06 +0000 (15:45 -0400)]
Document restrictions regarding incremental backups and standbys.

If you try to take an incremental backup on a standby and there hasn't
been much system activity, it might fail. Document why this happens.
Also add a hint to the error message you get, to make it more likely
that users will understand what has gone wrong.

Laurenz Albe and Robert Haas

Discussion: https://postgr.es/m/5468641ad821dad7aa3b2d65bf843146443a1b68.camel@cybertec.at

9 months agoAdd argument names to the regexp_XXX functions.
Tom Lane [Thu, 25 Jul 2024 18:51:46 +0000 (14:51 -0400)]
Add argument names to the regexp_XXX functions.

This change allows these functions to be called using named-argument
notation, which can be helpful for readability, particularly for
the ones with many arguments.

There was considerable debate about exactly which names to use,
but in the end we settled on the names already shown in our
documentation table 9.10.

The citext extension provides citext-aware versions of some of
these functions, so add argument names to those too.

In passing, fix table 9.10's syntax synopses for regexp_match,
which were slightly wrong about which combinations of arguments
are allowed.

Jian He, reviewed by Dian Fay and others

Discussion: https://postgr.es/m/CACJufxG3NFKKsh6x4fRLv8h3V-HvN4W5dA=zNKMxsNcDwOKang@mail.gmail.com

9 months agopg_createsubscriber: Message improvements
Peter Eisentraut [Thu, 25 Jul 2024 13:25:42 +0000 (15:25 +0200)]
pg_createsubscriber: Message improvements

Objects are typically "in" a database, not "on".

9 months agopg_upgrade: Remove unused macro
Daniel Gustafsson [Thu, 25 Jul 2024 13:03:50 +0000 (15:03 +0200)]
pg_upgrade: Remove unused macro

Commit f06b1c598 removed validate_exec from pg_upgrade and instead
exported it from src/common, but the macro for checking executable
suffix on Windows was accidentally left.  Fix by removing.

Author: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/c1d63754-cb85-2d8a-8409-bde2c4d2d04b@gmail.com

9 months agopgcrypto: Remove unused binary from clean target
Daniel Gustafsson [Thu, 25 Jul 2024 12:27:01 +0000 (14:27 +0200)]
pgcrypto: Remove unused binary from clean target

Generation of the gen-rtab binary was removed in db7d1a7b0 but it
was accidentally left in the cleaning target.  Remove since it is
no longer built.

Author: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/c1d63754-cb85-2d8a-8409-bde2c4d2d04b@gmail.com

9 months agoRemove useless unconstify() call
Peter Eisentraut [Thu, 25 Jul 2024 09:38:05 +0000 (11:38 +0200)]
Remove useless unconstify() call

This should have been part of 67c0ef9752 but was apparently forgotten
there.

9 months agoFix -Wmissing-variable-declarations warnings for float.c special case
Peter Eisentraut [Wed, 24 Jul 2024 04:21:40 +0000 (06:21 +0200)]
Fix -Wmissing-variable-declarations warnings for float.c special case

This adds extern declarations for the global variables defined in
float.c but not meant for external use.  This is a workaround to be
able to add -Wmissing-variable-declarations to the global set of
warning options in the near future.

Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/flat/e0a62134-83da-4ba4-8cdb-ceb0111c95ce@eisentraut.org

9 months agoAdd extern declarations for Bison global variables
Peter Eisentraut [Thu, 25 Jul 2024 07:26:08 +0000 (09:26 +0200)]
Add extern declarations for Bison global variables

This adds extern declarations for some global variables produced by
Bison that are not already declared in its generated header file.
This is a workaround to be able to add -Wmissing-variable-declarations
to the global set of warning options in the near future.

Another longer-term solution would be to convert these grammars to
"pure" parsers in Bison, to avoid global variables altogether.  Note
that the core grammar is already pure, so this patch did not need to
touch it.

Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/flat/e0a62134-83da-4ba4-8cdb-ceb0111c95ce@eisentraut.org

9 months agoAdd path column to pg_backend_memory_contexts view
David Rowley [Thu, 25 Jul 2024 03:03:28 +0000 (15:03 +1200)]
Add path column to pg_backend_memory_contexts view

"path" provides a reliable method of determining the parent/child
relationships between memory contexts.  Previously this could be done in
a non-reliable way by writing a recursive query and joining the "parent"
and "name" columns.  This wasn't reliable as the names were not unique,
which could result in joining to the wrong parent.

To make this reliable, "path" stores an array of numerical identifiers
starting with the identifier for TopLevelMemoryContext.  It contains an
element for each intermediate parent between that and the current context.

Incompatibility: Here we also adjust the "level" column to make it
1-based rather than 0-based.  A 1-based level provides a convenient way
to access elements in the "path" array. e.g. path[level] gives the
identifier for the current context.

Identifiers are not stable across multiple evaluations of the view.  In
an attempt to make these more stable for ad-hoc queries, the identifiers
are assigned breadth-first.  Contexts closer to TopLevelMemoryContext
are less likely to change between queries and during queries.

Author: Melih Mutlu <m.melihmutlu@gmail.com>
Discussion: https://postgr.es/m/CAGPVpCThLyOsj3e_gYEvLoHkr5w=tadDiN_=z2OwsK3VJppeBA@mail.gmail.com
Reviewed-by: Andres Freund, Stephen Frost, Atsushi Torikoshi,
Reviewed-by: Michael Paquier, Robert Haas, David Rowley
9 months agoci: Pin MacPorts version to 2.9.3.
Thomas Munro [Thu, 25 Jul 2024 02:46:01 +0000 (14:46 +1200)]
ci: Pin MacPorts version to 2.9.3.

Commit d01ce180 invented a new way to find the latest MacPorts version.
By bad luck, a new beta release has just been published, and it seems
to lack some packages we need.  Go back to searching for this specific
version for now.  We still search with a pattern so that we can find the
package for the running version of macOS, but for now we always look for
2.9.3.  The code to do that had been anticipated already in a commented
out line, I just didn't expect to have to use it so soon...

Also include the whole MacPorts installation script in the cache key, so
that changes to the script cause a fresh installation.  This should make
it a bit easier to reason about the effect of changes on cached state in
github accounts using CI, when we make adjustments.

Back-patch to 15, like d01ce180.

Discussion: https://postgr.es/m/CA%2BhUKGLqJdv6RcwyZ_0H7khxtLTNJyuK%2BvDFzv3uwYbn8hKH6A%40mail.gmail.com

9 months agodoc: Decorate psql page with application markup tags
Michael Paquier [Thu, 25 Jul 2024 01:59:49 +0000 (10:59 +0900)]
doc: Decorate psql page with application markup tags

Noticed while looking at this area of the documentation for a separate
patch.

9 months agoci: Upgrade macOS version from 13 to 14.
Thomas Munro [Wed, 24 Jul 2024 23:26:48 +0000 (11:26 +1200)]
ci: Upgrade macOS version from 13 to 14.

1.  Previously we were using ghcr.io/cirruslabs/macos-XXX-base:latest
images, but Cirrus has started ignoring that and using a particular
image, currently ghcr.io/cirruslabs/macos-runner:sonoma, for github
accounts using free CI resources (as opposed to dedicated runner
machines, as cfbot uses).  Let's just ask for that image anyway, to stay
in sync.

2.  Instead of hard-coding a MacPorts installation URL, deduce it from
the running macOS version and the available releases.  This removes the
need to keep the ci_macports_packages.sh in sync with .cirrus.task.yml,
and to advance the MacPorts version from time to time.

3.  Change the cache key we use to cache the whole macports installation
across builds to include the OS major version, to trigger a fresh
installation when appropriate.

Back-patch to 15 where CI began.

Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA%2BhUKGLqJdv6RcwyZ_0H7khxtLTNJyuK%2BvDFzv3uwYbn8hKH6A%40mail.gmail.com

9 months agopg_upgrade: Retrieve subscription count more efficiently.
Nathan Bossart [Wed, 24 Jul 2024 16:30:33 +0000 (11:30 -0500)]
pg_upgrade: Retrieve subscription count more efficiently.

Presently, pg_upgrade obtains the number of subscriptions in the
to-be-upgraded cluster by first querying pg_subscription in every
database for the number of subscriptions in only that database.
Then, in count_old_cluster_subscriptions(), it adds all the values
collected in the first step.  This is expensive, especially when
there are many databases.

Fortunately, there is a better way to retrieve the subscription
count.  Since pg_subscription is a shared catalog, we only need to
connect to a single database and query it once.  This commit
modifies pg_upgrade to use that approach, which also allows us to
trim several lines of code.  In passing, move the call to
get_db_subscription_count(), which has been renamed to
get_subscription_count(), from get_db_rel_and_slot_infos() to the
dedicated >= v17 section in check_and_dump_old_cluster().

We may be able to make similar improvements to
get_old_cluster_logical_slot_infos(), but that is left as a future
exercise.

Reviewed-by: Michael Paquier, Amit Kapila
Discussion: https://postgr.es/m/ZprQJv_TxccN3tkr%40nathan
Backpatch-through: 17

9 months agoFix a missing article in the documentation
Alvaro Herrera [Wed, 24 Jul 2024 12:13:55 +0000 (14:13 +0200)]
Fix a missing article in the documentation

Per complaint from Grant Gryczan.

It's a very old typo; backpatch all the way back.

Author: Laurenz Albe <laurenz.albe@cybertec.at>
Discussion: https://postgr.es/m/172179789219.915368.16590585529628354757@wrigleys.postgresql.org

9 months agopg_stat_statements: Add regression test for privilege handling.
Fujii Masao [Wed, 24 Jul 2024 11:54:51 +0000 (20:54 +0900)]
pg_stat_statements: Add regression test for privilege handling.

This commit adds a regression test to verify that pg_stat_statements
correctly handles privileges, improving its test coverage.

Author: Keisuke Kuroda
Reviewed-by: Michael Paquier, Fujii Masao
Discussion: https://postgr.es/m/2224ccf2e12c41ccb81702ef3303d5ac@nttcom.co.jp

9 months agoReset relhassubclass upon attaching table as a partition
Alvaro Herrera [Wed, 24 Jul 2024 10:38:18 +0000 (12:38 +0200)]
Reset relhassubclass upon attaching table as a partition

We don't allow inheritance parents as partitions, and have checks to
prevent this; but if a table _was_ in the past an inheritance parents
and all their children are removed, the pg_class.relhassubclass flag
may remain set, which confuses the partition pruning code (most
obviously, it results in an assertion failure; in production builds it
may be worse.)

Fix by resetting relhassubclass on attach.

Backpatch to all supported versions.

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/18550-d5e047e9a897a889@postgresql.org

9 months agoDoc: Fix the mistakes in the subscription's failover option.
Amit Kapila [Wed, 24 Jul 2024 08:54:45 +0000 (14:24 +0530)]
Doc: Fix the mistakes in the subscription's failover option.

The documentation incorrectly stated that users could not alter the
subscription's failover option when the two-phase commit is enabled.

The steps to confirm that the standby server is ready for failover were
incorrect.

Author: Shveta Malik, Hou Zhijie
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/OS0PR01MB571657B72F8D75BD858DCCE394AD2@OS0PR01MB5716.jpnprd01.prod.outlook.com
Discussion: https://postgr.es/m/CAJpy0uBBk+OZXXqQ00Gai09XR+mDi2=9sMBYY0F+BedoFivaMA@mail.gmail.com

9 months agoRefactor tidstore.c iterator buffering.
Thomas Munro [Wed, 24 Jul 2024 05:24:59 +0000 (17:24 +1200)]
Refactor tidstore.c iterator buffering.

Previously, TidStoreIterateNext() would expand the set of offsets for
each block into an internal buffer that it overwrote each time.  In
order to be able to collect the offsets for multiple blocks before
working with them, change the contract.  Now, the offsets are obtained
by a separate call to TidStoreGetBlockOffsets(), which can be called at
a later time.  TidStoreIteratorResult objects are safe to copy and store
in a queue.

Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/CAAKRu_bbkmwAzSBgnezancgJeXrQZXy4G4kBTd+5=cr86H5yew@mail.gmail.com

9 months agoAllow altering of two_phase option of a SUBSCRIPTION.
Amit Kapila [Wed, 24 Jul 2024 04:43:36 +0000 (10:13 +0530)]
Allow altering of two_phase option of a SUBSCRIPTION.

The two_phase option is controlled by both the publisher (as a slot
option) and the subscriber (as a subscription option), so the slot option
must also be modified.

Changing the 'two_phase' option for a subscription from 'true' to 'false'
is permitted only when there are no pending prepared transactions
corresponding to that subscription. Otherwise, the changes of already
prepared transactions can be replicated again along with their corresponding
commit leading to duplicate data or errors.

To avoid data loss, the 'two_phase' option for a subscription can only be
changed from 'false' to 'true' once the initial data synchronization is
completed. Therefore this is performed later by the logical replication worker.

Author: Hayato Kuroda, Ajin Cherian, Amit Kapila
Reviewed-by: Peter Smith, Hou Zhijie, Amit Kapila, Vitaly Davydov, Vignesh C
Discussion: https://postgr.es/m/8fab8-65d74c80-1-2f28e880@39088166

9 months agoMove all extern declarations for GUC variables to header files
Peter Eisentraut [Wed, 24 Jul 2024 04:21:39 +0000 (06:21 +0200)]
Move all extern declarations for GUC variables to header files

Add extern declarations in appropriate header files for global
variables related to GUC.  In many cases, this was handled quite
inconsistently before, with some GUC variables declared in a header
file and some only pulled in via ad-hoc extern declarations in various
.c files.

Also add PGDLLIMPORT qualifications to those variables.  These were
previously missing because src/tools/mark_pgdllimport.pl has only been
used with header files.

This also fixes -Wmissing-variable-declarations warnings for GUC
variables (not yet part of the standard warning options).

Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/flat/e0a62134-83da-4ba4-8cdb-ceb0111c95ce@eisentraut.org

9 months agoDetect integer overflow in array_set_slice().
Nathan Bossart [Wed, 24 Jul 2024 02:59:02 +0000 (21:59 -0500)]
Detect integer overflow in array_set_slice().

When provided an empty initial array, array_set_slice() fails to
check for overflow when computing the new array's dimensions.
While such overflows are ordinarily caught by ArrayGetNItems(),
commands with the following form are accepted:

INSERT INTO t (i[-2147483648:2147483647]) VALUES ('{}');

To fix, perform the hazardous computations using overflow-detecting
arithmetic routines.  As with commit 18b585155a, the added test
cases generate errors that include a platform-dependent value, so
we again use psql's VERBOSITY parameter to suppress printing the
message text.

Reported-by: Alexander Lakhin
Author: Joseph Koshakow
Reviewed-by: Jian He
Discussion: https://postgr.es/m/31ad2cd1-db94-bdb3-f91a-65ffdb4bef95%40gmail.com
Backpatch-through: 12

9 months agoMove extern declarations for EXEC_BACKEND to header files
Peter Eisentraut [Tue, 23 Jul 2024 12:58:30 +0000 (14:58 +0200)]
Move extern declarations for EXEC_BACKEND to header files

This fixes warnings from -Wmissing-variable-declarations (not yet part
of the standard warning options) under EXEC_BACKEND.  The
NON_EXEC_STATIC variables need a suitable declaration in a header file
under EXEC_BACKEND.

Also fix the inconsistent application of the volatile qualifier for
PMSignalState, which was revealed by this change.

Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/flat/e0a62134-83da-4ba4-8cdb-ceb0111c95ce@eisentraut.org

9 months agoFix private struct field name to match the code using it.
Noah Misch [Tue, 23 Jul 2024 12:32:03 +0000 (05:32 -0700)]
Fix private struct field name to match the code using it.

Commit 8720a15e9ab121e49174d889eaeafae8ac89de7b added the wrong name.

Nazir Bilal Yavuz

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

9 months agoUse more consistently int64 for page numbers in SLRU-related code
Michael Paquier [Tue, 23 Jul 2024 08:59:05 +0000 (17:59 +0900)]
Use more consistently int64 for page numbers in SLRU-related code

clog.c, async.c and predicate.c included some SLRU page numbers still
handled as 4-byte integers, while int64 should be used for this purpose.

These holes have been introduced in 4ed8f0913bfd, that has introduced
the use of 8-byte integers for SLRU page numbers, still forgot about the
code paths updated by this commit.

Reported-by: Noah Misch
Author: Aleksander Alekseev, Michael Paquier
Discussion: https://postgr.es/m/20240626002747.dc.nmisch@google.com
Backpatch-through: 17

9 months agoldapurl is supported with simple bind
Peter Eisentraut [Tue, 23 Jul 2024 08:14:38 +0000 (10:14 +0200)]
ldapurl is supported with simple bind

The docs currently imply that ldapurl is for search+bind only, but
that's not true.  Rearrange the docs to cover this better.

Add a test ldapurl with simple bind.  This was previously allowed but
unexercised, and now that it's documented it'd be good to pin the
behavior.

Improve error when mixing LDAP bind modes.  The option names had gone
stale; replace them with a more general statement.

Author: Jacob Champion <jacob.champion@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/CAOYmi+nyg9gE0LeP=xQ3AgyQGR=5ZZMkVVbWd0uR8XQmg_dd5Q@mail.gmail.com

9 months agoGet rid of a global variable
Peter Eisentraut [Tue, 23 Jul 2024 07:53:54 +0000 (09:53 +0200)]
Get rid of a global variable

bootstrap_data_checksum_version can just as easily be passed to where
it is used via function arguments.

Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/flat/e0a62134-83da-4ba4-8cdb-ceb0111c95ce@eisentraut.org

9 months agoImprove comments in slru.{c,h} about segment name format
Michael Paquier [Tue, 23 Jul 2024 07:54:51 +0000 (16:54 +0900)]
Improve comments in slru.{c,h} about segment name format

slru.h described incorrectly how SLRU segment names are formatted
depending on the segment number and if long or short segment names are
used.  This commit closes the gap with a better description, fitting
with the reality.

Reported-by: Noah Misch
Author: Aleksander Alekseev
Discussion: https://postgr.es/m/20240626002747.dc.nmisch@google.com
Backpatch-through: 17

9 months agoReplace remaining strtok() with strtok_r()
Peter Eisentraut [Tue, 23 Jul 2024 07:13:48 +0000 (09:13 +0200)]
Replace remaining strtok() with strtok_r()

for thread-safety in the server in the future

Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: David Steele <david@pgmasters.net>
Discussion: https://www.postgresql.org/message-id/flat/79692bf9-17d3-41e6-b9c9-fc8c3944222a@eisentraut.org

9 months agoWindows replacement for strtok_r()
Peter Eisentraut [Tue, 23 Jul 2024 07:13:48 +0000 (09:13 +0200)]
Windows replacement for strtok_r()

They spell it "strtok_s" there.

There are currently no uses, but some will be added soon.

Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: David Steele <david@pgmasters.net>
Discussion: https://www.postgresql.org/message-id/flat/79692bf9-17d3-41e6-b9c9-fc8c3944222a@eisentraut.org

9 months agoRemove redundant code in create_gather_merge_path
Richard Guo [Tue, 23 Jul 2024 02:18:53 +0000 (11:18 +0900)]
Remove redundant code in create_gather_merge_path

In create_gather_merge_path, we should always guarantee that the
subpath is adequately ordered, and we do not add a Sort node in
createplan.c for a Gather Merge node.  Therefore, the 'else' branch in
create_gather_merge_path, which computes the cost for a Sort node, is
redundant.

This patch removes the redundant code and emits an error if the
subpath is not sufficiently ordered.  Meanwhile, this patch changes
the check for the subpath's pathkeys in create_gather_merge_plan to an
Assert.

Author: Richard Guo
Discussion: https://postgr.es/m/CAMbWs48u=0bWf3epVtULjJ-=M9Hbkz+ieZQAOS=BfbXZFqbDCg@mail.gmail.com

9 months agoFix rowcount estimate for gather (merge) paths
Richard Guo [Tue, 23 Jul 2024 01:33:26 +0000 (10:33 +0900)]
Fix rowcount estimate for gather (merge) paths

In the case of a parallel plan, when computing the number of tuples
processed per worker, we divide the total number of tuples by the
parallel_divisor obtained from get_parallel_divisor(), which accounts
for the leader's contribution in addition to the number of workers.

Accordingly, when estimating the number of tuples for gather (merge)
nodes, we should multiply the number of tuples per worker by the same
parallel_divisor to reverse the division.  However, currently we use
parallel_workers rather than parallel_divisor for the multiplication.
This could result in an underestimation of the number of tuples for
gather (merge) nodes, especially when there are fewer than four
workers.

This patch fixes this issue by using the same parallel_divisor for the
multiplication.  There is one ensuing plan change in the regression
tests, but it looks reasonable and does not compromise its original
purpose of testing parallel-aware hash join.

In passing, this patch removes an unnecessary assignment for path.rows
in create_gather_merge_path, and fixes an uninitialized-variable issue
in generate_useful_gather_paths.

No backpatch as this could result in plan changes.

Author: Anthonin Bonnefoy
Reviewed-by: Rafia Sabih, Richard Guo
Discussion: https://postgr.es/m/CAO6_Xqr9+51NxgO=XospEkUeAg-p=EjAWmtpdcZwjRgGKJ53iA@mail.gmail.com

9 months agoDoc: improve description of plpgsql's FETCH and MOVE commands.
Tom Lane [Mon, 22 Jul 2024 23:43:12 +0000 (19:43 -0400)]
Doc: improve description of plpgsql's FETCH and MOVE commands.

We were not being clear about which variants of the "direction"
clause are permitted in MOVE.  Also, the text seemed to be
written with only the FETCH/MOVE NEXT case in mind, so it
didn't apply very well to other variants.

Also, document that "MOVE count IN cursor" only works if count
is a constant.  This is not the whole truth, because some other
cases such as a parenthesized expression will also work, but
we want to push people to use "MOVE FORWARD count" instead.
The constant case is enough to cover what we allow in plain SQL,
and that seems sufficient to claim support for.

Update a comment in pl_gram.y claiming that we don't document
that point.

Per gripe from Philipp Salvisberg.

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

9 months agoRevert "Test that vacuum removes tuples older than OldestXmin"
Melanie Plageman [Mon, 22 Jul 2024 20:13:56 +0000 (16:13 -0400)]
Revert "Test that vacuum removes tuples older than OldestXmin"

This reverts commit aa607980aee08416211f003ab41aa750f5559712.

This test proved to be unstable on the buildfarm, timing out before the
standby could catch up on 32-bit machines where more rows were required
and failing to reliably trigger multiple index vacuum rounds on 64-bit
machines where fewer rows should be required.

Because the instability is only known to be present on versions of
Postgres with TIDStore used for dead TID storage by vacuum, this is only
being reverted on master and REL_17_STABLE.

As having this coverage may be valuable, there is a discussion on the
thread of possible ways to stabilize the test. If that happens, a fixed
test can be committed again.

Backpatch-through: 17
Reported-by: Tom Lane
Discussion: https://postgr.es/m/614152.1721580711%40sss.pgh.pa.us

9 months agoInitialize wal_level in the initial checkpoint record.
Robert Haas [Mon, 22 Jul 2024 19:32:43 +0000 (15:32 -0400)]
Initialize wal_level in the initial checkpoint record.

As per Coverity and Tom Lane, commit 402b586d0 (back-patched to v17
as 2b5819e2b) forgot to initialize this new structure member in this
code path.

9 months agoRemove grotty use of disable_cost for TID scan plans.
Robert Haas [Mon, 22 Jul 2024 18:57:53 +0000 (14:57 -0400)]
Remove grotty use of disable_cost for TID scan plans.

Previously, the code charged disable_cost for CurrentOfExpr, and then
subtracted disable_cost from the cost of a TID path that used
CurrentOfExpr as the TID qual, effectively disabling all paths except
that one. Now, we instead suppress generation of the disabled paths
entirely, and generate only the one that the executor will actually
understand.

With this approach, we do not need to rely on disable_cost being
large enough to prevent the wrong path from being chosen, and we
save some CPU cycle by avoiding generating paths that we can't
actually use. In my opinion, the code is also easier to understand
like this.

Patch by me. Review by Heikki Linnakangas.

Discussion: http://postgr.es/m/591b3596-2ea0-4b8e-99c6-fad0ef2801f5@iki.fi

9 months agoAdd missing call to ConditionVariableCancelSleep().
Robert Haas [Wed, 17 Jul 2024 18:53:00 +0000 (14:53 -0400)]
Add missing call to ConditionVariableCancelSleep().

After calling ConditionVariableSleep() or ConditionVariableTimedSleep()
one or more times, code is supposed to call ConditionVariableCancelSleep()
to remove itself from the waitlist. This code neglected to do so.
As far as I know, that had no observable consequences, but let's make
the code correct.

Discussion: http://postgr.es/m/CA+TgmoYW8eR+KN6zhVH0sin7QH6AvENqw_bkN-bB4yLYKAnsew@mail.gmail.com

9 months agoReplace some strtok() with strsep()
Peter Eisentraut [Mon, 22 Jul 2024 13:45:46 +0000 (15:45 +0200)]
Replace some strtok() with strsep()

strtok() considers adjacent delimiters to be one delimiter, which is
arguably the wrong behavior in some cases.  Replace with strsep(),
which has the right behavior: Adjacent delimiters create an empty
token.

Affected by this are parsing of:

- Stored SCRAM secrets
  ("SCRAM-SHA-256$<iterations>:<salt>$<storedkey>:<serverkey>")

- ICU collation attributes
  ("und@colStrength=primary;colCaseLevel=yes") for ICU older than
  version 54

- PG_COLORS environment variable
  ("error=01;31:warning=01;35:note=01;36:locus=01")

- pg_regress command-line options with comma-separated list arguments
  (--dbname, --create-role) (currently only used pg_regress_ecpg)

Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: David Steele <david@pgmasters.net>
Discussion: https://www.postgresql.org/message-id/flat/79692bf9-17d3-41e6-b9c9-fc8c3944222a@eisentraut.org

9 months agopostgres_fdw: Split out the query_cancel test to its own file
Alvaro Herrera [Mon, 22 Jul 2024 10:49:57 +0000 (12:49 +0200)]
postgres_fdw: Split out the query_cancel test to its own file

This allows us to skip it in Cygwin, where it's reportedly flaky because
of platform bugs or something.

Backpatch to 17, where the test was introduced by commit 2466d6654f85.

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/e4d0cb33-6be5-e4d5-ae49-9eac3ff2b005@gmail.com

9 months agoAdd port/ replacement for strsep()
Peter Eisentraut [Mon, 22 Jul 2024 07:47:02 +0000 (09:47 +0200)]
Add port/ replacement for strsep()

from OpenBSD, similar to strlcat, strlcpy

There are currently no uses, but some will be added soon.

Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: David Steele <david@pgmasters.net>
Discussion: https://www.postgresql.org/message-id/flat/79692bf9-17d3-41e6-b9c9-fc8c3944222a@eisentraut.org