Commit Graph

55456 Commits

Author SHA1 Message Date
Michael Paquier 6ba6306256 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/K@paquier.xyz
Backpatch-through: 11
2023-01-19 10:01:53 +09:00
Tom Lane a3b65fb282 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.
2023-01-18 16:51:40 -05:00
Tom Lane 5ea2234670 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.7yjc4mminmv7iyy5@awork3.anarazel.de
2023-01-18 15:27:41 -05:00
Andres Freund 12605414a7 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 <thomas.munro@gmail.com> (in an older version)
Discussion: https://postgr.es/m/20221120055930.t6kl3tyivzhlrzu2@awork3.anarazel.de
Discussion: https://postgr.es/m/20200211042229.msv23badgqljrdg2@alap3.anarazel.de
2023-01-18 12:15:05 -08:00
Andres Freund 5764f611e1 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 <thomas.munro@gmail.com> (in an older version)
Discussion: https://postgr.es/m/20221120055930.t6kl3tyivzhlrzu2@awork3.anarazel.de
Discussion: https://postgr.es/m/20200211042229.msv23badgqljrdg2@alap3.anarazel.de
2023-01-18 11:41:14 -08:00
Andres Freund 51384cc40c 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 <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/20221120055930.t6kl3tyivzhlrzu2@awork3.anarazel.de
Discussion: https://postgr.es/m/20200211042229.msv23badgqljrdg2@alap3.anarazel.de
2023-01-18 11:41:14 -08:00
Andres Freund 2b16208753 Fix ILIST_DEBUG build
In c8ad4d8166 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.co7dp4oahiunwg57@awork3.anarazel.de
2023-01-18 10:26:15 -08:00
Tom Lane 47bb9db759 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
2023-01-18 13:23:57 -05:00
Tom Lane 8d83a5d0a2 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
2023-01-18 12:37:57 -05:00
Amit Kapila d540a02a72 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
2023-01-18 09:03:12 +05:30
Michael Paquier 14bdb3f13d 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
2023-01-18 11:15:48 +09:00
Michael Paquier 2f31f405e1 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
2023-01-18 08:55:26 +09:00
Tom Lane 09d465c397 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
2023-01-17 17:13:20 -05:00
Andres Freund 27da598961 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 <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/20230117173509.GV9837@telsasoft.com
2023-01-17 13:49:09 -08:00
Tom Lane 3a36ca03e4 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.
2023-01-17 16:00:39 -05:00
Peter Eisentraut 2a1d7071c4 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 <nathandbossart@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/504bc485-6bd6-dd1b-fe10-e7351aeb310d@enterprisedb.com
2023-01-17 20:06:06 +01:00
Peter Eisentraut 13b345df64 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 <nathandbossart@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/504bc485-6bd6-dd1b-fe10-e7351aeb310d@enterprisedb.com
2023-01-17 20:06:06 +01:00
John Naylor 15eb122880 Remove redundant relkind check
Ranier Vilela

Reviewed by Justin Pryzby
Discussion: https://www.postgresql.org/message-id/CAEudQAp2R2fbbi0OHHhv_n4%3DCh0t1VtjObR9YMqtGKHJ%2BfaUFQ%40mail.gmail.com
2023-01-17 14:25:01 +07:00
John Naylor e29c565343 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
2023-01-17 13:55:49 +07:00
Amit Kapila c981d9145d 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
2023-01-17 11:28:22 +05:30
Amit Kapila 4f985aba69 Fix typo in comment.
Author: Osumi Takamichi
Discussion: https://postgr.es/m/TYCPR01MB83737EA140C79B7D099F65E8EDC69@TYCPR01MB8373.jpnprd01.prod.outlook.com
2023-01-17 10:39:11 +05:30
Michael Paquier 0b717432ff 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
2023-01-17 13:41:09 +09:00
David Rowley da5800d5fa 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
2023-01-17 16:37:06 +13:00
Tom Lane 52585f8f07 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
2023-01-16 20:35:55 -05:00
Peter Geoghegan 980ae17310 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 <pg@bowt.ie>
Reviewed-By: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CAH2-WznuNGSzF8v6OsgjaC5aYsb3cZ6HW6MLm30X0d65cmSH6A@mail.gmail.com
2023-01-16 09:34:37 -08:00
Robert Haas 6fa66ec88f 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
2023-01-16 10:49:59 -05:00
Robert Haas 3cdf7502f8 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
2023-01-16 10:35:29 -05:00
Peter Eisentraut 20428d344a 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 <amit.kapila16@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/f3501945-c591-8cc3-5ef0-b72a2e0eaa9c@enterprisedb.com
2023-01-16 11:01:31 +01:00
Peter Eisentraut 1561612e3b 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
7897e3bb90, but some code was apparently
developed concurrently to that and not updated accordingly.

Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/f3501945-c591-8cc3-5ef0-b72a2e0eaa9c@enterprisedb.com
2023-01-16 09:44:04 +01:00
Michael Paquier 9a740f81eb 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
2023-01-16 16:31:43 +09:00
Michael Paquier 02d3448f4f 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
2023-01-16 13:58:07 +09:00
Tom Lane 647fa50054 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
2023-01-15 17:32:09 -05:00
Tom Lane 3f244d020f 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
2023-01-15 13:14:52 -05:00
Tatsuo Ishii 1334b79a35 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
2023-01-14 18:16:09 +09:00
Jeff Davis ff9618e82a 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
2023-01-14 00:16:23 -08:00
Andres Freund ff23b592ad Add 250c8ee07e to git-blame-ignore-revs 2023-01-13 15:33:49 -08:00
Andres Freund 250c8ee07e 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
2023-01-13 15:23:17 -08:00
Jeff Davis d46a9792a8 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
2023-01-13 14:42:03 -08:00
Jeff Davis c44f6334ca 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
2023-01-13 14:33:19 -08:00
Amit Kapila b7ae039536 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
2023-01-13 14:49:23 +05:30
Amit Kapila dca8b01f5f 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
2023-01-13 08:28:05 +05:30
Michael Paquier e753ae6397 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
2023-01-13 10:35:28 +09:00
Michael Paquier 58597edea1 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
2023-01-13 09:29:44 +09:00
Thomas Munro f1821b58fc 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 <andres@anarazel.de>
Discussion: https://postgr.es/m/901504.1673504836%40sss.pgh.pa.us
2023-01-13 11:02:12 +13:00
Alexander Korotkov 3161ae86ce 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
2023-01-12 18:16:34 +03:00
Peter Eisentraut c8ad4d8166 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
2023-01-12 08:00:51 +01:00
Peter Eisentraut 881fa869c6 Code cleanup
for commit c96de2ce17

Author: Nathan Bossart <nathandbossart@gmail.com>
Discussion: https://www.postgresql.org/message-id/20230111185434.GA1912982@nathanxps13
2023-01-12 07:37:39 +01:00
Michael Paquier 8607630d74 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
2023-01-12 14:23:20 +09:00
Michael Paquier bfd2542001 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.
2023-01-12 13:49:28 +09:00
Michael Paquier 5ad165d2c0 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
2023-01-12 13:40:33 +09:00