postgresql.git
4 years agoFix race condition between shutdown and unstarted background workers.
Tom Lane [Thu, 24 Dec 2020 22:00:43 +0000 (17:00 -0500)]
Fix race condition between shutdown and unstarted background workers.

If a database shutdown (smart or fast) is commanded between the time
some process decides to request a new background worker and the time
that the postmaster can launch that worker, then nothing happens
because the postmaster won't launch any bgworkers once it's exited
PM_RUN state.  This is fine ... unless the requesting process is
waiting for that worker to finish (or even for it to start); in that
case the requestor is stuck, and only manual intervention will get us
to the point of being able to shut down.

To fix, cancel pending requests for workers when the postmaster sends
shutdown (SIGTERM) signals, and similarly cancel any new requests that
arrive after that point.  (We can optimize things slightly by only
doing the cancellation for workers that have waiters.)  To fit within
the existing bgworker APIs, the "cancel" is made to look like the
worker was started and immediately stopped, causing deregistration of
the bgworker entry.  Waiting processes would have to deal with
premature worker exit anyway, so this should introduce no bugs that
weren't there before.  We do have a side effect that registration
records for restartable bgworkers might disappear when theoretically
they should have remained in place; but since we're shutting down,
that shouldn't matter.

Back-patch to v10.  There might be value in putting this into 9.6
as well, but the management of bgworkers is a bit different there
(notably see 8ff518699) and I'm not convinced it's worth the effort
to validate the patch for that branch.

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

4 years agoImprove client error messages for immediate-stop situations.
Tom Lane [Thu, 24 Dec 2020 17:58:32 +0000 (12:58 -0500)]
Improve client error messages for immediate-stop situations.

Up to now, if the DBA issued "pg_ctl stop -m immediate", the message
sent to clients was the same as for a crash-and-restart situation.
This is confusing, not least because the message claims that the
database will soon be up again, something we have no business
predicting.

Improve things so that we can generate distinct messages for the two
cases (and also recognize an ad-hoc SIGQUIT, should somebody try that).
To do that, add a field to pmsignal.c's shared memory data structure
that the postmaster sets just before broadcasting SIGQUIT to its
children.  No interlocking seems to be necessary; the intervening
signal-sending and signal-receipt should sufficiently serialize accesses
to the field.  Hence, this isn't any riskier than the existing usages
of pmsignal.c.

We might in future extend this idea to improve other
postmaster-to-children signal scenarios, although none of them
currently seem to be as badly overloaded as SIGQUIT.

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

4 years agoFix typos and grammar in docs and comments
Michael Paquier [Thu, 24 Dec 2020 08:05:49 +0000 (17:05 +0900)]
Fix typos and grammar in docs and comments

This fixes several areas of the documentation and some comments in
matters of style, grammar, or even format.

Author: Justin Pryzby
Discussion: https://postgr.es/m/20201222041153.GK30237@telsasoft.com

4 years agodummy commit
Bruce Momjian [Wed, 25 Nov 2020 17:29:21 +0000 (12:29 -0500)]
dummy commit

4 years agoFix portability issues with parsing of recovery_target_xid
Michael Paquier [Wed, 23 Dec 2020 03:51:22 +0000 (12:51 +0900)]
Fix portability issues with parsing of recovery_target_xid

The parsing of this parameter has been using strtoul(), which is not
portable across platforms.  On most Unix platforms, unsigned long has a
size of 64 bits, while on Windows it is 32 bits.  It is common in
recovery scenarios to rely on the output of txid_current() or even the
newer pg_current_xact_id() to get a transaction ID for setting up
recovery_target_xid.  The value returned by those functions includes the
epoch in the computed result, which would cause strtoul() to fail where
unsigned long has a size of 32 bits once the epoch is incremented.

WAL records and 2PC data include only information about 32-bit XIDs and
it is not possible to have XIDs across more than one epoch, so
discarding the high bits from the transaction ID set has no impact on
recovery.  On the contrary, the use of strtoul() prevents a consistent
behavior across platforms depending on the size of unsigned long.

This commit changes the parsing of recovery_target_xid to use
pg_strtouint64() instead, available down to 9.6.  There is one TAP test
stressing recovery with recovery_target_xid, where a tweak based on
pg_reset{xlog,wal} is added to bump the XID epoch so as this change gets
tested, as per an idea from Alexander Lakhin.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/16780-107fd0c0385b1035@postgresql.org
Backpatch-through: 9.6

4 years agoImprove autoprewarm's handling of early-shutdown scenarios.
Tom Lane [Tue, 22 Dec 2020 18:23:49 +0000 (13:23 -0500)]
Improve autoprewarm's handling of early-shutdown scenarios.

Bad things happen if the DBA issues "pg_ctl stop -m fast" before
autoprewarm finishes loading its list of blocks to prewarm.
The current worker process successfully terminates early, but
(if this wasn't the last database with blocks to prewarm) the
leader process will just try to launch another worker for the
next database.  Since the postmaster is now in PM_WAIT_BACKENDS
state, it ignores the launch request, and the leader just sits
until it's killed manually.

This is mostly the fault of our half-baked design for launching
background workers, but a proper fix for that is likely to be
too invasive to be back-patchable.  To ameliorate the situation,
fix apw_load_buffers() to check whether SIGTERM has arrived
just before trying to launch another worker.  That leaves us with
only a very narrow window in each worker launch where SIGTERM
could occur between the launch request and successful worker start.

Another issue is that if the leader process does manage to exit,
it unconditionally rewrites autoprewarm.blocks with only the
blocks currently in shared buffers, thus forgetting any blocks
that we hadn't reached yet while prewarming.  This seems quite
unhelpful, since the next database start will then not have the
expected prewarming benefit.  Fix it to not modify the file if
we shut down before the initial load attempt is complete.

Per bug #16785 from John Thompson.  Back-patch to v11 where
the autoprewarm code was introduced.

Discussion: https://postgr.es/m/16785-c0207d8c67fb5f25@postgresql.org

4 years agoIncrease timeout in 021_row_visibility.pl.
Tom Lane [Tue, 22 Dec 2020 16:10:12 +0000 (11:10 -0500)]
Increase timeout in 021_row_visibility.pl.

Commit 7b28913bc figured 30 seconds is long enough for anybody,
but in contexts like valgrind runs, it isn't necessarily.

4 years agoImprove find_em_expr_usable_for_sorting_rel comment
Tomas Vondra [Tue, 22 Dec 2020 01:00:39 +0000 (02:00 +0100)]
Improve find_em_expr_usable_for_sorting_rel comment

Clarify the relationship between find_em_expr_usable_for_sorting_rel and
prepare_sort_from_pathkeys, i.e. what restrictions need to be shared
between those two places.

Author: James Coleman
Reviewed-by: Tomas Vondra
Backpatch-through: 13
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs%3DhC0mSksZC%3DH5M8LSchj5e5OxpTAg%40mail.gmail.com

4 years agoDon't search for volatile expr in find_em_expr_usable_for_sorting_rel
Tomas Vondra [Mon, 21 Dec 2020 18:37:14 +0000 (19:37 +0100)]
Don't search for volatile expr in find_em_expr_usable_for_sorting_rel

While prepare_sort_from_pathkeys has to be concerned about matching up
a volatile expression to the proper tlist entry, we don't need to do
that in find_em_expr_usable_for_sorting_rel becausee such a sort will
have to be postponed anyway.

Author: James Coleman
Reviewed-by: Tomas Vondra
Backpatch-through: 13
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs%3DhC0mSksZC%3DH5M8LSchj5e5OxpTAg%40mail.gmail.com

4 years agoDisallow SRFs when considering sorts below Gather Merge
Tomas Vondra [Mon, 21 Dec 2020 17:58:32 +0000 (18:58 +0100)]
Disallow SRFs when considering sorts below Gather Merge

While we do allow SRFs in ORDER BY, scan/join processing should not
consider such cases - such sorts should only happen via final Sort atop
a ProjectSet. So make sure we don't try adding such sorts below Gather
Merge, just like we do for expressions that are volatile and/or not
parallel safe.

Backpatch to PostgreSQL 13, where this code was introduced as part of
the Incremental Sort patch.

Author: James Coleman
Reviewed-by: Tomas Vondra
Backpatch-through: 13
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs=hC0mSksZC=H5M8LSchj5e5OxpTAg@mail.gmail.com
Discussion: https://postgr.es/m/295524.1606246314%40sss.pgh.pa.us

4 years agoRemove "invalid concatenation of jsonb objects" error case.
Tom Lane [Mon, 21 Dec 2020 18:11:29 +0000 (13:11 -0500)]
Remove "invalid concatenation of jsonb objects" error case.

The jsonb || jsonb operator arbitrarily rejected certain combinations
of scalar and non-scalar inputs, while being willing to concatenate
other combinations.  This was of course quite undocumented.  Rather
than trying to document it, let's just remove the restriction,
creating a uniform rule that unless we are handling an object-to-object
concatenation, non-array inputs are converted to one-element arrays,
resulting in an array-to-array concatenation.  (This does not change
the behavior for any case that didn't throw an error before.)

Per complaint from Joel Jacobson.  Back-patch to all supported branches.

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

4 years agoCheck parallel safety in generate_useful_gather_paths
Tomas Vondra [Mon, 21 Dec 2020 17:29:46 +0000 (18:29 +0100)]
Check parallel safety in generate_useful_gather_paths

Commit ebb7ae839d ensured we ignore pathkeys with volatile expressions
when considering adding a sort below a Gather Merge. Turns out we need
to care about parallel safety of the pathkeys too, otherwise we might
try sorting e.g. on results of a correlated subquery (as demonstrated
by a report from Luis Roberto).

Initial investigation by Tom Lane, patch by James Coleman. Backpatch
to 13, where the code was instroduced (as part of Incremental Sort).

Reported-by: Luis Roberto
Author: James Coleman
Reviewed-by: Tomas Vondra
Backpatch-through: 13
Discussion: https://postgr.es/m/622580997.37108180.1604080457319.JavaMail.zimbra%40siscobra.com.br
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs=hC0mSksZC=H5M8LSchj5e5OxpTAg@mail.gmail.com

4 years agoConsider unsorted paths in generate_useful_gather_paths
Tomas Vondra [Mon, 21 Dec 2020 17:09:57 +0000 (18:09 +0100)]
Consider unsorted paths in generate_useful_gather_paths

generate_useful_gather_paths used to skip unsorted paths (without any
pathkeys), but that is unnecessary - the later code actually can handle
such paths just fine by adding a Sort node. This is clearly a thinko,
preventing construction of useful plans.

Backpatch to 13, where Incremental Sort was introduced.

Author: James Coleman
Reviewed-by: Tomas Vondra
Backpatch-through: 13
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs=hC0mSksZC=H5M8LSchj5e5OxpTAg@mail.gmail.com

4 years agoFix compiler warning in multirange_constructor0()
Alexander Korotkov [Mon, 21 Dec 2020 11:25:32 +0000 (14:25 +0300)]
Fix compiler warning in multirange_constructor0()

Discussion: https://postgr.es/m/X%2BBP8XE0UpIB6Yvh%40paquier.xyz
Author: Michael Paquier

4 years agoRefactor logic to check for ASCII-only characters in string
Michael Paquier [Mon, 21 Dec 2020 00:37:11 +0000 (09:37 +0900)]
Refactor logic to check for ASCII-only characters in string

The same logic was present for collation commands, SASLprep and
pgcrypto, so this removes some code.

Author: Michael Paquier
Reviewed-by: Stephen Frost, Heikki Linnakangas
Discussion: https://postgr.es/m/X9womIn6rne6Gud2@paquier.xyz

4 years agoFix typalign in rangetypes statistics
Alexander Korotkov [Sun, 20 Dec 2020 21:26:13 +0000 (00:26 +0300)]
Fix typalign in rangetypes statistics

6df7a9698b introduces multirange types, whose typanalyze function shares
infrastructure with range types typanalyze function.  Since 6df7a9698b,
information about type gathered by statistics is filled from typcache.
But typalign is mistakenly always set to double.  This commit fixes this
oversight.

4 years agoDoc: fix description of how to use src/tutorial files.
Tom Lane [Sun, 20 Dec 2020 20:28:22 +0000 (15:28 -0500)]
Doc: fix description of how to use src/tutorial files.

The separate "cd" command before invoking psql made sense (or at least
I thought so) when it was added in commit ed1939332.  But 4e3a61635
removed the supporting text that explained when to use it, making it
just confusing.  So drop it.

Also switch from four-dot to three-dot filler for the unsupplied
part of the path, since at least one person has read the four-dot
filler as a typo for "../..".  And fix these/those inconsistency.

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

4 years agoDoc: improve description of pgbench script weights.
Tom Lane [Sun, 20 Dec 2020 18:37:25 +0000 (13:37 -0500)]
Doc: improve description of pgbench script weights.

Point out the workaround to be used if you want to write a script
file name that includes "@".  Clean up the text a little.

Fabien Coelho, additional wordsmithing by me

Discussion: https://postgr.es/m/1c4e81550d214741827a03292222db8d@G08CNEXMBPEKD06.g08.fujitsu.local

4 years agoAvoid memcpy() with same source and destination in pgstat_recv_replslot.
Tom Lane [Sun, 20 Dec 2020 17:38:32 +0000 (12:38 -0500)]
Avoid memcpy() with same source and destination in pgstat_recv_replslot.

Same type of issue as in commit 53d4f5fef and earlier fixes; also
found by apparently-more-picky-than-the-buildfarm valgrind testing.
This one is an oversight in commit 986816750.  Since that's new in
HEAD, no need for a back-patch.

4 years agoFix compiler warning introduced in 6df7a9698b
Alexander Korotkov [Sun, 20 Dec 2020 13:27:01 +0000 (16:27 +0300)]
Fix compiler warning introduced in 6df7a9698b

4 years agoFixes for pg_dump.c regarding multiranges
Alexander Korotkov [Sun, 20 Dec 2020 05:14:35 +0000 (08:14 +0300)]
Fixes for pg_dump.c regarding multiranges

This commit fixes two wrong version number checks and one wrong check for null.

4 years agoMultirange datatypes
Alexander Korotkov [Sun, 20 Dec 2020 04:20:33 +0000 (07:20 +0300)]
Multirange datatypes

Multiranges are basically sorted arrays of non-overlapping ranges with
set-theoretic operations defined over them.

Since v14, each range type automatically gets a corresponding multirange
datatype.  There are both manual and automatic mechanisms for naming multirange
types.  Once can specify multirange type name using multirange_type_name
attribute in CREATE TYPE.  Otherwise, a multirange type name is generated
automatically.  If the range type name contains "range" then we change that to
"multirange".  Otherwise, we add "_multirange" to the end.

Implementation of multiranges comes with a space-efficient internal
representation format, which evades extra paddings and duplicated storage of
oids.  Altogether this format allows fetching a particular range by its index
in O(n).

Statistic gathering and selectivity estimation are implemented for multiranges.
For this purpose, stored multirange is approximated as union range without gaps.
This field will likely need improvements in the future.

Catversion is bumped.

Discussion: https://postgr.es/m/CALNJ-vSUpQ_Y%3DjXvTxt1VYFztaBSsWVXeF1y6gTYQ4bOiWDLgQ%40mail.gmail.com
Discussion: https://postgr.es/m/a0b8026459d1e6167933be2104a6174e7d40d0ab.camel%40j-davis.com#fe7218c83b08068bfffb0c5293eceda0
Author: Paul Jungwirth, revised by me
Reviewed-by: David Fetter, Corey Huinker, Jeff Davis, Pavel Stehule
Reviewed-by: Alvaro Herrera, Tom Lane, Isaac Morland, David G. Johnston
Reviewed-by: Zhihong Yu, Alexander Korotkov
4 years agoRemove now-useless ALWAYS_SUBDIRS entry in src/test/Makefile.
Tom Lane [Sat, 19 Dec 2020 22:58:30 +0000 (17:58 -0500)]
Remove now-useless ALWAYS_SUBDIRS entry in src/test/Makefile.

Commit 257836a75 added the "locale" subdirectory to SUBDIRS,
but neglected to remove it from ALWAYS_SUBDIRS.  This oversight
had no functional effect because the filter-out function would
remove it anyway.  Still, it's confusing to readers to list a
subdirectory in both places, especially because it makes the
associated comment into a partial lie.

4 years agoUpdate comment atop of ReorderBufferQueueMessage().
Amit Kapila [Sat, 19 Dec 2020 04:38:46 +0000 (10:08 +0530)]
Update comment atop of ReorderBufferQueueMessage().

The comments atop of this function describes behaviour in case of a
transactional WAL message only, but it accepts both transactional and
non-transactional WAL messages. Update the comments to describe
behaviour in case of non-transactional WAL message as well.

Ashutosh Bapat, rephrased by Amit Kapila
Discussion: https://postgr.es/m/CAGEoWWTTzNzHOi8bj0wfAo1siGi-YEh6wqH1oaz4DrkTJ6HbTQ@mail.gmail.com

4 years agoAdd a couple of missed .gitignore entries.
Tom Lane [Fri, 18 Dec 2020 21:24:49 +0000 (16:24 -0500)]
Add a couple of missed .gitignore entries.

Any subdirectory that's ignoring /output_iso/ should also
ignore /tmp_check_iso/, which could be left behind by a
failed pg_isolation_regress_check run.

I think these have been wrong for awhile, but it doesn't
seem important to fix in back branches.

4 years agoAvoid memcpy() with same source and destination during relmapper init.
Tom Lane [Fri, 18 Dec 2020 20:46:44 +0000 (15:46 -0500)]
Avoid memcpy() with same source and destination during relmapper init.

A narrow reading of the C standard says that memcpy(x,x,n) is undefined,
although it's hard to envision an implementation that would really
misbehave.  However, analysis tools such as valgrind might whine about
this; accordingly, let's band-aid relmapper.c to not do it.

See also 5b630501ed3f4e8a8aad7b48ea0, and other similar fixes.
Apparently, none of those folk tried valgrinding initdb?  This has been
like this for long enough that I'm surprised it hasn't been reported
before.

Back-patch, just in case anybody wants to use a back branch on a platform
that complains about this; we back-patched those earlier fixes too.

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

4 years agopg_stat_statements: Track time at which all statistics were last reset.
Fujii Masao [Fri, 18 Dec 2020 01:49:58 +0000 (10:49 +0900)]
pg_stat_statements: Track time at which all statistics were last reset.

This commit adds "stats_reset" column into the pg_stat_statements_info
view. This column indicates the time at which all statistics in the
pg_stat_statements view were last reset.

Per discussion, this commit also changes pg_stat_statements_info code
so that "dealloc" column is reset at the same time as "stats_reset" is reset,
i.e., whenever all pg_stat_statements entries are removed, for the sake
of consistency. Previously "dealloc" was reset only when
pg_stat_statements_reset(0, 0, 0) is called and was not reset when
pg_stat_statements_reset() with non-zero value argument discards all
entries. This was confusing.

Author: Naoki Nakamichi, Yuki Seino
Reviewed-by: Yuki Seino, Kyotaro Horiguchi, Li Japin, Fujii Masao
Discussion: https://postgr.es/m/c102cf3180d0ee73c1c5a0f7f8558322@oss.nttdata.com

4 years agoRevert "Get rid of the dedicated latch for signaling the startup process".
Fujii Masao [Thu, 17 Dec 2020 09:06:51 +0000 (18:06 +0900)]
Revert "Get rid of the dedicated latch for signaling the startup process".

Revert ac22929a26, as well as the followup fix 113d3591b8. Because it broke
the assumption that the startup process waiting for the recovery conflict
on buffer pin should be waken up only by buffer unpin or the timeout enabled
in ResolveRecoveryConflictWithBufferPin(). It caused, for example,
SIGHUP signal handler or walreceiver process to wake that startup process
up unnecessarily frequently.

Additionally, add the comments about why that dedicated latch that
the reverted patch tried to get rid of should not be removed.

Thanks to Kyotaro Horiguchi for the discussion.

Author: Fujii Masao
Discussion: https://postgr.es/m/d8c0c608-021b-3c73-fffd-3240829ee986@oss.nttdata.com

4 years agoFix varchar_2.out to match reality in cs_CZ locale.
Tom Lane [Wed, 16 Dec 2020 02:42:08 +0000 (21:42 -0500)]
Fix varchar_2.out to match reality in cs_CZ locale.

Seems to be a copy-and-pasteo in c06d6aa4c.  Per buildfarm.

4 years agodoc: Fix explanation related to pg_shmem_allocations
Michael Paquier [Wed, 16 Dec 2020 01:36:03 +0000 (10:36 +0900)]
doc: Fix explanation related to pg_shmem_allocations

Offsets are shown as NULL only for anonymous allocations.

Author: Benoit Lobréau
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/CAPE8EZ5Lnoyqoz7aZpvQM0E8sW+hw+k6G2NULe+m4arFRrA1aA@mail.gmail.com
Backpatch-through: 13

4 years agodoc: clarify COPY TO for partitioning/inheritance
Bruce Momjian [Wed, 16 Dec 2020 00:20:15 +0000 (19:20 -0500)]
doc:  clarify COPY TO for partitioning/inheritance

It was not clear how COPY TO behaved with partitioning/inheritance
because the paragraphs were so far apart.  Also reword to simplify.

Discussion: https://postgr.es/m/20201203211723.GR24052@telsasoft.com

Author: Justin Pryzby

Backpatch-through: 10

4 years agoRemove obsolete btrescan() comment.
Peter Geoghegan [Tue, 15 Dec 2020 23:55:07 +0000 (15:55 -0800)]
Remove obsolete btrescan() comment.

"Ordering stuff" refered to a _bt_first() call to _bt_orderkeys().
However, the _bt_orderkeys() function was renamed to
_bt_preprocess_keys() by commit fa5c8a055a0.

_bt_preprocess_keys() is directly referenced just after the removed
comment already, which seems sufficient.

4 years agoRemove useless variable stores
Alvaro Herrera [Tue, 15 Dec 2020 22:51:16 +0000 (19:51 -0300)]
Remove useless variable stores

Mistakenly introduced in 4cbe3ac3e867; bug repaired in 148e632c0541 but
the stores were accidentally.

4 years agoError out when Gather Merge input is not sorted
Tomas Vondra [Tue, 15 Dec 2020 22:13:38 +0000 (23:13 +0100)]
Error out when Gather Merge input is not sorted

To build Gather Merge path, the input needs to be sufficiently sorted.
Ensuring this is the responsibility of the code constructing the paths,
but create_gather_merge_plan tried to handle unsorted paths by adding
an explicit Sort. In light of the recent issues related to Incremental
Sort, this is rather fragile. Some of the expressions may be volatile
or parallel unsafe, in which case we can't add the Sort here.

We could do more checks and add the Sort in at least some cases, but
it seems cleaner to just error out and make it clear this is a bug in
code constructing those paths.

Author: James Coleman
Reviewed-by: Tomas Vondra
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs%3DhC0mSksZC%3DH5M8LSchj5e5OxpTAg%40mail.gmail.com
Discussion: https://postgr.es/m/CAJGNTeNaxpXgBVcRhJX%2B2vSbq%2BF2kJqGBcvompmpvXb7pq%2BoFA%40mail.gmail.com

4 years agoClean up ancient test style
Peter Eisentraut [Tue, 15 Dec 2020 20:54:06 +0000 (21:54 +0100)]
Clean up ancient test style

Many older tests where written in a style like

    SELECT '' AS two, i.* FROM INT2_TBL

where the first column indicated the number of expected result rows.
This has gotten increasingly out of date, as the test data fixtures
have expanded, so a lot of these were wrong and misleading.  Moreover,
this style isn't really necessary, since the psql output already shows
the number of result rows.

To clean this up, remove all those extra columns.

Discussion: https://www.postgresql.org/message-id/flat/1a25312b-2686-380d-3c67-7a69094a999f%40enterprisedb.com

4 years agoImprove hash_create()'s API for some added robustness.
Tom Lane [Tue, 15 Dec 2020 16:38:53 +0000 (11:38 -0500)]
Improve hash_create()'s API for some added robustness.

Invent a new flag bit HASH_STRINGS to specify C-string hashing, which
was formerly the default; and add assertions insisting that exactly
one of the bits HASH_STRINGS, HASH_BLOBS, and HASH_FUNCTION be set.
This is in hopes of preventing recurrences of the type of oversight
fixed in commit a1b8aa1e4 (i.e., mistakenly omitting HASH_BLOBS).

Also, when HASH_STRINGS is specified, insist that the keysize be
more than 8 bytes.  This is a heuristic, but it should catch
accidental use of HASH_STRINGS for integer or pointer keys.
(Nearly all existing use-cases set the keysize to NAMEDATALEN or
more, so there's little reason to think this restriction should
be problematic.)

Tweak hash_create() to insist that the HASH_ELEM flag be set, and
remove the defaults it had for keysize and entrysize.  Since those
defaults were undocumented and basically useless, no callers
omitted HASH_ELEM anyway.

Also, remove memset's zeroing the HASHCTL parameter struct from
those callers that had one.  This has never been really necessary,
and while it wasn't a bad coding convention it was confusing that
some callers did it and some did not.  We might as well save a few
cycles by standardizing on "not".

Also improve the documentation for hash_create().

In passing, improve reinit.c's usage of a hash table by storing
the key as a binary Oid rather than a string; and, since that's
a temporary hash table, allocate it in CurrentMemoryContext for
neatness.

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

4 years agoRevert "Cannot use WL_SOCKET_WRITEABLE without WL_SOCKET_READABLE."
Jeff Davis [Tue, 15 Dec 2020 07:47:30 +0000 (23:47 -0800)]
Revert "Cannot use WL_SOCKET_WRITEABLE without WL_SOCKET_READABLE."

This reverts commit 3a9e64aa0d96c8ffb6c682b082d0f72b1d373327.

Commit 4bad60e3 fixed the root of the problem that 3a9e64aa worked
around.

This enables proper pipelining of commands after terminating
replication, eliminating an undocumented limitation.

Discussion: https://postgr.es/m/3d57bc29-4459-578b-79cb-7641baf53c57%40iki.fi
Backpatch-through: 9.5

4 years agoImprove some code around cryptohash functions
Michael Paquier [Mon, 14 Dec 2020 03:38:13 +0000 (12:38 +0900)]
Improve some code around cryptohash functions

This adjusts some code related to recent changes for cryptohash
functions:
- Add a variable in md5.h to track down the size of a computed result,
moved from pgcrypto.  Note that pg_md5_hash() assumed a result of this
size already.
- Call explicit_bzero() on the hashed data when freeing the context for
fallback implementations.  For MD5, particularly, it would be annoying
to leave some non-zeroed data around.
- Clean up some code related to recent changes of uuid-ossp.  .gitignore
still included md5.c and a comment was incorrect.

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

4 years agoAdd some checkpoint/restartpoint status to ps display
Michael Paquier [Mon, 14 Dec 2020 02:53:58 +0000 (11:53 +0900)]
Add some checkpoint/restartpoint status to ps display

This is done for end-of-recovery and shutdown checkpoints/restartpoints
(end-of-recovery restartpoints don't exist) rather than all types of
checkpoints, in cases where it may not be possible to rely on
pg_stat_activity to get a status from the startup or checkpointer
processes.

For example, at the end of a crash recovery, this is useful to know if a
checkpoint is running in the startup process, while previously the ps
display may only show some information about "recovering" something,
that can be confusing while a checkpoint runs.

Author: Justin Pryzby
Reviewed-by: Nathan Bossart, Kirk Jamison, Fujii Masao, Michael Paquier
Discussion: https://postgr.es/m/20200818225238.GP17022@telsasoft.com

4 years agoUse HASH_BLOBS for xidhash.
Noah Misch [Sun, 13 Dec 2020 05:38:36 +0000 (21:38 -0800)]
Use HASH_BLOBS for xidhash.

This caused BufFile errors on buildfarm member sungazer, and SIGSEGV was
possible.  Conditions for reaching those symptoms were more frequent on
big-endian systems.

Discussion: https://postgr.es/m/20201129214441.GA691200@rfd.leadboat.com

4 years agoCorrect behavior descriptions in comments, and correct a test name.
Noah Misch [Sun, 13 Dec 2020 04:12:25 +0000 (20:12 -0800)]
Correct behavior descriptions in comments, and correct a test name.

4 years agoinitdb: complete getopt_long alphabetization
Bruce Momjian [Sat, 12 Dec 2020 17:59:09 +0000 (12:59 -0500)]
initdb:  complete getopt_long alphabetization

Backpatch-through: 9.5

4 years agoinitdb: properly alphabetize getopt_long options in C string
Bruce Momjian [Sat, 12 Dec 2020 17:51:16 +0000 (12:51 -0500)]
initdb:  properly alphabetize getopt_long options in C string

Backpatch-through: 9.5

4 years agoAllow subscripting of hstore values.
Tom Lane [Fri, 11 Dec 2020 23:58:07 +0000 (18:58 -0500)]
Allow subscripting of hstore values.

This is basically a finger exercise to prove that it's possible for
an extension module to add subscripting ability.  Subscripted fetch
from an hstore is not different from the existing "hstore -> text"
operator.  Subscripted update does seem to be a little easier to
use than the traditional update method using hstore concatenation,
but it's not a fundamentally new ability.

However, there may be some value in the code as sample code, since
it shows what's basically the minimum-complexity way to implement
subscripting when one needn't consider nested container objects.

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

4 years agoAllow ALTER TYPE to update an existing type's typsubscript value.
Tom Lane [Fri, 11 Dec 2020 23:07:02 +0000 (18:07 -0500)]
Allow ALTER TYPE to update an existing type's typsubscript value.

This is essential if we'd like to allow existing extension data types
to support subscripting in future, since dropping and recreating the
type isn't a practical thing for an extension upgrade script, and
direct manipulation of pg_type isn't a great answer either.

There was some discussion about also allowing alteration of typelem,
but it's less clear whether that's a good idea or not, so for now
I forebore.

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

4 years agoProvide an error cursor for "can't subscript" error messages.
Tom Lane [Fri, 11 Dec 2020 22:54:10 +0000 (17:54 -0500)]
Provide an error cursor for "can't subscript" error messages.

Commit c7aba7c14 didn't add this, but after more fooling with the
feature I feel that it'd be useful.  To make this possible, refactor
getSubscriptingRoutines() so that the caller is responsible for
throwing any error.  (In clauses.c, I just chose to make the
most conservative assumption rather than throwing an error.  We don't
expect failures there anyway really, so the code space for an error
message would be a poor investment.)

4 years agopg_dump: Don't use enums for defining bit mask values
Peter Eisentraut [Fri, 11 Dec 2020 18:15:30 +0000 (19:15 +0100)]
pg_dump: Don't use enums for defining bit mask values

This usage would mean that values of the enum type are potentially not
one of the enum values.  Use macros instead, like everywhere else.

Discussion: https://www.postgresql.org/message-id/14dde730-1d34-260e-fa9d-7664df2d6313@enterprisedb.com

4 years agoFix compilation of uuid-ossp
Michael Paquier [Thu, 10 Dec 2020 03:49:43 +0000 (12:49 +0900)]
Fix compilation of uuid-ossp

This module had a dependency on pgcrypto's md5.c that got removed by
b67b57a.  Instead of the code from pgcrypto, this code can just use the
new cryptohash routines for MD5 as a drop-in replacement, so let's just
do this switch.  This has also the merit to simplify a bit the
compilation of uuid-ossp.

This requires --with-uuid to be reproduced, and I have used e2fs as a
way to reproduce the failure, then test this commit.

Per reports from buildfarm members longfin, florican and sifaka.

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

4 years agoRefactor MD5 implementations according to new cryptohash infrastructure
Michael Paquier [Thu, 10 Dec 2020 02:59:10 +0000 (11:59 +0900)]
Refactor MD5 implementations according to new cryptohash infrastructure

This commit heavily reorganizes the MD5 implementations that exist in
the tree in various aspects.

First, MD5 is added to the list of options available in cryptohash.c and
cryptohash_openssl.c.  This means that if building with OpenSSL, EVP is
used for MD5 instead of the fallback implementation that Postgres had
for ages.  With the recent refactoring work for cryptohash functions,
this change is straight-forward.  If not building with OpenSSL, a
fallback implementation internal to src/common/ is used.

Second, this reduces the number of MD5 implementations present in the
tree from two to one, by moving the KAME implementation from pgcrypto to
src/common/, and by removing the implementation that existed in
src/common/.  KAME was already structured with an init/update/final set
of routines by pgcrypto (see original pgcrypto/md5.h) for compatibility
with OpenSSL, so moving it to src/common/ has proved to be a
straight-forward move, requiring no actual manipulation of the internals
of each routine.  Some benchmarking has not shown any performance gap
between both implementations.

Similarly to the fallback implementation used for SHA2, the fallback
implementation of MD5 is moved to src/common/md5.c with an internal
header called md5_int.h for the init, update and final routines.  This
gets then consumed by cryptohash.c.

The original routines used for MD5-hashed passwords are moved to a
separate file called md5_common.c, also in src/common/, aimed at being
shared between all MD5 implementations as utility routines to keep
compatibility with any code relying on them.

Like the SHA2 changes, this commit had its round of tests on both Linux
and Windows, across all versions of OpenSSL supported on HEAD, with and
even without OpenSSL.

Author: Michael Paquier
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/20201106073434.GA4961@paquier.xyz

4 years agoSupport subscripting of arbitrary types, not only arrays.
Tom Lane [Wed, 9 Dec 2020 17:40:37 +0000 (12:40 -0500)]
Support subscripting of arbitrary types, not only arrays.

This patch generalizes the subscripting infrastructure so that any
data type can be subscripted, if it provides a handler function to
define what that means.  Traditional variable-length (varlena) arrays
all use array_subscript_handler(), while the existing fixed-length
types that support subscripting use raw_array_subscript_handler().
It's expected that other types that want to use subscripting notation
will define their own handlers.  (This patch provides no such new
features, though; it only lays the foundation for them.)

To do this, move the parser's semantic processing of subscripts
(including coercion to whatever data type is required) into a
method callback supplied by the handler.  On the execution side,
replace the ExecEvalSubscriptingRef* layer of functions with direct
calls to callback-supplied execution routines.  (Thus, essentially
no new run-time overhead should be caused by this patch.  Indeed,
there is room to remove some overhead by supplying specialized
execution routines.  This patch does a little bit in that line,
but more could be done.)

Additional work is required here and there to remove formerly
hard-wired assumptions about the result type, collation, etc
of a SubscriptingRef expression node; and to remove assumptions
that the subscript values must be integers.

One useful side-effect of this is that we now have a less squishy
mechanism for identifying whether a data type is a "true" array:
instead of wiring in weird rules about typlen, we can look to see
if pg_type.typsubscript == F_ARRAY_SUBSCRIPT_HANDLER.  For this
to be bulletproof, we have to forbid user-defined types from using
that handler directly; but there seems no good reason for them to
do so.

This patch also removes assumptions that the number of subscripts
is limited to MAXDIM (6), or indeed has any hard-wired limit.
That limit still applies to types handled by array_subscript_handler
or raw_array_subscript_handler, but to discourage other dependencies
on this constant, I've moved it from c.h to utils/array.h.

Dmitry Dolgov, reviewed at various times by Tom Lane, Arthur Zakirov,
Peter Eisentraut, Pavel Stehule

Discussion: https://postgr.es/m/CA+q6zcVDuGBv=M0FqBYX8DPebS3F_0KQ6OVFobGJPM507_SZ_w@mail.gmail.com
Discussion: https://postgr.es/m/CA+q6zcVovR+XY4mfk-7oNk-rF91gH0PebnNfuUjuuDsyHjOcVA@mail.gmail.com

4 years agoChange get_constraint_index() to use pg_constraint.conindid
Peter Eisentraut [Wed, 9 Dec 2020 14:12:05 +0000 (15:12 +0100)]
Change get_constraint_index() to use pg_constraint.conindid

It was still using a scan of pg_depend instead of using the conindid
column that has been added since.

Since it is now just a catalog lookup wrapper and not related to
pg_depend, move from pg_depend.c to lsyscache.c.

Reviewed-by: Matthias van de Meent <boekewurm+postgres@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/4688d55c-9a2e-9a5a-d166-5f24fe0bf8db%40enterprisedb.com

4 years agoSimplify code for getting a unicode codepoint's canonical class.
Michael Paquier [Wed, 9 Dec 2020 04:24:38 +0000 (13:24 +0900)]
Simplify code for getting a unicode codepoint's canonical class.

Three places of unicode_norm.c use a similar logic for getting the
combining class from a codepoint.  Commit 2991ac5 has added the function
get_canonical_class() for this purpose, but it was only called by the
backend.  This commit refactors the code to use this function in all
the places where the combining class is retrieved from a given
codepoint.

Author: John Naylor
Discussion: https://postgr.es/m/CAFBsxsHUV7s7YrOm6hFz-Jq8Sc7K_yxTkfNZxsDV-DuM-k-gwg@mail.gmail.com

4 years agojit: Reference function pointer types via llvmjit_types.c.
Andres Freund [Mon, 7 Dec 2020 21:16:55 +0000 (13:16 -0800)]
jit: Reference function pointer types via llvmjit_types.c.

It is error prone (see 5da871bfa1b) and verbose to manually create function
types. Add a helper that can reference a function pointer type via
llvmjit_types.c and and convert existing instances of manual creation.

Author: Andres Freund <andres@anarazel.de>
Reviewed-By: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/20201207212142.wz5tnbk2jsaqzogb@alap3.anarazel.de

4 years agoTeach contain_leaked_vars that assignment SubscriptingRefs are leaky.
Tom Lane [Tue, 8 Dec 2020 22:50:54 +0000 (17:50 -0500)]
Teach contain_leaked_vars that assignment SubscriptingRefs are leaky.

array_get_element and array_get_slice qualify as leakproof, since
they will silently return NULL for bogus subscripts.  But
array_set_element and array_set_slice throw errors for such cases,
making them clearly not leakproof.  contain_leaked_vars was evidently
written with only the former case in mind, as it gave the wrong answer
for assignment SubscriptingRefs (nee ArrayRefs).

This would be a live security bug, were it not that assignment
SubscriptingRefs can only occur in INSERT and UPDATE target lists,
while we only care about leakproofness for qual expressions; so the
wrong answer can't occur in practice.  Still, that's a rather shaky
answer for a security-related question; and maybe in future somebody
will want to ask about leakproofness of a tlist.  So it seems wise to
fix and even back-patch this correction.

(We would need some change here anyway for the upcoming
generic-subscripting patch, since extensions might make different
tradeoffs about whether to throw errors.  Commit 558d77f20 attempted
to lay groundwork for that by asking check_functions_in_node whether a
SubscriptingRef contains leaky functions; but that idea fails now that
the implementation methods of a SubscriptingRef are not SQL-visible
functions that could be marked leakproof or not.)

Back-patch to 9.6.  While 9.5 has the same issue, the code's a bit
different.  It seems quite unlikely that we'd introduce any actual bug
in the short time 9.5 has left to live, so the work/risk/reward balance
isn't attractive for changing 9.5.

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

4 years agoRemove operator_precedence_warning.
Tom Lane [Tue, 8 Dec 2020 21:29:52 +0000 (16:29 -0500)]
Remove operator_precedence_warning.

This GUC was always intended as a temporary solution to help with
finding 9.4-to-9.5 migration issues.  Now that all pre-9.5 branches
are out of support, and 9.5 will be too before v14 is released,
it seems like it's okay to drop it.  Doing so allows removal of
several hundred lines of poorly-tested code in parse_expr.c,
which have been a fertile source of bugs when people did use this.

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

4 years agoImprove estimation of ANDs under ORs using extended statistics.
Dean Rasheed [Tue, 8 Dec 2020 20:10:11 +0000 (20:10 +0000)]
Improve estimation of ANDs under ORs using extended statistics.

Formerly, extended statistics only handled clauses that were
RestrictInfos. However, the restrictinfo machinery doesn't create
sub-AND RestrictInfos for AND clauses underneath OR clauses.
Therefore teach extended statistics to handle bare AND clauses,
looking for compatible RestrictInfo clauses underneath them.

Dean Rasheed, reviewed by Tomas Vondra.

Discussion: https://postgr.es/m/CAEZATCW=J65GUFm50RcPv-iASnS2mTXQbr=CfBvWRVhFLJ_fWA@mail.gmail.com

4 years agoImprove estimation of OR clauses using multiple extended statistics.
Dean Rasheed [Tue, 8 Dec 2020 19:39:24 +0000 (19:39 +0000)]
Improve estimation of OR clauses using multiple extended statistics.

When estimating an OR clause using multiple extended statistics
objects, treat the estimates for each set of clauses for each
statistics object as independent of one another. The overlap estimates
produced for each statistics object do not apply to clauses covered by
other statistics objects.

Dean Rasheed, reviewed by Tomas Vondra.

Discussion: https://postgr.es/m/CAEZATCW=J65GUFm50RcPv-iASnS2mTXQbr=CfBvWRVhFLJ_fWA@mail.gmail.com

4 years agoDoc: clarify that CREATE TABLE discards redundant unique constraints.
Tom Lane [Tue, 8 Dec 2020 18:09:47 +0000 (13:09 -0500)]
Doc: clarify that CREATE TABLE discards redundant unique constraints.

The SQL standard says that redundant unique constraints are disallowed,
but we long ago decided that throwing an error would be too
user-unfriendly, so we just drop redundant ones.  The docs weren't very
clear about that though, as this behavior was only explained for PRIMARY
KEY vs UNIQUE, not UNIQUE vs UNIQUE.

While here, I couldn't resist doing some copy-editing and markup-fixing
on the adjacent text about INCLUDE options.

Per bug #16767 from Matthias vd Meent.

Discussion: https://postgr.es/m/16767-1714a2056ca516d0@postgresql.org

4 years agoDoc: explain that the string types can't store \0 (ASCII NUL).
Tom Lane [Tue, 8 Dec 2020 17:06:19 +0000 (12:06 -0500)]
Doc: explain that the string types can't store \0 (ASCII NUL).

This restriction was mentioned in connection with string literals,
but it wasn't made clear that it's a general restriction not just
a syntactic limitation in query strings.

Per unsigned documentation comment.

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

4 years agoSpeed up rechecking if relation needs to be vacuumed or analyze in autovacuum.
Fujii Masao [Tue, 8 Dec 2020 14:59:39 +0000 (23:59 +0900)]
Speed up rechecking if relation needs to be vacuumed or analyze in autovacuum.

After autovacuum collects the relations to vacuum or analyze, it rechecks
whether each relation still needs to be vacuumed or analyzed before actually
doing that. Previously this recheck could be a significant overhead
especially when there were a very large number of relations. This was
because each recheck forced the statistics to be refreshed, and the refresh
of the statistics for a very large number of relations could cause heavy
overhead. There was the report that this issue caused autovacuum workers
to have gotten “stuck” in a tight loop of table_recheck_autovac() that
rechecks whether a relation needs to be vacuumed or analyzed.

This commit speeds up the recheck by making autovacuum worker reuse
the previously-read statistics for the recheck if possible. Then if that
"stale" statistics says that a relation still needs to be vacuumed or analyzed,
autovacuum refreshes the statistics and does the recheck again.

The benchmark shows that the more relations exist and autovacuum workers
are running concurrently, the more this change reduces the autovacuum
execution time. For example, when there are 20,000 tables and 10 autovacuum
workers are running, the benchmark showed that the change improved
the performance of autovacuum more than three times. On the other hand,
even when there are only 1000 tables and only a single autovacuum worker
is running, the benchmark didn't show any big performance regression by
the change.

Firstly POC patch was proposed by Jim Nasby. As the result of discussion,
we used Tatsuhito Kasahara's version of the patch using the approach
suggested by Tom Lane.

Reported-by: Jim Nasby
Author: Tatsuhito Kasahara
Reviewed-by: Masahiko Sawada, Fujii Masao
Discussion: https://postgr.es/m/3FC6C2F2-8A47-44C0-B997-28830B5716D0@amazon.com

4 years agoBump catversion for pg_stat_wal changes.
Fujii Masao [Tue, 8 Dec 2020 12:05:27 +0000 (21:05 +0900)]
Bump catversion for pg_stat_wal changes.

Oversight in 01469241b2.

Reported-by: Andres Freund
Discussion: https://postgr.es/m/20201207185614.zzf63vggm5r4sozg@alap3.anarazel.de

4 years agopgcrypto: Detect errors with EVP calls from OpenSSL
Michael Paquier [Tue, 8 Dec 2020 06:22:12 +0000 (15:22 +0900)]
pgcrypto: Detect errors with EVP calls from OpenSSL

The following routines are called within pgcrypto when handling digests
but there were no checks for failures:
- EVP_MD_CTX_size (can fail with -1 as of 3.0.0)
- EVP_MD_CTX_block_size (can fail with -1 as of 3.0.0)
- EVP_DigestInit_ex
- EVP_DigestUpdate
- EVP_DigestFinal_ex

A set of elog(ERROR) is added by this commit to detect such failures,
that should never happen except in the event of a processing failure
internal to OpenSSL.

Note that it would be possible to use ERR_reason_error_string() to get
more context about such errors, but these refer mainly to the internals
of OpenSSL, so it is not really obvious how useful that would be.  This
is left out for simplicity.

Per report from Coverity.  Thanks to Tom Lane for the discussion.

Backpatch-through: 9.5

4 years agojit: Correct parameter type for generated expression evaluation functions.
Andres Freund [Tue, 8 Dec 2020 02:21:06 +0000 (18:21 -0800)]
jit: Correct parameter type for generated expression evaluation functions.

clang only uses the 'i1' type for scalar booleans, not for pointers to
booleans (as the pointer might be pointing into a larger memory
allocation). Therefore a pointer-to-bool needs to the "storage" boolean.

There's no known case of wrong code generation due to this, but it seems quite
possible that it could cause problems (see e.g. 72559438f92).

Author: Andres Freund
Discussion: https://postgr.es/m/20201207212142.wz5tnbk2jsaqzogb@alap3.anarazel.de
Backpatch: 11-, where jit support was added

4 years agojit: configure: Explicitly reference 'native' component.
Andres Freund [Tue, 8 Dec 2020 02:12:23 +0000 (18:12 -0800)]
jit: configure: Explicitly reference 'native' component.

Until recently 'native' was implicitly included via 'orcjit', but a change
included in LLVM 11 (not yet released) removed a number of such indirect
component references.

Reported-By: Fabien COELHO <coelho@cri.ensmp.fr>
Reported-By: Andres Freund <andres@anarazel.de>
Reported-By: Thomas Munro <thomas.munro@gmail.com>
Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20201201064949.mex6kvi2kygby3ni@alap3.anarazel.de
Backpatch: 11-, where jit support was added

4 years agoAvoid using tuple from syscache for update of pg_database.datfrozenxid
Michael Paquier [Tue, 8 Dec 2020 03:13:19 +0000 (12:13 +0900)]
Avoid using tuple from syscache for update of pg_database.datfrozenxid

pg_database.datfrozenxid gets updated using an in-place update at the
end of vacuum or autovacuum.  Since 96cdeae, as pg_database has a toast
relation, it is possible for a pg_database tuple to have toast values
if there is a large set of ACLs in place.  In such a case, the in-place
update would fail because of the flattening of the toast values done for
the catcache entry fetched.  Instead of using a copy from the catcache,
this changes the logic to fetch the copy of the tuple by directly
scanning pg_database.

Per the lack of complaints on the matter, no backpatch is done.  Note
that before 96cdeae, attempting to insert such a tuple to pg_database
would cause a "row is too big" error, so the end-of-vacuum problem was
not reachable.

Author: Ashwin Agrawal, Junfeng Yang
Discussion: https://postgr.es/m/DM5PR0501MB38800D9E4605BCA72DD35557CCE10@DM5PR0501MB3880.namprd05.prod.outlook.com

4 years agoAdd a couple of regression test cases related to array subscripting.
Tom Lane [Mon, 7 Dec 2020 16:10:21 +0000 (11:10 -0500)]
Add a couple of regression test cases related to array subscripting.

Exercise some error cases that were never reached in the existing
regression tests.  This is partly for code-coverage reasons, and
partly to memorialize the current behavior in advance of planned
changes for generic subscripting.

Also, I noticed that type_sanity's check to verify that all standard
types have array types was never extended when we added arrays for
all system catalog rowtypes (f7f70d5e2), nor when we added arrays
over domain types (c12d570fa).  So do that.  Also, since the query's
expected output isn't empty, it seems like a good idea to add an
ORDER BY to make sure the result stays stable.

4 years agoFix more race conditions in the newly-added pg_rewind test.
Heikki Linnakangas [Mon, 7 Dec 2020 12:44:34 +0000 (14:44 +0200)]
Fix more race conditions in the newly-added pg_rewind test.

pg_rewind looks at the control file to check what timeline a server is on.
But promotion doesn't immediately write a checkpoint, it merely writes
an end-of-recovery WAL record. If pg_rewind runs immediately after
promotion, before the checkpoint has completed, it will think think that
the server is still on the earlier timeline. We ran into this issue a long
time ago already, see commit 484a848a73f.

It's a bit bogus that pg_rewind doesn't determine the timeline correctly
until the end-of-recovery checkpoint has completed. We probably should
fix that. But for now work around it by waiting for the checkpoint
to complete before running pg_rewind, like we did in commit 484a848a73f.

In the passing, tidy up the new test a little bit. Rerder the INSERTs so
that the comments make more sense, remove a spurious CHECKPOINT call after
pg_rewind has already run, and add --debug option, so that if this fails
again, we'll have more data.

Per buildfarm failure at https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=rorqual&dt=2020-12-06%2018%3A32%3A19&stg=pg_rewind-check.
Backpatch to all supported versions.

Discussion: https://www.postgresql.org/message-id/1713707e-e318-761c-d287-5b6a4aa807e8@iki.fi

4 years agopg_dump: Reorganize dumpBaseType()
Tom Lane [Mon, 7 Dec 2020 03:37:40 +0000 (22:37 -0500)]
pg_dump: Reorganize dumpBaseType()

Along the same lines as ed2c7f65b and daa9fe8a5, reduce code duplication
by having just one copy of the parts of the query that are the same
across all server versions; and make the conditionals control the
smallest possible amount of code.  This is in preparation for adding
another dumpable field to pg_type.

4 years agoFix fd leak in pg_verifybackup
Michael Paquier [Mon, 7 Dec 2020 00:29:32 +0000 (09:29 +0900)]
Fix fd leak in pg_verifybackup

An error code path newly-introduced by 87ae969 forgot to close a file
descriptor when verifying a file's checksum.

Per report from Coverity, via Tom Lane.

4 years agoFix missed step in removal of useless RESULT RTEs in the planner.
Tom Lane [Sat, 5 Dec 2020 21:16:13 +0000 (16:16 -0500)]
Fix missed step in removal of useless RESULT RTEs in the planner.

Commit 4be058fe9 forgot that the append_rel_list would already be
populated at the time we remove useless result RTEs, and it might contain
PlaceHolderVars that need to be adjusted like the ones in the main parse
tree.  This could lead to "no relation entry for relid N" failures later
on, when the planner tries to do something with an unadjusted PHV.

Per report from Tom Ellis.  Back-patch to v12 where the bug came in.

Discussion: https://postgr.es/m/20201205173056.GF30712@cloudinit-builder

4 years agoFix race conditions in newly-added test.
Heikki Linnakangas [Fri, 4 Dec 2020 16:20:18 +0000 (18:20 +0200)]
Fix race conditions in newly-added test.

Buildfarm has been failing sporadically on the new test.  I was able to
reproduce this by adding a random 0-10 s delay in the walreceiver, just
before it connects to the primary. There's a race condition where node_3
is promoted before it has fully caught up with node_1, leading to diverged
timelines. When node_1 is later reconfigured as standby following node_3,
it fails to catch up:

LOG:  primary server contains no more WAL on requested timeline 1
LOG:  new timeline 2 forked off current database system timeline 1 before current recovery point 0/30000A0

That's the situation where you'd need to use pg_rewind, but in this case
it happens already when we are just setting up the actual pg_rewind
scenario we want to test, so change the test so that it waits until
node_3 is connected and fully caught up before promoting it, so that you
get a clean, controlled failover.

Also rewrite some of the comments, for clarity. The existing comments
detailed what each step in the test did, but didn't give a good overview
of the situation the steps were trying to create.

For reasons I don't understand, the test setup had to be written slightly
differently in 9.6 and 9.5 than in later versions. The 9.5/9.6 version
needed node 1 to be reinitialized from backup, whereas in later versions
it could be shut down and reconfigured to be a standby. But even 9.5 should
support "clean switchover", where primary makes sure that pending WAL is
replicated to standby on shutdown. It would be nice to figure out what's
going on there, but that's independent of pg_rewind and the scenario that
this test tests.

Discussion: https://www.postgresql.org/message-id/b0a3b95b-82d2-6089-6892-40570f8c5e60%40iki.fi

4 years agoConvert elog(LOG) calls to ereport() where appropriate
Peter Eisentraut [Fri, 4 Dec 2020 13:25:23 +0000 (14:25 +0100)]
Convert elog(LOG) calls to ereport() where appropriate

User-visible log messages should go through ereport(), so they are
subject to translation.  Many remaining elog(LOG) calls are really
debugging calls.

Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://www.postgresql.org/message-id/flat/92d6f545-5102-65d8-3c87-489f71ea0a37%40enterprisedb.com

4 years agoRemove unnecessary grammar symbols
Peter Eisentraut [Fri, 4 Dec 2020 10:09:29 +0000 (11:09 +0100)]
Remove unnecessary grammar symbols

Instead of publication_name_list, we can use name_list.  We already
refer to publications everywhere else by the 'name' or 'name_list'
symbols, so this only improves consistency.

Reviewed-by: https://www.postgresql.org/message-id/flat/3e3ccddb-41bd-ecd8-29fe-195e34d9886f%40enterprisedb.com
Discussion: Tom Lane <tgl@sss.pgh.pa.us>

4 years agoRemove incorrect assertion in reorderbuffer.c.
Amit Kapila [Fri, 4 Dec 2020 08:24:50 +0000 (13:54 +0530)]
Remove incorrect assertion in reorderbuffer.c.

We start recording changes in ReorderBufferTXN even before we reach
SNAPBUILD_CONSISTENT state so that if the commit is encountered after
reaching that we should be able to send the changes of the entire transaction.
Now, while recording changes if the reorder buffer memory has exceeded
logical_decoding_work_mem then we can start streaming if it is allowed and
we haven't yet streamed that data. However, we must not allow streaming to
start unless the snapshot has reached SNAPBUILD_CONSISTENT state.

In passing, improve the comments atop ReorderBufferResetTXN to mention the
case when we need to continue streaming after getting an error.

Author: Amit Kapila
Reviewed-by: Dilip Kumar
Discussion: https://postgr.es/m/CAA4eK1KoOH0byboyYY40NBcC7Fe812trwTa+WY3jQF7WQWZbQg@mail.gmail.com

4 years agoRename cryptohashes.c to cryptohashfuncs.c
Michael Paquier [Fri, 4 Dec 2020 03:58:44 +0000 (12:58 +0900)]
Rename cryptohashes.c to cryptohashfuncs.c

87ae969 has created two new files called cryptohash{_openssl}.c in
src/common/, whose names overlap with the existing backend file called
cryptohashes.c dedicated to the SQL wrappers for SHA2 and MD5.  This
file is renamed to cryptohashfuncs.c to be more consistent with the
surroundings and reduce the confusion with the new cryptohash interface
of src/common/.

Author: Michael Paquier
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/X8hHhaQgbMbW+aGU@paquier.xyz

4 years agoChange SHA2 implementation based on OpenSSL to use EVP digest routines
Michael Paquier [Fri, 4 Dec 2020 01:49:23 +0000 (10:49 +0900)]
Change SHA2 implementation based on OpenSSL to use EVP digest routines

The use of low-level hash routines is not recommended by upstream
OpenSSL since 2000, and pgcrypto already switched to EVP as of 5ff4a67.
This takes advantage of the refactoring done in 87ae969 that has
introduced the allocation and free routines for cryptographic hashes.

Since 1.1.0, OpenSSL does not publish the contents of the cryptohash
contexts, forcing any consumers to rely on OpenSSL for all allocations.
Hence, the resource owner callback mechanism gains a new set of routines
to track and free cryptohash contexts when using OpenSSL, preventing any
risks of leaks in the backend.  Nothing is needed in the frontend thanks
to the refactoring of 87ae969, and the resowner knowledge is isolated
into cryptohash_openssl.c.

Note that this also fixes a failure with SCRAM authentication when using
FIPS in OpenSSL, but as there have been few complaints about this
problem and as this causes an ABI breakage, no backpatch is done.

Author: Michael Paquier
Reviewed-by: Daniel Gustafsson, Heikki Linnakangas
Discussion: https://postgr.es/m/20200924025314.GE7405@paquier.xyz
Discussion: https://postgr.es/m/20180911030250.GA27115@paquier.xyz

4 years agodoc: remove unnecessary blank before command option text
Bruce Momjian [Thu, 3 Dec 2020 16:33:24 +0000 (11:33 -0500)]
doc: remove unnecessary blank before command option text

Backpatch-through: 11

4 years agodocs: list single-letter options first in command-line summary
Bruce Momjian [Thu, 3 Dec 2020 15:28:58 +0000 (10:28 -0500)]
docs:  list single-letter options first in command-line summary

In a few places, the long-version options were listed before the
single-letter ones in the command summary of a few commands.  This
didn't match other commands, and didn't match the option ordering later
in the same reference page.

Backpatch-through: 9.5

4 years agoFix pg_rewind bugs when rewinding a standby server.
Heikki Linnakangas [Thu, 3 Dec 2020 13:57:48 +0000 (15:57 +0200)]
Fix pg_rewind bugs when rewinding a standby server.

If the target is a standby server, its WAL doesn't end at the last
checkpoint record, but at minRecoveryPoint. We must scan all the
WAL from the last common checkpoint all the way up to minRecoveryPoint
for modified pages, and also consider that portion when determining
whether the server needs rewinding.

Backpatch to all supported versions.

Author: Ian Barwick and me
Discussion: https://www.postgresql.org/message-id/CABvVfJU-LDWvoz4-Yow3Ay5LZYTuPD7eSjjE4kGyNZpXC6FrVQ%40mail.gmail.com

4 years agoSmall code simplifications
Peter Eisentraut [Thu, 3 Dec 2020 10:35:20 +0000 (11:35 +0100)]
Small code simplifications

strVal() can be used in a couple of places instead of coding the same
thing by hand.

4 years agoImprove estimation of OR clauses using extended statistics.
Dean Rasheed [Thu, 3 Dec 2020 10:03:49 +0000 (10:03 +0000)]
Improve estimation of OR clauses using extended statistics.

Formerly we only applied extended statistics to an OR clause as part
of the clauselist_selectivity() code path for an OR clause appearing
in an implicitly-ANDed list of clauses. This meant that it could only
use extended statistics if all sub-clauses of the OR clause were
covered by a single extended statistics object.

Instead, teach clause_selectivity() how to apply extended statistics
to an OR clause by handling its ORed list of sub-clauses in a similar
manner to an implicitly-ANDed list of sub-clauses, but with different
combination rules. This allows one or more extended statistics objects
to be used to estimate all or part of the list of sub-clauses. Any
remaining sub-clauses are then treated as if they are independent.

Additionally, to avoid double-application of extended statistics, this
introduces "extended" versions of clause_selectivity() and
clauselist_selectivity(), which include an option to ignore extended
statistics. This replaces the old clauselist_selectivity_simple()
function which failed to completely ignore extended statistics when
called from the extended statistics code.

A known limitation of the current infrastructure is that an AND clause
under an OR clause is not treated as compatible with extended
statistics (because we don't build RestrictInfos for such sub-AND
clauses). Thus, for example, "(a=1 AND b=1) OR (a=2 AND b=2)" will
currently be treated as two independent AND clauses (each of which may
be estimated using extended statistics), but extended statistics will
not currently be used to account for any possible overlap between
those clauses. Improving that is left as a task for the future.

Original patch by Tomas Vondra, with additional improvements by me.

Discussion: https://postgr.es/m/20200113230008.g67iyk4cs3xbnjju@development

4 years agoRefactor CLUSTER and REINDEX grammar to use DefElem for option lists
Michael Paquier [Thu, 3 Dec 2020 01:13:21 +0000 (10:13 +0900)]
Refactor CLUSTER and REINDEX grammar to use DefElem for option lists

This changes CLUSTER and REINDEX so as a parenthesized grammar becomes
possible for options, while unifying the grammar parsing rules for
option lists with the existing ones.

This is a follow-up of the work done in 873ea9e for VACUUM, ANALYZE and
EXPLAIN.  This benefits REINDEX for a potential backend-side filtering
for collatable-sensitive indexes and TABLESPACE, while CLUSTER would
benefit from the latter.

Author: Alexey Kondratov, Justin Pryzby
Discussion: https://postgr.es/m/8a8f5f73-00d3-55f8-7583-1375ca8f6a91@postgrespro.ru

4 years agoAdd GSS information to connection authorized log message
Stephen Frost [Wed, 2 Dec 2020 19:41:53 +0000 (14:41 -0500)]
Add GSS information to connection authorized log message

GSS information (if used) such as if the connection was authorized using
GSS or if it was encrypted using GSS, and perhaps most importantly, what
the GSS principal used for the authentication was, is extremely useful
but wasn't being included in the connection authorized log message.

Therefore, add to the connection authorized log message that
information, in a similar manner to how we log SSL information when SSL
is used for a connection.

Author: Vignesh C
Reviewed-by: Bharath Rupireddy
Discussion: https://www.postgresql.org/message-id/CALDaNm2N1385_Ltoo%3DS7VGT-ESu_bRQa-sC1wg6ikrM2L2Z49w%40mail.gmail.com

4 years agoTrack total number of WAL records, FPIs and bytes generated in the cluster.
Fujii Masao [Wed, 2 Dec 2020 04:00:15 +0000 (13:00 +0900)]
Track total number of WAL records, FPIs and bytes generated in the cluster.

Commit 6b466bf5f2 allowed pg_stat_statements to track the number of
WAL records, full page images and bytes that each statement generated.
Similarly this commit allows us to track the cluster-wide WAL statistics
counters.

New columns wal_records, wal_fpi and wal_bytes are added into the
pg_stat_wal view, and reports the total number of WAL records,
full page images and bytes generated in the , respectively.

Author: Masahiro Ikeda
Reviewed-by: Amit Kapila, Movead Li, Kyotaro Horiguchi, Fujii Masao
Discussion: https://postgr.es/m/35ef960128b90bfae3b3fdf60a3a860f@oss.nttdata.com

4 years agoFix compilation warnings in cryptohash_openssl.c
Michael Paquier [Wed, 2 Dec 2020 03:31:10 +0000 (12:31 +0900)]
Fix compilation warnings in cryptohash_openssl.c

These showed up with -O2.  Oversight in 87ae969.

Author: Fujii Masao
Discussion: https://postgr.es/m/cee3df00-566a-400c-1252-67c3701f918a@oss.nttdata.com

4 years agoAllow restore_command parameter to be changed with reload.
Fujii Masao [Wed, 2 Dec 2020 02:00:15 +0000 (11:00 +0900)]
Allow restore_command parameter to be changed with reload.

This commit changes restore_command from PGC_POSTMASTER to PGC_SIGHUP.

As the side effect of this commit, restore_command can be reset to
empty during archive recovery. In this setting, archive recovery
tries to replay only WAL files available in pg_wal directory. This is
the same behavior as when the command that always fails is specified
in restore_command.

Note that restore_command still must be specified (not empty) when
starting archive recovery, even after applying this commit. This is
necessary as the safeguard to prevent users from forgetting to
specify restore_command and starting archive recovery.

Thanks to Peter Eisentraut, Michael Paquier, Andres Freund,
Robert Haas and Anastasia Lubennikova for discussion.

Author: Sergei Kornilov
Reviewed-by: Kyotaro Horiguchi, Fujii Masao
Discussion: https://postgr.es/m/2317771549527294@sas2-985f744271ca.qloud-c.yandex.net

4 years agoMove SHA2 routines to a new generic API layer for crypto hashes
Michael Paquier [Wed, 2 Dec 2020 01:37:20 +0000 (10:37 +0900)]
Move SHA2 routines to a new generic API layer for crypto hashes

Two new routines to allocate a hash context and to free it are created,
as these become necessary for the goal behind this refactoring: switch
the all cryptohash implementations for OpenSSL to use EVP (for FIPS and
also because upstream does not recommend the use of low-level cryptohash
functions for 20 years).  Note that OpenSSL hides the internals of
cryptohash contexts since 1.1.0, so it is necessary to leave the
allocation to OpenSSL itself, explaining the need for those two new
routines.  This part is going to require more work to properly track
hash contexts with resource owners, but this not introduced here.
Still, this refactoring makes the move possible.

This reduces the number of routines for all SHA2 implementations from
twelve (SHA{224,256,386,512} with init, update and final calls) to five
(create, free, init, update and final calls) by incorporating the hash
type directly into the hash context data.

The new cryptohash routines are moved to a new file, called cryptohash.c
for the fallback implementations, with SHA2 specifics becoming a part
internal to src/common/.  OpenSSL specifics are part of
cryptohash_openssl.c.  This infrastructure is usable for more hash
types, like MD5 or HMAC.

Any code paths using the internal SHA2 routines are adapted to report
correctly errors, which are most of the changes of this commit.  The
zones mostly impacted are checksum manifests, libpq and SCRAM.

Note that e21cbb4 was a first attempt to switch SHA2 to EVP, but it
lacked the refactoring needed for libpq, as done here.

This patch has been tested on Linux and Windows, with and without
OpenSSL, and down to 1.0.1, the oldest version supported on HEAD.

Author: Michael Paquier
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/20200924025314.GE7405@paquier.xyz

4 years agopg_checksums: data_checksum_version is unsigned so use %u not %d
Bruce Momjian [Wed, 2 Dec 2020 01:27:06 +0000 (20:27 -0500)]
pg_checksums: data_checksum_version is unsigned so use %u not %d

While the previous behavior didn't generate a warning, we might as well
use an accurate *printf specification.

Backpatch-through: 12

4 years agoEnsure that expandTableLikeClause() re-examines the same table.
Tom Lane [Tue, 1 Dec 2020 19:02:27 +0000 (14:02 -0500)]
Ensure that expandTableLikeClause() re-examines the same table.

As it stood, expandTableLikeClause() re-did the same relation_openrv
call that transformTableLikeClause() had done.  However there are
scenarios where this would not find the same table as expected.
We hold lock on the LIKE source table, so it can't be renamed or
dropped, but another table could appear before it in the search path.
This explains the odd behavior reported in bug #16758 when cloning a
table as a temp table of the same name.  This case worked as expected
before commit 502898192 introduced the need to open the source table
twice, so we should fix it.

To make really sure we get the same table, let's re-open it by OID not
name.  That requires adding an OID field to struct TableLikeClause,
which is a little nervous-making from an ABI standpoint, but as long
as it's at the end I don't think there's any serious risk.

Per bug #16758 from Marc Boeren.  Like the previous patch,
back-patch to all supported branches.

Discussion: https://postgr.es/m/16758-840e84a6cfab276d@postgresql.org

4 years agoAvoid memcpy() with a NULL source pointer and count == 0
Alvaro Herrera [Tue, 1 Dec 2020 14:46:56 +0000 (11:46 -0300)]
Avoid memcpy() with a NULL source pointer and count == 0

When memcpy() is called on a pointer, the compiler is entitled to assume
that the pointer is not null, which can lead to optimizing nearby code
in potentially undesirable ways.  We still want such optimizations
(gcc's -fdelete-null-pointer-checks) in cases where they're valid.

Related: commit 13bba02271dc.

Backpatch to pg11, where this particular instance appeared.

Reported-by: Ranier Vilela <ranier.vf@gmail.com>
Reported-by: Zhihong Yu <zyu@yugabyte.com>
Discussion: https://postgr.es/m/CAEudQApUndmQkr5fLrCKXQ7+ib44i7S+Kk93pyVThS85PnG3bQ@mail.gmail.com
Discussion: https://postgr.es/m/CALNJ-vSdhwSM5f4tnNn1cdLHvXMVe_S+V3nR5GwNrmCPNB2VtQ@mail.gmail.com

4 years agodocs: ulink all references to RFC's
Heikki Linnakangas [Tue, 1 Dec 2020 12:36:30 +0000 (14:36 +0200)]
docs: ulink all references to RFC's

Make sure that the first mentions of RFC's are ulinked to their ietf.org
entry, and subsequent ones are marked as acronyms. This makes references
to RFC's consistent across the documentation.

Author: Daniel Gustafsson
Discussion: https://www.postgresql.org/message-id/2C697878-4D01-4F06-8312-2FEDE931E973%40yesql.se

4 years agodoc: Add additional index entries for progress reporting views.
Fujii Masao [Tue, 1 Dec 2020 08:19:23 +0000 (17:19 +0900)]
doc: Add additional index entries for progress reporting views.

In the docs, the index entries for progress reporting views link to
the "Viewing Statistics" section, but previously they did not link to
the dedicated section (e.g., "ANALYZE Progress Reporting") for
each view. This was inconvenient when finding the section describing
the detailed information about each view, from the index.
This commit adds additional index entries linking to those dedicated
sections.

Author: Fujii Masao
Reviewed-by: Shinya Kato
Discussion: https://postgr.es/m/e49c2768-65d2-188a-5424-270fa29ccc84@oss.nttdata.com

4 years agodoc: Remove more notes about compatibilities with past versions
Michael Paquier [Tue, 1 Dec 2020 07:32:26 +0000 (16:32 +0900)]
doc: Remove more notes about compatibilities with past versions

This is a follow-up of the work done in fa42c2e, that did not include
all the fixes previously agreed on.  The contents removed here can be
confusing to the reader as they refer to rather old server versions.

Author: Stephen Frost, Tom Lane, Heikki Linnakangas, Yaroslav Schekin
Discussion: https://postgr.es/m/CAB8KJ=jYHgnxLLZSNJz7gBTck4TxomngCmGkw3nEMSNF0yL6wA@mail.gmail.com
Discussion: https://postgr.es/m/1599765595731-0.post@n3.nabble.com

4 years agoUse truncate(2) where appropriate.
Thomas Munro [Tue, 1 Dec 2020 02:34:57 +0000 (15:34 +1300)]
Use truncate(2) where appropriate.

When truncating files by name, use truncate(2).  Windows hasn't got it,
so keep our previous coding based on ftruncate(2) as a fallback.

Discussion: https://postgr.es/m/16663-fe97ccf9932fc800%40postgresql.org

4 years agoFree disk space for dropped relations on commit.
Thomas Munro [Tue, 1 Dec 2020 00:21:03 +0000 (13:21 +1300)]
Free disk space for dropped relations on commit.

When committing a transaction that dropped a relation, we previously
truncated only the first segment file to free up disk space (the one
that won't be unlinked until the next checkpoint).

Truncate higher numbered segments too, even though we unlink them on
commit.  This frees the disk space immediately, even if other backends
have open file descriptors and might take a long time to get around to
handling shared invalidation events and closing them.  Also extend the
same behavior to the first segment, in recovery.

Back-patch to all supported releases.

Bug: #16663
Reported-by: Denis Patron <denis.patron@previnet.it>
Reviewed-by: Pavel Borisov <pashkin.elfe@gmail.com>
Reviewed-by: Neil Chen <carpenter.nail.cz@gmail.com>
Reviewed-by: David Zhang <david.zhang@highgo.ca>
Discussion: https://postgr.es/m/16663-fe97ccf9932fc800%40postgresql.org

4 years agoFix missing outfuncs.c support for IncrementalSortPath.
Tom Lane [Mon, 30 Nov 2020 21:32:56 +0000 (16:32 -0500)]
Fix missing outfuncs.c support for IncrementalSortPath.

For debugging purposes, Path nodes are supposed to have outfuncs
support, but this was overlooked in the original incremental sort patch.

While at it, clean up a couple other minor oversights, as well as
bizarre choice of return type for create_incremental_sort_path().
(All the existing callers just cast it to "Path *" immediately, so
they don't care, but some future caller might care.)

outfuncs.c fix by Zhijie Hou, the rest by me

Discussion: https://postgr.es/m/324c4d81d8134117972a5b1f6cdf9560@G08CNEXMBPEKD05.g08.fujitsu.local

4 years agoDocument concurrent indexes waiting on each other
Alvaro Herrera [Mon, 30 Nov 2020 21:24:55 +0000 (18:24 -0300)]
Document concurrent indexes waiting on each other

Because regular CREATE INDEX commands are independent, and there's no
logical data dependency, it's not immediately obvious that transactions
held by concurrent index builds on one table will block the second phase
of concurrent index creation on an unrelated table, so document this
caveat.

Backpatch this all the way back.  In branch master, mention that only
some indexes are involved.

Author: James Coleman <jtc331@gmail.com>
Reviewed-by: David Johnston <david.g.johnston@gmail.com>
Discussion: https://postgr.es/m/CAAaqYe994=PUrn8CJZ4UEo_S-FfRr_3ogERyhtdgHAb2WG_Ufg@mail.gmail.com

4 years agoRemove configure-time probe for DocBook DTD.
Tom Lane [Mon, 30 Nov 2020 20:24:13 +0000 (15:24 -0500)]
Remove configure-time probe for DocBook DTD.

Checking for DocBook being installed was valuable when we were on the
OpenSP docs toolchain, because that was rather hard to get installed
fully.  Nowadays, as long as you have xmllint and xsltproc installed,
you're good, because those programs will fetch the DocBook files off
the net at need.  Moreover, testing this at configure time means that
a network access may well occur whether or not you have any interest
in building the docs later.  That can be slow (typically 2 or 3
seconds, though much higher delays have been reported), and it seems
not very nice to be doing an off-machine access without warning, too.

Hence, drop the PGAC_CHECK_DOCBOOK probe, and adjust related
documentation.  Without that macro, there's not much left of
config/docbook.m4 at all, so I just removed it.

Back-patch to v11, where we started to use xmllint in the
PGAC_CHECK_DOCBOOK probe.

Discussion: https://postgr.es/m/E2EE6B76-2D96-408A-B961-CAE47D1A86F0@yesql.se
Discussion: https://postgr.es/m/A55A7FC9-FA60-47FE-98B5-139CDC57CE6E@gmail.com

4 years agoPrevent parallel index build in a standalone backend.
Tom Lane [Mon, 30 Nov 2020 19:38:00 +0000 (14:38 -0500)]
Prevent parallel index build in a standalone backend.

This can't work if there's no postmaster, and indeed the code got an
assertion failure trying.  There should be a check on IsUnderPostmaster
gating the use of parallelism, as the planner has for ordinary
parallel queries.

Commit 40d964ec9 got this right, so follow its model of checking
IsUnderPostmaster at the same place where we check for
max_parallel_maintenance_workers == 0.  In general, new code
implementing parallel utility operations should do the same.

Report and patch by Yulin Pei, cosmetically adjusted by me.
Back-patch to v11 where this code came in.

Discussion: https://postgr.es/m/HK0PR01MB22747D839F77142D7E76A45DF4F50@HK0PR01MB2274.apcprd01.prod.exchangelabs.com

4 years agoFix miscomputation of direct_lateral_relids for join relations.
Tom Lane [Mon, 30 Nov 2020 17:22:43 +0000 (12:22 -0500)]
Fix miscomputation of direct_lateral_relids for join relations.

If a PlaceHolderVar is to be evaluated at a join relation, but
its value is only needed there and not at higher levels, we neglected
to update the joinrel's direct_lateral_relids to include the PHV's
source rel.  This causes problems because join_is_legal() then won't
allow joining the joinrel to the PHV's source rel at all, leading
to "failed to build any N-way joins" planner failures.

Per report from Andreas Seltenreich.  Back-patch to 9.5
where the problem originated.

Discussion: https://postgr.es/m/87blfgqa4t.fsf@aurora.ydns.eu