59802 Commits

Author SHA1 Message Date
Michael Paquier
4dd3087300 Remove assertion checking query ID in execMain.c
This assertion has been added by 24f520594809, but Alexander Lakhin has
proved that the ExecutorRun() one can be broken by using a PL function
that manipulates compute_query_id and track_activities, while the ones
in ExecutorFinish() and ExecutorEnd() could be triggered when cleaning
up portals at the beginning of a new query execution.

Discussion: https://postgr.es/m/b37d8e6c-e83d-e157-8865-1b2460a6aef2@gmail.com
2024-10-04 12:51:17 +09:00
Dean Rasheed
259a0a99fe Fix wrong varnullingrels error for MERGE WHEN NOT MATCHED BY SOURCE.
If a MERGE command contains WHEN NOT MATCHED BY SOURCE actions, the
source relation appears on the outer side of the join. Thus, any Vars
referring to the source in the merge join condition, actions, and
RETURNING list should be marked as nullable by the join, since they
are used in the ModifyTable node above the join. Note that this only
applies to the copy of join condition used in the executor to
distinguish MATCHED from NOT MATCHED BY SOURCE cases. Vars in the
original join condition, inside the join node itself, should not be
marked.

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

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

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

Discussion: https://postgr.es/m/18634-db5299c937877f2b%40postgresql.org
2024-10-03 13:48:32 +01:00
Dean Rasheed
dddb5640c6 Fix incorrect non-strict join recheck in MERGE WHEN NOT MATCHED BY SOURCE.
If a MERGE command contains WHEN NOT MATCHED BY SOURCE actions, the
merge join condition is used by the executor to distinguish MATCHED
from NOT MATCHED BY SOURCE cases. However, this qual is executed using
the output from the join subplan node, which nulls the output from the
source relation in the not matched case, and so the result may be
incorrect if the join condition is "non-strict" -- for example,
something like "src.col IS NOT DISTINCT FROM tgt.col".

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

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

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

Discussion: https://postgr.es/m/18634-db5299c937877f2b%40postgresql.org
2024-10-03 12:53:03 +01:00
Amit Langote
19531968e8 Replace Unicode apostrophe with ASCII apostrophe
In commit babb3993dbe9, I accidentally introduced a Unicode
apostrophe (U+2019). This commit replaces it with the ASCII
apostrophe (U+0027) for consistency.

Reported-by: Alexander Korotkov <aekorotkov@gmail.com>
Discussion: https://postgr.es/m/CAPpHfduNWMBjkJFtqXJremk6b6YQYO2s3_VEpnj-T_CaUNUYYQ@mail.gmail.com
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
a1c4c8a9e1 file_fdw: Add on_error and log_verbosity options to file_fdw.
In v17, the on_error and log_verbosity options were introduced for
the COPY command. This commit extends support for these options
to file_fdw.

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

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

Author: Atsushi Torikoshi
Reviewed-by: Masahiko Sawada, Fujii Masao
Discussion: https://postgr.es/m/ab59dad10490ea3734cf022b16c24cfd@oss.nttdata.com
2024-10-03 15:57:32 +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
554d3a18f3 Adjust json_manifest_per_file_callback API in one more place.
Oversight in commit d94cf5ca7 (and in my testing of same).

Discussion: https://postgr.es/m/9468.1727895630@sss.pgh.pa.us
2024-10-02 20:27:45 -04:00
Tom Lane
920d51979a Parse libpq's "keepalives" option more like other integer options.
Use pqParseIntParam (nee parse_int_param) instead of using strtol
directly.  This allows trailing whitespace, which the previous coding
didn't, and makes the spelling of the error message consistent with
other similar cases.

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

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

Yuto Sasaki (some further cleanup by me)

Discussion: https://postgr.es/m/TY2PR01MB36286A7B97B9A15793335D18C1772@TY2PR01MB3628.jpnprd01.prod.outlook.com
2024-10-02 17:30:36 -04:00
Robert Haas
d94cf5ca7f File size in a backup manifest should use uint64, not size_t.
size_t is the size of an object in memory, not the size of a file on disk.

Thanks to Tom Lane for noting the error.

Discussion: http://postgr.es/m/1865585.1727803933@sss.pgh.pa.us
2024-10-02 09:59:04 -04:00
Daniel Gustafsson
7b2822ecf9 doc: Missing markup, punctuation and wordsmithing
Various improvements to the documentation like adding missing
markup, improving punctuation, ensuring consistent spelling of
words and minor wordsmithing.

Author: Oleg Sibiryakov <o.sibiryakov@postgrespro.ru>
Discussion: https://postgr.es/m/b7d0a03c-107e-48c7-a5c9-2c6f73cdf78f@postgrespro.ru
2024-10-02 14:50:56 +02:00
Daniel Gustafsson
9c73395104 Add fastpaths for when no objects are found
If there are no objects found, there is no reason to inspect the
result columns and mallocing a zero-sized  (which will be 1 byte
in reality) heap buffer for it.  Add a fast-path for immediately
returning like how other object inspection functions are already
doing it.

Reviewed-by: Ranier Vilela <ranier.vf@gmail.com>
Discussion: https://postgr.es/m/C2F05B3C-1414-45DD-AE09-6FEE4D0F89BD@yesql.se
2024-10-02 13:08:55 +02:00
Daniel Gustafsson
1a123e3b13 Remove superfluous PQExpBuffer resetting
Since the buffer was just created, there is no reason to immediately
reset it.

Reviewed-by: Ranier Vilela <ranier.vf@gmail.com>
Discussion: https://postgr.es/m/C2F05B3C-1414-45DD-AE09-6FEE4D0F89BD@yesql.se
2024-10-02 13:07:31 +02:00
Daniel Gustafsson
94902b146f doc: Add link to login event trigger example
The login event trigger is not listed on the trigger firing matrix
since it's not fired by a command.  Add a link to the example code
page similar to how the other event triggers link to the matrix.

Reported-by: Marcos Pegoraro <marcos@f10.com.br>
Discussion: https://postgr.es/m/CAB-JLwYS+78rX02BZ3wJ9ykVrd2i3O1K+7jzvZKQ0evquyQiLQ@mail.gmail.com
2024-10-02 12:24:39 +02:00
Fujii Masao
17cc5f666f Fix inconsistent reporting of checkpointer stats.
Previously, the pg_stat_checkpointer view and the checkpoint completion
log message could show different numbers for buffers written
during checkpoints. The view only counted shared buffers,
while the log message included both shared and SLRU buffers,
causing inconsistencies.

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

Bump catalog version.

Author: Nitin Jadhav
Reviewed-by: Bharath Rupireddy, Michael Paquier, Kyotaro Horiguchi, Robert Haas
Reviewed-by: Andres Freund, vignesh C, Fujii Masao
Discussion: https://postgr.es/m/CAMm1aWb18EpT0whJrjG+-nyhNouXET6ZUw0pNYYAe+NezpvsAA@mail.gmail.com
2024-10-02 11:17:47 +09:00
Michael Paquier
506eede711 doc: Clarify name of files generated by pg_waldump --save-fullpage
The fork name is always separated with the block number by an underscore
in the names of the files generated, but the docs stuck them together
without a separator, which was confusing.

Author: Christoph Berg
Discussion: https://postgr.es/m/ZvxtSLiix9eceMRM@msg.df7cb.de
Backpatch-through: 16
2024-10-02 11:12:40 +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
Peter Eisentraut
983a588e0b initdb: Add new option "--no-data-checksums"
Right now this does nothing except override any earlier
--data-checksums option.  But the idea is that --data-checksums could
become the default, and then this option would allow forcing it off
instead.

Author: Greg Sabino Mullane <greg@turnstep.com>
Discussion: https://www.postgresql.org/message-id/flat/CAKAnmmKwiMHik5AHmBEdf5vqzbOBbcwEPHo4-PioWeAbzwcTOQ@mail.gmail.com
2024-10-01 10:50:30 -04:00
Peter Eisentraut
efd72a3d42 Tweak docs to reduce possible impact of data checksums
Author: Greg Sabino Mullane <greg@turnstep.com>
Discussion: https://www.postgresql.org/message-id/flat/CAKAnmmKwiMHik5AHmBEdf5vqzbOBbcwEPHo4-PioWeAbzwcTOQ@mail.gmail.com
2024-10-01 09:58:20 -04:00
Peter Eisentraut
10b721821d Use macro to define the number of enum values
Refactoring in the interest of code consistency, a follow-up to 2e068db56e31.

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

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

Discussion: https://postgr.es/m/CAJ7c6TMsiaV5urU_Pq6zJ2tXPDwk69-NKVh4AMN5XrRiM7N%2BGA%40mail.gmail.com
2024-10-01 09:30:24 -04:00
Robert Haas
fc1b2ce0ee Fix some pg_verifybackup issues reported by Coverity.
Commit 8dfd3129027969fdd2d9d294220c867d2efd84aa introduced a few
problems. verify_tar_file() forgot to free a buffer; the leak can't
add up to anything material, but might as well fix it.
precheck_tar_backup_file() intended to return after reporting an
error but didn't actually do so. member_copy_control_data() could
try to copy zero bytes (and maybe Coverity thinks it can even be
trying to copy a negative number of bytes).

Per discussion with Tom Lane.

Discussion: http://postgr.es/m/1240823.1727629418@sss.pgh.pa.us
2024-10-01 08:36:54 -04:00
Peter Eisentraut
9c2a6c5a5f Simplify checking for xlocale.h
Instead of XXX_IN_XLOCALE_H for several features XXX, let's just
include <xlocale.h> if HAVE_XLOCALE_H.  The reason for the extra
complication was apparently that some old glibc systems also had an
<xlocale.h>, and you weren't supposed to include it directly, but it's
gone now (as far as I can tell it was harmless to do so anyway).

Author: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/CWZBBRR6YA8D.8EHMDRGLCKCD%40neon.tech
2024-10-01 07:23:45 -04:00
Peter Eisentraut
ee4859123e jit: Use opaque pointers in all supported LLVM versions.
LLVM's opaque pointer change began in LLVM 14, but remained optional
until LLVM 16.  When commit 37d5babb added opaque pointer support, we
didn't turn it on for LLVM 14 and 15 yet because we didn't want to risk
weird bitcode incompatibility problems in released branches of
PostgreSQL.  (That might have been overly cautious, I don't know.)

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

Author: Thomas Munro <thomas.munro@gmail.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Discussions: https://postgr.es/m/CA%2BhUKGLhNs5geZaVNj2EJ79Dx9W8fyWUU3HxcpZy55sMGcY%3DiA%40mail.gmail.com
2024-10-01 06:10:15 -04:00
Peter Eisentraut
972c2cd288 jit: Require at least LLVM 14, if enabled.
Remove support for LLVM versions 10-13.  The default on all non-EOL'd
OSes represented in our build farm will be at least LLVM 14 when
PostgreSQL 18 ships.

Author: Thomas Munro <thomas.munro@gmail.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/CA%2BhUKGLhNs5geZaVNj2EJ79Dx9W8fyWUU3HxcpZy55sMGcY%3DiA%40mail.gmail.com
2024-10-01 04:49:11 -04:00
Daniel Gustafsson
1b4d52c355 doc: Mention the connstring key word for PGSERVICE
The documentation for the connection service file was mentioning
the environment variable early but not the connection string key
word until the last sentence and only then in an example.  This
adds the keyword in the first paragraph to make it clearer

Author: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Discussion: https://postgr.es/m/87r09ibpke.fsf@wibble.ilmari.org
2024-10-01 10:20:14 +02:00
Michael Paquier
cf4401fe6c Fix race condition in COMMIT PREPARED causing orphaned 2PC files
COMMIT PREPARED removes on-disk 2PC files near its end, but the state
checked if a file is on-disk or not gets read from shared memory while
not holding the two-phase state lock.

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

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

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

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

Mea culpa.

Author: wuchengwen
Discussion: https://postgr.es/m/tencent_A7F059B5136A359625C7B2E4A386B3C3F007@qq.com
Backpatch-through: 12
2024-10-01 15:44:03 +09:00
Tatsuo Ishii
3b1a377def Doc: replace unnecessary non-breaking space with ordinal space.
There were unnecessary non-breaking spaces (nbsp, U+00A0, 0xc2a0 in
UTF-8) in the docs.  This commit replaces them with ASCII spaces
(0x20).

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

Discussion: https://postgr.es/m/20240930.153404.202479334310259810.ishii%40postgresql.org
Reviewed-by: Yugo Nagata, Daniel Gustafsson
Backpatch-through: 17, 13
2024-10-01 11:34:34 +09:00
Michael Paquier
5deb56387e Expand assertion check for query ID reporting in executor
As formulated, the assertion added in the executor by 24f520594809 to
check that a query ID is set had two problems:
- track_activities may be disabled while compute_query_id is enabled,
causing the query ID to not be reported to pg_stat_activity.
- debug_query_string may not be set in some context.  The only path
where this would matter is visibly autovacuum, should parallel workers
be enabled there at some point.  This is not the case currently.

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

Discussion: https://postgr.es/m/Zvn5616oYXmpXyHI@paquier.xyz
2024-10-01 08:56:21 +09:00
Daniel Gustafsson
102de3be73 Add missing command for pg_maintain in comment
The comment in pg_class_aclmask_ext() which lists the allowed commands
for the pg_maintain role lacked LOCK TABLE.

Reported-by: Yusuke Sugie <btsugieyuusuke@oss.nttdata.com>
Reviewed-by: Yugo Nagata <nagata@sraoss.co.jp>
Discussion: https://postgr.es/m/034d3c60f5daba1919cd90f236b2e22d@oss.nttdata.com
2024-10-01 00:01:32 +02: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
Fujii Masao
a19f83f879 docs: Enhance the pg_stat_checkpointer view documentation.
This commit updates the documentation for the pg_stat_checkpointer view
to clarify what kind of checkpoints or restartpoints each counter tracks.
This makes it easier to understand the meaning of each counter.

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

Author: Fujii Masao
Reviewed-by: Anton A. Melnikov
Discussion: https://postgr.es/m/4640258e-d959-4cf0-903c-cd02389c3e05@oss.nttdata.com
2024-10-01 02:01:57 +09:00
Tom Lane
04c64e3fb3 Remove incorrect entries in pg_walsummary's getopt_long call.
For some reason this listed "-f" and "-w" as valid switches, though
the code doesn't implement any such thing nor do the docs mention
them.  The effect of this was that if you tried to use one of these
switches, you'd get an unhelpful error message.

Yusuke Sugie

Discussion: https://postgr.es/m/68e72a2a70f4d84c1c7847b13bcdaef8@oss.nttdata.com
2024-09-30 12:06:54 -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 9139aa19423b we were adding a restriction against ADD constraint
ONLY on partitioned tables (which is sensible) and apparently we thought
the DROP case had to be symmetrical.  However, it isn't, and the
comments about it are mistaken about the effect it would have.  Remove
this limitation.

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

Reviewed-by: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/202409261752.nbvlawkxsttf@alvherre.pgsql
Discussion: https://postgr.es/m/7682253a-6f79-6a92-00aa-267c4c412870@lab.ntt.co.jp
	(about commit 9139aa19423b, previously not registered)
2024-09-30 11:58:13 +02:00
Michael Paquier
4c7cd07aa6 Bump catalog version for change in VariableSetStmt
Oversight in dc68515968e8, as this breaks SQL functions with a SET
command.

Reported-by: Tom Lane
Discussion: https://postgr.es/m/1364409.1727673407@sss.pgh.pa.us
2024-09-30 14:52:03 +09:00
Michael Paquier
dc68515968 Show values of SET statements as constants in pg_stat_statements
This is a continuation of work like 11c34b342bd7, done to reduce the
bloat of pg_stat_statements by applying more normalization to query
entries.  This commit is able to detect and normalize values in
VariableSetStmt, resulting in:
SET conf_param = $1

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

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

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

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

Author: Greg Sabino Mullane, Michael Paquier
Discussion: https://postgr.es/m/36e5bffe-e989-194f-85c8-06e7bc88e6f7@amazon.com
Discussion: https://postgr.es/m/B44FA29D-EBD0-4DD9-ABC2-16F1CB087074@amazon.com
Discussion: https://postgr.es/m/CAKAnmmJtJY2jzQN91=2QAD2eAJAA-Per61eyO48-TyxEg-q0Rg@mail.gmail.com
2024-09-30 14:02:00 +09:00
Fujii Masao
559efce1d6 Add num_done counter to the pg_stat_checkpointer view.
Checkpoints can be skipped when the server is idle. The existing num_timed and
num_requested counters in pg_stat_checkpointer track both completed and
skipped checkpoints, but there was no way to count only the completed ones.

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

Bump catalog version.

Author: Anton A. Melnikov
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/9ea77f40-818d-4841-9dee-158ac8f6e690@oss.nttdata.com
2024-09-30 11:56:05 +09:00
Fujii Masao
20cfec896c reindexdb: Skip reindexing temporary tables and indexes.
Reindexing temp tables or indexes of other sessions is not allowed.
However, reindexdb in parallel mode previously listed them as
the objects to process, leading to failures.

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

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

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

Author: Fujii Masao
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/5f37ee56-14fb-44fe-9150-9eb97e10538b@oss.nttdata.com
2024-09-30 11:13:55 +09: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 24f520594809 in the recovery TAP test
027_stream_regress.pl where pg_stat_statements is always loaded.  In
this case, the executor path was taken because the index expression
needs to be flattened when building its IndexInfo.

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

Reported-by: Alexander Lakhin
Reviewed-by: Sami Imseih
Discussion: https://postgr.es/m/cf3547c1-498a-6a61-7b01-819f902a251f@gmail.com
2024-09-30 08:43:28 +09:00
Noah Misch
0d5a3d7574 Remove NULL dereference from RenameRelationInternal().
Defect in last week's commit aac2c9b4fde889d13f859c233c2523345e72d32b,
per Coverity.  Reaching this would need catalog corruption.  Back-patch
to v12, like that commit.
2024-09-29 15:54:25 -07:00
Tom Lane
e9339782a6 In passwordFromFile, don't leak the open file after stat failures.
Oversight in e882bcae0.  Per Coverity.
2024-09-29 13:40:03 -04:00
Noah Misch
c1ff2d8bc5 Avoid 037_invalid_database.pl hang under debug_discard_caches.
Back-patch to v12 (all supported versions).
2024-09-27 15:28:56 -07:00
Nathan Bossart
d8ebcac547 doc: Note that CREATE MATERIALIZED VIEW restricts search_path.
Since v17, CREATE MATERIALIZED VIEW has set search_path to
"pg_catalog, pg_temp" while running the query.  The docs for the
other commands that restrict search_path mention it, but the page
for CREATE MATERIALIZED VIEW does not.  Fix that.

Oversight in commit 4b74ebf726.

Author: Yugo Nagata
Reviewed-by: Jeff Davis
Discussion: https://postgr.es/m/20240805160502.d2a4975802a832b1e04afb80%40sraoss.co.jp
Backpatch-through: 17
2024-09-27 16:21:21 -05:00
Tom Lane
a3179ab692 Recalculate where-needed data accurately after a join removal.
Up to now, remove_rel_from_query() has done a pretty shoddy job
of updating our where-needed bitmaps (per-Var attr_needed and
per-PlaceHolderVar ph_needed relid sets).  It removed direct mentions
of the to-be-removed baserel and outer join, which is the minimum
amount of effort needed to keep the data structures self-consistent.
But it didn't account for the fact that the removed join ON clause
probably mentioned Vars of other relations, and those Vars might now
not be needed as high up in the join tree as before.  It's easy to
show cases where this results in failing to remove a lower outer join
that could also have been removed.

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

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

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

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

Discussion: https://postgr.es/m/18627-44f950eb6a8416c2@postgresql.org
2024-09-27 16:04:04 -04:00
Robert Haas
7f7474a8e4 Reindent pg_verifybackup.c. 2024-09-27 11:14:31 -04:00
Robert Haas
8dfd312902 pg_verifybackup: Verify tar-format backups.
This also works for compressed tar-format backups. However, -n must be
used, because we use pg_waldump to verify WAL, and it doesn't yet know
how to verify WAL that is stored inside of a tarfile.

Amul Sul, reviewed by Sravan Kumar and by me, and revised by me.
2024-09-27 08:40:24 -04:00
Fujii Masao
8410f738ad Fix typo in pg_walsummary/nls.mk.
Author: Koki Nakamura
Discussion: https://postgr.es/m/485c613d1db8de2e8169d5afd43e7f9e@oss.nttdata.com
2024-09-27 10:20:22 +09:00
Michael Paquier
09620ea091 Fix incorrect memory access in VACUUM FULL with invalid toast indexes
An invalid toast index is skipped in reindex_relation().  These would be
remnants of a failed REINDEX CONCURRENTLY and they should never been
rebuilt as there can only be one valid toast index at a time.

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

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

Reported-by: Alexander Lakhin
Author: Tender Wang
Discussion: https://postgr.es/m/18630-9aed99c38830657d@postgresql.org
Backpatch-through: 12
2024-09-27 09:40:09 +09:00
Michael Paquier
f762d99c87 Fix catalog data of new LO privilege functions
This commit improves the catalog data in pg_proc for the three functions
for has_largeobject_privilege(), introduced in 4eada203a5a8:
- Fix their descriptions (typos and consistency).
- Reallocate OIDs to be within the 8000-9999 range as required by
a6417078c414.

Bump catalog version.

Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/ZvUYR0V0dzWaLnsV@paquier.xyz
2024-09-27 07:26:29 +09:00