]> granicus.if.org Git - postgresql/log
postgresql
5 years agoUse an unsigned char for bool if we don't use the native bool.
Andrew Gierth [Wed, 20 Feb 2019 21:31:02 +0000 (21:31 +0000)]
Use an unsigned char for bool if we don't use the native bool.

On (rare) platforms where sizeof(bool) > 1, we need to use our own
bool, but imported c99 code (such as Ryu) may want to use bool values
as array subscripts, which elicits warnings if bool is defined as
char. Using unsigned char instead should work just as well for our
purposes, and avoid such warnings.

Per buildfarm members prariedog and locust.

5 years agoImprove planner's understanding of strictness of type coercions.
Tom Lane [Wed, 20 Feb 2019 19:39:11 +0000 (14:39 -0500)]
Improve planner's understanding of strictness of type coercions.

PG type coercions are generally strict, ie a NULL input must produce
a NULL output (or, in domain cases, possibly an error).  The planner's
understanding of that was a bit incomplete though, so improve it:

* Teach contain_nonstrict_functions() that CoerceViaIO can always be
considered strict.  Previously it believed that only if the underlying
I/O functions were marked strict, which is often but not always true.

* Teach clause_is_strict_for() that CoerceViaIO, ArrayCoerceExpr,
ConvertRowtypeExpr, CoerceToDomain can all be considered strict.
Previously it knew nothing about any of them.

The main user-visible impact of this is that IS NOT NULL predicates
can be proven to hold from expressions involving casts in more cases
than before, allowing partial indexes with such predicates to be used
without extra pushups.  This reduces the surprise factor for users,
who may well be used to ordinary (function-call-based) casts being
known to be strict.

Per a gripe from Samuel Williams.  This doesn't rise to the level of
a bug, IMO, so no back-patch.

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

5 years agoFix incorrect strictness test for ArrayCoerceExpr expressions.
Tom Lane [Wed, 20 Feb 2019 18:36:55 +0000 (13:36 -0500)]
Fix incorrect strictness test for ArrayCoerceExpr expressions.

The recursion in contain_nonstrict_functions_walker() was done wrong,
causing the strictness check to be bypassed for a parse node that
is the immediate input of an ArrayCoerceExpr node.  This could allow,
for example, incorrect decisions about whether a strict SQL function
can be inlined.

I didn't add a regression test, because (a) the bug is so narrow
and (b) I couldn't think of a test case that wasn't dependent on a
large number of other behaviors, to the point where it would likely
soon rot to the point of not testing what it was intended to.

I broke this in commit c12d570fa, so back-patch to v11.

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

5 years agoMake object address handling more robust
Alvaro Herrera [Wed, 20 Feb 2019 12:12:02 +0000 (09:12 -0300)]
Make object address handling more robust

pg_identify_object_as_address crashes when passed certain tuples from
inconsistent system catalogs.  Make it more defensive.

Author: Álvaro Herrera
Reviewed-by: Michaël Paquier
Discussion: https://postgr.es/m/20190218202743.GA12392@alvherre.pgsql

5 years agoDoc: Update the documentation for FSM behavior for small tables.
Amit Kapila [Wed, 20 Feb 2019 12:07:39 +0000 (17:37 +0530)]
Doc: Update the documentation for FSM behavior for small tables.

In commit b0eaa4c51b, we have avoided the creation of FSM for small tables.
So the functions that use FSM to compute the free space can return zero for
such tables.  This was previously also possible for the cases where the
vacuum has not been triggered to update FSM.

This commit updates the comments in code and documentation to reflect this
behavior.

Author: John Naylor
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CACPNZCtba-3m1q3A8gxA_vxg=T7gQf7gMbpR4Ciy5LntY-j+0Q@mail.gmail.com

5 years agoFix DEFAULT-handling in multi-row VALUES lists for updatable views.
Dean Rasheed [Wed, 20 Feb 2019 08:30:21 +0000 (08:30 +0000)]
Fix DEFAULT-handling in multi-row VALUES lists for updatable views.

INSERT ... VALUES for a single VALUES row is implemented differently
from a multi-row VALUES list, which causes inconsistent behaviour in
the way that DEFAULT items are handled. In particular, when inserting
into an auto-updatable view on top of a table with a column default, a
DEFAULT item in a single VALUES row gets correctly replaced with the
table column's default, but for a multi-row VALUES list it is replaced
with NULL.

Fix this by allowing rewriteValuesRTE() to leave DEFAULT items in the
VALUES list untouched if the target relation is an auto-updatable view
and has no column default, deferring DEFAULT-expansion until the query
against the base relation is rewritten. For all other types of target
relation, including tables and trigger- and rule-updatable views, we
must continue to replace DEFAULT items with NULL in the absence of a
column default.

This is somewhat complicated by the fact that if an auto-updatable
view has DO ALSO rules attached, the VALUES lists for the product
queries need to be handled differently from the original query, since
the product queries need to act like rule-updatable views whereas the
original query has auto-updatable view semantics.

Back-patch to all supported versions.

Reported by Roger Curley (bug #15623). Patch by Amit Langote and me.

Discussion: https://postgr.es/m/15623-5d67a46788ec8b7f@postgresql.org

5 years agoMark correctly initial slot snapshots with MVCC type when built
Michael Paquier [Wed, 20 Feb 2019 03:31:07 +0000 (12:31 +0900)]
Mark correctly initial slot snapshots with MVCC type when built

When building an initial slot snapshot, snapshots are marked with
historic MVCC snapshots as type with the marker field being set in
SnapBuildBuildSnapshot() but not overriden in SnapBuildInitialSnapshot().
Existing callers of SnapBuildBuildSnapshot() do not care about the type
of snapshot used, but extensions calling it actually may, as reported.

While on it, mark correctly the snapshot type when importing one.  This
is cosmetic as the field is enforced to 0.

Author: Antonin Houska
Reviewed-by: Álvaro Herrera, Michael Paquier
Discussion: https://postgr.es/m/23215.1527665193@localhost
Backpatch-through: 9.4

5 years agoUse varargs macro for CACHEDEBUG
Peter Eisentraut [Mon, 18 Feb 2019 11:32:34 +0000 (12:32 +0100)]
Use varargs macro for CACHEDEBUG

Reviewed-by: Andres Freund <andres@anarazel.de>
5 years agoFix omissions in ecpg/test/sql/.gitignore.
Tom Lane [Tue, 19 Feb 2019 02:24:38 +0000 (21:24 -0500)]
Fix omissions in ecpg/test/sql/.gitignore.

Oversights in commits 050710b36 and e81f0e311.

5 years agoRemove line duplicated during conflict resolution.
Andres Freund [Mon, 18 Feb 2019 19:07:30 +0000 (11:07 -0800)]
Remove line duplicated during conflict resolution.

I included the duplicated ExecTypeFromTL in 578b2297 "Remove WITH OIDS
support".

Reported-By: Peter Eisentraut
Discussion: https://postgr.es/m/ba819888-63c6-7f98-6acb-3731142d9414@2ndquadrant.com

5 years agoDe-clutter display of script runtimes in pg_regress.
Tom Lane [Mon, 18 Feb 2019 16:16:39 +0000 (11:16 -0500)]
De-clutter display of script runtimes in pg_regress.

Add more whitespace, per suggestion from Peter Eisentraut.

Discussion: https://postgr.es/m/e265e2ae-e92e-5ab9-dc68-60b6cb047b3d@2ndquadrant.com

5 years agoProvide an extra-float-digits setting for pg_dump / pg_dumpall
Andrew Dunstan [Mon, 18 Feb 2019 12:22:00 +0000 (07:22 -0500)]
Provide an extra-float-digits setting for pg_dump / pg_dumpall

Changes made by commit 02ddd49 mean that dumps made against pre version
12 instances are no longer comparable with those made against version 12
or later instances. This makes cross-version upgrade testing fail in the
buildfarm. Experimentation has shown that the error is cured if the
dumps are made when extra_float_digits is set to 0. Hence this patch
allows for it to be explicitly set rather than relying on pg_dump's
builtin default (3 in almost all cases). This feature might have other
uses, but should not normally be used.

Discussion: https://postgr.es/m/c76f7051-8fd3-ec10-7579-1f8842305b85@2ndQuadrant.com

5 years agoProperly end string to make sure ecpglib does not read beyond its boundaries.
Michael Meskes [Mon, 18 Feb 2019 11:52:53 +0000 (12:52 +0100)]
Properly end string to make sure ecpglib does not read beyond its boundaries.

5 years agoSync ECPG's CREATE TABLE AS statement with backend's.
Michael Meskes [Mon, 18 Feb 2019 10:57:34 +0000 (11:57 +0100)]
Sync ECPG's CREATE TABLE AS statement with backend's.

Author: Higuchi-san ("Higuchi, Daisuke" <higuchi.daisuke@jp.fujitsu.com>)

5 years agoAdd bytea datatype to ECPG.
Michael Meskes [Mon, 18 Feb 2019 09:20:31 +0000 (10:20 +0100)]
Add bytea datatype to ECPG.

So far ECPG programs had to treat binary data for bytea column as 'char' type.
But this meant converting from/to escaped format with PQunescapeBytea/
PQescapeBytea() and therefore forcing users to add unnecessary code and cost
for the conversion in runtime. By adding a dedicated datatype for bytea most of
this special handling is no longer needed.

Author: Matsumura-san ("Matsumura, Ryo" <matsumura.ryo@jp.fujitsu.com>)

Discussion: https://postgr.es/m/flat/03040DFF97E6E54E88D3BFEE5F5480F737A141F9@G01JPEXMBYT04

5 years agoSave PathTargets for distinct/ordered relations in root->upper_targets[].
Etsuro Fujita [Mon, 18 Feb 2019 07:13:46 +0000 (16:13 +0900)]
Save PathTargets for distinct/ordered relations in root->upper_targets[].

For the convenience of extensions, we previously only saved PathTargets
for grouped, window, and final relations in root->upper_targets[] in
grouping_planner().  To improve the convenience, save PathTargets for
distinct and ordered relations as well.

Author: Antonin Houska, with an additional change by me
Discussion: https://postgr.es/m/10994.1549559088@localhost

5 years agoFix some issues with TAP tests of pg_basebackup and pg_verify_checksums
Michael Paquier [Mon, 18 Feb 2019 05:23:30 +0000 (14:23 +0900)]
Fix some issues with TAP tests of pg_basebackup and pg_verify_checksums

ee9e145 has fixed the tests of pg_basebackup for checksums a first time,
still one seek() call missed the shot.  Also, the data written in files
to emulate corruptions was not actually writing zeros as the quoting
style was incorrect.

Backpatch the portion for pg_basebackup to v11 where these tests have
been introduced.  The tests of pg_verify_checksums are new as of v12.

Author: Michael Banck
Discussion: https://postgr.es/m/1550153276.796.35.camel@credativ.de
Backpatch-through: 11

5 years agoFix typo in transam.h for OIDs assigned by genbki.pl
Michael Paquier [Mon, 18 Feb 2019 03:44:25 +0000 (12:44 +0900)]
Fix typo in transam.h for OIDs assigned by genbki.pl

The actual range of reserved OIDs in this case is [11000,11999] and not
[11000,12000].

Author: John Naylor
Discussion: https://postgr.es/m/CAJVSVGV5StmK-inxbmrf0nLbBGeaAKnjnqxXmk+4ufeav8JMSA@mail.gmail.com

5 years agoAvoid some unnecessary block reads in WAL reader
Michael Paquier [Mon, 18 Feb 2019 00:52:02 +0000 (09:52 +0900)]
Avoid some unnecessary block reads in WAL reader

When reading a new page internally and depending on the way the WAL
reader facility gets used by plugins, the current implementation of the
WAL reader may finish by reading a block multiple times while it is not
actually necessary as the requested data length may be equal to what has
been already read.  This can happen for any size, but is more likely to
happen at the end of a page.  This can cause performance penalties in
plugins which rely on the block reads to be purely sequential, zlib not
liking backward reads for example.  The new behavior also shaves some
cycles when doing recovery.

Author: Arthur Zakirov
Reviewed-by: Andrey Lepikhov, Michael Paquier, Grigory Smolkin
Discussion: https://postgr.es/m/2ddf4a32-517e-d6f4-d992-4a63b6035bfd@postgrespro.ru

5 years agoFix race in dsm_unpin_segment() when handles are reused.
Thomas Munro [Sun, 17 Feb 2019 20:53:26 +0000 (09:53 +1300)]
Fix race in dsm_unpin_segment() when handles are reused.

Teach dsm_unpin_segment() to skip segments that are in the process
of being destroyed by another backend, when searching for a handle.
Such a segment cannot possibly be the one we are looking for, even
if its handle matches.  Another slot might hold a recently created
segment that has the same handle value by coincidence, and we need
to keep searching for that one.

The bug caused rare "cannot unpin a segment that is not pinned"
errors on 10 and 11.  Similar to commit 6c0fb941 for dsm_attach().

Back-patch to 10, where dsm_unpin_segment() landed.

Author: Thomas Munro
Reported-by: Justin Pryzby
Tested-by: Justin Pryzby (along with other recent DSA/DSM fixes)
Discussion: https://postgr.es/m/20190216023854.GF30291@telsasoft.com

5 years agoFix documentation for dblink_error_message() return value
Joe Conway [Sun, 17 Feb 2019 18:15:14 +0000 (13:15 -0500)]
Fix documentation for dblink_error_message() return value

The dblink documentation claims that an empty string is returned if there
has been no error, however OK is actually returned in that case. Also,
clarify that an async error may not be seen unless dblink_is_busy() or
dblink_get_result() have been called first.

Backpatch to all supported branches.

Reported-by: realyota
Backpatch-through: 9.4
Discussion: https://postgr.es/m/153371978486.1298.2091761143788088262@wrigleys.postgresql.org

5 years agoFix CREATE VIEW to allow zero-column views.
Tom Lane [Sun, 17 Feb 2019 17:37:31 +0000 (12:37 -0500)]
Fix CREATE VIEW to allow zero-column views.

We should logically have allowed this case when we allowed zero-column
tables, but it was overlooked.

Although this might be thought a feature addition, it's really a bug
fix, because it was possible to create a zero-column view via
the convert-table-to-view code path, and then you'd have a situation
where dump/reload would fail.  Hence, back-patch to all supported
branches.

Arrange the added test cases to provide coverage of the related
pg_dump code paths (since these views will be dumped and reloaded
during the pg_upgrade regression test).  I also made them test
the case where pg_dump has to postpone the view rule into post-data,
which disturbingly had no regression coverage before.

Report and patch by Ashutosh Sharma (test case by me)

Discussion: https://postgr.es/m/CAE9k0PkmHdeSaeZt2ujnb_cKucmK3sDDceDzw7+d5UZoNJPYOg@mail.gmail.com

5 years agoMark pg_config() stable rather than immutable
Joe Conway [Sun, 17 Feb 2019 14:21:13 +0000 (09:21 -0500)]
Mark pg_config() stable rather than immutable

pg_config() has been marked immutable since its inception. As part of a
larger discussion around the definition of immutable versus stable and
related implications for marking functions parallel safe raised by
Andres, the consensus was clearly that pg_config() is stable, since
it could possibly change output even for the same minor version with
a recompile or installation of a new binary. So mark it stable.

Theoretically this could/should be backpatched, but it was deemed to be not
worth the effort since in practice this is very unlikely to cause problems
in the real world.

Discussion: https://postgr.es/m/20181126234521.rh3grz7aavx2ubjv@alap3.anarazel.de

5 years agoDoc: remove ancient comment.
Tatsuo Ishii [Sun, 17 Feb 2019 11:23:10 +0000 (20:23 +0900)]
Doc: remove ancient comment.

There's a very old comment in rules.sgml added back to 2003.  It
expected to a feature coming back but it never happened. So now we can
safely remove the comment. Back-patched to all supported branches.

Discussion: https://postgr.es/m/20190211.191004.219630835457494660.t-ishii%40sraoss.co.jp

5 years agoFix CLogTruncationLock documentation.
Noah Misch [Sun, 17 Feb 2019 08:51:11 +0000 (00:51 -0800)]
Fix CLogTruncationLock documentation.

Back-patch to v10, which introduced the lock.

5 years agoSuppress another case of MSVC warning 4146.
Noah Misch [Sat, 16 Feb 2019 23:28:27 +0000 (15:28 -0800)]
Suppress another case of MSVC warning 4146.

5 years agoIn imath.h, replace stdint.h usage with c.h equivalents.
Noah Misch [Sat, 16 Feb 2019 23:28:27 +0000 (15:28 -0800)]
In imath.h, replace stdint.h usage with c.h equivalents.

As things stood, buildfarm member dory failed.  MSVC versions lacking
stdint.h are unusable for building PostgreSQL, but pg_config.h.win32
doesn't know that.  Even so, we support other systems lacking stdint.h,
including buildfarm member gaur.  Per a suggestion from Tom Lane.

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

5 years agoRemove float8-small-is-zero regression test variant.
Andrew Gierth [Sat, 16 Feb 2019 22:11:04 +0000 (22:11 +0000)]
Remove float8-small-is-zero regression test variant.

Since this was also the variant used as an example in the docs, update
the docs to use float4-misrounded-input as an example instead, since
that is now the only remaining variant file.

5 years agoImport changes from IMath versions (1.3, 1.29].
Noah Misch [Sat, 16 Feb 2019 21:12:28 +0000 (13:12 -0800)]
Import changes from IMath versions (1.3, 1.29].

Upstream fixed bugs over the years, but none are confirmed to have
affected pgcrypto.  We're better off naively tracking upstream than
reactively maintaining a twelve-year-old snapshot of upstream.  Add a
header comment describing the synchronization procedure.  Discard use of
INVERT_COMPARE_RESULT(); the domain of the comparisons in question is
{-1,0,1}, controlled entirely by code in imath.c.

Andrew Gierth reviewed the Makefile change.  Tom Lane reviewed the
synchronization procedure description.

Discussion: https://postgr.es/m/20190203035704.GA6226@rfd.leadboat.com

5 years agoFix PERMIT_DECLARATION_AFTER_STATEMENT initialization.
Noah Misch [Sat, 16 Feb 2019 21:12:28 +0000 (13:12 -0800)]
Fix PERMIT_DECLARATION_AFTER_STATEMENT initialization.

The defect caused a mere warning and only for gcc versions before 3.4.0.

5 years agoAllow user control of CTE materialization, and change the default behavior.
Tom Lane [Sat, 16 Feb 2019 21:11:12 +0000 (16:11 -0500)]
Allow user control of CTE materialization, and change the default behavior.

Historically we've always materialized the full output of a CTE query,
treating WITH as an optimization fence (so that, for example, restrictions
from the outer query cannot be pushed into it).  This is appropriate when
the CTE query is INSERT/UPDATE/DELETE, or is recursive; but when the CTE
query is non-recursive and side-effect-free, there's no hazard of changing
the query results by pushing restrictions down.

Another argument for materialization is that it can avoid duplicate
computation of an expensive WITH query --- but that only applies if
the WITH query is called more than once in the outer query.  Even then
it could still be a net loss, if each call has restrictions that
would allow just a small part of the WITH query to be computed.

Hence, let's change the behavior for WITH queries that are non-recursive
and side-effect-free.  By default, we will inline them into the outer
query (removing the optimization fence) if they are called just once.
If they are called more than once, we will keep the old behavior by
default, but the user can override this and force inlining by specifying
NOT MATERIALIZED.  Lastly, the user can force the old behavior by
specifying MATERIALIZED; this would mainly be useful when the query had
deliberately been employing WITH as an optimization fence to prevent a
poor choice of plan.

Andreas Karlsson, Andrew Gierth, David Fetter

Discussion: https://postgr.es/m/87sh48ffhb.fsf@news-spur.riddles.org.uk

5 years agoFix previous MinGW fix.
Andrew Gierth [Sat, 16 Feb 2019 15:21:10 +0000 (15:21 +0000)]
Fix previous MinGW fix.

Definitions required for MinGW need to be outside #if _MSC_VER. Oops.

5 years agoAdd DECLARE STATEMENT support to ECPG.
Michael Meskes [Sat, 16 Feb 2019 09:55:17 +0000 (10:55 +0100)]
Add DECLARE STATEMENT support to ECPG.

DECLARE STATEMENT is a statement that lets users declare an identifier
pointing at a connection.  This identifier will be used in other embedded
dynamic SQL statement such as PREPARE, EXECUTE, DECLARE CURSOR and so on.
When connecting to a non-default connection, the AT clause can be used in
a DECLARE STATEMENT once and is no longer needed in every dynamic SQL
statement.  This makes ECPG applications easier and more efficient.  Moreover,
writing code without designating connection explicitly improves portability.

Authors: Ideriha-san ("Ideriha, Takeshi" <ideriha.takeshi@jp.fujitsu.com>)
         Kuroda-san ("Kuroda, Hayato" <kuroda.hayato@jp.fujitsu.com>)

Discussion: https://postgr.es/m4E72940DA2BF16479384A86D54D0988A565669DF@G01JPEXMBKW04

5 years agoMake use of compiler builtins and/or assembly for CLZ, CTZ, POPCNT.
Tom Lane [Sat, 16 Feb 2019 04:22:27 +0000 (23:22 -0500)]
Make use of compiler builtins and/or assembly for CLZ, CTZ, POPCNT.

Test for the compiler builtins __builtin_clz, __builtin_ctz, and
__builtin_popcount, and make use of these in preference to
handwritten C code if they're available.  Create src/port
infrastructure for "leftmost one", "rightmost one", and "popcount"
so as to centralize these decisions.

On x86_64, __builtin_popcount generally won't make use of the POPCNT
opcode because that's not universally supported yet.  Provide code
that checks CPUID and then calls POPCNT via asm() if available.
This requires indirecting through a function pointer, which is
an annoying amount of overhead for a one-instruction operation,
but it's probably not worth working harder than this for our
current use-cases.

I'm not sure we've found all the existing places that could profit
from this new infrastructure; but we at least touched all the
ones that used copied-and-pasted versions of the bitmapset.c code,
and got rid of multiple copies of the associated constant arrays.

While at it, replace c-compiler.m4's one-per-builtin-function
macros with a single one that can handle all the cases we need
to worry about so far.  Also, because I'm paranoid, make those
checks into AC_LINK checks rather than just AC_COMPILE; the
former coding failed to verify that libgcc has support for the
builtin, in cases where it's not inline code.

David Rowley, Thomas Munro, Alvaro Herrera, Tom Lane

Discussion: https://postgr.es/m/CAKJS1f9WTAGG1tPeJnD18hiQW5gAk59fQ6WK-vfdAKEHyRg2RA@mail.gmail.com

5 years agoCygwin and Mingw floating-point fixes.
Andrew Gierth [Sat, 16 Feb 2019 01:50:16 +0000 (01:50 +0000)]
Cygwin and Mingw floating-point fixes.

Deal with silent-underflow errors in float4 for cygwin and mingw by
using our strtof() wrapper; deal with misrounding errors by adding
them to the resultmap. Some slight reorganization of declarations was
done to avoid duplicating material between cygwin.h and win32_port.h.

While here, remove from the resultmap all references to
float8-small-is-zero; inspection of cygwin output suggests it's no
longer required there, and the freebsd/netbsd/openbsd entries should
no longer be necessary (these date back to c. 2000). This commit
doesn't remove the file itself nor the documentation references for
it; that will happen in a subsequent commit if all goes well.

5 years agoRevert attempts to use POPCNT etc instructions
Alvaro Herrera [Fri, 15 Feb 2019 19:32:30 +0000 (16:32 -0300)]
Revert attempts to use POPCNT etc instructions

This reverts commits fc6c72747ae6109de05cbb03d0b4663c23b7 and
711bab1e4d19.

Somebody will have to try harder before submitting this patch again.
I've spent entirely too much time on it already, and the #ifdef maze yet
to be written in order for it to build at all got on my nerves.  The
amount of work needed to get a platform-specific performance improvement
that's barely above the noise level is not worth it.

5 years agoRefactor index cost estimation functions in view of IndexClause changes.
Tom Lane [Fri, 15 Feb 2019 18:05:19 +0000 (13:05 -0500)]
Refactor index cost estimation functions in view of IndexClause changes.

Get rid of deconstruct_indexquals() in favor of just iterating over the
IndexClause list directly.  The extra services that that function used to
provide, such as hiding clause commutation and associating the right index
column with each clause, are no longer useful given the new data structure.
I'd originally thought that it'd provide a useful amount of abstraction
by freeing callers from paying attention to the exact clause type of each
indexqual, but that hope proves to have been vain, because few callers can
ignore the semantic differences between different clause types.  Indeed,
removing it results in a net code savings, and probably some cycles shaved
by not having to build an extra list-of-structs data structure.

Also, export a few formerly-static support functions, with the goal
of allowing extension AMs to write functionality equivalent to
genericcostestimate() without pointless code duplication.

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

5 years agoFix compiler builtin usage in new pg_bitutils.c
Alvaro Herrera [Fri, 15 Feb 2019 16:07:02 +0000 (13:07 -0300)]
Fix compiler builtin usage in new pg_bitutils.c

Split out these new functions in three parts: one in a new file that
uses the compiler builtin and gets compiled with the -mpopcnt compiler
option if it exists; another one that uses the compiler builtin but not
the compiler option; and finally the fallback with open-coded
algorithms.

Split out the configure logic: in the original commit, it was selecting
to use the -mpopcnt compiler switch together with deciding whether to
use the compiler builtin, but those two things are really separate.
Split them out.  Also, expose whether the builtin exists to
Makefile.global, so that src/port's Makefile can decide whether to
compile the hw-optimized file.

Remove CPUID test for CTZ/CLZ.  Make pg_{right,left}most_ones use either
the compiler intrinsic or open-coded algo; trying to use the
HW-optimized version is a waste of time.  Make them static inline
functions.

Discussion: https://postgr.es/m/20190213221719.GA15976@alvherre.pgsql

5 years agodoc: Update README.links
Peter Eisentraut [Fri, 15 Feb 2019 16:29:41 +0000 (17:29 +0100)]
doc: Update README.links

The guideline to "not use text with <ulink> so the URL appears in
printed output" is obsolete, since the URL appears as a footnote in
printed output in that case.

Reported-by: Chapman Flack <chap@anastigmatix.net>
Discussion: https://www.postgresql.org/message-id/5C4B1F2F.2000106@anastigmatix.net

5 years agoUse standard diff separator for regression.diffs
Peter Eisentraut [Wed, 2 Jan 2019 20:24:51 +0000 (21:24 +0100)]
Use standard diff separator for regression.diffs

Instead of ======..., use the standard separator for a multi-file
diff, which is, per POSIX,

    "diff %s %s %s\n", <diff_options>, <filename1>, <filename2>

This makes regression.diffs behave more like a proper diff file, for
use with other tools.  And it shows the diff options used, for
clarity.

Discussion: https://www.postgresql.org/message-id/70440c81-37bb-76dd-e48b-b5a9550d5613@2ndquadrant.com

5 years agoFix support for CREATE TABLE IF NOT EXISTS AS EXECUTE
Michael Paquier [Fri, 15 Feb 2019 08:12:24 +0000 (17:12 +0900)]
Fix support for CREATE TABLE IF NOT EXISTS AS EXECUTE

The grammar IF NOT EXISTS for CTAS is supported since 9.5 and documented
as such, however the case of using EXECUTE as query has never been
covered as EXECUTE CTAS statements and normal CTAS statements are parsed
separately.

Author: Andreas Karlsson
Discussion: https://postgr.es/m/2ddcc188-e37c-a0be-32bf-a56b07c3559e@proxel.se
Backpatch-through: 9.5

5 years agoFix race in dsm_attach() when handles are reused.
Thomas Munro [Thu, 14 Feb 2019 21:19:11 +0000 (10:19 +1300)]
Fix race in dsm_attach() when handles are reused.

DSM handle values can be reused as soon as the underlying shared memory
object has been destroyed.  That means that for a brief moment we
might have two DSM slots with the same handle.  While trying to attach,
if we encounter a slot with refcnt == 1, meaning that it is currently
being destroyed, we should continue our search in case the same handle
exists in another slot.

The race manifested as a rare "dsa_area could not attach to segment"
error, and was more likely in 10 and 11 due to the lack of distinct
seed for random() in parallel workers.  It was made very unlikely in
in master by commit 197e4af9, and older releases don't usually create
new DSM segments in background workers so it was also unlikely there.

This fixes the root cause of bug report #15585, in which the error
could also sometimes result in a self-deadlock in the error path.
It's not yet clear if further changes are needed to avoid that failure
mode.

Back-patch to 9.4, where dsm.c arrived.

Author: Thomas Munro
Reported-by: Justin Pryzby, Sergei Kornilov
Discussion: https://postgr.es/m/20190207014719.GJ29720@telsasoft.com
Discussion: https://postgr.es/m/15585-324ff6a93a18da46@postgresql.org

5 years agoSimplify the planner's new representation of indexable clauses a little.
Tom Lane [Fri, 15 Feb 2019 00:37:30 +0000 (19:37 -0500)]
Simplify the planner's new representation of indexable clauses a little.

In commit 1a8d5afb0, I thought it'd be a good idea to define
IndexClause.indexquals as NIL in the most common case where the given
clause (IndexClause.rinfo) is usable exactly as-is.  It'd be more
consistent to define the indexquals in that case as being a one-element
list containing IndexClause.rinfo, but I thought saving the palloc
overhead for making such a list would be worthwhile.

In hindsight, that was a great example of "premature optimization is the
root of all evil": it's complicated everyplace that needs to deal with
the indexquals, requiring duplicative code to handle both the simple
case and the not-simple case.  I'd initially found that tolerable but
it's getting less so as I mop up some areas that I'd not touched in
1a8d5afb0.  In any case, two more pallocs during a planner run are
surely at the noise level (a conclusion confirmed by a bit of
microbenchmarking).  So let's change this decision before it becomes
set in stone, and insist that IndexClause.indexquals always be a valid
list of the actual index quals for the clause.

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

5 years agoGet rid of another unconstify through API changes
Peter Eisentraut [Thu, 14 Feb 2019 19:44:47 +0000 (20:44 +0100)]
Get rid of another unconstify through API changes

This also makes the code in read_client_first_message() more similar
to read_client_final_message().

Reported-by: Mark Dilger <hornschnorter@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/53a28052-f9f3-1808-fed9-460fd43035ab%402ndquadrant.com

5 years agoResolve one unconstify use
Peter Eisentraut [Thu, 14 Feb 2019 16:00:25 +0000 (17:00 +0100)]
Resolve one unconstify use

A small API change makes it unnecessary.

Reported-by: Mark Dilger <hornschnorter@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/53a28052-f9f3-1808-fed9-460fd43035ab%402ndquadrant.com

5 years agoMove pattern selectivity code from selfuncs.c to like_support.c.
Tom Lane [Thu, 14 Feb 2019 15:51:59 +0000 (10:51 -0500)]
Move pattern selectivity code from selfuncs.c to like_support.c.

While at it, refactor patternsel() a bit so that it can be used from
the LIKE/regex planner support functions as well.  This makes the
planner able to deal equally well with either operator or function
syntax for these operations.  I'm not excited about that as a feature
in itself, but it provides a nice model for extensions to follow if
they want such behavior for their operations.

This change localizes the use of pattern_fixed_prefix() and
make_greater_string() so that they no longer need be exported.
(We might get pushback from extensions about that, perhaps,
in which case I'd be inclined to re-export them in a new header
file like_support.h.)

This reduces the bulk of selfuncs.c a fair amount, removing ~1370
lines or about one-sixth of that file; it's still too big, but this
is progress.

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

5 years agoFix portability issues in pg_bitutils
Alvaro Herrera [Wed, 13 Feb 2019 23:09:48 +0000 (20:09 -0300)]
Fix portability issues in pg_bitutils

We were using uint64 function arguments as "long int" arguments to
compiler builtins, which fails on machines where long ints are 32 bits:
the upper half of the uint64 was being ignored.  Fix by using the "ll"
builtin variants instead, which on those machines take 64 bit arguments.

Also, remove configure tests for __builtin_popcountl() (as well as
"long" variants for ctz and clz): the theory here is that any compiler
version will provide all widths or none, so one test suffices.  Were
this theory to be wrong, we'd have to add tests for
__builtin_popcountll() and friends, which would be tedious.

Per failures in buildfarm member lapwing and ensuing discussion.

5 years agoRemove a stray subnormal value from float tests.
Andrew Gierth [Wed, 13 Feb 2019 20:38:54 +0000 (20:38 +0000)]
Remove a stray subnormal value from float tests.

We don't care to assume that input of subnormal float values works,
but a stray subnormal value from the upstream Ryu regression test had
been left in the test data by mistake. Remove it.

Per buildfarm member fulmar.

5 years agoAdd -mpopcnt to all compiles of pg_bitutils.c
Alvaro Herrera [Wed, 13 Feb 2019 20:38:21 +0000 (17:38 -0300)]
Add -mpopcnt to all compiles of pg_bitutils.c

The way this makefile works, we need to specify it three times.

5 years agoMore float test and portability fixes.
Andrew Gierth [Wed, 13 Feb 2019 19:35:50 +0000 (19:35 +0000)]
More float test and portability fixes.

Avoid assuming exact results in tstypes test; some platforms vary.
(per buildfarm members eulachon, danio, lapwing)

Avoid dubious usage (inherited from upstream) of bool parameters to
copy_special_str, to see if this fixes the mac/ppc failures (per
buildfarm members prariedog and locust). (Isolated test programs on a
ppc mac don't seem to show any other cause that would explain them.)

5 years agoAdd basic support for using the POPCNT and SSE4.2s LZCNT opcodes
Alvaro Herrera [Wed, 13 Feb 2019 19:10:06 +0000 (16:10 -0300)]
Add basic support for using the POPCNT and SSE4.2s LZCNT opcodes

These opcodes have been around in the AMD world since 2007, and 2008 in
the case of intel.  They're supported in GCC and Clang via some __builtin
macros.  The opcodes may be unavailable during runtime, in which case we
fall back on a C-based implementation of the code.  In order to get the
POPCNT instruction we must pass the -mpopcnt option to the compiler.  We
do this only for the pg_bitutils.c file.

David Rowley (with fragments taken from a patch by Thomas Munro)

Discussion: https://postgr.es/m/CAKJS1f9WTAGG1tPeJnD18hiQW5gAk59fQ6WK-vfdAKEHyRg2RA@mail.gmail.com

5 years agoFix an overlooked UINT32_MAX.
Andrew Gierth [Wed, 13 Feb 2019 15:57:54 +0000 (15:57 +0000)]
Fix an overlooked UINT32_MAX.

Replace with PG_UINT32_MAX. Per buildfarm members dory and woodlouse.

5 years agoChange floating-point output format for improved performance.
Andrew Gierth [Wed, 13 Feb 2019 15:20:33 +0000 (15:20 +0000)]
Change floating-point output format for improved performance.

Previously, floating-point output was done by rounding to a specific
decimal precision; by default, to 6 or 15 decimal digits (losing
information) or as requested using extra_float_digits. Drivers that
wanted exact float values, and applications like pg_dump that must
preserve values exactly, set extra_float_digits=3 (or sometimes 2 for
historical reasons, though this isn't enough for float4).

Unfortunately, decimal rounded output is slow enough to become a
noticable bottleneck when dealing with large result sets or COPY of
large tables when many floating-point values are involved.

Floating-point output can be done much faster when the output is not
rounded to a specific decimal length, but rather is chosen as the
shortest decimal representation that is closer to the original float
value than to any other value representable in the same precision. The
recently published Ryu algorithm by Ulf Adams is both relatively
simple and remarkably fast.

Accordingly, change float4out/float8out to output shortest decimal
representations if extra_float_digits is greater than 0, and make that
the new default. Applications that need rounded output can set
extra_float_digits back to 0 or below, and take the resulting
performance hit.

We make one concession to portability for systems with buggy
floating-point input: we do not output decimal values that fall
exactly halfway between adjacent representable binary values (which
would rely on the reader doing round-to-nearest-even correctly). This
is known to be a problem at least for VS2013 on Windows.

Our version of the Ryu code originates from
https://github.com/ulfjack/ryu/ at commit c9c3fb1979, but with the
following (significant) modifications:

 - Output format is changed to use fixed-point notation for small
   exponents, as printf would, and also to use lowercase 'e', a
   minimum of 2 exponent digits, and a mandatory sign on the exponent,
   to keep the formatting as close as possible to previous output.

 - The output of exact midpoint values is disabled as noted above.

 - The integer fast-path code is changed somewhat (since we have
   fixed-point output and the upstream did not).

 - Our project style has been largely applied to the code with the
   exception of C99 declaration-after-statement, which has been
   retained as an exception to our present policy.

 - Most of upstream's debugging and conditionals are removed, and we
   use our own configure tests to determine things like uint128
   availability.

Changing the float output format obviously affects a number of
regression tests. This patch uses an explicit setting of
extra_float_digits=0 for test output that is not expected to be
exactly reproducible (e.g. due to numerical instability or differing
algorithms for transcendental functions).

Conversions from floats to numeric are unchanged by this patch. These
may appear in index expressions and it is not yet clear whether any
change should be made, so that can be left for another day.

This patch assumes that the only supported floating point format is
now IEEE format, and the documentation is updated to reflect that.

Code by me, adapting the work of Ulf Adams and other contributors.

References:
https://dl.acm.org/citation.cfm?id=3192369

Reviewed-by: Tom Lane, Andres Freund, Donald Dong
Discussion: https://postgr.es/m/87r2el1bx6.fsf@news-spur.riddles.org.uk

5 years agoUse strtof() and not strtod() for float4 input.
Andrew Gierth [Wed, 13 Feb 2019 15:19:44 +0000 (15:19 +0000)]
Use strtof() and not strtod() for float4 input.

Using strtod() creates a double-rounding problem; the input decimal
value is first rounded to the nearest double; rounding that to the
nearest float may then give an incorrect result.

An example is that 7.038531e-26 when input via strtod and then rounded
to float4 gives 0xAE43FEp-107 instead of the correct 0xAE43FDp-107.

Values output by earlier PG versions with extra_float_digits=3 should
all be read in with the same values as previously. However, values
supplied by other software using shortest representations could be
mis-read.

On platforms that lack a strtof() entirely, we fall back to the old
incorrect rounding behavior. (As strtof() is required by C99, such
platforms are considered of primarily historical interest.) On VS2013,
some workarounds are used to get correct error handling.

The regression tests now test for the correct input values, so
platforms that lack strtof() will need resultmap entries. An entry for
HP-UX 10 is included (more may be needed).

Reviewed-By: Tom Lane
Discussion: https://postgr.es/m/871s5emitx.fsf@news-spur.riddles.org.uk
Discussion: https://postgr.es/m/87d0owlqpv.fsf@news-spur.riddles.org.uk

5 years agoMore unconstify use
Peter Eisentraut [Tue, 29 Jan 2019 00:16:24 +0000 (01:16 +0100)]
More unconstify use

Replace casts whose only purpose is to cast away const with the
unconstify() macro.

Discussion: https://www.postgresql.org/message-id/flat/53a28052-f9f3-1808-fed9-460fd43035ab%402ndquadrant.com

5 years agoRemove useless casts
Peter Eisentraut [Tue, 29 Jan 2019 00:12:18 +0000 (01:12 +0100)]
Remove useless casts

Some of these were uselessly casting away "const", some were just
nearby, but they where all unnecessary anyway.

Discussion: https://www.postgresql.org/message-id/flat/53a28052-f9f3-1808-fed9-460fd43035ab%402ndquadrant.com

5 years agoFix comment related to calculation location of total_table_pages
Michael Paquier [Wed, 13 Feb 2019 07:31:20 +0000 (16:31 +0900)]
Fix comment related to calculation location of total_table_pages

As of commit c6e4133, the calculation happens in make_one_rel() and not
query_planner().

Author: Amit Langote
Discussion: https://postgr.es/m/c7a04a90-42e6-28a4-811a-a7e352831ba1@lab.ntt.co.jp

5 years agoFix rare dsa_allocate() failures due to freepage.c corruption.
Thomas Munro [Wed, 13 Feb 2019 00:14:10 +0000 (13:14 +1300)]
Fix rare dsa_allocate() failures due to freepage.c corruption.

In a corner case, a btree page was allocated during a clean-up operation
that could cause the tracking of the largest contiguous span of free
space to get out of whack.  That was supposed to be prevented by the use
of the "soft" flag to avoid allocating internal pages during incidental
clean-up work, but the flag was ignored in the case where the FPM was
promoted from singleton format to btree format.  Repair.

Remove an obsolete comment in passing.

Back-patch to 10, where freepage.c arrived (as support for dsa.c).

Author: Robert Haas
Diagnosed-by: Thomas Munro and Robert Haas
Reported-by: Justin Pryzby, Rick Otten, Sand Stone, Arne Roland and others
Discussion: https://postgr.es/m/CAMAYy4%2Bw3NTBM5JLWFi8twhWK4%3Dk_5L4nV5%2BbYDSPu8r4b97Zg%40mail.gmail.com

5 years agoClean up planner confusion between ncolumns and nkeycolumns.
Tom Lane [Tue, 12 Feb 2019 23:38:32 +0000 (18:38 -0500)]
Clean up planner confusion between ncolumns and nkeycolumns.

We're only going to consider key columns when creating indexquals,
so there is no point in having the outer loops in indxpath.c iterate
further than nkeycolumns.

Doing so in match_pathkeys_to_index() is actually wrong, and would have
caused crashes by now, except that we have no index AMs supporting both
amcanorderbyop and amcaninclude.

It's also wrong in relation_has_unique_index_for().  The effect there is
to fail to prove uniqueness even when the index does prove it, if there
are extra columns.

Also future-proof examine_variable() for the day when extra columns can
be expressions, and fix what's either a thinko or just an oversight in
btcostestimate(): we should consider the number of key columns, not the
total, when deciding whether to derate correlation.

None of these things seemed important enough to risk changing in a
just-before-wrap patch, but since we're past the release wrap window,
time to fix 'em.

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

5 years agoRelax overly strict assertion
Alvaro Herrera [Tue, 12 Feb 2019 21:42:37 +0000 (18:42 -0300)]
Relax overly strict assertion

Ever since its birth, ReorderBufferBuildTupleCidHash() has contained an
assertion that a catalog tuple cannot change Cmax after acquiring one.  But
that's wrong: if a subtransaction executes DDL that affects that catalog
tuple, and later aborts and another DDL affects the same tuple, it will
change Cmax.  Relax the assertion to merely verify that the Cmax remains
valid and monotonically increasing, instead.

Add a test that tickles the relevant code.

Diagnosed by, and initial patch submitted by: Arseny Sher
Co-authored-by: Arseny Sher
Discussion: https://postgr.es/m/874l9p8hyw.fsf@ars-thinkpad

5 years agoBlind attempt at fixing Windows build
Alvaro Herrera [Tue, 12 Feb 2019 21:29:26 +0000 (18:29 -0300)]
Blind attempt at fixing Windows build

Broken since fe33a196de.

5 years agoUse Getopt::Long for catalog scripts
Alvaro Herrera [Tue, 12 Feb 2019 14:53:36 +0000 (11:53 -0300)]
Use Getopt::Long for catalog scripts

Replace hand-rolled option parsing with the Getopt module. This is
shorter and easier to read. In passing, make some cosmetic adjustments
for consistency.

Author: John Naylor
Reviewed-by: David Fetter
Discussion: https://postgr.es/m/CACPNZCvRjepXh5b2N50njN+rO_2Nzcf=jhMkKX7=79XWUKJyKA@mail.gmail.com

5 years agoFix erroneous error reports in snapbuild.c.
Tom Lane [Tue, 12 Feb 2019 06:12:52 +0000 (01:12 -0500)]
Fix erroneous error reports in snapbuild.c.

It's pretty unhelpful to report the wrong file name in a complaint
about syscall failure, but SnapBuildSerialize managed to do that twice
in a span of 50 lines.  Also fix half a dozen missing or poorly-chosen
errcode assignments; that's mostly cosmetic, but still wrong.

Noted while studying recent failures on buildfarm member nightjar.
I'm not sure whether those reports are actually giving the wrong
filename, because there are two places here with identically
spelled error messages.  The other one is specifically coded not
to report ENOENT, but if it's this one, how could we be getting
ENOENT from open() with O_CREAT?  Need to sit back and await results.

However, these ereports are clearly broken from birth, so back-patch.

5 years agoFix description of WAL record XLOG_PARAMETER_CHANGE
Michael Paquier [Tue, 12 Feb 2019 04:10:59 +0000 (13:10 +0900)]
Fix description of WAL record XLOG_PARAMETER_CHANGE

max_wal_senders and max_worker_processes got reversed in the output
generated because of ea92368.

Reported-by: Kevin Hale Boyes
Discussion: https://postgr.es/m/CADAecHVAD4=26KAx4nj5DBvxqqvJkuwsy+riiiNhQqwnZg2K8Q@mail.gmail.com

5 years agoFix header inclusion issue.
Tom Lane [Tue, 12 Feb 2019 03:37:16 +0000 (22:37 -0500)]
Fix header inclusion issue.

partprune.h failed to compile by itself; needs to include partdefs.h.

I think I must've broken this in fa2cf164a, though I'd swear I ran
the appropriate tests when removing #includes.  Anyway, it's very
sensible for this file to include partdefs.h, so let's just do that.

Per cpluspluscheck.

5 years agoClarify docs about limitations of constraint exclusion with partitions
Michael Paquier [Tue, 12 Feb 2019 03:01:56 +0000 (12:01 +0900)]
Clarify docs about limitations of constraint exclusion with partitions

The current wording can confuse the reader about constraint exclusion
being available at query execution, but this only applies to partition
pruning.

Reported-by: Shouyu Luo
Author: David Rowley
Reviewed-by: Chapman Flack, Amit Langote
Discussion: https://postgr.es/m/15629-2ef8b22e61f8333f@postgresql.org

5 years agoAllow extensions to generate lossy index conditions.
Tom Lane [Tue, 12 Feb 2019 02:26:08 +0000 (21:26 -0500)]
Allow extensions to generate lossy index conditions.

For a long time, indxpath.c has had the ability to extract derived (lossy)
index conditions from certain operators such as LIKE.  For just as long,
it's been obvious that we really ought to make that capability available
to extensions.  This commit finally accomplishes that, by adding another
API for planner support functions that lets them create derived index
conditions for their functions.  As proof of concept, the hardwired
"special index operator" code formerly present in indxpath.c is pushed
out to planner support functions attached to LIKE and other relevant
operators.

A weak spot in this design is that an extension needs to know OIDs for
the operators, datatypes, and opfamilies involved in the transformation
it wants to make.  The core-code prototypes use hard-wired OID references
but extensions don't have that option for their own operators etc.  It's
usually possible to look up the required info, but that may be slow and
inconvenient.  However, improving that situation is a separate task.

I want to do some additional refactorization around selfuncs.c, but
that also seems like a separate task.

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

5 years agoMove max_wal_senders out of max_connections for connection slot handling
Michael Paquier [Tue, 12 Feb 2019 01:07:56 +0000 (10:07 +0900)]
Move max_wal_senders out of max_connections for connection slot handling

Since its introduction, max_wal_senders is counted as part of
max_connections when it comes to define how many connection slots can be
used for replication connections with a WAL sender context.  This can
lead to confusion for some users, as it could be possible to block a
base backup or replication from happening because other backend sessions
are already taken for other purposes by an application, and
superuser-only connection slots are not a correct solution to handle
that case.

This commit makes max_wal_senders independent of max_connections for its
handling of PGPROC entries in ProcGlobal, meaning that connection slots
for WAL senders are handled using their own free queue, like autovacuum
workers and bgworkers.

One compatibility issue that this change creates is that a standby now
requires to have a value of max_wal_senders at least equal to its
primary.  So, if a standby created enforces the value of
max_wal_senders to be lower than that, then this could break failovers.
Normally this should not be an issue though, as any settings of a
standby are inherited from its primary as postgresql.conf gets normally
copied as part of a base backup, so parameters would be consistent.

Author: Alexander Kukushkin
Reviewed-by: Kyotaro Horiguchi, Petr Jelínek, Masahiko Sawada, Oleksii
Kliukin
Discussion: https://postgr.es/m/CAFh8B=nBzHQeYAu0b8fjK-AF1X4+_p6GRtwG+cCgs6Vci2uRuQ@mail.gmail.com

5 years agoRedesign the partition dependency mechanism.
Tom Lane [Mon, 11 Feb 2019 19:41:13 +0000 (14:41 -0500)]
Redesign the partition dependency mechanism.

The original setup for dependencies of partitioned objects had
serious problems:

1. It did not verify that a drop cascading to a partition-child object
also cascaded to at least one of the object's partition parents.  Now,
normally a child object would share all its dependencies with one or
another parent (e.g. a child index's opclass dependencies would be shared
with the parent index), so that this oversight is usually harmless.
But if some dependency failed to fit this pattern, the child could be
dropped while all its parents remain, creating a logically broken
situation.  (It's easy to construct artificial cases that break it,
such as attaching an unrelated extension dependency to the child object
and then dropping the extension.  I'm not sure if any less-artificial
cases exist.)

2. Management of partition dependencies during ATTACH/DETACH PARTITION
was complicated and buggy; for example, after detaching a partition
table it was possible to create cases where a formerly-child index
should be dropped and was not, because the correct set of dependencies
had not been reconstructed.

Less seriously, because multiple partition relationships were
represented identically in pg_depend, there was an order-of-traversal
dependency on which partition parent was cited in error messages.
We also had some pre-existing order-of-traversal hazards for error
messages related to internal and extension dependencies.  This is
cosmetic to users but causes testing problems.

To fix #1, add a check at the end of the partition tree traversal
to ensure that at least one partition parent got deleted.  To fix #2,
establish a new policy that partition dependencies are in addition to,
not instead of, a child object's usual dependencies; in this way
ATTACH/DETACH PARTITION need not cope with adding or removing the
usual dependencies.

To fix the cosmetic problem, distinguish between primary and secondary
partition dependency entries in pg_depend, by giving them different
deptypes.  (They behave identically except for having different
priorities for being cited in error messages.)  This means that the
former 'I' dependency type is replaced with new 'P' and 'S' types.

This also fixes a longstanding bug that after handling an internal
dependency by recursing to the owning object, findDependentObjects
did not verify that the current target was now scheduled for deletion,
and did not apply the current recursion level's objflags to it.
Perhaps that should be back-patched; but in the back branches it
would only matter if some concurrent transaction had removed the
internal-linkage pg_depend entry before the recursive call found it,
or the recursive call somehow failed to find it, both of which seem
unlikely.

Catversion bump because the contents of pg_depend change for
partitioning relationships.

Patch HEAD only.  It's annoying that we're not fixing #2 in v11,
but there seems no practical way to do so given that the problem
is exactly a poor choice of what entries to put in pg_depend.
We can't really fix that while staying compatible with what's
in pg_depend in existing v11 installations.

Discussion: https://postgr.es/m/CAH2-Wzkypv1R+teZrr71U23J578NnTBt2X8+Y=Odr4pOdW1rXg@mail.gmail.com

5 years agoFix misleading PG_RE_THROW commentary
Alvaro Herrera [Mon, 11 Feb 2019 18:55:09 +0000 (15:55 -0300)]
Fix misleading PG_RE_THROW commentary

The old verbiage indicated that PG_RE_THROW is optional, which is not
really true.  This has confused many people, so it seems worth fixing.

Discussion: https://postgr.es/m/20190206160958.GA22304@alvherre.pgsql

5 years agoAdjust gratuitously different error message wording
Peter Eisentraut [Mon, 11 Feb 2019 13:01:05 +0000 (14:01 +0100)]
Adjust gratuitously different error message wording

5 years agoRemove unused macro
Peter Eisentraut [Mon, 11 Feb 2019 09:07:25 +0000 (10:07 +0100)]
Remove unused macro

Last use was removed in 2c66f9924c1162bfba27c77004ccf42fb6ea188d.

5 years agoFix indexable-row-comparison logic to account for covering indexes.
Tom Lane [Mon, 11 Feb 2019 03:51:32 +0000 (22:51 -0500)]
Fix indexable-row-comparison logic to account for covering indexes.

indxpath.c needs a good deal more attention for covering indexes than
it's gotten.  But so far as I can tell, the only really awful breakage
is in expand_indexqual_rowcompare (nee adjust_rowcompare_for_index),
which was only half fixed in c266ed31a.  The other problems aren't
bad enough to take the risk of a just-before-wrap fix.

The problem here is that if the leading column of a row comparison
matches an index (allowing this code to be reached), and some later
column doesn't match the index, it'll nonetheless believe that that
column matches the first included index column.  Typically that'll
lead to an error like "operator M is not a member of opfamily N" as
a result of fetching a garbage opfamily OID.  But with enough bad
luck, maybe a broken plan would be generated.

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

5 years agoAdd per-test-script runtime display to pg_regress.
Tom Lane [Sun, 10 Feb 2019 21:54:31 +0000 (16:54 -0500)]
Add per-test-script runtime display to pg_regress.

It seems useful to have this information available, so that it's
easier to tell when a test script is taking a disproportionate
amount of time.

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

5 years agoFix trigger drop procedure
Alvaro Herrera [Sun, 10 Feb 2019 13:00:11 +0000 (10:00 -0300)]
Fix trigger drop procedure

After commit 123cc697a8eb, we remove redundant FK action triggers during
partition ATTACH by merely deleting the catalog tuple, but that's wrong:
it should use performDeletion() instead.  Repair, and make the comments
more explicit.

Per code review from Tom Lane.

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

5 years agoSolve cross-version-upgrade testing problem induced by 1fb57af92.
Tom Lane [Sun, 10 Feb 2019 02:02:06 +0000 (21:02 -0500)]
Solve cross-version-upgrade testing problem induced by 1fb57af92.

Renaming varchar_transform to varchar_support had a side effect
I hadn't foreseen: the core regression tests leave around a
transform object that relies on that function, so the name
change breaks cross-version upgrade tests, because the name
used in the older branches doesn't match.

Since the dependency on varchar_transform was chosen with the
aid of a dartboard anyway (it would surely not work as a
language transform support function), fix by just choosing
a different random builtin function with the right signature.
Also add some comments explaining why this isn't horribly unsafe.

I chose to make the same substitution in a couple of other
copied-and-pasted test cases, for consistency, though those
aren't directly contributing to the testing problem.

Per buildfarm.  Back-patch, else it doesn't fix the problem.

5 years agoRepair unsafe/unportable snprintf usage in pg_restore.
Tom Lane [Sun, 10 Feb 2019 00:45:38 +0000 (19:45 -0500)]
Repair unsafe/unportable snprintf usage in pg_restore.

warn_or_exit_horribly() was blithely passing a potentially-NULL
string pointer to a %s format specifier.  That works (at least
to the extent of not crashing) on some platforms, but not all,
and since we switched to our own snprintf.c it doesn't work
for us anywhere.

Of the three string fields being handled this way here, I think
that only "owner" is supposed to be nullable ... but considering
that this is error-reporting code, it has very little business
assuming anything, so put in defenses for all three.

Per a crash observed on buildfarm member crake and then
reproduced here.  Because of the portability aspect,
back-patch to all supported versions.

5 years agoBuild out the planner support function infrastructure.
Tom Lane [Sat, 9 Feb 2019 23:32:23 +0000 (18:32 -0500)]
Build out the planner support function infrastructure.

Add support function requests for estimating the selectivity, cost,
and number of result rows (if a SRF) of the target function.

The lack of a way to estimate selectivity of a boolean-returning
function in WHERE has been a recognized deficiency of the planner
since Berkeley days.  This commit finally fixes it.

In addition, non-constant estimates of cost and number of output
rows are now possible.  We still fall back to looking at procost
and prorows if the support function doesn't service the request,
of course.

To make concrete use of the possibility of estimating output rowcount
for SRFs, this commit adds support functions for array_unnest(anyarray)
and the integer variants of generate_series; the lack of plausible
rowcount estimates for those, even when it's obvious to a human,
has been a repeated subject of complaints.  Obviously, much more
could now be done in this line, but I'm mostly just trying to get
the infrastructure in place.

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

5 years agoCreate the infrastructure for planner support functions.
Tom Lane [Sat, 9 Feb 2019 23:08:48 +0000 (18:08 -0500)]
Create the infrastructure for planner support functions.

Rename/repurpose pg_proc.protransform as "prosupport".  The idea is
still that it names an internal function that provides knowledge to
the planner about the behavior of the function it's attached to;
but redesign the API specification so that it's not limited to doing
just one thing, but can support an extensible set of requests.

The original purpose of simplifying a function call is handled by
the first request type to be invented, SupportRequestSimplify.
Adjust all the existing transform functions to handle this API,
and rename them fron "xxx_transform" to "xxx_support" to reflect
the potential generalization of what they do.  (Since we never
previously provided any way for extensions to add transform functions,
this change doesn't create an API break for them.)

Also add DDL and pg_dump support for attaching a support function to a
user-defined function.  Unfortunately, DDL access has to be restricted
to superusers, at least for now; but seeing that support functions
will pretty much have to be written in C, that limitation is just
theoretical.  (This support is untested in this patch, but a follow-on
patch will add cases that exercise it.)

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

5 years agoRefactor the representation of indexable clauses in IndexPaths.
Tom Lane [Sat, 9 Feb 2019 22:30:43 +0000 (17:30 -0500)]
Refactor the representation of indexable clauses in IndexPaths.

In place of three separate but interrelated lists (indexclauses,
indexquals, and indexqualcols), an IndexPath now has one list
"indexclauses" of IndexClause nodes.  This holds basically the same
information as before, but in a more useful format: in particular, there
is now a clear connection between an indexclause (an original restriction
clause from WHERE or JOIN/ON) and the indexquals (directly usable index
conditions) derived from it.

We also change the ground rules a bit by mandating that clause commutation,
if needed, be done up-front so that what is stored in the indexquals list
is always directly usable as an index condition.  This gets rid of repeated
re-determination of which side of the clause is the indexkey during costing
and plan generation, as well as repeated lookups of the commutator
operator.  To minimize the added up-front cost, the typical case of
commuting a plain OpExpr is handled by a new special-purpose function
commute_restrictinfo().  For RowCompareExprs, generating the new clause
properly commuted to begin with is not really any more complex than before,
it's just different --- and we can save doing that work twice, as the
pretty-klugy original implementation did.

Tracking the connection between original and derived clauses lets us
also track explicitly whether the derived clauses are an exact or lossy
translation of the original.  This provides a cheap solution to getting
rid of unnecessary rechecks of boolean index clauses, which previously
seemed like it'd be more expensive than it was worth.

Another pleasant (IMO) side-effect is that EXPLAIN now always shows
index clauses with the indexkey on the left; this seems less confusing.

This commit leaves expand_indexqual_conditions() and some related
functions in a slightly messy state.  I didn't bother to change them
any more than minimally necessary to work with the new data structure,
because all that code is going to be refactored out of existence in
a follow-on patch.

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

5 years agoCall set_rel_pathlist_hook before generate_gather_paths, not after.
Tom Lane [Sat, 9 Feb 2019 16:41:09 +0000 (11:41 -0500)]
Call set_rel_pathlist_hook before generate_gather_paths, not after.

The previous ordering of these steps satisfied the nominal requirement
that set_rel_pathlist_hook could editorialize on the whole set of Paths
constructed for a base relation.  In practice, though, trying to change
the set of partial paths was impossible.  Adding one didn't work because
(a) it was too late to be included in Gather paths made by the core code,
and (b) calling add_partial_path after generate_gather_paths is unsafe,
because it might try to delete a path it thinks is dominated, but that
is already embedded in some Gather path(s).  Nor could the hook safely
remove partial paths, for the same reason that they might already be
embedded in Gathers.

Better to call extensions first, let them add partial paths as desired,
and then gather.  In v11 and up, we already doubled down on that ordering
by postponing gathering even further for single-relation queries; so even
if the hook wished to editorialize on Gather path construction, it could
not.

Report and patch by KaiGai Kohei.  Back-patch to 9.6 where Gather paths
were added.

Discussion: https://postgr.es/m/CAOP8fzahwpKJRTVVTqo2AE=mDTz_efVzV6Get_0=U3SO+-ha1A@mail.gmail.com

5 years agoUse better comment marker in Autoconf input
Peter Eisentraut [Sat, 9 Feb 2019 14:55:17 +0000 (15:55 +0100)]
Use better comment marker in Autoconf input

The comment marker "#" is copied to the output, so it's only
appropriate for comments that make sense in the shell output.  For
comments about the Autoconf language, "dnl" should be used.

5 years agoReset, not recreate, execGrouping.c style hashtables.
Andres Freund [Sat, 9 Feb 2019 08:35:57 +0000 (00:35 -0800)]
Reset, not recreate, execGrouping.c style hashtables.

This uses the facility added in the preceding commit to fix
performance issues caused by rebuilding the hashtable (with its
comparator expression being the most expensive bit), after every
reset. That's especially important when the comparator is JIT
compiled.

Bug: #15592 #15486
Reported-By: Jakub Janeček, Dmitry Marakasov
Author: Andres Freund
Discussion:
    https://postgr.es/m/15486-05850f065da42931@postgresql.org
    https://postgr.es/m/20190114180423.ywhdg2iagzvh43we@alap3.anarazel.de
Backpatch: 11, where I broke this in bf6c614a2f2c5

5 years agoAllow to reset execGrouping.c style tuple hashtables.
Andres Freund [Sat, 9 Feb 2019 08:35:57 +0000 (00:35 -0800)]
Allow to reset execGrouping.c style tuple hashtables.

This has the advantage that the comparator expression, the table's
slot, etc do not have to be rebuilt. Additionally the simplehash.h
hashtable within the tuple hashtable now keeps its previous size and
doesn't need to be reallocated. That both reduces allocator overhead,
and improves performance in cases where the input estimation was off
by a significant factor.

To avoid an API/ABI break, the new parameter is exposed via the new
BuildTupleHashTableExt(), and BuildTupleHashTable() now is a wrapper
around the former, that continues to allocate the table itself in the
tablecxt.

Using this fixes performance issues discovered in the two bugs
referenced. This commit however has not converted the callers, that's
done in a separate commit.

Bug: #15592 #15486
Reported-By: Jakub Janeček, Dmitry Marakasov
Author: Andres Freund
Discussion:
    https://postgr.es/m/15486-05850f065da42931@postgresql.org
    https://postgr.es/m/20190114180423.ywhdg2iagzvh43we@alap3.anarazel.de
Backpatch: 11, this is a prerequisite for other fixes

5 years agosimplehash: Add support for resetting a hashtable's contents.
Andres Freund [Sat, 9 Feb 2019 08:35:57 +0000 (00:35 -0800)]
simplehash: Add support for resetting a hashtable's contents.

A hashtable reset just reset the hashtable entries, but does not free
memory.

Author: Andres Freund
Discussion: https://postgr.es/m/20190114180423.ywhdg2iagzvh43we@alap3.anarazel.de
Bug: #15592 #15486
Backpatch: 11, this is a prerequisite for other fixes

5 years agoPlug leak in BuildTupleHashTable by creating ExprContext in correct context.
Andres Freund [Sat, 9 Feb 2019 08:35:57 +0000 (00:35 -0800)]
Plug leak in BuildTupleHashTable by creating ExprContext in correct context.

In bf6c614a2f2c5 I added a expr context to evaluate the grouping
expression. Unfortunately the code I added initialized them while in
the calling context, rather the table context.  Additionally, I used
CreateExprContext() rather than CreateStandaloneExprContext(), which
creates the econtext in the estate's query context.

Fix that by using CreateStandaloneExprContext when in the table's
tablecxt. As we rely on the memory being freed by a memory context
reset that means that the econtext's shutdown callbacks aren't being
called, but that seems ok as the expressions are tightly controlled
due to ExecBuildGroupingEqual().

Bug: #15592
Reported-By: Dmitry Marakasov
Author: Andres Freund
Discussion: https://postgr.es/m/20190114222838.h6r3fuyxjxkykf6t@alap3.anarazel.de
Backpatch: 11, where I broke this in bf6c614a2f2c5

5 years agoDefend against null error message reported by libxml2.
Tom Lane [Fri, 8 Feb 2019 18:30:42 +0000 (13:30 -0500)]
Defend against null error message reported by libxml2.

While this isn't really supposed to happen, it can occur in OOM
situations and perhaps others.  Instead of crashing, substitute
"(no message provided)".

I didn't worry about localizing this text, since we aren't
localizing anything else here; besides, if we're on the edge of
OOM, it's unlikely gettext() would work.

Report and fix by Sergio Conde Gómez in bug #15624.

Discussion: https://postgr.es/m/15624-4dea54091a2864e6@postgresql.org

5 years agoDoc: fix thinko in description of how to escape a backslash in bytea.
Tom Lane [Fri, 8 Feb 2019 17:49:36 +0000 (12:49 -0500)]
Doc: fix thinko in description of how to escape a backslash in bytea.

Also clean up some discussion that had been left in a very confused
state thanks to half-hearted adjustments for the change to
standard_conforming_strings being the default.

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

5 years agoFix error handling around ssl_*_protocol_version settings
Peter Eisentraut [Fri, 8 Feb 2019 10:58:19 +0000 (11:58 +0100)]
Fix error handling around ssl_*_protocol_version settings

In case of a reload, we just want to LOG errors instead of FATAL when
processing SSL configuration, but the more recent code for the
ssl_*_protocol_version settings didn't behave like that.

Author: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
5 years agoAdd some const decorations
Peter Eisentraut [Fri, 8 Feb 2019 09:13:24 +0000 (10:13 +0100)]
Add some const decorations

These mainly help understanding the function signatures better.

5 years agoAdd pg_partition_root to display top-most parent of a partition tree
Michael Paquier [Thu, 7 Feb 2019 23:56:14 +0000 (08:56 +0900)]
Add pg_partition_root to display top-most parent of a partition tree

This is useful when looking at partition trees with multiple layers, and
combined with pg_partition_tree, it provides the possibility to show up
an entire tree by just knowing one member at any level.

Author: Michael Paquier
Reviewed-by: Álvaro Herrera, Amit Langote
Discussion: https://postgr.es/m/20181207014015.GP2407@paquier.xyz

5 years agoSplit create_foreignscan_path() into three functions.
Tom Lane [Thu, 7 Feb 2019 17:59:47 +0000 (12:59 -0500)]
Split create_foreignscan_path() into three functions.

Up to now postgres_fdw has been using create_foreignscan_path() to
generate not only base-relation paths, but also paths for foreign joins
and foreign upperrels.  This is wrong, because create_foreignscan_path()
calls get_baserel_parampathinfo() which will only do the right thing for
baserels.  It accidentally fails to fail for unparameterized paths, which
are the only ones postgres_fdw (thought it) was handling, but we really
need different APIs for the baserel and join cases.

In HEAD, the best thing to do seems to be to split up the baserel,
joinrel, and upperrel cases into three functions so that they can
have different APIs.  I haven't actually given create_foreign_join_path
a different API in this commit: we should spend a bit of time thinking
about just what we want to do there, since perhaps FDWs would want to
do something different from the build-up-a-join-pairwise approach that
get_joinrel_parampathinfo expects.  In the meantime, since postgres_fdw
isn't prepared to generate parameterized joins anyway, just give it a
defense against trying to plan joins with lateral refs.

In addition (and this is what triggered this whole mess) fix bug #15613
from Srinivasan S A, by teaching file_fdw and postgres_fdw that plain
baserel foreign paths still have outer refs if the relation has
lateral_relids.  Add some assertions in relnode.c to catch future
occurrences of the same error --- in particular, to catch other FDWs
doing that, but also as backstop against core-code mistakes like the
one fixed by commit bdd9a99aa.

Bug #15613 also needs to be fixed in the back branches, but the
appropriate fix will look quite a bit different there, since we don't
want to assume that existing FDWs get the word right away.

Discussion: https://postgr.es/m/15613-092be1be9576c728@postgresql.org

5 years agoFix perl searchpath for gen_keywordlist.pl
Andrew Dunstan [Thu, 7 Feb 2019 16:14:29 +0000 (11:14 -0500)]
Fix perl searchpath for gen_keywordlist.pl

as found by running src/tools/perlcheck/pgperlsyncheck

5 years agoFix searchpath and module location for pg_rewind and ssl TAP tests
Andrew Dunstan [Thu, 7 Feb 2019 15:22:49 +0000 (10:22 -0500)]
Fix searchpath and module location for pg_rewind and ssl TAP tests

The modules RewindTest.pm and ServerSetup.pm are really only useful for
TAP tests, so they really belong in the TAP test directories. In
addition, ServerSetup.pm is renamed to SSLServer.pm.

The test scripts have their own directories added to the search path so
that the relocated modules will be found, regardless of where the tests
are run from, even on modern perl where "." is no longer in the
searchpath.

Discussion: https://postgr.es/m/e4b0f366-269c-73c3-9c90-d9cb0f4db1f9@2ndQuadrant.com

Backpatch as appropriate to 9.5

5 years agoUse EXECUTE FUNCTION syntax for triggers more
Peter Eisentraut [Thu, 7 Feb 2019 08:01:54 +0000 (09:01 +0100)]
Use EXECUTE FUNCTION syntax for triggers more

Change pg_dump and ruleutils.c to use the FUNCTION keyword instead of
PROCEDURE in trigger and event trigger definitions.

This completes the pieces of the transition started in
0a63f996e018ac508c858e87fa39cc254a5db49f that were kept out of
PostgreSQL 11 because of the required catversion change.

Discussion: https://www.postgresql.org/message-id/381bef53-f7be-29c8-d977-948e389161d6@2ndquadrant.com

5 years agoAllow some recovery parameters to be changed with reload
Peter Eisentraut [Mon, 4 Feb 2019 08:28:17 +0000 (09:28 +0100)]
Allow some recovery parameters to be changed with reload

Change

archive_cleanup_command
promote_trigger_file
recovery_end_command
recovery_min_apply_delay

from PGC_POSTMASTER to PGC_SIGHUP.  This did not require any further
changes.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/ca28011a-cfaa-565c-d622-c1907c33ecf7%402ndquadrant.com

5 years agoAdd collation assignment to CALL statement
Peter Eisentraut [Tue, 5 Feb 2019 14:08:53 +0000 (15:08 +0100)]
Add collation assignment to CALL statement

Otherwise functions that require collation information will not have
it if they are called in arguments to a CALL statement.

Reported-by: Jean-Marc Voillequin <Jean-Marc.Voillequin@moodys.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/1EC8157EB499BF459A516ADCF135ADCE39FFAC54%40LON-WGMSX712.ad.moodys.net

5 years agoDoc: Update the documentation for row movement behavior across partitions.
Amit Kapila [Thu, 7 Feb 2019 03:28:29 +0000 (08:58 +0530)]
Doc: Update the documentation for row movement behavior across partitions.

In commit f16241bef7c, we have changed the behavior for concurrent updates
that move row to a different partition, but forgot to update the docs.
Previously when an UPDATE command causes a row to move from one partition
to another, there is a chance that another concurrent UPDATE or DELETE
misses this row.  However, now we raise a serialization failure error in
such a case.

Reported-by: David Rowley
Author: David Rowley and Amit Kapila
Backpatch-through: 11 where it was introduced
Discussion: https://postgr.es/m/CAKJS1f-iVhGD4-givQWpSROaYvO3c730W8yoRMTF9Gc3craY3w@mail.gmail.com

5 years agoAlign better test output regex with grammar in pg_dump TAP tests
Michael Paquier [Thu, 7 Feb 2019 01:04:55 +0000 (10:04 +0900)]
Align better test output regex with grammar in pg_dump TAP tests

This enforces one-or-more character matches in the regular expressions
for pg_dump testing on SQL syntax output where zero-or-more matches
implies a syntax error.

Author: Daniel Gustafsson
Reviewed-by: David G. Johnston, Michael Paquier
Discussion: https://postgr.es/m/B313C32C-0E24-4AFB-95FF-6DA0C4E18A89@yesql.se

5 years agoAdd more tests for CREATE TABLE AS with WITH NO DATA
Michael Paquier [Thu, 7 Feb 2019 00:21:57 +0000 (09:21 +0900)]
Add more tests for CREATE TABLE AS with WITH NO DATA

The relation creation is done at executor startup, however the main
regression test suite is lacking scenarios where no data is inserted
which is something that can happen when using EXECUTE or EXPLAIN with
CREATE TABLE AS and WITH NO DATA.

Some patches are worked on to reshape the way CTAS relations are
created, so this makes sure that we do not miss some query patterns
already supported.

Reported-by: Andreas Karlsson
Author: Michael Paquier
Reviewed-by: Andreas Karlsson
Discussion: https://postgr.es/m/20190206091817.GB14980@paquier.xyz