Andres Freund [Sat, 11 Feb 2023 17:51:58 +0000 (09:51 -0800)]
Add pg_stat_io view, providing more detailed IO statistics
Builds on
28e626bde00 and
f30d62c2fc6. 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 <
[email protected]>
Author: Samay Sharma <
[email protected]>
Reviewed-by: Maciek Sakrejda <[email protected]>
Reviewed-by: Lukas Fittl <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
Reviewed-by: Justin Pryzby <[email protected]>
Discussion: https://postgr.es/m/
20200124195226[email protected]
Tom Lane [Fri, 10 Feb 2023 19:52:36 +0000 (14:52 -0500)]
Fix join removal logic to clean up sub-RestrictInfos of OR clauses.
analyzejoins.c took care to clean out removed relids from the
clause_relids and required_relids of RestrictInfos associated with
the doomed rel ... but it paid no attention to the fact that if such a
RestrictInfo contains an OR clause, there will be sub-RestrictInfos
containing similar fields.
I'm more than a bit surprised that this oversight hasn't caused
visible problems before. In any case, it's certainly broken now,
so add logic to clean out the sub-RestrictInfos recursively.
We might need to back-patch this someday.
Per bug #17786 from Robins Tharakan.
Discussion: https://postgr.es/m/17786-
f1ea7fbdab97daec@postgresql.org
Tom Lane [Fri, 10 Feb 2023 18:31:00 +0000 (13:31 -0500)]
Further fixes in qual nullingrel adjustment for outer join commutation.
One of the add_nulling_relids calls in deconstruct_distribute_oj_quals
added an OJ relid to too few Vars, while the other added it to too
many. We should consider the syntactic structure not
min_left/righthand while deciding which Vars to decorate, and when
considering pushing up a lower outer join pursuant to transforming the
second form of OJ identity 3 to the first form, we only want to
decorate Vars coming from its LHS.
In a related bug, I realized that make_outerjoininfo was failing to
check a very basic property that's needed to apply OJ identity 3:
the syntactically-upper outer join clause can't refer to the lower
join's LHS. This didn't break the join order restriction logic,
but it led to setting bogus commute_xxx bits, possibly resulting
in bogus nullingrel markings in modified quals.
Richard Guo and Tom Lane
Discussion: https://postgr.es/m/CAMbWs497CmBruMx1SOjepWEz+T5NWa4scqbdE9v7ZzSXqH_gQw@mail.gmail.com
Discussion: https://postgr.es/m/CAEP4nAx9C5gXNBfEA0JBfz7B+5f1Bawt-RWQWyhev-wdps8BZA@mail.gmail.com
Peter Eisentraut [Fri, 10 Feb 2023 11:00:51 +0000 (12:00 +0100)]
Fix incorrect format placeholder
Michael Paquier [Fri, 10 Feb 2023 06:37:41 +0000 (15:37 +0900)]
Fix typo in parsenodes.h
Introduced in
a61b1f7 when RTEPermissionInfo got added. Issue spotted
while reviewing the area for a different patch.
Andres Freund [Fri, 10 Feb 2023 06:22:26 +0000 (22:22 -0800)]
pgstat: Track more detailed relation IO statistics
Commit
28e626bde00 introduced the infrastructure for tracking more detailed IO
statistics. This commit adds the actual collection of the new IO statistics
for relations and temporary relations. See aforementioned commit for goals and
high-level design.
The changes in this commit are fairly straight-forward. The bulk of the change
is to passing sufficient information to the callsites of pgstat_count_io_op().
A somewhat unsightly detail is that it currently is hard to find a better
place to count fsyncs than in md.c, whereas the other pgstat_count_io_op()
calls are in bufmgr.c/localbuf.c. As the number of fsyncs is tied to md.c
implementation details, it's not obvious there is a better answer.
Author: Melanie Plageman <
[email protected]>
Reviewed-by: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/
20200124195226[email protected]
Andrew Dunstan [Thu, 9 Feb 2023 18:31:52 +0000 (13:31 -0500)]
Fix help text spacing in pgindent
Author: Noriyoshi Shinoda
Michael Paquier [Thu, 9 Feb 2023 05:43:53 +0000 (14:43 +0900)]
Fix various typos in code and tests
Most of these are recent, and the documentation portions are new as of
v16 so there is no need for a backpatch.
Author: Justin Pryzby
Discussion: https://postgr.es/m/
20230208155644[email protected]
Andres Freund [Thu, 9 Feb 2023 05:06:46 +0000 (21:06 -0800)]
Remove uses of AssertVariableIsOfType() obsoleted by
f2b73c8
Author: Nathan Bossart <
[email protected]>
Discussion: https://postgr.es/m/
20230208172705.GA451849@nathanxps13
Andres Freund [Thu, 9 Feb 2023 04:53:42 +0000 (20:53 -0800)]
pgstat: Infrastructure for more detailed IO statistics
This commit adds the infrastructure for more detailed IO statistics. The calls
to actually count IOs, a system view to access the new statistics,
documentation and tests will be added in subsequent commits, to make review
easier.
While we already had some IO statistics, e.g. in pg_stat_bgwriter and
pg_stat_database, they did not provide sufficient detail to understand what
the main sources of IO are, or whether configuration changes could avoid
IO. E.g., pg_stat_bgwriter.buffers_backend does contain the number of buffers
written out by a backend, but as that includes extending relations (always
done by backends) and writes triggered by the use of buffer access strategies,
it cannot easily be used to tune background writer or checkpointer. Similarly,
pg_stat_database.blks_read cannot easily be used to tune shared_buffers /
compute a cache hit ratio, as the use of buffer access strategies will often
prevent a large fraction of the read blocks to end up in shared_buffers.
The new IO statistics count IO operations (evict, extend, fsync, read, reuse,
and write), and are aggregated for each combination of backend type (backend,
autovacuum worker, bgwriter, etc), target object of the IO (relations, temp
relations) and context of the IO (normal, vacuum, bulkread, bulkwrite).
What is tracked in this series of patches, is sufficient to perform the
aforementioned analyses. Further details, e.g. tracking the number of buffer
hits, would make that even easier, but was left out for now, to keep the scope
of the already large patchset manageable.
Bumps PGSTAT_FILE_FORMAT_ID.
Author: Melanie Plageman <
[email protected]>
Reviewed-by: Andres Freund <[email protected]>
Reviewed-by: Justin Pryzby <[email protected]>
Reviewed-by: Kyotaro Horiguchi <[email protected]>
Discussion: https://postgr.es/m/
20200124195226[email protected]
Andres Freund [Thu, 9 Feb 2023 02:19:36 +0000 (18:19 -0800)]
Fix bugs in GetSafeSnapshotBlockingPids(), introduced in
96003717645
While removing the use of SHM_QUEUE from predicate.c, in
96003717645, I made
two mistakes in GetSafeSnapshotBlockingPids():
- Removed the check for output_size
- Previously, when the first loop didn't find a matching proc, sxact would be
NULL. But with naive use of dlist_foreach() it ends up as the value of the
last iteration.
The second issue is the cause of occasional failures in the deadlock-hard and
deadlock-soft isolation tests that we have been observing on CI. The issue was
very hard to reproduce, as it requires the transactions.sql regression test to
run at the same time as the deadlock-{hard,soft} isolation test.
I did not find other similar mistakes in
96003717645.
Discussion: https://postgr.es/m/
20230208221145[email protected]
Tom Lane [Wed, 8 Feb 2023 22:15:23 +0000 (17:15 -0500)]
Stop recommending auto-download of DTD files, and indeed disable it.
It appears no longer possible to build the SGML docs without a local
installation of the DocBook DTD, because sourceforge.net now only
permits HTTPS access, and no common version of xsltproc supports that.
Hence, remove the bits of our documentation suggesting that that's
possible or useful.
In fact, we might as well add the --nonet option to the build recipes
automatically, for a bit of extra security.
Also fix our documentation-tool-installation recipes for macOS to
ensure that xmllint and xsltproc are pulled in from MacPorts or
Homebrew. The previous recipes assumed you could use the
Apple-supplied versions of these tools; which still works, except that
you'd need to set an environment variable to ensure that they would
find DTD files provided by those package managers. Simpler and easier
to just recommend pulling in the additional packages.
In HEAD, also document how to build docs using Meson, and adjust
"ninja docs" to just build the HTML docs, for consistency with the
default behavior of doc/src/sgml/Makefile.
In a fit of neatnik-ism, I also made the ordering of the package
lists match the order in which the tools are described at the head
of the appendix.
Aleksander Alekseev, Peter Eisentraut, Tom Lane
Discussion: https://postgr.es/m/CAJ7c6TO8Aro2nxg=EQsVGiSDe-TstP4EsSvDHd7DSRsP40PgGA@mail.gmail.com
Andrew Dunstan [Wed, 8 Feb 2023 22:01:54 +0000 (17:01 -0500)]
pgindent: more ways to find files to indent
A new --commit option will add all the files in a commit to the file
list. The option can be specified more than once.
Also, if a directory is given on the command line, all the files in that
directory tree will be added to the file list.
Per suggestions from Robert Haas
Reviewed by Jelte Fennema
Discussion: https://postgr.es/m/CA+TgmoY59Ksso81RNLArNxj0a7xaqV_F_u7gSMHbgdc2kG5Vpw@mail.gmail.com
Tom Lane [Wed, 8 Feb 2023 19:45:36 +0000 (14:45 -0500)]
Further tighten nullingrel marking rules in build_joinrel_tlist().
The code I added in
fee7b77b9 could misbehave if commute_above_r
contains multiple relids. While adding too many relids here is
probably harmless (pre-
fee7b77b9, we did it all the time), it's
not very expensive to be accurate: we just have to intersect
commute_above_r with the join's relids.
Discussion: https://postgr.es/m/17781-
c0405c8b3cd5e072@postgresql.org
Tom Lane [Wed, 8 Feb 2023 19:08:46 +0000 (14:08 -0500)]
remove_rel_from_query() must clean up PlaceHolderVar.phrels fields.
While we got away with this sloppiness before, it's not okay now
that
fee7b77b9 caused build_joinrel_tlist() to make use of phrels.
Per report from Robins Tharakan.
Richard Guo (some cosmetic tweaks by me)
Discussion: https://postgr.es/m/CAMbWs4_ngw9sKxpTE8hqk=-ooVX_CQP3DarA4HzkRMz_JKpTrA@mail.gmail.com
Michael Paquier [Wed, 8 Feb 2023 07:56:21 +0000 (16:56 +0900)]
Remove SQL regression tests for GUCs related to NO_SHOW_ALL
No GUCs that use NO_SHOW_ALL are reported in pg_show_all_settings(),
hence trying to check combinations of flags related to it is pointless.
These queries have been introduced by
d10e41d, so backpatch down to 15
to keep all the branches consistent. Equivalent checks based on
NO_SHOW_ALL could be added in check_GUC_init() when a GUC is initially
loaded, but this can be done only on HEAD.
Author: Nitin Jadhav
Discussion: https://postgr.es/m/CAMm1aWaYe0muu3ABo7iSAgK+OWDS9yNe8GGRYnCyeEpScYKa+g@mail.gmail.com
Backpatch-through: 15
David Rowley [Wed, 8 Feb 2023 03:03:26 +0000 (16:03 +1300)]
Remove stray duplicated comment in heapam.h
This is just the same as what's written under the rs_numblocks field.
Reported-by: Melanie Plageman
Discussion: https://postgr.es/m/
20230207204127.7vs6krqjqn5farr7@liskov
Amit Kapila [Wed, 8 Feb 2023 02:28:25 +0000 (07:58 +0530)]
Fix the logical replication timeout during large DDLs.
The DDLs like Refresh Materialized views that generate lots of temporary
data due to rewrite rules may not be processed by output plugins (for
example pgoutput). So, we won't send keep-alive messages for a long time
while processing such commands and that can lead the subscriber side to
timeout. We have previously fixed a similar case for large transactions in
commit
f95d53eded where the output plugin filters all or most of the
changes but missed to handle the DDLs.
We decided not to backpatch this as this adds a new callback in the
existing exposed structure and moreover, users can increase the
wal_sender_timeout and wal_receiver_timeout to avoid this problem.
Author: Wang wei, Hou Zhijie
Reviewed-by: Peter Smith, Ashutosh Bapat, Shi yu, Amit Kapila
Discussion: https://postgr.es/m/OS3PR01MB6275478E5D29E4A563302D3D9E2B9@OS3PR01MB6275.jpnprd01.prod.outlook.com
Discussion: https://postgr.es/m/CAA5-nLARN7-3SLU_QUxfy510pmrYK6JJb=bk3hcgemAM_pAv+w@mail.gmail.com
Tom Lane [Tue, 7 Feb 2023 23:26:16 +0000 (18:26 -0500)]
Rethink nullingrel marking rules in build_joinrel_tlist().
The logic for when to add the current outer join's own relid
to the nullingrels sets of output Vars and PHVs was overly
complicated and underly correct. Not sure why I didn't think
of this before, but since what we want is marking per the
syntactic structure, we can just consult our records about
the syntactic structure, ie syn_righthand/syn_lefthand.
Also, tighten the rule about when to add the commute_above_r
bits, in hopes of eliminating some squishy reasoning. I do not
know of a reason to think that that's broken as-is, but this way
seems better.
Per bug #17781 from Robins Tharakan.
Discussion: https://postgr.es/m/17781-
c0405c8b3cd5e072@postgresql.org
Tom Lane [Tue, 7 Feb 2023 19:30:30 +0000 (14:30 -0500)]
Doc: make src/test/*/README match current reality.
Commit
c3382a3c3, which moved the implementation of PG_TEST_EXTRA
from src/test/Makefile into individual test scripts, broke the
directions given in the subdirectory README files about how to run
these tests by hand. Update. Also mention wal_consistency_checking
in recovery/README --- that omission isn't the fault of
c3382a3c3,
but it's still an omission.
Tom Lane [Tue, 7 Feb 2023 16:56:43 +0000 (11:56 -0500)]
Remove leftover code in deconstruct_distribute_oj_quals().
The initial "put back OJ relids" adjustment of ojscope was
incorrect and unnecessary; it seems to be a leftover from
when I (tgl) was trying to get this function to work at all.
Richard Guo
Discussion: https://postgr.es/m/CAMbWs4-L2C47ZGZPabBAi5oDZsKmsbvhYcGCy5o=gCjsaG_ZQA@mail.gmail.com
Peter Eisentraut [Tue, 7 Feb 2023 05:53:05 +0000 (06:53 +0100)]
Remove useless casts to (void *) in arguments of some system functions
The affected functions are: bsearch, memcmp, memcpy, memset, memmove,
qsort, repalloc
Reviewed-by: Corey Huinker <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/
fd9adf5d-b1aa-e82f-e4c7-
263c30145807%40enterprisedb.com
Amit Kapila [Tue, 7 Feb 2023 04:28:19 +0000 (09:58 +0530)]
Use appropriate wait event when sending data in the apply worker.
Currently, we reuse WAIT_EVENT_LOGICAL_PARALLEL_APPLY_STATE_CHANGE in the
apply worker while sending data to the parallel apply worker via a shared
memory queue. This is not appropriate as one won't be able to distinguish
whether the worker is waiting for sending data or for the state change.
To patch instead uses the wait event WAIT_EVENT_MQ_SEND which has been
already used in blocking mode while sending data via a shared memory
queue.
Author: Hou Zhijie
Reviewed-by: Kuroda Hayato, Amit Kapila
Discussion: https://postgr.es/m/OS0PR01MB57161C680B22E4C591628EE994DA9@OS0PR01MB5716.jpnprd01.prod.outlook.com
David Rowley [Tue, 7 Feb 2023 04:24:07 +0000 (17:24 +1300)]
More refactoring of heapgettup() and heapgettup_pagemode()
Here we further simplify the code in heapgettup() and
heapgettup_pagemode() to make better use of the helper functions added in
the previous recent refactors in this area.
In passing, remove an unneeded cast added in
8ca6d49f6.
Author: Melanie Plageman
Reviewed-by: Andres Freund, David Rowley
Discussion: https://postgr.es/m/CAAKRu_YSOnhKsDyFcqJsKtBSrd32DP-jjXmv7hL0BPD-z0TGXQ@mail.gmail.com
Michael Paquier [Tue, 7 Feb 2023 00:03:54 +0000 (09:03 +0900)]
Include values of A_Const nodes in query jumbling
Like the implementation for node copy, write and read, this node
requires a custom implementation so as the query jumbling is able to
consider the correct value assigned to it, depending on its type (int,
float, bool, string, bitstring).
Based on a dump of pg_stat_statements from the regression database, this
would confuse the query jumbling of the following queries:
- SET.
- COPY TO with SELECT queries.
- START TRANSACTION with different isolation levels.
- ALTER TABLE with default expressions.
- CREATE TABLE with partition bounds.
Note that there may be a long-term argument in tracking the location of
such nodes so as query strings holding such nodes could be normalized,
but this is left as a separate discussion.
Oversight in
3db72eb.
Discussion: https://postgr.es/m/
[email protected]
Andres Freund [Mon, 6 Feb 2023 23:41:38 +0000 (15:41 -0800)]
ci: freebsd: stop postgres instance on failure, to prevent log upload failures
Discussion: https://postgr.es/m/
20230205233057[email protected]
Discussion: https://postgr.es/m/
20230206221422[email protected]
Peter Eisentraut [Mon, 6 Feb 2023 21:58:44 +0000 (22:58 +0100)]
Fix more outdated comments
Same as in
f5da3d8 but for write_relcache_init_file(), the comments
had gotten a bit wrong due to code added over time.
Peter Eisentraut [Mon, 6 Feb 2023 21:39:48 +0000 (22:39 +0100)]
Fix up outdated comments
The existing comments in load_relcache_init_file() were not flexible
when new entries were added at the end, so they ended up a bit wrong.
Simplify the comments to avoid this issue.
Tom Lane [Mon, 6 Feb 2023 20:44:57 +0000 (15:44 -0500)]
Fix up join removal's interaction with PlaceHolderVars.
The portion of join_is_removable() that checks PlaceHolderVars
can be made a little more accurate and intelligible than it was.
The key point is that we can allow join removal even if a PHV
mentions the target rel in ph_eval_at, if that mention was only
added as a consequence of forcing the PHV up to a join level
that's at/above the outer join we're trying to get rid of.
We can check that by testing for the OJ's relid appearing in
ph_eval_at, indicating that it's supposed to be evaluated after
the outer join, plus the existing test that the contained
expression doesn't actually mention the target rel.
While here, add an explicit check that there'll be something left
in ph_eval_at after we remove the target rel and OJ relid. There
is an Assert later on about that, and I'm not too sure that the
case could happen for a PHV satisfying the other constraints,
but let's just check. (There was previously a bms_is_subset test
that meant to cover this risk, but it's broken now because it
doesn't account for the fact that we'll also remove the OJ relid.)
The real reason for revisiting this code though is that the
Assert I left behind in
8538519db turns out to be easily
reachable, because if a PHV of this sort appears in an upper-level
qual clause then that clause's clause_relids will include the
PHV's ph_eval_at relids. This is a mirage though: we have or soon
will remove these relids from the PHV's ph_eval_at, and therefore
they no longer belong in qual clauses' clause_relids either.
Remove that Assert in join_is_removable, and replace the similar
one in remove_rel_from_query with code to remove the deleted relids
from clause_relids.
Per bug #17773 from Robins Tharakan.
Discussion: https://postgr.es/m/17773-
a592e6cedbc7bac5@postgresql.org
Peter Eisentraut [Mon, 6 Feb 2023 16:54:10 +0000 (17:54 +0100)]
doc: Fix SQL keywords lists
Some keywords were listed as both reserved and nonreserved. Fix as
appropriate.
Robert Haas [Mon, 6 Feb 2023 15:51:08 +0000 (10:51 -0500)]
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
Peter Eisentraut [Mon, 6 Feb 2023 13:01:16 +0000 (14:01 +0100)]
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.
Peter Eisentraut [Mon, 6 Feb 2023 08:05:20 +0000 (09:05 +0100)]
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
5999e78fc45dcb91784b64b6e9ae43f4e4f68ca2).
Since after this there is some more horizontal space available, do
some light reformatting where suitable.
Reviewed-by: Corey Huinker <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/
fd9adf5d-b1aa-e82f-e4c7-
263c30145807%40enterprisedb.com
Michael Paquier [Mon, 6 Feb 2023 06:22:04 +0000 (15:22 +0900)]
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
Michael Paquier [Mon, 6 Feb 2023 05:40:31 +0000 (14:40 +0900)]
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
Michael Paquier [Mon, 6 Feb 2023 02:20:07 +0000 (11:20 +0900)]
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
Michael Paquier [Sun, 5 Feb 2023 23:28:42 +0000 (08:28 +0900)]
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/
[email protected]
Tom Lane [Sun, 5 Feb 2023 19:25:10 +0000 (14:25 -0500)]
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
Tom Lane [Sat, 4 Feb 2023 22:40:35 +0000 (17:40 -0500)]
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
Tom Lane [Sat, 4 Feb 2023 20:19:54 +0000 (15:19 -0500)]
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
Tom Lane [Sat, 4 Feb 2023 17:45:53 +0000 (12:45 -0500)]
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
Dean Rasheed [Sat, 4 Feb 2023 09:48:51 +0000 (09:48 +0000)]
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
Peter Eisentraut [Sat, 4 Feb 2023 06:56:09 +0000 (07:56 +0100)]
Remove unused code related to unknown type
These are leftovers obsoleted by
cfd9be939e9c516243c5b6a49ad1e1a9a38f1052.
Reviewed-by: Tom Lane <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/
e7887965-9e70-fd01-c2d1-
5bc02f9169aa%40enterprisedb.com
Dean Rasheed [Fri, 3 Feb 2023 11:13:34 +0000 (11:13 +0000)]
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
Peter Eisentraut [Fri, 3 Feb 2023 08:04:35 +0000 (09:04 +0100)]
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
9bdad1b5153e5d6b77a8f9c6e32286d6bafcd76d to fix this for now.
Andres Freund [Fri, 3 Feb 2023 05:31:24 +0000 (21:31 -0800)]
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 <
[email protected]>
Discussion: https://postgr.es/m/
211afb88-6df6-b74d-f1b7-
84b5f21ad875@gmail.com
Backpatch: 15-, where CI was added
David Rowley [Fri, 3 Feb 2023 03:20:43 +0000 (16:20 +1300)]
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
Amit Kapila [Fri, 3 Feb 2023 02:59:08 +0000 (08:29 +0530)]
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
Thomas Munro [Fri, 3 Feb 2023 01:26:54 +0000 (14:26 +1300)]
ci: Upgrade macOS version from 12 to 13.
Back-patch to 15, where in-tree CI began.
Author: Justin Pryzby <
[email protected]>
Discussion: https://postgr.es/m/
1441145.
1675300332%40sss.pgh.pa.us
David Rowley [Thu, 2 Feb 2023 23:38:42 +0000 (12:38 +1300)]
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
David Rowley [Thu, 2 Feb 2023 22:48:39 +0000 (11:48 +1300)]
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
Thomas Munro [Thu, 2 Feb 2023 21:34:56 +0000 (10:34 +1300)]
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 <[email protected]>
Discussion: https://postgr.es/m/CA%2BhUKG%2BKfQgrhHP2DLTohX1WwubaCBHmTzGnAEDPZ-Gug-Xskg%40mail.gmail.com
Andrew Dunstan [Thu, 2 Feb 2023 13:54:38 +0000 (08:54 -0500)]
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.
Dean Rasheed [Thu, 2 Feb 2023 09:41:22 +0000 (09:41 +0000)]
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
Dean Rasheed [Thu, 2 Feb 2023 09:39:08 +0000 (09:39 +0000)]
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
Thomas Munro [Thu, 2 Feb 2023 05:13:44 +0000 (18:13 +1300)]
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
Amit Kapila [Thu, 2 Feb 2023 02:45:18 +0000 (08:15 +0530)]
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
David Rowley [Thu, 2 Feb 2023 01:17:15 +0000 (14:17 +1300)]
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
Peter Eisentraut [Wed, 1 Feb 2023 17:14:01 +0000 (18:14 +0100)]
meson: Fix typo in pkgconfig generation
Reviewed-by: Andres Freund <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/
07b37c70-349a-8fcd-bcc9-
6c3ce0f6c2a4%40enterprisedb.com
Michael Paquier [Wed, 1 Feb 2023 06:46:04 +0000 (15:46 +0900)]
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
Andres Freund [Wed, 1 Feb 2023 01:36:39 +0000 (17:36 -0800)]
dblink: Fix variable confusion introduced in
e4602483e95
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 <[email protected]>
Reported-by: Robins Tharakan <[email protected]>
Author: Nathan Bossart <
[email protected]>
Discussion: https://postgr.es/m/
20230130193008.GA2740781@nathanxps13
Thomas Munro [Wed, 1 Feb 2023 01:29:51 +0000 (14:29 +1300)]
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
Tom Lane [Tue, 31 Jan 2023 22:36:55 +0000 (17:36 -0500)]
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.
David Rowley [Tue, 31 Jan 2023 21:52:41 +0000 (10:52 +1300)]
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
Tom Lane [Tue, 31 Jan 2023 19:32:24 +0000 (14:32 -0500)]
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/
[email protected]
Tom Lane [Tue, 31 Jan 2023 16:57:47 +0000 (11:57 -0500)]
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
Michael Paquier [Tue, 31 Jan 2023 06:24:05 +0000 (15:24 +0900)]
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/
[email protected]
Michael Paquier [Tue, 31 Jan 2023 03:46:56 +0000 (12:46 +0900)]
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[email protected]
Backpatch-through: 11
Thomas Munro [Tue, 31 Jan 2023 00:07:44 +0000 (13:07 +1300)]
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
Tom Lane [Mon, 30 Jan 2023 18:50:25 +0000 (13:50 -0500)]
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
Tom Lane [Mon, 30 Jan 2023 18:44:36 +0000 (13:44 -0500)]
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
Tom Lane [Mon, 30 Jan 2023 18:16:20 +0000 (13:16 -0500)]
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
Tom Lane [Mon, 30 Jan 2023 16:59:37 +0000 (11:59 -0500)]
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
Dean Rasheed [Mon, 30 Jan 2023 10:04:57 +0000 (10:04 +0000)]
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
Amit Kapila [Mon, 30 Jan 2023 02:32:08 +0000 (08:02 +0530)]
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
Thomas Munro [Sat, 28 Jan 2023 01:57:31 +0000 (14:57 +1300)]
Remove unneeded volatile qualifiers from postc.
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 <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/CA%2BhUKGLMoeZNZY6gYdLUQmuoW_a8bKyLvtuZkd_zHcGVOfDzBA%40mail.gmail.com
Tom Lane [Fri, 27 Jan 2023 17:13:41 +0000 (12:13 -0500)]
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
Andrew Dunstan [Fri, 27 Jan 2023 14:38:59 +0000 (09:38 -0500)]
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.
Peter Eisentraut [Fri, 27 Jan 2023 09:49:17 +0000 (10:49 +0100)]
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 <[email protected]>
Discussion: https://www.postgresql.org/message-id/
a6a6de12-f705-2b33-2fd9-
9743277deb08@enterprisedb.com
Peter Eisentraut [Fri, 27 Jan 2023 07:42:08 +0000 (08:42 +0100)]
doc: Adjust a few more references to "postmaster"
Reported-by: Karl O. Pinc <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/
ece84b69-8f94-8b88-925f-
64207cb3a2f0@enterprisedb.com
David Rowley [Fri, 27 Jan 2023 03:08:41 +0000 (16:08 +1300)]
Teach planner about more monotonic window functions
9d9c02ccd introduced runConditions for window functions to allow
monotonic window function evaluation to be made more efficient when the
window function value went beyond some value that it would never go back
from due to its monotonic nature. That commit added prosupport functions
to inform the planner that row_number(), rank(), dense_rank() and some
forms of count(*) were monotonic. Here we add support for ntile(),
cume_dist() and percent_rank().
Reviewed-by: Melanie Plageman
Discussion: https://postgr.es/m/CAApHDvqR+VqB8s+xR-24bzJbU8xyFrBszJ17qKgECf7cWxLCaA@mail.gmail.com
Michael Paquier [Fri, 27 Jan 2023 01:19:50 +0000 (10:19 +0900)]
Fix behavior with pg_restore -l and compressed dumps
pg_restore -l has always been able to read the TOC data of a dump even
if its binary has no support for compression, for both compressed and
uncompressed dumps.
5e73a60 has introduced a backward-incompatible
behavior by switching a warning to a hard error in the code path reading
the header data of a dump, preventing the TOC items to be listed even if
pg_restore -l, with no support for compression, is used on a compressed
dump. Most modern systems should have support for zlib, but it can be
also possible that somebody relies on the past behavior when copying
over a dump where binaries are not built with zlib support (most likely
some WIN32 flavors these days, though most environments should provide
that).
There is no easy way to have a regression test for this pattern, as it
requires a mix of dump/restore commands with different compilation
options, with and without compression. One possibility I see here would
be to have a command-line option that enforces a non-compression check
for a build that supports compression, but that does not seem worth the
cost, either.
Reported-by: Justin Pryzby
Author: Georgios Kokolatos
Discussion: https://postgr.es/m/
20230125180020[email protected]
Tom Lane [Thu, 26 Jan 2023 22:09:12 +0000 (17:09 -0500)]
Improve TimestampDifferenceMilliseconds to cope with overflow sanely.
We'd like to use TimestampDifferenceMilliseconds with the stop_time
possibly being TIMESTAMP_INFINITY, but up to now it's disclaimed
responsibility for overflow cases. Define it to clamp its output to
the range [0, INT_MAX], handling overflow correctly. (INT_MAX rather
than LONG_MAX seems appropriate, because the function is already
described as being intended for calculating wait times for WaitLatch
et al, and that infrastructure only handles waits up to INT_MAX.
Also, this choice gets rid of cross-platform behavioral differences.)
Having done that, we can replace some ad-hoc code in walreceiver.c
with a simple call to TimestampDifferenceMilliseconds.
While at it, fix some buglets in existing callers of
TimestampDifferenceMilliseconds: basebackup_copy.c had not read the
memo about TimestampDifferenceMilliseconds never returning a negative
value, and postmaster.c had not read the memo about Min() and Max()
being macros with multiple-evaluation hazards. Neither of these
quite seem worth back-patching.
Patch by me; thanks to Nathan Bossart for review.
Discussion: https://postgr.es/m/
3126727.
1674759248@sss.pgh.pa.us
Tom Lane [Thu, 26 Jan 2023 17:51:00 +0000 (12:51 -0500)]
Code review for commit
05a7be935.
Avoid having walreceiver code know explicitly about the precision
and underlying datatype of TimestampTz. (There is still one
calculation that knows that, which should be replaced with use of
TimestampDifferenceMilliseconds; but we need to figure out what to do
about overflow cases first.)
In support of this, provide a TimestampTzPlusSeconds macro, as well
as TIMESTAMP_INFINITY and TIMESTAMP_MINUS_INFINITY macros. (We could
have used the existing DT_NOEND and DT_NOBEGIN symbols, but I judged
those too opaque and confusing.)
Move GetCurrentTimestamp calls so that it's more obvious that we
are not using stale values of "now" anyplace. This doesn't result
in net more calls, and might indeed make for net fewer.
Avoid having a dummy value in the WalRcvWakeupReason enum, so that
we can hope for the compiler to catch overlooked switch cases.
Nathan Bossart and Tom Lane
Discussion: https://postgr.es/m/
20230125235004.GA1327755@nathanxps13
Tom Lane [Thu, 26 Jan 2023 16:34:17 +0000 (11:34 -0500)]
Doc: use less-awkward phrasing.
Improve wording in note about tools required to build
from the source repository.
Laurenz Albe, per gripe from Riivo Kolka
Discussion: https://postgr.es/m/
167463493588.
2667301.
13267758265445155872@wrigleys.postgresql.org
Robert Haas [Thu, 26 Jan 2023 13:14:41 +0000 (08:14 -0500)]
DROP ROLE regress_role_limited_admin at end of test
This is required by project policy, and I overlooked the need for
it (again) by accident.
Reported by Álvaro Herrara.
Discussion: http://postgr.es/m/
20230126114659[email protected]
Peter Eisentraut [Thu, 26 Jan 2023 10:33:01 +0000 (11:33 +0100)]
Don't install postmaster symlink anymore
This has long been deprecated. Some of the build systems didn't even
install it.
Also remove man page.
Reviewed-by: Karl O. Pinc <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/
ece84b69-8f94-8b88-925f-
64207cb3a2f0@enterprisedb.com
Peter Eisentraut [Thu, 26 Jan 2023 09:48:32 +0000 (10:48 +0100)]
Remove gratuitous references to postmaster program
"postgres" has long been officially preferred over "postmaster" as the
name of the program to invoke to run the server. Some example scripts
and code comments still used the latter. Change those.
Discussion: https://www.postgresql.org/message-id/flat/
ece84b69-8f94-8b88-925f-
64207cb3a2f0@enterprisedb.com
Peter Geoghegan [Thu, 26 Jan 2023 06:22:27 +0000 (22:22 -0800)]
Revert "Add eager and lazy freezing strategies to VACUUM."
This reverts commit
4d417992613949af35530b4e8e83670c4e67e1b2. Broad
concerns about regressions caused by eager freezing strategy have been
raised. Whether or not these concerns can be worked through in any time
frame is far from certain.
Discussion: https://postgr.es/m/
20230126004347[email protected]
Jeff Davis [Thu, 26 Jan 2023 04:23:32 +0000 (20:23 -0800)]
Clarify documentation for CLUSTER on partitioned tables.
Author: Nathan Bossart
Discussion: https://postgr.es/m/
20230114224000.GA2505377@nathanxps13
Michael Paquier [Thu, 26 Jan 2023 03:23:16 +0000 (12:23 +0900)]
Make auto_explain print the query identifier in verbose mode
When auto_explain.log_verbose is on, auto_explain should print in the
logs plans equivalent to the EXPLAIN (VERBOSE). However, when
compute_query_id is on, query identifiers were not showing up, being
only handled by EXPLAIN (VERBOSE). This brings auto_explain on par with
EXPLAIN regarding that. Note that like EXPLAIN, auto_explain does not
show the query identifier when compute_query_id=regress.
The change is done so as the choice of printing the query identifier is
done in ExplainPrintPlan() rather than in ExplainOnePlan(), to avoid a
duplication of the logic dealing with the query ID. auto_explain is the
only in-core caller of ExplainPrintPlan().
While looking at the area, I have noticed that more consolidation
between EXPLAIN and auto_explain would be in order for the logging of
the plan duration and the buffer usage. This refactoring is left as a
future change.
Author: Atsushi Torikoshi
Reviewed-by: Justin Pryzby, Julien Rouhaud
Discussion: https://postgr.es/m/
1ea21936981f161bccfce05765c03bee@oss.nttdata.com
Thomas Munro [Thu, 26 Jan 2023 01:50:07 +0000 (14:50 +1300)]
Fix rare sharedtuplestore.c corruption.
If the final chunk of an oversized tuple being written out to disk was
exactly 32760 bytes, it would be corrupted due to a fencepost bug.
Bug #17619. Back-patch to 11 where the code arrived.
While testing that (see test module in archives), I (tmunro) noticed
that the per-participant page counter was not initialized to zero as it
should have been; that wasn't a live bug when it was written since DSM
memory was originally always zeroed, but since 14
min_dynamic_shared_memory might be configured and it supplies non-zeroed
memory, so that is also fixed here.
Author: Dmitry Astapov <
[email protected]>
Discussion: https://postgr.es/m/17619-
0de62ceda812b8b5%40postgresql.org
Michael Paquier [Thu, 26 Jan 2023 00:13:39 +0000 (09:13 +0900)]
Revert "Rename contrib module basic_archive to basic_wal_module"
This reverts commit
0ad3c60, as per feedback from Tom Lane, Robert Haas
and Andres Freund. The new name used for the module had little
support.
This moves back to basic_archive as module name, and we will likely use
that as template for recovery modules, as well.
Discussion: https://postgr.es/m/CA+TgmoYG5uGOp7DGFT5gzC1kKFWGjkLSj_wOQxGhfMcvVEiKGA@mail.gmail.com
Peter Geoghegan [Wed, 25 Jan 2023 22:31:41 +0000 (14:31 -0800)]
Doc: update VACUUM VERBOSE freezing tip.
VACUUM VERBOSE/autovacuuming logging have reported on the number of
pages frozen by VACUUM since commit
d977ffd9 added that capability.
This information is directly related to relfrozenxid advancement, so
update an older tip from the documentation about how relfrozenxid is
reported on by the same instrumentation code. Now the tip directly
mentions newly frozen pages, too.
Peter Geoghegan [Wed, 25 Jan 2023 22:15:38 +0000 (14:15 -0800)]
Add eager and lazy freezing strategies to VACUUM.
Eager freezing strategy avoids large build-ups of all-visible pages. It
makes VACUUM trigger page-level freezing whenever doing so will enable
the page to become all-frozen in the visibility map. This is useful for
tables that experience continual growth, particularly strict append-only
tables such as pgbench's history table. Eager freezing significantly
improves performance stability by spreading out the cost of freezing
over time, rather than doing most freezing during aggressive VACUUMs.
It complements the insert autovacuum mechanism added by commit
b07642db.
VACUUM determines its freezing strategy based on the value of the new
vacuum_freeze_strategy_threshold GUC (or reloption) with logged tables.
Tables that exceed the size threshold use the eager freezing strategy.
Unlogged tables and temp tables always use eager freezing strategy,
since the added cost is negligible there. Non-permanent relations won't
incur any extra overhead in WAL written (for the obvious reason), nor in
pages dirtied (since any extra freezing will only take place on pages
whose PD_ALL_VISIBLE bit needed to be set either way).
VACUUM uses lazy freezing strategy for logged tables that fall under the
GUC size threshold. Page-level freezing triggers based on the criteria
established in commit
1de58df4, which added basic page-level freezing.
Eager freezing is strictly more aggressive than lazy freezing. Settings
like vacuum_freeze_min_age still get applied in just the same way in
every VACUUM, independent of the strategy in use. The only mechanical
difference between eager and lazy freezing strategies is that only the
former applies its own additional criteria to trigger freezing pages.
Note that even lazy freezing strategy will trigger freezing whenever a
page happens to have required that an FPI be written during pruning,
provided that the page will thereby become all-frozen in the visibility
map afterwards (due to the FPI optimization from commit
1de58df4).
The vacuum_freeze_strategy_threshold default setting is 4GB. This is a
relatively low setting that prioritizes performance stability. It will
be reviewed at the end of the Postgres 16 beta period.
Author: Peter Geoghegan <
[email protected]>
Reviewed-By: Jeff Davis <[email protected]>
Reviewed-By: Andres Freund <[email protected]>
Reviewed-By: Matthias van de Meent <[email protected]>
Discussion: https://postgr.es/m/CAH2-WzkFok_6EAHuK39GaW4FjEFQsY=3J0AAd6FXk93u-Xq3Fg@mail.gmail.com
Andres Freund [Wed, 25 Jan 2023 17:59:26 +0000 (09:59 -0800)]
plpython: Stop undefining _POSIX_C_SOURCE, _XOPEN_SOURCE
We undefined them to avoid warnings about macro redefinitions. But we haven't
fully followed the necessary include order, since at least
147c2482542, in
2011. Recently the combination of the include order rules not being followed
and undefining _POSIX_C_SOURCE started to cause a compile failure, starting
with
03023a2664f. Undefining _POSIX_C_SOURCE hides clock_gettime(), which is
referenced in an inline function as of
03023a2664f, whereas it was a macro
before.
After seeing some evidence that undefining _POSIX_C_SOURCE et al isn't
required, I tried to build postgres with plpython on most of our supported
platforms (except DragonFlyBSD and Illumos, but similar systems were tested),
with/without the #undefines. No compiler warning / behavioral difference.
The oldest supported python version, 3.2, defines _POSIX_C_SOURCE to 200112L
ad _XOPEN_SOURCE to 600, whereas newer versions of python use 200809L/700
respectively. As _POSIX_C_SOURCE/_XOPEN_SOURCE will default to the newer
operating system on most platforms, it's possible that when using python 3.2
new warnings would be emitted - but that seems acceptable.
It's possible that this approach won't work on some older platforms. But
getting rid of most of the include-order complexity seems promising, and it's
an easily revertible patch if we end up having to go another way.
Discussion: https://postgr.es/m/
20230124165814[email protected]
Andres Freund [Wed, 25 Jan 2023 17:59:26 +0000 (09:59 -0800)]
plpython: Avoid the need to redefine *printf macros
Until now we undefined and then redefined a lot of *printf macros due to
worries about conflicts with Python.h macro definitions. Current Python.h
doesn't define any *printf macros, and older versions just defined snprintf,
vsnprintf, guarded by #if defined(MS_WIN32) && !defined(HAVE_SNPRINTF).
Thus we can replace the undefine/define section with a single
#define HAVE_SNPRINTF 1
Reviewed-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/
20230124165814[email protected]
Tom Lane [Wed, 25 Jan 2023 16:48:38 +0000 (11:48 -0500)]
Avoid type cheats for invalid dsa_handles and dshash_table_handles.
Invent separate macros for "invalid" values of these types, so that
we needn't embed knowledge of their representations into calling code.
These are all zeroes anyway ATM, so this is not fixing any live bug,
but it makes the code cleaner and more future-proof.
I (tgl) also chose to move DSM_HANDLE_INVALID into dsm_impl.h,
since it seems like it should live beside the typedef for dsm_handle.
Hou Zhijie, Nathan Bossart, Kyotaro Horiguchi, Tom Lane
Discussion: https://postgr.es/m/OS0PR01MB5716860B1454C34E5B179B6694C99@OS0PR01MB5716.jpnprd01.prod.outlook.com
Michael Paquier [Wed, 25 Jan 2023 11:00:36 +0000 (20:00 +0900)]
doc: Fix network_ops -> inet_ops in SpGiST operator class list
network_ops is an opclass family of SpGiST, and the opclass able to
work on the inet type is named inet_ops.
Oversight in
7a1cd52, that reworked the design of the table listing all
the operators available.
Reported-by: Laurence Parry
Reviewed-by: Tom Lane, David G. Johnston
Discussion: https://postgr.es/m/
167458110639.
2667300.
14741268666497110766@wrigleys.postgresql.org
Backpatch-through: 14
Michael Paquier [Wed, 25 Jan 2023 05:36:51 +0000 (14:36 +0900)]
Rename contrib module basic_archive to basic_wal_module
This rename is in preparation for the introduction of recovery modules,
where basic_wal_module will be used as a base template for the set of
callbacks introduced. The former name did not really reflect all that.
Author: Nathan Bossart
Discussion: https://postgr.es/m/
20221227192449.GA3672473@nathanxps13