]> git.ipfire.org Git - thirdparty/postgresql.git/log
thirdparty/postgresql.git
5 years agoFix misbehavior with expression indexes on ON COMMIT DELETE ROWS tables.
Tom Lane [Sun, 1 Dec 2019 18:09:27 +0000 (13:09 -0500)] 
Fix misbehavior with expression indexes on ON COMMIT DELETE ROWS tables.

We implement ON COMMIT DELETE ROWS by truncating tables marked that
way, which requires also truncating/rebuilding their indexes.  But
RelationTruncateIndexes asks the relcache for up-to-date copies of any
index expressions, which may cause execution of eval_const_expressions
on them, which can result in actual execution of subexpressions.
This is a bad thing to have happening during ON COMMIT.  Manuel Rigger
reported that use of a SQL function resulted in crashes due to
expectations that ActiveSnapshot would be set, which it isn't.
The most obvious fix perhaps would be to push a snapshot during
PreCommit_on_commit_actions, but I think that would just open the door
to more problems: CommitTransaction explicitly expects that no
user-defined code can be running at this point.

Fortunately, since we know that no tuples exist to be indexed, there
seems no need to use the real index expressions or predicates during
RelationTruncateIndexes.  We can set up dummy index expressions
instead (we do need something that will expose the right data type,
as there are places that build index tupdescs based on this), and
just ignore predicates and exclusion constraints.

In a green field it'd likely be better to reimplement ON COMMIT DELETE
ROWS using the same "init fork" infrastructure used for unlogged
relations.  That seems impractical without catalog changes though,
and even without that it'd be too big a change to back-patch.
So for now do it like this.

Per private report from Manuel Rigger.  This has been broken forever,
so back-patch to all supported branches.

5 years agoFix off-by-one error in PGTYPEStimestamp_fmt_asc
Tomas Vondra [Sat, 30 Nov 2019 13:51:27 +0000 (14:51 +0100)] 
Fix off-by-one error in PGTYPEStimestamp_fmt_asc

When using %b or %B patterns to format a date, the code was simply using
tm_mon as an index into array of month names. But that is wrong, because
tm_mon is 1-based, while array indexes are 0-based. The result is we
either use name of the next month, or a segfault (for December).

Fix by subtracting 1 from tm_mon for both patterns, and add a regression
test triggering the issue. Backpatch to all supported versions (the bug
is there far longer, since at least 2003).

Reported-by: Paul Spencer
Backpatch-through: 9.4
Discussion: https://postgr.es/m/16143-0d861eb8688d3fef%40postgresql.org

5 years agoFix typo in comment.
Etsuro Fujita [Wed, 27 Nov 2019 07:00:49 +0000 (16:00 +0900)] 
Fix typo in comment.

5 years agoDon't shut down Gather[Merge] early under Limit.
Amit Kapila [Mon, 18 Nov 2019 08:47:41 +0000 (14:17 +0530)] 
Don't shut down Gather[Merge] early under Limit.

Revert part of commit 19df1702f5.

Early shutdown was added by that commit so that we could collect
statistics from workers, but unfortunately, it interacted badly with
rescans.  The problem is that we ended up destroying the parallel context
which is required for rescans.  This leads to rescans of a Limit node over
a Gather node to produce unpredictable results as it tries to access
destroyed parallel context.  By reverting the early shutdown code, we
might lose statistics in some cases of Limit over Gather [Merge], but that
will require further study to fix.

Reported-by: Jerry Sievers
Diagnosed-by: Thomas Munro
Author: Amit Kapila, testcase by Vignesh C
Backpatch-through: 9.6
Discussion: https://postgr.es/m/87ims2amh6.fsf@jsievers.enova.com

5 years agoAvoid assertion failure with LISTEN in a serializable transaction.
Tom Lane [Sun, 24 Nov 2019 20:57:31 +0000 (15:57 -0500)] 
Avoid assertion failure with LISTEN in a serializable transaction.

If LISTEN is the only action in a serializable-mode transaction,
and the session was not previously listening, and the notify queue
is not empty, predicate.c reported an assertion failure.  That
happened because we'd acquire the transaction's initial snapshot
during PreCommit_Notify, which was called *after* predicate.c
expects any such snapshot to have been established.

To fix, just swap the order of the PreCommit_Notify and
PreCommit_CheckForSerializationFailure calls during CommitTransaction.
This will imply holding the notify-insertion lock slightly longer,
but the difference could only be meaningful in serializable mode,
which is an expensive option anyway.

It appears that this is just an assertion failure, with no
consequences in non-assert builds.  A snapshot used only to scan
the notify queue could not have been involved in any serialization
conflicts, so there would be nothing for
PreCommit_CheckForSerializationFailure to do except assign it a
prepareSeqNo and set the SXACT_FLAG_PREPARED flag.  And given no
conflicts, neither of those omissions affect the behavior of
ReleasePredicateLocks.  This admittedly once-over-lightly analysis
is backed up by the lack of field reports of trouble.

Per report from Mark Dilger.  The bug is old, so back-patch to all
supported branches; but the new test case only goes back to 9.6,
for lack of adequate isolationtester infrastructure before that.

Discussion: https://postgr.es/m/3ac7f397-4d5f-be8e-f354-440020675694@gmail.com
Discussion: https://postgr.es/m/13881.1574557302@sss.pgh.pa.us

5 years agodoc: Fix whitespace in syntax.
Thomas Munro [Sun, 24 Nov 2019 20:20:28 +0000 (09:20 +1300)] 
doc: Fix whitespace in syntax.

Back-patch to 10.

Author: Andreas Karlsson
Discussion: https://postgr.es/m/043acae2-a369-b7fa-be48-1933aa2e82d1%40proxel.se

5 years agoStabilize NOTIFY behavior by transmitting notifies before ReadyForQuery.
Tom Lane [Sun, 24 Nov 2019 19:42:59 +0000 (14:42 -0500)] 
Stabilize NOTIFY behavior by transmitting notifies before ReadyForQuery.

This patch ensures that, if any notify messages were received during
a just-finished transaction, they get sent to the frontend just before
not just after the ReadyForQuery message.  With libpq and other client
libraries that act similarly, this guarantees that the client will see
the notify messages as available as soon as it thinks the transaction
is done.

This probably makes no difference in practice, since in realistic
use-cases the application would have to cope with asynchronous
arrival of notify events anyhow.  However, it makes it a lot easier
to build cross-session-notify test cases with stable behavior.
I'm a bit surprised now that we've not seen any buildfarm instability
with the test cases added by commit b10f40bf0.  Tests that I intend
to add in an upcoming bug fix are definitely unstable without this.

Back-patch to 9.6, which is as far back as we can do NOTIFY testing
with the isolationtester infrastructure.

Discussion: https://postgr.es/m/13881.1574557302@sss.pgh.pa.us

5 years agoImprove test coverage for LISTEN/NOTIFY.
Tom Lane [Sat, 23 Nov 2019 22:30:01 +0000 (17:30 -0500)] 
Improve test coverage for LISTEN/NOTIFY.

Back-patch commit b10f40bf0 into older branches.  This adds reporting
of NOTIFY messages to isolationtester.c, and extends the async-notify
test to include direct tests of basic NOTIFY functionality.

This provides useful infrastructure for testing a bug fix I'm about
to back-patch, and there seems no good reason not to have better tests
of LISTEN/NOTIFY in the back branches.  The commit's survived long
enough in HEAD to make it unlikely that it will cause problems.

Back-patch as far as 9.6.  isolationtester.c changed too much in 9.6
to make it sane to try to fix older branches this way, and I don't
really want to back-patch those changes too.

Discussion: https://postgr.es/m/31304.1564246011@sss.pgh.pa.us

5 years agoAdd test coverage for "unchanged toast column" replication code path.
Tom Lane [Fri, 22 Nov 2019 17:52:26 +0000 (12:52 -0500)] 
Add test coverage for "unchanged toast column" replication code path.

It seems pretty unacceptable to have no regression test coverage
for this aspect of the logical replication protocol, especially
given the bugs we've found in related code.

Discussion: https://postgr.es/m/16129-a0c0f48e71741e5f@postgresql.org

5 years agoFix bogus tuple-slot management in logical replication UPDATE handling.
Tom Lane [Fri, 22 Nov 2019 16:31:19 +0000 (11:31 -0500)] 
Fix bogus tuple-slot management in logical replication UPDATE handling.

slot_modify_cstrings seriously abused the TupleTableSlot API by relying
on a slot's underlying data to stay valid across ExecClearTuple.  Since
this abuse was also quite undocumented, it's little surprise that the
case got broken during the v12 slot rewrites.  As reported in bug #16129
from Ondřej Jirman, this could lead to crashes or data corruption when
a logical replication subscriber processes a row update.  Problems would
only arise if the subscriber's table contained columns of pass-by-ref
types that were not being copied from the publisher.

Fix by explicitly copying the datum/isnull arrays from the source slot
that the old row was in already.  This ends up being about the same
thing that happened pre-v12, but hopefully in a less opaque and
fragile way.

We might've caught the problem sooner if there were any test cases
dealing with updates involving non-replicated or dropped columns.
Now there are.

Back-patch to v10 where this code came in.  Even though the failure
does not manifest before v12, IMO this code is too fragile to leave
as-is.  In any case we certainly want the additional test coverage.

Patch by me; thanks to Tomas Vondra for initial investigation.

Discussion: https://postgr.es/m/16129-a0c0f48e71741e5f@postgresql.org

5 years agoDefend against self-referential views in relation_is_updatable().
Tom Lane [Thu, 21 Nov 2019 21:21:44 +0000 (16:21 -0500)] 
Defend against self-referential views in relation_is_updatable().

While a self-referential view doesn't actually work, it's possible
to create one, and it turns out that this breaks some of the
information_schema views.  Those views call relation_is_updatable(),
which neglected to consider the hazards of being recursive.  In
older PG versions you get a "stack depth limit exceeded" error,
but since v10 it'd recurse to the point of stack overrun and crash,
because commit a4c35ea1c took out the expression_returns_set() call
that was incidentally checking the stack depth.

Since this function is only used by information_schema views, it
seems like it'd be better to return "not updatable" than suffer
an error.  Hence, add tracking of what views we're examining,
in just the same way that the nearby fireRIRrules() code detects
self-referential views.  I added a check_stack_depth() call too,
just to be defensive.

Per private report from Manuel Rigger.  Back-patch to all
supported versions.

5 years agoProvide statistics for hypothetical BRIN indexes
Michael Paquier [Thu, 21 Nov 2019 01:23:49 +0000 (10:23 +0900)] 
Provide statistics for hypothetical BRIN indexes

Trying to use hypothetical indexes with BRIN currently fails when trying
to access a relation that does not exist when looking for the
statistics.  With the current API, it is not possible to easily pass
a value for pages_per_range down to the hypothetical index, so this
makes use of the default value of BRIN_DEFAULT_PAGES_PER_RANGE, which
should be fine enough in most cases.

Being able to refine or enforce the hypothetical costs in more
optimistic ways would require more refactoring by filling in the
statistics when building IndexOptInfo in plancat.c.  This would involve
ABI breakages around the costing routines, something not fit for stable
branches.

This is broken since 7e534ad, so backpatch down to v10.

Author: Julien Rouhaud, Heikki Linnakangas
Reviewed-by: Álvaro Herrera, Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/CAOBaU_ZH0LKEA8VFCocr6Lpte1ab0b6FpvgS0y4way+RPSXfYg@mail.gmail.com
Backpatch-through: 10

5 years agoRemove incorrect markup
Magnus Hagander [Wed, 20 Nov 2019 16:03:07 +0000 (17:03 +0100)] 
Remove incorrect markup

Author: Daniel Gustafsson <daniel@yesql.se>

5 years agoRevise GIN README
Alexander Korotkov [Tue, 19 Nov 2019 20:11:24 +0000 (23:11 +0300)] 
Revise GIN README

We find GIN concurrency bugs from time to time.  One of the problems here is
that concurrency of GIN isn't well-documented in README.  So, it might be even
hard to distinguish design bugs from implementation bugs.

This commit revised concurrency section in GIN README providing more details.
Some examples are illustrated in ASCII art.

Also, this commit add the explanation of how is tuple layout in internal GIN
B-tree page different in comparison with nbtree.

Discussion: https://postgr.es/m/CAPpHfduXR_ywyaVN4%2BOYEGaw%3DcPLzWX6RxYLBncKw8de9vOkqw%40mail.gmail.com
Author: Alexander Korotkov
Reviewed-by: Peter Geoghegan
Backpatch-through: 9.4

5 years agoFix traversing to the deleted GIN page via downlink
Alexander Korotkov [Tue, 19 Nov 2019 20:08:14 +0000 (23:08 +0300)] 
Fix traversing to the deleted GIN page via downlink

Current GIN code appears to don't handle traversing to the deleted page via
downlink.  This commit fixes that by stepping right from the delete page like
we do in nbtree.

This commit also fixes setting 'deleted' flag to the GIN pages.  Now other page
flags are not erased once page is deleted.  That helps to keep our assertions
true if we arrive deleted page via downlink.

Discussion: https://postgr.es/m/CAPpHfdvMvsw-NcE5bRS7R1BbvA4BxoDnVVjkXC5W0Czvy9LVrg%40mail.gmail.com
Author: Alexander Korotkov
Reviewed-by: Peter Geoghegan
Backpatch-through: 9.4

5 years agoFix deadlock between ginDeletePage() and ginStepRight()
Alexander Korotkov [Tue, 19 Nov 2019 20:07:36 +0000 (23:07 +0300)] 
Fix deadlock between ginDeletePage() and ginStepRight()

When ginDeletePage() is about to delete page it locks its left sibling to revise
the rightlink.  So, it locks pages in right to left manner.  Int he same time
ginStepRight() locks pages in left to right manner, and that could cause a
deadlock.

This commit makes ginScanToDelete() keep exclusive lock on left siblings of
currently investigated path.  That elimites need to relock left sibling in
ginDeletePage().  Thus, deadlock with ginStepRight() can't happen anymore.

Reported-by: Chen Huajun
Discussion: https://postgr.es/m/5c332bd1.87b6.16d7c17aa98.Coremail.chjischj%40163.com
Author: Alexander Korotkov
Reviewed-by: Peter Geoghegan
Backpatch-through: 10

5 years agoDoc: clarify use of RECURSIVE in WITH.
Tom Lane [Tue, 19 Nov 2019 19:43:37 +0000 (14:43 -0500)] 
Doc: clarify use of RECURSIVE in WITH.

Apparently some people misinterpreted the syntax as being that
RECURSIVE is a prefix of individual WITH queries.  It's a modifier
for the WITH clause as a whole, so state that more clearly.

Discussion: https://postgr.es/m/ca53c6ce-a0c6-b14a-a8e3-162f0b2cc119@a-kretschmer.de

5 years agoDoc: clarify behavior of ALTER DEFAULT PRIVILEGES ... IN SCHEMA.
Tom Lane [Tue, 19 Nov 2019 19:21:41 +0000 (14:21 -0500)] 
Doc: clarify behavior of ALTER DEFAULT PRIVILEGES ... IN SCHEMA.

The existing text stated that "Default privileges that are specified
per-schema are added to whatever the global default privileges are for
the particular object type".  However, that bare-bones observation is
not quite clear enough, as demonstrated by the complaint in bug #16124.
Flesh it out by stating explicitly that you can't revoke built-in
default privileges this way, and by providing an example to drive
the point home.

Back-patch to all supported branches, since it's been like this
from the beginning.

Discussion: https://postgr.es/m/16124-423d8ee4358421bc@postgresql.org

5 years agoFurther fix dumping of views that contain just VALUES(...).
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

5 years agoSkip system attributes when applying mvdistinct stats
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

5 years agoAvoid downcasing/truncation of RADIUS authentication parameters.
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

5 years agoInclude TableFunc references when computing expression dependencies.
Tom Lane [Wed, 13 Nov 2019 17:11:50 +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

5 years agoHandle arrays and ranges in pg_upgrade's test for non-upgradable types.
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

5 years agoStamp 10.11. REL_10_11
Tom Lane [Mon, 11 Nov 2019 22:07:14 +0000 (17:07 -0500)] 
Stamp 10.11.

5 years agoDoc: fix ancient mistake, or at least obsolete info, in rules example.
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

5 years agoTranslation updates
Peter Eisentraut [Mon, 11 Nov 2019 09:33:28 +0000 (10:33 +0100)] 
Translation updates

Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: f052c053b7992cde6b854c5ab1b51055f4506133

5 years agoRelease notes for 12.1, 11.6, 10.11, 9.6.16, 9.5.20, 9.4.25.
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.

5 years agoFix subscription test
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.

5 years agoFix negative bitmapset member not allowed error in logical replication
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 <tim.clarke@minerva.info>
Analyzed-by: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
Discussion: https://www.postgresql.org/message-id/flat/a9139c29-7ddd-973b-aa7f-71fed9c38d75%40minerva.info

5 years agoFix gratuitous error message variation
Peter Eisentraut [Fri, 8 Nov 2019 17:12:51 +0000 (18:12 +0100)] 
Fix gratuitous error message variation

5 years agopostgres_fdw: Fix error message for PREPARE TRANSACTION.
Etsuro Fujita [Fri, 8 Nov 2019 08:00:34 +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

5 years agodocs: clarify that only INSERT and UPDATE triggers can mod. NEW
Bruce Momjian [Thu, 7 Nov 2019 20:49:59 +0000 (15:49 -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
Discussion: https://postgr.es/m/919823407.20191029175436@yandex.ru

Backpatch-through: 9.4

5 years agoMove declaration of ecpg_gettext() to a saner place.
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

5 years agoFix integer-overflow edge case detection in interval_mul and pgbench.
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

5 years agoFix assertion failure when running pgbench -s.
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

5 years agoFix memory allocation mistake
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 <jgdr@dalibo.com>
Discussion: https://www.postgresql.org/message-id/20191105172918.3e32a446@firost

5 years agoFix timestamp of sent message for write context in logical decoding
Michael Paquier [Wed, 6 Nov 2019 07:12:40 +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

5 years agoRequest small targetlist for input to WindowAgg.
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/87a7ator8n.fsf@news-spur.riddles.org.uk

5 years agodoc: fix plurality typo on bgwriter doc sentence
Bruce Momjian [Wed, 6 Nov 2019 01:54:04 +0000 (20:54 -0500)] 
doc:  fix plurality typo on bgwriter doc sentence

Reported-by: matthew.alton@gmail.com
Discussion: https://postgr.es/m/157204060717.1042.8194076510523669244@wrigleys.postgresql.org

Backpatch-through: 9.4

5 years agoAvoid logging complaints about abandoned connections when using PAM.
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

5 years agoChange pg_restore -f- to dump to stdout instead of to ./-
Alvaro Herrera [Tue, 5 Nov 2019 13:08:55 +0000 (10:08 -0300)] 
Change pg_restore -f- to dump to stdout instead of to ./-

Starting with PostgreSQL 12, pg_restore refuses to run when neither -d
nor -f are specified (c.f. commit 413ccaa74d9a), and it also makes "-f -"
mean the old implicit behavior of dumping to stdout.  However, older
branches write to a file called ./- when invoked like that, making it
impossible to write pg_restore scripts that work across versions.  This
is a partial backpatch of the aforementioned commit to all older
supported branches, providing an upgrade path.

Discussion: https://postgr.es/m/20191006190839.GE18030@telsasoft.com

5 years agoDoc: Improve description around ALTER TABLE ATTACH PARTITION
Michael Paquier [Tue, 5 Nov 2019 01:18:05 +0000 (10:18 +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.GB23808@telsasoft.com
Backpatch-through: 10

5 years agoStabilize pg_dump output order for similarly-named triggers and policies.
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

5 years agoCatch invalid typlens in a couple of places
Peter Eisentraut [Mon, 4 Nov 2019 08:54:47 +0000 (09:54 +0100)] 
Catch invalid typlens in a couple of places

Rearrange the logic in record_image_cmp() and record_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 <pg@bowt.ie>
5 years agoSuppress warning from older compilers.
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.

5 years agoValidate ispell dictionaries more carefully.
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

5 years agoFix race condition at backend exit when deleting element in syncrep queue
Michael Paquier [Fri, 1 Nov 2019 13:38:55 +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.lingce.ldm@alibaba-inc.com
Backpatch-through: 9.4

5 years agopg_waldump: Fix --bkp-details to not issue spurious newlines for FPWs.
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.4gnyau7e5v2lh5sc@alap3.anarazel.de
Backpatch: 9.5, like 2c03216d831.

5 years agopg_waldump: Fix small memory leak when rmgr->rm_identify returns NULL.
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.4gnyau7e5v2lh5sc@alap3.anarazel.de
Backpatch: 9.5, where rm_identify was introduced

5 years agoAvoid failure when selecting a namespace node in XMLTABLE.
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)

5 years agodocs: fix wording of REFRESH CONCURRENTLY manual page
Bruce Momjian [Thu, 24 Oct 2019 00:29:02 +0000 (20:29 -0400)] 
docs:  fix wording of REFRESH CONCURRENTLY manual page

Reported-by: Asim / apraveen@pivotal.io
Discussion: https://postgr.es/m/157076828181.26165.15231292023740913543@wrigleys.postgresql.org

Backpatch-through: 9.4

5 years agoClean up properly error_context_stack in autovacuum worker on exception
Michael Paquier [Wed, 23 Oct 2019 01:26:06 +0000 (10:26 +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

5 years agoDeal with yet another issue related to "Norwegian (Bokmål)" locale.
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

5 years agoUse CFLAGS_SL while probing linkability of libperl.
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.horikyota.ntt@gmail.com

5 years agoSelect CFLAGS_SL at configure time, not in platform-specific Makefiles.
Tom Lane [Mon, 21 Oct 2019 16:32:36 +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.horikyota.ntt@gmail.com

5 years agoFor PowerPC instruction "addi", use constraint "b".
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.GA4066266@rfd.leadboat.com

5 years agoFix failure of archive recovery with recovery_min_apply_delay enabled.
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

5 years agoFix timeout handling in logical replication worker
Michael Paquier [Fri, 18 Oct 2019 05:27:04 +0000 (14:27 +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

5 years agoFix minor bug in logical-replication walsender shutdown
Alvaro Herrera [Thu, 17 Oct 2019 13:06:05 +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

5 years agoWhen restoring GUCs in parallel workers, show an error context.
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

5 years agoFix bug that could try to freeze running multixacts.
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

5 years agoImprove the check for pg_catalog.unknown data type in pg_upgrade
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

5 years agoImprove the check for pg_catalog.line data type in pg_upgrade
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

5 years agoDoc: Fix various inconsistencies
Michael Paquier [Wed, 16 Oct 2019 04:10:49 +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

5 years agoAIX: Stop adding option -qsrcmsg.
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.GA3955242@rfd.leadboat.com

5 years agoFlush logical mapping files with fd opened for read/write at checkpoint
Michael Paquier [Wed, 9 Oct 2019 04:31:22 +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.GA14532@paquier.xyz
Backpatch-through: 9.4

5 years agodocs: Improve A?synchronous Multimaster Replication descr.
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: juha-pekka.eloranta@reaktor.fi
Discussion: https://postgr.es/m/156856543824.1274.12180817186798859836@wrigleys.postgresql.org

Backpatch-through: 9.4

5 years agodocs: clarify that today/tomorrow/yesterday is at 00:00
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

5 years agodoc: move mention of log_min_error_statement in a better spot
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: ivaylo.zlatanov@gmail.com
Discussion: https://postgr.es/m/157019615723.25307.15449102262106437404@wrigleys.postgresql.org

Backpatch-through: 9.4

5 years agoCheck for too many postmaster children before spawning a bgworker.
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

5 years agoReport test_atomic_ops() failures consistently, via macros.
Noah Misch [Sat, 5 Oct 2019 17:05:05 +0000 (10:05 -0700)] 
Report test_atomic_ops() failures consistently, via macros.

This prints the unexpected value in more failure cases, and it removes
forty-eight hand-maintained error messages.  Back-patch to 9.5, which
introduced these tests.

Reviewed (in an earlier version) by Andres Freund.

Discussion: https://postgr.es/m/20190915160021.GA24376@alvherre.pgsql

5 years agoAvoid use of wildcard in pg_waldump's .gitignore.
Tom Lane [Sat, 5 Oct 2019 16:26:55 +0000 (12:26 -0400)] 
Avoid use of wildcard in pg_waldump's .gitignore.

This would be all right, maybe, if it didn't also match a file that
definitely should not be ignored.  We don't add rmgrs so often that
manual maintenance of this file list is impractical, so just write
out the list.

(I find the equivalent wildcard use in the Makefile pretty lazy and
unsafe as well, but will leave that alone until it actually causes a
problem.)

Per bug #16042 from Denis Stuchalin.

Discussion: https://postgr.es/m/16042-c174ee692ac21cbd@postgresql.org

5 years agoDisable one more set of tests from c8841199509.
Andres Freund [Sat, 5 Oct 2019 15:05:11 +0000 (08:05 -0700)] 
Disable one more set of tests from c8841199509.

Discussion: https://postgr.es/m/20191004222437.45qmglpto43pd3jb@alap3.anarazel.de
Backpatch: 9.6-, just like c8841199509 and 6e61d75f525

5 years agoDisable one set of tests from c8841199509.
Andres Freund [Sat, 5 Oct 2019 04:38:08 +0000 (21:38 -0700)] 
Disable one set of tests from c8841199509.

One of the upsert related tests is unstable (sometimes even hanging
until isolationtester's step timeout is reached). Based on preliminary
analysis that might be a problem outside of just that test, but not
really related to EPQ and triggers.  Disable for now, to get the
buildfarm greener again.

Discussion: https://postgr.es/m/20191004222437.45qmglpto43pd3jb@alap3.anarazel.de
Backpatch: 9.6-, just like c8841199509.

5 years agoAdd isolation tests for the combination of EPQ and triggers.
Andres Freund [Fri, 4 Oct 2019 21:01:35 +0000 (14:01 -0700)] 
Add isolation tests for the combination of EPQ and triggers.

As evidenced by bug #16036 this area is woefully under-tested. Add
fairly extensive tests for the combination.

Backpatch back to 9.6 - before that isolationtester was not capable
enough. While we don't backpatch tests all the time, future fixes to
trigger.c would potentially look different enough in 12+ from the
earlier branches that introducing bugs during backpatching is more
likely than normal. Also, it's just a crucial and undertested area of
the code.

Author: Andres Freund
Discussion: https://postgr.es/m/16036-28184c90d952fb7f@postgresql.org
Backpatch: 9.6-, the earliest these tests work

5 years agoHandle spaces in OpenSSL install location for MSVC
Andrew Dunstan [Fri, 4 Oct 2019 19:34:40 +0000 (15:34 -0400)] 
Handle spaces in OpenSSL install location for MSVC

First, make sure that the .exe name is quoted when trying to get the
version number. Also, don't quote the lib name for using in the project
files if it's already been quoted. This second change applies to all
libraries, not just OpenSSL.

This has clearly been broken forever, so backpatch to all live branches.

5 years agoFix bitshiftright()'s zero-padding some more.
Tom Lane [Fri, 4 Oct 2019 14:34:21 +0000 (10:34 -0400)] 
Fix bitshiftright()'s zero-padding some more.

Commit 5ac0d9360 failed to entirely fix bitshiftright's habit of
leaving one-bits in the pad space that should be all zeroes,
because in a moment of sheer brain fade I'd concluded that only
the code path used for not-a-multiple-of-8 shift distances needed
to be fixed.  Of course, a multiple-of-8 shift distance can also
cause the problem, so we need to forcibly zero the extra bits
in both cases.

Per bug #16037 from Alexander Lakhin.  As before, back-patch to all
supported branches.

Discussion: https://postgr.es/m/16037-1d1ebca564db54f4@postgresql.org

5 years agoAvoid unnecessary out-of-memory errors during encoding conversion.
Tom Lane [Thu, 3 Oct 2019 21:34:26 +0000 (17:34 -0400)] 
Avoid unnecessary out-of-memory errors during encoding conversion.

Encoding conversion uses the very simplistic rule that the output
can't be more than 4X longer than the input, and palloc's a buffer
of that size.  This results in failure to convert any string longer
than 1/4 GB, which is becoming an annoying limitation.

As a band-aid to improve matters, allow the allocated output buffer
size to exceed 1GB.  We still insist that the final result fit into
MaxAllocSize (1GB), though.  Perhaps it'd be safe to relax that
restriction, but it'd require close analysis of all callers, which
is daunting (not least because external modules might call these
functions).  For the moment, this should allow a 2X to 4X improvement
in the longest string we can convert, which is a useful gain in
return for quite a simple patch.

Also, once we have successfully converted a long string, repalloc
the output down to the actual string length, returning the excess
to the malloc pool.  This seems worth doing since we can usually
expect to give back several MB if we take this path at all.

This still leaves much to be desired, most notably that the assumption
that MAX_CONVERSION_GROWTH == 4 is very fragile, and yet we have no
guard code verifying that the output buffer isn't overrun.  Fixing
that would require significant changes in the encoding conversion
APIs, so it'll have to wait for some other day.

The present patch seems safely back-patchable, so patch all supported
branches.

Alvaro Herrera and Tom Lane

Discussion: https://postgr.es/m/20190816181418.GA898@alvherre.pgsql
Discussion: https://postgr.es/m/3614.1569359690@sss.pgh.pa.us

5 years agoAllow repalloc() to give back space when a large chunk is downsized.
Tom Lane [Thu, 3 Oct 2019 17:56:26 +0000 (13:56 -0400)] 
Allow repalloc() to give back space when a large chunk is downsized.

Up to now, if you resized a large (>8K) palloc chunk down to a smaller
size, aset.c made no attempt to return any space to the malloc pool.
That's unpleasant if a really large allocation is resized to a
significantly smaller size.  I think no such cases existed when this
code was designed, and I'm not sure whether they're common even yet,
but an upcoming fix to encoding conversion will certainly create such
cases.  Therefore, fix AllocSetRealloc so that it gives realloc()
a chance to do something with the block.  This doesn't noticeably
increase complexity, we mostly just have to change the order in which
the cases are considered.

Back-patch to all supported branches.

Discussion: https://postgr.es/m/20190816181418.GA898@alvherre.pgsql
Discussion: https://postgr.es/m/3614.1569359690@sss.pgh.pa.us

5 years agoSelectively include window frames in expression walks/mutates.
Andrew Gierth [Thu, 3 Oct 2019 09:54:52 +0000 (10:54 +0100)] 
Selectively include window frames in expression walks/mutates.

query_tree_walker and query_tree_mutator were skipping the
windowClause of the query, without regard for the fact that the
startOffset and endOffset in a WindowClause node are expression trees
that need to be processed. This was an oversight in commit ec4be2ee6
from 2010 which added the expression fields; the main symptom is that
function parameters in window frame clauses don't work in inlined
functions.

Fix (as conservatively as possible since this needs to not break
existing out-of-tree callers) and add tests.

Backpatch all the way, since this has been broken since 9.0.

Per report from Alastair McKinley; fix by me with kibitzing and review
from Tom Lane.

Discussion: https://postgr.es/m/DB6PR0202MB2904E7FDDA9D81504D1E8C68E3800@DB6PR0202MB2904.eurprd02.prod.outlook.com

5 years agoRemove temporary WAL and history files at the end of archive recovery
Michael Paquier [Wed, 2 Oct 2019 06:54:01 +0000 (15:54 +0900)] 
Remove temporary WAL and history files at the end of archive recovery

cbc55da has reworked the order of some actions at the end of archive
recovery.  Unfortunately this overlooked the fact that the startup
process needs to remove RECOVERYXLOG (for temporary WAL segment newly
recovered from archives) and RECOVERYHISTORY (for temporary history
file) at this step, leaving the files around even after recovery ended.

Backpatch to 9.5, like the previous commit.

Author: Sawada Masahiko
Reviewed-by: Fujii Masao, Michael Paquier
Discussion: https://postgr.es/m/CAD21AoBO_eDQub6zojFnWtnmutRBWvYf7=cW4Hsqj+U_R26w3Q@mail.gmail.com
Backpatch-through: 9.5

5 years agoDoc: clean up markup for jsonb_set and related functions.
Tom Lane [Fri, 27 Sep 2019 15:01:36 +0000 (11:01 -0400)] 
Doc: clean up markup for jsonb_set and related functions.

The markup for optional parameters was neither correct nor consistent.
In passing, fix a spelling mistake.

Per report from Alex Macy.  Some of these mistakes are old, so
back-patch as appropriate.

Discussion: https://postgr.es/m/156953522258.1204.12736099368284950578@wrigleys.postgresql.org

5 years agoFix oversight in commit 4429f6a9e3e12bb4af6e3677fbc78cd80f160252.
Amit Kapila [Wed, 18 Sep 2019 03:44:26 +0000 (09:14 +0530)] 
Fix oversight in commit 4429f6a9e3e12bb4af6e3677fbc78cd80f160252.

The test name and the following test cases suggest the index created
should be hash index, but it forgot to add 'using hash' in the test case.
This in itself won't improve code coverage as there were some other tests
which were covering the corresponding code.  However, it is better if the
added tests serve their actual purpose.

Reported-by: Paul A Jungwirth
Author: Paul A Jungwirth
Reviewed-by: Mahendra Singh
Backpatch-through: 9.4
Discussion: https://postgr.es/m/CA+renyV=Us-5XfMC25bNp-uWSj39XgHHmGE9Rh2cQKMegSj52g@mail.gmail.com

5 years agoFix failure with lock mode used for custom relation options
Michael Paquier [Wed, 25 Sep 2019 01:08:36 +0000 (10:08 +0900)] 
Fix failure with lock mode used for custom relation options

In-core relation options can use a custom lock mode since 47167b7, that
has lowered the lock available for some autovacuum parameters.  However
it forgot to consider custom relation options.  This causes failures
with ALTER TABLE SET when changing a custom relation option, as its lock
is not defined.  The existing APIs to define a custom reloption does not
allow to define a custom lock mode, so enforce its initialization to
AccessExclusiveMode which should be safe enough in all cases.  An
upcoming patch will extend the existing APIs to allow a custom lock mode
to be defined.

The problem can be reproduced with bloom indexes, so add a test there.

Reported-by: Nikolay Sharplov
Analyzed-by: Thomas Munro, Michael Paquier
Author: Michael Paquier
Reviewed-by: Kuntal Ghosh
Discussion: https://postgr.es/m/20190920013831.GD1844@paquier.xyz
Backpatch-through: 9.6

5 years agoDoc: clarify handling of duplicate elements in array containment tests.
Tom Lane [Mon, 23 Sep 2019 16:37:04 +0000 (12:37 -0400)] 
Doc: clarify handling of duplicate elements in array containment tests.

The array <@ and @> operators do not worry about duplicates: if every
member of array X matches some element of array Y, then X is contained
in Y, even if several members of X get matched to the same Y member.
This was not explicitly stated in the docs though, so improve matters.

Discussion: https://postgr.es/m/156614120484.1310.310161642239149585@wrigleys.postgresql.org

5 years agoFix failure to zero-pad the result of bitshiftright().
Tom Lane [Sun, 22 Sep 2019 21:46:00 +0000 (17:46 -0400)] 
Fix failure to zero-pad the result of bitshiftright().

If the bitstring length is not a multiple of 8, we'd shift the
rightmost bits into the pad space, which must be zeroes --- bit_cmp,
for one, depends on that.  This'd lead to the result failing to
compare equal to what it should compare equal to, as reported in
bug #16013 from Daryl Waycott.

This is, if memory serves, not the first such bug in the bitstring
functions.  In hopes of making it the last one, do a bit more work
than minimally necessary to fix the bug:

* Add assertion checks to bit_out() and varbit_out() to complain if
they are given incorrectly-padded input.  This will improve the
odds that manual testing of any new patch finds problems.

* Encapsulate the padding-related logic in macros to make it
easier to use.

Also, remove unnecessary padding logic from bit_or() and bitxor().
Somebody had already noted that we need not re-pad the result of
bit_and() since the inputs are required to be the same length,
but failed to extrapolate that to the other two.

Also, move a comment block that once was near the head of varbit.c
(but people kept putting other stuff in front of it), to put it in
the header block.

Note for the release notes: if anyone has inconsistent data as a
result of saving the output of bitshiftright() in a table, it's
possible to fix it with something like
UPDATE mytab SET bitcol = ~(~bitcol) WHERE bitcol != ~(~bitcol);

This has been broken since day one, so back-patch to all supported
branches.

Discussion: https://postgr.es/m/16013-c2765b6996aacae9@postgresql.org

5 years agoUpdate time zone data files to tzdata release 2019c.
Tom Lane [Fri, 20 Sep 2019 23:53:33 +0000 (19:53 -0400)] 
Update time zone data files to tzdata release 2019c.

DST law changes in Fiji and Norfolk Island.  Historical corrections
for Alberta, Austria, Belgium, British Columbia, Cambodia, Hong Kong,
Indiana (Perry County), Kaliningrad, Kentucky, Michigan, Norfolk
Island, South Korea, and Turkey.

5 years agoFix typo in commit 82fa3ff8672.
Amit Kapila [Fri, 20 Sep 2019 02:35:54 +0000 (08:05 +0530)] 
Fix typo in commit 82fa3ff8672.

Reported-By: Kuntal Ghosh (off-list)
Backpatch-through: 9.4, like 82fa3ff8672

5 years agoFix oversight in backpatch of 6cae9d2c10
Alexander Korotkov [Thu, 19 Sep 2019 20:36:01 +0000 (23:36 +0300)] 
Fix oversight in backpatch of 6cae9d2c10

During backpatch of 6cae9d2c10 Float8GetDatum() was accidentally removed.  This
commit turns it back.

Reported-by: Erik Rijkers
Discussion: https://postgr.es/m/6d51305e1159241cabee132f7efc7eff%40xs4all.nl
Author: Tom Lane
Backpatch-through: from 11 to 9.5

5 years agoImprove handling of NULLs in KNN-GiST and KNN-SP-GiST
Alexander Korotkov [Thu, 19 Sep 2019 18:30:19 +0000 (21:30 +0300)] 
Improve handling of NULLs in KNN-GiST and KNN-SP-GiST

This commit improves subject in two ways:

 * It removes ugliness of 02f90879e7, which stores distance values and null
   flags in two separate arrays after GISTSearchItem struct.  Instead we pack
   both distance value and null flag in IndexOrderByDistance struct.  Alignment
   overhead should be negligible, because we typically deal with at most few
   "col op const" expressions in ORDER BY clause.
 * It fixes handling of "col op NULL" expression in KNN-SP-GiST.  Now, these
   expression are not passed to support functions, which can't deal with them.
   Instead, NULL result is implicitly assumed.  It future we may decide to
   teach support functions to deal with NULL arguments, but current solution is
   bugfix suitable for backpatch.

Reported-by: Nikita Glukhov
Discussion: https://postgr.es/m/826f57ee-afc7-8977-c44c-6111d18b02ec%40postgrespro.ru
Author: Nikita Glukhov
Reviewed-by: Alexander Korotkov
Backpatch-through: 9.4

5 years agoDoc: Fix incorrect mention to connection_object in CONNECT command of ECPG
Michael Paquier [Thu, 19 Sep 2019 04:19:09 +0000 (13:19 +0900)] 
Doc: Fix incorrect mention to connection_object in CONNECT command of ECPG

This fixes an inconsistency with this parameter name not listed in the
command synopsis, and connection_name is the parameter name more
commonly used in the docs for ECPG commands.

Reported-by: Yusuke Egashita
Discussion: https://postgr.es/m/156870956796.1259.11456186889345212399@wrigleys.postgresql.org
Backpatch-through: 9.4

5 years agoDoc: document autovacuum interruption.
Amit Kapila [Thu, 19 Sep 2019 03:11:05 +0000 (08:41 +0530)] 
Doc: document autovacuum interruption.

It's important users be able to know (without looking at the source code)
that running DDL or DDL-like commands can interrupt autovacuum which can
lead to a lot of dead tuples and hence slower database operations.

Reported-by: James Coleman
Author: James Coleman
Reviewed-by: Amit Kapila
Backpatch-through: 9.4
Discussion: https://postgr.es/m/CAAaqYe-XYyNwML1=f=gnd0qWg46PnvD=BDrCZ5-L94B887XVxQ@mail.gmail.com

5 years agopg_upgrade/test.sh: Quote sed(1) argument
Alvaro Herrera [Wed, 18 Sep 2019 14:24:12 +0000 (11:24 -0300)] 
pg_upgrade/test.sh: Quote sed(1) argument

Lack of quotes results in failure to run the test under older Solaris.

Author: Marina Polyakova, Victor Wagner
Discussion: https://postgr.es/m/feba89f89e8925b3535cb7d72b9e05e1@postgrespro.ru

5 years agoDoc: Update FDW documentation about direct foreign table modification.
Etsuro Fujita [Wed, 18 Sep 2019 09:50:03 +0000 (18:50 +0900)] 
Doc: Update FDW documentation about direct foreign table modification.

1. Commit 7086be6e3 should have documented the limitation that the direct
   modification is disabled when WCO constraints are present, but didn't,
   which is definitely my fault.  Update the documentation (Postgres 9.6
   onwards).

2. Commit fc22b6623 should have documented the limitation that the direct
   modification is disabled when generated columns are defined, but
   didn't.  Update the documentation (Postgres 12 onwards).

Author: Etsuro Fujita
Discussion: https://postgr.es/m/CAPmGK14AYCPunLb6TRz1CQsW5Le01Z2ox8LSOKH0P-cOVDcQRA%40mail.gmail.com

5 years agoReplace xlc __fetch_and_add() with inline asm.
Noah Misch [Sat, 14 Sep 2019 02:34:06 +0000 (19:34 -0700)] 
Replace xlc __fetch_and_add() with inline asm.

PostgreSQL has been unusable when built with xlc 13 and newer, which are
incompatible with our use of __fetch_and_add().  Back-patch to 9.5,
which introduced pg_atomic_fetch_add_u32().

Reviewed by Tom Lane.

Discussion: https://postgr.es/m/20190831071157.GA3251746@rfd.leadboat.com

5 years agoTest pg_atomic_fetch_add_ with variable addend and 16-bit edge cases.
Noah Misch [Sat, 14 Sep 2019 02:33:30 +0000 (19:33 -0700)] 
Test pg_atomic_fetch_add_ with variable addend and 16-bit edge cases.

Back-patch to 9.5, which introduced these functions.

Reviewed by Tom Lane.

Discussion: https://postgr.es/m/20190831071157.GA3251746@rfd.leadboat.com

5 years agological decoding: process ASSIGNMENT during snapshot build
Alvaro Herrera [Fri, 13 Sep 2019 19:36:28 +0000 (16:36 -0300)] 
logical decoding: process ASSIGNMENT during snapshot build

Most WAL records are ignored in early SnapBuild snapshot build phases.
But it's critical to process some of them, so that later messages have
the correct transaction state after the snapshot is completely built; in
particular, XLOG_XACT_ASSIGNMENT messages are critical in order for
sub-transactions to be correctly assigned to their parent transactions,
or at least one assert misbehaves, as reported by Ildar Musin.

Diagnosed-by: Masahiko Sawada
Author: Masahiko Sawada
Discussion: https://postgr.es/m/CAONYFtOv+Er1p3WAuwUsy1zsCFrSYvpHLhapC_fMD-zNaRWxYg@mail.gmail.com

5 years agoFix under-parenthesized macro definitions
Alvaro Herrera [Fri, 13 Sep 2019 19:26:55 +0000 (16:26 -0300)] 
Fix under-parenthesized macro definitions

Lack of parens in the definitions could cause a statement using these
macros to have unexpected semantics.  In current code no bug is
apparent, but best to fix the definitions to avoid problems down the
line.

Reported-by: Tom Lane
Discussion: https://postgr.es/m/19795.1568400476@sss.pgh.pa.us

5 years agoFix nbtree page split rmgr desc routine.
Peter Geoghegan [Thu, 12 Sep 2019 22:45:03 +0000 (15:45 -0700)] 
Fix nbtree page split rmgr desc routine.

Include newitemoff in rmgr desc output for nbtree page split records.
In passing, correct an obsolete comment that claimed that newitemoff is
only logged for _L variant nbtree page split WAL records.

Both issues were oversights in commit 2c03216d831, which revamped the
WAL format.

Author: Peter Geoghegan
Backpatch: 9.5-, where the WAL format was revamped.

5 years agoFix usage of whole-row variables in WCO and RLS policy expressions.
Tom Lane [Thu, 12 Sep 2019 22:29:18 +0000 (18:29 -0400)] 
Fix usage of whole-row variables in WCO and RLS policy expressions.

Since WITH CHECK OPTION was introduced, ExecInitModifyTable has
initialized WCO expressions with the wrong plan node as parent -- that is,
it passed its input subplan not the ModifyTable node itself.  Up to now
we thought this was harmless, but bug #16006 from Vinay Banakar shows it's
not: if the input node is a SubqueryScan then ExecInitWholeRowVar can get
confused into doing the wrong thing.  (The fact that ExecInitWholeRowVar
contains such logic is certainly a horrid kluge that doesn't deserve to
live, but figuring out another way to do that is a task for some other day.)

Andres had already noticed the wrong-parent mistake and fixed it in commit
148e632c0, but not being aware of any user-visible consequences, he quite
reasonably didn't back-patch.  This patch is simply a back-patch of
148e632c0, plus addition of a test case based on bug #16006.  I also added
the test case to v12/HEAD, even though the bug is already fixed there.

Back-patch to all supported branches.  9.4 lacks RLS policies so the
new test case doesn't work there, but I'm pretty sure a test could be
devised based on using a whole-row Var in a plain WITH CHECK OPTION
condition.  (I lack the cycles to do so myself, though.)

Andres Freund and Tom Lane

Discussion: https://postgr.es/m/16006-99290d2e4642cbd5@postgresql.org
Discussion: https://postgr.es/m/20181205225213.hiwa3kgoxeybqcqv@alap3.anarazel.de