Peter Geoghegan [Thu, 4 Nov 2021 00:34:19 +0000 (17:34 -0700)]
Add another old commit to git-blame-ignore-revs.
Add another historic pgindent commit that was missed by the initial work
done in commit
8e638845.
Heikki Linnakangas [Wed, 3 Nov 2021 17:38:17 +0000 (19:38 +0200)]
Update alternative expected output file.
Previous commit added a test to 'largeobject', but neglected the
alternative expected output file 'largeobject_1.source'. Per failure
on buildfarm animal 'hamerkop'.
Discussion: https://www.postgresql.org/message-id/
DBA08346-9962-4706-92D1-
230EE5201C10@yesql.se
Heikki Linnakangas [Wed, 3 Nov 2021 08:28:52 +0000 (10:28 +0200)]
Fix snapshot reference leak if lo_export fails.
If lo_export() fails to open the target file or to write to it, it leaks
the created LargeObjectDesc and its snapshot in the top-transaction
context and resource owner. That's pretty harmless, it's a small leak
after all, but it gives the user a "Snapshot reference leak" warning.
Fix by using a short-lived memory context and no resource owner for
transient LargeObjectDescs that are opened and closed within one function
call. The leak is easiest to reproduce with lo_export() on a directory
that doesn't exist, but in principle the other lo_* functions could also
fail.
Backpatch to all supported versions.
Reported-by: Andrew B
Reviewed-by: Alvaro Herrera
Discussion: https://www.postgresql.org/message-id/
32bf767a-2d65-71c4-f170-
122f416bab7e@iki.fi
Peter Eisentraut [Wed, 3 Nov 2021 06:34:28 +0000 (07:34 +0100)]
Fix incorrect format placeholder
Peter Geoghegan [Wed, 3 Nov 2021 02:52:11 +0000 (19:52 -0700)]
Fix parallel amvacuumcleanup safety bug.
Commit
b4af70cb inverted the return value of the function
parallel_processing_is_safe(), but missed the amvacuumcleanup test.
Index AMs that don't support parallel cleanup at all were affected.
The practical consequences of this bug were not very serious. Hash
indexes are affected, but since they just return the number of blocks
during hashvacuumcleanup anyway, it can't have had much impact.
Author: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAD21AoA-Em+aeVPmBbL_s1V-ghsJQSxYL-i3JP8nTfPiD1wjKw@mail.gmail.com
Backpatch: 14-, where commit
b4af70cb appears.
Tom Lane [Tue, 2 Nov 2021 19:18:00 +0000 (15:18 -0400)]
Blind attempt to silence SSL compile failures on hamerkop.
Buildfarm member hamerkop has been failing for the last few days
with errors that look like OpenSSL's X509-related symbols have
not been imported into be-secure-openssl.c. It's unclear why
this should be, but let's try adding an explicit #include of
<openssl/x509v3.h>, as there has long been in fe-secure-openssl.c.
Discussion: https://postgr.es/m/
1051867.
1635720347@sss.pgh.pa.us
Peter Geoghegan [Tue, 2 Nov 2021 19:06:17 +0000 (12:06 -0700)]
Don't overlook indexes during parallel VACUUM.
Commit
b4af70cb, which simplified state managed by VACUUM, performed
refactoring of parallel VACUUM in passing. Confusion about the exact
details of the tasks that the leader process is responsible for led to
code that made it possible for parallel VACUUM to miss a subset of the
table's indexes entirely. Specifically, indexes that fell under the
min_parallel_index_scan_size size cutoff were missed. These indexes are
supposed to be vacuumed by the leader (alongside any parallel unsafe
indexes), but weren't vacuumed at all. Affected indexes could easily
end up with duplicate heap TIDs, once heap TIDs were recycled for new
heap tuples. This had generic symptoms that might be seen with almost
any index corruption involving structural inconsistencies between an
index and its table.
To fix, make sure that the parallel VACUUM leader process performs any
required index vacuuming for indexes that happen to be below the size
cutoff. Also document the design of parallel VACUUM with these
below-size-cutoff indexes.
It's unclear how many users might be affected by this bug. There had to
be at least three indexes on the table to hit the bug: a smaller index,
plus at least two additional indexes that themselves exceed the size
cutoff. Cases with just one additional index would not run into
trouble, since the parallel VACUUM cost model requires two
larger-than-cutoff indexes on the table to apply any parallel
processing. Note also that autovacuum was not affected, since it never
uses parallel processing.
Test case based on tests from a larger patch to test parallel VACUUM by
Masahiko Sawada.
Many thanks to Kamigishi Rei for her invaluable help with tracking this
problem down.
Author: Peter Geoghegan <pg@bowt.ie>
Author: Masahiko Sawada <sawada.mshk@gmail.com>
Reported-By: Kamigishi Rei <iijima.yun@koumakan.jp>
Reported-By: Andrew Gierth <andrew@tao11.riddles.org.uk>
Diagnosed-By: Andres Freund <andres@anarazel.de>
Bug: #17245
Discussion: https://postgr.es/m/17245-
ddf06aaf85735f36@postgresql.org
Discussion: https://postgr.es/m/
20211030023740.qbnsl2xaoh2grq3d@alap3.anarazel.de
Backpatch: 14-, where the refactoring commit appears.
Tom Lane [Tue, 2 Nov 2021 18:28:50 +0000 (14:28 -0400)]
Ensure consistent logical replication of datetime and float8 values.
In walreceiver, set the publisher's relevant GUCs (datestyle,
intervalstyle, extra_float_digits) to the same values that pg_dump uses,
and for the same reason: we need the output to be read the same way
regardless of the receiver's settings. Without this, it's possible
for subscribers to misinterpret transmitted values.
Although this is clearly a bug fix, it's not without downsides:
subscribers that are storing values into some other datatype, such as
text, could get different results than before, and perhaps be unhappy
about that. Given the lack of previous complaints, it seems best
to change this only in HEAD, and to call it out as an incompatible
change in v15.
Japin Li, per report from Sadhuprasad Patro
Discussion: https://postgr.es/m/CAFF0-CF=D7pc6st-3A9f1JnOt0qmc+BcBPVzD6fLYisKyAjkGA@mail.gmail.com
Tom Lane [Tue, 2 Nov 2021 17:36:47 +0000 (13:36 -0400)]
Fix variable lifespan in ExecInitCoerceToDomain().
This undoes a mistake in
1ec7679f1: domainval and domainnull were
meant to live across loop iterations, but they were incorrectly
moved inside the loop. The effect was only to emit useless extra
EEOP_MAKE_READONLY steps, so it's not a big deal; nonetheless,
back-patch to v13 where the mistake was introduced.
Ranier Vilela
Discussion: https://postgr.es/m/CAEudQAqXuhbkaAp-sGH6dR6Nsq7v28_0TPexHOm6FiDYqwQD-w@mail.gmail.com
Tom Lane [Tue, 2 Nov 2021 16:54:35 +0000 (12:54 -0400)]
Doc: clean up some places that mentioned template1 but not template0.
Improve old text that wasn't updated when we added template0 to
the standard database set.
Per suggestion from P. Luzanov.
Discussion: https://postgr.es/m/
163583775122.675.
3700595100340939507@wrigleys.postgresql.org
Tom Lane [Tue, 2 Nov 2021 16:12:02 +0000 (12:12 -0400)]
Doc: be more precise about conflicts between relation names.
Use verbiage like "The name of the table must be distinct from the name
of any other relation (table, sequence, index, view, materialized view,
or foreign table) in the same schema." in the reference pages for all
those object types. The main change here is to mention materialized
views explicitly; although a couple of these pages failed to say
anything at all about name conflicts.
Per suggestion from Daniel Westermann.
Discussion: https://postgr.es/m/ZR0P278MB0920D0946509233459AF0DEFD2889@ZR0P278MB0920.CHEP278.PROD.OUTLOOK.COM
Tom Lane [Tue, 2 Nov 2021 15:31:54 +0000 (11:31 -0400)]
Avoid O(N^2) behavior in SyncPostCheckpoint().
As in commits
6301c3ada and
e9d9ba2a4, avoid doing repetitive
list_delete_first() operations, since that would be expensive when
there are many files waiting to be unlinked. This is a slightly
larger change than in those cases. We have to keep the list state
valid for calls to AbsorbSyncRequests(), so it's necessary to invent a
"canceled" field instead of immediately deleting PendingUnlinkEntry
entries. Also, because we might not be able to process all the
entries, we need a new list primitive list_delete_first_n().
list_delete_first_n() is almost list_copy_tail(), but it modifies the
input List instead of making a new copy. I found a couple of existing
uses of the latter that could profitably use the new function. (There
might be more, but the other callers look like they probably shouldn't
overwrite the input List.)
As before, back-patch to v13.
Discussion: https://postgr.es/m/
CD2F0E7F-9822-45EC-A411-
AE56F14DEA9F@amazon.com
Fujii Masao [Tue, 2 Nov 2021 14:08:02 +0000 (23:08 +0900)]
pgbench: Fix typo in comment.
Discussion: https://postgr.es/m/
f9041ec2-46b6-1b41-0e84-
9c8a1e2d6bda@oss.nttdata.com
Fujii Masao [Tue, 2 Nov 2021 13:49:57 +0000 (22:49 +0900)]
pgbench: Improve error-handling in pgbench.
Previously failures of initial connection and logfile open caused pgbench
to proceed the benchmarking, report the incomplete results and exit with
status 2. It didn't make sense to proceed the benchmarking even when
pgbench could not start as prescribed.
This commit improves pgbench so that early errors that occur when
starting benchmark such as those failures should make pgbench exit
immediately with status 1.
Author: Yugo Nagata
Reviewed-by: Fabien COELHO, Kyotaro Horiguchi, Fujii Masao
Discussion: https://postgr.es/m/TYCPR01MB5870057375ACA8A73099C649F5349@TYCPR01MB5870.jpnprd01.prod.outlook.com
Amit Kapila [Tue, 2 Nov 2021 03:40:17 +0000 (09:10 +0530)]
Move MarkCurrentTransactionIdLoggedIfAny() out of the critical section.
We don't modify any shared state in this function which could cause
problems for any concurrent session. This will make it look similar to the
other updates for the same structure (TransactionState) which avoids
confusion for future readers of code.
Author: Dilip Kumar
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/E1mSoYz-0007Fh-D9@gemulon.postgresql.org
Amit Kapila [Tue, 2 Nov 2021 03:05:29 +0000 (08:35 +0530)]
Replace XLOG_INCLUDE_XID flag with a more localized flag.
Commit
0bead9af484c introduced XLOG_INCLUDE_XID flag to indicate that the
WAL record contains subXID-to-topXID association. It uses that flag later
to mark in CurrentTransactionState that top-xid is logged so that we
should not try to log it again with the next WAL record in the current
subtransaction. However, we can use a localized variable to pass that
information.
In passing, change the related function and variable names to make them
consistent with what the code is actually doing.
Author: Dilip Kumar
Reviewed-by: Alvaro Herrera, Amit Kapila
Discussion: https://postgr.es/m/E1mSoYz-0007Fh-D9@gemulon.postgresql.org
Daniel Gustafsson [Mon, 1 Nov 2021 21:42:49 +0000 (22:42 +0100)]
Replace unicode characters in comments with ascii
The unicode characters, while in comments and not code, caused MSVC
to emit compiler warning C4819:
The file contains a character that cannot be represented in the
current code page (number). Save the file in Unicode format to
prevent data loss.
Fix by replacing the characters in print.c with descriptive comments
containing the codepoints and symbol names, and remove the character
in brin_bloom.c which was a footnote reference copied from the paper
citation.
Per report from hamerkop in the buildfarm.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/
340E4118-0D0C-4E85-8141-
8C40EB22DA3A@yesql.se
Tom Lane [Mon, 1 Nov 2021 20:24:39 +0000 (16:24 -0400)]
Avoid some other O(N^2) hazards in list manipulation.
In the same spirit as
6301c3ada, fix some more places where we were
using list_delete_first() in a loop and thereby risking O(N^2)
behavior. It's not clear that the lists manipulated in these spots
can get long enough to be really problematic ... but it's not clear
that they can't, either, and the fixes are simple enough.
As before, back-patch to v13.
Discussion: https://postgr.es/m/
CD2F0E7F-9822-45EC-A411-
AE56F14DEA9F@amazon.com
Alvaro Herrera [Mon, 1 Nov 2021 16:07:23 +0000 (13:07 -0300)]
Handle XLOG_OVERWRITE_CONTRECORD in DecodeXLogOp
Failing to do so results in inability of logical decoding to process the
WAL stream. Handle it by doing nothing.
Backpatch all the way back.
Reported-by: Petr Jelínek <petr.jelinek@enterprisedb.com>
Michael Paquier [Mon, 1 Nov 2021 04:16:04 +0000 (13:16 +0900)]
Add TAP test for pg_receivewal with timeline switch
pg_receivewal is able to follow a timeline switch, but this was not
tested. This test uses an empty archive location with a restart done
from a slot, making its implementation a tad simpler than if we would
reuse an existing archive directory.
Author: Ronan Dunklau
Reviewed-by: Kyotaro Horiguchi, Michael Paquier
Discussion: https://postgr.es/m/
18708360.4lzOvYHigE@aivenronan
Michael Paquier [Mon, 1 Nov 2021 02:38:23 +0000 (11:38 +0900)]
Preserve opclass parameters across REINDEX CONCURRENTLY
The opclass parameter Datums from the old index are fetched in the same
way as for predicates and expressions, by grabbing them directly from
the system catalogs. They are then copied into the new IndexInfo that
will be used for the creation of the new copy.
This caused the new index to be rebuilt with default parameters rather
than the ones pre-defined by a user. The only way to get back a new
index with correct opclass parameters would be to recreate a new index
from scratch.
The issue has been introduced by
911e702.
Author: Michael Paquier
Reviewed-by: Zhihong Yu
Discussion: https://postgr.es/m/YX0CG/QpLXcPr8HJ@paquier.xyz
Backpatch-through: 13
Tom Lane [Sun, 31 Oct 2021 23:13:48 +0000 (19:13 -0400)]
Don't try to read a multi-GB pg_stat_statements file in one call.
Windows fails on a request to read() more than INT_MAX bytes,
and perhaps other platforms could have similar issues. Let's
adjust this code to read at most 1GB per call.
(One would not have thought the file could get that big, but now
we have a field report of trouble, so it can. We likely ought to
add some mechanism to limit the size of the query-texts file
separately from the size of the hash table. That is not this
patch, though.)
Per bug #17254 from Yusuke Egashira. It's been like this for
awhile, so back-patch to all supported branches.
Discussion: https://postgr.es/m/17254-
a926c89dc03375c2@postgresql.org
Tom Lane [Sun, 31 Oct 2021 22:12:44 +0000 (18:12 -0400)]
Doc: improve README files associated with TAP tests.
Rearrange src/test/perl/README so that the first section is more
clearly "how to run these tests", and the rest "how to write new
tests". Add some basic info there about debugging test failures.
Then, add cross-refs to that READNE from other READMEs that
describe how to run TAP tests.
Per suggestion from Kevin Burke, though this is not his original
patch.
Discussion: https://postgr.es/m/CAKcy5eiSbwiQnmCfnOnDCVC7B8fYyev3E=6pvvECP9pLE-Fcuw@mail.gmail.com
Tom Lane [Sun, 31 Oct 2021 19:31:29 +0000 (15:31 -0400)]
Avoid O(N^2) behavior when the standby process releases many locks.
When replaying a transaction that held many exclusive locks on the
primary, a standby server's startup process would expend O(N^2)
effort on manipulating the list of locks. This code was fine when
written, but commit
1cff1b95a made repetitive list_delete_first()
calls inefficient, as explained in its commit message. Fix by just
iterating the list normally, and releasing storage only when done.
(This'd be inadequate if we needed to recover from an error occurring
partway through; but we don't.)
Back-patch to v13 where
1cff1b95a came in.
Nathan Bossart
Discussion: https://postgr.es/m/
CD2F0E7F-9822-45EC-A411-
AE56F14DEA9F@amazon.com
Tom Lane [Sun, 31 Oct 2021 16:43:47 +0000 (12:43 -0400)]
plpgsql: report proper line number for errors in variable initialization.
Previously, we pointed at the surrounding block's BEGIN keyword.
If there are multiple variables being initialized in a DECLARE section,
this isn't good enough: it can be quite confusing and unhelpful.
We do know where the variable's declaration started, so it just takes
a tiny bit more error-reporting infrastructure to use that.
Discussion: https://postgr.es/m/713975.
1635530414@sss.pgh.pa.us
Peter Eisentraut [Sat, 30 Oct 2021 17:05:12 +0000 (19:05 +0200)]
pg_dump: Refactor messages
This reduces the number of separate messages for translation.
Peter Eisentraut [Sat, 30 Oct 2021 11:14:52 +0000 (13:14 +0200)]
doc: Remove some obsolete pgcrypto documentation
The pgcrypto documentation contained acknowledgments of used external
code, but some of this code has been moved to src/common/, so
mentioning it with pgcrypto no longer makes sense, so remove it.
Peter Eisentraut [Sat, 30 Oct 2021 10:53:02 +0000 (12:53 +0200)]
uuid-ossp: Remove obsolete build connection with pgcrypto
unused since
a8ed6bb8f4cf259b95c1bff5da09a8f4c79dca46
Peter Eisentraut [Sat, 30 Oct 2021 10:38:14 +0000 (12:38 +0200)]
Remove unused chunk from standalone-profile.xsl
unused since
1707a0d2aa6b2bcfe78f63836c769943a1a6b9e0
Robert Haas [Fri, 29 Oct 2021 18:40:15 +0000 (14:40 -0400)]
Fix race condition in startup progress reporting.
Commit
9ce346eabf350a130bba46be3f8c50ba28506969 added startup
progress reporting, but begin_startup_progress_phase has a race
condition: the timeout for the previous phase might fire just
before we reschedule the interrupt for the next phase.
To avoid the race, disable the timeout, clear the flag, and then
re-enable the timeout.
Patch by me, reviewed by Nitin Jadhav.
Discussion: https://postgr.es/m/CA+TgmoYq38i6iAzfRLVxA6Cm+wMCf4WM8wC3o_a+X_JvWC8bJg@mail.gmail.com
Robert Haas [Fri, 29 Oct 2021 18:00:32 +0000 (14:00 -0400)]
When fetching WAL for a basebackup, report errors with a sensible TLI.
The previous code used ThisTimeLineID, which need not even be
initialized here, although it usually was in practice, because
pg_basebackup issues IDENTIFY_SYSTEM before calling BASE_BACKUP,
and that initializes ThisTimeLineID as a side effect. That's not
really good enough, though, not only because we shoudn't be counting
on side effects like that, but also because the TLI could change
meanwhile. Fortunately, we have convenient access to more meaningful
TLI values, so use those instead.
Because of the way this logic is coded, the consequences of using
a possibly-incorrect TLI here are no worse than a slightly confusing
error message, I don't want to take any risk here, so no back-patch
at least for now.
Patch by me, reviewed by Kyotaro Horiguchi and Michael Paquier
Discussion: http://postgr.es/m/CA+TgmoZRNWGWYDX9RgTXMG6_nwSdB=PB-PPRUbvMUTGfmL2sHQ@mail.gmail.com
Peter Geoghegan [Fri, 29 Oct 2021 17:53:48 +0000 (10:53 -0700)]
Demote pg_unreachable() in heapam to an assertion.
Commit
d168b66682, which overhauled index deletion, added a
pg_unreachable() to the end of a sort comparator used when sorting heap
TIDs from an index page. This allows the compiler to apply
optimizations that assume that the heap TIDs from the index AM must
always be unique.
That doesn't seem like a good idea now, given recent reports of
corruption involving duplicate TIDs in indexes on Postgres 14. Demote
to an assertion, just in case.
Backpatch: 14-, where index deletion was overhauled.
Tom Lane [Fri, 29 Oct 2021 16:45:33 +0000 (12:45 -0400)]
Test and document the behavior of initialization cross-refs in plpgsql.
We had a test showing that a variable isn't referenceable in its
own initialization expression, nor in prior ones in the same block.
It *is* referenceable in later expressions in the same block, but
AFAICS there is no test case exercising that. Add one, and also
add some error cases.
Also, document that this is possible, since the docs failed to
cover the point.
Per question from tomás at tuxteam. I don't feel any need to
back-patch this, but we should ensure we don't break it in future.
Discussion: https://postgr.es/m/
20211029121435.GA5414@tuxteam.de
Tom Lane [Fri, 29 Oct 2021 15:38:18 +0000 (11:38 -0400)]
Update time zone data files to tzdata release 2021e.
DST law changes in Fiji, Jordan, Palestine, and Samoa. Historical
corrections for Barbados, Cook Islands, Guyana, Niue, Portugal, and
Tonga.
Also, the Pacific/Enderbury zone has been renamed to Pacific/Kanton.
The following zones have been merged into nearby, more-populous zones
whose clocks have agreed since 1970: Africa/Accra, America/Atikokan,
America/Blanc-Sablon, America/Creston, America/Curacao,
America/Nassau, America/Port_of_Spain, Antarctica/DumontDUrville,
and Antarctica/Syowa.
Amit Kapila [Fri, 29 Oct 2021 02:18:10 +0000 (07:48 +0530)]
Add tap tests for the schema publications.
This adds additional tests for commit
5a2832465f ("Allow publishing the
tables of schema.). This allows testing streaming of data in tables that
are published via schema publications.
Author: Vignesh C, Haiying Tang
Reviewed-by: Greg Nancarrow, Hou Zhijie, Amit Kapila
Discussion: https://www.postgresql.org/message-id/CALDaNm0OANxuJ6RXqwZsM1MSY4s19nuH3734j4a72etDwvBETQ%40mail.gmail.com
Michael Paquier [Fri, 29 Oct 2021 01:41:44 +0000 (10:41 +0900)]
Speed up TAP tests of pg_receivewal
This commit improves the speed of those tests by 25~30%, using some
simple ideas to reduce the amount of data written by pg_receivewal:
- Use a segment size of 1MB. While reducing the amount of data zeroed
by pg_receivewal for the new segments, this improves the code coverage
with a non-default segment size.
- In the last test involving a slot's restart_lsn, generate a checkpoint
to advance the redo LSN and the WAL retained by the slot created,
reducing the number of segments that need to be archived. This counts
for most of the gain.
- Minimize the amount of data inserted into the dummy table.
Reviewed-by: Ronan Dunklau
Discussion: https://postgr.es/m/YXqYKAdVEqmyTltK@paquier.xyz
Tom Lane [Thu, 28 Oct 2021 17:39:57 +0000 (13:39 -0400)]
Speed up printing of integers in snprintf.c.
Since the only possible divisors are 8, 10, and 16, it doesn't cost
much code space to replace the division loop with three copies using
constant divisors. On most machines, division by a constant can be
done a lot more cheaply than division by an arbitrary value.
A microbenchmark testing just snprintf("foo %d") with a 9-digit value
showed about a 2X speedup for me (tgl). Most of Postgres isn't too
dependent on the speed of snprintf, so that the effect in real-world
cases is barely measurable. Still, a cycle saved is a cycle earned.
Arjan van de Ven
Discussion: https://postgr.es/m/
40a4b32a-b841-4667-11b2-
a0baedb12714@linux.intel.com
Discussion: https://postgr.es/m/
6e51c644-1b6d-956e-ac24-
2d1b0541d532@linux.intel.com
Tom Lane [Thu, 28 Oct 2021 15:45:14 +0000 (11:45 -0400)]
Improve contrib/amcheck's tests for CREATE INDEX CONCURRENTLY.
Commits
fdd965d07 and
3cd9c3b92 tested CREATE INDEX CONCURRENTLY by
launching two separate pgbench runs concurrently. This was needed so
that only a single client thread would run CREATE INDEX CONCURRENTLY,
avoiding deadlock between two CICs. However, there's a better way,
which is to use an advisory lock to prevent concurrent CICs. That's
better in part because the test code is shorter and more readable, but
mostly because it automatically scales things to launch an appropriate
number of CICs relative to the number of INSERT transactions.
As committed, typically half to three-quarters of the CIC transactions
were pointless because the INSERT transactions had already stopped.
In passing, remove background_pgbench, which was added to support
these tests and isn't needed anymore. We can always put it back
if we find a use for it later.
Back-patch to v12; older pgbench versions lack the
conditional-execution features needed for this method.
Tom Lane and Andrey Borodin
Discussion: https://postgr.es/m/139687.
1635277318@sss.pgh.pa.us
Michael Paquier [Thu, 28 Oct 2021 01:49:26 +0000 (10:49 +0900)]
Add TAP test for archive_cleanup_command and recovery_end_command
This adds tests checking for the execution of both commands. The
recovery test 002_archiving.pl is nicely adapted to that, as promotion
is triggered already twice there, and even if any of those commands fail
they don't affect recovery or promotion.
A command success is checked using a file generated by an "echo"
command, that should be able to work in all the buildfarm environments,
even Msys (but we'll know soon about that). Command failure is tested
with an "echo" command that points to a path that does not exist,
scanning the backend logs to make sure that the failure happens. Both
rely on the backend triggering the commands from the root of the data
folder, making its logic more robust.
Thanks to Neha Sharma for the extra tests on Windows.
Author: Amul Sul, Michael Paquier
Reviewed-by: Andres Freund, Euler Taveira
Discussion: https://postgr.es/m/CAAJ_b95R_c4T5moq30qsybSU=eDzDHm=4SPiAWaiMWc2OW7=1Q@mail.gmail.com
Michael Paquier [Thu, 28 Oct 2021 00:25:55 +0000 (09:25 +0900)]
doc: Fix link to SELinux user guide in sepgsql page
Reported-by: Anton Voloshin
Discussion: https://postgr.es/m/
15a86d4e-a237-1acd-18a2-
fd69730f1ab9@postgrespro.ru
Backpatch-through: 10
Peter Geoghegan [Wed, 27 Oct 2021 21:35:21 +0000 (14:35 -0700)]
Remove obsolete nbtree LP_DEAD item comments.
Comments above _bt_findinsertloc() that talk about LP_DEAD items are now
out of place. We already discuss index tuple deletion at an earlier
point in the same comment block.
Oversight in commit
d168b666.
Jeff Davis [Wed, 27 Oct 2021 19:37:09 +0000 (12:37 -0700)]
Grant memory views to pg_read_all_stats.
Grant privileges on views pg_backend_memory_contexts and
pg_shmem_allocations to the role pg_read_all_stats. Also grant on the
underlying functions that those views depend on.
Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Nathan Bossart <bossartn@amazon.com>
Discussion: https://postgr.es/m/CALj2ACWAZo3Ar_EVsn2Zf9irG+hYK3cmh1KWhZS_Od45nd01RA@mail.gmail.com
Daniel Gustafsson [Wed, 27 Oct 2021 20:38:38 +0000 (22:38 +0200)]
Fix typos in comments
Author: Peter Smith <smithpb2250@gmail.com>
Discussion: https://postgr.es/m/CAHut+PsN_gmKu-KfeEb9NDARoTPbs4AN4PPu=6LZXFZRJ13SEw@mail.gmail.com
Peter Geoghegan [Wed, 27 Oct 2021 20:05:35 +0000 (13:05 -0700)]
Fix ordering of items in nbtree error message.
Oversight in commit
a5213adf.
Backpatch: 13-, just like commit
a5213adf.
Daniel Gustafsson [Wed, 27 Oct 2021 19:49:48 +0000 (21:49 +0200)]
Fix VPATH builds for src/test/ssl targets
Commit
b4c4a00ea refactored the gist of the sslfiles target into a
separate makefile in order to override settings in Makefile.global.
The invocation of this this file didn't however include the absolute
path for VPATH builds, resulting in "make clean" failing. Fix by
providing the path to the new makefile.
Reported-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/
20211026174152.jjcagswnbhxu7uqz@alap3.anarazel.de
Peter Geoghegan [Wed, 27 Oct 2021 19:10:47 +0000 (12:10 -0700)]
Further harden nbtree posting split code.
Add more defensive checks around posting list split code. These should
detect corruption involving duplicate table TIDs earlier and more
reliably than any existing check.
Follow up to commit
8f72bbac.
Discussion: https://postgr.es/m/CAH2-WzkrSY_kjyd1_M5xJK1uM0govJXMxPn8JUSvwcUOiHuWVw@mail.gmail.com
Backpatch: 13-, where nbtree deduplication was introduced.
Magnus Hagander [Wed, 27 Oct 2021 14:20:02 +0000 (16:20 +0200)]
Clarify that --system reindexes system catalogs *only*
Make this more clear both in the help message and docs.
Reviewed-By: Michael Paquier
Backpatch-through: 9.6
Discussion: https://postgr.es/m/CABUevEw6Je0WUFTLhPKOk4+BoBuDrE-fKw3N4ckqgDBMFu4paA@mail.gmail.com
Michael Paquier [Wed, 27 Oct 2021 07:02:19 +0000 (16:02 +0900)]
Add test for copy of shared dependencies from template database
As
98ec35b has proved, there has never been any coverage in this area of
the code. This commit adds a new TAP test with a template database that
includes a small set of shared dependencies copied to a new database.
The test is added in createdb, where we have never tested that -T
generates a query with TEMPLATE, either.
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/YXDTl+PfSnqmbbkE@paquier.xyz
Amit Kapila [Wed, 27 Oct 2021 02:14:52 +0000 (07:44 +0530)]
Allow publishing the tables of schema.
A new option "FOR ALL TABLES IN SCHEMA" in Create/Alter Publication allows
one or more schemas to be specified, whose tables are selected by the
publisher for sending the data to the subscriber.
The new syntax allows specifying both the tables and schemas. For example:
CREATE PUBLICATION pub1 FOR TABLE t1,t2,t3, ALL TABLES IN SCHEMA s1,s2;
OR
ALTER PUBLICATION pub1 ADD TABLE t1,t2,t3, ALL TABLES IN SCHEMA s1,s2;
A new system table "pg_publication_namespace" has been added, to maintain
the schemas that the user wants to publish through the publication.
Modified the output plugin (pgoutput) to publish the changes if the
relation is part of schema publication.
Updates pg_dump to identify and dump schema publications. Updates the \d
family of commands to display schema publications and \dRp+ variant will
now display associated schemas if any.
Author: Vignesh C, Hou Zhijie, Amit Kapila
Syntax-Suggested-by: Tom Lane, Alvaro Herrera
Reviewed-by: Greg Nancarrow, Masahiko Sawada, Hou Zhijie, Amit Kapila, Haiying Tang, Ajin Cherian, Rahila Syed, Bharath Rupireddy, Mark Dilger
Tested-by: Haiying Tang
Discussion: https://www.postgresql.org/message-id/CALDaNm0OANxuJ6RXqwZsM1MSY4s19nuH3734j4a72etDwvBETQ@mail.gmail.com
Jeff Davis [Tue, 26 Oct 2021 20:13:52 +0000 (13:13 -0700)]
Allow GRANT on pg_log_backend_memory_contexts().
Remove superuser check, allowing any user granted permissions on
pg_log_backend_memory_contexts() to log the memory contexts of any
backend.
Note that this could allow a privileged non-superuser to log the
memory contexts of a superuser backend, but as discussed, that does
not seem to be a problem.
Reviewed-by: Nathan Bossart, Bharath Rupireddy, Michael Paquier, Kyotaro Horiguchi, Andres Freund
Discussion: https://postgr.es/m/
e5cf6684d17c8d1ef4904ae248605ccd6da03e72.camel@j-davis.com
Fujii Masao [Tue, 26 Oct 2021 15:46:52 +0000 (00:46 +0900)]
Improve HINT message that FDW reports when there are no valid options.
The foreign data wrapper's validator function provides a HINT message with
list of valid options for the object specified in CREATE or ALTER command,
when the option given in the command is invalid. Previously
postgresql_fdw_validator() and the validator functions for postgres_fdw and
dblink_fdw worked in that way even there were no valid options in the object,
which could lead to the HINT message with empty list (because there were
no valid options). For example, ALTER FOREIGN DATA WRAPPER postgres_fdw
OPTIONS (format 'csv') reported the following ERROR and HINT messages.
This behavior was confusing.
ERROR: invalid option "format"
HINT: Valid options in this context are:
There is no such issue in file_fdw. The validator function for file_fdw
reports the HINT message "There are no valid options in this context."
instead in that case.
This commit improves postgresql_fdw_validator() and the validator functions
for postgres_fdw and dblink_fdw so that they do likewise. For example,
this change causes the above ALTER FOREIGN DATA WRAPPER command to
report the following messages.
ERROR: invalid option "nonexistent"
HINT: There are no valid options in this context.
Author: Kosei Masumura
Reviewed-by: Bharath Rupireddy, Fujii Masao
Discussion: https://postgr.es/m/
557d06cebe19081bfcc83ee2affc98d3@oss.nttdata.com
Daniel Gustafsson [Tue, 26 Oct 2021 08:40:08 +0000 (10:40 +0200)]
Ensure that slots are zeroed before use
The previous coding relied on the memory for the slots being zeroed
elsewhere, which while it was true in this case is not an contract
which is guaranteed to hold. Explicitly clear the tts_isnull array
to ensure that the slots are filled from a known state.
Backpatch to v14 where the catalog multi-inserts were introduced.
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CAJ7c6TP0AowkUgNL6zcAK-s5HYsVHVBRWfu69FRubPpfwZGM9A@mail.gmail.com
Backpatch-through: 14
Michael Paquier [Tue, 26 Oct 2021 04:57:18 +0000 (13:57 +0900)]
doc: Fix grammar in page of pg_receivewal
Introduced by
f61e1dd.
Author: Kyotaro Horiguchi
Discussion: https://postgr.es/m/
20211026.112304.
1962954080884317968.horikyota.ntt@gmail.com
Michael Paquier [Tue, 26 Oct 2021 02:16:06 +0000 (11:16 +0900)]
Fix overly-lax regex pattern in TAP test of READ_REPLICATION_SLOT
The case checking for a NULL output when a slot does not exist was
too lax, as it was passing for any output generated by the query. This
fixes the matching pattern to be what it should be, matching only on
"||".
Oversight in
b4ada4e.
Michael Paquier [Tue, 26 Oct 2021 00:30:37 +0000 (09:30 +0900)]
Allow pg_receivewal to stream from a slot's restart LSN
Prior to this patch, when running pg_receivewal, the streaming start
point would be the current location of the archives if anything is
found in the local directory where WAL segments are written, and
pg_receivewal would fall back to the current WAL flush location if there
are no archives, as of the result of an IDENTIFY_SYSTEM command.
If for some reason the WAL files from pg_receivewal were moved, it is
better to try a restart where we left at, which is the replication
slot's restart_lsn instead of skipping right to the current flush
location, to avoid holes in the WAL backed up. This commit changes
pg_receivewal to use the following sequence of methods to determine the
starting streaming LSN:
- Scan the local archives.
- Use the slot's restart_lsn, if supported by the backend and if a slot
is defined.
- Fallback to the current flush LSN as reported by IDENTIFY_SYSTEM.
To keep compatibility with older server versions, we only attempt to use
READ_REPLICATION_SLOT if the backend version is at least 15, and
fallback to the older behavior of streaming from the current flush
LSN if the command is not supported.
Some TAP tests are added to cover this feature.
Author: Ronan Dunklau
Reviewed-by: Kyotaro Horiguchi, Michael Paquier, Bharath Rupireddy
Discussion: https://postgr.es/m/
18708360.4lzOvYHigE@aivenronan
Thomas Munro [Mon, 25 Oct 2021 23:54:55 +0000 (12:54 +1300)]
Reject huge_pages=on if shared_memory_type=sysv.
It doesn't work (it could, but hasn't been implemented).
Back-patch to 12, where shared_memory_type arrived.
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Reviewed-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/
163271880203.22789.
1125998876173795966@wrigleys.postgresql.org
Robert Haas [Mon, 25 Oct 2021 20:30:12 +0000 (16:30 -0400)]
Initialize variable to placate compiler.
Per Nathan Bossart.
Discussion: http://postgr.es/m/
FECEE7FC-CB74-45A9-BB24-
89FEE52A9585@amazon.com
Robert Haas [Mon, 25 Oct 2021 15:51:57 +0000 (11:51 -0400)]
Report progress of startup operations that take a long time.
Users sometimes get concerned whe they start the server and it
emits a few messages and then doesn't emit any more messages for
a long time. Generally, what's happening is either that the
system is taking a long time to apply WAL, or it's taking a
long time to reset unlogged relations, or it's taking a long
time to fsync the data directory, but it's not easy to tell
which is the case.
To fix that, add a new 'log_startup_progress_interval' setting,
by default 10s. When an operation that is known to be potentially
long-running takes more than this amount of time, we'll log a
status update each time this interval elapses.
To avoid undesirable log chatter, don't log anything about WAL
replay when in standby mode.
Nitin Jadhav and Robert Haas, reviewed by Amul Sul, Bharath
Rupireddy, Justin Pryzby, Michael Paquier, and Álvaro Herrera.
Discussion: https://postgr.es/m/CA+TgmoaHQrgDFOBwgY16XCoMtXxsrVGFB2jNCvb7-ubuEe1MGg@mail.gmail.com
Discussion: https://postgr.es/m/CAMm1aWaHF7VE69572_OLQ+MgpT5RUiUDgF1x5RrtkJBLdpRj3Q@mail.gmail.com
Robert Haas [Thu, 30 Sep 2021 16:04:50 +0000 (12:04 -0400)]
Add enable_timeout_every() to fire the same timeout repeatedly.
enable_timeout_at() and enable_timeout_after() can still be used
when you want to fire a timeout just once.
Patch by me, per a suggestion from Tom Lane.
Discussion: http://postgr.es/m/
2992585.
1632938816@sss.pgh.pa.us
Discussion: http://postgr.es/m/CA+TgmoYqSF5sCNrgTom9r3Nh=at4WmYFD=gsV-omStZ60S0ZUQ@mail.gmail.com
Robert Haas [Mon, 25 Oct 2021 14:57:12 +0000 (10:57 -0400)]
Remove useless code from CreateReplicationSlot.
According to the comments, we initialize sendTimeLineIsHistoric
and sendTimeLine here for the benefit of WalSndSegmentOpen.
However, the only way that can happen is if logical_read_xlog_page
calls WALRead. And since logical_read_xlog_page initializes the
same global variables internally, we don't need to also do it here.
These initializations have been here since replication slots were
introduced in commit
858ec11858a914d4c380971985709b6d6b7dd6fc. They
were certainly useless at that time, too, because logical decoding
didn't yet exist then, and physical replication doesn't examine any
WAL at the time of slot creation. I haven't checked all the
intermediate versions, but I suspect there's no point at which
this code ever did anything useful.
To reduce future confusion, remove the code. Since there's no
functional defect, no back-patch.
Discussion: http://postgr.es/m/CA+TgmobSWzacEs+r6C-7DrOPDHoDar4i9gzxB3SCBr5qjnLmVQ@mail.gmail.com
Robert Haas [Mon, 25 Oct 2021 14:16:28 +0000 (10:16 -0400)]
StartupXLOG: Don't repeatedly disable/enable local xlog insertion.
All the code that runs in the startup process to write WAL records
before that's allowed generally is now consecutive, so there's no
reason to shut the facility to write WAL locally off and then turn
it on again three times in a row.
Unfortunately, this requires a slight kludge in the checkpointer,
which needs to separately enable writing WAL in order to write the
checkpoint record. Because that code might run in the same process
as StartupXLOG() if we are in single-user mode, we must save/restore
the state of the LocalXLogInsertAllowed flag. Hopefully, we'll be
able to eliminate this wart in further refactoring, but it's
not too bad anyway.
Amul Sul, with modifications by me.
Discussion: http://postgr.es/m/CAAJ_b97fysj6sRSQEfOHj-y8Jfd5uPqOgO74qast89B4WfD+TA@mail.gmail.com
Robert Haas [Mon, 25 Oct 2021 13:49:46 +0000 (09:49 -0400)]
StartupXLOG: Call CleanupAfterArchiveRecovery after XLogReportParameters.
This does a better job grouping related operations together, since
all of the WAL records that we need to write prior to allowing WAL
writes generally and written by a single uninterrupted stretch of code.
Since CleanupAfterArchiveRecovery() just (1) runs recovery_end_command,
(2) removes non-parent xlog files, and (3) archives any final partial
segment, this should be safe, because all of those things are pretty
much unrelated to the WAL record written by XLogReportParameters().
Amul Sul, per a suggestion from me
Discussion: http://postgr.es/m/CAAJ_b97fysj6sRSQEfOHj-y8Jfd5uPqOgO74qast89B4WfD+TA@mail.gmail.com
Heikki Linnakangas [Mon, 25 Oct 2021 06:30:49 +0000 (09:30 +0300)]
Clarify the logic in a few places in the new balanced merge code.
In selectnewtape(), use 'nOutputTapes' rather than 'nOutputRuns' in the
check for whether to start a new tape or to append a new run to an
existing tape. Until 'maxTapes' is reached, nOutputTapes is always equal
to nOutputRuns, so it doesn't change the logic, but it seems more logical
to compare # of tapes with # of tapes. Also, currently maxTapes is never
modified after the merging begins, but written this way, the code would
still work if it was. (Although the nOutputRuns == nOutputTapes assertion
would need to be removed and using nOutputRuns % nOutputTapes to
distribute the runs evenly across the tapes wouldn't do a good job
anymore).
Similarly in mergeruns(), change to USEMEM(state->tape_buffer_mem) to
account for the memory used for tape buffers. It's equal to availMem
currently, but tape_buffer_mem is more direct and future-proof. For
example, if we changed the logic to only allocate half of the remaining
memory to tape buffers, USEMEM(state->tape_buffer_mem) would still be
correct.
Coverity complained about these. Hopefully this patch helps it to
understand the logic better. Thanks to Tom Lane for initial analysis.
Michael Paquier [Sun, 24 Oct 2021 22:40:42 +0000 (07:40 +0900)]
Add replication command READ_REPLICATION_SLOT
The command is supported for physical slots for now, and returns the
type of slot, its restart_lsn and its restart_tli.
This will be useful for an upcoming patch related to pg_receivewal, to
allow the tool to be able to stream from the position of a slot, rather
than the last WAL position flushed by the backend (as reported by
IDENTIFY_SYSTEM) if the archive directory is found as empty, which would
be an advantage in the case of switching to a different archive
locations with the same slot used to avoid holes in WAL segment
archives.
Author: Ronan Dunklau
Reviewed-by: Kyotaro Horiguchi, Michael Paquier, Bharath Rupireddy
Discussion: https://postgr.es/m/
18708360.4lzOvYHigE@aivenronan
Tom Lane [Sun, 24 Oct 2021 16:38:26 +0000 (12:38 -0400)]
Fix minor memory leaks in pg_dump.
I found these by running pg_dump under "valgrind --leak-check=full".
The changes in flagInhIndexes() and getIndexes() replace allocation of
an array of which we use only some elements by individual allocations
of just the actually-needed objects. The previous coding wasted some
memory, but more importantly it confused valgrind's leak tracking.
collectComments() and collectSecLabels() remain major blots on
the valgrind report, because they don't PQclear their query
results, in order to avoid a lot of strdup's. That's a dubious
tradeoff, but I'll leave it alone here; an upcoming patch will
modify those functions enough to justify changing the tradeoff.
Andrew Dunstan [Sun, 24 Oct 2021 14:28:19 +0000 (10:28 -0400)]
Move Perl test modules to a better namespace
The five modules in our TAP test framework all had names in the top
level namespace. This is unwise because, even though we're not
exporting them to CPAN, the names can leak, for example if they are
exported by the RPM build process. We therefore move the modules to the
PostgreSQL::Test namespace. In the process PostgresNode is renamed to
Cluster, and TestLib is renamed to Utils. PostgresVersion becomes simply
PostgreSQL::Version, to avoid possible confusion about what it's the
version of.
Discussion: https://postgr.es/m/
aede93a4-7d92-ef26-398f-
5094944c2504@dunslane.net
Reviewed by Erik Rijkers and Michael Paquier
Noah Misch [Sun, 24 Oct 2021 01:36:38 +0000 (18:36 -0700)]
Fix CREATE INDEX CONCURRENTLY for the newest prepared transactions.
The purpose of commit
8a54e12a38d1545d249f1402f66c8cde2837d97c was to
fix this, and it sufficed when the PREPARE TRANSACTION completed before
the CIC looked for lock conflicts. Otherwise, things still broke. As
before, in a cluster having used CIC while having enabled prepared
transactions, queries that use the resulting index can silently fail to
find rows. It may be necessary to reindex to recover from past
occurrences; REINDEX CONCURRENTLY suffices. Fix this for future index
builds by making CIC wait for arbitrarily-recent prepared transactions
and for ordinary transactions that may yet PREPARE TRANSACTION. As part
of that, have PREPARE TRANSACTION transfer locks to its dummy PGPROC
before it calls ProcArrayClearTransaction(). Back-patch to 9.6 (all
supported versions).
Andrey Borodin, reviewed (in earlier versions) by Andres Freund.
Discussion: https://postgr.es/m/
01824242-AA92-4FE9-9BA7-
AEBAFFEA3D0C@yandex-team.ru
Noah Misch [Sun, 24 Oct 2021 01:36:38 +0000 (18:36 -0700)]
Avoid race in RelationBuildDesc() affecting CREATE INDEX CONCURRENTLY.
CIC and REINDEX CONCURRENTLY assume backends see their catalog changes
no later than each backend's next transaction start. That failed to
hold when a backend absorbed a relevant invalidation in the middle of
running RelationBuildDesc() on the CIC index. Queries that use the
resulting index can silently fail to find rows. Fix this for future
index builds by making RelationBuildDesc() loop until it finishes
without accepting a relevant invalidation. It may be necessary to
reindex to recover from past occurrences; REINDEX CONCURRENTLY suffices.
Back-patch to 9.6 (all supported versions).
Noah Misch and Andrey Borodin, reviewed (in earlier versions) by Andres
Freund.
Discussion: https://postgr.es/m/
20210730022548.GA1940096@gust.leadboat.com
Michael Paquier [Sat, 23 Oct 2021 05:43:03 +0000 (14:43 +0900)]
doc: Describe calculation method of streaming start for pg_receivewal
The documentation was imprecise about the starting LSN used for WAL
streaming if nothing can be found in the local archive directory
defined with the pg_receivewal command, so be more talkative on this
matter.
Extracted from a larger patch by the same author.
Author: Ronan Dunklau, Michael Paquier
Discussion: https://postgr.es/m/
18708360.4lzOvYHigE@aivenronan
Backpatch-through: 10
Tom Lane [Fri, 22 Oct 2021 21:19:03 +0000 (17:19 -0400)]
In pg_dump, use simplehash.h to look up dumpable objects by OID.
Create a hash table that indexes dumpable objects by CatalogId
(that is, catalog OID + object OID). Use this to replace the
former catalogIdMap array, as well as various other single-
catalog index arrays, and also the extension membership map.
In principle this should be faster for databases with many objects,
since lookups are now O(1) not O(log N). However, it seems that these
lookups are pretty much negligible in context, so that no overall
performance change can be measured. But having only one lookup
data structure to maintain makes the code simpler and more flexible,
so let's do it anyway.
Discussion: https://postgr.es/m/
2595220.
1634855245@sss.pgh.pa.us
Tom Lane [Fri, 22 Oct 2021 20:43:38 +0000 (16:43 -0400)]
Fix frontend version of sh_error() in simplehash.h.
The code does not expect sh_error() to return, but the patch
that made this header usable in frontend didn't get that memo.
While here, plaster unlikely() on the tests that decide whether
to invoke sh_error(), and add our standard copyright notice.
Noted by Andres Freund. Back-patch to v13 where this frontend
support came in.
Discussion: https://postgr.es/m/
0D54435C-1199-4361-9D74-
2FBDCF8EA164@anarazel.de
Tom Lane [Fri, 22 Oct 2021 19:22:25 +0000 (15:22 -0400)]
pg_dump: fix mis-dumping of non-global default privileges.
Non-global default privilege entries should be dumped as-is,
not made relative to the default ACL for their object type.
This would typically only matter if one had revoked some
on-by-default privileges in a global entry, and then wanted
to grant them again in a non-global entry.
Per report from Boris Korzun. This is an old bug, so back-patch
to all supported branches.
Neil Chen, test case by Masahiko Sawada
Discussion: https://postgr.es/m/
111621616618184@mail.yandex.ru
Discussion: https://postgr.es/m/CAA3qoJnr2+1dVJObNtfec=qW4Z0nz=A9+r5bZKoTSy5RDjskMw@mail.gmail.com
Andrew Dunstan [Fri, 22 Oct 2021 13:11:52 +0000 (09:11 -0400)]
Add module build directory to the PATH for TAP tests
For non-MSVC builds this is make's $(CURDIR), while for MSVC builds it
is $topdir/$Config/$module. The directory is added as the second element
in the PATH, so that the install location takes precedence, but the
added PATH element takes precedence over the rest of the PATH.
The reason for this is to allow tests to find built products that are
not installed, such as the libpq_pipeline test driver.
The libpq_pipeline test is adjusted to take advantage of this.
Based on a suggestion from Andres Freund.
Backpatch to release 14.
Discussion: https://postgr.es/m/
4941f5a5-2d50-1a0e-6701-
14c5fefe92d6@dunslane.net
Tom Lane [Thu, 21 Oct 2021 21:08:53 +0000 (17:08 -0400)]
Doc: clarify a critical and undocumented aspect of simplehash.h.
I just got burnt by trying to use pg_malloc instead of pg_malloc0
with this. Save the next hacker some time by not leaving this
API detail undocumented.
Daniel Gustafsson [Thu, 21 Oct 2021 08:28:50 +0000 (10:28 +0200)]
Fix SSL tests on 32-bit Perl
The certificate serial number generation was changed in
b4c4a00ea to
use the current timestamp. The testharness must thus interrogate the
cert for the serialnumber using "openssl x509" which emits the serial
in hex format. Converting the serial to integer format to match whats
in pg_stat_ssl requires a 64-bit capable Perl. This adds a fallback
to checking for an integer when the tests with a 32-bit Perl.
Per failure on buildfarm member prairiedog.
Discussion: https://postgr.es/m/
0D295F43-806D-4B3F-AB98-
F941A19E0271@yesql.se
Amit Kapila [Thu, 21 Oct 2021 02:31:25 +0000 (08:01 +0530)]
Remove unused wait events.
Commit
464824323e introduced the wait events which were neither used by
that commit nor by follow-up commits for that work.
Author: Masahiro Ikeda
Backpatch-through: 14, where it was introduced
Discussion: https://postgr.es/m/
ff077840-3ab2-04dd-bbe4-
4f5dfd2ad481@oss.nttdata.com
Michael Paquier [Thu, 21 Oct 2021 01:39:01 +0000 (10:39 +0900)]
Fix corruption of pg_shdepend when copying deps from template database
Using for a new database a template database with shared dependencies
that need to be copied over was causing a corruption of pg_shdepend
because of an off-by-one computation error of the index number used for
the values inserted with a slot.
Issue introduced by
e3931d0. Monitoring the rest of the code, there are
no similar mistakes.
Reported-by: Sven Klemm
Author: Aleksander Alekseev
Reviewed-by: Daniel Gustafsson, Michael Paquier
Discussion: https://postgr.es/m/CAJ7c6TP0AowkUgNL6zcAK-s5HYsVHVBRWfu69FRubPpfwZGM9A@mail.gmail.com
Backpatch-through: 14
Tom Lane [Wed, 20 Oct 2021 22:44:37 +0000 (18:44 -0400)]
Improve pg_regress.c's infrastructure for issuing psql commands.
Support issuing more than one "-c command" switch to a single
psql invocation. This allows combining some things that formerly
required two or more backend launches into a single session.
In particular, we can issue DROP DATABASE as one of the -c commands
without getting "DROP DATABASE cannot run inside a transaction block".
In addition to reducing the number of sessions needed, this patch
also suppresses "NOTICE: database "foo" does not exist, skipping"
chatter that was formerly generated during pg_regress's DROP DATABASE
(or ROLE) IF NOT EXISTS calls. That moves us another step closer
to the ideal of not seeing any messages during successful build/test.
This also eliminates some hard-coded restrictions on the length of
the commands issued. I don't think we were anywhere near hitting
those, but getting rid of the limit is comforting.
Patch by me, but thanks to Nathan Bossart for starting the discussion.
Discussion: https://postgr.es/m/
DCBAE0E4-BD56-482F-8A70-
7FD0DC0860BE@amazon.com
Alvaro Herrera [Wed, 20 Oct 2021 16:05:42 +0000 (13:05 -0300)]
Protect against collation variations in test
Discussion: https://postgr.es/m/YW/MYdSRQZtPFBWR@paquier.xyz
Michael Paquier [Wed, 20 Oct 2021 07:48:24 +0000 (16:48 +0900)]
Fix build of MSVC with OpenSSL 3.0.0
The build scripts of Visual Studio would fail to detect properly a 3.0.0
build as the check on the second digit was failing. This is adjusted
where needed, allowing the builds to complete. Note that the MSIs of
OpenSSL mentioned in the documentation have not changed any library
names for Win32 and Win64, making this change straight-forward.
Reported-by: htalaco, via github
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/YW5XKYkq6k7OtrFq@paquier.xyz
Backpatch-through: 9.6
Alvaro Herrera [Tue, 19 Oct 2021 22:08:45 +0000 (19:08 -0300)]
Ensure correct lock level is used in ALTER ... RENAME
Commit
1b5d797cd4f7 intended to relax the lock level used to rename
indexes, but inadvertently allowed *any* relation to be renamed with a
lowered lock level, as long as the command is spelled ALTER INDEX.
That's undesirable for other relation types, so retry the operation with
the higher lock if the relation turns out not to be an index.
After this fix, ALTER INDEX <sometable> RENAME will require access
exclusive lock, which it didn't before.
Author: Nathan Bossart <bossartn@amazon.com>
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reported-by: Onder Kalaci <onderk@microsoft.com>
Discussion: https://postgr.es/m/PH0PR21MB1328189E2821CDEC646F8178D8AE9@PH0PR21MB1328.namprd21.prod.outlook.com
Tom Lane [Tue, 19 Oct 2021 21:22:22 +0000 (17:22 -0400)]
pg_dump: Reorganize getTables()
Along the same lines as
047329624,
ed2c7f65b and
daa9fe8a5, reduce
code duplication by having just one copy of the parts of the query
that are the same across all server versions; and make the
conditionals control the smallest possible amount of code.
This also gets rid of the confusing assortment of different ways
to accomplish the same result that we had here before.
While at it, make sure all three relevant parts of the function
list the fields in the same order. This is just neatnik-ism,
of course.
Discussion: https://postgr.es/m/
1240992.
1634419055@sss.pgh.pa.us
Andres Freund [Tue, 19 Oct 2021 17:14:49 +0000 (10:14 -0700)]
Adapt src/test/ldap/t/001_auth.pl to work with openldap 2.5.
ldapsearch's deprecated -h/-p arguments were removed, need to use -H now -
which has been around for over 20 years.
As perltidy insists on reflowing the parameters anyway, change order and
"phrasing" to yield a less confusing layout (per suggestion from Tom Lane).
Discussion: https://postgr.es/m/
20211009233850.wvr6apcrw2ai6cnj@alap3.anarazel.de
Backpatch: 11-, where the tests were added.
Daniel Gustafsson [Tue, 19 Oct 2021 18:11:42 +0000 (20:11 +0200)]
Refactor the sslfiles Makefile target for ease of use
The Makefile handling of certificate and keypairs used for TLS testing
had become quite difficult to work with. Adding a new cert without the
need to regenerate everything was too complicated. This patch refactors
the sslfiles make target such that adding a new certificate requires
only adding a .config file, adding it to the top of the Makefile, and
running make sslfiles.
Improvements:
- Interfile dependencies should be fixed, with the exception of the CRL
dirs.
- New certificates have serial numbers based on the current time,
reducing the chance of collision.
- The CA index state is created on demand and cleaned up automatically
at the end of the Make run.
- *.config files are now self-contained; one certificate needs one
config file instead of two.
- Duplication is reduced, and along with it some unneeded code (and
possible copy-paste errors).
- all configuration files underneath the conf/ directory.
The target is moved to its own makefile in order to avoid colliding
with global make settings.
Author: Jacob Champion <pchampion@vmware.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/
d15a9838344ba090e09fd866abf913584ea19fb7.camel@vmware.com
Tom Lane [Tue, 19 Oct 2021 17:54:45 +0000 (13:54 -0400)]
Fix assignment to array of domain over composite.
An update such as "UPDATE ... SET fld[n].subfld = whatever"
failed if the array elements were domains rather than plain
composites. That's because isAssignmentIndirectionExpr()
failed to cope with the CoerceToDomain node that would appear
in the expression tree in this case. The result would typically
be a crash, and even if we accidentally didn't crash, we'd not
correctly preserve other fields of the same array element.
Per report from Onder Kalaci. Back-patch to v11 where arrays of
domains came in.
Discussion: https://postgr.es/m/PH0PR21MB132823A46AA36F0685B7A29AD8BD9@PH0PR21MB1328.namprd21.prod.outlook.com
Tom Lane [Tue, 19 Oct 2021 15:35:15 +0000 (11:35 -0400)]
Remove bogus assertion in transformExpressionList().
I think when I added this assertion (in commit
8f889b108), I was only
thinking of the use of transformExpressionList at top level of INSERT
and VALUES. But it's also called by transformRowExpr(), which can
certainly occur in an UPDATE targetlist, so it's inappropriate to
suppose that p_multiassign_exprs must be empty. Besides, since the
input is not expected to contain ResTargets, there's no reason it
should contain MultiAssignRefs either. Hence this code need not
be concerned about the state of p_multiassign_exprs, and we should
just drop the assertion.
Per bug #17236 from ocean_li_996. It's been wrong for years,
so back-patch to all supported branches.
Discussion: https://postgr.es/m/17236-
3210de9bcba1d7ca@postgresql.org
Daniel Gustafsson [Tue, 19 Oct 2021 10:59:54 +0000 (12:59 +0200)]
Fix bug in TOC file error message printing
If the blob TOC file cannot be parsed, the error message was failing
to print the filename as the variable holding it was shadowed by the
destination buffer for parsing. When the filename fails to parse,
the error will print an empty string:
./pg_restore -d foo -F d dump
pg_restore: error: invalid line in large object TOC file "": ..
..instead of the intended error message:
./pg_restore -d foo -F d dump
pg_restore: error: invalid line in large object TOC file "dump/blobs.toc": ..
Fix by renaming both variables as the shared name was too generic to
store either and still convey what the variable held.
Backpatch all the way down to 9.6.
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/
A2B151F5-B32B-4F2C-BA4A-
6870856D9BDE@yesql.se
Backpatch-through: 9.6
Daniel Gustafsson [Tue, 19 Oct 2021 10:59:50 +0000 (12:59 +0200)]
Fix sscanf limits in pg_basebackup and pg_dump
Make sure that the string parsing is limited by the size of the
destination buffer.
In pg_basebackup the available values sent from the server
is limited to two characters so there was no risk of overflow.
In pg_dump the buffer is bounded by MAXPGPATH, and thus the limit
must be inserted via preprocessor expansion and the buffer increased
by one to account for the terminator. There is no risk of overflow
here, since in this case, the buffer scanned is smaller than the
destination buffer.
Backpatch the pg_basebackup fix to 11 where it was introduced, and
the pg_dump fix all the way down to 9.6.
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/
B14D3D7B-F98C-4E20-9459-
C122C67647FB@yesql.se
Backpatch-through: 11 and 9.6
Michael Paquier [Tue, 19 Oct 2021 02:03:52 +0000 (11:03 +0900)]
Block ALTER INDEX/TABLE index_name ALTER COLUMN colname SET (options)
The grammar of this command run on indexes with column names has always
been authorized by the parser, and it has never been documented.
Since
911e702, it is possible to define opclass parameters as of CREATE
INDEX, which actually broke the old case of ALTER INDEX/TABLE where
relation-level parameters n_distinct and n_distinct_inherited could be
defined for an index (see
76a47c0 and its thread where this point has
been touched, still remained unused). Attempting to do that in v13~
would cause the index to become unusable, as there is a new dedicated
code path to load opclass parameters instead of the relation-level ones
previously available. Note that it is possible to fix things with a
manual catalog update to bring the relation back online.
This commit disables this command for now as the use of column names for
indexes does not make sense anyway, particularly when it comes to index
expressions where names are automatically computed. One way to properly
support this case properly in the future would be to use column numbers
when it comes to indexes, in the same way as ALTER INDEX .. ALTER COLUMN
.. SET STATISTICS.
Partitioned indexes were already blocked, but not indexes. Some tests
are added for both cases.
There was some code in ANALYZE to enforce n_distinct to be used for an
index expression if the parameter was defined, but just remove it for
now until/if there is support for this (note that index-level parameters
never had support in pg_dump either, previously), so this was just dead
code.
Reported-by: Matthijs van der Vleuten
Author: Nathan Bossart, Michael Paquier
Reviewed-by: Vik Fearing, Dilip Kumar
Discussion: https://postgr.es/m/17220-
15d684c6c2171a83@postgresql.org
Backpatch-through: 13
Alvaro Herrera [Mon, 18 Oct 2021 22:08:25 +0000 (19:08 -0300)]
Invalidate partitions of table being attached/detached
Failing to do that, any direct inserts/updates of those partitions
would fail to enforce the correct constraint, that is, one that
considers the new partition constraint of their parent table.
Backpatch to 10.
Reported by: Hou Zhijie <houzj.fnst@fujitsu.com>
Author: Amit Langote <amitlangote09@gmail.com>
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Nitin Jadhav <nitinjadhavpostgres@gmail.com>
Reviewed-by: Pavel Borisov <pashkin.elfe@gmail.com>
Discussion: https://postgr.es/m/OS3PR01MB5718DA1C4609A25186D1FBF194089%40OS3PR01MB5718.jpnprd01.prod.outlook.com
Heikki Linnakangas [Mon, 18 Oct 2021 17:42:10 +0000 (20:42 +0300)]
Fix parallel sort, broken by the balanced merge patch.
The code for initializing the tapes on each merge iteration was skipped
in a parallel worker. I put the !WORKER(state) check in wrong place while
rebasing the patch.
That caused failures in the index build in 'multiple-row-versions'
isolation test, in multiple buildfarm members. On my laptop it was easier
to reproduce by building an index on a larger table, so that you got a
parallel sort more reliably.
Heikki Linnakangas [Mon, 18 Oct 2021 14:02:01 +0000 (17:02 +0300)]
Fix duplicate typedef LogicalTape.
To make buildfarm member locust happy.
Heikki Linnakangas [Mon, 18 Oct 2021 13:15:44 +0000 (16:15 +0300)]
Fix format modifier used in elog.
The previous commit
65014000b3 changed the variable passed to elog
from an int64 to a size_t variable, but neglected to change the modifier
in the format string accordingly.
Per failure on buildfarm member lapwing.
Heikki Linnakangas [Mon, 18 Oct 2021 11:33:42 +0000 (14:33 +0300)]
Replace polyphase merge algorithm with a simple balanced k-way merge.
The advantage of polyphase merge is that it can reuse the input tapes as
output tapes efficiently, but that is irrelevant on modern hardware, when
we can easily emulate any number of tape drives. The number of input tapes
we can/should use during merging is limited by work_mem, but output tapes
that we are not currently writing to only cost a little bit of memory, so
there is no need to skimp on them.
This makes sorts that need multiple merge passes faster.
Discussion: https://www.postgresql.org/message-id/
420a0ec7-602c-d406-1e75-
1ef7ddc58d83%40iki.fi
Reviewed-by: Peter Geoghegan, Zhihong Yu, John Naylor
Heikki Linnakangas [Mon, 18 Oct 2021 11:30:00 +0000 (14:30 +0300)]
Refactor LogicalTapeSet/LogicalTape interface.
All the tape functions, like LogicalTapeRead and LogicalTapeWrite, now
take a LogicalTape as argument, instead of LogicalTapeSet+tape number.
You can create any number of LogicalTapes in a single LogicalTapeSet, and
you don't need to decide the number upfront, when you create the tape set.
This makes the tape management in hash agg spilling in nodeAgg.c simpler.
Discussion: https://www.postgresql.org/message-id/
420a0ec7-602c-d406-1e75-
1ef7ddc58d83%40iki.fi
Reviewed-by: Peter Geoghegan, Zhihong Yu, John Naylor
Michael Paquier [Mon, 18 Oct 2021 02:55:42 +0000 (11:55 +0900)]
Reset properly snapshot export state during transaction abort
During a replication slot creation, an ERROR generated in the same
transaction as the one creating a to-be-exported snapshot would have
left the backend in an inconsistent state, as the associated static
export snapshot state was not being reset on transaction abort, but only
on the follow-up command received by the WAL sender that created this
snapshot on replication slot creation. This would trigger inconsistency
failures if this session tried to export again a snapshot, like during
the creation of a replication slot.
Note that a snapshot export cannot happen in a transaction block, so
there is no need to worry resetting this state for subtransaction
aborts. Also, this inconsistent state would very unlikely show up to
users. For example, one case where this could happen is an
out-of-memory error when building the initial snapshot to-be-exported.
Dilip found this problem while poking at a different patch, that caused
an error in this code path for reasons unrelated to HEAD.
Author: Dilip Kumar
Reviewed-by: Michael Paquier, Zhihong Yu
Discussion: https://postgr.es/m/CAFiTN-s0zA1Kj0ozGHwkYkHwa5U0zUE94RSc_g81WrpcETB5=w@mail.gmail.com
Backpatch-through: 9.6
Michael Paquier [Mon, 18 Oct 2021 00:51:21 +0000 (09:51 +0900)]
Fix portability issues in new TAP tests of psql
The tests added by
c0280bc and
d9ddc50 in 001_basic.pl have introduced
commands calling directly psql, making them sensitive to the
environment. One issue was that those commands forgot -X to not use a
local .psqlrc, causing all those tests to fail if psql cannot properly
parse this file.
TAP tests should be designed so as they run in an isolated fashion,
without any dependency on the environment where they are run. As
PostgresNode::psql gives already all the facilities those new tests
need, switch to that instead of calling plain psql commands where
interactions with a backend are needed. The test is slightly refactored
to be able to check after the expected patterns of stdout and stderr,
keeping the same amount of coverage as previously.
Reported-by: Peter Geoghegan
Discussion: https://postgr.es/m/CAH2-Wzn8ftvcDPwomn+y04JJzbT=TG7TN=QsmSEATUOW-ZuvQQ@mail.gmail.com
Tom Lane [Sat, 16 Oct 2021 19:02:55 +0000 (15:02 -0400)]
Avoid core dump in pg_dump when dumping from pre-8.3 server.
Commit
f0e21f2f6 missed adding a tgisinternal output column
to getTriggers' query for pre-8.3 servers. Back-patch to v11,
like that commit.
Tom Lane [Sat, 16 Oct 2021 16:23:57 +0000 (12:23 -0400)]
Make pg_dump acquire lock on partitioned tables that are to be dumped.
It was clearly the intent to do so all along, but the original coding
fat-fingered this by checking the wrong array element. We fixed it
in passing in
403a3d91c, but that later got reverted, and we forgot
to keep this bug fix.
Most of the time this'd be relatively harmless, since once we lock
any of the partitioned table's leaf partitions, that would suffice
to prevent major DDL on the partitioned table itself. However, a
childless partitioned table would get dumped with no relevant lock
whatsoever, possibly allowing dump failure or inconsistent output.
Unlike
403a3d91c, there are no versioning concerns, since every server
version that has partitioned tables will allow you to lock one.
Back-patch to v10 where partitioned tables were introduced.
Discussion: https://postgr.es/m/
1018205.
1634346327@sss.pgh.pa.us
Peter Geoghegan [Fri, 15 Oct 2021 22:25:20 +0000 (15:25 -0700)]
Remove obsolete nbtree deduplication comments.
Follow up to commit
2903f140.