Commit Graph

55475 Commits

Author SHA1 Message Date
Robert Haas 8a2f783cc4 Disable STARTUP_PROGRESS_TIMEOUT in standby mode.
In standby mode, we don't actually report progress of recovery,
but up until now, startup_progress_timeout_handler() nevertheless
got called every log_startup_progress_interval seconds. That's
an unnecessary expense, so avoid it.

Report by Thomas Munro. Patch by Bharath Rupireddy, reviewed by
Simon Riggs, Thomas Munro, and me. Back-patch to v15, where
the problem was introduced.

Discussion: https://www.postgresql.org/message-id/CA%2BhUKGKCHSffAj8zZJKJvNX7ygnQFxVD6wm1d-2j3fVw%2BMafPQ%40mail.gmail.com
2023-02-06 10:51:08 -05:00
Peter Eisentraut 0ae4e49fa6 doc: Fix description of SQL standard parts
The splitting into parts actually started earlier than the text had
claimed, but that is ancient history anyway by now and does not need
to be mentioned.  Update that and tweak the text a bit.
2023-02-06 14:01:16 +01:00
Peter Eisentraut 54a177a948 Remove useless casts to (void *) in hash_search() calls
Some of these appear to be leftovers from when hash_search() took a
char * argument (changed in 5999e78fc4).

Since after this there is some more horizontal space available, do
some light reformatting where suitable.

Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/fd9adf5d-b1aa-e82f-e4c7-263c30145807%40enterprisedb.com
2023-02-06 09:41:01 +01:00
Michael Paquier 009f8d1714 Extend check_GUC_init() with checks on flag combinations when loading GUCs
This extends the work begun by a73952b, with the addition of a GUC check
for flag combinations in check_GUC_init(), making sure that anything
defined with GUC_NO_SHOW_ALL also includes GUC_NOT_IN_SAMPLE, as first
step.  There has never been any GUCs of this kind in the core code, and
this combination makes little sense as a parameter marked as not fit for
SHOW ALL should not be hidden in postgresql.conf.sample.

Note that GUCs marked with GUC_NO_SHOW_ALL are not listed under
pg_settings or SHOW ALL (still they can be queried individually), making
them unfit for checks via SQL queries in the regression tests that do a
full scan of the parameters available.  The SQL tests are still a bit
incorrect about that, and will be cleaned up in a separate commit.  We
have also discussed the possibility to extend the SQL functions for GUCs
so as they could show more information about parameters defined with
GUC_NO_SHOW_ALL, though it has been concluded that this is not worth the
extra complication in the long run, an enforced policy at initialization
time being enough to do the same job.

Per discussion with Nitin Jadhav and Tom Lane.

Discussion: https://postgr.es/m/CAMm1aWaYe0muu3ABo7iSAgK+OWDS9yNe8GGRYnCyeEpScYKa+g@mail.gmail.com
2023-02-06 15:22:04 +09:00
Michael Paquier d07c2948bf Add support for progress reporting to pg_verifybackup
This adds a new option to pg_verifybackup called -P/--progress, showing
every second some information about the progress of the checksum
verification based on the data of a backup manifest.

Similarly to what is done for pg_rewind and pg_basebackup, the
information printed in the progress report consists of the current
amount of data computed and the total amount of data that will be
computed.  Note that files found with an incorrect size do not have
their checksum verified, hence their size is not appended to the total
amount of data estimated during the first scan of the manifest data
(such incorrect sizes could be overly high, for one, falsifying the
progress report).

Author: Masahiko Sawada
Discussion: https://postgr.es/m/CAD21AoC5+JOgMd4o3z_oxw0f8JDSsCYY7zSbhe-O9x7f33rw_A@mail.gmail.com
2023-02-06 14:40:31 +09:00
Michael Paquier 71c37797d7 Properly NULL-terminate GSS receive buffer on error packet reception
pqsecure_open_gss() includes a code path handling error messages with
v2-style protocol messages coming from the server.  The client-side
buffer holding the error message does not force a NULL-termination, with
the data of the server getting copied to the errorMessage of the
connection.  Hence, it would be possible for a server to send an
unterminated string and copy arbitrary bytes in the buffer receiving the
error message in the client, opening the door to a crash or even data
exposure.

As at this stage of the authentication process the exchange has not been
completed yet, this could be abused by an attacker without Kerberos
credentials.  Clients that have a valid kerberos cache are vulnerable as
libpq opportunistically requests for it except if gssencmode is
disabled.

Author: Jacob Champion
Backpatch-through: 12
Security: CVE-2022-41862
2023-02-06 11:20:07 +09:00
Michael Paquier 2f6e15ac93 Revert refactoring of restore command code to shell_restore.c
This reverts commits 24c35ec and 57169ad.  PreRestoreCommand() and
PostRestoreCommand() need to be put closer to the system() call calling
a restore_command, as they enable in_restore_command for the startup
process which would in turn trigger an immediate proc_exit() in the
SIGTERM handler.  Perhaps we could get rid of this behavior entirely,
but 24c35ec has made the window where the flag is enabled much larger
than it was, and any Postgres-like actions (palloc, etc.) taken by code
paths while the flag is enabled could lead to more severe issues in the
shutdown processing.

Note that curculio has showed that there are much more problems in this
area, unrelated to this change, actually, hence the issues related to
that had better be addressed first.  Keeping the code of HEAD in line
with the stable branches should make that a bit easier.

Per discussion with Andres Freund and Nathan Bossart.

Discussion: https://postgr.es/m/Y979NR3U5VnWrTwB@paquier.xyz
2023-02-06 08:28:42 +09:00
Tom Lane b2d0e13a0a Fix over-optimistic updating of info about commutable outer joins.
make_outerjoininfo was set up to update SpecialJoinInfo's
commute_below, commute_above_l, commute_above_r fields as soon as
it found a pair of outer joins that look like they can commute.
However, this decision could be negated later in the same loop due
to finding an intermediate outer join that prevents commutation.
That left us with commute_xxx fields that were contradictory to the
join order restrictions expressed in min_lefthand/min_righthand.
The latter fields would keep us from actually choosing a bad join
order; but the inconsistent commute_xxx fields could bollix details
such as the varnullingrels values created for intermediate join
relation targetlists, ending in an assertion failure in setrefs.c.

To fix, wait till the end of make_outerjoininfo where we have
accurate values for min_lefthand/min_righthand, and then insert
only relids not present in those sets into the commute_xxx fields.

Per SQLSmith testing by Robins Tharakan.  Note that while Robins
bisected the failure to commit b448f1c8d, it's really the fault of
2489d76c4.  The outerjoin_delayed logic removed in the later commit
was keeping us from deciding that troublesome join pairs commute,
at least in the specific example seen here.

Discussion: https://postgr.es/m/CAEP4nAyAORgE8K_RHSmvWbE9UaChhjbEL1RrDU3neePwwRUB=A@mail.gmail.com
2023-02-05 14:25:10 -05:00
Tom Lane 9f452feeeb Fix thinko in qual distribution.
deconstruct_distribute tweaks the outer join scope (ojscope)
it passes to distribute_qual_to_rels when considering an outer
join qual that's above potentially-commutable outer joins.
However, if the current join is *not* potentially commutable,
we shouldn't do that.  The argument that distribute_qual_to_rels
will not do something wrong with the bogus ojscope falls flat
if we don't pass it non-null postponed_oj_qual_list.  Moreover,
there's no need to play games in this case since we aren't going
to commute anything.

Per SQLSmith testing by Robins Tharakan.

Discussion: https://postgr.es/m/CAEP4nAw74k4b-=93gmfCNX3MOY3y4uPxqbk_MnCVEpdsqHJVsg@mail.gmail.com
2023-02-04 17:40:35 -05:00
Tom Lane 8538519db1 Fix thinko in outer-join removal.
If we have a RestrictInfo that mentions both the removal-candidate
relation and the outer join's relid, then that is a pushed-down
condition not a join condition, so it should be grounds for deciding
that we can't remove the outer join.  In commit 2489d76c4, I'd blindly
included the OJ's relid into "joinrelids" as per the new standard
convention, but the checks of attr_needed and ph_needed should only
allow the join's input rels to be mentioned.

Having done that, the check for references in pushed-down quals
a few lines further down should be redundant.  I left it in place
as an Assert, though.

While researching this I happened across a couple of comments that
worried about the effects of update_placeholder_eval_levels.
That's gone as of b448f1c8d, so we can remove some worry.

Per bug #17769 from Robins Tharakan.  The submitted test case
triggers this more or less accidentally because we flatten out
a LATERAL sub-select after we've done join strength reduction;
if we did that in the other order, this problem would be masked
because the outer join would get simplified to an inner join.
To ensure that the committed test case will continue to test
what it means to even if we make that happen someday, use a
test clause involving COALESCE(), which will prevent us from
using it to do join strength reduction.

Patch by me, but thanks to Richard Guo for initial investigation.

Discussion: https://postgr.es/m/17769-e4f7a5c9d84a80a7@postgresql.org
2023-02-04 15:19:54 -05:00
Tom Lane 5840c20272 Rethink treatment of "postponed" quals in deconstruct_jointree().
After pulling up LATERAL subqueries, we may have qual clauses that
refer to relations outside their syntactic scope.  Before doing any
such pullup, prepjointree.c checks to make sure that it wouldn't
create a semantically-invalid situation; but we leave it to
deconstruct_jointree() to actually move these quals up the join
tree to a place where they can be evaluated.  In commit 2489d76c4,
I (tgl) refactored deconstruct_jointree() in a way that caused
assertion failures while moving such quals, because the new logic
failed to distinguish "this jointree node is a parent of the source
one" from "this jointree node is processed after the source
one in depth-first order".

Fix this, and at the same time reduce the overhead a bit, by
getting rid of the common PostponedQual list and instead making each
JoinTreeItem contain a list of quals that needed to be postponed to
its level.  We can help distribute_qual_to_rels find the appropriate
JoinTreeItem efficiently by adding parent-item links to the
JoinTreeItem data structure.  This ends up being the same number
of relid subset checks as the original (pre-bug) logic, but less
list manipulation is required during multi-level postponements.

Richard Guo and Tom Lane, per bug #17768 from Robins Tharakan.

Discussion: https://postgr.es/m/17768-5ac8730ece54478f@postgresql.org
2023-02-04 12:45:53 -05:00
Dean Rasheed faff8f8e47 Allow underscores in integer and numeric constants.
This allows underscores to be used in integer and numeric literals,
and their corresponding type input functions, for visual grouping.
For example:

    1_500_000_000
    3.14159_26535_89793
    0xffff_ffff
    0b_1001_0001

A single underscore is allowed between any 2 digits, or immediately
after the base prefix indicator of non-decimal integers, per SQL:202x
draft.

Peter Eisentraut and Dean Rasheed

Discussion: https://postgr.es/m/84aae844-dc55-a4be-86d9-4f0fa405cc97%40enterprisedb.com
2023-02-04 09:48:51 +00:00
Peter Eisentraut 1b6f632a35 Remove unused code related to unknown type
These are leftovers obsoleted by
cfd9be939e.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/e7887965-9e70-fd01-c2d1-5bc02f9169aa%40enterprisedb.com
2023-02-04 07:56:09 +01:00
Dean Rasheed b2d4792890 Make int64_div_fast_to_numeric() more robust.
The prior coding of int64_div_fast_to_numeric() had a number of bugs
that would cause it to fail under different circumstances, such as
with log10val2 <= 0, or log10val2 a multiple of 4, or in the "slow"
numeric path with log10val2 >= 10.

None of those could be triggered by any of our current code, which
only uses log10val2 = 3 or 6. However, they made it a hazard for any
future code that might use it. Also, since this is exported by
numeric.c, users writing their own C code might choose to use it.

Therefore fix, and back-patch to v14, where it was introduced.

Dean Rasheed, reviewed by Tom Lane.

Discussion: https://postgr.es/m/CAEZATCW8gXgW0tgPxPgHDPhVX71%2BSWFRkhnXy%2BTfGDsKLepu2g%40mail.gmail.com
2023-02-03 11:13:34 +00:00
Peter Eisentraut 2010d8b75a doc: Fix XML formatting that psql cannot handle
Breaking <phrase> over two lines is not handled by psql's
create_help.pl.  (It creates faulty \help output.)

Undo the formatting change introduced by
9bdad1b515 to fix this for now.
2023-02-03 09:04:35 +01:00
Andres Freund 98811323c8 ci: Use windows VMs instead of windows containers
So far we have used containers for testing windows on cirrus-ci. Unfortunately
they come with substantial overhead: First, the container images are pulled
onto the host on-demand. Due to the large size of windows containers, that
ends up taking nearly 4 minutes. Secondly, IO is slow, leading to CI runs
taking long.

Thus switch to windows VMs, improving windows CI times by well over 2x.

Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/211afb88-6df6-b74d-f1b7-84b5f21ad875@gmail.com
Backpatch: 15-, where CI was added
2023-02-02 21:31:24 -08:00
David Rowley 7ae0ab0ad9 Reduce code duplication between heapgettup and heapgettup_pagemode
The code to get the next block number was exactly the same between these
two functions, so let's just put it into a helper function and call that
from both locations.

Author: Melanie Plageman
Reviewed-by: Andres Freund, David Rowley
Discussion: https://postgr.es/m/CAAKRu_bvkhka0CZQun28KTqhuUh5ZqY=_T8QEqZqOL02rpi2bw@mail.gmail.com
2023-02-03 16:20:43 +13:00
Amit Kapila 3e577ff602 Optimize the origin drop functionality.
To interlock against concurrent drops, we use to hold ExclusiveLock on
pg_replication_origin till xact commit. This blocks even concurrent drops
of different origins by tablesync workers. So, instead, lock the specific
origin to interlock against concurrent drops.

This reduces the test time variability in src/test/subscription where
multiple tables are being synced.

Author: Vignesh C
Reviewed-by: Hou Zhijie, Amit Kapila
Discussion: https://postgr.es/m/1412708.1674417574@sss.pgh.pa.us
2023-02-03 08:29:08 +05:30
Thomas Munro 2e9f120b65 ci: Upgrade macOS version from 12 to 13.
Back-patch to 15, where in-tree CI began.

Author: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/1441145.1675300332%40sss.pgh.pa.us
2023-02-03 14:26:54 +13:00
David Rowley 8ca6d49f63 Add helper functions to simplify heapgettup code
Here we add heapgettup_start_page() and heapgettup_continue_page() to
simplify the code in the heapgettup() function.

Author: Melanie Plageman
Reviewed-by: David Rowley
Discussion: https://postgr.es/m/CAAKRu_bvkhka0CZQun28KTqhuUh5ZqY=_T8QEqZqOL02rpi2bw@mail.gmail.com
2023-02-03 12:38:42 +13:00
David Rowley f9bc34fcb6 Further refactor of heapgettup and heapgettup_pagemode
Backward and forward scans share much of the same page acquisition code.
Here we consolidate that code to reduce some duplication.

Additionally, add a new rs_coffset field to HeapScanDescData to track the
offset of the current tuple.  The new field fits nicely into the padding
between a bool and BlockNumber field and saves having to look at the last
returned tuple to figure out which offset we should be looking at for the
current tuple.

Author: Melanie Plageman
Reviewed-by: David Rowley
Discussion: https://postgr.es/m/CAAKRu_bvkhka0CZQun28KTqhuUh5ZqY=_T8QEqZqOL02rpi2bw@mail.gmail.com
2023-02-03 11:48:39 +13:00
Thomas Munro cdf6518ef0 Retire PG_SETMASK() macro.
In the 90s we needed to deal with computers that still had the
pre-standard signal masking APIs.  That hasn't been relevant for a very
long time on Unix systems, and c94ae9d8 got rid of a remaining
dependency in our Windows porting code.  PG_SETMASK didn't expose
save/restore functionality, so we'd already started using sigprocmask()
directly in places, creating the visual distraction of having two ways
to spell it.  It's not part of the API that extensions are expected to
be using (but if they are, the change will be trivial).  It seems like a
good time to drop the old macro and just call the standard POSIX
function.

Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKG%2BKfQgrhHP2DLTohX1WwubaCBHmTzGnAEDPZ-Gug-Xskg%40mail.gmail.com
2023-02-03 11:29:46 +13:00
Andrew Dunstan e0d70a91a0 Document installing perltidy with cpanm
Installing with plain cpan failed for me recently, as the archive it
searched has been purged of old releases. However, you can give cpanm a
complete URL to the exact version you want to install, so document using
that.
2023-02-02 08:54:38 -05:00
Dean Rasheed 0736fc1ceb Clarify the choice of rscale in numeric_sqrt().
Improve the comment explaining the choice of rscale in numeric_sqrt(),
and ensure that the code works consistently when other values of
NBASE/DEC_DIGITS are used.

Note that, in practice, we always expect DEC_DIGITS == 4, and this
does not change the computation in that case.

Joel Jacobson and Dean Rasheed

Discussion: https://postgr.es/m/06712c29-98e9-43b3-98da-f234d81c6e49%40app.fastmail.com
2023-02-02 09:41:22 +00:00
Dean Rasheed 9a84f2947b Ensure that numeric.c compiles with other NBASE values.
As noted in the comments, support for different NBASE values is really
only of historical interest, but as long as we're keeping it, we might
as well make sure that it compiles.

Joel Jacobson

Discussion: https://postgr.es/m/06712c29-98e9-43b3-98da-f234d81c6e49%40app.fastmail.com
2023-02-02 09:39:08 +00:00
Thomas Munro 117d2604c2 Doc: Abstract AF_UNIX sockets don't work on Windows.
An early release of AF_UNIX in Windows apparently supported Linux-style
"abstract" Unix sockets, but they do not seem to work in current Windows
versions and there is no mention of any of this in the Winsock
documentation.  Remove the mention of Windows from the documentation.

Back-patch to 14, where commit c9f0624b landed.

Discussion: https://postgr.es/m/CA%2BhUKGKrYbSZhrk4NGfoQGT_3LQS5pC5KNE1g0tvE_pPBZ7uew%40mail.gmail.com
2023-02-02 18:25:10 +13:00
Amit Kapila 9f2213a7c5 Allow the logical_replication_mode to be used on the subscriber.
Extend the existing developer option 'logical_replication_mode' to help
test the parallel apply of large transactions on the subscriber.

When set to 'buffered', the leader sends changes to parallel apply workers
via a shared memory queue. When set to 'immediate', the leader serializes
all changes to files and notifies the parallel apply workers to read and
apply them at the end of the transaction.

This helps in adding tests to cover the serialization code path in
parallel streaming mode.

Author: Hou Zhijie
Reviewed-by: Peter Smith, Kuroda Hayato, Sawada Masahiko, Amit Kapila
Discussion: https://postgr.es/m/CAA4eK1+wyN6zpaHUkCLorEWNx75MG0xhMwcFhvjqm2KURZEAGw@mail.gmail.com
2023-02-02 08:15:18 +05:30
David Rowley fb1a59de0c Refactor heapam.c adding heapgettup_initial_block function
Here we adjust heapgettup() and heapgettup_pagemode() to move the code
that fetches the first block number to scan out into a helper function.
This removes some code duplication.

Author: Melanie Plageman
Reviewed-by: David Rowley
Discussion: https://postgr.es/m/CAAKRu_bvkhka0CZQun28KTqhuUh5ZqY=_T8QEqZqOL02rpi2bw@mail.gmail.com
2023-02-02 14:17:15 +13:00
Peter Eisentraut 253432f426 meson: Fix typo in pkgconfig generation
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/flat/07b37c70-349a-8fcd-bcc9-6c3ce0f6c2a4%40enterprisedb.com
2023-02-01 18:14:01 +01:00
Michael Paquier 38cc085464 Simplify main waiting loop of the archiver process
As coded, the timeout given to WaitLatch() was always equal to
PGARCH_AUTOWAKE_INTERVAL, as time() was called two times repeatedly.
This simplification could have been done in d75288f.

While on it, this adjusts a comment in pgarch.c to describe the archiver
in a more neutral way.

Author: Sravan Kumar, Nathan Bossart
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/CA+=NbjjqYE9-Lnw7H7DAiS5jebmoMikwZQb_sBP7kgBCn9q6Hg@mail.gmail.com
2023-02-01 15:46:04 +09:00
Andres Freund fbed4bc59e dblink: Fix variable confusion introduced in e4602483e9
Thanks to Robins to find the issue and Nathan for promptly writing a test case
to prevent future problems like this.

Reported-by: Nathan Bossart <nathandbossart@gmail.com>
Reported-by: Robins Tharakan <tharakan@gmail.com>
Author: Nathan Bossart <nathandbossart@gmail.com>
Discussion: https://postgr.es/m/20230130193008.GA2740781@nathanxps13
2023-01-31 17:47:53 -08:00
Thomas Munro c289117505 Try to fix pg_upgrade test on Windows, again.
Further to commit 54e72b66e, if rmtree() fails while cleaning up in
pg_upgrade, try again.  This gives our Windows unlink() wrapper a chance
to reach its wait-for-the-other-process-to-go-away logic, if the first
go around initiated the unlink of a file that a concurrently exiting
program still has open.

Discussion: https://postgr.es/m/CA%2BhUKGKCVy2%3Do%3Dd8c2Va6a_3Rpf_KkhUitkWCZ3hzuO2VwLMXA%40mail.gmail.com
2023-02-01 14:40:25 +13:00
Tom Lane 758f44bc3a Update time zone data files to tzdata release 2022g.
DST law changes in Greenland and Mexico.  Notably, a new timezone
America/Ciudad_Juarez has been split off from America/Ojinaga.

Historical corrections for northern Canada, Colombia, and Singapore.
2023-01-31 17:36:55 -05:00
David Rowley e9aaf06328 Remove dead NoMovementScanDirection code
Here remove some dead code from heapgettup() and heapgettup_pagemode()
which was trying to support NoMovementScanDirection scans.  This code can
never be reached as standard_ExecutorRun() never calls ExecutePlan with
NoMovementScanDirection.

Additionally, plans which were scanning an unordered index would use
NoMovementScanDirection rather than ForwardScanDirection.  There was no
real need for this, so here we adjust this so we use ForwardScanDirection
for unordered index scans.  A comment in pathnodes.h claimed that
NoMovementScanDirection was used for PathKey reasons, but if that was
true, it no longer is, per code in build_index_paths().

This does change the non-text format of the EXPLAIN output so that
unordered index scans now have a "Forward" scan direction rather than
"NoMovement".  The text format of EXPLAIN has not changed.

Author: Melanie Plageman
Reviewed-by: Tom Lane, David Rowley
Discussion: https://postgr.es/m/CAAKRu_bvkhka0CZQun28KTqhuUh5ZqY=_T8QEqZqOL02rpi2bw@mail.gmail.com
2023-02-01 10:52:41 +13:00
Tom Lane 856ca51a23 Doc: clarify use of NULL to drop comments and security labels.
This was only mentioned in the description of the text/label, which
are marked as being in quotes in the synopsis, which can cause
confusion (as witnessed on IRC).

Also separate the literal and NULL cases in the parameter list, per
suggestion from Tom Lane.

Also add an example of dropping a security label.

Dagfinn Ilmari Mannsåker, with some tweaks by me

Discussion: https://postgr.es/m/87sffqk4zp.fsf@wibble.ilmari.org
2023-01-31 14:32:24 -05:00
Tom Lane eae0e20def Remove over-optimistic Assert.
In commit 2489d76c4, I'd thought it'd be safe to assert that a
PlaceHolderVar appearing in a scan-level expression has empty
nullingrels.  However this is not so, as when we determine that a
join relation is certainly empty we'll put its targetlist into a
Result-with-constant-false-qual node, and nothing is done to adjust
the nullingrels of the Vars or PHVs therein.  (Arguably, a Result
used in this way isn't really a scan-level node, but it certainly
isn't an upper node either ...)

It's not clear this is worth any close analysis, so let's just
take out the faulty Assert.

Per report from Robins Tharakan.  I added a test case based on
his example, just in case somebody tries to tighten this up.

Discussion: https://postgr.es/m/CAEP4nAz7Enq3+DEthGG7j27DpuwSRZnW0Nh6jtNh75yErQ_nbA@mail.gmail.com
2023-01-31 11:57:47 -05:00
Michael Paquier 3db72ebcbe Generate code for query jumbling through gen_node_support.pl
This commit changes the query jumbling code in queryjumblefuncs.c to be
generated automatically based on the information of the nodes in the
headers of src/include/nodes/ by using gen_node_support.pl.  This
approach offers many advantages:
- Support for query jumbling for all the utility statements, based on the
state of their parsed Nodes and not only their query string.  This will
greatly ease the switch to normalize the information of some DDLs, like
SET or CALL for example (this is left unchanged and should be part of a
separate discussion).  With this feature, the number of entries stored
for utilities in pg_stat_statements is reduced (for example now
"CHECKPOINT" and "checkpoint" mean the same thing with the same query
ID).
- Documentation of query jumbling directly in the structure definition
of the nodes.  Since this code has been introduced in pg_stat_statements
and then moved to code, the reasons behind the choices of what should be
included in the jumble are rather sparse.  Note that some explanation is
added for the most relevant parts, as a start.
- Overall code reduction and more consistency with the other parts
generating read, write and copy depending on the nodes.

The query jumbling is controlled by a couple of new node attributes,
documented in nodes/nodes.h:
- custom_query_jumble, to mark a Node as having a custom
implementation.
- no_query_jumble, to ignore entirely a Node.
- query_jumble_ignore, to ignore a field in a Node.
- query_jumble_location, to mark a location in a Node, for
normalization.  This can apply only to int fields, with "location" in
their name (only Const as of this commit).

There should be no compatibility impact on pg_stat_statements, as the
new code applies the jumbling to the same fields for each node (its
regression tests have no modification, for one).

Some benchmark of the query jumbling between HEAD and this commit for
SELECT and DMLs has proved that this new code does not cause a
performance regression, with computation times close for both methods.
For utility queries, the new method is slower than the previous method
of calculating a hash of the query string, though we are talking about
extra ns-level changes based on what I measured, which is unnoticeable
even for OLTP workloads as a query ID is calculated once per query
post-parse analysis.

Author: Michael Paquier
Reviewed-by: Peter Eisentraut
Discussion: https://postgr.es/m/Y5BHOUhX3zTH/ig6@paquier.xyz
2023-01-31 15:24:05 +09:00
Michael Paquier 8c1cd726c5 Remove recovery test 011_crash_recovery.pl
This test has been added as of 857ee8e that has introduced the SQL
function txid_status(), with the purpose of checking that a transaction
ID still in-progress during a crash is correctly marked as aborted after
recovery finishes.

This test is unstable, and some configuration scenarios may that easier
to reproduce (wal_level=minimal, wal_compression=on) because the WAL
holding the information about the in-progress transaction ID may not
have made it to disk yet, hence a post-crash recovery may cause the same
XID to be reused, triggering a test failure.

We have discussed a few approaches, like making this function force a
WAL flush to make it reliable across crashes, but we don't want to pay a
performance penalty in some scenarios, as well.  The test could have
been tweaked to enforce a checkpoint but that actually breaks the
promise of the test to rely on a stable result of txid_status() after
a crash.

This issue has been reported a few times across the past years, with an
original report from Kyotaro Horiguchi.  The buildfarm machines tanager,
hachi and gokiburi enable wal_compression, and fail on this test
periodically.

Discussion: https://postgr.es/m/3163112.1674762209@sss.pgh.pa.us
Discussion: https://postgr.es/m/20210305.115011.558061052471425531.horikyota.ntt@gmail.com
Backpatch-through: 11
2023-01-31 12:46:56 +09:00
Thomas Munro 54e72b66ed Refactor rmtree() to use get_dirent_type().
Switch to get_dirent_type() instead of lstat() while traversing a
directory tree, to see if that fixes the intermittent ENOTEMPTY failures
seen in recent pg_upgrade tests, on Windows CI.  While refactoring, also
use AllocateDir() instead of opendir() in the backend, which knows how
to handle descriptor pressure.

Our CI system currently uses Windows Server 2019, a version known not to
have POSIX unlink semantics enabled by default yet, unlike typical
Windows 10 and 11 systems.  That might explain why we see this flapping
on CI but (apparently) not in the build farm, though the frequency is
quite low.

The theory is that some directory entry must be in state
STATUS_DELETE_PENDING, which lstat() would report as ENOENT, though
unfortunately we don't know exactly why yet.  With this change, rmtree()
will not skip them, and try to unlink (again).  Our unlink() wrapper
should either wait a short time for them to go away when some other
process closes the handle, or log a message to tell us the path of the
problem file if not, so we can dig further.

Discussion: https://postgr.es/m/20220919213217.ptqfdlcc5idk5xup%40awork3.anarazel.de
2023-01-31 13:46:25 +13:00
Tom Lane 3bef56e116 Invent "join domains" to replace the below_outer_join hack.
EquivalenceClasses are now understood as applying within a "join
domain", which is a set of inner-joined relations (possibly underneath
an outer join).  We no longer need to treat an EC from below an outer
join as a second-class citizen.

I have hopes of eventually being able to treat outer-join clauses via
EquivalenceClasses, by means of only applying deductions within the
EC's join domain.  There are still problems in the way of that, though,
so for now the reconsider_outer_join_clause logic is still here.

I haven't been able to get rid of RestrictInfo.is_pushed_down either,
but I wonder if that could be recast using JoinDomains.

I had to hack one test case in postgres_fdw.sql to make it still test
what it was meant to, because postgres_fdw is inconsistent about
how it deals with quals containing non-shippable expressions; see
https://postgr.es/m/1691374.1671659838@sss.pgh.pa.us.  That should
be improved, but I don't think it's within the scope of this patch
series.

Patch by me; thanks to Richard Guo for review.

Discussion: https://postgr.es/m/830269.1656693747@sss.pgh.pa.us
2023-01-30 13:50:25 -05:00
Tom Lane b448f1c8d8 Do assorted mop-up in the planner.
Remove RestrictInfo.nullable_relids, along with a good deal of
infrastructure that calculated it.  One use-case for it was in
join_clause_is_movable_to, but we can now replace that usage with
a check to see if the clause's relids include any outer join
that can null the target relation.  The other use-case was in
join_clause_is_movable_into, but that test can just be dropped
entirely now that the clause's relids include outer joins.
Furthermore, join_clause_is_movable_into should now be
accurate enough that it will accept anything returned by
generate_join_implied_equalities, so we can restore the Assert
that was diked out in commit 95f4e59c3.

Remove the outerjoin_delayed mechanism.  We needed this before to
prevent quals from getting evaluated below outer joins that should
null some of their vars.  Now that we consider varnullingrels while
placing quals, that's taken care of automatically, so throw the
whole thing away.

Teach remove_useless_result_rtes to also remove useless FromExprs.
Having done that, the delay_upper_joins flag serves no purpose any
more and we can remove it, largely reverting 11086f2f2.

Use constant TRUE for "dummy" clauses when throwing back outer joins.
This improves on a hack I introduced in commit 6a6522529.  If we
have a left-join clause l.x = r.y, and a WHERE clause l.x = constant,
we generate r.y = constant and then don't really have a need for the
join clause.  But we must throw the join clause back anyway after
marking it redundant, so that the join search heuristics won't think
this is a clauseless join and avoid it.  That was a kluge introduced
under time pressure, and after looking at it I thought of a better
way: let's just introduce constant-TRUE "join clauses" instead,
and get rid of them at the end.  This improves the generated plans for
such cases by not having to test a redundant join clause.  We can also
get rid of the ugly hack used to mark such clauses as redundant for
selectivity estimation.

Patch by me; thanks to Richard Guo for review.

Discussion: https://postgr.es/m/830269.1656693747@sss.pgh.pa.us
2023-01-30 13:44:36 -05:00
Tom Lane 2489d76c49 Make Vars be outer-join-aware.
Traditionally we used the same Var struct to represent the value
of a table column everywhere in parse and plan trees.  This choice
predates our support for SQL outer joins, and it's really a pretty
bad idea with outer joins, because the Var's value can depend on
where it is in the tree: it might go to NULL above an outer join.
So expression nodes that are equal() per equalfuncs.c might not
represent the same value, which is a huge correctness hazard for
the planner.

To improve this, decorate Var nodes with a bitmapset showing
which outer joins (identified by RTE indexes) may have nulled
them at the point in the parse tree where the Var appears.
This allows us to trust that equal() Vars represent the same value.
A certain amount of klugery is still needed to cope with cases
where we re-order two outer joins, but it's possible to make it
work without sacrificing that core principle.  PlaceHolderVars
receive similar decoration for the same reason.

In the planner, we include these outer join bitmapsets into the relids
that an expression is considered to depend on, and in consequence also
add outer-join relids to the relids of join RelOptInfos.  This allows
us to correctly perceive whether an expression can be calculated above
or below a particular outer join.

This change affects FDWs that want to plan foreign joins.  They *must*
follow suit when labeling foreign joins in order to match with the
core planner, but for many purposes (if postgres_fdw is any guide)
they'd prefer to consider only base relations within the join.
To support both requirements, redefine ForeignScan.fs_relids as
base+OJ relids, and add a new field fs_base_relids that's set up by
the core planner.

Large though it is, this commit just does the minimum necessary to
install the new mechanisms and get check-world passing again.
Follow-up patches will perform some cleanup.  (The README additions
and comments mention some stuff that will appear in the follow-up.)

Patch by me; thanks to Richard Guo for review.

Discussion: https://postgr.es/m/830269.1656693747@sss.pgh.pa.us
2023-01-30 13:16:20 -05:00
Tom Lane ec7e053a98 Doc: clarify behavior of boolean options in replication commands.
defGetBoolean() allows the "value" part of "option = value"
syntax to be omitted, in which case it's taken as "true".
This is acknowledged in our syntax summaries for relevant commands,
but we don't seem to have documented the actual behavior anywhere.
Do so for CREATE/ALTER PUBLICATION/SUBSCRIPTION.  Use generic
boilerplate text for this, with the idea that we can copy-and-paste
it into other relevant reference pages, whenever someone gets
around to that.

Peter Smith, edited a bit by me

Discussion: https://postgr.es/m/CAHut+PvwjZfdGt2R8HTXgSZft=jZKymrS8KUg31pS7zqaaWKKw@mail.gmail.com
2023-01-30 11:59:37 -05:00
Dean Rasheed fe9e658f4d Ensure that MERGE recomputes GENERATED expressions properly.
This fixes a bug that, under some circumstances, would cause MERGE to
fail to properly recompute expressions for GENERATED STORED columns.

Formerly, ExecInitModifyTable() did not call ExecInitStoredGenerated()
for a MERGE command, which meant that the generated expressions
information was not computed until later, when the first merge action
was executed. However, if the first merge action to execute was an
UPDATE, then ExecInitStoredGenerated() could decide to skip some some
generated columns, if the columns on which they depended were not
updated, which was a problem if the MERGE also contained an INSERT
action, for which no generated columns should be skipped.

So fix by having ExecInitModifyTable() call ExecInitStoredGenerated()
for MERGE, and assume that it isn't safe to skip any generated columns
in a MERGE. Possibly that could be relaxed, by allowing some generated
columns to be skipped for a MERGE without an INSERT action, but it's
not clear that it's worth the effort.

Noticed while investigating bug #17759. Back-patch to v15, where MERGE
was added.

Dean Rasheed, reviewed by Tom Lane.

Discussion:
  https://postgr.es/m/17759-e76d9bece1b5421c%40postgresql.org
  https://postgr.es/m/CAEZATCXb_ezoMCcL0tzKwRGA1x0oeE%3DawTaysRfTPq%2B3wNJn8g%40mail.gmail.com
2023-01-30 10:04:57 +00:00
Amit Kapila 1e8b61735c Rename GUC logical_decoding_mode to logical_replication_mode.
Rename the developer option 'logical_decoding_mode' to the more flexible
name 'logical_replication_mode' because doing so will make it easier to
extend this option in the future to help test other areas of logical
replication.

Currently, it is used on the publisher side to allow streaming or
serializing each change in logical decoding. In the upcoming patch, we are
planning to use it on the subscriber. On the subscriber, it will allow
serializing the changes to file and notifies the parallel apply workers to
read and apply them at the end of the transaction.

We discussed exposing this parameter as a subscription option but
it did not seem advisable since it is primarily used for testing/debugging
and there is no other such parameter. We also discussed having separate
GUCs for publisher and subscriber but for current testing/debugging
requirements, one GUC is sufficient.

Author: Hou Zhijie
Reviewed-by: Peter Smith, Kuroda Hayato, Sawada Masahiko, Amit Kapila
Discussion: https://postgr.es/m/CAD21AoAy2c=Mx=FTCs+EwUsf2kQL5MmU3N18X84k0EmCXntK4g@mail.gmail.com
Discussion: https://postgr.es/m/CAA4eK1+wyN6zpaHUkCLorEWNx75MG0xhMwcFhvjqm2KURZEAGw@mail.gmail.com
2023-01-30 08:02:08 +05:30
Thomas Munro 8d2c1913ed Remove unneeded volatile qualifiers from postmaster.c.
Several flags were marked volatile and in some cases used sig_atomic_t
because they were accessed from signal handlers.  After commit 7389aad6,
we can just use unqualified bool.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA%2BhUKGLMoeZNZY6gYdLUQmuoW_a8bKyLvtuZkd_zHcGVOfDzBA%40mail.gmail.com
2023-01-28 15:06:23 +13:00
Tom Lane e4e89eb5bb Minor GUC code refactoring.
Split out "ConfigOptionIsVisible" to perform the privilege
check for GUC_SUPERUSER_ONLY GUCs (which these days can also
be read by pg_read_all_settings role members), and move the
should-we-show-it checks from GetConfigOptionValues to its
sole caller.

This commit also removes get_explain_guc_options's check of
GUC_NO_SHOW_ALL, which seems to have got cargo-culted in there.
While there's no obvious use-case for marking a GUC both
GUC_EXPLAIN and GUC_NO_SHOW_ALL, if it were set up that way
one would expect EXPLAIN to show it --- if that's not what
you want, then don't set GUC_EXPLAIN.

In passing, simplify the loop logic in show_all_settings.

Nitin Jadhav, Bharath Rupireddy, Tom Lane

Discussion: https://postgr.es/m/CAMm1aWYgfekpRK-Jz5=pM_bV+Om=ktGq1vxTZ_dr1Z6MV-qokA@mail.gmail.com
2023-01-27 12:13:41 -05:00
Andrew Dunstan a1c4cd6f2c Allow multiple --excludes options in pgindent
This includes a unification of the logic used to find the excludes file
and the typedefs file.

Also, remove the dangerous and deprecated feature where the first
non-option argument was taken as a typdefs file if it wasn't a .c or .h
file, remove some extraneous blank lines, and improve the documentation
somewhat.
2023-01-27 09:52:02 -05:00
Peter Eisentraut 8f6858064b meson: Fix installation path computation
We have the long-standing logic to append "postgresql" to some
installation paths if it does not already contain "pgsql" or
"postgres".  The existing meson implementation of that only considered
the subdirectory under the prefix, not the prefix itself.  Fix that,
so that it now works the same way as the implementation in
Makefile.global.

Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/a6a6de12-f705-2b33-2fd9-9743277deb08@enterprisedb.com
2023-01-27 11:57:11 +01:00
Peter Eisentraut 17e72ec45d doc: Adjust a few more references to "postmaster"
Reported-by: Karl O. Pinc <kop@karlpinc.com>
Discussion: https://www.postgresql.org/message-id/flat/ece84b69-8f94-8b88-925f-64207cb3a2f0@enterprisedb.com
2023-01-27 08:42:08 +01:00