postgresql.git
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.

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

5 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

5 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

5 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

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

5 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

5 years agoDoc: improve documentation about ts_headline() function.
Tom Lane [Thu, 9 Apr 2020 19:11:08 +0000 (15:11 -0400)]
Doc: improve documentation about ts_headline() function.

Now that I've had my nose in that code, I thought the docs about
it left something to be desired.

5 years agoFix default text search parser's ts_headline code for phrase queries.
Tom Lane [Thu, 9 Apr 2020 17:19:23 +0000 (13:19 -0400)]
Fix default text search parser's ts_headline code for phrase queries.

This code could produce very poor results when asked to highlight a
string based on a query using phrase-match operators.  The root cause
is that hlCover(), which is supposed to find a minimal substring that
matches the query, was written assuming that word position is not
significant.  I'm only 95% convinced that its algorithm was correct even
for plain AND/OR queries; but it definitely fails completely for phrase
matches, causing it to possibly not identify a cover string at all.

Hence, rewrite hlCover() with a less-tense algorithm that just tries
all the possible substrings, earlier and shorter ones first.  (This is
not as bad as it sounds performance-wise, because all of the string
matching has been done already: the repeated tsquery match checks
boil down to pointer comparisons.)

Unfortunately, since that approach produces more candidate cover
strings than before, it also exposes that there were bugs in the
heuristics in mark_hl_words() for selecting a best cover string.
Fixes there include:
* Do not apply the ShortWord filter to words that appear in the query.
* Remove a misguided optimization for quickly rejecting a cover.
* Fix order-of-operation bug that could cause computation of a
wrong figure of merit (poslen) when shortening a cover.
* Change the preference rule so that candidate headlines that do not
include their whole cover string (after MaxWords trimming) are lowest
priority, since they may not actually satisfy the user's query.

This results in some changes in existing regression test cases,
but they all seem reasonable.  Note in particular that the tests
involving strings like "1 2 3" were previously being affected by
the ShortWord filter, masking the normal matching behavior.

Per bug #16345 from Augustinas Jokubauskas; the new test cases are
based on that example.  Back-patch to 9.6 where phrase search was
added to tsquery.

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

5 years agoCosmetic improvements for default text search parser's ts_headline code.
Tom Lane [Thu, 9 Apr 2020 16:37:00 +0000 (12:37 -0400)]
Cosmetic improvements for default text search parser's ts_headline code.

This code was woefully unreadable and under-commented.  Try to improve
matters by adding comments, using some macros to make complicated
if-tests more readable, using boolean type where appropriate, etc.
There are a couple of tiny coding improvements too, but this commit
includes (I hope) no behavioral change.

Nonetheless, back-patch as far as 9.6, because a followup bug-fixing
commit depends on this.

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

5 years agoAllow parallel create index to accumulate buffer usage stats.
Amit Kapila [Thu, 9 Apr 2020 03:55:57 +0000 (09:25 +0530)]
Allow parallel create index to accumulate buffer usage stats.

Currently, we don't account for buffer usage incurred by parallel workers
for parallel create index.  This commit allows each worker to record the
buffer usage stats and leader backend to accumulate that stats at the
end of the operation.  This will allow pg_stat_statements to display
correct buffer usage stats for (parallel) create index command.

Reported-by: Julien Rouhaud
Author: Sawada Masahiko
Reviewed-by: Dilip Kumar, Julien Rouhaud and Amit Kapila
Backpatch-through: 11, where this was introduced
Discussion: https://postgr.es/m/20200328151721.GB12854@nol

5 years agoFix pg_dump/pg_restore to restore event trigger comments later.
Tom Lane [Wed, 8 Apr 2020 15:23:39 +0000 (11:23 -0400)]
Fix pg_dump/pg_restore to restore event trigger comments later.

Repair an oversight in commit 8728b2c70: if we're postponing restore
of event triggers to the end, we must also postpone restoring any
comments on them, since of course we cannot create the comments first.
(This opens yet another opportunity for an event trigger to bollix
the restore, but there's no help for that.)

Per bug #16346 from Alexander Lakhin.

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

Hamid Akhtar and Tom Lane

Discussion: https://postgr.es/m/16346-6210ad7a0ea81be1@postgresql.org

5 years agoFix circle_in to accept "(x,y),r" as it's advertised to do.
Tom Lane [Wed, 8 Apr 2020 00:50:02 +0000 (20:50 -0400)]
Fix circle_in to accept "(x,y),r" as it's advertised to do.

Our documentation describes four allowed input syntaxes for circles,
but the regression tests tried only three ... with predictable
consequences.  Remarkably, this has been wrong since the circle
datatype was added in 1997, but nobody noticed till now.

David Zhang, with some help from me

Discussion: https://postgr.es/m/332c47fa-d951-7574-b5cc-a8f7f7201202@highgo.ca

5 years agoAdjust bytea get_bit/set_bit to cope with bytea strings > 256MB.
Tom Lane [Tue, 7 Apr 2020 20:30:55 +0000 (16:30 -0400)]
Adjust bytea get_bit/set_bit to cope with bytea strings > 256MB.

Since the existing bit number argument can't exceed INT32_MAX, it's
not possible for these functions to manipulate bits beyond the first
256MB of a bytea value.  However, it'd be good if they could do at
least that much, and not fall over entirely for longer bytea values.
Adjust the comparisons to be done in int64 arithmetic so that works.
Also tweak the error reports to show sane values in case of overflow.

Also add some test cases to improve the miserable code coverage
of these functions.

Apply patch to back branches only; HEAD has a better solution
as of commit 26a944cf2.

Extracted from a much larger patch by Movead Li

Discussion: https://postgr.es/m/20200312115135445367128@highgo.ca

5 years agoPreserve clustered index after rewrites with ALTER TABLE
Michael Paquier [Mon, 6 Apr 2020 02:05:54 +0000 (11:05 +0900)]
Preserve clustered index after rewrites with ALTER TABLE

A table rewritten by ALTER TABLE would lose tracking of an index usable
for CLUSTER.  This setting is tracked by pg_index.indisclustered and is
controlled by ALTER TABLE, so some extra work was needed to restore it
properly.  Note that ALTER TABLE only marks the index that can be used
for clustering, and does not do the actual operation.

Author: Amit Langote, Justin Pryzby
Reviewed-by: Ibrar Ahmed, Michael Paquier
Discussion: https://postgr.es/m/20200202161718.GI13621@telsasoft.com
Backpatch-through: 9.5

5 years agoUse TransactionXmin instead of RecentGlobalXmin in heap_abort_speculative().
Andres Freund [Mon, 6 Apr 2020 00:47:30 +0000 (17:47 -0700)]
Use TransactionXmin instead of RecentGlobalXmin in heap_abort_speculative().

There's a very low risk that RecentGlobalXmin could be far enough in
the past to be older than relfrozenxid, or even wrapped
around. Luckily the consequences of that having happened wouldn't be
too bad - the page wouldn't be pruned for a while.

Avoid that risk by using TransactionXmin instead. As that's announced
via MyPgXact->xmin, it is protected against wrapping around (see code
comments for details around relfrozenxid).

Author: Andres Freund
Discussion: https://postgr.es/m/20200328213023.s4eyijhdosuc4vcj@alap3.anarazel.de
Backpatch: 9.5-

5 years agoSave errno across LWLockRelease() calls
Peter Eisentraut [Sun, 5 Apr 2020 08:02:00 +0000 (10:02 +0200)]
Save errno across LWLockRelease() calls

Fixup for "Drop slot's LWLock before returning from SaveSlotToPath()"

Reported-by: Michael Paquier <michael@paquier.xyz>
5 years agoFix bugs in gin_fuzzy_search_limit processing.
Tom Lane [Fri, 3 Apr 2020 17:15:30 +0000 (13:15 -0400)]
Fix bugs in gin_fuzzy_search_limit processing.

entryGetItem()'s three code paths each contained bugs associated
with filtering the entries for gin_fuzzy_search_limit.

The posting-tree path failed to advance "advancePast" after having
decided to filter an item.  If we ran out of items on the current
page and needed to advance to the next, what would actually happen
is that entryLoadMoreItems() would re-load the same page.  Eventually,
the random dropItem() test would accept one of the same items it'd
previously rejected, and we'd move on --- but it could take awhile
with small gin_fuzzy_search_limit.  To add insult to injury, this
case would inevitably cause entryLoadMoreItems() to decide it needed
to re-descend from the root, making things even slower.

The posting-list path failed to implement gin_fuzzy_search_limit
filtering at all, so that all entries in the posting list would
be returned.

The bitmap-result path used a "gotitem" variable that it failed to
update in the one place where it'd actually make a difference, ie
at the one "continue" statement.  I think this was unreachable in
practice, because if we'd looped around then it shouldn't be the
case that the entries on the new page are before advancePast.
Still, the "gotitem" variable was contributing nothing to either
clarity or correctness, so get rid of it.

Refactor all three loops so that the termination conditions are
more alike and less unreadable.

The code coverage report showed that we had no coverage at all for
the re-descend-from-root code path in entryLoadMoreItems(), which
seems like a very bad thing, so add a test case that exercises it.
We also had exactly no coverage for gin_fuzzy_search_limit, so add a
simplistic test case that at least hits those code paths a little bit.

Back-patch to all supported branches.

Adé Heyward and Tom Lane

Discussion: https://postgr.es/m/CAEknJCdS-dE1Heddptm7ay2xTbSeADbkaQ8bU2AXRCVC2LdtKQ@mail.gmail.com

5 years agoFix bogus CALLED_AS_TRIGGER() defenses.
Tom Lane [Fri, 3 Apr 2020 15:24:56 +0000 (11:24 -0400)]
Fix bogus CALLED_AS_TRIGGER() defenses.

contrib/lo's lo_manage() thought it could use
trigdata->tg_trigger->tgname in its error message about
not being called as a trigger.  That naturally led to a core dump.

unique_key_recheck() figured it could Assert that fcinfo->context
is a TriggerData node in advance of having checked that it's
being called as a trigger.  That's harmless in production builds,
and perhaps not that easy to reach in any case, but it's logically
wrong.

The first of these per bug #16340 from William Crowell;
the second from manual inspection of other CALLED_AS_TRIGGER
call sites.

Back-patch the lo.c change to all supported branches, the
other to v10 where the thinko crept in.

Discussion: https://postgr.es/m/16340-591c7449dc7c8c47@postgresql.org

5 years agodoc: remove unnecessary INNER keyword
Bruce Momjian [Thu, 2 Apr 2020 21:42:09 +0000 (17:42 -0400)]
doc: remove unnecessary INNER keyword

A join that was added in commit 9b2009c4cf that did not use the INNER
keyword but the existing query used it.  It was cleaner to remove the
existing INNER keyword.

Reported-by: Peter Eisentraut
Discussion: https://postgr.es/m/a1ffbfda-59d2-5732-e5fb-3df8582b6434@2ndquadrant.com

Backpatch-through: 9.5

5 years agodoc: remove comma, related to commit 92d31085e9
Bruce Momjian [Thu, 2 Apr 2020 21:27:43 +0000 (17:27 -0400)]
doc:  remove comma, related to commit 92d31085e9

Reported-by: Peter Eisentraut
Discussion: https://postgr.es/m/750b8832-d123-7f9b-931e-43ce8321b2d7@2ndquadrant.com

Backpatch-through: 9.5

5 years agoCheck equality semantics for unique indexes on partitioned tables.
Tom Lane [Wed, 1 Apr 2020 18:49:49 +0000 (14:49 -0400)]
Check equality semantics for unique indexes on partitioned tables.

We require the partition key to be a subset of the set of columns
being made unique, so that physically-separate indexes on the different
partitions are sufficient to enforce the uniqueness constraint.

The existing code checked that the listed columns appear, but did not
inquire into the index semantics, which is a serious oversight given
that different index opclasses might enforce completely different
notions of uniqueness.

Ideally, perhaps, we'd just match the partition key opfamily to the
index opfamily.  But hash partitioning uses hash opfamilies which we
can't directly match to btree opfamilies.  Hence, look up the equality
operator in each family, and accept if it's the same operator.  This
should be okay in a fairly general sense, since the equality operator
ought to precisely represent the opfamily's notion of uniqueness.

A remaining weak spot is that we don't have a cross-index-AM notion of
which opfamily member is "equality".  But we know which one to use for
hash and btree AMs, and those are the only two that are relevant here
at present.  (Any non-core AMs that know how to enforce equality are
out of luck, for now.)

Back-patch to v11 where this feature was introduced.

Guancheng Luo, revised a bit by me

Discussion: https://postgr.es/m/D9C3CEF7-04E8-47A1-8300-CA1DCD5ED40D@gmail.com

5 years agopsql: do file completion for \gx
Bruce Momjian [Wed, 1 Apr 2020 03:01:33 +0000 (23:01 -0400)]
psql:  do file completion for \gx

This was missed when the feature was added.

Reported-by: Vik Fearing
Discussion: https://postgr.es/m/eca20529-0b06-b493-ee38-f071a75dcd5b@postgresfriends.org

Backpatch-through: 10

5 years agodoc: remove mention of bitwise operators as solely type-limited
Bruce Momjian [Tue, 31 Mar 2020 22:44:29 +0000 (18:44 -0400)]
doc:  remove mention of bitwise operators as solely type-limited

There are other operators that have limited number data type support, so
just remove the sentence.

Reported-by: Sergei Agalakov
Discussion: https://postgr.es/m/158032651854.19851.16261832706661813796@wrigleys.postgresql.org

Backpatch-through: 9.5

5 years agodoc: clarify hierarchy of objects: global, db, schema, etc.
Bruce Momjian [Tue, 31 Mar 2020 22:10:39 +0000 (18:10 -0400)]
doc:  clarify hierarchy of objects:  global, db, schema, etc.

The previous wording was confusing because it wasn't in decreasing order
and had to backtrack.  Also clarify role/user wording.

Reported-by: jbird@nuna.com
Discussion: https://postgr.es/m/158057750885.1123.2806779262588618988@wrigleys.postgresql.org

Backpatch-through: 9.5

5 years agodoc: restore wording from recent patch "rolled back to"
Bruce Momjian [Tue, 31 Mar 2020 21:52:48 +0000 (17:52 -0400)]
doc: restore wording from recent patch "rolled back to"

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

Backpatch-through: 9.5 - 12

5 years agodoc: clarify when row-level locks are released
Bruce Momjian [Tue, 31 Mar 2020 21:27:32 +0000 (17:27 -0400)]
doc:  clarify when row-level locks are released

They are released just like table-level locks.  Also clean up wording.

Reported-by: me@sillymon.ch
Discussion: https://postgr.es/m/158074944048.1095.4309647363871637715@wrigleys.postgresql.org

Backpatch-through: 9.5

5 years agodoc: add namespace column to pg_buffercache example query
Bruce Momjian [Tue, 31 Mar 2020 21:16:33 +0000 (17:16 -0400)]
doc:  add namespace column to pg_buffercache example query

Without the namespace, the table name could be ambiguous.

Reported-by: adunham@arbormetrix.com
Discussion: https://postgr.es/m/158155175140.23798.2189464781144503491@wrigleys.postgresql.org

Backpatch-through: 9.5

5 years agodoc: clarify which table creation is used for inheritance part.
Bruce Momjian [Tue, 31 Mar 2020 21:07:43 +0000 (17:07 -0400)]
doc:  clarify which table creation is used for inheritance part.

Previously people might assume that the partition syntax version of
CREATE TABLE is to be used for the inheritance partition table example;
mention that the non-partitioned version should be used.

Reported-by: mib@nic.at
Discussion: https://postgr.es/m/158089540905.1098.15071165437284409576@wrigleys.postgresql.org

Backpatch-through: 10

5 years agodoc: adjust UPDATE/DELETE's FROM/USING to match SELECT's FROM
Bruce Momjian [Tue, 31 Mar 2020 20:31:44 +0000 (16:31 -0400)]
doc: adjust UPDATE/DELETE's FROM/USING to match SELECT's FROM

Previously the syntax and wording were unclear.

Reported-by: Alexey Bashtanov
Discussion: https://postgr.es/m/968d4724-8e58-788f-7c45-f7b1813824cc@imap.cc

Backpatch-through: 9.5

5 years agoTeach pg_ls_dir_files() to ignore ENOENT failures from stat().
Tom Lane [Tue, 31 Mar 2020 16:57:55 +0000 (12:57 -0400)]
Teach pg_ls_dir_files() to ignore ENOENT failures from stat().

Buildfarm experience shows that this function can fail with ENOENT
if some other process unlinks a file between when we read the directory
entry and when we try to stat() it.  The problem is old but we had
not noticed it until 085b6b667 added regression test coverage.

To fix, just ignore ENOENT failures.  There is one other case that
this might hide: a symlink that points to nowhere.  That seems okay
though, at least better than erroring.

Back-patch to v10 where this function was added, since the regression
test cases were too.

Discussion: https://postgr.es/m/20200308173103.GC1357@telsasoft.com

5 years agoBack-patch addition of stack overflow and interrupt checks for lquery.
Tom Lane [Tue, 31 Mar 2020 15:37:44 +0000 (11:37 -0400)]
Back-patch addition of stack overflow and interrupt checks for lquery.

Experimentation shows that it's not hard at all to drive the
old implementation of "ltree ~ lquery" match to stack overflow,
so throw in a check_stack_depth() call, as I just did in HEAD.

I wasn't able to make it take a long time, because all the
pathological cases I tried hit stack overflow first; but
I bet there are some others that do take a long time, so add
CHECK_FOR_INTERRUPTS() too.

Discussion: https://postgr.es/m/CAP_rww=waX2Oo6q+MbMSiZ9ktdj6eaJj0cQzNu=Ry2cCDij5fw@mail.gmail.com

5 years agoBe more careful about extracting encoding from locale strings on Windows.
Tom Lane [Mon, 30 Mar 2020 15:14:58 +0000 (11:14 -0400)]
Be more careful about extracting encoding from locale strings on Windows.

GetLocaleInfoEx() can fail on strings that setlocale() was perfectly
happy with.  A common way for that to happen is if the locale string
is actually a Unix-style string, say "et_EE.UTF-8".  In that case,
what's after the dot is an encoding name, not a Windows codepage number;
blindly treating it as a codepage number led to failure, with a fairly
silly error message.  Hence, check to see if what's after the dot is
all digits, and if not, treat it as a literal encoding name rather than
a codepage number.  This will do the right thing with many Unix-style
locale strings, and produce a more sensible error message otherwise.

Somewhat independently of that, treat a zero (CP_ACP) result from
GetLocaleInfoEx() as meaning that we must use UTF-8 encoding.

Back-patch to all supported branches.

Juan José Santamaría Flecha

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

5 years agoDoc: correct misstatement about ltree label maximum length.
Tom Lane [Sun, 29 Mar 2020 22:54:19 +0000 (18:54 -0400)]
Doc: correct misstatement about ltree label maximum length.

The documentation says that the max length is 255 bytes, but
code inspection says it's actually 255 characters; and relevant
lengths are stored as uint16 so that that works.

5 years agoProtect against overflow of ltree.numlevel and lquery.numlevel.
Tom Lane [Sat, 28 Mar 2020 21:09:51 +0000 (17:09 -0400)]
Protect against overflow of ltree.numlevel and lquery.numlevel.

These uint16 fields could be overflowed by excessively long input,
producing strange results.  Complain for invalid input.

Likewise check for out-of-range values of the repeat counts in lquery.
(We don't try too hard on that one, notably not bothering to detect
if atoi's result has overflowed.)

Also detect length overflow in ltree_concat.

In passing, be more consistent about whether "syntax error" messages
include the type name.  Also, clarify the documentation about what
the size limit is.

This has been broken for a long time, so back-patch to all supported
branches.

Nikita Glukhov, reviewed by Benjie Gillam and Tomas Vondra

Discussion: https://postgr.es/m/CAP_rww=waX2Oo6q+MbMSiZ9ktdj6eaJj0cQzNu=Ry2cCDij5fw@mail.gmail.com