postgresql.git
2 years agoFix some 32-bit shift warnings in MSVC
David Rowley [Thu, 24 Nov 2022 22:05:22 +0000 (11:05 +1300)]
Fix some 32-bit shift warnings in MSVC

7b378237a widened AclMode to 64 bits which resulted in 3 new additional
warnings on MSVC.  Here we make use of UINT64CONST to reassure the
compiler that we do intend the bit shift expression to yield a 64-bit
result.

Discussion: https://postgr.es/m/CAApHDvo=pn01Y_3zASZZqn+cotF1c4QFCwWgk6MiF0VscaE5ug@mail.gmail.com

2 years agoImprove indenting in _hash_pgaddtup
David Rowley [Thu, 24 Nov 2022 21:10:44 +0000 (10:10 +1300)]
Improve indenting in _hash_pgaddtup

The Assert added in d09dbeb9b came out rather ugly after having run
pgindent on that code.  Here we adjust things to use some local variables
so that the Assert remains within the 80-character margin.

Author: Ted Yu
Discussion: https://postgr.es/m/CALte62wLSir1=x93Jf0xZvHaO009FEJfhVMFwnaR8q=csPP8kQ@mail.gmail.com

2 years agoMake multixact error message more explicit
Alvaro Herrera [Thu, 24 Nov 2022 09:45:10 +0000 (10:45 +0100)]
Make multixact error message more explicit

There are recent reports involving a very old error message that we have
no history of hitting -- perhaps a recently introduced bug.  Improve the
error message in an attempt to improve our chances of investigating the
bug.

Per reports from Dimos Stamatakis and Bob Krier.

Backpatch to 11.

Discussion: https://postgr.es/m/CO2PR0801MB2310579F65529380A4E5EDC0E20A9@CO2PR0801MB2310.namprd08.prod.outlook.com
Discussion: https://postgr.es/m/17518-04e368df5ad7f2ee@postgresql.org

2 years agodoc: Fix description of how the default user name is chosen
Peter Eisentraut [Thu, 24 Nov 2022 08:04:50 +0000 (09:04 +0100)]
doc: Fix description of how the default user name is chosen

This makes the distinction between operating-system user name and
database user name a bit clearer.  It also clarifies that the user
name is determined first, and then the default database name.

Author: David G. Johnston <david.g.johnston@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAKFQuwZUhgz=sUi+wGQV-PBTNjMovuA-BOV88RV-Vw0m0drCAg@mail.gmail.com

2 years agoAdd the database name to the ps display of logical WAL senders
Michael Paquier [Thu, 24 Nov 2022 07:07:59 +0000 (16:07 +0900)]
Add the database name to the ps display of logical WAL senders

Logical WAL senders display now as follows, gaining a database name:
postgres: walsender USER DATABASE HOST(PORT) STATE

Physical WAL senders show up the same, as of:
postgres: walsender USER HOST(PORT) STATE

This information was missing, hence it was not possible to know from ps
if a WAL sender was a logical or a physical one, and on which database
it is connected when it is logical.

Author: Tatsuhiro Nakamori
Reviewed-by: Fujii Masao, Bharath Rupireddy
Discussion: https://postgr.es/m/36a3b137e82e0ea9fe7e4234f03b64a1@oss.nttdata.com

2 years agoAdd support for file inclusions in HBA and ident configuration files
Michael Paquier [Thu, 24 Nov 2022 04:51:34 +0000 (13:51 +0900)]
Add support for file inclusions in HBA and ident configuration files

pg_hba.conf and pg_ident.conf gain support for three record keywords:
- "include", to include a file.
- "include_if_exists", to include a file, ignoring it if missing.
- "include_dir", to include a directory of files.  These are classified
by name (C locale, mostly) and need to be prefixed by ".conf", hence
following the same rules as GUCs.

This commit relies on the refactoring pieces done in efc9816ad6c528,
783e8c6 and 1b73d0b, adding a small wrapper to build a list of
TokenizedAuthLines (tokenize_include_file), and the code is shaped to
offer some symmetry with what is done for GUCs with the same options.

pg_hba_file_rules and pg_ident_file_mappings gain a new field called
file_name, to track from which file a record is located, taking
advantage of the addition of rule_number in c591300 to offer an
organized view of the HBA or ident records loaded.

Bump catalog version.

Author: Julien Rouhaud
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya@jrouhaud

2 years agoSpeedup hash index builds by skipping needless binary searches
David Rowley [Thu, 24 Nov 2022 04:21:44 +0000 (17:21 +1300)]
Speedup hash index builds by skipping needless binary searches

When building hash indexes using the spool method, tuples are added to the
index page in hashkey order.  Because of this, we can safely skip
performing the binary search on the existing tuples on the page to find
the location to insert the tuple based on its hashkey value.  For this
case, we can just always put the tuple at the end of the item array as the
tuples will always arrive in hashkey order.

Testing has shown that this can improve hash index build speeds by 5-15%
with a unique set of integer values.

Author: Simon Riggs
Reviewed-by: David Rowley
Tested-by: David Zhang, Tomas Vondra
Discussion: https://postgr.es/m/CANbhV-GBc5JoG0AneUGPZZW3o4OK5LjBGeKe_icpC3R1McrZWQ@mail.gmail.com

2 years agoCreate memory context for tokenization after opening top-level file in hba.c
Michael Paquier [Thu, 24 Nov 2022 01:27:38 +0000 (10:27 +0900)]
Create memory context for tokenization after opening top-level file in hba.c

The memory context was created before attempting to open the first HBA
or ident file, which would cause it to leak.  This had no consequences
for the system views for HBA and ident files, but this would cause
memory leaks in the postmaster on reload if the initial HBA and/or ident
files are missing, which is a valid behavior while the backend is
running.

Oversight in efc9816.

Author: Ted Yu
Discussion: https://postgr.es/m/CALte62xH6ivgiKKzPRJgfekPZC6FKLB3xbnf3=tZmc_gKj78dw@mail.gmail.com

2 years agoAdd missing initialization in tokenize_expand_file() for output list
Michael Paquier [Thu, 24 Nov 2022 01:03:11 +0000 (10:03 +0900)]
Add missing initialization in tokenize_expand_file() for output list

This should have been added in efc9816, but it looks like I have found a
way to mess up a bit a patch split.  This should have no consequence in
practice, but let's be clean.

Discussion: https://postgr.es/m/Y324HvGKiWxW2yxe@paquier.xyz

2 years agoRework memory contexts in charge of HBA/ident tokenization
Michael Paquier [Wed, 23 Nov 2022 23:21:55 +0000 (08:21 +0900)]
Rework memory contexts in charge of HBA/ident tokenization

The list of TokenizedAuthLines generated at parsing for the HBA and
ident files is now stored in a static context called tokenize_context,
where only all the parsed tokens are stored.  This context is created
when opening the first authentication file of a HBA/ident set (hba_file
or ident_file), and is cleaned up once we are done all the work around
it through a new routine called free_auth_file().  One call of
open_auth_file() should have one matching call of free_auth_file(), the
creation and deletion of the tokenization context is controlled by the
recursion depth of the tokenization.

Rather than having tokenize_auth_file() return a memory context that
includes all the records, the tokenization logic now creates and deletes
one memory context each time this function is called.  This will
simplify recursive calls to this routine for the upcoming inclusion
record logic.

While on it, rename tokenize_inc_file() to tokenize_expand_file() as
this would conflict with the upcoming patch that will add inclusion
records for HBA/ident files.  An '@' file has its tokens added to an
existing list.

Reloading HBA/indent configuration in a tight loop shows no leaks, as of
one type of test done (with and without -DEXEC_BACKEND).

Author: Michael Paquier
Reviewed-by: Julien Rouhaud
Discussion: https://postgr.es/m/Y324HvGKiWxW2yxe@paquier.xyz

2 years agoSupport for custom slots in the custom executor nodes
Alexander Korotkov [Wed, 23 Nov 2022 21:36:11 +0000 (00:36 +0300)]
Support for custom slots in the custom executor nodes

Some custom table access method may have their tuple format and use custom
executor nodes for their custom scan types. The ability to set a custom slot
would save them from tuple format conversion. Other users of custom executor
nodes may also benefit.

Discussion: https://postgr.es/m/CAPpHfduJUU6ToecvTyRE_yjxTS80FyPpct4OHaLFk3OEheMTNA@mail.gmail.com
Author: Alexander Korotkov
Reviewed-by: Pavel Borisov
2 years agoSimplify WARNING messages from skipped vacuum/analyze on a table
Andrew Dunstan [Wed, 23 Nov 2022 19:41:30 +0000 (14:41 -0500)]
Simplify WARNING messages from skipped vacuum/analyze on a table

This will more easily accomodate adding new permissions for vacuum and
analyze.

Nathan Bossart following a suggestion from Kyotaro Horiguchi

Discussion: https://postgr.es/m/20220726.104712.912995710251150228.horikyota.ntt@gmail.com

2 years agoExpand AclMode to 64 bits
Andrew Dunstan [Wed, 23 Nov 2022 19:41:30 +0000 (14:41 -0500)]
Expand AclMode to 64 bits

We're running out of bits for new permissions. This change doubles the
number of permissions we can accomodate from 16 to 32, so the
forthcoming new ones for vacuum/analyze don't exhaust the pool.

Nathan Bossart

Reviewed by: Bharath Rupireddy, Kyotaro Horiguchi, Stephen Frost, Robert
Haas, Mark Dilger, Tom Lane, Corey Huinker, David G. Johnston, Michael
Paquier.

Discussion: https://postgr.es/m/20220722203735.GB3996698@nathanxps13

2 years agoSimplify vacuum_set_xid_limits() signature.
Peter Geoghegan [Wed, 23 Nov 2022 19:10:06 +0000 (11:10 -0800)]
Simplify vacuum_set_xid_limits() signature.

Pass VACUUM parameters (VacuumParams state) to vacuum_set_xid_limits()
directly, rather than passing most individual VacuumParams fields as
separate arguments.

Also make vacuum_set_xid_limits() output parameter symbol names match
those used by its vacuumlazy.c caller.

Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-Wz=TE7gW5DgSahDkf0UEZigFGAoHNNN6EvSrdzC=Kn+hrA@mail.gmail.com

2 years agoDon't test HEAP_XMAX_INVALID when freezing xmax.
Peter Geoghegan [Wed, 23 Nov 2022 18:49:39 +0000 (10:49 -0800)]
Don't test HEAP_XMAX_INVALID when freezing xmax.

We shouldn't ever need to rely on whether HEAP_XMAX_INVALID is set in
t_infomask when considering whether or not an xmax should be deemed
already frozen, since that status flag is just a hint.  The only
acceptable representation for an "xmax_already_frozen" raw xmax field is
the transaction ID value zero (also known as InvalidTransactionId).

Adjust code that superficially appeared to rely on HEAP_XMAX_INVALID to
make the rule about xmax_already_frozen clear.  Also avoid needlessly
rereading the tuple's raw xmax.

Oversight in bugfix commit d2599ecf.  There is no evidence that this
ever led to incorrect behavior, so no backpatch.  The worst consequence
of this bug was that VACUUM could hypothetically fail to notice and
report on certain kinds of corruption, which seems fairly benign.

Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-Wzkh3DMCDRPfhZxj9xCq9v3WmzvmbiCpf1dNKUBPadhCbQ@mail.gmail.com

2 years agoFix perl warning from commit 9b4eafcaf4
Andrew Dunstan [Wed, 23 Nov 2022 12:03:06 +0000 (07:03 -0500)]
Fix perl warning from commit 9b4eafcaf4

per gripe from Andres Freund and Tom Lane

Backpatch to all live branches.

2 years agoGive better hints for ambiguous or unreferenceable columns.
Tom Lane [Tue, 22 Nov 2022 23:46:31 +0000 (18:46 -0500)]
Give better hints for ambiguous or unreferenceable columns.

Examine ParseNamespaceItem flags to detect whether a column name
is unreferenceable for lack of LATERAL, or could be referenced if
a qualified name were used, and give better hints for such cases.
Also, don't phrase the message to imply that there's only one
matching column when there is really more than one.

Many of the regression test output changes are not very interesting,
but just reflect reclassifying the "There is a column ... but it
cannot be referenced from this part of the query" messages as DETAIL
rather than HINT.  They are details per our style guide, in the sense
of being factual rather than offering advice; and this change provides
room to offer actual HINTs about what to do.

While here, adjust the fuzzy-name-matching code to be a shade less
impenetrable.  It was overloading the meanings of FuzzyAttrMatchState
fields way too much IMO, so splitting them into multiple fields seems
to make it clearer.  It's not like we need to shave bytes in that
struct.

Per discussion of bug #17233 from Alexander Korolev.

Discussion: https://postgr.es/m/17233-afb9d806aaa64b17@postgresql.org

2 years agoYA attempt at taming worst-case behavior of get_actual_variable_range.
Tom Lane [Tue, 22 Nov 2022 19:40:20 +0000 (14:40 -0500)]
YA attempt at taming worst-case behavior of get_actual_variable_range.

We've made multiple attempts at preventing get_actual_variable_range
from taking an unreasonable amount of time (3ca930fc3fccebe421).
But there's still an issue for the very first planning attempt after
deletion of a large number of extremal-valued tuples.  While that
planning attempt will set "killed" bits on the tuples it visits and
thereby reduce effort for next time, there's still a lot of work it
has to do to visit the heap and then set those bits.  It's (usually?)
not worth it to do that much work at plan time to have a slightly
better estimate, especially in a context like this where the table
contents are known to be mutating rapidly.

Therefore, let's bound the amount of work to be done by giving up
after we've visited 100 heap pages.  Giving up just means we'll
fall back on the extremal value recorded in pg_statistic, so it
shouldn't mean that planner estimates suddenly become worthless.

Note that this means we'll still gradually whittle down the problem
by setting a few more index "killed" bits in each planning attempt;
so eventually we'll reach a good state (barring further deletions),
even in the absence of VACUUM.

Simon Riggs, per a complaint from Jakub Wartak (with cosmetic
adjustments by me).  Back-patch to all supported branches.

Discussion: https://postgr.es/m/CAKZiRmznOwi0oaV=4PHOCM4ygcH4MgSvt8=5cu_vNCfc8FSUug@mail.gmail.com

2 years agoRemove useless MERGE test
Alvaro Herrera [Tue, 22 Nov 2022 10:26:47 +0000 (11:26 +0100)]
Remove useless MERGE test

This was trying to exercise an ERROR we don't actually have.

Backpatch to 15.

Reported by Teja Mupparti <Tejeswar.Mupparti@microsoft.com>
Discussion: https://postgr.es/m/SN6PR2101MB1040BDAF740EA4389484E92BF0079@SN6PR2101MB1040.namprd21.prod.outlook.com

2 years agoImprove comments atop pg_get_replication_slots.
Amit Kapila [Tue, 22 Nov 2022 09:52:00 +0000 (15:22 +0530)]
Improve comments atop pg_get_replication_slots.

Update comments atop pg_get_replication_slots to make it clear that it
shows all replication slots that currently exist on the database cluster.

Author: sirisha chamarthi
Discussion: https://postgr.es/m/CAKrAKeXRuFpeiWS+STGFm-RFfW19sUDxju66JkyRi13kdQf94Q@mail.gmail.com

2 years agoIgnore invalidated slots while computing oldest catalog Xmin
Alvaro Herrera [Tue, 22 Nov 2022 09:56:07 +0000 (10:56 +0100)]
Ignore invalidated slots while computing oldest catalog Xmin

Once a logical slot has acquired a catalog_xmin, it doesn't let go of
it, even when invalidated by exceeding the max_slot_wal_keep_size, which
means that dead catalog tuples are not removed by vacuum anymore since
the point is invalidated, until the slot is dropped.  This could be
catastrophic if catalog churn is high.

Change the computation of Xmin to ignore invalidated slots,
to prevent dead rows from accumulating.

Backpatch to 13, where slot invalidation appeared.

Author: Sirisha Chamarthi <sirichamarthi22@gmail.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://postgr.es/m/CAKrAKeUEDeqquN9vwzNeG-CN8wuVsfRYbeOUV9qKO_RHok=j+g@mail.gmail.com

2 years agoAdd wait event for pg_usleep() in perform_spin_delay()
Andres Freund [Tue, 22 Nov 2022 04:34:17 +0000 (20:34 -0800)]
Add wait event for pg_usleep() in perform_spin_delay()

The lwlock wait queue scalability issue fixed in a4adc31f690 was quite hard to
find because of the exponential backoff and because we adjust spins_per_delay
over time within a backend.

To make it easier to find similar issues in the future, add a wait event for
the pg_usleep() in perform_spin_delay(). Showing a wait event while spinning
without sleeping would increase the overhead of spinlocks, which we do not
want.

We may at some later point want to have more granular wait events, but that'd
be a substantial amount of work. This provides at least some insights into
something currently hard to observe.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
https://postgr.es/m/20221120204310.xywrhyxyytsajuuq@awork3.anarazel.de

2 years agoci: Use -fsanitize=undefined,alignment,address in linux tasks
Andres Freund [Mon, 21 Nov 2022 23:13:09 +0000 (15:13 -0800)]
ci: Use -fsanitize=undefined,alignment,address in linux tasks

We have coverage of the various sanitizers in the buildfarm. The sanitizers
however particularly interesting during the development of patches, where the
likelihood of bugs is even higher. There also have been complaints about only
seeing such failures on the buildfarm, rather than before commit.

This commit enables a reasonable set of sanitizers in CI. Use the linux task
for that, as it currently is one of the fastests tasks. Also several of the
sanitizers work best on linux.

The overhead of alignment sanitizer is low, undefined behaviour has moderate
overhead. Test alignment sanitizer in the meson task, as it does both 32 and
64 bit builds and is thus more likely to expose alignment bugs.

Address sanitizer in contrast somewhat expensive. Enable it in the autoconf
task, as the meson task tests both 32 and 64bit which would exacerbate the
cost.

Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/20220323173537.ll7klrglnp4gn2um@alap3.anarazel.de
Discussion: https://postgr.es/m/20221121220903.kf5u7rokfzbmqskm@alap3.anarazel.de

2 years agoci: Introduce SanityCheck task that other tasks depend on
Andres Freund [Mon, 21 Nov 2022 23:13:09 +0000 (15:13 -0800)]
ci: Introduce SanityCheck task that other tasks depend on

To avoid unnecessarily spinning up a lot of VMs / containers for entirely
broken commits, have a minimal task that all others depend on.

The concrete motivation for the change is to use sanitizers in the linux
tasks. As that makes the tests slower, the start of the CompilerWarnings would
be delayed even more. With this change the CompilerWarnings only depends on
the SanityCheck task.

This has the added advantage that now the CompilerWarnings task is not
prevented from running by (most) test failures (particularly annoying when
caused by a test that is flappy in HEAD).

Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/20221002205201.injtofbx4ax4erww@awork3.anarazel.de

2 years agoci: Clean up pre-meson cruft in windows task
Andres Freund [Mon, 21 Nov 2022 22:15:30 +0000 (14:15 -0800)]
ci: Clean up pre-meson cruft in windows task

We don't need CIRRUS_ESCAPING_PROCESSES anymore as the whole tests now run
within a single script: block. We don't need NO_TEMP_INSTALL anymore it was
addressing an issue specific to vcregress.pl.

Author: Justin Pryzby <pryzbyj@telsasoft.com>
Discussion: https://postgr.es/m/20221113235303.GA26337@telsasoft.com

2 years agoReplace link to Hunspell with the current homepage
Daniel Gustafsson [Mon, 21 Nov 2022 22:25:48 +0000 (23:25 +0100)]
Replace link to Hunspell with the current homepage

The Hunspell project moved from Sourceforge to Github sometime
in 2016, so update our links to match the new URL.  Backpatch
the doc changes to all supported versions.

Discussion: https://postgr.es/m/DC9A662A-360D-4125-A453-5A6CB9C6C4B4@yesql.se
Backpatch-through: v11

2 years agoAdd comments and a missing CHECK_FOR_INTERRUPTS in ts_headline.
Tom Lane [Mon, 21 Nov 2022 22:07:07 +0000 (17:07 -0500)]
Add comments and a missing CHECK_FOR_INTERRUPTS in ts_headline.

I just spent an annoying amount of time reverse-engineering the
100%-undocumented API between ts_headline and the text search
parser's prsheadline function.  Add some commentary about that
while it's fresh in mind.  Also remove some unused macros in
wparser_def.c.

While at it, I noticed that when commit 78e73e875 added a
CHECK_FOR_INTERRUPTS call in TS_execute_recurse, it missed
doing so in the parallel function TS_phrase_execute, which
surely needs one just as much.

Back-patch because of the missing CHECK_FOR_INTERRUPTS.
Might as well back-patch the rest of this too.

2 years agoAdd workaround to make ubsan and ps_status.c compatible
Andres Freund [Mon, 21 Nov 2022 21:54:54 +0000 (13:54 -0800)]
Add workaround to make ubsan and ps_status.c compatible

At least on linux, set_ps_display() breaks /proc/$pid/environ. The sanitizer's
helper library uses /proc/$pid/environ to implement getenv(), as it wants to
work independent of libc. When just using undefined and alignment sanitizers,
the sanitizer library is only initialized when the first error occurs, by
which time we've often already called set_ps_display(), preventing the
sanitizer libraries from seeing the options.

We can work around that by defining __ubsan_default_options, a weak symbol
libsanitizer uses to get defaults from the application, and return
getenv("UBSAN_OPTIONS"). But only if main already was reached, so that we
don't end up relying on a not-yet-working getenv().

As it's just a function that won't get called when not running a sanitizer, it
doesn't seem necessary to make compilation of the function conditional.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/20220323173537.ll7klrglnp4gn2um@alap3.anarazel.de

2 years agoMark pageinspect's disk-accessing functions as parallel restricted.
Tom Lane [Mon, 21 Nov 2022 20:37:10 +0000 (15:37 -0500)]
Mark pageinspect's disk-accessing functions as parallel restricted.

These functions have been marked parallel safe, but the buildfarm's
response to commit e2933a6e1 exposed the flaw in that thinking:
if you try to use them on a temporary table, and they run inside
a parallel worker, they'll fail with "cannot access temporary tables
during a parallel operation".

Fix that by marking them parallel restricted instead.  Maybe someday
we'll have a better answer and can reverse this decision.

Back-patch to v15.  To go back further, we'd have to devise variant
versions of pre-1.10 pageinspect versions.  Given the lack of field
complaints, it doesn't seem worth the trouble.  We'll just deem
this case unsupported pre-v15.  (If anyone does complain, it might
be good enough to update the markings manually in their DBs.)

Discussion: https://postgr.es/m/E1ox94a-000EHu-VH@gemulon.postgresql.org

2 years agoProvide options for postmaster to kill child processes with SIGABRT.
Tom Lane [Mon, 21 Nov 2022 16:59:29 +0000 (11:59 -0500)]
Provide options for postmaster to kill child processes with SIGABRT.

The postmaster normally sends SIGQUIT to force-terminate its
child processes after a child crash or immediate-stop request.
If that doesn't result in child exit within a few seconds,
we follow it up with SIGKILL.  This patch provides GUC flags
that allow either of these signals to be replaced with SIGABRT.
On typically-configured Unix systems, that will result in a
core dump being produced for each such child.  This can be
useful for debugging problems, although it's not something you'd
want to have on in production due to the risk of disk space
bloat from lots of core files.

The old postmaster -T switch, which sent SIGSTOP in place of
SIGQUIT, is changed to be the same as send_abort_for_crash.
As far as I can tell from the code comments, the intent of
that switch was just to block things for long enough to force
core dumps manually, which seems like an unnecessary extra step.
(Maybe at the time, there was no way to get most kernels to
produce core files with per-PID names, requiring manual core
file renaming after each one.  But now it's surely the hard way.)

I also took the opportunity to remove the old postmaster -n
(skip shmem reinit) switch, which hasn't actually done anything
in decades, though the documentation still claimed it did.

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

2 years agoPrevent instability in contrib/pageinspect's regression test.
Tom Lane [Mon, 21 Nov 2022 15:50:50 +0000 (10:50 -0500)]
Prevent instability in contrib/pageinspect's regression test.

pageinspect has occasionally failed on slow buildfarm members,
with symptoms indicating that the expected effects of VACUUM
FREEZE didn't happen.  This is presumably because a background
transaction such as auto-analyze was holding back global xmin.

We can work around that by using a temp table in the test.
Since commit a7212be8b, that will use an up-to-date cutoff xmin
regardless of other processes.  And pageinspect itself shouldn't
really care whether the table is temp.

Back-patch to v14.  There would be no point in older branches
without back-patching a7212be8b, which seems like more trouble
than the problem is worth.

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

2 years agoReplace SQLValueFunction by COERCE_SQL_SYNTAX
Michael Paquier [Mon, 21 Nov 2022 09:31:59 +0000 (18:31 +0900)]
Replace SQLValueFunction by COERCE_SQL_SYNTAX

This switch impacts 9 patterns related to a SQL-mandated special syntax
for function calls:
- LOCALTIME [ ( typmod ) ]
- LOCALTIMESTAMP [ ( typmod ) ]
- CURRENT_TIME [ ( typmod ) ]
- CURRENT_TIMESTAMP [ ( typmod ) ]
- CURRENT_DATE

Five new entries are added to pg_proc to compensate the removal of
SQLValueFunction to provide backward-compatibility and making this
change transparent for the end-user (for example for the attribute
generated when a keyword is specified in a SELECT or in a FROM clause
without an alias, or when specifying something else than an Iconst to
the parser).

The parser included a set of checks coming from the files in charge of
holding the C functions used for the SQLValueFunction calls (as of
transformSQLValueFunction()), which are now moved within each function's
execution path, so this reduces the dependencies between the execution
and the parsing steps.  As of this change, all the SQL keywords use the
same paths for their work, relying only on COERCE_SQL_SYNTAX.  Like
fb32748, no performance difference has been noticed, while the perf
profiles get reduced with ExecEvalSQLValueFunction() gone.

Bump catalog version.

Reviewed-by: Corey Huinker, Ted Yu
Discussion: https://postgr.es/m/YzaG3MoryCguUOym@paquier.xyz

2 years agoAdd additional checks while creating the initial decoding snapshot.
Amit Kapila [Mon, 21 Nov 2022 03:24:43 +0000 (08:54 +0530)]
Add additional checks while creating the initial decoding snapshot.

As per one of the CI reports, there is an assertion failure which
indicates that we were trying to use an unenforced xmin horizon for
decoding snapshots. Though, we couldn't figure out the reason for
assertion failure these checks would help us in finding the reason if the
problem happens again in the future.

Author: Amit Kapila based on suggestions by Andres Freund
Reviewd by: Andres Freund
Discussion: https://postgr.es/m/CAA4eK1L8wYcyTPxNzPGkhuO52WBGoOZbT0A73Le=ZUWYAYmdfw@mail.gmail.com

2 years agolwlock: Fix quadratic behavior with very long wait lists
Andres Freund [Sun, 20 Nov 2022 19:56:32 +0000 (11:56 -0800)]
lwlock: Fix quadratic behavior with very long wait lists

Until now LWLockDequeueSelf() sequentially searched the list of waiters to see
if the current proc is still is on the list of waiters, or has already been
removed. In extreme workloads, where the wait lists are very long, this leads
to a quadratic behavior. #backends iterating over a list #backends
long. Additionally, the likelihood of needing to call LWLockDequeueSelf() in
the first place also increases with the increased length of the wait queue, as
it becomes more likely that a lock is released while waiting for the wait list
lock, which is held for longer during lock release.

Due to the exponential back-off in perform_spin_delay() this is surprisingly
hard to detect. We should make that easier, e.g. by adding a wait event around
the pg_usleep() - but that's a separate patch.

The fix is simple - track whether a proc is currently waiting in the wait list
or already removed but waiting to be woken up in PGPROC->lwWaiting.

In some workloads with a lot of clients contending for a small number of
lwlocks (e.g. WALWriteLock), the fix can substantially increase throughput.

As the quadratic behavior arguably is a bug, we might want to decide to
backpatch this fix in the future.

Author: Andres Freund <andres@anarazel.de>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Discussion: https://postgr.es/m/20221027165914.2hofzp4cvutj6gin@awork3.anarazel.de
Discussion: https://postgr.es/m/CALj2ACXktNbG=K8Xi7PSqbofTZozavhaxjatVc14iYaLu4Maag@mail.gmail.com

2 years agopgstat: replace double lookup with IsSharedRelation()
Andres Freund [Sun, 20 Nov 2022 18:53:31 +0000 (10:53 -0800)]
pgstat: replace double lookup with IsSharedRelation()

As the list of shared relations is fixed, we can just dispatch based
IsSharedRelation(), instead of first trying to look up stats for a non-shared
rel and falling back to shared stats.

Author: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de
Discussion: https://postgr.es/m/8c1851a2-a98e-e1bc-7729-37b0b95f66ec@gmail.com

2 years agoFix sloppy cleanup of roles in privileges.sql.
Tom Lane [Sun, 20 Nov 2022 16:30:50 +0000 (11:30 -0500)]
Fix sloppy cleanup of roles in privileges.sql.

Commit 3d14e171e dropped regress_roleoption_donor twice and
regress_roleoption_protagonist not at all.  Leaving roles behind
after "make installcheck" is unfriendly in its own right, plus
it causes repeated runs of "make installcheck" to fail.

2 years agoFix long-obsolete comment.
Tom Lane [Sun, 20 Nov 2022 16:22:22 +0000 (11:22 -0500)]
Fix long-obsolete comment.

Commit c94959d41 fixed DROP OPERATOR to reset oprcom/oprnegate links
to the dropped operator; but it missed updating this old comment that
claimed we allow such links to dangle.

2 years agoPrevent port collisions between concurrent TAP tests
Andrew Dunstan [Sun, 20 Nov 2022 14:51:50 +0000 (09:51 -0500)]
Prevent port collisions between concurrent TAP tests

Currently there is a race condition where if concurrent TAP tests both
test that they can open a port they will assume that it is free and use
it, causing one of them to fail. To prevent this we record a reservation
using an exclusive lock, and any TAP test that discovers a reservation
checks to see if the reserving process is still alive, and looks for
another free port if it is.

Ports are reserved in a directory set by the environment setting
PG_TEST_PORT_DIR, or if that doesn't exist a subdirectory of the top
build directory as set by meson or Makefile.global, or its own
tmp_check directory.

The prove_check recipe in Makefile.global.in is extended to export
top_builddir to the TAP tests. This was already exported by the
prove_installcheck recipes.

Per complaint from Andres Freund

This will be backpatched in due course after some testing.

Discussion: https://postgr.es/m/20221002164931.d57hlutrcz4d2zi7@awork3.anarazel.de

2 years agoSwitch SQLValueFunction on "name" to use COERCE_SQL_SYNTAX
Michael Paquier [Sun, 20 Nov 2022 01:58:28 +0000 (10:58 +0900)]
Switch SQLValueFunction on "name" to use COERCE_SQL_SYNTAX

This commit changes six SQL keywords to use COERCE_SQL_SYNTAX rather
than relying on SQLValueFunction:
- CURRENT_ROLE
- CURRENT_USER
- USER
- SESSION_USER
- CURRENT_CATALOG
- CURRENT_SCHEMA

Among the six, "user", "current_role" and "current_catalog" require
specific SQL functions to allow ruleutils.c to map them to the SQL
keywords these require when using COERCE_SQL_SYNTAX.  Having
pg_proc.proname match with the keyword ensures that the compatibility
remains the same when projecting any of these keywords in a FROM clause
to an attribute name when an alias is not specified.  This is covered by
the tests added in 2e0d80c, making sure that a correct mapping happens
with each SQL keyword.  The three others (current_schema, session_user
and current_user) already have pg_proc entries for this job, so this
brings more consistency between the way such keywords are treated in the
parser, the executor and ruleutils.c.

SQLValueFunction is reduced to half its contents after this change,
simplifying its logic a bit as there is no need to enforce a C collation
anymore for the entries returning a name as a result.  I have made a few
performance tests, with a million-ish calls to these keywords without
seeing a difference in run-time or in perf profiles
(ExecEvalSQLValueFunction() is removed from the profiles).  The
remaining SQLValueFunctions are now related to timestamps and dates.

Bump catalog version.

Reviewed-by: Corey Huinker
Discussion: https://postgr.es/m/YzaG3MoryCguUOym@paquier.xyz

2 years agoFix catversion
Joe Conway [Sat, 19 Nov 2022 22:55:52 +0000 (17:55 -0500)]
Fix catversion

Commit 2fb6154fc didn't quite get the catversion correct per usual
norms. Fix it. Reported by Rishu Bagga.

2 years agoFix mislabeling of PROC_QUEUE->links as PGPROC, fixing UBSan on 32bit
Andres Freund [Thu, 17 Nov 2022 04:00:59 +0000 (20:00 -0800)]
Fix mislabeling of PROC_QUEUE->links as PGPROC, fixing UBSan on 32bit

ProcSleep() used a PGPROC* variable to point to PROC_QUEUE->links.next,
because that does "the right thing" with SHMQueueInsertBefore(). While that
largely works, it's certainly not correct and unnecessary - we can just use
SHM_QUEUE* to point to the insertion point.

Noticed when testing a 32bit of postgres with undefined behavior
sanitizer. UBSan noticed that sometimes the supposed PGPROC wasn't
sufficiently aligned (required since 46d6e5f5679, ensured indirectly, via
ShmemAllocRaw() guaranteeing cacheline alignment).

For now fix this by using a SHM_QUEUE* for the insertion point. Subsequently
we should replace all the use of PROC_QUEUE and SHM_QUEUE with ilist.h, but
that's a larger change that we don't want to backpatch.

Backpatch to all supported versions - it's useful to be able to run postgres
under UBSan.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/20221117014230.op5kmgypdv2dtqsf@awork3.anarazel.de
Backpatch: 11-

2 years agoDisable debug_discard_caches in test_oat_hooks test.
Tom Lane [Sat, 19 Nov 2022 18:42:53 +0000 (13:42 -0500)]
Disable debug_discard_caches in test_oat_hooks test.

The test output varies when debug_discard_caches is enabled,
because that causes extra executions of recomputeNamespacePath.
Maybe putting a hook in that was a bad idea, but as a stopgap,
just turn off debug_discard_caches in this test.

Per buildfarm (now that we have debug_discard_caches coverage
again).  Back-patch to v15 where this module was added.

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

2 years agoDoc: sync src/tutorial/basics.source with SGML documentation.
Tom Lane [Sat, 19 Nov 2022 18:09:14 +0000 (13:09 -0500)]
Doc: sync src/tutorial/basics.source with SGML documentation.

basics.source is supposed to be pretty closely in step with
the examples in chapter 2 of the tutorial, but I forgot to
update it in commit f05a5e000.  Fix that, and adjust a couple
of other discrepancies that had crept in over time.

(I notice that advanced.source is nowhere near being in sync
with chapter 3, but I lack the ambition to do something
about that right now.)

2 years agoFix typos and bump catversion.
Robert Haas [Fri, 18 Nov 2022 21:16:21 +0000 (16:16 -0500)]
Fix typos and bump catversion.

Typos reported by Álvaro Herrera and Erik Rijkers.

Catversion bump for 3d14e171e9e2236139e8976f3309a588bcc8683b was
inadvertently omitted.

2 years agoAdd a SET option to the GRANT command.
Robert Haas [Fri, 18 Nov 2022 17:32:50 +0000 (12:32 -0500)]
Add a SET option to the GRANT command.

Similar to how the INHERIT option controls whether or not the
permissions of the granted role are automatically available to the
grantee, the new SET permission controls whether or not the grantee
may use the SET ROLE command to assume the privileges of the granted
role.

In addition, the new SET permission controls whether or not it
is possible to transfer ownership of objects to the target role
or to create new objects owned by the target role using commands
such as CREATE DATABASE .. OWNER. We could alternatively have made
this controlled by the INHERIT option, or allow it when either
option is given. An advantage of this approach is that if you
are granted a predefined role with INHERIT TRUE, SET FALSE, you
can't go and create objects owned by that role.

The underlying theory here is that the ability to create objects
as a target role is not a privilege per se, and thus does not
depend on whether you inherit the target role's privileges. However,
it's surely something you could do anyway if you could SET ROLE
to the target role, and thus making it contingent on whether you
have that ability is reasonable.

Design review by Nathan Bossat, Wolfgang Walther, Jeff Davis,
Peter Eisentraut, and Stephen Frost.

Discussion: http://postgr.es/m/CA+Tgmob+zDSRS6JXYrgq0NWdzCXuTNzT5eK54Dn2hhgt17nm8A@mail.gmail.com

2 years agoDon't read MCV stats needlessly in eqjoinsel().
Tom Lane [Fri, 18 Nov 2022 16:01:03 +0000 (11:01 -0500)]
Don't read MCV stats needlessly in eqjoinsel().

eqjoinsel() currently makes use of MCV stats only when we have such
stats for both sides of the clause.  As coded, though, it would
fetch those stats even when they're present for just one side.
This can be a bit expensive with high statistics targets, leading
to wasted effort in common cases such as joining a unique column
to a non-unique column.  So it seems worth the trouble to do a quick
pre-check to confirm that both sides have MCVs before fetching either.

Also, tweak the API spec for get_attstatsslot() to document the
method we're using here.

David Geier, Tomas Vondra, Tom Lane

Discussion: https://postgr.es/m/b9846ca0-5f1c-9b26-5881-aad3f42b07f0@gmail.com

2 years agoMake object_address test output easier to update
Peter Eisentraut [Fri, 18 Nov 2022 15:00:52 +0000 (16:00 +0100)]
Make object_address test output easier to update

The object_address test file turns to psql unaligned output for some
tests to avoid huge diffs for changes.  But this is useful also to the
other large test in that file, so apply it there as well.  This also
makes verifying the null and whitespace behavior easier.

2 years agoClean up SQL code indentation in test file
Peter Eisentraut [Fri, 18 Nov 2022 14:31:55 +0000 (15:31 +0100)]
Clean up SQL code indentation in test file

This makes the code layout more consistent inside the same file.

2 years agoFix version comparison in Version.pm
Andrew Dunstan [Fri, 18 Nov 2022 13:38:26 +0000 (08:38 -0500)]
Fix version comparison in Version.pm

Version strings with unequal numbers of parts were being compared
incorrectly. We cure this by treating a missing part in the shorter
version as 0.

per complaint from Jehan-Guillaume de Rorthais, but the fix is mine, not
his.

Discussion: https://postgr.es/m/20220628225325.53d97b8d@karst

Backpatch to release 14 where this code was introduced.

2 years agoFix typo
Alvaro Herrera [Fri, 18 Nov 2022 11:52:44 +0000 (12:52 +0100)]
Fix typo

Erik Rijkers

2 years agodoc: Small wording improvement
Peter Eisentraut [Fri, 18 Nov 2022 11:46:07 +0000 (12:46 +0100)]
doc: Small wording improvement

2 years agoAdd glossary entries related to superusers
Alvaro Herrera [Fri, 18 Nov 2022 10:59:26 +0000 (11:59 +0100)]
Add glossary entries related to superusers

Extracted from a more ambitious patch.

Author: David G. Johnston <david.g.johnston@gmail.com>
Discussion: https://postgr.es/m/CAKFQuwZC4K0XYBm0bwBMDOZySBqhOSekDhLuaw4vPi+ozi8gqQ@mail.gmail.com

2 years agopsql: Improve tab completion for GRANT/REVOKE
Michael Paquier [Fri, 18 Nov 2022 02:26:49 +0000 (11:26 +0900)]
psql: Improve tab completion for GRANT/REVOKE

This commit improves the handling of the following clauses:
- Addition of "CREATE" for ALTER DEFAULT PRIVILEGES .. GRANT/REVOKE.
- Addition of GRANT|ADMIN|INHERIT OPTION FOR for REVOKE, with some
completion for roles, INHERIT being added recently by e3ce2de.
- Addition of GRANT WITH ADMIN|INHERIT.

The list of privilege options common to GRANT and REVOKE is refactored
to avoid its duplication.

Author: Shi Yu
Reviewed-by: Kyotaro Horiguchi, Michael Paquier, Peter Smith
Discussion: https://postgr.es/m/OSZPR01MB6310FCE8609185A56344EED2FD559@OSZPR01MB6310.jpnprd01.prod.outlook.com

2 years agoci: Add task testing windows with mingw
Andres Freund [Fri, 18 Nov 2022 00:22:25 +0000 (16:22 -0800)]
ci: Add task testing windows with mingw

For now the task has been set to be manually triggered, as we are already
limited by the amount of CI time available for windows, particularly on cfbot.

Author: Melih Mutlu <m.melihmutlu@gmail.com>
Reviewed-by: Andrew Dunstan <andrew@dunslane.net>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/CAGPVpCSKS9E0An4=e7ZDnme+y=WOcQFJYJegKO8kE9=gh8NJKQ@mail.gmail.com

2 years agoStandardize rmgrdesc recovery conflict XID output.
Peter Geoghegan [Thu, 17 Nov 2022 22:55:08 +0000 (14:55 -0800)]
Standardize rmgrdesc recovery conflict XID output.

Standardize on the name snapshotConflictHorizon for all XID fields from
WAL records that generate recovery conflicts when in hot standby mode.
This supersedes the previous latestRemovedXid naming convention.

The new naming convention places emphasis on how the values are actually
used by REDO routines.  How the values are generated during original
execution (details of which vary by record type) is deemphasized.  Users
of tools like pg_waldump can now grep for snapshotConflictHorizon to see
all potential sources of recovery conflicts in a standardized way,
without necessarily having to consider which specific record types might
be involved.

Also bring a couple of WAL record types that didn't follow any kind of
naming convention into line.  These are heapam's VISIBLE record type and
SP-GiST's VACUUM_REDIRECT record type.  Now every WAL record whose REDO
routine calls ResolveRecoveryConflictWithSnapshot() passes through the
snapshotConflictHorizon field from its WAL record.  This is follow-up
work to the refactoring from commit 9e540599 that made FREEZE_PAGE WAL
records use a standard snapshotConflictHorizon style XID cutoff.

No bump in XLOG_PAGE_MAGIC, since the underlying format of affected WAL
records doesn't change.

Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CAH2-Wzm2CQUmViUq7Opgk=McVREHSOorYaAjR1ZpLYkRN7_dPw@mail.gmail.com

2 years agoFix MERGE tuple count with DO NOTHING
Alvaro Herrera [Thu, 17 Nov 2022 17:56:11 +0000 (18:56 +0100)]
Fix MERGE tuple count with DO NOTHING

Reporting tuples for which nothing is done is useless and goes against
the documented behavior, so don't do it.

Backpatch to 15.

Reported by: Luca Ferrari
Discussion: https://postgr.es/m/CAKoxK+42MmACUh6s8XzASQKizbzrtOGA6G1UjzCP75NcXHsiNw@mail.gmail.com

2 years agoUse correct type name in comments about freezing.
Peter Geoghegan [Thu, 17 Nov 2022 17:34:12 +0000 (09:34 -0800)]
Use correct type name in comments about freezing.

Oversight in commit 9e540599, which added freeze plan deduplication.

2 years agoAccount for IPC::Run::result() Windows behavior change.
Noah Misch [Thu, 17 Nov 2022 15:35:06 +0000 (07:35 -0800)]
Account for IPC::Run::result() Windows behavior change.

This restores compatibility with the not-yet-released successor of
version 20220807.0.  Back-patch to 9.4, which introduced this code.

Reviewed by Andrew Dunstan.

Discussion: https://postgr.es/m/20221117061805.GA4020280@rfd.leadboat.com

2 years agolibpq: Handle NegotiateProtocolVersion message
Peter Eisentraut [Thu, 17 Nov 2022 14:14:44 +0000 (15:14 +0100)]
libpq: Handle NegotiateProtocolVersion message

Before, receiving a NegotiateProtocolVersion message would result in a
confusing error message like

    expected authentication request from server, but received v

This adds proper handling of this protocol message and produces an
on-topic error message from it.

Reviewed-by: Jacob Champion <jchampion@timescale.com>
Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/f9c7862f-b864-8ef7-a861-c4638c83e209%40enterprisedb.com

2 years agolibpq: Correct processing of startup response messages
Peter Eisentraut [Thu, 17 Nov 2022 13:12:04 +0000 (14:12 +0100)]
libpq: Correct processing of startup response messages

After sending a startup message, libpq expects either an error
response ('E') or an authentication request ('R').  Before processing
the message, it ensures it has read enough bytes to correspond to the
length specified in the message.  However, when processing the 'R'
message, if an EOF status is returned it loops back waiting for more
input, even though we already checked that we have enough input.  In
this particular case, this is probably not reachable anyway, because
other code ensures we have enough bytes for an authentication request
message, but the code is wrong and misleading.  In the more general
case, processing a faulty message could result in an EOF status, which
would then result in an infinite loop waiting for the end of a message
that will never come.  The correction is to make this an error.

Reported-by: Jacob Champion <jchampion@timescale.com>
Discussion: https://www.postgresql.org/message-id/flat/f9c7862f-b864-8ef7-a861-c4638c83e209@enterprisedb.com

2 years agoFix wording in comment
Daniel Gustafsson [Thu, 17 Nov 2022 12:17:19 +0000 (13:17 +0100)]
Fix wording in comment

Author: vignesh C <vignesh21@gmail.com>
Discussion: https://postgr.es/m/CALDaNm0jKY__83tUsem79+YqfjTWTAkDfiPS0T_Z4y0AYGd_HQ@mail.gmail.com

2 years agoFix outdated comment in ExecDelete
Alvaro Herrera [Thu, 17 Nov 2022 11:52:20 +0000 (12:52 +0100)]
Fix outdated comment in ExecDelete

This commend references a struct that disappeared before MERGE was
merged ... and ExecDelete is not called by the committed MERGE anyway.
Revert to the original wording.

Backpatch to 15

2 years agoAllow initdb to complete on systems without "locale" command
Peter Eisentraut [Thu, 17 Nov 2022 11:12:11 +0000 (12:12 +0100)]
Allow initdb to complete on systems without "locale" command

This partially reverts 2fe3bdbd691a5d11626308e7d660440be6c210c8, which
added an error check on the "locale -a" execution.  This is removed
again, adding a comment explaining why.  We already had code that
shows a warning if no system locales could be found, which should be
sufficient for feedback to the user.

Discussion: https://www.postgresql.org/message-id/flat/b2b491d1-3b36-15b9-6910-5b5540b27f5c%40enterprisedb.com

2 years agodoc: Fix wording of MERGE actions in README
Daniel Gustafsson [Thu, 17 Nov 2022 09:07:06 +0000 (10:07 +0100)]
doc: Fix wording of MERGE actions in README

UPDATE was listed twice and DELETE was omitted, replace one UPDATE
with DELETE instead.

Backpatch through v15 where MERGE was added.

Author: Myo Wai Thant <myo.waithant@fujitsu.com>
Reviewed-by: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/OSAPR01MB43247E46931E9E9CFC4AA0F29A079@OSAPR01MB4324.jpnprd01.prod.outlook.com
Backpatch-through: 15

2 years agoFix typos in comments
Daniel Gustafsson [Thu, 17 Nov 2022 08:12:51 +0000 (09:12 +0100)]
Fix typos in comments

Fix various misspellings of xl_running_xacts.

Author: Japin Li <japinli@hotmail.com>
Discussion: https://postgr.es/m/MEYP282MB1669CA2A39ACF0172774ED27B6069@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM

2 years agoRemove unneeded include in test_slru.c
Michael Paquier [Thu, 17 Nov 2022 07:00:09 +0000 (16:00 +0900)]
Remove unneeded include in test_slru.c

As introduced in 006b69f, the order of the headers was incorrect.
However, it happens that lwlock.h can just be dropped from the list, so
let's be clean and remove it, fixing the order of the listed headers.

2 years agoExport with_icu when running src/bin/scripts tests with meson
Peter Eisentraut [Thu, 17 Nov 2022 06:43:50 +0000 (07:43 +0100)]
Export with_icu when running src/bin/scripts tests with meson

Author: Marina Polyakova <m.polyakova@postgrespro.ru>
Discussion: https://www.postgresql.org/message-id/flat/534fed4a262fee534662bd07a691c5ef@postgrespro.ru

2 years agoUpdate some more ObjectType switch statements to not have default
Peter Eisentraut [Thu, 17 Nov 2022 06:08:44 +0000 (07:08 +0100)]
Update some more ObjectType switch statements to not have default

This allows the compiler to complain if a case has been missed.  In
these instances, the tradeoff of having to list a few unneeded cases
to silence the compiler seems better than the risk of actually missing
one.

Discussion: https://www.postgresql.org/message-id/flat/fce5c98a-45da-19e7-dad0-21096bccd66e%40enterprisedb.com

2 years agoImprove ruleutils' printout of LATERAL references within subplans.
Tom Lane [Thu, 17 Nov 2022 01:06:09 +0000 (20:06 -0500)]
Improve ruleutils' printout of LATERAL references within subplans.

Commit 1cc29fe7c, which taught EXPLAIN to print PARAM_EXEC Params as
the referenced expressions, included some checks to prevent matching
Params found in SubPlans or InitPlans to NestLoopParams of upper query
levels.  At the time, this seemed possibly necessary to avoid false
matches because of the planner's habit of re-using the same PARAM_EXEC
slot in multiple places in a plan.  Furthermore, in the absence of
LATERAL no such reference could be valid anyway.  But it's possible
now that we have LATERAL, and in the wake of 46c508fbc and 1db5667ba
I believe the false-match hazard is gone.  Hence, remove the
in_same_plan_level checks.  As shown in the regression test changes,
this provides a useful improvement in readability for EXPLAIN of
LATERAL-using subplans.

Richard Guo, reviewed by Greg Stark and myself

Discussion: https://postgr.es/m/CAMbWs4-YSOcQXAagJetP95cAeZPqzOy5kM5yijG0PVW5ztRb4w@mail.gmail.com

2 years agoFix slowdown in TAP tests due to recent walreceiver change.
Thomas Munro [Wed, 16 Nov 2022 22:30:14 +0000 (11:30 +1300)]
Fix slowdown in TAP tests due to recent walreceiver change.

Commit 05a7be93 changed the timing of the first reply sent by a
walreceiver, which caused a few TAP tests that call wait_for_catchup()
when they haven't actually streamed anything yet to wait ~10 seconds
(wal_receiver_status_interval).

Before commit 05a7be93 the initial reply was sent after 100ms, but
there's no reason not to send it immediately as a slight improvement.
Do the same for HS feedback for consistency.

Author: Nathan Bossart <nathandbossart@gmail.com>
Discussion: https://postgr.es/m/742545.1668377284%40sss.pgh.pa.us

2 years agoInvent "multibitmapsets", and use them to speed up antijoin detection.
Tom Lane [Wed, 16 Nov 2022 18:58:42 +0000 (13:58 -0500)]
Invent "multibitmapsets", and use them to speed up antijoin detection.

Implement a data structure that is a List of Bitmapsets, which is
essentially a 2-D boolean array except that the rows need not all
be the same width.  Operations such as union and intersection are
meaningful for these, just as they are for Bitmapsets.  Eventually
we might build many of the same operations that we have written for
Bitmapsets, but for the first use-case we just need a few.

That first use-case is for antijoin detection: reduce_outer_joins
needs to find the set of Vars that are certain to be non-null in a
successfully joined (not null-extended) left join row, and also
find the set of Vars subject to higher-level IS NULL constraints,
and intersect them.  We had been doing this by making Lists of
the Var nodes and then using list_intersect, which works but is
pretty inefficient compared to a bitmapset-like intersection.
Potentially it's O(N^2) if there are a lot of Vars involved,
which fortunately there generally aren't; still it's not great.
Moreover, that method requires the Vars of interest to be exactly
equal() in the join condition and the upper IS NULL condition,
which is problematic for my WIP patch that labels Vars according
to which outer joins have possibly nulled them.

Discussion: https://postgr.es/m/892228.1668437838@sss.pgh.pa.us
Discussion: https://postgr.es/m/CAMbWs4-mvPPCJ1W6iK6dD5HiNwoJdi6mZp=-7mE8N9Sh+cd0tQ@mail.gmail.com

2 years agoAdd missing object classes to object_address test
Peter Eisentraut [Wed, 16 Nov 2022 18:44:38 +0000 (19:44 +0100)]
Add missing object classes to object_address test

Per the comment, fill in classes mentioned in getObjectIdentityParts()
but not in the test.

2 years agoShave some cycles off subscription/t/100_bugs.pl tests.
Tom Lane [Wed, 16 Nov 2022 17:35:25 +0000 (12:35 -0500)]
Shave some cycles off subscription/t/100_bugs.pl tests.

We can re-use the clusters set up for this test script's first test,
instead of generating new ones.  On my machine this is good for
about a 20% reduction in this script's runtime, from ~6.5 sec to
~5.2 sec.

This idea could be taken further, but it'd require a much more invasive
patch.  These cases are easy because the Perl variable names were
already being re-used.

Anton A. Melnikov

Discussion: https://postgr.es/m/eb7aa992-c2d7-6ce7-4942-0c784231a362@inbox.ru

2 years agoVariable renaming in preparation for refactoring
Peter Eisentraut [Wed, 16 Nov 2022 15:17:18 +0000 (16:17 +0100)]
Variable renaming in preparation for refactoring

Rename page -> block and dp -> page where appropriate.  The old naming
mixed up block and page in confusing ways.

Author: Melanie Plageman <melanieplageman@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAAKRu_YSOnhKsDyFcqJsKtBSrd32DP-jjXmv7hL0BPD-z0TGXQ@mail.gmail.com

2 years agoRemove useless casts
Peter Eisentraut [Wed, 16 Nov 2022 15:01:06 +0000 (16:01 +0100)]
Remove useless casts

Maybe these are left from when PageGetItem() was a macro, but now they
are clearly useless.

2 years agoTurn HeapKeyTest macro into inline function
Peter Eisentraut [Wed, 16 Nov 2022 12:25:59 +0000 (13:25 +0100)]
Turn HeapKeyTest macro into inline function

It is easier to read as a function.

Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/flat/CAAKRu_YSOnhKsDyFcqJsKtBSrd32DP-jjXmv7hL0BPD-z0TGXQ@mail.gmail.com

2 years agoRemove unused include
Peter Eisentraut [Wed, 16 Nov 2022 10:28:44 +0000 (11:28 +0100)]
Remove unused include

Author: Melanie Plageman <melanieplageman@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAAKRu_YSOnhKsDyFcqJsKtBSrd32DP-jjXmv7hL0BPD-z0TGXQ@mail.gmail.com

2 years agodoc: document the TAP test environment variables
Daniel Gustafsson [Wed, 16 Nov 2022 09:25:21 +0000 (10:25 +0100)]
doc: document the TAP test environment variables

The TAP tests can, to some degree, be controlled by a set of environment
variables. These were however only documented in a README and not in the
main documentation.  This adds documentation of these variables, as well
as changes one CPAN reference to a ulink for consistency.  While there,
also tag CPAN as an acronym as it's listed in the acronyms section.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/YyPd9unV14SX2bLF@paquier.xyz

2 years agodoc: update metacpan.org links to avoid redirects
Daniel Gustafsson [Wed, 16 Nov 2022 09:24:37 +0000 (10:24 +0100)]
doc: update metacpan.org links to avoid redirects

The /release/ links are redirected to /dist/ and /pod/release/ to
/release/../view/, so update our links accordingly to avoid 301
redirects.

Discussion: https://postgr.es/m/CA672723-BAD2-436E-B6E6-163841E11A1B@yesql.se

2 years agoUse multi-inserts for pg_ts_config_map
Michael Paquier [Wed, 16 Nov 2022 05:32:09 +0000 (14:32 +0900)]
Use multi-inserts for pg_ts_config_map

Two locations working on pg_ts_config_map are switched from
CatalogTupleInsert() to a multi-insert approach with tuple slots:
- ALTER TEXT SEARCH CONFIGURATION ADD/ALTER MAPPING when inserting new
entries.  The number of entries to insert is known in advance, so is the
number of slots needed.  Note that CatalogTupleInsertWithInfo() is now
used for the entry updates.
- CREATE TEXT SEARCH CONFIGURATION, where up to ~20-ish records could be
inserted at once.  The number of slots is not known in advance, hence
a slot initialization is delayed until a tuple is stored in it.

Like all the changes of this kind (1ff416163110c6 or e3931d01), an
insert batch is capped at 64kB.

Author: Michael Paquier, Ranier Vilela
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/Y3M5bovrkTQbAO4W@paquier.xyz

2 years agoFix test in ae168c794f, per buildfarm.
Jeff Davis [Wed, 16 Nov 2022 03:35:12 +0000 (19:35 -0800)]
Fix test in ae168c794f, per buildfarm.

Reported-by: Michael Paquier
Discussion: https://postgr.es/m/Y3Q8SGMXhInL4o3X@paquier.xyz

2 years agoUse multi-inserts for pg_enum
Michael Paquier [Wed, 16 Nov 2022 03:41:29 +0000 (12:41 +0900)]
Use multi-inserts for pg_enum

This allows to insert at once all the enum values defined with a given
type into pg_enum, reducing the WAL produced by roughly 10%~.  pg_enum's
indexes are opened and closed now once rather than N times.  The number
of items to insert is known in advance, making this change
straight-forward, and would happen on a CREATE TYPE .. AS ENUM.

The amount of data inserted is capped at 64kB for each insert batch.
This is similar to commits 63110c6 and e3931d01, that worked on
different catalogs.

Reported-by: Ranier Vilela
Author: Michael Paquier
Reviewed-by: Kyotaro Horiguchi, Ranier Vilela
Discussion: https://postgr.es/m/Y3M5bovrkTQbAO4W@paquier.xyz

2 years agoAvoid some overhead with open and close of catalog indexes
Michael Paquier [Wed, 16 Nov 2022 01:49:05 +0000 (10:49 +0900)]
Avoid some overhead with open and close of catalog indexes

This commit improves two code paths to open and close indexes a
minimum amount of times when doing a series of catalog updates or
inserts.  CatalogTupleInsert() is costly when using it for multiple
inserts or updates compared to CatalogTupleInsertWithInfo(), as it would
need to open and close the indexes of the catalog worked each time an
operation is done.

This commit updates the following places:
- REINDEX CONCURRENTLY when copying statistics from one index relation
to the other.  Multi-INSERTs are avoided here, as this would begin to
show benefits only for indexes with multiple expressions, for example,
which may not be the most common pattern.  This change is noticeable in
profiles with indexes having many expressions, for example, and it would
improve any callers of CopyStatistics().
- Update of statistics on ANALYZE, that mixes inserts and updates.
In each case, the catalog indexes are opened only if at least one
insertion and/or update is required, to minimize the cost of the
operation.  Like the previous coding, no indexes are opened as long as
at least one insert or update of pg_statistic has happened.

Author: Ranier Vilela
Reviewed-by: Kyotaro Horiguchi, Michael Paquier
Discussion: https://postgr.es/m/CAEudQAqh0F9y6Di_Wc8xW4zkWm_5SDd-nRfVsCn=h0Nm1C_mrg@mail.gmail.com

2 years agoAdd test module for SLRUs
Michael Paquier [Wed, 16 Nov 2022 00:52:21 +0000 (09:52 +0900)]
Add test module for SLRUs

This commit introduces a basic facility to test SLRUs, in terms of
initialization, page reads, writes, flushes, truncation and deletions,
using SQL wrappers around the APIs of slru.c.  This should be easily
extensible at will, and it can be used as a starting point for someone
willing to implement an external module that makes use of SLRUs (LWLock
tranche registering and SLRU initialization particularly).

As this requires a loaded library, the tests use a custom configuration
file and are disabled under installcheck.

Author: Aleksander Alekseev, Michael Paquier
Reviewed-by: Pavel Borisov, Daniel Gustafsson, Noah Misch, Maxim Orlov
Discussion: https://postgr.es/m/CAJ7c6TOFoWcHOW4BVe3BG_uikCrO9B91ayx9d6rh5JZr_tPESg@mail.gmail.com

2 years agoMark argument of RegisterCustomRmgr() as const.
Jeff Davis [Tue, 15 Nov 2022 23:58:12 +0000 (15:58 -0800)]
Mark argument of RegisterCustomRmgr() as const.

2 years agoAdd test module for Custom WAL Resource Manager feature.
Jeff Davis [Tue, 15 Nov 2022 21:10:27 +0000 (13:10 -0800)]
Add test module for Custom WAL Resource Manager feature.

Author: Bharath Rupireddy, Jeff Davis
Discussion: https://postgr.es/m/CALj2ACVTBNA1wfVCsikfhygAbZe6kFY8Oz6PhOyhHyA4vAGouA%40mail.gmail.com

2 years agoDeduplicate freeze plans in freeze WAL records.
Peter Geoghegan [Tue, 15 Nov 2022 15:48:41 +0000 (07:48 -0800)]
Deduplicate freeze plans in freeze WAL records.

Make heapam WAL records that describe freezing performed by VACUUM more
space efficient by storing each distinct "freeze plan" once, alongside
an array of associated page offset numbers (one per freeze plan).  The
freeze plans required for most heap pages tend to naturally have a great
deal of redundancy, so this technique is very effective in practice.  It
often leads to freeze WAL records that are less than 20% of the size of
equivalent WAL records generated using the previous approach.

The freeze plan concept was introduced by commit 3b97e6823b, which fixed
bugs in VACUUM's handling of MultiXacts.  We retain the concept of
freeze plans, but go back to using page offset number arrays.  There is
no loss of generality here because deduplication is an additive process
that gets applied mechanically when FREEZE_PAGE WAL records are built.

More than anything else, freeze plan deduplication is an optimization
that reduces the marginal cost of freezing additional tuples on pages
that will need to have at least one or two tuples frozen in any case.
Ongoing work that adds page-level freezing to VACUUM will take full
advantage of the improved cost profile through batching.

Also refactor some of the details surrounding recovery conflicts needed
to REDO freeze records in passing: make original execution responsible
for generating a standard latestRemovedXid cutoff, rather than working
backwards to get the same cutoff in the REDO routine.  Bugfix commit
66fbcb0d2e did it the other way around, which is equivalent but obscures
what's going on.

Also rename the cutoff field from the WAL record/struct (rename the
field cutoff_xid to latestRemovedXid to match similar WAL records).
Processing of conflicts by REDO routines is already completely uniform,
so tools like pg_waldump should present the information driving the
process uniformly.  There are two remaining WAL record types that still
don't quite follow this convention (heapam's VISIBLE record type and
SP-GiST's VACUUM_REDIRECT record type).  They can be brought into line
by later work that totally standardizes how the cutoffs are presented.

Bump XLOG_PAGE_MAGIC.

Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-By: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-By: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/CAH2-Wz=XytErMnb8FAyFd+OQEbiipB0Q2FmFdXrggPL4VBnRYQ@mail.gmail.com

2 years agoCheck return value of pclose() correctly
Peter Eisentraut [Tue, 15 Nov 2022 14:35:37 +0000 (15:35 +0100)]
Check return value of pclose() correctly

Some callers didn't check the return value of pclose() or
ClosePipeStream() correctly.  Either they didn't check it at all or
they treated it like the return of fclose().

The correct way is to first check whether the return value is -1, and
then report errno, and then check the return value like a result from
system(), for which we already have wait_result_to_str() to make it
simpler.  To make this more compact, expand wait_result_to_str() to
also handle -1 explicitly.

Reviewed-by: Ankit Kumar Pandey <itsankitkp@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/8cd9fb02-bc26-65f1-a809-b1cb360eef73@enterprisedb.com

2 years agodoc: Use more concise wording for pl/pgSQL TG_ variables
Daniel Gustafsson [Tue, 15 Nov 2022 13:51:02 +0000 (14:51 +0100)]
doc: Use more concise wording for pl/pgSQL TG_ variables

To improve readability of the TG_ variables definition lists, this moves
the datatypes up to the defined term to avoid having each entry start
with "Data type". This also removes redundant wording that that didn't
carry any information from the descriptions.

Author: Christoph Berg <myon@debian.org>
Reviewed-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Discussion: https://postgr.es/m/Yw4Noe3A2s87A0qq@msg.df7cb.de

2 years agopsql: Add command to use extended query protocol
Peter Eisentraut [Tue, 15 Nov 2022 12:50:27 +0000 (13:50 +0100)]
psql: Add command to use extended query protocol

This adds a new psql command \bind that sets query parameters and
causes the next query to be sent using the extended query protocol.
Example:

    SELECT $1, $2 \bind 'foo' 'bar' \g

This may be useful for psql scripting, but one of the main purposes is
also to be able to test various aspects of the extended query protocol
from psql and to write tests more easily.

Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/e8dd1cd5-0e04-3598-0518-a605159fe314@enterprisedb.com

2 years agolibpq error message refactoring, part 2
Peter Eisentraut [Tue, 15 Nov 2022 10:50:04 +0000 (11:50 +0100)]
libpq error message refactoring, part 2

This applies the new APIs to the code.

Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://www.postgresql.org/message-id/flat/7c0232ef-7b44-68db-599d-b327d0640a77@enterprisedb.com

2 years agolibpq error message refactoring
Peter Eisentraut [Tue, 15 Nov 2022 10:50:04 +0000 (11:50 +0100)]
libpq error message refactoring

libpq now contains a mix of error message strings that end with
newlines and don't end with newlines, due to some newer code paths
with new ways of passing errors around.  This leads to confusion and
mistakes both during development and translation.

This adds new functions libpq_append_error() and
libpq_append_conn_error() that encapsulate common code paths for
producing error message strings.  Notably, these functions append the
newline, so that the string appearing in the code does not end with a
newline.  This makes (almost) all error message strings in libpq
uniform in this regard (and also consistent with how we handle it
outside of libpq code).  (There are a few exceptions that are
difficult to fit into this scheme, but they are only a few.)

Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://www.postgresql.org/message-id/flat/7c0232ef-7b44-68db-599d-b327d0640a77@enterprisedb.com

2 years agoDisallow setting archive_library and archive_command at the same time
Peter Eisentraut [Tue, 15 Nov 2022 09:03:12 +0000 (10:03 +0100)]
Disallow setting archive_library and archive_command at the same time

Setting archive_library and archive_command at the same time is now an
error.  Before, archive_library would take precedence over
archive_command.

Author: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Discussion: https://www.postgresql.org/message-id/20220914222736.GA3042279%40nathanxps13

2 years agoImprove comments referring snapshot's subxip array.
Amit Kapila [Tue, 15 Nov 2022 04:07:19 +0000 (09:37 +0530)]
Improve comments referring snapshot's subxip array.

It was referred to as subxact array in a few places and subxip array in
others. By changing it to subxip array, we make it consistent with similar
references to xip array.

Author: Japin Li
Reviewd by: Julien Rouhaud, Richard Guo
Discussion: https://postgr.es/m/MEYP282MB1669DCE7AC193A947CED2A95B6009@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM

2 years agoFix cleanup lock acquisition in SPLIT_ALLOCATE_PAGE replay.
Amit Kapila [Mon, 14 Nov 2022 05:13:33 +0000 (10:43 +0530)]
Fix cleanup lock acquisition in SPLIT_ALLOCATE_PAGE replay.

During XLOG_HASH_SPLIT_ALLOCATE_PAGE replay, we were checking for a
cleanup lock on the new bucket page after acquiring an exclusive lock on
it and raising a PANIC error on failure. However, it is quite possible
that checkpointer can acquire the pin on the same page before acquiring a
lock on it, and then the replay will lead to an error. So instead, directly
acquire the cleanup lock on the new bucket page during
XLOG_HASH_SPLIT_ALLOCATE_PAGE replay operation.

Reported-by: Andres Freund
Author: Robert Haas
Reviewed-By: Amit Kapila, Andres Freund, Vignesh C
Backpatch-through: 11
Discussion: https://postgr.es/m/20220810022617.fvjkjiauaykwrbse@awork3.anarazel.de

2 years agoAdd error context callback when tokenizing authentication files
Michael Paquier [Mon, 14 Nov 2022 02:58:10 +0000 (11:58 +0900)]
Add error context callback when tokenizing authentication files

The parsing of the authentication files for HBA and ident entries
happens in two phases:
- Tokenization of the files, creating a list of TokenizedAuthLines.
- Validation of the HBA and ident entries, building a set of HbaLines or
IdentLines.

The second phase doing the validation provides already some error
context about the configuration file and the line where a problem
happens, but there is no such information in the first phase when
tokenizing the files.  This commit adds an ErrorContextCallback in
tokenize_auth_file(), with a context made of the line number and the
configuration file name involved in a problem.  This is useful for files
included in an HBA file for user and database lists, and it will become
much more handy to track problems for files included via a potential
@include[_dir,_if_exists].

The error context is registered so as the full chain of events is
reported when using cascaded inclusions when for example
tokenize_auth_file() recurses over itself on new files, displaying one
context line for each file gone through when tokenizing things.

Author: Michael Paquier
Reviewed-by: Julien Rouhaud
Discussion: https://postgr.es/m/Y2xUBJ+S+Z0zbxRW@paquier.xyz

2 years agoInvent open_auth_file() in hba.c to refactor authentication file opening
Michael Paquier [Mon, 14 Nov 2022 01:21:42 +0000 (10:21 +0900)]
Invent open_auth_file() in hba.c to refactor authentication file opening

This adds a check on the recursion depth when including authentication
configuration files, something that has never been done when processing
'@' files for database and user name lists in pg_hba.conf.  On HEAD,
this was leading to a rather confusing error, as of:
FATAL:  exceeded maxAllocatedDescs (NN) while trying to open file "/path/blah.conf"

This refactors the code so as the error reported is now the following,
which is the same as for GUCs:
FATAL: could not open file "/path/blah.conf": maximum nesting depth exceeded

This reduces a bit the verbosity of the error message used for files
included in user and database lists, reporting only the file name of
what's failing to load, without mentioning the relative or absolute path
specified after '@' in a HBA file.  The absolute path is built upon what
'@' defines anyway, so there is no actual loss of information.  This
makes the future inclusion logic much simpler.  A follow-up patch will
add an error context to be able to track on which line of which file the
inclusion is failing, to close the loop, providing all the information
needed to know the full chain of events.

This logic has been extracted from a larger patch written by Julien,
rewritten by me to have a unique code path calling AllocateFile() on
authentication files, and is useful on its own.  This new interface
will be used later for authentication files included with
@include[_dir,_if_exists], in a follow-up patch.

Author: Michael Paquier, Julien Rouhaud
Discussion: https://www.postgresql.org/message-id/Y2xUBJ+S+Z0zbxRW@paquier.xyz

2 years agolibpq: Add missing newlines to error messages
Peter Eisentraut [Sun, 13 Nov 2022 20:09:09 +0000 (21:09 +0100)]
libpq: Add missing newlines to error messages

2 years agolibpq: Remove unneeded cast and adjust format placeholder
Peter Eisentraut [Sun, 13 Nov 2022 20:09:05 +0000 (21:09 +0100)]
libpq: Remove unneeded cast and adjust format placeholder

2 years agoMake Bitmapsets be valid Nodes.
Tom Lane [Sun, 13 Nov 2022 15:22:45 +0000 (10:22 -0500)]
Make Bitmapsets be valid Nodes.

Add a NodeTag field to struct Bitmapset.  This is free because of
alignment considerations on 64-bit hardware.  While it adds some
space on 32-bit machines, we aren't optimizing for that case anymore.
The advantage is that data structures such as Lists of Bitmapsets
are now first-class objects to the Node infrastructure, and don't
require special-case code to handle.

This patch includes removal of one such special case, in indxpath.c:
bms_equal_any() can now be replaced by list_member().  There may be
more existing code that could be simplified, but I didn't look very
hard.  We also get to drop the read_write_ignore annotations on a
couple of RelOptInfo fields.

The outfuncs/readfuncs support is arranged so that nothing changes
in the string representation of a Bitmapset field; therefore, this
doesn't need a catversion bump.

Amit Langote and Tom Lane

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