postgresql.git
3 years agopg_upgrade: improve docs about extension upgrades
Bruce Momjian [Tue, 3 Aug 2021 15:27:33 +0000 (11:27 -0400)]
pg_upgrade:  improve docs about extension upgrades

The previous wording was unclear about the steps needed to upgrade
extensions, and how to update them after pg_upgrade.

Reported-by: Dave Cramer
Discussion: https://postgr.es/m/CADK3HHKawwbOcGwMGnDuAf3-U8YfvTcS8jqDv3UM=niijs3MMA@mail.gmail.com

Backpatch-through: 9.6

3 years agodoc: mention inheritance's tableoid can be used in partitioning
Bruce Momjian [Tue, 3 Aug 2021 15:11:51 +0000 (11:11 -0400)]
doc:  mention inheritance's tableoid can be used in partitioning

Previously tableoid was not mentioned in the partition doc section.  We
only had a link to the "all the normal rules" of inheritance section.

Reported-by: michal.palenik@freemap.sk
Discussion: https://postgr.es/m/162627031219.693.11508199541771263335@wrigleys.postgresql.org

Backpatch-through: 10

3 years agodoc: add example of using pg_dump with GNU split and gzip
Bruce Momjian [Tue, 3 Aug 2021 14:57:32 +0000 (10:57 -0400)]
doc:  add example of using pg_dump with GNU split and gzip

This is only possible with GNU split, not other versions like BSD split.

Reported-by: jim@jdoherty.net
Discussion: https://postgr.es/m/162653459215.701.6323855956817776386@wrigleys.postgresql.org

Backpatch-through: 9.6

3 years agoFurther simplify a bit of logic in StartupXLOG().
Thomas Munro [Tue, 3 Aug 2021 02:11:55 +0000 (14:11 +1200)]
Further simplify a bit of logic in StartupXLOG().

Commit 7ff23c6d277d1d90478a51f0dd81414d343f3850 left us with two
identical cases. Collapse them.

Author: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKGJ8NRsqgkZEnsnRc2MFROBV-jCnacbYvtpptK2A9YYp9Q%40mail.gmail.com

3 years agoAllow ordered partition scans in more cases
David Rowley [Tue, 3 Aug 2021 00:25:52 +0000 (12:25 +1200)]
Allow ordered partition scans in more cases

959d00e9d added the ability to make use of an Append node instead of a
MergeAppend when we wanted to perform a scan of a partitioned table and
the required sort order was the same as the partitioned keys and the
partitioned table was defined in such a way that earlier partitions were
guaranteed to only contain lower-order values than later partitions.
However, previously we didn't allow these ordered partition scans for
LIST partitioned table when there were any partitions that allowed
multiple Datums.  This was a very cheap check to make and we could likely
have done a little better by checking if there were interleaved
partitions, but at the time we didn't have visibility about which
partitions were pruned, so we still may have disallowed cases where all
interleaved partitions were pruned.

Since 475dbd0b7, we now have knowledge of pruned partitions, we can do a
much better job inside partitions_are_ordered().

Here we pass which partitions survived partition pruning into
partitions_are_ordered() and, for LIST partitioning, have it check to see
if any live partitions exist that are also in the new "interleaved_parts"
field defined in PartitionBoundInfo.

For RANGE partitioning we can relax the code which caused the partitions
to be unordered if a DEFAULT partition existed.  Since we now know which
partitions were pruned, partitions_are_ordered() now returns true when the
DEFAULT partition was pruned.

Reviewed-by: Amit Langote, Zhihong Yu
Discussion: https://postgr.es/m/CAApHDvrdoN_sXU52i=QDXe2k3WAo=EVry29r2+Tq2WYcn2xhEA@mail.gmail.com

3 years agoTrack a Bitmapset of non-pruned partitions in RelOptInfo
David Rowley [Mon, 2 Aug 2021 23:47:24 +0000 (11:47 +1200)]
Track a Bitmapset of non-pruned partitions in RelOptInfo

For partitioned tables with large numbers of partitions where queries are
able to prune all but a very small number of partitions, the time spent in
the planner looping over RelOptInfo.part_rels checking for non-NULL
RelOptInfos could become a large portion of the overall planning time.

Here we add a Bitmapset that records the non-pruned partitions.  This
allows us to more efficiently skip the pruned partitions by looping over
the Bitmapset.

This will cause a very slight slow down in cases where no or not many
partitions could be pruned, however, those cases are already slow to plan
anyway and the overhead of looping over the Bitmapset would be
unmeasurable when compared with the other tasks such as path creation for
a large number of partitions.

Reviewed-by: Amit Langote, Zhihong Yu
Discussion: https://postgr.es/m/CAApHDvqnPx6JnUuPwaf5ao38zczrAb9mxt9gj4U1EKFfd4AqLA@mail.gmail.com

3 years agoDoc: minor improvements for logical replication protocol documentation.
Tom Lane [Mon, 2 Aug 2021 15:32:17 +0000 (11:32 -0400)]
Doc: minor improvements for logical replication protocol documentation.

Where appropriate, annotate message field data types with the backend
code's name for the data type, eg XLogRecPtr or TimestampTz.  Previously
we just said "Int64" which didn't provide as much info.

Also clarify references to object OIDs, and make use of the existing
convention to denote the value of a field that must have a fixed value.

Vignesh C, reviewed by Peter Smith and Euler Taveira.

Discussion: https://postgr.es/m/CALDaNm0+fatx57KFcBopUZWQpH_tz3WKKfm-_eiTwcXui5BnhQ@mail.gmail.com

3 years agoRun checkpointer and bgwriter in crash recovery.
Thomas Munro [Mon, 2 Aug 2021 05:32:20 +0000 (17:32 +1200)]
Run checkpointer and bgwriter in crash recovery.

Start up the checkpointer and bgwriter during crash recovery (except in
--single mode), as we do for replication.  This wasn't done back in
commit cdd46c76 out of caution.  Now it seems like a better idea to make
the environment as similar as possible in both cases.  There may also be
some performance advantages.

Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Reviewed-by: Aleksander Alekseev <aleksander@timescale.com>
Tested-by: Jakub Wartak <Jakub.Wartak@tomtom.com>
Discussion: https://postgr.es/m/CA%2BhUKGJ8NRsqgkZEnsnRc2MFROBV-jCnacbYvtpptK2A9YYp9Q%40mail.gmail.com

3 years agoRemove misplaced comment from AuxiliaryProcessMain().
Andres Freund [Mon, 2 Aug 2021 04:10:11 +0000 (21:10 -0700)]
Remove misplaced comment from AuxiliaryProcessMain().

The comment didn't make sense anymore since at least 626eb021988. As it didn't
actually explain anything anyway, just remove it.

Author: Andres Freund <andres@anarazel.de>

3 years agoFix oversight in commit 1ec7fca8592178281cd5cdada0f27a340fb813fc.
Etsuro Fujita [Mon, 2 Aug 2021 03:45:00 +0000 (12:45 +0900)]
Fix oversight in commit 1ec7fca8592178281cd5cdada0f27a340fb813fc.

I failed to account for the possibility that when
ExecAppendAsyncEventWait() notifies multiple async-capable nodes using
postgres_fdw, a preceding node might invoke process_pending_request() to
process a pending asynchronous request made by a succeeding node.  In
that case the succeeding node should produce a tuple to return to the
parent Append node from tuples fetched by process_pending_request() when
notified.  Repair.

Per buildfarm via Michael Paquier.  Back-patch to v14, like the previous
commit.

Thanks to Tom Lane for testing.

Discussion: https://postgr.es/m/YQP0UPT8KmPiHTMs%40paquier.xyz

3 years agoFix test failure in 021_twophase.pl.
Amit Kapila [Mon, 2 Aug 2021 02:59:26 +0000 (08:29 +0530)]
Fix test failure in 021_twophase.pl.

The test is expecting two prepared transactions corresponding to two
subscriptions but it waits to catch up for just one subscription. Fix it
by allowing to wait for both subscriptions.

Reported-by: Michael Paquier, as per buildfarm
Author: Ajin Cherian
Reviewed-By: Amit Kapila, Vignesh C, Peter Smith
Discussion: https://postgr.es/m/CAA4eK1+_0iNQ8Z=KVTjmmAqNX-hyv+1+fnZ-Yx8CVP=uAcekqw@mail.gmail.com

3 years agoSilence perl warning about uninitialized value
Andrew Dunstan [Sun, 1 Aug 2021 17:03:15 +0000 (13:03 -0400)]
Silence perl warning about uninitialized value

3 years agoDoc: alphabetize the regexp_foo() function descriptions in 9.7.3.
Tom Lane [Sat, 31 Jul 2021 19:35:27 +0000 (15:35 -0400)]
Doc: alphabetize the regexp_foo() function descriptions in 9.7.3.

For no visible reason (other than historical accident no doubt),
regexp_replace() was out of order.  Re-order to match the way
that these functions are listed in 9.4.  (That means substring()
remains first, because it's SQL-standard and the rest aren't.)
I've not touched the text other than to move it.  This is just
to reduce confusion in the diffs for upcoming additions.

3 years agoUse elog, not Assert, to report failure to provide an outer snapshot.
Tom Lane [Sat, 31 Jul 2021 15:50:14 +0000 (11:50 -0400)]
Use elog, not Assert, to report failure to provide an outer snapshot.

As of commit 84f5c2908, executing SQL commands (via SPI or otherwise)
requires having either an active Portal, or a caller-established
active snapshot.  We were simply Assert'ing that that's the case.
But we've now had a couple different reports of people testing
extensions that didn't meet this requirement, and were confused by
the resulting crash.  Let's convert the Assert to a test-and-elog,
in hopes of making the issue clearer for extension authors.

Per gripes from Liu Huailing and RekGRpth.  Back-patch to v11,
like the prior commit.

Discussion: https://postgr.es/m/OSZPR01MB6215671E3C5956A034A080DFBEEC9@OSZPR01MB6215.jpnprd01.prod.outlook.com
Discussion: https://postgr.es/m/17035-14607d308ac8643c@postgresql.org

3 years agoRemove redundant setting of pg_attribute.attcompression
John Naylor [Sat, 31 Jul 2021 11:25:27 +0000 (07:25 -0400)]
Remove redundant setting of pg_attribute.attcompression

Since e6241d8e0, no attribute needs a non-default value of this during
initdb, so let the usual machinery for defaults take care of it.

3 years agoFix corner-case errors and loss of precision in numeric_power().
Dean Rasheed [Sat, 31 Jul 2021 10:21:44 +0000 (11:21 +0100)]
Fix corner-case errors and loss of precision in numeric_power().

This fixes a couple of related problems that arise when raising
numbers to very large powers.

Firstly, when raising a negative number to a very large integer power,
the result should be well-defined, but the previous code would only
cope if the exponent was small enough to go through power_var_int().
Otherwise it would throw an internal error, attempting to take the
logarithm of a negative number. Fix this by adding suitable handling
to the general case in power_var() to cope with negative bases,
checking for integer powers there.

Next, when raising a (positive or negative) number whose absolute
value is slightly less than 1 to a very large power, the result should
approach zero as the power is increased. However, in some cases, for
sufficiently large powers, this would lose all precision and return 1
instead of 0. This was due to the way that the local_rscale was being
calculated for the final full-precision calculation:

  local_rscale = rscale + (int) val - ln_dweight + 8

The first two terms on the right hand side are meant to give the
number of significant digits required in the result ("val" being the
estimated result weight). However, this failed to account for the fact
that rscale is clipped to a maximum of NUMERIC_MAX_DISPLAY_SCALE
(1000), and the result weight might be less then -1000, causing their
sum to be negative, leading to a loss of precision. Fix this by
forcing the number of significant digits calculated to be nonnegative.
It's OK for it to be zero (when the result weight is less than -1000),
since the local_rscale value then includes a few extra digits to
ensure an accurate result.

Finally, add additional underflow checks to exp_var() and power_var(),
so that they consistently return zero for cases like this where the
result is indistinguishable from zero. Some paths through this code
already returned zero in such cases, but others were throwing overflow
errors.

Dean Rasheed, reviewed by Yugo Nagata.

Discussion: http://postgr.es/m/CAEZATCW6Dvq7+3wN3tt5jLj-FyOcUgT5xNoOqce5=6Su0bCR0w@mail.gmail.com

3 years agoMove InRecovery and standbyState global vars to xlogutils.c.
Heikki Linnakangas [Sat, 31 Jul 2021 06:50:26 +0000 (09:50 +0300)]
Move InRecovery and standbyState global vars to xlogutils.c.

They are used in code that runs both during normal operation and during
WAL replay, and needs to behave differently during replay. Move them to
xlogutils.c, because that's where we have other helper functions used by
redo routines.

Reviewed-by: Andres Freund
Discussion: https://www.postgresql.org/message-id/b3b71061-4919-e882-4857-27e370ab134a%40iki.fi

3 years agoExtract code to describe recovery stop reason to a function.
Heikki Linnakangas [Sat, 31 Jul 2021 06:49:30 +0000 (09:49 +0300)]
Extract code to describe recovery stop reason to a function.

StartupXLOG() is very long, this makes it a little bit more readable.

Reviewed-by: Andres Freund
Discussion: https://www.postgresql.org/message-id/b3b71061-4919-e882-4857-27e370ab134a%40iki.fi

3 years agoRemove unnecessary 'restoredFromArchive' global variable.
Heikki Linnakangas [Sat, 31 Jul 2021 06:38:32 +0000 (09:38 +0300)]
Remove unnecessary 'restoredFromArchive' global variable.

It might've been useful for debugging purposes, but meh. There's
'readSource' which does almost the same thing.

Reviewed-by: Andres Freund
Discussion: https://www.postgresql.org/message-id/b3b71061-4919-e882-4857-27e370ab134a%40iki.fi

3 years agoDon't use O_SYNC or similar when opening signal file to fsync it.
Heikki Linnakangas [Sat, 31 Jul 2021 06:36:11 +0000 (09:36 +0300)]
Don't use O_SYNC or similar when opening signal file to fsync it.

No need to use get_sync_bit() when we're calling pg_fsync() on the file.
We're not writing to the files, so it doesn't make any difference in
practice, but seems less surprising this way.

Reviewed-by: Andres Freund
Discussion: https://www.postgresql.org/message-id/b3b71061-4919-e882-4857-27e370ab134a%40iki.fi

3 years agoEnable TAP tests of pg_receivewal for ZLIB on Windows, take three
Michael Paquier [Sat, 31 Jul 2021 01:13:15 +0000 (10:13 +0900)]
Enable TAP tests of pg_receivewal for ZLIB on Windows, take three

This reverts commit 6a2c532.  fairywren and bowerbird failed those tests
because of incorrect versions of ZLIB linked to, causing errors like
SIGBREAKs that stopped buildfarm runs or EACCES failures when writing
compressed WAL segments.

Andrew Dunstan has done all the investigation here, so he deserves all
the credit for being able to enable those tests on Windows.

Discussion: https://postgr.es/m/9040d5ed-6462-66a4-07ac-2923785ae563@dunslane.net

3 years agoImprove documentation for START_REPLICATION ... LOGICAL.
Jeff Davis [Fri, 30 Jul 2021 21:59:19 +0000 (14:59 -0700)]
Improve documentation for START_REPLICATION ... LOGICAL.

The starting point may not be exactly what the client requested; it
may be at the slot's confirmed_flush_lsn.

Also, upgrade the message from DEBUG1 to LOG when this happens.

Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/c5c861d576f2511732f8002c76245da587110b1c.camel%40j-davis.com

3 years agoFix range check in ECPG numeric to int conversion
John Naylor [Fri, 30 Jul 2021 17:50:23 +0000 (13:50 -0400)]
Fix range check in ECPG numeric to int conversion

The previous coding guarded against -INT_MAX instead of INT_MIN,
leading to -2147483648 being rejected as out of range.

Per bug #17128 from Kevin Sweet

Discussion: https://www.postgresql.org/message-id/flat/17128-55a8a879727a3e3a%40postgresql.org
Reviewed-by: Tom Lane
Backpatch to all supported branches

3 years agoDoc: add a glossary entry for "domain".
Tom Lane [Fri, 30 Jul 2021 18:50:21 +0000 (14:50 -0400)]
Doc: add a glossary entry for "domain".

Anton Voloshin and Jürgen Purtz, reviewed by Laurenz Albe

Discussion: https://postgr.es/m/2ea65bdf-1380-f088-02bd-ff1a31ed265c@postgrespro.ru

3 years agoIn postgres_fdw, allow CASE expressions to be pushed to the remote server.
Tom Lane [Fri, 30 Jul 2021 17:39:48 +0000 (13:39 -0400)]
In postgres_fdw, allow CASE expressions to be pushed to the remote server.

This is simple enough except for the need to check whether CaseTestExpr
nodes have a collation that is not derived from a remote Var.  For that,
examine the CASE's "arg" expression and then pass that info down into the
recursive examination of the WHEN expressions.

Alexander Pyhalov, reviewed by Gilles Darold and myself

Discussion: https://postgr.es/m/fda09032e90d85d9b726a41e03f9097f@postgrespro.ru

3 years agoRemove unnecessary call to ReadCheckpointRecord().
Robert Haas [Fri, 30 Jul 2021 12:33:33 +0000 (08:33 -0400)]
Remove unnecessary call to ReadCheckpointRecord().

It should always be the case that the last checkpoint record is still
readable, because otherwise, a crash would leave us in a situation
from which we can't recover. Therefore the test removed by this patch
should always succeed. For it to fail, either there has to be a serious
bug in the code someplace, or the user has to be manually modifying
pg_wal while crash recovery is running. If it's the first one, we
should fix the bug. If it's the second one, they should stop, or
anyway they're doing so at their own risk. In neither case does
a full checkpoint instead of an end-of-recovery record seem like a
clear winner. Furthermore, rarely-taken code paths are particularly
vulnerable to bugs, so let's simplify by getting rid of this one.

Discussion: http://postgr.es/m/CA+TgmoYmw==TOJ6EzYb_vcjyS09NkzrVKSyBKUUyo1zBEaJASA@mail.gmail.com

3 years agoUse --no-loop for new calls of pg_receivewal --endpos in TAP tests
Michael Paquier [Fri, 30 Jul 2021 12:28:03 +0000 (21:28 +0900)]
Use --no-loop for new calls of pg_receivewal --endpos in TAP tests

Those tests are not designed to fail, but if they do, like on some cases
for Windows because of ZLIB (?), they could remain stuck.  Using
--no-loop makes the test fail immediately.  The oldest test with
--endpos already did that.

Those tests have been added in ffc9dda.

Reviewed-by: Andrew Dunstan
Discussion: https://postgr.es/m/ec093ff1-a53c-0091-46a2-4537354b0dd4@dunslane.net

3 years agoUpdate obsolete comment that still referred to CheckpointLock
Heikki Linnakangas [Fri, 30 Jul 2021 09:52:44 +0000 (12:52 +0300)]
Update obsolete comment that still referred to CheckpointLock

CheckpointLock was removed in commit d18e75664a, and commit ce197e91d0
updated a leftover comment in CreateCheckPoint, but there was another
copy of it in CreateRestartPoint still.

3 years agopostgres_fdw: Fix handling of pending asynchronous requests.
Etsuro Fujita [Fri, 30 Jul 2021 08:00:00 +0000 (17:00 +0900)]
postgres_fdw: Fix handling of pending asynchronous requests.

A pending asynchronous request is handled by process_pending_request(),
which previously not only processed an in-progress remote query but
performed ExecForeignScan() to produce a tuple to return to the local
server asynchronously from the result of the remote query.  But that led
to a server crash when executing a query or led to an "InstrStartNode
called twice in a row" or "InstrEndLoop called on running node" failure
when doing EXPLAIN ANALYZE of it, in cases where the plan tree for it
contained multiple async-capable nodes accessing the same
initplan/subplan that contained multiple async-capable nodes scanning
the same foreign tables as for the parent async-capable nodes, as
reported by Andrey Lepikhov.  The reason is that the second step in
process_pending_request() invoked when executing the initplan/subplan
for one of the parent async-capable nodes caused recursive execution of
the initplan/subplan for another of the parent async-capable nodes.

To fix, split process_pending_request() into the two steps and postpone
the second step until ForeignAsyncConfigureWait() is called for each of
the pending asynchronous requests.  Also, in ExecAppendAsyncEventWait()
we assumed that FDWs would register at least one wait event in a
WaitEventSet created there when they were called from
ForeignAsyncConfigureWait() in that function, but allow FDWs to register
zero wait events in the WaitEventSet; modify ExecAppendAsyncEventWait()
to just return in that case.

Oversight in commit 27e1f1456.  Back-patch to v14 where that commit went
in.

Andrey Lepikhov and Etsuro Fujita

Discussion: https://postgr.es/m/fe5eaa19-1704-e4a4-76ee-3b9d37ade399@postgrespro.ru

3 years agoRemove unused argument in apply_handle_commit_internal().
Amit Kapila [Fri, 30 Jul 2021 02:47:38 +0000 (08:17 +0530)]
Remove unused argument in apply_handle_commit_internal().

Oversight in commit 0926e96c49.

Author: Masahiko Sawada
Reviewed-By: Amit Kapila
Backpatch-through: 14, where it was introduced
Discussion: https://postgr.es/m/CAD21AoDeScrsHhLyEPYqN3sydg6PxAPVBboK=30xJfUVihNZDA@mail.gmail.com

3 years agoClose yet another race condition in replication slot test code
Alvaro Herrera [Thu, 29 Jul 2021 21:09:06 +0000 (17:09 -0400)]
Close yet another race condition in replication slot test code

Buildfarm shows that this test has a further failure mode when a
checkpoint starts earlier than expected, so we detect a "checkpoint
completed" line that's not the one we want.  Change the config to try
and prevent this.

Per buildfarm

While at it, update one comment that was forgotten in commit
d18e75664a2f.

Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/20210729.162038.534808353849568395.horikyota.ntt@gmail.com

3 years agodocs: Fix bit_count example output
Daniel Gustafsson [Thu, 29 Jul 2021 19:39:40 +0000 (21:39 +0200)]
docs: Fix bit_count example output

The returnvalue for the bit_count(::bytea) example was assuming a
non-default value of standard_conforming_strings.  This was fixed
in the tests in commit ebedd0c78.

Author: wangzk.fnstxz@fujitsu.com
Discussion: https://postgr.es/m/OSZPR01MB6551FFAC1088C82C3D799BE0FAEB9@OSZPR01MB6551.jpnprd01.prod.outlook.com
Backpatch-through: 14

3 years agoImprove libpq's handling of OOM during error message construction.
Tom Lane [Thu, 29 Jul 2021 17:33:31 +0000 (13:33 -0400)]
Improve libpq's handling of OOM during error message construction.

Commit ffa2e4670 changed libpq so that multiple error reports
occurring during one operation (a connection attempt or query)
are accumulated in conn->errorMessage, where before new ones
usually replaced any prior error.  At least in theory, that makes
us more vulnerable to running out of memory for the errorMessage
buffer.  If it did happen, the user would be left with just an
empty-string error report, which is pretty unhelpful.

We can improve this by relying on pqexpbuffer.c's existing "broken
buffer" convention to track whether we've hit OOM for the current
operation's error string, and then substituting a constant "out of
memory" string in the small number of places where the errorMessage
is read out.

While at it, apply the same method to similar OOM cases in
pqInternalNotice and pqGetErrorNotice3.

Back-patch to v14 where ffa2e4670 came in.  In principle this could
go back further; but in view of the lack of field reports, the
hazard seems negligible in older branches.

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

3 years agoAvoid calling TestLib::perl2host on a symlinked directory
Andrew Dunstan [Thu, 29 Jul 2021 16:15:03 +0000 (12:15 -0400)]
Avoid calling TestLib::perl2host on a symlinked directory

Certain versions of msys2/Windows have been observed to resolve symlinks
in perl2host rather than just follow them. This defeats using a
symlinked shorter path to a longer path, and makes certain tests fail.
We therefore call perl2host on the parent directory of the symlink and
thereafter just use that result.

Apply to release 14 where the problem has been observed.

3 years agoMake TestLib::perl2host more consistent and robust
Andrew Dunstan [Thu, 29 Jul 2021 16:15:03 +0000 (12:15 -0400)]
Make TestLib::perl2host more consistent and robust

Sometimes cygpath has been observed to return a path with a trailing
slash. That can cause problems, Also, make "cygpath" usage
consistent with "pwd -W" with respect to the use of forward slashes.

Backpatch to release 14 where the current code was introduced.

3 years agoRefactor to make common functions in proto.c and worker.c.
Amit Kapila [Thu, 29 Jul 2021 10:21:45 +0000 (15:51 +0530)]
Refactor to make common functions in proto.c and worker.c.

This is a non-functional change only to refactor code to extract some
replication logic into static functions.

This is done as preparation for the 2PC streaming patch which also shares
this common logic.

Author: Peter Smith
Reviewed-By: Amit Kapila
Discussion: https://postgr.es/m/CAHut+PuiSA8AiLcE2N5StzSKs46SQEP_vDOUD5fX2XCVtfZ7mQ@mail.gmail.com

3 years agoRemove unused directory from test/ssl .gitignore
Daniel Gustafsson [Thu, 29 Jul 2021 10:05:54 +0000 (12:05 +0200)]
Remove unused directory from test/ssl .gitignore

The clientside log saved from the testrun was removed in 1caef31d9
but the entry in the .gitignore file remained.  While this exists
in older branches as well, it's mostly a cosmetical fix so no back-
patching is done.

Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/F8E73040-BB6F-43BF-95B4-3CEC037BE856@yesql.se

3 years agoAdd a getter function for a PostgresNode install_path
Andrew Dunstan [Thu, 29 Jul 2021 09:58:08 +0000 (05:58 -0400)]
Add a getter function for a PostgresNode install_path

Experience has shown this can be useful, and while not strictly necessary
we should not normally expose the internals of PostgresNode objects.

3 years agoAdd PostgresVersion.pm method to emit the major version string
Andrew Dunstan [Thu, 29 Jul 2021 09:58:08 +0000 (05:58 -0400)]
Add PostgresVersion.pm method to emit the major version string

For versions before 10, this will produce dotted notation unless a
separator argument is given, in which case it is used.

3 years agoRemove the last vestiges of Exporter from PostgresNode
Andrew Dunstan [Thu, 29 Jul 2021 09:58:08 +0000 (05:58 -0400)]
Remove the last vestiges of Exporter from PostgresNode

Clients wanting to call get_free_port now need to do so via a qualified
name: PostgresNode::get_free_port().

3 years agoUnify PostgresNode's new() and get_new_node() methods
Andrew Dunstan [Thu, 29 Jul 2021 09:58:08 +0000 (05:58 -0400)]
Unify PostgresNode's new() and get_new_node() methods

There is only one constructor now for PostgresNode, with the idiomatic
name 'new'. The method is not exported by the class, and must be called
as "PostgresNode->new('name',[args])". All the TAP tests that use
PostgresNode are modified accordingly. Third party scripts will need
adjusting, which is a fairly mechanical process (I just used a sed
script).

3 years agoAdd adjust_conf method to PostgresNode
Andrew Dunstan [Thu, 29 Jul 2021 09:58:07 +0000 (05:58 -0400)]
Add adjust_conf method to PostgresNode

This method will modify or delete an existing line in the config file
rather than simply appending to the file. This makes adjustment of files
for older versions much simpler and more compact.

3 years agoAdd -w back to the flags for pg_ctl (re)start in PostgresNode
Andrew Dunstan [Thu, 29 Jul 2021 09:58:06 +0000 (05:58 -0400)]
Add -w back to the flags for pg_ctl (re)start in PostgresNode

This is now the default for pg_ctl, but having the flag here explicitly
does no harm and helps with backwards compatibility of the PostgresNode
module.

3 years agoFix typo in tab-complete.c
Michael Paquier [Thu, 29 Jul 2021 05:49:48 +0000 (14:49 +0900)]
Fix typo in tab-complete.c

Introduced in b048326.

Reported-by: Jeff Davis
Discussion: https://postgr.es/m/10785e3e9456a5d761164d3e60d9c4981b80e321.camel@j-davis.com

3 years agoAdd some missing exit() calls in error paths for various binaries
Michael Paquier [Thu, 29 Jul 2021 02:42:58 +0000 (11:42 +0900)]
Add some missing exit() calls in error paths for various binaries

The following changes are done:
- In pg_archivecleanup, the cleanup of older WAL segments would never
fail immediately.
- In pgbench, the initialization of a thread barrier would not fail
hard.
- In pg_recvlogical, a stat() failure never got the call.
- In pg_basebackup, two chmod() reported a failure without exit()'ing
when unpacking some tar data freshly received.  It may be possible to
continue writing some data even after this failure, but that could be
confusing to the user at the end.

These are arguably bugs, but they would happen for code paths where a
failure is unlikely going to happen, so no backpatch is done.

Reviewed-by: Robert Haas, Fabien Coelho
Discussion: https://postgr.es/m/YQDMdB+B68yePFeT@paquier.xyz

3 years agoAdd missing exit() in pg_verifybackup when failing to find pg_waldump
Michael Paquier [Thu, 29 Jul 2021 01:59:49 +0000 (10:59 +0900)]
Add missing exit() in pg_verifybackup when failing to find pg_waldump

pg_verifybackup needs by default pg_waldump to check after a range of
WAL segments required for a backup, except if --no-parse-wal is
specified.  The code checked for the presence of the binary pg_waldump
in an installation and reported an error, but it forgot to properly
exit().  This could lead to confusing errors reported.

Reviewed-by: Robert Haas, Fabien Coelho
Discussion: https://postgr.es/m/YQDMdB+B68yePFeT@paquier.xyz
Backpatch-through: 13

3 years agoAdjust MSVC build scripts to parse Makefiles for defines
David Rowley [Thu, 29 Jul 2021 00:01:23 +0000 (12:01 +1200)]
Adjust MSVC build scripts to parse Makefiles for defines

This adjusts the MSVC build scripts to look at the compile flags mentioned
in the Makefile to look for -D arguments in order to determine which
constants should be defined in Visual Studio builds.

One small anomaly that appeared as a result of this change is that the
Makefile for the ltree contrib module defined LOWER_NODE, but this was
not properly defined in the MSVC build scripts.  This meant that MSVC
builds would differ in case sensitivity in the ltree module when
compared to builds using a make build environment.  To maintain the same
behavior here we remove the -DLOWER_NODE from the Makefile and just always
define it in ltree.h for non-MSVC builds.  We need to maintain the old
behavior here as this affects the on-disk compatibility of GiST indexes
when using the ltree type.

The only other resulting change here is that REFINT_VERBOSE is now defined
for the autoinc, insert_username and moddatetime contrib modules.
Previously on MSVC, this was only defined for the refint module.  This
aligns the behavior to build environments using make as all 4 of these
modules share the same Makefile.

Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/CAApHDvpo6g5csCTjc_0C7DMvgFPomVb0Rh-AcW5afd=Ya=LRuw@mail.gmail.com

3 years agoDon't duplicate references and libraries in MSVC scripts
David Rowley [Wed, 28 Jul 2021 22:41:31 +0000 (10:41 +1200)]
Don't duplicate references and libraries in MSVC scripts

In order not to duplicate references and libraries in the Visual Studio
project files produced by the MSVC build scripts, have them check if a
particular reference or library already exists before adding the same one
again.

Reviewed-by: Álvaro Herrera, Andrew Dunstan, Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/CAApHDvpo6g5csCTjc_0C7DMvgFPomVb0Rh-AcW5afd=Ya=LRuw@mail.gmail.com

3 years agoMake the includes field an array in MSVC build scripts
David Rowley [Wed, 28 Jul 2021 22:14:25 +0000 (10:14 +1200)]
Make the includes field an array in MSVC build scripts

Previously the 'includes' field was a string.  It's slightly nicer to
manage this when it's defined as an array instead. This allows us to
more easily detect and eliminate duplicates.

Reviewed-by: Álvaro Herrera, Andrew Dunstan, Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/CAApHDvpo6g5csCTjc_0C7DMvgFPomVb0Rh-AcW5afd=Ya=LRuw@mail.gmail.com

3 years agoUpdate minimum recovery point on truncation during WAL replay of abort record.
Fujii Masao [Wed, 28 Jul 2021 16:30:02 +0000 (01:30 +0900)]
Update minimum recovery point on truncation during WAL replay of abort record.

If a file is truncated, we must update minRecoveryPoint. Once a file is
truncated, there's no going back; it would not be safe to stop recovery
at a point earlier than that anymore.

Commit 7bffc9b7bf changed xact_redo_commit() so that it updates
minRecoveryPoint on truncation, but forgot to change xact_redo_abort().

Back-patch to all supported versions.

Reported-by: mengjuan.cmj@alibaba-inc.com
Author: Fujii Masao
Reviewed-by: Heikki Linnakangas
Discussion: https://postgr.es/m/b029fce3-4fac-4265-968e-16f36ff4d075.mengjuan.cmj@alibaba-inc.com

3 years agoDisallow negative strides in date_bin()
John Naylor [Wed, 28 Jul 2021 15:22:58 +0000 (11:22 -0400)]
Disallow negative strides in date_bin()

It's not clear what the semantics of negative strides would be, so throw
an error instead.

Per report from Bauyrzhan Sakhariyev

Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://www.postgresql.org/message-id/CAKpL73vZmLuFVuwF26FJ%2BNk11PVHhAnQRoREFcA03x7znRoFvA%40mail.gmail.com
Backpatch to v14

3 years agoUse the AddFile function consistently in MSVC build scripts
David Rowley [Wed, 28 Jul 2021 11:43:40 +0000 (23:43 +1200)]
Use the AddFile function consistently in MSVC build scripts

We seem to be using a mix of manually adding to the 'files' hash and
calling the Addfile() method.  Let's just consistently use AddFile().

Reviewed-by: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/CAApHDvpo6g5csCTjc_0C7DMvgFPomVb0Rh-AcW5afd=Ya=LRuw@mail.gmail.com

3 years agoDoc: Clarify lock levels taken during ATTACH PARTITION
David Rowley [Wed, 28 Jul 2021 03:02:37 +0000 (15:02 +1200)]
Doc: Clarify lock levels taken during ATTACH PARTITION

It wasn't all that clear which lock levels, if any, would be held on the
DEFAULT partition during an ATTACH PARTITION operation.

Also, clarify which locks will be taken if the DEFAULT partition or the
table being attached are themselves partitioned tables.

Here I'm only backpatching to v12 as before then we obtained an ACCESS
EXCLUSIVE lock on the partitioned table.  It seems much less relevant to
mention which locks are taken on other tables when the partitioned table
itself is locked with an ACCESS EXCLUSIVE lock.

Author: Matthias van de Meent, David Rowley
Discussion: https://postgr.es/m/CAEze2WiTB6iwrV8W_J=fnrnZ7fowW3qu-8iQ8zCHP3FiQ6+o-A@mail.gmail.com
Backpatch-through: 12

3 years agoClarify some comments making use of leetspeak term "up2date"
Michael Paquier [Wed, 28 Jul 2021 01:31:24 +0000 (10:31 +0900)]
Clarify some comments making use of leetspeak term "up2date"

Most of these are new, as of a8fd13c, and "up-to-date" is much easier to
parse for the average reader.

Author: Peter Smith
Discussion: https://postgr.es/m/CAHut+PtHbHvgOjs_R9LyDF21j-Wn8SxoTtWMQNP2ifXN6t2cSg@mail.gmail.com

3 years agoAdd support for SET ACCESS METHOD in ALTER TABLE
Michael Paquier [Wed, 28 Jul 2021 01:10:44 +0000 (10:10 +0900)]
Add support for SET ACCESS METHOD in ALTER TABLE

The logic used to support a change of access method for a table is
similar to changes for tablespace or relation persistence, requiring a
table rewrite with an exclusive lock of the relation changed.  Table
rewrites done in ALTER TABLE already go through the table AM layer when
scanning tuples from the old relation and inserting them into the new
one, making this implementation straight-forward.

Note that partitioned tables are not supported as these have no access
methods defined.

Author: Justin Pryzby, Jeff Davis
Reviewed-by: Michael Paquier, Vignesh C
Discussion: https://postgr.es/m/20210228222530.GD20769@telsasoft.com

3 years agoRemove seemingly unneeded include directory in MSVC scripts
David Rowley [Wed, 28 Jul 2021 00:56:43 +0000 (12:56 +1200)]
Remove seemingly unneeded include directory in MSVC scripts

This appears to have been added way back in ee3b4188a but it's a little
unclear why the change made in that commit is even needed given that
320c7eb8c, dated 18 months earlier, added code to copy fmgroids.h to
src/include/utils.

amcheck seems to get away without adding the additional include directory,
so perhaps dblink can get away with it too.

This builds ok in my VS2017 environment, but the buildfarm may serve as a
reminder about why ee3b4188a was required.  There's only one way to find
out for sure.

Discussion: https://postgr.es/m/CAApHDvpo6g5csCTjc_0C7DMvgFPomVb0Rh-AcW5afd=Ya=LRuw@mail.gmail.com

3 years agoSet pg_setting.pending_restart when pertinent config lines are removed
Alvaro Herrera [Tue, 27 Jul 2021 19:44:12 +0000 (15:44 -0400)]
Set pg_setting.pending_restart when pertinent config lines are removed

This changes the behavior of examining the pg_file_settings view after
changing a config option that requires restart.  The user needs to know
that any change of such options does not take effect until a restart,
and this worked correctly if the line is edited without removing it.
However, for the case where the line is removed altogether, the flag
doesn't get set, because a flag was only set in set_config_option, but
that's not called for lines removed.  Repair.

(Ref.: commits 62d16c7fc561 and a486e35706ea)

Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/202107262302.xsfdfc5sb7sh@alvherre.pgsql

3 years agoFix bugs in polymorphic-argument resolution for multiranges.
Tom Lane [Tue, 27 Jul 2021 19:01:49 +0000 (15:01 -0400)]
Fix bugs in polymorphic-argument resolution for multiranges.

We failed to deal with an UNKNOWN-type input for
anycompatiblemultirange; that should throw an error indicating
that we don't know how to resolve the multirange type.

We also failed to infer the type of an anycompatiblerange output
from an anycompatiblemultirange input or vice versa.

Per bug #17066 from Alexander Lakhin.  Back-patch to v14
where multiranges were added.

Discussion: https://postgr.es/m/17066-16a37f6223a8470b@postgresql.org

3 years agoStabilize output of new regression test.
Tom Lane [Tue, 27 Jul 2021 16:49:40 +0000 (12:49 -0400)]
Stabilize output of new regression test.

Commit 48c5c9068 failed to allow for buildfarm animals that
force jit = on.  I'm surprised that this hasn't come up
elsewhere in explain.sql, so turn it off for that whole
test script not just the one new test case.

Per buildfarm.

3 years agoAvoid using ambiguous word "non-negative" in error messages.
Fujii Masao [Tue, 27 Jul 2021 16:20:16 +0000 (01:20 +0900)]
Avoid using ambiguous word "non-negative" in error messages.

The error messages using the word "non-negative" are confusing
because it's ambiguous about whether it accepts zero or not.
This commit improves those error messages by replacing it with
less ambiguous word like "greater than zero" or
"greater than or equal to zero".

Also this commit added the note about the word "non-negative" to
the error message style guide, to help writing the new error messages.

When postgres_fdw option fetch_size was set to zero, previously
the error message "fetch_size requires a non-negative integer value"
was reported. This error message was outright buggy. Therefore
back-patch to all supported versions where such buggy error message
could be thrown.

Reported-by: Hou Zhijie
Author: Bharath Rupireddy
Reviewed-by: Kyotaro Horiguchi, Fujii Masao
Discussion: https://postgr.es/m/OS0PR01MB5716415335A06B489F1B3A8194569@OS0PR01MB5716.jpnprd01.prod.outlook.com

3 years agoIn event triggers, use "pg_temp" only for our own temp schema.
Tom Lane [Tue, 27 Jul 2021 16:08:20 +0000 (12:08 -0400)]
In event triggers, use "pg_temp" only for our own temp schema.

pg_event_trigger_ddl_commands used "pg_temp" to refer to any
temp schema, not only that of the current backend.  This seems
like overreach.  It's somewhat unlikely that DDL commands would
refer to temp objects of other sessions to begin with, but if they
do, "pg_temp" would be a most misleading way to display the action.

While this seems like a bug, it's not quite out of the realm of
possibility that somebody out there is expecting the current
behavior.  Hence, fix in HEAD, but don't back-patch.

Discussion: https://postgr.es/m/CAAJ_b97W=QaGmag9AhWNbmx3uEYsNkXWL+OVW1_E1D3BtgWvtw@mail.gmail.com

3 years agoUse the "pg_temp" schema alias in EXPLAIN and related output.
Tom Lane [Tue, 27 Jul 2021 16:03:16 +0000 (12:03 -0400)]
Use the "pg_temp" schema alias in EXPLAIN and related output.

This patch causes EXPLAIN output to refer to objects that are in
the current session's temp schema with the "pg_temp" schema alias
rather than that schema's actual name.  This is useful for our own
testing purposes since it will stabilize EXPLAIN VERBOSE output
for such cases, allowing us to use that in regression tests.
It should be less confusing for end users too.

Since ruleutils.c needs to change behavior for this, the change
also leaks into a few other users of ruleutils.c, for example
pg_get_viewdef().  AFAICS that won't cause any problems.
We did find that aggressively trying to change this behavior
across-the-board would cause issues, but as long as "pg_temp"
only appears within generated SQL text, I think it'll be fine.

Along the way, make get_namespace_name_or_temp conform to the
same API as get_namespace_name, ie that it returns a palloc'd
string or NULL.  The current behavior hasn't caused any bugs
since no callers attempt to pfree the result, but if it gets
more widespread usage that could become a problem.

Amul Sul, reviewed and extended by me

Discussion: https://postgr.es/m/CAAJ_b97W=QaGmag9AhWNbmx3uEYsNkXWL+OVW1_E1D3BtgWvtw@mail.gmail.com

3 years agoDisable command echo in pg_upgrade-created windows scripts
Andrew Dunstan [Tue, 27 Jul 2021 15:15:38 +0000 (11:15 -0400)]
Disable command echo in pg_upgrade-created  windows scripts

This makes them more like the Unix equivalents.

Discussion: https://postgr.es/m/73deed30-3363-44e9-48a5-98aa66a8a00f@dunslane.net

3 years agodoc: for various substring funcs, document if only first match
Bruce Momjian [Tue, 27 Jul 2021 02:54:35 +0000 (22:54 -0400)]
doc:  for various substring funcs, document if only first match

Reported-by: troy@frericks.us
Discussion: https://postgr.es/m/162614304115.701.2392941350859387646@wrigleys.postgresql.org

Backpatch-through: 13

3 years agopg_resetxlog: add option to set oldest xid & use by pg_upgrade
Bruce Momjian [Tue, 27 Jul 2021 02:38:15 +0000 (22:38 -0400)]
pg_resetxlog:  add option to set oldest xid & use by pg_upgrade

Add pg_resetxlog -u option to set the oldest xid in pg_control.
Previously -x set this value be -2 billion less than the -x value.
However, this causes the server to immediately scan all relation's
relfrozenxid so it can advance pg_control's oldest xid to be inside the
autovacuum_freeze_max_age range, which is inefficient and might disrupt
diagnostic recovery.  pg_upgrade will use this option to better create
the new cluster to match the old cluster.

Reported-by: Jason Harvey, Floris Van Nee
Discussion: https://postgr.es/m/20190615183759.GB239428@rfd.leadboat.com, 87da83168c644fd9aae38f546cc70295@opammb0562.comp.optiver.com

Author: Bertrand Drouvot

Backpatch-through: 9.6

3 years agoSimplify matching pattern check in TAP tests of pg_receivewal
Michael Paquier [Tue, 27 Jul 2021 01:58:21 +0000 (10:58 +0900)]
Simplify matching pattern check in TAP tests of pg_receivewal

A check in the ZLIB portion of the test to match the name of a
non-compressed partial segment with a completed compressed segment was
using m//, while a simple equality check is enough.  This makes the test
a bit stricter without impacting its coverage.

Author: Kyotaro Horiguchi
Discussion: https://postgr.es/m/20210726.174622.826565852378770261.horikyota.ntt@gmail.com

3 years agoSkip trailing whitespaces when parsing integer options
Michael Paquier [Tue, 27 Jul 2021 01:39:05 +0000 (10:39 +0900)]
Skip trailing whitespaces when parsing integer options

strtoint(), via strtol(), would skip leading whitespaces but the same
rule was not applied for trailing whitespaces, leading to an
inconsistent behavior.  Some tests are changed to cover more this area.

Author: Michael Paquier
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/YP5Pv0d13Ct+03ve@paquier.xyz

3 years agoRemove newly added useless assertion check
Alvaro Herrera [Mon, 26 Jul 2021 16:56:33 +0000 (12:56 -0400)]
Remove newly added useless assertion check

Coverity complained that my commit 80ba4bb38353 added a dubious coding
for a consistency check that there isn't more than one row for a certain
tgrelid/tgparentid combination.  But we don't check for that explicitly
anywhere else, and if we were to do it, it should be a full
shouldn't-happen elog not just an assert.  It doesn't seem that this is
very important anyway, so remove it.

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

3 years agopsql \dX: check schema when listing statistics objects
Tomas Vondra [Mon, 26 Jul 2021 15:12:28 +0000 (17:12 +0200)]
psql \dX: check schema when listing statistics objects

Commit ad600bba04 added psql command \dX listing extended statistics
objects, but it failed to consider search_path when selecting the
elements so some of the returned elements might be invisible.

The visibility was already considered for tab completion (added by
commit d99d58cdc8), so adding it to the query is fairly simple.

Reported and fix by Justin Pryzby, regression tests by me. Backpatch
to PostgreSQL 14, where \dX was introduced.

Batchpatch-through: 14
Author: Justin Pryzby
Reviewed-by: Tatsuro Yamada
Discussion: https://postgr.es/m/c027a541-5856-75a5-0868-341301e1624b%40nttcom.co.jp_1

3 years agoAllow numeric scale to be negative or greater than precision.
Dean Rasheed [Mon, 26 Jul 2021 13:13:47 +0000 (14:13 +0100)]
Allow numeric scale to be negative or greater than precision.

Formerly, when specifying NUMERIC(precision, scale), the scale had to
be in the range [0, precision], which was per SQL spec. This commit
extends the range of allowed scales to [-1000, 1000], independent of
the precision (whose valid range remains [1, 1000]).

A negative scale implies rounding before the decimal point. For
example, a column might be declared with a scale of -3 to round values
to the nearest thousand. Note that the display scale remains
non-negative, so in this case the display scale will be zero, and all
digits before the decimal point will be displayed.

A scale greater than the precision supports fractional values with
zeros immediately after the decimal point.

Take the opportunity to tidy up the code that packs, unpacks and
validates the contents of a typmod integer, encapsulating it in a
small set of new inline functions.

Bump the catversion because the allowed contents of atttypmod have
changed for numeric columns. This isn't a change that requires a
re-initdb, but negative scale values in the typmod would confuse old
backends.

Dean Rasheed, with additional improvements by Tom Lane. Reviewed by
Tom Lane.

Discussion: https://postgr.es/m/CAEZATCWdNLgpKihmURF8nfofP0RFtAKJ7ktY6GcZOPnMfUoRqA@mail.gmail.com

3 years agodoc: Fix command example to run regression tests with PGOPTIONS
Michael Paquier [Mon, 26 Jul 2021 07:26:47 +0000 (16:26 +0900)]
doc: Fix command example to run regression tests with PGOPTIONS

The documentation mentioned the use of log_checkpoints, that cannot be
used in this context.  This commit replaces log_checkpoints with
force_parallel_mode, a developer option useful to perform checks related
to parallelism.

Oversight in 854434c.

Author: Haiying Tang
Discussion: https://postgr.es/m/OS0PR01MB6113954B883ACEB2DDC973F2FBE59@OS0PR01MB6113.jpnprd01.prod.outlook.com
Backpatch-through: 14

3 years agoHarden pg_stat_statements tests against CLOBBER_CACHE_ALWAYS.
Tom Lane [Mon, 26 Jul 2021 03:25:15 +0000 (23:25 -0400)]
Harden pg_stat_statements tests against CLOBBER_CACHE_ALWAYS.

Turns out the buildfarm hasn't been testing this, which will soon change.

Julien Rouhaud, per report from me

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

3 years agoFix incorrect comment for get_agg_clause_costs
David Rowley [Mon, 26 Jul 2021 02:55:31 +0000 (14:55 +1200)]
Fix incorrect comment for get_agg_clause_costs

Adjust the header comment in get_agg_clause_costs so that it matches what
the function currently does.  No recursive searching has been done ever
since 0a2bc5d61.  It also does not determine the aggtranstype like the
comment claimed. That's all done in preprocess_aggref().
preprocess_aggref also now determines the numOrderedAggs, so remove the
mention that get_agg_clause_costs also calculates "counts".

Normally, since this is just an adjustment of a comment it might not be
worth back-patching, but since this code is new to PG14 and that version
is still in beta, then it seems worth having the comments match.

Discussion: https://postgr.es/m/CAApHDvrrGrTJFPELrjx0CnDtz9B7Jy2XYW3Z2BKifAWLSaJYwQ@mail.gmail.com
Backpatch-though: 14

3 years agoFix a couple of memory leaks in src/bin/pg_basebackup/
Michael Paquier [Mon, 26 Jul 2021 02:13:37 +0000 (11:13 +0900)]
Fix a couple of memory leaks in src/bin/pg_basebackup/

These have been introduced by 7fbe0c8, and could happen for
pg_basebackup and pg_receivewal.

Per report from Coverity for the ones in walmethods.c, I have spotted
the ones in receivelog.c after more review.

Backpatch-through: 10

3 years agoGet rid of artificial restriction on hash table sizes on Windows.
Tom Lane [Sun, 25 Jul 2021 18:02:27 +0000 (14:02 -0400)]
Get rid of artificial restriction on hash table sizes on Windows.

The point of introducing the hash_mem_multiplier GUC was to let users
reproduce the old behavior of hash aggregation, i.e. that it could use
more than work_mem at need.  However, the implementation failed to get
the job done on Win64, where work_mem is clamped to 2GB to protect
various places that calculate memory sizes using "long int".  As
written, the same clamp was applied to hash_mem.  This resulted in
severe performance regressions for queries requiring a bit more than
2GB for hash aggregation, as they now spill to disk and there's no
way to stop that.

Getting rid of the work_mem restriction seems like a good idea, but
it's a big job and could not conceivably be back-patched.  However,
there's only a fairly small number of places that are concerned with
the hash_mem value, and it turns out to be possible to remove the
restriction there without too much code churn or any ABI breaks.
So, let's do that for now to fix the regression, and leave the
larger task for another day.

This patch does introduce a bit more infrastructure that should help
with the larger task, namely pg_bitutils.h support for working with
size_t values.

Per gripe from Laurent Hasson.  Back-patch to v13 where the
behavior change came in.

Discussion: https://postgr.es/m/997817.1627074924@sss.pgh.pa.us
Discussion: https://postgr.es/m/MN2PR15MB25601E80A9B6D1BA6F592B1985E39@MN2PR15MB2560.namprd15.prod.outlook.com

3 years agoDeduplicate choice of horizon for a relation procarray.c.
Andres Freund [Sun, 25 Jul 2021 03:14:03 +0000 (20:14 -0700)]
Deduplicate choice of horizon for a relation procarray.c.

5a1e1d83022 was a minimal bug fix for dc7420c2c92. To avoid future bugs of
that kind, deduplicate the choice of a relation's horizon into a new helper,
GlobalVisHorizonKindForRel().

As the code in question was only introduced in dc7420c2c92 it seems worth
backpatching this change as well, otherwise 14 will look different from all
other branches.

A different approach to this was suggested by Matthias van de Meent.

Author: Andres Freund
Discussion: https://postgr.es/m/20210621122919.2qhu3pfugxxp3cji@alap3.anarazel.de
Backpatch: 14, like 5a1e1d83022

3 years agoFix check for conflicting session- vs transaction-level locks.
Tom Lane [Sat, 24 Jul 2021 22:35:52 +0000 (18:35 -0400)]
Fix check for conflicting session- vs transaction-level locks.

We have an implementation restriction that PREPARE TRANSACTION can't
handle cases where both session-lifespan and transaction-lifespan locks
are held on the same lockable object.  (That's because we'd otherwise
need to acquire a new PROCLOCK entry during post-prepare cleanup, which
is an operation that might fail.  The situation can only arise with odd
usages of advisory locks, so removing the restriction is probably not
worth the amount of effort it would take.)  AtPrepare_Locks attempted
to enforce this, but its logic was many bricks shy of a load, because
it only detected cases where the session and transaction locks had the
same lockmode.  Locks of different modes on the same object would lead
to the rather unhelpful message "PANIC: we seem to have dropped a bit
somewhere".

To fix, build a transient hashtable with one entry per locktag,
not one per locktag + mode, and use that to detect conflicts.

Per bug #17122 from Alexander Pyhalov.  This bug is ancient,
so back-patch to all supported branches.

Discussion: https://postgr.es/m/17122-04f3c32098a62233@postgresql.org

3 years agoMake printf("%s", NULL) print "(null)" instead of crashing.
Tom Lane [Sat, 24 Jul 2021 17:41:17 +0000 (13:41 -0400)]
Make printf("%s", NULL) print "(null)" instead of crashing.

We previously took a hard-line attitude that callers should never print
a null string pointer, and doing so is worthy of an assertion failure
or crash.  However, we've long since flushed out any easy-to-find bugs
of that nature.  What remains is a lot of code that perhaps could fail
that way in hard-to-reach corner cases.  For example, in something as
simple as
    ereport(ERROR,
            (errcode(ERRCODE_UNDEFINED_OBJECT),
             errmsg("constraint \"%s\" for table \"%s\" does not exist",
                    conname, get_rel_name(relid))));
one must wonder whether it's completely guaranteed that get_rel_name
cannot return NULL in this context.  If such a situation did occur,
the existing policy converts what might be a pretty minor bug into
a server crash condition.  This is not good for robustness.

Hence, let's follow the lead of glibc and print "(null)" instead
of failing.  We should, of course, still consider it a bug if that
behavior is reachable in ordinary use; but crashing seems less
desirable than not crashing.

This fix works across-the-board in v12 and up, where we always use
src/port/snprintf.c.  Before that, on most platforms we're at the mercy
of the local libc, but it appears that Solaris 10 is the only supported
platform where we'd still get a crash.  Most other platforms such as
*BSD, macOS, and Solaris 11 have adopted glibc's behavior at some
point.  (AIX and HPUX just print "" not "(null)", but that's close
enough.)  I've not checked what Windows' native printf would do, but
it doesn't matter because we've long used snprintf.c on that platform.

In v12 and up, also const-ify related code so that we're not casting
away const on the constant string.  This is just neatnik-ism, since
next to no compilers will warn about that.

Discussion: https://postgr.es/m/17098-b960f3616c861f83@postgresql.org

3 years agoRemove configure-time thread safety checking (thread_test.c).
Tom Lane [Sat, 24 Jul 2021 16:16:39 +0000 (12:16 -0400)]
Remove configure-time thread safety checking (thread_test.c).

This testing was useful when it was written, nigh twenty years ago,
but it seems fairly pointless for any platform built in the last
dozen or more years.  (Compare also the comments at 8a2121185.)
Also we now have reports that the test program itself fails under
ThreadSanitizer.  Rather than invest effort in fixing it, let's
just drop it, and assume that the few people who still care
already know they need to use --disable-thread-safety.

Back-patch into v14, for consistency with 8a2121185.

Discussion: https://postgr.es/m/CADhDkKzPSiNvA3Hyq+wSR_icuPmazG0cFe=YnC3U-CFcYLc8Xw@mail.gmail.com

3 years agoFix failure of some headers to compile "standalone".
Tom Lane [Sat, 24 Jul 2021 15:34:33 +0000 (11:34 -0400)]
Fix failure of some headers to compile "standalone".

Recently-added references to ParseState weren't covered by #include
references, creating unwanted ordering dependencies for users of
these headers.

Oversight in commit 2bfb50b3d.  Per headerscheck/cpluspluscheck.

3 years agoAdd missing header declarations for pg_basebackup and pg_{dump,restore}
Michael Paquier [Sat, 24 Jul 2021 10:05:14 +0000 (19:05 +0900)]
Add missing header declarations for pg_basebackup and pg_{dump,restore}

This fixes two compilation failures caused by 6f164e6.  Interesting to
see that missing <limits.h> dies not fail in Linux or even Windows.  On
MacOS, it fails, though.

Per various buildfarm members.

3 years agoUnify parsing logic for command-line integer options
Michael Paquier [Sat, 24 Jul 2021 09:35:03 +0000 (18:35 +0900)]
Unify parsing logic for command-line integer options

Most of the integer options for command-line binaries now make use of a
single routine able to do the job, fixing issues with the detection of
sloppy values caused for example by the use of atoi(), that fails on
strings beginning with numerical characters with junk trailing
characters.

This commit cuts down the number of strings requiring translation by 26
per my count, switching the code to have two error types for invalid and
out-of-range values instead.

Much more could be done here, with float or even int64 options, but
int32 was the most appealing case as it is possible to rely on strtol()
to do the job reliably.  Note that there are some exceptions for now,
like pg_ctl or pg_upgrade that use their own logging logic.  A couple of
negative TAP tests required some adjustments for the new errors
generated.

pg_dump and pg_restore tracked the maximum number of parallel jobs
within the option parsing.  The code is refactored a bit to track that
in the code dedicated to parallelism instead.

Author: Kyotaro Horiguchi, Michael Paquier
Reviewed-by: David Rowley, Álvaro Herrera
Discussion: https://postgr.es/m/CALj2ACXqdG9WhqVoJ9zYf-iZt7sgK7Szv5USs=he6NnWQ2ofTA@mail.gmail.com

3 years agoMake new test immune to collation
Alvaro Herrera [Fri, 23 Jul 2021 15:01:22 +0000 (11:01 -0400)]
Make new test immune to collation

Animals running in Czech locale failed.  I could try to find table names
that don't have this problem, but it seems simpler to just use the C
locale.

Per buildfarm

3 years agoMake ALTER TRIGGER RENAME consistent for partitioned tables
Alvaro Herrera [Thu, 22 Jul 2021 22:33:47 +0000 (18:33 -0400)]
Make ALTER TRIGGER RENAME consistent for partitioned tables

Renaming triggers on partitioned tables had two problems: first,
it did not recurse to renaming the triggers on the partitions; and
second, it failed to prohibit renaming clone triggers.  Having triggers
with different names in partitions is pointless, and furthermore pg_dump
would not preserve names for partitions anyway.

Not backpatched -- making the ALTER TRIGGER throw an error in stable
versions might cause problems for existing scripts.

Co-authored-by: Arne Roland <A.Roland@index.de>
Co-authored-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Zhihong Yu <zyu@yugabyte.com>
Discussion: https://postgr.es/m/d0fd7040c2fb4de1a111b9d9ccc456b8@index.de

3 years agoRework installation instructions again
Alvaro Herrera [Thu, 22 Jul 2021 22:03:26 +0000 (18:03 -0400)]
Rework installation instructions again

Dept. of second thoughts: the new verbiage added in commit aaec237b1a2f
is targeted at the wrong audience.  Remove the bits about git and talk
about how to get tarballs only.  People looking for the git repo can
look in the appendix.  That'll need to be expanded, but this commit
doesn't do that.

In passing, fix a couple of typos that snuck in with the previous
commit.

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

3 years agoFix division by zero error in date_bin
John Naylor [Thu, 22 Jul 2021 21:34:19 +0000 (17:34 -0400)]
Fix division by zero error in date_bin

Bauyrzhan Sakhariyev, via Github

Backpatch to v14

3 years agoFix typo in comment
Peter Eisentraut [Thu, 22 Jul 2021 07:37:35 +0000 (09:37 +0200)]
Fix typo in comment

Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/20210716.170209.175434392011070182.horikyota.ntt%40gmail.com

3 years agoDisable TAP tests of pg_receivewal for ZLIB on Windows
Michael Paquier [Thu, 22 Jul 2021 03:57:43 +0000 (12:57 +0900)]
Disable TAP tests of pg_receivewal for ZLIB on Windows

This reverts commit 91d395f, to avoid running those tests on Windows.
The tests are globally stable across all buildfarm members, except
fairywren (crash of pg_receivewal) and bowerdird (SIGBREAK preventing
the buildfarm run to complete).  Those errors are rather strange, as
other hosts with very similar characteristics are able to run those
tests without breaking a sweat.

For now, disable those tests on Windows to turn back the buildfarm to
green.

Per discussion with Andrew Dunstan.

Discussion: https://postgr.es/m/9040d5ed-6462-66a4-07ac-2923785ae563@dunslane.net

3 years agojit: Don't inline functions that access thread-locals.
Thomas Munro [Thu, 22 Jul 2021 02:11:17 +0000 (14:11 +1200)]
jit: Don't inline functions that access thread-locals.

Code inlined by LLVM can crash or fail with "Relocation type not
implemented yet!" if it tries to access thread local variables.  Don't
inline such code.

Back-patch to 11, where LLVM arrived.  Bug #16696.

Author: Dmitry Marakasov <amdmi3@amdmi3.ru>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/16696-29d944a33801fbfe@postgresql.org

3 years agoMake nodeSort.c use Datum sorts for single column sorts
David Rowley [Thu, 22 Jul 2021 02:03:19 +0000 (14:03 +1200)]
Make nodeSort.c use Datum sorts for single column sorts

Datum sorts can be significantly faster than tuple sorts, especially when
the data type being sorted is a pass-by-value type.  Something in the
region of 50-70% performance improvements appear to be possible.

Just in case there's any confusion; the Datum sort is only used when the
targetlist of the Sort node contains a single column, not when there's a
single column in the sort key and multiple items in the target list.

Author: Ronan Dunklau
Reviewed-by: James Coleman, David Rowley, Ranier Vilela, Hou Zhijie
Tested-by: John Naylor
Discussion: https://postgr.es/m/3177670.itZtoPt7T5@aivenronan

3 years agoDoc: improve documentation about exponentiation operator.
Tom Lane [Wed, 21 Jul 2021 22:03:33 +0000 (18:03 -0400)]
Doc: improve documentation about exponentiation operator.

Now that we're not having to wedge this into the straitjacket of
the old operator table format, we can add another example to
clarify the point about left-to-right associativity.

Per suggestion from mdione at grulic.org.ar.

https://postgr.es/m/162661954599.693.13700316547731859171@wrigleys.postgresql.org

3 years agoImprove installation instructions a bit
Alvaro Herrera [Wed, 21 Jul 2021 17:56:15 +0000 (13:56 -0400)]
Improve installation instructions a bit

Reported-by: Daniel Westermann
Discussion: https://postgr.es/m/GV0P278MB04835AAEACF894617574EB6BD25A9@GV0P278MB0483.CHEP278.PROD.OUTLOOK.COM

3 years agoDocument "B" and "us" as accepted units in postgres.conf.sample
John Naylor [Wed, 21 Jul 2021 14:17:07 +0000 (10:17 -0400)]
Document "B" and "us" as accepted units in postgres.conf.sample

In postgresql.conf, memory and file size GUCs can be specified with "B"
(bytes) as of b06d8e58b. Likewise, time GUCs can be specified with "us"
(microseconds) as of caf626b2c. Update postgres.conf.sample to reflect
that fact.

Pavel Luzanov

Backpatch to v12, which is the earliest version that allows both of
these units. A separate commit will document the "B" case for v11.

Discussion: https://www.postgresql.org/message-id/flat/f10d16fc-8fa0-1b3c-7371-cb3a35a13b7a%40postgrespro.ru

3 years agoAdd missing check of noError parameter in euc_tw_and_big5.c
John Naylor [Wed, 21 Jul 2021 13:11:32 +0000 (09:11 -0400)]
Add missing check of noError parameter in euc_tw_and_big5.c

Oversight in ea1b99a66

Yukun Wang

Backpatch to v14 where this parameter was introduced

Discussion: https://www.postgresql.org/message-id/flat/OS0PR01MB6003FCEFF0201EF21685FD33B4E39%40OS0PR01MB6003.jpnprd01.prod.outlook.com

3 years agoAdd missing enum tags in enums used in nodes
Peter Eisentraut [Wed, 21 Jul 2021 09:03:25 +0000 (11:03 +0200)]
Add missing enum tags in enums used in nodes

Discussion: https://www.postgresql.org/message-id/flat/c1097590-a6a4-486a-64b1-e1f9cc0533ce@enterprisedb.com

3 years agoRename some node support functions for consistency
Peter Eisentraut [Wed, 21 Jul 2021 08:24:06 +0000 (10:24 +0200)]
Rename some node support functions for consistency

Some node function names didn't match their node type names exactly.
Fix those for consistency.

Discussion: https://www.postgresql.org/message-id/flat/c1097590-a6a4-486a-64b1-e1f9cc0533ce@enterprisedb.com

3 years agoRename argument of _outValue()
Peter Eisentraut [Wed, 21 Jul 2021 07:18:32 +0000 (09:18 +0200)]
Rename argument of _outValue()

Rename from value to node, for consistency with similar functions.

Discussion: https://www.postgresql.org/message-id/flat/c1097590-a6a4-486a-64b1-e1f9cc0533ce@enterprisedb.com

3 years agoRename NodeTag of ExprState
Peter Eisentraut [Wed, 21 Jul 2021 06:48:33 +0000 (08:48 +0200)]
Rename NodeTag of ExprState

Rename from tag to type, for consistency with all other node structs.

Discussion: https://www.postgresql.org/message-id/flat/c1097590-a6a4-486a-64b1-e1f9cc0533ce@enterprisedb.com

3 years agoMore improvements of error messages about mismatching relkind
Peter Eisentraut [Wed, 21 Jul 2021 05:40:05 +0000 (07:40 +0200)]
More improvements of error messages about mismatching relkind

Follow-up to 2ed532ee8c474e9767e76e1f3251cc3a0224358c, a few error
messages in the logical replication area currently only deal with
tables, but if we're anticipating more relkinds such as sequences
being handled, then these messages also fall into the category
affected by the previous patch, so adjust them too.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/c9ba5c6a-4bd5-e12c-1b3c-edbcaedbf392@enterprisedb.com

3 years agodoc: Document that only superusers can use pg_import_system_collations().
Fujii Masao [Wed, 21 Jul 2021 04:52:37 +0000 (13:52 +0900)]
doc: Document that only superusers can use pg_import_system_collations().

Back-patch to v10 where pg_import_system_collations() was added.

Author: Atsushi Torikoshi
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/b7f484692a3e283710032e68b7f40617@oss.nttdata.com