postgresql.git
3 years agopageinspect: Fix handling of page sizes and AM types
Michael Paquier [Wed, 16 Mar 2022 02:20:51 +0000 (11:20 +0900)]
pageinspect: Fix handling of page sizes and AM types

This commit fixes a set of issues related to the use of the SQL
functions in this module when the caller is able to pass down raw page
data as input argument:
- The page size check was fuzzy in a couple of places, sometimes
looking after only a sub-range, but what we are looking for is an exact
match on BLCKSZ.  After considering a few options here, I have settled
down to do a generalization of get_page_from_raw().  Most of the SQL
functions already used that, and this is not strictly required if not
accessing an 8-byte-wide value from a raw page, but this feels safer in
the long run for alignment-picky environment, particularly if a code
path begins to access such values.  This also reduces the number of
strings that need to be translated.
- The BRIN function brin_page_items() uses a Relation but it did not
check the access method of the opened index, potentially leading to
crashes.  All the other functions in need of a Relation already did
that.
- Some code paths could fail on elog(), but we should to use ereport()
for failures that can be triggered by the user.

Tests are added to stress all the cases that are fixed as of this
commit, with some junk raw pages (\set VERBOSITY ensures that this works
across all page sizes) and unexpected index types when functions open
relations.

Author: Michael Paquier, Justin Prysby
Discussion: https://postgr.es/m/20220218030020[email protected]
Backpatch-through: 10

3 years agoBack-patch LLVM 14 API changes.
Thomas Munro [Tue, 15 Mar 2022 22:35:00 +0000 (11:35 +1300)]
Back-patch LLVM 14 API changes.

Since LLVM 14 has stopped changing and is about to be released,
back-patch the following changes from the master branch:

  e6a7600202105919bffd62b3dfd941f4a94e082b
  807fee1a39de6bb8184082012e643951abb9ad1d
  a56e7b66010f330782243de9e25ac2a6596be0e1

Back-patch to 11, where LLVM JIT support came in.

3 years agodoc: Add ALTER/DROP ROUTINE to the event trigger matrix
Michael Paquier [Wed, 9 Mar 2022 05:59:22 +0000 (14:59 +0900)]
doc: Add ALTER/DROP ROUTINE to the event trigger matrix

ALTER ROUTINE triggers the events ddl_command_start and ddl_command_end,
and DROP ROUTINE triggers sql_drop, ddl_command_start and
ddl_command_end, but this was not mention on the matrix table.

Reported-by: Leslie Lemaire
Discussion: https://postgr.es/m/164647533363.646.5802968483136493025@wrigleys.postgresql.org
Backpatch-through: 11

3 years agoIntroduce PG_TEST_TIMEOUT_DEFAULT for TAP suite non-elapsing timeouts.
Noah Misch [Sat, 5 Mar 2022 02:53:13 +0000 (18:53 -0800)]
Introduce PG_TEST_TIMEOUT_DEFAULT for TAP suite non-elapsing timeouts.

Slow hosts may avoid load-induced, spurious failures by setting
environment variable PG_TEST_TIMEOUT_DEFAULT to some number of seconds
greater than 180.  Developers may see faster failures by setting that
environment variable to some lesser number of seconds.  In tests, write
$PostgreSQL::Test::Utils::timeout_default wherever the convention has
been to write 180.  This change raises the default for some briefer
timeouts.  Back-patch to v10 (all supported versions).

Discussion: https://postgr.es/m/20220218052842[email protected]

3 years agoFix pg_regress to print the correct postmaster address on Windows.
Tom Lane [Fri, 4 Mar 2022 18:23:58 +0000 (13:23 -0500)]
Fix pg_regress to print the correct postmaster address on Windows.

pg_regress reported "Unix socket" as the default location whenever
HAVE_UNIX_SOCKETS is defined.  However, that's not been accurate
on Windows since 8f3ec75de.  Update this logic to match what libpq
actually does now.

This is just cosmetic, but still it's potentially misleading.
Back-patch to v13 where 8f3ec75de came in.

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

3 years agoFix bogus casting in BlockIdGetBlockNumber().
Tom Lane [Fri, 4 Mar 2022 00:03:17 +0000 (19:03 -0500)]
Fix bogus casting in BlockIdGetBlockNumber().

This macro cast the result to BlockNumber after shifting, not before,
which is the wrong thing.  Per the C spec, the uint16 fields would
promote to int not unsigned int, so that (for 32-bit int) the shift
potentially shifts a nonzero bit into the sign position.  I doubt
there are any production systems where this would actually end with
the wrong answer, but it is undefined behavior per the C spec, and
clang's -fsanitize=undefined option reputedly warns about it on some
platforms.  (I can't reproduce that right now, but the code is
undeniably wrong per spec.)  It's easy to fix by casting to
BlockNumber (uint32) in the proper places.

It's been wrong for ages, so back-patch to all supported branches.

Report and patch by Zhihong Yu (cosmetic tweaking by me)

Discussion: https://postgr.es/m/CALNJ-vT9r0DSsAOw9OXVJFxLENoVS_68kJ5x0p44atoYH+H4dg@mail.gmail.com

3 years agoClean up assorted failures under clang's -fsanitize=undefined checks.
Tom Lane [Thu, 3 Mar 2022 23:13:24 +0000 (18:13 -0500)]
Clean up assorted failures under clang's -fsanitize=undefined checks.

Most of these are cases where we could call memcpy() or other libc
functions with a NULL pointer and a zero count, which is forbidden
by POSIX even though every production version of libc allows it.
We've fixed such things before in a piecemeal way, but apparently
never made an effort to try to get them all.  I don't claim that
this patch does so either, but it gets every failure I observe in
check-world, using clang 12.0.1 on current RHEL8.

numeric.c has a different issue that the sanitizer doesn't like:
"ln(-1.0)" will compute log10(0) and then try to assign the
resulting -Inf to an integer variable.  We don't actually use the
result in such a case, so there's no live bug.

Back-patch to all supported branches, with the idea that we might
start running a buildfarm member that tests this case.  This includes
back-patching c1132aae3 (Check the size in COPY_POINTER_FIELD),
which previously silenced some of these issues in copyfuncs.c.

Discussion: https://postgr.es/m/CALNJ-vT9r0DSsAOw9OXVJFxLENoVS_68kJ5x0p44atoYH+H4dg@mail.gmail.com

3 years agoAllow root-owned SSL private keys in libpq, not only the backend.
Tom Lane [Wed, 2 Mar 2022 16:57:02 +0000 (11:57 -0500)]
Allow root-owned SSL private keys in libpq, not only the backend.

This change makes libpq apply the same private-key-file ownership
and permissions checks that we have used in the backend since commit
9a83564c5.  Namely, that the private key can be owned by either the
current user or root (with different file permissions allowed in the
two cases).  This allows system-wide management of key files, which
is just as sensible on the client side as the server, particularly
when the client is itself some application daemon.

Sync the comments about this between libpq and the backend, too.

Back-patch of a59c79564 and 50f03473e into all supported branches.

David Steele

Discussion: https://postgr.es/m/f4b7bc55-97ac-9e69-7398-335e212f7743@pgmasters.net

3 years agoDisallow execution of SPI functions during plperl function compilation.
Tom Lane [Fri, 25 Feb 2022 22:40:21 +0000 (17:40 -0500)]
Disallow execution of SPI functions during plperl function compilation.

Perl can be convinced to execute user-defined code during compilation
of a plperl function (or at least a plperlu function).  That's not
such a big problem as long as the activity is confined within the
Perl interpreter, and it's not clear we could do anything about that
anyway.  However, if such code tries to use plperl's SPI functions,
we have a bigger problem.  In the first place, those functions are
likely to crash because current_call_data->prodesc isn't set up yet.
In the second place, because it isn't set up, we lack critical info
such as whether the function is supposed to be read-only.  And in
the third place, this path allows code execution during function
validation, which is strongly discouraged because of the potential
for security exploits.  Hence, reject execution of the SPI functions
until compilation is finished.

While here, add check_spi_usage_allowed() calls to various functions
that hadn't gotten the memo about checking that.  I think that perhaps
plperl_sv_to_literal may have been intentionally omitted on the grounds
that it was safe at the time; but if so, the addition of transforms
functionality changed that.  The others are more recently added and
seem to be flat-out oversights.

Per report from Mark Murawski.  Back-patch to all supported branches.

Discussion: https://postgr.es/m/9acdf918-7fff-4f40-f750-2ffa84f083d2@intellasoft.net

3 years agopg_waldump: Fix error message for WAL files smaller than XLOG_BLCKSZ.
Andres Freund [Fri, 25 Feb 2022 18:30:05 +0000 (10:30 -0800)]
pg_waldump: Fix error message for WAL files smaller than XLOG_BLCKSZ.

When opening a WAL file smaller than XLOG_BLCKSZ (e.g. 0 bytes long) while
determining the wal_segment_size, pg_waldump checked errno, despite errno not
being set by the short read. Resulting in a bogus error message.

Author: Kyotaro Horiguchi 
Discussion: https://postgr.es/m/20220214.181847.775024684568733277[email protected]
Backpatch: 11-, the bug was introducedin fc49e24fa

3 years agoFix temporary object cleanup failing due to toast access without snapshot.
Andres Freund [Sat, 19 Feb 2022 20:27:20 +0000 (12:27 -0800)]
Fix temporary object cleanup failing due to toast access without snapshot.

When cleaning up temporary objects during process exit the cleanup could fail
with:
  FATAL: cannot fetch toast data without an active snapshot

The bug is caused by RemoveTempRelationsCallback() not setting up a
snapshot. If an object with toasted catalog data needs to be cleaned up,
init_toast_snapshot() could fail with the above error.

Most of the time however the the problem is masked due to cached catalog
snapshots being returned by GetOldestSnapshot(). But dropping an object can
cause catalog invalidations to be emitted. If no further catalog accesses are
necessary between the invalidation processing and the next toast datum
deletion, the bug becomes visible.

It's easy to miss this bug because it typically happens after clients
disconnect and the FATAL error just ends up in the log.

Luckily temporary table cleanup at the next use of the same temporary schema
or during DISCARD ALL does not have the same problem.

Fix the bug by pushing a snapshot in RemoveTempRelationsCallback(). Also add
isolation tests for temporary object cleanup, including objects with toasted
catalog data.

A future HEAD only commit will add more assertions.

Reported-By: Miles Delahunty
Author: Andres Freund
Discussion: https://postgr.es/m/CAOFAq3BU5Mf2TTvu8D9n_ZOoFAeQswuzk7yziAb7xuw_qyw5gw@mail.gmail.com
Backpatch: 10-

3 years agoRemove most msys special processing in TAP tests
Andrew Dunstan [Sun, 20 Feb 2022 16:49:33 +0000 (11:49 -0500)]
Remove most msys special processing in TAP tests

Following migration of Windows buildfarm members running TAP tests to
use of ucrt64 perl for those tests, special processing for msys perl is
no longer necessary and so is removed.

Backpatch to release 10

Discussion: https://postgr.es/m/c65a8781-77ac-ea95-d185-6db291e1baeb@dunslane.net

3 years agoRemove PostgreSQL::Test::Utils::perl2host completely
Andrew Dunstan [Sun, 20 Feb 2022 13:55:06 +0000 (08:55 -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[email protected]

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 agoDoc: Update documentation for modifying postgres_fdw foreign tables.
Etsuro Fujita [Wed, 16 Feb 2022 06:15:04 +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 agoWAL log unchanged toasted replica identity key attributes.
Amit Kapila [Mon, 14 Feb 2022 02:54:44 +0000 (08:24 +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 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 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[email protected]

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 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[email protected]

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 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[email protected]

3 years agoFix back-patch of "Avoid race in RelationBuildDesc() ..."
Noah Misch [Thu, 10 Feb 2022 02:16:56 +0000 (18:16 -0800)]
Fix back-patch of "Avoid race in RelationBuildDesc() ..."

The back-patch of commit fdd965d074d46765c295223b119ca437dbcac973 broke
CLOBBER_CACHE_ALWAYS for v9.6 through v13.  It updated the
InvalidateSystemCaches() call for CLOBBER_CACHE_RECURSIVELY, neglecting
the one for CLOBBER_CACHE_ALWAYS.  Back-patch to v13, v12, v11, and v10.

Reviewed by Tomas Vondra.  Reported by Tomas Vondra.

Discussion: https://postgr.es/m/df7b4c0b-7d92-f03f-75c4-9e08b269a716@enterprisedb.com

3 years agoRemove ppport.h's broken re-implementation of eval_pv().
Tom Lane [Wed, 9 Feb 2022 00:25:56 +0000 (19:25 -0500)]
Remove ppport.h's broken re-implementation of eval_pv().

Recent versions of Devel::PPPort try to redefine eval_pv() to
dodge a bug in pre-5.31 Perl versions.  Unfortunately the redefinition
fails on compilers that don't support statements nested within
expressions.  However, we aren't actually interested in this bug fix,
since we always call eval_pv() with croak_on_error = FALSE.
So, until there's an upstream fix for this breakage, just comment
out the macro to revert to the older behavior.

Per report from Wei Sun, as well as previous buildfarm failure
on pademelon (which I'd unfortunately not looked at carefully
enough to understand the cause).  Back-patch to all supported
versions, since we're using the same ppport.h in all.

Discussion: https://postgr.es/m/[email protected]
Report: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=pademelon&dt=2022-02-02%2001%3A22%3A58

3 years agoStamp 13.6. REL_13_6
Tom Lane [Mon, 7 Feb 2022 21:17:41 +0000 (16:17 -0500)]
Stamp 13.6.

3 years agoTranslation updates
Peter Eisentraut [Mon, 7 Feb 2022 12:36:22 +0000 (13:36 +0100)]
Translation updates

Source-Git-URL: git://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: 9aa8bc576f9af5c61de4a6fc8119abfa36493d01

3 years agoRelease notes for 14.2, 13.6, 12.10, 11.15, 10.20.
Tom Lane [Sun, 6 Feb 2022 19:24:55 +0000 (14:24 -0500)]
Release notes for 14.2, 13.6, 12.10, 11.15, 10.20.

3 years agoDoc: be clearer that foreign-table partitions need user-added constraints.
Tom Lane [Sat, 5 Feb 2022 17:55:44 +0000 (12:55 -0500)]
Doc: be clearer that foreign-table partitions need user-added constraints.

A very well-informed user might deduce this from what we said already,
but I'd bet against it.  Lay it out explicitly.

While here, rewrite the comment about tuple routing to be more
intelligible to an average SQL user.

Per bug #17395 from Alexander Lakhin.  Back-patch to v11.  (The text
in this area is different in v10 and I'm not sufficiently excited
about this point to adapt the patch.)

Discussion: https://postgr.es/m/17395-8c326292078d1a57@postgresql.org

3 years agoTest, don't just Assert, that mergejoin's inputs are in order.
Tom Lane [Sat, 5 Feb 2022 16:59:30 +0000 (11:59 -0500)]
Test, don't just Assert, that mergejoin's inputs are in order.

There are two Asserts in nodeMergejoin.c that are reachable if
the input data is not in the expected order.  This seems way too
fragile.  Alexander Lakhin reported a case where the assertions
could be triggered with misconfigured foreign-table partitions,
and bitter experience with unstable operating system collation
definitions suggests another easy route to hitting them.  Neither
Assert is in a place where we can't afford one more test-and-branch,
so replace 'em with plain test-and-elog logic.

Per bug #17395.  While the reported symptom is relatively recent,
collation changes could happen anytime, so back-patch to all
supported branches.

Discussion: https://postgr.es/m/17395-8c326292078d1a57@postgresql.org

3 years agodoc: clarify syntax notation, particularly parentheses
Bruce Momjian [Thu, 3 Feb 2022 02:53:51 +0000 (21:53 -0500)]
doc:  clarify syntax notation, particularly parentheses

Also move TCL syntax to the PL/tcl section.

Reported-by: [email protected]
Discussion: https://postgr.es/m/164308146320.12460.3590769444508751574@wrigleys.postgresql.org

Backpatch-through: 10

3 years agodoc: Fix mistake in PL/Python documentation
Peter Eisentraut [Wed, 2 Feb 2022 08:14:26 +0000 (09:14 +0100)]
doc: Fix mistake in PL/Python documentation

Small thinko introduced by 94aceed317730953476bec490ce0148b2af3c383

Reported-by: [email protected]
3 years agoReplace use of deprecated Python module distutils.sysconfig, take 2.
Tom Lane [Wed, 2 Feb 2022 00:03:41 +0000 (19:03 -0500)]
Replace use of deprecated Python module distutils.sysconfig, take 2.

With Python 3.10, configure spits out warnings about the module
distutils.sysconfig being deprecated and scheduled for removal in
Python 3.12.  Change the uses in configure to use the module sysconfig
instead.  The logic stays largely the same, although we have to
rely on INCLUDEPY instead of the deprecated get_python_inc function.

Note that sysconfig exists since Python 2.7, so this moves the
minimum required version up from Python 2.6 (or 2.4, before v13).
Also, sysconfig didn't exist in Python 3.1, so the minimum 3.x
version is now 3.2.

Back-patch of commit bd233bdd8 into all supported branches.

In v10, this also includes back-patching v11's beff4bb9c, primarily
because this opinion is clearly out-of-date:

    While at it, get rid of the code's assumption that both the major and
    minor numbers contain exactly one digit.  That will foreseeably be
    broken by Python 3.10 in perhaps four or five years.  That's far enough
    out that we probably don't need to back-patch this.

Peter Eisentraut, Tom Lane, Andres Freund

Discussion: https://postgr.es/m/c74add3c-09c4-a9dd-1a03-a846e5b2fc52@enterprisedb.com

3 years agoRevert "plperl: Fix breakage of c89f409749c in back branches."
Tom Lane [Mon, 31 Jan 2022 20:07:39 +0000 (15:07 -0500)]
Revert "plperl: Fix breakage of c89f409749c in back branches."

This reverts commits d81cac47a et al.  We shouldn't need that
hack after the preceding commits.

Discussion: https://postgr.es/m/20220131015130[email protected]

3 years agoplperl: update ppport.h to Perl 5.34.0.
Tom Lane [Mon, 31 Jan 2022 20:01:05 +0000 (15:01 -0500)]
plperl: update ppport.h to Perl 5.34.0.

Also apply the changes suggested by running
perl ppport.h --compat-version=5.8.0

And remove some no-longer-required NEED_foo declarations.

Dagfinn Ilmari Mannsåker

Back-patch of commit 05798c9f7 into all supported branches.
At the time we thought this update was mostly cosmetic, but the
lack of it has caused trouble, while the patch itself hasn't.

Discussion: https://postgr.es/m/[email protected]
Discussion: https://postgr.es/m/20220131015130[email protected]

3 years agoplperl: Fix breakage of c89f409749c in back branches.
Andres Freund [Mon, 31 Jan 2022 01:53:53 +0000 (17:53 -0800)]
plperl: Fix breakage of c89f409749c in back branches.

ppport.h was only updated in 05798c9f7f0 (master). Unfortunately my commit
c89f409749c uses PERL_VERSION_LT which came in with that update. Breaking most
buildfarm animals.

I should have noticed that...

We might want to backpatch the ppport update instead, but for now lets get the
buildfarm green again.

Discussion: https://postgr.es/m/20220131015130[email protected]
Backpatch: 10-14, master doesn't need it

3 years agoplperl: windows: Use Perl_setlocale on 5.28+, fixing compile failure.
Andres Freund [Sun, 30 Jan 2022 22:29:04 +0000 (14:29 -0800)]
plperl: windows: Use Perl_setlocale on 5.28+, fixing compile failure.

For older versions we need our own copy of perl's setlocale(), because it was
not exposed (why we need the setlocale in the first place is explained in
plperl_init_interp) . The copy stopped working in 5.28, as some of the used
macros are not public anymore.  But Perl_setlocale is available in 5.28, so
use that.

Author: Victor Wagner 
Reviewed-By: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/20200501134711.08750c5f@antares.wagner.home
Backpatch: all versions

3 years agoFix failure to validate the result of select_common_type().
Tom Lane [Sat, 29 Jan 2022 16:41:12 +0000 (11:41 -0500)]
Fix failure to validate the result of select_common_type().

Although select_common_type() has a failure-return convention, an
apparent successful return just provides a type OID that *might* work
as a common supertype; we've not validated that the required casts
actually exist.  In the mainstream use-cases that doesn't matter,
because we'll proceed to invoke coerce_to_common_type() on each input,
which will fail appropriately if the proposed common type doesn't
actually work.  However, a few callers didn't read the (nonexistent)
fine print, and thought that if they got back a nonzero OID then the
coercions were sure to work.

This affects in particular the recently-added "anycompatible"
polymorphic types; we might think that a function/operator using
such types matches cases it really doesn't.  A likely end result
of that is unexpected "ambiguous operator" errors, as for example
in bug #17387 from James Inform.  Another, much older, case is that
the parser might try to transform an "x IN (list)" construct to
a ScalarArrayOpExpr even when the list elements don't actually have
a common supertype.

It doesn't seem desirable to add more checking to select_common_type
itself, as that'd just slow down the mainstream use-cases.  Instead,
write a separate function verify_common_type that performs the
missing checks, and add a call to that where necessary.  Likewise add
verify_common_type_from_oids to go with select_common_type_from_oids.

Back-patch to v13 where the "anycompatible" types came in.  (The
symptom complained of in bug #17387 doesn't appear till v14, but
that's just because we didn't get around to converting || to use
anycompatible till then.)  In principle the "x IN (list)" fix could
go back all the way, but I'm not currently convinced that it makes
much difference in real-world cases, so I won't bother for now.

Discussion: https://postgr.es/m/17387-5dfe54b988444963@postgresql.org

3 years agoFix ordering of XIDs in ProcArrayApplyRecoveryInfo
Tomas Vondra [Thu, 27 Jan 2022 16:53:53 +0000 (17:53 +0100)]
Fix ordering of XIDs in ProcArrayApplyRecoveryInfo

Commit 8431e296ea reworked ProcArrayApplyRecoveryInfo to sort XIDs
before adding them to KnownAssignedXids. But the XIDs are sorted using
xidComparator, which compares the XIDs simply as uint32 values, not
logically. KnownAssignedXidsAdd() however expects XIDs in logical order,
and calls TransactionIdFollowsOrEquals() to enforce that. If there are
XIDs for which the two orderings disagree, an error is raised and the
recovery fails/restarts.

Hitting this issue is fairly easy - you just need two transactions, one
started before the 4B limit (e.g. XID 4294967290), the other sometime
after it (e.g. XID 1000). Logically (4294967290 <= 1000) but when
compared using xidComparator we try to add them in the opposite order.
Which makes KnownAssignedXidsAdd() fail with an error like this:

  ERROR: out-of-order XID insertion in KnownAssignedXids

This only happens during replica startup, while processing RUNNING_XACTS
records to build the snapshot. Once we reach STANDBY_SNAPSHOT_READY, we
skip these records. So this does not affect already running replicas,
but if you restart (or create) a replica while there are transactions
with XIDs for which the two orderings disagree, you may hit this.

Long-running transactions and frequent replica restarts increase the
likelihood of hitting this issue. Once the replica gets into this state,
it can't be started (even if the old transactions are terminated).

Fixed by sorting the XIDs logically - this is fine because we're dealing
with normal XIDs (because it's XIDs assigned to backends) and from the
same wraparound epoch (otherwise the backends could not be running at
the same time on the primary node). So there are no problems with the
triangle inequality, which is why xidComparator compares raw values.

Investigation and root cause analysis by Abhijit Menon-Sen. Patch by me.

This issue is present in all releases since 9.4, however releases up to
9.6 are EOL already so backpatch to 10 only.

Reviewed-by: Abhijit Menon-Sen
Reviewed-by: Alvaro Herrera
Backpatch-through: 10
Discussion: https://postgr.es/m/36b8a501-5d73-277c-4972-f58a4dce088a%40enterprisedb.com

3 years agoOn sparc64+ext4, suppress test failures from known WAL read failure.
Noah Misch [Thu, 27 Jan 2022 02:06:19 +0000 (18:06 -0800)]
On sparc64+ext4, suppress test failures from known WAL read failure.

Buildfarm members kittiwake, tadarida and snapper began to fail
frequently when commits 3cd9c3b921977272e6650a5efbeade4203c4bca2 and
f47ed79cc8a0cfa154dc7f01faaf59822552363f added tests of concurrency, but
the problem was reachable before those commits.  Back-patch to v10 (all
supported versions).

Discussion: https://postgr.es/m/20220116210241[email protected]

3 years agoFix pg_hba_file_rules for authentication method cert
Magnus Hagander [Wed, 26 Jan 2022 08:52:41 +0000 (09:52 +0100)]
Fix pg_hba_file_rules for authentication method cert

For authentication method cert, clientcert=verify-full is implied. But
the pg_hba_file_rules entry would incorrectly show clientcert=verify-ca.

Per bug #17354

Reported-By: Feike Steenbergen
Reviewed-By: Jonathan Katz
Backpatch-through: 12

3 years agoRevert "graceful shutdown" changes for Windows, in back branches only.
Tom Lane [Tue, 25 Jan 2022 17:17:40 +0000 (12:17 -0500)]
Revert "graceful shutdown" changes for Windows, in back branches only.

This reverts commits 6051857fc and ed52c3707, but only in the back
branches.  Further testing has shown that while those changes do fix
some things, they also break others; in particular, it looks like
walreceivers fail to detect walsender-initiated connection close
reliably if the walsender shuts down this way.  We'll keep trying to
improve matters in HEAD, but it now seems unwise to push these changes
into stable releases.

Discussion: https://postgr.es/m/CA+hUKG+OeoETZQ=Qw5Ub5h3tmwQhBmDA=nuNO3KG=zWfUypFAw@mail.gmail.com

3 years agoConsider parallel awareness when removing single-child Appends
David Rowley [Tue, 25 Jan 2022 08:15:00 +0000 (21:15 +1300)]
Consider parallel awareness when removing single-child Appends

8edd0e794 added some code to remove Append and MergeAppend nodes when they
contained a single child node.  As it turned out, this was unsafe to do
when the Append/MergeAppend was parallel_aware and the child node was not.
Removing the Append/MergeAppend, in this case, could lead to the child plan
being called multiple times by parallel workers when it was unsafe to do
so.

Here we fix this by just not removing the Append/MergeAppend when the
parallel_aware flag of the parent and child node don't match.

Reported-by: Yura Sokolov
Bug: #17335
Discussion: https://postgr.es/m/b59605fecb20ba9ea94e70ab60098c237c870628.camel%40postgrespro.ru
Backpatch-through: 12, where 8edd0e794 was first introduced

3 years agodoc: Fix some grammar
Michael Paquier [Tue, 25 Jan 2022 01:49:41 +0000 (10:49 +0900)]
doc: Fix some grammar

This is an extraction of the user-visible changes done in 410aa24,
including all the relevant documentation parts.

Author: Justin Pryzby
Discussion: https://postgr.es/m/20220124030001[email protected]
Backpatch-through: 10

3 years agoFix limitations on what SQL commands can be issued to a walsender.
Tom Lane [Mon, 24 Jan 2022 20:33:34 +0000 (15:33 -0500)]
Fix limitations on what SQL commands can be issued to a walsender.

In logical replication mode, a WalSender is supposed to be able
to execute any regular SQL command, as well as the special
replication commands.  Poor design of the replication-command
parser caused it to fail in various cases, notably:

* semicolons embedded in a command, or multiple SQL commands
sent in a single message;

* dollar-quoted literals containing odd numbers of single
or double quote marks;

* commands starting with a comment.

The basic problem here is that we're trying to run repl_scanner.l
across the entire input string even when it's not a replication
command.  Since repl_scanner.l does not understand all of the
token types known to the core lexer, this is doomed to have
failure modes.

We certainly don't want to make repl_scanner.l as big as scan.l,
so instead rejigger stuff so that we only lex the first token of
a non-replication command.  That will usually look like an IDENT
to repl_scanner.l, though a comment would end up getting reported
as a '-' or '/' single-character token.  If the token is a replication
command keyword, we push it back and proceed normally with repl_gram.y
parsing.  Otherwise, we can drop out of exec_replication_command()
without examining the rest of the string.

(It's still theoretically possible for repl_scanner.l to fail on
the first token; but that could only happen if it's an unterminated
single- or double-quoted string, in which case you'd have gotten
largely the same error from the core lexer too.)

In this way, repl_gram.y isn't involved at all in handling general
SQL commands, so we can get rid of the SQLCmd node type.  (In
the back branches, we can't remove it because renumbering enum
NodeTag would be an ABI break; so just leave it sit there unused.)

I failed to resist the temptation to clean up some other sloppy
coding in repl_scanner.l while at it.  The only externally-visible
behavior change from that is it now accepts \r and \f as whitespace,
same as the core lexer.

Per bug #17379 from Greg Rychlewski.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/17379-6a5c6cfb3f1f5e77@postgresql.org

3 years agoRemember to reset yy_start state when firing up repl_scanner.l.
Tom Lane [Mon, 24 Jan 2022 17:09:46 +0000 (12:09 -0500)]
Remember to reset yy_start state when firing up repl_scanner.l.

Without this, we get odd behavior when the previous cycle of
lexing exited in a non-default exclusive state.  Every other
copy of this code is aware that it has to do BEGIN(INITIAL),
but repl_scanner.l did not get that memo.

The real-world impact of this is probably limited, since most
replication clients would abandon their connection after getting
a syntax error.  Still, it's a bug.

This mistake is old, so back-patch to all supported branches.

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

3 years agoSuppress variable-set-but-not-used warning from clang 13.
Tom Lane [Sun, 23 Jan 2022 16:09:00 +0000 (11:09 -0500)]
Suppress variable-set-but-not-used warning from clang 13.

In the normal configuration where GEQO_DEBUG isn't defined,
recent clang versions have started to complain that geqo_main.c
accumulates the edge_failures count but never does anything
with it.  As a minimal back-patchable fix, insert a void cast
to silence this warning.  (I'd speculated about ripping out the
GEQO_DEBUG logic altogether, but I don't think we'd wish to
back-patch that.)

Per recently-established project policy, this is a candidate
for back-patching into out-of-support branches: it suppresses
an annoying compiler warning but changes no behavior.  Hence,
back-patch all the way to 9.2.

Discussion: https://postgr.es/m/CA+hUKGLTSZQwES8VNPmWO9AO0wSeLt36OCPDAZTccT1h7Q7kTQ@mail.gmail.com

3 years agoCorrect type of front_pathkey to PathKey
Tomas Vondra [Sun, 23 Jan 2022 02:36:55 +0000 (03:36 +0100)]
Correct type of front_pathkey to PathKey

In sort_inner_and_outer we iterate a list of PathKey elements, but the
variable is declared as (List *). This mistake is benign, because we
only pass the pointer to lcons() and never dereference it.

This exists since ~2004, but it's confusing. So fix and backpatch to all
supported branches.

Backpatch-through: 10
Discussion: https://postgr.es/m/bf3a6ea1-a7d8-7211-0669-189d5c169374%40enterprisedb.com

3 years agoCheck syscache result in AlterStatistics
Tomas Vondra [Sun, 23 Jan 2022 01:49:41 +0000 (02:49 +0100)]
Check syscache result in AlterStatistics

The syscache lookup may return NULL even for valid OID, for example due
to a concurrent DROP STATISTICS, so a HeapTupleIsValid is necessary.
Without it, it may fail with a segfault.

Reported by Alexander Lakhin, patch by me. Backpatch to 13, where ALTER
STATISTICS ... SET STATISTICS was introduced.

Backpatch-through: 13
Discussion: https://postgr.es/m/17372-bf3b6e947e35ae77%40postgresql.org

3 years agoFlush table's relcache during ALTER TABLE ADD PRIMARY KEY USING INDEX.
Tom Lane [Sat, 22 Jan 2022 18:32:40 +0000 (13:32 -0500)]
Flush table's relcache during ALTER TABLE ADD PRIMARY KEY USING INDEX.

Previously, unless we had to add a NOT NULL constraint to the column,
this command resulted in updating only the index's relcache entry.
That's problematic when replication behavior is being driven off the
existence of a primary key: other sessions (and ours too for that
matter) failed to recalculate their opinion of whether the table can
be replicated.  Add a relcache invalidation to fix it.

This has been broken since pg_class.relhaspkey was removed in v11.
Before that, updating the table's relhaspkey value sufficed to cause
a cache flush.  Hence, backpatch to v11.

Report and patch by Hou Zhijie

Discussion: https://postgr.es/m/OS0PR01MB5716EBE01F112C62F8F9B786947B9@OS0PR01MB5716.jpnprd01.prod.outlook.com

3 years agoFix race condition in gettext() initialization in libpq and ecpglib.
Tom Lane [Fri, 21 Jan 2022 20:36:12 +0000 (15:36 -0500)]
Fix race condition in gettext() initialization in libpq and ecpglib.

In libpq and ecpglib, multiple threads can concurrently enter the
initialization logic for message localization.  Since we set the
its-done flag before actually doing the work, it'd be possible
for some threads to reach gettext() before anyone has called
bindtextdomain().  Barring bugs in libintl itself, this would not
result in anything worse than failure to localize some early
messages.  Nonetheless, it's a bug, and an easy one to fix.

Noted while investigating bug #17299 from Clemens Zeidler
(much thanks to Liam Bowen for followup investigation on that).
It currently appears that that actually *is* a bug in libintl itself,
but that doesn't let us off the hook for this bit.

Back-patch to all supported versions.

Discussion: https://postgr.es/m/17299-7270741958c0b1ab@postgresql.org
Discussion: https://postgr.es/m/CAE7q7Eit4Eq2=bxce=Fm8HAStECjaXUE=WBQc-sDDcgJQ7s7eg@mail.gmail.com

3 years agofsync pg_logical/mappings in CheckPointLogicalRewriteHeap().
Andres Freund [Fri, 21 Jan 2022 19:22:55 +0000 (11:22 -0800)]
fsync pg_logical/mappings in CheckPointLogicalRewriteHeap().

While individual logical rewrite files were synced to disk, the directory was
not. On some filesystems that could lead to loosing directory entries after a
crash.

Reported-By: Tom Lane
Author: Nathan Bossart 
Discussion: https://postgr.es/m/867F2E29-2782-4869-970E-B984C6D35A8F@amazon.com
Backpatch: 10-

3 years agoFix one-off bug causing missing commit timestamps for subtransactions
Michael Paquier [Fri, 21 Jan 2022 05:54:51 +0000 (14:54 +0900)]
Fix one-off bug causing missing commit timestamps for subtransactions

The logic in charge of writing commit timestamps (enabled with
track_commit_timestamp) for subtransactions had a one-bug bug,
where it would be possible that commit timestamps go missing for the
last subtransaction committed.

While on it, simplify a bit the iteration logic in the loop writing the
commit timestamps, as per suggestions from Kyotaro Horiguchi and Tom
Lane, so as some variable initializations are not part of the loop
itself.

Issue introduced in 73c986a.

Analyzed-by: Alex Kingsborough
Author: Alex Kingsborough, Kyotaro Horiguchi
Discussion: https://postgr.es/m/73A66172-4050-4F2A-B7F1-13508EDA2144@amazon.com
Backpatch-through: 10

3 years agoTighten TAP tests' tracking of postmaster state some more.
Tom Lane [Thu, 20 Jan 2022 22:28:07 +0000 (17:28 -0500)]
Tighten TAP tests' tracking of postmaster state some more.

Commits 6c4a8903b et al. had a couple of deficiencies:

* The logic I added to Cluster::start to see if a PID file is present
could be fooled by a stale PID file left over from a previous
postmaster.  To fix, if we're not sure whether we expect to find a
running postmaster or not, validate the PID using "kill 0".

* 017_shm.pl has a loop in which it just issues repeated Cluster::start
calls; this will fail if some invocation fails but leaves self->_pid
set.  Per buildfarm results, the above fix is not enough to make this
safe: we might have "validated" a PID for a postmaster that exits
immediately after we look.  Hence, match each failed start call with
a stop call that will get us back to the self->_pid == undef state.
Add a fail_ok option to Cluster::stop to make this work.

Discussion: https://postgr.es/m/CA+hUKGKV6fOHvfiPt8=dOKzvswjAyLoFoJF1iQXMNpi7+hD1JQ@mail.gmail.com

3 years agoAllow clean.bat to be run from anywhere
Andrew Dunstan [Thu, 20 Jan 2022 15:13:18 +0000 (10:13 -0500)]
Allow clean.bat to be run from anywhere

This was omitted from c3879a7b4c which modified the other msvc .bat
files.

Per request from Juan José Santamaría Flecha

Discussion: https://postgr.es/m/CAC+AXB0_fxYGbQoaYjCA8um7TTbOVP4L9aXnVmHwK8WzaT4gdA@mail.gmail.com

Backpatch to all live branches.

3 years agoTry to stabilize reloptions test, again.
Thomas Munro [Thu, 20 Jan 2022 09:11:48 +0000 (22:11 +1300)]
Try to stabilize reloptions test, again.

Since the test requires reproducible behavior from VACUUM, and since
DISABLE_PAGE_SKIPPING doesn't actually disable all forms of page
skipping, let's use a temporary table to avoid contention.

Back-patch to 12, like commit 3414099c.

Discussion: https://postgr.es/m/20220120052404.sonrhq3f3qgplpzj%40alap3.anarazel.de

3 years agodoc: Mention the level of locks taken on objects in COMMENT
Michael Paquier [Thu, 20 Jan 2022 07:54:58 +0000 (16:54 +0900)]
doc: Mention the level of locks taken on objects in COMMENT

This information was nowhere to be found.  This adds one note on the
page of COMMENT, and one note in the section dedicated to explicit
locking, both telling that a SHARE UPDATE EXCLUSIVE lock is taken on the
object commented.

Author: Nikolai Berkoff
Reviewed-by: Laurenz Albe
Discussion: https://postgr.es/m/_0HDHIGcCdCsUyXn22QwI2FEuNR6Fs71rtgGX6hfyBlUh5rrnE2qMmvIFu9EY4Pijr2gUmJEAXCjuNU2Oxku9TryLp9CdHllpsCfN3gD0-Y=@pm.me
Backpatch-through: 10

3 years agoTAP tests: check for postmaster.pid anyway when "pg_ctl start" fails.
Tom Lane [Wed, 19 Jan 2022 21:29:09 +0000 (16:29 -0500)]
TAP tests: check for postpid anyway when "pg_ctl start" fails.

"pg_ctl start" might start a new postmaster and then return failure
anyway, for example if PGCTLTIMEOUT is exceeded.  If there is a
postmaster there, it's still incumbent on us to shut it down at
script end, so check for the PID file even though we are about
to fail.

This has been broken all along, so back-patch to all supported branches.

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

3 years agodoc: Fix description of pg_replication_origin_oid() in error case
Michael Paquier [Wed, 19 Jan 2022 01:37:53 +0000 (10:37 +0900)]
doc: Fix description of pg_replication_origin_oid() in error case

This function returns NULL if the replication origin given in input
argument does not exist, contrary to what the docs described
previously.

Author: Ian Barwick
Discussion: https://postgr.es/m/CAB8KJ=htJjBL=103URqjOxV2mqb4rjphDpMeKdyKq_QXt6h05w@mail.gmail.com
Backpatch-through: 10

3 years agoTry to stabilize the reloptions test.
Thomas Munro [Tue, 18 Jan 2022 18:03:01 +0000 (07:03 +1300)]
Try to stabilize the reloptions test.

Where we test vacuum_truncate's effects, sometimes this is failing to
truncate as expected on the build farm.  That could be explained by page
skipping, so disable it explicitly, with the theory that commit fe246d1c
didn't go far enough.

Back-patch to 12, where the vacuum_truncate tests were added.

Discussion: https://postgr.es/m/CA%2BhUKGLT2UL5_JhmBzUgkdyKfc%3D5J-gJSQJLysMs4rqLUKLAzw%40mail.gmail.com

3 years agoFix psql \d's query for identifying parent triggers.
Tom Lane [Tue, 18 Jan 2022 02:18:49 +0000 (21:18 -0500)]
Fix psql \d's query for identifying parent triggers.

The original coding (from c33869cc3) failed with "more than one row
returned by a subquery used as an expression" if there were unrelated
triggers of the same tgname on parent partitioned tables.  (That's
possible because statement-level triggers don't get inherited.)  Fix
by applying LIMIT 1 after sorting the candidates by inheritance level.

Also, wrap the subquery in a CASE so that we don't have to execute it at
all when the trigger is visibly non-inherited.  Aside from saving some
cycles, this avoids the need for a confusing and undocumented NULLIF().

While here, tweak the format of the emitted query to look a bit
nicer for "psql -E", and add some explanation of this subquery,
because it badly needs it.

Report and patch by Justin Pryzby (with some editing by me).
Back-patch to v13 where the faulty code came in.

Discussion: https://postgr.es/m/20211217154356[email protected]

3 years agoAvoid calling gettext() in signal handlers.
Tom Lane [Mon, 17 Jan 2022 18:30:04 +0000 (13:30 -0500)]
Avoid calling gettext() in signal handlers.

It seems highly unlikely that gettext() can be relied on to be
async-signal-safe.  psql used to understand that, but someone got
it wrong long ago in the src/bin/scripts/ version of handle_sigint,
and then the bad idea was perpetuated when those two versions were
unified into src/fe_utils/cancel.c.

I'm unsure why there have not been field complaints about this
... maybe gettext() is signal-safe once it's translated at least
one message?  But we have no business assuming any such thing.

In cancel.c (v13 and up), I preserved our ability to localize
"Cancel request sent" messages by invoking gettext() before
the signal handler is set up.  In earlier branches I just made
src/bin/scripts/ not localize those messages, as psql did then.

(Just for extra unsafety, the src/bin/scripts/ version was
invoking fprintf() from a signal handler.  Sigh.)

Noted while fixing signal-safety issues in PQcancel() itself.
Back-patch to all supported branches.

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

3 years agoAvoid calling strerror[_r] in PQcancel().
Tom Lane [Mon, 17 Jan 2022 17:52:44 +0000 (12:52 -0500)]
Avoid calling strerror[_r] in PQcancel().

PQcancel() is supposed to be safe to call from a signal handler,
and indeed psql uses it that way.  All of the library functions
it uses are specified to be async-signal-safe by POSIX ...
except for strerror.  Neither plain strerror nor strerror_r
are considered safe.  When this code was written, back in the
dark ages, we probably figured "oh, strerror will just index
into a constant array of strings" ... but in any locale except C,
that's unlikely to be true.  Probably the reason we've not heard
complaints is that (a) this error-handling code is unlikely to be
reached in normal use, and (b) in many scenarios, localized error
strings would already have been loaded, after which maybe it's
safe to call strerror here.  Still, this is clearly unacceptable.

The best we can do without relying on strerror is to print the
decimal value of errno, so make it do that instead.  (This is
probably not much loss of user-friendliness, given that it is
hard to get a failure here.)

Back-patch to all supported branches.

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

3 years agoFix psql's tab-completion of enum label values.
Tom Lane [Sun, 16 Jan 2022 19:59:20 +0000 (14:59 -0500)]
Fix psql's tab-completion of enum label values.

Since enum labels have to be single-quoted, this part of the
tab completion machinery got side-swiped by commit cd69ec66c.
A side-effect of that commit is that (at least with some versions
of Readline) the text string passed for completion will omit the
leading quote mark of the enum label literal.  Libedit still acts
the same as before, though, so adapt COMPLETE_WITH_ENUM_VALUE so
that it can cope with either convention.

Also, when we fail to find any valid completion, set
rl_completion_suppress_quote = 1.  Otherwise readline will
go ahead and append a closing quote, which is unwanted.

Per report from Peter Eisentraut.  Back-patch to v13 where
cd69ec66c came in.

Discussion: https://postgr.es/m/8ca82d89-ec3d-8b28-8291-500efaf23b25@enterprisedb.com

3 years agoBuild inherited extended stats on partitioned tables
Tomas Vondra [Sat, 15 Jan 2022 17:17:20 +0000 (18:17 +0100)]
Build inherited extended stats on partitioned tables

Commit 859b3003de disabled building of extended stats for inheritance
trees, to prevent updating the same catalog row twice. While that
resolved the issue, it also means there are no extended stats for
declaratively partitioned tables, because there are no data in the
non-leaf relations.

That also means declaratively partitioned tables were not affected by
the issue 859b3003de addressed, which means this is a regression
affecting queries that calculate estimates for the whole inheritance
tree as a whole (which includes e.g. GROUP BY queries).

But because partitioned tables are empty, we can invert the condition
and build statistics only for the case with inheritance, without losing
anything. And we can consider them when calculating estimates.

It may be necessary to run ANALYZE on partitioned tables, to collect
proper statistics. For declarative partitioning there should no prior
statistics, and it might take time before autoanalyze is triggered. For
tables partitioned by inheritance the statistics may include data from
child relations (if built 859b3003de), contradicting the current code.

Report and patch by Justin Pryzby, minor fixes and cleanup by me.
Backpatch all the way back to PostgreSQL 10, where extended statistics
were introduced (same as 859b3003de).

Author: Justin Pryzby
Reported-by: Justin Pryzby
Backpatch-through: 10
Discussion: https://postgr.es/m/20210923212624.GI831%40telsasoft.com

3 years agoIgnore extended statistics for inheritance trees
Tomas Vondra [Sat, 15 Jan 2022 01:15:23 +0000 (02:15 +0100)]
Ignore extended statistics for inheritance trees

Since commit 859b3003de we only build extended statistics for individual
relations, ignoring the child relations. This resolved the issue with
updating catalog tuple twice, but we still tried to use the statistics
when calculating estimates for the whole inheritance tree. When the
relations contain very distinct data, it may produce bogus estimates.

This is roughly the same issue 427c6b5b9 addressed ~15 years ago, and we
fix it the same way - by ignoring extended statistics when calculating
estimates for the inheritance tree as a whole. We still consider
extended statistics when calculating estimates for individual child
relations, of course.

This may result in plan changes due to different estimates, but if the
old statistics were not describing the inheritance tree particularly
well it's quite likely the new plans is actually better.

Report and patch by Justin Pryzby, minor fixes and cleanup by me.
Backpatch all the way back to PostgreSQL 10, where extended statistics
were introduced (same as 859b3003de).

Author: Justin Pryzby
Reported-by: Justin Pryzby
Backpatch-through: 10
Discussion: https://postgr.es/m/20210923212624.GI831%40telsasoft.com

3 years agoFix ruleutils.c's dumping of whole-row Vars in more contexts.
Tom Lane [Thu, 13 Jan 2022 22:49:26 +0000 (17:49 -0500)]
Fix ruleutils.c's dumping of whole-row Vars in more contexts.

Commit 7745bc352 intended to ensure that whole-row Vars would be
printed with "::type" decoration in all contexts where plain
"var.*" notation would result in star-expansion, notably in
ROW() and VALUES() constructs.  However, it missed the case of
INSERT with a single-row VALUES, as reported by Timur Khanjanov.

Nosing around ruleutils.c, I found a second oversight: the
code for RowCompareExpr generates ROW() notation without benefit
of an actual RowExpr, and naturally it wasn't in sync :-(.
(The code for FieldStore also does this, but we don't expect that
to generate strictly parsable SQL anyway, so I left it alone.)

Back-patch to all supported branches.

Discussion: https://postgr.es/m/efaba6f9-4190-56be-8ff2-7a1674f9194f@intrans.baku.az

3 years agoDoc: fix bogus example about ambiguous timestamps.
Tom Lane [Mon, 10 Jan 2022 16:46:16 +0000 (11:46 -0500)]
Doc: fix bogus example about ambiguous timestamps.

I had a brain fade in commit d32899157, and used 2:30AM as the
example timestamp for both spring-forward and fall-back cases.
But it's not actually ambiguous at all in the fall-back case,
because that transition is from 2AM to 1AM under USA rules.
Fix the example to use 1:30AM, which *is* ambiguous.

Noted while answering a question from Aleksander Alekseev.
Back-patch to all supported branches.

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

3 years agoAvoid warning about uninitialized value in MSVC python3 tests
Andrew Dunstan [Mon, 10 Jan 2022 15:08:44 +0000 (10:08 -0500)]
Avoid warning about uninitialized value in MSVC python3 tests

Juan José Santamaría Flecha

Backpatch to all live branches

3 years agoFix results of index-only scans on btree_gist char(N) indexes.
Tom Lane [Sat, 8 Jan 2022 19:54:39 +0000 (14:54 -0500)]
Fix results of index-only scans on btree_gist char(N) indexes.

If contrib/btree_gist is used to make a GIST index on a char(N)
(bpchar) column, and that column is retrieved via an index-only
scan, what came out had all trailing spaces removed.  Since
that doesn't happen in any other kind of table scan, this is
clearly a bug.  The cause is that gbt_bpchar_compress() strips
trailing spaces (using rtrim1) before a new index entry is made.
That was probably a good idea when this code was first written,
but since we invented index-only scans, it's not so good.

One answer could be to mark this opclass as incapable of index-only
scans.  But to do so, we'd need an extension module version bump,
followed by manual action by DBAs to install the updated version
of btree_gist.  And it's not really a desirable place to end up,
anyway.

Instead, let's fix the code by removing the unwanted space-stripping
action and adjusting the opclass's comparison logic to ignore
trailing spaces as bpchar normally does.  This will not hinder
cases that work today, since index searches with this logic will
act the same whether trailing spaces are stored or not.  It will
not by itself fix the problem of getting space-stripped results
from index-only scans, of course.  Users who care about that can
REINDEX affected indexes after installing this update, to immediately
replace all improperly-truncated index entries.  Otherwise, it can
be expected that the index's behavior will change incrementally as
old entries are replaced by new ones.

Per report from Alexander Lakhin.  Back-patch to all supported branches.

Discussion: https://postgr.es/m/696c995b-b37f-5526-f45d-04abe713179f@gmail.com

3 years agoUpdate copyright for 2022
Bruce Momjian [Sat, 8 Jan 2022 00:04:56 +0000 (19:04 -0500)]
Update copyright for 2022

Backpatch-through: 10

3 years agoAllow MSVC .bat wrappers to be called from anywhere
Andrew Dunstan [Fri, 7 Jan 2022 21:07:45 +0000 (16:07 -0500)]
Allow MSVC .bat wrappers to be called from anywhere

Instead of using a hardcoded or default path to the perl file the .bat
file is a wrapper for, we use a path that means the file is found in
the same directory as the .bat file.

Patch by Anton Voloshin, slightly tweaked by me.

Backpatch to all live branches

Discussion: https://postgr.es/m/2b7a674b-5fb0-d264-75ef-ecc7a31e54f8@postgrespro.ru

3 years agoPrevent altering partitioned table's rowtype, if it's used elsewhere.
Tom Lane [Thu, 6 Jan 2022 21:46:46 +0000 (16:46 -0500)]
Prevent altering partitioned table's rowtype, if it's used elsewhere.

We disallow altering a column datatype within a regular table,
if the table's rowtype is used as a column type elsewhere,
because we lack code to go around and rewrite the other tables.
This restriction should apply to partitioned tables as well, but it
was not checked because ATRewriteTables and ATPrepAlterColumnType
were not on the same page about who should do it for which relkinds.

Per bug #17351 from Alexander Lakhin.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/17351-6db1870f3f4f612a@postgresql.org

3 years agodoc: Remove link to JSON support in the SQL specification
Michael Paquier [Thu, 6 Jan 2022 02:41:44 +0000 (11:41 +0900)]
doc: Remove link to JSON support in the SQL specification

The link used in the documentation is dead, and the only options to have
an access to this part of the SQL specification are not free.  Like any
other books referred, just remove the link to keep some neutrality but
keep its reference.

Reported-by: Erik Rijkers
Discussion: https://postgr.es/m/989abd7d-af30-ab52-1201-bf0b4f33b872@xs4all.nl
Backpatch-through: 12

3 years agoReduce relcache access in WAL sender streaming logical changes
Michael Paquier [Wed, 5 Jan 2022 01:27:53 +0000 (10:27 +0900)]
Reduce relcache access in WAL sender streaming logical changes

get_rel_sync_entry(), which is called each time a change needs to be
logically replicated, is a rather hot code path in the WAL sender
sending logical changes.  This code path was doing a relcache access on
relkind and relpartition for each logical change, but we only need to
know this information when building or re-building the cached
information for a relation.

Some measurements prove that this is noticeable in perf profiles,
particularly when attempting to replicate changes from relations that
are not published as these cause less overhead in the WAL sender,
delaying further the replication of changes for relations that are
published.

Issue introduced in 83fd453.

Author: Hou Zhijie
Reviewed-by: Kyotaro Horiguchi, Euler Taveira
Discussion: https://postgr.es/m/OS0PR01MB5716E863AA9E591C1F010F7A947D9@OS0PR01MB5716.jpnprd01.prod.outlook.com
Backpatch-through: 13

3 years agoFix silly mistake in Assert
Alvaro Herrera [Tue, 4 Jan 2022 16:21:23 +0000 (13:21 -0300)]
Fix silly mistake in Assert

3 years agoAllow special SKIP LOCKED condition in Assert()
Alvaro Herrera [Tue, 4 Jan 2022 16:01:05 +0000 (13:01 -0300)]
Allow special SKIP LOCKED condition in Assert()

Under concurrency, it is possible for two sessions to be merrily locking
and releasing a tuple and marking it again as HEAP_XMAX_INVALID all the
while a third session attempts to lock it, miserably fails at it, and
then contemplates life, the universe and everything only to eventually
fail an assertion that said bit is not set.  Before SKIP LOCKED that was
indeed a reasonable expectation, but alas! commit df630b0dd5ea falsified
it.

This bug is as old as time itself, and even older, if you think time
begins with the oldest supported branch.  Therefore, backpatch to all
supported branches.

Author: Simon Riggs 
Discussion: https://postgr.es/m/CANbhV-FeEwMnN8yuMyss7if1ZKjOKfjcgqB26n8pqu1e=q0ebg@mail.gmail.com

3 years agoFix index-only scan plans, take 2.
Tom Lane [Mon, 3 Jan 2022 20:42:27 +0000 (15:42 -0500)]
Fix index-only scan plans, take 2.

Commit 4ace45677 failed to fix the problem fully, because the
same issue of attempting to fetch a non-returnable index column
can occur when rechecking the indexqual after using a lossy index
operator.  Moreover, it broke EXPLAIN for such indexquals (which
indicates a gap in our test cases :-().

Revert the code changes of 4ace45677 in favor of adding a new field
to struct IndexOnlyScan, containing a version of the indexqual that
can be executed against the index-returned tuple without using any
non-returnable columns.  (The restrictions imposed by check_index_only
guarantee this is possible, although we may have to recompute indexed
expressions.)  Support construction of that during setrefs.c
processing by marking IndexOnlyScan.indextlist entries as resjunk
if they can't be returned, rather than removing them entirely.
(We could alternatively require setrefs.c to look up the IndexOptInfo
again, but abusing resjunk this way seems like a reasonably safe way
to avoid needing to do that.)

This solution isn't great from an API-stability standpoint: if there
are any extensions out there that build IndexOnlyScan structs directly,
they'll be broken in the next minor releases.  However, only a very
invasive extension would be likely to do such a thing.  There's no
change in the Path representation, so typical planner extensions
shouldn't have a problem.

As before, back-patch to all supported branches.

Discussion: https://postgr.es/m/3179992.1641150853@sss.pgh.pa.us
Discussion: https://postgr.es/m/17350-b5bdcf476e5badbb@postgresql.org

3 years agoFix typo
Magnus Hagander [Sun, 2 Jan 2022 16:03:32 +0000 (17:03 +0100)]
Fix typo

Reported-By: Eric Mutta
Backpatch-through: 10
Discussion: https://postgr.es/m/164052477973.21665.7888120874624887609@wrigleys.postgresql.org

3 years agoFix index-only scan plans when not all index columns can be returned.
Tom Lane [Sat, 1 Jan 2022 21:12:03 +0000 (16:12 -0500)]
Fix index-only scan plans when not all index columns can be returned.

If an index has both returnable and non-returnable columns, and one of
the non-returnable columns is an expression using a Var that is in a
returnable column, then a query returning that expression could result
in an index-only scan plan that attempts to read the non-returnable
column, instead of recomputing the expression from the returnable
column as intended.

To fix, redefine the "indextlist" list of an IndexOnlyScan plan node
as containing null Consts in place of any non-returnable columns.
This solves the problem by preventing setrefs.c from falsely matching
to such entries.  The executor is happy since it only cares about the
exposed types of the entries, and ruleutils.c doesn't care because a
correct plan won't reference those entries.  I considered some other
ways to prevent setrefs.c from doing the wrong thing, but this way
seems good since (a) it allows a very localized fix, (b) it makes
the indextlist structure more compact in many cases, and (c) the
indextlist is now a more faithful representation of what the index AM
will actually produce, viz. nulls for any non-returnable columns.

This is easier to hit since we introduced included columns, but it's
possible to construct failing examples without that, as per the
added regression test.  Hence, back-patch to all supported branches.

Per bug #17350 from Louis Jachiet.

Discussion: https://postgr.es/m/17350-b5bdcf476e5badbb@postgresql.org

3 years agoFix overly generic name in with.sql test.
Thomas Munro [Thu, 30 Dec 2021 03:09:53 +0000 (16:09 +1300)]
Fix overly generic name in with.sql test.

Avoid the name "test".  In the 10 branch, this could clash with
alter_table.sql, as seen in the build farm.  That other instance was
already renamed in later branches by commit 2cf8c7aa, but it's good to
future-proof the name here too.

Back-patch to 10.

Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/CA%2BhUKGJf4RAXUyAYVUcQawcptX%3DnhEco3SYpuPK5cCbA-F1eLA%40mail.gmail.com

3 years agodoc: clarify when expression indexes evaluate their expressions
Bruce Momjian [Wed, 22 Dec 2021 21:29:16 +0000 (16:29 -0500)]
doc:  clarify when expression indexes evaluate their expressions

Only non-HOT updates evaluate the index expression.

Reported-by: Chris Lowder
Discussion: https://postgr.es/m/163967385701.26064.15365003480975321072@wrigleys.postgresql.org

Backpatch-through: 10

3 years agoCorrect comment and some documentation about REPLICA_IDENTITY_INDEX
Michael Paquier [Wed, 22 Dec 2021 07:38:42 +0000 (16:38 +0900)]
Correct comment and some documentation about REPLICA_IDENTITY_INDEX

catalog/pg_class.h was stating that REPLICA_IDENTITY_INDEX with a
dropped index is equivalent to REPLICA_IDENTITY_DEFAULT.  The code tells
a different story, as it is equivalent to REPLICA_IDENTITY_NOTHING.

The behavior exists since the introduction of replica identities, and
fe7fd4e even added tests for this case but I somewhat forgot to fix this
comment.

While on it, this commit reorganizes the documentation about replica
identities on the ALTER TABLE page, and a note is added about the case
of dropped indexes with REPLICA_IDENTITY_INDEX.

Author: Michael Paquier, Wei Wang
Reviewed-by: Euler Taveira
Discussion: https://postgr.es/m/OS3PR01MB6275464AD0A681A0793F56879E759@OS3PR01MB6275.jpnprd01.prod.outlook.com
Backpatch-through: 10

3 years agoEnsure casting to typmod -1 generates a RelabelType.
Tom Lane [Thu, 16 Dec 2021 20:36:02 +0000 (15:36 -0500)]
Ensure casting to typmod -1 generates a RelabelType.

Fix the code changed by commit 5c056b0c2 so that we always generate
RelabelType, not something else, for a cast to unspecified typmod.
Otherwise planner optimizations might not happen.

It appears we missed this point because the previous experiments were
done on type numeric: the parser undesirably generates a call on the
numeric() length-coercion function, but then numeric_support()
optimizes that down to a RelabelType, so that everything seems fine.
It misbehaves for types that have a non-optimized length coercion
function, such as bpchar.

Per report from John Naylor.  Back-patch to all supported branches,
as the previous patch eventually was.  Unfortunately, that no longer
includes 9.6 ... we really shouldn't put this type of change into a
nearly-EOL branch.

Discussion: https://postgr.es/m/CAFBsxsEfbFHEkouc+FSj+3K1sHipLPbEC67L0SAe-9-da8QtYg@mail.gmail.com

3 years agoAdjust behavior of some env settings for the TAP tests of MSVC
Michael Paquier [Wed, 15 Dec 2021 01:40:12 +0000 (10:40 +0900)]
Adjust behavior of some env settings for the TAP tests of MSVC

edc2332 has introduced in vcregress.pl some control on the environment
variables LZ4, TAR and GZIP_PROGRAM to allow any TAP tests to be able
use those commands.  This makes the settings more consistent with
src/Makefile.global.in, as the same default gets used for Make and MSVC
builds.

Each parameter can be changed in buildenv.pl, but as a default gets
assigned after loading buldenv.pl, it is not possible to unset any of
these, and using an empty value would not work with "||=" either.  As
some environments may not have a compatible command in their PATH (tar
coming from MinGW is an issue, for one), this could break tests without
an exit path to bypass any failing test.  This commit changes things so
as the default values for LZ4, TAR and GZIP_PROGRAM are assigned before
loading buildenv.pl, not after.  This way, we keep the same amount of
compatibility as a GNU build with the same defaults, and it becomes
possible to unset any of those values.

While on it, this adds some documentation about those three variables in
the section dedicated to the TAP tests for MSVC.

Per discussion with Andrew Dunstan.

Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 10

3 years agoFix datatype confusion in logtape.c's right_offset().
Tom Lane [Tue, 14 Dec 2021 16:46:36 +0000 (11:46 -0500)]
Fix datatype confusion in logtape.c's right_offset().

This could only matter if (a) long is wider than int, and (b) the heap
of free blocks exceeds UINT_MAX entries, which seems pretty unlikely.
Still, it's a theoretical bug, so backpatch to v13 where the typo came
in (in commit c02fdc922).

In passing, also make swap_nodes() use consistent datatypes.

Ma Liangzhu

Discussion: https://postgr.es/m/17336-fc4e522d26a750fd@postgresql.org

3 years agoRemove assertion for replication origins in PREPARE TRANSACTION
Michael Paquier [Tue, 14 Dec 2021 01:58:29 +0000 (10:58 +0900)]
Remove assertion for replication origins in PREPARE TRANSACTION

When using replication origins, pg_replication_origin_xact_setup() is an
optional choice to be able to set a LSN and a timestamp to mark the
origin, which would be additionally added to WAL for transaction commits
or aborts (including 2PC transactions).  An assertion in the code path
of PREPARE TRANSACTION assumed that this data should always be set, so
it would trigger when using replication origins without setting up an
origin LSN.  Some tests are added to cover more this kind of scenario.

Oversight in commit 1eb6d65.

Per discussion with Amit Kapila and Masahiko Sawada.

Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 11

3 years agoDoc: de-document unimplemented geometric operators.
Tom Lane [Mon, 13 Dec 2021 22:49:36 +0000 (17:49 -0500)]
Doc: de-document unimplemented geometric operators.

In commit 791090bd7, I made an effort to fill in documentation
for all geometric operators listed in pg_operator.  However,
it now appears that at least some of the omissions may have been
intentional, because some of those operator entries point at
unimplemented stub functions.  Remove those from the docs again.

(In HEAD, poly_distance stays, because c5c192d7b just added an
implementation for it.)

Per complaint from Anton Voloshin.

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

3 years agoisolationtester: append session name to application_name.
Andres Freund [Mon, 13 Dec 2021 20:02:45 +0000 (12:02 -0800)]
isolationtester: append session name to application_name.

When writing / debugging an isolation test it sometimes is useful to see which
session holds what lock etc. To make it easier, both as part of spec files and
interactively, append the session name to application_name. Since b1907d688
application_name already contains the test name, this appends the session's
name to that.

insert-conflict-specconflict did something like this manually, which can now
be removed.

As we have done lately with other test infrastructure improvements, backpatch
this change, to make it easier to backpatch tests.

Author: Andres Freund 
Reviewed-By: Michael Paquier
Reviewed-By: Andrew Dunstan
Discussion: https://postgr.es/m/20211211012052[email protected]
Backpatch: 10-, to make backpatching of tests easier.

3 years agoFix double publish of child table's data.
Amit Kapila [Thu, 9 Dec 2021 03:30:35 +0000 (09:00 +0530)]
Fix double publish of child table's data.

We publish the child table's data twice for a publication that has both
child and parent tables and is published with publish_via_partition_root
as true. This happens because subscribers will initiate synchronization
using both parent and child tables, since it gets both as separate tables
in the initial table list.

Ensure that pg_publication_tables returns only parent tables in such
cases.

Author: Hou Zhijie
Reviewed-by: Greg Nancarrow, Amit Langote, Vignesh C, Amit Kapila
Backpatch-through: 13
Discussion: https://postgr.es/m/OS0PR01MB57167F45D481F78CDC5986F794B99@OS0PR01MB5716.jpnprd01.prod.outlook.com

3 years agoDoc: improve xfunc-c-type-table.
Tom Lane [Wed, 8 Dec 2021 21:54:31 +0000 (16:54 -0500)]
Doc: improve xfunc-c-type-table.

List types numeric and timestamptz, which don't seem to have ever been
included here.  Restore bigint, which was no-doubt-accidentally deleted
in v12.  Fix some errors, or at least obsolete usages (nobody declares
float arguments as "float8*" anymore, even though they might be that
under the hood).  Re-alphabetize.  Remove the seeming claim that this
is a complete list of built-in types.

Per question from Oskar Stenberg.

Discussion: https://postgr.es/m/HE1PR03MB2971DE2527ECE1E99D6C19A8F96E9@HE1PR03MB2971.eurprd03.prod.outlook.com

3 years agoFix corruption of toast indexes with REINDEX CONCURRENTLY
Michael Paquier [Wed, 8 Dec 2021 02:01:19 +0000 (11:01 +0900)]
Fix corruption of toast indexes with REINDEX CONCURRENTLY

REINDEX CONCURRENTLY run on a toast index or a toast relation could
corrupt the target indexes rebuilt, as a backend running in parallel
that manipulates toast values would directly release the lock on the
toast relation when its local operation is done, rather than releasing
the lock once the transaction that manipulated the toast values
committed.

The fix done here is simple: we now hold a ROW EXCLUSIVE lock on the
toast relation when saving or deleting a toast value until the
transaction working on them is committed, so as a concurrent reindex
happening in parallel would be able to wait for any activity and see any
new rows inserted (or deleted).

An isolation test is added to check after the case fixed here, which is
a bit fancy by design as it relies on allow_system_table_mods to rename
the toast table and its index to fixed names.  This way, it is possible
to reindex them directly without any dependency on the OID of the
underlying relation.  Note that this could not use a DO block either, as
REINDEX CONCURRENTLY cannot be run in a transaction block.  The test is
backpatched down to 13, where it is possible, thanks to c4a7a39, to use
allow_system_table_mods in a test suite.

Reported-by: Alexey Ermakov
Analyzed-by: Andres Freund, Noah Misch
Author: Michael Paquier
Reviewed-by: Nathan Bossart
Discussion: https://postgr.es/m/17268-d2fb426e0895abd4@postgresql.org
Backpatch-through: 12

3 years agoEnable settings used in TAP tests for MSVC builds
Andrew Dunstan [Tue, 7 Dec 2021 20:05:33 +0000 (15:05 -0500)]
Enable settings used in TAP tests for MSVC builds

Certain settings from configuration or the Makefile infrastructure are
used by the TAP tests, but were not being set up by vcregress.pl. This
remedies those omissions. This should increase test coverage, especially
on the buildfarm.

Reviewed by Noah Misch

Discussion: https://postgr.es/m/17093da5-e40d-8335-d53a-2bd803fc38b0@dunslane.net

Backpatch to all live branches.

3 years agoOn Windows, also call shutdown() while closing the client socket.
Tom Lane [Tue, 7 Dec 2021 18:34:06 +0000 (13:34 -0500)]
On Windows, also call shutdown() while closing the client socket.

Further experimentation shows that commit 6051857fc is not sufficient
when using (some versions of?) OpenSSL.  The reason is obscure, but
calling shutdown(socket, SD_SEND) improves matters.

Per testing by Andrew Dunstan and Alexander Lakhin.
Back-patch as before.

Discussion: https://postgr.es/m/af5e0bf3-6a61-bb97-6cba-061ddf22ff6b@dunslane.net

3 years agoDoc: Fix misleading wording of CRL parameters
Daniel Gustafsson [Fri, 3 Dec 2021 13:15:50 +0000 (14:15 +0100)]
Doc: Fix misleading wording of CRL parameters

ssl_crl_file and ssl_crl_dir are both used to for client certificate
revocation, not server certificates.  The description for the params
could be easily misread to mean the opposite however,  as evidenced
by the bugreport leading to this fix.  Similarly, expand sslcrl and
and sslcrldir to explicitly mention server certificates. While there
also mention sslcrldir where previously only sslcrl was discussed.

Backpatch down to v10, with the CRL dir fixes down to 14 where they
were introduced.

Author: Kyotaro Horiguchi 
Reviewed-by: Peter Eisentraut
Discussion: https://postgr.es/m/20211202.135441.590555657708629486[email protected]
Discussion: https://postgr.es/m/CABWY_HCBUCjY1EJHrEGePGEaSZ5b29apgTohCyygtsqe_ySYng@mail.gmail.com
Backpatch-through: 10

3 years agopostgres_fdw: Fix unexpected reporting of empty message.
Fujii Masao [Fri, 3 Dec 2021 08:35:29 +0000 (17:35 +0900)]
postgres_fdw: Fix unexpected reporting of empty message.

pgfdw_report_error() in postgres_fdw gets a message from PGresult or
PGconn to report an error received from a remote server. Previously
if it could get a message from neither of them, it reported empty
message unexpectedly. The cause of this issue was that pgfdw_report_error()
didn't handle properly the case where no message could be obtained
and its local variable message_primary was set to '\0'.

This commit improves pgfdw_report_error() so that it reports the message
"could not obtain ..." when it gets no message and message_primary
is set to '\0'. This is the same behavior as when message_primary is NULL.

dblink_res_error() in dblink has the same issue, so this commit also
improves it in the same way.

Back-patch to all supported branches.

Author: Fujii Masao
Reviewed-by: Bharath Rupireddy
Discussion: https://postgr.es/m/477c16c8-7ea4-20fc-38d5-ed3a77ed616c@oss.nttdata.com

3 years agoOn Windows, close the client socket explicitly during backend shutdown.
Tom Lane [Thu, 2 Dec 2021 22:14:43 +0000 (17:14 -0500)]
On Windows, close the client socket explicitly during backend shutdown.

It turns out that this is necessary to keep Winsock from dropping any
not-yet-sent data, such as an error message explaining the reason for
process termination.  It's pretty weird that the implicit close done
by the kernel acts differently from an explicit close, but it's hard
to argue with experimental results.

Independently submitted by Alexander Lakhin and Lars Kanis (comments
by me, though).  Back-patch to all supported branches.

Discussion: https://postgr.es/m/90b34057-4176-7bb0-0dbb-9822a5f6425b@greiz-reinsdorf.de
Discussion: https://postgr.es/m/16678-253e48d34dc0c376@postgresql.org

3 years agoMove into separate file all the SQL queries used in pg_upgrade tests
Michael Paquier [Thu, 2 Dec 2021 01:31:34 +0000 (10:31 +0900)]
Move into separate file all the SQL queries used in pg_upgrade tests

The existing pg_upgrade/test.sh and the buildfarm code have been holding
the same set of SQL queries when doing cross-version upgrade tests to
adapt the objects created by the regression tests before the upgrade
(mostly, incompatible or non-existing objects need to be dropped from
the origin, perhaps re-created).

This moves all those SQL queries into a new, separate, file with a set
of \if clauses to handle the version checks depending on the old version
of the cluster to-be-upgraded.

The long-term plan is to make the buildfarm code re-use this new SQL
file, so as committers are able to fix any compatibility issues in the
tests of pg_upgrade with a refresh of the core code, without having to
poke at the buildfarm client.  Note that this is only able to handle the
main regression test suite, and that nothing is done yet for contrib
modules yet (these have more issues like their database names).

A backpatch down to 10 is done, adapting the version checks as this
script needs to be only backward-compatible, so as it becomes possible
to clean up a maximum amount of code within the buildfarm client.

Author: Justin Pryzby, Michael Paquier
Discussion: https://postgr.es/m/20201206180248[email protected]
Backpatch-through: 10

3 years agoAvoid leaking memory during large-scale REASSIGN OWNED BY operations.
Tom Lane [Wed, 1 Dec 2021 18:44:47 +0000 (13:44 -0500)]
Avoid leaking memory during large-scale REASSIGN OWNED BY operations.

The various ALTER OWNER routines tend to leak memory in
CurrentMemoryContext.  That's not a problem when they're only called
once per command; but in this usage where we might be touching many
objects, it can amount to a serious memory leak.  Fix that by running
each call in a short-lived context.

(DROP OWNED BY likely has a similar issue, except that you'll probably
run out of lock table space before noticing.  REASSIGN is worth fixing
since for most non-table object types, it won't take any lock.)

Back-patch to all supported branches.  Unfortunately, in the back
branches this helps to only a limited extent, since the sinval message
queue bloats quite a lot in this usage before commit 3aafc030a,
consuming memory more or less comparable to what's actually leaked.
Still, it's clearly a leak with a simple fix, so we might as well fix it.

Justin Pryzby, per report from Guillaume Lelarge

Discussion: https://postgr.es/m/CAECtzeW2DAoioEGBRjR=CzHP6TdL=yosGku8qZxfX9hhtrBB0Q@mail.gmail.com

3 years agoDoc: Add "Attach Partition" limitation during logical replication.
Amit Kapila [Wed, 1 Dec 2021 04:56:59 +0000 (10:26 +0530)]
Doc: Add "Attach Partition" limitation during logical replication.

ATTACHing a table into a partition tree whose root is published using a
publication with publish_via_partition_root set to true does not result in
the table's existing contents being replicated. This happens because
subscriber doesn't consider replicating the newly attached partition as
the root table is already in a 'ready' state.

This behavior was introduced in PG13 (83fd4532a7) where we allowed to
publish partition changes via ancestors.

We can consider fixing this limitation in the future.

Author: Amit Langote
Reviewed-by: Hou Zhijie, Amit Kapila
Backpatch-through: 13
Discussion: https://postgr.es/m/OS0PR01MB5716E97F00732B52DC2BBC2594989@OS0PR01MB5716.jpnprd01.prod.outlook.com

3 years agoDoc: improve documentation about ORDER BY in matviews.
Tom Lane [Mon, 29 Nov 2021 17:13:13 +0000 (12:13 -0500)]
Doc: improve documentation about ORDER BY in matviews.

Remove the confusing use of ORDER BY in an example materialized
view.  It adds nothing to the example, but might encourage
people to follow bad practice.  Clarify REFRESH MATERIALIZED
VIEW's note about whether view ordering is retained (it isn't).

Maciek Sakrejda

Discussion: https://postgr.es/m/CAOtHd0D-OvrUU0C=4hX28p4BaSE1XL78BAQ0VcDaLLt8tdUzsg@mail.gmail.com