Commit Graph

38335 Commits

Author SHA1 Message Date
Alvaro Herrera 4c83e59e01
Increase size of shared memory for pg_commit_ts
Like 5364b357fb did for pg_commit, change the formula used to
determine number of pg_commit_ts buffers, which helps performance with
larger servers.

Discussion: https://postgr.es/m/20210115220744.GA24457@alvherre.pgsql
Reviewed-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Tomas Vondra <tomas.vondra@enterprisedb.com>
2021-11-30 14:29:31 -03:00
Daniel Gustafsson e7122548a3 Disable unused-variable warning C4101 in MSVC
The C4101 warning for unused variable cannot be individually suppressed
with PG_USED_FOR_ASSERTS_ONLY, and thus cause false-positive warnings
for variables which are defined but only read/written in an assertion.
Until a satisfactory solution for per-variable suppression like how we
do for gcc and clang, disable the warning.

Discussion: https://postgr.es/m/CAJcOf-c+KniGAp31pn8TC=9a-WHXpkX-3+8-2BkaCsZchhu=8w@mail.gmail.com
2021-11-30 14:02:37 +01:00
Daniel Gustafsson ac0db34e0e Remove PF_USED_FOR_ASSERTS_ONLY from variables in general use
fsstate in process_pending_requests (in postgres_fdw.c) was added in
8998e3cafa as an assertion-only variable,  1ec7fca859 stated using
the variable outside of assertions.

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

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

Reviewed-by: Greg Nancarrow <gregn4422@gmail.com>
Discussion: https://postgr.es/m/F959106C-0F21-43A5-B2AE-D007D51ACBEE@yesql.se
2021-11-30 14:02:14 +01:00
Daniel Gustafsson c113d8ad50 Use test-specific temp path for keys during SSL test
The SSL and SCRAM TAP test suites both use temporary copies of the
supplied test keys in order to ensure correct permissions.  These
were however copied inside the tree using temporary filenames rather
than a true temporary folder.  Fix by using tmp_check supplied by
PostgreSQL::Test::Utils. Spotted by Tom Lane during review of the
nearby sslinfo TAP test patch.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/599244.1638041239@sss.pgh.pa.us
2021-11-30 11:21:27 +01:00
Daniel Gustafsson ae81776a23 Add TAP tests for contrib/sslinfo
This adds rudimentary coverage of the sslinfo extension into the SSL
test harness.  The output is validated by comparing with pg_stat_ssl
to provide some level of test stability should the underlying certs
be slightly altered.  A new cert is added to provide an extension to
test against.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andrew Dunstan <andrew@dunslane.net>
Reviewed-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Discussion: https://postgr.es/m/E23F9811-0C77-45DA-912F-D809AB140741@yesql.se
2021-11-30 11:19:59 +01:00
Daniel Gustafsson 879fc1a579 Extend configure_test_server_for_ssl to add extensions
In order to be able to test extensions with SSL connections, allow
configure_test_server_for_ssl to create any extensions passed as
an array. Each extension is created in all the test databases.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andrew Dunstan <andrew@dunslane.net>
Reviewed-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Discussion: https://postgr.es/m/E23F9811-0C77-45DA-912F-D809AB140741@yesql.se
2021-11-30 11:13:26 +01:00
Michael Paquier be5455124b Fix flags of some GUCs and improve some descriptions
This commit fixes some issues with GUCs:
- enable_incremental_sort was not marked as GUC_EXPLAIN, causing it to
not be listed in the output of EXPLAIN (SETTINGS) if using a value
different than the default, contrary to the other planner-level GUCs.
- trace_recovery_messages missed GUC_NOT_IN_SAMPLE, like the other
developer options.
- ssl_renegotiation_limit should be marked as COMPAT_OPTIONS_PREVIOUS.

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

Extracted from a larger patch set by the same author.

Author: Justin Pryzby
Description: https://postgr.es/m/20211129030833.GJ17618@telsasoft.com
2021-11-30 14:38:49 +09:00
Amit Kapila 8d74fc96db Add a view to show the stats of subscription workers.
This commit adds a new system view pg_stat_subscription_workers, that
shows information about any errors which occur during the application of
logical replication changes as well as during performing initial table
synchronization. The subscription statistics entries are removed when the
corresponding subscription is removed.

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

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

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

Author: Masahiko Sawada
Reviewed-by: Greg Nancarrow, Hou Zhijie, Tang Haiying, Vignesh C, Dilip Kumar, Takamichi Osumi, Amit Kapila
Discussion: https://postgr.es/m/CAD21AoDeScrsHhLyEPYqN3sydg6PxAPVBboK=30xJfUVihNZDA@mail.gmail.com
2021-11-30 08:54:30 +05:30
Michael Paquier 98105e53e0 Fix typos
Author: Lingjie Qiang
Discussion: https://postgr.es/m/OSAPR01MB71654E773F62AC88DC1FC8CC80669@OSAPR01MB7165.jpnprd01.prod.outlook.com
2021-11-30 11:05:15 +09:00
Michael Paquier 58651d8dd6 Fix compatibility thinko for fstat() on standard streams in win32stat.c
GetFinalPathNameByHandleA() cannot be used in compilation environments
where _WIN32_WINNT < 0x0600, meaning at least Windows XP used by some
buildfarm members under MinGW that Postgres still needs to support.
This was reported as a compilation warning by the buildfarm, but this is
actually worse than the report as the code would have not worked.

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

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

Reported-by: Justin Pryzby, via buildfarm member jacana
Author: Michael Paquier
Reviewed-by: Juan José Santamaría Flecha
Discussion: https://postgr.es/m/20211129050122.GK17618@telsasoft.com
Backpatch-through: 14
2021-11-30 09:55:50 +09:00
Peter Geoghegan 4f8d9d1217 vacuumlazy.c: Rename dead_tuples to dead_items.
Commit 8523492d simplified what it meant for an item to be considered
"dead" to VACUUM: TIDs collected in memory (in preparation for index
vacuuming) must always come from LP_DEAD stub line pointers in heap
pages, found following pruning.  This formalized the idea that index
vacuuming (and heap vacuuming) are optional processes.  Unlike pruning,
they can be delayed indefinitely, without any risk of that violating
fundamental invariants.  For example, leaving LP_DEAD items behind
clearly won't add to the risk of transaction ID wraparound.  You can't
have transaction ID wraparound without transaction IDs.  Renaming
anything that references DEAD tuples (tuples with storage) reinforces
all this.

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

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

Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAH2-WzktGBg4si6DEdmq3q6SoXSDqNi6MtmB8CmmTmvhsxDTLA@mail.gmail.com
2021-11-29 09:58:01 -08:00
Tom Lane e04a8059a7 Simplify declaring variables exported from libpgcommon and libpgport.
This reverts commits c2d1eea9e and 11b500072, as well as similar hacks
elsewhere, in favor of setting up the PGDLLIMPORT macro so that it can
just be used unconditionally.  That can work because in frontend code,
we need no marking in either the defining or consuming files for a
variable exported from these libraries; and frontend code has no need
to access variables exported from the core backend, either.

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

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

Discussion: https://postgr.es/m/1160385.1638165449@sss.pgh.pa.us
2021-11-29 11:00:00 -05:00
Tom Lane 11b500072e Portability hack for pg_global_prng_state.
PGDLLIMPORT is only appropriate for variables declared in the backend,
not when the variable is coming from a library included in frontend code.
(This isn't a particularly nice fix, but for now, use the same method
employed elsewhere.)

Discussion: https://postgr.es/m/E1mrWUD-000235-Hq@gemulon.postgresql.org
2021-11-29 00:04:45 -05:00
Michael Paquier 6fb7c5d67c Centralize timestamp computation of control file on updates
This commit moves the timestamp computation of the control file within
the routine of src/common/ in charge of updating the backend's control
file, which is shared by multiple frontend tools (pg_rewind,
pg_checksums and pg_resetwal) and the backend itself.

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

Author: Amul Sul
Reviewed-by: Nathan Bossart, Michael Paquier, Bharath Rupireddy
Discussion: https://postgr.es/m/CAAJ_b97nd_ghRpyFV9Djf9RLXkoTbOUqnocq11WGq9TisX09Fw@mail.gmail.com
2021-11-29 13:36:13 +09:00
Tom Lane 3804539e48 Replace random(), pg_erand48(), etc with a better PRNG API and algorithm.
Standardize on xoroshiro128** as our basic PRNG algorithm, eliminating
a bunch of platform dependencies as well as fundamentally-obsolete PRNG
code.  In addition, this API replacement will ease replacing the
algorithm again in future, should that become necessary.

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

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

Discussion: https://postgr.es/m/alpine.DEB.2.22.394.2105241211230.165418@pseudo
2021-11-28 21:33:07 -05:00
Michael Paquier f44ceb46ec Improve psql tab completion for views, FDWs, sequences and transforms
The following improvements are done:
- Addition of type completion for ALTER SEQUENCE AS.
- Ignore ALTER for transforms, as the command is not supported.
- Addition of more completion for ALTER FOREIGN DATA WRAPPER.
- Addition of options related to columns in ALTER VIEW.

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

Author: Ken Kato
Discussion: https://postgr.es/m/9497ae9ca1b31eb9b1e97aded1c2ab07@oss.nttdata.com
2021-11-29 10:28:29 +09:00
Peter Geoghegan 276db875d4 vacuumlazy.c: prefer the term "cleanup lock".
The term "super-exclusive lock" is an acceptable synonym of "cleanup
lock".  Even still, switching from one term to the other in the same
file is confusing.  Standardize on "cleanup lock" within vacuumlazy.c.

Per a complaint from Andres Freund.
2021-11-27 16:05:01 -08:00
Peter Geoghegan 12b5ade902 Update high level vacuumlazy.c comments.
Update vacuumlazy.c file header comments (as well as comments above the
lazy_scan_heap function) that were largely written before the
introduction of the HOT optimization, when lazy_scan_heap did far less,
and didn't actually prune during its initial heap pass.

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

Finally, remove discussion of parallel VACUUM from header comments.
These don't add much, and are in the wrong place.
2021-11-27 14:29:43 -08:00
Alvaro Herrera f744519326
Harden be-gssapi-common.h for headerscheck
Surround the contents with a test that the feature is enabled by
configure, to silence header checking tools on systems without GSSAPI
installed.

Backpatch to 12, where the file appeared.

Discussion: https://postgr.es/m/202111161709.u3pbx5lxdimt@alvherre.pgsql
2021-11-26 17:00:29 -03:00
Peter Geoghegan 1a6f5a0e87 Go back to considering HOT on pages marked full.
Commit 2fd8685e7f simplified the checking of modified attributes that
takes place within heap_update().  This included a micro-optimization
affecting pages marked PD_PAGE_FULL: don't even try to use HOT to save a
few cycles on determining HOT safety.  The assumption was that it won't
work out this time around, since it can't have worked out last time
around.

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

Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/CAH2-WznU1L3+DMPr1F7o2eJBT7=3bAJoY6ZkWABAxNt+-afyTA@mail.gmail.com
2021-11-26 10:58:38 -08:00
Alvaro Herrera 44bd3ed332
Fix determination of broken LSN in OVERWRITTEN_CONTRECORD
In commit ff9f111bce I mixed up inconsistent definitions of the LSN of
the first record in a page, when the previous record ends exactly at the
page boundary.  The correct LSN is adjusted to skip the WAL page header;
I failed to use that when setting XLogReaderState->overwrittenRecPtr,
so at WAL replay time VerifyOverwriteContrecord would refuse to let
replay continue past that record.

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

Discussion: https://postgr.es/m/45597.1637694259@sss.pgh.pa.us
2021-11-26 11:14:27 -03:00
Daniel Gustafsson 4597fd78d6 Add test for REVOKE ADMIN OPTION
The REVOKE ADMIN OPTION FOR <role_name> syntax didn't have ample
test coverage. Fix by adding coverage in the privileges test suite.

Author: Mark Dilger <mark.dilger@enterprisedb.com>
Discussion: https://postgr.es/m/333B0203-D19B-4335-AE64-90EB0FAF46F0@enterprisedb.com
2021-11-26 14:02:14 +01:00
Daniel Gustafsson b2a459edfe Fix GRANTED BY support in REVOKE ROLE statements
Commit 6aaaa76bb added support for the GRANTED BY clause in GRANT and
REVOKE statements, but missed adding support for checking the role in
the REVOKE ROLE case. Fix by checking that the parsed role matches the
CURRENT_ROLE/CURRENT_USER requirement, and also add some tests for it.
Backpatch to v14 where GRANTED BY support was introduced.

Discussion: https://postgr.es/m/B7F6699A-A984-4943-B9BF-CEB84C003527@yesql.se
Backpatch-through: 14
2021-11-26 14:02:01 +01:00
Peter Eisentraut 36cb5e7c51 Update comments
Various places wanted to point out that tuple descriptors don't
contain the variable-length fields of pg_attribute.  This started when
attacl was added, but more fields have been added since, and these
comments haven't been kept up to date consistently.  Reword so that
the purpose is clearer and we don't have to keep updating them.
2021-11-26 09:57:23 +01:00
Peter Eisentraut 99e4d24a9d Remove unneeded Python includes
Inluding <compile.h> and <eval.h> has not been necessary since Python
2.4, since they are included via <Python.h>.  Morever, <eval.h> is
being removed in Python 3.11.  So remove these includes.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/84884.1637723223%40sss.pgh.pa.us
2021-11-25 14:19:22 +01:00
Michael Paquier f0d43947a1 Block ALTER TABLE .. DROP NOT NULL on columns in replica identity index
Replica identities that depend directly on an index rely on a set of
properties, one of them being that all the columns defined in this index
have to be marked as NOT NULL.  There was a hole in the logic with ALTER
TABLE DROP NOT NULL, where it was possible to remove the NOT NULL
property of a column part of an index used as replica identity, so block
it to avoid problems with logical decoding down the road.

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

Author: Haiying Tang, Hou Zhijie
Reviewed-by: Dilip Kumar, Michael Paquier
Discussion: https://postgr.es/m/OS0PR01MB6113338C102BEE8B2FFC5BD9FB619@OS0PR01MB6113.jpnprd01.prod.outlook.com
Backpatch-through: 10
2021-11-25 15:04:56 +09:00
Michael Paquier 10260c794b Fix fstat() emulation on Windows with standard streams
The emulation of fstat() in win32stat.c caused two issues with the
existing in-core callers, failing on EINVAL when using a stream as
argument:
- psql's \copy would crash when using a stream.
- pg_recvlogical would fail with -f -.

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

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

Author: Dmitry Koval, Juan José Santamaría Flecha
Discussion: https://postgr.es/m/17288-6b58a91025a8a8a3@postgresql.org
Backpatch-through: 14
2021-11-25 12:16:21 +09:00
Andres Freund 3030903dfe Replace straggling uses of ReadRecPtr/EndRecPtr.
d2ddfa681d removed ReadRecPtr/EndRecPtr, but two uses within an #ifdef
WAL_DEBUG escaped.

Discussion: https://postgr.es/m/20211124231206.gbadj5bblcljb6d5@alap3.anarazel.de
2021-11-24 16:56:14 -08:00
Robert Haas d2ddfa681d xlog.c: Remove global variables ReadRecPtr and EndRecPtr.
In most places, the variables necessarily store the same value as the
eponymous members of the XLogReaderState that we use during WAL
replay, because ReadRecord() assigns the values from the structure
members to the global variables just after XLogReadRecord() returns.
However, XLogBeginRead() adjusts the structure members but not the
global variables, so after XLogBeginRead() and before the completion
of XLogReadRecord() the values can differ. Otherwise, they must be
identical.  According to my analysis, the only place where either
variable is referenced at a point where it might not have the same
value as the structure member is the refrence to EndRecPtr within
XLogPageRead.

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

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

Discussion: http://postgr.es/m/CA+Tgmoao96EuNeSPd+hspRKcsCddu=b1h-QNRuKfY8VmfNQdfg@mail.gmail.com
2021-11-24 11:27:39 -05:00
Robert Haas e7ea2fa342 Fix corner-case failure to detect improper timeline switch.
rescanLatestTimeLine() contains a guard against switching to
a timeline that forked off from the current one prior to the
current recovery point, but that guard does not work if the
timeline switch occurs before the first WAL recod (which must
be the checkpoint record) is read. Without this patch, an
improper timeline switch is therefore possible in such cases.

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

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

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

Patch by me, reviewed by Amul Sul.

Discussion: http://postgr.es/m/CA+Tgmoao96EuNeSPd+hspRKcsCddu=b1h-QNRuKfY8VmfNQdfg@mail.gmail.com
2021-11-24 08:13:10 -05:00
Michael Paquier f79962d826 Remove useless LZ4 system call on failure when writing file header
If an error occurs when writing the LZ4 file header, LZ4F_compressEnd()
was called in the error code path of write(), followed by
LZ4F_freeCompressionContext() to finish the cleanup.  The code as-is was
not broken, but the LZ4F_compressEnd() proves to not be necessary as
there are no contents to flush at this stage, so remove it.

Per gripe from Jeevan Ladhe and Robert Haas.

Discussion: https://postgr.es/m/CAOgcT0PE33wbD7giAT1OSkNJt=p-vu8huq++qh=ny9O=SCP5aA@mail.gmail.com
2021-11-24 20:12:54 +09:00
David Rowley 411137a429 Flush Memoize cache when non-key parameters change, take 2
It's possible that a subplan below a Memoize node contains a parameter
from above the Memoize node.  If this parameter changes then cache entries
may become out-dated due to the new parameter value.

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

Bug: #17213
Reported by: Elvis Pranskevichus
Author: David Rowley
Discussion: https://postgr.es/m/17213-988ed34b225a2862@postgresql.org
Backpatch-through: 14, where Memoize was added
2021-11-24 23:29:14 +13:00
Michael Paquier b2265d305d Add support for Visual Studio 2022 in build scripts
Documentation and any code paths related to VS are updated to keep the
whole consistent.  Similarly to 2017 and 2019, the version of VS and the
version of nmake that we use to determine which code paths to use for
the build are still inconsistent in their own way.

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

Author: Hans Buschmann
Discussion: https://postgr.es/m/1633101364685.39218@nidsa.net
Backpatch-through: 10
2021-11-24 13:03:23 +09:00
Amit Kapila 875e02c2df Rename SnapBuild* macros in slot.c.
Same macro names for SnapBuildOnDiskNotChecksummedSize and
SnapBuildOnDiskChecksummedSize are being used in slot.c and snapbuild.c.
This patch renames them, in slot.c, to
ReplicationSlotOnDiskNotChecksummedSize and
ReplicationSlotOnDiskChecksummedSize similar to the other macros. This
makes all macro names look consistent in slot.c.

Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACVZo-piDGzBOJRY4ob=_goFR6t9DhZMDMjJWN7LQs34Aw@mail.gmail.com
2021-11-24 08:09:00 +05:30
David Rowley dad20ad470 Revert "Flush Memoize cache when non-key parameters change"
This reverts commit 1050048a31.
2021-11-24 15:27:43 +13:00
David Rowley 1050048a31 Flush Memoize cache when non-key parameters change
It's possible that a subplan below a Memoize node contains a parameter
from above the Memoize node.  If this parameter changes then cache entries
may become out-dated due to the new parameter value.

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

Bug: #17213
Reported by: Elvis Pranskevichus
Author: David Rowley
Discussion: https://postgr.es/m/17213-988ed34b225a2862@postgresql.org
Backpatch-through: 14, where Memoize was added
2021-11-24 14:56:18 +13:00
David Rowley e502150f7d Allow Memoize to operate in binary comparison mode
Memoize would always use the hash equality operator for the cache key
types to determine if the current set of parameters were the same as some
previously cached set.  Certain types such as floating points where -0.0
and +0.0 differ in their binary representation but are classed as equal by
the hash equality operator may cause problems as unless the join uses the
same operator it's possible that whichever join operator is being used
would be able to distinguish the two values.  In which case we may
accidentally return in the incorrect rows out of the cache.

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

Reported-by: Tom Lane
Author: David Rowley
Discussion: https://postgr.es/m/3004308.1632952496@sss.pgh.pa.us
Backpatch-through: 14, where Memoize was added
2021-11-24 10:06:59 +13:00
Michael Paquier 1922d7c6e1 Add SQL functions to monitor the directory contents of replication slots
This commit adds a set of functions able to look at the contents of
various paths related to replication slots:
- pg_ls_logicalsnapdir, for pg_logical/snapshots/
- pg_ls_logicalmapdir, for pg_logical/mappings/
- pg_ls_replslotdir, for pg_replslot/<slot_name>/

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

Bump catalog version.

Author: Bharath Rupireddy
Reviewed-by: Nathan Bossart, Justin Pryzby
Discussion: https://postgr.es/m/CALj2ACWsfizZjMN6bzzdxOk1ADQQeSw8HhEjhmVXn_Pu+7VzLw@mail.gmail.com
2021-11-23 19:29:42 +09:00
Tom Lane b55f2b6926 Adjust pg_dump's priority ordering for casts.
When a stored expression depends on a user-defined cast, the backend
records the dependency as being on the cast's implementation function
--- or indeed, if there's no cast function involved but just
RelabelType or CoerceViaIO, no dependency is recorded at all.  This
is problematic for pg_dump, which is at risk of dumping things in the
wrong order leading to restore failures.  Given the lack of previous
reports, the risk isn't that high, but it can be demonstrated if the
cast is used in some view whose rowtype is then used as an input or
result type for some other function.  (That results in the view
getting hoisted into the functions portion of the dump, ahead of
the cast.)

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

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

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

Discussion: https://postgr.es/m/CAPL5KHoGa3uvyKp6z6m48LwCnTsK+LRQ_mcA4uKGfqAVSEjV_A@mail.gmail.com
2021-11-22 17:16:29 -05:00
Tom Lane 0b126c6a4b Fix pg_dump --inserts mode for generated columns with dropped columns.
If a table contains a generated column that's preceded by a dropped
column, dumpTableData_insert failed to account for the dropped
column, and would emit DEFAULT placeholder(s) in the wrong column(s).
This resulted in failures at restore time.  The default COPY code path
did not have this bug, likely explaining why it wasn't noticed sooner.

While we're fixing this, we can be a little smarter about the
situation: (1) avoid unnecessarily fetching the values of generated
columns, (2) omit generated columns from the output, too, if we're
using --column-inserts.  While these modes aren't expected to be
as high-performance as the COPY path, we might as well be as
efficient as we can; it doesn't add much complexity.

Per report from Дмитрий Иванов.
Back-patch to v12 where generated columns came in.

Discussion: https://postgr.es/m/CAPL5KHrkBniyQt5e1rafm5DdXvbgiiqfEQEJ9GjtVzN71Jj5pA@mail.gmail.com
2021-11-22 15:25:48 -05:00
Alvaro Herrera 2fed48f48f
Be more specific about OOM in XLogReaderAllocate
A couple of spots can benefit from an added errdetail(), which matches
what we were already doing in other places; and those that cannot
withstand errdetail() can get a more descriptive primary message.

Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Discussion: https://postgr.es/m/CALj2ACV+cX1eM03GfcA=ZMLXh5fSn1X1auJLz3yuS1duPSb9QA@mail.gmail.com
2021-11-22 13:43:43 -03:00
Alvaro Herrera 042412879e
autovacuum: Improve wording in a couple places
A few strings (one WARNING and some memory context names) in the
autovacuum code were written in a world where "worker" had no other
possible meaning than "autovacuum worker", but that's long time gone.
Be more specific about it.

Also, change the WARNING from elog() to ereport(), to add translability.

Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Nathan Bossart <bossartn@amazon.com>
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: Dilip Kumar <dilipbalaut@gmail.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CALj2ACX2UHp76dqdoZq92a7v4APFuV5wJQ+AUrb+2HURrKN=NQ@mail.gmail.com
2021-11-22 12:55:36 -03:00
Alvaro Herrera 67385544ce
Add missing words in comment
Reported by Zhihong Yu.

Discussion: https://postgr.es/m/CALNJ-vR6uZivg_XkB1zKjEXeyZDEgoYanFXB-++1kBT9yZQoUw@mail.gmail.com
2021-11-22 12:38:41 -03:00
Peter Eisentraut d6d1dfcc99 Add ABI extra field to fmgr magic block
This allows derived products to intentionally make their fmgr ABI
incompatible, with a clean error message.

Discussion: https://www.postgresql.org/message-id/flat/55215fda-db31-a045-d6b7-d6f2d2dc9920%40enterprisedb.com
2021-11-22 08:00:14 +01:00
Fujii Masao 1b06d7bac9 Report wait events for local shell commands like archive_command.
This commit introduces new wait events for archive_command,
archive_cleanup_command, restore_command and recovery_end_command.

Author: Fujii Masao
Reviewed-by: Bharath Rupireddy, Michael Paquier
Discussion: https://postgr.es/m/4ca4f920-6b48-638d-08b2-93598356f5d3@oss.nttdata.com
2021-11-22 10:28:21 +09:00
Peter Geoghegan 97f5aef609 Remove lazy_scan_heap parallel VACUUM comment block.
This doesn't belong next to very high level discussion of the tasks that
lazy_scan_heap performs.  There is already a similar, longer comment
block at the top of vacuumlazy.c that mentions lazy_scan_heap directly.
2021-11-21 16:22:57 -08:00
Tom Lane 282b6d00ab pg_receivewal, pg_recvlogical: allow canceling initial password prompt.
Previously it was impossible to terminate these programs via control-C
while they were prompting for a password.  We can fix that trivially
for their initial password prompts, by moving setup of the SIGINT
handler from just before to just after their initial GetConnection()
calls.

This fix doesn't permit escaping out of later re-prompts, but those
should be exceedingly rare, since the user's password or the server's
authentication setup would have to have changed meanwhile.  We
considered applying a fix similar to commit 46d665bc2, but that
seemed more complicated than it'd be worth.  Moreover, this way is
back-patchable, which that wasn't.

The misbehavior exists in all supported versions, so back-patch to all.

Tom Lane and Nathan Bossart

Discussion: https://postgr.es/m/747443.1635536754@sss.pgh.pa.us
2021-11-21 14:13:35 -05:00
Tom Lane 92e70796e9 Doc: update some things relevant to minimum Test::More version.
Oversights in commit 405f32fc4.

Also, add a tip (discovered the hard way) about getting Test::More
0.98 to pass its regression tests on recent Linux platforms.
2021-11-21 11:49:16 -05:00
Andrew Dunstan 405f32fc49
Require version 0.98 of Test::More for TAP tests
This means that the subtest feature will be available for use.

We expect that this change will make prairiedog go red until it is
updated, but other buildfarm animals should be fine.

Discussion: https://postgr.es/m/f5e1d308-4e33-37a7-bdf1-f6e0c75119de@dunslane.net
2021-11-20 17:54:43 -05:00
Tom Lane f4e7ae2b8a Fix SP-GiST scan initialization logic for binary-compatible cases.
Commit ac9099fc1 rearranged the logic in spgGetCache() that determines
the index's attType (nominal input data type) and leafType (actual
type stored in leaf index tuples).  Turns out this broke things for
the case where (a) the actual input data type is different from the
nominal type, (b) the opclass's config function leaves leafType
defaulted, and (c) the opclass has no "compress" function.  (b) caused
us to assign the actual input data type as leafType, and then since
that's not attType, we complained that a "compress" function is
required.  For non-polymorphic opclasses, condition (a) arises in
binary-compatible cases, such as using SP-GiST text_ops for a varchar
column, or using any opclass on a domain over its nominal input type.

To fix, use attType for leafType when the index's declared column type
is different from but binary-compatible with attType.  Do this only in
the defaulted-leafType case, to avoid overriding any explicit
selection made by the opclass.

Per bug #17294 from Ilya Anfimov.  Back-patch to v14.

Discussion: https://postgr.es/m/17294-8f6c7962ce877edc@postgresql.org
2021-11-20 14:29:56 -05:00
Tom Lane 46d665bc26 Allow psql's other uses of simple_prompt() to be interrupted by ^C.
This fills in the work left un-done by 5f1148224.  \prompt can
be canceled out of now, and so can password prompts issued during
\connect.  (We don't need to do anything for password prompts
issued during startup, because we aren't yet trapping SIGINT
at that point.)

Nathan Bossart

Discussion: https://postgr.es/m/747443.1635536754@sss.pgh.pa.us
2021-11-19 12:11:46 -05:00
Andres Freund 3b34645678 Initialize backend status reporting during bootstrap.
This allows a later commit to reduce the number of branches in performance
sensitive functions during normal running, compared to a very minor saving
during bootstrapping.

Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-By: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CAAKRu_Yeg+vh6SHNEo1+=O7e-BPX35cU0XQM=YwQRnkFyv_y+w@mail.gmail.com
2021-11-19 08:43:12 -08:00
Amit Kapila 0f0cfb4940 Fix parallel operations that prevent oldest xmin from advancing.
While determining xid horizons, we skip over backends that are running
Vacuum. We also ignore Create Index Concurrently, or Reindex Concurrently
for the purposes of computing Xmin for Vacuum. But we were not setting the
flags corresponding to these operations when they are performed in
parallel which was preventing Xid horizon from advancing.

The optimization related to skipping Create Index Concurrently, or Reindex
Concurrently operations was implemented in PG-14 but the fix is the same
for the Parallel Vacuum as well so back-patched till PG-13.

Author: Masahiko Sawada
Reviewed-by: Amit Kapila
Backpatch-through: 13
Discussion: https://postgr.es/m/CAD21AoCLQqgM1sXh9BrDFq0uzd3RBFKi=Vfo6cjjKODm0Onr5w@mail.gmail.com
2021-11-19 09:04:40 +05:30
Michael Paquier 0cd6d3b3c5 Improve psql tab completion for transforms, domains and sequences
The following improvements are done:
- Addition of some tab completion for CREATE DOMAIN.
- Addition of some tab completion for CREATE TRANSFORM.
- Addition of type completion for CREATE SEQUENCE AS.

Author: Ken Kato
Reviewed-by: Kyotaro Horiguchi, Michael Paquier
Discussion: https://postgr.es/m/8d370135aef066659eef8e8fbfa6315b@oss.nttdata.com
2021-11-19 11:02:15 +09:00
Michael Paquier ac1c7458b1 Fix quoting of ACL item in table for upgrade binary compatibility checks
Per buildfarm member prion, that runs the regression tests under a role
name that uses a hyphen.  Issue introduced by 835bcba.

Discussion: https://postgr.es/m/YZW4MvzCZ+hQ34vw@paquier.xyz
Backpatch-through: 12
2021-11-18 12:52:49 +09:00
Michael Paquier 835bcba8b8 Add table to regression tests for binary-compatibility checks in pg_upgrade
This commit adds to the main regression test suite a table with all
the in-core data types (some exceptions apply).  This table is not
dropped, so as pg_upgrade would be able to check the binary
compatibility of the types tracked in the table.  If a new type is added
in core, this part of the tests would need a refresh but the tests are
designed to fail if that were to happen.

As this is useful for upgrades and that these rely on the objects
created in the regression test suite of the old version upgraded from,
a backpatch down to 12 is done, which is the last point where a binary
incompatible change has been done (7c15cef).  This will hopefully be
enough to find out if something gets broken during the development of a
new version of Postgres, so as it is possible to take actions in
pg_upgrade itself in this case (like 0ccfc28 for sql_identifier).

An area that is not covered yet is related to external modules, which
may create their own types.  The testing infrastructure of pg_upgrade is
not integrated yet with the external modules stored in core
(src/test/modules/ or contrib/, all use the same database name for their
tests so there would be an overlap).  This could be improved in the
future.

Author: Justin Pryzby
Reviewed-by: Jacob Champion, Peter Eisentraut, Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/20201206180248.GI24052@telsasoft.com
Backpatch-through: 12
2021-11-18 10:37:15 +09:00
Tom Lane 5f1148224b Provide a variant of simple_prompt() that can be interrupted by ^C.
Up to now, you couldn't escape out of psql's \password command
by typing control-C (or other local spelling of SIGINT).  This
is pretty user-unfriendly, so improve it.  To do so, we have to
modify the functions provided by pg_get_line.c; but we don't
want to mess with psql's SIGINT handler setup, so provide an
API that lets that handler cause the cancel to occur.

This relies on the assumption that we won't do any major harm by
longjmp'ing out of fgets().  While that's obviously a little shaky,
we've long had the same assumption in the main input loop, and few
issues have been reported.

psql has some other simple_prompt() calls that could usefully
be improved the same way; for now, just deal with \password.

Nathan Bossart, minor tweaks by me

Discussion: https://postgr.es/m/747443.1635536754@sss.pgh.pa.us
2021-11-17 19:09:54 -05:00
Tom Lane a148f8bc04 Add a planner support function for starts_with().
This fills in some gaps in planner support for starts_with() and
the equivalent ^@ operator:

* A condition such as "textcol ^@ constant" can now use a regular
btree index, not only an SP-GiST index, so long as the index's
collation is C.  (This works just like "textcol LIKE 'foo%'".)

* "starts_with(textcol, constant)" can be optimized the same as
"textcol ^@ constant".

* Fixed-prefix LIKE and regex patterns are now more like starts_with()
in another way: if you apply one to an SPGiST-indexed column, you'll
get an index condition using ^@ rather than two index conditions with
>= and <.

Per a complaint from Shay Rojansky.  Patch by me; thanks to
Nathan Bossart for review.

Discussion: https://postgr.es/m/232599.1633800229@sss.pgh.pa.us
2021-11-17 16:54:12 -05:00
Tom Lane 248c3a937d Clean up error handling in pg_basebackup's walmethods.c.
The error handling here was a mess, as a result of a fundamentally
bad design (relying on errno to keep its value much longer than is
safe to assume) as well as a lot of just plain sloppiness, both as
to noticing errors at all and as to reporting the correct errno.
Moreover, the recent addition of LZ4 compression broke things
completely, because liblz4 doesn't use errno to report errors.

To improve matters, keep the error state in the DirectoryMethodData or
TarMethodData struct, and add a string field so we can handle cases
that don't set errno.  (The tar methods already had a version of this,
but it can be done more efficiently since all these cases use a
constant error string.)  Make the dir and tar methods handle errors
in basically identical ways, which they didn't before.

This requires copying errno into the state struct in a lot of places,
which is a bit tedious, but it has the virtue that we can get rid of
ad-hoc code to save and restore errno in a number of places ... not
to mention that it fixes other places that should've saved/restored
errno but neglected to.

In passing, fix some pointlessly static buffers to be ordinary
local variables.

There remains an issue about exactly how to handle errors from
fsync(), but that seems like material for its own patch.

While the LZ4 problems are new, all the rest of this is fixes for
old bugs, so backpatch to v10 where walmethods.c was introduced.

Patch by me; thanks to Michael Paquier for review.

Discussion: https://postgr.es/m/1343113.1636489231@sss.pgh.pa.us
2021-11-17 14:16:34 -05:00
Tom Lane 3cac2c8caa Handle close() failures more robustly in pg_dump and pg_basebackup.
Coverity complained that applying get_gz_error after a failed gzclose,
as we did in one place in pg_basebackup, is unsafe.  I think it's
right: it's entirely likely that the call is touching freed memory.
Change that to inspect errno, as we do for other gzclose calls.

Also, be careful to initialize errno to zero immediately before any
gzclose() call where we care about the error status.  (There are
some calls where we don't, because we already failed at some previous
step.)  This ensures that we don't get a misleadingly irrelevant
error code if gzclose() fails in a way that doesn't set errno.
We could work harder at that, but it looks to me like all such cases
are basically can't-happen if we're not misusing zlib, so it's
not worth the extra notational cruft that would be required.

Also, fix several places that simply failed to check for close-time
errors at all, mostly at some remove from the close or gzclose itself;
and one place that did check but didn't bother to report the errno.

Back-patch to v12.  These mistakes are older than that, but between
the frontend logging API changes that happened in v12 and the fact
that frontend code can't rely on %m before that, the patch would need
substantial revision to work in older branches.  It doesn't quite
seem worth the trouble given the lack of related field complaints.

Patch by me; thanks to Michael Paquier for review.

Discussion: https://postgr.es/m/1343113.1636489231@sss.pgh.pa.us
2021-11-17 13:08:25 -05:00
Tom Lane a8d8445a7b Fix display of SQL-standard function's arguments in INSERT/SELECT.
If a SQL-standard function body contains an INSERT ... SELECT statement,
any function parameters referenced within the SELECT were always printed
in $N style, rather than using the parameter name if any.  While not
strictly incorrect, this wasn't the intention, and it's inconsistent
with the way that such parameters would be printed in any other kind
of statement.

The cause is that the recursion to get_query_def from
get_insert_query_def neglected to pass down the context->namespaces
list, passing constant NIL instead.  This is a very ancient oversight,
but AFAICT it had no visible consequences before commit e717a9a18
added an outermost namespace with function parameters.  We don't allow
INSERT ... SELECT as a sub-query, except in a top-level WITH clause,
where it couldn't contain any outer references that might need to access
upper namespaces.  So although that's arguably a bug, I don't see any
point in changing it before v14.

In passing, harden the code added to get_parameter by e717a9a18 so that
it won't crash if a PARAM_EXTERN Param appears in an unexpected place.

Per report from Erki Eessaar.  Code fix by me, regression test case
by Masahiko Sawada.

Discussion: https://postgr.es/m/AM9PR01MB8268347BED344848555167FAFE949@AM9PR01MB8268.eurprd01.prod.exchangelabs.com
2021-11-17 11:31:31 -05:00
Daniel Gustafsson aa12781b0d Improve publication error messages
Commit 81d5995b4b introduced more fine-grained errormessages for
incorrect relkinds for publication, while unlogged and temporary
tables were reported with using the same message.  This provides
separate error messages for these types of relpersistence.

Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Reviewed-by: Jeevan Ladhe <jeevan.ladhe@enterprisedb.com>
Reviewed-by: Euler Taveira <euler@eulerto.com>
Discussion: https://postgr.es/m/CALj2ACW9S=AswyQHjtO6WMcsergMkCBTtzXGrM8DX26DzfeTLQ@mail.gmail.com
2021-11-17 14:40:38 +01:00
Peter Eisentraut 303d4eb1c5 Fix incorrect format placeholders 2021-11-17 07:30:30 +01:00
Michael Paquier f975fc3a35 Remove global variable "LastRec" in xlog.c
This variable is used only by StartupXLOG() now, so let's make it local
to simplify the code.

Author: Amul Sul
Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/CAAJ_b96Qd023itERBRN9Z7P2saNDT3CYvGuMO8RXwndVNN6z7g@mail.gmail.com
2021-11-17 11:04:18 +09:00
Alvaro Herrera ad26ee2825
Fix headerscheck failure in replication/worker_internal.h
Broken by 31c389d8de
2021-11-16 13:30:37 -03:00
Robert Haas e51c46991f Move InitXLogInsert() call from InitXLOGAccess() to BaseInit().
At present, there is an undocumented coding rule that you must call
RecoveryInProgress(), or do something else that results in a call
to InitXLogInsert(), before trying to write WAL. Otherwise, the
WAL construction buffers won't be initialized, resulting in
failures.

Since it's not good to rely on a status inquiry function like
RecoveryInProgress() having the side effect of initializing
critical data structures, instead do the initialization eariler,
when the backend first starts up.

Patch by me. Reviewed by Nathan Bossart and Michael Paquier.

Discussion: http://postgr.es/m/CA+TgmoY7b65qRjzHN_tWUk8B4sJqk1vj1d31uepVzmgPnZKeLg@mail.gmail.com
2021-11-16 09:43:17 -05:00
Amit Kapila 354a1f8d22 Invalidate relcache when changing REPLICA IDENTITY index.
When changing REPLICA IDENTITY INDEX to another one, the target table's
relcache was not being invalidated. This leads to skipping update/delete
operations during apply on the subscriber side as the columns required to
search corresponding rows won't get logged.

Author: Tang Haiying, Hou Zhijie
Reviewed-by: Euler Taveira, Amit Kapila
Backpatch-through: 10
Discussion: https://postgr.es/m/OS0PR01MB61133CA11630DAE45BC6AD95FB939@OS0PR01MB6113.jpnprd01.prod.outlook.com
2021-11-16 08:10:13 +05:30
Robert Haas 1b098da200 Fix thinko in bbsink_throttle_manifest_contents.
Report and diagnosis by Dmitry Dolgov.

Discussion: http://postgr.es/m/20211115162641.dmo6l32fklh64gnw@localhost
2021-11-15 14:22:13 -05:00
Peter Geoghegan b0f7425ec2 Explain pruning pgstats accounting subtleties.
Add a comment explaining why the pgstats accounting used during
opportunistic heap pruning operations (to maintain the current number of
dead tuples in the relation) needs to compensate by subtracting away the
number of new LP_DEAD items.  This is needed so it can avoid completely
forgetting about tuples that become LP_DEAD items during pruning -- they
should still count.

It seems more natural to discuss this issue at the only relevant call
site (opportunistic pruning), since the same issue does not apply to the
only other caller (the VACUUM call site).  Move everything there too.

Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-Wzm7f+A6ej650gi_ifTgbhsadVW5cujAL3punpupHff5Yg@mail.gmail.com
2021-11-12 19:45:58 -08:00
Daniel Gustafsson 05d8785af2 Document PG_TEST_NOCLEAN in TAP test README
Commit 90627cf98 added support for retaining the data directory even on
successful tests, but failed to document the environment variable which
controls retention. This adds a small note to the TAP test README about
PG_TEST_NOCLEAN which when set skips removing the data directories from
successful tests.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/2B02C1B3-3F41-4E14-92B9-005D83623A0B@yesql.se
2021-11-12 21:38:10 +01:00
Tom Lane d6eb5a0c25 Make psql's \password default to CURRENT_USER, not PQuser(conn).
The documentation says plainly that \password acts on "the current user"
by default.  What it actually acted on, or tried to, was the username
used to log into the current session.  This is not the same thing if
one has since done SET ROLE or SET SESSION AUTHENTICATION.  Aside from
the possible surprise factor, it's quite likely that the current role
doesn't have permissions to set the password of the original role.

To fix, use "SELECT CURRENT_USER" to get the role name to act on.
(This syntax works with servers at least back to 7.0.)  Also, in
hopes of reducing confusion, include the role name that will be
acted on in the password prompt.

The discrepancy from the documentation makes this a bug, so
back-patch to all supported branches.

Patch by me; thanks to Nathan Bossart for review.

Discussion: https://postgr.es/m/747443.1635536754@sss.pgh.pa.us
2021-11-12 14:55:32 -05:00
Michael Paquier a45ed975c5 Fix memory overrun when querying pg_stat_slru
pg_stat_get_slru() in pgstatfuncs.c would point to one element after the
end of the array PgStat_SLRUStats when finishing to scan its entries.
This had no direct consequences as no data from the extra memory area
was read, but static analyzers would rightfully complain here.  So let's
be clean.

While on it, this adds one regression test in the area reserved for
system views.

Reported-by: Alexander Kozhemyakin, via AddressSanitizer
Author: Kyotaro Horiguchi
Discussion: https://postgr.es/m/17280-37da556e86032070@postgresql.org
Backpatch-through: 13
2021-11-12 21:49:21 +09:00
Noah Misch 3354746910 Report any XLogReadRecord() error in XlogReadTwoPhaseData().
Buildfarm members kittiwake and tadarida have witnessed errors at this
site.  The site discarded key facts.  Back-patch to v10 (all supported
versions).

Reviewed by Michael Paquier and Tom Lane.

Discussion: https://postgr.es/m/20211107013157.GB790288@rfd.leadboat.com
2021-11-11 17:10:18 -08:00
Peter Geoghegan 42f9427aa9 Update heap_page_prune() free space map comments.
It is up to the heap_page_prune() caller to decide what to do about
updating the FSM for a page following pruning.  Update old comments that
address what we might want to do as if it was the responsibility of
heap_page_prune() itself.  heap_page_prune() doesn't have enough
high-level context to make a sensible choice.
2021-11-11 13:42:17 -08:00
Peter Geoghegan eb9baef8e9 Update another obsolete reference in vacuumlazy.c.
Addresses an oversight in commit 7ab96cf6.
2021-11-11 13:13:08 -08:00
Robert Haas beb4e9ba16 Improve performance of pgarch_readyXlog() with many status files.
Presently, the archive_status directory was scanned for each file to
archive.  When there are many status files, say because archive_command
has been failing for a long time, these directory scans can get very
slow.  With this change, the archiver remembers several files to archive
during each directory scan, speeding things up.

To ensure timeline history files are archived as quickly as possible,
XLogArchiveNotify() forces the archiver to do a new directory scan as
soon as the .ready file for one is created.

Nathan Bossart, per a long discussion involving many people. It is
not clear to me exactly who out of all those people reviewed this
particular patch.

Discussion: http://postgr.es/m/CA+TgmobhAbs2yabTuTRkJTq_kkC80-+jw=pfpypdOJ7+gAbQbw@mail.gmail.com
Discussion: http://postgr.es/m/620F3CE1-0255-4D66-9D87-0EADE866985A@amazon.com
2021-11-11 15:20:26 -05:00
Tom Lane 01ec41a5fe Fall back to unsigned int, not int, for socklen_t.
It's a coin toss which of these is a better default assumption.
However, of the machines we have in the buildfarm, the only ones
relying on the fallback socklen_t definition are ancient HPUX,
and on that platform unsigned int is the right choice.  Minor
tweak to ee3a1a5b6.

Discussion: https://postgr.es/m/1440792.1636558888@sss.pgh.pa.us
2021-11-11 10:36:39 -05:00
Alvaro Herrera 0726c764bc
Restore lock level to set vacuum flags
Commit 27838981be mistakenly reduced the lock level from exclusive to
shared that is acquired to set PGPROC->statusFlags; this was reverted
by dcfff74fb1, but failed to do so in one spot.  Fix it.

Backpatch to 14.

Noted by Andres Freund.

Discussion: https://postgr.es/m/20211111020724.ggsfhcq3krq5r4hb@alap3.anarazel.de
2021-11-11 11:03:29 -03:00
Michael Paquier 098c134556 Fix buffer overrun in unicode string normalization with empty input
PostgreSQL 13 and newer versions are directly impacted by that through
the SQL function normalize(), which would cause a call of this function
to write one byte past its allocation if using in input an empty
string after recomposing the string with NFC and NFKC.  Older versions
(v10~v12) are not directly affected by this problem as the only code
path using normalization is SASLprep in SCRAM authentication that
forbids the case of an empty string, but let's make the code more robust
anyway there so as any out-of-core callers of this function are covered.

The solution chosen to fix this issue is simple, with the addition of a
fast-exit path if the decomposed string is found as empty.  This would
only happen for an empty string as at its lowest level a codepoint would
be decomposed as itself if it has no entry in the decomposition table or
if it has a decomposition size of 0.

Some tests are added to cover this issue in v13~.  Note that an empty
string has always been considered as normalized (grammar "IS NF[K]{C,D}
NORMALIZED", through the SQL function is_normalized()) for all the
operations allowed (NFC, NFD, NFKC and NFKD) since this feature has been
introduced as of 2991ac5.  This behavior is unchanged but some tests are
added in v13~ to check after that.

I have also checked "make normalization-check" in src/common/unicode/,
while on it (works in 13~, and breaks in older stable branches
independently of this commit).

The release notes should just mention this commit for v13~.

Reported-by: Matthijs van der Vleuten
Discussion: https://postgr.es/m/17277-0c527a373794e802@postgresql.org
Backpatch-through: 10
2021-11-11 15:00:59 +09:00
Tom Lane c3b33698cf Doc: improve protocol spec for logical replication Type messages.
protocol.sgml documented the layout for Type messages, but completely
dropped the ball otherwise, failing to explain what they are, when
they are sent, or what they're good for.  While at it, do a little
copy-editing on the description of Relation messages.

In passing, adjust the comment for apply_handle_type() to make it
clearer that we choose not to do anything when receiving a Type
message, not that we think it has no use whatsoever.

Per question from Stefen Hillman.

Discussion: https://postgr.es/m/CAPgW8pMknK5pup6=T4a_UG=Cz80Rgp=KONqJmTdHfaZb0RvnFg@mail.gmail.com
2021-11-10 13:13:04 -05:00
Robert Haas 10eae82b27 Fix thinko in assertion in basebackup.c.
Commit 5a1007a508 tried to introduce
an assertion that the block size was at least twice the size of a
tar block, but I got the math wrong. My error was reported to me
off-list.
2021-11-10 10:12:20 -05:00
Robert Haas a27048cbcb More cleanup of 'ThisTimeLineID'.
In XLogCtlData, rename the structure member ThisTimeLineID to
InsertTimeLineID and update the comments to make clear that it's only
expected to be set after recovery is complete.

In StartupXLOG, replace the local variables ThisTimeLineID and
PrevTimeLineID with new local variables replayTLI and newTLI.  In the
old scheme, ThisTimeLineID was the replay TLI until we created a new
timeline, and after that the replay TLI was in PrevTimeLineID. Now,
replayTLI is the TLI from which we last replayed WAL throughout the
entire function, and newTLI is either that, or the new timeline created
upon promotion.

Remove some misleading comments from the comment block just above where
recoveryTargetTimeLineGoal and friends are declared. It's become
incorrect, not only because ThisTimeLineID as a variable is now gone,
but also because the rmgr code does not care about ThisTimeLineID and
has not since what used to be the TLI field in the page header was
repurposed to store the page checksum.

Add a comment GetFlushRecPtr that it's only supposed to be used in
normal running, and an assertion to verify that this is so.

Per some ideas from Michael Paquier and some of my own. Review by
Michael Paquier also.

Discussion: http://postgr.es/m/CA+TgmoY1a2d1AnVR3tJcKmGGkhj7GGrwiNwjtKr21dxOuLBzCQ@mail.gmail.com
2021-11-10 09:45:24 -05:00
Michael Paquier c9c401a5e1 Improve error messages for some callers of XLogReadRecord()
A couple of code paths related to logical decoding (WAL sender, slot
advancing, etc.) use XLogReadRecord(), feeding on error messages
generated by walreader.c on a failure.  All those messages have no
context, making it harder to spot from where an error could come even if
these should not happen.  All the other callers of XLogReadRecord() do
that already.

Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/YYnTH6OyOwQcAdkw@paquier.xyz
2021-11-10 12:00:33 +09:00
Jeff Davis 4168a47454 Add pg_checkpointer predefined role for CHECKPOINT command.
Any user with the privileges of pg_checkpointer can issue a CHECKPOINT
command.

Reviewed-by: Stephen Frost
Discussion: https://postgr.es/m/67a1d667e8ec228b5e07f232184c80348c5d93f4.camel%40j-davis.com
2021-11-09 16:59:14 -08:00
Tom Lane b66767b56b Fix instability in 026_overwrite_contrecord.pl test.
We've seen intermittent failures in this test on slower buildfarm
machines, which I think can be explained by assuming that autovacuum
emitted some additional WAL.  Disable autovacuum to stabilize it.

In passing, use stringwise not numeric comparison to compare
WAL file names.  Doesn't matter at present, but they are
hex strings not decimal ...

Discussion: https://postgr.es/m/1372189.1636499287@sss.pgh.pa.us
2021-11-09 18:40:19 -05:00
Robert Haas 5a1007a508 Have the server properly terminate tar archives.
Earlier versions of PostgreSQL featured a version of pg_basebackup
that wanted to edit tar archives but was too dumb to parse them
properly. The server made things easier for the client by failing
to add the two blocks of zero bytes that ought to end a tar file,
leaving it up to the client to do that.

But since commit 23a1c6578c, we
don't need this hack any more, because pg_basebackup is now smarter
and can parse tar files even if they are properly terminated! So
change the server to always properly terminate the tar files. Older
versions of pg_basebackup can't talk to new servers anyway, so
there's no compatibility break.

On the pg_basebackup side, we see still need to add the terminating
zero bytes if we're talking to an older server, but not when the
server is v15+. Hopefully at some point we'll be able to remove
some of this compatibility cruft, but it seems best to hang on to
it for now.

In passing, add a file header comment to bbstreamer_tar.c, to make
it clearer what's going on here.

Discussion: http://postgr.es/m/CA+TgmoZbNzsWwM4BE5Jb_qHncY817DYZwGf+2-7hkMQ27ZwsMQ@mail.gmail.com
2021-11-09 14:29:15 -05:00
Peter Eisentraut ee3a1a5b63 Remove check for accept() argument types
This check was used to accommodate a staggering variety in particular
in the type of the third argument of accept().  This is no longer of
concern on currently supported systems.  We can just use socklen_t in
the code and put in a simple check that substitutes int for socklen_t
if it's missing, to cover the few stragglers.

Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/3538f4c4-1886-64f2-dcff-aaad8267fb82@enterprisedb.com
2021-11-09 15:35:26 +01:00
Michael Paquier 4cd046c203 Make some comments use the term "ProcSignal" for consistency
The surroundings in procsignal.c prefer using "ProcSignal" rather than
"procsignal".

Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACX99ghPmm1M_O4r4g+YsXFjCn=qF7PeDXntLwMpht_Gdg@mail.gmail.com
2021-11-09 12:56:34 +09:00
Amit Kapila b3812d0b9b Rename some enums to use TABLE instead of REL.
Commit 5a2832465f introduced some enums to represent all tables in schema
publications and used REL in their names. Use TABLE instead of REL in
those enums to avoid confusion with other objects like SEQUENCES that can
be part of a publication in the future.

In the passing, (a) Change one of the newly introduced error messages to
make it consistent for Create and Alter commands, (b) add missing alias in
one of the SQL Statements that is used to print publications associated
with the table.

Reported-by: Tomas Vondra, Peter Smith
Author: Vignesh C
Reviewed-by: Hou Zhijie, Peter Smith
Discussion: https://www.postgresql.org/message-id/CALDaNm0OANxuJ6RXqwZsM1MSY4s19nuH3734j4a72etDwvBETQ%40mail.gmail.com
2021-11-09 08:39:33 +05:30
Robert Haas 57b5a9646d Minimal fix for unterminated tar archive problem.
Commit 23a1c6578c improved
pg_basebackup's ability to parse tar archives, but also arranged
to parse them only when we need to make some modification to the
contents of the archive. That's a problem, because the server
doesn't actually terminate tar archives. When the new parsing
logic was engaged, pg_basebackup would properly terminate the
tar file, but when it was skipped, pg_basebackup would just write
whatever it got from the server, meaning that the terminator
was missing.

Most versions of tar are willing to overlook the missing terminator, but
the AIX buildfarm animals were not. Fix by inventing a new kind of
bbstreamer that just blindly adds a terminator, and using it whenever we
don't parse the tar archive.

Discussion: http://postgr.es/m/CA+TgmoZbNzsWwM4BE5Jb_qHncY817DYZwGf+2-7hkMQ27ZwsMQ@mail.gmail.com
2021-11-08 16:36:06 -05:00
Tom Lane b0cf5444f9 Fix incorrect format placeholder.
Per buildfarm warnings.
2021-11-08 14:32:29 -05:00
Tom Lane 160c025880 libpq: reject extraneous data after SSL or GSS encryption handshake.
libpq collects up to a bufferload of data whenever it reads data from
the socket.  When SSL or GSS encryption is requested during startup,
any additional data received with the server's yes-or-no reply
remained in the buffer, and would be treated as already-decrypted data
once the encryption handshake completed.  Thus, a man-in-the-middle
with the ability to inject data into the TCP connection could stuff
some cleartext data into the start of a supposedly encryption-protected
database session.

This could probably be abused to inject faked responses to the
client's first few queries, although other details of libpq's behavior
make that harder than it sounds.  A different line of attack is to
exfiltrate the client's password, or other sensitive data that might
be sent early in the session.  That has been shown to be possible with
a server vulnerable to CVE-2021-23214.

To fix, throw a protocol-violation error if the internal buffer
is not empty after the encryption handshake.

Our thanks to Jacob Champion for reporting this problem.

Security: CVE-2021-23222
2021-11-08 11:14:56 -05:00
Tom Lane 28e2412554 Reject extraneous data after SSL or GSS encryption handshake.
The server collects up to a bufferload of data whenever it reads data
from the client socket.  When SSL or GSS encryption is requested
during startup, any additional data received with the initial
request message remained in the buffer, and would be treated as
already-decrypted data once the encryption handshake completed.
Thus, a man-in-the-middle with the ability to inject data into the
TCP connection could stuff some cleartext data into the start of
a supposedly encryption-protected database session.

This could be abused to send faked SQL commands to the server,
although that would only work if the server did not demand any
authentication data.  (However, a server relying on SSL certificate
authentication might well not do so.)

To fix, throw a protocol-violation error if the internal buffer
is not empty after the encryption handshake.

Our thanks to Jacob Champion for reporting this problem.

Security: CVE-2021-23214
2021-11-08 11:01:43 -05:00
David Rowley 39a3105678 Fix incorrect hash equality operator bug in Memoize
In v14, because we don't have a field in RestrictInfo to cache both the
left and right type's hash equality operator, we just restrict the scope
of Memoize to only when the left and right types of a RestrictInfo are the
same.

In master we add another field to RestrictInfo and cache both hash
equality operators.

Reported-by: Jaime Casanova
Author: David Rowley
Discussion: https://postgr.es/m/20210929185544.GB24346%40ahch-to
Backpatch-through: 14
2021-11-08 14:40:33 +13:00
Robert Haas ccf289745d Remove tests added by bd807be693.
The buildfarm is unhappy. It's not obvious why it doesn't like
these tests, but let's remove them until we figure it out.

Discussion: http://postgr.es/m/462618.1636171009@sss.pgh.pa.us
2021-11-07 15:32:32 -05:00
Tom Lane c3ec4f8fe8 Silence uninitialized-variable warning.
Quite a few buildfarm animals are warning about this, and lapwing
is actually failing (because -Werror).  It's a false positive AFAICS,
so no need to do more than zero the variable to start with.

Discussion: https://postgr.es/m/YYXJnUxgw9dZKxlX@paquier.xyz
2021-11-07 12:18:18 -05:00
Tom Lane 568620dfd6 contrib/sslinfo needs a fix too to make hamerkop happy.
Re-ordering the #include's is a bit problematic here because
libpq/libpq-be.h needs to include <openssl/ssl.h>.  Instead,
let's #undef the unwanted macro after all the #includes.
This is definitely uglier than the other way, but it should
work despite possible future header rearrangements.

(A look at the openssl headers indicates that X509_NAME is the
only conflicting symbol that we use.)

In passing, remove a related but long-incorrect comment in
pg_backup_archiver.h.

Discussion: https://postgr.es/m/1051867.1635720347@sss.pgh.pa.us
2021-11-07 11:33:53 -05:00
Tom Lane 27ef132a80 Doc: add some notes about performance of the List functions.
Per suggestion from Andres Freund.

Discussion: https://postgr.es/m/20211104221248.pgo4h6wvnjl6uvkb@alap3.anarazel.de
2021-11-06 19:12:51 -04:00
Andres Freund 87bb606b20 windows: Remove use of WIN32_LEAN_AND_MEAN from crashdump.c.
Since 8162464a25 we do so in win32_port.h. But it likely didn't do much
before that either, because at that point windows.h was already included via
win32_port.h.

Reported-By: Tom Lane
Discussion: https://postgr.es/m/612842.1636237461@sss.pgh.pa.us
2021-11-06 15:43:22 -07:00
Tom Lane 3c2c391dc9 Blind attempt to fix MSVC pgcrypto build.
Commit db7d1a7b0 pulled out Mkvcbuild.pm's custom support for building
contrib/pgcrypto, but neglected to inform it that that module can now
be built normally.  Or at least I guess it can now be built normally.
But this is definitely causing bowerbird to fail, since it's trying to
test a module it hasn't built.
2021-11-06 18:02:27 -04:00