postgresql.git
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 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: Fix typos
Peter Eisentraut [Wed, 25 Nov 2020 08:49:00 +0000 (09:49 +0100)]
doc: Fix typos

Author: Justin Pryzby 
Discussion: https://www.postgresql.org/message-id/20201121194105[email protected]

4 years agoRemove obsolete comment atop ri_PlanCheck.
Amit Kapila [Wed, 25 Nov 2020 03:51:33 +0000 (09:21 +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

4 years agoProperly check index mark/restore in ExecSupportsMarkRestore.
Andrew Gierth [Tue, 24 Nov 2020 20:58:32 +0000 (20:58 +0000)]
Properly check index mark/restore in ExecSupportsMarkRestore.

Previously this code assumed that all IndexScan nodes supported
mark/restore, which is not true since it depends on optional index AM
support functions. This could lead to errors about missing support
functions in rare edge cases of mergejoins with no sort keys, where an
unordered non-btree index scan was placed on the inner path without a
protecting Materialize node. (Normally, the fact that merge join
requires ordered input would avoid this error.)

Backpatch all the way since this bug is ancient.

Per report from Eugen Konkov on irc.

Discussion: https://postgr.es/m/[email protected]

4 years agoSkip allocating hash table in EXPLAIN-only mode.
Heikki Linnakangas [Fri, 20 Nov 2020 12:41:14 +0000 (14:41 +0200)]
Skip allocating hash table in EXPLAIN-only mode.

This is a backpatch of commit 2cccb627f1, backpatched due to popular
demand. Backpatch to all supported versions.

Author: Alexey Bashtanov
Discussion: https://www.postgresql.org/message-id/36823f65-050d-ae24-aa4d-a37726998240%40imap.cc

4 years agoOn macOS, use -isysroot in link steps as well as compile steps.
Tom Lane [Fri, 20 Nov 2020 05:58:26 +0000 (00:58 -0500)]
On macOS, use -isysroot in link steps as well as compile steps.

We previously put the -isysroot switch only into CPPFLAGS, theorizing
that it was only needed to find the right copies of include files.
However, it seems that we also need to use it while linking programs,
to find the right stub ".tbd" files for libraries.  We got away
without that up to now, but apparently that was mostly luck.  It may
also be that failures are only observed when the Xcode version is
noticeably out of sync with the host macOS version; the case that's
prompting action right now is that builds fail when using latest Xcode
(12.2) on macOS Catalina, even though it's fine on Big Sur.

Hence, add -isysroot to LDFLAGS as well.  (It seems that the more
common practice is to put it in CFLAGS, whence it'd be included at
both compile and link steps.  However, we can't mess with CFLAGS in
the platform template file without confusing configure's logic for
choosing default CFLAGS.)

Back-patch of 49407dc32 into all supported branches.

Report and patch by James Hilliard (some cosmetic mods by me)

Discussion: https://postgr.es/m/20201120003314[email protected]

4 years agoFurther fixes for CREATE TABLE LIKE: cope with self-referential FKs.
Tom Lane [Thu, 19 Nov 2020 20:03:17 +0000 (15:03 -0500)]
Further fixes for CREATE TABLE LIKE: cope with self-referential FKs.

Commit 502898192 was too careless about the order of execution of the
additional ALTER TABLE operations generated by expandTableLikeClause.
It just stuck them all at the end, which seems okay for most purposes.
But it falls down in the case where LIKE is importing a primary key
or unique index and the outer CREATE TABLE includes a FOREIGN KEY
constraint that needs to depend on that index.  Weird as that is,
it used to work, so we ought to keep it working.

To fix, make parse_utilcmd.c insert LIKE clauses between index-creation
and FK-creation commands in the transformed list of commands, and change
utility.c so that the commands generated by expandTableLikeClause are
executed immediately not at the end.  One could imagine scenarios where
this wouldn't work either; but currently expandTableLikeClause only
makes column default expressions, CHECK constraints, and indexes, and
this ordering seems fine for those.

Per bug #16730 from Sofoklis Papasofokli.  Like the previous patch,
back-patch to all supported branches.

Discussion: https://postgr.es/m/16730-b902f7e6e0276b30@postgresql.org

4 years agoDon't Insert() a VFD entry until it's fully built.
Tom Lane [Tue, 17 Nov 2020 01:32:35 +0000 (20:32 -0500)]
Don't Insert() a VFD entry until it's fully built.

Otherwise, if FDDEBUG is enabled, the debugging output fails because
it tries to read the fileName, which isn't set up yet (and should in
fact always be NULL).

AFAICT, this has been wrong since Berkeley.  Before 96bf88d52,
it would accidentally fail to crash on platforms where snprintf()
is forgiving about being passed a NULL pointer for %s; but the
file name intended to be included in the debug output wouldn't
ever have shown up.

Report and fix by Greg Nancarrow.  Although this is only visibly
broken in custom-made builds, it still seems worth back-patching
to all supported branches, as the FDDEBUG code is pretty useless
as it stands.

Discussion: https://postgr.es/m/CAJcOf-cUDgm9qYtC_B6XrC6MktMPNRby2p61EtSGZKnfotMArw@mail.gmail.com

4 years agoDo not return NULL for error cases in satisfies_hash_partition().
Tom Lane [Mon, 16 Nov 2020 21:39:59 +0000 (16:39 -0500)]
Do not return NULL for error cases in satisfies_hash_partition().

Since this function is used as a CHECK constraint condition,
returning NULL is tantamount to returning TRUE, which would have the
effect of letting in a row that doesn't satisfy the hash condition.
Admittedly, the cases for which this is done should be unreachable
in practice, but that doesn't make it any less a bad idea.  It also
seems like a dartboard was used to decide which error cases should
throw errors as opposed to returning NULL.

For the checks for NULL input values, I just switched it to returning
false.  There's some argument that an error would be better; but the
case really should be can't-happen in a generated hash constraint,
so it's likely not worth more code for.

For the parent-relation-open-failure case, it seems like we might
as well let relation_open throw an error, instead of having an
impossible-to-diagnose constraint failure.

Back-patch to v11 where this code came in.

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

4 years agoUse "true" not "TRUE" in one ICU function call.
Tom Lane [Mon, 16 Nov 2020 20:16:39 +0000 (15:16 -0500)]
Use "true" not "TRUE" in one ICU function call.

This was evidently missed in commit 6337865f3, which generally did
s/TRUE/true/ everywhere.  It escaped notice up to now because ICU
versions before ICU 68 provided definitions of "TRUE" and "FALSE"
regardless.  With ICU 68, it fails to compile.

Per report from Condor.  Back-patch to v11 where 6337865f3 came in.
(I've not tested v10, where this call originated, but I imagine
it's fine since we defined TRUE in c.h back then.)

Discussion: https://postgr.es/m/7a6f3336165bfe3ca66abcda7966f9d0@stz-bg.com

4 years agodoc: update bgwriter description
Bruce Momjian [Mon, 16 Nov 2020 18:13:43 +0000 (13:13 -0500)]
doc:  update bgwriter description

This clarifies exactly what the bgwriter does, which should help with
tuning.

Reported-by: Chris Wilson
Discussion: https://postgr.es/m/160399562040.7809.7335281028960123489@wrigleys.postgresql.org

Backpatch-through: 9.5

4 years agodoc: clarify how to find pg_type_d.h in the install tree
Bruce Momjian [Mon, 16 Nov 2020 17:36:17 +0000 (12:36 -0500)]
doc:  clarify how to find pg_type_d.h in the install tree

Followup to patch 152ed04799.

Reported-by: Alvaro Herrera
Discussion: https://postgr.es/m/20201112202900[email protected]

Backpatch-through: 9.5

4 years agodoc: improve wording of the need for analyze of exp. indexes
Bruce Momjian [Mon, 16 Nov 2020 15:26:17 +0000 (10:26 -0500)]
doc:  improve wording of the need for analyze of exp. indexes

This is a followup commit on 3370207986.

Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/20201112211143[email protected]

Backpatch-through: 9.5

4 years agoFix typo
Alvaro Herrera [Mon, 16 Nov 2020 13:54:11 +0000 (10:54 -0300)]
Fix typo

Introduced in 90fdc259866e; backpatch to 12.

Author: Erik Rijkers 
Discussion: https://postgr.es/m/e92b3fba98a0c0f7afc0a2a37e765954@xs4all.nl

4 years agoFix fuzzy thinking about amcanmulticol versus amcaninclude.
Tom Lane [Sun, 15 Nov 2020 21:10:48 +0000 (16:10 -0500)]
Fix fuzzy thinking about amcanmulticol versus amcaninclude.

These flags should be independent: in particular an index AM should
be able to say that it supports include columns without necessarily
supporting multiple key columns.  The included-columns patch got
this wrong, possibly aided by the fact that it didn't bother to
update the documentation.

While here, clarify some text about amcanreturn, which was a little
vague about what should happen when amcanreturn reports that only
some of the index columns are returnable.

Noted while reviewing the SP-GiST included-columns patch, which
quite incorrectly (and unsafely) changed SP-GiST to claim
amcanmulticol = true as a workaround for this bug.

Backpatch to v11 where included columns were introduced.

4 years agoDoc: improve partitioning discussion in ddl.sgml.
Tom Lane [Sat, 14 Nov 2020 18:09:53 +0000 (13:09 -0500)]
Doc: improve partitioning discussion in ddl.sgml.

This started with the intent to explain that range upper bounds
are exclusive, which previously you could only find out by reading
the CREATE TABLE man page.  But I soon found that section 5.11
really could stand a fair amount of editorial attention.  It's
apparently been revised several times without much concern for
overall flow, nor careful copy-editing.

Back-patch to v11, which is as far as the patch goes easily.

Per gripe from Edson Richter.  Thanks to David Johnston for review.

Discussion: https://postgr.es/m/DM6PR13MB3988736CF8F5DC5720440231CFE60@DM6PR13MB3988.namprd13.prod.outlook.com

4 years agodoc: clarify where to find pg_type_d.h (PG 11+) and pg_type.h
Bruce Momjian [Thu, 12 Nov 2020 20:13:01 +0000 (15:13 -0500)]
doc:  clarify where to find pg_type_d.h (PG 11+) and pg_type.h

These files are in compiled directories and install directories.

Reported-by: [email protected]
Discussion: https://postgr.es/m/160379609706.24746.7506163279454026608@wrigleys.postgresql.org

Backpatch-through: 9.5

4 years agodocs: mention that expression indexes need analyze
Bruce Momjian [Thu, 12 Nov 2020 20:00:44 +0000 (15:00 -0500)]
docs: mention that expression indexes need analyze

Expression indexes can't benefit from pre-computed statistics on
columns.

Reported-by: Nikolay Samokhvalov
Discussion: https://postgr.es/m/CANNMO++5rw9RDA=p40iMVbMNPaW6O=S0AFzTU=KpYHRpCd1voA@mail.gmail.com

Author: Nikolay Samokhvalov, modified

Backpatch-through: 9.5

4 years agodoc: wire protocol data type for history file content is bytea
Bruce Momjian [Thu, 12 Nov 2020 19:33:28 +0000 (14:33 -0500)]
doc:  wire protocol data type for history file content is bytea

Document that though the history file content is marked as bytea, it is
the same a text, and neither is btyea-escaped or encoding converted.

Reported-by: Brar Piening
Discussion: https://postgr.es/m/6a1b9cd9-17e3-df67-be55-86102af6bdf5@gmx.de

Backpatch-through: 13 - 9.5 (not master)

4 years agopg_trgm: fix crash in 2-item picksplit
Andrew Gierth [Thu, 12 Nov 2020 14:34:37 +0000 (14:34 +0000)]
pg_trgm: fix crash in 2-item picksplit

Whether from size overflow in gistSplit or from secondary splits,
picksplit is (rarely) called with exactly two items to split.

Formerly, due to special-case handling of the last item, this would
lead to access to an uninitialized cache entry; prior to PG 13 this
might have been harmless or at worst led to an incorrect union datum,
but in 13 onwards it can cause a backend crash from using an
uninitialized pointer.

Repair by removing the special case, which was deemed not to have been
appropriate anyway. Backpatch all the way, because this bug has
existed since pg_trgm was added.

Per report on IRC from user "ftzdomino". Analysis and testing by me,
patch from Alexander Korotkov.

Discussion: https://postgr.es/m/[email protected]

4 years agoFix typo in contrib/pg_trgm/pg_trgm--1.4--1.5.sql
Alexander Korotkov [Thu, 12 Nov 2020 05:55:09 +0000 (08:55 +0300)]
Fix typo in contrib/pg_trgm/pg_trgm--1.4--1.5.sql

Backpatch-through: 13

4 years agoFix name of the macro for getting signature length trgm_gist.c
Alexander Korotkov [Thu, 12 Nov 2020 03:19:16 +0000 (06:19 +0300)]
Fix name of the macro for getting signature length trgm_gist.c

911e702077 has introduced the opclass parameters including signature length
for a set of GiST opclasses.  Due to copy-pasting, macro for getting the
signature length in trgm_gist.c was named LTREE_GET_ASIGLEN().  Fix that by
renaming this macro to just GET_SIGLEN().

Backpatch-through: 13

4 years agoRemove useless SHA256 initialization when not using backup manifests
Michael Paquier [Thu, 12 Nov 2020 01:56:40 +0000 (10:56 +0900)]
Remove useless SHA256 initialization when not using backup manifests

Attempting to take a base backup with Postgres linking to a build of
OpenSSL with FIPS enabled currently fails with or even without a backup
manifest requested because of this mandatory SHA256 initialization used
for the manifest file itself.  However, there is no need to do this
initialization at all if backup manifests are not needed because there
is no data to append to the manifest.

Note that being able to use backup manifests with OpenSSL+FIPS requires
a switch of the SHA2 implementation to use EVP, which would cause an ABI
breakage so this cannot be backpatched to 13 as it has been already
released, but at least avoiding this SHA256 initialization gives users
the possibility to take a base backup even when specifying --no-manifest
with pg_basebackup.

Author: Michael Paquier
Discussion: https://postgr.es/m/20201110020014[email protected]
Backpatch-through: 13

4 years agoRemove duplicate code in brin_memtuple_initialize
Tomas Vondra [Wed, 11 Nov 2020 17:37:36 +0000 (18:37 +0100)]
Remove duplicate code in brin_memtuple_initialize

Commit 8bf74967dab moved some of the code from brin_new_memtuple to
brin_memtuple_initialize, but this resulted in some of the code being
duplicate. Fix by removing the duplicate lines and backpatch to 10.

Author: Tomas Vondra
Backpatch-through: 10
Discussion: https://postgr.es/m/5eb50c97-9a8e-b691-8c40-1b2a55611c4c%40enterprisedb.com

4 years agoFix and simplify some usages of TimestampDifference().
Tom Lane [Wed, 11 Nov 2020 03:51:19 +0000 (22:51 -0500)]
Fix and simplify some usages of TimestampDifference().

Introduce TimestampDifferenceMilliseconds() to simplify callers
that would rather have the difference in milliseconds, instead of
the select()-oriented seconds-and-microseconds format.  This gets
rid of at least one integer division per call, and it eliminates
some apparently-easy-to-mess-up arithmetic.

Two of these call sites were in fact wrong:

* pg_prewarm's autoprewarm_main() forgot to multiply the seconds
by 1000, thus ending up with a delay 1000X shorter than intended.
That doesn't quite make it a busy-wait, but close.

* postgres_fdw's pgfdw_get_cleanup_result() thought it needed to compute
microseconds not milliseconds, thus ending up with a delay 1000X longer
than intended.  Somebody along the way had noticed this problem but
misdiagnosed the cause, and imposed an ad-hoc 60-second limit rather
than fixing the units.  This was relatively harmless in context, because
we don't care that much about exactly how long this delay is; still,
it's wrong.

There are a few more callers of TimestampDifference() that don't
have a direct need for seconds-and-microseconds, but can't use
TimestampDifferenceMilliseconds() either because they do need
microsecond precision or because they might possibly deal with
intervals long enough to overflow 32-bit milliseconds.  It might be
worth inventing another API to improve that, but that seems outside
the scope of this patch; so those callers are untouched here.

Given the fact that we are fixing some bugs, and the likelihood
that future patches might want to back-patch code that uses this
new API, back-patch to all supported branches.

Alexey Kondratov and Tom Lane

Discussion: https://postgr.es/m/3b1c053a21c07c1ed5e00be3b2b855ef@postgrespro.ru

4 years agodoc: fix spelling "connction" to "connection"
Bruce Momjian [Wed, 11 Nov 2020 00:18:35 +0000 (19:18 -0500)]
doc: fix spelling "connction" to "connection"

Was wrong in commit 1a9388bd0f.

Reported-by: Tom Lane, Justin Pryzby
Discussion: https://postgr.es/m/20201102063333[email protected]

Backpatch-through: 9.5

4 years agoWork around cross-version-upgrade issues created by commit 9e38c2bb5.
Tom Lane [Tue, 10 Nov 2020 23:32:36 +0000 (18:32 -0500)]
Work around cross-version-upgrade issues created by commit 9e38c2bb5.

Summarily changing the STYPE of regression-test aggregates that
depend on array_append or array_cat is an issue for the buildfarm's
cross-version-upgrade tests, because those aggregates (as defined
in the back branches) now won't load into HEAD.  Although this seems
like only a minimal risk for genuine user-defined aggregates, we
need to do something for the buildfarm.  Hence, adjust the aggregate
definitions, in both HEAD and the back branches.

Discussion: https://postgr.es/m/1401824.1604537031@sss.pgh.pa.us
Discussion: https://postgr.es/m/[email protected]

4 years agoStamp 13.1. REL_13_1
Tom Lane [Mon, 9 Nov 2020 22:24:30 +0000 (17:24 -0500)]
Stamp 13.1.

4 years agoLast-minute updates for release notes.
Tom Lane [Mon, 9 Nov 2020 18:02:13 +0000 (13:02 -0500)]
Last-minute updates for release notes.

Security: CVE-2020-25694, CVE-2020-25695, CVE-2020-25696

4 years agoDoc: clarify data type behavior of COALESCE and NULLIF.
Tom Lane [Mon, 9 Nov 2020 17:02:24 +0000 (12:02 -0500)]
Doc: clarify data type behavior of COALESCE and NULLIF.

After studying the code, NULLIF is a lot more subtle than you might
have guessed.

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

4 years agoIgnore attempts to \gset into specially treated variables.
Noah Misch [Mon, 9 Nov 2020 15:32:09 +0000 (07:32 -0800)]
Ignore attempts to \gset into specially treated variables.

If an interactive psql session used \gset when querying a compromised
server, the attacker could execute arbitrary code as the operating
system account running psql.  Using a prefix not found among specially
treated variables, e.g. every lowercase string, precluded the attack.
Fix by issuing a warning and setting no variable for the column in
question.  Users wanting the old behavior can use a prefix and then a
meta-command like "\set HISTSIZE :prefix_HISTSIZE".  Back-patch to 9.5
(all supported versions).

Reviewed by Robert Haas.  Reported by Nick Cleaton.

Security: CVE-2020-25696

4 years agoIn security-restricted operations, block enqueue of at-commit user code.
Noah Misch [Mon, 9 Nov 2020 15:32:09 +0000 (07:32 -0800)]
In security-restricted operations, block enqueue of at-commit user code.

Specifically, this blocks DECLARE ... WITH HOLD and firing of deferred
triggers within index expressions and materialized view queries.  An
attacker having permission to create non-temp objects in at least one
schema could execute arbitrary SQL functions under the identity of the
bootstrap superuser.  One can work around the vulnerability by disabling
autovacuum and not manually running ANALYZE, CLUSTER, REINDEX, CREATE
INDEX, VACUUM FULL, or REFRESH MATERIALIZED VIEW.  (Don't restore from
pg_dump, since it runs some of those commands.)  Plain VACUUM (without
FULL) is safe, and all commands are fine when a trusted user owns the
target object.  Performance may degrade quickly under this workaround,
however.  Back-patch to 9.5 (all supported versions).

Reviewed by Robert Haas.  Reported by Etienne Stalmans.

Security: CVE-2020-25695

4 years agoTranslation updates
Peter Eisentraut [Mon, 9 Nov 2020 11:34:05 +0000 (12:34 +0100)]
Translation updates

Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: 2ffedf5ea37677f39cdc1eb92a1e78762cd3fb0e

4 years agoRemove incorrect %s in string
Magnus Hagander [Mon, 9 Nov 2020 09:36:49 +0000 (10:36 +0100)]
Remove incorrect %s in string

Appears to have been a copy/paste error in the original commit that
moved the messages to fe_utils/.

Author: Tang, Haiying 
Backpatch-through: 13
Discussion: https://postgr.es/m/3321cbcea76d4d2c8320a05c19b9304a@G08CNEXMBPEKD05.g08.fujitsu.local

4 years agoRelease notes for 13.1, 12.5, 11.10, 10.15, 9.6.20, 9.5.24.
Tom Lane [Sun, 8 Nov 2020 20:16:12 +0000 (15:16 -0500)]
Release notes for 13.1, 12.5, 11.10, 10.15, 9.6.20, 9.5.24.

4 years agoIn INSERT/UPDATE, use the table's real tuple descriptor as target.
Tom Lane [Sun, 8 Nov 2020 18:08:36 +0000 (13:08 -0500)]
In INSERT/UPDATE, use the table's real tuple descriptor as target.

This back-patches commit 20d3fe900 into the v12 and v13 branches.
At the time I thought that commit was not fixing any observable
bug, but Bertrand Drouvot showed otherwise: adding a dropped column
to the previously-considered scenario crashes v12 and v13, unless the
dropped column happens to be an integer.  That is, of course, because
the tupdesc we derive from the plan output tlist fails to describe
the dropped column accurately, so that we'll do the wrong thing with
a tuple in which that column isn't NULL.

There is no bug in pre-v12 branches because they already did use
the table's real tuple descriptor for any trigger-returned tuple.
It seems that this set of bugs can be blamed on the changes that
removed es_trig_tuple_slot, though I've not attempted to pin that
down precisely.

Although there's no code change needed in HEAD, update the test case
to include a dropped column there too.

Discussion: https://postgr.es/m/db5d97c8-f48a-51e2-7b08-b73d5434d425@amazon.com
Discussion: https://postgr.es/m/16644-5da7ef98a7ac4545@postgresql.org

4 years agoFix test for error message change
Peter Eisentraut [Sun, 8 Nov 2020 06:49:07 +0000 (07:49 +0100)]
Fix test for error message change

fix for f3ad4fddfaf71e8f6f037cd627f398ba43625ca1

4 years agoMessage style improvements
Alvaro Herrera [Sat, 7 Nov 2020 22:33:43 +0000 (19:33 -0300)]
Message style improvements

* Avoid pointlessly highlighting that an index vacuum was executed by a
  parallel worker; user doesn't care.

* Don't give the impression that a non-concurrent reindex of an invalid
  index on a TOAST table would work, because it wouldn't.

* Add a "translator:" comment for a mysterious message.

Discussion: https://postgr.es/m/20201107034943[email protected]
Reviewed-by: Michael Paquier
4 years agoFix redundant error messages in client tools
Peter Eisentraut [Sat, 7 Nov 2020 21:15:52 +0000 (22:15 +0100)]
Fix redundant error messages in client tools

A few client tools duplicate error messages already provided by libpq.

Discussion: https://www.postgresql.org/message-id/flat/3e937641-88a1-e697-612e-99bba4b8e5e4%40enterprisedb.com

4 years agoAvoid re-using output variables in new ecpg test case.
Tom Lane [Sat, 7 Nov 2020 21:25:42 +0000 (16:25 -0500)]
Avoid re-using output variables in new ecpg test case.

The buildfarm thinks this leads to memory stomps, though annoyingly
I can't duplicate that here.  The existing code in strings.pgc is
doing something that doesn't seem to be sanctioned at all really
by the documentation, but I'm disinclined to try to make that nicer
right now.  Let's just declare some more output variables in hopes
of working around it.

4 years agoDoc: small release note updates.
Tom Lane [Sat, 7 Nov 2020 20:27:36 +0000 (15:27 -0500)]
Doc: small release note updates.

Add items committed in the last 24 hours.  Also correct my failure
to credit Nikita Glukhov for 52ad1e659, which did all the heavy
lifting for 3db322eaa.

4 years agoFix ecpg's mishandling of B'...' and X'...' literals.
Tom Lane [Sat, 7 Nov 2020 20:03:44 +0000 (15:03 -0500)]
Fix ecpg's mishandling of B'...' and X'...' literals.

These were broken in multiple ways:

* The xbstart and xhstart lexer actions neglected to set
"state_before_str_start" before transitioning to the xb/xh states,
thus possibly resulting in "internal error: unreachable state" later.

* The test for valid string contents at the end of xb state was flat out
wrong, as it accounted incorrectly for the "b" prefix that the xbstart
action had injected.  Meanwhile, the xh state had no such check at all.

* The generated literal value failed to include any quote marks.

* The grammar did the wrong thing anyway, typically ignoring the
literal value and emitting something else, since BCONST and XCONST
tokens were handled randomly differently from SCONST tokens.

The first of these problems is evidently an oversight in commit
7f380c59f, but the others seem to be very ancient.  The lack of
complaints shows that ECPG users aren't using these syntaxes much
(although I do vaguely remember one previous complaint).

As written, this patch is dependent on 7f380c59f, so it can't go
back further than v13.  Given the shortage of complaints, I'm not
excited about adapting the patch to prior branches.

Report and patch by Shenhao Wang (test case adjusted by me)

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

4 years agoPlug memory leak in index_get_partition
Alvaro Herrera [Sat, 7 Nov 2020 01:52:15 +0000 (22:52 -0300)]
Plug memory leak in index_get_partition

The list of indexes was being leaked when asked for an index that
doesn't have an index partition in the table partition.  Not a common
case admittedly --and in most cases where it occurs, caller throws an
error anyway-- but worth fixing for cleanliness and in case any
third-party code is calling this function.

While at it, remove use of lfirst_oid() to obtain a value we already
have.

Author: Justin Pryzby 
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/20201105203606[email protected]

4 years agoProperly detoast data in brin_form_tuple
Tomas Vondra [Fri, 6 Nov 2020 23:40:06 +0000 (00:40 +0100)]
Properly detoast data in brin_form_tuple

brin_form_tuple failed to consider the values may be toasted, inserting
the toast pointer into the index. This may easily result in index
corruption, as the toast data may be deleted and cleaned up by vacuum.
The cleanup however does not care about indexes, leaving invalid toast
pointers behind, which triggers errors like this:

  ERROR:  missing chunk number 0 for toast value 16433 in pg_toast_16426

A less severe consequence are inconsistent failures due to the index row
being too large, depending on whether brin_form_tuple operated on plain
or toasted version of the row. For example

    CREATE TABLE t (val TEXT);
    INSERT INTO t VALUES ('... long value ...')
    CREATE INDEX idx ON t USING brin (val);

would likely succeed, as the row would likely include toast pointer.
Switching the order of INSERT and CREATE INDEX would likely fail:

    ERROR:  index row size 8712 exceeds maximum 8152 for index "idx"

because this happens before the row values are toasted.

The bug exists since PostgreSQL 9.5 where BRIN indexes were introduced.
So backpatch all the way back.

Author: Tomas Vondra
Reviewed-by: Alvaro Herrera
Backpatch-through: 9.5
Discussion: https://postgr.es/m/20201001184133.oq5uq75sb45pu3aw@development
Discussion: https://postgr.es/m/20201104010544.zexj52mlldagzowv%40development

4 years agoFirst-draft release notes for 13.1.
Tom Lane [Fri, 6 Nov 2020 22:05:43 +0000 (17:05 -0500)]
First-draft release notes for 13.1.

As usual, the release notes for other branches will be made by cutting
these down, but put them up for community review first.

Also as usual for a .1 release, there are some entries here that
are not really relevant for v13 because they already appeared in 13.0.
Those'll be removed later.

4 years agoRevert "Accept relations of any kind in LOCK TABLE".
Tom Lane [Fri, 6 Nov 2020 21:17:56 +0000 (16:17 -0500)]
Revert "Accept relations of any kind in LOCK TABLE".

Revert 59ab4ac32, as well as the followup fix 33862cb9c, in all
branches.  We need to think a bit harder about what the behavior
of LOCK TABLE on views should be, and there's no time for that
before next week's releases.  We'll take another crack at this
later.

Discussion: https://postgr.es/m/16703-e348f58aab3cf6cc@postgresql.org

4 years agoRevert "pg_dump: Lock all relations, not just plain tables".
Tom Lane [Fri, 6 Nov 2020 20:48:21 +0000 (15:48 -0500)]
Revert "pg_dump: Lock all relations, not just plain tables".

Revert 403a3d91c, as well as the followup fix 7f4235032, in all
branches.  We need to think a bit harder about what the behavior
of LOCK TABLE on views should be, and there's no time for that
before next week's releases.  We'll take another crack at this
later.

Discussion: https://postgr.es/m/16703-e348f58aab3cf6cc@postgresql.org

4 years agoDon't throw an error for LOCK TABLE on a self-referential view.
Tom Lane [Thu, 5 Nov 2020 16:44:32 +0000 (11:44 -0500)]
Don't throw an error for LOCK TABLE on a self-referential view.

LOCK TABLE has complained about "infinite recursion" when applied
to a self-referential view, ever since we made it recurse into views
in v11.  However, that breaks pg_dump's new assumption that it's
okay to lock every relation.  There doesn't seem to be any good
reason to throw an error: if we just abandon the recursion, we've
still satisfied the requirement of locking every referenced relation.

Per bug #16703 from Andrew Bille (via Alexander Lakhin).

Discussion: https://postgr.es/m/16703-e348f58aab3cf6cc@postgresql.org

4 years agoFix nbtree cleanup-only VACUUM stats inaccuracies.
Peter Geoghegan [Thu, 5 Nov 2020 02:42:24 +0000 (18:42 -0800)]
Fix nbtree cleanup-only VACUUM stats inaccuracies.

Logic for counting heap TIDs from posting list tuples (added by commit
0d861bbb) was faulty.  It didn't count any TIDs/index tuples in the
event of no callback being set.  This meant that we incorrectly counted
no index tuples in clean-up only VACUUMs, which could lead to
pg_class.reltuples being spuriously set to 0 in affected indexes.

To fix, go back to counting items from the page in cases where there is
no callback.  This approach isn't very accurate, but it works well
enough in practice while avoiding the expense of accessing every index
tuple during cleanup-only VACUUMs.

Author: Peter Geoghegan 
Reported-By: Jehan-Guillaume de Rorthais
https://postgr.es/m/20201023174451.69e358f1@firost
Backpatch: 13-, where nbtree deduplication was introduced

4 years agoEnable hash partitioning of text arrays
Peter Eisentraut [Wed, 4 Nov 2020 06:47:06 +0000 (07:47 +0100)]
Enable hash partitioning of text arrays

hash_array_extended() needs to pass PG_GET_COLLATION() to the hash
function of the element type.  Otherwise, the hash function of a
collation-aware data type such as text will error out, since the
introduction of nondeterministic collation made hash functions require
a collation, too.

The consequence of this is that before this change, hash partitioning
using an array over text in the partition key would not work.

Reviewed-by: Heikki Linnakangas
Reviewed-by: Tom Lane
Reviewed-by: Michael Paquier
Discussion: https://www.postgresql.org/message-id/flat/32c1fdae-95c6-5dc6-058a-a90330a3b621%40enterprisedb.com

4 years agoUse INT64_FORMAT to print int64 variables in sort debug
Tomas Vondra [Tue, 3 Nov 2020 19:43:12 +0000 (20:43 +0100)]
Use INT64_FORMAT to print int64 variables in sort debug

Commit 6ee3b5fb99 cleaned up most of the long/int64 confusion related to
incremental sort, but the sort debug messages were still using %ld for
int64 variables. So fix that.

Author: Haiying Tang
Backpatch-through: 13, where the incremental sort code was added
Discussion: https://postgr.es/m/4250be9d350c4992abb722a76e288aef%40G08CNEXMBPEKD05.g08.fujitsu.local

4 years agoFix get_useful_pathkeys_for_relation for volatile expressions
Tomas Vondra [Tue, 3 Nov 2020 19:07:23 +0000 (20:07 +0100)]
Fix get_useful_pathkeys_for_relation for volatile expressions

When considering Incremental Sort below a Gather Merge, we need to be
a bit more careful when matching pathkeys to EC members. It's not enough
to find a member whose Vars are all in the current relation's target;
volatile expressions in particular need to be contained in the target,
otherwise it's too early to use the pathkey.

Reported-by: Jaime Casanova
Author: James Coleman
Reviewed-by: Tomas Vondra
Backpatch-through: 13, where the incremental sort code was added
Discussion: https://postgr.es/m/CAJGNTeNaxpXgBVcRhJX%2B2vSbq%2BF2kJqGBcvompmpvXb7pq%2BoFA%40mail.gmail.com

4 years agoGuard against core dump from uninitialized subplan.
Tom Lane [Tue, 3 Nov 2020 21:16:36 +0000 (16:16 -0500)]
Guard against core dump from uninitialized subplan.

If the planner erroneously puts a non-parallel-safe SubPlan into
a parallelized portion of the query tree, nodeSubplan.c will fail
in the worker processes because it finds a null in es_subplanstates,
which it's unable to cope with.  It seems worth a test-and-elog to
make that an error case rather than a core dump case.

This probably should have been included in commit 16ebab688, which
was responsible for allowing nulls to appear in es_subplanstates
to begin with.  So, back-patch to v10 where that came in.

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

4 years agoAllow users with BYPASSRLS to alter their own passwords.
Tom Lane [Tue, 3 Nov 2020 20:41:32 +0000 (15:41 -0500)]
Allow users with BYPASSRLS to alter their own passwords.

The intention in commit 491c029db was to require superuserness to
change the BYPASSRLS property, but the actual effect of the coding
in AlterRole() was to require superuserness to change anything at all
about a BYPASSRLS role.  Other properties of a BYPASSRLS role should
be changeable under the same rules as for a normal role, though.

Fix that, and also take care of some documentation omissions related
to BYPASSRLS and REPLICATION role properties.

Tom Lane and Stephen Frost, per bug report from Wolfgang Walther.
Back-patch to all supported branches.

Discussion: https://postgr.es/m/a5548a9f-89ee-3167-129d-162b5985fcf8@technowledgy.de

4 years agoDisallow ALTER TABLE ONLY / DROP EXPRESSION
Peter Eisentraut [Tue, 3 Nov 2020 14:14:50 +0000 (15:14 +0100)]
Disallow ALTER TABLE ONLY / DROP EXPRESSION

The current implementation cannot handle this correctly, so just
forbid it for now.

GENERATED clauses must be attached to the column definition and cannot
be added later like DEFAULT, so if a child table has a generation
expression that the parent does not have, the child column will
necessarily be an attlocal column.  So to implement ALTER TABLE ONLY /
DROP EXPRESSION, we'd need extra code to update attislocal of the
direct child tables, somewhat similar to how DROP COLUMN does it, so
that the resulting state can be properly dumped and restored.

Discussion: https://www.postgresql.org/message-id/flat/15830.1575468847%40sss.pgh.pa.us

4 years agoFix unportable use of getnameinfo() in pg_hba_file_rules view.
Tom Lane [Tue, 3 Nov 2020 02:11:50 +0000 (21:11 -0500)]
Fix unportable use of getnameinfo() in pg_hba_file_rules view.

fill_hba_line() thought it could get away with passing sizeof(struct
sockaddr_storage) rather than the actual addrlen previously returned
by getaddrinfo().  While that appears to work on many platforms,
it does not work on FreeBSD 11: you get back a failure, which leads
to the view showing NULL for the address and netmask columns in all
rows.  The POSIX spec for getnameinfo() is pretty clearly on
FreeBSD's side here: you should pass the actual address length.
So it seems plausible that there are other platforms where this
coding also fails, and we just hadn't noticed.

Also, IMO the fact that getnameinfo() failure leads to a NULL output
is pretty bogus in itself.  Our pg_getnameinfo_all() wrapper is
careful to emit "???" on failure, and we should use that in such
cases.  NULL should only be emitted in rows that don't have IP
addresses.

Per bug #16695 from Peter Vandivier.  Back-patch to v10 where this
code was added.

Discussion: https://postgr.es/m/16695-a665558e2f630be7@postgresql.org

4 years agoSecond thoughts on TOAST decompression.
Tom Lane [Mon, 2 Nov 2020 16:25:18 +0000 (11:25 -0500)]
Second thoughts on TOAST decompression.

On detecting a corrupted match tag, pglz_decompress() should just
summarily return -1.  Breaking out of the loop, as I did in dfc797730,
doesn't quite guarantee that will happen.  Also, we can use
unlikely() on that check, just in case it helps.

Backpatch to v13, like the previous patch.

4 years agoAdd missing comma in list of SSL versions
Magnus Hagander [Mon, 2 Nov 2020 14:20:19 +0000 (15:20 +0100)]
Add missing comma in list of SSL versions

4 years agoFix some grammar and typos in comments and docs
Michael Paquier [Mon, 2 Nov 2020 06:15:20 +0000 (15:15 +0900)]
Fix some grammar and typos in comments and docs

The documentation fixes are backpatched down to where they apply.

Author: Justin Pryzby
Discussion: https://postgr.es/m/20201031020801[email protected]
Backpatch-through: 9.6

4 years agoExtend PageIsVerified() to handle more custom options
Michael Paquier [Mon, 2 Nov 2020 01:41:23 +0000 (10:41 +0900)]
Extend PageIsVerified() to handle more custom options

This is useful for checks of relation pages without having to load the
pages into the shared buffers, and two cases can make use of that: page
verification in base backups and the online, lock-safe, flavor.

Compatibility is kept with past versions using a routine that calls the
new extended routine with the set of options compatible with the
original version.  Contrary to d401c576, a macro cannot be used as there
may be external code relying on the presence of the original routine.

This is applied down to 11, where this will be used by a follow-up
commit addressing a set of issues with page verification in base
backups.

Extracted from a larger patch by the same author.

Author: Anastasia Lubennikova
Reviewed-by: Michael Paquier, Julien Rouhaud
Discussion: https://postgr.es/m/608f3476-0598-2514-2c03-e05c7d2b0cbd@postgrespro.ru
Backpatch-through: 11

4 years agoFix two issues in TOAST decompression.
Tom Lane [Sun, 1 Nov 2020 23:38:42 +0000 (18:38 -0500)]
Fix two issues in TOAST decompression.

pglz_maximum_compressed_size() potentially underestimated the amount
of compressed data required to produce N bytes of decompressed data;
this is a fault in commit 11a078cf8.

Separately from that, pglz_decompress() failed to protect itself
against corrupt compressed data, particularly off == 0 in a match
tag.  Commit c60e520f6 turned such a situation into an infinite loop,
where before it'd just have resulted in garbage output.

The combination of these two bugs seems like it may explain bug #16694
from Tom Vijlbrief, though it's impossible to be quite sure without
direct inspection of the failing session.  (One needs to assume that
the pglz_maximum_compressed_size() bug caused us to fail to fetch the
second byte of a match tag, and what happened to be there instead was
a zero.  The reported infinite loop is hard to explain without off == 0,
though.)

Aside from fixing the bugs, rewrite associated comments for more
clarity.

Back-patch to v13 where both these commits landed.

Discussion: https://postgr.es/m/16694-f107871e499ec114@postgresql.org

4 years agoAvoid null pointer dereference if error result lacks SQLSTATE.
Tom Lane [Sun, 1 Nov 2020 16:26:16 +0000 (11:26 -0500)]
Avoid null pointer dereference if error result lacks SQLSTATE.

Although error results received from the backend should always have
a SQLSTATE field, ones generated by libpq won't, making this code
vulnerable to a crash after, say, untimely loss of connection.
Noted by Coverity.

Oversight in commit 403a3d91c.  Back-patch to 9.5, as that was.

4 years agoPreserve index data in pg_statistic across REINDEX CONCURRENTLY
Michael Paquier [Sun, 1 Nov 2020 12:24:10 +0000 (21:24 +0900)]
Preserve index data in pg_statistic across REINDEX CONCURRENTLY

Statistics associated to an index got lost after running REINDEX
CONCURRENTLY, while the non-concurrent case preserves these correctly.
The concurrent and non-concurrent operations need to be consistent for
the end-user, and missing statistics would force to wait for a new
analyze to happen, which could take some time depending on the activity
of the existing autovacuum workers.  This issue is fixed by copying any
existing entries in pg_statistic associated to the old index to the new
one.  Note that this copy is already done with the data of the index in
the stats collector.

Reported-by: Fabrízio de Royes Mello
Author: Michael Paquier, Fabrízio de Royes Mello
Reviewed-by: Justin Pryzby
Discussion: https://postgr.es/m/CAFcNs+qpFPmiHd1oTXvcPdvAHicJDA9qBUSujgAhUMJyUMb+SA@mail.gmail.com
Backpatch-through: 12

4 years agoReproduce debug_query_string==NULL on parallel workers.
Noah Misch [Sat, 31 Oct 2020 15:43:28 +0000 (08:43 -0700)]
Reproduce debug_query_string==NULL on parallel workers.

Certain background workers initiate parallel queries while
debug_query_string==NULL, at which point they attempted strlen(NULL) and
died to SIGSEGV.  Older debug_query_string observers allow NULL, so do
likewise in these newer ones.  Back-patch to v11, where commit
7de4a1bcc56f494acbd0d6e70781df877dc8ecb5 introduced the first of these.

Discussion: https://postgr.es/m/20201014022636[email protected]

4 years agoStabilize timetz test across DST transitions.
Tom Lane [Thu, 29 Oct 2020 19:28:14 +0000 (15:28 -0400)]
Stabilize timetz test across DST transitions.

The timetz test cases I added in commit a9632830b were unintentionally
sensitive to whether or not DST is active in the PST8PDT time zone.
Thus, they'll start failing this coming weekend, as reported by
Bernhard M. Wiedemann in bug #16689.  Fortunately, DST-awareness is
not significant to the purpose of these test cases, so we can just
force them all to PDT (DST hours) to preserve stability of the
results.

Back-patch to v10, as the prior patch was.

Discussion: https://postgr.es/m/16689-57701daa23b377bf@postgresql.org

4 years agoUse mode "r" for popen() in psql's evaluate_backtick().
Tom Lane [Wed, 28 Oct 2020 18:35:53 +0000 (14:35 -0400)]
Use mode "r" for popen() in psql's evaluate_backtick().

In almost all other places, we use plain "r" or "w" mode in popen()
calls (the exceptions being for COPY data).  This one has been
overlooked (possibly because it's buried in a ".l" flex file?),
but it's using PG_BINARY_R.

Kensuke Okamura complained in bug #16688 that we fail to strip \r
when stripping the trailing newline from a backtick result string.
That's true enough, but we'd also fail to convert embedded \r\n
cleanly, which also seems undesirable.  Fixing the popen() mode
seems like the best way to deal with this.

It's been like this for a long time, so back-patch to all supported
branches.

Discussion: https://postgr.es/m/16688-c649c7b69cd7e6f8@postgresql.org

4 years agoCalculate extraUpdatedCols in query rewriter, not parser.
Tom Lane [Wed, 28 Oct 2020 17:47:02 +0000 (13:47 -0400)]
Calculate extraUpdatedCols in query rewriter, not parser.

It's unsafe to do this at parse time because addition of generated
columns to a table would not invalidate stored rules containing
UPDATEs on the table ... but there might now be dependent generated
columns that were not there when the rule was made.  This also fixes
an oversight that rewriteTargetView failed to update extraUpdatedCols
when transforming an UPDATE on an updatable view.  (Since the new
calculation is downstream of that, rewriteTargetView doesn't actually
need to do anything; but before, there was a demonstrable bug there.)

In v13 and HEAD, this leads to easily-visible bugs because (since
commit c6679e4fc) we won't recalculate generated columns that aren't
listed in extraUpdatedCols.  In v12 this bitmap is mostly just used
for trigger-firing decisions, so you'd only notice a problem if a
trigger cared whether a generated column had been updated.

I'd complained about this back in May, but then forgot about it
until bug #16671 from Michael Paul Killian revived the issue.

Back-patch to v12 where this field was introduced.  If existing
stored rules contain any extraUpdatedCols values, they'll be
ignored because the rewriter will overwrite them, so the bug will
be fixed even for existing rules.  (But note that if someone were
to update to 13.1 or 12.5, store some rules with UPDATEs on tables
having generated columns, and then downgrade to a prior minor version,
they might observe issues similar to what this patch fixes.  That
seems unlikely enough to not be worth going to a lot of effort to fix.)

Discussion: https://postgr.es/m/10206.1588964727@sss.pgh.pa.us
Discussion: https://postgr.es/m/16671-2fa55851859fb166@postgresql.org

4 years agoMakefile comment: remove reference to tools/thread/thread_test
Bruce Momjian [Tue, 27 Oct 2020 18:00:38 +0000 (14:00 -0400)]
Makefile comment:  remove reference to tools/thread/thread_test

You can't compile thread_test alone anymore, and the location moved too.

Reported-by: Tom Lane
Discussion: https://postgr.es/m/1062278.1603819969@sss.pgh.pa.us

Backpatch-through: 9.5

4 years agopg_dump: Lock all relations, not just plain tables
Alvaro Herrera [Tue, 27 Oct 2020 17:31:37 +0000 (14:31 -0300)]
pg_dump: Lock all relations, not just plain tables

Now that LOCK TABLE can take any relation type, acquire lock on all
relations that are to be dumped.  This prevents schema changes or
deadlock errors that could cause a dump to fail after expending much
effort.  The server is tested to have the capability and the feature
disabled if it doesn't, so that a patched pg_dump doesn't fail when
connecting to an unpatched server.

Backpatch to 9.5.

Author: Álvaro Herrera 
Reviewed-by: Tom Lane
Reported-by: Wells Oliver
Discussion: https://postgr.es/m/20201021200659[email protected]

4 years agoAccept relations of any kind in LOCK TABLE
Alvaro Herrera [Tue, 27 Oct 2020 16:49:19 +0000 (13:49 -0300)]
Accept relations of any kind in LOCK TABLE

The restriction that only tables and views can be locked by LOCK TABLE
is quite arbitrary, since the underlying mechanism can lock any relation
type.  Drop the restriction so that programs such as pg_dump can lock
all relations they're interested in, preventing schema changes that
could cause a dump to fail after expending much effort.

Backpatch to 9.5.

Author: Álvaro Herrera 
Reviewed-by: Tom Lane
Reported-by: Wells Oliver
Discussion: https://postgr.es/m/20201021200659[email protected]

4 years agodocs: remove reference to src/tools/thread
Bruce Momjian [Tue, 27 Oct 2020 16:43:11 +0000 (12:43 -0400)]
docs:  remove reference to src/tools/thread

This directory and the ability to build the thread test independently
were removed in commit 8a2121185b.

Reported-by: [email protected]
Discussion: https://postgr.es/m/160379609706.24746.7506163279454026608@wrigleys.postgresql.org

Backpatch-through: 9.5

4 years agodoc: simplify wording of function error affects
Bruce Momjian [Tue, 27 Oct 2020 02:38:11 +0000 (22:38 -0400)]
doc:  simplify wording of function error affects

Reported-by: [email protected]
Discussion: https://postgr.es/m/160324449781.693.8298142858847611071@wrigleys.postgresql.org

Backpatch-through: 9.5

4 years agodoc: make blooms docs match reality
Bruce Momjian [Mon, 26 Oct 2020 23:17:05 +0000 (19:17 -0400)]
doc:  make blooms docs match reality

Parallel execution changed the way bloom queries are executed, so update
the EXPLAIN output, and restructure the docs to be clearer and more
accurate.

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

Author: Daniel Westermann and me

Backpatch-through: 9.6

4 years agoFix corner case for a BEFORE ROW UPDATE trigger returning OLD.
Tom Lane [Sun, 25 Oct 2020 17:57:46 +0000 (13:57 -0400)]
Fix corner case for a BEFORE ROW UPDATE trigger returning OLD.

If the old row has any "missing" attributes that are supposed to
be retrieved from an associated tuple descriptor, the wrong things
happened because the trigger result is shoved directly into an
executor slot that lacks the missing-attribute data.  Notably,
CHECK-constraint verification would incorrectly see those columns
as NULL, and so would RETURNING-list evaluation.

Band-aid around this by forcibly expanding the tuple before passing
it to the trigger function.  (IMO it was a fundamental misdesign to
put the missing-attribute data into tuple constraints, which so
much of the system considers to be optional.  But we're probably
stuck with that now, and will have to continue to apply band-aids
as we find other places with similar issues.)

Back-patch to v12.  v11 would also have the issue, except that
commit 920311ab1 already applied a similar band-aid.  That forced
expansion in more cases than seem really necessary, though, so
this isn't a directly equivalent fix.

Amit Langote, with some cosmetic changes by me

Discussion: https://postgr.es/m/16644-5da7ef98a7ac4545@postgresql.org

4 years agoFix incorrect parameter name in a function header comment
David Rowley [Sun, 25 Oct 2020 09:39:00 +0000 (22:39 +1300)]
Fix incorrect parameter name in a function header comment

Author: Zhijie Hou
Discussion: https://postgr.es/m/14cd74ea00204cc8a7ea5d738ac82cd1@G08CNEXMBPEKD05.g08.fujitsu.local
Backpatch-through: 12, where the mistake was introduced

4 years agoFix ancient bug in ecpg's pthread_once() emulation for Windows.
Tom Lane [Sat, 24 Oct 2020 17:12:08 +0000 (13:12 -0400)]
Fix ancient bug in ecpg's pthread_once() emulation for Windows.

We must not set the "done" flag until after we've executed the
initialization function.  Otherwise, other threads can fall through
the initial unlocked test before initialization is really complete.

This has been seen to cause rare failures of ecpg's thread/descriptor
test, and it could presumably cause other sorts of misbehavior in
threaded ECPG-using applications, since ecpglib relies on
pthread_once() in several places.

Diagnosis and patch by me, based on investigation by Alexander Lakhin.
Back-patch to all supported branches (the bug dates to 2007).

Discussion: https://postgr.es/m/16685-d6cd241872c101d3@postgresql.org

4 years agoFix broken XML formatting in EXPLAIN output for incremental sorts.
Tom Lane [Fri, 23 Oct 2020 15:32:33 +0000 (11:32 -0400)]
Fix broken XML formatting in EXPLAIN output for incremental sorts.

The ExplainCloseGroup arguments for incremental sort usage data
didn't match the corresponding ExplainOpenGroup.  This only matters
for XML-format output, which is probably why we'd not noticed.

Daniel Gustafsson, per bug #16683 from Frits Jalvingh

Discussion: https://postgr.es/m/16683-8005033324ad34e9@postgresql.org

4 years agoUpdate time zone data files to tzdata release 2020d.
Tom Lane [Fri, 23 Oct 2020 01:23:47 +0000 (21:23 -0400)]
Update time zone data files to tzdata release 2020d.

DST law changes in Palestine, with a whopping 120 hours' notice.
Also some historical corrections for Palestine.

4 years agoSync our copy of the timezone library with IANA release tzcode2020d.
Tom Lane [Fri, 23 Oct 2020 01:15:22 +0000 (21:15 -0400)]
Sync our copy of the timezone library with IANA release tzcode2020d.

There's no functional change at all here, but I'm curious to see
whether this change successfully shuts up Coverity's warning about
a useless strcmp(), which appeared with the previous update.

Discussion: http://mm.icann.org/pipermail/tz/2020-October/029370.html

4 years agoFix connection string handling in psql's \connect command.
Tom Lane [Wed, 21 Oct 2020 20:18:40 +0000 (16:18 -0400)]
Fix connection string handling in psql's \connect command.

psql's \connect claims to be able to re-use previous connection
parameters, but in fact it only re-uses the database name, user name,
host name (and possibly hostaddr, depending on version), and port.
This is problematic for assorted use cases.  Notably, pg_dump[all]
emits "\connect databasename" commands which we would like to have
re-use all other parameters.  If such a script is loaded in a psql run
that initially had "-d connstring" with some non-default parameters,
those other parameters would be lost, potentially causing connection
failure.  (Thus, this is the same kind of bug addressed in commits
a45bc8a4f and 8e5793ab6, although the details are much different.)

To fix, redesign do_connect() so that it pulls out all properties
of the old PGconn using PQconninfo(), and then replaces individual
properties in that array.  In the case where we don't wish to re-use
anything, get libpq's default settings using PQconndefaults() and
replace entries in that, so that we don't need different code paths
for the two cases.

This does result in an additional behavioral change for cases where
the original connection parameters allowed multiple hosts, say
"psql -h host1,host2", and the \connect request allows re-use of the
host setting.  Because the previous coding relied on PQhost(), it
would only permit reconnection to the same host originally selected.
Although one can think of scenarios where that's a good thing, there
are others where it is not.  Moreover, that behavior doesn't seem to
meet the principle of least surprise, nor was it documented; nor is
it even clear it was intended, since that coding long pre-dates the
addition of multi-host support to libpq.  Hence, this patch is content
to drop it and re-use the host list as given.

Per Peter Eisentraut's comments on bug #16604.  Back-patch to all
supported branches.

Discussion: https://postgr.es/m/16604-933f4b8791227b15@postgresql.org

4 years agoUse fast checkpoint in PostgresNode::backup()
Alvaro Herrera [Wed, 21 Oct 2020 17:37:26 +0000 (14:37 -0300)]
Use fast checkpoint in PostgresNode::backup()

Should cause tests to be a bit faster

4 years agoFix ALTER TABLE .. ENABLE/DISABLE TRIGGER recursion
Alvaro Herrera [Tue, 20 Oct 2020 22:22:09 +0000 (19:22 -0300)]
Fix ALTER TABLE .. ENABLE/DISABLE TRIGGER recursion

More precisely, correctly handle the ONLY flag indicating not to
recurse.  This was implemented in 86f575948c77 by recursing in
trigger.c, but that's the wrong place; use ATSimpleRecursion instead,
which behaves properly.  However, because legacy inheritance has never
recursed in that situation, make sure to do that only for new-style
partitioning.

I noticed this problem while testing a fix for another bug in the
vicinity.

This has been wrong all along, so backpatch to 11.

Discussion: https://postgr.es/m/20201016235925[email protected]

4 years agoAvoid invalid alloc size error in shm_mq
Peter Eisentraut [Mon, 19 Oct 2020 06:52:25 +0000 (08:52 +0200)]
Avoid invalid alloc size error in shm_mq

In shm_mq_receive(), a huge payload could trigger an unjustified
"invalid memory alloc request size" error due to the way the buffer
size is increased.

Add error checks (documenting the upper limit) and avoid the error by
limiting the allocation size to MaxAllocSize.

Author: Markus Wanner 
Discussion: https://www.postgresql.org/message-id/flat/3bb363e7-ac04-0ac4-9fe8-db1148755bfa%402ndquadrant.com

4 years agoFix typo in commit 99ae342fc4.
Amit Kapila [Tue, 20 Oct 2020 03:01:43 +0000 (08:31 +0530)]
Fix typo in commit 99ae342fc4.

In v13, the id for max_parallel_maintenance_workers is defined differently
as compared to HEAD in docs, so adjust the docs accordingly.

Reported-by: Magnus Hagander and Tom Lane
Discussion: https://postgr.es/m/CABUevEyAFQZ_jvjY_KtRUWbci4YMyQC1QAMzDQAbLs=XCo3m5Q@mail.gmail.com

4 years agoFix connection string handling in src/bin/scripts/ programs.
Tom Lane [Mon, 19 Oct 2020 23:03:46 +0000 (19:03 -0400)]
Fix connection string handling in src/bin/scripts/ programs.

When told to process all databases, clusterdb, reindexdb, and vacuumdb
would reconnect by replacing their --maintenance-db parameter with the
name of the target database.  If that parameter is a connstring (which
has been allowed for a long time, though we failed to document that
before this patch), we'd lose any other options it might specify, for
example SSL or GSS parameters, possibly resulting in failure to connect.
Thus, this is the same bug as commit a45bc8a4f fixed in pg_dump and
pg_restore.  We can fix it in the same way, by using libpq's rules for
handling multiple "dbname" parameters to add the target database name
separately.  I chose to apply the same refactoring approach as in that
patch, with a struct to handle the command line parameters that need to
be passed through to connectDatabase.  (Maybe someday we can unify the
very similar functions here and in pg_dump/pg_restore.)

Per Peter Eisentraut's comments on bug #16604.  Back-patch to all
supported branches.

Discussion: https://postgr.es/m/16604-933f4b8791227b15@postgresql.org

4 years agoFix list-munging bug that broke SQL function result coercions.
Tom Lane [Mon, 19 Oct 2020 18:33:02 +0000 (14:33 -0400)]
Fix list-munging bug that broke SQL function result coercions.

Since commit 913bbd88d, check_sql_fn_retval() can either insert type
coercion steps in-line in the Query that produces the SQL function's
results, or generate a new top-level Query to perform the coercions,
if modifying the Query's output in-place wouldn't be safe.  However,
it appears that the latter case has never actually worked, because
the code tried to inject the new Query back into the query list it was
passed ... which is not the list that will be used for later processing
when we execute the SQL function "normally" (without inlining it).
So we ended up with no coercion happening at run-time, leading to
wrong results or crashes depending on the datatypes involved.

While the regression tests look like they cover this area well enough,
through a huge bit of bad luck all the test cases that exercise the
separate-Query path were checking either inline-able cases (which
accidentally didn't have the bug) or cases that are no-ops at runtime
(e.g., varchar to text), so that the failure to perform the coercion
wasn't obvious.  The fact that the cases that don't work weren't
allowed at all before v13 probably contributed to not noticing the
problem sooner, too.

To fix, get rid of the separate "flat" list of Query nodes and instead
pass the real two-level list that is going to be used later.  I chose
to make the same change in check_sql_fn_statements(), although that has
no actual bug, just so that we don't need that data structure at all.

This is an API change, as evidenced by the adjustments needed to
callers outside functions.c.  That's a bit scary to be doing in a
released branch, but so far as I can tell from a quick search,
there are no outside callers of these functions (and they are
sufficiently specific to our semantics for SQL-language functions that
it's not apparent why any extension would need to call them).  In any
case, v13 already changed the API of check_sql_fn_retval() compared to
prior branches.

Per report from pinker.  Back-patch to v13 where this code came in.

Discussion: https://postgr.es/m/1603050466566[email protected]

4 years agoMisc documentation fixes.
Heikki Linnakangas [Mon, 19 Oct 2020 16:28:54 +0000 (19:28 +0300)]
Misc documentation fixes.

- Misc grammar and punctuation fixes.

- Stylistic cleanup: use spaces between function arguments and JSON fields
  in examples. For example "foo(a,b)" -> "foo(a, b)". Add semicolon after
  last END in a few PL/pgSQL examples that were missing them.

- Make sentence that talked about "..." and ".." operators more clear,
  by avoiding to end the sentence with "..". That makes it look the same
  as "..."

- Fix syntax description for HAVING: HAVING conditions cannot be repeated

Patch by Justin Pryzby, per Yaroslav Schekin's report. Backpatch to all
supported versions, to the extent that the patch applies easily.

Discussion: https://www.postgresql.org/message-id/20201005191922.GE17626%40telsasoft.com

4 years agoFix TRUNCATE doc: ALTER SEQUENCE RESTART is now transactional.
Heikki Linnakangas [Mon, 19 Oct 2020 16:02:25 +0000 (19:02 +0300)]
Fix TRUNCATE doc: ALTER SEQUENCE RESTART is now transactional.

ALTER SEQUENCE RESTART was made transactional in commit 3d79013b97.
Backpatch to v10, where that was introduced.

Patch by Justin Pryzby, per Yaroslav Schekin's report.

Discussion: https://www.postgresql.org/message-id/20201005191922.GE17626%40telsasoft.com

4 years agoFix output of tsquery example in docs.
Heikki Linnakangas [Mon, 19 Oct 2020 15:50:33 +0000 (18:50 +0300)]
Fix output of tsquery example in docs.

The output for this query changed in commit 4e2477b7b8. Backport to 9.6
like that commit.

Patch by Justin Pryzby, per Yaroslav Schekin's report.

Discussion: https://www.postgresql.org/message-id/20201005191922.GE17626%40telsasoft.com

4 years agoIn libpq for Windows, call WSAStartup once and WSACleanup not at all.
Tom Lane [Mon, 19 Oct 2020 15:23:51 +0000 (11:23 -0400)]
In libpq for Windows, call WSAStartup once and WSACleanup not at all.

The Windows documentation insists that every WSAStartup call should
have a matching WSACleanup call.  However, if that ever had actual
relevance, it wasn't in this century.  Every remotely-modern Windows
kernel is capable of cleaning up when a process exits without doing
that, and must be so to avoid resource leaks in case of a process
crash.  Moreover, Postgres backends have done WSAStartup without
WSACleanup since commit 4cdf51e64 in 2004, and we've never seen any
indication of a problem with that.

libpq's habit of doing WSAStartup during connection start and
WSACleanup during shutdown is also rather inefficient, since a
series of non-overlapping connection requests leads to repeated,
quite expensive DLL unload/reload cycles.  We document a workaround
for that (having the application call WSAStartup for itself), but
that's just a kluge.  It's also worth noting that it's far from
uncommon for applications to exit without doing PQfinish, and
we've not heard reports of trouble from that either.

However, the real reason for acting on this is that recent
experiments by Alexander Lakhin show that calling WSACleanup
during PQfinish is triggering the symptom we occasionally see
that a process using libpq fails to emit expected stdio output.

Therefore, let's change libpq so that it calls WSAStartup only
once per process, during the first connection attempt, and never
calls WSACleanup at all.

While at it, get rid of the only other WSACleanup call in our code
tree, in pg_dump/parallel.c; that presumably is equally useless.

Back-patch of HEAD commit 7d00a6b2d.

Discussion: https://postgr.es/m/ac976d8c-03df-d6b8-025c-15a2de8d9af1@postgrespro.ru

4 years agoFix doc for full text search distance operator.
Heikki Linnakangas [Mon, 19 Oct 2020 14:58:38 +0000 (17:58 +0300)]
Fix doc for full text search distance operator.

Commit 028350f619 changed its behavior from "at most" to "exactly", but
forgot to update the documentation. Backpatch to 9.6.

Patch by Justin Pryzby, per Yaroslav Schekin's report.

Discussion: https://www.postgresql.org/message-id/20201005191922.GE17626%40telsasoft.com

4 years agoUpdate link for pllua
Magnus Hagander [Mon, 19 Oct 2020 11:47:09 +0000 (13:47 +0200)]
Update link for pllua

Author: Daniel Gustafsson 
Discussion: https://postgr.es/m/A05874AE-8771-4C61-A24E-0B6249B8F3C2@yesql.se

4 years agoRelax some asserts in merge join costing code
David Rowley [Mon, 19 Oct 2020 11:06:40 +0000 (00:06 +1300)]
Relax some asserts in merge join costing code

In the planner, it was possible, given an extreme enough case containing a
large number of joins for the number of estimated rows to become infinite.
This could cause problems in initial_cost_mergejoin() where we perform
some calculations based on those row estimates.

A problem case, presented by Onder Kalaci showed an Assert failure from
an Assert checking outerstartsel <= outerendsel.  In his test case this
was effectively NaN <= Inf, which is false.  The NaN outerstartsel came
from multiplying the infinite outer_path_rows by 0.0.

In master, this problem was fixed by a90c950fc, however, that fix was too
invasive for the backbranches.  Here we just relax the Asserts to allow
them to pass.  The worst that appears to happen from this is that we show
NaN cost values and infinite row estimates in EXPLAIN.  add_path() would
have had a hard time doing anything useful with such costs, but that does
not really matter as if the row estimates were even close to accurate,
such plan would not complete this side of the heat death of the universe.

Reported-by: Onder Kalaci
Backpatch: 9.5 to 13
Discussion: https://postgr.es/m/DM6PR21MB1211FF360183BCA901B27F04D80B0@DM6PR21MB1211.namprd21.prod.outlook.com

4 years agoChange the docs for PARALLEL option of Vacuum.
Amit Kapila [Mon, 19 Oct 2020 04:04:04 +0000 (09:34 +0530)]
Change the docs for PARALLEL option of Vacuum.

The rules to choose the number of parallel workers to perform parallel
vacuum operation were not clearly specified.

Reported-by: Peter Eisentraut
Author: Amit Kapila
Backpatch-through: 13, where it was introduced
Discussion: https://postgr.es/m/36aa8aea-61b7-eb3c-263b-648e0cb117b7@2ndquadrant.com

4 years agoFix potential memory leak in pgcrypto
Michael Paquier [Mon, 19 Oct 2020 00:37:50 +0000 (09:37 +0900)]
Fix potential memory leak in pgcrypto

When allocating a EVP context, it would have been possible to leak some
memory allocated directly by OpenSSL, that PostgreSQL lost track of if
the initialization of the context allocated failed.  The cleanup can be
done with EVP_MD_CTX_destroy().

Note that EVP APIs exist since OpenSSL 0.9.7 and we have in the tree
equivalent implementations for older versions since ce9b75d (code
removed with 9b7cd59a as of 10~).  However, in 9.5 and 9.6, the existing
code makes use of EVP_MD_CTX_destroy() and EVP_MD_CTX_create() without
an equivalent implementation when building the tree with OpenSSL 0.9.6
or older, meaning that this code is in reality broken with such versions
since it got introduced in e2838c5.  As we have heard no complains about
that, it does not seem worth bothering with in 9.5 and 9.6, so I have
left that out for simplicity.

Author: Michael Paquier
Discussion: https://postgr.es/m/20201015072212[email protected]
Backpatch-through: 9.5

4 years agoDoc: caution against misuse of 'now' and related datetime literals.
Tom Lane [Sat, 17 Oct 2020 20:02:47 +0000 (16:02 -0400)]
Doc: caution against misuse of 'now' and related datetime literals.

Section 8.5.1.4, which defines these literals, made only a vague
reference to the fact that they might be evaluated too soon to be
safe in non-interactive contexts.  Provide a more explicit caution
against misuse.  Also, generalize the wording in the related tip in
section 9.9.4: while it clearly described this problem, it implied
(or really, stated outright) that the problem only applies to table
DEFAULT clauses.

Per gripe from Tijs van Dam.  Back-patch to all supported branches.

Discussion: https://postgr.es/m/c2LuRv9BiRT3bqIo5mMQiVraEXey_25B4vUn0kDqVqilwOEu_iVF1tbtvLnyQK7yDG3PFaz_GxLLPil2SDkj1MCObNRVaac-7j1dVdFERk8=@thalex.com

4 years agoUpdate time zone data files to tzdata release 2020c.
Tom Lane [Sat, 17 Oct 2020 01:53:33 +0000 (21:53 -0400)]
Update time zone data files to tzdata release 2020c.

DST law changes in Morocco, Canadian Yukon, Fiji, Macquarie Island,
Casey Station (Antarctica).  Historical corrections for France,
Hungary, Monaco.

4 years agoSync our copy of the timezone library with IANA release tzcode2020c.
Tom Lane [Sat, 17 Oct 2020 01:40:16 +0000 (21:40 -0400)]
Sync our copy of the timezone library with IANA release tzcode2020c.

This changes zic's default output format from "-b fat" to "-b slim".
We were already using "slim" in v13/HEAD, so those branches drop
the explicit -b switch in the Makefiles.  Instead, add an explicit
"-b fat" in v12 and before, so that we don't change the output file
format in those branches.  (This is perhaps excessively conservative,
but we decided not to do so in a12079109, and I'll stick with that.)

Other non-cosmetic changes are to drop support for zic's long-obsolete
"-y" switch, and to ensure that strftime() does not change errno
unless it fails.

As usual with tzcode changes, back-patch to all supported branches.

4 years agoAdd missing error check in pgcrypto/crypt-md5.c.
Tom Lane [Fri, 16 Oct 2020 15:59:13 +0000 (11:59 -0400)]
Add missing error check in pgcrypto/crypt-md5.c.

In theory, the second px_find_digest call in px_crypt_md5 could fail
even though the first one succeeded, since resource allocation is
required.  Don't skip testing for a failure.  (If one did happen,
the likely result would be a crash rather than clean recovery from
an OOM failure.)

The code's been like this all along, so back-patch to all supported
branches.

Daniel Gustafsson

Discussion: https://postgr.es/m/AA8D6FE9-4AB2-41B4-98CB-AE64BA668C03@yesql.se