postgresql-pgindent.git
4 years agoFix has_column_privilege function corner case
Joe Conway [Wed, 31 Mar 2021 17:55:25 +0000 (13:55 -0400)]
Fix has_column_privilege function corner case

According to the comments, when an invalid or dropped column oid is passed
to has_column_privilege(), the intention has always been to return NULL.
However, when the caller had table level privilege the invalid/missing
column was never discovered, because table permissions were checked first.

Fix that by introducing extended versions of pg_attribute_acl(check|mask)
and pg_class_acl(check|mask) which take a new argument, is_missing. When
is_missing is NULL, the old behavior is preserved. But when is_missing is
passed by the caller, no ERROR is thrown for dropped or missing
columns/relations, and is_missing is flipped to true. This in turn allows
has_column_privilege to check for column privileges first, providing the
desired semantics.

Not backpatched since it is a user visible behavioral change with no previous
complaints, and the fix is a bit on the invasive side.

Author: Joe Conway
Reviewed-By: Tom Lane
Reported by: Ian Barwick
Discussion: https://postgr.es/m/flat/9b5f4311-157b-4164-7fe7-077b4fe8ed84%40joeconway.com

4 years agoRework planning and execution of UPDATE and DELETE.
Tom Lane [Wed, 31 Mar 2021 15:52:34 +0000 (11:52 -0400)]
Rework planning and execution of UPDATE and DELETE.

This patch makes two closely related sets of changes:

1. For UPDATE, the subplan of the ModifyTable node now only delivers
the new values of the changed columns (i.e., the expressions computed
in the query's SET clause) plus row identity information such as CTID.
ModifyTable must re-fetch the original tuple to merge in the old
values of any unchanged columns.  The core advantage of this is that
the changed columns are uniform across all tables of an inherited or
partitioned target relation, whereas the other columns might not be.
A secondary advantage, when the UPDATE involves joins, is that less
data needs to pass through the plan tree.  The disadvantage of course
is an extra fetch of each tuple to be updated.  However, that seems to
be very nearly free in context; even worst-case tests don't show it to
add more than a couple percent to the total query cost.  At some point
it might be interesting to combine the re-fetch with the tuple access
that ModifyTable must do anyway to mark the old tuple dead; but that
would require a good deal of refactoring and it seems it wouldn't buy
all that much, so this patch doesn't attempt it.

2. For inherited UPDATE/DELETE, instead of generating a separate
subplan for each target relation, we now generate a single subplan
that is just exactly like a SELECT's plan, then stick ModifyTable
on top of that.  To let ModifyTable know which target relation a
given incoming row refers to, a tableoid junk column is added to
the row identity information.  This gets rid of the horrid hack
that was inheritance_planner(), eliminating O(N^2) planning cost
and memory consumption in cases where there were many unprunable
target relations.

Point 2 of course requires point 1, so that there is a uniform
definition of the non-junk columns to be returned by the subplan.
We can't insist on uniform definition of the row identity junk
columns however, if we want to keep the ability to have both
plain and foreign tables in a partitioning hierarchy.  Since
it wouldn't scale very far to have every child table have its
own row identity column, this patch includes provisions to merge
similar row identity columns into one column of the subplan result.
In particular, we can merge the whole-row Vars typically used as
row identity by FDWs into one column by pretending they are type
RECORD.  (It's still okay for the actual composite Datums to be
labeled with the table's rowtype OID, though.)

There is more that can be done to file down residual inefficiencies
in this patch, but it seems to be committable now.

FDW authors should note several API changes:

* The argument list for AddForeignUpdateTargets() has changed, and so
has the method it must use for adding junk columns to the query.  Call
add_row_identity_var() instead of manipulating the parse tree directly.
You might want to reconsider exactly what you're adding, too.

* PlanDirectModify() must now work a little harder to find the
ForeignScan plan node; if the foreign table is part of a partitioning
hierarchy then the ForeignScan might not be the direct child of
ModifyTable.  See postgres_fdw for sample code.

* To check whether a relation is a target relation, it's no
longer sufficient to compare its relid to root->parse->resultRelation.
Instead, check it against all_result_relids or leaf_result_relids,
as appropriate.

Amit Langote and Tom Lane

Discussion: https://postgr.es/m/CA+HiwqHpHdqdDn48yCEhynnniahH78rwcrv1rEX65-fsZGBOLQ@mail.gmail.com

4 years agoAllow an alias to be attached to a JOIN ... USING
Peter Eisentraut [Wed, 31 Mar 2021 15:09:24 +0000 (17:09 +0200)]
Allow an alias to be attached to a JOIN ... USING

This allows something like

    SELECT ... FROM t1 JOIN t2 USING (a, b, c) AS x

where x has the columns a, b, c and unlike a regular alias it does not
hide the range variables of the tables being joined t1 and t2.

Per SQL:2016 feature F404 "Range variable for common column names".

Reviewed-by: Vik Fearing <[email protected]>
Reviewed-by: Tom Lane <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/454638cf-d563-ab76-a585-2564428062af@2ndquadrant.com

4 years agoAdd support for asynchronous execution.
Etsuro Fujita [Wed, 31 Mar 2021 09:45:00 +0000 (18:45 +0900)]
Add support for asynchronous execution.

This implements asynchronous execution, which runs multiple parts of a
non-parallel-aware Append concurrently rather than serially to improve
performance when possible.  Currently, the only node type that can be
run concurrently is a ForeignScan that is an immediate child of such an
Append.  In the case where such ForeignScans access data on different
remote servers, this would run those ForeignScans concurrently, and
overlap the remote operations to be performed simultaneously, so it'll
improve the performance especially when the operations involve
time-consuming ones such as remote join and remote aggregation.

We may extend this to other node types such as joins or aggregates over
ForeignScans in the future.

This also adds the support for postgres_fdw, which is enabled by the
table-level/server-level option "async_capable".  The default is false.

Robert Haas, Kyotaro Horiguchi, Thomas Munro, and myself.  This commit
is mostly based on the patch proposed by Robert Haas, but also uses
stuff from the patch proposed by Kyotaro Horiguchi and from the patch
proposed by Thomas Munro.  Reviewed by Kyotaro Horiguchi, Konstantin
Knizhnik, Andrey Lepikhov, Movead Li, Thomas Munro, Justin Pryzby, and
others.

Discussion: https://postgr.es/m/CA%2BTgmoaXQEt4tZ03FtQhnzeDEMzBck%2BLrni0UWHVVgOTnA6C1w%40mail.gmail.com
Discussion: https://postgr.es/m/CA%2BhUKGLBRyu0rHrDCMC4%3DRn3252gogyp1SjOgG8SEKKZv%3DFwfQ%40mail.gmail.com
Discussion: https://postgr.es/m/20200228.170650.667613673625155850.horikyota.ntt%40gmail.com

4 years agoAdd p_names field to ParseNamespaceItem
Peter Eisentraut [Wed, 31 Mar 2021 08:52:37 +0000 (10:52 +0200)]
Add p_names field to ParseNamespaceItem

ParseNamespaceItem had a wired-in assumption that p_rte->eref
describes the table and column aliases exposed by the nsitem.  This
relaxes this by creating a separate p_names field in an nsitem.  This
is mainly preparation for a patch for JOIN USING aliases, but it saves
one indirection in common code paths, so it's possibly a win on its
own.

Author: Tom Lane <[email protected]>
Discussion: https://www.postgresql.org/message-id/785329.1616455091@sss.pgh.pa.us

4 years agoAdd errhint_plural() function and make use of it
Peter Eisentraut [Wed, 31 Mar 2021 07:15:51 +0000 (09:15 +0200)]
Add errhint_plural() function and make use of it

Similar to existing errmsg_plural() and errdetail_plural().  Some
errhint() calls hadn't received the proper plural treatment yet.

4 years agodoc: Remove Cyrillic from unistr example
Peter Eisentraut [Wed, 31 Mar 2021 06:20:35 +0000 (08:20 +0200)]
doc: Remove Cyrillic from unistr example

Not supported by PDF build right now, so let's do without it.

4 years agoRemove extra semicolon in postgres_fdw tests.
Amit Kapila [Wed, 31 Mar 2021 05:06:39 +0000 (10:36 +0530)]
Remove extra semicolon in postgres_fdw tests.

Author: Suraj Kharage
Reviewed-by: Bharath Rupireddy, Vignesh C
Discussion: https://postgr.es/m/CAF1DzPWRfxUeH-wShz7P_pK5Tx6M_nEK+TkS8gn5ngvg07Q5=g@mail.gmail.com

4 years agoDoc: Use consistent terminology for tablesync slots.
Amit Kapila [Wed, 31 Mar 2021 02:47:50 +0000 (08:17 +0530)]
Doc: Use consistent terminology for tablesync slots.

At some places in the docs, we refer to them as tablesync slots and at other
places as table synchronization slots. For consistency, we refer to them as
table synchronization slots at all places.

Author: Peter Smith
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CAHut+PvzYNKCeZ=kKBDkh3dw-r=2D3fk=nNc9SXSW=CZGk69xg@mail.gmail.com

4 years agoAccept slightly-filled pages for tuples larger than fillfactor.
Noah Misch [Wed, 31 Mar 2021 01:53:44 +0000 (18:53 -0700)]
Accept slightly-filled pages for tuples larger than fillfactor.

We always inserted a larger-than-fillfactor tuple into a newly-extended
page, even when existing pages were empty or contained nothing but an
unused line pointer.  This was unnecessary relation extension.  Start
tolerating page usage up to 1/8 the maximum space that could be taken up
by line pointers.  This is somewhat arbitrary, but it should allow more
cases to reuse pages.  This has no effect on tables with fillfactor=100
(the default).

John Naylor and Floris van Nee.  Reviewed by Matthias van de Meent.
Reported by Floris van Nee.

Discussion: https://postgr.es/m/6e263217180649339720afe2176c50aa@opammb0562.comp.optiver.com

4 years agoFix comment in parsenodes.h
Michael Paquier [Wed, 31 Mar 2021 00:35:58 +0000 (09:35 +0900)]
Fix comment in parsenodes.h

CreateStmt->inhRelations is a list of RangeVars, but a comment was
incorrect about that.

Author: Julien Rouhaud
Discussion: https://postgr.es/m/20210330123015.yzekhz5sweqbgxdr@nol

4 years agoAdd support for --extension in pg_dump
Michael Paquier [Wed, 31 Mar 2021 00:12:34 +0000 (09:12 +0900)]
Add support for --extension in pg_dump

When specified, only extensions matching the given pattern are included
in dumps.  Similarly to --table and --schema, when --strict-names is
used,  a perfect match is required.  Also, like the two other options,
this new option offers no guarantee that dependent objects have been
dumped, so a restore may fail on a clean database.

Tests are added in test_pg_dump/, checking after a set of positive and
negative cases, with or without an extension's contents added to the
dump generated.

Author: Guillaume Lelarge
Reviewed-by: David Fetter, Tom Lane, Michael Paquier, Asif Rehman,
Julien Rouhaud
Discussion: https://postgr.es/m/CAECtzeXOt4cnMU5+XMZzxBPJ_wu76pNy6HZKPRBL-j7yj1E4+g@mail.gmail.com

4 years agoRemove small inefficiency in ExecARDeleteTriggers/ExecARUpdateTriggers.
Tom Lane [Wed, 31 Mar 2021 00:01:27 +0000 (20:01 -0400)]
Remove small inefficiency in ExecARDeleteTriggers/ExecARUpdateTriggers.

Whilst poking at nodeModifyTable.c, I chanced to notice that while
its calls to ExecBR*Triggers and ExecIR*Triggers are protected by
tests to see if there are any relevant triggers to fire, its calls
to ExecAR*Triggers are not; the latter functions do the equivalent
tests themselves.  This seems possibly reasonable given the more
complex conditions involved, but what's less reasonable is that
the ExecAR* functions aren't careful to do no work when there is
no work to be done.  ExecARInsertTriggers gets this right, but the
other two will both force creation of a slot that the query may
have no use for.  ExecARUpdateTriggers additionally performed a
usually-useless ExecClearTuple() on that slot.  This is probably
all pretty microscopic in real workloads, but a cycle shaved is a
cycle earned.

4 years agoadjust dblink regression expected output for commit 5da9868ed9
Bruce Momjian [Tue, 30 Mar 2021 23:46:22 +0000 (19:46 -0400)]
adjust dblink regression expected output for commit 5da9868ed9

Seems the -1/singular output is used in the dblink regression tests.

Reported-by: Álvaro Herrera
Discussion: https://postgr.es/m/20210330231506[email protected]

4 years agolibpq_pipeline: add PQtrace() support and tests
Alvaro Herrera [Tue, 30 Mar 2021 23:33:04 +0000 (20:33 -0300)]
libpq_pipeline: add PQtrace() support and tests

The libpq_pipeline program recently introduced by commit acb7e4eb6b1c
is well equipped to test the PQtrace() functionality, so let's make it
do that.

Author: Álvaro Herrera <[email protected]>
Discussion: https://postgr.es/m/20210327192812[email protected]

4 years agoImprove PQtrace() output format
Alvaro Herrera [Tue, 30 Mar 2021 23:12:34 +0000 (20:12 -0300)]
Improve PQtrace() output format

Transform the PQtrace output format from its ancient (and mostly
useless) byte-level output format to a logical-message-level output,
making it much more usable.  This implementation allows the printing
code to be written (as it indeed was) by looking at the protocol
documentation, which gives more confidence that the three (docs, trace
code and actual code) actually match.

Author: 岩田 彩 (Aya Iwata) <[email protected]>
Reviewed-by: 綱川 貴之 (Takayuki Tsunakawa) <[email protected]>
Reviewed-by: Kirk Jamison <[email protected]>
Reviewed-by: Kyotaro Horiguchi <[email protected]>
Reviewed-by: Tom Lane <[email protected]>
Reviewed-by: 黒田 隼人 (Hayato Kuroda) <[email protected]>
Reviewed-by: "Nagaura, Ryohei" <[email protected]>
Reviewed-by: Ryo Matsumura <[email protected]>
Reviewed-by: Greg Nancarrow <[email protected]>
Reviewed-by: Jim Doty <[email protected]>
Reviewed-by: Álvaro Herrera <[email protected]>
Discussion: https://postgr.es/m/71E660EB361DF14299875B198D4CE5423DE3FBA4@g01jpexmbkw25

4 years agoIn messages, use singular nouns for -1, like we do for +1.
Bruce Momjian [Tue, 30 Mar 2021 22:34:27 +0000 (18:34 -0400)]
In messages, use singular nouns for -1, like we do for +1.

This outputs "-1 year", not "-1 years".

Reported-by: [email protected]
Bug: 16939

Discussion: https://postgr.es/m/16939-cceeb03fb72736ee@postgresql.org

4 years agoAdd tests for date_part of epoch near upper bound of timestamp range
Peter Eisentraut [Tue, 30 Mar 2021 20:05:18 +0000 (22:05 +0200)]
Add tests for date_part of epoch near upper bound of timestamp range

This exercises a special case in the implementations of
date_part('epoch', timestamp[tz]) that was previously not tested.

4 years agoUse a WaitLatch for vacuum/autovacuum sleeping
Stephen Frost [Tue, 30 Mar 2021 16:52:56 +0000 (12:52 -0400)]
Use a WaitLatch for vacuum/autovacuum sleeping

Instead of using pg_usleep() in vacuum_delay_point(), use a WaitLatch.
This has the advantage that we will realize if the postmaster has been
killed since the last time we decided to sleep while vacuuming.

Reviewed-by: Thomas Munro
Discussion: https://postgr.es/m/CAFh8B=kcdk8k-Y21RfXPu5dX=bgPqJ8TC3p_qxR_ygdBS=JN5w@mail.gmail.com

4 years agoFurther tweaking of pg_dump's handling of default_toast_compression.
Tom Lane [Tue, 30 Mar 2021 14:57:57 +0000 (10:57 -0400)]
Further tweaking of pg_dump's handling of default_toast_compression.

As committed in bbe0a81db, pg_dump from a pre-v14 server effectively
acts as though you'd said --no-toast-compression.  I think the right
thing is for it to act as though default_toast_compression is set to
"pglz", instead, so that the tables' toast compression behavior is
preserved.  You can always get the other behavior, if you want that,
by giving the switch.

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

4 years agoAllow estimate_num_groups() to pass back further details about the estimation
David Rowley [Tue, 30 Mar 2021 07:52:46 +0000 (20:52 +1300)]
Allow estimate_num_groups() to pass back further details about the estimation

Here we add a new output parameter to estimate_num_groups() to allow it to
inform the caller of additional, possibly useful information about the
estimation.

The new output parameter is a struct that currently contains just a single
field with a set of flags.  This was done rather than having the flags as
an output parameter to allow future fields to be added without having to
change the signature of the function at a later date when we want to pass
back further information that might not be suitable to store in the flags
field.

It seems reasonable that one day in the future that the planner would want
to know more about the estimation. For example, how many individual sets
of statistics was the estimation generated from?  The planner may want to
take that into account if we ever want to consider risks as well as costs
when generating plans.

For now, there's only 1 flag we set in the flags field.  This is to
indicate if the estimation fell back on using the hard-coded constants in
any part of the estimation. Callers may like to change their behavior if
this is set, and this gives them the ability to do so.  Callers may pass
the flag pointer as NULL if they have no interest in obtaining any
additional information about the estimate.

We're not adding any actual usages of these flags here.  Some follow-up
commits will make use of this feature.  Additionally, we're also not
making any changes to add support for clauselist_selectivity() and
clauselist_selectivity_ext().  However, if this is required in the future
then the same struct being added here should be fine to use as a new
output argument for those functions too.

Author: David Rowley
Discussion: https://postgr.es/m/CAApHDvqQqpk=1W-G_ds7A9CsXX3BggWj_7okinzkLVhDubQzjA@mail.gmail.com

4 years agoFix compiler warning in unistr function
David Rowley [Tue, 30 Mar 2021 07:28:09 +0000 (20:28 +1300)]
Fix compiler warning in unistr function

Some compilers are not aware that elog/ereport ERROR does not return.

4 years agoAllow users of simplehash.h to perform direct deletions
David Rowley [Tue, 30 Mar 2021 06:56:50 +0000 (19:56 +1300)]
Allow users of simplehash.h to perform direct deletions

Previously simplehash.h only exposed a method to perform a hash table
delete using the hash table key. This meant that the delete function had
to perform a hash lookup in order to find the entry to delete.  Here we
add a new function so that users of simplehash.h can perform a hash delete
directly using the entry pointer, thus saving the hash lookup.

An upcoming patch that uses simplehash.h already has performed the hash
lookup so already has the entry pointer.  This change will allow the
code in that patch to perform the hash delete without the code in
simplehash.h having to perform an additional hash lookup.

Author: David Rowley
Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/CAApHDvqFLXXge153WmPsjke5VGOSt7Ez0yD0c7eBXLfmWxs3Kw@mail.gmail.com

4 years agoAdd upper boundary tests for timestamp and timestamptz types
Peter Eisentraut [Tue, 30 Mar 2021 06:46:34 +0000 (08:46 +0200)]
Add upper boundary tests for timestamp and timestamptz types

The existing regression tests only tested the lower boundary of the
range supported by the timestamp and timestamptz types because "The
upper boundary differs between integer and float timestamps, so no
check".  Since this is obsolete, add similar tests for the upper
boundary.

4 years agoAdd a xid argument to the filter_prepare callback for output plugins.
Amit Kapila [Tue, 30 Mar 2021 05:04:43 +0000 (10:34 +0530)]
Add a xid argument to the filter_prepare callback for output plugins.

Along with gid, this provides a different way to identify the transaction.
The users that use xid in some way to prepare the transactions can use it
to filter prepare transactions. The later commands COMMIT PREPARED or
ROLLBACK PREPARED carries both identifiers, providing an output plugin the
choice of what to use.

Author: Markus Wanner
Reviewed-by: Vignesh C, Amit Kapila
Discussion: https://postgr.es/m/ee280000-7355-c4dc-e47b-2436e7be959c@enterprisedb.com

4 years agoUpdate obsolete comment.
Etsuro Fujita [Tue, 30 Mar 2021 04:00:00 +0000 (13:00 +0900)]
Update obsolete comment.

Back-patch to all supported branches.

Author: Etsuro Fujita
Discussion: https://postgr.es/m/CAPmGK17DwzaSf%2BB71dhL2apXdtG-OmD6u2AL9Cq2ZmAR0%2BzapQ%40mail.gmail.com

4 years agopsql: call clearerr() just before printing
Alvaro Herrera [Mon, 29 Mar 2021 21:34:39 +0000 (18:34 -0300)]
psql: call clearerr() just before printing

We were never doing clearerr() on the output stream, which results in a
message being printed after each result once an EOF is seen:

could not print result table: Success

This message was added by commit b03436994bcc (in the pg13 era); before
that, the error indicator would never be examined.  So backpatch only
that far back, even though the actual bug (to wit: the fact that the
error indicator is never cleared) is older.

4 years agoAdjust design of per-worker parallel seqscan data struct
David Rowley [Mon, 29 Mar 2021 21:17:09 +0000 (10:17 +1300)]
Adjust design of per-worker parallel seqscan data struct

The design of the data structures which allow storage of the per-worker
memory during parallel seq scans were not ideal. The work done in
56788d215 required an additional data structure to allow workers to
remember the range of pages that had been allocated to them for
processing during a parallel seqscan.  That commit added a void pointer
field to TableScanDescData to allow heapam to store the per-worker
allocation information.  However putting the field there made very little
sense given that we have AM specific structs for that, e.g.
HeapScanDescData.

Here we remove the void pointer field from TableScanDescData and add a
dedicated field for this purpose to HeapScanDescData.

Previously we also allocated memory for this parallel per-worker data for
all scans, regardless if it was a parallel scan or not.  This was just a
wasted allocation for non-parallel scans, so here we make the allocation
conditional on the scan being parallel.

Also, add previously missing pfree() to free the per-worker data in
heap_endscan().

Reported-by: Andres Freund
Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/20210317023101[email protected]

4 years agoAllow matching the DN of a client certificate for authentication
Andrew Dunstan [Mon, 29 Mar 2021 19:31:22 +0000 (15:31 -0400)]
Allow matching the DN of a client certificate for authentication

Currently we only recognize the Common Name (CN) of a certificate's
subject to be matched against the user name. Thus certificates with
subjects '/OU=eng/CN=fred' and '/OU=sales/CN=fred' will have the same
connection rights. This patch provides an option to match the whole
Distinguished Name (DN) instead of just the CN. On any hba line using
client certificate identity, there is an option 'clientname' which can
have values of 'DN' or 'CN'. The default is 'CN', the current procedure.

The DN is matched against the RFC2253 formatted DN, which looks like
'CN=fred,OU=eng'.

This facility of probably best used in conjunction with an ident map.

Discussion: https://postgr.es/m/92e70110-9273-d93c-5913-0bccb6562740@dunslane.net

Reviewed-By: Michael Paquier, Daniel Gustafsson, Jacob Champion
4 years agoClean up date_part tests a bit
Peter Eisentraut [Mon, 29 Mar 2021 15:53:30 +0000 (17:53 +0200)]
Clean up date_part tests a bit

Some tests for timestamp and timestamptz were in the date.sql test
file.  Move them to their appropriate files, or drop tests cases that
were already present there.

4 years agoAdd unistr function
Peter Eisentraut [Sun, 28 Mar 2021 06:16:15 +0000 (08:16 +0200)]
Add unistr function

This allows decoding a string with Unicode escape sequences.  It is
similar to Unicode escape strings, but offers some more flexibility.

Author: Pavel Stehule <[email protected]>
Reviewed-by: Asif Rehman <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/CAFj8pRA5GnKT+gDVwbVRH2ep451H_myBt+NTz8RkYUARE9+qOQ@mail.gmail.com

4 years agoReset standard_conforming_strings in strings test
Peter Eisentraut [Mon, 29 Mar 2021 06:40:39 +0000 (08:40 +0200)]
Reset standard_conforming_strings in strings test

After some tests relating to standard_conforming_strings behavior, the
value was not reset to the default value.  Therefore, the rest of the
tests in that file ran with the nondefault setting, which affected the
results of some tests.  For clarity, reset the value and run the rest
of the tests with the default setting again.

4 years agoPageAddItemExtended(): Add LP_UNUSED assertion.
Peter Geoghegan [Mon, 29 Mar 2021 03:10:02 +0000 (20:10 -0700)]
PageAddItemExtended(): Add LP_UNUSED assertion.

Assert that LP_UNUSED items have no storage.  If it's worth having
defensive code in non-assert builds then it's worth having an assertion
as well.

4 years agoCache if PathTarget and RestrictInfos contain volatile functions
David Rowley [Mon, 29 Mar 2021 01:47:05 +0000 (14:47 +1300)]
Cache if PathTarget and RestrictInfos contain volatile functions

Here we aim to reduce duplicate work done by contain_volatile_functions()
by caching whether PathTargets and RestrictInfos contain any volatile
functions the first time contain_volatile_functions() is called for them.
Any future calls for these nodes just use the cached value rather than
going to the trouble of recursively checking the sub-node all over again.
Thanks to Tom Lane for the idea.

Any locations in the code which make changes to a PathTarget or
RestrictInfo which could change the outcome of the volatility check must
change the cached value back to VOLATILITY_UNKNOWN again.
contain_volatile_functions() is the only code in charge of setting the
cache value to either VOLATILITY_VOLATILE or VOLATILITY_NOVOLATILE.

Some existing code does benefit from this additional caching, however,
this change is mainly aimed at an upcoming patch that must check for
volatility during the join search.  Repeated volatility checks in that
case can become very expensive when the join search contains more than a
few relations.

Author: David Rowley
Discussion: https://postgr.es/m/3795226.1614059027@sss.pgh.pa.us

4 years agodoc: Define TLS as an acronym
Stephen Frost [Sun, 28 Mar 2021 15:27:59 +0000 (11:27 -0400)]
doc: Define TLS as an acronym

Commit c6763156589 added an acronym reference for "TLS" but the definition
was never added.

Author: Daniel Gustafsson
Reviewed-by: Michael Paquier
Backpatch-through: 9.6
Discussion: https://postgr.es/m/27109504-82DB-41A8-8E63-C0498314F5B0@yesql.se

4 years agoStabilize stats_ext test with other collations
Tomas Vondra [Sat, 27 Mar 2021 17:26:52 +0000 (18:26 +0100)]
Stabilize stats_ext test with other collations

The tests used string concatenation to test statistics on expressions,
but that made the tests locale-dependent, e.g. because the ordering of
'11' and '1X' depends on the collation. This affected both the estimated
and actual row couts, breaking some of the tests.

Fixed by replacing the string concatenation with upper() function call,
so that the text values contain only digits.

Discussion: https://postgr.es/m/b650920b-2767-fbc3-c87a-cb8b5d693cbf%40enterprisedb.com

4 years agoImprove consistency of SQL code capitalization
Peter Eisentraut [Sat, 27 Mar 2021 09:17:12 +0000 (10:17 +0100)]
Improve consistency of SQL code capitalization

4 years agoExtended statistics on expressions
Tomas Vondra [Fri, 26 Mar 2021 22:22:01 +0000 (23:22 +0100)]
Extended statistics on expressions

Allow defining extended statistics on expressions, not just just on
simple column references.  With this commit, expressions are supported
by all existing extended statistics kinds, improving the same types of
estimates. A simple example may look like this:

  CREATE TABLE t (a int);
  CREATE STATISTICS s ON mod(a,10), mod(a,20) FROM t;
  ANALYZE t;

The collected statistics are useful e.g. to estimate queries with those
expressions in WHERE or GROUP BY clauses:

  SELECT * FROM t WHERE mod(a,10) = 0 AND mod(a,20) = 0;

  SELECT 1 FROM t GROUP BY mod(a,10), mod(a,20);

This introduces new internal statistics kind 'e' (expressions) which is
built automatically when the statistics object definition includes any
expressions. This represents single-expression statistics, as if there
was an expression index (but without the index maintenance overhead).
The statistics is stored in pg_statistics_ext_data as an array of
composite types, which is possible thanks to 79f6a942bd.

CREATE STATISTICS allows building statistics on a single expression, in
which case in which case it's not possible to specify statistics kinds.

A new system view pg_stats_ext_exprs can be used to display expression
statistics, similarly to pg_stats and pg_stats_ext views.

ALTER TABLE ... ALTER COLUMN ... TYPE now treats indexes the same way it
treats indexes, i.e. it drops and recreates the statistics. This means
all statistics are reset, and we no longer try to preserve at least the
functional dependencies. This should not be a major issue in practice,
as the functional dependencies actually rely on per-column statistics,
which were always reset anyway.

Author: Tomas Vondra
Reviewed-by: Justin Pryzby, Dean Rasheed, Zhihong Yu
Discussion: https://postgr.es/m/ad7891d2-e90c-b446-9fe2-7419143847d7%40enterprisedb.com

4 years agoReduce duration of stats_ext regression tests
Tomas Vondra [Fri, 26 Mar 2021 22:00:41 +0000 (23:00 +0100)]
Reduce duration of stats_ext regression tests

The regression tests of extended statistics were taking a fair amount of
time, due to using fairly large data sets with a couple thousand rows.
So far this was fine, but with tests for statistics on expressions the
duration would get a bit excessive.  So reduce the size of some of the
tests that will be used to test expressions, to keep the duration under
control.  Done in a separate commit before adding the statistics on
expressions, to make it clear which estimates are expected to change.

Author: Tomas Vondra
Discussion: https://postgr.es/m/ad7891d2-e90c-b446-9fe2-7419143847d7%40enterprisedb.com

4 years agoFix ndistinct estimates with system attributes
Tomas Vondra [Fri, 26 Mar 2021 21:34:53 +0000 (22:34 +0100)]
Fix ndistinct estimates with system attributes

When estimating the number of groups using extended statistics, the code
was discarding information about system attributes. This led to strange
situation that

    SELECT 1 FROM t GROUP BY ctid;

could have produced higher estimate (equal to pg_class.reltuples) than

    SELECT 1 FROM t GROUP BY a, b, ctid;

with extended statistics on (a,b). Fixed by retaining information about
the system attribute.

Backpatch all the way to 10, where extended statistics were introduced.

Author: Tomas Vondra
Backpatch-through: 10

4 years agoAdd "pg_database_owner" default role.
Noah Misch [Fri, 26 Mar 2021 17:42:17 +0000 (10:42 -0700)]
Add "pg_database_owner" default role.

Membership consists, implicitly, of the current database owner.  Expect
use in template databases.  Once pg_database_owner has rights within a
template, each owner of a database instantiated from that template will
exercise those rights.

Reviewed by John Naylor.

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

4 years agoMerge similar algorithms into roles_is_member_of().
Noah Misch [Fri, 26 Mar 2021 17:42:16 +0000 (10:42 -0700)]
Merge similar algorithms into roles_is_member_of().

The next commit would have complicated two or three algorithms, so take
this opportunity to consolidate.  No functional changes.

Reviewed by John Naylor.

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

4 years agoFix alignment in BRIN minmax-multi deserialization
Tomas Vondra [Fri, 26 Mar 2021 15:33:19 +0000 (16:33 +0100)]
Fix alignment in BRIN minmax-multi deserialization

The deserialization failed to ensure correct alignment, as it assumed it
can simply point into the serialized value. The serialization however
ignores alignment and copies just the significant bytes in order to make
the result as small as possible. This caused failures on systems that
are sensitive to mialigned addresses, like sparc, or with address
sanitizer enabled.

Fixed by copying the serialized data to ensure proper alignment. While
at it, fix an issue with serialization on big endian machines, using the
same store_att_byval/fetch_att trick as extended statistics.

Discussion: https://postgr.es/0c8c3304-d3dd-5e29-d5ac-b50589a23c8c%40enterprisedb.com

4 years agoBRIN minmax-multi indexes
Tomas Vondra [Fri, 26 Mar 2021 12:54:29 +0000 (13:54 +0100)]
BRIN minmax-multi indexes

Adds BRIN opclasses similar to the existing minmax, except that instead
of summarizing the page range into a single [min,max] range, the summary
consists of multiple ranges and/or points, allowing gaps. This allows
more efficient handling of data with poor correlation to physical
location within the table and/or outlier values, for which the regular
minmax opclassed tend to work poorly.

It's possible to specify the number of values kept for each page range,
either as a single point or an interval boundary.

  CREATE TABLE t (a int);
  CREATE INDEX ON t
   USING brin (a int4_minmax_multi_ops(values_per_range=16));

When building the summary, the values are combined into intervals with
the goal to minimize the "covering" (sum of interval lengths), using a
support procedure computing distance between two values.

Bump catversion, due to various catalog changes.

Author: Tomas Vondra <[email protected]>
Reviewed-by: Alvaro Herrera <[email protected]>
Reviewed-by: Alexander Korotkov <[email protected]>
Reviewed-by: Sokolov Yura <[email protected]>
Reviewed-by: John Naylor <[email protected]>
Discussion: https://postgr.es/m/c1138ead-7668-f0e1-0638-c3be3237e812@2ndquadrant.com
Discussion: https://postgr.es/m/5d78b774-7e9c-c94e-12cf-fef51cc89b1a%402ndquadrant.com

4 years agoBRIN bloom indexes
Tomas Vondra [Fri, 26 Mar 2021 12:35:29 +0000 (13:35 +0100)]
BRIN bloom indexes

Adds a BRIN opclass using a Bloom filter to summarize the range. Indexes
using the new opclasses allow only equality queries (similar to hash
indexes), but that works fine for data like UUID, MAC addresses etc. for
which range queries are not very common. This also means the indexes
work for data that is not well correlated to physical location within
the table, or perhaps even entirely random (which is a common issue with
existing BRIN minmax opclasses).

It's possible to specify opclass parameters with the usual Bloom filter
parameters, i.e. the desired false-positive rate and the expected number
of distinct values per page range.

  CREATE TABLE t (a int);
  CREATE INDEX ON t
   USING brin (a int4_bloom_ops(false_positive_rate = 0.05,
                                n_distinct_per_range = 100));

The opclasses do not operate on the indexed values directly, but compute
a 32-bit hash first, and the Bloom filter is built on the hash value.
Collisions should not be a huge issue though, as the number of distinct
values in a page ranges is usually fairly small.

Bump catversion, due to various catalog changes.

Author: Tomas Vondra <[email protected]>
Reviewed-by: Alvaro Herrera <[email protected]>
Reviewed-by: Alexander Korotkov <[email protected]>
Reviewed-by: Sokolov Yura <[email protected]>
Reviewed-by: Nico Williams <[email protected]>
Reviewed-by: John Naylor <[email protected]>
Discussion: https://postgr.es/m/c1138ead-7668-f0e1-0638-c3be3237e812@2ndquadrant.com
Discussion: https://postgr.es/m/5d78b774-7e9c-c94e-12cf-fef51cc89b1a%402ndquadrant.com

4 years agoSupport the old signature of BRIN consistent function
Tomas Vondra [Fri, 26 Mar 2021 12:17:56 +0000 (13:17 +0100)]
Support the old signature of BRIN consistent function

Commit a1c649d889 changed the signature of the BRIN consistent function
by adding a new required parameter.  Treating the parameter as optional,
which would make the change backwards incompatibile, was rejected with
the justification that there are few out-of-core extensions, so it's not
worth adding making the code more complex, and it's better to deal with
that in the extension.

But after further thought, that would be rather problematic, because
pg_upgrade simply dumps catalog contents and the same version of an
extension needs to work on both PostgreSQL versions. Supporting both
variants of the consistent function (with 3 or 4 arguments) makes that
possible.

The signature is not the only thing that changed, as commit 72ccf55cb9
moved handling of IS [NOT] NULL keys from the support procedures. But
this change is backward compatible - handling the keys in exension is
unnecessary, but harmless. The consistent function will do a bit of
unnecessary work, but it should be very cheap.

This also undoes most of the changes to the existing opclasses (minmax
and inclusion), making them use the old signature again. This should
make backpatching simpler.

Catversion bump, because of changes in pg_amproc.

Author: Tomas Vondra <[email protected]>
Author: Nikita Glukhov <[email protected]>
Reviewed-by: Mark Dilger <[email protected]>
Reviewed-by: Alexander Korotkov <[email protected]>
Reviewed-by: Masahiko Sawada <[email protected]>
Reviewed-by: John Naylor <[email protected]>
Discussion: https://postgr.es/m/c1138ead-7668-f0e1-0638-c3be3237e812@2ndquadrant.com

4 years agoRemove unnecessary pg_amproc BRIN minmax entries
Tomas Vondra [Fri, 26 Mar 2021 12:04:13 +0000 (13:04 +0100)]
Remove unnecessary pg_amproc BRIN minmax entries

The BRIN minmax opclasses included amproc entries with mismatching left
and right types, but those happen to be unnecessary.  The opclasses only
need cross-type operators, not cross-type support procedures. Discovered
when trying to define equivalent BRIN operator families in an extension.

Catversion bump, because of pg_amproc changes.

Author: Tomas Vondra
Reviewed-by: Alvaro Herrera
Discussion: https://postgr.es/m/78c357ab-3395-8433-e7b3-b2cfcc9fdc23%40enterprisedb.com

4 years agoFix interaction of TOAST compression with expression indexes.
Robert Haas [Thu, 25 Mar 2021 23:55:32 +0000 (19:55 -0400)]
Fix interaction of TOAST compression with expression indexes.

Before, trying to compress a value for insertion into an expression
index would crash.

Dilip Kumar, with some editing by me. Report by Jaime Casanova.

Discussion: http://postgr.es/m/CAJKUy5gcs0zGOp6JXU2mMVdthYhuQpFk=S3V8DOKT=LZC1L36Q@mail.gmail.com

4 years agoALTER TABLE ... DETACH PARTITION ... CONCURRENTLY
Alvaro Herrera [Thu, 25 Mar 2021 21:00:28 +0000 (18:00 -0300)]
ALTER TABLE ... DETACH PARTITION ... CONCURRENTLY

Allow a partition be detached from its partitioned table without
blocking concurrent queries, by running in two transactions and only
requiring ShareUpdateExclusive in the partitioned table.

Because it runs in two transactions, it cannot be used in a transaction
block.  This is the main reason to use dedicated syntax: so that users
can choose to use the original mode if they need it.  But also, it
doesn't work when a default partition exists (because an exclusive lock
would still need to be obtained on it, in order to change its partition
constraint.)

In case the second transaction is cancelled or a crash occurs, there's
ALTER TABLE .. DETACH PARTITION .. FINALIZE, which executes the final
steps.

The main trick to make this work is the addition of column
pg_inherits.inhdetachpending, initially false; can only be set true in
the first part of this command.  Once that is committed, concurrent
transactions that use a PartitionDirectory will include or ignore
partitions so marked: in optimizer they are ignored if the row is marked
committed for the snapshot; in executor they are always included.  As a
result, and because of the way PartitionDirectory caches partition
descriptors, queries that were planned before the detach will see the
rows in the detached partition and queries that are planned after the
detach, won't.

A CHECK constraint is created that duplicates the partition constraint.
This is probably not strictly necessary, and some users will prefer to
remove it afterwards, but if the partition is re-attached to a
partitioned table, the constraint needn't be rechecked.

Author: Álvaro Herrera <[email protected]>
Reviewed-by: Amit Langote <[email protected]>
Reviewed-by: Justin Pryzby <[email protected]>
Discussion: https://postgr.es/m/20200803234854[email protected]

4 years agoDocument lock obtained during partition detach
Alvaro Herrera [Thu, 25 Mar 2021 19:30:22 +0000 (16:30 -0300)]
Document lock obtained during partition detach

On partition detach, we acquire a SHARE lock on all tables that
reference the partitioned table that we're detaching a partition from,
but failed to document this fact.  My oversight in commit f56f8f8da6af.
Repair.  Backpatch to 12.

Author: Álvaro Herrera <[email protected]>
Discussion: https://postgr.es/m/20210325180244[email protected]

4 years agoAdd comments for AlteredTableInfo->rel
Alvaro Herrera [Thu, 25 Mar 2021 19:07:15 +0000 (16:07 -0300)]
Add comments for AlteredTableInfo->rel

The prior commit which introduced it was pretty squalid in terms of
code documentation, so add some comments.

4 years agoLet ALTER TABLE Phase 2 routines manage the relation pointer
Alvaro Herrera [Thu, 25 Mar 2021 18:56:11 +0000 (15:56 -0300)]
Let ALTER TABLE Phase 2 routines manage the relation pointer

Struct AlteredRelationInfo gains a new Relation member, to be used only
by Phase 2 (ATRewriteCatalogs); this allows ATExecCmd() subroutines open
and close the relation internally.

A future commit will use this facility to implement an ALTER TABLE
subcommand that closes and reopens the relation across transaction
boundaries.

(It is possible to keep the relation open past phase 2 to be used by
phase 3 instead of having to reopen it that point, but there are some
minor complications with that; it's not clear that there is much to be
won from doing that, though.)

Author: Álvaro Herrera <[email protected]>
Discussion: https://postgr.es/m/20200803234854[email protected]

4 years agoRework HeapTupleHeader macros to reuse itemptr.h
Alvaro Herrera [Mon, 22 Feb 2021 20:21:22 +0000 (17:21 -0300)]
Rework HeapTupleHeader macros to reuse itemptr.h

The original definitions pointlessly disregarded existing ItemPointer
macros that do the same thing.

Reported-by: Michael Paquier <[email protected]>
Discussion: https://postgr.es/m/20210222201557[email protected]

4 years agoRemove StoreSingleInheritance reimplementation
Alvaro Herrera [Thu, 25 Mar 2021 13:47:38 +0000 (10:47 -0300)]
Remove StoreSingleInheritance reimplementation

I introduced this duplicate code in commit 8b08f7d4820f for no good
reason.  Remove it, and backpatch to 11 where it was introduced.

Author: Álvaro Herrera <[email protected]>

4 years agoTrim some extra whitespace in parser file
Peter Eisentraut [Thu, 25 Mar 2021 09:17:52 +0000 (10:17 +0100)]
Trim some extra whitespace in parser file

4 years agoRename a parse node to be more general
Peter Eisentraut [Thu, 25 Mar 2021 09:06:32 +0000 (10:06 +0100)]
Rename a parse node to be more general

A WHERE clause will be used for row filtering in logical replication.
We already have a similar node: 'WHERE (condition here)'.  Let's
rename the node to a generic name and use it for row filtering too.

Author: Euler Taveira <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/CAHE3wggb715X+mK_DitLXF25B=jE6xyNCH4YOwM860JR7HarGQ@mail.gmail.com

4 years agoSanitize the term "combo CID" in code comments
Michael Paquier [Thu, 25 Mar 2021 07:08:03 +0000 (16:08 +0900)]
Sanitize the term "combo CID" in code comments

Combo CIDs were referred in the code comments using different terms
across various places of the code, so unify a bit the term used with
what is currently in use in some of the READMEs.

Author: "Hou, Zhijie"
Discussion: https://postgr.es/m/1d42865c91404f46af4562532fdbea31@G08CNEXMBPEKD05.g08.fujitsu.local

4 years agoFix bug in WAL replay of COMMIT_TS_SETTS record.
Fujii Masao [Thu, 25 Mar 2021 02:23:30 +0000 (11:23 +0900)]
Fix bug in WAL replay of COMMIT_TS_SETTS record.

Previously the WAL replay of COMMIT_TS_SETTS record called
TransactionTreeSetCommitTsData() with the argument write_xlog=true,
which generated and wrote new COMMIT_TS_SETTS record.
This should not be acceptable because it's during recovery.

This commit fixes the WAL replay of COMMIT_TS_SETTS record
so that it calls TransactionTreeSetCommitTsData() with write_xlog=false
and doesn't generate new WAL during recovery.

Back-patch to all supported branches.

Reported-by: lx zou <[email protected]>
Author: Fujii Masao
Reviewed-by: Alvaro Herrera
Discussion: https://postgr.es/m/16931-620d0f2fdc6108f1@postgresql.org

4 years agoImprove connection denied error message during recovery.
Fujii Masao [Thu, 25 Mar 2021 01:41:28 +0000 (10:41 +0900)]
Improve connection denied error message during recovery.

Previously when an archive recovery or a standby was starting and
reached the consistent recovery state but hot_standby was configured
to off, the error message when a client connectted was "the database
system is starting up", which was needless confusing and not really
all that accurate either.

This commit improves the connection denied error message during
recovery, as follows, so that the users immediately know that their
servers are configured to deny those connections.

- If hot_standby is disabled, the error message "the database system
  is not accepting connections" and the detail message "Hot standby
  mode is disabled." are output when clients connect while an archive
  recovery or a standby is running.

- If hot_standby is enabled, the error message "the database system
  is not yet accepting connections" and the detail message
  "Consistent recovery state has not been yet reached." are output
  when clients connect until the consistent recovery state is reached
  and postmaster starts accepting read only connections.

This commit doesn't change the connection denied error message of
"the database system is starting up" during normal server startup and
crash recovery. Because it's still suitable for those situations.

Author: James Coleman
Reviewed-by: Alvaro Herrera, Andres Freund, David Zhang, Tom Lane, Fujii Masao
Discussion: https://postgr.es/m/CAAaqYe8h5ES_B=F_zDT+Nj9XU7YEwNhKhHA2RE4CFhAQ93hfig@mail.gmail.com

4 years agoAllow for installation-aware instances of PostgresNode
Andrew Dunstan [Wed, 24 Mar 2021 22:52:25 +0000 (18:52 -0400)]
Allow for installation-aware instances of PostgresNode

Currently instances of PostgresNode find their Postgres executables in
the PATH of the caller. This modification allows for instances that know
the installation path they are supposed to use, and the module adjusts
the environment of methods that call Postgres executables appropriately.

This facility is activated by passing the installation path to the
constructor:

  my $node = PostgresNode->get_new_node('mynode',
     installation_path => '/path/to/installation');

This makes a number of things substantially easier, including

. testing third party modules
. testing different versions of postgres together
. testing different builds of postgres together

Discussion: https://postgr.es/m/a94c74f9-6b71-1957-7973-a734ea3cbef1@dunslane.net

Reviewed-By: Alvaro Herrera, Michael Paquier, Dagfinn Ilmari Mannsåker
4 years agoNeed to step forward in the loop to get to an end.
Michael Meskes [Wed, 24 Mar 2021 21:06:31 +0000 (22:06 +0100)]
Need to step forward in the loop to get to an end.

4 years agoAdd DECLARE STATEMENT command to ECPG
Michael Meskes [Wed, 24 Mar 2021 19:48:20 +0000 (20:48 +0100)]
Add DECLARE STATEMENT command to ECPG

This command declares a SQL identifier for a SQL statement to be used in other
embedded SQL statements. The identifier is linked to a connection.

Author: Hayato Kuroda <[email protected]>
Reviewed-by: Shawn Wang <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/TY2PR01MB24438A52DB04E71D0E501452F5630@TY2PR01MB2443.jpnprd01.prod.outlook.com

4 years agoFix stray double semicolons
Peter Eisentraut [Wed, 24 Mar 2021 19:42:05 +0000 (20:42 +0100)]
Fix stray double semicolons

Reported-by: John Naylor <[email protected]>
4 years agodoc: Fix typo
Peter Eisentraut [Wed, 24 Mar 2021 19:41:18 +0000 (20:41 +0100)]
doc: Fix typo

Reported-by: Erik Rijkers <[email protected]>
4 years agoChange checkpoint_completion_target default to 0.9
Stephen Frost [Wed, 24 Mar 2021 17:07:51 +0000 (13:07 -0400)]
Change checkpoint_completion_target default to 0.9

Common recommendations are that the checkpoint should be spread out as
much as possible, provided we avoid having it take too long.  This
change updates the default to 0.9 (from 0.5) to match that
recommendation.

There was some debate about possibly removing the option entirely but it
seems there may be some corner-cases where having it set much lower to
try to force the checkpoint to be as fast as possible could result in
fewer periods of time of reduced performance due to kernel flushing.
General agreement is that the "spread more" is the preferred approach
though and those who need to tune away from that value are much less
common.

Reviewed-By: Michael Paquier, Peter Eisentraut, Tom Lane, David Steele,
Nathan Bossart
Discussion: https://postgr.es/m/20201207175329.GM16415%40tamriel.snowman.net

4 years agoTidy up more loose ends related to configurable TOAST compression.
Robert Haas [Wed, 24 Mar 2021 16:36:08 +0000 (12:36 -0400)]
Tidy up more loose ends related to configurable TOAST compression.

Change the default_toast_compression GUC to be an enum rather than
a string. Earlier, uncommitted versions of the patch supported using
CREATE ACCESS METHOD to add new compression methods to a running
system, but that idea was dropped before commit. So, we can simplify
the GUC handling as well, which has the nice side effect of improving
the error messages.

While updating the documentation to reflect the new GUC type, also
move it back to the right place in the list. I moved this while
revising what became commit 24f0e395ac5892cd12e8914646fe921fac5ba23d,
but apparently the intended ordering is "alphabetical" rather than
"whatever Robert thinks looks nice."

Rejigger things to avoid having access/toast_compression.h depend on
utils/guc.h, so that we don't end up with every file that includes
it also depending on something largely unrelated. Move a few
inline functions back into the C source file partly to help reduce
dependencies and partly just to avoid clutter. A few very minor
cosmetic fixes.

Original patch by Justin Pryzby, but very heavily edited by me,
and reverse reviewed by him and also reviewed by by Tom Lane.

Discussion: http://postgr.es/m/CA+TgmoYp=GT_ztUCeZg2i4hkHAQv8o=-nVJ1-TKWTG1zQOmOpg@mail.gmail.com

4 years agoAdd date_bin function
Peter Eisentraut [Wed, 24 Mar 2021 15:16:14 +0000 (16:16 +0100)]
Add date_bin function

Similar to date_trunc, but allows binning by an arbitrary interval
rather than just full units.

Author: John Naylor <[email protected]>
Reviewed-by: David Fetter <[email protected]>
Reviewed-by: Isaac Morland <[email protected]>
Reviewed-by: Tom Lane <[email protected]>
Reviewed-by: Artur Zakirov <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/CACPNZCt4buQFRgy6DyjuZS-2aPDpccRkrJBmgUfwYc1KiaXYxg@mail.gmail.com

4 years agoImprove an error message
Peter Eisentraut [Wed, 24 Mar 2021 07:02:06 +0000 (08:02 +0100)]
Improve an error message

Make it the same as another nearby message.

4 years agoRevert "Enable parallel SELECT for "INSERT INTO ... SELECT ..."."
Amit Kapila [Wed, 24 Mar 2021 05:40:12 +0000 (11:10 +0530)]
Revert "Enable parallel SELECT for "INSERT INTO ... SELECT ..."."

To allow inserts in parallel-mode this feature has to ensure that all the
constraints, triggers, etc. are parallel-safe for the partition hierarchy
which is costly and we need to find a better way to do that. Additionally,
we could have used existing cached information in some cases like indexes,
domains, etc. to determine the parallel-safety.

List of commits reverted, in reverse chronological order:

ed62d3737c Doc: Update description for parallel insert reloption.
c8f78b6161 Add a new GUC and a reloption to enable inserts in parallel-mode.
c5be48f092 Improve FK trigger parallel-safety check added by 05c8482f7f.
e2cda3c20a Fix use of relcache TriggerDesc field introduced by commit 05c8482f7f.
e4e87a32cc Fix valgrind issue in commit 05c8482f7f.
05c8482f7f Enable parallel SELECT for "INSERT INTO ... SELECT ...".

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

4 years agoRename wait event WalrcvExit to WalReceiverExit.
Fujii Masao [Wed, 24 Mar 2021 01:37:54 +0000 (10:37 +0900)]
Rename wait event WalrcvExit to WalReceiverExit.

Commit de829ddf23 added wait event WalrcvExit. But its name is not
consistent with other wait events like WalReceiverMain or
WalReceiverWaitStart, etc. So this commit renames WalrcvExit to
WalReceiverExit.

Author: Fujii Masao
Reviewed-by: Thomas Munro
Discussion: https://postgr.es/m/cced9995-8fa2-7b22-9d91-3f22a2b8c23c@oss.nttdata.com

4 years agoLog when GetNewOidWithIndex() fails to find unused OID many times.
Fujii Masao [Wed, 24 Mar 2021 01:36:56 +0000 (10:36 +0900)]
Log when GetNewOidWithIndex() fails to find unused OID many times.

GetNewOidWithIndex() generates a new OID one by one until it finds
one not in the relation. If there are very long runs of consecutive
existing OIDs, GetNewOidWithIndex() needs to iterate many times
in the loop to find unused OID. Since TOAST table can have a large
number of entries and there can be such long runs of OIDs, there is
the case where it takes so many iterations to find new OID not in
TOAST table. Furthermore if all (i.e., 2^32) OIDs are already used,
GetNewOidWithIndex() enters something like busy loop and repeats
the iterations until at least one OID is marked as unused.

There are some reported troubles caused by a large number of
iterations in GetNewOidWithIndex(). For example, when inserting
a billion of records into the table, all the backends doing that
insertion operation got hang with 100% CPU usage at some point.

Previously there was no easy way to detect that GetNewOidWithIndex()
failed to find unused OID many times. So, for example, gdb full
backtrace of hanged backends needed to be taken, in order to
investigate that trouble. This is inconvenient and may not be
available in some production environments.

To provide easy way for that, this commit makes GetNewOidWithIndex()
log that it iterates more than GETNEWOID_LOG_THRESHOLD but have
not yet found OID unused in the relation. Also this commit makes
it repeat logging with exponentially increasing intervals until
it iterates more than GETNEWOID_LOG_MAX_INTERVAL, and makes it
finally repeat logging every GETNEWOID_LOG_MAX_INTERVAL unless
an unused OID is found. Those macro variables are used not to
fill up the server log with the similar messages.

In the discusion at pgsql-hackers, there was another idea to report
the lots of iterations in GetNewOidWithIndex() via wait event.
But since GetNewOidWithIndex() traverses indexes to find unused
OID and which will do I/O, acquire locks, etc, which will overwrite
the wait event and reset it to nothing once done. So that idea
doesn't work well, and we didn't adopt it.

Author: Tomohiro Hiramitsu
Reviewed-by: Tatsuhito Kasahara, Kyotaro Horiguchi, Tom Lane, Fujii Masao
Discussion: https://postgr.es/m/16722-93043fb459a41073@postgresql.org

4 years agoReword slightly logs generated for index stats in autovacuum
Michael Paquier [Wed, 24 Mar 2021 00:36:03 +0000 (09:36 +0900)]
Reword slightly logs generated for index stats in autovacuum

Using "remain" is confusing, as it implies that the index file can
shrink.  Instead, use "in total".

Per discussion with Peter Geoghegan.

Discussion: https://postgr.es/m/CAH2-WzkYgHZzpGOwR14CScJsjaQpvJrEkEfkh_=wGhzLb=yVdQ@mail.gmail.com

4 years agoAllow composite types in catalog bootstrap
Tomas Vondra [Tue, 23 Mar 2021 23:47:50 +0000 (00:47 +0100)]
Allow composite types in catalog bootstrap

When resolving types during catalog bootstrap, try to reload the pg_type
contents if a type is not found. That allows catalogs to contain
composite types, e.g. row types for other catalogs.

Author: Justin Pryzby
Reviewed-by: Dean Rasheed, Tomas Vondra
Discussion: https://postgr.es/m/ad7891d2-e90c-b446-9fe2-7419143847d7%40enterprisedb.com

4 years agoConvert Typ from array to list in bootstrap
Tomas Vondra [Tue, 23 Mar 2021 23:47:38 +0000 (00:47 +0100)]
Convert Typ from array to list in bootstrap

It's a bit easier and more convenient to free and reload a List,
compared to a plain array. This will be helpful when allowing catalogs
to contain composite types.

Author: Justin Pryzby
Reviewed-by: Dean Rasheed, Tomas Vondra
Discussion: https://postgr.es/m/ad7891d2-e90c-b446-9fe2-7419143847d7%40enterprisedb.com

4 years agonbtree VACUUM: Cope with buggy opclasses.
Peter Geoghegan [Tue, 23 Mar 2021 23:09:51 +0000 (16:09 -0700)]
nbtree VACUUM: Cope with buggy opclasses.

Teach nbtree VACUUM to press on with vacuuming in the event of a page
deletion attempt that fails to "re-find" a downlink for its child/target
page.

There is no good reason to treat this as an irrecoverable error.  But
there is a good reason not to: pressing on at this point removes any
question of VACUUM not making progress solely due to misbehavior from
user-defined operator class code.

Discussion: https://postgr.es/m/CAH2-Wzma5G9CTtMjbrXTwOym+U=aWg-R7=-htySuztgoJLvZXg@mail.gmail.com

4 years agoImprove pg_amcheck's TAP test 003_check.pl.
Robert Haas [Tue, 23 Mar 2021 18:57:45 +0000 (14:57 -0400)]
Improve pg_amcheck's TAP test 003_check.pl.

Disable autovacuum, because we don't want it to run against
intentionally corrupted tables. Also, before corrupting the tables,
run pg_amcheck and ensure that it passes. Otherwise, if something
unexpected happens when we check the corrupted tables, it's not so
clear whether it would have also happened before we corrupted
them.

Mark Dilger

Discussion: http://postgr.es/m/AA5506CE-7D2A-42E4-A51D-358635E3722D@enterprisedb.com

4 years agoFix psql's \connect command some more.
Tom Lane [Tue, 23 Mar 2021 18:27:50 +0000 (14:27 -0400)]
Fix psql's \connect command some more.

Jasen Betts reported yet another unintended side effect of commit
85c54287a: reconnecting with "\c service=whatever" did not have the
expected results.  The reason is that starting from the output of
PQconndefaults() effectively allows environment variables (such
as PGPORT) to override entries in the service file, whereas the
normal priority is the other way around.

Not using PQconndefaults at all would require yet a third main code
path in do_connect's parameter setup, so I don't really want to fix
it that way.  But we can have the logic effectively ignore all the
default values for just a couple more lines of code.

This patch doesn't change the behavior for "\c -reuse-previous=on
service=whatever".  That remains significantly different from before
85c54287a, because many more parameters will be re-used, and thus
not be possible for service entries to replace.  But I think this
is (mostly?) intentional.  In any case, since libpq does not report
where it got parameter values from, it's hard to do differently.

Per bug #16936 from Jasen Betts.  As with the previous patches,
back-patch to all supported branches.  (9.5 is unfortunately now
out of support, so this won't get fixed there.)

Discussion: https://postgr.es/m/16936-3f524322a53a29f0@postgresql.org

4 years agoAvoid possible crash while finishing up a heap rewrite.
Tom Lane [Tue, 23 Mar 2021 15:24:16 +0000 (11:24 -0400)]
Avoid possible crash while finishing up a heap rewrite.

end_heap_rewrite was not careful to ensure that the target relation
is open at the smgr level before performing its final smgrimmedsync.
In ordinary cases this is no problem, because it would have been
opened earlier during the rewrite.  However a crash can be reproduced
by re-clustering an empty table with CLOBBER_CACHE_ALWAYS enabled.

Although that exact scenario does not crash in v13, I think that's
a chance result of unrelated planner changes, and the problem is
likely still reachable with other test cases.  The true proximate
cause of this failure is commit c6b92041d, which replaced a call to
heap_sync (which was careful about opening smgr) with a direct call
to smgrimmedsync.  Hence, back-patch to v13.

Amul Sul, per report from Neha Sharma; cosmetic changes
and test case by me.

Discussion: https://postgr.es/m/CANiYTQsU7yMFpQYnv=BrcRVqK_3U3mtAzAsJCaqtzsDHfsUbdQ@mail.gmail.com

4 years agopgcrypto: Check for error return of px_cipher_decrypt()
Peter Eisentraut [Tue, 23 Mar 2021 10:35:12 +0000 (11:35 +0100)]
pgcrypto: Check for error return of px_cipher_decrypt()

This has previously not been a problem (that anyone ever reported),
but in future OpenSSL versions (3.0.0), where legacy ciphers are/can
be disabled, this is the place where this is reported.  So we need to
catch the error here, otherwise the higher-level functions would
return garbage.  The nearby encryption code already handled errors
similarly.

Reviewed-by: Daniel Gustafsson <[email protected]>
Discussion: https://www.postgresql.org/message-id/9e9c431c-0adc-7a6d-9b1a-915de1ba3fe7@enterprisedb.com

4 years agoAdd bit_count SQL function
Peter Eisentraut [Tue, 23 Mar 2021 07:45:51 +0000 (08:45 +0100)]
Add bit_count SQL function

This function for bit and bytea counts the set bits in the bit or byte
string.  Internally, we use the existing popcount functionality.

For the name, after some discussion, we settled on bit_count, which
also exists with this meaning in MySQL, Java, and Python.

Author: David Fetter <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/20201230105535[email protected]

4 years agoAdd per-index stats information in verbose logs of autovacuum
Michael Paquier [Tue, 23 Mar 2021 04:25:14 +0000 (13:25 +0900)]
Add per-index stats information in verbose logs of autovacuum

Once a relation's autovacuum is completed, the logs include more
information about this relation state if the threshold of
log_autovacuum_min_duration (or its relation option) is reached, with
for example contents about the statistics of the VACUUM operation for
the relation, WAL and system usage.

This commit adds more information about the statistics of the relation's
indexes, with one line of logs generated for each index.  The index
stats were already calculated, but not printed in the context of
autovacuum yet.  While on it, some refactoring is done to keep track of
the index statistics directly within LVRelStats, simplifying some
routines related to parallel VACUUMs.

Author: Masahiko Sawada
Reviewed-by: Michael Paquier, Euler Taveira
Discussion: https://postgr.es/m/CAD21AoAy6SxHiTivh5yAPJSUE4S=QRPpSZUdafOSz0R+fRcM6Q@mail.gmail.com

4 years agoFix dangling pointer reference in stream_cleanup_files.
Amit Kapila [Tue, 23 Mar 2021 04:13:33 +0000 (09:43 +0530)]
Fix dangling pointer reference in stream_cleanup_files.

We can't access the entry after it is removed from dynahash.

Author: Peter Smith
Discussion: https://postgr.es/m/CAHut+Ps-pL++f6CJwPx2+vUqXuew=Xt-9Bi-6kCyxn+Fwi2M7w@mail.gmail.com

4 years agoUse correct spelling of statistics kind
Tomas Vondra [Tue, 23 Mar 2021 03:45:26 +0000 (04:45 +0100)]
Use correct spelling of statistics kind

A couple error messages and comments used 'statistic kind', not the
correct 'statistics kind'. Fix and backpatch all the way back to 10,
where extended statistics were introduced.

Backpatch-through: 10

4 years agoChange the type of WalReceiverWaitStart wait event from Client to IPC.
Fujii Masao [Tue, 23 Mar 2021 01:09:42 +0000 (10:09 +0900)]
Change the type of WalReceiverWaitStart wait event from Client to IPC.

Previously the type of this wait event was Client. But while this
wait event is being reported, walreceiver process is waiting for
the startup process to set initial data for streaming replication.
It's not waiting for any activity on a socket connected to a user
application or walsender. So this commit changes the type for
WalReceiverWaitStart wait event to IPC.

Author: Fujii Masao
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/cdacc27c-37ff-f1a4-20e2-ce19933abfcc@oss.nttdata.com

4 years agopg_waldump: Fix bug in per-record statistics.
Fujii Masao [Tue, 23 Mar 2021 00:53:08 +0000 (09:53 +0900)]
pg_waldump: Fix bug in per-record statistics.

pg_waldump --stats=record identifies a record by a combination
of the RmgrId and the four bits of the xl_info field of the record.
But XACT records use the first bit of those four bits for an optional
flag variable, and the following three bits for the opcode to
identify a record. So previously the same type of XACT record
could have different four bits (three bits are the same but the
first one bit is different), and which could cause
pg_waldump --stats=record to show two lines of per-record statistics
for the same XACT record. This is a bug.

This commit changes pg_waldump --stats=record so that it processes
only XACT record differently, i.e., filters the opcode out of xl_info
and uses a combination of the RmgrId and those three bits as
the identifier of a record, only for XACT record. For other records,
the four bits of the xl_info field are still used.

Back-patch to all supported branches.

Author: Kyotaro Horiguchi
Reviewed-by: Shinya Kato, Fujii Masao
Discussion: https://postgr.es/m/2020100913412132258847@highgo.ca

4 years agoAdd macro RelationIsPermanent() to report relation permanence
Bruce Momjian [Tue, 23 Mar 2021 00:22:48 +0000 (20:22 -0400)]
Add macro RelationIsPermanent() to report relation permanence

Previously, to check relation permanence, the Relation's Form_pg_class
structure member relpersistence was compared to the value
RELPERSISTENCE_PERMANENT ("p"). This commit adds the macro
RelationIsPermanent() and is used in appropirate places to simplify the
code.  This matches other RelationIs* macros.

This macro will be used in more places in future cluster file encryption
patches.

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

4 years agoOptimize allocations in bringetbitmap
Tomas Vondra [Mon, 22 Mar 2021 23:47:06 +0000 (00:47 +0100)]
Optimize allocations in bringetbitmap

The bringetbitmap function allocates memory for various purposes, which
may be quite expensive, depending on the number of scan keys. Instead of
allocating them separately, allocate one bit chunk of memory an carve it
into smaller pieces as needed - all the pieces have the same lifespan,
and it saves quite a bit of CPU and memory overhead.

Author: Tomas Vondra <[email protected]>
Reviewed-by: Alvaro Herrera <[email protected]>
Reviewed-by: Mark Dilger <[email protected]>
Reviewed-by: Alexander Korotkov <[email protected]>
Reviewed-by: Masahiko Sawada <[email protected]>
Reviewed-by: John Naylor <[email protected]>
Discussion: https://postgr.es/m/c1138ead-7668-f0e1-0638-c3be3237e812@2ndquadrant.com

4 years agoMove IS [NOT] NULL handling from BRIN support functions
Tomas Vondra [Mon, 22 Mar 2021 23:45:33 +0000 (00:45 +0100)]
Move IS [NOT] NULL handling from BRIN support functions

The handling of IS [NOT] NULL clauses is independent of an opclass, and
most of the code was exactly the same in both minmax and inclusion. So
instead move the code from support procedures to the AM.

This simplifies the code - especially the support procedures - quite a
bit, as they don't need to care about NULL values and flags at all. It
also means the IS [NOT] NULL clauses can be evaluated without invoking
the support procedure.

Author: Tomas Vondra <[email protected]>
Author: Nikita Glukhov <[email protected]>
Reviewed-by: Nikita Glukhov <[email protected]>
Reviewed-by: Mark Dilger <[email protected]>
Reviewed-by: Alexander Korotkov <[email protected]>
Reviewed-by: Masahiko Sawada <[email protected]>
Reviewed-by: John Naylor <[email protected]>
Discussion: https://postgr.es/m/c1138ead-7668-f0e1-0638-c3be3237e812@2ndquadrant.com

4 years agoPass all scan keys to BRIN consistent function at once
Tomas Vondra [Mon, 22 Mar 2021 23:12:19 +0000 (00:12 +0100)]
Pass all scan keys to BRIN consistent function at once

This commit changes how we pass scan keys to BRIN consistent function.
Instead of passing them one by one, we now pass all scan keys for a
given attribute at once. That makes the consistent function a bit more
complex, as it has to loop through the keys, but it does allow more
elaborate opclasses that can use multiple keys to eliminate ranges much
more effectively.

The existing BRIN opclasses (minmax, inclusion) don't really benefit
from this change. The primary purpose is to allow future opclases to
benefit from seeing all keys at once.

This does change the BRIN API, because the signature of the consistent
function changes (a new parameter with number of scan keys). So this
breaks existing opclasses, and will require supporting two variants of
the code for different PostgreSQL versions. We've considered supporting
two variants of the consistent, but we've decided not to do that.
Firstly, there's another patch that moves handling of NULL values from
the opclass, which means the opclasses need to be updated anyway.
Secondly, we're not aware of any out-of-core BRIN opclasses, so it does
not seem worth the extra complexity.

Bump catversion, because of pg_proc changes.

Author: Tomas Vondra <[email protected]>
Reviewed-by: Alvaro Herrera <[email protected]>
Reviewed-by: Mark Dilger <[email protected]>
Reviewed-by: Alexander Korotkov <[email protected]>
Reviewed-by: John Naylor <[email protected]>
Reviewed-by: Nikita Glukhov <[email protected]>
Discussion: https://postgr.es/m/c1138ead-7668-f0e1-0638-c3be3237e812@2ndquadrant.com

4 years agoMove bsearch_arg to src/port
Tomas Vondra [Mon, 22 Mar 2021 23:11:20 +0000 (00:11 +0100)]
Move bsearch_arg to src/port

Until now the bsearch_arg function was used only in extended statistics
code, so it was defined in that code.  But we already have qsort_arg in
src/port, so let's move it next to it.

4 years agoShort-circuit slice requests that are for more than the object's size.
Tom Lane [Mon, 22 Mar 2021 18:01:20 +0000 (14:01 -0400)]
Short-circuit slice requests that are for more than the object's size.

substring(), and perhaps other callers, isn't careful to pass a
slice length that is no more than the datum's true size.  Since
toast_decompress_datum_slice's children will palloc the requested
slice length, this can waste memory.  Also, close study of the liblz4
documentation suggests that it is dependent on the caller to not ask
for more than the correct amount of decompressed data; this squares
with observed misbehavior with liblz4 1.8.3.  Avoid these problems
by switching to the normal full-decompression code path if the
slice request is >= datum's decompressed size.

Tom Lane and Dilip Kumar

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

4 years agoMostly-cosmetic adjustments of TOAST-related macros.
Tom Lane [Mon, 22 Mar 2021 17:43:10 +0000 (13:43 -0400)]
Mostly-cosmetic adjustments of TOAST-related macros.

The authors of bbe0a81db hadn't quite got the idea that macros named
like SOMETHING_4B_C were only meant for internal endianness-related
details in postgres.h.  Choose more legible names for macros that are
intended to be used elsewhere.  Rearrange postgres.h a bit to clarify
the separation between those internal macros and ones intended for
wider use.

Also, avoid using the term "rawsize" for true decompressed size;
we've used "extsize" for that, because "rawsize" generally denotes
total Datum size including header.  This choice seemed particularly
unfortunate in tests that were comparing one of these meanings to
the other.

This patch includes a couple of not-purely-cosmetic changes: be
sure that the shifts aligning compression methods are unsigned
(not critical today, but will be when compression method 2 exists),
and fix broken definition of VARATT_EXTERNAL_GET_COMPRESSION (now
VARATT_EXTERNAL_GET_COMPRESS_METHOD), whose callers worked only
accidentally.

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

4 years agoRemove useless configure probe for <lz4/lz4.h>.
Tom Lane [Mon, 22 Mar 2021 15:20:44 +0000 (11:20 -0400)]
Remove useless configure probe for <lz4/lz4.h>.

This seems to have been just copied-and-pasted from some other
header checks.  But our C code is entirely unprepared to support
such a header name, so it's only wasting cycles to look for it.
If we did need to support it, some #ifdefs would be required.

(A quick trawl at codesearch.debian.net finds some packages that
reference lz4/lz4.h; but they use *only* that spelling, and
appear to be intending to reference their own copy rather than
a system-level installation of liblz4.  There's no evidence of
freestanding installations that require this spelling.)

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

4 years agoError on invalid TOAST compression in CREATE or ALTER TABLE.
Robert Haas [Mon, 22 Mar 2021 14:57:08 +0000 (10:57 -0400)]
Error on invalid TOAST compression in CREATE or ALTER TABLE.

The previous coding treated an invalid compression method name as
equivalent to the default, which is certainly not right.

Justin Pryzby

Discussion: http://postgr.es/m/20210321235544[email protected]

4 years agodocs: Fix omissions related to configurable TOAST compression.
Robert Haas [Mon, 22 Mar 2021 14:34:10 +0000 (10:34 -0400)]
docs: Fix omissions related to configurable TOAST compression.

Previously, the default_toast_compression GUC was not documented,
and neither was pg_dump's new --no-toast-compression option.

Justin Pryzby and Robert Haas

Discussion: http://postgr.es/m/20210321235544[email protected]

4 years agoMore code cleanup for configurable TOAST compression.
Robert Haas [Mon, 22 Mar 2021 13:21:37 +0000 (09:21 -0400)]
More code cleanup for configurable TOAST compression.

Remove unused macro. Fix confusion about whether a TOAST compression
method is identified by an OID or a char.

Justin Pryzby

Discussion: http://postgr.es/m/20210321235544[email protected]

4 years agoFix concurrency issues with WAL segment recycling on Windows
Michael Paquier [Mon, 22 Mar 2021 05:02:26 +0000 (14:02 +0900)]
Fix concurrency issues with WAL segment recycling on Windows

This commit is mostly a revert of aaa3aed, that switched the routine
doing the internal renaming of recycled WAL segments to use on Windows a
combination of CreateHardLinkA() plus unlink() instead of rename().  As
reported by several users of Postgres 13, this is causing concurrency
issues when manipulating WAL segments, mostly in the shape of the
following error:
LOG:  could not rename file "pg_wal/000000XX000000YY000000ZZ":
Permission denied

This moves back to a logic where a single rename() (well, pgrename() for
Windows) is used.  This issue has proved to be hard to hit when I tested
it, facing it only once with an archive_command that was not able to do
its work, so it is environment-sensitive.  The reporters of this issue
have been able to confirm that the situation improved once we switched
back to a single rename().  In order to check things, I have provided to
the reporters a patched build based on 13.2 with aaa3aed reverted, to
test if the error goes away, and an unpatched build of 13.2 to test if
the error still showed up (just to make sure that I did not mess up my
build process).

Extra thanks to Fujii Masao for pointing out what looked like the
culprit commit, and to all the reporters for taking the time to test
what I have sent them.

Reported-by: Andrus, Guy Burgess, Yaroslav Pashinsky, Thomas Trenz
Reviewed-by: Tom Lane, Andres Freund
Discussion: https://postgr.es/m/3861ff1e-0923-7838-e826-094cc9bef737@hot.ee
Discussion: https://postgr.es/m/16874-c3eecd319e36a2bf@postgresql.org
Discussion: https://postgr.es/m/095ccf8d-7f58-d928-427c-b17ace23cae6@burgess.co.nz
Discussion: https://postgr.es/m/16927-67c570d968c99567%40postgresql.org
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 13

4 years agopgbench: Improve error-handling in \sleep command.
Fujii Masao [Mon, 22 Mar 2021 03:02:44 +0000 (12:02 +0900)]
pgbench: Improve error-handling in \sleep command.

This commit improves pgbench \sleep command so that it handles
the following three cases more properly.

(1) When only one argument was specified in \sleep command and
    it's not a number, previously pgbench reported a confusing error
    message like "unrecognized time unit, must be us, ms or s".
    This commit fixes this so that more proper error message like
    "invalid sleep time, must be an integer" is reported.

(2) When two arguments were specified in \sleep command and
    the first argument was not a number, previously pgbench treated
    that argument as the sleep time 0. No error was reported in this
    case. This commit fixes this so that an error is thrown in this
    case.

(3) When a variable was specified as the first argument in \sleep
    command and the variable stored non-digit value, previously
    pgbench treated that argument as the sleep time 0. No error
    was reported in this case. This commit fixes this so that
    an error is thrown in this case.

Author: Kota Miyake
Reviewed-by: Hayato Kuroda, Alvaro Herrera, Fujii Masao
Discussion: https://postgr.es/m/23b254daf20cec4332a2d9168505dbc9@oss.nttdata.com

4 years agoMake a test endure log_error_verbosity=verbose.
Noah Misch [Mon, 22 Mar 2021 02:09:29 +0000 (19:09 -0700)]
Make a test endure log_error_verbosity=verbose.

Back-patch to v13, which introduced the test code in question.

4 years agoFix new TAP test for 2PC transactions and PITRs on Windows
Michael Paquier [Mon, 22 Mar 2021 00:51:05 +0000 (09:51 +0900)]
Fix new TAP test for 2PC transactions and PITRs on Windows

The test added by 595b9cb forgot that on Windows it is necessary to set
up pg_hba.conf (see PostgresNode::set_replication_conf) with a specific
entry or base backups fail.  Any node that requires to support
replication just needs to pass down allows_streaming at initialization.
This updates the test to do so.  Simplify things a bit while on it.

Per buildfarm member fairywren.  Any Windows hosts running this test
would have failed, and I have reproduced the problem as well.

Backpatch-through: 10