Tom Lane [Wed, 24 Aug 2022 17:01:40 +0000 (13:01 -0400)]
Defend against stack overrun in a few more places.
SplitToVariants() in the ispell code, lseg_inside_poly() in geo_ops.c,
and regex_selectivity_sub() in selectivity estimation could recurse
until stack overflow; fix by adding check_stack_depth() calls.
So could next() in the regex compiler, but that case is better fixed by
converting its tail recursion to a loop. (We probably get better code
that way too, since next() can now be inlined into its sole caller.)
There remains a reachable stack overrun in the Turkish stemmer, but
we'll need some advice from the Snowball people about how to fix that.
Per report from Egor Chindyaskin and Alexander Lakhin. These mistakes
are old, so back-patch to all supported branches.
Richard Guo and Tom Lane
Discussion: https://postgr.es/m/
1661334672.
728714027@f473.i.mail.ru
David Rowley [Wed, 24 Aug 2022 11:46:40 +0000 (23:46 +1200)]
Doc: remove duplicate "a" from func.sgml
Author: Shinya Kato
Discussion: https://postgr.es/m/
76c01275776749a167f49379ebec57f1@oss.nttdata.com
Backpatch-through: 15, where that change was introduced
Peter Eisentraut [Wed, 24 Aug 2022 11:27:34 +0000 (13:27 +0200)]
Fix ICU locale option handling in CREATE DATABASE
The code took the LOCALE option as the default/fallback for
ICU_LOCALE, but this was neither documented nor intended, so remove
it. (It was probably left in from an earlier patch version.)
Reported-by: Marina Polyakova
Discussion: https://www.postgresql.org/message-id/flat/
f385ba25e7f8be427b8c582e5cca7d79%40postgrespro.ru#
515a31c5429d6d37ad1d5c9d66962a1e
Peter Eisentraut [Tue, 23 Aug 2022 19:50:12 +0000 (21:50 +0200)]
Message style adjustment
Tom Lane [Tue, 23 Aug 2022 13:55:37 +0000 (09:55 -0400)]
Doc: document possible need to raise kernel's somaxconn limit.
On fast machines, it's possible for applications such as pgbench
to issue connection requests so quickly that the postmaster's
listen queue overflows in the kernel, resulting in unexpected
failures (with not-very-helpful error messages). Most modern OSes
allow the queue size to be increased, so document how to do that.
Per report from Kevin McKibbin.
Discussion: https://postgr.es/m/CADc_NKg2d+oZY9mg4DdQdoUcGzN2kOYXBu-3--RW_hEe0tUV=g@mail.gmail.com
Tom Lane [Tue, 23 Aug 2022 13:41:37 +0000 (09:41 -0400)]
Doc: prefer sysctl to /proc/sys in docs and comments.
sysctl is more portable than Linux's /proc/sys file tree, and
often easier to use too. That's why most of our docs refer to
sysctl when talking about how to adjust kernel parameters.
Bring the few stragglers into line.
Discussion: https://postgr.es/m/361175.
1661187463@sss.pgh.pa.us
Amit Kapila [Tue, 23 Aug 2022 04:38:43 +0000 (10:08 +0530)]
Add CHECK_FOR_INTERRUPTS while decoding changes.
While decoding changes in a loop, if we skip all the changes there is no
CFI making the loop uninterruptible.
Reported-by: Whale Song and Andrey Borodin
Bug: 17580
Author: Masahiko Sawada
Reviwed-by: Amit Kapila
Backpatch-through: 10
Discussion: https://postgr.es/m/17580-
849c1d5b6d7eb422@postgresql.org
Discussion: https://postgr.es/m/
B319ECD6-9A28-4CDF-A8F4-
3591E0BF2369@yandex-team.ru
Andres Freund [Tue, 23 Aug 2022 03:16:50 +0000 (20:16 -0700)]
pgstat: Acquire lock when reading variable-numbered stats
Somewhere during the development of the patch acquiring a lock during read
access to variable-numbered stats got lost. The missing lock acquisition won't
cause corruption, but can lead to reading torn values when accessing
stats. Add the missing lock acquisitions.
Reported-by: Greg Stark
Reviewed-by: "Drouvot, Bertrand"
Reviewed-by: Andres Freund
Author: Kyotaro Horiguchi
Discussion: https://postgr.es/m/CAM-w4HMYkM_DkYhWtUGV+qE_rrBxKOzOF0+5faozxO3vXrc9wA@mail.gmail.com
Backpatch: 15-
Peter Eisentraut [Mon, 22 Aug 2022 13:31:50 +0000 (15:31 +0200)]
Fix assertion failure in CREATE DATABASE
An assertion would fail when creating a database with libc locale
provider from a template database with icu locale provider.
Reported-by: Marina Polyakova
Reviewed-by: Julien Rouhaud
Discussion: https://www.postgresql.org/message-id/flat/
f385ba25e7f8be427b8c582e5cca7d79%40postgrespro.ru#
515a31c5429d6d37ad1d5c9d66962a1e
Peter Eisentraut [Mon, 22 Aug 2022 11:26:52 +0000 (13:26 +0200)]
pg_upgrade: Fix thinko in database info acquisition routine
When checking whether the major version supports per-database locale
providers, it was always looking at the version of the old cluster
instead of the cluster that was passed in. This would lead to
failures to detect locale provider mismatches.
Reported-by: Marina Polyakova
Reviewed-by: Julien Rouhaud
Discussion: https://www.postgresql.org/message-id/flat/
f385ba25e7f8be427b8c582e5cca7d79%40postgrespro.ru#
515a31c5429d6d37ad1d5c9d66962a1e
Amit Kapila [Mon, 22 Aug 2022 03:21:25 +0000 (08:51 +0530)]
Use logical operator && instead of & in vacuumparallel.c.
As such the current usage of & won't produce incorrect results but it
would be better to use && to short-circuit the evaluation of second
condition when the same is not required.
Author: Ranier Vilela
Reviewed-by: Tom Lane, Bharath Rupireddy
Backpatch-through: 15, where it was introduced
Discussion: https://postgr.es/m/CAEudQApL8QcoYwQuutkWKY_h7gBY8F0Xs34YKfc7-G0i83K_pw@mail.gmail.com
David Rowley [Fri, 19 Aug 2022 23:40:59 +0000 (11:40 +1200)]
Remove shadowed local variables that are new in v15
Compiling with -Wshadow=compatible-local yields quite a few warnings about
local variables being shadowed by compatible local variables in an inner
scope. Of course, this is perfectly valid in C, but we have had bugs in
the past as a result of developers failing to notice this.
af7d270dd is a
recent example.
Here we do a cleanup of warnings we receive from -Wshadow=compatible-local
for code which is new to PostgreSQL 15. We've yet to have the discussion
about if we actually ever want to run that as a standard compilation flag.
We'll need to at least get the number of warnings down to something easier
to manage before we can realistically consider if we want this or not.
This commit is the first step towards reducing the warnings.
The changes being made here are all fairly trivial. Because of that, and
the fact that v15 is still in beta, this is being back-patched into 15.
It seems more risky not to do this as the risk of future bugs is increased
by the additional conflicts that this commit could cause for any future
bug fixes touching the same areas as this commit.
Author: Justin Pryzby
Discussion: https://postgr.es/m/
20220817145434.GC26426%40telsasoft.com
Backpatch-through: 15
Peter Geoghegan [Fri, 19 Aug 2022 16:26:06 +0000 (09:26 -0700)]
Avoid reltuples distortion in very small tables.
Consistently avoid trusting a sample of only one page at the point that
VACUUM determines a new reltuples for the target table (though only when
the table is larger than a single page). This is follow-up work to
commit
74388a1a, which added a heuristic to prevent reltuples from
becoming distorted by successive VACUUM operations that each scan only a
single heap page (which was itself more or less a bugfix for an issue in
commit
44fa8488, which simplified VACUUM's handling of scanned pages).
The original bugfix commit did not account for certain remaining cases
that where not affected by its "2% of total relpages" heuristic. This
happened with relations that are small enough that just one of its pages
exceeded the 2% threshold, yet still big enough for VACUUM to deem
skipping most of its pages via the visibility map worthwhile. reltuples
could still become distorted over time with such a table, at least in
scenarios where the VACUUM command is run repeatedly and without the
table itself ever changing.
Author: Peter Geoghegan
Discussion: https://postgr.es/m/CAH2-Wzk7d4m3oEbEWkWQKd+gz-eD_peBvdXVk1a_KBygXadFeg@mail.gmail.com
Backpatch: 15-, where the rules for scanned pages changed.
Michael Paquier [Fri, 19 Aug 2022 01:00:23 +0000 (10:00 +0900)]
doc: Improve some markups and some wording around archiving modules
This commit adds or fixes used markups in a couple of places in the docs
(for
, and ). While on it, clarify some
of the documentation added recently for archiving modules with
archive_command, that would still be used as default choice if no
external module is defined (though an archive module could as well use
an archive_command).
Author: Maxim Yablokov
Discussion: https://postgr.es/m/b47ec4e8-6f6a-2aba-038e-d5db150b245e@postgrespro.ru
Backpatch-through: 15
Peter Geoghegan [Fri, 19 Aug 2022 00:34:12 +0000 (17:34 -0700)]
Initialize index stats during parallel VACUUM.
Initialize shared memory allocated for index stats to avoid a hard
crash. This was possible when parallel VACUUM became confused about the
current phase of index processing.
Oversight in commit
8e1fae1938, which refactored parallel VACUUM.
Author: Masahiko Sawada
Reported-By: Justin Pryzby
Discussion: https://postgr.es/m/20220818133406[email protected]
Backpatch: 15-, the first version with the refactoring commit.
Jeff Davis [Thu, 18 Aug 2022 21:23:59 +0000 (14:23 -0700)]
Use correct LSN for error reporting in pg_walinspect
Usage of ReadNextXLogRecord()'s first_record parameter for error
reporting isn't always correct. For instance, in GetWALRecordsInfo()
and GetWalStats(), we're reading multiple records, and first_record
is always passed as the LSN of the first record which is then used
for error reporting for later WAL record read failures. This isn't
correct.
The correct parameter to use for error reports in case of WAL
reading failures is xlogreader->EndRecPtr. This change fixes it.
While on it, removed an unnecessary Assert in pg_walinspect code.
Reported-by: Robert Haas
Author: Bharath Rupireddy
Reviewed-by: Robert Haas
Discussion: https://www.postgresql.org/message-id/CA%2BTgmoZAOGzPUifrcZRjFZ2vbtcw3mp-mN6UgEoEcQg6bY3OVg%40mail.gmail.com
Backpatch-through: 15
Tom Lane [Thu, 18 Aug 2022 16:11:47 +0000 (12:11 -0400)]
Fix subtly-incorrect matching of parent and child partitioned indexes.
When creating a partitioned index, DefineIndex tries to identify
any existing indexes on the partitions that match the partitioned
index, so that it can absorb those as child indexes instead of
building new ones. Part of the matching is to compare IndexInfo
structs --- but that wasn't done quite right. We're comparing
the IndexInfo built within DefineIndex itself to one made from
existing catalog contents by BuildIndexInfo. Notably, while
BuildIndexInfo will run index expressions and predicates through
expression preprocessing, that has not happened to DefineIndex's
struct. The result is failure to match and subsequent creation
of duplicate indexes.
The easiest and most bulletproof fix is to build a new IndexInfo
using BuildIndexInfo, thereby guaranteeing that the processing done
is identical.
While here, let's also extract the opfamily and collation data
from the new partitioned index, removing ad-hoc logic that
duplicated knowledge about how those are constructed.
Per report from Christophe Pettus. Back-patch to v11 where
we invented partitioned indexes.
Richard Guo and Tom Lane
Discussion: https://postgr.es/m/
8864BFAA-81FD-4BF9-8E06-
7DEB8D4164ED@thebuild.com
Robert Haas [Thu, 18 Aug 2022 15:09:39 +0000 (11:09 -0400)]
When using the WAL-logged CREATE DATABASE strategy, bulk extend.
This should improve performance, and was suggested by Andres Freund.
Back-patch to v15 to keep the code consistent across branches.
Dilip Kumar
Discussion: http://postgr.es/m/
C3458199-FEDD-4356-865A-
08DFAA5D4065@anarazel.de
Discussion: http://postgr.es/m/CAFiTN-sJ0vVpJrZ=R5M+g7Tr8=NN4wKOtrqOcDEsfFfnZgivVA@mail.gmail.com
Peter Eisentraut [Thu, 18 Aug 2022 09:33:53 +0000 (11:33 +0200)]
Simplify and clarify an error message
John Naylor [Thu, 18 Aug 2022 01:57:13 +0000 (08:57 +0700)]
Refer to replication origin roident as "ID" in user facing messages and docs
The table column that stores this is of type oid, but is actually limited
to uint16 and has a different path for creating new values. Some of
the documentation already referred to it as an ID, so let's standardize
on that.
While at it, most format strings already use %u, so for consintency
change the remaining stragglers using %d.
Per suggestions from Tom Lane and Justin Pryzby
Discussion: https://www.postgresql.org/message-id/
3437166.
1659620465%40sss.pgh.pa.us
Backpatch to v15
Michael Paquier [Wed, 17 Aug 2022 05:55:24 +0000 (14:55 +0900)]
Allow event trigger table_rewrite for ALTER MATERIALIZED VIEW
This event can happen when using SET ACCESS METHOD, as the data files of
the materialized need a full refresh but this command tag was not
updated to reflect that. The documentation is updated to track this
behavior.
Author: Onder Kalaci
Discussion: https://postgr.es/m/CACawEhXwHN3X34FiwoYG8vXR-oyUdrp7qcfRWSzS+NPahS5gSw@mail.gmail.com
Backpatch-through: 15
Tomas Vondra [Tue, 16 Aug 2022 21:52:10 +0000 (23:52 +0200)]
Fix assert in logicalmsg_desc
The assert, introduced by
9f1cf97bb5, is intended to check if the prefix
is terminated by a \0 byte, but it has two flaws. Firstly, prefix_size
includes the \0 byte, so prefix[prefix_size] points to the byte after
the null byte. Secondly, the check ensures the byte is not equal \0,
while it should be checking the opposite.
Backpatch-through: 14
Discussion: https://postgr.es/m/
b99b6101-2f14-3796-3dfa-
4a6cd7d4326d@enterprisedb.com
Daniel Gustafsson [Tue, 16 Aug 2022 20:54:43 +0000 (22:54 +0200)]
doc: Remove reference to tty libpq connstring param
The tty connection string parameter was removed in commit
14d9b3760
but the reference to it in the docs was mistakenly kept. Fix by
removing it from the libpq documentation. Backpatch through v14
where the parameter was removed.
Author: Noriyoshi Shinoda
Discussion: https://postgr.es/m/DM4PR84MB173433216FCC2A3961879000EE6B9@DM4PR84MB1734.NAMPRD84.PROD.OUTLOOK.COM
Backpatch-through: 14
Daniel Gustafsson [Tue, 16 Aug 2022 10:44:24 +0000 (12:44 +0200)]
doc: Add missing parenthesis to keycombo
The SIGINT keycombo for the pg_waldump stats emission was lacking a
closing parenthesis. Backpatch to 15 where this feature was added.
Discussion: https://postgr.es/m/
EC39E60E-C8B6-4CDF-8BFA-
E4D140446B41@yesql.se
Backpatch-through: 15
Amit Kapila [Tue, 16 Aug 2022 09:44:27 +0000 (15:14 +0530)]
Fix replica identity check for a partitioned table.
The current publisher code checks if UPDATE or DELETE can be executed with
the replica identity of the table even if it's a partitioned table. We can
skip checking the replica identity for partitioned tables because the
operations are actually performed on the leaf partitions (not the
partitioned table).
Reported-by: Brad Nicholson
Author: Hou Zhijie
Reviewed-by: Peter Smith, Amit Kapila
Backpatch-through: 13
Discussion: https://postgr.es/m/CAMMnM%3D8i5DohH%3DYKzV0_wYuYSYvuOJoL9F5nzXTc%2ByzsG1f6rg%40mail.gmail.com
Tatsuo Ishii [Tue, 16 Aug 2022 00:26:40 +0000 (09:26 +0900)]
doc: fix wrong tag used in create sequence manual.
In ref/create_sequence.sgml tag was used for nextval function name.
This should have been tag.
Author: Noboru Saito
Discussion: https://postgr.es/m/CAAM3qnJTDFFfRf5JHJ4AYrNcqXgMmj0pbH0%2Bvm%3DYva%2BpJyGymA%40mail.gmail.com
Backpatch-through: 10
Tom Lane [Mon, 15 Aug 2022 19:40:07 +0000 (15:40 -0400)]
Add missing bad-PGconn guards in libpq entry points.
There's a convention that externally-visible libpq functions should
check for a NULL PGconn pointer, and fail gracefully instead of
crashing. PQflush() and PQisnonblocking() didn't get that memo
though. Also add a similar check to PQdefaultSSLKeyPassHook_OpenSSL;
while it's not clear that ordinary usage could reach that with a
null conn pointer, it's cheap enough to check, so let's be consistent.
Daniele Varrazzo and Tom Lane
Discussion: https://postgr.es/m/CA+mi_8Zm_mVVyW1iNFgyMd9Oh0Nv8-F+7Y3-BqwMgTMHuo_h2Q@mail.gmail.com
Michael Paquier [Mon, 15 Aug 2022 04:37:32 +0000 (13:37 +0900)]
Fix outdated --help message for postgres -f
This option switch supports a total of 8 values, as told by
set_plan_disabling_options() and the documentation, but this was not
reflected in the output generated by --help.
Author: Junwang Zhao
Discussion: https://postgr.es/m/CAEG8a3+pT3cWzyjzKs184L1XMNm8NDnoJLiSjAYSO7XqpRh_vA@mail.gmail.com
Backpatch-through: 10
Tom Lane [Sun, 14 Aug 2022 16:05:27 +0000 (12:05 -0400)]
Preserve memory context of VarStringSortSupport buffers.
When enlarging the work buffers of a VarStringSortSupport object,
varstrfastcmp_locale was careful to keep them in the ssup_cxt
memory context; but varstr_abbrev_convert just used palloc().
The latter creates a hazard that the buffers could be freed out
from under the VarStringSortSupport object, resulting in stomping
on whatever gets allocated in that memory later.
In practice, because we only use this code for ICU collations
(cf.
3df9c374e), the problem is confined to use of ICU collations.
I believe it may have been unreachable before the introduction
of incremental sort, too, as traditional sorting usually just
uses one context for the duration of the sort.
We could fix this by making the broken stanzas in varstr_abbrev_convert
match the non-broken ones in varstrfastcmp_locale. However, it seems
like a better idea to dodge the issue altogether by replacing the
pfree-and-allocate-anew coding with repalloc, which automatically
preserves the chunk's memory context. This fix does add a few cycles
because repalloc will copy the chunk's content, which the existing
coding assumes is useless. However, we don't expect that these buffer
enlargement operations are performance-critical. Besides that, it's
far from obvious that copying the buffer contents isn't required, since
these stanzas make no effort to mark the buffers invalid by resetting
last_returned, cache_blob, etc. That seems to be safe upon examination,
but it's fragile and could easily get broken in future, which wouldn't
get revealed in testing with short-to-moderate-size strings.
Per bug #17584 from James Inform. Whether or not the issue is
reachable in the older branches, this code has been broken on its
own terms from its introduction, so patch all the way back.
Discussion: https://postgr.es/m/17584-
95c79b4a7d771f44@postgresql.org
Tom Lane [Sat, 13 Aug 2022 20:59:58 +0000 (16:59 -0400)]
Avoid misbehavior when hash_table_bytes < bucket_size.
It's possible to reach this case when work_mem is very small and tupsize
is (relatively) very large. In that case ExecChooseHashTableSize would
get an assertion failure, or with asserts off it'd compute nbuckets = 0,
which'd likely cause misbehavior later (I've not checked). To fix,
clamp the number of buckets to be at least 1.
This is due to faulty conversion of old my_log2() coding in
28d936031.
Back-patch to v13, as that was.
Zhang Mingli
Discussion: https://postgr.es/m/
beb64ca0-91e2-44ac-bf4a-
7ea36275ec02@Spark
Tom Lane [Sat, 13 Aug 2022 19:21:28 +0000 (15:21 -0400)]
Catch stack overflow when recursing in transformFromClauseItem().
Most parts of the parser can expect that the stack overflow check
in transformExprRecurse() will trigger before things get desperate.
However, transformFromClauseItem() can recurse directly to self
without having analyzed any expressions, so it's possible to drive
it to a stack-overrun crash. Add a check to prevent that.
Per bug #17583 from Egor Chindyaskin. Back-patch to all supported
branches.
Richard Guo
Discussion: https://postgr.es/m/17583-
33be55b9f981f75c@postgresql.org
Peter Eisentraut [Sat, 13 Aug 2022 08:32:38 +0000 (10:32 +0200)]
Add missing fields to _outConstraint()
As of
897795240cfaaed724af2f53ed2c50c9862f951f, check constraints can
be declared invalid. But that patch didn't update _outConstraint() to
also show the relevant struct fields (which were only applicable to
foreign keys before that). This currently only affects debugging
output, so no impact in practice.
Peter Eisentraut [Fri, 12 Aug 2022 22:00:41 +0000 (00:00 +0200)]
pg_upgrade: Fix some minor code issues
96ef3b8ff1cf1950e897fd2f766d4bd9ef0d5d56 accidentally copied a not
applicable comment from the float8_pass_by_value code to the
data_checksums code. Remove that.
87d3b35a1ca31a9d947a8f919a6006679216dff0 changed pg_upgrade to
checking the checksum version rather than just the Boolean presence of
checksums, but didn't change the field type in its ControlData struct
from bool. So this would not work correctly if there ever is a
checksum version larger than 1.
Bruce Momjian [Fri, 12 Aug 2022 19:43:23 +0000 (15:43 -0400)]
doc: add missing role attributes to user management section
Reported-by: Shinya Kato
Discussion: https://postgr.es/m/
1ecdb1ff78e9b03dfce37e85eaca725a@oss.nttdata.com
Author: Shinya Kato
Backpatch-through: 10
Bruce Momjian [Fri, 12 Aug 2022 19:05:13 +0000 (15:05 -0400)]
doc: add section about heap-only tuples (HOT)
Reported-by: Jonathan S. Katz
Discussion: https://postgr.es/m/
c59ffbd5-96ac-a5a5-a401-
14f627ca1405@postgresql.org
Backpatch-through: 11
Bruce Momjian [Fri, 12 Aug 2022 16:02:21 +0000 (12:02 -0400)]
doc: warn about security issues around log files
Reported-by: Simon Riggs
Discussion: https://postgr.es/m/CANP8+jJESuuXYq9Djvf-+tx2vY2OFLmfEuu+UvwHNJ1RT7iJCQ@mail.gmail.com
Author: Simon Riggs
Backpatch-through: 10
Bruce Momjian [Fri, 12 Aug 2022 15:35:23 +0000 (11:35 -0400)]
doc: clarify configuration file for Windows builds
The use of file 'config.pl' was not clearly explained.
Reported-by: [email protected]
Discussion: https://postgr.es/m/
164246013804.31952.
4958087335645367498@wrigleys.postgresql.org
Backpatch-through: 10
Bruce Momjian [Fri, 12 Aug 2022 15:26:03 +0000 (11:26 -0400)]
doc: document the CREATE INDEX "USING" clause
Somehow this was in the syntax but had no description.
Reported-by: [email protected]
Discussion: https://postgr.es/m/
164228771825.31954.
2719791849363756957@wrigleys.postgresql.org
Backpatch-through: 10
Bruce Momjian [Fri, 12 Aug 2022 14:59:00 +0000 (10:59 -0400)]
doc: clarify CREATE TABLE AS ... IF NOT EXISTS
Mention that the table is not modified if it already exists.
Reported-by: [email protected]
Discussion: https://postgr.es/m/
164441177106.9677.
5991676148704507229@wrigleys.postgresql.org
Backpatch-through: 10
Bruce Momjian [Fri, 12 Aug 2022 14:30:01 +0000 (10:30 -0400)]
doc: improve wal_level docs for the 'minimal' level
Reported-by: David G. Johnston
Discussion: https://postgr.es/m/CAKFQuwZ24UcfkoyLLSW3PMGQATomOcw1nuYFRuMev-NoOF+mYw@mail.gmail.com
Author: David G. Johnston
Backpatch-through: 14, partial to 13
Bruce Momjian [Fri, 12 Aug 2022 13:06:48 +0000 (09:06 -0400)]
doc: clarify DROP EXTENSION dependent members text
Member tracking was added in PG 13.
Reported-by: David G. Johnston
Discussion: https://postgr.es/m/CAKFQuwY1YtxQHVWUFYvSnOjZ5VPpXjF33V52bSKEwFjK2K=1Aw@mail.gmail.com
Author: David G. Johnston
Backpatch-through: 13
Robert Haas [Fri, 12 Aug 2022 12:55:07 +0000 (08:55 -0400)]
Avoid using a fake relcache entry to own an SmgrRelation.
If an error occurs before we close the fake relcache entry, the the
fake relcache entry will be destroyed by the SmgrRelation will
survive until end of transaction. Its smgr_owner pointer ends up
pointing to already-freed memory.
The original reason for using a fake relcache entry here was to try
to avoid reusing an SMgrRelation across a relevant invalidation. To
avoid that problem, just call smgropen() again each time we need a
reference to it. Hopefully someday we will come up with a more
elegant approach, but accessing uninitialized memory is bad so let's
do this for now.
Dilip Kumar, reviewed by Andres Freund and Tom Lane. Report by
Justin Pryzby.
Discussion: http://postgr.es/m/
20220802175043[email protected]
Discussion: http://postgr.es/m/CAFiTN-vSFeE6_W9z698XNtFROOA_nSqUXWqLcG0emob_kJ+dEQ@mail.gmail.com
Alvaro Herrera [Fri, 12 Aug 2022 11:16:50 +0000 (13:16 +0200)]
MERGE docs adjustments
Per Justin Pryzby
Discussion: https://postgr.es/m/
20220801145257[email protected]
Discussion: https://postgr.es/m/
20220714162618[email protected]
Alvaro Herrera [Fri, 12 Aug 2022 10:05:50 +0000 (12:05 +0200)]
Reject MERGE in CTEs and COPY
The grammar added for MERGE inadvertently made it accepted syntax in
places that were not prepared to deal with it -- namely COPY and inside
CTEs, but invoking these things with MERGE currently causes assertion
failures or weird misbehavior in non-assertion builds. Protect those
places by checking for it explicitly until somebody decides to implement
it.
Reported-by: Alexey Borzov
Discussion: https://postgr.es/m/17579-
82482cd7b267b862@postgresql.org
Peter Eisentraut [Fri, 12 Aug 2022 06:17:30 +0000 (08:17 +0200)]
Fix _outConstraint() for "identity" constraints
The set of fields printed by _outConstraint() in the CONSTR_IDENTITY
case didn't match the set of fields actually used in that case. (The
code was probably uncarefully copied from the CONSTR_DEFAULT case.)
Fix that by using the right set of fields. Since there is no read
support for this node type, this is really just for debugging output
right now, so it doesn't affect anything important.
Amit Kapila [Fri, 12 Aug 2022 05:58:54 +0000 (11:28 +0530)]
Back-Patch "Add wait_for_subscription_sync for TAP tests."
This was originally done in commit
0c20dd33db for 16 only, to eliminate
duplicate code and as an infrastructure that makes it easier to write
future tests. However, it has been suggested that it would be good to
back-patch this testing infrastructure to aid future tests in
back-branches.
Backpatch to all supported versions.
Author: Masahiko Sawada
Reviewed by: Amit Kapila, Shi yu
Discussion: https://postgr.es/m/CAD21AoC-fvAkaKHa4t1urupwL8xbAcWRePeETvshvy80f6WV1A@mail.gmail.com
Discussion: https://postgr.es/m/
[email protected]
Peter Eisentraut [Thu, 11 Aug 2022 08:35:39 +0000 (10:35 +0200)]
Add missing space in _outA_Const() output
Mistake introduced by
639a86e36aaecb84faaf941dcd0b183ba0aba9e9.
Amit Kapila [Thu, 11 Aug 2022 04:26:49 +0000 (09:56 +0530)]
Fix catalog lookup with the wrong snapshot during logical decoding.
Previously, we relied on HEAP2_NEW_CID records and XACT_INVALIDATION
records to know if the transaction has modified the catalog, and that
information is not serialized to snapshot. Therefore, after the restart,
if the logical decoding decodes only the commit record of the transaction
that has actually modified a catalog, we will miss adding its XID to the
snapshot. Thus, we will end up looking at catalogs with the wrong
snapshot.
To fix this problem, this changes the snapshot builder so that it
remembers the last-running-xacts list of the decoded RUNNING_XACTS record
after restoring the previously serialized snapshot. Then, we mark the
transaction as containing catalog changes if it's in the list of initial
running transactions and its commit record has XACT_XINFO_HAS_INVALS. To
avoid ABI breakage, we store the array of the initial running transactions
in the static variables InitialRunningXacts and NInitialRunningXacts,
instead of storing those in SnapBuild or ReorderBuffer.
This approach has a false positive; we could end up adding the transaction
that didn't change catalog to the snapshot since we cannot distinguish
whether the transaction has catalog changes only by checking the COMMIT
record. It doesn't have the information on which (sub) transaction has
catalog changes, and XACT_XINFO_HAS_INVALS doesn't necessarily indicate
that the transaction has catalog change. But that won't be a problem since
we use snapshot built during decoding only to read system catalogs.
On the master branch, we took a more future-proof approach by writing
catalog modifying transactions to the serialized snapshot which avoids the
above false positive. But we cannot backpatch it because of a change in
the SnapBuild.
Reported-by: Mike Oh
Author: Masahiko Sawada
Reviewed-by: Amit Kapila, Shi yu, Takamichi Osumi, Kyotaro Horiguchi, Bertrand Drouvot, Ahsan Hadi
Backpatch-through: 10
Discussion: https://postgr.es/m/
81D0D8B0-E7C4-4999-B616-
1E5004DBDCD2%40amazon.com
Robert Haas [Wed, 10 Aug 2022 18:03:09 +0000 (14:03 -0400)]
Move basebackup code to new directory src/backend/backup
Reviewed by David Steele and Justin Pryzby
Discussion: http://postgr.es/m/CA+TgmoafqboATDSoXHz8VLrSwK_MDhjthK4hEpYjqf9_1Fmczw%40mail.gmail.com
Tom Lane [Wed, 10 Aug 2022 17:37:25 +0000 (13:37 -0400)]
Fix handling of R/W expanded datums that are passed to SQL functions.
fmgr_sql must make expanded-datum arguments read-only, because
it's possible that the function body will pass the argument to
more than one callee function. If one of those functions takes
the datum's R/W property as license to scribble on it, then later
callees will see an unexpected value, leading to wrong answers.
From a performance standpoint, it'd be nice to skip this in the
common case that the argument value is passed to only one callee.
However, detecting that seems fairly hard, and certainly not
something that I care to attempt in a back-patched bug fix.
Per report from Adam Mackler. This has been broken since we
invented expanded datums, so back-patch to all supported branches.
Discussion: https://postgr.es/m/WScDU5qfoZ7PB2gXwNqwGGgDPmWzz08VdydcPFLhOwUKZcdWbblbo-0Lku-qhuEiZoXJ82jpiQU4hOjOcrevYEDeoAvz6nR0IU4IHhXnaCA=@mackler.email
Discussion: https://postgr.es/m/187436.
1660143060@sss.pgh.pa.us
Daniel Gustafsson [Wed, 10 Aug 2022 12:16:31 +0000 (14:16 +0200)]
Fix typo in test_oat_hooks README
Discussion: https://postgr.es/m/
3F066AFE-19F9-4DF5-A498-
B09643857A39@yesql.se
Amit Kapila [Wed, 10 Aug 2022 11:00:59 +0000 (16:30 +0530)]
Improve the protocol message descriptions for 2PC logical replication.
The messages were using 'two-phase transaction' at some places and
'prepared transaction' at other places. Make them consistently use
'prepared transaction'.
Reported-by: Ekaterina Kiryanova
Author: Peter Smith
Reviewed by: Amit Kapila
Backpatch-through: 15
Discussion: https://postgr.es/m/
745414e7-efb2-a6ae-5b83-
fcbdf35aabc8@postgrespro.ru
Bruce Momjian [Wed, 10 Aug 2022 00:37:53 +0000 (20:37 -0400)]
doc: mention that COPY MATCH requires columns _in_ _order_
Question asked at SCaLE conference.
Reported-by: doc suggestion by Stephen Frost
Backpatch-through: 15
Tom Lane [Mon, 8 Aug 2022 20:42:02 +0000 (16:42 -0400)]
Stamp 15beta3.
Tom Lane [Mon, 8 Aug 2022 16:16:01 +0000 (12:16 -0400)]
Stabilize output of new regression test.
Per buildfarm, the output order of \dx+ isn't consistent across
locales. Apply NO_LOCALE to force C locale. There might be a
more localized way, but I'm not seeing it offhand, and anyway
there is nothing in this test module that particularly cares
about locales.
Security: CVE-2022-2625
Tom Lane [Mon, 8 Aug 2022 15:12:31 +0000 (11:12 -0400)]
In extensions, don't replace objects not belonging to the extension.
Previously, if an extension script did CREATE OR REPLACE and there was
an existing object not belonging to the extension, it would overwrite
the object and adopt it into the extension. This is problematic, first
because the overwrite is probably unintentional, and second because we
didn't change the object's ownership. Thus a hostile user could create
an object in advance of an expected CREATE EXTENSION command, and would
then have ownership rights on an extension object, which could be
modified for trojan-horse-type attacks.
Hence, forbid CREATE OR REPLACE of an existing object unless it already
belongs to the extension. (Note that we've always forbidden replacing
an object that belongs to some other extension; only the behavior for
previously-free-standing objects changes here.)
For the same reason, also fail CREATE IF NOT EXISTS when there is
an existing object that doesn't belong to the extension.
Our thanks to Sven Klemm for reporting this problem.
Security: CVE-2022-2625
Alvaro Herrera [Mon, 8 Aug 2022 10:39:52 +0000 (12:39 +0200)]
Translation updates
Source-Git-URL: ssh://
[email protected]/pgtranslation/messages.git
Source-Git-Hash:
32d3177724e332eac673a46449d46a4ee8670147
Tom Lane [Sun, 7 Aug 2022 19:53:34 +0000 (15:53 -0400)]
Doc: remove in-place tablespaces from v15 release notes.
Now that these have been back-patched, they're no longer a new
feature for v15.
Alvaro Herrera [Sun, 7 Aug 2022 08:19:40 +0000 (10:19 +0200)]
Remove unportable use of timezone in recent test
Per buildfarm member snapper
Discussion: https://postgr.es/m/129951.
1659812518@sss.pgh.pa.us
Tom Lane [Sat, 6 Aug 2022 15:50:23 +0000 (11:50 -0400)]
Fix data-corruption hazard in WAL-logged CREATE DATABASE.
RelationCopyStorageUsingBuffer thought it could skip copying
empty pages, but of course that does not work at all, because
subsequent blocks will be out of place.
Also fix it to acquire share lock on the source buffer. It *might*
be safe to not do that, but it's not very certain, and I don't think
this code deserves any benefit of the doubt.
Dilip Kumar, per complaint from me
Discussion: https://postgr.es/m/
3679800.
1659654066@sss.pgh.pa.us
Alvaro Herrera [Sat, 6 Aug 2022 13:52:10 +0000 (15:52 +0200)]
Improve recently-added test reliability
Commit
59be1c942a47 already tried to make
src/test/recovery/t/033_replay_tsp_drops more reliable, but it wasn't
enough. Try to improve on that by making this use of a replication slot
to be more like others. Also, don't drop the slot.
Make a few other stylistic changes while at it. It's still quite slow,
which is another thing that we need to fix in this script.
Backpatch to all supported branches.
Discussion: https://postgr.es/m/349302.
1659191875@sss.pgh.pa.us
Tom Lane [Fri, 5 Aug 2022 19:57:46 +0000 (15:57 -0400)]
Partially undo commit
94da73281.
On closer inspection, mcv.c isn't as broken for ScalarArrayOpExpr
as I thought. The Var-on-right issue is real enough, but actually
it does cope fine with a NULL array constant --- I was misled by
an XXX comment suggesting it didn't. Undo that part of the code
change, and replace the XXX comment with something less misleading.
Tom Lane [Fri, 5 Aug 2022 19:00:03 +0000 (15:00 -0400)]
Fix handling of bare boolean expressions in mcv_get_match_bitmap.
Since v14, the extended stats machinery will try to estimate for
otherwise-unsupported boolean expressions if they match an expression
available from an extended stats object. mcv.c did not get the memo
about this, and would spit up with "unknown clause type". Fortunately
the case is easy to handle, since we can expect the expression yields
boolean.
While here, replace some not-terribly-on-point assertions with
simpler runtime tests for lookup failure. That seems appropriate
so that we get an elog not a crash if we somehow get to the new
it-should-be-a-bool-expression code with a subexpression that
doesn't match any stats column.
Per report from Danny Shemesh. Thanks to Justin Pryzby for
preliminary investigation.
Discussion: https://postgr.es/m/CAFZC=QqD6=27wQPOW1pbRa98KPyuyn+7cL_Ay_Ck-roZV84vHg@mail.gmail.com
Tom Lane [Fri, 5 Aug 2022 17:58:37 +0000 (13:58 -0400)]
Fix non-bulletproof ScalarArrayOpExpr code for extended statistics.
statext_is_compatible_clause_internal() checked that the arguments
of a ScalarArrayOpExpr are one Var and one Const, but it would allow
cases where the Const was on the left. Subsequent uses of the clause
are not expecting that and would suffer assertion failures or core
dumps. mcv.c also had not bothered to cope with the case of a NULL
array constant, which seems really unacceptably sloppy of somebody.
(Although our tools failed us there too, since AFAIK neither Coverity
nor any compiler warned of the obvious use-of-uninitialized-variable
condition.) It seems best to handle that by having
statext_is_compatible_clause_internal() reject it.
Noted while fixing bug #17570. Back-patch to v13 where the
extended stats code grew some awareness of ScalarArrayOpExpr.
Alvaro Herrera [Fri, 5 Aug 2022 17:36:24 +0000 (19:36 +0200)]
Backpatch addition of .git-blame-ignore-revs
This makes it more convenient for git config to contain the
blame.ignoreRevsFile setting; otherwise current git versions complain if
the file is not present.
I constructed the file for each branch by scraping the file in branch
master for commits that appear in that branch. Because a few additional
pgindent commits have been added to the list in master since the list
was first created, this also propagates those to branches 14 and 15
where the file already existed. Also, some entries appear to have been
made using author-date rather than committer-date in the format string,
so some timestamps are changed. Also remove bogus whitespace in the
suggested `git log` format string.
Backpatch to all supported branches.
Discussion: https://postgr.es/m/
20220711163138[email protected]
Tom Lane [Fri, 5 Aug 2022 16:46:34 +0000 (12:46 -0400)]
Fix incorrect permissions-checking code for extended statistics.
Commit
a4d75c86b improved the extended-stats logic to allow extended
stats to be collected on expressions not just bare Vars. To apply
such stats, we first verify that the user has permissions to read all
columns used in the stats. (If not, the query will likely fail at
runtime, but the planner ought not do so.) That had to get extended
to check permissions of columns appearing within such expressions,
but the code for that was completely wrong: it applied pull_varattnos
to the wrong pointer, leading to "unrecognized node type" failures.
Furthermore, although you couldn't get to this because of that bug,
it failed to account for the attnum offset applied by pull_varattnos.
This escaped recognition so far because the code in question is not
reached when the user has whole-table SELECT privilege (which is the
common case), and because only subexpressions not specially handled
by statext_is_compatible_clause_internal() are at risk.
I think a large part of the reason for this bug is under-documentation
of what statext_is_compatible_clause() is doing and what its arguments
are, so do some work on the comments to try to improve that.
Per bug #17570 from Alexander Kozhemyakin. Patch by Richard Guo;
comments and other cosmetic improvements by me. (Thanks also to
Japin Li for diagnosis.) Back-patch to v14 where the bug came in.
Discussion: https://postgr.es/m/17570-
f2f2e0f4bccf0965@postgresql.org
Alvaro Herrera [Fri, 5 Aug 2022 16:00:17 +0000 (18:00 +0200)]
BRIN: mask BRIN_EVACUATE_PAGE for WAL consistency checking
That bit is unlogged and therefore it's wrong to consider it in WAL page
comparison.
Add a test that tickles the case, as branch testing technology allows.
This has been a problem ever since wal consistency checking was
introduced (commit
a507b86900f6 for pg10), so backpatch to all supported
branches.
Author: 王海洋 (Haiyang Wang)
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/CACciXAD2UvLMOhc4jX9VvOKt7DtYLr3OYRBhvOZ-jRxtzc_7Jg@mail.gmail.com
Discussion: https://postgr.es/m/CACciXADOfErX9Bx0nzE_SkdfXr6Bbpo5R=v_B6MUTEYW4ya+cg@mail.gmail.com
Alvaro Herrera [Fri, 5 Aug 2022 09:55:52 +0000 (11:55 +0200)]
regress: fix test instability
Having additional triggers in a test table made the ORDER BY clauses in
old queries underspecified. Add another column there for stability.
Per sporadic buildfarm pink.
Etsuro Fujita [Fri, 5 Aug 2022 08:15:01 +0000 (17:15 +0900)]
postgres_fdw: Disable batch insertion when there are WCO constraints.
When inserting a view referencing a foreign table that has WITH CHECK
OPTION constraints, in single-insert mode postgres_fdw retrieves the
data that was actually inserted on the remote side so that the WITH
CHECK OPTION constraints are enforced with the data locally, but in
batch-insert mode it cannot currently retrieve the data (except for the
row first inserted through the view), resulting in enforcing the WITH
CHECK OPTION constraints with the data passed from the core (except for
the first-inserted row), which led to incorrect results when inserting
into a view referencing a foreign table in which a remote BEFORE ROW
INSERT trigger changes the rows inserted through the view so that they
violate the view's WITH CHECK OPTION constraint. Also, the query
inserting into the view caused an assertion failure in assert-enabled
builds.
Fix these by disabling batch insertion when inserting into such a view.
Back-patch to v14 where batch insertion was added.
Discussion: https://postgr.es/m/CAPmGK17LpbTZs4m4a_6THP54UBeK9fHvX8aVVA%2BC6yEZDZwQcg%40mail.gmail.com
Alvaro Herrera [Thu, 4 Aug 2022 18:02:02 +0000 (20:02 +0200)]
Fix ENABLE/DISABLE TRIGGER to handle recursion correctly
Using ATSimpleRecursion() in ATPrepCmd() to do so as
bbb927b4db9b did is
not correct, because ATPrepCmd() can't distinguish between triggers that
may be cloned and those that may not, so would wrongly try to recurse
for the latter category of triggers.
So this commit restores the code in EnableDisableTrigger() that
86f575948c77 had added to do the recursion, which would do it only for
triggers that may be cloned, that is, row-level triggers. This also
changes tablecmds.c such that ATExecCmd() is able to pass the value of
ONLY flag down to EnableDisableTrigger() using its new 'recurse'
parameter.
This also fixes what seems like an oversight of
86f575948c77 that the
recursion to partition triggers would only occur if EnableDisableTrigger()
had actually changed the trigger. It is more apt to recurse to inspect
partition triggers even if the parent's trigger didn't need to be
changed: only then can we be certain that all descendants share the same
state afterwards.
Backpatch all the way back to 11, like
bbb927b4db9b. Care is taken not
to break ABI compatibility (and that no catversion bump is needed.)
Co-authored-by: Amit Langote
Reviewed-by: Dmitry Koval
Discussion: https://postgr.es/m/CA+HiwqG-cZT3XzGAnEgZQLoQbyfJApVwOTQaCaas1mhpf+4V5A@mail.gmail.com
David Rowley [Thu, 4 Aug 2022 22:14:40 +0000 (10:14 +1200)]
Fix failure to set correct operator in window run condition
This was a simple omission in
9d9c02ccd where the code didn't correctly
set the operator to use in the run condition OpExpr when the window
function was both monotonically increasing and decreasing.
Bug discovered by Julien Roze, although he did not report it.
Reported-by: Phil Florent
Discussion: https://postgr.es/m/PA4P191MB160009A09B9D0624359278CFBA9F9@PA4P191MB1600.EURP191.PROD.OUTLOOK.COM
Backpatch-through: 15, where
9d9c02ccd was added
Robert Haas [Thu, 4 Aug 2022 19:17:14 +0000 (15:17 -0400)]
Revert recent changes to 002_pg_upgrade.pl.
The test is proving to be unreliable in the buildfarm, and we neither
agree on how best to fix it nor have time to do so before the upcoming
release. So for now, put things back to the way they were before commit
d498e052b4b84ae21b3b68d5b3fda6ead65d1d4d.
Discussion: http://postgr.es/m/
3628089.
1659640252@sss.pgh.pa.us
Tom Lane [Thu, 4 Aug 2022 18:16:26 +0000 (14:16 -0400)]
Fix check_exclusion_or_unique_constraint for UNIQUE NULLS NOT DISTINCT.
Adjusting this function was overlooked in commit
94aa7cc5f. The only
visible symptom (so far) is that INSERT ... ON CONFLICT could go into
an endless loop when inserting a null that has a conflict.
Richard Guo and Tom Lane, per bug #17558 from Andrew Kesper
Discussion: https://postgr.es/m/17558-
3f6599ffcf52fd4a@postgresql.org
Tom Lane [Thu, 4 Aug 2022 18:10:06 +0000 (14:10 -0400)]
Add CHECK_FOR_INTERRUPTS in ExecInsert's speculative insertion loop.
Ordinarily the functions called in this loop ought to have plenty
of CFIs themselves; but we've now seen a case where no such CFI is
reached, making the loop uninterruptible. Even though that's from
a recently-introduced bug, it seems prudent to install a CFI at
the loop level in all branches.
Per discussion of bug #17558 from Andrew Kesper (an actual fix for
that bug will follow).
Discussion: https://postgr.es/m/17558-
3f6599ffcf52fd4a@postgresql.org
Tom Lane [Thu, 4 Aug 2022 15:11:22 +0000 (11:11 -0400)]
Add proper regression test for the recent SRFs-in-pathkeys problem.
Remove the test case added by commit
fac1b470a, which never actually
worked to expose the problem it claimed to test. Replace it with
a case that does expose the problem, and also covers the SRF-not-
at-the-top deficiency repaired in
1aa8dad41.
Richard Guo, with some editorialization by me
Discussion: https://postgr.es/m/17564-
c7472c2f90ef2da3@postgresql.org
John Naylor [Thu, 4 Aug 2022 08:59:32 +0000 (15:59 +0700)]
Fix assorted doc typos
Erik Rijkers and Justin Pryzby
Backpatch to v14
Discussion: https://www.postgresql.org/message-id/
b79bfeff-d0e3-29a3-2576-
0e325848dede%40xs4all.nl
John Naylor [Thu, 4 Aug 2022 08:29:25 +0000 (15:29 +0700)]
Clarify DROP EXTENSION docs regarding explicitly dependent routines
Per suggestion from Robert Haas
Backpatch to v14
Discussion: https://www.postgresql.org/message-id/CA%2BTgmoZ1QvHquYHLkMy1oHKqz4-E7QQctj6e0ocq_GP1B5%2B9bA%40mail.gmail.com
Tom Lane [Wed, 3 Aug 2022 21:33:42 +0000 (17:33 -0400)]
Fix incorrect tests for SRFs in relation_can_be_sorted_early().
Commit
fac1b470a thought we could check for set-returning functions
by testing only the top-level node in an expression tree. This is
wrong in itself, and to make matters worse it encouraged others
to make the same mistake, by exporting tlist.c's special-purpose
IS_SRF_CALL() as a widely-visible macro. I can't find any evidence
that anyone's taken the bait, but it was only a matter of time.
Use expression_returns_set() instead, and stuff the IS_SRF_CALL()
genie back in its bottle, this time with a warning label. I also
added a couple of cross-reference comments.
After a fair amount of fooling around, I've despaired of making
a robust test case that exposes the bug reliably, so no test case
here. (Note that the test case added by
fac1b470a is itself
broken, in that it doesn't notice if you remove the code change.
The repro given by the bug submitter currently doesn't fail either
in v15 or HEAD, though I suspect that may indicate an unrelated bug.)
Per bug #17564 from Martijn van Oosterhout. Back-patch to v13,
as the faulty patch was.
Discussion: https://postgr.es/m/17564-
c7472c2f90ef2da3@postgresql.org
Tom Lane [Wed, 3 Aug 2022 15:14:55 +0000 (11:14 -0400)]
Reduce test runtime of src/test/modules/snapshot_too_old.
The sto_using_cursor and sto_using_select tests were coded to exercise
every permutation of their test steps, but AFAICS there is no value in
exercising more than one. This matters because each permutation costs
about six seconds, thanks to the "pg_sleep(6)". Perhaps we could
reduce that, but the useless permutations seem worth getting rid of
in any case. (Note that sto_using_hash_index got it right already.)
While here, clean up some other sloppiness such as an unused table.
This doesn't make too much difference in interactive testing, since the
wasted time is typically masked by parallelization with other tests.
However, the buildfarm runs this as a serial step, which means we can
expect to shave ~40 seconds from every buildfarm run. That makes it
worth back-patching.
Discussion: https://postgr.es/m/
2515192.
1659454702@sss.pgh.pa.us
Tom Lane [Tue, 2 Aug 2022 22:05:34 +0000 (18:05 -0400)]
Be more wary about 32-bit integer overflow in pg_stat_statements.
We've heard a couple of reports of people having trouble with
multi-gigabyte-sized query-texts files. It occurred to me that on
32-bit platforms, there could be an issue with integer overflow
of calculations associated with the total query text size.
Address that with several changes:
1. Limit pg_stat_statements.max to INT_MAX / 2 not INT_MAX.
The hashtable code will bound it to that anyway unless "long"
is 64 bits. We still need overflow guards on its use, but
this helps.
2. Add a check to prevent extending the query-texts file to
more than MaxAllocHugeSize. If it got that big, qtext_load_file
would certainly fail, so there's not much point in allowing it.
Without this, we'd need to consider whether extent, query_offset,
and related variables shouldn't be off_t not size_t.
3. Adjust the comparisons in need_gc_qtexts() to be done in 64-bit
arithmetic on all platforms. It appears possible that under duress
those multiplications could overflow 32 bits, yielding a false
conclusion that we need to garbage-collect the texts file, which
could lead to repeatedly garbage-collecting after every hash table
insertion.
Per report from Bruno da Silva. I'm not convinced that these
issues fully explain his problem; there may be some other bug that's
contributing to the query-texts file becoming so large in the first
place. But it did get that big, so #2 is a reasonable defense,
and #3 could explain the reported performance difficulties.
(See also commit
8bbe4cbd9, which addressed some related bugs.
The second Discussion: link is the thread that led up to that.)
This issue is old, and is primarily a problem for old platforms,
so back-patch.
Discussion: https://postgr.es/m/CAB+Nuk93fL1Q9eLOCotvLP07g7RAv4vbdrkm0cVQohDVMpAb9A@mail.gmail.com
Discussion: https://postgr.es/m/
5601D354.
5000703@BlueTreble.com
Tom Lane [Tue, 2 Aug 2022 14:29:35 +0000 (10:29 -0400)]
Change type "char"'s I/O format for non-ASCII characters.
Previously, a byte with the high bit set was just transmitted
as-is by charin() and charout(). This is problematic if the
database encoding is multibyte, because the result of charout()
won't be validly encoded, which breaks various stuff that
expects all text strings to be validly encoded. We've
previously decided to enforce encoding validity rather than try
to individually harden each place that might have a problem with
such strings, so it's time to do something about "char".
To fix, represent high-bit-set characters as \ooo (backslash
and three octal digits), following the ancient "escape" format
for bytea. charin() will continue to accept the old way as well,
though that is only reachable in single-byte encodings.
Add some test cases just so there is coverage for this code.
We'll otherwise leave this question undocumented as it was before,
because we don't really want to encourage end-user use of "char".
For the moment, back-patch into v15 so that this change appears
in 15beta3. If there's not great pushback we should consider
absorbing this change into the older branches.
Discussion: https://postgr.es/m/
2318797.
1638558730@sss.pgh.pa.us
Michael Paquier [Tue, 2 Aug 2022 10:56:06 +0000 (19:56 +0900)]
doc: Fix typos in protocol.sgml
Author: Ekaterina Kiryanova
Discussion: https://postgr.es/m/
745414e7-efb2-a6ae-5b83-
fcbdf35aabc8@postgrespro.ru
Backpatch-through: 15
Amit Kapila [Tue, 2 Aug 2022 03:50:38 +0000 (09:20 +0530)]
Remove duplicated wait for subscription sync from 007_ddl.pl.
An oversight in
8f2e2bbf14.
Author: Masahiko Sawada
Reviewed by: Amit Kapila
Backpatch-through: 15, where it was introduced
Discussion: https://postgr.es/m/CAD21AoC-fvAkaKHa4t1urupwL8xbAcWRePeETvshvy80f6WV1A@mail.gmail.com
Tom Lane [Mon, 1 Aug 2022 16:22:35 +0000 (12:22 -0400)]
Check maximum number of columns in function RTEs, too.
I thought commit
fd96d14d9 had plugged all the holes of this sort,
but no, function RTEs could produce oversize tuples too, either
via long coldeflists or just from multiple functions in one RTE.
(I'm pretty sure the other variants of base RTEs aren't a problem,
because they ultimately refer to either a table or a sub-SELECT,
whose widths are enforced elsewhere. But we explicitly allow join
RTEs to be overwidth, as long as you don't try to form their
tuple result.)
Per further discussion of bug #17561. As before, patch all branches.
Discussion: https://postgr.es/m/17561-
80350151b9ad2ad4@postgresql.org
Michael Paquier [Mon, 1 Aug 2022 07:39:16 +0000 (16:39 +0900)]
Fix error reporting after ioctl() call with pg_upgrade --clone
errno was not reported correctly after attempting to clone a file,
leading to incorrect error reports. While scanning through the code, I
have not noticed any similar mistakes.
Error introduced in
3a769d8.
Author: Justin Pryzby
Discussion: https://postgr.es/m/
20220731134135[email protected]
Backpatch-through: 12
Andres Freund [Mon, 1 Aug 2022 00:40:42 +0000 (17:40 -0700)]
ci: remove minor version from freebsd image name
This way .cirrus.yml does not need to be changed just because freebsd releases
a new minor version.
Discussion: https://postgr.es/m/
20220728095704[email protected]
Backpatch: 15-, just like the CI support
Tom Lane [Sun, 31 Jul 2022 20:58:20 +0000 (16:58 -0400)]
Remove test_oat_hooks.c's nodetag_to_string().
In the short time this function has existed, it's already proven to be
a nontrivial maintenance burden, since it has to be updated whenever a
node tag is added or removed. Although in principle we could now
automate that, I see little justification for having such functionality
here at all. The function is only being applied to utility statements,
for which we already have infrastructure for obtaining string names.
Moreover, that infrastructure produces already-familiar-to-users names,
unlike nodetag_to_string().
So, remove this function and use the existing infrastructure instead.
That saves over a thousand lines of largely-unreachable code.
Back-patch to v15 where this code came in. Although it seems unlikely
that v15's nodetag list will change anymore, we might as well keep the
two branches looking and acting alike; otherwise back-patching any
test-results changes in this area will be painful.
Discussion: https://postgr.es/m/843818.
1659218928@sss.pgh.pa.us
Andres Freund [Sun, 31 Jul 2022 19:31:40 +0000 (12:31 -0700)]
ci: switch to freebsd 13.1
freebsd 13.0 is out of support, switch to 13.1. It might be a good idea to
remove the minor version number from the image name, but there's not been a
response to that so far...
Discussion: https://postgr.es/m/
20220728095704[email protected]
Backpatch: 15-, just like the CI support
Tom Lane [Sun, 31 Jul 2022 17:43:17 +0000 (13:43 -0400)]
Fix trim_array() for zero-dimensional array argument.
The code tried to access ARR_DIMS(v)[0] and ARR_LBOUND(v)[0]
whether or not those values exist. This made the range check
on the "n" argument unstable --- it might or might not fail, and
if it did it would report garbage for the allowed upper limit.
These bogus accesses would probably annoy Valgrind, and if you
were very unlucky even lead to SIGSEGV.
Report and fix by Martin Kalcher. Back-patch to v14 where this
function was added.
Discussion: https://postgr.es/m/
baaeb413-b8a8-4656-5757-
ef347e5ec11f@aboutsource.net
Tom Lane [Sun, 31 Jul 2022 16:29:44 +0000 (12:29 -0400)]
Make new auto_explain test safe for log_error_verbosity = verbose.
Allow for the possible presence of a SQLSTATE code in the expected
warning message, similarly to
b998196bb and
19408aae7 (although
here I see no need to allow more than one specific SQLSTATE).
Per gripe from Andrew Dunstan.
Discussion: https://postgr.es/m/
c550ac53-5db5-3958-1798-
50bae3d9af71@dunslane.net
Tom Lane [Sat, 30 Jul 2022 17:05:15 +0000 (13:05 -0400)]
Fix incorrect is-this-the-topmost-join tests in parallel planning.
Two callers of generate_useful_gather_paths were testing the wrong
thing when deciding whether to call that function: they checked for
being at the top of the current join subproblem, rather than being at
the actual top join. This'd result in failing to construct parallel
paths for a sub-join for which they might be useful.
While set_rel_pathlist() isn't actively broken, it seems best to
make its identical-in-intention test for this be like the other two.
This has been wrong all along, but given the lack of field complaints
I'm hesitant to back-patch into stable branches; we usually prefer
to avoid non-bug-fix changes in plan choices in minor releases.
It seems not too late for v15 though.
Richard Guo, reviewed by Antonin Houska and Tom Lane
Discussion: https://postgr.es/m/CAMbWs4-mH8Zf87-w+3P2J=nJB+5OyicO28ia9q_9o=Lamf_VHg@mail.gmail.com
Robert Haas [Sat, 30 Jul 2022 03:24:39 +0000 (23:24 -0400)]
Revise test case added in
43746996399541ecb5c7b188725a5f097c15ceae.
Instead of using command_ok() to run psql, use safe_psql(). wrasse
isn't happy, and it be because of failure to pass -X to the psql
invocation, which safe_psql() will do automatically.
Since safe_psql() returns standard output instead of writing it to
a file, this requires some changes to the incantation for running
'diff'.
Test against the 'regression' database rather than 'postgres' so
we test more than just one table. That also means we need to record
the horizons later, after the test does "VACUUM FULL pg_largeobject".
Add an ORDER BY clause to the horizon query for stability.
Patch by me, reviewed by Tom Lane.
Discussion: http://postgr.es/m/CA+TgmoaGBbpzgu3=du1f9zDUbkfycO0y=_uWrLFy=KKEqXWeLQ@mail.gmail.com
Andrew Dunstan [Fri, 29 Jul 2022 21:43:34 +0000 (17:43 -0400)]
Fix new recovery test for log_error_verbosity=verbose case
The new test is from commit
9e4f914b5e.
With this setting messages have SQL error numbers included, so that
needs to be provided for in the pattern looked for.
Backpatch to all live branches like the original.
Robert Haas [Fri, 29 Jul 2022 20:31:57 +0000 (16:31 -0400)]
Fix brown paper bag bug in
bbe08b8869bd29d587f24ef18eb45c7d4d14afca.
We must issue the TRUNCATE command first and update relfrozenxid
and relminmxid afterward; otherwise, TRUNCATE overwrites the
previously-set values.
Add a test case like I should have done the first time.
Per buildfarm report from TestUpgradeXversion.pm, by way of Tom
Lane.
Tom Lane [Fri, 29 Jul 2022 17:30:50 +0000 (13:30 -0400)]
In transformRowExpr(), check for too many columns in the row.
A RowExpr with more than MaxTupleAttributeNumber columns would fail at
execution anyway, since we cannot form a tuple datum with more than that
many columns. While heap_form_tuple() has a check for too many columns,
it emerges that there are some intermediate bits of code that don't
check and can be driven to failure with sufficiently many columns.
Checking this at parse time seems like the most appropriate place to
install a defense, since we already check SELECT list length there.
While at it, make the SELECT-list-length error use the same errcode
(TOO_MANY_COLUMNS) as heap_form_tuple does, rather than the generic
PROGRAM_LIMIT_EXCEEDED.
Per bug #17561 from Egor Chindyaskin. The given test case crashes
in all supported branches (and probably a lot further back),
so patch all.
Discussion: https://postgr.es/m/17561-
80350151b9ad2ad4@postgresql.org
Robert Haas [Fri, 29 Jul 2022 15:20:07 +0000 (11:20 -0400)]
Fix mistake in
bbe08b8869bd29d587f24ef18eb45c7d4d14afca.
The earlier commit used pg_class.relfilenode where it should have
used pg_class.oid. This could lead to emitting an UPDATE statement
into the dump that would update nothing (or the wrong thing) when
executed in the new cluster, resulting in relfrozenxid and
relminmxid being improperly carried forward for pg_largeobject.
Noticed by Dilip Kumar.
Discussion: http://postgr.es/m/CAFiTN-ty1Gzs6stk2vt9BJiq0m0hzf=aPnh3a-4Z3Tk5GzoENw@mail.gmail.com
Alvaro Herrera [Fri, 29 Jul 2022 10:50:47 +0000 (12:50 +0200)]
Fix test instability
On FreeBSD, the new test fails due to a WAL file being removed before
the standby has had the chance to copy it. Fix by adding a replication
slot to prevent the removal until after the standby has connected.
Author: Kyotaro Horiguchi
Reported-by: Matthias van de Meent
Discussion: https://postgr.es/m/CAEze2Wj5nau_qpjbwihvmXLfkAWOZ5TKdbnqOc6nKSiRJEoPyQ@mail.gmail.com
Robert Haas [Thu, 28 Jul 2022 19:32:24 +0000 (15:32 -0400)]
Use TRUNCATE to preserve relfilenode for pg_largeobject + index.
Commit
9a974cbcba005256a19991203583a94b4f9a21a9 arranged to preserve
the relfilenode of user tables across pg_upgrade, but failed to notice
that pg_upgrade treats pg_largeobject as a user table and thus it needs
the same treatment. Otherwise, large objects will appear to vanish
after a pg_upgrade.
Commit
d498e052b4b84ae21b3b68d5b3fda6ead65d1d4d fixed this problem
by teaching pg_dump to UPDATE pg_class.relfilenode for pg_largeobject
and its index. However, because an UPDATE on the catalog rows doesn't
change anything on disk, this can leave stray files behind in the new
cluster. They will normally be empty, but it's a little bit untidy.
Hence, this commit arranges to do the same thing using DDL. Specifically,
it makes TRUNCATE work for the pg_largeobject catalog when in
binary-upgrade mode, and it then uses that command in binary-upgrade
dumps as a way of setting pg_class.relfilenode for pg_largeobject and
its index. That way, the old files are removed from the new cluster.
Discussion: http://postgr.es/m/CA+TgmoYYMXGUJO5GZk1-MByJGu_bB8CbOL6GJQC8=Bzt6x6vDg@mail.gmail.com
Alvaro Herrera [Thu, 28 Jul 2022 16:21:56 +0000 (18:21 +0200)]
doc: avoid saying "inheritance" ...
... when it applies to partitioned relations. This is almost the
opposite of
0c06534bd63b, which removed references to "partition" in
favour of "child".
Author: Justin Pryzby
Reviewed-by: Ashutosh Bapat
Reviewed-by: Amit Langote
Discussion: https://postgr.es/m/20220525013248[email protected]
Alvaro Herrera [Thu, 28 Jul 2022 06:26:05 +0000 (08:26 +0200)]
Fix replay of create database records on standby
Crash recovery on standby may encounter missing directories
when replaying database-creation WAL records. Prior to this
patch, the standby would fail to recover in such a case;
however, the directories could be legitimately missing.
Consider the following sequence of commands:
CREATE DATABASE
DROP DATABASE
DROP TABLESPACE
If, after replaying the last WAL record and removing the
tablespace directory, the standby crashes and has to replay the
create database record again, crash recovery must be able to continue.
A fix for this problem was already attempted in
49d9cfc68bf4, but it
was reverted because of design issues. This new version is based
on Robert Haas' proposal: any missing tablespaces are created
during recovery before reaching consistency. Tablespaces
are created as real directories, and should be deleted
by later replay. CheckRecoveryConsistency ensures
they have disappeared.
The problems detected by this new code are reported as PANIC,
except when allow_in_place_tablespaces is set to ON, in which
case they are WARNING. Apart from making tests possible, this
gives users an escape hatch in case things don't go as planned.
Author: Kyotaro Horiguchi
Author: Asim R Praveen
Author: Paul Guo
Reviewed-by: Anastasia Lubennikova (older versions)
Reviewed-by: Fujii Masao (older versions)
Reviewed-by: Michaël Paquier
Diagnosed-by: Paul Guo
Discussion: https://postgr.es/m/CAEET0ZGx9AvioViLf7nbR_8tH9-=27DN5xWJ2P9-ROH16e4JUA@mail.gmail.com