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
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
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
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
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.
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
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
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
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
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
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
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
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
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.
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.
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
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
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.
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-
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
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.)
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.
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
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
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.
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.
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.
Alvaro Herrera [Fri, 18 Nov 2022 11:52:44 +0000 (12:52 +0100)]
Fix typo
Erik Rijkers
Peter Eisentraut [Fri, 18 Nov 2022 11:46:07 +0000 (12:46 +0100)]
doc: Small wording improvement
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
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
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
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
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
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.
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
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
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
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
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
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
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
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
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.
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
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
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
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
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
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.
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
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
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.
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
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
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
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
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 (
1ff4161,
63110c6 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
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
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
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
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
Jeff Davis [Tue, 15 Nov 2022 23:58:12 +0000 (15:58 -0800)]
Mark argument of RegisterCustomRmgr() as const.
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
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
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
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
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
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
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
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
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
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
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
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
Peter Eisentraut [Sun, 13 Nov 2022 20:09:09 +0000 (21:09 +0100)]
libpq: Add missing newlines to error messages
Peter Eisentraut [Sun, 13 Nov 2022 20:09:05 +0000 (21:09 +0100)]
libpq: Remove unneeded cast and adjust format placeholder
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
Andrew Dunstan [Sun, 13 Nov 2022 14:07:53 +0000 (09:07 -0500)]
Use installed postgresql.conf.sample for GUC sanity TAP test
The current code looks for the sample file in the source directory, but
it seems better to test against the installed sample file.
Backpatch to release 15 where the test was introduced.
Discussion: https://postgr.es/m/
73eea68e-3b6f-5f63-6024-
25ed26b52016@dunslane.net
Reviewed by Tom Lane, Alvaro Herrera, Michael Paquier.
Andrew Dunstan [Sun, 13 Nov 2022 13:45:14 +0000 (08:45 -0500)]
Make PostgreSQL::Test::Cluster::config_data more flexible
Currently this only allows for one argument, which must be present, and
always returns a single string. With this change the following now all
work:
$all_config = $node->config_data;
%config_map = ($node->config_data);
$incdir = $node->config_data('--include-dir');
($incdir, $sharedir) = $node->config_data(
qw(--include-dir --share-dir));
Backpatch to release 15 where this was introduced.
Discussion: https://postgr.es/m/
73eea68e-3b6f-5f63-6024-
25ed26b52016@dunslane.net
Reviewed by Tom Lane, Alvaro Herrera, Michael Paquier.
Peter Eisentraut [Sun, 13 Nov 2022 07:11:17 +0000 (08:11 +0100)]
Refactor aclcheck functions
Instead of dozens of mostly-duplicate pg_foo_aclcheck() functions,
write one common function object_aclcheck() that can handle almost all
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.
There are a few pg_foo_aclcheck() that don't work via the generic
function and have special APIs, so those stay as is.
I also changed most pg_foo_aclmask() functions to static functions,
since they are not used outside of aclchk.c.
Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Reviewed-by: Antonin Houska <ah@cybertec.at>
Discussion: https://www.postgresql.org/message-id/flat/
95c30f96-4060-2f48-98b5-
a4392d3b6066@enterprisedb.com
Peter Eisentraut [Sun, 13 Nov 2022 07:11:17 +0000 (08:11 +0100)]
Refactor ownercheck functions
Instead of dozens of mostly-duplicate pg_foo_ownercheck() functions,
write one common function object_ownercheck() that can handle almost
all 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 owner column.
Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Reviewed-by: Antonin Houska <ah@cybertec.at>
Discussion: https://www.postgresql.org/message-id/flat/
95c30f96-4060-2f48-98b5-
a4392d3b6066@enterprisedb.com
Peter Eisentraut [Sat, 12 Nov 2022 19:31:27 +0000 (20:31 +0100)]
Add repalloc0 and repalloc0_array
These zero out the space added by repalloc. This is a common pattern
that is quite hairy to code by hand.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/
b66dfc89-9365-cb57-4e1f-
b7d31813eeec@enterprisedb.com
Noah Misch [Sat, 12 Nov 2022 19:19:50 +0000 (11:19 -0800)]
If wait_for_catchup fails under has_wal_read_bug, skip balance of test.
Test files should now ignore has_wal_read_bug() so long as
wait_for_catchup() is their only known way of reaching the bug. That's
at least five files today, a number expected to grow over time. This
commit removes skip logic from three. By doing so, systems having the
bug regain the ability to catch other kinds of defects via those three
tests. The other two, 002_databases.pl and 031_recovery_conflict.pl,
have been unprotected. Back-patch to v15, where done_testing() first
became our standard.
Discussion: https://postgr.es/m/
20221030031639.GA3082137@rfd.leadboat.com
Tom Lane [Sat, 12 Nov 2022 18:29:41 +0000 (13:29 -0500)]
Fix volatility marking of timestamptz_trunc_zone.
It's safe to mark this as immutable, because it does not depend
on the timezone GUC setting. Oversight in commit
600b04d6b.
(There's an argument that timezone definitions do change from
time to time, but we have not worried about that in marking
other timestamp-related functions; for example AT TIME ZONE
has always been considered immutable. The situation is no
worse than our problems with time-varying locales, surely.)
Przemysław Sztoch
Discussion: https://postgr.es/m/
eaa3fabe-50fc-bbe8-b096-
ce62ddadab85@sztoch.pl
Jeff Davis [Sat, 12 Nov 2022 16:37:50 +0000 (08:37 -0800)]
Document WAL rules related to PD_ALL_VISIBLE in README.
Also improve comments.
Discussion: https://postgr.es/m/
a50005c1c537f89bb359057fd70e66bb83bce969.camel@j-davis.com
Reviewed-by: Peter Geoghegan
Jeff Davis [Thu, 10 Nov 2022 22:46:30 +0000 (14:46 -0800)]
Fix theoretical torn page hazard.
The original report was concerned with a possible inconsistency
between the heap and the visibility map, which I was unable to
confirm. The concern has been retracted.
However, there did seem to be a torn page hazard when using
checksums. By not setting the heap page LSN during redo, the
protections of minRecoveryPoint were bypassed. Fixed, along with a
misleading comment.
It may have been impossible to hit this problem in practice, because
it would require a page tear between the checksum and the flags, so I
am marking this as a theoretical risk. But, as discussed, it did
violate expectations about the page LSN, so it may have other
consequences.
Backpatch to all supported versions.
Reported-by: Konstantin Knizhnik
Reviewed-by: Konstantin Knizhnik
Discussion: https://postgr.es/m/
fed17dac-8cb8-4f5b-d462-
1bb4908c029e@garret.ru
Backpatch-through: 11
Jeff Davis [Fri, 11 Nov 2022 16:40:01 +0000 (08:40 -0800)]
Remove obsolete comments and code from prior to
f8f4227976.
XLogReadBufferForRedo() and XLogReadBufferForRedoExtended() only return
BLK_NEEDS_REDO if the record LSN is greater than the page LSN, so
the redo routine doesn't need to do the LSN check again.
Discussion: https://postgr.es/m/
0c37b80e62b1f3007d5a6d1292bd8fa0c275627a.camel@j-davis.com
Peter Eisentraut [Fri, 11 Nov 2022 15:00:48 +0000 (16:00 +0100)]
meson: Define HAVE_LOCALE_T for msvc
Meson doesn't see the redefinition of locale_t done in
src/include/port/win32_port.h, so it is not defining HAVE_LOCALE_T,
HAVE_WCSTOMBS_L nor HAVE_MBSTOWCS_L as the current
src/tools/msvc/build.pl script does. Add manual overrides to fix.
Author: Author: Juan Jose Santamaria Flecha <juanjo.santamaria@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAC%2BAXB1wJEqfKCuVcNpoH%3Dgxd61N%3D7c2fR3Ew6YRPpSfEUA%3DyQ%40mail.gmail.com
Tom Lane [Thu, 10 Nov 2022 23:20:49 +0000 (18:20 -0500)]
Support writing "CREATE/ALTER TABLE ... SET STORAGE DEFAULT".
We already allow explicitly writing DEFAULT for SET COMPRESSION,
so it seems a bit inflexible and non-orthogonal to not have it
for STORAGE.
Aleksander Alekseev
Discussion: https://postgr.es/m/CAJ7c6TMX9ui+6y3TQFaXJYVpZyBukvqhQbVDJ8OUokeLRhtnpA@mail.gmail.com
Tom Lane [Thu, 10 Nov 2022 22:24:26 +0000 (17:24 -0500)]
Fix alter_table.sql test case to test what it claims to.
The stanza "SET STORAGE may need to add a TOAST table" does not
test what it's supposed to, and hasn't done so since we added
the ability to store constant column default values as metadata.
We need to use a non-constant default to get the expected table
rewrite to actually happen.
Fix that, and add the missing checks that would have exposed the
problem to begin with.
Noted while reviewing a patch that made changes in this test case.
Back-patch to v11 where the problem came in.
Amit Kapila [Thu, 10 Nov 2022 11:26:49 +0000 (16:56 +0530)]
Fix comments atop ReorderBufferAddInvalidations.
The comments atop seem to indicate that we always accumulate invalidation
messages in a top-level transaction which is neither required nor matches
with the code.
Author: Amit Kapila
Reviewd by: Masahiko Sawada
Backpatch-through: 14, where it was introduced in commit
c55040ccd0
Discussion: https://postgr.es/m/CAA4eK1LxGgnUroPz8STb6OfjVU1yaHoSA+T63URwmGCLdMJ0LA@mail.gmail.com
Michael Paquier [Thu, 10 Nov 2022 07:32:29 +0000 (16:32 +0900)]
Fix comment of SimpleLruInit() in slru.c
sync_handler was not mentioned in the comment block of the function.
Oversight in
dee663f.
Author: Aleksander Alekseev
Discussion: https://postgr.es/m/CAJ7c6TPUd9BwNY47TtMxaijLHSbyHNdhu=kvbGnvO_bi+oC6_Q@mail.gmail.com
Backpatch-through: 14
Tom Lane [Wed, 9 Nov 2022 19:15:38 +0000 (14:15 -0500)]
Apply a better fix to mdunlinkfork().
Replace the stopgap fix I made in
0e758ae89 with a cleaner one.
The real problem with
4ab5dae94 is that it contorted this function's
logic substantially, by introducing a third code path that required
different behavior in the function's main loop. That seems quite
unnecessary on closer inspection: the new IsBinaryUpgrade case can
just share the behavior of the other immediate-unlink cases. Hence,
revert
4ab5dae94 and most of
0e758ae89 (keeping the latter's
save/restore errno fix), and add IsBinaryUpgrade to the set of
conditions tested to choose immediate unlink.
Also fix some additional places with sloppy handling of errno,
to ensure we have an invariant that we always continue processing
after any non-ENOENT failure of do_truncate. I doubt that that's
fixing any bug of field importance, so I don't feel it necessary to
back-patch; but we might as well get it right while we're here.
Also improve the comments, which had drifted a bit from what the
code actually does, and neglected to mention some important
considerations.
Back-patch to v15, not because this is fixing any bug but because
it doesn't seem like a good idea for v15's mdunlinkfork logic to be
significantly different from both v14 and v16.
Discussion: https://postgr.es/m/
3797575.
1667924888@sss.pgh.pa.us
Alvaro Herrera [Wed, 9 Nov 2022 17:27:31 +0000 (18:27 +0100)]
Remove redundant declaration for XidInMVCCSnapshot
This was added for no good reason by
c91560defc57, after
b7eda3e0e334
had just moved the prototype from utils/tqual.h to utils/snapmgr.h.
Author: Japin Li <japinli@hotmail.com>
Discussion: https://postgr.es/m/MEYP282MB16693A409F3282A9DB287BADB63E9@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
Tom Lane [Wed, 9 Nov 2022 17:28:34 +0000 (12:28 -0500)]
Report a more useful error for reloptions on a partitioned table.
Previously, trying to set storage parameters on a partitioned table
always led to "unrecognized parameter foo", because the code expected
there might be some valid parameters; but there aren't any. The docs
make clear that it's intended that there never will be any, so let's
replace this useless search with a more to-the-point message.
Simon Riggs and Karina Litskevich
Discussion: https://postgr.es/m/CANbhV-H=eZ9kTR9mUgKGK0Qv9uXP=U+dQg3rinQHfTdFMhBA2A@mail.gmail.com
Tom Lane [Wed, 9 Nov 2022 16:08:52 +0000 (11:08 -0500)]
Doc: add comments about PreventInTransactionBlock/IsInTransactionBlock.
Add a little to the header comments for these functions to make it
clearer what guarantees about commit behavior are provided to callers.
(See commit
f92944137 for context.)
Although this is only a comment change, it's really documentation
aimed at authors of extensions, so it seems appropriate to back-patch.
Yugo Nagata and Tom Lane, per further discussion of bug #17434.
Discussion: https://postgr.es/m/17434-
d9f7a064ce2a88a3@postgresql.org
Thomas Munro [Wed, 9 Nov 2022 00:05:16 +0000 (13:05 +1300)]
Provide sigaction() for Windows.
Commit
9abb2bfc left behind code to block signals inside signal
handlers on Windows, because our signal porting layer didn't have
sigaction(). Provide a minimal implementation that is capable of
blocking signals, to get rid of platform differences. See also related
commit
c94ae9d8.
Discussion: https://postgr.es/m/CA%2BhUKGKKKfcgx6jzok9AYenp2TNti_tfs8FMoJpL8%2B0Gsy%3D%3D_A%40mail.gmail.com
Michael Paquier [Tue, 8 Nov 2022 23:47:02 +0000 (08:47 +0900)]
Use AbsoluteConfigLocation() when building an included path in hba.c
The code building an absolute path to a file included, as prefixed by
'@' in authentication files, for user and database lists uses the same
logic as for GUCs, except that it has no need to know about DataDir as
there is always a calling file to rely to build the base directory path.
The refactoring done in
a1a7bb8 makes this move straight-forward, and
unifies the code used for GUCs and authentication files, and the
intention is to rely also on that for the upcoming patch to be able to
include full files from HBA or ident files.
Note that this gets rid of an inconsistency introduced in
370f909, that
copied the logic coming from GUCs but applied it for files included in
authentication files, where the result buffer given to
join_path_components() must have a size of MAXPGPATH. Based on a
double-check of the existing code, all the other callers of
join_path_components() already do that, except the code path changed
here.
Discussion: https://postgr.es/m/Y2igk7q8OMpg+Yta@paquier.xyz
Tom Lane [Tue, 8 Nov 2022 23:25:03 +0000 (18:25 -0500)]
Doc: improve tutorial section about grouped aggregates.
Commit
fede15417 introduced FILTER by jamming it into the existing
example introducing HAVING, which seems pedagogically poor to me;
and it added no information about what the keyword actually does.
Not to mention that the claimed output didn't match the sample
data being used in this running example.
Revert that and instead make an independent example using FILTER.
To help drive home the point that it's a per-aggregate filter,
we need to use two aggregates not just one; for consistency
expand all the examples in this segment to do that.
Also adjust the example using WHERE ... LIKE so that it'd produce
nonempty output with this sample data, and show that output.
Back-patch, as the previous patch was. (Sadly, v10 is now out
of scope.)
Discussion: https://postgr.es/m/
166794307526.652.
9073408178177444190@wrigleys.postgresql.org