]> granicus.if.org Git - postgresql/log
postgresql
5 years agoFix SHOW ALL command for non-superusers with replication connection
Michael Paquier [Mon, 15 Apr 2019 03:34:32 +0000 (12:34 +0900)]
Fix SHOW ALL command for non-superusers with replication connection

Since Postgres 10, SHOW commands can be triggered with replication
connections in a WAL sender context, however it missed that a
transaction context is needed for syscache lookups.  This commit makes
sure that the syscache lookups can happen correctly by setting a
transaction context when running SHOW commands in a WAL sender.

Superuser-only parameters can be displayed using SHOW commands not only
to superusers, but also to members of system role pg_read_all_settings,
which requires a syscache lookup to check if the connected role is a
member of this system role or not, or the instance crashes.  Superusers
do not need to check the syscache so it worked correctly in this case.

New tests are added to cover this issue.

Reported-by: Alexander Kukushkin
Author: Michael Paquier
Reviewed-by: Álvaro Herrera
Discussion: https://postgr.es/m/15734-2daa8761eeed8e20@postgresql.org
Backpatch-through: 10

5 years agoTest both 0.0.0.0 and 127.0.0.x addresses to find a usable port.
Noah Misch [Mon, 15 Apr 2019 03:02:19 +0000 (20:02 -0700)]
Test both 0.0.0.0 and 127.0.0.x addresses to find a usable port.

Commit c098509927f9a49ebceb301a2cb6a477ecd4ac3c changed
PostgresNode::get_new_node() to probe 0.0.0.0 instead of 127.0.0.1, but
the new test was less effective for Windows native Perl.  This increased
the failure rate of buildfarm members bowerbird and jacana.  Instead,
test 0.0.0.0 and concrete addresses.  This restores the old level of
defense, but the algorithm is still subject to its longstanding time of
check to time of use race condition.  Back-patch to 9.6, like the
previous change.

Discussion: https://postgr.es/m/GrdLgAdUK9FdyZg8VIcTDKVOkys122ZINEb3CjjoySfGj2KyPiMKTh1zqtRp0TAD7FJ27G-OBB3eplxIB5GhcQH5o8zzGZfp0MuJaXJxVxk=@yesql.se

5 years agoSwitch TAP tests of pg_rewind to use non-superuser role, take two
Michael Paquier [Sun, 14 Apr 2019 09:47:51 +0000 (18:47 +0900)]
Switch TAP tests of pg_rewind to use non-superuser role, take two

Up to now the tests of pg_rewind have been using a superuser for all its
tests (which is the default of many tests actually, and something that
ought to be reviewed) when involving an online source server, still it
is possible to use a non-superuser role to do that as long as this role
is granted permissions to execute all the source-side functions used for
the rewind.  This is possible since v11, and was already documented as
of bfc8068.

PostgresNode::init is extended so as callers of this routine can add
extra options to configure the authentication of a new node, which gets
used by this commit, and allows the tests to work properly on Windows
where SSPI is used.

This will allow to catch up easily any change in pg_rewind if the tool
begins to use more backend-side functions, so as the properties
introduced by v11 are kept.

Per suggestion from Peter Eisentraut.

Author: Michael Paquier
Reviewed-by: Magnus Hagander
Discussion: https://postgr.es/m/20190411041336.GM2728@paquier.xyz

5 years agoMSYS: Translate REGRESS_SHLIB to a Windows file name.
Noah Misch [Sun, 14 Apr 2019 07:42:34 +0000 (00:42 -0700)]
MSYS: Translate REGRESS_SHLIB to a Windows file name.

Per buildfarm member jacana.  Back-patch to v11; earlier branches skip
the affected test under msys.

Discussion: https://postgr.es/m/GrdLgAdUK9FdyZg8VIcTDKVOkys122ZINEb3CjjoySfGj2KyPiMKTh1zqtRp0TAD7FJ27G-OBB3eplxIB5GhcQH5o8zzGZfp0MuJaXJxVxk=@yesql.se

5 years agoWhen Perl "kill(9, ...)" fails, try "pg_ctl kill".
Noah Misch [Sat, 13 Apr 2019 18:09:27 +0000 (11:09 -0700)]
When Perl "kill(9, ...)" fails, try "pg_ctl kill".

Per buildfarm member jacana, the former fails under msys Perl 5.8.8.
Back-patch to 9.6, like the code in question.

Discussion: https://postgr.es/m/GrdLgAdUK9FdyZg8VIcTDKVOkys122ZINEb3CjjoySfGj2KyPiMKTh1zqtRp0TAD7FJ27G-OBB3eplxIB5GhcQH5o8zzGZfp0MuJaXJxVxk=@yesql.se

5 years agoPrevent memory leaks associated with relcache rd_partcheck structures.
Tom Lane [Sat, 13 Apr 2019 17:22:26 +0000 (13:22 -0400)]
Prevent memory leaks associated with relcache rd_partcheck structures.

The original coding of generate_partition_qual() just copied the list
of predicate expressions into the global CacheMemoryContext, making it
effectively impossible to clean up when the owning relcache entry is
destroyed --- the relevant code in RelationDestroyRelation() only managed
to free the topmost List header :-(.  This resulted in a session-lifespan
memory leak whenever a table partition's relcache entry is rebuilt.
Fortunately, that's not normally a large data structure, and rebuilds
shouldn't occur all that often in production situations; but this is
still a bug worth fixing back to v10 where the code was introduced.

To fix, put the cached expression tree into its own small memory context,
as we do with other complicated substructures of relcache entries.
Also, deal more honestly with the case that a partition has an empty
partcheck list; while that probably isn't a case that's very interesting
for production use, it's legal.

In passing, clarify comments about how partitioning-related relcache
data structures are managed, and add some Asserts that we're not leaking
old copies when we overwrite these data fields.

Amit Langote and Tom Lane

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

5 years agoConsistently test for in-use shared memory.
Noah Misch [Sat, 13 Apr 2019 05:36:38 +0000 (22:36 -0700)]
Consistently test for in-use shared memory.

postmaster startup scrutinizes any shared memory segment recorded in
postmaster.pid, exiting if that segment matches the current data
directory and has an attached process.  When the postmaster.pid file was
missing, a starting postmaster used weaker checks.  Change to use the
same checks in both scenarios.  This increases the chance of a startup
failure, in lieu of data corruption, if the DBA does "kill -9 `head -n1
postmaster.pid` && rm postmaster.pid && pg_ctl -w start".  A postmaster
will no longer stop if shmat() of an old segment fails with EACCES.  A
postmaster will no longer recycle segments pertaining to other data
directories.  That's good for production, but it's bad for integration
tests that crash a postmaster and immediately delete its data directory.
Such a test now leaks a segment indefinitely.  No "make check-world"
test does that.  win32_shmem.c already avoided all these problems.  In
9.6 and later, enhance PostgresNode to facilitate testing.  Back-patch
to 9.4 (all supported versions).

Reviewed (in earlier versions) by Daniel Gustafsson and Kyotaro HORIGUCHI.

Discussion: https://postgr.es/m/20190408064141.GA2016666@rfd.leadboat.com

5 years agoRevert "Switch TAP tests of pg_rewind to use a role with minimal permissions"
Michael Paquier [Sat, 13 Apr 2019 04:20:21 +0000 (13:20 +0900)]
Revert "Switch TAP tests of pg_rewind to use a role with minimal permissions"

This reverts commit d4e2a84, which added a new user with limited
permissions to run the TAP tests of pg_rewind.  Buildfarm machine
members on Windows jacana and bowerbird have been complaining about
that, the new role not being able to run the rewind because SSPI is not
configured to allow it.

Fixing the test requires passing down directly the new user to
pg_regress with --create-role so as SSPI can work properly.

Reported-by: Andrew Dunstan
Discussion: https://postgr.es/m/3cd43d33-f415-cc41-ade3-7230ab15b2c9@2ndQuadrant.com

5 years agoShow shared object statistics in pg_stat_database
Magnus Hagander [Fri, 12 Apr 2019 12:04:50 +0000 (14:04 +0200)]
Show shared object statistics in pg_stat_database

This adds a row to the pg_stat_database view with datoid 0 and datname
NULL for those objects that are not in a database. This was added
particularly for checksums, but we were already tracking more satistics
for these objects, just not returning it.

Also add a checksum_last_failure column that holds the timestamptz of
the last checksum failure that occurred in a database (or in a
non-dataabase file), if any.

Author: Julien Rouhaud <rjuju123@gmail.com>

5 years agoFix REINDEX CONCURRENTLY of partitions
Peter Eisentraut [Fri, 12 Apr 2019 06:36:05 +0000 (08:36 +0200)]
Fix REINDEX CONCURRENTLY of partitions

In case of a partition index, when swapping the old and new index, we
also need to attach the new index as a partition and detach the old
one.  Also, to handle partition indexes, we not only need to change
dependencies referencing the index, but also dependencies of the index
referencing something else.  The previous code did this only
specifically for a constraint, but we also need to do this for
partitioned indexes.  So instead write a generic function that does it
for all dependencies.

Author: Michael Paquier <michael@paquier.xyz>
Author: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Discussion: https://www.postgresql.org/message-id/flat/DF4PR8401MB11964EDB77C860078C343BEBEE5A0%40DF4PR8401MB1196.NAMPRD84.PROD.OUTLOOK.COM#154df1fedb735190a773481765f7b874

5 years agoFix GetNewTransactionId()'s interaction with xidVacLimit.
Thomas Munro [Fri, 12 Apr 2019 02:53:38 +0000 (14:53 +1200)]
Fix GetNewTransactionId()'s interaction with xidVacLimit.

Commit ad308058 switched to returning a FullTransactionId, but
failed to load the potentially updated value in the case where
xidVacLimit is reached and we release and reacquire the lock.
Repair, closing bug #15727.

While reviewing that commit, also fix the size computation used
by EstimateTransactionStateSize() and switch to the mul_size()
macro traditionally used in such expressions.

Author: Thomas Munro
Reported-by: Roman Zharkov
Discussion: https://postgr.es/m/15727-0be246e7d852d229%40postgresql.org

5 years agoFix typos in reloptions.c
Michael Paquier [Fri, 12 Apr 2019 03:56:38 +0000 (12:56 +0900)]
Fix typos in reloptions.c

Author: Kirk Jamison
Discussion: https://postgr.es/m/D09B13F772D2274BB348A310EE3027C6493463@g01jpexmbkw24

5 years agoSwitch TAP tests of pg_rewind to use a role with minimal permissions
Michael Paquier [Fri, 12 Apr 2019 01:46:43 +0000 (10:46 +0900)]
Switch TAP tests of pg_rewind to use a role with minimal permissions

Up to now the tests of pg_rewind have been using a superuser for all the
tests (which is the default of many tests actually, and something that
ought to be reviewed) when involving an online source server, still it
is possible to use a non-superuser role to do that as long as this role
is granted permissions to execute all the source-side functions used for
the rewind.  This is possible since v11, and was already documented as
of bfc8068.

This will allow to catch up easily any change in pg_rewind if the tool
begins to use more backend-side functions, so as the properties
introduced by v11 are kept.

Per suggestion from Peter Eisentraut.

Author: Michael Paquier
Reviewed-by: Magnus Hagander
Discussion: https://postgr.es/m/20190411041336.GM2728@paquier.xyz

5 years agoFix more strcmp() calls using boolean-like comparisons for result checks
Michael Paquier [Fri, 12 Apr 2019 01:16:49 +0000 (10:16 +0900)]
Fix more strcmp() calls using boolean-like comparisons for result checks

Such calls can confuse the reader as strcmp() uses an integer as result.
The places patched here have been spotted by Thomas Munro, David Rowley
and myself.

Author: Michael Paquier
Reviewed-by: David Rowley
Discussion: https://postgr.es/m/20190411021946.GG2728@paquier.xyz

5 years agoRe-order some regression test scripts for more parallelism.
Tom Lane [Thu, 11 Apr 2019 22:16:50 +0000 (18:16 -0400)]
Re-order some regression test scripts for more parallelism.

Move the strings, numerology, insert, insert_conflict, select and
errors tests to be parts of nearby parallel groups, instead of
executing by themselves.  (Moving "select" required adjusting the
constraints test, which uses a table named "tmp" as select also
does.  There don't seem to be any other conflicts.)

Move psql and stats_ext to the next parallel group, where the rules
test also has a long runtime.  To make it safe to run stats_ext in
parallel with rules, I adjusted the latter to only dump views/rules
from the pg_catalog and public schemas, which was what it was doing
anyway.  stats_ext makes some views in a transient schema, which now
will not affect rules.

Reorder serial_schedule to match parallel_schedule.

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

5 years agoSpeed up sort-order-comparison tests in create_index_spgist.
Tom Lane [Thu, 11 Apr 2019 21:01:35 +0000 (17:01 -0400)]
Speed up sort-order-comparison tests in create_index_spgist.

This test script verifies that KNN searches of an SP-GiST index
produce the same sort order as a seqscan-and-sort.  The FULL JOINs
used for that are exceedingly slow, however.  Investigation shows
that the problem is that the initial join is on the rank() values,
and we have a lot of duplicates due to the data set containing 1000
duplicate points.  We're therefore going to produce 1000000 join
rows that have to be thrown away again by the join filter.

We can improve matters by using row_number() instead of rank(),
so that the initial join keys are unique.  The catch is that
that makes the results sensitive to the sorting of rows with
equal distances from the reference point.  That doesn't matter
for the actually-equal points, but as luck would have it, the
data set also contains two distinct points that have identical
distances to the origin.  So those two rows could legitimately
appear in either order, causing unwanted output from the check
queries.

However, it doesn't seem like it's the job of this test to
check whether the <-> operator correctly computes distances;
its charter is just to verify that SP-GiST emits the values
in distance order.  So we can dodge the indeterminacy problem
by having the check only compare row numbers and distances
not the actual point values.

This change reduces the run time of create_index_spgist by a good
three-quarters, on my machine, with ensuing beneficial effects on
the runtime of create_index (thanks to interactions with CREATE
INDEX CONCURRENTLY tests in the latter).  I see a net improvement
of more than 2X in the runtime of their parallel test group.

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

5 years agoSplit up a couple of long-running regression test scripts.
Tom Lane [Thu, 11 Apr 2019 20:15:54 +0000 (16:15 -0400)]
Split up a couple of long-running regression test scripts.

The point of this change is to increase the potential for parallelism
while running the core regression tests.  Most people these days are
using parallel testing modes on multi-core machines, so we might as
well try a bit harder to keep multiple cores busy.  Hence, a test that
runs much longer than others in its parallel group is a candidate to
be sub-divided.

In this patch, create_index.sql and join.sql are split up.
I haven't changed the content of the tests in any way, just
moved them.

I moved create_index.sql's SP-GiST-related tests into a new script
create_index_spgist, and moved its btree multilevel page deletion test
over to the existing script btree_index.  (btree_index is a more natural
home for that test, and it's shorter than others in its parallel group,
so this doesn't hurt total runtime of that group.)  There might be
room for more aggressive splitting of create_index, but this is enough
to improve matters considerably.

Likewise, I moved join.sql's "exercises for the hash join code" into
a new file join_hash.  Those exercises contributed three-quarters of
the script's runtime.  Which might well be excessive ... but for the
moment, I'm satisfied with shoving them into a different parallel
group, where they can share runtime with the roughly-equally-lengthy
gist test.

(Note for anybody following along at home: there are interesting
interactions between the runtimes of create_index and anything running
in parallel with it, because the tests of CREATE INDEX CONCURRENTLY
in that file will repeatedly block waiting for concurrent transactions
to commit.  As committed in this patch, create_index and
create_index_spgist have roughly equal runtimes, but that's mostly an
artifact of forced synchronization of the CONCURRENTLY tests; when run
serially, create_index is much faster.  A followup patch will reduce
the runtime of create_index_spgist and thereby also create_index.)

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

5 years agoMove plpgsql error-trapping tests to a new module-specific test file.
Tom Lane [Thu, 11 Apr 2019 19:09:18 +0000 (15:09 -0400)]
Move plpgsql error-trapping tests to a new module-specific test file.

The test for statement timeout has a 2-second timeout, which was only
moderately annoying when it was written, but nowadays it contributes
a pretty significant chunk of the elapsed time needed to run the core
regression tests on a fast machine.  We can improve this situation by
pushing the test into a plpgsql-specific test file instead of having
it in a core regression test.  That's a clean win when considering
just the core tests.  Even when considering check-world or a buildfarm
test run, we should come out ahead because the core tests get run
more times in those sequences.

Furthermore, since the plpgsql tests aren't currently parallelized,
it seems likely that the timing problems reflected in commit f1e671a0b
(which increased that timeout from 1 sec to 2) will be much less severe
in this context.  Hence, let's try cutting the timeout back to 1 second
in hopes of a further win for check-world.  We can undo that if
buildfarm experience proves it to be a bad idea.

To give the new test file some modicum of intellectual coherency,
I moved the surrounding tests related to error-trapping along with
the statement timeout test proper.  Those other tests don't run long
enough to have any particular bearing on test-runtime considerations.
The tests are the same as before, except with minor adjustments to
not depend on an externally-created table.

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

5 years agoFix off-by-one check that can lead to a memory overflow in ecpg.
Michael Meskes [Thu, 11 Apr 2019 18:56:17 +0000 (20:56 +0200)]
Fix off-by-one check that can lead to a memory overflow in ecpg.

Patch by Liu Huailing <liuhuailing@cn.fujitsu.com>

5 years agoRemove duplicative polygon SP-GiST sequencing test.
Tom Lane [Thu, 11 Apr 2019 18:35:47 +0000 (14:35 -0400)]
Remove duplicative polygon SP-GiST sequencing test.

Code coverage comparisons confirm that the tests using
quad_poly_tbl_ord_seq1/quad_poly_tbl_ord_idx1 hit no code
paths not also covered by the similar tests using
quad_poly_tbl_ord_seq2/quad_poly_tbl_ord_idx2.  Since these
test cases are pretty expensive, they need to contribute more
than zero benefit.

In passing, make quad_poly_tbl_ord_seq2 a temp table, since
there seems little reason to keep it around after the test.

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

5 years agodoc: adjust libpq wording to be neither/nor
Bruce Momjian [Thu, 11 Apr 2019 17:25:34 +0000 (13:25 -0400)]
doc:  adjust libpq wording to be neither/nor

Reported-by: postgresql@cohi.at
Discussion: https://postgr.es/m/155419437926.737.10876947446993402227@wrigleys.postgresql.org

Backpatch-through: 9.4

5 years agoRemove redundant and ineffective test for btree insertion fast path.
Tom Lane [Thu, 11 Apr 2019 17:15:59 +0000 (13:15 -0400)]
Remove redundant and ineffective test for btree insertion fast path.

indexing.sql's test for this feature was added along with the
feature in commit 2b2727343.  However, shortly later that test was
rendered ineffective by commit 074251db6, which limited when the
optimization would be applied, so that the test didn't test it.
Since then, commit dd299df81 added new tests (in btree_index.sql)
that actually do test the feature.  Code coverage comparisons
confirm that this test sequence adds no meaningful coverage, and
it's rather expensive, accounting for nearly half of the runtime
of indexing.sql according to my measurements.  So let's remove it.

Per advice from Peter Geoghegan.

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

5 years agoFix declaration after statement
Alvaro Herrera [Thu, 11 Apr 2019 02:28:50 +0000 (22:28 -0400)]
Fix declaration after statement

This style is frowned upon.  I inadvertently introduced one in commit
fe0e0b4fc7f0.  (My compiler does not complain about it, even though
-Wdeclaration-after-statement is specified.  Weird.)

Author: Masahiko Sawada

5 years agoFix backwards test in operator_precedence_warning logic.
Tom Lane [Wed, 10 Apr 2019 23:02:21 +0000 (19:02 -0400)]
Fix backwards test in operator_precedence_warning logic.

Warnings about unary minus might have been wrong.  It's a bit
surprising that nobody noticed yet ... probably the precedence-warning
feature hasn't really been used much in the field.

Rikard Falkeborn

Discussion: https://postgr.es/m/CADRDgG6fzA8A2oeygUw4=o7ywo4kvz26NxCSgpq22nMD73Bx4Q@mail.gmail.com

5 years agopg_restore: Make not verbose by default
Peter Eisentraut [Wed, 10 Apr 2019 09:45:00 +0000 (11:45 +0200)]
pg_restore: Make not verbose by default

This was accidentally changed in
cc8d41511721d25d557fc02a46c053c0a602fed0.

Reported-by: Christoph Berg <myon@debian.org>
5 years agoAvoid counting transaction stats for parallel worker cooperating
Amit Kapila [Wed, 10 Apr 2019 02:54:15 +0000 (08:24 +0530)]
Avoid counting transaction stats for parallel worker cooperating
transaction.

The transaction that is initiated by the parallel worker to cooperate
with the actual transaction started by the main backend to complete the
query execution should not be counted as a separate transaction.  The
other internal transactions started and committed by the parallel worker
are still counted as separate transactions as we that is what we do in
other places like autovacuum.

This will partially fix the bloat in transaction stats due to additional
transactions performed by parallel workers.  For a complete fix, we need to
decide how we want to show all the transactions that are started internally
for various operations and that is a matter of separate patch.

Reported-by: Haribabu Kommi
Author: Haribabu Kommi
Reviewed-by: Amit Kapila, Jamison Kirk and Rahila Syed
Backpatch-through: 9.6
Discussion: https://postgr.es/m/CAJrrPGc9=jKXuScvNyQ+VNhO0FZk7LLAShAJRyZjnedd2D61EQ@mail.gmail.com

5 years agoImprove comment in sync.h.
Thomas Munro [Wed, 10 Apr 2019 00:49:49 +0000 (12:49 +1200)]
Improve comment in sync.h.

Per off-list complaint from Andres Freund.

5 years agoFix typos.
Thomas Munro [Tue, 9 Apr 2019 21:21:06 +0000 (09:21 +1200)]
Fix typos.

5 years agoPrevent inlining of multiply-referenced CTEs with outer recursive refs.
Tom Lane [Tue, 9 Apr 2019 19:47:26 +0000 (15:47 -0400)]
Prevent inlining of multiply-referenced CTEs with outer recursive refs.

This has to be prevented because inlining would result in multiple
self-references, which we don't support (and in fact that's disallowed
by the SQL spec, see statements about linearly vs. nonlinearly
recursive queries).  Bug fix for commit 608b167f9.

Per report from Yaroslav Schekin (via Andrew Gierth)

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

5 years agoFix typo
Alvaro Herrera [Tue, 9 Apr 2019 16:59:53 +0000 (12:59 -0400)]
Fix typo

5 years agoFix memory leak in pgbench
Alvaro Herrera [Tue, 9 Apr 2019 16:46:34 +0000 (12:46 -0400)]
Fix memory leak in pgbench

Commit 25ee70511ec2 introduced a memory leak in pgbench: some PGresult
structs were not being freed during error bailout, because we're now
doing more PQgetResult() calls than previously.  Since there's more
cleanup code outside the discard_response() routine than in it, refactor
the cleanup code, removing the routine.

This has little effect currently, since we abandon processing after
hitting errors, but if we ever get further pgbench features (such as
testing for serializable transactions), it'll matter.

Per Coverity.

Reviewed-by: Michaël Paquier
5 years agoTest some more cases with partitioned tables in EvalPlanQual.
Tom Lane [Tue, 9 Apr 2019 15:42:53 +0000 (11:42 -0400)]
Test some more cases with partitioned tables in EvalPlanQual.

We weren't testing anything involving EPQ on UPDATEs that move tuples
into different partitions.  Depending on the implementation,
it might be that these cases aren't actually very interesting ...
but given our thin coverage of EPQ in general, I think it's a good
idea to have a test case.

Amit Langote, minor tweak by me

Discussion: https://postgr.es/m/7889df35-ad1a-691a-00e3-4d4b18f364e3@lab.ntt.co.jp

5 years agoDefine WIN32_STACK_RLIMIT throughout win32 and cygwin builds.
Noah Misch [Tue, 9 Apr 2019 15:25:39 +0000 (08:25 -0700)]
Define WIN32_STACK_RLIMIT throughout win32 and cygwin builds.

The MSVC build system already did this, and commit
617dc6d299c957e2784320382b3277ede01d9c63 used it in a second file.
Back-patch to 9.4, like that commit.

Discussion: https://postgr.es/m/CAA8=A7_1SWc3+3Z=-utQrQFOtrj_DeohRVt7diA2tZozxsyUOQ@mail.gmail.com

5 years agoReplace tabs with spaces in one .sql file
Peter Eisentraut [Tue, 9 Apr 2019 12:37:37 +0000 (14:37 +0200)]
Replace tabs with spaces in one .sql file

Let's at least keep this consistent within the same file.

5 years agoFix example in comment.
Heikki Linnakangas [Tue, 9 Apr 2019 05:32:02 +0000 (08:32 +0300)]
Fix example in comment.

Author: Adrien Nayrat

5 years agoAvoid "could not reattach" by providing space for concurrent allocation.
Noah Misch [Tue, 9 Apr 2019 04:39:00 +0000 (21:39 -0700)]
Avoid "could not reattach" by providing space for concurrent allocation.

We've long had reports of intermittent "could not reattach to shared
memory" errors on Windows.  Buildfarm member dory fails that way when
PGSharedMemoryReAttach() execution overlaps with creation of a thread
for the process's "default thread pool".  Fix that by providing a second
region to receive asynchronous allocations that would otherwise intrude
into UsedShmemSegAddr.  In pgwin32_ReserveSharedMemoryRegion(), stop
trying to free reservations landing at incorrect addresses; the caller's
next step has been to terminate the affected process.  Back-patch to 9.4
(all supported versions).

Reviewed by Tom Lane.  He also did much of the prerequisite research;
see commit bcbf2346d69f6006f126044864dd9383d50d87b4.

Discussion: https://postgr.es/m/20190402135442.GA1173872@rfd.leadboat.com

5 years agotableam: comment and formatting fixes.
Andres Freund [Mon, 8 Apr 2019 17:25:16 +0000 (10:25 -0700)]
tableam: comment and formatting fixes.

Author: Heikki Linnakangas
Discussion: https://postgr.es/m/9a7fb9cc-2419-5db7-8840-ddc10c93f122@iki.fi

5 years agodoc: Fix whitespace
Peter Eisentraut [Mon, 8 Apr 2019 20:27:35 +0000 (22:27 +0200)]
doc: Fix whitespace

Author: Julien Rouhaud <rjuju123@gmail.com>

5 years agoFix improper interaction of FULL JOINs with lateral references.
Tom Lane [Mon, 8 Apr 2019 20:09:06 +0000 (16:09 -0400)]
Fix improper interaction of FULL JOINs with lateral references.

join_is_legal() needs to reject forming certain outer joins in cases
where that would lead the planner down a blind alley.  However, it
mistakenly supposed that the way to handle full joins was to treat them
as applying the same constraints as for left joins, only to both sides.
That doesn't work, as shown in bug #15741 from Anthony Skorski: given
a lateral reference out of a join that's fully enclosed by a full join,
the code would fail to believe that any join ordering is legal, resulting
in errors like "failed to build any N-way joins".

However, we don't really need to consider full joins at all for this
purpose, because we effectively force them to be evaluated in syntactic
order, and that order is always legal for lateral references.  Hence,
get rid of this broken logic for full joins and just ignore them instead.

This seems to have been an oversight in commit 7e19db0c0.
Back-patch to all supported branches, as that was.

Discussion: https://postgr.es/m/15741-276f1f464b3f40eb@postgresql.org

5 years agodoc: Update serial explanation
Peter Eisentraut [Mon, 8 Apr 2019 20:03:48 +0000 (22:03 +0200)]
doc: Update serial explanation

The CREATE SEQUENCE command should include a data type specification,
since PostgreSQL 10.

Reported-by: mjf@pearson.co.uk
5 years agoFix EvalPlanQualStart to handle partitioned result rels correctly.
Tom Lane [Mon, 8 Apr 2019 16:20:22 +0000 (12:20 -0400)]
Fix EvalPlanQualStart to handle partitioned result rels correctly.

The es_root_result_relations array needs to be shallow-copied in the
same way as the main es_result_relations array, else EPQ rechecks on
partitioned result relations fail, as seen in bug #15677 from
Norbert Benkocs.

Amit Langote, isolation test case added by me

Discussion: https://postgr.es/m/15677-0bf089579b4cd02d@postgresql.org
Discussion: https://postgr.es/m/19321.1554567786@sss.pgh.pa.us

5 years agodoc: Add note about generated columns in foreign tables
Peter Eisentraut [Mon, 8 Apr 2019 11:47:46 +0000 (13:47 +0200)]
doc: Add note about generated columns in foreign tables

Explain that it is not enforced that querying a generated column
returns data that is consistent with the data that was stored.  This
is similar to the note about constraints nearby.

Reported-by: Amit Langote <amitlangote09@gmail.com>
5 years agoAdd vacuum_truncate reloption.
Fujii Masao [Mon, 8 Apr 2019 07:43:57 +0000 (16:43 +0900)]
Add vacuum_truncate reloption.

vacuum_truncate controls whether vacuum tries to truncate off
any empty pages at the end of the table. Previously vacuum always
tried to do the truncation. However, the truncation could cause
some problems; for example, ACCESS EXCLUSIVE lock needs to
be taken on the table during the truncation and can cause
the query cancellation on the standby even if hot_standby_feedback
is true. Setting this reloption to false can be helpful to avoid
such problems.

Author: Tsunakawa Takayuki
Reviewed-By: Julien Rouhaud, Masahiko Sawada, Michael Paquier, Kirk Jamison and Fujii Masao
Discussion: https://postgr.es/m/CAHGQGwE5UqFqSq1=kV3QtTUtXphTdyHA-8rAj4A=Y+e4kyp3BQ@mail.gmail.com

5 years agoTweak wording of documentation for pg_checksums
Michael Paquier [Mon, 8 Apr 2019 06:30:45 +0000 (15:30 +0900)]
Tweak wording of documentation for pg_checksums

Author: Justin Pryzby
Discussion: https://postgr.es/m/20190329143210.GI5815@telsasoft.com

5 years agoReset memory context once per tuple in validateForeignKeyConstraint.
Andres Freund [Mon, 8 Apr 2019 05:42:42 +0000 (22:42 -0700)]
Reset memory context once per tuple in validateForeignKeyConstraint.

When using tableam ExecFetchSlotHeapTuple() might return a separately
allocated tuple. We could use the shouldFree argument to explicitly
free it, but it seems more robust to to protect

Also add a CHECK_FOR_INTERRUPTS() after each tuple. It's likely that
each AM has (heap does) a CFI somewhere in the relevant path, but it
seems more robust to have one in validateForeignKeyConstraint()
itself.

Note that this only affects the cases that couldn't be optimized to be
verified with a query.

Author: Andres Freund
Reviewed-By: Tom Lane (in an earlier version)
Discussion:
    https://postgr.es/m/19030.1554574075@sss.pgh.pa.us
    https://postgr.es/m/CAKJS1f_SHKcPYMsi39An5aUjhAcEMZb6Cx1Sj1QWEWSiKJkBVQ@mail.gmail.com
    https://postgr.es/m/20180711185628.mrvl46bjgk2uxoki@alap3.anarazel.de

5 years agoFix a number of issues around modifying a previously updated row.
Andres Freund [Mon, 8 Apr 2019 05:14:47 +0000 (22:14 -0700)]
Fix a number of issues around modifying a previously updated row.

This commit fixes three, unfortunately related, issues:

1) Since 5db6df0c01, the introduction of DML via tableam, it was
   possible to trigger "ERROR: unexpected table_lock_tuple status: 1"
   when updating a row that was previously updated in the same
   transaction - but only when the previously updated row was before
   updated in a concurrent transaction (and READ COMMITTED was
   used). The reason for that was that that case simply wasn't
   expected. Fixing that lead to:

2) Even before the above commit, there were error checks (introduced
   in 6868ed7491b7) preventing a row being updated by different
   commands within the same statement (say in a function called by an
   UPDATE) - but that check wasn't performed when the row was first
   updated in a concurrent transaction - instead the second update was
   silently skipped in that case. After this change we throw the same
   error as we'd without the concurrent transaction.

3) The error messages (introduced in 6868ed7491b7) preventing such
   updates emitted the same error message for both DELETE and
   UPDATE ("tuple to be updated was already modified by an operation
   triggered by the current command"). While that could be changed
   separately, it made it hard to write tests that verify the correct
   correct behavior of the code.

This commit changes heap's implementation of table_lock_tuple() to
return TM_SelfModified instead of TM_Invisible (previously loosely
modeled after EvalPlanQualFetch), and teaches nodeModifyTable.c to
handle that in response to table_lock_tuple() and not just in response
to table_(delete|update).

Additionally it fixes the wrong error message (see 3 above). The
comment for table_lock_tuple() is also adjusted to state that
TM_Deleted won't return information in TM_FailureData - it'll not
always be available.

This also adds tests to ensure that DELETE/UPDATE correctly error out
when affecting a row that concurrently was modified by another
transaction.

Author: Andres Freund
Reported-By: Tom Lane, when investigating a bug bug fix to another bug
    by Amit Langote
Discussion: https://postgr.es/m/19321.1554567786@sss.pgh.pa.us

5 years agoAdd more tests for partition tuple routing with dropped attributes
Michael Paquier [Mon, 8 Apr 2019 04:44:55 +0000 (13:44 +0900)]
Add more tests for partition tuple routing with dropped attributes

As bug #15733 has proved, we are lacking coverage for partition tuple
routing with dropped attributes when involving three levels of
partitioning or more.  There was only an active bug in this area for
v11, and HEAD is proving to handle those scenarios fine, still it lacked
some coverage for the previous problem.

Author: Amit Langote, Michael Paquier
Discussion: https://postgr.es/m/15733-7692379e310b80ec@postgresql.org

5 years agoAvoid fetching past the end of the indoption array.
Tom Lane [Sun, 7 Apr 2019 22:18:58 +0000 (18:18 -0400)]
Avoid fetching past the end of the indoption array.

pg_get_indexdef_worker carelessly fetched indoption entries even for
non-key index columns that don't have one.  99.999% of the time this
would be harmless, since the code wouldn't examine the value ... but
some fine day this will be a fetch off the end of memory, resulting
in SIGSEGV.

Detected through valgrind testing.  Odd that the buildfarm's valgrind
critters haven't noticed.

5 years agopsql \dP: list partitioned tables and indexes
Alvaro Herrera [Sun, 7 Apr 2019 11:59:12 +0000 (07:59 -0400)]
psql \dP: list partitioned tables and indexes

The new command lists partitioned relations (tables and/or indexes),
possibly with their sizes, possibly including partitioned partitions;
their parents (if not top-level); if indexes show the tables they belong
to; and their descriptions.

While there are various possible improvements to this, having it in this
form is already a great improvement over not having any way to obtain
this report.

Author: Pavel Stěhule, with help from Mathias Brossard, Amit Langote and
Justin Pryzby.
Reviewed-by: Amit Langote, Mathias Brossard, Melanie Plageman,
Michaël Paquier, Álvaro Herrera

5 years agoClean up side-effects of commits ab5fcf2b0 et al.
Tom Lane [Sun, 7 Apr 2019 16:54:22 +0000 (12:54 -0400)]
Clean up side-effects of commits ab5fcf2b0 et al.

Before those commits, partitioning-related code in the executor could
assume that ModifyTableState.resultRelInfo[] contains only leaf partitions.
However, now a fully-pruned update results in a dummy ModifyTable that
references the root partitioned table, and that breaks some stuff.

In v11, this led to an assertion or core dump in the tuple routing code.
Fix by disabling tuple routing, since we don't need that anyway.
(I chose to do that in HEAD as well for safety, even though the problem
doesn't manifest in HEAD as it stands.)

In v10, this confused ExecInitModifyTable's decision about whether it
needed to close the root table.  But we can get rid of that altogether
by being smarter about where to find the root table.

Note that since the referenced commits haven't shipped yet, this
isn't fixing any bug the field has seen.

Amit Langote, per a report from me

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

5 years agoReport progress of REINDEX operations
Peter Eisentraut [Sun, 7 Apr 2019 09:30:14 +0000 (11:30 +0200)]
Report progress of REINDEX operations

This uses the same infrastructure that the CREATE INDEX progress
reporting uses.  Add a column to pg_stat_progress_create_index to
report the OID of the index being worked on.  This was not necessary
for CREATE INDEX, but it's useful for REINDEX.

Also edit the phase descriptions a bit to be more consistent with the
source code comments.

Discussion: https://www.postgresql.org/message-id/ef6a6757-c36a-9e81-123f-13b19e36b7d7%402ndquadrant.com

5 years agoCast pg_stat_progress_cluster.cluster_index_relid to oid
Peter Eisentraut [Sun, 7 Apr 2019 08:31:32 +0000 (10:31 +0200)]
Cast pg_stat_progress_cluster.cluster_index_relid to oid

It's tracked internally as bigint, but when presented to the user it
should be oid.

5 years agoAvoid Python memory leaks in hstore_plpython and jsonb_plpython.
Tom Lane [Sat, 6 Apr 2019 21:54:29 +0000 (17:54 -0400)]
Avoid Python memory leaks in hstore_plpython and jsonb_plpython.

Fix some places where we might fail to do Py_DECREF() on a Python
object (thereby leaking it for the rest of the session).  Almost
all of the risks were in error-recovery paths, which we don't really
expect to hit anyway.  Hence, while this is definitely a bug fix,
it doesn't quite seem worth back-patching.

Nikita Glukhov, Michael Paquier, Tom Lane

Discussion: https://postgr.es/m/28053a7d-10d8-fc23-b05c-b4749c873f63@postgrespro.ru

5 years agoFix failures in validateForeignKeyConstraint's slow path.
Tom Lane [Sat, 6 Apr 2019 19:09:09 +0000 (15:09 -0400)]
Fix failures in validateForeignKeyConstraint's slow path.

The foreign-key-checking loop in ATRewriteTables failed to ignore
relations without storage (e.g., partitioned tables), unlike the
initial loop.  This accidentally worked as long as RI_Initial_Check
succeeded, which it does in most practical cases (including all the
ones exercised in the existing regression tests :-().  However, if
that failed, as for instance when there are permissions issues,
then we entered the slow fire-the-trigger-on-each-tuple path.
And that would try to read from the referencing relation, and fail
if it lacks storage.

A second problem, recently introduced in HEAD, was that this loop
had been broken by sloppy refactoring for the tableam API changes.

Repair both issues, and add a regression test case so we have some
coverage on this code path.  Back-patch as needed to v11.

(It looks like this code could do with additional bulletproofing,
but let's get a working test case in place first.)

Hadi Moshayedi, Tom Lane, Andres Freund

Discussion: https://postgr.es/m/CAK=1=WrnNmBbe5D9sm3t0a6dnAq3cdbF1vXY816j1wsMqzC8bw@mail.gmail.com
Discussion: https://postgr.es/m/19030.1554574075@sss.pgh.pa.us
Discussion: https://postgr.es/m/20190325180405.jytoehuzkeozggxx%40alap3.anarazel.de

5 years agoAdd support TCP user timeout in libpq and the backend server
Michael Paquier [Sat, 6 Apr 2019 06:23:37 +0000 (15:23 +0900)]
Add support TCP user timeout in libpq and the backend server

Similarly to the set of parameters for keepalive, a connection parameter
for libpq is added as well as a backend GUC, called tcp_user_timeout.

Increasing the TCP user timeout is useful to allow a connection to
survive extended periods without end-to-end connection, and decreasing
it allows application to fail faster.  By default, the parameter is 0,
which makes the connection use the system default, and follows a logic
close to the keepalive parameters in its handling.  When connecting
through a Unix-socket domain, the parameters have no effect.

Author: Ryohei Nagaura
Reviewed-by: Fabien Coelho, Robert Haas, Kyotaro Horiguchi, Kirk
Jamison, Mikalai Keida, Takayuki Tsunakawa, Andrei Yahorau
Discussion: https://postgr.es/m/EDA4195584F5064680D8130B1CA91C45367328@G01JPEXMBYT04

5 years agoUse Append rather than MergeAppend for scanning ordered partitions.
Tom Lane [Fri, 5 Apr 2019 23:20:30 +0000 (19:20 -0400)]
Use Append rather than MergeAppend for scanning ordered partitions.

If we need ordered output from a scan of a partitioned table, but
the ordering matches the partition ordering, then we don't need to
use a MergeAppend to combine the pre-ordered per-partition scan
results: a plain Append will produce the same results.  This
both saves useless comparison work inside the MergeAppend proper,
and allows us to start returning tuples after istarting up just
the first child node not all of them.

However, all is not peaches and cream, because if some of the
child nodes have high startup costs then there will be big
discontinuities in the tuples-returned-versus-elapsed-time curve.
The planner's cost model cannot handle that (yet, anyway).
If we model the Append's startup cost as being just the first
child's startup cost, we may drastically underestimate the cost
of fetching slightly more tuples than are available from the first
child.  Since we've had bad experiences with over-optimistic choices
of "fast start" plans for ORDER BY LIMIT queries, that seems scary.
As a klugy workaround, set the startup cost estimate for an ordered
Append to be the sum of its children's startup costs (as MergeAppend
would).  This doesn't really describe reality, but it's less likely
to cause a bad plan choice than an underestimated startup cost would.
In practice, the cases where we really care about this optimization
will have child plans that are IndexScans with zero startup cost,
so that the overly conservative estimate is still just zero.

David Rowley, reviewed by Julien Rouhaud and Antonin Houska

Discussion: https://postgr.es/m/CAKJS1f-hAqhPLRk_RaSFTgYxd=Tz5hA7kQ2h4-DhJufQk8TGuw@mail.gmail.com

5 years agoAdd facility to copy replication slots
Alvaro Herrera [Fri, 5 Apr 2019 17:52:45 +0000 (14:52 -0300)]
Add facility to copy replication slots

This allows the user to create duplicates of existing replication slots,
either logical or physical, and even changing properties such as whether
they are temporary or the output plugin used.

There are multiple uses for this, such as initializing multiple replicas
using the slot for one base backup; when doing investigation of logical
replication issues; and to select a different output plugins.

Author: Masahiko Sawada
Reviewed-by: Michael Paquier, Andres Freund, Petr Jelinek
Discussion: https://postgr.es/m/CAD21AoAm7XX8y_tOPP6j4Nzzch12FvA1wPqiO690RCk+uYVstg@mail.gmail.com

5 years agoWake up interested backends when a checkpoint fails.
Thomas Munro [Fri, 5 Apr 2019 20:31:48 +0000 (09:31 +1300)]
Wake up interested backends when a checkpoint fails.

Commit c6c9474a switched to condition variables instead of sleep
loops to notify backends of checkpoint start and stop, but forgot
to broadcast in case of checkpoint failure.

Author: Thomas Munro
Discussion: https://postgr.es/m/CA%2BhUKGJKbCd%2B_K%2BSEBsbHxVT60SG0ivWHHAdvL0bLTUt2xpA2w%40mail.gmail.com

5 years agoFix missing word.
Robert Haas [Fri, 5 Apr 2019 19:25:09 +0000 (15:25 -0400)]
Fix missing word.

Nathan Bossart

Discussion: http://postgr.es/m/2C63765B-AD31-4F6C-8DA7-C8544634C714@amazon.com

5 years agoEnsure consistent name matching behavior in processSQLNamePattern().
Tom Lane [Fri, 5 Apr 2019 16:59:46 +0000 (12:59 -0400)]
Ensure consistent name matching behavior in processSQLNamePattern().

Prior to v12, if you used a collation-sensitive regex feature in a
pattern handled by processSQLNamePattern() (for instance, \d '\\w+'
in psql), the behavior you got matched the database's default collation.
Since commit 586b98fdf you'd usually get C-collation behavior, because
the catalog "name"-type columns are now marked as COLLATE "C".  Add
explicit COLLATE specifications to restore the prior behavior.

(Note for whoever writes the v12 release notes: the need for this shows
that while 586b98fdf preserved pre-v12 behavior of "name" columns for
simple comparison operators, it changed the behavior of regex operators
on those columns.  Although this patch fixes it for pattern matches
generated by our own tools, user-written queries will still be affected.
So we'd better mention this issue as a compatibility item.)

Daniel Vérité

Discussion: https://postgr.es/m/701e51f0-0ec0-4e70-a365-1958d66dd8d2@manitou-mail.org

5 years agotable: docs: fix typos and grammar.
Andres Freund [Fri, 5 Apr 2019 16:45:59 +0000 (09:45 -0700)]
table: docs: fix typos and grammar.

Author: Justin Pryzby
Discussion: https://postgr.es/m/20190404055138.GA24864@telsasoft.com

5 years agoDoc: Update documentation on partitioning vs. foreign tables.
Etsuro Fujita [Fri, 5 Apr 2019 11:55:06 +0000 (20:55 +0900)]
Doc: Update documentation on partitioning vs. foreign tables.

The limitations that it is not allowed to create/attach a foreign table
as a partition of an indexed partitioned table were not documented.

Reported-By: Stepan Yankevych
Author: Etsuro Fujita
Reviewed-By: Amit Langote
Backpatch-through: 11 where partitioned index was introduced
Discussion: https://postgr.es/m/1553869152.858391073.5f8m3n0x@frv53.fwdcdn.com

5 years agoFix compiler warning
Peter Eisentraut [Fri, 5 Apr 2019 07:23:07 +0000 (09:23 +0200)]
Fix compiler warning

Rewrite get_attgenerated() to avoid compiler warning if the compiler
does not recognize that elog(ERROR) does not return.

Reported-by: David Rowley <david.rowley@2ndquadrant.com>
5 years agoRevert "Consistently test for in-use shared memory."
Noah Misch [Fri, 5 Apr 2019 07:00:52 +0000 (00:00 -0700)]
Revert "Consistently test for in-use shared memory."

This reverts commits 2f932f71d9f2963bbd201129d7b971c8f5f077fd,
16ee6eaf80a40007a138b60bb5661660058d0422 and
6f0e190056fe441f7cf788ff19b62b13c94f68f3.  The buildfarm has revealed
several bugs.  Back-patch like the original commits.

Discussion: https://postgr.es/m/20190404145319.GA1720877@rfd.leadboat.com

5 years agoFix bugs in mdsyncfiletag().
Thomas Munro [Fri, 5 Apr 2019 03:39:47 +0000 (16:39 +1300)]
Fix bugs in mdsyncfiletag().

Commit 3eb77eba moved a _mdfd_getseg() call from mdsync() into a new
callback function mdsyncfiletag(), but didn't get the arguments quite
right.  Without the EXTENSION_DONT_CHECK_SIZE flag we fail to open a
segment if lower-numbered segments have been truncated, and it wants
a block number rather than a segment number.

While comparing with the older coding, also remove an unnecessary
clobbering of errno, and adjust the code in mdunlinkfiletag() to
ressemble the original code from mdpostckpt() more closely instead
of using an unnecessary call to smgropen().

Author: Thomas Munro
Discussion: https://postgr.es/m/CA%2BhUKGL%2BYLUOA0eYiBXBfwW%2BbH5kFgh94%3DgQH0jHEJ-t5Y91wQ%40mail.gmail.com

5 years agoHandle errors during GSSAPI startup better
Stephen Frost [Fri, 5 Apr 2019 02:52:42 +0000 (22:52 -0400)]
Handle errors during GSSAPI startup better

There was some confusion over the format of the error message returned
from the server during GSSAPI startup; specifically, it was expected
that a length would be returned when, in reality, at this early stage in
the startup sequence, no length is returned from the server as part of
an error message.

Correct the client-side code for dealing with error messages sent by the
server during startup by simply reading what's available into our
buffer, after we've discovered it's an error message, and then reporting
back what was returned.

In passing, also add in documentation of the environment variable
PGGSSENCMODE which was missed previously, and adjust the code to look
for the PGGSSENCMODE variable (the environment variable change was
missed in the prior GSSMODE -> GSSENCMODE commit).

Error-handling issue discovered by Peter Eisentraut, the rest were items
discovered during testing of the error handling.

5 years agoFix some documentation in pg_rewind
Michael Paquier [Fri, 5 Apr 2019 01:37:59 +0000 (10:37 +0900)]
Fix some documentation in pg_rewind

Since 11, it is possible to use a non-superuser role when using an
online source cluster with pg_rewind as long as the role has proper
permissions to execute on the source all the functions used by
pg_rewind, and the documentation stated that a superuser is necessary.
Let's add at the same time all the details needed to create such a
role.

A second confusion which comes a lot from users is that it is necessary
to issue a checkpoint on a freshly-promoted standby so as its control
file has up-to-date timeline information which is used by pg_rewind to
validate the operation.  Let's document that properly.  This is
back-patched down to 9.5 where pg_rewind has been introduced.

Author: Michael Paquier
Reviewed-by: Magnus Hagander
Discussion: https://postgr.es/m/CABUevEz5bpvbwVsYCaSMV80CBZ5-82nkMzbb+Bu=h1m=rLdn=g@mail.gmail.com
Backpatch-through: 9.5

5 years agoRemove unused struct member, enforce multi_insert callback presence.
Andres Freund [Fri, 5 Apr 2019 00:36:45 +0000 (17:36 -0700)]
Remove unused struct member, enforce multi_insert callback presence.

Author: David Rowley, Andres Freund
Discussion: https://postgr.es/m/CAKJS1f9=9phmm66diAji4gvHnWSrK7BGFoNct+mEUT_c8pPOjw@mail.gmail.com

5 years agoHarden tableam against nonexistant / wrong kind of AMs.
Andres Freund [Fri, 5 Apr 2019 00:17:50 +0000 (17:17 -0700)]
Harden tableam against nonexistant / wrong kind of AMs.

Previously it was allowed to set default_table_access_method to an
empty string. That makes sense for default_tablespace, where that was
copied from, as it signals falling back to the database's default
tablespace. As there is no equivalent for table AMs, forbid that.

Also make sure to throw a usable error when creating a table using an
index AM, by using get_am_type_oid() to implement get_table_am_oid()
instead of a separate copy. Previously we'd error out only later, in
GetTableAmRoutine().

Thirdly remove GetTableAmRoutineByAmId() - it was only used in an
earlier version of 8586bf7ed8.

Add tests for the above (some for index AMs as well).

5 years agoAdd test coverage for rootdescend verification.
Peter Geoghegan [Fri, 5 Apr 2019 00:25:35 +0000 (17:25 -0700)]
Add test coverage for rootdescend verification.

Commit c1afd175, which added support for rootdescend verification to
amcheck, added only minimal regression test coverage.  Address this by
making sure that rootdescend verification is run on a multi-level index.
In passing, simplify some of the regression tests that exercise
multi-level nbtree page deletion.

Both issues spotted while rereviewing coverage of the nbtree patch
series using gcov.

5 years agotableam: Add table_multi_insert() and revamp/speed-up COPY FROM buffering.
Andres Freund [Thu, 4 Apr 2019 22:47:19 +0000 (15:47 -0700)]
tableam: Add table_multi_insert() and revamp/speed-up COPY FROM buffering.

This adds table_multi_insert(), and converts COPY FROM, the only user
of heap_multi_insert, to it.

A simple conversion of COPY FROM use slots would have yielded a
slowdown when inserting into a partitioned table for some
workloads. Different partitions might need different slots (both slot
types and their descriptors), and dropping / creating slots when
there's constant partition changes is measurable.

Thus instead revamp the COPY FROM buffering for partitioned tables to
allow to buffer inserts into multiple tables, flushing only when
limits are reached across all partition buffers. By only dropping
slots when there've been inserts into too many different partitions,
the aforementioned overhead is gone. By allowing larger batches, even
when there are frequent partition changes, we actuall speed such cases
up significantly.

By using slots COPY of very narrow rows into unlogged / temporary
might slow down very slightly (due to the indirect function calls).

Author: David Rowley, Andres Freund, Haribabu Kommi
Discussion:
    https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
    https://postgr.es/m/20190327054923.t3epfuewxfqdt22e@alap3.anarazel.de

5 years agoAdd a "SQLSTATE-only" error verbosity option to libpq and psql.
Tom Lane [Thu, 4 Apr 2019 21:22:02 +0000 (17:22 -0400)]
Add a "SQLSTATE-only" error verbosity option to libpq and psql.

This is intended for use mostly in test scripts for external tools,
which could do without cross-PG-version variations in error message
wording.  Of course, the SQLSTATE isn't guaranteed stable either, but
it should be more so than the error message text.

Note: there's a bit of an ABI change for libpq here, but it seems
OK because if somebody compiles against a newer version of libpq-fe.h,
and then tries to pass PQERRORS_SQLSTATE to PQsetErrorVerbosity()
of an older libpq library, it will be accepted and then act like
PQERRORS_DEFAULT, thanks to the way the tests in pqBuildErrorMessage3
have historically been phrased.  That seems acceptable.

Didier Gautheron, reviewed by Dagfinn Ilmari Mannsåker

Discussion: https://postgr.es/m/CAJRYxuKyj4zA+JGVrtx8OWAuBfE-_wN4sUMK4H49EuPed=mOBw@mail.gmail.com

5 years agopg_restore: Require "-f -" to mean stdout
Alvaro Herrera [Thu, 4 Apr 2019 19:34:58 +0000 (16:34 -0300)]
pg_restore: Require "-f -" to mean stdout

The previous convention that stdout was selected by default when nothing
is specified was just too error-prone.

After a suggestion from Andrew Gierth.
Author: Euler Taveira
Reviewed-by: Yoshikazu Imai, José Arthur Benetasso Villanova
Discussion: https://postgr.es/m/87sgwrmhdv.fsf@news-spur.riddles.org.uk

5 years agoMake queries' locking of indexes more consistent.
Tom Lane [Thu, 4 Apr 2019 19:12:51 +0000 (15:12 -0400)]
Make queries' locking of indexes more consistent.

The assertions added by commit b04aeb0a0 exposed that there are some
code paths wherein the executor will try to open an index without
holding any lock on it.  We do have some lock on the index's table,
so it seems likely that there's no fatal problem with this (for
instance, the index couldn't get dropped from under us).  Still,
it's bad practice and we should fix it.

To do so, remove the optimizations in ExecInitIndexScan and friends
that tried to avoid taking a lock on an index belonging to a target
relation, and just take the lock always.  In non-bug cases, this
will result in no additional shared-memory access, since we'll find
in the local lock table that we already have a lock of the desired
type; hence, no significant performance degradation should occur.

Also, adjust the planner and executor so that the type of lock taken
on an index is always identical to the type of lock taken for its table,
by relying on the recently added RangeTblEntry.rellockmode field.
This avoids some corner cases where that might not have been true
before (possibly resulting in extra locking overhead), and prevents
future maintenance issues from having multiple bits of logic that
all needed to be in sync.  In addition, this change removes all core
calls to ExecRelationIsTargetRelation, which avoids a possible O(N^2)
startup penalty for queries with large numbers of target relations.
(We'd probably remove that function altogether, were it not that we
advertise it as something that FDWs might want to use.)

Also adjust some places in selfuncs.c to not take any lock on indexes
they are transiently opening, since we can assume that plancat.c
did that already.

In passing, change gin_clean_pending_list() to take RowExclusiveLock
not AccessShareLock on its target index.  Although it's not clear that
that's actually a bug, it seemed very strange for a function that's
explicitly going to modify the index to use only AccessShareLock.

David Rowley, reviewed by Julien Rouhaud and Amit Langote,
a bit of further tweaking by me

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

5 years agoAllow VACUUM to be run with index cleanup disabled.
Robert Haas [Thu, 4 Apr 2019 18:58:53 +0000 (14:58 -0400)]
Allow VACUUM to be run with index cleanup disabled.

This commit adds a new reloption, vacuum_index_cleanup, which
controls whether index cleanup is performed for a particular
relation by default.  It also adds a new option to the VACUUM
command, INDEX_CLEANUP, which can be used to override the
reloption.  If neither the reloption nor the VACUUM option is
used, the default is true, as before.

Masahiko Sawada, reviewed and tested by Nathan Bossart, Alvaro
Herrera, Kyotaro Horiguchi, Darafei Praliaskouski, and me.
The wording of the documentation is mostly due to me.

Discussion: http://postgr.es/m/CAD21AoAt5R3DNUZSjOoXDUY=naYPUOuffVsRzuTYMz29yLzQCA@mail.gmail.com

5 years agoInvalidate binary search bounds consistently.
Peter Geoghegan [Thu, 4 Apr 2019 16:38:08 +0000 (09:38 -0700)]
Invalidate binary search bounds consistently.

_bt_check_unique() failed to invalidate binary search bounds in the
event of a live conflict following commit e5adcb78.  This resulted in
problems after waiting for the conflicting xact to commit or abort.  The
subsequent call to _bt_check_unique() would restore the initial binary
search bounds, rather than starting a new search.  Fix by explicitly
invalidating bounds when it becomes clear that there is a live conflict
that insertion will have to wait to resolve.

Ashutosh Sharma, with a few additional tweaks by me.

Author: Ashutosh Sharma
Reported-By: Ashutosh Sharma
Diagnosed-By: Ashutosh Sharma
Discussion: https://postgr.es/m/CAE9k0PnQp-qr-UYKMSCzdC2FBzdE4wKP41hZrZvvP26dKLonLg@mail.gmail.com

5 years agoMove the be_gssapi_get_* prototypes
Stephen Frost [Thu, 4 Apr 2019 15:11:46 +0000 (11:11 -0400)]
Move the be_gssapi_get_* prototypes

The be_gssapi_get_* prototypes were put close to similar ones for SSL-
but a bit too close since that meant they ended up only being included
for SSL-enabled builds.  Move those to be under ENABLE_GSS instead.

Pointed out by Tom.

5 years agoRefactor the fsync queue for wider use.
Thomas Munro [Thu, 4 Apr 2019 08:56:03 +0000 (21:56 +1300)]
Refactor the fsync queue for wider use.

Previously, md.c and checkpointer.c were tightly integrated so that
fsync calls could be handed off and processed in the background.
Introduce a system of callbacks and file tags, so that other modules
can hand off fsync work in the same way.

For now only md.c uses the new interface, but other users are being
proposed.  Since there may be use cases that are not strictly SMGR
implementations, use a new function table for sync handlers rather
than extending the traditional SMGR one.

Instead of using a bitmapset of segment numbers for each RelFileNode
in the checkpointer's hash table, make the segment number part of the
key.  This requires sending explicit "forget" requests for every
segment individually when relations are dropped, but suits the file
layout schemes of proposed future users better (ie sparse or high
segment numbers).

Author: Shawn Debnath and Thomas Munro
Reviewed-by: Thomas Munro, Andres Freund
Discussion: https://postgr.es/m/CAEepm=2gTANm=e3ARnJT=n0h8hf88wqmaZxk0JYkxw+b21fNrw@mail.gmail.com

5 years agofile_fdw: Fix for generated columns
Peter Eisentraut [Thu, 4 Apr 2019 07:24:48 +0000 (09:24 +0200)]
file_fdw: Fix for generated columns

Since file_fdw uses COPY internally, but COPY doesn't allow listing
generated columns in its column list, we need to make sure that we
don't add generated columns to the column lists internally generated
by file_fdw.

Reported-by: Erik Rijkers <er@xs4all.nl>
5 years agoSilence -Wimplicit-fallthrough in sysv_shmem.c.
Noah Misch [Thu, 4 Apr 2019 06:23:35 +0000 (23:23 -0700)]
Silence -Wimplicit-fallthrough in sysv_shmem.c.

Commit 2f932f71d9f2963bbd201129d7b971c8f5f077fd added code that elicits
a warning on buildfarm member flaviventris.  Back-patch to 9.4, like
that commit.

Reported by Andres Freund.

Discussion: https://postgr.es/m/20190404020057.galelv7by75ekqrh@alap3.anarazel.de

5 years agoMake src/test/recovery/t/017_shm.pl safe for concurrent execution.
Noah Misch [Thu, 4 Apr 2019 06:16:37 +0000 (23:16 -0700)]
Make src/test/recovery/t/017_shm.pl safe for concurrent execution.

Buildfarm members idiacanthus and komodoensis, which share a host, both
executed this test in the same second.  That failed.  Back-patch to 9.6,
where the test first appeared.

Discussion: https://postgr.es/m/20190404020543.GA1319573@rfd.leadboat.com

5 years agoImprove readability of some tests in strings.sql
Michael Paquier [Thu, 4 Apr 2019 01:24:56 +0000 (10:24 +0900)]
Improve readability of some tests in strings.sql

c251336 has added some tests to check if a toast relation should be
empty or not, hardcoding the toast relation name when calling
pg_relation_size().  pg_class.reltoastrelid offers the same information,
so simplify the tests to use that.

Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/20190403065949.GH3298@paquier.xyz

5 years agotableam: basic documentation.
Andres Freund [Thu, 4 Apr 2019 00:37:00 +0000 (17:37 -0700)]
tableam: basic documentation.

This adds documentation about the user oriented parts of table access
methods (i.e. the default_table_access_method GUC and the USING clause
for CREATE TABLE etc), adds a basic chapter about the table access
method interface, and adds a note to storage.sgml that it's contents
don't necessarily apply for non-builtin AMs.

Author: Haribabu Kommi and Andres Freund
Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de

5 years agoAssert that pgwin32_signal_initialize() has been called early enough.
Noah Misch [Thu, 4 Apr 2019 00:11:16 +0000 (17:11 -0700)]
Assert that pgwin32_signal_initialize() has been called early enough.

Before the pgwin32_signal_initialize() call, the backend version of
pg_usleep() has no effect.  No in-tree code falls afoul of that today,
but temporary commit 23078689a9921968ac0873b017be6e7f772f10bc did so.

Discussion: https://postgr.es/m/20190402135442.GA1173872@rfd.leadboat.com

5 years agoHandle USE_MODULE_DB for all tests able to use an installed postmaster.
Noah Misch [Thu, 4 Apr 2019 00:06:01 +0000 (17:06 -0700)]
Handle USE_MODULE_DB for all tests able to use an installed postmaster.

When $(MODULES) and $(MODULE_big) are empty, derive the database name
from the first element of $(REGRESS) instead of using a constant string.
When deriving the database name from $(MODULES), use its first element
instead of the entire list; the earlier approach would fail if any
multi-module directory had $(REGRESS) tests.  Treat isolation suites and
src/pl correspondingly.  Under USE_MODULE_DB=1, installcheck-world and
check-world no longer reuse any database name in a given postmaster.
Buildfarm members axolotl, mandrill and frogfish saw spurious "is being
accessed by other users" failures that would not have happened without
database name reuse.  (The CountOtherDBBackends() 5s deadline expired
during DROP DATABASE; a backend for an earlier test suite had used the
same database name and had not yet exited.)  Back-patch to 9.4 (all
supported versions), except bits pertaining to isolation suites.

Concept reviewed by Andrew Dunstan, Andres Freund and Tom Lane.

Discussion: https://postgr.es/m/20190401135213.GE891537@rfd.leadboat.com

5 years agoConsistently test for in-use shared memory.
Noah Misch [Thu, 4 Apr 2019 00:03:46 +0000 (17:03 -0700)]
Consistently test for in-use shared memory.

postmaster startup scrutinizes any shared memory segment recorded in
postmaster.pid, exiting if that segment matches the current data
directory and has an attached process.  When the postmaster.pid file was
missing, a starting postmaster used weaker checks.  Change to use the
same checks in both scenarios.  This increases the chance of a startup
failure, in lieu of data corruption, if the DBA does "kill -9 `head -n1
postmaster.pid` && rm postmaster.pid && pg_ctl -w start".  A postmaster
will no longer recycle segments pertaining to other data directories.
That's good for production, but it's bad for integration tests that
crash a postmaster and immediately delete its data directory.  Such a
test now leaks a segment indefinitely.  No "make check-world" test does
that.  win32_shmem.c already avoided all these problems.  In 9.6 and
later, enhance PostgresNode to facilitate testing.  Back-patch to 9.4
(all supported versions).

Reviewed by Daniel Gustafsson and Kyotaro HORIGUCHI.

Discussion: https://postgr.es/m/20130911033341.GD225735@tornado.leadboat.com

5 years agoDoc: clarify partial-index example.
Tom Lane [Wed, 3 Apr 2019 22:28:18 +0000 (18:28 -0400)]
Doc: clarify partial-index example.

Jonathan Katz

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

5 years agoAdd SETTINGS option to EXPLAIN, to print modified settings.
Tomas Vondra [Wed, 3 Apr 2019 22:04:31 +0000 (00:04 +0200)]
Add SETTINGS option to EXPLAIN, to print modified settings.

Query planning is affected by a number of configuration options, and it
may be crucial to know which of those options were set to non-default
values.  With this patch you can say EXPLAIN (SETTINGS ON) to include
that information in the query plan.  Only options affecting planning,
with values different from the built-in default are printed.

This patch also adds auto_explain.log_settings option, providing the
same capability in auto_explain module.

Author: Tomas Vondra
Reviewed-by: Rafia Sabih, John Naylor
Discussion: https://postgr.es/m/e1791b4c-df9c-be02-edc5-7c8874944be0@2ndquadrant.com

5 years agoTweak docs for log_statement_sample_rate
Alvaro Herrera [Wed, 3 Apr 2019 21:54:02 +0000 (18:54 -0300)]
Tweak docs for log_statement_sample_rate

Author: Justin Pryzby, partly after a suggestion from Masahiko Sawada
Discussion: https://postgr.es/m/20190328135918.GA27808@telsasoft.com
Discussion: https://postgr.es/m/CAD21AoB9+y8N4+Fan-ne-_7J5yTybPttxeVKfwUocKp4zT1vNQ@mail.gmail.com

5 years agoLog all statements from a sample of transactions
Alvaro Herrera [Wed, 3 Apr 2019 21:43:59 +0000 (18:43 -0300)]
Log all statements from a sample of transactions

This is useful to obtain a view of the different transaction types in an
application, regardless of the durations of the statements each runs.

Author: Adrien Nayrat
Reviewed-by: Masahiko Sawada, Hayato Kuroda, Andres Freund
5 years agoRemove now-unnecessary thread pointer arguments in pgbench.
Tom Lane [Wed, 3 Apr 2019 21:16:00 +0000 (17:16 -0400)]
Remove now-unnecessary thread pointer arguments in pgbench.

Not required after nuking the zipfian thread-local cache.

Also add a comment about hazardous pointer punning in threadRun(),
and avoid using "thread" to refer to the threads array as a whole.

Fabien Coelho and Tom Lane, per suggestion from Alvaro Herrera

Discussion: https://postgr.es/m/alpine.DEB.2.21.1904032126060.7997@lancre

5 years agoReduce overhead of pg_mcv_list (de)serialization
Tomas Vondra [Wed, 3 Apr 2019 19:08:36 +0000 (21:08 +0200)]
Reduce overhead of pg_mcv_list (de)serialization

Commit ea4e1c0e8f resolved issues with memory alignment in serialized
pg_mcv_list values, but it required copying data to/from the varlena
buffer during serialization and deserialization.  As the MCV lits may
be fairly large, the overhead (memory consumption, CPU usage) can get
rather significant too.

This change tweaks the serialization format so that the alignment is
correct with respect to the varlena value, and so the parts may be
accessed directly without copying the data.

Catversion bump, as it affects existing pg_statistic_ext data.

5 years agoGSSAPI encryption support
Stephen Frost [Wed, 3 Apr 2019 19:02:33 +0000 (15:02 -0400)]
GSSAPI encryption support

On both the frontend and backend, prepare for GSSAPI encryption
support by moving common code for error handling into a separate file.
Fix a TODO for handling multiple status messages in the process.
Eliminate the OIDs, which have not been needed for some time.

Add frontend and backend encryption support functions.  Keep the
context initiation for authentication-only separate on both the
frontend and backend in order to avoid concerns about changing the
requested flags to include encryption support.

In postmaster, pull GSSAPI authorization checking into a shared
function.  Also share the initiator name between the encryption and
non-encryption codepaths.

For HBA, add "hostgssenc" and "hostnogssenc" entries that behave
similarly to their SSL counterparts.  "hostgssenc" requires either
"gss", "trust", or "reject" for its authentication.

Similarly, add a "gssencmode" parameter to libpq.  Supported values are
"disable", "require", and "prefer".  Notably, negotiation will only be
attempted if credentials can be acquired.  Move credential acquisition
into its own function to support this behavior.

Add a simple pg_stat_gssapi view similar to pg_stat_ssl, for monitoring
if GSSAPI authentication was used, what principal was used, and if
encryption is being used on the connection.

Finally, add documentation for everything new, and update existing
documentation on connection security.

Thanks to Michael Paquier for the Windows fixes.

Author: Robbie Harwood, with changes to the read/write functions by me.
Reviewed in various forms and at different times by: Michael Paquier,
   Andres Freund, David Steele.
Discussion: https://www.postgresql.org/message-id/flat/jlg1tgq1ktm.fsf@thriss.redhat.com

5 years agoCopy name when cloning FKs recurses to partitions
Alvaro Herrera [Wed, 3 Apr 2019 18:32:53 +0000 (15:32 -0300)]
Copy name when cloning FKs recurses to partitions

We were passing a string owned by a syscache entry, which was released
before recursing.  Fix by pstrdup'ing the string.

Per buildfarm member prion.

5 years agoSupport foreign keys that reference partitioned tables
Alvaro Herrera [Wed, 3 Apr 2019 17:38:20 +0000 (14:38 -0300)]
Support foreign keys that reference partitioned tables

Previously, while primary keys could be made on partitioned tables, it
was not possible to define foreign keys that reference those primary
keys.  Now it is possible to do that.

Author: Álvaro Herrera
Reviewed-by: Amit Langote, Jesper Pedersen
Discussion: https://postgr.es/m/20181102234158.735b3fevta63msbj@alvherre.pgsql

5 years agoGenerate less WAL during GiST, GIN and SP-GiST index build.
Heikki Linnakangas [Wed, 3 Apr 2019 14:03:15 +0000 (17:03 +0300)]
Generate less WAL during GiST, GIN and SP-GiST index build.

Instead of WAL-logging every modification during the build separately,
first build the index without any WAL-logging, and make a separate pass
through the index at the end, to write all pages to the WAL. This
significantly reduces the amount of WAL generated, and is usually also
faster, despite the extra I/O needed for the extra scan through the index.
WAL generated this way is also faster to replay.

For GiST, the LSN-NSN interlock makes this a little tricky. All pages must
be marked with a valid (i.e. non-zero) LSN, so that the parent-child
LSN-NSN interlock works correctly. We now use magic value 1 for that during
index build. Change the fake LSN counter to begin from 1000, so that 1 is
safely smaller than any real or fake LSN. 2 would've been enough for our
purposes, but let's reserve a bigger range, in case we need more special
values in the future.

Author: Anastasia Lubennikova, Andrey V. Lepikhov
Reviewed-by: Heikki Linnakangas, Dmitry Dolgov
5 years agoCorrectly initialize newly added struct member
Alvaro Herrera [Wed, 3 Apr 2019 12:56:20 +0000 (09:56 -0300)]
Correctly initialize newly added struct member

Valgrind was rightly complaining that IndexVacuumInfo->report_progress
(added by commit ab0dfc961b6a) was not being initialized in some code
paths.  Repair.

Per buildfarm member lousyjack.

5 years agoPrevent use of uninitialized variable
Alvaro Herrera [Tue, 2 Apr 2019 19:03:26 +0000 (16:03 -0300)]
Prevent use of uninitialized variable

Per buildfarm member longfin.

5 years agoUpdate expected output for modified catalog definition
Alvaro Herrera [Tue, 2 Apr 2019 18:43:32 +0000 (15:43 -0300)]
Update expected output for modified catalog definition

Pilot error in previous commit

5 years agoReport progress of CREATE INDEX operations
Alvaro Herrera [Tue, 2 Apr 2019 18:18:08 +0000 (15:18 -0300)]
Report progress of CREATE INDEX operations

This uses the progress reporting infrastructure added by c16dc1aca5e0,
adding support for CREATE INDEX and CREATE INDEX CONCURRENTLY.

There are two pieces to this: one is index-AM-agnostic, and the other is
AM-specific.  The latter is fairly elaborate for btrees, including
reportage for parallel index builds and the separate phases that btree
index creation uses; other index AMs, which are much simpler in their
building procedures, have simplistic reporting only, but that seems
sufficient, at least for non-concurrent builds.

The index-AM-agnostic part is fairly complete, providing insight into
the CONCURRENTLY wait phases as well as block-based progress during the
index validation table scan.  (The index validation index scan requires
patching each AM, which has not been included here.)

Reviewers: Rahila Syed, Pavan Deolasee, Tatsuro Yamada
Discussion: https://postgr.es/m/20181220220022.mg63bhk26zdpvmcj@alvherre.pgsql