Thomas Munro [Thu, 12 Aug 2021 22:38:22 +0000 (10:38 +1200)]
Make EXEC_BACKEND more convenient on macOS.
It's hard to disable ASLR on current macOS releases, for testing with
-DEXEC_BACKEND. You could already set the environment variable
PG_SHMEM_ADDR to something not likely to collide with mappings created
earlier in process startup. Let's also provide a default value that
works on current releases and architectures, for developer convenience.
As noted in the pre-existing comment, this is a horrible hack, but
-DEXEC_BACKEND is only used by Unix-based PostgreSQL developers for
testing some otherwise Windows-only code paths, so it seems excusable.
Back-patch to all supported branches.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/
20210806032944.m4tz7j2w47mant26%40alap3.anarazel.de
Tomas Vondra [Thu, 12 Aug 2021 19:32:53 +0000 (21:32 +0200)]
Use appropriate tuple descriptor in FDW batching
The FDW batching code was using the same tuple descriptor both for all
slots (regular and plan slots), but that's incorrect - the subplan may
use a different descriptor. Currently this is benign, because batching
is used only for INSERTs, and in that case the descriptors always match.
But that would change if we allow batching UPDATEs.
Fix by copying the appropriate tuple descriptor. Backpatch to 14, where
the FDW batching was implemented.
Author: Amit Langote
Backpatch-through: 14, where FDW batching was added
Discussion: https://postgr.es/m/CA%2BHiwqEWd5B0-e-RvixGGUrNvGkjH2s4m95%3DJcwUnyV%3Df0rAKQ%40mail.gmail.com
John Naylor [Thu, 12 Aug 2021 13:08:56 +0000 (09:08 -0400)]
Speed up generation of Unicode hash functions.
Sets of Unicode keys are picky about the primes used when generating
a perfect hash function for them. Callers can spend many seconds
iterating through all the possible combinations of candidate
multipliers and seeds to find one that works.
Unicode updates typically happen only once a year, but it still makes
development and testing of Unicode scripts unnecessarily slow. To fix,
iterate over the primes in the innermost loop. This does not change
any existing functions checked into the tree.
John Naylor [Thu, 12 Aug 2021 12:53:41 +0000 (08:53 -0400)]
Fix grammar mistake in hash index README
Dilip Kumar
Discussion: https://www.postgresql.org/message-id/CAFiTN-tjZbuY6vy7kZZ6xO%2BD4mVcO5wOPB5KiwJ3AHhpytd8fg%40mail.gmail.com
Michael Paquier [Thu, 12 Aug 2021 11:12:47 +0000 (20:12 +0900)]
Avoid unnecessary shared invalidations in ROLLBACK PREPARED
The performance gain is minimal, but this makes the logic more
consistent with AtEOXact_Inval(). No other invalidation is needed in
this case as PREPARE takes already care of sending any local ones.
Author: Liu Huailing
Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/OSZPR01MB6215AA84D71EF2B3D354CF86BE139@OSZPR01MB6215.jpnprd01.prod.outlook.com
Heikki Linnakangas [Thu, 12 Aug 2021 08:02:29 +0000 (11:02 +0300)]
Fix segfault during EvalPlanQual with mix of local and foreign partitions.
It's not sensible to re-evaluate a direct-modify Foreign Update or Delete
during EvalPlanQual. However, ExecInitForeignScan() can still get called
if a table mixes local and foreign partitions. EvalPlanQualStart() left
the es_result_relations array uninitialized in the child EPQ EState, but
ExecInitForeignScan() still expected to find it. That caused a segfault.
Fix by skipping the es_result_relations lookup during EvalPlanQual
processing. To make things a bit more robust, also skip the
BeginDirectModify calls, and add a runtime check that ExecForeignScan()
is not called on direct-modify foreign scans during EvalPlanQual
processing.
This is new in v14, commit
1375422c782. Before that, EvalPlanQualStart()
copied the whole ResultRelInfo array to the EPQ EState. Backpatch to v14.
Report and diagnosis by Andrey Lepikhov.
Discussion: https://www.postgresql.org/message-id/
cb2b808d-cbaa-4772-76ee-
c8809bafcf3d%40postgrespro.ru
Tom Lane [Tue, 10 Aug 2021 22:10:29 +0000 (18:10 -0400)]
Fix failure of btree_gin indexscans with "char" type and </<= operators.
As a result of confusion about whether the "char" type is signed or
unsigned, scans for index searches like "col < 'x'" or "col <= 'x'"
would start at the middle of the index not the left end, thus missing
many or all of the entries they should find. Fortunately, this
is not a symptom of index corruption. It's only the search logic
that is broken, and we can fix it without unpleasant side-effects.
Per report from Jason Kim. This has been wrong since btree_gin's
beginning, so back-patch to all supported branches.
Discussion: https://postgr.es/m/
20210810001649.htnltbh7c63re42p@jasonk.me
Daniel Gustafsson [Tue, 10 Aug 2021 13:08:46 +0000 (15:08 +0200)]
Add alternative output for OpenSSL 3 without legacy loaded
OpenSSL 3 introduced the concept of providers to support modularization,
and moved the outdated ciphers to the new legacy provider. In case it's
not loaded in the users openssl.cnf file there will be a lot of regress
test failures, so add alternative outputs covering those.
Also document the need to load the legacy provider in order to use older
ciphers with OpenSSL-enabled pgcrypto.
This will be backpatched to all supported version once there is sufficient
testing in the buildfarm of OpenSSL 3.
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/
FEF81714-D479-4512-839B-
C769D2605F8A@yesql.se
Daniel Gustafsson [Tue, 10 Aug 2021 13:01:52 +0000 (15:01 +0200)]
Disable OpenSSL EVP digest padding in pgcrypto
The PX layer in pgcrypto is handling digest padding on its own uniformly
for all backend implementations. Starting with OpenSSL 3.0.0, DecryptUpdate
doesn't flush the last block in case padding is enabled so explicitly
disable it as we don't use it.
This will be backpatched to all supported version once there is sufficient
testing in the buildfarm of OpenSSL 3.
Reviewed-by: Peter Eisentraut, Michael Paquier
Discussion: https://postgr.es/m/
FEF81714-D479-4512-839B-
C769D2605F8A@yesql.se
Daniel Gustafsson [Tue, 10 Aug 2021 09:15:02 +0000 (11:15 +0200)]
Remove unused regression test certificate server-ss
The server-ss certificate was included in
e39250c64 but was never
used in the TLS regression tests so remove.
Author: Jacob Champion
Discussion: https://postgr.es/m/
d15a9838344ba090e09fd866abf913584ea19fb7.camel@vmware.com
Michael Paquier [Tue, 10 Aug 2021 06:54:42 +0000 (15:54 +0900)]
Add tab completion for DECLARE .. ASENSITIVE in psql
This option has been introduced in
dd13ad9.
Author: Shinya Kato
Discussion: https://postgr.es/m/TYAPR01MB289665526B76DA29DC70A031C4F09@TYAPR01MB2896.jpnprd01.prod.outlook.com
Michael Paquier [Tue, 10 Aug 2021 04:14:37 +0000 (13:14 +0900)]
Fix regression test output of sepgsql
The difference is caused by
7b56584, for the tests involving a table
rewrite.
Per buildfarm member rhinoceros.
Discussion: https://postgr.es/m/YRHxXcyFjPuPTZui@paquier.xyz
Michael Paquier [Tue, 10 Aug 2021 03:21:05 +0000 (12:21 +0900)]
Add call to object access hook at the end of table rewrite in ALTER TABLE
ALTER TABLE .. SET {LOGGED,UNLOGGED,ACCESS METHOD} would never do a
table-level object access hook, which was inconsistent with SET
TABLESPACE. Note that contrary to SET TABLESPACE, the no-op case is
left off for those commands as this requires tracking if commands have
been called, but they may not execute a physical rewrite. Another thing
worth noting is that the physical file swap at the end of a rewrite
does a couple of access calls for internal objects created for the swap
operation (internal objects are for example skipped by the tests of
sepgsql), but this does not trigger the hook for the table on which the
operation is done.
f41872d, that added support for SET LOGGED/UNLOGGED in ALTER TABLE,
visibly forgot to consider that.
Based on what I checked, two regression tests of sepgsql in ddl.sql are
going to log more information with this test, something that buildfarm
member rhinoceros will tell soon enough. I am not completely sure of
their format though, so these are not refreshed yet.
This is arguably a bug, but no backpatch is done as this could cause a
behavior change for anybody using object access hooks.
Reported-by: Jeff Davis
Discussion: https://postgr.es/m/YQJKV29/1a60uG68@paquier.xyz
Tom Lane [Tue, 10 Aug 2021 00:53:25 +0000 (20:53 -0400)]
Let regexp_replace() make use of REG_NOSUB when feasible.
If the replacement string doesn't contain \1...\9, then we don't
need sub-match locations, so we can use the REG_NOSUB optimization
here too. There's already a pre-scan of the replacement string
to look for backslashes, so extend that to check for digits, and
refactor to allow that to happen before we compile the regexp.
While at it, try to speed up the pre-scan by using memchr() instead
of a handwritten loop. It's likely that this is lost in the noise
compared to the regexp processing proper, but maybe not. In any
case, this coding is shorter.
Also, add some test cases to improve the poor coverage of
appendStringInfoRegexpSubstr().
Discussion: https://postgr.es/m/
3534632.
1628536485@sss.pgh.pa.us
Andres Freund [Mon, 9 Aug 2021 15:26:59 +0000 (08:26 -0700)]
Fix bogus assertion in BootstrapModeMain().
The assertion was always true, as written, thanks to me "simplifying" it
before commit.
Per coverity and Tom Lane.
Tom Lane [Mon, 9 Aug 2021 15:26:34 +0000 (11:26 -0400)]
Avoid determining regexp subexpression matches, when possible.
Identifying the precise match locations for parenthesized subexpressions
is a fairly expensive task given the way our regexp engine works, both
at regexp compile time (where we must create an optimized NFA for each
parenthesized subexpression) and at runtime (where determining exact
match locations requires laborious search).
Up to now we've made little attempt to optimize this situation. This
patch identifies cases where we know at compile time that we won't
need to know subexpression match locations, and teaches the regexp
compiler to not bother creating per-subexpression regexps for
parenthesis pairs that are not referenced by backrefs elsewhere in
the regexp. (To preserve semantics, we obviously still have to
pin down the match locations of backref references.) Users could
have obtained the same results before this by being careful to
write "non capturing" parentheses wherever possible, but few people
bother with that.
Discussion: https://postgr.es/m/
2219936.
1628115334@sss.pgh.pa.us
David Rowley [Mon, 9 Aug 2021 07:45:26 +0000 (19:45 +1200)]
Remove some special cases from MSVC build scripts
Here we add additional parsing of Makefiles to determine when to add
references to libpgport and libpgcommon. We also remove the need for
adding the current contrib_extrasource by adding sine very basic logic to
implement the Makefile rules which add .l and .y files when they exist for
a given .o file in the Makefile.
This is just some very basic additional parsing of Makefiles to try to
keep things more consistent between builds using make and MSVC builds.
This happens to work with how our current Makefiles are laid out, but it
could easily be broken in the future if someone chooses do something in
the Makefile that we don't have parsing support for. We will cross that
bridge when we come to it.
Author: David Rowley
Discussion: https://postgr.es/m/CAApHDvoPULi5JW3933NxgwxOmu9Ncvpcyt87UhEHAUX16QqmpA@mail.gmail.com
David Rowley [Mon, 9 Aug 2021 04:45:35 +0000 (16:45 +1200)]
Doc: Fix misleading statement about VACUUM memory limits
In
ec34040af I added a mention that there was no point in setting
maintenance_work_limit to anything higher than 1GB for vacuum, but that
was incorrect as ginInsertCleanup() also looks at what
maintenance_work_mem is set to during VACUUM and that's not limited to
1GB.
Here I attempt to make it more clear that the limitation is only around
the number of dead tuple identifiers that we can collect during VACUUM.
I've also added a note to autovacuum_work_mem to mention this limitation.
I didn't do that in
ec34040af as I'd had some wrong-headed ideas about
just limiting the maximum value for that GUC to 1GB.
Author: David Rowley
Discussion: https://postgr.es/m/CAApHDvpGwOAvunp-E-bN_rbAs3hmxMoasm5pzkYDbf36h73s7w@mail.gmail.com
Backpatch-through: 9.6, same as
ec34040af
David Rowley [Mon, 9 Aug 2021 03:47:00 +0000 (15:47 +1200)]
Use ExplainPropertyInteger for queryid in EXPLAIN
This saves a few lines of code. Also add a comment to mention why we use
ExplainPropertyInteger instead of ExplainPropertyUInteger given that
queryid is a uint64 type.
Author: David Rowley
Reviewed-by: Julien Rouhaud
Discussion: https://postgr.es/m/CAApHDvqhSLYpSU_EqUdN39w9Uvb8ogmHV7_3YhJ0S3aScGBjsg@mail.gmail.com
Backpatch-through: 14, where this code was originally added
Amit Kapila [Mon, 9 Aug 2021 03:28:38 +0000 (08:58 +0530)]
Fix typo in 022_twophase_cascade.pl.
Author: Peter Smith
Discussion: https://postgr.es/m/CAHut+Pta=zo8G1DWVVg-LU6b_JvHHCueC=AKVpKJOrwLzj9EZA@mail.gmail.com
David Rowley [Mon, 9 Aug 2021 03:23:48 +0000 (15:23 +1200)]
Add POPCNT support for MSVC x86_64 builds
02a6a54ec added code to make use of the POPCNT instruction when available
for many of our common platforms. Here we do the same for MSVC for x86_64
machines.
MSVC's intrinsic functions for popcnt seem to differ from GCCs in that
they always appear to emit the popcnt instructions. In GCC the behavior
will depend on if the source file was compiled with -mpopcnt or not. For
this reason, the MSVC intrinsic function has been lumped into the
pg_popcount*_asm function, however doing that sort of invalidates the name
of that function, so let's rename it to pg_popcount*_fast().
Author: David Rowley
Reviewed-by: John Naylor
Discussion: https://postgr.es/m/CAApHDvqL3cbbK%3DGzNcwzsNR9Gi%2BaUvTudKkC4XgnQfXirJ_oRQ%40mail.gmail.com
Bruce Momjian [Mon, 9 Aug 2021 01:05:46 +0000 (21:05 -0400)]
doc: mention pg_upgrade extension script
Since commit
e462856a7a, pg_upgrade automatically creates a script to
update extensions, so mention that instead of ALTER EXTENSION.
Backpatch-through: 9.6
Peter Eisentraut [Sun, 8 Aug 2021 20:05:42 +0000 (22:05 +0200)]
Remove some unnecessary casts in format arguments
We can use %zd or %zu directly, no need to cast to int. Conversely,
some code was casting away from int when it could be using %d
directly.
Tom Lane [Sun, 8 Aug 2021 19:35:30 +0000 (15:35 -0400)]
Doc: remove bogus <indexterm> items.
Copy-and-pasteo in
665c5855e, evidently. The 9.6 docs toolchain
whined about duplicate index entries, though our modern toolchain
doesn't. In any case, these GUCs surely are not about the
default settings of these values.
Peter Eisentraut [Sun, 8 Aug 2021 14:55:51 +0000 (16:55 +0200)]
Check the size in COPY_POINTER_FIELD
instead of making each caller do it.
Discussion: https://www.postgresql.org/message-id/flat/
c1097590-a6a4-486a-64b1-
e1f9cc0533ce@enterprisedb.com
Peter Eisentraut [Sun, 8 Aug 2021 14:55:51 +0000 (16:55 +0200)]
Change NestPath node to contain JoinPath node
This makes the structure of all JoinPath-derived nodes the same,
independent of whether they have additional fields.
Discussion: https://www.postgresql.org/message-id/flat/
c1097590-a6a4-486a-64b1-
e1f9cc0533ce@enterprisedb.com
Peter Eisentraut [Sun, 8 Aug 2021 14:55:51 +0000 (16:55 +0200)]
Change SeqScan node to contain Scan node
This makes the structure of all Scan-derived nodes the same,
independent of whether they have additional fields.
Discussion: https://www.postgresql.org/message-id/flat/
c1097590-a6a4-486a-64b1-
e1f9cc0533ce@enterprisedb.com
Tom Lane [Sun, 8 Aug 2021 15:56:29 +0000 (11:56 -0400)]
Rethink regexp engine's backref-related compilation state.
I had committer's remorse almost immediately after pushing
cb76fbd7e,
upon finding that removing capturing subexpressions' subREs from the
data structure broke my proposed patch for REG_NOSUB optimization.
Revert that data structure change. Instead, address the concern
about not changing capturing subREs' endpoints by not changing the
endpoints. We don't need to, because the point of that bit was just
to ensure that the atom has endpoints distinct from the outer state
pair that we're stringing the branch between. We already made
suitable states in the parenthesized-subexpression case, so the
additional ones were just useless overhead. This seems more
understandable than Spencer's original coding, and it ought to be
a shade faster too by saving a few state creations and arc changes.
(I actually see a couple percent improvement on Jacobson's web
corpus, though that's barely above the noise floor so I wouldn't
put much stock in that result.)
Also, fix the logic added by
ea1268f63 to ensure that the subRE
recorded in v->subs[subno] is exactly the one with capno == subno.
Spencer's original coding recorded the child subRE of the capture
node, which is okay so far as having the right endpoint states is
concerned, but as of
cb76fbd7e the capturing subRE itself always
has those endpoints too. I think the inconsistency is confusing
for the REG_NOSUB optimization.
As before, backpatch to v14.
Discussion: https://postgr.es/m/
0203588E-E609-43AF-9F4F-
902854231EE7@enterprisedb.com
David Rowley [Sun, 8 Aug 2021 11:27:57 +0000 (23:27 +1200)]
Remove unused function declaration
It appears that check_track_commit_timestamp was declared but has never
been defined in our code base. Likely this is just leftover cruft from
a development version of the original patch to add commit timestamps.
Let's just remove the useless declaration. The inclusion of guc.h also
seems surplus to requirements.
Author: Andrey Lepikhov
Discussion: https://postgr.es/m/
f49aefb5-edbb-633a-af07-
3e777023a94d@postgrespro.ru
Tom Lane [Sun, 8 Aug 2021 02:27:02 +0000 (22:27 -0400)]
Make regexp engine's backref-related compilation state more bulletproof.
Up to now, we remembered the definition of a capturing parenthesis
subexpression by storing a pointer to the associated subRE node.
That was okay before, because that subRE didn't get modified anymore
while parsing the rest of the regexp. However, in the wake of
commit
ea1268f63, that's no longer true: the outer invocation of
parseqatom() feels free to scribble on that subRE. This seems to
work anyway, because the states we jam into the child atom in the
"prepare a general-purpose state skeleton" stanza aren't really
semantically different from the original endpoints of the child atom.
But that would be mighty easy to break, and it's definitely not how
things worked before.
Between this and the issue fixed in the prior commit, it seems best
to get rid of this dependence on subRE nodes entirely. We don't need
the whole child subRE for future backrefs, only its starting and ending
NFA states; so let's just store pointers to those.
Also, in the corner case where we make an extra subRE to handle
immediately-nested capturing parentheses, it seems like it'd be smart
to have the extra subRE have the same begin/end states as the original
child subRE does (s/s2 not lp/rp). I think that linking it from lp to
rp might actually be semantically wrong, though since Spencer's original
code did it that way, I'm not totally certain. Using s/s2 is certainly
not wrong, in any case.
Per report from Mark Dilger. Back-patch to v14 where the problematic
patches came in.
Discussion: https://postgr.es/m/
0203588E-E609-43AF-9F4F-
902854231EE7@enterprisedb.com
Tom Lane [Sun, 8 Aug 2021 02:05:27 +0000 (22:05 -0400)]
Fix use-after-free issue in regexp engine.
Commit
cebc1d34e taught parseqatom() to optimize cases where a branch
contains only one, "messy", atom by getting rid of excess subRE nodes.
The way we really should do that is to keep the subRE built for the
"messy" child atom; but to avoid changing parseqatom's nominal API,
I made it delete that node after copying its fields to the outer subRE
made by parsebranch(). It seems that that actually worked at the time;
but it became dangerous after
ea1268f63, because that later commit
allowed the lower invocation of parse() to return a subRE that was also
pointed to by some v->subs[] entry. This meant we could wind up with a
dangling pointer in v->subs[], allowing a later backref to misbehave,
but only if that subRE struct had been reused in between. So the damage
seems confined to cases like '((...))...(...\2'.
To fix, do what I should have done before and modify parseqatom's API
to make it possible for it to remove the caller's subRE instead of the
callee's. That's safer because we know that subRE isn't complete yet,
so noplace else will have a pointer to it.
Per report from Mark Dilger. Back-patch to v14 where the problematic
patches came in.
Discussion: https://postgr.es/m/
0203588E-E609-43AF-9F4F-
902854231EE7@enterprisedb.com
Andres Freund [Sun, 8 Aug 2021 02:14:20 +0000 (19:14 -0700)]
Move temporary file cleanup to before_shmem_exit().
As reported by a few OSX buildfarm animals there exist at least one path where
temporary files exist during AtProcExit_Files() processing. As temporary file
cleanup causes pgstat reporting, the assertions added in
ee3f8d3d3ae caused
failures.
This is not an OSX specific issue, we were just lucky that timing on OSX
reliably triggered the problem. The known way to cause this is a FATAL error
during perform_base_backup() with a MANIFEST used - adding an elog(FATAL)
after InitializeBackupManifest() reliably reproduces the problem in isolation.
The problem is that the temporary file created in InitializeBackupManifest()
is not cleaned up via resource owner cleanup as WalSndResourceCleanup()
currently is only used for non-FATAL errors. That then allows to reach
AtProcExit_Files() with existing temporary files, causing the assertion
failure.
To fix this problem, move temporary file cleanup to a before_shmem_exit() hook
and add assertions ensuring that no temporary files are created before / after
temporary file management has been initialized / shut down. The cleanest way
to do so seems to be to split fd.c initialization into two, one for plain file
access and one for temporary file access.
Right now there's no need to perform further fd.c cleanup during process exit,
so I just renamed AtProcExit_Files() to BeforeShmemExit_Files(). Alternatively
we could perform another pass through the files to check that no temporary
files exist, but the added assertions seem to provide enough protection
against that.
It might turn out that the assertions added in
ee3f8d3d3ae will cause too much
noise - in that case we'll have to downgrade them to a WARNING, at least
temporarily.
This commit is not necessarily the best approach to address this issue, but it
should resolve the buildfarm failures. We can revise later.
Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/
20210807190131.2bm24acbebl4wl6i@alap3.anarazel.de
Peter Eisentraut [Sat, 7 Aug 2021 21:21:24 +0000 (23:21 +0200)]
Remove T_MemoryContext
This is an abstract node that shouldn't have a node tag defined.
Discussion: https://www.postgresql.org/message-id/flat/
c1097590-a6a4-486a-64b1-
e1f9cc0533ce@enterprisedb.com
Peter Eisentraut [Sat, 7 Aug 2021 18:34:49 +0000 (20:34 +0200)]
pg_amcheck: Message style improvements
Tom Lane [Sat, 7 Aug 2021 17:29:32 +0000 (13:29 -0400)]
Really fix the ambiguity in REFRESH MATERIALIZED VIEW CONCURRENTLY.
Rather than trying to pick table aliases that won't conflict with
any possible user-defined matview column name, adjust the queries'
syntax so that the aliases are only used in places where they can't be
mistaken for column names. Mostly this consists of writing "alias.*"
not just "alias", which adds clarity for humans as well as machines.
We do have the issue that "SELECT alias.*" acts differently from
"SELECT alias", but we can use the same hack ruleutils.c uses for
whole-row variables in SELECT lists: write "alias.*::compositetype".
We might as well revert to the original aliases after doing this;
they're a bit easier to read.
Like
75d66d10e, back-patch to all supported branches.
Discussion: https://postgr.es/m/
2488325.
1628261320@sss.pgh.pa.us
Peter Eisentraut [Sat, 7 Aug 2021 11:36:59 +0000 (13:36 +0200)]
pg_amcheck: Add missing translation markers
Peter Eisentraut [Sat, 7 Aug 2021 10:09:22 +0000 (12:09 +0200)]
Message style improvements
Andres Freund [Fri, 6 Aug 2021 17:05:57 +0000 (10:05 -0700)]
pgstat: Schedule per-backend pgstat shutdown via before_shmem_exit().
Previously on_shmem_exit() was used. The upcoming shared memory stats patch
uses DSM segments to store stats, which can not be used after the
dsm_backend_shutdown() call in shmem_exit().
The preceding commits were required to permit this change. This commit is
split off the shared memory stats patch to make it easier to isolate problems
caused by the ordering changes rather than the much larger changes in where
stats are stored.
Author: Andres Freund <andres@anarazel.de>
Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/
20210405092914.mmxqe7j56lsjfsej@alap3.anarazel.de
Discussion: https://postgr.es/m/
20210802164124.ufo5buo4apl6yuvs@alap3.anarazel.de
Discussion: https://postgr.es/m/
20210803023612.iziacxk5syn2r4ut@alap3.anarazel.de
Andres Freund [Fri, 26 Mar 2021 17:52:14 +0000 (10:52 -0700)]
Schedule ShutdownXLOG() in single user mode using before_shmem_exit().
Previously on_shmem_exit() was used. The upcoming shared memory stats patch
uses DSM segments to store stats, which can not be used after the
dsm_backend_shutdown() call in shmem_exit(). There does not seem to be any
reason to do ShutdownXLOG() via on_shmem_exit(), so change it.
Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/
20210405092914.mmxqe7j56lsjfsej@alap3.anarazel.de
Discussion: https://postgr.es/m/
20210803023612.iziacxk5syn2r4ut@alap3.anarazel.de
Andres Freund [Wed, 31 Mar 2021 07:50:28 +0000 (00:50 -0700)]
Make parallel worker shutdown complete entirely via before_shmem_exit().
This is a step toward storing stats in dynamic shared memory. As dynamic
shared memory segments are detached from just after before_shmem_exit()
callbacks are processed, but before on_shmem_exit() callbacks are, no stats
can be collected after before_shmem_exit() callbacks have been processed.
Parallel worker shutdown can cause stats to be emitted during DSM detach
callbacks, e.g. for SharedFileSet (which closes its files, which can causes
fd.c to emit stats about temporary files). Therefore parallel worker shutdown
needs to complete during the processing of before_shmem_exit callbacks.
One might think this problem could instead be solved by carefully ordering the
attaching to DSM segments, so that the pgstats segments get detached from
later than the parallel query ones. That turns out to not work because the
stats hash might need to grow which can cause new segments to be
allocated, which then will be detached from earlier.
There are two code changes:
First, call ParallelWorkerShutdown() via before_shmem_exit. That's a good idea
on its own, because other shutdown callbacks like ShutdownPostgres and
ShutdownAuxiliaryProcess are called via before_*.
Second, explicitly detach from the parallel query DSM segment, thereby
ensuring all stats are emitted during ParallelWorkerShutdown().
There are nicer solutions to these problems, but it's not obvious which of
those solutions is the correct one. As the shared memory stats work already is
a huge amount of work...
Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/
20210405092914.mmxqe7j56lsjfsej@alap3.anarazel.de
Discussion: https://postgr.es/m/
20210803023612.iziacxk5syn2r4ut@alap3.anarazel.de
Andres Freund [Sat, 7 Aug 2021 01:54:39 +0000 (18:54 -0700)]
pgstat: Bring up pgstat in BaseInit() to fix uninitialized use of pgstat by AV.
Previously pgstat_initialize() was called in InitPostgres() and
AuxiliaryProcessMain(). As it turns out there was at least one case where we
reported stats before pgstat_initialize() was called, see
AutoVacWorkerMain()'s intentionally early call to pgstat_report_autovac().
This turns out to not be a problem with the current pgstat implementation as
pgstat_initialize() only registers a shutdown callback. But in the shared
memory based stats implementation we are working towards pgstat_initialize()
has to do more work.
After
b406478b87e BaseInit() is a central place where initialization shared by
normal backends and auxiliary backends can be put. Obviously BaseInit() is
called before InitPostgres() registers ShutdownPostgres. Previously
ShutdownPostgres was the first before_shmem_exit callback, now that's commonly
pgstats. That should be fine.
Previously pgstat_initialize() was not called in bootstrap mode, but there
does not appear to be a need for that. It's now done unconditionally.
To detect future issues like this, assertions are added to a few places
verifying that the pgstat subsystem is initialized and not yet shut down.
Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/
20210405092914.mmxqe7j56lsjfsej@alap3.anarazel.de
Discussion: https://postgr.es/m/
20210802164124.ufo5buo4apl6yuvs@alap3.anarazel.de
Tom Lane [Fri, 6 Aug 2021 21:32:54 +0000 (17:32 -0400)]
Don't elide casting to typmod -1.
Casting a value that's already of a type with a specific typmod
to an unspecified typmod doesn't do anything so far as run-time
behavior is concerned. However, it really ought to change the
exposed type of the expression to match. Up to now,
coerce_type_typmod hasn't bothered with that, which creates gotchas
in contexts such as recursive unions. If for example one side of
the union is numeric(18,3), but it needs to be plain numeric to
match the other side, there's no direct way to express that.
This is easy enough to fix, by inserting a RelabelType to update the
exposed type of the expression. However, it's a bit nervous-making
to change this behavior, because it's stood for a really long time.
(I strongly suspect that it's like this in part because the logic
pre-dates the introduction of RelabelType in 7.0. The commit log
message for
57b30e8e2 is interesting reading here.) As a compromise,
we'll sneak the change into 14beta3, and consider back-patching to
stable branches if no complaints emerge in the next three months.
Discussion: https://postgr.es/m/CABNQVagu3bZGqiTjb31a8D5Od3fUMs7Oh3gmZMQZVHZ=uWWWfQ@mail.gmail.com
Dean Rasheed [Fri, 6 Aug 2021 20:29:15 +0000 (21:29 +0100)]
Adjust the integer overflow tests in the numeric code.
Formerly, the numeric code tested whether an integer value of a larger
type would fit in a smaller type by casting it to the smaller type and
then testing if the reverse conversion produced the original value.
That's perfectly fine, except that it caused a test failure on
buildfarm animal castoroides, most likely due to a compiler bug.
Instead, do these tests by comparing against PG_INT16/32_MIN/MAX. That
matches existing code in other places, such as int84(), which is more
widely tested, and so is less likely to go wrong.
While at it, add regression tests covering the numeric-to-int8/4/2
conversions, and adjust the recently added tests to the style of
434ddfb79a (on the v11 branch) to make failures easier to diagnose.
Per buildfarm via Tom Lane, reviewed by Tom Lane.
Discussion: https://postgr.es/m/
2394813.
1628179479%40sss.pgh.pa.us
Peter Eisentraut [Fri, 6 Aug 2021 20:11:02 +0000 (22:11 +0200)]
Add missing message punctuation
Peter Eisentraut [Fri, 6 Aug 2021 18:55:59 +0000 (20:55 +0200)]
Fix wording
Andres Freund [Thu, 5 Aug 2021 21:37:09 +0000 (14:37 -0700)]
process startup: Always call Init[Auxiliary]Process() before BaseInit().
For EXEC_BACKEND InitProcess()/InitAuxiliaryProcess() needs to have been
called well before we call BaseInit(), as SubPostmasterMain() needs LWLocks to
work. Having the order of initialization differ between platforms makes it
unnecessarily hard to understand the system and to add initialization points
for new subsystems without a lot of duplication.
To be able to change the order, BaseInit() cannot trigger
CreateSharedMemoryAndSemaphores() anymore - obviously that needs to have
happened before we can call InitProcess(). It seems cleaner to create shared
memory explicitly in single user/bootstrap mode anyway.
After this change the separation of bufmgr initialization into
InitBufferPoolAccess() / InitBufferPoolBackend() is not meaningful anymore so
the latter is removed.
Author: Andres Freund <andres@anarazel.de>
Reviewed-By: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/
20210802164124.ufo5buo4apl6yuvs@alap3.anarazel.de
Andres Freund [Thu, 5 Aug 2021 22:31:29 +0000 (15:31 -0700)]
Call pgwin32_signal_initialize() in postmaster as well.
This was an oversight in
07bf3785099. While it does reduce the benefit of the
simplification due to that commit, it still seems like a win to me.
It seems like it might be a good idea to have a function mirroring
InitPostmasterChild() / InitStandaloneProcess() for postmaster in miscinit.c
to make it easier to keep initialization between the three possible
environment in sync.
Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/
20210805214109.lzfk3r3ae37bahmv@alap3.anarazel.de
Andres Freund [Thu, 5 Aug 2021 19:36:06 +0000 (12:36 -0700)]
process startup: Centralize pgwin32_signal_initialize() calls.
For one, the existing location lead to somewhat awkward code in main(). For
another, the new location is easier to understand anyway.
Author: Andres Freund <andres@anarazel.de>
Reviewed-By: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-By: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/
20210802164124.ufo5buo4apl6yuvs@alap3.anarazel.de
Andres Freund [Thu, 5 Aug 2021 19:17:31 +0000 (12:17 -0700)]
process startup: Remove bootstrap / checker modes from AuxProcType.
Neither is actually initialized as an auxiliary process, so it does not really
make sense to reserve a PGPROC etc for them.
This keeps checker mode implemented by exiting partway through bootstrap
mode. That might be worth changing at some point, perhaps if we ever extend
checker mode to be a more general tool.
Author: Andres Freund <andres@anarazel.de>
Reviewed-By: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-By: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/
20210802164124.ufo5buo4apl6yuvs@alap3.anarazel.de
Andres Freund [Thu, 5 Aug 2021 19:09:19 +0000 (12:09 -0700)]
process startup: Move AuxiliaryProcessMain into its own file.
After the preceding commits the auxprocess code is independent from
bootstrap.c - so a dedicated file seems less confusing.
Author: Andres Freund <andres@anarazel.de>
Reviewed-By: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-By: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/
20210802164124.ufo5buo4apl6yuvs@alap3.anarazel.de
Andres Freund [Thu, 5 Aug 2021 02:30:12 +0000 (19:30 -0700)]
process startup: auxprocess: reindent block
Kept separate for ease of review, particularly because pgindent insists on
reflowing a few comments.
Author: Andres Freund <andres@anarazel.de>
Reviewed-By: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-By: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/
20210802164124.ufo5buo4apl6yuvs@alap3.anarazel.de
Andres Freund [Thu, 5 Aug 2021 02:29:45 +0000 (19:29 -0700)]
process startup: Separate out BootstrapModeMain from AuxiliaryProcessMain.
There practically was no shared code between the two, once all the ifs are
removed. And it was quite confusing that aux processes weren't actually
started by the call to AuxiliaryProcessMain() in main().
There's more to do, AuxiliaryProcessMain() should move out of bootstrap.c, and
BootstrapModeMain() shouldn't use/be part of AuxProcType.
Author: Andres Freund <andres@anarazel.de>
Reviewed-By: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-By: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/
20210802164124.ufo5buo4apl6yuvs@alap3.anarazel.de
Andres Freund [Thu, 5 Aug 2021 02:28:46 +0000 (19:28 -0700)]
process startup: Rename postmaster's --forkboot to --forkaux.
It is confusing that aux processes are started with --forkboot, given that
bootstrap mode cannot run below postmaster.
Author: Andres Freund <andres@anarazel.de>
Reviewed-By: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-By: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/
20210802164124.ufo5buo4apl6yuvs@alap3.anarazel.de
Etsuro Fujita [Thu, 5 Aug 2021 11:00:00 +0000 (20:00 +0900)]
postgres_fdw: Fix issues with generated columns in foreign tables.
postgres_fdw imported generated columns from the remote tables as plain
columns, and caused failures like "ERROR: cannot insert a non-DEFAULT
value into column "foo"" when inserting into the foreign tables, as it
tried to insert values into the generated columns. To fix, we do the
following under the assumption that generated columns in a postgres_fdw
foreign table are defined so that they represent generated columns in
the underlying remote table:
* Send DEFAULT for the generated columns to the foreign server on insert
or update, not generated column values computed on the local server.
* Add to postgresImportForeignSchema() an option "import_generated" to
include column generated expressions in the definitions of foreign
tables imported from a foreign server. The option is true by default.
The assumption seems reasonable, because that would make a query of the
postgres_fdw foreign table return values for the generated columns that
are consistent with the generated expression.
While here, fix another issue in postgresImportForeignSchema(): it tried
to include column generated expressions as column default expressions in
the foreign table definitions when the import_default option was enabled.
Per bug #16631 from Daniel Cherniy. Back-patch to v12 where generated
columns were added.
Discussion: https://postgr.es/m/16631-
e929fe9db0ffc7cf%40postgresql.org
Fujii Masao [Thu, 5 Aug 2021 08:49:51 +0000 (17:49 +0900)]
Remove unused argument "txn" in maybe_send_schema().
Commit
464824323e added the argument "txn" into maybe_send_schema()
though it was not used.
Author: Hou Zhijie
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/OS0PR01MB5716146E43928FB92D45D29794EC9@OS0PR01MB5716.jpnprd01.prod.outlook.com
Dean Rasheed [Thu, 5 Aug 2021 08:24:11 +0000 (09:24 +0100)]
Fix division-by-zero error in to_char() with 'EEEE' format.
This fixes a long-standing bug when using to_char() to format a
numeric value in scientific notation -- if the value's exponent is
less than -NUMERIC_MAX_DISPLAY_SCALE-1 (-1001), it produced a
division-by-zero error.
The reason for this error was that get_str_from_var_sci() divides its
input by 10^exp, which it produced using power_var_int(). However, the
underflow test in power_var_int() causes it to return zero if the
result scale is too small. That's not a problem for power_var_int()'s
only other caller, power_var(), since that limits the rscale to 1000,
but in get_str_from_var_sci() the exponent can be much smaller,
requiring a much larger rscale. Fix by introducing a new function to
compute 10^exp directly, with no rscale limit. This also allows 10^exp
to be computed more efficiently, without any numeric multiplication,
division or rounding.
Discussion: https://postgr.es/m/CAEZATCWhojfH4whaqgUKBe8D5jNHB8ytzemL-PnRx+KCTyMXmg@mail.gmail.com
Andres Freund [Thu, 5 Aug 2021 02:19:44 +0000 (19:19 -0700)]
pgbench: When using pipelining only do PQconsumeInput() when necessary.
Up to now we did a PQconsumeInput() for each pipelined query, asking the OS
for more input - which it often won't have, as all results might already have
been sent. That turns out to have a noticeable performance impact.
Alvaro Herrera reviewed the idea to add the PQisBusy() check, but not this
concrete patch.
Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/
20210720180039.23rivhdft3l4mayn@alap3.anarazel.de
Backpatch: 14, where libpq/pgbench pipelining was introduced.
Andres Freund [Thu, 5 Aug 2021 02:16:04 +0000 (19:16 -0700)]
pgstat: split reporting/fetching of bgwriter and checkpointer stats.
These have been unrelated since bgwriter and checkpointer were split into two
processes in
806a2aee379. As there several pending patches (shared memory
stats, extending the set of tracked IO / buffer statistics) that are made a
bit more awkward by the grouping, split them. Done separately to make
reviewing easier.
This does *not* change the contents of pg_stat_bgwriter or move fields out of
bgwriter/checkpointer stats that arguably do not belong in either. However
pgstat_fetch_global() was renamed and split into
pgstat_fetch_stat_checkpointer() and pgstat_fetch_stat_bgwriter().
Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/
20210405092914.mmxqe7j56lsjfsej@alap3.anarazel.de
Peter Geoghegan [Wed, 4 Aug 2021 04:53:41 +0000 (21:53 -0700)]
Make vacuum_index_cleanup reloption RELOPT_TYPE_ENUM.
Oversight in commit
3499df0d, which generalized the reloption as a way
of giving users a way to consistently avoid VACUUM's index bypass
optimization.
Per off-list report from Nikolay Shaplov.
Backpatch: 14-, where index cleanup reloption was extended.
Amit Kapila [Wed, 4 Aug 2021 02:17:06 +0000 (07:47 +0530)]
Add prepare API support for streaming transactions in logical replication.
Commit
a8fd13cab0 added support for prepared transactions to built-in
logical replication via a new option "two_phase" for a subscription. The
"two_phase" option was not allowed with the existing streaming option.
This commit permits the combination of "streaming" and "two_phase"
subscription options. It extends the pgoutput plugin and the subscriber
side code to add the prepare API for streaming transactions which will
apply the changes accumulated in the spool-file at prepare time.
Author: Peter Smith and Ajin Cherian
Reviewed-by: Vignesh C, Amit Kapila, Greg Nancarrow
Tested-By: Haiying Tang
Discussion: https://postgr.es/m/
02DA5F5E-CECE-4D9C-8B4B-
418077E2C010@postgrespro.ru
Discussion: https://postgr.es/m/CAMGcDxeqEpWj3fTXwqhSwBdXd2RS9jzwWscO-XbeCfso6ts3+Q@mail.gmail.com
Tom Lane [Tue, 3 Aug 2021 17:08:49 +0000 (13:08 -0400)]
Add assorted new regexp_xxx SQL functions.
This patch adds new functions regexp_count(), regexp_instr(),
regexp_like(), and regexp_substr(), and extends regexp_replace()
with some new optional arguments. All these functions follow
the definitions used in Oracle, although there are small differences
in the regexp language due to using our own regexp engine -- most
notably, that the default newline-matching behavior is different.
Similar functions appear in DB2 and elsewhere, too. Aside from
easing portability, these functions are easier to use for certain
tasks than our existing regexp_match[es] functions.
Gilles Darold, heavily revised by me
Discussion: https://postgr.es/m/
fc160ee0-c843-b024-29bb-
97b5da61971f@darold.net
Bruce Momjian [Tue, 3 Aug 2021 16:26:08 +0000 (12:26 -0400)]
C comment: correct heading of extension query
Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/
20210803161345.GZ12533@telsasoft.com
Backpatch-through: 9.6
Bruce Momjian [Tue, 3 Aug 2021 16:10:29 +0000 (12:10 -0400)]
interval: round values when spilling to months
Previously spilled units greater than months were truncated to months.
Also document the spill behavior.
Reported-by: Bryn Llewelly
Discussion: https://postgr.es/m/
BDAE4B56-3337-45A2-AC8A-
30593849D6C0@yugabyte.com
Backpatch-through: master
Bruce Momjian [Tue, 3 Aug 2021 15:58:15 +0000 (11:58 -0400)]
pg_upgrade: warn about extensions that need updating
Also create a script that can be run to update them.
Reported-by: Dave Cramer
Discussion: https://postgr.es/m/CADK3HHKawwbOcGwMGnDuAf3-U8YfvTcS8jqDv3UM=niijs3MMA@mail.gmail.com
Backpatch-through: 9.6
Bruce Momjian [Tue, 3 Aug 2021 15:27:33 +0000 (11:27 -0400)]
pg_upgrade: improve docs about extension upgrades
The previous wording was unclear about the steps needed to upgrade
extensions, and how to update them after pg_upgrade.
Reported-by: Dave Cramer
Discussion: https://postgr.es/m/CADK3HHKawwbOcGwMGnDuAf3-U8YfvTcS8jqDv3UM=niijs3MMA@mail.gmail.com
Backpatch-through: 9.6
Bruce Momjian [Tue, 3 Aug 2021 15:11:51 +0000 (11:11 -0400)]
doc: mention inheritance's tableoid can be used in partitioning
Previously tableoid was not mentioned in the partition doc section. We
only had a link to the "all the normal rules" of inheritance section.
Reported-by: michal.palenik@freemap.sk
Discussion: https://postgr.es/m/
162627031219.693.
11508199541771263335@wrigleys.postgresql.org
Backpatch-through: 10
Bruce Momjian [Tue, 3 Aug 2021 14:57:32 +0000 (10:57 -0400)]
doc: add example of using pg_dump with GNU split and gzip
This is only possible with GNU split, not other versions like BSD split.
Reported-by: jim@jdoherty.net
Discussion: https://postgr.es/m/
162653459215.701.
6323855956817776386@wrigleys.postgresql.org
Backpatch-through: 9.6
Thomas Munro [Tue, 3 Aug 2021 02:11:55 +0000 (14:11 +1200)]
Further simplify a bit of logic in StartupXLOG().
Commit
7ff23c6d277d1d90478a51f0dd81414d343f3850 left us with two
identical cases. Collapse them.
Author: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKGJ8NRsqgkZEnsnRc2MFROBV-jCnacbYvtpptK2A9YYp9Q%40mail.gmail.com
David Rowley [Tue, 3 Aug 2021 00:25:52 +0000 (12:25 +1200)]
Allow ordered partition scans in more cases
959d00e9d added the ability to make use of an Append node instead of a
MergeAppend when we wanted to perform a scan of a partitioned table and
the required sort order was the same as the partitioned keys and the
partitioned table was defined in such a way that earlier partitions were
guaranteed to only contain lower-order values than later partitions.
However, previously we didn't allow these ordered partition scans for
LIST partitioned table when there were any partitions that allowed
multiple Datums. This was a very cheap check to make and we could likely
have done a little better by checking if there were interleaved
partitions, but at the time we didn't have visibility about which
partitions were pruned, so we still may have disallowed cases where all
interleaved partitions were pruned.
Since
475dbd0b7, we now have knowledge of pruned partitions, we can do a
much better job inside partitions_are_ordered().
Here we pass which partitions survived partition pruning into
partitions_are_ordered() and, for LIST partitioning, have it check to see
if any live partitions exist that are also in the new "interleaved_parts"
field defined in PartitionBoundInfo.
For RANGE partitioning we can relax the code which caused the partitions
to be unordered if a DEFAULT partition existed. Since we now know which
partitions were pruned, partitions_are_ordered() now returns true when the
DEFAULT partition was pruned.
Reviewed-by: Amit Langote, Zhihong Yu
Discussion: https://postgr.es/m/CAApHDvrdoN_sXU52i=QDXe2k3WAo=EVry29r2+Tq2WYcn2xhEA@mail.gmail.com
David Rowley [Mon, 2 Aug 2021 23:47:24 +0000 (11:47 +1200)]
Track a Bitmapset of non-pruned partitions in RelOptInfo
For partitioned tables with large numbers of partitions where queries are
able to prune all but a very small number of partitions, the time spent in
the planner looping over RelOptInfo.part_rels checking for non-NULL
RelOptInfos could become a large portion of the overall planning time.
Here we add a Bitmapset that records the non-pruned partitions. This
allows us to more efficiently skip the pruned partitions by looping over
the Bitmapset.
This will cause a very slight slow down in cases where no or not many
partitions could be pruned, however, those cases are already slow to plan
anyway and the overhead of looping over the Bitmapset would be
unmeasurable when compared with the other tasks such as path creation for
a large number of partitions.
Reviewed-by: Amit Langote, Zhihong Yu
Discussion: https://postgr.es/m/CAApHDvqnPx6JnUuPwaf5ao38zczrAb9mxt9gj4U1EKFfd4AqLA@mail.gmail.com
Tom Lane [Mon, 2 Aug 2021 15:32:17 +0000 (11:32 -0400)]
Doc: minor improvements for logical replication protocol documentation.
Where appropriate, annotate message field data types with the backend
code's name for the data type, eg XLogRecPtr or TimestampTz. Previously
we just said "Int64" which didn't provide as much info.
Also clarify references to object OIDs, and make use of the existing
convention to denote the value of a field that must have a fixed value.
Vignesh C, reviewed by Peter Smith and Euler Taveira.
Discussion: https://postgr.es/m/CALDaNm0+fatx57KFcBopUZWQpH_tz3WKKfm-_eiTwcXui5BnhQ@mail.gmail.com
Thomas Munro [Mon, 2 Aug 2021 05:32:20 +0000 (17:32 +1200)]
Run checkpointer and bgwriter in crash recovery.
Start up the checkpointer and bgwriter during crash recovery (except in
--single mode), as we do for replication. This wasn't done back in
commit
cdd46c76 out of caution. Now it seems like a better idea to make
the environment as similar as possible in both cases. There may also be
some performance advantages.
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Reviewed-by: Aleksander Alekseev <aleksander@timescale.com>
Tested-by: Jakub Wartak <Jakub.Wartak@tomtom.com>
Discussion: https://postgr.es/m/CA%2BhUKGJ8NRsqgkZEnsnRc2MFROBV-jCnacbYvtpptK2A9YYp9Q%40mail.gmail.com
Andres Freund [Mon, 2 Aug 2021 04:10:11 +0000 (21:10 -0700)]
Remove misplaced comment from AuxiliaryProcessMain().
The comment didn't make sense anymore since at least
626eb021988. As it didn't
actually explain anything anyway, just remove it.
Author: Andres Freund <andres@anarazel.de>
Etsuro Fujita [Mon, 2 Aug 2021 03:45:00 +0000 (12:45 +0900)]
Fix oversight in commit
1ec7fca8592178281cd5cdada0f27a340fb813fc.
I failed to account for the possibility that when
ExecAppendAsyncEventWait() notifies multiple async-capable nodes using
postgres_fdw, a preceding node might invoke process_pending_request() to
process a pending asynchronous request made by a succeeding node. In
that case the succeeding node should produce a tuple to return to the
parent Append node from tuples fetched by process_pending_request() when
notified. Repair.
Per buildfarm via Michael Paquier. Back-patch to v14, like the previous
commit.
Thanks to Tom Lane for testing.
Discussion: https://postgr.es/m/YQP0UPT8KmPiHTMs%40paquier.xyz
Amit Kapila [Mon, 2 Aug 2021 02:59:26 +0000 (08:29 +0530)]
Fix test failure in 021_twophase.pl.
The test is expecting two prepared transactions corresponding to two
subscriptions but it waits to catch up for just one subscription. Fix it
by allowing to wait for both subscriptions.
Reported-by: Michael Paquier, as per buildfarm
Author: Ajin Cherian
Reviewed-By: Amit Kapila, Vignesh C, Peter Smith
Discussion: https://postgr.es/m/CAA4eK1+_0iNQ8Z=KVTjmmAqNX-hyv+1+fnZ-Yx8CVP=uAcekqw@mail.gmail.com
Andrew Dunstan [Sun, 1 Aug 2021 17:03:15 +0000 (13:03 -0400)]
Silence perl warning about uninitialized value
Tom Lane [Sat, 31 Jul 2021 19:35:27 +0000 (15:35 -0400)]
Doc: alphabetize the regexp_foo() function descriptions in 9.7.3.
For no visible reason (other than historical accident no doubt),
regexp_replace() was out of order. Re-order to match the way
that these functions are listed in 9.4. (That means substring()
remains first, because it's SQL-standard and the rest aren't.)
I've not touched the text other than to move it. This is just
to reduce confusion in the diffs for upcoming additions.
Tom Lane [Sat, 31 Jul 2021 15:50:14 +0000 (11:50 -0400)]
Use elog, not Assert, to report failure to provide an outer snapshot.
As of commit
84f5c2908, executing SQL commands (via SPI or otherwise)
requires having either an active Portal, or a caller-established
active snapshot. We were simply Assert'ing that that's the case.
But we've now had a couple different reports of people testing
extensions that didn't meet this requirement, and were confused by
the resulting crash. Let's convert the Assert to a test-and-elog,
in hopes of making the issue clearer for extension authors.
Per gripes from Liu Huailing and RekGRpth. Back-patch to v11,
like the prior commit.
Discussion: https://postgr.es/m/OSZPR01MB6215671E3C5956A034A080DFBEEC9@OSZPR01MB6215.jpnprd01.prod.outlook.com
Discussion: https://postgr.es/m/17035-
14607d308ac8643c@postgresql.org
John Naylor [Sat, 31 Jul 2021 11:25:27 +0000 (07:25 -0400)]
Remove redundant setting of pg_attribute.attcompression
Since
e6241d8e0, no attribute needs a non-default value of this during
initdb, so let the usual machinery for defaults take care of it.
Dean Rasheed [Sat, 31 Jul 2021 10:21:44 +0000 (11:21 +0100)]
Fix corner-case errors and loss of precision in numeric_power().
This fixes a couple of related problems that arise when raising
numbers to very large powers.
Firstly, when raising a negative number to a very large integer power,
the result should be well-defined, but the previous code would only
cope if the exponent was small enough to go through power_var_int().
Otherwise it would throw an internal error, attempting to take the
logarithm of a negative number. Fix this by adding suitable handling
to the general case in power_var() to cope with negative bases,
checking for integer powers there.
Next, when raising a (positive or negative) number whose absolute
value is slightly less than 1 to a very large power, the result should
approach zero as the power is increased. However, in some cases, for
sufficiently large powers, this would lose all precision and return 1
instead of 0. This was due to the way that the local_rscale was being
calculated for the final full-precision calculation:
local_rscale = rscale + (int) val - ln_dweight + 8
The first two terms on the right hand side are meant to give the
number of significant digits required in the result ("val" being the
estimated result weight). However, this failed to account for the fact
that rscale is clipped to a maximum of NUMERIC_MAX_DISPLAY_SCALE
(1000), and the result weight might be less then -1000, causing their
sum to be negative, leading to a loss of precision. Fix this by
forcing the number of significant digits calculated to be nonnegative.
It's OK for it to be zero (when the result weight is less than -1000),
since the local_rscale value then includes a few extra digits to
ensure an accurate result.
Finally, add additional underflow checks to exp_var() and power_var(),
so that they consistently return zero for cases like this where the
result is indistinguishable from zero. Some paths through this code
already returned zero in such cases, but others were throwing overflow
errors.
Dean Rasheed, reviewed by Yugo Nagata.
Discussion: http://postgr.es/m/CAEZATCW6Dvq7+3wN3tt5jLj-FyOcUgT5xNoOqce5=6Su0bCR0w@mail.gmail.com
Heikki Linnakangas [Sat, 31 Jul 2021 06:50:26 +0000 (09:50 +0300)]
Move InRecovery and standbyState global vars to xlogutils.c.
They are used in code that runs both during normal operation and during
WAL replay, and needs to behave differently during replay. Move them to
xlogutils.c, because that's where we have other helper functions used by
redo routines.
Reviewed-by: Andres Freund
Discussion: https://www.postgresql.org/message-id/
b3b71061-4919-e882-4857-
27e370ab134a%40iki.fi
Heikki Linnakangas [Sat, 31 Jul 2021 06:49:30 +0000 (09:49 +0300)]
Extract code to describe recovery stop reason to a function.
StartupXLOG() is very long, this makes it a little bit more readable.
Reviewed-by: Andres Freund
Discussion: https://www.postgresql.org/message-id/
b3b71061-4919-e882-4857-
27e370ab134a%40iki.fi
Heikki Linnakangas [Sat, 31 Jul 2021 06:38:32 +0000 (09:38 +0300)]
Remove unnecessary 'restoredFromArchive' global variable.
It might've been useful for debugging purposes, but meh. There's
'readSource' which does almost the same thing.
Reviewed-by: Andres Freund
Discussion: https://www.postgresql.org/message-id/
b3b71061-4919-e882-4857-
27e370ab134a%40iki.fi
Heikki Linnakangas [Sat, 31 Jul 2021 06:36:11 +0000 (09:36 +0300)]
Don't use O_SYNC or similar when opening signal file to fsync it.
No need to use get_sync_bit() when we're calling pg_fsync() on the file.
We're not writing to the files, so it doesn't make any difference in
practice, but seems less surprising this way.
Reviewed-by: Andres Freund
Discussion: https://www.postgresql.org/message-id/
b3b71061-4919-e882-4857-
27e370ab134a%40iki.fi
Michael Paquier [Sat, 31 Jul 2021 01:13:15 +0000 (10:13 +0900)]
Enable TAP tests of pg_receivewal for ZLIB on Windows, take three
This reverts commit
6a2c532. fairywren and bowerbird failed those tests
because of incorrect versions of ZLIB linked to, causing errors like
SIGBREAKs that stopped buildfarm runs or EACCES failures when writing
compressed WAL segments.
Andrew Dunstan has done all the investigation here, so he deserves all
the credit for being able to enable those tests on Windows.
Discussion: https://postgr.es/m/
9040d5ed-6462-66a4-07ac-
2923785ae563@dunslane.net
Jeff Davis [Fri, 30 Jul 2021 21:59:19 +0000 (14:59 -0700)]
Improve documentation for START_REPLICATION ... LOGICAL.
The starting point may not be exactly what the client requested; it
may be at the slot's confirmed_flush_lsn.
Also, upgrade the message from DEBUG1 to LOG when this happens.
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/
c5c861d576f2511732f8002c76245da587110b1c.camel%40j-davis.com
John Naylor [Fri, 30 Jul 2021 17:50:23 +0000 (13:50 -0400)]
Fix range check in ECPG numeric to int conversion
The previous coding guarded against -INT_MAX instead of INT_MIN,
leading to -
2147483648 being rejected as out of range.
Per bug #17128 from Kevin Sweet
Discussion: https://www.postgresql.org/message-id/flat/17128-
55a8a879727a3e3a%40postgresql.org
Reviewed-by: Tom Lane
Backpatch to all supported branches
Tom Lane [Fri, 30 Jul 2021 18:50:21 +0000 (14:50 -0400)]
Doc: add a glossary entry for "domain".
Anton Voloshin and Jürgen Purtz, reviewed by Laurenz Albe
Discussion: https://postgr.es/m/
2ea65bdf-1380-f088-02bd-
ff1a31ed265c@postgrespro.ru
Tom Lane [Fri, 30 Jul 2021 17:39:48 +0000 (13:39 -0400)]
In postgres_fdw, allow CASE expressions to be pushed to the remote server.
This is simple enough except for the need to check whether CaseTestExpr
nodes have a collation that is not derived from a remote Var. For that,
examine the CASE's "arg" expression and then pass that info down into the
recursive examination of the WHEN expressions.
Alexander Pyhalov, reviewed by Gilles Darold and myself
Discussion: https://postgr.es/m/
fda09032e90d85d9b726a41e03f9097f@postgrespro.ru
Robert Haas [Fri, 30 Jul 2021 12:33:33 +0000 (08:33 -0400)]
Remove unnecessary call to ReadCheckpointRecord().
It should always be the case that the last checkpoint record is still
readable, because otherwise, a crash would leave us in a situation
from which we can't recover. Therefore the test removed by this patch
should always succeed. For it to fail, either there has to be a serious
bug in the code someplace, or the user has to be manually modifying
pg_wal while crash recovery is running. If it's the first one, we
should fix the bug. If it's the second one, they should stop, or
anyway they're doing so at their own risk. In neither case does
a full checkpoint instead of an end-of-recovery record seem like a
clear winner. Furthermore, rarely-taken code paths are particularly
vulnerable to bugs, so let's simplify by getting rid of this one.
Discussion: http://postgr.es/m/CA+TgmoYmw==TOJ6EzYb_vcjyS09NkzrVKSyBKUUyo1zBEaJASA@mail.gmail.com
Michael Paquier [Fri, 30 Jul 2021 12:28:03 +0000 (21:28 +0900)]
Use --no-loop for new calls of pg_receivewal --endpos in TAP tests
Those tests are not designed to fail, but if they do, like on some cases
for Windows because of ZLIB (?), they could remain stuck. Using
--no-loop makes the test fail immediately. The oldest test with
--endpos already did that.
Those tests have been added in
ffc9dda.
Reviewed-by: Andrew Dunstan
Discussion: https://postgr.es/m/
ec093ff1-a53c-0091-46a2-
4537354b0dd4@dunslane.net
Heikki Linnakangas [Fri, 30 Jul 2021 09:52:44 +0000 (12:52 +0300)]
Update obsolete comment that still referred to CheckpointLock
CheckpointLock was removed in commit
d18e75664a, and commit
ce197e91d0
updated a leftover comment in CreateCheckPoint, but there was another
copy of it in CreateRestartPoint still.
Etsuro Fujita [Fri, 30 Jul 2021 08:00:00 +0000 (17:00 +0900)]
postgres_fdw: Fix handling of pending asynchronous requests.
A pending asynchronous request is handled by process_pending_request(),
which previously not only processed an in-progress remote query but
performed ExecForeignScan() to produce a tuple to return to the local
server asynchronously from the result of the remote query. But that led
to a server crash when executing a query or led to an "InstrStartNode
called twice in a row" or "InstrEndLoop called on running node" failure
when doing EXPLAIN ANALYZE of it, in cases where the plan tree for it
contained multiple async-capable nodes accessing the same
initplan/subplan that contained multiple async-capable nodes scanning
the same foreign tables as for the parent async-capable nodes, as
reported by Andrey Lepikhov. The reason is that the second step in
process_pending_request() invoked when executing the initplan/subplan
for one of the parent async-capable nodes caused recursive execution of
the initplan/subplan for another of the parent async-capable nodes.
To fix, split process_pending_request() into the two steps and postpone
the second step until ForeignAsyncConfigureWait() is called for each of
the pending asynchronous requests. Also, in ExecAppendAsyncEventWait()
we assumed that FDWs would register at least one wait event in a
WaitEventSet created there when they were called from
ForeignAsyncConfigureWait() in that function, but allow FDWs to register
zero wait events in the WaitEventSet; modify ExecAppendAsyncEventWait()
to just return in that case.
Oversight in commit
27e1f1456. Back-patch to v14 where that commit went
in.
Andrey Lepikhov and Etsuro Fujita
Discussion: https://postgr.es/m/
fe5eaa19-1704-e4a4-76ee-
3b9d37ade399@postgrespro.ru
Amit Kapila [Fri, 30 Jul 2021 02:47:38 +0000 (08:17 +0530)]
Remove unused argument in apply_handle_commit_internal().
Oversight in commit
0926e96c49.
Author: Masahiko Sawada
Reviewed-By: Amit Kapila
Backpatch-through: 14, where it was introduced
Discussion: https://postgr.es/m/CAD21AoDeScrsHhLyEPYqN3sydg6PxAPVBboK=30xJfUVihNZDA@mail.gmail.com
Alvaro Herrera [Thu, 29 Jul 2021 21:09:06 +0000 (17:09 -0400)]
Close yet another race condition in replication slot test code
Buildfarm shows that this test has a further failure mode when a
checkpoint starts earlier than expected, so we detect a "checkpoint
completed" line that's not the one we want. Change the config to try
and prevent this.
Per buildfarm
While at it, update one comment that was forgotten in commit
d18e75664a2f.
Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/
20210729.162038.
534808353849568395.horikyota.ntt@gmail.com
Daniel Gustafsson [Thu, 29 Jul 2021 19:39:40 +0000 (21:39 +0200)]
docs: Fix bit_count example output
The returnvalue for the bit_count(::bytea) example was assuming a
non-default value of standard_conforming_strings. This was fixed
in the tests in commit
ebedd0c78.
Author: wangzk.fnstxz@fujitsu.com
Discussion: https://postgr.es/m/OSZPR01MB6551FFAC1088C82C3D799BE0FAEB9@OSZPR01MB6551.jpnprd01.prod.outlook.com
Backpatch-through: 14
Tom Lane [Thu, 29 Jul 2021 17:33:31 +0000 (13:33 -0400)]
Improve libpq's handling of OOM during error message construction.
Commit
ffa2e4670 changed libpq so that multiple error reports
occurring during one operation (a connection attempt or query)
are accumulated in conn->errorMessage, where before new ones
usually replaced any prior error. At least in theory, that makes
us more vulnerable to running out of memory for the errorMessage
buffer. If it did happen, the user would be left with just an
empty-string error report, which is pretty unhelpful.
We can improve this by relying on pqexpbuffer.c's existing "broken
buffer" convention to track whether we've hit OOM for the current
operation's error string, and then substituting a constant "out of
memory" string in the small number of places where the errorMessage
is read out.
While at it, apply the same method to similar OOM cases in
pqInternalNotice and pqGetErrorNotice3.
Back-patch to v14 where
ffa2e4670 came in. In principle this could
go back further; but in view of the lack of field reports, the
hazard seems negligible in older branches.
Discussion: https://postgr.es/m/530153.
1627425648@sss.pgh.pa.us
Andrew Dunstan [Thu, 29 Jul 2021 16:15:03 +0000 (12:15 -0400)]
Avoid calling TestLib::perl2host on a symlinked directory
Certain versions of msys2/Windows have been observed to resolve symlinks
in perl2host rather than just follow them. This defeats using a
symlinked shorter path to a longer path, and makes certain tests fail.
We therefore call perl2host on the parent directory of the symlink and
thereafter just use that result.
Apply to release 14 where the problem has been observed.
Andrew Dunstan [Thu, 29 Jul 2021 16:15:03 +0000 (12:15 -0400)]
Make TestLib::perl2host more consistent and robust
Sometimes cygpath has been observed to return a path with a trailing
slash. That can cause problems, Also, make "cygpath" usage
consistent with "pwd -W" with respect to the use of forward slashes.
Backpatch to release 14 where the current code was introduced.
Amit Kapila [Thu, 29 Jul 2021 10:21:45 +0000 (15:51 +0530)]
Refactor to make common functions in proto.c and worker.c.
This is a non-functional change only to refactor code to extract some
replication logic into static functions.
This is done as preparation for the 2PC streaming patch which also shares
this common logic.
Author: Peter Smith
Reviewed-By: Amit Kapila
Discussion: https://postgr.es/m/CAHut+PuiSA8AiLcE2N5StzSKs46SQEP_vDOUD5fX2XCVtfZ7mQ@mail.gmail.com