1
0
mirror of https://github.com/postgres/postgres.git synced 2025-10-28 11:55:03 +03:00
Commit Graph

4925 Commits

Author SHA1 Message Date
Michael Paquier
2e7c4abe5a 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
2024-10-07 12:36:00 +09:00
Tom Lane
68dfecbef2 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
2024-10-05 14:46:44 -04:00
Amit Langote
19531968e8 Replace Unicode apostrophe with ASCII apostrophe
In commit babb3993db, 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
2024-10-03 20:00:36 +09:00
Fujii Masao
e55f025b05 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
2024-10-03 15:59:16 +09:00
Fujii Masao
e7834a1a25 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
2024-10-03 15:55:37 +09:00
Amit Langote
babb3993db 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
2024-10-03 11:59:09 +09:00
Michael Paquier
e2bab2d792 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
2024-10-03 10:55:02 +09:00
Tom Lane
da8a4c1666 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
2024-10-01 16:53:54 -04:00
Tom Lane
7702337489 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
2024-09-30 17:57:12 -04:00
Alvaro Herrera
4dea33ce76 Don't disallow DROP of constraints ONLY on partitioned tables
This restriction seems to have come about due to some fuzzy thinking: in
commit 9139aa1942 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 9139aa1942, previously not registered)
2024-09-30 11:58:13 +02:00
Michael Paquier
6fd5071909 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 24f5205948 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
2024-09-30 08:43:28 +09:00
Noah Misch
0d5a3d7574 Remove NULL dereference from RenameRelationInternal().
Defect in last week's commit aac2c9b4fd,
per Coverity.  Reaching this would need catalog corruption.  Back-patch
to v12, like that commit.
2024-09-29 15:54:25 -07:00
Nathan Bossart
b52adbad46 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
2024-09-26 15:51:23 -05:00
Alvaro Herrera
dce507356a Turn 'if' condition around to avoid Svace complaint
The unwritten assumption of this code is that both events->head and
events->tail are NULL together (an empty list) or they aren't.  So the
code was testing events->head for nullness and using that as a cue to
deference events->tail, which annoys the Svace static code analyzer.
We can silence it by testing events->tail member instead, and add an
assertion about events->head to ensure it's all consistent.

This code is very old and as far as we know, there's never been a bug
report related to this, so there's no need to backpatch.

This was found by the ALT Linux Team using Svace.

Author: Alexander Kuznetsov <kuznetsovam@altlinux.org>
Discussion: https://postgr.es/m/6d0323c3-3f5d-4137-af73-98a5ab90e77c@altlinux.org
2024-09-25 16:42:02 +02:00
Noah Misch
aac2c9b4fd For inplace update durability, make heap_update() callers wait.
The previous commit fixed some ways of losing an inplace update.  It
remained possible to lose one when a backend working toward a
heap_update() copied a tuple into memory just before inplace update of
that tuple.  In catalogs eligible for inplace update, use LOCKTAG_TUPLE
to govern admission to the steps of copying an old tuple, modifying it,
and issuing heap_update().  This includes MERGE commands.  To avoid
changing most of the pg_class DDL, don't require LOCKTAG_TUPLE when
holding a relation lock sufficient to exclude inplace updaters.
Back-patch to v12 (all supported versions).  In v13 and v12, "UPDATE
pg_class" or "UPDATE pg_database" can still lose an inplace update.  The
v14+ UPDATE fix needs commit 86dc90056d,
and it wasn't worth reimplementing that fix without such infrastructure.

Reviewed by Nitin Motiani and (in earlier versions) Heikki Linnakangas.

Discussion: https://postgr.es/m/20231027214946.79.nmisch@google.com
2024-09-24 15:25:18 -07:00
Noah Misch
a07e03fd8f Fix data loss at inplace update after heap_update().
As previously-added tests demonstrated, heap_inplace_update() could
instead update an unrelated tuple of the same catalog.  It could lose
the update.  Losing relhasindex=t was a source of index corruption.
Inplace-updating commands like VACUUM will now wait for heap_update()
commands like GRANT TABLE and GRANT DATABASE.  That isn't ideal, but a
long-running GRANT already hurts VACUUM progress more just by keeping an
XID running.  The VACUUM will behave like a DELETE or UPDATE waiting for
the uncommitted change.

For implementation details, start at the systable_inplace_update_begin()
header comment and README.tuplock.  Back-patch to v12 (all supported
versions).  In back branches, retain a deprecated heap_inplace_update(),
for extensions.

Reported by Smolkin Grigory.  Reviewed by Nitin Motiani, (in earlier
versions) Heikki Linnakangas, and (in earlier versions) Alexander
Lakhin.

Discussion: https://postgr.es/m/CAMp+ueZQz3yDk7qg42hk6-9gxniYbp-=bG2mgqecErqR5gGGOA@mail.gmail.com
2024-09-24 15:25:18 -07:00
David Rowley
62ddf7ee9a Add ONLY support for VACUUM and ANALYZE
Since autovacuum does not trigger an ANALYZE for partitioned tables,
users must perform these manually.  However, performing a manual ANALYZE
on a partitioned table would always result in recursively analyzing each
partition and that could be undesirable as autovacuum takes care of that.
For partitioned tables that contain a large number of partitions, having
to analyze each partition could take an unreasonably long time, especially
so for tables with a large number of columns.

Here we allow the ONLY keyword to prefix the name of the table to allow
users to have ANALYZE skip processing partitions.  This option can also
be used with VACUUM, but there is no work to do if VACUUM ONLY is used on
a partitioned table.

This commit also changes the behavior of VACUUM	and ANALYZE for
inheritance parents.  Previously inheritance child tables would not be
processed when operating on the parent.  Now, by default we *do* operate
on the child tables.  ONLY can be used to obtain the old behavior.
The release notes should note this as an incompatibility.  The default
behavior has not changed for partitioned tables as these always
recursively processed the partitions.

Author: Michael Harris <harmic@gmail.com>
Discussion: https://postgr.es/m/CADofcAWATx_haD=QkSxHbnTsAe6+e0Aw8Eh4H8cXyogGvn_kOg@mail.gmail.com
Discussion: https://postgr.es/m/CADofcAXVbD0yGp_EaC9chmzsOoSai3jcfBCnyva3j0RRdRvMVA@mail.gmail.com
Reviewed-by: Jelte Fennema-Nio <postgres@jeltef.nl>
Reviewed-by: Melih Mutlu <m.melihmutlu@gmail.com>
Reviewed-by: Atsushi Torikoshi <torikoshia@oss.nttdata.com>
Reviewed-by: jian he <jian.universality@gmail.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
2024-09-24 18:03:40 +12:00
Michael Paquier
bbba59e69a Remove ATT_TABLE for ALTER TABLE ... ATTACH/DETACH
Attempting these commands for a non-partitioned table would result in a
failure when creating the relation in transformPartitionCmd().  This
gives the possibility to throw an error earlier with a much better error
message, thanks to d69a3f4d70.

The extra test cases are from me.  Note that FINALIZE uses a different
subcommand and it had no coverage for its failure path with
non-partitioned tables.

Author: Álvaro Herrera, Michael Paquier
Reviewed-by: Nathan Bossart
Discussion: https://postgr.es/m/202409190803.tnis52adt2n5@alvherre.pgsql
2024-09-24 08:59:08 +09:00
Tatsuo Ishii
40708acd65 Add memory/disk usage for more executor nodes.
This commit is similar to 95d6e9af07, expanding the idea to CTE scan,
table function scan and recursive union scan nodes so that the maximum
tuplestore memory or disk usage is shown with EXPLAIN ANALYZE command.

Also adjust show_storage_info() so that it accepts storage type and
storage size arguments instead of Tuplestorestate. This allows the
node types to share the formatting code using show_storage_info(). Due
to this show_material_info() and show_windowagg_info() are also
modified.

Reviewed-by: David Rowley
Discussion: https://postgr.es/m/20240918.211246.1127161704188186085.ishii%40postgresql.org
2024-09-23 16:34:24 +09:00
Alexander Korotkov
014f9f34d2 Move pg_wal_replay_wait() to xlogfuncs.c
This commit moves pg_wal_replay_wait() procedure to be a neighbor of
WAL-related functions in xlogfuncs.c.  The implementation of LSN waiting
continues to reside in the same place.

By proposal from Michael Paquier.

Reported-by: Peter Eisentraut
Discussion: https://postgr.es/m/18c0fa64-0475-415e-a1bd-665d922c5201%40eisentraut.org
2024-09-19 14:26:11 +03:00
Michael Paquier
d69a3f4d70 Introduce ATT_PARTITIONED_TABLE in tablecmds.c
Partitioned tables and normal tables have been relying on ATT_TABLE in
ATSimplePermissions() to produce error messages that depend on the
relation's relkind, because both relkinds currently support the same set
of ALTER TABLE subcommands.

A patch to restrict SET LOGGED/UNLOGGED for partitioned tables is under
discussion, and introducing ATT_PARTITIONED_TABLE makes subcommand
restrictions for partitioned tables easier to deal with, so let's add
one.  There is no functional change.

Author: Michael Paquier
Reviewed-by: Nathan Bossart
Discussion: https://postgr.es/m/Zt6cDnwSvnuLLnak@paquier.xyz
2024-09-19 12:22:56 +09:00
Tom Lane
918e21d251 Repair pg_upgrade for identity sequences with non-default persistence.
Since we introduced unlogged sequences in v15, identity sequences
have defaulted to having the same persistence as their owning table.
However, it is possible to change that with ALTER SEQUENCE, and
pg_dump tries to preserve the logged-ness of sequences when it doesn't
match (as indeed it wouldn't for an unlogged table from before v15).

The fly in the ointment is that ALTER SEQUENCE SET [UN]LOGGED fails
in binary-upgrade mode, because it needs to assign a new relfilenode
which we cannot permit in that mode.  Thus, trying to pg_upgrade a
database containing a mismatching identity sequence failed.

To fix, add syntax to ADD/ALTER COLUMN GENERATED AS IDENTITY to allow
the sequence's persistence to be set correctly at creation, and use
that instead of ALTER SEQUENCE SET [UN]LOGGED in pg_dump.  (I tried to
make SET [UN]LOGGED work without any pg_dump modifications, but that
seems too fragile to be a desirable answer.  This way should be
markedly faster anyhow.)

In passing, document the previously-undocumented SEQUENCE NAME option
that pg_dump also relies on for identity sequences; I see no value
in trying to pretend it doesn't exist.

Per bug #18618 from Anthony Hsu.
Back-patch to v15 where we invented this stuff.

Discussion: https://postgr.es/m/18618-d4eb26d669ed110a@postgresql.org
2024-09-17 15:53:35 -04:00
Alexander Korotkov
85b98b8d5a Minor cleanup related to pg_wal_replay_wait() procedure
* Rename $node_standby1 to $node_standby in 043_wal_replay_wait.pl as there
   is only one standby.
 * Remove useless debug printing in 043_wal_replay_wait.pl.
 * Fix typo in one check description in 043_wal_replay_wait.pl.
 * Fix some wording in comments and documentation.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/1d7b08f2-64a2-77fb-c666-c9a74c68eeda%40gmail.com
Reviewed-by: Alexander Lakhin
2024-09-17 22:50:43 +03:00
Peter Eisentraut
89f908a6d0 Add temporal FOREIGN KEY contraints
Add PERIOD clause to foreign key constraint definitions.  This is
supported for range and multirange types.  Temporal foreign keys check
for range containment instead of equality.

This feature matches the behavior of the SQL standard temporal foreign
keys, but it works on PostgreSQL's native ranges instead of SQL's
"periods", which don't exist in PostgreSQL (yet).

Reference actions ON {UPDATE,DELETE} {CASCADE,SET NULL,SET DEFAULT}
are not supported yet.

(previously committed as 34768ee361, reverted by 8aee330af55; this is
essentially unchanged from those)

Author: Paul A. Jungwirth <pj@illuminatedcomputing.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: jian he <jian.universality@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com
2024-09-17 11:29:30 +02:00
Peter Eisentraut
fc0438b4e8 Add temporal PRIMARY KEY and UNIQUE constraints
Add WITHOUT OVERLAPS clause to PRIMARY KEY and UNIQUE constraints.
These are backed by GiST indexes instead of B-tree indexes, since they
are essentially exclusion constraints with = for the scalar parts of
the key and && for the temporal part.

(previously committed as 46a0cd4cef, reverted by 46a0cd4cefb; the new
part is this:)

Because 'empty' && 'empty' is false, the temporal PK/UQ constraint
allowed duplicates, which is confusing to users and breaks internal
expectations.  For instance, when GROUP BY checks functional
dependencies on the PK, it allows selecting other columns from the
table, but in the presence of duplicate keys you could get the value
from any of their rows.  So we need to forbid empties.

This all means that at the moment we can only support ranges and
multiranges for temporal PK/UQs, unlike the original patch (above).
Documentation and tests for this are added.  But this could
conceivably be extended by introducing some more general support for
the notion of "empty" for other types.

Author: Paul A. Jungwirth <pj@illuminatedcomputing.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: jian he <jian.universality@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com
2024-09-17 11:29:30 +02:00
Tatsuo Ishii
95d6e9af07 Add memory/disk usage for Window aggregate nodes in EXPLAIN.
This commit is similar to 1eff8279d and expands the idea to Window
aggregate nodes so that users can know how much memory or disk the
tuplestore used.

This commit uses newly introduced tuplestore_get_stats() to inquire this
information and add some additional output in EXPLAIN ANALYZE to
display the information for the Window aggregate node.

Reviewed-by: David Rowley, Ashutosh Bapat, Maxim Orlov, Jian He
Discussion: https://postgr.es/m/20240706.202254.89740021795421286.ishii%40postgresql.org
2024-09-17 14:38:53 +09:00
David Rowley
9fba1ed294 Adjust tuplestore stats API
1eff8279d added an API to tuplestore.c to allow callers to obtain
storage telemetry data.  That API wasn't quite good enough for callers
that perform tuplestore_clear() as the telemetry functions only
accounted for the current state of the tuplestore, not the maximums
before tuplestore_clear() was called.

There's a pending patch that would like to add tuplestore telemetry
output to EXPLAIN ANALYZE for WindowAgg.  That node type uses
tuplestore_clear() before moving to the next window partition and we
want to show the maximum space used, not the space used for the final
partition.

Reviewed-by: Tatsuo Ishii, Ashutosh Bapat
Discussion: https://postgres/m/CAApHDvoY8cibGcicLV0fNh=9JVx9PANcWvhkdjBnDCc9Quqytg@mail.gmail.com
2024-09-12 16:02:01 +12:00
Richard Guo
247dea89f7 Introduce an RTE for the grouping step
If there are subqueries in the grouping expressions, each of these
subqueries in the targetlist and HAVING clause is expanded into
distinct SubPlan nodes.  As a result, only one of these SubPlan nodes
would be converted to reference to the grouping key column output by
the Agg node; others would have to get evaluated afresh.  This is not
efficient, and with grouping sets this can cause wrong results issues
in cases where they should go to NULL because they are from the wrong
grouping set.  Furthermore, during re-evaluation, these SubPlan nodes
might use nulled column values from grouping sets, which is not
correct.

This issue is not limited to subqueries.  For other types of
expressions that are part of grouping items, if they are transformed
into another form during preprocessing, they may fail to match lower
target items.  This can also lead to wrong results with grouping sets.

To fix this issue, we introduce a new kind of RTE representing the
output of the grouping step, with columns that are the Vars or
expressions being grouped on.  In the parser, we replace the grouping
expressions in the targetlist and HAVING clause with Vars referencing
this new RTE, so that the output of the parser directly expresses the
semantic requirement that the grouping expressions be gotten from the
grouping output rather than computed some other way.  In the planner,
we first preprocess all the columns of this new RTE and then replace
any Vars in the targetlist and HAVING clause that reference this new
RTE with the underlying grouping expressions, so that we will have
only one instance of a SubPlan node for each subquery contained in the
grouping expressions.

Bump catversion because this changes the querytree produced by the
parser.

Thanks to Tom Lane for the idea to invent a new kind of RTE.

Per reports from Geoff Winkless, Tobias Wendorff, Richard Guo from
various threads.

Author: Richard Guo
Reviewed-by: Ashutosh Bapat, Sutou Kouhei
Discussion: https://postgr.es/m/CAMbWs4_dp7e7oTwaiZeBX8+P1rXw4ThkZxh1QG81rhu9Z47VsQ@mail.gmail.com
2024-09-10 12:35:34 +09:00
Masahiko Sawada
bb77752342 Add WAL usage reporting to ANALYZE VERBOSE output.
This change adds WAL usage reporting to the output of ANALYZE VERBOSE
and autoanalyze reports. It aligns the analyze output with VACUUM,
providing consistency. Additionally, it aids in troubleshooting cases
where WAL records are generated during analyze operations.

Author: Anthonin Bonnefoy
Reviewed-by: Masahiko Sawada
Discussion: https://postgr.es/m/CAO6_Xqr__kTTCLkftqS0qSCm-J7_xbRG3Ge2rWhucxQJMJhcRA%40mail.gmail.com
2024-09-09 14:56:08 -07:00
Tom Lane
218527d014 Don't bother checking the result of SPI_connect[_ext] anymore.
SPI_connect/SPI_connect_ext have not returned any value other than
SPI_OK_CONNECT since commit 1833f1a1c in v10; any errors are thrown
via ereport.  (The most likely failure is out-of-memory, which has
always been thrown that way, so callers had better be prepared for
such errors.)  This makes it somewhat pointless to check these
functions' result, and some callers within our code haven't been
bothering; indeed, the only usage example within spi.sgml doesn't
bother.  So it's likely that the omission has propagated into
extensions too.

Hence, let's standardize on not checking, and document the return
value as historical, while not actually changing these functions'
behavior.  (The original proposal was to change their return type
to "void", but that would needlessly break extensions that are
conforming to the old practice.)  This saves a small amount of
boilerplate code in a lot of places.

Stepan Neretin

Discussion: https://postgr.es/m/CAMaYL5Z9Uk8cD9qGz9QaZ2UBJFOu7jFx5Mwbznz-1tBbPDQZow@mail.gmail.com
2024-09-09 12:18:34 -04:00
Michael Paquier
5bbdfa8a18 Fix waits of REINDEX CONCURRENTLY for indexes with predicates or expressions
As introduced by f9900df5f9, a REINDEX CONCURRENTLY job done for an
index with predicates or expressions would set PROC_IN_SAFE_IC in its
MyProc->statusFlags, causing it to be ignored by other concurrent
operations.

Such concurrent index rebuilds should never be ignored, as a predicate
or an expression could call a user-defined function that accesses a
different table than the table where the index is rebuilt.

A test that uses injection points is added, backpatched down to 17.
Michail has proposed a different test, but I have added something
simpler with more coverage.

Oversight in f9900df5f9.

Author: Michail Nikolaev
Discussion: https://postgr.es/m/CANtu0oj9A3kZVduFTG0vrmGnKB+DCHgEpzOp0qAyOgmks84j0w@mail.gmail.com
Backpatch-through: 14
2024-09-09 13:49:36 +09:00
Michael Paquier
d8df7ac5c0 Update extension lookup routines to use the syscache
The following routines are changed to use the syscache entries added for
pg_extension in 490f869d92:
- get_extension_oid()
- get_extension_name()
- get_extension_schema()

A catalog scan is costly and could easily lead to a noticeable
performance impact when called once or more per query, so this is going
to be helpful for developers for extension data lookups.

Author: Andrei Lepikhov
Reviewed-by: Jelte Fennema-Nio
Discussion: https://postgr.es/m/529295b2-6ba9-4dae-acd1-20a9c6fb8f9a@gmail.com
2024-09-07 20:20:46 +09:00
Jeff Davis
06421b0843 Remove lc_collate_is_c().
Instead just look up the collation and check collate_is_c field.

Author: Andreas Karlsson
Discussion: https://postgr.es/m/60929555-4709-40a7-b136-bcb44cff5a3c@proxel.se
2024-09-04 14:35:25 -07:00
Michael Paquier
a68159ff2b Unify some error messages to ease work of translators
This commit updates a couple of error messages around control file data,
GUCs and server settings, unifying to the same message where possible.
This reduces the translation burden a bit.

Author: Peter Smith
Discussion: https://postgr.es/m/CAHut+Pv-kSN8SkxSdoHano_wPubqcg5789ejhCDZAcLFceBR-w@mail.gmail.com
2024-09-04 14:53:18 +09:00
Michael Paquier
4236825197 Fix typos and grammar in code comments and docs
Author: Alexander Lakhin
Discussion: https://postgr.es/m/f7e514cf-2446-21f1-a5d2-8c089a6e2168@gmail.com
2024-09-03 14:49:04 +09:00
Michael Paquier
c7cd2d6ed0 Define PG_TBLSPC_DIR for path pg_tblspc/ in data folder
Similarly to 2065ddf5e3, this introduces a define for "pg_tblspc".
This makes the style more consistent with the existing PG_STAT_TMP_DIR,
for example.

There is a difference with the other cases with the introduction of
PG_TBLSPC_DIR_SLASH, required in two places for recovery and backups.

Author: Bertrand Drouvot
Reviewed-by: Ashutosh Bapat, Álvaro Herrera, Yugo Nagata, Michael
Paquier
Discussion: https://postgr.es/m/ZryVvjqS9SnV1GPP@ip-10-97-1-34.eu-west-3.compute.internal
2024-09-03 09:11:54 +09:00
Michael Paquier
a83a944e9f Rename pg_sequence_read_tuple() to pg_get_sequence_data()
This commit removes log_cnt from the tuple returned by the SQL function.
This field is an internal counter that tracks when a WAL record should
be generated for a sequence, and it is reset each time the sequence is
restored or recovered.  It is not necessary to rebuild the sequence DDL
commands for pg_dump and pg_upgrade where this function is used.  The
field can still be queried with a scan of the "table" created
under-the-hood for a sequence.

Issue noticed while hacking on a feature that can rely on this new
function rather than pg_sequence_last_value(), aimed at making sequence
computation more easily pluggable.

Bump catalog version.

Reviewed-by: Nathan Bossart
Discussion: https://postgr.es/m/Zsvka3r-y2ZoXAdH@paquier.xyz
2024-08-30 08:49:24 +09:00
Peter Eisentraut
edee0c621d Message style improvements 2024-08-29 14:43:34 +02:00
Peter Eisentraut
4d68a04324 Disallow USING clause when altering type of generated column
This does not make sense.  It would write the output of the USING
clause into the converted column, which would violate the generation
expression.  This adds a check to error out if this is specified.

There was a test for this, but that test errored out for a different
reason, so it was not effective.

Reported-by: Jian He <jian.universality@gmail.com>
Reviewed-by: Yugo NAGATA <nagata@sraoss.co.jp>
Discussion: https://www.postgresql.org/message-id/flat/c7083982-69f4-4b14-8315-f9ddb20b9834%40eisentraut.org
2024-08-29 09:06:15 +02:00
Michael Paquier
9f87da1cff Refactor some code for ALTER TABLE SET LOGGED/UNLOGGED in tablecmds.c
Both sub-commands use the same routine to switch the relpersistence of a
relation, duplicated the same checks, and used a style inconsistent with
access methods and tablespaces.

SET LOGEED/UNLOGGED is refactored to avoid any duplication, setting the
reason why a relation rewrite happens within ATPrepChangePersistence().
This shaves some code.

Discussion: https://postgr.es/m/ZiiyGFTBNkqcMQi_@paquier.xyz
2024-08-29 15:31:30 +09:00
Peter Eisentraut
6654bb9204 Add prefetching support on macOS
macOS doesn't have posix_fadvise(), but fcntl() with the F_RDADVISE
command does the same thing.

Some related documentation has been generalized to not mention
posix_advise() specifically anymore.

Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/0827edec-1317-4917-a186-035eb1e3241d%40eisentraut.org
2024-08-28 07:28:27 +02:00
Alexander Korotkov
3890d90c15 Revert support for ALTER TABLE ... MERGE/SPLIT PARTITION(S) commands
This commit reverts 1adf16b8fb, 87c21bb941, and subsequent fixes and
improvements including df64c81ca9, c99ef1811a, 9dfcac8e15, 885742b9f8,
842c9b2705, fcf80c5d5f, 96c7381c4c, f4fc7cb54b, 60ae37a8bc, 259c96fa8f,
449cdcd486, 3ca43dbbb6, 2a679ae94e, 3a82c689fd, fbd4321fd5, d53a4286d7,
c086896625, 4e5d6c4091, 04158e7fa3.

The reason for reverting is security issues related to repeatable name lookups
(CVE-2014-0062).  Even though 04158e7fa3 solved part of the problem, there
are still remaining issues, which aren't feasible to even carefully analyze
before the RC deadline.

Reported-by: Noah Misch, Robert Haas
Discussion: https://postgr.es/m/20240808171351.a9.nmisch%40google.com
Backpatch-through: 17
2024-08-24 18:48:48 +03:00
Alexander Korotkov
04158e7fa3 Avoid repeated table name lookups in createPartitionTable()
Currently, createPartitionTable() opens newly created table using its name.
This approach is prone to privilege escalation attack, because we might end
up opening another table than we just created.

This commit address the issue above by opening newly created table by its
OID.  It appears to be tricky to get a relation OID out of ProcessUtility().
We have to extend TableLikeClause with new newRelationOid field, which is
filled within ProcessUtility() to be further accessed by caller.

Security: CVE-2014-0062
Reported-by: Noah Misch
Discussion: https://postgr.es/m/20240808171351.a9.nmisch%40google.com
Reviewed-by: Pavel Borisov, Dmitry Koval
2024-08-22 09:50:48 +03:00
Richard Guo
9bb842f95e Small code simplification
Apply the same code simplification to ATExecAddColumn as was done in
7ff9afbbd: apply GETSTRUCT() once instead of doing it repeatedly in
the same function.

Author: Tender Wang
Discussion: https://postgr.es/m/CAHewXNkO9+U437jvKT14s0MCu6Qpf6G-p2mZK5J9mAi4cHDgpQ@mail.gmail.com
2024-08-22 11:41:08 +09:00
Tom Lane
86488cdf12 Disallow creating binary-coercible casts involving range types.
For a long time we have forbidden binary-coercible casts to or from
composite and array types, because such a cast cannot work correctly:
the type OID embedded in the value would need to change, but it won't
in a binary coercion.  That reasoning applies equally to range types,
but we overlooked installing a similar restriction here when we
invented range types.  Do so now.

Given the lack of field complaints, we won't change this in stable
branches, but it seems not too late for v17.

Per discussion of a problem noted by Peter Eisentraut.

Discussion: https://postgr.es/m/076968e1-0852-40a9-bc0b-117cd3f0e43c@eisentraut.org
2024-08-21 12:00:03 -04:00
Robert Haas
c01743aa48 Show number of disabled nodes in EXPLAIN ANALYZE output.
Now that disable_cost is not included in the cost estimate, there's
no visible sign in EXPLAIN output of which plan nodes are disabled.
Fix that by propagating the number of disabled nodes from Path to
Plan, and then showing it in the EXPLAIN output.

There is some question about whether this is a desirable change.
While I personally believe that it is, it seems best to make it a
separate commit, in case we decide to back out just this part, or
rework it.

Reviewed by Andres Freund, Heikki Linnakangas, and David Rowley.

Discussion: http://postgr.es/m/CA+TgmoZ_+MS+o6NeGK2xyBv-xM+w1AfFVuHE4f_aq6ekHv7YSQ@mail.gmail.com
2024-08-21 10:14:35 -04:00
Peter Eisentraut
7ff9afbbd1 Small code simplification
Apply GETSTRUCT() once instead of doing it repeatedly in the same
function.  This simplifies the notation and makes the function's
structure more similar to the surrounding ones.

Discussion: https://www.postgresql.org/message-id/flat/a368248e-69e4-40be-9c07-6c3b5880b0a6@eisentraut.org
2024-08-21 09:21:25 +02:00
Tomas Vondra
5cb902e9d5 Explain dropdb can't use syscache because of TOAST
Add a comment explaining dropdb() can't rely on syscache. The issue with
flattened rows was fixed by commit 0f92b230f8, but better to have
a clear explanation why the systable scan is necessary. The other places
doing in-place updates on pg_database have the same comment.

Suggestion and patch by Yugo Nagata. Backpatch to 12, same as the fix.

Author: Yugo Nagata
Backpatch-through: 12
Discussion: https://postgr.es/m/CAJTYsWWNkCt+-UnMhg=BiCD3Mh8c2JdHLofPxsW3m2dkDFw8RA@mail.gmail.com
2024-08-19 13:31:51 +02:00
Tomas Vondra
0f92b230f8 Fix DROP DATABASE for databases with many ACLs
Commit c66a7d75e6 modified DROP DATABASE so that if interrupted, the
database is known to be in an invalid state and can only be dropped.
This is done by setting a flag using an in-place update, so that it's
not lost in case of rollback.

For databases with many ACLs, this may however fail like this:

  ERROR:  wrong tuple length

This happens because with many ACLs, the pg_database.datacl attribute
gets TOASTed. The dropdb() code reads the tuple from the syscache, which
means it's detoasted. But the in-place update expects the tuple length
to match the on-disk tuple.

Fixed by reading the tuple from the catalog directly, not from syscache.

Report and fix by Ayush Tiwari. Backpatch to 12. The DROP DATABASE fix
was backpatched to 11, but 11 is EOL at this point.

Reported-by: Ayush Tiwari
Author: Ayush Tiwari
Reviewed-by: Tomas Vondra
Backpatch-through: 12
Discussion: https://postgr.es/m/CAJTYsWWNkCt+-UnMhg=BiCD3Mh8c2JdHLofPxsW3m2dkDFw8RA@mail.gmail.com
2024-08-19 00:04:48 +02:00
Noah Misch
64740853f0 Fix comments on wal_level=minimal, CREATE TABLESPACE and CREATE DATABASE.
Commit 97ddda8a82 removed the rmtree()
behavior from XLOG_TBLSPC_CREATE, obsoleting that part of the comment.
The comment's point about XLOG_DBASE_CREATE was wrong when commit
fa0f466d53 introduced the point.  (It
would have been accurate if that commit had predated commit
fbcbc5d06f introducing the second
checkpoint of CREATE DATABASE.)  Nothing can skip log_smgrcreate() on
the basis of wal_level=minimal, so don't comment on that.

Commit c6b92041d3 expanded WAL skipping
from five specific operations to relfilenodes generally, hence the
CreateDatabaseUsingFileCopy() comment change.

Discussion: https://postgr.es/m/20231008022204.cc@rfd.leadboat.com
2024-08-18 12:03:59 -07:00