postgresql.git
4 years agoFix rare failure in LDAP tests.
Thomas Munro [Mon, 3 Aug 2020 00:39:15 +0000 (12:39 +1200)]
Fix rare failure in LDAP tests.

Instead of writing a query to psql's stdin, use -c.  This avoids a
failure where psql exits before we write, seen a few times on the build
farm.  Thanks to Tom Lane for the suggestion.

Back-patch to 11, where the LDAP tests arrived.

Reviewed-by: Noah Misch
Discussion: https://postgr.es/m/CA%2BhUKGLFmW%2BHQYPeKiwSp5sdFFHtFViCpw4Mh6yAgEx74r5-Cw%40mail.gmail.com

4 years agoFix minor issues in psql's new \dAc and related commands.
Tom Lane [Sun, 2 Aug 2020 21:00:26 +0000 (17:00 -0400)]
Fix minor issues in psql's new \dAc and related commands.

The type-name pattern in \dAc and \dAf was matched only to the actual
pg_type.typname string, which is fairly user-unfriendly in cases where
that is not what's shown to the user by format_type (compare "_int4"
and "integer[]").  Make this code match what \dT does, i.e. match the
pattern against either typname or format_type() output.  Also fix its
broken handling of schema-name restrictions.  (IOW, make these
processSQLNamePattern calls match \dT's.)  While here, adjust
whitespace to make the query a little prettier in -E output, too.

Also improve some inaccuracies and shaky grammar in the related
documentation.

Noted while working on a patch for intarray's opclasses; I wondered
why I couldn't get a match to "integer*" for the input type name.

4 years agoUse int64 instead of long in incremental sort code
David Rowley [Sun, 2 Aug 2020 02:23:57 +0000 (14:23 +1200)]
Use int64 instead of long in incremental sort code

Windows 64bit has 4-byte long values which is not suitable for tracking
disk space usage in the incremental sort code. Let's just make all these
fields int64s.

Author: James Coleman
Discussion: https://postgr.es/m/CAApHDvpky%2BUhof8mryPf5i%3D6e6fib2dxHqBrhp0Qhu0NeBhLJw%40mail.gmail.com
Backpatch-through: 13, where the incremental sort code was added

4 years agoRestore lost amcheck TOAST test coverage.
Peter Geoghegan [Fri, 31 Jul 2020 22:34:26 +0000 (15:34 -0700)]
Restore lost amcheck TOAST test coverage.

Commit eba77534 fixed an amcheck false positive bug involving
inconsistencies in TOAST input state between table and index.  A test
case was added that verified that such an inconsistency didn't result in
a spurious corruption related error.

Test coverage from the test was accidentally lost by commit 501e41dd,
which propagated ALTER TABLE ...  SET STORAGE attstorage state to
indexes.  This broke the test because the test specifically relied on
attstorage not being propagated.  This artificially forced there to be
index tuples whose datums were equivalent to the datums in the heap
without the datums actually being bitwise equal.

Fix this by updating pg_attribute directly instead.  Commit 501e41dd
made similar changes to a test_decoding TOAST-related test case which
made the same assumption, but overlooked the amcheck test case.

Backpatch: 11-, just like commit eba77534 (and commit 501e41dd).

4 years agoFix oversight in ALTER TYPE: typmodin/typmodout must propagate to arrays.
Tom Lane [Fri, 31 Jul 2020 21:11:28 +0000 (17:11 -0400)]
Fix oversight in ALTER TYPE: typmodin/typmodout must propagate to arrays.

If a base type supports typmods, its array type does too, with the
same interpretation.  Hence changes in pg_type.typmodin/typmodout
must be propagated to the array type.

While here, improve AlterTypeRecurse to not recurse to domains if
there is nothing we'd need to change.

Oversight in fe30e7ebf.  Back-patch to v13 where that came in.

4 years agoFix recently-introduced performance problem in ts_headline().
Tom Lane [Fri, 31 Jul 2020 15:43:12 +0000 (11:43 -0400)]
Fix recently-introduced performance problem in ts_headline().

The new hlCover() algorithm that I introduced in commit c9b0c678d
turns out to potentially take O(N^2) or worse time on long documents,
if there are many occurrences of individual query words but few or no
substrings that actually satisfy the query.  (One way to hit this
behavior is with a "common_word & rare_word" type of query.)  This
seems unavoidable given the original goal of checking every substring
of the document, so we have to back off that idea.  Fortunately, it
seems unlikely that anyone would really want headlines spanning all of
a long document, so we can avoid the worse-than-linear behavior by
imposing a maximum length of substring that we'll consider.

For now, just hard-wire that maximum length as a multiple of max_words
times max_fragments.  Perhaps at some point somebody will argue for
exposing it as a ts_headline parameter, but I'm hesitant to make such
a feature addition in a back-patched bug fix.

I also noted that the hlFirstIndex() function I'd added in that
commit was unnecessarily stupid: it really only needs to check whether
a HeadlineWordEntry's item pointer is null or not.  This wouldn't make
all that much difference in typical cases with queries having just
a few terms, but a cycle shaved is a cycle earned.

In addition, add a CHECK_FOR_INTERRUPTS call in TS_execute_recurse.
This ensures that hlCover's loop is cancellable if it manages to take
a long time, and it may protect some other TS_execute callers as well.

Back-patch to 9.6 as the previous commit was.  I also chose to add the
CHECK_FOR_INTERRUPTS call to 9.5.  The old hlCover() algorithm seems
to avoid the O(N^2) behavior, at least on the test case I tried, but
nonetheless it's not very quick on a long document.

Per report from Stephen Frost.

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

4 years agoDoc: fix high availability solutions comparison.
Tatsuo Ishii [Thu, 30 Jul 2020 22:46:25 +0000 (07:46 +0900)]
Doc: fix high availability solutions comparison.

In "High Availability, Load Balancing, and Replication" chapter,
certain descriptions of Pgpool-II were not correct at this point.  It
does not need conflict resolution. Also "Multiple-Server Parallel
Query Execution" is not supported anymore.

Discussion: https://postgr.es/m/20200726.230128.53842489850344110.t-ishii%40sraoss.co.jp
Author: Tatsuo Ishii
Reviewed-by: Bruce Momjian
Backpatch-through: 9.5

4 years agoUse pg_bitutils for HyperLogLog.
Jeff Davis [Thu, 30 Jul 2020 15:44:58 +0000 (08:44 -0700)]
Use pg_bitutils for HyperLogLog.

Using pg_leftmost_one_post32() yields substantial performance benefits.

Backpatching to version 13 because HLL is used for HashAgg
improvements in 9878b643, which was also backpatched to 13.

Reviewed-by: Peter Geoghegan
Discussion: https://postgr.es/m/CAH2-WzkGvDKVDo+0YvfvZ+1CE=iCi88DCOGFF3i1hTGGaxcKPw@mail.gmail.com
Backpatch-through: 13

4 years agodoc: Mention index references in pg_inherits
Michael Paquier [Thu, 30 Jul 2020 06:48:52 +0000 (15:48 +0900)]
doc: Mention index references in pg_inherits

Partitioned indexes are also registered in pg_inherits, but the
description of this catalog did not reflect that.

Author: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 11

4 years agoAdd hash_mem_multiplier GUC.
Peter Geoghegan [Wed, 29 Jul 2020 21:14:57 +0000 (14:14 -0700)]
Add hash_mem_multiplier GUC.

Add a GUC that acts as a multiplier on work_mem.  It gets applied when
sizing executor node hash tables that were previously size constrained
using work_mem alone.

The new GUC can be used to preferentially give hash-based nodes more
memory than the generic work_mem limit.  It is intended to enable admin
tuning of the executor's memory usage.  Overall system throughput and
system responsiveness can be improved by giving hash-based executor
nodes more memory (especially over sort-based alternatives, which are
often much less sensitive to being memory constrained).

The default value for hash_mem_multiplier is 1.0, which is also the
minimum valid value.  This means that hash-based nodes continue to apply
work_mem in the traditional way by default.

hash_mem_multiplier is generally useful.  However, it is being added now
due to concerns about hash aggregate performance stability for users
that upgrade to Postgres 13 (which added disk-based hash aggregation in
commit 1f39bce0).  While the old hash aggregate behavior risked
out-of-memory errors, it is nevertheless likely that many users actually
benefited.  Hash agg's previous indifference to work_mem during query
execution was not just faster; it also accidentally made aggregation
resilient to grouping estimate problems (at least in cases where this
didn't create destabilizing memory pressure).

hash_mem_multiplier can provide a certain kind of continuity with the
behavior of Postgres 12 hash aggregates in cases where the planner
incorrectly estimates that all groups (plus related allocations) will
fit in work_mem/hash_mem.  This seems necessary because hash-based
aggregation is usually much slower when only a small fraction of all
groups can fit.  Even when it isn't possible to totally avoid hash
aggregates that spill, giving hash aggregation more memory will reliably
improve performance (the same cannot be said for external sort
operations, which appear to be almost unaffected by memory availability
provided it's at least possible to get a single merge pass).

The PostgreSQL 13 release notes should advise users that increasing
hash_mem_multiplier can help with performance regressions associated
with hash aggregation.  That can be taken care of by a later commit.

Author: Peter Geoghegan
Reviewed-By: Álvaro Herrera, Jeff Davis
Discussion: https://postgr.es/m/20200625203629[email protected]
Discussion: https://postgr.es/m/CAH2-WzmD%2Bi1pG6rc1%2BCjc4V6EaFJ_qSuKCCHVnH%3DoruqD-zqow%40mail.gmail.com
Backpatch: 13-, where disk-based hash aggregation was introduced.

4 years agoHashAgg: use better cardinality estimate for recursive spilling.
Jeff Davis [Wed, 29 Jul 2020 06:15:47 +0000 (23:15 -0700)]
HashAgg: use better cardinality estimate for recursive spilling.

Use HyperLogLog to estimate the group cardinality in a spilled
partition. This estimate is used to choose the number of partitions if
we recurse.

The previous behavior was to use the number of tuples in a spilled
partition as the estimate for the number of groups, which lead to
overpartitioning. That could cause the number of batches to be much
higher than expected (with each batch being very small), which made it
harder to interpret EXPLAIN ANALYZE results.

Reviewed-by: Peter Geoghegan
Discussion: https://postgr.es/m/a856635f9284bc36f7a77d02f47bbb6aaf7b59b3[email protected]
Backpatch-through: 13

4 years agoRename another "hash_mem" local variable.
Peter Geoghegan [Wed, 29 Jul 2020 00:59:14 +0000 (17:59 -0700)]
Rename another "hash_mem" local variable.

Missed by my commit 564ce621.

Backpatch: 13-, where disk-based hash aggregation was introduced.

4 years agoCorrect obsolete UNION hash aggs comment.
Peter Geoghegan [Wed, 29 Jul 2020 00:14:06 +0000 (17:14 -0700)]
Correct obsolete UNION hash aggs comment.

Oversight in commit 1f39bce0, which added disk-based hash aggregation.

Backpatch: 13-, where disk-based hash aggregation was introduced.

4 years agoDoc: Remove obsolete CREATE AGGREGATE note.
Peter Geoghegan [Tue, 28 Jul 2020 23:58:59 +0000 (16:58 -0700)]
Doc: Remove obsolete CREATE AGGREGATE note.

The planner is in fact willing to use hash aggregation when work_mem is
not set high enough for everything to fit in memory.  This has been the
case since commit 1f39bce0, which added disk-based hash aggregation.

There are a few remaining cases in which hash aggregation is avoided as
a matter of policy when the planner surmises that spilling will be
necessary.  For example, callers of choose_hashed_setop() still
conservatively avoid hash aggregation when spilling is anticipated.
That doesn't seem like a good enough reason to mention hash aggregation
in this context.

Backpatch: 13-, where disk-based hash aggregation was introduced.

4 years agoMake EXPLAIN ANALYZE of HashAgg more similar to Hash Join
David Rowley [Tue, 28 Jul 2020 23:43:11 +0000 (11:43 +1200)]
Make EXPLAIN ANALYZE of HashAgg more similar to Hash Join

There were various unnecessary differences between Hash Agg's EXPLAIN
ANALYZE output and Hash Join's.  Here we modify the Hash Agg output so
that it's better aligned to Hash Join's.

The following changes have been made:
1. Start batches counter at 1 instead of 0.
2. Always display the "Batches" property, even when we didn't spill to
   disk.
3. Use the text "Batches" instead of "HashAgg Batches" for text format.
4. Use the text "Memory Usage" instead of "Peak Memory Usage" for text
   format.
5. Include "Batches" before "Memory Usage" in both text and non-text
   formats.

In passing also modify the "Planned Partitions" property so that we show
it regardless of if the value is 0 or not for non-text EXPLAIN formats.
This was pointed out by Justin Pryzby and probably should have been part
of 40efbf870.

Reviewed-by: Justin Pryzby, Jeff Davis
Discussion: https://postgr.es/m/CAApHDvrshRnA6C0VFnu7Fb9TVvgGo80PUMm5+2DiaS1gEkPvtw@mail.gmail.com
Backpatch-through: 13, where HashAgg batching was introduced

4 years agoDoc: Improve documentation for pg_jit_available()
David Rowley [Tue, 28 Jul 2020 10:52:43 +0000 (22:52 +1200)]
Doc: Improve documentation for pg_jit_available()

Per complaint from Scott Ribe. Based on wording suggestion from Tom Lane.

Discussion: https://postgr.es/m/1956E806-1468-4417-9A9D-235AE1D5FE1A@elevated-dev.com
Backpatch-through: 11, where pg_jit_available() was added

4 years agodoc: Mention the rename of wal_keep_segments GUC in release note.
Fujii Masao [Tue, 28 Jul 2020 02:23:02 +0000 (11:23 +0900)]
doc: Mention the rename of wal_keep_segments GUC in release note.

Commit f5dff45962 renamed wal_keep_segments to wal_keep_size.
This commit adds the mention to this change in the release note.

Author: Fujii Masao
Reviewed-by: David Steele
Discussion: https://postgr.es/m/dc4768f2-1eff-d2fc-35ba-6b2985b7cb6c@oss.nttdata.com

4 years agoFix some issues with step generation in partition pruning.
Etsuro Fujita [Tue, 28 Jul 2020 02:00:00 +0000 (11:00 +0900)]
Fix some issues with step generation in partition pruning.

In the case of range partitioning, get_steps_using_prefix() assumes that
the passed-in prefix list contains at least one clause for each of the
partition keys earlier than one specified in the passed-in
step_lastkeyno, but the caller (ie, gen_prune_steps_from_opexps())
didn't take it into account, which led to a server crash or incorrect
results when the list contained no clauses for such partition keys, as
reported in bug #16500 and #16501 from Kobayashi Hisanori.  Update the
caller to call that function only when the list created there contains
at least one clause for each of the earlier partition keys in the case
of range partitioning.

While at it, fix some other issues:

* The list to pass to get_steps_using_prefix() is allowed to contain
  multiple clauses for the same partition key, as described in the
  comment for that function, but that function actually assumed that the
  list contained just a single clause for each of middle partition keys,
  which led to an assertion failure when the list contained multiple
  clauses for such partition keys.  Update that function to match the
  comment.
* In the case of hash partitioning, partition keys are allowed to be
  NULL, in which case the list to pass to get_steps_using_prefix()
  contains no clauses for NULL partition keys, but that function treats
  that case as like the case of range partitioning, which led to the
  assertion failure.  Update the assertion test to take into account
  NULL partition keys in the case of hash partitioning.
* Fix a typo in a comment in get_steps_using_prefix_recurse().
* gen_partprune_steps() failed to detect self-contradiction from
  strict-qual clauses and an IS NULL clause for the same partition key
  in some cases, producing incorrect partition-pruning steps, which led
  to incorrect results of partition pruning, but didn't cause any
  user-visible problems fortunately, as the self-contradiction is
  detected later in the query planning.  Update that function to detect
  the self-contradiction.

Per bug #16500 and #16501 from Kobayashi Hisanori.  Patch by me, initial
diagnosis for the reported issue and review by Dmitry Dolgov.
Back-patch to v11, where partition pruning was introduced.

Discussion: https://postgr.es/m/16500-d1613f2a78e1e090%40postgresql.org
Discussion: https://postgr.es/m/16501-5234a9a0394f6754%40postgresql.org

4 years agoRemove hashagg_avoid_disk_plan GUC.
Peter Geoghegan [Tue, 28 Jul 2020 00:53:17 +0000 (17:53 -0700)]
Remove hashagg_avoid_disk_plan GUC.

Note: This GUC was originally named enable_hashagg_disk when it appeared
in commit 1f39bce0, which added disk-based hash aggregation.  It was
subsequently renamed in commit 92c58fd9.

Author: Peter Geoghegan
Reviewed-By: Jeff Davis, Álvaro Herrera
Discussion: https://postgr.es/m/9d9d1e1252a52ea1bad84ea40dbebfd54e672a0f.camel%40j-davis.com
Backpatch: 13-, where disk-based hash aggregation was introduced.

4 years agoFix corner case with 16kB-long decompression in pgcrypto, take 2
Michael Paquier [Mon, 27 Jul 2020 06:58:54 +0000 (15:58 +0900)]
Fix corner case with 16kB-long decompression in pgcrypto, take 2

A compressed stream may end with an empty packet.  In this case
decompression finishes before reading the empty packet and the
remaining stream packet causes a failure in reading the following
data.  This commit makes sure to consume such extra data, avoiding a
failure when decompression the data.  This corner case was reproducible
easily with a data length of 16kB, and existed since e94dd6a.  A cheap
regression test is added to cover this case based on a random,
incompressible string.

The first attempt of this patch has allowed to find an older failure
within the compression logic of pgcrypto, fixed by b9b6105.  This
involved SLES 15 with z390 where a custom flavor of libz gets used.
Bonus thanks to Mark Wong for providing access to the specific
environment.

Reported-by: Frank Gagnepain
Author: Kyotaro Horiguchi, Michael Paquier
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/16476-692ef7b84e5fb893@postgresql.org
Backpatch-through: 9.5

4 years agoFix handling of structure for bytea data type in ECPG
Michael Paquier [Mon, 27 Jul 2020 01:29:08 +0000 (10:29 +0900)]
Fix handling of structure for bytea data type in ECPG

Some code paths dedicated to bytea used the structure for varchar.  This
did not lead to any actual bugs, as bytea and varchar have the same
definition, but it could become a trap if one of these definitions
changes for a new feature or a bug fix.

Issue introduced by 050710b.

Author: Shenhao Wang
Reviewed-by: Vignesh C, Michael Paquier
Discussion: https://postgr.es/m/07ac7dee1efc44f99d7f53a074420177@G08CNEXMBPEKD06.g08.fujitsu.local
Backpatch-through: 12

4 years agoFix LookupTupleHashEntryHash() pipeline-stall issue.
Jeff Davis [Sun, 26 Jul 2020 21:55:52 +0000 (14:55 -0700)]
Fix LookupTupleHashEntryHash() pipeline-stall issue.

Refactor hash lookups in nodeAgg.c to improve performance.

Author: Andres Freund and Jeff Davis
Discussion: https://postgr.es/m/20200612213715.op4ye4q7gktqvpuo%40alap3.anarazel.de
Backpatch-through: 13

4 years agoTweak behavior of pg_stat_activity.leader_pid
Michael Paquier [Sun, 26 Jul 2020 07:32:20 +0000 (16:32 +0900)]
Tweak behavior of pg_stat_activity.leader_pid

The initial implementation of leader_pid in pg_stat_activity added by
b025f32 took the approach to strictly print what a PGPROC entry
includes.  In short, if a backend has been involved in parallel query at
least once, leader_pid would remain set as long as the backend is alive.
For a parallel group leader, this means that the field would always be
set after it participated at least once in parallel query, and after
more discussions this could be confusing if using for example a
connection pooler.

This commit changes the data printed so as leader_pid becomes always
NULL for a parallel group leader, showing up a non-NULL value only for
the parallel workers, and actually as long as a parallel query is
running as workers are shut down once the query has completed.

This does not change the definition of any catalog, so no catalog bump
is needed.  Per discussion with Justin Pryzby, Álvaro Herrera, Julien
Rouhaud and me.

Discussion: https://postgr.es/m/20200721035145[email protected]
Backpatch-through: 13

4 years agoFix buffer usage stats for nodes above Gather Merge.
Amit Kapila [Sat, 25 Jul 2020 05:01:19 +0000 (10:31 +0530)]
Fix buffer usage stats for nodes above Gather Merge.

Commit 85c9d347 addressed a similar problem for Gather and Gather
Merge nodes but forgot to account for nodes above parallel nodes.  This
still works for nodes above Gather node because we shut down the workers
for Gather node as soon as there are no more tuples.  We can do a similar
thing for Gather Merge as well but it seems better to account for stats
during nodes shutdown after completing the execution.

Reported-by: Stéphane Lorek, Jehan-Guillaume de Rorthais
Author: Jehan-Guillaume de Rorthais 
Reviewed-by: Amit Kapila
Backpatch-through: 10, where it was introduced
Discussion: https://postgr.es/m/20200718160206.584532a2@firost

4 years agoReplace TS_execute's TS_EXEC_CALC_NOT flag with TS_EXEC_SKIP_NOT.
Tom Lane [Fri, 24 Jul 2020 19:43:56 +0000 (15:43 -0400)]
Replace TS_execute's TS_EXEC_CALC_NOT flag with TS_EXEC_SKIP_NOT.

It's fairly silly that ignoring NOT subexpressions is TS_execute's
default behavior.  It's wrong on its face and it encourages errors
of omission.  Moreover, the only two remaining callers that aren't
specifying CALC_NOT are in ts_headline calculations, and it's very
arguable that those are bugs: if you've specified "!foo" in your
query, why would you want to get a headline that includes "foo"?

Hence, rip that out and change the default behavior to be to calculate
NOT accurately.  As a concession to the slim chance that there is still
somebody somewhere who needs the incorrect behavior, provide a new
SKIP_NOT flag to explicitly request that.

Back-patch into v13, mainly because it seems better to change this
at the same time as the previous commit's rejiggering of TS_execute
related APIs.  Any outside callers affected by this change are
probably also affected by that one.

Discussion: https://postgr.es/m/CALT9ZEE-aLotzBg-pOp2GFTesGWVYzXA3=mZKzRDa_OKnLF7Mg@mail.gmail.com

4 years agoFix assorted bugs by changing TS_execute's callback API to ternary logic.
Tom Lane [Fri, 24 Jul 2020 19:26:51 +0000 (15:26 -0400)]
Fix assorted bugs by changing TS_execute's callback API to ternary logic.

Text search sometimes failed to find valid matches, for instance
'!crew:A'::tsquery might fail to locate 'crew:1B'::tsvector during
an index search.  The root of the issue is that TS_execute's callback
functions were not changed to use ternary (yes/no/maybe) reporting
when we made the search logic itself do so.  It's somewhat annoying
to break that API, but on the other hand we now see that any code
using plain boolean logic is almost certainly broken since the
addition of phrase search.  There seem to be very few outside callers
of this code anyway, so we'll just break them intentionally to get
them to adapt.

This allows removal of tsginidx.c's private re-implementation of
TS_execute, since that's now entirely duplicative.  It's also no
longer necessary to avoid use of CALC_NOT in tsgistidx.c, since
the underlying callbacks can now do something reasonable.

Back-patch into v13.  We can't change this in stable branches,
but it seems not quite too late to fix it in v13.

Tom Lane and Pavel Borisov

Discussion: https://postgr.es/m/CALT9ZEE-aLotzBg-pOp2GFTesGWVYzXA3=mZKzRDa_OKnLF7Mg@mail.gmail.com

4 years agoFix ancient violation of zlib's API spec.
Tom Lane [Thu, 23 Jul 2020 21:19:37 +0000 (17:19 -0400)]
Fix ancient violation of zlib's API spec.

contrib/pgcrypto mishandled the case where deflate() does not consume
all of the offered input on the first try.  It reset the next_in pointer
to the start of the input instead of leaving it alone, causing the wrong
data to be fed to the next deflate() call.

This has been broken since pgcrypto was committed.  The reason for the
lack of complaints seems to be that it's fairly hard to get stock zlib
to not consume all the input, so long as the output buffer is big enough
(which it normally would be in pgcrypto's usage; AFAICT the input is
always going to be packetized into packets no larger than ZIP_OUT_BUF).
However, IBM's zlibNX implementation for AIX evidently will do it
in some cases.

I did not add a test case for this, because I couldn't find one that
would fail with stock zlib.  When we put back the test case for
bug #16476, that will cover the zlibNX situation well enough.

While here, write deflate()'s second argument as Z_NO_FLUSH per its
API spec, instead of hard-wiring the value zero.

Per buildfarm results and subsequent investigation.

Discussion: https://postgr.es/m/16476-692ef7b84e5fb893@postgresql.org

4 years agodoc: Document that ssl_ciphers does not affect TLS 1.3
Peter Eisentraut [Thu, 23 Jul 2020 15:13:00 +0000 (17:13 +0200)]
doc: Document that ssl_ciphers does not affect TLS 1.3

TLS 1.3 uses a different way of specifying ciphers and a different
OpenSSL API.  PostgreSQL currently does not support setting those
ciphers.  For now, just document this.  In the future, support for
this might be added somehow.

Reviewed-by: Jonathan S. Katz
Reviewed-by: Tom Lane
4 years agoFix error message.
Thomas Munro [Thu, 23 Jul 2020 09:10:49 +0000 (21:10 +1200)]
Fix error message.

Remove extra space.  Back-patch to all releases, like commit 7897e3bb.

Author: Lu, Chenyang 
Discussion: https://postgr.es/m/795d03c6129844d3803e7eea48f5af0d%40G08CNEXMBPEKD04.g08.fujitsu.local

4 years agoRevert "Fix corner case with PGP decompression in pgcrypto"
Michael Paquier [Wed, 22 Jul 2020 23:29:14 +0000 (08:29 +0900)]
Revert "Fix corner case with PGP decompression in pgcrypto"

This reverts commit 9e10898, after finding out that buildfarm members
running SLES 15 on z390 complain on the compression and decompression
logic of the new test: pipistrelles, barbthroat and steamerduck.

Those hosts are visibly using hardware-specific changes to improve zlib
performance, requiring more investigation.

Thanks to Tom Lane for the discussion.

Discussion: https://postgr.es/m/20200722093749[email protected]
Backpatch-through: 9.5

4 years agoFix corner case with PGP decompression in pgcrypto
Michael Paquier [Wed, 22 Jul 2020 05:52:36 +0000 (14:52 +0900)]
Fix corner case with PGP decompression in pgcrypto

A compressed stream may end with an empty packet, and PGP decompression
finished before reading this empty packet in the remaining stream.  This
caused a failure in pgcrypto, handling this case as corrupted data.
This commit makes sure to consume such extra data, avoiding a failure
when decompression the entire stream.  This corner case was reproducible
with a data length of 16kB, and existed since its introduction in
e94dd6a.  A cheap regression test is added to cover this case.

Thanks to Jeff Janes for the extra investigation.

Reported-by: Frank Gagnepain
Author: Kyotaro Horiguchi, Michael Paquier
Discussion: https://postgr.es/m/16476-692ef7b84e5fb893@postgresql.org
Backpatch-through: 9.5

4 years agoneqjoinsel must now pass through collation to eqjoinsel.
Tom Lane [Tue, 21 Jul 2020 23:40:44 +0000 (19:40 -0400)]
neqjoinsel must now pass through collation to eqjoinsel.

Since commit 044c99bc5, eqjoinsel passes the passed-in collation
to any operators it invokes.  However, neqjoinsel failed to pass
on whatever collation it got, so that if we invoked a
collation-dependent operator via that code path, we'd get "could not
determine which collation to use for string comparison" or the like.

Per report from Justin Pryzby.  Back-patch to v12, like the previous
commit.

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

4 years agoMinor glossary tweaks
Alvaro Herrera [Tue, 21 Jul 2020 17:08:16 +0000 (13:08 -0400)]
Minor glossary tweaks

Add "(process)" qualifier to two terms, remove self-reference in one
term.

Author: Jürgen Purtz 
Discussion: https://postgr.es/m/95f90a5d-7692-701d-2c0c-0c88eb5cea7d@purtz.de

4 years agoAssert that we don't insert nulls into attnotnull catalog columns.
Tom Lane [Tue, 21 Jul 2020 16:38:08 +0000 (12:38 -0400)]
Assert that we don't insert nulls into attnotnull catalog columns.

The executor checks for this error, and so does the bootstrap catalog
loader, but we never checked for it in retail catalog manipulations.
The folly of that has now been exposed, so let's add assertions
checking it.  Checking in CatalogTupleInsert[WithInfo] and
CatalogTupleUpdate[WithInfo] should be enough to cover this.

Back-patch to v10; the aforesaid functions didn't exist before that,
and it didn't seem worth adapting the patch to the oldest branches.
But given the risk of JIT crashes, I think we certainly need this
as far back as v11.

Pre-v13, we have to explicitly exclude pg_subscription.subslotname
and pg_subscription_rel.srsublsn from the checks, since they are
mismarked.  (Even if we change our mind about applying BKI_FORCE_NULL
in the branch tips, it doesn't seem wise to have assertions that
would fire in existing databases.)

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

4 years agoCorrectly mark pg_subscription_rel.srsublsn as nullable.
Tom Lane [Mon, 20 Jul 2020 18:55:56 +0000 (14:55 -0400)]
Correctly mark pg_subscription_rel.srsublsn as nullable.

The code has always set this column to NULL when it's not valid,
but the catalog header's description failed to reflect that,
as did the SGML docs, as did some of the code.  To prevent future
coding errors of the same ilk, let's hide the field from C code
as though it were variable-length (which, in a sense, it is).

As with commit 72eab84a5, we can only fix this cleanly in HEAD
and v13; the problem extends further back but we'll need some
klugery in the released branches.

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

4 years agoFix construction of updated-columns bitmap in logical replication.
Tom Lane [Mon, 20 Jul 2020 17:40:16 +0000 (13:40 -0400)]
Fix construction of updated-columns bitmap in logical replication.

Commit b9c130a1f failed to apply the publisher-to-subscriber column
mapping while checking which columns were updated.  Perhaps less
significantly, it didn't exclude dropped columns either.  This could
result in an incorrect updated-columns bitmap and thus wrong decisions
about whether to fire column-specific triggers on the subscriber while
applying updates.  In HEAD (since commit 9de77b545), it could also
result in accesses off the end of the colstatus array, as detected by
buildfarm member skink.  Fix the logic, and adjust 003_constraints.pl
so that the problem is exposed in unpatched code.

In HEAD, also add some assertions to check that we don't access off
the ends of these newly variable-sized arrays.

Back-patch to v10, as b9c130a1f was.

Discussion: https://postgr.es/m/CAH2-Wz=79hKQ4++c5A060RYbjTHgiYTHz=fw6mptCtgghH2gJA@mail.gmail.com

4 years agoRename wal_keep_segments to wal_keep_size.
Fujii Masao [Mon, 20 Jul 2020 04:30:18 +0000 (13:30 +0900)]
Rename wal_keep_segments to wal_keep_size.

max_slot_wal_keep_size that was added in v13 and wal_keep_segments are
the GUC parameters to specify how much WAL files to retain for
the standby servers. While max_slot_wal_keep_size accepts the number of
bytes of WAL files, wal_keep_segments accepts the number of WAL files.
This difference of setting units between those similar parameters could
be confusing to users.

To alleviate this situation, this commit renames wal_keep_segments to
wal_keep_size, and make users specify the WAL size in it instead of
the number of WAL files.

There was also the idea to rename max_slot_wal_keep_size to
max_slot_wal_keep_segments, in the discussion. But we have been moving
away from measuring in segments, for example, checkpoint_segments was
replaced by max_wal_size. So we concluded to rename wal_keep_segments
to wal_keep_size.

Back-patch to v13 where max_slot_wal_keep_size was added.

Author: Fujii Masao
Reviewed-by: Álvaro Herrera, Kyotaro Horiguchi, David Steele
Discussion: https://postgr.es/m/574b4ea3-e0f9-b175-ead2-ebea7faea855@oss.nttdata.com

4 years agoFix minor typo in nodeIncrementalSort.c.
Amit Kapila [Mon, 20 Jul 2020 02:24:04 +0000 (07:54 +0530)]
Fix minor typo in nodeIncrementalSort.c.

Author: Vignesh C
Reviewed-by: James Coleman
Backpatch-through: 13, where it was introduced
Discussion: https://postgr.es/m/CALDaNm0WjZqRvdeL59ZfYH0o4mLbKQ23jm-bnjXcFzgpANx55g@mail.gmail.com

4 years agoCorrectly mark pg_subscription.subslotname as nullable.
Tom Lane [Sun, 19 Jul 2020 16:37:23 +0000 (12:37 -0400)]
Correctly mark pg_subscription.subslotname as nullable.

Due to the layout of this catalog, subslotname has to be explicitly
marked BKI_FORCE_NULL, else initdb will default to the assumption
that it's non-nullable.  Since, in fact, CREATE/ALTER SUBSCRIPTION
will store null values there, the existing marking is just wrong,
and has been since this catalog was invented.

We haven't noticed because not much in the system actually depends
on attnotnull being truthful.  However, JIT'ed tuple deconstruction
does depend on that in some cases, allowing crashes or wrong answers
in queries that inspect pg_subscription.  Commit 9de77b545 quite
accidentally exposed this on the buildfarm members that force JIT
activation.

Back-patch to v13.  The problem goes further back, but we cannot
force initdb in released branches, so some klugier solution will
be needed there.  Before working on that, push this simple fix
to try to get the buildfarm back to green.

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

4 years agodoc: Refresh more URLs in the docs
Michael Paquier [Sat, 18 Jul 2020 13:43:41 +0000 (22:43 +0900)]
doc: Refresh more URLs in the docs

This updates some URLs that are redirections, mostly to an equivalent
using https.  One URL referring to generalized partial indexes was
outdated.

Author: Kyotaro Horiguchi
Discussion: https://postgr.es/m/20200717.121308.1369606287593685396[email protected]
Backpatch-through: 9.5

4 years agodoc: Fix description of \copy for psql
Michael Paquier [Sat, 18 Jul 2020 01:42:46 +0000 (10:42 +0900)]
doc: Fix description of \copy for psql

The WHERE clause introduced by 31f3817 was not described.  While on it,
split the grammar of \copy FROM and TO into two distinct parts for
clarity as they support different set of options.

Author: Vignesh C
Discussion: https://postgr.es/m/CALDaNm3zWr=OmxeNqOqfT=uZTSdam_j-gkX94CL8eTNfgUtf6A@mail.gmail.com
Backpatch-through: 12

4 years agoCope with data-offset-less archive files during out-of-order restores.
Tom Lane [Fri, 17 Jul 2020 17:03:50 +0000 (13:03 -0400)]
Cope with data-offset-less archive files during out-of-order restores.

pg_dump produces custom-format archive files that lack data offsets
when it is unable to seek its output.  Up to now that's been a hazard
for pg_restore.  But if pg_restore is able to seek in the archive
file, there is no reason to throw up our hands when asked to restore
data blocks out of order.  Instead, whenever we are searching for a
data block, record the locations of the blocks we passed over (that
is, fill in the missing data-offset fields in our in-memory copy of
the TOC data).  Then, when we hit a case that requires going
backwards, we can just seek back.

Also track the furthest point that we've searched to, and seek back
to there when beginning a search for a new data block.  This avoids
possible O(N^2) time consumption, by ensuring that each data block
is examined at most twice.  (On Unix systems, that's at most twice
per parallel-restore job; but since Windows uses threads here, the
threads can share block location knowledge, reducing the amount of
duplicated work.)

We can also improve the code a bit by using fseeko() to skip over
data blocks during the search.

This is all of some use even in simple restores, but it's really
significant for parallel pg_restore.  In that case, we require
seekability of the input already, and we will very probably need
to do out-of-order restores.

Back-patch to v12, as this fixes a regression introduced by commit
548e50976.  Before that, parallel restore avoided requesting
out-of-order restores, so it would work on a data-offset-less
archive.  Now it will again.

Ideally this patch would include some test coverage, but there are
other open bugs that need to be fixed before we can extend our
coverage of parallel restore very much.  Plan to revisit that later.

David Gilman and Tom Lane; reviewed by Justin Pryzby

Discussion: https://postgr.es/m/CALBH9DDuJ+scZc4MEvw5uO-=vRyR2=QF9+Yh=3hPEnKHWfS81A@mail.gmail.com

4 years agoRemove manual tracking of file position in pg_dump/pg_backup_custom.c.
Tom Lane [Fri, 17 Jul 2020 16:14:28 +0000 (12:14 -0400)]
Remove manual tracking of file position in pg_dump/pg_backup_custom.c.

We do not really need to track the file position by hand.  We were
already relying on ftello() whenever the archive file is seekable,
while if it's not seekable we don't need the file position info
anyway because we're not going to be able to re-write the TOC.

Moreover, that tracking was buggy since it failed to account for
the effects of fseeko().  Somewhat remarkably, that seems not to
have made for any live bugs up to now.  We could fix the oversights,
but it seems better to just get rid of the whole error-prone mess.

In itself this is merely code cleanup.  However, it's necessary
infrastructure for an upcoming bug-fix patch (because that code
*does* need valid file position after fseeko).  The bug fix
needs to go back as far as v12; hence, back-patch that far.

Discussion: https://postgr.es/m/CALBH9DDuJ+scZc4MEvw5uO-=vRyR2=QF9+Yh=3hPEnKHWfS81A@mail.gmail.com

4 years agoAvoid CREATE INDEX unique index deduplication.
Peter Geoghegan [Fri, 17 Jul 2020 16:50:46 +0000 (09:50 -0700)]
Avoid CREATE INDEX unique index deduplication.

There is no advantage to attempting deduplication for a unique index
during CREATE INDEX, since there cannot possibly be any duplicates.
Doing so wastes cycles due to unnecessary copying.  Make sure that we
avoid it consistently.

We already avoided unique index deduplication in the case where there
were some spool2 tuples to merge.  That didn't account for the fact that
spool2 is removed early/unset in the common case where it has no tuples
that need to be merged (i.e. it failed to account for the "spool2 turns
out to be unnecessary" optimization in _bt_spools_heapscan()).

Oversight in commit 0d861bbb, which added nbtree deduplication

Backpatch: 13-, where nbtree deduplication was introduced.

4 years agoEnsure that distributed timezone abbreviation files are plain ASCII.
Tom Lane [Fri, 17 Jul 2020 15:03:55 +0000 (11:03 -0400)]
Ensure that distributed timezone abbreviation files are plain ASCII.

We had two occurrences of "Mitteleuropäische Zeit" in Europe.txt,
though the corresponding entries in Default were spelled
"Mitteleuropaeische Zeit".  Standardize on the latter spelling to
avoid questions of which encoding to use.

While here, correct a couple of other trivial inconsistencies between
the Default file and the supposedly-matching entries in the *.txt
files, as exposed by some checking with comm(1).  Also, add BDST to
the Europe.txt file; it previously was only listed in Default.
None of this has any direct functional effect.

Per complaint from Christoph Berg.  As usual for timezone data patches,
apply to all branches.

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

4 years agoFix whitespace
Peter Eisentraut [Fri, 17 Jul 2020 13:16:13 +0000 (15:16 +0200)]
Fix whitespace

4 years agoResolve gratuitous tabs in SQL file
Peter Eisentraut [Fri, 17 Jul 2020 13:07:54 +0000 (15:07 +0200)]
Resolve gratuitous tabs in SQL file

4 years agoFix signal handler setup for SIGHUP in the apply launcher process.
Amit Kapila [Fri, 17 Jul 2020 03:13:06 +0000 (08:43 +0530)]
Fix signal handler setup for SIGHUP in the apply launcher process.

Commit 1e53fe0e70 has unified the usage of the config-file reload flag by
using the same signal handler function for the SIGHUP signal at many places
in the code.  By mistake, it used the wrong SIGNAL in apply launcher
process for the SIGHUP signal handler function.

Author: Bharath Rupireddy
Reviewed-by: Dilip Kumar
Backpatch-through: 13, where it was introduced
Discussion: https://postgr.es/m/CALj2ACVzHCRnS20bOiEHaLtP5PVBENZQn4khdsSJQgOv_GM-LA@mail.gmail.com

4 years agoSwitch pg_test_fsync to use binary mode on Windows
Michael Paquier [Thu, 16 Jul 2020 06:52:54 +0000 (15:52 +0900)]
Switch pg_test_fsync to use binary mode on Windows

pg_test_fsync has always opened files using the text mode on Windows, as
this is the default mode used if not enforced by _setmode().

This fixes a failure when running pg_test_fsync down to 12 because
O_DSYNC and the text mode are not able to work together nicely.  We
fixed the handling of O_DSYNC in 12~ for the tool by switching to the
concurrent-safe version of fopen() in src/port/ with 0ba06e0.  And
40cfe86, by enforcing the text mode for compatibility reasons if O_TEXT
or O_BINARY are not specified by the caller, broke pg_test_fsync.  For
all versions, this avoids any translation overhead, and pg_test_fsync
should test binary writes, so it is a gain in all cases.

Note that O_DSYNC is still not handled correctly in ~11, leading to
pg_test_fsync to show insanely high numbers for open_datasync() (using
this property it is easy to notice that the binary mode is much
faster).  This would require a backpatch of 0ba06e0 and 40cfe86, which
could potentially break existing applications, so this is left out.

There are no TAP tests for this tool yet, so I have checked all builds
manually using MSVC.  We could invent a new option to run a single
transaction instead of using a duration of 1s to make the tests a
maximum short, but this is left as future work.

Thanks to Bruce Momjian for the discussion.

Reported-by: Jeff Janes
Author: Michael Paquier
Discussion: https://postgr.es/m/16526-279ded30a230d275@postgresql.org
Backpatch-through: 9.5

4 years agodoc: Fix typo
Peter Eisentraut [Wed, 15 Jul 2020 19:01:29 +0000 (21:01 +0200)]
doc: Fix typo

4 years agoFix handling of missing files when using pg_rewind with online source
Michael Paquier [Wed, 15 Jul 2020 06:17:32 +0000 (15:17 +0900)]
Fix handling of missing files when using pg_rewind with online source

When working with an online source cluster, pg_rewind gets a list of all
the files in the source data directory using a WITH RECURSIVE query,
returning a NULL result for a file's metadata if it gets removed between
the moment it is listed in a directory and the moment its metadata is
obtained with pg_stat_file() (say a recycled WAL segment).  The query
result was processed in such a way that for each tuple we checked only
that the first file's metadata was NULL.  This could have two
consequences, both resulting in a failure of the rewind:
- If the first tuple referred to a removed file, all files from the
source would be ignored.
- Any file actually missing would not be considered as such.

While on it, rework slightly the code so as no values are saved if we
know that a file is going to be skipped.

Issue introduced by b36805f, so backpatch down to 9.5.

Author: Justin Pryzby, Michael Paquier
Reviewed-by: Daniel Gustafsson, Masahiko Sawada
Discussion: https://postgr.es/m/20200713061010[email protected]
Backpatch-through: 9.5

4 years agoFix bitmap AND/OR scans on the inside of a nestloop partition-wise join.
Tom Lane [Tue, 14 Jul 2020 22:56:49 +0000 (18:56 -0400)]
Fix bitmap AND/OR scans on the inside of a nestloop partition-wise join.

reparameterize_path_by_child() failed to reparameterize BitmapAnd
and BitmapOr paths.  This matters only if such a path is chosen as
the inside of a nestloop partition-wise join, where we have to pass
in parameters from the outside of the nestloop.  If that did happen,
we generated a bad plan that would likely lead to crashes at execution.

This is not entirely reparameterize_path_by_child()'s fault though;
it's the victim of an ancient decision (my ancient decision, I think)
to not bother filling in param_info in BitmapAnd/Or path nodes.  That
caused the function to believe that such nodes and their children
contain no parameter references and so need not be processed.

In hindsight that decision looks pretty penny-wise and pound-foolish:
while it saves a few cycles during path node setup, we do commonly
need the information later.  In particular, by reversing the decision
and requiring valid param_info data in all nodes of a bitmap path
tree, we can get rid of indxpath.c's get_bitmap_tree_required_outer()
function, which computed the data on-demand.  It's not unlikely that
that nets out as a savings of cycles in many scenarios.  A couple
of other things in indxpath.c can be simplified as well.

While here, get rid of some cases in reparameterize_path_by_child()
that are visibly dead or useless, given that we only care about
reparameterizing paths that can be on the inside of a parameterized
nestloop.  This case reminds one of the maxim that untested code
probably does not work, so I'm unwilling to leave unreachable code
in this function.  (I did leave the T_Gather case in place even
though it's not reached in the regression tests.  It's not very
clear to me when the planner might prefer to put Gather below
rather than above a nestloop, but at least in principle the case
might be interesting.)

Per bug #16536, originally from Arne Roland but with a test case
by Andrew Gierth.  Back-patch to v11 where this code came in.

Discussion: https://postgr.es/m/16536-2213ee0b3aad41fd@postgresql.org

4 years agoFix timing issue with ALTER TABLE's validate constraint
David Rowley [Tue, 14 Jul 2020 04:57:41 +0000 (16:57 +1200)]
Fix timing issue with ALTER TABLE's validate constraint

An ALTER TABLE to validate a foreign key in which another subcommand
already caused a pending table rewrite could fail due to ALTER TABLE
attempting to validate the foreign key before the actual table rewrite
takes place.  This situation could result in an error such as:

ERROR:  could not read block 0 in file "base/nnnnn/nnnnn": read only 0 of 8192 bytes

The failure here was due to the SPI call which validates the foreign key
trying to access an index which is yet to be rebuilt.

Similarly, we also incorrectly tried to validate CHECK constraints before
the heap had been rewritten.

The fix for both is to delay constraint validation until phase 3, after
the table has been rewritten.  For CHECK constraints this means a slight
behavioral change.  Previously ALTER TABLE VALIDATE CONSTRAINT on
inheritance tables would be validated from the bottom up.  This was
different from the order of evaluation when a new CHECK constraint was
added.  The changes made here aligns the VALIDATE CONSTRAINT evaluation
order for inheritance tables to be the same as ADD CONSTRAINT, which is
generally top-down.

Reported-by: Nazli Ugur Koyluoglu, using SQLancer
Discussion: https://postgr.es/m/CAApHDvp%3DZXv8wiRyk_0rWr00skhGkt8vXDrHJYXRMft3TjkxCA%40mail.gmail.com
Backpatch-through: 9.5 (all supported versions)

4 years agoFix comments related to table AMs
Michael Paquier [Tue, 14 Jul 2020 04:17:31 +0000 (13:17 +0900)]
Fix comments related to table AMs

Incorrect function names were referenced.  As this fixes some portions
of tableam.h, that is mentioned in the docs as something to look at when
implementing a table AM, backpatch down to 12 where this has been
introduced.

Author: Hironobu Suzuki
Discussion: https://postgr.es/m/8fe6d672-28dd-3f1d-7aed-ac2f6d599d3f@interdb.jp
Backpatch-through: 12

4 years agoCope with lateral references in the quals of a subquery RTE.
Tom Lane [Tue, 14 Jul 2020 00:38:20 +0000 (20:38 -0400)]
Cope with lateral references in the quals of a subquery RTE.

The qual pushdown logic assumed that all Vars in a restriction clause
must be Vars referencing subquery outputs; but since we introduced
LATERAL, it's possible for such a Var to be a lateral reference instead.
This led to an assertion failure in debug builds.  In a non-debug
build, there might be no ill effects (if qual_is_pushdown_safe decided
the qual was unsafe anyway), or we could get failures later due to
construction of an invalid plan.  I've not gone to much length to
characterize the possible failures, but at least segfaults in the
executor have been observed.

Given that this has been busted since 9.3 and it took this long for
anybody to notice, I judge that the case isn't worth going to great
lengths to optimize.  Hence, fix by just teaching qual_is_pushdown_safe
that such quals are unsafe to push down, matching the previous behavior
when it accidentally didn't fail.

Per report from Tom Ellis.  Back-patch to all supported branches.

Discussion: https://postgr.es/m/20200713175124.GQ8220@cloudinit-builder

4 years agoFix uninitialized value in segno calculation
Alvaro Herrera [Mon, 13 Jul 2020 17:49:50 +0000 (13:49 -0400)]
Fix uninitialized value in segno calculation

Remove previous hack in KeepLogSeg that added a case to deal with a
(badly represented) invalid segment number.  This was added for the sake
of GetWALAvailability.  But it's not needed if in that function we
initialize the segment number to be retreated to the currently being
written segment, so do that instead.

Per valgrind-running buildfarm member skink, and some sparc64 animals.

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

4 years agoFix bugs in libpq's management of GSS encryption state.
Tom Lane [Mon, 13 Jul 2020 15:57:55 +0000 (11:57 -0400)]
Fix bugs in libpq's management of GSS encryption state.

GSS-related resources should be cleaned up in pqDropConnection,
not freePGconn, else the wrong things happen when resetting
a connection or trying to switch to a different server.
It's also critical to reset conn->gssenc there.

During connection setup, initialize conn->try_gss at the correct
place, else switching to a different server won't work right.

Remove now-redundant cleanup of GSS resources around one (and, for
some reason, only one) pqDropConnection call in connectDBStart.

Per report from Kyotaro Horiguchi that psql would freeze up,
rather than successfully resetting a GSS-encrypted connection
after a server restart.

This is YA oversight in commit b0b39f72b, so back-patch to v12.

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

4 years agoImprovements to psql \dAo and \dAp commands
Alexander Korotkov [Sat, 11 Jul 2020 11:14:49 +0000 (14:14 +0300)]
Improvements to psql \dAo and \dAp commands

 * Strategy number and purpose are essential information for opfamily operator.
   So, show those columns in non-verbose output.
 * "Left/right arg type" \dAp column names are confusing, because those type
   don't necessary match to function arguments.  Rename them to "Registered
   left/right type".
 * Replace manual assembling of operator/procedure names with casts to
   regoperator/regprocedure.
 * Add schema-qualification for pg_catalog functions and tables.

Reported-by: Peter Eisentraut, Tom Lane
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/2edc7b27-031f-b2b6-0db2-864241c91cb9%402ndquadrant.com
Backpatch-through: 13

4 years agoHashAgg: before spilling tuples, set unneeded columns to NULL.
Jeff Davis [Mon, 13 Jul 2020 00:48:49 +0000 (17:48 -0700)]
HashAgg: before spilling tuples, set unneeded columns to NULL.

This is a replacement for 4cad2534. Instead of projecting all tuples
going into a HashAgg, only remove unnecessary attributes when actually
spilling. This avoids the regression for the in-memory case.

Discussion: https://postgr.es/m/a2fb7dfeb4f50aa0a123e42151ee3013933cb802.camel%40j-davis.com
Backpatch-through: 13

4 years agoRevert "Use CP_SMALL_TLIST for hash aggregate"
Jeff Davis [Sun, 12 Jul 2020 23:46:19 +0000 (16:46 -0700)]
Revert "Use CP_SMALL_TLIST for hash aggregate"

This reverts commit 4cad2534da6d17067d98cf04be2dfc1bda8f2cd0 due to a
performance regression. It will be replaced by a new approach in an
upcoming commit.

Reported-by: Andres Freund
Discussion: https://postgr.es/m/20200614181418[email protected]
Backpatch-through: 13

4 years agoRevert "Track statistics for spilling of changes from ReorderBuffer".
Amit Kapila [Mon, 13 Jul 2020 02:57:40 +0000 (08:27 +0530)]
Revert "Track statistics for spilling of changes from ReorderBuffer".

The stats with this commit was available only for WALSenders, however,
users might want to see for backends doing logical decoding via SQL API.
Then, users might want to reset and access these stats across server
restart which was not possible with the current patch.

List of commits reverted:

caa3c4242c   Don't call elog() while holding spinlock.
e641b2a995   Doc: Update the documentation for spilled transaction
statistics.
5883f5fe27   Fix unportable printf format introduced in commit 9290ad198.
9290ad198b   Track statistics for spilling of changes from ReorderBuffer.

Additionaly, remove the release notes entry for this feature.

Backpatch-through: 13, where it was introduced
Discussion: https://postgr.es/m/CA+fd4k5_pPAYRTDrO2PbtTOe0eHQpBvuqmCr8ic39uTNmR49Eg@mail.gmail.com

4 years agoAvoid trying to restore table ACLs and per-column ACLs in parallel.
Tom Lane [Sat, 11 Jul 2020 17:36:50 +0000 (13:36 -0400)]
Avoid trying to restore table ACLs and per-column ACLs in parallel.

Parallel pg_restore has always supposed that ACL items for different
objects are independent and can be restored in parallel without
conflicts.  However, there is one case where this fails: because
REVOKE on a table is defined to also revoke the privilege(s) at
column level, we can't restore per-column ACLs till after we restore
any table-level privileges on their table.  Failure to honor this
restriction can lead to "tuple concurrently updated" errors during
parallel restore, or even to the per-column ACLs silently disappearing
because the table-level REVOKE is executed afterwards.

To fix, add a dependency from each column-level ACL item to its table's
ACL item, if there is one.  Note that this doesn't fix the hazard
for pre-existing archive files, only for ones made with a corrected
pg_dump.  Given that the bug's been there quite awhile without
field reports, I think this is acceptable.

This requires changing the API of pg_dump's dumpACL() function.
To keep its argument list from getting even longer, I removed the
"CatalogId objCatId" argument, which has been unused for ages.

Per report from Justin Pryzby.  Back-patch to all supported branches.

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

4 years agoForbid numeric NaN in jsonpath
Alexander Korotkov [Sat, 11 Jul 2020 00:21:00 +0000 (03:21 +0300)]
Forbid numeric NaN in jsonpath

SQL standard doesn't define numeric Inf or NaN values.  It appears even more
ridiculous to support then in jsonpath assuming JSON doesn't support these
values as well.  This commit forbids returning NaN from .double(), which was
previously allowed.  NaN can't be result of inner-jsonpath computation over
non-NaNs.  So, we can not expect NaN in the jsonpath output.

Reported-by: Tom Lane
Discussion: https://postgr.es/m/203949.1591879542%40sss.pgh.pa.us
Author: Alexander Korotkov
Reviewed-by: Tom Lane
Backpatch-through: 12

4 years agoImprove error reporting for jsonpath .double() method
Alexander Korotkov [Sat, 11 Jul 2020 00:20:46 +0000 (03:20 +0300)]
Improve error reporting for jsonpath .double() method

When jsonpath .double() method detects that numeric or string can't be
converted to double precision, it throws an error.  This commit makes these
errors explicitly express the reason of failure.

Discussion: https://postgr.es/m/CAPpHfdtqJtiSXkP7tOXez18NxhLUH_-75bL8%3DOce4Ki%2Bbv7V6Q%40mail.gmail.com
Author: Alexander Korotkov
Reviewed-by: Tom Lane
Backpatch-through: 12

4 years agoDoc: update or remove dead external links.
Tom Lane [Fri, 10 Jul 2020 17:16:00 +0000 (13:16 -0400)]
Doc: update or remove dead external links.

Re-point comp.ai.genetic FAQ link to a more stable address.

Remove stale links to AIX documentation; we don't really need to
tell AIX users how to use their systems.

Remove stale links to HP documentation about SSL.  We've had to
update those twice before, making it increasingly obvious that
HP does not intend them to be stable landing points.  They're
not particularly authoritative, either.  (This change effectively
reverts bbd3bdba3.)

Daniel Gustafsson and Álvaro Herrera, per a gripe from
Kyotaro Horiguchi.  Back-patch, since these links are
just as dead in the back branches.

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

4 years agoLog the location field before any backtrace
Peter Eisentraut [Fri, 10 Jul 2020 06:27:00 +0000 (08:27 +0200)]
Log the location field before any backtrace

This order makes more sense because the location is effectively at the
lowest level of the backtrace.

Discussion: https://www.postgresql.org/message-id/flat/90f5fa04-c410-a54e-9449-aa3749fb7972%402ndquadrant.com

4 years agoRemove WARNING message from brin_desummarize_range
Alvaro Herrera [Fri, 10 Jul 2020 00:13:25 +0000 (20:13 -0400)]
Remove WARNING message from brin_desummarize_range

This message was being emitted on the grounds that only crashed
summarization could cause it, but in reality even an aborted vacuum
could do it ... which makes it way too noisy, particularly since it
shows up in regression tests and makes them die.

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

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