postgresql-pgindent.git
2 years agoUse MemoryContext API for regex memory management.
Thomas Munro [Sat, 8 Apr 2023 09:52:35 +0000 (21:52 +1200)]
Use MemoryContext API for regex memory management.

Previously, regex_t objects' memory was managed with malloc() and free()
directly.  Switch to palloc()-based memory management instead.
Advantages:

 * memory used by cached regexes is now visible with MemoryContext
   observability tools

 * cleanup can be done automatically in certain failure modes
   (something that later commits will take advantage of)

 * cleanup can be done in bulk

On the downside, there may be more fragmentation (wasted memory) due to
per-regex MemoryContext objects.  This is a problem shared with other
cached objects in PostgreSQL and can probably be improved with later
tuning.

Thanks to Noah Misch for suggesting this general approach, which
unblocks later work on interrupts.

Suggested-by: Noah Misch <[email protected]>
Reviewed-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/CA%2BhUKGK3PGKwcKqzoosamn36YW-fsuTdOPPF1i_rtEO%3DnEYKSg%40mail.gmail.com

2 years agoTAP test for logical decoding on standby
Andres Freund [Sat, 8 Apr 2023 09:24:50 +0000 (02:24 -0700)]
TAP test for logical decoding on standby

Author: "Drouvot, Bertrand" <[email protected]>
Author: Amit Khandekar <[email protected]>
Author: Craig Ringer <[email protected]> (in an older version)
Author: Andres Freund <[email protected]>
Reviewed-by: "Drouvot, Bertrand" <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
Reviewed-by: Robert Haas <[email protected]>
Reviewed-by: Amit Kapila <[email protected]>
Reviewed-by: Fabrízio de Royes Mello <[email protected]>
2 years agoAllow logical decoding on standbys
Andres Freund [Sat, 8 Apr 2023 09:20:01 +0000 (02:20 -0700)]
Allow logical decoding on standbys

Unsurprisingly, this requires wal_level = logical to be set on the primary and
standby. The infrastructure added in 26669757b6a ensures that slots are
invalidated if the primary's wal_level is lowered.

Creating a slot on a standby waits for a xl_running_xact record to be
processed. If the primary is idle (and thus not emitting xl_running_xact
records), that can take a while.  To make that faster, this commit also
introduces the pg_log_standby_snapshot() function. By executing it on the
primary, completion of slot creation on the standby can be accelerated.

Note that logical decoding on a standby does not itself enforce that required
catalog rows are not removed. The user has to use physical replication slots +
hot_standby_feedback or other measures to prevent that. If catalog rows
required for a slot are removed, the slot is invalidated.

See 6af1793954e for an overall design of logical decoding on a standby.

Bumps catversion, for the addition of the pg_log_standby_snapshot() function.

Author: "Drouvot, Bertrand" <[email protected]>
Author: Andres Freund <[email protected]> (in an older version)
Author: Amit Khandekar <[email protected]> (in an older version)
Reviewed-by: Andres Freund <[email protected]>
Reviewed-by: FabrÌzio de Royes Mello <[email protected]>
Reviewed-by: Amit Kapila <[email protected]>
Reviewed-By: Robert Haas <[email protected]>
2 years agoFor cascading replication, wake physical and logical walsenders separately
Andres Freund [Sat, 8 Apr 2023 07:24:24 +0000 (00:24 -0700)]
For cascading replication, wake physical and logical walsenders separately

Physical walsenders can't send data until it's been flushed; logical
walsenders can't decode and send data until it's been applied. On the
standby, the WAL is flushed first, which will only wake up physical
walsenders; and then applied, which will only wake up logical
walsenders.

Previously, all walsenders were awakened when the WAL was flushed. That
was fine for logical walsenders on the primary; but on the standby the
flushed WAL would have been not applied yet, so logical walsenders were
awakened too early.

Per idea from Jeff Davis and Amit Kapila.

Author: "Drouvot, Bertrand" <[email protected]>
Reviewed-By: Jeff Davis <[email protected]>
Reviewed-By: Robert Haas <[email protected]>
Reviewed-by: Amit Kapila <[email protected]>
Reviewed-by: Masahiko Sawada <[email protected]>
Discussion: https://postgr.es/m/CAA4eK1+zO5LUeisabX10c81LU-fWMKO4M9Wyg1cdkbW7Hqh6vQ@mail.gmail.com

2 years agoHandle logical slot conflicts on standby
Andres Freund [Sat, 8 Apr 2023 06:11:28 +0000 (23:11 -0700)]
Handle logical slot conflicts on standby

During WAL replay on the standby, when a conflict with a logical slot is
identified, invalidate such slots. There are two sources of conflicts:
1) Using the information added in 6af1793954e, logical slots are invalidated if
   required rows are removed
2) wal_level on the primary server is reduced to below logical

Uses the infrastructure introduced in the prior commit. FIXME: add commit
reference.

Change InvalidatePossiblyObsoleteSlot() to use a recovery conflict to
interrupt use of a slot, if called in the startup process. The new recovery
conflict is added to pg_stat_database_conflicts, as confl_active_logicalslot.

See 6af1793954e for an overall design of logical decoding on a standby.

Bumps catversion for the addition of the pg_stat_database_conflicts column.
Bumps PGSTAT_FILE_FORMAT_ID for the same reason.

Author: "Drouvot, Bertrand" <[email protected]>
Author: Andres Freund <[email protected]>
Author: Amit Khandekar <[email protected]> (in an older version)
Reviewed-by: "Drouvot, Bertrand" <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
Reviewed-by: Robert Haas <[email protected]>
Reviewed-by: Fabrízio de Royes Mello <[email protected]>
Reviewed-by: Bharath Rupireddy <[email protected]>
Reviewed-by: Amit Kapila <[email protected]>
Reviewed-by: Alvaro Herrera <[email protected]>
Discussion: https://postgr.es/m/20230407075009[email protected]

2 years agoSupport invalidating replication slots due to horizon and wal_level
Andres Freund [Sat, 8 Apr 2023 05:40:27 +0000 (22:40 -0700)]
Support invalidating replication slots due to horizon and wal_level

Needed for logical decoding on a standby. Slots need to be invalidated because
of the horizon if rows required for logical decoding are removed. If the
primary's wal_level is lowered from 'logical', logical slots on the standby
need to be invalidated.

The new invalidation methods will be used in a subsequent commit.

Logical slots that have been invalidated can be identified via the new
pg_replication_slots.conflicting column.

See 6af1793954e for an overall design of logical decoding on a standby.

Bumps catversion for the addition of the new pg_replication_slots column.

Author: "Drouvot, Bertrand" <[email protected]>
Author: Andres Freund <[email protected]>
Author: Amit Khandekar <[email protected]> (in an older version)
Reviewed-by: "Drouvot, Bertrand" <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
Reviewed-by: Robert Haas <[email protected]>
Reviewed-by: Fabrízio de Royes Mello <[email protected]>
Reviewed-by: Bharath Rupireddy <[email protected]>
Reviewed-by: Amit Kapila <[email protected]>
Reviewed-by: Melanie Plageman <[email protected]>
Reviewed-by: Alvaro Herrera <[email protected]>
Discussion: https://postgr.es/m/20230407075009[email protected]

2 years agoFix underspecified sort order in inherit.sql
Andres Freund [Sat, 8 Apr 2023 05:25:46 +0000 (22:25 -0700)]
Fix underspecified sort order in inherit.sql

Introduced in e056c557aef4.

Per buildfarm member prion.

2 years agoPrevent use of invalidated logical slot in CreateDecodingContext()
Andres Freund [Sat, 8 Apr 2023 05:19:05 +0000 (22:19 -0700)]
Prevent use of invalidated logical slot in CreateDecodingContext()

Previously we had checks for this in multiple places. Support for logical
decoding on standbys will add other forms of invalidation, making it worth
while to centralize the checks.

This slightly changes the error message for both the walsender and SQL
interface. Particularly the SQL interface error was inaccurate, as the "This
slot has never previously reserved WAL" portion was unreachable.

Reviewed-by: "Drouvot, Bertrand" <[email protected]>
Reviewed-by: Melanie Plageman <[email protected]>
Discussion: https://postgr.es/m/20230407075009[email protected]

2 years agoReplace replication slot's invalidated_at LSN with an enum
Andres Freund [Sat, 8 Apr 2023 04:47:25 +0000 (21:47 -0700)]
Replace replication slot's invalidated_at LSN with an enum

This is mainly useful because the upcoming logical-decoding-on-standby feature
adds further reasons for invalidating slots, and we don't want to end up with
multiple invalidated_* fields, or check different attributes.

Eventually we should consider not resetting restart_lsn when invalidating a
slot due to max_slot_wal_keep_size. But that's a user visible change, so left
for later.

Increases SLOT_VERSION, due to the changed field (with a different alignment,
no less).

Reviewed-by: "Drouvot, Bertrand" <[email protected]>
Reviewed-by: Alvaro Herrera <[email protected]>
Reviewed-by: Melanie Plageman <[email protected]>
Discussion: https://postgr.es/m/20230407075009[email protected]

2 years agoAdd io_direct setting (developer-only).
Thomas Munro [Fri, 7 Apr 2023 23:04:49 +0000 (11:04 +1200)]
Add io_direct setting (developer-only).

Provide a way to ask the kernel to use O_DIRECT (or local equivalent)
where available for data and WAL files, to avoid or minimize kernel
caching.  This hurts performance currently and is not intended for end
users yet.  Later proposed work would introduce our own I/O clustering,
read-ahead, etc to replace the facilities the kernel disables with this
option.

The only user-visible change, if the developer-only GUC is not used, is
that this commit also removes the obscure logic that would activate
O_DIRECT for the WAL when wal_sync_method=open_[data]sync and
wal_level=minimal (which also requires max_wal_senders=0).  Those are
non-default and unlikely settings, and this behavior wasn't (correctly)
documented.  The same effect can be achieved with io_direct=wal.

Author: Thomas Munro <[email protected]>
Author: Andres Freund <[email protected]>
Author: Bharath Rupireddy <[email protected]>
Reviewed-by: Justin Pryzby <[email protected]>
Reviewed-by: Bharath Rupireddy <[email protected]>
Discussion: https://postgr.es/m/CA%2BhUKGK1X532hYqJ_MzFWt0n1zt8trz980D79WbjwnT-yYLZpg%40mail.gmail.com

2 years agoIntroduce PG_IO_ALIGN_SIZE and align all I/O buffers.
Thomas Munro [Fri, 7 Apr 2023 22:38:09 +0000 (10:38 +1200)]
Introduce PG_IO_ALIGN_SIZE and align all I/O buffers.

In order to have the option to use O_DIRECT/FILE_FLAG_NO_BUFFERING in a
later commit, we need the addresses of user space buffers to be well
aligned.  The exact requirements vary by OS and file system (typically
sectors and/or memory pages).  The address alignment size is set to
4096, which is enough for currently known systems: it matches modern
sectors and common memory page size.  There is no standard governing
O_DIRECT's requirements so we might eventually have to reconsider this
with more information from the field or future systems.

Aligning I/O buffers on memory pages is also known to improve regular
buffered I/O performance.

Three classes of I/O buffers for regular data pages are adjusted:
(1) Heap buffers are now allocated with the new palloc_aligned() or
MemoryContextAllocAligned() functions introduced by commit 439f6175.
(2) Stack buffers now use a new struct PGIOAlignedBlock to respect
PG_IO_ALIGN_SIZE, if possible with this compiler.  (3) The buffer
pool is also aligned in shared memory.

WAL buffers were already aligned on XLOG_BLCKSZ.  It's possible for
XLOG_BLCKSZ to be configured smaller than PG_IO_ALIGNED_SIZE and thus
for O_DIRECT WAL writes to fail to be well aligned, but that's a
pre-existing condition and will be addressed by a later commit.

BufFiles are not yet addressed (there's no current plan to use O_DIRECT
for those, but they could potentially get some incidental speedup even
in plain buffered I/O operations through better alignment).

If we can't align stack objects suitably using the compiler extensions
we know about, we disable the use of O_DIRECT by setting PG_O_DIRECT to
0.  This avoids the need to consider systems that have O_DIRECT but
can't align stack objects the way we want; such systems could in theory
be supported with more work but we don't currently know of any such
machines, so it's easier to pretend there is no O_DIRECT support
instead.  That's an existing and tested class of system.

Add assertions that all buffers passed into smgrread(), smgrwrite() and
smgrextend() are correctly aligned, unless PG_O_DIRECT is 0 (= stack
alignment tricks may be unavailable) or the block size has been set too
small to allow arrays of buffers to be all aligned.

Author: Thomas Munro <[email protected]>
Author: Andres Freund <[email protected]>
Reviewed-by: Justin Pryzby <[email protected]>
Discussion: https://postgr.es/m/CA+hUKGK1X532hYqJ_MzFWt0n1zt8trz980D79WbjwnT-yYLZpg@mail.gmail.com

2 years agoDoc: Fix the datatype of the newly added SUBSCRIPTION options.
Amit Kapila [Sat, 8 Apr 2023 04:12:50 +0000 (09:42 +0530)]
Doc: Fix the datatype of the newly added SUBSCRIPTION options.

In docs, the datatype of "password_required" and "run_as_owner" was
incorrectly specified as a string.

Author: Amit Kapila
Reviewed-by: Sawada Masahiko
Discussion: https://postgr.es/m/CAHut+Pu=pnJf=SS1583pknSQ3CbOqLCkWcJCQYt6zxTagHEdmw@mail.gmail.com

2 years agoAdd missing .gitignore entry.
Tom Lane [Sat, 8 Apr 2023 03:32:49 +0000 (23:32 -0400)]
Add missing .gitignore entry.

Seems an oversight in 7d8219a44.  Fix before somebody commits
a generated file.

2 years agoRemove useless dependencies in daitch_mokotoff_header.pl.
Tom Lane [Sat, 8 Apr 2023 03:22:28 +0000 (23:22 -0400)]
Remove useless dependencies in daitch_mokotoff_header.pl.

Actually, the correct fix for this is "we don't need this at all",
because this program isn't dealing in any non-ASCII data.  The
dependency on Data::Dumper seems to be a leftover too.

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

2 years agoAdd support for Kerberos credential delegation
Stephen Frost [Sat, 8 Apr 2023 01:58:04 +0000 (21:58 -0400)]
Add support for Kerberos credential delegation

Support GSSAPI/Kerberos credentials being delegated to the server by a
client.  With this, a user authenticating to PostgreSQL using Kerberos
(GSSAPI) credentials can choose to delegate their credentials to the
PostgreSQL server (which can choose to accept them, or not), allowing
the server to then use those delegated credentials to connect to
another service, such as with postgres_fdw or dblink or theoretically
any other service which is able to be authenticated using Kerberos.

Both postgres_fdw and dblink are changed to allow non-superuser
password-less connections but only when GSSAPI credentials have been
delegated to the server by the client and GSSAPI is used to
authenticate to the remote system.

Authors: Stephen Frost, Peifeng Qiu
Reviewed-By: David Christensen
Discussion: https://postgr.es/m/CO1PR05MB8023CC2CB575E0FAAD7DF4F8A8E29@CO1PR05MB8023.namprd05.prod.outlook.com

2 years agoPacify perlcritic.
Tom Lane [Sat, 8 Apr 2023 01:32:44 +0000 (21:32 -0400)]
Pacify perlcritic.

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

2 years agoTrack IO times in pg_stat_io
Andres Freund [Fri, 7 Apr 2023 23:05:26 +0000 (16:05 -0700)]
Track IO times in pg_stat_io

a9c70b46dbe and 8aaa04b32S added counting of IO operations to a new view,
pg_stat_io. Now, add IO timing for reads, writes, extends, and fsyncs to
pg_stat_io as well.

This combines the tracking for pgBufferUsage with the tracking for pg_stat_io
into a new function pgstat_count_io_op_time(). This should make it a bit
easier to avoid the somewhat costly instr_time conversion done for
pgBufferUsage.

Author: Melanie Plageman <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
Reviewed-by: Bertrand Drouvot <[email protected]>
Discussion: https://postgr.es/m/flat/CAAKRu_ay5iKmnbXZ3DsauViF3eMxu4m1oNnJXqV_HyqYeg55Ww%40mail.gmail.com

2 years agoShow more detail in nbtree rmgr descriptions.
Peter Geoghegan [Fri, 7 Apr 2023 23:46:23 +0000 (16:46 -0700)]
Show more detail in nbtree rmgr descriptions.

Show a detailed description of the page offset number arrays that appear
in certain nbtree WAL records.

Also brings nbtree desc routines in line with the guidelines established
by recent commit 7d8219a4.

Author: Melanie Plageman <[email protected]>
Reviewed-By: Peter Geoghegan <[email protected]>
Discussion: https://postgr.es/m/flat/20230109215842.fktuhesvayno6o4g%40awork3.anarazel.de

2 years agoFor Kerberos testing, disable DNS lookups
Stephen Frost [Fri, 7 Apr 2023 23:36:46 +0000 (19:36 -0400)]
For Kerberos testing, disable DNS lookups

Similar to 8dff2f224, this disables DNS lookups by the Kerberos library
to look up the KDC and the realm while the Kerberos tests are running.
In some environments, these lookups can take a long time and end up
timing out and causing tests to fail.  Further, since this isn't really
our domain, we shouldn't be sending out these DNS requests during our
tests.

2 years agoShow more detail in heapam rmgr descriptions.
Peter Geoghegan [Fri, 7 Apr 2023 23:08:52 +0000 (16:08 -0700)]
Show more detail in heapam rmgr descriptions.

Add helper functions that output arrays in a standard format, and use
the functions inside heapdesc routines.  This allows tools like
pg_walinspect to show a detailed description of the page offset number
arrays for records like PRUNE and VACUUM (unless there was an FPI).

Also document the conventions that desc routines should follow.  Only
the heapdesc routines follow the conventions for now, so they're just
guidelines for the time being.

Based on a suggestion from Andres Freund.

Author: Melanie Plageman <[email protected]>
Reviewed-By: Peter Geoghegan <[email protected]>
Discussion: https://postgr.es/m/flat/20230109215842.fktuhesvayno6o4g%40awork3.anarazel.de

2 years agoAdjust contrib/sepgsql regression test expected outputs.
Tom Lane [Fri, 7 Apr 2023 22:17:22 +0000 (18:17 -0400)]
Adjust contrib/sepgsql regression test expected outputs.

Per buildfarm, the log output has changed as a consequence of
commit e056c557a changing the catalog accesses performed in
some commands.

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

2 years agoAdd support for Daitch-Mokotoff Soundex in contrib/fuzzystrmatch.
Tom Lane [Fri, 7 Apr 2023 21:31:51 +0000 (17:31 -0400)]
Add support for Daitch-Mokotoff Soundex in contrib/fuzzystrmatch.

This modernized version of Soundex works significantly better than
the original, particularly for non-English names.

Dag Lem, reviewed by quite a few people along the way

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

2 years agoFix table name clash in recently introduced test
Andres Freund [Fri, 7 Apr 2023 21:02:46 +0000 (14:02 -0700)]
Fix table name clash in recently introduced test

A few buildfarm animals recently started complaining about the "child"
relation already existing. e056c557aef added a new child table to inherit.sql,
but triggers.sql, running in the same parallel group, also uses a child table.

Rename the new table to inh_child. It maybe worth renaming child, parent in
other tests as well, but that's work for another day.

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

2 years agoImprove IO accounting for temp relation writes
Andres Freund [Fri, 7 Apr 2023 20:24:26 +0000 (13:24 -0700)]
Improve IO accounting for temp relation writes

Both pgstat_database and pgBufferUsage count IO timing for reads of temporary
relation blocks into local buffers. However, both failed to count write IO
timing for flushes of dirty local buffers. Fix.

Additionally, FlushRelationBuffers() seems to have omitted counting write
IO (both count and timing) stats for both pgstat_database and
pgBufferUsage. Fix.

Author: Melanie Plageman <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/20230321023451.7rzy4kjj2iktrg2r%40awork3.anarazel.de

2 years agoTest SCRAM iteration changes with psql \password
Daniel Gustafsson [Fri, 7 Apr 2023 20:14:23 +0000 (22:14 +0200)]
Test SCRAM iteration changes with psql \password

A version of this test was included in the original patch for altering
SCRAM iteration count, but was omitted due to how interactive psql TAP
sessions worked before being refactored.

Discussion: https://postgr.es/m/20230130194350[email protected]
Discussion: https://postgr.es/m/F72E7BC7-189F-4B17-BF47-9735EB72C364@yesql.se

2 years agoRefactor background psql TAP functions
Daniel Gustafsson [Fri, 7 Apr 2023 20:14:20 +0000 (22:14 +0200)]
Refactor background psql TAP functions

This breaks out the background and interactive psql functionality into a
new class, PostgreSQL::Test::BackgroundPsql.  Sessions are still initiated
via PostgreSQL::Test::Cluster, but once started they can be manipulated by
the new helper functions which intend to make querying easier.  A sample
session for a command which can be expected to finish at a later time can
be seen below.

  my $session = $node->background_psql('postgres');
  $bsession->query_until(qr/start/, q(
    \echo start
CREATE INDEX CONCURRENTLY idx ON t(a);
  ));
  $bsession->quit;

Patch by Andres Freund with some additional hacking by me.

Author: Andres Freund <[email protected]>
Reviewed-by: Andrew Dunstan <[email protected]>
Discussion: https://postgr.es/m/20230130194350[email protected]

2 years agoFix underspecified sort order in test query
Alvaro Herrera [Fri, 7 Apr 2023 18:19:16 +0000 (20:19 +0200)]
Fix underspecified sort order in test query

Fail in e056c557aef4.

2 years agoAdd pg_buffercache_usage_counts() to contrib/pg_buffercache.
Tom Lane [Fri, 7 Apr 2023 18:25:45 +0000 (14:25 -0400)]
Add pg_buffercache_usage_counts() to contrib/pg_buffercache.

It was pointed out that pg_buffercache_summary()'s report of
the overall average usage count isn't that useful, and what
would be more helpful in many cases is to report totals for
each possible usage count.  Add a new function to do it like
that.  Since pg_buffercache 1.4 is already new for v16,
we don't need to create a new extension version; we'll just
define this as part of 1.4.

Nathan Bossart

Discussion: https://postgr.es/m/20230130233040.GA2800702@nathanxps13

2 years agoCatalog NOT NULL constraints
Alvaro Herrera [Fri, 7 Apr 2023 17:20:53 +0000 (19:20 +0200)]
Catalog NOT NULL constraints

We now create pg_constaint rows for NOT NULL constraints with
contype='n'.

We propagate these constraints during operations such as adding
inheritance relationships, creating and attaching partitions, creating
tables LIKE other tables.  We mostly follow the well-known rules of
conislocal and coninhcount that we have for CHECK constraints, with some
adaptations; for example, as opposed to CHECK constraints, we don't
match NOT NULL ones by name when descending a hierarchy to alter it;
instead we match by column number.  This means we don't require the
constraint names to be identical across a hierarchy.

For now, we omit them from system catalogs.  Maybe this is worth
reconsidering.  We don't support NOT VALID nor DEFERRABLE clauses
either; these can be added as separate features later (this patch is
already large and complicated enough.)

This has been very long in the making.  The first patch was written by
Bernd Helmle in 2010 to add a new pg_constraint.contype value ('n'),
which I (Álvaro) then hijacked in 2011 and 2012, until that one was
killed by the realization that we ought to use contype='c' instead:
manufactured CHECK constraints.  However, later SQL standard
development, as well as nonobvious emergent properties of that design
(mostly, failure to distinguish them from "normal" CHECK constraints as
well as the performance implication of having to test the CHECK
expression) led us to reconsider this choice, so now the current
implementation uses contype='n' again.

In 2016 Vitaly Burovoy also worked on this feature[1] but found no
consensus for his proposed approach, which was claimed to be closer to
the letter of the standard, requiring additional pg_attribute columns to
track the OID of the NOT NULL constraint for that column.
[1] https://postgr.es/m/CAKOSWNkN6HSyatuys8xZxzRCR-KL1OkHS5-b9qd9bf1Rad3PLA@mail.gmail.com

Author: Álvaro Herrera <[email protected]>
Author: Bernd Helmle <[email protected]>
Reviewed-by: Justin Pryzby <[email protected]>
Reviewed-by: Peter Eisentraut <[email protected]>
Discussion: https://postgr.es/m/CACA0E642A0267EDA387AF2B%40%5B172.26.14.62%5D
Discussion: https://postgr.es/m/[email protected]
Discussion: https://postgr.es/m/20110707213401[email protected]
Discussion: https://postgr.es/m/1343682669[email protected]
Discussion: https://postgr.es/m/CAKOSWNkN6HSyatuys8xZxzRCR-KL1OkHS5-b9qd9bf1Rad3PLA@mail.gmail.com
Discussion: https://postgr.es/m/20220817181249[email protected]

2 years agoDoc: improve descriptions of max_[pred_]locks_per_transaction GUCs.
Tom Lane [Fri, 7 Apr 2023 17:29:29 +0000 (13:29 -0400)]
Doc: improve descriptions of max_[pred_]locks_per_transaction GUCs.

The old wording described these as being multiplied by max_connections
plus max_prepared_transactions, which hasn't been exactly right for
some time thanks to the addition of various auxiliary processes.
Moreover, exactness here is a bit pointless given that the lock tables
can expand into the initially-unallocated "slop" space in shared
memory.  Rather than trying to track exactly what the code is doing,
let's just use the term "server processes".

Likewise adjust these GUCs' description strings in guc_tables.c.

Wang Wei, reviewed by Nathan Bossart and myself

Discussion: https://postgr.es/m/OS3PR01MB6275BDD09C9B875C65FCC5AB9EA39@OS3PR01MB6275.jpnprd01.prod.outlook.com

2 years agoAdd array_sample() and array_shuffle() functions.
Tom Lane [Fri, 7 Apr 2023 15:47:07 +0000 (11:47 -0400)]
Add array_sample() and array_shuffle() functions.

These are useful in Monte Carlo applications.

Martin Kalcher, reviewed/adjusted by Daniel Gustafsson and myself

Discussion: https://postgr.es/m/9d160a44-7675-51e8-60cf-6d64b76db831@aboutsource.net

2 years agoFix locale-dependent test case.
Tom Lane [Fri, 7 Apr 2023 14:35:11 +0000 (10:35 -0400)]
Fix locale-dependent test case.

psql parses the interval argument of \watch with locale-dependent
strtod().  In commit 00beecfe8 I added a test case that exercises
a fractional interval, but I hard-coded 0.01 which doesn't work
in locales where the radix point isn't ".".  We don't want to
change this longstanding parsing behavior, so fix the test case
to generate a suitably locale-aware spelling.

Report and patch by Alexander Korotkov.

Discussion: https://postgr.es/m/CAPpHfdv+10Uk6FWjsh3+ju7kHYr76LaRXbYayXmrM7FBU-=Hgg@mail.gmail.com

2 years agoFix copy-paste bug in 12f3867f553 triggering an assert after a write error
Andres Freund [Fri, 7 Apr 2023 08:02:46 +0000 (01:02 -0700)]
Fix copy-paste bug in 12f3867f553 triggering an assert after a write error

The same condition accidentally was copied to both branches. Manual testing
confirms that otherwise the error recovery path works fine.

Found while reviewing the logical-decoding-on-standby patch.

2 years agoAdd tab-completion for newly added SUBSCRIPTION options.
Amit Kapila [Fri, 7 Apr 2023 05:02:36 +0000 (10:32 +0530)]
Add tab-completion for newly added SUBSCRIPTION options.

Commits c3afe8cf5a and 482675987b added new subscription options
"password_required" and "run_as_owner". This patch adds tab-completion
for these newly added options.

Author: Peter Smith
Discussion: https://postgr.es/m/CAHut+Pu=pnJf=SS1583pknSQ3CbOqLCkWcJCQYt6zxTagHEdmw@mail.gmail.com

2 years agoAdd more protections in WAL record APIs against overflows
Michael Paquier [Fri, 7 Apr 2023 01:10:17 +0000 (10:10 +0900)]
Add more protections in WAL record APIs against overflows

This commit adds a limit to the size of an XLogRecord at 1020MB, based
on a suggestion by Heikki Linnakangas.  This counts for the overhead
needed by the XLogReader when allocating the memory it needs to read a
record in DecodeXLogRecordRequiredSpace(), based on the record size.  An
assertion based on that is added to detect that any additions in the
XLogReader facilities would not cause any overflows.  If that's ever the
case, the upper bound allowed would need to be adjusted.

Before this, it was possible for an external module to create WAL
records large enough to be assembled but not replayable, causing
failures when replaying such WAL records on standbys.  One case
mentioned where this is possible is the in-core function
pg_logical_emit_message() (wrapper for LogLogicalMessage), that allows
to emit WAL records with an arbitrary amount of data potentially higher
than the replay limit of approximately 1GB (limit of a palloc, minus the
overhead needed by a XLogReader).

This commit is a follow-up of ffd1b6b that has added similar protections
for the block-level data.  Here, the checks are extended to the whole
record length, mainrdata_len being extended from uint32 to uint64 with
the routines registering buffer and record data still limited to uint32
to minimize the checks when assembling a record.  All the error messages
related to overflow checks are improved to provide more context about
the error happening.

Author: Matthias van de Meent
Reviewed-by: Andres Freund, Heikki Linnakangas, Michael Paquier
Discussion: https://postgr.es/m/CAEze2WgGiw+LZt+vHf8tWqB_6VxeLsMeoAuod0N=ij1q17n5pw@mail.gmail.com

2 years agoUse ExtendBufferedRelTo() in XLogReadBufferExtended()
Andres Freund [Fri, 7 Apr 2023 00:45:42 +0000 (17:45 -0700)]
Use ExtendBufferedRelTo() in XLogReadBufferExtended()

Instead of extending the relation block-by-block, use ExtendBufferedRelTo(),
introduced in 31966b151e6. This is faster and simpler.

This also somewhat reduces the danger that disconnected segments pose (which
can be "discovered" once the previous segment reaches SEGSIZE), as
ExtendBufferedRelTo() won't extend past the block it has been asked. However,
the risk of the content of such a disconnected segment being invalid
remains.

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

2 years agoAdd --buffer-usage-limit option to vacuumdb
David Rowley [Fri, 7 Apr 2023 00:47:10 +0000 (12:47 +1200)]
Add --buffer-usage-limit option to vacuumdb

1cbbee033 added BUFFER_USAGE_LIMIT to the VACUUM and ANALYZE commands, so
here we permit that option to be specified in vacuumdb.

In passing, adjust the documents for vacuum_buffer_usage_limit and the
BUFFER_USAGE_LIMIT VACUUM option to mention "kB" rather than "KB".  Do the
same for the ERROR message in ExecVacuum() and
check_vacuum_buffer_usage_limit().  Without that we might tell a user that
the valid minimum value is 128 KB only to reject that because we accept
only "kB" and not "KB".

Also, add a small reminder comment in vacuum.h to try to trigger the
memory of anyone adding new fields to VacuumParams that they might want to
consider if vacuumdb needs to grow a new option too.

Author: Melanie Plageman
Reviewed-by: Justin Pryzby
Reviewed-by: David Rowley
Discussion: https://postgr.es/m/[email protected]

2 years agohio: Use ExtendBufferedRelBy() to extend tables more efficiently
Andres Freund [Thu, 6 Apr 2023 23:35:21 +0000 (16:35 -0700)]
hio: Use ExtendBufferedRelBy() to extend tables more efficiently

While we already had some form of bulk extension for relations, it was fairly
limited. It only amortized the cost of acquiring the extension lock, the
relation itself was still extended one-by-one. Bulk extension was also solely
triggered by contention, not by the amount of data inserted.

To address this, use ExtendBufferedRelBy(), introduced in 31966b151e6, to
extend the relation. We try to extend the relation by multiple blocks in two
situations:

1) The caller tells RelationGetBufferForTuple() that it will need multiple
   pages. For now that's only used by heap_multi_insert(), see commit FIXME.

2) If there is contention on the extension lock, use the number of waiters for
   the lock as a multiplier for the number of blocks to extend by. This is
   similar to what we already did. Previously we additionally multiplied the
   numbers of waiters by 20, but with the new relation extension
   infrastructure I could not see a benefit in doing so.

Using the freespacemap to provide empty pages can cause significant
contention, and adds measurable overhead, even if there is no contention. To
reduce that, remember the blocks the relation was extended by in the
BulkInsertState, in the extending backend. In case 1) from above, the blocks
the extending backend needs are not entered into the FSM, as we know that we
will need those blocks.

One complication with using the FSM to record empty pages, is that we need to
insert blocks into the FSM, when we already hold a buffer content lock. To
avoid doing IO while holding a content lock, release the content lock before
recording free space. Currently that opens a small window in which another
backend could fill the block, if a concurrent VACUUM records the free
space. If that happens, we retry, similar to the already existing case when
otherBuffer is provided. In the future it might be worth closing the race by
preventing VACUUM from recording the space in newly extended pages.

This change provides very significant wins (3x at 16 clients, on my
workstation) for concurrent COPY into a single relation. Even single threaded
COPY is measurably faster, primarily due to not dirtying pages while
extending, if supported by the operating system (see commit 4d330a61bb1). Even
single-row INSERTs benefit, although to a much smaller degree, as the relation
extension lock rarely is the primary bottleneck.

Reviewed-by: Melanie Plageman <[email protected]>
Discussion: https://postgr.es/m/20221029025420[email protected]

2 years agoAdd VACUUM/ANALYZE BUFFER_USAGE_LIMIT option
David Rowley [Thu, 6 Apr 2023 23:40:31 +0000 (11:40 +1200)]
Add VACUUM/ANALYZE BUFFER_USAGE_LIMIT option

Add new options to the VACUUM and ANALYZE commands called
BUFFER_USAGE_LIMIT to allow users more control over how large to make the
buffer access strategy that is used to limit the usage of buffers in
shared buffers.  Larger rings can allow VACUUM to run more quickly but
have the drawback of VACUUM possibly evicting more buffers from shared
buffers that might be useful for other queries running on the database.

Here we also add a new GUC named vacuum_buffer_usage_limit which controls
how large to make the access strategy when it's not specified in the
VACUUM/ANALYZE command.  This defaults to 256KB, which is the same size as
the access strategy was prior to this change.  This setting also
controls how large to make the buffer access strategy for autovacuum.

Per idea by Andres Freund.

Author: Melanie Plageman
Reviewed-by: David Rowley
Reviewed-by: Andres Freund
Reviewed-by: Justin Pryzby
Reviewed-by: Bharath Rupireddy
Discussion: https://postgr.es/m/20230111182720[email protected]

2 years agoheapam: Pass number of required pages to RelationGetBufferForTuple()
Andres Freund [Thu, 6 Apr 2023 21:18:24 +0000 (14:18 -0700)]
heapam: Pass number of required pages to RelationGetBufferForTuple()

A future commit will use this information to determine how aggressively to
extend the relation by. In heap_multi_insert() we know accurately how many
pages we need once we need to extend the relation, providing an accurate lower
bound for how much to extend.

Reviewed-by: Melanie Plageman <[email protected]>
Discussion: https://postgr.es/m/20221029025420[email protected]

2 years agoRefresh cost-based delay params more frequently in autovacuum
Daniel Gustafsson [Thu, 6 Apr 2023 23:00:21 +0000 (01:00 +0200)]
Refresh cost-based delay params more frequently in autovacuum

Allow autovacuum to reload the config file more often so that cost-based
delay parameters can take effect while VACUUMing a relation. Previously,
autovacuum workers only reloaded the config file once per relation
vacuumed, so config changes could not take effect until beginning to
vacuum the next table.

Now, check if a reload is pending roughly once per block, when checking
if we need to delay.

In order for autovacuum workers to safely update their own cost delay
and cost limit parameters without impacting performance, we had to
rethink when and how these values were accessed.

Previously, an autovacuum worker's wi_cost_limit was set only at the
beginning of vacuuming a table, after reloading the config file.
Therefore, at the time that autovac_balance_cost() was called, workers
vacuuming tables with no cost-related storage parameters could still
have different values for their wi_cost_limit_base and wi_cost_delay.

Now that the cost parameters can be updated while vacuuming a table,
workers will (within some margin of error) have no reason to have
different values for cost limit and cost delay (in the absence of
cost-related storage parameters). This removes the rationale for keeping
cost limit and cost delay in shared memory. Balancing the cost limit
requires only the number of active autovacuum workers vacuuming a table
with no cost-based storage parameters.

Author: Melanie Plageman <[email protected]>
Reviewed-by: Masahiko Sawada <[email protected]>
Reviewed-by: Daniel Gustafsson <[email protected]>
Reviewed-by: Kyotaro Horiguchi <[email protected]>
Reviewed-by: Robert Haas <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/CAAKRu_ZngzqnEODc7LmS1NH04Kt6Y9huSjz5pp7%2BDXhrjDA0gw%40mail.gmail.com

2 years agoSeparate vacuum cost variables from GUCs
Daniel Gustafsson [Thu, 6 Apr 2023 22:54:53 +0000 (00:54 +0200)]
Separate vacuum cost variables from GUCs

Vacuum code run both by autovacuum workers and a backend doing
VACUUM/ANALYZE previously inspected VacuumCostLimit and VacuumCostDelay,
which are the global variables backing the GUCs vacuum_cost_limit and
vacuum_cost_delay.

Autovacuum workers needed to override these variables with their
own values, derived from autovacuum_vacuum_cost_limit and
autovacuum_vacuum_cost_delay and worker cost limit balancing logic.
This led to confusing code which, in some cases, both derived and
set a new value of VacuumCostLimit from VacuumCostLimit.

In preparation for refreshing these GUC values more often, introduce
new, independent global variables and add a function to update them
using the GUCs and existing logic.

Per suggestion by Kyotaro Horiguchi

Author: Melanie Plageman <[email protected]>
Reviewed-by: Masahiko Sawada <[email protected]>
Reviewed-by: Daniel Gustafsson <[email protected]>
Reviewed-by: Kyotaro Horiguchi <[email protected]>
Reviewed-by: Robert Haas <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/CAAKRu_ZngzqnEODc7LmS1NH04Kt6Y9huSjz5pp7%2BDXhrjDA0gw%40mail.gmail.com

2 years agoMake vacuum failsafe_active globally visible
Daniel Gustafsson [Thu, 6 Apr 2023 22:54:08 +0000 (00:54 +0200)]
Make vacuum failsafe_active globally visible

While vacuuming a table in failsafe mode, VacuumCostActive should
not be re-enabled.  This currently isn't a problem because vacuum
cost parameters are only refreshed in between vacuuming tables and
failsafe status is reset for every table.

In preparation for allowing vacuum cost parameters to be updated
more frequently, elevate LVRelState->failsafe_active to a global,
VacuumFailsafeActive, which will be checked when determining whether
or not to re-enable vacuum cost-related delays.

Author: Melanie Plageman <[email protected]>
Reviewed-by: Masahiko Sawada <[email protected]>
Reviewed-by: Daniel Gustafsson <[email protected]>
Reviewed-by: Kyotaro Horiguchi <[email protected]>
Reviewed-by: Robert Haas <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/CAAKRu_ZngzqnEODc7LmS1NH04Kt6Y9huSjz5pp7%2BDXhrjDA0gw%40mail.gmail.com

2 years agoStabilize just-added regression test cases.
Tom Lane [Thu, 6 Apr 2023 22:13:49 +0000 (18:13 -0400)]
Stabilize just-added regression test cases.

The tests added by commits 029dea882 et al turn out to produce
different output under -DRANDOMIZE_ALLOCATED_MEMORY.  This is
not a bug exactly: that flag causes coerce_type() to invoke
the input function twice when coercing an unknown-type literal
to a specific type.  So you get tsqueryin's bleat about an empty
tsquery twice.  Revise the test query to avoid that.

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

2 years agopsql: set SHELL_ERROR and SHELL_EXIT_CODE in more places.
Tom Lane [Thu, 6 Apr 2023 21:33:38 +0000 (17:33 -0400)]
psql: set SHELL_ERROR and SHELL_EXIT_CODE in more places.

Make the \g, \o, \w, and \copy commands set these variables
when closing a pipe.  We missed doing this in commit b0d8f2d98,
but it seems like a good idea.

There are some remaining places in psql that intentionally don't
update these variables after running a child program:
* pager invocations
* backtick evaluation within a prompt
* \e (edit query buffer)

Corey Huinker and Tom Lane

Discussion: https://postgr.es/m/CADkLM=eSKwRGF-rnRqhtBORRtL49QsjcVUCa-kLxKTqxypsakw@mail.gmail.com

2 years agoFix ts_headline() edge cases for empty query and empty search text.
Tom Lane [Thu, 6 Apr 2023 19:52:37 +0000 (15:52 -0400)]
Fix ts_headline() edge cases for empty query and empty search text.

tsquery's GETQUERY() macro is only safe to apply to a tsquery
that is known non-empty; otherwise it gives a pointer to garbage.
Before commit 5a617d75d, ts_headline() avoided this pitfall, but
only in a very indirect, nonobvious way.  (hlCover could not reach
its TS_execute call, because if the query contains no lexemes
then hlFirstIndex would surely return -1.)  After that commit,
it fell into the trap, resulting in weird errors such as
"unrecognized operator" and/or valgrind complaints.  In HEAD,
fix this by not calling TS_execute_locations() at all for an
empty query.  In the back branches, add a defensive check to
hlCover() --- that's not fixing any live bug, but I judge the
code a bit too fragile as-is.

Also, both mark_hl_fragments() and mark_hl_words() were careless
about the possibility of empty search text: in the cases where
no match has been found, they'd end up telling mark_fragment() to
mark from word indexes 0 to 0 inclusive, even when there is no
word 0.  This is harmless since we over-allocated the prs->words
array, but it does annoy valgrind.  Fix so that the end index is -1
and thus mark_fragment() will do nothing in such cases.

Bottom line is that this fixes a live bug in HEAD, but in the
back branches it's only getting rid of a valgrind nitpick.
Back-patch anyway.

Per report from Alexander Lakhin.

Discussion: https://postgr.es/m/c27f642d-020b-01ff-ae61-086af287c4fd@gmail.com

2 years agohio: Don't pin the VM while holding buffer lock while extending
Andres Freund [Thu, 6 Apr 2023 18:11:13 +0000 (11:11 -0700)]
hio: Don't pin the VM while holding buffer lock while extending

Starting with commit 7db0cd2145f, RelationGetBufferForTuple() did a
visibilitymap_pin() while holding an exclusive buffer content lock on a newly
extended page, when using COPY FREEZE. We elsewhere try hard to avoid to doing
IO while holding a content lock. And until 14f98e0af99, that happened while
holding the relation extension lock.

Practically, this isn't a huge issue, because COPY FREEZE is restricted to
relations created or truncated in the current session, so it's unlikely
there's a lot of contention.

We can't avoid doing IO while holding the content lock by pinning the VM
earlier, because we don't know which page it will be on.

While we could just ignore the issue in this case, a future commit will add
bulk relation extension, which needs to enter pages into the FSM while also
trying to hold onto a buffer lock.

To address this issue, use visibilitymap_pin_ok() to see if the relevant
buffer is already pinned. If not, release the buffer, pin the VM buffer, and
acquire the lock again. This opens up a small window for other backends to
insert data onto the page - as the page is not entered into the freespacemap,
other backends won't see it normally, but a concurrent vacuum could enter the
page, if it started just after the relation is extended. In case the page is
used by another backend, retry. This is very similar to how locking
"otherBuffer" is already dealt with.

Reviewed-by: Tomas Vondra <[email protected]>
Discussion: http://postgr.es/m/20230325025740[email protected]

2 years agohio: Relax rules for calling GetVisibilityMapPins()
Andres Freund [Thu, 6 Apr 2023 17:27:30 +0000 (10:27 -0700)]
hio: Relax rules for calling GetVisibilityMapPins()

GetVisibilityMapPins() insisted on the buffer1/buffer2 being in a specific
order. This required checks at the callsite. As a subsequent patch will add
another callsite, move related logic into GetVisibilityMapPins().

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

2 years agopsql: add an optional execution-count limit to \watch.
Tom Lane [Thu, 6 Apr 2023 17:18:14 +0000 (13:18 -0400)]
psql: add an optional execution-count limit to \watch.

\watch can now be told to stop after N executions of the query.

With the idea that we might want to add more options to \watch
in future, this patch generalizes the command's syntax to a list
of name=value options, with the interval allowed to omit the name
for backwards compatibility.

Andrey Borodin, reviewed by Kyotaro Horiguchi, Nathan Bossart,
Michael Paquier, Yugo Nagata, and myself

Discussion: https://postgr.es/m/CAAhFRxiZ2-n_L1ErMm9AZjgmUK=qS6VHb+0SaMn8sqqbhF7How@mail.gmail.com

2 years agoSupport long distance matching for zstd compression
Tomas Vondra [Thu, 6 Apr 2023 15:18:38 +0000 (17:18 +0200)]
Support long distance matching for zstd compression

zstd compression supports a special mode for finding matched in distant
past, which may result in better compression ratio, at the expense of
using more memory (the window size is 128MB).

To enable this optional mode, use the "long" keyword when specifying the
compression method (--compress=zstd:long).

Author: Justin Pryzby
Reviewed-by: Tomas Vondra, Jacob Champion
Discussion: https://postgr.es/m/20230224191840[email protected]
Discussion: https://postgr.es/m/20220327205020[email protected]

2 years agopostgres_fdw: Add support for parallel abort.
Etsuro Fujita [Thu, 6 Apr 2023 08:30:00 +0000 (17:30 +0900)]
postgres_fdw: Add support for parallel abort.

postgres_fdw aborts remote (sub)transactions opened on remote server(s)
in a local (sub)transaction one by one when the local (sub)transaction
aborts.  This patch allows it to abort the remote (sub)transactions in
parallel to improve performance.  This is enabled by the server option
"parallel_abort".  The default is false.

Etsuro Fujita, reviewed by David Zhang.

Discussion: http://postgr.es/m/CAPmGK15FuPVGx3TGHKShsbPKKtF1y58-ZLcKoxfN-nqLj1dZ%3Dg%40mail.gmail.com

2 years agoMove various prechecks from vacuum() into ExecVacuum()
David Rowley [Thu, 6 Apr 2023 03:44:52 +0000 (15:44 +1200)]
Move various prechecks from vacuum() into ExecVacuum()

vacuum() is used for both the VACUUM command and for autovacuum. There
were many prechecks being done inside vacuum() that were just not relevant
to autovacuum.  Let's move the bulk of these into ExecVacuum() so that
they're only executed when running the VACUUM command.  This removes a
small amount of overhead when autovacuum vacuums a table.

While we are at it, allocate VACUUM's BufferAccessStrategy in ExecVacuum()
and pass it into vacuum() instead of expecting vacuum() to make it if it's
not already made by the calling function.  To make this work, we need to
create the vacuum memory context slightly earlier, so we now need to pass
that down to vacuum() so that it's available for use in other memory
allocations.

Author: Melanie Plageman
Reviewed-by: David Rowley
Discussion: https://postgr.es/m/20230405211534[email protected]

2 years agoConvert many uses of ReadBuffer[Extended](P_NEW) to ExtendBufferedRel()
Andres Freund [Thu, 6 Apr 2023 01:57:29 +0000 (18:57 -0700)]
Convert many uses of ReadBuffer[Extended](P_NEW) to ExtendBufferedRel()

A few places are not converted. Some because they are tackled in later
commits (e.g. hio.c, xlogutils.c), some because they are more
complicated (e.g. brin_pageops.c).  Having a few users of ReadBuffer(P_NEW) is
good anyway, to ensure the backward compat path stays working.

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

2 years agoUse ExtendBufferedRelTo() in {vm,fsm}_extend()
Andres Freund [Thu, 6 Apr 2023 00:29:57 +0000 (17:29 -0700)]
Use ExtendBufferedRelTo() in {vm,fsm}_extend()

This uses ExtendBufferedRelTo(), introduced in 31966b151e6, to extend the
visibilitymap and freespacemap to the size needed.

It also happens to fix a warning introduced in 3d6a98457d8, reported by Tom
Lane.

Discussion: https://postgr.es/m/20221029025420[email protected]
Discussion: https://postgr.es/m/2194723.1680736788@sss.pgh.pa.us

2 years agoAlways make a BufferAccessStrategy for ANALYZE
David Rowley [Thu, 6 Apr 2023 00:37:03 +0000 (12:37 +1200)]
Always make a BufferAccessStrategy for ANALYZE

32fbe0239 changed things so we didn't bother allocating the
BufferAccessStrategy during VACUUM (ONLY_DATABASE_STATS); and VACUUM
(FULL), however, it forgot to consider that VACUUM (FULL, ANALYZE) is a
possible combination.  That change would have resulted in such a command
allowing ANALYZE to make full use of shared buffers, which wasn't
intended, so fix that.

Reported-by: Melanie Plageman
Discussion: https://postgr.es/m/CAAKRu_bJRKe+v_=OqwC+5sA3j5qv8rqdAwy3+yHaO3wmtfrCRg@mail.gmail.com

2 years agoFix row tracking in pg_stat_statements with extended query protocol
Michael Paquier [Thu, 6 Apr 2023 00:29:03 +0000 (09:29 +0900)]
Fix row tracking in pg_stat_statements with extended query protocol

pg_stat_statements relies on EState->es_processed to count the number of
rows processed by ExecutorRun().  This proves to be a problem under the
extended query protocol when the result of a query is fetched through
more than one call of ExecutorRun(), as es_processed is reset each time
ExecutorRun() is called.  This causes pg_stat_statements to report the
number of rows calculated in the last execute fetch, rather than the
global sum of all the rows processed.

As pquery.c tells, this is a problem when a portal does not use
holdStore.  For example, DMLs with RETURNING would report a correct
tuple count as these do one execution cycle when the query is first
executed to fill in the portal's store with one ExecutorRun(), feeding
on the portal's store for each follow-up execute fetch depending on the
fetch size requested by the client.

The fix proposed for this issue is simple with the addition of an extra
counter in EState that's preserved across multiple ExecutorRun() calls,
incremented with the value calculated in es_processed.  This approach is
not back-patchable, unfortunately.

Note that libpq does not currently give any way to control the fetch
size when using the extended v3 protocol, meaning that in-core testing
is not possible yet.  This issue can be easily verified with the JDBC
driver, though, with *autocommit disabled*.  Hence, having in-core tests
requires more features, left for future discussion:
- At least two new libpq routines splitting PQsendQueryGuts(), one for
the bind/describe and a second for a series of execute fetches with a
custom fetch size, likely in a fashion similar to what JDBC does.
- A psql meta-command for the execute phase.  This part is not strictly
mandatory, still it could be handy.

Reported-by: Andrew Dunstan (original discovery by Simon Siggs)
Author: Sami Imseih
Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/EBE6C507-9EB6-4142-9E4D-38B1673363A7@amazon.com
Discussion: https://postgr.es/m/c90890e7-9c89-c34f-d3c5-d5c763a34bd8@dunslane.net

2 years agobufmgr: Introduce infrastructure for faster relation extension
Andres Freund [Wed, 5 Apr 2023 23:21:09 +0000 (16:21 -0700)]
bufmgr: Introduce infrastructure for faster relation extension

The primary bottlenecks for relation extension are:

1) The extension lock is held while acquiring a victim buffer for the new
   page. Acquiring a victim buffer can require writing out the old page
   contents including possibly needing to flush WAL.

2) When extending via ReadBuffer() et al, we write a zero page during the
   extension, and then later write out the actual page contents. This can
   nearly double the write rate.

3) The existing bulk relation extension infrastructure in hio.c just amortized
   the cost of acquiring the relation extension lock, but none of the other
   costs.

Unfortunately 1) cannot currently be addressed in a central manner as the
callers to ReadBuffer() need to acquire the extension lock. To address that,
this this commit moves the responsibility for acquiring the extension lock
into bufmgr.c functions. That allows to acquire the relation extension lock
for just the required time. This will also allow us to improve relation
extension further, without changing callers.

The reason we write all-zeroes pages during relation extension is that we hope
to get ENOSPC errors earlier that way (largely works, except for CoW
filesystems). It is easier to handle out-of-space errors gracefully if the
page doesn't yet contain actual tuples. This commit addresses 2), by using the
recently introduced smgrzeroextend(), which extends the relation, without
dirtying the kernel page cache for all the extended pages.

To address 3), this commit introduces a function to extend a relation by
multiple blocks at a time.

There are three new exposed functions: ExtendBufferedRel() for extending the
relation by a single block, ExtendBufferedRelBy() to extend a relation by
multiple blocks at once, and ExtendBufferedRelTo() for extending a relation up
to a certain size.

To avoid duplicating code between ReadBuffer(P_NEW) and the new functions,
ReadBuffer(P_NEW) now implements relation extension with
ExtendBufferedRel(), using a flag to tell ExtendBufferedRel() that the
relation lock is already held.

Note that this commit does not yet lead to a meaningful performance or
scalability improvement - for that uses of ReadBuffer(P_NEW) will need to be
converted to ExtendBuffered*(), which will be done in subsequent commits.

Reviewed-by: Heikki Linnakangas <[email protected]>
Reviewed-by: Melanie Plageman <[email protected]>
Discussion: https://postgr.es/m/20221029025420[email protected]

2 years agoAllow to use system CA pool for certificate verification
Daniel Gustafsson [Wed, 5 Apr 2023 21:22:17 +0000 (23:22 +0200)]
Allow to use system CA pool for certificate verification

This adds a new option to libpq's sslrootcert, "system", which will load
the system trusted CA roots for certificate verification. This is a more
convenient way to achieve this than pointing to the system CA roots
manually since the location can differ by installation and be locally
adjusted by env vars in OpenSSL.

When sslrootcert is set to system, sslmode is forced to be verify-full
as weaker modes aren't providing much security for public CAs.

Changing the location of the system roots by setting environment vars is
not supported by LibreSSL so the tests will use a heuristic to determine
if the system being tested is LibreSSL or OpenSSL.

The workaround in .cirrus.yml is required to handle a strange interaction
between homebrew and the openssl@3 formula; hopefully this can be removed
in the near future.

The original patch was written by Thomas Habets, which was later revived
by Jacob Champion.

Author: Jacob Champion <[email protected]>
Author: Thomas Habets <[email protected]>
Reviewed-by: Jelte Fennema <[email protected]>
Reviewed-by: Andrew Dunstan <[email protected]>
Reviewed-by: Magnus Hagander <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/CA%2BkHd%2BcJwCUxVb-Gj_0ptr3_KZPwi3%2B67vK6HnLFBK9MzuYrLA%40mail.gmail.com

2 years agobufmgr: Support multiple in-progress IOs by using resowner
Andres Freund [Wed, 5 Apr 2023 20:55:15 +0000 (13:55 -0700)]
bufmgr: Support multiple in-progress IOs by using resowner

A future patch will add support for extending relations by multiple blocks at
once. To be concurrency safe, the buffers for those blocks need to be marked
as BM_IO_IN_PROGRESS. Until now we only had infrastructure for recovering from
an IO error for a single buffer. This commit extends that infrastructure to
multiple buffers by using the resource owner infrastructure.

This commit increases the size of the ResourceOwnerData struct, which appears
to have a just about measurable overhead in very extreme workloads. Medium
term we are planning to substantially shrink the size of
ResourceOwnerData. Short term the increase is small enough to not worry about
it for now.

Reviewed-by: Melanie Plageman <[email protected]>
Discussion: https://postgr.es/m/20221029025420[email protected]
Discussion: https://postgr.es/m/20221029200025[email protected]

2 years agoSupport "Right Anti Join" plan shapes.
Tom Lane [Wed, 5 Apr 2023 20:59:00 +0000 (16:59 -0400)]
Support "Right Anti Join" plan shapes.

Merge and hash joins can support antijoin with the non-nullable input
on the right, using very simple combinations of their existing logic
for right join and anti join.  This gives the planner more freedom
about how to order the join.  It's particularly useful for hash join,
since we may now have the option to hash the smaller table instead
of the larger.

Richard Guo, reviewed by Ronan Dunklau and myself

Discussion: https://postgr.es/m/CAMbWs48xh9hMzXzSy3VaPzGAz+fkxXXTUbCLohX1_L8THFRm2Q@mail.gmail.com

2 years agobufmgr: Acquire and clean victim buffer separately
Andres Freund [Wed, 5 Apr 2023 20:47:46 +0000 (13:47 -0700)]
bufmgr: Acquire and clean victim buffer separately

Previously we held buffer locks for two buffer mapping partitions at the same
time to change the identity of buffers.  Particularly for extending relations
needing to hold the extension lock while acquiring a victim buffer is
painful.But it also creates a bottleneck for workloads that just involve
reads.

Now we instead first acquire a victim buffer and write it out, if
necessary. Then we remove that buffer from the old partition with just the old
partition's partition lock held and insert it into the new partition with just
that partition's lock held.

By separating out the victim buffer acquisition, future commits will be able
to change relation extensions to scale better.

On my workstation, a micro-benchmark exercising buffered reads strenuously and
under a lot of concurrency, sees a >2x improvement.

Reviewed-by: Heikki Linnakangas <[email protected]>
Reviewed-by: Melanie Plageman <[email protected]>
Discussion: https://postgr.es/m/20221029025420[email protected]

2 years agoAcquire locks on views in AcquirePlannerLocks, too.
Tom Lane [Wed, 5 Apr 2023 19:56:07 +0000 (15:56 -0400)]
Acquire locks on views in AcquirePlannerLocks, too.

Commit 47bb9db75 taught AcquireExecutorLocks to re-acquire locks
on views using data from their RTE_SUBQUERY replacements, but
it now seems like we should make AcquirePlannerLocks do the same.
In this way, if a view has been redefined, we will notice that
a bit earlier while checking validity of a cached plan and thereby
avoid some wasted work.

Report and patch by Amit Langote.

Discussion: https://postgr.es/m/CA+HiwqH0xZOQ+GQAdKeckY1R4NOeHdzhtfxkAMJLSchpapNk5w@mail.gmail.com

2 years agopg_dump: Add support for zstd compression
Tomas Vondra [Wed, 5 Apr 2023 19:38:04 +0000 (21:38 +0200)]
pg_dump: Add support for zstd compression

Allow pg_dump to use the zstd compression, in addition to gzip/lz4. Bulk
of the new compression method is implemented in compress_zstd.{c,h},
covering the pg_dump compression APIs. The rest of the patch adds test
and makes various places aware of the new compression method.

The zstd library (which this patch relies on) supports multithreaded
compression since version 1.5. We however disallow that feature for now,
as it might interfere with parallel backups on platforms that rely on
threads (e.g. Windows). This can be improved / relaxed in the future.

This also fixes a minor issue in InitDiscoverCompressFileHandle(), which
was not updated to check if the file already has the .lz4 extension.

Adding zstd compression was originally proposed in 2020 (see the second
thread), but then was reworked to use the new compression API introduced
in e9960732a9. I've considered both threads when compiling the list of
reviewers.

Author: Justin Pryzby
Reviewed-by: Tomas Vondra, Jacob Champion, Andreas Karlsson
Discussion: https://postgr.es/m/20230224191840[email protected]
Discussion: https://postgr.es/m/20201221194924[email protected]

2 years agobufmgr: Add Pin/UnpinLocalBuffer()
Andres Freund [Wed, 5 Apr 2023 17:42:17 +0000 (10:42 -0700)]
bufmgr: Add Pin/UnpinLocalBuffer()

So far these were open-coded in quite a few places, without a good reason.

Reviewed-by: Melanie Plageman <[email protected]>
Reviewed-by: David Rowley <[email protected]>
Discussion: https://postgr.es/m/20221029025420[email protected]

2 years agobufmgr: Add some more error checking [infrastructure] around pinning
Andres Freund [Wed, 5 Apr 2023 17:42:17 +0000 (10:42 -0700)]
bufmgr: Add some more error checking [infrastructure] around pinning

This adds a few more assertions against a buffer being local in places we
don't expect, and extracts the check for a buffer being pinned exactly once
from LockBufferForCleanup() into its own function. Later commits will use this
function.

Reviewed-by: Heikki Linnakangas <[email protected]>
Reviewed-by: Melanie Plageman <[email protected]>
Discussion: http://postgr.es/m/419312fd-9255-078c-c3e3-f0525f911d7f@iki.fi

2 years agoAdd smgrzeroextend(), FileZero(), FileFallocate()
Andres Freund [Wed, 5 Apr 2023 17:06:39 +0000 (10:06 -0700)]
Add smgrzeroextend(), FileZero(), FileFallocate()

smgrzeroextend() uses FileFallocate() to efficiently extend files by multiple
blocks. When extending by a small number of blocks, use FileZero() instead, as
using posix_fallocate() for small numbers of blocks is inefficient for some
file systems / operating systems. FileZero() is also used as the fallback for
FileFallocate() on platforms / filesystems that don't support fallocate.

A big advantage of using posix_fallocate() is that it typically won't cause
dirty buffers in the kernel pagecache. So far the most common pattern in our
code is that we smgrextend() a page full of zeroes and put the corresponding
page into shared buffers, from where we later write out the actual contents of
the page. If the kernel, e.g. due to memory pressure or elapsed time, already
wrote back the all-zeroes page, this can lead to doubling the amount of writes
reaching storage.

There are no users of smgrzeroextend() as of this commit. That will follow in
future commits.

Reviewed-by: Melanie Plageman <[email protected]>
Reviewed-by: Heikki Linnakangas <[email protected]>
Reviewed-by: Kyotaro Horiguchi <[email protected]>
Reviewed-by: David Rowley <[email protected]>
Reviewed-by: John Naylor <[email protected]>
Discussion: https://postgr.es/m/20221029025420[email protected]

2 years agoFix another issue with ENABLE/DISABLE TRIGGER on partitioned tables.
Tom Lane [Wed, 5 Apr 2023 16:56:29 +0000 (12:56 -0400)]
Fix another issue with ENABLE/DISABLE TRIGGER on partitioned tables.

In v13 and v14, the ENABLE/DISABLE TRIGGER USER variant malfunctioned
on cloned triggers, failing to find the clones because it thought they
were system triggers.  Other variants of ENABLE/DISABLE TRIGGER would
improperly apply a superuserness check.  Fix by adjusting the is-it-
a-system-trigger check to match reality in those branches.  (As far
as I can find, this is the only place that got it wrong.)

There's no such bug in v15/HEAD, because we revised the catalog
representation of system triggers to be what this code was expecting.
However, add the test case to these branches anyway, because this area
is visibly pretty fragile.  Also remove an obsoleted comment.

The recent v15/HEAD commit 6949b921d fixed a nearby bug.  I now see
that my commit message for that was inaccurate: the behavior of
recursing to clone triggers is older than v15, but it didn't apply
to the case in v13/v14 because in those branches parent partitioned
tables have no pg_trigger entries for foreign-key triggers.  But add
the test case from that commit to v13/v14, just to show what is
happening there.

Per bug #17886 from DzmitryH.

Discussion: https://postgr.es/m/17886-5406d5d828aa4aa3@postgresql.org

2 years agoDon't initialize page in {vm,fsm}_extend(), not needed
Andres Freund [Wed, 5 Apr 2023 15:19:39 +0000 (08:19 -0700)]
Don't initialize page in {vm,fsm}_extend(), not needed

The read path needs to be able to initialize pages anyway, as relation
extensions are not durable. By avoiding initializing pages, we can, in a
future patch, extend the relation by multiple blocks at once.

Using smgrextend() for {vm,fsm}_extend() is not a good idea in general, as at
least one page of the VM/FSM will be read immediately after, always causing a
cache miss, requiring us to read content we just wrote.

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

2 years agoFix wrong word in comment.
Robert Haas [Wed, 5 Apr 2023 13:50:08 +0000 (09:50 -0400)]
Fix wrong word in comment.

Reported by Peter Smith.

Discussion: http://postgr.es/m/CAHut+Pt52ueOEAO-G5qeZiiPv1p9pBT_W5Vj3BTYfC8sD8LFxw@mail.gmail.com

2 years agoUpdate information_schema for SQL:2023
Peter Eisentraut [Wed, 5 Apr 2023 07:47:07 +0000 (09:47 +0200)]
Update information_schema for SQL:2023

This is mainly a light renumbering to match the sections in the
standard.

The comments for SQL_IMPLEMENTATION_INFO and SQL_SIZING are no longer
applicable because the required information has been moved into part
9.

2 years agodoc: Update error messages in RLS examples
John Naylor [Wed, 5 Apr 2023 07:16:19 +0000 (14:16 +0700)]
doc: Update error messages in RLS examples

Since 8b9e9644d, the messages for failed permissions checks report
"table" where appropriate, rather than "relation".

Backpatch to all supported branches

2 years agodoc: Update SQL features/conformance information to SQL:2023
Peter Eisentraut [Wed, 5 Apr 2023 06:55:44 +0000 (08:55 +0200)]
doc: Update SQL features/conformance information to SQL:2023

Optional subfeatures have been changed to top-level features, so there
is a bit of a churn in the list for that.

Some existing functions have been added to the standard, so they are
moved from the "other" to the "standard" lists in their sections.

Discussion: https://www.postgresql.org/message-id/flat/63f285d9-4ec8-0c9e-4bf5-e76334ddc0af@enterprisedb.com

2 years agoFix function reference in comment
Daniel Gustafsson [Wed, 5 Apr 2023 07:06:32 +0000 (09:06 +0200)]
Fix function reference in comment

Commit a61b1f748 renamed ExecCheckRTEPerms to ExecCheckPermissions
as part of a larger body of work, but missed this comment.  Fix by
updating the referenced function name to make the comment the same
as other occurrences.

Author: Koshi Shibagaki <[email protected]>
Discussion: https://postgr.es/m/OS3PR01MB653359ACBE8DBBE29EE2BC71FA909@OS3PR01MB6533.jpnprd01.prod.outlook.com

2 years agodoc: Update SQL keywords list to SQL:2023
Peter Eisentraut [Wed, 5 Apr 2023 05:55:28 +0000 (07:55 +0200)]
doc: Update SQL keywords list to SQL:2023

Per previous convention (see
ace397e9d24eddc56e7dffa921f506117b602d78), drop SQL:2011 and only keep
the latest two standards and SQL-92.

Discussion: https://www.postgresql.org/message-id/flat/63f285d9-4ec8-0c9e-4bf5-e76334ddc0af@enterprisedb.com

2 years agoFix minor signed/unsigned mixup
Peter Eisentraut [Wed, 5 Apr 2023 05:34:52 +0000 (07:34 +0200)]
Fix minor signed/unsigned mixup

The chunk header is unsigned, and the output format takes unsigned, so
casting it to signed in between is incorrect.

2 years agomeson: docs: Allow configuring simple/website style
Andres Freund [Wed, 5 Apr 2023 04:31:27 +0000 (21:31 -0700)]
meson: docs: Allow configuring simple/website style

The meson docs generation hardcoded using the website style so far. Make it
configurable via a meson option.

Reviewed-by: Justin Pryzby <[email protected]>
Reported-by: Peter Eisentraut <[email protected]>
Discussion: https://postgr.es/m/3fc3bb9b-f7f8-d442-35c1-ec82280c564a@enterprisedb.com

2 years agodocs: html: load stylesheet via custom.css.source
Andres Freund [Wed, 5 Apr 2023 04:10:46 +0000 (21:10 -0700)]
docs: html: load stylesheet via custom.css.source

Until now the meson built docs did not have a working reference to the css
stylesheet, it was copied in the make target. Instead of duplicating that for
meson, use the docbook-xsl parameter custom.css.source to reference it. An
additional benefit of that approach is that the stylesheet is now included in
the single-file HTML documentation.

Reported-by: Peter Eisentraut <[email protected]>
Discussion: https://postgr.es/m/3fc3bb9b-f7f8-d442-35c1-ec82280c564a@enterprisedb.com

2 years agodocs: html: copy images to output as part of xslt build
Andres Freund [Wed, 5 Apr 2023 04:05:52 +0000 (21:05 -0700)]
docs: html: copy images to output as part of xslt build

Until now the meson built HTML docs had non-working references to images. They
were copied in the make target. Instead of duplicating that for meson, copy
them as part of the xslt stylesheet.

Reported-by: Peter Eisentraut <[email protected]>
Discussion: https://postgr.es/m/3fc3bb9b-f7f8-d442-35c1-ec82280c564a@enterprisedb.com

2 years agomeson: add docs, docs_pdf options
Andres Freund [Wed, 5 Apr 2023 04:29:39 +0000 (21:29 -0700)]
meson: add docs, docs_pdf options

Detect and report if the tools necessary to build documentation are available
during configure. This is represented as two new options 'docs' and
'docs_pdf', both defaulting to 'auto'.

This should also fix a meson error about the installdocs target, when none of
the doc tools are found.

Reviewed-by: Justin Pryzby <[email protected]>
Discussion: https://postgr.es/m/20230325201414[email protected]
Discussion: https://postgr.es/m/ZB8331v5IhUA/[email protected]

2 years agomeson: docs: Preparatory cleanups
Andres Freund [Wed, 5 Apr 2023 03:50:43 +0000 (20:50 -0700)]
meson: docs: Preparatory cleanups

These are just minor prerequisite changes for later commits. Kept separate for
easier review.

Discussion: https://postgr.es/m/3fc3bb9b-f7f8-d442-35c1-ec82280c564a@enterprisedb.com
Discussion: https://postgr.es/m/20230329224132[email protected]

2 years agoAdd Copyright notice in 001_basic.pl and 002_pg_upgrade.pl.
Amit Kapila [Wed, 5 Apr 2023 03:50:14 +0000 (09:20 +0530)]
Add Copyright notice in 001_basic.pl and 002_pg_upgrade.pl.

Author: Kuroda Hayato
Discussion: https://postgr.es/m/TYCPR01MB587073D91E372B8EF719931EF5929@TYCPR01MB5870.jpnprd01.prod.outlook.com

2 years agodocs: Remove support for 'htmlhelp' format
Andres Freund [Wed, 5 Apr 2023 02:25:14 +0000 (19:25 -0700)]
docs: Remove support for 'htmlhelp' format

We had partial support for generating documentation suitable for .chm
files. However, we only had wired up generating the input files using
docbook-xsl, not generating an actual .chm file. Nor did we document how to do
so.  Additionally, it was very slow to generate htmlhelp, as we never applied
the docbook-xsl stylesheet performance improvements to htmlhelp.

It doesn't look like there's any interest in the htmlhelp output, so remove
it, instead of spending cycles to finish the support.

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

2 years agosequences: Lock buffer before initializing page
Andres Freund [Tue, 4 Apr 2023 23:38:06 +0000 (16:38 -0700)]
sequences: Lock buffer before initializing page

fill_seq_fork_with_data(), used to initialize a new sequence relation, only
locked the buffer after calling PageInit(), even though PageInit() modifies
page contents.

This is unlikely to cause real-world issues, as the relation is exclusively
locked at that point, and the buffer not yet marked dirty, so other processes
should not access the buffer.

This issue looks to have been present since the introduction of sequences in
e8647c45d66a.

Given the low risk, it does not seem worth backpatching the fix.

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

2 years agodoc: Add more details about pg_stat_get_xact_blocks_{fetched,hit}
Michael Paquier [Tue, 4 Apr 2023 22:59:43 +0000 (07:59 +0900)]
doc: Add more details about pg_stat_get_xact_blocks_{fetched,hit}

The explanation describing the dependency to system read() calls for
these two functions has been removed in ddfc2d9.  And after more
discussion about d69c404, we have concluded that adding more details
makes them easier to understand.

While on it, use the term "block read requests" (maybe found in cache)
rather than "buffers fetched" and "buffer hits".

Per discussion with Melanie Plageman, Kyotaro Horiguchi, Bertrand
Drouvot and myself.

Discussion: https://postgr.es/m/CAAKRu_ZmdiScT4q83OAbfmR5AH-L5zWya3SXjaxiJvhCob-e2A@mail.gmail.com
Backpatch-through: 11

2 years agoFix MSVC warning introduced in ea1db8ae70.
Jeff Davis [Tue, 4 Apr 2023 22:14:21 +0000 (15:14 -0700)]
Fix MSVC warning introduced in ea1db8ae70.

Discussion: https://postgr.es/m/CA+hUKGJR1BhCORa5WdvwxztD3arhENcwaN1zEQ1Upg20BwjKWA@mail.gmail.com
Reported-by: Thomas Munro
2 years agoRemove comment obsoleted by 11c2d6fd.
Thomas Munro [Tue, 4 Apr 2023 21:40:34 +0000 (09:40 +1200)]
Remove comment obsoleted by 11c2d6fd.

Reported-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/1604497.1680637072%40sss.pgh.pa.us

2 years agoCanonicalize ICU locale names to language tags.
Jeff Davis [Tue, 4 Apr 2023 17:28:08 +0000 (10:28 -0700)]
Canonicalize ICU locale names to language tags.

Convert to BCP47 language tags before storing in the catalog, except
during binary upgrade or when the locale comes from an existing
collation or template database.

The resulting language tags can vary slightly between ICU
versions. For instance, "@colBackwards=yes" is converted to
"und-u-kb-true" in older versions of ICU, and to the simpler (but
equivalent) "und-u-kb" in newer versions.

The process of canonicalizing to a language tag also understands more
input locale string formats than ucol_open(). For instance,
"fr_CA.UTF-8" is misinterpreted by ucol_open() and the region is
ignored; effectively treating it the same as the locale "fr" and
opening the wrong collator. Canonicalization properly interprets the
language and region, resulting in the language tag "fr-CA", which can
then be understood by ucol_open().

This commit fixes a problem in prior versions due to ucol_open()
misinterpreting locale strings as described above. For instance,
creating an ICU collation with locale "fr_CA.UTF-8" would store that
string directly in the catalog, which would later be passed to (and
misinterpreted by) ucol_open(). After this commit, the locale string
will be canonicalized to language tag "fr-CA" in the catalog, which
will be properly understood by ucol_open(). Because this fix affects
the resulting collator, we cannot change the locale string stored in
the catalog for existing databases or collations; otherwise we'd risk
corrupting indexes. Therefore, only canonicalize locales for
newly-created (not upgraded) collations/databases. For similar
reasons, do not backport.

Discussion: https://postgr.es/m/8c7af6820aed94dc7bc259d2aa7f9663518e6137[email protected]
Reviewed-by: Peter Eisentraut
2 years agoAdd a way to get the current function's OID in pl/pgsql.
Tom Lane [Tue, 4 Apr 2023 17:33:18 +0000 (13:33 -0400)]
Add a way to get the current function's OID in pl/pgsql.

Invent "GET DIAGNOSTICS oid_variable = PG_ROUTINE_OID".
This is useful for avoiding the maintenance nuisances that come
with embedding a function's name in its body, as one might do
for logging purposes for example.  Typically users would cast the
result to regproc or regprocedure to get something human-readable,
but we won't pre-judge whether that's appropriate.

Pavel Stehule, reviewed by Kirk Wolak and myself

Discussion: https://postgr.es/m/CAFj8pRA4zMd5pY-B89Gm64bDLRt-L+akOd34aD1j4PEstHHSVQ@mail.gmail.com

2 years agoAdd a run_as_owner option to subscriptions.
Robert Haas [Tue, 4 Apr 2023 16:03:03 +0000 (12:03 -0400)]
Add a run_as_owner option to subscriptions.

This option is normally false, but can be set to true to obtain
the legacy behavior where the subscription runs with the permissions
of the subscription owner rather than the permissions of the
table owner. The advantages of this mode are (1) it doesn't require
that the subscription owner have permission to SET ROLE to each
table owner and (2) since no role switching occurs, the
SECURITY_RESTRICTED_OPERATION restrictions do not apply.

On the downside, it allows any table owner to easily usurp
the privileges of the subscription owner - basically, to take
over their account. Because that's generally quite undesirable,
we don't make this mode the default, but we do make it available,
just in case the new behavior causes too many problems for someone.

Discussion: http://postgr.es/m/CA+TgmoZ-WEeG6Z14AfH7KhmpX2eFh+tZ0z+vf0=eMDdbda269g@mail.gmail.com

2 years agoPerform logical replication actions as the table owner.
Robert Haas [Tue, 4 Apr 2023 15:25:23 +0000 (11:25 -0400)]
Perform logical replication actions as the table owner.

Up until now, logical replication actions have been performed as the
subscription owner, who will generally be a superuser.  Commit
cec57b1a0fbcd3833086ba686897c5883e0a2afc documented hazards
associated with that situation, namely, that any user who owns a
table on the subscriber side could assume the privileges of the
subscription owner by attaching a trigger, expression index, or
some other kind of executable code to it. As a remedy, it suggested
not creating configurations where users who are not fully trusted
own tables on the subscriber.

Although that will work, it basically precludes using logical
replication in the way that people typically want to use it,
namely, to replicate a database from one node to another
without necessarily having any restrictions on which database
users can own tables. So, instead, change logical replication to
execute INSERT, UPDATE, DELETE, and TRUNCATE operations as the
table owner when they are replicated.

Since this involves switching the active user frequently within
a session that is authenticated as the subscription user, also
impose SECURITY_RESTRICTED_OPERATION restrictions on logical
replication code. As an exception, if the table owner can SET
ROLE to the subscription owner, these restrictions have no
security value, so don't impose them in that case.

Subscription owners are now required to have the ability to
SET ROLE to every role that owns a table that the subscription
is replicating. If they don't, replication will fail. Superusers,
who normally own subscriptions, satisfy this property by default.
Non-superusers users who own subscriptions will need to be
granted the roles that own relevant tables.

Patch by me, reviewed (but not necessarily in its entirety) by
Jelte Fennema, Jeff Davis, and Noah Misch.

Discussion: http://postgr.es/m/CA+TgmoaSCkg9ww9oppPqqs+9RVqCexYCE6Aq=UsYPfnOoDeFkw@mail.gmail.com

2 years agoAdd missing XML ID attributes
Peter Eisentraut [Tue, 4 Apr 2023 14:20:34 +0000 (16:20 +0200)]
Add missing XML ID attributes

Author: Brar Piening <[email protected]>
Discussion: https://www.postgresql.org/message-id/dc813a6f-60d9-991f-eecd-675a0921de11@gmx.de

2 years agoCode review for recent SQL/JSON commits
Alvaro Herrera [Tue, 4 Apr 2023 12:04:30 +0000 (14:04 +0200)]
Code review for recent SQL/JSON commits

- At the last minute and for no particularly good reason, I changed the
  WITHOUT token to be marked especially for lookahead, from the one in
  WITHOUT TIME to the one in WITHOUT UNIQUE.  Study of upcoming patches
  (where a new WITHOUT ARRAY WRAPPER clause is added) showed me that the
  former was better, so put it back the way the original patch had it.

- update exprTypmod() for JsonConstructorExpr to return the typmod of
  the RETURNING clause, as a comment there suggested.  Perhaps it's
  possible for this to make a difference with datetime types, but I
  didn't try to build a test case.

- The nodeFuncs.c support code for new nodes was calling walker()
  directly instead of the WALK() macro as introduced by commit 1c27d16e6e5c.
  Modernize that.  Also add exprLocation() support for a couple of nodes
  that missed it.  Lastly, reorder the code more sensibly.

The WITHOUT_LA -> WITHOUT change means that stored rules containing
either WITHOUT TIME ZONE or WITHOUT UNIQUE KEYS would change
representation.  Therefore, bump catversion.

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

2 years agobufmgr: Remove buffer-write-dirty tracepoints
Andres Freund [Tue, 4 Apr 2023 01:02:41 +0000 (18:02 -0700)]
bufmgr: Remove buffer-write-dirty tracepoints

The trace point was using the relfileno / fork / block for the to-be-read-in
buffer. Some upcoming work would make that more expensive to provide. We still
have buffer-flush-start/done, which can serve most tracing needs that
buffer-write-dirty could serve.

Discussion: https://postgr.es/m/f5164e7a-eef6-8972-75a3-8ac622ed0c6e@iki.fi

2 years agoMake SP-GiST redirect cleanup more aggressive.
Peter Geoghegan [Mon, 3 Apr 2023 18:47:48 +0000 (11:47 -0700)]
Make SP-GiST redirect cleanup more aggressive.

Commit 61b313e4 made VACUUM pass down a heaprel to index AM bulkdelete
and vacuumcleanup routines.  Although this was primarily intended as
preparation for logical decoding on standbys, it also made it easy to
correct an old deficiency in how we determine how to cleanup SP-GiST
redirect and placeholder tuples.

Pass the heaprel to GlobalVisTestFor() during cleanup of redirect and
placeholder tuples, rather than pessimistically passing NULL.

Author: Bertrand Drouvot <[email protected]>
Discussion: https://postgr.es/m/02392033-f030-a3c8-c7d0-5c27eb529fec@gmail.com

2 years agoRecycle deleted nbtree pages more aggressively.
Peter Geoghegan [Mon, 3 Apr 2023 18:31:43 +0000 (11:31 -0700)]
Recycle deleted nbtree pages more aggressively.

Commit 61b313e4 made nbtree consistently pass down a heaprel to low
level routines like _bt_getbuf().  Although this was primarily intended
as preparation for logical decoding on standbys, it also made it easy to
correct an old deficiency in how nbtree VACUUM determines whether or not
it's now safe to recycle deleted pages.

Pass the heaprel to GlobalVisTestFor() in nbtree routines that deal with
recycle safety.  nbtree now makes less pessimistic assumptions about
recycle safety within non-catalog relations.  This enhancement
complements the recycling enhancement added by commit 9dd963ae25.

nbtree remains just as pessimistic as ever when it comes to recycle
safety within indexes on catalog relations.  There is no fundamental
reason why we need to treat catalog relations differently, though.  The
behavioral inconsistency is a consequence of the way that nbtree uses
nextXID values to implement what Lanin and Shasha call "the drain
technique".  Note in particular that it has nothing to do with whether
or not index tuples might still be required for an older MVCC snapshot.

Author: Bertrand Drouvot <[email protected]>
Discussion: https://postgr.es/m/CAH2-WzkaiDxCje0yPuH=3Uh2p1V_2pFGY==xfbZoZu7Ax_NB8g@mail.gmail.com

2 years agoMove heaprel struct field next to index rel field.
Peter Geoghegan [Mon, 3 Apr 2023 18:01:11 +0000 (11:01 -0700)]
Move heaprel struct field next to index rel field.

Commit 61b313e4 added a heaprel struct member to IndexVacuumInfo, but
placed it last.  Move the heaprel struct member next to the index struct
member to improve the code's readability.

Author: Peter Geoghegan <[email protected]>
Discussion: https://postgr.es/m/CAH2-WznG=TV6S9d3VA=y0vBHbXwnLs9_LLdiML=aNJuHeriwxg@mail.gmail.com

2 years agoFix possible logical replication crash.
Robert Haas [Mon, 3 Apr 2023 17:11:00 +0000 (13:11 -0400)]
Fix possible logical replication crash.

Commit c3afe8cf5a1e465bd71e48e4bc717f5bfdc7a7d6 added a new
password_required option but forgot that you need database access
to check whether an arbitrary role ID is a superuser.

Report and patch by Hou Zhijie. I added a comment. Thanks to
Alexander Lakhin for devising a way to reproduce the crash.

Discussion: http://postgr.es/m/OS0PR01MB5716BFD7EC44284C89F40808948F9@OS0PR01MB5716.jpnprd01.prod.outlook.com

2 years agoWhen using valgrind, log the current query after an error is detected.
Tom Lane [Mon, 3 Apr 2023 14:18:38 +0000 (10:18 -0400)]
When using valgrind, log the current query after an error is detected.

In USE_VALGRIND builds, add code to print the text of the current query
to the valgrind log whenever the valgrind error count has increased
during the query.  Valgrind will already have printed its report,
if the error is distinct from ones already seen, so that this works
out fairly well as a way of annotating the log.

Onur Tirtir and Tom Lane

Discussion: https://postgr.es/m/AM9PR83MB0498531E804DC8DF8CFF0E8FE9D09@AM9PR83MB0498.EURPRD83.prod.outlook.com

2 years agoRevert 764da7710b
Alexander Korotkov [Mon, 3 Apr 2023 13:55:09 +0000 (16:55 +0300)]
Revert 764da7710b

Discussion: https://postgr.es/m/20230323003003.plgaxjqahjgkuxrk%40awork3.anarazel.de

2 years agoRevert 11470f544e
Alexander Korotkov [Mon, 3 Apr 2023 13:54:31 +0000 (16:54 +0300)]
Revert 11470f544e

Discussion: https://postgr.es/m/20230323003003.plgaxjqahjgkuxrk%40awork3.anarazel.de