users/gsingh/postgres.git
2 years agoAdd check on initial and boot values when loading GUCs
Michael Paquier [Mon, 31 Oct 2022 04:54:23 +0000 (13:54 +0900)]
Add check on initial and boot values when loading GUCs

This commit adds a function to perform a cross-check between the initial
value of the C declaration associated to a GUC and its actual boot
value in assert-enabled builds.  The purpose of this is to prevent
anybody reading these C declarations from being fooled by mismatched
values before they are loaded at program startup.

The following rules apply depending on the GUC type:
* bool - can be false, or same as boot_val.
* int - can be 0, or same as the boot_val.
* real - can be 0.0, or same as the boot_val.
* string - can be NULL, or strcmp'd equal to the boot_val.
* enum - equal to the boot_val.

This is done for the system as well custom GUCs loaded by external
modules, which may require extension developers to adapt the C
declaration of the variables used by these GUCs (testing this change
with some of my own modules has allowed me to catch some stupid typos,
FWIW).  This may finish by being a bad experiment depending on the
feedbcak received, but let's see how it goes.

Author: Peter Smith
Reviewed-by: Nathan Bossart, Tom Lane, Michael Paquier, Justin Pryzby
Discussion: https://postgr.es/m/CAHut+PtHE0XSfjjRQ6D4v7+dqzCw=d+1a64ujra4EX8aoc_Z+w@mail.gmail.com

2 years agoClean up some inconsistencies with GUC declarations
Michael Paquier [Mon, 31 Oct 2022 03:44:48 +0000 (12:44 +0900)]
Clean up some inconsistencies with GUC declarations

This is similar to 7d25958, and this commit takes care of all the
remaining inconsistencies between the initial value used in the C
variable associated to a GUC and its default value stored in the GUC
tables (as of pg_settings.boot_val).

Some of the initial values of the GUCs updated rely on a compile-time
default.  These are refactored so as the GUC table and its C declaration
use the same values.  This makes everything consistent with other
places, backend_flush_after, bgwriter_flush_after, port,
checkpoint_flush_after doing so already, for example.

Extracted from a larger patch by Peter Smith.  The spots updated in the
modules are from me.

Author: Peter Smith, Michael Paquier
Reviewed-by: Nathan Bossart, Tom Lane, Justin Pryzby
Discussion: https://postgr.es/m/CAHut+PtHE0XSfjjRQ6D4v7+dqzCw=d+1a64ujra4EX8aoc_Z+w@mail.gmail.com

2 years agoUnder has_wal_read_bug, skip recovery/t/032_relfilenode_reuse.pl.
Noah Misch [Sat, 29 Oct 2022 17:42:16 +0000 (10:42 -0700)]
Under has_wal_read_bug, skip recovery/t/032_relfilenode_reuse.pl.

Per buildfarm member kittiwake.  Back-patch to v15, where this test
first appeared.

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

2 years agoUse Limit instead of Unique to implement DISTINCT, when possible
David Rowley [Fri, 28 Oct 2022 10:04:38 +0000 (23:04 +1300)]
Use Limit instead of Unique to implement DISTINCT, when possible

When all of the query's DISTINCT pathkeys have been marked as redundant
due to EquivalenceClasses existing which contain constants, we can just
implement the DISTINCT operation on a query by just limiting the number of
returned rows to 1 instead of performing a Unique on all of the matching
(duplicate) rows.

This applies in cases such as:

SELECT DISTINCT col,col2 FROM tab WHERE col = 1 AND col2 = 10;

If there are any matching rows, then they must all be {1,10}.  There's no
point in fetching all of those and running a Unique operator on them to
leave only a single row.  Here we effectively just find the first row and
then stop.  We are obviously unable to apply this optimization if either
the col = 1 or col2 = 10 were missing from the WHERE clause or if there
were any additional columns in the SELECT clause.

Such queries are probably not all that common, but detecting when we can
apply this optimization amounts to checking if the distinct_pathkeys are
NULL, which is very cheap indeed.

Nothing is done here to check if the query already has a LIMIT clause.  If
it does then the plan may end up with 2 Limits nodes.  There's no harm in
that and it's probably not worth the complexity to unify them into a
single Limit node.

Author: David Rowley
Reviewed-by: Richard Guo
Discussion: https://postgr.es/m/CAApHDvqS0j8RUWRUSgCAXxOqnYjHUXmKwspRj4GzVfOO25ByHA@mail.gmail.com
Discussion: https://postgr.es/m/MEYPR01MB7101CD5DA0A07C9DE2B74850A4239@MEYPR01MB7101.ausprd01.prod.outlook.com

2 years agoRemove AssertArg and AssertState
Peter Eisentraut [Fri, 28 Oct 2022 07:19:06 +0000 (09:19 +0200)]
Remove AssertArg and AssertState

These don't offer anything over plain Assert, and their usage had
already been declared obsolescent.

Author: Nathan Bossart <[email protected]>
Reviewed-by: Michael Paquier <[email protected]>
Discussion: https://www.postgresql.org/message-id/20221009210148.GA900071@nathanxps13

2 years agoAllow nodeSort to perform Datum sorts for byref types
David Rowley [Thu, 27 Oct 2022 20:25:12 +0000 (09:25 +1300)]
Allow nodeSort to perform Datum sorts for byref types

Here we add a new 'copy' parameter to tuplesort_getdatum so that we can
instruct the function not to datumCopy() byref Datums before returning.

Similar to 91e9e89dc, this can provide significant performance
improvements in nodeSort when sorting by a single byref column and the
sort's targetlist contains only that column.

This allows us to re-enable Datum sorts for byref types which was disabled
in 3a5817695 due to a reported memory leak.

Additionally, here we slightly optimize DISTINCT aggregates so that we no
longer perform any datumCopy() when we find the current value not to be
distinct from the previous value.  Previously the code would always take a
copy of the most recent Datum and pfree the previous value, even when the
values were the same.  Testing shows a small but noticeable performance
increase when aggregate transitions are skipped due to the current
transition value being the same as the prior one.

Author: David Rowley
Discussion: https://postgr.es/m/CAApHDvqS6wC5U==k9Hd26E4EQXH3QR67-T4=Q1rQ36NGvjfVSg@mail.gmail.com
Discussion: https://postgr.es/m/CAApHDvqHonfe9G1cVaKeHbDx70R_zCrM3qP2AGXpGrieSKGnhA@mail.gmail.com

2 years agoAvoid making commutatively-duplicate clauses in EquivalenceClasses.
Tom Lane [Thu, 27 Oct 2022 18:42:18 +0000 (14:42 -0400)]
Avoid making commutatively-duplicate clauses in EquivalenceClasses.

When we decide we need to make a derived clause equating a.x and
b.y, we already will re-use a previously-made clause "a.x = b.y".
But we might instead have "b.y = a.x", which is perfectly usable
because equivclass.c has never promised anything about the
operand order in clauses it builds.  Saving construction of a
new RestrictInfo doesn't matter all that much in itself --- but
because we cache selectivity estimates and so on per-RestrictInfo,
there's a possibility of saving a fair amount of duplicative
effort downstream.

Hence, check for commutative matches as well as direct ones when
seeing if we have a pre-existing clause.  This changes the visible
clause order in several regression test cases, but they're all
clearly-insignificant changes.

Checking for the reverse operand order is simple enough, but
if we wanted to check for operator OID match we'd need to call
get_commutator here, which is not so cheap.  I concluded that
we don't really need the operator check anyway, so I just
removed it.  It's unlikely that an opfamily contains more than
one applicable operator for a given pair of operand datatypes;
and if it does they had better give the same answers, so there
seems little need to insist that we use exactly the one
select_equality_operator chose.

Using the current core regression suite as a test case, I see
this change reducing the number of new join clauses built by
create_join_clause from 9673 to 5142 (out of 26652 calls).
So not quite 50% savings, but pretty close to it.

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

2 years agoMove pg_pwritev_with_retry() to src/common/file_utils.c
Michael Paquier [Thu, 27 Oct 2022 05:39:42 +0000 (14:39 +0900)]
Move pg_pwritev_with_retry() to src/common/file_utils.c

This commit moves pg_pwritev_with_retry(), a convenience wrapper of
pg_writev() able to handle partial writes, to common/file_utils.c so
that the frontend code is able to use it.  A first use-case targetted
for this routine is pg_basebackup and pg_receivewal, for the
zero-padding of a newly-initialized WAL segment.  This is used currently
in the backend when the GUC wal_init_zero is enabled (default).

Author: Bharath Rupireddy
Reviewed-by: Nathan Bossart, Thomas Munro
Discussion: https://postgr.es/m/CALj2ACUq7nAb7=bJNbK3yYmp-SZhJcXFR_pLk8un6XgDzDF3OA@mail.gmail.com

2 years agoAdd some tests to check the SQL functions of control file
Michael Paquier [Thu, 27 Oct 2022 00:58:44 +0000 (09:58 +0900)]
Add some tests to check the SQL functions of control file

As the recent commit 05d4cbf (reverted after as a448e49) has proved,
there is zero coverage for the four SQL functions that can scan the
control file data:
- pg_control_checkpoint()
- pg_control_init()
- pg_control_recovery()
- pg_control_system()

This commit adds a minimal coverage for these functions, checking that
their execution is able to complete.  This would have been enough to
catch the problems introduced in the commit mentioned above.  More
checks could be done for each individual fields, but it is unclear
whether this would be better than the other checks in place in the
backend code.

Per discussion with Bharath Rupireddy.

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

2 years agoAdd rule_number to pg_hba_file_rules and map_number to pg_ident_file_mappings
Michael Paquier [Wed, 26 Oct 2022 06:22:15 +0000 (15:22 +0900)]
Add rule_number to pg_hba_file_rules and map_number to pg_ident_file_mappings

These numbers are strictly-monotone identifiers assigned to each rule
of pg_hba_file_rules and each map of pg_ident_file_mappings when loading
the HBA and ident configuration files, indicating the order in which
they are checked at authentication time, until a match is found.

With only one file loaded currently, this is equivalent to the line
numbers assigned to the entries loaded if one wants to know their order,
but this becomes mandatory once the inclusion of external files is
added to the HBA and ident files to be able to know in which order the
rules and/or maps are applied at authentication.  Note that NULL is used
when a HBA or ident entry cannot be parsed or validated, aka when an
error exists, contrary to the line number.

Bump catalog version.

Author: Julien Rouhaud
Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya@jrouhaud

2 years agoFix variable assignment thinko in hba.c
Michael Paquier [Wed, 26 Oct 2022 03:57:40 +0000 (12:57 +0900)]
Fix variable assignment thinko in hba.c

The intention behind 1b73d0b was to limit the use of TokenizedAuthLine,
but I have fat-fingered one location in parse_hba_line() when creating
the HbaLine, where this should use the local variable and not the value
coming from TokenizedAuthLine.  This logic is the exactly the same, but
let's be clean about all that on consistency grounds.

Reported-by: Julien Rouhaud
Discussion: https://postgr.es/m/20221026032730.k3sib5krgm7l6njk@jrouhaud

2 years agoRefactor code handling the names of files loaded in hba.c
Michael Paquier [Wed, 26 Oct 2022 02:36:21 +0000 (11:36 +0900)]
Refactor code handling the names of files loaded in hba.c

This has the advantage to limit the presence of the GUC values
hba_file and ident_file to the code paths where these files are loaded,
easing the introduction of an upcoming feature aimed at adding inclusion
logic for files and directories in HBA and ident files.

Note that this needs the addition of the source file name to HbaLine, in
addition to the line number, which is something needed by the backend in
two places of auth.c (authentication failure details and auth_id log
when log_connections is enabled).

While on it, adjust a log generated on authentication failure to report
the name of the actual HBA file on which the connection attempt matched,
where the line number and the raw line written in the HBA file were
already included.  This was previously hardcoded as pg_hba.conf, which
would be incorrect when a custom value is used at postmaster startup for
the GUC hba_file.

Extracted from a larger patch by the same author.

Author: Julien Rouhaud
Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya@jrouhaud

2 years agoDoc/improve confusing, inefficient tests to locate CTID variable.
Tom Lane [Tue, 25 Oct 2022 21:35:19 +0000 (17:35 -0400)]
Doc/improve confusing, inefficient tests to locate CTID variable.

The IsCTIDVar() tests in nodeTidscan.c and nodeTidrangescan.c
look buggy at first sight: they aren't checking that the varno
matches the table to be scanned.  Actually they're safe because
any Var in a scan-level qual must be for the correct table ...
but if we're depending on that, it's pretty pointless to verify
varlevelsup.  (Besides which, varlevelsup is *always* zero at
execution, since we've flattened the rangetable long since.)

Remove the useless varlevelsup check, and instead add some
commentary explaining why we don't need to check varno.

Noted while fooling with a planner change that causes the order
of "t1.ctid = t2.ctid" to change in some tidscan.sql tests;
I was briefly fooled into thinking there was a live bug here.

2 years agoUpdate outdated comment for TransactionIdSetTreeStatus
Heikki Linnakangas [Tue, 25 Oct 2022 19:43:52 +0000 (21:43 +0200)]
Update outdated comment for TransactionIdSetTreeStatus

Commit 06da3c570f changed the way subtransactions are marked as
SUBCOMMITTED, but the example it included actually documented the old
way. Update it.

Author: Japin Li
Discussion: https://www.postgresql.org/message-id/MEYP282MB16690BC96DFBE08CC857E1E3B6319%40MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM

2 years agoClean up some GUC declarations and comments
Michael Paquier [Tue, 25 Oct 2022 05:06:07 +0000 (14:06 +0900)]
Clean up some GUC declarations and comments

This adjusts a few things for GUCs related to logical replication,
replication slots and WAL senders, in the shape of incorrect comments
and values inconsistent with their initial default value.

Author: Peter Smith
Reviewed-by: Nathan Bossart, Tom Lane, Justin Pryzby
Discussion: https://postgr.es/m/CAHut+PtHE0XSfjjRQ6D4v7+dqzCw=d+1a64ujra4EX8aoc_Z+w@mail.gmail.com

2 years agoFix unlink() for STATUS_DELETE_PENDING on Windows.
Thomas Munro [Tue, 25 Oct 2022 02:26:03 +0000 (15:26 +1300)]
Fix unlink() for STATUS_DELETE_PENDING on Windows.

Commit f357233c assumed that it was OK to return ENOENT directly if
lstat() failed that way.  If we got STATUS_DELETE_PENDING while trying
to unlink a file that we had already unlinked successfully once before
but someone else still had open (on a kernel version that has "pending"
unlinks by default), then we would no longer reach the retry loop in
pgunlink().  That loop claims to be only for handling sharing violations
(a different phenomenon), but the errno is the same.

Restore that behavior with an explicit check, to see if it fixes the
occasional 'directory not empty' failures seen in the pg_upgrade tests
on CI.  Further improvements are possible with proposed upgrades to
modern Windows APIs that would replace this convoluted code.

Reported-by: Justin Pryzby <[email protected]>
Reviewed-by: Michael Paquier <[email protected]>
Discussion: https://postgr.es/m/20220920013122.GA31833%40telsasoft.com
Discussion: https://postgr.es/m/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg%40mail.gmail.com

2 years agoFix stat() for recursive junction points on Windows.
Thomas Munro [Tue, 25 Oct 2022 02:24:41 +0000 (15:24 +1300)]
Fix stat() for recursive junction points on Windows.

Commit c5cb8f3b supposed that we'd only ever have to follow one junction
point in stat(), because we don't construct longer chains of them ourselves.
When examining a parent directory supplied by the user, we should really be
able to cope with longer chains, just in case someone has their system
set up that way.  Choose an arbitrary cap of 8, to match the minimum
acceptable value of SYMLOOP_MAX in POSIX.

Previously I'd avoided reporting ELOOP thinking Windows didn't have it,
but it turns out that it does, so we can use the proper error number.

Reviewed-by: Roman Zharkov <[email protected]>
Discussion: https://postgr.es/m/CA%2BhUKGJ7JDGWYFt9%3D-TyJiRRy5q9TtPfqeKkneWDr1XPU1%2Biqw%40mail.gmail.com
Discussion: https://postgr.es/m/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg%40mail.gmail.com

2 years agoFix readlink() for non-PostgreSQL junction points on Windows.
Thomas Munro [Tue, 25 Oct 2022 02:21:42 +0000 (15:21 +1300)]
Fix readlink() for non-PostgreSQL junction points on Windows.

Since commit c5cb8f3b taught stat() to follow symlinks, and since initdb
uses pg_mkdir_p(), and that examines parent directories, our humble
readlink() implementation can now be exposed to junction points not of
PostgreSQL origin.  Those might be corrupted by our naive path mangling,
which doesn't really understand NT paths in general.

Simply decline to transform paths that don't look like a drive absolute
path.  That means that readlink() returns the NT path directly when
checking a parent directory of PGDATA that happen to point to a drive
using "rooted" format.  That  works for the purposes of our stat()
emulation.

Reported-by: Roman Zharkov <[email protected]>
Reviewed-by: Roman Zharkov <[email protected]>
Discussion: https://postgr.es/m/4590c37927d7b8ee84f9855d83229018%40postgrespro.ru
Discussion: https://postgr.es/m/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg%40mail.gmail.com

2 years agoFix lstat() for broken junction points on Windows.
Thomas Munro [Tue, 25 Oct 2022 02:20:00 +0000 (15:20 +1300)]
Fix lstat() for broken junction points on Windows.

When using junction points to emulate symlinks on Windows, one edge case
was not handled correctly by commit c5cb8f3b: if a junction point is
broken (pointing to a non-existent path), we'd report ENOENT.  This
doesn't break any known use case, but was noticed while developing a
test suite for these functions and is fixed here for completeness.

Also add translation ERROR_CANT_RESOLVE_FILENAME -> ENOENT, as that is
one of the errors Windows can report for some kinds of broken paths.

Discussion: https://postgr.es/m/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg%40mail.gmail.com

2 years agoFix readlink() return value on Windows.
Thomas Munro [Tue, 25 Oct 2022 02:13:52 +0000 (15:13 +1300)]
Fix readlink() return value on Windows.

Ancient bug noticed while working on a test suite for these functions.

Discussion: https://postgr.es/m/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg%40mail.gmail.com

2 years agoFix symlink() errno on Windows.
Thomas Munro [Tue, 25 Oct 2022 02:10:49 +0000 (15:10 +1300)]
Fix symlink() errno on Windows.

Ancient bug noticed while working on a test suite for these functions.

https://postgr.es/m/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg%40mail.gmail.com

2 years agodoc: Fix type of cursor_position in jsonlog table
Michael Paquier [Tue, 25 Oct 2022 00:29:21 +0000 (09:29 +0900)]
doc: Fix type of cursor_position in jsonlog table

This entry was listed as a "string", but it is a "number.  The other
fields are correctly described, on a second look.

Reported-by: Nuko Yokohama
Author: Tatsuo Ishii
Discussion: https://postgr.es/m/CAF3Gu1awoVoDP5d0_eN=cR=QkGVwH+OtFvwJkkc5cB_ZMWjyeA@mail.gmail.com
Backpatch-through: 15

2 years agoUpdate some comments that should've covered MERGE
Alvaro Herrera [Mon, 24 Oct 2022 10:52:43 +0000 (12:52 +0200)]
Update some comments that should've covered MERGE

Oversight in 7103ebb7aae8.  Backpatch to 15.

Author: Richard Guo <[email protected]>
Discussion: https://postgr.es/m/CAMbWs48gnDjZXq3-b56dVpQCNUJ5hD9kdtWN4QFwKCEapspNsA@mail.gmail.com

2 years agoFix recently added incorrect assertion
Alvaro Herrera [Mon, 24 Oct 2022 10:02:33 +0000 (12:02 +0200)]
Fix recently added incorrect assertion

Commit df3737a651f4 added an incorrect assertion about the preconditions
for invoking the backup cleanup callback: it misfires at session end in
case a backup completes successfully.  Fix it, using coding from Michaël
Paquier.  Also add some tests for the various cases.

Reported by Kyotaro Horiguchi <[email protected]>
Discussion: https://postgr.es/m/20221021.161038.1277961198945653224[email protected]

2 years agoImprove coverage of ruleutils.c for SQLValueFunctions
Michael Paquier [Mon, 24 Oct 2022 07:53:54 +0000 (16:53 +0900)]
Improve coverage of ruleutils.c for SQLValueFunctions

While looking at how these are handled in the parser and the executor, I
have noticed that there is no test coverage for most of these when
reverse-engineering an expression for a SQLValueFunction node in
ruleutils.c, including how these are reparsed when included in a FROM
clause.  Some hacking in this area has showed me that these could break
easily, so add some coverage to track the existing compatibility.

Extracted from a much larger patch by me.

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

2 years agoImprove tab completion for ALTER STATISTICS <name> SET in psql
Michael Paquier [Mon, 24 Oct 2022 06:46:42 +0000 (15:46 +0900)]
Improve tab completion for ALTER STATISTICS <name> SET in psql

The code was completing this pattern with a list of settable characters,
and it was possible to reach this state after completing a "ALTER
STATISTICS <name>" with SET.

Author: Vignesh C
Discussion: https://postgr.es/m/CALDaNm2HHF_371o+EeSjxDDS17Cx7d-ko2h1fLU94=ob=4_ktg@mail.gmail.com

2 years agoFix and improve TAP tests for pg_hba.conf and regexps
Michael Paquier [Mon, 24 Oct 2022 04:48:34 +0000 (13:48 +0900)]
Fix and improve TAP tests for pg_hba.conf and regexps

The new tests have been reporting a warning hidden in the logs, as of
"Odd number of elements in hash assignment" (perlcritic or similar did
not report an issue, actually).  This comes down to a typo in the test
"matching regexp for username" for a double-quoted regexp using commas,
where we passed an extra argument.  The test is intended to pass, but
this was causing the test to fail.  This also pointed out that the
newly-added role "md5,role" lacks an entry in the password file used to
provide the password, so add one.

While on it, make the tests pickier by checking the contents of the logs
generated on successful authentication.

Oversights in 8fea868.

2 years agoAdd support for regexps on database and user entries in pg_hba.conf
Michael Paquier [Mon, 24 Oct 2022 02:45:31 +0000 (11:45 +0900)]
Add support for regexps on database and user entries in pg_hba.conf

As of this commit, any database or user entry beginning with a slash (/)
is considered as a regular expression.  This is particularly useful for
users, as now there is no clean way to match pattern on multiple HBA
lines.  For example, a user name mapping with a regular expression needs
first to match with a HBA line, and we would skip the follow-up HBA
entries if the ident regexp does *not* match with what has matched in
the HBA line.

pg_hba.conf is able to handle multiple databases and roles with a
comma-separated list of these, hence individual regular expressions that
include commas need to be double-quoted.

At authentication time, user and database names are now checked in the
following order:
- Arbitrary keywords (like "all", the ones beginning by '+' for
membership check), that we know will never have a regexp.  A fancy case
is for physical WAL senders, we *have* to only match "replication" for
the database.
- Regular expression matching.
- Exact match.
The previous logic did the same, but without the regexp step.

We have discussed as well the possibility to support regexp pattern
matching for host names, but these happen to lead to tricky issues based
on what I understand, particularly with host entries that have CIDRs.

This commit relies heavily on the refactoring done in a903971 and
fc579e1, so as the amount of code required to compile and execute
regular expressions is now minimal.  When parsing pg_hba.conf, all the
computed regexps needs to explicitely free()'d, same as pg_ident.conf.

Documentation and TAP tests are added to cover this feature, including
cases where the regexps use commas (for clarity in the docs, coverage
for the parsing logic in the tests).

Note that this introduces a breakage with older versions, where a
database or user name beginning with a slash are treated as something to
check for an equal match.  Per discussion, we have discarded this as
being much of an issue in practice as it would require a cluster to
have database and/or role names that begin with a slash, as well as HBA
entries using these.  Hence, the consistency gained with regexps in
pg_ident.conf is more appealing in the long term.

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

Author: Bertrand Drouvot
Reviewed-by: Jacob Champion, Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/fff0d7c1-8ad4-76a1-9db3-0ab6ec338bf7@amazon.com

2 years agoRemove pgpid_t type, use pid_t instead
Peter Eisentraut [Sat, 22 Oct 2022 08:10:31 +0000 (10:10 +0200)]
Remove pgpid_t type, use pid_t instead

It's unclear why a separate type would be needed here.  We use plain
pid_t (or int) everywhere else.

(The only relevant platform where pid_t is not int is 64-bit MinGW,
where it is long long int.  So defining pid_t as long (which is 32-bit
on Windows), as was done here, doesn't even accommodate that one.)

Reverts 66fa6eba5a61be740a6c07de92c42221fae79e9c.

Discussion: https://www.postgresql.org/message-id/289c2e45-c7d9-5ce4-7eff-a9e2a33e1580@enterprisedb.com

2 years agopsql: Fix exit status when query is canceled
Peter Eisentraut [Sat, 22 Oct 2022 07:41:38 +0000 (09:41 +0200)]
psql: Fix exit status when query is canceled

Because of a small thinko in 7844c9918a43b494adde3575891d217a37062378,
psql -c would exit successfully when a query is canceled.  Fix this so
that it exits with a nonzero status, just like for all other errors.

2 years agoImprove memory handling across SQL-callable backup functions
Michael Paquier [Sat, 22 Oct 2022 02:54:02 +0000 (11:54 +0900)]
Improve memory handling across SQL-callable backup functions

Since pg_backup_start() and pg_backup_stop() exist, the tablespace map
data and the backup state data (backup_label string until 7d70809) have
been allocated in the TopMemoryContext.  This approach would cause
memory leaks in the session calling these functions if failures happen
before pg_backup_stop() ends, leaking more memory on repeated failures.
Both things need little memory so that would not be really noticeable
for most users, except perhaps connection poolers with long-lived
connections able to trigger backup failures with these functions.

This commit improves the logic in this area by not allocating anymore
the backup-related data that needs to travel across the SQL-callable
backup functions in TopMemoryContext, by using instead a dedicated
memory context child of TopMemoryContext.  The memory context is created
in pg_backup_start() and deleted when finishing pg_backup_stop().  In
the event of an in-flight failure, this memory context gets reset in the
follow-up pg_backup_start() call, so as we are sure that only one run
worth of data is leaked at any time.  Some cleanup was already done for
the backup data on a follow-up call of pg_backup_start(), but using a
memory context makes the whole simpler.

BASE_BACKUP commands are executed in isolation, relying on the memory
context created for replication commands, hence these do not need such
an extra logic.

Author: Bharath Rupireddy
Reviewed-by: Robert Haas, Alvaro Herrera, Cary Huang, Michael Paquier
Discussion: https://postgr.es/m/CALj2ACXqvfKF2B0beQ=aJMdWnpNohmBPsRg=EDQj_6y1t2O8mQ@mail.gmail.com

2 years agopg_basebackup: Fix cross-platform tablespace relocation.
Robert Haas [Fri, 21 Oct 2022 12:21:55 +0000 (08:21 -0400)]
pg_basebackup: Fix cross-platform tablespace relocation.

Specifically, when pg_basebackup is invoked with -Tx=y, don't error
out if x could plausibly be an absolute path either on Windows or on
non-Windows systems. We don't know whether the remote system is
running the same OS as the local system, so it's not appropriate to
assume that our local rule about absolute pathnames is the same as
the rule on the remote system.

Patch by me, reviewed by Tom Lane, Andrew Dunstan, and
Davinder Singh.

Discussion: http://postgr.es/m/CA+TgmoY+jC3YiskomvYKDPK3FbrmsDU7_8+wMHt02HOdJeRb0g@mail.gmail.com

2 years agoAdd CHECK_FOR_INTERRUPTS while restoring changes during decoding.
Amit Kapila [Fri, 21 Oct 2022 07:27:18 +0000 (12:57 +0530)]
Add CHECK_FOR_INTERRUPTS while restoring changes during decoding.

Previously in commit 42681dffaf, we added CFI during decoding changes but
missed another similar case that can happen while restoring changes
spilled to disk back into memory in a loop.

Reported-by: Robert Haas
Author: Amit Kapila
Backpatch-through: 10
Discussion: https://postgr.es/m/CA+TgmoaLObg0QbstbC8ykDwOdD1bDkr4AbPpB=0DPgA2JW0mFg@mail.gmail.com

2 years agoRefactor more logic for compilation of regular expressions in hba.c
Michael Paquier [Fri, 21 Oct 2022 00:55:56 +0000 (09:55 +0900)]
Refactor more logic for compilation of regular expressions in hba.c

It happens that the parts of hba.conf that are planned to be extended
to support regular expressions would finish by using the same error
message as the one used currently for pg_ident.conf when a regular
expression cannot be compiled, as long as the routine centralizing the
logic, regcomp_auth_token(), knows from which file the regexp comes from
and its line location in the so-said file.

This change makes the follow-up patches slightly simpler, and the logic
remains the same.  I suspect that this makes the proposal to add support
for file inclusions in pg_ident.conf and pg_hba.conf slightly simpler,
as well.

Extracted from a larger patch by the same author.  This is similar to
the refactoring done in fc579e1.

Author: Bertrand Drouvot
Discussion: https://postgr.es/m/fff0d7c1-8ad4-76a1-9db3-0ab6ec338bf7@amazon.com

2 years agoDoc: fix outdated wording about parallel seq scans
David Rowley [Thu, 20 Oct 2022 20:29:08 +0000 (09:29 +1300)]
Doc: fix outdated wording about parallel seq scans

56788d215 adjusted the parallel seq scan code so that instead of handing
out a single block at a time to parallel workers, it now hands out ranges
of blocks.

Here we update the documentation which still claimed that workers received
just 1 block at a time.

Reported-by: Zhang Mingli
Discussion: https://postgr.es/m/17c99615-2c3b-4e4e-9d0b-424a66a7bccd@Spark
Backpatch-through: 14, where 56788d215 was added.

2 years agoMake finding openssl program a configure or meson option
Peter Eisentraut [Thu, 20 Oct 2022 19:01:05 +0000 (21:01 +0200)]
Make finding openssl program a configure or meson option

Various test suites use the "openssl" program as part of their setup.
There isn't a way to override which openssl program is to be used,
other than by fiddling with the path, perhaps.  This has gotten
increasingly problematic because different versions of openssl have
different capabilities and do different things by default.

This patch checks for an openssl binary in configure and meson setup,
with appropriate ways to override it.  This is similar to how "lz4"
and "zstd" are handled, for example.  The meson build system actually
already did this, but the result was only used in some places.  This
is now applied more uniformly.

Reviewed-by: Michael Paquier <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/dc638b75-a16a-007d-9e1c-d16ed6cf0ad2%40enterprisedb.com

2 years agoImprove the accuracy of numeric power() for integer exponents.
Dean Rasheed [Thu, 20 Oct 2022 09:10:17 +0000 (10:10 +0100)]
Improve the accuracy of numeric power() for integer exponents.

This makes the choice of result scale of numeric power() for integer
exponents consistent with the choice for non-integer exponents, and
with the result scale of other numeric functions. Specifically, the
result scale will be at least as large as the scale of either input,
and sufficient to ensure that the result has at least 16 significant
digits.

Formerly, the result scale was based only on the scale of the first
input, without taking into account the weight of the result. For
results with negative weight, that could lead to results with very few
or even no non-zero significant digits (e.g., 10.0 ^ (-18) produced
0.0000000000000000).

Fix this by moving responsibility for the choice of result scale into
power_var_int(), which already has code to estimate the result weight.

Per report by Adrian Klaver and suggested fix by Tom Lane.

No back-patch -- arguably this is a bug fix, but one which is easy to
work around, so it doesn't seem worth the risk of changing query
results in stable branches.

Discussion: https://postgr.es/m/12a40226-70ac-3a3b-3d3a-fdaf9e32d312%40aklaver.com

2 years agoUse proper macro to access TransactionId
Alvaro Herrera [Thu, 20 Oct 2022 07:37:06 +0000 (09:37 +0200)]
Use proper macro to access TransactionId

In commit f10a025cfe97 I mistakenly used list_member_oid in a place
where list_member_xid is called for.  (Currently innocuous as both
typedefs are pretty much identical, but if we change either, it'll
become broken.)  Repair.

Author: Hou Zhijie <[email protected]>
Discussion: https://postgr.es/m/OS0PR01MB5716E2399494D4CB1A28A091942A9@OS0PR01MB5716.jpnprd01.prod.outlook.com

2 years agoFix assertion failures while processing NEW_CID record in logical decoding.
Amit Kapila [Thu, 20 Oct 2022 03:19:48 +0000 (08:49 +0530)]
Fix assertion failures while processing NEW_CID record in logical decoding.

When the logical decoding restarts from NEW_CID, since there is no
association between the top transaction and its subtransaction, both are
created as top transactions and have the same LSN. This caused the
assertion failure in AssertTXNLsnOrder().

This patch skips the assertion check until we reach the LSN at which we
start decoding the contents of the transaction, specifically
start_decoding_at LSN in SnapBuild. This is okay because we don't
guarantee to make the association between top transaction and
subtransaction until we try to decode the actual contents of transaction.
The ordering of the records prior to the start_decoding_at LSN should have
been checked before the restart.

The other assertion failure is due to the reason that we forgot to track
that we have considered top-level transaction id in the list of catalog
changing transactions that were committed when one of its subtransactions
is marked as containing catalog change.

Reported-by: Tomas Vondra, Osumi Takamichi
Author: Masahiko Sawada, Kuroda Hayato
Reviewed-by: Amit Kapila, Dilip Kumar, Kuroda Hayato, Kyotaro Horiguchi, Masahiko Sawada
Backpatch-through: 10
Discussion: https://postgr.es/m/a89b46b6-0239-2fd5-71a9-b19b1f7a7145%40enterprisedb.com
Discussion: https://postgr.es/m/TYCPR01MB83733C6CEAE47D0280814D5AED7A9%40TYCPR01MB8373.jpnprd01.prod.outlook.com

2 years agoBetter handle interrupting TAP tests
Alvaro Herrera [Wed, 19 Oct 2022 15:09:51 +0000 (17:09 +0200)]
Better handle interrupting TAP tests

Set up a signal handler for INT/TERM so that we run our END block if we
get them.  In END, if the exit status indicates a problem, call
_update_pid(-1) to improve chances of the stop working in case start()
hasn't returned yet.

Also, change END's teardown_node() so that it passes fail_ok=>1, so that
if a node fails to stop, we still stop the other nodes in the same test.

Per complaint from Andres Freund.

This doesn't seem important enough to backpatch, at least for now.

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

2 years agoGet rid of XLogCtlInsert->forcePageWrites
Alvaro Herrera [Wed, 19 Oct 2022 10:35:00 +0000 (12:35 +0200)]
Get rid of XLogCtlInsert->forcePageWrites

After commit 39969e2a1e4d, ->forcePageWrites is no longer very
interesting: we can just test whether runningBackups is different from 0.
This simplifies some code, so do away with it.

Reviewed-by: Bharath Rupireddy <[email protected]>
Discussion: https://postgr.es/m/39969e2a1e4d7f5a37f3ef37d53bbfe171e7d77a

2 years agoTrack LLVM 15 changes.
Thomas Munro [Wed, 19 Oct 2022 09:18:26 +0000 (22:18 +1300)]
Track LLVM 15 changes.

Per https://llvm.org/docs/OpaquePointers.html, support for non-opaque
pointers still exists and we can request that on our context.  We have
until LLVM 16 to move to opaque pointers, a much larger change.

Back-patch to 11, where LLVM support arrived.

Author: Thomas Munro <[email protected]>
Author: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/CAMHz58Sf_xncdyqsekoVsNeKcruKootLtVH6cYXVhhUR1oKPCg%40mail.gmail.com

2 years agoRemove pg_backup_start_callback and reuse similar code
Alvaro Herrera [Wed, 19 Oct 2022 08:35:53 +0000 (10:35 +0200)]
Remove pg_backup_start_callback and reuse similar code

We had two copies of almost identical logic to revert shared memory
state when a running backup aborts; we can remove
pg_backup_start_callback if we adapt do_pg_abort_backup so that it can
be used for this purpose too.

However, in order for this to work, we have to repurpose the flag passed
to do_pg_abort_backup.  It used to indicate whether to throw a warning
(and the only caller always passed true).  It now indicates whether the
callback is being called at start time (in which case the session backup
state is known not to have been set to RUNNING yet, so action is always
taken) or shmem time (in which case action is only taken if the session
backup state is RUNNING).  Thus the meaning of the flag is no longer
superfluous, but it's actually quite critical to get right.  I (Álvaro)
chose to change the polarity and the code flow re. the flag from what
Bharath submitted, for coding clarity.

Co-authored-by: Bharath Rupireddy <[email protected]>
Discussion: https://www.postgresql.org/message-id/20221013111330.564fk5tkwe3ha77l%40alvherre.pgsql

2 years agoRework shutdown callback of archiver modules
Michael Paquier [Wed, 19 Oct 2022 05:06:56 +0000 (14:06 +0900)]
Rework shutdown callback of archiver modules

As currently designed, with a callback registered in a ERROR_CLEANUP
block, the shutdown callback would get called twice when updating
archive_library on SIGHUP, which is something that we want to avoid to
ease the life of extension writers.

Anyway, an ERROR in the archiver process is treated as a FATAL, stopping
it immediately, hence there is no need for a ERROR_CLEANUP block.
Instead of that, the shutdown callback is not called upon
before_shmem_exit(), giving to the modules the opportunity to do any
cleanup actions before the server shuts down its subsystems.

While on it, this commit adds some testing coverage for the shutdown
callback.  Neither shell_archive nor basic_archive have been using it,
and one is added to shell_archive, whose trigger is checked in a TAP
test through a shutdown sequence.

Author: Nathan Bossart, Bharath Rupireddy
Reviewed-by: Kyotaro Horiguchi, Michael Paquier
Discussion: https://postgr.es/m/20221015221328.GB1821022@nathanxps13
Backpatch-through: 15

2 years agoEnhance make_ctags and make_etags.
Tatsuo Ishii [Wed, 19 Oct 2022 03:59:29 +0000 (12:59 +0900)]
Enhance make_ctags and make_etags.

make_ctags did not include field members of structs since the commit
964d01ae90c314eb31132c2e7712d5d9fc237331.

For example, in the following field of RestrictInfo:

   Selectivity norm_selec pg_node_attr(equal_ignore);

pg_node_attr was mistakenly interpreted to be the name of the field.
To fix this, add -I option to ctags command if the command is
Exuberant ctags or Universal ctags (for plain old ctags, struct
members are not included in the tags file anyway).

Also add "-e" and "-n" options to make_ctags. The -e option invokes
ctags command with -e option, which produces TAGS file for emacs. This
allows to eliminate duplicate codes in make_etags so that make_etags
just exec make_ctags with -e option.

The -n option allows not to produce symbolic links in each
sub directory (the default is producing symbolic links).

Author: Yugo Nagata
Reviewers: Alvaro Herrera, Tatsuo Ishii
Discussion: https://postgr.es/m/flat/20221007154442.76233afc7c5b255c4de6528a%40sraoss.co.jp

2 years agoFix typos in logical/launcher.c
Michael Paquier [Wed, 19 Oct 2022 01:27:23 +0000 (10:27 +0900)]
Fix typos in logical/launcher.c

Author: Peter Smith
Discussion: https://postgr.es/m/CAHut+Pvbma5HCc7==-B1ycyLQVyu7Fqq-qV=jhC5Zx4pWqk3uw@mail.gmail.com

2 years agoRefactor regular expression handling in hba.c
Michael Paquier [Wed, 19 Oct 2022 01:08:49 +0000 (10:08 +0900)]
Refactor regular expression handling in hba.c

AuthToken gains a regular expression, and IdentLine is changed so as it
uses an AuthToken rather than tracking separately the ident user string
used for the regex compilation and its generated regex_t.  In the case
of pg_ident.conf, a set of AuthTokens is built in the pre-parsing phase
of the file, and an extra regular expression is compiled when building
the list of IdentLines, after checking the sanity of the fields in a
pre-parsed entry.

The logic in charge of computing and executing regular expressions is
now done in a new set of routines called respectively
regcomp_auth_token() and regexec_auth_token() that are wrappers around
pg_regcomp() and pg_regexec(), working on AuthTokens.  While on it, this
patch adds a routine able to free an AuthToken, free_auth_token(), to
simplify a bit the logic around the requirement of using a specific free
routine for computed regular expressions.  Note that there are no
functional or behavior changes introduced by this commit.

The goal of this patch is to ease the use of regular expressions with
more items of pg_hba.conf (user list, database list, potentially
hostnames) where AuthTokens are used extensively.  This will be tackled
later in a separate patch.

Author: Bertrand Drouvot, Michael Paquier
Discussion: https://postgr.es/m/fff0d7c1-8ad4-76a1-9db3-0ab6ec338bf7@amazon.com

2 years agoFix confusion about havingQual vs hasHavingQual in planner.
Tom Lane [Tue, 18 Oct 2022 14:44:34 +0000 (10:44 -0400)]
Fix confusion about havingQual vs hasHavingQual in planner.

Preprocessing of the HAVING clause will reduce havingQual to NIL
if the clause is constant-TRUE.  This is one case where that
convention is rather unfortunate, because "HAVING TRUE" is not at all
the same as not having any HAVING clause at all.  (Per the SQL spec,
it still forces the query to be grouped.)  The planner deals with this
by having a boolean hasHavingQual that records whether havingQual was
originally nonempty; places that just want to check whether HAVING
was specified are supposed to consult that.

I found three places that got that wrong.  Fortunately, these could
only affect cost estimates not correctness.  It'd be hard even
to demonstrate the errors; for example, the one in allpaths.c would
only matter in a query that has HAVING TRUE but no GROUP BY and no
aggregates, which would require a completely variable-free SELECT
list, making the case probably of only academic interest.  Hence,
while these are worth fixing before someone copies the incorrect
coding somewhere more critical, they don't seem worth back-patching.
I didn't bother trying to devise regression tests, either.

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

2 years agoRemove no-longer-needed compatibility hack
Alvaro Herrera [Tue, 18 Oct 2022 09:51:50 +0000 (11:51 +0200)]
Remove no-longer-needed compatibility hack

Our Perl version requirement was raised to 5.14 by commit 4c1532763a00

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

2 years agoImprove errhint for ALTER SUBSCRIPTION ADD/DROP PUBLICATION
Alvaro Herrera [Tue, 18 Oct 2022 09:46:58 +0000 (11:46 +0200)]
Improve errhint for ALTER SUBSCRIPTION ADD/DROP PUBLICATION

The original hint says to use SET PUBLICATION when really ADD/DROP
PUBLICATION is called for, so this is arguably a bug fix.

Also, a very similar message elsewhere was using an inconsistent
SQLSTATE.

While at it, unwrap some strings.

Backpatch to 15.

Author: Hou zj <[email protected]>
Discussion: https://postgr.es/m/OS0PR01MB57160AD0E7386547BA978EB394299@OS0PR01MB5716.jpnprd01.prod.outlook.com

2 years agoRemove compatibility declarations for InitMaterializedSRF()
Michael Paquier [Tue, 18 Oct 2022 01:44:02 +0000 (10:44 +0900)]
Remove compatibility declarations for InitMaterializedSRF()

These routines have been renamed in a19e5ce.  There is no need to keep
the compatibility declarations on HEAD, as once an extension moves to
the new routine name when compiling with v16~ the code would work the
same way when recompiled on v15.  No backpatch to v15 for this one,
because ABI compatibility has to be maintained there.

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

2 years agoRename SetSingleFuncCall() to InitMaterializedSRF()
Michael Paquier [Tue, 18 Oct 2022 01:22:35 +0000 (10:22 +0900)]
Rename SetSingleFuncCall() to InitMaterializedSRF()

Per discussion, the existing routine name able to initialize a SRF
function with materialize mode is unpopular, so rename it.  Equally, the
flags of this function are renamed, as of:
- SRF_SINGLE_USE_EXPECTED -> MAT_SRF_USE_EXPECTED_DESC
- SRF_SINGLE_BLESS -> MAT_SRF_BLESS
The previous function and flags introduced in 9e98583 are kept around
for compatibility purposes, so as any extension code already compiled
with v15 continues to work as-is.  The declarations introduced here for
compatibility will be removed from HEAD in a follow-up commit.

The new names have been suggested by Andres Freund and Melanie
Plageman.

Discussion: https://postgr.es/m/20221013194820[email protected]
Backpatch-through: 15

2 years agodoc: move the mention of aggregate JSON functions up in section
Bruce Momjian [Mon, 17 Oct 2022 19:21:29 +0000 (15:21 -0400)]
doc:  move the mention of aggregate JSON functions up in section

It was previously easily overlooked at the end of several tables.

Reported-by: Alex Denman
Discussion: https://postgr.es/m/166335888474.659.16897487975376230364@wrigleys.postgresql.org

Backpatch-through: 10

2 years agodoc: warn pg_stat_reset() can cause vacuum/analyze problems
Bruce Momjian [Mon, 17 Oct 2022 19:07:03 +0000 (15:07 -0400)]
doc:  warn pg_stat_reset() can cause vacuum/analyze problems

The fix is to run ANALYZE.

Discussion: https://postgr.es/m/[email protected],
   https://postgr.es/m/flat/CAKJS1f8DTbCHf9gedU0He6ARsd58E6qOhEHM1caomqj_r9MOiQ%40mail.gmail.com,
   https://postgr.es/m/CAKJS1f80o98hcfSk8j%3DfdN09S7Sjz%2BvuzhEwbyQqvHJb_sZw0g%40mail.gmail.com

Backpatch-through: 10

2 years agoRecord dependencies of a cast on other casts that it requires.
Tom Lane [Mon, 17 Oct 2022 18:02:05 +0000 (14:02 -0400)]
Record dependencies of a cast on other casts that it requires.

When creating a cast that uses a conversion function, we've
historically allowed the input and result types to be
binary-compatible with the function's input and result types,
rather than necessarily being identical.  This means that the new
cast is logically dependent on the binary-compatible cast or casts
that it references: if those are defined by pg_cast entries, and you
try to restore the new cast without having defined them, it'll fail.
Hence, we should make pg_depend entries to record these dependencies
so that pg_dump knows that there is an ordering requirement.

This is not the only place where we allow such shortcuts; aggregate
functions for example are similarly lax, and in principle should gain
similar dependencies.  However, for now it seems sufficient to fix
the cast-versus-cast case, as pg_dump's other ordering heuristics
should keep it out of trouble for other object types.

Per report from David Turoň; thanks also to Robert Haas for
preliminary investigation.  I considered back-patching, but
seeing that this issue has existed for many years without
previous reports, it's not clear it's worth the trouble.
Moreover, back-patching wouldn't be enough to ensure that the
new pg_depend entries exist in existing databases anyway.

Discussion: https://postgr.es/m/OF0A160F3E.578B15D1-ONC12588DA.003E4857-C12588DA.0045A428@notes.linuxbox.cz

2 years agoReject non-ON-SELECT rules that are named "_RETURN".
Tom Lane [Mon, 17 Oct 2022 16:14:39 +0000 (12:14 -0400)]
Reject non-ON-SELECT rules that are named "_RETURN".

DefineQueryRewrite() has long required that ON SELECT rules be named
"_RETURN".  But we overlooked the converse case: we should forbid
non-ON-SELECT rules that are named "_RETURN".  In particular this
prevents using CREATE OR REPLACE RULE to overwrite a view's _RETURN
rule with some other kind of rule, thereby breaking the view.

Per bug #17646 from Kui Liu.  Back-patch to all supported branches.

Discussion: https://postgr.es/m/17646-70c93cfa40365776@postgresql.org

2 years agoGuard against table-AM-less relations in planner.
Tom Lane [Mon, 17 Oct 2022 15:35:23 +0000 (11:35 -0400)]
Guard against table-AM-less relations in planner.

The executor will dump core if it's asked to execute a seqscan on
a relation having no table AM, such as a view.  While that shouldn't
really happen, it's possible to get there via catalog corruption,
such as a missing ON SELECT rule.  It seems worth installing a defense
against that.  There are multiple plausible places for such a defense,
but I picked the planner's get_relation_info().

Per discussion of bug #17646 from Kui Liu.  Back-patch to v12 where
the tableam APIs were introduced; in older versions you won't get a
SIGSEGV, so it seems less pressing.

Discussion: https://postgr.es/m/17646-70c93cfa40365776@postgresql.org

2 years agoFix calculation related to temporary WAL segment name in basic_archive
Michael Paquier [Mon, 17 Oct 2022 02:40:14 +0000 (11:40 +0900)]
Fix calculation related to temporary WAL segment name in basic_archive

The file name used for its temporary destination, before renaming it to
the real deal, has been using a microseconds in a timestamp aimed to be
originally in milli-seconds.  This is harmless as this is aimed at being
a safeguard against name collisions (note MyProcPid in the name), but
let's be correct with the maths.

While on it, add a note in the module's makefile to document why
installcheck is not supported.

Author: Nathan Bossart
Reviewed-by: Bharath Rupireddy
Discussion: https://postgr.es/m/20221014044106.GA1673343@nathanxps13
Backpatch-through: 15

2 years agoAdd checks for regexes with user name map in test for peer authentication
Michael Paquier [Mon, 17 Oct 2022 02:06:00 +0000 (11:06 +0900)]
Add checks for regexes with user name map in test for peer authentication

There is already some coverage for that in the kerberos test suite,
though it requires PG_TEST_EXTRA to be set as per its insecure nature.
This provides coverage in a default setup, as long as peer is supported
on the platform where its test is run.

Author: Bertrand Drouvot
Discussion: https://postgr.es/m/7f87ca27-e184-29da-15d6-8be4325ad02e@gmail.com

2 years agoFix EXPLAIN of SEARCH BREADTH FIRST with a constant initial value.
Tom Lane [Sun, 16 Oct 2022 23:18:08 +0000 (19:18 -0400)]
Fix EXPLAIN of SEARCH BREADTH FIRST with a constant initial value.

If the non-recursive term of a SEARCH BREADTH FIRST recursive
query has only constants in its target list, the planner will
fold the starting RowExpr added by rewrite into a simple Const
of type RECORD.  The executor doesn't have any problem with
that --- but EXPLAIN VERBOSE will encounter the Const as the
ultimate source of truth about what the field names of the
SET column are, and it didn't know what to do with that.
Fortunately, we can pull the identifying typmod out of the
Const, in much the same way that record_out would.

For reasons that remain a bit obscure to me, this only fails
with SEARCH BREADTH FIRST, not SEARCH DEPTH FIRST or CYCLE.
But I added regression test cases for both of those options
too, just to make sure we don't break it in future.

Per bug #17644 from Matthijs van der Vleuten.  Back-patch
to v14 where these constructs were added.

Discussion: https://postgr.es/m/17644-3bd1f3036d6d7a16@postgresql.org

2 years agoRename parser token REF to REF_P to avoid a symbol conflict.
Tom Lane [Sun, 16 Oct 2022 19:27:04 +0000 (15:27 -0400)]
Rename parser token REF to REF_P to avoid a symbol conflict.

In the latest version of Apple's macOS SDK, <sys/socket.h>
fails to compile if "REF" is #define'd as something.
Apple may or may not agree that this is a bug, and even if
they do accept the bug report I filed, they probably won't
fix it very quickly.  In the meantime, our back branches will all
fail to compile gram.y.  v15 and HEAD currently escape the problem
thanks to the refactoring done in 98e93a1fc, but that's purely
accidental.  Moreover, since that patch removed a widely-visible
inclusion of <netdb.h>, back-patching it seems too likely to break
third-party code.

Instead, change the token's code name to REF_P, following our usual
convention for naming parser tokens that are likely to have symbol
conflicts.  The effects of that should be localized to the grammar
and immediately surrounding files, so it seems like a safer answer.

Per project policy that we want to keep recently-out-of-support
branches buildable on modern systems, back-patch all the way to 9.2.

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

2 years agoUse libc's snprintf, not sprintf, for special cases in snprintf.c.
Tom Lane [Sun, 16 Oct 2022 15:47:44 +0000 (11:47 -0400)]
Use libc's snprintf, not sprintf, for special cases in snprintf.c.

snprintf.c has always fallen back on libc's *printf implementation
when printing pointers (%p) and floats.  When this code originated,
we were still supporting some platforms that lacked native snprintf,
so we used sprintf for that.  That's not actually unsafe in our usage,
but nonetheless builds on macOS are starting to complain about sprintf
being unconditionally deprecated; and I wouldn't be surprised if other
platforms follow suit.  There seems little reason to believe that any
platform supporting C99 wouldn't have standards-compliant snprintf,
so let's just use that instead to suppress such warnings.

Back-patch to v12, which is where we started to require C99.  It's
also where we started to use our snprintf.c everywhere, so this
wouldn't be enough to suppress the warning in older branches anyway
--- that is, in older branches these aren't necessarily all our
usages of libc's sprintf.  It is enough in v12+ because any
deprecation annotation attached to libc's sprintf won't apply to
pg_sprintf.  (Whether all our usages of pg_sprintf are adequately
safe is not a matter I intend to address here, but perhaps it could
do with some review.)

Per report from Andres Freund and local testing.

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

2 years agomeson: macos: Use -Wl,-undefined,error for modules
Andres Freund [Sun, 16 Oct 2022 00:00:27 +0000 (17:00 -0700)]
meson: macos: Use -Wl,-undefined,error for modules

meson defaults to -Wl,-undefined,dynamic_lookup for modules, which we don't
want because a) it's different from what we do for autoconf, b) it causes
warnings starting in macOS Ventura.

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

2 years agomeson: catch up to a few configure changes
Andres Freund [Sat, 15 Oct 2022 19:00:16 +0000 (12:00 -0700)]
meson: catch up to a few configure changes

I (Andres) missed a few recent changes to configure when merging
e6927270cd1 "meson: Add initial version of meson based build system". Mirror
the changes in
ec3c9cc202f "Add definition pg_attribute_aligned() for MSVC"
b086a47a270 "Bump minimum version of Bison to 2.3"
8b878bffa8d "Bump minimum version of Flex to 2.5.35"

As MSVC does not implement 128 bit integers, the oversight of not using
pg_attribute_aligned() should not have current practical consequences. But of
course the code from c.h should still be correctly mirrored.

I (Andres) also hadn't implemented the minimum perl version check. Added that
now.

Reported-by: Junwang Zhao <[email protected]>
Author: Junwang Zhao <[email protected]>
Author: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/CAEG8a3K9c87EwAwmdOgmS0Li1J6P_7r-Uc0-zN6cJtrMr7VvPg@mail.gmail.com

2 years agoDisallow MERGE cleanly for foreign partitions
Alvaro Herrera [Sat, 15 Oct 2022 17:24:26 +0000 (19:24 +0200)]
Disallow MERGE cleanly for foreign partitions

While directly targetting a foreign table with MERGE was already
expressly forbidden, we failed to catch the case of a partitioned table
that has a foreign table as a partition; and the result if you try is an
incomprehensible error.  Fix that by adding a specific check.

Backpatch to 15.

Reported-by: Tatsuhiro Nakamori <[email protected]>
Discussion: https://postgr.es/m/[email protected]

2 years agoFix some comments in proc.h
Michael Paquier [Sat, 15 Oct 2022 03:22:29 +0000 (12:22 +0900)]
Fix some comments in proc.h

There was a typo and two places where delayChkpt was still mentioned,
but it is called delayChkptFlags these days.

Author: David Christensen
Discussion: https://postgr.es/m/CAOxo6XLB=ab_Y9jRw4iKyMZDns0wo=EGSRvijhhaL67RzqbtMg@mail.gmail.com

2 years agopgstat: Track time of the last scan of a relation
Andres Freund [Fri, 14 Oct 2022 18:11:34 +0000 (11:11 -0700)]
pgstat: Track time of the last scan of a relation

It can be useful to know when a relation has last been used, e.g., when
evaluating whether an index is still required. It was already possible to
infer the time of the last usage by tracking, e.g.,
pg_stat_all_indexes.idx_scan over time. But far from everybody does so.

To make it easier to detect the last time a relation has been scanned, track
that time in each relation's pgstat entry. To minimize overhead a) the
timestamp is updated only when the backend pending stats entry is flushed to
shared stats b) the last transaction's stop timestamp is used as the
timestamp.

Bumps catalog and stats format versions.

Author: Dave Page <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
Reviewed-by: Bruce Momjian <[email protected]>
Reviewed-by: Vik Fearing <[email protected]>
Discussion: https://postgr.es/m/CA+OCxozrVHNFVEPkweUHMZje+t1tfY816d9MZYc6eZwOOusOaQ@mail.gmail.com

2 years agoHave GetCurrentTransactionStopTimestamp() set xactStopTimestamp if unset
Andres Freund [Fri, 14 Oct 2022 18:11:33 +0000 (11:11 -0700)]
Have GetCurrentTransactionStopTimestamp() set xactStopTimestamp if unset

Previously GetCurrentTransactionStopTimestamp() computed a new timestamp
whenever xactStopTimestamp was unset and xactStopTimestamp was only set when a
commit or abort record was written.

An upcoming patch will add additional calls to
GetCurrentTransactionStopTimestamp() from pgstats. To avoid computing
timestamps multiple times, set xactStopTimestamp in
GetCurrentTransactionStopTimestamp() if not already set.

Author: Dave Page <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
Reviewed-by: Vik Fearing <[email protected]>
Discussion: https://postgr.es/m/20220906155325.an3xesq5o3fq36gt%40awork3.anarazel.de

2 years agolibpq: Reset singlerow flag correctly in pipeline mode
Alvaro Herrera [Fri, 14 Oct 2022 17:06:26 +0000 (19:06 +0200)]
libpq: Reset singlerow flag correctly in pipeline mode

When a query whose results were requested in single-row mode is the last
in the queue by the time those results are being read, the single-row
flag was not being reset, because we were returning early from
pqPipelineProcessQueue.  Move that stanza up so that the flag is always
reset at the end of sending that query's results.

Add a test for the situation.

Backpatch to 14.

Author: Denis Laxalde <[email protected]>
Discussion: https://postgr.es/m/01af18c5-dacc-a8c8-07ee-aecc7650c3e8@dalibo.com

2 years agoAdd auxiliary lists to GUC data structures for better performance.
Tom Lane [Fri, 14 Oct 2022 16:36:14 +0000 (12:36 -0400)]
Add auxiliary lists to GUC data structures for better performance.

The previous patch made addition of new GUCs cheap, but other GUC
operations aren't improved and indeed get a bit slower, because
hash_seq_search() is slower than just scanning a pointer array.

However, most performance-critical GUC operations only need
to touch a relatively small fraction of the GUCs; especially
so for AtEOXact_GUC().  We can improve matters at the cost
of a bit more space by adding dlist or slist links to the
GUC data structures.  This patch invents lists that track

(1) all GUCs with non-default "source";

(2) all GUCs with nonempty state stack (implying they've
been changed in the current transaction);

(3) all GUCs due for reporting to the client.

All of guc.c's performance-critical cases can make use of one or
another of these lists to avoid searching the whole hash table.
In particular, the stack list means that transaction end
doesn't take time proportional to the number of GUCs, but
only to the number changed in the current transaction.

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

2 years agoReplace the sorted array of GUC variables with a hash table.
Tom Lane [Fri, 14 Oct 2022 16:26:39 +0000 (12:26 -0400)]
Replace the sorted array of GUC variables with a hash table.

This gets rid of bsearch() in favor of hashed lookup.  The main
advantage is that it becomes far cheaper to add new GUCs, since
we needn't re-sort the pointer array.  Adding N new GUCs had
been O(N^2 log N), but now it's closer to O(N).  We need to
sort only in SHOW ALL and equivalent functions, which are
hopefully not performance-critical to anybody.

Also, merge GetNumConfigOptions() into get_guc_variables(),
because in a world where the set of GUCs isn't fairly static
you really want to consider those two results as tied together
not independent.

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

2 years agoStore GUC data in a memory context, instead of using malloc().
Tom Lane [Fri, 14 Oct 2022 16:10:48 +0000 (12:10 -0400)]
Store GUC data in a memory context, instead of using malloc().

The only real argument for using malloc directly was that we needed
the ability to not throw error on OOM; but mcxt.c grew that feature
awhile ago.

Keeping the data in a memory context improves accountability and
debuggability --- for example, without this it's almost impossible
to detect memory leaks in the GUC code with anything less costly
than valgrind.  Moreover, the next patch in this series will add a
hash table for GUC lookup, and it'd be pretty silly to be using
palloc-dependent hash facilities alongside malloc'd storage of the
underlying data.

This is a bit invasive though, in particular causing an API break
for GUC check hooks that want to modify the GUC's value or use an
"extra" data structure.  They must now use guc_malloc() and
guc_free() instead of malloc() and free().  Failure to change
affected code will result in assertion failures or worse; but
thanks to recent effort in the mcxt infrastructure, it shouldn't
be too hard to diagnose such oversights (at least in assert-enabled
builds).

One note is that this changes ParseLongOption() to return short-lived
palloc'd not malloc'd data.  There wasn't any caller for which the
previous definition was better.

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

2 years agoMake some minor improvements in memory-context infrastructure.
Tom Lane [Fri, 14 Oct 2022 15:55:56 +0000 (11:55 -0400)]
Make some minor improvements in memory-context infrastructure.

We lack a version of repalloc() that supports MCXT_ALLOC_NO_OOM
semantics, so invent repalloc_extended() with the usual set of
flags.  repalloc_huge() becomes a legacy wrapper for that.

Also, fix dynahash.c so that it can support HASH_ENTER_NULL
requests when using the default palloc-based allocator.
The only reason it didn't do that already was the lack of the
MCXT_ALLOC_NO_OOM option when that code was written, ages ago.

While here, simplify a few overcomplicated tests in mcxt.c.

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

2 years agoStandardize format for printing PIDs
Peter Eisentraut [Fri, 14 Oct 2022 06:37:12 +0000 (08:37 +0200)]
Standardize format for printing PIDs

Most code prints PIDs as %d, but some code tried to print them as long
or unsigned long.  While this is in theory allowed, the fact that PIDs
fit into int is deeply baked into all PostgreSQL code, so these random
deviations don't accomplish anything except confusion.

Note that we still need casts from pid_t to int, because on 64-bit
MinGW, pid_t is long long int.  (But per above, actually supporting
that range in PostgreSQL code would be major surgery and probably not
useful.)

Discussion: https://www.postgresql.org/message-id/289c2e45-c7d9-5ce4-7eff-a9e2a33e1580@enterprisedb.com

2 years agodoc: Correct type of bgw_notify_pid
Peter Eisentraut [Fri, 14 Oct 2022 06:37:12 +0000 (08:37 +0200)]
doc: Correct type of bgw_notify_pid

This has apparently been wrong since the beginning
(090d0f2050647958865cb495dff74af7257d2bb4).

Discussion: https://www.postgresql.org/message-id/289c2e45-c7d9-5ce4-7eff-a9e2a33e1580@enterprisedb.com

2 years agoFix incorrect comment regarding command completion tags
David Rowley [Fri, 14 Oct 2022 01:32:00 +0000 (14:32 +1300)]
Fix incorrect comment regarding command completion tags

The comment talked about some Asserts which did not exist and also a
variable name which seems to have long since disappeared.

Rewrite the comment in a way that will hopefully stand the test of
time and inform people why we always write "INSERT 0 <nrows>" instead of
"INSERT <nrows>" in the command completion tag for INSERT.

Reviewed-by: Mark Dilger
Discussion: https://postgr.es/m/CAApHDvpiUg09AvvGAVopNAKemA9z-kCmt7Fi6HKauc32bKzx4w@mail.gmail.com

2 years agoRemove redundant memset call following palloc0
Daniel Gustafsson [Thu, 13 Oct 2022 21:18:00 +0000 (23:18 +0200)]
Remove redundant memset call following palloc0

This is a follow-up commit to ca7f8e2 which removed the allocation
abstraction from pgcrypto and replaced px_alloc + memset calls with
palloc0 calls. The particular memset in this commit was missed in
that work though.

Author: Zhihong Yu <[email protected]>
Reviewed-by: Bruce Momjian <[email protected]>
Reviewed-by: Nathan Bossart <[email protected]>
Discussion: https://postgr.es/m/CALNJ-vT5qRucrFMPSzQyAWods1b4MnNPG-M=_ZUzh1SoTh0vNw@mail.gmail.com

2 years agopg_buffercache: Add pg_buffercache_summary()
Andres Freund [Thu, 13 Oct 2022 16:55:46 +0000 (09:55 -0700)]
pg_buffercache: Add pg_buffercache_summary()

Using pg_buffercache_summary() is significantly cheaper than querying
pg_buffercache and summarizing in SQL.

Author: Melih Mutlu <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
Reviewed-by: Aleksander Alekseev <[email protected]>
Reviewed-by: Zhang Mingli <[email protected]>
Discussion: https://postgr.es/m/CAGPVpCQAXYo54Q%3D8gqBsS%3Du0uk9qhnnq4%2B710BtUhUisX1XGEg%40mail.gmail.com

2 years agoFix typo in CREATE PUBLICATION reference page
Alvaro Herrera [Thu, 13 Oct 2022 11:36:14 +0000 (13:36 +0200)]
Fix typo in CREATE PUBLICATION reference page

While at it, simplify wording a bit.

Author: Takamichi Osumi <[email protected]>
Reviewed-by: Peter Smith <[email protected]>
Discussion: https://postgr.es/m/TYCPR01MB8373F93F5D094A2BE648990DED259@TYCPR01MB8373.jpnprd01.prod.outlook.com

2 years agoPut tests of md5() function into separate test file
Peter Eisentraut [Thu, 13 Oct 2022 09:46:18 +0000 (11:46 +0200)]
Put tests of md5() function into separate test file

In FIPS mode, these calls will fail.  By having them in a separate
file, it would make it easier to have an alternative output file or
selectively disable these tests.  This isn't done here; this is just
some preparation.

Reviewed-by: Michael Paquier <[email protected]>
Discussion: https://www.postgresql.org/message-id/647f6cc1-473d-f788-ade0-c09201e5ab6a@enterprisedb.com

2 years agoAllow batch insertion during COPY into a foreign table.
Etsuro Fujita [Thu, 13 Oct 2022 09:45:00 +0000 (18:45 +0900)]
Allow batch insertion during COPY into a foreign table.

Commit 3d956d956 allowed the COPY, but it's done by inserting individual
rows to the foreign table, so it can be inefficient due to the overhead
caused by each round-trip to the foreign server.  To improve performance
of the COPY in such a case, this patch allows batch insertion, by
extending the multi-insert machinery in CopyFrom() to the foreign-table
case so that we insert multiple rows to the foreign table at once using
the FDW callback routine added by commit b663a4136.  This patch also
allows this for postgres_fdw.  It is enabled by the "batch_size" option
added by commit b663a4136, which is disabled by default.

When doing batch insertion, we update progress of the COPY command after
performing the FDW callback routine, to count rows not suppressed by the
FDW as well as a BEFORE ROW INSERT trigger.  For consistency, this patch
changes the timing of updating it for plain tables: previously, we
updated it immediately after adding each row to the multi-insert buffer,
but we do so only after writing the rows stored in the buffer out to the
table using table_multi_insert(), which I think would be consistent even
with non-batching mode, because in that mode we update it after writing
each row out to the table using table_tuple_insert().

Andrey Lepikhov, heavily revised by me, with review from Ian Barwick,
Andrey Lepikhov, and Zhihong Yu.

Discussion: https://postgr.es/m/bc489202-9855-7550-d64c-ad2d83c24867%40postgrespro.ru

2 years agoAdd missing isolation test for test_decoding in meson build
Michael Paquier [Thu, 13 Oct 2022 07:03:01 +0000 (16:03 +0900)]
Add missing isolation test for test_decoding in meson build

Oversight in 7f13ac8, where catalog_change_snapshot was missing from the
list in meson.build.

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

2 years agoImprove the WARNING message for CREATE SUBSCRIPTION.
Amit Kapila [Thu, 13 Oct 2022 00:39:43 +0000 (06:09 +0530)]
Improve the WARNING message for CREATE SUBSCRIPTION.

Author: Peter Smith
Reviewed-By: Alvaro Herrera, Tom Lane, Amit Kapila
Discussion: https://postgr.es/m/CAHut+PvqdqOanheWSHDyhQiF+Z-7w=-+k4U+bwbT=b6YQ_hrXQ@mail.gmail.com

2 years agoFix ordering issue with WAL operations in GIN fast insert path
Michael Paquier [Thu, 13 Oct 2022 00:31:57 +0000 (09:31 +0900)]
Fix ordering issue with WAL operations in GIN fast insert path

Contrary to what is documented in src/backend/access/transam/README,
ginHeapTupleFastInsert() had a few ordering issues with the way it does
its WAL operations when inserting items in its fast path.

First, when using a separate list, XLogBeginInsert() was being always
called before START_CRIT_SECTION(), and in this case a second thing was
wrong when merging lists, as an exclusive lock was taken on the tail
page *before* calling XLogBeginInsert().  Finally, when inserting items
into a tail page, the order of XLogBeginInsert() and
START_CRIT_SECTION() was reversed.  This commit addresses all these
issues by moving the calls of XLogBeginInsert() after all the pages
logged are locked and pinned, within a critical section.

Author:  Matthias van de Meent, Zhang Mingli
Discussion: https://postgr.es/m/CAEze2WhL8uLMqynnnCu1LAPwxD5RKEo0nHV+eXGg_N6ELU88HQ@mail.gmail.com

2 years agodoc: Fix description of replication command CREATE_REPLICATION_SLOT
Michael Paquier [Wed, 12 Oct 2022 23:53:42 +0000 (08:53 +0900)]
doc: Fix description of replication command CREATE_REPLICATION_SLOT

The output plugin name is a mandatory option when creating a logical
slot, but the grammar documented was not described as such.  While on
it, fix two comments in repl_gram.y to show that TEMPORARY is an
optional grammar choice.

Author: Ayaki Tachikake
Discussion: https://postgr.es/m/OSAPR01MB2852607B2329FFA27834105AF1229@OSAPR01MB2852.jpnprd01.prod.outlook.com
Backpatch-through: 15

2 years agoDoc: improve recommended systemd unit file.
Tom Lane [Wed, 12 Oct 2022 14:51:11 +0000 (10:51 -0400)]
Doc: improve recommended systemd unit file.

Add
    After=network-online.target
    Wants=network-online.target
to the suggested unit file for starting a Postgres server.
This delays startup until the network interfaces have been
configured; without that, any attempt to bind to a specific
IP address will fail.

If listen_addresses is set to "localhost" or "*", it might be
possible to get away with the less restrictive "network.target",
but I don't think we need to get into such detail here.

Per suggestion from Pablo Federico.

Discussion: https://postgr.es/m/166552157407.591805.10036014441784710940@wrigleys.postgresql.org

2 years agoFix outdated code reference
Alvaro Herrera [Wed, 12 Oct 2022 07:53:00 +0000 (09:53 +0200)]
Fix outdated code reference

ExecCreatePartitionPruneState was renamed by commit 297daa9d4353, but
this test file didn't get the memo.  Repair.

Author: Amit Langote
Discussion: https://postgr.es/m/CA+HiwqFLw=oLX0tP9kcKBmoOExNjDaoAe99dRcxo-GdB9abP9A@mail.gmail.com

2 years agoReduce xlog.h inclusion footprint
Alvaro Herrera [Wed, 12 Oct 2022 07:44:40 +0000 (09:44 +0200)]
Reduce xlog.h inclusion footprint

This file needs xlogreader.h only for the XLogReaderState typedef; but
we can dodge that by forward-declaring it.  Many files use xlog.h for
reasons other than reading WAL, and it's not good to force all those
files to include xlogreader.h, so take it out.

Surprisingly, there is no fallout in core code from making this change.
Perhaps external code will have to start including xlogreader.h.

2 years agoReduce basebackup_sink.h inclusion footprint
Alvaro Herrera [Wed, 12 Oct 2022 07:42:20 +0000 (09:42 +0200)]
Reduce basebackup_sink.h inclusion footprint

This file doesn't need xlog_internal.h, only xlogdefs.h.

2 years agoAdd meson.build to version_stamp.pl
Peter Eisentraut [Wed, 12 Oct 2022 05:03:51 +0000 (07:03 +0200)]
Add meson.build to version_stamp.pl

Author: Dagfinn Ilmari Mannsåker <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/7567dd2d-5e28-c135-79ff-270d7ed83490%40enterprisedb.com

2 years agoRemove Abs()
Peter Eisentraut [Wed, 12 Oct 2022 04:36:12 +0000 (06:36 +0200)]
Remove Abs()

All callers have been replaced by standard C library functions.

Reviewed-by: Zhang Mingli <[email protected]>
Reviewed-by: Tom Lane <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/4beb42b5-216b-bce8-d452-d924d5794c63%40enterprisedb.com

2 years agoFix shadow variable in postgres.c
Michael Paquier [Wed, 12 Oct 2022 04:42:30 +0000 (13:42 +0900)]
Fix shadow variable in postgres.c

-Wshadow=compatible-local is added by default since 0fe954c, and this
warning was detected under -DWRITE_READ_PARSE_PLAN_TREES.

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

2 years agoSimplify some maths in xlogreader.c
Michael Paquier [Wed, 12 Oct 2022 00:59:36 +0000 (09:59 +0900)]
Simplify some maths in xlogreader.c

An LSN was calculated from a segment number, a segment size and a
position offset, matching exactly the LSN given by the caller of
XLogReaderValidatePageHeader().  This change removes the extra LSN
calculation, relying only on the LSN given by the function caller
instead.

Author: Bharath Rupireddy
Reviewed-by: Richard Guo, Álvaro Herrera, Kyotaro Horiguchi
Discussion: https://postgr.es/m/CALj2ACXuh4Ms9j9sxMYdtHEe=5sFcyrs-GAHyADu_A_G71kZTg@mail.gmail.com

2 years agoFix compilation warning in test_copy_callbacks
Michael Paquier [Tue, 11 Oct 2022 23:45:01 +0000 (08:45 +0900)]
Fix compilation warning in test_copy_callbacks

A passed-in parameter value was incorrect, for a warning coming from
MSVC.

Oversight in 9fcdf2c.

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

2 years agoHarden pmsignal.c against clobbered shared memory.
Tom Lane [Tue, 11 Oct 2022 22:54:31 +0000 (18:54 -0400)]
Harden pmsignal.c against clobbered shared memory.

The postmaster is not supposed to do anything that depends
fundamentally on shared memory contents, because that creates
the risk that a backend crash that trashes shared memory will
take the postmaster down with it, preventing automatic recovery.
In commit 969d7cd43 I lost sight of this principle and coded
AssignPostmasterChildSlot() in such a way that it could fail
or even crash if the shared PMSignalState structure became
corrupted.  Remarkably, we've not seen field reports of such
crashes; but I managed to induce one while testing the recent
changes around palloc chunk headers.

To fix, make a semi-duplicative state array inside the postmaster
so that we need consult only local state while choosing a "child
slot" for a new backend.  Ensure that other postmaster-executed
routines in pmsignal.c don't have critical dependencies on the
shared state, either.  Corruption of PMSignalState might now
lead ReleasePostmasterChildSlot() to conclude that backend X
failed, when actually backend Y was the one that trashed things.
But that doesn't matter, because we'll force a cluster-wide reset
regardless.

Back-patch to all supported branches, since this is an old bug.

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

2 years agoYet further fixes for multi-row VALUES lists for updatable views.
Tom Lane [Tue, 11 Oct 2022 22:24:14 +0000 (18:24 -0400)]
Yet further fixes for multi-row VALUES lists for updatable views.

DEFAULT markers appearing in an INSERT on an updatable view
could be mis-processed if they were in a multi-row VALUES clause.
This would lead to strange errors such as "cache lookup failed
for type NNNN", or in older branches even to crashes.

The cause is that commit 41531e42d tried to re-use rewriteValuesRTE()
to remove any SetToDefault nodes (that hadn't previously been replaced
by the view's own default values) appearing in "product" queries,
that is DO ALSO queries.  That's fundamentally wrong because the
DO ALSO queries might not even be INSERTs; and even if they are,
their targetlists don't necessarily match the view's column list,
so that almost all the logic in rewriteValuesRTE() is inapplicable.

What we want is a narrow focus on replacing any such nodes with NULL
constants.  (That is, in this context we are interpreting the defaults
as being strictly those of the view itself; and we already replaced
any that aren't NULL.)  We could add still more !force_nulls tests
to further lobotomize rewriteValuesRTE(); but it seems cleaner to
split out this case to a new function, restoring rewriteValuesRTE()
to the charter it had before.

Per bug #17633 from jiye_sw.  Patch by me, but thanks to
Richard Guo and Japin Li for initial investigation.
Back-patch to all supported branches, as the previous fix was.

Discussion: https://postgr.es/m/17633-98cc85e1fa91e905@postgresql.org

2 years agoDoc: add entry for pg_get_partkeydef().
Tom Lane [Tue, 11 Oct 2022 18:28:38 +0000 (14:28 -0400)]
Doc: add entry for pg_get_partkeydef().

Other pg_get_XXXdef() functions are documented, so it seems reasonable
to include this as well.

Ian Barwick

Discussion: https://postgr.es/m/CAB8KJ=hb2QZXdgyrrRjPCw++DsrRcui4fKArWabQ+oij+2x=_w@mail.gmail.com

2 years agoC comment: explain procArray->pgprocnos[]
Bruce Momjian [Tue, 11 Oct 2022 17:08:17 +0000 (13:08 -0400)]
C comment:  explain procArray->pgprocnos[]

Reported-by: Aleksander Alekseev
Discussion: https://postgr.es/m/CAJ7c6TOs9Dh3KNR2kiQJ3Ow0=TBucL_57DAbm--2p8w5x_8YXQ@mail.gmail.com

Author: Aleksander Alekseev

Backpatch-through: master

2 years agoAdd a common function to generate the origin name.
Amit Kapila [Tue, 11 Oct 2022 05:07:52 +0000 (10:37 +0530)]
Add a common function to generate the origin name.

Make a common replication origin name formatting function to replace
multiple snprintf() expressions. This also includes logic previously done
by ReplicationOriginNameForTablesync().

This makes the code to generate the origin name consistent among apply
worker and tablesync worker.

Author: Peter Smith
Reviewed-By: Aleksander Alekseev
Discussion: https://postgr.es/m/CAHut%2BPsa8hhfSE6ozUK-ih7GkQziAVAf4f3bqiXEj2nQiu-43g%40mail.gmail.com

2 years agoAdd TAP tests for role membership in pg_hba.conf
Michael Paquier [Tue, 11 Oct 2022 04:57:07 +0000 (13:57 +0900)]
Add TAP tests for role membership in pg_hba.conf

This commit expands the coverage of pg_hba.conf with checks specific to
role memberships (one "root" role combined with a member and a
non-member).  Coverage is added for the database keywords "samegroup"
and "samerole", where the specified role has to be be a member of the
role with the same name as the requested database, and '+' on the user
entry, where members are allowed.  These tests are plugged in the
authentication test 001_password.pl as of extra connection attempts
combined with resets of pg_hba.conf, making them rather cheap.

Author: Nathan Bossart
Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/20221009211348.GB900071@nathanxps13