Tom Lane [Sun, 6 Sep 2020 16:55:13 +0000 (12:55 -0400)]
Fix misleading error message about inconsistent moving-aggregate types.
We reported the wrong types when complaining that an aggregate's
moving-aggregate implementation is inconsistent with its regular
implementation.
This was wrong since the feature was introduced, so back-patch
to all supported branches.
Jeff Janes
Discussion: https://postgr.es/m/CAMkU=1x808LH=LPhZp9mNSP0Xd1xDqEd+XeGcvEe48dfE6xV=A@mail.gmail.com
Tom Lane [Sun, 6 Sep 2020 15:50:41 +0000 (11:50 -0400)]
Remove useless lstat() call in pg_rewind.
This is duplicative of an lstat that was just done by the calling
function (traverse_datadir), besides which we weren't really doing
anything with the results. There's not much point in checking to
see if someone removed the file since the previous lstat, since the
FILE_ACTION_REMOVE code would have to deal with missing-file cases
anyway. Moreover, the "exists = false" assignment was a dead store;
nothing was done with that value later.
A syscall saved is a syscall earned, so back-patch to 9.5
where this code was introduced.
Discussion: https://postgr.es/m/
1221796.
1599329320@sss.pgh.pa.us
Tom Lane [Sat, 5 Sep 2020 01:01:59 +0000 (21:01 -0400)]
Make new authentication test case more robust.
I happened to notice that the new test case I added in
b55b4dad9
falls over if one runs "make check" repeatedly; though not in branches
after v10. That's because it was assuming that tmp_check/pgpass
wouldn't exist already. However, it's only been since v11 that the
Makefiles forcibly remove all of tmp_check/ before starting a TAP run.
This fix to unlink the file is therefore strictly necessary only in
v10 ... but it seems wisest to do it across the board, rather than
let the test rely on external logic to get the conditions right.
Tom Lane [Sat, 5 Sep 2020 00:20:05 +0000 (20:20 -0400)]
Fix over-eager ping'ing in logical replication receiver.
Commit
3f60f690f only partially fixed the broken-status-tracking
issue in LogicalRepApplyLoop: we need ping_sent to have the same
lifetime as last_recv_timestamp. The effects are much less serious
than what that commit fixed, though. AFAICS this would just lead to
extra ping requests being sent, once per second until the sender
responds. Still, it's a bug, so backpatch to v10 as before.
Discussion: https://postgr.es/m/959627.
1599248476@sss.pgh.pa.us
Andrew Dunstan [Fri, 4 Sep 2020 17:53:09 +0000 (13:53 -0400)]
Collect attribute data on extension owned tables being dumped
If this data is not collected, pg_dump segfaults if asked for column
inserts.
Fix by Fabrízio de Royes Mello
Backpatch to release 12 where the bug was introduced.
Bruce Momjian [Fri, 4 Sep 2020 17:27:52 +0000 (13:27 -0400)]
C comment: correct use of 64-"byte" cache line size
Reported-by: Kelly Min
Discussion: https://postgr.es/m/CAPSbxatOiQO90LYpSC3+svAU9-sHgDfEP4oFhcEUt_X=DqFA9g@mail.gmail.com
Backpatch-through: 9.5
Tom Lane [Fri, 4 Sep 2020 16:40:28 +0000 (12:40 -0400)]
Fix rare deadlock failure in create_am regression test.
The "DROP ACCESS METHOD gist2" test will require locking the index
to be dropped and then its table; while most ordinary operations
lock a table first then its index. While no concurrent test scripts
should be touching fast_emp4000, autovacuum might chance to be
processing that table when the DROP runs, resulting in a deadlock
failure. This is pretty rare but we see it in the buildfarm from
time to time.
To fix, acquire a lock on fast_emp4000 before issuing the DROP.
Since the point of the exercise is mostly to prevent buildfarm
failures, back-patch to 9.6 where this test was introduced.
Discussion: https://postgr.es/m/839004.
1599185607@sss.pgh.pa.us
Tom Lane [Thu, 3 Sep 2020 20:52:09 +0000 (16:52 -0400)]
Avoid lockup of a parallel worker when reporting a long error message.
Because sigsetjmp() will restore the initial state with signals blocked,
the code path in bgworker.c for reporting an error and exiting would
execute that way. Usually this is fairly harmless; but if a parallel
worker had an error message exceeding the shared-memory communication
buffer size (16K) it would lock up, because it would wait for a
resume-sending signal from its parallel leader which it would never
detect.
To fix, just unblock signals at the appropriate point.
This can be shown to fail back to 9.6. The lack of parallel query
infrastructure makes it difficult to provide a simple test case for
9.5; but I'm pretty sure the issue exists in some form there as well,
so apply the code change there too.
Vignesh C, reviewed by Bharath Rupireddy, Robert Haas, and myself
Discussion: https://postgr.es/m/CALDaNm1d1hHPZUg3xU4XjtWBOLCrA+-2cJcLpw-cePZ=GgDVfA@mail.gmail.com
Alvaro Herrera [Wed, 2 Sep 2020 00:43:23 +0000 (20:43 -0400)]
Fix typo in comment
Introduced by
8b08f7d4820f; backpatch to 11.
Discussion: https://postgr.es/m/
20200812214918[email protected]
Bruce Momjian [Tue, 1 Sep 2020 21:00:10 +0000 (17:00 -0400)]
doc: clarify that max_wal_size is "during" checkpoints
Previous wording was "between".
Reported-by: Pavel Luzanov
Discussion: https://postgr.es/m/
26906a54-d7cb-2f8e-eed7-
e31660024694@postgrespro.ru
Backpatch-through: 9.5
Alvaro Herrera [Tue, 1 Sep 2020 17:40:43 +0000 (13:40 -0400)]
Raise error on concurrent drop of partitioned index
We were already raising an error for DROP INDEX CONCURRENTLY on a
partitioned table, albeit a different and confusing one:
ERROR: DROP INDEX CONCURRENTLY must be first action in transaction
Change that to throw a more comprehensible error:
ERROR: cannot drop partitioned index \"%s\" concurrently
Michael Paquier authored the test case for indexes on temporary
partitioned tables.
Backpatch to 11, where indexes on partitioned tables were added.
Reported-by: Jan Mussler
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/16594-
d2956ca909585067@postgresql.org
Tom Lane [Tue, 1 Sep 2020 17:14:44 +0000 (13:14 -0400)]
Teach libpq to handle arbitrary-length lines in .pgpass files.
Historically there's been a hard-wired assumption here that no line of
a .pgpass file could be as long as NAMEDATALEN*5 bytes. That's a bit
shaky to start off with, because (a) there's no reason to suppose that
host names fit in NAMEDATALEN, and (b) this figure fails to allow for
backslash escape characters. However, it fails completely if someone
wants to use a very long password, and we're now hearing reports of
people wanting to use "security tokens" that can run up to several
hundred bytes. Another angle is that the file is specified to allow
comment lines, but there's no reason to assume that long comment lines
aren't possible.
Rather than guessing at what might be a more suitable limit, let's
replace the fixed-size buffer with an expansible PQExpBuffer. That
adds one malloc/free cycle to the typical use-case, but that's surely
pretty cheap relative to the I/O this code has to do.
Also, add TAP test cases to exercise this code, because there was no
test coverage before.
This reverts most of commit
2eb3bc588, as there's no longer a need for
a warning message about overlength .pgpass lines. (I kept the explicit
check for comment lines, though.)
In HEAD and v13, this also fixes an oversight in
74a308cf5: there's not
much point in explicit_bzero'ing the line buffer if we only do so in two
of the three exit paths.
Back-patch to all supported branches, except that the test case only
goes back to v10 where src/test/authentication/ was added.
Discussion: https://postgr.es/m/
4187382.
1598909041@sss.pgh.pa.us
Bruce Momjian [Mon, 31 Aug 2020 22:33:37 +0000 (18:33 -0400)]
doc: add commas after 'i.e.' and 'e.g.'
This follows the American format,
https://jakubmarian.com/comma-after-i-e-and-e-g/. There is no intention
of requiring this format for future text, but making existing text
consistent every few years makes sense.
Discussion: https://postgr.es/m/
20200825183619[email protected]
Backpatch-through: 9.5
Bruce Momjian [Mon, 31 Aug 2020 21:51:31 +0000 (17:51 -0400)]
C comment: remove mention of use of t_hoff WAL structure member
Reported-by: Antonin Houska
Discussion: https://postgr.es/m/21643.
1595353537@antos
Backpatch-through: 9.5
Bruce Momjian [Mon, 31 Aug 2020 21:36:22 +0000 (17:36 -0400)]
pg_upgrade doc: mention saving postgresql.conf.auto files
Also mention files included by postgresql.conf.
Reported-by: Álvaro Herrera
Discussion: https://postgr.es/m/
08AD4526-75AB-457B-B2DD-
099663F28040@yesql.se
Backpatch-through: 9.5
Bruce Momjian [Mon, 31 Aug 2020 21:05:53 +0000 (17:05 -0400)]
docs: in mapping SQL to C data types, timestamp isn't a pointer
It is an int64.
Reported-by: [email protected]
Discussion: https://postgr.es/m/
159845038271.24995.
15682121015698255155@wrigleys.postgresql.org
Backpatch-through: 9.5
Bruce Momjian [Mon, 31 Aug 2020 20:59:58 +0000 (16:59 -0400)]
doc: cross-link file-fdw and CSV config log sections
There is an file-fdw example that reads the server config file, so cross
link them.
Reported-by: Oleg Samoilov
Discussion: https://postgr.es/m/
159800192078.2886.
10431506404995508950@wrigleys.postgresql.org
Backpatch-through: 9.5
Bruce Momjian [Mon, 31 Aug 2020 20:21:03 +0000 (16:21 -0400)]
docs: clarify intermediate certificate creation instructions
Specifically, explain the v3_ca openssl specification.
Discussion: https://postgr.es/m/
20200824175653[email protected]
Backpatch-through: 9.5
Bruce Momjian [Mon, 31 Aug 2020 19:23:19 +0000 (15:23 -0400)]
docs: replace "stable storage" with "durable" in descriptions
For PG, "durable storage" has a clear meaning, while "stable storage"
does not, so use the former.
Discussion: https://postgr.es/m/
20200817165222[email protected]
Backpatch-through: 9.5
Bruce Momjian [Mon, 31 Aug 2020 17:49:17 +0000 (13:49 -0400)]
doc: improve description of subscripting of arrays
It wasn't clear the non-integers are cast to integers for subscripting,
rather than throwing an error.
Reported-by: [email protected]
Discussion: https://postgr.es/m/
159538675800.624.
7728794628229799531@wrigleys.postgresql.org
Backpatch-through: 9.5
Bruce Momjian [Mon, 31 Aug 2020 17:43:04 +0000 (13:43 -0400)]
docs: improve 'capitals' inheritance example
Adds constraints and improves wording.
Reported-by: [email protected]
Discussion: https://postgr.es/m/
159586122762.680.
1361378513036616007@wrigleys.postgresql.org
Backpatch-through: 9.5
Bruce Momjian [Mon, 31 Aug 2020 17:20:04 +0000 (13:20 -0400)]
doc: clarify the useful features of procedures
This was not clearly documented when procedures were added in PG 11.
Reported-by: Robin Abbi
Discussion: https://postgr.es/m/CAGmg_NX327KKVuJmbWZD=pGutYFxzZjX1rU+3ji8UuX=8ONn9Q@mail.gmail.com
Backpatch-through: 11
Magnus Hagander [Mon, 31 Aug 2020 11:03:54 +0000 (13:03 +0200)]
Fix docs bug stating file_fdw requires absolute paths
It has always (since the first commit) worked with relative paths, so
use the same wording as other parts of the documentation.
Author: Bruce Momjian
Discussion: https://postgr.es/m/CABUevExx-hm=cit+A9LeKBH39srvk8Y2tEZeEAj5mP8YfzNKUg@mail.gmail.com
Tom Lane [Sun, 30 Aug 2020 20:03:19 +0000 (16:03 -0400)]
Mark factorial operator, and postfix operators in general, as deprecated.
Back-patch key parts of
4c5cf5431 and
6ca547cf7 into stable branches.
I didn't touch pg_description entries here, so it's purely a docs
change; and I didn't fool with any examples either. The main point
is so that anyone who's wondering if factorial() exists in the stable
branches will be reassured.
Mark Dilger and John Naylor, with some adjustments by me
Discussion: https://postgr.es/m/
BE2DF53D-251A-4E26-972F-
930E523580E9@enterprisedb.com
Tom Lane [Thu, 27 Aug 2020 21:36:13 +0000 (17:36 -0400)]
Fix code for re-finding scan position in a multicolumn GIN index.
collectMatchBitmap() needs to re-find the index tuple it was previously
looking at, after transiently dropping lock on the index page it's on.
The tuple should still exist and be at its prior position or somewhere
to the right of that, since ginvacuum never removes tuples but
concurrent insertions could add one. However, there was a thinko in
that logic, to the effect of expecting any inserted tuples to have the
same index "attnum" as what we'd been scanning. Since there's no
physical separation of tuples with different attnums, it's not terribly
hard to devise scenarios where this fails, leading to transient "lost
saved point in index" errors. (While I've duplicated this with manual
testing, it seems impossible to make a reproducible test case with our
available testing technology.)
Fix by just continuing the scan when the attnum doesn't match.
While here, improve the error message used if we do fail, so that it
matches the wording used in btree for a similar case.
collectMatchBitmap()'s posting-tree code path was previously not
exercised at all by our regression tests. While I can't make
a regression test that exhibits the bug, I can at least improve
the code coverage here, so do that. The test case I made for this
is an extension of one added by
4b754d6c1, so it only works in
HEAD and v13; didn't seem worth trying hard to back-patch it.
Per bug #16595 from Jesse Kinkead. This has been broken since
multicolumn capability was added to GIN (commit
27cb66fdf),
so back-patch to all supported branches.
Discussion: https://postgr.es/m/16595-
633118be8eef9ce2@postgresql.org
Bruce Momjian [Tue, 25 Aug 2020 13:53:12 +0000 (09:53 -0400)]
docs: client certificates are always sent to the server
They are not "requested" by the server.
Reported-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/
20200825.155320.
986648039251743210[email protected]
Backpatch-through: 9.5
Peter Eisentraut [Tue, 25 Aug 2020 05:29:05 +0000 (07:29 +0200)]
doc: Fix up title case
This fixes some instances that were missed in earlier processings and
that now look a bit strange because they are inconsistent with nearby
titles.
Tom Lane [Sat, 22 Aug 2020 18:46:40 +0000 (14:46 -0400)]
Avoid pushing quals down into sub-queries that have grouping sets.
The trouble with doing this is that an apparently-constant subquery
output column isn't really constant if it is a grouping column that
appears in only some of the grouping sets. A qual using such a
column would be subject to incorrect const-folding after push-down,
as seen in bug #16585 from Paul Sivash.
To fix, just disable qual pushdown altogether if the sub-query has
nonempty groupingSets. While we could imagine far less restrictive
solutions, there is not much point in working harder right now,
because subquery_planner() won't move HAVING clauses to WHERE within
such a subquery. If the qual stays in HAVING it's not going to be
a lot more useful than if we'd kept it at the outer level.
Having said that, this restriction could be removed if we used a
parsetree representation that distinguished such outputs from actual
constants, which is something I hope to do in future. Hence, make
the patch a minimal addition rather than integrating it more tightly
(e.g. by renumbering the existing items in subquery_is_pushdown_safe's
comment).
Back-patch to 9.5 where grouping sets were introduced.
Discussion: https://postgr.es/m/16585-
9d8c340d23ade8c1@postgresql.org
Bruce Momjian [Sat, 22 Aug 2020 00:23:09 +0000 (20:23 -0400)]
docs: improve description of how to handle multiple databases
This is a redesign of the intro to the managing databases chapter.
Discussion: https://postgr.es/m/
159586122762.680.
1361378513036616007@wrigleys.postgresql.org
Author: David G. Johnston
Backpatch-through: 9.5
Bruce Momjian [Fri, 21 Aug 2020 22:29:37 +0000 (18:29 -0400)]
docs: add COMMENT examples for new features, rename rtree
Reported-by: Jürgen Purtz
Discussion: https://postgr.es/m/
15ec5428-d46a-1725-f38d-
44986a977abb@purtz.de
Author: Jürgen Purtz
Backpatch-through: 11
Tom Lane [Fri, 21 Aug 2020 19:00:43 +0000 (15:00 -0400)]
Fix handling of CREATE TABLE LIKE with inheritance.
If a CREATE TABLE command uses both LIKE and traditional inheritance,
Vars in CHECK constraints and expression indexes that are absorbed
from a LIKE parent table tended to get mis-numbered, resulting in
wrong answers and/or bizarre error messages (though probably not any
actual crashes, thanks to validation occurring in the executor).
In v12 and up, the same could happen to Vars in GENERATED expressions,
even in cases with no LIKE clause but multiple traditional-inheritance
parents.
The cause of the problem for LIKE is that parse_utilcmd.c supposed
it could renumber such Vars correctly during transformCreateStmt(),
which it cannot since we have not yet accounted for columns added via
inheritance. Fix that by postponing processing of LIKE INCLUDING
CONSTRAINTS, DEFAULTS, GENERATED, INDEXES till after we've performed
DefineRelation().
The error with GENERATED and multiple inheritance is a simple oversight
in MergeAttributes(); it knows it has to renumber Vars in inherited
CHECK constraints, but forgot to apply the same processing to inherited
GENERATED expressions (a/k/a defaults).
Per bug #16272 from Tom Gottfried. The non-GENERATED variants of the
issue are ancient, presumably dating right back to the addition of
CREATE TABLE LIKE; hence back-patch to all supported branches.
Discussion: https://postgr.es/m/16272-
6e32da020e9a9381@postgresql.org
David Rowley [Thu, 20 Aug 2020 21:35:24 +0000 (09:35 +1200)]
Fix a few typos in JIT comments and README
Reviewed-by: Abhijit Menon-Sen
Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/CAApHDvobgmCs6CohqhKTUf7D8vffoZXQTCBTERo9gbOeZmvLTw%40mail.gmail.com
Backpatch-through: 11, where JIT was added
Alvaro Herrera [Thu, 20 Aug 2020 17:49:04 +0000 (13:49 -0400)]
Revise REINDEX CONCURRENTLY recovery instructions
When the leftover invalid index is "ccold", there's no need to re-run
the command. Reword the instructions to make that explicit.
Backpatch to 12, where REINDEX CONCURRENTLY appeared.
Author: Álvaro Herrera
Reviewed-by: Michael Paquier
Reviewed-by: Julien Rouhaud
Discussion: https://postgr.es/m/20200819211312[email protected]
Tom Lane [Wed, 19 Aug 2020 18:07:49 +0000 (14:07 -0400)]
Suppress unnecessary RelabelType nodes in yet more cases.
Commit
a477bfc1d fixed eval_const_expressions() to ensure that it
didn't generate unnecessary RelabelType nodes, but I failed to notice
that some other places in the planner had the same issue. Really
noplace in the planner should be using plain makeRelabelType(), for
fear of generating expressions that should be equal() to semantically
equivalent trees, but aren't.
An example is that because canonicalize_ec_expression() failed
to be careful about this, we could end up with an equivalence class
containing both a plain Const, and a Const-with-RelabelType
representing exactly the same value. So far as I can tell this led to
no visible misbehavior, but we did waste a bunch of cycles generating
and evaluating "Const = Const-with-RelabelType" to prove such entries
are redundant.
Hence, move the support function added by
a477bfc1d to where it can
be more generally useful, and use it in the places where planner code
previously used makeRelabelType.
Back-patch to v12, like the previous patch. While I have no concrete
evidence of any real misbehavior here, it's certainly possible that
I overlooked a case where equivalent expressions that aren't equal()
could cause a user-visible problem. In any case carrying extra
RelabelType nodes through planning to execution isn't very desirable.
Discussion: https://postgr.es/m/
1311836.
1597781384@sss.pgh.pa.us
Heikki Linnakangas [Tue, 18 Aug 2020 10:13:09 +0000 (13:13 +0300)]
Avoid non-constant format string argument to fprintf().
As Tom Lane pointed out, it could defeat the compiler's printf() format
string verification.
Backpatch to v12, like that patch that introduced it.
Discussion: https://www.postgresql.org/message-id/
1069283.
1597672779%40sss.pgh.pa.us
Alvaro Herrera [Mon, 17 Aug 2020 20:20:06 +0000 (16:20 -0400)]
Disable autovacuum for BRIN test table
This should improve stability in the tests.
Per buildfarm member hyrax (CLOBBER_CACHE_ALWAYS) via Tom Lane.
Discussion: https://postgr.es/m/871534.
1597503261@sss.pgh.pa.us
Tom Lane [Mon, 17 Aug 2020 19:40:07 +0000 (15:40 -0400)]
Doc: fix description of UNION/CASE/etc type unification.
The description of what select_common_type() does was not terribly
accurate. Improve it.
David Johnston and Tom Lane
Discussion: https://postgr.es/m/
1019930.
1597613200@sss.pgh.pa.us
Heikki Linnakangas [Mon, 17 Aug 2020 06:27:29 +0000 (09:27 +0300)]
Fix printing last progress report line in client programs.
A number of client programs have a "--progress" option that when printing
to a TTY, updates the current line by printing a '\r' and overwriting it.
After the last line, '\n' needs to be printed to move the cursor to the
next line. pg_basebackup and pgbench got this right, but pg_rewind and
pg_checksums were slightly wrong. pg_rewind printed the newline to stdout
instead of stderr, and pg_checksums printed the newline even when not
printing to a TTY. Fix them, and also add a 'finished' argument to
pg_basebackup's progress_report() function, to keep it consistent with
the other programs.
Backpatch to v12. pg_rewind's newline was broken with the logging changes
in commit
cc8d415117 in v12, and pg_checksums was introduced in v12.
Discussion: https://www.postgresql.org/message-id/
82b539e5-ae33-34b0-1aee-
22b3379fd3eb@iki.fi
Michael Paquier [Mon, 17 Aug 2020 01:24:31 +0000 (10:24 +0900)]
doc: Fix description about bgwriter and checkpoint in HA section
Since
806a2ae, the work of the bgwriter is split the checkpointer, but a
portion of the documentation did not get the message.
Author: Masahiko Sawada
Discussion: https://postgr.es/m/CA+fd4k6jXxjAtjMVC=wG3=QGpauZBtcgN3Jhw+oV7zXGKVLKzQ@mail.gmail.com
Backpatch-through: 9.5
Noah Misch [Sat, 15 Aug 2020 23:15:59 +0000 (16:15 -0700)]
Move new LOCKTAG_DATABASE_FROZEN_IDS to end of enum LockTagType.
Several PGXN modules reference LockTagType values; renumbering would
force a recompile of those modules. Oversight in back-patch of today's
commit
566372b3d6435639e4cc4476d79b8505a0297c87. Back-patch to released
branches, v12 through 9.5.
Reported by Tom Lane.
Discussion: https://postgr.es/m/921383.
1597523945@sss.pgh.pa.us
Noah Misch [Sat, 15 Aug 2020 17:15:53 +0000 (10:15 -0700)]
Prevent concurrent SimpleLruTruncate() for any given SLRU.
The SimpleLruTruncate() header comment states the new coding rule. To
achieve this, add locktype "frozenid" and two LWLocks. This closes a
rare opportunity for data loss, which manifested as "apparent
wraparound" or "could not access status of transaction" errors. Data
loss is more likely in pg_multixact, due to released branches' thin
margin between multiStopLimit and multiWrapLimit. If a user's physical
replication primary logged ": apparent wraparound" messages, the user
should rebuild standbys of that primary regardless of symptoms. At less
risk is a cluster having emitted "not accepting commands" errors or
"must be vacuumed" warnings at some point. One can test a cluster for
this data loss by running VACUUM FREEZE in every database. Back-patch
to 9.5 (all supported versions).
Discussion: https://postgr.es/m/
20190218073103[email protected]
Tom Lane [Sat, 15 Aug 2020 02:14:03 +0000 (22:14 -0400)]
Be more careful about the shape of hashable subplan clauses.
nodeSubplan.c expects that the testexpr for a hashable ANY SubPlan
has the form of one or more OpExprs whose LHS is an expression of the
outer query's, while the RHS is an expression over Params representing
output columns of the subquery. However, the planner only went as far
as verifying that the clauses were all binary OpExprs. This works
99.99% of the time, because the clauses have the right shape when
emitted by the parser --- but it's possible for function inlining to
break that, as reported by PegoraroF10. To fix, teach the planner
to check that the LHS and RHS contain the right things, or more
accurately don't contain the wrong things. Given that this has been
broken for years without anyone noticing, it seems sufficient to just
give up hashing when it happens, rather than go to the trouble of
commuting the clauses back again (which wouldn't necessarily work
anyway).
While poking at that, I also noticed that nodeSubplan.c had a baked-in
assumption that the number of hash clauses is identical to the number
of subquery output columns. Again, that's fine as far as parser output
goes, but it's not hard to break it via function inlining. There seems
little reason for that assumption though --- AFAICS, the only thing
it's buying us is not having to store the number of hash clauses
explicitly. Adding code to the planner to reject such cases would take
more code than getting nodeSubplan.c to cope, so I fixed it that way.
This has been broken for as long as we've had hashable SubPlans,
so back-patch to all supported branches.
Discussion: https://postgr.es/m/
1549209182255[email protected]
Alvaro Herrera [Fri, 14 Aug 2020 21:33:31 +0000 (17:33 -0400)]
pg_dump: fix dependencies on FKs to partitioned tables
Parallel-restoring a foreign key that references a partitioned table
with several levels of partitions can fail:
pg_restore: while PROCESSING TOC:
pg_restore: from TOC entry 6684; 2606 29166 FK CONSTRAINT fk fk_a_fkey postgres
pg_restore: error: could not execute query: ERROR: there is no unique constraint matching given keys for referenced table "pk"
Command was: ALTER TABLE fkpart3.fk
ADD CONSTRAINT fk_a_fkey FOREIGN KEY (a) REFERENCES fkpart3.pk(a);
This happens in parallel restore mode because some index partitions
aren't yet attached to the topmost partitioned index that the FK uses,
and so the index is still invalid. The current code marks the FK as
dependent on the first level of index-attach dump objects; the bug is
fixed by recursively marking the FK on their children.
Backpatch to 12, where FKs to partitioned tables were introduced.
Reported-by: Tom Lane
Author: Álvaro Herrera
Discussion: https://postgr.es/m/3170626.1594842723@sss.pgh.pa.us
Backpatch: 12-master
Tom Lane [Fri, 14 Aug 2020 17:26:57 +0000 (13:26 -0400)]
Fix postmaster's behavior during smart shutdown.
Up to now, upon receipt of a SIGTERM ("smart shutdown" command), the
postmaster has immediately killed all "optional" background processes,
and subsequently refused to launch new ones while it's waiting for
foreground client processes to exit. No doubt this seemed like an OK
policy at some point; but it's a pretty bad one now, because it makes
for a seriously degraded environment for the remaining clients:
* Parallel queries are killed, and new ones fail to launch. (And our
parallel-query infrastructure utterly fails to deal with the case
in a reasonable way --- it just hangs waiting for workers that are
not going to arrive. There is more work needed in that area IMO.)
* Autovacuum ceases to function. We can tolerate that for awhile,
but if bulk-update queries continue to run in the surviving client
sessions, there's eventually going to be a mess. In the worst case
the system could reach a forced shutdown to prevent XID wraparound.
* The bgwriter and walwriter are also stopped immediately, likely
resulting in performance degradation.
Hence, let's rearrange things so that the only immediate change in
behavior is refusing to let in new normal connections. Once the last
normal connection is gone, shut everything down as though we'd received
a "fast" shutdown. To implement this, remove the PM_WAIT_BACKUP and
PM_WAIT_READONLY states, instead staying in PM_RUN or PM_HOT_STANDBY
while normal connections remain. A subsidiary state variable tracks
whether or not we're letting in new connections in those states.
This also allows having just one copy of the logic for killing child
processes in smart and fast shutdown modes. I moved that logic into
PostmasterStateMachine() by inventing a new state PM_STOP_BACKENDS.
Back-patch to 9.6 where parallel query was added. In principle
this'd be a good idea in 9.5 as well, but the risk/reward ratio
is not as good there, since lack of autovacuum is not a problem
during typical uses of smart shutdown.
Per report from Bharath Rupireddy.
Patch by me, reviewed by Thomas Munro
Discussion: https://postgr.es/m/CALj2ACXAZ5vKxT9P7P89D87i3MDO9bfS+_bjMHgnWJs8uwUOOw@mail.gmail.com
Heikki Linnakangas [Fri, 14 Aug 2020 07:40:50 +0000 (10:40 +0300)]
Fix typo in test comment.
Alvaro Herrera [Thu, 13 Aug 2020 21:33:49 +0000 (17:33 -0400)]
Handle new HOT chains in index-build table scans
When a table is scanned by heapam_index_build_range_scan (née
IndexBuildHeapScan) and the table lock being held allows concurrent data
changes, it is possible for new HOT chains to sprout in a page that were
unknown when the scan of a page happened. This leads to an error such
as
ERROR: failed to find parent tuple for heap-only tuple at (X,Y) in table "tbl"
because the root tuple was not present when we first obtained the list
of the page's root tuples. This can be fixed by re-obtaining the list
of root tuples, if we see that a heap-only tuple appears to point to a
non-existing root.
This was reported by Anastasia as occurring for BRIN summarization
(which exists since 9.5), but I think it could theoretically also happen
with CREATE INDEX CONCURRENTLY (much older) or REINDEX CONCURRENTLY
(very recent). It seems a happy coincidence that BRIN forces us to
backpatch this all the way to 9.5.
Reported-by: Anastasia Lubennikova
Diagnosed-by: Anastasia Lubennikova
Co-authored-by: Anastasia Lubennikova
Co-authored-by: Álvaro Herrera
Discussion: https://postgr.es/m/
602d8487-f0b2-5486-0088-
0f372b2549fa@postgrespro.ru
Backpatch: 9.5 - master
Alvaro Herrera [Wed, 12 Aug 2020 19:33:36 +0000 (15:33 -0400)]
BRIN: Handle concurrent desummarization properly
If a page range is desummarized at just the right time concurrently with
an index walk, BRIN would raise an error indicating index corruption.
This is scary and unhelpful; silently returning that the page range is
not summarized is sufficient reaction.
This bug was introduced by commit
975ad4e602ff as additional protection
against a bug whose actual fix was elsewhere. Backpatch equally.
Reported-By: Anastasia Lubennikova
Diagnosed-By: Alexander Lakhin
Discussion: https://postgr.es/m/
2588667e-d07d-7e10-74e2-
7e1e46194491@postgrespro.ru
Backpatch: 9.5 - master
Tom Lane [Mon, 10 Aug 2020 21:15:53 +0000 (17:15 -0400)]
Stamp 12.4.
Tom Lane [Mon, 10 Aug 2020 19:35:46 +0000 (15:35 -0400)]
Last-minute updates for release notes.
Security: CVE-2020-14349, CVE-2020-14350
Noah Misch [Mon, 10 Aug 2020 16:22:54 +0000 (09:22 -0700)]
Document clashes between logical replication and untrusted users.
Back-patch to v10, which introduced logical replication.
Security: CVE-2020-14349
Noah Misch [Mon, 10 Aug 2020 16:22:54 +0000 (09:22 -0700)]
Empty search_path in logical replication apply worker and walsender.
This is like CVE-2018-1058 commit
582edc369cdbd348d68441fc50fa26a84afd0c1a. Today, a malicious user of a
publisher or subscriber database can invoke arbitrary SQL functions
under an identity running replication, often a superuser. This fix may
cause "does not exist" or "no schema has been selected to create in"
errors in a replication process. After upgrading, consider watching
server logs for these errors. Objects accruing schema qualification in
the wake of the earlier commit are unlikely to need further correction.
Back-patch to v10, which introduced logical replication.
Security: CVE-2020-14349
Noah Misch [Mon, 10 Aug 2020 16:22:54 +0000 (09:22 -0700)]
Move connect.h from fe_utils to src/include/common.
Any libpq client can use the header. Clients include backend components
postgres_fdw, dblink, and logical replication apply worker. Back-patch
to v10, because another fix needs this. In released branches, just copy
the header and keep the original.
Tom Lane [Mon, 10 Aug 2020 14:44:42 +0000 (10:44 -0400)]
Make contrib modules' installation scripts more secure.
Hostile objects located within the installation-time search_path could
capture references in an extension's installation or upgrade script.
If the extension is being installed with superuser privileges, this
opens the door to privilege escalation. While such hazards have existed
all along, their urgency increases with the v13 "trusted extensions"
feature, because that lets a non-superuser control the installation path
for a superuser-privileged script. Therefore, make a number of changes
to make such situations more secure:
* Tweak the construction of the installation-time search_path to ensure
that references to objects in pg_catalog can't be subverted; and
explicitly add pg_temp to the end of the path to prevent attacks using
temporary objects.
* Disable check_function_bodies within installation/upgrade scripts,
so that any security gaps in SQL-language or PL-language function bodies
cannot create a risk of unwanted installation-time code execution.
* Adjust lookup of type input/receive functions and join estimator
functions to complain if there are multiple candidate functions. This
prevents capture of references to functions whose signature is not the
first one checked; and it's arguably more user-friendly anyway.
* Modify various contrib upgrade scripts to ensure that catalog
modification queries are executed with secure search paths. (These
are in-place modifications with no extension version changes, since
it is the update process itself that is at issue, not the end result.)
Extensions that depend on other extensions cannot be made fully secure
by these methods alone; therefore, revert the "trusted" marking that
commit
eb67623c9 applied to earthdistance and hstore_plperl, pending
some better solution to that set of issues.
Also add documentation around these issues, to help extension authors
write secure installation scripts.
Patch by me, following an observation by Andres Freund; thanks
to Noah Misch for review.
Security: CVE-2020-14350
Peter Eisentraut [Mon, 10 Aug 2020 13:21:18 +0000 (15:21 +0200)]
Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash:
444a6779aafc552ac452715caa65cfca0e723073
Tom Lane [Sun, 9 Aug 2020 16:39:08 +0000 (12:39 -0400)]
Check for fseeko() failure in pg_dump's _tarAddFile().
Coverity pointed out, not unreasonably, that we checked fseeko's
result at every other call site but these. Failure to seek in the
temp file (note this is NOT pg_dump's output file) seems quite
unlikely, and even if it did happen the file length cross-check
further down would probably detect the problem. Still, that's a
poor excuse for not checking the result of a system call.
Tom Lane [Sun, 9 Aug 2020 00:01:41 +0000 (20:01 -0400)]
Release notes for 12.4, 11.9, 10.14, 9.6.19, 9.5.23.
Alvaro Herrera [Sat, 8 Aug 2020 16:31:55 +0000 (12:31 -0400)]
walsnd: Don't set waiting_for_ping_response spuriously
Ashutosh Bapat noticed that when logical walsender needs to wait for
WAL, and it realizes that it must send a keepalive message to
walreceiver to update the sent-LSN, which *does not* request a reply
from walreceiver, it wrongly sets the flag that it's going to wait for
that reply. That means that any future would-be sender of feedback
messages ends up not sending a feedback message, because they all
believe that a reply is expected.
With built-in logical replication there's not much harm in this, because
WalReceiverMain will send a ping-back every wal_receiver_timeout/2
anyway; but with other logical replication systems (e.g. pglogical) it
can cause significant pain.
This problem was introduced in commit
41d5f8ad734, where the
request-reply flag was changed from true to false to WalSndKeepalive,
without at the same time removing the line that sets
waiting_for_ping_response.
Just removing that line would be a sufficient fix, but it seems better
to shift the responsibility of setting the flag to WalSndKeepalive
itself instead of requiring caller to do it; this is clearly less
error-prone.
Author: Álvaro Herrera
Reported-by: Ashutosh Bapat
Backpatch: 9.5 and up
Discussion: https://postgr.es/m/20200806225558[email protected]
Etsuro Fujita [Fri, 7 Aug 2020 05:45:02 +0000 (14:45 +0900)]
Fix yet another issue with step generation in partition pruning.
Commit
13838740f fixed some issues with step generation in partition
pruning, but there was yet another one: get_steps_using_prefix() assumes
that clauses in the passed-in prefix list are sorted in ascending order
of their partition key numbers, but the caller failed to ensure this for
range partitioning, which led to an assertion failure in debug builds.
Adjust the caller function to arrange the clauses in the prefix list in
the required order for range partitioning.
Back-patch to v11, like the previous commit.
Patch by me, reviewed by Amit Langote.
Discussion: https://postgr.es/m/CAPmGK16jkXiFG0YqMbU66wte-oJTfW6D1HaNvQf%3D%2B5o9%3Dm55wQ%40mail.gmail.com
Tom Lane [Thu, 6 Aug 2020 19:49:45 +0000 (15:49 -0400)]
First-draft release notes for 12.4.
As usual, the release notes for other branches will be made by cutting
these down, but put them up for community review first.
Robert Haas [Thu, 6 Aug 2020 18:55:00 +0000 (14:55 -0400)]
Fix typo.
Per report from Tom Lane. Previously fixed in master by
commit
f057980149ddccd4b862d2c6b3920ed498b0d7ec.
Robert Haas [Thu, 6 Aug 2020 17:58:23 +0000 (13:58 -0400)]
Fix minor problems with non-exclusive backup cleanup.
The previous coding imagined that it could call before_shmem_exit()
when a non-exclusive backup began and then remove the previously-added
handler by calling cancel_before_shmem_exit() when that backup
ended. However, this only works provided that nothing else in the
system has registered a before_shmem_exit() hook in the interim,
because cancel_before_shmem_exit() is documented to remove a callback
only if it is the latest callback registered. It also only works
if nothing can ERROR out between the time that sessionBackupState
is reset and the time that cancel_before_shmem_exit(), which doesn't
seem to be strictly true.
To fix, leave the handler installed for the lifetime of the session,
arrange to install it just once, and teach it to quietly do nothing if
there isn't a non-exclusive backup in process.
This was originally committed to master as
303640199d0436c5e7acdf50b837a027b5726594, but I did not back-patch
at the time because the consequences were minor. However, now
there's been a second report of this causing trouble with a slightly
different test case than the one I reported originally, so now
I'm back-patching as far as v11 where JIT was introduced.
Patch by me, reviewed by Kyotaro Horiguchi, Michael Paquier (who
preferred a different approach, but got outvoted), Fujii Masao,
and Tom Lane, and with comments by various others. New problem
report from Bharath Rupireddy.
Discussion: http://postgr.es/m/CA+TgmobMjnyBfNhGTKQEDbqXYE3_rXWpc4CM63fhyerNCes3mA@mail.gmail.com
Discussion: http://postgr.es/m/CALj2ACWk7j4F2v2fxxYfrroOF=AdFNPr1WsV+AGtHAFQOqm_pw@mail.gmail.com
Bruce Momjian [Wed, 5 Aug 2020 21:12:10 +0000 (17:12 -0400)]
doc: clarify "state" table reference in tutorial
Reported-by: Vyacheslav Shablistyy
Discussion: https://postgr.es/m/
159586122762.680.
1361378513036616007@wrigleys.postgresql.org
Backpatch-through: 9.5
Tom Lane [Wed, 5 Aug 2020 19:38:55 +0000 (15:38 -0400)]
Fix matching of sub-partitions when a partitioned plan is stale.
Since we no longer require AccessExclusiveLock to add a partition,
the executor may see that a partitioned table has more partitions
than the planner saw. ExecCreatePartitionPruneState's code for
matching up the partition lists in such cases was faulty, and would
misbehave if the planner had successfully pruned any partitions from
the query. (Thus, trouble would occur only if a partition addition
happens concurrently with a query that uses both static and dynamic
partition pruning.) This led to an Assert failure in debug builds,
and probably to crashes or query misbehavior in production builds.
To repair the bug, just explicitly skip zeroes in the plan's
relid_map[] list. I also made some cosmetic changes to make the code
more readable (IMO anyway). Also, convert the cross-checking Assert
to a regular test-and-elog, since it's now apparent that this logic
is more fragile than one would like.
Currently, there's no way to repeatably exercise this code, except
with manual use of a debugger to stop the backend between planning
and execution. Hence, no test case in this patch. We oughta do
something about that testability gap, but that's for another day.
Amit Langote and Tom Lane, per report from Justin Pryzby. Oversight
in commit
898e5e329; backpatch to v12 where that appeared.
Discussion: https://postgr.es/m/
20200802181131[email protected]
Tom Lane [Tue, 4 Aug 2020 19:20:31 +0000 (15:20 -0400)]
Increase hard-wired timeout values in ecpg regression tests.
A couple of test cases had connect_timeout=14, a value that seems
to have been plucked from a hat. While it's more than sufficient
for normal cases, slow/overloaded buildfarm machines can get a timeout
failure here, as per recent report from "sungazer". Increase to 180
seconds, which is in line with our typical timeouts elsewhere in
the regression tests.
Back-patch to 9.6; the code looks different in 9.5, and this doesn't
seem to be quite worth the effort to adapt to that.
Report: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=sungazer&dt=2020-08-04%2007%3A12%3A22
Tom Lane [Mon, 3 Aug 2020 17:11:16 +0000 (13:11 -0400)]
Doc: fix obsolete info about allowed range of TZ offsets in timetz.
We've allowed UTC offsets up to +/- 15:59 since commit
cd0ff9c0f, but
that commit forgot to fix the documentation about timetz.
Per bug #16571 from osdba.
Discussion: https://postgr.es/m/16571-
eb7501598de78c8a@postgresql.org
Thomas Munro [Mon, 3 Aug 2020 00:39:15 +0000 (12:39 +1200)]
Fix rare failure in LDAP tests.
Instead of writing a query to psql's stdin, use -c. This avoids a
failure where psql exits before we write, seen a few times on the build
farm. Thanks to Tom Lane for the suggestion.
Back-patch to 11, where the LDAP tests arrived.
Reviewed-by: Noah Misch
Discussion: https://postgr.es/m/CA%2BhUKGLFmW%2BHQYPeKiwSp5sdFFHtFViCpw4Mh6yAgEx74r5-Cw%40mail.gmail.com
Peter Geoghegan [Fri, 31 Jul 2020 22:34:25 +0000 (15:34 -0700)]
Restore lost amcheck TOAST test coverage.
Commit
eba77534 fixed an amcheck false positive bug involving
inconsistencies in TOAST input state between table and index. A test
case was added that verified that such an inconsistency didn't result in
a spurious corruption related error.
Test coverage from the test was accidentally lost by commit
501e41dd,
which propagated ALTER TABLE ... SET STORAGE attstorage state to
indexes. This broke the test because the test specifically relied on
attstorage not being propagated. This artificially forced there to be
index tuples whose datums were equivalent to the datums in the heap
without the datums actually being bitwise equal.
Fix this by updating pg_attribute directly instead. Commit
501e41dd
made similar changes to a test_decoding TOAST-related test case which
made the same assumption, but overlooked the amcheck test case.
Backpatch: 11-, just like commit
eba77534 (and commit
501e41dd).
Tom Lane [Fri, 31 Jul 2020 15:43:12 +0000 (11:43 -0400)]
Fix recently-introduced performance problem in ts_headline().
The new hlCover() algorithm that I introduced in commit
c9b0c678d
turns out to potentially take O(N^2) or worse time on long documents,
if there are many occurrences of individual query words but few or no
substrings that actually satisfy the query. (One way to hit this
behavior is with a "common_word & rare_word" type of query.) This
seems unavoidable given the original goal of checking every substring
of the document, so we have to back off that idea. Fortunately, it
seems unlikely that anyone would really want headlines spanning all of
a long document, so we can avoid the worse-than-linear behavior by
imposing a maximum length of substring that we'll consider.
For now, just hard-wire that maximum length as a multiple of max_words
times max_fragments. Perhaps at some point somebody will argue for
exposing it as a ts_headline parameter, but I'm hesitant to make such
a feature addition in a back-patched bug fix.
I also noted that the hlFirstIndex() function I'd added in that
commit was unnecessarily stupid: it really only needs to check whether
a HeadlineWordEntry's item pointer is null or not. This wouldn't make
all that much difference in typical cases with queries having just
a few terms, but a cycle shaved is a cycle earned.
In addition, add a CHECK_FOR_INTERRUPTS call in TS_execute_recurse.
This ensures that hlCover's loop is cancellable if it manages to take
a long time, and it may protect some other TS_execute callers as well.
Back-patch to 9.6 as the previous commit was. I also chose to add the
CHECK_FOR_INTERRUPTS call to 9.5. The old hlCover() algorithm seems
to avoid the O(N^2) behavior, at least on the test case I tried, but
nonetheless it's not very quick on a long document.
Per report from Stephen Frost.
Discussion: https://postgr.es/m/
20200724160535[email protected]
Tatsuo Ishii [Thu, 30 Jul 2020 22:48:23 +0000 (07:48 +0900)]
Doc: fix high availability solutions comparison.
In "High Availability, Load Balancing, and Replication" chapter,
certain descriptions of Pgpool-II were not correct at this point. It
does not need conflict resolution. Also "Multiple-Server Parallel
Query Execution" is not supported anymore.
Discussion: https://postgr.es/m/
20200726.230128.
53842489850344110.t-ishii%40sraoss.co.jp
Author: Tatsuo Ishii
Reviewed-by: Bruce Momjian
Backpatch-through: 9.5
Michael Paquier [Thu, 30 Jul 2020 06:48:56 +0000 (15:48 +0900)]
doc: Mention index references in pg_inherits
Partitioned indexes are also registered in pg_inherits, but the
description of this catalog did not reflect that.
Author: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/
[email protected]
Backpatch-through: 11
David Rowley [Tue, 28 Jul 2020 10:52:51 +0000 (22:52 +1200)]
Doc: Improve documentation for pg_jit_available()
Per complaint from Scott Ribe. Based on wording suggestion from Tom Lane.
Discussion: https://postgr.es/m/
1956E806-1468-4417-9A9D-
235AE1D5FE1A@elevated-dev.com
Backpatch-through: 11, where pg_jit_available() was added
Etsuro Fujita [Tue, 28 Jul 2020 02:00:02 +0000 (11:00 +0900)]
Fix some issues with step generation in partition pruning.
In the case of range partitioning, get_steps_using_prefix() assumes that
the passed-in prefix list contains at least one clause for each of the
partition keys earlier than one specified in the passed-in
step_lastkeyno, but the caller (ie, gen_prune_steps_from_opexps())
didn't take it into account, which led to a server crash or incorrect
results when the list contained no clauses for such partition keys, as
reported in bug #16500 and #16501 from Kobayashi Hisanori. Update the
caller to call that function only when the list created there contains
at least one clause for each of the earlier partition keys in the case
of range partitioning.
While at it, fix some other issues:
* The list to pass to get_steps_using_prefix() is allowed to contain
multiple clauses for the same partition key, as described in the
comment for that function, but that function actually assumed that the
list contained just a single clause for each of middle partition keys,
which led to an assertion failure when the list contained multiple
clauses for such partition keys. Update that function to match the
comment.
* In the case of hash partitioning, partition keys are allowed to be
NULL, in which case the list to pass to get_steps_using_prefix()
contains no clauses for NULL partition keys, but that function treats
that case as like the case of range partitioning, which led to the
assertion failure. Update the assertion test to take into account
NULL partition keys in the case of hash partitioning.
* Fix a typo in a comment in get_steps_using_prefix_recurse().
* gen_partprune_steps() failed to detect self-contradiction from
strict-qual clauses and an IS NULL clause for the same partition key
in some cases, producing incorrect partition-pruning steps, which led
to incorrect results of partition pruning, but didn't cause any
user-visible problems fortunately, as the self-contradiction is
detected later in the query planning. Update that function to detect
the self-contradiction.
Per bug #16500 and #16501 from Kobayashi Hisanori. Patch by me, initial
diagnosis for the reported issue and review by Dmitry Dolgov.
Back-patch to v11, where partition pruning was introduced.
Discussion: https://postgr.es/m/16500-
d1613f2a78e1e090%40postgresql.org
Discussion: https://postgr.es/m/16501-
5234a9a0394f6754%40postgresql.org
Michael Paquier [Mon, 27 Jul 2020 06:58:59 +0000 (15:58 +0900)]
Fix corner case with 16kB-long decompression in pgcrypto, take 2
A compressed stream may end with an empty packet. In this case
decompression finishes before reading the empty packet and the
remaining stream packet causes a failure in reading the following
data. This commit makes sure to consume such extra data, avoiding a
failure when decompression the data. This corner case was reproducible
easily with a data length of 16kB, and existed since
e94dd6a. A cheap
regression test is added to cover this case based on a random,
incompressible string.
The first attempt of this patch has allowed to find an older failure
within the compression logic of pgcrypto, fixed by
b9b6105. This
involved SLES 15 with z390 where a custom flavor of libz gets used.
Bonus thanks to Mark Wong for providing access to the specific
environment.
Reported-by: Frank Gagnepain
Author: Kyotaro Horiguchi, Michael Paquier
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/16476-
692ef7b84e5fb893@postgresql.org
Backpatch-through: 9.5
Michael Paquier [Mon, 27 Jul 2020 01:29:11 +0000 (10:29 +0900)]
Fix handling of structure for bytea data type in ECPG
Some code paths dedicated to bytea used the structure for varchar. This
did not lead to any actual bugs, as bytea and varchar have the same
definition, but it could become a trap if one of these definitions
changes for a new feature or a bug fix.
Issue introduced by
050710b.
Author: Shenhao Wang
Reviewed-by: Vignesh C, Michael Paquier
Discussion: https://postgr.es/m/
07ac7dee1efc44f99d7f53a074420177@G08CNEXMBPEKD06.g08.fujitsu.local
Backpatch-through: 12
Amit Kapila [Sat, 25 Jul 2020 05:08:46 +0000 (10:38 +0530)]
Fix buffer usage stats for nodes above Gather Merge.
Commit
85c9d347 addressed a similar problem for Gather and Gather
Merge nodes but forgot to account for nodes above parallel nodes. This
still works for nodes above Gather node because we shut down the workers
for Gather node as soon as there are no more tuples. We can do a similar
thing for Gather Merge as well but it seems better to account for stats
during nodes shutdown after completing the execution.
Reported-by: Stéphane Lorek, Jehan-Guillaume de Rorthais
Author: Jehan-Guillaume de Rorthais
Reviewed-by: Amit Kapila
Backpatch-through: 10, where it was introduced
Discussion: https://postgr.es/m/20200718160206.584532a2@firost
Tom Lane [Thu, 23 Jul 2020 21:19:37 +0000 (17:19 -0400)]
Fix ancient violation of zlib's API spec.
contrib/pgcrypto mishandled the case where deflate() does not consume
all of the offered input on the first try. It reset the next_in pointer
to the start of the input instead of leaving it alone, causing the wrong
data to be fed to the next deflate() call.
This has been broken since pgcrypto was committed. The reason for the
lack of complaints seems to be that it's fairly hard to get stock zlib
to not consume all the input, so long as the output buffer is big enough
(which it normally would be in pgcrypto's usage; AFAICT the input is
always going to be packetized into packets no larger than ZIP_OUT_BUF).
However, IBM's zlibNX implementation for AIX evidently will do it
in some cases.
I did not add a test case for this, because I couldn't find one that
would fail with stock zlib. When we put back the test case for
bug #16476, that will cover the zlibNX situation well enough.
While here, write deflate()'s second argument as Z_NO_FLUSH per its
API spec, instead of hard-wiring the value zero.
Per buildfarm results and subsequent investigation.
Discussion: https://postgr.es/m/16476-
692ef7b84e5fb893@postgresql.org
Peter Eisentraut [Thu, 23 Jul 2020 15:13:00 +0000 (17:13 +0200)]
doc: Document that ssl_ciphers does not affect TLS 1.3
TLS 1.3 uses a different way of specifying ciphers and a different
OpenSSL API. PostgreSQL currently does not support setting those
ciphers. For now, just document this. In the future, support for
this might be added somehow.
Reviewed-by: Jonathan S. Katz
Reviewed-by: Tom Lane
Thomas Munro [Thu, 23 Jul 2020 09:10:49 +0000 (21:10 +1200)]
Fix error message.
Remove extra space. Back-patch to all releases, like commit
7897e3bb.
Author: Lu, Chenyang
Discussion: https://postgr.es/m/795d03c6129844d3803e7eea48f5af0d%40G08CNEXMBPEKD04.g08.fujitsu.local
Michael Paquier [Wed, 22 Jul 2020 23:29:18 +0000 (08:29 +0900)]
Revert "Fix corner case with PGP decompression in pgcrypto"
This reverts commit
9e10898, after finding out that buildfarm members
running SLES 15 on z390 complain on the compression and decompression
logic of the new test: pipistrelles, barbthroat and steamerduck.
Those hosts are visibly using hardware-specific changes to improve zlib
performance, requiring more investigation.
Thanks to Tom Lane for the discussion.
Discussion: https://postgr.es/m/
20200722093749[email protected]
Backpatch-through: 9.5
Michael Paquier [Wed, 22 Jul 2020 05:52:46 +0000 (14:52 +0900)]
Fix corner case with PGP decompression in pgcrypto
A compressed stream may end with an empty packet, and PGP decompression
finished before reading this empty packet in the remaining stream. This
caused a failure in pgcrypto, handling this case as corrupted data.
This commit makes sure to consume such extra data, avoiding a failure
when decompression the entire stream. This corner case was reproducible
with a data length of 16kB, and existed since its introduction in
e94dd6a. A cheap regression test is added to cover this case.
Thanks to Jeff Janes for the extra investigation.
Reported-by: Frank Gagnepain
Author: Kyotaro Horiguchi, Michael Paquier
Discussion: https://postgr.es/m/16476-
692ef7b84e5fb893@postgresql.org
Backpatch-through: 9.5
Tom Lane [Tue, 21 Jul 2020 23:40:44 +0000 (19:40 -0400)]
neqjoinsel must now pass through collation to eqjoinsel.
Since commit
044c99bc5, eqjoinsel passes the passed-in collation
to any operators it invokes. However, neqjoinsel failed to pass
on whatever collation it got, so that if we invoked a
collation-dependent operator via that code path, we'd get "could not
determine which collation to use for string comparison" or the like.
Per report from Justin Pryzby. Back-patch to v12, like the previous
commit.
Discussion: https://postgr.es/m/
20200721191606[email protected]
Tom Lane [Tue, 21 Jul 2020 16:38:08 +0000 (12:38 -0400)]
Assert that we don't insert nulls into attnotnull catalog columns.
The executor checks for this error, and so does the bootstrap catalog
loader, but we never checked for it in retail catalog manipulations.
The folly of that has now been exposed, so let's add assertions
checking it. Checking in CatalogTupleInsert[WithInfo] and
CatalogTupleUpdate[WithInfo] should be enough to cover this.
Back-patch to v10; the aforesaid functions didn't exist before that,
and it didn't seem worth adapting the patch to the oldest branches.
But given the risk of JIT crashes, I think we certainly need this
as far back as v11.
Pre-v13, we have to explicitly exclude pg_subscription.subslotname
and pg_subscription_rel.srsublsn from the checks, since they are
mismarked. (Even if we change our mind about applying BKI_FORCE_NULL
in the branch tips, it doesn't seem wise to have assertions that
would fire in existing databases.)
Discussion: https://postgr.es/m/298837.
1595196283@sss.pgh.pa.us
Tom Lane [Tue, 21 Jul 2020 15:40:46 +0000 (11:40 -0400)]
Avoid direct C access to possibly-null pg_subscription_rel.srsublsn.
This coding technique is unsafe, since we'd be accessing off the end
of the tuple if the field is null. SIGSEGV is pretty improbable, but
perhaps not impossible. Also, returning garbage for the LSN doesn't
seem like a great idea, even if callers aren't looking at it today.
Also update docs to point out explicitly that
pg_subscription.subslotname and pg_subscription_rel.srsublsn
can be null.
Perhaps we should mark these two fields BKI_FORCE_NULL, so that
they'd be correctly labeled in databases that are initdb'd in the
future. But we can't force that for existing databases, and on
balance it's not too clear that having a mix of different catalog
contents in the field would be wise.
Apply to v10 (where this code came in) through v12. Already
fixed in v13 and HEAD.
Discussion: https://postgr.es/m/732838.
1595278439@sss.pgh.pa.us
Tom Lane [Mon, 20 Jul 2020 19:54:24 +0000 (15:54 -0400)]
Kluge slot_compile_deform() to ignore incorrect attnotnull markings.
Since we mustn't force an initdb in released branches, there is no
simple way to correct the markings of pg_subscription.subslotname
and pg_subscription_rel.srsublsn as attnotnull in existing pre-v13
installations.
Fortunately, released branches don't rely on attnotnull being correct
for much. The planner looks at it in relation_excluded_by_constraints,
but it'd be difficult to get that to matter for a query on a system
catalog. The only place where it's really problematic is in JIT's
slot_compile_deform(), which can produce incorrect code that crashes
if there are NULLs in an allegedly not-null column.
Hence, hack up slot_compile_deform() to be specifically aware of
these two incorrect markings and not trust them.
This applies to v11 and v12; the JIT code didn't exist before that,
and we've fixed the markings in v13.
Discussion: https://postgr.es/m/229396.
1595191345@sss.pgh.pa.us
Tom Lane [Mon, 20 Jul 2020 17:40:16 +0000 (13:40 -0400)]
Fix construction of updated-columns bitmap in logical replication.
Commit
b9c130a1f failed to apply the publisher-to-subscriber column
mapping while checking which columns were updated. Perhaps less
significantly, it didn't exclude dropped columns either. This could
result in an incorrect updated-columns bitmap and thus wrong decisions
about whether to fire column-specific triggers on the subscriber while
applying updates. In HEAD (since commit
9de77b545), it could also
result in accesses off the end of the colstatus array, as detected by
buildfarm member skink. Fix the logic, and adjust 003_constraints.pl
so that the problem is exposed in unpatched code.
In HEAD, also add some assertions to check that we don't access off
the ends of these newly variable-sized arrays.
Back-patch to v10, as
b9c130a1f was.
Discussion: https://postgr.es/m/CAH2-Wz=79hKQ4++c5A060RYbjTHgiYTHz=fw6mptCtgghH2gJA@mail.gmail.com
Michael Paquier [Sat, 18 Jul 2020 13:43:45 +0000 (22:43 +0900)]
doc: Refresh more URLs in the docs
This updates some URLs that are redirections, mostly to an equivalent
using https. One URL referring to generalized partial indexes was
outdated.
Author: Kyotaro Horiguchi
Discussion: https://postgr.es/m/
20200717.121308.
1369606287593685396[email protected]
Backpatch-through: 9.5
Michael Paquier [Sat, 18 Jul 2020 01:42:50 +0000 (10:42 +0900)]
doc: Fix description of \copy for psql
The WHERE clause introduced by
31f3817 was not described. While on it,
split the grammar of \copy FROM and TO into two distinct parts for
clarity as they support different set of options.
Author: Vignesh C
Discussion: https://postgr.es/m/CALDaNm3zWr=OmxeNqOqfT=uZTSdam_j-gkX94CL8eTNfgUtf6A@mail.gmail.com
Backpatch-through: 12
Tom Lane [Fri, 17 Jul 2020 17:03:50 +0000 (13:03 -0400)]
Cope with data-offset-less archive files during out-of-order restores.
pg_dump produces custom-format archive files that lack data offsets
when it is unable to seek its output. Up to now that's been a hazard
for pg_restore. But if pg_restore is able to seek in the archive
file, there is no reason to throw up our hands when asked to restore
data blocks out of order. Instead, whenever we are searching for a
data block, record the locations of the blocks we passed over (that
is, fill in the missing data-offset fields in our in-memory copy of
the TOC data). Then, when we hit a case that requires going
backwards, we can just seek back.
Also track the furthest point that we've searched to, and seek back
to there when beginning a search for a new data block. This avoids
possible O(N^2) time consumption, by ensuring that each data block
is examined at most twice. (On Unix systems, that's at most twice
per parallel-restore job; but since Windows uses threads here, the
threads can share block location knowledge, reducing the amount of
duplicated work.)
We can also improve the code a bit by using fseeko() to skip over
data blocks during the search.
This is all of some use even in simple restores, but it's really
significant for parallel pg_restore. In that case, we require
seekability of the input already, and we will very probably need
to do out-of-order restores.
Back-patch to v12, as this fixes a regression introduced by commit
548e50976. Before that, parallel restore avoided requesting
out-of-order restores, so it would work on a data-offset-less
archive. Now it will again.
Ideally this patch would include some test coverage, but there are
other open bugs that need to be fixed before we can extend our
coverage of parallel restore very much. Plan to revisit that later.
David Gilman and Tom Lane; reviewed by Justin Pryzby
Discussion: https://postgr.es/m/CALBH9DDuJ+scZc4MEvw5uO-=vRyR2=QF9+Yh=3hPEnKHWfS81A@mail.gmail.com
Tom Lane [Fri, 17 Jul 2020 16:14:28 +0000 (12:14 -0400)]
Remove manual tracking of file position in pg_dump/pg_backup_custom.c.
We do not really need to track the file position by hand. We were
already relying on ftello() whenever the archive file is seekable,
while if it's not seekable we don't need the file position info
anyway because we're not going to be able to re-write the TOC.
Moreover, that tracking was buggy since it failed to account for
the effects of fseeko(). Somewhat remarkably, that seems not to
have made for any live bugs up to now. We could fix the oversights,
but it seems better to just get rid of the whole error-prone mess.
In itself this is merely code cleanup. However, it's necessary
infrastructure for an upcoming bug-fix patch (because that code
*does* need valid file position after fseeko). The bug fix
needs to go back as far as v12; hence, back-patch that far.
Discussion: https://postgr.es/m/CALBH9DDuJ+scZc4MEvw5uO-=vRyR2=QF9+Yh=3hPEnKHWfS81A@mail.gmail.com
Tom Lane [Fri, 17 Jul 2020 15:03:55 +0000 (11:03 -0400)]
Ensure that distributed timezone abbreviation files are plain ASCII.
We had two occurrences of "Mitteleuropäische Zeit" in Europe.txt,
though the corresponding entries in Default were spelled
"Mitteleuropaeische Zeit". Standardize on the latter spelling to
avoid questions of which encoding to use.
While here, correct a couple of other trivial inconsistencies between
the Default file and the supposedly-matching entries in the *.txt
files, as exposed by some checking with comm(1). Also, add BDST to
the Europe.txt file; it previously was only listed in Default.
None of this has any direct functional effect.
Per complaint from Christoph Berg. As usual for timezone data patches,
apply to all branches.
Discussion: https://postgr.es/m/
20200716100743[email protected]
Peter Eisentraut [Fri, 17 Jul 2020 13:16:13 +0000 (15:16 +0200)]
Fix whitespace
Michael Paquier [Thu, 16 Jul 2020 06:52:58 +0000 (15:52 +0900)]
Switch pg_test_fsync to use binary mode on Windows
pg_test_fsync has always opened files using the text mode on Windows, as
this is the default mode used if not enforced by _setmode().
This fixes a failure when running pg_test_fsync down to 12 because
O_DSYNC and the text mode are not able to work together nicely. We
fixed the handling of O_DSYNC in 12~ for the tool by switching to the
concurrent-safe version of fopen() in src/port/ with
0ba06e0. And
40cfe86, by enforcing the text mode for compatibility reasons if O_TEXT
or O_BINARY are not specified by the caller, broke pg_test_fsync. For
all versions, this avoids any translation overhead, and pg_test_fsync
should test binary writes, so it is a gain in all cases.
Note that O_DSYNC is still not handled correctly in ~11, leading to
pg_test_fsync to show insanely high numbers for open_datasync() (using
this property it is easy to notice that the binary mode is much
faster). This would require a backpatch of
0ba06e0 and
40cfe86, which
could potentially break existing applications, so this is left out.
There are no TAP tests for this tool yet, so I have checked all builds
manually using MSVC. We could invent a new option to run a single
transaction instead of using a duration of 1s to make the tests a
maximum short, but this is left as future work.
Thanks to Bruce Momjian for the discussion.
Reported-by: Jeff Janes
Author: Michael Paquier
Discussion: https://postgr.es/m/16526-
279ded30a230d275@postgresql.org
Backpatch-through: 9.5
Michael Paquier [Wed, 15 Jul 2020 06:17:36 +0000 (15:17 +0900)]
Fix handling of missing files when using pg_rewind with online source
When working with an online source cluster, pg_rewind gets a list of all
the files in the source data directory using a WITH RECURSIVE query,
returning a NULL result for a file's metadata if it gets removed between
the moment it is listed in a directory and the moment its metadata is
obtained with pg_stat_file() (say a recycled WAL segment). The query
result was processed in such a way that for each tuple we checked only
that the first file's metadata was NULL. This could have two
consequences, both resulting in a failure of the rewind:
- If the first tuple referred to a removed file, all files from the
source would be ignored.
- Any file actually missing would not be considered as such.
While on it, rework slightly the code so as no values are saved if we
know that a file is going to be skipped.
Issue introduced by
b36805f, so backpatch down to 9.5.
Author: Justin Pryzby, Michael Paquier
Reviewed-by: Daniel Gustafsson, Masahiko Sawada
Discussion: https://postgr.es/m/
20200713061010[email protected]
Backpatch-through: 9.5
Tom Lane [Tue, 14 Jul 2020 22:56:49 +0000 (18:56 -0400)]
Fix bitmap AND/OR scans on the inside of a nestloop partition-wise join.
reparameterize_path_by_child() failed to reparameterize BitmapAnd
and BitmapOr paths. This matters only if such a path is chosen as
the inside of a nestloop partition-wise join, where we have to pass
in parameters from the outside of the nestloop. If that did happen,
we generated a bad plan that would likely lead to crashes at execution.
This is not entirely reparameterize_path_by_child()'s fault though;
it's the victim of an ancient decision (my ancient decision, I think)
to not bother filling in param_info in BitmapAnd/Or path nodes. That
caused the function to believe that such nodes and their children
contain no parameter references and so need not be processed.
In hindsight that decision looks pretty penny-wise and pound-foolish:
while it saves a few cycles during path node setup, we do commonly
need the information later. In particular, by reversing the decision
and requiring valid param_info data in all nodes of a bitmap path
tree, we can get rid of indxpath.c's get_bitmap_tree_required_outer()
function, which computed the data on-demand. It's not unlikely that
that nets out as a savings of cycles in many scenarios. A couple
of other things in indxpath.c can be simplified as well.
While here, get rid of some cases in reparameterize_path_by_child()
that are visibly dead or useless, given that we only care about
reparameterizing paths that can be on the inside of a parameterized
nestloop. This case reminds one of the maxim that untested code
probably does not work, so I'm unwilling to leave unreachable code
in this function. (I did leave the T_Gather case in place even
though it's not reached in the regression tests. It's not very
clear to me when the planner might prefer to put Gather below
rather than above a nestloop, but at least in principle the case
might be interesting.)
Per bug #16536, originally from Arne Roland but with a test case
by Andrew Gierth. Back-patch to v11 where this code came in.
Discussion: https://postgr.es/m/16536-
2213ee0b3aad41fd@postgresql.org
David Rowley [Tue, 14 Jul 2020 05:03:12 +0000 (17:03 +1200)]
Fix timing issue with ALTER TABLE's validate constraint
An ALTER TABLE to validate a foreign key in which another subcommand
already caused a pending table rewrite could fail due to ALTER TABLE
attempting to validate the foreign key before the actual table rewrite
takes place. This situation could result in an error such as:
ERROR: could not read block 0 in file "base/nnnnn/nnnnn": read only 0 of 8192 bytes
The failure here was due to the SPI call which validates the foreign key
trying to access an index which is yet to be rebuilt.
Similarly, we also incorrectly tried to validate CHECK constraints before
the heap had been rewritten.
The fix for both is to delay constraint validation until phase 3, after
the table has been rewritten. For CHECK constraints this means a slight
behavioral change. Previously ALTER TABLE VALIDATE CONSTRAINT on
inheritance tables would be validated from the bottom up. This was
different from the order of evaluation when a new CHECK constraint was
added. The changes made here aligns the VALIDATE CONSTRAINT evaluation
order for inheritance tables to be the same as ADD CONSTRAINT, which is
generally top-down.
Reported-by: Nazli Ugur Koyluoglu, using SQLancer
Discussion: https://postgr.es/m/CAApHDvp%3DZXv8wiRyk_0rWr00skhGkt8vXDrHJYXRMft3TjkxCA%40mail.gmail.com
Backpatch-through: 9.5 (all supported versions)
Michael Paquier [Tue, 14 Jul 2020 04:17:34 +0000 (13:17 +0900)]
Fix comments related to table AMs
Incorrect function names were referenced. As this fixes some portions
of tableam.h, that is mentioned in the docs as something to look at when
implementing a table AM, backpatch down to 12 where this has been
introduced.
Author: Hironobu Suzuki
Discussion: https://postgr.es/m/
8fe6d672-28dd-3f1d-7aed-
ac2f6d599d3f@interdb.jp
Backpatch-through: 12
Tom Lane [Tue, 14 Jul 2020 00:38:21 +0000 (20:38 -0400)]
Cope with lateral references in the quals of a subquery RTE.
The qual pushdown logic assumed that all Vars in a restriction clause
must be Vars referencing subquery outputs; but since we introduced
LATERAL, it's possible for such a Var to be a lateral reference instead.
This led to an assertion failure in debug builds. In a non-debug
build, there might be no ill effects (if qual_is_pushdown_safe decided
the qual was unsafe anyway), or we could get failures later due to
construction of an invalid plan. I've not gone to much length to
characterize the possible failures, but at least segfaults in the
executor have been observed.
Given that this has been busted since 9.3 and it took this long for
anybody to notice, I judge that the case isn't worth going to great
lengths to optimize. Hence, fix by just teaching qual_is_pushdown_safe
that such quals are unsafe to push down, matching the previous behavior
when it accidentally didn't fail.
Per report from Tom Ellis. Back-patch to all supported branches.
Discussion: https://postgr.es/m/
20200713175124.GQ8220@cloudinit-builder
Tom Lane [Mon, 13 Jul 2020 15:57:55 +0000 (11:57 -0400)]
Fix bugs in libpq's management of GSS encryption state.
GSS-related resources should be cleaned up in pqDropConnection,
not freePGconn, else the wrong things happen when resetting
a connection or trying to switch to a different server.
It's also critical to reset conn->gssenc there.
During connection setup, initialize conn->try_gss at the correct
place, else switching to a different server won't work right.
Remove now-redundant cleanup of GSS resources around one (and, for
some reason, only one) pqDropConnection call in connectDBStart.
Per report from Kyotaro Horiguchi that psql would freeze up,
rather than successfully resetting a GSS-encrypted connection
after a server restart.
This is YA oversight in commit
b0b39f72b, so back-patch to v12.
Discussion: https://postgr.es/m/
20200710.173803.
435804731896516388[email protected]
Tom Lane [Sat, 11 Jul 2020 17:36:50 +0000 (13:36 -0400)]
Avoid trying to restore table ACLs and per-column ACLs in parallel.
Parallel pg_restore has always supposed that ACL items for different
objects are independent and can be restored in parallel without
conflicts. However, there is one case where this fails: because
REVOKE on a table is defined to also revoke the privilege(s) at
column level, we can't restore per-column ACLs till after we restore
any table-level privileges on their table. Failure to honor this
restriction can lead to "tuple concurrently updated" errors during
parallel restore, or even to the per-column ACLs silently disappearing
because the table-level REVOKE is executed afterwards.
To fix, add a dependency from each column-level ACL item to its table's
ACL item, if there is one. Note that this doesn't fix the hazard
for pre-existing archive files, only for ones made with a corrected
pg_dump. Given that the bug's been there quite awhile without
field reports, I think this is acceptable.
This requires changing the API of pg_dump's dumpACL() function.
To keep its argument list from getting even longer, I removed the
"CatalogId objCatId" argument, which has been unused for ages.
Per report from Justin Pryzby. Back-patch to all supported branches.
Discussion: https://postgr.es/m/
20200706050129[email protected]
Alexander Korotkov [Sat, 11 Jul 2020 00:21:00 +0000 (03:21 +0300)]
Forbid numeric NaN in jsonpath
SQL standard doesn't define numeric Inf or NaN values. It appears even more
ridiculous to support then in jsonpath assuming JSON doesn't support these
values as well. This commit forbids returning NaN from .double(), which was
previously allowed. NaN can't be result of inner-jsonpath computation over
non-NaNs. So, we can not expect NaN in the jsonpath output.
Reported-by: Tom Lane
Discussion: https://postgr.es/m/203949.
1591879542%40sss.pgh.pa.us
Author: Alexander Korotkov
Reviewed-by: Tom Lane
Backpatch-through: 12