postgresql.git
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.GK7349@momjian.us

Backpatch-through: 9.5

4 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).

4 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: petermpallesen@gmail.com
Discussion: https://postgr.es/m/159195294959.673.5752624528747900508@wrigleys.postgresql.org

Backpatch-through: 9.5

4 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.GA21852@nahmias.net

Backpatch-through: 9.5

4 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
<inttypes.h>'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

4 years agoAdd parens to ConvertToXSegs macro
Alvaro Herrera [Wed, 24 Jun 2020 17:57:21 +0000 (13:57 -0400)]
Add parens to ConvertToXSegs macro

The current definition (introduced in 9a3215026bd6) is dangerous.  No
bugs exist in our code at present, but backpatch to 11 nonetheless,
where that commit debuted.

Author: Álvaro Herrera <alvherre@alvh.no-ip.org>

4 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

4 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

4 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 <pryzby@telsasoft.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/20200611153753.GU14879@telsasoft.com

4 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

4 years agoFix C99isms introduced when backpatching atomics / spinlock tests.
Andres Freund [Thu, 18 Jun 2020 22:12:09 +0000 (15:12 -0700)]
Fix C99isms introduced when backpatching atomics / spinlock tests.

4 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.g6v3ydozy5txifji@alap3.anarazel.de
Backpatch: 9.5-

4 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.avzrctgv7476xj7i@alap3.anarazel.de

4 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

4 years agoFix oldest xmin and LSN computation across repslots after advancing
Michael Paquier [Thu, 18 Jun 2020 07:35:36 +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.kpltxxvjzislidks@alap3.anarazel.de
Backpatch-through: 11

4 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.

4 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.avzrctgv7476xj7i@alap3.anarazel.de
Backpatch: 9.5-

4 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

4 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 <amitdkhan.pg@gmail.com>
Reviewed-by: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Alvaro Herrera <alvherre@2ndquadrant.com>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Reviewed-by: Ibrar Ahmed <ibrar.ahmad@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CA%2BhUKGJE04G%3D8TLK0DLypT_27D9dR8F1RQgNp0jK6qR0tZGWOw%40mail.gmail.com

4 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.camel@cybertec.at

Backpatch-through: 9.5

4 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 <aphyr@jepsen.io>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Peter Geoghegan <pg@bowt.ie>
Reviewed-by: Tatsuo Ishii <ishii@sraoss.co.jp>
Discussion: https://postgr.es/m/db7b729d-0226-d162-a126-8a8ab2dc4443%40jepsen.io
Discussion: https://postgr.es/m/16454-9408996bb1750faf%40postgresql.org

4 years agodoc: remove xreflabels from commits 75fcdd2ae2 and 85af628da5
Bruce Momjian [Thu, 11 Jun 2020 22:19:25 +0000 (18:19 -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

4 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.)

4 years agoAvoid update conflict out serialization anomalies.
Peter Geoghegan [Thu, 11 Jun 2020 17:09:40 +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

4 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

4 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.

4 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-

4 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 <bossartn@amazon.com>
Author: Fujii Masao <masao.fujii@oss.nttdata.com>
Reviewed-by: Fujii Masao <masao.fujii@oss.nttdata.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/70BF24D6-DC51-443F-B55A-95735803842A%40amazon.com

4 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 <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CA%2BhUKGLmJUSwybaPQv39rB8ABpqJq84im2UjZvyUY4feYhpWMw%40mail.gmail.com

4 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).

4 years agoRefresh function name in CRC-associated Valgrind suppressions.
Noah Misch [Sat, 6 Jun 2020 03:10:53 +0000 (20:10 -0700)]
Refresh function name in CRC-associated Valgrind suppressions.

Back-patch to 9.5, where commit 4f700bcd20c087f60346cb8aefd0e269be8e2157
first appeared.

Reviewed by Tom Lane.  Reported by Andrew Dunstan.

Discussion: https://postgr.es/m/4dfabec2-a3ad-0546-2d62-f816c97edd0c@2ndQuadrant.com

4 years agoAdd unlikely() to CHECK_FOR_INTERRUPTS()
Joe Conway [Fri, 5 Jun 2020 20:49:32 +0000 (16:49 -0400)]
Add unlikely() to CHECK_FOR_INTERRUPTS()

Add the unlikely() branch hint macro to CHECK_FOR_INTERRUPTS().
Backpatch to REL_10_STABLE where we first started using unlikely().

Discussion: https://www.postgresql.org/message-id/flat/8692553c-7fe8-17d9-cbc1-7cddb758f4c6%40joeconway.com

4 years agoReject "23:59:60.nnn" in datetime input.
Tom Lane [Thu, 4 Jun 2020 20:42:08 +0000 (16:42 -0400)]
Reject "23:59:60.nnn" in datetime input.

It's intentional that we don't allow values greater than 24 hours,
while we do allow "24:00:00" as well as "23:59:60" as inputs.
However, the range check was miscoded in such a way that it would
accept "23:59:60.nnn" with a nonzero fraction.  For time or timetz,
the stored result would then be greater than "24:00:00" which would
fail dump/reload, not to mention possibly confusing other operations.

Fix by explicitly calculating the result and making sure it does not
exceed 24 hours.  (This calculation is redundant with what will happen
later in tm2time or tm2timetz.  Maybe someday somebody will find that
annoying enough to justify refactoring to avoid the duplication; but
that seems too invasive for a back-patched bug fix, and the cost is
probably unmeasurable anyway.)

Note that this change also rejects such input as the time portion
of a timestamp(tz) value.

Back-patch to v10.  The bug is far older, but to change this pre-v10
we'd need to ensure that the logic behaves sanely with float timestamps,
which is possibly nontrivial due to roundoff considerations.
Doesn't really seem worth troubling with.

Per report from Christoph Berg.

Discussion: https://postgr.es/m/20200520125807.GB296739@msg.df7cb.de

4 years agoFix instance of elog() called while holding a spinlock
Michael Paquier [Thu, 4 Jun 2020 01:18:06 +0000 (10:18 +0900)]
Fix instance of elog() called while holding a spinlock

This broke the project rule to not call any complex code while a
spinlock is held.  Issue introduced by b89e151.

Discussion: https://postgr.es/m/20200602.161518.1399689010416646074.horikyota.ntt@gmail.com
Backpatch-through: 9.5

4 years agoDon't call palloc() while holding a spinlock, either.
Tom Lane [Wed, 3 Jun 2020 16:36:00 +0000 (12:36 -0400)]
Don't call palloc() while holding a spinlock, either.

Fix some more violations of the "only straight-line code inside a
spinlock" rule.  These are hazardous not only because they risk
holding the lock for an excessively long time, but because it's
possible for palloc to throw elog(ERROR), leaving a stuck spinlock
behind.

copy_replication_slot() had two separate places that did pallocs
while holding a spinlock.  We can make the code simpler and safer
by copying the whole ReplicationSlot struct into a local variable
while holding the spinlock, and then referencing that copy.
(While that's arguably more cycles than we really need to spend
holding the lock, the struct isn't all that big, and this way seems
far more maintainable than copying fields piecemeal.  Anyway this
is surely much cheaper than a palloc.)  That bug goes back to v12.

InvalidateObsoleteReplicationSlots() not only did a palloc while
holding a spinlock, but for extra sloppiness then leaked the memory
--- probably for the lifetime of the checkpointer process, though
I didn't try to verify that.  Fortunately that silliness is new
in HEAD.

pg_get_replication_slots() had a cosmetic violation of the rule,
in that it only assumed it's safe to call namecpy() while holding
a spinlock.  Still, that's a hazard waiting to bite somebody, and
there were some other cosmetic coding-rule violations in the same
function, so clean it up.  I back-patched this as far as v10; the
code exists before that but it looks different, and this didn't
seem important enough to adapt the patch further back.

Discussion: https://postgr.es/m/20200602.161518.1399689010416646074.horikyota.ntt@gmail.com

4 years agoFix use-after-release mistake in currtid() and currtid2() for views
Michael Paquier [Mon, 1 Jun 2020 05:41:32 +0000 (14:41 +0900)]
Fix use-after-release mistake in currtid() and currtid2() for views

This issue has been present since the introduction of this code as of
a3519a2 from 2002, and has been found by buildfarm member prion that
uses RELCACHE_FORCE_RELEASE via the tests introduced recently in
e786be5.

Discussion: https://postgr.es/m/20200601022055.GB4121@paquier.xyz
Backpatch-through: 9.5

4 years agoMake install-tests target work with vpath builds
Andrew Dunstan [Sun, 31 May 2020 22:33:00 +0000 (18:33 -0400)]
Make install-tests target work with vpath builds

Also add a top-level install-tests target.

Backpatch to all live branches.

Craig Ringer, tweaked by me.

4 years agollvmjit: Fix building against LLVM 11 by removing unnecessary include.
Andres Freund [Thu, 28 May 2020 22:08:12 +0000 (15:08 -0700)]
llvmjit: Fix building against LLVM 11 by removing unnecessary include.

LLVM has removed this header, in the branch that will become llvm
11. But as it turns out we didn't actually need it, so just remove it.

Author: Jesse Zhang <sbjesse@gmail.com>
Discussion: https://postgr.es/m/CAGf+fX7bvtP0YXMu7pOsu_NwhxW6dArTkxb=jt7M2-UJkyJ_3g@mail.gmail.com
Backpatch: 11, where JIT support using llvm was introduced.

4 years agoInitialize dblink remoteConn struct in all cases
Joe Conway [Thu, 28 May 2020 17:45:02 +0000 (13:45 -0400)]
Initialize dblink remoteConn struct in all cases

Two of the members of rconn were left uninitialized. When
dblink_open() is called without an outer transaction it
handles the initialization for us, but with an outer
transaction it does not. Arrange for initialization
in all cases. Backpatch to all supported versions.

Reported-by: Alexander Lakhin
Discussion: https://www.postgresql.org/message-id/flat/9bd0744f-5f04-c778-c5b3-809efe9c30c7%40joeconway.com#c545909a41664991aca60c4d70a10ce7

4 years agoAdd CHECK_FOR_INTERRUPTS() to the repeat() function
Joe Conway [Thu, 28 May 2020 17:17:04 +0000 (13:17 -0400)]
Add CHECK_FOR_INTERRUPTS() to the repeat() function

The repeat() function loops for potentially a long time without
ever checking for interrupts. This prevents, for example, a query
cancel from interrupting until the work is all done. Fix by
inserting a CHECK_FOR_INTERRUPTS() into the loop.

Backpatch to all supported versions.

Discussion: https://www.postgresql.org/message-id/flat/8692553c-7fe8-17d9-cbc1-7cddb758f4c6%40joeconway.com

4 years agoAdd missing error code to "cannot attach index ..." error.
Heikki Linnakangas [Thu, 28 May 2020 09:37:00 +0000 (12:37 +0300)]
Add missing error code to "cannot attach index ..." error.

ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE was used in an ereport with the
same message but different errdetail a few lines earlier, so use that
here as well.

Backpatch-through: 11

4 years agoFix typo in test comment.
Heikki Linnakangas [Thu, 28 May 2020 09:35:18 +0000 (12:35 +0300)]
Fix typo in test comment.

The same comment was copied to a few different places, with the same typo.
Backpatch down to v11, where this typo was introduced.

4 years agoAdd a temp-install prerequisite to top-level "check-tests".
Noah Misch [Mon, 25 May 2020 23:21:04 +0000 (16:21 -0700)]
Add a temp-install prerequisite to top-level "check-tests".

The target failed, tested $PATH binaries, or tested a stale temporary
installation.  Commit c66b438db62748000700c9b90b585e756dd54141 missed
this.  Back-patch to 9.5 (all supported versions).

5 years agodoc: suggest 1.1 as a random_page_cost value for SSDs
Bruce Momjian [Fri, 22 May 2020 00:28:38 +0000 (20:28 -0400)]
doc:  suggest 1.1 as a random_page_cost value for SSDs

Reported-by: yigong hu
Discussion: https://postgr.es/m/CAOxFffcourucFqSk+tZA13ErS3XRYkDy6EeaPff4AvHGiEEuug@mail.gmail.com

Backpatch-through: 9.5

5 years agodoc: Simplify mention of unique indexes for NULL control
Bruce Momjian [Thu, 21 May 2020 23:49:30 +0000 (19:49 -0400)]
doc:  Simplify mention of unique indexes for NULL control

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

Backpatch-through: 9.5

5 years agoFix MSVC installations with multiple "configure" files detected
Michael Paquier [Thu, 21 May 2020 05:41:33 +0000 (14:41 +0900)]
Fix MSVC installations with multiple "configure" files detected

When installing binaries and libraries using the MSVC installation
routines, the operation gets done after moving to the root folder, whose
location is detected by checking if "configure" exists two times in a
row.  So, calling the installation script from src/tools/msvc/ with an
extra "configure" file four levels up the root path of the code tree
causes the execution to go further up, leading to a failure in finding
the builds.  This commit fixes the issue by moving to the root folder of
the code tree only once, when necessary.

Author: Arnold Müller
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/16343-f638f67e7e52b86c@postgresql.org
Backpatch-through: 9.5

5 years agoDoc: Fix description of pg_class.relreplident
Michael Paquier [Wed, 20 May 2020 05:21:50 +0000 (14:21 +0900)]
Doc: Fix description of pg_class.relreplident

The description missed a comma and lacked an explanation of what happens
with REPLICA IDENTITY USING INDEX when the dependent index is dropped.

Author: Marina Polyakova
Reviewed-by: Daniel Gustafsson, Michael Paquier
Discussion: https://postgr.es/m/ad1a0badc32658b1bbb07aa312346a1d@postgrespro.ru
Backpatch-through: 9.5

5 years agoFix comment in slot.c.
Amit Kapila [Mon, 18 May 2020 02:46:45 +0000 (08:16 +0530)]
Fix comment in slot.c.

Reported-by: Sawada Masahiko
Author: Sawada Masahiko
Reviewed-by: Amit Kapila
Backpatch-through: 9.5
Discussion: https://postgr.es/m/CA+fd4k4Ws7M7YQ8PqSym5WB1y75dZeBTd1sZJUQdfe0KJQ-iSA@mail.gmail.com

5 years agoFix assertion with relation using REPLICA IDENTITY FULL in subscriber
Michael Paquier [Sat, 16 May 2020 09:16:37 +0000 (18:16 +0900)]
Fix assertion with relation using REPLICA IDENTITY FULL in subscriber

In a logical replication subscriber, a table using REPLICA IDENTITY FULL
which has a primary key would try to use the primary key's index
available to scan for a tuple, but an assertion only assumed as correct
the case of an index associated to REPLICA IDENTITY USING INDEX.  This
commit corrects the assertion so as the use of a primary key index is a
valid case.

Reported-by: Dilip Kumar
Analyzed-by: Dilip Kumar
Author: Euler Taveira
Reviewed-by: Michael Paquier, Masahiko Sawada
Discussion: https://postgr.es/m/CAFiTN-u64S5bUiPL1q5kwpHNd0hRnf1OE-bzxNiOs5zo84i51w@mail.gmail.com
Backpatch-through: 10

5 years agoFix bogus initialization of replication origin shared memory state.
Tom Lane [Fri, 15 May 2020 23:05:39 +0000 (19:05 -0400)]
Fix bogus initialization of replication origin shared memory state.

The previous coding zeroed out offsetof(ReplicationStateCtl, states)
more bytes than it was entitled to, as a consequence of starting the
zeroing from the wrong pointer (or, if you prefer, using the wrong
calculation of how much to zero).

It's unsurprising that this has not caused any reported problems,
since it can be expected that the newly-allocated block is at the end
of what we've used in shared memory, and we always make the shmem
block substantially bigger than minimally necessary.  Nonetheless,
this is wrong and it could bite us someday; plus it's a dangerous
model for somebody to copy.

This dates back to the introduction of this code (commit 5aa235042),
so back-patch to all supported branches.

5 years agoAvoid killing btree items that are already dead
Alvaro Herrera [Fri, 15 May 2020 20:50:34 +0000 (16:50 -0400)]
Avoid killing btree items that are already dead

_bt_killitems marks btree items dead when a scan leaves the page where
they live, but it does so with only share lock (to improve concurrency).
This was historicall okay, since killing a dead item has no
consequences.  However, with the advent of data checksums and
wal_log_hints, this action incurs a WAL full-page-image record of the
page.  Multiple concurrent processes would write the same page several
times, leading to WAL bloat.  The probability of this happening can be
reduced by only killing items if they're not already dead, so change the
code to do that.

The problem could eliminated completely by having _bt_killitems upgrade
to exclusive lock upon seeing a killable item, but that would reduce
concurrency so it's considered a cure worse than the disease.

Backpatch all the way back to 9.5, since wal_log_hints was introduced in
9.4.

Author: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Discussion: https://postgr.es/m/CA+fd4k6PeRj2CkzapWNrERkja5G0-6D-YQiKfbukJV+qZGFZ_Q@mail.gmail.com

5 years agodocs: add xreflabel entries for autovacuum, SP-GiST, and TOAST
Bruce Momjian [Fri, 15 May 2020 16:38:40 +0000 (12:38 -0400)]
docs:  add xreflabel entries for autovacuum, SP-GiST, and TOAST

This is for use by the PG 13 release notes, but might be used for minor
release notes in the future.

Backpatch-through: 9.5

5 years agodoc: add missing xreflabels to the main docs (not refs)
Bruce Momjian [Fri, 15 May 2020 16:05:43 +0000 (12:05 -0400)]
doc:  add missing xreflabels to the main docs (not refs)

Add missing xreflabels for index types, geqo, libpq, spi, server-side
languages, ecpg, and vaacuumlo.

Backpatch-through: 9.5

5 years agodoc: remove extra blank line at the top of SGML files
Bruce Momjian [Fri, 15 May 2020 13:55:43 +0000 (09:55 -0400)]
doc:  remove extra blank line at the top of SGML files

Backpatch-through: 9.5

5 years agodoc: make ref/*.sgml file header comment layout consistent
Bruce Momjian [Fri, 15 May 2020 12:52:24 +0000 (08:52 -0400)]
doc:  make ref/*.sgml file header comment layout consistent

5 years agoFix amcheck for page checks concurrent to replay of btree page deletion
Alexander Korotkov [Wed, 6 May 2020 12:35:27 +0000 (15:35 +0300)]
Fix amcheck for page checks concurrent to replay of btree page deletion

amcheck expects at least hikey to always exist on leaf page even if it is
deleted page.  But replica reinitializes page during replay of page deletion,
causing deleted page to have no items.  Thus, replay of page deletion can
cause an error in concurrent amcheck run.

This commit relaxes amcheck expectation making it tolerate deleted page with
no items.

Reported-by: Konstantin Knizhnik
Discussion: https://postgr.es/m/CAPpHfdt_OTyQpXaPJcWzV2N-LNeNJseNB-K_A66qG%3DL518VTFw%40mail.gmail.com
Author: Alexander Korotkov
Reviewed-by: Peter Geoghegan
Backpatch-through: 11

5 years agoFix the MSVC build for versions 2015 and later.
Amit Kapila [Thu, 14 May 2020 04:09:04 +0000 (09:39 +0530)]
Fix the MSVC build for versions 2015 and later.

Visual Studio 2015 and later versions should still be able to do the same
as Visual Studio 2012, but the declaration of locale_name is missing in
_locale_t, causing the code compilation to fail, hence this falls back
instead on to enumerating all system locales by using EnumSystemLocalesEx
to find the required locale name.  If the input argument is in Unix-style
then we can get ISO Locale name directly by using GetLocaleInfoEx() with
LCType as LOCALE_SNAME.

In passing, change the documentation references of the now obsolete links.

Note that this problem occurs only with NLS enabled builds.

Author: Juan José Santamaría Flecha, Davinder Singh and Amit Kapila
Reviewed-by: Ranier Vilela and Amit Kapila
Backpatch-through: 9.5
Discussion: https://postgr.es/m/CAHzhFSFoJEWezR96um4-rg5W6m2Rj9Ud2CNZvV4NWc9tXV7aXQ@mail.gmail.com

5 years agoFix pg_recvlogical avoidance of superfluous Standby Status Update.
Noah Misch [Thu, 14 May 2020 03:42:09 +0000 (20:42 -0700)]
Fix pg_recvlogical avoidance of superfluous Standby Status Update.

The defect suppressed a Standby Status Update message when bytes flushed
to disk had changed but bytes received had not changed.  If
pg_recvlogical then exited with no intervening Standby Status Update,
the next pg_recvlogical repeated already-flushed records.  The defect
could also cause superfluous messages, which are functionally harmless.
Back-patch to 9.5 (all supported versions).

Discussion: https://postgr.es/m/20200502221647.GA3941274@rfd.leadboat.com

5 years agoIn successful pg_recvlogical, end PGRES_COPY_OUT cleanly.
Noah Misch [Thu, 14 May 2020 03:42:09 +0000 (20:42 -0700)]
In successful pg_recvlogical, end PGRES_COPY_OUT cleanly.

pg_recvlogical merely called PQfinish(), so the backend sent messages
after the disconnect.  When that caused EPIPE in internal_flush(),
before a LogicalConfirmReceivedLocation(), the next pg_recvlogical would
repeat already-acknowledged records.  Whether or not the defect causes
EPIPE, post-disconnect messages could contain an ErrorResponse that the
user should see.  One properly ends PGRES_COPY_OUT by repeating
PQgetCopyData() until it returns a negative value.  Augment one of the
tests to cover the case of WAL past --endpos.  Back-patch to v10, where
commit 7c030783a5bd07cadffc2a1018bc33119a4c7505 first appeared.  Before
that commit, pg_recvlogical never reached PGRES_COPY_OUT.

Reported by Thomas Munro.

Discussion: https://postgr.es/m/CAEepm=1MzM2Z_xNe4foGwZ1a+MO_2S9oYDq3M5D11=JDU_+0Nw@mail.gmail.com

5 years agoStamp 11.8. REL_11_8
Tom Lane [Mon, 11 May 2020 21:10:48 +0000 (17:10 -0400)]
Stamp 11.8.

5 years agoTranslation updates
Peter Eisentraut [Mon, 11 May 2020 11:24:12 +0000 (13:24 +0200)]
Translation updates

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

5 years agoRelease notes for 12.3, 11.8, 10.13, 9.6.18, 9.5.22.
Tom Lane [Sun, 10 May 2020 19:05:59 +0000 (15:05 -0400)]
Release notes for 12.3, 11.8, 10.13, 9.6.18, 9.5.22.

5 years agoPrevent archive recovery from scanning non-existent WAL files.
Fujii Masao [Tue, 7 Apr 2020 15:49:29 +0000 (00:49 +0900)]
Prevent archive recovery from scanning non-existent WAL files.

Previously when there were multiple timelines listed in the history file
of the recovery target timeline, archive recovery searched all of them,
starting from the newest timeline to the oldest one, to find the segment
to read. That is, archive recovery had to continuously fail scanning
the segment until it reached the timeline that the segment belonged to.
These scans for non-existent segment could be harmful on the recovery
performance especially when archival area was located on the remote
storage and each scan could take a long time.

To address the issue, this commit changes archive recovery so that
it skips scanning the timeline that the segment to read doesn't belong to.

Per discussion, back-patch to all supported versions.

Author: Kyotaro Horiguchi, tweaked a bit by Fujii Masao
Reviewed-by: David Steele, Pavel Suderevsky, Grigory Smolkin
Discussion: https://postgr.es/m/16159-f5a34a3a04dc67e0@postgresql.org
Discussion: https://postgr.es/m/20200129.120222.1476610231001551715.horikyota.ntt@gmail.com

5 years agopg_restore: Provide file name with one failure message
Alvaro Herrera [Fri, 8 May 2020 23:38:46 +0000 (19:38 -0400)]
pg_restore: Provide file name with one failure message

Almost all error messages already include file name where relevant, but
this one had been overlooked.  Repair.

Backpatch to 9.5.

Author: Euler Taveira <euler.taveira@2ndquadrant.com>
Discussion: https://postgr.es/m/CAH503wA_VOrcKL_43p9atRejCDYmOZ8MzfK9S6TJrQqBqNeAXA@mail.gmail.com
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
5 years agoPropagate ALTER TABLE ... SET STORAGE to indexes
Peter Eisentraut [Thu, 9 Apr 2020 12:10:01 +0000 (14:10 +0200)]
Propagate ALTER TABLE ... SET STORAGE to indexes

When creating a new index, the attstorage setting of the table column
is copied to regular (non-expression) index columns.  But a later
ALTER TABLE ... SET STORAGE is not propagated to indexes, thus
creating an inconsistent and undumpable state.

Discussion: https://www.postgresql.org/message-id/flat/9765d72b-37c0-06f5-e349-2a580aafd989%402ndquadrant.com

5 years agoFix inconsistency in pg_buffercache docs.
Amit Kapila [Fri, 8 May 2020 03:26:03 +0000 (08:56 +0530)]
Fix inconsistency in pg_buffercache docs.

Commit 6e654546fb avoids locking bufmgr partitions to make pg_buffercache
less disruptive on production systems but forgot to update the docs.

Reported-by: Sawada Masahiko
Author: Sawada Masahiko
Reviewed-by: Amit Kapila
Backpatch-through: 10
Discussion: https://postgr.es/m/CA+fd4k6sD8oeP1qJbFAor=rCpYckU9DsywHiYx3x5Hz5Z8Ua_w@mail.gmail.com

5 years agoReport missing wait event for timeline history file.
Fujii Masao [Fri, 8 May 2020 01:36:40 +0000 (10:36 +0900)]
Report missing wait event for timeline history file.

TimelineHistoryRead and TimelineHistoryWrite wait events are reported
during waiting for a read and write of a timeline history file, respectively.
However, previously, TimelineHistoryRead wait event was not reported
while readTimeLineHistory() was reading a timeline history file. Also
TimelineHistoryWrite was not reported while writeTimeLineHistory() was
writing one line with the details of the timeline split, at the end.
This commit fixes these issues.

Back-patch to v10 where wait events for a timeline history file was added.

Author: Masahiro Ikeda
Reviewed-by: Michael Paquier, Fujii Masao
Discussion: https://postgr.es/m/d11b0c910b63684424e06772eb844ab5@oss.nttdata.com

5 years agoFix YA text phrase search bug.
Tom Lane [Thu, 7 May 2020 19:59:52 +0000 (15:59 -0400)]
Fix YA text phrase search bug.

checkcondition_str() failed to report multiple matches for a prefix
pattern correctly: it would dutifully merge the match positions, but
then after exiting that loop, if the last prefix-matching word had
had no suitable positions, it would report there were no matches.
The upshot would be failing to recognize a match that the query
should match.

It looks like you need all of these conditions to see the bug:
* a phrase search (else we don't ask for match position details)
* a prefix search item (else we don't get to this code)
* a weight restriction (else checkclass_str won't fail)

Noted while investigating a problem report from Pavel Borisov,
though this is distinct from the issue he was on about.

Back-patch to 9.6 where phrase search was added.

5 years agoHeed lock protocol in DROP OWNED BY
Alvaro Herrera [Wed, 6 May 2020 16:29:41 +0000 (12:29 -0400)]
Heed lock protocol in DROP OWNED BY

We were acquiring object locks then deleting objects one by one, instead
of acquiring all object locks first, ignoring those that did not exist,
and then deleting all objects together.   The latter is the correct
protocol to use, and what this commits changes to code to do.  Failing
to follow that leads to "cache lookup failed for relation XYZ" error
reports when DROP OWNED runs concurrently with other DDL -- for example,
a session termination that removes some temp tables.

Author: Álvaro Herrera
Reported-by: Mithun Chicklore Yogendra (Mithun CY)
Reviewed-by: Ahsan Hadi, Tom Lane
Discussion: https://postgr.es/m/CADq3xVZTbzK4ZLKq+dn_vB4QafXXbmMgDP3trY-GuLnib2Ai1w@mail.gmail.com

5 years agoHandle spaces for Python install location in MSVC scripts
Michael Paquier [Wed, 6 May 2020 12:08:26 +0000 (21:08 +0900)]
Handle spaces for Python install location in MSVC scripts

Attempting to use an installation path of Python that includes spaces
caused the MSVC builds to fail.  This fixes the issue by using the same
quoting method as ad7595b for OpenSSL.

Author: Victor Wagner
Discussion: https://postgr.es/m/20200430150608.6dc6b8c4@antares.wagner.home
Backpatch-through: 9.5

5 years agoGet rid of trailing semicolons in C macro definitions.
Tom Lane [Fri, 1 May 2020 21:28:01 +0000 (17:28 -0400)]
Get rid of trailing semicolons in C macro definitions.

Writing a trailing semicolon in a macro is almost never the right thing,
because you almost always want to write a semicolon after each macro
call instead.  (Even if there was some reason to prefer not to, pgindent
would probably make a hash of code formatted that way; so within PG the
rule should basically be "don't do it".)  Thus, if we have a semi inside
the macro, the compiler sees "something;;".  Much of the time the extra
empty statement is harmless, but it could lead to mysterious syntax
errors at call sites.  In perhaps an overabundance of neatnik-ism, let's
run around and get rid of the excess semicolons whereever possible.

The only thing worse than a mysterious syntax error is a mysterious
syntax error that only happens in the back branches; therefore,
backpatch these changes where relevant, which is most of them because
most of these mistakes are old.  (The lack of reported problems shows
that this is largely a hypothetical issue, but still, it could bite
us in some future patch.)

John Naylor and Tom Lane

Discussion: https://postgr.es/m/CACPNZCs0qWTqJ2QUSGJ07B7uvAvzMb-KbG2q+oo+J3tsWN5cqw@mail.gmail.com

5 years agoClear up issue with FSM and oldest bpto.xact.
Peter Geoghegan [Fri, 1 May 2020 19:19:39 +0000 (12:19 -0700)]
Clear up issue with FSM and oldest bpto.xact.

On further reflection, code comments added by commit b0229f26 slightly
misrepresented how we determine the oldest bpto.xact for the index.
btvacuumpage() does not treat the bpto.xact of a page that it put in the
FSM as a candidate to be the oldest deleted page (the delete-marked page
that has the oldest bpto.xact XID among all pages encountered).

The definition of a deleted page for the purposes of the bpto.xact
calculation is different from the definition used by the bulk delete
statistics.  The bulk delete statistics don't distinguish between pages
that were deleted by the current VACUUM, pages deleted by a previous
VACUUM operation but not yet recyclable/reusable, and pages that are
reusable (though reusable pages are counted separately).

Backpatch: 11-, just like commit b0229f26.

5 years agoFix undercounting in VACUUM VERBOSE output.
Peter Geoghegan [Fri, 1 May 2020 16:51:06 +0000 (09:51 -0700)]
Fix undercounting in VACUUM VERBOSE output.

The logic for determining how many nbtree pages in an index are deleted
pages sometimes undercounted pages.  Pages that were deleted by the
current VACUUM operation (as opposed to some previous VACUUM operation
whose deleted pages have yet to be reused) were sometimes overlooked.
The final count is exposed to users through VACUUM VERBOSE's "%u index
pages have been deleted" output.

btvacuumpage() avoided double-counting when _bt_pagedel() deleted more
than one page by assuming that only one page was deleted, and that the
additional deleted pages would get picked up during a future call to
btvacuumpage() by the same VACUUM operation.  _bt_pagedel() can
legitimately delete pages that the btvacuumscan() scan will not visit
again, though, so that assumption was slightly faulty.

Fix the accounting by teaching _bt_pagedel() about its caller's
requirements.  It now only reports on pages that it knows btvacuumscan()
won't visit again (including the current btvacuumpage() page), so
everything works out in the end.

This bug has been around forever.  Only backpatch to v11, though, to
keep _bt_pagedel() is sync on the branches that have today's bugfix
commit b0229f26da.  Note that this commit changes the signature of
_bt_pagedel(), just like commit b0229f26da.

Author: Peter Geoghegan
Reviewed-By: Masahiko Sawada
Discussion: https://postgr.es/m/CAH2-WzkrXBcMQWAYUJMFTTvzx_r4q=pYSjDe07JnUXhe+OZnJA@mail.gmail.com
Backpatch: 11-

5 years agoFix bug in nbtree VACUUM "skip full scan" feature.
Peter Geoghegan [Fri, 1 May 2020 15:39:49 +0000 (08:39 -0700)]
Fix bug in nbtree VACUUM "skip full scan" feature.

Commit 857f9c36cda (which taught nbtree VACUUM to skip a scan of the
index from btcleanup in situations where it doesn't seem worth it) made
VACUUM maintain the oldest btpo.xact among all deleted pages for the
index as a whole.  It failed to handle all the details surrounding pages
that are deleted by the current VACUUM operation correctly (though pages
deleted by some previous VACUUM operation were processed correctly).

The most immediate problem was that the special area of the page was
examined without a buffer pin at one point.  More fundamentally, the
handling failed to account for the full range of _bt_pagedel()
behaviors.  For example, _bt_pagedel() sometimes deletes internal pages
in passing, as part of deleting an entire subtree with btvacuumpage()
caller's page as the leaf level page.  The original leaf page passed to
_bt_pagedel() might not be the page that it deletes first in cases where
deletion can take place.

It's unclear how disruptive this bug may have been, or what symptoms
users might want to look out for.  The issue was spotted during
unrelated code review.

To fix, push down the logic for maintaining the oldest btpo.xact to
_bt_pagedel().  btvacuumpage() is now responsible for pages that were
fully deleted by a previous VACUUM operation, while _bt_pagedel() is now
responsible for pages that were deleted by the current VACUUM operation
(this includes half-dead pages from a previous interrupted VACUUM
operation that become fully deleted in _bt_pagedel()).  Note that
_bt_pagedel() should never encounter an existing deleted page.

This commit theoretically breaks the ABI of a stable release by changing
the signature of _bt_pagedel().  However, if any third party extension
is actually affected by this, then it must already be completely broken
(since there are numerous assumptions made in _bt_pagedel() that cannot
be met outside of VACUUM).  It seems highly unlikely that such an
extension actually exists, in any case.

Author: Peter Geoghegan
Reviewed-By: Masahiko Sawada
Discussion: https://postgr.es/m/CAH2-WzkrXBcMQWAYUJMFTTvzx_r4q=pYSjDe07JnUXhe+OZnJA@mail.gmail.com
Backpatch: 11-, where the "skip full scan" feature was introduced.

5 years agoFix full text search to handle NOT above a phrase search correctly.
Tom Lane [Mon, 27 Apr 2020 16:21:04 +0000 (12:21 -0400)]
Fix full text search to handle NOT above a phrase search correctly.

Queries such as '!(foo<->bar)' failed to find matching rows when
implemented as a GiST or GIN index search.  That's because of
failing to handle phrase searches as tri-valued when considering
a query without any position information for the target tsvector.
We can only say that the phrase operator might match, not that it
does match; and therefore its NOT also might match.  The previous
coding incorrectly inverted the approximate phrase result to
decide that there was certainly no match.

To fix, we need to make TS_phrase_execute return a real ternary result,
and then bubble that up accurately in TS_execute.  As long as we have
to do that anyway, we can simplify the baroque things TS_phrase_execute
was doing internally to manage tri-valued searching with only a bool
as explicit result.

For now, I left the externally-visible result of TS_execute as a plain
bool.  There do not appear to be any outside callers that need to
distinguish a three-way result, given that they passed in a flag
saying what to do in the absence of position data.  This might need
to change someday, but we wouldn't want to back-patch such a change.

Although tsginidx.c has its own TS_execute_ternary implementation for
use at upper index levels, that sadly managed to get this case wrong
as well :-(.  Fixing it is a lot easier fortunately.

Per bug #16388 from Charles Offenbacher.  Back-patch to 9.6 where
phrase search was introduced.

Discussion: https://postgr.es/m/16388-98cffba38d0b7e6e@postgresql.org

5 years agoFix error case for CREATE ROLE ... IN ROLE.
Andrew Gierth [Sat, 25 Apr 2020 04:10:19 +0000 (05:10 +0100)]
Fix error case for CREATE ROLE ... IN ROLE.

CreateRole() was passing a Value node, not a RoleSpec node, for the
newly-created role name when adding the role as a member of existing
roles for the IN ROLE syntax.

This mistake went unnoticed because the node in question is used only
for error messages and is not accessed on non-error paths.

In older pg versions (such as 9.5 where this was found), this results
in an "unexpected node type" error in place of the real error. That
node type check was removed at some point, after which the code would
accidentally fail to fail on 64-bit platforms (on which accessing the
Value node as if it were a RoleSpec would be mostly harmless) or give
an "unexpected role type" error on 32-bit platforms.

Fix the code to pass the correct node type, and add an lfirst_node
assertion just in case.

Per report on irc from user m1chelangelo.

Backpatch all the way, because this error has been around for a long
time.

5 years agoUpdate Windows timezone name list to include currently-known zones.
Tom Lane [Fri, 24 Apr 2020 21:53:23 +0000 (17:53 -0400)]
Update Windows timezone name list to include currently-known zones.

Thanks to Juan José Santamaría Flecha.

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

5 years agoImprove placement of "display name" comment in win32_tzmap[] entries.
Tom Lane [Fri, 24 Apr 2020 21:21:44 +0000 (17:21 -0400)]
Improve placement of "display name" comment in win32_tzmap[] entries.

Sticking this comment at the end of the last line was a bad idea: it's
not particularly readable, and it tempts pgindent to mess with line
breaks within the comment, which in turn reveals that win32tzlist.pl's
clean_displayname() does the wrong thing to clean up such line breaks.
While that's not hard to fix, there's basically no excuse for this
arrangement to begin with, especially since it makes the table layout
needlessly vary across back branches with different pgindent rules.
Let's just put the comment inside the braces, instead.

This commit just moves and reformats the comments, and updates
win32tzlist.pl to match; there's no actual data change.

Per odd-looking results from Juan José Santamaría Flecha.
Back-patch, since the point is to make win32_tzmap[] look the
same in all supported branches again.

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

5 years agoRepair performance regression in information_schema.triggers view.
Tom Lane [Fri, 24 Apr 2020 16:02:36 +0000 (12:02 -0400)]
Repair performance regression in information_schema.triggers view.

Commit 32ff26911 introduced use of rank() into the triggers view to
calculate the spec-mandated action_order column.  As written, this
prevents query constraints on the table-name column from being pushed
below the window aggregate step.  That's bad for performance of this
typical usage pattern, since the view now has to be evaluated for all
tables not just the one(s) the user wants to see.  It's also the cause
of some recent buildfarm failures, in which trying to evaluate the view
rows for triggers in process of being dropped resulted in "cache lookup
failed for function NNN" errors.  Those rows aren't of interest to the
test script doing the query, but the filter that would eliminate them
is being applied too late.  None of this happened before the rank()
call was there, so it's a regression compared to v10 and before.

We can improve matters by changing the rank() call so that instead of
partitioning by OIDs, it partitions by nspname and relname, casting
those to sql_identifier so that they match the respective view output
columns exactly.  The planner has enough intelligence to know that
constraints on partitioning columns are safe to push down, so this
eliminates the performance problem and the regression test failure
risk.  We could make the other partitioning columns match view outputs
as well, but it'd be more complicated and the performance benefits
are questionable.

Side note: as this stands, the planner will push down constraints on
event_object_table and trigger_schema, but not on event_object_schema,
because it checks for ressortgroupref matches not expression
equivalence.  That might be worth improving someday, but it's not
necessary to fix the immediate concern.

Back-patch to v11 where the rank() call was added.  Ordinarily we'd not
change information_schema in released branches, but the test failure has
been seen in v12 and presumably could happen in v11 as well, so we need
to do this to keep the buildfarm happy.  The change is harmless so far
as users are concerned.  Some might wish to apply it to existing
installations if performance of this type of query is of concern,
but those who don't are no worse off.

I bumped catversion in HEAD as a pro forma matter (there's no
catalog incompatibility that would really require a re-initdb).
Obviously that can't be done in the back branches.

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

5 years agoUpdate time zone data files to tzdata release 2020a.
Tom Lane [Fri, 24 Apr 2020 14:54:47 +0000 (10:54 -0400)]
Update time zone data files to tzdata release 2020a.

DST law changes in Morocco and the Canadian Yukon.
Historical corrections for Shanghai.

The America/Godthab zone is renamed to America/Nuuk to reflect
current English usage; however, the old name remains available as a
compatibility link.

5 years agoRemove some unstable parts from new TAP test for archive status check
Michael Paquier [Fri, 24 Apr 2020 02:34:03 +0000 (11:34 +0900)]
Remove some unstable parts from new TAP test for archive status check

The test is proving to have timing issues when looking at archive status
files on standbys after crash recovery, while other parts of the test
rely on pg_stat_archiver as a wait point to make sure that a given state
of the archiving is reached.  The coverage is not heavily impacted by
the removal those extra tests.

Per reports from several buildfarm animals, like crake, piculet,
culicidae and francolin.

Discussion: https://postgr.es/m/20200424005929.GK33034@paquier.xyz
Backpatch-through: 9.5

5 years agoFix handling of WAL segments ready to be archived during crash recovery
Michael Paquier [Thu, 23 Apr 2020 23:48:40 +0000 (08:48 +0900)]
Fix handling of WAL segments ready to be archived during crash recovery

78ea8b5 has fixed an issue related to the recycling of WAL segments on
standbys depending on archive_mode.  However, it has introduced a
regression with the handling of WAL segments ready to be archived during
crash recovery, causing those files to be recycled without getting
archived.

This commit fixes the regression by tracking in shared memory if a live
cluster is either in crash recovery or archive recovery as the handling
of WAL segments ready to be archived is different in both cases (those
WAL segments should not be removed during crash recovery), and by using
this new shared memory state to decide if a segment can be recycled or
not.  Previously, it was not possible to know if a cluster was in crash
recovery or archive recovery as the shared state was able to track only
if recovery was happening or not, leading to the problem.

A set of TAP tests is added to close the gap here, making sure that WAL
segments ready to be archived are correctly handled when a cluster is in
archive or crash recovery with archive_mode set to "on" or "always", for
both standby and primary.

Reported-by: Benoît Lobréau
Author: Jehan-Guillaume de Rorthais
Reviewed-by: Kyotaro Horiguchi, Fujii Masao, Michael Paquier
Discussion: https://postgr.es/m/20200331172229.40ee00dc@firost
Backpatch-through: 9.5

5 years agodocs: land height is "elevation", not "altitude"
Bruce Momjian [Wed, 22 Apr 2020 20:23:19 +0000 (16:23 -0400)]
docs:  land height is "elevation", not "altitude"

See https://mapscaping.com/blogs/geo-candy/what-is-the-difference-between-elevation-relief-and-altitude
No patching of regression tests.

Reported-by: taf1@cornell.edu
Discussion: https://postgr.es/m/158506544539.679.2278386310645558048@wrigleys.postgresql.org

Backpatch-through: 9.5

5 years agoFix memory leak in libpq when using sslmode=verify-full
Michael Paquier [Tue, 21 Apr 2020 22:27:49 +0000 (07:27 +0900)]
Fix memory leak in libpq when using sslmode=verify-full

Checking if Subject Alternative Names (SANs) from a certificate match
with the hostname connected to leaked memory after each lookup done.

This is broken since acd08d7 that added support for SANs in SSL
certificates, so backpatch down to 9.5.

Author: Roman Peshkurov
Reviewed-by: Hamid Akhtar, Michael Paquier, David Steele
Discussion: https://postgr.es/m/CALLDf-pZ-E3mjxd5=bnHsDu9zHEOnpgPgdnO84E2RuwMCjjyPw@mail.gmail.com
Backpatch-through: 9.5

5 years agoDocument partitiong tables ancillary object handling some more
Alvaro Herrera [Tue, 21 Apr 2020 21:14:18 +0000 (17:14 -0400)]
Document partitiong tables ancillary object handling some more

Add a couple of lines to make it explicit that indexes, constraints,
triggers are added, removed, or left alone.

Backpatch to pg11.

Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/20200421162038.GA18628@alvherre.pgsql

5 years agoFix possible crash during FATAL exit from reindexing.
Tom Lane [Tue, 21 Apr 2020 19:58:42 +0000 (15:58 -0400)]
Fix possible crash during FATAL exit from reindexing.

index.c supposed that it could just use a PG_TRY block to clean up the
state associated with an active REINDEX operation.  However, that code
doesn't run if we do a FATAL exit --- for example, due to a SIGTERM
shutdown signal --- while the REINDEX is happening.  And that state does
get consulted during catalog accesses, which makes it problematic if we
do any catalog accesses during shutdown --- for example, to clean up any
temp tables created in the session.

If this combination of circumstances occurred, we could find ourselves
trying to access already-freed memory.  In debug builds that'd fairly
reliably cause an assertion failure.  In production we might often
get away with it, but with some bad luck it could cause a core dump.

Another possible bad outcome is an erroneous conclusion that an
index-to-be-accessed is being reindexed; but it looks like that would
be unlikely to have any consequences worse than failing to drop temp
tables right away.  (They'd still get dropped by the next session that
uses that temp schema.)

To fix, get rid of the use of PG_TRY here, and instead hook into
the transaction abort mechanisms to clean up reindex state.

Per bug #16378 from Alexander Lakhin.  This has been wrong for a
very long time, so back-patch to all supported branches.

Discussion: https://postgr.es/m/16378-7a70ca41b3ec2009@postgresql.org

5 years agoFix minor violations of FunctionCallInvoke usage protocol.
Tom Lane [Tue, 21 Apr 2020 18:23:42 +0000 (14:23 -0400)]
Fix minor violations of FunctionCallInvoke usage protocol.

Working on commit 1c455078b led me to check through FunctionCallInvoke
call sites to see if every one was being honest about (a) making sure
that fcinfo.isnull is initially false, and (b) checking its state after
the call.  Sure enough, I found some violations.

The main one is that finalize_partialaggregate re-used serialfn_fcinfo
without resetting isnull, even though it clearly intends to cater for
serialfns that return NULL.  There would only be an issue with a
non-strict serialfn, since it's unlikely that a serialfn would return
NULL for non-null input.  We have no non-strict serialfns in core, and
there may be none in the wild either, which would account for the lack
of complaints.  Still, it's clearly wrong, so back-patch that fix to
9.6 where finalize_partialaggregate was introduced.

Also, arrayfuncs.c and rowtypes.c contained various callers that were
not bothering to check for result nulls.  While what's being called is
a comparison or hash function that probably *shouldn't* return null,
that's a lousy excuse for not having any check at all.  There are
existing places that just Assert(!fcinfo->isnull) in comparable
situations, so I added that to the places that were calling btree
comparison or hash support functions.  In the places calling
boolean-returning equality functions, it's quite cheap to have them
treat isnull as FALSE, so make those places do that.  Also remove some
"locfcinfo->isnull = false" assignments that are unnecessary given the
assumption that no previous call returned null.  These changes seem like
mostly neatnik-ism or debugging support, so I didn't back-patch.

5 years agoFix detaching partitions with cloned row triggers
Alvaro Herrera [Tue, 21 Apr 2020 17:57:00 +0000 (13:57 -0400)]
Fix detaching partitions with cloned row triggers

When a partition is detached, any triggers that had been cloned from its
parent were not properly disentangled from its parent triggers.
This resulted in triggers that could not be dropped because they
depended on the trigger in the trigger in the no-longer-parent table:
  ALTER TABLE t DETACH PARTITION t1;
  DROP TRIGGER trig ON t1;
    ERROR:  cannot drop trigger trig on table t1 because trigger trig on table t requires it
    HINT:  You can drop trigger trig on table t instead.

Moreover the table can no longer be re-attached to its parent, because
the trigger name is already taken:
  ALTER TABLE t ATTACH PARTITION t1 FOR VALUES FROM (1)TO(2);
    ERROR:  trigger "trig" for relation "t1" already exists

The former is a bug introduced in commit 86f575948c77.  (The latter is
not necessarily a bug, but it makes the bug more uncomfortable.)

To avoid the complexity that would be needed to tell whether the trigger
has a local definition that has to be merged with the one coming from
the parent table, establish the behavior that the trigger is removed
when the table is detached.

Backpatch to pg11.

Author: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Amit Langote <amitlangote09@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://www.postgresql.org/message-id/flat/20200408152412.GZ2228@telsasoft.com

5 years agodoc: change SGML markup "figure" to "example"
Bruce Momjian [Tue, 21 Apr 2020 01:41:13 +0000 (21:41 -0400)]
doc:  change SGML markup "figure" to "example"

Reported-by: Jürgen Purtz
Discussion: https://postgr.es/m/709d7809-d7f4-8175-47f3-4d131341bba8@purtz.de

Author: Jürgen Purtz

Backpatch-through: 9.5

5 years agoAllow pg_read_all_stats to access all stats views again
Magnus Hagander [Mon, 20 Apr 2020 10:53:40 +0000 (12:53 +0200)]
Allow pg_read_all_stats to access all stats views again

The views pg_stat_progress_* had not gotten the memo that
pg_read_all_stats is supposed to be able to read all statistics. Also
make a pass over all text-returning pg_stat_xyz functions that could
return "insufficient privilege" and make sure they also respect
pg_read_all_status.

Reported-by: Andrey M. Borodin
Reviewed-by: Andrey M. Borodin, Kyotaro Horiguchi
Discussion: https://postgr.es/m/13145F2F-8458-4977-9D2D-7B2E862E5722@yandex-team.ru

5 years agoFix race conditions in synchronous standby management.
Tom Lane [Sat, 18 Apr 2020 18:02:44 +0000 (14:02 -0400)]
Fix race conditions in synchronous standby management.

We have repeatedly seen the buildfarm reach the Assert(false) in
SyncRepGetSyncStandbysPriority.  This apparently is due to failing to
consider the possibility that the sync_standby_priority values in
shared memory might be inconsistent; but they will be whenever only
some of the walsenders have updated their values after a change in
the synchronous_standby_names setting.  That function is vastly too
complex for what it does, anyway, so rewriting it seems better than
trying to apply a band-aid fix.

Furthermore, the API of SyncRepGetSyncStandbys is broken by design:
it returns a list of WalSnd array indexes, but there is nothing
guaranteeing that the contents of the WalSnd array remain stable.
Thus, if some walsender exits and then a new walsender process
takes over that WalSnd array slot, a caller might make use of
WAL position data that it should not, potentially leading to
incorrect decisions about whether to release transactions that
are waiting for synchronous commit.

To fix, replace SyncRepGetSyncStandbys with a new function
SyncRepGetCandidateStandbys that copies all the required data
from shared memory while holding the relevant mutexes.  If the
associated walsender process then exits, this data is still safe to
make release decisions with, since we know that that much WAL *was*
sent to a valid standby server.  This incidentally means that we no
longer need to treat sync_standby_priority as protected by the
SyncRepLock rather than the per-walsender mutex.

SyncRepGetSyncStandbys is no longer used by the core code, so remove
it entirely in HEAD.  However, it seems possible that external code is
relying on that function, so do not remove it from the back branches.
Instead, just remove the known-incorrect Assert.  When the bug occurs,
the function will return a too-short list, which callers should treat
as meaning there are not enough sync standbys, which seems like a
reasonably safe fallback until the inconsistent state is resolved.
Moreover it's bug-compatible with what has been happening in non-assert
builds.  We cannot do anything about the walsender-replacement race
condition without an API/ABI break.

The bogus assertion exists back to 9.6, but 9.6 is sufficiently
different from the later branches that the patch doesn't apply at all.
I chose to just remove the bogus assertion in 9.6, feeling that the
probability of a bad outcome from the walsender-replacement race
condition is too low to justify rewriting the whole patch for 9.6.

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

5 years agoUse a slightly more liberal regex to detect Visual Studio version
Andrew Dunstan [Fri, 17 Apr 2020 18:52:42 +0000 (14:52 -0400)]
Use a slightly more liberal regex to detect Visual Studio version

Apparently in some language versions of Visual Studio nmake outputs some
material after the version number and before the end of the line. This
has been seen in Chinese versions. Therefore, we no longer demand that
the version string comes at the end of a line.

Per complaint from Cuiping Lin.

Backpatch to all live branches.

5 years agoFix minor memory leak in pg_basebackup and pg_receivewal
Michael Paquier [Fri, 17 Apr 2020 01:45:20 +0000 (10:45 +0900)]
Fix minor memory leak in pg_basebackup and pg_receivewal

The result of the query used to retrieve the WAL segment size from the
backend was not getting freed in two code paths.  Both pg_basebackup and
pg_receivewal exit immediately if a failure happened on this query, so
this was not an actual problem, but it could be an issue if this code
gets used for other tools in different ways, be they future tools in
this code tree or external, existing, ones.

Oversight in commit fc49e24, so backpatch down to 11.

Author: Jie Zhang
Discussion: https://postgr.es/m/970ad9508461469b9450b64027842331@G08CNEXMBPEKD06.g08.fujitsu.local
Backpatch-through: 11

5 years agoFix cache reference leak in contrib/sepgsql.
Tom Lane [Thu, 16 Apr 2020 18:45:54 +0000 (14:45 -0400)]
Fix cache reference leak in contrib/sepgsql.

fixup_whole_row_references() did the wrong thing with a dropped column,
resulting in a commit-time warning about a cache reference leak.

I (tgl) added a test case exercising this, but back-patched the test
only as far as v10; the patch didn't apply cleanly to 9.6 and it
didn't seem worth the trouble to adapt it.  The bug is pretty old
though, so apply the code change all the way back.

Michael Luo, with cosmetic improvements by me

Discussion: https://postgr.es/m/BYAPR08MB5606D1453D7F50E2AF4D2FD29AD80@BYAPR08MB5606.namprd08.prod.outlook.com

5 years agoFix minor memory leak in pg_dump
Michael Paquier [Wed, 15 Apr 2020 06:56:48 +0000 (15:56 +0900)]
Fix minor memory leak in pg_dump

A query used to read default ACL information from the catalogs did not
free a set of PQExpBuffer.

Oversight in commit e2090d9, so backpatch down to 9.6.

Author: Jie Zhang
Reviewed-by: Sawada Masahiko
Discussion: https://postgr.es/m/05bcbc5857f948efa0b451b85a48ae10@G08CNEXMBPEKD06.g08.fujitsu.local
Backpatch-through: 9.6

5 years agoAdd a wait_for_catchup() before immediate stop of a test master.
Noah Misch [Tue, 14 Apr 2020 01:47:28 +0000 (18:47 -0700)]
Add a wait_for_catchup() before immediate stop of a test 

Per buildfarm member hoverfly, a slow walsender could make the test
fail.  Back-patch to v10, where the test was introduced.

Discussion: https://postgr.es/m/20200414013849.GA886648@rfd.leadboat.com

5 years agoClear dangling pointer to avoid bogus EXPLAIN printout in a corner case.
Tom Lane [Sat, 11 Apr 2020 16:29:06 +0000 (12:29 -0400)]
Clear dangling pointer to avoid bogus EXPLAIN printout in a corner case.

ExecReScanHashJoin will destroy the join's hash table if it expects
that the inner relation will produce different rows on rescan.
Up to now it's not bothered to clear the additional pointer to that
hash table that exists in the child HashState node.  However, it's
possible for the query to terminate without building a fresh hash
table (this happens if the outer relation is found to be empty
during the final rescan).  So we can end with a dangling pointer
to a deleted hash table.  That was harmless originally, but since
9.0 EXPLAIN ANALYZE has used that pointer to print hash table
statistics.  In debug builds this reproducibly results in garbage
statistics.  In non-debug builds there's frequently no ill effects,
but in principle one could get wrong EXPLAIN ANALYZE output, or
perhaps even a crash if free() has released the hashtable memory
back to the OS.

To fix, just make sure we clear the additional pointer when destroying
the hash table.  In problematic cases, EXPLAIN ANALYZE will then print
no hashtable statistics (reverting to its pre-9.0 behavior).  This isn't
ideal, but since the problem manifests only in unusual corner cases,
it's hard to justify taking any risks to do better in the back
branches.  A follow-on patch will improve matters in HEAD.

Konstantin Knizhnik and Tom Lane, per diagnosis by Thomas Munro
of a trouble report from Alvaro Herrera.

Discussion: https://postgr.es/m/20200323165059.GA24950@alvherre.pgsql

5 years agoDoc: clarify locking requirements for ALTER TABLE ADD FOREIGN KEY.
Tom Lane [Fri, 10 Apr 2020 17:12:58 +0000 (13:12 -0400)]
Doc: clarify locking requirements for ALTER TABLE ADD FOREIGN KEY.

The docs explained that a SHARE ROW EXCLUSIVE lock is needed on the
referenced table, but failed to say the same about the table being
altered.  Since the page says that ACCESS EXCLUSIVE lock is taken
unless otherwise stated, this left readers with the wrong conclusion.

Discussion: https://postgr.es/m/834603375.3470346.1586482852542@mail.yahoo.com

5 years agoDoc: sync CREATE GROUP syntax synopsis with CREATE ROLE.
Tom Lane [Fri, 10 Apr 2020 14:44:10 +0000 (10:44 -0400)]
Doc: sync CREATE GROUP syntax synopsis with CREATE ROLE.

CREATE GROUP is an exact alias for CREATE ROLE, and CREATE USER is
almost an exact alias, as can easily be confirmed by checking the
code.  So the man page syntax descriptions ought to match up.  The
last few additions of role options seem to have forgotten to update
create_group.sgml, though.  Fix that, and add a naggy reminder to
create_role.sgml in hopes of not forgetting again.

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

5 years agoFurther cleanup of ts_headline code.
Tom Lane [Thu, 9 Apr 2020 19:38:43 +0000 (15:38 -0400)]
Further cleanup of ts_headline code.

Suppress a probably-meaningless uninitialized-variable warning
(induced by my previous patch, I'm sorry to say).

Improve mark_hl_fragments()'s test for overlapping cover strings:
it failed to consider the possibility that the current string is
strictly within another one.  That's unlikely given the preceding
splitting into MaxWords fragments, but I don't think it's impossible.

Discussion: https://postgr.es/m/16345-2e0cf5cddbdcd3b4@postgresql.org