Commit Graph

52186 Commits

Author SHA1 Message Date
Tom Lane f9a8bc9f27 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
2021-12-16 15:36:02 -05:00
Michael Paquier 4ddbd7456a 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/YbGYe483803il3X7@paquier.xyz
Backpatch-through: 10
2021-12-15 10:40:08 +09:00
Tom Lane 25c7faa1fe 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
2021-12-14 11:46:44 -05:00
Michael Paquier 4be3e005e5 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/YbbBfNSvMm5nIINV@paquier.xyz
Backpatch-through: 11
2021-12-14 10:58:25 +09:00
Tom Lane a80fd2ae4c 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
2021-12-13 17:49:36 -05:00
Andres Freund 5b5455b035 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 <andres@anarazel.de>
Reviewed-By: Michael Paquier <michael@paquier.xyz>
Reviewed-By: Andrew Dunstan <andrew@dunslane.net>
Discussion: https://postgr.es/m/20211211012052.2blmzcmxnxqawd2z@alap3.anarazel.de
Backpatch: 10-, to make backpatching of tests easier.
2021-12-13 12:02:42 -08:00
Alexander Korotkov 7615edd1db Fix alignment in multirange_get_range() function
The multirange_get_range() function fails when two boundaries of the same
range have different alignments.  Fix that by adding proper pointer alignment.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/17300-dced2d01ddeb1f2f%40postgresql.org
Backpatch-through: 14
2021-12-13 17:20:07 +03:00
Michael Paquier a04fc56d03 Fix some typos with {a,an}
One of the changes impacts the documentation, so backpatch.

Author: Peter Smith
Discussion: https://postgr.es/m/CAHut+Pu6+c+r3mY24VT7u+H+E_s6vMr5OdRiZ8NT3EOa-E5Lmw@mail.gmail.com
Backpatch-through: 14
2021-12-09 15:20:45 +09:00
Amit Kapila 614b77d65a 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
2021-12-09 08:49:50 +05:30
Tom Lane 3d60343197 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
2021-12-08 16:54:53 -05:00
Andrew Dunstan e4d73d089f
Revert "Check that we have a working tar before trying to use it"
This reverts commit f920f7e799.

The patch in effect fixed a problem we didn't have and caused another
instead.

Backpatch to release 14 like original

Discussion: https://postgr.es/m/3655283.1638977975@sss.pgh.pa.us
2021-12-08 16:50:04 -05:00
Andrew Dunstan 90c08ed113
Check that we have a working tar before trying to use it
Issue exposed by commit edc2332550 and the buildfarm.

Backpatch to release 14 where this usage started.
2021-12-08 10:23:48 -05:00
Amit Kapila f2e1730ee9 Fix origin timestamp during decoding of ROLLBACK PREPARED operation.
This happens because we were passing incorrect arguments to
ReorderBufferFinishPrepared().

Author: Masahiko Sawada
Reviewed-by: Vignesh C
Backpatch-through: 14
Discussion: https://postgr.es/m/CAD21AoBqhUqgDZUhUVnnwKRubPDNJ6m6fJDPgok3E5cWJLL+pA@mail.gmail.com
2021-12-08 15:21:12 +05:30
Michael Paquier 64ab21f0e5 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
2021-12-08 11:01:14 +09:00
Andrew Dunstan 93094232c8
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.
2021-12-07 15:01:16 -05:00
Tom Lane ea5ecdadf6 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
2021-12-07 13:34:15 -05:00
Daniel Gustafsson b6360aa46c 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 <horikyota.ntt@gmail.com>
Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Discussion: https://postgr.es/m/20211202.135441.590555657708629486.horikyota.ntt@gmail.com
Discussion: https://postgr.es/m/CABWY_HCBUCjY1EJHrEGePGEaSZ5b29apgTohCyygtsqe_ySYng@mail.gmail.com
Backpatch-through: 10
2021-12-03 14:15:50 +01:00
Fujii Masao 5cb86a2622 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
2021-12-03 17:37:08 +09:00
Tom Lane 4cd2928543 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
2021-12-02 17:14:56 -05:00
Michael Paquier b6dac98b05 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.GI24052@telsasoft.com
Backpatch-through: 10
2021-12-02 10:31:29 +09:00
Tom Lane 8f4b0200e1 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
2021-12-01 13:44:47 -05:00
Amit Kapila 4b8eec716a 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
2021-12-01 10:18:27 +05:30
Tom Lane 175edafd1f Cope with cross-compiling when checking for a random-number source.
Commit 16f96c74d neglected to consider the possibility of cross-compiling,
causing cross-compiles to fail at the configure stage unless you'd
selected --with-openssl.  Since we're now more or less assuming that
/dev/urandom is available everywhere, it seems reasonable to assume
that the cross-compile target has it too, rather than failing.

Per complaint from Vincas Dargis.  Back-patch to v14 where this came in.

Discussion: https://postgr.es/m/0dc14a31-acaf-8cae-0df4-a87339b22bd9@gmail.com
2021-11-30 17:18:04 -05:00
Michael Paquier 5550a9c385 Fix compatibility thinko for fstat() on standard streams in win32stat.c
GetFinalPathNameByHandleA() cannot be used in compilation environments
where _WIN32_WINNT < 0x0600, meaning at least Windows XP used by some
buildfarm members under MinGW that Postgres still needs to support.
This was reported as a compilation warning by the buildfarm, but this is
actually worse than the report as the code would have not worked.

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

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

Reported-by: Justin Pryzby, via buildfarm member jacana
Author: Michael Paquier
Reviewed-by: Juan José Santamaría Flecha
Discussion: https://postgr.es/m/20211129050122.GK17618@telsasoft.com
Backpatch-through: 14
2021-11-30 09:55:56 +09:00
Tom Lane ce43b3dc52 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
2021-11-29 12:13:13 -05:00
Alvaro Herrera 1b1e4bfe7d
Harden be-gssapi-common.h for headerscheck
Surround the contents with a test that the feature is enabled by
configure, to silence header checking tools on systems without GSSAPI
installed.

Backpatch to 12, where the file appeared.

Discussion: https://postgr.es/m/202111161709.u3pbx5lxdimt@alvherre.pgsql
2021-11-26 17:00:29 -03:00
Alvaro Herrera 2187a22941
Document units for max_slot_wal_keep_size
The doc blurb failed to mention units, as well as lacking the point
about changeability.

Backpatch to 13.

Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reported by: b1000101@pm.me
Discussion: https://postgr.es/m/163760291192.26193.10801700492025355788@wrigleys.postgresql.org
2021-11-26 14:31:57 -03:00
Alvaro Herrera d24dac9549
Fix determination of broken LSN in OVERWRITTEN_CONTRECORD
In commit ff9f111bce I mixed up inconsistent definitions of the LSN of
the first record in a page, when the previous record ends exactly at the
page boundary.  The correct LSN is adjusted to skip the WAL page header;
I failed to use that when setting XLogReaderState->overwrittenRecPtr,
so at WAL replay time VerifyOverwriteContrecord would refuse to let
replay continue past that record.

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

Discussion: https://postgr.es/m/45597.1637694259@sss.pgh.pa.us
2021-11-26 11:14:27 -03:00
Daniel Gustafsson 371087d006 Fix GRANTED BY support in REVOKE ROLE statements
Commit 6aaaa76bb added support for the GRANTED BY clause in GRANT and
REVOKE statements, but missed adding support for checking the role in
the REVOKE ROLE case. Fix by checking that the parsed role matches the
CURRENT_ROLE/CURRENT_USER requirement, and also add some tests for it.
Backpatch to v14 where GRANTED BY support was introduced.

Discussion: https://postgr.es/m/B7F6699A-A984-4943-B9BF-CEB84C003527@yesql.se
Backpatch-through: 14
2021-11-26 14:02:01 +01:00
Peter Eisentraut 1cc13b83eb Remove unneeded Python includes
Inluding <compile.h> and <eval.h> has not been necessary since Python
2.4, since they are included via <Python.h>.  Morever, <eval.h> is
being removed in Python 3.11.  So remove these includes.

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

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

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

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

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

Author: Dmitry Koval, Juan José Santamaría Flecha
Discussion: https://postgr.es/m/17288-6b58a91025a8a8a3@postgresql.org
Backpatch-through: 14
2021-11-25 12:17:05 +09:00
Tom Lane 6d1bdd5e07 Doc: improve documentation about nextval()/setval().
Clarify that the results of nextval and setval are not guaranteed
persistent until the calling transaction commits.  Some people
seem to have drawn the opposite conclusion from the statement that
these functions are never rolled back, so re-word to avoid saying
it quite that way.

Discussion: https://postgr.es/m/CAKU4AWohO=NfM-4KiZWvdc+z3c1C9FrUBR6xnReFJ6sfy0i=Lw@mail.gmail.com
2021-11-24 13:37:12 -05:00
Heikki Linnakangas 88482d75cc Fix missing space in docs.
Author: Japin Li
Discussion: https://www.postgresql.org/message-id/MEYP282MB1669C36E5F733C2EFBDCB80BB6619@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
2021-11-24 18:34:30 +02:00
David Rowley c2dc7b9e15 Flush Memoize cache when non-key parameters change, take 2
It's possible that a subplan below a Memoize node contains a parameter
from above the Memoize node.  If this parameter changes then cache entries
may become out-dated due to the new parameter value.

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

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

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

Author: Hans Buschmann
Discussion: https://postgr.es/m/1633101364685.39218@nidsa.net
Backpatch-through: 10
2021-11-24 13:03:55 +09:00
David Rowley 7933bc0d13 Revert "Flush Memoize cache when non-key parameters change"
This reverts commit f94edb06ab.
2021-11-24 15:28:34 +13:00
David Rowley f94edb06ab Flush Memoize cache when non-key parameters change
It's possible that a subplan below a Memoize node contains a parameter
from above the Memoize node.  If this parameter changes then cache entries
may become out-dated due to the new parameter value.

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

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

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

Reported-by: Tom Lane
Author: David Rowley
Discussion: https://postgr.es/m/3004308.1632952496@sss.pgh.pa.us
Backpatch-through: 14, where Memoize was added
2021-11-24 10:07:38 +13:00
Tom Lane 0fdf67476c Adjust pg_dump's priority ordering for casts.
When a stored expression depends on a user-defined cast, the backend
records the dependency as being on the cast's implementation function
--- or indeed, if there's no cast function involved but just
RelabelType or CoerceViaIO, no dependency is recorded at all.  This
is problematic for pg_dump, which is at risk of dumping things in the
wrong order leading to restore failures.  Given the lack of previous
reports, the risk isn't that high, but it can be demonstrated if the
cast is used in some view whose rowtype is then used as an input or
result type for some other function.  (That results in the view
getting hoisted into the functions portion of the dump, ahead of
the cast.)

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

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

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

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

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

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

Discussion: https://postgr.es/m/CAPL5KHrkBniyQt5e1rafm5DdXvbgiiqfEQEJ9GjtVzN71Jj5pA@mail.gmail.com
2021-11-22 15:25:48 -05:00
Tom Lane e9af18c6c5 Probe $PROVE not $PERL while checking for modules needed by TAP tests.
Normally "prove" and "perl" come from the same Perl installation,
but we support the case where they don't (mainly because the MSys
buildfarm animals need this).  In that case, AX_PROG_PERL_MODULES
is completely the wrong thing to use, because it's checking what
"perl" has.  Instead, make a little TAP test script including the
required modules, and run that under "prove".

We don't need ax_prog_perl_modules.m4 at all after this change,
so remove it.

Back-patch to all supported branches, for the buildfarm's benefit.
(In v10, this also back-patches the effects of commit 264eb03aa.)

Andrew Dunstan and Tom Lane, per an observation by Noah Misch

Discussion: https://postgr.es/m/E1moZHS-0002Cu-Ei@gemulon.postgresql.org
2021-11-22 12:54:52 -05:00
Alvaro Herrera c985a43df3
Add missing words in comment
Reported by Zhihong Yu.

Discussion: https://postgr.es/m/CALNJ-vR6uZivg_XkB1zKjEXeyZDEgoYanFXB-++1kBT9yZQoUw@mail.gmail.com
2021-11-22 12:38:41 -03:00
Tom Lane 3bd7556bbe pg_receivewal, pg_recvlogical: allow canceling initial password prompt.
Previously it was impossible to terminate these programs via control-C
while they were prompting for a password.  We can fix that trivially
for their initial password prompts, by moving setup of the SIGINT
handler from just before to just after their initial GetConnection()
calls.

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

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

Tom Lane and Nathan Bossart

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

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

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

Discussion: https://postgr.es/m/17294-8f6c7962ce877edc@postgresql.org
2021-11-20 14:29:56 -05:00
Amit Kapila ead49ebc07 Fix parallel operations that prevent oldest xmin from advancing.
While determining xid horizons, we skip over backends that are running
Vacuum. We also ignore Create Index Concurrently, or Reindex Concurrently
for the purposes of computing Xmin for Vacuum. But we were not setting the
flags corresponding to these operations when they are performed in
parallel which was preventing Xid horizon from advancing.

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

Author: Masahiko Sawada
Reviewed-by: Amit Kapila
Backpatch-through: 13
Discussion: https://postgr.es/m/CAD21AoCLQqgM1sXh9BrDFq0uzd3RBFKi=Vfo6cjjKODm0Onr5w@mail.gmail.com
2021-11-19 09:14:09 +05:30
Tom Lane ed1c261a00 Use appropriate -Wno-warning switches when compiling bitcode.
We use "clang" to compile bitcode files for LLVM inlining.  That might
be different from the build's main C compiler, so it needs its own set
of compiler flags.  To simplify configure, we don't bother adding any
-W switches to that flag set; there's little need since the main build
will show us any warnings.  However, if we don't want to see unwanted
warnings, we still have to add any -Wno-warning switches we'd normally
use with clang.

This escaped notice before commit 9ff47ea41, which tried to add
-Wno-compound-token-split-by-macro; buildfarm animals using mismatched
CC and CLANG still showed those warnings.  I'm not sure why we never
saw any effects from the lack of -Wno-unused-command-line-argument
(maybe that's only activated by -Wall?).  clang does not currently
support -Wno-format-truncation or -Wno-stringop-truncation, although
in the interests of future-proofing and consistency I included tests
for those.

Back-patch to v11 where we started building bitcode files.

Discussion: https://postgr.es/m/2921539.1637254619@sss.pgh.pa.us
2021-11-18 14:50:13 -05:00
Michael Paquier 048f3ee618 Fix quoting of ACL item in table for upgrade binary compatibility checks
Per buildfarm member prion, that runs the regression tests under a role
name that uses a hyphen.  Issue introduced by 835bcba.

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

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

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

Author: Justin Pryzby
Reviewed-by: Jacob Champion, Peter Eisentraut, Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/20201206180248.GI24052@telsasoft.com
Backpatch-through: 12
2021-11-18 10:37:25 +09:00