users/gsingh/postgres.git
5 years agoTeach datum_image_eq() about cstring datums.
Peter Geoghegan [Tue, 12 Nov 2019 19:25:34 +0000 (11:25 -0800)]
Teach datum_image_eq() about cstring datums.

Bring datum_image_eq() in line with datumIsEqual() by adding support for
comparing cstring datums.

An upcoming patch that adds deduplication to the nbtree AM will use
datum_image_eq().  datum_image_eq() will need to work with all datatypes
that can be used as the storage type of a B-Tree index column, including
cstring.  (cstring is used as the storage type for columns of type
"name" as a space-saving optimization.)

Discussion: https://postgr.es/m/CAH2-Wzn3Ee49Gmxb7V1VJ3-AC8fWn-Fr8pfWQebHe8rYRxt5OQ@mail.gmail.com

5 years agoFix ecpglib.h to declare bool consistently with c.h.
Tom Lane [Tue, 12 Nov 2019 18:00:04 +0000 (13:00 -0500)]
Fix ecpglib.h to declare bool consistently with c.h.

This completes the task begun in commit 1408d5d86, to synchronize
ECPG's exported definitions with the definition of bool used by
c.h (and, therefore, the one actually in use in the ECPG library).
On practically all modern platforms, ecpglib.h will now just
include <stdbool.h>, which should surprise nobody anymore.
That removes a header-inclusion-order hazard for ECPG clients,
who previously might get build failures or unexpected behavior
depending on whether they'd included <stdbool.h> themselves,
and if so, whether before or after ecpglib.h.

On platforms where sizeof(_Bool) is not 1 (only old PPC-based
Mac systems, as far as I know), things are still messy, as
inclusion of <stdbool.h> could still break ECPG client code.
There doesn't seem to be any clean fix for that, and given the
probably-negligible population of users who would care anymore,
it's not clear we should go far out of our way to cope with it.
This change at least fixes some header-inclusion-order hazards
for our own code, since c.h and ecpglib.h previously disagreed
on whether bool should be char or unsigned char.

To implement this with minimal invasion of ECPG client namespace,
move the choice of whether to rely on <stdbool.h> into configure,
and have it export a configuration symbol PG_USE_STDBOOL.

ecpglib.h no longer exports definitions for TRUE and FALSE,
only their lowercase brethren.  We could undo that if we get
push-back about it.

Ideally we'd back-patch this as far as v11, which is where c.h
started to rely on <stdbool.h>.  But the odds of creating problems
for formerly-working ECPG client code seem about as large as the
odds of fixing any non-working cases, so we'll just do this in HEAD.

Discussion: https://postgr.es/m/CAA4eK1LmaKO7Du9M9Lo=kxGU8sB6aL8fa3sF6z6d5yYYVe3BuQ@mail.gmail.com

5 years agogitattributes: Add new file
Peter Eisentraut [Tue, 12 Nov 2019 07:13:55 +0000 (08:13 +0100)]
gitattributes: Add new file

5 years agoMake the order of the header file includes consistent in backend modules.
Amit Kapila [Tue, 12 Nov 2019 03:00:16 +0000 (08:30 +0530)]
Make the order of the header file includes consistent in backend modules.

Similar to commits 7e735035f2 and dddf4cdc33, this commit makes the order
of header file inclusion consistent for backend modules.

In the passing, removed a couple of duplicate inclusions.

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

5 years agoDoc: fix ancient mistake, or at least obsolete info, in rules example.
Tom Lane [Mon, 11 Nov 2019 19:39:54 +0000 (14:39 -0500)]
Doc: fix ancient mistake, or at least obsolete info, in rules example.

The example of expansion of multiple views claimed that the resulting
subquery nest would not get fully flattened because of an aggregate
function.  There's no aggregate in the example, though, only a user
defined function confusingly named MIN().  In a modern server, the
reason for the non-flattening is that MIN() is volatile, but I'm
unsure whether that was true back when this text was written.

Let's reduce the confusion level by using LEAST() instead (which
we didn't have at the time this example was created).  And then
we can just say that the planner will flatten the sub-queries, so
the rewrite system doesn't have to.

Noted by Paul Jungwirth.  This text is old enough to vote, so
back-patch to all supported branches.

Discussion: https://postgr.es/m/CA+renyXZFnmp9PcvX1EVR2dR=XG5e6E-AELr8AHCNZ8RYrpnPw@mail.gmail.com

5 years agoFurther improve stability of partition_prune regression test.
Tom Lane [Mon, 11 Nov 2019 15:33:00 +0000 (10:33 -0500)]
Further improve stability of partition_prune regression test.

Commits 4ea03f3f4 et al arranged to filter out row counts in parallel
plans, because those are dependent on the number of workers actually
obtained.  Somehow I missed that the 'Rows Removed by Filter' counts
can also vary, so fix that too.  Per buildfarm.

This seems worth a last-minute patch because unreliable regression
tests are a serious pain in the rear for packagers.

Like the previous patch, back-patch to v11 where this test was
introduced.

5 years agogitattributes: Remove entries for no longer existing files
Peter Eisentraut [Mon, 11 Nov 2019 10:54:12 +0000 (11:54 +0100)]
gitattributes: Remove entries for no longer existing files

5 years agoFix whitespace
Peter Eisentraut [Mon, 11 Nov 2019 08:51:10 +0000 (09:51 +0100)]
Fix whitespace

5 years agoRerun autoheader
Peter Eisentraut [Mon, 11 Nov 2019 08:50:07 +0000 (09:50 +0100)]
Rerun autoheader

This puts pg_config.h.in content back into the "correct" order.

5 years agoOptimize PredicateLockTuple().
Thomas Munro [Mon, 11 Nov 2019 03:34:01 +0000 (16:34 +1300)]
Optimize PredicateLockTuple().

PredicateLockTuple() has a fast exit if tuple was written by the current
transaction, as in that case it already has a lock.  This check can be
performed using TransactionIdIsCurrentTransactionId() instead of
SubTransGetTopmostTransaction(), to avoid any chance of having to hit the
disk.

Author: Ashwin Agrawal, based on a suggestion from Andres Freund
Reviewed-by: Thomas Munro
Discussion: https://postgr.es/m/CALfoeiv0k3hkEb3Oqk%3DziWqtyk2Jys1UOK5hwRBNeANT_yX%2Bng%40mail.gmail.com

5 years agoOptimize TransactionIdIsCurrentTransactionId().
Thomas Munro [Mon, 11 Nov 2019 03:33:04 +0000 (16:33 +1300)]
Optimize TransactionIdIsCurrentTransactionId().

If the passed in xid is the current top transaction, we can do a fast
check and exit early.  This should work well for the current heap but
also works very well for proposed AMs that don't use a separate xid
for subtransactions.

Author: Ashwin Agrawal, based on a suggestion from Andres Freund
Reviewed-by: Thomas Munro
Discussion: https://postgr.es/m/CALfoeiv0k3hkEb3Oqk%3DziWqtyk2Jys1UOK5hwRBNeANT_yX%2Bng%40mail.gmail.com

5 years agoRearrange dropdb() to avoid errors after allowing other sessions to exit.
Amit Kapila [Sat, 9 Nov 2019 11:58:27 +0000 (17:28 +0530)]
Rearrange dropdb() to avoid errors after allowing other sessions to exit.

During Drop Database, it is better to error out before allowing other
sessions to exit and forcefully terminating autovacuum workers.  All the
other errors except for checking subscriptions are already done before.

Author: Amit Kapila
Discussion: https://postgr.es/m/CAA4eK1+qhLkCYG2oy9xug9ur_j=G2wQNRYAyd+-kZfZ1z42pLw@mail.gmail.com

5 years agoFix subscription test
Peter Eisentraut [Sat, 9 Nov 2019 12:19:27 +0000 (13:19 +0100)]
Fix subscription test

After altering a subscription, we should wait until the updated table
sync data has been fetched by the subscriber.

5 years agodoc: Clarify documentation about SSL passphrases
Peter Eisentraut [Sat, 9 Nov 2019 09:13:14 +0000 (10:13 +0100)]
doc: Clarify documentation about SSL passphrases

The previous statement that using a passphrase disables the ability to
change the server's SSL configuration without a server restart was no
longer completely true since the introduction of
ssl_passphrase_command_supports_reload.

5 years agodoc: Further tweak recovery parameters documentation
Peter Eisentraut [Sat, 9 Nov 2019 08:35:21 +0000 (09:35 +0100)]
doc: Further tweak recovery parameters documentation

Remove one sentence that was deemed misleading.

Discussion: https://www.postgresql.org/message-id/flat/E1iEgSp-0004R5-2E%40gemulon.postgresql.org

5 years agoFix negative bitmapset member not allowed error in logical replication
Peter Eisentraut [Thu, 7 Nov 2019 12:48:59 +0000 (13:48 +0100)]
Fix negative bitmapset member not allowed error in logical replication

This happens when we add a replica identity column on a subscriber
that does not yet exist on the publisher, according to the mapping
maintained by the subscriber.  Code that checks whether the target
relation on the subscriber is updatable would check the replica
identity attribute bitmap with a column number -1, which would result
in an error.  To fix, skip such columns in the bitmap lookup and
consider the relation not updatable.  The result is consistent with
the rule that the replica identity columns on the subscriber must be a
subset of those on the publisher, since if the column doesn't exist on
the publisher, the column set on the subscriber can't be a subset.

Reported-by: Tim Clarke <tim.clarke@minerva.info>
Analyzed-by: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
Discussion: https://www.postgresql.org/message-id/flat/a9139c29-7ddd-973b-aa7f-71fed9c38d75%40minerva.info

5 years agoFix new COPY test of PL/pgSQL with VPATH builds
Michael Paquier [Sat, 9 Nov 2019 06:41:34 +0000 (15:41 +0900)]
Fix new COPY test of PL/pgSQL with VPATH builds

The buildfarm has turned red after 1858b10 because VPATH builds need to
use "@abs_srcdir@" and not "@abs_builddir@" for paths coming directly
from the source tree.  The input file of the new test got that right,
but not the output file.

Per complaints from several buildfarm animals, including desmoxytes and
culicidae.  I have also reproduced the error by myself.

5 years agoAdd tests for COPY in PL/pgSQL
Michael Paquier [Sat, 9 Nov 2019 05:50:20 +0000 (14:50 +0900)]
Add tests for COPY in PL/pgSQL

This stresses the error handling of COPY inside SPI which does not
support the operation using stdin or stdout, and these scenarios were
not tested up to now.

Author: Mark Dilger
Discussion: https://postgr.es/m/a6e9b130-7fd5-387b-4ec5-89bda24373ab@gmail.com

5 years agoPass ItemPointer not HeapTuple to IndexBuildCallback.
Andres Freund [Fri, 8 Nov 2019 08:44:52 +0000 (00:44 -0800)]
Pass ItemPointer not HeapTuple to IndexBuildCallback.

Not all AMs use HeapTuples internally, making it inconvenient to pass
a HeapTuple. As the index callbacks really only need the TID, not the
full tuple, modify callback to only take ItemPointer.

Author: Ashwin Agrawal
Reviewed-By: Andres Freund
Discussion: https://postgr.es/m/CALfoeis6=8ehuR=VNtHvj3z16cYfCwPdTcpaxU+sfSUJ5QgR3g@mail.gmail.com

5 years agoAdd backtrace support for error reporting
Alvaro Herrera [Fri, 8 Nov 2019 18:44:20 +0000 (15:44 -0300)]
Add backtrace support for error reporting

Add some support for automatically showing backtraces in certain error
situations in the server.  Backtraces are shown on assertion failure;
also, a new setting backtrace_functions can be set to a list of C
function names, and all ereport()s and elog()s from the mentioned
functions will have backtraces generated.  Finally, the function
errbacktrace() can be manually added to an ereport() call to generate a
backtrace for that call.

Authors: Peter Eisentraut, Álvaro Herrera
Discussion: https://postgr.es/m//5f48cb47-bf1e-05b6-7aae-3bf2cd01586d@2ndquadrant.com
Discussion: https://postgr.es/m/CAMsr+YGL+yfWE=JvbUbnpWtrRZNey7hJ07+zT4bYJdVp4Szdrg@mail.gmail.com

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

5 years agopostgres_fdw: Fix error message for PREPARE TRANSACTION.
Etsuro Fujita [Fri, 8 Nov 2019 08:00:30 +0000 (17:00 +0900)]
postgres_fdw: Fix error message for PREPARE TRANSACTION.

Currently, postgres_fdw does not support preparing a remote transaction
for two-phase commit even in the case where the remote transaction is
read-only, but the old error message appeared to imply that that was not
supported only if the remote transaction modified remote tables.  Change
the message so as to include the case where the remote transaction is
read-only.

Also fix a comment above the message.

Also add a note about the lack of supporting PREPARE TRANSACTION to the
postgres_fdw documentation.

Reported-by: Gilles Darold
Author: Gilles Darold and Etsuro Fujita
Reviewed-by: Michael Paquier and Kyotaro Horiguchi
Backpatch-through: 9.4
Discussion: https://postgr.es/m/08600ed3-3084-be70-65ba-279ab19618a5%40darold.net

5 years agoMore precise errors from initial pg_control check
Peter Eisentraut [Fri, 8 Nov 2019 07:03:16 +0000 (08:03 +0100)]
More precise errors from initial pg_control check

Use a separate error message for invalid checkpoint location and
invalid state instead of just "invalid data" for both.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/20191107041630.GK1768@paquier.xyz

5 years agoUse "low key" terminology in nbtsort.c.
Peter Geoghegan [Fri, 8 Nov 2019 01:12:09 +0000 (17:12 -0800)]
Use "low key" terminology in nbtsort.c.

nbtree index builds once stashed the "minimum key" for a page, which was
used as the basis of the pivot tuple that gets placed in the next level
up (i.e. the tuple that stores the downlink to the page in question).
It doesn't quite work that way anymore, so the "minimum key" terminology
now seems misleading (these days the minimum key is actually a straight
copy of the high key from the left sibling, which is a distinct thing in
subtle but important ways).  Rename this concept to "low key".  This
name is a lot clearer given that there is now a sharp distinction
between pivot and non-pivot tuples.  Also remove comments that describe
obsolete details about how the minimum key concept used to work.

Rather than generating the minus infinity item for the leftmost page on
a level by copying the new item and truncating that copy, simply
allocate a small buffer.  The old approach confusingly created the
impression that the new item had some kind of significance.  This was
another artifact of how things used to work before commits 8224de4f and
dd299df8.

5 years agodocs: clarify that only INSERT and UPDATE triggers can mod. NEW
Bruce Momjian [Thu, 7 Nov 2019 20:50:00 +0000 (15:50 -0500)]
docs:  clarify that only INSERT and UPDATE triggers can mod. NEW

The point is that DELETE triggers cannot modify any values.

Reported-by: Eugen Konkov
Discussion: https://postgr.es/m/919823407.20191029175436@yandex.ru

Backpatch-through: 9.4

5 years agoMove declaration of ecpg_gettext() to a saner place.
Tom Lane [Thu, 7 Nov 2019 19:21:52 +0000 (14:21 -0500)]
Move declaration of ecpg_gettext() to a saner place.

Declaring this in the client-visible header ecpglib.h was a pretty
poor decision.  It's not meant to be application-callable (and if
it was, putting it outside the extern "C" { ... } wrapper means
that C++ clients would fail to call it).  And the declaration would
not even compile for a client, anyway, since it would not have the
macro pg_attribute_format_arg().  Fortunately, it seems that no
clients have tried to include this header with ENABLE_NLS defined,
or we'd have gotten complaints about that.  But we have no business
putting such a restriction on client code.

Move the declaration to ecpglib_extern.h, since in fact nothing
outside src/interfaces/ecpg/ecpglib/ needs to call it.

The practical effect of this is just that clients can now safely
#include ecpglib.h while having ENABLE_NLS defined, but that seems
like enough of a reason to back-patch it.

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

5 years agoFix SET CONSTRAINTS .. DEFERRED on partitioned tables
Alvaro Herrera [Thu, 7 Nov 2019 16:59:24 +0000 (13:59 -0300)]
Fix SET CONSTRAINTS .. DEFERRED on partitioned tables

SET CONSTRAINTS ... DEFERRED failed on partitioned tables, because of a
sanity check that ensures that the affected constraints have triggers.
On partitioned tables, the triggers are in the leaf partitions, not in
the partitioned relations themselves, so the sanity check fails.
Removing the sanity check solves the problem, because the code needed to
support the case is already there.

Backpatch to 11.

Note: deferred unique constraints are not affected by this bug, because
they do have triggers in the parent partitioned table.  I did not add a
test for this scenario.

Discussion: https://postgr.es/m/20191105212915.GA11324@alvherre.pgsql

5 years agoFix integer-overflow edge case detection in interval_mul and pgbench.
Tom Lane [Thu, 7 Nov 2019 16:22:52 +0000 (11:22 -0500)]
Fix integer-overflow edge case detection in interval_mul and pgbench.

This patch adopts the overflow check logic introduced by commit cbdb8b4c0
into two more places.  interval_mul() failed to notice if it computed a
new microseconds value that was one more than INT64_MAX, and pgbench's
double-to-int64 logic had the same sorts of edge-case problems that
cbdb8b4c0 fixed in the core code.

To make this easier to get right in future, put the guts of the checks
into new macros in c.h, and add commentary about how to use the macros
correctly.

Back-patch to all supported branches, as we did with the previous fix.

Yuya Watari

Discussion: https://postgr.es/m/CAJ2pMkbkkFw2hb9Qb1Zj8d06EhWAQXFLy73St4qWv6aX=vqnjw@mail.gmail.com

5 years agoRemove HAVE_LONG_LONG_INT
Peter Eisentraut [Thu, 7 Nov 2019 12:30:04 +0000 (13:30 +0100)]
Remove HAVE_LONG_LONG_INT

The presence of long long int is now implied in the requirement for
C99 and the configure check for the same.

We keep the define hard-coded in ecpg_config.h for backward
compatibility with ecpg-using user code.

Discussion: https://www.postgresql.org/message-id/flat/5cdd6a2b-b2c7-c6f6-344c-a406d5c1a254%402ndquadrant.com

5 years agoFix nested error handling in PG_FINALLY
Peter Eisentraut [Thu, 7 Nov 2019 08:54:09 +0000 (09:54 +0100)]
Fix nested error handling in PG_FINALLY

We need to pop the error stack before running the user-supplied
PG_FINALLY code.  Otherwise an error in the cleanup code would end up
at the same sigsetjmp() invocation and result in an infinite error
handling loop.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/95a822c3-728b-af0e-d7e5-71890507ae0c%402ndquadrant.com

5 years agoFix assertion failure when running pgbench -s.
Fujii Masao [Thu, 7 Nov 2019 07:31:36 +0000 (16:31 +0900)]
Fix assertion failure when running pgbench -s.

If there is the WAL page that the continuation WAL record just fits within
(i.e., the continuation record ends just at the end of the page) and
the LSN in such page is specified with -s option, previously pg_waldump
caused an assertion failure. The cause of this assertion failure was that
XLogFindNextRecord() that pg_waldump -s calls mistakenly handled
such special WAL page.

This commit changes XLogFindNextRecord() so that it can handle
such WAL page correctly.

Back-patch to all supported versions.

Author: Andrey Lepikhov
Reviewed-by: Fujii Masao, Michael Paquier
Discussion: https://postgr.es/m/99303554-5dd5-06e6-f943-b3005ccd6edd@postgrespro.ru

5 years agoAdd reusable routine for making arrays unique.
Thomas Munro [Thu, 7 Nov 2019 03:51:04 +0000 (16:51 +1300)]
Add reusable routine for making arrays unique.

Introduce qunique() and qunique_arg(), which can be used after qsort()
and qsort_arg() respectively to remove duplicate values.  Use it where
appropriate.

Author: Thomas Munro
Reviewed-by: Tom Lane (in an earlier version)
Discussion: https://postgr.es/m/CAEepm%3D2vmFTNpAmwbGGD2WaryM6T3hSDVKQPfUwjdD_5XY6vAA%40mail.gmail.com

5 years agoCheck after errors of SPI_execute() in xml.c
Michael Paquier [Thu, 7 Nov 2019 02:13:31 +0000 (11:13 +0900)]
Check after errors of SPI_execute() in xml.c

SPI gets used to build a list of relation OIDs for XML object
generation, and one code path building a list uses SPI_execute() without
looking at errors it produces.  So fix that.

Author: Mark Dilger
Reviewed-by: Michael Paquier, Pavel Stehule
Discussion: https://postgr.es/m/17d30445-4862-7917-170f-84328dcd292d@gmail.com

5 years agoAllow sampling of statements depending on duration
Tomas Vondra [Mon, 4 Nov 2019 00:57:45 +0000 (01:57 +0100)]
Allow sampling of statements depending on duration

This allows logging a sample of statements, without incurring excessive
log traffic (which may impact performance).  This can be useful when
analyzing workloads with lots of short queries.

The sampling is configured using two new GUC parameters:

 * log_min_duration_sample - minimum required statement duration

 * log_statement_sample_rate - sample rate (0.0 - 1.0)

Only statements with duration exceeding log_min_duration_sample are
considered for sampling. To enable sampling, both those GUCs have to
be set correctly.

The existing log_min_duration_statement GUC has a higher priority, i.e.
statements with duration exceeding log_min_duration_statement will be
always logged, irrespectedly of how the sampling is configured. This
means only configurations

  log_min_duration_sample < log_min_duration_statement

do actually sample the statements, instead of logging everything.

Author: Adrien Nayrat
Reviewed-by: David Rowley, Vik Fearing, Tomas Vondra
Discussion: https://postgr.es/m/bbe0a1a8-a8f7-3be2-155a-888e661cc06c@anayrat.info

5 years agoDocument log_transaction_sample_rate as superuser-only
Tomas Vondra [Mon, 4 Nov 2019 01:00:26 +0000 (02:00 +0100)]
Document log_transaction_sample_rate as superuser-only

The docs do say which GUCs can be changed only by superusers, but we
forgot to mention this for the new log_transaction_sample_rate. This
GUC was introduced in PostgreSQL 12, so backpatch accordingly.

Author: Adrien Nayrat
Backpatch-through: 12

5 years agoMinor code review for tuple slot rewrite.
Tom Lane [Wed, 6 Nov 2019 17:00:17 +0000 (12:00 -0500)]
Minor code review for tuple slot rewrite.

Avoid creating transiently-inconsistent slot states where possible,
by not setting TTS_FLAG_SHOULDFREE until after the slot actually has
a free'able tuple pointer, and by making sure that we reset tts_nvalid
and related derived state before we replace the tuple contents.  This
would only matter if something were to examine the slot after we'd
suffered some kind of error (e.g. out of memory) while manipulating
the slot.  We typically don't do that, so these changes might just be
cosmetic --- but even if so, it seems like good future-proofing.

Also remove some redundant Asserts, and add a couple for consistency.

Back-patch to v12 where all this code was rewritten.

Discussion: https://postgr.es/m/16095-c3ff2e5283b8dba5@postgresql.org

5 years agoSync our DTrace infrastructure with c.h's definition of type bool.
Tom Lane [Wed, 6 Nov 2019 16:11:40 +0000 (11:11 -0500)]
Sync our DTrace infrastructure with c.h's definition of type bool.

Since commit d26a810eb, we've defined bool as being either _Bool from
<stdbool.h>, or "unsigned char"; but that commit overlooked the fact
that probes.d has "#define bool char".  For consistency, make it say
"unsigned char" instead.  This should be strictly a cosmetic change,
but it seems best to be in sync.

Formally, in the now-normal case where we're using <stdbool.h>, it'd
be better to write "#define bool _Bool".  However, then we'd need
some build infrastructure to inject that configuration choice into
probes.d, and it doesn't seem worth the trouble.  We only use
<stdbool.h> if sizeof(_Bool) is 1, so having DTrace think that
bool parameters are "unsigned char" should be close enough.

Back-patch to v12 where d26a810eb came in.

Discussion: https://postgr.es/m/CAA4eK1LmaKO7Du9M9Lo=kxGU8sB6aL8fa3sF6z6d5yYYVe3BuQ@mail.gmail.com

5 years agoFix memory allocation mistake
Peter Eisentraut [Wed, 6 Nov 2019 13:20:29 +0000 (14:20 +0100)]
Fix memory allocation mistake

The previous code was allocating more memory than necessary because
the formula used the wrong data type.

Reported-by: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
Discussion: https://www.postgresql.org/message-id/20191105172918.3e32a446@firost

5 years agoRemove unused function argument
Peter Eisentraut [Wed, 6 Nov 2019 07:07:04 +0000 (08:07 +0100)]
Remove unused function argument

The cache_plan argument to ri_PlanCheck has not been used since
e8c9fd5fdf768323911f7088e8287f63b513c3c6.

Reviewed-by: vignesh C <vignesh21@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/ec8a8b45-a30b-9193-cd4b-985d60d1497e%402ndquadrant.com

5 years agoFix timestamp of sent message for write context in logical decoding
Michael Paquier [Wed, 6 Nov 2019 07:12:21 +0000 (16:12 +0900)]
Fix timestamp of sent message for write context in logical decoding

When sending data for logical decoding using the streaming replication
protocol via a WAL sender, the timestamp of the sent write message is
allocated at the beginning of the message when preparing for the write,
and actually computed when the write message is ready to be sent.

The timestamp was getting computed after sending the message.  This
impacts anything using logical decoding, causing for example logical
replication to report mostly NULL for last_msg_send_time in
pg_stat_subscription.

This commit makes sure that the timestamp is computed before sending the
message.  This is wrong since 5a991ef, so backpatch down to 9.4.

Author: Jeff Janes
Discussion: https://postgr.es/m/CAMkU=1z=WMn8jt7iEdC5sYNaPgAgOASb_OW5JYv-vMdYaJSL-w@mail.gmail.com
Backpatch-through: 9.4

5 years agoRequest small targetlist for input to WindowAgg.
Andrew Gierth [Wed, 6 Nov 2019 04:13:30 +0000 (04:13 +0000)]
Request small targetlist for input to WindowAgg.

WindowAgg will potentially store large numbers of input rows into
tuplestores to allow access to other rows in the frame. If the input
is coming via an explicit Sort node, then unneeded columns will
already have been discarded (since Sort requests a small tlist); but
there are idioms like COUNT(*) OVER () that result in the input not
being sorted at all, and cases where the input is being sorted by some
means other than a Sort; if we don't request a small tlist, then
WindowAgg's storage requirement is inflated by the unneeded columns.

Backpatch back to 9.6, where the current tlist handling was added.
(Prior to that, WindowAgg would always use a small tlist.)

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

5 years agoCorrect the command tags for ALTER ... RENAME COLUMN.
Fujii Masao [Wed, 6 Nov 2019 03:54:17 +0000 (12:54 +0900)]
Correct the command tags for ALTER ... RENAME COLUMN.

Previously ALTER MATERIALIZED VIEW / FOREIGN TABLE ... RENAME COLUMN ...
returned "ALTER TABLE" as a command tag. This commit fixes them so that
they return "ALTER MATERIALIZED VIEW" and "ALTER FOREIGN TABLE" as
command tags, respectively.

This issue exists in all supported versions, but we don't back-patch this
because it's not enough of a bug to justify taking any compatibility risks for.
Otherwise, the back-patch would cause minor version update to break,
for example, the existing event trigger functions using TG_TAG.

Author: Fujii Masao
Reviewed-by: Ibrar Ahmed
Discussion: https://postgr.es/m/CAHGQGwGUaC03FFdTFoHsCuDrrNvFvNVQ6xyd40==P25WvuBJjg@mail.gmail.com

5 years agoAdd "G" (server-side data generation) as an initialization step in pgbench.
Fujii Masao [Wed, 6 Nov 2019 02:02:30 +0000 (11:02 +0900)]
Add "G" (server-side data generation) as an initialization step in pgbench.

This commit allows --init-steps option in pgbench to accept "G" character
meaning server-side data generation as an initialization step.
With "G", only limited queries are sent from pgbench client and
then data is actually generated in the server. This might make
the initialization phase faster if the bandwidth between pgbench client
and the server is low.

Author: Fabien Coelho
Reviewed-by: Anna Endo, Ibrar Ahmed, Fujii Masao
Discussion: https://postgr.es/m/alpine.DEB.2.21.1904061826420.3678@lancre

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

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

Backpatch-through: 9.4

5 years agoMake StringInfo available to frontend code.
Andres Freund [Tue, 5 Nov 2019 22:56:40 +0000 (14:56 -0800)]
Make StringInfo available to frontend code.

There's plenty places in frontend code that could benefit from a
string buffer implementation. Some because it yields simpler and
faster code, and some others because of the desire to share code
between backend and frontend.

While there is a string buffer implementation available to frontend
code, libpq's PQExpBuffer, it is clunkier than stringinfo, it
introduces a libpq dependency, doesn't allow for sharing between
frontend and backend code, and has a higher API/ABI stability
requirement due to being exposed via libpq.

Therefore it seems best to just making StringInfo being usable by
frontend code. There's not much to do for that, except for rewriting
two subsequent elog/ereport calls into others types of error
reporting, and deciding on a maximum string length.

For the maximum string size I decided to privately define MaxAllocSize
to the same value as used in the backend. It seems likely that we'll
want to reconsider this for both backend and frontend code in the not
too far away future.

For now I've left stringinfo.h in lib/, rather than common/, to reduce
the likelihood of unnecessary breakage. We could alternatively decide
to provide a redirecting stringinfo.h in lib/, or just not provide
compatibility.

Author: Andres Freund
Reviewed-By: Kyotaro Horiguchi, Daniel Gustafsson
Discussion: https://postgr.es/m/20190920051857.2fhnvhvx4qdddviz@alap3.anarazel.de

5 years agoSplit all OBJS style lines in makefiles into one-line-per-entry style.
Andres Freund [Tue, 5 Nov 2019 22:41:07 +0000 (14:41 -0800)]
Split all OBJS style lines in makefiles into one-line-per-entry style.

When maintaining or merging patches, one of the most common sources
for conflicts are the list of objects in makefiles. Especially when
the split across lines has been changed on both sides, which is
somewhat common due to attempting to stay below 80 columns, those
conflicts are unnecessarily laborious to resolve.

By splitting, and alphabetically sorting, OBJS style lines into one
object per line, conflicts should be less frequent, and easier to
resolve when they still occur.

Author: Andres Freund
Discussion: https://postgr.es/m/20191029200901.vww4idgcxv74cwes@alap3.anarazel.de

5 years agoTweak some authentication debug messages to follow project style.
Tom Lane [Tue, 5 Nov 2019 19:29:08 +0000 (14:29 -0500)]
Tweak some authentication debug messages to follow project style.

Avoid initial capital, since that's not how we do it.

Discussion: https://postgr.es/m/CACP=ajbrFFYUrLyJBLV8=q+eNCapa1xDEyvXhMoYrNphs-xqPw@mail.gmail.com

5 years agoAvoid logging complaints about abandoned connections when using PAM.
Tom Lane [Tue, 5 Nov 2019 19:27:37 +0000 (14:27 -0500)]
Avoid logging complaints about abandoned connections when using PAM.

For a long time (since commit aed378e8d) we have had a policy to log
nothing about a connection if the client disconnects when challenged
for a password.  This is because libpq-using clients will typically
do that, and then come back for a new connection attempt once they've
collected a password from their user, so that logging the abandoned
connection attempt will just result in log spam.  However, this did
not work well for PAM authentication: the bottom-level function
pam_passwd_conv_proc() was on board with it, but we logged messages
at higher levels anyway, for lack of any reporting mechanism.
Add a flag and tweak the logic so that the case is silent, as it is
for other password-using auth mechanisms.

Per complaint from Yoann La Cancellera.  It's been like this for awhile,
so back-patch to all supported branches.

Discussion: https://postgr.es/m/CACP=ajbrFFYUrLyJBLV8=q+eNCapa1xDEyvXhMoYrNphs-xqPw@mail.gmail.com

5 years agoFix "unexpected relkind" error when denying permissions on toast tables.
Tom Lane [Tue, 5 Nov 2019 18:40:37 +0000 (13:40 -0500)]
Fix "unexpected relkind" error when denying permissions on toast tables.

get_relkind_objtype, and hence get_object_type, failed when applied to a
toast table.  This is not a good thing, because it prevents reporting of
perfectly legitimate permissions errors.  (At present, these functions
are in fact *only* used to determine the ObjectType argument for
acl_error() calls.)  It seems best to have them fall back to returning
OBJECT_TABLE in every case where they can't determine an object type
for a pg_class entry, so do that.

In passing, make some edits to alter.c to make it more obvious that
those calls of get_object_type() are used only for error reporting.
This might save a few cycles in the non-error code path, too.

Back-patch to v11 where this issue originated.

John Hsu, Michael Paquier, Tom Lane

Discussion: https://postgr.es/m/C652D3DF-2B0C-4128-9420-FB5379F6B1E4@amazon.com

5 years agoGenerate EquivalenceClass members for partitionwise child join rels.
Tom Lane [Tue, 5 Nov 2019 16:42:24 +0000 (11:42 -0500)]
Generate EquivalenceClass members for partitionwise child join rels.

Commit d25ea0127 got rid of what I thought were entirely unnecessary
derived child expressions in EquivalenceClasses for EC members that
mention multiple baserels.  But it turns out that some of the child
expressions that code created are necessary for partitionwise joins,
else we fail to find matching pathkeys for Sort nodes.  (This happens
only for certain shapes of the resulting plan; it may be that
partitionwise aggregation is also necessary to show the failure,
though I'm not sure of that.)

Reverting that commit entirely would be quite painful performance-wise
for large partition sets.  So instead, add code that explicitly
generates child expressions that match only partitionwise child join
rels we have actually generated.

Per report from Justin Pryzby.  (Amit Langote noticed the problem
earlier, though it's not clear if he recognized then that it could
result in a planner error, not merely failure to exploit partitionwise
join, in the code as-committed.)  Back-patch to v12 where commit
d25ea0127 came in.

Amit Langote, with lots of kibitzing from me

Discussion: https://postgr.es/m/CA+HiwqG2WVUGmLJqtR0tPFhniO=H=9qQ+Z3L_ZC+Y3-EVQHFGg@mail.gmail.com
Discussion: https://postgr.es/m/20191011143703.GN10470@telsasoft.com

5 years agoDoc: Clarify locks taken when using ALTER TABLE ATTACH PARTITION
Michael Paquier [Tue, 5 Nov 2019 01:32:38 +0000 (10:32 +0900)]
Doc: Clarify locks taken when using ALTER TABLE ATTACH PARTITION

Since 898e5e32, this command uses partially ShareUpdateExclusiveLock,
but the docs did not get the call.

Author: Justin Pryzby
Reviewed-by: Amit Langote, Álvaro Herrera, Michael Paquier
Discussion: https://postgr.es/m/20191028001207.GB23808@telsasoft.com
Backpatch-through: 12

5 years agoDoc: Improve description around ALTER TABLE ATTACH PARTITION
Michael Paquier [Tue, 5 Nov 2019 01:17:33 +0000 (10:17 +0900)]
Doc: Improve description around ALTER TABLE ATTACH PARTITION

This clarifies more how to use and how to take advantage of constraints
when attaching a new partition.

Author: Justin Pryzby
Reviewed-by: Amit Langote, Álvaro Herrera, Michael Paquier
Discussion: https://postgr.es/m/20191028001207.GB23808@telsasoft.com
Backpatch-through: 10

5 years agoRefactor code building relation options
Michael Paquier [Tue, 5 Nov 2019 00:17:05 +0000 (09:17 +0900)]
Refactor code building relation options

Historically, the code to build relation options has been shaped the
same way in multiple code paths by using a set of datums in input with
the options parsed with a static table which is then filled with the
option values.  This introduces a new common routine in reloptions.c to
do most of the legwork for the in-core code paths.

Author: Amit Langote
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/CA+HiwqGsoSn_uTPPYT19WrtR7oYpYtv4CdS0xuedTKiHHWuk_g@mail.gmail.com

5 years agoStabilize pg_dump output order for similarly-named triggers and policies.
Tom Lane [Mon, 4 Nov 2019 21:25:05 +0000 (16:25 -0500)]
Stabilize pg_dump output order for similarly-named triggers and policies.

The code only compared two triggers' names and namespaces (the latter
being the owning table's schema).  This could result in falling back
to an OID-based sort of similarly-named triggers on different tables.
We prefer to avoid that, so add a comparison of the table names too.
(The sort order is thus table namespace, trigger name, table name,
which is a bit odd, but it doesn't seem worth contorting the code
to work around that.)

Likewise for policy objects, in 9.5 and up.

Complaint and fix by Benjie Gillam.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/CAMThMzEEt2mvBbPgCaZ1Ap1N-moGn=Edxmadddjq89WG4NpPtQ@mail.gmail.com

5 years agoFix ginEntryInsert's counting of GIN leaf tuples.
Tom Lane [Mon, 4 Nov 2019 19:16:42 +0000 (14:16 -0500)]
Fix ginEntryInsert's counting of GIN leaf tuples.

As the code stands, nEntries counts the number of ginEntryInsert()
calls, so that's what you end up with at the end of a GIN index build.
However, ginvacuumcleanup() recomputes nEntries as the number of
surviving leaf tuples, and that's generally consistent with the way that
gincostestimate() uses the value.  So let's clearly define nEntries
as the number of leaf tuples, and therefore adjust ginEntryInsert() to
increment it only when we make a new one, not when we add TIDs into an
existing tuple or posting tree.

In practice this inconsistency probably has little impact, so I don't
feel a need to back-patch.

Insung Moon and Keisuke Kuroda

Discussion: https://postgr.es/m/CAEMmqBuH_O-oXL+3_ArQ6F5cJ7kXVow2SGQB3HRacku_T+xkmA@mail.gmail.com

5 years agoFix some compiler warnings on older compilers
Peter Eisentraut [Mon, 4 Nov 2019 10:07:32 +0000 (11:07 +0100)]
Fix some compiler warnings on older compilers

Some older compilers appear to not understand the recently introduced
PG_FINALLY code structure that well in some circumstances and complain
about possibly uninitialized variables.  So to fix, initialize the
variables explicitly in the cases complained about.

Discussion: https://www.postgresql.org/message-id/flat/95a822c3-728b-af0e-d7e5-71890507ae0c%402ndquadrant.com

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

Rearrange the logic in record_image_cmp() and datum_image_eq() to
error out on unexpected typlens (either not supported there or
completely invalid due to corruption).  Barring corruption, this is
not possible today but it seems more future-proof and robust to fix
this.

Reported-by: Peter Geoghegan <pg@bowt.ie>
5 years agoSuppress warning from older compilers.
Tom Lane [Sun, 3 Nov 2019 21:10:23 +0000 (16:10 -0500)]
Suppress warning from older compilers.

Commit 8af1624e3 introduced a warning about possibly returning
without a value, on compilers that don't realize that ereport(ERROR)
doesn't return.  Tweak the code to avoid that.

Per buildfarm.  Back-patch to 9.6, like the aforesaid commit.

5 years agoFix PG_GETARG_SEG_P() definition.
Tom Lane [Sun, 3 Nov 2019 15:57:49 +0000 (10:57 -0500)]
Fix PG_GETARG_SEG_P() definition.

DatumGetPointer() takes a Datum argument, not a pointer.
This is cosmetic given the current definitions of the
underlying macros, but it's still formally a type violation.

Bug was introduced in commit 389bb2818, but there seems
no need to back-patch.

Dagfinn Ilmari Mannsåker

Discussion: https://postgr.es/m/d8jlfsxq3a0.fsf@dalvik.ping.uio.no

5 years agoValidate ispell dictionaries more carefully.
Tom Lane [Sat, 2 Nov 2019 20:45:32 +0000 (16:45 -0400)]
Validate ispell dictionaries more carefully.

Using incorrect, or just mismatched, dictionary and affix files
could result in a crash, due to failure to cross-check offsets
obtained from the file.  Add necessary validation, as well as
some Asserts for future-proofing.

Per bug #16050 from Alexander Lakhin.  Back-patch to 9.6 where the
problem was introduced.

Arthur Zakirov, per initial investigation by Tomas Vondra

Discussion: https://postgr.es/m/16050-024ae722464ab604@postgresql.org
Discussion: https://postgr.es/m/20191013012610.2p2fp3zzpoav7jzf@development

5 years agoFix failure when creating cloned indexes for a partition
Michael Paquier [Sat, 2 Nov 2019 05:16:04 +0000 (14:16 +0900)]
Fix failure when creating cloned indexes for a partition

When using CREATE TABLE for a new partition, the partitioned indexes of
the parent are created automatically in a fashion similar to LIKE
INDEXES.  The new partition and its parent use a mapping for attribute
numbers for this operation, and while the mapping was correctly built,
its length was defined as the number of attributes of the newly-created
child, and not the parent.  If the parent includes dropped columns, this
could cause failures.

This is wrong since 8b08f7d which has introduced the concept of
partitioned indexes, so backpatch down to 11.

Reported-by: Wyatt Alt
Author: Michael Paquier
Reviewed-by: Amit Langote
Discussion: https://postgr.es/m/CAGem3qCcRmhbs4jYMkenYNfP2kEusDXvTfw-q+eOhM0zTceG-g@mail.gmail.com
Backpatch-through: 11

5 years agoAdd some assertions in syncrep.c
Michael Paquier [Fri, 1 Nov 2019 13:51:05 +0000 (22:51 +0900)]
Add some assertions in syncrep.c

A couple of routines assume that the LWLock SyncRepLock needs to be
taken, so add a couple of assertions to be sure of that.  Also, when
waiting for a given LSN at transaction commit, the code implied that the
syncrep queue cleanup happens while holding interrupts, but the code
never checked after that.

Author: Michael Paquier
Reviewed-by: Fujii Masao, Kyotaro Horiguchi, Dongming Liu
Discussion: https://postgr.es/m/a0806273-8bbb-43b3-bbe1-c45a58f6ae21.lingce.ldm@alibaba-inc.com

5 years agoFix race condition at backend exit when deleting element in syncrep queue
Michael Paquier [Fri, 1 Nov 2019 13:38:32 +0000 (22:38 +0900)]
Fix race condition at backend exit when deleting element in syncrep queue

When a backend exits, it gets deleted from the syncrep queue if present.
The queue was checked without SyncRepLock taken in exclusive mode, so it
would have been possible for a backend to remove itself after a WAL
sender already did the job.  Fix this issue based on a suggestion from
Fujii Masao, by first checking the queue without the lock.  Then, if the
backend is present in the queue, take the lock and perform an additional
lookup check before doing the element deletion.

Author: Dongming Liu
Reviewed-by: Kyotaro Horiguchi, Fujii Masao, Michael Paquier
Discussion: https://postgr.es/m/a0806273-8bbb-43b3-bbe1-c45a58f6ae21.lingce.ldm@alibaba-inc.com
Backpatch-through: 9.4

5 years agoAdd some assertions to view reloption macros
Peter Eisentraut [Fri, 1 Nov 2019 12:16:21 +0000 (13:16 +0100)]
Add some assertions to view reloption macros

In these macros, the rd_options pointer is cast to ViewOption *.  Add
some assertions that the passed-in relation is actually a view before
doing that.

Author: Nikolay Shaplov <dhyan@nataraj.su>
Discussion: https://www.postgresql.org/message-id/flat/3634983.eHpMQ1mJnI@x200m

5 years agoPG_FINALLY
Peter Eisentraut [Fri, 1 Nov 2019 10:09:52 +0000 (11:09 +0100)]
PG_FINALLY

This gives an alternative way of catching exceptions, for the common
case where the cleanup code is the same in the error and non-error
cases.  So instead of

    PG_TRY();
    {
        ... code that might throw ereport(ERROR) ...
    }
    PG_CATCH();
    {
        cleanup();
PG_RE_THROW();
    }
    PG_END_TRY();
    cleanup();

one can write

    PG_TRY();
    {
        ... code that might throw ereport(ERROR) ...
    }
    PG_FINALLY();
    {
        cleanup();
    }
    PG_END_TRY();

Discussion: https://www.postgresql.org/message-id/flat/95a822c3-728b-af0e-d7e5-71890507ae0c%402ndquadrant.com

5 years agoAdd const qualifiers to internal range type APIs
Peter Eisentraut [Thu, 31 Oct 2019 06:41:41 +0000 (07:41 +0100)]
Add const qualifiers to internal range type APIs

Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/flat/dc9b45fa-b950-fadc-4751-85d6f729df55%402ndquadrant.com

5 years agoFix typo in comment of syncrep.c
Michael Paquier [Thu, 31 Oct 2019 01:22:24 +0000 (10:22 +0900)]
Fix typo in comment of syncrep.c

Author: Kyotaro Horiguchi
Discussion: https://postgr.es/m/20191030.123428.18823202335157111.horikyota.ntt@gmail.com

5 years agoRemove one use of IDENT_USERNAME_MAX
Peter Eisentraut [Wed, 30 Oct 2019 10:01:44 +0000 (11:01 +0100)]
Remove one use of IDENT_USERNAME_MAX

IDENT_USERNAME_MAX is the maximum length of the information returned
by an ident server, per RFC 1413.  Using it as the buffer size in peer
authentication is inappropriate.  It was done here because of the
historical relationship between peer and ident authentication.  To
reduce confusion between the two authenticaton methods and disentangle
their code, use a dynamically allocated buffer instead.

Discussion: https://www.postgresql.org/message-id/flat/c798fba5-8b71-4f27-c78e-37714037ea31%402ndquadrant.com

5 years agoUpdate code comments about peer authenticaton
Peter Eisentraut [Wed, 30 Oct 2019 08:13:39 +0000 (09:13 +0100)]
Update code comments about peer authenticaton

For historical reasons, the functions for peer authentication were
grouped under ident authentication.  But they are really completely
separate, so give them their own section headings.

5 years agopg_waldump: Fix --bkp-details to not issue spurious newlines for FPWs.
Andres Freund [Wed, 30 Oct 2019 05:46:40 +0000 (22:46 -0700)]
pg_waldump: Fix --bkp-details to not issue spurious newlines for FPWs.

The additional newline seems to have accidentally been introduced in
2c03216d831, in 9.5. The newline is only issued when an FPW is
present for the block reference.

While there could be an argument that removing the newlines in the
back branches could cause a problem for somebody parsing the
pg_waldump output, the likelihood of that seems small enough. It seems
at least equally likely that the randomness of when newlines are
issued causes problems.

Author: Andres Freund
Discussion: https://postgr.es/m/20191029233341.4gnyau7e5v2lh5sc@alap3.anarazel.de
Backpatch: 9.5, like 2c03216d831.

5 years agopg_waldump: Fix small memory leak when rmgr->rm_identify returns NULL.
Andres Freund [Wed, 30 Oct 2019 02:18:07 +0000 (19:18 -0700)]
pg_waldump: Fix small memory leak when rmgr->rm_identify returns NULL.

This got broken in 604f7956b94, shortly after rm_identify's
introduction.

Author: Andres Freund
Discussion: https://postgr.es/m/20191029233341.4gnyau7e5v2lh5sc@alap3.anarazel.de
Backpatch: 9.5, where rm_identify was introduced

5 years agoFix typos in the code
Michael Paquier [Wed, 30 Oct 2019 01:03:00 +0000 (10:03 +0900)]
Fix typos in the code

Author: Vignesh C
Reviewed-by: Dilip Kumar, Michael Paquier
Discussion: https://postgr.es/m/CALDaNm0ni+GAOe4+fbXiOxNrVudajMYmhJFtXGX-zBPoN8ixhw@mail.gmail.com

5 years agoFix compiler warnings in ecpg tests
Peter Eisentraut [Tue, 29 Oct 2019 08:14:43 +0000 (09:14 +0100)]
Fix compiler warnings in ecpg tests

Under MinGW, when compiling the ecpg test files, you get compiler
warnings about the use of %lld in printf().

These files don't use our printf replacement or the c.h porting layer,
so determine the appropriate format conversion the hard way.

Reviewed-by: Michael Meskes <meskes@postgresql.org>
Discussion: https://www.postgresql.org/message-id/flat/760c9dd1-2d80-c223-3f90-609b615f7918%402ndquadrant.com

5 years agoFix handling of pg_class.relispartition at swap phase in REINDEX CONCURRENTLY
Michael Paquier [Tue, 29 Oct 2019 02:08:09 +0000 (11:08 +0900)]
Fix handling of pg_class.relispartition at swap phase in REINDEX CONCURRENTLY

When cancelling REINDEX CONCURRENTLY after swapping the old and new
indexes (for example interruption at step 5), the old index remains
around and is marked as invalid.  The old index should also be manually
droppable to clean up the parent relation from any invalid indexes still
remaining.  For a partition index reindexed, pg_class.relispartition was
not getting updated, causing the index to not be droppable as DROP INDEX
would look for dependencies in a partition tree, which do not exist
anymore after the swap phase is done.

The fix here is simple: when swapping the old and new indexes, make sure
that pg_class.relispartition is correctly switched, similarly to what is
done for the index name.

Reported-by: Justin Pryzby
Author: Michael Paquier
Discussion: https://postgr.es/m/20191015164047.GA22729@telsasoft.com
Backpatch-through: 12

5 years agoAllow extracting fields from a ROW() expression in more cases.
Tom Lane [Mon, 28 Oct 2019 19:08:24 +0000 (15:08 -0400)]
Allow extracting fields from a ROW() expression in more cases.

Teach get_expr_result_type() to manufacture a tuple descriptor directly
from a RowExpr node.  If the RowExpr has type RECORD, this is the only
way to get a tupdesc for its result, since even if the rowtype has been
blessed, we don't have its typmod available at this point.  (If the
RowExpr has some named composite type, we continue to let the existing
code handle it, since the RowExpr might well not have the correct column
names embedded in it.)

This fixes assorted corner cases illustrated by the added regression
tests.

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

5 years agoOn Windows, use COMSPEC to find the location of cmd.exe.
Tom Lane [Mon, 28 Oct 2019 18:15:03 +0000 (14:15 -0400)]
On Windows, use COMSPEC to find the location of cmd.exe.

Historically, psql consulted COMSPEC to spawn a shell in its \! command,
but we just invoked "cmd" when spawning shells in pg_ctl and pg_regress.
It seems better to rely on the environment variable, if it's set,
in all cases.

It's debatable whether this is a bug fix or just a behavioral change,
so no back-patch.

Juan José Santamaría Flecha

Discussion: https://postgr.es/m/16080-5d7f03222469f717@postgresql.org

5 years agoHandle empty-string edge cases correctly in strpos().
Tom Lane [Mon, 28 Oct 2019 16:21:13 +0000 (12:21 -0400)]
Handle empty-string edge cases correctly in strpos().

Commit 9556aa01c rearranged the innards of text_position() in a way
that would make it not work for empty search strings.  Which is fine,
because all callers of that code special-case an empty pattern in
some way.  However, the primary use-case (text_position itself) got
special-cased incorrectly: historically it's returned 1 not 0 for
an empty search string.  Restore the historical behavior.

Per complaint from Austin Drenski (via Shay Rojansky).
Back-patch to v12 where it got broken.

Discussion: https://postgr.es/m/CADT4RqAz7oN4vkPir86Kg1_mQBmBxCp-L_=9vRpgSNPJf0KRkw@mail.gmail.com

5 years agoDoc: Add missing step for pg_stat_progress_cluster
Michael Paquier [Mon, 28 Oct 2019 05:23:42 +0000 (14:23 +0900)]
Doc: Add missing step for pg_stat_progress_cluster

There is a step to track when the new heap is written, but this was
missing in the documentation.

Author: Noriyoshi Shinoda
Discussion: https://postgr.es/m/AT5PR8401MB06447FAE88E1592754E958B8EE640@AT5PR8401MB0644.NAMPRD84.PROD.OUTLOOK.COM
Backpatch-through: 12

5 years agoFix dependency handling at swap phase of REINDEX CONCURRENTLY
Michael Paquier [Mon, 28 Oct 2019 02:57:31 +0000 (11:57 +0900)]
Fix dependency handling at swap phase of REINDEX CONCURRENTLY

When swapping the dependencies of the old and new indexes, the code has
been correctly switching all links in pg_depend from the old to the new
index for both referencing and referenced entries.  However it forgot
the fact that the new index may itself have existing entries in
pg_depend, like references to the parent table attributes.  This
resulted in duplicated entries in pg_depend after running REINDEX
CONCURRENTLY.

Fix this problem by removing any existing entries in pg_depend on the
new index before switching the dependencies of the old index to the new
one.  More regression tests are added to check the consistency of
entries in pg_depend for indexes, including partition indexes.

Author: Michael Paquier
Discussion: https://postgr.es/m/20191025064318.GF8671@paquier.xyz
Backpatch-through: 12

5 years agoFix initialization of fake LSN for unlogged relations
Michael Paquier [Sun, 27 Oct 2019 04:54:12 +0000 (13:54 +0900)]
Fix initialization of fake LSN for unlogged relations

9155580 has changed the value of the first fake LSN for unlogged
relations from 1 to FirstNormalUnloggedLSN (aka 1000), GiST requiring a
non-zero LSN on some pages to allow an interlocking logic to work, but
its value was still initialized to 1 at the beginning of recovery or
after running pg_resetwal.  This fixes the initialization for both code
paths.

Author: Takayuki Tsunakawa
Reviewed-by: Dilip Kumar, Kyotaro Horiguchi, Michael Paquier
Discussion: https://postgr.es/m/OSBPR01MB2503CE851940C17DE44AE3D9FE6F0@OSBPR01MB2503.jpnprd01.prod.outlook.com
Backpatch-through: 12

5 years agoFix copy-paste defect in comment.
Noah Misch [Sat, 26 Oct 2019 19:55:16 +0000 (12:55 -0700)]
Fix copy-paste defect in comment.

Commit a7471bd85c05f849e88d6cfe9da3c795008e8f2e introduced it.

5 years agoUpdate comment about __sync_lock_test_and_set() bug.
Noah Misch [Sat, 26 Oct 2019 19:55:06 +0000 (12:55 -0700)]
Update comment about __sync_lock_test_and_set() bug.

State the earliest known fixed version, so we can someday judge the
workaround to be obsolete.

5 years agoDoc: improve documentation of configuration settings that have units.
Tom Lane [Sat, 26 Oct 2019 16:30:41 +0000 (12:30 -0400)]
Doc: improve documentation of configuration settings that have units.

When we added the GUC units feature, we didn't make any great effort
to adjust the documentation of individual GUCs; they tended to still
say things like "this is the number of milliseconds that ...", even
though users might prefer to write some other units, and SHOW might
even show the value in other units.  Commit 6c9fb69f2 made an effort
to improve this situation, but I thought it made things less readable
by injecting units information in mid-sentence.  It also wasn't very
consistent, and did not touch all the GUCs that have units.

To improve matters, standardize on the phrasing "If this value is
specified without units, it is taken as <units>".  Also, try to
standardize where this is mentioned, right before the specification
of the default.  (In a couple of places, doing that would've required
more rewriting than seemed justified, so I wasn't 100% consistent
about that.)  I also tried to use the phrases "amount of time",
"amount of memory", etc rather than describing the contents of GUCs
in other ways, as those were the majority usage in places that weren't
overcommitting to a particular unit.  (I left "length of time" alone
in a couple of places, though.)

I failed to resist the temptation to copy-edit some awkward text, too.

Backpatch to v12, like 6c9fb69f2, mainly because v12 hasn't diverged
much from HEAD yet.

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

5 years agoRemove obsolete information schema tables
Peter Eisentraut [Fri, 25 Oct 2019 19:11:48 +0000 (21:11 +0200)]
Remove obsolete information schema tables

Remove SQL_LANGUAGES, which was eliminated in SQL:2008, and
SQL_PACKAGES and SQL_SIZING_PROFILES, which were eliminated in
SQL:2011.  Since they were dropped by the SQL standard, the
information in them was no longer updated and therefore no longer
useful.

This also removes the feature-package association information in
sql_feature_packages.txt, but for the time begin we are keeping the
information which features are in the Core package (that is, mandatory
SQL features).  Maybe at some point someone wants to invent a way to
store that that does not involve using the "package" mechanism
anymore.

Discussion https://www.postgresql.org/message-id/flat/91334220-7900-071b-9327-0c6ecd012017%402ndquadrant.com

5 years agoAvoid failure when selecting a namespace node in XMLTABLE.
Tom Lane [Fri, 25 Oct 2019 19:22:40 +0000 (15:22 -0400)]
Avoid failure when selecting a namespace node in XMLTABLE.

It appears that libxml2 doesn't bother to set the "children" field of
an XML_NAMESPACE_DECL node to null; that field just contains garbage.
In v10 and v11, this can result in a crash in XMLTABLE().  The rewrite
done in commit 251cf2e27 fixed this, somewhat accidentally, in v12.
We're not going to back-patch 251cf2e27, however.  The case apparently
doesn't have wide use, so rather than risk introducing other problems,
just add a safety check to throw an error.

Even though no bug manifests in v12/HEAD, add the relevant test case
there too, to prevent future regressions.

Chapman Flack (per private report)

5 years agodoc: Use proper em and en dashes
Peter Eisentraut [Fri, 25 Oct 2019 18:23:44 +0000 (20:23 +0200)]
doc: Use proper em and en dashes

5 years agoRevert "Revert part of commit dddf4cdc3."
Tom Lane [Fri, 25 Oct 2019 16:18:11 +0000 (12:18 -0400)]
Revert "Revert part of commit dddf4cdc3."

This reverts commit c114229ca2519620703a4be4e38181290cad8c0a.
Commit 1408d5d869925c8ea7ca01c2644e8903fbab23de should make it
safe to include these headers in the natural order.

5 years agoGet rid of useless/dangerous redefinition of bool in ECPG.
Tom Lane [Fri, 25 Oct 2019 16:17:41 +0000 (12:17 -0400)]
Get rid of useless/dangerous redefinition of bool in ECPG.

pgtypeslib_extern.h contained fallback definitions of "bool", "FALSE",
and "TRUE".  The latter two are just plain unused, and have been for
awhile.  The former came into play only if there wasn't a macro
definition of "bool", which is true only if we aren't using <stdbool.h>.
However, it then defined bool as "char"; since commit d26a810eb that
conflicts with c.h's desire to use "unsigned char".  We'd missed seeing
any bad effects of that due to accidental header inclusion order choices,
but dddf4cdc3 exposed that it was problematic.

To fix, let's just get rid of these definitions.  They should not be
needed because everyplace in Postgres should be relying on c.h to
provide a definition for type bool.  (Note that despite its name,
pgtypeslib_extern.h isn't exposed to any outside code; we don't
install it.)

This doesn't fully resolve the issue, because ecpglib.h is doing
similar things, but that seems to require more thought to fix.

Back-patch to v12 where d26a810eb came in, to forestall any unpleasant
surprises from future back-patched bug fixes.

Discussion: https://postgr.es/m/CAA4eK1LmaKO7Du9M9Lo=kxGU8sB6aL8fa3sF6z6d5yYYVe3BuQ@mail.gmail.com

5 years agoImprove management of statement timeouts.
Tom Lane [Fri, 25 Oct 2019 15:41:16 +0000 (11:41 -0400)]
Improve management of statement timeouts.

Commit f8e5f156b added private state in postgres.c to track whether
a statement timeout is running.  This seems like bad design to me;
timeout.c's private state should be the single source of truth about
that.  We already fixed one bug associated with failure to keep those
states in sync (cf. be42015fc), and I've got little faith that we
won't find more in future.  So get rid of postgres.c's local variable
by exposing a way to ask timeout.c whether a timeout is running.
(Obviously, such an inquiry is subject to race conditions, but it
seems fine for the purpose at hand.)

To make get_timeout_active() as cheap as possible, add a flag in
the per-timeout struct showing whether that timeout is active.
This allows some small savings elsewhere in timeout.c, mainly
elimination of unnecessary searches of the active_timeouts array.

While at it, fix enable_statement_timeout to not call disable_timeout
when statement_timeout is 0 and the timeout is not running.  This
avoids a useless deschedule-and-reschedule-timeouts cycle, which
represents a significant savings (at least one kernel call) when
there is any other active timeout.  Right now, there usually isn't,
but there are proposals around to change that.

Discussion: https://postgr.es/m/16035-456e6e69ebfd4374@postgresql.org

5 years agoReset statement_timeout between queries of a multi-query string.
Tom Lane [Fri, 25 Oct 2019 15:15:50 +0000 (11:15 -0400)]
Reset statement_timeout between queries of a multi-query string.

Historically, we started the timer (if StatementTimeout > 0) at the
beginning of a simple-Query message and usually let it run until the
end, so that the timeout limit applied to the entire query string,
and intra-string changes of the statement_timeout GUC had no effect.
But, confusingly, a COMMIT within the string would reset the state
and allow a fresh timeout cycle to start with the current setting.

Commit f8e5f156b changed the behavior of statement_timeout for extended
query protocol, and as an apparently-unintended side effect, a change in
the statement_timeout GUC during a multi-statement simple-Query message
might have an effect immediately --- but only if it was going from
"disabled" to "enabled".

This is all pretty confusing, not to mention completely undocumented.
Let's change things so that the timeout is always reset between queries
of a multi-query string, whether they're transaction control commands
or not.  Thus the active timeout setting is applied to each query in
the string, separately.  This costs a few more cycles if statement_timeout
is active, but it provides much more intuitive behavior, especially if one
changes statement_timeout in one of the queries of the string.

Also, add something to the documentation to explain all this.

Per bug #16035 from Raj Mohite.  Although this is a bug fix, I'm hesitant
to back-patch it; conceivably somebody has worked out the old behavior
and is depending on it.  (But note that this change should make the
behavior less restrictive in most cases, since the timeout will now
be applied to shorter segments of code.)

Discussion: https://postgr.es/m/16035-456e6e69ebfd4374@postgresql.org

5 years agoRevert part of commit dddf4cdc3.
Amit Kapila [Fri, 25 Oct 2019 04:34:20 +0000 (10:04 +0530)]
Revert part of commit dddf4cdc3.

The commit dddf4cdc3 tries to ensure that the Postgres header file
inclusions are in order based on their ASCII value.  However, in one of
the case there is a header file dependency due to which we can't maintain
such order.

Author: Amit Kapila
Discussion: https://postgr.es/m/E1iNpHW-000855-1u@gemulon.postgresql.org

5 years agoMake the order of the header file includes consistent in non-backend modules.
Amit Kapila [Wed, 23 Oct 2019 04:08:53 +0000 (09:38 +0530)]
Make the order of the header file includes consistent in non-backend modules.

Similar to commit 7e735035f2, this commit makes the order of header file
inclusion consistent for non-backend modules.

In passing, fix the case where we were using angle brackets (<>) for the
local module includes instead of quotes ("").

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

5 years agoHandle interrupts within a transaction context in REINDEX CONCURRENTLY
Michael Paquier [Fri, 25 Oct 2019 01:20:08 +0000 (10:20 +0900)]
Handle interrupts within a transaction context in REINDEX CONCURRENTLY

Phases 2 (building the new index) and 3 (validating the new index)
checked for interrupts outside a transaction context, having as
consequence to not release session-level locks taken on the parent
relation and the old and new indexes processed.  This could for example
be triggered with statement_timeout and a bad timing, and would issue
confusing error messages when shutting down the session still holding
the locks (note that an assertion failure would be triggered first), on
top of more issues with concurrent sessions trying to take a lock that
would interfere with the SHARE UPDATE EXCLUSIVE locks hold here.

This moves all the interruption checks inside a transaction context.
Note that I have manually tested all interruptions to make sure that
invalid indexes can be cleaned up properly.  Partition indexes still
have issues on their own with some missing dependency handling, which
will be dealt with in a follow-up patch.

Reported-by: Justin Pryzby
Author: Michael Paquier
Discussion: https://postgr.es/m/20191013025145.GC4475@telsasoft.com
Backpatch-through: 12

5 years agoFix typo in xlog.c.
Fujii Masao [Thu, 24 Oct 2019 05:13:36 +0000 (14:13 +0900)]
Fix typo in xlog.c.

Author: Fujii Masao
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CAHGQGwH7dtYvOZZ8c0AG5AJwH5pfiRdKaCptY1_RdHy0HYeRfQ@mail.gmail.com

5 years agoMake the order of the header file includes consistent in contrib modules.
Amit Kapila [Wed, 23 Oct 2019 03:56:22 +0000 (09:26 +0530)]
Make the order of the header file includes consistent in contrib modules.

The basic rule we follow here is to always first include 'postgres.h' or
'postgres_fe.h' whichever is applicable, then system header includes and
then Postgres header includes.  In this, we also follow that all the
Postgres header includes are in order based on their ASCII value.  We
generally follow these rules, but the code has deviated in many places.
This commit makes it consistent just for contrib modules.  The later
commits will enforce similar rules in other parts of code.

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

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

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

Backpatch-through: 9.4

5 years agopg_upgrade: adjust error output to use new database list format
Bruce Momjian [Wed, 23 Oct 2019 22:06:38 +0000 (18:06 -0400)]
pg_upgrade:  adjust error output to use new database list format

Commit a524f50d0f added
old_11_check_for_sql_identifier_data_type_usage(), but it did not use
the clearer database error list format added to the master branch in
commit 1634d36157.  This commit fixes that.

Backpatch-through: master

5 years agoAcquire properly session-level lock on new index in REINDEX CONCURRENTLY
Michael Paquier [Wed, 23 Oct 2019 06:04:48 +0000 (15:04 +0900)]
Acquire properly session-level lock on new index in REINDEX CONCURRENTLY

In the first transaction run for REINDEX CONCURRENTLY, a thinko in the
existing logic caused two session locks to be taken on the old index,
causing the session lock on the newly-created index to be missed.  This
made possible concurrent DDL commands (like ALTER INDEX) on the new
index while REINDEX CONCURRENTLY was processing from the point where the
first internal transaction committed.

This issue has been discovered while digging into another bug.

Author: Michael Paquier
Discussion: https://postgr.es/m/20191021074323.GB1869@paquier.xyz
Backpatch-through: 12

5 years agoRemove libpq-dist.rc
Peter Eisentraut [Wed, 23 Oct 2019 05:10:09 +0000 (07:10 +0200)]
Remove libpq-dist.rc

The use of this was removed by
6da56f3f84d430671d5edd8f9336bd744c089e31.

Discussion: https://www.postgresql.org/message-id/87d95052-3780-b833-9953-27eab80186cf%402ndquadrant.com

5 years agoRemove last traces of --adduser/--no-adduser in createuser
Michael Paquier [Wed, 23 Oct 2019 03:27:03 +0000 (12:27 +0900)]
Remove last traces of --adduser/--no-adduser in createuser

8ae0d47 marked those options as obsolete back in 2005, with the options
removed from the documentation.  This removes the last references to
both options in the code which were kept around for compatibility
purposes with past commands.

Author: Alexander Lakhin
Discussion: https://postgr.es/m/5da284a2-62d9-e338-88d1-26ee5009d93e@gmail.com