postgresql.git
4 years agoClarify documentation around pg_dump -t option
Magnus Hagander [Tue, 6 Oct 2020 13:46:36 +0000 (15:46 +0200)]
Clarify documentation around pg_dump -t option

The behavior is different for different types of objects, so make that
more clear.

Author: Ian Barwick

4 years agoExpand installation documentation to cover binary installations
Magnus Hagander [Tue, 6 Oct 2020 12:15:32 +0000 (14:15 +0200)]
Expand installation documentation to cover binary installations

Reviewed-By: David G. Johnston, Daniel Gustafsson
4 years agoFix compilation warning in xlog.c
Michael Paquier [Tue, 6 Oct 2020 06:29:34 +0000 (15:29 +0900)]
Fix compilation warning in xlog.c

Oversight in 9d0bd95.

Reported-by: Andres Freund
Discussion: https://postgr.es/m/20201006023802.qqfi6m5bw5y77zql@alap3.anarazel.de

4 years agoTry to unbreak 021_row_visibility.pl on mingw.
Andres Freund [Tue, 6 Oct 2020 02:20:17 +0000 (19:20 -0700)]
Try to unbreak 021_row_visibility.pl on mingw.

Thanks to Andrew for proposing and testing this fix.

It's possible that we should address this on a more fundamental basis,
e.g. by configuring PerlIO to to CR/LF conversion for us, but this
approach already exists in other places. And it's nice to unbreak the
BF.

Proposed-By: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
Discussion: https://postgr.es/m/2355d1f0-0244-da9c-ef0c-7542b944e1ac@2ndQuadrant.com

4 years agopostgres_fdw: reestablish new connection if cached one is detected as broken.
Fujii Masao [Tue, 6 Oct 2020 01:31:09 +0000 (10:31 +0900)]
postgres_fdw: reestablish new connection if cached one is detected as broken.

In postgres_fdw, once remote connections are established, they are cached
and re-used for subsequent queries and transactions. There can be some
cases where those cached connections are unavaiable, for example,
by the restart of remote server. In these cases, previously an error was
reported and the query accessing to remote server failed if new remote
transaction failed to start because the cached connection was broken.

This commit improves postgres_fdw so that new connection is remade
if broken connection is detected when starting new remote transaction.
This is useful to avoid unnecessary failure of queries when connection is
broken but can be reestablished.

Author: Bharath Rupireddy, tweaked a bit by Fujii Masao
Reviewed-by: Ashutosh Bapat, Tatsuhito Kasahara, Fujii Masao
Discussion: https://postgr.es/m/CALj2ACUAi23vf1WiHNar_LksM9EDOWXcbHCo-fD4Mbr1d=78YQ@mail.gmail.com

4 years agodoc: show functions returning record types and use of ROWS FROM
Bruce Momjian [Mon, 5 Oct 2020 20:27:33 +0000 (16:27 -0400)]
doc:  show functions returning record types and use of ROWS FROM

Previously it was unclear exactly how ROWS FROM behaved and how to cast
the data types of columns returned by FROM functions.  Also document
that only non-OUT record functions can have their columns cast to data
types.

Reported-by: guyren@gmail.com
Discussion: https://postgr.es/m/158638264419.662.2482095087061084020@wrigleys.postgresql.org

Backpatch-through: 9.5

4 years agoOverhaul pg_hba.conf clientcert's API
Bruce Momjian [Mon, 5 Oct 2020 19:48:40 +0000 (15:48 -0400)]
Overhaul pg_hba.conf clientcert's API

Since PG 12, clientcert no longer supported only on/off, so remove 1/0
as possible values, and instead support only the text strings
'verify-ca' and 'verify-full'.

Remove support for 'no-verify' since that is possible by just not
specifying clientcert.

Also, throw an error if 'verify-ca' is used and 'cert' authentication is
used, since cert authentication requires verify-full.

Also improve the docs.

THIS IS A BACKWARD INCOMPATIBLE API CHANGE.

Reported-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/20200716.093012.1627751694396009053.horikyota.ntt@gmail.com

Author: Kyotaro Horiguchi

Backpatch-through: master

4 years agoInclude the process PID in assertion-failure messages.
Tom Lane [Mon, 5 Oct 2020 17:40:28 +0000 (13:40 -0400)]
Include the process PID in assertion-failure messages.

This should help to identify what happened when studying the postmaster
log after-the-fact.

While here, clean up some old comments in the same function.

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

4 years agoFix two latent(?) bugs in equivclass.c.
Tom Lane [Mon, 5 Oct 2020 17:15:39 +0000 (13:15 -0400)]
Fix two latent(?) bugs in equivclass.c.

get_eclass_for_sort_expr() computes expr_relids and nullable_relids
early on, even though they won't be needed unless we make a new
EquivalenceClass, which we often don't.  Aside from the probably-minor
inefficiency, there's a memory management problem: these bitmapsets will
be built in the caller's context, leading to dangling pointers if that
is shorter-lived than root->planner_cxt.  This would be a live bug if
get_eclass_for_sort_expr() could be called with create_it = true during
GEQO join planning.  So far as I can find, the core code never does
that, but it's hard to be sure that no extensions do, especially since
the comments make it clear that that's supposed to be a supported case.
Fix by not computing these values until we've switched into planner_cxt
to build the new EquivalenceClass.

generate_join_implied_equalities() uses inner_rel->relids to look up
relevant eclasses, but it ought to be using nominal_inner_relids.
This is presently harmless because a child RelOptInfo will always have
exactly the same eclass_indexes as its topmost parent; but that might
not be true forever, and anyway it makes the code confusing.

The first of these is old (introduced by me in f3b3b8d5b), so back-patch
to all supported branches.  The second only dates to v13, but we might
as well back-patch it to keep the code looking similar across branches.

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

4 years agoDoc: fix parameter names in the docs of a couple of functions.
Tom Lane [Mon, 5 Oct 2020 15:42:33 +0000 (11:42 -0400)]
Doc: fix parameter names in the docs of a couple of functions.

The descriptions of make_interval() and pg_options_to_table()
were randomly different from the reality embedded in pg_proc.

(These are not all the discrepancies I found in a quick search,
but the others perhaps require more discussion, since there's
at least a case to be made for changing pg_proc not the docs.)

make_interval issue noted by Thomas Kellerer.

Discussion: https://postgr.es/m/7b154ef0-9f22-90b9-7734-4bf23686695b@gmx.net

4 years agoSupport for OUT parameters in procedures
Peter Eisentraut [Mon, 5 Oct 2020 07:09:09 +0000 (09:09 +0200)]
Support for OUT parameters in procedures

Unlike for functions, OUT parameters for procedures are part of the
signature.  Therefore, they have to be listed in pg_proc.proargtypes
as well as mentioned in ALTER PROCEDURE and DROP PROCEDURE.

Reviewed-by: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/2b8490fe-51af-e671-c504-47359dc453c5@2ndquadrant.com

4 years agoImprove stability of identity.sql regression test.
Tom Lane [Mon, 5 Oct 2020 00:45:06 +0000 (20:45 -0400)]
Improve stability of identity.sql regression test.

I noticed while trying to run the regression tests under a low
geqo_threshold that one query on information_schema.columns had
unstable (as in, variable from one run to the next) output order.
This is pretty unsurprising given the complexity of the underlying
plan.  Interestingly, of this test's three nigh-identical queries on
information_schema.columns, the other two already had ORDER BY clauses
guaranteeing stable output.  Let's make this one look the same.

Back-patch to v10 where this test was added.  We've not heard field
reports of the test failing, but this experience shows that it can
happen when testing under even slightly unusual conditions.

4 years agoFix handling of redundant options with COPY for "freeze" and "header"
Michael Paquier [Mon, 5 Oct 2020 00:43:17 +0000 (09:43 +0900)]
Fix handling of redundant options with COPY for "freeze" and "header"

The handling of those options was inconsistent, as the processing used
directly the value assigned to the option to check if it was redundant,
leading to patterns like this one to succeed (note that false is
specified first):
COPY hoge to '/path/to/file/' (header off, header on);

And the opposite would fail correctly (note that true is first here):
COPY hoge to '/path/to/file/' (header on, header off);

While on it, add some tests to check for all redundant patterns with the
options of COPY.  I have gone through the code and did not notice
similar mistakes for other commands.

"header" got it wrong since b63990c, and "freeze" was wrong from the
start as of 8de72b6.  No backpatch is done per the lack of complaints.

Reported-by: Rémi Lapeyre
Discussion: https://postgr.es/m/20200929072433.GA15570@paquier.xyz
Discussion: https://postgr.es/m/0B55BD07-83E4-439F-AACC-FA2D7CF50532@lenstra.fr

4 years agoMake postgres.bki use the same literal-string syntax as postgresql.conf.
Tom Lane [Sun, 4 Oct 2020 20:09:55 +0000 (16:09 -0400)]
Make postgres.bki use the same literal-string syntax as postgresql.conf.

The BKI file's string quoting conventions were previously quite weird,
perhaps as a result of repurposing a function built to scan
single-quoted strings to scan double-quoted ones.  Change to use the
same rules as we use in GUC files, allowing some simplifications in
genbki.pl and initdb.c.

While at it, completely remove the backend's scanstr() function, which
was essentially a duplicate of the string dequoting code in guc-file.l.
Instead export that one (under a less generic name than it had) and let
bootscanner.l use it.  Now we can clarify that scansup.c exists only to
support the main lexer. We could alternatively have removed GUC_scanstr,
but this way seems better since the previous arrangement could mislead
a reader into thinking that scanstr() had something to do with the main
lexer's handling of string literals.  Maybe it did once, but if so it
was a long time ago.

This patch does not bump catversion, since the initially-installed
catalog contents don't change.  Note however that successful initdb
after applying this patch will require up-to-date postgres.bki as well
as postgres and initdb executables.

In passing, remove a bunch of very-long-obsolete #include's in
bootparse.y and bootscanner.l.

John Naylor

Discussion: https://postgr.es/m/CACPNZCtDpd18T0KATTmCggO2GdVC4ow86ypiq5ENff1VnauL8g@mail.gmail.com

4 years agoImprove <xref> vs. <command> formatting in the documentation
Peter Eisentraut [Sat, 3 Oct 2020 14:16:51 +0000 (16:16 +0200)]
Improve <xref> vs. <command> formatting in the documentation

SQL commands are generally marked up as <command>, except when a link
to a reference page is used using <xref>.  But the latter doesn't
create monospace markup, so this looks strange especially when a
paragraph contains a mix of links and non-links.

We considered putting <command> in the <refentrytitle> on the target
side, but that creates some formatting side effects elsewhere.
Generally, it seems safer to solve this on the link source side.

We can't put the <xref> inside the <command>; the DTD doesn't allow
this.  DocBook 5 would allow the <command> to have the linkend
attribute itself, but we are not there yet.

So to solve this for now, convert the <xref>s to <link> plus
<command>.  This gives the correct look and also gives some more
flexibility what we can put into the link text (e.g., subcommands or
other clauses).  In the future, these could then be converted to
DocBook 5 style.

I haven't converted absolutely all xrefs to SQL command reference
pages, only those where we care about the appearance of the link text
or where it was otherwise appropriate to make the appearance match a
bit better.  Also in some cases, the links where repetitive, so in
those cases the links where just removed and replaced by a plain
<command>.  In cases where we just want the link and don't
specifically care about the generated link text (typically phrased
"for further information see <xref ...>") the xref is kept.

Reported-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Discussion: https://www.postgresql.org/message-id/flat/87o8pco34z.fsf@wibble.ilmari.org

4 years agodoc: libpq connection options can override command-line flags
Bruce Momjian [Sat, 3 Oct 2020 02:19:31 +0000 (22:19 -0400)]
doc:  libpq connection options can override command-line flags

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/16486-b9c93d71c02c4907@postgresql.org

Backpatch-through: 9.5

4 years agodoc: clarify the use of ssh port forwarding
Bruce Momjian [Sat, 3 Oct 2020 01:39:33 +0000 (21:39 -0400)]
doc:  clarify the use of ssh port forwarding

Reported-by: karimelghazouly@gmail.com
Discussion: https://postgr.es/m/159854511172.24991.4373145230066586863@wrigleys.postgresql.org

Backpatch-through: 9.5

4 years agoTidy up error reporting when converting PL/Python arrays.
Heikki Linnakangas [Fri, 2 Oct 2020 15:23:39 +0000 (18:23 +0300)]
Tidy up error reporting when converting PL/Python arrays.

Use PLy_elog() only when a call to a Python C API function failed, and
ereport() for other errors. Add an error code to the "wrong length of
inner sequence" ereport().

Reviewed-by: Daniel Gustafsson
Discussion: https://www.postgresql.org/message-id/B8B72889-D6D7-48FF-B782-D670A6CA4D37%40yesql.se

4 years agodoc: Improve some documentation about HA and replication
Michael Paquier [Fri, 2 Oct 2020 01:36:35 +0000 (10:36 +0900)]
doc: Improve some documentation about HA and replication

This clarifies some wording in the description of the options available
as replication solutions.  While on it, this replaces some instances of
"master" with "primary", for consistency with recent changes like
9e101cf.

Author: Robert Treat
Reviewed-by: Magnus Hagander, Michael Paquier
Discussion: https://postgr.es/m/CAJSLCQ2TPaK_K8raofCamrqELCxY-H6mJrpDNRzc-LKpPY7c+g@mail.gmail.com

4 years agoAdd pg_stat_wal statistics view.
Fujii Masao [Fri, 2 Oct 2020 01:17:11 +0000 (10:17 +0900)]
Add pg_stat_wal statistics view.

This view shows the statistics about WAL activity. Currently it has only
two columns: wal_buffers_full and stats_reset. wal_buffers_full column
indicates the number of times WAL data was written to the disk because
WAL buffers got full. This information is useful when tuning wal_buffers.
stats_reset column indicates the time at which these statistics were
last reset.

pg_stat_wal view is also the basic infrastructure to expose other
various statistics about WAL activity later.

Bump PGSTAT_FILE_FORMAT_ID due to the change in pgstat format.

Bump catalog version.

Author: Masahiro Ikeda
Reviewed-by: Takayuki Tsunakawa, Kyotaro Horiguchi, Amit Kapila, Fujii Masao
Discussion: https://postgr.es/m/188bd3f2d2233cf97753b5ced02bb050@oss.nttdata.com

4 years agoAdd block information in error context of WAL REDO apply loop
Michael Paquier [Fri, 2 Oct 2020 00:31:50 +0000 (09:31 +0900)]
Add block information in error context of WAL REDO apply loop

Providing this information can be useful for example when diagnosing
problems related to recovery conflicts or for recovery issues without
having to go through the output generated by pg_waldump to get some
information about the blocks a WAL record works on.

The block information is printed in the same format as pg_waldump.  This
already existed in xlog.c for debugging purposes with -DWAL_DEBUG, so
adding the block information in the callback has required just a small
refactoring.

Author: Bertrand Drouvot
Reviewed-by: Michael Paquier, Masahiko Sawada
Discussion: https://postgr.es/m/c31e2cba-efda-762c-f4ad-5c25e5dac3d0@amazon.com

4 years agoPut back explicit setting of replication values within TAP tests.
Tom Lane [Thu, 1 Oct 2020 14:59:20 +0000 (10:59 -0400)]
Put back explicit setting of replication values within TAP tests.

Commit 151c0c5f7 neglected the possibility that a TEMP_CONFIG file
would explicitly set max_wal_senders=0; as indeed buildfarm member
thorntail does, so that it can test wal_level=minimal in other test
suites.  Hence, rather than assuming that max_wal_senders=10 will
prevail if we say nothing, set it explicitly.

Set max_replication_slots=10 explicitly too, just to be safe.

Back-patch to v10, like the previous patch.

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

4 years agoFix incorrect assertion on number of array dimensions.
Heikki Linnakangas [Thu, 1 Oct 2020 08:48:48 +0000 (11:48 +0300)]
Fix incorrect assertion on number of array dimensions.

This has been wrong ever since the support for multi-dimensional
arrays as PL/python function arguments and return values was
introduced in commit 94aceed317.

Backpatch-through: 10
Discussion: https://www.postgresql.org/message-id/61647b8e-961c-0362-d5d3-c8a18f4a7ec6%40iki.fi

4 years agoSet right-links during sorted GiST index build.
Heikki Linnakangas [Thu, 1 Oct 2020 08:10:43 +0000 (11:10 +0300)]
Set right-links during sorted GiST index build.

This is not strictly necessary, as the right-links are only needed by
scans that are concurrent with page splits, and neither scans or page
splits can happen during sorted index build. But it seems like a good
idea to set them anyway, if we e.g. want to add a check to amcheck in
the future to verify that the chain of right-links is complete.

Author: Andrey Borodin
Discussion: https://www.postgresql.org/message-id/4D68C21F-9FB9-41DA-B663-FDFC8D143788%40yandex-team.ru

4 years agoRemove logging.c from the shared library of src/common/
Michael Paquier [Thu, 1 Oct 2020 01:37:34 +0000 (10:37 +0900)]
Remove logging.c from the shared library of src/common/

As fe0a1dc has proved, it is not a good concept to add to libpq
dependencies that would enforce the error output to a central logging
facility because it breaks the promise of reporting the error back to
an application in a consistent way, with the application to potentially
exit() suddenly if using pieces from for example jsonapi.c.  prairiedog
has allowed to report an actual design problem with fe0a1dc, but it will
not be around forever, so removing logging.c from libpgcommon_shlib is a
simple and much better long-term way to prevent any attempt to load the
central logging in libraries with general purposes.

Author: Michael Paquier
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/20200928073330.GC2316@paquier.xyz

4 years agoFix and test snapshot behavior on standby.
Andres Freund [Thu, 1 Oct 2020 00:28:51 +0000 (17:28 -0700)]
Fix and test snapshot behavior on standby.

I (Andres) broke this in 623a9CA79bx, because I didn't think about the
way snapshots are built on standbys sufficiently. Unfortunately our
existing tests did not catch this, as they are all just querying with
psql (therefore ending up with fresh snapshots).

The fix is trivial, we just need to increment the transaction
completion counter in ExpireTreeKnownAssignedTransactionIds(), which
is the equivalent of ProcArrayEndTransaction() during recovery.

This commit also adds a new test doing some basic testing of the
correctness of snapshots built on standbys. To avoid the
aforementioned issue of one-shot psql's not exercising the snapshot
caching, the test uses a long lived psqls, similar to
013_crash_restart.pl. It'd be good to extend the test further.

Reported-By: Ian Barwick <ian.barwick@2ndquadrant.com>
Author: Andres Freund <andres@anarazel.de>
Author: Ian Barwick <ian.barwick@2ndquadrant.com>
Discussion: https://postgr.es/m/61291ffe-d611-f889-68b5-c298da9fb18f@2ndquadrant.com

4 years agoReword partitioning error message
Alvaro Herrera [Wed, 30 Sep 2020 21:25:23 +0000 (18:25 -0300)]
Reword partitioning error message

The error message about columns in the primary key not including all of
the partition key was unclear; reword it.

Backpatch all the way to pg11, where it appeared.

Reported-by: Nagaraj Raj <nagaraj.sf@yahoo.com>
Discussion: https://postgr.es/m/64062533.78364.1601415362244@mail.yahoo.com

4 years agoFix handling of BC years in to_date/to_timestamp.
Tom Lane [Wed, 30 Sep 2020 19:40:23 +0000 (15:40 -0400)]
Fix handling of BC years in to_date/to_timestamp.

Previously, a conversion such as
to_date('-44-02-01','YYYY-MM-DD')
would result in '0045-02-01 BC', as the code attempted to interpret
the negative year as BC, but failed to apply the correction needed
for our internal handling of BC years.  Fix the off-by-one problem.

Also, arrange for the combination of a negative year and an
explicit "BC" marker to cancel out and produce AD.  This is how
the negative-century case works, so it seems sane to do likewise.

Continue to read "year 0000" as 1 BC.  Oracle would throw an error,
but we've accepted that case for a long time so I'm hesitant to
change it in a back-patch.

Per bug #16419 from Saeed Hubaishan.  Back-patch to all supported
branches.

Dar Alathar-Yemen and Tom Lane

Discussion: https://postgr.es/m/16419-d8d9db0a7553f01b@postgresql.org

4 years agopgbench: Use PQExpBuffer to simplify code that constructs SQL.
Heikki Linnakangas [Wed, 30 Sep 2020 07:58:09 +0000 (10:58 +0300)]
pgbench: Use PQExpBuffer to simplify code that constructs SQL.

Author: Fabien Coelho
Reviewed-by: Jeevan Ladhe
Discussion: https://www.postgresql.org/message-id/alpine.DEB.2.21.1910220826570.15559%40lancre

4 years agoFix XML id to match GUC name
Peter Eisentraut [Wed, 30 Sep 2020 05:39:38 +0000 (07:39 +0200)]
Fix XML id to match GUC name

For some reason, the id of the description of
max_parallel_maintenance_workers has been
guc-max-parallel-workers-maintenance since the beginning.  Flip that
around to make it consistent.

4 years agoRemove obsolete replication settings within TAP tests.
Tom Lane [Wed, 30 Sep 2020 00:02:58 +0000 (20:02 -0400)]
Remove obsolete replication settings within TAP tests.

PostgresNode.pm set "max_wal_senders = 5" for replication testing,
but this seems to be slightly too low for our current test suite.
Slower buildfarm members frequently report "number of requested standby
connections exceeds max_wal_senders" failures, due to old walsenders
not exiting instantaneously.  Usually, the test does not fail overall
because of automatic walreceiver restart, but sometimes the failure
becomes visible; and in any case such retries slow down the test.

That value came in with commit 89ac7004d, but was soon obsoleted by
f6d6d2920, which raised the built-in default from zero to 10; so that
PostgresNode.pm is actually setting it to less than the conservative
built-in default.  That seems pretty pointless, so let's remove the
special setting and let the default prevail, in hopes of making
the TAP tests more robust.

Likewise, the setting "max_replication_slots = 5" is obsolete and
can be removed.

While here, reverse-engineer a comment about why we're choosing
less-than-default values for some other settings.

(Note: before v12, max_wal_senders counted against max_connections
so that the latter setting also needs some fiddling with.)

Back-patch to v10 where the subscription tests were added.
It's likely that the older branches aren't pushing the boundaries
of max_wal_senders, but I'm disinclined to spend time trying to
figure out exactly when it started to be a problem.

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

4 years agoDoc: Improve clarity on partitioned table limitations
David Rowley [Wed, 30 Sep 2020 00:02:08 +0000 (13:02 +1300)]
Doc: Improve clarity on partitioned table limitations

Explicitly mention that primary key constraints are also included in the
limitation that the constraint columns must be a superset of the partition key
columns.

Wording suggestion from Tom Lane.

Discussion: https://postgr.es/m/64062533.78364.1601415362244@mail.yahoo.com
Backpatch-through: 11, where unique constraints on partitioned tables were added

4 years agoFix make_timestamp[tz] to accept negative years as meaning BC.
Tom Lane [Tue, 29 Sep 2020 17:48:06 +0000 (13:48 -0400)]
Fix make_timestamp[tz] to accept negative years as meaning BC.

Previously we threw an error.  But make_date already allowed the case,
so it is inconsistent as well as unhelpful for make_timestamp not to.

Both functions continue to reject year zero.

Code and test fixes by Peter Eisentraut, doc changes by me

Discussion: https://postgr.es/m/13c0992c-f15a-a0ca-d839-91d3efd965d9@2ndquadrant.com

4 years agoFix memory leak in plpgsql's CALL processing.
Tom Lane [Tue, 29 Sep 2020 15:18:30 +0000 (11:18 -0400)]
Fix memory leak in plpgsql's CALL processing.

When executing a CALL or DO in a non-atomic context (i.e., not inside
a function or query), plpgsql creates a new plan each time through,
as a rather hacky solution to some resource management issues.  But
it failed to free this plan until exit of the current procedure or DO
block, resulting in serious memory bloat in procedures that called
other procedures many times.  Fix by remembering to free the plan,
and by being more honest about restoring the previous state (otherwise,
recursive procedure calls have a problem).

There was also a smaller leak associated with recalculation of the
"target" list of output variables.  Fix that by using the statement-
lifespan context to hold non-permanent values.

Back-patch to v11 where procedures were introduced.

Pavel Stehule and Tom Lane

Discussion: https://postgr.es/m/CAFj8pRDiiU1dqym+_P4_GuTWm76knJu7z9opWayBJTC0nQGUUA@mail.gmail.com

4 years agoSupport for ISO 8601 in the jsonpath .datetime() method
Alexander Korotkov [Tue, 29 Sep 2020 08:41:46 +0000 (11:41 +0300)]
Support for ISO 8601 in the jsonpath .datetime() method

The SQL standard doesn't require jsonpath .datetime() method to support the
ISO 8601 format.  But our to_json[b]() functions convert timestamps to text in
the ISO 8601 format in the sake of compatibility with javascript.  So, we add
support of the  ISO 8601 to the jsonpath .datetime() in the sake compatibility
with to_json[b]().

The standard mode of datetime parsing currently supports just template patterns
and separators in the format string.  In order to implement ISO 8601, we have to
add support of the format string double quotes to the standard parsing mode.

Discussion: https://postgr.es/m/94321be0-cc96-1a81-b6df-796f437f7c66%40postgrespro.ru
Author: Nikita Glukhov, revised by me
Backpatch-through: 13

4 years agoRemove excess space from jsonpath .datetime() default format string
Alexander Korotkov [Tue, 29 Sep 2020 08:00:22 +0000 (11:00 +0300)]
Remove excess space from jsonpath .datetime() default format string

bffe1bd684 has introduced jsonpath .datetime() method, but default formats
for time and timestamp contain excess space between time and timezone.  This
commit removes this excess space making behavior of .datetime() method
standard-compliant.

Discussion: https://postgr.es/m/94321be0-cc96-1a81-b6df-796f437f7c66%40postgrespro.ru
Author: Nikita Glukhov
Backpatch-through: 13

4 years agoArchive timeline history files in standby if archive_mode is set to "always".
Fujii Masao [Tue, 29 Sep 2020 07:21:46 +0000 (16:21 +0900)]
Archive timeline history files in standby if archive_mode is set to "always".

Previously the standby server didn't archive timeline history files
streamed from the primary even when archive_mode is set to "always",
while it archives the streamed WAL files. This could cause the PITR to
fail because there was no required timeline history file in the archive.
The cause of this issue was that walreceiver didn't mark those files as
ready for archiving.

This commit makes walreceiver mark those streamed timeline history
files as ready for archiving if archive_mode=always. Then the archiver
process archives the marked timeline history files.

Back-patch to all supported versions.

Reported-by: Grigory Smolkin
Author: Grigory Smolkin, Fujii Masao
Reviewed-by: David Zhang, Anastasia Lubennikova
Discussion: https://postgr.es/m/54b059d4-2b48-13a4-6f43-95a087c92367@postgrespro.ru

4 years agoFix progress reporting of REINDEX CONCURRENTLY
Michael Paquier [Tue, 29 Sep 2020 05:15:57 +0000 (14:15 +0900)]
Fix progress reporting of REINDEX CONCURRENTLY

This addresses a couple of issues with the so-said subject:
- Report the correct parent relation with the index actually being
rebuilt or validated.  Previously, the command status remained set to
the last index created for the progress of the index build and
validation, which would be incorrect when working on a table that has
more than one index.
- Use the correct phase when waiting before the drop of the old
indexes.  Previously, this was reported with the same status as when
waiting before the old indexes are marked as dead.

Author: Matthias van de Meent, Michael Paquier
Discussion: https://postgr.es/m/CAEze2WhqFgcwe1_tv=sFYhLWV2AdpfukumotJ6JNcAOQs3jufg@mail.gmail.com
Backpatch-through: 12

4 years agoAdd for_each_from, to simplify loops starting from non-first list cells.
Tom Lane [Tue, 29 Sep 2020 00:32:53 +0000 (20:32 -0400)]
Add for_each_from, to simplify loops starting from non-first list cells.

We have a dozen or so places that need to iterate over all but the
first cell of a List.  Prior to v13 this was typically written as
for_each_cell(lc, lnext(list_head(list)))
Commit 1cff1b95a changed these to
for_each_cell(lc, list, list_second_cell(list))
This patch introduces a new macro for_each_from() which expresses
the start point as a list index, allowing these to be written as
for_each_from(lc, list, 1)
This is marginally more efficient, since ForEachState.i can be
initialized directly instead of backing into it from a ListCell
address.  It also seems clearer and less typo-prone.

Some of the remaining uses of for_each_cell() look like they could
profitably be changed to for_each_from(), but here I confined myself
to changing uses of list_second_cell().

Also, fix for_each_cell_setup() and for_both_cell_setup() to
const-ify their arguments; that's a simple oversight in 1cff1b95a.

Back-patch into v13, on the grounds that (1) the const-ification
is a minor bug fix, and (2) it's better for back-patching purposes
if we only have two ways to write these loops rather than three.

In HEAD, also remove list_third_cell() and list_fourth_cell(),
which were also introduced in 1cff1b95a, and are unused as of
cc99baa43.  It seems unlikely that any third-party code would
have started to use them already; anyone who has can be directed
to list_nth_cell instead.

Discussion: https://postgr.es/m/CAApHDvpo1zj9KhEpU2cCRZfSM3Q6XGdhzuAS2v79PH7WJBkYVA@mail.gmail.com

4 years agoRevert "Change SHA2 implementation based on OpenSSL to use EVP digest routines"
Michael Paquier [Tue, 29 Sep 2020 00:25:51 +0000 (09:25 +0900)]
Revert "Change SHA2 implementation based on OpenSSL to use EVP digest routines"

This reverts commit e21cbb4, as the switch to EVP routines requires a
more careful design where we would need to have at least our wrapper
routines return a status instead of issuing an error by themselves to
let the caller do the error handling.  The memory handling was also
incorrect and could cause leaks in the backend if a failure happened,
requiring most likely a callback to do the necessary cleanup as the only
clean way to be able to allocate an EVP context requires the use of an
allocation within OpenSSL.  The potential rework of the wrappers also
impacts the fallback implementation when not building with OpenSSL.

Originally, prairiedog has reported a compilation failure, but after
discussion with Tom Lane this needs a better design.

Discussion: https://postgr.es/m/20200928073330.GC2316@paquier.xyz

4 years agoStabilize create_table regression test.
Tom Lane [Mon, 28 Sep 2020 18:48:01 +0000 (14:48 -0400)]
Stabilize create_table regression test.

Adding \d+ to the test in commit 2dfa3fea8 was ill-advised,
because the partitions' names are such that their sort order
is locale dependent.  We could rename them to avoid that,
but it doesn't seem worth the trouble; just take \d+ out again.

Per buildfarm.

4 years agoAssign collations in partition bound expressions.
Tom Lane [Mon, 28 Sep 2020 18:12:38 +0000 (14:12 -0400)]
Assign collations in partition bound expressions.

Failure to do this can result in errors during evaluation of
the bound expression, as illustrated by the new regression test.

Back-patch to v12 where the ability for partition bounds to be
expressions was added.

Discussion: https://postgr.es/m/CAJV4CdrZ5mKuaEsRSbLf2URQ3h6iMtKD=hik8MaF5WwdmC9uZw@mail.gmail.com

4 years agoRemove complaints about COLLATE clauses in partition bound values.
Tom Lane [Mon, 28 Sep 2020 17:44:01 +0000 (13:44 -0400)]
Remove complaints about COLLATE clauses in partition bound values.

transformPartitionBoundValue went out of its way to do the wrong
thing: there is no reason to complain about a non-matching COLLATE
clause in a partition boundary expression.  We're coercing the
bound expression to the target column type as though by an
implicit assignment, and the rules for implicit assignment say
that collations can be implicitly converted.

What we *do* need to do, and the code is not doing, is apply
assign_expr_collations() to the bound expression.  While this is
merely a definition disagreement, that is a bug that needs to be
back-patched, so I'll commit it separately.

Discussion: https://postgr.es/m/CAJV4CdrZ5mKuaEsRSbLf2URQ3h6iMtKD=hik8MaF5WwdmC9uZw@mail.gmail.com

4 years agoCache the result of converting now() to a struct pg_tm.
Tom Lane [Mon, 28 Sep 2020 16:05:03 +0000 (12:05 -0400)]
Cache the result of converting now() to a struct pg_tm.

SQL operations such as CURRENT_DATE, CURRENT_TIME, LOCALTIME, and
conversion of "now" in a datetime input string have to obtain the
transaction start timestamp ("now()") as a broken-down struct pg_tm.
This is a remarkably expensive conversion, and since now() does not
change intra-transaction, it doesn't really need to be done more than
once per transaction.  Introducing a simple cache provides visible
speedups in queries that compute these values many times, for example
insertion of many rows that use a default value of CURRENT_DATE.

Peter Smith, with a bit of kibitzing by me

Discussion: https://postgr.es/m/CAHut+Pu89TWjq530V2gY5O6SWi=OEJMQ_VHMt8bdZB_9JFna5A@mail.gmail.com

4 years agoChange SHA2 implementation based on OpenSSL to use EVP digest routines
Michael Paquier [Mon, 28 Sep 2020 03:47:13 +0000 (12:47 +0900)]
Change SHA2 implementation based on OpenSSL to use EVP digest routines

The use of low-level hash routines is not recommended by upstream
OpenSSL since 2000, and pgcrypto already switched to EVP as of 5ff4a67.
Note that this also fixes a failure with SCRAM authentication when using
FIPS in OpenSSL, but as there have been few complaints about this
problem and as this causes an ABI breakage, no backpatch is done.

Author: Michael Paquier, Alessandro Gherardi
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/20200924025314.GE7405@paquier.xyz
Discussion: https://postgr.es/m/20180911030250.GA27115@paquier.xyz

4 years agoMinor mop-up for List improvements.
Tom Lane [Mon, 28 Sep 2020 02:30:44 +0000 (22:30 -0400)]
Minor mop-up for List improvements.

Fix a few places that were using written-out versions of the
pg_list.h macros that commit cc99baa43 just improved, making them
also use those macros so as to gain whatever performance improvement
is to be had.

Discussion: https://postgr.es/m/CAApHDvpo1zj9KhEpU2cCRZfSM3Q6XGdhzuAS2v79PH7WJBkYVA@mail.gmail.com

4 years agoImprove tab-completion for DEALLOCATE.
Fujii Masao [Mon, 28 Sep 2020 02:23:15 +0000 (11:23 +0900)]
Improve tab-completion for DEALLOCATE.

Author: Naoki Nakamichi
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/ec1a45b06edfce13706f2c765778d8c2@oss.nttdata.com

4 years agoImprove pg_list.h's linitial(), lsecond() and co macros
David Rowley [Mon, 28 Sep 2020 01:47:19 +0000 (14:47 +1300)]
Improve pg_list.h's linitial(), lsecond() and co macros

Prior to this commit, the linitial(), lsecond(), lthird(), lfourth()
macros and their int and Oid list cousins would call their corresponding
inlined function to fetch the cell of interest.  Those inline functions
were kind enough to return NULL if the particular cell did not exist.
Unfortunately, the care that these functions took was of no relevance to
the calling macros as they proceeded to directly dereference the returned
value without any regard to whether that value was NULL or not.  If it had
been, we'd have segfaulted.

Of course, the fact that we would have segfaulted on misuse of these
macros just goes to prove that nobody is relying on the empty or list too
small checks.  So here we just get rid of those checks completely.

The existing inline functions have been left alone as someone may be using
those directly.  We just replace the call within each macro to use
list_nth_cell().

For the llast*() case we require a new list_last_cell() inline function to
get away from the multiple evaluation hazard that we'd get if we fetched
->length on the macro's parameter.

Author: David Rowley
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/CAApHDvpo1zj9KhEpU2cCRZfSM3Q6XGdhzuAS2v79PH7WJBkYVA@mail.gmail.com

4 years agoImprove range checks of options for pg_test_fsync and pg_test_timing
Michael Paquier [Mon, 28 Sep 2020 01:13:59 +0000 (10:13 +0900)]
Improve range checks of options for pg_test_fsync and pg_test_timing

Both tools never had safeguard checks for the options provided, and it
was possible to make pg_test_fsync run an infinite amount of time or
pass down buggy values to pg_test_timing.

These behaviors have existed for a long time, with no actual complaints,
so no backpatch is done.  Basic TAP tests are introduced for both tools.

Author: Michael Paquier
Reviewed-by: Peter Eisentraut
Discussion: https://postgr.es/m/20200806062759.GE16470@paquier.xyz

4 years agoMove resolution of AlternativeSubPlan choices to the planner.
Tom Lane [Sun, 27 Sep 2020 16:51:28 +0000 (12:51 -0400)]
Move resolution of AlternativeSubPlan choices to the planner.

When commit bd3daddaf introduced AlternativeSubPlans, I had some
ambitions towards allowing the choice of subplan to change during
execution.  That has not happened, or even been thought about, in the
ensuing twelve years; so it seems like a failed experiment.  So let's
rip that out and resolve the choice of subplan at the end of planning
(in setrefs.c) rather than during executor startup.  This has a number
of positive benefits:

* Removal of a few hundred lines of executor code, since
AlternativeSubPlans need no longer be supported there.

* Removal of executor-startup overhead (particularly, initialization
of subplans that won't be used).

* Removal of incidental costs of having a larger plan tree, such as
tree-scanning and copying costs in the plancache; not to mention
setrefs.c's own costs of processing the discarded subplans.

* EXPLAIN no longer has to print a weird (and undocumented)
representation of an AlternativeSubPlan choice; it sees only the
subplan actually used.  This should mean less confusion for users.

* Since setrefs.c knows which subexpression of a plan node it's
working on at any instant, it's possible to adjust the estimated
number of executions of the subplan based on that.  For example,
we should usually estimate more executions of a qual expression
than a targetlist expression.  The implementation used here is
pretty simplistic, because we don't want to expend a lot of cycles
on the issue; but it's better than ignoring the point entirely,
as the executor had to.

That last point might possibly result in shifting the choice
between hashed and non-hashed EXISTS subplans in a few cases,
but in general this patch isn't meant to change planner choices.
Since we're doing the resolution so late, it's really impossible
to change any plan choices outside the AlternativeSubPlan itself.

Patch by me; thanks to David Rowley for review.

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

4 years agoFurther stabilize output from rolenames regression test.
Tom Lane [Sat, 26 Sep 2020 21:42:20 +0000 (17:42 -0400)]
Further stabilize output from rolenames regression test.

Commit e5209bf37 didn't quite get the job done, as I failed to
notice that chksetconfig() also needed to have its ORDER BY
extended.  Per buildfarm member dory.

Report: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dory&dt=2020-09-26%2020%3A10%3A13

4 years agoRevise RelationBuildRowSecurity() to avoid memory leaks.
Tom Lane [Sat, 26 Sep 2020 20:04:06 +0000 (16:04 -0400)]
Revise RelationBuildRowSecurity() to avoid memory leaks.

This function leaked some memory while loading qual clauses for
an RLS policy.  While ordinarily negligible, that could build up
in some repeated-reload cases, as reported by Konstantin Knizhnik.
We can improve matters by borrowing the coding long used in
RelationBuildRuleLock: build stringToNode's result directly in
the target context, and remember to explicitly pfree the
input string.

This patch by no means completely guarantees zero leaks within
this function, since we have no real guarantee that the catalog-
reading subroutines it calls don't leak anything.  However,
practical tests suggest that this is enough to resolve the issue.
In any case, any remaining leaks are similar to those risked by
RelationBuildRuleLock and other relcache-loading subroutines.
If we need to fix them, we should adopt a more global approach
such as that used by the RECOVER_RELATION_BUILD_MEMORY hack.

While here, let's remove the need for an expensive PG_TRY block by
using MemoryContextSetParent to reparent an initially-short-lived
context for the RLS data.

Back-patch to all supported branches.

Discussion: https://postgr.es/m/21356c12-8917-8249-b35f-1c447231922b@postgrespro.ru

4 years agoFix the logical replication from HEAD to lower versions.
Amit Kapila [Sat, 26 Sep 2020 04:38:00 +0000 (10:08 +0530)]
Fix the logical replication from HEAD to lower versions.

Commit 464824323e changed the logical replication protocol to allow the
streaming of in-progress transactions and used the new version of protocol
irrespective of the server version. Use the appropriate version of the
protocol based on the server version.

Reported-by: Ashutosh Sharma
Author: Dilip Kumar
Reviewed-by: Ashutosh Sharma and Amit Kapila
Discussion: https://postgr.es/m/CAE9k0P=9OpXcNrcU5Gsvd5MZ8GFpiN833vNHzX6Uc=8+h1ft1Q@mail.gmail.com

4 years agoDefer flushing of SLRU files.
Thomas Munro [Fri, 25 Sep 2020 06:49:43 +0000 (18:49 +1200)]
Defer flushing of SLRU files.

Previously, we called fsync() after writing out individual pg_xact,
pg_multixact and pg_commit_ts pages due to cache pressure, leading to
regular I/O stalls in user backends and recovery.  Collapse requests for
the same file into a single system call as part of the next checkpoint,
as we already did for relation files, using the infrastructure developed
by commit 3eb77eba.  This can cause a significant improvement to
recovery performance, especially when it's otherwise CPU-bound.

Hoist ProcessSyncRequests() up into CheckPointGuts() to make it clearer
that it applies to all the SLRU mini-buffer-pools as well as the main
buffer pool.  Rearrange things so that data collected in CheckpointStats
includes SLRU activity.

Also remove the Shutdown{CLOG,CommitTS,SUBTRANS,MultiXact}() functions,
because they were redundant after the shutdown checkpoint that
immediately precedes them.  (I'm not sure if they were ever needed, but
they aren't now.)

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> (parts)
Tested-by: Jakub Wartak <Jakub.Wartak@tomtom.com>
Discussion: https://postgr.es/m/CA+hUKGLJ=84YT+NvhkEEDAuUtVHMfQ9i-N7k_o50JmQ6Rpj_OQ@mail.gmail.com

4 years agoRemove custom memory allocation layer in pgcrypto
Michael Paquier [Fri, 25 Sep 2020 01:25:55 +0000 (10:25 +0900)]
Remove custom memory allocation layer in pgcrypto

PX_OWN_ALLOC was intended as a way to disable the use of palloc(), and
over the time new palloc() or equivalent calls have been added like in
32984d8, making this extra layer losing its original purpose.  This
simplifies on the way some code paths to use palloc0() rather than
palloc() followed by memset(0).

Author: Daniel Gustafsson
Discussion: https://postgr.es/m/A5BFAA1A-B2E8-4CBC-895E-7B1B9475A527@yesql.se

4 years agoFix handling of -d "connection string" in pg_dump/pg_restore.
Tom Lane [Thu, 24 Sep 2020 22:19:38 +0000 (18:19 -0400)]
Fix handling of -d "connection string" in pg_dump/pg_restore.

Parallel pg_dump failed if its -d parameter was a connection string
containing any essential information other than host, port, or username.
The same was true for pg_restore with --create.

The reason is that these scenarios failed to preserve the connection
string from the command line; the code felt free to replace that with
just the database name when reconnecting from a pg_dump parallel worker
or after creating the target database.  By chance, parallel pg_restore
did not suffer this defect, as long as you didn't say --create.

In practice it seems that the error would be obvious only if the
connstring included essential, non-default SSL or GSS parameters.
This may explain why it took us so long to notice.  (It also makes
it very difficult to craft a regression test case illustrating the
problem, since the test would fail in builds without those options.)

Fix by refactoring so that ConnectDatabase always receives all the
relevant options directly from the command line, rather than
reconstructed values.  Inject a different database name, when necessary,
by relying on libpq's rules for handling multiple "dbname" parameters.

While here, let's get rid of the essentially duplicate _connectDB
function, as well as some obsolete nearby cruft.

Per bug #16604 from Zsolt Ero.  Back-patch to all supported branches.

Discussion: https://postgr.es/m/16604-933f4b8791227b15@postgresql.org

4 years agoFix two bugs in MaintainOldSnapshotTimeMapping.
Robert Haas [Thu, 24 Sep 2020 19:27:19 +0000 (15:27 -0400)]
Fix two bugs in MaintainOldSnapshotTimeMapping.

The previous coding was confused about whether head_timestamp was
intended to represent the timestamp for the newest bucket in the
mapping or the oldest timestamp for the oldest bucket in the mapping.
Decide that it's intended to be the oldest one, and repair
accordingly.

To do that, we need to do two things. First, when advancing to a
new bucket, don't categorically set head_timestamp to the new
timestamp. Do this only if we're blowing out the map completely
because a lot of time has passed since we last maintained it. If
we're replacing entries one by one, advance head_timestamp by
1 minute for each; if we're filling in unused entries, don't
advance head_timestamp at all.

Second, fix the computation of how many buckets we need to advance.
The previous formula would be correct if head_timestamp were the
timestamp for the new bucket, but we're now making all the code
agree that it's the timestamp for the oldest bucket, so adjust the
formula accordingly.

This is certainly a bug fix, but I don't feel good about
back-patching it without the introspection tools added by commit
aecf5ee2bb36c597d3c6142e367e38d67816c777, and perhaps also some
actual tests. Since back-patching the introspection tools might
not attract sufficient support and since there are no automated
tests of these fixes yet, I'm just committing this to master for
now.

Patch by me, reviewed by Thomas Munro, Dilip Kumar, Hamid Akhtar.

Discussion: http://postgr.es/m/CA+TgmoY=aqf0zjTD+3dUWYkgMiNDegDLFjo+6ze=Wtpik+3XqA@mail.gmail.com

4 years agoStandardize the printf format for st_size
Peter Eisentraut [Thu, 24 Sep 2020 18:45:57 +0000 (20:45 +0200)]
Standardize the printf format for st_size

Existing code used various inconsistent ways to printf struct stat's
st_size member.  The type of that is off_t, which is in most cases a
signed 64-bit integer, so use the long long int format for it.

4 years agoAdd new 'old_snapshot' contrib module.
Robert Haas [Thu, 24 Sep 2020 17:55:47 +0000 (13:55 -0400)]
Add new 'old_snapshot' contrib module.

You can use this to view the contents of the time to XID mapping
which the server maintains when old_snapshot_threshold != -1.
Being able to view that information may be interesting for users,
and it's definitely useful for figuring out whether the mapping
is being maintained correctly. It isn't, so that will need to be
fixed in a subsequent commit.

Patch by me, reviewed by Thomas Munro, Dilip Kumar, Hamid Akhtar.

Discussion: http://postgr.es/m/CA+TgmoY=aqf0zjTD+3dUWYkgMiNDegDLFjo+6ze=Wtpik+3XqA@mail.gmail.com

4 years agoExpose oldSnapshotControl definition via new header.
Robert Haas [Thu, 24 Sep 2020 17:32:39 +0000 (13:32 -0400)]
Expose oldSnapshotControl definition via new header.

This makes it possible for code outside snapmgr.c to examine the
contents of this data structure. This commit does not add any code
which actually does so; a subsequent commit will make that change.

Patch by me, reviewed by Thomas Munro, Dilip Kumar, Hamid Akhtar.

Discussion: http://postgr.es/m/CA+TgmoY=aqf0zjTD+3dUWYkgMiNDegDLFjo+6ze=Wtpik+3XqA@mail.gmail.com

4 years agoDoc: sync lobj.sgml's copy of testlo.c with the latter file.
Tom Lane [Thu, 24 Sep 2020 14:39:11 +0000 (10:39 -0400)]
Doc: sync lobj.sgml's copy of testlo.c with the latter file.

Zhijie Hou

Discussion: https://postgr.es/m/ce2cd951fe9b448a9cda99dc1a871fb9@G08CNEXMBPEKD05.g08.fujitsu.local

4 years agoImprove behavior of tsearch_readline(), and remove t_readline().
Tom Lane [Thu, 24 Sep 2020 00:26:58 +0000 (20:26 -0400)]
Improve behavior of tsearch_readline(), and remove t_readline().

Commit fbeb9da22, which added the tsearch_readline APIs, left
t_readline() in place as a compatibility measure.  But that function
has been unused and deprecated for twelve years now, so that seems
like enough time to remove it.  Doing so, and merging t_readline's
code into tsearch_readline, aids in making several useful
improvements:

* The hard-wired 4K limit on line length in tsearch data files is
removed, by using a StringInfo buffer instead of a fixed-size buffer.

* We can buy back the per-line palloc/pfree added by 3ea7e9550
in the common case where encoding conversion is not required.

* We no longer need a separate pg_verify_mbstr call, as that
functionality was folded into encoding conversion some time ago.

(We could have done some of this stuff while keeping t_readline as a
separate API, but there seems little point, since there's no reason
for anyone to still be using t_readline directly.)

Discussion: https://postgr.es/m/48A4FA71-524E-41B9-953A-FD04EF36E2E7@yesql.se

4 years agoFix missing fsync of SLRU directories.
Thomas Munro [Wed, 23 Sep 2020 21:26:09 +0000 (09:26 +1200)]
Fix missing fsync of SLRU directories.

Harmonize behavior by moving reponsibility for fsyncing directories down
into slru.c.  In 10 and later, only the multixact directories were
missed (see commit 1b02be21), and in older branches all SLRUs were
missed.

Back-patch to all supported releases.

Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CA%2BhUKGLtsTUOScnNoSMZ-2ZLv%2BwGh01J6kAo_DM8mTRq1sKdSQ%40mail.gmail.com

4 years agoImprove error cursor positions for problems with partition bounds.
Tom Lane [Wed, 23 Sep 2020 22:04:53 +0000 (18:04 -0400)]
Improve error cursor positions for problems with partition bounds.

We failed to pass down the query string to check_new_partition_bound,
so that its attempts to provide error cursor positions were for naught;
one must have the query string to get parser_errposition to do anything.
Adjust its API to require a ParseState to be passed down.

Also, improve the logic inside check_new_partition_bound so that the
cursor points at the partition bound for the specific column causing
the issue, when one can be identified.

That part is also for naught if we can't determine the query position of
the column with the problem.  Improve transformPartitionBoundValue so
that it makes sure that const-simplified partition expressions will be
properly labeled with positions.  In passing, skip calling evaluate_expr
if the value is already a Const, which is surely the most common case.

Alexandra Wang, Ashwin Agrawal, Amit Langote; reviewed by Ashutosh Bapat

Discussion: https://postgr.es/m/CACiyaSopZoqssfMzgHk6fAkp01cL6vnqBdmTw2C5_KJaFR_aMg@mail.gmail.com
Discussion: https://postgr.es/m/CAJV4CdrZ5mKuaEsRSbLf2URQ3h6iMtKD=hik8MaF5WwdmC9uZw@mail.gmail.com

4 years agoAvoid possible dangling-pointer access in tsearch_readline_callback.
Tom Lane [Wed, 23 Sep 2020 15:36:13 +0000 (11:36 -0400)]
Avoid possible dangling-pointer access in tsearch_readline_callback.

tsearch_readline() saves the string pointer it returns to the caller
for possible use in the associated error context callback.  However,
the caller will usually pfree that string sometime before it next
calls tsearch_readline(), so that there is a window where an ereport
will try to print an already-freed string.

The built-in users of tsearch_readline() happen to all do that pfree
at the bottoms of their loops, so that the window is effectively
empty for them.  However, this is not documented as a requirement,
and contrib/dict_xsyn doesn't do it like that, so it seems likely
that third-party dictionaries might have live bugs here.

The practical consequences of this seem pretty limited in any case,
since production builds wouldn't clobber the freed string immediately,
besides which you'd not expect syntax errors in dictionary files
being used in production.  Still, it's clearly a bug waiting to bite
somebody.

Fix by pstrdup'ing the string to be saved for the error callback,
and then pfree'ing it next time through.  It's been like this for
a long time, so back-patch to all supported branches.

Discussion: https://postgr.es/m/48A4FA71-524E-41B9-953A-FD04EF36E2E7@yesql.se

4 years agoAllow WaitLatch() to be used without a latch.
Thomas Munro [Wed, 23 Sep 2020 03:17:30 +0000 (15:17 +1200)]
Allow WaitLatch() to be used without a latch.

Due to flaws in commit 3347c982bab, using WaitLatch() without
WL_LATCH_SET could cause an assertion failure or crash.  Repair.

While here, also add a check that the latch we're switching to belongs
to this backend, when changing from one latch to another.

Discussion: https://postgr.es/m/CA%2BhUKGK1607VmtrDUHQXrsooU%3Dap4g4R2yaoByWOOA3m8xevUQ%40mail.gmail.com

4 years agoSimplify SortTocFromFile() by removing fixed buffer-size limit.
Tom Lane [Tue, 22 Sep 2020 20:03:32 +0000 (16:03 -0400)]
Simplify SortTocFromFile() by removing fixed buffer-size limit.

pg_restore previously coped with overlength TOC-file lines using some
complicated logic to ignore additional bufferloads.  While this isn't
wrong, since we don't expect that the interesting part of a line would
run to more than a dozen or so bytes, it's more complex than it needs
to be.  Use a StringInfo instead of a fixed-size buffer so that we can
process long lines as single entities and thus not need the extra
logic.

Daniel Gustafsson

Discussion: https://postgr.es/m/48A4FA71-524E-41B9-953A-FD04EF36E2E7@yesql.se

4 years agoRemove arbitrary line length limit for libpq service files.
Tom Lane [Tue, 22 Sep 2020 19:59:23 +0000 (15:59 -0400)]
Remove arbitrary line length limit for libpq service files.

Use a StringInfo instead of a fixed-size buffer in parseServiceInfo().
While we've not heard complaints about the existing 255-byte limit,
it certainly seems possible that complex cases could run afoul of it.

Daniel Gustafsson

Discussion: https://postgr.es/m/48A4FA71-524E-41B9-953A-FD04EF36E2E7@yesql.se

4 years agoRethink API for pg_get_line.c, one more time.
Tom Lane [Tue, 22 Sep 2020 19:55:13 +0000 (15:55 -0400)]
Rethink API for pg_get_line.c, one more time.

Further experience says that the appending behavior offered by
pg_get_line_append is useful to only a very small minority of callers.
For most, the requirement to reset the buffer after each line is just
an error-prone nuisance.  Hence, invent another alternative call
pg_get_line_buf, which takes care of that detail.

Noted while reviewing a patch from Daniel Gustafsson.

Discussion: https://postgr.es/m/48A4FA71-524E-41B9-953A-FD04EF36E2E7@yesql.se

4 years agoExclude fmgrprotos.h from pgindent processing.
Tom Lane [Tue, 22 Sep 2020 15:32:10 +0000 (11:32 -0400)]
Exclude fmgrprotos.h from pgindent processing.

pgindent messes up entries in this file if their names match
typedef names.  While there's reason to avoid choosing conflicting
names, we have some historical exceptions, and there's no guarantee
that more duplicates won't appear in future.  Since this is a derived
file anyway, there's little harm in just excluding it.

I said yesterday that all our derived files are pgindent-clean, or else
explicitly excluded from indentation, but I'd forgotten about this one.
Now that project is really done, as confirmed by a test run.

Discussion: https://postgr.es/m/79ed5348-be7a-b647-dd40-742207186a22@2ndquadrant.com

4 years agoImprove the error message for an inappropriate column definition list.
Tom Lane [Tue, 22 Sep 2020 14:49:11 +0000 (10:49 -0400)]
Improve the error message for an inappropriate column definition list.

The existing message about "a column definition list is only allowed for
functions returning "record"" could be given in some cases where it was
fairly confusing; in particular, a function with multiple OUT parameters
*does* return record according to pg_proc.  Break it down into a couple
more cases to deliver a more on-point complaint.  Per complaint from
Bruce Momjian.

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

4 years agoFix a few more generator scripts to produce pgindent-clean output.
Tom Lane [Mon, 21 Sep 2020 17:58:26 +0000 (13:58 -0400)]
Fix a few more generator scripts to produce pgindent-clean output.

This completes the project of making all our derived files be
pgindent-clean (or else explicitly excluded from indentation),
so that no surprises result when running pgindent in a built-out
development tree.

Discussion: https://postgr.es/m/79ed5348-be7a-b647-dd40-742207186a22@2ndquadrant.com

4 years agoCopy editing: fix a bunch of misspellings and poor wording.
Tom Lane [Mon, 21 Sep 2020 16:43:42 +0000 (12:43 -0400)]
Copy editing: fix a bunch of misspellings and poor wording.

99% of this is docs, but also a couple of comments.  No code changes.

Justin Pryzby

Discussion: https://postgr.es/m/20200919175804.GE30557@telsasoft.com

4 years agoStandardize order of use strict and use warnings in Perl code
Peter Eisentraut [Mon, 21 Sep 2020 13:22:41 +0000 (15:22 +0200)]
Standardize order of use strict and use warnings in Perl code

The standard order in PostgreSQL and other code is use strict first,
but some code was uselessly inconsistent about this.

4 years agoFix checksum calculation in the new sorting GiST build.
Heikki Linnakangas [Mon, 21 Sep 2020 11:50:07 +0000 (14:50 +0300)]
Fix checksum calculation in the new sorting GiST build.

Since we're bypassing the buffer manager, we need to call
PageSetChecksumInplace() directly. As reported by Justin Pryzby.

In the passing, add RelationOpenSmgr() calls before all smgrwrite() and
smgrextend() calls. Tom added one before the first smgrextend() call in
commit c2bb287025, which seems to be enough, but let's play it safe and
do it before each one. That's how it's done in the similar code in
nbtsort.c, too.

Discussion: https://www.postgresql.org/message-id/20200920224446.GF30557@telsasoft.com

4 years agoFix new GIST build code to work under CLOBBER_CACHE_ALWAYS.
Tom Lane [Sun, 20 Sep 2020 21:08:49 +0000 (17:08 -0400)]
Fix new GIST build code to work under CLOBBER_CACHE_ALWAYS.

Can't say if this fixes *all* cases, but at least we get through
the "point" regression test now, which hyrax's last run did not.

Report: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hyrax&dt=2020-09-19%2021%3A27%3A23

4 years agoFix whitespace
Peter Eisentraut [Sun, 20 Sep 2020 12:42:54 +0000 (14:42 +0200)]
Fix whitespace

4 years agoRemove precedence hacks no longer needed without postfix operators.
Tom Lane [Sat, 19 Sep 2020 19:11:26 +0000 (15:11 -0400)]
Remove precedence hacks no longer needed without postfix operators.

It's no longer necessary to assign explicit precedences to GENERATED,
NULL_P, PRESERVE, or STRIP_P.

Actually, we don't need to assign precedence to IDENT either; that was
really just there to govern the behavior of target_el's "a_expr IDENT"
production, which no longer ends with that terminal.  However, it seems
like a good idea to continue to do so, because it provides a reference
point for a precedence level that we can assign to other unreserved
keywords that lack a natural precedence level.

Research by Peter Eisentraut and John Naylor; comment rewrite by me.

Discussion: https://postgr.es/m/38ca86db-42ab-9b48-2902-337a0d6b8311@2ndquadrant.com

4 years agoRemove unused parameters
Peter Eisentraut [Tue, 25 Aug 2020 05:24:15 +0000 (07:24 +0200)]
Remove unused parameters

Remove various unused parameters in pg_dump code.  These have all
become unused over time or were never used.

Discussion: https://www.postgresql.org/message-id/flat/511bb100-f829-ba21-2f10-9f952ec06ead%402ndquadrant.com

4 years agoCode review for dynahash change.
Thomas Munro [Sat, 19 Sep 2020 03:39:48 +0000 (15:39 +1200)]
Code review for dynahash change.

Commit be0a6666 left behind a comment about the order of some tests that
didn't make sense without the expensive division, and in fact we might
as well change the order to one that fails more cheaply most of the time
as a micro-optimization.  Also, remove the "+ 1" applied to max_bucket,
to drop an instruction and match the original behavior.  Per review
from Tom Lane.

Discussion: https://postgr.es/m/VI1PR0701MB696044FC35013A96FECC7AC8F62D0%40VI1PR0701MB6960.eurprd07.prod.outlook.com

4 years agoRemove large fill factor support from dynahash.c.
Thomas Munro [Fri, 18 Sep 2020 23:28:34 +0000 (11:28 +1200)]
Remove large fill factor support from dynahash.c.

Since ancient times we have had support for a fill factor (maximum load
factor) to be set for a dynahash hash table, but:

1.  It was an integer, whereas for in-memory hash tables interesting
load factor targets are probably somewhere near the 0.75-1.0 range.

2.  It was implemented in a way that performed an expensive division
operation that regularly showed up in profiles.

3.  We are not aware of anyone ever having used a non-default value.

Therefore, remove support, effectively fixing it at 1.

Author: Jakub Wartak <Jakub.Wartak@tomtom.com>
Reviewed-by: Alvaro Herrera <alvherre@2ndquadrant.com>
Reviewed-by: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/VI1PR0701MB696044FC35013A96FECC7AC8F62D0%40VI1PR0701MB6960.eurprd07.prod.outlook.com

4 years agoAllow most keywords to be used as column labels without requiring AS.
Tom Lane [Fri, 18 Sep 2020 20:46:26 +0000 (16:46 -0400)]
Allow most keywords to be used as column labels without requiring AS.

Up to now, if you tried to omit "AS" before a column label in a SELECT
list, it would only work if the column label was an IDENT, that is not
any known keyword.  This is rather unfriendly considering that we have
so many keywords and are constantly growing more.  In the wake of commit
1ed6b8956 it's possible to improve matters quite a bit.

We'd originally tried to make this work by having some of the existing
keyword categories be allowed without AS, but that didn't work too well,
because each category contains a few special cases that don't work
without AS.  Instead, invent an entirely orthogonal keyword property
"can be bare column label", and mark all keywords that way for which
we don't get shift/reduce errors by doing so.

It turns out that of our 450 current keywords, all but 39 can be made
bare column labels, improving the situation by over 90%.  This number
might move around a little depending on future grammar work, but it's
a pretty nice improvement.

Mark Dilger, based on work by myself and Robert Haas;
review by John Naylor

Discussion: https://postgr.es/m/38ca86db-42ab-9b48-2902-337a0d6b8311@2ndquadrant.com

4 years agopg_surgery: Try to stabilize regression tests.
Robert Haas [Fri, 18 Sep 2020 17:26:48 +0000 (13:26 -0400)]
pg_surgery: Try to stabilize regression tests.

According to buildfarm member sungazer, the behavior of VACUUM can be
unstable in these tests even if we prevent autovacuum from running on
the tables in question, apparently because even a manual vacuum can
behave differently depending on whether anything else is running that
holds back the global xmin. So use a temporary table instead, which
as of commit a7212be8b9e0885ee769e8c55f99ef742cda487b enables
vacuuming using a more aggressive cutoff.

This approach can't be used for the regression test that involves a
materialized view, but that test doesn't run vacuum, so it shouldn't
be prone to this particular failure mode.

Analysis by Tom Lane. Patch by Ashutosh Sharma and me.

Discussion: http://postgr.es/m/665524.1599948007@sss.pgh.pa.us

4 years agoUpdate file header comments for logical/relation.c.
Amit Kapila [Fri, 18 Sep 2020 04:44:30 +0000 (10:14 +0530)]
Update file header comments for logical/relation.c.

Author: Amit Langote
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CA+HiwqE20oZoix13JyCeALpTf_SmjarZWtBFe5sND6zz+iupAw@mail.gmail.com

4 years agoFix comments in heapam.c.
Amit Kapila [Fri, 18 Sep 2020 04:10:04 +0000 (09:40 +0530)]
Fix comments in heapam.c.

After commits 85f6b49c2c and 3ba59ccc89, we can allow parallel inserts
which was earlier not possible as parallel group members won't conflict
for relation extension and page lock.  In those commits, we forgot to
update comments at few places.

Author: Amit Kapila
Reviewed-by: Robert Haas and Dilip Kumar
Backpatch-through: 13
Discussion: https://postgr.es/m/CAFiTN-tMrQh5FFMPx5aWJ+1gi1H6JxktEhq5mDwCHgnEO5oBkA@mail.gmail.com

4 years agoTry to stabilize output from rolenames regression test.
Tom Lane [Fri, 18 Sep 2020 01:02:55 +0000 (21:02 -0400)]
Try to stabilize output from rolenames regression test.

It's not quite clear why commit 45b980570 has resulted in
some instability here, though interference from concurrent
autovacuum runs seems like a reasonable guess.  What is
clear is that the output ordering of the test queries is
underdetermined for no very good reason.  Extend the
ORDER BY keys in hopes of fixing the buildfarm.

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

4 years agoRemove support for postfix (right-unary) operators.
Tom Lane [Thu, 17 Sep 2020 23:38:05 +0000 (19:38 -0400)]
Remove support for postfix (right-unary) operators.

This feature has been a thorn in our sides for a long time, causing
many grammatical ambiguity problems.  It doesn't seem worth the
pain to continue to support it, so remove it.

There are some follow-on improvements we can make in the grammar,
but this commit only removes the bare minimum number of productions,
plus assorted backend support code.

Note that pg_dump and psql continue to have full support, since
they may be used against older servers.  However, pg_dump warns
about postfix operators.  There is also a check in pg_upgrade.

Documentation-wise, I (tgl) largely removed the "left unary"
terminology in favor of saying "prefix operator", which is
a more standard and IMO less confusing term.

I included a catversion bump, although no initial catalog data
changes here, to mark the boundary at which oprkind = 'r'
stopped being valid in pg_operator.

Mark Dilger, based on work by myself and Robert Haas;
review by John Naylor

Discussion: https://postgr.es/m/38ca86db-42ab-9b48-2902-337a0d6b8311@2ndquadrant.com

4 years agoRemove factorial operators, leaving only the factorial() function.
Tom Lane [Thu, 17 Sep 2020 20:17:27 +0000 (16:17 -0400)]
Remove factorial operators, leaving only the factorial() function.

The "!" operator is our only built-in postfix operator.  Remove it,
on the way to removal of grammar support for postfix operators.

There is also a "!!" prefix operator, but since it's been marked
deprecated for most of its existence, we might as well remove it too.

Also zap the SQL alias function numeric_fac(), which seems to have
equally little reason to live.

Mark Dilger, based on work by myself and Robert Haas;
review by John Naylor

Discussion: https://postgr.es/m/38ca86db-42ab-9b48-2902-337a0d6b8311@2ndquadrant.com

4 years agoFurther improve pgindent's list of file exclusions.
Tom Lane [Thu, 17 Sep 2020 18:16:18 +0000 (14:16 -0400)]
Further improve pgindent's list of file exclusions.

I despair of people keeping the README file's notes in sync with
the actual exclusion list, so move the notes into the exclusion file.
Adjust the pgindent script to explicitly ignore comments in the file,
just in case (though it's hard to believe any would match filenames).

Extend the list so that it doesn't process any files we'd not wish it to
even in a fully-built-out development directory.  (There are still a
couple of derived files that it wants to reformat, but fixing the
generator scripts for those seems like fit material for a separate patch.)

Discussion: https://postgr.es/m/79ed5348-be7a-b647-dd40-742207186a22@2ndquadrant.com

4 years agoImprove common/logging.c's support for multiple verbosity levels.
Tom Lane [Thu, 17 Sep 2020 16:52:18 +0000 (12:52 -0400)]
Improve common/logging.c's support for multiple verbosity levels.

Instead of hard-wiring specific verbosity levels into the option
processing of client applications, invent pg_logging_increase_verbosity()
and encourage clients to implement --verbose by calling that.  Then,
the common convention that more -v's gets you more verbosity just works.

In particular, this allows resurrection of the debug-grade messages that
have long existed in pg_dump and its siblings.  They were unreachable
before this commit due to lack of a way to select PG_LOG_DEBUG logging
level.  (It appears that they may have been unreachable for some time
before common/logging.c was introduced, too, so I'm not specifically
blaming cc8d41511 for the oversight.  One reason for thinking that is
that it's now apparent that _allocAH()'s message needs a null-pointer
guard.  Testing might have failed to reveal that before 96bf88d52.)

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

4 years agoUpdate parallel BTree scan state when the scan keys can't be satisfied.
Amit Kapila [Thu, 17 Sep 2020 09:46:46 +0000 (15:16 +0530)]
Update parallel BTree scan state when the scan keys can't be satisfied.

For parallel btree scan to work for array of scan keys, it should reach
BTPARALLEL_DONE state once for every distinct combination of array keys.
This is required to ensure that the parallel workers don't try to seize
blocks at the same time for different scan keys. We missed to update this
state when we discovered that the scan keys can't be satisfied.

Author: James Hunter
Reviewed-by: Amit Kapila
Tested-by: Justin Pryzby
Backpatch-through: 10, where it was introduced
Discussion: https://postgr.es/m/4248CABC-25E3-4809-B4D0-128E1BAABC3C@amazon.com

4 years agoAllow CURRENT_ROLE where CURRENT_USER is accepted
Peter Eisentraut [Thu, 17 Sep 2020 09:39:28 +0000 (11:39 +0200)]
Allow CURRENT_ROLE where CURRENT_USER is accepted

In the particular case of GRANTED BY, this is specified in the SQL
standard.  Since in PostgreSQL, CURRENT_ROLE is equivalent to
CURRENT_USER, and CURRENT_USER is already supported here, adding
CURRENT_ROLE is trivial.  The other cases are PostgreSQL extensions,
but for the same reason it also makes sense there.

Reviewed-by: Vik Fearing <vik@postgresfriends.org>
Reviewed-by: Asif Rehman <asifr.rehman@gmail.com>
Reviewed-by: Alvaro Herrera <alvherre@2ndquadrant.com>
Discussion: https://www.postgresql.org/message-id/flat/f2feac44-b4c5-f38f-3699-2851d6a76dc9%402ndquadrant.com

4 years agoAdd support for building GiST index by sorting.
Heikki Linnakangas [Thu, 17 Sep 2020 08:33:40 +0000 (11:33 +0300)]
Add support for building GiST index by sorting.

This adds a new optional support function to the GiST access method:
sortsupport. If it is defined, the GiST index is built by sorting all data
to the order defined by the sortsupport's comparator function, and packing
the tuples in that order to GiST pages. This is similar to how B-tree
index build works, and is much faster than inserting the tuples one by
one. The resulting index is smaller too, because the pages are packed more
tightly, upto 'fillfactor'. The normal build method works by splitting
pages, which tends to lead to more wasted space.

The quality of the resulting index depends on how good the opclass-defined
sort order is. A good order preserves locality of the input data.

As the first user of this facility, add 'sortsupport' function to the
point_ops opclass. It sorts the points in Z-order (aka Morton Code), by
interleaving the bits of the X and Y coordinates.

Author: Andrey Borodin
Reviewed-by: Pavel Borisov, Thomas Munro
Discussion: https://www.postgresql.org/message-id/1A36620E-CAD8-4267-9067-FB31385E7C0D%40yandex-team.ru

4 years agodoc: Apply more consistently <productname> markup for OpenSSL
Michael Paquier [Thu, 17 Sep 2020 07:33:22 +0000 (16:33 +0900)]
doc: Apply more consistently <productname> markup for OpenSSL

OpenSSL was quoted in inconsistent ways in many places of the docs,
sometimes with <application>, <productname> or just nothing.

Author: Daniel Gustafsson
Discussion: https://postgr.es/m/DA91E5F0-5F9D-41A7-A7A6-B91CDE0F1D63@yesql.se

4 years agoImprove tab completion of IMPORT FOREIGN SCHEMA in psql
Michael Paquier [Thu, 17 Sep 2020 02:49:29 +0000 (11:49 +0900)]
Improve tab completion of IMPORT FOREIGN SCHEMA in psql

It is not possible to get a list of foreign schemas as the server is not
known, so this provides instead a list of local schemas, which is more
useful than nothing if using a loopback server or having schema names
matching in the local and remote servers.

Author: Jeff Janes
Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/CAMkU=1wr7Roj41q-XiJs=Uyc2xCmHhcGGy7J-peJQK-e+w=ghw@mail.gmail.com

4 years agoTeach walsender to update its process title for replication commands.
Tom Lane [Thu, 17 Sep 2020 01:06:50 +0000 (21:06 -0400)]
Teach walsender to update its process title for replication commands.

Because the code path taken for SQL commands executed in a walsender
will update the process title, we pretty much have to update the
title for replication commands as well.  Otherwise, the title shows
"idle" for the rest of a logical walsender's lifetime once it's
executed any SQL command.

Playing with this, I confirm that a walsender now typically spends
most of its life reporting
walsender postgres [local] START_REPLICATION
Considering this in isolation, it might be better to have it say
walsender postgres [local] sending replication data
However, consistency with the other cases seems to be a stronger
argument.

In passing, remove duplicative pgstat_report_activity call.

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

4 years agoImprove formatting of create_help.pl and plperl_opmask.pl output.
Tom Lane [Thu, 17 Sep 2020 00:31:19 +0000 (20:31 -0400)]
Improve formatting of create_help.pl and plperl_opmask.pl output.

Adjust the whitespace in the emitted files so that it matches
what pgindent would do.  This makes the generated files look
like they match project style, and avoids confusion if someone
does run pgindent on the generated files.

Also, add probes.h to pgindent's exclusion list, because it can
confuse pgindent, plus there's not much point in processing it.

Daniel Gustafsson, additional fixes by me

Discussion: https://postgr.es/m/79ed5348-be7a-b647-dd40-742207186a22@2ndquadrant.com

4 years agoFix bogus completion tag usage in walsender
Alvaro Herrera [Wed, 16 Sep 2020 16:04:38 +0000 (13:04 -0300)]
Fix bogus completion tag usage in walsender

Since commit fd5942c18f97 (2012, 9.3-era), walsender has been sending
completion tags for certain replication commands twice -- and they're
not even consistent.  Apparently neither libpq nor JDBC have a problem
with it, but it's not kosher.  Fix by remove the EndCommand() call in
the common code path for them all, and inserting specific calls to
EndReplicationCommand() specifically in those places where it's needed.

EndReplicationCommand() is a new simple function to send the completion
tag for replication commands.  Do this instead of sending a generic
SELECT completion tag for them all, which was also pretty bogus (if
innocuous).  While at it, change StartReplication() to use
EndReplicationCommand() instead of pg_puttextmessage().

In commit 2f9661311b83, I failed to realize that replication commands
are not close-enough kin of regular SQL commands, so the
DROP_REPLICATION_SLOT tag I added is undeserved and a type pun.  Take it
out.

Backpatch to 13, where the latter commit appeared.  The duplicate tag
has been sent since 9.3, but since nothing is broken, it doesn't seem
worth fixing.

Per complaints from Tom Lane.

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

4 years agoCentralize setup of SIGQUIT handling for postmaster child processes.
Tom Lane [Wed, 16 Sep 2020 20:04:36 +0000 (16:04 -0400)]
Centralize setup of SIGQUIT handling for postmaster child processes.

We decided that the policy established in commit 7634bd4f6 for
the bgwriter, checkpointer, walwriter, and walreceiver processes,
namely that they should accept SIGQUIT at all times, really ought
to apply uniformly to all postmaster children.  Therefore, get
rid of the duplicative and inconsistent per-process code for
establishing that signal handler and removing SIGQUIT from BlockSig.
Instead, make InitPostmasterChild do it.

The handler set up by InitPostmasterChild is SignalHandlerForCrashExit,
which just summarily does _exit(2).  In interactive backends, we
almost immediately replace that with quickdie, since we would prefer
to try to tell the client that we're dying.  However, this patch is
changing the behavior of autovacuum (both launcher and workers), as
well as walsenders.  Those processes formerly also used quickdie,
but AFAICS that was just mindless copy-and-paste: they don't have
any interactive client that's likely to benefit from being told this.

The stats collector continues to be an outlier, in that it thinks
SIGQUIT means normal exit.  That should probably be changed for
consistency, but there's another patch set where that's being
dealt with, so I didn't do so here.

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

4 years agoDon't fetch partition check expression during InitResultRelInfo.
Tom Lane [Wed, 16 Sep 2020 18:28:11 +0000 (14:28 -0400)]
Don't fetch partition check expression during InitResultRelInfo.

Since there is only one place that actually needs the partition check
expression, namely ExecPartitionCheck, it's better to fetch it from
the relcache there.  In this way we will never fetch it at all if
the query never has use for it, and we still fetch it just once when
we do need it.

The reason for taking an interest in this is that if the relcache
doesn't already have the check expression cached, fetching it
requires obtaining AccessShareLock on the partition root.  That
means that operations that look like they should only touch the
partition itself will also take a lock on the root.  In particular
we observed that TRUNCATE on a partition may take a lock on the
partition's root, contributing to a deadlock situation in parallel
pg_restore.

As written, this patch does have a small cost, which is that we
are microscopically reducing efficiency for the case where a partition
has an empty check expression.  ExecPartitionCheck will be called,
and will go through the motions of setting up and checking an empty
qual, where before it would not have been called at all.  We could
avoid that by adding a separate boolean flag to track whether there
is a partition expression to test.  However, this case only arises
for a default partition with no siblings, which surely is not an
interesting case in practice.  Hence adding complexity for it
does not seem like a good trade-off.

Amit Langote, per a suggestion by me

Discussion: https://postgr.es/m/VI1PR03MB31670CA1BD9625C3A8C5DD05EB230@VI1PR03MB3167.eurprd03.prod.outlook.com