Magnus Hagander [Wed, 11 Nov 2020 15:47:13 +0000 (16:47 +0100)]
Remove vacuumdb --analyze-in-stages from pg_upgrade tests
This step was only there to test the script when we generated those, but
commit
8f113698b6 removed those scripts, so it's not needed anymore.
Reported-By: Peter Eisentraut
Discussion: https://postgr.es/m/
ea403f46-2b33-a7de-618e-
9cab35a698c8@enterprisedb.com
Peter Eisentraut [Sat, 17 Oct 2020 06:38:39 +0000 (08:38 +0200)]
Add pg_nodiscard decorations to some functions
Especially for the list API such as lappend() forgetting to assign the
return value is a common problem.
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/
e3753562-99cd-b65f-5aca-
687dfd1ec2fc@2ndquadrant.com
Peter Eisentraut [Sat, 17 Oct 2020 06:38:39 +0000 (08:38 +0200)]
Add pg_nodiscard function declaration specifier
pg_nodiscard means the compiler should warn if the result of a
function call is ignored. The name "nodiscard" is chosen in alignment
with (possibly future) C and C++ standards. It maps to the GCC
attribute warn_unused_result.
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/
e3753562-99cd-b65f-5aca-
687dfd1ec2fc@2ndquadrant.com
Peter Eisentraut [Sat, 17 Oct 2020 06:38:39 +0000 (08:38 +0200)]
Fix cases of discarding result from list API functions
Two cases violated list APIs by throwing away the return value. While
the code was technically correct, it relied on internal knowledge of
the list implementation, and the code wasn't really gaining anything
that way. It is planned to make this a compiler warning in the
future, so just fix these cases by assigning the return value
properly.
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/
e3753562-99cd-b65f-5aca-
687dfd1ec2fc@2ndquadrant.com
Tom Lane [Wed, 11 Nov 2020 03:51:18 +0000 (22:51 -0500)]
Fix and simplify some usages of TimestampDifference().
Introduce TimestampDifferenceMilliseconds() to simplify callers
that would rather have the difference in milliseconds, instead of
the select()-oriented seconds-and-microseconds format. This gets
rid of at least one integer division per call, and it eliminates
some apparently-easy-to-mess-up arithmetic.
Two of these call sites were in fact wrong:
* pg_prewarm's autoprewarm_main() forgot to multiply the seconds
by 1000, thus ending up with a delay 1000X shorter than intended.
That doesn't quite make it a busy-wait, but close.
* postgres_fdw's pgfdw_get_cleanup_result() thought it needed to compute
microseconds not milliseconds, thus ending up with a delay 1000X longer
than intended. Somebody along the way had noticed this problem but
misdiagnosed the cause, and imposed an ad-hoc 60-second limit rather
than fixing the units. This was relatively harmless in context, because
we don't care that much about exactly how long this delay is; still,
it's wrong.
There are a few more callers of TimestampDifference() that don't
have a direct need for seconds-and-microseconds, but can't use
TimestampDifferenceMilliseconds() either because they do need
microsecond precision or because they might possibly deal with
intervals long enough to overflow 32-bit milliseconds. It might be
worth inventing another API to improve that, but that seems outside
the scope of this patch; so those callers are untouched here.
Given the fact that we are fixing some bugs, and the likelihood
that future patches might want to back-patch code that uses this
new API, back-patch to all supported branches.
Alexey Kondratov and Tom Lane
Discussion: https://postgr.es/m/
3b1c053a21c07c1ed5e00be3b2b855ef@postgrespro.ru
Bruce Momjian [Wed, 11 Nov 2020 00:18:35 +0000 (19:18 -0500)]
doc: fix spelling "connction" to "connection"
Was wrong in commit
1a9388bd0f.
Reported-by: Tom Lane, Justin Pryzby
Discussion: https://postgr.es/m/
20201102063333.GE22691@telsasoft.com
Backpatch-through: 9.5
Tom Lane [Tue, 10 Nov 2020 23:32:36 +0000 (18:32 -0500)]
Work around cross-version-upgrade issues created by commit
9e38c2bb5.
Summarily changing the STYPE of regression-test aggregates that
depend on array_append or array_cat is an issue for the buildfarm's
cross-version-upgrade tests, because those aggregates (as defined
in the back branches) now won't load into HEAD. Although this seems
like only a minimal risk for genuine user-defined aggregates, we
need to do something for the buildfarm. Hence, adjust the aggregate
definitions, in both HEAD and the back branches.
Discussion: https://postgr.es/m/
1401824.
1604537031@sss.pgh.pa.us
Discussion: https://postgr.es/m/E1kaQ2c-0005lx-Eg@gemulon.postgresql.org
Heikki Linnakangas [Tue, 10 Nov 2020 17:25:46 +0000 (19:25 +0200)]
pg_rewind: Fix thinko in parsing target WAL.
It's entirely possible to see WAL for a relation that doesn't exist in
the target anymore. That happens when the relation was dropped later.
The refactoring in commit
eb00f1d4b broke that case, by sanity-checking
the file type in the target before checking the flag forwhether it
exists there at all.
I noticed this during manual testing. Modify the 001_basic.pl test so
that it covers this case.
Magnus Hagander [Tue, 10 Nov 2020 12:14:09 +0000 (13:14 +0100)]
Fix out of date comment
Magnus Hagander [Tue, 10 Nov 2020 12:08:21 +0000 (13:08 +0100)]
Remove -o option to postmaster
This option was declared obsolete many years ago.
Reviewed-By: Tom Lane
Discussion: https://postgr.es/m/CABUevEyOE=9CQwZm2j=vwP5+6OLCSoxn9pBjK8gyRdkTzMfqtQ@mail.gmail.com
Andres Freund [Tue, 10 Nov 2020 04:01:33 +0000 (20:01 -0800)]
jit: Add support for LLVM 12.
LLVM 12, to be released in a few months, made some breaking changes to
the Orc JIT interface. OrcV2 eventually will make it easier to support
features like concurrent JIT compilation, but this commit only allows
to compile against LLVM 12.
This commit is a bit bigger than desirable. That partially is because
the V2 interface is more granular than V1 interface, but also because
I chose to make some minor changes to < LLVM 12 code to keep the code
somewhat readable.
The LLVM 12 support will need to be backpatched. I plan to do so after
the patch stewed on the buildfarm for a few days.
Author: Andres Freund
Discussion: https://postgr.es/m/
20201016011244.pmyvr3ee2gbzplq4@alap3.anarazel.de
Tom Lane [Mon, 9 Nov 2020 17:02:24 +0000 (12:02 -0500)]
Doc: clarify data type behavior of COALESCE and NULLIF.
After studying the code, NULLIF is a lot more subtle than you might
have guessed.
Discussion: https://postgr.es/m/
160486028730.25500.
15740897403028593550@wrigleys.postgresql.org
Peter Geoghegan [Mon, 9 Nov 2020 17:00:12 +0000 (09:00 -0800)]
Remove ineffective heapam CHECK_FOR_INTERRUPTS().
Remove a CHECK_FOR_INTERRUPTS() call that could never actually handle an
interrupt. We always have a heap page buffer lock at this point.
Having a useless CHECK_FOR_INTERRUPTS() call is harmless but misleading.
It is probably possible to work around the immediate problem by moving
the CHECK_FOR_INTERRUPTS() to before the heap page buffer lock is
acquired. That isn't enough to make the function responsive to
interrupts, though. The index AM caller will still hold an exclusive
buffer lock of its own.
Noah Misch [Mon, 9 Nov 2020 15:32:09 +0000 (07:32 -0800)]
Ignore attempts to \gset into specially treated variables.
If an interactive psql session used \gset when querying a compromised
server, the attacker could execute arbitrary code as the operating
system account running psql. Using a prefix not found among specially
treated variables, e.g. every lowercase string, precluded the attack.
Fix by issuing a warning and setting no variable for the column in
question. Users wanting the old behavior can use a prefix and then a
meta-command like "\set HISTSIZE :prefix_HISTSIZE". Back-patch to 9.5
(all supported versions).
Reviewed by Robert Haas. Reported by Nick Cleaton.
Security: CVE-2020-25696
Noah Misch [Mon, 9 Nov 2020 15:32:09 +0000 (07:32 -0800)]
In security-restricted operations, block enqueue of at-commit user code.
Specifically, this blocks DECLARE ... WITH HOLD and firing of deferred
triggers within index expressions and materialized view queries. An
attacker having permission to create non-temp objects in at least one
schema could execute arbitrary SQL functions under the identity of the
bootstrap superuser. One can work around the vulnerability by disabling
autovacuum and not manually running ANALYZE, CLUSTER, REINDEX, CREATE
INDEX, VACUUM FULL, or REFRESH MATERIALIZED VIEW. (Don't restore from
pg_dump, since it runs some of those commands.) Plain VACUUM (without
FULL) is safe, and all commands are fine when a trusted user owns the
target object. Performance may degrade quickly under this workaround,
however. Back-patch to 9.5 (all supported versions).
Reviewed by Robert Haas. Reported by Etienne Stalmans.
Security: CVE-2020-25695
Magnus Hagander [Mon, 9 Nov 2020 11:14:59 +0000 (12:14 +0100)]
Remove analyze_new_cluster script from pg_upgrade
Since this script just runs vacuumdb anyway, remove the script and
replace the instructions to run it with instructions to run vacuumdb
directly.
Reviewed-By: Michael Paquier
Discussion: https://postgr.es/m/CABUevEwg5LDFzthhxzSj7sZGMiVsZe0VVNbzzwTQOHJ=rN7+5A@mail.gmail.com
Magnus Hagander [Mon, 9 Nov 2020 09:36:49 +0000 (10:36 +0100)]
Remove incorrect %s in string
Appears to have been a copy/paste error in the original commit that
moved the messages to fe_utils/.
Author: Tang, Haiying <tanghy.fnst@cn.fujitsu.com>
Backpatch-through: 13
Discussion: https://postgr.es/m/
3321cbcea76d4d2c8320a05c19b9304a@G08CNEXMBPEKD05.g08.fujitsu.local
Fujii Masao [Mon, 9 Nov 2020 06:10:26 +0000 (15:10 +0900)]
doc: Add note about pg_settings and customized options into catalogs.sgml.
The pg_settings view does not display customized options until
the extension module that defines them has been loaded. This commit
add the note about that behavior, into the docs.
Author: John Naylor
Reviewed-by: Tom Lane, Fujii Masao
Discussion: https://postgr.es/m/CAFBsxsGsBZsG=cLM0Op5HFb2Ks6SzJrOc_eRO_jcKSNuqFRKnQ@mail.gmail.com
Thomas Munro [Mon, 9 Nov 2020 03:01:51 +0000 (16:01 +1300)]
Fix parsePGArray() error checking in pg_dump.
Coverity complained about a defect in commit
257836a7:
Calling "parsePGArray" without checking return value (as is
done elsewhere 11 out of 13 times).
Fix, and also check for empty strings explicitly (NULL as represented by
PQgetvalue()). That worked correctly before only because parsePGArray()
happens to set *nitems = 0 when it fails on an empty string. Also
convert a sanity check assertion to an error to be more paranoid, and
pgindent a nearby line.
Reported-by: Michael Paquier <michael@paquier.xyz>
Tom Lane [Sun, 8 Nov 2020 18:08:36 +0000 (13:08 -0500)]
In INSERT/UPDATE, use the table's real tuple descriptor as target.
This back-patches commit
20d3fe900 into the v12 and v13 branches.
At the time I thought that commit was not fixing any observable
bug, but Bertrand Drouvot showed otherwise: adding a dropped column
to the previously-considered scenario crashes v12 and v13, unless the
dropped column happens to be an integer. That is, of course, because
the tupdesc we derive from the plan output tlist fails to describe
the dropped column accurately, so that we'll do the wrong thing with
a tuple in which that column isn't NULL.
There is no bug in pre-v12 branches because they already did use
the table's real tuple descriptor for any trigger-returned tuple.
It seems that this set of bugs can be blamed on the changes that
removed es_trig_tuple_slot, though I've not attempted to pin that
down precisely.
Although there's no code change needed in HEAD, update the test case
to include a dropped column there too.
Discussion: https://postgr.es/m/
db5d97c8-f48a-51e2-7b08-
b73d5434d425@amazon.com
Discussion: https://postgr.es/m/16644-
5da7ef98a7ac4545@postgresql.org
Thomas Munro [Sun, 8 Nov 2020 07:43:45 +0000 (20:43 +1300)]
Fix assertion in collation version lookup.
Commit
257836a7 included an assertion that a version lookup routine is
not trying to look up "C" or "POSIX", but that case is reachable with
the user-facing SQL function pg_collation_actual_version(). Remove the
assertion.
Peter Eisentraut [Sun, 8 Nov 2020 06:48:18 +0000 (07:48 +0100)]
Fix test for error message change
fix for
6be725e701611660b36642de9ff1d665a1eb24f5
Peter Geoghegan [Sun, 8 Nov 2020 02:51:12 +0000 (18:51 -0800)]
Improve nbtree README's LP_DEAD section.
The description of how LP_DEAD bit setting by index scans works
following commit
2ed5b87f was rather unclear. Clean that up a bit.
Also refer to LP_DEAD bit setting within _bt_check_unique() at the start
of the same section. This mechanism may actually be more important than
the generic kill_prior_tuple mechanism that the section focuses on, so
it at least deserves to be mentioned in passing.
Alvaro Herrera [Sat, 7 Nov 2020 22:33:43 +0000 (19:33 -0300)]
Message style improvements
* Avoid pointlessly highlighting that an index vacuum was executed by a
parallel worker; user doesn't care.
* Don't give the impression that a non-concurrent reindex of an invalid
index on a TOAST table would work, because it wouldn't.
* Add a "translator:" comment for a mysterious message.
Discussion: https://postgr.es/m/
20201107034943.GA16596@alvherre.pgsql
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Peter Eisentraut [Sat, 7 Nov 2020 21:15:52 +0000 (22:15 +0100)]
Fix redundant error messages in client tools
A few client tools duplicate error messages already provided by libpq.
Discussion: https://www.postgresql.org/message-id/flat/
3e937641-88a1-e697-612e-
99bba4b8e5e4%40enterprisedb.com
Tom Lane [Sat, 7 Nov 2020 21:25:42 +0000 (16:25 -0500)]
Avoid re-using output variables in new ecpg test case.
The buildfarm thinks this leads to memory stomps, though annoyingly
I can't duplicate that here. The existing code in strings.pgc is
doing something that doesn't seem to be sanctioned at all really
by the documentation, but I'm disinclined to try to make that nicer
right now. Let's just declare some more output variables in hopes
of working around it.
Tom Lane [Sat, 7 Nov 2020 20:03:44 +0000 (15:03 -0500)]
Fix ecpg's mishandling of B'...' and X'...' literals.
These were broken in multiple ways:
* The xbstart and xhstart lexer actions neglected to set
"state_before_str_start" before transitioning to the xb/xh states,
thus possibly resulting in "internal error: unreachable state" later.
* The test for valid string contents at the end of xb state was flat out
wrong, as it accounted incorrectly for the "b" prefix that the xbstart
action had injected. Meanwhile, the xh state had no such check at all.
* The generated literal value failed to include any quote marks.
* The grammar did the wrong thing anyway, typically ignoring the
literal value and emitting something else, since BCONST and XCONST
tokens were handled randomly differently from SCONST tokens.
The first of these problems is evidently an oversight in commit
7f380c59f, but the others seem to be very ancient. The lack of
complaints shows that ECPG users aren't using these syntaxes much
(although I do vaguely remember one previous complaint).
As written, this patch is dependent on
7f380c59f, so it can't go
back further than v13. Given the shortage of complaints, I'm not
excited about adapting the patch to prior branches.
Report and patch by Shenhao Wang (test case adjusted by me)
Discussion: https://postgr.es/m/
d6402f1bacb74ecba22ef715dbba17fd@G08CNEXMBPEKD06.g08.fujitsu.local
Peter Eisentraut [Sat, 7 Nov 2020 11:11:40 +0000 (12:11 +0100)]
Move catalog index declarations
Move the system catalog index declarations from catalog/indexing.h to
the respective parent tables' catalog/pg_*.h files. The original
reason for having it split was that the old genbki system produced the
output in the order of the catalog files it read, so all the indexing
stuff needed to come separately. But this is no longer the case, and
keeping it together makes more sense.
Reviewed-by: John Naylor <john.naylor@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/
c7cc82d6-f976-75d6-2e3e-
b03d2cab26bb@2ndquadrant.com
Peter Eisentraut [Sat, 7 Nov 2020 11:11:40 +0000 (12:11 +0100)]
Move catalog toast table declarations
Move the system catalog toast table declarations from
catalog/toasting.h to the respective parent tables' catalog/pg_*.h
files. The original reason for having it split was that the old
genbki system produced the output in the order of the catalog files it
read, so all the toasting stuff needed to come separately. But this
is no longer the case, and keeping it together makes more sense.
Reviewed-by: John Naylor <john.naylor@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/
c7cc82d6-f976-75d6-2e3e-
b03d2cab26bb@2ndquadrant.com
Alvaro Herrera [Sat, 7 Nov 2020 01:52:16 +0000 (22:52 -0300)]
Plug memory leak in index_get_partition
The list of indexes was being leaked when asked for an index that
doesn't have an index partition in the table partition. Not a common
case admittedly --and in most cases where it occurs, caller throws an
error anyway-- but worth fixing for cleanliness and in case any
third-party code is calling this function.
While at it, remove use of lfirst_oid() to obtain a value we already
have.
Author: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/
20201105203606.GF22691@telsasoft.com
Michael Paquier [Sat, 7 Nov 2020 01:30:22 +0000 (10:30 +0900)]
Add GUC_LIST_INPUT and GUC_LIST_QUOTE to unix_socket_directories
This should have been done in the initial commit that made
unix_socket_directories a list as of
c9b0cbe. This change allows to
support correctly the case of ALTER SYSTEM, where it is possible to
specify multiple paths as a list, like the following pattern where
flattening is applied to each item:
ALTER SYSTEM SET unix_socket_directories = '/path1', '/path2';
Any parameters specified in postgresql.conf are parsed the same way, so
there is no compatibility change. pg_dump has a hardcoded list of
parameters marked with GUC_LIST_QUOTE, that gets its routine update.
These are reordered alphabetically for clarity.
Author: Ian Lawrence Barwick
Reviewed-by: Peter Eisentraunt, Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/CAB8KJ=iMOtNY6_sUwV=LQVCJ2zgYHBDyNzVfvE5GN3WQ3v9kQg@mail.gmail.com
Michael Paquier [Sat, 7 Nov 2020 01:15:58 +0000 (10:15 +0900)]
Fix minor issues with new unicode {de,re}composition code
The table generation script would incorrectly complain in the
recomposition sorting when matching code points. This would not have
caused the generation of an incorrect table. Note that this condition
is not reachable yet, but could have been reached with future updates.
pg_bswap.h does not need to be included in the frontend.x
Author: John Naylor
Discussion: https://postgr.es/m/CAFBsxsGWmExpvv=61vtDKCs7+kBbhkwBDL2Ph9CacziFKnV_yw@mail.gmail.com
Tomas Vondra [Fri, 6 Nov 2020 23:39:19 +0000 (00:39 +0100)]
Properly detoast data in brin_form_tuple
brin_form_tuple failed to consider the values may be toasted, inserting
the toast pointer into the index. This may easily result in index
corruption, as the toast data may be deleted and cleaned up by vacuum.
The cleanup however does not care about indexes, leaving invalid toast
pointers behind, which triggers errors like this:
ERROR: missing chunk number 0 for toast value 16433 in pg_toast_16426
A less severe consequence are inconsistent failures due to the index row
being too large, depending on whether brin_form_tuple operated on plain
or toasted version of the row. For example
CREATE TABLE t (val TEXT);
INSERT INTO t VALUES ('... long value ...')
CREATE INDEX idx ON t USING brin (val);
would likely succeed, as the row would likely include toast pointer.
Switching the order of INSERT and CREATE INDEX would likely fail:
ERROR: index row size 8712 exceeds maximum 8152 for index "idx"
because this happens before the row values are toasted.
The bug exists since PostgreSQL 9.5 where BRIN indexes were introduced.
So backpatch all the way back.
Author: Tomas Vondra
Reviewed-by: Alvaro Herrera
Backpatch-through: 9.5
Discussion: https://postgr.es/m/
20201001184133.oq5uq75sb45pu3aw@development
Discussion: https://postgr.es/m/
20201104010544.zexj52mlldagzowv%40development
Tom Lane [Fri, 6 Nov 2020 21:17:56 +0000 (16:17 -0500)]
Revert "Accept relations of any kind in LOCK TABLE".
Revert
59ab4ac32, as well as the followup fix
33862cb9c, in all
branches. We need to think a bit harder about what the behavior
of LOCK TABLE on views should be, and there's no time for that
before next week's releases. We'll take another crack at this
later.
Discussion: https://postgr.es/m/16703-
e348f58aab3cf6cc@postgresql.org
Tom Lane [Fri, 6 Nov 2020 20:48:04 +0000 (15:48 -0500)]
Revert "pg_dump: Lock all relations, not just plain tables".
Revert
403a3d91c, as well as the followup fix
7f4235032, in all
branches. We need to think a bit harder about what the behavior
of LOCK TABLE on views should be, and there's no time for that
before next week's releases. We'll take another crack at this
later.
Discussion: https://postgr.es/m/16703-
e348f58aab3cf6cc@postgresql.org
Fujii Masao [Fri, 6 Nov 2020 17:08:06 +0000 (02:08 +0900)]
pg_prewarm: make autoprewarm leader use standard SIGHUP and SIGTERM handlers.
Commit
1e53fe0e70 changed background processes so that they use
standard SIGHUP handler. Like that, this commit makes autoprewarm leader
process also use standard SIGHUP and SIGTERM handlers, to simplify the code.
Author: Bharath Rupireddy
Reviewed-by: Kyotaro Horiguchi, Fujii Masao
Discussion: https://postgr.es/m/CALj2ACXPorUqePswDtOeM_s82v9RW32E1fYmOPZ5NuE+TWKj_A@mail.gmail.com
Magnus Hagander [Fri, 6 Nov 2020 12:21:28 +0000 (13:21 +0100)]
Add pg_strong_random_init function to initialize random number generator
Currently only OpenSSL requires this initialization, but in the future
other SSL implementations are likely to need it as well. Abstracting
this functionality out into a separate function makes this cleaner and
more clear, and also removes the dependency on OpenSSL headers from
fork_process.c.
OpenSSL is special in that we need to initialize this random number
generator even if we're not going to use it directly, until we drop
support for everything prior to OpenSSL 1.1.1. (And of course also if we
actually use it). All other implementations are left empty at this time,
but more are expected to be added in the future.
Author: Daniel Gustafsson <daniel@yesql.se>, Michael Paquier <michael@paquier.xyz>
Reviewed-By: Magnus Hagander <magnus@hagander.net>
Discussion: https://postgr.es/m/
F6291C3C-747C-4C93-BCE0-
28BB420B1FF5@yesql.se
Amit Kapila [Fri, 6 Nov 2020 02:42:48 +0000 (08:12 +0530)]
Use strlcpy instead of memcpy for copying the slot name in pgstat.c.
There is no outright bug here but it is better to be consistent with the
usage at other places in the same file. In the passing, fix a wrong
assertion in pgstat_recv_replslot.
Author: Kyotaro Horiguchi
Reviewed-by: Sawada Masahiko and Amit Kapila
Discussion: https://postgr.es/m/
20201104.175523.
1704166915688949637.horikyota.ntt@gmail.com
Peter Geoghegan [Thu, 5 Nov 2020 23:01:40 +0000 (15:01 -0800)]
Fix wal_consistency_checking nbtree bug.
wal_consistency_checking indicated an inconsistency in certain cases
involving nbtree page deletion. The underlying issue is that there was
a minor difference between the page image produced after a REDO routine
ran and the corresponding page image following original execution.
This harmless inconsistency has been around forever. We more or less
expect total consistency among even deleted nbtree pages these days,
though, so this won't do anymore.
To fix, tweak the REDO routine to match original execution.
Oversight in commit
f47b5e13.
Tom Lane [Thu, 5 Nov 2020 16:44:32 +0000 (11:44 -0500)]
Don't throw an error for LOCK TABLE on a self-referential view.
LOCK TABLE has complained about "infinite recursion" when applied
to a self-referential view, ever since we made it recurse into views
in v11. However, that breaks pg_dump's new assumption that it's
okay to lock every relation. There doesn't seem to be any good
reason to throw an error: if we just abandon the recursion, we've
still satisfied the requirement of locking every referenced relation.
Per bug #16703 from Andrew Bille (via Alexander Lakhin).
Discussion: https://postgr.es/m/16703-
e348f58aab3cf6cc@postgresql.org
Peter Geoghegan [Thu, 5 Nov 2020 02:42:27 +0000 (18:42 -0800)]
Fix nbtree cleanup-only VACUUM stats inaccuracies.
Logic for counting heap TIDs from posting list tuples (added by commit
0d861bbb) was faulty. It didn't count any TIDs/index tuples in the
event of no callback being set. This meant that we incorrectly counted
no index tuples in clean-up only VACUUMs, which could lead to
pg_class.reltuples being spuriously set to 0 in affected indexes.
To fix, go back to counting items from the page in cases where there is
no callback. This approach isn't very accurate, but it works well
enough in practice while avoiding the expense of accessing every index
tuple during cleanup-only VACUUMs.
Author: Peter Geoghegan <pg@bowt.ie>
Reported-By: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
https://postgr.es/m/
20201023174451.
69e358f1@firost
Backpatch: 13-, where nbtree deduplication was introduced
Thomas Munro [Thu, 5 Nov 2020 00:45:32 +0000 (13:45 +1300)]
Fix unlinking of SLRU segments.
Commit
dee663f7 intended to drop any queued up fsync requests before
unlinking segment files, but missed a code path. Fix, by centralizing
the forget-and-unlink code into a single function.
Reported-by: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Discussion: https://postgr.es/m/
20201104013205.icogbi773przyny5%40development
Tom Lane [Wed, 4 Nov 2020 23:11:15 +0000 (18:11 -0500)]
Remove underflow error in float division with infinite divisor.
float4_div and float8_div correctly produced zero for zero divided
by infinity, but threw an underflow error for nonzero finite values
divided by infinity. This seems wrong; at the very least it's
inconsistent with the behavior recently implemented for numeric
infinities. Remove the error and allow zero to be returned.
This patch also removes a useless isinf() test from the overflow
checks in these functions (non-Inf divided by Inf can't produce Inf).
Extracted from a larger patch; this seems significant outside the
context of geometric operators, so it deserves its own commit.
Kyotaro Horiguchi
Discussion: https://postgr.es/m/CAGf+fX70rWFOk5cd00uMfa__0yP+vtQg5ck7c2Onb-Yczp0URA@mail.gmail.com
Tom Lane [Wed, 4 Nov 2020 21:09:55 +0000 (16:09 -0500)]
Declare assorted array functions using anycompatible not anyelement.
Convert array_append, array_prepend, array_cat, array_position,
array_positions, array_remove, array_replace, and width_bucket
to use anycompatiblearray. This is a simple extension of commit
5c292e6b9 to hit some other places where there's a pretty obvious
gain in usability from doing so.
Ideally we'd also modify other functions taking multiple old-style
polymorphic arguments. But most of the remainder are tied into one
or more operator classes, making any such change a much larger can of
worms than I desire to open right now.
Discussion: https://postgr.es/m/
77675130-89da-dab1-51dd-
492c93dcf5d1@postgresfriends.org
Tom Lane [Wed, 4 Nov 2020 20:08:37 +0000 (15:08 -0500)]
Declare lead() and lag() using anycompatible not anyelement.
This allows use of a "default" expression that doesn't slavishly
match the data column's type. Formerly you got something like
"function lag(numeric, integer, integer) does not exist", which
is not just unhelpful but actively misleading.
The SQL spec suggests that the default should be coerced to the data
column's type, but this implementation instead chooses the common
supertype, which seems at least as reasonable.
(Note: I took the opportunity to run "make reformat-dat-files" on
pg_proc.dat, so this commit includes some cosmetic changes to
recently-added entries that aren't related to lead/lag.)
Vik Fearing
Discussion: https://postgr.es/m/
77675130-89da-dab1-51dd-
492c93dcf5d1@postgresfriends.org
Tom Lane [Wed, 4 Nov 2020 17:34:50 +0000 (12:34 -0500)]
Improve our ability to regurgitate SQL-syntax function calls.
The SQL spec calls out nonstandard syntax for certain function calls,
for example substring() with numeric position info is supposed to be
spelled "SUBSTRING(string FROM start FOR count)". We accept many
of these things, but up to now would not print them in the same format,
instead simplifying down to "substring"(string, start, count).
That's long annoyed me because it creates an interoperability
problem: we're gratuitously injecting Postgres-specific syntax into
what might otherwise be a perfectly spec-compliant view definition.
However, the real reason for addressing it right now is to support
a planned change in the semantics of EXTRACT() a/k/a date_part().
When we switch that to returning numeric, we'll have the parser
translate EXTRACT() to some new function name (might as well be
"extract" if you ask me) and then teach ruleutils.c to reverse-list
that per SQL spec. In this way existing calls to date_part() will
continue to have the old semantics.
To implement this, invent a new CoercionForm value COERCE_SQL_SYNTAX,
and make the parser insert that rather than COERCE_EXPLICIT_CALL when
the input has SQL-spec decoration. (But if the input has the form of
a plain function call, continue to mark it COERCE_EXPLICIT_CALL, even
if it's calling one of these functions.) Then ruleutils.c recognizes
COERCE_SQL_SYNTAX as a cue to emit SQL call syntax. It can know
which decoration to emit using hard-wired knowledge about the
functions that could be called this way. (While this solution isn't
extensible without manual additions, neither is the grammar, so this
doesn't seem unmaintainable.) Notice that this solution will
reverse-list a function call with SQL decoration only if it was
entered that way; so dump-and-reload will not by itself produce any
changes in the appearance of views.
This requires adding a CoercionForm field to struct FuncCall.
(I couldn't resist the temptation to rearrange that struct's
field order a tad while I was at it.) FuncCall doesn't appear
in stored rules, so that change isn't a reason for a catversion
bump, but I did one anyway because the new enum value for
CoercionForm fields could confuse old backend code.
Possible future work:
* Perhaps CoercionForm should now be renamed to DisplayForm,
or something like that, to reflect its more general meaning.
This'd require touching a couple hundred places, so it's not
clear it's worth the code churn.
* The SQLValueFunction node type, which was invented partly for
the same goal of improving SQL-compatibility of view output,
could perhaps be replaced with regular function calls marked
with COERCE_SQL_SYNTAX. It's unclear if this would be a net
code savings, however.
Discussion: https://postgr.es/m/
42b73d2d-da12-ba9f-570a-
420e0cce19d9@phystech.edu
Tom Lane [Wed, 4 Nov 2020 16:25:56 +0000 (11:25 -0500)]
Remove useless entries for aggregate functions from fmgrtab.c.
Gen_fmgrtab.pl treated aggregate functions the same as other built-in
functions, which is wasteful because there is no real need to have
entries for them in the fmgr_builtins[] table. Suppressing those
entries saves about 3KB in the compiled table on my machine; which
is not a lot but it's not nothing either, considering that that
table is pretty "hot". The only outside code change needed is
that ExecInitWindowAgg() can't be allowed to call fmgr_info_cxt()
on a plain aggregate function. But that saves a few cycles anyway.
Having done that, the aggregate_dummy() function is unreferenced
and might as well be dropped. Using "aggregate_dummy" as the prosrc
value for an aggregate is now just a documentation convention not
something that matters. There was some discussion of using NULL
instead to save a few bytes in pg_proc, but we'd have to remove
prosrc's BKI_FORCE_NOT_NULL marking which doesn't seem a great idea.
Anyway, it's possible there's client-side code that expects to
see "aggregate_dummy" there, so I'm loath to change it without a
strong reason.
Discussion: https://postgr.es/m/533989.
1604263665@sss.pgh.pa.us
Fujii Masao [Wed, 4 Nov 2020 12:49:00 +0000 (21:49 +0900)]
Fix segmentation fault that commit
ac22929a26 caused.
Commit
ac22929a26 changed recoveryWakeupLatch so that it's reset to
NULL at the end of recovery. This change could cause a segmentation fault
in the buildfarm member 'elver'.
Previously the latch was reset to NULL after calling ShutdownWalRcv().
But there could be a window between ShutdownWalRcv() and the actual
exit of walreceiver. If walreceiver set the latch during that window,
the segmentation fault could happen.
To fix the issue, this commit changes walreceiver so that it sets
the latch only when the latch has not been reset to NULL yet.
Author: Fujii Masao
Discussion: https://postgr.es/m/
5c1f8a85-747c-7bf9-241e-
dd467d8a3586@iki.fi
Peter Eisentraut [Wed, 4 Nov 2020 06:47:06 +0000 (07:47 +0100)]
Enable hash partitioning of text arrays
hash_array_extended() needs to pass PG_GET_COLLATION() to the hash
function of the element type. Otherwise, the hash function of a
collation-aware data type such as text will error out, since the
introduction of nondeterministic collation made hash functions require
a collation, too.
The consequence of this is that before this change, hash partitioning
using an array over text in the partition key would not work.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/
32c1fdae-95c6-5dc6-058a-
a90330a3b621%40enterprisedb.com
Heikki Linnakangas [Wed, 4 Nov 2020 09:21:18 +0000 (11:21 +0200)]
pg_rewind: Refactor the abstraction to fetch from local/libpq source.
This makes the abstraction of a "source" server more clear, by introducing
a common abstract class, borrowing the object-oriented programming term,
that represents all the operations that can be done on the source server.
There are two implementations of it, one for fetching via libpq, and
another to fetch from a local directory. This adds some code, but makes it
easier to understand what's going on.
The copy_executeFileMap() and libpq_executeFileMap() functions contained
basically the same logic, just calling different functions to fetch the
source files. Refactor so that the common logic is in one place, in a new
function called perform_rewind().
Reviewed-by: Kyotaro Horiguchi, Soumyadeep Chakraborty
Discussion: https://www.postgresql.org/message-id/
0c5b3783-af52-3ee5-f8fa-
6e794061f70d%40iki.fi
Heikki Linnakangas [Wed, 4 Nov 2020 09:21:14 +0000 (11:21 +0200)]
pg_rewind: Replace the hybrid list+array data structure with simplehash.
Now that simplehash can be used in frontend code, let's make use of it.
Reviewed-by: Kyotaro Horiguchi, Soumyadeep Chakraborty
Discussion: https://www.postgresql.org/message-id/
0c5b3783-af52-3ee5-f8fa-
6e794061f70d%40iki.fi
Heikki Linnakangas [Wed, 4 Nov 2020 09:21:09 +0000 (11:21 +0200)]
Refactor pg_rewind for more clear decision making.
Deciding what to do with each file is now a separate step after all the
necessary information has been gathered. It is more clear that way.
Previously, the decision-making was divided between process_source_file()
and process_target_file(), and it was a bit hard to piece together what
the overall rules were.
Reviewed-by: Kyotaro Horiguchi, Soumyadeep Chakraborty
Discussion: https://www.postgresql.org/message-id/
0c5b3783-af52-3ee5-f8fa-
6e794061f70d%40iki.fi
Heikki Linnakangas [Wed, 4 Nov 2020 08:38:39 +0000 (10:38 +0200)]
pg_rewind: Move syncTargetDirectory() to file_ops.c
For consistency. All the other low-level functions that operate on the
target directory are in file_ops.c.
Reviewed-by: Michael Paquier
Discussion: https://www.postgresql.org/message-id/
0c5b3783-af52-3ee5-f8fa-
6e794061f70d%40iki.fi
Fujii Masao [Wed, 4 Nov 2020 07:41:29 +0000 (16:41 +0900)]
Get rid of the dedicated latch for signaling the startup process.
This commit gets rid of the dedicated latch for signaling the startup
process in favor of using its procLatch, since that comports better
with possible generic signal handlers using that latch.
Commit
1e53fe0e70 changed background processes so that they use standard
SIGHUP handler. Like that, this commit also makes the startup process use
standard SIGHUP handler to simplify the code.
Author: Fujii Masao
Reviewed-by: Bharath Rupireddy, Michael Paquier
Discussion: https://postgr.es/m/CALj2ACXPorUqePswDtOeM_s82v9RW32E1fYmOPZ5NuE+TWKj_A@mail.gmail.com
Fujii Masao [Wed, 4 Nov 2020 05:48:02 +0000 (14:48 +0900)]
Use standard SIGHUP handler in syslogger.
Commit
1e53fe0e70 changed background processes so that they use
standard SIGHUP handler. Like that, this commit makes syslogger use
standard SIGHUP handler to simplify the code.
Author: Bharath Rupireddy
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/CALj2ACXPorUqePswDtOeM_s82v9RW32E1fYmOPZ5NuE+TWKj_A@mail.gmail.com
Thomas Munro [Wed, 4 Nov 2020 01:58:34 +0000 (14:58 +1300)]
Tolerate version lookup failure for old style Windows locale names.
Accept that we can't get versions for such locale names for now. Users
will need to specify the newer language tag format to enable the
collation versioning feature. It's not clear that we can do automatic
conversion from the old style to the new style reliably enough for this
purpose.
Unfortunately, this means that collation versioning probably won't work
for the default collation unless you provide something like en-US at
initdb or CREATE DATABASE time (though, for reasons not yet understood,
it does seem to work on some systems). It'd be nice to find a better
solution, or document this quirk if we settle on it, but this should
unbreak the 3 failing build farm animals in the meantime.
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAEepm%3D0uEQCpfq_%2BLYFBdArCe4Ot98t1aR4eYiYTe%3DyavQygiQ%40mail.gmail.com
Michael Paquier [Wed, 4 Nov 2020 01:21:46 +0000 (10:21 +0900)]
Revert pg_relation_check_pages()
This reverts the following set of commits, following complaints about
the lack of portability of the central part of the code in bufmgr.c as
well as the use of partition mapping locks during page reads:
c780a7a9
f2b88396
b787d4ce
ce7f772c
60a51c6b
Per discussion with Andres Freund, Robert Haas and myself.
Bump catalog version.
Discussion: https://postgr.es/m/
20201029181729.2nrub47u7yqncsv7@alap3.anarazel.de
Tomas Vondra [Tue, 3 Nov 2020 19:43:12 +0000 (20:43 +0100)]
Use INT64_FORMAT to print int64 variables in sort debug
Commit
6ee3b5fb99 cleaned up most of the long/int64 confusion related to
incremental sort, but the sort debug messages were still using %ld for
int64 variables. So fix that.
Author: Haiying Tang
Backpatch-through: 13, where the incremental sort code was added
Discussion: https://postgr.es/m/
4250be9d350c4992abb722a76e288aef%40G08CNEXMBPEKD05.g08.fujitsu.local
Tomas Vondra [Tue, 3 Nov 2020 19:07:23 +0000 (20:07 +0100)]
Fix get_useful_pathkeys_for_relation for volatile expressions
When considering Incremental Sort below a Gather Merge, we need to be
a bit more careful when matching pathkeys to EC members. It's not enough
to find a member whose Vars are all in the current relation's target;
volatile expressions in particular need to be contained in the target,
otherwise it's too early to use the pathkey.
Reported-by: Jaime Casanova
Author: James Coleman
Reviewed-by: Tomas Vondra
Backpatch-through: 13, where the incremental sort code was added
Discussion: https://postgr.es/m/CAJGNTeNaxpXgBVcRhJX%2B2vSbq%2BF2kJqGBcvompmpvXb7pq%2BoFA%40mail.gmail.com
Tom Lane [Tue, 3 Nov 2020 21:16:36 +0000 (16:16 -0500)]
Guard against core dump from uninitialized subplan.
If the planner erroneously puts a non-parallel-safe SubPlan into
a parallelized portion of the query tree, nodeSubplan.c will fail
in the worker processes because it finds a null in es_subplanstates,
which it's unable to cope with. It seems worth a test-and-elog to
make that an error case rather than a core dump case.
This probably should have been included in commit
16ebab688, which
was responsible for allowing nulls to appear in es_subplanstates
to begin with. So, back-patch to v10 where that came in.
Discussion: https://postgr.es/m/924226.
1604422326@sss.pgh.pa.us
Tom Lane [Tue, 3 Nov 2020 20:49:05 +0000 (15:49 -0500)]
Improve error messages around REPLICATION and BYPASSRLS properties.
Clarify wording as per suggestion from Wolfgang Walther.
No back-patch; this doesn't seem worth thrashing translatable
strings in the back branches.
Tom Lane and Stephen Frost
Discussion: https://postgr.es/m/
a5548a9f-89ee-3167-129d-
162b5985fcf8@technowledgy.de
Tom Lane [Tue, 3 Nov 2020 20:41:32 +0000 (15:41 -0500)]
Allow users with BYPASSRLS to alter their own passwords.
The intention in commit
491c029db was to require superuserness to
change the BYPASSRLS property, but the actual effect of the coding
in AlterRole() was to require superuserness to change anything at all
about a BYPASSRLS role. Other properties of a BYPASSRLS role should
be changeable under the same rules as for a normal role, though.
Fix that, and also take care of some documentation omissions related
to BYPASSRLS and REPLICATION role properties.
Tom Lane and Stephen Frost, per bug report from Wolfgang Walther.
Back-patch to all supported branches.
Discussion: https://postgr.es/m/
a5548a9f-89ee-3167-129d-
162b5985fcf8@technowledgy.de
Peter Eisentraut [Tue, 3 Nov 2020 14:14:50 +0000 (15:14 +0100)]
Disallow ALTER TABLE ONLY / DROP EXPRESSION
The current implementation cannot handle this correctly, so just
forbid it for now.
GENERATED clauses must be attached to the column definition and cannot
be added later like DEFAULT, so if a child table has a generation
expression that the parent does not have, the child column will
necessarily be an attlocal column. So to implement ALTER TABLE ONLY /
DROP EXPRESSION, we'd need extra code to update attislocal of the
direct child tables, somewhat similar to how DROP COLUMN does it, so
that the resulting state can be properly dumped and restored.
Discussion: https://www.postgresql.org/message-id/flat/15830.
1575468847%40sss.pgh.pa.us
Peter Eisentraut [Tue, 3 Nov 2020 09:32:20 +0000 (10:32 +0100)]
Remove deprecated containment operators for built-in types
Remove old containment operators @ and ~ for built-in geometry data
types. These have been deprecated; use <@ and @> instead.
(Some contrib modules still contain the same deprecated operators.
That will be dealt with separately.)
Author: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://www.postgresql.org/message-id/flat/
20201027032511.GF9241@telsasoft.com
Magnus Hagander [Tue, 3 Nov 2020 09:19:55 +0000 (10:19 +0100)]
Use the non-deprecated TG_TABLE_MAME in test trigger
Commit
3a9ae3d2068 (back in 2006) deprecated TG_RELNAME
in favor of TG_TABLE_NAME, but the existing usage in test
cases has remained till today. Change to use TG_TABLE_NAME
instead (TG_RELNAME is still covered by a test case).
Magnus Hagander [Tue, 3 Nov 2020 08:55:51 +0000 (09:55 +0100)]
Improve error handling in backend OpenSSL implementation
Commit
d94c36a45ab introduced error handling to sslinfo to handle
OpenSSL errors gracefully. This ports this errorhandling to the
backend TLS implementation.
Author: Daniel Gustafsson <daniel@yesql.se>
Magnus Hagander [Tue, 3 Nov 2020 08:47:36 +0000 (09:47 +0100)]
Use be_tls_* API for SSL information in sslinfo
sslinfo was passing the Port->ssl member directly to OpenSSL in order
to extract information regarding the connection. This breaks the API
provided by the backend TLS implementation, as well as duplicates code
for no benefit. Rewrite to make use of the backend API as much as
possible.
Author: Daniel Gustafsson <daniel@yesql.se>
Peter Eisentraut [Tue, 3 Nov 2020 08:03:22 +0000 (09:03 +0100)]
Remove use of deprecated containment operators in tests
Switch @ to <@ and ~ to @> in gist-related tests. The old operator
names have been deprecated and will eventually (possibly soon) be
removed.
Author: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://www.postgresql.org/message-id/flat/
20201027032511.GF9241@telsasoft.com
Amit Kapila [Tue, 3 Nov 2020 03:08:27 +0000 (08:38 +0530)]
Fix typos.
Author: Hou Zhijie
Discussion: https://postgr.es/m/
855a9421839d402b8b351d273c89a8f8@G08CNEXMBPEKD05.g08.fujitsu.local
Tom Lane [Tue, 3 Nov 2020 02:11:50 +0000 (21:11 -0500)]
Fix unportable use of getnameinfo() in pg_hba_file_rules view.
fill_hba_line() thought it could get away with passing sizeof(struct
sockaddr_storage) rather than the actual addrlen previously returned
by getaddrinfo(). While that appears to work on many platforms,
it does not work on FreeBSD 11: you get back a failure, which leads
to the view showing NULL for the address and netmask columns in all
rows. The POSIX spec for getnameinfo() is pretty clearly on
FreeBSD's side here: you should pass the actual address length.
So it seems plausible that there are other platforms where this
coding also fails, and we just hadn't noticed.
Also, IMO the fact that getnameinfo() failure leads to a NULL output
is pretty bogus in itself. Our pg_getnameinfo_all() wrapper is
careful to emit "???" on failure, and we should use that in such
cases. NULL should only be emitted in rows that don't have IP
addresses.
Per bug #16695 from Peter Vandivier. Back-patch to v10 where this
code was added.
Discussion: https://postgr.es/m/16695-
a665558e2f630be7@postgresql.org
Tom Lane [Mon, 2 Nov 2020 19:34:34 +0000 (14:34 -0500)]
Remove special checks for pg_rewrite.ev_qual and ev_action being NULL.
make_ruledef() and make_viewdef() were coded to cope with possible
null-ness of these columns, but they've been marked BKI_FORCE_NOT_NULL
for some time. So there's not really any need to do more than what
we do for the other columns of pg_rewrite, i.e. just Assert that
we got non-null results.
(There is a school of thought that says Asserts aren't the thing
to do to check for corrupt data, but surely here is not the place
to start if we want such a policy.)
Also, remove long-dead-if-indeed-it-ever-wasn't-dead handling of
an empty actions list in make_ruledef(). That's an error case
and should be treated as such. (DO INSTEAD NOTHING is represented
by a CMD_NOTHING Query, not an empty list; cf transformRuleStmt.)
Kyotaro Horiguchi, some changes by me
Discussion: https://postgr.es/m/CAEudQApoA=tMTic6xEPYP_hsNZ8XtToVThK_0x7D_aFQYowq3w@mail.gmail.com
Tom Lane [Mon, 2 Nov 2020 16:57:28 +0000 (11:57 -0500)]
Rethink the generation rule for fmgroids.h macros.
Traditionally, the names of fmgroids.h macros for pg_proc OIDs
have been constructed from the prosrc field. But sometimes the
same C function underlies multiple pg_proc entries, forcing us
to make an arbitrary choice of which OID to reference; the other
entries are then not namable via fmgroids.h. Moreover, we could
not have macros at all for pg_proc entries that aren't for
C-coded functions.
Instead, use the proname field, and append the proargtypes field
(replacing inter-argument spaces with underscores) if proname is
not unique. Special-casing unique entries such as F_OIDEQ removes
the need to change a lot of code. Indeed, I can only find two
places in the tree that need to be adjusted; while this changes
quite a few existing entries in fmgroids.h, few of them are
referenced from C code.
With this patch, all entries in pg_proc.dat have macros in fmgroids.h.
Discussion: https://postgr.es/m/472274.
1604258384@sss.pgh.pa.us
Tom Lane [Mon, 2 Nov 2020 16:25:18 +0000 (11:25 -0500)]
Second thoughts on TOAST decompression.
On detecting a corrupted match tag, pglz_decompress() should just
summarily return -1. Breaking out of the loop, as I did in
dfc797730,
doesn't quite guarantee that will happen. Also, we can use
unlikely() on that check, just in case it helps.
Backpatch to v13, like the previous patch.
Peter Eisentraut [Mon, 2 Nov 2020 15:48:22 +0000 (16:48 +0100)]
Use PG_GETARG_TRANSACTIONID where appropriate
Some places were using PG_GETARG_UINT32 where PG_GETARG_TRANSACTIONID
would be more appropriate. (Of course, they are the same internally,
so there is no externally visible effect.) To do that, export
PG_GETARG_TRANSACTIONID outside of xid.c. We also export
PG_RETURN_TRANSACTIONID for symmetry, even though there are currently
no external users.
Author: Ashutosh Bapat <ashutosh.bapat@2ndquadrant.com>
Discussion: https://www.postgresql.org/message-id/flat/
d8f6bdd536df403b9b33816e9f7e0b9d@G08CNEXMBPEKD05.g08.fujitsu.local
Magnus Hagander [Mon, 2 Nov 2020 14:00:24 +0000 (15:00 +0100)]
Clarify temporary table name shadowing in CREATE TABLE docs
Author: David Johnston
Thomas Munro [Mon, 2 Nov 2020 06:50:45 +0000 (19:50 +1300)]
Track collation versions for indexes.
Record the current version of dependent collations in pg_depend when
creating or rebuilding an index. When accessing the index later, warn
that the index may be corrupted if the current version doesn't match.
Thanks to Douglas Doole, Peter Eisentraut, Christoph Berg, Laurenz Albe,
Michael Paquier, Robert Haas, Tom Lane and others for very helpful
discussion.
Author: Thomas Munro <thomas.munro@gmail.com>
Author: Julien Rouhaud <rjuju123@gmail.com>
Reviewed-by: Peter Eisentraut <peter.eisentraut@2ndquadrant.com> (earlier versions)
Discussion: https://postgr.es/m/CAEepm%3D0uEQCpfq_%2BLYFBdArCe4Ot98t1aR4eYiYTe%3DyavQygiQ%40mail.gmail.com
Thomas Munro [Mon, 2 Nov 2020 06:40:49 +0000 (19:40 +1300)]
Add pg_depend.refobjversion.
Provide a place for the version of referenced database objects to be
recorded. A follow-up commit will use this to record dependencies on
collation versions for indexes, but similar ideas for other kinds of
objects have also been mooted.
Author: Thomas Munro <thomas.munro@gmail.com>
Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Reviewed-by: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Discussion: https://postgr.es/m/CAEepm%3D0uEQCpfq_%2BLYFBdArCe4Ot98t1aR4eYiYTe%3DyavQygiQ%40mail.gmail.com
Thomas Munro [Mon, 2 Nov 2020 06:36:09 +0000 (19:36 +1300)]
Remove pg_collation.collversion.
This model couldn't be extended to cover the default collation, and
didn't have any information about the affected database objects when the
version changed. Remove, in preparation for a follow-up commit that
will add a new mechanism.
Author: Thomas Munro <thomas.munro@gmail.com>
Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Reviewed-by: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Discussion: https://postgr.es/m/CAEepm%3D0uEQCpfq_%2BLYFBdArCe4Ot98t1aR4eYiYTe%3DyavQygiQ%40mail.gmail.com
Heikki Linnakangas [Mon, 2 Nov 2020 10:51:46 +0000 (12:51 +0200)]
doc: Mention UNION/ORDER BY etc. keywords in section headers.
Most of the section and sub-section headers in the Queries chapter have
the keywords literally stated, but neither "Sorting Rows" nor "Combining
Rows" did. There's no rule that they must be, but it seems like a good
practice. The keywords will ring a bell to anyone with with even a little
bit of SQL experience.
David G. Johnston, per suggestion by bilge@scriptfusion.com
Discussion: https://www.postgresql.org/message-id/
159981394174.31338.
7014519396749859167%40wrigleys.postgresql.org
David Rowley [Mon, 2 Nov 2020 06:59:02 +0000 (19:59 +1300)]
Fix unstable partition_prune regression tests
This was broken recently by
a929e17e5. I'd failed to remember that
parallel tests should have their EXPLAIN output run through the
explain_parallel_append function so that the output is stable when
parallel workers fail to start.
fairywren was first to notice.
Reported-by: Michael Paquier
Discussion: https://postgr.es/m/
20201102062951.GB15770@paquier.xyz
Michael Paquier [Mon, 2 Nov 2020 06:14:41 +0000 (15:14 +0900)]
Fix some grammar and typos in comments and docs
The documentation fixes are backpatched down to where they apply.
Author: Justin Pryzby
Discussion: https://postgr.es/m/
20201031020801.GD3080@telsasoft.com
Backpatch-through: 9.6
Amit Kapila [Mon, 2 Nov 2020 02:48:18 +0000 (08:18 +0530)]
Use Enum for top level logical replication message types.
Logical replication protocol uses a single byte character to identify a
message type in logical replication protocol. The code uses string
literals for the same. Use Enum so that
1. All the string literals used can be found at a single place. This
makes it easy to add more types without the risk of conflicts.
2. It's easy to locate the code handling a given message type.
3. When used with switch statements, it is easy to identify the missing
cases using -Wswitch.
Author: Ashutosh Bapat
Reviewed-by: Kyotaro Horiguchi, Andres Freund, Peter Smith and Amit Kapila
Discussion: https://postgr.es/m/CAExHW5uPzQ7L0oAd_ENyvaiYMOPgkrAoJpE+ZY5-obdcVT6NPg@mail.gmail.com
David Rowley [Mon, 2 Nov 2020 00:46:56 +0000 (13:46 +1300)]
Allow run-time pruning on nested Append/MergeAppend nodes
Previously we only tagged on the required information to allow the
executor to perform run-time partition pruning for Append/MergeAppend
nodes belonging to base relations. It was thought that nested
Append/MergeAppend nodes were just about always pulled up into the
top-level Append/MergeAppend and that making the run-time pruning info for
any sub Append/MergeAppend nodes was a waste of time. However, that was
likely badly thought through.
Some examples of cases we're unable to pullup nested Append/MergeAppends
are: 1) Parallel Append nodes with a mix of parallel and non-parallel
paths into a Parallel Append. 2) When planning an ordered Append scan a
sub-partition which is unordered may require a nested MergeAppend path to
ensure sub-partitions don't mix up the order of tuples being fed into the
top-level Append.
Unfortunately, it was not just as simple as removing the lines in
createplan.c which were purposefully not building the run-time pruning
info for anything but RELOPT_BASEREL relations. The code in
add_paths_to_append_rel() was far too sloppy about which partitioned_rels
it included for the Append/MergeAppend paths. The original code there
would always assume accumulate_append_subpath() would pull each sub-Append
and sub-MergeAppend path into the top-level path. While it does not
appear that there were any actual bugs caused by having the additional
partitioned table RT indexes recorded, what it did mean is that later in
planning, when we built the run-time pruning info that we wasted effort
and built PartitionedRelPruneInfos for partitioned tables that we had no
subpaths for the executor to run-time prune.
Here we tighten that up so that partitioned_rels only ever contains the RT
index for partitioned tables which actually have subpaths in the given
Append/MergeAppend. We can now Assert that every PartitionedRelPruneInfo
has a non-empty present_parts. That should allow us to catch any weird
corner cases that have been missed.
In passing, it seems there is no longer a good reason to have the
AppendPath and MergeAppendPath's partitioned_rel fields a List of IntList.
We can simply have a List of Relids instead. This is more compact in
memory and faster to add new members to. We still know which is the root
level partition as these always have a lower relid than their children.
Previously this field was used for more things, but run-time partition
pruning now remains the only user of it and it has no need for a List of
IntLists.
Here we also get rid of the RelOptInfo partitioned_child_rels field. This
is what was previously used to (sometimes incorrectly) set the
Append/MergeAppend path's partitioned_rels field. That was the only usage
of that field, so we can happily just remove it.
I also couldn't resist changing some nearby code to make use of the newly
added for_each_from macro so we can skip the first element in the list
without checking if the current item was the first one on each
iteration.
A bug report from Andreas Kretschmer prompted all this work, however,
after some consideration, I'm not personally classing this as a bug fix.
So no backpatch. In Andreas' test case, it just wasn't that clear that
there was a nested Append since the top-level Append just had a single
sub-path which was pulled up a level, per
8edd0e794.
Author: David Rowley
Reviewed-by: Amit Langote
Discussion: https://postgr.es/m/flat/CAApHDvqSchs%2BubdybcfFaSPB%2B%2BEA7kqMaoqajtP0GtZvzOOR3g%40mail.gmail.com
Tom Lane [Sun, 1 Nov 2020 23:38:42 +0000 (18:38 -0500)]
Fix two issues in TOAST decompression.
pglz_maximum_compressed_size() potentially underestimated the amount
of compressed data required to produce N bytes of decompressed data;
this is a fault in commit
11a078cf8.
Separately from that, pglz_decompress() failed to protect itself
against corrupt compressed data, particularly off == 0 in a match
tag. Commit
c60e520f6 turned such a situation into an infinite loop,
where before it'd just have resulted in garbage output.
The combination of these two bugs seems like it may explain bug #16694
from Tom Vijlbrief, though it's impossible to be quite sure without
direct inspection of the failing session. (One needs to assume that
the pglz_maximum_compressed_size() bug caused us to fail to fetch the
second byte of a match tag, and what happened to be there instead was
a zero. The reported infinite loop is hard to explain without off == 0,
though.)
Aside from fixing the bugs, rewrite associated comments for more
clarity.
Back-patch to v13 where both these commits landed.
Discussion: https://postgr.es/m/16694-
f107871e499ec114@postgresql.org
Tom Lane [Sun, 1 Nov 2020 16:26:16 +0000 (11:26 -0500)]
Avoid null pointer dereference if error result lacks SQLSTATE.
Although error results received from the backend should always have
a SQLSTATE field, ones generated by libpq won't, making this code
vulnerable to a crash after, say, untimely loss of connection.
Noted by Coverity.
Oversight in commit
403a3d91c. Back-patch to 9.5, as that was.
Michael Paquier [Sun, 1 Nov 2020 12:22:07 +0000 (21:22 +0900)]
Preserve index data in pg_statistic across REINDEX CONCURRENTLY
Statistics associated to an index got lost after running REINDEX
CONCURRENTLY, while the non-concurrent case preserves these correctly.
The concurrent and non-concurrent operations need to be consistent for
the end-user, and missing statistics would force to wait for a new
analyze to happen, which could take some time depending on the activity
of the existing autovacuum workers. This issue is fixed by copying any
existing entries in pg_statistic associated to the old index to the new
one. Note that this copy is already done with the data of the index in
the stats collector.
Reported-by: Fabrízio de Royes Mello
Author: Michael Paquier, Fabrízio de Royes Mello
Reviewed-by: Justin Pryzby
Discussion: https://postgr.es/m/CAFcNs+qpFPmiHd1oTXvcPdvAHicJDA9qBUSujgAhUMJyUMb+SA@mail.gmail.com
Backpatch-through: 12
Michael Paquier [Sun, 1 Nov 2020 10:22:59 +0000 (19:22 +0900)]
Add error code for encryption failure in pgcrypto
PXE_DECRYPT_FAILED exists already for decryption errors, and an
equivalent for encryption did not exist. There is one code path that
deals with such failures for OpenSSL but it used PXE_ERR_GENERIC, which
was inconsistent. This switches this code path to use the new error
PXE_ENCRYPT_FAILED instead of PXE_ERR_GENERIC, making the code used for
encryption more consistent with the decryption.
Author: Daniel Gustafsson
Discussion: https://postgr.es/m/
03049139-CB7A-436E-B71B-
42696D3E2EF7@yesql.se
Noah Misch [Sat, 31 Oct 2020 15:47:02 +0000 (08:47 -0700)]
Set debug_query_string in worker_spi.
This makes elog.c emit the string, which is good practice for a
background worker that executes SQL strings.
Reviewed by Tom Lane.
Discussion: https://postgr.es/m/
20201014022636.GA1962668@rfd.leadboat.com
Noah Misch [Sat, 31 Oct 2020 15:43:28 +0000 (08:43 -0700)]
Reproduce debug_query_string==NULL on parallel workers.
Certain background workers initiate parallel queries while
debug_query_string==NULL, at which point they attempted strlen(NULL) and
died to SIGSEGV. Older debug_query_string observers allow NULL, so do
likewise in these newer ones. Back-patch to v11, where commit
7de4a1bcc56f494acbd0d6e70781df877dc8ecb5 introduced the first of these.
Discussion: https://postgr.es/m/
20201014022636.GA1962668@rfd.leadboat.com
Tom Lane [Fri, 30 Oct 2020 21:00:59 +0000 (17:00 -0400)]
Fix assertion failure in check_new_partition_bound().
Commit
6b2c4e59d was overly confident about not being able to see
a negative cmpval result from partition_range_bsearch(). Adjust
the code to cope with that.
Report and patch by Amul Sul; some additional cosmetic changes by me
Discussion: https://postgr.es/m/CAAJ_b97WCO=EyVA7fKzc86kKfojHXLU04_zs7-7+yVzm=-1QkQ@mail.gmail.com
Heikki Linnakangas [Fri, 30 Oct 2020 17:30:19 +0000 (19:30 +0200)]
Fix missing validation for the new GiST sortsupport functions.
Because of this, if you tried to create an operator family with the new
sortsupport function, you got an error:
ERROR: support function number 11 is invalid for access method gist
We missed this in commit
16fa9b2b30 that added the sortsupport function,
because it only added sortsupport to a built-in operator family.
Author: Andrey Borodin
Discussion: https://www.postgresql.org/message-id/
3520A18A-5C38-4697-A2E3-
F3BDE3496CD5%40yandex-team.ru
Tom Lane [Fri, 30 Oct 2020 14:38:49 +0000 (10:38 -0400)]
Doc: clarify description for pg_constraint.convalidated.
Jimmy Angelakos
Discussion: https://postgr.es/m/CABgVKCW_zPnvFXn24FTF0299_yU6+1p6JRUc0xpiZFWEXH1_jg@mail.gmail.com
Tom Lane [Thu, 29 Oct 2020 19:28:14 +0000 (15:28 -0400)]
Stabilize timetz test across DST transitions.
The timetz test cases I added in commit
a9632830b were unintentionally
sensitive to whether or not DST is active in the PST8PDT time zone.
Thus, they'll start failing this coming weekend, as reported by
Bernhard M. Wiedemann in bug #16689. Fortunately, DST-awareness is
not significant to the purpose of these test cases, so we can just
force them all to PDT (DST hours) to preserve stability of the
results.
Back-patch to v10, as the prior patch was.
Discussion: https://postgr.es/m/16689-
57701daa23b377bf@postgresql.org
Tom Lane [Thu, 29 Oct 2020 17:33:38 +0000 (13:33 -0400)]
Don't use custom OID symbols in pg_type.dat, either.
On the same reasoning as in commit
36b931214, forbid using custom
oid_symbol macros in pg_type as well as pg_proc, so that we always
rely on the predictable macro names generated by genbki.pl.
We do continue to grant grandfather status to the names CASHOID and
LSNOID, although those are now considered deprecated aliases for the
preferred names MONEYOID and PG_LSNOID. This is because there's
likely to be client-side code using the old names, and this bout of
neatnik-ism doesn't quite seem worth breaking client code.
There might be a case for grandfathering EVTTRIGGEROID, too, since
externally-maintained PLs may reference that symbol. But renaming
such references to EVENT_TRIGGEROID doesn't seem like a particularly
heavy lift --- we make far more significant backend API changes in
every major release. For now I didn't add that, but we could
reconsider if there's pushback.
The other names changed here seem pretty unlikely to have any outside
uses. Again, we could add alias macros if there are complaints, but
for now I didn't.
As before, no need for a catversion bump.
John Naylor
Discussion: https://postgr.es/m/CAFBsxsHpCbjfoddNGpnnnY5pHwckWfiYkMYSF74PmP1su0+ZOw@mail.gmail.com
Andres Freund [Thu, 29 Oct 2020 04:48:38 +0000 (21:48 -0700)]
Fix wrong data table horizon computation during backend startup.
When ComputeXidHorizons() was called before MyDatabaseOid is set,
e.g. because a dead row in a shared relation is encountered during
InitPostgres(), the horizon for normal tables was computed too
aggressively, ignoring all backends connected to a database.
During subsequent pruning in a data table the too aggressive horizon
could end up still being used, possibly leading to still needed tuples
being removed. Not good.
This is a bug in
dc7420c2c92, which the test added in
94bc27b5768 made
visible, if run with force_parallel_mode set to regress. In that case
the bug is reliably triggered, because "pruning_query" is run in a
parallel worker and the start of that parallel worker is likely to
encounter a dead row in pg_database.
The fix is trivial: Compute a more pessimistic data table horizon if
MyDatabaseId is not yet known.
Author: Andres Freund
Discussion: https://postgr.es/m/
20201029040030.p4osrmaywhqaesd4@alap3.anarazel.de
Amit Kapila [Thu, 29 Oct 2020 03:41:51 +0000 (09:11 +0530)]
Track statistics for streaming of changes from ReorderBuffer.
This adds the statistics about transactions streamed to the decoding
output plugin from ReorderBuffer. Users can query the
pg_stat_replication_slots view to check these stats and call
pg_stat_reset_replication_slot to reset the stats of a particular slot.
Users can pass NULL in pg_stat_reset_replication_slot to reset stats of
all the slots.
Commit
9868167500 has added the basic infrastructure to capture the stats
of slot and this commit extends the statistics collector to track
additional information about slots.
Bump the catversion as we have added new columns in the catalog entry.
Author: Ajin Cherian and Amit Kapila
Reviewed-by: Sawada Masahiko and Dilip Kumar
Discussion: https://postgr.es/m/CAA4eK1+chpEomLzgSoky-D31qev19AmECNiEAietPQUGEFhtVA@mail.gmail.com
Andres Freund [Thu, 29 Oct 2020 00:53:41 +0000 (17:53 -0700)]
Centralize horizon determination for temp tables, fixing bug due to skew.
This fixes a bug in the edge case where, for a temp table, heap_page_prune()
can end up with a different horizon than heap_vacuum_rel(). Which can trigger
errors like "ERROR: cannot freeze committed xmax ...".
The bug was introduced due to interaction of
a7212be8b9e "Set cutoff xmin more
aggressively when vacuuming a temporary table." with
dc7420c2c92 "snapshot
scalability: Don't compute global horizons while building snapshots.".
The problem is caused by lazy_scan_heap() assuming that the only reason its
HeapTupleSatisfiesVacuum() call would return HEAPTUPLE_DEAD is if the tuple is
a HOT tuple, or if the tuple's inserting transaction has aborted since the
heap_page_prune() call. But after
a7212be8b9e that was also possible in other
cases for temp tables, because heap_page_prune() uses a different visibility
test after
dc7420c2c92.
The fix is fairly simple: Move the special case logic for temp tables from
vacuum_set_xid_limits() to the infrastructure introduced in
dc7420c2c92. That
ensures that the horizon used for pruning is at least as aggressive as the one
used by lazy_scan_heap(). The concrete horizon used for temp tables is
slightly different than the logic in
dc7420c2c92, but should always be as
aggressive as before (see comments).
A significant benefit to centralizing the logic procarray.c is that now the
more aggressive horizons for temp tables does not just apply to VACUUM but
also to e.g. HOT pruning and the nbtree killtuples logic.
Because isTopLevel is not needed by vacuum_set_xid_limits() anymore, I
undid the the related changes from
a7212be8b9e.
This commit also adds an isolation test ensuring that the more aggressive
vacuuming and pruning of temp tables keeps working.
Debugged-By: Amit Kapila <amit.kapila16@gmail.com>
Debugged-By: Tom Lane <tgl@sss.pgh.pa.us>
Debugged-By: Ashutosh Sharma <ashu.coek88@gmail.com>
Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/
20201014203103.72oke6hqywcyhx7s@alap3.anarazel.de
Discussion: https://postgr.es/m/
20201015083735.derdzysdtqdvxshp@alap3.anarazel.de
Michael Paquier [Thu, 29 Oct 2020 00:17:34 +0000 (09:17 +0900)]
Fix incorrect placement of pfree() in pg_relation_check_pages()
This would cause the function to crash when more than one page is
considered as broken and reported in the SRF.
Reported-by: Noriyoshi Shinoda
Discussion: https://postgr.es/m/TU4PR8401MB11523D42C315AAF822E74275EE170@TU4PR8401MB1152.NAMPRD84.PROD.OUTLOOK.COM
Tom Lane [Wed, 28 Oct 2020 21:03:05 +0000 (17:03 -0400)]
Doc: clean up pg_relation_check_pages() documentation.
Commit
f2b883969 did not get the memo about the new formatting
style for tables documenting built-in functions. I noticed because
of a PDF build warning about an overwidth table.
Tom Lane [Wed, 28 Oct 2020 20:31:40 +0000 (16:31 -0400)]
Doc: clean up verify_heapam() documentation.
I started with the intention of just suppressing a PDF build warning
by removing the example output, but ended up doing more: correcting
factual errors in the function's signature, moving a bunch of
generalized handwaving into the "Using amcheck Effectively" section
which seemed a better place for it, and improving wording and markup
a little bit.
Discussion: https://postgr.es/m/732904.
1603728748@sss.pgh.pa.us