Andres Freund [Sat, 26 Feb 2022 00:58:48 +0000 (16:58 -0800)]
Add further debug info to help debug 019_replslot_limit.pl failures.
See also
afdeff10526. Failures after that commit provided a few more hints,
but not yet enough to understand what's going on.
In 019_replslot_limit.pl shut down nodes with fast instead of immediate mode
if we observe the failure mode. That should tell us whether the failures we're
observing are just a timing issue under high load. PGCTLTIMEOUT should prevent
buildfarm animals from hanging endlessly.
Also adds a bit more logging to replication slot drop and ShutdownPostgres().
Discussion: https://postgr.es/m/
20220225192941.hqnvefgdzaro6gzg@alap3.anarazel.de
Tom Lane [Fri, 25 Feb 2022 22:40:21 +0000 (17:40 -0500)]
Disallow execution of SPI functions during plperl function compilation.
Perl can be convinced to execute user-defined code during compilation
of a plperl function (or at least a plperlu function). That's not
such a big problem as long as the activity is confined within the
Perl interpreter, and it's not clear we could do anything about that
anyway. However, if such code tries to use plperl's SPI functions,
we have a bigger problem. In the first place, those functions are
likely to crash because current_call_data->prodesc isn't set up yet.
In the second place, because it isn't set up, we lack critical info
such as whether the function is supposed to be read-only. And in
the third place, this path allows code execution during function
validation, which is strongly discouraged because of the potential
for security exploits. Hence, reject execution of the SPI functions
until compilation is finished.
While here, add check_spi_usage_allowed() calls to various functions
that hadn't gotten the memo about checking that. I think that perhaps
plperl_sv_to_literal may have been intentionally omitted on the grounds
that it was safe at the time; but if so, the addition of transforms
functionality changed that. The others are more recently added and
seem to be flat-out oversights.
Per report from Mark Murawski. Back-patch to all supported branches.
Discussion: https://postgr.es/m/
9acdf918-7fff-4f40-f750-
2ffa84f083d2@intellasoft.net
Andres Freund [Fri, 25 Feb 2022 18:30:05 +0000 (10:30 -0800)]
pg_waldump: Fix error message for WAL files smaller than XLOG_BLCKSZ.
When opening a WAL file smaller than XLOG_BLCKSZ (e.g. 0 bytes long) while
determining the wal_segment_size, pg_waldump checked errno, despite errno not
being set by the short read. Resulting in a bogus error message.
Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/
20220214.181847.
775024684568733277.horikyota.ntt@gmail.com
Backpatch: 11-, the bug was introducedin
fc49e24fa
Peter Geoghegan [Fri, 25 Feb 2022 03:01:54 +0000 (19:01 -0800)]
vacuumlazy.c: Remove obsolete num_tuples field.
Commit
49c9d9fc unified VACUUM VERBOSE and autovacuum logging. It
neglected to remove an old vacrel field that was only used by the old
VACUUM VERBOSE, so remove it now.
The previous num_tuples approach doesn't seem to have any real advantage
over the approach VACUUM VERBOSE takes now (also the approach used by
the autovacuum logging code), which is to show new_rel_tuples.
new_rel_tuples is the possibly-estimated total number of tuples left in
the table, whereas num_tuples meant the number of tuples encountered
during the VACUUM operation, after pruning, without regard for tuples
from pages skipped via the visibility map.
In passing, reorder a related vacrel field for consistency.
Amit Kapila [Fri, 25 Feb 2022 02:21:21 +0000 (07:51 +0530)]
Fix few values in pg_proc for pg_stat_get_replication_slot.
The function pg_stat_get_replication_slot() is not a SRF but marked
incorrectly in the pg_proc.
Reported-by: Michael Paquier
Discussion: https://postgr.es/m/YhMk4RjoMK3CCXy2@paquier.xyz
Peter Geoghegan [Fri, 25 Feb 2022 02:31:07 +0000 (18:31 -0800)]
Remove unnecessary heap_tuple_needs_freeze argument.
The buffer argument hasn't been used since the function was first added
by commit
bbb6e559c4. The sibling heap_prepare_freeze_tuple function
doesn't have such an argument either. Remove it.
Daniel Gustafsson [Thu, 24 Feb 2022 19:58:18 +0000 (20:58 +0100)]
Guard against reallocation failure in pg_regress
realloc() will return NULL on a failed reallocation, so the destination
pointer must be inspected to avoid null pointer dereference. Further,
assigning the return value to the source pointer leak the allocation in
the case of reallocation failure. Fix by using pg_realloc instead which
has full error handling.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/
9FC7E603-9246-4C62-B466-
A39CFAF454AE@yesql.se
Heikki Linnakangas [Thu, 24 Feb 2022 14:15:12 +0000 (16:15 +0200)]
Fix data loss on crash after sorted GiST index build.
If a checkpoint happens during sorted GiST index build, and the system
crashes after the checkpoint and after the index build has finished,
the data written to the index before the checkpoint started could be
lost. The checkpoint won't fsync it, and it won't be replayed at crash
recovery either. Fix by calling smgrimmedsync() after the index build,
just like in B-tree index build.
Backpatch to v14 where the sorted GiST index build was introduced.
Reported-by: Melanie Plageman
Discussion: https://www.postgresql.org/message-id/CAAKRu_ZJJynimxKj5xYBSziL62-iEtPE+fx-B=JzR=jUtP92mw@mail.gmail.com
Michael Paquier [Thu, 24 Feb 2022 07:54:59 +0000 (16:54 +0900)]
Simplify more checks related to set-returning functions
This makes more consistent the SRF-related checks in the area of
PL/pgSQL, PL/Perl, PL/Tcl, pageinspect and some of the JSON worker
functions, making it easier to grep for the same error patterns through
the code, reducing a bit the translation work.
It is worth noting that each_worker_jsonb()/each_worker() in jsonfuncs.c
and pageinspect's brin_page_items() were doing a check on expectedDesc
that is not required as they fetch their tuple descriptor directly from
get_call_result_type(). This looks like a set of copy-paste errors that
have spread over the years.
This commit is a continuation of the changes begun in
07daca5, for any
remaining code paths on sight. Like
fcc2817, this makes the code more
consistent, easing the integration of a larger patch that will refactor
the way tuplestores are created and checked in a good portion of the
set-returning functions present in core.
I have worked my way through the changes of this patch by myself, and
Ranier has proposed the same changes in a different thread in parallel,
though there were some inconsistencies related in expectedDesc in what
was proposed by him.
Author: Michael Paquier, Ranier Vilela
Discussion: https://postgr.es/m/CAAKRu_azyd1Z3W_r7Ou4sorTjRCs+PxeHw1CWJeXKofkE6TuZg@mail.gmail.com
Discussion: https://postgr.es/m/CAEudQApm=AFuJjEHLBjBcJbxcw4pBMwg2sHwXyCXYcbBOj3hpg@mail.gmail.com
Michael Paquier [Thu, 24 Feb 2022 07:11:34 +0000 (16:11 +0900)]
Clean up and simplify code in a couple of set-returning functions
The following set-returning functions have their logic simplified, to be
more consistent with other in-core areas:
- pg_prepared_statement()'s tuple descriptor is now created with
get_call_result_type() instead of being created from scratch, saving
from some duplication with pg_proc.dat.
- show_all_file_settings(), similarly, now uses get_call_result_type()
to build its tuple descriptor instead of creating it from scratch.
- pg_options_to_table() made use of a static routine called only once.
This commit removes this internal routine to make the function easier to
follow.
- pg_config() was using a unique logic style, doing checks on the tuple
descriptor passed down in expectedDesc, but it has no need to do so.
This switches the function to use a tuplestore with a tuple descriptor
retrieved from get_call_result_type(), instead.
This simplifies an upcoming patch aimed at refactoring the way
tuplestores are created and checked in set-returning functions, this
change making sense as its own independent cleanup by shaving some
code.
Author: Melanie Plageman, Michael Paquier
Reviewed-by: Justin Pryzby
Discussion: https://postgr.es/m/CAAKRu_azyd1Z3W_r7Ou4sorTjRCs+PxeHw1CWJeXKofkE6TuZg@mail.gmail.com
Etsuro Fujita [Thu, 24 Feb 2022 05:30:00 +0000 (14:30 +0900)]
postgres_fdw: Add support for parallel commit.
postgres_fdw commits remote (sub)transactions opened on remote server(s)
in a local (sub)transaction one by one when the local (sub)transaction
commits. This patch allows it to commit the remote (sub)transactions in
parallel to improve performance. This is enabled by the server option
"parallel_commit". The default is false.
Etsuro Fujita, reviewed by Fujii Masao and David Zhang.
Discussion: http://postgr.es/m/CAPmGK17dAZCXvwnfpr1eTfknTGdt%3DhYTV9405Gt5SqPOX8K84w%40mail.gmail.com
Amit Kapila [Thu, 24 Feb 2022 03:24:39 +0000 (08:54 +0530)]
Fix one of the tests introduced in commit
52e4f0cd47.
In the Publisher-Subscriber setup, after performing a DML operation on the
publisher, we need to wait for it to be replayed on the subscriber before
querying the same data on the subscriber. One of the tests missed the wait
step.
As per buildfarm.
Author: Peter Smith
Discussion: https://postgr.es/m/CAHut+Pv=e9Qd1TSYo8Og6x6Abfz3b9_htwinLp4ENPgV45DACQ@mail.gmail.com
Tom Lane [Wed, 23 Feb 2022 16:10:46 +0000 (11:10 -0500)]
Re-allow underscore as first character of custom GUC names.
Commit
3db826bd5 intended that valid_custom_variable_name's
rules for valid identifiers match those of scan.l. However,
I (tgl) had some kind of brain fade and put "_" in the wrong
list.
Fix by Japin Li, per bug #17415 from Daniel Polski.
Discussion: https://postgr.es/m/17415-
ebdb683d7e09a51c@postgresql.org
Daniel Gustafsson [Wed, 23 Feb 2022 13:24:43 +0000 (14:24 +0100)]
Quick exit on log stream child exit in pg_basebackup
If the log streaming child process (thread on Windows) dies during
backup then the whole backup will be aborted at the end of the
backup. Instead, trap ungraceful termination of the log streaming
child and exit early. This also adds a TAP test for simulating this
by terminating the responsible backend.
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Magnus Hagander <magnus@hagander.net>
Discussion: https://postgr.es/m/
0F69E282-97F9-4DB7-8D6D-
F927AA6340C8@yesql.se
Discussion: https://postgr.es/m/VI1PR83MB0189818B82C19059CB62E26199A89@VI1PR83MB0189.EURPRD83.prod.outlook.com
Daniel Gustafsson [Wed, 23 Feb 2022 13:23:50 +0000 (14:23 +0100)]
Remove duplicated word in comment
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/
B7C15416-BD61-4926-9843-
5C557BCD7007@yesql.se
Daniel Gustafsson [Wed, 23 Feb 2022 13:22:16 +0000 (14:22 +0100)]
Add function to pump IPC process until string match
Refactor the recovery tests to not carry a local duplicated copy of
the pump_until function which pumps a process until a defined string
is seen on a stream. This reduces duplication, and is in preparation
for another patch which will also use this functionality.
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion https://postgr.es/m/YgynUafCyIu3jIhC@paquier.xyz
Daniel Gustafsson [Wed, 23 Feb 2022 10:22:46 +0000 (11:22 +0100)]
Use test functions in pg_rewind test module
Commit
61081e75c introduced pg_rewind along with the test suite, which
ensured that subroutines didn't incur more than one test to plan. Now
that we no longer explicitly plan tests (since
549ec201d), we can use
the usual Test::More functions.
Reviewed-by: Andrew Dunstan <andrew@dunslane.net>
Discussion: https://postgr.es/m/
AA527525-F0CC-4AA2-AF98-
543CABFDAF59@yesql.se
Daniel Gustafsson [Wed, 23 Feb 2022 09:54:03 +0000 (10:54 +0100)]
Fix statenames in mergejoin comments
The names in the comments were on a few states not consistent with
the documented state.
Author: Zhihong Yu <zyu@yugabyte.com>
Discussion: https://postgr.es/m/CALNJ-vQVthfQXVqmrHR8BKHtC4fMGbhM1xbvJNJAPexTq_dH=w@mail.gmail.com
Andres Freund [Wed, 23 Feb 2022 02:02:34 +0000 (18:02 -0800)]
Add temporary debug info to help debug 019_replslot_limit.pl failures.
I have not been able to reproduce the occasional failures of
019_replslot_limit.pl we are seeing in the buildfarm and not for lack of
trying. The additional logging and increased log level will hopefully help.
Will be reverted once the cause is identified.
Discussion: https://postgr.es/m/
20220218231415.c4plkp4i3reqcwip@alap3.anarazel.de
Peter Eisentraut [Tue, 22 Feb 2022 09:08:11 +0000 (10:08 +0100)]
Put typtype letters back into consistent order
Amit Kapila [Tue, 22 Feb 2022 02:24:12 +0000 (07:54 +0530)]
Allow specifying row filters for logical replication of tables.
This feature adds row filtering for publication tables. When a publication
is defined or modified, an optional WHERE clause can be specified. Rows
that don't satisfy this WHERE clause will be filtered out. This allows a
set of tables to be partially replicated. The row filter is per table. A
new row filter can be added simply by specifying a WHERE clause after the
table name. The WHERE clause must be enclosed by parentheses.
The row filter WHERE clause for a table added to a publication that
publishes UPDATE and/or DELETE operations must contain only columns that
are covered by REPLICA IDENTITY. The row filter WHERE clause for a table
added to a publication that publishes INSERT can use any column. If the
row filter evaluates to NULL, it is regarded as "false". The WHERE clause
only allows simple expressions that don't have user-defined functions,
user-defined operators, user-defined types, user-defined collations,
non-immutable built-in functions, or references to system columns. These
restrictions could be addressed in the future.
If you choose to do the initial table synchronization, only data that
satisfies the row filters is copied to the subscriber. If the subscription
has several publications in which a table has been published with
different WHERE clauses, rows that satisfy ANY of the expressions will be
copied. If a subscriber is a pre-15 version, the initial table
synchronization won't use row filters even if they are defined in the
publisher.
The row filters are applied before publishing the changes. If the
subscription has several publications in which the same table has been
published with different filters (for the same publish operation), those
expressions get OR'ed together so that rows satisfying any of the
expressions will be replicated.
This means all the other filters become redundant if (a) one of the
publications have no filter at all, (b) one of the publications was
created using FOR ALL TABLES, (c) one of the publications was created
using FOR ALL TABLES IN SCHEMA and the table belongs to that same schema.
If your publication contains a partitioned table, the publication
parameter publish_via_partition_root determines if it uses the partition's
row filter (if the parameter is false, the default) or the root
partitioned table's row filter.
Psql commands \dRp+ and \d <table-name> will display any row filters.
Author: Hou Zhijie, Euler Taveira, Peter Smith, Ajin Cherian
Reviewed-by: Greg Nancarrow, Haiying Tang, Amit Kapila, Tomas Vondra, Dilip Kumar, Vignesh C, Alvaro Herrera, Andres Freund, Wei Wang
Discussion: https://www.postgresql.org/message-id/flat/CAHE3wggb715X%2BmK_DitLXF25B%3DjE6xyNCH4YOwM860JR7HarGQ%40mail.gmail.com
Michael Paquier [Tue, 22 Feb 2022 01:22:15 +0000 (10:22 +0900)]
Add compute_query_id = regress
"regress" is a new mode added to compute_query_id aimed at facilitating
regression testing when a module computing query IDs is loaded into the
backend, like pg_stat_statements. It works the same way as "auto",
meaning that query IDs are computed if a module enables it, except that
query IDs are hidden in EXPLAIN outputs to ensure regression output
stability.
Like any GUCs of the kind (force_parallel_mode, etc.), this new
configuration can be added to an instance's postgresql.conf, or just
passed down with PGOPTIONS at command level. compute_query_id uses an
enum for its set of option values, meaning that this addition ensures
ABI compatibility.
Using this new configuration mode allows installcheck-world to pass when
running the tests on an instance with pg_stat_statements enabled,
stabilizing the test output while checking the paths doing query ID
computations.
Reported-by: Anton Melnikov
Reviewed-by: Julien Rouhaud
Discussion: https://postgr.es/m/
1634283396.
372373993@f75.i.mail.ru
Discussion: https://postgr.es/m/YgHlxgc/OimuPYhH@paquier.xyz
Backpatch-through: 14
Tom Lane [Mon, 21 Feb 2022 19:10:15 +0000 (14:10 -0500)]
Disallow setting bogus GUCs within an extension's reserved namespace.
Commit
75d22069e tried to throw a warning for setting a custom GUC whose
prefix belongs to a previously-loaded extension, if there is no such GUC
defined by the extension. But that caused unstable behavior with
parallel workers, because workers don't necessarily load extensions and
GUCs in the same order their leader did. To make that work safely, we
have to completely disallow the case. We now actually remove any such
GUCs at the time of initial extension load, and then throw an error not
just a warning if you try to add one later. While this might create a
compatibility issue for a few people, the improvement in error-detection
capability seems worth it; it's hard to believe that there's any good
use-case for choosing such GUC names.
This also un-reverts
5609cc01c (Rename EmitWarningsOnPlaceholders() to
MarkGUCPrefixReserved()), since that function's old name is now even
more of a misnomer.
Florin Irion and Tom Lane
Discussion: https://postgr.es/m/
1902182.
1640711215@sss.pgh.pa.us
Andres Freund [Sat, 19 Feb 2022 20:42:37 +0000 (12:42 -0800)]
Assert in init_toast_snapshot() that some snapshot registered or active.
Commit <FIXME> fixed the bug that RemoveTempRelationsCallback() did not
push/register a snapshot. That only went unnoticed because often a valid
catalog snapshot exists and is returned by GetOldestSnapshot(). But due to
invalidation processing that is not reliable.
Thus assert in init_toast_snapshot() that there is a registered or active
snapshot, using the new HaveRegisteredOrActiveSnapshot().
Author: Andres Freund
Discussion: https://postgr.es/m/
20220219180002.6tubjq7iw7m52bgd@alap3.anarazel.de
Andres Freund [Mon, 21 Feb 2022 16:57:34 +0000 (08:57 -0800)]
Fix temporary object cleanup failing due to toast access without snapshot.
When cleaning up temporary objects during process exit the cleanup could fail
with:
FATAL: cannot fetch toast data without an active snapshot
The bug is caused by RemoveTempRelationsCallback() not setting up a
snapshot. If an object with toasted catalog data needs to be cleaned up,
init_toast_snapshot() could fail with the above error.
Most of the time however the the problem is masked due to cached catalog
snapshots being returned by GetOldestSnapshot(). But dropping an object can
cause catalog invalidations to be emitted. If no further catalog accesses are
necessary between the invalidation processing and the next toast datum
deletion, the bug becomes visible.
It's easy to miss this bug because it typically happens after clients
disconnect and the FATAL error just ends up in the log.
Luckily temporary table cleanup at the next use of the same temporary schema
or during DISCARD ALL does not have the same problem.
Fix the bug by pushing a snapshot in RemoveTempRelationsCallback(). Also add
isolation tests for temporary object cleanup, including objects with toasted
catalog data.
A future HEAD only commit will add an assertion trying to make this more
visible.
Reported-By: Miles Delahunty
Author: Andres Freund
Discussion: https://postgr.es/m/CAOFAq3BU5Mf2TTvu8D9n_ZOoFAeQswuzk7yziAb7xuw_qyw5gw@mail.gmail.com
Backpatch: 10-
Andres Freund [Mon, 21 Feb 2022 16:34:59 +0000 (08:34 -0800)]
pg_upgrade: Don't print progress status when output is not a tty.
Until this change pg_upgrade with output redirected to a file / pipe would end
up printing all files in the cluster. This has made check-world output
exceedingly verbose.
Author: Andres Freund <andres@anarazel.de>
Reviewed-By: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-By: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/CA+hUKGKjrV61ZVJ8OSag+3rKRmCZXPc03bDyWMqhXg3rdZ=fOw@mail.gmail.com
Peter Eisentraut [Mon, 21 Feb 2022 09:55:03 +0000 (10:55 +0100)]
pgcrypto: Remove unused error code
PXE_DEV_READ_ERROR hasn't been used since random device support was
removed from pgcrypto (
fe0a0b5993dfe24e4b3bcf52fa64ff41a444b8f1).
Peter Eisentraut [Mon, 21 Feb 2022 09:28:43 +0000 (10:28 +0100)]
pgcrypto: Remove unused error code
PXE_MCRYPT_INTERNAL was apparently never used even when it was added.
Peter Eisentraut [Mon, 21 Feb 2022 08:42:46 +0000 (09:42 +0100)]
Fix possible null pointer reference
Per Coverity. Introduced in
37851a8b83d3d57ca48736093b10aa5f3bc0c177.
Michael Paquier [Mon, 21 Feb 2022 00:55:55 +0000 (09:55 +0900)]
doc: Mention environment variable ZSTD in the TAP tests for MSVC
6c417bb has added the build infrastructure to support ZSTD, but forgot
to update this section of the docs to mention the variable ZSTD, as per
the change done in vcregress.pl.
While on it, reword this section of the docs to describe what happens in
the default case, as per a suggestion from Robert Haas.
Discussion: https://postgr.es/m/YhCL0fKnDv/Zvtuo@paquier.xyz
Andres Freund [Sun, 20 Feb 2022 21:51:36 +0000 (13:51 -0800)]
Fix meaning-changing typo introduced in
fa0e03c15a9f.
Tom Lane [Sun, 20 Feb 2022 20:02:41 +0000 (15:02 -0500)]
Reset conn->errorReported when PQrequestCancel sets errorMessage.
Oversight in commit
618c16707. This is mainly neatnik-ism, since
if PQrequestCancel is used per its API contract, we should perform
pqClearConnErrorState before reaching any place that would consult
errorReported. But still, it seems like a bad idea to potentially
leave errorReported pointing past errorMessage.len.
Andrew Dunstan [Sun, 20 Feb 2022 16:47:56 +0000 (11:47 -0500)]
Remove most msys special processing in TAP tests
Following migration of Windows buildfarm members running TAP tests to
use of ucrt64 perl for those tests, special processing for msys perl is
no longer necessary and so is removed.
Backpatch to release 10
Discussion: https://postgr.es/m/
c65a8781-77ac-ea95-d185-
6db291e1baeb@dunslane.net
Andrew Dunstan [Fri, 18 Feb 2022 22:00:03 +0000 (17:00 -0500)]
Remove PostgreSQL::Test::Utils::perl2host completely
Commit
f1ac4a74de disabled this processing, and as nothing has broken (as
expected) here we proceed to remove the routine and adjust all the call
sites.
Backpatch to release 10
Discussion: https://postgr.es/m/
0ba775a2-8aa0-0d56-d780-
69427cf6f33d@dunslane.net
Discussion: https://postgr.es/m/
20220125023609.5ohu3nslxgoygihl@alap3.anarazel.de
Andrew Dunstan [Fri, 18 Feb 2022 21:59:30 +0000 (16:59 -0500)]
Ensure the right perl is used for TAP tests on msys
In particular, perl with $Config{osname} = msys should only be used if
the build target is msys (which is currently buildable but not usable).
For builds targeted at native Windows, perl from the ucrt64 toolchain is
suitable.
Discussion: https://postgr.es/m/
20220216210141.5glt5isg5qtwty4c@alap3.anarazel.de
Heikki Linnakangas [Sun, 20 Feb 2022 16:33:09 +0000 (18:33 +0200)]
Fix uninitialized variable.
I'm very surprised the compiler didn't warn about it. But Coverity and
Valgrind did.
John Naylor [Sun, 20 Feb 2022 06:22:08 +0000 (13:22 +0700)]
Use bitwise rotate functions in more places
There were a number of places in the code that used bespoke bit-twiddling
expressions to do bitwise rotation. While we've had pg_rotate_right32()
for a while now, we hadn't gotten around to standardizing on that. Do so
now. Since many potential call sites look more natural with the "left"
equivalent, add that function too.
Reviewed by Tom Lane and Yugo Nagata
Discussion:
https://www.postgresql.org/message-id/CAFBsxsH7c1LC0CGZ0ADCBXLHU5-%3DKNXx-r7tHYPAW51b2HK4Qw%40mail.gmail.com
Michael Paquier [Sat, 19 Feb 2022 06:06:53 +0000 (15:06 +0900)]
doc: Simplify description of --with-lz4
LZ4 is used in much more areas of the system now than just WAL and table
data. This commit simplifies the installation documentation of Windows
and *nix by removing any details of the areas extended when building
with LZ4.
Author: Jeevan Ladhe
Discussion: https://postgr.es/m/CANm22Cgny8AF76pitomXp603NagwKXbA4dyN2Fac4yHPebqdqg@mail.gmail.com
Michael Paquier [Sat, 19 Feb 2022 05:58:51 +0000 (14:58 +0900)]
Fix inconsistencies in SRF checks of pg_config() and string_to_table()
The execution paths of those functions have been using a set of checks
inconsistent with any other SRF function:
- string_to_table() missed a check on expectedDesc, the tuple descriptor
expected by the caller, that should never be NULL. Introduced in
66f1630.
- pg_config() should check for a ReturnSetInfo, and expectedDesc cannot
be NULL. Its error messages were also inconsistent. Introduced in
a5c43b8.
Extracted from a larger patch by the same author, in preparation for a
larger patch set aimed at refactoring the way tuplestores are created
and checked in SRF functions.
Author: Melanie Plageman
Reviewed-by: Justin Pryzby
Discussion: https://postgr.es/m/CAAKRu_azyd1Z3W_r7Ou4sorTjRCs+PxeHw1CWJeXKofkE6TuZg@mail.gmail.com
Tom Lane [Fri, 18 Feb 2022 20:35:15 +0000 (15:35 -0500)]
Rearrange libpq's error reporting to avoid duplicated error text.
Since commit
ffa2e4670, libpq accumulates text in conn->errorMessage
across a whole query cycle. In some situations, we may report more
than one error event within a cycle: the easiest case to reach is
where we report a FATAL error message from the server, and then a
bit later we detect loss of connection. Since, historically, each
error PGresult bears the entire content of conn->errorMessage,
this results in duplication of the FATAL message in any output that
concatenates the contents of the PGresults.
Accumulation in errorMessage still seems like a good idea, especially
in view of the number of places that did ad-hoc error concatenation
before
ffa2e4670. So to fix this, let's track how much of
conn->errorMessage has been read out into error PGresults, and only
include new text in later PGresults. The tricky part of that is
to be sure that we never discard an error PGresult once made (else
we'd risk dropping some text, a problem much worse than duplication).
While libpq formerly did that in some code paths, a little bit of
rearrangement lets us postpone making an error PGresult at all until
we are about to return it.
A side benefit of that postponement is that it now becomes practical
to return a dummy static PGresult in cases where we hit out-of-memory
while trying to manufacture an error PGresult. This eliminates the
admittedly-very-rare case where we'd return NULL from PQgetResult,
indicating successful query completion, even though what actually
happened was an OOM failure.
Discussion: https://postgr.es/m/
ab4288f8-be5c-57fb-2400-
e3e857f53e46@enterprisedb.com
Robert Haas [Fri, 18 Feb 2022 18:40:31 +0000 (13:40 -0500)]
Add support for building with ZSTD.
This commit doesn't actually add anything that uses ZSTD; that will be
done separately. It just puts the basic infrastructure into place.
Jeevan Ladhe, Robert Haas, and Michael Paquier. Reviewed by Justin
Pryzby and Andres Freund.
Discussion: http://postgr.es/m/CA+TgmoatQKGd+8SjcV+bzvw4XaoEwminHjU83yG12+NXtQzTTQ@mail.gmail.com
Tom Lane [Fri, 18 Feb 2022 16:43:04 +0000 (11:43 -0500)]
Don't let libpq PGEVT_CONNRESET callbacks break a PGconn.
As currently implemented, failure of a PGEVT_CONNRESET callback
forces the PGconn into the CONNECTION_BAD state (without closing
the socket, which is inconsistent with other failure paths), and
prevents later callbacks from being called. This seems highly
questionable, and indeed is questioned by comments in the source.
Instead, let's just ignore the result value of PGEVT_CONNRESET
calls. Like the preceding commit, this converts event callbacks
into "pure observers" that cannot affect libpq's processing logic.
Discussion: https://postgr.es/m/
3185105.
1644960083@sss.pgh.pa.us
Tom Lane [Fri, 18 Feb 2022 16:37:27 +0000 (11:37 -0500)]
Don't let libpq "event" procs break the state of PGresult objects.
As currently implemented, failure of a PGEVT_RESULTCREATE callback
causes the PGresult to be converted to an error result. This is
intellectually inconsistent (shouldn't a failing callback likewise
prevent creation of the error result? what about side-effects on the
behavior seen by other event procs? why does PQfireResultCreateEvents
act differently from PQgetResult?), but more importantly it destroys
any promises we might wish to make about the behavior of libpq in
nontrivial operating modes, such as pipeline mode. For example,
it's not possible to promise that PGRES_PIPELINE_SYNC results will
be returned if an event callback fails on those. With this
definition, expecting applications to behave sanely in the face of
possibly-failing callbacks seems like a very big lift.
Hence, redefine the result of a callback failure as being simply
that that event procedure won't be called any more for this PGresult
(which was true already). Event procedures can still signal failure
back to the application through out-of-band mechanisms, for example
via their passthrough arguments.
Similarly, don't let failure of a PGEVT_RESULTCOPY callback prevent
PQcopyResult from succeeding. That definition allowed a misbehaving
event proc to break single-row mode (our sole internal use of
PQcopyResult), and it probably had equally deleterious effects for
outside uses.
Discussion: https://postgr.es/m/
3185105.
1644960083@sss.pgh.pa.us
Tom Lane [Fri, 18 Feb 2022 03:45:34 +0000 (22:45 -0500)]
Suppress warning about stack_base_ptr with late-model GCC.
GCC 12 complains that set_stack_base is storing the address of
a local variable in a long-lived pointer. This is an entirely
reasonable warning (indeed, it just helped us find a bug);
but that behavior is intentional here. We can work around it
by using __builtin_frame_address(0) instead of a specific local
variable; that produces an address a dozen or so bytes different,
in my testing, but we don't care about such a small difference.
Maybe someday a compiler lacking that function will start to issue
a similar warning, but we'll worry about that when it happens.
Patch by me, per a suggestion from Andres Freund. Back-patch to
v12, which is as far back as the patch will go without some pain.
(Recently-established project policy would permit a back-patch as
far as 9.2, but I'm disinclined to expend the work until GCC 12
is much more widespread.)
Discussion: https://postgr.es/m/
3773792.
1645141467@sss.pgh.pa.us
Fujii Masao [Fri, 18 Feb 2022 03:19:10 +0000 (12:19 +0900)]
Fix comment in CheckIndexCompatible().
Commit
5f173040 removed the parameter "heapRelation" from
CheckIndexCompatible(), but forgot to remove the mention of it
from the comment. This commit removes that unnecessary mention.
Also this commit adds the missing mention of the parameter "oldId"
in the comment.
Author: Yugo Nagata
Reviewed-by: Nathan Bossart, Fujii Masao
Discussion: https://postgr.es/m/
20220204014634.
b39314f278ff4ae3de96e201@sraoss.co.jp
Fujii Masao [Fri, 18 Feb 2022 02:38:12 +0000 (11:38 +0900)]
postgres_fdw: Make postgres_fdw.application_name support more escape sequences.
Commit
6e0cb3dec1 allowed postgres_fdw.application_name to include
escape sequences %a (application name), %d (database name), %u (user name)
and %p (pid). In addition to them, this commit makes it support
the escape sequences for session ID (%c) and cluster name (%C).
These are helpful to investigate where each remote transactions came from.
Author: Fujii Masao
Reviewed-by: Ryohei Takahashi, Kyotaro Horiguchi
Discussion: https://postgr.es/m/
1041dc9a-c976-049f-9f14-
e7d94c29c4b2@oss.nttdata.com
Amit Kapila [Fri, 18 Feb 2022 02:14:24 +0000 (07:44 +0530)]
Fix a comment in worker.c.
The comment incorrectly states that worker gets killed during
ALTER SUBSCRIPTION ... DISABLE. Remove that part of the comment.
Author: Masahiko Sawada
Discussion: https://postgr.es/m/CAD21AoCbEN==oH7BhP3U6WPHg3zgH6sDOeKhJjy4W2dx-qoVCw@mail.gmail.com
Tom Lane [Thu, 17 Feb 2022 20:03:40 +0000 (15:03 -0500)]
Avoid dangling-pointer usage in pg_basebackup progress reports.
Ill-considered refactoring in
23a1c6578 led to progress_filename
sometimes pointing to data that had gone out of scope. The most
bulletproof fix is to hang onto a copy of whatever's passed in.
Compared to the work spent elsewhere per file, that's not very
expensive, plus we can skip it except in verbose logging mode.
Per buildfarm.
Discussion: https://postgr.es/m/
20220212211316.GK31460@telsasoft.com
Robert Haas [Thu, 17 Feb 2022 15:53:51 +0000 (10:53 -0500)]
Add missing binary-upgrade guard.
Commit
9a974cbcba005256a19991203583a94b4f9a21a9 arranged for
pg_dumpall to preserve tablespace OIDs, but it should only do that
in binary upgrade mode, not all the time.
Reported by Christoph Berg.
Discussion: http://postgr.es/m/YgjwrkEvNEqoz4Vm@msg.df7cb.de
Andrew Dunstan [Thu, 17 Feb 2022 14:59:59 +0000 (09:59 -0500)]
Disable perl2host() processing in TAP tests
This is a preliminary step towards removing it altogether, but this lets
us double check that nothing breaks in the buildfarm before we do.
Discussion: https://postgr.es/m/
0ba775a2-8aa0-0d56-d780-
69427cf6f33d@dunslane.net
Andres Freund [Thu, 17 Feb 2022 06:47:35 +0000 (22:47 -0800)]
plpython: Reject Python 2 during build configuration.
Python 2.7 went EOL 2020-01-01 and the support for Python 2 requires a fair
bit of infrastructure. Therefore we are removing Python 2 support in plpython.
This patch just rejects Python 2 during configure / mkvcbuild.pl. Future
commits will remove the code and infrastructure for Python 2 support and
adjust more of the documentation. This way we can see the buildfarm state
after the removal sooner and we can be sure that failures are due to
desupporting Python 2, rather than caused by infrastructure cleanup.
Reviewed-By: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/
20211031184548.g4sxfe47n2kyi55r@alap3.anarazel.de
Peter Geoghegan [Thu, 17 Feb 2022 02:41:52 +0000 (18:41 -0800)]
Increase hash_mem_multiplier default to 2.0.
Double the default setting for hash_mem_multiplier, from 1.0 to 2.0.
This setting makes hash-based executor nodes use twice the usual
work_mem limit.
The PostgreSQL 15 release notes should have a compatibility note about
this change.
Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-Wzndc_ROk6CY-bC6p9O53q974Y0Ey4WX8jcPbuTZYM4Q3A@mail.gmail.com
Peter Geoghegan [Thu, 17 Feb 2022 01:15:50 +0000 (17:15 -0800)]
Avoid VACUUM reltuples distortion.
Add a heuristic that avoids distortion in the pg_class.reltuples
estimates used by VACUUM. Without the heuristic, successive manually
run VACUUM commands (run against a table that is never modified after
initial bulk loading) will scan the same page in each VACUUM operation.
Eventually pg_class.reltuples may reach the point where one single heap
page is accidentally considered highly representative of the entire
table. This is likely to be completely wrong, since the last heap page
typically has fewer tuples than average for the table.
It's not obvious that this was a problem prior to commit
44fa8488, which
made vacuumlazy.c consistently scan the last heap page (even when it is
all-visible in the visibility map). It seems possible that there were
more subtle variants of the same problem that went unnoticed for quite
some time, though. Commit
44fa8488 simplified certain aspects of when
and how relation truncation was considered, but it did not introduce the
"scan the last page" behavior. Essentially the same behavior was
introduced much earlier, in commit
e8429082. It was conditioned on
whether or not truncation looked promising towards the end of the
initial heap pass by VACUUM until recently, which was at least somewhat
protective. That doesn't seem like something that we should be relying
on, though.
Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-WzkNKORurux459M64mR63Aw4Jq7MBRVcX=CvALqN3A88WA@mail.gmail.com
Michael Paquier [Thu, 17 Feb 2022 00:52:02 +0000 (09:52 +0900)]
Remove all traces of tuplestore_donestoring() in the C code
This routine is a no-op since
dd04e95 from 2003, with a macro kept
around for compatibility purposes. This has led to the same code
patterns being copy-pasted around for no effect, sometimes in confusing
ways like in pg_logical_slot_get_changes_guts() from logical.c where the
code was actually incorrect.
This issue has been discussed on two different threads recently, so
rather than living with this legacy, remove any uses of this routine in
the C code to simplify things. The compatibility macro is kept to avoid
breaking any out-of-core modules that depend on it.
Reported-by: Tatsuhito Kasahara, Justin Pryzby
Author: Tatsuhito Kasahara
Discussion: https://postgr.es/m/
20211217200419.GQ17618@telsasoft.com
Discussion: https://postgr.es/m/CAP0=ZVJeeYfAeRfmzqAF2Lumdiv4S4FewyBnZd4DPTrsSQKJKw@mail.gmail.com
Heikki Linnakangas [Wed, 16 Feb 2022 21:15:08 +0000 (23:15 +0200)]
Fix bogus log message when starting from a cleanly shut down state.
In commit
70e81861fa to split xlog.c, I moved the startup code that
updates the state in the control file and prints out the "database
system was not properly shut down" message to the log, but I
accidentally removed the "if (InRecovery)" check around it. As a
result, that message was printed even if the system was cleanly shut
down, also during 'initdb'.
Discussion: https://www.postgresql.org/message-id/
3357075.
1645031062@sss.pgh.pa.us
John Naylor [Wed, 16 Feb 2022 12:33:28 +0000 (19:33 +0700)]
Add missing TYPEALIGN macros
A couple call sites still had hard-coded characters.
Amul Sul
Discussion: https://www.postgresql.org/message-id/CAAJ_b94Y35MWB3PJoCbc_O-_Q4%2B-9DHKhWtAwboEyx8wm4mqcA%40mail.gmail.com
Heikki Linnakangas [Wed, 16 Feb 2022 10:01:32 +0000 (12:01 +0200)]
Fix read beyond buffer bug introduced by the split xlog.c patch.
FinishWalRecovery() copied the valid part of the last WAL block into a
palloc'd buffer, and the code in StartupXLOG() copied it to the WAL
buffer. But the memcpy in StartupXLOG() copied a full 8kB block, not
just the valid part, i.e. it copied from beyond the end of the buffer.
The invalid part was cleared immediately afterwards, so as long as the
memory was allocated and didn't segfault, it didn't do any harm, but
it can definitely segfault.
Discussion: https://www.postgresql.org/message-id/
efc12e32-5af2-3485-5b1d-
5af9f707491a@iki.fi
Peter Eisentraut [Wed, 16 Feb 2022 09:32:36 +0000 (10:32 +0100)]
Reject trailing junk after numeric literals
After this, the PostgreSQL lexers no longer accept numeric literals
with trailing non-digits, such as 123abc, which would be scanned as
two tokens: 123 and abc. This is undocumented and surprising, and it
might also interfere with some extended numeric literal syntax being
contemplated for the future.
Reviewed-by: John Naylor <john.naylor@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/
b239564c-cad0-b23e-c57e-
166d883cb97d@enterprisedb.com
Heikki Linnakangas [Wed, 16 Feb 2022 07:30:38 +0000 (09:30 +0200)]
Split xlog.c into xlog.c and xlogrecovery.c.
This moves the functions related to performing WAL recovery into the new
xlogrecovery.c source file, leaving xlog.c responsible for maintaining
the WAL buffers, coordinating the startup and switch from recovery to
normal operations, and other miscellaneous stuff that have always been in
xlog.c.
Reviewed-by: Andres Freund, Kyotaro Horiguchi, Robert Haas
Discussion: https://www.postgresql.org/message-id/
a31f27b4-a31d-f976-6217-
2b03be646ffa%40iki.fi
Heikki Linnakangas [Wed, 16 Feb 2022 07:22:44 +0000 (09:22 +0200)]
Move code around in StartupXLOG().
This is in preparation for the next commit, which will split off
recovery-related code from xlog.c into a new source file. This is the
order that things will happen with the next commit, and the point of
this commit is to make these ordering changes more explicit, while the
next commit mechanically moves the source code to the new file. To aid
review, I added "BEGIN/END function" comments to mark which blocks of
code are moved to which functions in the next commit. They will be gone
in the next commit.
Reviewed-by: Andres Freund, Kyotaro Horiguchi, Robert Haas
Discussion: https://www.postgresql.org/message-id/
a31f27b4-a31d-f976-6217-
2b03be646ffa%40iki.fi
Heikki Linnakangas [Wed, 16 Feb 2022 07:22:41 +0000 (09:22 +0200)]
Refactor setting XLP_FIRST_IS_OVERWRITE_CONTRECORD.
Set it directly in CreateOverwriteContrecordRecord(). That way,
AdvanceXLInsertBuffer() doesn't need the missingContrecPtr global
variable. This is in preparation for splitting xlog.c into multiple
files.
Reviewed-by: Robert Haas
Discussion: https://www.postgresql.org/message-id/
a462d79c-cb5a-47cc-e9ac-
616b5003965f%40iki.fi
Heikki Linnakangas [Wed, 16 Feb 2022 07:22:34 +0000 (09:22 +0200)]
Run pgindent on xlog.c.
To tidy up after some recent refactorings in xlog.c. These would be
fixed by the pgindent run we do at the end of the development cycle,
but I want to clean these up now as I'm about to do some more big
refactorings on xlog.c.
Etsuro Fujita [Wed, 16 Feb 2022 06:15:00 +0000 (15:15 +0900)]
Doc: Update documentation for modifying postgres_fdw foreign tables.
Document that they can be modified using COPY as well.
Back-patch to v11 where commit
3d956d956 added support for COPY in
postgres_fdw.
Michael Paquier [Wed, 16 Feb 2022 01:25:12 +0000 (10:25 +0900)]
Add TAP test to automate the equivalent of check_guc, take two
src/backend/utils/misc/check_guc is a script that cross-checks the
consistency of the GUCs with postgresql.conf.sample, making sure that
its format is in line with what guc.c has. It has never been run
automatically, and has rotten over the years, creating a lot of false
positives as per a report from Justin Pryzby.
d10e41d has introduced a SQL function to publish the most relevant flags
associated to a GUC, with tests added in the main regression test suite
to make sure that we avoid most of the inconsistencies in the GUC
settings, based on recent reports, but there was nothing able to
cross-check postgresql.conf.sample with the contents of guc.c.
This commit adds a TAP test that covers the remaining gap. It emulates
the most relevant checks that check_guc did, so as any format mistakes
are detected in postgresql.conf.sample at development stage, with the
following checks:
- Check that parameters marked as NOT_IN_SAMPLE are not in the sample
file.
- Check that there are no dead entries in postgresql.conf.sample for
parameters not marked as NOT_IN_SAMPLE.
- Check that no parameters are missing from the sample file if listed in
guc.c without NOT_IN_SAMPLE.
The idea of building a list of the GUCs by parsing the sample file comes
from Justin, and he wrote the regex used in the patch to find all the
GUCs (this same formatting rule basically applies for the last 20~ years
or so). In order to test this patch, I have played with manual
modifications of postgresql.conf.sample and guc.c, making sure that we
detect problems with the GUC rules and the sample file format.
The test is located in src/test/modules/test_misc, which is the best
location I could think about for such sanity checks, rather than the
main regression test suite (src/test/regress) to avoid a new type of
dependency with the source tree.
The first attempt of this patch was
b0a55f4, where the location of
postgresql.conf.sample was retrieved using pg_config --sharedir. This
has proven to be an issue for distributions that patch pg_config to
enforce the installation paths at some wanted location (like Debian),
that may not exist when the test is run, hence causing a failure.
Instead of that, as per a suggestion from Andres Freund, rely on the
fact that the test is always executed from its directory in the source
tree and use a relative path to find the sample file. This works for
the CI, VPATH builds and on Windows, and tests like the recovery one
added in
f47ed79 rely on that already.
Reviewed-by: Justin Pryzby
Discussion: https://postgr.es/m/Yf9YGSwPiMu0c7fP@paquier.xyz
Heikki Linnakangas [Tue, 15 Feb 2022 23:37:48 +0000 (01:37 +0200)]
Fix race condition in 028_pitr_timelines.pl test, add note to docs.
The 028_pitr_timelines.pl test would sometimes hang, waiting for a WAL
segment that was just filled up to be archived. It was because the
test used 'pg_stat_archiver.last_archived_wal' to check if a file was
archived, but the order that WAL files are archived when a standby is
promoted is not fully deterministic, and 'last_archived_wal' tracks
the last segment that was archived, not the highest-numbered WAL
segment. Because of that, if the archiver archived segment 3, and then
2, 'last_archived_wal' say 2, and the test query would think that 3
has not been archived yet.
Normally, WAL files are marked ready for archival in order, and the
archiver process will process them in order, so that issue doesn't
arise. We have used the same query on 'last_archived_wal' in a few
other tests with no problem. But when a standby is promoted, things
are a bit chaotic. After promotion, the server will try to archive all
the WAL segments from the old timeline that are in pg_wal, as well as
the history file and any new WAL segments on the new timeline. The
end-of-recovery checkpoint will create the .ready files for all the
WAL files on the old timeline, but at the same time, the new timeline
is opened up for business. A file from the new timeline can therefore
be archived before the files from the old timeline have been marked as
ready for archival.
It turns out that we don't really need to wait for the archival in
this particular test, because the standby server is about to be
stopped, and stopping a server will wait for the end-of-recovery
checkpoint and all WAL archivals to finish, anyway. So we can just
remove it from the test.
Add a note to the docs on 'pg_stat_archiver' view that files can be
archived out of order.
Reviewed-by: Tom Lane
Discussion: https://www.postgresql.org/message-id/
3186114.
1644960507@sss.pgh.pa.us
Peter Geoghegan [Tue, 15 Feb 2022 23:16:19 +0000 (15:16 -0800)]
Update "don't truncate with failsafe" rationale.
There is a very good (though non-obvious) reason to avoid relation
truncation during a VACUUM that has triggered the failsafe mechanism,
which was missed before now. Update related comments, so this isn't
forgotten.
Reported-By: John Naylor <john.naylor@enterprisedb.com>
Discussion: https://postgr.es/m/CAFBsxsFiMPxQ-dHZ8tOgktn=+ffeJT3+GinZ4zdOGbmAnCYadA@mail.gmail.com
Tom Lane [Tue, 15 Feb 2022 22:28:17 +0000 (17:28 -0500)]
Ensure that length argument of memcmp() isn't seen as negative.
I think this will shut up a weird warning from buildfarm member
serinus. Perhaps it'd be better to change tsCompareString's
length arguments to unsigned, but that seems more invasive
than is justified.
Part of a general push to remove off-the-beaten-track warnings
where we can easily do so.
Tom Lane [Tue, 15 Feb 2022 22:17:25 +0000 (17:17 -0500)]
Ensure that the argument of shmdt(2) is declared "void *".
Our gcc-on-Solaris buildfarm members emit "incompatible pointer type"
warnings in places where it's not. This is a bit odd, since AFAICT
Solaris follows the POSIX spec in declaring shmdt's argument as
"const void *", and you'd think any pointer argument would satisfy that.
But whatever. Part of a general push to remove off-the-beaten-track
warnings where we can easily do so.
Andres Freund [Tue, 15 Feb 2022 21:44:22 +0000 (13:44 -0800)]
docs: Work around bug in the docbook xsl stylesheets.
docbook-xsl's index generation stylesheet (autoidx.xsl) has a small bug: It
doesn't include xlink in exclude-result-prefixes. Normally just leads to a a
single xmlns:xlink in the <div> containing the index, but because our
customization emits that, xmlns:xlink intead gets added to every element
output by autoidx.xsl below the <div>, totalling around 100kB.
Adding the spurious xmlns:xlink to the <div> ourselves isn't great, but avoids
the duplication.
Reviewed-By: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/
20220213201618.qz6p6noon3wagr3f%40alap3.anarazel.de
Tom Lane [Tue, 15 Feb 2022 17:57:44 +0000 (12:57 -0500)]
Reject change of output-column collation in CREATE OR REPLACE VIEW.
checkViewTupleDesc() didn't get the memo that it should verify
same attcollation along with same type/typmod. (A quick scan
did not find other similar oversights.)
Per bug #17404 from Pierre-Aurélien Georges. On another day
I might've back-patched this, but today I'm feeling paranoid
about unnecessary behavioral changes in back branches.
Discussion: https://postgr.es/m/17404-
8a4a270ef30a6709@postgresql.org
Daniel Gustafsson [Tue, 15 Feb 2022 10:35:17 +0000 (11:35 +0100)]
Ensure that STDERR is empty in connect_ok tests
Connections performed via connect_ok() in TAP tests should not write
anything to STDERR.
Author: Jacob Champion <pchampion@vmware.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/
9D4FFB61-392B-4A2C-B7E4-
911797B4AC14@yesql.se
Discussion: https://postgr.es/m/
ec146256e31afa0542f9fa970ec258c5f1a5f98.camel@vmware.com
Peter Eisentraut [Tue, 15 Feb 2022 09:58:28 +0000 (10:58 +0100)]
Fix XML namespace declarations
The XSL stylesheets used a mix of incorrect or outdated namespace
declarations for XHTML, probably based on ancient advice and examples.
Clean all this up.
Besides improving correctness (although probably no impact in
practice, other than possible validation failures), this removes a
bunch of useless namespace declarations in the HTML output.
Reported-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/flat/
20220213201618.qz6p6noon3wagr3f%40alap3.anarazel.de
Heikki Linnakangas [Tue, 15 Feb 2022 09:55:52 +0000 (11:55 +0200)]
Add more logging to new 028_pitr_timelines.pl test.
The test has failed a couple of times on buildfarm member 'hoverfly'. It
gets stuck waiting for the standby to archive
000000020000000000000003
WAL segment. I don't understand why, but with DEBUG1, we will get messages
in the log whenever a segment is archived, which hopefully will give a
clue the next time it happens.
Peter Eisentraut [Tue, 15 Feb 2022 09:03:52 +0000 (10:03 +0100)]
Remove IS_AF_UNIX macro
The AF_UNIX macro was being used unprotected by HAVE_UNIX_SOCKETS,
apparently since 2008. So the redirection through IS_AF_UNIX() is
apparently no longer necessary. (More generally, all supported
platforms are now HAVE_UNIX_SOCKETS, but even if there were a new
platform in the future, it seems plausible that it would define the
AF_UNIX symbol even without kernel support.) So remove the
IS_AF_UNIX() macro and make the code a bit more consistent.
Discussion: https://www.postgresql.org/message-id/flat/
f2d26815-9832-e333-d52d-
72fbc0ade896%40enterprisedb.com
John Naylor [Tue, 15 Feb 2022 07:30:57 +0000 (14:30 +0700)]
Spell "startup process" with lower case in the documentation
Most uses were already lower case, so this just makes all user-visible
spellings consistent.
Bharath Rupireddy
The proposed patch also had analagous changes for the code comments,
but I decided that wasn't worth the churn.
Discussion:
https://www.postgresql.org/message-id/flat/CALj2ACW7%2Bv_0QBPoWB%3DqKr67JKC019Htm%3DX8sKewS17bOquefg%40mail.gmail.com
Peter Eisentraut [Mon, 14 Feb 2022 20:29:45 +0000 (21:29 +0100)]
Add test case for trailing junk after numeric literals
PostgreSQL currently accepts numeric literals with trailing
non-digits, such as 123abc where the abc is treated as the next token.
This may be a bit surprising. This commit adds test cases for this;
subsequent commits intend to change this behavior.
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 [Mon, 14 Feb 2022 20:29:45 +0000 (21:29 +0100)]
Remove pg_atoi()
The last caller was int2vectorin(), and having such a general function
for one user didn't seem useful, so just put the required parts inline
and remove the function.
Reviewed-by: John Naylor <john.naylor@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/
b239564c-cad0-b23e-c57e-
166d883cb97d@enterprisedb.com
Andres Freund [Tue, 15 Feb 2022 05:52:55 +0000 (21:52 -0800)]
Add isolation test for errors during logical slot creation.
I didn't include this test in
2f6501fa3c5, because I was not sure the error
messages for the terminated connection is stable across platforms. But it
sounds like it is, and if not, we'd want to do something about the instability
anyway...
Discussion: https://postgr.es/m/CAD21AoDAeEpAbZEyYJsPZJUmSPaRicVSBObaL7sPaofnKz+9zg@mail.gmail.com
Michael Paquier [Tue, 15 Feb 2022 04:41:40 +0000 (13:41 +0900)]
Remove command checks in tests of pg_basebackup and pg_receivewal
The TAP tests of those commands have been checking if commands of "gzip"
and "lz4" existed by launching them with an extra --version. Based on
the buildfarm, this is not required for "gzip" as the command always
exists. Since
1d084fb, "lz4" has a ./configure check doing the same
thing.
Reported-by: Andres Freund
Discussion: https://postgr.es/m/
20220212220643.ozuvq2k4cjkcnr2v@alap3.anarazel.de
Discussion: https://postgr.es/m/Ygm2ADakjlqGc2Ro@paquier.xyz
Michael Paquier [Tue, 15 Feb 2022 02:46:55 +0000 (11:46 +0900)]
Fix thinko with subdirectories generated by pg_upgrade for internal files
38bfae3 has mixed the "dump/" and "log/" subdirectories generated in
"pg_upgrade_output.d/", causing the internal dump files to be generated
in "log/" and the log files to be in "dump/", but the opposite should be
done. This was not directly an issue for pg_upgrade runs, as the
internal dump files were still picked up at the location of their
creation, but the newest version of the buildfarm client would have
reported the dump files instead of the log files on failures of
pg_upgrade.
Issue spotted while testing the TAP tests of pg_upgrade.
Andres Freund [Tue, 15 Feb 2022 00:44:28 +0000 (16:44 -0800)]
Move replication slot release to before_shmem_exit().
Previously, replication slots were released in ProcKill() on error, resulting
in reporting replication slot drop of ephemeral slots after the stats
subsystem was already shut down.
To fix this problem, move replication slot release to a before_shmem_exit()
hook that is called before the stats collector shuts down. There wasn't really
a good reason for the slot handling to be in ProcKill() anyway.
Patch by Masahiko Sawada, with very minor polishing by me.
I, Andres, wrote a test for dropping slots during process exit, but there may
be some OS dependent issues around the number of times FATAL error messages
are displayed due to a still debated libpq issue. So that test will be
committed separately / later.
Reviewed-By: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-By: Andres Freund <andres@anarazel.de>
Author: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAD21AoDAeEpAbZEyYJsPZJUmSPaRicVSBObaL7sPaofnKz+9zg@mail.gmail.com
Peter Eisentraut [Mon, 14 Feb 2022 20:29:45 +0000 (21:29 +0100)]
Remove one use of pg_atoi()
There was no real need to use this here instead of a simpler API.
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 [Mon, 14 Feb 2022 20:29:45 +0000 (21:29 +0100)]
Move scanint8() to numutils.c
Move scanint8() to numutils.c and rename to pg_strtoint64(). We
already have a "16" and "32" version of that, and the code inside the
functions was aligned, so this move makes all three versions
consistent. The API is also changed to no longer provide the errorOK
case. Users that need the error checking can use strtoi64().
Reviewed-by: John Naylor <john.naylor@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/
b239564c-cad0-b23e-c57e-
166d883cb97d@enterprisedb.com
Tom Lane [Mon, 14 Feb 2022 16:25:46 +0000 (11:25 -0500)]
Suppress integer-overflow compiler warning for inconsistent sun_len.
On AIX 7.1, struct sockaddr_un is declared to be 1025 bytes long,
but the sun_len field that should hold the length is only a byte.
Clamp the value we try to store to ensure it will fit in the field.
(This coding might need adjustment if there are any machines out
there where sun_len is as wide as size_t; but a preliminary survey
suggests there's not, so let's keep it simple.)
Discussion: https://postgr.es/m/
2781112.
1644819528@sss.pgh.pa.us
Tom Lane [Mon, 14 Feb 2022 15:56:19 +0000 (10:56 -0500)]
Delete contrib/xml2's legacy implementation of xml_is_well_formed().
This function is unreferenced in modern usage; it was superseded in 9.1
by a core function of the same name. It has been left in place in the C
code only so that pre-9.1 SQL definitions of the contrib/xml2 functions
would continue to work. Eleven years seems like enough time for people
to have updated to the extension-style version of the xml2 module, so
let's drop this.
We did this once before, in
20540710e, and then reverted it because
the intended change of PGDLLEXPORT markings didn't happen. This
time the reason is to suppress link-time duplicate-symbol warnings
on AIX. That's not worth a lot perhaps, but the value of keeping
this function has surely dropped to about zero by now.
Discussion: https://postgr.es/m/
2717731.
1644778752@sss.pgh.pa.us
Heikki Linnakangas [Mon, 14 Feb 2022 09:33:57 +0000 (11:33 +0200)]
Add test case for an archive recovery corner case.
While I was working on a patch to refactor things around xlog.c, I mixed
up EndOfLogTLI and replayTLI at the end of recovery. As a result, if you
recovered to a point with a lower-numbered timeline in a WAL segment
that has a higher TLI in the filename, the end-of-recovery WAL record
was created with invalid PrevTimeLineId. I noticed that while
self-reviewing, but no tests failed. So add a test to cover that corner
case.
Thanks to Amul Sul who also submitted a test case for the same corner
case, although this patch is different from that.
Reviewed-by: Amul Sul, Michael Paquier
Discussion: https://www.postgresql.org/message-id/
52bc9ccd-8591-431b-0086-
15d9acf25a3f@iki.fi
Discussion: https://www.postgresql.org/message-id/CAAJ_b94Vjt5cXGza_1MkjLQWciNdEemsmiWuQj0d%3DM7JfjAa1g%40mail.gmail.com
Peter Eisentraut [Mon, 14 Feb 2022 08:11:13 +0000 (09:11 +0100)]
Add missing node support functions
forgotten in
37851a8b83d3d57ca48736093b10aa5f3bc0c177
Peter Eisentraut [Mon, 14 Feb 2022 07:09:04 +0000 (08:09 +0100)]
Database-level collation version tracking
This adds to database objects the same version tracking that collation
objects have. There is a new pg_database column datcollversion that
stores the version, a new function
pg_database_collation_actual_version() to get the version from the
operating system, and a new subcommand ALTER DATABASE ... REFRESH
COLLATION VERSION.
This was not originally added together with pg_collation.collversion,
since originally version tracking was only supported for ICU, and ICU
on a database-level is not currently supported. But we now have
version tracking for glibc (since PG13), FreeBSD (since PG14), and
Windows (since PG13), so this is useful to have now.
Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/
f0ff3190-29a3-5b39-a179-
fa32eee57db6%40enterprisedb.com
Peter Eisentraut [Sun, 13 Feb 2022 20:07:25 +0000 (21:07 +0100)]
Improve correlation names in sanity tests
Some of the queries in the "sanity" tests in the regression test suite
(opr_sanity, type_sanity) are very confusing. One main stumbling
block is that for some probably ancient reason many of the older
queries are written with correlation names p1, p2, etc. independent of
the name of the catalog. This one is a good example:
SELECT p1.oid, p1.oprname, p2.oid, p2.proname
FROM pg_operator AS p1, pg_proc AS p2 <-- HERE
WHERE p1.oprcode = p2.oid AND
p1.oprkind = 'l' AND
(p2.pronargs != 1
OR NOT binary_coercible(p2.prorettype, p1.oprresult)
OR NOT binary_coercible(p1.oprright, p2.proargtypes[0])
OR p1.oprleft != 0);
This is better written as
SELECT o1.oid, o1.oprname, p1.oid, p1.proname
FROM pg_operator AS o1, pg_proc AS p1
WHERE o1.oprcode = p1.oid AND
o1.oprkind = 'l' AND
(p1.pronargs != 1
OR NOT binary_coercible(p1.prorettype, o1.oprresult)
OR NOT binary_coercible(o1.oprright, p1.proargtypes[0])
OR o1.oprleft != 0);
This patch cleans up all the queries in this manner.
(As in the above case, I kept the digits like o1 and p1 even in cases
where only one of each letter is used in a query. This is mainly to
keep the style consistent.)
Discussion: https://www.postgresql.org/message-id/flat/
c538308b-319c-8784-e250-
1284d12d5411%40enterprisedb.com
Thomas Munro [Mon, 14 Feb 2022 03:29:44 +0000 (16:29 +1300)]
Use WL_SOCKET_CLOSED for client_connection_check_interval.
Previously we used poll() directly to check for a POLLRDHUP event.
Instead, use the WaitEventSet API to poll the socket for
WL_SOCKET_CLOSED, which knows how to detect this condition on many more
operating systems.
Reviewed-by: Zhihong Yu <zyu@yugabyte.com>
Reviewed-by: Maksim Milyutin <milyutinma@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/
77def86b27e41f0efcba411460e929ae%40postgrespro.ru
Thomas Munro [Mon, 14 Feb 2022 03:29:28 +0000 (16:29 +1300)]
Add WL_SOCKET_CLOSED for socket shutdown events.
Provide a way for WaitEventSet to report that the remote peer has shut
down its socket, independently of whether there is any buffered data
remaining to be read. This works only on systems where the kernel
exposes that information, namely:
* WAIT_USE_POLL builds using POLLRDHUP, if available
* WAIT_USE_EPOLL builds using EPOLLRDHUP
* WAIT_USE_KQUEUE builds using EV_EOF
Reviewed-by: Zhihong Yu <zyu@yugabyte.com>
Reviewed-by: Maksim Milyutin <milyutinma@gmail.com>
Discussion: https://postgr.es/m/
77def86b27e41f0efcba411460e929ae%40postgrespro.ru
Amit Kapila [Mon, 14 Feb 2022 03:25:58 +0000 (08:55 +0530)]
WAL log unchanged toasted replica identity key attributes.
Currently, during UPDATE, the unchanged replica identity key attributes
are not logged separately because they are getting logged as part of the
new tuple. But if they are stored externally then the untoasted values are
not getting logged as part of the new tuple and logical replication won't
be able to replicate such UPDATEs. So we need to log such attributes as
part of the old_key_tuple during UPDATE.
Reported-by: Haiying Tang
Author: Dilip Kumar and Amit Kapila
Reviewed-by: Alvaro Herrera, Haiying Tang, Andres Freund
Backpatch-through: 10
Discussion: https://postgr.es/m/OS0PR01MB611342D0A92D4F4BF26C0F47FB229@OS0PR01MB6113.jpnprd01.prod.outlook.com
Thomas Munro [Mon, 14 Feb 2022 02:51:43 +0000 (15:51 +1300)]
Track LLVM 15 changes.
This isn't an API change, it's just a missing #include that we got away
with before. Per buildfarm animal seawasp.
John Naylor [Mon, 14 Feb 2022 02:03:46 +0000 (09:03 +0700)]
Correct Makefile dependencies for catalog scripts
At some point, Gen_fmgrtab.pl stopped needing the value of defined symbols
from access/transam.h, while genbki.pl starting doing so. The Makefiles
didn't get the memo, so update the relevant dependencies.
Michael Paquier [Mon, 14 Feb 2022 01:40:34 +0000 (10:40 +0900)]
Add ./configure check for "lz4" command
Some environments may compile with --with-lz4 while the command "lz4"
goes missing, causing two failures in the TAP tests of pg_verifybackup
(008_untar.pl and 010_client_untar.pl) as the code assumed that the
command always existed with a hardcoded value in src/Makefile.global.
Rather than this method, this adds a ./configure check based on
PGAC_PATH_PROGS() to find automatically the command and get an absolute
path to it.
Both tests need to be adjusted for the case where the command does not
exist, actually, as Makefile.global would set now LZ4 to an empty value
in this case. The TAP tests of pg_receivewal already do that.
Per report from buildfarm member copperhead, as an effect of
dab2984.
The origin of the failure is actually
babbbb5 that did not centralize
the check for the existence of a "lz4" command at ./configure to shave a
few cycles. Note that one just needs to tweak an environment to move
"lz4" out of the way to reproduce the problem, which is what I did to
test this change.
Per discussion with Robert Haas, Tom Lane, Andres Freund and myself.
Discussion: https://postgr.es/m/Ygc51WVAFGocSu4h@paquier.xyz
Alexander Korotkov [Mon, 14 Feb 2022 00:26:55 +0000 (03:26 +0300)]
Fix memory leak in IndexScan node with reordering
Fix ExecReScanIndexScan() to free the referenced tuples while emptying the
priority queue. Backpatch to all supported versions.
Discussion: https://postgr.es/m/CAHqSB9gECMENBQmpbv5rvmT3HTaORmMK3Ukg73DsX5H7EJV7jw%40mail.gmail.com
Author: Aliaksandr Kalenik
Reviewed-by: Tom Lane, Alexander Korotkov
Backpatch-through: 10
Michael Paquier [Mon, 14 Feb 2022 00:30:35 +0000 (09:30 +0900)]
Make origin data initialization consistent other fields in 2PC header
As of
1eb6d65, the origin data is optionally stored in a 2PC file
header, with the data filled in EndPrepare() even in the default case
where there is no origin data to add. This was inconsistent with all
the other fields of TwoPhaseFileHeader which are initialized in
StartPrepare(), so move the initialization of origin_lsn and
origin_timestamp there instead. The effect of missing the
initialization at this early stage is only cosmetic based on the current
logic of the code, but could have led to issues in the long-term, and it
is more consistent done this way.
Reported-by: Ranier Vilela
Discussion: https://postgr.es/m/CAEudQAooECJ+gU_RZB-yhioPOV94R4ucoHAf68PiJhLpgpVpBw@mail.gmail.com
Tom Lane [Mon, 14 Feb 2022 00:11:21 +0000 (19:11 -0500)]
Fix misuse of "const" qualifier.
"const foo *" is quite different from "foo * const".
This code was evidently trying to avoid casting away
const from the arguments, but entirely failed to do so.
Per study of some buildfarm warnings from anole
(which unfortunately are mostly ignorable, since it
seems not to understand "restrict" very well).
I'm surprised though that nothing else has complained.
Thomas Munro [Sun, 13 Feb 2022 23:52:57 +0000 (12:52 +1300)]
Remove REGRESS_OUTPUTDIR environment variable.
Andres Freund points out that the tmp_check path is already available as
perl variable PostgreSQL::Test::Utils::tmp_check, so we can drop the new
environment variable introduced by commit
f47ed79cc.
Discussion: https://postgr.es/m/
20220213052955.dh7lheehit7bsemf%40alap3.anarazel.de
Tom Lane [Sun, 13 Feb 2022 18:06:55 +0000 (13:06 -0500)]
Silence minor compiler warnings.
Depending on compiler version and optimization level, we might
get a complaint that lazy_scan_heap's "freespace" is used
uninitialized.
Compilers not aware that ereport(ERROR) doesn't return complained
about bbsink_lz4_new().
Assigning "-1" to a uint64 value has unportable results; fortunately,
the value of xlogreadsegno is unimportant when xlogreadfd is -1.
(It looks to me like there is no need for xlogreadsegno to be static
in the first place, but I didn't venture to change that.)