users/gsingh/postgres.git
5 years agoRefactor query cancellation code into src/fe_utils/
Michael Paquier [Mon, 2 Dec 2019 02:18:56 +0000 (11:18 +0900)]
Refactor query cancellation code into src/fe_utils/

Originally, this code was duplicated in src/bin/psql/ and
src/bin/scripts/, but it can be useful for other frontend applications,
like pgbench.  This refactoring offers the possibility to setup a custom
callback which would get called in the signal handler for SIGINT or when
the interruption console events happen on Windows.

Author: Fabien Coelho, with contributions from Michael Paquier
Reviewed-by: Álvaro Herrera, Ibrar Ahmed
Discussion: https://postgr.es/m/alpine.DEB.2.21.1910311939430.27369@lancre

5 years agoAdd dummy versions of new SSL functions for non-SSL builds
Andrew Dunstan [Sun, 1 Dec 2019 22:49:43 +0000 (17:49 -0500)]
Add dummy versions of new SSL functions for non-SSL builds

This rectifies an oversight in commit 4dc6355210, which caused certain
builds to fail, especially on Windows.

5 years agoFix misbehavior with expression indexes on ON COMMIT DELETE ROWS tables.
Tom Lane [Sun, 1 Dec 2019 18:09:26 +0000 (13:09 -0500)]
Fix misbehavior with expression indexes on ON COMMIT DELETE ROWS tables.

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

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

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

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

5 years agolibq support for sslpassword connection param, DER format keys
Andrew Dunstan [Sat, 30 Nov 2019 20:27:13 +0000 (15:27 -0500)]
libq support for sslpassword connection param,  DER format keys

This patch providies for support for password protected SSL client
keys in libpq, and for DER format keys, both encrypted and unencrypted.
There is a new connection parameter sslpassword, which is supplied to
the OpenSSL libraries via a callback function. The callback function can
also be set by an application by calling PQgetSSLKeyPassHook(). There is
also a function to retreive the connection setting, PQsslpassword().

Craig Ringer and Andrew Dunstan

Reviewed by: Greg Nancarrow

Discussion: https://postgr.es/m/f7ee88ed-95c4-95c1-d4bf-7b415363ab62@2ndQuadrant.com

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

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

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

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

5 years agoRevert commits 290acac92b and 8a7e9e9dad.
Amit Kapila [Sat, 30 Nov 2019 02:13:42 +0000 (07:43 +0530)]
Revert commits 290acac92b and 8a7e9e9dad.

This commit revert the commits to add a test case that tests the 'force'
option when there is an active backend connected to the database being
dropped.

This feature internally sends SIGTERM to all the backends connected to the
database being dropped and then the same is reported to the client.  We
found that on Windows, the client end of the socket is not able to read
the data once we close the socket in the server which leads to loss of
error message which is not what we expect.  We also observed  similar
behavior in other cases like pg_terminate_backend(),
pg_ctl kill TERM <pid>.  There are probably a few others like that.  The
fix for this requires further study.

Discussion: https://postgr.es/m/E1iaD8h-0004us-K9@gemulon.postgresql.org

5 years agoSmall code simplification
Peter Eisentraut [Fri, 29 Nov 2019 09:55:31 +0000 (10:55 +0100)]
Small code simplification

FLOAT8PASSBYVAL can be used instead of USE_FLOAT8_BYVAL here.

5 years agoAdd a regression test for allow_system_table_mods
Peter Eisentraut [Fri, 29 Nov 2019 09:04:45 +0000 (10:04 +0100)]
Add a regression test for allow_system_table_mods

Add a regression test file that exercises the kinds of commands that
allow_system_table_mods allows.

This is put in the "unsafe_tests" suite, so it won't accidentally
create a mess if someone runs the normal regression tests against an
instance that they care about.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/8b00ea5e-28a7-88ba-e848-21528b632354%402ndquadrant.com

5 years agoMake allow_system_table_mods settable at run time
Peter Eisentraut [Fri, 29 Nov 2019 09:04:45 +0000 (10:04 +0100)]
Make allow_system_table_mods settable at run time

Make allow_system_table_mods settable at run time by superusers.  It
was previously postmaster start only.

We don't want to make system catalog DDL wide-open, but there are
occasionally useful things to do like setting reloptions or statistics
on a busy system table, and blocking those doesn't help anyone.  Also,
this enables the possibility of writing a test suite for this setting.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/8b00ea5e-28a7-88ba-e848-21528b632354%402ndquadrant.com

5 years agoRemove any-user DML capability from allow_system_table_mods
Peter Eisentraut [Fri, 29 Nov 2019 09:04:45 +0000 (10:04 +0100)]
Remove any-user DML capability from allow_system_table_mods

Previously, allow_system_table_mods allowed a non-superuser to do DML
on a system table without further permission checks.  This has been
removed, as it was quite inconsistent with the rest of the meaning of
this setting.  (Since allow_system_table_mods was previously only
accessible with a server restart, it is unlikely that anyone was using
this possibility.)

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/8b00ea5e-28a7-88ba-e848-21528b632354%402ndquadrant.com

5 years agoAdd error position to an error message
Peter Eisentraut [Fri, 29 Nov 2019 08:10:17 +0000 (09:10 +0100)]
Add error position to an error message

Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/6e7aa4a1-be6a-1a75-b1f9-83a678e5184a@2ndquadrant.com

5 years agoUse memcpy instead of a byte loop in pglz_decompress
Tomas Vondra [Thu, 28 Nov 2019 22:29:30 +0000 (23:29 +0100)]
Use memcpy instead of a byte loop in pglz_decompress

The byte loop used in pglz_decompress() because of possible overlap may
be quite inefficient, so this commit replaces it with memcpy. The gains
do depend on the data (compressibility) and hardware, but seem to be
quite significant.

Author: Andrey Borodin
Reviewed-by: Michael Paquier, Konstantin Knizhnik, Tels
Discussion: https://postgr.es/m/469C9ED9-348C-4FE7-A7A7-B0FA671BEE4C@yandex-team.ru

5 years agoRemove unnecessary clauses_attnums variable
Tomas Vondra [Thu, 28 Nov 2019 22:25:14 +0000 (23:25 +0100)]
Remove unnecessary clauses_attnums variable

Commit c676e659b2 reworked how choose_best_statistics() picks the best
extended statistics, but failed to remove clauses_attnums which is now
unnecessary. So get rid of it and backpatch to 12, same as c676e659b2.

Author: Tomas Vondra
Discussion: https://postgr.es/m/CA+u7OA7H5rcE2=8f263w4NZD6ipO_XOrYB816nuLXbmSTH9pQQ@mail.gmail.com
Backpatch-through: 12

5 years agoFix choose_best_statistics to check clauses individually
Tomas Vondra [Thu, 28 Nov 2019 21:20:28 +0000 (22:20 +0100)]
Fix choose_best_statistics to check clauses individually

When picking the best extended statistics object for a list of clauses,
it's not enough to look at attnums extracted from the clause list as a
whole. Consider for example this query with OR clauses:

   SELECT * FROM t WHERE (t.a = 1) OR (t.b = 1) OR (t.c = 1)

with a statistics defined on columns (a,b). Relying on attnums extracted
from the whole OR clause, we'd consider the statistics usable. That does
not work, as we see the conditions as a single OR-clause, referencing an
attribute not covered by the statistic, leading to empty list of clauses
to be estimated using the statistics and an assert failure.

This changes choose_best_statistics to check which clauses are actually
covered, and only using attributes from the fully covered ones. For the
previous example this means the statistics object will not be considered
as compatible with the OR-clause.

Backpatch to 12, where MCVs were introduced. The issue does not affect
older versions because functional dependencies don't handle OR clauses.

Author: Tomas Vondra
Reviewed-by: Dean Rasheed
Reported-By: Manuel Rigger
Discussion: https://postgr.es/m/CA+u7OA7H5rcE2=8f263w4NZD6ipO_XOrYB816nuLXbmSTH9pQQ@mail.gmail.com
Backpatch-through: 12

5 years agoRemove useless "return;" lines
Alvaro Herrera [Thu, 28 Nov 2019 19:48:37 +0000 (16:48 -0300)]
Remove useless "return;" lines

Discussion: https://postgr.es/m/20191128144653.GA27883@alvherre.pgsql

5 years agoAdd tests for '-f' option in dropdb utility.
Amit Kapila [Thu, 28 Nov 2019 06:16:57 +0000 (11:46 +0530)]
Add tests for '-f' option in dropdb utility.

This will test that the force option works when there is an active backend
connected to the database being dropped.

Author: Pavel Stehule and Vignesh C
Reviewed-by: Amit Kapila and Vignesh C
Discussion: https://postgr.es/m/CAP_rwwmLJJbn70vLOZFpxGw3XD7nLB_7+NKz46H5EOO2k5H7OQ@mail.gmail.com

5 years agoMove pump_until to TestLib.pm.
Amit Kapila [Thu, 28 Nov 2019 02:58:17 +0000 (08:28 +0530)]
Move pump_until to TestLib.pm.

The subroutine pump_until provides the functionality to poll until the
given string is matched, or a timeout occurs.  This can be used from other
places as well, so moving it to TestLib.pm.  The immediate need is for an
upcoming regression test patch for dropdb utility.

Author: Vignesh C
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CAP_rwwmLJJbn70vLOZFpxGw3XD7nLB_7+NKz46H5EOO2k5H7OQ@mail.gmail.com

5 years agopg_upgrade: adjust error paragraph width to be consistent
Bruce Momjian [Thu, 28 Nov 2019 01:36:33 +0000 (20:36 -0500)]
pg_upgrade:  adjust error paragraph width to be consistent

Previously these error paragraphs were too wide.

Backpatch-through: 13

5 years agopg_upgrade: improve instructions for fixing incompatible isn use
Bruce Momjian [Thu, 28 Nov 2019 01:24:57 +0000 (20:24 -0500)]
pg_upgrade: improve instructions for fixing incompatible isn use

This clarifies instructions on how to dump/reload databases that use
incompatible isn versions.

Reported-by: Alvaro Herrera
Discussion: https://postgr.es/m/20191114190652.GA23586@alvherre.pgsql

Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Backpatch-through: 13

5 years agoDon't use native methods in TestLib::slurp_file on Msys
Andrew Dunstan [Wed, 27 Nov 2019 20:45:44 +0000 (15:45 -0500)]
Don't use native methods in TestLib::slurp_file on Msys

Commit 114541d58e has upset some buildfarm members running Msys, that
weren't previously having problems, so the use of native Windows methods
to open files is restricted by this patch to only MSVC builds.

5 years agoRevert "Close stdin where it's not needed in TestLib.pm procedures"
Andrew Dunstan [Wed, 27 Nov 2019 12:19:22 +0000 (07:19 -0500)]
Revert "Close stdin where it's not needed in TestLib.pm procedures"

This reverts commits 9af34f3c6b and 792dba73c8.

The code has been found not to be portable.

Discussion: https://postgr.es/m/2658c496-f885-02db-13bb-228423782524@2ndQuadrant.com

5 years agoMove configure --disable-float8-byval to pg_config_manual.h
Peter Eisentraut [Wed, 27 Nov 2019 10:21:02 +0000 (11:21 +0100)]
Move configure --disable-float8-byval to pg_config_manual.h

This build option was once useful to maintain compatibility with
version-0 functions, but those are no longer supported, so this option
is no longer useful for end users.  We keep the option available to
developers in pg_config_manual.h so that it is easy to test the
pass-by-reference code paths without having to fire up a 32-bit
machine.

Discussion: https://www.postgresql.org/message-id/flat/f3e1e576-2749-bbd7-2d57-3f9dcf75255a@2ndquadrant.com

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

5 years agoFix closing stdin in TestLib.pm
Andrew Dunstan [Tue, 26 Nov 2019 21:23:00 +0000 (16:23 -0500)]
Fix closing stdin in TestLib.pm

Commit 9af34f3c6b naively assumed that all non-windows platforms would
have pseudoterminals and that perl would have IO::Pty. Such is not the
case. Instead of assumung anything, test for the presence of IO::Pty and
only use code that might depend on it if it's present. The test result is
exposed in $TestLib::have_io_pty so that it can be conveniently used in
SKIP tests.

Discussion: https://postgr.es/m/20191126044110.GB5435@paquier.xyz

5 years agoAllow access to child table statistics if user can read parent table.
Tom Lane [Tue, 26 Nov 2019 19:41:48 +0000 (14:41 -0500)]
Allow access to child table statistics if user can read parent table.

The fix for CVE-2017-7484 disallowed use of pg_statistic data for
planning purposes if the user would not be able to select the associated
column and a non-leakproof function is to be applied to the statistics
values.  That turns out to disable use of pg_statistic data in some
common cases involving inheritance/partitioning, where the user does
have permission to select from the parent table that was actually named
in the query, but not from a child table whose stats are needed.  Since,
in non-corner cases, the user *can* select the child table's data via
the parent, this restriction is not actually useful from a security
standpoint.  Improve the logic so that we also check the permissions of
the originally-named table, and allow access if select permission exists
for that.

When checking access to stats for a simple child column, we can map
the child column number back to the parent, and perform this test
exactly (including not allowing access if the child column isn't
exposed by the parent).  For expression indexes, the current logic
just insists on whole-table select access, and this patch allows
access if the user can select the whole parent table.  In principle,
if the child table has extra columns, this might allow access to
stats on columns the user can't read.  In practice, it's unlikely
that the planner is going to do any stats calculations involving
expressions that are not visible to the query, so we'll ignore that
fine point for now.  Perhaps someday we'll improve that logic to
detect exactly which columns are used by an expression index ...
but today is not that day.

Back-patch to v11.  The issue was created in 9.2 and up by the
CVE-2017-7484 fix, but this patch depends on the append_rel_array[]
planner data structure which only exists in v11 and up.  In
practice the issue is most urgent with partitioned tables, so
fixing v11 and later should satisfy much of the practical need.

Dilip Kumar and Amit Langote, with some kibitzing by me

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

5 years agoAdd safeguards for pg_fsync() called with incorrectly-opened fds
Michael Paquier [Tue, 26 Nov 2019 04:32:52 +0000 (13:32 +0900)]
Add safeguards for pg_fsync() called with incorrectly-opened fds

On some platforms, fsync() returns EBADFD when opening a file descriptor
with O_RDONLY (read-only), leading ultimately now to a PANIC to prevent
data corruption.

This commit adds a new sanity check in pg_fsync() based on fcntl() to
make sure that we don't repeat again mistakes with incorrectly-set file
descriptors so as problems are detected at an early stage.  Without
that, such errors could only be detected after running Postgres on a
specific supported platform for the culprit code path, which could take
some time before being found.  b8e19b93 was a fix for such a problem,
which got undetected for more than 5 years, and a586cc4b fixed another
similar issue.

Note that the new check added works as well when fsync=off is
configured, so as all regression tests would detect problems as long as
assertions are enabled.  fcntl() being not available on Windows, the
new checks do not happen there.

Author: Michael Paquier
Reviewed-by: Mark Dilger
Discussion: https://postgr.es/m/20191009062640.GB21379@paquier.xyz

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

Revert part of commit 19df1702f5.

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

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

5 years agoUse procsignal_sigusr1_handler for auxiliary processes.
Robert Haas [Mon, 25 Nov 2019 21:08:53 +0000 (16:08 -0500)]
Use procsignal_sigusr1_handler for auxiliary processes.

AuxiliaryProcessMain does ProcSignalInit, so one might expect that
auxiliary processes would need to respond to SendProcSignal, but none
of the auxiliary processes do that. Change them to use
procsignal_sigusr1_handler instead of their own private handlers so
that they do. Besides seeming more correct, this is also less code. It
shouldn't make any functional difference right now because, as far as
we know, there are no current cases where SendProcSignal targets an
auxiliary process, but there are plans to change that in the future.

Andres Freund

Discussion: http://postgr.es/m/20181030051643.elbxjww5jjgnjaxg@alap3.anarazel.de

5 years agoClose stdin where it's not needed in TestLib.pm procedures
Andrew Dunstan [Mon, 25 Nov 2019 20:51:51 +0000 (15:51 -0500)]
Close stdin where it's not needed in TestLib.pm procedures

Where possible, do this using a pseudoterminal, so that things like
openssl that want to open /dev/tty if stdin isn't a tty won't.
Elsewhere, i.e. Windows, just close by providing an empty string using
the standard IPC::Run pipe mechanism.

Patch by Andrew Dunstan, based on an idea from Craig Ringer.

Reviewed by Mark Dilger.

Discussion: https://postgr.es/m/873ebb57-fc98-340d-949d-691b1810bf66@2ndQuadrant.com

5 years agoRefactor WAL file-reading code into WALRead()
Alvaro Herrera [Mon, 25 Nov 2019 18:04:54 +0000 (15:04 -0300)]
Refactor WAL file-reading code into WALRead()

XLogReader, walsender and pg_waldump all had their own routines to read
data from WAL files to memory, with slightly different approaches
according to the particular conditions of each environment.  There's a
lot of commonality, so we can refactor that into a single routine
WALRead in XLogReader, and move the differences to a separate (simpler)
callback that just opens the next WAL-segment.  This results in a
clearer (ahem) code flow.

The error reporting needs are covered by filling in a new error-info
struct, WALReadError, and it's the caller's responsibility to act on it.
The backend has WALReadRaiseError() to do so.

We no longer ever need to seek in this interface; switch to using
pg_pread().

Author: Antonin Houska, with contributions from Álvaro Herrera
Reviewed-by: Michaël Paquier, Kyotaro Horiguchi
Discussion: https://postgr.es/m/14984.1554998742@spoje.net

5 years agoFix unportable printf format introduced in commit 9290ad198.
Tom Lane [Mon, 25 Nov 2019 15:48:36 +0000 (10:48 -0500)]
Fix unportable printf format introduced in commit 9290ad198.

"%ld" is not an acceptable format spec for int64 variables, though
it accidentally works on most non-Windows 64-bit platforms.  Follow
the lead of commit 6a1cd8b92, and use "%lld" with an explicit cast
to long long.  Per buildfarm.

5 years agoMake the order of the header file includes consistent.
Amit Kapila [Mon, 25 Nov 2019 02:38:57 +0000 (08:08 +0530)]
Make the order of the header file includes consistent.

Similar to commits 14aec035027e735035f2 and dddf4cdc33, this commit
makes the order of header file inclusion consistent in more places.

Author: Vignesh C
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CALDaNm2Sznv8RR6Ex-iJO6xAdsxgWhCoETkaYX=+9DW3q0QCfA@mail.gmail.com

5 years agoFix inconsistent variable name in static function of mac8.c
Michael Paquier [Mon, 25 Nov 2019 00:57:35 +0000 (09:57 +0900)]
Fix inconsistent variable name in static function of mac8.c

Both argument names were reversed in the declaration of the function.

Author: Ranier Vilela
Discussion: https://postgr.es/m/MN2PR18MB292755AEFF9A9144B220ABEEE34B0@MN2PR18MB2927.namprd18.prod.outlook.com

5 years agoRefactor reloption handling for index AMs in-core
Michael Paquier [Mon, 25 Nov 2019 00:40:53 +0000 (09:40 +0900)]
Refactor reloption handling for index AMs in-core

This reworks the reloption parsing and build of a couple of index AMs by
creating new structures for each index AM's options.  This split was
already done for BRIN, GIN and GiST (which actually has a fillfactor
parameter), but not for hash, B-tree and SPGiST which relied on
StdRdOptions due to an overlap with the default option set.

This saves a couple of bytes for rd_options in each relcache entry with
indexes making use of relation options, and brings more consistency
between all index AMs.  While on it, add a couple of AssertMacro() calls
to make sure that utility macros to grab values of reloptions are used
with the expected index AM.

Author: Nikolay Shaplov
Reviewed-by: Amit Langote, Michael Paquier, Álvaro Herrera, Dent John
Discussion: https://postgr.es/m/4127670.gFlpRb6XCm@x200m

5 years agoUse native methods to open input in TestLib::slurp_file on Windows.
Andrew Dunstan [Sun, 24 Nov 2019 23:25:22 +0000 (18:25 -0500)]
Use native methods to open input in TestLib::slurp_file on Windows.

It is hoped that this will avoid some errors that we have seen before,
but lately with greater frequency, in buildfarm animals.

For now apply only to master. If this proves effective it can be
backpatched.

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

Author: Juan José Santamaría Flecha

5 years agoDoc: improve discussion of race conditions involved in LISTEN.
Tom Lane [Sun, 24 Nov 2019 23:03:39 +0000 (18:03 -0500)]
Doc: improve discussion of race conditions involved in LISTEN.

The user docs didn't really explain how to use LISTEN safely,
so clarify that.  Also clean up some fuzzy-headed explanations
in comments.  No code changes.

Discussion: https://postgr.es/m/3ac7f397-4d5f-be8e-f354-440020675694@gmail.com

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

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

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

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

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

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

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

Back-patch to 10.

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

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

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

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

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

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

5 years agoStabilize the results of pg_notification_queue_usage().
Tom Lane [Sun, 24 Nov 2019 19:09:33 +0000 (14:09 -0500)]
Stabilize the results of pg_notification_queue_usage().

This function wasn't touched in commit 51004c717, but that turns out
to be a bad idea, because its results now include any dead space
that exists in the NOTIFY queue on account of our being lazy about
advancing the queue tail.  Notably, the isolation tests now fail
if run twice without a server restart between, because async-notify's
first test of the function will already show a positive value.
It seems likely that end users would be equally unhappy about the
result's instability.  To fix, just make the function call
asyncQueueAdvanceTail before computing its result.  That should end
in producing the same value as before, and it's hard to believe that
there's any practical use-case where pg_notification_queue_usage()
is called so often as to create a performance degradation, especially
compared to what we did before.

Out of paranoia, also mark this function parallel-restricted (it
was volatile, but parallel-safe by default, before).  Although the
code seems to work fine when run in a parallel worker, that's outside
the design scope of async.c, and it's a bit scary to have intentional
side-effects happening in a parallel worker.  There seems no plausible
use-case where it'd be important to try to parallelize this, so let's
not take any risk of introducing new bugs.

In passing, re-pgindent async.c and run reformat-dat-files on
pg_proc.dat, just because I'm a neatnik.

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

5 years agoRemove a couple of unnecessary if-tests.
Tom Lane [Sun, 24 Nov 2019 17:03:16 +0000 (12:03 -0500)]
Remove a couple of unnecessary if-tests.

Commit abd9ca377 replaced a couple of while-loops in fmtfloat()
with calls to dopr_outchmulti, but I (tgl) failed to notice that
the new if-tests guarding those calls were really unnecessary,
because they're inside a larger if-block checking the same thing.

Ranier Vilela

Discussion: https://postgr.es/m/MN2PR18MB2927850AB00CF39CC370D107E34B0@MN2PR18MB2927.namprd18.prod.outlook.com

5 years agoRemove debugging aid
Alvaro Herrera [Sat, 23 Nov 2019 16:19:20 +0000 (13:19 -0300)]
Remove debugging aid

This Assert(false) was not supposed to be in the committed copy.

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

5 years agoUpdate sepgsql to add mandatory access control for TRUNCATE
Joe Conway [Sat, 23 Nov 2019 15:41:52 +0000 (10:41 -0500)]
Update sepgsql to add mandatory access control for TRUNCATE

Use SELinux "db_table: { truncate }" to check if permission is granted to
TRUNCATE. Update example SELinux policy to grant needed permission for
TRUNCATE. Add new regression test to demonstrate a positive and negative
cases. Test will only be run if the loaded SELinux policy has the
"db_table: { truncate }" permission. Makes use of recent commit which added
object TRUNCATE hook. Patch by Yuli Khodorkovskiy with minor
editorialization by me. Not back-patched because the object TRUNCATE hook
was not.

Author: Yuli Khodorkovskiy
Reviewed-by: Joe Conway
Discussion: https://postgr.es/m/CAFL5wJcomybj1Xdw7qWmPJRpGuFukKgNrDb6uVBaCMgYS9dkaA%40mail.gmail.com

5 years agoAdd object TRUNCATE hook
Joe Conway [Sat, 23 Nov 2019 15:39:20 +0000 (10:39 -0500)]
Add object TRUNCATE hook

All operations with acl permissions checks should have a corresponding hook
so that, for example, mandatory access control (MAC) may be enforced by an
extension. The command TRUNCATE is missing this hook, so add it. Patch by
Yuli Khodorkovskiy with some editorialization by me. Based on the discussion
not back-patched. A separate patch will exercise the hook in the sepgsql
extension.

Author: Yuli Khodorkovskiy
Reviewed-by: Joe Conway
Discussion: https://postgr.es/m/CAFL5wJcomybj1Xdw7qWmPJRpGuFukKgNrDb6uVBaCMgYS9dkaA%40mail.gmail.com

5 years agoMake psql redisplay the query buffer after \e.
Tom Lane [Fri, 22 Nov 2019 22:07:54 +0000 (17:07 -0500)]
Make psql redisplay the query buffer after \e.

Up to now, whatever you'd edited was put back into the query buffer
but not redisplayed, which is less than user-friendly.  But we can
improve that just by printing the text along with a prompt, if we
enforce that the editing result ends with a newline (which it
typically would anyway).  You then continue typing more lines if
you want, or you can type ";" or do \g or \r or another \e.

This is intentionally divorced from readline's processing,
for simplicity and so that it works the same with or without
readline enabled.  We discussed possibly integrating things
more closely with readline; but that seems difficult, uncertainly
portable across different readline and libedit versions, and
of limited real benefit anyway.  Let's try the simple way and
see if it's good enough.

Patch by me, thanks to Fabien Coelho and Laurenz Albe for review

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

5 years agoAvoid taking a new snapshot for an immutable simple expression in plpgsql.
Tom Lane [Fri, 22 Nov 2019 20:02:18 +0000 (15:02 -0500)]
Avoid taking a new snapshot for an immutable simple expression in plpgsql.

We already used this optimization if the plpgsql function is read-only.
But it seems okay to do it even in a read-write function, if the
expression contains only immutable functions/operators.  There would
only be a change of behavior if an "immutable" called function depends
on seeing database updates made during the current plpgsql function.
That's enough of a violation of the promise of immutability that anyone
who complains won't have much of a case.

The benefits are significant --- for simple cases like

  while i < 10000000
  loop
    i := i + 1;
  end loop;

I see net performance improvements around 45%.  Of course, real-world
cases won't get that much faster, but it ought to be noticeable.
At the very least, this removes much of the performance penalty that
used to exist for forgetting to mark a plpgsql function non-volatile.

Konstantin Knizhnik, reviewed by Pavel Stehule, cosmetic changes by me

Discussion: https://postgr.es/m/ed9da20e-01aa-d04b-d085-e6c16b14b9d7@postgrespro.ru

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

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

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

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

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

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

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

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

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

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

5 years agoAdd .gitignore to src/tutorial/
Michael Paquier [Fri, 22 Nov 2019 12:14:54 +0000 (21:14 +0900)]
Add .gitignore to src/tutorial/

A set of SQL files are generated for the tutorial when compiling the
code, so let's avoid any risk to have them added in the tree in the
future.

5 years agoRemove traces of version-0 calling convention in src/tutorial/
Michael Paquier [Fri, 22 Nov 2019 12:08:49 +0000 (21:08 +0900)]
Remove traces of version-0 calling convention in src/tutorial/

Support has been removed as of 5ded4bd, but code related to the tutorial
still used it.  Functions using version-1 are already present for some
time in the tutorial, and the documentation mentions them, so just
replace the old version with the new one.

Reported-by: Pavel Stehule
Analyzed-by: Euler Taveira
Author: Michael Paquier
Reviewed-by: Tom Lane, Pavel Stehule
Discussion: https://postgr.es/m/CAFj8pRCgC2uDzrw-vvanXu6Z3ofyviEOQPEpH6_aL4OCe7JRag@mail.gmail.com

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

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

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

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

5 years agoRemove configure --disable-float4-byval
Peter Eisentraut [Thu, 21 Nov 2019 17:00:07 +0000 (18:00 +0100)]
Remove configure --disable-float4-byval

This build option was only useful to maintain compatibility for
version-0 functions, but those are no longer supported, so this option
can be removed.

float4 is now always pass-by-value; the pass-by-reference code path is
completely removed.

Discussion: https://www.postgresql.org/message-id/flat/f3e1e576-2749-bbd7-2d57-3f9dcf75255a@2ndquadrant.com

5 years agoBump WAL version.
Fujii Masao [Thu, 21 Nov 2019 13:17:28 +0000 (22:17 +0900)]
Bump WAL version.

Oversight in commit e6d8069522. Since that commit changed the format of
XLOG_DBASE_DROP WAL record, XLOG_PAGE_MAGIC needs to be bumped.

Spotted by Michael Paquier

5 years agoMake DROP DATABASE command generate less WAL records.
Fujii Masao [Thu, 21 Nov 2019 12:10:37 +0000 (21:10 +0900)]
Make DROP DATABASE command generate less WAL records.

Previously DROP DATABASE generated as many XLOG_DBASE_DROP WAL records
as the number of tablespaces that the database to drop uses. This caused
the scans of shared_buffers as many times as the number of the tablespaces
during recovery because WAL replay of one XLOG_DBASE_DROP record needs
that full scan. This could make the recovery time longer especially
when shared_buffers is large.

This commit changes DROP DATABASE so that it generates only one
XLOG_DBASE_DROP record, and registers the information of all the tablespaces
into it. Then, WAL replay of XLOG_DBASE_DROP record needs full scan of
shared_buffers only once, and which may improve the recovery performance.

Author: Fujii Masao
Reviewed-by: Kirk Jamison, Simon Riggs
Discussion: https://postgr.es/m/CAHGQGwF8YwNH0ZaL+2wjZPkj+ji9UhC+Z4ScnG97WKtVY5L9iw@mail.gmail.com

5 years agoAllow ALTER VIEW command to rename the column in the view.
Fujii Masao [Thu, 21 Nov 2019 10:55:13 +0000 (19:55 +0900)]
Allow ALTER VIEW command to rename the column in the view.

ALTER TABLE RENAME COLUMN command always can be used to rename the column
in the view, but it's reasonable to add that syntax to ALTER VIEW too.

Author: Fujii Masao
Reviewed-by: Ibrar Ahmed, Yu Kimura
Discussion: https://postgr.es/m/CAHGQGwHoQMD3b-MqTLcp1MgdhCpOKU7QNRwjFooT4_d+ti5v6g@mail.gmail.com

5 years agoImprove tab-completion for ALTER MATERIALIZED VIEW.
Fujii Masao [Thu, 21 Nov 2019 10:22:37 +0000 (19:22 +0900)]
Improve tab-completion for ALTER MATERIALIZED VIEW.

Author: Takao Fujii
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/f9dcdef78c124517edc9e5e5880f651e@oss.nttdata.com

5 years agoTrack statistics for spilling of changes from ReorderBuffer.
Amit Kapila [Sat, 16 Nov 2019 12:54:00 +0000 (18:24 +0530)]
Track statistics for spilling of changes from ReorderBuffer.

This adds the statistics about transactions spilled to disk from
ReorderBuffer.  Users can query the pg_stat_replication view to check
these stats.

Author: Tomas Vondra, with bug-fixes and minor changes by Dilip Kumar
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/688b0b7f-2f6c-d827-c27b-216a8e3ea700@2ndquadrant.com

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

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

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

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

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

5 years agoSync patternsel_common's operator selection logic with pattern_prefix's.
Tom Lane [Wed, 20 Nov 2019 20:00:11 +0000 (15:00 -0500)]
Sync patternsel_common's operator selection logic with pattern_prefix's.

Make patternsel_common() select the comparison operators to use with
hardwired logic that matches pattern_prefix()'s new logic, eliminating
its dependencies on particular index opfamilies.

This shouldn't change any behavior, as it's just replacing runtime
operator lookups with the same values hard-wired.  But it makes these
closely-related functions look more alike, and saving some runtime
syscache lookups is worth something.

Actually, it's not quite true that this is zero behavioral change:
when estimating for a column of type "name", the comparison constant
will be kept as "text" not coerced to "name".  But that's more correct
anyway, and it allows additional simplification of the coercion logic,
again syncing this more closely with pattern_prefix().

Per consideration of a report from Manuel Rigger.

Discussion: https://postgr.es/m/CA+u7OA7nnGYy8rY0vdTe811NuA+Frr9nbcBO9u2Z+JxqNaud+g@mail.gmail.com

5 years agoFix HeapTupleSatisfiesNonVacuumable() comment.
Peter Geoghegan [Wed, 20 Nov 2019 19:36:54 +0000 (11:36 -0800)]
Fix HeapTupleSatisfiesNonVacuumable() comment.

Oversight in commit 63746189b23.

5 years agoReduce match_pattern_prefix()'s dependencies on index opfamilies.
Tom Lane [Wed, 20 Nov 2019 19:13:04 +0000 (14:13 -0500)]
Reduce match_pattern_prefix()'s dependencies on index opfamilies.

Historically, the planner's LIKE/regex index optimizations were only
carried out for specific index opfamilies.  That's never been a great
idea from the standpoint of extensibility, but it didn't matter so
much as long as we had no practical way to extend such behaviors anyway.
With the addition of planner support functions, and in view of ongoing
work to support additional table and index AMs, it seems like a good
time to relax this.

Hence, recast the decisions in match_pattern_prefix() so that rather
than decide which operators to generate by looking at what the index
opfamily contains, we decide which operators to generate a-priori
and then see if the opfamily supports them.  This is much more
defensible from a semantic standpoint anyway, since we know the
semantics of the chosen operators precisely, and we only need to
assume that the opfamily correctly implements operators it claims
to support.

The existing "pattern" opfamilies put a crimp in this approach, since
we need to select the pattern operators if we want those to work.
So we still have to special-case those opfamilies.  But that seems
all right, since in view of the addition of collations, the pattern
opfamilies seem like a legacy hack that nobody will be building on.

The only immediate effect of this change, so far as the core code is
concerned, is that anchored LIKE/regex patterns can be mapped onto
BRIN index searches, and exact-match patterns can be mapped onto hash
indexes, not only btree and spgist indexes as before.  That's not a
terribly exciting result, but it does fix an omission mentioned in
the ancient comments here.

Note: no catversion bump, even though this touches pg_operator.dat,
because it's only adding OID macros not changing the contents of
postgres.bki.

Per consideration of a report from Manuel Rigger.

Discussion: https://postgr.es/m/CA+u7OA7nnGYy8rY0vdTe811NuA+Frr9nbcBO9u2Z+JxqNaud+g@mail.gmail.com

5 years agoDoc: improve discussion of object owners' inherent privileges.
Tom Lane [Wed, 20 Nov 2019 17:27:00 +0000 (12:27 -0500)]
Doc: improve discussion of object owners' inherent privileges.

In particular, clarify that the role membership mechanism allows
members to inherit the ownership privileges of an object's owning
role.

Laurenz Albe, with some kibitzing by me

Discussion: https://postgr.es/m/504497aca66bf34bdcdd90bd0bcebdc3a33f577b.camel@cybertec.at

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

Author: Daniel Gustafsson <daniel@yesql.se>

5 years agoFix comment in xact.h
Michael Paquier [Wed, 20 Nov 2019 08:48:31 +0000 (17:48 +0900)]
Fix comment in xact.h

xl_xact_relfilenodes refers to a number of relations, not XIDs, whose
relfilenodes are processed.

Author: Yu Kimura
Discussion: https://postgr.es/m/a6ba6cf6bd0c990e019f008bae83437f@oss.nttdata.com

5 years agoHandle ReadFile() EOF correctly on Windows.
Thomas Munro [Wed, 20 Nov 2019 04:52:15 +0000 (17:52 +1300)]
Handle ReadFile() EOF correctly on Windows.

When ReadFile() encounters the end of a file while reading from
a synchronous handle with an offset provided via OVERLAPPED, it
reports an error instead of returning 0.  By not handling that
(undocumented) result correctly, we caused some noisy LOG
messages about an unknown error code.  Repair.

Back-patch to 12, where we started using pread()/ReadFile() with
an offset.

Reported-by: ZhenHua Cai, Amit Kapila
Diagnosed-by: Juan Jose Santamaria Flecha
Tested-by: Amit Kapila
Discussion: https://postgr.es/m/CAA4eK1LK3%2BWRtpz68TiRdpHwxxWm%3D%2Bt1BMf-G68hhQsAQ41PZg%40mail.gmail.com

5 years agoAdd the support for '-f' option in dropdb utility.
Amit Kapila [Mon, 18 Nov 2019 04:58:32 +0000 (10:28 +0530)]
Add the support for '-f' option in dropdb utility.

Specifying '-f' will add the 'force' option to the DROP DATABASE command
sent to the server.  This will try to terminate all existing connections
to the target database before dropping it.

Author: Pavel Stehule
Reviewed-by: Vignesh C and Amit Kapila
Discussion: https://postgr.es/m/CAP_rwwmLJJbn70vLOZFpxGw3XD7nLB_7+NKz46H5EOO2k5H7OQ@mail.gmail.com

5 years agoDoc: fix minor typo in func.sgml.
Tatsuo Ishii [Wed, 20 Nov 2019 00:08:43 +0000 (09:08 +0900)]
Doc: fix minor typo in func.sgml.

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

5 years agoFix corner-case failure in match_pattern_prefix().
Tom Lane [Tue, 19 Nov 2019 22:03:26 +0000 (17:03 -0500)]
Fix corner-case failure in match_pattern_prefix().

The planner's optimization code for LIKE and regex operators could
error out with a complaint like "no = operator for opfamily NNN"
if someone created a binary-compatible index (for example, a
bpchar_ops index on a text column) on the LIKE's left argument.

This is a consequence of careless refactoring in commit 74dfe58a5.
The old code in match_special_index_operator only accepted specific
combinations of the pattern operator and the index opclass, thereby
indirectly guaranteeing that the opclass would have a comparison
operator with the same LHS input type as the pattern operator.
While moving the logic out to a planner support function, I simplified
that test in a way that no longer guarantees that.  Really though we'd
like an altogether weaker dependency on the opclass, so rather than
put back exactly the old code, just allow lookup failure.  I have in
mind now to rewrite this logic completely, but this is the minimum
change needed to fix the bug in v12.

Per report from Manuel Rigger.  Back-patch to v12 where the mistake
came in.

Discussion: https://postgr.es/m/CA+u7OA7nnGYy8rY0vdTe811NuA+Frr9nbcBO9u2Z+JxqNaud+g@mail.gmail.com

5 years agoFix page modification outside of critical section in GIN
Alexander Korotkov [Tue, 19 Nov 2019 21:12:33 +0000 (00:12 +0300)]
Fix page modification outside of critical section in GIN

By oversight 52ac6cd2d0 makes ginDeletePage() sets pd_prune_xid of page to be
deleted before entering the critical section.  It appears that only versions 11
and later were affected by this oversight.

Backpatch-through: 11

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

5 years agoAllow invisible PROMPT2 in psql.
Thomas Munro [Tue, 19 Nov 2019 02:17:15 +0000 (15:17 +1300)]
Allow invisible PROMPT2 in psql.

Keep track of the visible width of PROMPT1, and provide %w as a way
for PROMPT2 to generate the same number of spaces.

Author: Thomas Munro, with ideas from others
Reviewed-by: Tom Lane (earlier version)
Discussion: https://postgr.es/m/CA%2BhUKG%2BzGd7RigjWbxwhzGW59gUpf76ydQECeGdEdodH6nd__A%40mail.gmail.com

5 years agoAdd logical_decoding_work_mem to limit ReorderBuffer memory usage.
Amit Kapila [Sat, 16 Nov 2019 12:19:33 +0000 (17:49 +0530)]
Add logical_decoding_work_mem to limit ReorderBuffer memory usage.

Instead of deciding to serialize a transaction merely based on the
number of changes in that xact (toplevel or subxact), this makes
the decisions based on amount of memory consumed by the changes.

The memory limit is defined by a new logical_decoding_work_mem GUC,
so for example we can do this

    SET logical_decoding_work_mem = '128kB'

to reduce the memory usage of walsenders or set the higher value to
reduce disk writes. The minimum value is 64kB.

When adding a change to a transaction, we account for the size in
two places. Firstly, in the ReorderBuffer, which is then used to
decide if we reached the total memory limit. And secondly in the
transaction the change belongs to, so that we can pick the largest
transaction to evict (and serialize to disk).

We still use max_changes_in_memory when loading changes serialized
to disk. The trouble is we can't use the memory limit directly as
there might be multiple subxact serialized, we need to read all of
them but we don't know how many are there (and which subxact to
read first).

We do not serialize the ReorderBufferTXN entries, so if there is a
transaction with many subxacts, most memory may be in this type of
objects. Those records are not included in the memory accounting.

We also do not account for INTERNAL_TUPLECID changes, which are
kept in a separate list and not evicted from memory. Transactions
with many CTID changes may consume significant amounts of memory,
but we can't really do much about that.

The current eviction algorithm is very simple - the transaction is
picked merely by size, while it might be useful to also consider age
(LSN) of the changes for example. With the new Generational memory
allocator, evicting the oldest changes would make it more likely
the memory gets actually pfreed.

The logical_decoding_work_mem can be set in postgresql.conf, in which
case it serves as the default for all publishers on that instance.

Author: Tomas Vondra, with changes by Dilip Kumar and Amit Kapila
Reviewed-by: Dilip Kumar and Amit Kapila
Tested-By: Vignesh C
Discussion: https://postgr.es/m/688b0b7f-2f6c-d827-c27b-216a8e3ea700@2ndquadrant.com

5 years agonbtree: Tweak _bt_pgaddtup() comments.
Peter Geoghegan [Mon, 18 Nov 2019 21:04:53 +0000 (13:04 -0800)]
nbtree: Tweak _bt_pgaddtup() comments.

Make it clear that _bt_pgaddtup() truncates the first data item on an
internal page because its key is supposed to be treated as minus
infinity within _bt_compare().

5 years agoFurther fix dumping of views that contain just VALUES(...).
Tom Lane [Sun, 17 Nov 2019 01:00:19 +0000 (20:00 -0500)]
Further fix dumping of views that contain just VALUES(...).

It turns out that commit e9f1c01b7 missed a case: we must print a
VALUES clause in long format if get_query_def is given a resultDesc
that would require the query's output column name(s) to be different
from what the bare VALUES clause would produce.

This applies in case an ALTER ... RENAME COLUMN has been done to
a view that formerly could be printed in simple format, as shown
in the added regression test case.  It also explains bug #16119
from Dmitry Telpt, because it turns out that (unlike CREATE VIEW)
CREATE MATERIALIZED VIEW fails to apply any column aliases it's
given to the stored ON SELECT rule.  So to get them to be printed,
we have to account for the resultDesc renaming.  It might be worth
changing the matview code so that it creates the ON SELECT rule
with the correct aliases; but we'd still need these messy checks in
get_simple_values_rte to handle the case of a subsequent column
rename, so any such change would be just neatnik-ism not a bug fix.

Like the previous patch, back-patch to all supported branches.

Discussion: https://postgr.es/m/16119-e64823f30a45a754@postgresql.org

5 years agoAdd tuplesort test to serial_schedule.
Peter Geoghegan [Sat, 16 Nov 2019 18:51:03 +0000 (10:51 -0800)]
Add tuplesort test to serial_schedule.

Oversight in commit 4a252996.

5 years agoImprove stability of tests for VACUUM (SKIP_LOCKED)
Michael Paquier [Sat, 16 Nov 2019 06:23:12 +0000 (15:23 +0900)]
Improve stability of tests for VACUUM (SKIP_LOCKED)

Concurrent autovacuums running with the main regression test suite
could cause the tests with VACUUM (SKIP_LOCKED) to generate randomly
WARNING messages.  For these tests, set client_min_messages to ERROR to
get rid of those random failures, as disabling autovacuum for the
relations operated would not completely close the failure window.

For isolation tests, disable autovacuum for the relations vacuumed with
SKIP_LOCKED.  The tests are designed so as LOCK commands are taken
in a first session before running a concurrent VACUUM (SKIP_LOCKED) in a
second to generate WARNING messages, but a concurrent autovacuum could
cause the tests to be slower.

Reported-by: Tom Lane
Author: Michael Paquier
Reviewed-by: Andres Freund, Tom Lane
Discussion: https://postgr.es/m/25294.1573077278@sss.pgh.pa.us
Backpatch-through: 12

5 years agoProperly determine length for on-disk TOAST values
Tomas Vondra [Sat, 16 Nov 2019 01:40:02 +0000 (02:40 +0100)]
Properly determine length for on-disk TOAST values

In detoast_attr_slice, VARSIZE_ANY was used to compute compressed length
of on-disk TOAST values. That's incorrect, because the varlena value may
be just a TOAST pointer, producing either bogus value or crashing.

This is likely why the code was crashing on big-endian machines before
540f31680913 replaced the VARSIZE with VARSIZE_ANY, which however only
masked the issue.

Reported-by: Rushabh Lathia
Discussion: https://postgr.es/m/CAL-OGkthU9Gs7TZchf5OWaL-Gsi=hXqufTxKv9qpNG73d5na_g@mail.gmail.com

5 years agoSkip system attributes when applying mvdistinct stats
Tomas Vondra [Sat, 16 Nov 2019 00:17:15 +0000 (01:17 +0100)]
Skip system attributes when applying mvdistinct stats

When estimating number of distinct groups, we failed to ignore system
attributes when matching the group expressions to mvdistinct stats,
causing failures like

  ERROR: negative bitmapset member not allowed

Fix that by simply skipping anything that is not a regular attribute.
Backpatch to PostgreSQL 10, where the extended stats were introduced.

Bug: #16111
Reported-by: Tuomas Leikola
Author: Tomas Vondra
Backpatch-through: 10
Discussion: https://postgr.es/m/16111-687799584c3a7e73@postgresql.org

5 years agoAlways call ExecShutdownNode() if appropriate.
Thomas Munro [Fri, 15 Nov 2019 21:04:52 +0000 (10:04 +1300)]
Always call ExecShutdownNode() if appropriate.

Call ExecShutdownNode() after ExecutePlan()'s loop, rather than at each
break.  We had forgotten to do that in one case.  The omission caused
intermittent "temporary file leak" warnings from multi-batch parallel
hash joins with a LIMIT clause.

Back-patch to 11.  Though the problem exists in theory in earlier
parallel query releases, nothing really depended on it.

Author: Kyotaro Horiguchi
Reviewed-by: Thomas Munro, Amit Kapila
Discussion: https://postgr.es/m/20191111.212418.2222262873417235945.horikyota.ntt%40gmail.com

5 years agoRemove the word "virgins" for documentation
Alvaro Herrera [Thu, 14 Nov 2019 20:33:26 +0000 (17:33 -0300)]
Remove the word "virgins" for documentation

Apparently, it's no longer welcome.  Therefore replace it with "pristine",
and add some explanatory text while at it.

Reported by Brian Williams
Discussion: https://postgr.es/m/157313712259.14261.16141263269989647311@wrigleys.postgresql.org

5 years agoCleanup code in reloptions.h regarding reloption handling
Michael Paquier [Thu, 14 Nov 2019 04:59:59 +0000 (13:59 +0900)]
Cleanup code in reloptions.h regarding reloption handling

reloptions.h includes since ba748f7 a set of macros to handle reloption
types in a way similar to how parseRelOptions() works.  They have never
been used in the core code, and we have more simple methods now to parse
and fill in rd_options for a given relation depending on its relkind, so
remove this interface to simplify things.

Per discussion between Amit Langote, Álvaro Herrera and me.

Discussion: https://postgr.es/m/CA+HiwqE6zbNO92az6pp5GiTw4tr-9rfCE0t84whQSP+YwSKjMQ@mail.gmail.com

5 years agoSplit handling of reloptions for partitioned tables
Michael Paquier [Thu, 14 Nov 2019 03:34:28 +0000 (12:34 +0900)]
Split handling of reloptions for partitioned tables

Partitioned tables do not have relation options yet, but, similarly to
what's done for views which have their own parsing table, it could make
sense to introduce new parameters for some of the existing default ones
like fillfactor, autovacuum, etc.  Splitting things has the advantage to
make the information stored in rd_options include only the necessary
information, reducing the amount of memory used for a relcache entry
with partitioned tables if new reloptions are introduced at this level.

Author:  Nikolay Shaplov
Reviewed-by: Amit Langote, Michael Paquier
Discussion: https://postgr.es/m/1627387.Qykg9O6zpu@x200m

5 years agoFix plan instability in the new tuplesort test.
Andres Freund [Thu, 14 Nov 2019 00:36:31 +0000 (16:36 -0800)]
Fix plan instability in the new tuplesort test.

At least buildfarm member florican doesn't use a material node above a
sort in the mark/restore case. As material is not intended to be
tested with that query, disallow.

5 years agoRemove unused code from tuplesort.
Andres Freund [Wed, 13 Nov 2019 23:57:01 +0000 (15:57 -0800)]
Remove unused code from tuplesort.

copytup_index() is unused, as tuplesort_putindextuplevalues() doesn't
use COPYTUP(). Replace function body with an elog(ERROR), as already
done e.g. for copytup_datum().

Author: Andres Freund
Discussion: https://postgr.es/m/20191013144153.ooxrfglvnaocsrx2@alap3.anarazel.de

5 years agoAdd tests for tuplesort.c.
Andres Freund [Thu, 24 Oct 2019 20:58:40 +0000 (13:58 -0700)]
Add tests for tuplesort.c.

Previously significant parts of tuplesort.c were untested. This
commit, while not testing every path, significantly increases
coverage.  In particular, this adds tests for abbreviated key logic,
forward/backward scans & scrolling and mark/restore.

I tried to keep the table sizes reasonable, and stress the on-disk
paths by setting work_mem to low values for specific tests. The
buildfarm will tell whether more attention to test time is needed.

Author: Andres Freund
Discussion: https://postgr.es/m/20191013144153.ooxrfglvnaocsrx2@alap3.anarazel.de

5 years agoAdd missing check_collation_set call to bpcharne().
Tom Lane [Wed, 13 Nov 2019 20:53:53 +0000 (15:53 -0500)]
Add missing check_collation_set call to bpcharne().

We should throw an error for indeterminate collation, but bpcharne()
was missing that logic, resulting in a much less user-friendly error
(either an assertion failure or "cache lookup failed for collation 0").

Per report from Manuel Rigger.  Back-patch to v12 where the mistake
came in, evidently in commit 5e1963fb7.  (Before non-deterministic
collations, this function wasn't collation sensitive.)

Discussion: https://postgr.es/m/CA+u7OA4HOjtymxAbuGNh4-X_2R0Lw5n01tzvP8E5-i-2gQXYWA@mail.gmail.com

5 years agoFix silly initializations (cosmetic only).
Tom Lane [Wed, 13 Nov 2019 20:26:54 +0000 (15:26 -0500)]
Fix silly initializations (cosmetic only).

Initializing a pointer to "false" isn't per project style,
and reportedly some compilers warn about it (though I've
not seen any such warnings in the buildfarm).

Seems to have come in with commit ff11e7f4b, so back-patch
to v12 where that was added.

Didier Gautheron

Discussion: https://postgr.es/m/CAJRYxu+XQuM0qnSqt1Ujztu6fBPzMMAT3VEn6W32rgKG6A2Fsw@mail.gmail.com

5 years agoAvoid using SplitIdentifierString to parse ListenAddresses, too.
Tom Lane [Wed, 13 Nov 2019 18:51:58 +0000 (13:51 -0500)]
Avoid using SplitIdentifierString to parse ListenAddresses, too.

This gets rid of our former behavior of forcibly downcasing
the postmaster's hostname list and truncating the elements to
NAMEDATALEN.  In principle, DNS hostnames are case-insensitive
so the first behavior should be harmless, and server hostnames
are seldom long enough for the second behavior to be an issue.
But it's still dubious, and an easy fix is available: just use
SplitGUCList instead.

AFAICT, all other SplitIdentifierString calls in the backend are
OK: either the items actually are SQL identifiers, or they are
keywords that are short and case-insensitive.

Per thinking about bug #16106.  While this has been wrong for
a very long time, the lack of field complaints means there's
little reason to back-patch.

Discussion: https://postgr.es/m/16106-7d319e4295d08e70@postgresql.org

5 years agoAvoid downcasing/truncation of RADIUS authentication parameters.
Tom Lane [Wed, 13 Nov 2019 18:41:04 +0000 (13:41 -0500)]
Avoid downcasing/truncation of RADIUS authentication parameters.

Commit 6b76f1bb5 changed all the RADIUS auth parameters to be lists
rather than single values.  But its use of SplitIdentifierString
to parse the list format was not very carefully thought through,
because that function thinks it's parsing SQL identifiers, which
means it will (a) downcase the strings and (b) truncate them to
be shorter than NAMEDATALEN.  While downcasing should be harmless
for the server names and ports, it's just wrong for the shared
secrets, and probably for the NAS Identifier strings as well.
The truncation aspect is at least potentially a problem too,
though typical values for these parameters would fit in 63 bytes.

Fortunately, we now have a function SplitGUCList that is exactly
the same except for not doing the two unwanted things, so fixing
this is a trivial matter of calling that function instead.

While here, improve the documentation to show how to double-quote
the parameter values.  I failed to resist the temptation to do
some copy-editing as well.

Report and patch from Marcos David (bug #16106); doc changes by me.
Back-patch to v10 where the aforesaid commit came in, since this is
arguably a regression from our previous behavior with RADIUS auth.

Discussion: https://postgr.es/m/16106-7d319e4295d08e70@postgresql.org

5 years agoInclude TableFunc references when computing expression dependencies.
Tom Lane [Wed, 13 Nov 2019 17:11:49 +0000 (12:11 -0500)]
Include TableFunc references when computing expression dependencies.

The TableFunc node (i.e., XMLTABLE) includes type and collation OIDs
that might not be referenced anywhere else in the expression tree,
so they need to be accounted for when extracting dependencies.

Fortunately, the practical effects of this are limited, since
(a) it's somewhat unlikely that people would be extracting
columns of non-builtin types from an XML document, and (b)
in many scenarios, the query would contain other references
to such types, or functions depending on them.  However, it's
not hard to construct examples wherein the existing code lets
one drop a type used in XMLTABLE and thereby break a view.

This is evidently an original oversight in the XMLTABLE patch,
so back-patch to v10 where that came in.

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

5 years agoHandle arrays and ranges in pg_upgrade's test for non-upgradable types.
Tom Lane [Wed, 13 Nov 2019 16:35:37 +0000 (11:35 -0500)]
Handle arrays and ranges in pg_upgrade's test for non-upgradable types.

pg_upgrade needs to check whether certain non-upgradable data types
appear anywhere on-disk in the source cluster.  It knew that it has
to check for these types being contained inside domains and composite
types; but it somehow overlooked that they could be contained in
arrays and ranges, too.  Extend the existing recursive-containment
query to handle those cases.

We probably should have noticed this oversight while working on
commit 0ccfc2822 and follow-ups, but we failed to :-(.  The whole
thing's possibly a bit overdesigned, since we don't really expect
that any of these types will appear on disk; but if we're going to
the effort of doing a recursive search then it's silly not to cover
all the possibilities.

While at it, refactor so that we have only one copy of the search
logic, not three-and-counting.  Also, to keep the branches looking
more alike, back-patch the output wording change of commit 1634d3615.

Back-patch to all supported branches.

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

5 years agoMake pg_waldump report more detail information about PREPARE TRANSACTION record.
Fujii Masao [Wed, 13 Nov 2019 07:59:17 +0000 (16:59 +0900)]
Make pg_waldump report more detail information about PREPARE TRANSACTION record.

This commit changes xact_desc() so that it reports the detail information about
PREPARE TRANSACTION record, like GID (global transaction identifier),
timestamp at prepare transaction, delete-on-abort/commit relations,
XID of subtransactions, and invalidation messages. These are helpful
when diagnosing 2PC-related troubles.

Author: Fujii Masao
Reviewed-by: Michael Paquier, Andrey Lepikhov, Kyotaro Horiguchi, Julien Rouhaud, Alvaro Herrera
Discussion: https://postgr.es/m/CAHGQGwEvhASad4JJnCv=0dW2TJypZgW_Vpb-oZik2a3utCqcrA@mail.gmail.com

5 years agoAdd regression test for two-phase transaction in postgres_fdw
Michael Paquier [Wed, 13 Nov 2019 04:30:14 +0000 (13:30 +0900)]
Add regression test for two-phase transaction in postgres_fdw

postgres_fdw does not support two-phase transactions, so let's add a
small negative test case to check after it.  Note that this is checked
using an end-of-xact callback to ensure a proper connection cleanup with
the foreign server, which is called before checking if a server is able
to handle 2PC with max_prepared_xacts, so this test does not need an
alternate output file.

Author: Gilles Darold
Discussion: https://postgr.es/m/20191108090507.GC1768@paquier.xyz

5 years agoIntroduce the 'force' option for the Drop Database command.
Amit Kapila [Tue, 12 Nov 2019 05:36:13 +0000 (11:06 +0530)]
Introduce the 'force' option for the Drop Database command.

This new option terminates the other sessions connected to the target
database and then drop it.  To terminate other sessions, the current user
must have desired permissions (same as pg_terminate_backend()).  We don't
allow to terminate the sessions if prepared transactions, active logical
replication slots or subscriptions are present in the target database.

Author: Pavel Stehule with changes by me
Reviewed-by: Dilip Kumar, Vignesh C, Ibrar Ahmed, Anthony Nowocien,
Ryan Lambert and Amit Kapila
Discussion: https://postgr.es/m/CAP_rwwmLJJbn70vLOZFpxGw3XD7nLB_7+NKz46H5EOO2k5H7OQ@mail.gmail.com

5 years agoFinish reverting commit 0a52d378b.
Tom Lane [Tue, 12 Nov 2019 21:58:00 +0000 (16:58 -0500)]
Finish reverting commit 0a52d378b.

Apply the solution adopted in commit dcb7d3caf (ie, explicitly
don't call memcmp for a zero-length comparison) to func_get_detail()
as well, removing one other place where we were passing an
uninitialized array to a parse_func.c entry point.

Discussion: https://postgr.es/m/MN2PR18MB2927F24692485D754794F01BE3740@MN2PR18MB2927.namprd18.prod.outlook.com
Discussion: https://postgr.es/m/MN2PR18MB2927F6873DF2774A505AC298E3740@MN2PR18MB2927.namprd18.prod.outlook.com

5 years agopg_stat_activity: document client_port being null
Alvaro Herrera [Tue, 12 Nov 2019 21:34:20 +0000 (18:34 -0300)]
pg_stat_activity: document client_port being null

As suggested by Stephen Frost.
Discussion: https://postgr.es/m/20191104160605.GC6962@tamriel.snowman.net