Tom Lane [Wed, 29 Jan 2025 20:31:55 +0000 (15:31 -0500)]
Handle default NULL insertion a little better.
If a column is omitted in an INSERT, and there's no column default,
the code in preptlist.c generates a NULL Const to be inserted.
Furthermore, if the column is of a domain type, we wrap the Const
in CoerceToDomain, so as to throw a run-time error if the domain
has a NOT NULL constraint. That's fine as far as it goes, but
there are two problems:
1. We're being sloppy about the type/typmod that the Const is
labeled with. It really should have the domain's base type/typmod,
since it's the input to CoerceToDomain not the output. This can
result in coerce_to_domain inserting a useless length-coercion
function (useless because it's being applied to a null). The
coercion would typically get const-folded away later, but it'd
be better not to create it in the first place.
2. We're not applying expression preprocessing (specifically,
eval_const_expressions) to the resulting expression tree.
The planner's primary expression-preprocessing pass already happened,
so that means the length coercion step and CoerceToDomain node miss
preprocessing altogether.
This is at the least inefficient, since it means the length coercion
and CoerceToDomain will actually be executed for each inserted row,
though they could be const-folded away in most cases. Worse, it
seems possible that missing preprocessing for the length coercion
could result in an invalid plan (for example, due to failing to
perform default-function-argument insertion). I'm not aware of
any live bug of that sort with core datatypes, and it might be
unreachable for extension types as well because of restrictions of
CREATE CAST, but I'm not entirely convinced that it's unreachable.
Hence, it seems worth back-patching the fix (although I only went
back to v14, as the patch doesn't apply cleanly at all in v13).
There are several places in the rewriter that are building null
domain constants the same way as preptlist.c. While those are
before the planner and hence don't have any reachable bug, they're
still applying a length coercion that will be const-folded away
later, uselessly wasting cycles. Hence, make a utility routine
that all of these places can call to do it right.
Making this code more careful about the typmod assigned to the
generated NULL constant has visible but cosmetic effects on some
of the plans shown in contrib/postgres_fdw's regression tests.
Discussion: https://postgr.es/m/
1865579.
1738113656@sss.pgh.pa.us
Backpatch-through: 14
Tom Lane [Wed, 29 Jan 2025 19:24:36 +0000 (14:24 -0500)]
Avoid breaking SJIS encoding while de-backslashing Windows paths.
When running on Windows, canonicalize_path() converts '\' to '/'
to prevent confusing the Windows command processor. It was
doing that in a non-encoding-aware fashion; but in SJIS there
are valid two-byte characters whose second byte matches '\'.
So encoding corruption ensues if such a character is used in
the path.
We can fairly easily fix this if we know which encoding is
in use, but a lot of our utilities don't have much of a clue
about that. After some discussion we decided we'd settle for
fixing this only in psql, and assuming that its value of
client_encoding matches what the user is typing.
It seems hopeless to get the server to deal with the problematic
characters in database path names, so we'll just declare that
case to be unsupported. That means nothing need be done in
the server, nor in utility programs whose only contact with
file path names is for database paths. But psql frequently
deals with client-side file paths, so it'd be good if it
didn't mess those up.
Bug: #18735
Reported-by: Koichi Suzuki <koichi.suzuki@enterprisedb.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Koichi Suzuki <koichi.suzuki@enterprisedb.com>
Discussion: https://postgr.es/m/18735-
4acdb3998bb9f2b1@postgresql.org
Backpatch-through: 13
Tom Lane [Wed, 29 Jan 2025 18:23:31 +0000 (13:23 -0500)]
Make BufferIsExclusiveLocked and BufferIsDirty work for local buffers.
These functions tried to check the state of the buffer's content lock
even for local buffers. Since we don't use the content lock for a
local buffer, that would lead to a "false" result from
LWLockHeldByMeInMode, which would mean a misleading "false" answer
from BufferIsExclusiveLocked (we'd rather that case always return
"true") or an assertion failure in BufferIsDirty.
The core code never applies these two functions to local buffers,
and apparently no extensions do either, since we've not heard
complaints. Still, in the name of future-proofing, let's fix
them to act as though a pinned local buffer is content-locked.
Author: Srinath Reddy <srinath2133@gmail.com>
Discussion: https://postgr.es/m/
19396ef77f8.
1098c4a1810508.
2255483659262451647@zohocorp.com
John Naylor [Wed, 29 Jan 2025 07:28:20 +0000 (14:28 +0700)]
Fix grammatical typos around possessive "its"
Some places spelled it "it's", which is short for "it is".
In passing, fix a couple other nearby grammatical errors.
Author: Jacob Brazeal <jacob.brazeal@gmail.com>
Discussion: https://postgr.es/m/CA+COZaAO8g1KJCV0T48=CkJMjAnnfTGLWOATz+2aCh40c2Nm+g@mail.gmail.com
John Naylor [Wed, 29 Jan 2025 06:35:43 +0000 (13:35 +0700)]
Revert "Speed up tail processing when hashing aligned C strings, take two"
This reverts commit
a365d9e2e8c1ead27203a4431211098292777d3b.
Older versions of Valgrind raise an error, so go back to the bytewise
loop for the final word in the input.
Reported-by: Anton A. Melnikov <a.melnikov@postgrespro.ru>
Discussion: https://postgr.es/m/
a3a959f6-14b8-4819-ac04-
eaf2aa2e868d@postgrespro.ru
Backpatch-through: 17
Michael Paquier [Tue, 28 Jan 2025 23:49:48 +0000 (08:49 +0900)]
Improve test coverage of network address functions
The following functions were not covered by any tests:
- abbrev(inet)
- set_masklen(cidr)
- set_masklen(inet)
- netmask(inet)
- hostmask(inet)
While on it, this improves the output of some of the existing queries in
the test inet to use better aliases.
Author: Aleksander Alekseev
Reviewed-by: Jacob Champion, Keisuke Kuroda, Tom Lane
Discussion: https://postgr.es/m/CAJ7c6TOyZ9bGNrDK6Z3Q0gr9ow8ZpOm+=+01mpE0dsdH4C+u9A@mail.gmail.com
Amit Kapila [Tue, 28 Jan 2025 05:12:46 +0000 (10:42 +0530)]
Rename pubgencols_type to pubgencols in pg_publication.
The column added in commit
e65dbc9927, pubgencols_type, was inconsistent
with the naming conventions of other columns in the pg_publication
catalog.
Author: Vignesh C <vignesh21@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Discussion: https://postgr.es/m/CALDaNm1u-ufVOW-RUsXSooqzkpohxfZYy=z78fbcr_9Pq5hbCg@mail.gmail.com
Michael Paquier [Tue, 28 Jan 2025 00:57:32 +0000 (09:57 +0900)]
Track per-relation cumulative time spent in [auto]vacuum and [auto]analyze
This commit adds four fields to the statistics of relations, aggregating
the amount of time spent for each operation on a relation:
- total_vacuum_time, for manual vacuum.
- total_autovacuum_time, for vacuum done by the autovacuum daemon.
- total_analyze_time, for manual analyze.
- total_autoanalyze_time, for analyze done by the autovacuum daemon.
This gives users the option to derive the average time spent for these
operations with the help of the related "count" fields.
Bump catalog version (for the catalog changes) and PGSTAT_FILE_FORMAT_ID
(for the additions in PgStat_StatTabEntry).
Author: Sami Imseih
Reviewed-by: Bertrand Drouvot, Michael Paquier
Discussion: https://postgr.es/m/CAA5RZ0uVOGBYmPEeGF2d1B_67tgNjKx_bKDuL+oUftuoz+=Y1g@mail.gmail.com
Peter Eisentraut [Mon, 27 Jan 2025 11:02:00 +0000 (12:02 +0100)]
doc: Meson is not experimental on Windows
The installation documentation stated that using Meson is
experimental. But since this is the only way to build using Visual
Studio on Windows, this would imply that that whole build procedure is
experimental, which isn't true. So qualify this statement a bit more.
We keep the statement that Meson is experimental on other platforms,
since it doesn't have full, confirmed feature parity with the make
build system.
Author: Aleksander Alekseev <aleksander@timescale.com>
Discussion: https://www.postgresql.org/message-id/flat/
a3e76618-4cb5-4d54-a71c-
da4fb8ba571b@eisentraut.org
Michael Paquier [Mon, 27 Jan 2025 04:51:23 +0000 (13:51 +0900)]
Print out error position for some ALTER TABLE ALTER COLUMN type
A ParseState exists in ATPrepAlterColumnType() since its introduction
in
077db40fa1f3, and it has never relied on a query string that could be
used to point at a location in the origin string on error.
The output of some regression tests are updated, showing the error
location where applicable. Six error strings are upgraded with the
error location.
Author: Jian He
Discussion: https://postgr.es/m/CACJufxGfbPfWLjcEz33G9eW_epDW0UDi2H05i9eSTPKGJ4rxSA@mail.gmail.com
Michael Paquier [Sun, 26 Jan 2025 23:00:03 +0000 (08:00 +0900)]
pg_amcheck: Fix test failure on Windows with non-existing role
For SSPI auth extra users need to be explicitly allowed, or we get
"SSPI authentication failed" instead of the expected "role does not
exist" error.
This report also means that the test has never worked on Windows since
its introduction in
9706092839db, because it has always bumped on an
authentication failure rather than an error about the role not existing.
Oversight in
eef4a33f62f7, that has added a pattern check on the error
generated by the command.
Per report from Tom Lane, via buildfarm member drongo.
Author: Dagfinn Ilmari Mannsåker
Reviewed-by: Andrew Dunstan
Discussion: https://postgr.es/m/379085.
1737734611@sss.pgh.pa.us
Noah Misch [Sun, 26 Jan 2025 17:39:05 +0000 (09:39 -0800)]
Test postmaster with program_options_handling_ok() et al.
Most executables already get that testing. To occupy the customary
001_basic.pl name, this renumbers the new-in-October tests of
src/test/postmaster/t.
Reviewed by Thomas Munro.
Discussion: https://postgr.es/m/
20241215022701.a1.nmisch@google.com
Álvaro Herrera [Sun, 26 Jan 2025 16:34:28 +0000 (17:34 +0100)]
Add missing CommandCounterIncrement
For commit
b663b9436e75 I thought this was useless, but turns out not to
be for the case where a partitioned table has two identical foreign key
constraints which can both be matched by the same constraint in a
partition during attach. This CCI makes the match search for the second
constraint in the parent ignore the constraint in the child that has
already been matched by the first constraint in the parent.
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/
c599253c-1ccd-4161-80fc-
c9065e037a09@gmail.com
Noah Misch [Sat, 25 Jan 2025 19:28:14 +0000 (11:28 -0800)]
At update of non-LP_NORMAL TID, fail instead of corrupting page header.
The right mix of DDL and VACUUM could corrupt a catalog page header such
that PageIsVerified() durably fails, requiring a restore from backup.
This affects only catalogs that both have a syscache and have DDL code
that uses syscache tuples to construct updates. One of the test
permutations shows a variant not yet fixed.
This makes !TransactionIdIsValid(TM_FailureData.xmax) possible with
TM_Deleted. I think core and PGXN are indifferent to that.
Per bug #17821 from Alexander Lakhin. Back-patch to v13 (all supported
versions). The test case is v17+, since it uses INJECTION_POINT.
Discussion: https://postgr.es/m/17821-
dd8c334263399284@postgresql.org
Noah Misch [Sat, 25 Jan 2025 19:28:14 +0000 (11:28 -0800)]
Merge copies of converting an XID to a FullTransactionId.
Assume twophase.c is the performance-sensitive caller, and preserve its
choice of unlikely() branch hint. Add some retrospective rationale for
that choice. Back-patch to v17, for the next commit to use it.
Reviewed (in earlier versions) by Michael Paquier.
Discussion: https://postgr.es/m/17821-
dd8c334263399284@postgresql.org
Discussion: https://postgr.es/m/
20250116010051.f3.nmisch@google.com
Noah Misch [Sat, 25 Jan 2025 19:28:14 +0000 (11:28 -0800)]
Disable runningcheck for src/test/modules/injection_points/specs.
Directory "injection_points" has specified NO_INSTALLCHECK since before
commit
c35f419d6efbdf1a050250d84b687e6705917711 added the specs, but
that commit neglected to disable the corresponding meson runningcheck.
The alternative would be to enable "make installcheck" for ISOLATION,
but the GNU make build system lacks a concept of setting NO_INSTALLCHECK
for REGRESS without also setting it for ISOLATION. Back-patch to v17,
where that commit first appeared, to avoid surprises when back-patching
additional specs.
Discussion: https://postgr.es/m/17821-
dd8c334263399284@postgresql.org
Noah Misch [Sat, 25 Jan 2025 19:28:14 +0000 (11:28 -0800)]
Test ECPG decadd(), decdiv(), decmul(), and decsub() for risnull() input.
Since commit
757fb0e5a9a61ac8d3a67e334faeea6dc0084b3f, these
Informix-compat functions return 0 without changing the output
parameter. Initialize the output parameter before the test call, making
that obvious. Before this, the expected test output has been depending
on freed stack memory. "gcc -ftrivial-auto-var-init=pattern" revealed
that. Back-patch to v13 (all supported versions).
Discussion: https://postgr.es/m/
20250106192748.cf.nmisch@google.com
Tom Lane [Sat, 25 Jan 2025 17:42:05 +0000 (12:42 -0500)]
Doc: recommend "psql -X" for restoring pg_dump scripts.
This practice avoids possible problems caused by non-default psql
options, such as disabling AUTOCOMMIT.
Author: Shinya Kato <Shinya11.Kato@oss.nttdata.com>
Reviewed-by: Robert Treat <rob@xzilla.net>
Discussion: https://postgr.es/m/
96ff23a5d858ff72ca8e823a014d16fe@oss.nttdata.com
Backpatch-through: 13
Andres Freund [Sat, 25 Jan 2025 16:37:13 +0000 (11:37 -0500)]
Change shutdown sequence to terminate checkpointer last
The main motivation for this change is to have a process that can serialize
stats after all other processes have terminated. Serializing stats already
happens in checkpointer, even though walsenders can be active longer.
The only reason the current shutdown sequence does not actively cause problems
is that walsender currently does not generate any stats. However, there is an
upcoming patch changing that.
Another need for this change originates in the AIO patchset, where IO
workers (which, in some edge cases, can emit stats of their own) need to run
while the shutdown checkpoint is being written.
This commit changes the shutdown sequence so checkpointer is signalled (via
SIGINT) to trigger writing the shutdown checkpoint without also causing
checkpointer to exit. Once checkpointer wrote the shutdown checkpoint it
notifies postmaster via PMSIGNAL_XLOG_IS_SHUTDOWN and waits for the
termination signal (SIGUSR2, as before). Checkpointer now is terminated after
all children, other than dead-end children and logger, have been terminated,
tracked using the new PM_WAIT_CHECKPOINTER PMState.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/kgng5nrvnlv335evmsuvpnh354rw7qyazl73kdysev2cr2v5zu@m3cfzxicm5kp
Tom Lane [Sat, 25 Jan 2025 16:24:16 +0000 (11:24 -0500)]
Tighten pg_restore's recognition of its -F (format) option values.
Instead of checking just the first letter, match the whole string
using pg_strcasecmp. Per the documentation, we allow either just
the first letter (e.g. "c") or the whole name ("custom"); but we
will no longer accept random variations such as "chump". This
matches pg_dump's longstanding parsing code for the same option.
Also for consistency with pg_dump, recognize "p"/"plain". We don't
support it, but we can give a more helpful error message than
"unrecognized archive format".
Author: Srinath Reddy <srinath2133@gmail.com>
Discussion: https://postgr.es/m/CAFC+b6pfK-BGcWW1kQmtxVrCh-JGjB2X02rLPQs_ZFaDGjZDsQ@mail.gmail.com
Jeff Davis [Sat, 25 Jan 2025 08:12:30 +0000 (00:12 -0800)]
Fix PDF doc build.
Reported-by: Tom Lane
Discussion: https://postgr.es/m/608525.
1737781222@sss.pgh.pa.us
Tomas Vondra [Fri, 24 Jan 2025 23:36:48 +0000 (00:36 +0100)]
Use the correct sizeof() in BufFileLoadBuffer
The sizeof() call should reference buffer.data, because that's the
buffer we're reading data into, not the whole PGAlignedBuffer union.
This was introduced by
44cac93464, which replaced the simple buffer
with a PGAlignedBuffer field.
It's benign, because the buffer is the largest field of the union, so
the sizes are the same. But it's easy to trip over this in a patch, so
fix and backpatch. Commit
44cac93464 went into 12, but that's EOL.
Backpatch-through: 13
Discussion: https://postgr.es/m/
928bdab1-6567-449f-98c4-
339cd2203b87@vondra.me
Jeff Davis [Fri, 24 Jan 2025 22:56:22 +0000 (14:56 -0800)]
Add SQL function CASEFOLD().
Useful for caseless matching. Similar to LOWER(), but avoids edge-case
problems with using LOWER() for caseless matching.
For collations that support it, CASEFOLD() handles characters with
more than two case variations or multi-character case variations. Some
characters may fold to uppercase. The results of case folding are also
more stable across Unicode versions than LOWER() or UPPER().
Discussion: https://postgr.es/m/
a1886ddfcd8f60cb3e905c93009b646b4cfb74c5.camel%40j-davis.com
Reviewed-by: Ian Lawrence Barwick
Andres Freund [Fri, 24 Jan 2025 22:00:10 +0000 (17:00 -0500)]
postmaster: Adjust which processes we expect to have exited
Comments and code stated that we expect checkpointer to have been signalled in
case of immediate shutdown / fatal errors, but didn't treat archiver and
walsenders the same. That doesn't seem right.
I had started digging through the history to see where this oddity was
introduced, but it's not the fault of a single commit.
Instead treat archiver, checkpointer, and walsenders the same.
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/kgng5nrvnlv335evmsuvpnh354rw7qyazl73kdysev2cr2v5zu@m3cfzxicm5kp
Andres Freund [Fri, 24 Jan 2025 22:08:31 +0000 (17:08 -0500)]
postmaster: Commonalize FatalError paths
This includes some behavioral changes:
- Previously PM_WAIT_XLOG_ARCHIVAL wasn't handled in HandleFatalError(), that
doesn't seem quite right.
- Previously a fatal error in PM_WAIT_XLOG_SHUTDOWN lead to jumping back to
PM_WAIT_BACKENDS, no we go to PM_WAIT_DEAD_END. Jumping backwards doesn't
seem quite right and we didn't do so when checkpointer failed to fork during
a shutdown.
- Previously a checkpointer fork failure didn't call SetQuitSignalReason(),
which would lead to quickdie() reporting
"terminating connection because of unexpected SIGQUIT signal"
which seems even worse than the PMQUIT_FOR_CRASH message. If I saw that in
the log I'd suspect somebody outside of postgres sent SIGQUITs
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/kgng5nrvnlv335evmsuvpnh354rw7qyazl73kdysev2cr2v5zu@m3cfzxicm5kp
Andres Freund [Fri, 24 Jan 2025 22:00:10 +0000 (17:00 -0500)]
postmaster: Move code to switch into FatalError state into function
There are two places switching to FatalError mode, behaving somewhat
differently. An upcoming commit will introduce a third. That doesn't seem seem
like a good idea.
This commit just moves the FatalError related code from HandleChildCrash()
into its own function, a subsequent commit will evolve the state machine
change to be suitable for other callers.
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/kgng5nrvnlv335evmsuvpnh354rw7qyazl73kdysev2cr2v5zu@m3cfzxicm5kp
Andres Freund [Fri, 24 Jan 2025 22:00:10 +0000 (17:00 -0500)]
postmaster: Don't repeatedly transition to crashing state
Previously HandleChildCrash() skipped logging and signalling child exits if
already in an immediate shutdown or in FatalError state, but still
transitioned server state in response to a crash. That's redundant.
In the other place we transition to FatalError, we do take care to not do so
when already in FatalError state.
To make it easier to combine different paths for entering FatalError state,
only do so once in HandleChildCrash().
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/kgng5nrvnlv335evmsuvpnh354rw7qyazl73kdysev2cr2v5zu@m3cfzxicm5kp
Andres Freund [Fri, 24 Jan 2025 22:00:10 +0000 (17:00 -0500)]
postmaster: Don't open-code TerminateChildren() in HandleChildCrash()
After removing the duplication no user of sigquit_child() remains, therefore
remove it.
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/kgng5nrvnlv335evmsuvpnh354rw7qyazl73kdysev2cr2v5zu@m3cfzxicm5kp
Andres Freund [Fri, 24 Jan 2025 22:00:10 +0000 (17:00 -0500)]
checkpointer: Request checkpoint via latch instead of signal
The motivation for this change is that a future commit will use SIGINT for
another purpose (postmaster requesting WAL access to be shut down) and that
there no other signals that we could readily use (see code comment for the
reason why SIGTERM shouldn't be used). But it's also a tad nicer / more
efficient to use SetLatch(), as it avoids sending signals when checkpointer
already is busy.
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/kgng5nrvnlv335evmsuvpnh354rw7qyazl73kdysev2cr2v5zu@m3cfzxicm5kp
Tom Lane [Fri, 24 Jan 2025 18:20:44 +0000 (13:20 -0500)]
Make jsonb casts to scalar types translate JSON null to SQL NULL.
Formerly, these cases threw an error "cannot cast jsonb null to type
<whatever>". That seems less than helpful though. It's also
inconsistent with the behavior of the ->> operator, which translates
JSON null to SQL NULL, as do some other jsonb functions.
Discussion: https://postgr.es/m/
3851203.
1722552717@sss.pgh.pa.us
Peter Eisentraut [Fri, 24 Jan 2025 16:45:29 +0000 (17:45 +0100)]
Fix copy-and-paste typo
Daniel Gustafsson [Fri, 24 Jan 2025 13:25:08 +0000 (14:25 +0100)]
pgcrypto: Make it possible to disable built-in crypto
When using OpenSSL and/or the underlying operating system in FIPS
mode no non-FIPS certified crypto implementations should be used.
While that is already possible by just not invoking the built-in
crypto in pgcrypto, this adds a GUC which prohibit the code from
being called. This doesn't change the FIPS status of PostgreSQL
but can make it easier for sites which target FIPS compliance to
ensure that violations cannot occur.
Author: Daniel Gustafsson <daniel@yesql.se>
Author: Joe Conway <mail@joeconway.com>
Reviewed-by: Joe Conway <mail@joeconway.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Discussion: https://postgr.es/m/
16b4a157-9ea1-44d0-b7b3-
4c85df5de97b@joeconway.com
Daniel Gustafsson [Fri, 24 Jan 2025 13:18:40 +0000 (14:18 +0100)]
pgcrypto: Add function to check FIPS mode
This adds a SQL callable function for reading and returning the status
of FIPS configuration of OpenSSL. If OpenSSL is operating with FIPS
enabled it will return true, otherwise false. As this adds a function
to the SQL file, bump the extension version to 1.4.
Author: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Joe Conway <mail@joeconway.com>
Discussion: https://postgr.es/m/
8f979145-e206-475a-a31b-
73c977a4134c@joeconway.com
Álvaro Herrera [Fri, 24 Jan 2025 11:54:46 +0000 (12:54 +0100)]
Fix instability in recently added regression tests
We missed the usual ORDER BY clause.
Author: Amul Sul <amul.sul@enterprisedb.com>
Discussion: https://postgr.es/m/CAAJ_b974U3Vvf-qGwFyZ73DFHqyFJP9TOmuiXR2Kp8KVcJtP6w@mail.gmail.com
Peter Eisentraut [Fri, 24 Jan 2025 10:37:20 +0000 (11:37 +0100)]
Convert sepgsql tests to TAP
Add a TAP test for sepgsql. This automates the previously required
manual setup before the test. The actual tests are still run by
pg_regress, as before, but now called from within the TAP Perl script.
The previous manual test script (test_sepgsql) is left in place, since
its purpose is (also) to test whether a running instance was properly
initialized for sepgsql. But it has been changed to call pg_regress
directly and no longer require make.
Reviewed-by: Andreas Karlsson <andreas@proxel.se>
Discussion: https://www.postgresql.org/message-id/flat/
651a5baf-5c45-4a5a-a202-
0c8453a4ebf8@eisentraut.org
Peter Eisentraut [Fri, 24 Jan 2025 09:26:12 +0000 (10:26 +0100)]
meson: Fix sepgsql installation
The sepgsql.sql file should be installed under share/contrib/, not
share/extension/, since it is not an extension. This makes it match
what make install does.
Discussion: https://www.postgresql.org/message-id/flat/
651a5baf-5c45-4a5a-a202-
0c8453a4ebf8@eisentraut.org
Michael Paquier [Fri, 24 Jan 2025 06:19:38 +0000 (15:19 +0900)]
initdb: Convert tests to use long options with fat comma style
This is similar to
ce1b0f9da03e, but this time this rule is applied to
some of the TAP tests of initdb.
Author: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/878qr146ra.fsf@wibble.ilmari.org
Peter Eisentraut [Fri, 24 Jan 2025 05:55:39 +0000 (06:55 +0100)]
Return yyparse() result not via global variable
Instead of passing the parse result from yyparse() via a global
variable, pass it via a function output argument.
This complements earlier work to make the parsers reentrant.
Discussion: Discussion: https://www.postgresql.org/message-id/flat/
eb6faeac-2a8a-4b69-9189-
c33c520e5b7b@eisentraut.org
Amit Kapila [Fri, 24 Jan 2025 02:55:21 +0000 (08:25 +0530)]
Doc: Fix a typo introduced in
4a0e7314f1.
Author: Erik Rijkers <er@xs4all.nl>
Discussion: https://postgr.es/m/
6e625c81-968e-42d0-802d-
edfaf9cfac11@xs4all.nl
Amit Kapila [Fri, 24 Jan 2025 02:41:29 +0000 (08:11 +0530)]
Doc: Fix column name in pg_publication catalog.
Commit
e65dbc9927 incorrectly spelled the column name in the
pg_publication catalog. In passing make the order of columns in the doc
match the actual catalog.
Author: Shinoda, Noriyoshi <noriyoshi.shinoda@hpe.com>
Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Discussion: https://postgr.es/m/DM4PR84MB1734F8F140E4477580761F93EEE02@DM4PR84MB1734.NAMPRD84.PROD.OUTLOOK.COM
Tom Lane [Thu, 23 Jan 2025 19:23:04 +0000 (14:23 -0500)]
Don't ask for bug reports about pthread_is_threaded_np() != 0.
We thought that this condition was unreachable in ExitPostmaster,
but actually it's possible if you have both a misconfigured locale
setting and some other mistake that causes PostmasterMain to bail
out before reaching its own check of pthread_is_threaded_np().
Given the lack of other reports, let's not ask for bug reports if
this occurs; instead just give the same hint as in PostmasterMain.
Bug: #18783
Reported-by: anani191181515@gmail.com
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/18783-
d1873b95a59b9103@postgresql.org
Discussion: https://postgr.es/m/206317.
1737656533@sss.pgh.pa.us
Backpatch-through: 13
Tom Lane [Thu, 23 Jan 2025 17:25:45 +0000 (12:25 -0500)]
Ensure that AFTER triggers run as the instigating user.
With deferred triggers, it is possible that the current role changes
between the time when the trigger is queued and the time it is
executed (for example, the triggering data modification could have
been executed in a SECURITY DEFINER function).
Up to now, deferred trigger functions would run with the current role
set to whatever was active at commit time. That does not matter for
foreign-key constraints, whose correctness doesn't depend on the
current role. But for user-written triggers, the current role
certainly can matter.
Hence, fix things so that AFTER triggers are fired under the role
that was active when they were queued, matching the behavior of
BEFORE triggers which would have actually fired at that time.
(If the trigger function is marked SECURITY DEFINER, that of course
overrides this, as it always has.)
This does not create any new security exposure: if you do DML on a
table owned by a hostile user, that user has always had various ways
to exploit your permissions, such as the aforementioned BEFORE
triggers, default expressions, etc. It might remove some security
exposure, because the old behavior could potentially expose some
other role besides the one directly modifying the table.
There was discussion of making a larger change, such as running as
the trigger's owner. However, that would break the common idiom of
capturing the value of CURRENT_USER in a trigger for auditing/logging
purposes. This change will make no difference in the typical scenario
where the current role doesn't change before commit.
Arguably this is a bug fix, but it seems too big a semantic change
to consider for back-patching.
Author: Laurenz Albe <laurenz.albe@cybertec.at>
Reviewed-by: Joseph Koshakow <koshy44@gmail.com>
Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com>
Discussion: https://postgr.es/m/
77ee784cf248e842f74588418f55c2931e47bd78.camel@cybertec.at
Jeff Davis [Thu, 23 Jan 2025 17:06:50 +0000 (09:06 -0800)]
Add support for Unicode case folding.
Expand case mapping tables to include entries for case folding, which
are parsed from CaseFolding.txt.
Discussion: https://postgr.es/m/
a1886ddfcd8f60cb3e905c93009b646b4cfb74c5.camel%40j-davis.com
Tom Lane [Thu, 23 Jan 2025 16:08:05 +0000 (11:08 -0500)]
Reverse the search order in afterTriggerAddEvent().
When scanning existing AfterTriggerSharedData records in search
of a match to the event being queued, we were examining the
records from oldest to newest. But it makes more sense to do
the opposite. The newest record is likely to be from the current
query, while the oldest is likely to be from some previous command
in the same transaction, which will likely have different details.
There aren't expected to be very many active AfterTriggerSharedData
records at once, so that this change is unlikely to make any
spectacular difference. Still, having added a nontrivially-expensive
bms_equal call to this loop yesterday, I feel a need to shave cycles
where possible.
Discussion: https://postgr.es/m/
4166712.
1737583961@sss.pgh.pa.us
Álvaro Herrera [Thu, 23 Jan 2025 14:54:38 +0000 (15:54 +0100)]
Allow NOT VALID foreign key constraints on partitioned tables
This feature was intentionally omitted when FKs were first implemented
for partitioned tables, and had been requested a few times; the
usefulness is clear.
Validation can happen for each partition individually, which is useful
to contain the number of locks held and the duration; or it can be
executed for the partitioning hierarchy as a single command, which
validates all child constraints that haven't been validated already.
This is also useful to implement NOT ENFORCED constraints on top.
Author: Amul Sul <sulamul@gmail.com>
Discussion: https://postgr.es/m/CAAJ_b96Bp=-ZwihPPtuaNX=SrZ0U6ZsXD3+fgARO0JuKa8v2jQ@mail.gmail.com
Amit Kapila [Thu, 23 Jan 2025 12:17:15 +0000 (17:47 +0530)]
Fix buildfarm failure introduced by commit
e65dbc9927.
The patch had incorrectly specified the default value for
publish_generated_columns during the query formation in pg_dump.
Author: Vignesh C <vignesh21@gmail.com>
Discussion: https://postgr.es/m/CAA4eK1KfZYTD8Hpi9TD1KaB8rNUBR9baUvTxa5wYyZDGbEaa6g@mail.gmail.com
Peter Eisentraut [Thu, 23 Jan 2025 11:07:38 +0000 (12:07 +0100)]
Convert macros to static inline functions (htup_details.h, itup.h)
Discussion: https://www.postgresql.org/message-id/flat/
5b558da8-99fb-0a99-83dd-
f72f05388517@enterprisedb.com
Peter Eisentraut [Thu, 23 Jan 2025 11:07:38 +0000 (12:07 +0100)]
Add some const decorations (htup.h)
Discussion: https://www.postgresql.org/message-id/flat/
5b558da8-99fb-0a99-83dd-
f72f05388517@enterprisedb.com
Amit Kapila [Thu, 23 Jan 2025 09:58:37 +0000 (15:28 +0530)]
Change publication's publish_generated_columns option type to enum.
The current boolean publish_generated_columns option only supports a
binary choice, which is insufficient for future enhancements where
generated columns can be of different types (e.g., stored or virtual). The
supported values for the publish_generated_columns option are 'none' and
'stored'.
Author: Vignesh C <vignesh21@gmail.com>
Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/
d718d219-dd47-4a33-bb97-
56e8fc4da994@eisentraut.org
Discussion: https://postgr.es/m/
B80D17B2-2C8E-4C7D-87F2-
E5B4BE3C069E@gmail.com
Michael Paquier [Thu, 23 Jan 2025 07:03:48 +0000 (16:03 +0900)]
Add error pattern checks for some TAP tests for non-existing objects
Some tests are updated to use command_fails_like(), gaining a check for
the error output generated. The test changed in pg_amcheck has come up
after noticing that an incorrect option name still made the test to
pass, while the command failed. The three other tests changed in
src/bin/scripts/ have been noticed by me, in passing.
Author: Dagfinn Ilmari Mannsåker, Michael Paquier
Discussion: https://postgr.es/m/87bjvy50cs.fsf@wibble.ilmari.org
Michael Paquier [Thu, 23 Jan 2025 06:15:36 +0000 (15:15 +0900)]
Improve TAP tests of pg_basebackup
This addresses some minor issues with the TAP tests of pg_basebackup:
- Remove three duplicated tests used for incorrect option combinations.
- Add more pattern checks for commands doomed to fail, to make sure that
the error generated is the expected one. These are for tests related to
the tablespace mapping and incorrect option combinations.
- Fix the description of one test for the case of backup target versus
format.
Issues noticed while reviewing this area of the tests.
Discussion: https://postgr.es/m/87bjvy50cs.fsf@wibble.ilmari.org
Tom Lane [Wed, 22 Jan 2025 20:18:40 +0000 (15:18 -0500)]
Support RN (roman-numeral format) in to_number().
We've long had roman-numeral output support in to_char(),
but lacked the reverse conversion. Here it is.
Author: Hunaid Sohail <hunaidpgml@gmail.com>
Reviewed-by: Maciek Sakrejda <m.sakrejda@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Tomas Vondra <tomas@vondra.me>
Discussion: https://postgr.es/m/CAMWA6ybh4M1VQqpmnu2tfSwO+3gAPeA8YKnMHVADeB=XDEvT_A@mail.gmail.com
Nathan Bossart [Wed, 22 Jan 2025 20:11:37 +0000 (14:11 -0600)]
Fix comment about AVX-512 popcount support.
Since commit
f78667bd91, we've used __attribute__((target(...)))
instead of extra compiler flags for AVX-512 support, but this
comment still says that we put the code in a separate file because
it might require extra compiler flags. Let's just remove that part
of the comment.
Tom Lane [Wed, 22 Jan 2025 16:58:20 +0000 (11:58 -0500)]
Repair incorrect handling of AfterTriggerSharedData.ats_modifiedcols.
This patch fixes two distinct errors that both ultimately trace
to commit
71d60e2aa, which added the ats_modifiedcols field.
The more severe error is that ats_modifiedcols wasn't accounted for
in afterTriggerAddEvent's scanning loop that looks for a pre-existing
duplicate AfterTriggerSharedData. Thus, a new event could be
incorrectly matched to an AfterTriggerSharedData that has a different
value of ats_modifiedcols, resulting in the wrong tg_updatedcols
bitmap getting passed to the trigger whenever it finally gets fired.
We'd not noticed because (a) few triggers consult tg_updatedcols,
and (b) we had no tests exercising a case where such a trigger was
called as an AFTER trigger. In the test case added by this commit,
contrib/lo's trigger fails to remove a large object when expected
because (without this fix) it thinks the LO OID column hasn't changed.
The other problem was introduced by commit
ce5aaea8c, which copied the
modified-columns bitmap into trigger-related storage. It made a copy
for every trigger event, whereas what we really want is to make a new
copy only when we make a new AfterTriggerSharedData entry. (We could
imagine adding extra logic to reduce the number of bitmap copies still
more, but it doesn't look worthwhile at the moment.) In a simple test
of an UPDATE of
10000000 rows with a single AFTER trigger, this thinko
roughly tripled the amount of memory consumed by the pending-triggers
data structures, from
160446744 to
480443440 bytes.
Fixing the first problem requires introducing a bms_equal() call into
afterTriggerAddEvent's scanning loop, which is slightly annoying from
a speed perspective. However, getting rid of the excessive bms_copy()
calls from the second problem balances that out; overall speed of
trigger operations is the same or slightly better, in my tests.
Discussion: https://postgr.es/m/
3496294.
1737501591@sss.pgh.pa.us
Backpatch-through: 13
Amit Kapila [Wed, 22 Jan 2025 09:57:37 +0000 (15:27 +0530)]
Fix \dRp+ output when describing publications with a lower server version.
The psql was not careful that the new column "Generated columns" won't be
present in the lower version. This was introduced in recent commit
7054186c4e.
Author: Vignesh C
Reviewed-by: Peter Smith
Discussion: https://postgr.es/m/CALDaNm3OcXdY0EzDEKAfaK9gq2B67Mfsgxu93+_249ohyts=0g@mail.gmail.com
Peter Eisentraut [Wed, 22 Jan 2025 06:32:21 +0000 (07:32 +0100)]
Additional tests for stored generated columns
Some additional tests have been created during the development of
virtual generated columns (not included here). This commit adds
equivalent tests to the existing test set for stored generated
columns. This includes expanded tests related to MERGE, subqueries,
whole-row references, permissions, domains, partitioning, and
triggers.
Author: Peter Eisentraut <peter@eisentraut.org>
Co-authored-by: jian he <jian.universality@gmail.com>
Co-authored-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/
a368248e-69e4-40be-9c07-
6c3b5880b0a6@eisentraut.org
Michael Paquier [Wed, 22 Jan 2025 05:47:13 +0000 (14:47 +0900)]
Improve grammar of options for command arrays in TAP tests
This commit rewrites a good chunk of the command arrays in TAP tests
with a grammar based on the following rules:
- Fat commas are used between option names and their values, making it
clear to both humans and perltidy that values and names are bound
together. This is particularly useful for the readability of multi-line
command arrays, and there are plenty of them in the TAP tests. Most of
the test code is updated to use this style. Some commands used
parenthesis to show the link, or attached values and options in a single
string. These are updated to use fat commas instead.
- Option names are switched to use their long names, making them more
self-documented. Based on a suggestion by Andrew Dunstan.
- Add some trailing commas after the last item in multi-line arrays,
which is a common perl style.
Not all the places are taken care of, but this covers a very good chunk
of them.
Author: Dagfinn Ilmari Mannsåker
Reviewed-by: Michael Paquier, Peter Smith, Euler Taveira
Discussion: https://postgr.es/m/87jzc46d8u.fsf@wibble.ilmari.org
Amit Kapila [Wed, 22 Jan 2025 05:24:53 +0000 (10:54 +0530)]
Doc: Update the interaction of tablesync with wal_retrieve_retry_interval.
In passing, update the documentation that explains the process of initial
data replication to explicitly state that it uses a table synchronization
worker.
Author: Vignesh C
Reviewed-by: Peter Smith, Shlok Kyal, Amit Kapila
Discussion: https://postgr.es/m/CALDaNm3RxGcD4cDAV5Q0_A4n06F3+AAMpxiyND9Zn0dB86hFmg@mail.gmail.com
Michael Paquier [Wed, 22 Jan 2025 01:13:59 +0000 (10:13 +0900)]
Run perltidy
A follow-up patch will adjust the TAP tests to follow a more-structured
format for option lists in commands, that perltidy is able to cope
better with. Putting the tree first in a clean state makes the next
change a bit easier. v20230309 has been used.
Author: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/87jzc46d8u.fsf@wibble.ilmari.org
Tom Lane [Tue, 21 Jan 2025 19:43:21 +0000 (14:43 -0500)]
Doc: simplify the tutorial's window-function examples.
For the purposes of this discussion, row_number() is just as good
as rank(), and its behavior is easier to understand and describe.
So let's switch the examples to using row_number().
Along the way to checking the results given in the tutorial,
I found it helpful to extract the empsalary table we use in the
regression tests, which is evidently the same data that was used
to make these results. So I shoved that into advanced.source
to improve the coverage of that file a little. (There's still
several pages of the tutorial that are not included in it,
but at least now 3.5 Window Functions is covered.)
Suggested-by: "David G. Johnston" <david.g.johnston@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/
173737973383.1070.
1832752929070067441@wrigleys.postgresql.org
Álvaro Herrera [Tue, 21 Jan 2025 14:24:49 +0000 (15:24 +0100)]
Reword recent error messages: "should" -> "must"
Most were introduced in the 17 timeframe. The ones in wparser_def.c are
very old.
I also changed "JSON path expression for column \"%s\" should return
single item without wrapper" to "JSON path expression for column \"%s\"
must return single item when no wrapper is requested" to avoid
ambiguity.
Backpatch to 17.
Crickets: https://postgr.es/m/
202501131819.26ors7oouafu@alvherre.pgsql
Álvaro Herrera [Tue, 21 Jan 2025 13:53:46 +0000 (14:53 +0100)]
Fix detach of a partition that has a toplevel FK to a partitioned table
In common cases, foreign keys are defined on the toplevel partitioned
table; but if instead one is defined on a partition and references a
partitioned table, and the referencing partition is detached, we would
examine the pg_constraint row on the partition being detached, and fail
to realize that the sub-constraints must be left alone. This causes the
ALTER TABLE DETACH process to fail with
ERROR: could not find ON INSERT check triggers of foreign key constraint NNN
This is similar but not quite the same as what was fixed by
53af9491a043. This bug doesn't affect branches earlier than 15, because
the detach procedure was different there, so we only backpatch down to
15.
Fix by skipping such modifying constraints that are children of other
constraints being detached.
Author: Amul Sul <sulamul@gmail.com>
Diagnosys-by: Sami Imseih <samimseih@gmail.com>
Discussion: https://postgr.es/m/CAAJ_b97GuPh6wQPbxQS-Zpy16Oh+0aMv-w64QcGrLhCOZZ6p+g@mail.gmail.com
Peter Eisentraut [Tue, 21 Jan 2025 13:34:44 +0000 (14:34 +0100)]
Fix NO ACTION temporal foreign keys when the referenced endpoints change
If a referenced UPDATE changes the temporal start/end times, shrinking
the span the row is valid, we get a false return from
ri_Check_Pk_Match(), but overlapping references may still be valid, if
their reference didn't overlap with the removed span.
We need to consider what span(s) are still provided in the referenced
table. Instead of returning that from ri_Check_Pk_Match(), we can
just look it up in the main SQL query.
Reported-by: Sam Gabrielsson <sam@movsom.se>
Author: Paul Jungwirth <pj@illuminatedcomputing.com>
Discussion: https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com
Peter Eisentraut [Tue, 21 Jan 2025 11:14:49 +0000 (12:14 +0100)]
Improve whitespace in without_overlaps test
Make some indentation better and more consistent. Extracted from
another patch with some actual test changes.
Discussion: https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com
Peter Eisentraut [Tue, 21 Jan 2025 07:13:40 +0000 (08:13 +0100)]
Improve generated_stored test
The test table names gtest11s and gtest12s were way originally chosen
to signify "stored", when the idea was to have virtual columns in the
same test file. This is no longer the idea, so this naming is
irrelevant. (The upcoming feature of virtual generated columns will
have a test file that is initially a copy of generated_stored.sql, and
this random difference will be even more annoying then.) Clean this
up by dropping the suffix.
Discussion: https://www.postgresql.org/message-id/flat/
a368248e-69e4-40be-9c07-
6c3b5880b0a6@eisentraut.org
Amit Langote [Tue, 21 Jan 2025 03:53:03 +0000 (12:53 +0900)]
Refactor ExecScan() to allow inlining of its core logic
This commit refactors ExecScan() by moving its tuple-fetching,
filtering, and projection logic into an inline-able function,
ExecScanExtended(), defined in src/include/executor/execScan.h.
ExecScanExtended() accepts parameters for EvalPlanQual state,
qualifiers (ExprState), and projection (ProjectionInfo).
Specialized variants of the execution function of a given Scan node
(for example, ExecSeqScan() for SeqScan) can then pass const-NULL for
unused parameters. This allows the compiler to inline the logic and
eliminate unnecessary branches or checks. Each variant function thus
contains only the necessary code, optimizing execution for scans
where these features are not needed.
The variant function to be used is determined in the ExecInit*()
function of the node and assigned to the ExecProcNode function pointer
in the node's PlanState, effectively turning runtime checks and
conditional branches on the NULLness of epqstate, qual, and projInfo
into static ones, provided the compiler successfully eliminates
unnecessary checks from the inlined code of ExecScanExtended().
Currently, only ExecSeqScan() is modified to take advantage of this
inline-ability. Other Scan nodes might benefit from such specialized
variant functions but that is left as future work.
Benchmarks performed by Junwang Zhao, David Rowley and myself show up
to a 5% reduction in execution time for queries that rely heavily on
Seq Scans. The most significant improvements were observed in
scenarios where EvalPlanQual, qualifiers, and projection were not
required, but other cases also benefit from reduced runtime overhead
due to the inlining and removal of unnecessary code paths.
The idea for this patch first came from Andres Freund in an off-list
discussion. The refactoring approach implemented here is based on a
proposal by David Rowley, significantly improving upon the patch I
(amitlan) initially proposed.
Suggested-by: Andres Freund <andres@anarazel.de>
Co-authored-by: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: Junwang Zhao <zhjwpku@gmail.com>
Tested-by: Junwang Zhao <zhjwpku@gmail.com>
Tested-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/CA+HiwqGaH-otvqW_ce-paL=96JvU4j+Xbuk+14esJNDwefdkOg@mail.gmail.com
Michael Paquier [Tue, 21 Jan 2025 02:30:42 +0000 (11:30 +0900)]
Rework handling of pending data for backend statistics
9aea73fc61d4 has added support for backend statistics, relying on
PgStat_EntryRef->pending for its data pending for flush. This design
lacks in flexibility, because the pending list does some memory
allocation, making it unsuitable if incrementing counters in critical
sections.
Pending data of backend statistics is reworked so the implementation
does not depend on PgStat_EntryRef->pending anymore, relying on a static
area of memory to store the counters that are flushed when stats are
reported to the pgstats dshash. An advantage of this approach is to
allow the pending data to be manipulated in critical sections; some
patches are under discussion and require that.
The pending data is tracked by PendingBackendStats, local to
pgstat_backend.c. Two routines are introduced to allow IO statistics to
update the backend-side counters. have_static_pending_cb and
flush_static_cb are used for the flush, instead of flush_pending_cb.
Author: Bertrand Drouvot, Michael Paquier
Discussion: https://postgr.es/m/66efowskppsns35v5u2m7k4sdnl7yoz5bo64tdjwq7r5lhplrz@y7dme5xwh2r5
Michael Paquier [Tue, 21 Jan 2025 01:12:39 +0000 (10:12 +0900)]
Rename some pgstats callbacks related to flush of entries
The two callbacks have_fixed_pending_cb and flush_fixed_cb have been
introduced in
fc415edf8ca8 to provide a way for fixed-numbered
statistics to control the flush of their data. These are renamed to
respectively have_static_pending_cb and flush_static_cb. The
restriction that these only apply to fixed-numbered stats is removed.
A follow-up patch will make use of them for backend statistics. This
stats kind is variable-numbered, and patches are under discussion to
track WAL data for IO and backend stats which cannot use
PgStat_EntryRef->pending as pending data would be touched in critical
sections, where no memory allocation can happen.
Per discussion with Andres Freund.
Author: Bertrand Drouvot
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/66efowskppsns35v5u2m7k4sdnl7yoz5bo64tdjwq7r5lhplrz@y7dme5xwh2r5
Tom Lane [Mon, 20 Jan 2025 21:49:15 +0000 (16:49 -0500)]
Update time zone data files to tzdata release 2025a.
DST law changes in Paraguay.
Historical corrections for the Philippines.
Backpatch-through: 13
Tom Lane [Mon, 20 Jan 2025 20:47:53 +0000 (15:47 -0500)]
Avoid using timezone Asia/Manila in regression tests.
The freshly-released 2025a version of tzdata has a refined estimate
for the longitude of Manila, changing their value for LMT in
pre-standardized-timezone days. This changes the output of one of
our test cases. Since we need to be able to run with system tzdata
files that may or may not contain this update, we'd better stop
making that specific test.
I switched it to use Asia/Singapore, which has a roughly similar UTC
offset. That LMT value hasn't changed in tzdb since 2003, so we can
hope that it's well established.
I also noticed that this set of make_timestamptz tests only exercises
zones east of Greenwich, which seems rather sad, and was not the
original intent AFAICS. (We've already changed these tests once
to stabilize their results across tzdata updates, cf
66b737cd9;
it looks like I failed to consider the UTC-offset-sign aspect then.)
To improve that, add a test with Pacific/Honolulu. That LMT offset
is also quite old in tzdb, so we'll cross our fingers that it doesn't
get improved.
Reported-by: Christoph Berg <cb@df7cb.de>
Discussion: https://postgr.es/m/Z46inkznCxesvDEb@msg.df7cb.de
Backpatch-through: 13
Peter Eisentraut [Mon, 20 Jan 2025 14:27:33 +0000 (15:27 +0100)]
Improve generated_stored test
It makes more sense to put the catalog sanity check at the end of the
test rather than at the beginning, so that it can also check whatever
the tests did rather than just whatever happened before the tests.
Suggested-by: jian he <jian.universality@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/
a368248e-69e4-40be-9c07-
6c3b5880b0a6@eisentraut.org
Peter Eisentraut [Mon, 20 Jan 2025 12:05:50 +0000 (13:05 +0100)]
Add some more use of Page/PageData rather than char *
Discussion: https://www.postgresql.org/message-id/flat/
692ee0da-49da-4d32-8dca-
da224cc2800e@eisentraut.org
Peter Eisentraut [Mon, 20 Jan 2025 09:53:47 +0000 (10:53 +0100)]
Add const qualifiers to bufpage.h
This makes use of the new PageData type.
PageGetSpecialPointer() had to be turned back into a macro, because it
is used in a way that sometimes it takes const and returns const and
sometimes takes non-const and returns non-const.
Discussion: https://www.postgresql.org/message-id/flat/
692ee0da-49da-4d32-8dca-
da224cc2800e@eisentraut.org
Peter Eisentraut [Mon, 20 Jan 2025 09:53:47 +0000 (10:53 +0100)]
Add PageData C type
This adds the C type PageData and makes the existing type Page a
pointer to it. This follows the usual PostgreSQL C type naming scheme
of Foo/FooData pairs. (Prior to commit
ddbba3aac86, PageData existed
as an unrelated type.) The type definitions are compatible, so this
doesn't change anything except some of the naming.
Discussion: https://www.postgresql.org/message-id/flat/
692ee0da-49da-4d32-8dca-
da224cc2800e@eisentraut.org
Thomas Munro [Mon, 20 Jan 2025 02:17:47 +0000 (15:17 +1300)]
Fix latch event policy that hid socket events.
If a WaitEventSetWait() caller asks for multiple events, an already set
latch would previously prevent other events from being reported at the
same time. Now, we'll also poll the kernel for other events that would
fit in the caller's output buffer with a zero wait time. This policy
change doesn't affect callers that ask for only one event.
The main caller affected is the postmaster. If its latch is set
extremely frequently by backends launching workers and workers exiting,
we don't want it to handle only those jobs and ignore incoming client
connections.
Back-patch to 16 where the postmaster began using the API. The
fast-return policy changed here is older than that, but doesn't cause
any known problems in earlier releases.
Reported-by: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Discussion: https://postgr.es/m/Z1n5UpAiGDmFcMmd%40nathan
Michael Paquier [Mon, 20 Jan 2025 00:29:42 +0000 (09:29 +0900)]
Fix header check for continuation records where standbys could be stuck
XLogPageRead() checks immediately for an invalid WAL record header on a
standby, to be able to handle the case of continuation records that need
to be read across two different sources. As written, the check was too
generic, applying to any target LSN. Based on an analysis by Kyotaro
Horiguchi, what really matters is to make sure that the page header is
checked when attempting to read a LSN at the boundary of a segment, to
handle the case of a continuation record that spawns across multiple
pages when dealing with multiple segments, as WAL receivers are spawned
they request WAL from the beginning of a segment. This fix has been
proposed by Kyotaro Horiguchi.
This could cause standbys to loop infinitely when dealing with a
continuation record during a timeline jump, in the case where the
contents of the record in the follow-up page are invalid.
Some regression tests are added to check such scenarios, able to
reproduce the original problem. In the test, the contents of a
continuation record are overwritten with junk zeros on its follow-up
page, and replayed on standbys. This is inspired by 039_end_of_wal.pl,
and is enough to show how standbys should react on promotion by not
being stuck. Without the fix, the test would fail with a timeout. The
test to reproduce the problem has been written by Alexander Kukushkin.
The original check has been introduced in
066871980183, for a similar
problem.
Author: Kyotaro Horiguchi, Alexander Kukushkin
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/CAFh8B=mozC+e1wGJq0H=0O65goZju+6ab5AU7DEWCSUA2OtwDg@mail.gmail.com
Backpatch-through: 13
Tom Lane [Sun, 19 Jan 2025 19:00:22 +0000 (14:00 -0500)]
Remove PrintBufferDescs() and PrintPinnedBufs().
These have been #ifdef'd out for a long time, and in fact have
been uncompilable since commit
48354581a of 2016-04-10. The
fact that nobody noticed for so long demonstrates their lack of
usefulness, so let's remove them rather than fix them.
Author: Jacob Brazeal <jacob.brazeal@gmail.com>
Discussion: https://postgr.es/m/CA+COZaB+9CN_f63PPRoVhHjYmCwwmb_9CWLxqCJdMWDqs1a-JA@mail.gmail.com
Andrew Dunstan [Sun, 19 Jan 2025 14:09:58 +0000 (09:09 -0500)]
Be clearer about when jsonapi's need_escapes is needed
Most operations beyond pure json parsing need to set need_escapes to
true to get access to field names and string scalars. Document this
fact more explicitly.
Slightly tweaked patch from:
Author: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://postgr.es/m/CADkLM=c49Vkfg2+A8ubSuEtaGEjuaKZXCA6SrXA8kdwHjx3uxQ@mail.gmail.com
Jeff Davis [Fri, 17 Jan 2025 23:56:30 +0000 (15:56 -0800)]
Support PG_UNICODE_FAST locale in the builtin collation provider.
The PG_UNICODE_FAST locale uses code point sort order (fast,
memcmp-based) combined with Unicode character semantics. The character
semantics are based on Unicode full case mapping.
Full case mapping can map a single codepoint to multiple codepoints,
such as "ß" uppercasing to "SS". Additionally, it handles
context-sensitive mappings like the "final sigma", and it uses
titlecase mappings such as "Dž" when titlecasing (rather than plain
uppercase mappings).
Importantly, the uppercasing of "ß" as "SS" is specifically mentioned
by the SQL standard. In Postgres, UCS_BASIC uses plain ASCII semantics
for case mapping and pattern matching, so if we changed it to use the
PG_UNICODE_FAST locale, it would offer better compliance with the
standard. For now, though, do not change the behavior of UCS_BASIC.
Discussion: https://postgr.es/m/
ddfd67928818f138f51635712529bc5e1d25e4e7.camel@j-davis.com
Discussion: https://postgr.es/m/
27bb0e52-801d-4f73-a0a4-
02cfdd4a9ada@eisentraut.org
Reviewed-by: Peter Eisentraut, Daniel Verite
Jeff Davis [Fri, 17 Jan 2025 23:56:20 +0000 (15:56 -0800)]
Support Unicode full case mapping and conversion.
Generate tables from Unicode SpecialCasing.txt to support more
sophisticated case mapping behavior:
* support case mappings to multiple codepoints, such as "ß"
uppercasing to "SS"
* support conditional case mappings, such as the "final sigma"
* support titlecase variants, such as "dž" uppercasing to "DŽ" but
titlecasing to "Dž"
Discussion: https://postgr.es/m/
ddfd67928818f138f51635712529bc5e1d25e4e7.camel@j-davis.com
Discussion: https://postgr.es/m/
27bb0e52-801d-4f73-a0a4-
02cfdd4a9ada@eisentraut.org
Reviewed-by: Peter Eisentraut, Daniel Verite
Nathan Bossart [Fri, 17 Jan 2025 21:23:14 +0000 (15:23 -0600)]
vacuumdb: Fix comment for vacuum_one_database().
Since commit
e0c2933a76, vacuum_one_database() always uses a
catalog query to discover the tables to process, but this comment
still notes the special case for which we used a catalog query
before that commit. Let's just remove that note.
Also, commit
7781f4e3e7 renamed the "tables" parameter to "objects"
but missed updating this comment. This commit fixes that as well.
Tom Lane [Fri, 17 Jan 2025 19:37:38 +0000 (14:37 -0500)]
Add documentation about calling version-1 C functions from C.
This topic wasn't really covered before, so fill in some details.
Author: Florents Tselai <florents.tselai@gmail.com>
Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/
90853055-5BBD-493D-91E5-
721677C7C59B@gmail.com
Dean Rasheed [Fri, 17 Jan 2025 10:35:07 +0000 (10:35 +0000)]
Fix parsing of qualified relation names in RETURNING.
Given a qualified refname, refnameNamespaceItem() will search for a
matching namespace item by relation OID, rather than by name. Commit
80feb727c8 broke this by adding additional namespace items for OLD and
NEW in the RETURNING list, which have the same relation OID, causing
ambiguity. Fix this by ignoring these in the search, which is correct
since they don't match the qualified relation name, and so there is no
real ambiguity.
Reported by Richard Guo.
Discussion: https://postgr.es/m/CAMbWs49MBjWYWDROJ8MZ%3DY%2B4UgRQa10wzik1tWrD5yto9eoGXg%40mail.gmail.com
John Naylor [Wed, 15 Jan 2025 06:28:26 +0000 (13:28 +0700)]
Speed up hex_encode with bytewise lookup
Previously, hex_encode looked up each nibble of the input
separately. We now use a larger lookup table containing the two-byte
encoding of every possible input byte, resulting in a 1/3 reduction
in encoding time.
Reviewed by Tom Lane, Michael Paquier, Nathan Bossart, David Rowley
Discussion: https://postgr.es/m/CANWCAZZvXuJMgqMN4u068Yqa19CEjS31tQKZp_qFFFbgYfaXqQ%40mail.gmail.com
Peter Eisentraut [Fri, 17 Jan 2025 07:35:52 +0000 (08:35 +0100)]
Remove flex version checks
Remove the flex version checks from configure and meson. The cutoff
versions are all so ancient that this is no longer relevant, and what
the actual cutoff should be is a bit fuzzy.
This also removes the ancient behavior that configure would also
accept a "lex" program if it is actuall flex. This aligns the check
with meson in this respect.
For future reference, as of this commit, these are relevant flex
versions:
- The hard required minimum is flex 2.5.34 as of commit
b1ef48980dd,
but this has not actually been tested.
- Prior to this, the minimum enforced by configure/meson was flex
2.5.35, which is the oldest present in the buildfarm right now.
- As of commit
6fdd5d95634, the oldest version that will compile
without warnings due to flex-generated code is flex 2.5.36.
- The oldest version that probably still has some practical relevance
is flex 2.5.37, which ships with CentOS/RHEL 7.
Discussion: https://www.postgresql.org/message-id/
1a204ccd-7ae6-478c-a431-
407b5c48ccc6@eisentraut.org
Peter Eisentraut [Fri, 17 Jan 2025 07:06:24 +0000 (08:06 +0100)]
Add pg_nodiscard decorations to base64 functions
The result of pg_b64_encode() and pg_b64_decode() should be checked
for errors. This attribute could detect mistakes such as those fixed
in commit
ff030ebe250 and
d278541be42.
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Ranier Vilela <ranier.vf@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAEudQAq-3yHsSdWoOOaw%2BgAQYgPMpMGuB5pt2yCXgv-YuxG2Hg%40mail.gmail.com
Michael Paquier [Fri, 17 Jan 2025 04:27:39 +0000 (13:27 +0900)]
Revert recent changes related to handling of 2PC files at recovery
This commit reverts
8f67f994e8ea (down to v13) and
c3de0f9eed38 (down to
v17), as these are proving to not be completely correct regarding two
aspects:
- In v17 and newer branches,
c3de0f9eed38's check for epoch handling is
incorrect, and does not correctly handle frozen epochs. A logic closer
to widen_snapshot_xid() should be used. The 2PC code should try to
integrate deeper with FullTransactionIds,
5a1dfde8334b being not enough.
- In v13 and newer branches,
8f67f994e8ea is a workaround for the real
issue, which is that we should not attempt CLOG lookups without reaching
consistency. This exists since
728bd991c3c4, and this is reachable with
ProcessTwoPhaseBuffer() called by restoreTwoPhaseData() at the beginning
of recovery.
Per discussion with Noah Misch.
Discussion: https://postgr.es/m/
20250116010051.f3.nmisch@google.com
Backpatch-through: 13
Nathan Bossart [Fri, 17 Jan 2025 02:55:24 +0000 (20:55 -0600)]
Remove redefinitions of SIG_* macros in win32_port.h.
It is not clear why these were originally added. One hypothesis is
that an ancient version of MinGW didn't define them. In any case,
they appear to now be superfluous, so let's remove them. If
nothing else, the buildfarm might offer us clues to their origins.
Reviewed-by: Thomas Munro
Discussion: https://postgr.es/m/Z4chOKfnthRH71mw%40nathan
Tom Lane [Fri, 17 Jan 2025 01:40:07 +0000 (20:40 -0500)]
Fix setrefs.c's failure to do expression processing on prune steps.
We should run the expression subtrees of PartitionedRelPruneInfo
structs through fix_scan_expr. Failure to do so means that
AlternativeSubPlans within those expressions won't be cleaned up
properly, resulting in "unrecognized node type" errors since v14.
It seems fairly likely that at least some of the other steps done
by fix_scan_expr are important here as well, resulting in as-yet-
undetected bugs. Therefore, I've chosen to back-patch this to
all supported branches including v13, even though the known
symptom doesn't manifest in v13.
Per bug #18778 from Alexander Lakhin.
Discussion: https://postgr.es/m/18778-
24cd399df6c806af@postgresql.org
Melanie Plageman [Thu, 16 Jan 2025 23:42:39 +0000 (18:42 -0500)]
Add and use BitmapHeapScanDescData struct
Move the several members of HeapScanDescData which are specific to
Bitmap Heap Scans into a new struct, BitmapHeapScanDescData, which
inherits from HeapScanDescData.
This reduces the size of the HeapScanDescData for other types of scans
and will allow us to add additional bitmap heap scan-specific members in
the future without fear of bloating the HeapScanDescData.
Reviewed-by: Tomas Vondra
Discussion: https://postgr.es/m/
c736f6aa-8b35-4e20-9621-
62c7c82e2168%40vondra.me
Michael Paquier [Thu, 16 Jan 2025 23:26:17 +0000 (08:26 +0900)]
Rework macro pgstat_is_ioop_tracked_in_bytes()
As written, it was triggering a compilation warning for old versions of
clang, as reported by buildfarm members ayu, batfish and demoiselle.
Forcing a cast with "unsigned int" should fix the warning.
While on it, the macro is moved to pgstat.h, closer to the declaration
of IOOp, per suggestion from Tom Lane.
Reported-by: Tom Lane
Reviewed-by: Bertrand Drouvot, Tom Lane, Nazir Bilal Yavuz
Discussion: https://postgr.es/m/
1272824.
1736961543@sss.pgh.pa.us
Nathan Bossart [Thu, 16 Jan 2025 22:41:05 +0000 (16:41 -0600)]
Convert libpgport's pqsignal() to a void function.
The protections added by commit
3b00fdba9f introduced race
conditions to this function that can lead to bogus return values.
Since nobody seems to inspect the return value, this is of little
consequence, but it would have been nice to convert it to a void
function to avoid any possibility of a bogus return value. I
originally thought that doing so would have required also modifying
legacy-pqsignal.c's version of the function (which would've
required an SONAME bump), but commit
9a45a89c38 gave
legacy-pqsignal.c its own dedicated extern for pqsignal(), thereby
decoupling it enough that libpgport's pqsignal() can be modified.
This commit also adds an assertion for the return value of
sigaction()/signal(). Since a failure most likely indicates a
coding error, and nobody has ever bothered to check pqsignal()'s
return value, it's probably not worth the effort to do anything
fancier.
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/Z4chOKfnthRH71mw%40nathan
Nathan Bossart [Thu, 16 Jan 2025 21:56:39 +0000 (15:56 -0600)]
Avoid calling pqsignal() with invalid signals on Windows frontends.
As noted by the comment at the top of port/pqsignal.c, Windows
frontend programs can only use pqsignal() with the 6 signals
required by C. Most places avoid using invalid signals via #ifndef
WIN32, but initdb and pg_test_fsync check whether the signal itself
is defined, which doesn't work because win32_port.h defines many
extra signals for the signal emulation code. pg_regress seems to
have missed the memo completely. These issues aren't causing any
real problems today because nobody checks the return value of
pqsignal(), but a follow-up commit will add some error checking.
To fix, surround all frontend calls to pqsignal() that use signals
that are invalid on Windows with #ifndef WIN32. We cannot simply
skip defining the extra signals in win32_port.h for frontends
because they are needed in places such as pgkill().
Reviewed-by: Thomas Munro
Discussion: https://postgr.es/m/Z4chOKfnthRH71mw%40nathan
Tom Lane [Thu, 16 Jan 2025 19:11:19 +0000 (14:11 -0500)]
Seek zone abbreviations in the IANA data before timezone_abbreviations.
If a time zone abbreviation used in datetime input is defined in
the currently active timezone, use that definition in preference
to looking in the timezone_abbreviations list. That allows us to
correctly handle abbreviations that have different meanings in
different timezones. Also, it eliminates an inconsistency between
datetime input and datetime output: the non-ISO datestyles for
timestamptz have always printed abbreviations taken from the IANA
data, not from timezone_abbreviations. Before this fix, it was
possible to demonstrate cases where casting a timestamp to text
and back fails or changes the value significantly because of that
inconsistency.
While this change removes the ability to override the IANA data about
an abbreviation known in the current zone, it's not clear that there's
any real use-case for doing so. But it is clear that this makes life
a lot easier for dealing with abbreviations that have conflicts across
different time zones.
Also update the pg_timezone_abbrevs view to report abbreviations
that are recognized via the IANA data, and *not* report any
timezone_abbreviations entries that are thereby overridden.
Under the hood, there are now two SRFs, one that pulls the IANA
data and one that pulls timezone_abbreviations entries. They're
combined by logic in the view. This approach was useful for
debugging (since the functions can be called on their own).
While I don't intend to document the functions explicitly,
they might be useful to call directly.
Also improve DecodeTimezoneAbbrev's caching logic so that it can
cache zone abbreviations found in the IANA data. Without that,
this patch would have caused a noticeable degradation of the
runtime of timestamptz_in.
Per report from Aleksander Alekseev and additional investigation.
Discussion: https://postgr.es/m/CAJ7c6TOATjJqvhnYsui0=CO5XFMF4dvTGH+skzB--jNhqSQu5g@mail.gmail.com
Tom Lane [Thu, 16 Jan 2025 17:43:03 +0000 (12:43 -0500)]
Make pg_interpret_timezone_abbrev() check sp->defaulttype too.
This omission caused it to not recognize the furthest-back zone
abbreviation when working with timezone data compiled with relatively
recent zic (2018f or newer). Older versions of zic produced a dummy
DST transition at the Big Bang, so that the oldest abbreviation could
always be found in the sp->types[] array; but newer versions don't do
that, so that we must examine defaulttype as well as the types[] array
to be sure of seeing all the abbreviations.
While this has been broken for six or so years, we'd managed not
to notice for two reasons: (1) many platforms are still using
ancient zic for compatibility reasons, so that the issue did not
manifest in builds using --with-system-tzdata; (2) the oldest
zone abbreviation is almost always "LMT", which we weren't
supporting anyway (but an upcoming patch will accept that).
While at it, update pg_next_dst_boundary() to use sp->defaulttype
as the time type for non-DST zones and times before the oldest
DST transition. The existing code there predates upstream's
invention of the sp->defaulttype field, and its heuristic for
finding the oldest time type has now been subsumed into the
code that sets sp->defaulttype.
Possibly this should be back-patched, but I'm not currently aware
of any visible consequences of this bug in released branches.
Per report from Aleksander Alekseev and additional investigation.
Discussion: https://postgr.es/m/CAJ7c6TOATjJqvhnYsui0=CO5XFMF4dvTGH+skzB--jNhqSQu5g@mail.gmail.com
Peter Geoghegan [Thu, 16 Jan 2025 16:26:41 +0000 (11:26 -0500)]
Fix nbtree contradictory array element comment.
Oversight in commit
5bf748b8, which enhanced nbtree ScalarArrayOp
execution.
Álvaro Herrera [Thu, 16 Jan 2025 15:44:24 +0000 (16:44 +0100)]
Split ATExecValidateConstraint into reusable pieces
With this, we have separate functions to add validation requests to
ALTER TABLE's phase 3 queue for check and foreign key constraints, which
allows reusing them in future commits -- particularly this will allow us
to perform validation of invalid foreign key constraints in partitioned
tables.
We could have let the check constraint code alone since we don't need to
reuse that for anything at this point, but it seems cleaner and more
consistent to do both at the same time.
Author: Amul Sul <sulamul@gmail.com>
Discussion: https://postgr.es/m/CAAJ_b96Bp=-ZwihPPtuaNX=SrZ0U6ZsXD3+fgARO0JuKa8v2jQ@mail.gmail.com
Dean Rasheed [Thu, 16 Jan 2025 14:57:35 +0000 (14:57 +0000)]
Add OLD/NEW support to RETURNING in DML queries.
This allows the RETURNING list of INSERT/UPDATE/DELETE/MERGE queries
to explicitly return old and new values by using the special aliases
"old" and "new", which are automatically added to the query (if not
already defined) while parsing its RETURNING list, allowing things
like:
RETURNING old.colname, new.colname, ...
RETURNING old.*, new.*
Additionally, a new syntax is supported, allowing the names "old" and
"new" to be changed to user-supplied alias names, e.g.:
RETURNING WITH (OLD AS o, NEW AS n) o.colname, n.colname, ...
This is useful when the names "old" and "new" are already defined,
such as inside trigger functions, allowing backwards compatibility to
be maintained -- the interpretation of any existing queries that
happen to already refer to relations called "old" or "new", or use
those as aliases for other relations, is not changed.
For an INSERT, old values will generally be NULL, and for a DELETE,
new values will generally be NULL, but that may change for an INSERT
with an ON CONFLICT ... DO UPDATE clause, or if a query rewrite rule
changes the command type. Therefore, we put no restrictions on the use
of old and new in any DML queries.
Dean Rasheed, reviewed by Jian He and Jeff Davis.
Discussion: https://postgr.es/m/CAEZATCWx0J0-v=Qjc6gXzR=KtsdvAE7Ow=D=mu50AgOe+pvisQ@mail.gmail.com
Peter Eisentraut [Thu, 16 Jan 2025 13:37:28 +0000 (14:37 +0100)]
Remove dead code
As of commit
9895b35cb88, AlterDomainAddConstraint() can only be
called with constraints of type CONSTR_CHECK and CONSTR_NOTNULL. So
all the code to check for and reject other constraint type values is
dead and can be removed.
Author: jian he <jian.universality@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CACJufxHitd5LGLBSSAPShhtDWxT0ViVKTHinkYW-skBX93TcpA@mail.gmail.com
Peter Eisentraut [Thu, 16 Jan 2025 12:22:01 +0000 (13:22 +0100)]
refactor: split ATExecAlterConstrRecurse()
This splits out a couple of subroutines from
ATExecAlterConstrRecurse(). This makes the main function a bit
smaller, and a future patch (NOT ENFORCED foreign-key constraints)
will also want to call some of the pieces separately.
Author: Amul Sul <amul.sul@enterprisedb.com>
Reviewed-by: jian he <jian.universality@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAAJ_b962c5AcYW9KUt_R_ER5qs3fUGbe4az-SP-vuwPS-w-AGA%40mail.gmail.com