libpqwalreceiver: Convert to libpq-be-fe-helpers.h

In contrast to the changes to dblink and postgres_fdw, this does not fix a
bug, as libpqwalreceiver did already process interrupts.

Besides reducing code duplication, the conversion leads to libpqwalreceiver
now using reserving file descriptors for libpq connections. While not strictly
required for the use in walreceiver, we are also using libpqwalreceiver for
logical replication, where it does seem more important.

Even if we eventually decide to backpatch the prior commits, there'd be no
need to backpatch this commit, due to not fixing an active bug.

Reviewed-by: Thomas Munro <[email protected]>
Discussion: https://postgr.es/m/20220925232237[email protected]

dblink, postgres_fdw: Handle interrupts during connection establishment

Until now dblink and postgres_fdw did not process interrupts during connection
establishment. Besides preventing query cancellations etc, this can lead to
undetected deadlocks, as global barriers are not processed.

These aforementioned undetected deadlocks are the reason for the spate of CI
test failures in the FreeBSD 'test_running' step.

Fix the bug by using the helper from libpq-be-fe-helpers.h, introduced in a
prior commit. Besides fixing the bug, this also removes duplicated code
around reserving file descriptors.

As the change is relatively large and there are no field reports of the
problem, don't backpatch for now.

Reviewed-by: Thomas Munro <[email protected]>
Discussion: https://postgr.es/m/20220925232237[email protected]
Backpatch:

Add helper library for use of libpq inside the server environment

Currently dblink and postgres_fdw don't process interrupts during connection
establishment. Besides preventing query cancellations etc, this can lead to
undetected deadlocks, as global barriers are not processed.

Libpqwalreceiver in contrast, processes interrupts during connection
establishment. The required code is not trivial, so duplicating it into
additional places does not seem like a good option.

These aforementioned undetected deadlocks are the reason for the spate of CI
test failures in the FreeBSD 'test_running' step.

For now the helper library is just a header, as it needs to be linked into
each extension using libpq, and it seems too small to be worth adding a
dedicated static library for.

The conversion to the helper are done in subsequent commits.

Reviewed-by: Thomas Munro <[email protected]>
Discussion: https://postgr.es/m/20220925232237[email protected]

Fix error handling in libpqrcv_connect()

When libpqrcv_connect (also known as walrcv_connect()) failed, it leaked the
libpq connection. In most paths that's fairly harmless, as the calling process
will exit soon after. But e.g. CREATE SUBSCRIPTION could lead to a somewhat
longer lived leak.

Fix by releasing resources, including the libpq connection, on error.

Add a test exercising the error code path. To make it reliable and safe, the
test tries to connect to port=-1, which happens to fail during connection
establishment, rather than during connection string parsing.

Reviewed-by: Noah Misch <[email protected]>
Discussion: https://postgr.es/m/20230121011237[email protected]
Backpatch: 11-

Use OFFSET 0 instead of ORDER BY to stop subquery pullup

b762fed64 recently changed this test to prevent subquery pullup to allow
us to test Memoize with lateral_vars.  As pointed out by Tom Lane, OFFSET
0 is our standard way of preventing subquery pullups, so do it that way
instead.

Discussion: https://postgr.es/m/2144818.1674517061@sss.pgh.pa.us
Backpatch-through: 14, same as b762fed64

Fix LATERAL join test in test memoize.sql

The test in question was meant to be testing Memoize to ensure it worked
correctly when the inner side of the join contained lateral vars, however,
nothing in the lateral subquery stopped it from being pulled up into the
main query, so the planner did that, and that meant no more lateral vars.

Here we add a simple ORDER BY to stop the planner from being able to
pullup the lateral subquery.

Author: Richard Guo
Discussion: https://postgr.es/m/CAMbWs4_LHJaN4L-tXpKMiPFnsCJWU1P8Xh59o0W7AA6UN99=cQ@mail.gmail.com
Backpatch-through: 14, where Memoize was added.

Fix XLogPageRead() comment

7fcbf6a and 2ff6555 changed the function signature of XLogPageRead()
but did not update the comment.

XLogReaderRoutine contains up to date information about the API, so no
need to repeat all that at XLogPageRead(), but fix the mentions of the
no longer existing function arguments.

Add non-decimal integer support to type numeric.

This enhances the numeric type input function, adding support for
hexadecimal, octal, and binary integers of any size, up to the limits
of the numeric type.

Since 6fcda9aba8, such non-decimal integers have been accepted by the
parser as integer literals and passed through to numeric_in(). This
commit gives numeric_in() the ability to handle them.

While at it, simplify the handling of NaN and infinities, reducing the
number of calls to pg_strncasecmp(), and arrange for pg_strncasecmp()
to not be called at all for regular numbers. This gives a significant
performance improvement for decimal inputs, more than offsetting the
small performance hit of checking for non-decimal input.

Discussion: https://postgr.es/m/CAEZATCV8XShnmT9HZy25C%2Bo78CVOFmUN5EM9FRAZ5xvYTggPMg%40mail.gmail.com

Fix pgindent --show-diff option.

At least on my machine, the initial coding of this didn't actually
work, because interpolation of "$post_fh->filename" doesn't act
as intended.

I threw in some double quotes too, just in case anybody tries
to run this in a path containing spaces.

Remove special outfuncs/readfuncs handling of RangeVar.catalogname.

Historically we skipped writing/reading this field, but that no
longer works under WRITE_READ_PARSE_PLAN_TREES since we expanded
the coverage of that option to include utility commands (787102b56).
Remove the special case and just treat this field normally.

Bump catversion out of an abundance of caution --- I do not think
we currently ever store RangeVar nodes in the catalogs, but
perhaps I'm wrong.

Per report from Pavel Stehule.

Discussion: https://postgr.es/m/CAFj8pRAYvYu-qU7-NieqRRyaQZk-yr3UjtHQ2LR62PS9M1dZMA@mail.gmail.com

Add a test using ldapbindpasswd in pg_hba.conf

This feature has not been covered in tests up to now.

John Naylor and Andrew Dunstan

Discussion: https://postgr.es/m/06005bfb-0fd7-9d08-e0e5-440f277b73b4@dunslane.net

Restructure Ldap TAP test

The code for detecting the Ldap installation and setting up a test
server is broken out into a reusable module that can be used  for
additional tests to be added in later patches.

Discussion: https://postgr.es/m/06005bfb-0fd7-9d08-e0e5-440f277b73b4@dunslane.net

Add non-destructive modes to pgindent

This adds two modes of running pgindent, neither of which results in
any changes being made to the source code. The --show-diff option shows
what changes would have been made, and the --silent-diff option just
exits with a status of 2 if any changes would be made. The second of
these is intended for scripting use in places such as git hooks.

Along the way some code cleanup is done, and a --help option is also
added.

Reviewed by Tom Lane

Discussion: https://postgr.es/m/c9c9fa6d-6de6-48c2-4f8b-0fbeef026439@dunslane.net

Optimise numeric division for 3 and 4 base-NBASE digit divisors.

On platforms with 128-bit integer support, introduce a new function
div_var_int64(), along the same lines as div_var_int() added in
d1b307eef2 for divisors with 1 or 2 base-NBASE digits, and use it to
speed up div_var() and div_var_fast() in a similar way when the
divisor has 3 or 4 base-NBASE digits.

This gives significant performance gains for divisors with 9-16
decimal digits.

Joel Jacobson.

Discussion:
  https://postgr.es/m/b7a5893d-af18-4c0b-8918-96de5f1bbf39%40app.fastmail.com
  https://postgr.es/m/CAEZATCXGm%3DDyTq%3DFrcOqC0gPMVveKUYTaD5KRRoajrUTiWxVMw%40mail.gmail.com

Run pgindent on heapam.c

An upcoming patch by Melanie Plageman does some refactoring work in this
area.  Run pgindent on that file now before making any changes so that
it's easier to maintain/evolve each of the individual patches doing the
refactor work.  Additionally, add a few new required typedefs to the list
to make it easier to do future pgindent runs on this file during the
refactor work.

Discussion: https://postgr.es/m/CAAKRu_YSOnhKsDyFcqJsKtBSrd32DP-jjXmv7hL0BPD-z0TGXQ@mail.gmail.com

Fix and clarify function comment on LogicalTapeSetCreate.

Commit c4649cce39 removed the "shared" and "ntapes" arguments, but the
comment still talked about "shared". It also talked about "a shared
file handle", which was technically correct because even before commit
c4649cce39, the "shared file handle" referred to the "fileset"
argument, not "shared". But it was very confusing. Improve the
comment.

Also add a comment on what the "preallocate" argument does.

Backpatch to v15, just to make backpatching other patches easier in
the future.

Discussion: https://www.postgresql.org/message-id/af989685-91d5-aad4-8f60-1d066b5ec309@enterprisedb.com
Reviewed-by: Peter Eisentraut

Harden new parallel string_agg/array_agg regression test

Per buildfarm member mandrill, it seems that
max_parallel_workers_per_gather may not always be set to the default value
of 2 when the new test added in 16fd03e95 is executed.  Here let's just
explicitly set that to 2 so that the planner never opts to use more than
that many parallel workers.

pg_walinspect: Add pg_get_wal_fpi_info()

This function is able to extract the full page images from a range of
records, specified as of input arguments start_lsn and end_lsn.  Like
the other functions of this module, an error is returned if using LSNs
that do not reflect real system values.  All the FPIs stored in a single
record are extracted.

The module's version is bumped to 1.1.

Author: Bharath Rupireddy
Reviewed-by: Bertrand Drouvot
Discussion: https://postgr.es/m/CALj2ACVCcvzd7WiWvD=6_7NBvVB_r6G0EGSxL4F8vosAi6Se4g@mail.gmail.com

Allow parallel aggregate on string_agg and array_agg

This adds combine, serial and deserial functions for the array_agg() and
string_agg() aggregate functions, thus allowing these aggregates to
partake in partial aggregations.  This allows both parallel aggregation to
take place when these aggregates are present and also allows additional
partition-wise aggregation plan shapes to include plans that require
additional aggregation once the partially aggregated results from the
partitions have been combined.

Author: David Rowley
Reviewed-by: Andres Freund, Tomas Vondra, Stephen Frost, Tom Lane
Discussion: https://postgr.es/m/CAKJS1f9sx_6GTcvd6TMuZnNtCh0VhBzhX6FZqw17TgVFH-ga_A@mail.gmail.com

Track logrep apply workers' last start times to avoid useless waits.

Enforce wal_retrieve_retry_interval on a per-subscription basis,
rather than globally, and arrange to skip that delay in case of
an intentional worker exit.  This probably makes little difference
in the field, where apply workers wouldn't be restarted often;
but it has a significant impact on the runtime of our logical
replication regression tests (even though those tests use
artificially-small wal_retrieve_retry_interval settings already).

Nathan Bossart, with mostly-cosmetic editorialization by me

Discussion: https://postgr.es/m/20221122004119.GA132961@nathanxps13

Allow REPLICA IDENTITY to be set on an index that's not (yet) valid.

The motivation for this change is that when pg_dump dumps a
partitioned index that's marked REPLICA IDENTITY, it generates a
command sequence that applies REPLICA IDENTITY before the partitioned
index has been marked valid, causing restore to fail.  We could
perhaps change pg_dump to not do it like that, but that would be
difficult and would not fix existing dump files with the problem.
There seems to be very little reason for the backend to disallow
this anyway --- the code ignores indisreplident when the index
isn't valid --- so instead let's fix it by allowing the case.

Commit 9511fb37a previously expressed a concern that allowing
indisreplident to be set on invalid indexes might allow us to
wind up in a situation where a table could have indisreplident
set on multiple indexes.  I'm not sure I follow that concern
exactly, but in any case the only way that could happen is because
relation_mark_replica_identity is too trusting about the existing set
of markings being valid.  Let's just rip out its early-exit code path
(which sure looks like premature optimization anyway; what are we
doing expending code to make redundant ALTER TABLE ... REPLICA
IDENTITY commands marginally faster and not-redundant ones marginally
slower?) and fix it to positively guarantee that no more than one
index is marked indisreplident.

The pg_dump failure can be demonstrated in all supported branches,
so back-patch all the way.  I chose to back-patch 9511fb37a as well,
just to keep indisreplident handling the same in all branches.

Per bug #17756 from Sergey Belyashov.

Discussion: https://postgr.es/m/17756-dd50e8e0c8dd4a40@postgresql.org

Reject CancelRequestPacket having unexpected length.

When the length was too short, the server read outside the allocation.
That yielded the same log noise as sending the correct length with
(backendPID,cancelAuthCode) matching nothing.  Change to a message about
the unexpected length.  Given the attacker's lack of control over the
memory layout and the general lack of diversity in memory layouts at the
code in question, we doubt a would-be attacker could cause a segfault.
Hence, while the report arrived via [email protected], this is not
a vulnerability.  Back-patch to v11 (all supported versions).

Andrey Borodin, reviewed by Tom Lane.  Reported by Andrey Borodin.

instr_time: Represent time as an int64 on all platforms

Until now we used struct timespec for instr_time on all platforms but
windows. Using struct timespec causes a fair bit of memory (struct timeval is
16 bytes) and runtime overhead (much more complicated additions). Instead we
can convert the time to nanoseconds in INSTR_TIME_SET_CURRENT(), making the
remaining operations cheaper.

Representing time as int64 nanoseconds provides sufficient range, ~292 years
relative to a starting point (depending on clock source, relative to the unix
epoch or the system's boot time). That'd not be sufficient for calendar time
stored on disk, but is plenty for runtime interval time measurement.

On windows instr_time already is represented as cycles. It might make sense to
represent time as cycles on other platforms as well, as using cycle
acquisition instructions like rdtsc directly can reduce the overhead of time
acquisition substantially. This could be done in a fairly localized manner as
the code stands after this commit.

Because the windows and non-windows paths are now more similar, use a common
set of macros. To make that possible, most of the use of LARGE_INTEGER had to
be removed, which looks nicer anyway.

To avoid users of the API relying on the integer representation, we wrap the
64bit integer inside struct struct instr_time.

Author: Andres Freund <[email protected]>
Author: Lukas Fittl <[email protected]>
Author: David Geier <[email protected]>
Reviewed-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/20230113195547[email protected]

Zero initialize uses of instr_time about to trigger compiler warnings

These are all not necessary from a correctness POV. However, in the near
future instr_time will be simplified to an int64, at which point gcc would
otherwise start to warn about the changed places.

Reviewed-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/20230116023639[email protected]

Rework format of comments in headers for nodes

This is similar to 835d476, except that this one is to add node
attributes related to query jumbling and avoid long lines in the headers
and in the node structures changed by this commit.

Reviewed-by: Peter Eisentraut
Discussion: https://postgr.es/m/Y5BHOUhX3zTH/[email protected]

Move queryjumble.c code to src/backend/nodes/

This will ease a follow-up move that will generate automatically this
code.  The C file is renamed, for consistency with the node-related
files whose code are generated by gen_node_support.pl:
- queryjumble.c -> queryjumblefuncs.c
- utils/queryjumble.h -> nodes/queryjumble.h

Per a suggestion from Peter Eisentraut.

Reviewed-by: Peter Eisentraut
Discussion: https://postgr.es/m/Y5BHOUhX3zTH/[email protected]

Bump catversion for 6e2775e4d4e47775f0d933e4a93c148024a3bc63.

It creates a new predefined role.

Add new GUC reserved_connections.

This provides a way to reserve connection slots for non-superusers.
The slots reserved via the new GUC are available only to users who
have the new predefined role pg_use_reserved_connections.
superuser_reserved_connections remains as a final reserve in case
reserved_connections has been exhausted.

Patch by Nathan Bossart. Reviewed by Tushar Ahuja and by me.

Discussion: http://postgr.es/m/20230119194601.GA4105788@nathanxps13

Rename ReservedBackends variable to SuperuserReservedConnections.

This is in preparation for adding a new reserved_connections GUC,
but aligning the GUC name with the variable name is also a good
idea on general principle.

Patch by Nathan Bossart. Reviewed by Tushar Ahuja and by me.

Discussion: http://postgr.es/m/20230119194601.GA4105788@nathanxps13

Update docs and error message for superuser_reserved_connections.

Commit ea92368cd1da1e290f9ab8efb7f60cb7598fc310 made max_wal_senders
a separate pool of backends from max_connections, but the documentation
and error message for superuser_reserved_connections weren't updated
at the time, and as a result are somewhat misleading. Update.

This is arguably a back-patchable bug fix, but because it seems quite
minor, no back-patch.

Patch by Nathan Bossart. Reviewed by Tushar Ahuja and by me.

Discussion: http://postgr.es/m/20230119194601.GA4105788@nathanxps13

Describe each contrib module in its SGML section title

The original titles only had the module name, which is not very useful
when scanning the list.  By adding a very brief description to each
title, the table of contents becomes friendlier.

Also amend the introduction in the "additional modules" appendix, using
the word "Extension" more extensively.  Nowadays, almost all contrib
modules are extensions, so this is also helpful.

Author: Karl O. Pinc <[email protected]>
Reviewed-by: Brar Piening <[email protected]>
Discussion: https://postgr.es/m/20230102180015.372995a9@slate.karlpinc.com

Remove SHM_QUEUE

Prior patches got rid of all the uses of SHM_QUEUE. ilist.h style lists are
more widely used and have an easier to use interface. As there are no users
left, remove SHM_QUEUE.

Reviewed-by: Thomas Munro <[email protected]> (in an older version)
Discussion: https://postgr.es/m/20221120055930[email protected]
Discussion: https://postgr.es/m/20200211042229[email protected]

Use dlists instead of SHM_QUEUE for predicate locking

Part of a series to remove SHM_QUEUE. ilist.h style lists are more widely used
and have an easier to use interface.

Reviewed-by: Thomas Munro <[email protected]> (in an older version)
Discussion: https://postgr.es/m/20221120055930[email protected]
Discussion: https://postgr.es/m/20200211042229[email protected]

Improve the description of Output Plugin Callbacks.

We were inconsistently specifying the required and optional marking for
plugin callbacks. Additionally, this patch improves the description for
stream_prepare callback.

Author: Wang wei
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/OS3PR01MB627553DAFD39ECDADD08DC909EFC9@OS3PR01MB6275.jpnprd01.prod.outlook.com

Support the same patterns for pg-user in pg_ident.conf as in pg_hba.conf

While pg_hba.conf has support for non-literal username matches, and
this commit extends the capabilities that are supported for the
PostgreSQL user listed in an ident entry part of pg_ident.conf, with
support for:
1. The "all" keyword, where all the requested users are allowed.
2. Membership checks using the + prefix.
3. Using a regex to match against multiple roles.

1. is a feature that has been requested by Jelte Fennema, 2. something
that has been mentioned independently by Andrew Dunstan, and 3. is
something I came up with while discussing how to extend the first one,
whose implementation is facilitated by 8fea868.

This allows matching certain system users against many different
postgres users with a single line in pg_ident.conf.  Without this, one
would need one line for each of the postgres users that a system user
can log in as, which can be cumbersome to maintain.

Tests are added to the TAP test of peer authentication to provide
coverage for all that.

Note that this introduces a set of backward-incompatible changes to be
able to detect the new patterns, for the following cases:
- A role named "all".
- A role prefixed with '+' characters, which is something that would not
have worked in HBA entries anyway.
- A role prefixed by a slash character, similarly to 8fea868.
Any of these can be still be handled by using quotes in the Postgres
role defined in an ident entry.

A huge advantage of this change is that the code applies the same checks
for the Postgres roles in HBA and ident entries, via the common routine
check_role().

**This compatibility change should be mentioned in the release notes.**

Author: Jelte Fennema
Discussion: https://postgr.es/m/DBBPR83MB0507FEC2E8965012990A80D0F7FC9@DBBPR83MB0507.EURPRD83.prod.outlook.com

Avoid harmless warning from pg_dump --if-exists mode.

If the public schema has a non-default owner (perhaps due to
dropping and recreating it) then use of pg_dump's "--if-exists"
option results in a warning message:

warning: could not find where to insert IF EXISTS in statement "-- *not* dropping schema, since initdb creates it"

This is harmless since the dump output is the same either way,
but nonetheless it's undesirable.  It's the fault of commit
a7a7be1f2, which created situations where a TOC entry's "defn"
or "dropStmt" fields could be just comments.  Although that
commit fixed up the kluges in pg_backup_archiver.c that munge defn
strings, it missed doing so for the one that munges dropStmts.

Per bug# 17753 from Justin Zhang.

Discussion: https://postgr.es/m/17753-9c8773631747ee1c@postgresql.org

Use appendStringInfoSpaces in more places

This adjusts a few places which were appending a string constant
containing spaces onto a StringInfo.  We have appendStringInfoSpaces for
that job, so let's use that instead.

For the change to jsonb.c's add_indent() function, appendStringInfoString
was being called inside a loop to append 4 spaces on each loop.  This
meant that enlargeStringInfo would get called once per loop.  Here it
should be much more efficient to get rid of the loop and just calculate
the number of spaces with "level * 4" and just append all the spaces in
one go.

Here we additionally adjust the appendStringInfoSpaces function so it
makes use of memset rather than a while loop to apply the required spaces
to the StringInfo.  One of the problems with the while loop was that it
was incrementing one variable and decrementing another variable once per
loop.  That's more work than what's required to get the job done.  We may
as well use memset for this rather than trying to optimize the existing
loop.  Some testing has shown memset is faster even for very small sizes.

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

Improve comment about GetWALAvailability's WALAVAIL_REMOVED code.

Sirisha Chamarthi and Kyotaro Horiguchi

Discussion: https://postgr.es/m/CAKrAKeXt-=bgm=d+EDmcC9kWoikp8kbVb3LH0K3K+AGGsykpHQ@mail.gmail.com

Fix ts_headline() to handle ORs and phrase queries more honestly.

This patch largely reverts what I did in commits c9b0c678d and
78e73e875.  The maximum cover length limit that I added in 78e73e875
(to band-aid over c9b0c678d's performance issues) creates too many
user-visible behavior discrepancies, as complained of for example in
bug #17691.  The real problem with hlCover() is not what I thought
at the time, but more that it seems to have been designed with only
AND tsquery semantics in mind.  It doesn't work quite right for OR,
and even less so for NOT or phrase queries.  However, we can improve
that situation by building a variant of TS_execute() that returns a
list of match locations.  We already get an ExecPhraseData struct
representing match locations for the primitive case of a simple match,
as well as one for a phrase match; we just need to add some logic to
combine these for AND and OR operators.  The result is a list of
ExecPhraseDatas, which hlCover can regard as having simple AND
semantics, so that its old algorithm works correctly.

There's still a lot not to like about ts_headline's behavior, but
I think the remaining issues have to do with the heuristics used
in mark_hl_words and mark_hl_fragments (which, likewise, were not
revisited when phrase search was added).  Improving those is a task
for another day.

Patch by me; thanks to Alvaro Herrera for review.

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

Log the correct ending timestamp in recovery_target_xid mode.

When ending recovery based on recovery_target_xid matching with
recovery_target_inclusive = off, we printed an incorrect timestamp
(always 2000-01-01) in the "recovery stopping before ... transaction"
log message.  This is a consequence of sloppy refactoring in
c945af80c: the code to fetch recordXtime out of the commit/abort
record used to be executed unconditionally, but it was changed
to get called only in the RECOVERY_TARGET_TIME case.  We need only
flip the order of operations to restore the intended behavior.

Per report from Torsten Förtsch.  Back-patch to all supported
branches.

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

Remove some dead code in selfuncs.c

RelOptInfo.userid is the same for all relations in a given inheritance
tree, so the code in examine_variable() and example_simple_variable()
that repeats the ACL checks on the root parent rel instead of a given
leaf child relations need not recompute userid too.

Author: Amit Langote <[email protected]>
Reported-by: Justin Pryzby <[email protected]>
Discussion: https://postgr.es/m/20221210201753[email protected]

Constify proclist.h

This is a follow-up to c8ad4d81.

Author: Aleksander Alekseev
Discussion: https://www.postgresql.org/message-id/flat/CAJ7c6TM084Ai_8%3DfZaWtULJBLtT1bgzL%3Dk9vHMYom3eyZsekAA%40mail.gmail.com

doc: Fix some issues in logical replication section

wal_retrieve_retry_interval was mentioned under an incorrect name, and
wal_sender_timeout was not listed as affecting WAL senders in logical
replication mode.

Author: Takamichi Osumi
Discussion: https://postgr.es/m/TYCPR01MB8373D65E6B0A769ED12EADCBEDC79@TYCPR01MB8373.jpnprd01.prod.outlook.com

Add missing assign hook for GUC checkpoint_completion_target

This is wrong since 88e9823, that has switched the WAL sizing
configuration from checkpoint_segments to min_wal_size and
max_wal_size.  This missed the recalculation of the internal value of
the internal "CheckPointSegments", that works as a mapping of the old
GUC checkpoint_segments, on reload, for example, and it controls the
timing of checkpoints depending on the volume of WAL generated.

Most users tend to leave checkpoint_completion_target at 0.9 to smooth
the I/O workload, which is why I guess this has gone unnoticed for so
long, still it can be useful to tweak and reload the value dynamically
in some cases to control the timing of checkpoints.

Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACXgPPAm28mruojSBno+F_=9cTOOxHAywu_dfZPeBdybQw@mail.gmail.com
Backpatch-through: 11

Fix failure with perlcritic in psql's create_help.pl

No buildfarm members have reported that yet, but a recently-refreshed
Debian host did.

Reviewed-by: Andrew Dunstan
Discussion: https://postgr.es/m/Y8ey5z4Nav62g4/[email protected]
Backpatch-through: 11

Fix AdjustUpgrade.pm's view conversion list for --with-lz4.

Turns out the compression.sql test creates a view that needs
to be adjusted in the wake of 47bb9db75 --- except that without
--with-lz4, it fails to create the view at all, so I'd not
noticed this in testing.

Per buildfarm member crake.

Update expected/collate.windows.win1252.out for 47bb9db75.

This delta in expected output wasn't spotted in any previous
testing of the patch.  Reported by Andres Freund.

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

Use dlists instead of SHM_QUEUE for syncrep queue

Part of a series to remove SHM_QUEUE. ilist.h style lists are more widely used
and have an easier to use interface.

Reviewed-by: Thomas Munro <[email protected]> (in an older version)
Discussion: https://postgr.es/m/20221120055930[email protected]
Discussion: https://postgr.es/m/20200211042229[email protected]

Use dlist/dclist instead of PROC_QUEUE / SHM_QUEUE for heavyweight locks

Part of a series to remove SHM_QUEUE. ilist.h style lists are more widely used
and have an easier to use interface.

As PROC_QUEUE is now unused, remove it.

Reviewed-by: Thomas Munro <[email protected]> (in an older version)
Discussion: https://postgr.es/m/20221120055930[email protected]
Discussion: https://postgr.es/m/20200211042229[email protected]

Add detached node functions to ilist

These allow to test whether an element is in a list by checking whether
prev/next are NULL. Needed to replace SHMQueueIsDetached() when converting
from SHM_QUEUE to ilist.h style lists.

Reviewed-by: Thomas Munro <[email protected]>
Discussion: https://postgr.es/m/20221120055930[email protected]
Discussion: https://postgr.es/m/20200211042229[email protected]

Fix ILIST_DEBUG build

In c8ad4d8166a dlist_member_check()'s arguments were made const. Unfortunately
the implementation of dlist_member_check() used dlist_foreach(), which
currently doesn't work for const lists.

As a workaround, open-code the list iteration. The other check functions
already do so.

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

Get rid of the "new" and "old" entries in a view's rangetable.

The rule system needs "old" and/or "new" pseudo-RTEs in rule actions
that are ON INSERT/UPDATE/DELETE.  Historically it's put such entries
into the ON SELECT rules of views as well, but those are really quite
vestigial.  The only thing we've used them for is to carry the
view's relid forward to AcquireExecutorLocks (so that we can
re-lock the view to verify it hasn't changed before re-using a plan)
and to carry its relid and permissions data forward to execution-time
permissions checks.  What we can do instead of that is to retain
these fields of the RTE_RELATION RTE for the view even after we
convert it to an RTE_SUBQUERY RTE.  This requires a tiny amount of
extra complication in the planner and AcquireExecutorLocks, but on
the other hand we can get rid of the logic that moves that data from
one place to another.

The principal immediate benefit of doing this, aside from a small
saving in the pg_rewrite data for views, is that these pseudo-RTEs
no longer trigger ruleutils.c's heuristic about qualifying variable
names when the rangetable's length is more than 1.  That results
in quite a number of small simplifications in regression test outputs,
which are all to the good IMO.

Bump catversion because we need to dump a few more fields of
RTE_SUBQUERY RTEs.  While those will always be zeroes anyway in
stored rules (because we'd never populate them until query rewrite)
they are useful for debugging, and it seems like we'd better make
sure to transmit such RTEs accurately in plans sent to parallel
workers.  I don't think the executor actually examines these fields
after startup, but someday it might.

This is a second attempt at committing 1b4d280ea.  The difference
from the first time is that now we can add some filtering rules to
AdjustUpgrade.pm to allow cross-version upgrade testing to pass
despite all the cosmetic changes in CREATE VIEW outputs.

Amit Langote (filtering rules by me)

Discussion: https://postgr.es/m/CA+HiwqEf7gPN4Hn+LoZ4tP2q_Qt7n3vw7-6fJKOf92tSEnX6Gg@mail.gmail.com
Discussion: https://postgr.es/m/891521.1673657296@sss.pgh.pa.us

Remove redundant grouping and DISTINCT columns.

Avoid explicitly grouping by columns that we know are redundant
for sorting, for example we need group by only one of x and y in
SELECT ... WHERE x = y GROUP BY x, y
This comes up more often than you might think, as shown by the
changes in the regression tests.  It's nearly free to detect too,
since we are just piggybacking on the existing logic that detects
redundant pathkeys.  (In some of the existing plans that change,
it's visible that a sort step preceding the grouping step already
didn't bother to sort by the redundant column, making the old plan
a bit silly-looking.)

To do this, build processed_groupClause and processed_distinctClause
lists that omit any provably-redundant sort items, and consult those
not the originals where relevant.  This means that within the
planner, one should usually consult root->processed_groupClause or
root->processed_distinctClause if one wants to know which columns
are to be grouped on; but to check whether grouping or distinct-ing
is happening at all, check non-NIL-ness of parse->groupClause or
parse->distinctClause.  This is comparable to longstanding rules
about handling the HAVING clause, so I don't think it'll be a huge
maintenance problem.

nodeAgg.c also needs minor mods, because it's now possible to generate
AGG_PLAIN and AGG_SORTED Agg nodes with zero grouping columns.

Patch by me; thanks to Richard Guo and David Rowley for review.

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

Display the leader apply worker's PID for parallel apply workers.

Add leader_pid to pg_stat_subscription. leader_pid is the process ID of
the leader apply worker if this process is a parallel apply worker. If
this field is NULL, it indicates that the process is a leader apply
worker or a synchronization worker. The new column makes it easier to
distinguish parallel apply workers from other kinds of workers and helps
to identify the leader for the parallel workers corresponding to a
particular subscription.

Additionally, update the leader_pid column in pg_stat_activity as well to
display the PID of the leader apply worker for parallel apply workers.

Author: Hou Zhijie
Reviewed-by: Peter Smith, Sawada Masahiko, Amit Kapila, Shveta Mallik
Discussion: https://postgr.es/m/CAA4eK1+wyN6zpaHUkCLorEWNx75MG0xhMwcFhvjqm2KURZEAGw@mail.gmail.com

Refactor code for restoring files via shell commands

Presently, restore_command uses a different code path than
archive_cleanup_command and recovery_end_command.  These code paths
are similar and can be easily combined, as long as it is possible to
identify if a command should:
- Issue a FATAL on signal.
- Exit immediately on SIGTERM.

While on it, this removes src/common/archive.c and its associated
header.  Since the introduction of c96de2c, BuildRestoreCommand() has
become a simple wrapper of replace_percent_placeholders() able to call
make_native_path().  This simplifies shell_restore.c as long as
RestoreArchivedFile() includes a call to make_native_path().

Author: Nathan Bossart
Reviewed-by: Andres Freund, Michael Paquier
Discussion: https://postgr.es/m/20221227192449.GA3672473@nathanxps13

Constify the arguments of copydir.h functions

This makes sure that the internal logic of these functions does not
attempt to change the value of the arguments constified, and it removes
one unconstify() in basic_archive.c.

Author: Nathan Bossart
Reviewed-by: Andrew Dunstan, Peter Eisentraut
Discussion: https://postgr.es/m/20230114231126.GA2580330@nathanxps13

Doc: fix a few oddly-spelled SGML ID attributes.

Avoid use of "_" in SGML IDs.  Awhile back that was actually
disallowed by the toolchain, as a consequence of which our convention
has been to use "-" instead.  Fix a couple of stragglers that are
particularly inconsistent with that convention and with related IDs.

This is just neatnik-ism, so no need for back-patch.

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

meson: Add two missing regress tests

It's likely worth adding some automated way of preventing further
omissions. We're discussing how to best do that.

Reported-by: Justin Pryzby <[email protected]>
Discussion: https://postgr.es/m/20230117173509[email protected]

AdjustUpgrade.pm should zap test_ext_cine, too.

test_extensions' test_ext_cine extension has the same upgrade hazard
as test_ext7: the regression test leaves it in an updated state
from which no downgrade path to default is provided.  This causes
the update_extensions.sql script helpfully provided by pg_upgrade
to fail.  So drop it in cross-version-upgrade testing.

Not entirely sure how come I didn't hit this in testing yesterday;
possibly I'd built the upgrade reference databases with
testmodules-install-check disabled.

Backpatch to v10 where this module was introduced.

Refactor recordExtObjInitPriv()

Instead of half a dozen of mostly-duplicate conditional branches,
write one common one that can handle most catalogs.  We already have
all the information we need, such as which system catalog corresponds
to which catalog table and which column is the ACL column.

Reviewed-by: Nathan Bossart <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/504bc485-6bd6-dd1b-fe10-e7351aeb310d@enterprisedb.com

Remove AggregateRelationId from recordExtObjInitPriv()

This was erroneous, because AggregateRelationId has no OID, so it
cannot be part of an extension directly.  (Aggregates are registered
via pg_proc.)  No harm in practice, but better to make it correct.

Reviewed-by: Nathan Bossart <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/504bc485-6bd6-dd1b-fe10-e7351aeb310d@enterprisedb.com

Remove redundant relkind check

Ranier Vilela

Reviewed by Justin Pryzby
Discussion: https://www.postgresql.org/message-id/CAEudQAp2R2fbbi0OHHhv_n4%3DCh0t1VtjObR9YMqtGKHJ%2BfaUFQ%40mail.gmail.com

Remove dead code in formatting.c

Remove some code guarded by IS_MINUS() or IS_PLUS(), where the entire
stanza is inside an else-block where both of these are false. This
should slightly improve test coverage.

While at it, remove coding that apparently assumes that unsetting a
bit is so expensive that we have to first check if it's already set
in the first place.

Per Coverity report from Ranier Vilela
Analysis and review by Justin Pryzby

Discussion: https://www.postgresql.org/message-id/20221223010818.GP1153%40telsasoft.com

Improve the code to decide and process the apply action.

The code that decides the apply action missed to handle non-transactional
messages and we didn't catch it in our testing as currently such messages
are simply ignored by the apply worker. This was introduced by changes in
commit 216a784829.

While testing this, I noticed that we forgot to reset stream_xid after
processing the stream stop message which could also result in the wrong
apply action after the fix for non-transactional messages.

In passing, change assert to elog for unexpected apply action in some of
the routines so as to catch the problems in the production environment, if
any.

Reported-by: Tomas Vondra
Author: Amit Kapila
Reviewed-by: Tomas Vondra, Sawada Masahiko, Hou Zhijie
Discussion: https://postgr.es/m/984ff689-adde-9977-affe-cd6029e850be@enterprisedb.com
Discussion: https://postgr.es/m/CAA4eK1+wyN6zpaHUkCLorEWNx75MG0xhMwcFhvjqm2KURZEAGw@mail.gmail.com

Fix typo in comment.

Author: Osumi Takamichi
Discussion: https://postgr.es/m/TYCPR01MB83737EA140C79B7D099F65E8EDC69@TYCPR01MB8373.jpnprd01.prod.outlook.com

Track behavior of \1 in pg_ident.conf when quoted

Entries of pg-user in pg_ident.conf that are quoted and include '\1'
allow a replacement from a subexpression in a system user regexp.  This
commit adds a test to track this behavior and a note in the
documentation, as it could be affected by the use of an AuthToken for
the pg-user in the IdentLines parsed.

This subject has come up in the discussion aimed at extending the
support of pg-user in ident entries for more patterns.

Author: Jelte Fennema
Discussion: https://postgr.es/m/CAGECzQRNow4MwkBjgPxywXdJU_K3a9+Pm78JB7De3yQwwkTDew@mail.gmail.com

Don't presort ORDER BY/DISTINCT Aggrefs with volatile functions

In 1349d2790, we gave the planner the ability to provide ORDER BY/DISTINCT
Aggrefs with presorted input so that nodeAgg would not have to perform
sorts during execution.  That commit failed to properly consider the
implications of if the Aggref had a volatile function in its ORDER
BY/DISTINCT clause.  As it happened, this resulted in an ERROR about the
volatile function being missing from the targetlist.

Here, instead of adding the volatile function to the targetlist, we just
never consider an Aggref with a volatile function in its ORDER BY/DISTINCT
clause when choosing which Aggrefs we should sort by.  We do this as if we
were to choose a plan which provided these aggregates with presorted
input, then if there were many such aggregates which could all share the
same sort order, then it may be surprising if they all shared the same
sort sometimes and didn't at other times when some other set of aggregates
were given presorted results.  We can avoid this inconsistency by just
never providing these volatile function aggregates with presorted input.

Reported-by: Dean Rasheed
Discussion: https://postgr.es/m/CAEZATCWETioXs5kY8vT6BVguY41_wD962VDk=u_Nvd7S1UXzuQ@mail.gmail.com

Create common infrastructure for cross-version upgrade testing.

To test pg_upgrade across major PG versions, we have to be able to
modify or drop any old objects with no-longer-supported properties,
and we have to be able to deal with cosmetic changes in pg_dump output.
Up to now, the buildfarm and pg_upgrade's own test infrastructure had
separate implementations of the former, and we had nothing but very
ad-hoc rules for the latter (including an arbitrary threshold on how
many lines of unchecked diff were okay!).  This patch creates a Perl
module that can be shared by both those use-cases, and adds logic
that deals with pg_dump output diffs in a much more tightly defined
fashion.

This largely supersedes previous efforts in commits 0df9641d3,
9814ff550, and 62be9e4cd, which developed a SQL-script-based solution
for the task of dropping old objects.  There was nothing fundamentally
wrong with that work in itself, but it had no basis for solving the
output-formatting problem.  The most plausible way to deal with
formatting is to build a Perl module that can perform editing on the
dump files; and once we commit to that, it makes more sense for the
same module to also embed the knowledge of what has to be done for
dropping old objects.

Back-patch versions of the helper module as far as 9.2, to
support buildfarm animals that still test that far back.
It's also necessary to back-patch PostgreSQL/Version.pm,
because the new code depends on that.  I fixed up pg_upgrade's
002_pg_upgrade.pl in v15, but did not look into back-patching
it further than that.

Tom Lane and Andrew Dunstan

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

Tighten up VACUUM's approach to setting VM bits.

Tighten up the way that visibilitymap_set() is called: request that both
the all-visible and all-frozen bits get set whenever the all-frozen bit
is set, regardless of what we think we know about the present state of
the all-visible bit.  Also make sure that the page level PD_ALL_VISIBLE
flag is set in the same code path.

In practice there doesn't seem to be a concrete scenario in which the
previous approach could lead to inconsistencies.  It was almost possible
in scenarios involving concurrent HOT updates from transactions that
abort, but (unlike pruning) freezing can never remove XIDs > VACUUM's
OldestXmin, even those from transactions that are known to have aborted.
That was protective here.

These issues have been around since commit a892234f83, which added the
all-frozen bit to the VM fork.  There is no known live bug here, so no
backpatch.

In passing, add some defensive assertions to catch the issue, and stop
reading the existing state of the VM when setting the VM in VACUUM's
final heap pass.  We already know that affected pages must have had at
least one LP_DEAD item before we set it LP_UNUSED, so there is no point
in reading the VM when it is set like this.

Author: Peter Geoghegan <[email protected]>
Reviewed-By: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/CAH2-WznuNGSzF8v6OsgjaC5aYsb3cZ6HW6MLm30X0d65cmSH6A@mail.gmail.com

Assorted improvements to SECURITY DEFINER functions documentation.

Add a cross-reference from the part of the page that introdues SECURITY
INVOKER and SECURITY DEFINER to the part of the page that talks about
writing SECURITY DEFINER functions safely, so that users are less likely
to miss it.

Remove discussion of the pre-8.3 behavior on the theory that it's
probably not very relevant any more, that release having gone out of
support nearly a decade ago.

Add a mention of the new createrole_self_grant GUC, which in
certain cases might need to be set to a safe value to avoid
unexpected consequences.

Possibly this section needs major surgery rather than just these
small tweaks, but hopefully this is at least a small step
forward.

Discussion: http://postgr.es/m/CA+Tgmoauqd1cHQjsNEoxL5O-kEO4iC9dAPyCudSvmNqPJGmy9g@mail.gmail.com

More documentation update for GRANT ... WITH SET OPTION.

Update the reference pages for various ALTER commands that
mentioned that you must be a member of role that will be the
new owner to instead say that you must be able to SET ROLE
to the new owner. Update ddl.sgml's generate statement on this
topic along similar lines.

Likewise, update CREATE SCHEMA and CREATE DATABASE, which
have options to specify who will own the new objects, to say
that you must be able to SET ROLE to the role that will own
them.

Finally, update the documentation for the GRANT statement
itself with some general principles about how the SET option
works and how it can be used.

Patch by me, reviewed (but not fully endorsed) by Noah Misch.

Discussion: http://postgr.es/m/CA+TgmoZk6VB3DQ83+DO5P_HP=M9PQAh1yj-KgeV30uKefVaWDg@mail.gmail.com

Add BufFileRead variants with short read and EOF detection

Most callers of BufFileRead() want to check whether they read the full
specified length.  Checking this at every call site is very tedious.
This patch provides additional variants BufFileReadExact() and
BufFileReadMaybeEOF() that include the length checks.

I considered changing BufFileRead() itself, but this function is also
used in extensions, and so changing the behavior like this would
create a lot of problems there.  The new names are analogous to the
existing LogicalTapeReadExact().

Reviewed-by: Amit Kapila <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/f3501945-c591-8cc3-5ef0-b72a2e0eaa9c@enterprisedb.com

Fix some BufFileRead() error reporting

Remove "%m" from error messages where errno would be bogus.  Add short
read byte counts where appropriate.

This is equivalent to what was done in
7897e3bb902c557412645b82120f4d95f7474906, but some code was apparently
developed concurrently to that and not updated accordingly.

Reviewed-by: Amit Kapila <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/f3501945-c591-8cc3-5ef0-b72a2e0eaa9c@enterprisedb.com

Refactor code in charge of running shell-based recovery commands

The code specific to the execution of archive_cleanup_command,
recovery_end_command and restore_command is moved to a new file named
shell_restore.c.  The code is split into three functions:
- shell_restore(), that attempts the execution of a shell-based
restore_command.
- shell_archive_cleanup(), for archive_cleanup_command.
- shell_recovery_end(), for recovery_end_command.

This introduces no functional changes, with failure patterns and logs
generated in consequence being the same as before (one case actually
generates one less DEBUG2 message "could not restore" when a restore
command succeeds but the follow-up stat() to check the size fails, but
that only matters with a elevel high enough).

This is preparatory work for allowing recovery modules, a facility
similar to archive modules, with callbacks shaped similarly to the
functions introduced here.

Author: Nathan Bossart
Reviewed-by: Andres Freund, Michael Paquier
Discussion: https://postgr.es/m/20221227192449.GA3672473@nathanxps13

Store IdentLine->pg_user as an AuthToken

While system_user was stored as an AuthToken in IdentLine, pg_user was
stored as a plain string.  This commit changes the code as we start
storing pg_user as an AuthToken too.

This does not have any functional changes, as all the operations on
pg_user only use the string from the AuthToken.  There is no regexp
compiled and no check based on its quoting, yet.  This is in preparation
of more features that intend to extend its capabilities, like support
for regexps and group membership.

Author: Jelte Fennema
Discussion: https://postgr.es/m/CAGECzQRNow4MwkBjgPxywXdJU_K3a9+Pm78JB7De3yQwwkTDew@mail.gmail.com

Remove arbitrary FUNC_MAX_ARGS limit in int2vectorin and oidvectorin.

int2vectorin limited the number of array elements it'd take to
FUNC_MAX_ARGS, which is probably fine for the traditional use-cases.
But now that pg_publication_rel.prattrs is an int2vector, it's not
fine at all: it's easy to construct cases where that can have up to
about MaxTupleAttributeNumber entries.  Trying to replicate such
tables leads to logical-replication failures.

As long as we have to touch this code anyway, let's just remove
the a-priori limit altogether, and let it accept any size that'll
be allowed by repalloc.  (Note that since int2vector isn't toastable,
we cannot store arrays longer than about BLCKSZ/2; but there is no
good excuse for letting int2vectorin depend on that.  Perhaps we
will lift the no-toast restriction someday.)

While at it, also improve the equivalent logic in oidvectorin.
I don't know of any practical use-case for long oidvectors right
now, but doing it right actually makes the code shorter.

Per report from Erik Rijkers.  Back-patch to v15 where
pg_publication_rel.prattrs was added.

Discussion: https://postgr.es/m/668ba539-33c5-8190-ca11-def2913cb94b@xs4all.nl

Make new GENERATED-expressions code more bulletproof.

In commit 8bf6ec3ba I assumed that no code path could reach
ExecGetExtraUpdatedCols without having gone through
ExecInitStoredGenerated.  That turns out not to be the case in
logical replication: if there's an ON UPDATE trigger on the target
table, trigger.c will call this code before anybody has set up its
generated columns.  Having seen that, I don't have a lot of faith in
there not being other such paths.  ExecGetExtraUpdatedCols can call
ExecInitStoredGenerated for itself, as long as we are willing to
assume that it is only called in CMD_UPDATE operations, which on
the whole seems like a safer leap of faith.

Per report from Vitaly Davydov.

Discussion: https://postgr.es/m/d259d69652b8c2ff50e14cda3c236c7f@postgrespro.ru

Doc: fix typo in backup.sgml.

<varname>archive_command</varname> was unnecessarily repeated.

Author: Tatsuo Ishii
Reviewed-by: Amit Kapila
Backpatch-through: 15
Discussion: https://postgr.es/m/flat/20230114.110234.666053507266410467.t-ishii%40sranhm.sra.co.jp

Fix MAINTAIN privileges for toast tables and partitions.

Commit 60684dd8 left loose ends when it came to maintaining toast
tables or partitions.

For toast tables, simply skip the privilege check if the toast table
is an indirect target of the maintenance command, because the main
table privileges have already been checked.

For partitions, allow the maintenance command if the user has the
MAINTAIN privilege on the partition or any parent.

Also make CLUSTER emit "skipping" messages when the user doesn't have
privileges, similar to VACUUM.

Author: Nathan Bossart
Reported-by: Pavel Luzanov
Reviewed-by: Pavel Luzanov, Ted Yu
Discussion: https://postgr.es/m/20230113231339.GA2422750@nathanxps13

Add 250c8ee07ed to git-blame-ignore-revs

Manual cleanup and pgindent of pgstat and bufmgr related code

This is in preparation for commiting a larger patch series in the area.

Discussion: https://postgr.es/m/CAAKRu_bHwGEbzNxxy+MQDkrsgog6aO6iUvajJ4d6PD98gFU7+w@mail.gmail.com

Clean up useless "skipping" messages for VACUUM/ANALYZE.

When VACUUM/ANALYZE are run on an entire database, it warns of
skipping relations for which the user doesn't have sufficient
privileges. That only makes sense for tables, so skip such messages
for indexes, etc.

Reviewed-by: Nathan Bossart
Discussion: https://postgr.es/m/c0a85c2e83158560314b576b6241c8ed0aea1745.camel%40j-davis.com

Simplify permissions for LOCK TABLE.

The prior behavior was confusing and hard to document. For instance,
if you had UPDATE privileges, you could lock a table in any lock mode
except ACCESS SHARE mode.

Now, if granted a privilege to lock at a given mode, one also has
privileges to lock at a less-conflicting mode. MAINTAIN, UPDATE,
DELETE, and TRUNCATE privileges allow any lock mode. INSERT privileges
allow ROW EXCLUSIVE (or below). SELECT privileges allow ACCESS SHARE.

Reviewed-by: Nathan Bossart
Discussion: https://postgr.es/m/9550c76535404a83156252b25a11babb4792ea1e.camel%40j-davis.com

Ignore dropped and generated columns from the column list.

We don't allow different column lists for the same table in the different
publications of the single subscription. A publication with a column list
except for dropped and generated columns should be considered the same as
a publication with no column list (which implicitly includes all columns
as part of the columns list). However, as we were not excluding the
dropped and generated columns from the column list combining such
publications leads to an error "cannot use different column lists for
table ...".

We decided not to backpatch this fix as there is a risk of users seeing
this as a behavior change and also we didn't see any field report of this
case.

Author: Shi yu
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/OSZPR01MB631091CCBC56F195B1B9ACB0FDFE9@OSZPR01MB6310.jpnprd01.prod.outlook.com

Avoid creating parallel apply state hash table unless required.

This hash table is used to cache the state of streaming transactions being
applied by the parallel apply workers. So, this should be created only
when we are successful in launching at least one worker. This avoids rare
case memory leak when we are never able to launch any worker.

Author: Ted Yu
Discussion: https://postgr.es/m/CALte62wg0rBR3Vj2beV=HiWo2qG9L0hzKcX=yULNER0wmf4aEw@mail.gmail.com

Add tests for regex replacement with \1 in pg_ident.conf to 0003_peer.pl

Regexp replacement with \1 in pg_ident.conf is tested in one check of
the kerberos test suite, still it requires a dependency on
--with-gssapi to be triggered.  This commit adds to the test suite of
peer authentication two tests to check the replacement of \1 in a
pg-username, coupled with a system-username regexp:
- With a subexpression in system-username, similarly to the kerberos
test suite.
- Without a subexpression in system-username, checking for a failure.
This had no coverage until now, and the error pattern is checked in the
server logs.

Author: Jelte Fennema
Discussion: https://postgr.es/m/CAGECzQRNow4MwkBjgPxywXdJU_K3a9+Pm78JB7De3yQwwkTDew@mail.gmail.com

doc: Simplify description of functions for pg_walinspect

As introduced in 2258e76, the docs were hard to parse:
- The examples used listed a lot of long records, bloating the output.
These are switched to show less records with the expanded format,
similarly to pageinspect.
- The function descriptions listed all the OUT parameters, producing
long lines.  This is updated so as only the input parameters are
documented, clarifying the whole.
- Remove one example on pg_get_wal_stats() when per_record is set to
true, which is not really necessary once we know the output produced,
and the behavior of the parameter is documented.

While on it, fix a few grammar mistakes and simplify a couple of
sentences.

Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACVGcUpziGgQrcT-1G3dHWQQfWjYBu1YQ2ypv9y86dgogg@mail.gmail.com
Backpatch-through: 15

Fix WaitEventSetWait() buffer overrun.

The WAIT_USE_EPOLL and WAIT_USE_KQUEUE implementations of
WaitEventSetWaitBlock() confused the size of their internal buffer with
the size of the caller's output buffer, and could ask the kernel for too
many events.  In fact the set of events retrieved from the kernel needs
to be able to fit in both buffers, so take the smaller of the two.

The WAIT_USE_POLL and WAIT_USE WIN32 implementations didn't have this
confusion.

This probably didn't come up before because we always used the same
number in both places, but commit 7389aad6 calculates a dynamic size at
construction time, while using MAXLISTEN for its output event buffer on
the stack.  That seems like a reasonable thing to want to do, so
consider this to be a pre-existing bug worth fixing.

As discovered by valgrind on skink.

Back-patch to all supported releases for epoll, and to release 13 for
the kqueue part, which copied the incorrect epoll code.

Reviewed-by: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/901504.1673504836%40sss.pgh.pa.us

Fix jsonpath existense checking of missing variables

The current jsonpath code assumes that the referenced variable always exists.
It could only throw an error at the value valuation time.  At the same time
existence checking assumes variable is present without valuation, and error
suppression doesn't work for missing variables.

This commit makes existense checking trigger an error for missing variables.
This makes the overall behavior consistent.

Backpatch to 12 where jsonpath was introduced.

Reported-by: David G. Johnston
Discussion: https://postgr.es/m/CAKFQuwbeytffJkVnEqDyLZ%3DrQsznoTh1OgDoOF3VmOMkxcTMjA%40mail.gmail.com
Author: Alexander Korotkov, David G. Johnston
Backpatch-through: 12

Constify the arguments of ilist.c/h functions

Const qualifiers ensure that we don't do something stupid in the
function implementation.  Additionally they clarify the interface.  As
an example:

    void
    slist_delete(slist_head *head, const slist_node *node)

Here one can instantly tell that node->next is not going to be set to
NULL.  Finally, const qualifiers potentially allow the compiler to do
more optimizations.  This being said, no benchmarking was done for
this patch.

The functions that return non-const pointers like slist_next_node(),
dclist_next_node() etc. are not affected by the patch intentionally.

Author: Aleksander Alekseev
Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/CAJ7c6TM2%3D08mNKD9aJg8vEY9hd%2BG4L7%2BNvh30UiNT3kShgRgNg%40mail.gmail.com

Code cleanup

for commit c96de2ce1782116bd0489b1cd69ba88189a495e8

Author: Nathan Bossart <[email protected]>
Discussion: https://www.postgresql.org/message-id/20230111185434.GA1912982@nathanxps13

Rename some variables related to ident files in hba.{c,h}

The code that handles authentication for user maps was pretty confusing
with its choice of variable names.  It involves two types of users: a
system user and a Postgres user (well, role), and these were not named
consistently throughout the code that processes the user maps loaded
from pg_ident.conf at authentication.

This commit changes the following things to improve the situation:
- Rename "pg_role" to "pg_user" and "token" to "system_user" in
IndetLine.  These choices are more consistent with the pg_ident.conf
example in the docs, as well.  "token" has been introduced recently in
fc579e1, and it is way worse than the choice before that, "ident_user".
- Switch the order of the fields in IdentLine to map with the order of
the items in the ident files, as of map name, system user and PG user.
- In check_ident_usermap(), rename "regexp_pgrole" to "expanded_pg_user"
when processing a regexp for the system user entry in a user map.  This
variable does not store a regular expression at all: it would be either
a string or a substitution to \1 if the Postgres role is specified as
such.

Author: Jelte Fennema
Discussion: https://postgr.es/m/CAGECzQTkwELHUOAKhvdA+m3tWbUQySHHkExJV8GAZ1pwgbEgXg@mail.gmail.com

Fix incorrect comment in hba.h

A comment in hba.h mentioned that AuthTokens are used when building the
IdentLines from pg_ident.conf, but since 8fea868 that has added support
of regexps for databases and roles in pg_hba.conf, it is also the case
of HBA files.  This refreshes the comment to refer to both HBA and ident
files.

Issue spotted while going through a different patch.

Acquire spinlock when updating 2PC slot data during logical decoding creation

The creation of a logical decoding context in CreateDecodingContext()
updates some data of its slot for two-phase transactions if enabled by
the caller, but the code forgot to acquire a spinlock when updating
these fields like any other code paths.  This could lead to the read of
inconsistent data.

Oversight in a8fd13c.

Author: Sawada Masahiko
Discussion: https://postgr.es/m/CAD21AoAD8_fp47191LKuecjDd3DYhoQ4TaucFco1_TEr_jQ-Zw@mail.gmail.com
Backpatch-through: 15

Revert "Get rid of the "new" and "old" entries in a view's rangetable."

This reverts commit 1b4d280ea1eb7ddb2e16654d5fa16960bb959566.
It's broken the buildfarm members that run cross-version-upgrade tests,
because they're not prepared to deal with cosmetic differences between
CREATE VIEW commands emitted by older servers and HEAD.  Even if we had
a solution to that, which we don't, it'd take some time to roll it out
to the affected animals.  This improvement isn't valuable enough to
justify addressing that problem on an emergency basis, so revert it
for now.

Refactor DetermineSleepTime() to use milliseconds.

Since we're not using select() anymore, we don't need to bother with
struct timeval.  We can work directly in milliseconds, which the latch
API wants.

Discussion: https://postgr.es/m/CA%2BhUKG%2BZ-HpOj1JsO9eWUP%2Bar7npSVinsC_npxSy%2BjdOMsx%3DGg%40mail.gmail.com

Use WaitEventSet API for postmaster's event loop.

Switch to a design similar to regular backends, instead of the previous
arrangement where signal handlers did non-trivial state management and
called fork().  The main changes are:

* The postmaster now has its own local latch to wait on.  (For now, we
  don't want other backends setting its latch directly, but that could
  probably be made to work with more research on robustness.)

* The existing signal handlers are cut in two: a handle_pm_XXX() part
  that just sets pending_pm_XXX flags and the latch, and a
  process_pm_XXX() part that runs later when the latch is seen.

* Signal handlers are now installed with the regular pqsignal()
  function rather than the special pqsignal_pm() function; historical
  portability concerns about the effect of SA_RESTART on select() are no
  longer relevant, and we don't need to block signals anymore.

Reviewed-by: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/CA%2BhUKG%2BZ-HpOj1JsO9eWUP%2Bar7npSVinsC_npxSy%2BjdOMsx%3DGg%40mail.gmail.com

Doc: fix silly thinko in 8bf6ec3ba.

Amit Langote

Discussion: https://postgr.es/m/CA+HiwqG2v-SnWyJuyVM-Z8DEFukY8+qe3XLMwSG4Xp7Yf=RioA@mail.gmail.com

Make lazy_vacuum_heap_rel match lazy_scan_heap.

Make lazy_vacuum_heap_rel variable names match those from lazy_scan_heap
where that makes sense.

Extracted from a larger patch to deal with issues with how vacuumlazy.c
sets pages all-frozen.

Author: Peter Geoghegan <[email protected]>
Discussion: https://postgr.es/m/CAH2-WznuNGSzF8v6OsgjaC5aYsb3cZ6HW6MLm30X0d65cmSH6A@mail.gmail.com

vacuumlazy.c: Tweak local variable name.

Make a local variable name consistent with the name from its WAL record.

Extracted from a larger patch to deal with issues with how vacuumlazy.c
sets pages all-frozen.

Author: Peter Geoghegan <[email protected]>
Discussion: https://postgr.es/m/CAH2-WznuNGSzF8v6OsgjaC5aYsb3cZ6HW6MLm30X0d65cmSH6A@mail.gmail.com