postgresql.git
3 years agoFix double publish of child table's data.
Amit Kapila [Thu, 9 Dec 2021 03:06:59 +0000 (08:36 +0530)]
Fix double publish of child table's data.

We publish the child table's data twice for a publication that has both
child and parent tables and is published with publish_via_partition_root
as true. This happens because subscribers will initiate synchronization
using both parent and child tables, since it gets both as separate tables
in the initial table list.

Ensure that pg_publication_tables returns only parent tables in such
cases.

Author: Hou Zhijie
Reviewed-by: Greg Nancarrow, Amit Langote, Vignesh C, Amit Kapila
Backpatch-through: 13
Discussion: https://postgr.es/m/OS0PR01MB57167F45D481F78CDC5986F794B99@OS0PR01MB5716.jpnprd01.prod.outlook.com

3 years agoStandardize cleanup lock terminology.
Peter Geoghegan [Thu, 9 Dec 2021 01:24:45 +0000 (17:24 -0800)]
Standardize cleanup lock terminology.

The term "super-exclusive lock" is a synonym for "buffer cleanup lock"
that first appeared in nbtree many years ago.  Standardize things by
consistently using the term cleanup lock.  This finishes work started by
commit 276db875.

There is no good reason to have two terms.  But there is a good reason
to only have one: to avoid confusion around why VACUUM acquires a full
cleanup lock (not just an ordinary exclusive lock) in index AMs, during
ambulkdelete calls.  This has nothing to do with protecting the physical
index data structure itself.  It is needed to implement a locking
protocol that ensures that TIDs pointing to the heap/table structure
cannot get marked for recycling by VACUUM before it is safe (which is
somewhat similar to how VACUUM uses cleanup locks during its first heap
pass).  Note that it isn't strictly necessary for index AMs to implement
this locking protocol -- several index AMs use an MVCC snapshot as their
sole interlock to prevent unsafe TID recycling.

In passing, update the nbtree README.  Cleanly separate discussion of
the aforementioned index vacuuming locking protocol from discussion of
the "drop leaf page pin" optimization added by commit 2ed5b87f.  We now
structure discussion of the latter by describing how individual index
scans may safely opt out of applying the standard locking protocol (and
so can avoid blocking progress by VACUUM).  Also document why the
optimization is not safe to apply during nbtree index-only scans.

Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-WzngHgQa92tz6NQihf4nxJwRzCV36yMJO_i8dS+2mgEVKw@mail.gmail.com
Discussion: https://postgr.es/m/CAH2-WzkHPgsBBvGWjz=8PjNhDefy7XRkDKiT5NxMs-n5ZCf2dA@mail.gmail.com

3 years agoDoc: improve xfunc-c-type-table.
Tom Lane [Wed, 8 Dec 2021 21:54:31 +0000 (16:54 -0500)]
Doc: improve xfunc-c-type-table.

List types numeric and timestamptz, which don't seem to have ever been
included here.  Restore bigint, which was no-doubt-accidentally deleted
in v12.  Fix some errors, or at least obsolete usages (nobody declares
float arguments as "float8*" anymore, even though they might be that
under the hood).  Re-alphabetize.  Remove the seeming claim that this
is a complete list of built-in types.

Per question from Oskar Stenberg.

Discussion: https://postgr.es/m/HE1PR03MB2971DE2527ECE1E99D6C19A8F96E9@HE1PR03MB2971.eurprd03.prod.outlook.com

3 years agoRevert "Check that we have a working tar before trying to use it"
Andrew Dunstan [Wed, 8 Dec 2021 21:45:39 +0000 (16:45 -0500)]
Revert "Check that we have a working tar before trying to use it"

This reverts commit f920f7e799c587228227ec94356c760e3f3d5f2b.

The patch in effect fixed a problem we didn't have and caused another
instead.

Backpatch to release 14 like original

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

3 years agoCheck that we have a working tar before trying to use it
Andrew Dunstan [Wed, 8 Dec 2021 15:21:35 +0000 (10:21 -0500)]
Check that we have a working tar before trying to use it

Issue exposed by commit edc2332550 and the buildfarm.

Backpatch to release 14 where this usage started.

3 years agopostgres_fdw: Report warning when timeout expires while getting query result.
Fujii Masao [Wed, 8 Dec 2021 14:31:46 +0000 (23:31 +0900)]
postgres_fdw: Report warning when timeout expires while getting query result.

When aborting remote transaction or sending cancel request to a remote server,
postgres_fdw calls pgfdw_get_cleanup_result() to wait for the result of
transaction abort query or cancel request to arrive. It fails to get the result
if the timeout expires or a connection trouble happens.

Previously postgres_fdw reported no warning message even when the timeout
expired or a connection trouble happened in pgfdw_get_cleanup_result().
This could make the troubleshooting harder when such an event occurred.

This commit makes pgfdw_get_cleanup_result() tell its caller what trouble
(timeout or connection error) occurred, on failure, and also makes its caller
report the proper warning message based on that information.

Author: Fujii Masao
Reviewed-by: Bharath Rupireddy
Discussion: https://postgr.es/m/15aa988c-722e-ad3e-c936-4420c5b2bfea@oss.nttdata.com

3 years agoAllow specifying column list for foreign key ON DELETE SET actions
Peter Eisentraut [Wed, 8 Dec 2021 10:09:44 +0000 (11:09 +0100)]
Allow specifying column list for foreign key ON DELETE SET actions

Extend the foreign key ON DELETE actions SET NULL and SET DEFAULT by
allowing the specification of a column list, like

    CREATE TABLE posts (
        ...
        FOREIGN KEY (tenant_id, author_id) REFERENCES users ON DELETE SET NULL (author_id)
    );

If a column list is specified, only those columns are set to
null/default, instead of all the columns in the foreign-key
constraint.

This is useful for multitenant or sharded schemas, where the tenant or
shard ID is included in the primary key of all tables but shouldn't be
set to null.

Author: Paul Martinez <paulmtz@google.com>
Discussion: https://www.postgresql.org/message-id/flat/CACqFVBZQyMYJV=njbSMxf+rbDHpx=W=B7AEaMKn8dWn9OZJY7w@mail.gmail.com

3 years agoFix origin timestamp during decoding of ROLLBACK PREPARED operation.
Amit Kapila [Wed, 8 Dec 2021 09:48:56 +0000 (15:18 +0530)]
Fix origin timestamp during decoding of ROLLBACK PREPARED operation.

This happens because we were passing incorrect arguments to
ReorderBufferFinishPrepared().

Author: Masahiko Sawada
Reviewed-by: Vignesh C
Backpatch-through: 14
Discussion: https://postgr.es/m/CAD21AoBqhUqgDZUhUVnnwKRubPDNJ6m6fJDPgok3E5cWJLL+pA@mail.gmail.com

3 years agopgrowlocks: Fix incorrect format placeholders
Peter Eisentraut [Wed, 8 Dec 2021 08:27:15 +0000 (09:27 +0100)]
pgrowlocks: Fix incorrect format placeholders

Transaction IDs should be printed as unsigned, similar to xidout().

3 years agoFix changing the ownership of ALL TABLES IN SCHEMA publication.
Amit Kapila [Wed, 8 Dec 2021 06:01:16 +0000 (11:31 +0530)]
Fix changing the ownership of ALL TABLES IN SCHEMA publication.

Ensure that the new owner of ALL TABLES IN SCHEMA publication must be a
superuser. The same is already ensured during CREATE PUBLICATION.

Author: Vignesh C
Reviewed-by: Nathan Bossart, Greg Nancarrow, Michael Paquier, Haiying Tang
Discussion: https://postgr.es/m/CALDaNm0E5U-RqxFuFrkZrQeG7ae5trGa=xs=iRtPPHULtT4zOw@mail.gmail.com

3 years agoDe-duplicate the result of pg_publication_tables view.
Amit Kapila [Wed, 8 Dec 2021 05:45:25 +0000 (11:15 +0530)]
De-duplicate the result of pg_publication_tables view.

We show duplicate values for child tables in publications that have both
child and parent tables and are published with publish_via_partition_root
as false which is not what the user would expect.

We decided not to backpatch this as there is no user complaint about this
and it doesn't seem to be a critical issue.

Author: Hou Zhijie
Reviewed-by: Bharath Rupireddy, Amit Langote, Amit Kapila
Discussion: https://postgr.es/m/OS0PR01MB5716E97F00732B52DC2BBC2594989@OS0PR01MB5716.jpnprd01.prod.outlook.com

3 years agopgcrypto: Remove explicit hex encoding/decoding from tests
Peter Eisentraut [Wed, 8 Dec 2021 05:01:35 +0000 (06:01 +0100)]
pgcrypto: Remove explicit hex encoding/decoding from tests

This was from before the hex format was available in bytea.  Now we
can remove the extra explicit encoding/decoding calls and rely on the
default output format.

Discussion: https://www.postgresql.org/message-id/flat/17dcb4f7-7ac1-e2b6-d5f7-2dfba06cd9ee%40enterprisedb.com

3 years agoImprove parsing of options of CREATE/ALTER SUBSCRIPTION
Michael Paquier [Wed, 8 Dec 2021 03:36:31 +0000 (12:36 +0900)]
Improve parsing of options of CREATE/ALTER SUBSCRIPTION

This simplifies the code so as it is not necessary anymore for the
caller of parse_subscription_options() to zero SubOpts, holding a
bitmaps of the provided options as well as the default/parsed option
values.  This also simplifies some checks related to the options
supported by a command when checking for incompatibilities.

While on it, the errors generated for unsupported combinations with
"slot_name = NONE" are reordered.  This may generate a different errors
compared to the previous major versions, but users have to go through
all those errors to get a correct command in this case when using
incorrect values for options "enabled" and "create\slot", so at the end
the resulting command would remain the same.

Author: Peter Smith
Reviewed-by: Nathan Bossart
Discussion: https://postgr.es/m/CAHut+PtXHfLgLHDDJ8ZN5f5Be_37mJoxpEsRg8LNmm4XCr06Rw@mail.gmail.com

3 years agoFix corruption of toast indexes with REINDEX CONCURRENTLY
Michael Paquier [Wed, 8 Dec 2021 02:01:08 +0000 (11:01 +0900)]
Fix corruption of toast indexes with REINDEX CONCURRENTLY

REINDEX CONCURRENTLY run on a toast index or a toast relation could
corrupt the target indexes rebuilt, as a backend running in parallel
that manipulates toast values would directly release the lock on the
toast relation when its local operation is done, rather than releasing
the lock once the transaction that manipulated the toast values
committed.

The fix done here is simple: we now hold a ROW EXCLUSIVE lock on the
toast relation when saving or deleting a toast value until the
transaction working on them is committed, so as a concurrent reindex
happening in parallel would be able to wait for any activity and see any
new rows inserted (or deleted).

An isolation test is added to check after the case fixed here, which is
a bit fancy by design as it relies on allow_system_table_mods to rename
the toast table and its index to fixed names.  This way, it is possible
to reindex them directly without any dependency on the OID of the
underlying relation.  Note that this could not use a DO block either, as
REINDEX CONCURRENTLY cannot be run in a transaction block.  The test is
backpatched down to 13, where it is possible, thanks to c4a7a39, to use
allow_system_table_mods in a test suite.

Reported-by: Alexey Ermakov
Analyzed-by: Andres Freund, Noah Misch
Author: Michael Paquier
Reviewed-by: Nathan Bossart
Discussion: https://postgr.es/m/17268-d2fb426e0895abd4@postgresql.org
Backpatch-through: 12

3 years agoEnable settings used in TAP tests for MSVC builds
Andrew Dunstan [Tue, 7 Dec 2021 19:53:39 +0000 (14:53 -0500)]
Enable settings used in TAP tests for MSVC builds

Certain settings from configuration or the Makefile infrastructure are
used by the TAP tests, but were not being set up by vcregress.pl. This
remedies those omissions. This should increase test coverage, especially
on the buildfarm.

Reviewed by Noah Misch

Discussion: https://postgr.es/m/17093da5-e40d-8335-d53a-2bd803fc38b0@dunslane.net

Backpatch to all live branches.

3 years agoOn Windows, also call shutdown() while closing the client socket.
Tom Lane [Tue, 7 Dec 2021 18:34:06 +0000 (13:34 -0500)]
On Windows, also call shutdown() while closing the client socket.

Further experimentation shows that commit 6051857fc is not sufficient
when using (some versions of?) OpenSSL.  The reason is obscure, but
calling shutdown(socket, SD_SEND) improves matters.

Per testing by Andrew Dunstan and Alexander Lakhin.
Back-patch as before.

Discussion: https://postgr.es/m/af5e0bf3-6a61-bb97-6cba-061ddf22ff6b@dunslane.net

3 years agoUpdate snowball
Peter Eisentraut [Tue, 7 Dec 2021 05:26:05 +0000 (06:26 +0100)]
Update snowball

Update to snowball tag v2.2.0.  Minor changes only.

3 years agoAccount for TOAST data while scheduling parallel dumps.
Tom Lane [Mon, 6 Dec 2021 18:23:07 +0000 (13:23 -0500)]
Account for TOAST data while scheduling parallel dumps.

In parallel mode, pg_dump tries to order the table-data-dumping
jobs with the largest tables first.  However, it was only
consulting the pg_class.relpages value to determine table size.
This ignores TOAST data, and so we could make poor scheduling
decisions in cases where some large tables are mostly TOASTed
data while others have very little.  To fix, add in the relpages
value for the TOAST table as well.

This patch also fixes a potential integer-overflow issue that
could result in poor scheduling on machines where off_t is
only 32 bits wide.  Such platforms are probably extinct in the
wild, but we do still nominally support them, so repair.

Per complaint from Hans Buschmann.

Discussion: https://postgr.es/m/7d7eb6128f40401d81b3b7a898b6b4de@W2012-02.nidsa.loc

3 years agoUse PREPARE/EXECUTE for repetitive per-object queries in pg_dump.
Tom Lane [Mon, 6 Dec 2021 18:14:29 +0000 (13:14 -0500)]
Use PREPARE/EXECUTE for repetitive per-object queries in pg_dump.

For objects such as functions, pg_dump issues the same secondary
data-collection query against each object to be dumped.  This can't
readily be refactored to avoid the repetitive queries, but we can
PREPARE these queries to reduce planning costs.

This patch applies the idea to functions, aggregates, operators, and
data types.  While it could be carried further, the remaining sorts of
objects aren't likely to appear in typical databases enough times to
be worth worrying over.  Moreover, doing the PREPARE is likely to be a
net loss if there aren't at least some dozens of objects to apply the
prepared query to.

Discussion: https://postgr.es/m/7d7eb6128f40401d81b3b7a898b6b4de@W2012-02.nidsa.loc

3 years agoAvoid per-object queries in performance-critical paths in pg_dump.
Tom Lane [Mon, 6 Dec 2021 18:07:31 +0000 (13:07 -0500)]
Avoid per-object queries in performance-critical paths in pg_dump.

Instead of issuing a secondary data-collection query against each
table to be dumped, issue just one query, with a WHERE clause
restricting it to be applied to only the tables we intend to dump.
Likewise for indexes, constraints, and triggers.  This greatly
reduces the number of queries needed to dump a database containing
many tables.  It might seem that WHERE clauses listing many target
OIDs could be inefficient, but at least on recent server versions
this provides a very substantial speedup.

(In principle the same thing could be done with other object types
such as functions; but that would require significant refactoring
of pg_dump, so those will be tackled in a different way in a
following patch.)

The new WHERE clauses depend on the unnest() function, which is
only present in 8.4 and above.  We could implement them differently
for older servers, but there is an ongoing discussion that will
probably result in dropping pg_dump support for servers before 9.2,
so that seems like it'd be wasted work.  For now, just bump the
server version check to require >= 8.4, without stopping to remove
any of the code that's thereby rendered dead.  We'll mop that
situation up soon.

Patch by me, based on an idea from Andres Freund.

Discussion: https://postgr.es/m/7d7eb6128f40401d81b3b7a898b6b4de@W2012-02.nidsa.loc

3 years agoPostpone calls of unsafe server-side functions in pg_dump.
Tom Lane [Mon, 6 Dec 2021 17:49:49 +0000 (12:49 -0500)]
Postpone calls of unsafe server-side functions in pg_dump.

Avoid calling pg_get_partkeydef(), pg_get_expr(relpartbound),
and regtypeout until we have lock on the relevant tables.
The existing coding is at serious risk of failure if there
are any concurrent DROP TABLE commands going on --- including
drops of other sessions' temp tables.

Arguably this is a bug fix that should be back-patched, but it's
moderately invasive and we've not had all that many complaints
about such failures.  Let's just put it in HEAD for now.

Discussion: https://postgr.es/m/2273648.1634764485@sss.pgh.pa.us
Discussion: https://postgr.es/m/7d7eb6128f40401d81b3b7a898b6b4de@W2012-02.nidsa.loc

3 years agoRethink pg_dump's handling of object ACLs.
Tom Lane [Mon, 6 Dec 2021 17:39:45 +0000 (12:39 -0500)]
Rethink pg_dump's handling of object ACLs.

Throw away most of the existing logic for this, as it was very
inefficient thanks to expensive sub-selects executed to collect
ACL data that we very possibly would have no interest in dumping.
Reduce the ACL handling in the initial per-object-type queries
to be just collection of the catalog ACL fields, as it was
originally.  Fetch pg_init_privs data separately in a single
scan of that catalog, and do the merging calculations on the
client side.  Remove the separate code path used for pre-9.6
source servers; there is no good reason to treat them differently
from newer servers that happen to have empty pg_init_privs.

Discussion: https://postgr.es/m/2273648.1634764485@sss.pgh.pa.us
Discussion: https://postgr.es/m/7d7eb6128f40401d81b3b7a898b6b4de@W2012-02.nidsa.loc

3 years agoRefactor pg_dump's tracking of object components to be dumped.
Tom Lane [Mon, 6 Dec 2021 17:25:48 +0000 (12:25 -0500)]
Refactor pg_dump's tracking of object components to be dumped.

Split the DumpableObject.dump bitmask field into separate bitmasks
tracking which components are requested to be dumped (in the
existing "dump" field) and which components exist for the particular
object (in the new "components" field).  This gets rid of some
klugy and easily-broken logic that involved setting bits and later
clearing them.  More importantly, it restores the originally intended
behavior that pg_dump's secondary data-gathering queries should not
be executed for objects we have no interest in dumping.  That
optimization got broken when the dump flag was turned into a bitmask,
because irrelevant bits tended to remain set in many cases.  Since
the "components" field starts from a minimal set of bits and is
added onto as needed, ANDing it with "dump" provides a reliable
indicator of what we actually have to dump, without having to
complicate the logic that manages the request bits.  This makes
a significant difference in the number of queries needed when,
for example, there are many functions in extensions.

Discussion: https://postgr.es/m/2273648.1634764485@sss.pgh.pa.us
Discussion: https://postgr.es/m/7d7eb6128f40401d81b3b7a898b6b4de@W2012-02.nidsa.loc

3 years agoFix inappropriate uses of PG_GETARG_UINT32()
Peter Eisentraut [Mon, 6 Dec 2021 12:26:50 +0000 (13:26 +0100)]
Fix inappropriate uses of PG_GETARG_UINT32()

The chr() function used PG_GETARG_UINT32() even though the argument is
declared as (signed) integer.  As a result, you can pass negative
arguments to this function and it internally interprets them as
positive.  Ultimately ends up being harmless, but it seems wrong, so
fix this and rearrange the internal error checking a bit to
accommodate this.

Another case was in the documentation, where example code used
PG_GETARG_UINT32() with an argument declared as signed integer.

Reviewed-by: Nathan Bossart <bossartn@amazon.com>
Discussion: https://www.postgresql.org/message-id/flat/7e43869b-d412-8f81-30a3-809783edc9a3%40enterprisedb.com

3 years agoSilence perl complaint in ssl test
Andrew Dunstan [Sun, 5 Dec 2021 16:50:03 +0000 (11:50 -0500)]
Silence perl complaint in ssl test

Perl's hex() function complains if its argument contains trailing white
space (or in fact anything other than hex digits), so remove the
offending text.

3 years agoDoc: Fix misleading wording of CRL parameters
Daniel Gustafsson [Fri, 3 Dec 2021 13:15:50 +0000 (14:15 +0100)]
Doc: Fix misleading wording of CRL parameters

ssl_crl_file and ssl_crl_dir are both used to for client certificate
revocation, not server certificates.  The description for the params
could be easily misread to mean the opposite however,  as evidenced
by the bugreport leading to this fix.  Similarly, expand sslcrl and
and sslcrldir to explicitly mention server certificates. While there
also mention sslcrldir where previously only sslcrl was discussed.

Backpatch down to v10, with the CRL dir fixes down to 14 where they
were introduced.

Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Discussion: https://postgr.es/m/20211202.135441.590555657708629486.horikyota.ntt@gmail.com
Discussion: https://postgr.es/m/CABWY_HCBUCjY1EJHrEGePGEaSZ5b29apgTohCyygtsqe_ySYng@mail.gmail.com
Backpatch-through: 10

3 years agoSome RELKIND macro refactoring
Peter Eisentraut [Fri, 3 Dec 2021 12:38:26 +0000 (13:38 +0100)]
Some RELKIND macro refactoring

Add more macros to group some RELKIND_* macros:

- RELKIND_HAS_PARTITIONS()
- RELKIND_HAS_TABLESPACE()
- RELKIND_HAS_TABLE_AM()

Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://www.postgresql.org/message-id/flat/a574c8f1-9c84-93ad-a9e5-65233d6fc00f%40enterprisedb.com

3 years agoFix path delimiters in connection string on Windows
Daniel Gustafsson [Fri, 3 Dec 2021 10:41:17 +0000 (11:41 +0100)]
Fix path delimiters in connection string on Windows

The temporary path generated in commit c113d8ad5 cannot be passed as-is in
the connection string on Windows since the path delimiting backslashes will
be treated as escape characters. Fix by converting backslash to slash as in
similar path usecases in other tests.

Reported-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20211202195130.e7pprpsx4ell22sp@alap3.anarazel.de

3 years agopostgres_fdw: Fix unexpected reporting of empty message.
Fujii Masao [Fri, 3 Dec 2021 08:35:29 +0000 (17:35 +0900)]
postgres_fdw: Fix unexpected reporting of empty message.

pgfdw_report_error() in postgres_fdw gets a message from PGresult or
PGconn to report an error received from a remote server. Previously
if it could get a message from neither of them, it reported empty
message unexpectedly. The cause of this issue was that pgfdw_report_error()
didn't handle properly the case where no message could be obtained
and its local variable message_primary was set to '\0'.

This commit improves pgfdw_report_error() so that it reports the message
"could not obtain ..." when it gets no message and message_primary
is set to '\0'. This is the same behavior as when message_primary is NULL.

dblink_res_error() in dblink has the same issue, so this commit also
improves it in the same way.

Back-patch to all supported branches.

Author: Fujii Masao
Reviewed-by: Bharath Rupireddy
Discussion: https://postgr.es/m/477c16c8-7ea4-20fc-38d5-ed3a77ed616c@oss.nttdata.com

3 years agoImprove the description of various GUCs
Michael Paquier [Fri, 3 Dec 2021 00:39:03 +0000 (09:39 +0900)]
Improve the description of various GUCs

This commit fixes a couple of inconsistencies in the descriptions of
some GUCs, while making their wording more general regarding the units
they rely on.

For most of them, this removes the use of terms like "N seconds" or "N
bytes", which may not apply easily to all the languages these strings
are translated to (from my own experience, this works in French and
English, less in Japanese).

Per debate between the authors listed below.

Author: Justin Pryzby, Michael Paquier
Discussion: https://postgr.es/m/20211129030833.GJ17618@telsasoft.com

3 years agoOn Windows, close the client socket explicitly during backend shutdown.
Tom Lane [Thu, 2 Dec 2021 22:14:43 +0000 (17:14 -0500)]
On Windows, close the client socket explicitly during backend shutdown.

It turns out that this is necessary to keep Winsock from dropping any
not-yet-sent data, such as an error message explaining the reason for
process termination.  It's pretty weird that the implicit close done
by the kernel acts differently from an explicit close, but it's hard
to argue with experimental results.

Independently submitted by Alexander Lakhin and Lars Kanis (comments
by me, though).  Back-patch to all supported branches.

Discussion: https://postgr.es/m/90b34057-4176-7bb0-0dbb-9822a5f6425b@greiz-reinsdorf.de
Discussion: https://postgr.es/m/16678-253e48d34dc0c376@postgresql.org

3 years agoAdd configure probe for rl_variable_bind().
Tom Lane [Thu, 2 Dec 2021 18:06:27 +0000 (13:06 -0500)]
Add configure probe for rl_variable_bind().

Some exceedingly ancient readline libraries lack this function, causing
commit 3d858af07 to fail.  Per buildfarm (via Michael Paquier).

Discussion: https://postgr.es/m/E1msTLm-0007Cm-Ri@gemulon.postgresql.org

3 years agopg_dump: Add missing relkind case
Peter Eisentraut [Thu, 2 Dec 2021 15:46:28 +0000 (16:46 +0100)]
pg_dump: Add missing relkind case

Checking for RELKIND_MATVIEW was forgotten in
guessConstraintInheritance().  This isn't a live problem, since it is
checked in flagInhTables() which relkinds can have parents, and those
entries will have numParents==0 after that.  But after discussion it
was felt that this place should be kept consistent with
flagInhTables() and flagInhAttrs().

Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/a574c8f1-9c84-93ad-a9e5-65233d6fc00f@enterprisedb.com

3 years agopg_waldump: Emit stats summary when interrupted by SIGINT
Michael Paquier [Thu, 2 Dec 2021 04:52:16 +0000 (13:52 +0900)]
pg_waldump: Emit stats summary when interrupted by SIGINT

Previously, pg_waldump would not display its statistics summary if it
got interrupted by SIGINT (or say a simple Ctrl+C).  It gains with this
commit a signal handler for SIGINT, trapping the signal to exit at the
earliest convenience to allow a display of the stats summary before
exiting.  This makes the reports more interactive, similarly to strace
-c.

This new behavior makes the combination of the options --stats and
--follow much more useful, so as the user will get a report for any
invocation of pg_waldump in such a case.  Information about the LSN
range of the stats computed is added as a header to the report
displayed.

This implementation comes from a suggestion by Álvaro Herrera and
myself, following a complaint by the author of this patch about --stats
and --follow not being useful together originally.

As documented, this is not supported on Windows, though its support
would be possible by catching the terminal events associated to Ctrl+C,
for example (this may require a more centralized implementation, as
other tools could benefit from a common API).

Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACUUx3PcK2z9h0_m7vehreZAUbcmOky9WSEpe8TofhV=PQ@mail.gmail.com

3 years agoMove into separate file all the SQL queries used in pg_upgrade tests
Michael Paquier [Thu, 2 Dec 2021 01:31:20 +0000 (10:31 +0900)]
Move into separate file all the SQL queries used in pg_upgrade tests

The existing pg_upgrade/test.sh and the buildfarm code have been holding
the same set of SQL queries when doing cross-version upgrade tests to
adapt the objects created by the regression tests before the upgrade
(mostly, incompatible or non-existing objects need to be dropped from
the origin, perhaps re-created).

This moves all those SQL queries into a new, separate, file with a set
of \if clauses to handle the version checks depending on the old version
of the cluster to-be-upgraded.

The long-term plan is to make the buildfarm code re-use this new SQL
file, so as committers are able to fix any compatibility issues in the
tests of pg_upgrade with a refresh of the core code, without having to
poke at the buildfarm client.  Note that this is only able to handle the
main regression test suite, and that nothing is done yet for contrib
modules yet (these have more issues like their database names).

A backpatch down to 10 is done, adapting the version checks as this
script needs to be only backward-compatible, so as it becomes possible
to clean up a maximum amount of code within the buildfarm client.

Author: Justin Pryzby, Michael Paquier
Discussion: https://postgr.es/m/20201206180248.GI24052@telsasoft.com
Backpatch-through: 10

3 years agoAvoid leaking memory during large-scale REASSIGN OWNED BY operations.
Tom Lane [Wed, 1 Dec 2021 18:44:46 +0000 (13:44 -0500)]
Avoid leaking memory during large-scale REASSIGN OWNED BY operations.

The various ALTER OWNER routines tend to leak memory in
CurrentMemoryContext.  That's not a problem when they're only called
once per command; but in this usage where we might be touching many
objects, it can amount to a serious memory leak.  Fix that by running
each call in a short-lived context.

(DROP OWNED BY likely has a similar issue, except that you'll probably
run out of lock table space before noticing.  REASSIGN is worth fixing
since for most non-table object types, it won't take any lock.)

Back-patch to all supported branches.  Unfortunately, in the back
branches this helps to only a limited extent, since the sinval message
queue bloats quite a lot in this usage before commit 3aafc030a,
consuming memory more or less comparable to what's actually leaked.
Still, it's clearly a leak with a simple fix, so we might as well fix it.

Justin Pryzby, per report from Guillaume Lelarge

Discussion: https://postgr.es/m/CAECtzeW2DAoioEGBRjR=CzHP6TdL=yosGku8qZxfX9hhtrBB0Q@mail.gmail.com

3 years agopsql: initialize comment-begin setting to a useful value by default.
Tom Lane [Wed, 1 Dec 2021 17:24:50 +0000 (12:24 -0500)]
psql: initialize comment-begin setting to a useful value by default.

Readline's meta-# command is supposed to insert a comment marker
at the start of the current line.  However, the default marker is
"#" which is entirely unhelpful for SQL.  Set it to "-- " instead.
(This setting can still be overridden in one's ~/.inputrc file,
so this change won't affect people who have already taken steps
to make the command useful.)

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

3 years agopsql: treat "--" comments between queries as separate history entries.
Tom Lane [Wed, 1 Dec 2021 17:18:25 +0000 (12:18 -0500)]
psql: treat "--" comments between queries as separate history entries.

If we've not yet collected any non-whitespace, non-comment token for a
new query, flush the current input line to history before reading
another line.  This aligns psql's history behavior with the observation
that lines containing only comments are generally not thought of as
being part of the next query.  psql's prompting behavior is consistent
with that view, too, since it won't change the prompt until you
enter something that's neither whitespace nor a "--" comment.

Greg Nancarrow, simplified a bit by me

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

3 years agopsql: include intra-query "--" comments in what's sent to the server.
Tom Lane [Wed, 1 Dec 2021 17:06:31 +0000 (12:06 -0500)]
psql: include intra-query "--" comments in what's sent to the server.

psql's lexer has historically deleted dash-dash (single-line) comments
from what's collected and sent to the server.  This is inconsistent
with what it does for slash-star comments, and people have complained
before that they wish such comments would be captured in the server log.
Undoing the decision completely seems like too big a behavioral change,
however.  In particular, comments on lines preceding the start of a
query are generally not thought of as being part of that query.

What we can do to improve the situation is to capture comments that
are clearly *within* a query, that is after the first non-whitespace,
non-comment token but before the query's ending semicolon or backslash
command.  This is a nearly trivial code change, and it affects only a
few regression test results.

(It is tempting to try to apply the same rule to slash-star comments.
But it's hard to see how to do that without getting strange history
behavior for comments that cross lines, especially if the user then
starts a new query on the same line as the star-slash.  In view of
the lack of complaints, let's leave that case alone.)

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

3 years agoRemove unused includes
Peter Eisentraut [Wed, 24 Nov 2021 11:30:23 +0000 (12:30 +0100)]
Remove unused includes

These haven't been needed for a long time.

Reviewed-by: John Naylor <john.naylor@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/b239564c-cad0-b23e-c57e-166d883cb97d@enterprisedb.com

3 years agoImprove some comments in scanner files
Peter Eisentraut [Wed, 24 Nov 2021 08:10:32 +0000 (09:10 +0100)]
Improve some comments in scanner files

Reviewed-by: John Naylor <john.naylor@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/b239564c-cad0-b23e-c57e-166d883cb97d@enterprisedb.com

3 years agoWarning on SET of nonexisting setting with a prefix reserved by an extension
Peter Eisentraut [Wed, 1 Dec 2021 14:07:29 +0000 (15:07 +0100)]
Warning on SET of nonexisting setting with a prefix reserved by an extension

An extension can already de facto reserve a GUC prefix using
EmitWarningsOnPlaceholders().  But this was only checked against
settings that exist at the time the extension is loaded (or the
extension chooses to call this).  No diagnostic is given when a SET
command later uses a nonexisting setting with a custom prefix.

With this change, EmitWarningsOnPlaceholders() saves the prefixes it
reserves in a list, and SET checks when it finds a "placeholder"
setting whether it belongs to a reserved prefix and issues a warning
in that case.

Add a regression test that checks the patch using the "plpgsql"
registered prefix.

Author: Florin Irion <florin.irion@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/CA+HEvJDhWuuTpGTJT9Tgbdzm4QS4EzPAwDBScWK18H2Q=FVJFw@mail.gmail.com

3 years agoFix certificate paths to use perl2host
Daniel Gustafsson [Wed, 1 Dec 2021 13:59:51 +0000 (14:59 +0100)]
Fix certificate paths to use perl2host

Commit c113d8ad50 moved the copying of certificates into a temporary path
for the duration of the tests, instead of using the source tree. This broke
the tests on msys as the absolute path wasn't adapted for the msys platform.
Ensure to convert the path with perl2host before copying and passing in the
connection string.

While there also make certificate copying error handling uniform across all
the test suites.

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

3 years agoDocument that tar archives are now properly terminated.
Robert Haas [Wed, 1 Dec 2021 13:55:00 +0000 (08:55 -0500)]
Document that tar archives are now properly terminated.

Commit 5a1007a5088cd6ddf892f7422ea8dbaef362372f changed the server
behavior, but I didn't notice that the existing behavior was
documented, and therefore did not update the documentation.
This commit does that.

I chose to mention that the behavior has changed rather than just
removing the reference to a deviation from a standard. It seemed
like that might be helpful to tool authors.

Discussion: http://postgr.es/m/CA+TgmoaYZbz0=Yk797aOJwkGJC-LK3iXn+wzzMx7KdwNpZhS5g@mail.gmail.com

3 years agoRemove mention of TimeLineID update from comments
Daniel Gustafsson [Wed, 1 Dec 2021 13:17:24 +0000 (14:17 +0100)]
Remove mention of TimeLineID update from comments

Commit 4a92a1c3d removed the TimeLineID update from RecoveryInProgress,
update comments accordingly.

Author: Amul Sul <sulamul@gmail.com>
Discussion: https://postgr.es/m/CAAJ_b96wyzs8N45jc-kYd-bTE02hRWQieLZRpsUtNbhap7_PuQ@mail.gmail.com

3 years agoFix comment grammar in slotfuncs.c
Michael Paquier [Wed, 1 Dec 2021 11:28:19 +0000 (20:28 +0900)]
Fix comment grammar in slotfuncs.c

Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACUkrNR2xTak+QaqxoTjPKGn8zXWripv7SR27t+Q5qF1Wg@mail.gmail.com

3 years agodoc: Some additional information about when to use referential actions
Peter Eisentraut [Wed, 1 Dec 2021 10:25:58 +0000 (11:25 +0100)]
doc: Some additional information about when to use referential actions

3 years agoFix regression test failure caused by commit 8d74fc96db.
Amit Kapila [Wed, 1 Dec 2021 07:21:37 +0000 (12:51 +0530)]
Fix regression test failure caused by commit 8d74fc96db.

The tests didn't considered that an error unrelated to apply changes, e.g.
"replication origin with OID %d is already active ...", could occur on the
table sync worker before starting to copy changes.

To make the test robust we instead need to check the expected error and
the source of error which will be either tablesync or apply worker.

In passing remove the harmless option "streaming = off" from Create
Subscription command as that is anyway the default.

Per buildfarm member sidewinder.

Author: Masahiko Sawada
Reviewed-by: Hou Zhijie, Vignesh C, Amit Kapila
Discussion: https://postgr.es/m/CAD21AoDeScrsHhLyEPYqN3sydg6PxAPVBboK=30xJfUVihNZDA@mail.gmail.com
Discussion: https://postgr.es/m/E1mrtvV-0002Gz-73@gemulon.postgresql.org

3 years agoDoc: Add "Attach Partition" limitation during logical replication.
Amit Kapila [Wed, 1 Dec 2021 04:37:45 +0000 (10:07 +0530)]
Doc: Add "Attach Partition" limitation during logical replication.

ATTACHing a table into a partition tree whose root is published using a
publication with publish_via_partition_root set to true does not result in
the table's existing contents being replicated. This happens because
subscriber doesn't consider replicating the newly attached partition as
the root table is already in a 'ready' state.

This behavior was introduced in PG13 (83fd4532a7) where we allowed to
publish partition changes via ancestors.

We can consider fixing this limitation in the future.

Author: Amit Langote
Reviewed-by: Hou Zhijie, Amit Kapila
Backpatch-through: 13
Discussion: https://postgr.es/m/OS0PR01MB5716E97F00732B52DC2BBC2594989@OS0PR01MB5716.jpnprd01.prod.outlook.com

3 years agoImprove psql tab completion for various DROP commands
Michael Paquier [Wed, 1 Dec 2021 01:50:51 +0000 (10:50 +0900)]
Improve psql tab completion for various DROP commands

The following improvements are done:
- Handling of RESTRICT/CASCADE for DROP OWNED, matviews and policies.
- Handling of DROP TRANSFORM

This is a continuation of the work done in 0cd6d3b and f44ceb4.

Author: Ken Kato
Reviewed-by: Asif Rehman
Discussion: https://postgr.es/m/0fafb73f3a0c6bcec817a25ca9d5a853@oss.nttdata.com

3 years agoExtend the private key stat checking error handling
Daniel Gustafsson [Tue, 30 Nov 2021 22:23:57 +0000 (23:23 +0100)]
Extend the private key stat checking error handling

If the stat operation on the private key failed, the code assumed it
was due to an ENOENT, which may or may not be true. Extend the check
by printing a different error message on non-ENOENT errors for easier
debugging.

Per suggestion by Tom Lane due to an issue with the fairywren animal
in the buildfarm.

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

3 years agoCope with cross-compiling when checking for a random-number source.
Tom Lane [Tue, 30 Nov 2021 22:18:04 +0000 (17:18 -0500)]
Cope with cross-compiling when checking for a random-number source.

Commit 16f96c74d neglected to consider the possibility of cross-compiling,
causing cross-compiles to fail at the configure stage unless you'd
selected --with-openssl.  Since we're now more or less assuming that
/dev/urandom is available everywhere, it seems reasonable to assume
that the cross-compile target has it too, rather than failing.

Per complaint from Vincas Dargis.  Back-patch to v14 where this came in.

Discussion: https://postgr.es/m/0dc14a31-acaf-8cae-0df4-a87339b22bd9@gmail.com

3 years agovacuumlazy.c: fix remaining "dead tuple" references.
Peter Geoghegan [Tue, 30 Nov 2021 19:40:33 +0000 (11:40 -0800)]
vacuumlazy.c: fix remaining "dead tuple" references.

Oversight in commit 4f8d9d12.

Reported-By: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAD21AoDm38Em0bvRqeQKr4HPvOj65Y8cUgCP4idMk39iaLrxyw@mail.gmail.com

3 years agoIgnore BRIN indexes when checking for HOT udpates
Tomas Vondra [Tue, 30 Nov 2021 18:15:14 +0000 (19:15 +0100)]
Ignore BRIN indexes when checking for HOT udpates

When determining whether an index update may be skipped by using HOT, we
can ignore attributes indexed only by BRIN indexes. There are no index
pointers to individual tuples in BRIN, and the page range summary will
be updated anyway as it relies on visibility info.

This also removes rd_indexattr list, and replaces it with rd_attrsvalid
flag. The list was not used anywhere, and a simple flag is sufficient.

Patch by Josef Simanek, various fixes and improvements by me.

Author: Josef Simanek
Reviewed-by: Tomas Vondra, Alvaro Herrera
Discussion: https://postgr.es/m/CAFp7QwpMRGcDAQumN7onN9HjrJ3u4X3ZRXdGFT0K5G2JWvnbWg%40mail.gmail.com

3 years agoIncrease size of shared memory for pg_commit_ts
Alvaro Herrera [Tue, 30 Nov 2021 17:29:31 +0000 (14:29 -0300)]
Increase size of shared memory for pg_commit_ts

Like 5364b357fb11 did for pg_commit, change the formula used to
determine number of pg_commit_ts buffers, which helps performance with
larger servers.

Discussion: https://postgr.es/m/20210115220744.GA24457@alvherre.pgsql
Reviewed-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Tomas Vondra <tomas.vondra@enterprisedb.com>
3 years agoDisable unused-variable warning C4101 in MSVC
Daniel Gustafsson [Tue, 30 Nov 2021 13:02:37 +0000 (14:02 +0100)]
Disable unused-variable warning C4101 in MSVC

The C4101 warning for unused variable cannot be individually suppressed
with PG_USED_FOR_ASSERTS_ONLY, and thus cause false-positive warnings
for variables which are defined but only read/written in an assertion.
Until a satisfactory solution for per-variable suppression like how we
do for gcc and clang, disable the warning.

Discussion: https://postgr.es/m/CAJcOf-c+KniGAp31pn8TC=9a-WHXpkX-3+8-2BkaCsZchhu=8w@mail.gmail.com

3 years agoRemove PF_USED_FOR_ASSERTS_ONLY from variables in general use
Daniel Gustafsson [Tue, 30 Nov 2021 13:02:14 +0000 (14:02 +0100)]
Remove PF_USED_FOR_ASSERTS_ONLY from variables in general use

fsstate in process_pending_requests (in postgres_fdw.c) was added in
8998e3cafa2 as an assertion-only variable,  1ec7fca8592 stated using
the variable outside of assertions.

rd_index in get_index_column_opclass (in lsyscache.c) was introduced
in 2a6368343ff, and then promptly used in the fix commit 7e041603904
shortly thereafter.

This removes the PG_USED_FOR_ASSERTS_ONLY variable decoration from
the above mentioned variables.

Reviewed-by: Greg Nancarrow <gregn4422@gmail.com>
Discussion: https://postgr.es/m/F959106C-0F21-43A5-B2AE-D007D51ACBEE@yesql.se

3 years agoUse test-specific temp path for keys during SSL test
Daniel Gustafsson [Tue, 30 Nov 2021 10:21:27 +0000 (11:21 +0100)]
Use test-specific temp path for keys during SSL test

The SSL and SCRAM TAP test suites both use temporary copies of the
supplied test keys in order to ensure correct permissions.  These
were however copied inside the tree using temporary filenames rather
than a true temporary folder.  Fix by using tmp_check supplied by
PostgreSQL::Test::Utils. Spotted by Tom Lane during review of the
nearby sslinfo TAP test patch.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/599244.1638041239@sss.pgh.pa.us

3 years agoAdd TAP tests for contrib/sslinfo
Daniel Gustafsson [Tue, 30 Nov 2021 10:19:59 +0000 (11:19 +0100)]
Add TAP tests for contrib/sslinfo

This adds rudimentary coverage of the sslinfo extension into the SSL
test harness.  The output is validated by comparing with pg_stat_ssl
to provide some level of test stability should the underlying certs
be slightly altered.  A new cert is added to provide an extension to
test against.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andrew Dunstan <andrew@dunslane.net>
Reviewed-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Discussion: https://postgr.es/m/E23F9811-0C77-45DA-912F-D809AB140741@yesql.se

3 years agoExtend configure_test_server_for_ssl to add extensions
Daniel Gustafsson [Tue, 30 Nov 2021 10:13:26 +0000 (11:13 +0100)]
Extend configure_test_server_for_ssl to add extensions

In order to be able to test extensions with SSL connections, allow
configure_test_server_for_ssl to create any extensions passed as
an array. Each extension is created in all the test databases.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andrew Dunstan <andrew@dunslane.net>
Reviewed-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Discussion: https://postgr.es/m/E23F9811-0C77-45DA-912F-D809AB140741@yesql.se

3 years agoFix flags of some GUCs and improve some descriptions
Michael Paquier [Tue, 30 Nov 2021 05:38:49 +0000 (14:38 +0900)]
Fix flags of some GUCs and improve some descriptions

This commit fixes some issues with GUCs:
- enable_incremental_sort was not marked as GUC_EXPLAIN, causing it to
not be listed in the output of EXPLAIN (SETTINGS) if using a value
different than the default, contrary to the other planner-level GUCs.
- trace_recovery_messages missed GUC_NOT_IN_SAMPLE, like the other
developer options.
- ssl_renegotiation_limit should be marked as COMPAT_OPTIONS_PREVIOUS.

While on it, this fixes one incorrect comment related to
autovacuum_freeze_max_age, and improves the descriptions of some other
GUCs, recently introduced.

Extracted from a larger patch set by the same author.

Author: Justin Pryzby
Description: https://postgr.es/m/20211129030833.GJ17618@telsasoft.com

3 years agoAdd a view to show the stats of subscription workers.
Amit Kapila [Tue, 30 Nov 2021 03:24:30 +0000 (08:54 +0530)]
Add a view to show the stats of subscription workers.

This commit adds a new system view pg_stat_subscription_workers, that
shows information about any errors which occur during the application of
logical replication changes as well as during performing initial table
synchronization. The subscription statistics entries are removed when the
corresponding subscription is removed.

It also adds an SQL function pg_stat_reset_subscription_worker() to reset
single subscription errors.

The contents of this view can be used by an upcoming patch that skips the
particular transaction that conflicts with the existing data on the
subscriber.

This view can be extended in the future to track other xact related
statistics like the number of xacts committed/aborted for subscription
workers.

Author: Masahiko Sawada
Reviewed-by: Greg Nancarrow, Hou Zhijie, Tang Haiying, Vignesh C, Dilip Kumar, Takamichi Osumi, Amit Kapila
Discussion: https://postgr.es/m/CAD21AoDeScrsHhLyEPYqN3sydg6PxAPVBboK=30xJfUVihNZDA@mail.gmail.com

3 years agoFix typos
Michael Paquier [Tue, 30 Nov 2021 02:05:15 +0000 (11:05 +0900)]
Fix typos

Author: Lingjie Qiang
Discussion: https://postgr.es/m/OSAPR01MB71654E773F62AC88DC1FC8CC80669@OSAPR01MB7165.jpnprd01.prod.outlook.com

3 years agoFix compatibility thinko for fstat() on standard streams in win32stat.c
Michael Paquier [Tue, 30 Nov 2021 00:55:50 +0000 (09:55 +0900)]
Fix compatibility thinko for fstat() on standard streams in win32stat.c

GetFinalPathNameByHandleA() cannot be used in compilation environments
where _WIN32_WINNT < 0x0600, meaning at least Windows XP used by some
buildfarm members under MinGW that Postgres still needs to support.
This was reported as a compilation warning by the buildfarm, but this is
actually worse than the report as the code would have not worked.

Instead, this switches to GetFileInformationByHandle() that is able to
fail for standard streams and succeed for redirected ones, which is what
we are looking for herein the code emulating fstat().  We also know that
it is able to work in all the environments still supported, thanks to
the existing logic of win32stat.c.

Issue introduced by 10260c7, so backpatch down to 14.

Reported-by: Justin Pryzby, via buildfarm member jacana
Author: Michael Paquier
Reviewed-by: Juan José Santamaría Flecha
Discussion: https://postgr.es/m/20211129050122.GK17618@telsasoft.com
Backpatch-through: 14

3 years agovacuumlazy.c: Rename dead_tuples to dead_items.
Peter Geoghegan [Mon, 29 Nov 2021 17:58:01 +0000 (09:58 -0800)]
vacuumlazy.c: Rename dead_tuples to dead_items.

Commit 8523492d simplified what it meant for an item to be considered
"dead" to VACUUM: TIDs collected in memory (in preparation for index
vacuuming) must always come from LP_DEAD stub line pointers in heap
pages, found following pruning.  This formalized the idea that index
vacuuming (and heap vacuuming) are optional processes.  Unlike pruning,
they can be delayed indefinitely, without any risk of that violating
fundamental invariants.  For example, leaving LP_DEAD items behind
clearly won't add to the risk of transaction ID wraparound.  You can't
have transaction ID wraparound without transaction IDs.  Renaming
anything that references DEAD tuples (tuples with storage) reinforces
all this.

Code outside vacuumlazy.c continues to fudge the distinction between
dead/deleted tuples, and LP_DEAD items.  This is necessary because
autovacuum scheduling is still mostly driven by "dead items/tuples"
statistics.  In the future we may find it useful to replace this model
with something more sophisticated, as a step towards teaching autovacuum
to perform more frequent vacuuming that targeting individual indexes
that happen to be more prone to becoming bloated through version churn.

In passing, simplify some function signatures that deal with VACUUM's
dead_items array.

Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAH2-WzktGBg4si6DEdmq3q6SoXSDqNi6MtmB8CmmTmvhsxDTLA@mail.gmail.com

3 years agoDoc: improve documentation about ORDER BY in matviews.
Tom Lane [Mon, 29 Nov 2021 17:13:12 +0000 (12:13 -0500)]
Doc: improve documentation about ORDER BY in matviews.

Remove the confusing use of ORDER BY in an example materialized
view.  It adds nothing to the example, but might encourage
people to follow bad practice.  Clarify REFRESH MATERIALIZED
VIEW's note about whether view ordering is retained (it isn't).

Maciek Sakrejda

Discussion: https://postgr.es/m/CAOtHd0D-OvrUU0C=4hX28p4BaSE1XL78BAQ0VcDaLLt8tdUzsg@mail.gmail.com

3 years agoSimplify declaring variables exported from libpgcommon and libpgport.
Tom Lane [Mon, 29 Nov 2021 16:00:00 +0000 (11:00 -0500)]
Simplify declaring variables exported from libpgcommon and libpgport.

This reverts commits c2d1eea9e and 11b500072, as well as similar hacks
elsewhere, in favor of setting up the PGDLLIMPORT macro so that it can
just be used unconditionally.  That can work because in frontend code,
we need no marking in either the defining or consuming files for a
variable exported from these libraries; and frontend code has no need
to access variables exported from the core backend, either.

While at it, write some actual documentation about the PGDLLIMPORT
and PGDLLEXPORT macros.

Patch by me, based on a suggestion from Robert Haas.

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

3 years agoPortability hack for pg_global_prng_state.
Tom Lane [Mon, 29 Nov 2021 05:04:45 +0000 (00:04 -0500)]
Portability hack for pg_global_prng_state.

PGDLLIMPORT is only appropriate for variables declared in the backend,
not when the variable is coming from a library included in frontend code.
(This isn't a particularly nice fix, but for now, use the same method
employed elsewhere.)

Discussion: https://postgr.es/m/E1mrWUD-000235-Hq@gemulon.postgresql.org

3 years agoCentralize timestamp computation of control file on updates
Michael Paquier [Mon, 29 Nov 2021 04:36:13 +0000 (13:36 +0900)]
Centralize timestamp computation of control file on updates

This commit moves the timestamp computation of the control file within
the routine of src/common/ in charge of updating the backend's control
file, which is shared by multiple frontend tools (pg_rewind,
pg_checksums and pg_resetwal) and the backend itself.

This change has as direct effect to update the control file's timestamp
when writing the control file in pg_rewind and pg_checksums, something
that is helpful to keep track of control file updates for those
operations, something also tracked by the backend at startup within its
logs.  This part is arguably a bug, as ControlFileData->time should be
updated each time a new version of the control file is written, but this
is a behavior change so no backpatch is done.

Author: Amul Sul
Reviewed-by: Nathan Bossart, Michael Paquier, Bharath Rupireddy
Discussion: https://postgr.es/m/CAAJ_b97nd_ghRpyFV9Djf9RLXkoTbOUqnocq11WGq9TisX09Fw@mail.gmail.com

3 years agoReplace random(), pg_erand48(), etc with a better PRNG API and algorithm.
Tom Lane [Mon, 29 Nov 2021 02:32:36 +0000 (21:32 -0500)]
Replace random(), pg_erand48(), etc with a better PRNG API and algorithm.

Standardize on xoroshiro128** as our basic PRNG algorithm, eliminating
a bunch of platform dependencies as well as fundamentally-obsolete PRNG
code.  In addition, this API replacement will ease replacing the
algorithm again in future, should that become necessary.

xoroshiro128** is a few percent slower than the drand48 family,
but it can produce full-width 64-bit random values not only 48-bit,
and it should be much more trustworthy.  It's likely to be noticeably
faster than the platform's random(), depending on which platform you
are thinking about; and we can have non-global state vectors easily,
unlike with random().  It is not cryptographically strong, but neither
are the functions it replaces.

Fabien Coelho, reviewed by Dean Rasheed, Aleksander Alekseev, and myself

Discussion: https://postgr.es/m/alpine.DEB.2.22.394.2105241211230.165418@pseudo

3 years agoImprove psql tab completion for views, FDWs, sequences and transforms
Michael Paquier [Mon, 29 Nov 2021 01:28:29 +0000 (10:28 +0900)]
Improve psql tab completion for views, FDWs, sequences and transforms

The following improvements are done:
- Addition of type completion for ALTER SEQUENCE AS.
- Ignore ALTER for transforms, as the command is not supported.
- Addition of more completion for ALTER FOREIGN DATA WRAPPER.
- Addition of options related to columns in ALTER VIEW.

This is a continuation of the work done in 0cd6d3b.

Author: Ken Kato
Discussion: https://postgr.es/m/9497ae9ca1b31eb9b1e97aded1c2ab07@oss.nttdata.com

3 years agovacuumlazy.c: prefer the term "cleanup lock".
Peter Geoghegan [Sun, 28 Nov 2021 00:05:01 +0000 (16:05 -0800)]
vacuumlazy.c: prefer the term "cleanup lock".

The term "super-exclusive lock" is an acceptable synonym of "cleanup
lock".  Even still, switching from one term to the other in the same
file is confusing.  Standardize on "cleanup lock" within vacuumlazy.c.

Per a complaint from Andres Freund.

3 years agoUpdate high level vacuumlazy.c comments.
Peter Geoghegan [Sat, 27 Nov 2021 22:29:43 +0000 (14:29 -0800)]
Update high level vacuumlazy.c comments.

Update vacuumlazy.c file header comments (as well as comments above the
lazy_scan_heap function) that were largely written before the
introduction of the HOT optimization, when lazy_scan_heap did far less,
and didn't actually prune during its initial heap pass.

Since lazy_scan_heap now outsources far more work to lower level
functions, it makes sense to introduce the function by talking about the
high level invariant that dictates the order in which each phase takes
place.  Also deemphasize the case where we run out of memory for TIDs,
since delaying that discussion makes it easier to talk about issues of
central importance.

Finally, remove discussion of parallel VACUUM from header comments.
These don't add much, and are in the wrong place.

3 years agoHarden be-gssapi-common.h for headerscheck
Alvaro Herrera [Fri, 26 Nov 2021 20:00:29 +0000 (17:00 -0300)]
Harden be-gssapi-common.h for headerscheck

Surround the contents with a test that the feature is enabled by
configure, to silence header checking tools on systems without GSSAPI
installed.

Backpatch to 12, where the file appeared.

Discussion: https://postgr.es/m/202111161709.u3pbx5lxdimt@alvherre.pgsql

3 years agoGo back to considering HOT on pages marked full.
Peter Geoghegan [Fri, 26 Nov 2021 18:58:38 +0000 (10:58 -0800)]
Go back to considering HOT on pages marked full.

Commit 2fd8685e7f simplified the checking of modified attributes that
takes place within heap_update().  This included a micro-optimization
affecting pages marked PD_PAGE_FULL: don't even try to use HOT to save a
few cycles on determining HOT safety.  The assumption was that it won't
work out this time around, since it can't have worked out last time
around.

Remove the micro-optimization.  It could only ever save cycles that are
consumed by the vast majority of heap_update() calls, which hardly seems
worth the added complexity.  It also seems quite possible that there are
workloads that will do worse over time by repeated application of the
micro-optimization, despite saving some cycles on average, in the short
term.

Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/CAH2-WznU1L3+DMPr1F7o2eJBT7=3bAJoY6ZkWABAxNt+-afyTA@mail.gmail.com

3 years agoCopy-edit vacuuumdb --analyze-in-stages doc blurb
Alvaro Herrera [Fri, 26 Nov 2021 17:42:15 +0000 (14:42 -0300)]
Copy-edit vacuuumdb --analyze-in-stages doc blurb

I had made a few typos, and Nikolai Berkoff made a wording change
suggestion.

Discussion: https://postgr.es/m/VMwe7-sGegrQPQ7fJjSCdsEbESKeJFOb6G4DFxxNrf45I7DzHio7sNUH88wWRMnAy5a5G0-FB31dxPM47ldigW6WdiCPncHgqO9bNl6F240=@pm.me

3 years agoDocument units for max_slot_wal_keep_size
Alvaro Herrera [Fri, 26 Nov 2021 17:31:57 +0000 (14:31 -0300)]
Document units for max_slot_wal_keep_size

The doc blurb failed to mention units, as well as lacking the point
about changeability.

Backpatch to 13.

Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reported by: b1000101@pm.me
Discussion: https://postgr.es/m/163760291192.26193.10801700492025355788@wrigleys.postgresql.org

3 years agoFix determination of broken LSN in OVERWRITTEN_CONTRECORD
Alvaro Herrera [Fri, 26 Nov 2021 14:14:27 +0000 (11:14 -0300)]
Fix determination of broken LSN in OVERWRITTEN_CONTRECORD

In commit ff9f111bce24 I mixed up inconsistent definitions of the LSN of
the first record in a page, when the previous record ends exactly at the
page boundary.  The correct LSN is adjusted to skip the WAL page header;
I failed to use that when setting XLogReaderState->overwrittenRecPtr,
so at WAL replay time VerifyOverwriteContrecord would refuse to let
replay continue past that record.

Backpatch to 10.  9.6 also contains this bug, but it's no longer being
maintained.

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

3 years agoAdd test for REVOKE ADMIN OPTION
Daniel Gustafsson [Fri, 26 Nov 2021 13:02:14 +0000 (14:02 +0100)]
Add test for REVOKE ADMIN OPTION

The REVOKE ADMIN OPTION FOR <role_name> syntax didn't have ample
test coverage. Fix by adding coverage in the privileges test suite.

Author: Mark Dilger <mark.dilger@enterprisedb.com>
Discussion: https://postgr.es/m/333B0203-D19B-4335-AE64-90EB0FAF46F0@enterprisedb.com

3 years agoFix GRANTED BY support in REVOKE ROLE statements
Daniel Gustafsson [Fri, 26 Nov 2021 13:02:01 +0000 (14:02 +0100)]
Fix GRANTED BY support in REVOKE ROLE statements

Commit 6aaaa76bb added support for the GRANTED BY clause in GRANT and
REVOKE statements, but missed adding support for checking the role in
the REVOKE ROLE case. Fix by checking that the parsed role matches the
CURRENT_ROLE/CURRENT_USER requirement, and also add some tests for it.
Backpatch to v14 where GRANTED BY support was introduced.

Discussion: https://postgr.es/m/B7F6699A-A984-4943-B9BF-CEB84C003527@yesql.se
Backpatch-through: 14

3 years agoUpdate comments
Peter Eisentraut [Fri, 26 Nov 2021 08:57:23 +0000 (09:57 +0100)]
Update comments

Various places wanted to point out that tuple descriptors don't
contain the variable-length fields of pg_attribute.  This started when
attacl was added, but more fields have been added since, and these
comments haven't been kept up to date consistently.  Reword so that
the purpose is clearer and we don't have to keep updating them.

3 years agoRemove unneeded Python includes
Peter Eisentraut [Thu, 25 Nov 2021 13:19:22 +0000 (14:19 +0100)]
Remove unneeded Python includes

Inluding <compile.h> and <eval.h> has not been necessary since Python
2.4, since they are included via <Python.h>.  Morever, <eval.h> is
being removed in Python 3.11.  So remove these includes.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/84884.1637723223%40sss.pgh.pa.us

3 years agoBlock ALTER TABLE .. DROP NOT NULL on columns in replica identity index
Michael Paquier [Thu, 25 Nov 2021 06:04:56 +0000 (15:04 +0900)]
Block ALTER TABLE .. DROP NOT NULL on columns in replica identity index

Replica identities that depend directly on an index rely on a set of
properties, one of them being that all the columns defined in this index
have to be marked as NOT NULL.  There was a hole in the logic with ALTER
TABLE DROP NOT NULL, where it was possible to remove the NOT NULL
property of a column part of an index used as replica identity, so block
it to avoid problems with logical decoding down the road.

The same check was already done columns part of a primary key, so the
fix is straight-forward.

Author: Haiying Tang, Hou Zhijie
Reviewed-by: Dilip Kumar, Michael Paquier
Discussion: https://postgr.es/m/OS0PR01MB6113338C102BEE8B2FFC5BD9FB619@OS0PR01MB6113.jpnprd01.prod.outlook.com
Backpatch-through: 10

3 years agoFix fstat() emulation on Windows with standard streams
Michael Paquier [Thu, 25 Nov 2021 03:16:21 +0000 (12:16 +0900)]
Fix fstat() emulation on Windows with standard streams

The emulation of fstat() in win32stat.c caused two issues with the
existing in-core callers, failing on EINVAL when using a stream as
argument:
- psql's \copy would crash when using a stream.
- pg_recvlogical would fail with -f -.

The tests in copyselect.sql from the main test suite covers the first
case, and there is a TAP test for the second case.  However, in both
cases, as the standard streams are always redirected, automated tests
did not notice those issues, requiring a terminal on Windows to be
reproducible.

This issue has been introduced in bed9075, and the origin of the problem
is that GetFileInformationByHandle() does not work directly on streams,
so this commit adds an extra code path to emulate and return a set of
stats that match best with the reality.  Note that redirected streams
rely on handles that can be queried with GetFileInformationByHandle(),
but we can rely on GetFinalPathNameByHandleA() to detect this case.

Author: Dmitry Koval, Juan José Santamaría Flecha
Discussion: https://postgr.es/m/17288-6b58a91025a8a8a3@postgresql.org
Backpatch-through: 14

3 years agoReplace straggling uses of ReadRecPtr/EndRecPtr.
Andres Freund [Thu, 25 Nov 2021 00:54:11 +0000 (16:54 -0800)]
Replace straggling uses of ReadRecPtr/EndRecPtr.

d2ddfa681db removed ReadRecPtr/EndRecPtr, but two uses within an #ifdef
WAL_DEBUG escaped.

Discussion: https://postgr.es/m/20211124231206.gbadj5bblcljb6d5@alap3.anarazel.de

3 years agoDoc: improve documentation about nextval()/setval().
Tom Lane [Wed, 24 Nov 2021 18:37:11 +0000 (13:37 -0500)]
Doc: improve documentation about nextval()/setval().

Clarify that the results of nextval and setval are not guaranteed
persistent until the calling transaction commits.  Some people
seem to have drawn the opposite conclusion from the statement that
these functions are never rolled back, so re-word to avoid saying
it quite that way.

Discussion: https://postgr.es/m/CAKU4AWohO=NfM-4KiZWvdc+z3c1C9FrUBR6xnReFJ6sfy0i=Lw@mail.gmail.com

3 years agoFix missing space in docs.
Heikki Linnakangas [Wed, 24 Nov 2021 16:32:56 +0000 (18:32 +0200)]
Fix missing space in docs.

Author: Japin Li
Discussion: https://www.postgresql.org/message-id/MEYP282MB1669C36E5F733C2EFBDCB80BB6619@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM

3 years agoxlog.c: Remove global variables ReadRecPtr and EndRecPtr.
Robert Haas [Wed, 24 Nov 2021 16:27:39 +0000 (11:27 -0500)]
xlog.c: Remove global variables ReadRecPtr and EndRecPtr.

In most places, the variables necessarily store the same value as the
eponymous members of the XLogReaderState that we use during WAL
replay, because ReadRecord() assigns the values from the structure
members to the global variables just after XLogReadRecord() returns.
However, XLogBeginRead() adjusts the structure members but not the
global variables, so after XLogBeginRead() and before the completion
of XLogReadRecord() the values can differ. Otherwise, they must be
identical.  According to my analysis, the only place where either
variable is referenced at a point where it might not have the same
value as the structure member is the refrence to EndRecPtr within
XLogPageRead.

Therefore, at every other place where we are using the global
variable, we can just switch to using the structure member instead,
and remove the global variable. However, we can, and in fact should,
do this in XLogPageRead() as well, because at that point in the code,
the global variable will actually store the start of the record we
want to read - either because it's where the last WAL record ended, or
because the read position has been changed using XLogBeginRead since
the last record was read. The structure member, on the other hand,
will already have been updated to point to the end of the record we
just read. Elsewhere, the latter is what we use as an argument to
emode_for_corrupt_record(), so we should do the same here.

This part of the patch is perhaps a bug fix, but I don't think it has
any important consequences, so no back-patch. The point here is just
to continue to whittle down the entirely excessive use of global
variables in xlog.c.

Discussion: http://postgr.es/m/CA+Tgmoao96EuNeSPd+hspRKcsCddu=b1h-QNRuKfY8VmfNQdfg@mail.gmail.com

3 years agoFix corner-case failure to detect improper timeline switch.
Robert Haas [Wed, 24 Nov 2021 13:13:10 +0000 (08:13 -0500)]
Fix corner-case failure to detect improper timeline switch.

rescanLatestTimeLine() contains a guard against switching to
a timeline that forked off from the current one prior to the
current recovery point, but that guard does not work if the
timeline switch occurs before the first WAL recod (which must
be the checkpoint record) is read. Without this patch, an
improper timeline switch is therefore possible in such cases.

This happens because rescanLatestTimeLine() relies on the global
variable EndRecPtr to understand the current position of WAL
replay. However, EndRecPtr at this point in the code contains
the endpoint of the last-replayed record, not the startpoint or
endpoint of the record being replayed now. Thus, before any
records have been replayed, it's zero, which causes the sanity
check to always pass.

To fix, pass down the correct timeline explicitly. The
EndRecPtr value we want is the one from the xlogreader, which
will be the starting position of the record we're about to
try to read, rather than the global variable, which is the
ending position of the last record we successfully read.
They're usually the same, but not in the corner case described
here.

No back-patch, because in v14 and earlier branhes, we were using
the wrong TLI here as well as the wrong LSN. In master, that was
fixed by commit 4a92a1c3d1c361ffb031ed05bf65b801241d7cdd, but
that and it's prerequisite patches are too invasive to
back-patch for such a minor issue.

Patch by me, reviewed by Amul Sul.

Discussion: http://postgr.es/m/CA+Tgmoao96EuNeSPd+hspRKcsCddu=b1h-QNRuKfY8VmfNQdfg@mail.gmail.com

3 years agoRemove useless LZ4 system call on failure when writing file header
Michael Paquier [Wed, 24 Nov 2021 11:12:54 +0000 (20:12 +0900)]
Remove useless LZ4 system call on failure when writing file header

If an error occurs when writing the LZ4 file header, LZ4F_compressEnd()
was called in the error code path of write(), followed by
LZ4F_freeCompressionContext() to finish the cleanup.  The code as-is was
not broken, but the LZ4F_compressEnd() proves to not be necessary as
there are no contents to flush at this stage, so remove it.

Per gripe from Jeevan Ladhe and Robert Haas.

Discussion: https://postgr.es/m/CAOgcT0PE33wbD7giAT1OSkNJt=p-vu8huq++qh=ny9O=SCP5aA@mail.gmail.com

3 years agoFlush Memoize cache when non-key parameters change, take 2
David Rowley [Wed, 24 Nov 2021 10:29:14 +0000 (23:29 +1300)]
Flush Memoize cache when non-key parameters change, take 2

It's possible that a subplan below a Memoize node contains a parameter
from above the Memoize node.  If this parameter changes then cache entries
may become out-dated due to the new parameter value.

Previously Memoize was mistakenly not aware of this.  We fix this here by
flushing the cache whenever a parameter that's not part of the cache
key changes.

Bug: #17213
Reported by: Elvis Pranskevichus
Author: David Rowley
Discussion: https://postgr.es/m/17213-988ed34b225a2862@postgresql.org
Backpatch-through: 14, where Memoize was added

3 years agoFix incorrect format placeholders
Peter Eisentraut [Wed, 24 Nov 2021 07:15:17 +0000 (08:15 +0100)]
Fix incorrect format placeholders

Also choose better types for the underlying variables to make this
more consistent.

3 years agoAdd support for Visual Studio 2022 in build scripts
Michael Paquier [Wed, 24 Nov 2021 04:03:23 +0000 (13:03 +0900)]
Add support for Visual Studio 2022 in build scripts

Documentation and any code paths related to VS are updated to keep the
whole consistent.  Similarly to 2017 and 2019, the version of VS and the
version of nmake that we use to determine which code paths to use for
the build are still inconsistent in their own way.

Backpatch down to 10, so as buildfarm members are able to use this new
version of Visual Studio on all the stable branches supported.

Author: Hans Buschmann
Discussion: https://postgr.es/m/1633101364685.39218@nidsa.net
Backpatch-through: 10

3 years agoRename SnapBuild* macros in slot.c.
Amit Kapila [Wed, 24 Nov 2021 02:36:55 +0000 (08:06 +0530)]
Rename SnapBuild* macros in slot.c.

Same macro names for SnapBuildOnDiskNotChecksummedSize and
SnapBuildOnDiskChecksummedSize are being used in slot.c and snapbuild.c.
This patch renames them, in slot.c, to
ReplicationSlotOnDiskNotChecksummedSize and
ReplicationSlotOnDiskChecksummedSize similar to the other macros. This
makes all macro names look consistent in slot.c.

Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACVZo-piDGzBOJRY4ob=_goFR6t9DhZMDMjJWN7LQs34Aw@mail.gmail.com

3 years agoRevert "Flush Memoize cache when non-key parameters change"
David Rowley [Wed, 24 Nov 2021 02:27:43 +0000 (15:27 +1300)]
Revert "Flush Memoize cache when non-key parameters change"

This reverts commit 1050048a315790a505465bfcceb26eaf8dbc7e2e.

3 years agoFlush Memoize cache when non-key parameters change
David Rowley [Wed, 24 Nov 2021 01:56:18 +0000 (14:56 +1300)]
Flush Memoize cache when non-key parameters change

It's possible that a subplan below a Memoize node contains a parameter
from above the Memoize node.  If this parameter changes then cache entries
may become out-dated due to the new parameter value.

Previously Memoize was mistakenly not aware of this.  We fix this here by
flushing the cache whenever a parameter that's not part of the cache
key changes.

Bug: #17213
Reported by: Elvis Pranskevichus
Author: David Rowley
Discussion: https://postgr.es/m/17213-988ed34b225a2862@postgresql.org
Backpatch-through: 14, where Memoize was added

3 years agoAllow Memoize to operate in binary comparison mode
David Rowley [Tue, 23 Nov 2021 21:06:59 +0000 (10:06 +1300)]
Allow Memoize to operate in binary comparison mode

Memoize would always use the hash equality operator for the cache key
types to determine if the current set of parameters were the same as some
previously cached set.  Certain types such as floating points where -0.0
and +0.0 differ in their binary representation but are classed as equal by
the hash equality operator may cause problems as unless the join uses the
same operator it's possible that whichever join operator is being used
would be able to distinguish the two values.  In which case we may
accidentally return in the incorrect rows out of the cache.

To fix this here we add a binary mode to Memoize to allow it to the
current set of parameters to previously cached values by comparing
bit-by-bit rather than logically using the hash equality operator.  This
binary mode is always used for LATERAL joins and it's used for normal
joins when any of the join operators are not hashable.

Reported-by: Tom Lane
Author: David Rowley
Discussion: https://postgr.es/m/3004308.1632952496@sss.pgh.pa.us
Backpatch-through: 14, where Memoize was added

3 years agoAdd SQL functions to monitor the directory contents of replication slots
Michael Paquier [Tue, 23 Nov 2021 10:29:42 +0000 (19:29 +0900)]
Add SQL functions to monitor the directory contents of replication slots

This commit adds a set of functions able to look at the contents of
various paths related to replication slots:
- pg_ls_logicalsnapdir, for pg_logical/snapshots/
- pg_ls_logicalmapdir, for pg_logical/mappings/
- pg_ls_replslotdir, for pg_replslot/<slot_name>/

These are intended to be used by monitoring tools.  Unlike pg_ls_dir(),
execution permission can be granted to non-superusers.  Roles members of
pg_monitor gain have access to those functions.

Bump catalog version.

Author: Bharath Rupireddy
Reviewed-by: Nathan Bossart, Justin Pryzby
Discussion: https://postgr.es/m/CALj2ACWsfizZjMN6bzzdxOk1ADQQeSw8HhEjhmVXn_Pu+7VzLw@mail.gmail.com

3 years agoAdjust pg_dump's priority ordering for casts.
Tom Lane [Mon, 22 Nov 2021 22:16:29 +0000 (17:16 -0500)]
Adjust pg_dump's priority ordering for casts.

When a stored expression depends on a user-defined cast, the backend
records the dependency as being on the cast's implementation function
--- or indeed, if there's no cast function involved but just
RelabelType or CoerceViaIO, no dependency is recorded at all.  This
is problematic for pg_dump, which is at risk of dumping things in the
wrong order leading to restore failures.  Given the lack of previous
reports, the risk isn't that high, but it can be demonstrated if the
cast is used in some view whose rowtype is then used as an input or
result type for some other function.  (That results in the view
getting hoisted into the functions portion of the dump, ahead of
the cast.)

A logically bulletproof fix for this would require including the
cast's OID in the parsed form of the expression, whence it could be
extracted by dependency.c, and then the stored dependency would force
pg_dump to do the right thing.  Such a change would be fairly invasive,
and certainly not back-patchable.  Moreover, since we'd prefer that
an expression using cast syntax be equal() to one doing the same
thing by explicit function call, the cast OID field would have to
have special ignored-by-comparisons semantics, making things messy.

So, let's instead fix this by a very simple hack in pg_dump: change
the object-type priority order so that casts are initially sorted
before functions, immediately after types.  This fixes the problem
in a fairly direct way for casts that have no implementation function.
For those that do, the implementation function will be hoisted to just
before the cast by the dependency sorting step, so that we still have
a valid dump order.  (I'm not sure that this provides a full guarantee
of no problems; but since it's been like this for many years without
any previous reports, this is probably enough to fix it in practice.)

Per report from Дмитрий Иванов.
Back-patch to all supported branches.

Discussion: https://postgr.es/m/CAPL5KHoGa3uvyKp6z6m48LwCnTsK+LRQ_mcA4uKGfqAVSEjV_A@mail.gmail.com

3 years agoPacify perlcritic.
Tom Lane [Mon, 22 Nov 2021 20:57:31 +0000 (15:57 -0500)]
Pacify perlcritic.

Per buildfarm.