postgresql.git
4 months agodoc: Mention BRIN indexes support parallel builds
Tomas Vondra [Mon, 16 Dec 2024 18:08:52 +0000 (19:08 +0100)]
doc: Mention BRIN indexes support parallel builds

Two places in the documentation suggest B-tree is the only index access
method allowing parallel builds. Commit b4375717 added parallel builds
for BRIN too, but failed to update the docs. So fix that, and backpatch
to 17, where parallel BRIN builds were introduced.

Author: Egor Rogov
Backpatch-through: 17
Discussion: https://postgr.es/m/114e2d5d-125e-07d8-94aa-5ad175fb7443@postgrespro.ru

4 months agopsql: Tab completion for JOIN ... USING column list
Tomas Vondra [Mon, 16 Dec 2024 17:12:29 +0000 (18:12 +0100)]
psql: Tab completion for JOIN ... USING column list

For JOIN ... USING, offer attribute names for the first member of the
column list.

Author: Andreas Karlsson
Reviewed-By: Tomas Vondra
Discussion: https://postgr.es/m/3a7e27bc-d6ed-4cb0-9b21-f21143fc1b37@proxel.se

4 months agopsql: Tab completion for JOIN ... ON/USING
Tomas Vondra [Mon, 16 Dec 2024 17:08:30 +0000 (18:08 +0100)]
psql: Tab completion for JOIN ... ON/USING

Offer ON/USING clauses for join types that require join conditions (i.e.
anything except for NATURAL/CROSS joins).

Author: Andreas Karlsson
Reviewed-By: Tomas Vondra
Discussion: https://postgr.es/m/3a7e27bc-d6ed-4cb0-9b21-f21143fc1b37@proxel.se

4 months agopsql: Tab completion for LATERAL joins
Tomas Vondra [Mon, 16 Dec 2024 16:55:00 +0000 (17:55 +0100)]
psql: Tab completion for LATERAL joins

When listing selectable objects after a JOIN, offer also LATERAL.

Author: Andreas Karlsson
Reviewed-By: Tomas Vondra
Discussion: https://postgr.es/m/3a7e27bc-d6ed-4cb0-9b21-f21143fc1b37@proxel.se

4 months agoRefactor string case conversion into provider-specific files.
Jeff Davis [Mon, 16 Dec 2024 17:35:18 +0000 (09:35 -0800)]
Refactor string case conversion into provider-specific files.

Create API entry points pg_strlower(), etc., that work with any
provider and give the caller control over the destination
buffer. Then, move provider-specific logic into pg_locale_builtin.c,
pg_locale_icu.c, and pg_locale_libc.c as appropriate.

Discussion: https://postgr.es/m/7aa46d77b377428058403723440862d12a8a129a.camel@j-davis.com

4 months agopsql: Tab completion for CREATE MATERIALIZED VIEW ... USING
Tomas Vondra [Mon, 16 Dec 2024 15:46:56 +0000 (16:46 +0100)]
psql: Tab completion for CREATE MATERIALIZED VIEW ... USING

The tab completion didn't offer USING for CREATE MATERIALIZED VIEW, so
add it, and offer a list of access methods, followed by SELECT.

Author: Kirill Reshke
Reviewed-By: Karina Litskevich
Discussion: https://postgr.es/m/CALdSSPhVELkvutquqrDB=Ujfq_Pjz=6jn-kzh+291KPNViLTfw@mail.gmail.com

4 months agopsql: Tab completion for CREATE TEMP TABLE ... USING
Tomas Vondra [Mon, 16 Dec 2024 15:38:35 +0000 (16:38 +0100)]
psql: Tab completion for CREATE TEMP TABLE ... USING

The USING keyword was offered only for persistent tables, not for
temporary ones. So improve that.

Author: Kirill Reshke
Reviewed-By: Karina Litskevich
Discussion: https://postgr.es/m/CALdSSPhVELkvutquqrDB=Ujfq_Pjz=6jn-kzh+291KPNViLTfw@mail.gmail.com

4 months agopsql: Tab completion for ALTER TYPE ... CASCADE/RESTRICT
Tomas Vondra [Mon, 16 Dec 2024 15:20:04 +0000 (16:20 +0100)]
psql: Tab completion for ALTER TYPE ... CASCADE/RESTRICT

Updates table completion for ALTER TYPE to offer CASCADE/RESTRICT for a
number of actions on attributes:

    ALTER TYPE ... ADD/DROP/RENAME ATTRIBUTE ... [CASCADE|RESTRICT]
    ALTER TYPE ... TYPE ... [CASCADE|RESTRICT]

Author: Kirill Reshke
Reviewed-By: Karina Litskevich
Discussion: https://postgr.es/m/CALdSSPhVELkvutquqrDB=Ujfq_Pjz=6jn-kzh+291KPNViLTfw@mail.gmail.com

4 months agopsql: Tab completion for ALTER TYPE ... ADD ATTRIBUTE
Tomas Vondra [Mon, 16 Dec 2024 14:53:36 +0000 (15:53 +0100)]
psql: Tab completion for ALTER TYPE ... ADD ATTRIBUTE

Improve psql tab completion for ALTER TYPE ... ADD ATTRIBUTE to offer a
list of existing data types (until now no options were offered).

Author: Kirill Reshke
Reviewed-By: Karina Litskevich
Discussion: https://postgr.es/m/CALdSSPhVELkvutquqrDB=Ujfq_Pjz=6jn-kzh+291KPNViLTfw@mail.gmail.com

4 months agoMake 009_twophase.pl test pass with recovery_min_apply_delay set
Heikki Linnakangas [Mon, 16 Dec 2024 13:56:38 +0000 (15:56 +0200)]
Make 009_twophase.pl test pass with recovery_min_apply_delay set

The test failed if you ran the regression tests with TEMP_CONFIG with
recovery_min_apply_delay = '500ms'. Fix the race condition by waiting
for transaction to be applied in the replica, like in a few other
tests.

The failing test was introduced in commit cbfbda7841. Backpatch to all
supported versions like that commit (except v12, which is no longer
supported).

Reported-by: Alexander Lakhin
Discussion: https://www.postgresql.org/message-id/09e2a70a-a6c2-4b5c-aeae-040a7449c9f2@gmail.com

4 months agoPrint out error position for CREATE DOMAIN
Michael Paquier [Mon, 16 Dec 2024 05:52:11 +0000 (14:52 +0900)]
Print out error position for CREATE DOMAIN

This is simply done by pushing down the ParseState available in
ProcessUtility() to DefineDomain(), giving more information about the
position of an error when running a CREATE DOMAIN query.

Most of the queries impacted by this change have been added previously
in 0172b4c9449e.

Author: Kirill Reshke, Jian He
Reviewed-by: Álvaro Herrera, Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/CALdSSPhqfvKbDwqJaY=yEePi_aq61GmMpW88i6ZH7CMG_2Z4Cg@mail.gmail.com

4 months agoAdd some tests for encoding conversion in COPY TO/FROM
Michael Paquier [Mon, 16 Dec 2024 02:23:38 +0000 (11:23 +0900)]
Add some tests for encoding conversion in COPY TO/FROM

This adds a couple of tests to trigger encoding conversion when input
and server encodings do not match in COPY FROM/TO, or need_transcoding
set to true in the COPY state data.  These tests rely on UTF8 <-> LATIN1
for the valid cases as LATIN1 accepts any bytes, and UTF8 <-> EUC_JP for
some of the invalid cases where a character cannot be understood,
causing a conversion failure.

Both ENCODING and client_encoding are covered.  Test suggested by Andres
Freund.

Author: Sutou Kouhei
Discussion: https://postgr.es/m/20240206222445.hzq22pb2nye7rm67@awork3.anarazel.de

4 months agoDeclare a couple of variables inside not outside a PG_TRY block.
Tom Lane [Sun, 15 Dec 2024 20:50:07 +0000 (15:50 -0500)]
Declare a couple of variables inside not outside a PG_TRY block.

I went through the buildfarm's reports of "warning: variable 'foo'
might be clobbered by 'longjmp' or 'vfork' [-Wclobbered]".  As usual,
none of them are live problems according to my understanding of the
effects of setjmp/longjmp, to wit that local variables might revert
to their values as of PG_TRY entry, due to being kept in registers.
But I did happen to notice that XmlTableGetValue's "cstr" variable
doesn't need to be declared outside the PG_TRY block at all (thus
giving further proof that the -Wclobbered warning has little
connection to real problems).  We might as well move it inside,
and "cur" too, in hopes of eliminating one of the bogus warnings.

4 months agopgbench: fix misprocessing of some nested \if constructs.
Tom Lane [Sun, 15 Dec 2024 19:14:14 +0000 (14:14 -0500)]
pgbench: fix misprocessing of some nested \if constructs.

An \if command appearing within a false (not-to-be-executed) \if
branch was incorrectly treated the same as \elif.  This could allow
statements within the inner \if to be executed when they should
not be.  Also the missing inner \if stack entry would result in an
assertion failure (in assert-enabled builds) when the final \endif
is reached.

Report and patch by Michail Nikolaev.  Back-patch to all
supported branches.

Discussion: https://postgr.es/m/CANtu0oiA1ke=SP6tauhNqkUdv5QFsJtS1p=aOOf_iU+EhyKkjQ@mail.gmail.com

4 months agodoc: Clarify old WAL files are kept until they are summarized.
Fujii Masao [Sun, 15 Dec 2024 02:18:18 +0000 (11:18 +0900)]
doc: Clarify old WAL files are kept until they are summarized.

The documentation in wal.sgml explains that old WAL files cannot be
removed or recycled until they are archived (when WAL archiving is used)
or replicated (when using replication slots). However, it did not mention
that, similarly, old WAL files are also kept until they are summarized
if WAL summarization is enabled. This commit adds that clarification
to the documentation.

Back-patch to v17 where WAL summarization was added.

Author: Fujii Masao
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/fd0eb0a5-f43b-4e06-b450-cbca011b6cff@oss.nttdata.com

4 months agocontrib/earthdistance: Use SQL-standard function bodies.
Tom Lane [Sat, 14 Dec 2024 21:07:18 +0000 (16:07 -0500)]
contrib/earthdistance: Use SQL-standard function bodies.

The @extschema:name@ feature added by 72a5b1fc8 allows us to
make earthdistance's references to the cube extension fully
search-path-secure, so long as all those references are
resolved at extension installation time not runtime.
To do that, we must convert earthdistance's SQL functions to
the new SQL-standard style; but we wanted to do that anyway.

The functions can be updated in our customary style by running
CREATE OR REPLACE FUNCTION in an extension update script.
However, there's still problems in the "CREATE DOMAIN earth"
command: its references to cube functions could be captured
by hostile objects in earthdistance's installation schema,
if that's not where the cube extension is.  Worse, the reference
to the cube type itself as the domain's base could be captured,
and that's not something we could fix after-the-fact in the
update script.

What I've done about that is to change the "CREATE DOMAIN earth"
command in the base script earthdistance--1.1.sql.  Ordinarily,
changing a released extension script is forbidden; but I think
it's okay here since the results of successful (non-trojaned)
script execution will be identical to before.

A good deal of care is still needed to make the extension's scripts
proof against search-path-based attacks.  We have to make sure that
all the function and operator invocations have exact argument-type
matches, to forestall attacks based on supplying a better match.
Fortunately earthdistance isn't very big, so I've just gone through
it and inspected each call to be sure of that.  The only actual code
changes needed were to spell all floating-point constants in the style
'-1'::float8, rather than depending on runtime type conversions and/or
negations.  (I'm not sure that the shortcuts previously used were
attackable, but removing run-time effort is a good thing anyway.)

I believe that this fixes earthdistance enough that we could
mark it trusted and remove the warnings about it that were
added by 7eeb1d986; but I've not done that here.

The primary reason for dealing with this now is that we've
received reports of pg_upgrade failing for databases that use
earthdistance functions in contexts like generated columns.
That's a consequence of 2af07e2f7 having restricted the search_path
used while evaluating such expressions.  The only way to fix that
is to make the earthdistance functions independent of run-time
search_path.  This patch is very much nicer than the alternative of
attaching "SET search_path" clauses to earthdistance's functions:
it is more secure and doesn't create a run-time penalty.  Therefore,
I've chosen to back-patch this to v16 where @extschema:name@
was added.  It won't help unless users update to 16.7 and issue
"ALTER EXTENSION earthdistance UPDATE" before upgrading to 17,
but at least there's now a way to deal with the problem without
manual intervention in the dump/restore process.

Tom Lane and Ronan Dunklau

Discussion: https://postgr.es/m/3316564.aeNJFYEL58@aivenlaptop
Discussion: https://postgr.es/m/6a6439f1-8039-44e2-8fb9-59028f7f2014@mailbox.org

4 months agoRefactor some SQL/JSON error messages
Álvaro Herrera [Sat, 14 Dec 2024 11:55:00 +0000 (12:55 +0100)]
Refactor some SQL/JSON error messages

Turn type names into "%s" specifiers to 1) avoid getting them translated
and 2) reduce the total number of messages.

4 months agoFix warnings about declaration of environ on MinGW.
Thomas Munro [Sat, 14 Dec 2024 11:36:30 +0000 (00:36 +1300)]
Fix warnings about declaration of environ on MinGW.

POSIX says that the global variable environ shouldn't be declared in a
header, and that you have to declare it yourself.  MinGW declares it in
<stdlib.h> with some macrology that messes up our declarations.  Visual
Studio doesn't warn (there are clues that it may also declare it, but if
so, apparently compatibly).  Suppress our declarations, on MinGW only.

This clears the last warnings on CI's optional MinGW task, and hopefully
on build farm animal fairywren too.

Like 1319997d, no back-patch for now as it's not known to be breaking
anything, and my humble goal is just to keep the MinGW build clean going
forward.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> (earlier version)
Discussion: https://postgr.es/m/CA%2BhUKGJLMh%2B6W5E4M_jSFb43gnrA_-Q6-%2BBf3HkBXyGfRFcBsQ%40mail.gmail.com

4 months agoRemove EXTENSION_DONT_CHECK_SIZE from md.c.
Thomas Munro [Sat, 14 Dec 2024 07:59:58 +0000 (20:59 +1300)]
Remove EXTENSION_DONT_CHECK_SIZE from md.c.

Commits 7bb3102c and 3eb77eba removed the only user of the
EXTENSION_DONT_CHECK_SIZE flag, which had previously been required to
checkpoint truncated relations.  Since 7bb3102c, segments have been
opened directly for synchronization without calling _mdfd_getseg(), so
it doesn't need a mode that tolerates non-final short segments.  Remove
the redundant flag and associated comments.

Reported-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/nyj4k7yur5t27rtygvx2i2lrlp6rqfvvhoiiwx4fznynksf2et%404hj2sp42alpe

4 months agoFix typo
John Naylor [Sat, 14 Dec 2024 02:52:08 +0000 (09:52 +0700)]
Fix typo

Ryo Kanbayashi

Discussion: https://postgr.es/m/CANOn0ExEQiPVrzkjULkENVac_n4Lknm6dxsU69MSncQap0kJVA%40mail.gmail.com

4 months agoFix possible crash in pg_dump with identity sequences.
Tom Lane [Fri, 13 Dec 2024 19:21:36 +0000 (14:21 -0500)]
Fix possible crash in pg_dump with identity sequences.

If an owned sequence is considered interesting, force its owning
table to be marked interesting too.  This ensures, in particular,
that we'll fetch the owning table's column names so we have the
data needed for ALTER TABLE ... ADD GENERATED.  Previously there were
edge cases where pg_dump could get SIGSEGV due to not having filled in
the column names.  (The known case is where the owning table has been
made part of an extension while its identity sequence is not a member;
but there may be others.)

Also, if it's an identity sequence, force its dumped-components mask
to exactly match the owning table: dump definition only if we're
dumping the table's definition, dump data only if we're dumping the
table's data, etc.  This generalizes the code introduced in commit
b965f2617 that set the sequence's dump mask to NONE if the owning
table's mask is NONE.  That's insufficient to prevent failures,
because for example the table's mask might only request dumping ACLs,
which would lead us to still emit ALTER TABLE ADD GENERATED even
though we didn't create the table.  It seems better to treat an
identity sequence as though it were an inseparable aspect of the
table, matching the treatment used in the backend's dependency logic.
Perhaps this policy needs additional refinement, but let's wait to
see some field use-cases before changing it further.

While here, add a comment in pg_dump.h warning against writing tests
like "if (dobj->dump == DUMP_COMPONENT_NONE)", which was a bug in this
case.  There is one other example in getPublicationNamespaces, which
if it's not a bug is at least remarkably unclear and under-documented.
Changing that requires a separate discussion, however.

Per report from Artur Zakirov.  Back-patch to all supported branches.

Discussion: https://postgr.es/m/CAKNkYnwXFBf136=u9UqUxFUVagevLQJ=zGd5BsLhCsatDvQsKQ@mail.gmail.com

4 months agoRewrite maybe_reread_subscription() comment
Álvaro Herrera [Fri, 13 Dec 2024 06:41:36 +0000 (07:41 +0100)]
Rewrite maybe_reread_subscription() comment

One sentence was gramatically wrong, but also too terse.  Expand on it.

4 months agoDump not-null constraints on inherited columns correctly
Álvaro Herrera [Fri, 13 Dec 2024 06:38:49 +0000 (07:38 +0100)]
Dump not-null constraints on inherited columns correctly

With not-null constraints defined in child tables for columns that are
coming from their parent tables, we were printing ALTER TABLE SET NOT
NULL commands that were missing the constraint name, so the original
constraint name was being lost, which is bogus.  Fix by instead adding
a table-constraint constraint declaration with the correct constraint
name in the CREATE TABLE instead.

Oversight in commit 14e87ffa5c54.

We could have fixed it by changing the ALTER TABLE SET NOT NULL to ALTER
TABLE ADD CONSTRAINT, but I'm not sure that's any better.  A potential
problem here might be that if sent to a non-Postgres server, the new
pg_dump output would fail because the "CONSTRAINT foo NOT NULL colname"
syntax isn't SQL-conforming.  However, Postgres' implementation of
inheritance is already non-SQL-conforming, so that'd likely fail anyway.

This problem was only noticed by Ashutosh's proposed test framework for
pg_dump, https://postgr.es/m/CAExHW5uF5V=Cjecx3_Z=7xfh4rg2Wf61PT+hfquzjBqouRzQJQ@mail.gmail.com

Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reported-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/CAExHW5tbdgAKDfqjDJ-7Fk6PJtHg8D4zUF6FQ4H2Pq8zK38Nyw@mail.gmail.com

4 months agoRevert "Don't truncate database and user names in startup packets."
Nathan Bossart [Thu, 12 Dec 2024 21:52:04 +0000 (15:52 -0600)]
Revert "Don't truncate database and user names in startup packets."

This reverts commit 562bee0fc13dc95710b8db6a48edad2f3d052f2e.

We received a report from the field about this change in behavior,
so it seems best to revert this commit and to add proper
multibyte-aware truncation as a follow-up exercise.

Fixes bug #18711.

Reported-by: Adam Rauch
Reviewed-by: Tom Lane, Bertrand Drouvot, Bruce Momjian, Thomas Munro
Discussion: https://postgr.es/m/18711-7503ee3e449d2c47%40postgresql.org
Backpatch-through: 17

4 months agoAdjust some comments about structure properties in pg_stat.h
Michael Paquier [Thu, 12 Dec 2024 07:59:22 +0000 (16:59 +0900)]
Adjust some comments about structure properties in pg_stat.h

One comment of PgStat_TableCounts mentioned that its pending stats use
memcmp() to check for the all-zero case if there is any activity.  This
is not true since 07e9e28b56, as pg_memory_is_all_zeros() is used.

PgStat_FunctionCounts incorrectly documented that it relied on memcpy().
This has never been correct, and not relevant because function
statistics do not have an all-zero check for pending stats.

Checkpoint and bgwriter statistics have been always relying on memcmp()
or pg_memory_is_all_zeros() (since 07e9e28b56 for the latter), and never
mentioned the dependency on event counters for their all-zero checks.
Let's document these properties, like the table statistics.

Author: Bertrand Drouvot
Discussion: https://postgr.es/m/Z1hNLvcPgVLPxCoc@ip-10-97-1-34.eu-west-3.compute.internal

4 months agoDetect redundant GROUP BY columns using UNIQUE indexes
David Rowley [Thu, 12 Dec 2024 02:28:38 +0000 (15:28 +1300)]
Detect redundant GROUP BY columns using UNIQUE indexes

d4c3a156c added support that when the GROUP BY contained all of the
columns belonging to a relation's PRIMARY KEY, all other columns
belonging to that relation would be removed from the GROUP BY clause.
That's possible because all other columns are functionally dependent on
the PRIMARY KEY and those columns alone ensure the groups are distinct.

Here we expand on that optimization and allow it to work for any unique
indexes on the table rather than just the PRIMARY KEY index.  This
normally requires that all columns in the index are defined with NOT NULL,
however, we can relax that requirement when the index is defined with
NULLS NOT DISTINCT.

When there are multiple suitable indexes to allow columns to be removed,
we prefer the index with the least number of columns as this allows us
to remove the highest number of GROUP BY columns.  One day, we may want to
revisit that decision as it may make more sense to use the narrower set of
columns in terms of the width of the data types and stored/queried data.

This also adjusts the code to make use of RelOptInfo.indexlist rather
than looking up the catalog tables.

In passing, add another short-circuit path to allow bailing out earlier
in cases where it's certainly not possible to remove redundant GROUP BY
columns.  This early exit is now cheaper to do than when this code was
originally written as 00b41463c made it cheaper to check for empty
Bitmapsets.

Patch originally by Zhang Mingli and later worked on by jian he, but after
I (David) worked on it, there was very little of the original left.

Author: Zhang Mingli, jian he, David Rowley
Reviewed-by: jian he, Andrei Lepikhov
Discussion: https://postgr.es/m/327990c8-b9b2-4b0c-bffb-462249f82de0%40Spark

4 months agoImprove the test case from 5668a857d
Richard Guo [Thu, 12 Dec 2024 02:21:51 +0000 (11:21 +0900)]
Improve the test case from 5668a857d

In commit 5668a857d, we fixed an issue with incorrect results in right
semi joins and introduced a test case to verify the fix.  The test
case involves SubPlans and InitPlans, which may not be immediately
apparent in relation to the issue we addressed.

This patch simplifies the test case with a more straightforward query.

Per discussion with Melanie Plageman.

Author: Richard Guo
Discussion: https://postgr.es/m/CAAKRu_a-Cip2XCXp13fmxq+T9BhLAVApHTyjr94awL2mbXHC-Q@mail.gmail.com

4 months agoAdd some regression tests for missing DDL patterns
Michael Paquier [Thu, 12 Dec 2024 02:16:45 +0000 (11:16 +0900)]
Add some regression tests for missing DDL patterns

The following commands gain increased coverage for some of the errors
they can trigger:
- ALTER TABLE .. ALTER COLUMN
- CREATE DOMAIN
- CREATE TYPE (LIKE)

This has come up while discussing the possibility to add more
information about the location of the error in such queries, and it
is useful on its own as there was no coverage until now for the
patterns added in this commit.

Author: Jian He, Kirill Reshke
Reviewed-By: Álvaro Herrera, Michael Paquier
Discussion: https://postgr.es/m/CALdSSPhqfvKbDwqJaY=yEePi_aq61GmMpW88i6ZH7CMG_2Z4Cg@mail.gmail.com

4 months agoDefer remove_useless_groupby_columns() work until query_planner()
David Rowley [Thu, 12 Dec 2024 01:22:15 +0000 (14:22 +1300)]
Defer remove_useless_groupby_columns() work until query_planner()

Traditionally, remove_useless_groupby_columns() was called during
grouping_planner() directly after the call to preprocess_groupclause().
While in many ways, it made sense to populate the field and remove the
functionally dependent columns from processed_groupClause at the same
time, it's just that doing so had the disadvantage that
remove_useless_groupby_columns() was being called before the RelOptInfos
were populated for the relations mentioned in the query.  Not having
RelOptInfos available meant we needed to manually query the catalog tables
to get the required details about the primary key constraint for the
table.

Here we move the remove_useless_groupby_columns() call to
query_planner() and put it directly after the RelOptInfos are populated.
This is fine to do as processed_groupClause still isn't final at this
point as it can still be modified inside standard_qp_callback() by
make_pathkeys_for_sortclauses_extended().

This commit is just a refactor and simply moves
remove_useless_groupby_columns() into initsplan.c.  A planned follow-up
commit will adjust that function so it uses RelOptInfo instead of doing
catalog lookups and also teach it how to use unique indexes as proofs to
expand the cases where we can remove functionally dependent columns from
the GROUP BY.

Reviewed-by: Andrei Lepikhov, jian he
Discussion: https://postgr.es/m/CAApHDvqLezKwoEBBQd0dp4Y9MDkFBDbny0f3SzEeqOFoU7Z5+A@mail.gmail.com

5 months agoAdd UUID version 7 generation function.
Masahiko Sawada [Wed, 11 Dec 2024 23:54:41 +0000 (15:54 -0800)]
Add UUID version 7 generation function.

This commit introduces the uuidv7() SQL function, which generates UUID
version 7 as specified in RFC 9652. UUIDv7 combines a Unix timestamp
in milliseconds and random bits, offering both uniqueness and
sortability.

In our implementation, the 12-bit sub-millisecond timestamp fraction
is stored immediately after the timestamp, in the space referred to as
"rand_a" in the RFC. This ensures additional monotonicity within a
millisecond. The rand_a bits also function as a counter. We select a
sub-millisecond timestamp so that it monotonically increases for
generated UUIDs within the same backend, even when the system clock
goes backward or when generating UUIDs at very high
frequency. Therefore, the monotonicity of generated UUIDs is ensured
within the same backend.

This commit also expands the uuid_extract_timestamp() function to
support UUID version 7.

Additionally, an alias uuidv4() is added for the existing
gen_random_uuid() SQL function to maintain consistency.

Bump catalog version.

Author: Andrey Borodin
Reviewed-by: Sergey Prokhorenko, Przemysław Sztoch, Nikolay Samokhvalov
Reviewed-by: Peter Eisentraut, Jelte Fennema-Nio, Aleksander Alekseev
Reviewed-by: Masahiko Sawada, Lukas Fittl, Michael Paquier, Japin Li
Reviewed-by: Marcos Pegoraro, Junwang Zhao, Stepan Neretin
Reviewed-by: Daniel Vérité
Discussion: https://postgr.es/m/CAAhFRxitJv%3DyoGnXUgeLB_O%2BM7J2BJAmb5jqAT9gZ3bij3uLDA%40mail.gmail.com

5 months agoFix further fallout from EXPLAIN ANALYZE BUFFERS change
David Rowley [Wed, 11 Dec 2024 20:50:00 +0000 (09:50 +1300)]
Fix further fallout from EXPLAIN ANALYZE BUFFERS change

c2a4078eb adjusted EXPLAIN ANALYZE to default the BUFFERS to ON.  This
(hopefully) fixes the last remaining issue with regression test failures
with -D RELCACHE_FORCE_RELEASE -D CATCACHE_FORCE_RELEASE builds, where
the planner accesses more buffers due to the cold caches.

Discussion: https://postgr.es/m/CAApHDvqLdzgz77JsE-yTki3w9UiKQ-uTMLRctazcu+99-ips3g@mail.gmail.com

5 months agoUse pg_memory_is_all_zeros() in pgstatfuncs.c.
Nathan Bossart [Wed, 11 Dec 2024 20:19:14 +0000 (14:19 -0600)]
Use pg_memory_is_all_zeros() in pgstatfuncs.c.

There are a few places in this file that use memset() and memcmp()
to determine whether a section of memory is all zeros.  This commit
modifies them to use pg_memory_is_all_zeros() instead.  These
aren't expected to be hot code paths, but this may optimize them a
bit.  Plus, this allows us to remove some variables that were only
needed for the memset() and memcmp().

Author: Bertrand Drouvot
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/Z1hNubHfvMxlW6eu%40ip-10-97-1-34.eu-west-3.compute.internal

5 months agoUnmark gen_random_uuid() function leakproof.
Masahiko Sawada [Wed, 11 Dec 2024 18:35:57 +0000 (10:35 -0800)]
Unmark gen_random_uuid() function leakproof.

The functions without arguments don't need to be marked
leakproof. This commit unmarks gen_random_uuid() leakproof for
consistency with upcoming UUID generation functions. Also, this commit
adds a regression test to prevent reintroducing such cases.

Bump catalog version.

Reported-by: Peter Eisentraut
Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/CAD21AoBE1ePPWY1NQEgk3DkqjYzLPZwYTzCySHm0e%2B9a69PfZw%40mail.gmail.com

5 months agoFix a memory leak in dumping functions with TRANSFORMs
Daniel Gustafsson [Wed, 11 Dec 2024 11:48:22 +0000 (12:48 +0100)]
Fix a memory leak in dumping functions with TRANSFORMs

The gneration of the dump clause for functions with TRANSFORM
calls would leak the memory for holding the result of the Oid
array parsing.  Fix by freeing.

While in there, switch to using pg_malloc instead of palloc in
order to be consistent with the rest of the file.

Author: Oleg Tselebrovskiy <o.tselebrovskiy@postgrespro.ru>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/baf1ae4511288e5b421f41e79a3df1a0@postgrespro.ru

5 months agoAdd missing BUFFERS OFF in regression tests, take 2
David Rowley [Wed, 11 Dec 2024 10:16:44 +0000 (23:16 +1300)]
Add missing BUFFERS OFF in regression tests, take 2

Similar to 9fa1aaa65, but running with -D RELCACHE_FORCE_RELEASE and
-D CATCACHE_FORCE_RELEASE yielded some additional missing places that
needed BUFFERS OFF.

Discussion: https://postgr.es/m/CANNMO++W7MM8T0KyXN3ZheXXt-uLVM3aEtZd+WNfZ=obxffUiA@mail.gmail.com

5 months agoAdd missing BUFFERS OFF in select_into regression tests
David Rowley [Wed, 11 Dec 2024 09:56:36 +0000 (22:56 +1300)]
Add missing BUFFERS OFF in select_into regression tests

c2a4078eb adjusted EXPLAIN ANALYZE to include BUFFERS by default, but
a few tests in select_into.sql neglected to add BUFFERS OFF.  The
failing tests seem unlikely to ever access buffers during execution, but
they certainly could during planning.

Per buildfarm member kestrel, tayra and calliphoridae.

Discussion: https://postgr.es/m/CANNMO++W7MM8T0KyXN3ZheXXt-uLVM3aEtZd+WNfZ=obxffUiA@mail.gmail.com

5 months agoEnable BUFFERS with EXPLAIN ANALYZE by default
David Rowley [Wed, 11 Dec 2024 09:35:11 +0000 (22:35 +1300)]
Enable BUFFERS with EXPLAIN ANALYZE by default

The topic of turning EXPLAIN's BUFFERS option on with the ANALYZE option
has come up a few times over the past few years.  In many ways, doing this
seems like a good idea as it may be more obvious to users why a given
query is running more slowly than they might expect.  Also, from my own
(David's) personal experience, I've seen users posting to the mailing
lists with two identical plans, one slow and one fast asking why their
query is sometimes slow.  In many cases, this is due to additional reads.
Having BUFFERS on by default may help reduce some of these questions, and
if not, make it more obvious to the user before they post, or save a
round-trip to the mailing list when additional I/O effort is the cause of
the slowness.

The general consensus is that we want BUFFERS on by default with
ANALYZE.  However, there were more than zero concerns raised with doing
so.  The primary reason against is the additional verbosity, making it
harder to read large plans.  Another concern was that buffer information
isn't always useful so may not make sense to have it on by default.

It's currently December, so let's commit this to see if anyone comes
forward with a strong objection against making this change.  We have over
half a year remaining in the v18 cycle where we could still easily consider
reverting this if someone were to come forward with a convincing enough
reason as to why doing this is a bad idea.

There were two patches independently submitted to achieve this goal, one
by me and the other by Guillaume.  This commit is a mix of both of these
patches with some additional work done by me to adjust various
additional places in the documentation which include EXPLAIN ANALYZE
output.

Author: Guillaume Lelarge, David Rowley
Reviewed-by: Robert Haas, Greg Sabino Mullane, Michael Christofides
Discussion: https://postgr.es/m/CANNMO++W7MM8T0KyXN3ZheXXt-uLVM3aEtZd+WNfZ=obxffUiA@mail.gmail.com

5 months agoUse ExprStates for hashing in GROUP BY and SubPlans
David Rowley [Wed, 11 Dec 2024 00:47:16 +0000 (13:47 +1300)]
Use ExprStates for hashing in GROUP BY and SubPlans

This speeds up obtaining hash values for GROUP BY and hashed SubPlans by
using the ExprState support for hashing, thus allowing JIT compilation for
obtaining hash values for these operations.

This, even without JIT compilation, has been shown to improve Hash
Aggregate performance in some cases by around 15% and hashed NOT IN
queries in one case by over 30%, however, real-world cases are likely to
see smaller gains as the test cases used were purposefully designed to
have high hashing overheads by keeping the hash table small to prevent
additional memory overheads that would be a factor when working with large
hash tables.

In passing, fix a hypothetical bug in ExecBuildHash32Expr() so that the
initial value is stored directly in the ExprState's result field if
there are no expressions to hash.  None of the current users of this
function use an initial value, so the bug is only hypothetical.

Reviewed-by: Andrei Lepikhov <lepihov@gmail.com>
Discussion: https://postgr.es/m/CAApHDvpYSO3kc9UryMevWqthTBrxgfd9djiAjKHMPUSQeX9vdQ@mail.gmail.com

5 months agoUse in-place updates for pg_restore_relation_stats().
Jeff Davis [Wed, 11 Dec 2024 00:30:37 +0000 (16:30 -0800)]
Use in-place updates for pg_restore_relation_stats().

This matches the behavior of vac_update_relstats(), which is important
to avoid bloating pg_class.

Author: Corey Huinker
Discussion: https://postgr.es/m/CADkLM=fc3je+ufv3gsHqjjSSf+t8674RXpuXW62EL55MUEQd-g@mail.gmail.com

5 months agoImprove reporting of pg_upgrade log files on test failure
Michael Paquier [Tue, 10 Dec 2024 23:48:47 +0000 (08:48 +0900)]
Improve reporting of pg_upgrade log files on test failure

On failure, the pg_upgrade log files are automatically appended to the
test log file, but the information reported was inconsistent.

A header, with the log file name, was reported with note(), while the
log contents and a footer used print(), making it harder to diagnose
failures when these are split into console output and test log file
because the pg_upgrade log file path in the header may not be included
in the test log file.

The output is now consolidated so as the header uses print() rather than
note().  An extra note() is added to inform that the contents of a
pg_upgrade log file are appended to the test log file.

The diffs from the regression test suite and dump files all use print()
to show their contents on failure.

Author: Joel Jacobson
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/49f7e64a-b9be-4a90-a9fe-210a7740405e@app.fastmail.com
Backpatch-through: 15

5 months agoSpeedup Hash Joins with dedicated functions for ExprState hashing
David Rowley [Tue, 10 Dec 2024 22:32:15 +0000 (11:32 +1300)]
Speedup Hash Joins with dedicated functions for ExprState hashing

Hashing of a single Var is a very common operation for ExprState to
perform.  Here we add dedicated ExecJust* functions which helps speed up
Hash Joins by removing the interpretation overhead in ExecInterpExpr().

This change currently only affects Hash Joins on a single column.  Hash
Joins with multiple join keys or an expression still run through
ExecInterpExpr().

Some testing has shown up to 10% query performance increases on recent AMD
hardware and nearly 7% increase on an Apple M2 for a query performing a
hash join with a large number of lookups on a small hash table.

This change was extracted from a larger patch which adjusts GROUP BY /
hashed subplans / hashed set operations to use ExprState hashing.

Discussion: https://postgr.es/m/CAApHDvr8Zc0ZgzVoCZLdHGOFNhiJeQ6vrUcS9V7N23zMWQb-eA@mail.gmail.com

5 months agoDoc: add some commentary about ExecutorRun's NoMovement special case.
Tom Lane [Tue, 10 Dec 2024 22:17:28 +0000 (17:17 -0500)]
Doc: add some commentary about ExecutorRun's NoMovement special case.

Robert Haas expressed concern about whether commit 3eea7a0c9 exposed
the parallel-execution machinery to a case it isn't tested for, namely
a second non-parallel execution of a plan after a parallel execution.
Investigation shows that that can't happen because of pquery.c's
manipulation of the scan direction, but it sure wasn't obvious to
start with.  Add some commentary about that.

Discussion: https://postgr.es/m/CA+TgmoagyKQy=HFw+wLo0AKTYWHui+iKswZ8Jnqqd-cFby-WVg@mail.gmail.com

5 months agoFix elog(FATAL) before PostmasterMain() or just after fork().
Noah Misch [Tue, 10 Dec 2024 21:51:59 +0000 (13:51 -0800)]
Fix elog(FATAL) before PostmasterMain() or just after fork().

Since commit 97550c0711972a9856b5db751539bbaf2f88884c, these failed with
"PANIC:  proc_exit() called in child process" due to uninitialized or
stale MyProcPid.  That was reachable if close() failed in
ClosePostmasterPorts() or setlocale(category, "C") failed, both
unlikely.  Back-patch to v13 (all supported versions).

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

5 months agoTests for logical replication with temporal keys
Peter Eisentraut [Tue, 10 Dec 2024 14:05:58 +0000 (15:05 +0100)]
Tests for logical replication with temporal keys

This covers some cases that were previously failing before the
"Support for GiST in get_equal_strategy_number()" patch.

Author: Paul A. Jungwirth <pj@illuminatedcomputing.com>
Reviewed-by: vignesh C <vignesh21@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com

5 months agoSupport for GiST in get_equal_strategy_number()
Peter Eisentraut [Tue, 10 Dec 2024 12:26:09 +0000 (13:26 +0100)]
Support for GiST in get_equal_strategy_number()

A WITHOUT OVERLAPS primary key or unique constraint is accepted as a
REPLICA IDENTITY, since it guarantees uniqueness.  But subscribers
applying logical decoding messages would fail because there was not
support for looking up the equals operator for a gist index.  This
fixes that: For GiST indexes we can use the stratnum GiST support
function.

Reviewed-by: Paul Jungwirth <pj@illuminatedcomputing.com>
Reviewed-by: vignesh C <vignesh21@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com

5 months agoMake the conditions in IsIndexUsableForReplicaIdentityFull() more explicit
Peter Eisentraut [Tue, 10 Dec 2024 12:11:34 +0000 (13:11 +0100)]
Make the conditions in IsIndexUsableForReplicaIdentityFull() more explicit

IsIndexUsableForReplicaIdentityFull() described a number of conditions
that a suitable index has to fulfill.  But not all of these were
actually checked in the code.  Instead, it appeared to rely on
get_equal_strategy_number() to filter out any indexes that are not
btree or hash.  As we look to generalize index AM capabilities, this
would possibly break if we added additional support in
get_equal_strategy_number().  Instead, write out code to check for the
required capabilities explicitly.  This shouldn't change any behaviors
at the moment.

Reviewed-by: Paul Jungwirth <pj@illuminatedcomputing.com>
Reviewed-by: vignesh C <vignesh21@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com

5 months agoReplace get_equal_strategy_number_for_am() by get_equal_strategy_number()
Peter Eisentraut [Tue, 10 Dec 2024 11:53:27 +0000 (12:53 +0100)]
Replace get_equal_strategy_number_for_am() by get_equal_strategy_number()

get_equal_strategy_number_for_am() gets the equal strategy number for
an AM.  This currently only supports btree and hash.  In the more
general case, this also depends on the operator class (see for example
GistTranslateStratnum()).  To support that, replace this function with
get_equal_strategy_number() that takes an opclass and derives it from
there.  (This function already existed before as a static function, so
the signature is kept for simplicity.)

This patch is only a refactoring, it doesn't add support for other
index AMs such as gist.  This will be done separately.

Reviewed-by: Paul Jungwirth <pj@illuminatedcomputing.com>
Reviewed-by: vignesh C <vignesh21@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com

5 months agoImprove internal logical replication error for missing equality strategy
Peter Eisentraut [Tue, 10 Dec 2024 11:30:42 +0000 (12:30 +0100)]
Improve internal logical replication error for missing equality strategy

This "shouldn't happen", except right now it can with a temporal gist
index (to be fixed soon), because of missing gist support in
get_equal_strategy_number().  But right now, the error is not caught
right away, but instead you get the subsequent error about a "missing
operator 0".  This makes the error more accurate.

Author: Paul Jungwirth <pj@illuminatedcomputing.com>
Discussion: https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com

5 months agoFix comments of GUC hooks for timezone_abbreviations
Michael Paquier [Tue, 10 Dec 2024 04:02:21 +0000 (13:02 +0900)]
Fix comments of GUC hooks for timezone_abbreviations

The GUC assign and check hooks used "assign_timezone_abbreviations",
which was incorrect.

Issue noticed while browsing this area of the code, introduced in
0a20ff54f5e6.

Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/Z1eV6Y8yk77GZhZI@paquier.xyz
Backpatch-through: 16

5 months agoFix outdated comment of scram_build_secret()
Michael Paquier [Tue, 10 Dec 2024 03:54:09 +0000 (12:54 +0900)]
Fix outdated comment of scram_build_secret()

This routine documented that "iterations" would use a default value if
set to 0 by the caller.  However, the iteration should always be set by
the caller to a value strictly more than 0, as documented by an
assertion.

Oversight in b577743000cd, that has made the iteration count of SCRAM
configurable.

Author: Matheus Alcantara
Discussion: https://postgr.es/m/ac858943-4743-44cd-b4ad-08a0c10cbbc8@gmail.com
Backpatch-through: 16

5 months agoInclude necessary header files in radixtree.h.
Masahiko Sawada [Mon, 9 Dec 2024 21:07:06 +0000 (13:07 -0800)]
Include necessary header files in radixtree.h.

When #include'ing radixtree.h with RT_SHMEM, it could happen to raise
compiler errors due to missing some declarations of types and
functions.

This commit also removes the inclusion of postgres.h since it's
against our usual convention.

Backpatch to v17, where radixtree.h was introduced.

Reviewed-by: Heikki Linnakangas, Álvaro Herrera
Discussion: https://postgr.es/m/CAD21AoCU9YH%2Bb9Rr8YRw7UjmB%3D1zh8GKQkWNiuN9mVhMvkyrRg%40mail.gmail.com
Backpatch-through: 17

5 months agoDoc: fix incorrect EXPLAIN ANALYZE output for bloom indexes
David Rowley [Mon, 9 Dec 2024 20:24:43 +0000 (09:24 +1300)]
Doc: fix incorrect EXPLAIN ANALYZE output for bloom indexes

It looks like the example case was once modified to increase the number
of rows but the EXPLAIN ANALYZE output wasn't updated to reflect that.

Also adjust the text which discusses the index sizes.  With the example
table size, the bloom index isn't quite 8 times more space efficient
than the btree indexes.

Discussion: https://postgr.es/m/CAApHDvovx8kQ0=HTt85gFDAwmTJHpCgiSvRmQZ_6u_g-vQYM_w@mail.gmail.com
Backpatch-through: 13, all supported versions

5 months agoFix small memory leaks in GUC checks
Daniel Gustafsson [Mon, 9 Dec 2024 19:58:23 +0000 (20:58 +0100)]
Fix small memory leaks in GUC checks

Follow-up commit to a9d58bfe8a3a.  Backpatch down to v16 where
this was added in order to keep the code consistent for future
backpatches.

Author: Tofig Aliev <t.aliev@postgrespro.ru>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/bba4313fdde9db46db279f96f3b748b1@postgrespro.ru
Backpatch-through: 16

5 months agoFix various overflow hazards in date and timestamp functions.
Nathan Bossart [Mon, 9 Dec 2024 19:47:23 +0000 (13:47 -0600)]
Fix various overflow hazards in date and timestamp functions.

This commit makes use of the overflow-aware routines in int.h to
fix a variety of reported overflow bugs in the date and timestamp
code.  It seems unlikely that this fixes all such bugs in this
area, but since the problems seem limited to cases that are far
beyond any realistic usage, I'm not going to worry too much.  Note
that for one bug, I've chosen to simply add a comment about the
overflow hazard because fixing it would require quite a bit of code
restructuring that doesn't seem worth the risk.

Since this is a bug fix, it could be back-patched, but given the
risk of conflicts with the new routines in int.h and the overall
risk/reward ratio of this patch, I've opted not to do so for now.

Fixes bug #18585 (except for the one case that's just commented).

Reported-by: Alexander Lakhin
Author: Matthew Kim, Nathan Bossart
Reviewed-by: Joseph Koshakow, Jian He
Discussion: https://postgr.es/m/31ad2cd1-db94-bdb3-f91a-65ffdb4bef95%40gmail.com
Discussion: https://postgr.es/m/18585-db646741dd649abd%40postgresql.org

5 months agoSimplify executor's determination of whether to use parallelism.
Tom Lane [Mon, 9 Dec 2024 19:38:19 +0000 (14:38 -0500)]
Simplify executor's determination of whether to use parallelism.

Our parallel-mode code only works when we are executing a query
in full, so ExecutePlan must disable parallel mode when it is
asked to do partial execution.  The previous logic for this
involved passing down a flag (variously named execute_once or
run_once) from callers of ExecutorRun or PortalRun.  This is
overcomplicated, and unsurprisingly some of the callers didn't
get it right, since it requires keeping state that not all of
them have handy; not to mention that the requirements for it were
undocumented.  That led to assertion failures in some corner
cases.  The only state we really need for this is the existing
QueryDesc.already_executed flag, so let's just put all the
responsibility in ExecutePlan.  (It could have been done in
ExecutorRun too, leading to a slightly shorter patch -- but if
there's ever more than one caller of ExecutePlan, it seems better
to have this logic in the subroutine than the callers.)

This makes those ExecutorRun/PortalRun parameters unnecessary.
In master it seems okay to just remove them, returning the
API for those functions to what it was before parallelism.
Such an API break is clearly not okay in stable branches,
but for them we can just leave the parameters in place after
documenting that they do nothing.

Per report from Yugo Nagata, who also reviewed and tested
this patch.  Back-patch to all supported branches.

Discussion: https://postgr.es/m/20241206062549.710dc01cf91224809dd6c0e1@sraoss.co.jp

5 months agoRemove remants of "snapshot too old"
Heikki Linnakangas [Mon, 9 Dec 2024 16:13:03 +0000 (18:13 +0200)]
Remove remants of "snapshot too old"

Remove the 'whenTaken' and 'lsn' fields from SnapshotData. After the
removal of the "snapshot too old" feature, they were never set to a
non-zero value.

This largely reverts commit 3e2f3c2e423, which added the
OldestActiveSnapshot tracking, and the init_toast_snapshot()
function. That was only required for setting the 'whenTaken' and 'lsn'
fields. SnapshotToast is now a constant again, like SnapshotSelf and
SnapshotAny. I kept a thin get_toast_snapshot() wrapper around
SnapshotToast though, to check that you have a registered or active
snapshot. That's still a useful sanity check.

Reviewed-by: Nathan Bossart, Andres Freund, Tom Lane
Discussion: https://www.postgresql.org/message-id/cd4b4f8c-e63a-41c0-95f6-6e6cd9b83f6d@iki.fi

5 months agoAvoid unnecessary wrapping for Vars and PHVs
Richard Guo [Mon, 9 Dec 2024 11:38:22 +0000 (20:38 +0900)]
Avoid unnecessary wrapping for Vars and PHVs

When pulling up a lateral subquery that is under an outer join, the
current code always wraps a Var or PHV in the subquery's targetlist
into a new PlaceHolderVar if it is a lateral reference to something
outside the subquery.  This is necessary when the Var/PHV references
the non-nullable side of the outer join from the nullable side: we
need to ensure that it is evaluated at the right place and hence is
forced to null when the outer join should do so.  However, if the
referenced rel is under the same lowest nulling outer join, we can
actually omit the wrapping.  That's safe because if the subquery
variable is forced to NULL by the outer join, the lateral reference
variable will come out as NULL too.  It could be beneficial to get rid
of such PHVs because they imply lateral dependencies, which force us
to resort to nestloop joins.

This patch leverages the newly introduced nullingrel_info to check if
the nullingrels of the subquery RTE are a subset of those of the
laterally referenced rel, in order to determine if the referenced rel
is under the same lowest nulling outer join.

No backpatch as this could result in plan changes.

Author: Richard Guo
Reviewed-by: James Coleman, Dmitry Dolgov, Andrei Lepikhov
Discussion: https://postgr.es/m/CAMbWs48uk6C7Z9m_FNT8_21CMCk68hrgAsz=z6zpP1PNZMkeoQ@mail.gmail.com

5 months agoFix right-semi-joins in HashJoin rescans
Richard Guo [Mon, 9 Dec 2024 11:36:23 +0000 (20:36 +0900)]
Fix right-semi-joins in HashJoin rescans

When resetting a HashJoin node for rescans, if it is a single-batch
join and there are no parameter changes for the inner subnode, we can
just reuse the existing hash table without rebuilding it.  However,
for join types that depend on the inner-tuple match flags in the hash
table, we need to reset these match flags to avoid incorrect results.
This applies to right, right-anti, right-semi, and full joins.

When I introduced "Right Semi Join" plan shapes in aa86129e1, I failed
to reset the match flags in the hash table for right-semi joins in
rescans.  This oversight has been shown to produce incorrect results.
This patch fixes it.

Author: Richard Guo
Discussion: https://postgr.es/m/CAMbWs4-nQF9io2WL2SkD0eXvfPdyBc9Q=hRwfQHCGV2usa0jyA@mail.gmail.com

5 months agoFix memory leak in pgoutput with publication list cache
Michael Paquier [Mon, 9 Dec 2024 07:41:46 +0000 (16:41 +0900)]
Fix memory leak in pgoutput with publication list cache

The pgoutput module caches publication names in a list and frees it upon
invalidation.  However, the code forgot to free the actual publication
names within the list elements, as publication names are pstrdup()'d in
GetPublication().  This would cause memory to leak in
CacheMemoryContext, bloating it over time as this context is not
cleaned.

This is a problem for WAL senders running for a long time, as an
accumulation of invalidation requests would bloat its cache memory
usage.  A second case, where this leak is easier to see, involves a
backend calling SQL functions like pg_logical_slot_{get,peek}_changes()
which create a new decoding context with each execution.  More
publications create more bloat.

To address this, this commit adds a new memory context within the
logical decoding context and resets it each time the publication names
cache is invalidated, based on a suggestion from Amit Kapila.  This
ensures that the lifespan of the publication names aligns with that of
the logical decoding context.

This solution changes PGOutputData, which is fine for HEAD but it could
cause an ABI breakage in stable branches as the structure size would
change, so these are left out for now.

Analyzed-by: Michael Paquier, Jeff Davis
Author: Zhijie Hou
Reviewed-by: Michael Paquier, Masahiko Sawada, Euler Taveira
Discussion: https://postgr.es/m/Z0khf9EVMVLOc_YY@paquier.xyz

5 months agoImprove comment about dropped entries in pgstat.c
Michael Paquier [Mon, 9 Dec 2024 05:35:39 +0000 (14:35 +0900)]
Improve comment about dropped entries in pgstat.c

pgstat_write_statsfile() discards any entries marked as dropped from
being written to the stats file at shutdown, and also included an
assertion based on the same condition.

The intention of the assertion is to track that no pgstats entries
should be left around as terminating backends should drop any entries
they still hold references on before the stats file is written by the
checkpointer, and it not worth taking down the server in this case if
there is a bug making that possible.

Let's improve the comment of this area to document clearly what's
intended.

Based on a discussion with Bertrand Drouvot and Anton A. Melnikov.

Author: Bertrand Drouvot
Discussion: https://postgr.es/m/a13e8cdf-b97a-4ecb-8f42-aaa367974e29@postgrespro.ru
Backpatch-through: 15

5 months agoImprove the error message introduced in commit 87ce27de696.
Amit Kapila [Mon, 9 Dec 2024 03:41:45 +0000 (09:11 +0530)]
Improve the error message introduced in commit 87ce27de696.

The error detail message "Replica identity consists of an unpublished
generated column." implies that the entire replica identity is made up of
an unpublished generated column which may not be the case.

Reported-by: Peter Smith
Author: Shlok Kyal
Reviewed-by: Peter Smith, Amit Kapila
Discussion: https://postgr.es/m/CAHut+PuwMhKx0PhOA4APhJTLoBGNykbeCQpr_CuwGT-SkswG5w@mail.gmail.com

5 months agoFix invalidation of local pgstats references for entry reinitialization
Michael Paquier [Mon, 9 Dec 2024 01:45:28 +0000 (10:45 +0900)]
Fix invalidation of local pgstats references for entry reinitialization

818119afccd3 has introduced the "generation" concept in pgstats entries,
incremented a counter when a pgstats entry is reinitialized, but it did
not count on the fact that backends still holding local references to
such entries need to be refreshed if the cache age is outdated.  The
previous logic only updated local references when an entry was dropped,
but it needs also to consider entries that are reinitialized.

This matters for replication slot stats (as well as custom pgstats kinds
in 18~), where concurrent drops and creates of a slot could cause
incorrect stats to be locally referenced.  This would lead to an
assertion failure at shutdown when writing out the stats file, as the
backend holding an outdated local reference would not be able to drop
during its shutdown sequence the stats entry that should be dropped, as
the last process holding a reference to the stats entry.  The
checkpointer was then complaining about such an entry late in the
shutdown sequence, after the shutdown checkpoint is finished with the
control file updated, causing the stats file to not be generated.  In
non-assert builds, the entry would just be skipped with the stats file
written.

Note that only logical replication slots use statistics.

A test case based on TAP is added to test_decoding, where a persistent
connection peeking at a slot's data is kept with concurrent drops and
creates of the same slot.  This is based on the isolation test case that
Anton has sent.  As it requires a node shutdown with a check to make
sure that the stats file is written with this specific sequence of
events, TAP is used instead.

Reported-by: Anton A. Melnikov
Reviewed-by: Bertrand Drouvot
Discussion: https://postgr.es/m/56bf8ff9-dd8c-47b2-872a-748ede82af99@postgrespro.ru
Backpatch-through: 15

5 months agoFix possible crash during WindowAgg evaluation
David Rowley [Mon, 9 Dec 2024 01:23:21 +0000 (14:23 +1300)]
Fix possible crash during WindowAgg evaluation

When short-circuiting WindowAgg node evaluation on the top-level
WindowAgg node using quals on monotonic window functions, because the
WindowAgg run condition can mean there's no need to evaluate subsequent
window function results in the same partition once the run condition
becomes false, it was possible that the executor would use stale results
from the previous invocation of the window function in some cases.

A fix for this was partially done by a5832722, but that commit only
fixed the issue for non-top-level WindowAgg nodes.  I mistakenly thought
that the top-level WindowAgg didn't have this issue, but Jayesh's example
case clearly shows that's incorrect.  At the time, I also thought that
this only affected 32-bit systems as all window functions which then
supported run conditions returned BIGINT, however, that's wrong as
ExecProject is still called and that could cause evaluation of any other
window function belonging to the same WindowAgg node, one of which may
return a byref type.

The only queries affected by this are WindowAggs with a "Run Condition"
which contains at least one window function with a byref result type,
such as lead() or lag() on a byref column.  The window clause must also
contain a PARTITION BY clause (without a PARTITION BY, execution of the
WindowAgg stops immediately when the run condition becomes false and
there's no risk of using the stale results).

Reported-by: Jayesh Dehankar
Discussion: https://postgr.es/m/193261e2c4d.3dd3cd7c1842.871636075166132237@zohocorp.com
Backpatch-through: 15, where WindowAgg run conditions were added

5 months agoEnsure that pg_amop/amproc entries depend on their lefttype/righttype.
Tom Lane [Sat, 7 Dec 2024 20:56:28 +0000 (15:56 -0500)]
Ensure that pg_amop/amproc entries depend on their lefttype/righttype.

Usually an entry in pg_amop or pg_amproc does not need a dependency on
its amoplefttype/amoprighttype/amproclefttype/amprocrighttype types,
because there is an indirect dependency via the argument types of its
referenced operator or procedure, or via the opclass it belongs to.
However, for some support procedures in some index AMs, the argument
types of the support procedure might not mention the column data type
at all.  Also, the amop/amproc entry might be treated as "loose" in
the opfamily, in which case it lacks a dependency on any particular
opclass; or it might be a cross-type entry having a reference to a
datatype that is not its opclass' opcintype.

The upshot of all this is that there are cases where a datatype can
be dropped while leaving behind amop/amproc entries that mention it,
because there is no path in pg_depend showing that those entries
depend on that type.  Such entries are harmless in normal activity,
because they won't get used, but they cause problems for maintenance
actions such as dropping the operator family.  They also cause pg_dump
to produce bogus output.  The previous commit put a band-aid on the
DROP OPERATOR FAMILY failure, but a real fix is needed.

To fix, add pg_depend entries showing that a pg_amop/pg_amproc entry
depends on its lefttype/righttype.  To avoid bloating pg_depend too
much, skip this if the referenced operator or function has that type
as an input type.  (I did not bother with considering the possible
indirect dependency via the opclass' opcintype; at least in the
reported case, that wouldn't help anyway.)

Probably, the reason this has escaped notice for so long is that
add-on datatypes and relevant opclasses/opfamilies are usually
packaged as extensions nowadays, so that there's no way to drop
a type without dropping the referencing opclasses/opfamilies too.
Still, in the absence of pg_depend entries there's nothing that
constrains DROP EXTENSION to drop the opfamily entries before the
datatype, so it seems possible for a DROP failure to occur anyway.

The specific case that was reported doesn't fail in v13, because
v13 prefers to attach the support procedure to the opclass not the
opfamily.  But it's surely possible to construct other edge cases
that do fail in v13, so patch that too.

Per report from Yoran Heling.  Back-patch to all supported branches.

Discussion: https://postgr.es/m/Z1MVCOh1hprjK5Sf@gmai021

5 months agoMake getObjectDescription robust against dangling amproc type links.
Tom Lane [Sat, 7 Dec 2024 19:28:16 +0000 (14:28 -0500)]
Make getObjectDescription robust against dangling amproc type links.

Yoran Heling reported a case where a data type could be dropped
while references to its OID remain behind in pg_amproc.  This
causes getObjectDescription to fail, which blocks dropping the
operator family (since our DROP code likes to construct descriptions
of everything it's dropping).  The proper fix for this requires
adding more pg_depend entries.  But to allow DROP to go through with
already-corrupt catalogs, tweak getObjectDescription to print "???"
for the type instead of failing when it processes such an entry.

I changed the logic for pg_amop similarly, for consistency,
although it is not known that the problem can manifest in pg_amop.

Per report from Yoran Heling.  Back-patch to all supported
branches (although the problem may be unreachable in v13).

Discussion: https://postgr.es/m/Z1MVCOh1hprjK5Sf@gmai021

5 months agoFix is_digit labeling of to_timestamp's FFn format codes.
Tom Lane [Sat, 7 Dec 2024 18:12:32 +0000 (13:12 -0500)]
Fix is_digit labeling of to_timestamp's FFn format codes.

These format codes produce or consume strings of digits, so they
should be labeled with is_digit = true, but they were not.
This has effect in only one place, where is_next_separator()
is checked to see if the preceding format code should slurp up
all the available digits.  Thus, with a format such as '...SSFF3'
with remaining input '12345', the 'SS' code would consume all
five digits (and then complain about seconds being out of range)
when it should eat only two digits.

Per report from Nick Davies.  This bug goes back to d589f9446
where the FFn codes were introduced, so back-patch to v13.

Discussion: https://postgr.es/m/AM8PR08MB6356AC979252CFEA78B56678B6312@AM8PR08MB6356.eurprd08.prod.outlook.com

5 months agodoc: remove LC_COLLATE and LC_CTYPE from SHOW command
Peter Eisentraut [Sat, 7 Dec 2024 11:55:55 +0000 (12:55 +0100)]
doc: remove LC_COLLATE and LC_CTYPE from SHOW command

The corresponding read-only server settings have been removed since
in PG16. See commit b0f6c437160db6.

Author: Pierre Giraud <pierre.giraud@dalibo.com>
Discussion: https://www.postgresql.org/message-id/flat/a75a2fb0-f4b3-4c0c-be3d-7a62d266d760%40dalibo.com

5 months agoComment fix: "buffer context lock" to "buffer content lock".
Jeff Davis [Fri, 6 Dec 2024 17:59:12 +0000 (09:59 -0800)]
Comment fix: "buffer context lock" to "buffer content lock".

The term "buffer context lock" is outdated as of commit 5d5087363d.

5 months agoRemove useless casts to (const void *)
Peter Eisentraut [Fri, 6 Dec 2024 16:22:19 +0000 (17:22 +0100)]
Remove useless casts to (const void *)

Similar to commit 7f798aca1d5, but I didn't think to look for "const"
as well.

5 months agoFix printf format string warning on MinGW.
Thomas Munro [Thu, 5 Dec 2024 23:34:33 +0000 (12:34 +1300)]
Fix printf format string warning on MinGW.

Commit 517bf2d91 changed a printf format string to placate MinGW, which
at the time warned about "%lld".  Current MinGW is now warning about the
replacement "%I64d".  Reverting the change clears the warning on the
MinGW CI task, and hopefully it will clear it on build farm animal
fairywren too.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reported-by: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
Discussion: https://postgr.es/m/TYAPR01MB5866A71B744BE01B3BF71791F5AEA%40TYAPR01MB5866.jpnprd01.prod.outlook.com

5 months agoRemove pg_regex_collation
Peter Eisentraut [Thu, 5 Dec 2024 06:19:37 +0000 (07:19 +0100)]
Remove pg_regex_collation

We can also use the existing pg_regex_locale as the cache key, which
is the only use of this variable.

Reviewed-by: Jeff Davis <pgsql@j-davis.com>
Discussion: https://www.postgresql.org/message-id/flat/b1b92ae1-2e06-4619-a87a-4b4858e547ec%40eisentraut.org

5 months agoFix header inclusion order in c.h.
Thomas Munro [Thu, 5 Dec 2024 01:27:35 +0000 (14:27 +1300)]
Fix header inclusion order in c.h.

Commit 962da900a added #include <stdint.h> to postgres_ext.h, which
broke c.h's header ordering rule.

The system headers on some systems would then lock down off_t's size in
private macros, before they'd had a chance to see our definition of
_FILE_OFFSET_BITS (and presumably other things).  This was picked up by
perl's ABI compatibility checks on some 32 bit systems in the build
farm.

Move #include "postgres_ext.h" down below the system header section, and
make the comments clearer (thanks to Tom for the new wording).

Diagnosed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/2397643.1733347237%40sss.pgh.pa.us

5 months agoProvide a better error message for misplaced dispatch options.
Nathan Bossart [Wed, 4 Dec 2024 21:04:15 +0000 (15:04 -0600)]
Provide a better error message for misplaced dispatch options.

Before this patch, misplacing a special must-be-first option for
dispatching to a subprogram (e.g., postgres -D . --single) would
fail with an error like

FATAL:  --single requires a value

This patch adjusts this error to more accurately complain that the
special option wasn't listed first.  The aforementioned error
message now looks like

FATAL:  --single must be first argument

The dispatch option parsing code has been refactored for use
wherever ParseLongOption() is called.  Beyond the obvious advantage
of avoiding code duplication, this should prevent similar problems
when new dispatch options are added.  Note that we assume that none
of the dispatch option names match another valid command-line
argument, such as the name of a configuration parameter.

Ideally, we'd remove this must-be-first requirement for these
options, but after some investigation, we decided that wasn't worth
the added complexity and behavior changes.

Author: Nathan Bossart, Greg Sabino Mullane
Reviewed-by: Greg Sabino Mullane, Peter Eisentraut, Álvaro Herrera, Tom Lane
Discussion: https://postgr.es/m/CAKAnmmJkZtZAiSryho%3DgYpbvC7H-HNjEDAh16F3SoC9LPu8rqQ%40mail.gmail.com

5 months agoReturn actual error code from FOP failure in PDF build
Bruce Momjian [Wed, 4 Dec 2024 19:37:24 +0000 (14:37 -0500)]
Return actual error code from FOP failure in PDF build

Previously we returned "1" on error.  Improvement on 77c189cdafe.

Backpatch-through: master

5 months agoFix dead code
Peter Eisentraut [Wed, 4 Dec 2024 15:43:07 +0000 (16:43 +0100)]
Fix dead code

from commit 85b7efa1cdd

per Coverity report

5 months agoFix use-after-free in parallel_vacuum_reset_dead_items
John Naylor [Wed, 4 Dec 2024 09:51:55 +0000 (16:51 +0700)]
Fix use-after-free in parallel_vacuum_reset_dead_items

parallel_vacuum_reset_dead_items used a local variable to hold a
pointer from the passed vacrel, purely as a shorthand. This pointer
was later freed and a new allocation was made and stored to the
struct. Then the local pointer was mistakenly referenced again.

This apparently happened not to break anything since the freed chunk
would have been put on the context's freelist, so it was accidentally
the same pointer anyway, in which case the DSA handle was correctly
updated. The minimal fix is to change two places so they access
dead_items through the vacrel. This coding style is a maintenance
hazard, so while at it get rid of most other similar usages, which
were inconsistently used anyway.

Analysis and patch by Vallimaharajan G, with further defensive coding
by me

Backpath to v17, when TidStore came in

Discussion: https://postgr.es/m/1936493cc38.68cb2ef27266.7456585136086197135@zohocorp.com

5 months agoSimplify IsIndexUsableForReplicaIdentityFull()
Peter Eisentraut [Wed, 4 Dec 2024 07:33:28 +0000 (08:33 +0100)]
Simplify IsIndexUsableForReplicaIdentityFull()

Take Relation as argument instead of IndexInfo.  Building the
IndexInfo is an unnecessary intermediate step here.

A future patch wants to get some information that is in the relcache
but not in IndexInfo, so this will also help there.

Discussion: https://www.postgresql.org/message-id/333d3886-b737-45c3-93f4-594c96bb405d@eisentraut.org

5 months agoEnsure stored generated columns must be published when required.
Amit Kapila [Wed, 4 Dec 2024 04:15:18 +0000 (09:45 +0530)]
Ensure stored generated columns must be published when required.

Ensure stored generated columns that are part of REPLICA IDENTITY must be
published explicitly for UPDATE and DELETE operations to be published. We
can publish generated columns by listing them in the column list or by
enabling the publish_generated_columns option.

This commit changes the behavior of the test added in commit adedf54e65 by
giving an ERROR for the UPDATE operation in such cases. There is no way to
trigger the bug reported in commit adedf54e65 but we didn't remove the
corresponding code change because it is still relevant when replicating
changes from a publisher with version less than 18.

We decided not to backpatch this behavior change to avoid the risk of
breaking existing output plugins that may be sending generated columns by
default although we are not aware of any such plugin. Also, we didn't see
any reports related to this on STABLE branches which is another reason not
to backpatch this change.

Author: Shlok Kyal, Hou Zhijie
Reviewed-by: Vignesh C, Amit Kapila
Discussion: https://postgr.es/m/CANhcyEVw4V2Awe2AB6i0E5AJLNdASShGfdBLbUd1XtWDboymCA@mail.gmail.com

5 months agoProperly use $(AWK) in Makefile, not 'awk'
Bruce Momjian [Wed, 4 Dec 2024 03:31:12 +0000 (22:31 -0500)]
Properly use $(AWK) in Makefile, not 'awk'

Fix for commit 498f1307569.

Backpatch-through: master

5 months agoUse <stdint.h> and <inttypes.h> for c.h integers.
Thomas Munro [Wed, 4 Dec 2024 01:46:59 +0000 (14:46 +1300)]
Use <stdint.h> and <inttypes.h> for c.h integers.

Redefine our exact width types with standard C99 types and macros,
including int64_t, INT64_MAX, INT64_C(), PRId64 etc.  We were already
using <stdint.h> types in a few places.

One complication is that Windows' <inttypes.h> uses format strings like
"%I64d", "%I32", "%I" for PRI*64, PRI*32, PTR*PTR, instead of mapping to
other standardized format strings like "%lld" etc as seen on other known
systems.  Teach our snprintf.c to understand them.

This removes a lot of configure clutter, and should also allow 64-bit
numbers and other standard types to be used in localized messages
without casting.

Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/ME3P282MB3166F9D1F71F787929C0C7E7B6312%40ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM

5 months agoDefine __EXTENSIONS__ on Solaris, too.
Tom Lane [Wed, 4 Dec 2024 01:21:23 +0000 (20:21 -0500)]
Define __EXTENSIONS__ on Solaris, too.

Apparently, if you define _POSIX_C_SOURCE on Solaris,
that's interpreted as "you get ONLY what's defined by POSIX".
Results from BF member hake show that that breaks perl.h,
and doubtless it'd cause more problems if we got past that.
Adopt the suggestion from standards(7) that we also need to
define __EXTENSIONS__, in hopes of un-breaking things.

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

5 months agoFix Makefile so invalid characters warning preserves error code
Bruce Momjian [Tue, 3 Dec 2024 23:27:41 +0000 (18:27 -0500)]
Fix Makefile so invalid characters warning preserves error code

Fix for commit e4c8865196f.

Reported-by: Peter Eisentraut
Discussion: https://postgr.es/m/88cb6ecf-22bb-431e-974b-1cd236a80364@eisentraut.org

Backpatch-through: master

5 months agoNow that we have non-Latin1 SGML detection, restore Latin1 chars
Bruce Momjian [Tue, 3 Dec 2024 22:09:49 +0000 (17:09 -0500)]
Now that we have non-Latin1 SGML detection, restore Latin1 chars

This reverts the change in commit 641a5b7a144 that converted them to
HTML entities.

Reported-by: Peter Eisentraut
Discussion: https://postgr.es/m/Z05ssoVheWI-rqax@momjian.us

Backpatch-through: master

5 months agoMove check for ucol_strcollUTF8 to pg_locale_icu.c
Jeff Davis [Tue, 3 Dec 2024 19:32:14 +0000 (11:32 -0800)]
Move check for ucol_strcollUTF8 to pg_locale_icu.c

The result of the check is only used by pg_locale_icu.c.

Author: Andreas Karlsson
Discussion: https://postgr.es/m/4548a168-62cd-457b-8d06-9ba7b985c477@proxel.se

5 months agoDefine _POSIX_C_SOURCE as 200112L on Solaris.
Tom Lane [Tue, 3 Dec 2024 17:44:43 +0000 (12:44 -0500)]
Define _POSIX_C_SOURCE as 200112L on Solaris.

This is an attempt to suppress some compiler warnings that appeared in
the wake of commit 7f798aca1: it seems that by default Solaris/illumos
declares shmdt() to take "char *" not "void *".  We'd like the system
headers to provide modern POSIX APIs, and POSIX 2001 seems to be as
modern as is available there.

illumos' standards(7) man page suggests that we might also need to
define __EXTENSIONS__, but let's see what happens with just this.

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

5 months agoFix synchronized_standby_slots GUC check hook
Álvaro Herrera [Tue, 3 Dec 2024 16:50:57 +0000 (17:50 +0100)]
Fix synchronized_standby_slots GUC check hook

The validate_sync_standby_slots subroutine requires an LWLock, so it
cannot run in processes without PGPROC; skip it there to avoid a crash.

This replaces the current test for ReplicationSlotCtl being not null,
which appears to be a solution for the same problem but less general.
I also rewrote a related comment that mentioned ReplicationSlotCtl in
StandbySlotsHaveCaughtup.

This code came in with commit bf279ddd1c28; backpatch to 17.

Reported-by: Gabriele Bartolini <gabriele.bartolini@enterprisedb.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com>
Discussion: https://postgr.es/m/202411281216.sutbxtr6idnn@alvherre.pgsql

5 months agoDrop "Lock" suffix from LWLock wait event names
Álvaro Herrera [Tue, 3 Dec 2024 14:50:03 +0000 (15:50 +0100)]
Drop "Lock" suffix from LWLock wait event names

Commit da952b415f44 unintentially reverted the SQL-visible part of
commit 14a910109126, which breaks queries joining pg_wait_events with
pg_stat_acivity.  Remove the suffix again.

Backpatch to 17.

Reported-by: Christophe Courtois <christophe.courtois@dalibo.com>
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/18728-450924477056a339%40postgresql.org
Discussion: https://postgr.es/m/Z01w1+LihtRiS0Te@ip-10-97-1-34.eu-west-3.compute.internal

5 months agoUpdate obsolete comment
Álvaro Herrera [Tue, 3 Dec 2024 13:46:31 +0000 (14:46 +0100)]
Update obsolete comment

Commit 3aa0395d4ed3 made worrying about BKI_ROWTYPE_OID matching no
longer necessary, but this comment didn't get the memo.

5 months agoFix handling of CREATE DOMAIN with GENERATED constraint syntax
Peter Eisentraut [Tue, 3 Dec 2024 13:32:45 +0000 (14:32 +0100)]
Fix handling of CREATE DOMAIN with GENERATED constraint syntax

Stuff like

    CREATE DOMAIN foo AS int CONSTRAINT cc GENERATED ALWAYS AS (2) STORED

is not supported for domains, but the parser allows it, because it's
the same syntax as for table constraints.  But CreateDomain() did not
explicitly handle all ConstrType values, so the above would get an
internal error like

    ERROR:  unrecognized constraint subtype: 4

Fix that by providing a user-facing error message for all ConstrType
values.  Also, remove the switch default case, so future additions to
ConstrType are caught.

Reported-by: Jian He <jian.universality@gmail.com>
Discussion: https://www.postgresql.org/message-id/CACJufxF8fmM=Dbm4pDFuV_nKGz2-No0k4YifhrF3-rjXTWJM3w@mail.gmail.com

5 months agoFix temporary memory leak in system table index scans
Peter Eisentraut [Tue, 3 Dec 2024 08:04:20 +0000 (09:04 +0100)]
Fix temporary memory leak in system table index scans

Commit 811af9786b introduced palloc() calls into systable_beginscan()
and systable_beginscan_ordered().  But there was no pfree(), as is the
usual style.

It turns out that an ANALYZE of a partitioned table can invoke many
thousand system table index scans, and this memory is not cleaned up
until the end of the command, so this can temporarily leak quite a bit
of memory.  Maybe there are improvements to be made at a higher level
about this, but for now, insert a couple of corresponding pfree()
calls to fix this particular issue.

Reported-by: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://www.postgresql.org/message-id/Z0XTfIq5xUtbkiIh@pryzbyj2023

5 months agoPerform provider-specific initialization in new functions.
Jeff Davis [Tue, 3 Dec 2024 07:20:32 +0000 (23:20 -0800)]
Perform provider-specific initialization in new functions.

Reviewed-by: Andreas Karlsson
Discussion: https://postgr.es/m/4548a168-62cd-457b-8d06-9ba7b985c477@proxel.se

5 months agodoc: Clarify some terms for pg_createsubscriber
Michael Paquier [Tue, 3 Dec 2024 07:21:07 +0000 (16:21 +0900)]
doc: Clarify some terms for pg_createsubscriber

The last section of pg_createsubscriber used the terms
"publication-name", "replication-slot-name", and "subscription-name".
These terms are not defined on the page, which was confusing, and the
intention is clearly to refer to the values one would give to the
options --publication, --subscription and --replication-slot.  Let's
simplify the documentation by mentioning the option switches, instead of
these terms.

Reported-by: Christophe Courtois
Author: Shubham Khanna
Reviewed-by: Vignesh C, Peter Smith
Discussion: https://postgr.es/m/173288198026.714.15127074046508836738@wrigleys.postgresql.org
Backpatch-through: 17

5 months agoFix unintentional behavior change in commit e9931bfb75.
Jeff Davis [Tue, 3 Dec 2024 05:59:02 +0000 (21:59 -0800)]
Fix unintentional behavior change in commit e9931bfb75.

Prior to that commit, there was special case to use ASCII case mapping
behavior for the libc provider with a single-byte encoding when that's
the default collation. Commit e9931bfb75 mistakenly eliminated that
special case; this commit restores it.

Discussion: https://postgr.es/m/01a104f0d2179d756261e90d96fd65c36ad6fcf0.camel@j-davis.com

5 months agoRevert "Introduce CompactAttribute array in TupleDesc"
David Rowley [Tue, 3 Dec 2024 04:12:38 +0000 (17:12 +1300)]
Revert "Introduce CompactAttribute array in TupleDesc"

This reverts commit d28dff3f6cd6a7562fb2c211ac0fb74a33ffd032.

Quite a large number of buildfarm members didn't like this commit and
it's not yet clear why.  Reverting this before too many animals turn
red.

Discussion: https://postgr.es/m/CAApHDvr9i6T5=iAwQCxFDgMsthr_obVxgwBaEJkC8KUH6yM3Hw@mail.gmail.com

5 months agoIntroduce CompactAttribute array in TupleDesc
David Rowley [Tue, 3 Dec 2024 03:50:59 +0000 (16:50 +1300)]
Introduce CompactAttribute array in TupleDesc

The new compact_attrs array stores a few select fields from
FormData_pg_attribute in a more compact way, using only 16 bytes per
column instead of the 104 bytes that FormData_pg_attribute uses.  Using
CompactAttribute allows performance-critical operations such as tuple
deformation to be performed without looking at the FormData_pg_attribute
element in TupleDesc which means fewer cacheline accesses.  With this
change, NAMEDATALEN could be increased with a much smaller negative impact
on performance.

For some workloads, tuple deformation can be the most CPU intensive part
of processing the query.  Some testing with 16 columns on a table
where the first column is variable length showed around a 10% increase in
transactions per second for an OLAP type query performing aggregation on
the 16th column.  However, in certain cases, the increases were much
higher, up to ~25% on one AMD Zen4 machine.

This also makes pg_attribute.attcacheoff redundant.  A follow-on commit
will remove it, thus shrinking the FormData_pg_attribute struct by 4
bytes.

Author: David Rowley
Discussion: https://postgr.es/m/CAApHDvrBztXP3yx=NKNmo3xwFAFhEdyPnvrDg3=M0RhDs+4vYw@mail.gmail.com
Reviewed-by: Andres Freund, Victor Yegorov
5 months agodoc Makefile: issue warning about chars that cannot be output
Bruce Momjian [Tue, 3 Dec 2024 02:25:12 +0000 (21:25 -0500)]
doc Makefile: issue warning about chars that cannot be output

A follow-up improvement to commit 641a5b7a144.

Reported-by: Tatsuo Ishii, Tom Lane
Discussion: https://postgr.es/m/20241126.182513.1752581942460106099.ishii@postgresql.org

Backpatch-through: master

5 months agoRework some code handling pg_subscription data in psql and pg_dump
Michael Paquier [Tue, 3 Dec 2024 00:48:12 +0000 (09:48 +0900)]
Rework some code handling pg_subscription data in psql and pg_dump

This commit fixes some inconsistencies found in the frontend code when
dealing with subscription catalog data.

The following changes are done:
- pg_subscription.h gains a EXPOSE_TO_CLIENT_CODE, so as more content
defined in pg_subscription.h becomes available in pg_subscription_d.h
for the frontend.
- In psql's describe.c, substream can be switched to use CppAsString2()
with its three LOGICALREP_STREAM_* values, with pg_subscription_d.h
included.
- pg_dump.c included pg_subscription.h, which is a header that should
only be used in the backend code.  The code is updated to use
pg_subscription_d.h instead.
- pg_dump stored all the data from pg_subscription in SubscriptionInfo
with only strings, and a good chunk of them are boolean and char values.
Using strings is not necessary, complicates the code (see for example
two_phase_disabled[] removed here), and is inconsistent with the way
other catalogs' data is handled.  The fields of SubscriptionInfo are
reordered to match with the order in its catalog, while on it.

Reviewed-by: Hayato Kuroda
Discussion: https://postgr.es/m/Z0lB2kp0ksHgmVuk@paquier.xyz

5 months agoRelationTruncate() must set DELAY_CHKPT_START.
Thomas Munro [Mon, 2 Dec 2024 20:27:05 +0000 (09:27 +1300)]
RelationTruncate() must set DELAY_CHKPT_START.

Previously, it set only DELAY_CHKPT_COMPLETE. That was important,
because it meant that if the XLOG_SMGR_TRUNCATE record preceded a
XLOG_CHECKPOINT_ONLINE record in the WAL, then the truncation would also
happen on disk before the XLOG_CHECKPOINT_ONLINE record was
written.

However, it didn't guarantee that the sync request for the truncation
was processed before the XLOG_CHECKPOINT_ONLINE record was written. By
setting DELAY_CHKPT_START, we guarantee that if an XLOG_SMGR_TRUNCATE
record is written to WAL before the redo pointer of a concurrent
checkpoint, the sync request queued by that operation must be processed
by that checkpoint, rather than being left for the following one.

This is a refinement of commit 412ad7a5563.  Back-patch to all supported
releases, like that commit.

Author: Robert Haas <robertmhaas@gmail.com>
Reported-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKG%2B-2rjGZC2kwqr2NMLBcEBp4uf59QT1advbWYF_uc%2B0Aw%40mail.gmail.com

5 months agoDeprecate MD5 passwords.
Nathan Bossart [Mon, 2 Dec 2024 19:30:07 +0000 (13:30 -0600)]
Deprecate MD5 passwords.

MD5 has been considered to be unsuitable for use as a cryptographic
hash algorithm for some time.  Furthermore, MD5 password hashes in
PostgreSQL are vulnerable to pass-the-hash attacks, i.e., knowing
the username and hashed password is sufficient to authenticate.
The SCRAM-SHA-256 method added in v10 is not subject to these
problems and is considered to be superior to MD5.

This commit marks MD5 password support in PostgreSQL as deprecated
and to be removed in a future release.  The documentation now
contains several deprecation notices, and CREATE ROLE and ALTER
ROLE now emit deprecation warnings when setting MD5 passwords.  The
warnings can be disabled by setting the md5_password_warnings
parameter to "off".

Reviewed-by: Greg Sabino Mullane, Jim Nasby
Discussion: https://postgr.es/m/ZwbfpJJol7lDWajL%40nathan

5 months agoAdd a planner support function for numeric generate_series().
Dean Rasheed [Mon, 2 Dec 2024 11:37:57 +0000 (11:37 +0000)]
Add a planner support function for numeric generate_series().

This allows the planner to estimate the number of rows returned by
generate_series(numeric, numeric[, numeric]), when the input values
can be estimated at plan time.

Song Jinzhou, reviewed by Dean Rasheed and David Rowley.

Discussion: https://postgr.es/m/tencent_F43E7F4DD50EF5986D1051DE8DE547910206%40qq.com
Discussion: https://postgr.es/m/tencent_1F6D5B9A1545E02FD7D0EE508DFD056DE50A%40qq.com