postgresql.git
3 years agoUpdate time zone data files to tzdata release 2021e.
Tom Lane [Fri, 29 Oct 2021 15:38:18 +0000 (11:38 -0400)]
Update time zone data files to tzdata release 2021e.

DST law changes in Fiji, Jordan, Palestine, and Samoa.  Historical
corrections for Barbados, Cook Islands, Guyana, Niue, Portugal, and
Tonga.

Also, the Pacific/Enderbury zone has been renamed to Pacific/Kanton.
The following zones have been merged into nearby, more-populous zones
whose clocks have agreed since 1970: Africa/Accra, America/Atikokan,
America/Blanc-Sablon, America/Creston, America/Curacao,
America/Nassau, America/Port_of_Spain, Antarctica/DumontDUrville,
and Antarctica/Syowa.

3 years agoImprove contrib/amcheck's tests for CREATE INDEX CONCURRENTLY.
Tom Lane [Thu, 28 Oct 2021 15:45:14 +0000 (11:45 -0400)]
Improve contrib/amcheck's tests for CREATE INDEX CONCURRENTLY.

Commits fdd965d07 and 3cd9c3b92 tested CREATE INDEX CONCURRENTLY by
launching two separate pgbench runs concurrently.  This was needed so
that only a single client thread would run CREATE INDEX CONCURRENTLY,
avoiding deadlock between two CICs.  However, there's a better way,
which is to use an advisory lock to prevent concurrent CICs.  That's
better in part because the test code is shorter and more readable, but
mostly because it automatically scales things to launch an appropriate
number of CICs relative to the number of INSERT transactions.
As committed, typically half to three-quarters of the CIC transactions
were pointless because the INSERT transactions had already stopped.

In passing, remove background_pgbench, which was added to support
these tests and isn't needed anymore.  We can always put it back
if we find a use for it later.

Back-patch to v12; older pgbench versions lack the
conditional-execution features needed for this method.

Tom Lane and Andrey Borodin

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

3 years agodoc: Fix link to SELinux user guide in sepgsql page
Michael Paquier [Thu, 28 Oct 2021 00:26:18 +0000 (09:26 +0900)]
doc: Fix link to SELinux user guide in sepgsql page

Reported-by: Anton Voloshin
Discussion: https://postgr.es/m/15a86d4e-a237-1acd-18a2-fd69730f1ab9@postgrespro.ru
Backpatch-through: 10

3 years agoFix ordering of items in nbtree error message.
Peter Geoghegan [Wed, 27 Oct 2021 20:09:00 +0000 (13:09 -0700)]
Fix ordering of items in nbtree error message.

Oversight in commit a5213adf.

Backpatch: 13-, just like commit a5213adf.

3 years agoFurther harden nbtree posting split code.
Peter Geoghegan [Wed, 27 Oct 2021 19:10:43 +0000 (12:10 -0700)]
Further harden nbtree posting split code.

Add more defensive checks around posting list split code.  These should
detect corruption involving duplicate table TIDs earlier and more
reliably than any existing check.

Follow up to commit 8f72bbac.

Discussion: https://postgr.es/m/CAH2-WzkrSY_kjyd1_M5xJK1uM0govJXMxPn8JUSvwcUOiHuWVw@mail.gmail.com
Backpatch: 13-, where nbtree deduplication was introduced.

3 years agoClarify that --system reindexes system catalogs *only*
Magnus Hagander [Wed, 27 Oct 2021 14:20:02 +0000 (16:20 +0200)]
Clarify that --system reindexes system catalogs *only*

Make this more clear both in the help message and docs.

Reviewed-By: Michael Paquier
Backpatch-through: 9.6
Discussion: https://postgr.es/m/CABUevEw6Je0WUFTLhPKOk4+BoBuDrE-fKw3N4ckqgDBMFu4paA@mail.gmail.com

3 years agoReject huge_pages=on if shared_memory_type=sysv.
Thomas Munro [Mon, 25 Oct 2021 23:54:55 +0000 (12:54 +1300)]
Reject huge_pages=on if shared_memory_type=sysv.

It doesn't work (it could, but hasn't been implemented).
Back-patch to 12, where shared_memory_type arrived.

Reported-by: Alexander Lakhin
Reviewed-by: Alexander Lakhin
Discussion: https://postgr.es/m/163271880203.22789.1125998876173795966@wrigleys.postgresql.org

3 years agoFix CREATE INDEX CONCURRENTLY for the newest prepared transactions.
Noah Misch [Sun, 24 Oct 2021 01:36:38 +0000 (18:36 -0700)]
Fix CREATE INDEX CONCURRENTLY for the newest prepared transactions.

The purpose of commit 8a54e12a38d1545d249f1402f66c8cde2837d97c was to
fix this, and it sufficed when the PREPARE TRANSACTION completed before
the CIC looked for lock conflicts.  Otherwise, things still broke.  As
before, in a cluster having used CIC while having enabled prepared
transactions, queries that use the resulting index can silently fail to
find rows.  It may be necessary to reindex to recover from past
occurrences; REINDEX CONCURRENTLY suffices.  Fix this for future index
builds by making CIC wait for arbitrarily-recent prepared transactions
and for ordinary transactions that may yet PREPARE TRANSACTION.  As part
of that, have PREPARE TRANSACTION transfer locks to its dummy PGPROC
before it calls ProcArrayClearTransaction().  Back-patch to 9.6 (all
supported versions).

Andrey Borodin, reviewed (in earlier versions) by Andres Freund.

Discussion: https://postgr.es/m/01824242-AA92-4FE9-9BA7-AEBAFFEA3D0C@yandex-team.ru

3 years agoAvoid race in RelationBuildDesc() affecting CREATE INDEX CONCURRENTLY.
Noah Misch [Sun, 24 Oct 2021 01:36:38 +0000 (18:36 -0700)]
Avoid race in RelationBuildDesc() affecting CREATE INDEX CONCURRENTLY.

CIC and REINDEX CONCURRENTLY assume backends see their catalog changes
no later than each backend's next transaction start.  That failed to
hold when a backend absorbed a relevant invalidation in the middle of
running RelationBuildDesc() on the CIC index.  Queries that use the
resulting index can silently fail to find rows.  Fix this for future
index builds by making RelationBuildDesc() loop until it finishes
without accepting a relevant invalidation.  It may be necessary to
reindex to recover from past occurrences; REINDEX CONCURRENTLY suffices.
Back-patch to 9.6 (all supported versions).

Noah Misch and Andrey Borodin, reviewed (in earlier versions) by Andres
Freund.

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

3 years agodoc: Describe calculation method of streaming start for pg_receivewal
Michael Paquier [Sat, 23 Oct 2021 05:43:45 +0000 (14:43 +0900)]
doc: Describe calculation method of streaming start for pg_receivewal

The documentation was imprecise about the starting LSN used for WAL
streaming if nothing can be found in the local archive directory
defined with the pg_receivewal command, so be more talkative on this
matter.

Extracted from a larger patch by the same author.

Author: Ronan Dunklau, Michael Paquier
Discussion: https://postgr.es/m/18708360.4lzOvYHigE@aivenronan
Backpatch-through: 10

3 years agoFix frontend version of sh_error() in simplehash.h.
Tom Lane [Fri, 22 Oct 2021 20:43:38 +0000 (16:43 -0400)]
Fix frontend version of sh_error() in simplehash.h.

The code does not expect sh_error() to return, but the patch
that made this header usable in frontend didn't get that memo.

While here, plaster unlikely() on the tests that decide whether
to invoke sh_error(), and add our standard copyright notice.

Noted by Andres Freund.  Back-patch to v13 where this frontend
support came in.

Discussion: https://postgr.es/m/0D54435C-1199-4361-9D74-2FBDCF8EA164@anarazel.de

3 years agopg_dump: fix mis-dumping of non-global default privileges.
Tom Lane [Fri, 22 Oct 2021 19:22:26 +0000 (15:22 -0400)]
pg_dump: fix mis-dumping of non-global default privileges.

Non-global default privilege entries should be dumped as-is,
not made relative to the default ACL for their object type.
This would typically only matter if one had revoked some
on-by-default privileges in a global entry, and then wanted
to grant them again in a non-global entry.

Per report from Boris Korzun.  This is an old bug, so back-patch
to all supported branches.

Neil Chen, test case by Masahiko Sawada

Discussion: https://postgr.es/m/111621616618184@mail.yandex.ru
Discussion: https://postgr.es/m/CAA3qoJnr2+1dVJObNtfec=qW4Z0nz=A9+r5bZKoTSy5RDjskMw@mail.gmail.com

3 years agoBack-patch "Add parent table name in an error in reorderbuffer.c."
Amit Kapila [Thu, 21 Oct 2021 04:06:27 +0000 (09:36 +0530)]
Back-patch "Add parent table name in an error in reorderbuffer.c."

This was originally done in commit 5e77625b26 for 15 only, as a
troubleshooting aid but multiple people showed interest in back-patching
this.

Author: Jeremy Schneider
Reviewed-by: Amit Kapila
Backpatch-through: 9.6
Discussion: https://postgr.es/m/808ed65b-994c-915a-361c-577f088b837f@amazon.com

3 years agoProtect against collation variations in test
Alvaro Herrera [Wed, 20 Oct 2021 16:05:42 +0000 (13:05 -0300)]
Protect against collation variations in test

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

3 years agoFix build of MSVC with OpenSSL 3.0.0
Michael Paquier [Wed, 20 Oct 2021 07:49:00 +0000 (16:49 +0900)]
Fix build of MSVC with OpenSSL 3.0.0

The build scripts of Visual Studio would fail to detect properly a 3.0.0
build as the check on the second digit was failing.  This is adjusted
where needed, allowing the builds to complete.  Note that the MSIs of
OpenSSL mentioned in the documentation have not changed any library
names for Win32 and Win64, making this change straight-forward.

Reported-by: htalaco, via github
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 9.6

3 years agoEnsure correct lock level is used in ALTER ... RENAME
Alvaro Herrera [Tue, 19 Oct 2021 22:08:45 +0000 (19:08 -0300)]
Ensure correct lock level is used in ALTER ... RENAME

Commit 1b5d797cd4f7 intended to relax the lock level used to rename
indexes, but inadvertently allowed *any* relation to be renamed with a
lowered lock level, as long as the command is spelled ALTER INDEX.
That's undesirable for other relation types, so retry the operation with
the higher lock if the relation turns out not to be an index.

After this fix, ALTER INDEX  RENAME will require access
exclusive lock, which it didn't before.

Author: Nathan Bossart 
Author: Álvaro Herrera 
Reported-by: Onder Kalaci
Discussion: https://postgr.es/m/PH0PR21MB1328189E2821CDEC646F8178D8AE9@PH0PR21MB1328.namprd21.prod.outlook.com

3 years agoAdapt src/test/ldap/t/001_auth.pl to work with openldap 2.5.
Andres Freund [Tue, 19 Oct 2021 17:14:49 +0000 (10:14 -0700)]
Adapt src/test/ldap/t/001_auth.pl to work with openldap 2.5.

ldapsearch's deprecated -h/-p arguments were removed, need to use -H now -
which has been around for over 20 years.

As perltidy insists on reflowing the parameters anyway, change order and
"phrasing" to yield a less confusing layout (per suggestion from Tom Lane).

Discussion: https://postgr.es/m/20211009233850[email protected]
Backpatch: 11-, where the tests were added.

3 years agoFix assignment to array of domain over composite.
Tom Lane [Tue, 19 Oct 2021 17:54:46 +0000 (13:54 -0400)]
Fix assignment to array of domain over composite.

An update such as "UPDATE ... SET fld[n].subfld = whatever"
failed if the array elements were domains rather than plain
composites.  That's because isAssignmentIndirectionExpr()
failed to cope with the CoerceToDomain node that would appear
in the expression tree in this case.  The result would typically
be a crash, and even if we accidentally didn't crash, we'd not
correctly preserve other fields of the same array element.

Per report from Onder Kalaci.  Back-patch to v11 where arrays of
domains came in.

Discussion: https://postgr.es/m/PH0PR21MB132823A46AA36F0685B7A29AD8BD9@PH0PR21MB1328.namprd21.prod.outlook.com

3 years agoRemove bogus assertion in transformExpressionList().
Tom Lane [Tue, 19 Oct 2021 15:35:15 +0000 (11:35 -0400)]
Remove bogus assertion in transformExpressionList().

I think when I added this assertion (in commit 8f889b108), I was only
thinking of the use of transformExpressionList at top level of INSERT
and VALUES.  But it's also called by transformRowExpr(), which can
certainly occur in an UPDATE targetlist, so it's inappropriate to
suppose that p_multiassign_exprs must be empty.  Besides, since the
input is not expected to contain ResTargets, there's no reason it
should contain MultiAssignRefs either.  Hence this code need not
be concerned about the state of p_multiassign_exprs, and we should
just drop the assertion.

Per bug #17236 from ocean_li_996.  It's been wrong for years,
so back-patch to all supported branches.

Discussion: https://postgr.es/m/17236-3210de9bcba1d7ca@postgresql.org

3 years agoFix bug in TOC file error message printing
Daniel Gustafsson [Tue, 19 Oct 2021 10:59:54 +0000 (12:59 +0200)]
Fix bug in TOC file error message printing

If the blob TOC file cannot be parsed, the error message was failing
to print the filename as the variable holding it was shadowed by the
destination buffer for parsing.  When the filename fails to parse,
the error will print an empty string:

 ./pg_restore -d foo -F d dump
 pg_restore: error: invalid line in large object TOC file "": ..

..instead of the intended error message:

 ./pg_restore -d foo -F d dump
 pg_restore: error: invalid line in large object TOC file "dump/blobs.toc": ..

Fix by renaming both variables as the shared name was too generic to
store either and still convey what the variable held.

Backpatch all the way down to 9.6.

Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/A2B151F5-B32B-4F2C-BA4A-6870856D9BDE@yesql.se
Backpatch-through: 9.6

3 years agoFix sscanf limits in pg_basebackup and pg_dump
Daniel Gustafsson [Tue, 19 Oct 2021 10:59:50 +0000 (12:59 +0200)]
Fix sscanf limits in pg_basebackup and pg_dump

Make sure that the string parsing is limited by the size of the
destination buffer.

In pg_basebackup the available values sent from the server
is limited to two characters so there was no risk of overflow.

In pg_dump the buffer is bounded by MAXPGPATH, and thus the limit
must be inserted via preprocessor expansion and the buffer increased
by one to account for the terminator. There is no risk of overflow
here, since in this case, the buffer scanned is smaller than the
destination buffer.

Backpatch the pg_basebackup fix to 11 where it was introduced, and
the pg_dump fix all the way down to 9.6.

Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/B14D3D7B-F98C-4E20-9459-C122C67647FB@yesql.se
Backpatch-through: 11 and 9.6

3 years agoBlock ALTER INDEX/TABLE index_name ALTER COLUMN colname SET (options)
Michael Paquier [Tue, 19 Oct 2021 02:04:04 +0000 (11:04 +0900)]
Block ALTER INDEX/TABLE index_name ALTER COLUMN colname SET (options)

The grammar of this command run on indexes with column names has always
been authorized by the parser, and it has never been documented.

Since 911e702, it is possible to define opclass parameters as of CREATE
INDEX, which actually broke the old case of ALTER INDEX/TABLE where
relation-level parameters n_distinct and n_distinct_inherited could be
defined for an index (see 76a47c0 and its thread where this point has
been touched, still remained unused).  Attempting to do that in v13~
would cause the index to become unusable, as there is a new dedicated
code path to load opclass parameters instead of the relation-level ones
previously available.  Note that it is possible to fix things with a
manual catalog update to bring the relation back online.

This commit disables this command for now as the use of column names for
indexes does not make sense anyway, particularly when it comes to index
expressions where names are automatically computed.  One way to properly
support this case properly in the future would be to use column numbers
when it comes to indexes, in the same way as ALTER INDEX .. ALTER COLUMN
.. SET STATISTICS.

Partitioned indexes were already blocked, but not indexes.  Some tests
are added for both cases.

There was some code in ANALYZE to enforce n_distinct to be used for an
index expression if the parameter was defined, but just remove it for
now until/if there is support for this (note that index-level parameters
never had support in pg_dump either, previously), so this was just dead
code.

Reported-by: Matthijs van der Vleuten
Author: Nathan Bossart, Michael Paquier
Reviewed-by: Vik Fearing, Dilip Kumar
Discussion: https://postgr.es/m/17220-15d684c6c2171a83@postgresql.org
Backpatch-through: 13

3 years agoInvalidate partitions of table being attached/detached
Alvaro Herrera [Mon, 18 Oct 2021 22:08:25 +0000 (19:08 -0300)]
Invalidate partitions of table being attached/detached

Failing to do that, any direct inserts/updates of those partitions
would fail to enforce the correct constraint, that is, one that
considers the new partition constraint of their parent table.

Backpatch to 10.

Reported by: Hou Zhijie 
Author: Amit Langote 
Author: Álvaro Herrera 
Reviewed-by: Nitin Jadhav
Reviewed-by: Pavel Borisov
Discussion: https://postgr.es/m/OS3PR01MB5718DA1C4609A25186D1FBF194089%40OS3PR01MB5718.jpnprd01.prod.outlook.com

3 years agoReset properly snapshot export state during transaction abort
Michael Paquier [Mon, 18 Oct 2021 02:56:52 +0000 (11:56 +0900)]
Reset properly snapshot export state during transaction abort

During a replication slot creation, an ERROR generated in the same
transaction as the one creating a to-be-exported snapshot would have
left the backend in an inconsistent state, as the associated static
export snapshot state was not being reset on transaction abort, but only
on the follow-up command received by the WAL sender that created this
snapshot on replication slot creation.  This would trigger inconsistency
failures if this session tried to export again a snapshot, like during
the creation of a replication slot.

Note that a snapshot export cannot happen in a transaction block, so
there is no need to worry resetting this state for subtransaction
aborts.  Also, this inconsistent state would very unlikely show up to
users.  For example, one case where this could happen is an
out-of-memory error when building the initial snapshot to-be-exported.
Dilip found this problem while poking at a different patch, that caused
an error in this code path for reasons unrelated to HEAD.

Author: Dilip Kumar
Reviewed-by: Michael Paquier, Zhihong Yu
Discussion: https://postgr.es/m/CAFiTN-s0zA1Kj0ozGHwkYkHwa5U0zUE94RSc_g81WrpcETB5=w@mail.gmail.com
Backpatch-through: 9.6

3 years agoAvoid core dump in pg_dump when dumping from pre-8.3 server.
Tom Lane [Sat, 16 Oct 2021 19:02:55 +0000 (15:02 -0400)]
Avoid core dump in pg_dump when dumping from pre-8.3 server.

Commit f0e21f2f6 missed adding a tgisinternal output column
to getTriggers' query for pre-8.3 servers.  Back-patch to v11,
like that commit.

3 years agoMake pg_dump acquire lock on partitioned tables that are to be dumped.
Tom Lane [Sat, 16 Oct 2021 16:23:57 +0000 (12:23 -0400)]
Make pg_dump acquire lock on partitioned tables that are to be dumped.

It was clearly the intent to do so all along, but the original coding
fat-fingered this by checking the wrong array element.  We fixed it
in passing in 403a3d91c, but that later got reverted, and we forgot
to keep this bug fix.

Most of the time this'd be relatively harmless, since once we lock
any of the partitioned table's leaf partitions, that would suffice
to prevent major DDL on the partitioned table itself.  However, a
childless partitioned table would get dumped with no relevant lock
whatsoever, possibly allowing dump failure or inconsistent output.

Unlike 403a3d91c, there are no versioning concerns, since every server
version that has partitioned tables will allow you to lock one.

Back-patch to v10 where partitioned tables were introduced.

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

3 years agoCheck criticalSharedRelcachesBuilt in GetSharedSecurityLabel().
Jeff Davis [Thu, 14 Oct 2021 19:24:47 +0000 (12:24 -0700)]
Check criticalSharedRelcachesBuilt in GetSharedSecurityLabel().

An extension may want to call GetSecurityLabel() on a shared object
before the shared relcaches are fully initialized. For instance, a
ClientAuthentication_hook might want to retrieve the security label on
a role.

Discussion: https://postgr.es/m/ecb7af0b26e3be1d96d291c8453a86f1f82d9061[email protected]
Backpatch-through: 9.6

3 years agoFix planner error with pulling up subquery expressions into function RTEs.
Tom Lane [Thu, 14 Oct 2021 16:43:43 +0000 (12:43 -0400)]
Fix planner error with pulling up subquery expressions into function RTEs.

If a function-in-FROM laterally references the output of some sub-SELECT
earlier in the FROM clause, and we are able to flatten that sub-SELECT
into the outer query, the expression(s) copied into the function RTE
missed being processed by eval_const_expressions.  This'd lead to trouble
and probable crashes at execution if such expressions contained
named-argument function call syntax or functions with defaulted arguments.
The bug is masked if the query contains any explicit JOIN syntax, which
may help explain why we'd not noticed.

Per bug #17227 from Bernd Dorn.  This is an oversight in commit 7266d0997,
so back-patch to v13 where that came in.

Discussion: https://postgr.es/m/17227-5a28ed1512189fa4@postgresql.org

3 years agoChange recently added test code for stability
Alvaro Herrera [Wed, 13 Oct 2021 21:49:27 +0000 (18:49 -0300)]
Change recently added test code for stability

The test code added with ff9f111bce24 fails under valgrind, and probably
other slow cases too, because if (say) autovacuum runs in between and
produces WAL of its own, the large INSERT fails to account for that in
the LSN calculations.  Rewrite to use a DO loop.

Per complaint from Andres Freund

Backpatch to all branches.

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

3 years agopostgres_fdw: Move comments about elog level in (sub)abort cleanup.
Etsuro Fujita [Wed, 13 Oct 2021 10:00:03 +0000 (19:00 +0900)]
postgres_fdw: Move comments about elog level in (sub)abort cleanup.

The comments were misplaced when adding postgres_fdw.  Fix that by
moving the comments to more appropriate functions.

Author: Etsuro Fujita
Backpatch-through: 9.6
Discussion: https://postgr.es/m/CAPmGK164sAXQtC46mDFyu6d-T25Mzvh5qaRNkit06VMmecYnOA%40mail.gmail.com

3 years agoFix tests of pg_upgrade across different major versions
Michael Paquier [Wed, 13 Oct 2021 00:22:00 +0000 (09:22 +0900)]
Fix tests of pg_upgrade across different major versions

This fixes a set of issues that cause different breakages or annoyances
when using pg_upgrade's test.sh to do upgrades across different major
versions:
- test.sh is completely broken when using v14 as new version because of
the removal of testtablespace/ as Makefile rule.  Older versions of
pg_regress don't support --make-tablespacedir, blocking the creation of
the tablespace.  In order to fix that, it is simple enough to create
those directories in the script itself, but only do that when an old
version is involved.  This fix is needed on HEAD and REL_14_STABLE.
- The script would fail when using PG <= v11 as old version because of
WITH OIDS relations not supported in v12.  In order to fix this, this
steals a method from the buildfarm that uses a DO block to change all
the relations marked as WITH OIDS, allowing pg_upgrade to pass.  This is
more portable than using ALTER TABLE queries on the relations causing
issues.  This is fixed down to v12, and authored originally by Andrew
Dunstan.
- Not using --extra-float-digits=0 with v11 as old version causes
a lot of diffs in the dumps, making the whole unreadable.  This gets
only done when using v11 as old version.  This is fixed down to v12.
The buildfarm code uses that already.

Note that the addition of --wal-segsize and --allow-group-access breaks
the script when using v10 or older at initdb time as these got added in
11.  10 would be EOL'd next year and nobody has complained about those
problems yet, so nothing is done about that.  This means that this
commit fixes upgrade tests using test.sh with v11 as minimum older
version, up to HEAD, and that it is enough to apply this change down to
12.  The old and new dumps still generate diffs, still require manual
checks, and more could be done to reduce the noise, but this allows the
tests to run with a rather minimal amount of them.

I have tested this commit and test.sh with v11 as minimum across all the
branches where this is applied.  Note that this commit has no impact on
the normal pg_upgrade test run with a simple "make check".

Author:  Justin Pryzby, Andrew Dunstan, Michael Paquier
Discussion: https://postgr.es/m/20201206180248[email protected]
Backpatch-through: 12

3 years agoAdd more $Test::Builder::Level in the TAP tests
Michael Paquier [Tue, 12 Oct 2021 02:16:25 +0000 (11:16 +0900)]
Add more $Test::Builder::Level in the TAP tests

Incrementing the level of the call stack reported is useful for
debugging purposes as it allows to control which part of the test is
exactly failing, especially if a test is structured with subroutines
that call routines from Test::More.

This adds more incrementations of $Test::Builder::Level where debugging
gets improved (for example it does not make sense for some paths like
pg_rewind where long subroutines are used).

A note is added to src/test/perl/README about that, based on a
suggestion from Andrew Dunstan and a wording coming from both of us.

Usage of Test::Builder::Level has spread in 12, so a backpatch down to
this version is done.

Reviewed-by: Andrew Dunstan, Peter Eisentraut, Daniel Gustafsson
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 12

3 years agoAdd missing word to comment in joinrels.c.
Etsuro Fujita [Thu, 7 Oct 2021 08:45:03 +0000 (17:45 +0900)]
Add missing word to comment in joinrels.c.

Author: Amit Langote
Backpatch-through: 13
Discussion: https://postgr.es/m/CA%2BHiwqGQNbtamQ_9DU3osR1XiWR4wxWFZurPmN6zgbdSZDeWmw%40mail.gmail.com

3 years agoFix null-pointer crash in postgres_fdw's conversion_error_callback.
Tom Lane [Wed, 6 Oct 2021 19:50:24 +0000 (15:50 -0400)]
Fix null-pointer crash in postgres_fdw's conversion_error_callback.

Commit c7b7311f6 adjusted conversion_error_callback to always use
information from the query's rangetable, to avoid doing catalog lookups
in an already-failed transaction.  However, as a result of the utterly
inadequate documentation for make_tuple_from_result_row, I failed to
realize that fsstate could be NULL in some contexts.  That led to a
crash if we got a conversion error in such a context.  Fix by falling
back to the previous coding when fsstate is NULL.  Improve the
commentary, too.

Per report from Andrey Borodin.  Back-patch to 9.6, like the previous
patch.

Discussion: https://postgr.es/m/08916396-55E4-4D68-AB3A-BD6066F9E5C0@yandex-team.ru

3 years agoFix corner-case loss of precision in numeric_power().
Dean Rasheed [Wed, 6 Oct 2021 12:20:23 +0000 (13:20 +0100)]
Fix corner-case loss of precision in numeric_power().

This fixes a loss of precision that occurs when the first input is
very close to 1, so that its logarithm is very small.

Formerly, during the initial low-precision calculation to estimate the
result weight, the logarithm was computed to a local rscale that was
capped to NUMERIC_MAX_DISPLAY_SCALE (1000). However, the base may be
as close as 1e-16383 to 1, hence its logarithm may be as small as
1e-16383, and so the local rscale needs to be allowed to exceed 16383,
otherwise all precision is lost, leading to a poor choice of rscale
for the full-precision calculation.

Fix this by removing the cap on the local rscale during the initial
low-precision calculation, as we already do in the full-precision
calculation. This doesn't change the fact that the initial calculation
is a low-precision approximation, computing the logarithm to around 8
significant digits, which is very fast, especially when the base is
very close to 1.

Patch by me, reviewed by Alvaro Herrera.

Discussion: https://postgr.es/m/CAEZATCV-Ceu%2BHpRMf416yUe4KKFv%3DtdgXQAe5-7S9tD%3D5E-T1g%40mail.gmail.com

3 years agoFix warning in TAP test of pg_verifybackup
Michael Paquier [Wed, 6 Oct 2021 04:28:35 +0000 (13:28 +0900)]
Fix warning in TAP test of pg_verifybackup

Oversight in a3fcbcd.

Reported-by: Thomas Munro
Discussion: https://postgr.es/m/CA+hUKGKnajZEwe91OTjro9kQLCMGGFHh2vvFn8tgHgbyn4bF9w@mail.gmail.com
Backpatch-through: 13

3 years agoDoc: improve description of UNION/INTERSECT/EXCEPT syntax.
Tom Lane [Tue, 5 Oct 2021 14:24:14 +0000 (10:24 -0400)]
Doc: improve description of UNION/INTERSECT/EXCEPT syntax.

queries.sgml failed to mention the rather important point that
INTERSECT binds more tightly than UNION or EXCEPT.  I thought
it could also use more discussion of the role of parentheses
in these constructs.

Per gripe from Christopher Painter-Wakefield.

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

3 years agodoc: remove URL for ICU explorer/locexp
Bruce Momjian [Mon, 4 Oct 2021 21:10:59 +0000 (17:10 -0400)]
doc:  remove URL for ICU explorer/locexp

The old URL was HTTP 404 and the git link didn't build.  Also update two
other ICU links.  If we ever get a good link we will add it back.

Reported-by: Anton Voloshin
Author: Laurenz Albe

Backpatch-through: 10

3 years agoFix TestLib::slurp_file() with offset on windows.
Andres Freund [Mon, 4 Oct 2021 20:28:06 +0000 (13:28 -0700)]
Fix TestLib::slurp_file() with offset on windows.

3c5b0685b921 used setFilePointer() to set the position of the filehandle, but
passed the wrong filehandle, always leaving the position at 0. Instead of just
fixing that, remove use of setFilePointer(), we have a perl fd at this point,
so we can just use perl's seek().

Additionally, the perl filehandle wasn't closed, just the windows filehandle.

Reviewed-By: Andrew Dunstan
Author: Andres Freund 
Discussion: https://postgr.es/m/20211003173038[email protected]
Backpatch: 9.6-, like 3c5b0685b921

3 years agoUpdate our mapping of Windows time zone names some more.
Tom Lane [Mon, 4 Oct 2021 18:52:17 +0000 (14:52 -0400)]
Update our mapping of Windows time zone names some more.

Per discussion, let's just follow CLDR's default zone mappings
faithfully.  There are two changes here that are clear improvements:

* Mapping "Greenwich Standard Time" to Atlantic/Reykjavik is actually
a better fit than using London, because Iceland hasn't observed DST
since 1968, so this is more nearly what people might expect.

* Since the "Samoa" zone is specified to be UTC+13:00, we must map
it to Pacific/Apia not Pacific/Samoa; the latter refers to American
Samoa which is now on the other side of the date line.

The rest of these changes look like they're choosing the most populous
IANA zone as representative.  Whatever the details, we're just going
to say "if you don't like this mapping, complain to CLDR".

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

3 years agoFix snapshot builds during promotion of hot standby node with 2PC
Michael Paquier [Mon, 4 Oct 2021 05:05:52 +0000 (14:05 +0900)]
Fix snapshot builds during promotion of hot standby node with 2PC

Some specific logic is done at the end of recovery when involving 2PC
transactions:
1) Call RecoverPreparedTransactions(), to recover the state of 2PC
transactions into memory (re-acquire locks, etc.).
2) ShutdownRecoveryTransactionEnvironment(), to move back to normal
operations, mainly cleaning up recovery locks and KnownAssignedXids
(including any 2PC transaction tracked previously).
3) Switch XLogCtl->SharedRecoveryState to RECOVERY_STATE_DONE, which is
the tipping point for any process calling RecoveryInProgress() to check
if the cluster is still in recovery or not.

Any snapshot taken between steps 2) and 3) would be empty, causing any
transaction relying on a snapshot at this point to potentially corrupt
data as there could still be some 2PC transactions to track, with
RecentXmin moving backwards on successive calls to GetSnapshotData() in
the same transaction.

As SharedRecoveryState is the point to take into account to know if it
is safe to discard KnownAssignedXids, this commit moves step 2) after
step 3), so as we can never finish with empty snapshots.

This exists since the introduction of hot standby, so backpatch all the
way down.  The window with incorrect snapshots is extremely small, but I
have seen it when running 023_pitr_prepared_xact.pl, as did buildfarm
member fairywren.  Thomas Munro also found it independently.  Special
thanks to Andres Freund for taking the time to analyze this issue.

Reported-by: Thomas Munro, Michael Paquier
Analyzed-by: Andres Freund
Discussion: https://postgr.es/m/20210422203603[email protected]
Backpatch-through: 9.6

3 years agoUpdate our mapping of Windows time zone names using CLDR info.
Tom Lane [Sat, 2 Oct 2021 20:05:42 +0000 (16:05 -0400)]
Update our mapping of Windows time zone names using CLDR info.

This corrects a bunch of entries in win32_tzmap[], and adds a few
new ones, based on the CLDR project's windowsZones.xml file.
Non-cosmetic changes fall into four main categories:

* Flat-out errors:

US/Aleutan doesn't exist
America/Salvador doesn't exist
Asia/Baku is wrong for Yerevan
Asia/Dhaka (Bangladesh) is wrong for Astana (Kazakhstan)
Europe/Bucharest is wrong for Chisinau
America/Mexico_City is wrong for Chetumal
America/Buenos_Aires is wrong for Cayenne
America/Caracas has its own zone, so poor fit for La Paz
US/Eastern is wrong for Haiti
US/Eastern is wrong for Indiana (East)
Asia/Karachi is wrong for Tashkent
Etc/UTC+12 doesn't exist
Signs of Etc/GMT zones were backwards

* Judgment calls:

(These changes follow CLDR's choices, except for the first one)

Use Europe/London for "Greenwich Standard Time", since that seems much
more likely than Africa/Casablanca to be what people will think that
zone name means.  CLDR has Atlantic/Reykjavik here, but that's no better.

Asia/Shanghai seems a better fit than Hong Kong for "China Standard
Time".

Europe/Sarajevo is now a link to Belgrade, ie "Central Europe Standard
Time"; so use Warsaw for "Central European Standard Time".

America/Sao_Paulo seems more representative than Araguaina for
"E. South America Standard Time".

Africa/Johannesburg seems more representative than Harare for
"South Africa Standard Time".

* New Windows zone names:

"Israel Standard Time"
"Kaliningrad Standard Time"
"Russia Time Zone N" for various N
"Singapore Standard Time"
"South Sudan Standard Time"
"W. Central Africa Standard Time"
"West Bank Standard Time"
"Yukon Standard Time"

Some of these replace older spellings, but I kept the older spellings
too in case our code runs on a machine with the older data.

* Replace aliases (tzdb Links) with underlying city-named zones:

(This tracks tzdb's longstanding practice, and reduces inconsistency
with the rest of the entries, as well as with CLDR.)

US/Alaska
Asia/Kuwait
Asia/Muscat
Canada/Atlantic
Australia/Canberra
Canada/Saskatchewan
US/Central
US/Eastern
US/Hawaii
US/Mountain
Canada/Newfoundland
US/Pacific

Back-patch to all supported branches, as is our usual practice for
time zone data updates.

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

3 years agoRe-alphabetize the win32_tzmap[] array.
Tom Lane [Sat, 2 Oct 2021 20:05:10 +0000 (16:05 -0400)]
Re-alphabetize the win32_tzmap[] array.

The original intent seems to have been to sort case-insensitively
by the Windows zone name, but various changes over the years did
not get that memo.  This commit just moves a few entries to
restore exact alphabetic order, to ease comparison to the outputs
of processing scripts.

Back-patch to all supported branches, as is our usual practice for
time zone data updates.

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

3 years agoError out if SKIP LOCKED and WITH TIES are both specified
Alvaro Herrera [Fri, 1 Oct 2021 21:29:18 +0000 (18:29 -0300)]
Error out if SKIP LOCKED and WITH TIES are both specified

Both bugs #16676[1] and #17141[2] illustrate that the combination of
SKIP LOCKED and FETCH FIRST WITH TIES break expectations when it comes
to rows returned to other sessions accessing the same row.  Since this
situation is detectable from the syntax and hard to fix otherwise,
forbid for now, with the potential to fix in the future.

[1] https://postgr.es/m/16676-fd62c3c835880da6@postgresql.org
[2] https://postgr.es/m/17141-913d78b9675aac8e@postgresql.org

Backpatch-through: 13, where WITH TIES was introduced
Author: David Christensen 
Discussion: https://postgr.es/m/CAOxo6XLPccCKru3xPMaYDpa+AXyPeWFs+SskrrL+HKwDjJnLhg@mail.gmail.com

3 years agoAvoid believing incomplete MCV-only stats in get_variable_range().
Tom Lane [Fri, 1 Oct 2021 18:59:35 +0000 (14:59 -0400)]
Avoid believing incomplete MCV-only stats in get_variable_range().

get_variable_range() would incautiously believe that statistics
containing only an MCV list are sufficient to derive a range estimate.
That's okay for an enum-like column that contains only MCVs, but
otherwise the estimate could be pretty bad.  Make it report that the
range is indeterminate unless the MCVs plus nullfrac account for
the whole table.

I don't think this needs a dedicated test case, since a quick code
coverage check verifies that the existing regression tests traverse
all the alternatives.  There is room to doubt that a future-proof
test case could be built anyway, given that the submitted example
accidentally doesn't fail before v11.

Per bug #17207 from Simon Perepelitsa.  Back-patch to v10.
In principle this has been broken all along, but I'm hesitant to
make such changes in 9.6, since if anyone is unhappy with 9.6.24's
behavior there will be no second chance to fix it.

Discussion: https://postgr.es/m/17207-5265aefa79e333b4@postgresql.org

3 years agoFix Portal snapshot tracking to handle subtransactions properly.
Tom Lane [Fri, 1 Oct 2021 15:10:12 +0000 (11:10 -0400)]
Fix Portal snapshot tracking to handle subtransactions properly.

Commit 84f5c2908 forgot to consider the possibility that
EnsurePortalSnapshotExists could run inside a subtransaction with
lifespan shorter than the Portal's.  In that case, the new active
snapshot would be popped at the end of the subtransaction, leaving
a dangling pointer in the Portal, with mayhem ensuing.

To fix, make sure the ActiveSnapshot stack entry is marked with
the same subtransaction nesting level as the associated Portal.
It's certainly safe to do so since we won't be here at all unless
the stack is empty; hence we can't create an out-of-order stack.

Let's also apply this logic in the case where PortalRunUtility
sets portalSnapshot, just to be sure that path can't cause similar
problems.  It's slightly less clear that that path can't create
an out-of-order stack, so add an assertion guarding it.

Report and patch by Bertrand Drouvot (with kibitzing by me).
Back-patch to v11, like the previous commit.

Discussion: https://postgr.es/m/ff82b8c5-77f4-3fe7-6028-fcf3303e82dd@amazon.com

3 years agoRemove gratuitous environment dependency in 002_types.pl test.
Tom Lane [Thu, 30 Sep 2021 20:23:10 +0000 (16:23 -0400)]
Remove gratuitous environment dependency in 002_types.pl test.

Computing related timestamps by subtracting "N days" is sensitive
to the prevailing timezone, since we interpret that as "same local
time on the N'th prior day".  Even though the intervals in question
are only two to four days, through remarkable bad luck they managed
to cross the end of Ramadan in 2014, causing the test's output to
change if timezone is set to Africa/Casablanca.  (Maybe in other
Muslim areas as well; I didn't check.)  There's absolutely no reason
for this test to exercise interval subtraction, so just get rid of
that and use plain timestamptz constants representing the intended
values.

Per report from Andres Freund.  Back-patch to v10 where this test
script came in.

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

3 years agoFix WAL replay in presence of an incomplete record
Alvaro Herrera [Wed, 29 Sep 2021 14:21:51 +0000 (11:21 -0300)]
Fix WAL replay in presence of an incomplete record

Physical replication always ships WAL segment files to replicas once
they are complete.  This is a problem if one WAL record is split across
a segment boundary and the primary server crashes before writing down
the segment with the next portion of the WAL record: WAL writing after
crash recovery would happily resume at the point where the broken record
started, overwriting that record ... but any standby or backup may have
already received a copy of that segment, and they are not rewinding.
This causes standbys to stop following the primary after the latter
crashes:
  LOG:  invalid contrecord length 7262 at A8/D9FFFBC8
because the standby is still trying to read the continuation record
(contrecord) for the original long WAL record, but it is not there and
it will never be.  A workaround is to stop the replica, delete the WAL
file, and restart it -- at which point a fresh copy is brought over from
the primary.  But that's pretty labor intensive, and I bet many users
would just give up and re-clone the standby instead.

A fix for this problem was already attempted in commit 515e3d84a0b5, but
it only addressed the case for the scenario of WAL archiving, so
streaming replication would still be a problem (as well as other things
such as taking a filesystem-level backup while the server is down after
having crashed), and it had performance scalability problems too; so it
had to be reverted.

This commit fixes the problem using an approach suggested by Andres
Freund, whereby the initial portion(s) of the split-up WAL record are
kept, and a special type of WAL record is written where the contrecord
was lost, so that WAL replay in the replica knows to skip the broken
parts.  With this approach, we can continue to stream/archive segment
files as soon as they are complete, and replay of the broken records
will proceed across the crash point without a hitch.

Because a new type of WAL record is added, users should be careful to
upgrade standbys first, primaries later. Otherwise they risk the standby
being unable to start if the primary happens to write such a record.

A new TAP test that exercises this is added, but the portability of it
is yet to be seen.

This has been wrong since the introduction of physical replication, so
backpatch all the way back.  In stable branches, keep the new
XLogReaderState members at the end of the struct, to avoid an ABI
break.

Author: Álvaro Herrera 
Reviewed-by: Kyotaro Horiguchi
Reviewed-by: Nathan Bossart
Discussion: https://postgr.es/m/202108232252[email protected]

3 years agopgbench: Fix handling of socket errors during benchmark.
Fujii Masao [Wed, 29 Sep 2021 12:01:10 +0000 (21:01 +0900)]
pgbench: Fix handling of socket errors during benchmark.

Previously socket errors such as invalid socket or socket wait method failures
during benchmark caused pgbench to exit with status 0. Instead, errors during
the run should result in exit status 2.

Back-patch to v12 where pgbench started reporting exit status.

Original complaint and patch by Hayato Kuroda.

Author: Yugo Nagata, Fabien COELHO
Reviewed-by: Kyotaro Horiguchi, Fujii Masao
Discussion: https://postgr.es/m/TYCPR01MB5870057375ACA8A73099C649F5349@TYCPR01MB5870.jpnprd01.prod.outlook.com

3 years agopgbench: Correct log level of message output when socket wait method fails.
Fujii Masao [Wed, 29 Sep 2021 11:35:00 +0000 (20:35 +0900)]
pgbench: Correct log level of message output when socket wait method fails.

The failure of socket wait method like "select()" doesn't terminate pgbench.
So the log level of error message when that failure happens should be ERROR.
But previously FATAL was used in that case.

Back-patch to v13 where pgbench started using common logging API.

Author: Yugo Nagata, Fabien COELHO
Reviewed-by: Kyotaro Horiguchi, Fujii Masao
Discussion: https://postgr.es/m/20210617005934.8bd37bf72efd5f1b38e6f482@sraoss.co.jp

3 years agoFix instability in contrib/bloom TAP tests.
Tom Lane [Tue, 28 Sep 2021 21:34:31 +0000 (17:34 -0400)]
Fix instability in contrib/bloom TAP tests.

It turns out that the instability complained of in commit d3c09b9b1
has an embarrassingly simple explanation.  The test script waits for
the standby to flush incoming WAL to disk, but it should wait for
the WAL to be replayed, since we are testing for the effects of that
to be visible.

While at it, use wait_for_catchup instead of reinventing that logic,
and adjust $Test::Builder::Level to improve future error reports.

Back-patch to v12 where the necessary infrastructure came in
(cf. aforesaid commit).  Also back-patch 7d1aa6bf1 so that the
test will actually get run.

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

3 years agoFix typos in docs
Michael Paquier [Sun, 26 Sep 2021 10:18:27 +0000 (19:18 +0900)]
Fix typos in docs

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

3 years agoDoc: extend warnings about collation-mismatch hazards in postgres_fdw.
Tom Lane [Sat, 25 Sep 2021 14:53:54 +0000 (10:53 -0400)]
Doc: extend warnings about collation-mismatch hazards in postgres_fdw.

Be a little more vocal about the risks of remote collations not
matching local ones.  Actually fixing these risks seems hard,
and I've given up on the idea that it might be back-patchable.
So the best we can do for the back branches is add documentation.

Per discussion of bug #16583 from Jiří Fejfar.

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

3 years agoAdd alternative output for OpenSSL 3 without legacy loaded
Daniel Gustafsson [Sat, 25 Sep 2021 09:27:28 +0000 (11:27 +0200)]
Add alternative output for OpenSSL 3 without legacy loaded

OpenSSL 3 introduced the concept of providers to support modularization,
and moved the outdated ciphers to the new legacy provider. In case it's
not loaded in the users openssl.cnf file there will be a lot of regress
test failures, so add alternative outputs covering those.

Also document the need to load the legacy provider in order to use older
ciphers with OpenSSL-enabled pgcrypto.

This will be backpatched to all supported version once there is sufficient
testing in the buildfarm of OpenSSL 3.

Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/FEF81714-D479-4512-839B-C769D2605F8A@yesql.se
Backpatch-through: 9.6

3 years agoDisable OpenSSL EVP digest padding in pgcrypto
Daniel Gustafsson [Sat, 25 Sep 2021 09:27:20 +0000 (11:27 +0200)]
Disable OpenSSL EVP digest padding in pgcrypto

The PX layer in pgcrypto is handling digest padding on its own uniformly
for all backend implementations. Starting with OpenSSL 3.0.0, DecryptUpdate
doesn't flush the last block in case padding is enabled so explicitly
disable it as we don't use it.

This will be backpatched to all supported version once there is sufficient
testing in the buildfarm of OpenSSL 3.

Reviewed-by: Peter Eisentraut, Michael Paquier
Discussion: https://postgr.es/m/FEF81714-D479-4512-839B-C769D2605F8A@yesql.se
Backpatch-through: 9.6

3 years agopgcrypto: Check for error return of px_cipher_decrypt()
Daniel Gustafsson [Sat, 25 Sep 2021 09:25:48 +0000 (11:25 +0200)]
pgcrypto: Check for error return of px_cipher_decrypt()

This has previously not been a problem (that anyone ever reported),
but in future OpenSSL versions (3.0.0), where legacy ciphers are/can
be disabled, this is the place where this is reported.  So we need to
catch the error here, otherwise the higher-level functions would
return garbage.  The nearby encryption code already handled errors
similarly.

Author: Peter Eisentraut 
Reviewed-by: Daniel Gustafsson
Discussion: https://www.postgresql.org/message-id/9e9c431c-0adc-7a6d-9b1a-915de1ba3fe7@enterprisedb.com
Backpatch-through: 9.6

3 years agodoc: Improve description of index vacuuming with GUCs
Michael Paquier [Sat, 25 Sep 2021 06:12:00 +0000 (15:12 +0900)]
doc: Improve description of index vacuuming with GUCs

Index vacuums may happen multiple times depending on the number of dead
tuples stored, as of maintenance_work_mem for a manual VACUUM.  For
autovacuum, this is controlled by autovacuum_work_mem instead, if set.
The documentation mentioned the former, but not the latter in the
context of autovacuum.

Reported-by: Nikolai Berkoff
Author: Laurenz Albe, Euler Taveira
Discussion: https://postgr.es/m/161545365522.10134.12195402324485546870@wrigleys.postgresql.org
Backpatch-through: 9.6

3 years agodoc: Add missing markup in CREATE EVENT TRIGGER page
Michael Paquier [Sat, 25 Sep 2021 05:48:13 +0000 (14:48 +0900)]
doc: Add missing markup in CREATE EVENT TRIGGER page

Reported-by: rir
Discussion: https://postgr.es/m/20210924183658.3syyitp3yuxjv2fp@localhost
Backpatch-through: 9.6

3 years agoSplit macros from visibilitymap.h into a separate header
Alexander Korotkov [Thu, 23 Sep 2021 16:59:03 +0000 (19:59 +0300)]
Split macros from visibilitymap.h into a separate header

That allows to include just visibilitymapdefs.h from file.c, and in turn,
remove include of postgres.h from relcache.h.

Reported-by: Andres Freund
Discussion: https://postgr.es/m/20210913232614.czafiubr435l6egi%40alap3.anarazel.de
Author: Alexander Korotkov
Reviewed-by: Andres Freund, Tom Lane, Alvaro Herrera
Backpatch-through: 13

3 years agoRelease memory allocated by dependency_degree
Tomas Vondra [Mon, 20 Sep 2021 23:13:11 +0000 (01:13 +0200)]
Release memory allocated by dependency_degree

Calculating degree of a functional dependency may allocate a lot of
memory - we have released mot of the explicitly allocated memory, but
e.g. detoasted varlena values were left behind. That may be an issue,
because we consider a lot of dependencies (all combinations), and the
detoasting may happen for each one again.

Fixed by calling dependency_degree() in a dedicated context, and
resetting it after each call. We only need the calculated dependency
degree, so we don't need to copy anything.

Backpatch to PostgreSQL 10, where extended statistics were introduced.

Backpatch-through: 10
Discussion: https://www.postgresql.org/message-id/20210915200928.GP831%40telsasoft.com

3 years agoFree memory after building each statistics object
Tomas Vondra [Mon, 20 Sep 2021 23:14:11 +0000 (01:14 +0200)]
Free memory after building each statistics object

Until now, all extended statistics on a given relation were built in the
same memory context, without resetting. Some of the memory was released
explicitly, but not all of it - for example memory allocated while
detoasting values is hard to free. This is how it worked since extended
statistics were introduced in PostgreSQL 10, but adding support for
extended stats on expressions made the issue somewhat worse as it
increases the number of statistics to build.

Fixed by adding a memory context which gets reset after building each
statistics object (all the statistics kinds included in it). Resetting
it after building each statistics kind would be even better, but it
would require more invasive changes and copying of results, making it
harder to backpatch.

Backpatch to PostgreSQL 10, where extended statistics were introduced.

Author: Justin Pryzby
Reported-by: Justin Pryzby
Reviewed-by: Tomas Vondra
Backpatch-through: 10
Discussion: https://www.postgresql.org/message-id/20210915200928.GP831%40telsasoft.com

3 years agoInvalidate all partitions for a partitioned table in publication.
Amit Kapila [Wed, 22 Sep 2021 02:54:20 +0000 (08:24 +0530)]
Invalidate all partitions for a partitioned table in publication.

Updates/Deletes on a partition were allowed even without replica identity
after the parent table was added to a publication. This would later lead
to an error on subscribers. The reason was that we were not invalidating
the partition's relcache and the publication information for partitions
was not getting rebuilt. Similarly, we were not invalidating the
partitions' relcache after dropping a partitioned table from a publication
which will prohibit Updates/Deletes on its partition without replica
identity even without any publication.

Reported-by: Haiying Tang
Author: Hou Zhijie and Vignesh C
Reviewed-by: Vignesh C and Amit Kapila
Backpatch-through: 13
Discussion: https://postgr.es/m/OS0PR01MB6113D77F583C922F1CEAA1C3FBD29@OS0PR01MB6113.jpnprd01.prod.outlook.com

3 years agoFix places in TestLib.pm in need of adaptation to the output of Msys perl
Michael Paquier [Tue, 21 Sep 2021 23:43:10 +0000 (08:43 +0900)]
Fix places in TestLib.pm in need of adaptation to the output of Msys perl

Contrary to the output of native perl, Msys perl generates outputs with
CRLFs characters.  There are already places in the TAP code where CRLFs
(\r\n) are automatically converted to LF (\n) on Msys, but we missed a
couple of places when running commands and using their output for
comparison, that would lead to failures.

This problem has been found thanks to the test added in 5adb067 using
TestLib::command_checks_all(), but after a closer look more code paths
were missing a filter.

This is backpatched all the way down to prevent any surprises if a new
test is introduced in stable branches.

Reviewed-by: Andrew Dunstan, Álvaro Herrera
Discussion: https://postgr.es/m/1252480.1631829409@sss.pgh.pa.us
Backpatch-through: 9.6

3 years agoFix misevaluation of STABLE parameters in CALL within plpgsql.
Tom Lane [Tue, 21 Sep 2021 23:06:33 +0000 (19:06 -0400)]
Fix misevaluation of STABLE parameters in CALL within plpgsql.

Before commit 84f5c2908, a STABLE function in a plpgsql CALL
statement's argument list would see an up-to-date snapshot,
because exec_stmt_call would push a new snapshot.  I got rid of
that because the possibility of the snapshot disappearing within
COMMIT made it too hard to manage a snapshot across the CALL
statement.  That's fine so far as the procedure itself goes,
but I forgot to think about the possibility of STABLE functions
within the CALL argument list.  As things now stand, those'll
be executed with the Portal's snapshot as ActiveSnapshot,
keeping them from seeing updates more recent than Portal startup.

(VOLATILE functions don't have a problem because they take their
own snapshots; which indeed is also why the procedure itself
doesn't have a problem.  There are no STABLE procedures.)

We can fix this by pushing a new snapshot transiently within
ExecuteCallStmt itself.  Popping the snapshot before we get
into the procedure proper eliminates the management problem.
The possibly-useless extra snapshot-grab is slightly annoying,
but it's no worse than what happened before 84f5c2908.

Per bug #17199 from Alexander Nawratil.  Back-patch to v11,
like the previous patch.

Discussion: https://postgr.es/m/17199-1ab2561f0d94af92@postgresql.org

3 years agoRemove overzealous index deletion assertion.
Peter Geoghegan [Mon, 20 Sep 2021 21:26:22 +0000 (14:26 -0700)]
Remove overzealous index deletion assertion.

A broken HOT chain is not an unexpected condition, even when the offset
number points past the end of the page's line pointer array.
heap_prune_chain() does not (and never has) treated this condition as
unexpected, so derivative code in heap_index_delete_tuples() shouldn't
do so either.

Oversight in commit 4228817449.

The assertion can probably only fail on Postgres 14 and master.  Earlier
releases don't have commit 3c3b8a4b, which taught VACUUM to truncate the
line pointer array of heap pages.  Backpatch all the same, just to be
consistent.

Author: Peter Geoghegan 
Reported-By: Alexander Lakhin
Discussion: https://postgr.es/m/17197-9438f31f46705182@postgresql.org
Backpatch: 12-, just like commit 4228817449.

3 years agoDon't elide casting to typmod -1.
Tom Lane [Mon, 20 Sep 2021 15:48:52 +0000 (11:48 -0400)]
Don't elide casting to typmod -1.

Casting a value that's already of a type with a specific typmod
to an unspecified typmod doesn't do anything so far as run-time
behavior is concerned.  However, it really ought to change the
exposed type of the expression to match.  Up to now,
coerce_type_typmod hasn't bothered with that, which creates gotchas
in contexts such as recursive unions.  If for example one side of
the union is numeric(18,3), but it needs to be plain numeric to
match the other side, there's no direct way to express that.

This is easy enough to fix, by inserting a RelabelType to update the
exposed type of the expression.  However, it's a bit nervous-making
to change this behavior, because it's stood for a really long time.
But no complaints have emerged about 14beta3, so go ahead and
back-patch.

Back-patch of 5c056b0c2 into previous supported branches.

Discussion: https://postgr.es/m/CABNQVagu3bZGqiTjb31a8D5Od3fUMs7Oh3gmZMQZVHZ=uWWWfQ@mail.gmail.com
Discussion: https://postgr.es/m/1488389.1631984807@sss.pgh.pa.us

3 years agoDoc: fix typos.
Tom Lane [Sun, 19 Sep 2021 15:36:53 +0000 (11:36 -0400)]
Doc: fix typos.

"PGcon" should be "PGconn".  Noted by D. Frey.

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

3 years agoFix pull_varnos to cope with translated PlaceHolderVars.
Tom Lane [Fri, 17 Sep 2021 19:41:16 +0000 (15:41 -0400)]
Fix pull_varnos to cope with translated PlaceHolderVars.

Commit 55dc86eca changed pull_varnos to use (if possible) the associated
ph_eval_at for a PlaceHolderVar.  I missed a fine point though: we might
be looking at a PHV in the quals or tlist of a child appendrel, in which
case we need to compute a ph_eval_at value that's been translated in the
same way that the PHV itself has been (cf. adjust_appendrel_attrs).
Fortunately, enough info is available in the PlaceHolderInfo to make
such translation possible without additional outside data, so we don't
need another round of uglification of planner APIs.  This is a little
bit complicated, but since it's a hard-to-hit corner case, I'm not much
worried about adding cycles here.

Per report from Jaime Casanova.  Back-patch to v12, like the previous
commit.

Discussion: https://postgr.es/m/20210915230959.GB17635@ahch-to

3 years agoFix variable shadowing in procarray.c.
Fujii Masao [Thu, 16 Sep 2021 04:07:29 +0000 (13:07 +0900)]
Fix variable shadowing in procarray.c.

ProcArrayGroupClearXid function has a parameter named "proc",
but the same name was used for its local variables. This commit fixes
this variable shadowing, to improve code readability.

Back-patch to all supported versions, to make future back-patching
easy though this patch is classified as refactoring only.

Reported-by: Ranier Vilela
Author: Ranier Vilela, Aleksander Alekseev
https://postgr.es/m/CAEudQAqyoTZC670xWi6w-Oe2_Bk1bfu2JzXz6xRfiOUzm7xbyQ@mail.gmail.com

3 years agoDisallow LISTEN in background workers.
Tom Lane [Wed, 15 Sep 2021 16:31:56 +0000 (12:31 -0400)]
Disallow LISTEN in background workers.

It's possible to execute user-defined SQL in some background processes;
for example, logical replication workers can fire triggers.  This opens
the possibility that someone would try to execute LISTEN in such a
context.  But since only regular backends ever call
ProcessNotifyInterrupt, no messages would actually be received, and
thus the registered listener would simply prevent the message queue
from being cleaned.  Eventually NOTIFY would stop working, which is bad.

Perhaps someday somebody will invent infrastructure to make listening
in a background worker actually useful.  In the meantime, forbid it.

Back-patch to v13, which is where we introduced the MyBackendType
variable.  It'd be a lot harder to implement the check without that,
and it doesn't seem worth the trouble.

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

3 years agoSend NOTIFY signals during CommitTransaction.
Tom Lane [Tue, 14 Sep 2021 21:18:25 +0000 (17:18 -0400)]
Send NOTIFY signals during CommitTransaction.

Formerly, we sent signals for outgoing NOTIFY messages within
ProcessCompletedNotifies, which was also responsible for sending
relevant ones of those messages to our connected client.  It therefore
had to run during the main-loop processing that occurs just before
going idle.  This arrangement had two big disadvantages:

* Now that procedures allow intra-command COMMITs, it would be
useful to send NOTIFYs to other sessions immediately at COMMIT
(though, for reasons of wire-protocol stability, we still shouldn't
forward them to our client until end of command).

* Background processes such as replication workers would not send
NOTIFYs at all, since they never execute the client communication
loop.  We've had requests to allow triggers running in replication
workers to send NOTIFYs, so that's a problem.

To fix these things, move transmission of outgoing NOTIFY signals
into AtCommit_Notify, where it will happen during CommitTransaction.
Also move the possible call of asyncQueueAdvanceTail there, to
ensure we don't bloat the async SLRU if a background worker sends
many NOTIFYs with no one listening.

We can also drop the call of asyncQueueReadAllNotifications,
allowing ProcessCompletedNotifies to go away entirely.  That's
because commit 790026972 added a call of ProcessNotifyInterrupt
adjacent to PostgresMain's call of ProcessCompletedNotifies,
and that does its own call of asyncQueueReadAllNotifications,
meaning that we were uselessly doing two such calls (inside two
separate transactions) whenever inbound notify signals coincided
with an outbound notify.  We need only set notifyInterruptPending
to ensure that ProcessNotifyInterrupt runs, and we're done.

The existing documentation suggests that custom background workers
should call ProcessCompletedNotifies if they want to send NOTIFY
messages.  To avoid an ABI break in the back branches, reduce it
to an empty routine rather than removing it entirely.  Removal
will occur in v15.

Although the problems mentioned above have existed for awhile,
I don't feel comfortable back-patching this any further than v13.
There was quite a bit of churn in adjacent code between 12 and 13.
At minimum we'd have to also backpatch 51004c717, and a good deal
of other adjustment would also be needed, so the benefit-to-risk
ratio doesn't look attractive.

Per bug #15293 from Michael Powers (and similar gripes from others).

Artur Zakirov and Tom Lane

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

3 years agojit: Do not try to shut down LLVM state in case of LLVM triggered errors.
Andres Freund [Tue, 14 Sep 2021 01:07:19 +0000 (18:07 -0700)]
jit: Do not try to shut down LLVM state in case of LLVM triggered errors.

If an allocation failed within LLVM it is not safe to call back into LLVM as
LLVM is not generally safe against exceptions / stack-unwinding. Thus errors
while in LLVM code are promoted to FATAL. However llvm_shutdown() did call
back into LLVM even in such cases, while llvm_release_context() was careful
not to do so.

We cannot generally skip shutting down LLVM, as that can break profiling. But
it's OK to do so if there was an error from within LLVM.

Reported-By: Jelte Fennema
Author: Andres Freund 
Author: Justin Pryzby 
Discussion: https://postgr.es/m/AM5PR83MB0178C52CCA0A8DEA0207DC14F7FF9@AM5PR83MB0178.EURPRD83.prod.outlook.com
Backpatch: 11-, where jit was introduced

3 years agoFix EXIT out of outermost block in plpgsql.
Tom Lane [Mon, 13 Sep 2021 16:42:03 +0000 (12:42 -0400)]
Fix EXIT out of outermost block in plpgsql.

Ordinarily, using EXIT this way would draw "control reached end of
function without RETURN".  However, if the function is one where we
don't require an explicit RETURN (such as a DO block), that should
not happen.  It did anyway, because add_dummy_return() neglected to
account for the case.

Per report from Herwig Goemans.  Back-patch to all supported branches.

Discussion: https://postgr.es/m/868ae948-e3ca-c7ec-95a6-83cfc08ef750@gmail.com

3 years agoDoc: Remove type information for import_generated in postgres-fdw.sgml.
Etsuro Fujita [Mon, 13 Sep 2021 08:30:02 +0000 (17:30 +0900)]
Doc: Remove type information for import_generated in postgres-fdw.sgml.

The type information for FDW options is only added to HEAD; remove this
from back branches.  Oversight in commit aa769f80e.

Apply the patch to v12, v13, and v14.

Discussion: https://postgr.es/m/CAPmGK14z92twaKwRoccHbbh5Va5vbRDZcTYYTx50+0JTQ8xx_g@mail.gmail.com

3 years agoFix reorder buffer memory accounting for toast changes.
Amit Kapila [Mon, 13 Sep 2021 05:16:58 +0000 (10:46 +0530)]
Fix reorder buffer memory accounting for toast changes.

While processing toast changes in logical decoding, we rejigger the
tuple change to point to in-memory toast tuples instead to on-disk toast
tuples. And, to make sure the memory accounting is correct, we were
subtracting the old change size and then after re-computing the new tuple,
re-adding its size at the end. Now, if there is any error before we add
the new size, we will release the changes and that will update the
accounting info (subtracting the size from the counters). And we were
underflowing there which leads to an assertion failure in assert enabled
builds and wrong memory accounting in reorder buffer otherwise.

Author: Bertrand Drouvot
Reviewed-by: Amit Kapila
Backpatch-through: 13, where memory accounting was introduced
Discussion: https://postgr.es/m/92b0ee65-b8bd-e42d-c082-4f3f4bf12d34@amazon.com

3 years agoFix error handling with threads on OOM in ECPG connection logic
Michael Paquier [Mon, 13 Sep 2021 04:24:20 +0000 (13:24 +0900)]
Fix error handling with threads on OOM in ECPG connection logic

An out-of-memory failure happening when allocating the structures to
store the connection parameter keywords and values would mess up with
the set of connections saved, as on failure the pthread mutex would
still be hold with the new connection object listed but free()'d.

Rather than just unlocking the mutex, which would leave the static list
of connections into an inconsistent state, move the allocation for the
structures of the connection parameters before beginning the test
manipulation.  This ensures that the list of connections and the
connection mutex remain consistent all the time in this code path.

This error is unlikely going to happen, but this could mess up badly
with ECPG clients in surprising ways, so backpatch all the way down.

Reported-by: ryancaicse
Discussion: https://postgr.es/m/17186-b4cfd8f0eb4d1dee@postgresql.org
Backpatch-through: 9.6

3 years agoMake pg_regexec() robust against out-of-range search_start.
Tom Lane [Sat, 11 Sep 2021 19:19:31 +0000 (15:19 -0400)]
Make pg_regexec() robust against out-of-range search_start.

If search_start is greater than the length of the string, we should just
return REG_NOMATCH immediately.  (Note that the equality case should
*not* be rejected, since the pattern might be able to match zero
characters.)  This guards various internal assumptions that the min of a
range of string positions is not more than the max.  Violation of those
assumptions could allow an attempt to fetch string[search_start-1],
possibly causing a crash.

Jaime Casanova pointed out that this situation is reachable with the
new regexp_xxx functions that accept a user-specified start position.
I don't believe it's reachable via any in-core call site in v14 and
below.  However, extensions could possibly call pg_regexec with an
out-of-range search_start, so let's back-patch the fix anyway.

Discussion: https://postgr.es/m/20210911180357.GA6870@ahch-to

3 years agoFix some anomalies with NO SCROLL cursors.
Tom Lane [Fri, 10 Sep 2021 17:18:32 +0000 (13:18 -0400)]
Fix some anomalies with NO SCROLL cursors.

We have long forbidden fetching backwards from a NO SCROLL cursor,
but the prohibition didn't extend to cases in which we rewind the
query altogether and then re-fetch forwards.  I think the reason is
that this logic was mainly meant to protect plan nodes that can't
be run in the reverse direction.  However, re-reading the query output
is problematic if the query is volatile (which includes SELECT FOR
UPDATE, not just queries with volatile functions): the re-read can
produce different results, which confuses the cursor navigation logic
completely.  Another reason for disliking this approach is that some
code paths will either fetch backwards or rewind-and-fetch-forwards
depending on the distance to the target row; so that seemingly
identical use-cases may or may not draw the "cursor can only scan
forward" error.  Hence, let's clean things up by disallowing rewind
as well as fetch-backwards in a NO SCROLL cursor.

Ordinarily we'd only make such a definitional change in HEAD, but
there is a third reason to consider this change now.  Commit ba2c6d6ce
created some new user-visible anomalies for non-scrollable cursors
WITH HOLD, in that navigation in the cursor result got confused if the
cursor had been partially read before committing.  The only good way
to resolve those anomalies is to forbid rewinding such a cursor, which
allows removal of the incorrect cursor state manipulations that
ba2c6d6ce added to PersistHoldablePortal.

To minimize the behavioral change in the back branches (including
v14), refuse to rewind a NO SCROLL cursor only when it has a holdStore,
ie has been held over from a previous transaction due to WITH HOLD.
This should avoid breaking most applications that have been sloppy
about whether to declare cursors as scrollable.  We'll enforce the
prohibition across-the-board beginning in v15.

Back-patch to v11, as ba2c6d6ce was.

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

3 years agoAvoid fetching from an already-terminated plan.
Tom Lane [Thu, 9 Sep 2021 17:36:31 +0000 (13:36 -0400)]
Avoid fetching from an already-terminated plan.

Some plan node types don't react well to being called again after
they've already returned NULL.  PortalRunSelect() has long dealt
with this by calling the executor with NoMovementScanDirection
if it sees that we've already run the portal to the end.  However,
commit ba2c6d6ce overlooked this point, so that persisting an
already-fully-fetched cursor would fail if it had such a plan.

Per report from Tomas Barton.  Back-patch to v11, as the faulty
commit was.  (I've omitted a test case because the type of plan
that causes a problem isn't all that stable.)

Discussion: https://postgr.es/m/CAPV2KRjd=ErgVGbvO2Ty20tKTEZZr6cYsYLxgN_W3eAo9pf5sw@mail.gmail.com

3 years agoCheck for relation length overrun soon enough.
Tom Lane [Thu, 9 Sep 2021 15:45:48 +0000 (11:45 -0400)]
Check for relation length overrun soon enough.

We don't allow relations to exceed 2^32-1 blocks, because block
numbers are 32 bits and the last possible block number is reserved
to mean InvalidBlockNumber.  There is a check for this in mdextend,
but that's really way too late, because the smgr API requires us to
create a buffer for the block-to-be-added, and we do not want to
have any buffer with blocknum InvalidBlockNumber.  (Such a case
can trigger assertions in bufmgr.c, plus I think it might confuse
ReadBuffer's logic for data-past-EOF later on.)  So put the check
into ReadBuffer.

Per report from Christoph Berg.  It's been like this forever,
so back-patch to all supported branches.

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

3 years agoFix issue with WAL archiving in standby.
Fujii Masao [Thu, 9 Sep 2021 14:58:05 +0000 (23:58 +0900)]
Fix issue with WAL archiving in standby.

Previously, walreceiver always closed the currently-opened WAL segment
and created its archive notification file, after it finished writing
the current segment up and received any WAL data that should be
written into the next segment. If walreceiver exited just before
any WAL data in the next segment arrived at standby, it did not
create the archive notification file of the current segment
even though that's known completed. This behavior could cause
WAL archiving of the segment to be delayed until subsequent
restartpoints or checkpoints created its notification file.

To fix the issue, this commit changes walreceiver so that it creates
an archive notification file of a current WAL segment immediately
if that's known completed before receiving next WAL data.

Back-patch to all supported branches.

Reported-by: Kyotaro Horiguchi
Author: Fujii Masao
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/20200630.165503.1465894182551545886[email protected]

3 years agoAvoid useless malloc/free traffic around getFormattedTypeName().
Tom Lane [Wed, 8 Sep 2021 19:09:42 +0000 (15:09 -0400)]
Avoid useless malloc/free traffic around getFormattedTypeName().

Coverity complained that one caller of getFormattedTypeName() failed
to free the returned string.  Which is true, but rather than fixing
that one, let's get rid of this tedious and error-prone requirement.
Now that getFormattedTypeName() caches its result, strdup'ing that
result and expecting the caller to free it accomplishes little except
to waste cycles.  We do create a leak in the case where getTypes didn't
make a TypeInfo for the type, but that basically shouldn't ever happen.

Back-patch, as commit 6c450a861 was.  This isn't a particularly
interesting bug fix, but the API change seems like a hazard for
future back-patching activity if we don't back-patch it.

3 years agoFix rewriter to set hasModifyingCTE correctly on rewritten queries.
Tom Lane [Wed, 8 Sep 2021 16:05:43 +0000 (12:05 -0400)]
Fix rewriter to set hasModifyingCTE correctly on rewritten queries.

If we copy data-modifying CTEs from the original query to a replacement
query (from a DO INSTEAD rule), we must set hasModifyingCTE properly
in the replacement query.  Failure to do this can cause various
unpleasantness, such as unsafe usage of parallel plans.  The code also
neglected to propagate hasRecursive, though that's only cosmetic at
the moment.

A difficulty arises if the rule action is an INSERT...SELECT.  We
attach the original query's RTEs and CTEs to the sub-SELECT Query, but
data-modifying CTEs are only allowed to appear in the topmost Query.
For the moment, throw an error in such cases.  It would probably be
possible to avoid this error by attaching the CTEs to the top INSERT
Query instead; but that would require a bunch of new code to adjust
ctelevelsup references.  Given the narrowness of the use-case, and
the need to back-patch this fix, it does not seem worth the trouble
for now.  We can revisit this if we get field complaints.

Per report from Greg Nancarrow.  Back-patch to all supported branches.
(The test case added here does not fail before v10, but there are
plenty of places checking top-level hasModifyingCTE in 9.6, so I have
no doubt that this code change is necessary there too.)

Greg Nancarrow and Tom Lane

Discussion: https://postgr.es/m/CAJcOf-f68DT=26YAMz_i0+Au3TcLO5oiHY5=fL6Sfuits6r+_w@mail.gmail.com
Discussion: https://postgr.es/m/CAJcOf-fAdj=nDKMsRhQzndm-O13NY4dL6xGcEvdX5Xvbbi0V7g@mail.gmail.com

3 years agoInvalidate relcache for publications defined for all tables.
Amit Kapila [Wed, 8 Sep 2021 04:50:02 +0000 (10:20 +0530)]
Invalidate relcache for publications defined for all tables.

Updates/Deletes on a relation were allowed even without replica identity
after we define the publication for all tables. This would later lead to
an error on subscribers. The reason was that for such publications we were
not invalidating the relcache and the publication information for
relations was not getting rebuilt. Similarly, we were not invalidating the
relcache after dropping of such publications which will prohibit
Updates/Deletes without replica identity even without any publication.

Author: Vignesh C and Hou Zhijie
Reviewed-by: Hou Zhijie, Kyotaro Horiguchi, Amit Kapila
Backpatch-through: 10, where it was introduced
Discussion: https://postgr.es/m/CALDaNm0pF6zeWqCA8TCe2sDuwFAy8fCqba=nHampCKag-qLixg@mail.gmail.com

3 years agoAIX: Fix missing libpq symbols by respecting SHLIB_EXPORTS.
Noah Misch [Mon, 6 Sep 2021 18:27:59 +0000 (11:27 -0700)]
AIX: Fix missing libpq symbols by respecting SHLIB_EXPORTS.

We make each AIX shared library export all globals found in .o files
that originate in the library.  That doesn't include symbols acquired by
-lpgcommon_shlib.  That is good on average, but it became a problem for
libpq when commit e6afa8918c461c1dd80c5063a950518fa4e950cd moved five
official libpq API symbols into src/common.  Fix this by implementing
the SHLIB_EXPORTS mechanism for AIX, so affected libraries export the
same symbols that they export on Linux.  This reintroduces symbols
pg_encoding_to_char, pg_utf_mblen, pg_char_to_encoding,
pg_valid_server_encoding, and pg_valid_server_encoding_id.  Back-patch
to v13, where the aforementioned commit first appeared.  While a minor
release is usually the wrong time to add or remove symbol exports in
libpq or libecpg, we should expect users to want each documented symbol.

Tony Reix

Discussion: https://postgr.es/m/PR3PR02MB6396742E2FC3E77D37A920BC86C79@PR3PR02MB6396.eurprd02.prod.outlook.com

3 years agoFix bogus timetz_zone() results for DYNTZ abbreviations.
Tom Lane [Mon, 6 Sep 2021 15:29:52 +0000 (11:29 -0400)]
Fix bogus timetz_zone() results for DYNTZ abbreviations.

timetz_zone() delivered completely wrong answers if the zone was
specified by a dynamic TZ abbreviation, because it failed to account
for the difference between the POSIX conventions for field values in
struct pg_tm and the conventions used in PG-specific datetime code.

As a stopgap fix, just adjust the tm_year and tm_mon fields to match
PG conventions.  This is fixed in a different way in HEAD (388e71af8)
but I don't want to back-patch the change of reference point.

Discussion: https://postgr.es/m/CAJ7c6TOMG8zSNEZtCn5SPe+cCk3Lfxb71ZaQwT2F4T7PJ_t=KA@mail.gmail.com

3 years agoFix pkg-config files for static linking
Peter Eisentraut [Mon, 6 Sep 2021 07:41:03 +0000 (09:41 +0200)]
Fix pkg-config files for static linking

Since ea53100d5 (PostgreSQL 12), the shipped pkg-config files have
been broken for statically linking libpq because libpgcommon and
libpgport are missing.  This patch adds those two missing private
dependencies (in a non-hardcoded way).

Reported-by: Filip Gospodinov
Discussion: https://www.postgresql.org/message-id/flat/c7108bde-e051-11d5-a234-99beec01ce2a@gospodinov.ch

3 years agoFurther portability tweaks for float4/float8 hash functions.
Tom Lane [Sat, 4 Sep 2021 20:29:08 +0000 (16:29 -0400)]
Further portability tweaks for float4/float8 hash functions.

Attempting to make hashfloat4() look as much as possible like
hashfloat8(), I'd figured I could replace NaNs with get_float4_nan()
before widening to float8.  However, results from protosciurus
and topminnow show that on some platforms that produces a different
bit-pattern from get_float8_nan(), breaking the intent of ce773f230.
Rearrange so that we use the result of get_float8_nan() for all NaN
cases.  As before, back-patch.

3 years agoRevert "Avoid creating archive status ".ready" files too early"
Alvaro Herrera [Sat, 4 Sep 2021 16:14:30 +0000 (12:14 -0400)]
Revert "Avoid creating archive status ".ready" files too early"

This reverts commit 515e3d84a0b5 and equivalent commits in back
branches.  This solution to the problem has a number of problems, so
we'll try again with a different approach.

Per note from Andres Freund

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

3 years agoRemove arbitrary MAXPGPATH limit on command lengths in pg_ctl.
Tom Lane [Sat, 4 Sep 2021 01:04:44 +0000 (21:04 -0400)]
Remove arbitrary MAXPGPATH limit on command lengths in pg_ctl.

Replace fixed-length command buffers with psprintf() calls.  We didn't
have anything as convenient as psprintf() when this code was written,
but now that we do, there's little reason for the limitation to
stand.  Removing it eliminates some corner cases where (for example)
starting the postmaster with a whole lot of options fails.

Most individual file names that pg_ctl deals with are still restricted
to MAXPGPATH, but we've seldom had complaints about that limitation
so long as it only applies to one filename.

Back-patch to all supported branches.

Phil Krylov

Discussion: https://postgr.es/m/567e199c6b97ee19deee600311515b86@krylov.eu

3 years agoDisallow creating an ICU collation if the DB encoding won't support it.
Tom Lane [Fri, 3 Sep 2021 20:38:55 +0000 (16:38 -0400)]
Disallow creating an ICU collation if the DB encoding won't support it.

Previously this was allowed, but the collation effectively vanished
into the ether because of the way lookup_collation() works: you could
not use the collation, nor even drop it.  Seems better to give an
error up front than to leave the user wondering why it doesn't work.

(Because this test is in DefineCollation not CreateCollation, it does
not prevent pg_import_system_collations from creating ICU collations,
regardless of the initially-chosen encoding.)

Per bug #17170 from Andrew Bille.  Back-patch to v10 where ICU support
was added.

Discussion: https://postgr.es/m/17170-95845cf3f0a9c36d@postgresql.org

3 years agoFix portability issue in tests from commit ce773f230.
Tom Lane [Fri, 3 Sep 2021 14:01:02 +0000 (10:01 -0400)]
Fix portability issue in tests from commit ce773f230.

Modern POSIX seems to require strtod() to accept "-NaN", but there's
nothing about NaN in SUSv2, and some of our oldest buildfarm members
don't like it.  Let's try writing it as -'NaN' instead; that seems
to produce the same result, at least on Intel hardware.

Per buildfarm.

3 years agoFix float4/float8 hash functions to produce uniform results for NaNs.
Tom Lane [Thu, 2 Sep 2021 21:24:42 +0000 (17:24 -0400)]
Fix float4/float8 hash functions to produce uniform results for NaNs.

The IEEE 754 standard allows a wide variety of bit patterns for NaNs,
of which at least two ("NaN" and "-NaN") are pretty easy to produce
from SQL on most machines.  This is problematic because our btree
comparison functions deem all NaNs to be equal, but our float hash
functions know nothing about NaNs and will happily produce varying
hash codes for them.  That causes unexpected results from queries
that hash a column containing different NaN values.  It could also
produce unexpected lookup failures when using a hash index on a
float column, i.e. "WHERE x = 'NaN'" will not find all the rows
it should.

To fix, special-case NaN in the float hash functions, not too much
unlike the existing special case that forces zero and minus zero
to hash the same.  I arranged for the most vanilla sort of NaN
(that coming from the C99 NAN constant) to still have the same
hash code as before, to reduce the risk to existing hash indexes.

I dithered about whether to back-patch this into stable branches,
but ultimately decided to do so.  It's a clear improvement for
queries that hash internally.  If there is anybody who has -NaN
in a hash index, they'd be well advised to re-index after applying
this patch ... but the misbehavior if they don't will not be much
worse than the misbehavior they had before.

Per bug #17172 from Ma Liangzhu.

Discussion: https://postgr.es/m/17172-7505bea9e04e230f@postgresql.org

3 years agodoc: Replace some uses of "which" by "that" in parallel.sgml
Michael Paquier [Thu, 2 Sep 2021 02:36:01 +0000 (11:36 +0900)]
doc: Replace some uses of "which" by "that" in parallel.sgml

This makes the documentation more accurate grammatically.

Author: Elena Indrupskaya
Discussion: https://postgr.es/m/1c994b3d-951e-59bb-1ac2-7b9221c0e4cf@postgrespro.ru
Backpatch-through: 9.6

3 years agoFix the random test failure in 001_rep_changes.
Amit Kapila [Wed, 1 Sep 2021 03:46:35 +0000 (09:16 +0530)]
Fix the random test failure in 001_rep_changes.

The check to test whether the subscription workers were restarting after a
change in the subscription was failing. The reason was that the test was
assuming the walsender started before it reaches the 'streaming' state and
the walsender was exiting due to an error before that. Now, the walsender
was erroring out before reaching the 'streaming' state because it tries to
acquire the slot before the previous walsender has exited.

In passing, improve the die messages so that it is easier to investigate
the failures in the future if any.

Reported-by: Michael Paquier, as per buildfarm
Author: Ajin Cherian
Reviewed-by: Masahiko Sawada, Amit Kapila
Backpatch-through: 10, where this test was introduced
Discussion: https://postgr.es/m/[email protected]

3 years agoIn pg_dump, avoid doing per-table queries for RLS policies.
Tom Lane [Tue, 31 Aug 2021 19:04:05 +0000 (15:04 -0400)]
In pg_dump, avoid doing per-table queries for RLS policies.

For no particularly good reason, getPolicies() queried pg_policy
separately for each table.  We can collect all the policies in
a single query instead, and attach them to the correct TableInfo
objects using findTableByOid() lookups.  On the regression
database, this reduces the number of queries substantially, and
provides a visible savings even when running against a local
server.

Per complaint from Hubert Depesz Lubaczewski.  Since this is such
a simple fix and can have a visible performance benefit, back-patch
to all supported branches.

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

3 years agoCache the results of format_type() queries in pg_dump.
Tom Lane [Tue, 31 Aug 2021 17:53:33 +0000 (13:53 -0400)]
Cache the results of format_type() queries in pg_dump.

There's long been a "TODO: there might be some value in caching
the results" annotation on pg_dump's getFormattedTypeName function;
but we hadn't gotten around to checking what it was costing us to
repetitively look up type names.  It turns out that when dumping the
current regression database, about 10% of the total number of queries
issued are duplicative format_type() queries.  However, Hubert Depesz
Lubaczewski reported a not-unusual case where these account for over
half of the queries issued by pg_dump.  Individually these queries
aren't expensive, but when network lag is a factor, they add up to a
problem.  We can very easily add some caching to getFormattedTypeName
to solve it.

Since this is such a simple fix and can have a visible performance
benefit, back-patch to all supported branches.

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

3 years agoRename the role in stats_ext to have regress_ prefix
Tomas Vondra [Tue, 31 Aug 2021 17:21:29 +0000 (19:21 +0200)]
Rename the role in stats_ext to have regress_ prefix

Commit 5be8ce82e8 added a new role to the stats_ext regression suite,
but the role name did not start with regress_ causing failures when
running with ENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS. Fixed by
renaming the role to start with the expected regress_ prefix.

Backpatch-through: 10, same as the new regression test
Discussion: https://postgr.es/m/1F238937-7CC2-4703-A1B1-6DC225B8978A%40enterprisedb.com

3 years agoFix lookup error in extended stats ownership check
Tomas Vondra [Tue, 31 Aug 2021 16:03:05 +0000 (18:03 +0200)]
Fix lookup error in extended stats ownership check

When an ownership check on extended statistics object failed, the code
was calling aclcheck_error_type to report the failure, which is clearly
wrong, resulting in cache lookup errors. Fix by calling aclcheck_error.

This issue exists since the introduction of extended statistics, so
backpatch all the way back to PostgreSQL 10. It went unnoticed because
there were no tests triggering the error, so add one.

Reported-by: Mark Dilger
Backpatch-through: 10, where extended stats were introduced
Discussion: https://postgr.es/m/1F238937-7CC2-4703-A1B1-6DC225B8978A%40enterprisedb.com

3 years agoReport tuple address in data-corruption error message
Alvaro Herrera [Mon, 30 Aug 2021 20:29:12 +0000 (16:29 -0400)]
Report tuple address in data-corruption error message

Most data-corruption reports mention the location of the problem, but
this one failed to.  Add it.

Backpatch all the way back.  In 12 and older, also assign the
ERRCODE_DATA_CORRUPTED error code as was done in commit fd6ec93bf890 for
13 and later.

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