]> granicus.if.org Git - postgresql/log
postgresql
5 years agoFix bogus sizeof calculations.
Tom Lane [Sun, 15 Sep 2019 15:51:57 +0000 (11:51 -0400)]
Fix bogus sizeof calculations.

Noted by Coverity.  Typo in 27cc7cd2b, so back-patch to v12
as that was.

5 years agoFix intermittent self-test failures caused by the stats_ext test.
Dean Rasheed [Sun, 15 Sep 2019 12:16:59 +0000 (13:16 +0100)]
Fix intermittent self-test failures caused by the stats_ext test.

Commit d7f8d26d9 added new tests to the stats_ext regression test that
included creating a view in the public schema, without realising that
the stats_ext test runs in the same parallel group as the rules test,
which makes doing that unsafe.

This led to intermittent failures of the rules test on the buildfarm,
although I wasn't able to reproduce that locally. Fix by creating the
view in a different schema.

Tomas Vondra and Dean Rasheed, report and diagnosis by Thomas Munro.

Discussion: https://postgr.es/m/CA+hUKGKX9hFZrYA7rQzAMRE07L4hziCc-nO_b3taJpiuKyLLxg@mail.gmail.com

5 years agoReplace xlc __fetch_and_add() with inline asm.
Noah Misch [Sat, 14 Sep 2019 02:34:06 +0000 (19:34 -0700)]
Replace xlc __fetch_and_add() with inline asm.

PostgreSQL has been unusable when built with xlc 13 and newer, which are
incompatible with our use of __fetch_and_add().  Back-patch to 9.5,
which introduced pg_atomic_fetch_add_u32().

Reviewed by Tom Lane.

Discussion: https://postgr.es/m/20190831071157.GA3251746@rfd.leadboat.com

5 years agoTest pg_atomic_fetch_add_ with variable addend and 16-bit edge cases.
Noah Misch [Sat, 14 Sep 2019 02:33:30 +0000 (19:33 -0700)]
Test pg_atomic_fetch_add_ with variable addend and 16-bit edge cases.

Back-patch to 9.5, which introduced these functions.

Reviewed by Tom Lane.

Discussion: https://postgr.es/m/20190831071157.GA3251746@rfd.leadboat.com

5 years agological decoding: process ASSIGNMENT during snapshot build
Alvaro Herrera [Fri, 13 Sep 2019 19:36:28 +0000 (16:36 -0300)]
logical decoding: process ASSIGNMENT during snapshot build

Most WAL records are ignored in early SnapBuild snapshot build phases.
But it's critical to process some of them, so that later messages have
the correct transaction state after the snapshot is completely built; in
particular, XLOG_XACT_ASSIGNMENT messages are critical in order for
sub-transactions to be correctly assigned to their parent transactions,
or at least one assert misbehaves, as reported by Ildar Musin.

Diagnosed-by: Masahiko Sawada
Author: Masahiko Sawada
Discussion: https://postgr.es/m/CAONYFtOv+Er1p3WAuwUsy1zsCFrSYvpHLhapC_fMD-zNaRWxYg@mail.gmail.com

5 years agoFix under-parenthesized macro definitions
Alvaro Herrera [Fri, 13 Sep 2019 19:26:55 +0000 (16:26 -0300)]
Fix under-parenthesized macro definitions

Lack of parens in the definitions could cause a statement using these
macros to have unexpected semantics.  In current code no bug is
apparent, but best to fix the definitions to avoid problems down the
line.

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

5 years agoFix progress reporting of CLUSTER / VACUUM FULL
Alvaro Herrera [Fri, 13 Sep 2019 17:51:13 +0000 (14:51 -0300)]
Fix progress reporting of CLUSTER / VACUUM FULL

The progress state was being clobbered once the first index completed
being rebuilt, causing the final phases of the operation not show
anything in the progress view.  This was inadvertently broken in
03f9e5cba0ee, which added progress tracking for REINDEX.

(The reason this bugfix is this small is that I had already noticed this
problem when writing monitoring for CREATE INDEX, and had already worked
around it, as can be seen in discussion starting at
https://postgr.es/m/20190329150218.GA25010@alvherre.pgsql Fixing the
problem is just a matter of fixing one place touched by the REINDEX
monitoring.)

Reported by: Álvaro Herrera
Author: Álvaro Herrera
Discussion: https://postgr.es/m/20190801184333.GA21369@alvherre.pgsql

5 years agoTypo fixes for documentation
Alexander Korotkov [Fri, 13 Sep 2019 14:21:20 +0000 (17:21 +0300)]
Typo fixes for documentation

Discussion: https://postgr.es/m/CAEkD-mDUZrRE%3Dk-FznEg4Ed2VdjpZCyHoyo%2Bp0%2B8KvHqR%3DpNVQ%40mail.gmail.com
Author: Liudmila Mantrova, Alexander Lakhin
Reviewed-by: Alexander Korotkov, Alvaro Herrera
Backpatch-through: 12

5 years agoDocumentation improvements to jsonpath
Alexander Korotkov [Fri, 13 Sep 2019 14:12:20 +0000 (17:12 +0300)]
Documentation improvements to jsonpath

Besides cosmetic improvements it removes statement that operators necessary
need to be separated from operands with spaces, which is not really true.

Discussion: https://postgr.es/m/CAEkD-mDUZrRE%3Dk-FznEg4Ed2VdjpZCyHoyo%2Bp0%2B8KvHqR%3DpNVQ%40mail.gmail.com
Author: Liudmila Mantrova, Alexander Lakhin
Reviewed-by: Alexander Korotkov, Alvaro Herrera
Backpatch-through: 12

5 years agoFix nbtree page split rmgr desc routine.
Peter Geoghegan [Thu, 12 Sep 2019 22:45:07 +0000 (15:45 -0700)]
Fix nbtree page split rmgr desc routine.

Include newitemoff in rmgr desc output for nbtree page split records.
In passing, correct an obsolete comment that claimed that newitemoff is
only logged for _L variant nbtree page split WAL records.

Both issues were oversights in commit 2c03216d831, which revamped the
WAL format.

Author: Peter Geoghegan
Backpatch: 9.5-, where the WAL format was revamped.

5 years agoFix usage of whole-row variables in WCO and RLS policy expressions.
Tom Lane [Thu, 12 Sep 2019 22:29:17 +0000 (18:29 -0400)]
Fix usage of whole-row variables in WCO and RLS policy expressions.

Since WITH CHECK OPTION was introduced, ExecInitModifyTable has
initialized WCO expressions with the wrong plan node as parent -- that is,
it passed its input subplan not the ModifyTable node itself.  Up to now
we thought this was harmless, but bug #16006 from Vinay Banakar shows it's
not: if the input node is a SubqueryScan then ExecInitWholeRowVar can get
confused into doing the wrong thing.  (The fact that ExecInitWholeRowVar
contains such logic is certainly a horrid kluge that doesn't deserve to
live, but figuring out another way to do that is a task for some other day.)

Andres had already noticed the wrong-parent mistake and fixed it in commit
148e632c0, but not being aware of any user-visible consequences, he quite
reasonably didn't back-patch.  This patch is simply a back-patch of
148e632c0, plus addition of a test case based on bug #16006.  I also added
the test case to v12/HEAD, even though the bug is already fixed there.

Back-patch to all supported branches.  9.4 lacks RLS policies so the
new test case doesn't work there, but I'm pretty sure a test could be
devised based on using a whole-row Var in a plain WITH CHECK OPTION
condition.  (I lack the cycles to do so myself, though.)

Andres Freund and Tom Lane

Discussion: https://postgr.es/m/16006-99290d2e4642cbd5@postgresql.org
Discussion: https://postgr.es/m/20181205225213.hiwa3kgoxeybqcqv@alap3.anarazel.de

5 years agoDoc: Update PL/pgSQL sample function in plpgsql.sgml.
Amit Kapila [Wed, 11 Sep 2019 04:55:49 +0000 (10:25 +0530)]
Doc: Update PL/pgSQL sample function in plpgsql.sgml.

The example used to explain 'Looping Through Query Results' uses
pseudo-materialized views.  Replace it with a more up-to-date example
which does the same thing with actual materialized views, which have
been available since PostgreSQL 9.3.

In the passing, change '%' as format specifier instead of '%s' as is used
in other examples in plpgsql.sgml.

Reported-by: Ian Barwick
Author: Ian Barwick
Reviewed-by: Amit Kapila
Backpatch-through: 9.4
Discussion: https://postgr.es/m/9a70d393-7904-4918-c97c-649f6d114b6a@2ndquadrant.com

5 years agoExpand properly list of TAP tests used for prove in vcregress.pl
Michael Paquier [Wed, 11 Sep 2019 02:07:25 +0000 (11:07 +0900)]
Expand properly list of TAP tests used for prove in vcregress.pl

Depending on the system used, t/*.pl may not be expanded into a list of
tests which can be consumed by prove when attempting to run TAP tests on
a given path.  Fix that by using glob() directly in the script, to make
sure that a complete list of tests is provided.  This has not proved to
be an issue with MSVC as the list was properly expanded, but it is on
Linux with perl's system().

This is extracted from a larger patch.

Author: Tom Lane
Discussion: https://postgr.es/m/6628.1567958876@sss.pgh.pa.us
Backpatch-through: 9.4

5 years agoDon't drop NOTICE messages in isolation tests.
Tom Lane [Tue, 10 Sep 2019 16:04:57 +0000 (12:04 -0400)]
Don't drop NOTICE messages in isolation tests.

For its entire existence, isolationtester.c has forced client_min_messages
to WARNING, but that seems like a very poor choice of test design.  It
should be up to individual test scripts to manage whether they emit notices
and to ensure that the results are stable.  (There were no NOTICE messages
in the original set of isolation tests, so this was certainly dead code
when committed, but perhaps it was needed at some earlier point.)

It's possible that the original motivation was due to platform-dependent
variations in the timing of stdout vs. stderr output.  That should be
moot since commits 73bcb76b7/6eda3e9c2, but just in case, adjust
isotesterNoticeProcessor to print to stdout not stderr.  (stderr seems
like the wrong thing anyway: it should be for error printouts not expected
test output.)

Back-patch of commit ebd499282 into v12.  I'll separately push this
into older branches, but this is as much change as v12 needs.

Discussion: https://postgr.es/m/14616.1564251339@sss.pgh.pa.us
Discussion: https://postgr.es/m/E1i7IqC-0000Uc-5H@gemulon.postgresql.org

5 years agoFix isolationtester race condition for notices sent before blocking.
Tom Lane [Tue, 10 Sep 2019 02:53:51 +0000 (22:53 -0400)]
Fix isolationtester race condition for notices sent before blocking.

If a test sends a notice just before blocking, it's possible on
slow machines for isolationtester to detect the blocked state before
it's consumed the notice.  (For this to happen, the notice would have
to arrive after isolationtester has waited for data for 10ms, so on
fast/lightly-loaded machines it's hard to reproduce the failure.)
But, if we have seen the backend as blocked, it's certainly already
sent any notices it's going to send.  Therefore, one more round of
PQconsumeInput and PQisBusy should be enough to collect and process
any such notices.

Back-patch of 30717637c into v12.  We're still discussing whether
to back-patch this further and/or back-patch some other recent
isolationtester fixes, but this much is provably necessary to
make the test cases added by 27cc7cd2b stable in v12.

Discussion: https://postgr.es/m/14616.1564251339@sss.pgh.pa.us
Discussion: https://postgr.es/m/E1i7IqC-0000Uc-5H@gemulon.postgresql.org

5 years agoStamp 12beta4. REL_12_BETA4
Tom Lane [Mon, 9 Sep 2019 20:24:29 +0000 (16:24 -0400)]
Stamp 12beta4.

5 years agoBe more careful about port selection in src/test/ldap/.
Tom Lane [Mon, 9 Sep 2019 18:21:40 +0000 (14:21 -0400)]
Be more careful about port selection in src/test/ldap/.

Don't just assume that the next port is free; it might not be, or
if we're really unlucky it might even be out of the TCP range.
Do it honestly with two get_free_port() calls instead.

This is surely a pretty low-probability problem, but I think it
explains a buildfarm failure seen today, so let's fix it.

Back-patch to v11 where this script was added.

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

5 years agoPrevent msys2 conversion of "cmd /c" switch to a file path
Andrew Dunstan [Mon, 9 Sep 2019 12:56:33 +0000 (08:56 -0400)]
Prevent msys2 conversion of "cmd /c" switch to a file path

Modern versions of msys2 have changed the treatment of "cmd /c" so that
the runtime will try to convert the switch to a native file path. This
patch adds a setting to inhibit that behaviour.

Discussion: https://postgr.es/m/3227042f-cfcc-745a-57dd-fb8c471f8ddf@2ndQuadrant.com

Backpatch to all live branches.

5 years agoAlways skip recovery SysV shared memory tests on Windows
Andrew Dunstan [Mon, 9 Sep 2019 12:42:06 +0000 (08:42 -0400)]
Always skip recovery SysV shared memory tests on Windows

These tests were disabled on git master in commit 8e5ce1c3f8. This does
the same thing on the back branches.

Discussion: https://postgr.es/m/a1c40fab-a38c-cb42-6879-125f834e837b@2ndQuadrant.com

5 years agoReorder EPQ work, to fix rowmark related bugs and improve efficiency.
Andres Freund [Mon, 9 Sep 2019 12:21:30 +0000 (05:21 -0700)]
Reorder EPQ work, to fix rowmark related bugs and improve efficiency.

In ad0bda5d24ea I changed the EvalPlanQual machinery to store
substitution tuples in slot, instead of using plain HeapTuples. The
main motivation for that was that using HeapTuples will be inefficient
for future tableams.  But it turns out that that conversion was buggy
for non-locking rowmarks - the wrong tuple descriptor was used to
create the slot.

As a secondary issue 5db6df0c0 changed ExecLockRows() to begin EPQ
earlier, to allow to fetch the locked rows directly into the EPQ
slots, instead of having to copy tuples around. Unfortunately, as Tom
complained, that forces some expensive initialization to happen
earlier.

As a third issue, the test coverage for EPQ was clearly insufficient.

Fixing the first issue is unfortunately not trivial: Non-locked row
marks were fetched at the start of EPQ, and we don't have the type
information for the rowmarks available at that point. While we could
change that, it's not easy. It might be worthwhile to change that at
some point, but to fix this bug, it seems better to delay fetching
non-locking rowmarks when they're actually needed, rather than
eagerly. They're referenced at most once, and in cases where EPQ
fails, might never be referenced. Fetching them when needed also
increases locality a bit.

To be able to fetch rowmarks during execution, rather than
initialization, we need to be able to access the active EPQState, as
that contains necessary data. To do so move EPQ related data from
EState to EPQState, and, only for EStates creates as part of EPQ,
reference the associated EPQState from EState.

To fix the second issue, change EPQ initialization to allow use of
EvalPlanQualSlot() to be used before EvalPlanQualBegin() (but
obviously still requiring EvalPlanQualInit() to have been done).

As these changes made struct EState harder to understand, e.g. by
adding multiple EStates, significantly reorder the members, and add a
lot more comments.

Also add a few more EPQ tests, including one that fails for the first
issue above. More is needed.

Reported-By: yi huang
Author: Andres Freund
Reviewed-By: Tom Lane
Discussion:
    https://postgr.es/m/CAHU7rYZo_C4ULsAx_LAj8az9zqgrD8WDd4hTegDTMM1LMqrBsg@mail.gmail.com
    https://postgr.es/m/24530.1562686693@sss.pgh.pa.us
Backpatch: 12-, where the EPQ changes were introduced

5 years agoFix handling of non-key columns get_index_column_opclass()
Alexander Korotkov [Mon, 9 Sep 2019 10:50:12 +0000 (13:50 +0300)]
Fix handling of non-key columns get_index_column_opclass()

f2e40380 introduces support of non-key attributes in GiST indexes.  Then if
get_index_column_opclass() is asked by gistproperty() to get an opclass of
non-key column, it returns garbage past oidvector value.  This commit fixes
that by making get_index_column_opclass() return InvalidOid in this case.

Discussion: https://postgr.es/m/20190902231948.GA5343%40alvherre.pgsql
Author: Nikita Glukhov, Alexander Korotkov
Backpatch-through: 12

5 years agoTranslation updates
Peter Eisentraut [Mon, 9 Sep 2019 09:34:06 +0000 (11:34 +0200)]
Translation updates

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

5 years agoImprove new AND CHAIN tests
Peter Eisentraut [Mon, 9 Sep 2019 08:30:22 +0000 (10:30 +0200)]
Improve new AND CHAIN tests

Tweak the tests so that we're not just testing the default setting of
transaction_read_only.

Reported-by: fn ln <emuser20140816@gmail.com>
5 years agoFix RelationIdGetRelation calls that weren't bothering with error checks.
Tom Lane [Sun, 8 Sep 2019 21:00:29 +0000 (17:00 -0400)]
Fix RelationIdGetRelation calls that weren't bothering with error checks.

Some of these are quite old, but that doesn't make them not bugs.
We'd rather report a failure via elog than SIGSEGV.

While at it, uniformly spell the error check as !RelationIsValid(rel)
rather than a bare rel == NULL test.  The machine code is the same
but it seems better to be consistent.

Coverity complained about this today, not sure why, because the
mistake is in fact old.

5 years agoFix handling of NULL distances in KNN-GiST
Alexander Korotkov [Sun, 8 Sep 2019 18:13:40 +0000 (21:13 +0300)]
Fix handling of NULL distances in KNN-GiST

In order to implement NULL LAST semantic GiST previously assumed distance to
the NULL value to be Inf.  However, our distance functions can return Inf and
NaN for non-null values.  In such cases, NULL LAST semantic appears to be
broken.  This commit fixes that by introducing separate array of null flags for
distances.

Backpatch to all supported versions.

Discussion: https://postgr.es/m/CAPpHfdsNvNdA0DBS%2BwMpFrgwT6C3-q50sFVGLSiuWnV3FqOJuQ%40mail.gmail.com
Author: Alexander Korotkov
Backpatch-through: 9.4

5 years agoFix handling Inf and Nan values in GiST pairing heap comparator
Alexander Korotkov [Sun, 8 Sep 2019 18:07:30 +0000 (21:07 +0300)]
Fix handling Inf and Nan values in GiST pairing heap comparator

Previously plain float comparison was used in GiST pairing heap.  Such
comparison doesn't provide proper ordering for value sets containing Inf and Nan
values.  This commit fixes that by usage of float8_cmp_internal().  Note, there
is remaining problem with NULL distances, which are represented as Inf in
pairing heap.  It would be fixes in subsequent commit.

Backpatch to all supported versions.

Reported-by: Andrey Borodin
Discussion: https://postgr.es/m/CAPpHfdsNvNdA0DBS%2BwMpFrgwT6C3-q50sFVGLSiuWnV3FqOJuQ%40mail.gmail.com
Author: Alexander Korotkov
Reviewed-by: Heikki Linnakangas
Backpatch-through: 9.4

5 years agoFix behavior of AND CHAIN outside of explicit transaction blocks
Peter Eisentraut [Sun, 8 Sep 2019 14:11:21 +0000 (16:11 +0200)]
Fix behavior of AND CHAIN outside of explicit transaction blocks

When using COMMIT AND CHAIN or ROLLBACK AND CHAIN not in an explicit
transaction block, the previous implementation would leave a
transaction block active in the ROLLBACK case but not the COMMIT case.
To fix for now, error out when using these commands not in an explicit
transaction block.  This restriction could be lifted if a sensible
definition and implementation is found.

Bug: #15977
Author: fn ln <emuser20140816@gmail.com>
Reviewed-by: Fabien COELHO <coelho@cri.ensmp.fr>
5 years agodoc: effective -> efficient
Peter Eisentraut [Sun, 8 Sep 2019 09:10:49 +0000 (11:10 +0200)]
doc: effective -> efficient

5 years agodoc: Clean up title case use
Peter Eisentraut [Sun, 8 Sep 2019 08:26:35 +0000 (10:26 +0200)]
doc: Clean up title case use

Note: Following existing practice, titles of formalpara and step are
not titlecased.

5 years agoAvoid using INFO elevel for what are fundamentally debug messages.
Tom Lane [Sat, 7 Sep 2019 23:03:11 +0000 (19:03 -0400)]
Avoid using INFO elevel for what are fundamentally debug messages.

Commit 6f6b99d13 stuck an INFO message into the fast path for
checking partition constraints, for no very good reason except
that it made it easy for the regression tests to verify that
that path was taken.  Assorted later patches did likewise,
increasing the unsuppressable-chatter level from ALTER TABLE
even more.  This isn't good for the user experience, so let's
drop these messages down to DEBUG1 where they belong.  So as
not to have a loss of test coverage, create a TAP test that
runs the relevant queries with client_min_messages = DEBUG1
and greps for the expected messages.

This testing method is a bit brute-force --- in particular,
it duplicates the execution of a fair amount of the core
create_table and alter_table tests.  We experimented with
other solutions, but running any significant amount of
standard testing with client_min_messages = DEBUG1 seems
to have a lot of output-stability pitfalls, cf commits
bbb96c370 and 5655565c0.  Possibly at some point we'll look
into whether we can reduce the amount of test duplication.

Backpatch into v12, because some of these messages are new
in v12 and we don't really want to ship it that way.

Sergei Kornilov

Discussion: https://postgr.es/m/81911511895540@web58j.yandex.ru
Discussion: https://postgr.es/m/4859321552643736@myt5-02b80404fd9e.qloud-c.yandex.net

5 years agoDoc: make an editing pass over the v12 release notes.
Tom Lane [Sat, 7 Sep 2019 15:21:24 +0000 (11:21 -0400)]
Doc: make an editing pass over the v12 release notes.

Copy-edit or rewrite some items, add some items that I thought
deserved documenting, remove some others that didn't (notably,
I'm not on board with documenting documentation changes), fix
some poorly-pointed links, move some items to other sections,
etc etc.

5 years agoMessage style fixes
Peter Eisentraut [Fri, 6 Sep 2019 14:12:28 +0000 (16:12 +0200)]
Message style fixes

5 years agodoc: Fix awkward markup
Peter Eisentraut [Fri, 6 Sep 2019 20:19:53 +0000 (22:19 +0200)]
doc: Fix awkward markup

5 years agodoc: Postgres -> PostgreSQL
Peter Eisentraut [Fri, 6 Sep 2019 20:16:58 +0000 (22:16 +0200)]
doc: Postgres -> PostgreSQL

5 years agoUpdate v12 release notes through today, and add major-enhancements list.
Tom Lane [Fri, 6 Sep 2019 19:15:38 +0000 (15:15 -0400)]
Update v12 release notes through today, and add major-enhancements list.

I still want to review the rest of the notes, but this seems like the
minimum work required to prepare for beta4.

Major-enhancements text, and a couple of other fixes, from Jonathan Katz
(with minor copy-editing by me).

5 years agoDoc: tweak installation doc edits made by commit 76c2af926.
Tom Lane [Fri, 6 Sep 2019 15:24:36 +0000 (11:24 -0400)]
Doc: tweak installation doc edits made by commit 76c2af926.

We don't consider that building with MinGW is deprecated,
so adjust some places that gave that impression.
Per discussion with Peter Eisentraut.

Discussion: https://postgr.es/m/4a023388-8652-fea0-a0b4-35ad5e734e9a@2ndquadrant.com

5 years agoWhen performing a base backup, check for read errors.
Robert Haas [Fri, 6 Sep 2019 12:22:32 +0000 (08:22 -0400)]
When performing a base backup, check for read errors.

The old code didn't differentiate between a read error and a
concurrent truncation. fread reports both of these by returning 0;
you have to use feof() or ferror() to distinguish between them,
which this code did not do.

It might be a better idea to use read() rather than fread() here,
so that we can display a less-generic error message, but I'm not
sure that would qualify as a back-patchable bug fix, so just do
this much for now.

Jeevan Chalke, reviewed by Jeevan Ladhe and by me.

Discussion: http://postgr.es/m/CA+TgmobG4ywMzL5oQq2a8YKp8x2p3p1LOMMcGqpS7aekT9+ETA@mail.gmail.com

5 years agolibpq: ccache -> credential cache
Peter Eisentraut [Fri, 6 Sep 2019 07:15:35 +0000 (09:15 +0200)]
libpq: ccache -> credential cache

The term "ccache" is overloaded.  Let's be more clear, in case someone
other than a Kerberos wizard has to read this code.

5 years agoMake pg_promote() detect postmaster death while waiting for promotion to end.
Fujii Masao [Fri, 6 Sep 2019 05:27:25 +0000 (14:27 +0900)]
Make pg_promote() detect postmaster death while waiting for promotion to end.

Previously even if postmaster died and WaitLatch() woke up with that event
while pg_promote() was waiting for the standby promotion to finish,
pg_promote() did nothing special and kept waiting until timeout occurred.
This could cause a busy loop.

This patch make pg_promote() return false immediately when postmaster
dies, to avoid such a busy loop.

Back-patch to v12 where pg_promote() was added.

Author: Fujii Masao
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/CAHGQGwEs9ROgSp+QF+YdDU+xP8W=CY1k-_Ov-d_Z3JY+to3eXA@mail.gmail.com

5 years agoFix thinko when ending progress report for a backend
Michael Paquier [Wed, 4 Sep 2019 06:46:45 +0000 (15:46 +0900)]
Fix thinko when ending progress report for a backend

The logic ending progress reporting for a backend entry introduced by
b6fb647 causes callers of pgstat_progress_end_command() to do some extra
work when track_activities is enabled as the process fields are reset in
the backend entry even if no command were started for reporting.

This resets the fields only if a command is registered for progress
reporting, and only if track_activities is enabled.

Author: Masahiho Sawada
Discussion: https://postgr.es/m/CAD21AoCry_vJ0E-m5oxJXGL3pnos-xYGCzF95rK5Bbi3Uf-rpA@mail.gmail.com
Backpatch-through: 9.6

5 years agoDelay fsyncs of pg_basebackup until the end of backup
Michael Paquier [Wed, 4 Sep 2019 04:23:41 +0000 (13:23 +0900)]
Delay fsyncs of pg_basebackup until the end of backup

Since the addition of fsync requests in bc34223 to make base backup data
consistent on disk once pg_basebackup finishes, each tablespace tar file
is individually flushed once completed, with an additional flush of the
parent directory when the base backup finishes.  While holding a
connection to the server, a fsync request taking a long time may cause a
failure of the base backup, which is annoying for any integration.  A
recent example of breakage can involve tcp_user_timeout, but
wal_sender_timeout can cause similar problems.

While reviewing the code, there was a second issue causing too many
fsync requests to be done for the same WAL data.  As recursive fsyncs
are done at the end of the backup for both the plain and tar formats
from the base target directory where everything is written, it is fine
to disable fsyncs when fetching or streaming WAL.

Reported-by: Ryohei Takahashi
Author: Michael Paquier
Reviewed-by: Ryohei Takahashi
Discussion: https://postgr.es/m/OSBPR01MB4550DAE2F8C9502894A45AAB82BE0@OSBPR01MB4550.jpnprd01.prod.outlook.com
Backpatch-through: 10

5 years agoClarify pg_dump documentation
Peter Eisentraut [Tue, 3 Sep 2019 12:25:26 +0000 (14:25 +0200)]
Clarify pg_dump documentation

Clarify in the help output and documentation that -n, -t etc. take a
"pattern" rather than a "schema" or "table" etc.  This was especially
confusing now that the new pg_dumpall --exclude-database option was
documented with "pattern" and the others not, even though they all
behave the same.

Discussion: https://www.postgresql.org/message-id/flat/b85f3fa1-b350-38d1-1893-4f7911bd7310%402ndquadrant.com

5 years agopg_checksums: Handle read and write returns correctly
Peter Eisentraut [Tue, 3 Sep 2019 06:26:55 +0000 (08:26 +0200)]
pg_checksums: Handle read and write returns correctly

The read() return was not checking for errors, the write() return was
not checking for short writes.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/5de61b6b-8be9-7771-0048-860328efe027%402ndquadrant.com

5 years agoBetter error messages for short reads/writes in SLRU
Peter Eisentraut [Tue, 3 Sep 2019 06:26:55 +0000 (08:26 +0200)]
Better error messages for short reads/writes in SLRU

This avoids getting a

    Could not read from file ...: Success.

for a short read or write (since errno is not set in that case).
Instead, report a more specific error messages.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/5de61b6b-8be9-7771-0048-860328efe027%402ndquadrant.com

5 years agoFix memory leak with lower, upper and initcap with ICU-provided collations
Michael Paquier [Tue, 3 Sep 2019 03:31:03 +0000 (12:31 +0900)]
Fix memory leak with lower, upper and initcap with ICU-provided collations

The leak happens in str_tolower, str_toupper and str_initcap, which are
used in several places including their equivalent SQL-level functions,
and can only be triggered when using an ICU-provided collation when
converting the input string.

b615920 fixed a similar leak.  Backpatch down 10 where ICU collations
have been introduced.

Author: Konstantin Knizhnik
Discussion: https://postgr.es/m/94c0ad0a-cbc2-e4a3-7829-2bdeaf9146db@postgrespro.ru
Backpatch-through: 10

5 years agoAvoid touching replica identity index in ExtractReplicaIdentity().
Tom Lane [Mon, 2 Sep 2019 20:10:37 +0000 (16:10 -0400)]
Avoid touching replica identity index in ExtractReplicaIdentity().

In what seems like a fit of misplaced optimization,
ExtractReplicaIdentity() accessed the relation's replica-identity
index without taking any lock on it.  Usually, the surrounding query
already holds some lock so this is safe enough ... but in the case
of a previously-planned delete, there might be no existing lock.
Given a suitable test case, this is exposed in v12 and HEAD by an
assertion added by commit b04aeb0a0.

The whole thing's rather poorly thought out anyway; rather than
looking directly at the index, we should use the index-attributes
bitmap that's held by the parent table's relcache entry, as the
caller functions do.  This is more consistent and likely a bit
faster, since it avoids a cache lookup.  Hence, change to doing it
that way.

While at it, rather than blithely assuming that the identity
columns are non-null (with catastrophic results if that's wrong),
add assertion checks that they aren't null.  Possibly those should
be actual test-and-elog, but I'll leave it like this for now.

In principle, this is a bug that's been there since this code was
introduced (in 9.4).  In practice, the risk seems quite low, since
we do have a lock on the index's parent table, so concurrent
changes to the index's catalog entries seem unlikely.  Given the
precedent that commit 9c703c169 wasn't back-patched, I won't risk
back-patching this further than v12.

Per report from Hadi Moshayedi.

Discussion: https://postgr.es/m/CAK=1=Wrek44Ese1V7LjKiQS-Nd-5LgLi_5_CskGbpggKEf3tKQ@mail.gmail.com

5 years agoHandle corner cases correctly in psql's reconnection logic.
Tom Lane [Mon, 2 Sep 2019 18:02:45 +0000 (14:02 -0400)]
Handle corner cases correctly in psql's reconnection logic.

After an unexpected connection loss and successful reconnection,
psql neglected to resynchronize its internal state about the server,
such as server version.  Ordinarily we'd be reconnecting to the same
server and so this isn't really necessary, but there are scenarios
where we do need to update --- one example is where we have a list
of possible connection targets and they're not all alike.

Define "resynchronize" as including connection_warnings(), so that
this case acts the same as \connect.  This seems useful; for example,
if the server version did change, the user might wish to know that.
An attuned user might also notice that the new connection isn't
SSL-encrypted, for example, though this approach isn't especially
in-your-face about such changes.  Although this part is a behavioral
change, it only affects interactive sessions, so it should not break
any applications.

Also, in do_connect, make sure that we desynchronize correctly when
abandoning an old connection in non-interactive mode.

These problems evidently are the result of people patching only one
of the two places where psql deals with connection changes, so insert
some cross-referencing comments in hopes of forestalling future bugs
of the same ilk.

Lastly, in Windows builds, issue codepage mismatch warnings only at
startup, not during reconnections.  psql's codepage can't change
during a reconnect, so complaining about it again seems like useless
noise.

Peter Billen and Tom Lane.  Back-patch to all supported branches.

Discussion: https://postgr.es/m/CAMTXbE8e6U=EBQfNSe01Ej17CBStGiudMAGSOPaw-ALxM-5jXg@mail.gmail.com

5 years agoDoc: describe the "options" allowed in an ECPG connection target string.
Tom Lane [Sat, 31 Aug 2019 18:05:32 +0000 (14:05 -0400)]
Doc: describe the "options" allowed in an ECPG connection target string.

These have been there a long time, but their format was never explained
in the docs.  Per complaint from Yusuke Egashira.

Discussion: https://postgr.es/m/848B1649C8A6274AA527C4472CA11EDD5FC70CBE@G01JPEXMBYT02

5 years agoDoc: remove some long-obsolete information from installation.sgml.
Tom Lane [Fri, 30 Aug 2019 17:02:35 +0000 (13:02 -0400)]
Doc: remove some long-obsolete information from installation.sgml.

Section 16.2 pointed to platform-specific FAQ files that we removed
way back in 8.4.  Section 16.7 contained a bunch of information about
AIX and HPUX bugs that were squashed decades ago, plus discussions of
old compiler versions that are certainly moot now that we require C99
support.  Since we're obviously not maintaining this stuff carefully,
just remove it.  The HPUX sub-section seems like it can go away
entirely, since everything it said that was still applicable was
redundant with material elsewhere in the chapter.

In passing, I couldn't resist the temptation to do a small amount
of copy-editing on nearby text.

Back-patch to v12, since this stuff is surely obsolete in any
branch that requires C99.

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

5 years agoFix typos in regression test comments.
Etsuro Fujita [Thu, 29 Aug 2019 09:45:01 +0000 (18:45 +0900)]
Fix typos in regression test comments.

5 years agoFix overflow check and comment in GIN posting list encoding.
Heikki Linnakangas [Wed, 28 Aug 2019 09:55:33 +0000 (12:55 +0300)]
Fix overflow check and comment in GIN posting list encoding.

The comment did not match what the code actually did for integers with
the 43rd bit set. You get an integer like that, if you have a posting
list with two adjacent TIDs that are more than 2^31 blocks apart.
According to the comment, we would store that in 6 bytes, with no
continuation bit on the 6th byte, but in reality, the code encodes it
using 7 bytes, with a continuation bit on the 6th byte as normal.

The decoding routine also handled these 7-byte integers correctly, except
for an overflow check that assumed that one integer needs at most 6 bytes.
Fix the overflow check, and fix the comment to match what the code
actually does. Also fix the comment that claimed that there are 17 unused
bits in the 64-bit representation of an item pointer. In reality, there
are 64-32-11=21.

Fitting any item pointer into max 6 bytes was an important property when
this was written, because in the old pre-9.4 format, item pointers were
stored as plain arrays, with 6 bytes for every item pointer. The maximum
of 6 bytes per integer in the new format guaranteed that we could convert
any page from the old format to the new format after upgrade, so that the
new format was never larger than the old format. But we hardly need to
worry about that anymore, and running into that problem during upgrade,
where an item pointer is expanded from 6 to 7 bytes such that the data
doesn't fit on a page anymore, is implausible in practice anyway.

Backpatch to all supported versions.

This also includes a little test module to test these large distances
between item pointers, without requiring a 16 TB table. It is not
backpatched, I'm including it more for the benefit of future development
of new posting list formats.

Discussion: https://www.postgresql.org/message-id/33bfc20a-5c86-f50c-f5a5-58e9925d05ff%40iki.fi
Reviewed-by: Masahiko Sawada, Alexander Korotkov
5 years agoAvoid catalog lookups in RelationAllowsEarlyPruning().
Thomas Munro [Wed, 28 Aug 2019 01:37:03 +0000 (13:37 +1200)]
Avoid catalog lookups in RelationAllowsEarlyPruning().

RelationAllowsEarlyPruning() performed a catalog scan, but is used
in two contexts where that was a bad idea:

1.  In heap_page_prune_opt(), which runs very frequently in some large
    scans.  This caused major performance problems in a field report
    that was easy to reproduce.

2.  In TestForOldSnapshot(), which runs while we hold a buffer content
    lock.  It's not clear if this was guaranteed to be free of buffer
    deadlock risk.

The check was introduced in commit 2cc41acd8 and defended against a
real problem: 9.6's hash indexes have no page LSN and so we can't
allow early pruning (ie the snapshot-too-old feature).  We can remove
the check from all later releases though: hash indexes are now logged,
and there is no way to create UNLOGGED indexes on regular logged
tables.

If a future release allows such a combination, it might need to put
a similar check in place, but it'll need some more thought.

Back-patch to 10.

Author: Thomas Munro
Reviewed-by: Tom Lane, who spotted the second problem
Discussion: https://postgr.es/m/CA%2BhUKGKT8oTkp5jw_U4p0S-7UG9zsvtw_M47Y285bER6a2gD%2Bg%40mail.gmail.com
Discussion: https://postgr.es/m/CAA4eK1%2BWy%2BN4eE5zPm765h68LrkWc3Biu_8rzzi%2BOYX4j%2BiHRw%40mail.gmail.com

5 years agoDisable timeouts when running pg_rewind with online source cluster
Michael Paquier [Wed, 28 Aug 2019 02:48:15 +0000 (11:48 +0900)]
Disable timeouts when running pg_rewind with online source cluster

In this case, the transfer uses a libpq connection, which is subject to
the timeout parameters set at system level, and this can make the rewind
operation suddenly canceled which is not good for automation.  One
workaround to such issues would be to use PGOPTIONS to enforce the
wanted timeout parameters, but that's annoying, and for example pg_dump,
which can run potentially long-running queries disables all types of
timeouts.

lock_timeout and statement_timeout are the ones which can cause problems
now.  Note that pg_rewind does not use transactions, so disabling
idle_in_transaction_session_timeout is optional, but it feels safer to
do so for the future.

This is back-patched down to 9.5.  idle_in_transaction_session_timeout
is only present since 9.6.

Author: Alexander Kukushkin
Discussion: https://postgr.es/m/CAFh8B=krcVXksxiwVQh1SoY+ziJ-JC=6FcuoBL3yce_40Es5_g@mail.gmail.com
Backpatch-through: 9.5

5 years agoDoc: improve documentation of pg_signal_backend default role.
Tom Lane [Tue, 27 Aug 2019 22:03:09 +0000 (18:03 -0400)]
Doc: improve documentation of pg_signal_backend default role.

Give it an explanatory para like the other default roles have.
Don't imply that it can send any signal whatever.

In passing, reorder the table entries and explanatory paras
for the default roles into some semblance of consistency.

Ian Barwick, tweaked a bit by me.

Discussion: https://postgr.es/m/89907e32-76f3-7282-a89c-ea19c722fe5d@2ndquadrant.com

5 years agoImprove what pg_strsignal prints if we haven't got strsignal(3).
Tom Lane [Tue, 27 Aug 2019 21:24:13 +0000 (17:24 -0400)]
Improve what pg_strsignal prints if we haven't got strsignal(3).

Turns out that returning "unrecognized signal" is confusing.
Make it explicit that the platform lacks any support for signal names.
(At least of the machines in the buildfarm, only HPUX lacks it.)

Back-patch to v12 where we invented this function.

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

5 years agoDoc: clarify behavior of standard aggregates for null inputs.
Tom Lane [Tue, 27 Aug 2019 20:37:22 +0000 (16:37 -0400)]
Doc: clarify behavior of standard aggregates for null inputs.

Section 4.2.7 says that unless otherwise specified, built-in
aggregates ignore rows in which any input is null.  This is
not true of the JSON aggregates, but it wasn't documented.
Fix that.

Of the other entries in table 9.55, some were explicit about
ignoring nulls, and some weren't; for consistency and
self-contained-ness, make them all say it explicitly.

Per bug #15884 from Tim Möhlmann.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/15884-c32d848f787fcae3@postgresql.org

5 years agoReject empty names and recursion in config-file include directives.
Tom Lane [Tue, 27 Aug 2019 18:44:26 +0000 (14:44 -0400)]
Reject empty names and recursion in config-file include directives.

An empty file name or subdirectory name leads join_path_components() to
just produce the parent directory name, which leads to weird failures or
recursive inclusions.  Let's throw a specific error for that.  It takes
only slightly more code to detect all-blank names, so do so.

Also, detect direct recursion, ie a file calling itself.  As coded
this will also detect recursion via "include_dir '.'", which is
perhaps more likely than explicitly including the file itself.

Detecting indirect recursion would require API changes for guc-file.l
functions, which seems not worth it since extensions might call them.
The nesting depth limit will catch such cases eventually, just not
with such an on-point error message.

In passing, adjust the example usages in postgresql.conf.sample
to perhaps eliminate the problem at the source: there's no reason
for the examples to suggest that an empty value is valid.

Per a trouble report from Brent Bates.  Back-patch to 9.5; the
issue is old, but the code in 9.4 is enough different that the
patch doesn't apply easily, and it doesn't seem worth the trouble
to fix there.

Ian Barwick and Tom Lane

Discussion: https://postgr.es/m/8c8bcbca-3bd9-dc6e-8986-04a5abdef142@2ndquadrant.com

5 years agoFix failure of --jobs with vacuumdb on Windows
Michael Paquier [Tue, 27 Aug 2019 00:11:38 +0000 (09:11 +0900)]
Fix failure of --jobs with vacuumdb on Windows

FD_SETSIZE needs to be declared before winsock2.h, or it is possible to
run into buffer overflow issues when using --jobs.  This is similar to
pgbench's solution done in a23c641.

This has been introduced by 71d84ef, and older versions have been using
the default value of FD_SETSIZE, defined at 64.

Per buildfarm member jacana, but this impacts all Windows animals
running the TAP tests.  I have reproduced the failure locally to check
the patch.

Author: Michael Paquier
Reviewed-by: Andrew Dunstan
Discussion: https://postgr.es/m/20190826054000.GE7005@paquier.xyz
Backpatch-through: 9.5

5 years agoFix gettext triggers specification
Peter Eisentraut [Mon, 26 Aug 2019 17:04:35 +0000 (19:04 +0200)]
Fix gettext triggers specification

In cc8d41511721d25d557fc02a46c053c0a602fed0, the arguments of
warn_or_exit_horribly() were changed but this was not updated.

5 years agoAdjust to latest Msys2 kernel release number
Andrew Dunstan [Mon, 26 Aug 2019 12:11:27 +0000 (08:11 -0400)]
Adjust to latest Msys2 kernel release number

Previously 'uname -r' on Msys2 reported a kernele release starting with
2. The latest version starts with 3. In commit 1638623f we specifically
looked for one starting with 2. This is now changed to look for any
digit between 2 and 9.

backpatch to release 10.

5 years agoTreat MINGW and MSYS the same in pg_upgrade test script
Andrew Dunstan [Mon, 26 Aug 2019 11:44:34 +0000 (07:44 -0400)]
Treat MINGW and MSYS the same in pg_upgrade test script

On msys2, 'uname -s' reports a string starting MSYS instead on MINGW
as happens on msys1. Treat these both the same way. This reverts
608a710195a4b in favor of a more general solution.

Backpatch to all live branches.

5 years agoFix error handling of vacuumdb when running out of fds
Michael Paquier [Mon, 26 Aug 2019 02:14:24 +0000 (11:14 +0900)]
Fix error handling of vacuumdb when running out of fds

When trying to use a high number of jobs, vacuumdb has only checked for
a maximum number of jobs used, causing confusing failures when running
out of file descriptors when the jobs open connections to Postgres.
This commit changes the error handling so as we do not check anymore for
a maximum number of allowed jobs when parsing the option value with
FD_SETSIZE, but check instead if a file descriptor is within the
supported range when opening the connections for the jobs so as this is
detected at the earliest time possible.

Also, improve the error message to give a hint about the number of jobs
recommended, using a wording given by the reviewers of the patch.

Reported-by: Andres Freund
Author: Michael Paquier
Reviewed-by: Andres Freund, Álvaro Herrera, Tom Lane
Discussion: https://postgr.es/m/20190818001858.ho3ev4z57fqhs7a5@alap3.anarazel.de
Backpatch-through: 9.5

5 years agoAvoid platform-specific null pointer dereference in psql.
Tom Lane [Sun, 25 Aug 2019 19:04:04 +0000 (15:04 -0400)]
Avoid platform-specific null pointer dereference in psql.

POSIX permits getopt() to advance optind beyond argc when the last
argv entry is an option that requires an argument and hasn't got one.
It seems that no major platforms actually do that, but musl does,
so that something like "psql -f" would crash with that libc.
Add a check that optind is in range before trying to look at the
possibly-bogus option.

Report and fix by Quentin Rameau.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/20190825100617.GA6087@fifth.space

5 years agoBack off output precision in circle.sql regression test.
Tom Lane [Sun, 25 Aug 2019 16:14:50 +0000 (12:14 -0400)]
Back off output precision in circle.sql regression test.

We were setting extra_float_digits = 0 to avoid platform-dependent
output in this test, but that's still able to expose platform-specific
roundoff behavior in some new test cases added by commit a3d284485,
as reported by Peter Eisentraut.  Reduce it to -1 to hide that.

(Over in geometry.sql, we're using -3, which is an ancient decision
dating to 337f73b1b.  I wonder whether that's overkill now.  But
there's probably little value in trying to change it.)

Back-patch to v12 where a3d284485 came in; there's no evidence that
we have any platform-dependent issues here before that.

Discussion: https://postgr.es/m/15551268-e224-aa46-084a-124b64095ee3@2ndquadrant.com

5 years agoDon't rely on llvm::make_unique.
Thomas Munro [Sun, 25 Aug 2019 01:54:48 +0000 (13:54 +1200)]
Don't rely on llvm::make_unique.

Bleeding-edge LLVM has stopped supplying replacements for various
C++14 library features, for people on older C++ versions.  Since we're
not ready to require C++14 yet, just use plain old new instead of
make_unique.  As revealed by buildfarm animal seawasp.

Back-patch to 11.

Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/CA%2BhUKGJWG7unNqmkxg7nC5o3o-0p2XP6co4r%3D9epqYMm8UY4Mw%40mail.gmail.com

5 years agoImprove documentation of pageinspect
Michael Paquier [Fri, 23 Aug 2019 11:41:13 +0000 (20:41 +0900)]
Improve documentation of pageinspect

This adds a section for heap-related functions.  These were previously
mixed with functions having a more general purpose, leading to
confusion.  While on it, add a query example for fsm_page_contents.

Backpatch down to 10, where b5e3942 introduced the subsections for
function types in pageinspect documentation.

Author: Masahiko Sawada
Discussion: https://postgr.es/m/CAD21AoDyM7E1+cK3-aWejxKTGC-wVVP2B+RnJhN6inXyeRmqzw@mail.gmail.com
Backpatch-through: 10

5 years agodoc: PG 12 relnotes, correct pg_stat_database.datoid to be datid
Bruce Momjian [Fri, 23 Aug 2019 00:19:20 +0000 (20:19 -0400)]
doc: PG 12 relnotes, correct pg_stat_database.datoid to be datid

Previous column name was incorrect.

Reported-by: Justin Pryzby
Backpatch-through: 12

5 years agoFixes for "Glyph not available" warnings from FOP
Peter Eisentraut [Thu, 22 Aug 2019 20:36:45 +0000 (22:36 +0200)]
Fixes for "Glyph not available" warnings from FOP

see d315639c82e8a2cfd1d1b98b7acf0f6c033ab955

5 years agoAdd list of acknowledgments to release notes
Peter Eisentraut [Thu, 22 Aug 2019 20:35:00 +0000 (22:35 +0200)]
Add list of acknowledgments to release notes

This contains all individuals mentioned in the commit messages during
PostgreSQL 12 development.

current through 068bc300c6ed5333d9561e55cb5f45d17de88422

5 years agoMake SQL/JSON error code names match SQL standard
Peter Eisentraut [Thu, 22 Aug 2019 08:17:30 +0000 (10:17 +0200)]
Make SQL/JSON error code names match SQL standard

There were some minor differences that didn't seem necessary.

Discussion: https://www.postgresql.org/message-id/flat/86b67eef-bb26-c97d-3e35-64f1fbd4f9fe%402ndquadrant.com

5 years agoDoc: Remove mention to "Visual Studio Express 2019"
Michael Paquier [Thu, 22 Aug 2019 00:59:16 +0000 (09:59 +0900)]
Doc: Remove mention to "Visual Studio Express 2019"

The "Express" flavor of Visual Studio exists up to 2017, and the
documentation referred to "Express" for Visual Studio 2019.

Author: Takuma Hoshiai
Discussion: https://postgr.es/m/20190820120231.f905542e685140258ca73d82@sraoss.co.jp
Backpatch-through: 9.4

5 years agoFix typo
Alvaro Herrera [Wed, 21 Aug 2019 15:12:44 +0000 (11:12 -0400)]
Fix typo

In early development patches, "replication origins" were called "identifiers";
almost everything was renamed, but these references to the old terminology
went unnoticed.

Reported-by: Craig Ringer
5 years agoFix bogus comment
Alvaro Herrera [Tue, 20 Aug 2019 20:04:09 +0000 (16:04 -0400)]
Fix bogus comment

Author: Alexander Lakhin
Discussion: https://postgr.es/m/20190819072244.GE18166@paquier.xyz

5 years agoDoc: Fix various typos
Michael Paquier [Tue, 20 Aug 2019 04:45:53 +0000 (13:45 +0900)]
Doc: Fix various typos

All those fixes are already included on HEAD thanks to for example
c96581a and 66bde49, and have gone missing on back-branches.

Author: Alexander Lakhin, Liudmila Mantrova
Discussion: https://postgr.es/m/CAEkD-mDJHV3bhgezu3MUafJLoAKsOOT86+wHukKU8_NeiJYhLQ@mail.gmail.com
Backpatch-through: 9.4

5 years agoDoc: Improve wording of multiple places in documentation
Michael Paquier [Tue, 20 Aug 2019 03:37:30 +0000 (12:37 +0900)]
Doc: Improve wording of multiple places in documentation

This has been found during its translation.

Author: Liudmila Mantrova
Discussion: https://postgr.es/m/CAEkD-mDJHV3bhgezu3MUafJLoAKsOOT86+wHukKU8_NeiJYhLQ@mail.gmail.com
Backpatch-through: 12

5 years agoRestore json{b}_populate_record{set}'s ability to take type info from AS.
Tom Lane [Mon, 19 Aug 2019 22:00:57 +0000 (18:00 -0400)]
Restore json{b}_populate_record{set}'s ability to take type info from AS.

If the record argument is NULL and has no declared type more concrete
than RECORD, we can't extract useful information about the desired
rowtype from it.  In this case, see if we're in FROM with an AS clause,
and if so extract the needed rowtype info from AS.

It worked like this before v11, but commit 37a795a60 removed the
behavior, reasoning that it was undocumented, inefficient, and utterly
not self-consistent.  If you want to take type info from an AS clause,
you should be using the json_to_record() family of functions not the
json_populate_record() family.  Also, it was already the case that
the "populate" functions would fail for a null-valued RECORD input
(with an unfriendly "record type has not been registered" error)
when there wasn't an AS clause at hand, and it wasn't obvious that
that behavior wasn't OK when there was one.  However, it emerges
that some people were depending on this to work, and indeed the
rather off-point error message you got if you left off AS encouraged
slapping on AS without switching to the json_to_record() family.

Hence, put back the fallback behavior of looking for AS.  While at it,
improve the run-time error you get when there's no place to obtain type
info; we can do a lot better than "record type has not been registered".
(We can't, unfortunately, easily improve the parse-time error message
that leads people down this path in the first place.)

While at it, I refactored the code a bit to avoid duplicating the
same logic in several different places.

Per bug #15940 from Jaroslav Sivy.  Back-patch to v11 where the
current coding came in.  (The pre-v11 deficiencies in this area
aren't regressions, so we'll leave those branches alone.)

Patch by me, based on preliminary analysis by Dmitry Dolgov.

Discussion: https://postgr.es/m/15940-2ab76dc58ffb85b6@postgresql.org

5 years agodoc: Fix image use in PDF build with vpath
Peter Eisentraut [Mon, 19 Aug 2019 08:30:47 +0000 (10:30 +0200)]
doc: Fix image use in PDF build with vpath

In a vpath build, we need to point to the source directory to allow
FOP to find the images.

5 years agoDisallow changing an inherited column's type if not all parents changed.
Tom Lane [Sun, 18 Aug 2019 21:11:57 +0000 (17:11 -0400)]
Disallow changing an inherited column's type if not all parents changed.

If a table inherits from multiple unrelated parents, we must disallow
changing the type of a column inherited from multiple such parents, else
it would be out of step with the other parents.  However, it's possible
for the column to ultimately be inherited from just one common ancestor,
in which case a change starting from that ancestor should still be
allowed.  (I would not be excited about preserving that option, were
it not that we have regression test cases exercising it already ...)

It's slightly annoying that this patch looks different from the logic
with the same end goal in renameatt(), and more annoying that it
requires an extra syscache lookup to make the test.  However, the
recursion logic is quite different in the two functions, and a
back-patched bug fix is no place to be trying to unify them.

Per report from Manuel Rigger.  Back-patch to 9.5.  The bug exists in
9.4 too (and doubtless much further back); but the way the recursion
is done in 9.4 is a good bit different, so that substantial refactoring
would be needed to fix it in 9.4.  I'm disinclined to do that, or risk
introducing new bugs, for a bug that has escaped notice for this long.

Discussion: https://postgr.es/m/CA+u7OA4qogDv9rz1HAb-ADxttXYPqQdUdPY_yd4kCzywNxRQXA@mail.gmail.com

5 years agoAdd default_table_access_method to postgresql.conf.sample.
Andres Freund [Fri, 16 Aug 2019 22:24:22 +0000 (15:24 -0700)]
Add default_table_access_method to postgresql.conf.sample.

Reported-By: Heikki Linnakangas
Author: Michael Paquier
Discussion: https://postgr.es/m/d6ffbebb-a0d2-181c-811d-b029b2225ed7@iki.fi
Backpatch: 12-, where pluggable table access methods were introduced

5 years agoPrevent possible double-free when update trigger returns old tuple.
Tom Lane [Fri, 16 Aug 2019 00:04:19 +0000 (20:04 -0400)]
Prevent possible double-free when update trigger returns old tuple.

This is a variant of the problem fixed in commit 25b692568, which
unfortunately we failed to detect at the time.  If an update trigger
returns the "old" tuple, as it's entitled to do, then a subsequent
iteration of the loop in ExecBRUpdateTriggers would have "oldtuple"
equal to "trigtuple" and would fail to notice that it shouldn't
free that.

In addition to fixing the code, extend the test case added by
25b692568 so that it covers multiple-trigger-iterations cases.

This problem does not manifest in v12/HEAD, as a result of the
relevant code having been largely rewritten for slotification.
However, include the test case into v12/HEAD anyway, since this
is clearly an area that someone could break again in future.

Per report from Piotr Gabriel Kosinski.  Back-patch into all
supported branches, since the bug seems quite old.

Diagnosis and code fix by Thomas Munro, test case by me.

Discussion: https://postgr.es/m/CAFMLSdP0rd7LqC3j-H6Fh51FYSt5A10DDh-3=W4PPc4LLUQ8YQ@mail.gmail.com

5 years agoFix plpgsql to re-look-up composite type names at need.
Tom Lane [Thu, 15 Aug 2019 19:21:47 +0000 (15:21 -0400)]
Fix plpgsql to re-look-up composite type names at need.

Commit 4b93f5799 rearranged things in plpgsql to make it cope better with
composite types changing underneath it intra-session.  However, I failed to
consider the case of a composite type being dropped and recreated entirely.
In my defense, the previous coding didn't consider that possibility at all
either --- but it would accidentally work so long as you didn't change the
type's field list, because the built-at-compile-time list of component
variables would then still match the type's new definition.  The new
coding, however, occasionally tries to re-look-up the type by OID, and
then fails to find the dropped type.

To fix this, we need to save the TypeName struct, and then redo the type
OID lookup from that.  Of course that's expensive, so we don't want to do
it every time we need the type OID.  This can be fixed in the same way that
4b93f5799 dealt with changes to composite types' definitions: keep an eye
on the type's typcache entry to see if its tupledesc has been invalidated.
(Perhaps, at some point, this mechanism should be generalized so it can
work for non-composite types too; but for now, plpgsql only tries to
cope with intra-session redefinitions of composites.)

I'm slightly hesitant to back-patch this into v11, because it changes
the contents of struct PLpgSQL_type as well as the signature of
plpgsql_build_datatype(), so in principle it could break code that is
poking into the innards of plpgsql.  However, the only popular extension
of that ilk is pldebugger, and it doesn't seem to be affected.  Since
this is a regression for people who were relying on the old behavior,
it seems worth taking the small risk of causing compatibility issues.

Per bug #15913 from Daniel Fiori.  Back-patch to v11 where 4b93f5799
came in.

Discussion: https://postgr.es/m/15913-a7e112e16dedcffc@postgresql.org

5 years agoDoc: improve documentation about postgresql.auto.conf.
Tom Lane [Thu, 15 Aug 2019 15:14:26 +0000 (11:14 -0400)]
Doc: improve documentation about postgresql.auto.conf.

Clarify what external tools can do to this file, and add a bit
of detail about what ALTER SYSTEM itself does.

Discussion: https://postgr.es/m/aed6cc9f-98f3-2693-ac81-52bb0052307e@2ndquadrant.com

5 years agoFix ALTER SYSTEM to cope with duplicate entries in postgresql.auto.conf.
Tom Lane [Wed, 14 Aug 2019 19:09:20 +0000 (15:09 -0400)]
Fix ALTER SYSTEM to cope with duplicate entries in postgresql.auto.conf.

ALTER SYSTEM itself normally won't make duplicate entries (although
up till this patch, it was possible to confuse it by writing case
variants of a GUC's name).  However, if some external tool has appended
entries to the file, that could result in duplicate entries for a single
GUC name.  In such a situation, ALTER SYSTEM did exactly the wrong thing,
because it replaced or removed only the first matching entry, leaving
the later one(s) still there and hence still determining the active value.

This patch fixes that by making ALTER SYSTEM sweep through the file and
remove all matching entries, then (if not ALTER SYSTEM RESET) append the
new setting to the end.  This means entries will be in order of last
setting rather than first setting, but that shouldn't hurt anything.

Also, make the comparisons case-insensitive so that the right things
happen if you do, say, ALTER SYSTEM SET "TimeZone" = 'whatever'.

This has been broken since ALTER SYSTEM was invented, so back-patch
to all supported branches.

Ian Barwick, with minor mods by me

Discussion: https://postgr.es/m/aed6cc9f-98f3-2693-ac81-52bb0052307e@2ndquadrant.com

5 years agoFix random regression failure in test case "collate.icu.utf8"
Michael Paquier [Wed, 14 Aug 2019 04:38:16 +0000 (13:38 +0900)]
Fix random regression failure in test case "collate.icu.utf8"

This is a fix similar to 2d7d67cc, where slight plan alteration can
cause a random failure of this regression test because of an incorect
tuple ordering, except that this one involves lookups of pg_type.
Similarly to the other case, add ORDER BY clauses to ensure the output
order.

The failure has been seen at least once on buildfarm member skink.

Reported-by: Thomas Munro
Discussion: https://postgr.es/m/CA+hUKGLjR9ZBvhXcr9b-NSBHPw9aRgbjyzGE+kqLsT4vwX+nkQ@mail.gmail.com
Backpatch-through: 12

5 years agoUn-break pg_dump for pre-8.3 source servers.
Tom Lane [Tue, 13 Aug 2019 20:57:58 +0000 (16:57 -0400)]
Un-break pg_dump for pre-8.3 source servers.

Commit 07b39083c inserted an unconditional reference to pg_opfamily,
which of course fails on servers predating that catalog.  Fortunately,
the case it's trying to solve can't occur on such old servers (AFAIK).
Hence, just skip the additional code when the source predates 8.3.

Per bug #15955 from sly.  Back-patch to all supported branches,
like the previous patch.

Discussion: https://postgr.es/m/15955-1daa2e676e903d87@postgresql.org

5 years agoFix random regression failure in test case "temp"
Michael Paquier [Tue, 13 Aug 2019 01:55:58 +0000 (10:55 +0900)]
Fix random regression failure in test case "temp"

This test case could fail because of an incorrect result ordering when
looking up at pg_class entries.  This commit adds an ORDER BY to the
culprit query.  The cause of the failure was likely caused by a plan
switch.  By default, the planner would likely choose an index-only scan
or an index scan, but even a small change in the startup cost could have
caused a bitmap heap scan to be chosen, causing the failure.

While on it, switch some filtering quals to a regular expression as per
an idea of Tom Lane.  As previously shaped, the quals would have
selected any relations whose name begins with "temp".  And that could
cause failures if another test running in parallel began to use similar
relation names.

Per report from buildfarm member anole, though the failure was very
rare.  This test has been introduced by 319a810, so backpatch down to
v10.

Discussion: https://postgr.es/m/20190807132422.GC15695@paquier.xyz
Backpatch-through: 10

5 years agoamcheck: Skip unlogged relations during recovery.
Peter Geoghegan [Mon, 12 Aug 2019 22:21:30 +0000 (15:21 -0700)]
amcheck: Skip unlogged relations during recovery.

contrib/amcheck failed to consider the possibility that unlogged
relations will not have any main relation fork files when running in hot
standby mode.  This led to low-level "can't happen" errors that complain
about the absence of a relfilenode file.

To fix, simply skip verification of unlogged index relations during
recovery.  In passing, add a direct check for the presence of a main
fork just before verification proper begins, so that we cleanly verify
the presence of the main relation fork file.

Author: Andrey Borodin, Peter Geoghegan
Reported-By: Andrey Borodin
Diagnosed-By: Andrey Borodin
Discussion: https://postgr.es/m/DA9B33AC-53CB-4643-96D4-7A0BBC037FA1@yandex-team.ru
Backpatch: 10-, where amcheck was introduced.

5 years agoFix planner's test for case-foldable characters in ILIKE with ICU.
Tom Lane [Mon, 12 Aug 2019 17:15:47 +0000 (13:15 -0400)]
Fix planner's test for case-foldable characters in ILIKE with ICU.

As coded, the ICU-collation path in pattern_char_isalpha() failed
to consider regular ASCII letters to be case-varying.  This led to
like_fixed_prefix treating too much of an ILIKE pattern as being a
fixed prefix, so that indexscans derived from an ILIKE clause might
miss entries that they should find.

Per bug #15892 from James Inform.  This is an oversight in the original
ICU patch (commit eccfef81e), so back-patch to v10 where that came in.

Discussion: https://postgr.es/m/15892-e5d2bea3e8a04a1b@postgresql.org

5 years agoFix string comparison in jsonpath
Alexander Korotkov [Mon, 12 Aug 2019 03:19:19 +0000 (06:19 +0300)]
Fix string comparison in jsonpath

Take into account pg_server_to_any() may return input string "as is".

Reported-by: Andrew Dunstan, Thomas Munro
Discussion: https://postgr.es/m/0ed83a33-d900-466a-880a-70ef456c721f%402ndQuadrant.com
Author: Alexander Korotkov, Thomas Munro
Backpatch-through: 12

5 years agoAdjust string comparison in jsonpath
Alexander Korotkov [Sun, 11 Aug 2019 19:54:53 +0000 (22:54 +0300)]
Adjust string comparison in jsonpath

We have implemented jsonpath string comparison using default database locale.
However, standard requires us to compare Unicode codepoints.  This commit
implements that, but for performance reasons we still use per-byte comparison
for "==" operator.  Thus, for consistency other comparison operators do per-byte
comparison if Unicode codepoints appear to be equal.

In some edge cases, when same Unicode codepoints have different binary
representations in database encoding, we diverge standard to achieve better
performance of "==" operator.  In future to implement strict standard
conformance, we can do normalization of input JSON strings.

Original patch was written by Nikita Glukhov, rewritten by me.

Reported-by: Markus Winand
Discussion: https://postgr.es/m/8B7FA3B4-328D-43D7-95A8-37B8891B8C78%40winand.at
Author: Nikita Glukhov, Alexander Korotkov
Backpatch-through: 12

5 years agoFix "ANALYZE t, t" inside a transaction block.
Tom Lane [Sat, 10 Aug 2019 15:30:11 +0000 (11:30 -0400)]
Fix "ANALYZE t, t" inside a transaction block.

This failed with either "tuple already updated by self" or "duplicate
key value violates unique constraint", depending on whether the table
had previously been analyzed or not.  The reason is that ANALYZE tried
to insert or update the same pg_statistic rows twice, and there was no
CommandCounterIncrement between.  So add one.  The same case works fine
outside a transaction block, because then there's a whole transaction
boundary between, as a consequence of the way VACUUM works.

This issue has been latent all along, but the problem was unreachable
before commit 11d8d72c2 added the ability to specify multiple tables
in ANALYZE.  We could, perhaps, alternatively fix it by adding code to
de-duplicate the list of VacuumRelations --- but that would add a
lot of overhead to work around dumb commands, so it's not attractive.

Per bug #15946 from Yaroslav Schekin.  Back-patch to v11.

(Note: in v11 I also back-patched the test added by commit 23224563d;
otherwise the problem doesn't manifest in the test I added, because
"vactst" is empty when the tests for multiple ANALYZE targets are
reached.  That seems like not a very good thing anyway, so I did this
rather than rethinking the choice of test case.)

Discussion: https://postgr.es/m/15946-5c7570a2884a26cf@postgresql.org

5 years agoFix SIGSEGV in pruning for ScalarArrayOp with constant-null array.
Tom Lane [Fri, 9 Aug 2019 17:20:28 +0000 (13:20 -0400)]
Fix SIGSEGV in pruning for ScalarArrayOp with constant-null array.

Not much to be said here: commit 9fdb675fc should have checked
constisnull, didn't.

Per report from Piotr Włodarczyk.  Back-patch to v11 where
bug was introduced.

Discussion: https://postgr.es/m/CAP-dhMr+vRpwizEYjUjsiZ1vwqpohTm+3Pbdt6Pr7FEgPq9R0Q@mail.gmail.com

5 years agoClarify the default partition's role
Alvaro Herrera [Thu, 8 Aug 2019 20:03:14 +0000 (16:03 -0400)]
Clarify the default partition's role

Reviewed by Tom Lane and Amit Langote
Discussion: https://postgr.es/m/20190806222735.GA9535@alvherre.pgsql

5 years agoFix certificate subjects in ldap test
Andrew Dunstan [Thu, 8 Aug 2019 18:57:48 +0000 (14:57 -0400)]
Fix certificate subjects in ldap test

openssl doesn't like lower case subject attribute names. Error observed
in buildfarm results.

Backpatch to release 11.

5 years agoDoc: document permissions required for ANALYZE.
Tom Lane [Wed, 7 Aug 2019 22:09:28 +0000 (18:09 -0400)]
Doc: document permissions required for ANALYZE.

VACUUM's reference page had this text, but ANALYZE's didn't.  That's
a clear oversight given that section 5.7 explicitly delegates the
responsibility to define permissions requirements to the individual
commands' man pages.

Per gripe from Isaac Morland.  Back-patch to all supported branches.

Discussion: https://postgr.es/m/CAMsGm5fp3oBUs-2iRfii0iEO=fZuJALVyM2zJLhNTjG34gpAVQ@mail.gmail.com

5 years agoFix some typos in jsonpath documentation
Alexander Korotkov [Wed, 7 Aug 2019 13:06:45 +0000 (16:06 +0300)]
Fix some typos in jsonpath documentation

Discussion: https://postgr.es/m/8B7FA3B4-328D-43D7-95A8-37B8891B8C78%40winand.at
Author: Markus Winand
Backpatch-through: 12

5 years agoFix typos in comments.
Etsuro Fujita [Wed, 7 Aug 2019 10:05:18 +0000 (19:05 +0900)]
Fix typos in comments.

5 years agoFix predicate-locking of HOT updated rows.
Heikki Linnakangas [Wed, 7 Aug 2019 09:40:49 +0000 (12:40 +0300)]
Fix predicate-locking of HOT updated rows.

In serializable mode, heap_hot_search_buffer() incorrectly acquired a
predicate lock on the root tuple, not the returned tuple that satisfied
the visibility checks. As explained in README-SSI, the predicate lock does
not need to be copied or extended to other tuple versions, but for that to
work, the correct, visible, tuple version must be locked in the first
place.

The original SSI commit had this bug in it, but it was fixed back in 2013,
in commit 81fbbfe335. But unfortunately, it was reintroduced a few months
later in commit b89e151054. Wising up from that, add a regression test
to cover this, so that it doesn't get reintroduced again. Also, move the
code that sets 't_self', so that it happens at the same time that the
other HeapTuple fields are set, to make it more clear that all the code in
the loop operate on the "current" tuple in the chain, not the root tuple.

Bug spotted by Andres Freund, analysis and original fix by Thomas Munro,
test case and some additional changes to the fix by Heikki Linnakangas.
Backpatch to all supported versions (9.4).

Discussion: https://www.postgresql.org/message-id/20190731210630.nqhszuktygwftjty%40alap3.anarazel.de

5 years agoFix some incorrect parsing of time with time zone strings
Michael Paquier [Wed, 7 Aug 2019 09:17:34 +0000 (18:17 +0900)]
Fix some incorrect parsing of time with time zone strings

When parsing a timetz string with a dynamic timezone abbreviation or a
timezone not specified, it was possible to generate incorrect timestamps
based on a date which uses some non-initialized variables if the input
string did not specify fully a date to parse.  This is already checked
when a full timezone spec is included in the input string, but the two
other cases mentioned above missed the same checks.

This gets fixed by generating an error as this input is invalid, or in
short when a date is not fully specified.

Valgrind was complaining about this problem.

Bug: #15910
Author: Alexander Lakhin
Discussion: https://postgr.es/m/15910-2eba5106b9aa0c61@postgresql.org
Backpatch-through: 9.4

5 years agoFix intarray's GiST opclasses to not fail for empty arrays with <@.
Tom Lane [Tue, 6 Aug 2019 22:04:51 +0000 (18:04 -0400)]
Fix intarray's GiST opclasses to not fail for empty arrays with <@.

contrib/intarray considers "arraycol <@ constant-array" to be indexable,
but its GiST opclass code fails to reliably find index entries for empty
array values (which of course should trivially match such queries).
This is because the test condition to see whether we should descend
through a non-leaf node is wrong.

Unfortunately, empty array entries could be anywhere in the index,
as these index opclasses are currently designed.  So there's no way
to fix this except by lobotomizing <@ indexscans to scan the whole
index ... which is what this patch does.  That's pretty unfortunate:
the performance is now actually worse than a seqscan, in most cases.
We'd be better off to remove <@ from the GiST opclasses entirely,
and perhaps a future non-back-patchable patch will do so.

In the meantime, applications whose performance is adversely impacted
have a couple of options.  They could switch to a GIN index, which
doesn't have this bug, or they could replace "arraycol <@ constant-array"
with "arraycol <@ constant-array AND arraycol && constant-array".
That will provide about the same performance as before, and it will find
all non-empty subsets of the given constant-array, which is all that
could reliably be expected of the query before.

While at it, add some more regression test cases to improve code
coverage of contrib/intarray.

In passing, adjust resize_intArrayType so that when it's returning an
empty array, it uses construct_empty_array for that rather than
cowboy hacking on the input array.  While the hack produces an array
that looks valid for most purposes, it isn't bitwise equal to empty
arrays produced by other code paths, which could have subtle odd
effects.  I don't think this code path is performance-critical
enough to justify such shortcuts.  (Back-patch this part only as far
as v11; before commit 01783ac36 we were not careful about this in
other intarray code paths either.)

Back-patch the <@ fixes to all supported versions, since this was
broken from day one.

Patch by me; thanks to Alexander Korotkov for review.

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