postgresql.git
3 years agoRemove PostgreSQL::Test::Utils::perl2host completely
Andrew Dunstan [Fri, 18 Feb 2022 22:00:03 +0000 (17:00 -0500)]
Remove PostgreSQL::Test::Utils::perl2host completely

Commit f1ac4a74de disabled this processing, and as nothing has broken (as
expected) here we proceed to remove the routine and adjust all the call
sites.

Backpatch to release 10

Discussion: https://postgr.es/m/0ba775a2-8aa0-0d56-d780-69427cf6f33d@dunslane.net
Discussion: https://postgr.es/m/20220125023609.5ohu3nslxgoygihl@alap3.anarazel.de

3 years agoEnsure the right perl is used for TAP tests on msys
Andrew Dunstan [Fri, 18 Feb 2022 21:59:30 +0000 (16:59 -0500)]
Ensure the right perl is used for TAP tests on msys

In particular, perl with $Config{osname} = msys should only be used if
the build target is msys (which is currently buildable but not usable).
For builds targeted at native Windows, perl from the ucrt64 toolchain is
suitable.

Discussion: https://postgr.es/m/20220216210141.5glt5isg5qtwty4c@alap3.anarazel.de

3 years agoFix uninitialized variable.
Heikki Linnakangas [Sun, 20 Feb 2022 16:33:09 +0000 (18:33 +0200)]
Fix uninitialized variable.

I'm very surprised the compiler didn't warn about it. But Coverity and
Valgrind did.

3 years agoUse bitwise rotate functions in more places
John Naylor [Sun, 20 Feb 2022 06:22:08 +0000 (13:22 +0700)]
Use bitwise rotate functions in more places

There were a number of places in the code that used bespoke bit-twiddling
expressions to do bitwise rotation. While we've had pg_rotate_right32()
for a while now, we hadn't gotten around to standardizing on that. Do so
now. Since many potential call sites look more natural with the "left"
equivalent, add that function too.

Reviewed by Tom Lane and Yugo Nagata

Discussion:
https://www.postgresql.org/message-id/CAFBsxsH7c1LC0CGZ0ADCBXLHU5-%3DKNXx-r7tHYPAW51b2HK4Qw%40mail.gmail.com

3 years agodoc: Simplify description of --with-lz4
Michael Paquier [Sat, 19 Feb 2022 06:06:53 +0000 (15:06 +0900)]
doc: Simplify description of --with-lz4

LZ4 is used in much more areas of the system now than just WAL and table
data.  This commit simplifies the installation documentation of Windows
and *nix by removing any details of the areas extended when building
with LZ4.

Author: Jeevan Ladhe
Discussion: https://postgr.es/m/CANm22Cgny8AF76pitomXp603NagwKXbA4dyN2Fac4yHPebqdqg@mail.gmail.com

3 years agoFix inconsistencies in SRF checks of pg_config() and string_to_table()
Michael Paquier [Sat, 19 Feb 2022 05:58:51 +0000 (14:58 +0900)]
Fix inconsistencies in SRF checks of pg_config() and string_to_table()

The execution paths of those functions have been using a set of checks
inconsistent with any other SRF function:
- string_to_table() missed a check on expectedDesc, the tuple descriptor
expected by the caller, that should never be NULL.  Introduced in
66f1630.
- pg_config() should check for a ReturnSetInfo, and expectedDesc cannot
be NULL.  Its error messages were also inconsistent.  Introduced in
a5c43b8.

Extracted from a larger patch by the same author, in preparation for a
larger patch set aimed at refactoring the way tuplestores are created
and checked in SRF functions.

Author: Melanie Plageman
Reviewed-by: Justin Pryzby
Discussion: https://postgr.es/m/CAAKRu_azyd1Z3W_r7Ou4sorTjRCs+PxeHw1CWJeXKofkE6TuZg@mail.gmail.com

3 years agoRearrange libpq's error reporting to avoid duplicated error text.
Tom Lane [Fri, 18 Feb 2022 20:35:15 +0000 (15:35 -0500)]
Rearrange libpq's error reporting to avoid duplicated error text.

Since commit ffa2e4670, libpq accumulates text in conn->errorMessage
across a whole query cycle.  In some situations, we may report more
than one error event within a cycle: the easiest case to reach is
where we report a FATAL error message from the server, and then a
bit later we detect loss of connection.  Since, historically, each
error PGresult bears the entire content of conn->errorMessage,
this results in duplication of the FATAL message in any output that
concatenates the contents of the PGresults.

Accumulation in errorMessage still seems like a good idea, especially
in view of the number of places that did ad-hoc error concatenation
before ffa2e4670.  So to fix this, let's track how much of
conn->errorMessage has been read out into error PGresults, and only
include new text in later PGresults.  The tricky part of that is
to be sure that we never discard an error PGresult once made (else
we'd risk dropping some text, a problem much worse than duplication).
While libpq formerly did that in some code paths, a little bit of
rearrangement lets us postpone making an error PGresult at all until
we are about to return it.

A side benefit of that postponement is that it now becomes practical
to return a dummy static PGresult in cases where we hit out-of-memory
while trying to manufacture an error PGresult.  This eliminates the
admittedly-very-rare case where we'd return NULL from PQgetResult,
indicating successful query completion, even though what actually
happened was an OOM failure.

Discussion: https://postgr.es/m/ab4288f8-be5c-57fb-2400-e3e857f53e46@enterprisedb.com

3 years agoAdd support for building with ZSTD.
Robert Haas [Fri, 18 Feb 2022 18:40:31 +0000 (13:40 -0500)]
Add support for building with ZSTD.

This commit doesn't actually add anything that uses ZSTD; that will be
done separately. It just puts the basic infrastructure into place.

Jeevan Ladhe, Robert Haas, and Michael Paquier. Reviewed by Justin
Pryzby and Andres Freund.

Discussion: http://postgr.es/m/CA+TgmoatQKGd+8SjcV+bzvw4XaoEwminHjU83yG12+NXtQzTTQ@mail.gmail.com

3 years agoDon't let libpq PGEVT_CONNRESET callbacks break a PGconn.
Tom Lane [Fri, 18 Feb 2022 16:43:04 +0000 (11:43 -0500)]
Don't let libpq PGEVT_CONNRESET callbacks break a PGconn.

As currently implemented, failure of a PGEVT_CONNRESET callback
forces the PGconn into the CONNECTION_BAD state (without closing
the socket, which is inconsistent with other failure paths), and
prevents later callbacks from being called.  This seems highly
questionable, and indeed is questioned by comments in the source.

Instead, let's just ignore the result value of PGEVT_CONNRESET
calls.  Like the preceding commit, this converts event callbacks
into "pure observers" that cannot affect libpq's processing logic.

Discussion: https://postgr.es/m/3185105.1644960083@sss.pgh.pa.us

3 years agoDon't let libpq "event" procs break the state of PGresult objects.
Tom Lane [Fri, 18 Feb 2022 16:37:27 +0000 (11:37 -0500)]
Don't let libpq "event" procs break the state of PGresult objects.

As currently implemented, failure of a PGEVT_RESULTCREATE callback
causes the PGresult to be converted to an error result.  This is
intellectually inconsistent (shouldn't a failing callback likewise
prevent creation of the error result? what about side-effects on the
behavior seen by other event procs? why does PQfireResultCreateEvents
act differently from PQgetResult?), but more importantly it destroys
any promises we might wish to make about the behavior of libpq in
nontrivial operating modes, such as pipeline mode.  For example,
it's not possible to promise that PGRES_PIPELINE_SYNC results will
be returned if an event callback fails on those.  With this
definition, expecting applications to behave sanely in the face of
possibly-failing callbacks seems like a very big lift.

Hence, redefine the result of a callback failure as being simply
that that event procedure won't be called any more for this PGresult
(which was true already).  Event procedures can still signal failure
back to the application through out-of-band mechanisms, for example
via their passthrough arguments.

Similarly, don't let failure of a PGEVT_RESULTCOPY callback prevent
PQcopyResult from succeeding.  That definition allowed a misbehaving
event proc to break single-row mode (our sole internal use of
PQcopyResult), and it probably had equally deleterious effects for
outside uses.

Discussion: https://postgr.es/m/3185105.1644960083@sss.pgh.pa.us

3 years agoSuppress warning about stack_base_ptr with late-model GCC.
Tom Lane [Fri, 18 Feb 2022 03:45:34 +0000 (22:45 -0500)]
Suppress warning about stack_base_ptr with late-model GCC.

GCC 12 complains that set_stack_base is storing the address of
a local variable in a long-lived pointer.  This is an entirely
reasonable warning (indeed, it just helped us find a bug);
but that behavior is intentional here.  We can work around it
by using __builtin_frame_address(0) instead of a specific local
variable; that produces an address a dozen or so bytes different,
in my testing, but we don't care about such a small difference.
Maybe someday a compiler lacking that function will start to issue
a similar warning, but we'll worry about that when it happens.

Patch by me, per a suggestion from Andres Freund.  Back-patch to
v12, which is as far back as the patch will go without some pain.
(Recently-established project policy would permit a back-patch as
far as 9.2, but I'm disinclined to expend the work until GCC 12
is much more widespread.)

Discussion: https://postgr.es/m/3773792.1645141467@sss.pgh.pa.us

3 years agoFix comment in CheckIndexCompatible().
Fujii Masao [Fri, 18 Feb 2022 03:19:10 +0000 (12:19 +0900)]
Fix comment in CheckIndexCompatible().

Commit 5f173040 removed the parameter "heapRelation" from
CheckIndexCompatible(), but forgot to remove the mention of it
from the comment. This commit removes that unnecessary mention.

Also this commit adds the missing mention of the parameter "oldId"
in the comment.

Author: Yugo Nagata
Reviewed-by: Nathan Bossart, Fujii Masao
Discussion: https://postgr.es/m/20220204014634.b39314f278ff4ae3de96e201@sraoss.co.jp

3 years agopostgres_fdw: Make postgres_fdw.application_name support more escape sequences.
Fujii Masao [Fri, 18 Feb 2022 02:38:12 +0000 (11:38 +0900)]
postgres_fdw: Make postgres_fdw.application_name support more escape sequences.

Commit 6e0cb3dec1 allowed postgres_fdw.application_name to include
escape sequences %a (application name), %d (database name), %u (user name)
and %p (pid). In addition to them, this commit makes it support
the escape sequences for session ID (%c) and cluster name (%C).
These are helpful to investigate where each remote transactions came from.

Author: Fujii Masao
Reviewed-by: Ryohei Takahashi, Kyotaro Horiguchi
Discussion: https://postgr.es/m/1041dc9a-c976-049f-9f14-e7d94c29c4b2@oss.nttdata.com

3 years agoFix a comment in worker.c.
Amit Kapila [Fri, 18 Feb 2022 02:14:24 +0000 (07:44 +0530)]
Fix a comment in worker.c.

The comment incorrectly states that worker gets killed during
ALTER SUBSCRIPTION ... DISABLE. Remove that part of the comment.

Author: Masahiko Sawada
Discussion: https://postgr.es/m/CAD21AoCbEN==oH7BhP3U6WPHg3zgH6sDOeKhJjy4W2dx-qoVCw@mail.gmail.com

3 years agoAvoid dangling-pointer usage in pg_basebackup progress reports.
Tom Lane [Thu, 17 Feb 2022 20:03:40 +0000 (15:03 -0500)]
Avoid dangling-pointer usage in pg_basebackup progress reports.

Ill-considered refactoring in 23a1c6578 led to progress_filename
sometimes pointing to data that had gone out of scope.  The most
bulletproof fix is to hang onto a copy of whatever's passed in.
Compared to the work spent elsewhere per file, that's not very
expensive, plus we can skip it except in verbose logging mode.

Per buildfarm.

Discussion: https://postgr.es/m/20220212211316.GK31460@telsasoft.com

3 years agoAdd missing binary-upgrade guard.
Robert Haas [Thu, 17 Feb 2022 15:53:51 +0000 (10:53 -0500)]
Add missing binary-upgrade guard.

Commit 9a974cbcba005256a19991203583a94b4f9a21a9 arranged for
pg_dumpall to preserve tablespace OIDs, but it should only do that
in binary upgrade mode, not all the time.

Reported by Christoph Berg.

Discussion: http://postgr.es/m/YgjwrkEvNEqoz4Vm@msg.df7cb.de

3 years agoDisable perl2host() processing in TAP tests
Andrew Dunstan [Thu, 17 Feb 2022 14:59:59 +0000 (09:59 -0500)]
Disable perl2host() processing in TAP tests

This is a preliminary step towards removing it altogether, but this lets
us double check that nothing breaks in the buildfarm before we do.

Discussion: https://postgr.es/m/0ba775a2-8aa0-0d56-d780-69427cf6f33d@dunslane.net

3 years agoplpython: Reject Python 2 during build configuration.
Andres Freund [Thu, 17 Feb 2022 06:47:35 +0000 (22:47 -0800)]
plpython: Reject Python 2 during build configuration.

Python 2.7 went EOL 2020-01-01 and the support for Python 2 requires a fair
bit of infrastructure. Therefore we are removing Python 2 support in plpython.

This patch just rejects Python 2 during configure / mkvcbuild.pl. Future
commits will remove the code and infrastructure for Python 2 support and
adjust more of the documentation. This way we can see the buildfarm state
after the removal sooner and we can be sure that failures are due to
desupporting Python 2, rather than caused by infrastructure cleanup.

Reviewed-By: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/20211031184548.g4sxfe47n2kyi55r@alap3.anarazel.de

3 years agoIncrease hash_mem_multiplier default to 2.0.
Peter Geoghegan [Thu, 17 Feb 2022 02:41:52 +0000 (18:41 -0800)]
Increase hash_mem_multiplier default to 2.0.

Double the default setting for hash_mem_multiplier, from 1.0 to 2.0.
This setting makes hash-based executor nodes use twice the usual
work_mem limit.

The PostgreSQL 15 release notes should have a compatibility note about
this change.

Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-Wzndc_ROk6CY-bC6p9O53q974Y0Ey4WX8jcPbuTZYM4Q3A@mail.gmail.com

3 years agoAvoid VACUUM reltuples distortion.
Peter Geoghegan [Thu, 17 Feb 2022 01:15:50 +0000 (17:15 -0800)]
Avoid VACUUM reltuples distortion.

Add a heuristic that avoids distortion in the pg_class.reltuples
estimates used by VACUUM.  Without the heuristic, successive manually
run VACUUM commands (run against a table that is never modified after
initial bulk loading) will scan the same page in each VACUUM operation.
Eventually pg_class.reltuples may reach the point where one single heap
page is accidentally considered highly representative of the entire
table.  This is likely to be completely wrong, since the last heap page
typically has fewer tuples than average for the table.

It's not obvious that this was a problem prior to commit 44fa8488, which
made vacuumlazy.c consistently scan the last heap page (even when it is
all-visible in the visibility map).  It seems possible that there were
more subtle variants of the same problem that went unnoticed for quite
some time, though.  Commit 44fa8488 simplified certain aspects of when
and how relation truncation was considered, but it did not introduce the
"scan the last page" behavior.  Essentially the same behavior was
introduced much earlier, in commit e8429082.  It was conditioned on
whether or not truncation looked promising towards the end of the
initial heap pass by VACUUM until recently, which was at least somewhat
protective.  That doesn't seem like something that we should be relying
on, though.

Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-WzkNKORurux459M64mR63Aw4Jq7MBRVcX=CvALqN3A88WA@mail.gmail.com

3 years agoRemove all traces of tuplestore_donestoring() in the C code
Michael Paquier [Thu, 17 Feb 2022 00:52:02 +0000 (09:52 +0900)]
Remove all traces of tuplestore_donestoring() in the C code

This routine is a no-op since dd04e95 from 2003, with a macro kept
around for compatibility purposes.  This has led to the same code
patterns being copy-pasted around for no effect, sometimes in confusing
ways like in pg_logical_slot_get_changes_guts() from logical.c where the
code was actually incorrect.

This issue has been discussed on two different threads recently, so
rather than living with this legacy, remove any uses of this routine in
the C code to simplify things.  The compatibility macro is kept to avoid
breaking any out-of-core modules that depend on it.

Reported-by: Tatsuhito Kasahara, Justin Pryzby
Author: Tatsuhito Kasahara
Discussion: https://postgr.es/m/20211217200419.GQ17618@telsasoft.com
Discussion: https://postgr.es/m/CAP0=ZVJeeYfAeRfmzqAF2Lumdiv4S4FewyBnZd4DPTrsSQKJKw@mail.gmail.com

3 years agoFix bogus log message when starting from a cleanly shut down state.
Heikki Linnakangas [Wed, 16 Feb 2022 21:15:08 +0000 (23:15 +0200)]
Fix bogus log message when starting from a cleanly shut down state.

In commit 70e81861fa to split xlog.c, I moved the startup code that
updates the state in the control file and prints out the "database
system was not properly shut down" message to the log, but I
accidentally removed the "if (InRecovery)" check around it. As a
result, that message was printed even if the system was cleanly shut
down, also during 'initdb'.

Discussion: https://www.postgresql.org/message-id/3357075.1645031062@sss.pgh.pa.us

3 years agoAdd missing TYPEALIGN macros
John Naylor [Wed, 16 Feb 2022 12:33:28 +0000 (19:33 +0700)]
Add missing TYPEALIGN macros

A couple call sites still had hard-coded characters.

Amul Sul

Discussion: https://www.postgresql.org/message-id/CAAJ_b94Y35MWB3PJoCbc_O-_Q4%2B-9DHKhWtAwboEyx8wm4mqcA%40mail.gmail.com

3 years agoFix read beyond buffer bug introduced by the split xlog.c patch.
Heikki Linnakangas [Wed, 16 Feb 2022 10:01:32 +0000 (12:01 +0200)]
Fix read beyond buffer bug introduced by the split xlog.c patch.

FinishWalRecovery() copied the valid part of the last WAL block into a
palloc'd buffer, and the code in StartupXLOG() copied it to the WAL
buffer. But the memcpy in StartupXLOG() copied a full 8kB block, not
just the valid part, i.e. it copied from beyond the end of the buffer.
The invalid part was cleared immediately afterwards, so as long as the
memory was allocated and didn't segfault, it didn't do any harm, but
it can definitely segfault.

Discussion: https://www.postgresql.org/message-id/efc12e32-5af2-3485-5b1d-5af9f707491a@iki.fi

3 years agoReject trailing junk after numeric literals
Peter Eisentraut [Wed, 16 Feb 2022 09:32:36 +0000 (10:32 +0100)]
Reject trailing junk after numeric literals

After this, the PostgreSQL lexers no longer accept numeric literals
with trailing non-digits, such as 123abc, which would be scanned as
two tokens: 123 and abc.  This is undocumented and surprising, and it
might also interfere with some extended numeric literal syntax being
contemplated for the future.

Reviewed-by: John Naylor <john.naylor@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/b239564c-cad0-b23e-c57e-166d883cb97d@enterprisedb.com

3 years agoSplit xlog.c into xlog.c and xlogrecovery.c.
Heikki Linnakangas [Wed, 16 Feb 2022 07:30:38 +0000 (09:30 +0200)]
Split xlog.c into xlog.c and xlogrecovery.c.

This moves the functions related to performing WAL recovery into the new
xlogrecovery.c source file, leaving xlog.c responsible for maintaining
the WAL buffers, coordinating the startup and switch from recovery to
normal operations, and other miscellaneous stuff that have always been in
xlog.c.

Reviewed-by: Andres Freund, Kyotaro Horiguchi, Robert Haas
Discussion: https://www.postgresql.org/message-id/a31f27b4-a31d-f976-6217-2b03be646ffa%40iki.fi

3 years agoMove code around in StartupXLOG().
Heikki Linnakangas [Wed, 16 Feb 2022 07:22:44 +0000 (09:22 +0200)]
Move code around in StartupXLOG().

This is in preparation for the next commit, which will split off
recovery-related code from xlog.c into a new source file. This is the
order that things will happen with the next commit, and the point of
this commit is to make these ordering changes more explicit, while the
next commit mechanically moves the source code to the new file. To aid
review, I added "BEGIN/END function" comments to mark which blocks of
code are moved to which functions in the next commit. They will be gone
in the next commit.

Reviewed-by: Andres Freund, Kyotaro Horiguchi, Robert Haas
Discussion: https://www.postgresql.org/message-id/a31f27b4-a31d-f976-6217-2b03be646ffa%40iki.fi

3 years agoRefactor setting XLP_FIRST_IS_OVERWRITE_CONTRECORD.
Heikki Linnakangas [Wed, 16 Feb 2022 07:22:41 +0000 (09:22 +0200)]
Refactor setting XLP_FIRST_IS_OVERWRITE_CONTRECORD.

Set it directly in CreateOverwriteContrecordRecord(). That way,
AdvanceXLInsertBuffer() doesn't need the missingContrecPtr global
variable. This is in preparation for splitting xlog.c into multiple
files.

Reviewed-by: Robert Haas
Discussion: https://www.postgresql.org/message-id/a462d79c-cb5a-47cc-e9ac-616b5003965f%40iki.fi

3 years agoRun pgindent on xlog.c.
Heikki Linnakangas [Wed, 16 Feb 2022 07:22:34 +0000 (09:22 +0200)]
Run pgindent on xlog.c.

To tidy up after some recent refactorings in xlog.c. These would be
fixed by the pgindent run we do at the end of the development cycle,
but I want to clean these up now as I'm about to do some more big
refactorings on xlog.c.

3 years agoDoc: Update documentation for modifying postgres_fdw foreign tables.
Etsuro Fujita [Wed, 16 Feb 2022 06:15:00 +0000 (15:15 +0900)]
Doc: Update documentation for modifying postgres_fdw foreign tables.

Document that they can be modified using COPY as well.

Back-patch to v11 where commit 3d956d956 added support for COPY in
postgres_fdw.

3 years agoAdd TAP test to automate the equivalent of check_guc, take two
Michael Paquier [Wed, 16 Feb 2022 01:25:12 +0000 (10:25 +0900)]
Add TAP test to automate the equivalent of check_guc, take two

src/backend/utils/misc/check_guc is a script that cross-checks the
consistency of the GUCs with postgresql.conf.sample, making sure that
its format is in line with what guc.c has.  It has never been run
automatically, and has rotten over the years, creating a lot of false
positives as per a report from Justin Pryzby.

d10e41d has introduced a SQL function to publish the most relevant flags
associated to a GUC, with tests added in the main regression test suite
to make sure that we avoid most of the inconsistencies in the GUC
settings, based on recent reports, but there was nothing able to
cross-check postgresql.conf.sample with the contents of guc.c.

This commit adds a TAP test that covers the remaining gap.  It emulates
the most relevant checks that check_guc did, so as any format mistakes
are detected in postgresql.conf.sample at development stage, with the
following checks:
- Check that parameters marked as NOT_IN_SAMPLE are not in the sample
file.
- Check that there are no dead entries in postgresql.conf.sample for
parameters not marked as NOT_IN_SAMPLE.
- Check that no parameters are missing from the sample file if listed in
guc.c without NOT_IN_SAMPLE.

The idea of building a list of the GUCs by parsing the sample file comes
from Justin, and he wrote the regex used in the patch to find all the
GUCs (this same formatting rule basically applies for the last 20~ years
or so).  In order to test this patch, I have played with manual
modifications of postgresql.conf.sample and guc.c, making sure that we
detect problems with the GUC rules and the sample file format.

The test is located in src/test/modules/test_misc, which is the best
location I could think about for such sanity checks, rather than the
main regression test suite (src/test/regress) to avoid a new type of
dependency with the source tree.

The first attempt of this patch was b0a55f4, where the location of
postgresql.conf.sample was retrieved using pg_config --sharedir.  This
has proven to be an issue for distributions that patch pg_config to
enforce the installation paths at some wanted location (like Debian),
that may not exist when the test is run, hence causing a failure.
Instead of that, as per a suggestion from Andres Freund, rely on the
fact that the test is always executed from its directory in the source
tree and use a relative path to find the sample file.  This works for
the CI, VPATH builds and on Windows, and tests like the recovery one
added in f47ed79 rely on that already.

Reviewed-by: Justin Pryzby
Discussion: https://postgr.es/m/Yf9YGSwPiMu0c7fP@paquier.xyz

3 years agoFix race condition in 028_pitr_timelines.pl test, add note to docs.
Heikki Linnakangas [Tue, 15 Feb 2022 23:37:48 +0000 (01:37 +0200)]
Fix race condition in 028_pitr_timelines.pl test, add note to docs.

The 028_pitr_timelines.pl test would sometimes hang, waiting for a WAL
segment that was just filled up to be archived. It was because the
test used 'pg_stat_archiver.last_archived_wal' to check if a file was
archived, but the order that WAL files are archived when a standby is
promoted is not fully deterministic, and 'last_archived_wal' tracks
the last segment that was archived, not the highest-numbered WAL
segment. Because of that, if the archiver archived segment 3, and then
2, 'last_archived_wal' say 2, and the test query would think that 3
has not been archived yet.

Normally, WAL files are marked ready for archival in order, and the
archiver process will process them in order, so that issue doesn't
arise.  We have used the same query on 'last_archived_wal' in a few
other tests with no problem. But when a standby is promoted, things
are a bit chaotic. After promotion, the server will try to archive all
the WAL segments from the old timeline that are in pg_wal, as well as
the history file and any new WAL segments on the new timeline. The
end-of-recovery checkpoint will create the .ready files for all the
WAL files on the old timeline, but at the same time, the new timeline
is opened up for business. A file from the new timeline can therefore
be archived before the files from the old timeline have been marked as
ready for archival.

It turns out that we don't really need to wait for the archival in
this particular test, because the standby server is about to be
stopped, and stopping a server will wait for the end-of-recovery
checkpoint and all WAL archivals to finish, anyway. So we can just
remove it from the test.

Add a note to the docs on 'pg_stat_archiver' view that files can be
archived out of order.

Reviewed-by: Tom Lane
Discussion: https://www.postgresql.org/message-id/3186114.1644960507@sss.pgh.pa.us

3 years agoUpdate "don't truncate with failsafe" rationale.
Peter Geoghegan [Tue, 15 Feb 2022 23:16:19 +0000 (15:16 -0800)]
Update "don't truncate with failsafe" rationale.

There is a very good (though non-obvious) reason to avoid relation
truncation during a VACUUM that has triggered the failsafe mechanism,
which was missed before now.  Update related comments, so this isn't
forgotten.

Reported-By: John Naylor <john.naylor@enterprisedb.com>
Discussion: https://postgr.es/m/CAFBsxsFiMPxQ-dHZ8tOgktn=+ffeJT3+GinZ4zdOGbmAnCYadA@mail.gmail.com

3 years agoEnsure that length argument of memcmp() isn't seen as negative.
Tom Lane [Tue, 15 Feb 2022 22:28:17 +0000 (17:28 -0500)]
Ensure that length argument of memcmp() isn't seen as negative.

I think this will shut up a weird warning from buildfarm member
serinus.  Perhaps it'd be better to change tsCompareString's
length arguments to unsigned, but that seems more invasive
than is justified.

Part of a general push to remove off-the-beaten-track warnings
where we can easily do so.

3 years agoEnsure that the argument of shmdt(2) is declared "void *".
Tom Lane [Tue, 15 Feb 2022 22:17:25 +0000 (17:17 -0500)]
Ensure that the argument of shmdt(2) is declared "void *".

Our gcc-on-Solaris buildfarm members emit "incompatible pointer type"
warnings in places where it's not.  This is a bit odd, since AFAICT
Solaris follows the POSIX spec in declaring shmdt's argument as
"const void *", and you'd think any pointer argument would satisfy that.
But whatever.  Part of a general push to remove off-the-beaten-track
warnings where we can easily do so.

3 years agodocs: Work around bug in the docbook xsl stylesheets.
Andres Freund [Tue, 15 Feb 2022 21:44:22 +0000 (13:44 -0800)]
docs: Work around bug in the docbook xsl stylesheets.

docbook-xsl's index generation stylesheet (autoidx.xsl) has a small bug: It
doesn't include xlink in exclude-result-prefixes. Normally just leads to a a
single xmlns:xlink in the <div> containing the index, but because our
customization emits that, xmlns:xlink intead gets added to every element
output by autoidx.xsl below the <div>, totalling around 100kB.

Adding the spurious xmlns:xlink to the <div> ourselves isn't great, but avoids
the duplication.

Reviewed-By: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/20220213201618.qz6p6noon3wagr3f%40alap3.anarazel.de

3 years agoReject change of output-column collation in CREATE OR REPLACE VIEW.
Tom Lane [Tue, 15 Feb 2022 17:57:44 +0000 (12:57 -0500)]
Reject change of output-column collation in CREATE OR REPLACE VIEW.

checkViewTupleDesc() didn't get the memo that it should verify
same attcollation along with same type/typmod.  (A quick scan
did not find other similar oversights.)

Per bug #17404 from Pierre-Aurélien Georges.  On another day
I might've back-patched this, but today I'm feeling paranoid
about unnecessary behavioral changes in back branches.

Discussion: https://postgr.es/m/17404-8a4a270ef30a6709@postgresql.org

3 years agoEnsure that STDERR is empty in connect_ok tests
Daniel Gustafsson [Tue, 15 Feb 2022 10:35:17 +0000 (11:35 +0100)]
Ensure that STDERR is empty in connect_ok tests

Connections performed via connect_ok() in TAP tests should not write
anything to STDERR.

Author: Jacob Champion <pchampion@vmware.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/9D4FFB61-392B-4A2C-B7E4-911797B4AC14@yesql.se
Discussion: https://postgr.es/m/ec146256e31afa0542f9fa970ec258c5f1a5f98.camel@vmware.com

3 years agoFix XML namespace declarations
Peter Eisentraut [Tue, 15 Feb 2022 09:58:28 +0000 (10:58 +0100)]
Fix XML namespace declarations

The XSL stylesheets used a mix of incorrect or outdated namespace
declarations for XHTML, probably based on ancient advice and examples.
Clean all this up.

Besides improving correctness (although probably no impact in
practice, other than possible validation failures), this removes a
bunch of useless namespace declarations in the HTML output.

Reported-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/flat/20220213201618.qz6p6noon3wagr3f%40alap3.anarazel.de

3 years agoAdd more logging to new 028_pitr_timelines.pl test.
Heikki Linnakangas [Tue, 15 Feb 2022 09:55:52 +0000 (11:55 +0200)]
Add more logging to new 028_pitr_timelines.pl test.

The test has failed a couple of times on buildfarm member 'hoverfly'. It
gets stuck waiting for the standby to archive 000000020000000000000003
WAL segment. I don't understand why, but with DEBUG1, we will get messages
in the log whenever a segment is archived, which hopefully will give a
clue the next time it happens.

3 years agoRemove IS_AF_UNIX macro
Peter Eisentraut [Tue, 15 Feb 2022 09:03:52 +0000 (10:03 +0100)]
Remove IS_AF_UNIX macro

The AF_UNIX macro was being used unprotected by HAVE_UNIX_SOCKETS,
apparently since 2008.  So the redirection through IS_AF_UNIX() is
apparently no longer necessary.  (More generally, all supported
platforms are now HAVE_UNIX_SOCKETS, but even if there were a new
platform in the future, it seems plausible that it would define the
AF_UNIX symbol even without kernel support.)  So remove the
IS_AF_UNIX() macro and make the code a bit more consistent.

Discussion: https://www.postgresql.org/message-id/flat/f2d26815-9832-e333-d52d-72fbc0ade896%40enterprisedb.com

3 years agoSpell "startup process" with lower case in the documentation
John Naylor [Tue, 15 Feb 2022 07:30:57 +0000 (14:30 +0700)]
Spell "startup process" with lower case in the documentation

Most uses were already lower case, so this just makes all user-visible
spellings consistent.

Bharath Rupireddy

The proposed patch also had analagous changes for the code comments,
but I decided that wasn't worth the churn.

Discussion:
https://www.postgresql.org/message-id/flat/CALj2ACW7%2Bv_0QBPoWB%3DqKr67JKC019Htm%3DX8sKewS17bOquefg%40mail.gmail.com

3 years agoAdd test case for trailing junk after numeric literals
Peter Eisentraut [Mon, 14 Feb 2022 20:29:45 +0000 (21:29 +0100)]
Add test case for trailing junk after numeric literals

PostgreSQL currently accepts numeric literals with trailing
non-digits, such as 123abc where the abc is treated as the next token.
This may be a bit surprising.  This commit adds test cases for this;
subsequent commits intend to change this behavior.

Reviewed-by: John Naylor <john.naylor@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/b239564c-cad0-b23e-c57e-166d883cb97d@enterprisedb.com

3 years agoRemove pg_atoi()
Peter Eisentraut [Mon, 14 Feb 2022 20:29:45 +0000 (21:29 +0100)]
Remove pg_atoi()

The last caller was int2vectorin(), and having such a general function
for one user didn't seem useful, so just put the required parts inline
and remove the function.

Reviewed-by: John Naylor <john.naylor@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/b239564c-cad0-b23e-c57e-166d883cb97d@enterprisedb.com

3 years agoAdd isolation test for errors during logical slot creation.
Andres Freund [Tue, 15 Feb 2022 05:52:55 +0000 (21:52 -0800)]
Add isolation test for errors during logical slot creation.

I didn't include this test in 2f6501fa3c5, because I was not sure the error
messages for the terminated connection is stable across platforms. But it
sounds like it is, and if not, we'd want to do something about the instability
anyway...

Discussion: https://postgr.es/m/CAD21AoDAeEpAbZEyYJsPZJUmSPaRicVSBObaL7sPaofnKz+9zg@mail.gmail.com

3 years agoRemove command checks in tests of pg_basebackup and pg_receivewal
Michael Paquier [Tue, 15 Feb 2022 04:41:40 +0000 (13:41 +0900)]
Remove command checks in tests of pg_basebackup and pg_receivewal

The TAP tests of those commands have been checking if commands of "gzip"
and "lz4" existed by launching them with an extra --version.  Based on
the buildfarm, this is not required for "gzip" as the command always
exists.  Since 1d084fb, "lz4" has a ./configure check doing the same
thing.

Reported-by: Andres Freund
Discussion: https://postgr.es/m/20220212220643.ozuvq2k4cjkcnr2v@alap3.anarazel.de
Discussion: https://postgr.es/m/Ygm2ADakjlqGc2Ro@paquier.xyz

3 years agoFix thinko with subdirectories generated by pg_upgrade for internal files
Michael Paquier [Tue, 15 Feb 2022 02:46:55 +0000 (11:46 +0900)]
Fix thinko with subdirectories generated by pg_upgrade for internal files

38bfae3 has mixed the "dump/" and "log/" subdirectories generated in
"pg_upgrade_output.d/", causing the internal dump files to be generated
in "log/" and the log files to be in "dump/", but the opposite should be
done.  This was not directly an issue for pg_upgrade runs, as the
internal dump files were still picked up at the location of their
creation, but the newest version of the buildfarm client would have
reported the dump files instead of the log files on failures of
pg_upgrade.

Issue spotted while testing the TAP tests of pg_upgrade.

3 years agoMove replication slot release to before_shmem_exit().
Andres Freund [Tue, 15 Feb 2022 00:44:28 +0000 (16:44 -0800)]
Move replication slot release to before_shmem_exit().

Previously, replication slots were released in ProcKill() on error, resulting
in reporting replication slot drop of ephemeral slots after the stats
subsystem was already shut down.

To fix this problem, move replication slot release to a before_shmem_exit()
hook that is called before the stats collector shuts down. There wasn't really
a good reason for the slot handling to be in ProcKill() anyway.

Patch by Masahiko Sawada, with very minor polishing by me.

I, Andres, wrote a test for dropping slots during process exit, but there may
be some OS dependent issues around the number of times FATAL error messages
are displayed due to a still debated libpq issue. So that test will be
committed separately / later.

Reviewed-By: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-By: Andres Freund <andres@anarazel.de>
Author: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAD21AoDAeEpAbZEyYJsPZJUmSPaRicVSBObaL7sPaofnKz+9zg@mail.gmail.com

3 years agoRemove one use of pg_atoi()
Peter Eisentraut [Mon, 14 Feb 2022 20:29:45 +0000 (21:29 +0100)]
Remove one use of pg_atoi()

There was no real need to use this here instead of a simpler API.

Reviewed-by: John Naylor <john.naylor@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/b239564c-cad0-b23e-c57e-166d883cb97d@enterprisedb.com

3 years agoMove scanint8() to numutils.c
Peter Eisentraut [Mon, 14 Feb 2022 20:29:45 +0000 (21:29 +0100)]
Move scanint8() to numutils.c

Move scanint8() to numutils.c and rename to pg_strtoint64().  We
already have a "16" and "32" version of that, and the code inside the
functions was aligned, so this move makes all three versions
consistent.  The API is also changed to no longer provide the errorOK
case.  Users that need the error checking can use strtoi64().

Reviewed-by: John Naylor <john.naylor@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/b239564c-cad0-b23e-c57e-166d883cb97d@enterprisedb.com

3 years agoSuppress integer-overflow compiler warning for inconsistent sun_len.
Tom Lane [Mon, 14 Feb 2022 16:25:46 +0000 (11:25 -0500)]
Suppress integer-overflow compiler warning for inconsistent sun_len.

On AIX 7.1, struct sockaddr_un is declared to be 1025 bytes long,
but the sun_len field that should hold the length is only a byte.
Clamp the value we try to store to ensure it will fit in the field.

(This coding might need adjustment if there are any machines out
there where sun_len is as wide as size_t; but a preliminary survey
suggests there's not, so let's keep it simple.)

Discussion: https://postgr.es/m/2781112.1644819528@sss.pgh.pa.us

3 years agoDelete contrib/xml2's legacy implementation of xml_is_well_formed().
Tom Lane [Mon, 14 Feb 2022 15:56:19 +0000 (10:56 -0500)]
Delete contrib/xml2's legacy implementation of xml_is_well_formed().

This function is unreferenced in modern usage; it was superseded in 9.1
by a core function of the same name.  It has been left in place in the C
code only so that pre-9.1 SQL definitions of the contrib/xml2 functions
would continue to work.  Eleven years seems like enough time for people
to have updated to the extension-style version of the xml2 module, so
let's drop this.

We did this once before, in 20540710e, and then reverted it because
the intended change of PGDLLEXPORT markings didn't happen.  This
time the reason is to suppress link-time duplicate-symbol warnings
on AIX.  That's not worth a lot perhaps, but the value of keeping
this function has surely dropped to about zero by now.

Discussion: https://postgr.es/m/2717731.1644778752@sss.pgh.pa.us

3 years agoAdd test case for an archive recovery corner case.
Heikki Linnakangas [Mon, 14 Feb 2022 09:33:57 +0000 (11:33 +0200)]
Add test case for an archive recovery corner case.

While I was working on a patch to refactor things around xlog.c, I mixed
up EndOfLogTLI and replayTLI at the end of recovery. As a result, if you
recovered to a point with a lower-numbered timeline in a WAL segment
that has a higher TLI in the filename, the end-of-recovery WAL record
was created with invalid PrevTimeLineId. I noticed that while
self-reviewing, but no tests failed. So add a test to cover that corner
case.

Thanks to Amul Sul who also submitted a test case for the same corner
case, although this patch is different from that.

Reviewed-by: Amul Sul, Michael Paquier
Discussion: https://www.postgresql.org/message-id/52bc9ccd-8591-431b-0086-15d9acf25a3f@iki.fi
Discussion: https://www.postgresql.org/message-id/CAAJ_b94Vjt5cXGza_1MkjLQWciNdEemsmiWuQj0d%3DM7JfjAa1g%40mail.gmail.com

3 years agoAdd missing node support functions
Peter Eisentraut [Mon, 14 Feb 2022 08:11:13 +0000 (09:11 +0100)]
Add missing node support functions

forgotten in 37851a8b83d3d57ca48736093b10aa5f3bc0c177

3 years agoDatabase-level collation version tracking
Peter Eisentraut [Mon, 14 Feb 2022 07:09:04 +0000 (08:09 +0100)]
Database-level collation version tracking

This adds to database objects the same version tracking that collation
objects have.  There is a new pg_database column datcollversion that
stores the version, a new function
pg_database_collation_actual_version() to get the version from the
operating system, and a new subcommand ALTER DATABASE ... REFRESH
COLLATION VERSION.

This was not originally added together with pg_collation.collversion,
since originally version tracking was only supported for ICU, and ICU
on a database-level is not currently supported.  But we now have
version tracking for glibc (since PG13), FreeBSD (since PG14), and
Windows (since PG13), so this is useful to have now.

Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/f0ff3190-29a3-5b39-a179-fa32eee57db6%40enterprisedb.com

3 years agoImprove correlation names in sanity tests
Peter Eisentraut [Sun, 13 Feb 2022 20:07:25 +0000 (21:07 +0100)]
Improve correlation names in sanity tests

Some of the queries in the "sanity" tests in the regression test suite
(opr_sanity, type_sanity) are very confusing.  One main stumbling
block is that for some probably ancient reason many of the older
queries are written with correlation names p1, p2, etc. independent of
the name of the catalog. This one is a good example:

SELECT p1.oid, p1.oprname, p2.oid, p2.proname
FROM pg_operator AS p1, pg_proc AS p2          <-- HERE
WHERE p1.oprcode = p2.oid AND
    p1.oprkind = 'l' AND
    (p2.pronargs != 1
     OR NOT binary_coercible(p2.prorettype, p1.oprresult)
     OR NOT binary_coercible(p1.oprright, p2.proargtypes[0])
     OR p1.oprleft != 0);

This is better written as

SELECT o1.oid, o1.oprname, p1.oid, p1.proname
FROM pg_operator AS o1, pg_proc AS p1
WHERE o1.oprcode = p1.oid AND
    o1.oprkind = 'l' AND
    (p1.pronargs != 1
     OR NOT binary_coercible(p1.prorettype, o1.oprresult)
     OR NOT binary_coercible(o1.oprright, p1.proargtypes[0])
     OR o1.oprleft != 0);

This patch cleans up all the queries in this manner.

(As in the above case, I kept the digits like o1 and p1 even in cases
where only one of each letter is used in a query.  This is mainly to
keep the style consistent.)

Discussion: https://www.postgresql.org/message-id/flat/c538308b-319c-8784-e250-1284d12d5411%40enterprisedb.com

3 years agoUse WL_SOCKET_CLOSED for client_connection_check_interval.
Thomas Munro [Mon, 14 Feb 2022 03:29:44 +0000 (16:29 +1300)]
Use WL_SOCKET_CLOSED for client_connection_check_interval.

Previously we used poll() directly to check for a POLLRDHUP event.
Instead, use the WaitEventSet API to poll the socket for
WL_SOCKET_CLOSED, which knows how to detect this condition on many more
operating systems.

Reviewed-by: Zhihong Yu <zyu@yugabyte.com>
Reviewed-by: Maksim Milyutin <milyutinma@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/77def86b27e41f0efcba411460e929ae%40postgrespro.ru

3 years agoAdd WL_SOCKET_CLOSED for socket shutdown events.
Thomas Munro [Mon, 14 Feb 2022 03:29:28 +0000 (16:29 +1300)]
Add WL_SOCKET_CLOSED for socket shutdown events.

Provide a way for WaitEventSet to report that the remote peer has shut
down its socket, independently of whether there is any buffered data
remaining to be read.  This works only on systems where the kernel
exposes that information, namely:

* WAIT_USE_POLL builds using POLLRDHUP, if available
* WAIT_USE_EPOLL builds using EPOLLRDHUP
* WAIT_USE_KQUEUE builds using EV_EOF

Reviewed-by: Zhihong Yu <zyu@yugabyte.com>
Reviewed-by: Maksim Milyutin <milyutinma@gmail.com>
Discussion: https://postgr.es/m/77def86b27e41f0efcba411460e929ae%40postgrespro.ru

3 years agoWAL log unchanged toasted replica identity key attributes.
Amit Kapila [Mon, 14 Feb 2022 03:25:58 +0000 (08:55 +0530)]
WAL log unchanged toasted replica identity key attributes.

Currently, during UPDATE, the unchanged replica identity key attributes
are not logged separately because they are getting logged as part of the
new tuple. But if they are stored externally then the untoasted values are
not getting logged as part of the new tuple and logical replication won't
be able to replicate such UPDATEs. So we need to log such attributes as
part of the old_key_tuple during UPDATE.

Reported-by: Haiying Tang
Author: Dilip Kumar and Amit Kapila
Reviewed-by: Alvaro Herrera, Haiying Tang, Andres Freund
Backpatch-through: 10
Discussion: https://postgr.es/m/OS0PR01MB611342D0A92D4F4BF26C0F47FB229@OS0PR01MB6113.jpnprd01.prod.outlook.com

3 years agoTrack LLVM 15 changes.
Thomas Munro [Mon, 14 Feb 2022 02:51:43 +0000 (15:51 +1300)]
Track LLVM 15 changes.

This isn't an API change, it's just a missing #include that we got away
with before.  Per buildfarm animal seawasp.

3 years agoCorrect Makefile dependencies for catalog scripts
John Naylor [Mon, 14 Feb 2022 02:03:46 +0000 (09:03 +0700)]
Correct Makefile dependencies for catalog scripts

At some point, Gen_fmgrtab.pl stopped needing the value of defined symbols
from access/transam.h, while genbki.pl starting doing so. The Makefiles
didn't get the memo, so update the relevant dependencies.

3 years agoAdd ./configure check for "lz4" command
Michael Paquier [Mon, 14 Feb 2022 01:40:34 +0000 (10:40 +0900)]
Add ./configure check for "lz4" command

Some environments may compile with --with-lz4 while the command "lz4"
goes missing, causing two failures in the TAP tests of pg_verifybackup
(008_untar.pl and 010_client_untar.pl) as the code assumed that the
command always existed with a hardcoded value in src/Makefile.global.
Rather than this method, this adds a ./configure check based on
PGAC_PATH_PROGS() to find automatically the command and get an absolute
path to it.

Both tests need to be adjusted for the case where the command does not
exist, actually, as Makefile.global would set now LZ4 to an empty value
in this case.  The TAP tests of pg_receivewal already do that.

Per report from buildfarm member copperhead, as an effect of dab2984.
The origin of the failure is actually babbbb5 that did not centralize
the check for the existence of a "lz4" command at ./configure to shave a
few cycles.  Note that one just needs to tweak an environment to move
"lz4" out of the way to reproduce the problem, which is what I did to
test this change.

Per discussion with Robert Haas, Tom Lane, Andres Freund and myself.

Discussion: https://postgr.es/m/Ygc51WVAFGocSu4h@paquier.xyz

3 years agoFix memory leak in IndexScan node with reordering
Alexander Korotkov [Mon, 14 Feb 2022 00:26:55 +0000 (03:26 +0300)]
Fix memory leak in IndexScan node with reordering

Fix ExecReScanIndexScan() to free the referenced tuples while emptying the
priority queue.  Backpatch to all supported versions.

Discussion: https://postgr.es/m/CAHqSB9gECMENBQmpbv5rvmT3HTaORmMK3Ukg73DsX5H7EJV7jw%40mail.gmail.com
Author: Aliaksandr Kalenik
Reviewed-by: Tom Lane, Alexander Korotkov
Backpatch-through: 10

3 years agoMake origin data initialization consistent other fields in 2PC header
Michael Paquier [Mon, 14 Feb 2022 00:30:35 +0000 (09:30 +0900)]
Make origin data initialization consistent other fields in 2PC header

As of 1eb6d65, the origin data is optionally stored in a 2PC file
header, with the data filled in EndPrepare() even in the default case
where there is no origin data to add.  This was inconsistent with all
the other fields of TwoPhaseFileHeader which are initialized in
StartPrepare(), so move the initialization of origin_lsn and
origin_timestamp there instead.  The effect of missing the
initialization at this early stage is only cosmetic based on the current
logic of the code, but could have led to issues in the long-term, and it
is more consistent done this way.

Reported-by: Ranier Vilela
Discussion: https://postgr.es/m/CAEudQAooECJ+gU_RZB-yhioPOV94R4ucoHAf68PiJhLpgpVpBw@mail.gmail.com

3 years agoFix misuse of "const" qualifier.
Tom Lane [Mon, 14 Feb 2022 00:11:21 +0000 (19:11 -0500)]
Fix misuse of "const" qualifier.

"const foo *" is quite different from "foo * const".
This code was evidently trying to avoid casting away
const from the arguments, but entirely failed to do so.

Per study of some buildfarm warnings from anole
(which unfortunately are mostly ignorable, since it
seems not to understand "restrict" very well).
I'm surprised though that nothing else has complained.

3 years agoRemove REGRESS_OUTPUTDIR environment variable.
Thomas Munro [Sun, 13 Feb 2022 23:52:57 +0000 (12:52 +1300)]
Remove REGRESS_OUTPUTDIR environment variable.

Andres Freund points out that the tmp_check path is already available as
perl variable PostgreSQL::Test::Utils::tmp_check, so we can drop the new
environment variable introduced by commit f47ed79cc.

Discussion: https://postgr.es/m/20220213052955.dh7lheehit7bsemf%40alap3.anarazel.de

3 years agoSilence minor compiler warnings.
Tom Lane [Sun, 13 Feb 2022 18:06:55 +0000 (13:06 -0500)]
Silence minor compiler warnings.

Depending on compiler version and optimization level, we might
get a complaint that lazy_scan_heap's "freespace" is used
uninitialized.

Compilers not aware that ereport(ERROR) doesn't return complained
about bbsink_lz4_new().

Assigning "-1" to a uint64 value has unportable results; fortunately,
the value of xlogreadsegno is unimportant when xlogreadfd is -1.
(It looks to me like there is no need for xlogreadsegno to be static
in the first place, but I didn't venture to change that.)

3 years agoci: fix copy-paste mistake in 16eb8231d1b.
Andres Freund [Sun, 13 Feb 2022 03:38:06 +0000 (19:38 -0800)]
ci: fix copy-paste mistake in 16eb8231d1b.

Reported-By: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/20220213024704.GN31460@telsasoft.com

3 years agoci: macos: align sysinfo_script to other tasks.
Andres Freund [Sat, 12 Feb 2022 23:50:53 +0000 (15:50 -0800)]
ci: macos: align sysinfo_script to other tasks.

Author: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/20220203035827.GG23027@telsasoft.com

3 years agoci: Only use one artifact instruction for logs.
Andres Freund [Sat, 12 Feb 2022 23:29:08 +0000 (15:29 -0800)]
ci: Only use one artifact instruction for logs.

Having the different types of logs in artifacts instruction makes it quicker
to navigate between them. Either that didn't use to be possible, or I just did
it wrong. I saw that / how it works in a patch by Justin Pryzby.

Also remove a few unnecessary uses of **, suggested by Justin Pryzby.

Discussion: https://postgr.es/m/20220203195718.smqo5xg4ygp5qktq@alap3.anarazel.de

3 years agoci: s/CCACHE_SIZE/CCACHE_MAXSIZE/.
Andres Freund [Sat, 12 Feb 2022 22:44:16 +0000 (14:44 -0800)]
ci: s/CCACHE_SIZE/CCACHE_MAXSIZE/.

The wrong environment variable was used in one place.

Author: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/20220203035827.GG23027@telsasoft.com

3 years agoFix skip-empty-xacts with sequences in test_decoding
Tomas Vondra [Sat, 12 Feb 2022 22:50:42 +0000 (23:50 +0100)]
Fix skip-empty-xacts with sequences in test_decoding

Regression tests need to use skip-empty-xacts = false, because there
might be accidental concurrent activity (like autovacuum), particularly
on slow machines. The tests added by 80901b3291 failed to do that in a
couple places, triggering occasional failures on buildfarm.

Fixing the tests however uncovered a bug in the code, because sequence
callbacks did not handle skip-empty-xacts properly. For trasactional
increments we need to check/update the xact_wrote_changes flag, and emit
the BEGIN if it's the first change in the transaction.

Reported-by: Andres Freund
Discussion: https://postgr.es/m/20220212220413.b25amklo7t4xb7ni%40alap3.anarazel.de

3 years agoMove libpq's write_failed mechanism down to pqsecure_raw_write().
Tom Lane [Sat, 12 Feb 2022 19:00:09 +0000 (14:00 -0500)]
Move libpq's write_failed mechanism down to pqsecure_raw_write().

Commit 1f39a1c06 implemented write-failure postponement in pqSendSome,
which is above SSL/GSS processing.  However, we've now seen failures
indicating that (some versions of?) OpenSSL have a tendency to report
write failures prematurely too.  Hence, move the primary responsibility
for postponing write failures down to pqsecure_raw_write(), below
SSL/GSS processing.  pqSendSome now sets write_failed only in corner
cases where we'd lost the connection already.

A side-effect of this change is that errors detected in the SSL/GSS
layer itself will be reported immediately (as if they were read
errors) rather than being postponed like write errors.  That's
reverting an effect of 1f39a1c06, and I think it's fine: if there's
not a socket-level error, it's hard to be sure whether an OpenSSL
error ought to be considered a read or write failure anyway.

Another important point is that write-failure postponement is now
effective during connection setup.  OpenSSL's misbehavior of this
sort occurs during SSL_connect(), so that's a change we want.

Per bug #17391 from Nazir Bilal Yavuz.  Possibly this should be
back-patched, but I think it prudent to let it age awhile in HEAD
first.

Discussion: https://postgr.es/m/17391-304f81bcf724b58b@postgresql.org

3 years agoFix thinko in PQisBusy().
Tom Lane [Sat, 12 Feb 2022 18:23:20 +0000 (13:23 -0500)]
Fix thinko in PQisBusy().

In commit 1f39a1c06 I made PQisBusy consider conn->write_failed, but
that is now looking like complete brain fade.  In the first place, the
logic is quite wrong: it ought to be like "and not" rather than "or".
This meant that once we'd gotten into a write_failed state, PQisBusy
would always return true, probably causing the calling application to
iterate its loop until PQconsumeInput returns a hard failure thanks
to connection loss.  That's not what we want: the intended behavior
is to return an error PGresult, which the application probably has
much cleaner support for.

But in the second place, checking write_failed here seems like the
wrong thing anyway.  The idea of the write_failed mechanism is to
postpone handling of a write failure until we've read all we can from
the server; so that flag should not interfere with input-processing
behavior.  (Compare 7247e243a.)  What we *should* check for is
status = CONNECTION_BAD, ie, socket already closed.  (Most places that
close the socket don't touch asyncStatus, but they do reset status.)
This primarily ensures that if PQisBusy() returns true then there is
an open socket, which is assumed by several call sites in our own
code, and probably other applications too.

While at it, fix a nearby thinko in libpq's my_sock_write: we should
only consult errno for res < 0, not res == 0.  This is harmless since
pqsecure_raw_write would force errno to zero in such a case, but it
still could confuse readers.

Noted by Andres Freund.  Backpatch to v12 where 1f39a1c06 came in.

Discussion: https://postgr.es/m/20220211011025.ek7exh6owpzjyudn@alap3.anarazel.de

3 years agoRevert "Add TAP test to automate the equivalent of check_guc"
Michael Paquier [Sat, 12 Feb 2022 03:53:59 +0000 (12:53 +0900)]
Revert "Add TAP test to automate the equivalent of check_guc"

This reverts commit b0a55f4, to remove for now the TAP test that did the
equivalent of check_guc.  The test has been using pg_config --sharedir
to find the location of postgresql.conf.sample.  While the buildfarm and
normal build environments rather liked that, this proves to be an issue
for Debian where pg_config is patched to not be relocatable, causing the
test to fail.

Rather than relying on pg_config, we'd better find the sample file based
on its location from the source directory.  However, this is also an
issue as a TAP test only offers the build directory as of TESTDIR in the
environment context, so this would fail with VPATH builds.  Perhaps the
source path could be provided additionally when running the TAP tests.
Or perhaps we may be able to get away by just switching to a SQL
approach, by using PG_ABS_SRCDIR but this is going to require some extra
loops to get the sample file from the correct path in src/backend/.  In
any case, this needs more thoughts, so just revert the test case until
something better is done about this relocation problem.

Reported-by: Christopher Berg
Discussion: https://postgr.es/m/YgYw25OXV5men8Fj@msg.df7cb.de

3 years agoConsolidate VACUUM xid cutoff logic.
Peter Geoghegan [Sat, 12 Feb 2022 02:26:15 +0000 (18:26 -0800)]
Consolidate VACUUM xid cutoff logic.

Push the logic for determining whether or not a VACUUM operation will be
aggressive down into vacuum_set_xid_limits().  This makes the function's
signature significantly simpler, and seems clearer overall.

Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-WzkymFbz6D_vL+jmqSn_5q1wsFvFrE+37yLgL_Rkfd6Gzg@mail.gmail.com

3 years agoAdd VACUUM instrumentation for scanned pages, relfrozenxid.
Peter Geoghegan [Sat, 12 Feb 2022 00:48:40 +0000 (16:48 -0800)]
Add VACUUM instrumentation for scanned pages, relfrozenxid.

Report on scanned pages within VACUUM VERBOSE and autovacuum logging.
These are pages that were physically examined during the VACUUM
operation.  Note that this can include a small number of pages that were
marked all-visible in the visibility map by some earlier VACUUM
operation.  VACUUM won't skip all-visible pages that aren't part of a
range of all-visible pages that's at least 32 blocks in length (partly
to avoid missing out on opportunities to advance relfrozenxid during
non-aggressive VACUUMs).

Commit 44fa8488 simplified the definition of scanned pages.  It became
the complement of the pages (of those pages from rel_pages) that were
skipped using the visibility map.  And so scanned pages precisely
indicates how effective the visibility map was at saving work.  (Before
now we displayed the number of pages skipped via the visibility map when
happened to be frozen pages, but not when they were merely all-visible,
which was less useful to users.)

Rename the user-visible OldestXmin output field to "removal cutoff", and
show some supplementary information: how far behind the cutoff is
(number of XIDs behind) by the time the VACUUM operation finished.  This
will help users to figure out what's _not_ working in extreme cases
where VACUUM is fundamentally unable to remove dead tuples or freeze
older tuples (e.g., due to a leaked replication slot).  Also report when
relfrozenxid is advanced by VACUUM in output that immediately follows
"removal cutoff".  This structure is intended to highlight the
relationship between the new relfrozenxid value for the table, and the
VACUUM operation's removal cutoff.

Finally, add instrumentation of "missed dead tuples", and the number of
pages that had at least one such tuple.  These are fully DEAD (not just
RECENTLY_DEAD) tuples with storage that could not be pruned due to
failure to acquire a cleanup lock on a heap page.  This is a replacement
for the "skipped due to pin" instrumentation removed by commit 44fa8488.
It shows more details than before for pages where failing to get a
cleanup lock actually resulted in VACUUM missing out on useful work, but
usually shows nothing at all instead (the mere fact that we couldn't get
a cleanup lock is usually of no consequence whatsoever now).

Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CAH2-Wznp=c=Opj8Z7RMR3G=ec3_JfGYMN_YvmCEjoPCHzWbx0g@mail.gmail.com

3 years agoAdd decoding of sequences to test_decoding
Tomas Vondra [Fri, 11 Feb 2022 23:46:07 +0000 (00:46 +0100)]
Add decoding of sequences to test_decoding

Commit 0da92dc530 improved the logical decoding infrastructure to handle
sequences, and did various changes to related parts (WAL logging etc.).
But it did not include any implementation of the new callbacks added to
OutputPluginCallbacks.

This extends test_decoding with two callbacks to decode sequences. The
decoding of sequences may be disabled using 'include-sequences', a new
option of the output plugin.

Author: Tomas Vondra, Cary Huang
Reviewed-by: Peter Eisentraut, Hannu Krosing, Andres Freund
Discussion: https://postgr.es/m/d045f3c2-6cfb-06d3-5540-e63c320df8bc@enterprisedb.com
Discussion: https://postgr.es/m/1710ed7e13b.cd7177461430746.3372264562543607781@highgo.ca

3 years agoSimplify lazy_scan_heap's handling of scanned pages.
Peter Geoghegan [Fri, 11 Feb 2022 22:32:17 +0000 (14:32 -0800)]
Simplify lazy_scan_heap's handling of scanned pages.

Redefine a scanned page as any heap page that actually gets pinned by
VACUUM's first pass over the heap, regardless of whether or not the page
was cleanup locked.  Although it's fundamentally impossible to prune a
heap page without a cleanup lock (since we cannot safely defragment the
page), we can do just about everything else.  The only notable further
exception is freezing tuples, though even that is arguably a consequence
of not being able to prune (not a separate issue).

VACUUM now does as much of the same processing as possible for pages
that could not be cleanup locked.  Any failure to do specific required
processing is treated as a special case exception, which will be rare in
practice.  We now collect any preexisting LP_DEAD items (left behind by
earlier opportunistic pruning) in the dead_items array for these heap
pages, and count their tuples in the usual way.  Steps used to decide if
we'll attempt relation truncation are performed in the usual way for
no-cleanup-lock scanned pages, too.

Although eliminating these special cases is intrinsically useful, it's
even more useful as an enabler of further simplifications.  The only
essential difference between aggressive and non-aggressive is that only
aggressive is _guaranteed_ to be able to advance relfrozenxid up to
FreezeLimit.  Advancing relfrozenxid is always useful, but before now
non-aggressive VACUUMs threw away the opportunity to do so whenever a
cleanup lock could not be acquired on any page, no matter what the
details were.  This was very pessimistic.

It isn't actually necessary to "behave aggressively" to maintain the
ability to advance relfrozenxid when a cleanup lock isn't immediately
available (most of the time).  The non-aggressive case will now make
sure that it isn't safe to advance relfrozenxid (without waiting) using
only a share lock.  It will usually notice that there are no tuples that
need to be frozen anyway, just like in the aggressive case -- and so it
no longer wastes an opportunity to advance relfrozenxid over nothing.
(The non-aggressive case still won't wait for a cleanup lock when there
really are tuples on the page that need to be frozen, since that really
would amount to "behaving aggressively".)

VACUUM currently has a tendency to set heap pages to all-visible in the
visibility map before it freezes all of the tuples on the page.  Only a
subsequent aggressive VACUUM will visit these pages to freeze their
tuples, usually only when the tuple XIDs are much older than the
vacuum_freeze_min_age GUC (FreezeLimit cutoff) is supposed to allow.
And so non-aggressive VACUUMs are still far less likely to be able to
advance relfrozenxid in practice, even with the enhancements from this
commit.  This remaining issue will be addressed by future work that
overhauls the criteria for freezing tuples.  Once that's in place,
almost every VACUUM operation will be able to advance relfrozenxid in
practice.

Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Andres Freund <andres@anarazel.de>
Reviewed-By: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAH2-Wznp=c=Opj8Z7RMR3G=ec3_JfGYMN_YvmCEjoPCHzWbx0g@mail.gmail.com

3 years agoFix DROP {DATABASE,TABLESPACE} on Windows.
Thomas Munro [Fri, 11 Feb 2022 21:21:23 +0000 (10:21 +1300)]
Fix DROP {DATABASE,TABLESPACE} on Windows.

Previously, it was possible for DROP DATABASE, DROP TABLESPACE and ALTER
DATABASE SET TABLESPACE to fail because other backends still had file
handles open for dropped tables.  Windows won't allow a directory
containing unlinked-but-still-open files to be unlinked.  Tackle this
problem by forcing all backends to close all smgr fds.  No change for
Unix systems, which don't suffer from the problem, but the new code path
can be tested by Unix-based developers by defining
USE_BARRIER_SMGRRELEASE explicitly.

It's possible that PROCSIGNAL_BARRIER_SMGRRELEASE will have more
bug-fixing applications soon (under discussion).  Note that this is the
first user of the ProcSignalBarrier mechanism from commit 16a4e4aec.  It
could in principle be back-patched as far as 14, but since field
complaints are rare and ProcSignalBarrier hasn't been battle-tested,
that seems like a bad idea.  Fix in master only, where these failures
have started to show up in automated testing due to new tests.

Suggested-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/CA+hUKGLdemy2gBm80kz20GTe6hNVwoErE8KwcJk6-U56oStjtg@mail.gmail.com

3 years agoDon't use_physical_tlist for an IOS with non-returnable columns.
Tom Lane [Fri, 11 Feb 2022 20:23:52 +0000 (15:23 -0500)]
Don't use_physical_tlist for an IOS with non-returnable columns.

createplan.c tries to save a runtime projection step by specifying
a scan plan node's output as being exactly the table's columns, or
index's columns in the case of an index-only scan, if there is not a
reason to do otherwise.  This logic did not previously pay attention
to whether an index's columns are returnable.  That worked, sort of
accidentally, until commit 9a3ddeb51 taught setrefs.c to reject plans
that try to read a non-returnable column.  I have no desire to loosen
setrefs.c's new check, so instead adjust use_physical_tlist() to not
try to optimize this way when there are non-returnable column(s).

Per report from Ryan Kelly.  Like the previous patch, back-patch
to all supported branches.

Discussion: https://postgr.es/m/CAHUie24ddN+pDNw7fkhNrjrwAX=fXXfGZZEHhRuofV_N_ftaSg@mail.gmail.com

3 years agoReplace Test::More plans with done_testing
Daniel Gustafsson [Fri, 11 Feb 2022 19:54:44 +0000 (20:54 +0100)]
Replace Test::More plans with done_testing

Rather than doing manual book keeping to plan the number of tests to run
in each TAP suite, conclude each run with done_testing() summing up the
the number of tests that ran. This removes the need for maintaning and
updating the plan count at the expense of an accurate count of remaining
during the test suite runtime.

This patch has been discussed a number of times, often in the context of
other patches which updates tests, so a larger number of discussions can
be found in the archives.

Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Reviewed-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/DD399313-3D56-4666-8079-88949DAC870F@yesql.se

3 years agopg_basebackup: Allow client-side LZ4 (de)compression.
Robert Haas [Fri, 11 Feb 2022 14:41:42 +0000 (09:41 -0500)]
pg_basebackup: Allow client-side LZ4 (de)compression.

LZ4 compression can now be performed on the client using
pg_basebackup -Ft --compress client-lz4, and LZ4 decompression of
a backup compressed on the server can be performed on the client
using pg_basebackup -Fp --compress server-lz4.

Dipesh Pandit, reviewed and tested by Jeevan Ladhe and Tushar Ahuja,
with a few corrections - and some documentation - by me.

Discussion: http://postgr.es/m/CAN1g5_FeDmiA9D8wdG2W6Lkq5CpubxOAqTmd2et9hsinTJtsMQ@mail.gmail.com

3 years agoAdd suport for server-side LZ4 base backup compression.
Robert Haas [Fri, 11 Feb 2022 13:29:38 +0000 (08:29 -0500)]
Add suport for server-side LZ4 base backup compression.

LZ4 compression can be a lot faster than gzip compression, so users
may prefer it even if the compression ratio is not as good. We will
want pg_basebackup to support LZ4 compression and decompression on the
client side as well, and there is a pending patch for that, but it's
by a different author, so I am committing this part separately for
that reason.

Jeevan Ladhe, reviewed by Tushar Ahuja and by me.

Discussion: http://postgr.es/m/CANm22Cg9cArXEaYgHVZhCnzPLfqXCZLAzjwTq7Fc0quXRPfbxA@mail.gmail.com

3 years agoMake pg_ctl stop/restart/promote recheck postmaster aliveness.
Tom Lane [Thu, 10 Feb 2022 21:49:39 +0000 (16:49 -0500)]
Make pg_ctl stop/restart/promote recheck postmaster aliveness.

"pg_ctl stop/restart" checked that the postmaster PID is valid just
once, as a side-effect of sending the stop signal, and then would
wait-till-timeout for the postmaster.pid file to go away.  This
neglects the case wherein the postmaster dies uncleanly after we
signal it.  Similarly, once "pg_ctl promote" has sent the signal,
it'd wait for the corresponding on-disk state change to occur
even if the postmaster dies.

I'm not sure how we've managed not to notice this problem, but it
seems to explain slow execution of the 017_shm.pl test script on AIX
since commit 4fdbf9af5, which added a speculative "pg_ctl stop" with
the idea of making real sure that the postmaster isn't there.  In the
test steps that kill-9 and then restart the postmaster, it's possible
to get past the initial signal attempt before kill() stops working
for the doomed postmaster.  If that happens, pg_ctl waited till
PGCTLTIMEOUT before giving up ... and the buildfarm's AIX members
have that set very high.

To fix, include a "kill(pid, 0)" test (similar to what
postmaster_is_alive uses) in these wait loops, so that we'll
give up immediately if the postmaster PID disappears.

While here, I chose to refactor those loops out of where they were.
do_stop() and do_restart() can perfectly well share one copy of the
wait-for-stop loop, and it seems desirable to put a similar function
beside that for wait-for-promote.

Back-patch to all supported versions, since pg_ctl's wait logic
is substantially identical in all, and we're seeing the slow test
behavior in all branches.

Discussion: https://postgr.es/m/20220210023537.GA3222837@rfd.leadboat.com

3 years agoUse gendef instead of pexports for building windows .def files
Andrew Dunstan [Thu, 10 Feb 2022 18:44:05 +0000 (13:44 -0500)]
Use gendef instead of pexports for building windows .def files

Modern msys systems lack pexports but have gendef instead, so use that.

Discussion: https://postgr.es/m/3ccde7a9-e4f9-e194-30e0-0936e6ad68ba@dunslane.net

Backpatch to release 9.4 to enable building with perl on older branches.
Before that pexports is not used for plperl.

3 years agoLogical decoding of sequences
Tomas Vondra [Thu, 10 Feb 2022 17:43:28 +0000 (18:43 +0100)]
Logical decoding of sequences

This extends the logical decoding to also decode sequence increments.
We differentiate between sequences created in the current (in-progress)
transaction, and sequences created earlier. This mixed behavior is
necessary because while sequences are not transactional (increments are
not subject to ROLLBACK), relfilenode changes are. So we do this:

* Changes for sequences created in the same top-level transaction are
  treated as transactional, i.e. just like any other change from that
  transaction, and discarded in case of a rollback.

* Changes for sequences created earlier are applied immediately, as if
  performed outside any transaction. This applies also after ALTER
  SEQUENCE, which may create a new relfilenode.

Moreover, if we ever get support for DDL replication, the sequence
won't exist until the transaction gets applied.

Sequences created in the current transaction are tracked in a simple
hash table, identified by a relfilenode. That means a sequence may
already exist, but if a transaction does ALTER SEQUENCE then the
increments for the new relfilenode will be treated as transactional.

For each relfilenode we track the XID of (sub)transaction that created
it, which is needed for cleanup at transaction end. We don't need to
check the XID to decide if an increment is transactional - if we find a
match in the hash table, it has to be the same transaction.

This requires two minor changes to WAL-logging. Firstly, we need to
ensure the sequence record has a valid XID - until now the the increment
might have XID 0 if it was the first change in a subxact. But the
sequence might have been created in the same top-level transaction. So
we ensure the XID is assigned when WAL-logging increments.

The other change is addition of "created" flag, marking increments for
newly created relfilenodes. This makes it easier to maintain the hash
table of sequences that need transactional handling.
Note: This is needed because of subxacts. A XID 0 might still have the
sequence created in a different subxact of the same top-level xact.

This does not include any changes to test_decoding and/or the built-in
replication - those will be committed in separate patches.

A patch adding decoding of sequences was originally submitted by Cary
Huang. This commit reworks various important aspects (e.g. the WAL
logging and transactional/non-transactional handling). However, the
original patch and reviews were very useful.

Author: Tomas Vondra, Cary Huang
Reviewed-by: Peter Eisentraut, Hannu Krosing, Andres Freund
Discussion: https://postgr.es/m/d045f3c2-6cfb-06d3-5540-e63c320df8bc@enterprisedb.com
Discussion: https://postgr.es/m/1710ed7e13b.cd7177461430746.3372264562543607781@highgo.ca

3 years agoRemove server support for the previous base backup protocol.
Robert Haas [Tue, 8 Feb 2022 16:17:21 +0000 (11:17 -0500)]
Remove server support for the previous base backup protocol.

Commit cc333f32336f5146b75190f57ef587dff225f565 added a new COPY
sub-protocol for taking base backups, but retained support for the
previous protocol. For the same reasons articulated in the message
for commit 9cd28c2e5f11dfeef64a14035b82e70acead65fd, remove support
for the previous protocol from the server.

Discussion: http://postgr.es/m/CA+TgmoazKcKUWtqVa0xZqSzbKgTH+X-aw4V7GyLD68EpDLMh8A@mail.gmail.com

3 years agoMake timeout.c more robust against missed timer interrupts.
Tom Lane [Thu, 10 Feb 2022 16:52:20 +0000 (11:52 -0500)]
Make timeout.c more robust against missed timer interrupts.

Commit 09cf1d522 taught schedule_alarm() to not do anything if
the next requested event is after when we expect the next interrupt
to fire.  However, if somehow an interrupt gets lost, we'll continue
to not do anything indefinitely, even after the "next interrupt" time
is obviously in the past.  Thus, one missed interrupt can break
timeout scheduling for the life of the session.  Michael Harris
reported a scenario where a bug in a user-defined function caused this
to happen, so you don't even need to assume kernel bugs exist to think
this is worth fixing.  We can make things more robust at little cost
by detecting the case where signal_due_at is before "now" and forcing
a new setitimer call to occur.  This isn't a completely bulletproof
fix of course; but in our typical usage pattern where we frequently set
timeouts and clear them before they are reached, the interrupt will
get re-enabled after at most one timeout interval, which with a little
luck will be before we really need it.

While here, let's mark signal_due_at as volatile, since the signal
handler can both examine and set it.  I'm not sure there's any
actual risk given that signal_pending is already volatile, but
it's surely questionable.

Backpatch to v14 where this logic came in.

Michael Harris and Tom Lane

Discussion: https://postgr.es/m/CADofcAWbMrvgwSMqO4iG_iD3E2v8ZUrC-_crB41my=VMM02-CA@mail.gmail.com

3 years agoRemove server support for old BASE_BACKUP command syntax.
Robert Haas [Tue, 8 Feb 2022 15:53:59 +0000 (10:53 -0500)]
Remove server support for old BASE_BACKUP command syntax.

Commit 0ba281cb4bf9f5f65529dfa4c8282abb734dd454 added a new syntax
for the BASE_BACKUP command, with extensible options, but maintained
support for the legacy syntax. This isn't important for PostgreSQL,
where pg_basebackup works with older server versions but not newer
ones, but it could in theory matter for out-of-core users of the
replication protocol.

Discussion on pgsql-hackers, however, suggests that no one is aware
of any out-of-core use of the BASE_BACKUP command, and the consensus
is in favor of removing support for the old syntax to simplify the
code, so do that.

Discussion: http://postgr.es/m/CA+TgmoazKcKUWtqVa0xZqSzbKgTH+X-aw4V7GyLD68EpDLMh8A@mail.gmail.com

3 years agoSet SNI ClientHello extension to localhost in tests
Daniel Gustafsson [Thu, 10 Feb 2022 13:23:36 +0000 (14:23 +0100)]
Set SNI ClientHello extension to localhost in tests

The connection strings in the SSL client tests were using the host
set up from Cluster.pm which is a temporary pathname. When SNI is
enabled we pass the host to OpenSSL in order to set the server name
indication ClientHello extension via SSL_set_tlsext_host_name.

OpenSSL doesn't validate the hostname apart from checking the max
length, but LibreSSL checks for RFC 5890 conformance which results
in errors during testing as the pathname from Cluster.pm is not a
valid hostname.

Fix by setting the host explicitly to localhost, as that's closer
to the intent of the test.

Backpatch through 14 where SNI support came in.

Reported-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/17391-304f81bcf724b58b@postgresql.org
Backpatch-through: 14

3 years agoRemove unnecessary resetPQExpBuffer call
Peter Eisentraut [Thu, 10 Feb 2022 11:23:40 +0000 (12:23 +0100)]
Remove unnecessary resetPQExpBuffer call

Oversight in e2c52beecdea152ca680a22ef35c6a7da55aa30f.

Author: Julien Rouhaud <rjuju123@gmail.com>
Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/20220209025007.eogz2aivcnvw46ym%40jrouhaud

3 years agopsql: Rename results to result when only a single one is meant
Peter Eisentraut [Thu, 10 Feb 2022 11:03:35 +0000 (12:03 +0100)]
psql: Rename results to result when only a single one is meant

This makes the naming more consistent with the libpq API and the rest
of the code, and makes actually supporting multiple result sets in the
future less confusing.

Discussion: https://www.postgresql.org/message-id/flat/db72fb98-9b43-d776-7247-6ed38f28e7c6%40enterprisedb.com

3 years agoUpdate comment
Peter Eisentraut [Thu, 10 Feb 2022 08:16:17 +0000 (09:16 +0100)]
Update comment

Update a comment that assumed that libc collations don't support
versioning.  Also improve an adjacent error message a bit.

3 years agoAdd min() and max() aggregates for xid8.
Fujii Masao [Thu, 10 Feb 2022 03:33:41 +0000 (12:33 +0900)]
Add min() and max() aggregates for xid8.

Bump catalog version.

Author: Ken Kato
Reviewed-by: Kyotaro Horiguchi, Fujii Masao
Discussion: https://postgr.es/m/47d77b18c44f87f8222c4c7a3e2dee6b@oss.nttdata.com

3 years agoUse Test::Builder::todo_start(), replacing $::TODO.
Noah Misch [Thu, 10 Feb 2022 02:16:59 +0000 (18:16 -0800)]
Use Test::Builder::todo_start(), replacing $::TODO.

Some pre-2017 Test::More versions need perfect $Test::Builder::Level
maintenance to find the variable.  Buildfarm member snapper reported an
overall failure that the file intended to hide via the TODO construct.
That trouble was reachable in v11 and v10.  For later branches, this
serves as defense in depth.  Back-patch to v10 (all supported versions).

Discussion: https://postgr.es/m/20220202055556.GB2745933@rfd.leadboat.com

3 years agoFix typo in multixact.c
Michael Paquier [Thu, 10 Feb 2022 01:45:14 +0000 (10:45 +0900)]
Fix typo in multixact.c

Introduced in aa64f23.

Author: Nathan Bossart
Discussion: https://postgr.es/m/20220209175338.GB1627503@nathanxps13

3 years agoReduce more the number of calls to GetMaxBackends()
Michael Paquier [Thu, 10 Feb 2022 01:27:29 +0000 (10:27 +0900)]
Reduce more the number of calls to GetMaxBackends()

Some of the code paths changed by aa64f23 can reduce the number of times
GetMaxBackends() is called.  The performance gain is marginal, but most
of the code changed by this commit already did that.  Hence, let's be
clean and apply the same rule everywhere, for consistency.

Some of the code paths, like in deadlock.c, involve only assertions.
These are left unchanged.

Reviewed-by: Nathan Bossart, Robert Haas
Discussion: https://postgr.es/m/YgMpGZhPOjNfS7er@paquier.xyz

3 years agoFurther tweaks for psql's new tab-completion logic.
Tom Lane [Wed, 9 Feb 2022 22:06:21 +0000 (17:06 -0500)]
Further tweaks for psql's new tab-completion logic.

The behavior I proposed, of matching case only when only keywords
are available to complete, turns out to be too cute.  It adds about
as many problems as it removes.  Simplify down to ilmari's original
proposal of just always matching case when completing a keyword.

Also, I noticed while testing this that we've pessimized the behavior
for qualified GUC names: the code is insisting that they be
double-quoted, which was not the case before.  Fix that by treating
GUC names as verbatim matches instead of possibly-schema-qualified
names.  (While it's tempting to try to split qualified GUC names
so that we *could* treat them with the schema-qualified-name code
path, that really isn't going to work in light of guc.c's willingness
to allow more than two name components.)

Dagfinn Ilmari Mannsåker and Tom Lane

Discussion: https://postgr.es/m/445692.1644018081@sss.pgh.pa.us

3 years agoTest honestly for <sys/signalfd.h>.
Tom Lane [Wed, 9 Feb 2022 19:24:54 +0000 (14:24 -0500)]
Test honestly for <sys/signalfd.h>.

Commit 6a2a70a02 supposed that any platform having <sys/epoll.h>
would also have <sys/signalfd.h>.  It turns out there are still a
few people using platforms where that's not so, so we'd better make
a separate configure probe for it.  But since it took this long to
notice, I'm content with the decision to not have a separate code
path for epoll-only machines; we'll just fall back to using poll()
for these stragglers.

Per gripe from Gabriela Serventi.  Back-patch to v14 where this
code came in.

Discussion: https://postgr.es/m/CAHOHWE-JjJDfcYuLAAEO7Jk07atFAU47z8TzHzg71gbC0aMy=g@mail.gmail.com