postgresql.git
3 years agoServer-side gzip compression.
Robert Haas [Mon, 24 Jan 2022 20:13:18 +0000 (15:13 -0500)]
Server-side gzip compression.

pg_basebackup's --compression option now lets you write either
"client-gzip" or "server-gzip" instead of just "gzip" to specify
where the compression should be performed. If you write simply
"gzip" it's taken to mean "client-gzip" unless you also use
--target, in which case it is interpreted to mean "server-gzip",
because that's the only thing that makes any sense in that case.

To make this work, the BASE_BACKUP command now takes new
COMPRESSION and COMPRESSION_LEVEL options.

At present, pg_basebackup cannot decompress .gz files, so
server-side compression will cause a failure if (1) -Ft is not
used or (2) -R is used or (3) -D- is used without --no-manifest.

Along the way, I removed the information message added by commit
5c649fe153367cdab278738ee4aebbfd158e0546 which occurred if you
specified no compression level and told you that the default level
had been used instead. That seemed like more output than most
people would want.

Also along the way, this adds a check to the server for
unrecognized base backup options. This repairs a bug introduced
by commit 0ba281cb4bf9f5f65529dfa4c8282abb734dd454.

This commit also adds some new test cases for pg_verifybackup.
They take a server-side backup with and without compression, and
then extract the backup if we have the OS facilities available
to do so, and then run pg_verifybackup on the extracted
directory. That is a good test of the functionality added by
this commit and also improves test coverage for the backup target
patch (commit 3500ccc39b0dadd1068a03938e4b8ff562587ccc) and for
pg_verifybackup itself.

Patch by me, with a bug fix by Jeevan Ladhe.  The patch set of which
this is a part has also had review and/or testing from Tushar Ahuja,
Suraj Kharage, Dipesh Pandit, and Mark Dilger.

Discussion: http://postgr.es/m/CA+Tgmoa-ST7fMLsVJduOB7Eub=2WjfpHS+QxHVEpUoinf4bOSg@mail.gmail.com

3 years agopg_upgrade: Preserve database OIDs.
Robert Haas [Mon, 24 Jan 2022 19:23:15 +0000 (14:23 -0500)]
pg_upgrade: Preserve database OIDs.

Commit 9a974cbcba005256a19991203583a94b4f9a21a9 arranged to preserve
relfilenodes and tablespace OIDs. For similar reasons, also arrange
to preserve database OIDs.

One problem is that, up until now, the OIDs assigned to the template0
and postgres databases have not been fixed. This could be a problem
when upgrading, because pg_upgrade might try to migrate a database
from the old cluster to the new cluster while keeping the OID and find
a different database with that OID, resulting in a failure. If it finds
a database with the same name and the same OID that's OK: it will be
dropped and recreated. But the same OID and a different name is a
problem.

To prevent that, fix the OIDs for postgres and template0 to specific
values less than 16384. To avoid running afoul of this rule, these
values should not be changed in future releases. It's not a problem
that these OIDs aren't fixed in existing releases, because the OIDs
that we're assigning here weren't used for either of these databases
in any previous release. Thus, there's no chance that an upgrade of
a cluster from any previous release will collide with the OIDs we're
assigning here. And going forward, the OIDs will always be fixed, so
the only potential collision is with a system database having the
same name and the same OID, which is OK.

This patch lets users assign a specific OID to a database as well,
provided however that it can't be less than 16384. I (rhaas) thought
it might be better not to expose this capability to users, but the
consensus was otherwise, so the syntax is documented. Letting users
assign OIDs below 16384 would not be OK, though, because a
user-created database with a low-numbered OID might collide with a
system-created database in a future release. We therefore prohibit
that.

Shruthi KC, based on an earlier patch from Antonin Houska, reviewed
and with some adjustments by me.

Discussion: http://postgr.es/m/CA+TgmoYgTwYcUmB=e8+hRHOFA0kkS6Kde85+UNdon6q7bt1niQ@mail.gmail.com
Discussion: http://postgr.es/m/CAASxf_Mnwm1Dh2vd5FAhVX6S1nwNSZUB1z12VddYtM++H2+p7w@mail.gmail.com

3 years agoUnbreak pg_basebackup/t/010_pg_basebackup.pl on msys
Andrew Dunstan [Mon, 24 Jan 2022 19:03:46 +0000 (14:03 -0500)]
Unbreak pg_basebackup/t/010_pg_basebackup.pl on msys

Once again we ran foul of the rather baroque msys2 path translation
rules. The cure as in many cases is to do the translation ourselves.

Discussion: https://postgr.es/m/CA+TgmoZU+1yj8TZ8PZrPHxPmr6Wz84V2RfZnsd5HnZugYtqZng@mail.gmail.com

3 years agoRemember to reset yy_start state when firing up repl_scanner.l.
Tom Lane [Mon, 24 Jan 2022 17:09:46 +0000 (12:09 -0500)]
Remember to reset yy_start state when firing up repl_scanner.l.

Without this, we get odd behavior when the previous cycle of
lexing exited in a non-default exclusive state.  Every other
copy of this code is aware that it has to do BEGIN(INITIAL),
but repl_scanner.l did not get that memo.

The real-world impact of this is probably limited, since most
replication clients would abandon their connection after getting
a syntax error.  Still, it's a bug.

This mistake is old, so back-patch to all supported branches.

Discussion: https://postgr.es/m/1874781.1643035952@sss.pgh.pa.us

3 years agoTemporarily add some information about python include paths to configure.
Andres Freund [Mon, 24 Jan 2022 07:30:40 +0000 (23:30 -0800)]
Temporarily add some information about python include paths to configure.

We're still (see e0e567a1067e0e567a1067) working on replacing use of the
deprecated distutils. This commit just makes configure print out the results
of different ways of determining the include path. Hopefully this will help us
to find a way to transition away from distutils without turning the buildfarm
red for prolonged amounts of time.

Discussion: https://postgr.es/m/20220124025301.qu36x44w6m67cnap@alap3.anarazel.de

3 years agopg_basebackup: Skip a few more fsyncs if --no-sync is specified.
Andres Freund [Sun, 23 Jan 2022 21:59:23 +0000 (13:59 -0800)]
pg_basebackup: Skip a few more fsyncs if --no-sync is specified.

This is mostly interesting for running the regression tests on machines with
slow / overloaded IO.

Discussion: https://postgr.es/m/20220119041646.rhuo3youiqxqjmo2@alap3.anarazel.de

3 years agopg_dump: avoid useless query in binary_upgrade_set_type_oids_by_type_oid
Tom Lane [Sun, 23 Jan 2022 18:54:24 +0000 (13:54 -0500)]
pg_dump: avoid useless query in binary_upgrade_set_type_oids_by_type_oid

Commit 6df7a9698 wrote appendPQExpBuffer where it should have
written printfPQExpBuffer.  This resulted in re-issuing the
previous query along with the desired one, which very accidentally
had no negative consequences except for some wasted cycles.

Back-patch to v14 where that came in.

Discussion: https://postgr.es/m/1714711.1642962663@sss.pgh.pa.us

3 years agoClean up recent Coverity complaints.
Tom Lane [Sun, 23 Jan 2022 17:51:38 +0000 (12:51 -0500)]
Clean up recent Coverity complaints.

Commit 5c649fe15 introduced a memory leak into pg_basebackup's
parse_compress_options.  (I simplified nearby code while at it.)

Commit 9a974cbcb introduced a memory leak into pg_dump's
binary_upgrade_set_pg_class_oids.

Coverity also complained about a call of SnapBuildProcessChange that
ignored the result, unlike every other call of that function.  This
is evidently intentional, so add a (void) cast to indicate that.
(It's also old, dating to b89e15105; I suppose the reason it showed
up now is 7a5f6b474's recent rearrangement of nearby code.)

3 years agoSuppress variable-set-but-not-used warning from clang 13.
Tom Lane [Sun, 23 Jan 2022 16:09:00 +0000 (11:09 -0500)]
Suppress variable-set-but-not-used warning from clang 13.

In the normal configuration where GEQO_DEBUG isn't defined,
recent clang versions have started to complain that geqo_main.c
accumulates the edge_failures count but never does anything
with it.  As a minimal back-patchable fix, insert a void cast
to silence this warning.  (I'd speculated about ripping out the
GEQO_DEBUG logic altogether, but I don't think we'd wish to
back-patch that.)

Per recently-established project policy, this is a candidate
for back-patching into out-of-support branches: it suppresses
an annoying compiler warning but changes no behavior.  Hence,
back-patch all the way to 9.2.

Discussion: https://postgr.es/m/CA+hUKGLTSZQwES8VNPmWO9AO0wSeLt36OCPDAZTccT1h7Q7kTQ@mail.gmail.com

3 years agoCorrect type of front_pathkey to PathKey
Tomas Vondra [Sun, 23 Jan 2022 02:36:55 +0000 (03:36 +0100)]
Correct type of front_pathkey to PathKey

In sort_inner_and_outer we iterate a list of PathKey elements, but the
variable is declared as (List *). This mistake is benign, because we
only pass the pointer to lcons() and never dereference it.

This exists since ~2004, but it's confusing. So fix and backpatch to all
supported branches.

Backpatch-through: 10
Discussion: https://postgr.es/m/bf3a6ea1-a7d8-7211-0669-189d5c169374%40enterprisedb.com

3 years agoCheck syscache result in AlterStatistics
Tomas Vondra [Sun, 23 Jan 2022 01:49:41 +0000 (02:49 +0100)]
Check syscache result in AlterStatistics

The syscache lookup may return NULL even for valid OID, for example due
to a concurrent DROP STATISTICS, so a HeapTupleIsValid is necessary.
Without it, it may fail with a segfault.

Reported by Alexander Lakhin, patch by me. Backpatch to 13, where ALTER
STATISTICS ... SET STATISTICS was introduced.

Backpatch-through: 13
Discussion: https://postgr.es/m/17372-bf3b6e947e35ae77%40postgresql.org

3 years agoRemove useless inline marker.
Tom Lane [Sat, 22 Jan 2022 22:11:33 +0000 (17:11 -0500)]
Remove useless inline marker.

Putting "inline" on a function that's not used anywhere in its
own file is useless unless the linker is doing global optimization,
a method we don't generally enable.  Moreover, it draws warnings
from some buildfarm members (curculio at least).

Looks like this was sloppiness in cc8b25712, which moved the
function from somewhere else where the inline marker was
more appropriate.

3 years agoDoc: add cross-references between array_to_string and string_to_array.
Tom Lane [Sat, 22 Jan 2022 20:44:51 +0000 (15:44 -0500)]
Doc: add cross-references between array_to_string and string_to_array.

These functions aren't exact inverses, but they're closely related;
yet we document them in two different sections.  Add cross-ref
<link>s to improve that situation.

While here, move the strpos and substr entries to re-alphabetize
Table 9.10.  Also, drop an ancient compatibility note about
string_to_array, which wasn't even on the right page, so probably
few people ever saw it.

Discussion: https://postgr.es/m/164287017550.704.5840412183184961677@wrigleys.postgresql.org

3 years agoFlush table's relcache during ALTER TABLE ADD PRIMARY KEY USING INDEX.
Tom Lane [Sat, 22 Jan 2022 18:32:40 +0000 (13:32 -0500)]
Flush table's relcache during ALTER TABLE ADD PRIMARY KEY USING INDEX.

Previously, unless we had to add a NOT NULL constraint to the column,
this command resulted in updating only the index's relcache entry.
That's problematic when replication behavior is being driven off the
existence of a primary key: other sessions (and ours too for that
matter) failed to recalculate their opinion of whether the table can
be replicated.  Add a relcache invalidation to fix it.

This has been broken since pg_class.relhaspkey was removed in v11.
Before that, updating the table's relhaspkey value sufficed to cause
a cache flush.  Hence, backpatch to v11.

Report and patch by Hou Zhijie

Discussion: https://postgr.es/m/OS0PR01MB5716EBE01F112C62F8F9B786947B9@OS0PR01MB5716.jpnprd01.prod.outlook.com

3 years agoFix race condition in gettext() initialization in libpq and ecpglib.
Tom Lane [Fri, 21 Jan 2022 20:36:12 +0000 (15:36 -0500)]
Fix race condition in gettext() initialization in libpq and ecpglib.

In libpq and ecpglib, multiple threads can concurrently enter the
initialization logic for message localization.  Since we set the
its-done flag before actually doing the work, it'd be possible
for some threads to reach gettext() before anyone has called
bindtextdomain().  Barring bugs in libintl itself, this would not
result in anything worse than failure to localize some early
messages.  Nonetheless, it's a bug, and an easy one to fix.

Noted while investigating bug #17299 from Clemens Zeidler
(much thanks to Liam Bowen for followup investigation on that).
It currently appears that that actually *is* a bug in libintl itself,
but that doesn't let us off the hook for this bit.

Back-patch to all supported versions.

Discussion: https://postgr.es/m/17299-7270741958c0b1ab@postgresql.org
Discussion: https://postgr.es/m/CAE7q7Eit4Eq2=bxce=Fm8HAStECjaXUE=WBQc-sDDcgJQ7s7eg@mail.gmail.com

3 years agofsync pg_logical/mappings in CheckPointLogicalRewriteHeap().
Andres Freund [Fri, 21 Jan 2022 19:22:55 +0000 (11:22 -0800)]
fsync pg_logical/mappings in CheckPointLogicalRewriteHeap().

While individual logical rewrite files were synced to disk, the directory was
not. On some filesystems that could lead to loosing directory entries after a
crash.

Reported-By: Tom Lane <tgl@sss.pgh.pa.us>
Author: Nathan Bossart <bossartn@amazon.com>
Discussion: https://postgr.es/m/867F2E29-2782-4869-970E-B984C6D35A8F@amazon.com
Backpatch: 10-

3 years agopostgres_fdw: Fix subabort cleanup of connections used in asynchronous execution.
Etsuro Fujita [Fri, 21 Jan 2022 08:45:00 +0000 (17:45 +0900)]
postgres_fdw: Fix subabort cleanup of connections used in asynchronous execution.

Commit 27e1f1456 resets the per-connection states of connections used to
scan foreign tables asynchronously during abort cleanup at main
transaction end, but it failed to do so during subabort cleanup at
subtransaction end, leading to a segmentation fault when re-executing an
asynchronous-foreign-table-scan query in a transaction that was
cancelled in a subtransaction of it.

Fix by modifying pgfdw_abort_cleanup() to reset the per-connection state
of a given connection also when called for subabort cleanup.  Also,
modify that function to do the reset in both the abort-cleanup and
subabort-cleanup cases if necessary, to save cycles, and improve a
comment on it a little bit.

Back-patch to v14 where the aforesaid commit came in.

Reviewed by Alexander Pyhalov

Discussion: https://postgr.es/m/CAPmGK14cCV-JA7kNsyt2EUTKvZ4xkr2LNRthi1U1C3cqfGppAw@mail.gmail.com

3 years agoFix one-off bug causing missing commit timestamps for subtransactions
Michael Paquier [Fri, 21 Jan 2022 05:54:04 +0000 (14:54 +0900)]
Fix one-off bug causing missing commit timestamps for subtransactions

The logic in charge of writing commit timestamps (enabled with
track_commit_timestamp) for subtransactions had a one-bug bug,
where it would be possible that commit timestamps go missing for the
last subtransaction committed.

While on it, simplify a bit the iteration logic in the loop writing the
commit timestamps, as per suggestions from Kyotaro Horiguchi and Tom
Lane, so as some variable initializations are not part of the loop
itself.

Issue introduced in 73c986a.

Analyzed-by: Alex Kingsborough
Author: Alex Kingsborough, Kyotaro Horiguchi
Discussion: https://postgr.es/m/73A66172-4050-4F2A-B7F1-13508EDA2144@amazon.com
Backpatch-through: 10

3 years agoAdd new simple TAP test for tablespaces, attempt II.
Thomas Munro [Fri, 21 Jan 2022 02:12:32 +0000 (15:12 +1300)]
Add new simple TAP test for tablespaces, attempt II.

See commit message for d1511fe1b040853f6e10d353e56b42bb96ae239d.  This
new version attempts to fix path translation problem on MSYS/Windows.

Discussion: https://postgr.es/m/20220117055326.GD756210%40rfd.leadboat.com

3 years agoExtend the options of pg_basebackup to control compression
Michael Paquier [Fri, 21 Jan 2022 02:08:43 +0000 (11:08 +0900)]
Extend the options of pg_basebackup to control compression

The option --compress is extended to accept a compression method and an
optional compression level, as of the grammar METHOD[:LEVEL].  The
methods currently support are "none" and "gzip", for client-side
compression.  Any of those methods use only an integer value for the
compression level, but any method implemented in the future could use
more specific keywords if necessary.

This commit keeps the logic backward-compatible.  Hence, the following
compatibility rules apply for the new format of the option --compress:
* -z/--gzip is a synonym of --compress=gzip.
* --compress=NUM implies:
** --compress=none if NUM = 0.
** --compress=gzip:NUM if NUM > 0.

Note that there are also plans to extend more this grammar with
server-side compression.

Reviewed-by: Robert Haas, Magnus Hagander, Álvaro Herrera, David
G. Johnston, Georgios Kokolatos
Discussion: https://postgr.es/m/Yb3GEgWwcu4wZDuA@paquier.xyz

3 years agoRevert "Make configure prefer python3 to plain python."
Tom Lane [Thu, 20 Jan 2022 22:32:21 +0000 (17:32 -0500)]
Revert "Make configure prefer python3 to plain python."

This reverts commit f201da39edcd6ac1ab9a3edf3e20e2a73bbbe69e.
The buildfarm is not ready for python3, evidently, so we'll
give the owners some more time to get set up.

Discussion: https://postgr.es/m/2872c9a0-4b0a-1354-d5f6-94d6f85ba354@enterprisedb.com

3 years agoTighten TAP tests' tracking of postmaster state some more.
Tom Lane [Thu, 20 Jan 2022 22:28:07 +0000 (17:28 -0500)]
Tighten TAP tests' tracking of postmaster state some more.

Commits 6c4a8903b et al. had a couple of deficiencies:

* The logic I added to Cluster::start to see if a PID file is present
could be fooled by a stale PID file left over from a previous
postmaster.  To fix, if we're not sure whether we expect to find a
running postmaster or not, validate the PID using "kill 0".

* 017_shm.pl has a loop in which it just issues repeated Cluster::start
calls; this will fail if some invocation fails but leaves self->_pid
set.  Per buildfarm results, the above fix is not enough to make this
safe: we might have "validated" a PID for a postmaster that exits
immediately after we look.  Hence, match each failed start call with
a stop call that will get us back to the self->_pid == undef state.
Add a fail_ok option to Cluster::stop to make this work.

Discussion: https://postgr.es/m/CA+hUKGKV6fOHvfiPt8=dOKzvswjAyLoFoJF1iQXMNpi7+hD1JQ@mail.gmail.com

3 years agoSupport base backup targets.
Robert Haas [Tue, 16 Nov 2021 20:20:50 +0000 (15:20 -0500)]
Support base backup targets.

pg_basebackup now has a --target=TARGET[:DETAIL] option. If specfied,
it is sent to the server as the value of the TARGET option to the
BASE_BACKUP command. If DETAIL is included, it is sent as the value of
the new TARGET_DETAIL option to the BASE_BACKUP command.  If the
target is anything other than 'client', pg_basebackup assumes that it
will now be the server's job to write the backup in a location somehow
defined by the target, and that it therefore needs to write nothing
locally. However, the server will still send messages to the client
for progress reporting purposes.

On the server side, we now support two additional types of backup
targets.  There is a 'blackhole' target, which just throws away the
backup data without doing anything at all with it. Naturally, this
should only be used for testing and debugging purposes, since you will
not actually have a backup when it finishes running. More usefully,
there is also a 'server' target, so you can now use something like
'pg_basebackup -Xnone -t server:/SOME/PATH' to write a backup to some
location on the server. We can extend this to more types of targets
in the future, and might even want to create an extensibility
mechanism for adding new target types.

Since WAL fetching is handled with separate client-side logic, it's
not part of this mechanism; thus, backups with non-default targets
must use -Xnone or -Xfetch.

Patch by me, with a bug fix by Jeevan Ladhe.  The patch set of which
this is a part has also had review and/or testing from Tushar Ahuja,
Suraj Kharage, Dipesh Pandit, and Mark Dilger.

Discussion: http://postgr.es/m/CA+TgmoaYZbz0=Yk797aOJwkGJC-LK3iXn+wzzMx7KdwNpZhS5g@mail.gmail.com

3 years agoAllow clean.bat to be run from anywhere
Andrew Dunstan [Thu, 20 Jan 2022 15:13:18 +0000 (10:13 -0500)]
Allow clean.bat to be run from anywhere

This was omitted from c3879a7b4c which modified the other msvc .bat
files.

Per request from Juan José Santamaría Flecha

Discussion: https://postgr.es/m/CAC+AXB0_fxYGbQoaYjCA8um7TTbOVP4L9aXnVmHwK8WzaT4gdA@mail.gmail.com

Backpatch to all live branches.

3 years agoRemove 'datlastsysoid'.
Robert Haas [Thu, 20 Jan 2022 13:56:54 +0000 (08:56 -0500)]
Remove 'datlastsysoid'.

It hasn't been used for anything for a long time. Up until recently,
we still queried it when dumping very old servers, but since
commit 30e7c175b81d53c0f60f6ad12d1913a6d7d77008, there's no longer any
code at all that cares about it.

Discussion: http://postgr.es/m/CA+Tgmoa14=BRq0WEd0eevjEMn9EkghDB1FZEkBw7+UAb7tF49A@mail.gmail.com

3 years agoTry to stabilize reloptions test, again.
Thomas Munro [Thu, 20 Jan 2022 09:11:48 +0000 (22:11 +1300)]
Try to stabilize reloptions test, again.

Since the test requires reproducible behavior from VACUUM, and since
DISABLE_PAGE_SKIPPING doesn't actually disable all forms of page
skipping, let's use a temporary table to avoid contention.

Back-patch to 12, like commit 3414099c.

Discussion: https://postgr.es/m/20220120052404.sonrhq3f3qgplpzj%40alap3.anarazel.de

3 years agoCall pg_newlocale_from_collation() also with default collation
Peter Eisentraut [Thu, 20 Jan 2022 08:38:05 +0000 (09:38 +0100)]
Call pg_newlocale_from_collation() also with default collation

Previously, callers of pg_newlocale_from_collation() did not call it
if the collation was DEFAULT_COLLATION_OID and instead proceeded with
a pg_locale_t of 0.  Instead, now we call it anyway and have it return
0 if the default collation was passed.  It already did this, so we
just have to adjust the callers.  This simplifies all the call sites
and also makes future enhancements easier.

After discussion and testing, the previous comment in pg_locale.c
about avoiding this for performance reasons may have been mistaken
since it was testing a very different patch version way back when.

Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Discussion: https://www.postgresql.org/message-id/ed3baa81-7fac-7788-cc12-41e3f7917e34@enterprisedb.com

3 years agodoc: Mention the level of locks taken on objects in COMMENT
Michael Paquier [Thu, 20 Jan 2022 07:54:47 +0000 (16:54 +0900)]
doc: Mention the level of locks taken on objects in COMMENT

This information was nowhere to be found.  This adds one note on the
page of COMMENT, and one note in the section dedicated to explicit
locking, both telling that a SHARE UPDATE EXCLUSIVE lock is taken on the
object commented.

Author: Nikolai Berkoff
Reviewed-by: Laurenz Albe
Discussion: https://postgr.es/m/_0HDHIGcCdCsUyXn22QwI2FEuNR6Fs71rtgGX6hfyBlUh5rrnE2qMmvIFu9EY4Pijr2gUmJEAXCjuNU2Oxku9TryLp9CdHllpsCfN3gD0-Y=@pm.me
Backpatch-through: 10

3 years agoMake logical decoding a part of the rmgr.
Jeff Davis [Wed, 19 Jan 2022 22:58:04 +0000 (14:58 -0800)]
Make logical decoding a part of the rmgr.

Add a new rmgr method, rm_decode, and use that rather than a switch
statement.

In preparation for rmgr extensibility.

Reviewed-by: Julien Rouhaud
Discussion: https://postgr.es/m/ed1fb2e22d15d3563ae0eb610f7b61bb15999c0a.camel%40j-davis.com
Discussion: https://postgr.es/m/20220118095332.6xtlcjoyxobv6cbk@jrouhaud

3 years agointerval_out() must be marked STABLE, not IMMUTABLE.
Tom Lane [Wed, 19 Jan 2022 22:17:55 +0000 (17:17 -0500)]
interval_out() must be marked STABLE, not IMMUTABLE.

Its results vary depending on the IntervalStyle GUC, so it cannot
be considered immutable.

This is an extremely ancient bug.  AFAICT it was a sloppy mistake
in 6f58115dd, which marked it "cacheable" alongside marking several
other interval functions that way.  At the time, interval_out()
depended on DateStyle not IntervalStyle, but it was still wrong.

Back-patching this change doesn't look very practical, so I won't.
Aside from the usual difficulties of getting catalog changes
applied to existing databases, people might have indexes,
generated columns, etc that depend on interval-to-text casts
being considered immutable.  (This'd not really give them any
problem as long as they never change IntervalStyle.)  They
wouldn't appreciate us breaking such usage in minor releases.

Per bug #17371 from Marcus Gartner.

Discussion: https://postgr.es/m/17371-8f57e6e9ca5e35bf@postgresql.org

3 years agoTAP tests: check for postmaster.pid anyway when "pg_ctl start" fails.
Tom Lane [Wed, 19 Jan 2022 21:29:09 +0000 (16:29 -0500)]
TAP tests: check for postpid anyway when "pg_ctl start" fails.

"pg_ctl start" might start a new postmaster and then return failure
anyway, for example if PGCTLTIMEOUT is exceeded.  If there is a
postmaster there, it's still incumbent on us to shut it down at
script end, so check for the PID file even though we are about
to fail.

This has been broken all along, so back-patch to all supported branches.

Discussion: https://postgr.es/m/647439.1642622744@sss.pgh.pa.us

3 years agoMake configure prefer python3 to plain python.
Tom Lane [Wed, 19 Jan 2022 20:38:58 +0000 (15:38 -0500)]
Make configure prefer python3 to plain python.

This avoids possibly selecting Python 2.x on systems that have
both Python 2 and Python 3.  We used to feel that what "python"
links to is a user choice that we should honor.  However, we're
about to cease support for Python 2, so users will no longer have
any choice of that sort.  This small change is being made ahead
of the big Python-2-ectomy so that we can see how much of the
buildfarm is not yet prepared for that.  Systems with only
Python 2 will continue to build that way, for now.

Discussion: https://postgr.es/m/2872c9a0-4b0a-1354-d5f6-94d6f85ba354@enterprisedb.com

3 years agoDon't enable fsync in src/test/recovery/t/008_fsm_truncation.pl.
Tom Lane [Wed, 19 Jan 2022 17:36:49 +0000 (12:36 -0500)]
Don't enable fsync in src/test/recovery/t/008_fsm_truncation.pl.

In adverse circumstances, the fsync calls cause this test to run for
quite a long time (multiple minutes) and even suffer timeout failures.
This seems to date from before we made an effort to disable fsync in
all our test cases; there's not a lot of point in using it if there's
not a plan to force an O/S crash during the test.

Discussion: https://postgr.es/m/440239.1642560607@sss.pgh.pa.us

3 years agoRemove redundant memory context switches in BeginCopyFrom().
Tom Lane [Wed, 19 Jan 2022 17:31:15 +0000 (12:31 -0500)]
Remove redundant memory context switches in BeginCopyFrom().

This is probably a leftover from code refactoring.

Japin Li

Discussion: https://postgr.es/m/MEYP282MB16693DDABDFEC7949AC31857B6599@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM

3 years agoDynamically find correct installation docs in Makefile.
Daniel Gustafsson [Wed, 19 Jan 2022 13:48:25 +0000 (14:48 +0100)]
Dynamically find correct installation docs in Makefile.

The base Makefile will output help to the user when invoking make in
an unconfigured tree, the help was however always referring to a file
which may not be present as it's only in tarballs.  Dynamically check
for the presence of the INSTALL file and fall back on README.git when
it's not available (which is the case of Git checkouts).

Reported-by: Tim McNamara <tim@mcnamara.nz>
Reviewed-by: Magnus Hagander <magnus@hagander.net>
Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/730dae39-abaa-4140-893b-95d732fed003@www.fastmail.com

3 years agoFix alignment problem with bbsink_copystream buffer.
Robert Haas [Wed, 19 Jan 2022 13:07:16 +0000 (08:07 -0500)]
Fix alignment problem with bbsink_copystream buffer.

bbsink_copystream wants to store a type byte just before the buffer,
but basebackup.c wants the buffer to be aligned so that it can call
PageIsNew() and PageGetLSN() on it. Therefore, instead of inserting
1 extra byte before the buffer, insert MAXIMUM_ALIGNOF extra bytes
and only use the last one.

On most machines this doesn't cause any problem (except perhaps for
performance) but some buildfarm machines with -fsanitize=alignment
dump core.

Discussion: http://postgr.es/m/CA+TgmoYx5=1A2K9JYV-9zdhyokU4KKTyNQ9q7CiXrX=YBBMWVw@mail.gmail.com

3 years agodoc: Fix description of pg_replication_origin_oid() in error case
Michael Paquier [Wed, 19 Jan 2022 01:37:40 +0000 (10:37 +0900)]
doc: Fix description of pg_replication_origin_oid() in error case

This function returns NULL if the replication origin given in input
argument does not exist, contrary to what the docs described
previously.

Author: Ian Barwick
Discussion: https://postgr.es/m/CAB8KJ=htJjBL=103URqjOxV2mqb4rjphDpMeKdyKq_QXt6h05w@mail.gmail.com
Backpatch-through: 10

3 years agoMake PQcancel use the PGconn's tcp_user_timeout and keepalives settings.
Tom Lane [Tue, 18 Jan 2022 19:02:43 +0000 (14:02 -0500)]
Make PQcancel use the PGconn's tcp_user_timeout and keepalives settings.

If connectivity to the server has been lost or become flaky, the
user might well try to send a query cancel.  It's highly annoying
if PQcancel hangs up in such a case, but that's exactly what's likely
to happen.  To ameliorate this problem, apply the PGconn's
tcp_user_timeout and keepalives settings to the TCP connection used
to send the cancel.  This should be safe on Unix machines, since POSIX
specifies that setsockopt() is async-signal-safe.  We are guessing
that WSAIoctl(SIO_KEEPALIVE_VALS) is similarly safe on Windows.
(Note that at least in psql and our other frontend programs, there's
no safety issue involved anyway, since we run PQcancel in its own
thread rather than in a signal handler.)

Most of the value here comes from the expectation that tcp_user_timeout
will be applied as a connection timeout.  That appears to happen on
Linux, even though its tcp(7) man page claims differently.  The
keepalive options probably won't help much, but as long as we can
apply them for not much code, we might as well.

Jelte Fennema, reviewed by Fujii Masao and myself

Discussion: https://postgr.es/m/AM5PR83MB017870DE81FC84D5E21E9D1EF7AA9@AM5PR83MB0178.EURPRD83.prod.outlook.com

3 years agoModify pg_basebackup to use a new COPY subprotocol for base backups.
Robert Haas [Tue, 18 Jan 2022 18:47:26 +0000 (13:47 -0500)]
Modify pg_basebackup to use a new COPY subprotocol for base backups.

In the new approach, all files across all tablespaces are sent in a
single COPY OUT operation. The CopyData messages are no longer raw
archive content; rather, each message is prefixed with a type byte
that describes its purpose, e.g. 'n' signifies the start of a new
archive and 'd' signifies archive or manifest data. This protocol
is significantly more extensible than the old approach, since we can
later create more message types, though not without concern for
backward compatibility.

The new protocol sends a few things to the client that the old one
did not. First, it sends the name of each archive explicitly, instead
of letting the client compute it. This is intended to make it easier
to write future patches that might send archives in a format other
that tar (e.g. cpio, pax, tar.gz). Second, it sends explicit progress
messages rather than allowing the client to assume that progress is
defined by the number of bytes received. This will help with future
features where the server compresses the data, or sends it someplace
directly rather than transmitting it to the client.

The old protocol is still supported for compatibility with previous
releases. The new protocol is selected by means of a new
TARGET option to the BASE_BACKUP command. Currently, the
only supported target is 'client'. Support for additional
targets will be added in a later commit.

Patch by me. The patch set of which this is a part has had review
and/or testing from Jeevan Ladhe, Tushar Ahuja, Suraj Kharage,
Dipesh Pandit, and Mark Dilger.

Discussion: http://postgr.es/m/CA+TgmoaYZbz0=Yk797aOJwkGJC-LK3iXn+wzzMx7KdwNpZhS5g@mail.gmail.com

3 years agoTry to stabilize the reloptions test.
Thomas Munro [Tue, 18 Jan 2022 18:03:01 +0000 (07:03 +1300)]
Try to stabilize the reloptions test.

Where we test vacuum_truncate's effects, sometimes this is failing to
truncate as expected on the build farm.  That could be explained by page
skipping, so disable it explicitly, with the theory that commit fe246d1c
didn't go far enough.

Back-patch to 12, where the vacuum_truncate tests were added.

Discussion: https://postgr.es/m/CA%2BhUKGLT2UL5_JhmBzUgkdyKfc%3D5J-gJSQJLysMs4rqLUKLAzw%40mail.gmail.com

3 years agoRevert "Replace use of deprecated Python module distutils.sysconfig"
Peter Eisentraut [Tue, 18 Jan 2022 16:42:29 +0000 (17:42 +0100)]
Revert "Replace use of deprecated Python module distutils.sysconfig"

This reverts commit e0e567a106726f6709601ee7cffe73eb6da8084e.

On various platforms, the new approach using the sysconfig module
reported incorrect values for the include directory, and so any
Python-related compilations failed.  Revert for now and revisit later.

3 years agoFix thinko in psql test
Peter Eisentraut [Tue, 18 Jan 2022 15:53:41 +0000 (16:53 +0100)]
Fix thinko in psql test

The tests added by 14d755b00037ce04b9e24504f4b540d9e731c29e added a
test case for psql's \set ECHO errors.  After the test, it then reset
this to \set ECHO none, which is the default.  But the regression
tests are actually run under \set ECHO all (psql -a), so that would
have been the correct way to restore the previous state.  Otherwise,
test cases added after that point would not have their input lines
displayed.  This was never the intention, so fix this now.

3 years agoReplace use of deprecated Python module distutils.sysconfig
Peter Eisentraut [Tue, 18 Jan 2022 05:35:04 +0000 (06:35 +0100)]
Replace use of deprecated Python module distutils.sysconfig

With Python 3.10, configure spits out warnings about the module
distutils.sysconfig being deprecated and scheduled for removal in
Python 3.12.  Change the uses in configure to use the module sysconfig
instead.  The logic stays the same.

Note that sysconfig exists since Python 2.7, so this moves the minimum
required version up from Python 2.6.

Discussion: https://www.postgresql.org/message-id/flat/c74add3c-09c4-a9dd-1a03-a846e5b2fc52%40enterprisedb.com

3 years agoImprove code clarity in epilogue of UTF-8 verification fast path
John Naylor [Tue, 18 Jan 2022 03:53:50 +0000 (22:53 -0500)]
Improve code clarity in epilogue of UTF-8 verification fast path

The previous coding was correct, but the style and commentary were a bit
vague about which operations had to happen, in what circumstances, and
in what order. Rearrange so that the epilogue does nothing in the DFA END
state. That allows turning some conditional statements in the backtracking
logic into asserts. With that, we can be more explicit about needing
to backtrack at least one byte in non-END states to ensure checking the
current byte sequence in the slow path. No change to the regression tests,
since they should be able catch deficiencies here already.

In passing, improve the comments around DFA states where the first
continuation byte has a restricted range.

3 years agoFix psql \d's query for identifying parent triggers.
Tom Lane [Tue, 18 Jan 2022 02:18:49 +0000 (21:18 -0500)]
Fix psql \d's query for identifying parent triggers.

The original coding (from c33869cc3) failed with "more than one row
returned by a subquery used as an expression" if there were unrelated
triggers of the same tgname on parent partitioned tables.  (That's
possible because statement-level triggers don't get inherited.)  Fix
by applying LIMIT 1 after sorting the candidates by inheritance level.

Also, wrap the subquery in a CASE so that we don't have to execute it at
all when the trigger is visibly non-inherited.  Aside from saving some
cycles, this avoids the need for a confusing and undocumented NULLIF().

While here, tweak the format of the emitted query to look a bit
nicer for "psql -E", and add some explanation of this subquery,
because it badly needs it.

Report and patch by Justin Pryzby (with some editing by me).
Back-patch to v13 where the faulty code came in.

Discussion: https://postgr.es/m/20211217154356.GJ17618@telsasoft.com

3 years agotests: Consistently use pg_basebackup -cfast --no-sync to accelerate tests.
Andres Freund [Mon, 17 Jan 2022 23:40:00 +0000 (15:40 -0800)]
tests: Consistently use pg_basebackup -cfast --no-sync to accelerate tests.

Most tests invoking pg_basebackup themselves did not yet use -cfast, which
makes pg_basebackup take considerably longer. The only reason this didn't
cause the tests to take many minutes is that spread checkpoints only throttle
when writing out a buffer and there aren't that many dirty buffers in the
tests...

Discussion: https://postgr.es/m/20220117195711.xx4qbxutrrlmo2dg@alap3.anarazel.de

3 years agoheap pruning: Only call BufferGetBlockNumber() once.
Andres Freund [Mon, 17 Jan 2022 23:31:28 +0000 (15:31 -0800)]
heap pruning: Only call BufferGetBlockNumber() once.

BufferGetBlockNumber() is not that cheap and obviously cannot change during
one heap_prune_page(), so only call it once. We might be able to do better and
pass the block number from the caller, but that'd be a larger change...

Discussion: https://postgr.es/m/20211211045710.ljtuu4gfloh754rs@alap3.anarazel.de

3 years agoMove 027_stream_regress.pl's output to tmp_check.
Thomas Munro [Mon, 17 Jan 2022 18:17:37 +0000 (07:17 +1300)]
Move 027_stream_regress.pl's output to tmp_check.

Cleanup for commit f47ed79c.

Discussion: https://postgr.es/m/CA%2BhUKGKU%3DtiZoE7vp7qYFQNPdBd2pHoaOwkPMDg9YWk1h%3DFtmQ%40mail.gmail.com

3 years agopg_upgrade: Preserve relfilenodes and tablespace OIDs.
Robert Haas [Mon, 17 Jan 2022 18:32:44 +0000 (13:32 -0500)]
pg_upgrade: Preserve relfilenodes and tablespace OIDs.

Currently, database OIDs, relfilenodes, and tablespace OIDs can all
change when a cluster is upgraded using pg_upgrade. It seems better
to preserve them, because (1) it makes troubleshooting pg_upgrade
easier, since you don't have to do a lot of work to match up files
in the old and new clusters, (2) it allows 'rsync' to save bandwidth
when used to re-sync a cluster after an upgrade, and (3) if we ever
encrypt or sign blocks, we would likely want to use a nonce that
depends on these values.

This patch only arranges to preserve relfilenodes and tablespace
OIDs. The task of preserving database OIDs is left for another patch,
since it involves some complexities that don't exist in these cases.

Database OIDs have a similar issue, but there are some tricky points
in that case that do not apply to these cases, so that problem is left
for another patch.

Shruthi KC, based on an earlier patch from Antonin Houska, reviewed
and with some adjustments by me.

Discussion: http://postgr.es/m/CA+TgmoYgTwYcUmB=e8+hRHOFA0kkS6Kde85+UNdon6q7bt1niQ@mail.gmail.com

3 years agoAvoid calling gettext() in signal handlers.
Tom Lane [Mon, 17 Jan 2022 18:30:04 +0000 (13:30 -0500)]
Avoid calling gettext() in signal handlers.

It seems highly unlikely that gettext() can be relied on to be
async-signal-safe.  psql used to understand that, but someone got
it wrong long ago in the src/bin/scripts/ version of handle_sigint,
and then the bad idea was perpetuated when those two versions were
unified into src/fe_utils/cancel.c.

I'm unsure why there have not been field complaints about this
... maybe gettext() is signal-safe once it's translated at least
one message?  But we have no business assuming any such thing.

In cancel.c (v13 and up), I preserved our ability to localize
"Cancel request sent" messages by invoking gettext() before
the signal handler is set up.  In earlier branches I just made
src/bin/scripts/ not localize those messages, as psql did then.

(Just for extra unsafety, the src/bin/scripts/ version was
invoking fprintf() from a signal handler.  Sigh.)

Noted while fixing signal-safety issues in PQcancel() itself.
Back-patch to all supported branches.

Discussion: https://postgr.es/m/2937814.1641960929@sss.pgh.pa.us

3 years agoAvoid calling strerror[_r] in PQcancel().
Tom Lane [Mon, 17 Jan 2022 17:52:44 +0000 (12:52 -0500)]
Avoid calling strerror[_r] in PQcancel().

PQcancel() is supposed to be safe to call from a signal handler,
and indeed psql uses it that way.  All of the library functions
it uses are specified to be async-signal-safe by POSIX ...
except for strerror.  Neither plain strerror nor strerror_r
are considered safe.  When this code was written, back in the
dark ages, we probably figured "oh, strerror will just index
into a constant array of strings" ... but in any locale except C,
that's unlikely to be true.  Probably the reason we've not heard
complaints is that (a) this error-handling code is unlikely to be
reached in normal use, and (b) in many scenarios, localized error
strings would already have been loaded, after which maybe it's
safe to call strerror here.  Still, this is clearly unacceptable.

The best we can do without relying on strerror is to print the
decimal value of errno, so make it do that instead.  (This is
probably not much loss of user-friendliness, given that it is
hard to get a failure here.)

Back-patch to all supported branches.

Discussion: https://postgr.es/m/2937814.1641960929@sss.pgh.pa.us

3 years agoFix for new Boolean node
Peter Eisentraut [Mon, 17 Jan 2022 12:59:46 +0000 (13:59 +0100)]
Fix for new Boolean node

The token in nodeTokenType() is actually the whole rest of the string,
so we need to take into account the length to do the correct
comparison.

Without this, postgres_fdw tests fail under
-DWRITE_READ_PARSE_PLAN_TREES.

3 years agoAdd Boolean node
Peter Eisentraut [Fri, 14 Jan 2022 09:46:49 +0000 (10:46 +0100)]
Add Boolean node

Before, SQL-level boolean constants were represented by a string with
a cast, and internal Boolean values in DDL commands were usually
represented by Integer nodes.  This takes the place of both of these
uses, making the intent clearer and having some amount of type safety.

Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/8c1a2e37-c68d-703c-5a83-7a6077f4f997@enterprisedb.com

3 years agoFix typo in pg_dumpall.c
Michael Paquier [Mon, 17 Jan 2022 07:03:12 +0000 (16:03 +0900)]
Fix typo in pg_dumpall.c

Oversight in 2158628.

Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/20220117062006.GY14051@telsasoft.com

3 years agoAdd support for --no-table-access-method in pg_{dump,dumpall,restore}
Michael Paquier [Mon, 17 Jan 2022 05:51:46 +0000 (14:51 +0900)]
Add support for --no-table-access-method in pg_{dump,dumpall,restore}

The logic is similar to default_tablespace in some ways, so as no SET
queries on default_table_access_method are generated before dumping or
restoring an object (table or materialized view support table AMs) when
specifying this new option.

This option is useful to enforce the use of a default access method even
if some tables included in a dump use an AM different than the system's
default.

There are already two cases in the TAP tests of pg_dump with a table and
a materialized view that use a non-default table AM, and these are
extended that the new option does not generate SET clauses on
default_table_access_method.

Author: Justin Pryzby
Discussion: https://postgr.es/m/20211207153930.GR17618@telsasoft.com

3 years agoTest replay of regression tests, attempt II.
Thomas Munro [Mon, 17 Jan 2022 03:34:55 +0000 (16:34 +1300)]
Test replay of regression tests, attempt II.

See commit message for 123828a7fa563025d0ceee10cf1b2a253cd05319.  The
only change this time is the order of the arguments passed to
pg_regress.  The previously version broke in the build farm environment
due to the contents of EXTRA_REGRESS_OPTS (see also commit 8cade04c
which had to do something similar).

Discussion: https://postgr.es/m/CA%2BhUKGKpRWQ9SxdxxDmTBCJoR0YnFpMBe7kyzY8SUQk%2BHeskxg%40mail.gmail.com

3 years agoConsistently use the function name CreateCheckPoint in code and comments.
Amit Kapila [Mon, 17 Jan 2022 02:20:00 +0000 (07:50 +0530)]
Consistently use the function name CreateCheckPoint in code and comments.

Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACVZmKsvDjtd45+9oTcnjUJtC4LF2BYK8TpWT1f=NjJX3w@mail.gmail.com

3 years agoIntroduce log_destination=jsonlog
Michael Paquier [Mon, 17 Jan 2022 01:16:53 +0000 (10:16 +0900)]
Introduce log_destination=jsonlog

"jsonlog" is a new value that can be added to log_destination to provide
logs in the JSON format, with its output written to a file, making it
the third type of destination of this kind, after "stderr" and
"csvlog".  The format is convenient to feed logs to other applications.
There is also a plugin external to core that provided this feature using
the hook in elog.c, but this had to overwrite the output of "stderr" to
work, so being able to do both at the same time was not possible.  The
files generated by this log format are suffixed with ".json", and use
the same rotation policies as the other two formats depending on the
backend configuration.

This takes advantage of the refactoring work done previously in ac7c807,
bed6ed38b76f89 and 2d77d83 for the backend parts, and 72b76f7 for the
TAP tests, making the addition of any new file-based format rather
straight-forward.

The documentation is updated to list all the keys and the values that
can exist in this new format.  pg_current_logfile() also required a
refresh for the new option.

Author: Sehrope Sarkuni, Michael Paquier
Reviewed-by: Nathan Bossart, Justin Pryzby
Discussion: https://postgr.es/m/CAH7T-aqswBM6JWe4pDehi1uOiufqe06DJWaU5=X7dDLyqUExHg@mail.gmail.com

3 years agoTeach hash_ok_operator() that record_eq is only sometimes hashable.
Tom Lane [Sun, 16 Jan 2022 21:39:26 +0000 (16:39 -0500)]
Teach hash_ok_operator() that record_eq is only sometimes hashable.

The need for this was foreseen long ago, but when record_eq
actually became hashable (in commit 01e658fa7), we missed updating
this spot.

Per bug #17363 from Elvis Pranskevichus.  Back-patch to v14 where
the faulty commit came in.

Discussion: https://postgr.es/m/17363-f6d42fd0d726be02@postgresql.org

3 years agoFix psql's tab-completion of enum label values.
Tom Lane [Sun, 16 Jan 2022 19:59:20 +0000 (14:59 -0500)]
Fix psql's tab-completion of enum label values.

Since enum labels have to be single-quoted, this part of the
tab completion machinery got side-swiped by commit cd69ec66c.
A side-effect of that commit is that (at least with some versions
of Readline) the text string passed for completion will omit the
leading quote mark of the enum label literal.  Libedit still acts
the same as before, though, so adapt COMPLETE_WITH_ENUM_VALUE so
that it can cope with either convention.

Also, when we fail to find any valid completion, set
rl_completion_suppress_quote = 1.  Otherwise readline will
go ahead and append a closing quote, which is unwanted.

Per report from Peter Eisentraut.  Back-patch to v13 where
cd69ec66c came in.

Discussion: https://postgr.es/m/8ca82d89-ec3d-8b28-8291-500efaf23b25@enterprisedb.com

3 years agoClean up TAP tests' usage of wait_for_catchup().
Tom Lane [Sun, 16 Jan 2022 18:29:02 +0000 (13:29 -0500)]
Clean up TAP tests' usage of wait_for_catchup().

By default, wait_for_catchup() waits for the replication connection
to reach the primary's write LSN.  That's fine, but in an apparent
attempt to save one query round-trip, it was coded so that we
executed pg_current_wal_lsn() again during each probe query.
Thus, we presented the standby with a moving target to be reached.
(While the test script itself couldn't be causing the write LSN
to advance while it's blocked in wait_for_catchup(), it's plenty
plausible that background activity such as autovacuum is emitting
more WAL.)  That could make the test take longer than necessary,
and potentially it could mask bugs by allowing the standby to process
more WAL than a strict interpretation of the test scenario allows.
So, change wait_for_catchup() to do it "by the book", explicitly
collecting the write LSN to wait for at the outset.

Also, various call sites were instructing wait_for_catchup() to
wait for the standby to reach the primary's insert LSN rather than
its write LSN.  This also seems like a bad idea.  While in most
test scenarios those are the same, if they are different then the
inserted-but-not-yet-written WAL is not presently available to the
standby.  The test isn't doing anything to make it become so, so
again we have the potential for unwanted test delay, perhaps even
a test timeout.  (Again, background activity would be needed to
make this more than a hypothetical problem.)  Hence, change the
callers where necessary so that the wait target is always the
primary's write LSN.

While at it, simplify callers by making use of wait_for_catchup's
default arguments wherever possible (the preceding change makes
this possible in more places than it was before).  And rewrite
wait_for_catchup's documentation a bit.

Patch by me; thanks to Julien Rouhaud for review.

Discussion: https://postgr.es/m/2368336.1641843098@sss.pgh.pa.us

3 years agoAdd stxdinherit flag to pg_statistic_ext_data
Tomas Vondra [Sun, 16 Jan 2022 12:37:56 +0000 (13:37 +0100)]
Add stxdinherit flag to pg_statistic_ext_data

Add pg_statistic_ext_data.stxdinherit flag, so that for each extended
statistics definition we can store two versions of data - one for the
relation alone, one for the whole inheritance tree. This is analogous to
pg_statistic.stainherit, but we failed to include such flag in catalogs
for extended statistics, and we had to work around it (see commits
859b3003de36c4bc6e72 and 20b9fa308e).

This changes the relationship between the two catalogs storing extended
statistics objects (pg_statistic_ext and pg_statistic_ext_data). Until
now, there was a simple 1:1 mapping - for each definition there was one
pg_statistic_ext_data row, and this row was inserted while creating the
statistics (and then updated during ANALYZE). With the stxdinherit flag,
we don't know how many rows there will be (child relations may be added
after the statistics object is defined), so there may be up to two rows.

We could make CREATE STATISTICS to always create both rows, but that
seems wasteful - without partitioning we only need stxdinherit=false
rows, and declaratively partitioned tables need only stxdinherit=true.
So we no longer initialize pg_statistic_ext_data in CREATE STATISTICS,
and instead make that a responsibility of ANALYZE. Which is what we do
for regular statistics too.

Patch by me, with extensive improvements and fixes by Justin Pryzby.

Author: Tomas Vondra, Justin Pryzby
Reviewed-by: Tomas Vondra, Justin Pryzby
Discussion: https://postgr.es/m/20210923212624.GI831%40telsasoft.com

3 years agoUpdate copyright notice to 2022 for recently-introduced TAP test
Michael Paquier [Sun, 16 Jan 2022 12:19:30 +0000 (21:19 +0900)]
Update copyright notice to 2022 for recently-introduced TAP test

Subscription test 027_nosuperuser.pl has been introduced in a2ab9c0,
after the notices got refreshed to 2022 in 27b77ec.

3 years agoRemove standby_schedule and associated test files.
Tom Lane [Sat, 15 Jan 2022 20:54:10 +0000 (15:54 -0500)]
Remove standby_schedule and associated test files.

Since this test schedule is not run by default, it's next door to
unused.  Moreover, its test coverage is very thin, and what there is
is just about entirely superseded by the src/test/recovery tests.
Let's drop it instead of carrying obsolete tests.

Discussion: https://postgr.es/m/3911012.1641246643@sss.pgh.pa.us

3 years agoAdd simple test for physical replication of sequences.
Tom Lane [Sat, 15 Jan 2022 20:47:00 +0000 (15:47 -0500)]
Add simple test for physical replication of sequences.

AFAICS we had no coverage of this point except in the seldom-used,
slated-for-removal standby_schedule test suite.  Sequence updates
are enough different from regular table updates that it seems worth
covering them explicitly in src/test/recovery.

Discussion: https://postgr.es/m/999497.1641431891@sss.pgh.pa.us

3 years agoBuild inherited extended stats on partitioned tables
Tomas Vondra [Sat, 15 Jan 2022 17:17:20 +0000 (18:17 +0100)]
Build inherited extended stats on partitioned tables

Commit 859b3003de disabled building of extended stats for inheritance
trees, to prevent updating the same catalog row twice. While that
resolved the issue, it also means there are no extended stats for
declaratively partitioned tables, because there are no data in the
non-leaf relations.

That also means declaratively partitioned tables were not affected by
the issue 859b3003de addressed, which means this is a regression
affecting queries that calculate estimates for the whole inheritance
tree as a whole (which includes e.g. GROUP BY queries).

But because partitioned tables are empty, we can invert the condition
and build statistics only for the case with inheritance, without losing
anything. And we can consider them when calculating estimates.

It may be necessary to run ANALYZE on partitioned tables, to collect
proper statistics. For declarative partitioning there should no prior
statistics, and it might take time before autoanalyze is triggered. For
tables partitioned by inheritance the statistics may include data from
child relations (if built 859b3003de), contradicting the current code.

Report and patch by Justin Pryzby, minor fixes and cleanup by me.
Backpatch all the way back to PostgreSQL 10, where extended statistics
were introduced (same as 859b3003de).

Author: Justin Pryzby
Reported-by: Justin Pryzby
Backpatch-through: 10
Discussion: https://postgr.es/m/20210923212624.GI831%40telsasoft.com

3 years agoAdd tab-completion for CREATE FOREIGN TABLE.
Fujii Masao [Sat, 15 Jan 2022 02:22:24 +0000 (11:22 +0900)]
Add tab-completion for CREATE FOREIGN TABLE.

Unlike CREATE TABLE, CREATE FOREIGN TABLE is not allowed inside
CREATE SCHEMA, so Matches() is used instead of TailMatches() for
the tab-completion.

Author: Tang <tanghy.fnst@fujitsu.com>
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/OS0PR01MB61137E96E0551278782D11CDFB519@OS0PR01MB6113.jpnprd01.prod.outlook.com

3 years agoIgnore extended statistics for inheritance trees
Tomas Vondra [Sat, 15 Jan 2022 01:15:23 +0000 (02:15 +0100)]
Ignore extended statistics for inheritance trees

Since commit 859b3003de we only build extended statistics for individual
relations, ignoring the child relations. This resolved the issue with
updating catalog tuple twice, but we still tried to use the statistics
when calculating estimates for the whole inheritance tree. When the
relations contain very distinct data, it may produce bogus estimates.

This is roughly the same issue 427c6b5b9 addressed ~15 years ago, and we
fix it the same way - by ignoring extended statistics when calculating
estimates for the inheritance tree as a whole. We still consider
extended statistics when calculating estimates for individual child
relations, of course.

This may result in plan changes due to different estimates, but if the
old statistics were not describing the inheritance tree particularly
well it's quite likely the new plans is actually better.

Report and patch by Justin Pryzby, minor fixes and cleanup by me.
Backpatch all the way back to PostgreSQL 10, where extended statistics
were introduced (same as 859b3003de).

Author: Justin Pryzby
Reported-by: Justin Pryzby
Backpatch-through: 10
Discussion: https://postgr.es/m/20210923212624.GI831%40telsasoft.com

3 years agoUnify VACUUM VERBOSE and autovacuum logging.
Peter Geoghegan [Sat, 15 Jan 2022 00:50:34 +0000 (16:50 -0800)]
Unify VACUUM VERBOSE and autovacuum logging.

The log_autovacuum_min_duration instrumentation used its own dedicated
code for logging, which was not reused by VACUUM VERBOSE.  This was
highly duplicative, and sometimes led to each code path using slightly
different accounting for essentially the same information.

Clean things up by making VACUUM VERBOSE reuse the same instrumentation
code.  This code restructuring changes the structure of the VACUUM
VERBOSE output itself, but that seems like an overall improvement.  The
most noticeable change in VACUUM VERBOSE output is that it no longer
outputs a distinct message per index per round of index vacuuming.  Most
of the same information (about each index) is now shown in its new
per-operation summary message.  This is far more legible.

A few details are no longer displayed by VACUUM VERBOSE, but that's no
real loss in practice, especially in the common case where we don't need
multiple index scans/rounds of vacuuming.  This super fine-grained
information is still available via DEBUG2 messages, which might still be
useful in debugging scenarios.

VACUUM VERBOSE now shows new instrumentation, which is typically very
useful: all of the log_autovacuum_min_duration instrumentation that it
missed out on before now.  This includes information about WAL overhead,
buffers hit/missed/dirtied information, and I/O timing information.

VACUUM VERBOSE still retains a few INFO messages of its own.  This is
limited to output concerning the progress of heap rel truncation, as
well as some basic information about parallel workers.  These details
are still potentially quite useful.  They aren't a good fit for the log
output, which must summarize the whole operation.

Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-By: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CAH2-WzmW4Me7_qR4X4ka7pxP-jGmn7=Npma_-Z-9Y1eD0MQRLw@mail.gmail.com

3 years agoRevert "Add new simple TAP test for tablespaces."
Thomas Munro [Fri, 14 Jan 2022 13:16:07 +0000 (02:16 +1300)]
Revert "Add new simple TAP test for tablespaces."

This reverts commit d1511fe1b040853f6e10d353e56b42bb96ae239d.

Discussion: https://postgr.es/m/CA%2BhUKG%2BGBC-6QhOKt6Y7ccrXSjbRHB7Di295%3D0rAGhE7a7hSrQ%40mail.gmail.com

3 years agoRevert "Test replay of regression tests."
Thomas Munro [Fri, 14 Jan 2022 11:38:44 +0000 (00:38 +1300)]
Revert "Test replay of regression tests."

This reverts commit 123828a7fa563025d0ceee10cf1b2a253cd05319.

Discussion: https://postgr.es/m/CA%2BhUKG%2BGBC-6QhOKt6Y7ccrXSjbRHB7Di295%3D0rAGhE7a7hSrQ%40mail.gmail.com

3 years agoTest replay of regression tests.
Thomas Munro [Fri, 14 Jan 2022 08:39:18 +0000 (21:39 +1300)]
Test replay of regression tests.

Add a new TAP test under src/test/recovery to run the standard
regression tests while a streaming replica replays the WAL.  This
provides a basic workout for WAL decoding and redo code, and compares
the replicated result.

Optionally, enable (expensive) wal_consistency_checking if listed in
the env variable PG_TEST_EXTRA.

Reviewed-by: 綱川 貴之 (Takayuki Tsunakawa) <tsunakawa.takay@fujitsu.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Andrew Dunstan <andrew@dunslane.net>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Anastasia Lubennikova <lubennikovaav@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CA%2BhUKGKpRWQ9SxdxxDmTBCJoR0YnFpMBe7kyzY8SUQk%2BHeskxg%40mail.gmail.com

3 years agoAdd new simple TAP test for tablespaces.
Thomas Munro [Fri, 14 Jan 2022 08:29:59 +0000 (21:29 +1300)]
Add new simple TAP test for tablespaces.

The tablespace tests in the main regression tests have been changed to
use "in-place" tablespaces, so that they work when streamed to a replica
on the same host.  Add a new TAP test that exercises tablespaces with
absolute paths, for coverage.

Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CA%2BhUKGKpRWQ9SxdxxDmTBCJoR0YnFpMBe7kyzY8SUQk%2BHeskxg%40mail.gmail.com

3 years agoUse in-place tablespaces in regression test.
Thomas Munro [Fri, 14 Jan 2022 08:29:17 +0000 (21:29 +1300)]
Use in-place tablespaces in regression test.

Remove the machinery from pg_regress that manages the testtablespace
directory.  Instead, use "in-place" tablespaces, because they work
correctly when there is a streaming replica running on the same host.

Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CA%2BhUKGKpRWQ9SxdxxDmTBCJoR0YnFpMBe7kyzY8SUQk%2BHeskxg%40mail.gmail.com

3 years agoAllow "in place" tablespaces.
Thomas Munro [Fri, 14 Jan 2022 08:27:44 +0000 (21:27 +1300)]
Allow "in place" tablespaces.

Provide a developer-only GUC allow_in_place_tablespaces, disabled by
default.  When enabled, tablespaces can be created with an empty
LOCATION string, meaning that they should be created as a directory
directly beneath pg_tblspc.  This can be used for new testing scenarios,
in a follow-up patch.  Not intended for end-user usage, since it might
confuse backup tools that expect symlinks.

Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CA%2BhUKGKpRWQ9SxdxxDmTBCJoR0YnFpMBe7kyzY8SUQk%2BHeskxg%40mail.gmail.com

3 years agoRename value node fields
Peter Eisentraut [Fri, 14 Jan 2022 09:46:49 +0000 (10:46 +0100)]
Rename value node fields

For the formerly-Value node types, rename the "val" field to a name
specific to the node type, namely "ival", "fval", "sval", and "bsval".
This makes some code clearer and catches mixups better.

Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/8c1a2e37-c68d-703c-5a83-7a6077f4f997@enterprisedb.com

3 years agoRefactor AlterRole()
Peter Eisentraut [Fri, 14 Jan 2022 09:46:49 +0000 (10:46 +0100)]
Refactor AlterRole()

Get rid of the three-valued logic for the Boolean variables to track
whether the value was been specified and what the new value should be.
Instead, we can use the "dfoo" variables to determine whether the
value was specified and should be applied.  This was already done in
some cases, so this makes this more uniform and removes one layer of
indirection.

Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/8c1a2e37-c68d-703c-5a83-7a6077f4f997@enterprisedb.com

3 years agoAssert redirect pointers are sensible after heap_page_prune().
Andres Freund [Mon, 22 Nov 2021 17:52:45 +0000 (09:52 -0800)]
Assert redirect pointers are sensible after heap_page_prune().

Corruption of redirect item pointers often only becomes visible well after
being corrupted, as e.g. bug #17255 shows: In the original reproducer,
gigabyte of WAL were between the source of the corruption and the corruption
becoming visible.

To make it easier to find / prevent such bugs, verify whether redirect
pointers are sensible at the end of heap_page_prune_execute(). 5cd7eb1f1c32
introduced related assertions while modifying the page, but they can't easily
detect marking the target of an existing redirect as unused. Sometimes the
corruption will be detected later, but that's harder to diagnose.

Author: Andres Freund <andres@andres@anarazel.de>
Reviewed-By: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/20211122175914.ayk6gg6nvdwuhrzb@alap3.anarazel.de

3 years agoFix possible HOT corruption when RECENTLY_DEAD changes to DEAD while pruning.
Andres Freund [Sat, 11 Dec 2021 04:12:26 +0000 (20:12 -0800)]
Fix possible HOT corruption when RECENTLY_DEAD changes to DEAD while pruning.

Since dc7420c2c92 the horizon used for pruning is determined "lazily". A more
accurate horizon is built on-demand, rather than in GetSnapshotData(). If a
horizon computation is triggered between two HeapTupleSatisfiesVacuum() calls
for the same tuple, the result can change from RECENTLY_DEAD to DEAD.

heap_page_prune() can process the same tid multiple times (once following an
update chain, once "directly"). When the result of HeapTupleSatisfiesVacuum()
of a tuple changes from RECENTLY_DEAD during the first access, to DEAD in the
second, the "tuple is DEAD and doesn't chain to anything else" path in
heap_prune_chain() can end up marking the target of a LP_REDIRECT ItemId
unused.

Initially not easily visible,
Once the target of a LP_REDIRECT ItemId is marked unused, a new tuple version
can reuse it. At that point the corruption may become visible, as index
entries pointing to the "original" redirect item, now point to a unrelated
tuple.

To fix, compute HTSV for all tuples on a page only once. This fixes the entire
class of problems of HTSV changing inside heap_page_prune(). However,
visibility changes can obviously still occur between HTSV checks inside
heap_page_prune() and outside (e.g. in lazy_scan_prune()).

The computation of HTSV is now done in bulk, in heap_page_prune(), rather than
on-demand in heap_prune_chain(). Besides being a bit simpler, it also is
faster: Memory accesses can happen sequentially, rather than in the order of
HOT chains.

There are other causes of HeapTupleSatisfiesVacuum() results changing between
two visibility checks for the same tuple, even before dc7420c2c92. E.g.
HEAPTUPLE_INSERT_IN_PROGRESS can change to HEAPTUPLE_DEAD when a transaction
aborts between the two checks. None of the these other visibility status
changes are known to cause corruption, but heap_page_prune()'s approach makes
it hard to be confident.

A patch implementing a more fundamental redesign of heap_page_prune(), which
fixes this bug and simplifies pruning substantially, has been proposed by
Peter Geoghegan in
https://postgr.es/m/CAH2-WzmNk6V6tqzuuabxoxM8HJRaWU6h12toaS-bqYcLiht16A@mail.gmail.com

However, that redesign is larger change than desirable for backpatching. As
the new design still benefits from the batched visibility determination
introduced in this commit, it makes sense to commit this narrower fix to 14
and master, and then commit Peter's improvement in master.

The precise sequence required to trigger the bug is complicated and hard to do
exercise in an isolation test (until we have wait points). Due to that the
isolation test initially posted at
https://postgr.es/m/20211119003623.d3jusiytzjqwb62p%40alap3.anarazel.de
and updated in
https://postgr.es/m/20211122175914.ayk6gg6nvdwuhrzb%40alap3.anarazel.de
isn't committable.

A followup commit will introduce additional assertions, to detect problems
like this more easily.

Bug: #17255
Reported-By: Alexander Lakhin <exclusion@gmail.com>
Debugged-By: Andres Freund <andres@anarazel.de>
Debugged-By: Peter Geoghegan <pg@bowt.ie>
Author: Andres Freund <andres@andres@anarazel.de>
Reviewed-By: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/20211122175914.ayk6gg6nvdwuhrzb@alap3.anarazel.de
Backpatch: 14-, the oldest branch containing dc7420c2c92

3 years agoFix ruleutils.c's dumping of whole-row Vars in more contexts.
Tom Lane [Thu, 13 Jan 2022 22:49:25 +0000 (17:49 -0500)]
Fix ruleutils.c's dumping of whole-row Vars in more contexts.

Commit 7745bc352 intended to ensure that whole-row Vars would be
printed with "::type" decoration in all contexts where plain
"var.*" notation would result in star-expansion, notably in
ROW() and VALUES() constructs.  However, it missed the case of
INSERT with a single-row VALUES, as reported by Timur Khanjanov.

Nosing around ruleutils.c, I found a second oversight: the
code for RowCompareExpr generates ROW() notation without benefit
of an actual RowExpr, and naturally it wasn't in sync :-(.
(The code for FieldStore also does this, but we don't expect that
to generate strictly parsable SQL anyway, so I left it alone.)

Back-patch to all supported branches.

Discussion: https://postgr.es/m/efaba6f9-4190-56be-8ff2-7a1674f9194f@intrans.baku.az

3 years agoci: windows: run initdb with --no-sync.
Andres Freund [Thu, 13 Jan 2022 18:56:41 +0000 (10:56 -0800)]
ci: windows: run initdb with --no-sync.

Author: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/20220110220748.GS14051@telsasoft.com

3 years agoci: windows: enable build summary to make it easier to spot warnings / errors.
Andres Freund [Thu, 13 Jan 2022 18:26:22 +0000 (10:26 -0800)]
ci: windows: enable build summary to make it easier to spot warnings / errors.

The build summary was disabled unintentionally by setting the build verbosity
lower.

While at it, also add ForceNoAlign to prevent msbuild from introducing
linebreaks in the middle of filenames etc - they make it harder to copy
output.

Discussion: https://postgr.es/m/20220113175554.u6gw7olrdfzivl3n@alap3.anarazel.de

3 years agoImprove error handling of HMAC computations
Michael Paquier [Thu, 13 Jan 2022 07:17:21 +0000 (16:17 +0900)]
Improve error handling of HMAC computations

This is similar to b69aba7, except that this completes the work for
HMAC with a new routine called pg_hmac_error() that would provide more
context about the type of error that happened during a HMAC computation:
- The fallback HMAC implementation in hmac.c relies on cryptohashes, so
in some code paths it is necessary to return back the error generated by
cryptohashes.
- For the OpenSSL implementation (hmac_openssl.c), the logic is very
similar to cryptohash_openssl.c, where the error context comes from
OpenSSL if one of its internal routines failed, with different error
codes if something internal to hmac_openssl.c failed or was incorrect.

Any in-core code paths that use the centralized HMAC interface are
related to SCRAM, for errors that are unlikely going to happen, with
only SHA-256.  It would be possible to see errors when computing some
HMACs with MD5 for example and OpenSSL FIPS enabled, and this commit
would help in reporting the correct errors but nothing in core uses
that.  So, at the end, no backpatch to v14 is done, at least for now.

Errors in SCRAM related to the computation of the server key, stored
key, etc. need to pass down the potential error context string across
more layers of their respective call stacks for the frontend and the
backend, so each surrounding routine is adapted for this purpose.

Reviewed-by: Sergey Shinderuk
Discussion: https://postgr.es/m/Yd0N9tSAIIkFd+qi@paquier.xyz

3 years agodoc: Add "(process)" to the term "WAL receiver" in glossary.
Fujii Masao [Thu, 13 Jan 2022 03:17:33 +0000 (12:17 +0900)]
doc: Add "(process)" to the term "WAL receiver" in glossary.

This commit changes the term "WAL receiver" to "WAL receiver (process)"
in the glossary, so that users can easily understand WAL receiver is
obviously a process.

Author: Fujii Masao
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/51b76596-ab4f-c9f0-1005-60ce1bb14b6b@oss.nttdata.com

3 years agoFix incorrect comments in hmac.c and hmac_openssl.c
Michael Paquier [Thu, 13 Jan 2022 00:43:36 +0000 (09:43 +0900)]
Fix incorrect comments in hmac.c and hmac_openssl.c

Both files referred to pg_hmac_ctx->data, which, I guess, comes from the
early versions of the patch that has resulted in commit e6bdfd9.

Author: Sergey Shinderuk
Discussion: https://postgr.es/m/8cbb56dd-63d6-a581-7a65-25a97ac4be03@postgrespro.ru
Backpatch-through: 14

3 years agoFix memory leak in indexUnchanged hint mechanism.
Peter Geoghegan [Wed, 12 Jan 2022 23:41:04 +0000 (15:41 -0800)]
Fix memory leak in indexUnchanged hint mechanism.

Commit 9dc718bd added a "logically unchanged by UPDATE" hinting
mechanism, which is currently used within nbtree indexes only (see
commit d168b666).  This mechanism determined whether or not the incoming
item is a logically unchanged duplicate (a duplicate needed only for
MVCC versioning purposes) once per row updated per non-HOT update.  This
approach led to memory leaks which were noticeable with an UPDATE
statement that updated sufficiently many rows, at least on tables that
happen to have an expression index.

On HEAD, fix the issue by adding a cache to the executor's per-index
IndexInfo struct.

Take a different approach on Postgres 14 to avoid an ABI break: simply
pass down the hint to all indexes unconditionally with non-HOT UPDATEs.
This is deemed acceptable because the hint is currently interpreted
within btinsert() as "perform a bottom-up index deletion pass if and
when the only alternative is splitting the leaf page -- prefer to delete
any LP_DEAD-set items first".  nbtree must always treat the hint as a
noisy signal about what might work, as a strategy of last resort, with
costs imposed on non-HOT updaters.  (The same thing might not be true
within another index AM that applies the hint, which is why the original
behavior is preserved on HEAD.)

Author: Peter Geoghegan <pg@bowt.ie>
Reported-By: Klaudie Willis <Klaudie.Willis@protonmail.com>
Diagnosed-By: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/261065.1639497535@sss.pgh.pa.us
Backpatch: 14-, where the hinting mechanism was added.

3 years agovacuumlazy.c: fix "garbage tuples" reference.
Peter Geoghegan [Wed, 12 Jan 2022 22:13:35 +0000 (14:13 -0800)]
vacuumlazy.c: fix "garbage tuples" reference.

Another minor oversight in commit 4f8d9d12.

3 years agoConsider fractional paths in generate_orderedappend_paths
Tomas Vondra [Wed, 12 Jan 2022 18:59:30 +0000 (19:59 +0100)]
Consider fractional paths in generate_orderedappend_paths

When building append paths, we've been looking only at startup and total
costs for the paths. When building fractional paths that may eliminate
the cheapest one, because it may be dominated by two separate paths (one
for startup, one for total cost).

This extends generate_orderedappend_paths() to also consider which paths
have lowest fractional cost. Currently we only consider paths matching
pathkeys - in the future this may be improved by also considering paths
that are only partially sorted, with an incremental sort on top.

Original report of an issue by Arne Roland, patch by me (based on a
suggestion by Tom Lane).

Reviewed-by: Arne Roland, Zhihong Yu
Discussion: https://postgr.es/m/e8f9ec90-546d-e948-acce-0525f3e92773%40enterprisedb.com
Discussion: https://postgr.es/m/1581042da8044e71ada2d6e3a51bf7bb%40index.de

3 years agoAdd index on pg_publication_rel.prpubid
Alvaro Herrera [Wed, 12 Jan 2022 19:23:42 +0000 (16:23 -0300)]
Add index on pg_publication_rel.prpubid

This should have been added for the benefit of GetPublicationRelations;
let's add it now.

I couldn't measure a performance difference in the TAP tests, but that
may be because the tests use very few publications.

Discussion: https://postgr.es/m/202201120041.p24wvsfcsope@alvherre.pgsql

3 years agoInclude permissive/enforcing state in sepgsql log messages.
Tom Lane [Wed, 12 Jan 2022 19:23:13 +0000 (14:23 -0500)]
Include permissive/enforcing state in sepgsql log messages.

SELinux itself does this (at least in modern releases), and it
seems like a good idea to reduce confusion.

Dave Page

Discussion: https://postgr.es/m/CA+OCxowsQoLEYc=jN7OtNvOdX0Jg5L7nMYt++=k0X78HGq-sXg@mail.gmail.com

3 years agoecpg: Catch zero-length Unicode identifiers correctly
Peter Eisentraut [Wed, 12 Jan 2022 09:39:57 +0000 (10:39 +0100)]
ecpg: Catch zero-length Unicode identifiers correctly

The previous code to detect a zero-length identifier when using
Unicode identifiers such as

exec sql select u&"";

did not work.  This fixes that.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/82fafa79-331c-9d65-e51b-8b5d1b2383fc%40enterprisedb.com

3 years agoMove any code specific to log_destination=csvlog to its own file
Michael Paquier [Wed, 12 Jan 2022 06:03:48 +0000 (15:03 +0900)]
Move any code specific to log_destination=csvlog to its own file

The recent refactoring done in ac7c807 makes this move possible and
simple, as this just moves some code around.  This reduces the size of
elog.c by 7%.

Author: Michael Paquier, Sehrope Sarkuni
Reviewed-by: Nathan Bossart
Discussion: https://postgr.es/m/CAH7T-aqswBM6JWe4pDehi1uOiufqe06DJWaU5=X7dDLyqUExHg@mail.gmail.com

simply moves the routines related to csvlog into their own file

3 years agoRefactor set of routines specific to elog.c
Michael Paquier [Wed, 12 Jan 2022 05:16:59 +0000 (14:16 +0900)]
Refactor set of routines specific to elog.c

This refactors the following routines and facilities coming from
elog.c, to ease their use across multiple log destinations:
- Start timestamp, including its reset, to store when a process has been
started.
- The log timestamp, associated to an entry (the same timestamp is used
when logging across multiple destinations).
- Routine deciding if a query can be logged or not.
- The backend type names, depending on the process that logs any
information (postmaster, bgworker name or just GetBackendTypeDesc() with
a regular backend).
- Write of logs using the logging piped protocol, with the log collector
enabled.
- Error severity converted to a string.

These refactored routines will be used for some follow-up changes
to move all the csvlog logic into its own file and to potentially add
JSON as log destination, reducing the overall size of elog.c as the end
result.

Author: Michael Paquier, Sehrope Sarkuni
Reviewed-by: Nathan Bossart
Discussion: https://postgr.es/m/CAH7T-aqswBM6JWe4pDehi1uOiufqe06DJWaU5=X7dDLyqUExHg@mail.gmail.com

3 years agoFix comment related to pg_cryptohash_error()
Michael Paquier [Wed, 12 Jan 2022 03:39:36 +0000 (12:39 +0900)]
Fix comment related to pg_cryptohash_error()

One of the comments introduced in b69aba7 was worded a bit weirdly, so
improve it.

Reported-by: Sergey Shinderuk
Discussion: https://postgr.es/m/71b9a5d2-a3bf-83bc-a243-93dcf0bcfb3b@postgrespro.ru
Backpatch-through: 14

3 years agoAdd missing include guard to win32ntdll.h.
Thomas Munro [Tue, 11 Jan 2022 21:11:50 +0000 (10:11 +1300)]
Add missing include guard to win32ntdll.h.

Oversight in commit e2f0f8ed.  Also add this file to the exclusion lists
in headerscheck and cpluscpluscheck, because Unix systems don't have a
header it includes.

Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/2760528.1641929756%40sss.pgh.pa.us

3 years agoImprove error message for missing extension.
Tom Lane [Tue, 11 Jan 2022 19:22:00 +0000 (14:22 -0500)]
Improve error message for missing extension.

If we get ENOENT while trying to read an extension control file,
report that as a missing extension (with a HINT to install it)
rather than as a filesystem access problem.  The message wording
was extensively bikeshedded in hopes of pointing people to the
idea that they need to do a software installation before they
can install the extension into the current database.

Nathan Bossart, with review/wording suggestions from Daniel
Gustafsson, Chapman Flack, and myself

Discussion: https://postgr.es/m/3950D56A-4E47-48E7-BF9B-F5F22E268BE7@amazon.com

3 years agoClean up messy API for src/port/thread.c.
Tom Lane [Tue, 11 Jan 2022 18:46:12 +0000 (13:46 -0500)]
Clean up messy API for src/port/thread.c.

The point of this patch is to reduce inclusion spam by not needing
to #include <netdb.h> or <pwd.h> in port.h (which is read by every
compile in our tree).  To do that, we must remove port.h's
declarations of pqGetpwuid and pqGethostbyname.

pqGethostbyname is only used, and is only ever likely to be used,
in src/port/getaddrinfo.c --- which isn't even built on most
platforms, making pqGethostbyname dead code for most people.
Hence, deal with that by just moving it into getaddrinfo.c.

To clean up pqGetpwuid, invent a couple of simple wrapper
functions with less-messy APIs.  This allows removing some
duplicate error-handling code, too.

In passing, remove thread.c from the MSVC build, since it
contains nothing we use on Windows.

Noted while working on 376ce3e40.

Discussion: https://postgr.es/m/1634252654444.90107@mit.edu

3 years agoImprove warning message in pg_signal_backend()
John Naylor [Tue, 7 Dec 2021 22:31:47 +0000 (22:31 +0000)]
Improve warning message in pg_signal_backend()

Previously, invoking pg_terminate_backend() or pg_cancel_backend()
with the postmaster PID produced a "PID XXXX is not a PostgresSQL
server process" warning, which does not make sense. Change to
"backend process" to make the message more exact.

Nathan Bossart, based on an idea from Bharath Rupireddy with
input from Tom Lane and Euler Taveira

Discussion: https://www.postgresql.org/message-id/flat/CALj2ACW7Rr-R7mBcBQiXWPp=JV5chajjTdudLiF5YcpW-BmHhg@mail.gmail.com

3 years agoClean up error message reported after \password encryption failure.
Tom Lane [Tue, 11 Jan 2022 17:03:06 +0000 (12:03 -0500)]
Clean up error message reported after \password encryption failure.

Experimenting with FIPS mode enabled, I saw

regression=# \password joe
Enter new password for user "joe":
Enter it again:
could not encrypt password: disabled for FIPS
out of memory

because PQencryptPasswordConn was still of the opinion that "out of
memory" is always appropriate to print.

Minor oversight in b69aba745.  Like that one, back-patch to v14.

3 years agoEnhance pg_log_backend_memory_contexts() for auxiliary processes.
Fujii Masao [Tue, 11 Jan 2022 14:19:59 +0000 (23:19 +0900)]
Enhance pg_log_backend_memory_contexts() for auxiliary processes.

Previously pg_log_backend_memory_contexts() could request to
log the memory contexts of backends, but not of auxiliary processes
such as checkpointer. This commit enhances the function so that
it can also send the request to auxiliary processes. It's useful to
look at the memory contexts of those processes for debugging purpose
and better understanding of the memory usage pattern of them.

Note that pg_log_backend_memory_contexts() cannot send the request
to logger or statistics collector. Because this logging request
mechanism is based on shared memory but those processes aren't
connected to that.

Author: Bharath Rupireddy
Reviewed-by: Vignesh C, Kyotaro Horiguchi, Fujii Masao
Discussion: https://postgr.es/m/CALj2ACU1nBzpacOK2q=a65S_4+Oaz_rLTsU1Ri0gf7YUmnmhfQ@mail.gmail.com