postgresql.git
2 years agoChange argument of appendBinaryStringInfo from char * to void *
Peter Eisentraut [Fri, 30 Dec 2022 09:59:25 +0000 (10:59 +0100)]
Change argument of appendBinaryStringInfo from char * to void *

There is some code that uses this function to assemble some kind of
packed binary layout, which requires a bunch of casts because of this.
Functions taking binary data plus length should take void * instead,
like memcpy() for example.

Discussion: https://www.postgresql.org/message-id/flat/a0086cfc-ff0f-2827-20fe-52b591d2666c%40enterprisedb.com

2 years agoUse appendStringInfoString instead of appendBinaryStringInfo where possible
Peter Eisentraut [Fri, 30 Dec 2022 09:59:25 +0000 (10:59 +0100)]
Use appendStringInfoString instead of appendBinaryStringInfo where possible

For the jsonpath output, we don't need to squeeze out every bit of
performance, so instead use a more robust coding style.  There are
similar calls in jsonb.c, which we leave alone here since there is
indeed a performance impact for bulk exports.

Discussion: https://www.postgresql.org/message-id/flat/a0086cfc-ff0f-2827-20fe-52b591d2666c%40enterprisedb.com

2 years agoAdd const to BufFileWrite
Peter Eisentraut [Fri, 30 Dec 2022 09:02:59 +0000 (10:02 +0100)]
Add const to BufFileWrite

Make data buffer argument to BufFileWrite a const pointer and bubble
this up to various callers and related APIs.  This makes the APIs
clearer and more consistent.

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

2 years agoRemove unnecessary casts
Peter Eisentraut [Fri, 30 Dec 2022 09:02:59 +0000 (10:02 +0100)]
Remove unnecessary casts

Some code carefully cast all data buffer arguments for data write and
read function calls to void *, even though the respective arguments
are already void *.  Remove this unnecessary clutter.

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

2 years agoperl: Hide warnings inside perl.h when using gcc compatible compiler
Andres Freund [Thu, 29 Dec 2022 20:47:29 +0000 (12:47 -0800)]
perl: Hide warnings inside perl.h when using gcc compatible compiler

New versions of perl trigger warnings within perl.h with our compiler
flags. At least -Wdeclaration-after-statement, -Wshadow=compatible-local are
known to be problematic.

To avoid these warnings, conditionally use #pragma GCC system_header before
including plperl.h.

Alternatively, we could add the include paths for problematic headers with
-isystem, but that is a larger hammer and is harder to search for.

A more granular alternative would be to use #pragma GCC diagnostic
push/ignored/pop, but gcc warns about unknown warnings being ignored, so every
to-be-ignored-temporarily compiler warning would require its own pg_config.h
symbol and #ifdef.

As the warnings are voluminous, it makes sense to backpatch this change. But
don't do so yet, we first want gather buildfarm coverage - it's e.g. possible
that some compiler claiming to be gcc compatible has issues with the pragma.

Author: Andres Freund <andres@anarazel.de>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: Discussion: https://postgr.es/m/20221228182455.hfdwd22zztvkojy2@awork3.anarazel.de

2 years agoFix oversight in 7a05425d96
Andrew Dunstan [Thu, 29 Dec 2022 12:16:41 +0000 (07:16 -0500)]
Fix oversight in 7a05425d96

This patch was changed as a result of review but one line didn't get the
message. Mea Culpa.

2 years agoAdd page-level freezing to VACUUM.
Peter Geoghegan [Wed, 28 Dec 2022 16:50:47 +0000 (08:50 -0800)]
Add page-level freezing to VACUUM.

Teach VACUUM to decide on whether or not to trigger freezing at the
level of whole heap pages.  Individual XIDs and MXIDs fields from tuple
headers now trigger freezing of whole pages, rather than independently
triggering freezing of each individual tuple header field.

Managing the cost of freezing over time now significantly influences
when and how VACUUM freezes.  The overall amount of WAL written is the
single most important freezing related cost, in general.  Freezing each
page's tuples together in batch allows VACUUM to take full advantage of
the freeze plan WAL deduplication optimization added by commit 9e540599.

Also teach VACUUM to trigger page-level freezing whenever it detects
that heap pruning generated an FPI.  We'll have already written a large
amount of WAL just to do that much, so it's very likely a good idea to
get freezing out of the way for the page early.  This only happens in
cases where it will directly lead to marking the page all-frozen in the
visibility map.

In most cases "freezing a page" removes all XIDs < OldestXmin, and all
MXIDs < OldestMxact.  It doesn't quite work that way in certain rare
cases involving MultiXacts, though.  It is convenient to define "freeze
the page" in a way that gives FreezeMultiXactId the leeway to put off
the work of processing an individual tuple's xmax whenever it happens to
be a MultiXactId that would require an expensive second pass to process
aggressively (allocating a new multi is especially worth avoiding here).
FreezeMultiXactId is eager when processing is cheap (as it usually is),
and lazy in the event of an individual multi that happens to require
expensive second pass processing.  This avoids regressions related to
processing of multis that page-level freezing might otherwise cause.

Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Jeff Davis <pgsql@j-davis.com>
Reviewed-By: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CAH2-WzkFok_6EAHuK39GaW4FjEFQsY=3J0AAd6FXk93u-Xq3Fg@mail.gmail.com

2 years agoConvert contrib/ltree's input functions to report errors softly
Andrew Dunstan [Wed, 28 Dec 2022 14:58:04 +0000 (09:58 -0500)]
Convert contrib/ltree's input functions to report errors softly

Reviewed by Tom Lane and Amul Sul

Discussion: https://postgr.es/m/49e598c2-cfe8-0928-b6fb-d0cc51aab626@dunslane.net

2 years agoConvert contrib/intarray's bqarr_in() to report errors softly
Andrew Dunstan [Wed, 28 Dec 2022 14:53:00 +0000 (09:53 -0500)]
Convert contrib/intarray's bqarr_in() to report errors softly

Reviewed by Tom Lane and Amul Sul

Discussion: https://postgr.es/m/49e598c2-cfe8-0928-b6fb-d0cc51aab626@dunslane.net

2 years agoReorder some object files in makefiles
Peter Eisentraut [Wed, 28 Dec 2022 13:20:53 +0000 (14:20 +0100)]
Reorder some object files in makefiles

This restores some once-intended alphabetical orders and makes the
lists consistent between the different build systems.

2 years agoSuppress uninitialized-variable warning from a61b1f748.
Tom Lane [Tue, 27 Dec 2022 23:07:48 +0000 (18:07 -0500)]
Suppress uninitialized-variable warning from a61b1f748.

Some compilers complain about sub_rteperminfos not being
initialized, evidently because they don't detect that it
is only used and set if isGeneralSelect is true.
Make it follow the long-established pattern for its
sibling variable sub_rtable.

Per reports from Pavel Stehule and the buildfarm.

Discussion: https://postgr.es/m/CAFj8pRDOvGOi-n616kM0Cc7qSbg_nGoS=-haB+D785sUXADqSg@mail.gmail.com

2 years agoConvert hstore_in to report errors softly.
Tom Lane [Tue, 27 Dec 2022 19:50:56 +0000 (14:50 -0500)]
Convert hstore_in to report errors softly.

The error reporting here was not only old and crufty, but untested.
I took the opportunity to bring the messages into some sort of
compliance with our message style guidelines.

Discussion: https://postgr.es/m/6B6A5C77-60AD-4A71-9F3A-B2C026A281A6@dunslane.net

2 years agoRemove new locale dependency in regproc regression test.
Tom Lane [Tue, 27 Dec 2022 18:06:42 +0000 (13:06 -0500)]
Remove new locale dependency in regproc regression test.

The modified error message for regcollationin failure includes
the database encoding, which it should've occurred to me is a
portability hazard for the regression tests.  Adjust the test
so the expected output doesn't include that.

In passing, fix a comment typo introduced in b8c0ffbd2.

Per buildfarm.

2 years agoSimplify the implementations of the to_reg* functions.
Tom Lane [Tue, 27 Dec 2022 17:33:04 +0000 (12:33 -0500)]
Simplify the implementations of the to_reg* functions.

Given the soft-input-error feature, we can reduce these functions
to be just thin wrappers around a soft-error call of the
corresponding datatype input function.  This means less code and
more certainty that the to_reg* functions match the normal input
behavior.

Notably, it also means that they will accept numeric OID input,
which they didn't before.  It's not clear to me if that omission
had more than laziness behind it, but it doesn't seem like
something we need to work hard to preserve.

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

2 years agoConvert the reg* input functions to report (most) errors softly.
Tom Lane [Tue, 27 Dec 2022 17:26:01 +0000 (12:26 -0500)]
Convert the reg* input functions to report (most) errors softly.

This is not really complete, but it catches most cases of practical
interest.  The main omissions are:

* regtype, regprocedure, and regoperator parse type names by
calling the main grammar, so any grammar-detected syntax error
will still be a hard error.  Also, if one includes a type
modifier in such a type specification, errors detected by the
typmodin function will be hard errors.

* Lookup errors are handled just by passing missing_ok = true
to the relevant catalog lookup function.  Because we've used
quite a restrictive definition of "missing_ok", this means that
edge cases such as "the named schema exists, but you lack
USAGE permission on it" are still hard errors.

It would make sense to me to replace most/all missing_ok
parameters with an escontext parameter and then allow these
additional lookup failure cases to be trapped too.  But that's
a job for some other day.

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

2 years agoConvert tsqueryin and tsvectorin to report errors softly.
Tom Lane [Tue, 27 Dec 2022 17:00:31 +0000 (12:00 -0500)]
Convert tsqueryin and tsvectorin to report errors softly.

This is slightly tedious because the adjustments cascade through
a couple of levels of subroutines, but it's not very hard.
I chose to avoid changing function signatures more than absolutely
necessary, by passing the escontext pointer in existing structs
where possible.

tsquery's nuisance NOTICEs about empty queries are suppressed in
soft-error mode, since they're not errors and we surely don't want
them to be shown to the user anyway.  Maybe that whole behavior
should be reconsidered.

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

2 years agoDetect bad input for types xid, xid8, and cid.
Tom Lane [Tue, 27 Dec 2022 16:40:01 +0000 (11:40 -0500)]
Detect bad input for types xid, xid8, and cid.

Historically these input functions just called strtoul or strtoull
and returned the result, with no error detection whatever.  Upgrade
them to reject garbage input and out-of-range values, similarly to
our other numeric input routines.

To share the code for this with type oid, adjust the existing
"oidin_subr" to be agnostic about the SQL name of the type it is
handling, and move it to numutils.c; then clone it for 64-bit types.

Because the xid types previously accepted hex and octal input by
reason of calling strtoul[l] with third argument zero, I made the
common subroutine do that too, with the consequence that type oid
now also accepts hex and octal input.  In view of 6fcda9aba, that
seems like a good thing.

While at it, simplify the existing over-complicated handling of
syntax errors from strtoul: we only need one ereturn not three.

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

2 years agoRemove overzealous MultiXact freeze assertion.
Peter Geoghegan [Tue, 27 Dec 2022 07:36:02 +0000 (23:36 -0800)]
Remove overzealous MultiXact freeze assertion.

When VACUUM determines that an existing MultiXact should use a freeze
plan that sets xmax to InvalidTransactionId, the original Multi may or
may not be before OldestMxact.  Remove an incorrect assertion that
expected it to always be from before OldestMxact.

Oversight in commit 4ce3af.

Author: Peter Geoghegan <pg@bowt.ie>
Reported-By: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Discussion: https://postgr.es/m/TYAPR01MB5866B24104FD80B5D7E65C3EF5ED9@TYAPR01MB5866.jpnprd01.prod.outlook.com

2 years agoAdd custom filtering rules to the TAP tests of pg_upgrade
Michael Paquier [Tue, 27 Dec 2022 05:35:56 +0000 (14:35 +0900)]
Add custom filtering rules to the TAP tests of pg_upgrade

002_pg_upgrade.pl gains support for a new environment variable called
"filter_rules", that can be used to point to a file that includes a
set of custom regular expressions that would be applied to the dumps of
the origin and target clusters when doing a cross-version test (aka when
defining olddump and oldinstall), to give the possibility to reshape
dynamically the dumps in the same way as the internals of the buildfarm
code so as the tests are able to pass in scenarios where one expects
them to even if pg_dump generates slightly-different outputs depending
on the versions involved.

This option is not used when pg_upgrade runs with the same version for
the origin and target clusters, and it is the last piece I see as
required to be able to plug-in more efficiently the TAP tests of
pg_upgrade with the buildfarm or just a CI.

Author: Anton A. Melnikov
Discussion: https://postgr.es/m/49f389ba-95ce-8a9b-09ae-f60650c0e7c7@inbox.ru

2 years agoFix incorrect copy-pasto in error message of pg_waldump.c
Michael Paquier [Tue, 27 Dec 2022 03:16:24 +0000 (12:16 +0900)]
Fix incorrect copy-pasto in error message of pg_waldump.c

The error message used on fclose() failure was incorrect, so fix it.
Oversight in d497093, that I have somehow managed to miss.

2 years agopg_waldump: Add --save-fullpage=PATH to save full page images from WAL records
Michael Paquier [Mon, 26 Dec 2022 23:27:53 +0000 (08:27 +0900)]
pg_waldump: Add --save-fullpage=PATH to save full page images from WAL records

This option extracts (potentially decompressing) full-page images
included in WAL records into a given target directory.  These images are
subject to the same filtering rules as the normal display of the WAL
records, hence with --relation one can for example extract only the FPIs
issued on the relation defined.  By default, the records are printed or
their stats computed (--stats), using --quiet would only save the images
without any output generated.

This is a tool aimed mostly for very experienced users, useful for
fixing page-level corruption or just analyzing the past state of a page,
and there were no easy way to do that with the in-core tools up to now
when looking at WAL.

Each block is saved in a separate file, to ease their manipulation, with
the file respecting <lsn>.<ts>.<db>.<rel>.<blk>_<fork> with as format.

For instance, 00000000-010000C0.1663.1.6117.123_main refers to:
- WAL record LSN in hexa format (00000000-010000C0).
- Tablespace OID (1663).
- Database OID (1).
- Relfilenode (6117).
- Block number (123).
- Fork name of the file this block came from (_main).

Author: David Christensen
Reviewed-by: Sho Kato, Justin Pryzby, Bharath Rupireddy, Matthias van de
Meent
Discussion: https://postgr.es/m/CAOxo6XKjQb2bMSBRpePf3ZpzfNTwjQUc4Tafh21=jzjX6bX8CA@mail.gmail.com

2 years agoAdd 'logical_decoding_mode' GUC.
Amit Kapila [Mon, 26 Dec 2022 03:28:16 +0000 (08:58 +0530)]
Add 'logical_decoding_mode' GUC.

This enables streaming or serializing changes immediately in logical
decoding. This parameter is intended to be used to test logical decoding
and replication of large transactions for which otherwise we need to
generate the changes till logical_decoding_work_mem is reached.

This helps in reducing the timing of existing tests related to logical
replication of in-progress transactions and will help in writing tests for
for the upcoming feature for parallelly applying large in-progress
transactions.

Author: Shi yu
Reviewed-by: Sawada Masahiko, Shveta Mallik, Amit Kapila, Dilip Kumar, Kuroda Hayato, Kyotaro Horiguchi
Discussion: https://postgr.es/m/OSZPR01MB63104E7449DBE41932DB19F1FD1B9@OSZPR01MB6310.jpnprd01.prod.outlook.com

2 years agoSwitch query fixing aclitems in ~15 from O(N^2) to O(N) in upgrade_adapt.sql
Michael Paquier [Sun, 25 Dec 2022 23:00:55 +0000 (08:00 +0900)]
Switch query fixing aclitems in ~15 from O(N^2) to O(N) in upgrade_adapt.sql

f4f2f2b was doing a sequential scan of pg_class before checking if a
relation had attributes dependent on aclitem as data typewhen building
the set of ALTER TABLE queries, but it would be costly on a regression
database.

While on it, make the query style more consistent with the rest.

Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/20221223032724.GQ1153@telsasoft.com

2 years agoConvert enum_in() to report errors softly.
Tom Lane [Sun, 25 Dec 2022 19:32:02 +0000 (14:32 -0500)]
Convert enum_in() to report errors softly.

I missed this in my initial survey, probably because I examined
the contents of pg_type in the postgres database, which lacks
any enumerated types.

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

2 years agoFix thinko in 720e0327bc
Andrew Dunstan [Sun, 25 Dec 2022 09:36:58 +0000 (04:36 -0500)]
Fix thinko in 720e0327bc

2 years agoConvert contrib/isn's input functions to report errors softly
Andrew Dunstan [Sat, 24 Dec 2022 20:28:13 +0000 (15:28 -0500)]
Convert contrib/isn's input functions to report errors softly

2 years agoConvert jsonpath's input function to report errors softly
Andrew Dunstan [Sat, 24 Dec 2022 20:19:14 +0000 (15:19 -0500)]
Convert jsonpath's input function to report errors softly

Reviewed by Tom Lane

Discussion: https://postgr.es/m/a8dc5700-c341-3ba8-0507-cc09881e6200@dunslane.net

2 years agoMake the numeric-OID cases of regprocin and friends be non-throwing.
Tom Lane [Sat, 24 Dec 2022 20:01:21 +0000 (15:01 -0500)]
Make the numeric-OID cases of regprocin and friends be non-throwing.

While at it, use a common subroutine already.

This doesn't move the needle very far in terms of making these
functions non-throwing; the only case we're now able to trap is
numeric-OID-is-out-of-range.  Still, it seems like a pretty
non-controversial step in that direction.

2 years agoFix recent accidental omission in pg_proc.dat
David Rowley [Sat, 24 Dec 2022 00:18:35 +0000 (13:18 +1300)]
Fix recent accidental omission in pg_proc.dat

ed1a88dda added support functions for the ntile(), percent_rank() and
cume_dist() window functions but neglected to actually add these support
functions to the pg_proc entry for the corresponding window function.

Also, take this opportunity to add these window functions to one of the
regression tests added in ed1a88dda to give the support functions a little
bit of exercise.  If I'd done that in the first place then the omission
would have been more obvious.

Bump the catversion, again.

2 years agoRework <warning> box about column list combining in logical replication
Alvaro Herrera [Fri, 23 Dec 2022 16:49:51 +0000 (17:49 +0100)]
Rework <warning> box about column list combining in logical replication

After some copy-edit I made in commit 3a06a79cd137, we have a <sect2>
that only contains a warning box.  This doesn't look good.  Rework by
moving the sect2 title to be the warning's title, and put the 'id' to it
as well, so that the external reference continues to work.

Backpatch to 15.

In branch master, I also take the opportunity to add titles to a couple
of other warning boxes elsewhere in the documentation.

Discussion: https://postgr.es/m/20221219164713.ccnlvtkyj6lmshqq@alvherre.pgsql

2 years agoFix end LSN determination in recently added test
Alvaro Herrera [Fri, 23 Dec 2022 16:27:05 +0000 (17:27 +0100)]
Fix end LSN determination in recently added test

The test added in commit e44dae07f931 has a thinko: it wants to read
info about a few WAL records, but it obtains the LSN of the final record
to read by asking for the WAL insert position; however,
pg_get_wal_records_info only accepts to read up to the flush position
(cf. IsFutureLSN()).  In normal conditions there is no difference, since
the last record written by the preceding loop is known flushed and it's
the one the test wants; but it's possible to have some other process
insert another WAL record that isn't flushed, and that causes the whole
test to explode.

Fix by having pg_get_wal_records_info() read only up to the flushed
position.  Backpatch to 15, which is where pg_walinspect appeared.

Author: Karina Litskevich <litskevichkarina@gmail.com>
Discussion: https://postgr.es/m/a5559c95-52c3-5eea-cd63-9b4f1c70ff96@gmail.com

2 years agoConvert contrib/seg's input function to report errors softly
Andrew Dunstan [Fri, 23 Dec 2022 14:17:24 +0000 (09:17 -0500)]
Convert contrib/seg's input function to report errors softly

Reviewed by Tom Lane

Discussion: https://postgr.es/m/a8dc5700-c341-3ba8-0507-cc09881e6200@dunslane.net

2 years agoFix event trigger example
Alvaro Herrera [Fri, 23 Dec 2022 12:21:41 +0000 (13:21 +0100)]
Fix event trigger example

Commit 2f9661311b changed command tags from strings to numbers, but
forgot to adjust the code in the event trigger example, which
consequently failed to compile.

While fixing that, improve the indentation to adhere to pgindent style.

Backpatch to v13, where the change was introduced.

Author: Laurenz Albe
Discussion: https://postgr.es/m/81e36ac17dc80489e74dc5b6914afa6ccdb1a99d.camel@cybertec.at

2 years agoFix bug in translate_col_privs_multilevel
David Rowley [Fri, 23 Dec 2022 11:58:34 +0000 (00:58 +1300)]
Fix bug in translate_col_privs_multilevel

Fix incorrect code which was trying to convert a Bitmapset of columns at
the attnums according to a parent table and transform them into the
equivalent Bitmapset with same attnums according to the given child table.
This code is new as of a61b1f748 and was failing to do the correct
translation when there was an intermediate parent table between 'rel' and
'top_parent_rel'.

Reported-by: Ranier Vilela
Author: Richard Guo, Amit Langote
Discussion: https://postgr.es/m/CAEudQArohfB_Gy%2BhcH2-bANUkxgjJiP%3DABq01_LgTNTbcNijag%40mail.gmail.com

2 years agoAllow parent's WaitEventSets to be freed after fork().
Thomas Munro [Fri, 23 Dec 2022 07:26:52 +0000 (20:26 +1300)]
Allow parent's WaitEventSets to be freed after fork().

An epoll fd belonging to the parent should be closed in the child.  A
kqueue fd is automatically closed by fork(), but we should still adjust
our counter.  For poll and Windows systems, nothing special is required.
On all systems we free the memory.

No caller yet, but we'll need this if we start using WaitEventSet in the
postmaster as planned.

Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA%2BhUKG%2BZ-HpOj1JsO9eWUP%2Bar7npSVinsC_npxSy%2BjdOMsx%3DGg%40mail.gmail.com

2 years agoDon't leak a signalfd when using latches in the postmaster.
Thomas Munro [Fri, 23 Dec 2022 07:24:41 +0000 (20:24 +1300)]
Don't leak a signalfd when using latches in the post

At the time of commit 6a2a70a02 we didn't use latch infrastructure in
the postmaster.  We're planning to start doing that, so we'd better make
sure that the signalfd inherited from a postmaster is not duplicated and
then leaked in the child.

Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/CA%2BhUKG%2BZ-HpOj1JsO9eWUP%2Bar7npSVinsC_npxSy%2BjdOMsx%3DGg%40mail.gmail.com

2 years agoAdd WL_SOCKET_ACCEPT event to WaitEventSet API.
Thomas Munro [Fri, 23 Dec 2022 07:21:47 +0000 (20:21 +1300)]
Add WL_SOCKET_ACCEPT event to WaitEventSet API.

To be able to handle incoming connections on a server socket with
the WaitEventSet API, we'll need a new kind of event to indicate that
the the socket is ready to accept a connection.

On Unix, it's just the same as WL_SOCKET_READABLE, but on Windows there
is a different underlying kernel event that we need to map our
abstraction to.

No user yet, but a proposed patch would use this.

Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA%2BhUKG%2BZ-HpOj1JsO9eWUP%2Bar7npSVinsC_npxSy%2BjdOMsx%3DGg%40mail.gmail.com

2 years agoUpdate upgrade_adapt.sql to handle tables using aclitem as data type
Michael Paquier [Fri, 23 Dec 2022 02:36:57 +0000 (11:36 +0900)]
Update upgrade_adapt.sql to handle tables using aclitem as data type

The regression test suite includes a table called "tab_core_types" that
has one attribute based on the type "aclitem".  Keeping this attribute
as-is causes hard failures when running pg_upgrade with an origin on
~15.  This commit updates upgrade_adapt.sql to automatically detect the
tables with such attributes and switch them to text so as pg_upgrade
is able to go through its run.

This does not provide the same detection coverage as pg_upgrade, where
we are able to find out aclitems used in arrays, domains or even
composite types, but this is (I guess) enough for most things like an
instance that had installcheck run on before the upgrade with a dump
generated from it.

Note that the buildfarm code has taken the simplest approach of just
dropping "tab_core_types", so what we have here is more modular.

Author: Anton A. Melnikov
Discussion: https://postgr.es/m/49f389ba-95ce-8a9b-09ae-f60650c0e7c7@inbox.ru

2 years agoFix some incorrectness in upgrade_adapt.sql on query for WITH OIDS
Michael Paquier [Fri, 23 Dec 2022 02:26:49 +0000 (11:26 +0900)]
Fix some incorrectness in upgrade_adapt.sql on query for WITH OIDS

The query used to disable WITH OIDS in all the relations making use of
it was checking for materialized views, but this is not a supported
operation.  On the contrary, this needs to be done on foreign tables.

While on it, use quote_ident() in the ALTER TABLE strings built on the
relation name.

Author: Anton A. Melnikov, Michael Paquier
Discussion: https://postgr.es/m/49f389ba-95ce-8a9b-09ae-f60650c0e7c7@inbox.ru
Backpatch-through: 12

2 years agoFix come incorrect elog() messages in aclchk.c
Michael Paquier [Fri, 23 Dec 2022 01:04:18 +0000 (10:04 +0900)]
Fix come incorrect elog() messages in aclchk.c

Three error strings used with cache lookup failures were referring to
incorrect object types for ACL checks:
- Schemas
- Types
- Foreign Servers
There errors should never be triggered, but if they do incorrect
information would be reported.

Author: Justin Pryzby
Discussion: https://postgr.es/m/20221222153041.GN1153@telsasoft.com
Backpatch-through: 11

2 years agoRename pg_dissect_walfile_name() to pg_split_walfile_name()
Michael Paquier [Fri, 23 Dec 2022 00:15:01 +0000 (09:15 +0900)]
Rename pg_dissect_walfile_name() to pg_split_walfile_name()

The former name was discussed as being confusing, so use "split", as per
a suggestion from Magnus Hagander.

While on it, one of the output arguments is renamed from "segno" to
"segment_number", as per a suggestion from Kyotaro Horiguchi.

The documentation is updated to reflect all these changes.

Bump catalog version.

Author: Bharath Rupireddy, Michael Paquier
Discussion: https://postgr.es/m/CABUevEytQVaOOhGdoh0D7hGwe3fuKcRF6NthsSW7ww04EmtFgQ@mail.gmail.com

2 years agoAllow window functions to adjust their frameOptions
David Rowley [Thu, 22 Dec 2022 23:43:52 +0000 (12:43 +1300)]
Allow window functions to adjust their frameOptions

WindowFuncs such as row_number() don't care if it's called with ROWS
UNBOUNDED PRECEDING AND CURRENT ROW or with RANGE UNBOUNDED PRECEDING AND
CURRENT ROW.  The latter is less efficient as the RANGE option requires
that the executor check for peer rows, so using the ROW option instead
would cause less overhead.  Because RANGE is part of the default frame
options for WindowClauses, it means WindowAgg is, by default, working much
harder than it needs to for window functions where the ROWS / RANGE option
has no effect on the window function's result.

On a test query from the discussion thread, a performance improvement of
344% was seen by using ROWS instead of RANGE.

Here we add a new support function node type to allow support functions to
be called for window functions so that the most optimal version of the
frame options can be set.  The planner has been adjusted so that the frame
options are changed only if all window functions sharing the same window
clause agree on what the optimized frame options are.

Here we give the ability for row_number(), rank(), dense_rank(),
percent_rank(), cume_dist() and ntile() to alter their WindowClause's
frameOptions.

Reviewed-by: Vik Fearing, Erwin Brandstetter, Zhihong Yu
Discussion: https://postgr.es/m/CAGHENJ7LBBszxS+SkWWFVnBmOT2oVsBhDMB1DFrgerCeYa_DyA@mail.gmail.com
Discussion: https://postgr.es/m/CAApHDvohAKEtTXxq7Pc-ic2dKT8oZfbRKeEJP64M0B6+S88z+A@mail.gmail.com

2 years agoImprove notation of cacheinfo table in syscache.c.
Thomas Munro [Thu, 22 Dec 2022 21:40:18 +0000 (10:40 +1300)]
Improve notation of cacheinfo table in syscache.c.

Use C99 designated initializer syntax for the array elements, instead of
writing the enumerator name and position in a comment.  Replace nkeys
and key with a local variadic macro, for a shorter notation.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Discussion: https://postgr.es/m/CA%2BhUKGKdpDjKL2jgC-GpoL4DGZU1YPqnOFHbDqFkfRQcPaR5DQ%40mail.gmail.com

2 years agoUse scanned_pages to decide when to failsafe check.
Peter Geoghegan [Thu, 22 Dec 2022 18:41:40 +0000 (10:41 -0800)]
Use scanned_pages to decide when to failsafe check.

Perform a failsafe check every time VACUUM's first heap scan scans a
further FAILSAFE_EVERY_PAGES pages, rather than using an approach based
on the number of physical blocks that our current blkno is from the
blkno at the time of the previous failsafe check.  That way VACUUM will
perform a failsafe check every time it has scanned a uniform number of
pages, without it mattering when or how VACUUM skipped pages using the
visibility map.

Sami Imseih, with changes to FAILSAFE_EVERY_PAGES comments added by me.

Author: Sami Imseih <simseih@amazon.com>
Reviewed-By: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/401CE010-4049-4B94-9961-0B610A5D254D%40amazon.com

2 years agoRefactor how VACUUM passes around its XID cutoffs.
Peter Geoghegan [Thu, 22 Dec 2022 17:37:59 +0000 (09:37 -0800)]
Refactor how VACUUM passes around its XID cutoffs.

Use a dedicated struct for the XID/MXID cutoffs used by VACUUM, such as
FreezeLimit and OldestXmin.  This state is initialized in vacuum.c, and
then passed around by code from vacuumlazy.c to heapam.c freezing
related routines.  The new convention is that everybody works off of the
same cutoff state, which is passed around via pointers to const.

Also simplify some of the logic for dealing with frozen xmin in
heap_prepare_freeze_tuple: add dedicated "xmin_already_frozen" state to
clearly distinguish xmin XIDs that we're going to freeze from those that
were already frozen from before.  That way the routine's xmin handling
code is symmetrical with the existing xmax handling code.  This is
preparation for an upcoming commit that will add page level freezing.

Also refactor the control flow within FreezeMultiXactId(), while adding
stricter sanity checks.  We now test OldestXmin directly, instead of
using FreezeLimit as an inexact proxy for OldestXmin.  This is further
preparation for the page level freezing work, which will make the
function's caller cede control of page level freezing to the function
where appropriate (where heap_prepare_freeze_tuple sees a tuple that
happens to contain a MultiXactId in its xmax).

Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Jeff Davis <pgsql@j-davis.com>
Discussion: https://postgr.es/m/CAH2-WznS9TxXmz2_=SY+SyJyDFbiOftKofM9=aDo68BbXNBUMA@mail.gmail.com

2 years agoAvoid O(N^2) cost when pulling up lots of UNION ALL subqueries.
Tom Lane [Thu, 22 Dec 2022 16:02:03 +0000 (11:02 -0500)]
Avoid O(N^2) cost when pulling up lots of UNION ALL subqueries.

perform_pullup_replace_vars() knows how to scan the whole parent
query tree when we are replacing Vars during a subquery flattening
operation.  However, for the specific case of flattening a
UNION ALL leaf query, that's mostly wasted work: the only place
where relevant Vars could exist is in the AppendRelInfo that we
just made for this leaf.  Teaching perform_pullup_replace_vars()
to just deal with that and exit is worthwhile because, if we have
N such subqueries to pull up, we were spending O(N^2) work uselessly
mutating the AppendRelInfos for all the other subqueries.

While we're at it, avoid calling substitute_phv_relids if there are no
PlaceHolderVars, and remove an obsolete check of parse->hasSubLinks.

Andrey Lepikhov and Tom Lane

Discussion: https://postgr.es/m/703c09a2-08f3-d2ec-b33d-dbecd62428b8@postgrespro.ru

2 years agoAdd some recursion and looping defenses in prepjointree.c.
Tom Lane [Thu, 22 Dec 2022 15:35:02 +0000 (10:35 -0500)]
Add some recursion and looping defenses in prepjointree.c.

Andrey Lepikhov demonstrated a case where we spend an unreasonable
amount of time in pull_up_subqueries().  Not only is that recursing
with no explicit check for stack overrun, but the code seems not
interruptable by control-C.  Let's stick a CHECK_FOR_INTERRUPTS
there, along with sprinkling some stack depth checks.

An actual fix for the excessive time consumption seems a bit
risky to back-patch; but this isn't, so let's do so.

Discussion: https://postgr.es/m/703c09a2-08f3-d2ec-b33d-dbecd62428b8@postgrespro.ru

2 years agoRemove dead code
Peter Eisentraut [Thu, 22 Dec 2022 07:12:41 +0000 (08:12 +0100)]
Remove dead code

The second appearance of NamespaceRelationId in this if-else chain is
in error and can be removed.

2 years agoAdd work-around for VA_ARGS_NARGS() on MSVC.
Thomas Munro [Thu, 22 Dec 2022 04:14:23 +0000 (17:14 +1300)]
Add work-around for VA_ARGS_NARGS() on MSVC.

The previous coding of VA_ARGS_NARGS() always returned 1 on Visual
Studio, because it treats __VA_ARGS__ as a single token unless you jump
through extra hoops.  Newer compilers have an option to fix that.  Add a
comment about that so that we can remember to clean this up in the
future when our minimum MSVC version advances.

Author: Victor Spirin <v.spirin@postgrespro.ru>
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/f450fc57-a147-19d0-e50c-33571c52cc13%40postgrespro.ru

2 years agoFix operator typo in tablecmds.c
Michael Paquier [Thu, 22 Dec 2022 03:08:45 +0000 (12:08 +0900)]
Fix operator typo in tablecmds.c

A bitwise operator was getting used on two bools in
ATAddCheckConstraint() to track if constraints should be merged or not
with the existing ones of a relation, though obviously this should use
a boolean OR operator.  This led to the same result, but let's be
clean.

Oversight in 074c5cf.

Author: Ranier Vilela
Reviewed-by: Justin Pryzby
Discussion: https://postgr.es/m/CAEudQAp2R2fbbi0OHHhv_n4=Ch0t1VtjObR9YMqtGKHJ+faUFQ@mail.gmail.com

2 years agoAdd palloc_aligned() to allow aligned memory allocations
David Rowley [Thu, 22 Dec 2022 00:32:05 +0000 (13:32 +1300)]
Add palloc_aligned() to allow aligned memory allocations

This introduces palloc_aligned() and MemoryContextAllocAligned() which
allow callers to obtain memory which is allocated to the given size and
also aligned to the specified alignment boundary.  The alignment
boundaries may be any power-of-2 value.  Currently, the alignment is
capped at 2^26, however, we don't expect values anything like that large.
The primary expected use case is to align allocations to perhaps CPU
cache line size or to maybe I/O page size.  Certain use cases can benefit
from having aligned memory by either having better performance or more
predictable performance.

The alignment is achieved by requesting 'alignto' additional bytes from
the underlying allocator function and then aligning the address that is
returned to the requested alignment.  This obviously does waste some
memory, so alignments should be kept as small as what is required.

It's also important to note that these alignment bytes eat into the
maximum allocation size.  So something like:

palloc_aligned(MaxAllocSize, 64, 0);

will not work as we cannot request MaxAllocSize + 64 bytes.

Additionally, because we're just requesting the requested size plus the
alignment requirements from the given MemoryContext, if that context is
the Slab allocator, then since slab can only provide chunks of the size
that's specified when the slab context is created, then this is not going
to work.  Slab will generate an error to indicate that the requested size
is not supported.

The alignment that is requested in palloc_aligned() is stored along with
the allocated memory.  This allows the alignment to remain intact through
repalloc() calls.

Author: Andres Freund, David Rowley
Reviewed-by: Maxim Orlov, Andres Freund, John Naylor
Discussion: https://postgr.es/m/CAApHDvpxLPUMV1mhxs6g7GNwCP6Cs6hfnYQL5ffJQTuFAuxt8A%40mail.gmail.com

2 years agoFix contrib/seg to be more wary of long input numbers.
Tom Lane [Wed, 21 Dec 2022 22:51:50 +0000 (17:51 -0500)]
Fix contrib/seg to be more wary of long input numbers.

seg stores the number of significant digits in an input number
in a "char" field.  If char is signed, and the input is more than
127 digits long, the count can read out as negative causing
seg_out() to print garbage (or, if you're really unlucky,
even crash).

To fix, clamp the digit count to be not more than FLT_DIG.
(In theory this loses some information about what the original
input was, but it doesn't seem like useful information; it would
not survive dump/restore in any case.)

Also, in case there are stored values of the seg type containing
bad data, add a clamp in seg_out's restore() subroutine.

Per bug #17725 from Robins Tharakan.  It's been like this
forever, so back-patch to all supported branches.

Discussion: https://postgr.es/m/17725-0a09313b67fbe86e@postgresql.org

2 years agoIntroduce float4in_internal
Andrew Dunstan [Wed, 21 Dec 2022 13:37:17 +0000 (08:37 -0500)]
Introduce float4in_internal

This is the guts of float4in, callable as a routine to input floats,
which will be useful in an upcoming patch for allowing soft errors in
the seg module's input function.

A similar operation was performed some years ago for float8in in
commit 50861cd683e.

Reviewed by Tom Lane

Discussion: https://postgr.es/m/cee4e426-d014-c0b7-aa22-a659f2cd9130@dunslane.net

2 years agoFix newly introduced bug in slab.c
David Rowley [Wed, 21 Dec 2022 20:57:49 +0000 (09:57 +1300)]
Fix newly introduced bug in slab.c

d21ded75f changed the way slab.c works but introduced a bug that meant we
could end up with the slab's curBlocklistIndex pointing to the wrong list.
The condition which was checking for this was failing to account for two
things:

1. The curBlocklistIndex could be 0 as we've currently got no non-full
blocks to put chunks on.  In this case, the dlist_is_empty() check cannot
be performed as there can be any number of completely full blocks at that
index.

2. The curBlocklistIndex may be greater than the index we just moved the
block onto.  Since we need to ensure we fill up fuller blocks first, we
must reset curBlocklistIndex when changing any blocklist element that's
less than the curBlocklistIndex too.

Reported-by: Takamichi Osumi
Discussion: https://postgr.es/m/TYCPR01MB8373329C6329768D7E093D68EDEB9@TYCPR01MB8373.jpnprd01.prod.outlook.com

2 years agoMake more consistent some translated strings related to compression
Michael Paquier [Wed, 21 Dec 2022 01:39:06 +0000 (10:39 +0900)]
Make more consistent some translated strings related to compression

This commit changes some of the bbstreamer files and pg_dump to use the
same style as a few other places (like common/compression.c), where the
name of the compression method is not part of the string, but an
argument of it.  This reduces a bit the translation work with less
string patterns.

Discussion: https://postgr.es/m/Y5/5tdK+4n3clvtU@paquier.xyz

2 years agoSwitch some system functions to use get_call_result_type()
Michael Paquier [Wed, 21 Dec 2022 01:11:22 +0000 (10:11 +0900)]
Switch some system functions to use get_call_result_type()

This shaves some code by replacing the combinations of
CreateTemplateTupleDesc()/TupleDescInitEntry() hardcoding a mapping of
the attributes listed in pg_proc.dat by get_call_result_type() to build
the TupleDesc needed for the rows generated.

get_call_result_type() is more expensive than the former style, but this
removes some duplication with the lists of OUT parameters (pg_proc.dat
and the attributes hardcoded in these code paths).  This is applied to
functions that are not considered as critical (aka that could be called
repeatedly for monitoring purposes).

Author: Bharath Rupireddy
Reviewed-by: Robert Haas, Álvaro Herrera, Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/CALj2ACV23HW5HP5hFjd89FNS-z5X8r2jNXdMXcpN2BgTtKd87w@mail.gmail.com

2 years agoUse existing SSL certs in LDAP tests instead of generating them
Andrew Dunstan [Mon, 19 Dec 2022 10:58:08 +0000 (05:58 -0500)]
Use existing SSL certs in LDAP tests instead of generating them

The SSL test suite has a bunch of pre-existing certificates, so it's
better simply to use what we already have than generate new certificates
each time the LDAP tests are run.

Discussion: https://postgr.es/m/bc305c7a-f390-44f2-2e82-9bcaec6108da@dunslane.net

2 years agoAdd copyright notices to meson files
Andrew Dunstan [Tue, 20 Dec 2022 12:54:39 +0000 (07:54 -0500)]
Add copyright notices to meson files

Discussion: https://postgr.es/m/222b43a5-2fb3-2c1b-9cd0-375d376c8246@dunslane.net

2 years agoAllow batching of inserts during cross-partition updates.
Etsuro Fujita [Tue, 20 Dec 2022 10:05:00 +0000 (19:05 +0900)]
Allow batching of inserts during cross-partition updates.

Commit 927f453a9 disallowed batching added by commit b663a4136 to be
used for the inserts performed as part of cross-partition updates of
partitioned tables, mainly because the previous code in
nodeModifyTable.c couldn't handle pending inserts into foreign-table
partitions that are also UPDATE target partitions.  But we don't have
such a limitation anymore (cf. commit ffbb7e65a), so let's allow for
this by removing from execPartition.c the restriction added by commit
927f453a9 that batching is only allowed if the query command type is
CMD_INSERT.

In postgres_fdw, since commit 86dc90056 changed it to effectively
disable cross-partition updates in the case where a foreign-table
partition chosen to insert rows into is also an UPDATE target partition,
allow batching in the case where a foreign-table partition chosen to
do so is *not* also an UPDATE target partition.  This is enabled by the
"batch_size" option added by commit b663a4136, which is disabled by
default.

This patch also adjusts the test case added by commit 927f453a9 to
confirm that the inserts performed as part of a cross-partition update
of a partitioned table indeed uses batching.

Amit Langote, reviewed and/or tested by Georgios Kokolatos, Zhihong Yu,
Bharath Rupireddy, Hou Zhijie, Vignesh C, and me.

Discussion: http://postgr.es/m/CA%2BHiwqH1Lz1yJmPs%3DaD-pzd_HLLynLHvq5iYeT9mB0bBV7oJ6w%40mail.gmail.com

2 years agoAdd enable_presorted_aggregate GUC
David Rowley [Tue, 20 Dec 2022 09:28:58 +0000 (22:28 +1300)]
Add enable_presorted_aggregate GUC

1349d279 added query planner support to allow more efficient execution of
aggregate functions which have an ORDER BY or a DISTINCT clause.  Prior to
that commit, the planner would only request that the lower planner produce
a plan with the order required for the GROUP BY clause and it would be
left up to nodeAgg.c to perform the final sort of records within each
group so that the aggregate transition functions were called in the
correct order.  Now that the planner requests the lower planner produce a
plan with the GROUP BY and the ORDER BY / DISTINCT aggregates in mind,
there is the possibility that the planner chooses a plan which could be
less efficient than what would have been produced before 1349d279.

While developing 1349d279, I had in mind that Incremental Sort would help
us in cases where an index exists only on the GROUP BY column(s).
Incremental Sort would just replace the implicit tuplesorts which are
being performed in nodeAgg.c.  However, because the planner has the
flexibility to instead choose a plan which just performs a full sort on
both the GROUP BY and ORDER BY / DISTINCT aggregate columns, there is
potential for the planner to make a bad choice.  The costing for
Incremental Sort is not perfect as it assumes an even distribution of rows
to sort within each sort group.

Here we add an escape hatch in the form of the enable_presorted_aggregate
GUC.  This will allow users to get the pre-PG16 behavior in cases where
they have no other means to convince the query planner to produce a plan
which only sorts on the GROUP BY column(s).

Discussion: https://postgr.es/m/CAApHDvr1Sm+g9hbv4REOVuvQKeDWXcKUAhmbK5K+dfun0s9CvA@mail.gmail.com

2 years agoImprove the performance of the slab memory allocator
David Rowley [Tue, 20 Dec 2022 08:48:51 +0000 (21:48 +1300)]
Improve the performance of the slab memory allocator

Slab has traditionally been fairly slow when compared with the AllocSet or
Generation memory allocators.  Part of this slowness came from having to
write out an entire block when we allocate a new block in order to
populate the free list indexes within the block's memory.  Additional
slowness came from having to move a block onto another dlist each time we
palloc or pfree a chunk from it.

Here we optimize both of those cases and do a little bit extra to improve
the performance of the slab allocator.

Here, instead of writing out the free list indexes when allocating a new
block, we introduce the concept of "unused" chunks.  When a block is first
allocated all chunks are unused.  These chunks only make it onto the
free list when they are pfree'd.  When allocating new chunks on an
existing block, we have the choice of consuming a chunk from the free list
or an unused chunk.  When both exist, we opt to use one from the free
list, as these have been used already and the memory of them is more
likely to be cached by the CPU.

Here we also reduce the number of block lists from there being one for
every possible value of free chunks on a block to just having a small
fixed number of block lists.  We keep the 0th block list for completely
full blocks and anything else stores blocks for some range of free chunks
with fuller blocks appearing on lower block list array elements.  This
reduces how often we must move a block to another list when we allocate or
free chunks, but still allows us to prefer to put new chunks on fuller
blocks and perhaps allow blocks with fewer chunks to be free'd later
once all their remaining chunks have been pfree'd.

Additionally, we now store a list of "emptyblocks", which are blocks that
no longer contain any allocated chunks.  We now keep up to 10 of these
around to avoid having to thrash malloc/free when allocation patterns
continually cause blocks to become free of any allocated chunks only to
allocate more chunks again.  Now only once we have 10 of these, we free
the block.  This does raise the high water mark for the total memory that
a slab context can consume.  It does not seem entirely unreasonable that
we might one day want to make this a property of SlabContext rather than a
compile-time constant.  Let's wait and see if there is any evidence to
support that this is required before doing it.

Author: Andres Freund, David Rowley
Tested-by: Tomas Vondra, John Naylor
Discussion: https://postgr.es/m/20210717194333.mr5io3zup3kxahfm@alap3.anarazel.de

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