postgresql.git
2 years agoFix JSON error reporting for many cases of erroneous string values.
Tom Lane [Mon, 13 Mar 2023 19:19:00 +0000 (15:19 -0400)]
Fix JSON error reporting for many cases of erroneous string values.

The majority of error exit cases in json_lex_string() failed to
set lex->token_terminator, causing problems for the error context
reporting code: it would see token_terminator less than token_start
and do something more or less nuts.  In v14 and up the end result
could be as bad as a crash in report_json_context().  Older
versions accidentally avoided that fate; but all versions produce
error context lines that are far less useful than intended,
because they'd stop at the end of the prior token instead of
continuing to where the actually-bad input is.

To fix, invent some macros that make it less notationally painful
to do the right thing.  Also add documentation about what the
function is actually required to do; and in >= v14, add an assertion
in report_json_context about token_terminator being sufficiently
far advanced.

Per report from Nikolay Shaplov.  Back-patch to all supported
versions.

Discussion: https://postgr.es/m/7332649.x5DLKWyVIX@thinkpad-pgpro

2 years agoFix failure to detect some cases of improperly-nested aggregates.
Tom Lane [Mon, 13 Mar 2023 16:40:28 +0000 (12:40 -0400)]
Fix failure to detect some cases of improperly-nested aggregates.

check_agg_arguments_walker() supposed that it needn't descend into
the arguments of a lower-level aggregate function, but this is
just wrong in the presence of multiple levels of sub-select.  The
oversight would lead to executor failures on queries that should
be rejected.  (Prior to v11, they actually were rejected, thanks
to a "redundant" execution-time check.)

Per bug #17835 from Anban Company.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/17835-4f29f3098b2d0ba4@postgresql.org

2 years agoAdd a DEFAULT option to COPY FROM
Andrew Dunstan [Mon, 13 Mar 2023 14:01:56 +0000 (10:01 -0400)]
Add a DEFAULT option to COPY  FROM

This allows for a string which if an input field matches causes the
column's default value to be inserted. The advantage of this is that
the default can be inserted in some rows and not others, for which
non-default data is available.

The file_fdw extension is also modified to take allow use of this
option.

Israel Barth Rubio

Discussion: https://postgr.es/m/CAO_rXXAcqesk6DsvioOZ5zmeEmpUN5ktZf-9=9yu+DTr0Xr8Uw@mail.gmail.com

2 years agoFix MERGE command tag for actions blocked by BEFORE ROW triggers.
Dean Rasheed [Mon, 13 Mar 2023 11:09:39 +0000 (11:09 +0000)]
Fix MERGE command tag for actions blocked by BEFORE ROW triggers.

This ensures that the row count in the command tag for a MERGE is
correctly computed in the case where UPDATEs or DELETEs are skipped
due to a BEFORE ROW trigger returning NULL (the INSERT case was
already handled correctly by ExecMergeNotMatched() calling
ExecInsert()).

Back-patch to v15, where MERGE was introduced.

Discussion: https://postgr.es/m/CAEZATCU8XEmR0JWKDtyb7iZ%3DqCffxS9uyJt0iOZ4TV4RT%2Bow1w%40mail.gmail.com

2 years agoFix concurrent update issues with MERGE.
Dean Rasheed [Mon, 13 Mar 2023 10:22:22 +0000 (10:22 +0000)]
Fix concurrent update issues with MERGE.

If MERGE attempts an UPDATE or DELETE on a table with BEFORE ROW
triggers, or a cross-partition UPDATE (with or without triggers), and
a concurrent UPDATE or DELETE happens, the merge code would fail.

In some cases this would lead to a crash, while in others it would
cause the wrong merge action to be executed, or no action at all. The
immediate cause of the crash was the trigger code calling
ExecGetUpdateNewTuple() as part of the EPQ mechanism, which fails
because during a merge ri_projectNew is NULL, since merge has its own
per-action projection information, which ExecGetUpdateNewTuple() knows
nothing about.

Fix by arranging for the trigger code to exit early, returning the
TM_Result and TM_FailureData information, if a concurrent modification
is detected, allowing the merge code to do the necessary EPQ handling
in its own way. Similarly, prevent the cross-partition update code
from doing any EPQ processing for a merge, allowing the merge code to
work out what it needs to do.

This leads to a number of simplifications in nodeModifyTable.c. Most
notably, the ModifyTableContext->GetUpdateNewTuple() callback is no
longer needed, and mergeGetUpdateNewTuple() can be deleted, since
there is no longer any requirement for get-update-new-tuple during a
merge. Similarly, ModifyTableContext->cpUpdateRetrySlot is no longer
needed. Thus ExecGetUpdateNewTuple() and the retry_slot handling of
ExecCrossPartitionUpdate() can be restored to how they were in v14,
before the merge code was added, and ExecMergeMatched() no longer
needs any special-case handling for cross-partition updates.

While at it, tidy up ExecUpdateEpilogue() a bit, making it handle
recheckIndexes locally, rather than passing it in as a parameter,
ensuring that it is freed properly. This dates back to when it was
split off from ExecUpdate() to support merge.

Per bug #17809 from Alexander Lakhin, and follow-up investigation of
bug #17792, also from Alexander Lakhin.

Back-patch to v15, where MERGE was introduced, taking care to preserve
backwards-compatibility of the trigger API in v15 for any extensions
that might use it.

Discussion:
  https://postgr.es/m/17809-9e6650bef133f0fe%40postgresql.org
  https://postgr.es/m/17792-0f89452029662c36%40postgresql.org

2 years agoFix expected test output
Peter Eisentraut [Mon, 13 Mar 2023 10:15:21 +0000 (11:15 +0100)]
Fix expected test output

For builds without lz4, for 208bf364a9.

2 years agoRemove incidental md5() function uses from main regression tests
Peter Eisentraut [Mon, 13 Mar 2023 09:15:44 +0000 (10:15 +0100)]
Remove incidental md5() function uses from main regression tests

Most of these calls were to generate some random data.  These can be
replaced by appropriately adapted sha256() calls.  To keep the diff
smaller, we wrap this into a helper function that produces the same
output format and length as the md5() call.

This will eventually allow these tests to pass in OpenSSL FIPS mode
(which does not allow MD5 use).

Similar work for other test suites will follow later.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/dbbd927f-ef1f-c9a1-4ec6-c759778ac852@enterprisedb.com

2 years agoImprove support for UNICODE collation on older ICU
Peter Eisentraut [Mon, 13 Mar 2023 08:04:41 +0000 (09:04 +0100)]
Improve support for UNICODE collation on older ICU

The recently added standard collation UNICODE (0d21d4b9bc) doesn't
give consistent results on some build farm members with old ICU
versions.  Apparently, the ICU locale specification 'und' (language
tag style) misbehaves on some older ICU versions.  Replacing it with
'' (ICU locale ID style) fixes it at least on some OS versions.  Let's
see what the build farm says.

2 years agoFix inconsistent error handling for GSS encryption in PQconnectPoll()
Michael Paquier [Mon, 13 Mar 2023 07:36:20 +0000 (16:36 +0900)]
Fix inconsistent error handling for GSS encryption in PQconnectPoll()

The error cases for TLS and GSS encryption were inconsistent.  After TLS
fails, the connection is marked as dead and follow-up calls of
PQconnectPoll() would return immediately, but GSS encryption was not
doing that, so the connection would still have been allowed to enter the
GSS handling code.  This was handled incorrectly when gssencmode was set
to "require".  "prefer" was working correctly, and this could not happen
under "disable" as GSS encryption would not be attempted.

This commit makes the error handling of GSS encryption on par with TLS
portion, fixing the case of gssencmode=require.

Reported-by: Jacob Champion
Author: Michael Paquier
Reviewed-by: Jacob Champion, Stephen Frost
Discussion: https://postgr.es/m/23787477-5fe1-a161-6d2a-e459f74c4713@timescale.com
Backpatch-through: 12

2 years agocirrus/ccache: Use G rather than GB suffix
Peter Eisentraut [Mon, 13 Mar 2023 06:23:45 +0000 (07:23 +0100)]
cirrus/ccache: Use G rather than GB suffix

The former being the documented spelling.

Author: Justin Pryzby <pryzbyj@telsasoft.com>
Discussion: https://www.postgresql.org/message-id/20230203142656.GA1653%40telsasoft.com

2 years agomeson: Make auto the default of the ssl option
Peter Eisentraut [Mon, 13 Mar 2023 05:46:09 +0000 (06:46 +0100)]
meson: Make auto the default of the ssl option

The 'ssl' option is of type 'combo', but we add a choice 'auto' that
simulates the behavior of a feature option.  This way, openssl is used
automatically by default if present, but we retain the ability to
potentially select another ssl library.

Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/ad65ffd1-a9a7-fda1-59c6-f7dc763c3051%40enterprisedb.com

2 years agoRefactor and improve tests of pg_walinspect
Michael Paquier [Mon, 13 Mar 2023 04:03:29 +0000 (13:03 +0900)]
Refactor and improve tests of pg_walinspect

The regression tests of pg_walinspect are reworked on a few aspects:
- Reorganization on the validation checks done for the start and end
LSNs on the six SQL functions currently available in 1.1.
- Addition of a few patterns doing bound checks for invalid start LSN,
invalid end LSN, and failures in reading LSN positions, for anything
that's been missing.
- Use of a consistent style across the whole, limiting blank lines
across the queries.
- Addition of a new test script for upgrades.  For the time being, this
is straight-forward with a check that the upgrade from 1.0 works
correctly.  This will be made more complicated once the interface of
this extension is reworked in 1.1 with a follow-up patch.

Most of the contents of this commit are extracted from a larger patch by
the same author, largely reorganized by me to minimize the differences
with the upcoming work aimed to lift the bound checks on the input LSNs
used by the functions of this extension.

Author: Bharath Rupireddy, Michael Paquier
Discussion: https://postgr.es/m/CALj2ACU0_q-o4DSweyaW9NO1KBx-QkN6G_OzYQvpjf3CZVASkg@mail.gmail.com

2 years agoWork around implementation restriction in adjust_appendrel_attrs.
Tom Lane [Sun, 12 Mar 2023 18:20:34 +0000 (14:20 -0400)]
Work around implementation restriction in adjust_appendrel_attrs.

adjust_appendrel_attrs can't transfer nullingrel labeling to a non-Var
translation expression (mainly because it's too late to wrap such an
expression in a PlaceHolderVar).  I'd supposed in commit 2489d76c4
that that restriction was unreachable because we'd not attempt to push
problematic clauses down to an appendrel child relation.  I forgot that
set_append_rel_size blindly converts all the parent rel's joininfo
clauses to child clauses, and that list could well contain clauses
from above a nulling outer join.

We might eventually have to devise a direct fix for this implementation
restriction, but for now it seems enough to filter out troublesome
clauses while constructing the child's joininfo list.  Such clauses
are certainly not useful while constructing paths for the child rel;
they'll have to be applied later when we join the completed appendrel
to something else.  So we don't need them here, and omitting them from
the list should save a few cycles while processing the child rel.

Per bug #17832 from Marko Tiikkaja.

Discussion: https://postgr.es/m/17832-d0a8106cdf1b722e@postgresql.org

2 years agoMark unsafe_tests module as not runnable with installcheck
Andrew Dunstan [Sun, 12 Mar 2023 13:00:32 +0000 (09:00 -0400)]
Mark unsafe_tests module as not runnable with installcheck

This was an omission in the original creation of the module.

Also slightly adjust some wording to avoid a double "is".

Backpatch the non-meson piece of this to release 12, where the module
was introduced.

Discussion: https://postgr.es/m/be869e1c-8e3f-4cde-8609-212c899cccf9@dunslane.net

2 years agopg_amcheck: Minor test speedups
Andres Freund [Sat, 11 Mar 2023 23:41:47 +0000 (15:41 -0800)]
pg_amcheck: Minor test speedups

Freezing the relation N times and fetching the tuples one-by-one isn't that
cheap. On my machine this reduces test times by a bit less than one second, on
windows CI it's a few seconds.

Reviewed-by: Mark Dilger <mark.dilger@enterprisedb.com>
Discussion: https://postgr.es/m/20230309001558.b7shzvio645ebdta@awork3.anarazel.de

2 years agoamcheck: Fix FullTransactionIdFromXidAndCtx() for xids before epoch 0
Andres Freund [Sat, 11 Mar 2023 22:12:51 +0000 (14:12 -0800)]
amcheck: Fix FullTransactionIdFromXidAndCtx() for xids before epoch 0

64bit xids can't represent xids before epoch 0 (see also be504a3e974). When
FullTransactionIdFromXidAndCtx() was passed such an xid, it'd create a 64bit
xid far into the future. Noticed while adding assertions in the course of
investigating be504a3e974, as amcheck's test create such xids.

To fix the issue, just return FirstNormalFullTransactionId in this case. A
freshly initdb'd cluster already has a newer horizon. The most minimal version
of this would make the messages for some detected corruptions differently
inaccurate. To make those cases accurate, switch
FullTransactionIdFromXidAndCtx() to use the 32bit modulo difference between
xid and nextxid to compute the 64bit xid, yielding sensible "in the future" /
"in the past" answers.

Reviewed-by: Mark Dilger <mark.dilger@enterprisedb.com>
Discussion: https://postgr.es/m/20230108002923.cyoser3ttmt63bfn@awork3.anarazel.de
Backpatch: 14-, where heapam verification was introduced

2 years agoamcheck: Fix ordering bug in update_cached_xid_range()
Andres Freund [Sat, 11 Mar 2023 22:12:51 +0000 (14:12 -0800)]
amcheck: Fix ordering bug in update_cached_xid_range()

The initialization order in update_cached_xid_range() was wrong, calling
FullTransactionIdFromXidAndCtx() before setting
->next_xid. FullTransactionIdFromXidAndCtx() uses ->next_xid.

In most situations this will not cause visible issues, because the next call
to update_cached_xid_range() will use a less wrong ->next_xid. It's rare that
xids advance fast enough for this to be a problem.

Found while adding more asserts to the 64bit xid infrastructure.

Reviewed-by: Mark Dilger <mark.dilger@enterprisedb.com>
Discussion: https://postgr.es/m/20230108002923.cyoser3ttmt63bfn@awork3.anarazel.de
Backpatch: 14-, where heapam verification was introduced

2 years agomeson: fix header path of ossp-uuid
Andres Freund [Sat, 11 Mar 2023 01:18:52 +0000 (17:18 -0800)]
meson: fix header path of ossp-uuid

The ossp-uuid pkg-config file includes the necessary -I to include the header
as uuid.h. Previously this would only work if ossp-uuid has its headers in
ossp/ in an already searched path.

Reported-by: Andrew Dunstan <andrew@dunslane.net>
Discussion: https://postgr.es/m/7bb8b8a5-5297-ab3c-3412-466fd0124d00@dunslane.net

2 years agoFix misbehavior in contrib/pg_trgm with an unsatisfiable regex.
Tom Lane [Sat, 11 Mar 2023 17:15:41 +0000 (12:15 -0500)]
Fix misbehavior in contrib/pg_trgm with an unsatisfiable regex.

If the regex compiler can see that a regex is unsatisfiable
(for example, '$foo') then it may emit an NFA having no arcs.
pg_trgm's packGraph function did the wrong thing in this case;
it would access off the end of a work array, and with bad luck
could produce a corrupted output data structure causing more
problems later.  This could end with wrong answers or crashes
in queries using a pg_trgm GIN or GiST index with such a regex.

Fix by not trying to de-duplicate if there aren't at least 2 arcs.

Per bug #17830 from Alexander Lakhin.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/17830-57ff5f89bdb02b09@postgresql.org

2 years agoEnsure COPY TO on an RLS-enabled table copies no more than it should.
Tom Lane [Fri, 10 Mar 2023 18:52:28 +0000 (13:52 -0500)]
Ensure COPY TO on an RLS-enabled table copies no more than it should.

The COPY documentation is quite clear that "COPY relation TO" copies
rows from only the named table, not any inheritance children it may
have.  However, if you enabled row-level security on the table then
this stopped being true, because the code forgot to apply the ONLY
modifier in the "SELECT ... FROM relation" query that it constructs
in order to allow RLS predicates to be attached.  Fix that.

Report and patch by Antonin Houska (comment adjustments and test case
by me).  Back-patch to all supported branches.

Discussion: https://postgr.es/m/3472.1675251957@antos

2 years agoinitdb: derive encoding from locale for ICU; similar to libc.
Jeff Davis [Fri, 10 Mar 2023 18:51:24 +0000 (10:51 -0800)]
initdb: derive encoding from locale for ICU; similar to libc.

Previously, the default encoding was derived from the locale when
using libc; while the default was always UTF-8 when using ICU. That
would throw an error when the locale was not compatible with UTF-8.

This commit causes initdb to derive the default encoding from the
locale for both providers. If --no-locale is specified (or if the
locale is C or POSIX), the default encoding will be UTF-8 for ICU
(because ICU does not support SQL_ASCII) and SQL_ASCII for libc.

Per buildfarm failure on system "hoverfly" related to commit
27b62377b4.

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

2 years agoFix tests for non-ICU build
Peter Eisentraut [Fri, 10 Mar 2023 13:27:55 +0000 (14:27 +0100)]
Fix tests for non-ICU build

missed in 0d21d4b9bc

2 years agoAdd standard collation UNICODE
Peter Eisentraut [Fri, 10 Mar 2023 12:35:00 +0000 (13:35 +0100)]
Add standard collation UNICODE

This adds a new predefined collation named UNICODE, which sorts by the
default Unicode collation algorithm specifications, per SQL standard.

This only works if ICU support is built.

Reviewed-by: Jeff Davis <pgsql@j-davis.com>
Discussion: https://www.postgresql.org/message-id/flat/1293e382-2093-a2bf-a397-c04e8f83d3c2@enterprisedb.com

2 years agoInclude headers of archive/ in installation
Michael Paquier [Fri, 10 Mar 2023 11:08:10 +0000 (20:08 +0900)]
Include headers of archive/ in installation

These new headers have been recently added in 35739b8, but they were not
installed.  Sravan has provided the patch for configure/make, while I
have fixed the meson part.

Author: Sravan Kumar, Michael Paquier
Discussion: https://postgr.es/m/CA+=NbjguiQy-MbVqfQ-jQ=2Fcmx3Zs36OkKb-vjt28jMTG0OOg@mail.gmail.com

2 years agoAdd a test for UCS_BASIC collation
Peter Eisentraut [Fri, 10 Mar 2023 10:00:51 +0000 (11:00 +0100)]
Add a test for UCS_BASIC collation

2 years agodoc: Better example for custom ICU rules
Peter Eisentraut [Fri, 10 Mar 2023 08:25:03 +0000 (09:25 +0100)]
doc: Better example for custom ICU rules

Use a more practical example, and also add some explanation.

Reported-by: Jeff Davis <pgsql@j-davis.com>
2 years agoFix incorrect format placeholders
Peter Eisentraut [Fri, 10 Mar 2023 06:10:43 +0000 (07:10 +0100)]
Fix incorrect format placeholders

2 years agopg_walinspect: pg_get_wal_fpi_info() -> pg_get_wal_block_info()
Michael Paquier [Fri, 10 Mar 2023 01:09:07 +0000 (10:09 +0900)]
pg_walinspect: pg_get_wal_fpi_info() -> pg_get_wal_block_info()

This commit reworks pg_get_wal_fpi_info() to become aware of all the
block information that can be attached to a record rather than just its
full-page writes:
- Addition of the block id as assigned by XLogRegisterBuffer(),
XLogRegisterBlock() or XLogRegisterBufData().
- Addition of the block data, as bytea, or NULL if none.  The length of
the block data can be guessed with length(), so there is no need to
store its length in a separate field.
- Addition of the full-page image length, as counted without a hole or
even compressed.
- Modification of the handling of the full-page image data.  This is
still a bytea, but it could become NULL if none is assigned to a block.
- Addition of the full-page image flags, tracking if a page is stored
with a hole, if it needs to be applied and the type of compression
applied to it, as of all the BKPIMAGE_* values in xlogrecord.h.

The information of each block is returned as one single record, with the
record's ReadRecPtr included to be able to join the block information
with the existing pg_get_wal_records_info().  Note that it is perfectly
possible for a block to hold both data and full-page image.

Thanks also to Kyotaro Horiguchi and Matthias van de Meent for the
discussion.

This commit uses some of the work proposed by Melanie, though it has
been largely redesigned and rewritten by me.  Bharath has helped in
refining a bit the whole.

Reported-by: Melanie Plageman
Author: Michael Paquier, Melanie Plageman, Bharath Rupireddy
Discussion: https://postgr.es/m/CAAKRu_bORebdZmcV8V4cZBzU8M_C6tDDdbiPhCZ6i-iuSXW9TA@mail.gmail.com

2 years agoFix test failure caused in 27b62377b4.
Jeff Davis [Thu, 9 Mar 2023 23:34:41 +0000 (15:34 -0800)]
Fix test failure caused in 27b62377b4.

Per buildfarm system "prion".

2 years agoReject combining "epoch" and "infinity" with other datetime fields.
Tom Lane [Thu, 9 Mar 2023 21:49:03 +0000 (16:49 -0500)]
Reject combining "epoch" and "infinity" with other datetime fields.

Datetime input formerly accepted combinations such as
'1995-08-06 infinity', but this seems like a clear error.
Reject any combination of regular y/m/d/h/m/s fields with
these special tokens.

Joseph Koshakow, reviewed by Keisuke Kuroda and myself

Discussion: https://postgr.es/m/CAAvxfHdm8wwXwG_FFRaJ1nTHiMWb7YXS2YKCzCt8Q0a2ZoMcHg@mail.gmail.com

2 years agoUse ICU by default at initdb time.
Jeff Davis [Thu, 9 Mar 2023 18:52:41 +0000 (10:52 -0800)]
Use ICU by default at initdb time.

If the ICU locale is not specified, initialize the default collator
and retrieve the locale name from that.

Discussion: https://postgr.es/m/510d284759f6e943ce15096167760b2edcb2e700.camel@j-davis.com
Reviewed-by: Peter Eisentraut
2 years agoHTML docs: Add padding to table.simplelist for more readable output
Alvaro Herrera [Thu, 9 Mar 2023 18:47:19 +0000 (19:47 +0100)]
HTML docs: Add padding to table.simplelist for more readable output

This couples with a to-be-pushed pgweb patch to synchronize the other
stylesheet under which these docs are rendered on the website.

Author: Karl Pinc <kop@karlpinc.com>
Discussion: https://postgr.es/m/20230122144246.0ff87372@slate.karlpinc.com

2 years agoFix 9637badd9f.
Jeff Davis [Thu, 9 Mar 2023 17:32:24 +0000 (09:32 -0800)]
Fix 9637badd9f.

Discussion: https://postgr.es/m/0a364430-266e-1e1a-d5d8-1a5273c9ddb6@dunslane.net
Reported-by: Andrew Dunstan
2 years agopg_upgrade: copy locale and encoding information to new cluster.
Jeff Davis [Thu, 9 Mar 2023 16:28:05 +0000 (08:28 -0800)]
pg_upgrade: copy locale and encoding information to new cluster.

Previously, pg_upgrade checked that the old and new clusters were
compatible, including the locale and encoding. But the new cluster was
just created, and only template0 from the new cluster will be
preserved (template1 and postgres are both recreated during the
upgrade process).

Because template0 is not sensitive to locale or encoding, just update
the pg_database entry to be the same as template0 from the original
cluster.

This commit makes it easier to change the default initdb locale or
encoding settings without causing needless incompatibilities.

Discussion: https://postgr.es/m/d62b2874-729b-d26a-2d0a-0d64f509eca4@enterprisedb.com
Reviewed-by: Peter Eisentraut
2 years agoFor Kerberos testing, disable reverse DNS lookup
Stephen Frost [Thu, 9 Mar 2023 15:32:49 +0000 (10:32 -0500)]
For Kerberos testing, disable reverse DNS lookup

In our Kerberos test suite, there isn't much need to worry about the
normal canonicalization that Kerberos provides by looking up the reverse
DNS for the IP address connected to, and in some cases it can actively
cause problems (eg: a captive portal wifi where the normally not
resolvable localhost address used ends up being resolved anyway, and
not to the domain we are using for testing, causing the entire
regression test to fail with errors about not being able to get a TGT
for the remote realm for cross-realm trust).

Therefore, disable it by adding rdns = false into the krb5.conf that's
generated for the test.

Reviewed-By: Heikki Linnakangas
Discussion: https://postgr.es/m/Y/QD2zDkDYQA1GQt@tamriel.snowman.net

2 years agoAvoid criticizable perl code
Alvaro Herrera [Thu, 9 Mar 2023 11:02:18 +0000 (12:02 +0100)]
Avoid criticizable perl code

Using `require` / `->import` instead of `use` avoids the use of a
"stringy eval", making for cleaner code that we don't need to silence
perlcritic about.

Per Andrew Dunstan

Discussion: https://postgr.es/m/7cd3bbbd-0216-4436-d571-8f80c9259a07@dunslane.net

2 years agoAdd b6dfee28 to .git-blame-ignore-revs
Michael Paquier [Thu, 9 Mar 2023 10:26:03 +0000 (19:26 +0900)]
Add b6dfee28 to .git-blame-ignore-revs

Thanks to Álvaro Herrera for the poke.

2 years agoImprove/correct comments
Peter Eisentraut [Thu, 9 Mar 2023 08:59:46 +0000 (09:59 +0100)]
Improve/correct comments

Change comments for pg_cryptohash_init(), pg_cryptohash_update(),
pg_cryptohash_final() in cryptohash.c to match cryptohash_openssl.c.
In particular, the claim that these functions were "designed" to never
fail was incorrect, since by design callers need to be prepared to
handle failures, for compatibility with the cryptohash_openssl.c
versions.

Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://www.postgresql.org/message-id/301F4EDD-27B9-460F-B462-B9DB2BDE4ACF@yesql.se

2 years agoDisallow specifying ICU rules unless locale provider is ICU
Peter Eisentraut [Thu, 9 Mar 2023 07:09:40 +0000 (08:09 +0100)]
Disallow specifying ICU rules unless locale provider is ICU

Follow-up for 30a53b7929; this was not checked in all cases.

Reported-by: Jeff Davis <pgsql@j-davis.com>
2 years agoRun pgindent on libpq's fe-auth.c, fe-auth-scram.c and fe-connect.c
Michael Paquier [Thu, 9 Mar 2023 06:09:45 +0000 (15:09 +0900)]
Run pgindent on libpq's fe-auth.c, fe-auth-scram.c and fe-connect.c

A patch sent by Jacob Champion has been touching this area of the code,
and the set of changes done in a9e9a9f has made a run of pgindent on
these files a bit annoying to handle.  So let's clean up a bit the area,
first, to ease the work on follow-up patches.

Discussion: https://postgr.es/m/9e5a8ccddb8355ea9fa4b75a1e3a9edc88a70cd3.camel@vmware.com

2 years agoFix race in SERIALIZABLE READ ONLY.
Thomas Munro [Thu, 9 Mar 2023 03:33:24 +0000 (16:33 +1300)]
Fix race in SERIALIZABLE READ ONLY.

Commit bdaabb9b started skipping doomed transactions when building the
list of possible conflicts for SERIALIZABLE READ ONLY.  That makes
sense, because doomed transactions won't commit, but a couple of subtle
things broke:

1.  If all uncommitted r/w transactions are doomed, a READ ONLY
transaction would arbitrarily not benefit from the safe snapshot
optimization.  It would not be taken immediately, and yet no other
transaction would set SXACT_FLAG_RO_SAFE later.

2.  In the same circumstances but with DEFERRABLE, GetSafeSnapshot()
would correctly exit its wait loop without sleeping and then take the
optimization in non-assert builds, but assert builds would fail a sanity
check that SXACT_FLAG_RO_SAFE had been set by another transaction.

This is similar to the case for PredXact->WritableSxactCount == 0.  We
should opt out immediately if our possibleUnsafeConflicts list is empty
after filtering.

The code to maintain the serializable global xmin is moved down below
the new opt out site, because otherwise we'd have to reverse its effects
before returning.

Back-patch to all supported releases.  Bug #17368.

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/17116-d6ca217acc180e30%40postgresql.org
Discussion: https://postgr.es/m/20110707212159.GF76634%40csail.mit.edu

2 years agomeson: tests: Adjust with_icu/ZSTD env vars for pg_dump, pg_basebackup
Andres Freund [Thu, 9 Mar 2023 00:53:57 +0000 (16:53 -0800)]
meson: tests: Adjust with_icu/ZSTD env vars for pg_dump, pg_basebackup

396d348b0 omitted adding with_icu to the pg_dump tests under
meson. Conversely, e6927270c exported ZSTD for pg_basebackup's tests, despite
pg_basebackup's ZSTD support not having any tests.

Reported-by: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/20230226225239.GL1653@telsasoft.com

2 years agodoc: Add guidelines to generate coverage reports with meson
Michael Paquier [Thu, 9 Mar 2023 00:21:47 +0000 (09:21 +0900)]
doc: Add guidelines to generate coverage reports with meson

These instructions were already available for configure-based builds,
but not the meson-based builds.  This commit closes the gap.

Reviewed-by: Peter Eisentraut
Discussion: https://postgr.es/m/d3751ca3-1ac9-cc5e-9315-cf9fb0eaa9e9@enterprisedb.com

2 years agomeson: Add target for installing test files & improve install_test_files
Andres Freund [Wed, 8 Mar 2023 00:14:18 +0000 (16:14 -0800)]
meson: Add target for installing test files & improve install_test_files

The changes in b6a0d469cae prevented installation of the test files during a
normal install. However, the buildfarm intentionally tries to trun the tests
against a "real" installation. The new install-test-files target provides that
ability.

Because we want to install into a normal directory, I removed the necessary
munging of the target paths from meson.build and moved it into
install-test-files. I also added DESTDIR support, so that installing can
redirect the directory if desired. That's used for the tmp_install/
installation now.

I didn't like the number of arguments necessary for install_test_files, so I
changed it to use
  --install target list of files
which makes it easier to use for further directories, if/when we need them.

Discussion: https://postgr.es/m/20230308012940.edexipb3vqylcu6r@awork3.anarazel.de

2 years ago001_libpq_pipeline.pl: use Test::Differences if available
Alvaro Herrera [Wed, 8 Mar 2023 17:31:55 +0000 (18:31 +0100)]
001_libpq_pipeline.pl: use Test::Differences if available

When one of these tests fails to match the trace, this better shows what
the problem is.

Discussion: https://postgr.es/m/20220617183150.ilgokxp22mzywnhh@alvherre.pgsql
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2 years agoAllow tailoring of ICU locales with custom rules
Peter Eisentraut [Wed, 8 Mar 2023 15:35:42 +0000 (16:35 +0100)]
Allow tailoring of ICU locales with custom rules

This exposes the ICU facility to add custom collation rules to a
standard collation.

New options are added to CREATE COLLATION, CREATE DATABASE, createdb,
and initdb to set the rules.

Reviewed-by: Laurenz Albe <laurenz.albe@cybertec.at>
Reviewed-by: Daniel Verite <daniel@manitou-mail.org>
Discussion: https://www.postgresql.org/message-id/flat/821c71a4-6ef0-d366-9acf-bb8e367f739f@enterprisedb.com

2 years agoClean up comments
Peter Eisentraut [Wed, 8 Mar 2023 14:56:32 +0000 (15:56 +0100)]
Clean up comments

Reformat some of the comments in MergeAttributes().  A lot of code has
been added here over time, and the comments could use a bit of editing
to make the code flow read better.

2 years agoBreak up long GETTEXT_FILES lists
Peter Eisentraut [Wed, 8 Mar 2023 14:05:43 +0000 (15:05 +0100)]
Break up long GETTEXT_FILES lists

One file per line seems best.  We already did this in some cases.
This adopts the same format everywhere (except in some cases where the
list reasonably fits on one line).

2 years agoUpdate comment
Peter Eisentraut [Wed, 8 Mar 2023 13:22:06 +0000 (14:22 +0100)]
Update comment

There was apparently an attempt here to list all the object types that
ACL_USAGE applies to, but it wasn't complete.  So instead of trying to
keep up, put in a more timeless comment.

2 years agoReflect normalization of query strings for utilities in pg_stat_statements
Michael Paquier [Wed, 8 Mar 2023 06:00:50 +0000 (15:00 +0900)]
Reflect normalization of query strings for utilities in pg_stat_statements

Applying normalization changes how the following query strings are
reflected in pg_stat_statements, by showing Const nodes with a
dollar-signed parameter as this is how such queries are structured
internally once parsed:
- DECLARE
- EXPLAIN
- CREATE MATERIALIZED VIEW
- CREATE TABLE AS

More normalization could be done in the future depending on the parts
where query jumbling is applied (like A_Const nodes?), the changes being
reflected in the regression tests in majority created in de2aca2.  This
just allows the basics to work for utility queries using Const nodes.

Reviewed-by: Bertrand Drouvot
Discussion: https://postgr.es/m/Y+MRdEq9W9XVa2AB@paquier.xyz

2 years agoFix corruption due to vacuum_defer_cleanup_age underflowing 64bit xids
Andres Freund [Wed, 8 Mar 2023 05:36:46 +0000 (21:36 -0800)]
Fix corruption due to vacuum_defer_cleanup_age underflowing 64bit xids

When vacuum_defer_cleanup_age is bigger than the current xid, including the
epoch, the subtraction of vacuum_defer_cleanup_age would lead to a wrapped
around xid. While that normally is not a problem, the subsequent conversion to
a 64bit xid results in a 64bit-xid very far into the future. As that xid is
used as a horizon to detect whether rows versions are old enough to be
removed, that allows removal of rows that are still visible (i.e. corruption).

If vacuum_defer_cleanup_age was never changed from the default, there is no
chance of this bug occurring.

This bug was introduced in dc7420c2c92.  A lesser version of it exists in
12-13, introduced by fb5344c969a, affecting only GiST.

The 12-13 version of the issue can, in rare cases, lead to pages in a gist
index getting recycled too early, potentially causing index entries to be
found multiple times.

The fix is fairly simple - don't allow vacuum_defer_cleanup_age to retreat
further than FirstNormalTransactionId.

Patches to make similar bugs easier to find, by adding asserts to the 64bit
xid infrastructure, have been proposed, but are not suitable for backpatching.

Currently there are no tests for vacuum_defer_cleanup_age. A patch introducing
infrastructure to make writing a test easier has been posted to the list.

Reported-by: Michail Nikolaev <michail.nikolaev@gmail.com>
Reviewed-by: Matthias van de Meent <boekewurm+postgres@gmail.com>
Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20230108002923.cyoser3ttmt63bfn@awork3.anarazel.de
Backpatch: 12-, but impact/fix is smaller for 12-13

2 years agoRefine query jumbling handling for CallStmt
Michael Paquier [Wed, 8 Mar 2023 05:38:35 +0000 (14:38 +0900)]
Refine query jumbling handling for CallStmt

Previously, all the nodes of CallStmt were included in the jumbling,
causing a duplicate in the computation as the transformed state of the
CALL query was included as well as the parsed state (transformed
FuncCall with all the input arguments and potential output arguments).

Reviewed-by: Bertrand Drouvot
Discussion: https://postgr.es/m/Y+MRdEq9W9XVa2AB@paquier.xyz

2 years agomeson: don't require 'touch' binary, make use of 'cp' optional
Andres Freund [Wed, 8 Mar 2023 02:24:18 +0000 (18:24 -0800)]
meson: don't require 'touch' binary, make use of 'cp' optional

We already didn't use touch (some earlier version of the meson build did ),
and cp is only used for updating unicode files. The latter already depends on
the optional availability of 'wget', so doing the same for 'cp' makes sense.

Eventually we probably want a portable command for updating source code as
part of a target, but for now...

Reported-by: Andrew Dunstan <andrew@dunslane.net>
Discussion: https://postgr.es/m/70e96c34-64ee-e549-8c4a-f91a7a668804@dunslane.net

2 years agoIgnore IntoClause.viewQuery in query jumbling
Michael Paquier [Wed, 8 Mar 2023 02:41:52 +0000 (11:41 +0900)]
Ignore IntoClause.viewQuery in query jumbling

IntoClause.viewQuery is a copy of the parsed-but-not-rewritten SELECT
clause copied to IntoClause when transforming CreateTableAsStmt for a
materialized view.  Including a second copy of the SELECT Query into the
query jumbling was leading to an incorrect numbering of the Const node
locations, as these would be counted twice instead of once.

This becomes visible once the query normalization is applied to CREATE
MATERIALIZED VIEW in pg_stat_statements in the shape of a query string
using only odd numbers for the normalized constants, (regression tests
added in pg_stat_statements as of de2aca2 would show the difference).
Including the original Query from CreateTableAsStmt is enough for the
query jumbling.

Reviewed-by: Bertrand Drouvot
Discussion: https://postgr.es/m/Y+MRdEq9W9XVa2AB@paquier.xyz

2 years agoImprove readability of code PROCESS_MAIN in vacuum_rel()
Michael Paquier [Wed, 8 Mar 2023 00:16:44 +0000 (09:16 +0900)]
Improve readability of code PROCESS_MAIN in vacuum_rel()

4211fbd has been handling PROCESS_MAIN in vacuum_rel() with an "if/else
if" structure to avoid an extra level of indentation, but this has been
found as being rather parse to read.  This commit updates the code so as
we check for PROCESS_MAIN in a single place and then handle its
subpaths, FULL or non-FULL vacuums.  Some comments are added to make
that clearer for the reader.

Reported-by: Melanie Plageman
Author: Nathan Bossart
Reviewed-by: Michael Paquier, Melanie Plageman
Discussion: https://postgr.es/m/20230306194009.5cn6sp3wjotd36nu@liskov

2 years agoFix more bugs caused by adding columns to the end of a view.
Tom Lane [Tue, 7 Mar 2023 23:21:37 +0000 (18:21 -0500)]
Fix more bugs caused by adding columns to the end of a view.

If a view is defined atop another view, and then CREATE OR REPLACE
VIEW is used to add columns to the lower view, then when the upper
view's referencing RTE is expanded by ApplyRetrieveRule we will have
a subquery RTE with fewer eref->colnames than output columns.  This
confuses various code that assumes those lists are always in sync,
as they are in plain parser output.

We have seen such problems before (cf commit d5b760ecb), and now
I think the time has come to do what was speculated about in that
commit: let's make ApplyRetrieveRule synthesize some column names to
preserve the invariant that holds in parser output.  Otherwise we'll
be chasing this class of bugs indefinitely.  Moreover, it appears from
testing that this actually gives us better results in the test case
d5b760ecb added, and likely in other corner cases that we lack
coverage for.

In HEAD, I replaced d5b760ecb's hack to make expandRTE exit early with
an elog(ERROR) call, since the case is now presumably unreachable.
But it seems like changing that in back branches would bring more risk
than benefit, so there I just updated the comment.

Per bug #17811 from Alexander Lakhin.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/17811-d31686b78f0dffc9@postgresql.org

2 years agoAdd support for unit "B" to pg_size_bytes()
Peter Eisentraut [Tue, 7 Mar 2023 19:26:29 +0000 (20:26 +0100)]
Add support for unit "B" to pg_size_bytes()

This makes it consistent with the units support in GUC.

Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/0106914a-9eb5-22be-40d8-652cc88c827d%40enterprisedb.com

2 years agodoc: Update pg_size_pretty documentation about petabytes support
Peter Eisentraut [Tue, 7 Mar 2023 18:30:14 +0000 (19:30 +0100)]
doc: Update pg_size_pretty documentation about petabytes support

Missing documentation update for ca2e4472ba.

Discussion: https://www.postgresql.org/message-id/CAApHDvrCwMgSD_93LZr4CLMas8Hc61fXAQ-Cd4%3D%2ByoRfHnYbJA%40mail.gmail.com

2 years agoFix flakey pg_stat_io test
Andres Freund [Tue, 7 Mar 2023 18:03:42 +0000 (10:03 -0800)]
Fix flakey pg_stat_io test

Wrap test of pg_stat_io's tracking of shared buffer reads in a transaction to
prevent concurrent accesses (e.g. by autovacuum) causing spurious test
failures.

Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Author: Melanie Plageman <melanieplageman@gmail.com>
Discussion: https://www.postgresql.org/message-id/20230306190919.ai6mxdq3sygyyths%40awork3.anarazel.de

2 years agoMake get_extension_schema() available
Michael Paquier [Tue, 7 Mar 2023 05:18:20 +0000 (14:18 +0900)]
Make get_extension_schema() available

This routine is able to retrieve the OID of the schema used with an
extension (pg_extension.extnamespace), or InvalidOid if this information
is not available.  plpgsql_check embeds a copy of this code when
performing checks on functions, as one out-of-core example.

Author: Pavel Stehule
Reviewed-by: Julien Rouhaud
Discussion: https://postgr.es/m/CAFj8pRD+9x55hjDoi285jCcjPc8uuY_D+FLn5RpXggdz+4O2sQ@mail.gmail.com

2 years agoFix incorrect comment in pg_get_partkeydef()
David Rowley [Tue, 7 Mar 2023 01:33:28 +0000 (14:33 +1300)]
Fix incorrect comment in pg_get_partkeydef()

The comment claimed the output of the function was prefixed by "PARTITION
BY".  This is incorrect.

Author: Japin Li
Reviewed-by: Ashutosh Bapat
Discussion: https://postgr.es/m/MEYP282MB166923B446FF5FE55B9DACB7B6B69@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM

2 years agoImprove cleanup phases in regression tests of pg_stat_statements
Michael Paquier [Mon, 6 Mar 2023 23:58:13 +0000 (08:58 +0900)]
Improve cleanup phases in regression tests of pg_stat_statements

As shaped, two DROP ROLE queries included in "user_activity" were
showing in the reports for "wal".  The intention is to keep each test
isolated and independent, so this is incorrect.  This commit adds some
calls to pg_stat_statements_reset() to clean up the statistics once each
test finishes, so as there are no risks of overlap in the reports for
individial scenarios.

The addition in "user_activity" fixes the output of "wal".  The new
resets done in "level_tracking" and "utility" are added for consistency
with the rest, though they do not affect the stats generated in the
other tests.

Oversight in d0028e3.

Reported-by: Andrei Zubkov
Discussion: https://postgr.es/m/7beb722dd016bf54f1c78bfd6d44a684e28da624.camel@moonset.ru

2 years agoFix some more cases of missed GENERATED-column updates.
Tom Lane [Mon, 6 Mar 2023 23:31:16 +0000 (18:31 -0500)]
Fix some more cases of missed GENERATED-column updates.

If UPDATE is forced to retry after an EvalPlanQual check, it neglected
to repeat GENERATED-column computations, even though those might well
have changed since we're dealing with a different tuple than before.
Fixing this is mostly a matter of looping back a bit further when
we retry.  In v15 and HEAD that's most easily done by altering the API
of ExecUpdateAct so that it includes computing GENERATED expressions.

Also, if an UPDATE in a partitioned table turns into a cross-partition
INSERT operation, we failed to recompute GENERATED columns.  That's a
bug since 8bf6ec3ba allowed partitions to have different generation
expressions; although it seems to have no ill effects before that.
Fixing this is messier because we can now have situations where the same
query needs both the UPDATE-aligned set of GENERATED columns and the
INSERT-aligned set, and it's unclear which set will be generated first
(else we could hack things by forcing the INSERT-aligned set to be
generated, which is indeed how fe9e658f4 made it work for MERGE).
The best fix seems to be to build and store separate sets of expressions
for the INSERT and UPDATE cases.  That would create ABI issues in the
back branches, but so far it seems we can leave this alone in the back
branches.

Per bug #17823 from Hisahiro Kauchi.  The first part of this affects all
branches back to v12 where GENERATED columns were added.

Discussion: https://postgr.es/m/17823-b64909cf7d63de84@postgresql.org

2 years agoSilence -Wmissing-braces complaints in file_utils.c
Michael Paquier [Mon, 6 Mar 2023 22:42:36 +0000 (07:42 +0900)]
Silence -Wmissing-braces complaints in file_utils.c

Per buildfarm member lapwing, coupled with an offline poke from Julien
Rouhaud.

6392f2a was a similar case.

2 years agoFill EState.es_rteperminfos more systematically.
Tom Lane [Mon, 6 Mar 2023 18:10:57 +0000 (13:10 -0500)]
Fill EState.es_rteperminfos more systematically.

While testing a fix for bug #17823, I discovered that EvalPlanQualStart
failed to copy es_rteperminfos from the parent EState, resulting in
failure if anything in EPQ execution wanted to consult that information.

This led me to conclude that commit a61b1f748 had been too haphazard
about where to fill es_rteperminfos, and that we need to be sure that
that happens exactly where es_range_table gets filled.  So I changed the
signature of ExecInitRangeTable to help ensure that this new requirement
doesn't get missed.  (Indeed, pgoutput.c was also failing to fill it.
Maybe we don't ever need it there, but I wouldn't bet on that.)

No test case yet; one will arrive with the fix for #17823.
But that needs to be back-patched, while this fix is HEAD-only.

Discussion: https://postgr.es/m/17823-b64909cf7d63de84@postgresql.org

2 years agoReword overly-optimistic comment about backup checksum verification.
Robert Haas [Mon, 6 Mar 2023 15:35:15 +0000 (10:35 -0500)]
Reword overly-optimistic comment about backup checksum verification.

The comment implies that a single retry is sufficient to avoid
spurious checksum failures, but in fact no number of retries is
sufficient for that purpose. Update the comment accordingly.

Patch by me, reviewed by Michael Paquier.

Discussion: http://postgr.es/m/CA+TgmoZ_fFAoU6mrHt9QBs+dcYhN6yXenGTTMRebZNhtwPwHyg@mail.gmail.com

2 years agoRemove an old comment that doesn't seem especially useful.
Robert Haas [Thu, 2 Feb 2023 19:51:56 +0000 (14:51 -0500)]
Remove an old comment that doesn't seem especially useful.

The functions that follow are concerned with various things, of
which the tar format is only one, so this comment doesn't really
seem helpful. The file isn't really divided into sections in the
way that this comment seems to contemplate -- or at least, not
any more.

Patch by me, reviewed by Michael Paquier.

Discussion: http://postgr.es/m/CA+TgmoZ_fFAoU6mrHt9QBs+dcYhN6yXenGTTMRebZNhtwPwHyg@mail.gmail.com

2 years agoIn basebackup.c, perform end-of-file test after checksum validation.
Robert Haas [Thu, 2 Feb 2023 17:04:16 +0000 (12:04 -0500)]
In basebackup.c, perform end-of-file test after checksum validation.

We read blocks of data from files that we're backing up in chunks,
some multiple of BLCKSZ for each read. If checksum verification fails,
we then try rereading just the one block for which validation failed.
If that block happened to be the first block of the chunk, and if
the file was concurrently truncated to remove that block, then we'd
reach a call to bbsink_archive_contents() with a buffer length of 0.
That causes an assertion failure.

As far as I can see, there are no particularly bad consequences if
this happens in a non-assert build, and it's pretty unlikely to happen
in the first place because it requires a series of somewhat unlikely
things to happen in very quick succession. However, assertion failures
are bad, so rearrange the code to avoid that possibility.

Patch by me, reviewed by Michael Paquier.

Discussion: http://postgr.es/m/CA+TgmoZ_fFAoU6mrHt9QBs+dcYhN6yXenGTTMRebZNhtwPwHyg@mail.gmail.com

2 years agoFix handling of default option values in createuser
Daniel Gustafsson [Mon, 6 Mar 2023 13:16:32 +0000 (14:16 +0100)]
Fix handling of default option values in createuser

Add description of which one is the default between two complementary
options of --bypassrls and --replication in the help text and docs. In
correspondence let the command always include the tokens corresponding
to every options of that kind in the SQL command sent to server. Tests
are updated accordingly.

Also fix the checks of some trivalue vars which were using literal zero
for checking default value instead of the enum label TRI_DEFAULT. While
not a bug, since TRI_DEFAULT is defined as zero, fixing improves read-
ability improved readability (and avoid bugs if the enum is changed).

Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/20220810.151243.1073197628358749087.horikyota.ntt@gmail.com

2 years agoAdd PROCESS_MAIN to VACUUM
Michael Paquier [Mon, 6 Mar 2023 07:41:05 +0000 (16:41 +0900)]
Add PROCESS_MAIN to VACUUM

Disabling this option is useful to run VACUUM (with or without FULL) on
only the toast table of a relation, bypassing the main relation.  This
option is enabled by default.

Running directly VACUUM on a toast table was already possible without
this feature, by using the non-deterministic name of a toast relation
(as of pg_toast.pg_toast_N, where N would be the OID of the parent
relation) in the VACUUM command, and it required a scan of pg_class to
know the name of the toast table.  So this feature is basically a
shortcut to be able to run VACUUM or VACUUM FULL on a toast relation,
using only the name of the parent relation.

A new switch called --no-process-main is added to vacuumdb, to work as
an equivalent of PROCESS_MAIN.

Regression tests are added to cover VACUUM and VACUUM FULL, looking at
pg_stat_all_tables.vacuum_count to see how many vacuums have run on
each table, main or toast.

Author: Nathan Bossart
Reviewed-by: Masahiko Sawada
Discussion: https://postgr.es/m/20221230000028.GA435655@nathanxps13

2 years agoImprove the regression tests of VACUUM (PROCESS_TOAST)
Michael Paquier [Mon, 6 Mar 2023 06:40:56 +0000 (15:40 +0900)]
Improve the regression tests of VACUUM (PROCESS_TOAST)

All the regression tests of VACUUM (PROCESS_TOAST) were only checking if
the commands were able to run, without checking if VACUUM was really
running on what it should.  This expands this set of tests so as we now
look at pg_stat_all_tables.vacuum_count to see how many vacuums have
been run on a given table and its toast relation.

Extracted from a larger patch by the same author, as this is useful on
its own.

Special thanks to Álvaro Herrera for the idea of using
pg_stat_all_tables to check the state of the toast relation.

Author: Nathan Bossart
Reviewed-by: Masahiko Sawada
Discussion: https://postgr.es/m/20221230000028.GA435655@nathanxps13

2 years agoDeduplicate handling of binary and text modes in logicalrep_read_tuple().
Amit Kapila [Mon, 6 Mar 2023 04:24:57 +0000 (09:54 +0530)]
Deduplicate handling of binary and text modes in logicalrep_read_tuple().

Author: Bharath Rupireddy
Reviewed-by: Peter Smith
Discussion: https://postgr.es/m/CALj2ACXdbq7kW_+bRrSGMsR6nefCvwbHBJ5J51mr3gFf7QysTA@mail.gmail.com

2 years agoRevise pg_pwrite_zeros()
Michael Paquier [Mon, 6 Mar 2023 04:21:33 +0000 (13:21 +0900)]
Revise pg_pwrite_zeros()

The following changes are made to pg_write_zeros(), the API able to
write series of zeros using vectored I/O:
- Add of an "offset" parameter, to write the size from this position
(the 'p' of "pwrite" seems to mean position, though POSIX does not
outline ythat directly), hence the name of the routine is incorrect if
it is not able to handle offsets.
- Avoid memset() of "zbuffer" on every call.
- Avoid initialization of the whole IOV array if not needed.
- Group the trailing write() call with the main write() call,
simplifying the function logic.

Author: Andres Freund
Reviewed-by: Michael Paquier, Bharath Rupireddy
Discussion: https://postgr.es/m/20230215005525.mrrlmqrxzjzhaipl@awork3.anarazel.de

2 years agoFix assert failures in parallel SERIALIZABLE READ ONLY.
Thomas Munro [Mon, 6 Mar 2023 02:07:15 +0000 (15:07 +1300)]
Fix assert failures in parallel SERIALIZABLE READ ONLY.

1.  Make sure that we don't decrement SxactGlobalXminCount twice when
the SXACT_FLAG_RO_SAFE optimization is reached in a parallel query.
This could trigger a sanity check failure in assert builds.  Non-assert
builds recompute the count in SetNewSxactGlobalXmin(), so the problem
was hidden, explaining the lack of field reports.  Add a new isolation
test to exercise that case.

2.  Remove an assertion that the DOOMED flag can't be set on a partially
released SERIALIZABLEXACT.  Instead, ignore the flag (our transaction
was already determined to be read-only safe, and DOOMED is in fact set
during partial release, and there was already an assertion that it
wasn't set sooner).  Improve an existing isolation test so that it
reaches that case (previously it wasn't quite testing what it was
supposed to be testing; see discussion).

Back-patch to 12.  Bug #17116.  Defects in commit 47a338cf.

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/17116-d6ca217acc180e30%40postgresql.org

2 years agoSQL JSON path enhanced numeric literals
Peter Eisentraut [Sun, 5 Mar 2023 14:02:01 +0000 (15:02 +0100)]
SQL JSON path enhanced numeric literals

Add support for non-decimal integer literals and underscores in
numeric literals to SQL JSON path language.  This follows the rules of
ECMAScript, as referred to by the SQL standard.

Internally, all the numeric literal parsing of jsonpath goes through
numeric_in, which already supports all this, so this patch is just a
bit of lexer work and some tests and documentation.

Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/b11b25bb-6ec1-d42f-cedd-311eae59e1fb@enterprisedb.com

2 years agoAvoid failure when altering state of partitioned foreign-key triggers.
Tom Lane [Sat, 4 Mar 2023 18:32:35 +0000 (13:32 -0500)]
Avoid failure when altering state of partitioned foreign-key triggers.

Beginning in v15, if you apply ALTER TABLE ENABLE/DISABLE TRIGGER to
a partitioned table, it also affects the partitions' cloned versions
of the affected trigger(s).  The initial implementation of this
located the clones by name, but that fails on foreign-key triggers
which have names incorporating their own OIDs.  We can fix that, and
also make the behavior more bulletproof in the face of user-initiated
trigger renames, by identifying the cloned triggers by tgparentid.

Following the lead of earlier commits in this area, I took care not
to break ABI in the v15 branch, even though I rather doubt there
are any external callers of EnableDisableTrigger.

While here, update the documentation, which was not touched when
the semantics were changed.

Per bug #17817 from Alan Hodgson.  Back-patch to v15; older versions
do not have this behavior.

Discussion: https://postgr.es/m/17817-31dfb7c2100d9f3d@postgresql.org

2 years agoTighten header pre-inclusions in headerscheck and cpluspluscheck.
Tom Lane [Sat, 4 Mar 2023 17:11:50 +0000 (12:11 -0500)]
Tighten header pre-inclusions in headerscheck and cpluspluscheck.

We allow our header files to depend on the appropriate one of
postgres.h, postgres_fe.h, or c.h having already been included.
However, there are a few headers such as libpq-fe.h that are
meant to be used by client applications and therefore must
compile without any assumptions about previous inclusions.
These test scripts failed to consider that, which seems quite
hazardous since we might not immediately notice such a problem
otherwise.  Hence, adjust these scripts to test relevant libpq
and ecpg headers with no prior inclusion.

While at it, we can also make an effort to actually use the
relevant one of postgres.h, postgres_fe.h, or c.h.  I added
some rules that guess which one to use based on the first-level
src subdirectory, e.g. use postgres_fe.h under src/bin/.
These rules are hardly water-tight but they seem to work today,
and we can always refine them in the future.

These changes don't reveal any live problems today, which is good,
but they should make these scripts more able to catch future bugs.

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

2 years agoUpdate some incorrect comments about xlog records.
Robert Haas [Fri, 3 Mar 2023 17:52:04 +0000 (12:52 -0500)]
Update some incorrect comments about xlog records.

The comments claim that certain pieces of data are part of the main
WAL record data when in reality they are part of the data for
block 0. Repair.

Bertrand Drouvot, reviewed by Amit Kapila. Originally reported by me.

Discussion: http://postgr.es/m/80db7836-4415-d54a-64c3-66b88b1430e7@gmail.com

2 years agomeson: Prevent installation of test files during main install
Peter Eisentraut [Fri, 3 Mar 2023 06:18:20 +0000 (07:18 +0100)]
meson: Prevent installation of test files during main install

Previously, meson installed modules under src/test/modules/ as part of
a normal installation, even though these files are only meant for use
by tests.  This is because there is no way to set up up the build
system to install extra things only when told.

This patch fixes that with a workaround: We don't install these
modules as part of meson install, but we create a new "test" that runs
before the real tests whose action it is to install these files.  The
installation is done by manual copies using a small helper script.

Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/2a039e8e-f31f-31e8-afe7-bab3130ad2de%40enterprisedb.com

2 years agoFix incorrect format placeholders
Peter Eisentraut [Fri, 3 Mar 2023 06:01:18 +0000 (07:01 +0100)]
Fix incorrect format placeholders

2 years agoForce testing of query jumbling in 027_stream_regress.pl
Michael Paquier [Fri, 3 Mar 2023 01:41:51 +0000 (10:41 +0900)]
Force testing of query jumbling in 027_stream_regress.pl

Coverage of the query jumbling code has always relied on the queries
included in the regression tests of pg_stat_statements.  This has its
limitations, as a lot of query patterns have never really stressed the
query jumbling code.  The situation got a bit worse since the query
jumbling has been added in the backend core code (5fd9dfa), hence new
nodes that should be included in the jumbling could easily be missed,
resulting in failures in pg_stat_statements or any modules that require
query ID computations.  Forcing a load of pg_stat_statements in
027_stream_regress.pl ensures that nodes are never missed in the
computations, without having to rely on a buildfarm member for this
check.

Before this commit, the line coverage of queryjumblefuncs.funcs.c was
around 48.5%, now up to 94.6% just by running 027_stream_regress.pl.
A basic check is added to show that pg_stat_statements reports are
generated after the main regression test suite is finished.

Discussion: https://postgr.es/m/Y+nD9LN70w+8eaG9@paquier.xyz

2 years agoRefactor more the regression tests of pg_stat_statements
Michael Paquier [Thu, 2 Mar 2023 23:46:11 +0000 (08:46 +0900)]
Refactor more the regression tests of pg_stat_statements

This commit expands more the refactoring of the regression tests of
pg_stat_statements, with tests moved out of pg_stat_statements.sql into
separate files.  The following file structure is now used:
- select is mostly the former pg_stat_statements.sql, renamed.
- dml for INSERT/UPDATE/DELETE and MERGE
- user_activity, to test role-level checks and stat resets.
- wal, to check the WAL generation after some queries.

Like e8dbdb1, there is no change in terms of code coverage or results,
and this finishes the split I was aiming for in these tests.  Most of
the tests used "test" of "pgss_test" as names for the tables used, these
are renamed to less generic names.

Reviewed-by: Bertrand Drouvot
Discussion: https://postgr.es/m/Y/7Y9U/y/keAW3qH@paquier.xyz

2 years agoHarden new test case against force_parallel_mode = regress.
Tom Lane [Thu, 2 Mar 2023 22:47:20 +0000 (17:47 -0500)]
Harden new test case against force_parallel_mode = regress.

Per buildfarm: worker processes can't see a role created in
the current transaction.

2 years agoShow "internal name" not "source code" in psql's \df+ command.
Tom Lane [Thu, 2 Mar 2023 22:15:00 +0000 (17:15 -0500)]
Show "internal name" not "source code" in psql's \df+ command.

Our previous habit of showing the full function body is really
pretty unfriendly for tabular viewing of functions, and now that
we have \sf and \ef commands there seems no good reason why \df+
has to do it.  It still seems to make sense to show prosrc for
internal and C-language functions, since in those cases prosrc
is just the C function name; but then let's rename the column to
"Internal name" which is a more accurate descriptor.

Isaac Morland

Discussion: https://postgr.es/m/CAMsGm5eqKc6J1=Lwn=ZONG=6ZDYWRQ4cgZQLqMuZGB1aVt_JBg@mail.gmail.com

2 years agoDon't leak descriptors into subprograms.
Thomas Munro [Thu, 2 Mar 2023 21:28:47 +0000 (10:28 +1300)]
Don't leak descriptors into subprograms.

Open long-lived data and WAL file descriptors with O_CLOEXEC.  This flag
was introduced by SUSv4 (POSIX.1-2008), and by now all of our target
Unix systems have it.  Our open() implementation for Windows already had
that behavior, so provide a dummy O_CLOEXEC flag on that platform.

For now, callers of open() and the "thin" wrappers in fd.c that deal in
raw descriptors need to pass in O_CLOEXEC explicitly if desired.  This
commit does that for WAL files, and automatically for everything
accessed via VFDs including SMgrRelation and BufFile.  (With more
discussion we might decide to turn it on automatically for the thin
open()-wrappers too to avoid risk of missing places that need it, but
these are typically used for short-lived descriptors where we don't
expect to fork/exec, and it's remotely possible that extensions could be
using these APIs and passing descriptors to subprograms deliberately, so
that hasn't been done here.)

Do the same for sockets and the postmaster pipe with FD_CLOEXEC.  (Later
commits might use modern interfaces to remove these extra fcntl() calls
and more where possible, but we'll need them as a fallback for a couple
of systems, so do it that way in this initial commit.)

With this change, subprograms executed for archiving, copying etc will
no longer have access to the server's descriptors, other than the ones
that we decide to pass down.

Reviewed-by: Andres Freund <andres@anarazel.de> (earlier version)
Discussion: https://postgr.es/m/CA%2BhUKGKb6FsAdQWcRL35KJsftv%2B9zXqQbzwkfRf1i0J2e57%2BhQ%40mail.gmail.com

2 years agoRemove local optimizations of empty Bitmapsets into null pointers.
Tom Lane [Thu, 2 Mar 2023 17:01:47 +0000 (12:01 -0500)]
Remove local optimizations of empty Bitmapsets into null pointers.

These are all dead code now that it's done centrally.

Patch by me; thanks to Nathan Bossart and Richard Guo for review.

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

2 years agoRequire empty Bitmapsets to be represented as NULL.
Tom Lane [Thu, 2 Mar 2023 16:47:26 +0000 (11:47 -0500)]
Require empty Bitmapsets to be represented as NULL.

When I designed the Bitmapset module, I set things up so that an empty
Bitmapset could be represented either by a NULL pointer, or by an
allocated object all of whose bits are zero.  I've recently come to
the conclusion that that was a bad idea and we should instead have a
convention like the longstanding invariant for Lists, whereby an empty
list is represented by NIL and nothing else.

To do this, we need to fix bms_intersect, bms_difference, and a couple
of other functions to check for having produced an empty result; but
then we can replace bms_is_empty(a) by a simple "a == NULL" test.

This is very likely a (marginal) win performance-wise, because we
call bms_is_empty many more times than those other functions put
together.  However, the real reason to do it is that we have various
places that have hand-implemented a rule about "this Bitmapset
variable must be exactly NULL if empty", so that they can use
checks-for-null in place of bms_is_empty calls in particularly hot
code paths.  That is a really fragile, mistake-prone way to do things,
and I'm surprised that we've seldom been bitten by it.  It's not well
documented at all which variables have this property, so you can't
readily tell which code might be violating those conventions.  By
making the convention universal, we can eliminate a subtle source of
bugs.

Patch by me; thanks to Nathan Bossart and Richard Guo for review.

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

2 years agoMop up some undue familiarity with the innards of Bitmapsets.
Tom Lane [Thu, 2 Mar 2023 16:37:37 +0000 (11:37 -0500)]
Mop up some undue familiarity with the innards of Bitmapsets.

nodeAppend.c used non-nullness of appendstate->as_valid_subplans as
a state flag to indicate whether it'd done ExecFindMatchingSubPlans
(or some sufficient approximation to that).  This was pretty
questionable even in the beginning, since it wouldn't really work
right if there are no valid subplans.  It got more questionable
after commit 27e1f1456 added logic that could reduce as_valid_subplans
to an empty set: at that point we were depending on unspecified
behavior of bms_del_members, namely that it'd not return an empty
set as NULL.  It's about to start doing that, which breaks this
logic entirely.  Hence, add a separate boolean flag to signal
whether as_valid_subplans has been computed.

Also fix a previously-cosmetic bug in nodeAgg.c, wherein it ignored
the return value of bms_del_member instead of updating its pointer.

Patch by me; thanks to Nathan Bossart and Richard Guo for review.

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

2 years agoRemove bms_first_member().
Tom Lane [Thu, 2 Mar 2023 16:34:29 +0000 (11:34 -0500)]
Remove bms_first_member().

This function has been semi-deprecated ever since we invented
bms_next_member().  Its habit of scribbling on the input bitmapset
isn't great, plus for sufficiently large bitmapsets it would take
O(N^2) time to complete a loop.  Now we have the additional problem
that reducing the input to empty while leaving it still accessible
would violate a planned invariant.  So let's just get rid of it,
after updating the few extant callers to use bms_next_member().

Patch by me; thanks to Nathan Bossart and Richard Guo for review.

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

2 years agoMark options as deprecated in usage output
Daniel Gustafsson [Thu, 2 Mar 2023 13:36:37 +0000 (14:36 +0100)]
Mark options as deprecated in usage output

Some deprecated options were not marked as such in usage output.  This
does so across the installed binaries in an attempt to provide consistent
markup for this.

Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://postgr.es/m/062C6A8A-A4E8-4F52-9E31-45F0C9E9915E@yesql.se

2 years agoFix outdated references to guc.c
Daniel Gustafsson [Thu, 2 Mar 2023 12:49:39 +0000 (13:49 +0100)]
Fix outdated references to guc.c

Commit 0a20ff54f split out the GUC variables from guc.c into a new file
guc_tables.c. This updates comments referencing guc.c regarding variables
which are now in guc_tables.c.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/6B50C70C-8C1F-4F9A-A7C0-EEAFCC032406@yesql.se

2 years agoMake some xlogreader messages more accurate
Peter Eisentraut [Thu, 2 Mar 2023 06:42:39 +0000 (07:42 +0100)]
Make some xlogreader messages more accurate

When you have some invalid WAL, you often get a message like "wanted
24, got 0".  This is a bit incorrect, since it really wanted *at
least* 24, not exactly 24.  This updates the messages to that effect,
and also adds that detail to one message where it was available but
not printed.

Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Jeevan Ladhe <jeevanladhe.os@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/726d782b-5e45-0c3e-d775-6686afe9aa83%40enterprisedb.com

2 years agopageinspect: Fix crash with gist_page_items()
Michael Paquier [Thu, 2 Mar 2023 05:03:02 +0000 (14:03 +0900)]
pageinspect: Fix crash with gist_page_items()

Attempting to use this function with a raw page not coming from a GiST
index would cause a crash, as it was missing the same sanity checks as
gist_page_items_bytea().  This slightly refactors the code so as all the
basic validation checks for GiST pages are done in a single routine,
in the same fashion as the pageinspect functions for hash and BRIN.

This fixes an issue similar to 076f4d9.  A test is added to stress for
this case.  While on it, I have added a similar test for
brin_page_items() with a combination make of a valid GiST index and a
raw btree page.  This one was already protected, but it was not tested.

Reported-by: Egor Chindyaskin
Author: Dmitry Koval
Discussion: https://postgr.es/m/17815-fc4a2d3b74705703@postgresql.org
Backpatch-through: 14

2 years agoAvoid fetching one past the end of translate()'s "to" parameter.
Tom Lane [Wed, 1 Mar 2023 16:30:17 +0000 (11:30 -0500)]
Avoid fetching one past the end of translate()'s "to" parameter.

This is usually harmless, but if you were very unlucky it could
provoke a segfault due to the "to" string being right up against
the end of memory.  Found via valgrind testing (so we might've
found it earlier, except that our regression tests lacked any
exercise of translate()'s deletion feature).

Fix by switching the order of the test-for-end-of-string and
advance-pointer steps.  While here, compute "to_ptr + tolen"
just once.  (Smarter compilers might figure that out for
themselves, but let's just make sure.)

Report and fix by Daniil Anisimov, in bug #17816.

Discussion: https://postgr.es/m/17816-70f3d2764e88a108@postgresql.org

2 years agoImprove wording in pg_dump compression docs
Tomas Vondra [Wed, 1 Mar 2023 15:08:33 +0000 (16:08 +0100)]
Improve wording in pg_dump compression docs

A couple minor corrections in pg_dump comments and docs, related to the
recently introduced compression API.

Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/20230227044910.GO1653@telsasoft.com

2 years agoFix condition in pg_dump TAP test
Tomas Vondra [Wed, 1 Mar 2023 14:53:51 +0000 (15:53 +0100)]
Fix condition in pg_dump TAP test

The condition checking compression support was parenthesized
incorrectly after adding lz4, so fix that.

Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/20230227044910.GO1653@telsasoft.com

2 years agomeson: Add equivalent of configure --disable-rpath option
Peter Eisentraut [Wed, 1 Mar 2023 06:41:02 +0000 (07:41 +0100)]
meson: Add equivalent of configure --disable-rpath option

Discussion: https://www.postgresql.org/message-id/flat/33e957e6-4b4e-b0ed-1cc1-6335a24543ff%40enterprisedb.com

2 years agodoc: Mention de-normalization of deallocated entries in pg_stat_statements
Michael Paquier [Wed, 1 Mar 2023 01:47:01 +0000 (10:47 +0900)]
doc: Mention de-normalization of deallocated entries in pg_stat_statements

The current implementation of query normalization in pg_stat_statements
is optimistic.  If an entry is deallocated between the post-analyze hook
and the planner and/or execution hook, it can be possible to find query
strings with literal constant values (like "SELECT 1, 2") rather than
their normalized flavor (like "SELECT $1, $2").

This commit adds in the documentation a paragraph about this limitation,
and that this risk can be reduced by increasing pg_stat_statements.max,
particularly if pg_stat_statements_info reports a high number of
deallocations.

Author: Sami Imseih
Discussion: https://postgr.es/m/9CFF3512-355B-4676-8CCC-6CF622F4DC1A@amazon.com

2 years agodoc: Update pg_stat_statements about query ID calculation of utilities
Michael Paquier [Wed, 1 Mar 2023 01:31:55 +0000 (10:31 +0900)]
doc: Update pg_stat_statements about query ID calculation of utilities

Since 3db72eb, the calculation of the query ID hash for utilities is not
done based on the textual query strings, but on their internal Query
representation, meaning that there can be an overlap when they use
literal constants.  The documentation of pg_stat_statements was missing
a refresh about that.

Extracted from a larger patch by me.

Discussion: https://postgr.es/m/Y+MRdEq9W9XVa2AB@paquier.xyz

2 years agoSuppress more compiler warnings in new pgstats code.
Tom Lane [Wed, 1 Mar 2023 01:30:37 +0000 (20:30 -0500)]
Suppress more compiler warnings in new pgstats code.

Per buildfarm, we didn't get rid of quite all of the
-Wtautological-constant-out-of-range-compare warnings
in pgstat_io.c.

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