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
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
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
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
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.
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
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
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
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().
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
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
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
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
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
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.
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
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.
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
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
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
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
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
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
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
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
Peter Eisentraut [Wed, 1 Dec 2021 10:25:58 +0000 (11:25 +0100)]
doc: Some additional information about when to use referential actions
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
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
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
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
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
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
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
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>
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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.
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
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
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
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
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
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
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
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.
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
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
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
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
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
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
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
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
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
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
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.
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
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
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.
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
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
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
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
Tom Lane [Mon, 22 Nov 2021 20:57:31 +0000 (15:57 -0500)]
Pacify perlcritic.
Per buildfarm.