Commit Graph

55554 Commits

Author SHA1 Message Date
Alvaro Herrera 038f586d5f
pgbench: Prepare commands in pipelines in advance
Failing to do so results in an error when a pgbench script tries to
start a serializable transaction inside a pipeline, because by the time
BEGIN ISOLATION LEVEL SERIALIZABLE is executed, we're already in a
transaction that has acquired a snapshot, so the server rightfully
complains.

We can work around that by preparing all commands in the pipeline before
actually starting the pipeline.  This changes the existing code in two
aspects: first, we now prepare each command individually at the point
where that command is about to be executed; previously, we would prepare
all commands in a script as soon as the first command of that script
would be executed.  It's hard to see that this would make much of a
difference (particularly since it only affects the first time to execute
each script in a client), but I didn't actually try to measure it.

Secondly, we no longer use PQsendPrepare() in pipeline mode, but only
PQprepare.  There's no specific reason for this change other than no
longer needing to do differently in pipeline mode.  (Previously we had
no choice, because in pipeline mode PQprepare could not be used.)

Backpatch to 14, where pgbench got support for pipeline mode.

Reported-by: Yugo NAGATA <nagata@sraoss.co.jp>
Discussion: https://postgr.es/m/20210716153013.fc53b1c780b06fccc07a7f0d@sraoss.co.jp
2023-02-21 10:56:37 +01:00
Tom Lane 8028e294b4 Detect overflow in timestamp[tz] subtraction.
It's possible to overflow the int64 microseconds field of the
output interval when subtracting two timestamps.  Detect that
instead of silently returning a bogus result.

Nick Babadzhanian

Discussion: https://postgr.es/m/CABw73Uq2oJ3E+kYvvDuY04EkhhkChim2e-PaghBDjOmgUAMWGw@mail.gmail.com
2023-02-20 17:26:25 -05:00
Tom Lane f0d0394e84 Fix parsing of ISO-8601 interval fields with exponential notation.
Historically we've accepted interval input like 'P.1e10D'.  This
is probably an accident of having used strtod() to do the parsing,
rather than something anyone intended, but it's been that way for
a long time.  Commit e39f99046 broke this by trying to parse the
integer and fractional parts separately, without accounting for
the possibility of an exponent.  In principle that coding allowed
for precise conversions of field values wider than 15 decimal
digits, but that does not seem like a goal worth sweating bullets
for.  So, rather than trying to manage an exponent on top of the
existing complexity, let's just revert to the previous coding that
used strtod() by itself.  We can still improve on the old code to
the extent of allowing the value to range up to 1.0e15 rather than
only INT_MAX.  (Allowing more than that risks creating problems
due to precision loss: the converted fractional part might have
absolute value more than 1.  Perhaps that could be dealt with in
some way, but it really does not seem worth additional effort.)

Per bug #17795 from Alexander Lakhin.  Back-patch to v15 where
the faulty code came in.

Discussion: https://postgr.es/m/17795-748d6db3ed95d313@postgresql.org
2023-02-20 16:55:59 -05:00
Tom Lane f6db76c555 Prevent join removal from removing the query's result relation.
This was not something that required consideration before MERGE
was invented; but MERGE builds a join tree that left-joins to the
result relation, meaning that remove_useless_joins will consider
removing it.  That should generally be stopped by the query's use
of output variables from the result relation.  However, if the
result relation is inherited (e.g. a partitioned table) then
we don't add any row identity variables to the query until
expand_inherited_rtentry, which happens after join removal.

This was exposed as of commit 3c569049b, which made it possible
to deduce that a partitioned table could contain at most one row
matching a join key, enabling removal of the not-yet-expanded
result relation.  Ooops.

To fix, let's just teach join_is_removable that the query result
rel is never removable.  It's a cheap enough test in any case,
and it'll save some cycles that we'd otherwise expend in proving
that it's not removable, even in the cases we got right.

Back-patch to v15 where MERGE was added.  Although I think the
case cannot be reached in v15, this seems like cheap insurance.

Per investigation of a report from Alexander Lakhin.

Discussion: https://postgr.es/m/36bee393-b351-16ac-93b2-d46d83637e45@gmail.com
2023-02-20 15:18:32 -05:00
Jeff Davis 69e8c7cf1d Limit memory usage of pg_walinspect functions.
GetWALRecordsInfo() and pg_get_wal_fpi_info() can leak memory across
WAL record iterations. Fix this by using a temporary memory context
that's reset for each WAL record iteraion.

Also a use temporary context for loops in GetXLogSummaryStats(). The
number of iterations is a small constant, so the previous behavior was
not a leak, but fix for clarity (but no need to backport).

Backport GetWALRecordsInfo() change to version
15. pg_get_wal_fpi_info() didn't exist in version 15.

Reported-by: Peter Geoghegan
Author: Bharath Rupireddy
Discussion: https://www.postgresql.org/message-id/CAH2-WznLEJjn7ghmKOABOEZYuJvkTk%3DGKU3m0%2B-XBAH%2BerPiJQ%40mail.gmail.com
Backpatch-through: 15
2023-02-20 11:07:24 -08:00
Tom Lane c6c3b3bc3d Remove gratuitous assumptions about what make_modifytable can see.
For no clearly good reason, make_modifytable assumed that it
could not reach its get-the-FDW-info-the-hard-way path in MERGE.
It's currently possible to demonstrate that assertion failing,
which seems to be due to an upstream planner bug; but there's no
good reason to do it like this at all.  Let's apply the principle
of separation of concerns and make the MERGE check separately,
after getting or not getting the fdwroutine pointer.

Per report from Alexander Lakhin.  No test case, since I think
the potential test condition will go away soon.

Discussion: https://postgr.es/m/36bee393-b351-16ac-93b2-d46d83637e45@gmail.com
2023-02-20 12:06:30 -05:00
Alvaro Herrera a316a3bc6d
Correctly set userid of subquery relations' child rels
The RelOptInfo->userid field (the user ID to check permissions as) of an
"otherrel" relation was being copied from its parent relation, which is
correct in most cases but wrong when the parent is a subquery.  In that
case, using the value from the RTEPermissionInfo of the child itself is
the appropriate thing to do.

Coming up with a test case where user-visible behavior changes proves
hard enough, so we don't add one here.

Bug introduced by a61b1f7482, discovered by Amit while reviewing
nearby code.

Author: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/CA+HiwqE0WY_AhLnGtTsY7eYebG212XWbM-D8gr2A_ToOHyCywQ@mail.gmail.com
2023-02-20 16:00:42 +01:00
David Rowley 94cad7a3e6 Optimize generate_orderedappend_paths
In generate_orderedappend_paths(), when match_partition_order_desc was
true, we would lcons() items to various lists in a loop over each live
partition.  When the number of live partitions was large, the lcons()
could show up in profiles due to it having to perform memmove() to make
way for the new list item.

Here we adjust things so that we just perform the loop over the live
partitions backwards when match_partition_order_desc is true.  This allows
us to simplify the logic in the loop.  Now, as far as the guts of the loop
knows, there's no difference between match_partition_order and
match_partition_order_desc.  We can just set match_partition_order to true
so that we build the correct list of paths for the asc and desc case. Per
idea from Andres Freund.

Discussion: https://postgr.es/m/20230217002351.nyt4y5tdzg6hugdt@awork3.anarazel.de
2023-02-20 22:48:58 +13:00
John Naylor 6773197464 Add MSVC support for pg_leftmost_one_pos32() and friends
To allow testing for general support for fast bitscan intrinsics,
add symbols HAVE_BITSCAN_REVERSE and HAVE_BITSCAN_FORWARD.

Also do related cleanup in AllocSetFreeIndex(): Previously, we
tested for HAVE__BUILTIN_CLZ and copied the relevant internals of
pg_leftmost_one_pos32(), with a special fallback that does less
work than the general fallback for that function. Now that we have
a more general test, we just call pg_leftmost_one_pos32() directly
for platforms with intrinsic support. On gcc at least, there is no
difference in the binary for non-assert builds.

Discussion: https://www.postgresql.org/message-id/CAFBsxsEPc%2BFnX_0vmmQ5DHv60sk4rL_RZJ%2BMD6ei%3D76L0kFMvA%40mail.gmail.com
2023-02-20 14:55:32 +07:00
John Naylor 204b0cbecb Add assert checking to pg_leftmost_one_pos32() and friends
Discussion: https://www.postgresql.org/message-id/CAFBsxsEPc%2BFnX_0vmmQ5DHv60sk4rL_RZJ%2BMD6ei%3D76L0kFMvA%40mail.gmail.com
2023-02-20 14:16:34 +07:00
David Rowley 2cb82e2acf Speedup and increase usability of set proc title functions
The setting of the process title could be seen on profiles of very
fast-to-execute queries.  In many locations where we call
set_ps_display() we pass along a string constant, the length of which is
known during compilation.  Here we effectively rename set_ps_display() to
set_ps_display_with_len() and then add a static inline function named
set_ps_display() which calls strlen() on the given string.  This allows
the compiler to optimize away the strlen() call when dealing with
call sites passing a string constant.  We can then also use memcpy()
instead of strlcpy() to copy the string into the destination buffer.
That's significantly faster than strlcpy's byte-at-a-time way of
copying.

Here we also take measures to improve some code which was adjusting the
process title to add a " waiting" suffix to it.  Call sites which require
this can now just call set_ps_display_suffix() to add or adjust the suffix
and call set_ps_display_remove_suffix() to remove it again.

Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/CAApHDvocBvvk-0gWNA2Gohe+sv9fMcv+fK_G+siBKJrgDG4O7g@mail.gmail.com
2023-02-20 16:18:27 +13:00
Michael Paquier de2aca2885 Expand regression tests of pg_stat_statements for utility queries
This commit adds more coverage for utility statements so as it is
possible to track down all the effects of query normalization done for
all the queries that use either Const or A_Const nodes, which are the
nodes where normalization makes the most sense as they apply to
constants (well, most of the time, really).

This set of queries is extracted from an analysis done while looking at
full dumps of the regression database when applying different levels of
normalization to either Const or A_Const nodes for utilities, as of a
minimal set of these, for:
- All relkinds (CREATE, ALTER, DROP)
- Policies
- Cursors
- Triggers
- Types
- Rules
- Statistics
- CALL
- Transaction statements (isolation level, options)
- EXPLAIN
- COPY

Note that pg_stat_statements is not switched yet to show any
normalization for utilities, still it improves the default coverage of
the query jumbling code (not by as much as enabling query jumbling on
the main regression test suite, though):
- queryjumblefuncs.funcs.c: 36.8% => 48.5%
- queryjumblefuncs.switch.c: 33.2% => 43.1%

Reviewed-by: Bertrand Drouvot
Discussion: https://postgr.es/m/Y+MRdEq9W9XVa2AB@paquier.xyz
2023-02-20 10:16:51 +09:00
Michael Paquier e8dbdb15db Refactor tests of pg_stat_statements for planning, utility and level tracking
pg_stat_statements.sql acts as the main file for all the core tests of
the module, but things have become complicated to follow over the years
as some of the sub-scenarios tested in this file rely on assumptions
that come from completely different areas of it, like a GUC setup or a
relation created previously.  For example, row tracking for CTAS/COPY
was looking at the number of plans, which was not necessary, or level
tracking was mixed with checks on planner counts.

This commit refactors the tests of pg_stat_statements, by moving test
cases out of pg_stat_statements.sql into their own file, as of:
- Planning-related tests in planning.sql, for [re]plan counts and
top-level handling.  These depend on pg_stat_statements.track_planning.
- Utilities in utility.sql (pg_stat_statements.track_utility), that
includes now the tests for:
-- Row tracking for CTAS, CREATE MATERIALIZED VIEW, COPY.
-- Basic utility statements.
-- SET statements.
- Tracking level, depending on pg_stat_statements.track.  This part has
been looking at scenarios with DO blocks, PL functions and SQL
functions.

pg_stat_statements.sql (still named the same for now) still includes
some checks for role-level tracking and WAL generation metrics, that
ought to become independent in the long term for clarity.

While on it, this switches the order of the attributes when querying
pg_stat_statements, the query field becoming last.  This makes much
easier the tracking of changes related to normalization, as queries are
the only variable-length attributes queried (unaligned mode would be one
extra choice, but that reduces the checks on the other fields).

Test scenarios and their results match exactly with what was happening
before this commit in terms of calls, number of plans, number of rows,
cached data or level tracking, so this has no effect on the coverage in
terms of what is produced by the reports in the table
pg_stat_statements.  A follow-up patch will extend more the tests of
pg_stat_statements around utilities, so this split creates a foundation
for this purpose, without complicating more pg_stat_statements.sql.

Reviewed-by: Bertrand Drouvot
Discussion: https://postgr.es/m/Y+MRdEq9W9XVa2AB@paquier.xyz
2023-02-20 09:28:29 +09:00
Tomas Vondra e72910f808 Fix handling of multi-column BRIN indexes
When evaluating clauses on multiple scan keys of a multi-column BRIN
index, we can stop processing as soon as we find a scan key eliminating
the range, and the range should not be added to tbe bitmap.

That's how it worked before 14, but since a681e3c107 the code treated
the range as matching if it matched at least the last scan key.

Backpatch to 14, where this code was introduced.

Backpatch-through: 14
Discussion: https://postgr.es/m/ebc18613-125e-60df-7520-fcbe0f9274fc%40enterprisedb.com
2023-02-19 01:46:58 +01:00
Tom Lane 393430f575 Print the correct aliases for DML target tables in ruleutils.
ruleutils.c blindly printed the user-given alias (or nothing if there
hadn't been one) for the target table of INSERT/UPDATE/DELETE queries.
That works a large percentage of the time, but not always: for queries
appearing in WITH, it's possible that we chose a different alias to
avoid conflict with outer-scope names.  Since the chosen alias would
be used in any Var references to the target table, this'd lead to an
inconsistent printout with consequences such as dump/restore failures.

The correct logic for printing (or not) a relation alias was embedded
in get_from_clause_item.  Factor it out to a separate function so that
we don't need a jointree node to use it.  (Only a limited part of that
function can be reached from these new call sites, but this seems like
the cleanest non-duplicative factorization.)

In passing, I got rid of a redundant "\d+ rules_src" step in rules.sql.

Initial report from Jonathan Katz; thanks to Vignesh C for analysis.
This has been broken for a long time, so back-patch to all supported
branches.

Discussion: https://postgr.es/m/e947fa21-24b2-f922-375a-d4f763ef3e4b@postgresql.org
Discussion: https://postgr.es/m/CALDaNm1MMntjmT_NJGp-Z=xbF02qHGAyuSHfYHias3TqQbPF2w@mail.gmail.com
2023-02-17 16:40:34 -05:00
Peter Eisentraut 881a91781f Fix incorrect format placeholder 2023-02-17 14:10:48 +01:00
Michael Paquier 35739b87dc Redesign archive modules
A new callback named startup_cb, called shortly after a module is
loaded, is added.  This makes possible the initialization of any
additional state data required by a module.  This initial state data can
be saved in a ArchiveModuleState, that is now passed down to all the
callbacks that can be defined in a module.  With this design, it is
possible to have a per-module state, aimed at opening the door to the
support of more than one archive module.

The initialization of the callbacks is changed so as
_PG_archive_module_init() does not anymore give in input a
ArchiveModuleCallbacks that a module has to fill in with callback
definitions.  Instead, a module now needs to return a const
ArchiveModuleCallbacks.

All the structure and callback definitions of archive modules are moved
into their own header, named archive_module.h, from pgarch.h.
Command-based archiving follows the same line, with a new set of files
named shell_archive.{c,h}.

There are a few more items that are under discussion to improve the
design of archive modules, like the fact that basic_archive calls
sigsetjmp() by itself to define its own error handling flow.  These will
be adjusted later, the changes done here cover already a good portion
of what has been discussed.

Any modules created for v15 will need to be adjusted to this new
design.

Author: Nathan Bossart
Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/20230130194810.6fztfgbn32e7qarj@awork3.anarazel.de
2023-02-17 14:26:42 +09:00
Thomas Munro d2ea2d310d Remove obsolete platforms from ps_status.c.
Time to remove various code, comments and configure/meson probes
relating to ancient BSD, SunOS, GNU/Hurd, IRIX, NeXT and Unixware.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CA%2BhUKGJMNGUAqf27WbckYFrM-Mavy0RKJvocfJU%3DJ2XcAZyv%2Bw%40mail.gmail.com
2023-02-17 15:18:18 +13:00
Tom Lane a0fa18cc0d Fix check for child column generation status matching parent.
In commit 8bf6ec3ba, I mistakenly supposed that MergeAttributes'
loop over saved_schema was reprocessing column definitions that
had already been checked earlier: there is a variant syntax for
creating a child partition in which that's not true.  So we need
to duplicate the full check appearing further up.

(Actually, I believe that the "if (restdef->identity)" part is
not reachable, because we reject identity on partitions earlier.
But it seems wise to keep the check, in case that's ever relaxed,
and to keep this code in sync with the other instance.)

Per report from Alexander Lakhin.

Discussion: https://postgr.es/m/4a8200ca-8378-653e-38ed-b2e1f1611aa6@gmail.com
2023-02-16 18:51:55 -05:00
Andrew Dunstan afa7930462 pgindent: mention directory arguments in help text
Shi Yu
2023-02-16 11:42:13 -05:00
Michael Paquier 17feb6a566 Remove duplicated comment in nodeModifyTable.c
Author: Amul Sul
Discussion: https://postgr.es/m/CAAJ_b97badUU8_DHNoFCXZxF6YUk0Yb=53rrum168hd1haJgpQ@mail.gmail.com
2023-02-16 17:13:26 +09:00
Amit Kapila fce003cfde Add a new wait state and use it when sending data in the apply worker.
d9d7fe68d3 made use of an existing wait event when sending data from the
apply worker, but we should have invented a new wait event since this is a
new place to wait.

This patch corrects the mistake by using a new wait event
"LogicalApplySendData".

Author: Hou Zhijie
Reviewed-by: Peter Smith
Discussion: https://postgr.es/m/CA+TgmobWzbr9H3yN3dLVckviEZKemPwd+XyCFKEgyZQZhgP66Q@mail.gmail.com
2023-02-16 07:46:31 +05:30
Michael Paquier 1b43743f11 Add description for new patterns supported in HBA and ident sample files
Support for regexps in database and role entries for pg_hba.conf has
been added in 8fea8683, and efb6f4a has extended support of pg-user in
pg_ident.conf, still both of them have missed a short description about
the new patterns supported in their respective sample files.

This commit closes the gap, by providing a short description of all the
new features supported for each entry type.

Reported-by: Pavel Luzanov
Reviewed-by: Jelte Fennema, Pavel Luzanov
Discussion: https://postgr.es/m/e495112d-8741-e651-64a2-ecb5728f1a56@postgrespro.ru
2023-02-16 07:38:52 +09:00
Alvaro Herrera fd923b5de9
Don't rely on uninitialized value in MERGE / DELETE
On MERGE / WHEN MATCHED DELETE it's not possible to get cross-partition
updates, so we don't initialize cpUpdateRetrySlot; however, the code was
not careful to ignore the value in that case.  Make it do so.

Backpatch to 15.

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Discussion: https://postgr.es/m/17792-0f89452029662c36@postgresql.org
2023-02-15 20:37:44 +01:00
David Rowley 5352ca22e0 Rename force_parallel_mode to debug_parallel_query
force_parallel_mode is meant to be used to allow us to exercise the
parallel query infrastructure to ensure that it's working as we expect.
It seems some users think this GUC is for forcing the query planner into
picking a parallel plan regardless of the costs.  A quick look at the
documentation would have made them realize that they were wrong, but the
GUC is likely too conveniently named which, evidently, seems to often
result in users expecting that it forces the planner into usefully
parallelizing queries.

Here we rename the GUC to something which casual users are less likely to
mistakenly think is what they need to make their query run more quickly.

For now, the old name can still be used.  We'll revisit if the old name
mapping can be removed once the buildfarm configs are all updated.

Reviewed-by: John Naylor
Discussion: https://postgr.es/m/CAApHDvrsOi92_uA7PEaHZMH-S4Xv+MGhQWA+GrP8b1kjpS1HjQ@mail.gmail.com
2023-02-15 21:21:59 +13:00
Michael Paquier 8e0e0698f1 doc: Fix spelling of option -Dtap_tests for meson
This option was documented as -Dtap-tests, but the option requires an
underscore in its name.

Author: Katsuragi Yuta
Discussion: https://postgr.es/m/cff4558e170be78d8fd64ed45edbe1cb@oss.nttdata.com
2023-02-15 15:01:24 +09:00
Michael Paquier 9244c11afe Fix handling of SCRAM-SHA-256's channel binding with RSA-PSS certificates
OpenSSL 1.1.1 and newer versions have added support for RSA-PSS
certificates, which requires the use of a specific routine in OpenSSL to
determine which hash function to use when compiling it when using
channel binding in SCRAM-SHA-256.  X509_get_signature_nid(), that is the
original routine the channel binding code has relied on, is not able to
determine which hash algorithm to use for such certificates.  However,
X509_get_signature_info(), new to OpenSSL 1.1.1, is able to do it.  This
commit switches the channel binding logic to rely on
X509_get_signature_info() over X509_get_signature_nid(), which would be
the choice when building with 1.1.1 or newer.

The error could have been triggered on the client or the server, hence
libpq and the backend need to have their related code paths patched.
Note that attempting to load an RSA-PSS certificate with OpenSSL 1.1.0
or older leads to a failure due to an unsupported algorithm.

The discovery of relying on X509_get_signature_info() comes from Jacob,
the tests have been written by Heikki (with few tweaks from me), while I
have bundled the whole together while adding the bits needed for MSVC
and meson.

This issue exists since channel binding exists, so backpatch all the way
down.  Some tests are added in 15~, triggered if compiling with OpenSSL
1.1.1 or newer, where the certificate and key files can easily be
generated for RSA-PSS.

Reported-by: Gunnar "Nick" Bluth
Author: Jacob Champion, Heikki Linnakangas
Discussion: https://postgr.es/m/17760-b6c61e752ec07060@postgresql.org
Backpatch-through: 11
2023-02-15 10:12:16 +09:00
Tatsuo Ishii 87f21d2c68 Fix make_etags failure on Mac.
Previously make_etags always ran make_ctags -e when make_etags was
executed.  However, because non-Exuberant ctags on Mac does not
support -e option (and also on other platforms including old Linux),
ctags failed. To avoid the failure change make_ctags so that if
non-Exuberant ctags is used and ctags -e option is requested, run
etags command instead. If etags command does not exist, make_ctags
will fail.

Also refactor make_ctags and tweak make_etags to emit proper usage
message.

Author: Fujii Masao
Reviewed-by: Tatsuo Ishii
Discussion: https://www.postgresql.org/message-id/369c13b9-8b0f-d6f9-58fc-61258ec8f713%40oss.nttdata.com
2023-02-15 09:52:42 +09:00
Peter Eisentraut 3b12e68a5c Change argument type of pq_sendbytes from char * to void *
This is a follow-up to 1f605b82ba.  It
allows getting rid of further casts at call sites.

Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://www.postgresql.org/message-id/783a4edb-84f9-6df2-7470-2ef5ccc6607a@enterprisedb.com
2023-02-14 13:32:19 +01:00
Jeff Davis a8a44828a2 Correct docs for the default locale_provider of a new database.
If the locale provider is not specified, it defaults to be the same as
the template from which it was created. Previously, the documentation
said the default was libc.

Also adjust wording of CREATE DATABASE and CREATE COLLATION docs to be
definite that there are exactly two possible collation providers.

Discussion: https://postgr.es/m/6befdaada61c046b67f3b269f7fa6f069a35803e.camel%40j-davis.com
Reviewed-by: Nathan Bossart
2023-02-13 17:16:13 -08:00
Tom Lane e9a20e451f When removing a relation from the query, drop its RelOptInfo.
In commit b78f6264e I opined that it was "too risky" to delete a
relation's RelOptInfo from the planner's data structures when we have
realized that we don't need to join to it; so instead we just marked
it as a dead relation.  In hindsight that judgment seems flawed: any
subsequent access to such a dead relation is arguably a bug in
itself, so leaving the RelOptInfo present just helps to mask bugs.
Let's delete it instead, allowing removal of the whole notion of a
"dead relation".  So far as the regression tests can find, this
requires no other code changes, except for one Assert in equivclass.c
that was very dubiously not complaining about access to a dead rel.

Discussion: https://postgr.es/m/229905.1676062220@sss.pgh.pa.us
2023-02-13 13:35:38 -05:00
Tom Lane c7468c73f7 Fix buggy recursion in flatten_rtes_walker().
Must save-and-restore the context we are modifying.
Oversight in commit a61b1f748.

Tender Wang

Discussion: https://postgr.es/m/CAHewXNnnNySD_YcKNuFpQDV2gxWA7_YLWqHmYVcyoOYxn8kY2A@mail.gmail.com
Discussion: https://postgr.es/m/20230212233711.GA1316@telsasoft.com
2023-02-13 12:19:58 -05:00
Tom Lane f50f029c49 Fix thinkos in have_unsafe_outer_join_ref; reduce to Assert check.
Late in the development of commit 2489d76c4, I (tgl) incorrectly
concluded that the new function have_unsafe_outer_join_ref couldn't
ever reach its inner loop.  That should be the case if the inner
rel's parameterization is based on just one Var, but it could be
based on Vars from several relations, and then not only is the
inner loop reachable but it's wrongly coded.

Despite those errors, it still appears that the whole thing is
redundant given previous join_is_legal checks, so let's arrange
to only run it in assert-enabled builds.

Diagnosis and patch by Richard Guo, per fuzz testing by Justin Pryzby.

Discussion: https://postgr.es/m/20230212235823.GW1653@telsasoft.com
2023-02-13 11:45:32 -05:00
Andrew Dunstan b16259b3c1 Remove obsolete pgindent options --code-base and --build
Now that we have the sources for pg_bsd_indent in our code base these
are redundant.

It is now required to provide a list of files or directories to pgindent,
either by using --commit or on the command line. The equivalent of
previously running pgindent with no parameters is now `pgindent .`

Some extra checks are also added. duplicate files in the file list are
skipped, and there is a warning if no files are specified.

If the --commit option is used, the script now chdir's to the source
root, as git always reports files relative to that. (Fixes a gripe from
Justin Pryzby)

Reviewed by Tom Lane

Discussion: https://postgr.es/m/842819.1676219054@sss.pgh.pa.us
2023-02-13 08:24:54 -05:00
Alvaro Herrera 9a31256231
Fix object identity string for transforms
In commit ad89a5d115, we added an unhelpful 'ON' that doesn't match
the input syntax.  This was discovered while adding code to support for
DDL in logical replication.

No backpatch because of the change of behavior, however improbable it
may be that somebody is depending on this.

Author: Zheng Li <zhengli10@gmail.com>
Discussion: https://postgr.es/m/CAAD30UKg8rXeGM8Oy_MAmxKBL_K5DiHXdeNF=hUefcu1C_6VfQ@mail.gmail.com
2023-02-13 14:03:09 +01:00
Alvaro Herrera a1acdacada
Add wait_for_replay_catchup wrapper to Cluster.pm
This simplifies a few lines of Perl test code a bit.

Author: Bertrand Drouvot
Discussion: https://postgr.es/m/846724b5-0723-f4c2-8b13-75301ec7509e@gmail.com
2023-02-13 11:52:19 +01:00
David Rowley ec5a010ab2 Fix pfree issue in presorted DISTINCT aggregate code
The logic in this area was recently changed in 7da51590e, however, in that
commit, I neglected to consider that the conditions in which we should
pfree the old Datum needed to be updated after that change.  This could
result in trying to pfree a NULL value, as was demonstrated by Alexander
Lakhin.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/4103db46-d888-6d1d-e88d-87c21ed99472@gmail.com
2023-02-13 23:38:21 +13:00
Peter Eisentraut bd944884e9 Consolidate ItemPointer to Datum conversion functions
Instead of defining the same set of macros several times, define it
once in an appropriate header file.  In passing, convert to inline
functions.

Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://www.postgresql.org/message-id/flat/844dd4c5-e5a1-3df1-bfaf-d1e1c2a16e45%40enterprisedb.com
2023-02-13 09:57:15 +01:00
David Rowley 7da51590ed Fix incorrect presorted DISTINCT aggregate if condition
Here we fix a faulty "if" condition which failed to correctly handle two
or more consecutive NULL transition values when checking if the new value
is DISTINCT from the old value for presorted aggregates.  Given a suitably
non-strict aggregate transition function, a byref aggregate could cause a
crash due to calling the type's equality function and passing along a
(Datum) 0 value to test for equality, the equality function would then try
to dereference that 0 Datum and segfault.  For byval types, there'd have
been no crash and the equality function would have seen that the two 0
Datums matched, which (only by chance) meant the calling code would have
worked correctly.

Here we ensure that we only call the equality function when neither of
the input values are NULL.

This code is all new as of 1349d2790, so no backpatch needed.

Reported-by: Fujii Masao
Discussion: https://postgr.es/m/860c6d6f-a3c5-3ae9-9da2-827177bede06@oss.nttdata.com
2023-02-13 20:38:37 +13:00
David Rowley 836c31ba50 Disable WindowAgg inverse transitions when subplans are present
When an aggregate function is used as a WindowFunc and a tuple transitions
out of the window frame, we ordinarily try to make use of the aggregate
function's inverse transition function to "unaggregate" the exiting tuple.

This optimization is disabled for various cases, including when the
aggregate contains a volatile function.  In such a case we'd be unable to
ensure that the transition value was calculated to the same value during
transitions and inverse transitions.  Unfortunately, we did this check by
calling contain_volatile_functions() which does not recursively search
SubPlans for volatile functions.  If the aggregate function's arguments or
its FILTER clause contained a subplan with volatile functions then we'd
fail to notice this.

Here we fix this by just disabling the optimization when the WindowFunc
contains any subplans.  Volatile functions are not the only reason that a
subplan may have nonrepeatable results.

Bug: #17777
Reported-by: Anban Company
Discussion: https://postgr.es/m/17777-860b739b6efde977%40postgresql.org
Reviewed-by: Tom Lane
Backpatch-through: 11
2023-02-13 17:11:03 +13:00
Michael Paquier 2a507f6fd8 Mark more nodes with attribute no_query_jumble
This commit removes most of the Plan and Path nodes, which should never
be included in the query jumbling because we ignore these in Query
nodes.  This is facilitated by making no_query_jumble an inherited
attribute, like no_copy, no_equal and no_read when the supertype of a
node is found as marked with that.

RawStmt is not used in parsed queries, so it can be removed from the
query jumbling.  A couple of nodes defined in pathnodes.h, plannodes.h
and primnodes.h with NodeTag as supertype need to be marked
individually.

Forcing the execution of the query jumbling code with compute_query_id =
auto while pg_stat_statements is loaded brings the code coverage of
queryjumblefuncs.funcs.c to 95.6%.

The core code does not yet include a way to enforce the execution in
query jumbling except in pg_stat_statements, so the numbers I am
mentioning above will not reflect on the default coverage report with
just what is done in this commit.

Reported-by: Tom Lane
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/3344827.1675809127@sss.pgh.pa.us
2023-02-13 09:07:33 +09:00
Tom Lane 6ded4a5a3d Make pg_bsd_indent's .h files inclusion-order-safe.
As-is, they failed headerscheck.  Per buildfarm.
2023-02-12 13:06:37 -05:00
Tom Lane 5e80d35154 Avoid dereferencing an undefined pointer in DecodeInterval().
Commit e39f99046 moved some code up closer to the start of
DecodeInterval(), without noticing that it had been implicitly
relying on previous checks to reject the case of empty input.
Given empty input, we'd now dereference a pointer that hadn't been
set, possibly leading to a core dump.  (But if we fail to provoke
a SIGSEGV, nothing bad happens, and the expected syntax error is
thrown a bit later.)

Per bug #17788 from Alexander Lakhin.  Back-patch to v15 where
the fault was introduced.

Discussion: https://postgr.es/m/17788-dabac9f98f7eafd5@postgresql.org
2023-02-12 12:50:55 -05:00
Tom Lane 156c049bee Integrate pg_bsd_indent into our build/test infrastructure.
Update the Makefile and build directions for in-tree build,
and add Meson build infrastructure.  Also convert the ad-hoc
test target into a TAP test.

Currently, the Make build system will not build pg_bsd_indent
by default, while the Meson system will.  Both will test it
during "make check-world" or "ninja test".  Neither will install
it automatically.  (We might change some of these decisions later.)

Also fix a few portability nits noted during early testing.

Also, exclude pg_bsd_indent from pgindent's purview; at least for
now, we'll leave it formatted similarly to the FreeBSD original.

Tom Lane and Andres Freund

Discussion: https://postgr.es/m/3935719.1675967430@sss.pgh.pa.us
Discussion: https://postgr.es/m/20200812223409.6di3y2qsnvynao7a@alap3.anarazel.de
2023-02-12 12:22:21 -05:00
Tom Lane b44e5fced3 Sync pg_bsd_indent's copyright notices with Postgres practice.
To avoid confusion, make the copyright notices in these files
match the 3-clause form of the BSD license, per the blanket
policy update that UC Berkeley issued years ago.

Discussion: https://postgr.es/m/3935719.1675967430@sss.pgh.pa.us
Discussion: https://postgr.es/m/20230123011002.fzcaa3krql3mqsfn@awork3.anarazel.de
2023-02-12 12:03:01 -05:00
Tom Lane 4e831f4cee Import pg_bsd_indent sources.
This brings in an exact copy of the pg_bsd_indent repo as of
commit d301442799cea44e5ccb04331afc537764ec77c5 (2020-12-28).

Discussion: https://postgr.es/m/3935719.1675967430@sss.pgh.pa.us
Discussion: https://postgr.es/m/20200812223409.6di3y2qsnvynao7a@alap3.anarazel.de
2023-02-12 11:59:18 -05:00
Andrew Dunstan dab07e8c68 pgindent: filter files for the --commit option
per gripe from Shi Yu, solution from Jelte Fennema

Also add a check that the file exists, and issue a warning if it
doesn't.

As an efficiency measure, avoid processing any file more than once.

Discussion: https://postgr.es/m/TYAPR01MB6315B86619944D4A6B56842DFDDE9@TYAPR01MB6315.jpnprd01.prod.outlook.com
2023-02-12 08:51:01 -05:00
Andres Freund 10a082bf72 Add tests for pg_stat_io
Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/20200124195226.lth52iydq2n2uilq@alap3.anarazel.de
2023-02-11 10:05:04 -08:00
Andres Freund 9c83bbcf7e Create regress_tblspc in test_setup
An upcoming test needs to use a tablespace as part of its test. Historically,
we wanted tablespace creation be done in a dedicated file, so it's easy to
disable when testing replication. But that is not necessary anymore, due to
allow_in_place_tablespaces.

Create regress_tblspace tablespace in test_setup. Move the tablespace test to
the end of the parallel schedule, so other tests can use it.

Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20200124195226.lth52iydq2n2uilq@alap3.anarazel.de
2023-02-11 10:03:04 -08:00
Andres Freund a9c70b46db Add pg_stat_io view, providing more detailed IO statistics
Builds on 28e626bde0 and f30d62c2fc. See the former for motivation.

Rows of the view show IO operations for a particular backend type, IO target
object, IO context combination (e.g. a client backend's operations on
permanent relations in shared buffers) and each column in the view is the
total number of IO Operations done (e.g. writes). So a cell in the view would
be, for example, the number of blocks of relation data written from shared
buffers by client backends since the last stats reset.

In anticipation of tracking WAL IO and non-block-oriented IO (such as
temporary file IO), the "op_bytes" column specifies the unit of the "reads",
"writes", and "extends" columns for a given row.

Rows for combinations of IO operation, backend type, target object and context
that never occur, are ommitted entirely. For example, checkpointer will never
operate on temporary relations.

Similarly, if an IO operation never occurs for such a combination, the IO
operation's cell will be null, to distinguish from 0 observed IO
operations. For example, bgwriter should not perform reads.

Note that some of the cells in the view are redundant with fields in
pg_stat_bgwriter (e.g. buffers_backend). For now, these have been kept for
backwards compatibility.

Bumps catversion.

Author: Melanie Plageman <melanieplageman@gmail.com>
Author: Samay Sharma <smilingsamay@gmail.com>
Reviewed-by: Maciek Sakrejda <m.sakrejda@gmail.com>
Reviewed-by: Lukas Fittl <lukas@fittl.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/20200124195226.lth52iydq2n2uilq@alap3.anarazel.de
2023-02-11 09:52:15 -08:00