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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
Peter Eisentraut [Fri, 3 Mar 2023 06:01:18 +0000 (07:01 +0100)]
Fix incorrect format placeholders
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
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
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
Michael Paquier [Tue, 28 Feb 2023 23:38:43 +0000 (08:38 +0900)]
doc: Fix description of pg_get_wal_stats_till_end_of_wal() in pg_walinspect
end_lsn was mentioned as an input parameter, but that should not be the
case. Error introduced in
58597ed.
Author: Nathan Bossart
Discussion: https://postgr.es/m/
20230228195740.GA1397484@nathanxps13
Backpatch-through: 15
Tom Lane [Tue, 28 Feb 2023 23:06:45 +0000 (18:06 -0500)]
Fix logic buglets in pg_dump's flagInhAttrs().
As it stands, flagInhAttrs() can make changes in table properties that
change decisions made at other tables during other iterations of its
loop. This is a pretty bad idea, since we visit the tables in OID
order which is not necessarily related to inheritance relationships.
So far as I can tell, the consequences are just cosmetic: we might
dump DEFAULT or GENERATED expressions that we don't really need to
because they match properties of the parent. Nonetheless, it's buggy,
and somebody might someday add functionality here that fails less
benignly when the traversal order varies.
One issue is that when we decide we needn't dump a particular
GENERATED expression, we physically unlink the struct for it,
so that it will now look like the table has no such expression,
causing the wrong choice to be made at any child visited later.
We can improve that by instead clearing the dobj.dump flag,
and taking care to check that flag when it comes time to dump
the expression or not.
The other problem is that if we decide we need to fake up a DEFAULT
NULL clause to override a default that would otherwise get inherited,
we modify the data structure in the reverse fashion, creating an
attrdefs entry where there hadn't been one. It's harder to avoid
doing that, but since the backend won't report a plain "DEFAULT NULL"
property we can modify the code to recognize ones we just added.
Add some commentary to perhaps forestall future mistakes of the
same ilk.
Since the effects of this seem only cosmetic, no back-patch.
Discussion: https://postgr.es/m/
1506298.
1676323579@sss.pgh.pa.us
Andrew Dunstan [Tue, 28 Feb 2023 20:47:07 +0000 (15:47 -0500)]
Remove unnecessary and problematic collate.windows.win1252 tests
Some windows instances can't handle setting lc_time to a non BCP 47
locale, and the removed tests in any case don't really make lots of
sense here.
Juan José Santamaría Flecha
Discussion: https://postgr.es/m/
237b255b-e063-a82e-66e1-
c63a12bf9664@dunslane.net
Tom Lane [Tue, 28 Feb 2023 01:27:48 +0000 (20:27 -0500)]
Drop test view when done with it.
The view just added by commit
53fe7e6cb decompiles differently
in v15 than HEAD (presumably as a consequence of
47bb9db75).
That causes failures in cross-version upgrade testing.
We could teach AdjustUpgrade.pm to compensate for that, but it
seems less painful to just drop the view after we're done with it.
Per buildfarm.
Michael Paquier [Mon, 27 Feb 2023 23:37:37 +0000 (08:37 +0900)]
Fix expected output of xml_2.out
Per buildfarm members snakefly, parula and prion, that reflect the
results coming from the latest versions of libxml2.
Oversight in
b8da37b in the shape of an incorrect copy-paste. The CI
was green, but it does not stress this expected output.
Michael Paquier [Mon, 27 Feb 2023 23:04:13 +0000 (08:04 +0900)]
Rework pg_input_error_message(), now renamed pg_input_error_info()
pg_input_error_info() is now a SQL function able to return a row with
more than just the error message generated for incorrect data type
inputs when these are able to handle soft failures, returning more
contents of ErrorData, as of:
- The error message (same as before).
- The error detail, if set.
- The error hint, if set.
- SQL error code.
All the regression tests that relied on pg_input_error_message() are
updated to reflect the effects of the rename.
Per discussion with Tom Lane and Andrew Dunstan.
Author: Nathan Bossart
Discussion: https://postgr.es/m/
139a68e1-bd1f-a9a7-b5fe-
0be9845c6311@dunslane.net
Tom Lane [Mon, 27 Feb 2023 22:21:31 +0000 (17:21 -0500)]
Suppress compiler warnings in new pgstats code.
Some clang versions whine about comparing an enum variable to
a value outside the range of the enum, on the grounds that the
result must be constant. In the cases we fix here, the loops
will terminate only if the enum variable can in fact hold a
value one beyond its declared range. While that's very likely
to always be true for these enum types, it still seems like a
poor coding practice to assume it; so use "int" loop variables
instead to silence the warnings. (This matches what we've done
in other places, for example loops over the range of ForkNumber.)
While at it, let's drop the XXX_FIRST macros for these enums and just
write zeroes for the loop start values. The apparent flexibility
seems rather illusory given that iterating up to one-less-than-
the-number-of-values is only correct for a zero-based range.
Melanie Plageman
Discussion: https://postgr.es/m/20520.
1677435600@sss.pgh.pa.us
Tom Lane [Mon, 27 Feb 2023 21:29:51 +0000 (16:29 -0500)]
Harden postgres_fdw tests against unexpected cache flushes.
postgres_fdw will close its remote session if an sinval cache reset
occurs, since it's possible that that means some FDW parameters
changed. We had two tests that were trying to ensure that the
session remains alive by setting debug_discard_caches = 0; but
that's not sufficient. Even though the tests seem stable enough
in the buildfarm, they flap a lot under CI.
In the first test, which is checking the ability to recover from
a lost connection, we can stabilize the results by just not
caring whether pg_terminate_backend() finds a victim backend.
If a reset did happen, there won't be a session to terminate
anymore, but the test can proceed anyway. (Arguably, we are
then not testing the unintentional-disconnect case, but as long
as that scenario is exercised in most runs I think it's fine;
testing the reset-driven case is of value too.)
In the second test, which is trying to verify the application_name
displayed in pg_stat_activity by a remote session, we had a race
condition in that the remote session might go away before we can
fetch its pg_stat_activity entry. We can close that race and make
the test more certainly test what it intends to by arranging things
so that the remote session itself fetches its pg_stat_activity entry
(based on PID rather than a somewhat-circular assumption about the
application name).
Both tests now demonstrably pass under debug_discard_caches = 1,
so we can remove that hack.
Back-patch into relevant back branches.
Discussion: https://postgr.es/m/
20230226194340.u44bkfgyz64c67i6@awork3.anarazel.de
Heikki Linnakangas [Mon, 27 Feb 2023 07:55:39 +0000 (09:55 +0200)]
Replace single-quotes with double-quotes in a few SGML attributes.
Both are valid SGML, but let's be consistent.
Author: Peter Smith
Discussion: https://www.postgresql.org/message-id/CAHut%2BPtghjg0SBUTv%3D4Bpcy68d1zD3VAnZ3wX1DQSp39XKD9Sw%40mail.gmail.com
Heikki Linnakangas [Mon, 27 Feb 2023 07:34:43 +0000 (09:34 +0200)]
pg_rewind: Remove notice in docs about running CHECKPOINT after promote.
Commit
009eeee746 made it unnecessary. pg_rewind now works on a
recently promoted standby.
Author: Kyotaro Horiguchi, Keisuke Kuroda
Discussion: https://www.postgresql.org/message-id/
aeb5f31a-8de2-40a8-64af-
ab659a309d6b%40iki.fi
Peter Eisentraut [Mon, 27 Feb 2023 06:45:44 +0000 (07:45 +0100)]
Update types in smgr API
Change data buffer to void *, from char *, and add const where
appropriate. This makes it match the File API (see also
2d4f1ba6cfc2f0a977f1c30bda9848041343e248) and stdio.
Discussion: https://www.postgresql.org/message-id/flat/
11dda853-bb5b-59ba-a746-
e168b1ce4bdb%40enterprisedb.com
Amit Kapila [Mon, 27 Feb 2023 03:02:45 +0000 (08:32 +0530)]
Change xl_hash_vacuum_one_page.ntuples from int to uint16.
This will create two bytes of padding space in xl_hash_vacuum_one_page which
can be used for future patches. This makes the datatype of
xl_hash_vacuum_one_page.ntuples same as gistxlogDelete.ntodelete which is
advisable as both are used for the same purpose.
Author: Bertrand Drouvot
Reviewed-by: Nathan Bossart
Discussion: https://postgr.es/m/
b0e20c40-cb7a-fc1c-c607-
2a78dac5021e@gmail.com
Tom Lane [Sun, 26 Feb 2023 19:05:03 +0000 (14:05 -0500)]
Silence more compiler warnings introduced by
d87d548cd0.
Per buildfarm, there are still a couple of functions where we
get warnings from compilers that don't know that elog(ERROR)
doesn't return.
Andrew Dunstan [Sun, 26 Feb 2023 11:48:41 +0000 (06:48 -0500)]
Don't force SQL_ASCII/no-locale for installcheck in vcregress.pl
It's been this way for a very long time, but it appears to have been
masking an issue that only manifests with different settings. Therefore,
run the tests in the installation's default encoding/locale.
Backpatch to all live branches.
Dean Rasheed [Sun, 26 Feb 2023 09:06:04 +0000 (09:06 +0000)]
Doc: Miscellaneous doc updates for MERGE.
Update a few places in the documentation that should mention MERGE
among the list of applicable commands. In a couple of places, a
slightly more detailed description of what happens for MERGE seems
appropriate.
Reviewed by Alvaro Herrera.
Discussion: http://postgr.es/m/CAEZATCWqHLcxab89ATMQZNGFG_mxDPM%2BjzkSbXKD3JYPfRGvtw%40mail.gmail.com
Tom Lane [Sat, 25 Feb 2023 19:44:14 +0000 (14:44 -0500)]
Fix MULTIEXPR_SUBLINK with partitioned target tables, yet again.
We already tried to fix this in commits
3f7323cbb et al (and follow-on
fixes), but now it emerges that there are still unfixed cases;
moreover, these cases affect all branches not only pre-v14. I thought
we had eliminated all cases of making multiple clones of an UPDATE's
target list when we nuked inheritance_planner. But it turns out we
still do that in some partitioned-UPDATE cases, notably including
INSERT ... ON CONFLICT UPDATE, because ExecInitPartitionInfo thinks
it's okay to clone and modify the parent's targetlist.
This fix is based on a suggestion from Andres Freund: let's stop
abusing the ParamExecData.execPlan mechanism, which was only ever
meant to handle initplans, and instead solve the execution timing
problem by having the expression compiler move MULTIEXPR_SUBLINK steps
to the front of their expression step lists. This is feasible because
(a) all branches still in support compile the entire targetlist of
an UPDATE into a single ExprState, and (b) we know that all
MULTIEXPR_SUBLINKs do need to be evaluated --- none could be buried
inside a CASE, for example. There is a minor semantics change
concerning the order of execution of the MULTIEXPR's subquery versus
other parts of the parent targetlist, but that seems like something
we can get away with. By doing that, we no longer need to worry
about whether different clones of a MULTIEXPR_SUBLINK share output
Params; their usage of that data structure won't overlap.
Per bug #17800 from Alexander Lakhin. Back-patch to all supported
branches. In v13 and earlier, we can revert
3f7323cbb and follow-on
fixes; however, I chose to keep the SubPlan.subLinkId field added
in
ccbb54c72. We don't need that anymore in the core code, but it's
cheap enough to fill, and removing a plan node field in a minor
release seems like it'd be asking for trouble.
Andres Freund and Tom Lane
Discussion: https://postgr.es/m/17800-
ff90866b3906c964@postgresql.org
Dean Rasheed [Sat, 25 Feb 2023 14:41:12 +0000 (14:41 +0000)]
Fix mishandling of OLD/NEW references in subqueries in rule actions.
If a rule action contains a subquery that refers to columns from OLD
or NEW, then those are really lateral references, and the planner will
complain if it sees such things in a subquery that isn't marked as
lateral. However, at rule-definition time, the user isn't required to
mark the subquery with LATERAL, and so it can fail when the rule is
used.
Fix this by marking such subqueries as lateral in the rewriter, at the
point where they're used.
Dean Rasheed and Tom Lane, per report from Alexander Lakhin.
Back-patch to all supported branches.
Discussion: https://postgr.es/m/
5e09da43-aaba-7ea7-0a51-
a2eb981b058b%40gmail.com
Jeff Davis [Fri, 24 Feb 2023 17:11:35 +0000 (09:11 -0800)]
Silence compiler warnings introduced by
d87d548cd0.
Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/
20230224002029.GQ1653@telsasoft.com
Peter Eisentraut [Fri, 24 Feb 2023 13:29:18 +0000 (14:29 +0100)]
Fix comment indentation and whitespace
The previous layout satisfied pgindent but failed the git whitespace
check. Fix by not putting the comment first in the line, which
pgindent does not handle well.
Discussion: https://www.postgresql.org/message-id/flat/
480e3c67-b703-46ff-a418-
d3b481d68372%40enterprisedb.com
Daniel Gustafsson [Fri, 24 Feb 2023 10:09:50 +0000 (11:09 +0100)]
Disallow NULLS NOT DISTINCT indexes for primary keys
A unique index which is created with non-distinct NULLS cannot be
used for backing a primary key constraint. Make sure to disallow
such table alterations and teach pg_dump to drop the non-distinct
NULLS clause on indexes where this has been set.
Bug: 17720
Reported-by: Reiner Peterke <zedaardv@drizzle.com>
Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/17720-
dab8ee0fa85d316d@postgresql.org
Daniel Gustafsson [Fri, 24 Feb 2023 07:49:28 +0000 (08:49 +0100)]
pg_dump: Remove move "blob" terminology
Commit
e9960732a9618 accidentally introduced the blob terminology in
error messages which had previously been altered by commit
35ce24c33
from "blob" to "LO". This reverts back to "LO".
Reported-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/
20230224.163127.
68506240520261483.horikyota.ntt@gmail.com
Discussion: https://www.postgresql.org/message-id/flat/
868a381f-4650-9460-1726-
1ffd39a270b4%40enterprisedb.com
Peter Eisentraut [Fri, 24 Feb 2023 07:02:48 +0000 (08:02 +0100)]
Fix incorrect format placeholders
Andres Freund [Fri, 24 Feb 2023 03:34:25 +0000 (19:34 -0800)]
meson: windows: Fix tmp_install + prefix computation with meson 1.0.1
In
d0366bfb3b21 I said:
Instead of trying to do this in meson.build, call out to the implementation
meson install uses. This isn't pretty, but it's more reliable than what we had
before.
Unfortunately it was too ugly - to fix a bug, meson 1.0.1 changed the way the
meson internal runpython helper works, resulting in the previous sys.argv[]
indices not working anymore. Just open-code it - it's just a few characters
longer.
Committing this quickly to allow ci/cfbot to work with meson 1.0.1 on windows.
Tom Lane [Thu, 23 Feb 2023 20:40:27 +0000 (15:40 -0500)]
Don't repeatedly register cache callbacks in pgoutput plugin.
Multiple cycles of starting up and shutting down the plugin within a
single session would eventually lead to "out of relcache_callback_list
slots", because pgoutput_startup blindly re-registered its cache
callbacks each time. Fix it to register them only once, as all other
users of cache callbacks already take care to do.
This has been broken all along, so back-patch to all supported branches.
Shi Yu
Discussion: https://postgr.es/m/OSZPR01MB631004A78D743D68921FFAD3FDA79@OSZPR01MB6310.jpnprd01.prod.outlook.com
Tomas Vondra [Thu, 23 Feb 2023 20:19:19 +0000 (21:19 +0100)]
Add LZ4 compression to pg_dump
Expand pg_dump's compression streaming and file APIs to support the lz4
algorithm. The newly added compress_lz4.{c,h} files cover all the
functionality of the aforementioned APIs. Minor changes were necessary
in various pg_backup_* files, where code for the 'lz4' file suffix has
been added, as well as pg_dump's compression option parsing.
Author: Georgios Kokolatos
Reviewed-by: Michael Paquier, Rachel Heaton, Justin Pryzby, Shi Yu, Tomas Vondra
Discussion: https://postgr.es/m/faUNEOpts9vunEaLnmxmG-DldLSg_ql137OC3JYDmgrOMHm1RvvWY2IdBkv_CRxm5spCCb_OmKNk2T03TMm0fBEWveFF9wA1WizPuAgB7Ss%3D%40protonmail.com
Jeff Davis [Thu, 23 Feb 2023 19:20:00 +0000 (11:20 -0800)]
Remove unnecessary #ifdef USE_ICU and branch.
Now that the provider-independent API pg_strnxfrm() is available, we
no longer need the special cases for ICU in hashfunc.c and varchar.c.
Reviewed-by: Peter Eisentraut, Peter Geoghegan
Discussion: https://postgr.es/m/
a581136455c940d7bd0ff482d3a2bd51af25a94f.camel%40j-davis.com
Jeff Davis [Thu, 23 Feb 2023 19:17:41 +0000 (11:17 -0800)]
Refactor to introduce pg_locale_deterministic().
Avoids the need of callers to test for NULL, and also avoids the need
to access the pg_locale_t structure directly.
Reviewed-by: Peter Eisentraut, Peter Geoghegan
Discussion: https://postgr.es/m/
a581136455c940d7bd0ff482d3a2bd51af25a94f.camel%40j-davis.com
Jeff Davis [Thu, 23 Feb 2023 18:55:20 +0000 (10:55 -0800)]
Refactor to add pg_strcoll(), pg_strxfrm(), and variants.
Offers a generally better separation of responsibilities for collation
code. Also, a step towards multi-lib ICU, which should be based on a
clean separation of the routines required for collation providers.
Callers with NUL-terminated strings should call pg_strcoll() or
pg_strxfrm(); callers with strings and their length should call the
variants pg_strncoll() or pg_strnxfrm().
Reviewed-by: Peter Eisentraut, Peter Geoghegan
Discussion: https://postgr.es/m/
a581136455c940d7bd0ff482d3a2bd51af25a94f.camel%40j-davis.com
Tomas Vondra [Thu, 23 Feb 2023 17:33:30 +0000 (18:33 +0100)]
Introduce a generic pg_dump compression API
Switch pg_dump to use the Compression API, implemented by
bf9aa490db.
The CompressFileHandle replaces the cfp* family of functions with a
struct of callbacks for accessing (compressed) files. This allows adding
new compression methods simply by introducing a new struct instance with
appropriate implementation of the callbacks.
Archives compressed using custom compression methods store an identifier
of the compression algorithm in their header instead of the compression
level. The header version is bumped.
Author: Georgios Kokolatos
Reviewed-by: Michael Paquier, Rachel Heaton, Justin Pryzby, Tomas Vondra
Discussion: https://postgr.es/m/faUNEOpts9vunEaLnmxmG-DldLSg_ql137OC3JYDmgrOMHm1RvvWY2IdBkv_CRxm5spCCb_OmKNk2T03TMm0fBEWveFF9wA1WizPuAgB7Ss%3D%40protonmail.com
Tom Lane [Thu, 23 Feb 2023 16:05:58 +0000 (11:05 -0500)]
Fix mis-handling of outer join quals generated by EquivalenceClasses.
It's possible, in admittedly-rather-contrived cases, for an eclass
to generate a derived "join" qual that constrains the post-outer-join
value(s) of some RHS variable(s) without mentioning the LHS at all.
While the mechanisms were set up to work for this, we fell foul of
the "get_common_eclass_indexes" filter installed by commit
3373c7155:
it could decide that such an eclass wasn't relevant to the join, so
that the required qual clause wouldn't get emitted there or anywhere
else.
To fix, apply get_common_eclass_indexes only at inner joins, where
its rule is still valid. At an outer join, fall back to examining all
eclasses that mention either input (or the OJ relid, though it should
be impossible for an eclass to mention that without mentioning either
input). Perhaps we can improve on that later, but the cost/benefit of
adding more complexity to skip some irrelevant eclasses is dubious.
To allow cheaply distinguishing outer from inner joins, pass the
ojrelid to generate_join_implied_equalities as a separate argument.
This also allows cleaning up some sloppiness that had crept into
the definition of its join_relids argument, and it allows accurate
calculation of nominal_join_relids for a child outer join. (The
latter oversight seems not to have been a live bug, but it certainly
could have caused problems in future.)
Also fix what might be a live bug in check_index_predicates: it was
being sloppy about what it passed to generate_join_implied_equalities.
Per report from Richard Guo.
Discussion: https://postgr.es/m/CAMbWs4-DsTBfOvXuw64GdFss2=M5cwtEhY=0DCS7t2gT7P6hSA@mail.gmail.com
Tomas Vondra [Thu, 23 Feb 2023 14:38:14 +0000 (15:38 +0100)]
Prepare pg_dump internals for additional compression methods
Commit
bf9aa490db introduced a compression API in compress_io.{c,h} to
make reuse easier, and allow adding more compression algorithms.
However, pg_backup_archiver.c was not switched to this API and continued
to call the compression directly.
This commit teaches pg_backup_archiver.c about the compression API, so
that it can benefit from
bf9aa490db (simpler code, easier addition of
new compression methods).
Author: Georgios Kokolatos
Reviewed-by: Michael Paquier, Rachel Heaton, Justin Pryzby, Tomas Vondra
Discussion: https://postgr.es/m/faUNEOpts9vunEaLnmxmG-DldLSg_ql137OC3JYDmgrOMHm1RvvWY2IdBkv_CRxm5spCCb_OmKNk2T03TMm0fBEWveFF9wA1WizPuAgB7Ss%3D%40protonmail.com
Heikki Linnakangas [Thu, 23 Feb 2023 13:22:53 +0000 (15:22 +0200)]
pg_rewind: Fix determining TLI when server was just promoted.
If the source server was just promoted, and it hasn't written the
checkpoint record yet, pg_rewind considered the server to be still on
the old timeline. Because of that, it would claim incorrectly that no
rewind is required. Fix that by looking at minRecoveryPointTLI in the
control file in addition to the ThisTimeLineID on the checkpoint.
This has been a known issue since forever, and we had worked around it
in the regression tests by issuing a checkpoint after each promotion,
before running pg_rewind. But that was always quite hacky, so better
to fix this properly. This doesn't add any new tests for this, but
removes the previously-added workarounds from the existing tests, so
that they should occasionally hit this codepath again.
This is arguably a bug fix, but don't backpatch because we haven't
really treated it as a bug so far. Also, the patch didn't apply
cleanly to v13 and below. I'm sure sure it could be made to work on
v13, but doesn't seem worth the risk and effort.
Reviewed-by: Kyotaro Horiguchi, Ibrar Ahmed, Aleksander Alekseev
Discussion: https://www.postgresql.org/message-id/
9f568c97-87fe-a716-bd39-
65299b8a60f4%40iki.fi
Dean Rasheed [Thu, 23 Feb 2023 10:53:01 +0000 (10:53 +0000)]
Fix multi-row DEFAULT handling for INSERT ... SELECT rules.
Given an updatable view with a DO ALSO INSERT ... SELECT rule, a
multi-row INSERT ... VALUES query on the view fails if the VALUES list
contains any DEFAULTs that are not replaced by view defaults. This
manifests as an "unrecognized node type" error, or an Assert failure,
in an assert-enabled build.
The reason is that when RewriteQuery() attempts to replace the
remaining DEFAULT items with NULLs in any product queries, using
rewriteValuesRTEToNulls(), it assumes that the VALUES RTE is located
at the same rangetable index in each product query. However, if the
product query is an INSERT ... SELECT, then the VALUES RTE is actually
in the SELECT part of that query (at the same index), rather than the
top-level product query itself.
Fix, by descending to the SELECT in such cases. Note that we can't
simply use getInsertSelectQuery() for this, since that expects to be
given a raw rule action with OLD and NEW placeholder entries, so we
duplicate its logic instead.
While at it, beef up the checks in getInsertSelectQuery() by checking
that the jointree->fromlist node is indeed a RangeTblRef, and that the
RTE it points to has rtekind == RTE_SUBQUERY.
Per bug #17803, from Alexander Lakhin. Back-patch to all supported
branches.
Dean Rasheed, reviewed by Tom Lane.
Discussion: https://postgr.es/m/17803-
53c63ed4ecb4eac6%40postgresql.org
Daniel Gustafsson [Thu, 23 Feb 2023 08:25:47 +0000 (09:25 +0100)]
Consider a failed process as a failed test in pg_regress
Commit
55de145d1cf added reporting of child process failures, but the
test suite is still allowed to pass even if the process failed. Since
regress tests are higher level tests, a false positive is more likely
in this case so report failed test processes as failed tests.
Reported-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/
82C46B5E-1821-4039-82C2-
56BCA5992989@yesql.se
Discussion: https://postgr.es/m/
20221122235636.4frx7hjterq6bmls@awork3.anarazel.de
Andres Freund [Wed, 22 Feb 2023 22:27:56 +0000 (14:27 -0800)]
Add static assertion ensuring sizeof(ExprEvalStep) <= 64 bytes
This was previously only documented in a comment. Given the size of the
struct, it's not hard to miss that comment. As evidenced by the commits
leading up to
fe3caa14393,
67b26703b41.
It's possible, but not likely, that we might have to weaken these assertions
on a less commonly used architecture.
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/295606.
1677101684@sss.pgh.pa.us
Heikki Linnakangas [Wed, 22 Feb 2023 19:27:38 +0000 (21:27 +0200)]
Check for unbounded authentication exchanges in libpq.
A couple of code paths in CONNECTION_AWAITING_RESPONSE will eagerly read
bytes off a connection that should be closed. Don't let a misbehaving
server chew up client resources here; a v2 error can't be infinitely
long, and a v3 error should be bounded by its original message length.
For the existing error_return cases, I added some additional error
messages for symmetry with the new ones, and cleaned up some message
rot.
Author: Jacob Champion
Discussion: https://www.postgresql.org/message-id/
8e729daf-7d71-6965-9687-
8bc0630599b3%40timescale.com
Tom Lane [Wed, 22 Feb 2023 17:39:07 +0000 (12:39 -0500)]
Fix some issues with wrong placement of pseudo-constant quals.
initsplan.c figured that it could push Var-free qual clauses to
the top of the current JoinDomain, which is okay in the abstract.
But if the current domain is inside some outer join, and we later
commute an inside-the-domain outer join with one outside it,
we end up placing the pushed-up qual clause incorrectly.
In distribute_qual_to_rels, avoid this by using the syntactic scope
of the qual clause; with the exception that if we're in the top-level
join domain we can still use the full query relid set, ensuring the
resulting gating Result node goes to the top of the plan. (This is
approximately as smart as the pre-v16 code was. Perhaps we can do
better later, but it's not clear that such cases are worth a lot of
sweat.)
In process_implied_equality, we don't have a clear notion of syntactic
scope, but we do have the results of SpecialJoinInfo construction.
Thumb through those and remove any lower outer joins that might get
commuted to above the join domain. Again, we can make an exception
for the top-level join domain. It'd be possible to work harder here
(for example, by keeping outer joins that aren't shown as potentially
commutable), but I'm going to stop here for the moment. This issue
has convinced me that the current representation of join domains
probably needs further refinement, so I'm disinclined to write
inessential dependent logic just yet.
In passing, tighten the qualscope passed to process_implied_equality
by generate_base_implied_equalities_no_const; there's no need for
it to be larger than the rel we are currently considering.
Tom Lane and Richard Guo, per report from Tender Wang.
Discussion: https://postgr.es/m/CAHewXNk9eJ35ru5xATWioTV4+xZPHptjy9etdcNPjUfY9RQ+uQ@mail.gmail.com
Tomas Vondra [Wed, 22 Feb 2023 14:24:09 +0000 (15:24 +0100)]
Fix snapshot handling in logicalmsg_decode
Whe decoding a transactional logical message, logicalmsg_decode called
SnapBuildGetOrBuildSnapshot. But we may not have a consistent snapshot
yet at that point. We don't actually need the snapshot in this case
(during replay we'll have the snapshot from the transaction), so in
practice this is harmless. But in assert-enabled build this crashes.
Fixed by requesting the snapshot only in non-transactional case, where
we are guaranteed to have SNAPBUILD_CONSISTENT.
Backpatch to 11. The issue exists since 9.6.
Backpatch-through: 11
Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/
84d60912-6eab-9b84-5de3-
41765a5449e8@enterprisedb.com
Dean Rasheed [Wed, 22 Feb 2023 13:23:09 +0000 (13:23 +0000)]
Add missing support for the latest SPI status codes.
SPI_result_code_string() was missing support for SPI_OK_TD_REGISTER,
and in v15 and later, it was missing support for SPI_OK_MERGE, as was
pltcl_process_SPI_result().
The last of those would trigger an error if a MERGE was executed from
PL/Tcl. The others seem fairly innocuous, but worth fixing.
Back-patch to all supported branches. Before v15, this is just adding
SPI_OK_TD_REGISTER to SPI_result_code_string(), which is unlikely to
be seen by anyone, but seems worth doing for completeness.
Reviewed by Tom Lane.
Discussion:
https://postgr.es/m/CAEZATCUg8V%2BK%2BGcafOPqymxk84Y_prXgfe64PDoopjLFH6Z0Aw%40mail.gmail.com
https://postgr.es/m/CAEZATCUMe%2B_KedPMM9AxKqm%3DSZogSxjUcrMe%2BsakusZh3BFcQw%40mail.gmail.com
Dean Rasheed [Wed, 22 Feb 2023 10:51:34 +0000 (10:51 +0000)]
Fix Assert failure for MERGE into a partitioned table with RLS.
In ExecInitPartitionInfo(), the Assert when building the WITH CHECK
OPTION list for the new partition assumed that the command would be an
INSERT or UPDATE, but it can also be a MERGE. This can be triggered by
a MERGE into a partitioned table with RLS checks to enforce.
Fix, and back-patch to v15, where MERGE was introduced.
Discussion: https://postgr.es/m/CAEZATCWWFtQmW67F3XTyMU5Am10Oxa_b8oe0x%2BNu5Mo%2BCdRErg%40mail.gmail.com
John Naylor [Wed, 22 Feb 2023 10:22:43 +0000 (17:22 +0700)]
Remove newly added asserts from pg_bitutils.h
These were valuable during development, but are unlikely to tell us
anything going forward. This reverts
204b0cbec and adjusts the content
of
677319746 to more closely match the more-readable original style.
Per review from Tom Lane
Discussion: https://www.postgresql.org/message-id/
3567481.
1676906261%40sss.pgh.pa.us
Daniel Gustafsson [Wed, 22 Feb 2023 10:05:20 +0000 (11:05 +0100)]
doc: Add default value of createrole_self_grant
Document that the default value for createrole_self_grant is an empty
string which in turn disable the feature.
Author: Shi Yu <shiy.fnst@fujitsu.com>
Discussion: https://postgr.es/m/OSZPR01MB63105D0D96A9A72A7FCD4FFEFDA09@OSZPR01MB6310.jpnprd01.prod.outlook.com
Dean Rasheed [Wed, 22 Feb 2023 09:39:09 +0000 (09:39 +0000)]
Fix MERGE command tag for cross-partition updates.
This ensures that the row count in the command tag for a MERGE is
correctly computed. Previously, if MERGE updated a partitioned table,
the row count would be incorrect if any row was moved to a different
partition, since such updates were counted twice.
Back-patch to v15, where MERGE was introduced.
Discussion: https://postgr.es/m/CAEZATCWRMG7XX2QEsVL1LswmNo2d_YG8tKTLkpD3=Lp644S7rg@mail.gmail.com
Peter Eisentraut [Wed, 22 Feb 2023 08:32:12 +0000 (09:32 +0100)]
Implement ANY_VALUE aggregate
SQL:2023 defines an ANY_VALUE aggregate whose purpose is to emit an
implementation-dependent (i.e. non-deterministic) value from the
aggregated rows.
Author: Vik Fearing <vik@postgresfriends.org>
Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/
5cff866c-10a8-d2df-32cb-
e9072e6b04a2@postgresfriends.org
Peter Eisentraut [Wed, 22 Feb 2023 08:00:28 +0000 (09:00 +0100)]
gitattributes: Ignore imported pg_bsd_indent code for whitespace checks
Peter Eisentraut [Wed, 22 Feb 2023 07:21:16 +0000 (08:21 +0100)]
pg_dump: Remove some dead code
Client-side tracking of atttypmod has been unused since
64f3524, when
server-side format_type() started being used exclusively. So remove
this dead code.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/
144be239-c893-9361-704f-
ac85b5b98d1a%40enterprisedb.com
Michael Paquier [Wed, 22 Feb 2023 05:22:13 +0000 (14:22 +0900)]
Fix small memory leak in psql's \bind command
psql_scan_slash_option() returns a malloc()'d result through a
PQExpBuffer, and exec_command_bind() was doing an extra allocation of
this option for no effect.
Introduced in
5b66de3.
Author: Kyotaro Horiguchi
Reviewed-by: Corey Huinker
Discussion: https://postgr.es/m/
20230221.115555.
89096938631423206.horikyota.ntt@gmail.com
Michael Paquier [Wed, 22 Feb 2023 01:55:14 +0000 (10:55 +0900)]
doc: Add missing values for meson options -Dcassert and -Db_coverage
The -Dcassert and -Db_coverage that can be specified to a meson command
require values after an equal sign but the documentation did not
properly reflect that. All the other options specify the argument
values they expect, so close the gap.
Author: Jelte Fennema
Discussion: https://postgr.es/m/CAGECzQRXd1z+AoQ4tC5tqPk1_NQJohf6xwdEL=z9KgxHau2maQ@mail.gmail.com
Michael Paquier [Wed, 22 Feb 2023 01:14:52 +0000 (10:14 +0900)]
Fix corruption of templates after CREATE DATABASE .. STRATEGY WAL_LOG
WAL_LOG does a scan of the template's pg_class to determine the set of
relations that need to be copied from a template database to the new
one. However, as coded in
9c08aea, this copy strategy would load the
pages of pg_class without considering it as a permanent relation,
causing the loaded pages to never be flushed when they should. Any
modification of the template's pg_class, mostly through DDLs, would then
be missed, causing corruptions.
STRATEGY = WAL_LOG is the default over FILE_COPY since it has been
introduced, so any changes done to pg_class on a database template would
be gone. Updates of database templates should be a rare thing, so the
impact of this bug should be hopefully limited. The pre-14 default
strategy FILE_COPY is safe, and can be used as a workaround.
Ryo Matsumura has found and analyzed the issue, and Nathan has written a
test able to reproduce the failure (with few tweaks from me).
Backpatch down to 15, where STRATEGY = WAL_LOG has been introduced.
Author: Nathan Bossart, Ryo Matsumura
Reviewed-by: Dilip Kumar, Michael Paquier
Discussion: https://postgr.es/m/TYCPR01MB6868677E499C9AD5123084B5E8A39@TYCPR01MB6868.jpnprd01.prod.outlook.com
Backpatch-through: 15