postgresql.git
14 months agoecpg: Fix zero-termination of string generated by intoasc()
Michael Paquier [Mon, 19 Feb 2024 02:38:18 +0000 (11:38 +0900)]
ecpg: Fix zero-termination of string generated by intoasc()

intoasc(), a wrapper for PGTYPESinterval_to_asc that converts an
interval to its textual representation, used a plain memcpy() when
copying its result.  This could miss a zero-termination in the result
string, leading to an incorrect result.

The routines in informix.c do not provide the length of their result
buffer, which would allow a replacement of strcpy() to safer strlcpy()
calls, but this requires an ABI breakage and that cannot happen in
back-branches.

Author: Oleg Tselebrovskiy
Reviewed-by: Ashutosh Bapat
Discussion: https://postgr.es/m/bf47888585149f83b276861a1662f7e4@postgrespro.ru
Backpatch-through: 12

14 months agoecpg: Fix error handling on OOMs when parsing timestamps
Michael Paquier [Mon, 19 Feb 2024 00:05:51 +0000 (09:05 +0900)]
ecpg: Fix error handling on OOMs when parsing timestamps

pgtypes_alloc() can return NULL when failing an allocation, which is
something that PGTYPEStimestamp_defmt_asc() has forgotten about when
translating a timestamp for 'D', 'r', 'R' and 'T' as these require a
temporary allocation.

This is unlikely going to be a problem in practice, so no backpatch is
done.

Author: Oleg Tselebrovskiy
Discussion: https://postgr.es/m/bf47888585149f83b276861a1662f7e4@postgrespro.ru

14 months agoRemove remaining references to timeouts-long
Alexander Korotkov [Fri, 16 Feb 2024 21:58:44 +0000 (23:58 +0200)]
Remove remaining references to timeouts-long

Reported-by: Alexander Lakhin
14 months agoUse new overflow-safe integer comparison functions.
Nathan Bossart [Fri, 16 Feb 2024 20:05:36 +0000 (14:05 -0600)]
Use new overflow-safe integer comparison functions.

Commit 6b80394781 introduced integer comparison functions designed
to be as efficient as possible while avoiding overflow.  This
commit makes use of these functions in many of the in-tree qsort()
comparators to help ensure transitivity.  Many of these comparator
functions should also see a small performance boost.

Author: Mats Kindahl
Reviewed-by: Andres Freund, Fabrízio de Royes Mello
Discussion: https://postgr.es/m/CA%2B14426g2Wa9QuUpmakwPxXFWG_1FaY0AsApkvcTBy-YfS6uaw%40mail.gmail.com

14 months agoIntroduce overflow-safe integer comparison functions.
Nathan Bossart [Fri, 16 Feb 2024 19:37:02 +0000 (13:37 -0600)]
Introduce overflow-safe integer comparison functions.

This commit adds integer comparison functions that are designed to
be as efficient as possible while avoiding overflow.  A follow-up
commit will make use of these functions in many of the in-tree
qsort() comparators.  The new functions are not better in all cases
(e.g., when the comparator function is inlined), so it is important
to consider the context before using them.

Author: Mats Kindahl
Reviewed-by: Tom Lane, Heikki Linnakangas, Andres Freund, Thomas Munro, Andrey Borodin, Fabrízio de Royes Mello
Discussion: https://postgr.es/m/CA%2B14426g2Wa9QuUpmakwPxXFWG_1FaY0AsApkvcTBy-YfS6uaw%40mail.gmail.com

14 months agoPass correct count to WALRead().
Jeff Davis [Fri, 16 Feb 2024 19:09:11 +0000 (11:09 -0800)]
Pass correct count to WALRead().

Previously, some callers requested XLOG_BLCKSZ bytes
unconditionally. While this did not cause a problem, because the extra
bytes are ignored, it's confusing and makes it harder to add safety
checks. Additionally, the comment about zero padding was incorrect.

With this commit, all callers request the number of bytes they
actually need.

Author: Bharath Rupireddy
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/CALj2ACWBRFac2TingD3PE3w2EBHXUHY3=AEEZPJmqhpEOBGExg@mail.gmail.com

14 months agoAdd assert to WALReadFromBuffers().
Jeff Davis [Fri, 16 Feb 2024 18:35:42 +0000 (10:35 -0800)]
Add assert to WALReadFromBuffers().

Per suggestion from Andres.

Discussion: https://postgr.es/m/20240214025508.6mcblauossthvaw3@awork3.anarazel.de

14 months agoReplace calls to pg_qsort() with the qsort() macro.
Nathan Bossart [Fri, 16 Feb 2024 17:37:50 +0000 (11:37 -0600)]
Replace calls to pg_qsort() with the qsort() macro.

Calls to this function might give the impression that pg_qsort()
is somehow different than qsort(), when in fact there is a qsort()
macro in port.h that expands all in-tree uses to pg_qsort().

Reviewed-by: Mats Kindahl
Discussion: https://postgr.es/m/CA%2B14426g2Wa9QuUpmakwPxXFWG_1FaY0AsApkvcTBy-YfS6uaw%40mail.gmail.com

14 months agoAdd missing check_stack_depth() to some recursive functions
Alexander Korotkov [Fri, 16 Feb 2024 14:02:00 +0000 (16:02 +0200)]
Add missing check_stack_depth() to some recursive functions

Reported-by: Egor Chindyaskin, Alexander Lakhin
Discussion: https://postgr.es/m/1672760457.940462079%40f306.i.mail.ru

14 months agoRemove timeouts-long.out
Alexander Korotkov [Fri, 16 Feb 2024 13:57:37 +0000 (15:57 +0200)]
Remove timeouts-long.out

bf82f43790 removes timeouts-long.spec, but forgets to remove timeouts-long.out.

Reported-by: Alexander Lakhin
14 months agoImprove compression and storage support with inheritance
Peter Eisentraut [Fri, 16 Feb 2024 10:51:35 +0000 (11:51 +0100)]
Improve compression and storage support with inheritance

A child table can specify a compression or storage method different
from its parents.  This was previously an error.  (But this was
inconsistently enforced because for example the settings could be
changed later using ALTER TABLE.)  This now also allows an explicit
override if multiple parents have different compression or storage
settings, which was previously an error that could not be overridden.

The compression and storage properties remains unchanged in a child
inheriting from parent(s) after its creation, i.e., when using ALTER
TABLE ...  INHERIT.  (This is not changed.)

Before this change, the error detail would mention the first pair of
conflicting parent compression or storage methods.  But with this
change it waits till the child specification is considered by which
time we may have encountered many such conflicting pairs.  Hence the
error detail after this change does not include the conflicting
compression/storage methods.  Those can be obtained from parent
definitions if necessary.  The code to maintain list of all
conflicting methods or even the first conflicting pair does not seem
worth the convenience it offers.  This change is inline with what we
do with conflicting default values.

Before this commit, the specified storage method could be stored in
ColumnDef::storage (CREATE TABLE ... LIKE) or ColumnDef::storage_name
(CREATE TABLE ...).  This caused the MergeChildAttribute() and
MergeInheritedAttribute() to ignore a storage method specified in the
child definition since it looked only at ColumnDef::storage.  This
commit removes ColumnDef::storage and instead uses
ColumnDef::storage_name to save any storage method specification. This
is similar to how compression method specification is handled.

Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/24656cec-d6ef-4d15-8b5b-e8dfc9c833a7@eisentraut.org

14 months agoRemove non-existing file from .gitattributes
Peter Eisentraut [Fri, 16 Feb 2024 10:39:09 +0000 (11:39 +0100)]
Remove non-existing file from .gitattributes

The file was removed by ac25173cdbc.

Author: Jelte Fennema-Nio <postgres@jeltef.nl>
Discussion: https://www.postgresql.org/message-id/flat/CAGECzQQGzbroAXi%2BYicp3HvcCo4%3Dg84kaOgjuvQ5MW9F0ubOGg%40mail.gmail.com

14 months agoDisable autovacuum on primary in 040_standby_failover_slots_sync test.
Amit Kapila [Fri, 16 Feb 2024 09:12:50 +0000 (14:42 +0530)]
Disable autovacuum on primary in 040_standby_failover_slots_sync test.

Disable autovacuum to avoid generating xid during stats update as
otherwise the new XID could then be replicated to standby at some random
point making slots at primary lag behind standby during slot sync.

As per buildfarm

Author: Hou Zhijie
Discussion: https://postgr.es/m/514f6f2f-6833-4539-39f1-96cd1e011f23@enterprisedb.com
Discussion: https://postgr.es/m/CAA4eK1Jun8SGCoc6JEktxY_+L7GmoJWrdsx-KCEP=GL-SsWggQ@mail.gmail.com

14 months agoFix the incorrect format specifier used in commit 7a424ece48.
Amit Kapila [Fri, 16 Feb 2024 06:04:11 +0000 (11:34 +0530)]
Fix the incorrect format specifier used in commit 7a424ece48.

Author: Hou Zhijie
Discussion: https://postgr.es/m/514f6f2f-6833-4539-39f1-96cd1e011f23@enterprisedb.com
Discussion: https://postgr.es/m/OS0PR01MB5716CB015BAD807B29BC55BE944C2@OS0PR01MB5716.jpnprd01.prod.outlook.com

14 months agoChange the LOG level in 040_standby_failover_slots_sync.pl to DEBUG2.
Amit Kapila [Fri, 16 Feb 2024 04:43:51 +0000 (10:13 +0530)]
Change the LOG level in 040_standby_failover_slots_sync.pl to DEBUG2.

Temporarily change the log level of 040_standby_failover_slots_sync.pl to
DEBUG2. This is to get more information about BF failures. We will reset
it back to default once the tests are stabilized.

Author: Hou Zhijie
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/514f6f2f-6833-4539-39f1-96cd1e011f23@enterprisedb.com
Discussion: https://postgr.es/m/OS0PR01MB571633C23B2A4CAC5FB0371A944C2@OS0PR01MB5716.jpnprd01.prod.outlook.com

14 months agoAdd more LOG and DEBUG messages for slot synchronization.
Amit Kapila [Fri, 16 Feb 2024 03:32:54 +0000 (09:02 +0530)]
Add more LOG and DEBUG messages for slot synchronization.

This provides more information about remote slots during synchronization
which helps in debugging bugs and BF failures due to test case issues. We
might later want to change the LOG message added by this patch to DEBUG1.

Author: Hou Zhijie
Reviewed-by: Amit Kapila, Bertrand Drouvot
Discussion: https://postgr.es/m/514f6f2f-6833-4539-39f1-96cd1e011f23@enterprisedb.com
Discussion: https://postgr.es/m/OS0PR01MB571633C23B2A4CAC5FB0371A944C2@OS0PR01MB5716.jpnprd01.prod.outlook.com

14 months agoAttempt to stabilize flapping regression test
David Rowley [Fri, 16 Feb 2024 02:01:29 +0000 (15:01 +1300)]
Attempt to stabilize flapping regression test

Per buildfarm animal mylodon, the plan for this test was sometimes
swapping the join order for tenk1 and tenk2.  Given that add_path() has
no code that would cause this fluctuation when given paths with consistent
costs, this indicates that the costs must be fluctuating in some runs.
The only proven reason I've seen where that could happen was slight
variations in pg_class.relpages for some tables.  This was demonstrated to
be true by f03a9ca43 and related discussion.  Manually adjusting tenk2's
pg_class.relpages by subtracting just 1 page does cause the plan to change
for this test.

Here we've not gone to the same lengths to prove that's what's going on
in this case.  Proving that does not seem worth the time.  Let's just
shrink one side of the join so the additional cost of the swapped join
order is sufficiently different that if the relpages estimate is off a few
pages that the planner still shouldn't swap the join order.

Reported-by: Thomas Munro
Author: Andy Fan, David Rowley
Discussion: https://postgr.es/m/CA+hUKGLqC-NobKYfjxNM3Gexv9OJ-Fhvy9bugUcXsZjTqH7W=Q@mail.gmail.com

14 months agoFollowup fixes for transaction_timeout
Alexander Korotkov [Fri, 16 Feb 2024 01:36:38 +0000 (03:36 +0200)]
Followup fixes for transaction_timeout

Don't deal with transaction timeout in PostgresMain().  Instead, release
transaction timeout activated by StartTransaction() in
CommitTransaction()/AbortTransaction()/PrepareTransaction().  Deal with both
enabling and disabling transaction timeout in assign_transaction_timeout().

Also, remove potentially flaky timeouts-long isolation test, which has no
guarantees to pass on slow/busy machines.

Reported-by: Andres Freund
Discussion: https://postgr.es/m/20240215230856.pc6k57tqxt7fhldm%40awork3.anarazel.de

14 months agoIntroduce transaction_timeout
Alexander Korotkov [Thu, 15 Feb 2024 21:34:11 +0000 (23:34 +0200)]
Introduce transaction_timeout

This commit adds timeout that is expected to be used as a prevention
of long-running queries. Any session within the transaction will be
terminated after spanning longer than this timeout.

However, this timeout is not applied to prepared transactions.
Only transactions with user connections are affected.

Discussion: https://postgr.es/m/CAAhFRxiQsRs2Eq5kCo9nXE3HTugsAAJdSQSmxncivebAxdmBjQ%40mail.gmail.com
Author: Andrey Borodin <amborodin@acm.org>
Author: Japin Li <japinli@hotmail.com>
Author: Junwang Zhao <zhjwpku@gmail.com>
Reviewed-by: Nikolay Samokhvalov <samokhvalov@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Fujii Masao <masao.fujii@oss.nttdata.com>
Reviewed-by: bt23nguyent <bt23nguyent@oss.nttdata.com>
Reviewed-by: Yuhang Qiu <iamqyh@gmail.com>
14 months agoDoc: improve a couple of comments in postgresql.conf.sample.
Tom Lane [Thu, 15 Feb 2024 21:45:03 +0000 (16:45 -0500)]
Doc: improve a couple of comments in postgresql.conf.sample.

Clarify comments associated with max_parallel_workers and
related settings.

Per bug #18343 from Christopher Kline.

Discussion: https://postgr.es/m/18343-3a5e903d1d3692ab@postgresql.org

14 months agoPull up ANY-SUBLINK with the necessary lateral support.
Alexander Korotkov [Thu, 15 Feb 2024 10:05:52 +0000 (12:05 +0200)]
Pull up ANY-SUBLINK with the necessary lateral support.

For ANY-SUBLINK, we adopted a two-stage pull-up approach to handle
different types of scenarios. In the first stage, the sublink is pulled up
as a subquery. Because of this, when writing this code, we did not have
the ability to perform lateral joins, and therefore, we were unable to
pull up Var with varlevelsup=1. Now that we have the ability to use
lateral joins, we can eliminate this limitation.

Author: Andy Fan <zhihui.fan1213@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Alena Rybakina <lena.ribackina@yandex.ru>
Reviewed-by: Andrey Lepikhov <a.lepikhov@postgrespro.ru>
14 months agoAllow passing extra options to initdb for tests
Peter Eisentraut [Thu, 15 Feb 2024 08:52:08 +0000 (09:52 +0100)]
Allow passing extra options to initdb for tests

Setting the environment variable PG_TEST_INITDB_EXTRA_OPTS passes
extra options to initdb run by pg_regress or
PostgreSQL::Test::Cluster's init.

This can be useful for a wide variety of uses, like running all tests
with checksums enabled, or with JIT enabled, or with different GUC
settings, or with different locale settings.  (Not all tests are going
to pass with arbitrary options, but it is useful to run this against
specific test suites.)

Reviewed-by: Ian Lawrence Barwick <barwick@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/d4d2ad9f-1c1d-47a1-bb4d-c10a747d4f15%40eisentraut.org

14 months agoAnother try to fix BF failure introduced in commit ddd5f4f54a.
Amit Kapila [Thu, 15 Feb 2024 05:07:28 +0000 (10:37 +0530)]
Another try to fix BF failure introduced in commit ddd5f4f54a.

Before attempting to sync the slot on standby by
pg_sync_replication_slots(), ensure that on the primary restart_lsn for
the slot has moved to a recent WAL position, by re-creating the
subscription and the logical slot.

Author: Hou Zhijie
Discussion: https://postgr.es/m/CAA4eK1+d5Lne8vCAn0un4SP9x-ZBr2-xfxg01uSfeBTSCKFZoQ@mail.gmail.com

14 months agoSimplify PathKey checking code
David Rowley [Thu, 15 Feb 2024 05:01:28 +0000 (18:01 +1300)]
Simplify PathKey checking code

pathkeys_useful_for_ordering() contained some needless checks to return
0 when either root->query_pathkeys or pathkeys lists were empty.  This is
already handled by pathkeys_count_contained_in(), so let's have it do the
work instead of having redundant checks.

Similarly, in pathkeys_useful_for_grouping(), checking pathkeys is an
empty list just before looping over it isn't required.  Technically,
neither is the list empty check for group_pathkeys, but I felt a bit
more work would have to be done to get the equivalent behavior if we'd
left it up to the foreach loop to call list_member_ptr().

This was noticed by Andy while he was reviewing a patch to improve the
UNION planner.  Since that patch adds another function similar to
pathkeys_useful_for_ordering() and since I wasn't planning to copy these
redundant checks over to the new function, let's adjust the existing
code so that both functions will be consistent.

Author: Andy Fan
Discussion: https://postgr.es/m/87o7cti48f.fsf@163.com

14 months agoClarify the 'rows' parameter in create_append_path
David Rowley [Thu, 15 Feb 2024 00:13:31 +0000 (13:13 +1300)]
Clarify the 'rows' parameter in create_append_path

This is extracted from a larger patch to improve the UNION planner.
While working on that, I found myself having to check what the 'rows'
parameter is for.  It's not obvious that passing a negative number is the
way to have the rows estimate calculated and to find that out you need
to read code in create_append_path() and in cost_append().

Discussion: https://postgr.es/m/CAApHDvpb_63XQodmxKUF8vb9M7CxyUyT4sWvEgqeQU-GB7QFoQ@mail.gmail.com

14 months agoRemove obsolete check in SIGTERM handler for the startup process.
Nathan Bossart [Wed, 14 Feb 2024 23:09:31 +0000 (17:09 -0600)]
Remove obsolete check in SIGTERM handler for the startup process.

Thanks to commit 3b00fdba9f, this check in the SIGTERM handler for
the startup process is now obsolete and can be removed.  Instead of
leaving around the dead function write_stderr_signal_safe(), I've
opted to just remove it for now.

This partially reverts commit 97550c0711.

Reviewed-by: Andres Freund, Noah Misch
Discussion: https://postgr.es/m/20231121212008.GA3742740%40nathanxps13

14 months agoCentralize logic for restoring errno in signal handlers.
Nathan Bossart [Wed, 14 Feb 2024 22:34:18 +0000 (16:34 -0600)]
Centralize logic for restoring errno in signal handlers.

Presently, we rely on each individual signal handler to save the
initial value of errno and then restore it before returning if
needed.  This is easily forgotten and, if missed, often goes
undetected for a long time.

In commit 3b00fdba9f, we introduced a wrapper signal handler
function that checks whether MyProcPid matches getpid().  This
commit moves the aforementioned errno restoration code from the
individual signal handlers to the new wrapper handler so that we no
longer need to worry about missing it.

Reviewed-by: Andres Freund, Noah Misch
Discussion: https://postgr.es/m/20231121212008.GA3742740%40nathanxps13

14 months agoCheck that MyProcPid == getpid() in backend signal handlers.
Nathan Bossart [Wed, 14 Feb 2024 20:52:14 +0000 (14:52 -0600)]
Check that MyProcPid == getpid() in backend signal handlers.

In commit 97550c0711, we added a similar check to the SIGTERM
handler for the startup process.  This commit adds this check to
backend signal handlers installed with pqsignal().  This is done by
using a wrapper function that performs the check before calling the
actual handler.

The hope is that this will offer more general protection against
child processes of Postgres backends inadvertently modifying shared
memory due to inherited signal handlers.  Another potential
follow-up improvement is to use this wrapper handler function to
restore errno instead of relying on each individual handler
function to do so.

This commit makes the changes in commit 97550c0711 obsolete but
leaves reverting it for a follow-up commit.

Reviewed-by: Andres Freund, Noah Misch
Discussion: https://postgr.es/m/20231121212008.GA3742740%40nathanxps13

14 months agoAllow pg_monitor to execute pg_current_logfile().
Nathan Bossart [Wed, 14 Feb 2024 17:48:29 +0000 (11:48 -0600)]
Allow pg_monitor to execute pg_current_logfile().

We allow roles with privileges of pg_monitor to execute functions
like pg_ls_logdir(), so it seems natural that such roles would also
be able to execute this function.

Bumps catversion.

Co-authored-by: Pavlo Golub
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/CAK7ymcLmEYWyQkiCZ64WC-HCzXAB0omM%3DYpj9B3rXe8vUAFMqw%40mail.gmail.com

14 months agoFix multiranges to behave more like dependent types.
Tom Lane [Wed, 14 Feb 2024 16:30:39 +0000 (11:30 -0500)]
Fix multiranges to behave more like dependent types.

For most purposes, multiranges act like dependent objects of the
associated range type: you can't create them separately or drop them
separately.  This is like the way that autogenerated array types
behave.  However, a couple of points were overlooked: array types
automatically track the ownership of their base type, and array types
do not have their own permissions but use those of the base type,
while multiranges didn't emulate those behaviors.  This is fairly
broken, mainly because pg_dump doesn't think it needs to worry about
multiranges as separate objects, and thus it fails to dump/restore
ownership or permissions of multiranges.

There's no apparent value in letting a multirange diverge from
its parent's ownership or permissions, so let's make them act like
arrays in these respects.  However, we continue to let multiranges
be renamed or moved to a different schema independently of their
parent, since that doesn't break anything.

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

14 months agoFix BF introduced in commit ddd5f4f54a.
Amit Kapila [Wed, 14 Feb 2024 10:46:08 +0000 (16:16 +0530)]
Fix BF introduced in commit ddd5f4f54a.

The failure is that the remote slot is not synchronized after the same
slot on standby gets invalidated. The reason was that remote_slot's
restart_lsn was lagged behind the standby's oldest WAL segment. The test
didn't ensure that remote_slot's LSN was advanced to the latest position
before we tried to sync the slots via new the function
pg_sync_replication_slots().

Discussion: https://postgr.es/m/CAA4eK1JLBi3HzenB6do3_hd78kN0UDD1mz-vumWE52XHHEq5Bw@mail.gmail.com

14 months agodoc: Remove links to further reading from pgcrypto docs
Daniel Gustafsson [Wed, 14 Feb 2024 10:05:10 +0000 (11:05 +0100)]
doc: Remove links to further reading from pgcrypto docs

The pgcrypto docs contained a set of links for useful reading and
technical references. These sets of links were however not actively
curated and had stale content and dead links. Rather than investing
time into maintining these, this removes them altogether since there
are lots of resources online which are actively maintained.

Backpatch to all supported versions since these links have been in
the docs for a long time.

Reported-by: Hanefi Onaldi <hanefi.onaldi@microsoft.com>
Reviewed-by: Magnus Hagander <magnus@hagander.net>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/170774255387.3279713.2822272755998870925@wrigleys.postgresql.org
Backpatch-through: v12

14 months agoAdd a slot synchronization function.
Amit Kapila [Wed, 14 Feb 2024 04:15:36 +0000 (09:45 +0530)]
Add a slot synchronization function.

This commit introduces a new SQL function pg_sync_replication_slots()
which is used to synchronize the logical replication slots from the
primary server to the physical standby so that logical replication can be
resumed after a failover or planned switchover.

A new 'synced' flag is introduced in pg_replication_slots view, indicating
whether the slot has been synchronized from the primary server. On a
standby, synced slots cannot be dropped or consumed, and any attempt to
perform logical decoding on them will result in an error.

The logical replication slots on the primary can be synchronized to the
hot standby by using the 'failover' parameter of
pg-create-logical-replication-slot(), or by using the 'failover' option of
CREATE SUBSCRIPTION during slot creation, and then calling
pg_sync_replication_slots() on standby. For the synchronization to work,
it is mandatory to have a physical replication slot between the primary
and the standby aka 'primary_slot_name' should be configured on the
standby, and 'hot_standby_feedback' must be enabled on the standby. It is
also necessary to specify a valid 'dbname' in the 'primary_conninfo'.

If a logical slot is invalidated on the primary, then that slot on the
standby is also invalidated.

If a logical slot on the primary is valid but is invalidated on the
standby, then that slot is dropped but will be recreated on the standby in
the next pg_sync_replication_slots() call provided the slot still exists
on the primary server. It is okay to recreate such slots as long as these
are not consumable on standby (which is the case currently). This
situation may occur due to the following reasons:
- The 'max_slot_wal_keep_size' on the standby is insufficient to retain
WAL records from the restart_lsn of the slot.
- 'primary_slot_name' is temporarily reset to null and the physical slot
is removed.

The slot synchronization status on the standby can be monitored using the
'synced' column of pg_replication_slots view.

A functionality to automatically synchronize slots by a background worker
and allow logical walsenders to wait for the physical will be done in
subsequent commits.

Author: Hou Zhijie, Shveta Malik, Ajin Cherian based on an earlier version by Peter Eisentraut
Reviewed-by: Masahiko Sawada, Bertrand Drouvot, Peter Smith, Dilip Kumar, Nisha Moond, Kuroda Hayato, Amit Kapila
Discussion: https://postgr.es/m/514f6f2f-6833-4539-39f1-96cd1e011f23@enterprisedb.com

14 months agoRevert "Refactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM"
Michael Paquier [Wed, 14 Feb 2024 01:07:22 +0000 (10:07 +0900)]
Revert "Refactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM"

This reverts commit 95fb5b49024, for reasons similar to what led to
1aa8324b81fa.  In this case, the callback was called once per row, which
is less worse than the previous callback introduced for COPY TO called
once per argument for each row, still the patch set discussed to plug in
custom routines to the COPY paths would be able to know which subroutine
to use depending on its CopyFromState, so this led to a suboptimal
approach at the end.

For now, this part is reverted to consider better which approach to use.

Discussion: https://postgr.es/m/20240206014125.qofww7ew3dx3v3uk@awork3.anarazel.de

14 months agopgcrypto: Fix incorrect argument vs PG_GETARG*() mappings
Michael Paquier [Tue, 13 Feb 2024 23:59:05 +0000 (08:59 +0900)]
pgcrypto: Fix incorrect argument vs PG_GETARG*() mappings

The following functions use a mix of bytea and text arguments, but their
C internals always used PG_GETARG_BYTEA_PP(), creating an incorrect mix
with the argument types expected by encrypt_internal():
- pgp_sym_encrypt_bytea(bytea,text[,text])
- pgp_sym_encrypt(text,text[,text])
- pgp_sym_decrypt_bytea(bytea,text[,text])
- pgp_sym_decrypt(bytea,text[,text])
- pgp_pub_encrypt_bytea(bytea,bytea[,text])
- pgp_pub_encrypt(text,bytea[,text])
- pgp_pub_decrypt_bytea(bytea, bytea[,text[,text]])
- pgp_pub_decrypt(bytea,bytea[,text[,text]])

This commit fixes the inconsistencies between the PG_GETARG*() macros
and the argument types of each function.

Both BYTEA_PP() and TEXT_PP() rely on PG_DETOAST_DATUM_PACKED(), that
returns an unaligned pointer, so this was not leading to an actual
problem as far as I know, but let's be consistent.

Author: Shihao Zhong
Discussion: https://postgr.es/m/CAGRkXqRfiWT--DzVPx_UGpNHTt0YT5Jo8eV2CtT56jNP=QpXSQ@mail.gmail.com

14 months agoImprove comment about query_id_enabled in queryjumblefuncs.c
Michael Paquier [Tue, 13 Feb 2024 22:20:15 +0000 (07:20 +0900)]
Improve comment about query_id_enabled in queryjumblefuncs.c

The comment was inexact because query_id_enabled will not be switched to
"true" even if compute_query_id is "on", unless a module requests for
it.

While on it, this adds a comment to mention that IsQueryIdEnabled()
should be used to check if query ID computation is enabled or not.

Author: Yugo Nagata
Reviewed-by: Julien Rouhaud
Discussion: https://postgr.es/m/20240209153823.e29a68cadb14225f1362a2cf@sraoss.co.jp

14 months agoCatch overflow when rounding intervals in AdjustIntervalForTypmod.
Tom Lane [Tue, 13 Feb 2024 20:58:40 +0000 (15:58 -0500)]
Catch overflow when rounding intervals in AdjustIntervalForTypmod.

Previously, an interval microseconds field close to INT64_MAX or
INT64_MIN could overflow, producing a result with not even the
correct sign, while being rounded to match a precision specification.

This seems worth fixing, but not worth back-patching, in part
because the ereturn() notation doesn't exist very far back.

Report and patch by Joseph Koshakow (some cosmetic mods by me)

Discussion: https://postgr.es/m/CAAvxfHfpuLgqJYzkUcher466Z1LpmE+5Sm+zc8L6zKCOQ+6TDQ@mail.gmail.com

14 months agoFix 'mmap' DSM implementation with allocations larger than 4 GB
Heikki Linnakangas [Tue, 13 Feb 2024 19:23:41 +0000 (21:23 +0200)]
Fix 'mmap' DSM implementation with allocations larger than 4 GB

Fixes bug #18341. Backpatch to all supported versions.

Discussion: https://www.postgresql.org/message-id/18341-ce16599e7fd6228c@postgresql.org

14 months agoUse a safer outfuncs/readfuncs representation for BitStrings.
Tom Lane [Tue, 13 Feb 2024 17:18:25 +0000 (12:18 -0500)]
Use a safer outfuncs/readfuncs representation for BitStrings.

For a long time, our outfuncs.c code has supposed that the string
contents of a BitString node could just be printed literally with
no concern for quoting/escaping.  Now, that's okay if the string
literal contains only valid binary or hex digits ... but our lexer
doesn't check that, preferring to let bitin() be the sole authority
on what's valid.  So we could have raw parse trees that contain
incorrect BitString literals, and that can result in failures when
WRITE_READ_PARSE_PLAN_TREES debugging is enabled.

Fix by using outToken() to print the string field, and debackslash()
to read it.  This results in a change in the emitted representation
only in cases that would have failed before, and don't represent valid
SQL in the first place.  Between that and the fact that we don't store
raw parse trees in the catalogs, I judge this safe to apply without a
catversion bump.

Per bug #18340 from Alexander Lakhin.  Back-patch to v16; before that,
we lacked readfuncs support for BitString nodes, so that the problem
was only cosmetic.

Discussion: https://postgr.es/m/18340-4aa1ae6ed4121912@postgresql.org

14 months agoSkip .DS_Store files in server side utils
Daniel Gustafsson [Tue, 13 Feb 2024 12:47:12 +0000 (13:47 +0100)]
Skip .DS_Store files in server side utils

The macOS Finder application creates .DS_Store files in directories
when opened,  which creates problems for serverside utilities which
expect all files to be PostgreSQL specific files.  Skip these files
when encountered in pg_checksums, pg_rewind and pg_basebackup.

This was extracted from a larger patchset for skipping hidden files
and system files, where the concencus was to just skip these. Since
this is equally likely to happen in every version, backpatch to all
supported versions.

Reported-by: Mark Guertin <markguertin@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Tobias Bussmann <t.bussmann@gmx.net>
Discussion: https://postgr.es/m/E258CE50-AB0E-455D-8AAD-BB4FE8F882FB@gmail.com
Backpatch-through: v12

15 months agoUse correct format placeholder for timeline IDs
Peter Eisentraut [Tue, 13 Feb 2024 05:54:58 +0000 (06:54 +0100)]
Use correct format placeholder for timeline IDs

Should be %u rather than %d.

15 months agoDoc: Improve upgrade for streaming replication section.
Amit Kapila [Tue, 13 Feb 2024 04:15:01 +0000 (09:45 +0530)]
Doc: Improve upgrade for streaming replication section.

Currently the documentation of upgrade for streaming replication section
says that logical replication slots will be copied, but the logical
replication slots are copied only if the old primary is version 17.0 or
later.

Author: Shubham Khanna
Discussion: https://postgr.es/m/CAHv8RjJHCw0jpUo9PZxjcguzGt3j2W1_NH=QuREoN0nYiVdVeA@mail.gmail.com

15 months agoRead WAL directly from WAL buffers.
Jeff Davis [Mon, 12 Feb 2024 18:36:18 +0000 (10:36 -0800)]
Read WAL directly from WAL buffers.

If available, read directly from WAL buffers, avoiding the need to go
through the filesystem. Only for physical replication for now, but can
be expanded to other callers.

In preparation for replicating unflushed WAL data.

Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACXKKK%3DwbiG5_t6dGao5GoecMwRkhr7GjVBM_jg54%2BNa%3DQ%40mail.gmail.com
Reviewed-by: Andres Freund, Alvaro Herrera, Nathan Bossart, Dilip Kumar, Nitin Jadhav, Melih Mutlu, Kyotaro Horiguchi
15 months agoRemove "#ifdef WIN32" guards from src/port/win32*.c
Heikki Linnakangas [Mon, 12 Feb 2024 09:57:45 +0000 (11:57 +0200)]
Remove "#ifdef WIN32" guards from src/port/win32*.c

These files are only compiled on Windows, and most of them didn't have
"#ifdef WIN32" guards. Remove them from the few that did, for
consistency.

Author: Tristan Partin
Discussion: https://www.postgresql.org/message-id/CXGM9RYSXA2J.1DBO4MRXGZA9P@neon.tech

15 months agoRemove unnecessary smgropen() calls
Heikki Linnakangas [Mon, 12 Feb 2024 08:59:45 +0000 (10:59 +0200)]
Remove unnecessary smgropen() calls

Now that RelationCreateStorage() returns the SmgrRelation (since
commit 5c1560606dc), use that.

Author: Japin Li
Discussion: https://www.postgresql.org/message-id/ME3P282MB316600FA62F6605477F26F6AB6742@ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM

15 months agoFix some typos in event trigger docs
Alexander Korotkov [Sun, 11 Feb 2024 22:33:51 +0000 (00:33 +0200)]
Fix some typos in event trigger docs

Discussion: https://postgr.es/m/CALj2ACWFUW4jX9EW7CLxbzSS%2Bb7b0Z%3DxKYrqzj2Rstc9MCEx7g%40mail.gmail.com
Author: Bharath Rupireddy

15 months agoUse heap_inplace_update() to unset pg_database.dathasloginevt
Alexander Korotkov [Sun, 11 Feb 2024 22:33:44 +0000 (00:33 +0200)]
Use heap_inplace_update() to unset pg_database.dathasloginevt

Doing this instead of regular updates serves two purposes. First, that avoids
possible waiting on the row-level lock.  Second, that avoids dealing with
TOAST.

It's known that changes made by heap_inplace_update() may be lost due to
concurrent normal updates.  However, we are OK with that.  The subsequent
connections will still have a chance to set "dathasloginevt" to false.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/e2a0248e-5f32-af0c-9832-a90d303c2c61%40gmail.com

15 months agoRemove obsolete script related to MSVC build system
Peter Eisentraut [Sun, 11 Feb 2024 22:42:40 +0000 (23:42 +0100)]
Remove obsolete script related to MSVC build system

15 months agoFix gai_strerror() thread-safety on Windows.
Thomas Munro [Sun, 11 Feb 2024 21:47:57 +0000 (10:47 +1300)]
Fix gai_strerror() thread-safety on Windows.

Commit 5579388d removed code that supplied a fallback implementation of
getaddrinfo(), which was dead code on modern systems.  One tiny piece of
the removed code was still doing something useful on Windows, though:
that OS's own gai_strerror()/gai_strerrorA() function returns a pointer
to a static buffer that it overwrites each time, so it's not
thread-safe.  In rare circumstances, a multi-threaded client program
could get an incorrect or corrupted error message.

Restore the replacement gai_strerror() function, though now that it's
only for Windows we can put it into a win32-specific file and cut it
down to the errors that Windows documents.  The error messages here are
taken from FreeBSD, because Windows' own messages seemed too verbose.

Back-patch to 16.

Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKGKz%2BF9d2PTiXwfYV7qJw%2BWg2jzACgSDgPizUw7UG%3Di58A%40mail.gmail.com

15 months agoUse extensible buffers to assemble command lines
Peter Eisentraut [Sun, 11 Feb 2024 08:08:35 +0000 (09:08 +0100)]
Use extensible buffers to assemble command lines

This makes use of StringInfo to assemble command lines, instead of
using fixed-size buffers and the (remote) possibility of "command too
long" errors.  Also makes the code a bit simpler.

This covers the test driver programs pg_regress and
pg_isolation_regress.

Similar to the changes done for pg_rewind in a33e17f210.

Discussion: https://www.postgresql.org/message-id/2be4fee5-738f-4749-b9f8-b452032c7ade%40eisentraut.org

15 months agoDisallow jsonpath methods involving TZ in immutable functions
Andrew Dunstan [Sat, 10 Feb 2024 17:12:39 +0000 (12:12 -0500)]
Disallow jsonpath methods involving TZ in immutable functions

Timezones are not immutable and so neither is any function that relies on
them. In commit 66ea94e8, we introduced a few methods which do casting
from one time to another and thus may involve the current timezone.  To
preserve the immutability of jsonpath functions currently marked
immutable, disallow these methods from being called from non-TZ aware
functions.

Jeevan Chalke, per a report from Jian He.

15 months agoRemove race condition in pg_get_expr().
Tom Lane [Fri, 9 Feb 2024 17:29:41 +0000 (12:29 -0500)]
Remove race condition in pg_get_expr().

Since its introduction, pg_get_expr() has intended to silently
return NULL if called with an invalid relation OID, as can happen
when scanning the catalogs concurrently with relation drops.
However, there is a race condition: we check validity of the OID
at the start, but it could get dropped just afterward, leading to
failures.  This is the cause of some intermittent instability we're
seeing in a proposed new test case, and presumably it's a hazard in
the field as well.

We can fix this by AccessShareLock-ing the target relation for the
duration of pg_get_expr().  Since we don't require any permissions
on the target relation, this is semantically a bit undesirable.  But
it turns out that the set_relation_column_names() subroutine already
takes a transient AccessShareLock on that relation, and has done since
commit 2ffa740be in 2012.  Given the lack of complaints about that, it
seems like there should be no harm in holding the lock a bit longer.

Back-patch to all supported branches.

Discussion: https://postgr.es/m/31ddcc01-a71b-4e8c-9948-01d1c47293ca@eisentraut.org

15 months agoAvoid concurrent calls to bindtextdomain().
Tom Lane [Fri, 9 Feb 2024 16:21:08 +0000 (11:21 -0500)]
Avoid concurrent calls to bindtextdomain().

We previously supposed that it was okay for different threads to
call bindtextdomain() concurrently (cf. commit 1f655fdc3).
It now emerges that there's at least one gettext implementation
in which that triggers an abort() crash, so let's stop doing that.
Add mutexes guarding libpq's and ecpglib's calls, which are the
only ones that need worry about multithreaded callers.

Note: in libpq, we could perhaps have piggybacked on
default_threadlock() to avoid defining a new mutex variable.
I judge that not terribly safe though, since libpq_gettext could
be called from code that is holding the default mutex.  If that
were the first such call in the process, it'd fail.  An extra
mutex is cheap insurance against unforeseen interactions.

Per bug #18312 from Christian Maurer.  Back-patch to all
supported versions.

Discussion: https://postgr.es/m/18312-bbbabc8113592b78@postgresql.org
Discussion: https://postgr.es/m/264860.1707163416@sss.pgh.pa.us

15 months agoClean up Windows-specific mutex code in libpq and ecpglib.
Tom Lane [Fri, 9 Feb 2024 16:11:39 +0000 (11:11 -0500)]
Clean up Windows-specific mutex code in libpq and ecpglib.

Fix pthread-win32.h and pthread-win32.c to provide a more complete
emulation of POSIX pthread mutexes: define PTHREAD_MUTEX_INITIALIZER
and make sure that pthread_mutex_lock() can operate on a mutex
object that's been initialized that way.  Then we don't need the
duplicative platform-specific logic in default_threadlock() and
pgtls_init(), which we'd otherwise need yet a third copy of for
an upcoming bug fix.

Also, since default_threadlock() supposes that pthread_mutex_lock()
cannot fail, try to ensure that that's actually true, by getting
rid of the malloc call that was formerly involved in initializing
an emulated mutex.  We can define an extra state for the spinlock
field instead.

Also, replace the similar code in ecpglib/misc.c with this version.
While ecpglib's version at least had a POSIX-compliant API, it
also had the potential of failing during mutex init (but here,
because of CreateMutex failure rather than malloc failure).  Since
all of misc.c's callers ignore failures, it seems like a wise idea
to avoid failures here too.

A further improvement in this area could be to unify libpq's and
ecpglib's implementations into a src/port/pthread-win32.c file.
But that doesn't seem like a bug fix, so I'll desist for now.

In preparation for the aforementioned bug fix, back-patch to all
supported branches.

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

15 months agoRefactor pipe_read_line to return the full line
Daniel Gustafsson [Fri, 9 Feb 2024 14:03:16 +0000 (15:03 +0100)]
Refactor pipe_read_line to return the full line

Commit 5b2f4afffe6 refactored find_other_exec() and in the process
created pipe_read_line() into a static routine for reading a single
line of output, aimed at reading version numbers.  Commit a7e8ece41
later exposed it externally in order to read a postgresql.conf GUC
using "postgres -C ..".  Further, f06b1c598 also made use of it for
reading a version string much like find_other_exec().  The internal
variable remained "pgver", even when used for other purposes.

Since the function requires passing a buffer and its size, and at
most size - 1 bytes will be read via fgets(), there is a truncation
risk when using this for reading GUCs (like how pg_rewind does,
though the risk in this case is marginal).

To keep this as generic functionality for reading a line from a pipe,
this refactors pipe_read_line() into returning an allocated buffer
containing all of the line to remove the risk of silent truncation.

Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: John Naylor <johncnaylorls@gmail.com>
Discussion: https://postgr.es/m/DEDF73CE-D528-49A3-9089-B3592FD671A9@yesql.se

15 months agoFix usage of aggregate pathkeys in group_keys_reorder_by_pathkeys()
Alexander Korotkov [Fri, 9 Feb 2024 10:56:26 +0000 (12:56 +0200)]
Fix usage of aggregate pathkeys in group_keys_reorder_by_pathkeys()

group_keys_reorder_by_pathkeys() function searched for matching pathkeys
within root->group_pathkeys.  That could lead to picking an aggregate pathkey
and using its pathkey->pk_eclass->ec_sortref as an argument of
get_sortgroupref_clause_noerr().  Given that ec_sortref of an aggregate pathkey
references aggregate targetlist not query targetlist, this leads to incorrect
query optimization.

Fix this by looking for matching pathkeys only within the first
num_groupby_pathkeys pathkeys.

Reported-by: David G. Johnston
Discussion: https://postgr.es/m/CAKFQuwY3Ek%3DcLThgd8FdaSc5JRDVt0FaV00gMcWra%2BTAR4gGUw%40mail.gmail.com
Author: Andrei Lepikhov, Alexander Korotkov

15 months agoFix propagation of persistence to sequences in ALTER TABLE / ADD COLUMN
Peter Eisentraut [Fri, 9 Feb 2024 06:57:31 +0000 (07:57 +0100)]
Fix propagation of persistence to sequences in ALTER TABLE / ADD COLUMN

Fix for 344d62fb9a9: That commit introduced unlogged sequences and
made it so that identity/serial sequences automatically get the
persistence level of their owning table.  But this works only for
CREATE TABLE and not for ALTER TABLE / ADD COLUMN.  The latter would
always create the sequence as logged (default), independent of the
persistence setting of the table.  This is fixed here.

Note: It is allowed to change the persistence of identity sequences
directly using ALTER SEQUENCE.  So mistakes in existing databases can
be fixed manually.

Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/c4b6e2ed-bcdf-4ea7-965f-e49761094827%40eisentraut.org

15 months agoAdd previous commit to .git-blame-ignore-revs
Michael Paquier [Fri, 9 Feb 2024 02:06:32 +0000 (11:06 +0900)]
Add previous commit to .git-blame-ignore-revs

15 months agoFix indentation of copyto.c
Michael Paquier [Fri, 9 Feb 2024 02:05:01 +0000 (11:05 +0900)]
Fix indentation of copyto.c

Issue introduced by b619852086ed.

Per buildfarm member koel.

15 months agoImprove COPY TO performance when server and client encodings match
Michael Paquier [Fri, 9 Feb 2024 00:30:53 +0000 (09:30 +0900)]
Improve COPY TO performance when server and client encodings match

This commit fixes an oversight introduced in c61a2f58418e, where COPY TO
would attempt to do encoding conversions even if the encodings of the
client and the server matched for multi-byte encodings.  All conversions
go through pg_any_to_server() that makes the conversion a no-op when the
encodings of the client and the server match, even for multi-byte
encodings.

The logic was fine, but setting CopyToStateData->need_transcoding would
cause strlen() to be called for nothing for each attribute of all the
rows copied, and that was showing high in some profiles (more attributes
make that easier to reach).  This change improves the runtime of some
worst-case COPY TO queries by 15%~ (number present at least here).

This is a performance improvement, so no backpatch is done out of
caution as this is not a regression.

Reported-by: Andres Freund
Analyzed-by: Andres Freund
Author: Michael Paquier
Reviewed-by: Heikki Linnakangas
Discussion: https://postgr.es/m/20240206020504.edijzczkgd25ek6z@awork3.anarazel.de

15 months agoApply pg_dump test cleanups to test_pg_dump as well
Peter Eisentraut [Thu, 8 Feb 2024 20:19:03 +0000 (21:19 +0100)]
Apply pg_dump test cleanups to test_pg_dump as well

Apply the changes from 41a284411e0 to the test_pg_dump module as well.
Here, we just apply the new test consistency checks, but we don't need
to fix any existing tests.

Discussion: https://www.postgresql.org/message-id/flat/1f8cb371-e84e-434e-0367-6b716fb16fa1@eisentraut.org

15 months agoFix gcc >= 10 warning
Alexander Korotkov [Thu, 8 Feb 2024 19:59:28 +0000 (21:59 +0200)]
Fix gcc >= 10 warning

Reported-by: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACVQoFXxFm2kCmhHcdM7DjA84_bOjoM8HVAKHbE%2BKrZ1uA%40mail.gmail.com

15 months agodoc: Remove superfluous bracket in synopsis
Daniel Gustafsson [Thu, 8 Feb 2024 11:19:34 +0000 (12:19 +0100)]
doc: Remove superfluous bracket in synopsis

Commit 9c08aea6a30 accidentally added one too many end brackets
in the synopsis for CREATE DATABASE .. strategy = strat. Fix by
removing. Backpatch to v15 where it was introduced.

Reported-by: tim.needham2@gmail.com
Discussion: https://postgr.es/m/170734160862.3279712.810853722572951776@wrigleys.postgresql.org
Backpatch-through: v15

15 months agoFix wrong logic in TransactionIdInRecentPast()
Alexander Korotkov [Thu, 8 Feb 2024 10:45:26 +0000 (12:45 +0200)]
Fix wrong logic in TransactionIdInRecentPast()

The TransactionIdInRecentPast() should return false for all the transactions
older than TransamVariables->oldestClogXid.  However, the function contains
a bug in comparison FullTransactionId to TransactionID allowing full
transactions between nextXid - 2^32 and oldestClogXid - 2^31.

This commit fixes TransactionIdInRecentPast() by turning the oldestClogXid into
FullTransactionId first, then performing the comparison.

Backpatch to all supported versions.

Reported-by: Egor Chindyaskin
Bug: 18212
Discussion: https://postgr.es/m/18212-547307f8adf57262%40postgresql.org
Author: Karina Litskevich
Reviewed-by: Kyotaro Horiguchi
Backpatch-through: 12

15 months agoFix documentation build with older docbook-xsl
Peter Eisentraut [Thu, 8 Feb 2024 10:37:11 +0000 (11:37 +0100)]
Fix documentation build with older docbook-xsl

Commit b0f0a9432d0 backpatched some code from upstream DocBook XSL to
our customization layer.  It turned out that this failed to work with
anything but the latest DocBook XSL upstream version (1.79.*), because
the backpatched code references an XSLT parameter (autolink.index.see)
that is not defined in earlier versions (because the feature it is
used for did not exist yet).

There is no way in XSLT to test whether a parameter is declared before
the stylesheet processor tries and fails to access it.  So the
possibilities to fix this would be to either remove the code that uses
the parameter (and thus give up on the feature it is used for) or
declare the parameter in our customization layer.  The latter seems
easier, and with a few more lines of code we can backport the entire
autolink.index.see feature, so let's do that.  (If we didn't, then
with older stylesheets the parameter will appear as on, but it won't
actually do anything, because of the way the stylesheets are laid out,
so it's less confusing to just make it work.)

With this, the documentation build should work again with docbook-xsl
versions 1.77.*, 1.78.*, and 1.79.* (which already worked before).
Version 1.76.1 already didn't work before all this, so was not
considered here.

Reported-by: Peter Smith <smithpb2250@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/9077b779-a9f8-09c8-6e85-da1ebfba15af@eisentraut.org

15 months agoUpdate comment
Peter Eisentraut [Thu, 8 Feb 2024 09:18:50 +0000 (10:18 +0100)]
Update comment

The documentation output format htmlhelp is no longer supported, but a
comment still mentioned it.

15 months agoFix meson installation of xid_wraparound test.
Masahiko Sawada [Thu, 8 Feb 2024 08:03:59 +0000 (17:03 +0900)]
Fix meson installation of xid_wraparound test.

Fix for e255b646a, to prevent installation of xid_wraparound test
module during main install.

Reported-by: Peter Eisentraut
Discussion: https://postgr.es/m/84cd416a-0e37-4019-8380-1c8a3cdd8c5c%40eisentraut.org

15 months agoFix warnings in cpluspluscheck
John Naylor [Thu, 8 Feb 2024 03:04:57 +0000 (10:04 +0700)]
Fix warnings in cpluspluscheck

Various int variables were compared to macros that are of type size_t,
which caused -Wsign-compare warnings in cpluspluscheck.  Change those
to size_t, which also better describes their purpose.

Per report from Peter Eisentraut

Discussion: https://postgr.es/m/486847dc-6de5-464a-938e-bac98ec2438b%40eisentraut.org

15 months agoRename static function to avoid conflicting names
Daniel Gustafsson [Wed, 7 Feb 2024 21:16:21 +0000 (22:16 +0100)]
Rename static function to avoid conflicting names

Commit a4fd3aa719e moved setup_cancel_handler out of psql and
exporeted it as a global function.  While pg_dump isn't using
the header it's exported in,  having a conflicting name still
risks causing confusion when grepping the code for callsites,
so rename the static function in pg_dump to avoid this.

Author: Yugo Nagata <nagata@sraoss.co.jp>
Discussion: https://postgr.es/m/20240126094245.cf6718cc659273765f3ab69a@sraoss.co.jp

15 months agoRemove Start* macros in postmaster.c.
Nathan Bossart [Wed, 7 Feb 2024 18:50:48 +0000 (12:50 -0600)]
Remove Start* macros in postmaster.c.

These macros are just shorthands for calling StartChildProcess()
with the appropriate process type, and they arguably make the code
harder to understand.

Suggested-by: Andres Freund
Author: Reid Thompson
Reviewed-by: Bharath Rupireddy
Discussion: https://postgr.es/m/e88934c02a5c66f5e8caab2025f85da6b9026d0b.camel%40crunchydata.com

15 months agoUpdate PQparameterStatus and ParameterStatus docs
Alvaro Herrera [Wed, 7 Feb 2024 18:25:07 +0000 (19:25 +0100)]
Update PQparameterStatus and ParameterStatus docs

Cover scram_iterations, which was added in commit b577743000cd.  While
at it, turn the list into a <simplelist> with 2 columns, which is much
nicer to read.

In master, remove mentions of antediluvian versions before which some
parameters were not reported.

Noticed while investigating a question by Maiquel Grassi.

Backpatch to 16.

Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Jelte Fennema-Nio <postgres@jeltef.nl>
Discussion: https://postgr.es/m/202401301236.mc5ebrohhtsd@alvherre.pgsql

15 months agoAdjust reltarget assignment for UPPERREL_PARTIAL_DISTINCT rel
David Rowley [Wed, 7 Feb 2024 08:22:34 +0000 (21:22 +1300)]
Adjust reltarget assignment for UPPERREL_PARTIAL_DISTINCT rel

A comment in grouping_planner() claimed that the PlannerInfo
upper_targets array was not used in core code.  However, the code that
generated the paths for the UPPERREL_PARTIAL_DISTINCT rel made that
comment untrue.

Here we adjust the create_distinct_paths() function signature to pass
down the PathTarget the same as is done for create_grouping_paths(),
thus making the aforementioned comment true again.

In passing adjust the order of the upper_targets[] assignments.  These
seem to be following the reverse enum order apart from
UPPERREL_PARTIAL_DISTINCT.

Also, update the header comment for generate_gather_paths() to mention
the function is also used to create gather paths for partial distinct
paths.

Author: Richard Guo, David Rowley
Discussion: https://postgr.es/m/CAMbWs48u9VoVOouJsys1qOaC9WVGVmBa+wT1dx8KvxF5GPzezA@mail.gmail.com

15 months agoSet LSN for wbuf in _hash_freeovflpage() iff wbuf is modified.
Amit Kapila [Wed, 7 Feb 2024 05:40:12 +0000 (11:10 +0530)]
Set LSN for wbuf in _hash_freeovflpage() iff wbuf is modified.

Commit 861f86beea used REGBUF_NO_CHANGE at one of the places in the hash
index to register the clean buffers but forgot to avoid setting LSN in
that case.

Reported-by: Michael Paquier
Author: Kuroda Hayato
Reviewed-by: Amit Kapila, Michael Paquier
Discussion: https://postgr.es/m/ZbyVVG_7eW3YD5-A@paquier.xyz

15 months agoClean-ups for 776621a5e4 and 7329240437.
Amit Kapila [Wed, 7 Feb 2024 04:34:04 +0000 (10:04 +0530)]
Clean-ups for 776621a5e4 and 7329240437.

Following are a few clean-ups related to failover option support in slots:
1. Improve the documentation in create_subscription.sgml.
2. Remove the spurious blank line in subscriptioncmds.c.
3. Remove the NOTICE for alter_replication_slot in subscriptioncmds.c as
we would sometimes print it even when nothing has changed. One can find
the change by enabling log_replication_commands on the publisher.
4. Optimize ReplicationSlotAlter() function to prevent disk flushing when
the slot's data remains unchanged.

Author: Hou Zhijie
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/514f6f2f-6833-4539-39f1-96cd1e011f23@enterprisedb.com
Discussion: https://postgr.es/m/OS0PR01MB57164904651FB588A518E98894472@OS0PR01MB5716.jpnprd01.prod.outlook.com

15 months agoSimplify signature of CopyAttributeOutCSV() in copyto.c
Michael Paquier [Wed, 7 Feb 2024 03:28:55 +0000 (12:28 +0900)]
Simplify signature of CopyAttributeOutCSV() in copyto.c

This has come up in 2889fd23be56, reverted later on, and is still useful
on its own to reduce a bit the differences between the code paths
dedicated to CSV and text.

Discussion: https://postgr.es/m/ZcCKwAeFrlOqPBuN@paquier.xyz

15 months agoRevert "Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO"
Michael Paquier [Tue, 6 Feb 2024 23:04:26 +0000 (08:04 +0900)]
Revert "Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO"

This reverts commit 2889fd23be56, following a discussion with Andres
Freund as this callback, being called once per attribute when sending a
relation's row, can involve a lot of indirect function calls (more
attributes to deal with means more impact).  The effects of a dispatch
at this level would become more visible when improving the per-row code
execution of COPY TO, impacting future potential performance
improvements.

Discussion: https://postgr.es/m/20240206014125.qofww7ew3dx3v3uk@awork3.anarazel.de

15 months agoChange initial use of pg_atomic_write_u64 to init
Alvaro Herrera [Tue, 6 Feb 2024 11:08:39 +0000 (12:08 +0100)]
Change initial use of pg_atomic_write_u64 to init

This only matters when using atomics emulation with semaphores.

Per buildfarm member rorqual.

15 months agoUse atomic access for SlruShared->latest_page_number
Alvaro Herrera [Tue, 6 Feb 2024 09:54:10 +0000 (10:54 +0100)]
Use atomic access for SlruShared->latest_page_number

The new concurrency model proposed for slru.c to improve performance
does not include any single lock that would coordinate processes
doing concurrent reads/writes on SlruShared->latest_page_number.
We can instead use atomic reads and writes for that variable.

Author: Dilip Kumar <dilipbalaut@gmail.com>
Reviewed-by: Andrey M. Borodin <x4mmm@yandex-team.ru>
Discussion: https://postgr.es/m/CAFiTN-vzDvNz=ExGXz6gdyjtzGixKSqs0mKHMmaQ8sOSEFZ33A@mail.gmail.com

15 months agoFurther cosmetic review of hashfn_unstable.h
John Naylor [Tue, 6 Feb 2024 06:09:03 +0000 (13:09 +0700)]
Further cosmetic review of hashfn_unstable.h

In follow-up to e97b672c8,
* Flesh out comments explaining the incremental interface
* Clarify detection of zero bytes when hashing aligned C strings

The latter was suggested and reviewed by Jeff Davis

Discussion: https://postgr.es/m/48e8f8bbe0be9c789f98776c7438244ab7a7cc63.camel%40j-davis.com

15 months agoSimplify initialization of incremental hash state
John Naylor [Sun, 21 Jan 2024 12:19:14 +0000 (19:19 +0700)]
Simplify initialization of incremental hash state

The standalone functions fasthash{32,64} use length for two purposes:
how many bytes to hash, and how to perturb the internal seed.

Developers using the incremental interface may not know the length
ahead of time (e.g. for C strings). In this case, it's advised to
pass length to the finalizer, but initialization still needed some
length up front, in the form of a placeholder macro.

Separate the concerns by having the standalone functions perturb the
internal seed themselves from their own length parameter, allowing
to remove "len" from fasthash_init(), as well as the placeholder macro.

Discussion: https://postgr.es/m/CANWCAZbTUk2LOyhsFo33gjLyLAHZ7ucXCi5K9u%3D%2BPtnTShDKtw%40mail.gmail.com

15 months agodoc: Spell I/O consistently
Michael Paquier [Tue, 6 Feb 2024 04:29:22 +0000 (13:29 +0900)]
doc: Spell I/O consistently

The pg_stat_io and pg_stat_copy_progress view docs spelled "I/O" as "IO"
or even "io" in some places when not referring to literal names or
string values.

Author: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/87fry6lx5s.fsf@wibble.ilmari.org

15 months agoFix meson installation of new generated files
Peter Eisentraut [Mon, 5 Feb 2024 14:45:29 +0000 (15:45 +0100)]
Fix meson installation of new generated files

Fix for 9b1a6f50b9: We want to install catalog/syscache_ids.h but not
catalog/syscache_info.h.  The meson code has this backwards.  The
makefiles are ok.

Reported-by: Aleksander Alekseev <aleksander@timescale.com>
Discussion: https://www.postgresql.org/message-id/CAJ7c6TMDGmAiozDjJQ3%3DP3cd-1BidC_GpitcAuU0aqq-r1eSoQ%40mail.gmail.com

15 months agoFix assertion if index is dropped during REFRESH CONCURRENTLY
Heikki Linnakangas [Mon, 5 Feb 2024 09:01:30 +0000 (11:01 +0200)]
Fix assertion if index is dropped during REFRESH CONCURRENTLY

When assertions are disabled, the built SQL statement is invalid and
you get a "syntax error". So this isn't a serious problem, but let's
avoid the assertion failure.

Backpatch to all supported versions.

Reviewed-by: Noah Misch
15 months agoRun REFRESH MATERIALIZED VIEW CONCURRENTLY in right security context
Heikki Linnakangas [Mon, 5 Feb 2024 09:01:23 +0000 (11:01 +0200)]
Run REFRESH MATERIALIZED VIEW CONCURRENTLY in right security context

The internal commands in REFRESH MATERIALIZED VIEW CONCURRENTLY are
correctly executed in SECURITY_RESTRICTED_OPERATION mode, except for
creating the temporary "diff" table, because you cannot create
temporary tables in SRO mode. But creating the temporary "diff" table
is a pretty complex CTAS command that selects from another temporary
table created earlier in the command. If you can cajole that CTAS
command to execute code defined by the table owner, the table owner
can run code with the privileges of the user running the REFRESH
command.

The proof-of-concept reported to the security team relied on CREATE
RULE to convert the internally-built temp table to a view. That's not
possible since commit b23cd185fd, and I was not able to find a
different way to turn the SELECT on the temp table into code
execution, so as far as I know this is only exploitable in v15 and
below. That's a fiddly assumption though, so apply this patch to
master and all stable versions.

Thanks to Pedro Gallegos for the report.

Security: CVE-2023-5869
Reviewed-by: Noah Misch
15 months agoEnhance libpqrcv APIs to support slot synchronization.
Amit Kapila [Mon, 5 Feb 2024 05:15:34 +0000 (10:45 +0530)]
Enhance libpqrcv APIs to support slot synchronization.

This patch provides support for regular (non-replication) connections in
libpqrcv_connect(). This can be used to execute SQL statements on the
primary server without starting a walsender.

A new API libpqrcv_get_dbname_from_conninfo() is also added to extract the
database name from the given connection-info.

Note that this patch doesn't change any existing functionality but later
patches implementing the slot synchronization will use this functionality
to connect to the primary server to fetch required slot information.

Author: Shveta Malik, Hou Zhijie, Ajin Cherian
Reviewed-by: Peter Smith, Bertrand Drouvot, Dilip Kumar, Masahiko Sawada, Nisha Moond, Kuroda Hayato, Amit Kapila
Discussion: https://postgr.es/m/514f6f2f-6833-4539-39f1-96cd1e011f23@enterprisedb.com

15 months agoImprove the comments in 004_subscription.pl.
Amit Kapila [Mon, 5 Feb 2024 03:21:33 +0000 (08:51 +0530)]
Improve the comments in 004_subscription.pl.

It was not clear whether the subscriptions in the upgraded instance were
the ones that retained the previous subscription's properties.

Author: Peter Smith
Reviewed-by: Vignesh C, Alvaro Herrera
Discussion: https://postgr.es/m/CAHut+Pu1usLPHRySPTacY1K_Q-ddSRXNFhmj_2u1NfqBC1ytng@mail.gmail.com

15 months agoRefactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO
Michael Paquier [Mon, 5 Feb 2024 02:12:37 +0000 (11:12 +0900)]
Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO

These routines are used by the text and CSV formats to send an output
representation of a string, applying quotes if required.  This is
similar to 95fb5b49024a, reducing the number of "if" branches that need
to be checked on a per-row basis when sending representation of fields
in text or CSV mode.

While on it, this simplifies the signature of CopyAttributeOutCSV() as
it is possible to know that an attribute is alone on a line thanks to
CopyToState.  Headers should not use quotes, even if forced at query
level.

Extracted from a larger patch by the same author.

Author: Sutou Kouhei
Discussion: https://postgr.es/m/20231204.153548.2126325458835528809.kou@clear-code.com

15 months agoRefactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM
Michael Paquier [Mon, 5 Feb 2024 00:46:02 +0000 (09:46 +0900)]
Refactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM

CopyReadAttributes{CSV,Text}() are used to parse lines for text and CSV
format.  This reduces the number of "if" branches that need to be
checked when parsing fields in CSV and text mode when dealing with a
COPY FROM, something that can become more noticeable with more
attributes and more lines to process.

Extracted from a larger patch by the same author.

Author: Sutou Kouhei
Discussion: https://postgr.es/m/20231204.153548.2126325458835528809.kou@clear-code.com

15 months agolibpq: Change some static functions to extern
Alvaro Herrera [Sun, 4 Feb 2024 15:35:16 +0000 (16:35 +0100)]
libpq: Change some static functions to extern

This is in preparation of a follow up commit that starts using these
functions from fe-cancel.c.

Author: Jelte Fennema-Nio <jelte.fennema@microsoft.com>
Discussion: https://postgr.es/m/AM5PR83MB0178D3B31CA1B6EC4A8ECC42F7529@AM5PR83MB0178.EURPRD83.prod.outlook.com

15 months agolibpq: Add pqReleaseConnHosts function
Alvaro Herrera [Sun, 4 Feb 2024 15:19:20 +0000 (16:19 +0100)]
libpq: Add pqReleaseConnHosts function

In a follow up commit we'll need to free this connhost field in a
function defined in fe-cancel.c, so here we extract the logic to a
dedicated extern function.

Author: Jelte Fennema-Nio <jelte.fennema@microsoft.com>
Discussion: https://postgr.es/m/AM5PR83MB0178D3B31CA1B6EC4A8ECC42F7529@AM5PR83MB0178.EURPRD83.prod.outlook.com

15 months agopg_basebackup: Fix check for MINIMUM_VERSION_FOR_WAL_SUMMARIES
Michael Paquier [Sun, 4 Feb 2024 01:51:53 +0000 (10:51 +0900)]
pg_basebackup: Fix check for MINIMUM_VERSION_FOR_WAL_SUMMARIES

MINIMUM_VERSION_FOR_WAL_SUMMARIES is used to check if the directory
pg_wal/summaries/ should be created in a base backup, but its condition
was reversed: pg_wal/summaries/ would be created when taking base
backups from backends of v16 and older versions, but it should be
created in base backups taken from backends of v17 and newer versions.

Author: Artur Zakirov
Reviewed-by: Yugo Nagata, Nazir Bilal Yavuz
Discussion: https://postgr.es/m/CAKNkYnzkkQ0gb_ZmLTY0r2-qV1q6imXgcCWxdA6UoA6yJkujGg@mail.gmail.com

15 months agoImprove documentation for COPY ... ON_ERROR ...
Alexander Korotkov [Fri, 2 Feb 2024 23:49:51 +0000 (01:49 +0200)]
Improve documentation for COPY ... ON_ERROR ...

Discussion: https://postgr.es/m/20240126112829.d420b28859fbe84379fdb7ad%40sraoss.co.jp
Author: Yugo Nagata
Reviewed-by: Masahiko Sawada, David G. Johnston, Atsushi Torikoshi
15 months agoFix typo in comments
Heikki Linnakangas [Fri, 2 Feb 2024 22:18:21 +0000 (00:18 +0200)]
Fix typo in comments

Backpatch-through: v16

15 months agoTranslate ENOMEM to ERRCODE_OUT_OF_MEMORY in errcode_for_file_access().
Tom Lane [Fri, 2 Feb 2024 20:34:29 +0000 (15:34 -0500)]
Translate ENOMEM to ERRCODE_OUT_OF_MEMORY in errcode_for_file_access().

Previously you got ERRCODE_INTERNAL_ERROR, which seems inappropriate,
especially given that we're trying to avoid emitting that in reachable
cases.

Alexander Kuzmenkov

Discussion: https://postgr.es/m/CALzhyqzgQph0BY8-hFRRGdHhF8CoqmmDHW9S=hMZ-HMzLxRqDQ@mail.gmail.com

15 months agoFix bug in bulk extending temp relation after failure
Heikki Linnakangas [Fri, 2 Feb 2024 19:12:30 +0000 (21:12 +0200)]
Fix bug in bulk extending temp relation after failure

A ResourceOwnerEnlarge() call was missing. That led to an error:

ERROR:  ResourceOwnerRemember called but array was full

and an assertion failure, if you tried to extend a temp relation again
after a failure. Alexander's test case used running out of disk space
to trigger the original failure.

This bug was introduced in the large ResourceOwner rewrite commit
b8bff07daa. Before that, the UnpinLocalBuffer() call guaranteed that
the subsequent PinLocalBuffer() will succeed, but after the rewrite,
releasing an old resource doesn't guarantee that there is space for a
new one.

Add a comment explaining why the UnpinBuffer + PinBuffer calls in
BufferAlloc(), with no ResourceOwnerEnlarge() in between, are safe.

Reported-by: Alexander Lakhin
Discussion: https://www.postgresql.org/message-id/dc574fea-c83e-a600-08cd-10881762e4fa@gmail.com

15 months agoAllow Gather Merge in more cases for parallel DISTINCT
David Rowley [Fri, 2 Feb 2024 11:20:18 +0000 (00:20 +1300)]
Allow Gather Merge in more cases for parallel DISTINCT

Here we adjust the partial path generation for parallel DISTINCT queries
to add Sort nodes on top of any unsorted partial distinct paths.

This increases the likelihood of the planner pushing a Sort below a Gather
Merge which enables the final phase of the parallel distinct to be
implemented using a Unique node in more cases.

Sorting the partial distinct paths is particularly useful when the
DISTINCT query has an ORDER BY and LIMIT clause as this can allow cheaper
plans by having the workers Hash Aggregate then Sort before feeding the
results into the Gather Merge.  The non-parallel portion of the plan then
becomes very cheap as it leaves only Unique and Limit to do in the leader
process.

Author: Richard Guo
Reviewed-by: David Rowley
Discussion: https://postgr.es/m/CAMbWs48u9VoVOouJsys1qOaC9WVGVmBa+wT1dx8KvxF5GPzezA@mail.gmail.com

15 months agoSync PG_VERSION file in CREATE DATABASE.
Noah Misch [Thu, 1 Feb 2024 21:44:19 +0000 (13:44 -0800)]
Sync PG_VERSION file in CREATE DATABASE.

An OS crash could leave PG_VERSION empty or missing.  The same symptom
appeared in a backup by block device snapshot, taken after the next
checkpoint and before the OS flushes the PG_VERSION blocks.  Device
snapshots are not a documented backup method, however.  Back-patch to
v15, where commit 9c08aea6a3090a396be334cc58c511edab05776a introduced
STRATEGY=WAL_LOG and made it the default.

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

15 months agoHandle interleavings between CREATE DATABASE steps and base backup.
Noah Misch [Thu, 1 Feb 2024 21:44:19 +0000 (13:44 -0800)]
Handle interleavings between CREATE DATABASE steps and base backup.

Restoring a base backup taken in the middle of CreateDirAndVersionFile()
or write_relmap_file() would lose the function's effects.  The symptom
was absence of the database directory, PG_VERSION file, or
pg_filenode.map.  If missing the directory, recovery would fail.  Either
missing file would not fail recovery but would render the new database
unusable.  Fix CreateDirAndVersionFile() with the transam/README "action
first and then write a WAL entry" strategy.  That has a side benefit of
moving filesystem mutations out of a critical section, reducing the ways
to PANIC.  Fix the write_relmap_file() call with a lock acquisition, so
it interacts with checkpoints like non-CREATE DATABASE calls do.
Back-patch to v15, where commit 9c08aea6a3090a396be334cc58c511edab05776a
introduced STRATEGY=WAL_LOG and made it the default.

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

15 months agoUpdate time zone data files to tzdata release 2024a.
Tom Lane [Thu, 1 Feb 2024 20:57:53 +0000 (15:57 -0500)]
Update time zone data files to tzdata release 2024a.

DST law changes in Ittoqqortoormiit, Greenland (America/Scoresbysund),
Kazakhstan (Asia/Almaty and Asia/Qostanay) and Palestine; as well as
updates for the Antarctic stations Casey and Vostok.

Historical corrections for Vietnam, Toronto, and Miquelon.

15 months agoAvoid package qualification of $windows_os
Andrew Dunstan [Thu, 1 Feb 2024 20:17:41 +0000 (15:17 -0500)]
Avoid package qualification of $windows_os

Further fallout from commit 6ee26c6a4b. To keep code in sync and avoid
issues on older releases with different package names, simply use the
unqualified name like many other places in our code.