Commit Graph

55657 Commits

Author SHA1 Message Date
Michael Paquier daa8365a90 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
2023-03-08 15:00:50 +09:00
Andres Freund be504a3e97 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 dc7420c2c9.  A lesser version of it exists in
12-13, introduced by fb5344c969, 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
2023-03-07 21:52:32 -08:00
Michael Paquier a4e003338d 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
2023-03-08 14:38:35 +09:00
Andres Freund 401874ab02 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
2023-03-07 18:44:42 -08:00
Michael Paquier d69cd3a2e2 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
2023-03-08 11:41:52 +09:00
Michael Paquier ee56048b0e 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
2023-03-08 09:16:44 +09:00
Tom Lane 99be6feec9 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
2023-03-07 18:21:53 -05:00
Peter Eisentraut ce1215d9b0 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
2023-03-07 20:31:16 +01:00
Peter Eisentraut af4d5715fc 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
2023-03-07 19:56:37 +01:00
Andres Freund 1be0fdb9de 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
2023-03-07 10:08:38 -08:00
Michael Paquier e20b1ea157 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
2023-03-07 14:18:20 +09:00
David Rowley cf96907aad 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
2023-03-07 14:33:28 +13:00
Michael Paquier 9a714b9d6e 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
2023-03-07 08:58:13 +09:00
Tom Lane 7fee7871b4 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
2023-03-06 18:31:27 -05:00
Michael Paquier d937904cce 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.
2023-03-07 07:42:36 +09:00
Tom Lane b803b7d132 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
2023-03-06 13:10:57 -05:00
Robert Haas e76cbb6cd6 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
2023-03-06 10:35:15 -05:00
Robert Haas f3948b5c91 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
2023-03-06 10:27:06 -05:00
Robert Haas 33352b9279 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
2023-03-06 10:15:32 -05:00
Daniel Gustafsson d3406d8036 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
2023-03-06 14:16:32 +01:00
Michael Paquier 4211fbd841 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
2023-03-06 16:41:05 +09:00
Michael Paquier 46d490ac19 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
2023-03-06 15:40:56 +09:00
Amit Kapila 9effa55236 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
2023-03-06 09:54:57 +05:30
Michael Paquier ce340e530d 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
2023-03-06 13:21:33 +09:00
Thomas Munro 47c0accbe0 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
2023-03-06 15:07:15 +13:00
Peter Eisentraut 102a5c164a 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
2023-03-05 15:19:58 +01:00
Tom Lane 6949b921d5 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
2023-03-04 13:32:35 -05:00
Tom Lane f62975b2af 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
2023-03-04 12:11:50 -05:00
Robert Haas ebd551f586 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
2023-03-03 12:52:04 -05:00
Peter Eisentraut b6a0d469ca 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
2023-03-03 07:45:52 +01:00
Peter Eisentraut b1307b8b60 Fix incorrect format placeholders 2023-03-03 07:01:18 +01:00
Michael Paquier d28a449854 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
2023-03-03 10:41:51 +09:00
Michael Paquier d0028e35a0 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
2023-03-03 08:46:11 +09:00
Tom Lane 98a88bc2bc Harden new test case against force_parallel_mode = regress.
Per buildfarm: worker processes can't see a role created in
the current transaction.
2023-03-02 17:47:20 -05:00
Tom Lane 3dfae91f7a 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
2023-03-02 17:15:13 -05:00
Thomas Munro 1da569ca1f 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
2023-03-03 10:43:33 +13:00
Tom Lane 6b661b01f4 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
2023-03-02 12:01:47 -05:00
Tom Lane 00b41463c2 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
2023-03-02 11:47:26 -05:00
Tom Lane 141225b251 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
2023-03-02 11:37:37 -05:00
Tom Lane 462bb7f128 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
2023-03-02 11:34:29 -05:00
Daniel Gustafsson 2f80c95740 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
2023-03-02 14:36:37 +01:00
Daniel Gustafsson 7ab1bc2939 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
2023-03-02 13:49:39 +01:00
Peter Eisentraut 4ac30ba4f2 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
2023-03-02 07:46:12 +01:00
Michael Paquier be753639d3 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
2023-03-02 14:03:02 +09:00
Tom Lane d7056bc1c7 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
2023-03-01 11:30:31 -05:00
Tomas Vondra 6095069b40 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
2023-03-01 16:11:38 +01:00
Tomas Vondra 34ce114374 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
2023-03-01 15:58:25 +01:00
Peter Eisentraut d2f44cc36e meson: Add equivalent of configure --disable-rpath option
Discussion: https://www.postgresql.org/message-id/flat/33e957e6-4b4e-b0ed-1cc1-6335a24543ff%40enterprisedb.com
2023-03-01 08:05:42 +01:00
Michael Paquier 6da67a0c11 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
2023-03-01 10:47:01 +09:00
Michael Paquier 6a88a05861 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
2023-03-01 10:31:55 +09:00