postgresql.git
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

4 years agoRefactor parsing rules for option lists of EXPLAIN, VACUUM and ANALYZE
Michael Paquier [Mon, 30 Nov 2020 11:27:37 +0000 (20:27 +0900)]
Refactor parsing rules for option lists of EXPLAIN, VACUUM and ANALYZE

Those three commands have been using the same grammar rules to handle a
a list of parenthesized options.  This refactors the code so as they use
the same parsing rules, shaving some code.  A future commit will make
use of those option parsing rules for more utility commands, like
REINDEX and CLUSTER.

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

4 years agoRemove leftover comments, left behind by removal of WITH OIDS.
Heikki Linnakangas [Mon, 30 Nov 2020 08:26:43 +0000 (10:26 +0200)]
Remove leftover comments, left behind by removal of WITH OIDS.

Author: Amit Langote
Discussion: https://www.postgresql.org/message-id/CA%2BHiwqGaRoF3XrhPW-Y7P%2BG7bKo84Z_h%3DkQHvMh-80%3Dav3wmOw%40mail.gmail.com

4 years agoFix typo in comment.
Fujii Masao [Mon, 30 Nov 2020 03:54:31 +0000 (12:54 +0900)]
Fix typo in comment.

Author: Haiying Tang <tanghy.fnst@cn.fujitsu.com>
Discussion: https://postgr.es/m/48a0928ac94b497d9c40acf1de394c15@G08CNEXMBPEKD05.g08.fujitsu.local

4 years agoImprove log message about termination of background workers.
Fujii Masao [Mon, 30 Nov 2020 02:05:19 +0000 (11:05 +0900)]
Improve log message about termination of background workers.

Previously the shutdown of a background worker that uses die() as
SIGTERM signal handler produced the log message "terminating
connection due to administrator command". This log message was
confusing because a background worker is not a connection.
This commit improves that log message to "terminating background
worker XXX due to administrator command" (XXX is replaced with
the name of the background worker). This is the same log message
as another SIGTERM signal handler bgworker_die() for a background
worker reports.

Author: Bharath Rupireddy
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/3f292fbb-f155-9a01-7cb2-7ccc9007ab3f@oss.nttdata.com

4 years agoFix recently-introduced breakage in psql's \connect command.
Tom Lane [Sun, 29 Nov 2020 20:22:04 +0000 (15:22 -0500)]
Fix recently-introduced breakage in psql's \connect command.

Through my misreading of what the existing code actually did,
commits 85c54287a et al. broke psql's behavior for the case where
"\c connstring" provides a password in the connstring.  We should
use that password in such a case, but as of 85c54287a we ignored it
(and instead, prompted for a password).

Commit 94929f1cf fixed that in HEAD, but since I thought it was
cleaning up a longstanding misbehavior and not one I'd just created,
I didn't back-patch it.

Hence, back-patch the portions of 94929f1cf having to do with
password management.  In addition to fixing the introduced bug,
this means that "\c -reuse-previous=on connstring" will allow
re-use of an existing connection's password if the connstring
doesn't change user/host/port.  That didn't happen before, but
it seems like a bug fix, and anyway I'm loath to have significant
differences in this code across versions.

Also fix an error with the same root cause about whether or not to
override a connstring's setting of client_encoding.  As of 85c54287a
we always did so; restore the previous behavior of overriding only
when stdin/stdout are a terminal and there's no environment setting
of PGCLIENTENCODING.  (I find that definition a bit surprising, but
right now doesn't seem like the time to revisit it.)

Per bug #16746 from Krzysztof Gradek.  As with the previous patch,
back-patch to all supported branches.

Discussion: https://postgr.es/m/16746-44b30e2edf4335d4@postgresql.org

4 years agoDoc: clarify behavior of PQconnectdbParams().
Tom Lane [Sun, 29 Nov 2020 18:58:30 +0000 (13:58 -0500)]
Doc: clarify behavior of PQconnectdbParams().

The documentation omitted the critical tidbit that a keyword-array entry
is simply ignored if its corresponding value-array entry is NULL or an
empty string; it will *not* override any previously-obtained value for
the parameter.  (See conninfo_array_parse().)  I'd supposed that would
force the setting back to default, which is what led me into bug #16746;
but it doesn't.

While here, I couldn't resist the temptation to do some copy-editing,
both in the description of PQconnectdbParams() and in the section
about connection URI syntax.

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

4 years agoRetry initial slurp_file("current_logfiles"), in test 004_logrotate.pl.
Noah Misch [Sun, 29 Nov 2020 05:52:27 +0000 (21:52 -0800)]
Retry initial slurp_file("current_logfiles"), in test 004_logrotate.pl.

Buildfarm member topminnow failed when the test script attempted this
before the syslogger would have created the file.  Back-patch to v12,
which introduced the test.

4 years agoClean up after tests in src/test/locale/.
Tom Lane [Sat, 28 Nov 2020 21:08:36 +0000 (16:08 -0500)]
Clean up after tests in src/test/locale/.

Oversight in 257836a75, which added these tests.

4 years agoFix a recently-introduced race condition in LISTEN/NOTIFY handling.
Tom Lane [Sat, 28 Nov 2020 19:03:40 +0000 (14:03 -0500)]
Fix a recently-introduced race condition in LISTEN/NOTIFY handling.

Commit 566372b3d fixed some race conditions involving concurrent
SimpleLruTruncate calls, but it introduced new ones in async.c.
A newly-listening backend could attempt to read Notify SLRU pages that
were in process of being truncated, possibly causing an error.  Also,
the QUEUE_TAIL pointer could become set to a value that's not equal to
the queue position of any backend.  While that's fairly harmless in
v13 and up (thanks to commit 51004c717), in older branches it resulted
in near-permanent disabling of the queue truncation logic, so that
continued use of NOTIFY led to queue-fill warnings and eventual
inability to send any more notifies.  (A server restart is enough to
make that go away, but it's still pretty unpleasant.)

The core of the problem is confusion about whether QUEUE_TAIL
represents the "logical" tail of the queue (i.e., the oldest
still-interesting data) or the "physical" tail (the oldest data we've
not yet truncated away).  To fix, split that into two variables.
QUEUE_TAIL regains its definition as the logical tail, and we
introduce a new variable to track the oldest un-truncated page.

Per report from Mikael Gustavsson.  Like the previous patch,
back-patch to all supported branches.

Discussion: https://postgr.es/m/1b8561412e8a4f038d7a491c8b922788@smhi.se

4 years agoFix CLUSTER progress reporting of number of blocks scanned.
Fujii Masao [Fri, 27 Nov 2020 11:16:44 +0000 (20:16 +0900)]
Fix CLUSTER progress reporting of number of blocks scanned.

Previously pg_stat_progress_cluster view reported the current block
number in heap scan as the number of heap blocks scanned (i.e.,
heap_blks_scanned). This reported number could be incorrect when
synchronize_seqscans is enabled, because it allowed the heap scan to
start at block in middle. This could result in wraparounds in the
heap_blks_scanned column when the heap scan wrapped around.
This commit fixes the bug by calculating the number of blocks from
the block that the heap scan starts at to the current block in scan,
and reporting that number in the heap_blks_scanned column.

Also, in pg_stat_progress_cluster view, previously heap_blks_scanned
could not reach heap_blks_total at the end of heap scan phase
if the last pages scanned were empty. This commit fixes the bug by
manually updating heap_blks_scanned to the same value as
heap_blks_total when the heap scan phase finishes.

Back-patch to v12 where pg_stat_progress_cluster view was introduced.

Reported-by: Matthias van de Meent
Author: Matthias van de Meent
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/CAEze2WjCBWSGkVfYag001Rc4+-nNLDpWM7QbyD6yPvuhKs-gYQ@mail.gmail.com

4 years agoUse standard SIGTERM signal handler die() in test_shm_mq worker.
Fujii Masao [Fri, 27 Nov 2020 06:45:01 +0000 (15:45 +0900)]
Use standard SIGTERM signal handler die() in test_shm_mq worker.

Previously test_shm_mq worker used the stripped-down version of die()
as the SIGTERM signal handler. This commit makes it use die(), instead,
to simplify the code.

In terms of the code, the difference between die() and the stripped-down
version previously used is whether the signal handler directly may call
ProcessInterrupts() or not. But this difference doesn't exist in
a background worker because, in bgworker, DoingCommandRead flag will
never be true and die() will never call ProcessInterrupts() directly.
Therefore test_shm_mq worker can safely use die(), like other bgworker
proceses (e.g., logical replication apply launcher or autoprewarm worker)
currently do.

Thanks to Craig Ringer for the report and investigation of the issue.

Author: Bharath Rupireddy
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/CAGRY4nxsAe_1k_9g5b47orA0S011iBoHsXHFMH7cg7HV0O1bwQ@mail.gmail.com

4 years agoUse standard SIGHUP and SIGTERM signal handlers in worker_spi.
Fujii Masao [Fri, 27 Nov 2020 06:11:19 +0000 (15:11 +0900)]
Use standard SIGHUP and SIGTERM signal handlers in worker_spi.

Previously worker_spi used its custom signal handlers for SIGHUP and
SIGTERM. This commit makes worker_spi use the standard signal handlers,
to simplify the code.

Note that die() is used as the standard SIGTERM signal handler in
worker_spi instead of SignalHandlerForShutdownRequest() or bgworker_die().
Previously the exit handling was only able to exit from within the main loop,
and not from within the backend code it calls. This is why die() needs to
be used here, so worker_spi can respond to SIGTERM signal while it's
executing a query.

Maybe we can say that it's a bug that worker_spi could not respond to
SIGTERM during query execution. But since worker_spi is a just example
of the background worker code, we don't do the back-patch.

Thanks to Craig Ringer for the report and investigation of the issue.

Author: Bharath Rupireddy
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/CALj2ACXDEZhAFOTDcqO9cFSRvrFLyYOnPKrcA1UG4uZn9hUAVg@mail.gmail.com
Discussion: https://postgr.es/m/CAGRY4nxsAe_1k_9g5b47orA0S011iBoHsXHFMH7cg7HV0O1bwQ@mail.gmail.com

4 years agoFix replication of in-progress transactions in tablesync worker.
Amit Kapila [Fri, 27 Nov 2020 02:13:34 +0000 (07:43 +0530)]
Fix replication of in-progress transactions in tablesync worker.

Tablesync worker runs under a single transaction but in streaming mode, we
were committing the transaction on stream_stop, stream_abort, and
stream_commit. We need to avoid committing the transaction in a streaming
mode in tablesync worker.

In passing move the call to process_syncing_tables in
apply_handle_stream_commit after clean up of stream files. This will
allow clean up of files to happen before the exit of tablesync worker
which would otherwise be handled by one of the proc exit routines.

Author: Dilip Kumar
Reviewed-by: Amit Kapila and Peter Smith
Tested-by: Peter Smith
Discussion: https://postgr.es/m/CAHut+Pt4PyKQCwqzQ=EFF=bpKKJD7XKt_S23F6L20ayQNxg77A@mail.gmail.com

4 years agoRestore lock level to update statusFlags
Alvaro Herrera [Thu, 26 Nov 2020 15:30:48 +0000 (12:30 -0300)]
Restore lock level to update statusFlags

Reverts 27838981be9d (some comments are kept).  Per discussion, it does
not seem safe to relax the lock level used for this; in order for it to
be safe, there would have to be memory barriers between the point we set
the flag and the point we set the trasaction Xid, which perhaps would
not be so bad; but there would also have to be barriers at the readers'
side, which from a performance perspective might be bad.

Now maybe this analysis is wrong and it *is* safe for some reason, but
proof of that is not trivial.

Discussion: https://postgr.es/m/20201118190928.vnztes7c2sldu43a@alap3.anarazel.de

4 years agopg_stat_statements: Track number of times pgss entries were deallocated.
Fujii Masao [Thu, 26 Nov 2020 12:18:05 +0000 (21:18 +0900)]
pg_stat_statements: Track number of times pgss entries were deallocated.

If more distinct statements than pg_stat_statements.max are observed,
pg_stat_statements entries about the least-executed statements are
deallocated. This commit enables us to track the total number of times
those entries were deallocated. That number can be viewed in the
pg_stat_statements_info view that this commit adds. It's useful when
tuning pg_stat_statements.max parameter. If it's high, i.e., the entries
are deallocated very frequently, which might cause the performance
regression and we can increase pg_stat_statements.max to avoid those
frequent deallocations.

The pg_stat_statements_info view is intended to display the statistics
of pg_stat_statements module itself. Currently it has only one column
"dealloc" indicating the number of times entries were deallocated.
But an upcoming patch will add other columns (for example, the time
at which pg_stat_statements statistics were last reset) into the view.

Author: Katsuragi Yuta, Yuki Seino
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/0d9f1107772cf5c3f954e985464c7298@oss.nttdata.com

4 years agodoc: Add description about re-analysis and re-planning of a prepared statement.
Fujii Masao [Thu, 26 Nov 2020 07:17:10 +0000 (16:17 +0900)]
doc: Add description about re-analysis and re-planning of a prepared statement.

A prepared statement is re-analyzed and re-planned whenever database
objects used in the statement have undergone definitional changes or
the planner statistics of them have been updated. The former has been
documented from before, but the latter was not previously. This commit
adds the description about the latter case into the docs.

Author: Atsushi Torikoshi
Reviewed-by: Andy Fan, Fujii Masao
Discussion: https://postgr.es/m/3ac82f4817c9fe274a905c8a38d87bd9@oss.nttdata.com

4 years agoUse Enums for logical replication message types at more places.
Amit Kapila [Thu, 26 Nov 2020 03:51:14 +0000 (09:21 +0530)]
Use Enums for logical replication message types at more places.

Commit 644f0d7cc9 added logical replication message type enums to use
instead of character literals but some char substitutions were overlooked.

Author: Peter Smith
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CAHut+PsTG=Vrv8hgrvOnAvCNR21jhqMdPk2n0a1uJPoW0p+UfQ@mail.gmail.com

4 years agoAvoid spurious waits in concurrent indexing
Alvaro Herrera [Wed, 25 Nov 2020 21:21:08 +0000 (18:21 -0300)]
Avoid spurious waits in concurrent indexing

In the various waiting phases of CREATE INDEX CONCURRENTLY (CIC) and
REINDEX CONCURRENTLY (RC), we wait for other processes to release their
snapshots; this is necessary in general for correctness.  However,
processes doing CIC in other tables cannot possibly affect CIC or RC
done in "this" table, so we don't need to wait for those.  This commit
adds a flag in MyProc->statusFlags to indicate that the current process
is doing CIC, so that other processes doing CIC or RC can ignore it when
waiting.

Note that this logic is only valid if the index does not access other
tables.  For simplicity we avoid setting the flag if the index has a
column that's an expression, or has a WHERE predicate.  (It is possible
to have expressional or partial indexes that do not access other tables,
but figuring that out would require more work.)

This flag can potentially also be used by processes doing REINDEX
CONCURRENTLY to be skipped; and by VACUUM to ignore processes in CIC or
RC for the purposes of computing an Xmin.  That's left for future
commits.

Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Author: Dimitry Dolgov <9erthalion6@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/20200810233815.GA18970@alvherre.pgsql

4 years agoIn psql's \d commands, don't truncate attribute default values.
Tom Lane [Wed, 25 Nov 2020 21:19:25 +0000 (16:19 -0500)]
In psql's \d commands, don't truncate attribute default values.

Historically, psql has truncated the text of a column's default
expression at 128 characters.  This is unlike any other behavior
in describe.c, and it's become particularly confusing now that
the limit is only applied to the expression proper and not to
the "generated always as (...) stored" text that may get wrapped
around it.

Excavation in our git history suggests that the original motivation
for this limit was not really to limit the display width (as I'd long
supposed), but to make it safe to use a fixed-width output buffer to
store the result.  That implementation restriction is long gone of
course, but the limit remained.  Let's just get rid of it.

While here, rearrange the logic about when to free the output string
so that it's not so dependent on unstated assumptions about the
possible values of attidentity and attgenerated.

Per bug #16743 from David Turon.  Back-patch to v12 where GENERATED
came in.  (Arguably we could take it back further, but I'm hesitant
to change the behavior of long-stable branches for this.)

Discussion: https://postgr.es/m/16743-7b1bacc4af76e7ad@postgresql.org

4 years agoDoc: minor improvements for section 11.2 "Index Types".
Tom Lane [Wed, 25 Nov 2020 19:04:28 +0000 (14:04 -0500)]
Doc: minor improvements for section 11.2 "Index Types".

Break the per-index-type discussions into <sect2>'s so as to make
them more visually separate and easier to find.  Improve the markup,
and make a couple of small wording adjustments.

This also fixes one stray reference to the now-deprecated point
operators <^ and >^.

Dagfinn Ilmari Mannsåker, reviewed by David Johnston and Jürgen Purtz

Discussion: https://postgr.es/m/877dukhvzg.fsf@wibble.ilmari.org

4 years agoAvoid spamming the client with multiple ParameterStatus messages.
Tom Lane [Wed, 25 Nov 2020 16:40:44 +0000 (11:40 -0500)]
Avoid spamming the client with multiple ParameterStatus messages.

Up to now, we sent a ParameterStatus message to the client immediately
upon any change in the active value of any GUC_REPORT variable.  This
was only barely okay when the feature was designed; now that we have
things like function SET clauses, there are very plausible use-cases
where a GUC_REPORT variable might change many times within a query
--- and even end up back at its original value, perhaps.  Fortunately
most of our GUC_REPORT variables are unlikely to be changed often;
but there are proposals in play to enlarge that set, or even make it
user-configurable.

Hence, let's fix things to not generate more than one ParameterStatus
message per variable per query, and to not send any message at all
unless the end-of-query value is different from what we last reported.

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

4 years agotablefunc: Reject negative number of tuples passed to normal_rand()
Peter Eisentraut [Wed, 25 Nov 2020 14:30:18 +0000 (15:30 +0100)]
tablefunc: Reject negative number of tuples passed to normal_rand()

The function converted the first argument i.e. the number of tuples to
return into an unsigned integer which turns out to be huge number when
a negative value is passed.  This causes the function to take much
longer time to execute.  Instead, reject a negative value.

(If someone really wants to generate many more result rows, they
should consider adding a bigint or numeric variant.)

While at it, improve SQL test to test the number of tuples returned by
this function.

Author: Ashutosh Bapat <ashutosh.bapat@2ndquadrant.com>
Discussion: https://www.postgresql.org/message-id/CAG-ACPW3PUUmSnM6cLa9Rw4BEC5cEMKjX8Gogc8gvQcT3cYA1A@mail.gmail.com

4 years agodoc: Fix typos
Peter Eisentraut [Wed, 25 Nov 2020 08:49:00 +0000 (09:49 +0100)]
doc: Fix typos

Author: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://www.postgresql.org/message-id/20201121194105.GO24784@telsasoft.com

4 years agoMake error hint from bind() failure more accurate
Peter Eisentraut [Wed, 25 Nov 2020 07:14:23 +0000 (08:14 +0100)]
Make error hint from bind() failure more accurate

The hint "Is another postmaster already running ..." should only be
printed for errors that are really about something else already using
the address.  In other cases it is misleading.  So only show that hint
if errno == EADDRINUSE.

Also, since Unix-domain sockets in the file-system namespace never
report EADDRINUSE for an existing file (they would just overwrite it),
the part of the hint saying "If not, remove socket file \"%s\" and
retry." can never happen, so remove it.  Unix-domain sockets in the
abstract namespace can report EADDRINUSE, but in that case there is no
file to remove, so the hint doesn't work there either.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/6dee8574-b0ad-fc49-9c8c-2edc796f0033@2ndquadrant.com

4 years agoAdd support for abstract Unix-domain sockets
Peter Eisentraut [Wed, 25 Nov 2020 07:14:23 +0000 (08:14 +0100)]
Add support for abstract Unix-domain sockets

This is a variant of the normal Unix-domain sockets that don't use the
file system but a separate "abstract" namespace.  At the user
interface, such sockets are represented by names starting with "@".
Supported on Linux and Windows right now.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/6dee8574-b0ad-fc49-9c8c-2edc796f0033@2ndquadrant.com

4 years agoFix WaitLatch(NULL) on Windows.
Thomas Munro [Wed, 25 Nov 2020 04:44:15 +0000 (17:44 +1300)]
Fix WaitLatch(NULL) on Windows.

Further to commit 733fa9aa, on Windows when a latch is triggered but we
aren't currently waiting for it, we need to locate the latch's HANDLE
rather than calling ResetEvent(NULL).

Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reported-by: Ranier Vilela <ranier.vf@gmail.com>
Discussion: https://postgr.es/m/CAEudQArTPi1YBc%2Bn1fo0Asy3QBFhVjp_QgyKG-8yksVn%2ByRTiw%40mail.gmail.com

4 years agoRemove obsolete comment atop ri_PlanCheck.
Amit Kapila [Wed, 25 Nov 2020 03:44:45 +0000 (09:14 +0530)]
Remove obsolete comment atop ri_PlanCheck.

Commit 5b7ba75f7f removed the unused parameter but forgot to update the
nearby comments.

Author: Li Japin
Backpatch-through: 13, where it was introduced
Discussion: https://postgr.es/m/0E2F62A2-B2F1-4052-83AE-F0BEC8A75789@hotmail.com