postgresql.git
6 months agoConsider explicit incremental sort for mergejoins
Richard Guo [Wed, 9 Oct 2024 08:14:42 +0000 (17:14 +0900)]
Consider explicit incremental sort for mergejoins

For a mergejoin, if the given outer path or inner path is not already
well enough ordered, we need to do an explicit sort.  Currently, we
only consider explicit full sort and do not account for incremental
sort.

In this patch, for the outer path of a mergejoin, we choose to use
explicit incremental sort if it is enabled and there are presorted
keys.  For the inner path, though, we cannot use incremental sort
because it does not support mark/restore at present.

The rationale is based on the assumption that incremental sort is
always faster than full sort when there are presorted keys, a premise
that has been applied in various parts of the code.  In addition, the
current cost model tends to favor incremental sort as being cheaper
than full sort in the presence of presorted keys, making it reasonable
not to consider full sort in such cases.

It could be argued that what if a mergejoin with an incremental sort
as the outer path is selected as the inner path of another mergejoin.
However, this should not be a problem, because mergejoin itself does
not support mark/restore either, and we will add a Material node on
top of it anyway in this case (see final_cost_mergejoin).

There is one ensuing plan change in the regression tests, and we have
to modify that test case to ensure that it continues to test what it
is intended to.

No backpatch as this could result in plan changes.

Author: Richard Guo
Reviewed-by: David Rowley, Tomas Vondra
Discussion: https://postgr.es/m/CAMbWs49x425QrX7h=Ux05WEnt8GS757H-jOP3_xsX5t1FoUsZw@mail.gmail.com

6 months agoRemove incorrect function import from pgindent
Daniel Gustafsson [Wed, 9 Oct 2024 07:34:34 +0000 (09:34 +0200)]
Remove incorrect function import from pgindent

Commit 149ac7d4559 which re-implemented pgindent in Perl explicitly
imported the devnull function from File::Spec, but the module does
not export anything.  In recent versions of Perl calling a missing
import function cause a warning, which combined with warnings being
fatal cause pgindent to error out.

Backpatch to all supported versions.

Author: Erik Wienhold <ewie@ewie.name>
Reviewed-by: Andrew Dunstan <andrew@dunslane.net>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discusson: https://postgr.es/m/2372cd74-11b0-46f9-b28e-8f9627215d19@ewie.name
Backpatch-through: v12

6 months agoAllow roles created by new test to log in under SSPI.
Tom Lane [Tue, 8 Oct 2024 23:46:39 +0000 (19:46 -0400)]
Allow roles created by new test to log in under SSPI.

Semi-blind attempt to fix 6a1d0d470 to work on Windows,
along the same lines as a70f2a57f.  Per buildfarm.

6 months agopg_stat_statements: Add columns to track parallel worker activity
Michael Paquier [Tue, 8 Oct 2024 23:30:45 +0000 (08:30 +0900)]
pg_stat_statements: Add columns to track parallel worker activity

The view pg_stat_statements gains two columns:
- parallel_workers_to_launch, the number of parallel workers planned to
be launched.
- parallel_workers_launched, the number of parallel workers actually
launched.

The ratio of both columns offers hints that parallel workers are lacking
on a per-statement basis, requiring some tuning, in coordination with
"calls", the number of times a query is executed.

As of now, these numbers are tracked within Gather and GatherMerge
nodes.  They could be extended to utilities that make use of parallel
workers (parallel btree and brin, VACUUM).

The module is bumped to 1.12.

Author: Guillaume Lelarge
Discussion: https://postgr.es/m/CAECtzeWtTGOK0UgKXdDGpfTVSa5bd_VbUt6K6xn8P7X+_dZqKw@mail.gmail.com

6 months agoIntroduce two fields in EState to track parallel worker activity
Michael Paquier [Tue, 8 Oct 2024 23:07:48 +0000 (08:07 +0900)]
Introduce two fields in EState to track parallel worker activity

These fields can be set by executor nodes to record how many parallel
workers were planned to be launched and how many of them have been
actually launched within the number initially planned.  This data is
able to give an approximation of the parallel worker draught a system
is facing, making easier the tuning of related configuration parameters.

These fields will be used by some follow-up patches to populate other
parts of the system with their data.

Author: Guillaume Lelarge, Benoit Lobréau
Discussion: https://postgr.es/m/783bc7f7-659a-42fa-99dd-ee0565644e25@dalibo.com
Discussion: https://postgr.es/m/CAECtzeWtTGOK0UgKXdDGpfTVSa5bd_VbUt6K6xn8P7X+_dZqKw@mail.gmail.com

6 months agoSilence assorted annoying test output.
Tom Lane [Tue, 8 Oct 2024 18:13:01 +0000 (14:13 -0400)]
Silence assorted annoying test output.

Remove unnecessary chatter about "checking if IO::Socket::UNIX works";
our tests should never print anything on stderr unless there's a
problem.

Add .gitignore entry for temporary directory now being left behind
in src/test/postmaster.

6 months agoAdd min and max aggregates for bytea type.
Tom Lane [Tue, 8 Oct 2024 17:52:14 +0000 (13:52 -0400)]
Add min and max aggregates for bytea type.

Similar to a0f1fce80, although we chose to duplicate logic
rather than invoke byteacmp, primarily to avoid repeat detoasting.

Marat Buharov, Aleksander Alekseev

Discussion: https://postgr.es/m/CAPCEVGXiASjodos4P8pgyV7ixfVn-ZgG9YyiRZRbVqbGmfuDyg@mail.gmail.com

6 months agoUse aux process resource owner in walsender
Andres Freund [Tue, 8 Oct 2024 15:37:45 +0000 (11:37 -0400)]
Use aux process resource owner in walsender

AIO will need a resource owner to do IO. Right now we create a resowner
on-demand during basebackup, and we could do the same for AIO. But it seems
easier to just always create an aux process resowner.

Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/1f6b50a7-38ef-4d87-8246-786d39f46ab9@iki.fi

6 months agobufmgr/smgr: Don't cross segment boundaries in StartReadBuffers()
Andres Freund [Tue, 8 Oct 2024 15:37:45 +0000 (11:37 -0400)]
bufmgr/smgr: Don't cross segment boundaries in StartReadBuffers()

With real AIO it doesn't make sense to cross segment boundaries with one
IO. Add smgrmaxcombine() to allow upper layers to query which buffers can be
merged.

We could continue to cross segment boundaries when not using AIO, but it
doesn't really make sense, because md.c will never be able to perform the read
across the segment boundary in one system call. Which means we'll mark more
buffers as undergoing IO than really makes sense - if another backend desires
to read the same blocks, it'll be blocked longer than necessary. So it seems
better to just never cross the boundary.

Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/1f6b50a7-38ef-4d87-8246-786d39f46ab9@iki.fi

6 months agobufmgr: Return early in ScheduleBufferTagForWriteback() if fsync=off
Andres Freund [Tue, 8 Oct 2024 15:37:45 +0000 (11:37 -0400)]
bufmgr: Return early in ScheduleBufferTagForWriteback() if fsync=off

As pg_flush_data() doesn't do anything with fsync disabled, there's no point
in tracking the buffer for writeback. Arguably the better fix would be to
change pg_flush_data() to flush data even with fsync off, but that's a
behavioral change, whereas this is just a small optimization.

Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/1f6b50a7-38ef-4d87-8246-786d39f46ab9@iki.fi

6 months agoSilence buildfarm warning chatter from bd1276a3c.
Tom Lane [Tue, 8 Oct 2024 15:15:16 +0000 (11:15 -0400)]
Silence buildfarm warning chatter from bd1276a3c.

Buildfarm members using -Wextra complained about "warning: suggest
braces around empty body in an 'if' statement".  Do it gcc's way,
though I see no actual readability benefit in this.

6 months agoFix typo and run pgperltidy on newly-added test
Heikki Linnakangas [Tue, 8 Oct 2024 12:47:51 +0000 (15:47 +0300)]
Fix typo and run pgperltidy on newly-added test

From commit 85ec945b78.

6 months agoUse an shmem_exit callback to remove backend from PMChildFlags on exit
Heikki Linnakangas [Tue, 8 Oct 2024 12:06:34 +0000 (15:06 +0300)]
Use an shmem_exit callback to remove backend from PMChildFlags on exit

This seems nicer than having to duplicate the logic between
InitProcess() and ProcKill() for which child processes have a
PMChildFlags slot.

Move the MarkPostmasterChildActive() call earlier in InitProcess(),
out of the section protected by the spinlock.

Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/a102f15f-eac4-4ff2-af02-f9ff209ec66f@iki.fi

6 months agoAdd test for dead-end backends
Heikki Linnakangas [Tue, 8 Oct 2024 12:06:31 +0000 (15:06 +0300)]
Add test for dead-end backends

The code path for launching a dead-end backend because we're out of
slots was not covered by any tests, so add one. (Some tests did hit
the case of launching a dead-end backend because the server is still
starting up, though, so the gap in our test coverage wasn't as big as
it sounds.)

Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/a102f15f-eac4-4ff2-af02-f9ff209ec66f@iki.fi

6 months agoAdd test for connection limits
Heikki Linnakangas [Tue, 8 Oct 2024 12:06:26 +0000 (15:06 +0300)]
Add test for connection limits

Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/a102f15f-eac4-4ff2-af02-f9ff209ec66f@iki.fi

6 months agoDoc: add check to detect non-breaking spaces in the docs.
Tatsuo Ishii [Tue, 8 Oct 2024 11:25:18 +0000 (20:25 +0900)]
Doc: add check to detect non-breaking spaces in the docs.

There were multiple instances where accidentally adding non-breaking
space (nbsp, U+00A0, 0xc2a0 in UTF-8) to sgml files. This commit adds
additional checking to detect nbsp. You can check the nbsp by:

make -C doc/src/sgml check

or

make -C doc/src/sgml check-nbsp

Authors: Yugo Nagata, Daniel Gustafsson
Reviewed-by: Tatsuo Ishii, Daniel Gustafsson
Discussion: https://postgr.es/m/20240930.153404.202479334310259810.ishii%40postgresql.org

6 months agoMove check for binary mode and on_error option to the appropriate location.
Fujii Masao [Tue, 8 Oct 2024 09:23:43 +0000 (18:23 +0900)]
Move check for binary mode and on_error option to the appropriate location.

Commit 9e2d870119 placed the check for binary mode and on_error
before default values were inserted, which was not ideal.
This commit moves the check to a more appropriate position
after default values are set.

Additionally, the comment incorrectly mentioned two checks before
inserting defaults, when there are actually three. This commit corrects
that comment.

Author: Atsushi Torikoshi
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/8830518a-28ac-43a2-8a11-1676d9a3cdf8@oss.nttdata.com

6 months agoAdd REJECT_LIMIT option to the COPY command.
Fujii Masao [Tue, 8 Oct 2024 09:19:58 +0000 (18:19 +0900)]
Add REJECT_LIMIT option to the COPY command.

Previously, when ON_ERROR was set to 'ignore', the COPY command
would skip all rows with data type conversion errors, with no way to
limit the number of skipped rows before failing.

This commit introduces the REJECT_LIMIT option, allowing users to
specify the maximum number of erroneous rows that can be skipped.
If more rows encounter data type conversion errors than allowed by
REJECT_LIMIT, the COPY command will fail with an error, even when
ON_ERROR = 'ignore'.

Author: Atsushi Torikoshi
Reviewed-by: Junwang Zhao, Kirill Reshke, jian he, Fujii Masao
Discussion: https://postgr.es/m/63f99327aa6b404cc951217fa3e61fe4@oss.nttdata.com

6 months agoStabilize the test added by commit 022564f60c.
Amit Kapila [Tue, 8 Oct 2024 06:55:52 +0000 (12:25 +0530)]
Stabilize the test added by commit 022564f60c.

The test was unstable in branches 14 and 15 as we were relying on the
number of changes in the table having a toast column to start streaming.
On branches >= 16, we have a GUC debug_logical_replication_streaming which
can stream each change, so the test was stable in those branches.

Change the test to use PREPARE TRANSACTION as that should make the result
consistent and test the code changed in 022564f60c.

Reported-by: Daniel Gustafsson as per buildfarm
Author: Hou Zhijie, Amit Kapila
Backpatch-through: 14
Discussion: https://postgr.es/m/8C2F86AA-981E-4803-B14D-E264C0255330@yesql.se

7 months agoImprove style of two code paths
Michael Paquier [Tue, 8 Oct 2024 01:51:20 +0000 (10:51 +0900)]
Improve style of two code paths

In execGrouping.c, execTuplesMatchPrepare() was doing a memory
allocation that was not necessary when the number of columns was 0.
In foreign.c, pg_options_to_table() was assigning twice a variable to
the same value.

Author: Ranier Vilela
Discussion: https://postgr.es/m/CAEudQAqup0agbSzMjSLSTn=OANyCzxENF1+HrSYnr3WyZib7=Q@mail.gmail.com

7 months agoFix search_path cache initialization.
Jeff Davis [Tue, 8 Oct 2024 00:51:14 +0000 (17:51 -0700)]
Fix search_path cache initialization.

The cache needs to be available very early, so don't rely on
InitializeSearchPath() to initialize the it.

Reported-by: Murat Efendioğlu
Discussion: https://postgr.es/m/CACbCzujQ4zS8MM1bx-==+tr+D3Hk5G1cjN4XkUQ+Q=cEpwhzqg@mail.gmail.com
Backpatch-through: 17

7 months agoFix test for password hash length limit.
Nathan Bossart [Mon, 7 Oct 2024 22:17:39 +0000 (17:17 -0500)]
Fix test for password hash length limit.

In commit 8275325a06, I forgot to update password_1.out (an
alternative expected test output file added by commit 3c44e7d8d4),
so this test began failing on machines with FIPS mode enabled.

7 months agovacuumdb: Schema-qualify operator in catalog query's WHERE clause.
Nathan Bossart [Mon, 7 Oct 2024 21:49:20 +0000 (16:49 -0500)]
vacuumdb: Schema-qualify operator in catalog query's WHERE clause.

Commit 1ab67c9dfa, which modified this catalog query so that it
doesn't return temporary relations, forgot to schema-qualify the
operator.  A comment earlier in the function implores us to fully
qualify everything in the query:

 * Since we execute the constructed query with the default search_path
 * (which could be unsafe), everything in this query MUST be fully
 * qualified.

This commit fixes that.  While at it, add a newline for consistency
with surrounding code.

Reviewed-by: Noah Misch
Discussion: https://postgr.es/m/ZwQJYcuPPUsF0reU%40nathan
Backpatch-through: 12

7 months agoFix Y2038 issues with MyStartTime.
Nathan Bossart [Mon, 7 Oct 2024 18:51:03 +0000 (13:51 -0500)]
Fix Y2038 issues with MyStartTime.

Several places treat MyStartTime as a "long", which is only 32 bits
wide on some platforms.  In reality, MyStartTime is a pg_time_t,
i.e., a signed 64-bit integer.  This will lead to interesting bugs
on the aforementioned systems in 2038 when signed 32-bit integers
are no longer sufficient to store Unix time (e.g., "pg_ctl start"
hanging).  To fix, ensure that MyStartTime is handled as a 64-bit
value everywhere.  (Of course, users will need to ensure that
time_t is 64 bits wide on their system, too.)

Co-authored-by: Max Johnson
Discussion: https://postgr.es/m/CO1PR07MB905262E8AC270FAAACED66008D682%40CO1PR07MB9052.namprd07.prod.outlook.com
Backpatch-through: 12

7 months agoConvert tab-complete's long else-if chain to a switch statement.
Tom Lane [Mon, 7 Oct 2024 16:22:10 +0000 (12:22 -0400)]
Convert tab-complete's long else-if chain to a switch statement.

Rename tab-complete.c to tab-complete.in.c, create the preprocessor
script gen_tabcomplete.pl, and install Makefile/meson.build rules
to create tab-complete.c from tab-complete.in.c.  The preprocessor
converts match_previous_words' else-if chain into a switch and
populates tcpatterns[] with the data needed by the driver loop.

The initial HeadMatches/TailMatches/Matches test in each else-if arm
is now performed in a table-driven loop.  Where we get a match, the
corresponding switch case is invoked to see if the match succeeds.
(It might not, if there were additional conditions in the original
else-if test.)

The total number of string comparisons done is just about the
same as it was in the previous coding; however, now that we
have table-driven logic underlying the handmade rules, there
is room to improve that.  For now I haven't bothered because
tab completion is still plenty fast enough for human use.
If the number of rules keeps increasing, we might someday
need to do more in that area.

The immediate benefit of all this thrashing is that C compilers
frequently don't deal well with long else-if chains.  On gcc 8.5.0,
this reduces the compile time of tab-complete.c by about a factor of
four, while MSVC is reported to crash outright with the previous
coding.

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

7 months agoPrepare tab-complete.c for preprocessing.
Tom Lane [Mon, 7 Oct 2024 16:19:12 +0000 (12:19 -0400)]
Prepare tab-complete.c for preprocessing.

Separate out psql_completion's giant else-if chain of *Matches
tests into a new function.  Add the infrastructure needed for
table-driven checking of the initial match of each completion
rule.  As-is, however, the code continues to operate as it did.
The new behavior applies only if SWITCH_CONVERSION_APPLIED
is #defined, which it is not here.  (The preprocessor added
in the next patch will add a #define for that.)

The first and last couple of bits of psql_completion are not
based on HeadMatches/TailMatches/Matches tests, so they stay
where they are; they won't become part of the switch.

This patch also fixes up a couple of if-conditions that didn't meet
the conditions enumerated in the comment for match_previous_words().
Those restrictions exist to simplify the preprocessor.

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

7 months agoInvent "MatchAnyN" option for tab-complete.c's Matches/MatchesCS.
Tom Lane [Mon, 7 Oct 2024 16:13:02 +0000 (12:13 -0400)]
Invent "MatchAnyN" option for tab-complete.c's Matches/MatchesCS.

This argument matches any number (including zero) of previous words.
Use it to replace the common coding pattern

if (HeadMatches("A", "B") && TailMatches("X", "Y"))

with

if (Matches("A", "B", MatchAnyN, "X", "Y"))

In itself this feature doesn't do much except (arguably) make the
code slightly shorter and more readable.  However, it reduces the
number of complex if-condition patterns that have to be dealt with
in the next commits in this series.

While here, restructure the *Matches implementation functions so
that the actual work is done in functions that take a char **
array of pattern strings, and the versions taking variadic arguments
are thin wrappers around the array ones.  This simplifies the
new Matches logic considerably.  At the end of this patch series,
the array functions will be the only ones that are material to
performance, so having the variadic ones be wrappers makes sense.

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

7 months agoRestrict password hash length.
Nathan Bossart [Mon, 7 Oct 2024 15:56:16 +0000 (10:56 -0500)]
Restrict password hash length.

Commit 6aa44060a3 removed pg_authid's TOAST table because the only
varlena column is rolpassword, which cannot be de-TOASTed during
authentication because we haven't selected a database yet and
cannot read pg_class.  Since that change, attempts to set password
hashes that require out-of-line storage will fail with a "row is
too big" error.  This error message might be confusing to users.

This commit places a limit on the length of password hashes so that
attempts to set long password hashes will fail with a more
user-friendly error.  The chosen limit of 512 bytes should be
sufficient to avoid "row is too big" errors independent of BLCKSZ,
but it should also be lenient enough for all reasonable use-cases
(or at least all the use-cases we could imagine).

Reviewed-by: Tom Lane, Jonathan Katz, Michael Paquier, Jacob Champion
Discussion: https://postgr.es/m/89e8649c-eb74-db25-7945-6d6b23992394%40gmail.com

7 months agoFix fetching default toast value during decoding of in-progress transactions.
Amit Kapila [Mon, 7 Oct 2024 10:08:45 +0000 (15:38 +0530)]
Fix fetching default toast value during decoding of in-progress transactions.

During logical decoding of in-progress transactions, we perform the toast
table scan while fetching the default toast value for an attribute. We
forgot to initialize the flag during this scan to indicate that the system
table scan is in progress. We need this flag to ensure that during logical
decoding we never directly access the tableam or heap APIs because we check
for concurrent aborts only in systable_* APIs.

Reported-by: Alexander Lakhin
Author: Takeshi Ideriha, Hou Zhijie
Reviewed-by: Amit Kapila, Hou Zhijie
Backpatch-through: 14
Discussion: https://postgr.es/m/18641-6687273b7f15269d@postgresql.org

7 months agodoc: Quote value in SET NAMES documentation
Daniel Gustafsson [Mon, 7 Oct 2024 09:50:39 +0000 (11:50 +0200)]
doc: Quote value in SET NAMES documentation

The value passed to SET NAMES should be wrapped in single quotes.

Reported-by: jian he <jian.universality@gmail.com>
Discussion: https://postgr.es/m/CACJufxG3EoUsbX4ZoMFkWrvBJcSCbPjdpRvPhuQN65fADc3mFg@mail.gmail.com

7 months agodoc: Add minimal C and SQL example to add a custom table AM handler
Michael Paquier [Mon, 7 Oct 2024 06:47:40 +0000 (15:47 +0900)]
doc: Add minimal C and SQL example to add a custom table AM handler

The documentation was rather sparse on this matter and there is no
extension in-core that shows how to do it.  Adding a small example will
hopefully help newcomers.  An advantage of writing things this way is
that the contents are not going to rot because of backend changes.

Author: Phil Eaton
Reviewed-by: Robert Haas, Fabrízio de Royes Mello
Discussion: https://postgr.es/m/CAByiw+r+CS-ojBDP7Dm=9YeOLkZTXVnBmOe_ajK=en8C_zB3_g@mail.gmail.com

7 months agoUse camel case for "DateStyle" in some error messages
Michael Paquier [Mon, 7 Oct 2024 03:36:00 +0000 (12:36 +0900)]
Use camel case for "DateStyle" in some error messages

This GUC is written as camel-case in most of the documentation and the
GUC table (but not postgresql.conf.sample), and two error messages
hardcoded it with lower case characters.  Let's use a style more
consistent.

Most of the noise comes from the regression tests, updated to reflect
the GUC name in these error messages.

Author: Peter Smith
Reviewed-by: Peter Eisentraut, Álvaro Herrera
Discussion: https://postgr.es/m/CAHut+Pv-kSN8SkxSdoHano_wPubqcg5789ejhCDZAcLFceBR-w@mail.gmail.com

7 months agoIgnore not-yet-defined Portals in pg_cursors view.
Tom Lane [Sun, 6 Oct 2024 20:03:48 +0000 (16:03 -0400)]
Ignore not-yet-defined Portals in pg_cursors view.

pg_cursor() supposed that any Portal it finds in the hash table must
have sourceText set up, but there's an edge case where that is not so.
A newly-created Portal has sourceText = NULL, and that doesn't change
until PortalDefineQuery is called.  In SPI_cursor_open_internal,
we perform GetCachedPlan between CreatePortal and PortalDefineQuery,
and it's possible for user-defined code to execute during that
planning and cause a fetch from the pg_cursors view, resulting in a
null-pointer-dereference crash.  (It looks like the same could happen
in exec_bind_message, but I've not tried to provoke a failure there.)

I considered trying to fix this by setting sourceText sooner, but
there may be instances of this same calling pattern in extensions,
and we couldn't be sure they'd get the memo promptly.  It seems
better to redefine pg_cursor as not showing Portals that have
not yet had PortalDefineQuery called on them, which we can do by
just skipping them if sourceText is still NULL.

(Before a1c692358, pg_cursor would instead return a row with NULL
in the statement column.  We could revert to that behavior but it
doesn't really seem like a better definition, especially since our
documentation doesn't suggest that the column could be NULL.)

Per report from PetSerAl.  Back-patch to all supported branches.

Discussion: https://postgr.es/m/CAKygsHTBXLXjwV43kpZa+Cs+XTiaeeJiZdL4cPBm9f4MTdw7wg@mail.gmail.com

7 months agoMove Cluster.pm initialization code to a more obvious place
Andrew Dunstan [Sun, 6 Oct 2024 14:34:45 +0000 (10:34 -0400)]
Move Cluster.pm initialization code to a more obvious place

Commit 460c0076e8 added some module intialization code to set signal
handlers. However, that code has now become somewhat buried, as later
commits added new subroutines. Therefore, move the initialization code
to the module's INIT block where it won't become obscured.

7 months agolibpq: Discard leading and trailing spaces for parameters and values in URIs
Michael Paquier [Sun, 6 Oct 2024 09:23:02 +0000 (18:23 +0900)]
libpq: Discard leading and trailing spaces for parameters and values in URIs

Integer values applied a parsing rule through pqParseIntParam() that
made URIs like this one working, even if these include spaces around
values:
"postgresql://localhost:5432/postgres?keepalives=1 &keepalives_idle=1 "

This commit changes the parsing so as spaces before and after parameters
and values are discarded, offering more consistency with the parsing
that already applied to libpq for integer values in URIs.

Note that %20 can be used in a URI for a space character.  ECPGconnect()
has been discarded leading and trailing spaces around parameters and
values that for a long time, as well.  Like f22e84df1dea, this is done
as a HEAD-only change.

Reviewed-by: Yuto Sasaki
Discussion: https://postgr.es/m/Zv3oWOfcrHTph7JK@paquier.xyz

7 months agoUse generateClonedIndexStmt to propagate CREATE INDEX to partitions.
Tom Lane [Sat, 5 Oct 2024 18:46:44 +0000 (14:46 -0400)]
Use generateClonedIndexStmt to propagate CREATE INDEX to partitions.

When instantiating an existing partitioned index for a new child
partition, we use generateClonedIndexStmt to build a suitable
IndexStmt to pass to DefineIndex.  However, when DefineIndex needs
to recurse to instantiate a newly created partitioned index on an
existing child partition, it was doing copyObject on the given
IndexStmt and then applying a bunch of ad-hoc fixups.  This has
a number of problems, primarily that it implies fresh lookups of
referenced objects such as opclasses and collations.  Since commit
2af07e2f7 caused DefineIndex to restrict search_path internally, those
lookups could fail or deliver different results than the original one.
We can avoid those problems and save a few dozen lines of code by
using generateClonedIndexStmt in this code path too.

Another thing this fixes is incorrect propagation of parent-index
comments to child indexes (because the copyObject approach copies
the idxcomment field while generateClonedIndexStmt doesn't).  I had
noticed this in connection with commit c01eb619a, but not run the
problem to ground.

I'm tempted to back-patch this further than v17, but the only thing
it's known to fix in older branches is the comment issue, which is
pretty minor and doesn't seem worth the risk of introducing new
issues in stable branches.  (If anyone does care about that,
clearing idxcomment in the copied IndexStmt would be a safer fix.)

Per bug #18637 from usamoi.  Back-patch to v17 where the search_path
change came in.

Discussion: https://postgr.es/m/18637-f51e314546e3ba2a@postgresql.org

7 months agoClean up WaitLatch calls that passed latch without WL_LATCH_SET
Heikki Linnakangas [Sat, 5 Oct 2024 12:31:06 +0000 (15:31 +0300)]
Clean up WaitLatch calls that passed latch without WL_LATCH_SET

The 'latch' argument is ignored if WL_LATCH_SET is not given. Clarify
these calls by not pointlessly passing MyLatch.

Discussion: https://www.postgresql.org/message-id/391abe21-413e-4d91-a650-b663af49500c@iki.fi

7 months agoRemove unneeded #include
Heikki Linnakangas [Sat, 5 Oct 2024 12:09:32 +0000 (15:09 +0300)]
Remove unneeded #include

Unneeded since commit d72731a704.

Discussion: https://www.postgresql.org/message-id/391abe21-413e-4d91-a650-b663af49500c@iki.fi

7 months agoRemove unused latch
Heikki Linnakangas [Sat, 5 Oct 2024 12:09:27 +0000 (15:09 +0300)]
Remove unused latch

It was left unused by commit bc971f4025, which replaced the latch
usage with a condition variable

Discussion: https://www.postgresql.org/message-id/391abe21-413e-4d91-a650-b663af49500c@iki.fi

7 months agoReject non-ASCII locale names.
Thomas Munro [Sat, 5 Oct 2024 00:48:33 +0000 (13:48 +1300)]
Reject non-ASCII locale names.

Commit bf03cfd1 started scanning all available BCP 47 locale names on
Windows.  This caused an abort/crash in the Windows runtime library if
the default locale name contained non-ASCII characters, because of our
use of the setlocale() save/restore pattern with "char" strings.  After
switching to another locale with a different encoding, the saved name
could no longer be understood, and setlocale() would abort.

"Turkish_Türkiye.1254" is the example from recent reports, but there are
other examples of countries and languages with non-ASCII characters in
their names, and they appear in Windows' (old style) locale names.

To defend against this:

1.  In initdb, reject non-ASCII locale names given explicity on the
command line, or returned by the operating system environment with
setlocale(..., ""), or "canonicalized" by the operating system when we
set it.

2.  In initdb only, perform the save-and-restore with Windows'
non-standard wchar_t variant of setlocale(), so that it is not subject
to round trip failures stemming from char string encoding confusion.

3.  In the backend, we don't have to worry about the save-and-restore
problem because we have already vetted the defaults, so we just have to
make sure that CREATE DATABASE also rejects non-ASCII names in any new
databases.  SET lc_XXX doesn't suffer from the problem, but the ban
applies to it too because it uses check_locale().  CREATE COLLATION
doesn't suffer from the problem either, but it doesn't use
check_locale() so it is not included in the new ban for now, to minimize
the change.

Anyone who encounters the new error message should either create a new
duplicated locale with an ASCII-only name using Windows Locale Builder,
or consider using BCP 47 names like "tr-TR".  Users already couldn't
initialize a cluster with "Turkish_Türkiye.1254" on PostgreSQL 16+, but
the new failure mode is an error message that explains why, instead of a
crash.

Back-patch to 16, where bf03cfd1 landed.  Older versions are affected
in theory too, but only 16 and later are causing crash reports.

Reviewed-by: Andrew Dunstan <andrew@dunslane.net> (the idea, not the patch)
Reported-by: Haifang Wang (Centific Technologies Inc) <v-haiwang@microsoft.com>
Discussion: https://postgr.es/m/PH8PR21MB3902F334A3174C54058F792CE5182%40PH8PR21MB3902.namprd21.prod.outlook.com

7 months agoecpg: avoid adding whitespace around '&' in connection URLs.
Tom Lane [Fri, 4 Oct 2024 16:01:50 +0000 (12:01 -0400)]
ecpg: avoid adding whitespace around '&' in connection URLs.

The preprocessor really should not have done this to begin with.
The space after '&' was masked by ECPGconnect's skipping spaces
before option keywords, and the space before by dint of libpq
being (mostly) insensitive to trailing space in option values.
We fixed the one known problem with that in 920d51979.  Hence
this patch is mostly cosmetic, and we'll just change it in HEAD.

Discussion: https://postgr.es/m/TY2PR01MB36286A7B97B9A15793335D18C1772@TY2PR01MB3628.jpnprd01.prod.outlook.com

7 months agoRename PageData to GenericXLogPageData
Peter Eisentraut [Fri, 4 Oct 2024 10:47:35 +0000 (12:47 +0200)]
Rename PageData to GenericXLogPageData

In the PostgreSQL C type naming schema, the type PageData should be
what the pointer of type Page points to.  But in this case it's
actually an unrelated type local to generic_xlog.c.  Rename that to a
more specific name.  This makes room to possible add a PageData type
with the mentioned meaning, but this is not done here.

Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/001d457e-c118-4219-8132-e1846c2ae3c9%40eisentraut.org

7 months agoSpeed up numeric division by always using the "fast" algorithm.
Dean Rasheed [Fri, 4 Oct 2024 08:49:24 +0000 (09:49 +0100)]
Speed up numeric division by always using the "fast" algorithm.

Formerly there were two internal functions in numeric.c to perform
numeric division, div_var() and div_var_fast(). div_var() performed
division exactly to a specified rscale using Knuth's long division
algorithm, while div_var_fast() used the algorithm from the "FM"
library, which approximates each quotient digit using floating-point
arithmetic, and computes a truncated quotient with DIV_GUARD_DIGITS
extra digits. div_var_fast() could be many times faster than
div_var(), but did not guarantee correct results in all cases, and was
therefore only suitable for use in transcendental functions, where
small errors are acceptable.

This commit merges div_var() and div_var_fast() together into a single
function with an extra "exact" boolean parameter, which can be set to
false if the caller is OK with an approximate result. The new function
uses the faster algorithm from the "FM" library, except that when
"exact" is true, it does not truncate the computation with
DIV_GUARD_DIGITS extra digits, but instead performs the full-precision
computation, subtracting off complete multiples of the divisor for
each quotient digit. However, it is able to retain most of the
performance benefits of div_var_fast(), by delaying the propagation of
carries, allowing the inner loop to be auto-vectorized.

Since this may still lead to an inaccurate result, when "exact" is
true, it then inspects the remainder and uses that to adjust the
quotient, if necessary, to make it correct. In practice, the quotient
rarely needs to be adjusted, and never by more than one in the final
digit, though it's difficult to prove that, so the code allows for
larger adjustments, just in case.

In addition, use base-NBASE^2 arithmetic and a 64-bit dividend array,
similar to mul_var(), so that the number of iterations of the outer
loop is roughly halved. Together with the faster algorithm, this makes
div_var() up to around 20 times as fast as the old Knuth algorithm
when "exact" is true, and up to 2 or 3 times as fast as the old
div_var_fast() function when "exact" is false.

Dean Rasheed, reviewed by Joel Jacobson.

Discussion: https://postgr.es/m/CAEZATCVHR10BPDJSANh0u2+Sg6atO3mD0G+CjKDNRMD-C8hKzQ@mail.gmail.com

7 months agoRemove assertion checking query ID in execMain.c
Michael Paquier [Fri, 4 Oct 2024 03:51:17 +0000 (12:51 +0900)]
Remove assertion checking query ID in execMain.c

This assertion has been added by 24f520594809, but Alexander Lakhin has
proved that the ExecutorRun() one can be broken by using a PL function
that manipulates compute_query_id and track_activities, while the ones
in ExecutorFinish() and ExecutorEnd() could be triggered when cleaning
up portals at the beginning of a new query execution.

Discussion: https://postgr.es/m/b37d8e6c-e83d-e157-8865-1b2460a6aef2@gmail.com

7 months agoFix wrong varnullingrels error for MERGE WHEN NOT MATCHED BY SOURCE.
Dean Rasheed [Thu, 3 Oct 2024 12:48:32 +0000 (13:48 +0100)]
Fix wrong varnullingrels error for MERGE WHEN NOT MATCHED BY SOURCE.

If a MERGE command contains WHEN NOT MATCHED BY SOURCE actions, the
source relation appears on the outer side of the join. Thus, any Vars
referring to the source in the merge join condition, actions, and
RETURNING list should be marked as nullable by the join, since they
are used in the ModifyTable node above the join. Note that this only
applies to the copy of join condition used in the executor to
distinguish MATCHED from NOT MATCHED BY SOURCE cases. Vars in the
original join condition, inside the join node itself, should not be
marked.

Failure to correctly mark these Vars led to a "wrong varnullingrels"
error in the final stage of query planning, in some circumstances. We
happened to get away without this in all previous tests, since they
all involved a ModifyTable node directly on top of the join node, so
that the top plan targetlist coincided with the output of the join,
and the varnullingrels check was more lax. However, if another plan
node, such as a one-time filter Result node, gets inserted between the
ModifyTable node and the join node, then a stricter check is applied,
which fails.

Per bug #18634 from Alexander Lakhin. Thanks to Tom Lane and Richard
Guo for review and analysis.

Back-patch to v17, where WHEN NOT MATCHED BY SOURCE support was added
to MERGE.

Discussion: https://postgr.es/m/18634-db5299c937877f2b%40postgresql.org

7 months agoFix incorrect non-strict join recheck in MERGE WHEN NOT MATCHED BY SOURCE.
Dean Rasheed [Thu, 3 Oct 2024 11:53:03 +0000 (12:53 +0100)]
Fix incorrect non-strict join recheck in MERGE WHEN NOT MATCHED BY SOURCE.

If a MERGE command contains WHEN NOT MATCHED BY SOURCE actions, the
merge join condition is used by the executor to distinguish MATCHED
from NOT MATCHED BY SOURCE cases. However, this qual is executed using
the output from the join subplan node, which nulls the output from the
source relation in the not matched case, and so the result may be
incorrect if the join condition is "non-strict" -- for example,
something like "src.col IS NOT DISTINCT FROM tgt.col".

Fix this by enhancing the join recheck condition with an additional
"src IS NOT NULL" check, so that it does the right thing when
evaluated using the output from the join subplan.

Noted by Tom Lane while investigating bug #18634 from Alexander
Lakhin.

Back-patch to v17, where WHEN NOT MATCHED BY SOURCE support was added
to MERGE.

Discussion: https://postgr.es/m/18634-db5299c937877f2b%40postgresql.org

7 months agoReplace Unicode apostrophe with ASCII apostrophe
Amit Langote [Thu, 3 Oct 2024 10:51:38 +0000 (19:51 +0900)]
Replace Unicode apostrophe with ASCII apostrophe

In commit babb3993dbe9, I accidentally introduced a Unicode
apostrophe (U+2019). This commit replaces it with the ASCII
apostrophe (U+0027) for consistency.

Reported-by: Alexander Korotkov <aekorotkov@gmail.com>
Discussion: https://postgr.es/m/CAPpHfduNWMBjkJFtqXJremk6b6YQYO2s3_VEpnj-T_CaUNUYYQ@mail.gmail.com

7 months agoRefactor CopyFrom() in copyfrom.c.
Fujii Masao [Thu, 3 Oct 2024 06:59:16 +0000 (15:59 +0900)]
Refactor CopyFrom() in copyfrom.c.

This commit simplifies CopyFrom() by removing the unnecessary local variable
'skipped', which tracked the number of rows skipped due to on_error = 'ignore'.
That count is already handled by cstate->num_errors, so the 'skipped' variable
was redundant.

Additionally, the condition on_error != COPY_ON_ERROR_STOP is removed.
Since on_error == COPY_ON_ERROR_IGNORE is already checked, and on_error
only has two values (ignore and stop), the additional check was redundant
and made the logic harder to read. Seemingly this was introduced
in preparation for a future patch, but the current checks don’t offer
clear value and have been removed to improve readability.

Author: Atsushi Torikoshi
Reviewed-by: Masahiko Sawada, Fujii Masao
Discussion: https://postgr.es/m/ab59dad10490ea3734cf022b16c24cfd@oss.nttdata.com

7 months agofile_fdw: Add on_error and log_verbosity options to file_fdw.
Fujii Masao [Thu, 3 Oct 2024 06:57:32 +0000 (15:57 +0900)]
file_fdw: Add on_error and log_verbosity options to file_fdw.

In v17, the on_error and log_verbosity options were introduced for
the COPY command. This commit extends support for these options
to file_fdw.

Setting on_error = 'ignore' for a file_fdw foreign table allows users
to query it without errors, even when the input file contains
malformed rows, by skipping the problematic rows.

Both on_error and log_verbosity options apply to SELECT and ANALYZE
operations on file_fdw foreign tables.

Author: Atsushi Torikoshi
Reviewed-by: Masahiko Sawada, Fujii Masao
Discussion: https://postgr.es/m/ab59dad10490ea3734cf022b16c24cfd@oss.nttdata.com

7 months agoAdd log_verbosity = 'silent' support to COPY command.
Fujii Masao [Thu, 3 Oct 2024 06:55:37 +0000 (15:55 +0900)]
Add log_verbosity = 'silent' support to COPY command.

Previously, when the on_error option was set to ignore, the COPY command
would always log NOTICE messages for input rows discarded due to
data type incompatibility. Users had no way to suppress these messages.

This commit introduces a new log_verbosity setting, 'silent',
which prevents the COPY command from emitting NOTICE messages
when on_error = 'ignore' is used, even if rows are discarded.
This feature is particularly useful when processing malformed files
frequently, where a flood of NOTICE messages can be undesirable.

For example, when frequently loading malformed files via the COPY command
or querying foreign tables using file_fdw (with an upcoming patch to
add on_error support for file_fdw), users may prefer to suppress
these messages to reduce log noise and improve clarity.

Author: Atsushi Torikoshi
Reviewed-by: Masahiko Sawada, Fujii Masao
Discussion: https://postgr.es/m/ab59dad10490ea3734cf022b16c24cfd@oss.nttdata.com

7 months agoFix expression list handling in ATExecAttachPartition()
Amit Langote [Thu, 3 Oct 2024 02:59:09 +0000 (11:59 +0900)]
Fix expression list handling in ATExecAttachPartition()

This commit addresses two issues related to the manipulation of the
partition constraint expression list in ATExecAttachPartition().

First, the current use of list_concat() to combine the partition's
constraint (retrieved via get_qual_from_partbound()) with the parent
table’s partition constraint can lead to memory safety issues. After
calling list_concat(), the original constraint (partBoundConstraint)
might no longer be safe to access, as list_concat() may free or modify
it.

Second, there's a logical error in constructing the constraint for
validating against the default partition. The current approach
incorrectly includes a negated version of the parent table's partition
constraint, which is redundant, as it always evaluates to false for
rows in the default partition.

To resolve these issues, list_concat() is replaced with
list_concat_copy(), ensuring that partBoundConstraint remains unchanged
and can be safely reused when constructing the validation constraint
for the default partition.

This fix is not applied to back-branches, as there is no live bug and
the issue has not caused any reported problems in practice.

Nitin Jadhav posted a patch to address the memory safety issue, but I
decided to follow Alvaro Herrera's suggestion from the initial
discussion, as it allows us to fix both the memory safety and logical
issues.

Reported-by: Andres Freund <andres@anarazel.de>
Reported-by: Nitin Jadhav <nitinjadhavpostgres@gmail.com>
Reviewed-by: Junwang Zhao <zhjwpku@gmail.com>
Discussion: https://postgr.es/m/20231115165737.zeulb575cgrbqo74@awork3.anarazel.de
Discussion: https://postgr.es/m/CAMm1aWbmYHM3bqtjyMQ-a+4Ub=dgsb_2E3_up2cn=UGdHNrGTg@mail.gmail.com

7 months agoRemove support for unlogged on partitioned tables
Michael Paquier [Thu, 3 Oct 2024 01:55:02 +0000 (10:55 +0900)]
Remove support for unlogged on partitioned tables

The following commands were allowed on partitioned tables, with
different effects:
1) ALTER TABLE SET [UN]LOGGED did not issue an error, and did not update
pg_class.relpersistence.
2) CREATE UNLOGGED TABLE was working with pg_class.relpersistence marked
as initially defined, but partitions did not inherit the UNLOGGED
property, which was confusing.

This commit causes the commands mentioned above to fail for partitioned
tables, instead.

pg_dump is tweaked so as partitioned tables marked as UNLOGGED ignore
the option when dumped from older server versions.  pgbench needs a
tweak for --unlogged and --partitions=N to ignore the UNLOGGED option on
the partitioned tables created, its partitions still being unlogged.

Author: Michael Paquier
Reviewed-by: Nathan Bossart
Discussion: https://postgr.es/m/ZiiyGFTBNkqcMQi_@paquier.xyz

7 months agoAdjust json_manifest_per_file_callback API in one more place.
Tom Lane [Thu, 3 Oct 2024 00:27:45 +0000 (20:27 -0400)]
Adjust json_manifest_per_file_callback API in one more place.

Oversight in commit d94cf5ca7 (and in my testing of same).

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

7 months agoParse libpq's "keepalives" option more like other integer options.
Tom Lane [Wed, 2 Oct 2024 21:30:36 +0000 (17:30 -0400)]
Parse libpq's "keepalives" option more like other integer options.

Use pqParseIntParam (nee parse_int_param) instead of using strtol
directly.  This allows trailing whitespace, which the previous coding
didn't, and makes the spelling of the error message consistent with
other similar cases.

This seems to be an oversight in commit e7a221797, which introduced
parse_int_param.  That fixed places that were using atoi(), but missed
this place which was randomly using strtol() instead.

Ordinarily I'd consider this minor cleanup not worth back-patching.
However, it seems that ecpg assumes it can add trailing whitespace
to URL parameters, so that use of the keepalives option fails in
that context.  Perhaps that's worth improving as a separate matter.
In the meantime, back-patch this to all supported branches.

Yuto Sasaki (some further cleanup by me)

Discussion: https://postgr.es/m/TY2PR01MB36286A7B97B9A15793335D18C1772@TY2PR01MB3628.jpnprd01.prod.outlook.com

7 months agoFile size in a backup manifest should use uint64, not size_t.
Robert Haas [Wed, 2 Oct 2024 13:59:04 +0000 (09:59 -0400)]
File size in a backup manifest should use uint64, not size_t.

size_t is the size of an object in memory, not the size of a file on disk.

Thanks to Tom Lane for noting the error.

Discussion: http://postgr.es/m/1865585.1727803933@sss.pgh.pa.us

7 months agodoc: Missing markup, punctuation and wordsmithing
Daniel Gustafsson [Wed, 2 Oct 2024 12:50:56 +0000 (14:50 +0200)]
doc: Missing markup, punctuation and wordsmithing

Various improvements to the documentation like adding missing
markup, improving punctuation, ensuring consistent spelling of
words and minor wordsmithing.

Author: Oleg Sibiryakov <o.sibiryakov@postgrespro.ru>
Discussion: https://postgr.es/m/b7d0a03c-107e-48c7-a5c9-2c6f73cdf78f@postgrespro.ru

7 months agoAdd fastpaths for when no objects are found
Daniel Gustafsson [Wed, 2 Oct 2024 11:08:55 +0000 (13:08 +0200)]
Add fastpaths for when no objects are found

If there are no objects found, there is no reason to inspect the
result columns and mallocing a zero-sized  (which will be 1 byte
in reality) heap buffer for it.  Add a fast-path for immediately
returning like how other object inspection functions are already
doing it.

Reviewed-by: Ranier Vilela <ranier.vf@gmail.com>
Discussion: https://postgr.es/m/C2F05B3C-1414-45DD-AE09-6FEE4D0F89BD@yesql.se

7 months agoRemove superfluous PQExpBuffer resetting
Daniel Gustafsson [Wed, 2 Oct 2024 11:07:31 +0000 (13:07 +0200)]
Remove superfluous PQExpBuffer resetting

Since the buffer was just created, there is no reason to immediately
reset it.

Reviewed-by: Ranier Vilela <ranier.vf@gmail.com>
Discussion: https://postgr.es/m/C2F05B3C-1414-45DD-AE09-6FEE4D0F89BD@yesql.se

7 months agodoc: Add link to login event trigger example
Daniel Gustafsson [Wed, 2 Oct 2024 10:24:39 +0000 (12:24 +0200)]
doc: Add link to login event trigger example

The login event trigger is not listed on the trigger firing matrix
since it's not fired by a command.  Add a link to the example code
page similar to how the other event triggers link to the matrix.

Reported-by: Marcos Pegoraro <marcos@f10.com.br>
Discussion: https://postgr.es/m/CAB-JLwYS+78rX02BZ3wJ9ykVrd2i3O1K+7jzvZKQ0evquyQiLQ@mail.gmail.com

7 months agoFix inconsistent reporting of checkpointer stats.
Fujii Masao [Wed, 2 Oct 2024 02:17:47 +0000 (11:17 +0900)]
Fix inconsistent reporting of checkpointer stats.

Previously, the pg_stat_checkpointer view and the checkpoint completion
log message could show different numbers for buffers written
during checkpoints. The view only counted shared buffers,
while the log message included both shared and SLRU buffers,
causing inconsistencies.

This commit resolves the issue by updating both the view and the log message
to separately report shared and SLRU buffers written during checkpoints.
A new slru_written column is added to the pg_stat_checkpointer view
to track SLRU buffers, while the existing buffers_written column now
tracks only shared buffers. This change would help users distinguish
between the two types of buffers, in the pg_stat_checkpointer view and
the checkpoint complete log message, respectively.

Bump catalog version.

Author: Nitin Jadhav
Reviewed-by: Bharath Rupireddy, Michael Paquier, Kyotaro Horiguchi, Robert Haas
Reviewed-by: Andres Freund, vignesh C, Fujii Masao
Discussion: https://postgr.es/m/CAMm1aWb18EpT0whJrjG+-nyhNouXET6ZUw0pNYYAe+NezpvsAA@mail.gmail.com

7 months agodoc: Clarify name of files generated by pg_waldump --save-fullpage
Michael Paquier [Wed, 2 Oct 2024 02:12:40 +0000 (11:12 +0900)]
doc: Clarify name of files generated by pg_waldump --save-fullpage

The fork name is always separated with the block number by an underscore
in the names of the files generated, but the docs stuck them together
without a separator, which was confusing.

Author: Christoph Berg
Discussion: https://postgr.es/m/ZvxtSLiix9eceMRM@msg.df7cb.de
Backpatch-through: 16

7 months agoReject a copy EOF marker that has data ahead of it on the same line.
Tom Lane [Tue, 1 Oct 2024 20:53:54 +0000 (16:53 -0400)]
Reject a copy EOF marker that has data ahead of it on the same line.

We have always documented that a copy EOF marker (\.) must appear
by itself on a line, and that is how psql interprets the rule.
However, the backend's actual COPY FROM logic only insists that
there not be data between the \. and the following newline.
Any data ahead of the \. is parsed as a final line of input.
It's hard to interpret this as anything but an ancient mistake
that we've faithfully carried forward.  Continuing to allow it
is not cost-free, since it could mask client-side bugs that
unnecessarily backslash-escape periods (and thereby risk
accidentally creating an EOF marker).  So, let's remove that
provision and throw error if the EOF marker isn't alone on its
line, matching what the documentation has said right along.
Adjust the relevant error messages to be clearer, too.

Discussion: https://postgr.es/m/ed659f37-a9dd-42a7-82b9-0da562cc4006@manitou-mail.org

7 months agoinitdb: Add new option "--no-data-checksums"
Peter Eisentraut [Tue, 1 Oct 2024 14:27:39 +0000 (10:27 -0400)]
initdb: Add new option "--no-data-checksums"

Right now this does nothing except override any earlier
--data-checksums option.  But the idea is that --data-checksums could
become the default, and then this option would allow forcing it off
instead.

Author: Greg Sabino Mullane <greg@turnstep.com>
Discussion: https://www.postgresql.org/message-id/flat/CAKAnmmKwiMHik5AHmBEdf5vqzbOBbcwEPHo4-PioWeAbzwcTOQ@mail.gmail.com

7 months agoTweak docs to reduce possible impact of data checksums
Peter Eisentraut [Tue, 1 Oct 2024 13:58:20 +0000 (09:58 -0400)]
Tweak docs to reduce possible impact of data checksums

Author: Greg Sabino Mullane <greg@turnstep.com>
Discussion: https://www.postgresql.org/message-id/flat/CAKAnmmKwiMHik5AHmBEdf5vqzbOBbcwEPHo4-PioWeAbzwcTOQ@mail.gmail.com

7 months agoUse macro to define the number of enum values
Peter Eisentraut [Tue, 1 Oct 2024 13:30:24 +0000 (09:30 -0400)]
Use macro to define the number of enum values

Refactoring in the interest of code consistency, a follow-up to 2e068db56e31.

The argument against inserting a special enum value at the end of the enum
definition is that a switch statement might generate a compiler warning unless
it has a default clause.

Aleksander Alekseev, reviewed by Michael Paquier, Dean Rasheed, Peter Eisentraut

Discussion: https://postgr.es/m/CAJ7c6TMsiaV5urU_Pq6zJ2tXPDwk69-NKVh4AMN5XrRiM7N%2BGA%40mail.gmail.com

7 months agoFix some pg_verifybackup issues reported by Coverity.
Robert Haas [Tue, 1 Oct 2024 12:31:33 +0000 (08:31 -0400)]
Fix some pg_verifybackup issues reported by Coverity.

Commit 8dfd3129027969fdd2d9d294220c867d2efd84aa introduced a few
problems. verify_tar_file() forgot to free a buffer; the leak can't
add up to anything material, but might as well fix it.
precheck_tar_backup_file() intended to return after reporting an
error but didn't actually do so. member_copy_control_data() could
try to copy zero bytes (and maybe Coverity thinks it can even be
trying to copy a negative number of bytes).

Per discussion with Tom Lane.

Discussion: http://postgr.es/m/1240823.1727629418@sss.pgh.pa.us

7 months agoSimplify checking for xlocale.h
Peter Eisentraut [Tue, 1 Oct 2024 11:16:04 +0000 (07:16 -0400)]
Simplify checking for xlocale.h

Instead of XXX_IN_XLOCALE_H for several features XXX, let's just
include <xlocale.h> if HAVE_XLOCALE_H.  The reason for the extra
complication was apparently that some old glibc systems also had an
<xlocale.h>, and you weren't supposed to include it directly, but it's
gone now (as far as I can tell it was harmless to do so anyway).

Author: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/CWZBBRR6YA8D.8EHMDRGLCKCD%40neon.tech

7 months agojit: Use opaque pointers in all supported LLVM versions.
Peter Eisentraut [Tue, 1 Oct 2024 09:05:51 +0000 (05:05 -0400)]
jit: Use opaque pointers in all supported LLVM versions.

LLVM's opaque pointer change began in LLVM 14, but remained optional
until LLVM 16.  When commit 37d5babb added opaque pointer support, we
didn't turn it on for LLVM 14 and 15 yet because we didn't want to risk
weird bitcode incompatibility problems in released branches of
PostgreSQL.  (That might have been overly cautious, I don't know.)

Now that PostgreSQL 18 has dropped support for LLVM versions < 14, and
since it hasn't been released yet and no extensions or bitcode have been
built against it in the wild yet, we can be more aggressive.  We can rip
out the support code and build system clutter that made opaque pointer
use optional.

Author: Thomas Munro <thomas.munro@gmail.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Discussions: https://postgr.es/m/CA%2BhUKGLhNs5geZaVNj2EJ79Dx9W8fyWUU3HxcpZy55sMGcY%3DiA%40mail.gmail.com

7 months agojit: Require at least LLVM 14, if enabled.
Peter Eisentraut [Tue, 1 Oct 2024 08:49:11 +0000 (04:49 -0400)]
jit: Require at least LLVM 14, if enabled.

Remove support for LLVM versions 10-13.  The default on all non-EOL'd
OSes represented in our build farm will be at least LLVM 14 when
PostgreSQL 18 ships.

Author: Thomas Munro <thomas.munro@gmail.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/CA%2BhUKGLhNs5geZaVNj2EJ79Dx9W8fyWUU3HxcpZy55sMGcY%3DiA%40mail.gmail.com

7 months agodoc: Mention the connstring key word for PGSERVICE
Daniel Gustafsson [Tue, 1 Oct 2024 08:20:14 +0000 (10:20 +0200)]
doc: Mention the connstring key word for PGSERVICE

The documentation for the connection service file was mentioning
the environment variable early but not the connection string key
word until the last sentence and only then in an example.  This
adds the keyword in the first paragraph to make it clearer

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

7 months agoFix race condition in COMMIT PREPARED causing orphaned 2PC files
Michael Paquier [Tue, 1 Oct 2024 06:44:03 +0000 (15:44 +0900)]
Fix race condition in COMMIT PREPARED causing orphaned 2PC files

COMMIT PREPARED removes on-disk 2PC files near its end, but the state
checked if a file is on-disk or not gets read from shared memory while
not holding the two-phase state lock.

Because of that, there was a small window where a second backend doing a
PREPARE TRANSACTION could reuse the GlobalTransaction put back into the
2PC free list by the COMMIT PREPARED, overwriting the "ondisk" flag read
afterwards by the COMMIT PREPARED to decide if its on-disk two-phase
state file should be removed, preventing the file deletion.

This commit fixes this issue so as the "ondisk" flag in the
GlobalTransaction is read while holding the two-phase state lock, not
from shared memory after its entry has been added to the free list.

Orphaned two-phase state files flushed to disk after a checkpoint are
discarded at the beginning of recovery.  However, a truncation of
pg_xact/ would make the startup process issue a FATAL when it cannot
read the SLRU page holding the state of the transaction whose 2PC file
was orphaned, which is a necessary step to decide if the 2PC file should
be removed or not.  Removing manually the file would be necessary in
this case.

Issue introduced by effe7d9552dd, so backpatch all the way down.

Mea culpa.

Author: wuchengwen
Discussion: https://postgr.es/m/tencent_A7F059B5136A359625C7B2E4A386B3C3F007@qq.com
Backpatch-through: 12

7 months agoDoc: replace unnecessary non-breaking space with ordinal space.
Tatsuo Ishii [Tue, 1 Oct 2024 02:34:34 +0000 (11:34 +0900)]
Doc: replace unnecessary non-breaking space with ordinal space.

There were unnecessary non-breaking spaces (nbsp, U+00A0, 0xc2a0 in
UTF-8) in the docs.  This commit replaces them with ASCII spaces
(0x20).

config.sgml is backpatched through 17.
ref/drop_extension.sgml is backpatched through 13.

Discussion: https://postgr.es/m/20240930.153404.202479334310259810.ishii%40postgresql.org
Reviewed-by: Yugo Nagata, Daniel Gustafsson
Backpatch-through: 17, 13

7 months agoExpand assertion check for query ID reporting in executor
Michael Paquier [Mon, 30 Sep 2024 23:56:21 +0000 (08:56 +0900)]
Expand assertion check for query ID reporting in executor

As formulated, the assertion added in the executor by 24f520594809 to
check that a query ID is set had two problems:
- track_activities may be disabled while compute_query_id is enabled,
causing the query ID to not be reported to pg_stat_activity.
- debug_query_string may not be set in some context.  The only path
where this would matter is visibly autovacuum, should parallel workers
be enabled there at some point.  This is not the case currently.

There was no test showing the interactions between the query ID and
track_activities, so let's add one based on a scan of pg_stat_activity.
This assertion is still an experimentation at this stage, but let's see
if this shows more paths where query IDs are not properly set while they
should.

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

7 months agoAdd missing command for pg_maintain in comment
Daniel Gustafsson [Mon, 30 Sep 2024 22:01:32 +0000 (00:01 +0200)]
Add missing command for pg_maintain in comment

The comment in pg_class_aclmask_ext() which lists the allowed commands
for the pg_maintain role lacked LOCK TABLE.

Reported-by: Yusuke Sugie <btsugieyuusuke@oss.nttdata.com>
Reviewed-by: Yugo Nagata <nagata@sraoss.co.jp>
Discussion: https://postgr.es/m/034d3c60f5daba1919cd90f236b2e22d@oss.nttdata.com

7 months agoDo not treat \. as an EOF marker in CSV mode for COPY IN.
Tom Lane [Mon, 30 Sep 2024 21:57:12 +0000 (17:57 -0400)]
Do not treat \. as an EOF marker in CSV mode for COPY IN.

Since backslash is (typically) not special in CSV data, we should
not be treating \. as special either.  The server historically did
this to keep CSV and TEXT modes more alike and to support V2 protocol;
but V2 protocol is long dead, and the inconsistency with CSV standards
is annoying.  Remove that behavior in CopyReadLineText, and make some
minor consequent code simplifications.

On the client side, we need to fix psql so that it does not check
for \. except when reading data from STDIN (that is, the script
source).  We must do that regardless of TEXT/CSV mode or there is
no way to end the COPY short of script EOF.  Also, be careful
not to send the \. to the server in that case.

This is a small compatibility break in that other applications
beside psql may need similar adjustment.  Also, using an older
version of psql with a v18 server may result in misbehavior
during CSV-mode COPY IN.

Daniel Vérité, reviewed by vignesh C, Robert Haas, and myself

Discussion: https://postgr.es/m/ed659f37-a9dd-42a7-82b9-0da562cc4006@manitou-mail.org

7 months agodocs: Enhance the pg_stat_checkpointer view documentation.
Fujii Masao [Mon, 30 Sep 2024 16:55:45 +0000 (01:55 +0900)]
docs: Enhance the pg_stat_checkpointer view documentation.

This commit updates the documentation for the pg_stat_checkpointer view
to clarify what kind of checkpoints or restartpoints each counter tracks.
This makes it easier to understand the meaning of each counter.

Previously, the num_requested description included "backend,"
which could be misleading since requests come from other sources as well.
This commit also removes "backend" from the description of num_requested,
to avoid confusion.

Author: Fujii Masao
Reviewed-by: Anton A. Melnikov
Discussion: https://postgr.es/m/4640258e-d959-4cf0-903c-cd02389c3e05@oss.nttdata.com

7 months agoRemove incorrect entries in pg_walsummary's getopt_long call.
Tom Lane [Mon, 30 Sep 2024 16:06:54 +0000 (12:06 -0400)]
Remove incorrect entries in pg_walsummary's getopt_long call.

For some reason this listed "-f" and "-w" as valid switches, though
the code doesn't implement any such thing nor do the docs mention
them.  The effect of this was that if you tried to use one of these
switches, you'd get an unhelpful error message.

Yusuke Sugie

Discussion: https://postgr.es/m/68e72a2a70f4d84c1c7847b13bcdaef8@oss.nttdata.com

7 months agoDon't disallow DROP of constraints ONLY on partitioned tables
Alvaro Herrera [Mon, 30 Sep 2024 09:58:13 +0000 (11:58 +0200)]
Don't disallow DROP of constraints ONLY on partitioned tables

This restriction seems to have come about due to some fuzzy thinking: in
commit 9139aa19423b we were adding a restriction against ADD constraint
ONLY on partitioned tables (which is sensible) and apparently we thought
the DROP case had to be symmetrical.  However, it isn't, and the
comments about it are mistaken about the effect it would have.  Remove
this limitation.

There have been no reports of users bothered by this limitation, so I'm
not backpatching it just yet.  We can revisit this decision later, as needed.

Reviewed-by: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/202409261752.nbvlawkxsttf@alvherre.pgsql
Discussion: https://postgr.es/m/7682253a-6f79-6a92-00aa-267c4c412870@lab.ntt.co.jp
(about commit 9139aa19423b, previously not registered)

7 months agoBump catalog version for change in VariableSetStmt
Michael Paquier [Mon, 30 Sep 2024 05:52:03 +0000 (14:52 +0900)]
Bump catalog version for change in VariableSetStmt

Oversight in dc68515968e8, as this breaks SQL functions with a SET
command.

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

7 months agoShow values of SET statements as constants in pg_stat_statements
Michael Paquier [Mon, 30 Sep 2024 05:02:00 +0000 (14:02 +0900)]
Show values of SET statements as constants in pg_stat_statements

This is a continuation of work like 11c34b342bd7, done to reduce the
bloat of pg_stat_statements by applying more normalization to query
entries.  This commit is able to detect and normalize values in
VariableSetStmt, resulting in:
SET conf_param = $1

Compared to other parse nodes, VariableSetStmt is embedded in much more
places in the parser, impacting many query patterns in
pg_stat_statements.  A custom jumble function is used, with an extra
field in the node to decide if arguments should be included in the
jumbling or not, a location field being not enough for this purpose.
This approach allows for a finer tuning.

Clauses relying on one or more keywords are not normalized, for example:
* DEFAULT
* FROM CURRENT
* List of keywords.  SET SESSION CHARACTERISTICS AS TRANSACTION,
where it is critical to differentiate different sets of options, is a
good example of why normalization should not happen.

Some queries use VariableSetStmt for some subclauses with SET, that also
have their values normalized:
- ALTER DATABASE
- ALTER ROLE
- ALTER SYSTEM
- CREATE/ALTER FUNCTION

ba90eac7a995 has added test coverage for most of the existing SET
patterns.  The expected output of these tests shows the difference this
commit creates.  Normalization could be perhaps applied to more portions
of the grammar but what is done here is conservative, and good enough as
a starting point.

Author: Greg Sabino Mullane, Michael Paquier
Discussion: https://postgr.es/m/36e5bffe-e989-194f-85c8-06e7bc88e6f7@amazon.com
Discussion: https://postgr.es/m/B44FA29D-EBD0-4DD9-ABC2-16F1CB087074@amazon.com
Discussion: https://postgr.es/m/CAKAnmmJtJY2jzQN91=2QAD2eAJAA-Per61eyO48-TyxEg-q0Rg@mail.gmail.com

7 months agoAdd num_done counter to the pg_stat_checkpointer view.
Fujii Masao [Mon, 30 Sep 2024 02:56:05 +0000 (11:56 +0900)]
Add num_done counter to the pg_stat_checkpointer view.

Checkpoints can be skipped when the server is idle. The existing num_timed and
num_requested counters in pg_stat_checkpointer track both completed and
skipped checkpoints, but there was no way to count only the completed ones.

This commit introduces the num_done counter, which tracks only completed
checkpoints, making it easier to see how many were actually performed.

Bump catalog version.

Author: Anton A. Melnikov
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/9ea77f40-818d-4841-9dee-158ac8f6e690@oss.nttdata.com

7 months agoreindexdb: Skip reindexing temporary tables and indexes.
Fujii Masao [Mon, 30 Sep 2024 02:13:55 +0000 (11:13 +0900)]
reindexdb: Skip reindexing temporary tables and indexes.

Reindexing temp tables or indexes of other sessions is not allowed.
However, reindexdb in parallel mode previously listed them as
the objects to process, leading to failures.

This commit ensures reindexdb in parallel mode skips temporary tables
and indexes by adding a condition based on the relpersistence column
in pg_class to the object listing queries, preventing these issues.

Note that this commit does not affect reindexdb when temporary tables
or indexes are explicitly specified using the -t or -j options;
reindexdb in that case still does not skip them and can cause an error.

Back-patch to v13 where parallel mode was introduced in reindexdb.

Author: Fujii Masao
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/5f37ee56-14fb-44fe-9150-9eb97e10538b@oss.nttdata.com

7 months agoSet query ID in parallel workers for vacuum, BRIN and btree
Michael Paquier [Sun, 29 Sep 2024 23:43:28 +0000 (08:43 +0900)]
Set query ID in parallel workers for vacuum, BRIN and btree

All these code paths use their own entry point when starting parallel
workers, but failed to set a query ID, even if they set a text query.
Hence, this data would be missed in pg_stat_activity for the worker
processes.  The main entry point for parallel query processing,
ParallelQueryMain(), is already doing that by saving its query ID in a
dummy PlannedStmt, but not the others.  The code is changed so as the
query ID of these queries is set in their shared state, and reported
back once the parallel workers start.

Some tests are added to show how the failures can happen for btree and
BRIN with a parallel build enforced, which are able to trigger a failure
in an assertion added by 24f520594809 in the recovery TAP test
027_stream_regress.pl where pg_stat_statements is always loaded.  In
this case, the executor path was taken because the index expression
needs to be flattened when building its IndexInfo.

Alexander Lakhin has noticed the problem in btree, and I have noticed
that the issue was more spread.  This is arguably a bug, but nobody has
complained about that until now, so no backpatch is done out of caution.
If folks would like to see a backpatch, well, let me know.

Reported-by: Alexander Lakhin
Reviewed-by: Sami Imseih
Discussion: https://postgr.es/m/cf3547c1-498a-6a61-7b01-819f902a251f@gmail.com

7 months agoRemove NULL dereference from RenameRelationInternal().
Noah Misch [Sun, 29 Sep 2024 22:54:25 +0000 (15:54 -0700)]
Remove NULL dereference from RenameRelationInternal().

Defect in last week's commit aac2c9b4fde889d13f859c233c2523345e72d32b,
per Coverity.  Reaching this would need catalog corruption.  Back-patch
to v12, like that commit.

7 months agoIn passwordFromFile, don't leak the open file after stat failures.
Tom Lane [Sun, 29 Sep 2024 17:40:03 +0000 (13:40 -0400)]
In passwordFromFile, don't leak the open file after stat failures.

Oversight in e882bcae0.  Per Coverity.

7 months agoAvoid 037_invalid_database.pl hang under debug_discard_caches.
Noah Misch [Fri, 27 Sep 2024 22:28:56 +0000 (15:28 -0700)]
Avoid 037_invalid_database.pl hang under debug_discard_caches.

Back-patch to v12 (all supported versions).

7 months agodoc: Note that CREATE MATERIALIZED VIEW restricts search_path.
Nathan Bossart [Fri, 27 Sep 2024 21:21:21 +0000 (16:21 -0500)]
doc: Note that CREATE MATERIALIZED VIEW restricts search_path.

Since v17, CREATE MATERIALIZED VIEW has set search_path to
"pg_catalog, pg_temp" while running the query.  The docs for the
other commands that restrict search_path mention it, but the page
for CREATE MATERIALIZED VIEW does not.  Fix that.

Oversight in commit 4b74ebf726.

Author: Yugo Nagata
Reviewed-by: Jeff Davis
Discussion: https://postgr.es/m/20240805160502.d2a4975802a832b1e04afb80%40sraoss.co.jp
Backpatch-through: 17

7 months agoRecalculate where-needed data accurately after a join removal.
Tom Lane [Fri, 27 Sep 2024 20:04:04 +0000 (16:04 -0400)]
Recalculate where-needed data accurately after a join removal.

Up to now, remove_rel_from_query() has done a pretty shoddy job
of updating our where-needed bitmaps (per-Var attr_needed and
per-PlaceHolderVar ph_needed relid sets).  It removed direct mentions
of the to-be-removed baserel and outer join, which is the minimum
amount of effort needed to keep the data structures self-consistent.
But it didn't account for the fact that the removed join ON clause
probably mentioned Vars of other relations, and those Vars might now
not be needed as high up in the join tree as before.  It's easy to
show cases where this results in failing to remove a lower outer join
that could also have been removed.

To fix, recalculate the where-needed bitmaps from scratch after
each successful join removal.  This sounds expensive, but it seems
to add only negligible planner runtime.  (We cheat a little bit
by preserving "relation 0" entries in the bitmaps, allowing us to
skip re-scanning the targetlist and HAVING qual.)

The submitted test case drew attention because we had successfully
optimized away the lower join prior to v16.  I suspect that that's
somewhat accidental and there are related cases that were never
optimized before and now can be.  I've not tried to come up with
one, though.

Perhaps we should back-patch this into v16 and v17 to repair the
performance regression.  However, since it took a year for anyone
to notice the problem, it can't be affecting too many people.  Let's
let the patch bake awhile in HEAD, and see if we get more complaints.

Per bug #18627 from Mikaël Gourlaouen.  No back-patch for now.

Discussion: https://postgr.es/m/18627-44f950eb6a8416c2@postgresql.org

7 months agoReindent pg_verifybackup.c.
Robert Haas [Fri, 27 Sep 2024 15:14:31 +0000 (11:14 -0400)]
Reindent pg_verifybackup.c.

7 months agopg_verifybackup: Verify tar-format backups.
Robert Haas [Fri, 27 Sep 2024 12:40:24 +0000 (08:40 -0400)]
pg_verifybackup: Verify tar-format backups.

This also works for compressed tar-format backups. However, -n must be
used, because we use pg_waldump to verify WAL, and it doesn't yet know
how to verify WAL that is stored inside of a tarfile.

Amul Sul, reviewed by Sravan Kumar and by me, and revised by me.

7 months agoFix typo in pg_walsummary/nls.mk.
Fujii Masao [Fri, 27 Sep 2024 01:20:22 +0000 (10:20 +0900)]
Fix typo in pg_walsummary/nls.mk.

Author: Koki Nakamura
Discussion: https://postgr.es/m/485c613d1db8de2e8169d5afd43e7f9e@oss.nttdata.com

7 months agoFix incorrect memory access in VACUUM FULL with invalid toast indexes
Michael Paquier [Fri, 27 Sep 2024 00:40:09 +0000 (09:40 +0900)]
Fix incorrect memory access in VACUUM FULL with invalid toast indexes

An invalid toast index is skipped in reindex_relation().  These would be
remnants of a failed REINDEX CONCURRENTLY and they should never been
rebuilt as there can only be one valid toast index at a time.

REINDEX_REL_SUPPRESS_INDEX_USE, used by CLUSTER and VACUUM FULL, needs
to maintain a list of the indexes being processed.  The list of indexes
is retrieved from the relation cache, and includes invalid indexes.  The
code has missed that invalid toast indexes are ignored in
reindex_relation() as this leads to a hard failure in reindex_index(),
and they were left in the reindex pending list, making the list
inconsistent when rechecked.  The incorrect memory access was happening
when scanning pg_class for the refresh of pg_database.datfrozenxid, when
doing a scan of pg_class.

This issue exists since REINDEX CONCURRENTLY exists, where invalid toast
indexes can exist, so backpatch all the way down.

Reported-by: Alexander Lakhin
Author: Tender Wang
Discussion: https://postgr.es/m/18630-9aed99c38830657d@postgresql.org
Backpatch-through: 12

7 months agoFix catalog data of new LO privilege functions
Michael Paquier [Thu, 26 Sep 2024 22:26:29 +0000 (07:26 +0900)]
Fix catalog data of new LO privilege functions

This commit improves the catalog data in pg_proc for the three functions
for has_largeobject_privilege(), introduced in 4eada203a5a8:
- Fix their descriptions (typos and consistency).
- Reallocate OIDs to be within the 8000-9999 range as required by
a6417078c414.

Bump catalog version.

Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/ZvUYR0V0dzWaLnsV@paquier.xyz

7 months agoEnsure we have a snapshot when updating pg_index entries.
Nathan Bossart [Thu, 26 Sep 2024 20:51:23 +0000 (15:51 -0500)]
Ensure we have a snapshot when updating pg_index entries.

Creating, reindexing, and dropping an index concurrently could
entail accessing pg_index's TOAST table, which was recently added
in commit b52c4fc3c0.  These code paths start and commit their own
transactions, but they do not always set an active snapshot.  This
rightfully leads to assertion failures and ERRORs when trying to
access pg_index's TOAST table, such as the following:

ERROR:  cannot fetch toast data without an active snapshot

To fix, push an active snapshot just before each section of code
that might require accessing pg_index's TOAST table, and pop it
shortly afterwards.

Reported-by: Alexander Lakhin
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/a97d7401-e7c9-f771-6a00-037379f0a8bb%40gmail.com

7 months agoImprove style of pg_upgrade task callback functions.
Nathan Bossart [Thu, 26 Sep 2024 18:54:37 +0000 (13:54 -0500)]
Improve style of pg_upgrade task callback functions.

I wanted to avoid adjusting this code too much when converting
these tasks to use the new parallelization framework (see commit
40e2e5e92b), which is why this is being done as a follow-up commit.
These stylistic adjustments result in fewer lines of code and fewer
levels of indentation in some places.

While at it, add names to the UpgradeTaskSlotState enum and the
UpgradeTaskSlot struct.  I'm not aware of any established project
policy in this area, but let's at least be consistent within the
same file.

Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/ZunW7XHLd2uTts4f%40nathan

7 months agoModernize to_char's Roman-numeral code, fixing overflow problems.
Tom Lane [Thu, 26 Sep 2024 15:02:31 +0000 (11:02 -0400)]
Modernize to_char's Roman-numeral code, fixing overflow problems.

int_to_roman() only accepts plain "int" input, which is fine since
we're going to produce '###############' for any value above 3999
anyway.  However, the numeric and int8 variants of to_char() would
throw an error if the given input exceeded the integer range, while
the float-input variants invoked undefined-per-C-standard behavior.
Fix things so that you uniformly get '###############' for out of
range input.

Also add test cases covering this code, plus the equally-untested
EEEE, V, and PL format codes.

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

7 months agoDoc: InitPlans aren't parallel-restricted any more.
Tom Lane [Thu, 26 Sep 2024 14:37:51 +0000 (10:37 -0400)]
Doc: InitPlans aren't parallel-restricted any more.

Commit e08d74ca1 removed that restriction, but missed updating
the documentation about it.  Noted by Egor Rogov.

Discussion: https://postgr.es/m/cdc8f87b-a378-4e22-6d29-40ae32dd97d1@postgrespro.ru

7 months agoDoc: Add a note in the upgrade of logical replication clusters.
Amit Kapila [Thu, 26 Sep 2024 10:44:07 +0000 (16:14 +0530)]
Doc: Add a note in the upgrade of logical replication clusters.

The steps used to upgrade the cluster first upgraded the publisher node
but ideally, any node could be upgraded first.

Author: Vignesh C
Discussion: https://postgr.es/m/CALDaNm1_iDO6srWzntqTr0ZDVkk2whVhNKEWAvtgZBfSmuBeZQ@mail.gmail.com
Discussion: https://postgr.es/m/CALDaNm3Y-M+kAqr_mf=_C1kNwAB-cS6S5hTHnKMEqDw4sGEh4Q@mail.gmail.com

7 months agoUpdate oid for pg_wal_replay_wait() procedure
Alexander Korotkov [Thu, 26 Sep 2024 08:48:23 +0000 (11:48 +0300)]
Update oid for pg_wal_replay_wait() procedure

Use an oid from 8000-9999 range, as required by 98eab30b93d5.

Reported-by: Michael Paquier
Discussion: https://postgr.es/m/ZvUY6bfTwB0GsyzP%40paquier.xyz

7 months agoRemove extra whitespace in pg_upgrade status message.
Nathan Bossart [Wed, 25 Sep 2024 16:18:56 +0000 (11:18 -0500)]
Remove extra whitespace in pg_upgrade status message.

There's no need to add another level of indentation to this status
message.  pg_log() will put it in the right place.

Oversight in commit 347758b120.

Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/ZunW7XHLd2uTts4f%40nathan
Backpatch-through: 17