users/gsingh/postgres.git
3 years agoFix old-fd issues using global barriers everywhere.
Thomas Munro [Sat, 7 May 2022 03:19:52 +0000 (15:19 +1200)]
Fix old-fd issues using global barriers everywhere.

Commits 4eb21763 and b74e94dc introduced a way to force every backend to
close all relation files, to fix an ancient Windows-only bug.

This commit extends that behavior to all operating systems and adds
a couple of extra barrier points, to fix a totally different class of
bug: the reuse of relfilenodes in scenarios that have no other kind of
cache invalidation to prevent file descriptor mix-ups.

In all releases, data corruption could occur when you moved a database
to another tablespace and then back again.  Despite that, no back-patch
for now as the infrastructure required is too new and invasive.  In
master only, since commit aa010514, it could also happen when using
CREATE DATABASE with a user-supplied OID or via pg_upgrade.

Author: Andres Freund <andres@anarazel.de>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/20220209220004.kb3dgtn2x2k2gtdm%40alap3.anarazel.de

3 years agoRethink PROCSIGNAL_BARRIER_SMGRRELEASE.
Thomas Munro [Sat, 7 May 2022 04:19:42 +0000 (16:19 +1200)]
Rethink PROCSIGNAL_BARRIER_SMGRRELEASE.

With sufficiently bad luck, it was possible for IssuePendingWritebacks()
to reopen a file after we'd processed PROCSIGNAL_BARRIER_SMGRRELEASE and
before the file was unlinked by some other backend.  That left a small
hole in commit 4eb21763's plan to fix all spurious errors from DROP
TABLESPACE and similar on Windows.

Fix by closing md.c's segments, instead of just closing fd.c's
descriptors, and then teaching smgrwriteback() not to open files that
aren't already open.

Reported-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/20220209220004.kb3dgtn2x2k2gtdm%40alap3.anarazel.de

3 years agoFix misleading comments about background worker registration.
Robert Haas [Fri, 6 May 2022 13:24:06 +0000 (09:24 -0400)]
Fix misleading comments about background worker registration.

Since 6bc8ef0b7f1f1df3998745a66e1790e27424aa0c, the maximum number
of backends can't change as background workers are registered, but
these comments still reflect the way things worked prior to that.

Also, per recent discussion, some modules call SetConfigOption()
from _PG_init(). It's not entirely clear to me whether we want to
regard that as a fully supported operation, but since we know it's
a thing that happens, it at least deserves a mention in the comments,
so add that.

Nathan Bossart, reviewed by Anton A. Melnikov

Discussion: http://postgr.es/m/20220419154658.GA2487941@nathanxps13

3 years agopgcrypto: remove questionmark from error message
Daniel Gustafsson [Fri, 6 May 2022 12:41:36 +0000 (14:41 +0200)]
pgcrypto: remove questionmark from error message

The PXE_CIPHER_INIT error is used to report initialization errors, so
appending a questionmark to the error isn't entirely accurate (using a
space before the questionmark doubly so).

Discussion: https://postgr.es/m/C89D932C-501E-4473-9750-638CFCD9095E@yesql.se

3 years agopgcrypto: report init errors as PXE_CIPHER_INIT
Daniel Gustafsson [Fri, 6 May 2022 12:41:33 +0000 (14:41 +0200)]
pgcrypto: report init errors as PXE_CIPHER_INIT

Report OpenSSL errors during initialization as PXE_CIPHER_INIT since
that's just what they were, and not generic unknown errors. This also
removes the last users of the generic error, and thus it can be removed.

Discussion: http://postgr.es/m/C89D932C-501E-4473-9750-638CFCD9095E@yesql.se

3 years agoClear the OpenSSL error queue before cryptohash operations
Daniel Gustafsson [Fri, 6 May 2022 12:41:31 +0000 (14:41 +0200)]
Clear the OpenSSL error queue before cryptohash operations

Setting up an EVP context for ciphers banned under FIPS generate
two OpenSSL errors in the queue, and as we only consume one from
the queue the other is at the head for the next invocation:

  postgres=# select md5('foo');
  ERROR:  could not compute MD5 hash: unsupported
  postgres=# select md5('foo');
  ERROR:  could not compute MD5 hash: initialization error

Clearing the error queue when creating the context ensures that
we don't pull in an error from an earlier operation.

Discussion: https://postgr.es/m/C89D932C-501E-4473-9750-638CFCD9095E@yesql.se

3 years agoFix typo in origin.c
Michael Paquier [Fri, 6 May 2022 11:01:15 +0000 (20:01 +0900)]
Fix typo in origin.c

Introduced in 5aa2350.

Author: Peter Smith
Discussion: https://postgr.es/m/CAHut+PsuWz6_7aCmivNU5FahgQxDUTQtc3+__XnWkBzQcfn43w@mail.gmail.com

3 years agoUpdate SQL features
Peter Eisentraut [Fri, 6 May 2022 07:17:38 +0000 (09:17 +0200)]
Update SQL features

Update a few items that have become supported or mostly supported but
weren't updated at the time.

3 years agoFix some whitespace in documentation markup
Peter Eisentraut [Fri, 6 May 2022 07:14:15 +0000 (09:14 +0200)]
Fix some whitespace in documentation markup

3 years agodoc: Fix typos
Peter Eisentraut [Fri, 6 May 2022 07:07:14 +0000 (09:07 +0200)]
doc: Fix typos

introduced by 222b697ec077047024a96392a2f5cb9b1803ccf7

3 years agoUpdate time zone data files to tzdata release 2022a.
Tom Lane [Thu, 5 May 2022 18:54:53 +0000 (14:54 -0400)]
Update time zone data files to tzdata release 2022a.

DST law changes in Palestine.  Historical corrections for
Chile and Ukraine.

3 years agoFix timing issue in deadlock recovery conflict test.
Andres Freund [Wed, 4 May 2022 19:50:38 +0000 (12:50 -0700)]
Fix timing issue in deadlock recovery conflict test.

Per buildfarm members longfin and skink.

Discussion: https://postgr.es/m/20220413002626.udl7lll7f3o7nre7@alap3.anarazel.de
Backpatch: 10-

3 years agoFix rowcount estimate for SubqueryScan that's under a Gather.
Tom Lane [Wed, 4 May 2022 18:44:40 +0000 (14:44 -0400)]
Fix rowcount estimate for SubqueryScan that's under a Gather.

SubqueryScan was always getting labeled with a rowcount estimate
appropriate for non-parallel cases.  However, nodes that are
underneath a Gather should be treated as processing only one
worker's share of the rows, whether the particular node is explicitly
parallel-aware or not.  Most non-scan-level node types get this
right automatically because they base their rowcount estimate on
that of their input sub-Path(s).  But SubqueryScan didn't do that,
instead using the whole-relation rowcount estimate as if it were
a non-parallel-aware scan node.  If there is a parallel-aware node
below the SubqueryScan, this is wrong, and it results in inflating
the cost estimates for nodes above the SubqueryScan, which can cause
us to not choose a parallel plan, or choose a silly one --- as indeed
is visible in the one regression test whose results change with this
patch.  (Although that plan tree appears to contain no SubqueryScans,
there were some in it before setrefs.c deleted them.)

To fix, use path->subpath->rows not baserel->tuples as the number
of input tuples we'll process.  This requires estimating the quals'
selectivity afresh, which is slightly annoying; but it shouldn't
really add much cost thanks to the caching done in RestrictInfo.

This is pretty clearly a bug fix, but I'll refrain from back-patching
as people might not appreciate plan choices changing in stable branches.
The fact that it took us this long to identify the bug suggests that
it's not a major problem.

Per report from bucoo, though this is not his proposed patch.

Discussion: https://postgr.es/m/202204121457159307248@sohu.com

3 years agoRemove JsonPathSpec typedef
Peter Eisentraut [Wed, 4 May 2022 15:36:31 +0000 (17:36 +0200)]
Remove JsonPathSpec typedef

It doesn't seem very useful, and it's a bit in the way of the planned
node support automation.

Discussion: https://www.postgresql.org/message-id/202204191140.3wsbevfhqmu3@alvherre.pgsql

3 years agoAdd missing enum tag in enum used in nodes
Peter Eisentraut [Wed, 4 May 2022 15:34:22 +0000 (17:34 +0200)]
Add missing enum tag in enum used in nodes

Similar to 983bdc4fac492a99bb8ab5a471ca7437139e5cf6.

Author: Alvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://www.postgresql.org/message-id/202204191140.3wsbevfhqmu3@alvherre.pgsql

3 years agoSimplify configure test
Peter Eisentraut [Wed, 4 May 2022 11:33:59 +0000 (13:33 +0200)]
Simplify configure test

The test for lz4.h used AC_CHECK_HEADERS, but nothing was using the
resulting symbol HAVE_LZ4_H.  Change this to use AC_CHECK_HEADER
instead.  This was probably an oversight, seeing that the nearby
similar tests do this correctly.

3 years agoRename libpq test programs with libpq_ prefix
Daniel Gustafsson [Wed, 4 May 2022 12:15:25 +0000 (14:15 +0200)]
Rename libpq test programs with libpq_ prefix

The testclient and uri-regress programs in the libpq test suite had
quite generic names which didn't convey much meaning. Since they are
installed as part of the MSVC test runs, ensure that their purpose
is a little bit clearer by renaming with a libpq_ prefix. While at
it rename uri-regress to uri_regress to avoid mixing dash and under-
score in the same filename.

Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20220501080706.GA1542365@rfd.leadboat.com

3 years agoFix incorrect format placeholders
Peter Eisentraut [Wed, 4 May 2022 05:57:39 +0000 (07:57 +0200)]
Fix incorrect format placeholders

3 years agoFix possibility of self-deadlock in ResolveRecoveryConflictWithBufferPin().
Andres Freund [Tue, 3 May 2022 01:25:00 +0000 (18:25 -0700)]
Fix possibility of self-deadlock in ResolveRecoveryConflictWithBufferPin().

The tests added in 9f8a050f68d failed nearly reliably on FreeBSD in CI, and
occasionally on the buildfarm. That turns out to be caused not by a bug in the
test, but by a longstanding bug in recovery conflict handling.

The standby timeout handler, used by ResolveRecoveryConflictWithBufferPin(),
executed SendRecoveryConflictWithBufferPin() inside a signal handler. A bad
idea, because the deadlock timeout handler (or a spurious latch set) could
have interrupted ProcWaitForSignal(). If unlucky that could cause a
self-deadlock on ProcArrayLock, if the deadlock check is in
SendRecoveryConflictWithBufferPin()->CancelDBBackends().

To fix, set a flag in StandbyTimeoutHandler(), and check the flag in
ResolveRecoveryConflictWithBufferPin().

Subsequently the recovery conflict tests will be backpatched.

Discussion: https://postgr.es/m/20220413002626.udl7lll7f3o7nre7@alap3.anarazel.de
Backpatch: 10-

3 years agoAdd tests for recovery deadlock conflicts.
Andres Freund [Tue, 3 May 2022 00:19:11 +0000 (17:19 -0700)]
Add tests for recovery deadlock conflicts.

The recovery conflict tests added in 9f8a050f68d surfaced a bug in the
interaction between buffer pin and deadlock recovery conflicts. To make sure
that the bugfix won't break deadlock conflict detection, add a test for that
scenario.

031_recovery_conflict.pl will later be backpatched, with this included.

Discussion: https://postgr.es/m/20220413002626.udl7lll7f3o7nre7@alap3.anarazel.de

3 years agobasebackup_to_shell: Add missing MarkGUCPrefixReserved()
Michael Paquier [Mon, 2 May 2022 11:16:19 +0000 (20:16 +0900)]
basebackup_to_shell: Add missing MarkGUCPrefixReserved()

Oversight in c6306db24, as per a requirement from 88103567.  All the
other modules in the tree, be they in contrib/ or src/test/modules/,
already do that.

Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACUy7q_KwSMda+2SHPSWep32tNUM8cXGRS3=-Vfodo9OUg@mail.gmail.com

3 years agoFix typo in comment.
Etsuro Fujita [Mon, 2 May 2022 07:45:00 +0000 (16:45 +0900)]
Fix typo in comment.

3 years agopg_walinspect: fix case where flush LSN is in the middle of a record.
Jeff Davis [Sat, 30 Apr 2022 15:28:33 +0000 (08:28 -0700)]
pg_walinspect: fix case where flush LSN is in the middle of a record.

Instability in the test for pg_walinspect revealed that
pg_get_wal_records_info_till_end_of_wal(x) would try to decode all the
records with a start LSN earlier than the flush LSN, even though that
might include a partial record at the end of the range. In that case,
read_local_xlog_page_no_wait() would return NULL when it tried to read
past the flush LSN, which would be interpreted as an error by the
caller. That caused a test failure only on a BF animal that had been
restarted recently, but could be expected to happen in the wild quite
easily depending on the alignment of various parameters.

Fix by using private data in read_local_xlog_page_no_wait() to signal
end-of-wal to the caller, so that it can be properly distinguished
from a real error.

Discussion: https://postgr.es/m/Ymd/e5eeZMNAkrXo%40paquier.xyz
Discussion: https://postgr.es/m/111657.1650910309@sss.pgh.pa.us

Authors: Thomas Munro, Bharath Rupireddy.

3 years agoTighten enforcement of variable CONSTANT markings in plpgsql.
Tom Lane [Sat, 30 Apr 2022 15:54:28 +0000 (11:54 -0400)]
Tighten enforcement of variable CONSTANT markings in plpgsql.

I noticed that plpgsql would allow assignment of a new value to a
variable even when that variable is marked CONSTANT, if the variable
is used as an output parameter in CALL or is a refcursor variable
that OPEN assigns a new value to.  Fix these oversights.

In the CALL case, the check has to be done at runtime because we
cannot know at parse time which parameters are OUT parameters.
For OPEN, it seems best to likewise enforce at runtime because
then we needn't throw error if the variable has a nonnull value
(since OPEN will only try to overwrite a null value).

Although this is surely a bug fix, no back-patch: it seems unlikely
that anyone would thank us for breaking formerly-working code in
minor releases.

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

3 years agoClaim SQL standard compliance for SQL/JSON features
Andrew Dunstan [Fri, 29 Apr 2022 13:01:05 +0000 (09:01 -0400)]
Claim SQL standard compliance for SQL/JSON features

Discussion: https://postgr.es/m/d03d809c-d0fb-fd6a-1476-d6dc18ec940e@dunslane.net

3 years agoFix JSON_OBJECTAGG uniquefying bug
Andrew Dunstan [Thu, 28 Apr 2022 19:28:20 +0000 (15:28 -0400)]
Fix JSON_OBJECTAGG uniquefying bug

Commit f4fb45d15c contained a bug in removing items with null values when
unique keys are required, where the leading items that are sorted
contained such values. Fix that and add a test for it.

Discussion: https://postgr.es/m/CAJA4AWQ_XbSmsNbW226UqNyRLJ+wb=iQkQMj77cQyoNkqtf=2Q@mail.gmail.com

3 years agoDisable asynchronous execution if using gating Result nodes.
Etsuro Fujita [Thu, 28 Apr 2022 06:15:00 +0000 (15:15 +0900)]
Disable asynchronous execution if using gating Result nodes.

mark_async_capable_plan(), which is called from create_append_plan() to
determine whether subplans are async-capable, failed to take into
account that the given subplan created from a given subpath might
include a gating Result node if the subpath is a SubqueryScanPath or
ForeignPath, causing a segmentation fault there when the subplan created
from a SubqueryScanPath includes the Result node, or causing
ExecAsyncRequest() to throw an error about an unrecognized node type
when the subplan created from a ForeignPath includes the Result node,
because in the latter case the Result node was unintentionally
considered as async-capable, but we don't currently support executing
Result nodes asynchronously.  Fix by modifying mark_async_capable_plan()
to disable asynchronous execution in such cases.  Also, adjust code in
the ProjectionPath case in mark_async_capable_plan(), for consistency
with other cases, and adjust/improve comments there.

is_async_capable_path() added in commit 27e1f1456, which was rewritten
to mark_async_capable_plan() in a later commit, has the same issue,
causing the error at execution mentioned above, so back-patch to v14
where the aforesaid commit went in.

Per report from Justin Pryzby.

Etsuro Fujita, reviewed by Zhihong Yu and Justin Pryzby.

Discussion: https://postgr.es/m/20220408124338.GK24419%40telsasoft.com

3 years agoRevert recent changes with durable_rename_excl()
Michael Paquier [Thu, 28 Apr 2022 04:08:16 +0000 (13:08 +0900)]
Revert recent changes with durable_rename_excl()

This reverts commits 2c902bb and ccfbd92.  Per buildfarm members
kestrel, rorqual and calliphoridae, the assertions checking that a TLI
history file should not exist when created by a WAL receiver have been
failing, and switching to durable_rename() over durable_rename_excl()
would cause the newest TLI history file to overwrite the existing one.
We need to think harder about such cases, so revert the new logic for
now.

Note that all the failures have been reported in the test
025_stuck_on_old_timeline.

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

3 years agoFix SQL syntax in comment in logical/worker.c
John Naylor [Thu, 28 Apr 2022 02:27:32 +0000 (09:27 +0700)]
Fix SQL syntax in comment in logical/worker.c

Euler Taveira

Disussion: https://www.postgresql.org/message-id/25f95189-eef8-43c4-9d7b-419b651963c8%40www.fastmail.com

3 years agoRemove durable_rename_excl()
Michael Paquier [Thu, 28 Apr 2022 02:10:40 +0000 (11:10 +0900)]
Remove durable_rename_excl()

ccfbd92 has replaced all existing in-core callers of this function in
favor of durable_rename().  durable_rename_excl() is by nature unsafe on
crashes happening at the wrong time, so just remove it.

Author: Nathan Bossart
Reviewed-by: Robert Haas, Kyotaro Horiguchi, Michael Paquier
Discussion: https://postgr.es/m/20220407182954.GA1231544@nathanxps13

3 years agoReplace existing durable_rename_excl() calls with durable_rename()
Michael Paquier [Thu, 28 Apr 2022 01:11:45 +0000 (10:11 +0900)]
Replace existing durable_rename_excl() calls with durable_rename()

durable_rename_excl() attempts to avoid overwriting any existing files
by using link() and unlink(), falling back to rename() on some platforms
(e.g., Windows where link() followed by unlink() is not concurrent-safe,
see 909b449).  Most callers of durable_rename_excl() use it just in case
there is an existing file, but it happens that for all of them we never
expect a target file to exist (WAL segment recycling, creation of
timeline history file and basic_archive).

basic_archive used durable_rename_excl() to avoid overwriting an archive
concurrently created by another server.  Now, there is a stat() call to
avoid overwriting an existing archive a couple of lines above, so note
that this change opens a small TOCTOU window in this module between the
stat() call and durable_rename().

Furthermore, as mentioned in the top comment of durable_rename_excl(),
this routine can result in multiple hard links to the same file and data
corruption, with two or more links to the same file in pg_wal/ if a
crash happens before the unlink() call during WAL recycling.
Specifically, this would produce links to the same file for the current
WAL file and the next one because the half-recycled WAL file was
re-recycled during crash recovery of a follow-up cluster restart.

This change replaces all calls to durable_rename_excl() with
durable_rename().  This removes the protection against accidentally
overwriting an existing file, but some platforms are already living
without it, and all those code paths never expect an existing file (a
couple of assertions are added to check after that, in case).

This is a bug fix, but knowing the unlikeliness of the problem involving
one of more crashes at an exceptionally bad moment, no backpatch is
done.  This could be revisited in the future.

Author: Nathan Bossart
Reviewed-by: Robert Haas, Kyotaro Horiguchi, Michael Paquier
Discussion: https://postgr.es/m/20220407182954.GA1231544@nathanxps13

3 years agoFix incorrect format placeholders
Peter Eisentraut [Wed, 27 Apr 2022 07:49:10 +0000 (09:49 +0200)]
Fix incorrect format placeholders

3 years agoHandle NULL fields in WRITE_INDEX_ARRAY
Peter Eisentraut [Wed, 27 Apr 2022 07:15:09 +0000 (09:15 +0200)]
Handle NULL fields in WRITE_INDEX_ARRAY

Unlike existing WRITE_*_ARRAY macros, WRITE_INDEX_ARRAY needs to
handle the case that the field is NULL.  We already have the
convention to print NULL fields as "<>", so we do that here as well.
There is currently no corresponding read function for this, so reading
this back in is not implemented, but it could be if needed.

Reported-by: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/CAMbWs4-LN%3DbF8f9eU2R94dJtF54DfDvBq%2BovqHnOQqbinYDrUw%40mail.gmail.com

3 years agoFix typo in pg_walinspect.c
Michael Paquier [Tue, 26 Apr 2022 05:24:13 +0000 (14:24 +0900)]
Fix typo in pg_walinspect.c

Spotted while looking at the surroundings, introduced by 2258e76.

3 years agoAdd some isolation tests for CLUSTER
Michael Paquier [Tue, 26 Apr 2022 04:41:17 +0000 (13:41 +0900)]
Add some isolation tests for CLUSTER

This commit adds two isolation tests for CLUSTER, using:
- A normal table, making sure that CLUSTER blocks and completes if the
table is locked by a concurrent session.
- A partitioned table with a partition owned by a different user.  If
the partitioned table is locked by a concurrent session, CLUSTER on the
partitioned table should block.  If the partition owned by a different
user is locked, CLUSTER on its partitioned table should complete and
skip the partition.  3f19e17 has added an early check to ignore such a
partition with a SQL regression test, but this was not checking that
CLUSTER should not block.

Discussion: https://postgr.es/m/YlqveniXn9AI6RFZ@paquier.xyz

3 years agoInhibit mingw CRT's auto-globbing of command line arguments
Andrew Dunstan [Mon, 25 Apr 2022 19:02:13 +0000 (15:02 -0400)]
Inhibit mingw CRT's auto-globbing of command line arguments

For some reason by default the mingw C Runtime takes it upon itself to
expand program arguments that look like shell globbing characters. That
has caused much scratching of heads and mis-attribution of the causes of
some TAP test failures, so stop doing that.

This removes an inconsistency with Windows binaries built with MSVC,
which have no such behaviour.

Per suggestion from Noah Misch.

Backpatch to all live branches.

Discussion: https://postgr.es/m/20220423025927.GA1274057@rfd.leadboat.com

3 years agoDrop unlogged table after test is done
Alvaro Herrera [Mon, 25 Apr 2022 13:48:13 +0000 (15:48 +0200)]
Drop unlogged table after test is done

Another test is constructed on top of regression tests, which does not
work correctly with unlogged tables.  For now, cope with that by making
sure no unlogged table is left behind.

Per buildfarm pink after 4fb5c794e586.

3 years agoCover brin/gin/gist/spgist ambuildempty routines in regression tests
Alvaro Herrera [Mon, 25 Apr 2022 13:00:49 +0000 (15:00 +0200)]
Cover brin/gin/gist/spgist ambuildempty routines in regression tests

Changing some TEMP or permanent tables to UNLOGGED is sufficient to
invoke these ambuildempty routines, which were all not uncovered by any
tests.  These changes do not otherwise affect the test suite.

Author: Amul Sul <sulamul@gmail.com>
Discussion: https://postgr.es/m/CAAJ_b95nneRCLM-=qELEdgCYSk6W_++-C+Q_t+wH3SW-hF50iw@mail.gmail.com

3 years agoAlways pfree strings returned by GetDatabasePath
Alvaro Herrera [Mon, 25 Apr 2022 08:32:13 +0000 (10:32 +0200)]
Always pfree strings returned by GetDatabasePath

Several places didn't do it, and in many cases it didn't matter because
it would be a small allocation in a short-lived context; but other
places may accumulate a few (for example, in CreateDatabaseUsingFileCopy,
one per tablespace).  In most databases this is highly unlikely to be
very serious either, but it seems better to make the code consistent in
case there's future copy-and-paste.

The only case of actual concern seems to be the aforementioned routine,
which is new with commit 9c08aea6a309, so there's no need to backpatch.

As pointed out by Coverity.

3 years agoFix incautious CTE matching in rewriteSearchAndCycle().
Tom Lane [Sat, 23 Apr 2022 16:16:12 +0000 (12:16 -0400)]
Fix incautious CTE matching in rewriteSearchAndCycle().

This function looks for a reference to the recursive WITH CTE,
but it checked only the CTE name not ctelevelsup, so that it could
seize on a lower CTE that happened to have the same name.  This
would result in planner failures later, either weird errors such as
"could not find attribute 2 in subquery targetlist", or crashes
or assertion failures.  The code also merely Assert'ed that it found
a matching entry, which is not guaranteed at all by the parser.

Per bugs #17320 and #17318 from Zhiyong Wu.
Thanks to Kyotaro Horiguchi for investigation.

Discussion: https://postgr.es/m/17320-70e37868182512ab@postgresql.org
Discussion: https://postgr.es/m/17318-2eb65a3a611d2368@postgresql.org

3 years agoTest ALIGNOF_DOUBLE==4 compatibility under ALIGNOF_DOUBLE==8.
Noah Misch [Sat, 23 Apr 2022 03:20:11 +0000 (20:20 -0700)]
Test ALIGNOF_DOUBLE==4 compatibility under ALIGNOF_DOUBLE==8.

Today's test case detected alignment problems only when executing on
AIX.  This change lets popular platforms detect the same problems.

Reviewed by Masahiko Sawada.

Discussion: https://postgr.es/m/20220415072601.GG862547@rfd.leadboat.com

3 years agoRemove some recently-added pg_dump test cases.
Robert Haas [Fri, 22 Apr 2022 20:16:52 +0000 (16:16 -0400)]
Remove some recently-added pg_dump test cases.

Commit d2d35479796c3510e249d6fc72adbd5df918efbf included a pretty
extensive set of test cases, and some of them don't work on all
of our Windows machines. This happens because IPC::Run expands
its arguments as shell globs on a few machines, but doesn't on most
of the buildfarm. It might be good to fix that problem systematically
somehow, but in the meantime, there are enough test cases for this
commit that it seems OK to just remove the ones that are failing.

Discussion: http://postgr.es/m/3a190754-b2b0-d02b-dcfd-4ec1610ffbcb@dunslane.net
Discussion: http://postgr.es/m/CA+TgmoYRGUcFBy6VgN0+Pn4f6Wv=2H0HZLuPHqSy6VC8Ba7vdg@mail.gmail.com

3 years agodoc: Add links to tables
Peter Eisentraut [Fri, 22 Apr 2022 09:19:17 +0000 (11:19 +0200)]
doc: Add links to tables

Formal tables should generally have an xref in the text that points to
them.  Add them here.

3 years agoFix performance regression in tuplesort specializations
David Rowley [Fri, 22 Apr 2022 04:02:15 +0000 (16:02 +1200)]
Fix performance regression in tuplesort specializations

697492434 added 3 new qsort specialization functions aimed to improve the
performance of sorting many of the common pass-by-value data types when
they're the leading or only sort key.

Unfortunately, that has caused a performance regression when sorting
datasets where many of the values being compared were equal.  What was
happening here was that we were falling back to the standard sort
comparison function to handle tiebreaks.  When the two given Datums
compared equally we would incur both the overhead of an indirect function
call to the standard comparer to perform the tiebreak and also the
standard comparer function would go and compare the leading key needlessly
all over again.

Here improve the situation in the 3 new comparison functions.  We now
return 0 directly when the two Datums compare equally and we're performing
a 1-key sort.

Here we don't do anything to help the multi-key sort case where the
leading key uses one of the sort specializations functions.  On testing
this case, even when the leading key's values are all equal, there
appeared to be no performance regression.  Let's leave it up to future
work to optimize that case so that the tiebreak function no longer
re-compares the leading key over again.

Another possible fix for this would have been to add 3 additional sort
specialization functions to handle single-key sorts for these
pass-by-value types.  The reason we didn't do that here is that we may
deem some other sort specialization to be more useful than single-key
sorts.  It may be impractical to have sort specialization functions for
every single combination of what may be useful and it was already decided
that further analysis into which ones are the most useful would be delayed
until the v16 cycle.  Let's not let this regression force our hand into
trying to make that decision for v15.

Author: David Rowley
Reviewed-by: John Naylor
Discussion: https://postgr.es/m/CA+hUKGJRbzaAOUtBUcjF5hLtaSHnJUqXmtiaLEoi53zeWSizeA@mail.gmail.com

3 years agoRemove inadequate assertion check in CTE inlining.
Tom Lane [Thu, 21 Apr 2022 21:58:52 +0000 (17:58 -0400)]
Remove inadequate assertion check in CTE inlining.

inline_cte() expected to find exactly as many references to the
target CTE as its cterefcount indicates.  While that should be
accurate for the tree as emitted by the parser, there are some
optimizations that occur upstream of here that could falsify it,
notably removal of unused subquery output expressions.

Trying to make the accounting 100% accurate seems expensive and
doomed to future breakage.  It's not really worth it, because
all this code is protecting is downstream assumptions that every
referenced CTE has a plan.  Let's convert those assertions to
regular test-and-elog just in case there's some actual problem,
and then drop the failing assertion.

Per report from Tomas Vondra (thanks also to Richard Guo for
analysis).  Back-patch to v12 where the faulty code came in.

Discussion: https://postgr.es/m/29196a1e-ed47-c7ca-9be2-b1c636816183@enterprisedb.com

3 years agoFix missed cases in libpq's error handling.
Tom Lane [Thu, 21 Apr 2022 21:12:49 +0000 (17:12 -0400)]
Fix missed cases in libpq's error handling.

Commit 618c16707 invented an "error_result" flag in PGconn, which
intends to represent the state that we have an error condition and
need to build a PGRES_FATAL_ERROR PGresult from the message text in
conn->errorMessage, but have not yet done so.  (Postponing construction
of the error object simplifies dealing with out-of-memory conditions
and with concatenation of messages for multiple errors.)  For nearly all
purposes, this "virtual" PGresult object should act the same as if it
were already materialized.  But a couple of places in fe-protocol3.c
didn't get that memo, and were only testing conn->result as they used
to, without also checking conn->error_result.

In hopes of reducing the probability of similar mistakes in future,
I invented a pgHavePendingResult() macro that includes both tests.

Per report from Peter Eisentraut.

Discussion: https://postgr.es/m/b52277b9-fa66-b027-4a37-fb8989c73ff8@enterprisedb.com

3 years agoRethink method for assigning OIDs to the template0 and postgres DBs.
Tom Lane [Thu, 21 Apr 2022 20:23:12 +0000 (16:23 -0400)]
Rethink method for assigning OIDs to the template0 and postgres DBs.

Commit aa0105141 assigned fixed OIDs to template0 and postgres
in a very ad-hoc way.  Notably, instead of teaching Catalog.pm
about these OIDs, the unused_oids script was just hacked to
not show them as unused.  That's problematic since, for example,
duplicate_oids wouldn't report any future conflict.  Hence,
invent a macro DECLARE_OID_DEFINING_MACRO() that can be used to
define an OID that is known to Catalog.pm and will participate
in duplicate-detection as well as renumbering by renumber_oids.pl.
(We don't anticipate renumbering these particular OIDs, but we
might as well build out all the Catalog.pm infrastructure while
we're here.)

Another issue is that aa0105141 neglected to touch IsPinnedObject,
with the result that it now claimed template0 and postgres are
pinned.  The right thing to do there seems to be to teach it that
no database is pinned, since in fact DROP DATABASE doesn't check
for pinned-ness (and at least for these cases, that is an
intentional choice).  It's not clear whether this wrong answer
had any visible effect, but perhaps it could have resulted in
erroneous management of dependency entries.

In passing, rename the TemplateDbOid macro to Template1DbOid
to reduce confusion (likely we should have done that way back
when we invented template0, but we didn't), and rename the
OID macros for template0 and postgres to have a similar style.

There are no changes to postgres.bki here, so no need for a
catversion bump.

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

3 years agoStandardize references to Zstandard as <productname>
Alvaro Herrera [Thu, 21 Apr 2022 17:12:21 +0000 (19:12 +0200)]
Standardize references to Zstandard as <productname>

Some places used ZSTD, which isn't widely used anywhere.  Use ZSTD only
to refer to the environment variable; use zstd (all lowercase) to refer
to the utility.

Per complaint from Justin Pryzby.

Discussion: https://postgr.es/m/20220414003301.GT26620@telsasoft.com

3 years agoCREATE PUBLICATION ref: Minor tweaks to row filters
Alvaro Herrera [Thu, 21 Apr 2022 16:57:40 +0000 (18:57 +0200)]
CREATE PUBLICATION ref: Minor tweaks to row filters

Prompted by a complaint from Justin Pryzby.

Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/20220414003301.GT26620@telsasoft.com

3 years agoUse DECLARE_TOAST_WITH_MACRO() to simplify toast-table declarations.
Tom Lane [Thu, 21 Apr 2022 16:02:23 +0000 (12:02 -0400)]
Use DECLARE_TOAST_WITH_MACRO() to simplify toast-table declarations.

This is needed so that renumber_oids.pl can handle renumbering
shared catalog declarations, which need to provide C macros for
the OIDs of the shared toast table and index.  The previous
method of writing a C macro separately was error-prone anyway.

Also teach renumber_oids.pl about DECLARE_UNIQUE_INDEX_PKEY,
as we missed doing when inventing that macro.

There are no changes to postgres.bki here, so no need for a
catversion bump.

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

3 years agopostgres_fdw: Disable batch insert when BEFORE ROW INSERT triggers exist.
Etsuro Fujita [Thu, 21 Apr 2022 06:30:00 +0000 (15:30 +0900)]
postgres_fdw: Disable batch insert when BEFORE ROW INSERT triggers exist.

Previously, we allowed this, but such triggers might query the table to
insert into and act differently if the tuples that have already been
processed and prepared for insertion are not there, so disable it in
such cases.

Back-patch to v14 where batch insert was added.

Discussion: https://postgr.es/m/CAPmGK16_uPqsmgK0-LpLSUk54_BoK13bPrhxhfjSoSTVz414hA%40mail.gmail.com

3 years agovacuumlazy.c: MultiXactIds are MXIDs, not XMIDs.
Peter Geoghegan [Thu, 21 Apr 2022 01:29:02 +0000 (18:29 -0700)]
vacuumlazy.c: MultiXactIds are MXIDs, not XMIDs.

Oversights in commits 0b018fab and f3c15cbe.

3 years agoFix CLUSTER tuplesorts on abbreviated expressions.
Peter Geoghegan [Thu, 21 Apr 2022 00:17:43 +0000 (17:17 -0700)]
Fix CLUSTER tuplesorts on abbreviated expressions.

CLUSTER sort won't use the datum1 SortTuple field when clustering
against an index whose leading key is an expression.  This makes it
unsafe to use the abbreviated keys optimization, which was missed by the
logic that sets up SortSupport state.  Affected tuplesorts output tuples
in a completely bogus order as a result (the wrong SortSupport based
comparator was used for the leading attribute).

This issue is similar to the bug fixed on the master branch by recent
commit cc58eecc5d.  But it's a far older issue, that dates back to the
introduction of the abbreviated keys optimization by commit 4ea51cdfe8.

Backpatch to all supported versions.

Author: Peter Geoghegan <pg@bowt.ie>
Author: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/CA+hUKG+bA+bmwD36_oDxAoLrCwZjVtST2fqe=b4=qZcmU7u89A@mail.gmail.com
Backpatch: 10-

3 years agoDisallow infinite endpoints in generate_series() for timestamps.
Tom Lane [Wed, 20 Apr 2022 22:08:15 +0000 (18:08 -0400)]
Disallow infinite endpoints in generate_series() for timestamps.

Such cases will lead to infinite loops, so they're of no practical
value.  The numeric variant of generate_series() already threw error
for this, so borrow its message wording.

Per report from Richard Wesley.  Back-patch to all supported branches.

Discussion: https://postgr.es/m/91B44E7B-68D5-448F-95C8-B4B3B0F5DEAF@duckdblabs.com

3 years agoAllow db.schema.table patterns, but complain about random garbage.
Robert Haas [Wed, 20 Apr 2022 15:02:35 +0000 (11:02 -0400)]
Allow db.schema.table patterns, but complain about random garbage.

psql, pg_dump, and pg_amcheck share code to process object name
patterns like 'foo*.bar*' to match all tables with names starting in
'bar' that are in schemas starting with 'foo'. Before v14, any number
of extra name parts were silently ignored, so a command line '\d
foo.bar.baz.bletch.quux' was interpreted as '\d bletch.quux'.  In v14,
as a result of commit 2c8726c4b0a496608919d1f78a5abc8c9b6e0868, we
instead treated this as a request for table quux in a schema named
'foo.bar.baz.bletch'. That caused problems for people like Justin
Pryzby who were accustomed to copying strings of the form
db.schema.table from messages generated by PostgreSQL itself and using
them as arguments to \d.

Accordingly, revise things so that if an object name pattern contains
more parts than we're expecting, we throw an error, unless there's
exactly one extra part and it matches the current database name.
That way, thisdb.myschema.mytable is accepted as meaning just
myschema.mytable, but otherdb.myschema.mytable is an error, and so
is some.random.garbage.myschema.mytable.

Mark Dilger, per report from Justin Pryzby and discussion among
various people.

Discussion: https://www.postgresql.org/message-id/20211013165426.GD27491%40telsasoft.com

3 years agoRemove trailing whitespace from *.sgml files.
Tom Lane [Wed, 20 Apr 2022 15:04:28 +0000 (11:04 -0400)]
Remove trailing whitespace from *.sgml files.

Historically we've been lax about this, but seeing that we're not
lax in C files, there doesn't seem to be a good reason to be so
in the documentation.  Remove the existing occurrences (mostly
though not entirely in copied-n-pasted psql output), and modify
.gitattributes so that "git diff --check" will warn about future
cases.

While at it, add *.pm to the set of extensions .gitattributes
knows about, and remove some obsolete entries for files that
we don't have in the tree anymore.

Per followup discussion of commit 5a892c9b1.

Discussion: https://postgr.es/m/E1nfcV1-000kOR-E5@gemulon.postgresql.org

3 years agoFix incorrect format placeholders
Peter Eisentraut [Wed, 20 Apr 2022 14:11:14 +0000 (16:11 +0200)]
Fix incorrect format placeholders

3 years agoset_deparse_plan: Reuse variable to appease Coverity
Alvaro Herrera [Wed, 20 Apr 2022 09:44:08 +0000 (11:44 +0200)]
set_deparse_plan: Reuse variable to appease Coverity

Coverity complains that dpns->outer_plan is deferenced (to obtain
->targetlist) when possibly NULL.  We can avoid this by using
dpns->outer_tlist instead, which was already obtained a few lines up.

The fact that we end up with
  dpns->inner_tlist = dpns->outer_tlist
is a bit suspicious-looking and maybe worthy of more investigation, but
I'll leave that for another day.

Reviewed-by: Michaël Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/202204191345.qerjy3kxi3eb@alvherre.pgsql

3 years agoMove ModifyTableContext->lockmode to UpdateContext
Alvaro Herrera [Wed, 20 Apr 2022 09:18:04 +0000 (11:18 +0200)]
Move ModifyTableContext->lockmode to UpdateContext

Should have been done this way to start with, but I failed to notice
This way we avoid some pointless initialization, and better contains the
variable to exist in the scope where it is really used.

Reviewed-by: Michaël Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/202204191345.qerjy3kxi3eb@alvherre.pgsql

3 years agoExecModifyTable: use context.planSlot instead of planSlot
Alvaro Herrera [Wed, 20 Apr 2022 08:34:58 +0000 (10:34 +0200)]
ExecModifyTable: use context.planSlot instead of planSlot

There's no reason to keep a separate local variable when we have a place
for it elsewhere.  This allows to simplify some code.

Reviewed-by: Michaël Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/202204191345.qerjy3kxi3eb@alvherre.pgsql

3 years agoStabilize streaming tests in test_decoding.
Amit Kapila [Wed, 20 Apr 2022 03:29:55 +0000 (08:59 +0530)]
Stabilize streaming tests in test_decoding.

We have some streaming tests that rely on the size of changes which can
fail if there are additional changes like invalidation messages by
background activity like auto analyze. Avoid such failures by increasing
autovacuum_naptime to a reasonably high value (1d).

Author: Dilip Kumar
Backpatch-through: 14
Discussion: https://postgr.es/m/1958043.1650129119@sss.pgh.pa.us

3 years agoDoc: use "an SQL" consistently rather than "a SQL"
David Rowley [Wed, 20 Apr 2022 03:17:56 +0000 (15:17 +1200)]
Doc: use "an SQL" consistently rather than "a SQL"

Similarly to what was done in 04539e73f, we standardized on SQL being
pronounced "es-que-ell" rather than "sequel" in our documentation.

Two inconsistencies have crept in during the v15 cycle.  The others
existed before but were missed in 04539e73f due to none of the searches
accounting for "SQL" being wrapped in tags.

As with 04539e73f, we don't touch code comments here in order to not
create unnecessary back-patching pain.

Discussion: https://postgr.es/m/CAApHDvpML27UqFXnrYO1MJddsKVMQoiZisPvsAGhKE_tsKXquw%40mail.gmail.com

3 years agoFix breakage in AlterFunction().
Tom Lane [Wed, 20 Apr 2022 03:03:59 +0000 (23:03 -0400)]
Fix breakage in AlterFunction().

An ALTER FUNCTION command that tried to update both the function's
proparallel property and its proconfig list failed to do the former,
because it stored the new proparallel value into a tuple that was
no longer the interesting one.  Carelessness in 7aea8e4f2.

(I did not bother with a regression test, because the only likely
future breakage would be for someone to ignore the comment I added
and add some other field update after the heap_modify_tuple step.
A test using existing function properties could not catch that.)

Per report from Bryn Llewellyn.  Back-patch to all supported branches.

Discussion: https://postgr.es/m/8AC9A37F-99BD-446F-A2F7-B89AD0022774@yugabyte.com

3 years agoRemove duplicated word in comment of basebackup.c
Michael Paquier [Wed, 20 Apr 2022 02:05:34 +0000 (11:05 +0900)]
Remove duplicated word in comment of basebackup.c

Oversight in 39969e2.

Author: Martín Marqués
Discussion: https://postgr.es/m/CABeG9LviA01oHC5h=ksLUuhMyXxmZR_tftRq6q3341CMT=j=4g@mail.gmail.com

3 years agoFix extract epoch from interval calculation
Peter Eisentraut [Tue, 19 Apr 2022 18:38:53 +0000 (20:38 +0200)]
Fix extract epoch from interval calculation

The new numeric code for extract epoch from interval accidentally
truncated the DAYS_PER_YEAR value to an integer, leading to results
that mismatched the floating-point interval_part calculations.

The commit a2da77cdb4661826482ebf2ddba1f953bc74afe4 that introduced
this actually contains the regression test change that this reverts.
I suppose this was missed at the time.

Reported-by: Joseph Koshakow <koshy44@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/CAAvxfHd5n%3D13NYA2q_tUq%3D3%3DSuWU-CufmTf-Ozj%3DfrEgt7pXwQ%40mail.gmail.com

3 years agoFix aggregate logging of pgbench.
Tatsuo Ishii [Tue, 19 Apr 2022 08:04:27 +0000 (17:04 +0900)]
Fix aggregate logging of pgbench.

Remove meaningless "failures" column from the aggregate logging. It
was just a sum of "serialization failures" and "deadlock failures".
Pointed out by Tom Lane. Patch reviewed by Fabien COELHO.

Discussion: https://postgr.es/m/4183048.1649536705%40sss.pgh.pa.us

3 years agoFix the check to limit sync workers.
Amit Kapila [Tue, 19 Apr 2022 03:19:49 +0000 (08:49 +0530)]
Fix the check to limit sync workers.

We don't allow to invoke more sync workers once we have reached the sync
worker limit per subscription. But the check to enforce this also doesn't
allow to launch an apply worker if it gets restarted.

This code was introduced by commit de43897122 but we caught the problem
only with the test added by recent commit c91f71b9dc which started failing
occasionally in the buildfarm.

As per buildfarm.
Diagnosed-by: Amit Kapila, Masahiko Sawada, Tomas Vondra
Author: Amit Kapila
Backpatch-through: 10
Discussion: https://postgr.es/m/CAH2L28vddB_NFdRVpuyRBJEBWjz4BSyTB=_ektNRH8NJ1jf95g@mail.gmail.com
    https://postgr.es/m/f90d2b03-4462-ce95-a524-d91464e797c8@enterprisedb.com

3 years agoAdd missing error handling in pg_md5_hash().
Tom Lane [Tue, 19 Apr 2022 00:04:55 +0000 (20:04 -0400)]
Add missing error handling in pg_md5_hash().

It failed to provide an error string as expected for the
admittedly-unlikely case of OOM in pg_cryptohash_create().
Also, make it initialize *errstr to NULL for success,
as pg_md5_binary() does.

Also add missing comments.  Readers should not have to
reverse-engineer the API spec for a publicly visible routine.

3 years agoAvoid invalid array reference in transformAlterTableStmt().
Tom Lane [Mon, 18 Apr 2022 16:16:45 +0000 (12:16 -0400)]
Avoid invalid array reference in transformAlterTableStmt().

Don't try to look at the attidentity field of system attributes,
because they're not there in the TupleDescAttr array.  Sometimes
this is harmless because we accidentally pick up a zero, but
otherwise we'll report "no owned sequence found" from an attempt
to alter a system attribute.  (It seems possible that a SIGSEGV
could occur, too, though I've not seen it in testing.)

It's not in this function's charter to complain that you can't
alter a system column, so instead just hard-wire an assumption
that system attributes aren't identities.  I didn't bother with
a regression test because the appearance of the bug is very
erratic.

Per bug #17465 from Roman Zharkov.  Back-patch to all supported
branches.  (There's not actually a live bug before v12, because
before that get_attidentity() did the right thing anyway.
But for consistency I changed the test in the older branches too.)

Discussion: https://postgr.es/m/17465-f2a554a6cb5740d3@postgresql.org

3 years agoFix second race condition in 002_archiving.pl with archive_cleanup_command
Michael Paquier [Mon, 18 Apr 2022 04:41:40 +0000 (13:41 +0900)]
Fix second race condition in 002_archiving.pl with archive_cleanup_command

Checking the execution of archive_cleanup_command on a standby requires
a valid checkpoint coming from its primary, but the logic did not check
that the standby replayed up to the point of the checkpoint, causing the
test checking for the execution of archive_cleanup_command to fail.
This race was more visible in slow environments.

Issue introduced in 46dea24, so no backpatch is needed.

Author: Tom Lane
Discussion: https://postgr.es/m/4015413.1649454951@sss.pgh.pa.us

3 years agoAdd additional documentation for row filters.
Amit Kapila [Mon, 18 Apr 2022 03:12:37 +0000 (08:42 +0530)]
Add additional documentation for row filters.

Commit 52e4f0cd47 added a feature to allow specifying row filters for
logical replication of tables. This patch adds detailed documentation on
that feature including examples to make it easier for users to understand.

Author: Peter Smith, Euler Taveira
Reviewed By: Greg Nancarrow, Aleksander Alekseev, Amit Kapila, Ajin Cherian, Alvaro Herrera
Discussion: https://postgr.es/m/CAHut+PtnsBr59=_NvxXp_=S-em0WxyuDOQmSTuHGb4sVhkHffg@mail.gmail.com

3 years agoFix race in TAP test 002_archiving.pl when restoring history file
Michael Paquier [Mon, 18 Apr 2022 02:39:50 +0000 (11:39 +0900)]
Fix race in TAP test 002_archiving.pl when restoring history file

This test, introduced in df86e52, uses a second standby to check that
it is able to remove correctly RECOVERYHISTORY and RECOVERYXLOG at the
end of recovery.  This standby uses the archives of the primary to
restore its contents, with some of the archive's contents coming from
the first standby previously promoted.  In slow environments, it was
possible that the test did not check what it should, as the history file
generated by the promotion of the first standby may not be stored yet on
the archives the second standby feeds on.  So, it could be possible that
the second standby selects an incorrect timeline, without restoring a
history file at all.

This commits adds a wait phase to make sure that the history file
required by the second standby is archived before this cluster is
created.  This relies on poll_query_until() with pg_stat_file() and an
absolute path, something not supported in REL_10_STABLE.

While on it, this adds a new test to check that the history file has
been restored by looking at the logs of the second standby.  This
ensures that a RECOVERYHISTORY, whose removal needs to be checked,
is created in the first place.  This should make the test more robust.

This test has been introduced by df86e52, but it came in light as an
effect of the bug fixed by acf1dd42, where the extra restore_command
calls made the test much slower.

Reported-by: Andres Freund
Discussion: https://postgr.es/m/YlT23IvsXkGuLzFi@paquier.xyz
Backpatch-through: 11

3 years agoHandle compression level in pg_receivewal for LZ4
Michael Paquier [Mon, 18 Apr 2022 01:18:34 +0000 (10:18 +0900)]
Handle compression level in pg_receivewal for LZ4

The new option set of pg_receivewal introduced in 042a923 to control the
compression method makes it now easy to pass down various options,
including the compression level.  The change to be able to do is simple,
and requires one LZ4F_preferences_t fed to LZ4F_compressBegin().

Note that LZ4F_INIT_PREFERENCES could be used to initialize the contents
of LZ4F_preferences_t as required by LZ4, but this is only available
since v1.8.3.  memset()'ing its structure to 0 is enough.

Discussion: https://postgr.es/m/YlPQGNAAa04raObK@paquier.xyz

3 years agoAdd a temp-install prerequisite to src/interfaces/ecpg "checktcp".
Noah Misch [Sun, 17 Apr 2022 00:43:54 +0000 (17:43 -0700)]
Add a temp-install prerequisite to src/interfaces/ecpg "checktcp".

The target failed, tested $PATH binaries, or tested a stale temporary
installation.  Commit c66b438db62748000700c9b90b585e756dd54141 missed
this.  Back-patch to v10 (all supported versions).

3 years agoDon't retry restore_command while reading ahead.
Thomas Munro [Sat, 16 Apr 2022 22:22:03 +0000 (10:22 +1200)]
Don't retry restore_command while reading ahead.

Suppress further attempts to read ahead in the WAL if we run out of
data, until the records already decoded have been replayed.  This
restores the traditional behavior for continuous archive recovery, which
is to retry the failing restore_command only every 5 seconds.  With the
coding in 5dc0418f, we would start retrying every time through the
recovery loop when our WAL decoding window hit the end of the current
segment and we tried to look ahead into a not-yet-available next file.
That was very slow.

Also change the no_readahead_until mechanism to use <= rather than <,
which seems more useful.  Otherwise we'd either get one extra unwanted
retry of restore_command, or we'd need to add 1 to an LSN.

No change in behavior for regular streaming.  That was already limited
by the flushedUpto variable, which won't be updated until we replay what
we have already.

Reported by Andres Freund while analyzing the failure of a TAP test on
build farm animal skink (investigation ongoing but probably due to
otherwise unrelated timing bugs triggered by this slowness magnified by
valgrind).

Discussion: https://postgr.es/m/20220409005910.alw46xqmmgny2sgr%40alap3.anarazel.de

3 years agopgstat: Use correct lock level in pgstat_drop_all_entries().
Andres Freund [Sat, 16 Apr 2022 19:13:31 +0000 (12:13 -0700)]
pgstat: Use correct lock level in pgstat_drop_all_entries().

Previously we didn't, which lead to an assertion failure when resetting
partially loaded statistics. This was encountered on the buildfarm, for
as-of-yet unknown reasons.

Ttighten up a validity check when reading the stats file, verifying 'E'
signals the end of the file (rather than just stopping reading). That's then
used in a test appending to the stats file that crashed before the fix in
pgstat_drop_all_entries().

Reported by buildfarm animals mylodon and kestrel, via Tom Lane.

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

3 years agoFix incorrect logic in HaveRegisteredOrActiveSnapshot().
Tom Lane [Sat, 16 Apr 2022 20:04:50 +0000 (16:04 -0400)]
Fix incorrect logic in HaveRegisteredOrActiveSnapshot().

This function gave the wrong answer when there's more than one
RegisteredSnapshots entry, whether or not any of them is the
CatalogSnapshot.  This leads to assertion failure in some scenarios
involving fetching toasted data using a cursor.  (As per discussion,
I'm dubious that this is the right contract to be enforcing at all;
but it surely doesn't help to be enforcing it incorrectly.)

Fetching toasted data using a cursor is evidently under-tested,
so add a test case too.

Per report from Erik Rijkers.  This is new code, so no need for
back-patch.

Discussion: https://postgr.es/m/dc9dd229-ed30-6c62-4c41-d733ffff776b@xs4all.nl

3 years agoBuild libpq test programs under MSVC
Andrew Dunstan [Sat, 16 Apr 2022 13:35:15 +0000 (09:35 -0400)]
Build libpq test programs under MSVC

This allows the newly added TAP tests to run.

3 years agoFix some trailing whitespace in documentation files
Peter Eisentraut [Sat, 16 Apr 2022 07:05:07 +0000 (09:05 +0200)]
Fix some trailing whitespace in documentation files

3 years agoUse standard timeout, in 010_pg_basebackup.pl.
Noah Misch [Sat, 16 Apr 2022 06:15:38 +0000 (23:15 -0700)]
Use standard timeout, in 010_pg_basebackup.pl.

Per buildfarm member mandrill.  The test is new in v15, so no back-patch.

3 years agoFix multi-table VACUUM VERBOSE accounting.
Peter Geoghegan [Fri, 15 Apr 2022 22:48:39 +0000 (15:48 -0700)]
Fix multi-table VACUUM VERBOSE accounting.

Per-backend global variables like VacuumPageHit are initialized once per
VACUUM command.  This was missed by commit 49c9d9fc, which unified
VACUUM VERBOSE and autovacuum logging.  As a result of that oversight,
incorrect values were shown when multiple relations were processed by a
single VACUUM VERBOSE command.

Relations that happened to be processed later on would show "buffer
usage:" values that incorrectly included buffer accesses made while
processing earlier unrelated relations.  The same accesses were counted
multiple times.

To fix, take initial values for the tracker variables at the start of
heap_vacuum_rel(), and report delta values later on.

3 years agopsql: fix \l display for pre-v15 databases.
Tom Lane [Fri, 15 Apr 2022 22:31:01 +0000 (18:31 -0400)]
psql: fix \l display for pre-v15 databases.

With a pre-v15 server, show NULL for the "ICU Locale" column,
matching what you see in v15 when the database locale isn't ICU.
The previous coding incorrectly repeated datcollate here.

(There's an unfinished discussion about whether to consolidate
these columns in \l output, but in any case we'd want this fix
for \l+ output.)

Euler Taveira, per report from Christoph Berg

Discussion: https://postgr.es/m/YlmIFCqu+TZSW4rB@msg.df7cb.de

3 years agoTighten ComputeXidHorizons' handling of walsenders.
Tom Lane [Fri, 15 Apr 2022 21:50:01 +0000 (17:50 -0400)]
Tighten ComputeXidHorizons' handling of walsenders.

ComputeXidHorizons (nee GetOldestXmin) thought that it could identify
walsenders by checking for proc->databaseId == 0.  Perhaps that was
safe when the code was written, but it's been wrong at least since
autovacuum was invented.  Background processes that aren't connected
to any particular database, such as the autovacuum launcher and
logical replication launcher, look like that too.

This imprecision is harmful because when such a process advertises an
xmin, the result is to hold back dead-tuple cleanup in all databases,
though it'd be sufficient to hold it back in shared catalogs (which
are the only relations such a process can access).  Aside from being
generally inefficient, this has recently been seen to cause regression
test failures in the buildfarm, as a consequence of the logical
replication launcher's startup transaction preventing VACUUM from
marking pages of a user table as all-visible.

We only want that global hold-back effect for the case where a
walsender is advertising a hot standby feedback xmin.  Therefore,
invent a new PGPROC flag that says that a process' xmin should be
considered globally, and check that instead of using the incorrect
databaseId == 0 test.  Currently only a walsender sets that flag,
and only if it is not connected to any particular database.  (This is
for bug-compatibility with the undocumented behavior of the existing
code, namely that feedback sent by a client who has connected to a
particular database would not be applied globally.  I'm not sure this
is a great definition; however, such a client is capable of issuing
plain SQL commands, and I don't think we want xmins advertised for
such commands to be applied globally.  Perhaps this could do with
refinement later.)

While at it, I rewrote the comment in ComputeXidHorizons, and
re-ordered the commented-upon if-tests, to make them match up
for intelligibility's sake.

This is arguably a back-patchable bug fix, but given the lack of
complaints I think it prudent to let it age awhile in HEAD first.

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

3 years agoVACUUM VERBOSE: Show dead items for an empty table.
Peter Geoghegan [Fri, 15 Apr 2022 21:20:56 +0000 (14:20 -0700)]
VACUUM VERBOSE: Show dead items for an empty table.

Be consistent about the lines that VACUUM VERBOSE outputs by including
an "index scan not needed: " line for completely empty tables. This
makes the output more readable, especially with multiple distinct VACUUM
operations processed by the same VACUUM command.  It's also more
consistent; even empty tables can use the failsafe, which wasn't
reported in the standard way until now.

Follow-up to commit 6e20f460, which taught VACUUM VERBOSE to be more
consistent about reporting on scanned pages with empty tables.

3 years agoAdjust VACUUM's removable cutoff log message.
Peter Geoghegan [Fri, 15 Apr 2022 20:21:43 +0000 (13:21 -0700)]
Adjust VACUUM's removable cutoff log message.

The age of OldestXmin (a.k.a. "removable cutoff") when VACUUM ends often
indicates the approximate number of XIDs consumed while VACUUM ran.
However, there is at least one important exception: the cutoff could be
held back by a snapshot that was acquired before our VACUUM even began.
Successive VACUUM operations may even use exactly the same old cutoff in
extreme cases involving long held snapshots.

The log messages that described how removable cutoff aged (which were
added by commit 872770fd) created the impression that we were reporting
on how VACUUM's usable cutoff advanced while VACUUM ran, which was
misleading in these extreme cases.  Fix by using a more general wording.

Per gripe from Tom Lane.

In passing, relocate related instrumentation code for clarity.

Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/1643035.1650035653@sss.pgh.pa.us

3 years agoRevert "Temporarily add some probes of tenk1's relallvisible in create_index.sql."
Tom Lane [Fri, 15 Apr 2022 17:29:39 +0000 (13:29 -0400)]
Revert "Temporarily add some probes of tenk1's relallvisible in create_index.sql."

This reverts commit 5bb2b6abc8d6cf120a814317816e4384bcbb9c1e.
Not needed anymore.

3 years agoSmall cleanups in SQL/JSON code
Andrew Dunstan [Fri, 15 Apr 2022 11:47:12 +0000 (07:47 -0400)]
Small cleanups in SQL/JSON code

These are to keep Coverity happy. In one case remove a redundant NULL
check, and in another explicitly ignore a function result that is already
known.

3 years agopgstat: set timestamps of fixed-numbered stats after a crash.
Andres Freund [Fri, 15 Apr 2022 00:40:25 +0000 (17:40 -0700)]
pgstat: set timestamps of fixed-numbered stats after a crash.

When not loading stats at startup (i.e. pgstat_discard_stats() getting
called), reset timestamps of fixed numbered stats would be left at
0. Oversight in 5891c7a8ed8.

Instead use pgstat_reset_after_failure() and add tests verifying that
fixed-numbered reset timestamps are set appropriately.

Reported-By: "David G. Johnston" <david.g.johnston@gmail.com>
Discussion: https://postgr.es/m/CAKFQuwamFuaQHKdhcMt4Gbw5+Hca2UE741B8gOOXoA=TtAd2Yw@mail.gmail.com

3 years agoHave CLUSTER ignore partitions not owned by caller
Alvaro Herrera [Thu, 14 Apr 2022 20:11:06 +0000 (22:11 +0200)]
Have CLUSTER ignore partitions not owned by caller

If a partitioned table has partitions owned by roles other than the
owner of the partitioned table, don't include them in the to-be-
clustered list.  This is similar to what VACUUM FULL does (except we do
it sooner, because there is no reason to postpone it).  Add a simple
test to verify that only owned partitions are clustered.

While at it, change memory context switch-and-back to occur once per
partition instead of outside of the loop.

Author: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Zhihong Yu <zyu@yugabyte.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/20220411140609.GF26620@telsasoft.com

3 years agoReword text on ROW SHARE lock as acquired by SELECT FOR <lock>
Alvaro Herrera [Thu, 14 Apr 2022 19:52:20 +0000 (21:52 +0200)]
Reword text on ROW SHARE lock as acquired by SELECT FOR <lock>

It was missing lock levels FOR KEY SHARE and FOR NO KEY EXCLUSIVE; but
also SELECT FOR UPDATE is not a command separate from SELECT, as the
original text implied.  It is clearer to state that FOR <lock strength>
is an option of regular SELECT.

Per suggestion from Joey Bodoia <jbodoia21@cmc.edu>

Reviewed-by: Joey Bodoia <jbodoia21@cmc.edu> (offlist)
Reviewed-by: Erikjan Rijkers <er@xs4all.nl>
Discussion: https://postgr.es/m/164908765512.682.17348032020747341013@wrigleys.postgresql.org

3 years agoTemporarily add some probes of tenk1's relallvisible in create_index.sql.
Tom Lane [Thu, 14 Apr 2022 16:14:01 +0000 (12:14 -0400)]
Temporarily add some probes of tenk1's relallvisible in create_index.sql.

This is to gather some more evidence about why buildfarm member wrasse
is failing.  We should revert it (or at least scale it way back) once
that's resolved.

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

3 years agoImprove a couple of sql/json error messages
Andrew Dunstan [Thu, 14 Apr 2022 14:26:29 +0000 (10:26 -0400)]
Improve a couple of sql/json error messages

Fix the grammar in two, and add a hint to one.

3 years agoFix transformJsonBehavior
Andrew Dunstan [Thu, 14 Apr 2022 12:57:09 +0000 (08:57 -0400)]
Fix transformJsonBehavior

Commit 1a36bc9dba8 conained some logic that was a little opaque and
could have involved a NULL dereference, as complained about by Coverity.
Make the logic more transparent and in doing so avoid the NULL
dereference.

3 years agopageinspect: Fix handling of all-zero pages
Michael Paquier [Thu, 14 Apr 2022 06:08:03 +0000 (15:08 +0900)]
pageinspect: Fix handling of all-zero pages

Getting from get_raw_page() an all-zero page is considered as a valid
case by the buffer manager and it can happen for example when finding a
corrupted page with zero_damaged_pages enabled (using zero_damaged_pages
to look at corrupted pages happens), or after a crash when a relation
file is extended before any WAL for its new data is generated (before a
vacuum or autovacuum job comes in to do some cleanup).

However, all the functions of pageinspect, as of the index AMs (except
hash that has its own idea of new pages), heap, the FSM or the page
header have never worked with all-zero pages, causing various crashes
when going through the page internals.

This commit changes all the pageinspect functions to be compliant with
all-zero pages, where the choice is made to return NULL or no rows for
SRFs when finding a new page.  get_raw_page() still works the same way,
returning a batch of zeros in the bytea of the page retrieved.  A hard
error could be used but NULL, while more invasive, is useful when
scanning relation files in full to get a batch of results for a single
relation in one query.  Tests are added for all the code paths
impacted.

Reported-by: Daria Lepikhova
Author: Michael Paquier
Discussion: https://postgr.es/m/561e187b-3549-c8d5-03f5-525c14e65bd0@postgrespro.ru
Backpatch-through: 10

3 years agoAdd missing spaces after single-line comments
David Rowley [Wed, 13 Apr 2022 21:28:56 +0000 (09:28 +1200)]
Add missing spaces after single-line comments

Only 1 of 3 of these changes appear to be handled by pgindent. That change
is new to v15.  The remaining two appear to be left alone by pgindent. The
exact reason for that is not 100% clear to me.  It seems related to the
fact that it's a line that contains *only* a single line comment and no
actual code.  It does not seem worth investigating this in too much
detail.  In any case, these do not conform to our usual practices, so fix
them.

Author: Justin Pryzby
Discussion: https://postgr.es/m/20220411020336.GB26620@telsasoft.com

3 years agoDocs: fix some spelling mistakes and also do some wordsmithing
David Rowley [Wed, 13 Apr 2022 21:16:05 +0000 (09:16 +1200)]
Docs: fix some spelling mistakes and also do some wordsmithing

All except one of these are new to v15.  Only one of the wordsmithing
changes appears in older versions. The wordsmithing improvement does not
seem significant enough to warrant backpatching.

Author: Justin Pryzby
Discussion: https://postgr.es/m/20220411020336.GB26620@telsasoft.com

3 years agoFix case sensitivity in psql's tab completion for GUC names.
Tom Lane [Wed, 13 Apr 2022 20:26:34 +0000 (16:26 -0400)]
Fix case sensitivity in psql's tab completion for GUC names.

Input for these should be case-insensitive, but was not completely
so.  Comparing to the similar queries for timezone names, I realized
that we'd missed forcing the comparison pattern to lower-case.
With that, it behaves as I expect.

While here, flatten the sub-selects in these queries; I don't
find that those add any readability.

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

3 years agoFurther tweak the default behavior of psql's \dconfig.
Tom Lane [Wed, 13 Apr 2022 19:03:58 +0000 (15:03 -0400)]
Further tweak the default behavior of psql's \dconfig.

Define "parameters with non-default settings" as being those that
not only have pg_settings.source different from 'default', but
also have a current value different from the hard-wired boot_val.
Adding the latter restriction removes a number of not-very-interesting
cases where the active setting is chosen by initdb but in practice
tends to be the same all the time.

Per discussion with Jonathan Katz.

Discussion: https://postgr.es/m/YlFQLzlPi4QD0wSi@msg.df7cb.de

3 years agoPrevent access to no-longer-pinned buffer in heapam_tuple_lock().
Tom Lane [Wed, 13 Apr 2022 17:35:02 +0000 (13:35 -0400)]
Prevent access to no-longer-pinned buffer in heapam_tuple_lock().

heap_fetch() used to have a "keep_buf" parameter that told it to return
ownership of the buffer pin to the caller after finding that the
requested tuple TID exists but is invisible to the specified snapshot.
This was thoughtlessly removed in commit 5db6df0c0, which broke
heapam_tuple_lock() (formerly EvalPlanQualFetch) because that function
needs to do more accesses to the tuple even if it's invisible.  The net
effect is that we would continue to touch the page for a microsecond or
two after releasing pin on the buffer.  Usually no harm would result;
but if a different session decided to defragment the page concurrently,
we could see garbage data and mistakenly conclude that there's no newer
tuple version to chain up to.  (It's hard to say whether this has
happened in the field.  The bug was actually found thanks to a later
change that allowed valgrind to detect accesses to non-pinned buffers.)

The most reasonable way to fix this is to reintroduce keep_buf,
although I made it behave slightly differently: buffer ownership
is passed back only if there is a valid tuple at the requested TID.
In HEAD, we can just add the parameter back to heap_fetch().
To avoid an API break in the back branches, introduce an additional
function heap_fetch_extended() in those branches.

In HEAD there is an additional, less obvious API change: tuple->t_data
will be set to NULL in all cases where buffer ownership is not returned,
in particular when the tuple exists but fails the time qual (and
!keep_buf).  This is to defend against any other callers attempting to
access non-pinned buffers.  We concluded that making that change in back
branches would be more likely to introduce problems than cure any.

In passing, remove a comment about heap_fetch that was obsoleted by
9a8ee1dc6.

Per bug #17462 from Daniil Anisimov.  Back-patch to v12 where the bug
was introduced.

Discussion: https://postgr.es/m/17462-9c98a0f00df9bd36@postgresql.org

3 years agoRemove extraneous blank lines before block-closing braces
Alvaro Herrera [Wed, 13 Apr 2022 17:14:20 +0000 (19:14 +0200)]
Remove extraneous blank lines before block-closing braces

These are useless and distracting.  We wouldn't have written the code
with them to begin with, so there's no reason to keep them.

Author: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/20220411020336.GB26620@telsasoft.com
Discussion: https://postgr.es/m/attachment/133167/0016-Extraneous-blank-lines.patch