Peter Geoghegan [Sun, 3 Oct 2021 00:12:59 +0000 (17:12 -0700)]
Enable deduplication in system catalog indexes.
The "equality implies image equality" opclass infrastructure disallowed
deduplication in system catalog indexes and TOAST indexes before now.
That seemed like the right approach back when the infrastructure was
added by commit
612a1ab7, since ALTER INDEX cannot set deduplicate_items
to 'off' (due to an old implementation restriction). But that decision
now seems arbitrary at best. Remove special case handling implementing
this policy.
No catversion bump, since existing catalog indexes will still work.
Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-Wz=rYQHFaJ3WYBdK=xgwxKzaiGMSSrh-ZCREa-pS-7Zjew@mail.gmail.com
Tom Lane [Sat, 2 Oct 2021 20:05:42 +0000 (16:05 -0400)]
Update our mapping of Windows time zone names using CLDR info.
This corrects a bunch of entries in win32_tzmap[], and adds a few
new ones, based on the CLDR project's windowsZones.xml file.
Non-cosmetic changes fall into four main categories:
* Flat-out errors:
US/Aleutan doesn't exist
America/Salvador doesn't exist
Asia/Baku is wrong for Yerevan
Asia/Dhaka (Bangladesh) is wrong for Astana (Kazakhstan)
Europe/Bucharest is wrong for Chisinau
America/Mexico_City is wrong for Chetumal
America/Buenos_Aires is wrong for Cayenne
America/Caracas has its own zone, so poor fit for La Paz
US/Eastern is wrong for Haiti
US/Eastern is wrong for Indiana (East)
Asia/Karachi is wrong for Tashkent
Etc/UTC+12 doesn't exist
Signs of Etc/GMT zones were backwards
* Judgment calls:
(These changes follow CLDR's choices, except for the first one)
Use Europe/London for "Greenwich Standard Time", since that seems much
more likely than Africa/Casablanca to be what people will think that
zone name means. CLDR has Atlantic/Reykjavik here, but that's no better.
Asia/Shanghai seems a better fit than Hong Kong for "China Standard
Time".
Europe/Sarajevo is now a link to Belgrade, ie "Central Europe Standard
Time"; so use Warsaw for "Central European Standard Time".
America/Sao_Paulo seems more representative than Araguaina for
"E. South America Standard Time".
Africa/Johannesburg seems more representative than Harare for
"South Africa Standard Time".
* New Windows zone names:
"Israel Standard Time"
"Kaliningrad Standard Time"
"Russia Time Zone N" for various N
"Singapore Standard Time"
"South Sudan Standard Time"
"W. Central Africa Standard Time"
"West Bank Standard Time"
"Yukon Standard Time"
Some of these replace older spellings, but I kept the older spellings
too in case our code runs on a machine with the older data.
* Replace aliases (tzdb Links) with underlying city-named zones:
(This tracks tzdb's longstanding practice, and reduces inconsistency
with the rest of the entries, as well as with CLDR.)
US/Alaska
Asia/Kuwait
Asia/Muscat
Canada/Atlantic
Australia/Canberra
Canada/Saskatchewan
US/Central
US/Eastern
US/Hawaii
US/Mountain
Canada/Newfoundland
US/Pacific
Back-patch to all supported branches, as is our usual practice for
time zone data updates.
Discussion: https://postgr.es/m/
3266414.
1633045628@sss.pgh.pa.us
Tom Lane [Sat, 2 Oct 2021 20:05:10 +0000 (16:05 -0400)]
Re-alphabetize the win32_tzmap[] array.
The original intent seems to have been to sort case-insensitively
by the Windows zone name, but various changes over the years did
not get that memo. This commit just moves a few entries to
restore exact alphabetic order, to ease comparison to the outputs
of processing scripts.
Back-patch to all supported branches, as is our usual practice for
time zone data updates.
Discussion: https://postgr.es/m/
3266414.
1633045628@sss.pgh.pa.us
Michael Paquier [Sat, 2 Oct 2021 08:40:13 +0000 (17:40 +0900)]
pg_stat_statements: Add some tests for older versions still usable
When the newest version is loaded, the backend would load objects from
the oldest complete SQL file (here 1.4) and then update to the latest
version with transition scripts (up to 1.9 currently). This provides
some coverage for upgrades of pg_stat_statements, but there is no test
to show how things have changed across each version.
This adds a couple of tests for the upgrade paths using objects from
each version supported, stressing the objects whose behaviors have
changed across each version supported.
Author: Erica Zhang
Reviewed-by: Julien Rouhaud, Michael Paquier
Discussion: https://postgr.es/m/tencent_BBA974AFF61379F2345E782FD6C55891950A@qq.com
Andres Freund [Thu, 30 Sep 2021 01:02:32 +0000 (18:02 -0700)]
Reference test binary using TESTDIR in 001_libpq_pipeline.pl.
The previous approach didn't really work on windows, due to the PATH separator
being ';' not ':'. Instead of making the PATH change more complicated,
reference the binary using the TESTDIR environment.
Reported-By: Andres Freund <andres@anarazel.de>
Suggested-By: Andrew Dunstan <andrew@dunslane.net>
Discussion: https://postgr.es/m/
20210930214040.odkdd42vknvzifm6@alap3.anarazel.de
Backpatch: 14-, where the test was introduced.
Alvaro Herrera [Fri, 1 Oct 2021 21:29:18 +0000 (18:29 -0300)]
Error out if SKIP LOCKED and WITH TIES are both specified
Both bugs #16676[1] and #17141[2] illustrate that the combination of
SKIP LOCKED and FETCH FIRST WITH TIES break expectations when it comes
to rows returned to other sessions accessing the same row. Since this
situation is detectable from the syntax and hard to fix otherwise,
forbid for now, with the potential to fix in the future.
[1] https://postgr.es/m/16676-
fd62c3c835880da6@postgresql.org
[2] https://postgr.es/m/17141-
913d78b9675aac8e@postgresql.org
Backpatch-through: 13, where WITH TIES was introduced
Author: David Christensen <david.christensen@crunchydata.com>
Discussion: https://postgr.es/m/CAOxo6XLPccCKru3xPMaYDpa+AXyPeWFs+SskrrL+HKwDjJnLhg@mail.gmail.com
Alvaro Herrera [Fri, 1 Oct 2021 21:03:11 +0000 (18:03 -0300)]
Remove unstable, unnecessary test; fix typo
Commit
ff9f111bce24 added some test code that's unportable and doesn't
add meaningful coverage. Remove it rather than try and get it to work
everywhere.
While at it, fix a typo in a log message added by the aforementioned
commit.
Backpatch to 14.
Discussion: https://postgr.es/m/
3000074.
1632947632@sss.pgh.pa.us
Daniel Gustafsson [Fri, 1 Oct 2021 20:47:05 +0000 (22:47 +0200)]
Fix memory leak in pg_hmac
The intermittent h buffer was not freed, causing it to leak. Backpatch
through 14 where HMAC was refactored to the current API.
Author: Sergey Shinderuk <s.shinderuk@postgrespro.ru>
Discussion: https://postgr.es/m/
af07e620-7e28-a742-4637-
2bc44aa7c2be@postgrespro.ru
Backpatch-through: 14
Tom Lane [Fri, 1 Oct 2021 18:59:35 +0000 (14:59 -0400)]
Avoid believing incomplete MCV-only stats in get_variable_range().
get_variable_range() would incautiously believe that statistics
containing only an MCV list are sufficient to derive a range estimate.
That's okay for an enum-like column that contains only MCVs, but
otherwise the estimate could be pretty bad. Make it report that the
range is indeterminate unless the MCVs plus nullfrac account for
the whole table.
I don't think this needs a dedicated test case, since a quick code
coverage check verifies that the existing regression tests traverse
all the alternatives. There is room to doubt that a future-proof
test case could be built anyway, given that the submitted example
accidentally doesn't fail before v11.
Per bug #17207 from Simon Perepelitsa. Back-patch to v10.
In principle this has been broken all along, but I'm hesitant to
make such changes in 9.6, since if anyone is unhappy with 9.6.24's
behavior there will be no second chance to fix it.
Discussion: https://postgr.es/m/17207-
5265aefa79e333b4@postgresql.org
Tom Lane [Fri, 1 Oct 2021 15:10:12 +0000 (11:10 -0400)]
Fix Portal snapshot tracking to handle subtransactions properly.
Commit
84f5c2908 forgot to consider the possibility that
EnsurePortalSnapshotExists could run inside a subtransaction with
lifespan shorter than the Portal's. In that case, the new active
snapshot would be popped at the end of the subtransaction, leaving
a dangling pointer in the Portal, with mayhem ensuing.
To fix, make sure the ActiveSnapshot stack entry is marked with
the same subtransaction nesting level as the associated Portal.
It's certainly safe to do so since we won't be here at all unless
the stack is empty; hence we can't create an out-of-order stack.
Let's also apply this logic in the case where PortalRunUtility
sets portalSnapshot, just to be sure that path can't cause similar
problems. It's slightly less clear that that path can't create
an out-of-order stack, so add an assertion guarding it.
Report and patch by Bertrand Drouvot (with kibitzing by me).
Back-patch to v11, like the previous commit.
Discussion: https://postgr.es/m/
ff82b8c5-77f4-3fe7-6028-
fcf3303e82dd@amazon.com
Amit Kapila [Fri, 1 Oct 2021 02:47:56 +0000 (08:17 +0530)]
Doc: Move pg_stat_replication_slots view to "Collected Statistics Views" section.
Commit
9868167500 added pg_stat_replication_slots view to monitor
ReorderBuffer stats but mistakenly added it under
"Dynamic Statistics Views" section in the docs whereas it belongs to
"Collected Statistics Views" section.
Author: Amit Kapila
Reviewed-by: Masahiko Sawada
Backpatch-through: 14, where it was introduced
Discussion: https://postgr.es/m/CAA4eK1Kb5ur=OC-G4cAsqPOjoVe+S8LNw1WmUY8Owasjk8o5WQ@mail.gmail.com
David Rowley [Fri, 1 Oct 2021 02:09:49 +0000 (15:09 +1300)]
Ensure interleaved_parts field is always initialized
This field was recently added in
db632fbca, however that commit missed one
place where it should have initialized the new field to NULL. The missed
location is where the PartitionBoundInfo is created for partition-wise
join relations. Technically there could be interleaved partitions in a
partition-wise join relation, but currently the only optimization we use
this field for only does so for base rels and other member rels. So just
document that we don't populate this field for join rels.
Reported-by: Amit Langote
Author: Amit Langote, David Rowley
Reviewed-by: Amit Langote, David Rowley
Discussion: https://postgr.es/m/CA+HiwqE76Rps24kwHsd2Cr82Ua07tJC9t9reG0c7ScX9n_xrEA@mail.gmail.com
Tom Lane [Thu, 30 Sep 2021 20:23:10 +0000 (16:23 -0400)]
Remove gratuitous environment dependency in 002_types.pl test.
Computing related timestamps by subtracting "N days" is sensitive
to the prevailing timezone, since we interpret that as "same local
time on the N'th prior day". Even though the intervals in question
are only two to four days, through remarkable bad luck they managed
to cross the end of Ramadan in 2014, causing the test's output to
change if timezone is set to Africa/Casablanca. (Maybe in other
Muslim areas as well; I didn't check.) There's absolutely no reason
for this test to exercise interval subtraction, so just get rid of
that and use plain timestamptz constants representing the intended
values.
Per report from Andres Freund. Back-patch to v10 where this test
script came in.
Discussion: https://postgr.es/m/
20210930183641.7lh4jhvpipvromca@alap3.anarazel.de
Tom Lane [Thu, 30 Sep 2021 18:16:08 +0000 (14:16 -0400)]
Treat ETIMEDOUT as indicating a non-recoverable connection failure.
Add ETIMEDOUT to ALL_CONNECTION_FAILURE_ERRNOS' list of "errnos that
identify hard failure of a previously-established network connection".
While one could imagine that this is sometimes recoverable, the same
could be said of other entries such as ENETDOWN.
In support of this, handle ETIMEDOUT on par with other socket errors
in relevant infrastructure, such as TranslateSocketError().
(I made a couple of cosmetic adjustments in TranslateSocketError(),
too.) The code now assumes that ETIMEDOUT is defined everywhere,
which it should be given that POSIX has required it since SUSv2.
Perhaps this should be back-patched, but I'm hesitant to do so given
the lack of previous complaints, and the hazard that there's a small
ABI break on Windows from redefining the symbol. Even if we decide
to do that, it'd be prudent to let this bake awhile in HEAD first.
Jelte Fennema
Discussion: https://postgr.es/m/AM5PR83MB01782BFF2978505F6D6C559AF7AA9@AM5PR83MB0178.EURPRD83.prod.outlook.com
Alvaro Herrera [Thu, 30 Sep 2021 13:01:03 +0000 (10:01 -0300)]
Repair two portability oversights of new test
First, as pointed out by Tom Lane and Michael Paquier, I failed to
realize that Windows' PostgresNode needs an extra pg_hba.conf line
(added by PostgresNode->set_replication_conf, called internally by
->init() when 'allows_streaming=>1' is given -- but I purposefully
omitted that). I think a good fix should be to have nodes with only
'has_archiving=>1' set up for replication too, but that's a bigger
discussion. Fix it by calling ->set_replication_conf, which is not
unprecedented, as pointed out by Andrew Dunstan.
I also forgot to uncomment a ->finish() call for a pumpable IPC::Run
file descriptor. Apparently this is innocuous in almost all platforms.
Backpatch to 14. The older branches were added this file too, but not
this particular part of the test.
Discussion: https://postgr.es/m/
3000074.
1632947632@sss.pgh.pa.us
Discussion: https://postgr.es/m/YVT7qwhR8JmC2kfz@paquier.xyz
Peter Eisentraut [Wed, 29 Sep 2021 21:16:00 +0000 (23:16 +0200)]
psql: Add various tests
Add tests for psql features
- AUTOCOMMIT
- ON_ERROR_ROLLBACK
- ECHO errors
Reviewed-by: Fabien COELHO <coelho@cri.ensmp.fr>
Discussion: https://www.postgresql.org/message-id/
6954328d-96f2-77f7-735f-
7ce493a40949%40enterprisedb.com
Alvaro Herrera [Wed, 29 Sep 2021 14:21:51 +0000 (11:21 -0300)]
Fix WAL replay in presence of an incomplete record
Physical replication always ships WAL segment files to replicas once
they are complete. This is a problem if one WAL record is split across
a segment boundary and the primary server crashes before writing down
the segment with the next portion of the WAL record: WAL writing after
crash recovery would happily resume at the point where the broken record
started, overwriting that record ... but any standby or backup may have
already received a copy of that segment, and they are not rewinding.
This causes standbys to stop following the primary after the latter
crashes:
LOG: invalid contrecord length 7262 at A8/
D9FFFBC8
because the standby is still trying to read the continuation record
(contrecord) for the original long WAL record, but it is not there and
it will never be. A workaround is to stop the replica, delete the WAL
file, and restart it -- at which point a fresh copy is brought over from
the primary. But that's pretty labor intensive, and I bet many users
would just give up and re-clone the standby instead.
A fix for this problem was already attempted in commit
515e3d84a0b5, but
it only addressed the case for the scenario of WAL archiving, so
streaming replication would still be a problem (as well as other things
such as taking a filesystem-level backup while the server is down after
having crashed), and it had performance scalability problems too; so it
had to be reverted.
This commit fixes the problem using an approach suggested by Andres
Freund, whereby the initial portion(s) of the split-up WAL record are
kept, and a special type of WAL record is written where the contrecord
was lost, so that WAL replay in the replica knows to skip the broken
parts. With this approach, we can continue to stream/archive segment
files as soon as they are complete, and replay of the broken records
will proceed across the crash point without a hitch.
Because a new type of WAL record is added, users should be careful to
upgrade standbys first, primaries later. Otherwise they risk the standby
being unable to start if the primary happens to write such a record.
A new TAP test that exercises this is added, but the portability of it
is yet to be seen.
This has been wrong since the introduction of physical replication, so
backpatch all the way back. In stable branches, keep the new
XLogReaderState members at the end of the struct, to avoid an ABI
break.
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: Nathan Bossart <bossartn@amazon.com>
Discussion: https://postgr.es/m/
202108232252.dh7uxf6oxwcy@alvherre.pgsql
Fujii Masao [Wed, 29 Sep 2021 12:01:10 +0000 (21:01 +0900)]
pgbench: Fix handling of socket errors during benchmark.
Previously socket errors such as invalid socket or socket wait method failures
during benchmark caused pgbench to exit with status 0. Instead, errors during
the run should result in exit status 2.
Back-patch to v12 where pgbench started reporting exit status.
Original complaint and patch by Hayato Kuroda.
Author: Yugo Nagata, Fabien COELHO
Reviewed-by: Kyotaro Horiguchi, Fujii Masao
Discussion: https://postgr.es/m/TYCPR01MB5870057375ACA8A73099C649F5349@TYCPR01MB5870.jpnprd01.prod.outlook.com
Fujii Masao [Wed, 29 Sep 2021 11:35:00 +0000 (20:35 +0900)]
pgbench: Correct log level of message output when socket wait method fails.
The failure of socket wait method like "select()" doesn't terminate pgbench.
So the log level of error message when that failure happens should be ERROR.
But previously FATAL was used in that case.
Back-patch to v13 where pgbench started using common logging API.
Author: Yugo Nagata, Fabien COELHO
Reviewed-by: Kyotaro Horiguchi, Fujii Masao
Discussion: https://postgr.es/m/
20210617005934.
8bd37bf72efd5f1b38e6f482@sraoss.co.jp
Michael Paquier [Wed, 29 Sep 2021 06:29:38 +0000 (15:29 +0900)]
Clarify use of "statistics objects" in the code
The code inconsistently used "statistic object" or "statistics" where
the correct term, as discussed, is actually "statistics object". This
improves the state of the code to be more consistent.
While on it, fix an incorrect error message introduced in
a4d75c8. This
error should never happen, as the code states, but it would be
misleading.
Author: Justin Pryzby
Reviewed-by: Álvaro Herrera, Michael Paquier
Discussion: https://postgr.es/m/
20210924215827.GS831@telsasoft.com
Backpatch-through: 14
Peter Eisentraut [Wed, 29 Sep 2021 06:12:23 +0000 (08:12 +0200)]
Fix incorrect format placeholder
Michael Paquier [Wed, 29 Sep 2021 02:56:13 +0000 (11:56 +0900)]
doc: Fix some typos and markups
Author: Ekaterina Kiryanova
Discussion: https://postgr.es/m/
8a14e78f-6991-7a6e-4711-
fe376635f2ad@postgrespro.ru
Backpatch-through: 14
Michael Paquier [Wed, 29 Sep 2021 01:54:45 +0000 (10:54 +0900)]
Refactor output file handling when forking syslogger under EXEC_BACKEND
A forked logging collector in EXEC_BACKEND builds passes down file
descriptors (or HANDLEs in WIN32) through a command for files to be
reopened (for stderr and csvlog). Some of its logic was duplicated, and
this commit refactors the code with some wrapper routines for file
reopening after forking and fd grabbing when building the command for
the fork.
While on it, this simplifies a use of "long" in the code, introduced by
ab0ba6e to take care of a warning related to MinGW-W64 when mapping a
intptr_t to a printed value. "long" is 32-bit long on Windows, and
interoperability of Win32 and Win64 ensures that handles are always
32-bit significant, so we can just use "int" for the same result. This
also makes the new routines more symmetric.
This change makes easier the introduction of new log destinations in the
logging collector, and this is not the only piece of refactoring
planned. I have tested this change with EXEC_BACKEND on linux, macos,
and of course MSVC (both Win32 and Win64), but not MinGW so the
buildfarm may have something to say here.
Author: Sehrope Sarkuni, Michael Paquier
Discussion: https://postgr.es/m/CAH7T-aqswBM6JWe4pDehi1uOiufqe06DJWaU5=X7dDLyqUExHg@mail.gmail.com
Tom Lane [Tue, 28 Sep 2021 21:34:31 +0000 (17:34 -0400)]
Fix instability in contrib/bloom TAP tests.
It turns out that the instability complained of in commit
d3c09b9b1
has an embarrassingly simple explanation. The test script waits for
the standby to flush incoming WAL to disk, but it should wait for
the WAL to be replayed, since we are testing for the effects of that
to be visible.
While at it, use wait_for_catchup instead of reinventing that logic,
and adjust $Test::Builder::Level to improve future error reports.
Back-patch to v12 where the necessary infrastructure came in
(cf. aforesaid commit). Also back-patch
7d1aa6bf1 so that the
test will actually get run.
Discussion: https://postgr.es/m/
2854602.
1632852664@sss.pgh.pa.us
Magnus Hagander [Tue, 28 Sep 2021 14:23:18 +0000 (16:23 +0200)]
Properly schema-prefix reference to pg_catalog.pg_get_statisticsobjdef_columns
Author: Tatsuro Yamada
Backpatch-through: 14
Discussion: https://www.postgresql.org/message-id/
7ad8cd13-db5b-5cf6-8561-
dccad1a934cb@nttcom.co.jp
Peter Eisentraut [Tue, 28 Sep 2021 13:26:25 +0000 (15:26 +0200)]
Support amcheck of sequences
Sequences were left out of the list of relation kinds that
verify_heapam knew how to check, though it is fairly trivial to allow
them. Doing that, and while at it, updating pg_amcheck to include
sequences in relations matched by table and relation patterns.
Author: Mark Dilger <mark.dilger@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/
81ad4757-92c1-4aa3-7bee-
f609544837e3%40enterprisedb.com
Tom Lane [Mon, 27 Sep 2021 22:48:01 +0000 (18:48 -0400)]
Re-enable contrib/bloom's TAP tests.
These tests were disabled back in 2018 (commit
d3c09b9b1) because of
failures observed in the buildfarm. I've not been able to reproduce
any failure on longfin's host, though, so I'm curious whether or to
what extent we've fixed the problem. Let's re-enable it (in HEAD
only) and see what blows up.
Discussion: https://postgr.es/m/
2769443.
1632773967@sss.pgh.pa.us
Michael Paquier [Mon, 27 Sep 2021 05:21:28 +0000 (14:21 +0900)]
Fix typos and grammar in code comments
Several mistakes have piled in the code comments over the time,
including incorrect grammar, function names and simple typos. This
commit takes care of a portion of these.
No backpatch is done as this is only cosmetic.
Author: Justin Pryzby
Discussion: https://postgr.es/m/
20210924215827.GS831@telsasoft.com
Peter Geoghegan [Mon, 27 Sep 2021 03:25:14 +0000 (20:25 -0700)]
Remove unneeded nbtree latestRemovedXid comments.
Discussing the low level issue of nbtree VACUUM and recovery conflicts
in btvacuumpage() now seems inappropriate. The same issue is discussed
in nbtxlog.h, as well as in a comment block above _bt_delitems_vacuum().
The comment block made more sense when it was part of a broader
discussion of nbtree VACUUM "pin scans". These were removed by commit
9f83468b.
Thomas Munro [Sun, 26 Sep 2021 21:39:01 +0000 (10:39 +1300)]
Track LLVM 14 API changes.
Only done on the master branch for now to fix build farm animal seawasp
(which tests bleeeding edge PostgreSQL with bleeding edge LLVM). We can
back-patch a consolidated fix closer to LLVM 14's release, once its API
has stopped moving around.
Discussion: https://postgr.es/m/CA%2BhUKGL%3Dyg6qqgg6W6SAuvRQejditeoDNy-X3b9H_6Fnw8j5Wg%40mail.gmail.com
Tom Lane [Sun, 26 Sep 2021 18:24:03 +0000 (14:24 -0400)]
Avoid unnecessary division in interval_cmp_value().
Splitting the time field into days and microseconds is pretty
useless when we're just going to recombine those values.
It's unclear if anyone will notice the speedup in real-world
cases, but a cycle shaved is a cycle earned.
Discussion: https://postgr.es/m/
2629129.
1632675713@sss.pgh.pa.us
Michael Paquier [Sun, 26 Sep 2021 10:17:30 +0000 (19:17 +0900)]
Fix typos in docs
Author: Justin Pryzby
Discussion: https://postgr.es/m/
20210924215827.GS831@telsasoft.com
Backpatch-through: 9.6
Peter Geoghegan [Sat, 25 Sep 2021 22:05:56 +0000 (15:05 -0700)]
Update obsolete nbtree deletion comments.
_bt_delitems_delete() is no longer the high-level entry point used by
index tuple deletion driven by index tuples whose LP_DEAD bits are set
(now called "simple index tuple deletion"). It became a lower level
routine that's only called by _bt_delitems_delete_check() following
commit
d168b66682.
Peter Geoghegan [Sat, 25 Sep 2021 17:22:53 +0000 (10:22 -0700)]
vacuumlazy.c: Remove obsolete 'onecall' comment.
Remove obsolete reference to lazy_vacuum()'s onecall argument. The
function argument was removed by commit
3499df0dee.
Also remove adjoining comment block that introduces the wraparound
failsafe concept. Talking about the failsafe here no longer makes
sense, since lazy_vacuum() (and related functions) are no longer the
only place where the failsafe might be triggered. This has been the
case since commit
c242baa4a8 taught VACUUM to consider triggering the
failsafe mechanism during its initial heap scan.
Tom Lane [Sat, 25 Sep 2021 14:53:54 +0000 (10:53 -0400)]
Doc: extend warnings about collation-mismatch hazards in postgres_fdw.
Be a little more vocal about the risks of remote collations not
matching local ones. Actually fixing these risks seems hard,
and I've given up on the idea that it might be back-patchable.
So the best we can do for the back branches is add documentation.
Per discussion of bug #16583 from Jiří Fejfar.
Discussion: https://postgr.es/m/
2438715.
1632510693@sss.pgh.pa.us
Michael Paquier [Sat, 25 Sep 2021 06:10:24 +0000 (15:10 +0900)]
doc: Improve description of index vacuuming with GUCs
Index vacuums may happen multiple times depending on the number of dead
tuples stored, as of maintenance_work_mem for a manual VACUUM. For
autovacuum, this is controlled by autovacuum_work_mem instead, if set.
The documentation mentioned the former, but not the latter in the
context of autovacuum.
Reported-by: Nikolai Berkoff
Author: Laurenz Albe, Euler Taveira
Discussion: https://postgr.es/m/
161545365522.10134.
12195402324485546870@wrigleys.postgresql.org
Backpatch-through: 9.6
Michael Paquier [Sat, 25 Sep 2021 05:48:03 +0000 (14:48 +0900)]
doc: Add missing markup in CREATE EVENT TRIGGER page
Reported-by: rir
Discussion: https://postgr.es/m/
20210924183658.3syyitp3yuxjv2fp@localhost
Backpatch-through: 9.6
Peter Geoghegan [Fri, 24 Sep 2021 20:53:48 +0000 (13:53 -0700)]
nbtree README: Add note about latestRemovedXid.
Point out that index tuple deletion generally needs a latestRemovedXid
value for the deletion operation's WAL record. This is bound to be the
most expensive part of the whole deletion operation now that it takes
place up front, during original execution.
This was arguably an oversight in commit
558a9165e08, which moved the
work required to generate these values from index deletion REDO routines
to original execution of index deletion operations.
Peter Eisentraut [Thu, 23 Sep 2021 20:49:20 +0000 (22:49 +0200)]
Add missing $Test::Builder::Level settings
One of these was accidentally removed by
c50624c. The others are
added by analogy.
Discussion: https://www.postgresql.org/message-id/
ae1143fb-455c-c80f-ed66-
78d45bd93303@enterprisedb.com
John Naylor [Thu, 23 Sep 2021 19:30:25 +0000 (15:30 -0400)]
Add exception for unicode_east_asian_fw_table.h to headerscheck also
Followup to
a315b19cc
John Naylor [Thu, 23 Sep 2021 19:14:22 +0000 (15:14 -0400)]
Add exception for unicode_east_asian_fw_table.h to cpluspluscheck
unicode_east_asian_fw_table.h should not be compiled standalone, similarly
to unicode_combining_table.h, but cpluspluscheck did not get the memo.
Oversight in
bab982161.
Per report from Tom Lane
Alexander Korotkov [Thu, 23 Sep 2021 16:59:03 +0000 (19:59 +0300)]
Split macros from visibilitymap.h into a separate header
That allows to include just visibilitymapdefs.h from file.c, and in turn,
remove include of postgres.h from relcache.h.
Reported-by: Andres Freund
Discussion: https://postgr.es/m/
20210913232614.czafiubr435l6egi%40alap3.anarazel.de
Author: Alexander Korotkov
Reviewed-by: Andres Freund, Tom Lane, Alvaro Herrera
Backpatch-through: 13
Tomas Vondra [Mon, 20 Sep 2021 23:13:11 +0000 (01:13 +0200)]
Release memory allocated by dependency_degree
Calculating degree of a functional dependency may allocate a lot of
memory - we have released mot of the explicitly allocated memory, but
e.g. detoasted varlena values were left behind. That may be an issue,
because we consider a lot of dependencies (all combinations), and the
detoasting may happen for each one again.
Fixed by calling dependency_degree() in a dedicated context, and
resetting it after each call. We only need the calculated dependency
degree, so we don't need to copy anything.
Backpatch to PostgreSQL 10, where extended statistics were introduced.
Backpatch-through: 10
Discussion: https://www.postgresql.org/message-id/
20210915200928.GP831%40telsasoft.com
Tomas Vondra [Mon, 20 Sep 2021 23:14:11 +0000 (01:14 +0200)]
Free memory after building each statistics object
Until now, all extended statistics on a given relation were built in the
same memory context, without resetting. Some of the memory was released
explicitly, but not all of it - for example memory allocated while
detoasting values is hard to free. This is how it worked since extended
statistics were introduced in PostgreSQL 10, but adding support for
extended stats on expressions made the issue somewhat worse as it
increases the number of statistics to build.
Fixed by adding a memory context which gets reset after building each
statistics object (all the statistics kinds included in it). Resetting
it after building each statistics kind would be even better, but it
would require more invasive changes and copying of results, making it
harder to backpatch.
Backpatch to PostgreSQL 10, where extended statistics were introduced.
Author: Justin Pryzby
Reported-by: Justin Pryzby
Reviewed-by: Tomas Vondra
Backpatch-through: 10
Discussion: https://www.postgresql.org/message-id/
20210915200928.GP831%40telsasoft.com
Peter Geoghegan [Thu, 23 Sep 2021 02:21:36 +0000 (19:21 -0700)]
Document issue with heapam line pointer truncation.
Checking that an offset number isn't past the end of a heap page's line
pointer array was just a defensive sanity check for HOT-chain traversal
code before commit
3c3b8a4b. It's etrictly necessary now, though. Add
comments that reference the issue to code in heapam that needs to get it
right.
Per suggestion from Alexander Lakhin.
Discussion: https://postgr.es/m/
f76a292c-9170-1aef-91a0-
59d9443b99a3@gmail.com
Fujii Masao [Wed, 22 Sep 2021 14:47:36 +0000 (23:47 +0900)]
postgres_fdw: Refactor transaction rollback code to avoid code duplication.
In postgres_fdw, pgfdw_xact_callback() and pgfdw_subxact_callback()
callback functions do almost the same thing to rollback remote toplevel-
and sub-transaction. But previously their such rollback logics were
implemented separately in each function and in different way. Which
could decrease the readability and maintainability of the code.
To fix the issue, this commit creates the common function to rollback
remote transactions, and makes those callback functions use it. Which
allows us to avoid unnecessary code duplication.
Author: Fujii Masao
Reviewed-by: Zhihong Yu, Bharath Rupireddy
Discussion: https://postgr.es/m/
62fbb63a-d46c-fb47-a56d-
f6be1909aa44@oss.nttdata.com
Peter Eisentraut [Wed, 22 Sep 2021 05:31:05 +0000 (07:31 +0200)]
Make use of PG_INT64_MAX/PG_INT64_MIN
This code was written before those symbols were introduced, but now we
can simplify it.
Amit Kapila [Wed, 22 Sep 2021 02:30:54 +0000 (08:00 +0530)]
Invalidate all partitions for a partitioned table in publication.
Updates/Deletes on a partition were allowed even without replica identity
after the parent table was added to a publication. This would later lead
to an error on subscribers. The reason was that we were not invalidating
the partition's relcache and the publication information for partitions
was not getting rebuilt. Similarly, we were not invalidating the
partitions' relcache after dropping a partitioned table from a publication
which will prohibit Updates/Deletes on its partition without replica
identity even without any publication.
Reported-by: Haiying Tang
Author: Hou Zhijie and Vignesh C
Reviewed-by: Vignesh C and Amit Kapila
Backpatch-through: 13
Discussion: https://postgr.es/m/OS0PR01MB6113D77F583C922F1CEAA1C3FBD29@OS0PR01MB6113.jpnprd01.prod.outlook.com
Amit Kapila [Wed, 22 Sep 2021 02:12:52 +0000 (07:42 +0530)]
Add parent table name in an error in reorderbuffer.c.
This can help in troubleshooting the cause of a particular error that can
occur during decoding.
Author: Jeremy Schneider
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/
808ed65b-994c-915a-361c-
577f088b837f@amazon.com
Peter Geoghegan [Wed, 22 Sep 2021 01:57:32 +0000 (18:57 -0700)]
Fix "single value strategy" index deletion issue.
It is not appropriate for deduplication to apply single value strategy
when triggered by a bottom-up index deletion pass. This wastes cycles
because later bottom-up deletion passes will overinterpret older
duplicate tuples that deduplication actually just skipped over "by
design". It also makes bottom-up deletion much less effective for low
cardinality indexes that happen to cross a meaningless "index has single
key value per leaf page" threshold.
To fix, slightly narrow the conditions under which deduplication's
single value strategy is considered. We already avoided the strategy
for a unique index, since our high level goal must just be to buy time
for VACUUM to run (not to buy space). We'll now also avoid it when we
just had a bottom-up pass that reported failure. The two cases share
the same high level goal, and already overlapped significantly, so this
approach is quite natural.
Oversight in commit
d168b666, which added bottom-up index deletion.
Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-WznaOvM+Gyj-JQ0X=JxoMDxctDTYjiEuETdAGbF5EUc3MA@mail.gmail.com
Backpatch: 14-, where bottom-up deletion was introduced.
Michael Paquier [Wed, 22 Sep 2021 01:13:38 +0000 (10:13 +0900)]
Fix some issues with TAP tests for postgres -C
This addresses two issues with the tests added in
0c39c292 for runtime
GUCs:
- Re-enable the test on Msys. The test could fail because of \r\n
generated by Msys perl.
0d91c52a has taken care of this issue.
- Allow the test to run in the context of a privileged account. CIs
running under privileged accounts would fail on permission failures, as
reported by Andres Freund. This issue is fixed by wrapping the postgres
command within pg_ctl as the latter will take care of any permissions
needed. The test checking a failure of postgres -C for a runtime
parameter with an instance running is removed, as pg_ctl produces an
unstable error code (no need for a CI to reproduce that).
Discussion: https://postgr.es/m/
20210921032040.lyl4lcax37aedx2x@alap3.anarazel.de
Michael Paquier [Tue, 21 Sep 2021 23:42:42 +0000 (08:42 +0900)]
Fix places in TestLib.pm in need of adaptation to the output of Msys perl
Contrary to the output of native perl, Msys perl generates outputs with
CRLFs characters. There are already places in the TAP code where CRLFs
(\r\n) are automatically converted to LF (\n) on Msys, but we missed a
couple of places when running commands and using their output for
comparison, that would lead to failures.
This problem has been found thanks to the test added in
5adb067 using
TestLib::command_checks_all(), but after a closer look more code paths
were missing a filter.
This is backpatched all the way down to prevent any surprises if a new
test is introduced in stable branches.
Reviewed-by: Andrew Dunstan, Álvaro Herrera
Discussion: https://postgr.es/m/
1252480.
1631829409@sss.pgh.pa.us
Backpatch-through: 9.6
Tom Lane [Tue, 21 Sep 2021 23:06:33 +0000 (19:06 -0400)]
Fix misevaluation of STABLE parameters in CALL within plpgsql.
Before commit
84f5c2908, a STABLE function in a plpgsql CALL
statement's argument list would see an up-to-date snapshot,
because exec_stmt_call would push a new snapshot. I got rid of
that because the possibility of the snapshot disappearing within
COMMIT made it too hard to manage a snapshot across the CALL
statement. That's fine so far as the procedure itself goes,
but I forgot to think about the possibility of STABLE functions
within the CALL argument list. As things now stand, those'll
be executed with the Portal's snapshot as ActiveSnapshot,
keeping them from seeing updates more recent than Portal startup.
(VOLATILE functions don't have a problem because they take their
own snapshots; which indeed is also why the procedure itself
doesn't have a problem. There are no STABLE procedures.)
We can fix this by pushing a new snapshot transiently within
ExecuteCallStmt itself. Popping the snapshot before we get
into the procedure proper eliminates the management problem.
The possibly-useless extra snapshot-grab is slightly annoying,
but it's no worse than what happened before
84f5c2908.
Per bug #17199 from Alexander Nawratil. Back-patch to v11,
like the previous patch.
Discussion: https://postgr.es/m/17199-
1ab2561f0d94af92@postgresql.org
Alvaro Herrera [Tue, 21 Sep 2021 22:47:53 +0000 (19:47 -0300)]
Document XLOG_INCLUDE_XID a little better
I noticed that commit
0bead9af484c left this flag undocumented in
XLogSetRecordFlags, which led me to discover that the flag doesn't
actually do what the one comment on it said it does. Improve the
situation by adding some more comments.
Backpatch to 14, where the aforementioned commit appears.
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/
202109212119.c3nhfp64t2ql@alvherre.pgsql
Michael Paquier [Tue, 21 Sep 2021 01:31:58 +0000 (10:31 +0900)]
Introduce GUC shared_memory_size_in_huge_pages
This runtime-computed GUC shows the number of huge pages required
for the server's main shared memory area, taking advantage of the
work done in
0c39c29 and
0bd305e. This is useful for users to estimate
the amount of huge pages required for a server as it becomes possible to
do an estimation without having to start the server and potentially
allocate a large chunk of shared memory.
The number of huge pages is calculated based on the existing GUC
huge_page_size if set, or by using the system's default by looking at
/proc/meminfo on Linux. There is nothing new here as this commit reuses
the existing calculation methods, and just exposes this information
directly to the user. The routine calculating the huge page size is
refactored to limit the number of files with platform-specific flags.
This new GUC's name was the most popular choice based on the discussion
done. This is only supported on Linux.
I have taken the time to test the change on Linux, Windows and MacOS,
though for the last two ones large pages are not supported. The first
one calculates correctly the number of pages depending on the existing
GUC huge_page_size or the system's default.
Thanks to Andres Freund, Robert Haas, Kyotaro Horiguchi, Tom Lane,
Justin Pryzby (and anybody forgotten here) for the discussion.
Author: Nathan Bossart
Discussion: https://postgr.es/m/
F2772387-CE0F-46BF-B5F1-
CC55516EB885@amazon.com
Peter Geoghegan [Mon, 20 Sep 2021 21:26:25 +0000 (14:26 -0700)]
Remove overzealous index deletion assertion.
A broken HOT chain is not an unexpected condition, even when the offset
number points past the end of the page's line pointer array.
heap_prune_chain() does not (and never has) treated this condition as
unexpected, so derivative code in heap_index_delete_tuples() shouldn't
do so either.
Oversight in commit
4228817449.
The assertion can probably only fail on Postgres 14 and master. Earlier
releases don't have commit
3c3b8a4b, which taught VACUUM to truncate the
line pointer array of heap pages. Backpatch all the same, just to be
consistent.
Author: Peter Geoghegan <pg@bowt.ie>
Reported-By: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/17197-
9438f31f46705182@postgresql.org
Backpatch: 12-, just like commit
4228817449.
Andres Freund [Sun, 4 Apr 2021 00:39:54 +0000 (17:39 -0700)]
pgstat: Prepare to use mechanism for truncated rels also for droppped rels.
The upcoming shared memory stats patch drops stats for dropped objects in a
transactional manner, rather than removing them later as part of vacuum. This
means that stats for DROP inside a transaction needs to handle aborted
(sub-)transactions similar to TRUNCATE: The stats up to the DROP should be
restored.
Rename the existing infrastructure in preparation.
Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/
20210405092914.mmxqe7j56lsjfsej@alap3.anarazel.de
Andres Freund [Mon, 20 Sep 2021 20:56:16 +0000 (13:56 -0700)]
pgstat: Split out relation stats handling from AtEO[Sub]Xact_PgStat() etc.
An upcoming patch will add additional work to these functions. To avoid the
functions getting too complicated / doing too many things at once, split out
sub-tasks into their own functions.
Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/
20210405092914.mmxqe7j56lsjfsej@alap3.anarazel.de
Alvaro Herrera [Mon, 20 Sep 2021 15:22:02 +0000 (12:22 -0300)]
Doc: add glossary term for "auxiliary process"
Add entries for existing processes not documented, too, and adjust
existing definitions for consistency.
Per question from Bharath Rupireddy.
Author: Justin Pryzby <pryzby@telsasoft.com>
Author: Alvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/CALj2ACVpYCT0M+k8zqrAa4ZQZV+ce5s6G=yajwoS1m=h-jj8NQ@mail.gmail.com
Tom Lane [Mon, 20 Sep 2021 14:48:02 +0000 (10:48 -0400)]
Doc: minor improvements for "Formatting" section.
Add more-specific links into the source tree.
Tomas Vondra [Sun, 19 Sep 2021 22:34:57 +0000 (00:34 +0200)]
Disallow extended statistics on system columns
Since introduction of extended statistics, we've disallowed references
to system columns. So for example
CREATE STATISTICS s ON ctid FROM t;
would fail. But with extended statistics on expressions, it was possible
to work around this limitation quite easily
CREATE STATISTICS s ON (ctid::text) FROM t;
This is an oversight in
a4d75c86bf, fixed by adding a simple check.
Backpatch to PostgreSQL 14, where support for extended statistics on
expressions was introduced.
Backpatch-through: 14
Discussion: https://postgr.es/m/
20210816013255.GS10479%40telsasoft.com
Tom Lane [Sun, 19 Sep 2021 15:36:53 +0000 (11:36 -0400)]
Doc: fix typos.
"PGcon" should be "PGconn". Noted by D. Frey.
Discussion: https://postgr.es/m/
163191739352.4680.
16994248583642672629@wrigleys.postgresql.org
Andres Freund [Wed, 8 Sep 2021 19:19:50 +0000 (12:19 -0700)]
process startup: Split single user code out of PostgresMain().
It was harder than necessary to understand PostgresMain() because the code for
a normal backend was interspersed with single-user mode specific code. Split
most of the single-user mode code into its own function
PostgresSingleUserMain(), that does all the necessary setup for single-user
mode, and then hands off after that to PostgresMain().
There still is some single-user mode code in InitPostgres(), and it'd likely
be worth moving at least some of it out. But that's for later.
Reviewed-By: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/
20210802164124.ufo5buo4apl6yuvs@alap3.anarazel.de
Michael Paquier [Sat, 18 Sep 2021 01:42:13 +0000 (10:42 +0900)]
Improve some check logic in pg_receivewal
The following things are improved:
- Fetch the system identifier from the source server before any
WAL streaming loop. This triggers extra checks to make sure that
pg_receivewal is still connected to a server with the same system ID
with a correct timeline.
- Switch umask() (for file creation mode mask) and RetrieveWalSegSize()
(to fetch the size of WAL segments) a bit later before the initial
stream attempt. If the connection was done with a database,
pg_receivewal would fail but those commands were still executed, which
was a waste. The slot creation and drop are now done before retrieving
the segment size.
Author: Bharath Rupireddy
Reviewed-by: Ronan Dunklau, Michael Paquier
Discussion: https://postgr.es/m/CALj2ACX00YYeyBfoi55Cy=NrP-FcfMgiYYx1qRUEib3yjCVoaA@mail.gmail.com
Peter Geoghegan [Fri, 17 Sep 2021 21:19:51 +0000 (14:19 -0700)]
pageinspect: Make page deletion elog less chatty.
An elog that reports the value of a transaction ID stored on a deleted
nbtree page was added by commit
e5d8a999, which taught page deletion to
store full 64-bit XIDs. It seems very chatty on further reflection, so
lower its elevel from NOTICE to DEBUG2.
Author: Peter Geoghegan <pg@bowt.ie>
Backpatch: 14-, just like the nbtree XID enhancement.
Tom Lane [Fri, 17 Sep 2021 19:41:16 +0000 (15:41 -0400)]
Fix pull_varnos to cope with translated PlaceHolderVars.
Commit
55dc86eca changed pull_varnos to use (if possible) the associated
ph_eval_at for a PlaceHolderVar. I missed a fine point though: we might
be looking at a PHV in the quals or tlist of a child appendrel, in which
case we need to compute a ph_eval_at value that's been translated in the
same way that the PHV itself has been (cf. adjust_appendrel_attrs).
Fortunately, enough info is available in the PlaceHolderInfo to make
such translation possible without additional outside data, so we don't
need another round of uglification of planner APIs. This is a little
bit complicated, but since it's a hard-to-hit corner case, I'm not much
worried about adding cycles here.
Per report from Jaime Casanova. Back-patch to v12, like the previous
commit.
Discussion: https://postgr.es/m/
20210915230959.GB17635@ahch-to
Michael Paquier [Fri, 17 Sep 2021 06:53:01 +0000 (15:53 +0900)]
Clarify some errors in pg_receivewal when closing WAL segments
A WAL segment closed during a WAL stream for pg_receivewal would
generate incorrect error messages depending on the context, as the file
name used when referring to a WAL segment ignored partial files or the
compression method used. In such cases, the error message generated
(failure on close, seek or rename) would not match a physical file
name. The same code paths are used by pg_basebackup, but it uses no
partial suffix so it is not impacted.
7fbe0c8 has introduced in walmethods.c a callback to get the exact
physical file name used for a given context, this commit makes use of it
to improve those error messages. This could be extended to more code
paths of pg_basebackup/ in the future, if necessary.
Extracted from a larger patch by the same author.
Author: Georgios Kokolatos
Discussion: https://postgr.es/m/ZCm1J5vfyQ2E6dYvXz8si39HQ2gwxSZ3IpYaVgYa3lUwY88SLapx9EEnOf5uEwrddhx2twG7zYKjVeuP5MwZXCNPybtsGouDsAD1o2L_I5E=@pm.me
Michael Paquier [Fri, 17 Sep 2021 00:11:47 +0000 (09:11 +0900)]
Disable test for postgres -C on Msys
The output generated on Msys is incorrect because of the different way
IPC::Run processes outputs with native Perl (converts natively \r\n to
\n) and Msys perl (\r\n kept as-is), causing this test to fail.
For now, just disable the test to bring the buildfarm to a green state.
I think that the correct long-term solution would be to tweak all the
routines command_checks_* in PostgresNode.pm to handle this output like
psql does when using Msys, by discarding \r automatically before
comparing it.
Per report from jacana and fairywren. Thanks to Tom Lane for the ping.
Discussion: https://postgr.es/m/
1252480.
1631829409@sss.pgh.pa.us
Tom Lane [Thu, 16 Sep 2021 14:45:42 +0000 (10:45 -0400)]
Fix EXPLAIN to handle SEARCH BREADTH FIRST queries.
The rewriter transformation for SEARCH BREADTH FIRST produces a
FieldSelect on a Var of type RECORD, where the Var references the
recursive union's worktable output. EXPLAIN VERBOSE failed to handle
this case, because it only expected such Vars to appear in CteScans
not WorkTableScans. Fix that, and add some test cases exercising
EXPLAIN on SEARCH and CYCLE queries.
In principle this oversight is an old bug, but it seems that the
case is unreachable without SEARCH BREADTH FIRST, because the
parser fails when attempting to create such a reference manually.
So for today I'll just patch HEAD/v14. Someday we might find that
the code portion of this patch needs to be back-patched further.
Per report from Atsushi Torikoshi.
Discussion: https://postgr.es/m/
5bafa66ad529e11860339565c9e7c166@oss.nttdata.com
Peter Eisentraut [Thu, 16 Sep 2021 12:48:52 +0000 (14:48 +0200)]
Message style improvements
Andres Freund [Thu, 16 Sep 2021 10:23:05 +0000 (03:23 -0700)]
process startup: Do InitProcess() at the same time regardless of EXEC_BACKEND.
An upcoming patch splits single user mode into its own function. This makes
that easier. Split out for easier review / testing.
Reviewed-By: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/
20210802164124.ufo5buo4apl6yuvs@alap3.anarazel.de
Andres Freund [Thu, 16 Sep 2021 09:02:40 +0000 (02:02 -0700)]
Fix performance regression from session statistics.
Session statistics, as introduced by
960869da08, had several shortcomings:
- an additional GetCurrentTimestamp() call that also impaired the accuracy of
the data collected
This can be avoided by passing the current timestamp we already have in
pgstat_report_stat().
- an additional statistics UDP packet sent every 500ms
This is solved by adding the new statistics to PgStat_MsgTabstat.
This is conceptually ugly, because session statistics are not
table statistics. But the struct already contains data unrelated
to tables, so there is not much damage done.
Connection and disconnection are reported in separate messages, which
reduces the number of additional messages to two messages per session and a
slight increase in PgStat_MsgTabstat size (but the same number of table
stats fit).
- Session time computation could overflow on systems where long is 32 bit.
Reported-By: Andres Freund <andres@anarazel.de>
Author: Andres Freund <andres@anarazel.de>
Author: Laurenz Albe <laurenz.albe@cybertec.at>
Discussion: https://postgr.es/m/
20210801205501.nyxzxoelqoo4x2qc%40alap3.anarazel.de
Backpatch: 14-, where the feature was introduced.
Fujii Masao [Thu, 16 Sep 2021 04:06:21 +0000 (13:06 +0900)]
Fix variable shadowing in procarray.c.
ProcArrayGroupClearXid function has a parameter named "proc",
but the same name was used for its local variables. This commit fixes
this variable shadowing, to improve code readability.
Back-patch to all supported versions, to make future back-patching
easy though this patch is classified as refactoring only.
Reported-by: Ranier Vilela
Author: Ranier Vilela, Aleksander Alekseev
https://postgr.es/m/CAEudQAqyoTZC670xWi6w-Oe2_Bk1bfu2JzXz6xRfiOUzm7xbyQ@mail.gmail.com
Fujii Masao [Thu, 16 Sep 2021 03:52:30 +0000 (12:52 +0900)]
Use int instead of size_t in procarray.c.
All size_t variables declared in procarray.c are actually int ones.
Let's use int instead of size_t for those variables. Which would
reduce Wsign-compare compiler warnings.
Back-patch to v14 where commit
941697c3c1 added size_t variables
in procarray.c, to make future back-patching easy though
this patch is classified as refactoring only.
Reported-by: Ranier Vilela
Author: Ranier Vilela, Aleksander Alekseev
https://postgr.es/m/CAEudQAqyoTZC670xWi6w-Oe2_Bk1bfu2JzXz6xRfiOUzm7xbyQ@mail.gmail.com
Michael Paquier [Thu, 16 Sep 2021 01:59:26 +0000 (10:59 +0900)]
Support "postgres -C" with runtime-computed GUCs
Until now, the -C option of postgres was handled before a small subset
of GUCs computed at runtime are initialized, leading to incorrect
results as GUC machinery would fall back to default values for such
parameters.
For example, data_checksums could report "off" for a cluster as the
control file is not loaded yet. Or wal_segment_size would show a
segment size at 16MB even if initdb --wal-segsize used something else.
Worse, the command would fail to properly report the recently-introduced
shared_memory, that requires to load shared_preload_libraries as these
could ask for a chunk of shared memory.
Support for runtime GUCs comes with a limitation, as the operation is
now allowed on a running server. One notable reason for this is that
_PG_init() functions of loadable libraries are called before all
runtime-computed GUCs are initialized, and this is not guaranteed to be
safe to do on running servers. For the case of shared_memory_size,
where we want to know how much memory would be used without allocating
it, this limitation is fine. Another case where this will help is for
huge pages, with the introduction of a different GUC to evaluate the
amount of huge pages required for a server before starting it, without
having to allocate large chunks of memory.
This feature is controlled with a new GUC flag, and four parameters are
classified as runtime-computed as of this change:
- data_checksums
- shared_memory_size
- data_directory_mode
- wal_segment_size
Some TAP tests are added to provide some coverage here, using
data_checksums in the tests of pg_checksums.
Per discussion with Andres Freund, Justin Pryzby, Magnus Hagander and
more.
Author: Nathan Bossart
Discussion: https://postgr.es/m/
F2772387-CE0F-46BF-B5F1-
CC55516EB885@amazon.com
Andres Freund [Wed, 15 Sep 2021 20:16:00 +0000 (13:16 -0700)]
process startup: Initialize PgStartTime earlier in single user mode.
An upcoming patch splits single user mode handling out of PostgresMain(). The
startup time only needs to be determined in single user mode. Currently the
initialization happens late, which makes the split a bit harder. As postmaster
determines the time earlier it makes sense to move the time for single user
mode to a roughly similar point in time.
Reviewd-By: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/
20210802164124.ufo5buo4apl6yuvs@alap3.anarazel.de
Tom Lane [Wed, 15 Sep 2021 18:11:21 +0000 (14:11 -0400)]
Remove arbitrary 64K-or-so limit on rangetable size.
Up to now the size of a query's rangetable has been limited by the
constants INNER_VAR et al, which mustn't be equal to any real
rangetable index. 65000 doubtless seemed like enough for anybody,
and it still is orders of magnitude larger than the number of joins
we can realistically handle. However, we need a rangetable entry
for each child partition that is (or might be) processed by a query.
Queries with a few thousand partitions are getting more realistic,
so that the day when that limit becomes a problem is in sight,
even if it's not here yet. Hence, let's raise the limit.
Rather than just increase the values of INNER_VAR et al, this patch
adopts the approach of making them small negative values, so that
rangetables could theoretically become as long as INT_MAX.
The bulk of the patch is concerned with changing Var.varno and some
related variables from "Index" (unsigned int) to plain "int". This
is basically cosmetic, with little actual effect other than to help
debuggers print their values nicely. As such, I've only bothered
with changing places that could actually see INNER_VAR et al, which
the parser and most of the planner don't. We do have to be careful
in places that are performing less/greater comparisons on varnos,
but there are very few such places, other than the IS_SPECIAL_VARNO
macro itself.
A notable side effect of this patch is that while it used to be
possible to add INNER_VAR et al to a Bitmapset, that will now
draw an error. I don't see any likelihood that it wouldn't be a
bug to include these fake varnos in a bitmapset of real varnos,
so I think this is all to the good.
Although this touches outfuncs/readfuncs, I don't think a catversion
bump is required, since stored rules would never contain Vars
with these fake varnos.
Andrey Lepikhov and Tom Lane, after a suggestion by Peter Eisentraut
Discussion: https://postgr.es/m/
43c7f2f5-1e27-27aa-8c65-
c91859d15190@postgrespro.ru
Peter Eisentraut [Wed, 15 Sep 2021 16:56:13 +0000 (18:56 +0200)]
Add Cardinality typedef
Similar to Cost and Selectivity, this is just a double, which can be
used in path and plan nodes to give some hint about the meaning of a
field.
Discussion: https://www.postgresql.org/message-id/
c091e5cd-45f8-69ee-6a9b-
de86912cc7e7@enterprisedb.com
Tom Lane [Wed, 15 Sep 2021 16:31:56 +0000 (12:31 -0400)]
Disallow LISTEN in background workers.
It's possible to execute user-defined SQL in some background processes;
for example, logical replication workers can fire triggers. This opens
the possibility that someone would try to execute LISTEN in such a
context. But since only regular backends ever call
ProcessNotifyInterrupt, no messages would actually be received, and
thus the registered listener would simply prevent the message queue
from being cleaned. Eventually NOTIFY would stop working, which is bad.
Perhaps someday somebody will invent infrastructure to make listening
in a background worker actually useful. In the meantime, forbid it.
Back-patch to v13, which is where we introduced the MyBackendType
variable. It'd be a lot harder to implement the check without that,
and it doesn't seem worth the trouble.
Discussion: https://postgr.es/m/
153243441449.1404.
2274116228506175596@wrigleys.postgresql.org
Peter Eisentraut [Wed, 15 Sep 2021 14:35:41 +0000 (16:35 +0200)]
Make node output prefix match node structure name
In most cases, the prefix string in a node output is the upper case of
the node structure name, e.g., MergeAppend -> MERGEAPPEND. There were
a few exceptions that for either no apparent reason or perhaps minor
aesthetic reasons deviated from this. In order to simplify this and
perhaps allow automatic generation without having to deal with
exception cases, make them all match.
Discussion: https://www.postgresql.org/message-id/
c091e5cd-45f8-69ee-6a9b-
de86912cc7e7@enterprisedb.com
Peter Eisentraut [Wed, 15 Sep 2021 09:59:34 +0000 (11:59 +0200)]
Fix hash_array
Commit
a3d2b1bbe904b0ca8d9fdde20f25295ff3e21f79 neglected to
initialize the type_id field of the synthesized type cache entry, so
it would make a new one on every call.
Also, better use the per-function memory context for this; otherwise
it leaks memory.
Discussion: https://www.postgresql.org/message-id/flat/17158-
8a2ba823982537a4%40postgresql.org
Daniel Gustafsson [Wed, 15 Sep 2021 07:54:45 +0000 (09:54 +0200)]
doc: Clarify refresh options for DROP PUBLICATION
The available refresh options are specified as refresh_options under
REFRESH PUBLICATION, and DROP PUBLICATION itself has an option named
refresh. Clarify what we mean by refresh options to avoid confusion.
Backpatch through v14 where ALTER SUBSCRIPTION ... DROP PUBLICATION
was introduced.
Author: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Discussion: https://postgr.es/m/CAD21AoCm1wJ3A8Q9EmBjRbShYkJ+o+Oa_z9O0hvwhvhUa2BSyg@mail.gmail.com
Backpatch-through: 14
Peter Eisentraut [Wed, 15 Sep 2021 07:19:01 +0000 (09:19 +0200)]
Fix incorrect format placeholders
Also remove obsolete comments about why the 64-bit integers need to be
printed in a separate buffer. The reason used to be portability, but
now the remaining reason is that we need the string lengths for the
progress displays. That is evident by looking at the code right
below, so a new comment doesn't seem necessary.
Peter Eisentraut [Wed, 15 Sep 2021 06:04:49 +0000 (08:04 +0200)]
Update Unicode data to Unicode 14.0.0
Michael Paquier [Wed, 15 Sep 2021 01:47:44 +0000 (10:47 +0900)]
Update README for resource owners about the resource types supported
All the types supported were listed directly in the README, but it was
very outdated. Rather than listing all the types supported in the
README, this commit adds a reference to look at ResourceOwnerData in
resowner.c to get this information.
The order of the paragraphs is reworked a bit for clarity.
Author: Amit Langote
Discussion: https://postgr.es/m/CA+HiwqHtfT9z=4H5+F7DOy0OyNHAaVwuRcakt9b2t2uADOaiag@mail.gmail.com
Tom Lane [Tue, 14 Sep 2021 22:55:15 +0000 (18:55 -0400)]
Improve log messages from pg_import_system_collations().
pg_import_system_collations() was a bit inconsistent about how it
reported locales (names output by "locale -a") that it didn't make
pg_collation entries for. IMV we should print some suitable message
for every locale that we reject, except when it matches a pre-existing
pg_collation entry. (This is all at DEBUG1 log level, though, so as
not to create noise during initdb.) Add messages for the two cases
that were previously not logged, namely unrecognized encoding and
client-only encoding. Re-word the existing messages to have a
consistent style.
Anton Voloshin and Tom Lane
Discussion: https://postgr.es/m/
429d64ee-188d-3ce1-106a-
53a8b45c4fce@postgrespro.ru
Tom Lane [Tue, 14 Sep 2021 21:18:25 +0000 (17:18 -0400)]
Send NOTIFY signals during CommitTransaction.
Formerly, we sent signals for outgoing NOTIFY messages within
ProcessCompletedNotifies, which was also responsible for sending
relevant ones of those messages to our connected client. It therefore
had to run during the main-loop processing that occurs just before
going idle. This arrangement had two big disadvantages:
* Now that procedures allow intra-command COMMITs, it would be
useful to send NOTIFYs to other sessions immediately at COMMIT
(though, for reasons of wire-protocol stability, we still shouldn't
forward them to our client until end of command).
* Background processes such as replication workers would not send
NOTIFYs at all, since they never execute the client communication
loop. We've had requests to allow triggers running in replication
workers to send NOTIFYs, so that's a problem.
To fix these things, move transmission of outgoing NOTIFY signals
into AtCommit_Notify, where it will happen during CommitTransaction.
Also move the possible call of asyncQueueAdvanceTail there, to
ensure we don't bloat the async SLRU if a background worker sends
many NOTIFYs with no one listening.
We can also drop the call of asyncQueueReadAllNotifications,
allowing ProcessCompletedNotifies to go away entirely. That's
because commit
790026972 added a call of ProcessNotifyInterrupt
adjacent to PostgresMain's call of ProcessCompletedNotifies,
and that does its own call of asyncQueueReadAllNotifications,
meaning that we were uselessly doing two such calls (inside two
separate transactions) whenever inbound notify signals coincided
with an outbound notify. We need only set notifyInterruptPending
to ensure that ProcessNotifyInterrupt runs, and we're done.
The existing documentation suggests that custom background workers
should call ProcessCompletedNotifies if they want to send NOTIFY
messages. To avoid an ABI break in the back branches, reduce it
to an empty routine rather than removing it entirely. Removal
will occur in v15.
Although the problems mentioned above have existed for awhile,
I don't feel comfortable back-patching this any further than v13.
There was quite a bit of churn in adjacent code between 12 and 13.
At minimum we'd have to also backpatch
51004c717, and a good deal
of other adjustment would also be needed, so the benefit-to-risk
ratio doesn't look attractive.
Per bug #15293 from Michael Powers (and similar gripes from others).
Artur Zakirov and Tom Lane
Discussion: https://postgr.es/m/
153243441449.1404.
2274116228506175596@wrigleys.postgresql.org
Tom Lane [Tue, 14 Sep 2021 19:11:21 +0000 (15:11 -0400)]
Fix planner error with multiple copies of an AlternativeSubPlan.
It's possible for us to copy an AlternativeSubPlan expression node
into multiple places, for example the scan quals of several
partition children. Then it's possible that we choose a different
one of the alternatives as optimal in each place. Commit
41efb8340
failed to consider this scenario, so its attempt to remove "unused"
subplans could remove subplans that were still used elsewhere.
Fix by delaying the removal logic until we've examined all the
AlternativeSubPlans in a given query level. (This does assume that
AlternativeSubPlans couldn't get copied to other query levels, but
for the foreseeable future that's fine; cf qual_is_pushdown_safe.)
Per report from Rajkumar Raghuwanshi. Back-patch to v14
where the faulty logic came in.
Discussion: https://postgr.es/m/CAKcux6==O3NNZC3bZ2prRYv3cjm3_Zw1GfzmOjEVqYN4jub2+Q@mail.gmail.com
Peter Eisentraut [Tue, 14 Sep 2021 07:34:50 +0000 (09:34 +0200)]
Add WRITE_INDEX_ARRAY
We have a few WRITE_{name of type}_ARRAY macros, but the one case
using the Index type was hand-coded. Wrap it into a macro as well.
This also changes the behavior slightly: Before, the field name was
skipped if the length was zero. Now it prints the field name even in
that case. This is more consistent with how other array fields are
handled.
Reviewed-by: Jacob Champion <pchampion@vmware.com>
Discussion: https://www.postgresql.org/message-id/
c091e5cd-45f8-69ee-6a9b-
de86912cc7e7@enterprisedb.com
Peter Eisentraut [Tue, 14 Sep 2021 07:34:50 +0000 (09:34 +0200)]
Add COPY_ARRAY_FIELD and COMPARE_ARRAY_FIELD
These handle node fields that are inline arrays (as opposed to
dynamically allocated arrays handled by COPY_POINTER_FIELD and
COMPARE_POINTER_FIELD). These cases were hand-coded until now.
Reviewed-by: Jacob Champion <pchampion@vmware.com>
Discussion: https://www.postgresql.org/message-id/
c091e5cd-45f8-69ee-6a9b-
de86912cc7e7@enterprisedb.com
Peter Eisentraut [Tue, 14 Sep 2021 07:34:50 +0000 (09:34 +0200)]
Remove T_Expr
This is an abstract node that shouldn't have a node tag defined.
Reviewed-by: Jacob Champion <pchampion@vmware.com>
Discussion: https://www.postgresql.org/message-id/
c091e5cd-45f8-69ee-6a9b-
de86912cc7e7@enterprisedb.com
Andres Freund [Tue, 14 Sep 2021 01:07:19 +0000 (18:07 -0700)]
jit: Do not try to shut down LLVM state in case of LLVM triggered errors.
If an allocation failed within LLVM it is not safe to call back into LLVM as
LLVM is not generally safe against exceptions / stack-unwinding. Thus errors
while in LLVM code are promoted to FATAL. However llvm_shutdown() did call
back into LLVM even in such cases, while llvm_release_context() was careful
not to do so.
We cannot generally skip shutting down LLVM, as that can break profiling. But
it's OK to do so if there was an error from within LLVM.
Reported-By: Jelte Fennema <Jelte.Fennema@microsoft.com>
Author: Andres Freund <andres@anarazel.de>
Author: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/AM5PR83MB0178C52CCA0A8DEA0207DC14F7FF9@AM5PR83MB0178.EURPRD83.prod.outlook.com
Backpatch: 11-, where jit was introduced
Michael Paquier [Tue, 14 Sep 2021 01:15:49 +0000 (10:15 +0900)]
Remove code duplication for permission checks with replication slots
Two functions, both named check_permissions(), used the same checks to
verify if a user had required privileges to work on replication slots.
This commit removes the duplication, and moves the function doing the
checks to slot.c to be centralized.
Author: Bharath Rupireddy
Reviewed-by: Nathan Bossart, Euler Taveira
Discussion: https://postgr.es/m/CALj2ACUPpVw1u7sQocFVWrSs0n10pt_G_4NPZKSxXK6cW1dErw@mail.gmail.com
Tom Lane [Mon, 13 Sep 2021 20:53:11 +0000 (16:53 -0400)]
Clear conn->errorMessage at successful completion of PQconnectdb().
Commits
ffa2e4670 and
52a10224e caused libpq's connection-establishment
functions to usually leave a nonempty string in the connection's
errorMessage buffer, even after a successful connection. While that
was intentional on my part, more sober reflection says that it wasn't
a great idea: the string would be a bit confusing. Also this broke at
least one application that checked for connection success by examining
the errorMessage, instead of using PQstatus() as documented. Let's
clear the buffer at success exit, restoring the pre-v14 behavior.
Discussion: https://postgr.es/m/
4170264.
1620321747@sss.pgh.pa.us
Tom Lane [Mon, 13 Sep 2021 18:27:02 +0000 (14:27 -0400)]
Doc: improve documentation of CREATE/ALTER SUBSCRIPTION.
Improve the descriptions of some options. Fix sloppy grammar and markup.
Peter Smith and Tom Lane
Discussion: https://postgr.es/m/CAHut+PtPJDSOxtuMGpO2yDrRPKxcYGL4n7HqJP9HernZE=Cj+g@mail.gmail.com
Tom Lane [Mon, 13 Sep 2021 16:42:03 +0000 (12:42 -0400)]
Fix EXIT out of outermost block in plpgsql.
Ordinarily, using EXIT this way would draw "control reached end of
function without RETURN". However, if the function is one where we
don't require an explicit RETURN (such as a DO block), that should
not happen. It did anyway, because add_dummy_return() neglected to
account for the case.
Per report from Herwig Goemans. Back-patch to all supported branches.
Discussion: https://postgr.es/m/
868ae948-e3ca-c7ec-95a6-
83cfc08ef750@gmail.com
Amit Kapila [Mon, 13 Sep 2021 04:54:00 +0000 (10:24 +0530)]
Fix reorder buffer memory accounting for toast changes.
While processing toast changes in logical decoding, we rejigger the
tuple change to point to in-memory toast tuples instead to on-disk toast
tuples. And, to make sure the memory accounting is correct, we were
subtracting the old change size and then after re-computing the new tuple,
re-adding its size at the end. Now, if there is any error before we add
the new size, we will release the changes and that will update the
accounting info (subtracting the size from the counters). And we were
underflowing there which leads to an assertion failure in assert enabled
builds and wrong memory accounting in reorder buffer otherwise.
Author: Bertrand Drouvot
Reviewed-by: Amit Kapila
Backpatch-through: 13, where memory accounting was introduced
Discussion: https://postgr.es/m/
92b0ee65-b8bd-e42d-c082-
4f3f4bf12d34@amazon.com
Michael Paquier [Mon, 13 Sep 2021 04:23:50 +0000 (13:23 +0900)]
Fix error handling with threads on OOM in ECPG connection logic
An out-of-memory failure happening when allocating the structures to
store the connection parameter keywords and values would mess up with
the set of connections saved, as on failure the pthread mutex would
still be hold with the new connection object listed but free()'d.
Rather than just unlocking the mutex, which would leave the static list
of connections into an inconsistent state, move the allocation for the
structures of the connection parameters before beginning the test
manipulation. This ensures that the list of connections and the
connection mutex remain consistent all the time in this code path.
This error is unlikely going to happen, but this could mess up badly
with ECPG clients in surprising ways, so backpatch all the way down.
Reported-by: ryancaicse
Discussion: https://postgr.es/m/17186-
b4cfd8f0eb4d1dee@postgresql.org
Backpatch-through: 9.6
Amit Kapila [Mon, 13 Sep 2021 02:59:10 +0000 (08:29 +0530)]
Doc: Change optional parameters grouping in Create Subscription.
The subscription parameters are rearranged into two groups:
a) those that control what happens during Create Subscription
b) those that control the replication behavior
This makes the documentation of Create Subscription easier to follow.
Author: Peter Smith
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CAHut+PtPJDSOxtuMGpO2yDrRPKxcYGL4n7HqJP9HernZE=Cj+g@mail.gmail.com
Michael Paquier [Mon, 13 Sep 2021 01:23:59 +0000 (10:23 +0900)]
Add regression tests for csvlog with the logging collector
These are added in the existing tests of pg_ctl for log rotation, that
already tested stderr. The same amount of coverage is added for csvlog:
- Checks for pg_current_logfile().
- Log rotation with expected file name.
- Log contents generated.
This test is refactored to minimize the amount of work required to add
tests for new log formats, easing some upcoming work.
Author: Michael Paquier, Sehrope Sarkuni
Discussion: https://postgr.es/m/CAH7T-aqswBM6JWe4pDehi1uOiufqe06DJWaU5=X7dDLyqUExHg@mail.gmail.com