Tom Lane [Sun, 17 Nov 2019 01:00:19 +0000 (20:00 -0500)]
Further fix dumping of views that contain just VALUES(...).
It turns out that commit
e9f1c01b7 missed a case: we must print a
VALUES clause in long format if get_query_def is given a resultDesc
that would require the query's output column name(s) to be different
from what the bare VALUES clause would produce.
This applies in case an ALTER ... RENAME COLUMN has been done to
a view that formerly could be printed in simple format, as shown
in the added regression test case. It also explains bug #16119
from Dmitry Telpt, because it turns out that (unlike CREATE VIEW)
CREATE MATERIALIZED VIEW fails to apply any column aliases it's
given to the stored ON SELECT rule. So to get them to be printed,
we have to account for the resultDesc renaming. It might be worth
changing the matview code so that it creates the ON SELECT rule
with the correct aliases; but we'd still need these messy checks in
get_simple_values_rte to handle the case of a subsequent column
rename, so any such change would be just neatnik-ism not a bug fix.
Like the previous patch, back-patch to all supported branches.
Discussion: https://postgr.es/m/16119-
e64823f30a45a754@postgresql.org
Michael Paquier [Sat, 16 Nov 2019 06:23:50 +0000 (15:23 +0900)]
Improve stability of tests for VACUUM (SKIP_LOCKED)
Concurrent autovacuums running with the main regression test suite
could cause the tests with VACUUM (SKIP_LOCKED) to generate randomly
WARNING messages. For these tests, set client_min_messages to ERROR to
get rid of those random failures, as disabling autovacuum for the
relations operated would not completely close the failure window.
For isolation tests, disable autovacuum for the relations vacuumed with
SKIP_LOCKED. The tests are designed so as LOCK commands are taken
in a first session before running a concurrent VACUUM (SKIP_LOCKED) in a
second to generate WARNING messages, but a concurrent autovacuum could
cause the tests to be slower.
Reported-by: Tom Lane
Author: Michael Paquier
Reviewed-by: Andres Freund, Tom Lane
Discussion: https://postgr.es/m/25294.
1573077278@sss.pgh.pa.us
Backpatch-through: 12
Tomas Vondra [Sat, 16 Nov 2019 00:17:15 +0000 (01:17 +0100)]
Skip system attributes when applying mvdistinct stats
When estimating number of distinct groups, we failed to ignore system
attributes when matching the group expressions to mvdistinct stats,
causing failures like
ERROR: negative bitmapset member not allowed
Fix that by simply skipping anything that is not a regular attribute.
Backpatch to PostgreSQL 10, where the extended stats were introduced.
Bug: #16111
Reported-by: Tuomas Leikola
Author: Tomas Vondra
Backpatch-through: 10
Discussion: https://postgr.es/m/16111-
687799584c3a7e73@postgresql.org
Thomas Munro [Fri, 15 Nov 2019 21:04:52 +0000 (10:04 +1300)]
Always call ExecShutdownNode() if appropriate.
Call ExecShutdownNode() after ExecutePlan()'s loop, rather than at each
break. We had forgotten to do that in one case. The omission caused
intermittent "temporary file leak" warnings from multi-batch parallel
hash joins with a LIMIT clause.
Back-patch to 11. Though the problem exists in theory in earlier
parallel query releases, nothing really depended on it.
Author: Kyotaro Horiguchi
Reviewed-by: Thomas Munro, Amit Kapila
Discussion: https://postgr.es/m/
20191111.212418.
2222262873417235945.horikyota.ntt%40gmail.com
Tom Lane [Fri, 15 Nov 2019 17:31:53 +0000 (12:31 -0500)]
Doc: in v12 release notes, explain how to replace uses of consrc and adsrc.
While you can find that info if you drill down far enough, it seems more
helpful to put something right in the compatibility notes. Per a question
from Ivan Sergio Borgonovo.
Discussion: https://postgr.es/m/
a6359855-2a5e-a56c-ebba-
4ea46a1f0ebe@webthatworks.it
Tom Lane [Wed, 13 Nov 2019 20:53:53 +0000 (15:53 -0500)]
Add missing check_collation_set call to bpcharne().
We should throw an error for indeterminate collation, but bpcharne()
was missing that logic, resulting in a much less user-friendly error
(either an assertion failure or "cache lookup failed for collation 0").
Per report from Manuel Rigger. Back-patch to v12 where the mistake
came in, evidently in commit
5e1963fb7. (Before non-deterministic
collations, this function wasn't collation sensitive.)
Discussion: https://postgr.es/m/CA+u7OA4HOjtymxAbuGNh4-X_2R0Lw5n01tzvP8E5-i-2gQXYWA@mail.gmail.com
Tom Lane [Wed, 13 Nov 2019 20:26:54 +0000 (15:26 -0500)]
Fix silly initializations (cosmetic only).
Initializing a pointer to "false" isn't per project style,
and reportedly some compilers warn about it (though I've
not seen any such warnings in the buildfarm).
Seems to have come in with commit
ff11e7f4b, so back-patch
to v12 where that was added.
Didier Gautheron
Discussion: https://postgr.es/m/CAJRYxu+XQuM0qnSqt1Ujztu6fBPzMMAT3VEn6W32rgKG6A2Fsw@mail.gmail.com
Tom Lane [Wed, 13 Nov 2019 18:41:04 +0000 (13:41 -0500)]
Avoid downcasing/truncation of RADIUS authentication parameters.
Commit
6b76f1bb5 changed all the RADIUS auth parameters to be lists
rather than single values. But its use of SplitIdentifierString
to parse the list format was not very carefully thought through,
because that function thinks it's parsing SQL identifiers, which
means it will (a) downcase the strings and (b) truncate them to
be shorter than NAMEDATALEN. While downcasing should be harmless
for the server names and ports, it's just wrong for the shared
secrets, and probably for the NAS Identifier strings as well.
The truncation aspect is at least potentially a problem too,
though typical values for these parameters would fit in 63 bytes.
Fortunately, we now have a function SplitGUCList that is exactly
the same except for not doing the two unwanted things, so fixing
this is a trivial matter of calling that function instead.
While here, improve the documentation to show how to double-quote
the parameter values. I failed to resist the temptation to do
some copy-editing as well.
Report and patch from Marcos David (bug #16106); doc changes by me.
Back-patch to v10 where the aforesaid commit came in, since this is
arguably a regression from our previous behavior with RADIUS auth.
Discussion: https://postgr.es/m/16106-
7d319e4295d08e70@postgresql.org
Tom Lane [Wed, 13 Nov 2019 17:11:49 +0000 (12:11 -0500)]
Include TableFunc references when computing expression dependencies.
The TableFunc node (i.e., XMLTABLE) includes type and collation OIDs
that might not be referenced anywhere else in the expression tree,
so they need to be accounted for when extracting dependencies.
Fortunately, the practical effects of this are limited, since
(a) it's somewhat unlikely that people would be extracting
columns of non-builtin types from an XML document, and (b)
in many scenarios, the query would contain other references
to such types, or functions depending on them. However, it's
not hard to construct examples wherein the existing code lets
one drop a type used in XMLTABLE and thereby break a view.
This is evidently an original oversight in the XMLTABLE patch,
so back-patch to v10 where that came in.
Discussion: https://postgr.es/m/18427.
1573508501@sss.pgh.pa.us
Tom Lane [Wed, 13 Nov 2019 16:35:37 +0000 (11:35 -0500)]
Handle arrays and ranges in pg_upgrade's test for non-upgradable types.
pg_upgrade needs to check whether certain non-upgradable data types
appear anywhere on-disk in the source cluster. It knew that it has
to check for these types being contained inside domains and composite
types; but it somehow overlooked that they could be contained in
arrays and ranges, too. Extend the existing recursive-containment
query to handle those cases.
We probably should have noticed this oversight while working on
commit
0ccfc2822 and follow-ups, but we failed to :-(. The whole
thing's possibly a bit overdesigned, since we don't really expect
that any of these types will appear on disk; but if we're going to
the effort of doing a recursive search then it's silly not to cover
all the possibilities.
While at it, refactor so that we have only one copy of the search
logic, not three-and-counting. Also, to keep the branches looking
more alike, back-patch the output wording change of commit
1634d3615.
Back-patch to all supported branches.
Discussion: https://postgr.es/m/31473.
1573412838@sss.pgh.pa.us
Bruce Momjian [Wed, 13 Nov 2019 03:04:31 +0000 (22:04 -0500)]
docs: clarify that only INSERT and UPDATE triggers can mod. NEW
The point is that DELETE triggers cannot modify any values.
Reported-by: Eugen Konkov, Liudmila Mantrova
Discussion: https://postgr.es/m/
919823407.
20191029175436@yandex.ru
Backpatch-through: 12 only, where commit as missing
Tom Lane [Mon, 11 Nov 2019 22:03:10 +0000 (17:03 -0500)]
Stamp 12.1.
Tom Lane [Mon, 11 Nov 2019 19:39:54 +0000 (14:39 -0500)]
Doc: fix ancient mistake, or at least obsolete info, in rules example.
The example of expansion of multiple views claimed that the resulting
subquery nest would not get fully flattened because of an aggregate
function. There's no aggregate in the example, though, only a user
defined function confusingly named MIN(). In a modern server, the
reason for the non-flattening is that MIN() is volatile, but I'm
unsure whether that was true back when this text was written.
Let's reduce the confusion level by using LEAST() instead (which
we didn't have at the time this example was created). And then
we can just say that the planner will flatten the sub-queries, so
the rewrite system doesn't have to.
Noted by Paul Jungwirth. This text is old enough to vote, so
back-patch to all supported branches.
Discussion: https://postgr.es/m/CA+renyXZFnmp9PcvX1EVR2dR=XG5e6E-AELr8AHCNZ8RYrpnPw@mail.gmail.com
Tom Lane [Mon, 11 Nov 2019 15:33:00 +0000 (10:33 -0500)]
Further improve stability of partition_prune regression test.
Commits
4ea03f3f4 et al arranged to filter out row counts in parallel
plans, because those are dependent on the number of workers actually
obtained. Somehow I missed that the 'Rows Removed by Filter' counts
can also vary, so fix that too. Per buildfarm.
This seems worth a last-minute patch because unreliable regression
tests are a serious pain in the rear for packagers.
Like the previous patch, back-patch to v11 where this test was
introduced.
Peter Eisentraut [Mon, 11 Nov 2019 09:53:15 +0000 (10:53 +0100)]
Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash:
99bbc57cce0a1024898ac8d38b35fc6df7294e9e
Tom Lane [Sun, 10 Nov 2019 23:31:13 +0000 (18:31 -0500)]
Release notes for 12.1, 11.6, 10.11, 9.6.16, 9.5.20, 9.4.25.
Peter Eisentraut [Sat, 9 Nov 2019 12:19:27 +0000 (13:19 +0100)]
Fix subscription test
After altering a subscription, we should wait until the updated table
sync data has been fetched by the subscriber.
Peter Eisentraut [Sat, 9 Nov 2019 09:13:14 +0000 (10:13 +0100)]
doc: Clarify documentation about SSL passphrases
The previous statement that using a passphrase disables the ability to
change the server's SSL configuration without a server restart was no
longer completely true since the introduction of
ssl_passphrase_command_supports_reload.
Peter Eisentraut [Sat, 9 Nov 2019 08:35:21 +0000 (09:35 +0100)]
doc: Further tweak recovery parameters documentation
Remove one sentence that was deemed misleading.
Discussion: https://www.postgresql.org/message-id/flat/E1iEgSp-0004R5-2E%40gemulon.postgresql.org
Peter Eisentraut [Thu, 7 Nov 2019 12:48:59 +0000 (13:48 +0100)]
Fix negative bitmapset member not allowed error in logical replication
This happens when we add a replica identity column on a subscriber
that does not yet exist on the publisher, according to the mapping
maintained by the subscriber. Code that checks whether the target
relation on the subscriber is updatable would check the replica
identity attribute bitmap with a column number -1, which would result
in an error. To fix, skip such columns in the bitmap lookup and
consider the relation not updatable. The result is consistent with
the rule that the replica identity columns on the subscriber must be a
subset of those on the publisher, since if the column doesn't exist on
the publisher, the column set on the subscriber can't be a subset.
Reported-by: Tim Clarke
Analyzed-by: Jehan-Guillaume de Rorthais
Discussion: https://www.postgresql.org/message-id/flat/
a9139c29-7ddd-973b-aa7f-
71fed9c38d75%40minerva.info
Tom Lane [Sat, 9 Nov 2019 01:12:59 +0000 (20:12 -0500)]
First-draft release notes for 12.1.
As usual, the release notes for other branches will be made by cutting
these down, but put them up for community review first. Note that a
fair percentage of the entries apply only to prior branches because
their issue was already fixed in 12.0. I'll remove those from the 12.1
list later.
Peter Eisentraut [Fri, 8 Nov 2019 17:12:51 +0000 (18:12 +0100)]
Fix gratuitous error message variation
Etsuro Fujita [Fri, 8 Nov 2019 08:00:31 +0000 (17:00 +0900)]
postgres_fdw: Fix error message for PREPARE TRANSACTION.
Currently, postgres_fdw does not support preparing a remote transaction
for two-phase commit even in the case where the remote transaction is
read-only, but the old error message appeared to imply that that was not
supported only if the remote transaction modified remote tables. Change
the message so as to include the case where the remote transaction is
read-only.
Also fix a comment above the message.
Also add a note about the lack of supporting PREPARE TRANSACTION to the
postgres_fdw documentation.
Reported-by: Gilles Darold
Author: Gilles Darold and Etsuro Fujita
Reviewed-by: Michael Paquier and Kyotaro Horiguchi
Backpatch-through: 9.4
Discussion: https://postgr.es/m/
08600ed3-3084-be70-65ba-
279ab19618a5%40darold.net
Tom Lane [Thu, 7 Nov 2019 19:21:52 +0000 (14:21 -0500)]
Move declaration of ecpg_gettext() to a saner place.
Declaring this in the client-visible header ecpglib.h was a pretty
poor decision. It's not meant to be application-callable (and if
it was, putting it outside the extern "C" { ... } wrapper means
that C++ clients would fail to call it). And the declaration would
not even compile for a client, anyway, since it would not have the
macro pg_attribute_format_arg(). Fortunately, it seems that no
clients have tried to include this header with ENABLE_NLS defined,
or we'd have gotten complaints about that. But we have no business
putting such a restriction on client code.
Move the declaration to ecpglib_extern.h, since in fact nothing
outside src/interfaces/ecpg/ecpglib/ needs to call it.
The practical effect of this is just that clients can now safely
#include ecpglib.h while having ENABLE_NLS defined, but that seems
like enough of a reason to back-patch it.
Discussion: https://postgr.es/m/20590.
1573069709@sss.pgh.pa.us
Alvaro Herrera [Thu, 7 Nov 2019 16:59:24 +0000 (13:59 -0300)]
Fix SET CONSTRAINTS .. DEFERRED on partitioned tables
SET CONSTRAINTS ... DEFERRED failed on partitioned tables, because of a
sanity check that ensures that the affected constraints have triggers.
On partitioned tables, the triggers are in the leaf partitions, not in
the partitioned relations themselves, so the sanity check fails.
Removing the sanity check solves the problem, because the code needed to
support the case is already there.
Backpatch to 11.
Note: deferred unique constraints are not affected by this bug, because
they do have triggers in the parent partitioned table. I did not add a
test for this scenario.
Discussion: https://postgr.es/m/
20191105212915[email protected]
Tom Lane [Thu, 7 Nov 2019 16:22:52 +0000 (11:22 -0500)]
Fix integer-overflow edge case detection in interval_mul and pgbench.
This patch adopts the overflow check logic introduced by commit
cbdb8b4c0
into two more places. interval_mul() failed to notice if it computed a
new microseconds value that was one more than INT64_MAX, and pgbench's
double-to-int64 logic had the same sorts of edge-case problems that
cbdb8b4c0 fixed in the core code.
To make this easier to get right in future, put the guts of the checks
into new macros in c.h, and add commentary about how to use the macros
correctly.
Back-patch to all supported branches, as we did with the previous fix.
Yuya Watari
Discussion: https://postgr.es/m/CAJ2pMkbkkFw2hb9Qb1Zj8d06EhWAQXFLy73St4qWv6aX=vqnjw@mail.gmail.com
Fujii Masao [Thu, 7 Nov 2019 07:31:36 +0000 (16:31 +0900)]
Fix assertion failure when running pgbench -s.
If there is the WAL page that the continuation WAL record just fits within
(i.e., the continuation record ends just at the end of the page) and
the LSN in such page is specified with -s option, previously pg_waldump
caused an assertion failure. The cause of this assertion failure was that
XLogFindNextRecord() that pg_waldump -s calls mistakenly handled
such special WAL page.
This commit changes XLogFindNextRecord() so that it can handle
such WAL page correctly.
Back-patch to all supported versions.
Author: Andrey Lepikhov
Reviewed-by: Fujii Masao, Michael Paquier
Discussion: https://postgr.es/m/
99303554-5dd5-06e6-f943-
b3005ccd6edd@postgrespro.ru
Tomas Vondra [Mon, 4 Nov 2019 01:00:26 +0000 (02:00 +0100)]
Document log_transaction_sample_rate as superuser-only
The docs do say which GUCs can be changed only by superusers, but we
forgot to mention this for the new log_transaction_sample_rate. This
GUC was introduced in PostgreSQL 12, so backpatch accordingly.
Author: Adrien Nayrat
Backpatch-through: 12
Tom Lane [Wed, 6 Nov 2019 17:00:17 +0000 (12:00 -0500)]
Minor code review for tuple slot rewrite.
Avoid creating transiently-inconsistent slot states where possible,
by not setting TTS_FLAG_SHOULDFREE until after the slot actually has
a free'able tuple pointer, and by making sure that we reset tts_nvalid
and related derived state before we replace the tuple contents. This
would only matter if something were to examine the slot after we'd
suffered some kind of error (e.g. out of memory) while manipulating
the slot. We typically don't do that, so these changes might just be
cosmetic --- but even if so, it seems like good future-proofing.
Also remove some redundant Asserts, and add a couple for consistency.
Back-patch to v12 where all this code was rewritten.
Discussion: https://postgr.es/m/16095-
c3ff2e5283b8dba5@postgresql.org
Tom Lane [Wed, 6 Nov 2019 16:11:40 +0000 (11:11 -0500)]
Sync our DTrace infrastructure with c.h's definition of type bool.
Since commit
d26a810eb, we've defined bool as being either _Bool from
, or "unsigned char"; but that commit overlooked the fact
that probes.d has "#define bool char". For consistency, make it say
"unsigned char" instead. This should be strictly a cosmetic change,
but it seems best to be in sync.
Formally, in the now-normal case where we're using , it'd
be better to write "#define bool _Bool". However, then we'd need
some build infrastructure to inject that configuration choice into
probes.d, and it doesn't seem worth the trouble. We only use
if sizeof(_Bool) is 1, so having DTrace think that
bool parameters are "unsigned char" should be close enough.
Back-patch to v12 where d26a810eb came in.
Discussion: https://postgr.es/m/CAA4eK1LmaKO7Du9M9Lo=kxGU8sB6aL8fa3sF6z6d5yYYVe3BuQ@mail.gmail.com
Peter Eisentraut [Wed, 6 Nov 2019 13:20:29 +0000 (14:20 +0100)]
Fix memory allocation mistake
The previous code was allocating more memory than necessary because
the formula used the wrong data type.
Reported-by: Jehan-Guillaume de Rorthais
Discussion: https://www.postgresql.org/message-id/
20191105172918.
3e32a446@firost
Michael Paquier [Wed, 6 Nov 2019 07:12:28 +0000 (16:12 +0900)]
Fix timestamp of sent message for write context in logical decoding
When sending data for logical decoding using the streaming replication
protocol via a WAL sender, the timestamp of the sent write message is
allocated at the beginning of the message when preparing for the write,
and actually computed when the write message is ready to be sent.
The timestamp was getting computed after sending the message. This
impacts anything using logical decoding, causing for example logical
replication to report mostly NULL for last_msg_send_time in
pg_stat_subscription.
This commit makes sure that the timestamp is computed before sending the
message. This is wrong since
5a991ef, so backpatch down to 9.4.
Author: Jeff Janes
Discussion: https://postgr.es/m/CAMkU=1z=WMn8jt7iEdC5sYNaPgAgOASb_OW5JYv-vMdYaJSL-w@mail.gmail.com
Backpatch-through: 9.4
Andrew Gierth [Wed, 6 Nov 2019 04:13:30 +0000 (04:13 +0000)]
Request small targetlist for input to WindowAgg.
WindowAgg will potentially store large numbers of input rows into
tuplestores to allow access to other rows in the frame. If the input
is coming via an explicit Sort node, then unneeded columns will
already have been discarded (since Sort requests a small tlist); but
there are idioms like COUNT(*) OVER () that result in the input not
being sorted at all, and cases where the input is being sorted by some
means other than a Sort; if we don't request a small tlist, then
WindowAgg's storage requirement is inflated by the unneeded columns.
Backpatch back to 9.6, where the current tlist handling was added.
(Prior to that, WindowAgg would always use a small tlist.)
Discussion: https://postgr.es/m/
[email protected]
Bruce Momjian [Wed, 6 Nov 2019 02:29:02 +0000 (21:29 -0500)]
doc: fix for plurality typo on bgwriter doc sentence
Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/
20191106022353[email protected]
Backpatch-through: 11, 12
Bruce Momjian [Wed, 6 Nov 2019 01:54:04 +0000 (20:54 -0500)]
doc: fix plurality typo on bgwriter doc sentence
Reported-by: [email protected]
Discussion: https://postgr.es/m/
157204060717.1042.
8194076510523669244@wrigleys.postgresql.org
Backpatch-through: 9.4
Tom Lane [Tue, 5 Nov 2019 19:27:37 +0000 (14:27 -0500)]
Avoid logging complaints about abandoned connections when using PAM.
For a long time (since commit
aed378e8d) we have had a policy to log
nothing about a connection if the client disconnects when challenged
for a password. This is because libpq-using clients will typically
do that, and then come back for a new connection attempt once they've
collected a password from their user, so that logging the abandoned
connection attempt will just result in log spam. However, this did
not work well for PAM authentication: the bottom-level function
pam_passwd_conv_proc() was on board with it, but we logged messages
at higher levels anyway, for lack of any reporting mechanism.
Add a flag and tweak the logic so that the case is silent, as it is
for other password-using auth mechanisms.
Per complaint from Yoann La Cancellera. It's been like this for awhile,
so back-patch to all supported branches.
Discussion: https://postgr.es/m/CACP=ajbrFFYUrLyJBLV8=q+eNCapa1xDEyvXhMoYrNphs-xqPw@mail.gmail.com
Tom Lane [Tue, 5 Nov 2019 18:40:37 +0000 (13:40 -0500)]
Fix "unexpected relkind" error when denying permissions on toast tables.
get_relkind_objtype, and hence get_object_type, failed when applied to a
toast table. This is not a good thing, because it prevents reporting of
perfectly legitimate permissions errors. (At present, these functions
are in fact *only* used to determine the ObjectType argument for
acl_error() calls.) It seems best to have them fall back to returning
OBJECT_TABLE in every case where they can't determine an object type
for a pg_class entry, so do that.
In passing, make some edits to alter.c to make it more obvious that
those calls of get_object_type() are used only for error reporting.
This might save a few cycles in the non-error code path, too.
Back-patch to v11 where this issue originated.
John Hsu, Michael Paquier, Tom Lane
Discussion: https://postgr.es/m/
C652D3DF-2B0C-4128-9420-
FB5379F6B1E4@amazon.com
Tom Lane [Tue, 5 Nov 2019 16:42:25 +0000 (11:42 -0500)]
Generate EquivalenceClass members for partitionwise child join rels.
Commit
d25ea0127 got rid of what I thought were entirely unnecessary
derived child expressions in EquivalenceClasses for EC members that
mention multiple baserels. But it turns out that some of the child
expressions that code created are necessary for partitionwise joins,
else we fail to find matching pathkeys for Sort nodes. (This happens
only for certain shapes of the resulting plan; it may be that
partitionwise aggregation is also necessary to show the failure,
though I'm not sure of that.)
Reverting that commit entirely would be quite painful performance-wise
for large partition sets. So instead, add code that explicitly
generates child expressions that match only partitionwise child join
rels we have actually generated.
Per report from Justin Pryzby. (Amit Langote noticed the problem
earlier, though it's not clear if he recognized then that it could
result in a planner error, not merely failure to exploit partitionwise
join, in the code as-committed.) Back-patch to v12 where commit
d25ea0127 came in.
Amit Langote, with lots of kibitzing from me
Discussion: https://postgr.es/m/CA+HiwqG2WVUGmLJqtR0tPFhniO=H=9qQ+Z3L_ZC+Y3-EVQHFGg@mail.gmail.com
Discussion: https://postgr.es/m/
20191011143703[email protected]
Michael Paquier [Tue, 5 Nov 2019 01:32:43 +0000 (10:32 +0900)]
Doc: Clarify locks taken when using ALTER TABLE ATTACH PARTITION
Since
898e5e32, this command uses partially ShareUpdateExclusiveLock,
but the docs did not get the call.
Author: Justin Pryzby
Reviewed-by: Amit Langote, Álvaro Herrera, Michael Paquier
Discussion: https://postgr.es/m/
20191028001207[email protected]
Backpatch-through: 12
Michael Paquier [Tue, 5 Nov 2019 01:17:55 +0000 (10:17 +0900)]
Doc: Improve description around ALTER TABLE ATTACH PARTITION
This clarifies more how to use and how to take advantage of constraints
when attaching a new partition.
Author: Justin Pryzby
Reviewed-by: Amit Langote, Álvaro Herrera, Michael Paquier
Discussion: https://postgr.es/m/
20191028001207[email protected]
Backpatch-through: 10
Tom Lane [Mon, 4 Nov 2019 21:25:05 +0000 (16:25 -0500)]
Stabilize pg_dump output order for similarly-named triggers and policies.
The code only compared two triggers' names and namespaces (the latter
being the owning table's schema). This could result in falling back
to an OID-based sort of similarly-named triggers on different tables.
We prefer to avoid that, so add a comparison of the table names too.
(The sort order is thus table namespace, trigger name, table name,
which is a bit odd, but it doesn't seem worth contorting the code
to work around that.)
Likewise for policy objects, in 9.5 and up.
Complaint and fix by Benjie Gillam. Back-patch to all supported
branches.
Discussion: https://postgr.es/m/CAMThMzEEt2mvBbPgCaZ1Ap1N-moGn=Edxmadddjq89WG4NpPtQ@mail.gmail.com
Peter Eisentraut [Mon, 4 Nov 2019 07:30:00 +0000 (08:30 +0100)]
Catch invalid typlens in a couple of places
Rearrange the logic in record_image_cmp() and datum_image_eq() to
error out on unexpected typlens (either not supported there or
completely invalid due to corruption). Barring corruption, this is
not possible today but it seems more future-proof and robust to fix
this.
Reported-by: Peter Geoghegan
Tom Lane [Sun, 3 Nov 2019 21:10:23 +0000 (16:10 -0500)]
Suppress warning from older compilers.
Commit
8af1624e3 introduced a warning about possibly returning
without a value, on compilers that don't realize that ereport(ERROR)
doesn't return. Tweak the code to avoid that.
Per buildfarm. Back-patch to 9.6, like the aforesaid commit.
Tom Lane [Sat, 2 Nov 2019 20:45:32 +0000 (16:45 -0400)]
Validate ispell dictionaries more carefully.
Using incorrect, or just mismatched, dictionary and affix files
could result in a crash, due to failure to cross-check offsets
obtained from the file. Add necessary validation, as well as
some Asserts for future-proofing.
Per bug #16050 from Alexander Lakhin. Back-patch to 9.6 where the
problem was introduced.
Arthur Zakirov, per initial investigation by Tomas Vondra
Discussion: https://postgr.es/m/16050-
024ae722464ab604@postgresql.org
Discussion: https://postgr.es/m/
20191013012610.2p2fp3zzpoav7jzf@development
Michael Paquier [Sat, 2 Nov 2019 05:16:11 +0000 (14:16 +0900)]
Fix failure when creating cloned indexes for a partition
When using CREATE TABLE for a new partition, the partitioned indexes of
the parent are created automatically in a fashion similar to LIKE
INDEXES. The new partition and its parent use a mapping for attribute
numbers for this operation, and while the mapping was correctly built,
its length was defined as the number of attributes of the newly-created
child, and not the parent. If the parent includes dropped columns, this
could cause failures.
This is wrong since
8b08f7d which has introduced the concept of
partitioned indexes, so backpatch down to 11.
Reported-by: Wyatt Alt
Author: Michael Paquier
Reviewed-by: Amit Langote
Discussion: https://postgr.es/m/CAGem3qCcRmhbs4jYMkenYNfP2kEusDXvTfw-q+eOhM0zTceG-g@mail.gmail.com
Backpatch-through: 11
Michael Paquier [Fri, 1 Nov 2019 13:38:45 +0000 (22:38 +0900)]
Fix race condition at backend exit when deleting element in syncrep queue
When a backend exits, it gets deleted from the syncrep queue if present.
The queue was checked without SyncRepLock taken in exclusive mode, so it
would have been possible for a backend to remove itself after a WAL
sender already did the job. Fix this issue based on a suggestion from
Fujii Masao, by first checking the queue without the lock. Then, if the
backend is present in the queue, take the lock and perform an additional
lookup check before doing the element deletion.
Author: Dongming Liu
Reviewed-by: Kyotaro Horiguchi, Fujii Masao, Michael Paquier
Discussion: https://postgr.es/m/
a0806273-8bbb-43b3-bbe1-
c45a58f6ae21[email protected]
Backpatch-through: 9.4
Bruce Momjian [Wed, 30 Oct 2019 17:43:16 +0000 (13:43 -0400)]
relnotes: PG 12, mention change in libpq parameter parsing
Reported-by: [email protected]
Diagnosed-by: Michael Paquier
Discussion: https://postgr.es/m/
157123155668.25311.
9369950798665566339@wrigleys.postgresql.org
Author: Michael Paquier
Reviewed-by: me
Backpatch-through: 12 only
Andres Freund [Wed, 30 Oct 2019 05:46:40 +0000 (22:46 -0700)]
pg_waldump: Fix --bkp-details to not issue spurious newlines for FPWs.
The additional newline seems to have accidentally been introduced in
2c03216d831, in 9.5. The newline is only issued when an FPW is
present for the block reference.
While there could be an argument that removing the newlines in the
back branches could cause a problem for somebody parsing the
pg_waldump output, the likelihood of that seems small enough. It seems
at least equally likely that the randomness of when newlines are
issued causes problems.
Author: Andres Freund
Discussion: https://postgr.es/m/
20191029233341[email protected]
Backpatch: 9.5, like
2c03216d831.
Andres Freund [Wed, 30 Oct 2019 02:18:07 +0000 (19:18 -0700)]
pg_waldump: Fix small memory leak when rmgr->rm_identify returns NULL.
This got broken in
604f7956b94, shortly after rm_identify's
introduction.
Author: Andres Freund
Discussion: https://postgr.es/m/
20191029233341[email protected]
Backpatch: 9.5, where rm_identify was introduced
Michael Paquier [Tue, 29 Oct 2019 02:08:16 +0000 (11:08 +0900)]
Fix handling of pg_class.relispartition at swap phase in REINDEX CONCURRENTLY
When cancelling REINDEX CONCURRENTLY after swapping the old and new
indexes (for example interruption at step 5), the old index remains
around and is marked as invalid. The old index should also be manually
droppable to clean up the parent relation from any invalid indexes still
remaining. For a partition index reindexed, pg_class.relispartition was
not getting updated, causing the index to not be droppable as DROP INDEX
would look for dependencies in a partition tree, which do not exist
anymore after the swap phase is done.
The fix here is simple: when swapping the old and new indexes, make sure
that pg_class.relispartition is correctly switched, similarly to what is
done for the index name.
Reported-by: Justin Pryzby
Author: Michael Paquier
Discussion: https://postgr.es/m/
20191015164047[email protected]
Backpatch-through: 12
Tom Lane [Mon, 28 Oct 2019 16:21:13 +0000 (12:21 -0400)]
Handle empty-string edge cases correctly in strpos().
Commit
9556aa01c rearranged the innards of text_position() in a way
that would make it not work for empty search strings. Which is fine,
because all callers of that code special-case an empty pattern in
some way. However, the primary use-case (text_position itself) got
special-cased incorrectly: historically it's returned 1 not 0 for
an empty search string. Restore the historical behavior.
Per complaint from Austin Drenski (via Shay Rojansky).
Back-patch to v12 where it got broken.
Discussion: https://postgr.es/m/CADT4RqAz7oN4vkPir86Kg1_mQBmBxCp-L_=9vRpgSNPJf0KRkw@mail.gmail.com
Michael Paquier [Mon, 28 Oct 2019 05:23:48 +0000 (14:23 +0900)]
Doc: Add missing step for pg_stat_progress_cluster
There is a step to track when the new heap is written, but this was
missing in the documentation.
Author: Noriyoshi Shinoda
Discussion: https://postgr.es/m/AT5PR8401MB06447FAE88E1592754E958B8EE640@AT5PR8401MB0644.NAMPRD84.PROD.OUTLOOK.COM
Backpatch-through: 12
Michael Paquier [Mon, 28 Oct 2019 02:58:29 +0000 (11:58 +0900)]
Fix dependency handling at swap phase of REINDEX CONCURRENTLY
When swapping the dependencies of the old and new indexes, the code has
been correctly switching all links in pg_depend from the old to the new
index for both referencing and referenced entries. However it forgot
the fact that the new index may itself have existing entries in
pg_depend, like references to the parent table attributes. This
resulted in duplicated entries in pg_depend after running REINDEX
CONCURRENTLY.
Fix this problem by removing any existing entries in pg_depend for the
new index before switching the dependencies of the old index to the new
one. More regression tests are added to check the consistency of
entries in pg_depend for indexes, including partitions.
Author: Michael Paquier
Discussion: https://postgr.es/m/
20191025064318[email protected]
Backpatch-through: 12
Michael Paquier [Sun, 27 Oct 2019 04:54:20 +0000 (13:54 +0900)]
Fix initialization of fake LSN for unlogged relations
9155580 has changed the value of the first fake LSN for unlogged
relations from 1 to FirstNormalUnloggedLSN (aka 1000), GiST requiring a
non-zero LSN on some pages to allow an interlocking logic to work, but
its value was still initialized to 1 at the beginning of recovery or
after running pg_resetwal. This fixes the initialization for both code
paths.
Author: Takayuki Tsunakawa
Reviewed-by: Dilip Kumar, Kyotaro Horiguchi, Michael Paquier
Discussion: https://postgr.es/m/OSBPR01MB2503CE851940C17DE44AE3D9FE6F0@OSBPR01MB2503.jpnprd01.prod.outlook.com
Backpatch-through: 12
Tom Lane [Sat, 26 Oct 2019 16:30:41 +0000 (12:30 -0400)]
Doc: improve documentation of configuration settings that have units.
When we added the GUC units feature, we didn't make any great effort
to adjust the documentation of individual GUCs; they tended to still
say things like "this is the number of milliseconds that ...", even
though users might prefer to write some other units, and SHOW might
even show the value in other units. Commit
6c9fb69f2 made an effort
to improve this situation, but I thought it made things less readable
by injecting units information in mid-sentence. It also wasn't very
consistent, and did not touch all the GUCs that have units.
To improve matters, standardize on the phrasing "If this value is
specified without units, it is taken as
". Also, try to
standardize where this is mentioned, right before the specification
of the default. (In a couple of places, doing that would've required
more rewriting than seemed justified, so I wasn't 100% consistent
about that.) I also tried to use the phrases "amount of time",
"amount of memory", etc rather than describing the contents of GUCs
in other ways, as those were the majority usage in places that weren't
overcommitting to a particular unit. (I left "length of time" alone
in a couple of places, though.)
I failed to resist the temptation to copy-edit some awkward text, too.
Backpatch to v12, like 6c9fb69f2, mainly because v12 hasn't diverged
much from HEAD yet.
Discussion: https://postgr.es/m/15882.1571942223@sss.pgh.pa.us
Tom Lane [Fri, 25 Oct 2019 19:22:40 +0000 (15:22 -0400)]
Avoid failure when selecting a namespace node in XMLTABLE.
It appears that libxml2 doesn't bother to set the "children" field of
an XML_NAMESPACE_DECL node to null; that field just contains garbage.
In v10 and v11, this can result in a crash in XMLTABLE(). The rewrite
done in commit
251cf2e27 fixed this, somewhat accidentally, in v12.
We're not going to back-patch
251cf2e27, however. The case apparently
doesn't have wide use, so rather than risk introducing other problems,
just add a safety check to throw an error.
Even though no bug manifests in v12/HEAD, add the relevant test case
there too, to prevent future regressions.
Chapman Flack (per private report)
Tom Lane [Fri, 25 Oct 2019 16:17:41 +0000 (12:17 -0400)]
Get rid of useless/dangerous redefinition of bool in ECPG.
pgtypeslib_extern.h contained fallback definitions of "bool", "FALSE",
and "TRUE". The latter two are just plain unused, and have been for
awhile. The former came into play only if there wasn't a macro
definition of "bool", which is true only if we aren't using
.
However, it then defined bool as "char"; since commit d26a810eb that
conflicts with c.h's desire to use "unsigned char". We'd missed seeing
any bad effects of that due to accidental header inclusion order choices,
but dddf4cdc3 exposed that it was problematic.
To fix, let's just get rid of these definitions. They should not be
needed because everyplace in Postgres should be relying on c.h to
provide a definition for type bool. (Note that despite its name,
pgtypeslib_extern.h isn't exposed to any outside code; we don't
install it.)
This doesn't fully resolve the issue, because ecpglib.h is doing
similar things, but that seems to require more thought to fix.
Back-patch to v12 where d26a810eb came in, to forestall any unpleasant
surprises from future back-patched bug fixes.
Discussion: https://postgr.es/m/CAA4eK1LmaKO7Du9M9Lo=kxGU8sB6aL8fa3sF6z6d5yYYVe3BuQ@mail.gmail.com
Michael Paquier [Fri, 25 Oct 2019 01:20:16 +0000 (10:20 +0900)]
Handle interrupts within a transaction context in REINDEX CONCURRENTLY
Phases 2 (building the new index) and 3 (validating the new index)
checked for interrupts outside a transaction context, having as
consequence to not release session-level locks taken on the parent
relation and the old and new indexes processed. This could for example
be triggered with statement_timeout and a bad timing, and would issue
confusing error messages when shutting down the session still holding
the locks (note that an assertion failure would be triggered first), on
top of more issues with concurrent sessions trying to take a lock that
would interfere with the SHARE UPDATE EXCLUSIVE locks hold here.
This moves all the interruption checks inside a transaction context.
Note that I have manually tested all interruptions to make sure that
invalid indexes can be cleaned up properly. Partition indexes still
have issues on their own with some missing dependency handling, which
will be dealt with in a follow-up patch.
Reported-by: Justin Pryzby
Author: Michael Paquier
Discussion: https://postgr.es/m/
20191013025145[email protected]
Backpatch-through: 12
Bruce Momjian [Thu, 24 Oct 2019 00:29:02 +0000 (20:29 -0400)]
docs: fix wording of REFRESH CONCURRENTLY manual page
Reported-by: Asim / [email protected]
Discussion: https://postgr.es/m/
157076828181.26165.
15231292023740913543@wrigleys.postgresql.org
Backpatch-through: 9.4
Michael Paquier [Wed, 23 Oct 2019 06:05:09 +0000 (15:05 +0900)]
Acquire properly session-level lock on new index in REINDEX CONCURRENTLY
In the first transaction run for REINDEX CONCURRENTLY, a thinko in the
existing logic caused two session locks to be taken on the old index,
causing the session lock on the newly-created index to be missed. This
made possible concurrent DDL commands (like ALTER INDEX) on the new
index while REINDEX CONCURRENTLY was processing from the point where the
first internal transaction committed.
This issue has been discovered while digging into another bug.
Author: Michael Paquier
Discussion: https://postgr.es/m/
20191021074323[email protected]
Backpatch-through: 12
Michael Paquier [Wed, 23 Oct 2019 02:34:42 +0000 (11:34 +0900)]
Fix thinkos from
4f4061b for libpq integer parsing
A check was redundant. While on it, add an assertion to make sure that
the parsing routine is never called with a NULL input. All the code
paths currently calling the parsing routine are careful with NULL inputs
already, but future callers may forget that.
Reported-by: Peter Eisentraut, Lars Kanis
Discussion: https://postgr.es/m/
ec64956b-4597-56b6-c3db-
457d15250fe4@2ndquadrant.com
Backpatch-through: 12
Michael Paquier [Wed, 23 Oct 2019 01:25:46 +0000 (10:25 +0900)]
Clean up properly error_context_stack in autovacuum worker on exception
Any callback set would have no meaning in the context of an exception.
As an autovacuum worker exits quickly in this context, this could be
only an issue within EmitErrorReport(), where the elog hook is for
example called. That's unlikely to going to be a problem, but let's be
clean and consistent with other code paths handling exceptions. This is
present since
2909419, which introduced autovacuum.
Author: Ashwin Agrawal
Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/CALfoeisM+_+dgmAdAOHAu0k-ZpEHHqSSG=GRf3pKJGm8OqWX0w@mail.gmail.com
Backpatch-through: 9.4
Tom Lane [Mon, 21 Oct 2019 18:18:01 +0000 (14:18 -0400)]
Deal with yet another issue related to "Norwegian (Bokmål)" locale.
It emerges that recent versions of Windows (at least 2016 Standard)
spell this locale name as "Norwegian Bokmål_Norway.1252", defeating
our mapping code that translates "Norwegian (Bokmål)_Norway" to
something that's all-ASCII (cf commits
db29620d4 and
aa1d2fc5e).
Add another mapping entry to handle this spelling.
Per bug #16068 from Robert Ford. Like the previous patches,
back-patch to all supported branches.
Discussion: https://postgr.es/m/16068-
4cb6eeaa7eb46d93@postgresql.org
Tom Lane [Mon, 21 Oct 2019 17:52:25 +0000 (13:52 -0400)]
Use CFLAGS_SL while probing linkability of libperl.
On recent Red Hat platforms (at least RHEL 8 and Fedora 30, maybe older),
configure's probe for libperl failed if the user forces CFLAGS to be -O0.
This is because some code in perl's inline.h fails to be optimized away
at -O0, and said code doesn't work if compiled without -fPIC.
To fix, add CFLAGS_SL to the compile flags used during the libperl probe.
This is a better simulation of the way that plperl is built, anyway,
so it might forestall other issues in future.
Per gripe from Kyotaro Horiguchi. Back-patch to all supported branches,
since people might want to build older branches on these platforms.
Discussion: https://postgr.es/m/
20191010.144533.
263180400[email protected]
Tom Lane [Mon, 21 Oct 2019 16:32:35 +0000 (12:32 -0400)]
Select CFLAGS_SL at configure time, not in platform-specific Makefiles.
Move the platform-dependent logic that sets CFLAGS_SL from
src/makefiles/Makefile.foo to src/template/foo, so that the value
is determined at configure time and thus is available while running
configure's tests.
On a couple of platforms this might save a few microseconds of build
time by eliminating a test that make otherwise has to do over and over.
Otherwise it's pretty much a wash for build purposes; in particular,
this makes no difference to anyone who might be overriding CFLAGS_SL
via a make option.
This patch in itself does nothing with the value and thus should not
change any behavior, though you'll probably have to re-run configure
to get a correctly updated Makefile.global. We'll use the new
configure variable in a follow-on patch.
Per gripe from Kyotaro Horiguchi. Back-patch to all supported branches,
because the follow-on patch is a portability bug fix.
Discussion: https://postgr.es/m/
20191010.144533.
263180400[email protected]
Etsuro Fujita [Mon, 21 Oct 2019 08:30:01 +0000 (17:30 +0900)]
Update obsolete comment.
Commit
b52b7dc25, which moved code creating PartitionBoundInfo in
RelationBuildPartitionDesc() in partcache.c (relocated to partdesc.c
afterwards) to partbounds.c, should have updated this, but didn't.
Author: Etsuro Fujita
Reviewed-by: Alvaro Herrera
Backpatch-through: 12
Discussion: https://postgr.es/m/CAPmGK16Uxr%3DPatiGyaRwiQVLB7Y-GqbkK3AxRLVYzU0Czv%3DsEw%40mail.gmail.com
Amit Kapila [Thu, 17 Oct 2019 03:15:43 +0000 (08:45 +0530)]
Fix memory leak introduced in commit
7df159a620.
We memorize all internal and empty leaf pages in the 1st vacuum stage for
gist indexes. They are used in the 2nd stage, to delete all the empty
pages. There was a memory context page_set_context for this purpose, but
we never used it.
Reported-by: Amit Kapila
Author: Dilip Kumar
Reviewed-by: Amit Kapila
Backpatch-through: 12, where it got introduced
Discussion: https://postgr.es/m/CAA4eK1LGr+MN0xHZpJ2dfS8QNQ1a_aROKowZB+MPNep8FVtwAA@mail.gmail.com
Michael Paquier [Mon, 21 Oct 2019 02:39:28 +0000 (11:39 +0900)]
Fix error reporting of connect_timeout in libpq for value parsing
The logic was correctly detecting a parsing failure, but the parsing
error did not get reported back to the client properly.
Reported-by: Ed Morley
Author: Lars Kanis
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/
a9b4cbd7-4ecb-06b2-ebd7-
1739bbff3217@greiz-reinsdorf.de
Backpatch-through: 12
Michael Paquier [Mon, 21 Oct 2019 02:17:43 +0000 (11:17 +0900)]
Fix parsing of integer values for connection parameters in libpq
Commit
e7a2217 has introduced stricter checks for integer values in
connection parameters for libpq. However this failed to correctly check
after trailing whitespaces, while leading whitespaces were discarded per
the use of strtol(3). This fixes and refactors the parsing logic to
handle both cases consistently. Note that trying to restrict the use of
trailing whitespaces can easily break connection strings like in ECPG
regression tests (these have allowed me to catch the parsing bug with
connect_timeout).
Author: Michael Paquier
Reviewed-by: Lars Kanis
Discussion: https://postgr.es/m/
a9b4cbd7-4ecb-06b2-ebd7-
1739bbff3217@greiz-reinsdorf.de
Backpatch-through: 12
Noah Misch [Sat, 19 Oct 2019 03:20:28 +0000 (20:20 -0700)]
For PowerPC instruction "addi", use constraint "b".
Without "b", a variant of the tas() code miscompiles on macOS 10.4.
This may also fix a compilation failure involving macOS 10.1. Today's
compilers have been allocating acceptable registers with or without this
change, but this future-proofs the code by precisely conveying the
acceptable registers. Back-patch to 9.4 (all supported versions).
Reviewed by Tom Lane.
Discussion: https://postgr.es/m/
20191009063900[email protected]
Fujii Masao [Fri, 18 Oct 2019 13:32:18 +0000 (22:32 +0900)]
Fix failure of archive recovery with recovery_min_apply_delay enabled.
recovery_min_apply_delay parameter is intended for use with streaming
replication deployments. However, the document clearly explains that
the parameter will be honored in all cases if it's specified. So it should
take effect even if in archive recovery. But, previously, archive recovery
with recovery_min_apply_delay enabled always failed, and caused assertion
failure if --enable-caasert is enabled.
The cause of this problem is that; the ownership of recoveryWakeupLatch
that recovery_min_apply_delay uses was taken only when standby mode
is requested. So unowned latch could be used in archive recovery, and
which caused the failure.
This commit changes recovery code so that the ownership of
recoveryWakeupLatch is taken even in archive recovery. Which prevents
archive recovery with recovery_min_apply_delay from failing.
Back-patch to v9.4 where recovery_min_apply_delay was added.
Author: Fujii Masao
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/CAHGQGwEyD6HdZLfdWc+95g=VQFPR4zQL4n+yHxQgGEGjaSVheQ@mail.gmail.com
Fujii Masao [Fri, 18 Oct 2019 13:24:18 +0000 (22:24 +0900)]
Make crash recovery ignore recovery_min_apply_delay setting.
In v11 or before, this setting could not take effect in crash recovery
because it's specified in recovery.conf and crash recovery always
starts without recovery.conf. But commit
2dedf4d9a8 integrated
recovery.conf into postgresql.conf and which unexpectedly allowed
this setting to take effect even in crash recovery. This is definitely
not good behavior.
To fix the issue, this commit makes crash recovery always ignore
recovery_min_apply_delay setting.
Back-patch to v12 where the issue was added.
Author: Fujii Masao
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/CAHGQGwEyD6HdZLfdWc+95g=VQFPR4zQL4n+yHxQgGEGjaSVheQ@mail.gmail.com
Discussion: https://postgr.es/m/
e445616d-023e-a268-8aa1-
67b8b335340c@pgmasters.net
Alvaro Herrera [Fri, 18 Oct 2019 12:49:39 +0000 (14:49 +0200)]
Fix typo
Apparently while this code was being developed,
ReindexRelationConcurrently operated on multiple relations. The version
that was ultimately pushed doesn't, so this comment's use of plural is
inaccurate.
Alvaro Herrera [Fri, 18 Oct 2019 10:18:50 +0000 (07:18 -0300)]
Update comments about progress reporting by index_drop
Michaël Paquier complained that index_drop is requesting progress
reporting for non-obvious reasons, so let's add a comment to explain
why.
Discussion: https://postgr.es/m/
20191017010412[email protected]
Michael Paquier [Fri, 18 Oct 2019 05:26:53 +0000 (14:26 +0900)]
Fix timeout handling in logical replication worker
The timestamp tracking the last moment a message is received in a
logical replication worker was initialized in each loop checking if a
message was received or not, causing wal_receiver_timeout to be ignored
in basically any logical replication deployments. This also broke the
ping sent to the server when reaching half of wal_receiver_timeout.
This simply moves the initialization of the timestamp out of the apply
loop to the beginning of LogicalRepApplyLoop().
Reported-by: Jehan-Guillaume De Rorthais
Author: Julien Rouhaud
Discussion: https://postgr.es/m/CAOBaU_ZHESFcWva8jLjtZdCLspMj7vqaB2k++rjHLY897ZxbYw@mail.gmail.com
Backpatch-through: 10
Alvaro Herrera [Thu, 17 Oct 2019 13:06:06 +0000 (15:06 +0200)]
Fix minor bug in logical-replication walsender shutdown
Logical walsender should exit when it catches up with sending WAL during
shutdown; but there was a rare corner case when it failed to because of
a race condition that puts it back to wait for more WAL instead -- but
since there wasn't any, it'd not shut down immediately. It would only
continue the shutdown when wal_sender_timeout terminates the sleep,
which causes annoying waits during shutdown procedure. Restructure the
code so that we no longer forget to set WalSndCaughtUp in that case.
This was an oversight in commit
c6c333436.
Backpatch all the way down to 9.4.
Author: Craig Ringer, Álvaro Herrera
Discussion: https://postgr.es/m/CAMsr+YEuz4XwZX_QmnX_-2530XhyAmnK=zCmicEnq1vLr0aZ-g@mail.gmail.com
Alvaro Herrera [Thu, 17 Oct 2019 07:58:01 +0000 (09:58 +0200)]
Fix parallel restore of FKs to partitioned tables
When an FK constraint is created, it needs the index on the referenced
table to exist and be valid. When doing parallel pg_restore and the
referenced table was partitioned, this condition can sometimes not be
met, because pg_dump didn't emit sufficient object dependencies to
ensure so; this means that parallel pg_restore would fail in certain
conditions. Fix by having pg_dump make the FK constraint object
dependent on the partition attachment objects for the constraint's
referenced index.
This has been broken since
f56f8f8da6af, so backpatch to Postgres 12.
Discussion: https://postgr.es/m/
20191005224333[email protected]
Thomas Munro [Thu, 17 Oct 2019 00:24:50 +0000 (13:24 +1300)]
When restoring GUCs in parallel workers, show an error context.
Otherwise it can be hard to see where an error is coming from, when
the parallel worker sets all the GUCs that it received from the
leader. Bug #15726. Back-patch to 9.5, where RestoreGUCState()
appeared.
Reported-by: Tiago Anastacio
Reviewed-by: Daniel Gustafsson, Tom Lane
Discussion: https://postgr.es/m/15726-
6d67e4fa14f027b3%40postgresql.org
Thomas Munro [Wed, 16 Oct 2019 20:59:21 +0000 (09:59 +1300)]
Fix bug that could try to freeze running multixacts.
Commits
801c2dc7 and
801c2dc7 made it possible for vacuum to
try to freeze a multixact that is still running. That was
prevented by a check, but raised an error. Repair.
Back-patch all the way.
Author: Nathan Bossart, Jeremy Schneider
Reported-by: Jeremy Schneider
Reviewed-by: Jim Nasby, Thomas Munro
Discussion: https://postgr.es/m/
DAFB8AFF-2F05-4E33-AD7F-
FF8B0F760C17%40amazon.com
Alvaro Herrera [Wed, 16 Oct 2019 12:51:23 +0000 (14:51 +0200)]
Fix crash when reporting CREATE INDEX progress
A race condition can make us try to dereference a NULL pointer to the
PGPROC struct of a process that's already finished. That results in
crashes during REINDEX CONCURRENTLY and CREATE INDEX CONCURRENTLY.
This was introduced in
ab0dfc961b6a, so backpatch to pg12.
Reported by: Justin Pryzby
Reviewed-by: Michaël Paquier
Discussion: https://postgr.es/m/
20191012004446[email protected]
Tomas Vondra [Wed, 16 Oct 2019 11:23:18 +0000 (13:23 +0200)]
Improve the check for pg_catalog.unknown data type in pg_upgrade
The pg_upgrade check for pg_catalog.unknown type when upgrading from 9.6
had a couple of issues with domains and composite types - it detected
even composite types unused in objects with storage. So for example this
was enough to trigger an unnecessary pg_upgrade failure:
CREATE TYPE unknown_composite AS (u pg_catalog.unknown)
On the other hand, this only happened with composite types directly on
the pg_catalog.unknown data type, but not with a domain. So this was not
detected
CREATE DOMAIN unknown_domain AS pg_catalog.unknown;
CREATE TYPE unknown_composite_2 AS (u unknown_domain);
unlike the first example. These false positives and inconsistencies are
unfortunate, but what's worse we've failed to detected objects using the
pg_catalog.unknown type through a domain. So we missed cases like this
CREATE TABLE t (u unknown_composite_2);
The consequence is clusters broken after a pg_upgrade.
This fixes these false positives and false negatives by using the same
recursive CTE introduced by
eaf900e842 for sql_identifier. Backpatch all
the way to 10, where the of pg_catalog.unknown data type was restricted.
Author: Tomas Vondra
Backpatch-to: 10-
Discussion: https://postgr.es/m/16045-
673e8fa6b5ace196%40postgresql.org
Tomas Vondra [Wed, 16 Oct 2019 11:23:14 +0000 (13:23 +0200)]
Improve the check for pg_catalog.line data type in pg_upgrade
The pg_upgrade check for pg_catalog.line data type when upgrading from
9.3 had a couple of issues with domains and composite types. Firstly, it
triggered false positives for composite types unused in objects with
storage. This was enough to trigger an unnecessary pg_upgrade failure:
CREATE TYPE line_composite AS (l pg_catalog.line)
On the other hand, this only happened with composite types directly on
the pg_catalog.line data type, but not with a domain. So this was not
detected
CREATE DOMAIN line_domain AS pg_catalog.line;
CREATE TYPE line_composite_2 AS (l line_domain);
unlike the first example. These false positives and inconsistencies are
unfortunate, but what's worse we've failed to detected objects using the
pg_catalog.line data type through a domain. So we missed cases like this
CREATE TABLE t (l line_composite_2);
The consequence is clusters broken after a pg_upgrade.
This fixes these false positives and false negatives by using the same
recursive CTE introduced by
eaf900e842 for sql_identifier. 9.3 did not
support domains on composite types, but we can still have multi-level
composite types.
Backpatch all the way to 9.4, where the format for pg_catalog.line data
type changed.
Author: Tomas Vondra
Backpatch-to: 9.4-
Discussion: https://postgr.es/m/16045-
673e8fa6b5ace196%40postgresql.org
Andres Freund [Wed, 16 Oct 2019 09:37:34 +0000 (02:37 -0700)]
Replace alter_table.sql test usage of event triggers.
The test in
93765bd956b added an event trigger to ensure that the
tested table rewrites do not get optimized away (as happened in the
past). But doing so would require running the tests in isolation, as
otherwise the trigger might also fire in concurrent sessions, causing
test failures there.
Reported-By: Tom Lane
Discussion: https://postgr.es/m/3328.
1570740683@sss.pgh.pa.us
Backpatch: 12, just as
93765bd956b
Michael Paquier [Wed, 16 Oct 2019 04:15:11 +0000 (13:15 +0900)]
Doc: Fix incorrect contributor name in release notes
A typo from commit
27b4121 is at the origin of the mistake.
Author: Alexander Lakhin
Discussion: https://postgr.es/m/
bf95233a-9943-b341-e2ff-
a860c28af481@gmail.com
Michael Paquier [Wed, 16 Oct 2019 04:10:31 +0000 (13:10 +0900)]
Doc: Fix various inconsistencies
This fixes multiple areas of the documentation:
- COPY for its past compatibility section.
- SET ROLE mentioning INHERITS instead of INHERIT
- PREPARE referring to stmt_name, that is not present.
- Extension documentation about format name with upgrade scripts.
Backpatch down to 9.4 for the relevant parts.
Author: Alexander Lakhin
Discussion: https://postgr.es/m/
bf95233a-9943-b341-e2ff-
a860c28af481@gmail.com
Backpatch-through: 9.4
Andres Freund [Tue, 15 Oct 2019 17:40:13 +0000 (10:40 -0700)]
Fix CLUSTER on expression indexes.
Since the introduction of different slot types, in
1a0586de3657, we
create a virtual slot in tuplesort_begin_cluster(). While that looks
right, it unfortunately doesn't actually work, as ExecStoreHeapTuple()
is used to store tuples in the slot. Unfortunately no regression tests
for CLUSTER on expression indexes existed so far.
Fix the slot type, and add bare bones tests for CLUSTER on expression
indexes.
Reported-By: Justin Pryzby
Author: Andres Freund
Discussion: https://postgr.es/m/
20191011210320[email protected]
Backpatch: 12, like
1a0586de3657
Tomas Vondra [Mon, 14 Oct 2019 22:25:04 +0000 (00:25 +0200)]
Correct reference to pg_catalog.regtype in pg_upgrade query
The recursive CTE added in
0ccfc28223 referenced pg_catalog.regtype,
without the schema part, unlike all other queries in pg_upgrade.
Backpatch-to: 12
Tomas Vondra [Mon, 14 Oct 2019 20:31:56 +0000 (22:31 +0200)]
Check for tables with sql_identifier during pg_upgrade
Commit
7c15cef86d changed sql_identifier data type to be based on name
instead of varchar. Unfortunately, this breaks on-disk format for this
data type. Luckily, that should be a very rare problem, as this data
type is used only in information_schema views, so this only affects user
objects (tables, materialized views and indexes). One way to end in
such situation is to do CTAS with a query on those system views.
There are two options to deal with this - we can either abort pg_upgrade
if there are user objects with sql_identifier columns in pg_upgrade, or
we could replace the sql_identifier type with varchar. Considering how
rare the issue is expected to be, and the complexity of replacing the
data type (e.g. in matviews), we've decided to go with the simple check.
The query is somewhat complex - the sql_identifier data type may be used
indirectly - through a domain, a composite type or both, possibly in
multiple levels. Detecting this requires a recursive CTE.
Backpatch to 12, where the sql_identifier definition changed.
Reported-by: Hans Buschmann
Author: Tomas Vondra
Reviewed-by: Tom Lane
Backpatch-to: 12
Discussion: https://postgr.es/m/16045-
673e8fa6b5ace196%40postgresql.org
Michael Paquier [Sun, 13 Oct 2019 23:58:47 +0000 (08:58 +0900)]
Update test output of sepgsql for ALTER TABLE COLUMN DROP
1df5875 has changed the way dependencies are dropped for this command
with inheritance trees, which impacts sepgsql. This just updates the
regression test output to take care of the failures and adapt to the new
code.
Reported by buildfarm member rhinoceros.
Author: Michael Paquier
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/
20191013101331[email protected]
Backpatch-through: 12
Michael Paquier [Sun, 13 Oct 2019 08:53:08 +0000 (17:53 +0900)]
Fix dependency handling of column drop with partitioned tables
When dropping a column on a partitioned table which has one or more
partitioned indexes, the operation was failing as dependencies with
partitioned indexes using the column dropped were not getting removed in
a way consistent with the columns involved across all the relations part
of an inheritance tree.
This commit refactors the code executing column drop so as all the
columns from an inheritance tree to remove are gathered first, and
dropped all at the end. This way, we let the dependency machinery sort
out by itself the deletion of all the columns with the partitioned
indexes across a partition tree.
This issue has been introduced by
1d92a0c, so backpatch down to
REL_12_STABLE.
Author: Amit Langote, Michael Paquier
Reviewed-by: Álvaro Herrera, Ashutosh Sharma
Discussion: https://postgr.es/m/CA+HiwqE9kuBsZ3b5pob2-cvE8ofzPWs-og+g8bKKGnu6b4-yTQ@mail.gmail.com
Backpatch-through: 12
Noah Misch [Sat, 12 Oct 2019 07:21:47 +0000 (00:21 -0700)]
AIX: Stop adding option -qsrcmsg.
With xlc v16.1.0, it causes internal compiler errors. With xlc versions
not exhibiting that bug, removing -qsrcmsg merely changes the compiler
error reporting format. Back-patch to 9.4 (all supported versions).
Discussion: https://postgr.es/m/
20191003064105[email protected]
Fujii Masao [Fri, 11 Oct 2019 06:47:59 +0000 (15:47 +0900)]
Make crash recovery ignore restore_command and recovery_end_command settings.
In v11 or before, those settings could not take effect in crash recovery
because they are specified in recovery.conf and crash recovery always
starts without recovery.conf. But commit
2dedf4d9a8 integrated
recovery.conf into postgresql.conf and which unexpectedly allowed
those settings to take effect even in crash recovery. This is definitely
not good behavior.
To fix the issue, this commit makes crash recovery always ignore
restore_command and recovery_end_command settings.
Back-patch to v12 where the issue was added.
Author: Fujii Masao
Reviewed-by: Peter Eisentraut
Discussion: https://postgr.es/m/
e445616d-023e-a268-8aa1-
67b8b335340c@pgmasters.net
Tom Lane [Thu, 10 Oct 2019 18:24:57 +0000 (14:24 -0400)]
Put back pqsignal() as an exported libpq symbol.
This reverts commit
f7ab80285. Per discussion, we can't remove an
exported symbol without a SONAME bump, which we don't want to do.
In particular that breaks usage of current libpq.so with pre-9.3
versions of psql etc, which need libpq to export pqsignal().
As noted in that commit message, exporting the symbol from libpgport.a
won't work reliably; but actually we don't want to export src/port's
implementation anyway. Any pre-9.3 client is going to be expecting the
definition that pqsignal() had before 9.3, which was that it didn't
set SA_RESTART for SIGALRM. Hence, put back pqsignal() in a separate
source file in src/interfaces/libpq, and give it the old semantics.
Back-patch to v12.
Discussion: https://postgr.es/m/
[email protected]
Andres Freund [Thu, 10 Oct 2019 05:00:50 +0000 (22:00 -0700)]
Fix table rewrites that include a column without a default.
In
c2fe139c201c I made ATRewriteTable() use tuple slots. Unfortunately
I did not notice that columns can be added in a rewrite that do not
have a default, when another column is added/altered requiring one.
Initialize columns to NULL again, and add tests.
Bug: #16038
Reported-By: anonymous
Author: Andres Freund
Discussion: https://postgr.es/m/16038-
5c974541f2bf6749@postgresql.org
Backpatch: 12, where the bug was introduced in
c2fe139c201c
Michael Paquier [Wed, 9 Oct 2019 04:31:13 +0000 (13:31 +0900)]
Flush logical mapping files with fd opened for read/write at checkpoint
The file descriptor was opened with read-only to fsync a regular file,
which would cause EBADFD errors on some platforms.
This is similar to the recent fix done by
a586cc4b (which was broken by
me with
82a5649), except that I noticed this issue while monitoring the
backend code for similar mistakes. Backpatch to 9.4, as this has been
introduced since logical decoding exists as of
b89e151.
Author: Michael Paquier
Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/
20191006045548[email protected]
Backpatch-through: 9.4
Bruce Momjian [Wed, 9 Oct 2019 01:49:08 +0000 (21:49 -0400)]
doc: improve docs so config value default units are clearer
Previously, our docs would say "Specifies the number of milliseconds"
but it wasn't clear that "milliseconds" was merely the default unit.
New text says "Specifies duration (defaults to milliseconds)", which is
clearer.
Reported-by: [email protected]
Discussion: https://postgr.es/m/15912-
2e35e9026f61230b@postgresql.org
Backpatch-through: 12
Bruce Momjian [Mon, 7 Oct 2019 22:06:08 +0000 (18:06 -0400)]
docs: Improve A?synchronous Multimaster Replication descr.
The docs for sync and async multimaster replication were unclear about
when to use it, and when it has benefits; this change clarifies that.
Reported-by: [email protected]
Discussion: https://postgr.es/m/
156856543824.1274.
12180817186798859836@wrigleys.postgresql.org
Backpatch-through: 9.4
Bruce Momjian [Mon, 7 Oct 2019 21:26:46 +0000 (17:26 -0400)]
docs: clarify that today/tomorrow/yesterday is at 00:00
This should help people clearly know that these days start at midnight.
Reported-by: David Harper
Discussion: https://postgr.es/m/
156258047907.1181.
11324468080514061996@wrigleys.postgresql.org
Backpatch-through: 9.4
Bruce Momjian [Mon, 7 Oct 2019 18:33:31 +0000 (14:33 -0400)]
doc: move mention of log_min_error_statement in a better spot
Previously it was mentioned in the lock_timeout docs in a confusing
location.
Reported-by: [email protected]
Discussion: https://postgr.es/m/
157019615723.25307.
15449102262106437404@wrigleys.postgresql.org
Backpatch-through: 9.4
Tom Lane [Mon, 7 Oct 2019 16:39:09 +0000 (12:39 -0400)]
Check for too many postmaster children before spawning a bgworker.
The postmaster's code path for spawning a bgworker neglected to check
whether we already have the max number of live child processes. That's
a bit hard to hit, since it would necessarily be a transient condition;
but if we do, AssignPostmasterChildSlot() fails causing a postmaster
crash, as seen in a report from Bhargav Kamineni.
To fix, invoke canAcceptConnections() in the bgworker code path, as we
do in the other code paths that spawn children. Since we don't want
the same pmState tests in this case, add a child-process-type parameter
to canAcceptConnections() so that it can know what to do.
Back-patch to 9.5. In principle the same hazard exists in 9.4, but the
code is enough different that this patch wouldn't quite fix it there.
Given the tiny usage of bgworkers in that branch it doesn't seem worth
creating a variant patch for it.
Discussion: https://postgr.es/m/18733.
1570382257@sss.pgh.pa.us