Bruce Momjian [Wed, 1 Jan 2020 17:21:44 +0000 (12:21 -0500)]
Update copyrights for 2020
Backpatch-through: update all files in master, backpatch legal files through 9.4
Alvaro Herrera [Fri, 27 Dec 2019 21:34:30 +0000 (18:34 -0300)]
Add pg_dump test for triggers on partitioned tables
This currently works, but add this test to ensure it continues to work.
Lack of this test became evident after a recent bugfix submission that
would have inadvertently broken it, in
https://postgr.es/m/CA+HiwqFM2=i+uHB9o4OkLbE2S3sjPHoVe2wXuAD1GLJ4+Pk9eg@mail.gmail.com
Bruce Momjian [Fri, 27 Dec 2019 19:49:08 +0000 (14:49 -0500)]
doc: add examples of creative use of unique expression indexes
Unique expression indexes can constrain data in creative ways, so show
two examples.
Reported-by: Tuomas Leikola
Discussion: https://postgr.es/m/
156760275564.1127.
12321702656456074572@wrigleys.postgresql.org
Backpatch-through: 9.4
Bruce Momjian [Fri, 27 Dec 2019 19:33:30 +0000 (14:33 -0500)]
docs: clarify infinite range values from data-type infinities
The previous docs referenced these distinct ideas confusingly.
Reported-by: Eugen Konkov
Discussion: https://postgr.es/m/
376945611.
20191026161529@yandex.ru
Backpatch-through: 9.4
Michael Paquier [Fri, 27 Dec 2019 08:59:16 +0000 (17:59 +0900)]
Forbid DROP SCHEMA on temporary namespaces
This operation was possible for the owner of the schema or a superuser.
Down to 9.4, doing this operation would cause inconsistencies in a
session whose temporary schema was dropped, particularly if trying to
create new temporary objects after the drop. A more annoying
consequence is a crash of autovacuum on an assertion failure when
logging information about an orphaned temp table dropped. Note that
because of
246a6c8 (present in v11~), which has made the removal of
orphaned temporary tables more aggressive, the failure could be
triggered more easily, but it is possible to reproduce down to 9.4.
Reported-by: Mahendra Singh, Prabhat Sahu
Author: Michael Paquier
Reviewed-by: Kyotaro Horiguchi, Mahendra Singh
Discussion: https://postgr.es/m/CAKYtNAr9Zq=1-ww4etHo-VCC-k120YxZy5OS01VkaLPaDbv2tg@mail.gmail.com
Backpatch-through: 9.4
Tom Lane [Thu, 26 Dec 2019 20:19:39 +0000 (15:19 -0500)]
Fix possible loss of sync between rectypeid and underlying PLpgSQL_type.
When revalidate_rectypeid() acts to update a stale record type OID
in plpgsql's data structures, it fixes the active PLpgSQL_rec struct
as well as the PLpgSQL_type struct it references. However, the latter
is shared across function executions while the former is not. In a
later function execution, the PLpgSQL_rec struct would be reinitialized
by copy_plpgsql_datums and would then contain a stale type OID,
typically leading to "could not open relation with OID NNNN" errors.
revalidate_rectypeid() can easily fix this, fortunately, just by
treating typ->typoid as authoritative.
Per report and diagnosis from Ashutosh Sharma, though this is not his
suggested fix. Back-patch to v11 where this code came in.
Discussion: https://postgr.es/m/CAE9k0Pkd4dZwt9J5pS9xhJFWpUtqs05C9xk_GEwPzYdV=GxwWg@mail.gmail.com
Michael Paquier [Thu, 26 Dec 2019 13:26:31 +0000 (22:26 +0900)]
Fix some comments related to logical repslot advancing
confirmed_flush is part of a replication slot's information, but not
confirmed_lsn.
Author: Kyotaro Horiguchi
Discussion: https://postgr.es/m/
20191226.175919.
17237335658671970.horikyota.ntt@gmail.com
Backpatch-through: 11
Thomas Munro [Mon, 23 Dec 2019 22:31:24 +0000 (11:31 +1300)]
Rotate instead of shifting hash join batch number.
Our algorithm for choosing batch numbers turned out not to work
effectively for multi-billion key inner relations. We would use
more hash bits than we have, and effectively concentrate all tuples
into a smaller number of batches than we intended. While ideally
we should switch to wider hashes, for now, change the algorithm to
one that effectively gives up bits from the bucket number when we
don't have enough bits. That means we'll finish up with longer
bucket chains than would be ideal, but that's better than having
batches that don't fit in work_mem and can't be divided.
Batch-patch to all supported releases.
Author: Thomas Munro
Reviewed-by: Tom Lane, thanks also to Tomas Vondra, Alvaro Herrera, Andres Freund for testing and discussion
Reported-by: James Coleman
Discussion: https://postgr.es/m/16104-
dc11ed911f1ab9df%40postgresql.org
Joe Conway [Mon, 23 Dec 2019 18:33:42 +0000 (13:33 -0500)]
Disallow null category in crosstab_hash
While building a hash map of categories in load_categories_hash,
resulting category names have not thus far been checked to ensure
they are not null. Prior to pg12 null category names worked to the
extent that they did not crash on some platforms. This is because
those system libraries have an snprintf which can deal with being
passed a null pointer argument for a string. But even in those cases
null categories did nothing useful. And on some platforms it crashed.
As of pg12, our own version of snprintf gets called, and it does
not deal with null pointer arguments at all, and crashes consistently.
Fix that by disallowing null categories. They never worked usefully,
and no one has ever asked for them to work previously. Back-patch to
all supported branches.
Reported-By: Ireneusz Pluta
Discussion: https://postgr.es/m/16176-
7489719b05e4303c@postgresql.org
Tom Lane [Mon, 23 Dec 2019 17:53:13 +0000 (12:53 -0500)]
Disallow partition key expressions that return pseudo-types.
This wasn't checked originally, but it should have been, because
in general pseudo-types can't be stored to and retrieved from disk.
Notably, partition bound values of type "record" would not be
interpretable by another session.
In v12 and HEAD, add another flag to CheckAttributeType's repertoire
so that it can produce a specific error message for this case. That's
infeasible in older branches without an ABI break, so fall back to
a slightly-less-nicely-worded error message in v10 and v11.
Problem noted by Amit Langote, though this patch is not his initial
solution. Back-patch to v10 where partitioning was introduced.
Discussion: https://postgr.es/m/CA+HiwqFUzjfj9HEsJtYWcr1SgQ_=iCAvQ=O2Sx6aQxoDu4OiHw@mail.gmail.com
Tom Lane [Mon, 23 Dec 2019 17:08:24 +0000 (12:08 -0500)]
Prevent a rowtype from being included in itself via a range.
We probably should have thought of this case when ranges were added,
but we didn't. (It's not the fault of commit
eb51af71f, because
ranges didn't exist then.)
It's an old bug, so back-patch to all supported branches.
Discussion: https://postgr.es/m/7782.
1577051475@sss.pgh.pa.us
Tom Lane [Sun, 22 Dec 2019 23:00:17 +0000 (18:00 -0500)]
Avoid low-probability regression test failures in timestamp[tz] tests.
If the first transaction block in these tests were entered exactly
at midnight (California time), they'd report a bogus failure due
to 'now' and 'midnight' having the same values. Commit
8c2ac75c5
had dismissed this as being of negligible probability, but we've
now seen it happen in the buildfarm, so let's prevent it. We can
get pretty much the same test coverage without an it's-not-midnight
assumption by moving the does-'now'-work cases into their own test step.
While here, apply commit
47169c255's s/DELETE/TRUNCATE/ change to
timestamptz as well as timestamp (not sure why that didn't
occur to me at the time; the risk of failure is the same).
Back-patch to all supported branches, since the main point is
to get rid of potential buildfarm failures.
Discussion: https://postgr.es/m/14821.
1577031117@sss.pgh.pa.us
Tom Lane [Sat, 21 Dec 2019 22:39:36 +0000 (17:39 -0500)]
In pgwin32_open, loop after ERROR_ACCESS_DENIED only if we can't stat.
This fixes a performance problem introduced by commit
6d7547c21.
ERROR_ACCESS_DENIED is returned in some other cases besides the
delete-pending case considered by that commit; notably, if the
given path names a directory instead of a plain file. In that
case we'll uselessly loop for 1 second before returning the
failure condition. That slows down some usage scenarios enough
to cause test timeout failures on our Windows buildfarm critters.
To fix, try to stat() the file, and sleep/loop only if that fails.
It will fail in the delete-pending case, and also in the case where
the deletion completed before we could stat(), so we have the cases
where we want to loop covered. In the directory case, the stat()
should succeed, letting us exit without a wait.
One case where we'll still wait uselessly is if the access-denied
problem pertains to a directory in the given pathname. But we don't
expect that to happen in any performance-critical code path.
There might be room to refine this further, but I'll push it now
in hopes of making the buildfarm green again.
Back-patch, like the preceding commit.
Alexander Lakhin and Tom Lane
Discussion: https://postgr.es/m/23073.
1576626626@sss.pgh.pa.us
Bruce Momjian [Sat, 21 Dec 2019 17:44:38 +0000 (12:44 -0500)]
docs: clarify handling of column lists in COPY TO/FROM
Previously it was unclear how COPY FROM handled cases where not all
columns were specified, or if the order didn't match.
Reported-by: pavlo.golub@gmail.com
Discussion: https://postgr.es/m/
157487729344.7213.
14245726713444755296@wrigleys.postgresql.org
Backpatch-through: 9.4
Tom Lane [Fri, 20 Dec 2019 20:34:08 +0000 (15:34 -0500)]
libpq should expose GSS-related parameters even when not implemented.
We realized years ago that it's better for libpq to accept all
connection parameters syntactically, even if some are ignored or
restricted due to lack of the feature in a particular build.
However, that lesson from the SSL support was for some reason never
applied to the GSSAPI support. This is causing various buildfarm
members to have problems with a test case added by commit
6136e94dc,
and it's just a bad idea from a user-experience standpoint anyway,
so fix it.
While at it, fix some places where parameter-related infrastructure
was added with the aid of a dartboard, or perhaps with the aid of
the anti-pattern "add new stuff at the end". It should be safe
to rearrange the contents of struct pg_conn even in released
branches, since that's private to libpq (and we'd have to move
some fields in some builds to fix this, anyway).
Back-patch to all supported branches.
Discussion: https://postgr.es/m/11297.
1576868677@sss.pgh.pa.us
Tom Lane [Thu, 19 Dec 2019 14:42:47 +0000 (09:42 -0500)]
Doc: add a short summary of available authentication methods.
The "auth-methods" <sect1> used to include descriptions of all our
authentication methods. Commit
56811e573 promoted its child <sect2>'s
to <sect1>'s, which has advantages but also created some issues:
* The auth-methods page itself is essentially empty/useless.
* Links that pointed to "auth-methods" as a placeholder for all
auth methods were rendered a bit nonsensical.
* DocBook no longer provides a subsection table-of-contents here,
which formerly was a useful if terse summary of available auth methods.
To improve matters, add a handwritten list of all the auth methods.
Per gripe from Dave Cramer. Back-patch to v11 where the previous
commit came in.
Discussion: https://postgr.es/m/CADK3HH+xQLhcPgg=kWqfogtXGGZr-JdSo=x=WQC0PkAVyxUWyQ@mail.gmail.com
Amit Kapila [Wed, 18 Dec 2019 02:46:31 +0000 (08:16 +0530)]
Fix subscriber invalid memory access on DDL.
This patch allows building the local relmap cache for a subscribed
relation after processing pending invalidation messages and potential
relcache updates. Without this, the attributes in the local cache don't
tally with the updated relcache entry leading to invalid memory access.
Reported-by Jehan-Guillaume de Rorthais
Author: Jehan-Guillaume de Rorthais and Vignesh C
Reviewed-by: Amit Kapila
Backpatch-through: 10
Discussion: https://postgr.es/m/
20191025175929.
7e90dbf5@firost
Michael Paquier [Wed, 18 Dec 2019 01:11:36 +0000 (10:11 +0900)]
Remove shadow variables linked to RedoRecPtr in xlog.c
This changes the routines in charge of recycling WAL segments past the
last redo LSN to not use anymore "RedoRecPtr" as a local variable, which
is also available in the context of the session as a static declaration,
replacing it with "lastredoptr". This confusion has been introduced by
d9fadbf, so backpatch down to v11 like the other commit.
Thanks to Tom Lane, Robert Haas, Alvaro Herrera, Mark Dilger and Kyotaro
Horiguchi for the input provided.
Author: Ranier Vilela
Discussion: https://postgr.es/m/MN2PR18MB2927F7B5F690065E1194B258E35D0@MN2PR18MB2927.namprd18.prod.outlook.com
Backpatch-through: 11
Tom Lane [Tue, 17 Dec 2019 22:44:28 +0000 (17:44 -0500)]
Fix error reporting for index expressions of prohibited types.
If CheckAttributeType() threw an error about the datatype of an
index expression column, it would report an empty column name,
which is pretty unhelpful and certainly not the intended behavior.
I (tgl) evidently broke this in commit
cfc5008a5, by not noticing
that the column's attname was used above where I'd placed the
assignment of it.
In HEAD and v12, this is trivially fixable by moving up the
assignment of attname. Before v12 the code is a bit more messy;
to avoid doing substantial refactoring, I took the lazy way out
and just put in two copies of the assignment code.
Report and patch by Amit Langote. Back-patch to all supported
branches.
Discussion: https://postgr.es/m/CA+HiwqFA+BGyBFimjiYXXMa2Hc3fcL0+OJOyzUNjhU4NCa_XXw@mail.gmail.com
Amit Kapila [Thu, 12 Dec 2019 06:21:30 +0000 (11:51 +0530)]
Change overly strict Assert in TransactionGroupUpdateXidStatus.
This Assert thought that an overflowed transaction can never get registered
for the group update. But that is not true, because even when the number
of children for a transaction got reduced, the overflow flag is not
changed. And, for group update, we only care about the current number of
children for a transaction that is being committed.
Based on comments by Andres Freund, remove a redundant Assert in
TransactionIdSetPageStatus as we already had a static Assert for the same
condition a few lines earlier.
Reported-by: Vignesh C
Author: Dilip Kumar
Reviewed-by: Amit Kapila
Backpatch-through: 11
Discussion: https://postgr.es/m/CAFiTN-s5=uJw-Z6JC9gcqtBSjXsrHnU63PXBrA=pnBjqnkm5UA@mail.gmail.com
Tom Lane [Mon, 16 Dec 2019 20:10:55 +0000 (15:10 -0500)]
On Windows, wait a little to see if ERROR_ACCESS_DENIED goes away.
Attempting to open a file fails with ERROR_ACCESS_DENIED if the file
is flagged for deletion but not yet actually gone (another in a long
list of reasons why Windows is broken, if you ask me). This seems
likely to explain a lot of irreproducible failures we see in the
buildfarm. This state generally persists for only a millisecond or so,
so just wait a bit and retry. If it's a real permissions problem,
we'll eventually give up and report it as such. If it's the pending
deletion case, we'll see file-not-found and report that after the
deletion completes, and the caller will treat that in an appropriate
way.
In passing, rejigger the existing retry logic for some other error
cases so that we don't uselessly wait an extra time when we're
not going to retry anymore.
Alexander Lakhin (with cosmetic tweaks by me). Back-patch to all
supported branches, since this seems like a pretty safe change and
the problem is definitely real.
Discussion: https://postgr.es/m/16161-
7a985d2f1bbe8f71@postgresql.org
Etsuro Fujita [Mon, 16 Dec 2019 08:00:17 +0000 (17:00 +0900)]
Clean up some misplaced comments in partition_join.sql regression test.
Also, add a comment explaining a test case.
Back-patch to 11 where the regression test was added.
Discussion: https://postgr.es/m/CAPmGK15adZPh2B%2BmGUjSOMH%2BH39ogDRWfCfm4G6jncZCAs9V_Q%40mail.gmail.com
Tom Lane [Thu, 12 Dec 2019 17:30:44 +0000 (12:30 -0500)]
Fix EXTRACT(ISOYEAR FROM timestamp) for years BC.
The test cases added by commit
26ae3aa80 exposed an old oversight in
timestamp[tz]_part: they didn't correct the result of date2isoyear()
for BC years, so that we produced an off-by-one answer for such years.
Fix that, and back-patch to all supported branches.
Discussion: https://postgr.es/m/SG2PR06MB37762CAE45DB0F6CA7001EA9B6550@SG2PR06MB3776.apcprd06.prod.outlook.com
Tom Lane [Thu, 12 Dec 2019 17:12:35 +0000 (12:12 -0500)]
Remove redundant function calls in timestamp[tz]_part().
The DTK_DOW/DTK_ISODOW and DTK_DOY switch cases in timestamp_part() and
timestamptz_part() contained calls of timestamp2tm() that were fully
redundant with the ones done just above the switch. This evidently crept
in during commit
258ee1b63, which relocated that code from another place
where the calls were indeed needed. Just delete the redundant calls.
I (tgl) noted that our test coverage of these functions left quite a
bit to be desired, so extend timestamp.sql and timestamptz.sql to
cover all the branches.
Back-patch to all supported branches, as the previous commit was.
There's no real issue here other than some wasted cycles in some
not-too-heavily-used code paths, but the test coverage seems valuable.
Report and patch by Li Japin; test case adjustments by me.
Discussion: https://postgr.es/m/SG2PR06MB37762CAE45DB0F6CA7001EA9B6550@SG2PR06MB3776.apcprd06.prod.outlook.com
Etsuro Fujita [Thu, 12 Dec 2019 06:45:02 +0000 (15:45 +0900)]
Remove extra parenthesis from comment.
Tom Lane [Wed, 11 Dec 2019 20:53:36 +0000 (15:53 -0500)]
Doc: back-patch documentation about limitations of CHECK constraints.
Back-patch commits
36d442a25 and
1f66c657f into all supported
branches. I'd considered doing this when putting in the latter
commit, but failed to pull the trigger. Now that we've had an
actual field complaint about the lack of such docs, let's do it.
Per bug #16158 from Piotr Jander. Original patches by Lætitia Avrot,
Patrick Francelle, and me.
Discussion: https://postgr.es/m/16158-
7ccf2f74b3d655db@postgresql.org
Tom Lane [Mon, 9 Dec 2019 20:03:51 +0000 (15:03 -0500)]
Fix race condition in our Windows signal emulation.
pg_signal_dispatch_thread() responded to the client (signal sender)
and disconnected the pipe before actually setting the shared variables
that make the signal visible to the backend process's main thread.
In the worst case, it seems, effective delivery of the signal could be
postponed for as long as the machine has any other work to do.
To fix, just move the pg_queue_signal() call so that we do it before
responding to the client. This essentially makes pgkill() synchronous,
which is a stronger guarantee than we have on Unix. That may be
overkill, but on the other hand we have not seen comparable timing bugs
on any Unix platform.
While at it, add some comments to this sadly underdocumented code.
Problem diagnosis and fix by Amit Kapila; I just added the comments.
Back-patch to all supported versions, as it appears that this can cause
visible NOTIFY timing oddities on all of them, and there might be
other misbehavior due to slow delivery of other signals.
Discussion: https://postgr.es/m/32745.
1575303812@sss.pgh.pa.us
Tom Lane [Mon, 9 Dec 2019 19:31:57 +0000 (14:31 -0500)]
Improve isolationtester's timeout management.
isolationtester.c had a hard-wired limit of 3 minutes per test step.
It now emerges that this isn't quite enough for some of the slowest
buildfarm animals. This isn't the first time we've had to raise
this limit (cf.
1db439ad4), so let's make it configurable. This
patch raises the default to 5 minutes, and introduces an environment
variable PGISOLATIONTIMEOUT that can be set if more time is needed,
following the precedent of PGCTLTIMEOUT.
Also, modify isolationtester so that when the timeout is hit,
it explicitly reports having sent a cancel. This makes the regression
failure log considerably more intelligible. (In the worst case, a
timed-out test might actually be reported as "passing" without this
extra output, so arguably this is a bug fix in itself.)
In passing, update the README file, which had apparently not gotten
touched when we added "make check" support here.
Back-patch to 9.6; older versions don't have comparable timeout logic.
Discussion: https://postgr.es/m/22964.
1575842935@sss.pgh.pa.us
Amit Kapila [Mon, 9 Dec 2019 03:09:34 +0000 (08:39 +0530)]
Fix typos in miscinit.c.
Commit
f13ea95f9e moved the description of postmaster.pid file contents
from miscadmin.h to pidfile.h, but missed to update the comments in
miscinit.c.
Author: Hadi Moshayedi
Reviewed-by: Amit Kapila
Backpatch-through: 10
Discussion: https://postgr.es/m/CAK=1=WpYEM9x3LGkaxgXaxeYQjnkdW8XLsxrYRTE2Gq-H83FMw@mail.gmail.com
Noah Misch [Sun, 8 Dec 2019 19:06:26 +0000 (11:06 -0800)]
Document search_path security with untrusted dbowner or CREATEROLE.
Commit
5770172cb0c9df9e6ce27c507b449557e5b45124 wrote, incorrectly, that
certain schema usage patterns are secure against CREATEROLE users and
database owners. When an untrusted user is the database owner or holds
CREATEROLE privilege, a query is secure only if its session started with
SELECT pg_catalog.set_config('search_path', '', false) or equivalent.
Back-patch to 9.4 (all supported versions).
Discussion: https://postgr.es/m/
20191013013512.GC4131753@rfd.leadboat.com
Tom Lane [Sun, 8 Dec 2019 15:36:29 +0000 (10:36 -0500)]
Doc: improve documentation about run-time pruning's effects on EXPLAIN.
Tatsuo Ishii complained that this para wasn't very intelligible.
Try to make it better.
Discussion: https://postgr.es/m/
20191207.200500.
989741087350666720.t-ishii@sraoss.co.jp
Michael Paquier [Fri, 6 Dec 2019 06:14:31 +0000 (15:14 +0900)]
Fix handling of OpenSSL's SSL_clear_options
This function is supported down to OpenSSL 0.9.8, which is the oldest
version supported since
593d4e4 (from Postgres 10 onwards), and is used
since
e3bdb2d (from 11 onwards). It is defined as a macro from OpenSSL
0.9.8 to 1.0.2, and as a function in 1.1.0 and newer versions. However,
the configure check present is only adapted for functions. So, even if
the code would be able to compile, configure fails to detect the macro,
causing it to be ignored when compiling the code with OpenSSL from 0.9.8
to 1.0.2.
The code needs a configure check as per
a364dfa, which has fixed a
compilation issue with a past version of LibreSSL in NetBSD 5.1. On
HEAD, just remove the configure check as the last release of NetBSD 5 is
from 2014 (and we have no more buildfarm members for it). In 11 and 12,
improve the configure logic so as both macros and functions are
correctly detected. This makes NetBSD 5 still work on already-released
branches, but not for 13 onwards.
The patch for HEAD is from me, and Daniel has written the version to use
for the back-branches.
Author: Michael Paquier, Daniel Gustaffson
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/
20191205083252.GE5064@paquier.xyz
Discussion: https://postgr.es/m/
98F7F99E-1129-41D8-B86B-
FE3B1E286881@yesql.se
Backpatch-through: 11
Tomas Vondra [Tue, 3 Dec 2019 15:55:51 +0000 (16:55 +0100)]
Ensure maxlen is at leat 1 in dict_int
The dict_int text search dictionary template accepts maxlen parameter,
which is then used to cap the length of input strings. The value was
not properly checked, and the code simply does
txt[d->maxlen] = '\0';
to insert a terminator, leading to segfaults with negative values.
This commit simply rejects values less than 1. The issue was there since
dct_int was introduced in 9.3, so backpatch all the way back to 9.4
which is the oldest supported version.
Reported-by: cili
Discussion: https://postgr.es/m/16144-
a36a5bef7657047d@postgresql.org
Backpatch-through: 9.4
Michael Paquier [Tue, 3 Dec 2019 04:01:43 +0000 (13:01 +0900)]
Fix failures with TAP tests of pg_ctl on Windows
On Windows, all the hosts spawned by the TAP tests bind to 127.0.0.1.
Hence, if there is a port conflict, starting a cluster would immediately
fail. One of the test scripts of pg_ctl initializes a node without
PostgresNode.pm, using the default port 5432. This could cause
unexpected startup failures in the tests if an independent server was up
and running on the same host (the reverse is also possible, though more
unlikely). Fix this issue by assigning properly a free port to the node
configured, in the same range used as for the other nodes part of the
tests.
Author: Michael Paquier
Reviewed-by: Andrew Dunstan
Discussion: https://postgr.es/m/
20191202031444.GC1696@paquier.xyz
Backpatch-through: 11
Tom Lane [Sun, 1 Dec 2019 18:09:26 +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.
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
Etsuro Fujita [Wed, 27 Nov 2019 07:00:48 +0000 (16:00 +0900)]
Fix typo in comment.
Tom Lane [Tue, 26 Nov 2019 19:41:48 +0000 (14:41 -0500)]
Allow access to child table statistics if user can read parent table.
The fix for CVE-2017-7484 disallowed use of pg_statistic data for
planning purposes if the user would not be able to select the associated
column and a non-leakproof function is to be applied to the statistics
values. That turns out to disable use of pg_statistic data in some
common cases involving inheritance/partitioning, where the user does
have permission to select from the parent table that was actually named
in the query, but not from a child table whose stats are needed. Since,
in non-corner cases, the user *can* select the child table's data via
the parent, this restriction is not actually useful from a security
standpoint. Improve the logic so that we also check the permissions of
the originally-named table, and allow access if select permission exists
for that.
When checking access to stats for a simple child column, we can map
the child column number back to the parent, and perform this test
exactly (including not allowing access if the child column isn't
exposed by the parent). For expression indexes, the current logic
just insists on whole-table select access, and this patch allows
access if the user can select the whole parent table. In principle,
if the child table has extra columns, this might allow access to
stats on columns the user can't read. In practice, it's unlikely
that the planner is going to do any stats calculations involving
expressions that are not visible to the query, so we'll ignore that
fine point for now. Perhaps someday we'll improve that logic to
detect exactly which columns are used by an expression index ...
but today is not that day.
Back-patch to v11. The issue was created in 9.2 and up by the
CVE-2017-7484 fix, but this patch depends on the append_rel_array[]
planner data structure which only exists in v11 and up. In
practice the issue is most urgent with partitioned tables, so
fixing v11 and later should satisfy much of the practical need.
Dilip Kumar and Amit Langote, with some kibitzing by me
Discussion: https://postgr.es/m/3876.
1531261875@sss.pgh.pa.us
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
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
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
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
Tom Lane [Sat, 23 Nov 2019 22:30:00 +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
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
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
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.
Michael Paquier [Thu, 21 Nov 2019 01:23:43 +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
Magnus Hagander [Wed, 20 Nov 2019 16:03:07 +0000 (17:03 +0100)]
Remove incorrect markup
Author: Daniel Gustafsson <daniel@yesql.se>
Alexander Korotkov [Tue, 19 Nov 2019 21:12:33 +0000 (00:12 +0300)]
Fix page modification outside of critical section in GIN
By oversight
52ac6cd2d0 makes ginDeletePage() sets pd_prune_xid of page to be
deleted before entering the critical section. It appears that only versions 11
and later were affected by this oversight.
Backpatch-through: 11
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
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
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
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
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
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
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 [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
Tom Lane [Mon, 11 Nov 2019 22:05:05 +0000 (17:05 -0500)]
Stamp 11.6.
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:50:22 +0000 (10:50 +0100)]
Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash:
36cb12a154ee719a594401e7f8763e472f41614a
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 [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
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:32 +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
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
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.GA11324@alvherre.pgsql
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
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
Michael Paquier [Wed, 6 Nov 2019 07:12:34 +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/87a7ator8n.fsf@news-spur.riddles.org.uk
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.GX4999@telsasoft.com
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: matthew.alton@gmail.com
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
Alvaro Herrera [Mon, 4 Nov 2019 18:50:57 +0000 (15:50 -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
Michael Paquier [Tue, 5 Nov 2019 01:18:01 +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
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 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>
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:17:12 +0000 (14:17 +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:51 +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
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.
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
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)
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
Michael Paquier [Wed, 23 Oct 2019 01:25:50 +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.horikyota.ntt@gmail.com
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
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
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