postgresql.git
4 years agoTighten up Windows CRLF conversion in our TAP test scripts.
Tom Lane [Thu, 9 Jul 2020 21:38:52 +0000 (17:38 -0400)]
Tighten up Windows CRLF conversion in our TAP test scripts.

Back-patch commits 91bdf499b and ffb4cee43, so that all branches
agree on when and how to do Windows CRLF conversion.

This should close the referenced thread.  Thanks to Andrew Dunstan
for discussion/review.

Discussion: https://postgr.es/m/412ae8da-76bb-640f-039a-f3513499e53d@gmx.net

4 years agoFix pg_current_logfile() to not emit a carriage return on Windows.
Tom Lane [Thu, 9 Jul 2020 20:02:23 +0000 (16:02 -0400)]
Fix pg_current_logfile() to not emit a carriage return on Windows.

Due to not having our signals straight about CRLF vs. LF line
termination, the output of pg_current_logfile() included a trailing
\r on Windows.  To fix, force the file descriptor it uses into text
mode.

While here, move a couple of local variable declarations to make
the function's logic clearer.

In v12 and v13, also back-patch the test added by 1c4e88e2f so that
this function has some test coverage.  However, the 004_logrotate.pl
test script doesn't exist before v12, and it didn't seem worth adding
to older branches just for this.

Per report from Thomas Kellerer.  Back-patch to v10 where this
function was added.

Discussion: https://postgr.es/m/412ae8da-76bb-640f-039a-f3513499e53d@gmx.net

4 years agodoc: Correct the description about the length of pg_stat_activity.query.
Fujii Masao [Thu, 9 Jul 2020 04:31:33 +0000 (13:31 +0900)]
doc: Correct the description about the length of pg_stat_activity.query.

pg_stat_activity.query text is truncated at 1024 bytes. But previously
the document described that it's truncated at 1024 characters.
This was not accurate when considering multibyte characters.

Back-patch to v10 where this inaccurate description was added.

Author: Atsushi Torikoshi
Reviewed-by: Daniel Gustafsson, Fujii Masao
Discussion: https://postgr.es/m/cd5b49a5a14e887542f5f569c1c6bde2@oss.nttdata.com

4 years agoFix whitespace in HashAgg EXPLAIN ANALYZE
David Rowley [Wed, 8 Jul 2020 22:07:00 +0000 (10:07 +1200)]
Fix whitespace in HashAgg EXPLAIN ANALYZE

The Sort node does not put a space between the number of kilobytes and
the "kB" of memory or disk space used, but HashAgg does.  Here we align
HashAgg to do the same as Sort.  Sort has been displaying this
information for longer than HashAgg, so it makes sense to align HashAgg
to Sort rather than the other way around.

Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/20200708163021[email protected]
Backpatch-through: 13, where the hashagg started showing these details

4 years agoFix incorrect variable datatype.
Fujii Masao [Wed, 8 Jul 2020 12:24:34 +0000 (21:24 +0900)]
Fix incorrect variable datatype.

Since slot_keep_segs indicates the number of WAL segments not LSN,
its datatype should not be XLogRecPtr.

Back-patch to v13 where this issue was added.

Reported-by: Atsushi Torikoshi
Author: Atsushi Torikoshi, tweaked by Fujii Masao
Discussion: https://postgr.es/m/ebd0d674f3e050222238a960cac5251a@oss.nttdata.com

4 years agodoc: Fix inconsistencies in GIN, BRIN and SP-GiST for optional opclass methods
Michael Paquier [Wed, 8 Jul 2020 01:42:15 +0000 (10:42 +0900)]
doc: Fix inconsistencies in GIN, BRIN and SP-GiST for optional opclass methods

The GIN and SP-GiST parts were out-of-sync since the changes of 14903f2,
and the BRIN part was wrong since its introduction in 15cb2bd.

Author: Guillaume Lelarge
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/CAECtzeXKvEPEr967h0PRYRi39uTmdEms=oUtc_PWGjZRNN1prw@mail.gmail.com
Backpatch-through: 13

4 years agoMorph pg_replication_slots.min_safe_lsn to safe_wal_size
Alvaro Herrera [Tue, 7 Jul 2020 17:08:00 +0000 (13:08 -0400)]
Morph pg_replication_slots.min_safe_lsn to safe_wal_size

The previous definition of the column was almost universally disliked,
so provide this updated definition which is more useful for monitoring
purposes: a large positive value is good, while zero or a negative value
means danger.  This should be operationally more convenient.

Backpatch to 13, where the new column to pg_replication_slots (and the
feature it represents) were added.

Author: Kyotaro Horiguchi 
Author: Álvaro Herrera 
Reported-by: Fujii Masao
Discussion: https://postgr.es/m/9ddfbf8c-2f67-904d-44ed-cf8bc5916228@oss.nttdata.com

4 years agodoc: Add note about possible performance overhead by enabling track_planning.
Fujii Masao [Mon, 6 Jul 2020 05:27:09 +0000 (14:27 +0900)]
doc: Add note about possible performance overhead by enabling track_planning.

Enabling pg_stat_statements.track_plaanning may incur a noticeable
performance penalty, especially when a fewer kinds of queries are executed
on many concurrent connections. This commit documents this note.

Back-patch to v13 where pg_stat_statements.track_plaanning was added.

Suggested-by: Pavel Stehule
Author: Fujii Masao
Reviewed-by: Pavel Stehule
Discussion: https://postgr.es/m/CAFj8pRC9Jxa8r5i0TNBWLb8mzuaYzEoLq3QOvip0jVpHPOLbVA@mail.gmail.com

4 years agoRemove extra whitespace in comments atop ReorderBufferCheckMemoryLimit.
Amit Kapila [Mon, 6 Jul 2020 03:06:58 +0000 (08:36 +0530)]
Remove extra whitespace in comments atop ReorderBufferCheckMemoryLimit.

Backpatch-through: 13, where it was introduced

4 years agoRemove unused function parameter in end_parallel_vacuum.
Amit Kapila [Mon, 6 Jul 2020 02:54:12 +0000 (08:24 +0530)]
Remove unused function parameter in end_parallel_vacuum.

Author: Vignesh C
Reviewed-by: Sawada Masahiko
Backpatch-through: 13, where it was introduced
Discussion: https://postgr.es/m/CALDaNm3Ppt71NafGY5mk3V2i3Q+mm93pVibDq-0NpW7WU67Jcg@mail.gmail.com

4 years agodoc: Spell checking
Peter Eisentraut [Sun, 5 Jul 2020 13:37:57 +0000 (15:37 +0200)]
doc: Spell checking

4 years agodoc: Fix incorrect reference to textout in plpgsql examples
Michael Paquier [Sun, 5 Jul 2020 10:36:12 +0000 (19:36 +0900)]
doc: Fix incorrect reference to textout in plpgsql examples

This error has survived for 22 years, and has been introduced by
da63386.

Reported-by: Erwin Brandstetter
Discussion: https://postgr.es/m/CAGHENJ57wogGOvGXo5LgWYcqswxafLck8ELqHDR+zrkTPgs_OQ@mail.gmail.com
Backpatch-through: 9.5

4 years agoRename enable_incrementalsort for clarity
Peter Eisentraut [Sun, 5 Jul 2020 09:41:52 +0000 (11:41 +0200)]
Rename enable_incrementalsort for clarity

Author: James Coleman 
Discussion: https://www.postgresql.org/message-id/flat/df652910-e985-9547-152c-9d4357dc3979%402ndquadrant.com

4 years agoFix "ignoring return value" complaints from commit 96d1f423f9
Joe Conway [Sat, 4 Jul 2020 17:47:07 +0000 (13:47 -0400)]
Fix "ignoring return value" complaints from commit 96d1f423f9

The cfbot and some BF animals are complaining about the previous
read_binary_file commit because of ignoring return value of ‘fread’.
So let's make everyone happy by testing the return value even though
not strictly needed.

Reported by Justin Pryzby, and suggested patch by Tom Lane. Backpatched
to v11 same as the previous commit.

Reported-By: Justin Pryzby
Reviewed-By: Tom Lane
Discussion: https://postgr.es/m/flat/969b8d82-5bb2-5fa8-4eb1-f0e685c5d736%40joeconway.com
Backpatch-through: 11

4 years agoRead until EOF vice stat-reported size in read_binary_file
Joe Conway [Sat, 4 Jul 2020 10:28:21 +0000 (06:28 -0400)]
Read until EOF vice stat-reported size in read_binary_file

read_binary_file(), used by SQL functions pg_read_file() and friends,
uses stat to determine file length to read, when not passed an explicit
length as an argument. This is problematic, for example, if the file
being read is a virtual file with a stat-reported length of zero.
Arrange to read until EOF, or StringInfo data string lenth limit, is
reached instead.

Original complaint and patch by me, with significant review, corrections,
advice, and code optimizations by Tom Lane. Backpatched to v11. Prior to
that only paths relative to the data and log dirs were allowed for files,
so no "zero length" files were reachable anyway.

Reviewed-By: Tom Lane
Discussion: https://postgr.es/m/flat/969b8d82-5bb2-5fa8-4eb1-f0e685c5d736%40joeconway.com
Backpatch-through: 11

4 years agoClamp total-tuples estimates for foreign tables to ensure planner sanity.
Tom Lane [Fri, 3 Jul 2020 23:01:21 +0000 (19:01 -0400)]
Clamp total-tuples estimates for foreign tables to ensure planner sanity.

After running GetForeignRelSize for a foreign table, adjust rel->tuples
to be at least as large as rel->rows.  This prevents bizarre behavior
in estimate_num_groups() and perhaps other places, especially in the
scenario where rel->tuples is zero because pg_class.reltuples is
(suggesting that ANALYZE has never been run for the table).  As things
stood, we'd end up estimating one group out of any GROUP BY on such a
table, whereas the default group-count estimate is more likely to result
in a sane plan.

Also, clarify in the documentation that GetForeignRelSize has the option
to override the rel->tuples value if it has a better idea of what to use
than what is in pg_class.reltuples.

Per report from Jeff Janes.  Back-patch to all supported branches.

Patch by me; thanks to Etsuro Fujita for review

Discussion: https://postgr.es/m/CAMkU=1xNo9cnan+Npxgz0eK7394xmjmKg-QEm8wYG9P5-CcaqQ@mail.gmail.com

4 years agoFix temporary tablespaces for shared filesets some more.
Tom Lane [Fri, 3 Jul 2020 21:01:34 +0000 (17:01 -0400)]
Fix temporary tablespaces for shared filesets some more.

Commit ecd9e9f0b fixed the problem in the wrong place, causing unwanted
side-effects on the behavior of GetNextTempTableSpace().  Instead,
let's make SharedFileSetInit() responsible for subbing in the value
of MyDatabaseTableSpace when the default tablespace is called for.

The convention about what is in the tempTableSpaces[] array is
evidently insufficiently documented, so try to improve that.

It also looks like SharedFileSetInit() is doing the wrong thing in the
case where temp_tablespaces is empty.  It was hard-wiring use of the
pg_default tablespace, but it seems like using MyDatabaseTableSpace
is more consistent with what happens for other temp files.

Back-patch the reversion of PrepareTempTablespaces()'s behavior to
9.5, as ecd9e9f0b was.  The changes in SharedFileSetInit() go back
to v11 where that was introduced.  (Note there is net zero code change
before v11 from these two patch sets, so nothing to release-note.)

Magnus Hagander and Tom Lane

Discussion: https://postgr.es/m/CABUevExg5YEsOvqMxrjoNvb3ApVyH+9jggWGKwTDFyFCVWczGQ@mail.gmail.com

4 years agoFix temporary tablespaces for shared filesets
Magnus Hagander [Fri, 3 Jul 2020 13:09:06 +0000 (15:09 +0200)]
Fix temporary tablespaces for shared filesets

A likely copy/paste error in 98e8b480532 from  back in 2004 would
cause temp tablespace to be reset to InvalidOid if temp_tablespaces
was set to the same value as the primary tablespace in the database.
This would cause shared filesets (such as for parallel hash joins)
to ignore them, putting the temporary files in the default tablespace
instead of the configured one. The bug is in the old code, but it
appears to have been exposed only once we had shared filesets.

Reviewed-By: Daniel Gustafsson
Discussion: https://postgr.es/m/CABUevExg5YEsOvqMxrjoNvb3ApVyH+9jggWGKwTDFyFCVWczGQ@mail.gmail.com
Backpatch-through: 9.5

4 years agodoc: Correct description of restart_lsn in pg_replication_slots
Fujii Masao [Fri, 3 Jul 2020 03:08:35 +0000 (12:08 +0900)]
doc: Correct description of restart_lsn in pg_replication_slots

Previously the document explained that restart_lsn indicates the LSN of
oldest WAL won't be automatically removed during checkpoints. But
since v13 this was no longer true thanks to max_slot_wal_keep_size.

Back-patch to v13 where max_slot_wal_keep_size was added.

Author: Fujii Masao
Discussion: https://postgr.es/m/6497f1e9-3148-c5da-7e49-b2fddad9a42f@oss.nttdata.com

4 years agoChange default of pg_stat_statements.track_planning to off.
Fujii Masao [Fri, 3 Jul 2020 02:35:22 +0000 (11:35 +0900)]
Change default of pg_stat_statements.track_planning to off.

Since v13 pg_stat_statements is allowed to track the planning time of
statements when track_planning option is enabled. Its default was on.

But this feature could cause more terrible spinlock contentions in
pg_stat_statements. As a result of this, Robins Tharakan reported that
v13 beta1 showed ~45% performance drop at high DB connection counts
(when compared with v12.3) during fully-cached SELECT-only test using
pgbench.

To avoid this performance regression by the default setting,
this commit changes default of pg_stat_statements.track_planning to off.

Back-patch to v13 where pg_stat_statements.track_planning was introduced.

Reported-by: Robins Tharakan
Author: Fujii Masao
Reviewed-by: Julien Rouhaud
Discussion: https://postgr.es/m/2895b53b033c47ccb22972b589050dd9@EX13D05UWC001.ant.amazon.com

4 years agoImprove vacuum error context handling.
Amit Kapila [Wed, 1 Jul 2020 02:36:00 +0000 (08:06 +0530)]
Improve vacuum error context handling.

Use separate functions to save and restore error context information as
that made code easier to understand.  Also, make it clear that the index
information required for error context is sane.

Author: Andres Freund, Justin Pryzby, Amit Kapila
Backpatch-through: 13, where it was introduced
Discussion: https://postgr.es/m/CAA4eK1LWo+v1OWu=Sky27GTGSCuOmr7iaURNbc5xz6jO+SaPeA@mail.gmail.com

4 years agoFix removal of files generated by TAP tests for SSL
Michael Paquier [Wed, 1 Jul 2020 01:47:29 +0000 (10:47 +0900)]
Fix removal of files generated by TAP tests for SSL

001_ssltests.pl and 002_scram.pl both generated an extra file for a
client key used in the tests that were not removed.  In Debian, this
causes repeated builds to fail.

The code refactoring done in 4dc6355 broke the cleanup done in
001_ssltests.pl, and the new tests added in 002_scram.pl via d6e612f
forgot the removal of one file.  While on it, fix a second issue
introduced in 002_scram.pl where we use the same file name in 001 and
002 for the temporary client key whose permissions are changed in the
test, as using the same file name in both tests could cause failures
with parallel jobs of src/test/ssl/ if one test removes a file still
needed by the second test.

Reported-by: Felix Lechner
Author: Daniel Gustafsson, Felix Lechner
Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/CAFHYt543sjX=Cm_aEeoejStyP47C+Y3+Wh6WbirLXsgUMaw7iw@mail.gmail.com
Backpatch-through: 13

4 years agoFurther adjustments to Hashagg EXPLAIN ANALYZE output
David Rowley [Wed, 1 Jul 2020 00:16:42 +0000 (12:16 +1200)]
Further adjustments to Hashagg EXPLAIN ANALYZE output

The "Disk Usage" and "HashAgg Batches" properties in the EXPLAIN ANALYZE
output for HashAgg were previously only shown if the number of batches
was greater than 0.  Here we change this so that these properties are
always shown for EXPLAIN ANALYZE formats other than "text".  The idea here
is that since the HashAgg could have spilled to disk if there had been
more data or groups to aggregate, then it's relevant that we're clear in
the EXPLAIN ANALYZE output when no spilling occurred in this particular
execution of the given plan.

For the "text" EXPLAIN format, we still hide these properties when no
spilling occurs.  This EXPLAIN format is designed to be easy for humans
to read.  To maintain the readability we have a higher threshold for which
properties we display for this format.

Discussion: https://postgr.es/m/CAApHDvo_dmNozQQTmN-2jGp1vT%3Ddxx7Q0vd%2BMvD1cGpv2HU%3DSg%40mail.gmail.com
Backpatch-through: 13, where the hashagg spilling code was added.

4 years agoFix ecpg crash with bytea and cursor variables.
Michael Meskes [Tue, 30 Jun 2020 15:31:08 +0000 (17:31 +0200)]
Fix ecpg crash with bytea and cursor variables.

Author: Jehan-Guillaume de Rorthais 

4 years agodoc: clarify that storage parameter values are optional
Bruce Momjian [Tue, 30 Jun 2020 16:26:51 +0000 (12:26 -0400)]
doc:  clarify that storage parameter values are optional

In a few cases, the documented syntax specified storage parameter values
as required.

Reported-by: [email protected]
Discussion: https://postgr.es/m/159283163235.684.4482737698910467437@wrigleys.postgresql.org

Backpatch-through: 9.5

4 years agodoc: change pg_upgrade wal_level to be not minimal
Bruce Momjian [Tue, 30 Jun 2020 15:55:53 +0000 (11:55 -0400)]
doc: change pg_upgrade wal_level to be not minimal

Previously it was specified to be only replica.

Discussion: https://postgr.es/m/20200618180058[email protected]

Backpatch-through: 9.5

4 years agoRemove support for timezone "posixrules" file.
Tom Lane [Mon, 29 Jun 2020 22:55:01 +0000 (18:55 -0400)]
Remove support for timezone "posixrules" file.

The IANA tzcode library has a feature to read a time zone file named
"posixrules" and apply the daylight-savings transition dates and times
therein, when it is given a POSIX-style time zone specification that
lacks an explicit transition rule.  However, there's a problem with
that code: it doesn't work for dates past the Y2038 time_t rollover.
(Effectively, all times beyond that point are treated as standard
time.)  The IANA crew regard this feature as legacy, so their plan is
to remove it not fix it.  The time frame in which that will happen
is unclear, but presumably it'll happen well before 2038.

Moreover, effective with the next IANA data update (probably this
fall), the recommended default will be to not install a "posixrules"
file in the first place.  The time frame in which tzdata packagers
might adopt that suggestion is likewise unclear, but at least some
platforms will probably do it in the next year or so.  While we could
ignore that recommendation so far as PG-supplied tzdata trees are
concerned, builds using --with-system-tzdata will be subject to
whatever the platform's tzdata packager decides to do.

Thus, whether or not we do anything, some increasing fraction of
Postgres users will be exposed to the behavior observed when there
is no "posixrules" file; and if we do nothing, we'll have essentially
no control over the timing of that change.

The best thing to do to ameliorate the uncertainty seems to be to
proactively remove the posixrules-reading feature.  If we do that in
a scheduled release then at least we can release-note the behavioral
change, rather than having users be surprised by it after a routine
tzdata update.

The change in question is fairly minor anyway: to be affected,
you have to be using a POSIX-style timezone spec, it has to not
have an explicit rule, and it has to not be one of the four traditional
continental-USA zone names (EST5EDT, CST6CDT, MST7MDT, or PST8PDT),
as those are special-cased.  Since the default "posixrules" file
provides USA DST rules, the number of people who are likely to find
such a zone spec useful is probably quite small.  Moreover, the
fallback behavior with no explicit rule and no "posixrules" file is to
apply current USA rules, so the only thing that really breaks is the
DST transitions in years before 2007 (and you get the countervailing
fix that transitions after 2038 will be applied).

Now, some installations might have replaced the "posixrules" file,
allowing e.g. EU rules to be applied to a POSIX-style timezone spec.
That won't work anymore.  But it's not exactly clear why this solution
would be preferable to using a regular named zone.  In any case, given
the Y2038 issue, we need to be pushing users to stop depending on this.

Back-patch into v13; it hasn't been released yet, so it seems OK to
change its behavior.  (Personally I think we ought to back-patch
further, but I've been outvoted.)

Discussion: https://postgr.es/m/1390.1562258309@sss.pgh.pa.us
Discussion: https://postgr.es/m/20200621211855[email protected]

5 years agoFix documentation of "must be vacuumed within" warning.
Noah Misch [Sun, 28 Jun 2020 05:05:04 +0000 (22:05 -0700)]
Fix documentation of "must be vacuumed within" warning.

Warnings start 10M transactions before xidStopLimit, which is 11M
transactions before wraparound.  The sample WARNING output showed a
value greater than 11M, and its HINT message predated commit
25ec228ef760eb91c094cc3b6dea7257cc22ffb5.  Hence, the sample was
impossible.  Back-patch to 9.5 (all supported versions).

5 years agoFix list of SSL error codes for older OpenSSL versions.
Tom Lane [Sat, 27 Jun 2020 17:26:17 +0000 (13:26 -0400)]
Fix list of SSL error codes for older OpenSSL versions.

Apparently 1.0.1 lacks SSL_R_VERSION_TOO_HIGH and
SSL_R_VERSION_TOO_LOW.  Per buildfarm.

5 years agoAdd hints about protocol-version-related SSL connection failures.
Tom Lane [Sat, 27 Jun 2020 16:47:58 +0000 (12:47 -0400)]
Add hints about protocol-version-related SSL connection failures.

OpenSSL's native reports about problems related to protocol version
restrictions are pretty opaque and inconsistent.  When we get an
SSL error that is plausibly due to this, emit a hint message that
includes the range of SSL protocol versions we (think we) are
allowing.  This should at least get the user thinking in the right
direction to resolve the problem, even if the hint isn't totally
accurate, which it might not be for assorted reasons.

Back-patch to v13 where we increased the default minimum protocol
version, thereby increasing the risk of this class of failure.

Patch by me, reviewed by Daniel Gustafsson

Discussion: https://postgr.es/m/a9408304-4381-a5af-d259-e55d349ae4ce@2ndquadrant.com

5 years agoChange libpq's default ssl_min_protocol_version to TLSv1.2.
Tom Lane [Sat, 27 Jun 2020 16:20:33 +0000 (12:20 -0400)]
Change libpq's default ssl_min_protocol_version to TLSv1.2.

When we initially created this parameter, in commit ff8ca5fad, we left
the default as "allow any protocol version" on grounds of backwards
compatibility.  However, that's inconsistent with the backend's default
since b1abfec82; protocol versions prior to 1.2 are not considered very
secure; and OpenSSL has had TLSv1.2 support since 2012, so the number
of PG servers that need a lesser minimum is probably quite small.

On top of those things, it emerges that some popular distros (including
Debian and RHEL) set MinProtocol=TLSv1.2 in openssl.cnf.  Thus, far
from having "allow any protocol version" behavior in practice, what
we actually have as things stand is a platform-dependent lower limit.

So, change our minds and set the min version to TLSv1.2.  Anybody
wanting to connect with a new libpq to a pre-2012 server can either
set ssl_min_protocol_version=TLSv1 or accept the fallback to non-SSL.

Back-patch to v13 where the aforementioned patches appeared.

Patch by me, reviewed by Daniel Gustafsson

Discussion: https://postgr.es/m/a9408304-4381-a5af-d259-e55d349ae4ce@2ndquadrant.com

5 years agoPersist slot invalidation correctly
Alvaro Herrera [Sat, 27 Jun 2020 00:41:29 +0000 (20:41 -0400)]
Persist slot invalidation correctly

We failed to save slot to disk after invalidating it, so the state was
lost in case of server restart or crash.  Fix by marking it dirty and
flushing.

Also, if the slot is known invalidated we don't need to reason about the
LSN at all -- it's known invalidated.  Only test the LSN if the slot is
known not invalidated.

Author: Fujii Masao 
Author: Kyotaro Horiguchi 
Reviewed-by: Álvaro Herrera
Discussion: https://postgr.es/m/17a69cfe-f1c1-a416-ee25-ae15427c69eb@oss.nttdata.com

5 years agodoc: PG 13 relnotes; remove FOREIGN keyword item and clarify
Bruce Momjian [Fri, 26 Jun 2020 22:24:12 +0000 (18:24 -0400)]
doc: PG 13 relnotes;  remove FOREIGN keyword item and clarify

Clarify --include-foreign-data option addition.

Reported-by: Masahiko Sawada, Alvaro Herrera
Discussion: https://postgr.es/m/CA+fd4k62hYtce8VrEMGm6Y+1c24QBgCksXvOaH5kE8PbY+68sA@mail.gmail.com

Backpatch-through: 13 only

5 years agoDoc: explain that "timestamp - timestamp" applies justify_hours().
Tom Lane [Fri, 26 Jun 2020 17:54:01 +0000 (13:54 -0400)]
Doc: explain that "timestamp - timestamp" applies justify_hours().

Back-patch to v13; before that, there's not really space for this
kind of detail.

Discussion: https://postgr.es/m/c1696f68-fa8d-7759-6a9c-eb293ab1bbc9@gmx.net

5 years agodoc: mention trigger helper functions in CREATE TRIGGER docs
Bruce Momjian [Thu, 25 Jun 2020 22:33:28 +0000 (18:33 -0400)]
doc:  mention trigger helper functions in CREATE TRIGGER docs

Reported-by: [email protected]
Discussion: https://postgr.es/m/159195294959.673.5752624528747900508@wrigleys.postgresql.org

Backpatch-through: 9.5

5 years agodocs: clarify that CREATE DATABASE does not copy db permissions
Bruce Momjian [Thu, 25 Jun 2020 22:22:44 +0000 (18:22 -0400)]
docs:  clarify that CREATE DATABASE does not copy db permissions

That is, those database permissions set by GRANT.

Diagnosed-by: Joseph Nahmias
Discussion: https://postgr.es/m/20200614072613[email protected]

Backpatch-through: 9.5

5 years agoFix misuse of table_index_fetch_tuple_check().
Peter Geoghegan [Thu, 25 Jun 2020 17:55:26 +0000 (10:55 -0700)]
Fix misuse of table_index_fetch_tuple_check().

Commit 0d861bbb, which added deduplication to nbtree, had
_bt_check_unique() pass a TID to table_index_fetch_tuple_check() that
isn't safe to mutate.  table_index_fetch_tuple_check()'s tid argument is
modified when the TID in question is not the latest visible tuple in a
hot chain, though this wasn't documented.

To fix, go back to using a local copy of the TID in _bt_check_unique(),
and update comments above table_index_fetch_tuple_check().

Backpatch: 13-, where B-Tree deduplication was introduced.

5 years agoDoc: correct nitpicky mistakes in array_position/array_positions examples.
Tom Lane [Thu, 25 Jun 2020 17:28:30 +0000 (13:28 -0400)]
Doc: correct nitpicky mistakes in array_position/array_positions examples.

Daniel Gustafsson and Erik Rijkers, per report from nick@cleaton

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

5 years agoRemove erroneous assertion from pg_copy_logical_replication_slot().
Fujii Masao [Thu, 25 Jun 2020 02:13:13 +0000 (11:13 +0900)]
Remove erroneous assertion from pg_copy_logical_replication_slot().

If restart_lsn of logical replication slot gets behind more than
max_slot_wal_keep_size from the current LSN, the logical replication slot
would be invalidated and its restart_lsn is reset to an invalid LSN.
If this logical replication slot with an invalid restart_lsn was specified as
the source slot in pg_copy_logical_replication_slot(), the function caused
the assertion failure unexpectedly.

This assertion was added because restart_lsn should not be invalid before.
But in v13, it can be invalid thanks to max_slot_wal_keep_size. So since this
assertion is no longer useful, this commit removes it.

This commit also changes the errcode in the error message that
pg_copy_logical_replication_slot() emits when the slot with an invalid
restart_lsn is specified, to more appropriate one.

Back-patch to v13 where max_slot_wal_keep_size was added and
the assertion was no longer valid.

Author: Fujii Masao
Reviewed-by: Alvaro Herrera, Kyotaro Horiguchi
Discussion: https://postgr.es/m/f91de4fb-a7ab-b90e-8132-74796e049d51@oss.nttdata.com

5 years agoFix compiler warning induced by commit d8b15eeb8.
Tom Lane [Wed, 24 Jun 2020 19:47:30 +0000 (15:47 -0400)]
Fix compiler warning induced by commit d8b15eeb8.

I forgot that INT64_FORMAT can't be used with sscanf on Windows.
Use the same trick of sscanf'ing into a temp variable as we do in
some other places in zic.c.

The upstream IANA code avoids the portability problem by relying on
's SCNdFAST64 macro.  Once we're requiring C99 in all
branches, we should do likewise and drop this set of diffs from
upstream.  For now, though, a hack seems fine, since we do not
actually care about leapseconds anyway.

Discussion: https://postgr.es/m/4e5d1a5b-143e-e70e-a99d-a3b01c1ae7c3@2ndquadrant.com

5 years agoAdjust max_slot_wal_keep_size behavior per review
Alvaro Herrera [Wed, 24 Jun 2020 18:23:39 +0000 (14:23 -0400)]
Adjust max_slot_wal_keep_size behavior per review

In pg_replication_slot, change output from normal/reserved/lost to
reserved/extended/unreserved/ lost, which better expresses the possible
states particularly near the time where segments are no longer safe but
checkpoint has not run yet.

Under the new definition, reserved means the slot is consuming WAL
that's still under the normal WAL size constraints; extended means it's
consuming WAL that's being protected by wal_keep_segments or the slot
itself, whose size is below max_slot_wal_keep_size; unreserved means the
WAL is no longer safe, but checkpoint has not yet removed those files.
Such as slot is in imminent danger, but can still continue for a little
while and may catch up to the reserved WAL space.

Also, there were some bugs in the calculations used to report the
status; fixed those.

Backpatch to 13.

Reported-by: Fujii Masao
Author: Kyotaro Horiguchi 
Reviewed-by: Fujii Masao
Reviewed-by: Álvaro Herrera
Discussion: https://postgr.es/m/20200616.120236.1809496990963386593[email protected]

5 years agoSave slot's restart_lsn when invalidated due to size
Alvaro Herrera [Wed, 24 Jun 2020 18:15:17 +0000 (14:15 -0400)]
Save slot's restart_lsn when invalidated due to size

We put it aside as invalidated_at, which let us show "lost" in
pg_replication slot.  Prior to this change, the state value was reported
as NULL.

Backpatch to 13.

Author: Kyotaro Horiguchi 
Reviewed-by: Álvaro Herrera
Discussion: https://postgr.es/m/20200617.101707.1735599255100002667[email protected]
Discussion: https://postgr.es/m/20200407.120905.1507671100168805403[email protected]

5 years agoAdd parens to ConvertToXSegs macro
Alvaro Herrera [Wed, 24 Jun 2020 18:00:37 +0000 (14:00 -0400)]
Add parens to ConvertToXSegs macro

The current definition is dangerous.  No bugs exist in our code at
present, but backpatch to 11 nonetheless where it was introduced.

Author: Álvaro Herrera 

5 years agoStamp 13beta2. REL_13_BETA2
Tom Lane [Mon, 22 Jun 2020 21:16:25 +0000 (17:16 -0400)]
Stamp 13beta2.

5 years agoDoc fixup for hashagg_avoid_disk_plan GUC.
Jeff Davis [Mon, 22 Jun 2020 19:14:55 +0000 (12:14 -0700)]
Doc fixup for hashagg_avoid_disk_plan GUC.

Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/20200620220402[email protected]
Backport-through: 13

5 years agoUndo double-quoting of index names in non-text EXPLAIN output formats.
Tom Lane [Mon, 22 Jun 2020 15:46:41 +0000 (11:46 -0400)]
Undo double-quoting of index names in non-text EXPLAIN output formats.

explain_get_index_name() applied quote_identifier() to the index name.
This is fine for text output, but the non-text output formats all have
their own quoting conventions and would much rather start from the
actual index name.  For example in JSON you'd get something like

       "Index Name": "\"My Index\"",

which is surely not desirable, especially when the same does not
happen for table names.  Hence, move the responsibility for applying
quoting out to the callers, where it can go into already-existing
special code paths for text format.

This changes the API spec for users of explain_get_index_name_hook:
before, they were supposed to apply quote_identifier() if necessary,
now they should not.  Research suggests that the only publicly
available user of the hook is hypopg, and it actually forgot to
apply quoting anyway, so it's fine.  (In any case, there's no
behavioral change for the output of a hook as seen in non-text
EXPLAIN formats, so this won't break any case that programs should
be relying on.)

Digging in the commit logs, it appears that quoting was included in
explain_get_index_name's duties when commit 604ffd280 invented it;
and that was fine at the time because we only had text output format.
This should have been rethought when non-text formats were invented,
but it wasn't.

This is a fairly clear bug for users of non-text EXPLAIN formats,
so back-patch to all supported branches.

Per bug #16502 from Maciek Sakrejda.  Patch by me (based on
investigation by Euler Taveira); thanks to Julien Rouhaud for review.

Discussion: https://postgr.es/m/16502-57bd1c9f913ed1d1@postgresql.org

5 years agoTranslation updates
Peter Eisentraut [Mon, 22 Jun 2020 12:08:30 +0000 (14:08 +0200)]
Translation updates

Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: 434134899af310153f7511ccaa3f376e4c817e66

5 years agoLanguage fixes for docs related to opclass options
Alexander Korotkov [Sun, 21 Jun 2020 01:48:03 +0000 (04:48 +0300)]
Language fixes for docs related to opclass options

Discussion: https://postgr.es/m/20200620232145.GB17995%40telsasoft.com
Author: Justin Pryzby
Backpatch-through: 13

5 years agoDoc: Tweak description of B-Tree duplicate tuples.
Peter Geoghegan [Sun, 21 Jun 2020 00:34:06 +0000 (17:34 -0700)]
Doc: Tweak description of B-Tree duplicate tuples.

Defining duplicates as "close by" to each other was unclear.  Simplify
the definition.

Backpatch: 13-, where deduplication was introduced (by commit 0d861bbb)

5 years agoMinor corrections to docs related to opclass options
Alexander Korotkov [Sat, 20 Jun 2020 21:35:42 +0000 (00:35 +0300)]
Minor corrections to docs related to opclass options

Reported-by: Peter Geoghegan
Discussion: https://postgr.es/m/CAH2-WzmwhYbxuoL0WjTLaiCxW3gj6qadeNpBhWAo_KZsE5-FGw%40mail.gmail.com

5 years agoFix masking of SP-GiST pages during xlog consistency check
Alexander Korotkov [Sat, 20 Jun 2020 14:34:51 +0000 (17:34 +0300)]
Fix masking of SP-GiST pages during xlog consistency check

spg_mask() didn't take into account that pd_lower equal to SizeOfPageHeaderData
is still valid value.  This commit fixes that.  Backpatch to 11, where
spg_mask() pg_lower check was introduced.

Reported-by: Michael Paquier
Discussion: https://postgr.es/m/20200615131405.GM52676%40paquier.xyz
Backpatch-through: 11

5 years agoAdd documentation for opclass options
Alexander Korotkov [Sat, 20 Jun 2020 10:34:54 +0000 (13:34 +0300)]
Add documentation for opclass options

911e7020770 added opclass options and adjusted documentation for each
particular affected opclass.  However, documentation for extendability was
not adjusted.  This commit adjusts documentation for interfaces of index AMs
and opclasses.

Discussion: https://postgr.es/m/CAH2-WzmQnW6%2Bz5F9AW%2BSz%2BzEcEvXofTwh_A9J3%3D_WA-FBP0wYg%40mail.gmail.com
Author: Alexander Korotkov
Reported-by: Peter Geoghegan
Reviewed-by: Peter Geoghegan
5 years agoEnsure write failure reports no-disk-space
Alvaro Herrera [Fri, 19 Jun 2020 20:46:07 +0000 (16:46 -0400)]
Ensure write failure reports no-disk-space

A few places calling fwrite and gzwrite were not setting errno to ENOSPC
when reporting errors, as is customary; this led to some failures being
reported as
"could not write file: Success"
which makes us look silly.  Make a few of these places in pg_dump and
pg_basebackup use our customary pattern.

Backpatch-to: 9.5
Author: Justin Pryzby 
Author: Tom Lane 
Author: Álvaro Herrera 
Discussion: https://postgr.es/m/20200611153753[email protected]

5 years agoFuture-proof regression tests against possibly-missing posixrules file.
Tom Lane [Fri, 19 Jun 2020 17:55:21 +0000 (13:55 -0400)]
Future-proof regression tests against possibly-missing posixrules file.

The IANA time zone folk have deprecated use of a "posixrules" file in
the tz database.  While for now it's our choice whether to keep
supplying one in our own builds, installations built with
--with-system-tzdata will soon be needing to cope with that file not
being present, at least on some platforms.

This causes a problem for the horology test, which expected the
nonstandard POSIX zone spec "CST7CDT" to apply pre-2007 US daylight
savings rules.  That does happen if the posixrules file supplies such
information, but otherwise the test produces undesired results.
To fix, add an explicit transition date rule that matches 2005 practice.
(We could alternatively have switched the test to use some real time
zone, but it seems useful to have coverage of this type of zone spec.)

While at it, update a documentation example that also relied on
"CST7CDT"; use a real-world zone name instead.  Also, document why
the zone names EST5EDT, CST6CDT, MST7MDT, PST8PDT aren't subject to
similar failures when "posixrules" is missing.

Back-patch to all supported branches, since the hazard is the same
for all.

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

5 years agoAdjust some glossary terms
Alvaro Herrera [Fri, 19 Jun 2020 16:55:43 +0000 (12:55 -0400)]
Adjust some glossary terms

Mostly in response to Jürgen Purtz critique of previous definitions,
though I added many other changes.

Author: Álvaro Herrera 
Reviewed-by: Jürgen Purtz
Reviewed-by: Justin Pryzby
Reviewed-by: Erik Rijkers
Discussion: https://postgr.es/m/c1e06008-2132-30f4-9b38-877e8683d418@purtz.de

5 years agoFix deduplication "single value" strategy bug.
Peter Geoghegan [Fri, 19 Jun 2020 15:57:23 +0000 (08:57 -0700)]
Fix deduplication "single value" strategy bug.

It was possible for deduplication's single value strategy to mistakenly
believe that a very small duplicate tuple counts as one of the six large
tuples that it aims to leave behind after the page finally splits.  This
could cause slightly suboptimal space utilization with very low
cardinality indexes, though only under fairly narrow conditions.

To fix, be particular about what kind of tuple counts as a
maxpostingsize-capped tuple.  This avoids confusion in the event of a
small tuple that gets "wedged" between two large tuples, where all
tuples on the page are duplicates of the same value.

Discussion: https://postgr.es/m/CAH2-Wz=Y+sgSFc-O3LpiZX-POx2bC+okec2KafERHuzdVa7-rQ@mail.gmail.com
Backpatch: 13-, where deduplication was introduced (by commit 0d861bbb)

5 years agoFix issues in invalidation of obsolete replication slots.
Fujii Masao [Fri, 19 Jun 2020 08:15:52 +0000 (17:15 +0900)]
Fix issues in invalidation of obsolete replication slots.

This commit fixes the following issues.

1. There is the case where the slot is dropped while trying to invalidate it.
    InvalidateObsoleteReplicationSlots() did not handle this case, and
    which could cause checkpoint to fail.

2. InvalidateObsoleteReplicationSlots() could emit the same log message
    multiple times unnecessary. It should be logged only once.

3. When marking the slot as used, we always searched the target slot from
    all the replication slots even if we already found it. This could cause
    useless waste of cycles.

Back-patch to v13 where these issues were added as a part of
max_slot_wal_keep_size code.

Author: Fujii Masao
Reviewed-by: Kyotaro Horiguchi, Alvaro Herrera
Discussion: https://postgr.es/m/66c05b67-3396-042c-1b41-bfa6c3ddcf82@oss.nttdata.com

5 years agoFix EXPLAIN ANALYZE for parallel HashAgg plans
David Rowley [Fri, 19 Jun 2020 05:25:07 +0000 (17:25 +1200)]
Fix EXPLAIN ANALYZE for parallel HashAgg plans

Since 1f39bce02, HashAgg nodes have had the ability to spill to disk when
memory consumption exceeds work_mem. That commit added new properties to
EXPLAIN ANALYZE to show the maximum memory usage and disk usage, however,
it didn't quite go as far as showing that information for parallel
workers.  Since workers may have experienced something very different from
the main process, we should show this information per worker, as is done
in Sort.

Reviewed-by: Justin Pryzby
Reviewed-by: Jeff Davis
Discussion: https://postgr.es/m/CAApHDvpEKbfZa18mM1TD7qV6PG+w97pwCWq5tVD0dX7e11gRJw@mail.gmail.com
Backpatch-through: 13, where the hashagg spilling code was added.

5 years agoFix deadlock danger when atomic ops are done under spinlock.
Andres Freund [Mon, 8 Jun 2020 23:50:37 +0000 (16:50 -0700)]
Fix deadlock danger when atomic ops are done under spinlock.

This was a danger only for --disable-spinlocks in combination with
atomic operations unsupported by the current platform.

While atomics.c was careful to signal that a separate semaphore ought
to be used when spinlock emulation is active, spin.c didn't actually
implement that mechanism. That's my (Andres') fault, it seems to have
gotten lost during the development of the atomic operations support.

Fix that issue and add test for nesting atomic operations inside a
spinlock.

Author: Andres Freund
Discussion: https://postgr.es/m/20200605023302[email protected]
Backpatch: 9.5-

5 years agoAdd basic spinlock tests to regression tests.
Andres Freund [Mon, 8 Jun 2020 23:36:51 +0000 (16:36 -0700)]
Add basic spinlock tests to regression tests.

As s_lock_test, the already existing test for spinlocks, isn't run in
an automated fashion (and doesn't test a normal backend environment),
adding tests that are run as part of a normal regression run is a good
idea. Particularly in light of several recent and upcoming spinlock
related fixes.

Currently the new tests are run as part of the pre-existing
test_atomic_ops() test. That perhaps can be quibbled about, but for
now seems ok.

The only operations that s_lock_test tests but the new tests don't are
the detection of a stuck spinlock and S_LOCK_FREE (which is otherwise
unused, not implemented on all platforms, and will be removed).

This currently contains a test for more than INT_MAX spinlocks (only
run with --disable-spinlocks), to ensure the recent commit fixing a
bug with more than INT_MAX spinlock initializations is correct. That
test is somewhat slow, so we might want to disable it after a few
days.

It might be worth retiring s_lock_test after this. The added coverage
of a stuck spinlock probably isn't worth the added complexity?

Author: Andres Freund
Discussion: https://postgr.es/m/20200606023103[email protected]

5 years agoDoc: document POSIX-style time zone specifications in full.
Tom Lane [Thu, 18 Jun 2020 20:27:18 +0000 (16:27 -0400)]
Doc: document POSIX-style time zone specifications in full.

We'd glossed over most of this complexity for years, but it's hard
to avoid writing it all down now, so that we can explain what happens
when there's no "posixrules" file in the IANA time zone database.
That was at best a tiny minority situation till now, but it's likely
to become quite common in the future, so we'd better explain it.

Nonetheless, we don't really encourage people to use POSIX zone specs;
picking a named zone is almost always what you really want, unless
perhaps you're stuck with an out-of-date zone database.  Therefore,
let's shove all this detail into an appendix.

Patch by me; thanks to Robert Haas for help with some awkward wording.

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

5 years agoFix oldest xmin and LSN computation across repslots after advancing
Michael Paquier [Thu, 18 Jun 2020 07:35:29 +0000 (16:35 +0900)]
Fix oldest xmin and LSN computation across repslots after advancing

Advancing a replication slot did not recompute the oldest xmin and LSN
values across replication slots, preventing resource removal like
segments not recycled at checkpoint time.  The original commit that
introduced the slot advancing in 9c7d06d never did the update of those
oldest values, and b0afdca removed this code.

This commit adds a TAP test to check segment recycling with advancing
for physical slots, enforcing an extra segment switch before advancing
to check if the segment gets correctly recycled after a checkpoint.

Reported-by: Andres Freund
Reviewed-by: Alexey Kondratov, Kyptaro Horiguchi
Discussion: https://postgr.es/m/20200609171904[email protected]
Backpatch-through: 11

5 years agodoc: Fix formatting typo
Peter Eisentraut [Thu, 18 Jun 2020 01:22:26 +0000 (03:22 +0200)]
doc: Fix formatting typo

5 years agoSync our copy of the timezone library with IANA release tzcode2020a.
Tom Lane [Wed, 17 Jun 2020 22:29:29 +0000 (18:29 -0400)]
Sync our copy of the timezone library with IANA release tzcode2020a.

This absorbs a leap-second-related bug fix in localtime.c, and
teaches zic to handle an expiration marker in the leapseconds file.
Neither are of any interest to us (for the foreseeable future
anyway), but we need to stay more or less in sync with upstream.

Also adjust some over-eager changes in the README from commit 957338418.
I have no intention of making changes that require C99 in this code,
until such time as all the live back branches require C99.  Otherwise
back-patching will get too exciting.

For the same reason, absorb assorted whitespace and other cosmetic
changes from HEAD into the back branches; mostly this reflects use of
improved versions of pgindent.

All in all then, quite a boring update.  But I figured I'd get it
done while I was looking at this code.

5 years agoFix nbtree.h dedup state comment.
Peter Geoghegan [Wed, 17 Jun 2020 22:23:54 +0000 (15:23 -0700)]
Fix nbtree.h dedup state comment.

Oversight in commit 0d861bbb.

5 years agospinlock emulation: Fix bug when more than INT_MAX spinlocks are initialized.
Andres Freund [Mon, 8 Jun 2020 22:25:49 +0000 (15:25 -0700)]
spinlock emulation: Fix bug when more than INT_MAX spinlocks are initialized.

Once the counter goes negative we ended up with spinlocks that errored
out on first use (due to check in tas_sema).

Author: Andres Freund
Reviewed-By: Robert Haas
Discussion: https://postgr.es/m/20200606023103[email protected]
Backpatch: 9.5-

5 years agoAvoid potential spinlock in a signal handler as part of global barriers.
Andres Freund [Tue, 16 Jun 2020 01:23:10 +0000 (18:23 -0700)]
Avoid potential spinlock in a signal handler as part of global barriers.

On platforms without support for 64bit atomic operations where we also
cannot rely on 64bit reads to have single copy atomicity, such atomics
are implemented using a spinlock based fallback. That means it's not
safe to even read such atomics from within a signal handler (since the
signal handler might run when the spinlock already is held).

To avoid this issue defer global barrier processing out of the signal
handler. Instead of checking local / shared barrier generation to
determine whether to set ProcSignalBarrierPending, introduce
PROCSIGNAL_BARRIER and always set ProcSignalBarrierPending when
receiving such a signal. Additionally avoid redundant work in
ProcessProcSignalBarrier if ProcSignalBarrierPending is unnecessarily.

Also do a small amount of other polishing.

Author: Andres Freund
Reviewed-By: Robert Haas
Discussion: https://postgr.es/m/20200609193723[email protected]
Backpatch: 13-, where the code was introduced.

5 years agoDoc: fix copy-and-pasteo in ecpg docs.
Tom Lane [Tue, 16 Jun 2020 20:41:11 +0000 (16:41 -0400)]
Doc: fix copy-and-pasteo in ecpg docs.

The synopsis for PGTYPESinterval_free() used the wrong name.

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

5 years agoFix file reference in nls.mk
Peter Eisentraut [Tue, 16 Jun 2020 15:25:20 +0000 (17:25 +0200)]
Fix file reference in nls.mk

Broken by move of fe_archive.c to fe_utils.

5 years agoFix buffile.c error handling.
Thomas Munro [Tue, 16 Jun 2020 01:50:56 +0000 (13:50 +1200)]
Fix buffile.c error handling.

Convert buffile.c error handling to use ereport.  This fixes cases where
I/O errors were indistinguishable from EOF or not reported.  Also remove
"%m" from error messages where errno would be bogus.  While we're
modifying those strings, add block numbers and short read byte counts
where appropriate.

Back-patch to all supported releases.

Reported-by: Amit Khandekar
Reviewed-by: Melanie Plageman
Reviewed-by: Alvaro Herrera
Reviewed-by: Robert Haas
Reviewed-by: Ibrar Ahmed
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/CA%2BhUKGJE04G%3D8TLK0DLypT_27D9dR8F1RQgNp0jK6qR0tZGWOw%40mail.gmail.com

5 years agopg_upgrade: set vacuum_defer_cleanup_age to zero
Bruce Momjian [Tue, 16 Jun 2020 00:59:40 +0000 (20:59 -0400)]
pg_upgrade:  set vacuum_defer_cleanup_age to zero

Non-zero vacuum_defer_cleanup_age values cause pg_upgrade freezing of
the system catalogs to be incomplete, or do nothing.  This will cause
the upgrade to fail in confusing ways.

Reported-by: Laurenz Albe
Discussion: https://postgr.es/m/7d6f6c22ba05ce0c526e9e8b7bfa8105e7da45e6[email protected]

Backpatch-through: 9.5

5 years agoDoc: Add references for SI and SSI.
Thomas Munro [Sun, 14 Jun 2020 23:33:13 +0000 (11:33 +1200)]
Doc: Add references for SI and SSI.

Our documentation failed to point out that REPEATABLE READ is really
snapshot isolation, which might be important to some users.  Point to
the standard reference paper for this complicated topic.

Likewise, add a reference to the VLDB paper about PostgreSQL SSI, for
technical information about our SSI implementation and how it compares
to S2PL.

While here, add a note about catalog access using a lower isolation
level, per recent user complaint.

Back-patch to all releases.

Reported-by: Kyle Kingsbury
Reviewed-by: Andres Freund
Reviewed-by: Peter Geoghegan
Reviewed-by: Tatsuo Ishii
Discussion: https://postgr.es/m/db7b729d-0226-d162-a126-8a8ab2dc4443%40jepsen.io
Discussion: https://postgr.es/m/16454-9408996bb1750faf%40postgresql.org

5 years agoFix behavior of float aggregates for single Inf or NaN inputs.
Tom Lane [Sat, 13 Jun 2020 17:43:24 +0000 (13:43 -0400)]
Fix behavior of float aggregates for single Inf or NaN inputs.

When there is just one non-null input value, and it is infinity or NaN,
aggregates such as stddev_pop and covar_pop should produce a NaN
result, because the calculation is not well-defined.  They used to do
so, but since we adopted Youngs-Cramer aggregation in commit e954a727f,
they produced zero instead.  That's an oversight, so fix it.  Add tests
exercising these edge cases.

Affected aggregates are

 var_pop(double precision)
 stddev_pop(double precision)
 var_pop(real)
 stddev_pop(real)
 regr_sxx(double precision,double precision)
 regr_syy(double precision,double precision)
 regr_sxy(double precision,double precision)
 regr_r2(double precision,double precision)
 regr_slope(double precision,double precision)
 regr_intercept(double precision,double precision)
 covar_pop(double precision,double precision)
 corr(double precision,double precision)

Back-patch to v12 where the behavior change was accidentally introduced.

Report and patch by me; thanks to Dean Rasheed for review.

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

5 years agoSilence _bt_check_unique compiler warning.
Peter Geoghegan [Sat, 13 Jun 2020 16:33:31 +0000 (09:33 -0700)]
Silence _bt_check_unique compiler warning.

Reported-By: Tom Lane
Discussion: https://postgr.es/m/841649.1592065060@sss.pgh.pa.us

5 years agoAdd missing extern keyword for a couple of numutils functions
David Rowley [Fri, 12 Jun 2020 23:28:12 +0000 (11:28 +1200)]
Add missing extern keyword for a couple of numutils functions

In passing, also remove a few surplus empty lines from pg_ltoa and
pg_ulltoa_n in numutils.c

Reported-by: Andrew Gierth
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 13, where these changes were introduced

5 years agoImprove comments for [Heap]CheckForSerializableConflictOut().
Thomas Munro [Thu, 11 Jun 2020 22:44:32 +0000 (10:44 +1200)]
Improve comments for [Heap]CheckForSerializableConflictOut().

Rewrite the documentation of these functions, in light of recent bug fix
commit 5940ffb2.

Back-patch to 13 where the check-for-conflict-out code was split up into
AM-specific and generic parts, and new documentation was added that now
looked wrong.

Reviewed-by: Peter Geoghegan
Discussion: https://postgr.es/m/db7b729d-0226-d162-a126-8a8ab2dc4443%40jepsen.io

5 years agodoc: remove xreflabels used in PG 13 relnotes
Bruce Momjian [Thu, 11 Jun 2020 22:27:59 +0000 (18:27 -0400)]
doc:  remove xreflabels used in PG 13 relnotes

xreflabels were removed in a previous commit

Discussion: https://postgr.es/m/8315c0ca-7758-8823-fcb6-f37f9413e6b6@2ndquadrant.com

Backpatch-through: 13 only

5 years agodoc: remove xreflabels from commits 75fcdd2ae2 and 85af628da5
Bruce Momjian [Thu, 11 Jun 2020 22:25:46 +0000 (18:25 -0400)]
doc:  remove xreflabels from commits 75fcdd2ae2 and 85af628da5

xreflabels prevent references to the chapter numbers of sections id's.
It should only be used in specific cases.

Discussion: https://postgr.es/m/8315c0ca-7758-8823-fcb6-f37f9413e6b6@2ndquadrant.com

Backpatch-through: 9.5

5 years agoFix doc build, broken by 13e0fa7a.
Jeff Davis [Thu, 11 Jun 2020 21:44:31 +0000 (14:44 -0700)]
Fix doc build, broken by 13e0fa7a.

Reported-by: Peter Geoghegan
5 years agoFix mishandling of NaN counts in numeric_[avg_]combine.
Tom Lane [Thu, 11 Jun 2020 21:38:42 +0000 (17:38 -0400)]
Fix mishandling of NaN counts in numeric_[avg_]combine.

When merging two NumericAggStates, the code missed adding the new
state's NaNcount unless its N was also nonzero; since those counts
are independent, this is wrong.

This would only have visible effect if some partial aggregate scans
found only NaNs while earlier ones found only non-NaNs; then we could
end up falsely deciding that there were no NaNs and fail to return a
NaN final result as expected.  That's pretty improbable, so it's no
surprise this hasn't been reported from the field.  Still, it's a bug.

I didn't try to produce a regression test that would show the bug,
but I did notice that these functions weren't being reached at all
in our regression tests, so I improved the tests to at least
exercise them.  With these additions, I see pretty complete code
coverage on the aggregation-related functions in numeric.c.

Back-patch to 9.6 where this code was introduced.  (I only added
the improved test case as far back as v10, though, since the
relevant part of aggregates.sql isn't there at all in 9.6.)

5 years agoRework HashAgg GUCs.
Jeff Davis [Thu, 11 Jun 2020 18:58:16 +0000 (11:58 -0700)]
Rework HashAgg GUCs.

Eliminate enable_groupingsets_hash_disk, which was primarily useful
for testing grouping sets that use HashAgg and spill. Instead, hack
the table stats to convince the planner to choose hashed aggregation
for grouping sets that will spill to disk. Suggested by Melanie
Plageman.

Rename enable_hashagg_disk to hashagg_avoid_disk_plan, and invert the
meaning of on/off. The new name indicates more strongly that it only
affects the planner. Also, the word "avoid" is less definite, which
should avoid surprises when HashAgg still needs to use the
disk. Change suggested by Justin Pryzby, though I chose a different
GUC name.

Discussion: https://postgr.es/m/CAAKRu_aisiENMsPM2gC4oUY1hHG3yrCwY-fXUg22C6_MJUwQdA%40mail.gmail.com
Discussion: https://postgr.es/m/20200610021544[email protected]
Backpatch-through: 13

5 years agoAvoid update conflict out serialization anomalies.
Peter Geoghegan [Thu, 11 Jun 2020 17:09:45 +0000 (10:09 -0700)]
Avoid update conflict out serialization anomalies.

SSI's HeapCheckForSerializableConflictOut() test failed to correctly
handle conditions involving a concurrently inserted tuple which is later
concurrently updated by a separate transaction .  A SELECT statement
that called HeapCheckForSerializableConflictOut() could end up using the
same XID (updater's XID) for both the original tuple, and the successor
tuple, missing the XID of the xact that created the original tuple
entirely.  This only happened when neither tuple from the chain was
visible to the transaction's MVCC snapshot.

The observable symptoms of this bug were subtle.  A pair of transactions
could commit, with the later transaction failing to observe the effects
of the earlier transaction (because of the confusion created by the
update to the non-visible row).  This bug dates all the way back to
commit dafaa3ef, which added SSI.

To fix, make sure that we check the xmin of concurrently inserted tuples
that happen to also have been updated concurrently.

Author: Peter Geoghegan
Reported-By: Kyle Kingsbury
Reviewed-By: Thomas Munro
Discussion: https://postgr.es/m/db7b729d-0226-d162-a126-8a8ab2dc4443@jepsen.io
Backpatch: All supported versions

5 years agoFix typos.
Amit Kapila [Thu, 11 Jun 2020 08:40:43 +0000 (14:10 +0530)]
Fix typos.

Reported-by: John Naylor
Author: John Naylor
Backpatch-through: 9.5
Discussion: https://postgr.es/m/CACPNZCtRuvs6G+EYqejhVJgBq2AKeZdXRVJsbX4syhO9gn5SNQ@mail.gmail.com

5 years agoMove frontend-side archive APIs from src/common/ to src/fe_utils/
Michael Paquier [Thu, 11 Jun 2020 06:48:56 +0000 (15:48 +0900)]
Move frontend-side archive APIs from src/common/ to src/fe_utils/

fe_archive.c was compiled only for the frontend in src/common/, but as
it will never share anything with the backend, it makes most sense to
move this file to src/fe_utils/.

Reported-by: Peter Eisentraut
Discussion: https://postgr.es/m/e9766d71-8655-ac86-bdf6-77e0e7169977@2ndquadrant.com
Backpatch-through: 13

5 years agoUpdate description of parameter password_encryption
Peter Eisentraut [Wed, 10 Jun 2020 09:57:41 +0000 (11:57 +0200)]
Update description of parameter password_encryption

The previous description string still described the pre-PostgreSQL
10 (pre eb61136dc75a76caef8460fa939244d8593100f2) behavior of
selecting between encrypted and unencrypted, but it is now choosing
between encryption algorithms.

5 years agoFix ReorderBuffer memory overflow check.
Amit Kapila [Wed, 10 Jun 2020 04:50:10 +0000 (10:20 +0530)]
Fix ReorderBuffer memory overflow check.

Commit cec2edfa78 introduced logical_decoding_work_mem to limit
ReorderBuffer memory usage. We spill the changes once the memory occupied
by changes exceeds logical_decoding_work_mem.  There was an assumption
in the code that by evicting the largest (sub)transaction we will come
under the memory limit as the selected transaction will be at least as
large as the most recent change (which caused us to go over the memory
limit).  However, that is not true because a user can reduce the
logical_decoding_work_mem to a smaller value before the most recent
change.

We fix it by allowing to evict the transactions until we reach under the
memory limit.

Reported-by: Fujii Masao
Author: Amit Kapila
Reviewed-by: Fujii Masao
Backpatch-through: 13, where it was introduced
Discussion: https://postgr.es/m/2b7ba291-22e0-a187-d167-9e5309a3458d@oss.nttdata.com

5 years agoSpelling adjustments
Peter Eisentraut [Tue, 9 Jun 2020 08:41:41 +0000 (10:41 +0200)]
Spelling adjustments

similar to 0fd2a79a637f9f96b9830524823df0454e962f96

5 years agoFix invalid function references in a few comments
David Rowley [Tue, 9 Jun 2020 06:43:58 +0000 (18:43 +1200)]
Fix invalid function references in a few comments

These appear to have been forgotten when the functions were renamed in
1fd687a03.

Backpatch-through: 13, where the functions were renamed

5 years agoRepair unstable regression test.
Tom Lane [Tue, 9 Jun 2020 05:17:59 +0000 (01:17 -0400)]
Repair unstable regression test.

Commit 0c882e52a tried to force table atest12 to have more-accurate-
than-default statistics; but transiently setting default_statistics_target
isn't enough for that, because autovacuum could come along and overwrite
the stats later.  This evidently explains some intermittent buildfarm
failures we've seen since then.  Repair by disabling autovac on this table.

Thanks to David Rowley for correctly diagnosing the cause.

Discussion: https://postgr.es/m/CA+hUKG+OUkQSOUTg=qo=S=fWa_tbm99i7rB7mfbHz1SYm4v-jQ@mail.gmail.com

5 years agoFix HashAgg regression from choosing too many initial buckets.
Jeff Davis [Tue, 9 Jun 2020 03:59:45 +0000 (20:59 -0700)]
Fix HashAgg regression from choosing too many initial buckets.

Diagnosis by Andres.

Reported-by: Pavel Stehule
Discussion: https://postgr.es/m/CAFj8pRDLVakD5Aagt3yZeEQeTeEWaS3YE5h8XC3Q3qJ6TYkc2Q%40mail.gmail.com
Backpatch-through: 13

5 years agoAvoid need for valgrind suppressions for pg_atomic_init_u64 on some platforms.
Andres Freund [Tue, 9 Jun 2020 02:52:19 +0000 (19:52 -0700)]
Avoid need for valgrind suppressions for pg_atomic_init_u64 on some platforms.

Previously we used pg_atomic_write_64_impl inside
pg_atomic_init_u64. That works correctly, but on platforms without
64bit single copy atomicity it could trigger spurious valgrind errors
about uninitialized memory, because we use compare_and_swap for atomic
writes on such platforms.

I previously suppressed one instance of this problem (6c878edc1df),
but as Tom reports that wasn't enough. As the atomic variable cannot
yet be concurrently accessible during initialization, it seems better
to have pg_atomic_init_64_impl set the value directly.

Change pg_atomic_init_u32_impl for symmetry.

Reported-By: Tom Lane
Author: Andres Freund
Discussion: https://postgr.es/m/1714601.1591503815@sss.pgh.pa.us
Backpatch: 9.5-

5 years agoFix locking bugs that could corrupt pg_control.
Thomas Munro [Mon, 8 Jun 2020 01:57:24 +0000 (13:57 +1200)]
Fix locking bugs that could corrupt pg_control.

The redo routines for XLOG_CHECKPOINT_{ONLINE,SHUTDOWN} must acquire
ControlFileLock before modifying ControlFile->checkPointCopy, or the
checkpointer could write out a control file with a bad checksum.

Likewise, XLogReportParameters() must acquire ControlFileLock before
modifying ControlFile and calling UpdateControlFile().

Back-patch to all supported releases.

Author: Nathan Bossart 
Author: Fujii Masao 
Reviewed-by: Fujii Masao
Reviewed-by: Michael Paquier
Reviewed-by: Thomas Munro
Discussion: https://postgr.es/m/70BF24D6-DC51-443F-B55A-95735803842A%40amazon.com

5 years agoDoc: Update example symptom of systemd misconfiguration.
Thomas Munro [Mon, 8 Jun 2020 01:20:46 +0000 (13:20 +1200)]
Doc: Update example symptom of systemd misconfiguration.

In PostgreSQL 10, we stopped using System V semaphores on Linux
systems.  Update the example we give of an error message from a
misconfigured system to show what people are most likely to see these
days.

Back-patch to 10, where PREFERRED_SEMAPHORES=UNNAMED_POSIX arrived.

Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/CA%2BhUKGLmJUSwybaPQv39rB8ABpqJq84im2UjZvyUY4feYhpWMw%40mail.gmail.com

5 years agoFix crash in WAL sender when starting physical replication
Michael Paquier [Mon, 8 Jun 2020 01:12:31 +0000 (10:12 +0900)]
Fix crash in WAL sender when starting physical replication

Since database connections can be used with WAL senders in 9.4, it is
possible to use physical replication.  This commit fixes a crash when
starting physical replication with a WAL sender using a database
connection, caused by the refactoring done in 850196b.

There have been discussions about forbidding the use of physical
replication in a database connection, but this is left for later,
taking care only of the crash new to 13.

While on it, add a test to check for a failure when attempting logical
replication if the WAL sender does not have a database connection.  This
part is extracted from a larger patch by Kyotaro Horiguchi.

Reported-by: Vladimir Sitnikov
Author: Michael Paquier, Kyotaro Horiguchi
Reviewed-by: Kyotaro Horiguchi, Álvaro Herrera
Discussion: https://postgr.es/m/CAB=Je-GOWMj1PTPkeUhjqQp-4W3=nW-pXe2Hjax6rJFffB5_Aw@mail.gmail.com
Backpatch-through: 13

5 years agoMSVC: Avoid warning when testing a TAP suite without PROVE_FLAGS.
Noah Misch [Sun, 7 Jun 2020 23:27:13 +0000 (16:27 -0700)]
MSVC: Avoid warning when testing a TAP suite without PROVE_FLAGS.

Commit 7be5d8df1f74b78620167d3abf32ee607e728919 surfaced the logic
error, which had no functional implications, by adding "use warnings".
The buildfarm always customizes PROVE_FLAGS, so the warning did not
appear there.  Back-patch to 9.5 (all supported versions).

5 years agopgindent run prior to branching v13.
Tom Lane [Sun, 7 Jun 2020 20:57:08 +0000 (16:57 -0400)]
pgindent run prior to branching v13.

pgperltidy and reformat-dat-files too, though those didn't
find anything to change.

5 years agoTry to read data from the socket in pqSendSome's write_failed paths.
Tom Lane [Sun, 7 Jun 2020 17:44:13 +0000 (13:44 -0400)]
Try to read data from the socket in pqSendSome's write_failed paths.

Even when we've concluded that we have a hard write failure on the
socket, we should continue to try to read data.  This gives us an
opportunity to collect any final error message that the backend might
have sent before closing the connection; moreover it is the job of
pqReadData not pqSendSome to close the socket once EOF is detected.

Due to an oversight in 1f39a1c06, pqSendSome failed to try to collect
data in the case where we'd already set write_failed.  The problem was
masked for ordinary query operations (which really only make one write
attempt anyway), but COPY to the server would continue to send data
indefinitely after a mid-COPY connection loss.

Hence, add pqReadData calls into the paths where pqSendSome drops data
because of write_failed.  If we've lost the connection, this will
eventually result in closing the socket and setting CONNECTION_BAD,
which will cause PQputline and siblings to report failure, allowing
the application to terminate the COPY sooner.  (Basically this restores
what happened before 1f39a1c06.)

There are related issues that this does not solve; for example, if the
backend sends an error but doesn't drop the connection, we did and
still will keep pumping COPY data as long as the application sends it.
Fixing that will require application-visible behavior changes though,
and anyway it's an ancient behavior that we've had few complaints about.
For now I'm just trying to fix the regression from 1f39a1c06.

Per a complaint from Andres Freund.  Back-patch into v12 where
1f39a1c06 came in.

Discussion: https://postgr.es/m/20200603201242[email protected]

5 years agoRethink definition of cancel.c's CancelRequested flag.
Tom Lane [Sun, 7 Jun 2020 17:07:31 +0000 (13:07 -0400)]
Rethink definition of cancel.c's CancelRequested flag.

As it stands, this flag is only set when we've successfully sent a
cancel request, not if we get SIGINT and then fail to send a cancel.
However, for almost all callers, that's the Wrong Thing: we'd prefer
to abort processing after control-C even if no cancel could be sent.

As an example, since commit 1d468b9ad "pgbench -i" fails to give up
sending COPY data even after control-C, if the postmaster has been
stopped, which is clearly not what the code intends and not what anyone
would want.  (The fact that it keeps going at all is the fault of a
separate bug in libpq, but not letting CancelRequested become set is
clearly not what we want here.)

The sole exception, as far as I can find, is that scripts_parallel.c's
ParallelSlotsGetIdle tries to consume a query result after issuing a
cancel, which of course might not terminate quickly if no cancel
happened.  But that behavior was poorly thought out too.  No user of
ParallelSlotsGetIdle tries to continue processing after a cancel,
so there is really no point in trying to clear the connection's state.
Moreover this has the same defect as for other users of cancel.c,
that if the cancel request fails for some reason then we end up with
control-C being completely ignored.  (On top of that, select_loop failed
to distinguish clearly between SIGINT and other reasons for select(2)
failing, which means that it's possible that the existing code would
think that a cancel has been sent when it hasn't.)

Hence, redefine CancelRequested as simply meaning that SIGINT was
received.  We could add a second flag with the other meaning, but
in the absence of any compelling argument why such a flag is needed,
I think it would just offer an opportunity for future callers to
get it wrong.  Also remove the consumeQueryResult call in
ParallelSlotsGetIdle's failure exit.  In passing, simplify the
API of select_loop.

It would now be possible to re-unify psql's cancel_pressed with
CancelRequested, partly undoing 5d43c3c54.  But I'm not really
convinced that that's worth the trouble, so I left psql alone,
other than fixing a misleading comment.

This code is new in v13 (cf a4fd3aa71), so no need for back-patch.

Per investigation of a complaint from Andres Freund.

Discussion: https://postgr.es/m/20200603201242[email protected]

5 years agoFix platform-specific performance regression in logtape.c.
Jeff Davis [Sun, 7 Jun 2020 16:14:24 +0000 (09:14 -0700)]
Fix platform-specific performance regression in logtape.c.

Commit 24d85952 made a change that indirectly caused a performance
regression by triggering a change in the way GCC optimizes memcpy() on
some platforms.

The behavior seemed to contradict a GCC document, so I filed a report:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95556

This patch implements a narrow workaround which eliminates the
regression I observed. The workaround is benign enough that it seems
unlikely to cause a different regression on another platform.

Discussion: https://postgr.es/m/99b2eab335c1592c925d8143979c8e9e81e1575f[email protected]

5 years agopsql: Format \? output a little better
Peter Eisentraut [Sun, 7 Jun 2020 14:12:05 +0000 (16:12 +0200)]
psql: Format \? output a little better