postgresql.git
4 years agoMake some sanity-check elogs more verbose
Alvaro Herrera [Mon, 23 Nov 2020 16:10:03 +0000 (13:10 -0300)]
Make some sanity-check elogs more verbose

A few sanity checks in funcapi.c were not mentioning all the possible
clauses for failure, confusing developers who fat-fingered catalog data
additions.  Make the errors more detailed to avoid wasting time in
pinpointing mistakes.

Per complaint from Craig Ringer.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAMsr+YH7Kd87A3cU5m_wKo46HPQ46zFv5wesFNL0YWxkGhGv3g@mail.gmail.com

4 years agoFix a few comments that referred to copy.c.
Heikki Linnakangas [Mon, 23 Nov 2020 09:36:13 +0000 (11:36 +0200)]
Fix a few comments that referred to copy.c.

Missed these in the previous commit.

4 years agoSplit copy.c into four files.
Heikki Linnakangas [Mon, 23 Nov 2020 08:50:50 +0000 (10:50 +0200)]
Split copy.c into four files.

Copy.c has grown really large. Split it into more manageable parts:

- copy.c now contains only a few functions that are common to COPY FROM
  and COPY TO.

- copyto.c contains code for COPY TO.

- copyfrom.c contains code for initializing COPY FROM, and inserting the
  tuples to the correct table.

- copyfromparse.c contains code for reading from the client/file/program,
  and parsing the input text/CSV/binary format into tuples.

All of these parts are fairly complicated, and fairly independent of each
other. There is a patch being discussed to implement parallel COPY FROM,
which will add a lot of new code to the COPY FROM path, and another patch
which would allow INSERTs to use the same multi-insert machinery as COPY
FROM, both of which will require refactoring that code. With those two
patches, there's going to be a lot of code churn in copy.c anyway, so now
seems like a good time to do this refactoring.

The CopyStateData struct is also split. All the formatting options, like
FORMAT, QUOTE, ESCAPE, are put in a new CopyFormatOption struct, which
is used by both COPY FROM and TO. Other state data are kept in separate
CopyFromStateData and CopyToStateData structs.

Reviewed-by: Soumyadeep Chakraborty, Erik Rijkers, Vignesh C, Andres Freund
Discussion: https://www.postgresql.org/message-id/8e15b560-f387-7acc-ac90-763986617bfb%40iki.fi

4 years agoAllow a multi-row INSERT to specify DEFAULTs for a generated column.
Tom Lane [Sun, 22 Nov 2020 20:48:32 +0000 (15:48 -0500)]
Allow a multi-row INSERT to specify DEFAULTs for a generated column.

One can say "INSERT INTO tab(generated_col) VALUES (DEFAULT)" and not
draw an error.  But the equivalent case for a multi-row VALUES list
always threw an error, even if one properly said DEFAULT in each row.
Fix that.  While here, improve the test cases for nearby logic about
OVERRIDING SYSTEM/USER values.

Dean Rasheed

Discussion: https://postgr.es/m/9q0sgcr416t.fsf@gmx.us

4 years agoIn geo_ops.c, represent infinite slope as Infinity, not DBL_MAX.
Tom Lane [Sat, 21 Nov 2020 22:24:07 +0000 (17:24 -0500)]
In geo_ops.c, represent infinite slope as Infinity, not DBL_MAX.

Since we're assuming IEEE floats these days, there seems little
reason not to do this.  It has the advantage that when the slope is
computed as infinite due to the presence of Inf coordinates, we get
saner behavior than before from line_construct(), and thence also
in some dependent operations such as finding the closest point.

Also fix line_construct() to special-case slope zero.  The previous
coding got the right answer in most cases, but it could compute
C as NaN when the point has Inf coordinates.

Discussion: https://postgr.es/m/CAGf+fX70rWFOk5cd00uMfa__0yP+vtQg5ck7c2Onb-Yczp0URA@mail.gmail.com

4 years agoFix FPeq() and friends to get the right answers for infinities.
Tom Lane [Sat, 21 Nov 2020 21:46:43 +0000 (16:46 -0500)]
Fix FPeq() and friends to get the right answers for infinities.

"FPeq(infinity, infinity)" returned false, on account of getting NaN
when it subtracts the two inputs.  Fix that by adding a separate
check for exact equality.

FPle() and FPge() similarly got the wrong answer for two like-signed
infinities.  In those cases, we can just rearrange the comparisons
to avoid potentially subtracting infinities.

While the sibling functions FPne() etc accidentally gave the right
answers even with the internal NaN results, it seems best to make
similar adjustments to them to avoid depending on this.

FPeq() has to be converted to an inline function to avoid double
evaluations of its arguments, and I did the same for the others
just for consistency.

In passing, make the handling of NaN cases in line_eq() and
point_eq_point() simpler and easier to reason about, and perhaps
faster.

This results in just one visible regression test change: slope()
now gives DBL_MAX for two inputs of (inf,1e300), which is consistent
with what it does for (1e300,inf), so that seems like a bug fix.

Discussion: https://postgr.es/m/CAGf+fX70rWFOk5cd00uMfa__0yP+vtQg5ck7c2Onb-Yczp0URA@mail.gmail.com

4 years agoExtend the geometric regression test cases a little.
Tom Lane [Sat, 21 Nov 2020 21:34:22 +0000 (16:34 -0500)]
Extend the geometric regression test cases a little.

Add another edge-case value to "point_tbl", and add a test for
the line(point, point) function.

Some of the behaviors exposed here are wrong, but the idea of
committing this separately is to memorialize what we were getting,
and to allow easier inspection of the behavior changes caused by
upcoming patches.

Kyotaro Horiguchi (line() test added by me)

Discussion: https://postgr.es/m/CAGf+fX70rWFOk5cd00uMfa__0yP+vtQg5ck7c2Onb-Yczp0URA@mail.gmail.com

4 years agoRemove INSERT privilege check at table creation of CTAS and matview
Michael Paquier [Sat, 21 Nov 2020 10:45:30 +0000 (19:45 +0900)]
Remove INSERT privilege check at table creation of CTAS and matview

As per discussion with Peter Eisentraunt, the SQL standard specifies
that any tuple insertion done as part of CREATE TABLE AS happens without
any extra ACL check, so it makes little sense to keep a check for INSERT
privileges when using WITH DATA.  Materialized views are not part of the
standard, but similarly, this check can be confusing as this refers to
an access check on a table created within the same command as the one
that would insert data into this table.

This commit removes the INSERT privilege check for WITH DATA, the
default, that 846005e removed partially, but only for WITH NO DATA.

Author: Bharath Rupireddy
Discussion: https://postgr.es/m/d049c272-9a47-d783-46b0-46665b011598@enterprisedb.com

4 years agodoc: Improve tableoid description
Peter Eisentraut [Sat, 21 Nov 2020 07:26:20 +0000 (08:26 +0100)]
doc: Improve tableoid description

Mention that it's useful for determining table names for partitioned
tables as well as for those in inheritance hierarchies.

Author: Ian Lawrence Barwick <barwick@gmail.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAB8KJ=gFmBXP=P9htziOj+WM5PDAK4qc7iGQta+8kUh306kQnw@mail.gmail.com

4 years agoMake pg_rewind test case more stable.
Heikki Linnakangas [Fri, 20 Nov 2020 14:08:58 +0000 (16:08 +0200)]
Make pg_rewind test case more stable.

If replication is exceptionally slow for some reason, pg_rewind might run
before the test row has been replicated. Add an explicit wait for it.

Reported-by: Andres Freund
Discussion: https://www.postgresql.org/message-id/20201120003811.iknhqwatitw2vvxf%40alap3.anarazel.de

4 years agoRemove ability to independently select random number generator
Magnus Hagander [Fri, 20 Nov 2020 11:26:57 +0000 (12:26 +0100)]
Remove ability to independently select random number generator

Remove the ability to select random number generator independently from
SSL library. Instead, use the random number generator from the SSL
library (today only OpenSSL supported) if one is configured. If no SSL
library is configured, use the platform default (which means use
CryptoAPI on Win32 and /dev/urandom on Linux).

This also restructures pg_strong_random.c to have three clearly separate
sections, one for each implementation, with two functions in each,
instead of a scattered set of ifdefs throughout the whole file.

Author: Daniel Gustafsson, Magnus Hagander, Michael Paquier
Discussion: https://postgr.es/m/632623.1605460616@sss.pgh.pa.us

4 years agoReplace a macro by a function
Peter Eisentraut [Fri, 20 Nov 2020 10:25:25 +0000 (11:25 +0100)]
Replace a macro by a function

Using a macro is ugly and not justified here.

Discussion: https://www.postgresql.org/message-id/flat/4ad69a4c-cc9b-0dfe-0352-8b1b0cd36c7b@2ndquadrant.com

4 years agoAdd collation versions for FreeBSD.
Thomas Munro [Fri, 20 Nov 2020 08:49:57 +0000 (21:49 +1300)]
Add collation versions for FreeBSD.

On FreeBSD 13, use querylocale() to read the current version of libc
collations.  Similar to commits 352f6f2d for Windows and d5ac14f9 for
GNU/Linux.

Discussion: https://postgr.es/m/CAEepm%3D0uEQCpfq_%2BLYFBdArCe4Ot98t1aR4eYiYTe%3DyavQygiQ%40mail.gmail.com

4 years agoEmit log when restore_command succeeds but archived file faills to be restored.
Fujii Masao [Fri, 20 Nov 2020 06:42:47 +0000 (15:42 +0900)]
Emit log when restore_command succeeds but archived file faills to be restored.

Previously, when restore_command claimed to succeed but failed to restore
the file with the right name, for example, due to mis-configuration of
restore_command, no log message was reported. Then the recovery failed
later with an error message not directly related to the issue.

This commit changes the recovery so that a log message is emitted in
this error case. This would enable us to investigate what happened in
this case more easily.

Author: Jeff Janes, Fujii Masao
Reviewed-by: Pavel Borisov, Kyotaro Horiguchi
Discussion: https://postgr.es/m/CAMkU=1xkFs3Omp4JR4wMYWdam_KLuj6LXnTYfU8u3T0h=PLLMQ@mail.gmail.com

4 years agoOn macOS, use -isysroot in link steps as well as compile steps.
Tom Lane [Fri, 20 Nov 2020 05:07:09 +0000 (00:07 -0500)]
On macOS, use -isysroot in link steps as well as compile steps.

We previously put the -isysroot switch only into CPPFLAGS, theorizing
that it was only needed to find the right copies of include files.
However, it seems that we also need to use it while linking programs,
to find the right stub ".tbd" files for libraries.  We got away
without that up to now, but apparently that was mostly luck.  It may
also be that failures are only observed when the Xcode version is
noticeably out of sync with the host macOS version; the case that's
prompting action right now is that builds fail when using latest Xcode
(12.2) on macOS Catalina, even though it's fine on Big Sur.

Hence, add -isysroot to LDFLAGS as well.  (It seems that the more
common practice is to put it in CFLAGS, whence it'd be included at
both compile and link steps.  However, we can't mess with CFLAGS in
the platform template file without confusing configure's logic for
choosing default CFLAGS.)

This should be back-patched, but first let's see if the buildfarm
likes it on HEAD.

Report and patch by James Hilliard (some cosmetic mods by me)

Discussion: https://postgr.es/m/20201120003314.20560-1-james.hilliard1@gmail.com

4 years agoRemove undocumented IS [NOT] OF syntax.
Tom Lane [Thu, 19 Nov 2020 22:39:39 +0000 (17:39 -0500)]
Remove undocumented IS [NOT] OF syntax.

This feature was added a long time ago, in 7c1e67bd5 and eb121ba2c,
but never documented in any user-facing way.  (Documentation added
in 6126d3e70 was commented out almost immediately, in 8272fc3f7.)
That's because, while this syntax is defined by SQL:99, our
implementation is only vaguely related to the standard's semantics.
The standard appears to intend a run-time not parse-time test, and
it definitely intends that the test should understand subtype
relationships.

No one has stepped up to fix that in the intervening years, but
people keep coming across the code and asking why it's not documented.
Let's just get rid of it: if anyone ever wants to make it work per
spec, they can easily recover whatever parts of this code are still
of value from our git history.

If there's anyone out there who's actually using this despite its
undocumented status, they can switch to using pg_typeof() instead,
eg. "pg_typeof(something) = 'mytype'::regtype".  That gives
essentially the same semantics as what our IS OF code did.
(We didn't have that function last time this was discussed, or
we would have ripped out IS OF then.)

Discussion: https://postgr.es/m/CAKFQuwZ2pTc-DSkOiTfjauqLYkNREeNZvWmeg12Q-_69D+sYZA@mail.gmail.com
Discussion: https://postgr.es/m/BAY20-F23E9F2B4DAB3E4E88D3623F99B0@phx.gbl
Discussion: https://postgr.es/m/3E7CF81D.1000203@joeconway.com

4 years agoFurther fixes for CREATE TABLE LIKE: cope with self-referential FKs.
Tom Lane [Thu, 19 Nov 2020 20:03:17 +0000 (15:03 -0500)]
Further fixes for CREATE TABLE LIKE: cope with self-referential FKs.

Commit 502898192 was too careless about the order of execution of the
additional ALTER TABLE operations generated by expandTableLikeClause.
It just stuck them all at the end, which seems okay for most purposes.
But it falls down in the case where LIKE is importing a primary key
or unique index and the outer CREATE TABLE includes a FOREIGN KEY
constraint that needs to depend on that index.  Weird as that is,
it used to work, so we ought to keep it working.

To fix, make parse_utilcmd.c insert LIKE clauses between index-creation
and FK-creation commands in the transformed list of commands, and change
utility.c so that the commands generated by expandTableLikeClause are
executed immediately not at the end.  One could imagine scenarios where
this wouldn't work either; but currently expandTableLikeClause only
makes column default expressions, CHECK constraints, and indexes, and
this ordering seems fine for those.

Per bug #16730 from Sofoklis Papasofokli.  Like the previous patch,
back-patch to all supported branches.

Discussion: https://postgr.es/m/16730-b902f7e6e0276b30@postgresql.org

4 years agoRename object in test to avoid conflict
Peter Eisentraut [Thu, 19 Nov 2020 10:34:54 +0000 (11:34 +0100)]
Rename object in test to avoid conflict

In 01e658fa74cb7e3292448f6663b549135958003b, the hash_func test
creates a type t1, but apparently a test running in parallel might
also use that name, depending on timing.  Rename the type to avoid the
issue.

4 years agoHash support for row types
Peter Eisentraut [Thu, 19 Nov 2020 08:24:37 +0000 (09:24 +0100)]
Hash support for row types

Add hash functions for the record type as well as a hash operator
family and operator class for the record type.  This enables all the
hash functionality for the record type such as hash-based plans for
UNION/INTERSECT/EXCEPT DISTINCT, recursive queries using UNION
DISTINCT, hash joins, and hash partitioning.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/38eccd35-4e2d-6767-1b3c-dada1eac3124%402ndquadrant.com

4 years agoAdd BarrierArriveAndDetachExceptLast().
Thomas Munro [Thu, 19 Nov 2020 05:09:50 +0000 (18:09 +1300)]
Add BarrierArriveAndDetachExceptLast().

Provide a way for one process to continue the remaining phases of a
(previously) parallel computation alone.  Later patches will use this to
extend Parallel Hash Join.

Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKG%2BA6ftXPz4oe92%2Bx8Er%2BxpGZqto70-Q_ERwRaSyA%3DafNg%40mail.gmail.com

4 years agoImprove failure detection with array parsing in pg_dump
Michael Paquier [Thu, 19 Nov 2020 01:36:08 +0000 (10:36 +0900)]
Improve failure detection with array parsing in pg_dump

Similarly to 3636efa, the checks done in pg_dump when parsing array
values from catalogs have been too lax.  Under memory pressure, it could
be possible, though very unlikely, to finish with dumps that miss some
data like:
- Statistics for indexes
- Run-time configuration of functions
- Configuration of extensions
- Publication list for a subscription

No backpatch is done as this is not going to be a problem in practice.
For example, if an OOM causes an array parsing to fail, a follow-up code
path of pg_dump would most likely complain with an allocation failure
due to the memory pressure.

Author: Michael Paquier
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/20201111061319.GE2276@paquier.xyz

4 years agoRelax lock level for setting PGPROC->statusFlags
Alvaro Herrera [Wed, 18 Nov 2020 16:24:22 +0000 (13:24 -0300)]
Relax lock level for setting PGPROC->statusFlags

We don't actually need a lock to set PGPROC->statusFlags itself; what we
do need is a shared lock on either XidGenLock or ProcArrayLock in order to
ensure MyProc->pgxactoff keeps still while we modify the mirror array in
ProcGlobal->statusFlags.  Some places were using an exclusive lock for
that, which is excessive.  Relax those to use shared lock only.

procarray.c has a couple of places with somewhat brittle assumptions
about PGPROC changes: ProcArrayEndTransaction uses only shared lock, so
it's permissible to change MyProc only.  On the other hand,
ProcArrayEndTransactionInternal also changes other procs, so it must
hold exclusive lock.  Add asserts to ensure those assumptions continue
to hold.

Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/20201117155501.GA13805@alvherre.pgsql

4 years agoSkip allocating hash table in EXPLAIN-only mode.
Heikki Linnakangas [Wed, 18 Nov 2020 10:39:15 +0000 (12:39 +0200)]
Skip allocating hash table in EXPLAIN-only mode.

Author: Alexey Bashtanov
Discussion: https://www.postgresql.org/message-id/36823f65-050d-ae24-aa4d-a37726998240%40imap.cc

4 years agodocs: Clarify that signal functions have no feedback.
Heikki Linnakangas [Wed, 18 Nov 2020 08:28:50 +0000 (10:28 +0200)]
docs: Clarify that signal functions have no feedback.

Bug #16652 complains that pg_reload_conf() returned true, even though
the configuration file contained errors. That's the way pg_reload_conf()
works, by design, but the documentation wasn't very clear on it. Clarify
that a 'true' return value only means that the signal was sent
successfully. Also add links to the system views that can be used to
check the configuration files for errors.

David G. Johnston, with some rewording by me.

Discussion: https://www.postgresql.org/message-id/CAKFQuwax6GxhUQEes0D045UtXG-fBraM39_6UMd5JyR5K1HWCQ%40mail.gmail.com

4 years agoAdd more tests for hashing and hash-based plans
Peter Eisentraut [Wed, 18 Nov 2020 06:58:37 +0000 (07:58 +0100)]
Add more tests for hashing and hash-based plans

- Test hashing of an array of a non-hashable element type.

- Test UNION [DISTINCT] with hash- and sort-based plans.  (Previously,
  only INTERSECT and EXCEPT where tested there.)

- Test UNION [DISTINCT] with a non-hashable column type.  This
  currently reverts to a sort-based plan even if enable_hashagg is on.

- Test UNION/INTERSECT/EXCEPT hash- and sort-based plans with arrays
  as column types.  Also test an array with a non-hashable element
  type.

- Test UNION/INTERSECT/EXCEPT similarly with row types as column
  types.  Currently, this uses only sort-based plans because there is
  no hashing support for row types.

- Add a test case that shows that recursive queries using UNION
  [DISTINCT] require hashable column types.

- Add a currently failing test that uses UNION DISTINCT in a
  cycle-detection use case using row types as column types.

Discussion: https://www.postgresql.org/message-id/flat/38eccd35-4e2d-6767-1b3c-dada1eac3124%402ndquadrant.com

4 years agoAdd tab completion for CREATE [OR REPLACE] TRIGGER in psql
Michael Paquier [Wed, 18 Nov 2020 05:01:53 +0000 (14:01 +0900)]
Add tab completion for CREATE [OR REPLACE] TRIGGER in psql

92bf7e2 has added support for this grammar.

Author: Noriyoshi Shinoda
Discussion: https://postgr.es/m/TU4PR8401MB115244623CF4724DCA0D507FEEE30@TU4PR8401MB1152.NAMPRD84.PROD.OUTLOOK.COM

4 years agoDeprecate nbtree's BTP_HAS_GARBAGE flag.
Peter Geoghegan [Tue, 17 Nov 2020 17:45:56 +0000 (09:45 -0800)]
Deprecate nbtree's BTP_HAS_GARBAGE flag.

Streamline handling of the various strategies that we have to avoid a
page split in nbtinsert.c.  When it looks like a leaf page is about to
overflow, we now perform deleting LP_DEAD items and deduplication in one
central place.  This greatly simplifies _bt_findinsertloc().

This has an independently useful consequence: nbtree no longer relies on
the BTP_HAS_GARBAGE page level flag/hint for anything important.  We
still set and unset the flag in the same way as before, but it's no
longer treated as a gating condition when considering if we should check
for already-set LP_DEAD bits.  This happens at the point where the page
looks like it might have to be split anyway, so simply checking the
LP_DEAD bits in passing is practically free.  This avoids missing
LP_DEAD bits just because the page-level hint is unset, which is
probably reasonably common (e.g. it happens when VACUUM unsets the
page-level flag without actually removing index tuples whose LP_DEAD-bit
was set recently, after the VACUUM operation began but before it reached
the leaf page in question).

Note that this isn't a big behavioral change compared to PostgreSQL 13.
We were already checking for set LP_DEAD bits regardless of whether the
BTP_HAS_GARBAGE page level flag was set before we considered doing a
deduplication pass.  This commit only goes slightly further by doing the
same check for all indexes, even indexes where deduplication won't be
performed.

We don't completely remove the BTP_HAS_GARBAGE flag.  We still rely on
it as a gating condition with pg_upgrade'd indexes from before B-tree
version 4/PostgreSQL 12.  That makes sense because we sometimes have to
make a choice among pages full of duplicates when inserting a tuple with
pre version 4 indexes.  It probably still pays to avoid accessing the
line pointer array of a page there, since it won't yet be clear whether
we'll insert on to the page in question at all, let alone split it as a
result.

Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Victor Yegorov <vyegorov@gmail.com>
Discussion: https://postgr.es/m/CAH2-Wz%3DYpc1PDdk8OVJDChGJBjT06%3DA0Mbv9HyTLCsOknGcUFg%40mail.gmail.com

4 years agoindexcmds.c: reorder function prototypes
Alvaro Herrera [Tue, 17 Nov 2020 14:37:36 +0000 (11:37 -0300)]
indexcmds.c: reorder function prototypes

... out of an overabundance of neatnikism, perhaps.

4 years agonbtree: Rename nbtinsert.c variables for consistency.
Peter Geoghegan [Tue, 17 Nov 2020 17:01:14 +0000 (09:01 -0800)]
nbtree: Rename nbtinsert.c variables for consistency.

Stop naming special area/opaque pointer variables 'lpageop' in contexts
where it doesn't make sense.  This is a holdover from a time when logic
that performs tasks that are now spread across _bt_insertonpg(),
_bt_findinsertloc(), and _bt_split() was more centralized.  'lpageop'
denotes "left page", which doesn't make sense outside of contexts in
which there isn't also a right page.

Also acquire page flag variables up front within _bt_insertonpg().  This
makes it closer to _bt_split() following refactoring commit bc3087b626d.
This allows the page split and retail insert paths to both make use of
the same variables.

4 years agoFix 'skip-empty-xacts' option in test_decoding for streaming mode.
Amit Kapila [Tue, 17 Nov 2020 06:44:53 +0000 (12:14 +0530)]
Fix 'skip-empty-xacts' option in test_decoding for streaming mode.

In streaming mode, the transaction can be decoded in multiple streams and
those streams can be interleaved with streams of other transactions. So,
we can't remember the transaction's write status in the logical decoding
context because that might get changed due to some other transactions and
lead to wrong answers for 'skip-empty-xacts' option. We decided to keep
each transaction's write status in the ReorderBufferTxn to avoid
interleaved streams changing the status of some unrelated transactions.

Diagnosed-by: Amit Kapila
Author: Dilip Kumar
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CAA4eK1LR7=XNM_TLmpZMFuV8ZQpoxkem--NZJYf8YXmesbvwLA@mail.gmail.com

4 years agoDon't Insert() a VFD entry until it's fully built.
Tom Lane [Tue, 17 Nov 2020 01:32:35 +0000 (20:32 -0500)]
Don't Insert() a VFD entry until it's fully built.

Otherwise, if FDDEBUG is enabled, the debugging output fails because
it tries to read the fileName, which isn't set up yet (and should in
fact always be NULL).

AFAICT, this has been wrong since Berkeley.  Before 96bf88d52,
it would accidentally fail to crash on platforms where snprintf()
is forgiving about being passed a NULL pointer for %s; but the
file name intended to be included in the debug output wouldn't
ever have shown up.

Report and fix by Greg Nancarrow.  Although this is only visibly
broken in custom-made builds, it still seems worth back-patching
to all supported branches, as the FDDEBUG code is pretty useless
as it stands.

Discussion: https://postgr.es/m/CAJcOf-cUDgm9qYtC_B6XrC6MktMPNRby2p61EtSGZKnfotMArw@mail.gmail.com

4 years agoRename PGPROC->vacuumFlags to statusFlags
Alvaro Herrera [Mon, 16 Nov 2020 22:42:55 +0000 (19:42 -0300)]
Rename PGPROC->vacuumFlags to statusFlags

With more flags associated to a PGPROC entry that are not related to
vacuum (currently existing or planned), the name "statusFlags" describes
its purpose better.

(The same is done to the mirroring PROC_HDR->vacuumFlags.)

No functional changes in this commit.

This was suggested first by Hari Babu Kommi in [1] and then by Michael
Paquier at [2].

[1] https://postgr.es/m/CAJrrPGcsDC-oy1AhqH0JkXYa0Z2AgbuXzHPpByLoBGMxfOZMEQ@mail.gmail.com
[2] https://postgr.es/m/20200820060929.GB3730@paquier.xyz

Author: Dmitry Dolgov <9erthalion6@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/20201116182446.qcg3o6szo2zookyr@localhost

4 years agoDo not return NULL for error cases in satisfies_hash_partition().
Tom Lane [Mon, 16 Nov 2020 21:39:59 +0000 (16:39 -0500)]
Do not return NULL for error cases in satisfies_hash_partition().

Since this function is used as a CHECK constraint condition,
returning NULL is tantamount to returning TRUE, which would have the
effect of letting in a row that doesn't satisfy the hash condition.
Admittedly, the cases for which this is done should be unreachable
in practice, but that doesn't make it any less a bad idea.  It also
seems like a dartboard was used to decide which error cases should
throw errors as opposed to returning NULL.

For the checks for NULL input values, I just switched it to returning
false.  There's some argument that an error would be better; but the
case really should be can't-happen in a generated hash constraint,
so it's likely not worth more code for.

For the parent-relation-open-failure case, it seems like we might
as well let relation_open throw an error, instead of having an
impossible-to-diagnose constraint failure.

Back-patch to v11 where this code came in.

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

4 years agoUse "true" not "TRUE" in one ICU function call.
Tom Lane [Mon, 16 Nov 2020 20:16:39 +0000 (15:16 -0500)]
Use "true" not "TRUE" in one ICU function call.

This was evidently missed in commit 6337865f3, which generally did
s/TRUE/true/ everywhere.  It escaped notice up to now because ICU
versions before ICU 68 provided definitions of "TRUE" and "FALSE"
regardless.  With ICU 68, it fails to compile.

Per report from Condor.  Back-patch to v11 where 6337865f3 came in.
(I've not tested v10, where this call originated, but I imagine
it's fine since we defined TRUE in c.h back then.)

Discussion: https://postgr.es/m/7a6f3336165bfe3ca66abcda7966f9d0@stz-bg.com

4 years agodoc: update bgwriter description
Bruce Momjian [Mon, 16 Nov 2020 18:13:43 +0000 (13:13 -0500)]
doc:  update bgwriter description

This clarifies exactly what the bgwriter does, which should help with
tuning.

Reported-by: Chris Wilson
Discussion: https://postgr.es/m/160399562040.7809.7335281028960123489@wrigleys.postgresql.org

Backpatch-through: 9.5

4 years agodoc: clarify how to find pg_type_d.h in the install tree
Bruce Momjian [Mon, 16 Nov 2020 17:36:17 +0000 (12:36 -0500)]
doc:  clarify how to find pg_type_d.h in the install tree

Followup to patch 152ed04799.

Reported-by: Alvaro Herrera
Discussion: https://postgr.es/m/20201112202900.GA28098@alvherre.pgsql

Backpatch-through: 9.5

4 years agoRemove unused and deprecated strategy numbers from BRIN code
Peter Eisentraut [Mon, 16 Nov 2020 16:25:41 +0000 (17:25 +0100)]
Remove unused and deprecated strategy numbers from BRIN code

These were dead code.

Discussion: https://www.postgresql.org/message-id/flat/20201027032511.GF9241@telsasoft.com

4 years agodoc: improve wording of the need for analyze of exp. indexes
Bruce Momjian [Mon, 16 Nov 2020 15:26:17 +0000 (10:26 -0500)]
doc:  improve wording of the need for analyze of exp. indexes

This is a followup commit on 3370207986.

Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/20201112211143.GL30691@telsasoft.com

Backpatch-through: 9.5

4 years agoFix typo
Alvaro Herrera [Mon, 16 Nov 2020 13:54:11 +0000 (10:54 -0300)]
Fix typo

Introduced in 90fdc259866e; backpatch to 12.

Author: Erik Rijkers <er@xs4all.nl>
Discussion: https://postgr.es/m/e92b3fba98a0c0f7afc0a2a37e765954@xs4all.nl

4 years agoNormalize comment in empty grammar rules
Peter Eisentraut [Mon, 16 Nov 2020 10:54:52 +0000 (11:54 +0100)]
Normalize comment in empty grammar rules

Change lower case /* empty */ to /* EMPTY */ for consistency with the
majority.

Discussion: https://www.postgresql.org/message-id/flat/e9eed669-e32d-6919-fed4-acc0daea857b%40enterprisedb.com

4 years agoRemove code handling removed deprecated containment operators
Peter Eisentraut [Mon, 16 Nov 2020 10:45:00 +0000 (11:45 +0100)]
Remove code handling removed deprecated containment operators

This removes the code that was there for handling the operators
removed by 2f70fdb0644c32c4154236c2b5c241bec92eac5e.

Discussion: https://www.postgresql.org/message-id/flat/20201027032511.GF9241@telsasoft.com

4 years agoMake the standby server promptly handle interrupt signals.
Fujii Masao [Mon, 16 Nov 2020 09:27:51 +0000 (18:27 +0900)]
Make the standby server promptly handle interrupt signals.

This commit changes the startup process in the standby server so that
it handles the interrupt signals after waiting for wal_retrieve_retry_interval
on the latch and resetting it, before entering another wait on the latch.
This change causes the standby server to promptly handle interrupt signals.

Otherwise, previously, there was the case where the standby needs to
wait extra five seconds to shutdown when the shutdown request arrived
while the startup process was waiting for wal_retrieve_retry_interval
on the latch.

Author: Fujii Masao, but implementation idea is from Soumyadeep Chakraborty
Reviewed-by: Soumyadeep Chakraborty
Discussion: https://postgr.es/m/9d7e6ab0-8a53-ddb9-63cd-289bcb25fe0e@oss.nttdata.com

4 years agoReword 'simple comparison' => 'inequality' in pgtrgm.sgml
Alexander Korotkov [Mon, 16 Nov 2020 06:09:07 +0000 (09:09 +0300)]
Reword 'simple comparison' => 'inequality' in pgtrgm.sgml

Discussion: https://postgr.es/m/CAMkU%3D1zxfA8_MGBW6sJMj54p8nPoe4bMb5LoG-rMYZVPq4j08Q%40mail.gmail.com
Author: Jeff Janes

4 years agoRelax INSERT privilege requirement for CTAS and matviews WITH NO DATA
Michael Paquier [Mon, 16 Nov 2020 02:52:40 +0000 (11:52 +0900)]
Relax INSERT privilege requirement for CTAS and matviews WITH NO DATA

When specified, WITH NO DATA does not insert any data into the relation
created, so skip checking for the insert permissions.  With WITH DATA or
WITH NO DATA, it is always required for the user to have CREATE
privileges on the schema targeted for the relation.

Note that plain CREATE TABLE AS or CREATE MATERIALIZED VIEW queries have
begun to work accidentally without INSERT privilege checks as of
874fe3ae, while using EXECUTE or EXPLAIN ANALYZE would fail with the ACL
check, so this makes the behavior for all the command flavors consistent
with each other.  This is arguably a bug fix, but there have been no
complaints about the current behavior either so stable branches are not
changed.

While on it, document properly the privileges requirements for each
commands with more tests for all the scenarios possible, and avoid a
useless bulk-insert allocation when using WITH NO DATA.

Author: Bharath Rupireddy
Reviewed-by: Anastasia Lubennikova, Michael Paquier
Discussion: https://postgr.es/m/CALj2ACWc3N8j0_9nMPz9wcAUnVcdKHzFdDZJ3hVFNEbqtcyG9w@mail.gmail.com

4 years agoFix fuzzy thinking about amcanmulticol versus amcaninclude.
Tom Lane [Sun, 15 Nov 2020 21:10:48 +0000 (16:10 -0500)]
Fix fuzzy thinking about amcanmulticol versus amcaninclude.

These flags should be independent: in particular an index AM should
be able to say that it supports include columns without necessarily
supporting multiple key columns.  The included-columns patch got
this wrong, possibly aided by the fact that it didn't bother to
update the documentation.

While here, clarify some text about amcanreturn, which was a little
vague about what should happen when amcanreturn reports that only
some of the index columns are returnable.

Noted while reviewing the SP-GiST included-columns patch, which
quite incorrectly (and unsafely) changed SP-GiST to claim
amcanmulticol = true as a workaround for this bug.

Backpatch to v11 where included columns were introduced.

4 years agonbtree: Demote incomplete split "can't happen" error.
Peter Geoghegan [Sun, 15 Nov 2020 19:53:37 +0000 (11:53 -0800)]
nbtree: Demote incomplete split "can't happen" error.

Only a basic logic bug in a _bt_insertonpg() caller could lead to a
violation of this invariant (index corruption won't do it).  A "can't
happen" error seems inappropriate (it is arbitrary at best).

Demote the error to a simple assertion.  This matches similar nearby
sanity checks.

4 years agoSuppress "warning: variable 'collcollate' set but not used".
Tom Lane [Sun, 15 Nov 2020 17:39:49 +0000 (12:39 -0500)]
Suppress "warning: variable 'collcollate' set but not used".

Buildfarm members that lack both HAVE_LOCALE_T and USE_ICU have been
complaining about pg_newlocale_from_collation's collcollate variable.
This is evidently fallout from commit 7d1297df0, which removed the
only usage outside those two #ifdef'd code paths.  Mark the variable
pg_attribute_unused(), like its sibling collctype, which has been that
way for a long time.

4 years agoFix timing issue in pg_rewind test.
Heikki Linnakangas [Sun, 15 Nov 2020 15:09:31 +0000 (17:09 +0200)]
Fix timing issue in pg_rewind test.

The test inserts a row in primary server, waits until the insertion has
been replicated to a cascaded standby, and checks that it's visible there
by querying the cascaded standby. In order for that to work reliably, the
test needs to wait until the insertion WAL record has been fully replayed.

This should fix the occasional buildfarm failures. Diagnosis by Tom Lane.

Discussion: https://www.postgresql.org/message-id/606796.1605424022@sss.pgh.pa.us

4 years agoAdd missing 'the' to pgtrgm.sgml
Alexander Korotkov [Sun, 15 Nov 2020 11:01:22 +0000 (14:01 +0300)]
Add missing 'the' to pgtrgm.sgml

Author: Erik Rijkers
Discussion: https://postgr.es/m/159fba265fe7c37334640fdc0444cc4b%40xs4all.nl

4 years agoFix alphabetic ordering in typedefs.list
Alexander Korotkov [Sun, 15 Nov 2020 05:56:31 +0000 (08:56 +0300)]
Fix alphabetic ordering in typedefs.list

4 years agoHandle equality operator in contrib/pg_trgm
Alexander Korotkov [Sun, 15 Nov 2020 05:52:12 +0000 (08:52 +0300)]
Handle equality operator in contrib/pg_trgm

Obviously, in order to equality operator be satisfiable, target string must
contain all the trigrams of the search string.  On this base, we implement
equality operator in GiST/GIN indexes with recheck.

Discussion: https://postgr.es/m/CAOBaU_YWwtT7tdggtROacjdOdeYHCz-tmSwuC-j-TOG-g97J0w%40mail.gmail.com
Author: Julien Rouhaud
Reviewed-by: Tom Lane, Alexander Korotkov, Georgios Kokolatos, Erik Rijkers
4 years agoProvide the OR REPLACE option for CREATE TRIGGER.
Tom Lane [Sat, 14 Nov 2020 22:05:34 +0000 (17:05 -0500)]
Provide the OR REPLACE option for CREATE TRIGGER.

This is mostly straightforward.  However, we disallow replacing
constraint triggers or changing the is-constraint property; perhaps
that can be added later, but the complexity versus benefit tradeoff
doesn't look very good.

Also, no special thought is taken here for whether replacing an
existing trigger should result in changes to queued-but-not-fired
trigger actions.  We just document that if you're surprised by the
results, too bad, don't do that.  (Note that any such pending trigger
activity would have to be within the current session.)

Takamichi Osumi, reviewed at various times by Surafel Temesgen,
Peter Smith, and myself

Discussion: https://postgr.es/m/0DDF369B45A1B44B8A687ED43F06557C010BC362@G01JPEXMBYT03

4 years agoDoc: improve partitioning discussion in ddl.sgml.
Tom Lane [Sat, 14 Nov 2020 18:09:53 +0000 (13:09 -0500)]
Doc: improve partitioning discussion in ddl.sgml.

This started with the intent to explain that range upper bounds
are exclusive, which previously you could only find out by reading
the CREATE TABLE man page.  But I soon found that section 5.11
really could stand a fair amount of editorial attention.  It's
apparently been revised several times without much concern for
overall flow, nor careful copy-editing.

Back-patch to v11, which is as far as the patch goes easily.

Per gripe from Edson Richter.  Thanks to David Johnston for review.

Discussion: https://postgr.es/m/DM6PR13MB3988736CF8F5DC5720440231CFE60@DM6PR13MB3988.namprd13.prod.outlook.com

4 years agoFix some typos
Michael Paquier [Sat, 14 Nov 2020 02:43:10 +0000 (11:43 +0900)]
Fix some typos

Author: Daniel Gustafsson
Discussion: https://postgr.es/m/C36ADFDF-D09A-4EE5-B186-CB46C3653F4C@yesql.se

4 years agoSupport negative indexes in split_part().
Tom Lane [Fri, 13 Nov 2020 18:49:48 +0000 (13:49 -0500)]
Support negative indexes in split_part().

This provides a handy way to get, say, the last field of the string.
Use of a negative index in this way has precedent in the nearby
left() and right() functions.

The implementation scans the string twice when N < -1, but it seems
likely that N = -1 will be the huge majority of actual use cases,
so I'm not really excited about adding complexity to avoid that.

Nikhil Benesch, reviewed by Jacob Champion; cosmetic tweakage by me

Discussion: https://postgr.es/m/cbb7f861-6162-3a51-9823-97bc3aa0b638@gmail.com

4 years agoRemove another test that doesn't work on Windows.
Heikki Linnakangas [Fri, 13 Nov 2020 11:41:58 +0000 (13:41 +0200)]
Remove another test that doesn't work on Windows.

Apparently double-quotes are not allowed in filenames on Windows, either.

Per buildfarm.

4 years agodoc: clarify where to find pg_type_d.h (PG 11+) and pg_type.h
Bruce Momjian [Thu, 12 Nov 2020 20:13:02 +0000 (15:13 -0500)]
doc:  clarify where to find pg_type_d.h (PG 11+) and pg_type.h

These files are in compiled directories and install directories.

Reported-by: e.indrupskaya@postgrespro.ru
Discussion: https://postgr.es/m/160379609706.24746.7506163279454026608@wrigleys.postgresql.org

Backpatch-through: 9.5

4 years agodocs: mention that expression indexes need analyze
Bruce Momjian [Thu, 12 Nov 2020 20:00:44 +0000 (15:00 -0500)]
docs: mention that expression indexes need analyze

Expression indexes can't benefit from pre-computed statistics on
columns.

Reported-by: Nikolay Samokhvalov
Discussion: https://postgr.es/m/CANNMO++5rw9RDA=p40iMVbMNPaW6O=S0AFzTU=KpYHRpCd1voA@mail.gmail.com

Author: Nikolay Samokhvalov, modified

Backpatch-through: 9.5

4 years agochange wire protocol data type for history file content
Bruce Momjian [Thu, 12 Nov 2020 19:08:59 +0000 (14:08 -0500)]
change wire protocol data type for history file content

This was marked as BYTEA, but is more like TEXT, which is how we already
pass the history timeline file name.  Internally, we don't do any
encoding or bytea escape handling, but TEXT seems closest.  This should
cause no behavioral change.

Reported-by: Brar Piening
Discussion: https://postgr.es/m/6a1b9cd9-17e3-df67-be55-86102af6bdf5@gmx.de

Backpatch-through: master

4 years agoRemove tests that don't work on Windows.
Heikki Linnakangas [Thu, 12 Nov 2020 17:18:34 +0000 (19:18 +0200)]
Remove tests that don't work on Windows.

On Windows, a filename cannot contain backslashes, because a backslash
is used directory separator. Remove tests I added in commit 9c4f5192f
that tried to do that. We could perhaps use a SKIP block to only skip
them on Windows, but I'm not sure how exactly to formulate that, so just
remove the tests to make the buildfarm green again.

Per buildfarm.

4 years agopg_trgm: fix crash in 2-item picksplit
Andrew Gierth [Thu, 12 Nov 2020 14:34:37 +0000 (14:34 +0000)]
pg_trgm: fix crash in 2-item picksplit

Whether from size overflow in gistSplit or from secondary splits,
picksplit is (rarely) called with exactly two items to split.

Formerly, due to special-case handling of the last item, this would
lead to access to an uninitialized cache entry; prior to PG 13 this
might have been harmless or at worst led to an incorrect union datum,
but in 13 onwards it can cause a backend crash from using an
uninitialized pointer.

Repair by removing the special case, which was deemed not to have been
appropriate anyway. Backpatch all the way, because this bug has
existed since pg_trgm was added.

Per report on IRC from user "ftzdomino". Analysis and testing by me,
patch from Alexander Korotkov.

Discussion: https://postgr.es/m/87k0usfdxg.fsf@news-spur.riddles.org.uk

4 years agoAllow pg_rewind to use a standby server as the source system.
Heikki Linnakangas [Thu, 12 Nov 2020 12:52:24 +0000 (14:52 +0200)]
Allow pg_rewind to use a standby server as the source system.

Using a hot standby server as the source has not been possible, because
pg_rewind creates a temporary table in the source system, to hold the
list of file ranges that need to be fetched. Refactor it to queue up the
file fetch requests in pg_rewind's memory, so that the temporary table
is no longer needed.

Also update the logic to compute 'minRecoveryPoint' correctly, when the
source is a standby server.

Reviewed-by: Kyotaro Horiguchi, Soumyadeep Chakraborty
Discussion: https://www.postgresql.org/message-id/0c5b3783-af52-3ee5-f8fa-6e794061f70d%40iki.fi

4 years agoClean up optional rules in grammar
Peter Eisentraut [Thu, 12 Nov 2020 06:48:36 +0000 (07:48 +0100)]
Clean up optional rules in grammar

Various rules for optional keywords contained unnecessary rules and
type declarations.  Remove those, thus making the output a tiny bit
smaller.

Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://www.postgresql.org/message-id/flat/e9eed669-e32d-6919-fed4-acc0daea857b%40enterprisedb.com

4 years agoFix typo in contrib/pg_trgm/pg_trgm--1.4--1.5.sql
Alexander Korotkov [Thu, 12 Nov 2020 05:55:09 +0000 (08:55 +0300)]
Fix typo in contrib/pg_trgm/pg_trgm--1.4--1.5.sql

Backpatch-through: 13

4 years agoUse standard SIGHUP and SIGTERM handlers in walreceiver.
Fujii Masao [Thu, 12 Nov 2020 04:25:23 +0000 (13:25 +0900)]
Use standard SIGHUP and SIGTERM handlers in walreceiver.

Commit 1e53fe0e70 changed background processes so that they use
standard SIGHUP handler. Like that, this commit makes walreceiver
use standard SIGHUP and SIGTERM handlers, to simplify the code.

As the side effect of this commit, walreceiver can wake up and process
the configuration files promptly when receiving SIGHUP. Because the
standard SIGHUP handler sets the latch. On the other hand, previously
there could be a time lag between the receipt of SIGHUP and
the process of configuration files since the dedicated handler didn't
set the latch.

Author: Bharath Rupireddy, tweaked by Fujii Masao
Reviewed-by: Kyotaro Horiguchi, Fujii Masao
Discussion: https://postgr.es/m/CALj2ACXPorUqePswDtOeM_s82v9RW32E1fYmOPZ5NuE+TWKj_A@mail.gmail.com

4 years agoFix name of the macro for getting signature length trgm_gist.c
Alexander Korotkov [Thu, 12 Nov 2020 03:19:16 +0000 (06:19 +0300)]
Fix name of the macro for getting signature length trgm_gist.c

911e702077 has introduced the opclass parameters including signature length
for a set of GiST opclasses.  Due to copy-pasting, macro for getting the
signature length in trgm_gist.c was named LTREE_GET_ASIGLEN().  Fix that by
renaming this macro to just GET_SIGLEN().

Backpatch-through: 13

4 years agopg_stat_statements: track number of rows processed by REFRESH MATERIALIZED VIEW.
Fujii Masao [Thu, 12 Nov 2020 02:26:55 +0000 (11:26 +0900)]
pg_stat_statements: track number of rows processed by REFRESH MATERIALIZED VIEW.

Commit 6023b7ea71 allowed pg_stat_statements to track the number
of rows retrieved or affected by some utility commands including
CREATE MATERIALIZED VIEW. However it did not track the rowcount
of REFRESH MATERIALIZED VIEW. This commit allows pg_stat_statements
to track that.

To track that, this commit changes the query completion for
REFRESH MATERIALIZED VIEW so that it saves the rowcount. But note that
the rowcount is still not displayed in the command completion tag output.
That is, the display_rowcount flag of CMDTAG_REFRESH_MATERIALIZED_VIEW
command tag is left false in cmdtaglist.h. Otherwise, the change of
completion tag output might break applications using it.

Author: Katsuragi Yuta, Seino Yuki
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/71f6bc72f8bbaa06e701f8bd2562c347@oss.nttdata.com
Discussion: https://postgr.es/m/aadbfba9-e4bb-9531-6b3a-d13c31c8f4fe@oss.nttdata.com

4 years agoRemove useless SHA256 initialization when not using backup manifests
Michael Paquier [Thu, 12 Nov 2020 01:56:33 +0000 (10:56 +0900)]
Remove useless SHA256 initialization when not using backup manifests

Attempting to take a base backup with Postgres linking to a build of
OpenSSL with FIPS enabled currently fails with or even without a backup
manifest requested because of this mandatory SHA256 initialization used
for the manifest file itself.  However, there is no need to do this
initialization at all if backup manifests are not needed because there
is no data to append to the manifest.

Note that being able to use backup manifests with OpenSSL+FIPS requires
a switch of the SHA2 implementation to use EVP, which would cause an ABI
breakage so this cannot be backpatched to 13 as it has been already
released, but at least avoiding this SHA256 initialization gives users
the possibility to take a base backup even when specifying --no-manifest
with pg_basebackup.

Author: Michael Paquier
Discussion: https://postgr.es/m/20201110020014.GE1887@paquier.xyz
Backpatch-through: 13

4 years agoRemove duplicate code in brin_memtuple_initialize
Tomas Vondra [Wed, 11 Nov 2020 17:37:36 +0000 (18:37 +0100)]
Remove duplicate code in brin_memtuple_initialize

Commit 8bf74967dab moved some of the code from brin_new_memtuple to
brin_memtuple_initialize, but this resulted in some of the code being
duplicate. Fix by removing the duplicate lines and backpatch to 10.

Author: Tomas Vondra
Backpatch-through: 10
Discussion: https://postgr.es/m/5eb50c97-9a8e-b691-8c40-1b2a55611c4c%40enterprisedb.com

4 years agoFix some stray whitespace in parser files
Peter Eisentraut [Wed, 11 Nov 2020 16:37:18 +0000 (17:37 +0100)]
Fix some stray whitespace in parser files

4 years agoRemove line missed in previous commit
Magnus Hagander [Wed, 11 Nov 2020 15:51:36 +0000 (16:51 +0100)]
Remove line missed in previous commit

4 years agoRemove vacuumdb --analyze-in-stages from pg_upgrade tests
Magnus Hagander [Wed, 11 Nov 2020 15:47:13 +0000 (16:47 +0100)]
Remove vacuumdb --analyze-in-stages from pg_upgrade tests

This step was only there to test the script when we generated those, but
commit 8f113698b6 removed those scripts, so it's not needed anymore.

Reported-By: Peter Eisentraut
Discussion: https://postgr.es/m/ea403f46-2b33-a7de-618e-9cab35a698c8@enterprisedb.com

4 years agoAdd pg_nodiscard decorations to some functions
Peter Eisentraut [Sat, 17 Oct 2020 06:38:39 +0000 (08:38 +0200)]
Add pg_nodiscard decorations to some functions

Especially for the list API such as lappend() forgetting to assign the
return value is a common problem.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/e3753562-99cd-b65f-5aca-687dfd1ec2fc@2ndquadrant.com

4 years agoAdd pg_nodiscard function declaration specifier
Peter Eisentraut [Sat, 17 Oct 2020 06:38:39 +0000 (08:38 +0200)]
Add pg_nodiscard function declaration specifier

pg_nodiscard means the compiler should warn if the result of a
function call is ignored.  The name "nodiscard" is chosen in alignment
with (possibly future) C and C++ standards.  It maps to the GCC
attribute warn_unused_result.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/e3753562-99cd-b65f-5aca-687dfd1ec2fc@2ndquadrant.com

4 years agoFix cases of discarding result from list API functions
Peter Eisentraut [Sat, 17 Oct 2020 06:38:39 +0000 (08:38 +0200)]
Fix cases of discarding result from list API functions

Two cases violated list APIs by throwing away the return value.  While
the code was technically correct, it relied on internal knowledge of
the list implementation, and the code wasn't really gaining anything
that way.  It is planned to make this a compiler warning in the
future, so just fix these cases by assigning the return value
properly.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/e3753562-99cd-b65f-5aca-687dfd1ec2fc@2ndquadrant.com

4 years agoFix and simplify some usages of TimestampDifference().
Tom Lane [Wed, 11 Nov 2020 03:51:18 +0000 (22:51 -0500)]
Fix and simplify some usages of TimestampDifference().

Introduce TimestampDifferenceMilliseconds() to simplify callers
that would rather have the difference in milliseconds, instead of
the select()-oriented seconds-and-microseconds format.  This gets
rid of at least one integer division per call, and it eliminates
some apparently-easy-to-mess-up arithmetic.

Two of these call sites were in fact wrong:

* pg_prewarm's autoprewarm_main() forgot to multiply the seconds
by 1000, thus ending up with a delay 1000X shorter than intended.
That doesn't quite make it a busy-wait, but close.

* postgres_fdw's pgfdw_get_cleanup_result() thought it needed to compute
microseconds not milliseconds, thus ending up with a delay 1000X longer
than intended.  Somebody along the way had noticed this problem but
misdiagnosed the cause, and imposed an ad-hoc 60-second limit rather
than fixing the units.  This was relatively harmless in context, because
we don't care that much about exactly how long this delay is; still,
it's wrong.

There are a few more callers of TimestampDifference() that don't
have a direct need for seconds-and-microseconds, but can't use
TimestampDifferenceMilliseconds() either because they do need
microsecond precision or because they might possibly deal with
intervals long enough to overflow 32-bit milliseconds.  It might be
worth inventing another API to improve that, but that seems outside
the scope of this patch; so those callers are untouched here.

Given the fact that we are fixing some bugs, and the likelihood
that future patches might want to back-patch code that uses this
new API, back-patch to all supported branches.

Alexey Kondratov and Tom Lane

Discussion: https://postgr.es/m/3b1c053a21c07c1ed5e00be3b2b855ef@postgrespro.ru

4 years agodoc: fix spelling "connction" to "connection"
Bruce Momjian [Wed, 11 Nov 2020 00:18:35 +0000 (19:18 -0500)]
doc: fix spelling "connction" to "connection"

Was wrong in commit 1a9388bd0f.

Reported-by: Tom Lane, Justin Pryzby
Discussion: https://postgr.es/m/20201102063333.GE22691@telsasoft.com

Backpatch-through: 9.5

4 years agoWork around cross-version-upgrade issues created by commit 9e38c2bb5.
Tom Lane [Tue, 10 Nov 2020 23:32:36 +0000 (18:32 -0500)]
Work around cross-version-upgrade issues created by commit 9e38c2bb5.

Summarily changing the STYPE of regression-test aggregates that
depend on array_append or array_cat is an issue for the buildfarm's
cross-version-upgrade tests, because those aggregates (as defined
in the back branches) now won't load into HEAD.  Although this seems
like only a minimal risk for genuine user-defined aggregates, we
need to do something for the buildfarm.  Hence, adjust the aggregate
definitions, in both HEAD and the back branches.

Discussion: https://postgr.es/m/1401824.1604537031@sss.pgh.pa.us
Discussion: https://postgr.es/m/E1kaQ2c-0005lx-Eg@gemulon.postgresql.org

4 years agopg_rewind: Fix thinko in parsing target WAL.
Heikki Linnakangas [Tue, 10 Nov 2020 17:25:46 +0000 (19:25 +0200)]
pg_rewind: Fix thinko in parsing target WAL.

It's entirely possible to see WAL for a relation that doesn't exist in
the target anymore. That happens when the relation was dropped later.
The refactoring in commit eb00f1d4b broke that case, by sanity-checking
the file type in the target before checking the flag forwhether it
exists there at all.

I noticed this during manual testing. Modify the 001_basic.pl test so
that it covers this case.

4 years agoFix out of date comment
Magnus Hagander [Tue, 10 Nov 2020 12:14:09 +0000 (13:14 +0100)]
Fix out of date comment

4 years agoRemove -o option to postmaster
Magnus Hagander [Tue, 10 Nov 2020 12:08:21 +0000 (13:08 +0100)]
Remove -o option to postmaster

This option was declared obsolete many years ago.

Reviewed-By: Tom Lane
Discussion: https://postgr.es/m/CABUevEyOE=9CQwZm2j=vwP5+6OLCSoxn9pBjK8gyRdkTzMfqtQ@mail.gmail.com

4 years agojit: Add support for LLVM 12.
Andres Freund [Tue, 10 Nov 2020 04:01:33 +0000 (20:01 -0800)]
jit: Add support for LLVM 12.

LLVM 12, to be released in a few months, made some breaking changes to
the Orc JIT interface. OrcV2 eventually will make it easier to support
features like concurrent JIT compilation, but this commit only allows
to compile against LLVM 12.

This commit is a bit bigger than desirable. That partially is because
the V2 interface is more granular than V1 interface, but also because
I chose to make some minor changes to < LLVM 12 code to keep the code
somewhat readable.

The LLVM 12 support will need to be backpatched. I plan to do so after
the patch stewed on the buildfarm for a few days.

Author: Andres Freund
Discussion: https://postgr.es/m/20201016011244.pmyvr3ee2gbzplq4@alap3.anarazel.de

4 years agoDoc: clarify data type behavior of COALESCE and NULLIF.
Tom Lane [Mon, 9 Nov 2020 17:02:24 +0000 (12:02 -0500)]
Doc: clarify data type behavior of COALESCE and NULLIF.

After studying the code, NULLIF is a lot more subtle than you might
have guessed.

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

4 years agoRemove ineffective heapam CHECK_FOR_INTERRUPTS().
Peter Geoghegan [Mon, 9 Nov 2020 17:00:12 +0000 (09:00 -0800)]
Remove ineffective heapam CHECK_FOR_INTERRUPTS().

Remove a CHECK_FOR_INTERRUPTS() call that could never actually handle an
interrupt.  We always have a heap page buffer lock at this point.
Having a useless CHECK_FOR_INTERRUPTS() call is harmless but misleading.

It is probably possible to work around the immediate problem by moving
the CHECK_FOR_INTERRUPTS() to before the heap page buffer lock is
acquired.  That isn't enough to make the function responsive to
interrupts, though.  The index AM caller will still hold an exclusive
buffer lock of its own.

4 years agoIgnore attempts to \gset into specially treated variables.
Noah Misch [Mon, 9 Nov 2020 15:32:09 +0000 (07:32 -0800)]
Ignore attempts to \gset into specially treated variables.

If an interactive psql session used \gset when querying a compromised
server, the attacker could execute arbitrary code as the operating
system account running psql.  Using a prefix not found among specially
treated variables, e.g. every lowercase string, precluded the attack.
Fix by issuing a warning and setting no variable for the column in
question.  Users wanting the old behavior can use a prefix and then a
meta-command like "\set HISTSIZE :prefix_HISTSIZE".  Back-patch to 9.5
(all supported versions).

Reviewed by Robert Haas.  Reported by Nick Cleaton.

Security: CVE-2020-25696

4 years agoIn security-restricted operations, block enqueue of at-commit user code.
Noah Misch [Mon, 9 Nov 2020 15:32:09 +0000 (07:32 -0800)]
In security-restricted operations, block enqueue of at-commit user code.

Specifically, this blocks DECLARE ... WITH HOLD and firing of deferred
triggers within index expressions and materialized view queries.  An
attacker having permission to create non-temp objects in at least one
schema could execute arbitrary SQL functions under the identity of the
bootstrap superuser.  One can work around the vulnerability by disabling
autovacuum and not manually running ANALYZE, CLUSTER, REINDEX, CREATE
INDEX, VACUUM FULL, or REFRESH MATERIALIZED VIEW.  (Don't restore from
pg_dump, since it runs some of those commands.)  Plain VACUUM (without
FULL) is safe, and all commands are fine when a trusted user owns the
target object.  Performance may degrade quickly under this workaround,
however.  Back-patch to 9.5 (all supported versions).

Reviewed by Robert Haas.  Reported by Etienne Stalmans.

Security: CVE-2020-25695

4 years agoRemove analyze_new_cluster script from pg_upgrade
Magnus Hagander [Mon, 9 Nov 2020 11:14:59 +0000 (12:14 +0100)]
Remove analyze_new_cluster script from pg_upgrade

Since this script just runs vacuumdb anyway, remove the script and
replace the instructions to run it with instructions to run vacuumdb
directly.

Reviewed-By: Michael Paquier
Discussion: https://postgr.es/m/CABUevEwg5LDFzthhxzSj7sZGMiVsZe0VVNbzzwTQOHJ=rN7+5A@mail.gmail.com

4 years agoRemove incorrect %s in string
Magnus Hagander [Mon, 9 Nov 2020 09:36:49 +0000 (10:36 +0100)]
Remove incorrect %s in string

Appears to have been a copy/paste error in the original commit that
moved the messages to fe_utils/.

Author: Tang, Haiying <tanghy.fnst@cn.fujitsu.com>
Backpatch-through: 13
Discussion: https://postgr.es/m/3321cbcea76d4d2c8320a05c19b9304a@G08CNEXMBPEKD05.g08.fujitsu.local

4 years agodoc: Add note about pg_settings and customized options into catalogs.sgml.
Fujii Masao [Mon, 9 Nov 2020 06:10:26 +0000 (15:10 +0900)]
doc: Add note about pg_settings and customized options into catalogs.sgml.

The pg_settings view does not display customized options until
the extension module that defines them has been loaded. This commit
add the note about that behavior, into the docs.

Author: John Naylor
Reviewed-by: Tom Lane, Fujii Masao
Discussion: https://postgr.es/m/CAFBsxsGsBZsG=cLM0Op5HFb2Ks6SzJrOc_eRO_jcKSNuqFRKnQ@mail.gmail.com

4 years agoFix parsePGArray() error checking in pg_dump.
Thomas Munro [Mon, 9 Nov 2020 03:01:51 +0000 (16:01 +1300)]
Fix parsePGArray() error checking in pg_dump.

Coverity complained about a defect in commit 257836a7:

  Calling "parsePGArray" without checking return value (as is
  done elsewhere 11 out of 13 times).

Fix, and also check for empty strings explicitly (NULL as represented by
PQgetvalue()).  That worked correctly before only because parsePGArray()
happens to set *nitems = 0 when it fails on an empty string.  Also
convert a sanity check assertion to an error to be more paranoid, and
pgindent a nearby line.

Reported-by: Michael Paquier <michael@paquier.xyz>
4 years agoIn INSERT/UPDATE, use the table's real tuple descriptor as target.
Tom Lane [Sun, 8 Nov 2020 18:08:36 +0000 (13:08 -0500)]
In INSERT/UPDATE, use the table's real tuple descriptor as target.

This back-patches commit 20d3fe900 into the v12 and v13 branches.
At the time I thought that commit was not fixing any observable
bug, but Bertrand Drouvot showed otherwise: adding a dropped column
to the previously-considered scenario crashes v12 and v13, unless the
dropped column happens to be an integer.  That is, of course, because
the tupdesc we derive from the plan output tlist fails to describe
the dropped column accurately, so that we'll do the wrong thing with
a tuple in which that column isn't NULL.

There is no bug in pre-v12 branches because they already did use
the table's real tuple descriptor for any trigger-returned tuple.
It seems that this set of bugs can be blamed on the changes that
removed es_trig_tuple_slot, though I've not attempted to pin that
down precisely.

Although there's no code change needed in HEAD, update the test case
to include a dropped column there too.

Discussion: https://postgr.es/m/db5d97c8-f48a-51e2-7b08-b73d5434d425@amazon.com
Discussion: https://postgr.es/m/16644-5da7ef98a7ac4545@postgresql.org

4 years agoFix assertion in collation version lookup.
Thomas Munro [Sun, 8 Nov 2020 07:43:45 +0000 (20:43 +1300)]
Fix assertion in collation version lookup.

Commit 257836a7 included an assertion that a version lookup routine is
not trying to look up "C" or "POSIX", but that case is reachable with
the user-facing SQL function pg_collation_actual_version().  Remove the
assertion.

4 years agoFix test for error message change
Peter Eisentraut [Sun, 8 Nov 2020 06:48:18 +0000 (07:48 +0100)]
Fix test for error message change

fix for 6be725e701611660b36642de9ff1d665a1eb24f5

4 years agoImprove nbtree README's LP_DEAD section.
Peter Geoghegan [Sun, 8 Nov 2020 02:51:12 +0000 (18:51 -0800)]
Improve nbtree README's LP_DEAD section.

The description of how LP_DEAD bit setting by index scans works
following commit 2ed5b87f was rather unclear.  Clean that up a bit.

Also refer to LP_DEAD bit setting within _bt_check_unique() at the start
of the same section.  This mechanism may actually be more important than
the generic kill_prior_tuple mechanism that the section focuses on, so
it at least deserves to be mentioned in passing.

4 years agoMessage style improvements
Alvaro Herrera [Sat, 7 Nov 2020 22:33:43 +0000 (19:33 -0300)]
Message style improvements

* Avoid pointlessly highlighting that an index vacuum was executed by a
  parallel worker; user doesn't care.

* Don't give the impression that a non-concurrent reindex of an invalid
  index on a TOAST table would work, because it wouldn't.

* Add a "translator:" comment for a mysterious message.

Discussion: https://postgr.es/m/20201107034943.GA16596@alvherre.pgsql
Reviewed-by: Michael Paquier <michael@paquier.xyz>
4 years agoFix redundant error messages in client tools
Peter Eisentraut [Sat, 7 Nov 2020 21:15:52 +0000 (22:15 +0100)]
Fix redundant error messages in client tools

A few client tools duplicate error messages already provided by libpq.

Discussion: https://www.postgresql.org/message-id/flat/3e937641-88a1-e697-612e-99bba4b8e5e4%40enterprisedb.com

4 years agoAvoid re-using output variables in new ecpg test case.
Tom Lane [Sat, 7 Nov 2020 21:25:42 +0000 (16:25 -0500)]
Avoid re-using output variables in new ecpg test case.

The buildfarm thinks this leads to memory stomps, though annoyingly
I can't duplicate that here.  The existing code in strings.pgc is
doing something that doesn't seem to be sanctioned at all really
by the documentation, but I'm disinclined to try to make that nicer
right now.  Let's just declare some more output variables in hopes
of working around it.

4 years agoFix ecpg's mishandling of B'...' and X'...' literals.
Tom Lane [Sat, 7 Nov 2020 20:03:44 +0000 (15:03 -0500)]
Fix ecpg's mishandling of B'...' and X'...' literals.

These were broken in multiple ways:

* The xbstart and xhstart lexer actions neglected to set
"state_before_str_start" before transitioning to the xb/xh states,
thus possibly resulting in "internal error: unreachable state" later.

* The test for valid string contents at the end of xb state was flat out
wrong, as it accounted incorrectly for the "b" prefix that the xbstart
action had injected.  Meanwhile, the xh state had no such check at all.

* The generated literal value failed to include any quote marks.

* The grammar did the wrong thing anyway, typically ignoring the
literal value and emitting something else, since BCONST and XCONST
tokens were handled randomly differently from SCONST tokens.

The first of these problems is evidently an oversight in commit
7f380c59f, but the others seem to be very ancient.  The lack of
complaints shows that ECPG users aren't using these syntaxes much
(although I do vaguely remember one previous complaint).

As written, this patch is dependent on 7f380c59f, so it can't go
back further than v13.  Given the shortage of complaints, I'm not
excited about adapting the patch to prior branches.

Report and patch by Shenhao Wang (test case adjusted by me)

Discussion: https://postgr.es/m/d6402f1bacb74ecba22ef715dbba17fd@G08CNEXMBPEKD06.g08.fujitsu.local

4 years agoMove catalog index declarations
Peter Eisentraut [Sat, 7 Nov 2020 11:11:40 +0000 (12:11 +0100)]
Move catalog index declarations

Move the system catalog index declarations from catalog/indexing.h to
the respective parent tables' catalog/pg_*.h files.  The original
reason for having it split was that the old genbki system produced the
output in the order of the catalog files it read, so all the indexing
stuff needed to come separately.  But this is no longer the case, and
keeping it together makes more sense.

Reviewed-by: John Naylor <john.naylor@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/c7cc82d6-f976-75d6-2e3e-b03d2cab26bb@2ndquadrant.com

4 years agoMove catalog toast table declarations
Peter Eisentraut [Sat, 7 Nov 2020 11:11:40 +0000 (12:11 +0100)]
Move catalog toast table declarations

Move the system catalog toast table declarations from
catalog/toasting.h to the respective parent tables' catalog/pg_*.h
files.  The original reason for having it split was that the old
genbki system produced the output in the order of the catalog files it
read, so all the toasting stuff needed to come separately.  But this
is no longer the case, and keeping it together makes more sense.

Reviewed-by: John Naylor <john.naylor@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/c7cc82d6-f976-75d6-2e3e-b03d2cab26bb@2ndquadrant.com