postgresql.git
2 years agoMove variable increment to the end of the loop
John Naylor [Tue, 20 Dec 2022 07:13:14 +0000 (14:13 +0700)]
Move variable increment to the end of the loop

This is less error prone and matches the placement of other code
in the file.

Justin Pryzby

Reviewed by Tom Lane
Discussion: https://www.postgresql.org/message-id/20221123172436.GJ11463@telsasoft.com

2 years agoAdd pg_dissect_walfile_name()
Michael Paquier [Tue, 20 Dec 2022 04:36:27 +0000 (13:36 +0900)]
Add pg_dissect_walfile_name()

This function takes in input a WAL segment name and returns a tuple made
of the segment sequence number (dependent on the WAL segment size of the
cluster) and its timeline, as of a thin SQL wrapper around the existing
XLogFromFileName().

This function has multiple usages, like being able to compile a LSN from
a file name and an offset, or finding the timeline of a segment without
having to do to some maths based on the first eight characters of the
segment.

Bump catalog version.

Author: Bharath Rupireddy
Reviewed-by: Nathan Bossart, Kyotaro Horiguchi, Maxim Orlov, Michael
Paquier
Discussion: https://postgr.es/m/CALj2ACWV=FCddsxcGbVOA=cvPyMr75YCFbSQT6g4KDj=gcJK4g@mail.gmail.com

2 years agoRemove hardcoded dependency to cryptohash type in the internals of SCRAM
Michael Paquier [Mon, 19 Dec 2022 23:53:22 +0000 (08:53 +0900)]
Remove hardcoded dependency to cryptohash type in the internals of SCRAM

SCRAM_KEY_LEN was a variable used in the internal routines of SCRAM to
size a set of fixed-sized arrays used in the SHA and HMAC computations
during the SASL exchange or when building a SCRAM password.  This had a
hard dependency on SHA-256, reducing the flexibility of SCRAM when it
comes to the addition of more hash methods.  A second issue was that
SHA-256 is assumed as the cryptohash method to use all the time.

This commit renames SCRAM_KEY_LEN to a more generic SCRAM_KEY_MAX_LEN,
which is used as the size of the buffers used by the internal routines
of SCRAM.  This is aimed at tracking centrally the maximum size
necessary for all the hash methods supported by SCRAM.  A global
variable has the advantage of keeping the code in its simplest form,
reducing the need of more alloc/free logic for all the buffers used in
the hash calculations.

A second change is that the key length (SHA digest length) and hash
types are now tracked by the state data in the backend and the frontend,
the common portions being extended to handle these as arguments by the
internal routines of SCRAM.  There are a few RFC proposals floating
around to extend the SCRAM protocol, including some to use stronger
cryptohash algorithms, so this lifts some of the existing restrictions
in the code.

The code in charge of parsing and building SCRAM secrets is extended to
rely on the key length and on the cryptohash type used for the exchange,
assuming currently that only SHA-256 is supported for the moment.  Note
that the mock authentication simply enforces SHA-256.

Author: Michael Paquier
Reviewed-by: Peter Eisentraut, Jonathan Katz
Discussion: https://postgr.es/m/Y5k3Qiweo/1g9CG6@paquier.xyz

2 years agoFix comment that was missing a word.
Robert Haas [Mon, 19 Dec 2022 20:56:17 +0000 (15:56 -0500)]
Fix comment that was missing a word.

Ted Yu

Discussion: http://postgr.es/m/CALte62wkFB05=RTWf7BL_6MfWs2=DY=ai-K7LWn_+0TJUuPJ2w@mail.gmail.com

2 years agoFix typo in comment
Peter Eisentraut [Mon, 19 Dec 2022 20:08:28 +0000 (21:08 +0100)]
Fix typo in comment

Author: Ted Yu <yuzhihong@gmail.com>

2 years agoExpose some information about backend subxact status.
Robert Haas [Mon, 19 Dec 2022 19:43:09 +0000 (14:43 -0500)]
Expose some information about backend subxact status.

A new function pg_stat_get_backend_subxact() can be used to get
information about the number of subtransactions in the cache of
a particular backend and whether that cache has overflowed. This
can be useful for tracking down performance problems that can
result from overflowed snapshots.

Dilip Kumar, reviewed by Zhihong Yu, Nikolay Samokhvalov,
Justin Pryzby, Nathan Bossart, Ashutosh Sharma, Julien
Rouhaud. Additional design comments from Andres Freund,
Tom Lane, Bruce Momjian, and David G. Johnston.

Discussion: http://postgr.es/m/CAFiTN-ut0uwkRJDQJeDPXpVyTWD46m3gt3JDToE02hTfONEN=Q@mail.gmail.com

2 years agoFix bit-rotted planner test case.
Tom Lane [Sat, 17 Dec 2022 23:51:24 +0000 (18:51 -0500)]
Fix bit-rotted planner test case.

While fooling with my pet outer-join-variables patch, I discovered
that the test case I added in commit 11086f2f2 no longer demonstrates
what it's supposed to.  The idea is to tempt the planner to reverse
the order of the two outer joins, which would leave noplace to
correctly evaluate the WHERE clause that's inserted between them.
Before the addition of the delay_upper_joins mechanism, it would
have taken the bait.

However, subsequent improvements broke the test in two different ways.
First, we now recognize the IS NULL coding pattern as an antijoin, and
we won't re-order antijoins; even if we did, the IS NULL test clauses
get removed so there would be no opportunity for them to misbehave.
Second, the planner now discovers that nested parameterized indexscans
are a lot cheaper than the double hash join it used back in the day,
and that approach doesn't want to re-order the joins anyway.  Thus,
in HEAD the test passes even if one dikes out delay_upper_joins.

To fix, change the IS NULL tests to COALESCE clauses, which produce
the same results but the planner isn't smart enough to convert them
to antijoins.  It'll still go for parameterized indexscans though,
so drop the index enabling that (don't know why I added that in the
first place), and disable nestloop joining just to be sure.

This time around, add an EXPLAIN to make the choice of plan visible.

2 years agoDoc: update pg_list.h header comments to include XidLists.
Tom Lane [Sat, 17 Dec 2022 15:31:25 +0000 (10:31 -0500)]
Doc: update pg_list.h header comments to include XidLists.

I realize that the XidList infrastructure is rather incomplete,
but failing to mention it in adjacent comments takes that a bit
too far.

2 years agoFix inability to reference CYCLE column from inside its CTE.
Tom Lane [Fri, 16 Dec 2022 18:07:42 +0000 (13:07 -0500)]
Fix inability to reference CYCLE column from inside its CTE.

Such references failed with "cache lookup failed for type 0"
because we didn't resolve the type of the CYCLE column until after
analyzing the CTE's query.  We can just move that processing
to before the recursive parse_sub_analyze call, though.

While here, invent a couple of local variables to make this
code less egregiously wider-than-80-columns.

Per bug #17723 from Vik Fearing.  Back-patch to v14 where
the CYCLE feature was added.

Discussion: https://postgr.es/m/17723-2c4985ff111e7bba@postgresql.org

2 years agopg_upgrade: Make testing different transfer modes easier
Peter Eisentraut [Fri, 16 Dec 2022 16:49:59 +0000 (17:49 +0100)]
pg_upgrade: Make testing different transfer modes easier

The environment variable PG_TEST_PG_UPGRADE_MODE can be set to
override the default transfer mode for the pg_upgrade tests.
(Automatically running the pg_upgrade tests for all supported modes
would be too slow.)

Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/50a97009-8ff9-ca4d-a0f6-6086a6775a5b%40enterprisedb.com

2 years agopg_upgrade: Add --copy option
Peter Eisentraut [Fri, 16 Dec 2022 16:49:59 +0000 (17:49 +0100)]
pg_upgrade: Add --copy option

This option selects the default transfer mode.  Having an explicit
option is handy to make scripts and tests more explicit.  It also
makes it easier to talk about a "copy" mode rather than "the default
mode" or something like that, since until now the default mode didn't
have an externally visible name.

Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/50a97009-8ff9-ca4d-a0f6-6086a6775a5b%40enterprisedb.com

2 years agoC comment: fix wording
Bruce Momjian [Fri, 16 Dec 2022 17:15:54 +0000 (12:15 -0500)]
C comment:  fix wording

Backpatch-through: master

2 years agoClean up dubious error handling in wellformed_xml().
Tom Lane [Fri, 16 Dec 2022 16:10:36 +0000 (11:10 -0500)]
Clean up dubious error handling in wellformed_xml().

This ancient bit of code was summarily trapping any ereport longjmp
whatsoever and assuming that it must represent an invalid-XML report.
It's not really appropriate to handle OOM-like situations that way:
maybe the input is valid or maybe not, but we couldn't find out.
And it'd be a seriously bad idea to ignore, say, a query cancel
error that way.  (Perhaps that can't happen because there is no
CHECK_FOR_INTERRUPTS anywhere within xml_parse, but even if that's
true today it's obviously a very fragile assumption.)

But in the wake of the previous commit, we can drop the PG_TRY
here altogether, and use the soft error mechanism to catch only
the kinds of errors that are legitimate to treat as invalid-XML.

(This is our first use of the soft error mechanism for something
not directly related to a datatype input function.  It won't be
the last.)

xml_is_document can be converted in the same way.  That one is
not actively broken, because it was checking specifically for
ERRCODE_INVALID_XML_DOCUMENT rather than trapping everything;
but the code is still shorter and probably faster this way.

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

2 years agoConvert xml_in to report errors softly.
Tom Lane [Fri, 16 Dec 2022 15:58:49 +0000 (10:58 -0500)]
Convert xml_in to report errors softly.

The key idea here is that xml_parse must distinguish hard errors
from soft errors.  We want to throw a hard error for libxml
initialization failures: those might be out-of-memory, or something
else, but in any case they are not the fault of the input string.
If we get to the point of parsing the input, and something goes
wrong, we can fairly consider that to mean bad input.

One thing that arguably does mean bad input, but I didn't trouble
to handle softly, is encoding conversion failure while converting
the server encoding to UTF8.  This might be something to improve
later, but it seems like a pretty low-probability scenario.

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

2 years agoFix typo in reference to __FreeBSD__.
Thomas Munro [Fri, 16 Dec 2022 04:36:22 +0000 (17:36 +1300)]
Fix typo in reference to __FreeBSD__.

Commit a2a8acd152 introduced a platform-dependent mechanism to prevent
developers from referencing errno in the argument list of
elog()/ereport(), but didn't use the right macro to detect FreeBSD, so
it didn't actually work there.

Reported-by: Japin Li <japinli@hotmail.com>
Discussion: https://postgr.es/m/MEYP282MB16693AAEEF84F47D8F7CA007B6E69%40MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM

2 years agoRemove pessimistic cost penalization from Incremental Sort
David Rowley [Fri, 16 Dec 2022 02:22:23 +0000 (15:22 +1300)]
Remove pessimistic cost penalization from Incremental Sort

When incremental sorts were added in v13 a 1.5x pessimism factor was added
to the cost modal.  Seemingly this was done because the cost modal only
has an estimate of the total number of input rows and the number of
presorted groups.  It assumes that the input rows will be evenly
distributed throughout the presorted groups.  The 1.5x pessimism factor
was added to slightly reduce the likelihood of incremental sorts being
used in the hope to avoid performance regressions where an incremental
sort plan was picked and turned out slower due to a large skew in the
number of rows in the presorted groups.

An additional quirk with the path generation code meant that we could
consider both a sort and an incremental sort on paths with presorted keys.
This meant that with the pessimism factor, it was possible that we opted
to perform a sort rather than an incremental sort when the given path had
presorted keys.

Here we remove the 1.5x pessimism factor to allow incremental sorts to
have a fairer chance at being chosen against a full sort.

Previously we would generally create a sort path on the cheapest input
path (if that wasn't sorted already) and incremental sort paths on any
path which had presorted keys.  This meant that if the cheapest input path
wasn't completely sorted but happened to have presorted keys, we would
create a full sort path *and* an incremental sort path on that input path.
Here we change this logic so that if there are presorted keys, we only
create an incremental sort path, and create sort paths only when a full
sort is required.

Both the removal of the cost pessimism factor and the changes made to the
path generation make it more likely that incremental sorts will now be
chosen.  That, of course, as with teaching the planner any new tricks,
means an increased likelihood that the planner will perform an incremental
sort when it's not the best method.  Our standard escape hatch for these
cases is an enable_* GUC.  enable_incremental_sort already exists for
this.

This came out of a report by Pavel Luzanov where he mentioned that the
master branch was choosing to perform a Seq Scan -> Sort -> Group
Aggregate for his query with an ORDER BY aggregate function.  The v15 plan
for his query performed an Index Scan -> Group Aggregate, of course, the
aggregate performed the final sort internally in nodeAgg.c for the
aggregate's ORDER BY.  The ideal plan would have been to use the index,
which provided partially sorted input then use an incremental sort to
provide the aggregate with the sorted input.  This was not being chosen
due to the pessimism in the incremental sort cost modal, so here we remove
that and rationalize the path generation so that sort and incremental sort
plans don't have to needlessly compete.  We assume that it's senseless
to ever use a full sort on a given input path where an incremental sort
can be performed.

Reported-by: Pavel Luzanov
Reviewed-by: Richard Guo
Discussion: https://postgr.es/m/9f61ddbf-2989-1536-b31e-6459370a6baa%40postgrespro.ru

2 years agoRe-adjust drop-index-concurrently-1 isolation test
David Rowley [Thu, 15 Dec 2022 22:39:40 +0000 (11:39 +1300)]
Re-adjust drop-index-concurrently-1 isolation test

It seems that drop-index-concurrently-1 has started to forget what it was
originally meant to be testing.  d2d8a229b, which added incremental sorts
changed the expected plan to be an Index Scan plan instead of a Seq Scan
plan.  This occurred as the primary key index of the table in question
provided presorted input and, because that index happened to be the
cheapest input path due to enable_seqscan being disabled, the incremental
sort changes just added a Sort on top of that.  It seems based on the name
of the PREPAREd statement that the intention here is that the query
produces a seqscan plan.

The reason this test has become broken seems to be due to how the test was
originally coded.  The test was trying to force a seqscan plan by
performing some casting to make it so the test_dc index couldn't be used
to perform the required filtering.  Trying to coax the planner into using
a plan which has costed in a disable_cost seems like it's always going to
be flakey as small changes in costs are drowned out by the large
disable_cost combined with add_path's STD_FUZZ_FACTOR.  Here we get rid of
the casts that we're using to try to trick the planner into a seqscan and
instead toggle enable_seqscan as and when required to get the desired
plan.

Additionally, rename a few things in the test and add some additional
wording to the comments to try and make it more clear in the future what
we expect this test to be doing.

Discussion: https://postgr.es/m/CAApHDvrbDhObhLV+=U_K_-t+2Av2av1aL9d+2j_3AO-XndaviA@mail.gmail.com
Backpatch-through: 13, where d2d8a229b changed the expected test output

2 years agoSpeed up creation of command completion tags
David Rowley [Thu, 15 Dec 2022 21:31:25 +0000 (10:31 +1300)]
Speed up creation of command completion tags

The building of command completion tags could often be seen showing up in
profiles when running high tps workloads.

The query completion tags were being built with snprintf, which is slow at
the best of times when compared with more manual ways of formatting
strings.  Here we introduce BuildQueryCompletionString() to do this job
for us.  We also now store the completion tag's strlen in the
CommandTagBehavior struct so that we can quickly memcpy this number of
bytes into the completion tag string.  Appending the rows affected is done
via pg_ulltoa_n.  BuildQueryCompletionString returns the length of the
built string.  This saves us having to call strlen to figure out how many
bytes to pass to pq_putmessage().

Author: David Rowley, Andres Freund
Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/CAHoyFK-Xwqc-iY52shj0G+8K9FJpse+FuZ36XBKy78wDVnd=Qg@mail.gmail.com

2 years agoConvert range_in and multirange_in to report errors softly.
Tom Lane [Thu, 15 Dec 2022 17:18:36 +0000 (12:18 -0500)]
Convert range_in and multirange_in to report errors softly.

This is mostly straightforward, except that if the range type
has a canonical function, that might throw an error during range
input.  (Such errors probably only occur for edge cases: in the
in-core canonical functions, it happens only if a bound has the
maximum valid value for the underlying type.)  Hence, this patch
extends the soft-error regime to allow canonical functions to
return errors softly as well.  Extensions implementing range
canonical functions will need modification anyway because of the
API change for range_serialize(); while at it, they might want
to do something similar to what's been done here in the in-core
canonical functions.

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

2 years agoStatic assertions cleanup
Peter Eisentraut [Thu, 8 Dec 2022 13:30:01 +0000 (14:30 +0100)]
Static assertions cleanup

Because we added StaticAssertStmt() first before StaticAssertDecl(),
some uses as well as the instructions in c.h are now a bit backwards
from the "native" way static assertions are meant to be used in C.
This updates the guidance and moves some static assertions to better
places.

Specifically, since the addition of StaticAssertDecl(), we can put
static assertions at the file level.  This moves a number of static
assertions out of function bodies, where they might have been stuck
out of necessity, to perhaps better places at the file level or in
header files.

Also, when the static assertion appears in a position where a
declaration is allowed, then using StaticAssertDecl() is more native
than StaticAssertStmt().

Reviewed-by: John Naylor <john.naylor@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/941a04e7-dd6f-c0e4-8cdf-a33b3338cbda%40enterprisedb.com

2 years agoMove provariadic sanity check to a more appropriate place
Peter Eisentraut [Thu, 15 Dec 2022 06:49:30 +0000 (07:49 +0100)]
Move provariadic sanity check to a more appropriate place

35f059e9bdfb3b14ac9d22a9e159d36ec0ccf804 put the provariadic sanity
check into type_sanity.sql, even though it's not about types, and
moreover in the middle of some connected test group, which makes it
all very confusing.  Move it to opr_sanity.sql, where it is in better
company.

2 years agoConvert a few more datatype input functions to report errors softly.
Tom Lane [Thu, 15 Dec 2022 00:42:05 +0000 (19:42 -0500)]
Convert a few more datatype input functions to report errors softly.

Convert the remaining string-category input functions
(bpcharin, varcharin, byteain) to the new style.

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

2 years agoConvert a few more datatype input functions to report errors softly.
Tom Lane [Wed, 14 Dec 2022 23:03:11 +0000 (18:03 -0500)]
Convert a few more datatype input functions to report errors softly.

Convert cash_in and uuid_in to the new style.

Amul Sul, minor mods by me

Discussion: https://postgr.es/m/CAAJ_b97KeDWUdpTKGOaFYPv0OicjOu6EW+QYWj-Ywrgj_aEy1g@mail.gmail.com

2 years agoConvert a few more datatype input functions to report errors softly.
Tom Lane [Wed, 14 Dec 2022 22:50:24 +0000 (17:50 -0500)]
Convert a few more datatype input functions to report errors softly.

Convert assorted internal-ish datatypes, namely aclitemin,
int2vectorin, oidin, oidvectorin, pg_lsn_in, pg_snapshot_in,
and tidin to the new style.

(Some others you might expect to find in this group, such as
cidin and xidin, need no changes because they never throw
errors at all.  That seems a little cheesy ... but it is not in
the charter of this patch series to add new error conditions.)

Amul Sul, minor mods by me

Discussion: https://postgr.es/m/CAAJ_b97KeDWUdpTKGOaFYPv0OicjOu6EW+QYWj-Ywrgj_aEy1g@mail.gmail.com

2 years agoConvert the geometric input functions to report errors softly.
Tom Lane [Wed, 14 Dec 2022 21:10:20 +0000 (16:10 -0500)]
Convert the geometric input functions to report errors softly.

Convert box_in, circle_in, line_in, lseg_in, path_in, point_in,
and poly_in to the new style.

line_in still throws hard errors for overflows/underflows that can occur
when the input is specified as two points rather than in the canonical
"Ax + By + C = 0" style.  I'm not too concerned about that: it won't be
reached in normal dump/restore cases, and it's fairly debatable that
such conversion should ever have been made part of a type input function
in the first place.  But in any case, I don't want to extend the soft
error conventions into float.h without more discussion than this patch
has had.

Amul Sul, minor mods by me

Discussion: https://postgr.es/m/CAAJ_b97KeDWUdpTKGOaFYPv0OicjOu6EW+QYWj-Ywrgj_aEy1g@mail.gmail.com

2 years agoConvert a few more datatype input functions to report errors softly.
Tom Lane [Wed, 14 Dec 2022 18:22:08 +0000 (13:22 -0500)]
Convert a few more datatype input functions to report errors softly.

Convert bit_in, varbit_in, inet_in, cidr_in, macaddr_in, and
macaddr8_in to the new style.

Amul Sul, minor mods by me

Discussion: https://postgr.es/m/CAAJ_b97KeDWUdpTKGOaFYPv0OicjOu6EW+QYWj-Ywrgj_aEy1g@mail.gmail.com

2 years agoRearrange some static assertions for consistency
Peter Eisentraut [Wed, 14 Dec 2022 15:08:13 +0000 (16:08 +0100)]
Rearrange some static assertions for consistency

Put lengthof first.

Reported-by: Peter Smith <smithpb2250@gmail.com>
Discussion: https://www.postgresql.org/message-id/CAHut+PsUDMySVRuRc=h+P5N3+=TGvj4W_mi32XXg9dt4o-BXbA@mail.gmail.com

2 years agoNon-decimal integer literals
Peter Eisentraut [Wed, 14 Dec 2022 04:40:38 +0000 (05:40 +0100)]
Non-decimal integer literals

Add support for hexadecimal, octal, and binary integer literals:

    0x42F
    0o273
    0b100101

per SQL:202x draft.

This adds support in the lexer as well as in the integer type input
functions.

Reviewed-by: John Naylor <john.naylor@enterprisedb.com>
Reviewed-by: Zhihong Yu <zyu@yugabyte.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/b239564c-cad0-b23e-c57e-166d883cb97d@enterprisedb.com

2 years agoAdd grantable MAINTAIN privilege and pg_maintain role.
Jeff Davis [Wed, 14 Dec 2022 01:33:28 +0000 (17:33 -0800)]
Add grantable MAINTAIN privilege and pg_maintain role.

Allows VACUUM, ANALYZE, REINDEX, REFRESH MATERIALIZED VIEW, CLUSTER,
and LOCK TABLE.

Effectively reverts 4441fc704d. Instead of creating separate
privileges for VACUUM, ANALYZE, and other maintenance commands, group
them together under a single MAINTAIN privilege.

Author: Nathan Bossart
Discussion: https://postgr.es/m/20221212210136.GA449764@nathanxps13
Discussion: https://postgr.es/m/45224.1670476523@sss.pgh.pa.us

2 years agoRemove SHA256_HMAC_B from scram-common.h
Michael Paquier [Wed, 14 Dec 2022 00:51:19 +0000 (09:51 +0900)]
Remove SHA256_HMAC_B from scram-common.h

This referred to the size of the buffers for k_ipad and k_opad in HMAC
computations.  This is unused since e6bdfd9, where SCRAM has switched to
the cryptohash routines for its HMAC calculations rather than its own
maths.

Reviewed-by: Jacob Champion
Discussion: https://postgr.es/m/Y5gGMjXhyp0oK0mH@paquier.xyz

2 years agoRethink handling of [Prevent|Is]InTransactionBlock in pipeline mode.
Tom Lane [Tue, 13 Dec 2022 19:23:58 +0000 (14:23 -0500)]
Rethink handling of [Prevent|Is]InTransactionBlock in pipeline mode.

Commits f92944137 et al. made IsInTransactionBlock() set the
XACT_FLAGS_NEEDIMMEDIATECOMMIT flag before returning "false",
on the grounds that that kept its API promises equivalent to those of
PreventInTransactionBlock().  This turns out to be a bad idea though,
because it allows an ANALYZE in a pipelined series of commands to
cause an immediate commit, which is unexpected.

Furthermore, if we return "false" then we have another issue,
which is that ANALYZE will decide it's allowed to do internal
commit-and-start-transaction sequences, thus possibly unexpectedly
committing the effects of previous commands in the pipeline.

To fix the latter situation, invent another transaction state flag
XACT_FLAGS_PIPELINING, which explicitly records the fact that we
have executed some extended-protocol command and not yet seen a
commit for it.  Then, require that flag to not be set before allowing
InTransactionBlock() to return "false".

Having done that, we can remove its setting of NEEDIMMEDIATECOMMIT
without fear of causing problems.  This means that the API guarantees
of IsInTransactionBlock now diverge from PreventInTransactionBlock,
which is mildly annoying, but it seems OK given the very limited usage
of IsInTransactionBlock.  (In any case, a caller preferring the old
behavior could always set NEEDIMMEDIATECOMMIT for itself.)

For consistency also require XACT_FLAGS_PIPELINING to not be set
in PreventInTransactionBlock.  This too is meant to prevent commands
such as CREATE DATABASE from silently committing previous commands
in a pipeline.

Per report from Peter Eisentraut.  As before, back-patch to all
supported branches (which sadly no longer includes v10).

Discussion: https://postgr.es/m/65a899dd-aebc-f667-1d0a-abb89ff3abf8@enterprisedb.com

2 years agobasebackup_to_shell: Add some const qualifiers for consistency
Peter Eisentraut [Tue, 13 Dec 2022 09:39:44 +0000 (10:39 +0100)]
basebackup_to_shell: Add some const qualifiers for consistency

2 years agoRefactor ExecGrant_*() functions
Peter Eisentraut [Tue, 13 Dec 2022 06:43:42 +0000 (07:43 +0100)]
Refactor ExecGrant_*() functions

Instead of half a dozen of mostly-duplicate ExecGrant_Foo() functions,
write one common function ExecGrant_generic() that can handle most of
them.  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: Andres Freund <andres@anarazel.de>
Reviewed-by: Antonin Houska <ah@cybertec.at>
Discussion: https://www.postgresql.org/message-id/flat/22c7e802-4e7d-8d87-8b71-cba95e6f4bcf%40enterprisedb.com

2 years agoFix jsonb subscripting to cope with toasted subscript values.
Tom Lane [Mon, 12 Dec 2022 21:17:49 +0000 (16:17 -0500)]
Fix jsonb subscripting to cope with toasted subscript values.

jsonb_get_element() was incautious enough to use VARDATA() and
VARSIZE() directly on an arbitrary text Datum.  That of course
fails if the Datum is short-header, compressed, or out-of-line.
The typical result would be failing to match any element of a
jsonb object, though matching the wrong one seems possible as well.

setPathObject() was slightly brighter, in that it used VARDATA_ANY
and VARSIZE_ANY_EXHDR, but that only kept it out of trouble for
short-header Datums.  push_path() had the same issue.  This could
result in faulty subscripted insertions, though keys long enough to
cause a problem are likely rare in the wild.

Having seen these, I looked around for unsafe usages in the rest
of the adt/json* files.  There are a couple of places where it's not
immediately obvious that the Datum can't be compressed or out-of-line,
so I added pg_detoast_datum_packed() to cope if it is.  Also, remove
some other usages of VARDATA/VARSIZE on Datums we just extracted from
a text array.  Those aren't actively broken, but they will become so
if we ever start allowing short-header array elements, which does not
seem like a terribly unreasonable thing to do.  In any case they are
not great coding examples, and they could also do with comments
pointing out that we're assuming we don't need pg_detoast_datum_packed.

Per report from exe-dealer@yandex.ru.  Patch by me, but thanks to
David Johnston for initial investigation.  Back-patch to v14 where
jsonb subscripting was introduced.

Discussion: https://postgr.es/m/205321670615953@mail.yandex.ru

2 years agoDisable clang 16's -Wcast-function-type-strict.
Thomas Munro [Mon, 12 Dec 2022 21:03:28 +0000 (10:03 +1300)]
Disable clang 16's -Wcast-function-type-strict.

Clang 16 is still in development, but seawasp reveals that it has
started warning about many of our casts of function pointers (those
introduced by commit 1c27d16e, and some older ones).  Disable the new
warning for now, since otherwise buildfarm animal seawasp fails, and we
have no current plans to change our strategy for these callback function
types.

May be back-patched with other Clang/LLVM 16 changes around release
time.

Discussion: https://postgr.es/m/CA%2BhUKGJvX%2BL3aMN84ksT-cGy08VHErRNip3nV-WmTx7f6Pqhyw%40mail.gmail.com

2 years agoBetter document logical replication parameters
Alvaro Herrera [Mon, 12 Dec 2022 19:18:56 +0000 (20:18 +0100)]
Better document logical replication parameters

Add some cross-links between chapter "20. Server Parameters" and
"31. Logical Replication" regarding the available configuration
parameters, for easier navigation; and some more explanatory text too.

I (Álvaro) chose to duplicate max_replication_slots in Chapter 20,
because it has completely different meanings at each side of the
replication link.

Author: Peter Smith <smithpb2250@gmail.com>
Reviewed-by: vignesh C <vignesh21@gmail.com>
Reviewed-by: samay sharma <smilingsamay@gmail.com>
Discussion: https://postgr.es/m/CAHut+PsESqpy7w3Y6cX98c255ZuCjvipkhKjy6hZBjOv4E6iJA@mail.gmail.com

2 years agoRemove extra space from dumped ALTER DEFAULT PRIVILEGES.
Jeff Davis [Mon, 12 Dec 2022 17:25:50 +0000 (09:25 -0800)]
Remove extra space from dumped ALTER DEFAULT PRIVILEGES.

Author: Nathan Bossart
Discussion: https://postgr.es/m/20221206232744.GA3560301@nathanxps13

2 years agoFix failure to advance content pointer in sendFileWithContent.
Robert Haas [Mon, 12 Dec 2022 15:17:02 +0000 (10:17 -0500)]
Fix failure to advance content pointer in sendFileWithContent.

If sendFileWithContent were used to send a file larger than the
bbsink buffer size, this would result in corruption. The only
files that are sent via sendFileWithContent are the backup label
file, the tablespace map file, and .done files for WAL segments
included in the backup. Of these, it seems that only the
tablespace_map file can become large enough to cause a problem,
and then only if you have a lot of tablespaces. If you do have
that situation, you might end up with a corrupted
tablespace_map file, which would be bad.

My commit bef47ff85df18bf4a3a9b13bd2a54820e27f3614 introduced
this problem.

Report and patch by Antonin Houska.

Discussion: http://postgr.es/m/15764.1670528645@antos

2 years agoOrder getopt arguments
Peter Eisentraut [Mon, 12 Dec 2022 13:33:41 +0000 (14:33 +0100)]
Order getopt arguments

Order the letters in the arguments of getopt() and getopt_long(), as
well as in the subsequent switch statements.  In most cases, I used
alphabetical with lower case first.  In a few cases, existing
different orders (e.g., upper case first) was kept to reduce the diff
size.

Discussion: https://www.postgresql.org/message-id/flat/3efd0fe8-351b-f836-9122-886002602357%40enterprisedb.com

2 years agoGet rid of recursion-marker values in enum AlterTableType
Alvaro Herrera [Mon, 12 Dec 2022 10:13:26 +0000 (11:13 +0100)]
Get rid of recursion-marker values in enum AlterTableType

During ALTER TABLE execution, when prep-time handling of subcommands of
certain types determine that execution-time handling requires recursion,
they signal this by changing the subcommand type to a special value.
This can be done in a simpler way by using a separate flag introduced by
commit ec0925c22a3d, so do that.

Catversion bumped.  It's not clear to me that ALTER TABLE subcommands
are stored anywhere in catalogs (CREATE FUNCTION rejects it in BEGIN
ATOMIC function bodies), but we do have both write and read support for
them, so be safe.

Discussion: https://postgr.es/m/20220929090033.zxuaezcdwh2fgfjb@alvherre.pgsql

2 years agoAdd support for GRANT SET in psql tab completion
Michael Paquier [Mon, 12 Dec 2022 07:47:24 +0000 (16:47 +0900)]
Add support for GRANT SET in psql tab completion

3d14e17 has added support for this query but psql was not able to
complete it.  Spotted while working on a different patch in the same
area.

Reviewed-by: Robert Haas
Discussion: https://postgr.es/m/Y3hw7yvG0VwpC1jq@paquier.xyz

2 years agoRemove direct call to GetNewObjectId() for pg_auth_members.oid
Michael Paquier [Mon, 12 Dec 2022 00:01:39 +0000 (09:01 +0900)]
Remove direct call to GetNewObjectId() for pg_auth_members.oid

This routine should not be called directly as mentioned at its top, so
replace it by GetNewOidWithIndex().  Issue introduced by 6566133 when
pg_auth_members.oid got added, so no backpatch is needed.

Author: Maciek Sakrejda
Discussion: https://postgr.es/m/CAOtHd0Ckbih7Ur7XeVyLAJ26VZOfTNcq9qV403bNF4uTGtAN+Q@mail.gmail.com

2 years agoConvert domain_in to report errors softly.
Tom Lane [Sun, 11 Dec 2022 17:56:54 +0000 (12:56 -0500)]
Convert domain_in to report errors softly.

This is straightforward as far as it goes.  However, it does not
attempt to trap errors occurring during the execution of domain
CHECK constraints.  Since those are general user-defined
expressions, the only way to do that would involve starting up a
subtransaction for each check.  Of course the entire point of
the soft-errors feature is to not need subtransactions, so that
would be self-defeating.  For now, we'll rely on the assumption
that domain checks are written to avoid throwing errors.

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

2 years agoConvert json_in and jsonb_in to report errors softly.
Tom Lane [Sun, 11 Dec 2022 16:28:15 +0000 (11:28 -0500)]
Convert json_in and jsonb_in to report errors softly.

This requires a bit of further infrastructure-extension to allow
trapping errors reported by numeric_in and pg_unicode_to_server,
but otherwise it's pretty straightforward.

In the case of jsonb_in, we are only capturing errors reported
during the initial "parse" phase.  The value-construction phase
(JsonbValueToJsonb) can also throw errors if assorted implementation
limits are exceeded.  We should improve that, but it seems like a
separable project.

Andrew Dunstan and Tom Lane

Discussion: https://postgr.es/m/3bac9841-fe07-713d-fa42-606c225567d6@dunslane.net

2 years agoChange JsonSemAction to allow non-throw error reporting.
Tom Lane [Sun, 11 Dec 2022 15:39:05 +0000 (10:39 -0500)]
Change JsonSemAction to allow non-throw error reporting.

Formerly, semantic action functions for the JSON parser returned void,
so that there was no way for them to affect the parser's behavior.
That means in particular that they can't force an error exit except by
longjmp'ing.  That won't do in the context of our project to make input
functions return errors softly.  Hence, change them to return the same
JsonParseErrorType enum value as the parser itself uses.  If an action
function returns anything besides JSON_SUCCESS, the parse is abandoned
and that error code is returned.

Action functions can thus easily return the same error conditions that
the parser already knows about.  As an escape hatch for expansion, also
invent a code JSON_SEM_ACTION_FAILED that the core parser does not know
the exact meaning of.  When returning this code, an action function
must use some out-of-band mechanism for reporting the error details.

This commit simply makes the API change and causes all the existing
action functions to return JSON_SUCCESS, so that there is no actual
change in behavior here.  This is long enough and boring enough that
it seemed best to commit it separately from the changes that make
real use of the new mechanism.

In passing, remove a duplicate assignment of
transform_string_values_scalar.

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

2 years agoStandardize error reports in unimplemented I/O functions.
Tom Lane [Sat, 10 Dec 2022 23:26:43 +0000 (18:26 -0500)]
Standardize error reports in unimplemented I/O functions.

We chose a specific wording of the not-implemented errors for
pseudotype I/O functions and other cases where there's little
value in implementing input and/or output.  gtsvectorin never
got that memo though, nor did most of contrib.  Make these all
fall in line, mostly because I'm a neatnik but also to remove
unnecessary translatable strings.

gbtreekey_in needs a bit of extra love since it supports
multiple SQL types.  Sadly, gbtreekey_out doesn't have the
ability to do that, but I think it's unreachable anyway.

Noted while surveying datatype input functions to see what we
have left to fix.

2 years agoUse the macro, not handwritten code, to construct anymultirange_in().
Tom Lane [Sat, 10 Dec 2022 22:22:16 +0000 (17:22 -0500)]
Use the macro, not handwritten code, to construct anymultirange_in().

Apparently anymultirange_in was written before we converted all
these pseudotype input functions to use a common macro, and it didn't
get fixed before committing.  Sloppy merging probably explains its
unintuitive ordering, too, so rearrange.

Noted while surveying datatype input functions to see what we
have left to fix.  I'm inclined to leave the pseudotypes as
throwing hard errors, because it's difficult to see a reason why
anyone would need something else.  But in any case, if we want
to change that, we shouldn't have to change multiple copies of
the code.

2 years agoAdd subquery pullup handling for WindowClause runCondition
David Rowley [Sat, 10 Dec 2022 06:27:20 +0000 (19:27 +1300)]
Add subquery pullup handling for WindowClause runCondition

9d9c02ccd added code to allow WindowAgg to take some shortcuts when a
monotonic WindowFunc reached some value that it could never come back
from due to the function's monotonic nature.  That commit added a
runCondition field to WindowClause to store the condition which, when it
becomes false we can start taking shortcuts in nodeWindowAgg.c.

Here we fix an issue where subquery pullups didn't properly update the
runCondition to update the Vars to properly reference the new query level.

Here we also add a missing call to preprocess_expression() for the
WindowClause's runCondtion.  The WindowFuncs in the targetlist will have
had this process done, so we must also do it for the WindowFuncs in the
runCondition so that they can be correctly found in the targetlist
during setrefs.c

Bug: #17709
Reported-by: Alexey Makhmutov
Author: Richard Guo, David Rowley
Discussion: https://postgr.es/m/17709-4f557160e3e8ee9a@postgresql.org
Backpatch-through: 15, where 9d9c02ccd was introduced

2 years agoFix macro definitions in pgstatfuncs.c
Michael Paquier [Sat, 10 Dec 2022 04:28:02 +0000 (13:28 +0900)]
Fix macro definitions in pgstatfuncs.c

Buildfarm member wrasse has been complaining about empty declarations
as an effect of 8018ffb and 83a1a1b due to extra semicolons.

While on it, remove also the last backslash of the macros definitions,
causing more lines to be eaten in it than necessary, per comment from
Tom Lane.

Reported-by: Tom Lane, and buildfarm member wrasse
Author: Nathan Bossart, Michael Paquier
Discussion: https://postgr.es/m/1188769.1670640236@sss.pgh.pa.us

2 years agoRestructure soft-error handling in formatting.c.
Tom Lane [Sat, 10 Dec 2022 01:15:56 +0000 (20:15 -0500)]
Restructure soft-error handling in formatting.c.

Replace the error trapping scheme introduced in 5bc450629 with our
shiny new errsave/ereturn mechanism.  This doesn't have any real
functional impact (although I think that the new coding is able
to report a few more errors softly than v15 did).  And I doubt
there's any measurable performance difference either.  But this
gets rid of an ad-hoc, one-of-a-kind design in favor of a mechanism
that will be widely used going forward, so it should be a net win
for code readability.

Discussion: https://postgr.es/m/3bbbb0df-7382-bf87-9737-340ba096e034@postgrespro.ru

2 years agoConvert datetime input functions to use "soft" error reporting.
Tom Lane [Fri, 9 Dec 2022 21:07:49 +0000 (16:07 -0500)]
Convert datetime input functions to use "soft" error reporting.

This patch converts the input functions for date, time, timetz,
timestamp, timestamptz, and interval to the new soft-error style.
There's some related stuff in formatting.c that remains to be
cleaned up, but that seems like a separable project.

Discussion: https://postgr.es/m/3bbbb0df-7382-bf87-9737-340ba096e034@postgrespro.ru

2 years agoAllow DateTimeParseError to handle bad-timezone error messages.
Tom Lane [Fri, 9 Dec 2022 18:30:43 +0000 (13:30 -0500)]
Allow DateTimeParseError to handle bad-timezone error messages.

Pay down some ancient technical debt (dating to commit 022fd9966):
fix a couple of places in datetime parsing that were throwing
ereport's immediately instead of returning a DTERR code that could be
interpreted by DateTimeParseError.  The reason for that was that there
was no mechanism for passing any auxiliary data (such as a zone name)
to DateTimeParseError, and these errors seemed to really need it.
Up to now it didn't matter that much just where the error got thrown,
but now we'd like to have a hard policy that datetime parse errors
get thrown from just the one place.

Hence, invent a "DateTimeErrorExtra" struct that can be used to
carry any extra values needed for specific DTERR codes.  Perhaps
in the future somebody will be motivated to use this to improve
the specificity of other DateTimeParseError messages, but for now
just deal with the timezone-error cases.

This is on the way to making the datetime input functions report
parse errors softly; but it's really an independent change, so
commit separately.

Discussion: https://postgr.es/m/3bbbb0df-7382-bf87-9737-340ba096e034@postgrespro.ru

2 years agomeson: Improve PG_VERSION_STR generation
Andres Freund [Fri, 9 Dec 2022 16:53:23 +0000 (08:53 -0800)]
meson: Improve PG_VERSION_STR generation

Previously the host operating system and 32/64 bit were not included and the
build machine's cpu was used, which is potentially wrong for cross builds.

Author: Juan José Santamaría Flecha <juanjo.santamaria@gmail.com>
Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CAC+AXB16gwYhKCdS+t0pk3U7kKtpVj5L-ynmhK3Gbea330At3w@mail.gmail.com

2 years agoConst-ify a couple of datetime parsing subroutines.
Tom Lane [Fri, 9 Dec 2022 15:43:45 +0000 (10:43 -0500)]
Const-ify a couple of datetime parsing subroutines.

More could be done in this line, but I just grabbed some low-hanging
fruit.  Principal objective was to remove the need for several ugly
unconstify() usages in formatting.c.

2 years agoConvert a few datatype input functions to use "soft" error reporting.
Tom Lane [Fri, 9 Dec 2022 15:14:53 +0000 (10:14 -0500)]
Convert a few datatype input functions to use "soft" error reporting.

This patch converts the input functions for bool, int2, int4, int8,
float4, float8, numeric, and contrib/cube to the new soft-error style.
array_in and record_in are also converted.  There's lots more to do,
but this is enough to provide proof-of-concept that the soft-error
API is usable, as well as reference examples for how to convert
input functions.

This patch is mostly by me, but it owes very substantial debt to
earlier work by Nikita Glukhov, Andrew Dunstan, and Amul Sul.
Thanks to Andres Freund for review.

Discussion: https://postgr.es/m/3bbbb0df-7382-bf87-9737-340ba096e034@postgrespro.ru

2 years agoAdd test scaffolding for soft error reporting from input functions.
Tom Lane [Fri, 9 Dec 2022 15:08:44 +0000 (10:08 -0500)]
Add test scaffolding for soft error reporting from input functions.

pg_input_is_valid() returns boolean, while pg_input_error_message()
returns the primary error message if the input is bad, or NULL
if the input is OK.  The main reason for having two functions is
so that we can test both the details-wanted and the no-details-wanted
code paths.

Although these are primarily designed with testing in mind,
it could well be that they'll be useful to end users as well.

This patch is mostly by me, but it owes very substantial debt to
earlier work by Nikita Glukhov, Andrew Dunstan, and Amul Sul.
Thanks to Andres Freund for review.

Discussion: https://postgr.es/m/3bbbb0df-7382-bf87-9737-340ba096e034@postgrespro.ru

2 years agoCreate infrastructure for "soft" error reporting.
Tom Lane [Fri, 9 Dec 2022 14:58:38 +0000 (09:58 -0500)]
Create infrastructure for "soft" error reporting.

Postgres' standard mechanism for reporting errors (ereport() or elog())
is used for all sorts of error conditions.  This means that throwing
an exception via ereport(ERROR) requires an expensive transaction or
subtransaction abort and cleanup, since the exception catcher dare not
make many assumptions about what has gone wrong.  There are situations
where we would rather have a lighter-weight mechanism for dealing
with errors that are known to be safe to recover from without a full
transaction cleanup.  This commit creates infrastructure to let us
adapt existing error-reporting code for that purpose.  See the
included documentation changes for details.  Follow-on commits will
provide test code and usage examples.

The near-term plan is to convert most if not all datatype input
functions to report invalid input "softly".  This will enable
implementing some SQL/JSON features cleanly and without the cost
of subtransactions, and it will also allow creating COPY options
to deal with bad input without cancelling the whole COPY.

This patch is mostly by me, but it owes very substantial debt to
earlier work by Nikita Glukhov, Andrew Dunstan, and Amul Sul.
Thanks also to Andres Freund for review.

Discussion: https://postgr.es/m/3bbbb0df-7382-bf87-9737-340ba096e034@postgrespro.ru

2 years agoFix invalid role names introduced in 096dd80f3c
Alexander Korotkov [Fri, 9 Dec 2022 10:53:32 +0000 (13:53 +0300)]
Fix invalid role names introduced in 096dd80f3c

096dd80f3c added new regression tests dealing with roles.  By oversight, role
names didn't start with regress_ prefix.  This commit fixes that.

2 years agoAdd USER SET parameter values for pg_db_role_setting
Alexander Korotkov [Fri, 9 Dec 2022 10:12:20 +0000 (13:12 +0300)]
Add USER SET parameter values for pg_db_role_setting

The USER SET flag specifies that the variable should be set on behalf of an
ordinary role.  That lets ordinary roles set placeholder variables, which
permission requirements are not known yet.  Such a value wouldn't be used if
the variable finally appear to require superuser privileges.

The new flags are stored in the pg_db_role_setting.setuser array.  Catversion
is bumped.

This commit is inspired by the previous work by Steve Chavez.

Discussion: https://postgr.es/m/CAPpHfdsLd6E--epnGqXENqLP6dLwuNZrPMcNYb3wJ87WR7UBOQ%40mail.gmail.com
Author: Alexander Korotkov, Steve Chavez
Reviewed-by: Pavel Borisov, Steve Chavez
2 years agoUpdate MERGE docs to mention that ONLY is supported.
Dean Rasheed [Fri, 9 Dec 2022 10:00:01 +0000 (10:00 +0000)]
Update MERGE docs to mention that ONLY is supported.

Commit 7103ebb7aa added support for MERGE, which included support for
inheritance hierarchies, but didn't document the fact that ONLY could
be specified before the source and/or target tables to exclude tables
inheriting from the tables specified.

Update merge.sgml to mention this, and while at it, add some
regression tests to cover it.

Dean Rasheed, reviewed by Nathan Bossart.

Backpatch to 15, where MERGE was added.

Discussion: https://postgr.es/m/CAEZATCU0XM-bJCvpJuVRU3UYNRqEBS6g4-zH%3Dj9Ye0caX8F6uQ%40mail.gmail.com

2 years agoRemove unnecessary casts
Peter Eisentraut [Thu, 8 Dec 2022 07:51:38 +0000 (08:51 +0100)]
Remove unnecessary casts

Some code carefully cast all data buffer arguments for BufFileWrite()
and BufFileRead() to void *, even though the arguments are already
void * (and AFAICT were never anything else).  Remove this unnecessary
clutter.

Discussion: https://www.postgresql.org/message-id/flat/11dda853-bb5b-59ba-a746-e168b1ce4bdb%40enterprisedb.com

2 years agoUpdate types in File API
Peter Eisentraut [Thu, 8 Dec 2022 07:51:38 +0000 (08:51 +0100)]
Update types in File API

Make the argument types of the File API match stdio better:

- Change the data buffer to void *, from char *.
- Change FileWrite() data buffer to const on top of that.
- Change amounts to size_t, from int.

In passing, change the FilePrefetch() amount argument from int to
off_t, to match the underlying posix_fadvise().

Discussion: https://www.postgresql.org/message-id/flat/11dda853-bb5b-59ba-a746-e168b1ce4bdb%40enterprisedb.com

2 years agoRemove new structure member from ResultRelInfo.
Etsuro Fujita [Thu, 8 Dec 2022 07:15:00 +0000 (16:15 +0900)]
Remove new structure member from ResultRelInfo.

In commit ffbb7e65a, I added a ModifyTableState member to ResultRelInfo
to save the owning ModifyTableState for use by nodeModifyTable.c when
performing batch inserts, but as pointed out by Tom Lane, that changed
the array stride of es_result_relations, and that would break any
previously-compiled extension code that accesses that array.  Fix by
removing that member from ResultRelInfo and instead adding a List member
at the end of EState to save such ModifyTableStates.

Per report from Tom Lane.  Back-patch to v14, like the previous commit;
I chose to apply the patch to HEAD as well, to make back-patching easy.

Discussion: http://postgr.es/m/4065383.1669395453%40sss.pgh.pa.us

2 years agoAdd option to specify segment size in blocks
Andres Freund [Thu, 8 Dec 2022 03:32:59 +0000 (19:32 -0800)]
Add option to specify segment size in blocks

The tests don't have much coverage of segment related code, as we don't create
large enough tables. To make it easier to test these paths, add a new option
specifying the segment size in blocks.

Set the new option to 6 blocks in one of the CI tasks. Smaller numbers
currently fail one of the tests, for understandable reasons.

While at it, fix some segment size related issues in the meson build.

Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20221107171355.c23fzwanfzq2pmgt@awork3.anarazel.de

2 years agoAvoid unnecessary streaming of transactions during logical replication.
Amit Kapila [Thu, 8 Dec 2022 00:35:09 +0000 (06:05 +0530)]
Avoid unnecessary streaming of transactions during logical replication.

After restart, we don't perform streaming of an in-progress transaction if
it was previously decoded and confirmed by the client. To achieve that we
were comparing the END location of the WAL record being decoded with the
WAL location we have already decoded and confirmed by the client. While
decoding the commit record, to decide whether to process and send the
complete transaction, we compare its START location with the WAL location
we have already decoded and confirmed by the client. Now, if we need to
queue some change in the transaction while decoding the commit record
(e.g. snapshot), it is possible that we decide to stream the transaction
but later commit processing decides to skip it. In such a case, we would
needlessly send the changes and later when we decide to skip it, we will
send stream abort.

We also sometimes decide to stream the changes when we actually just need
to process them locally like a change for invalidations. This will lead us
to send empty streams. To avoid this, while queuing each change for
decoding, we remember whether the transaction has any change that actually
needs to be sent downstream and use that information later to decide
whether to stream the transaction or not.

Note, we can't avoid all cases where we have to send empty streams like
the case where the plugin later decides that the change is not
publishable. However, we will no longer need to send stream_abort when we
skip sending a particular transaction.

Author: Dilip Kumar
Reviewed-by: Hou Zhijie, Ashutosh Bapat, Shi yu, Amit Kapila
Discussion: https://postgr.es/m/CAFiTN-tHK=7LzfrPs8fbT2ksrOJGQbzywcgXst2bM9-rJJAAUg@mail.gmail.com

2 years agomeson: Add 'running' test setup, as a replacement for installcheck
Andres Freund [Wed, 7 Dec 2022 20:13:35 +0000 (12:13 -0800)]
meson: Add 'running' test setup, as a replacement for installcheck

To run all tests that support running against existing server:
$ meson test --setup running

To run just the main pg_regress tests against existing server:
$ meson test --setup running regress-running/regress

To ensure the 'running' setup continues to work, test it as part of the
freebsd CI task.

Discussion: https://postgr.es/m/CAH2-Wz=XDQcmLoo7RR_i6FKQdDmcyb9q5gStnfuuQXrOGhB2sQ@mail.gmail.com

2 years agoMinor code refactoring in elog.c (no functional change).
Tom Lane [Wed, 7 Dec 2022 19:39:25 +0000 (14:39 -0500)]
Minor code refactoring in elog.c (no functional change).

Combine some duplicated code stanzas by creating small functions.
Most of these duplications arose at a time when I wouldn't have
trusted C compilers to auto-inline small functions intelligently,
but they're probably poor practice now.  Similarly split out some
bits that aren't actually duplicative as the code stands, but would
become so after an upcoming patch to add another error-handling
code path.

Take the opportunity to add some lengthier comments about what
we're doing here, too.  Re-order one function that seemed not
very well-placed.

Patch by me, per suggestions from Andres Freund.

Discussion: https://postgr.es/m/3bbbb0df-7382-bf87-9737-340ba096e034@postgrespro.ru

2 years agoDoc: subdivide System Information Functions and Operators.
Tom Lane [Wed, 7 Dec 2022 18:56:48 +0000 (13:56 -0500)]
Doc: subdivide System Information Functions and Operators.

Provide <sect2> subdivisions in 9.26 System Information Functions and
Operators.  This is useful because it adds a mini-TOC at the top of
the page to aid jumping to portions of what's become quite a long
section.  Also, now that several of the subsections contain multiple
tables, it's hard to see the overall structure without headings.

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

2 years agoFix FK comment think-o
Peter Eisentraut [Wed, 7 Dec 2022 16:06:50 +0000 (17:06 +0100)]
Fix FK comment think-o

from commit d6f96ed94e7

Author: Paul Jungwirth <pj@illuminatedcomputing.com>
Reviewed-by: Ian Lawrence Barwick <barwick@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/6a7c7338-1aa2-4689-d171-0b0b294fdd84%40illuminatedcomputing.com

2 years agoUpdate outdated comment in ApplyRetrieveRule
Alvaro Herrera [Wed, 7 Dec 2022 11:35:59 +0000 (12:35 +0100)]
Update outdated comment in ApplyRetrieveRule

After a61b1f74823c.

Author: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/CA+HiwqGZm7hb2VAy8HGM22-fTDaQzqE6T=5GbAk=GkT9H0hJEg@mail.gmail.com

2 years agomeson: Add basic PGXS compatibility
Andres Freund [Wed, 7 Dec 2022 02:56:46 +0000 (18:56 -0800)]
meson: Add basic PGXS compatibility

Generate a Makefile.global that's complete enough for PGXS to work for some
extensions. It is likely that this compatibility layer will not suffice for
every extension and not all platforms - we can expand it over time.

This allows extensions to use a single buildsystem across all the supported
postgres versions. Once all supported PG versions support meson, we can remove
the compatibility layer.

Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Discussion: https://postgr.es/m/20221005200710.luvw5evhwf6clig6@awork3.anarazel.de

2 years agoautoconf: Move export_dynamic determination to configure
Andres Freund [Wed, 7 Dec 2022 02:55:28 +0000 (18:55 -0800)]
autoconf: Move export_dynamic determination to configure

Previously export_dynamic was set in src/makefiles/Makefile.$port. For solaris
this required exporting with_gnu_ld. The determination of with_gnu_ld would be
nontrivial to copy for meson PGXS compatibility.  It's also nice to delete
libtool.m4.

This uses -Wl,--export-dynamic on all platforms, previously all platforms but
FreeBSD used -Wl,-E. The likelihood of a name conflict seems lower with the
longer spelling.

Discussion: https://postgr.es/m/20221005200710.luvw5evhwf6clig6@awork3.anarazel.de

2 years agoGenerate pg_stat_get*() functions for databases using macros
Michael Paquier [Wed, 7 Dec 2022 00:11:48 +0000 (09:11 +0900)]
Generate pg_stat_get*() functions for databases using macros

The same code pattern is repeated 21 times for int64 counters (0 for
missing entry) and 5 times for doubles (0 for missing entry) on database
entries.  This code is switched to use macros for the basic code
instead, shaving a few hundred lines of originally-duplicated code
patterns.  The function names remain the same, but some fields of
PgStat_StatDBEntry have to be renamed to cope with the new style.

This is in the same spirit as 83a1a1b.

Author: Michael Paquier
Reviewed-by: Nathan Bossart, Bertrand Drouvot
Discussion: https://postgr.es/m/Y46stlxQ2LQE20Na@paquier.xyz

2 years agomeson: Basic cygwin support
Andres Freund [Tue, 6 Dec 2022 19:25:54 +0000 (11:25 -0800)]
meson: Basic cygwin support

There likely are further issues, but as evidenced by the CI task proposed by
Justin in the referenced thread, this suffices to build and run basic tests in
cygwin (some fixes for the test infrastructure are needed, but that's
independent of the meson aspect).

Author: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20221021034040.GT16921@telsasoft.com

2 years agoRework query relation permission checking
Alvaro Herrera [Tue, 6 Dec 2022 15:09:24 +0000 (16:09 +0100)]
Rework query relation permission checking

Currently, information about the permissions to be checked on relations
mentioned in a query is stored in their range table entries.  So the
executor must scan the entire range table looking for relations that
need to have permissions checked.  This can make the permission checking
part of the executor initialization needlessly expensive when many
inheritance children are present in the range range.  While the
permissions need not be checked on the individual child relations, the
executor still must visit every range table entry to filter them out.

This commit moves the permission checking information out of the range
table entries into a new plan node called RTEPermissionInfo.  Every
top-level (inheritance "root") RTE_RELATION entry in the range table
gets one and a list of those is maintained alongside the range table.
This new list is initialized by the parser when initializing the range
table.  The rewriter can add more entries to it as rules/views are
expanded.  Finally, the planner combines the lists of the individual
subqueries into one flat list that is passed to the executor for
checking.

To make it quick to find the RTEPermissionInfo entry belonging to a
given relation, RangeTblEntry gets a new Index field 'perminfoindex'
that stores the corresponding RTEPermissionInfo's index in the query's
list of the latter.

ExecutorCheckPerms_hook has gained another List * argument; the
signature is now:
typedef bool (*ExecutorCheckPerms_hook_type) (List *rangeTable,
      List *rtePermInfos,
      bool ereport_on_violation);
The first argument is no longer used by any in-core uses of the hook,
but we leave it in place because there may be other implementations that
do.  Implementations should likely scan the rtePermInfos list to
determine which operations to allow or deny.

Author: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/CA+HiwqGjJDmUhDSfv-U2qhKJjt9ST7Xh9JXC_irsAQ1TAUsJYg@mail.gmail.com

2 years agoFix typo
Alvaro Herrera [Mon, 5 Dec 2022 18:03:35 +0000 (19:03 +0100)]
Fix typo

2 years agoFix 32-bit build dangling pointer issue in WindowAgg
David Rowley [Tue, 6 Dec 2022 11:09:36 +0000 (00:09 +1300)]
Fix 32-bit build dangling pointer issue in WindowAgg

9d9c02ccd added window "run conditions", which allows the evaluation of
monotonic window functions to be skipped when the run condition is no
longer true.  Prior to this commit, once the run condition was no longer
true and we stopped evaluating the window functions, we simply just left
the ecxt_aggvalues[] and ecxt_aggnulls[] arrays alone to store whatever
value was stored there the last time the window function was evaluated.
Leaving a stale value in there isn't really a problem on 64-bit builds as
all of the window functions which we recognize as monotonic all return
int8, which is passed by value on 64-bit builds.  However, on 32-bit
builds, this was a problem as the value stored in the ecxt_values[]
element would be a by-ref value and it would be pointing to some memory
which would get reset once the tuple context is destroyed.  Since the
WindowAgg node will output these values in the resulting tupleslot, this
could be problematic for the top-level WindowAgg node which must look at
these values to filter out the rows that don't meet its filter condition.

Here we fix this by just zeroing the ecxt_aggvalues[] and setting the
ecxt_aggnulls[] array to true when the run condition first becomes false.
This results in the WindowAgg's output having NULLs for the WindowFunc's
columns rather than the stale or pointer pointing to possibly freed
memory.  These tuples with the NULLs can only make it as far as the
top-level WindowAgg node before they're filtered out.  To ensure that
these tuples *are* always filtered out, we now insist that OpExprs making
up the run condition are strict OpExprs.  Currently, all the window
functions which the planner recognizes as monotonic return INT8 and the
operator which is used for the run condition must be a member of a btree
opclass.  In reality, these restrictions exclude nothing that's built-in
to Postgres and are unlikely to exclude anyone's custom operators due to
the requirement that the operator is part of a btree opclass.  It would be
unusual if those were not strict.

Reported-by: Sergey Shinderuk, using valgrind
Reviewed-by: Richard Guo, Sergey Shinderuk
Discussion: https://postgr.es/m/29184c50-429a-ebd7-f1fb-0589c6723a35@postgrespro.ru
Backpatch-through: 15, where 9d9c02ccd was added

2 years agoGenerate pg_stat_get*() functions for tables using macros
Michael Paquier [Tue, 6 Dec 2022 01:46:35 +0000 (10:46 +0900)]
Generate pg_stat_get*() functions for tables using macros

The same code pattern is repeated 17 times for int64 counters (0 for
missing entry) and 5 times for timestamps (NULL for missing entry) on
table entries.  This code is switched to use a macro for the basic code
instead, shaving a few hundred lines of originally-duplicated code.  The
function names remain the same, but some fields of PgStat_StatTabEntry
have to be renamed to cope with the new style.

Author: Bertrand Drouvot
Reviewed-by: Nathan Bossart
Discussion: https:/postgr.es/m/20221204173207.GA2669116@nathanxps13

2 years agoCheck the snapshot argument of index_beginscan and family
Alexander Korotkov [Tue, 6 Dec 2022 00:29:18 +0000 (03:29 +0300)]
Check the snapshot argument of index_beginscan and family

Passing a NULL snapshot (InvalidSnapshot) is going to work but only as long
as the index can't find any matching rows.  This can be confusing for
the extension authors, so add an explicit check for this argument.  The check
is implemented with Assert() in order to avoid overhead in release builds.

Reported-by: Sven Klemm
Discussion: https://postgr.es/m/CAJ7c6TPxitD4vbKyP-mpmC1XwyHdPPqvjLzm%2BVpB88h8LGgneQ%40mail.gmail.com
Author: Aleksander Alekseev
Reviewed-by: Pavel Borisov
2 years agoProvide test coverage in pg_dump for default behaviors with compression
Michael Paquier [Tue, 6 Dec 2022 00:20:13 +0000 (09:20 +0900)]
Provide test coverage in pg_dump for default behaviors with compression

By default, the contents generated by the custom and directory dump
formats are compressed.  However, with the existing test facility, the
restore program will succeed regardless of whether the dumped output was
compressed or not without checking if anything has been compressed.

This commit implements a portable way to check the contents of the
custom and directory dump formats:
- glob_patterns, that can be defined for each test as an array of
glob()-compilable strings, tracking the contents that should or should
not be compressed.  While this is useful to make sure that the table
data is compressed, this also checks that blobs.toc and toc.dat are
never compressed.
- command_like, to execute a command on a dump and check its generated
output.  This is used here in correlation with pg_restore -l to check if
the dumps have been compressed or not, depending on if the build
supports gzip, or not.

This hole in the tests has come up when working on 5e73a60, where
compression has to be applied by default, if available, for both dump
formats.

The idea of glob_patterns comes from me, and Georgios has come up with
the design for command_like.

Author: Georgios Kokolatos, Michael Paquier
Discussion: https://postgr.es/m/DQn4czCWR1rcbGPLL7p3LfEr5-kGmlySm-H05VgroINdikvhtS5r9EdI6b8D8sjnbKdJ09k-cxs2AqijBeHAWk9Q8gvEAxPRHuLRhwONcGc=@pm.me

2 years agoinitdb: Refactor PG_CMD_PUTS loops
Peter Eisentraut [Mon, 5 Dec 2022 22:28:59 +0000 (23:28 +0100)]
initdb: Refactor PG_CMD_PUTS loops

Keeping the SQL commands that initdb runs in string arrays before
feeding them to PG_CMD_PUTS() seems unnecessarily verbose and
inflexible.  In some cases, the array only has one member.  In other
cases, one might want to use PG_CMD_PRINTF() instead, to parametrize a
command, but that would require breaking up the loop or using
workarounds like replace_token().  Unwind all that; it's much simpler
that way.

Reviewed-by: John Naylor <john.naylor@enterprisedb.com>
Reviewed-by: Andrew Dunstan <andrew@dunslane.net>
Discussion: https://www.postgresql.org/message-id/flat/2c50823b-f453-bb97-e38b-34751c51dcdf%40enterprisedb.com

2 years agoFix Memoize to work with partitionwise joining.
Tom Lane [Mon, 5 Dec 2022 17:36:40 +0000 (12:36 -0500)]
Fix Memoize to work with partitionwise joining.

A couple of places weren't up to speed for this.  By sheer good
luck, we didn't fail but just selected a non-memoized join plan,
at least in the test case we have.  Nonetheless, it's a bug,
and I'm not quite sure that it couldn't have worse consequences
in other examples.  So back-patch to v14 where Memoize came in.

Richard Guo

Discussion: https://postgr.es/m/CAMbWs48GkNom272sfp0-WeD6_0HSR19BJ4H1c9ZKSfbVnJsvRg@mail.gmail.com

2 years agopg_dump: Remove "blob" terminology
Peter Eisentraut [Mon, 5 Dec 2022 07:52:11 +0000 (08:52 +0100)]
pg_dump: Remove "blob" terminology

For historical reasons, pg_dump refers to large objects as "BLOBs".
This term is not used anywhere else in PostgreSQL, and it also means
something different in the SQL standard and other SQL systems.

This patch renames internal functions, code comments, documentation,
etc. to use the "large object" or "LO" terminology instead.  There is
no functionality change, so the archive format still uses the name
"BLOB" for the archive entry.  Additional long command-line options
are added with the new naming.

Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://www.postgresql.org/message-id/flat/868a381f-4650-9460-1726-1ffd39a270b4%40enterprisedb.com

2 years agodoc: Add missing <varlistentry> markups for developer GUCs
Michael Paquier [Mon, 5 Dec 2022 02:23:08 +0000 (11:23 +0900)]
doc: Add missing <varlistentry> markups for developer GUCs

Missing such markups makes it impossible to create links back to these
GUCs, and all the other parameters have one already.

Author: Ian Lawrence Barwick
Discussion: https://postgr.es/m/CAB8KJ=jx=6dFB_EN3j0UkuvG3cPu5OmQiM-ZKRAz+fKvS+u8Ng@mail.gmail.com
Backpatch-through: 11

2 years agoAdd LSN location in some error messages related to WAL pages
Michael Paquier [Mon, 5 Dec 2022 00:28:29 +0000 (09:28 +0900)]
Add LSN location in some error messages related to WAL pages

The error messages reported during any failures while reading or
validating the header of a WAL currently includes only the offset of the
page but not the compiled LSN referring to the page, requiring an extra
step to compile it if looking at the surroundings with pg_waldump or
similar.  Adding this information costs a bit in translation, but also
eases debugging.

Author: Bharath Rupireddy
Reviewed-by: Álvaro Herrera, Kyotaro Horiguchi, Maxim Orlov, Michael
Paquier
Discussion: https://postgr.es/m/CALj2ACWV=FCddsxcGbVOA=cvPyMr75YCFbSQT6g4KDj=gcJK4g@mail.gmail.com

2 years agoFix thinko introduced in 6b423ec67
David Rowley [Sun, 4 Dec 2022 22:55:05 +0000 (11:55 +1300)]
Fix thinko introduced in 6b423ec67

As pointed out by Dean Rasheed, we really should be using tmp >
-(PG_INTNN_MIN / 10) rather than tmp > (PG_INTNN_MAX / 10) for checking
for overflows in the accumulation in the pg_strtointNN functions.  This
does happen to be the same number when dividing by 10, but there is a
pending patch which adds other bases and this is not the same number if we
were to divide by 2 rather than 10, for example.  If the base 2 parsing
was to follow this example then we could accidentally think a string
containing the value of PG_INT32_MIN was an overflow in pg_strtoint32.
Clearly that shouldn't overflow.

This does not fix any actual live bugs, only some bad examples of overflow
checks for future bases.

Reported-by: Dean Rasheed
Discussion: https://postgr.es/m/CAEZATCVEtwfhdm-K-etZYFB0=qsR0nT6qXta_W+GQx4RYph1dg@mail.gmail.com

2 years agoUpdate top-level .gitignore.
Tom Lane [Sun, 4 Dec 2022 20:23:00 +0000 (15:23 -0500)]
Update top-level .gitignore.

Add a comment explaining our policy that we don't put excludes
for nonstandard tools into committed .gitignore files.

Also, remove the entries about libraries with .sl extensions,
since that convention was only used on now-desupported HP-UX.

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

2 years agoRe-pgindent a few files.
Tom Lane [Sun, 4 Dec 2022 19:25:53 +0000 (14:25 -0500)]
Re-pgindent a few files.

Just because I'm a neatnik, and I'm currently working on
code in this area.  It annoys me to not be able to pgindent
my patches without working around unrelated changes.

2 years agoFix broken MemoizePath support in reparameterize_path().
Tom Lane [Sun, 4 Dec 2022 18:48:12 +0000 (13:48 -0500)]
Fix broken MemoizePath support in reparameterize_path().

It neglected to recurse to the subpath, meaning you'd get back
a path identical to the input.  This could produce wrong query
results if the omission meant that the subpath fails to enforce
some join clause it should be enforcing.  We don't have a test
case for this at the moment, but the code is obviously broken
and the fix is equally obvious.  Back-patch to v14 where
Memoize was introduced.

Richard Guo

Discussion: https://postgr.es/m/CAMbWs4_R=ORpz=Lkn2q3ebPC5EuWyfZF+tmfCPVLBVK5W39mHA@mail.gmail.com

2 years agoAdd missing MaterialPath support in reparameterize_path[_by_child].
Tom Lane [Sun, 4 Dec 2022 18:35:42 +0000 (13:35 -0500)]
Add missing MaterialPath support in reparameterize_path[_by_child].

These two functions failed to cover MaterialPath.  That's not a
fatal problem, but we can generate better plans in some cases
if we support it.

Tom Lane and Richard Guo

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

2 years agoFix generate_partitionwise_join_paths() to tolerate failure.
Tom Lane [Sun, 4 Dec 2022 18:17:18 +0000 (13:17 -0500)]
Fix generate_partitionwise_join_paths() to tolerate failure.

We might fail to generate a partitionwise join, because
reparameterize_path_by_child() does not support all path types.
This should not be a hard failure condition: we should just fall back
to a non-partitioned join.  However, generate_partitionwise_join_paths
did not consider this possibility and would emit the (misleading)
error "could not devise a query plan for the given query" if we'd
failed to make any paths for a child join.  Fix it to give up on
partitionwise joining if so.  (The accepted technique for giving up
appears to be to set rel->nparts = 0, which I find pretty bizarre,
but there you have it.)

I have not added a test case because there'd be little point:
any omissions of this sort that we identify would soon get fixed
by extending reparameterize_path_by_child(), so the test would stop
proving anything.  However, right now there is a known test case based
on failure to cover MaterialPath, and with that I've found that this
is broken in all supported versions.  Hence, patch all the way back.

Original report and patch by me; thanks to Richard Guo for
identifying a test case that works against committed versions.

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

2 years agoImprove performance of pg_strtointNN functions
David Rowley [Sun, 4 Dec 2022 03:18:18 +0000 (16:18 +1300)]
Improve performance of pg_strtointNN functions

Experiments have shown that modern versions of both gcc and clang are
unable to fully optimize the multiplication by 10 that we're doing in the
pg_strtointNN functions.  Both compilers seem to be making use of "imul",
which is not the most efficient way to multiply by 10.  This seems to be
due to the overflow checking that we're doing.  Without the overflow
checks, both those compilers switch to a more efficient method of
multiplying by 10.  In absence of overflow concern, integer multiplication
by 10 can be done by bit-shifting left 3 places to multiply by 8 and then
adding the original value twice.

To allow compilers this flexibility, here we adjust the code so that we
accumulate the number as an unsigned version of the type and remove the
use of pg_mul_sNN_overflow() and pg_sub_sNN_overflow().  The overflow
checking can be done simply by checking if the accumulated value has gone
beyond a 10th of the maximum *signed* value for the given type.  If it has
then the accumulation of the next digit will cause an overflow.  After
this is done, we do a final overflow check before converting the unsigned
version of the number back to its signed counterpart.

Testing has shown about an 8% speedup of a COPY into a table containing 2
INT columns.

Author: David Rowley, Dean Rasheed
Discussion: https://postgr.es/m/CAApHDvrL6_+wKgPqRHr7gH_6xy3hXM6a3QCsZ5ForurjDFfenA@mail.gmail.com
Discussion: https://postgr.es/m/CAApHDvrdYByjfj-=WbmVNFgmVZg88-dE7heukw8p55aJ+W=qxQ@mail.gmail.com

2 years agoDoc: flesh out fmgr/README's description of context-node usage.
Tom Lane [Sat, 3 Dec 2022 15:50:39 +0000 (10:50 -0500)]
Doc: flesh out fmgr/README's description of context-node usage.

I wrote this to provide a home for a planned discussion of error
return conventions for non-error-throwing functions.  But it seems
useful as documentation of existing code no matter what becomes of
that proposal, so commit separately.

2 years agoFix DEFAULT handling for multi-row INSERT rules.
Dean Rasheed [Sat, 3 Dec 2022 12:11:33 +0000 (12:11 +0000)]
Fix DEFAULT handling for multi-row INSERT rules.

When updating a relation with a rule whose action performed an INSERT
from a multi-row VALUES list, the rewriter might skip processing the
VALUES list, and therefore fail to replace any DEFAULTs in it. This
would lead to an "unrecognized node type" error.

The reason was that RewriteQuery() assumed that a query doing an
INSERT from a multi-row VALUES list would necessarily only have one
item in its fromlist, pointing to the VALUES RTE to read from. That
assumption is correct for the original query, but not for product
queries produced for rule actions. In such cases, there may be
multiple items in the fromlist, possibly including multiple VALUES
RTEs.

What is required instead is for RewriteQuery() to skip any RTEs from
the product query's originating query, which might include one or more
already-processed VALUES RTEs. What's left should then include at most
one VALUES RTE (from the rule action) to be processed.

Patch by me. Thanks to Tom Lane for reviewing.

Back-patch to all supported branches.

Discussion: https://postgr.es/m/CAEZATCV39OOW7LAR_Xq4i%2BLc1Byux%3DeK3Q%3DHD_pF1o9LBt%3DphA%40mail.gmail.com

2 years agoAdd missing const qualifier
David Rowley [Sat, 3 Dec 2022 07:33:22 +0000 (20:33 +1300)]
Add missing const qualifier

This is present in the declaration for ReadDataFromArchive, so we'd better
have it in the definition too in order to avoid compilers from complaining
about the mismatch of function signatures.

2 years agoPrevent pgstats from getting confused when relkind of a relation changes
Andres Freund [Sat, 3 Dec 2022 02:09:55 +0000 (18:09 -0800)]
Prevent pgstats from getting confused when relkind of a relation changes

When the relkind of a relache entry changes, because a table is converted into
a view, pgstats can get confused in 15+, leading to crashes or assertion
failures.

For HEAD, Tom fixed this in b23cd185fd5, by removing support for converting a
table to a view, removing the source of the inconsistency. This commit just
adds an assertion that a relcache entry's relkind does not change, just in
case we end up with another case of that in the future. As there's no cases of
changing relkind anymore, we can't add a test that that's handled correctly.

For 15, fix the problem by not maintaining the association with the old pgstat
entry when the relkind changes during a relcache invalidation processing. In
that case the pgstat entry needs to be unlinked first, to avoid
PgStat_TableStatus->relation getting out of sync. Also add a test reproducing
the issues.

No known problem exists in 11-14, so just add the test there.

Reported-by: vignesh C <vignesh21@gmail.com>
Author: Andres Freund <andres@anarazel.de>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CALDaNm2yXz+zOtv7y5zBd5WKT8O0Ld3YxikuU3dcyCvxF7gypA@mail.gmail.com
Discussion: https://postgr.es/m/CALDaNm3oZA-8Wbps2Jd1g5_Gjrr-x3YWrJPek-mF5Asrrvz2Dg@mail.gmail.com
Backpatch: 15-

2 years agoFix broken hash function hashbpcharextended().
Jeff Davis [Thu, 1 Dec 2022 19:08:47 +0000 (11:08 -0800)]
Fix broken hash function hashbpcharextended().

Ignore trailing spaces for non-deterministic collations when
hashing.

The previous behavior could lead to tuples falling into the wrong
partitions when hash partitioning is combined with the BPCHAR type and
a non-deterministic collation. Fortunately, it did not affect hash
indexes, because hash indexes do not use extended hash functions.

Decline to backpatch, per discussion.

Discussion: https://postgr.es/m/eb83d0ac7b299eb08f9b900dd08a5a0c5d90e517.camel@j-davis.com
Reviewed-by: Richard Guo, Tom Lane
2 years agoRemove gen_node_support.pl's special treatment of EquivalenceClasses.
Tom Lane [Fri, 2 Dec 2022 20:20:30 +0000 (15:20 -0500)]
Remove gen_node_support.pl's special treatment of EquivalenceClasses.

It seems better to deal with this by explicit annotations on the
fields in question, instead of magic knowledge embedded in the
script.  While that creates a risk-of-omission from failing to
annotate fields, the preceding commit should catch any such
oversights.

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

2 years agoAdd some error cross-checks to gen_node_support.pl.
Tom Lane [Fri, 2 Dec 2022 20:09:51 +0000 (15:09 -0500)]
Add some error cross-checks to gen_node_support.pl.

Check that if we generate a call to copy, compare, write, or read
a specific node type, that node type does have the appropriate
support function.  (This doesn't protect against trying to invoke
nonexistent code when considering generic field types such as
"Node *", but it seems like a useful check anyway.)

Check that array_size() refers to a field appearing earlier in
the struct.  Aside from catching obvious errors like a misspelled
field name, this protects against a more subtle mistake: if the
size field appears later in the struct than the array field, then
compare and read functions would misbehave.  There is actually
exactly that situation in PlannerInfo, but it's okay since we
do not need compare or read functionality for that (today anyway).

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

2 years agoFix psql's \sf and \ef for new-style SQL functions.
Tom Lane [Fri, 2 Dec 2022 19:24:44 +0000 (14:24 -0500)]
Fix psql's \sf and \ef for new-style SQL functions.

Some options of these commands need to be able to identify the start
of the function body within the output of pg_get_functiondef().
It used to be that that always began with "AS", but since the
introduction of new-style SQL functions, it might also start with
"BEGIN" or "RETURN".  Fix that on the psql side, and add some
regression tests.

Noted by me awhile ago, but I didn't do anything about it.
Thanks to David Johnston for a nag.

Discussion: https://postgr.es/m/AM9PR01MB8268D5CDABDF044EE9F42173FE8C9@AM9PR01MB8268.eurprd01.prod.exchangelabs.com