19 ri fastpath revert - #1
Closed
amitlan wants to merge 481 commits into
Closed
Conversation
A two-phase transaction that is assigned an XID but produces no change to be decoded -- for example, one that only acquires row locks via SELECT ... FOR SHARE -- has no base snapshot in the reorder buffer. ReorderBufferReplay() already skips such a transaction at PREPARE time and never invokes the begin_prepare/change/prepare callbacks for it, but ReorderBufferFinishPrepared() still called the commit_prepared (or rollback_prepared) callback. As a result a spurious COMMIT/ROLLBACK PREPARED was sent to the output plugin with no preceding PREPARE. For the built-in subscriber this breaks replication (the apply worker fails to find the prepared transaction), and test_decoding could even crash. Fix this by detecting an empty transaction (base_snapshot == NULL) in ReorderBufferFinishPrepared() and cleaning it up without invoking the commit/rollback prepared callbacks, mirroring the existing empty transaction handling in ReorderBufferReplay(). On v18 and newer versions, commit 072ee84 changed ReorderBufferPrepare() to send the prepare whenever it had not already been sent, which also fires for empty transactions and emits a spurious PREPARE. On those branches ReorderBufferPrepare() is therefore additionally guarded with base_snapshot != NULL. This guard and the Assert(!rbtxn_sent_prepare()) added in ReorderBufferFinishPrepared(), are not necessary on v17 and older versions: there ReorderBufferPrepare() only sends a prepare for concurrently-aborted transactions (which never applies to an empty transaction) and the RBTXN_SENT_PREPARE flag does not exist. Back-patch to v14, where decoding of two-phase transactions was introduced. Bug: #19556 Reported-by: Alexander Kozhemyakin <a.kozhemyakin@postgrespro.ru> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Discussion: https://postgr.es/m/19556-daa6d7ea65054d48@postgresql.org Backpatch-through: 14
The file_copy strategy check in createdb() runs during option validation, before the transaction has an XID and before the pg_database row exists, so the datachecksumsworker launcher can start in that window and see neither the new database nor the transaction creating it. It then raw-copies a template that was not processed yet, and those files stay unchecksummed, failing verification from then on. Recheck the state in CreateDatabaseUsingFileCopy(): the XID is assigned by then, so a launcher starting after this point waits for the transaction and finds the new database, and the copy errors out instead. Add an injection point before the catalog insert to test the window. Backpatch to v19 where online checksums were introduced. Author: Zsolt Parragi <zsolt.parragi@percona.com> Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://postgr.es/m/CAN4CZFPEBsz8JeY4ixQ1V4ZL_xOY6pJaZS8ZLGH7R+wF--pEtg@mail.gmail.com Backpatch-through: 19
Enable errors out early with a hint when an invalid database exists, since the worker cannot connect to it and its files stay on disk. A worker that started but failed gets the same dropped-database heuristic as one that failed to start, so a concurrent drop during processing no longer aborts the whole run. The existence check locks the database first, otherwise a DROP DATABASE ... WITH (FORCE) which killed the worker is still only halfway done and the database looks like it is there to stay. Backpatch to v19 where online checksums were introduced. Author: Zsolt Parragi <zsolt.parragi@percona.com> Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://postgr.es/m/CAN4CZFOGdqxtZ5-6gb4apqmvoH=Z+TNH8RKJ3mVtoR1HirKQWg@mail.gmail.com Backpatch-through: 19
find_nonnullable_rels and find_nonnullable_vars mistakenly treated a ScalarArrayOpExpr that could return FALSE as strict, but that's okay only at top level of a qual expression; further down, we've got to insist on a guaranteed-NULL result. The result was that we could draw mistaken conclusions about whether outer joins can be simplified, if the decision hinged on a non-top-level ScalarArrayOpExpr with a potentially-empty array argument. I believe this error dates to commit 72a070a, which taught find_nonnullable_rels to descend into non-top-level parts of qual expressions. is_strict_saop (added earlier by 72153c0) already had enough intelligence to do the case correctly, but it wasn't passed the proper flag, ie "top_level" needs to be passed for "falseOK". e006a24 copied that mistake into find_nonnullable_vars. Later, over-eager refactoring in commit 2f153dd broke contain_nonstrict_functions' handling of ScalarArrayOpExpr by treating it as though it were no different from an OpExpr. It is, because we must also prove the array is non-empty before concluding that the expression is strict. This could result in misclassifying an expression as strict when it is not, leading to assorted planning mistakes such as inlining a SQL function that shouldn't be inlined. We can almost fix this by just re-adding the previous handling of ScalarArrayOpExpr in that function, but doing only that would lead to also calling check_functions_in_node() and thus redundantly checking the operator's strictness. Avoid that by turning the if-series into an else-if chain, as it arguably should have been all along. The reason these errors have escaped detection for decades is that they are exposed only in arcane corner cases. ScalarArrayOpExpr with an empty array isn't typical usage, and even when that's possible several other conditions apply before the planner can reach a mistaken conclusion. While it's possible to build test cases demonstrating these mistakes, I (tgl) judged them too indirect and special-purpose to justify consuming regression test cycles forevermore. Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAJTYsWV3vqRJmST-gv1NsXEef-zOnjVJpYS910aBaiuMij4nFg@mail.gmail.com Discussion: https://postgr.es/m/CAJTYsWWcLGmz0f8_QPP_Liq-fc7-geiFSCdqoq3XGeRHPPsWeA@mail.gmail.com Backpatch-through: 14
While collecting the sequences to synchronize, the sequence sync worker opened each INIT sequence with RowExclusiveLock and held it until the transaction committed. With many such sequences, this could exhaust the shared lock table and fail with "out of shared memory". The worker only reads each sequence's identity (namespace and name) here and needs it to stay stable while read, for which AccessShareLock is enough, as it conflicts with the AccessExclusiveLock taken by DROP, RENAME, and SET SCHEMA. Take that lock instead and release it as soon as the identity is read. The later synchronization re-opens each sequence, so it does not rely on the lock being retained. Reported-by: Noah Misch <noah@leadboat.com> Author: vignesh C <vignesh21@gmail.com> Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Backpatch-through: 19, where it was introduced Discussion: https://postgr.es/m/20260710045217.f0.noahmisch@microsoft.com
TerminateBackgroundWorkersForDatabase() uses BackendPidGetProc() and, until now, accessed fields of the returned PGPROC after releasing ProcArrayLock, including its database OID. If the PGPROC slot is recycled during this window, the database OID being checked may belong to a different backend, causing an unrelated background worker to be terminated. Triggering this bug requires a very narrow race: the background worker identified by BackendPidGetProc() must exit, its PGPROC slot must be released and reused, and only then must TerminateBackgroundWorkersForDatabase() examine the database OID. TerminateBackgroundWorkersForDatabase() holds BackgroundWorkerLock, preventing parallel workers and dynamically registered workers (such as those created by worker_spi) from reusing the slot. As far as I know, the only plausible scenario is a static background worker that exits and is restarted quickly enough to reuse the same PGPROC slot within the race window. In practice, this race is extremely unlikely, still reachable in theory. Oversight in f1e251b. Author: Chao Li <li.evan.chao@gmail.com> Reviewed-by: Aya Iwata <iwata.aya@fujitsu.com> Reviewed-by: Haibo Yan <tristan.yim@gmail.com> Discussion: https://postgr.es/m/78E81763-EA1D-4788-9741-4092BCB997A5@gmail.com Backpatch-through: 19
A cascading standby could fail to reconnect to its upstream standby with "requested starting point ... is ahead of the WAL flush position" after falling back to archive recovery. This happened because archive recovery processes whole segment files, so after replaying a segment the cascade's next read position lands at the start of the following segment, which is ahead of the upstream's flush position reported by GetStandbyFlushRecPtr() (still inside the just-replayed segment). Fix by having the walreceiver check the upstream's current WAL flush position via IDENTIFY_SYSTEM before issuing START_REPLICATION. IDENTIFY_SYSTEM already returns this position (as xlogpos), but walrcv_identify_system() previously discarded it; now we have a use for it. If the requested start point exceeds the upstream's flush position on the same timeline, the walreceiver waits for wal_retrieve_retry_interval and retries. The wait is limited to gaps of at most one WAL segment, which is the expected case from the segment-granularity of archive recovery. Larger gaps indicate the upstream is genuinely behind, so START_REPLICATION is allowed to proceed (and fail) normally, letting the startup process fall back to other WAL sources. The first wait is logged at LOG level; subsequent waits are demoted to DEBUG1 to avoid log noise. The walreceiver honors wal_receiver_timeout during the wait, so it will exit if the upstream doesn't catch up in time. To preserve ABI compatibility on back branches, the flush position from IDENTIFY_SYSTEM is communicated via a new global variable (WalRcvIdentifySystemLsn) rather than changing the signature of walrcv_identify_system(). The bug was introduced in Postgres 9.3 by commit abfd192, which added a flush-position check in StartReplication() that rejects requests ahead of the upstream server's WAL flush position. Author: Marco Nenciarini <marco.nenciarini@enterprisedb.com> Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com> Backpatch-through: 14 Discussion: https://postgr.es/m/CA+nrD2cTuTkkX5WXVZengTYYZbAO6zV8K+Tri-R0fbLFuoyMBA@mail.gmail.com
The comment claimed that a parallel vacuum worker has only the PROC_IN_VACUUM flag because parallel vacuum is not supported for autovacuum, but commit 1ff3180 allowed autovacuum to use parallel vacuum workers. The assertion itself still holds: the leader, whether a backend running VACUUM or an autovacuum worker, sets PROC_IN_VACUUM before taking its snapshot, and a parallel worker inherits the flag when importing the leader's snapshot. The leader's other flags don't reach the worker, since the snapshot import copies only the PROC_XMIN_FLAGS bits and PROC_IS_AUTOVACUUM is never set on parallel workers, which run as regular background workers. Reword the comment to explain that. Oversight in commit 1ff3180. Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://postgr.es/m/CALj2ACVwQ4WABqq8Lnf+VZEJ45jcTFhyFLFr_ctfS4=QLL-r5w@mail.gmail.com Backpatch-through: 19
Restructuring the tags makes the output consistent and doesn't require added spaces. Reported-by: Peter Smith Author: Peter Smith Discussion: https://postgr.es/m/CAHut+Pu8JahGm76CMdpzH350pHJedA4R2b8JmOim3+m3yxft3Q@mail.gmail.com Backpatch-through: 19
refint has been removed from the spi contrib module in v20. Add a note to the documentation of the still-supported back branches so that users are aware the module is going away. Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com> Reported-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://postgr.es/m/CAJTYsWUHq8Ohc6-N-xamOPYz-q3qUYMtwQX-1=Zi=5N1Q_GSEQ@mail.gmail.com Backpatch-through: 14
pg_dump in --binary-upgrade mode emits "SUBSCRIPTION TABLE" TOC entries to preserve pg_subscription_rel state across pg_upgrade. When such a dump was restored with --no-subscriptions, _tocEntryRequired() skipped the "SUBSCRIPTION" entry but not the associated "SUBSCRIPTION TABLE" entries, so the restore would try to apply subscription-relation state for a subscription that was never created. Skip "SUBSCRIPTION TABLE" entries as well when no_subscriptions is set. This can happen when pg_subscription_rel has entries, the dump is taken with --binary-upgrade, and it is restored with --no-subscriptions. Reported-by: Hayato Kuroda <kuroda.hayato@fujitsu.com> Author: Hayato Kuroda <kuroda.hayato@fujitsu.com> Reviewed-by: Shlok Kyal <shlok.kyal.oss@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Backpatch-through: 17, where it was introduced Discussion: https://postgr.es/m/OS9PR01MB121493DA4C1A7748B11A646D8F5C02@OS9PR01MB12149.jpnprd01.prod.outlook.com
The new test for enabling data checksums with concurrent CREATE
DATABASE calls use the same injection points as a previous test
but accidentally missed detaching the injection point first.
Fix by detaching the injection point in the PG_TEST_EXTRA SKIP
block to make it can be reused. Pointed out by buildfarm member
porpoise which failed with:
die: error running SQL: 'psql:<stdin>:1:
ERROR: injection point "datachecksumsworker-fake-temptable-wait"
already defined'
Backpatch to v19 where online checksums were introduced.
Author: Daniel Gustafsson <daniel@yesql.se>
Reported-by: Buildfarm member porpoise
Reviewed-by: Jonathan Gonzalez V. <jonathan.abdiel@gmail.com>
Discussion: https://postgr.es/m/28CF6FD9-E1C4-4C04-8270-E3305AC46171@yesql.se
Backpatch-through: 19
Index builds update pg_class.reltuples for the table. In parallel GIN builds, workers track the number of processed rows, and report it to the leader, who then updates the pg_class with a total. However, gin_parallel_build_main failed to initialize the bs_reltuples field, leaving it set to whatever happens to be on the stack (which may be bogus values like Infinity or NaN, or just impossibly high values). If such values get reported to the leader and stored in pg_class, that can have serious consequences. The pg_class.reltuples field is used to decide when a table is due for autovacuum or autoanalyze, and if it happens to be set to a bogus value, that may never happen. The field is also used by the optimizer when calculating costs. Fixed by initializing bs_reltuples together with the rest of the build state. The bs_numtuples was initialized later, but it seems cleaner to just initialize all the fields at once. After a bogus value gets persisted in pg_class, affected systems are unlikely to self-heal. That would require an ANALYZE, but preventing that is one of the consequences. We have considered forcing autoanalyze in these cases, but there's not a good way to reliably identify bogus values (except for a small minority like Infitiny/NaN). A manual ANALYZE on (possibly) affected tables is the only solution. Backpatch to 18, where parallel GIN builds were introduced. Reported-by: Jan Nidzwetzki <jan@planetscale.com> Discussion: https://postgr.es/m/518BA772-8026-412A-AA8F-A7FE4C6B3717@planetscale.com Backpatch-through: 18
When restoring relation stats, pg_restore_relation_stats() rejected calls with (reltuples < -1.0). But that is insufficient - Infinity and NaN values both pass that check, and get stored in pg_class verbatim. This can have various undesirable consequences. Fixed by rejecting non-finite reltuple values, in the same non-fatal way as for the existing checks (emit WARNING and skip the update). Adds a regression test to stats_import for these non-finite values, and to check the -1.0 special value is still accepted. Backpatch to 18, where pg_restore_relation_stats() was introduced. Patch by Jan Nidzwetzki, minor commit message tweaks by me. Author: Jan Nidzwetzki <jan@planetscale.com> Discussion: https://postgr.es/m/518BA772-8026-412A-AA8F-A7FE4C6B3717@planetscale.com Backpatch-through: 18
On standbys, logical decoding can be deactivated while a logical slot is being created: either by replaying an XLOG_LOGICAL_DECODING_STATUS_CHANGE record, or by the end-of-recovery transition upon promotion, which deactivates logical decoding if no valid logical slot exists. Both could interleave with a check of the logical decoding status performed before creating a new slot because the slot invalidation executed as part of the deactivation cannot find a slot being created. For regular slot creation on standbys, EnsureLogicalDecodingEnabled() assumed that logical decoding must still be enabled during recovery since the caller had already checked it, tripping an assertion failure if a concurrent deactivation interleaved. For slot synchronization, the local slot could be created and persisted based on the remote slot information fetched before the deactivation was replayed, leaving a valid slot whose restart_lsn precedes the deactivation. Fix both paths by re-checking the logical decoding status after the new slot has been created: regular slot creation raises an error, and slot synchronization skips persisting the slot. If the deactivation happens after the re-check instead, it is guaranteed to invalidate the newly created slot. Reviewed-by: Srinath Reddy Sadipiralla <srinath2133@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Discussion: https://postgr.es/m/CAD21AoDEB99VtNbQdDrNd=1gQupJNGMfW_5kdnxq03Q82EK3ag@mail.gmail.com Backpatch-through: 19
Commit 6aba42c added quit() calls for two background psql sessions whose slot creation is canceled by pg_cancel_backend(). Both sessions ran with the default ON_ERROR_STOP=1 and ended their script with \q, so psql exited as soon as the cancellation error arrived. quit() then wrote another \q to the already-closed pipe, making the test die with "ack Broken pipe". Run both sessions with on_error_stop => 0 and drop the trailing \q, so that psql stays at the prompt after reporting the error and quit() can shut it down cleanly. Discussion: https://postgr.es/m/CAD21AoCZY1fKYgfkvHGWGiXpatUKd23FSLnDCL4m9bWFjdXNZw@mail.gmail.com Backpatch-through: 19
This fixes some incorrect flattening of nested Result nodes during create_plan that was introduced by f2bae51. That commit failed to maintain the logic that checks for subplans and gating quals from the nested Result node before flattening, and that could result in the nested gating qual and subplan being lost, which could produce incorrect results. Bug: #19579 Reported-by: Viktor Leis <leis@in.tum.de> Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com> Reviewed-by: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/19579-e6296b6c9fc0591c@postgresql.org Backpatch-through: 19
Partition pruning for RANGE-partitioned tables could mistakenly prune the DEFAULT partition in some cases when it was not valid to do so, which could lead to rows missing from query results. The only known cases where this could happen is when combining pruning steps from an IS NOT NULL clause with other steps that matched to the DEFAULT partition. This could occur due to RANGE partitioned tables having two distinct internal representations for marking if the DEFAULT partition should be scanned. The IS NOT NULL steps would mark the "scan_default" boolean, but other steps created for different purposes could mark a bound_offset Bitmapset, which would ultimately translate into also scanning the default partition. This could all fail after multiple steps were combined with a combine intersect operator, as that will intersect the bound_offset bits and only set scan_default if all pruning steps have that flag set. When both input steps to the intersect operator had different representations of whether to scan the DEFAULT partition, the resulting intersect step result would contain neither representation. Here, we fix this by having the IS NOT NULL pruning result mark the bound_offsets so that it uses both representations to mark that the DEFAULT partition must be scanned. Reported-by: Jacob Brazeal <jacob.brazeal@gmail.com> Diagnosed-by: Jacob Brazeal <jacob.brazeal@gmail.com> Author: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/CA+COZaDXrfTaBjLE=Z79MTaH6Xun1V4PeKxLvCNv8mXS8wn0rw@mail.gmail.com Backpatch-through: 14
check_publications_origin_sequences() warns when a subscription with
origin = NONE synchronizes sequence values that may have originated from
another subscription. The existing warning is phrased in terms of
copy_data and copying data, which is appropriate for table synchronization
but misleading for sequence synchronization.
Reword the warning, detail, and hint to describe sequence synchronization
and the associated origin = NONE semantics more accurately.
Also fix a typo ("rathen" -> "rather") in a comment in sequencesync.c.
Reported-by: Noah Misch <noah@leadboat.com>
Reported-by: Peter Smith <smithpb2250@gmail.com>
Author: vignesh C <vignesh21@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Backpatch-through: 19, where it was introduced
Discussion: https://postgr.es/m/20260710045217.f0.noahmisch@microsoft.com
adf97c1 allowed expression evaluation to perform hashing, and subsequently 9ca6765 fixed a memory stomping bug in that commit that caused unrelated-to-hashing expression op steps to stomp on the intermediate hash value. The intermediate hash value needs to be maintained when hashing multiple hash keys. 9ca6765 didn't quite get things right when in "strict" mode when it aborted hashing early after encountering a NULL hash key. What was meant to happen was that the expression returns NULL directly to indicate to the caller the value hashed to NULL. The problem was that any EEOP_HASHDATUM_FIRST_STRICT or EEOP_HASHDATUM_NEXT32_STRICT op step that didn't belong to the final key to be hashed would have its op->resnull and op->resvalue pointing to the location to store the intermediate hash value. That's correct for non-NULLs since we bit-rotate the intermediate value and continue hashing, but with the strict case, when we get a NULL key, we immediately jump to the "jumpdone" step. The problem is the jumpdone step expects the ExprState resnull and resvalue fields to be set (as they would be if we didn't abort hashing early due to the NULL), but when we aborted early, the ExprState fields never got set. This would result in inserting records into the hash table that would never match to any join partner, which is a waste of CPU and memory. Here we fix this by having EEOP_HASHDATUM_FIRST_STRICT and EEOP_HASHDATUM_NEXT32_STRICT populate the ExprState resnull and resvalue fields directly when the value to hash is NULL. Although Hash Agg and Hashed Subplans do use hashing from ExprStates, those were unaffected by this bug, as neither of those uses the STRICT op steps. Thanks to Tomas Vondra for finding the offending commit. Reported-by: Dan Stefura <dstefura@bluecatnetworks.com> Author: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/YQBPR0101MB89738FB972FBD02A3640C6D3D6C92@YQBPR0101MB8973.CANPRD01.PROD.OUTLOOK.COM Backpatch-through: 18
When db_comparator() was updated to use pg_cmp_s32(), the arguments were listed in the wrong order. This caused autovacuum to sort the databases by their scores in ascending order instead of descending order. To fix, swap the arguments to pg_cmp_s32(). Oversight in commit 3b42bdb. Reported-by: Хамидуллин Рустам <r.khamidullin@postgrespro.ru> Author: Хамидуллин Рустам <r.khamidullin@postgrespro.ru> Discussion: https://postgr.es/m/5c5a7984-b149-b505-7ad9-2a7766c65b55%40postgrespro.ru Backpatch-through: 17
As previously coded, walsummarizer only wants to read WAL from a file where the TimeLineID in the filename exactly matches the TimeLineID being summarized. But in some cases, when a timeline switch occurs, the WAL file from the old timeline is not archived, because it's never completely filled, so the only way to obtain the contents of that last partial segment is to read from the first segment on the new timeline. Teach WAL summarizer to do that, and add a test case to make sure that it works. Reported-by: Nick Ivanov <nick.ivanov@enterprisedb.com> Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru> Tested-by: Amit Kapila <amit.kapila16@gmail.com> Reviewed-by: Srinath Reddy Sadipiralla <srinath2133@gmail.com> Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com> Reviewed-by: Thom Brown <thom@linux.com> Discussion: http://postgr.es/m/CA+Tgmobr27GpKDZx3_ezW2+C5_g18i+jSK3sGF_cR-_ESv5N5A@mail.gmail.com Backpatch-through: 17
The slow_down interval parsing code checks explicitly for overflow, but since it does that after the signed overflow has already occurred, we end up inviting undefined behavior from the compiler anyway. Use checked arithmetic instead. set_timer() takes a long int in order to interface nicely with libcurl, so use an int32 as the interval counter and clamp to LONG_MAX during conversion to milliseconds. Backpatch to 18, where libpq-oauth was introduced. Reported-by: Andres Freund <andres@anarazel.de> Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://postgr.es/m/qtclihmrkq67ach3xjxyi4qcksstin5qxwsnkqefkmotxwh4g6%40ae2bj6jvcmry Backpatch-through: 18
Reviewed-by: Michael Banck <mbanck@gmx.net> Discussion: https://postgr.es/m/akWIxtcathhoUuCQ%40nathan Backpatch-through: 19 only
…t_io Since 999dec9, pg_stat_io can show read time with zero reads for an IO Context: a foreign IO is counted as a read only in the initiating backend, while other waiters record only the wait time. That violates pgstat_bktype_io_stats_valid(). Relax the check to allow time without a matching operation count, since we want to count read wait time even in backends that did not initiate the read. This also enables future accounting of waits on IO resources (e.g., AIO handles) in backends that didn't start the IO. Author: Andrey Rachitskiy <pl0h0yp1@gmail.com> Reported-by: Justin Pryzby <pryzby@telsasoft.com> Reviewed-by: Melanie Plageman <melanieplageman@gmail.com> Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru> Discussion: https://postgr.es/m/ak5lccE4qiQpOBHn@pryzbyj2023 Backpatch-through: 19
The glossary entry for data checksums workers incorrectly stated that they were auxiliary processes, but they are implemented as background workers. Fix, and while there, simplify the entry by combining the worker and launcher into a single glossary term. Backpatch down to v19 where online checksums were introduced. Author: Daniel Gustafsson <daniel@yesql.se> Reported-by: Fujii Masao <masao.fujii@gmail.com> Reviewed-by: Fujii Masao <masao.fujii@gmail.com> Discussion: https://postgr.es/m/CAHGQGwEv-C9ia+rBYyePzO8F=5FVvS412ZqcOupazuOb5RafNg@mail.gmail.com Backpatch-through: 19
The data checksums entries were seemingly auxiliary processes from reading the code, but they are in fact background workers. Add a comment to clarify. Backpatch down to v19 where online checksums were introduced. Author: Daniel Gustafsson <daniel@yesql.se> Reported-by: Fujii Masao <masao.fujii@gmail.com> Reviewed-by: Fujii Masao <masao.fujii@gmail.com> Discussion: https://postgr.es/m/CAHGQGwFsBjQs2fv7b72hxzGV_fJMh6LAg4E83pNfDOu1jVgWCA@mail.gmail.com Backpatch-through: 19
Alberta (America/Edmonton) moved to permanent UTC-06 on 2026-06-18, which will affect their clocks beginning on 2026-11-01. For lack of any clarity on the point, assume their TZ abbreviation will be CST from that time forward. Morocco (Africa/Casablanca) will move to permanent UTC+00, without daylight saving time transitions, on 2026-09-20. Backpatch-through: 14
This commit reverts f2e4cc4 and 4b3d173, and the subsequent fixes and improvements c5ae07a, 713e553, 52e629b, ecb2508, 9354896, 971017c, 83df16f, e64a9ba, ff8bec8, cdae794, 57f1977, and 881033a. d8af730 and 0392fb9 cancelled each other out and are not reverted separately. The feature is reverted due to multiple design issues which are too late to address in this release cycle. Discussion: https://postgr.es/m/CAN4CZFNCU%3Dt09M%3D%2Br2t9hHLJuujdM4oQ8hCK_Sx-GpfiwMAicw%40mail.gmail.com
Inferred property graph keys used all attributes stored in the primary key index, causing non-key INCLUDE columns to become part of the graph key. Use only the index's key attributes. Author: Muhammad Taha Naveed <m.taha.naveed27@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CAPTqav%2BVUjYgm1jZy0Scy%3D1-PfdgznVj2%3DPwH8disiXF76HEow%40mail.gmail.com
stringify_grant_objtype() treated OBJECT_PROPGRAPH as unused, so pg_event_trigger_ddl_commands() failed with "unsupported object type" when a ddl_command_end trigger inspected GRANT/REVOKE on a property graph. Return "PROPERTY GRAPH" like the GRANT command syntax. Bug: #19637 Reported-by: Alexander Lakhin <exclusion@gmail.com> Author: Andrey Rachitskiy <pl0h0yp1@gmail.com> Reviewed-by: Fujii Masao <masao.fujii@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/19637-4446f72945492ed8%40postgresql.org
Refactor the checks in the function to move all the conditions for when to attempt creating a shell type into one place. Add a check for the array syntax. In addition to rejecting array syntax, another user-visible effect is that the error message is now different if the type specified a typmod. You now get "type does not exist" instead of the more specific "type modifier cannot be specified for shell type". That seems better; the implicit shell type creation exists only for backwards compatibility, and it never worked with type modifiers, so if there's a type modifier it's most likely not because the user tried to create a shell type, Add test for the array syntax, the type modifier, and some other cases for which we don't create shell types. Discussion: https://www.postgresql.org/message-id/de673feb-41b4-4685-b24b-6408b95e58ab@iki.fi Backpatch-through: 14
The pg_restore_*_stats() functions could report SQLSTATE XX000 for invalid variadic arguments, such as an unmatched name/value pair, a NULL argument name, or a non-text argument name. Attribute and extended statistics restores could also report XX000 when the supplied statistics exceed the number of slots PostgreSQL can store. These are not internal errors. They result from invalid caller input or a PostgreSQL implementation limit, but the lack of specific SQLSTATEs made clients treat them as internal errors. Assign appropriate SQLSTATEs to these errors so that applications and tests can classify them correctly. Backpatch to v19, but no further; changing ERRCODE assignments in released stable branches doesn't seem like a good idea. Bug: #19629 Reported-by: Zheng Wang <hackerzheng666@gmail.com> Reported-by: Yanjie Zhao Reported-by: Yiyang Liu Author: Fujii Masao <masao.fujii@gmail.com> Discussion: https://postgr.es/m/19629-76babc04b683594d@postgresql.org Backpatch-through: 19
The test assumed that advancing WAL would lead to a checkpoint that invalidates the obsolete replication slot. If a checkpoint that started before the WAL switch completes first, the following checkpoint can be skipped as idle, so the expected walsender termination is not logged. Force a CHECKPOINT in a background psql session after advancing WAL, so the slot invalidation is exercised deterministically. This has been observed on buildfarm members alligator and partridge: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=alligator&dt=2024-12-13%2001%3A24%3A58 https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=partridge&dt=2026-08-06%2018%3A00%3A11 Backpatch to all supported versions. Reported-by: Alexander Lakhin <exclusion@gmail.com> Author: Hayato Kuroda <kuroda.hayato@fujitsu.com> Reviewed-by: Alexander Lakhin <exclusion@gmail.com> Reviewed-by: Fujii Masao <masao.fujii@gmail.com> Discussion: https://postgr.es/m/0b07ead5-a5da-445e-9698-a7d340708bdf@gmail.com Backpatch-through: 14
Previously, when wal_receiver_create_temp_slot was enabled, a timeline switch could cause the walreceiver to try to create the same temporary replication slot again on the same connection. The slot had already been created before the first streaming attempt and still existed, so the second creation attempt failed with a FATAL error such as "could not create replication slot ...". The walreceiver would later be restarted and streaming replication could continue, so this did not permanently break replication. Nevertheless, the unexpected failure is a bug and should be fixed. Fix this by tracking whether the temporary replication slot has already been created for the lifetime of the walreceiver and skipping subsequent creation attempts. Also copy the retained slot name to shared memory on each streaming attempt, since RequestXLogStreaming() clears it when streaming is restarted without a configured primary slot. This also keeps pg_stat_wal_receiver.slot_name populated after timeline switches. Backpatch to all supported versions. Author: ChangAo Chen <cca5507@qq.com> Reviewed-by: Quan Zongliang <quanzongliang@yeah.net> Reviewed-by: Fujii Masao <masao.fujii@gmail.com> Discussion: https://postgr.es/m/tencent_628FDAF814231923BC8E8357BBBC50F94207@qq.com Backpatch-through: 14
FOREIGN_JOIN target sublists must contain at least two relation identifiers. However, the parser checked only for sublists with exactly one identifier, so FOREIGN_JOIN(()) was accepted. Reject sublists with fewer than two relation identifiers, and add regression coverage. Author: Chao Li <lic@highgo.com> Discussion: http://postgr.es/m/BEDC04E0-6732-4310-95BA-6EC34BC1442C@gmail.com
Commit bd8d9c9 widened MultiXactOffset to 64 bits and widened ControlData.chkpnt_nxtmxoff accordingly, but get_control_data() still read the "Latest checkpoint's NextMultiOffset" line with str2uint(), which returns unsigned int. This commit adds str2uint64(), mirroring the str2uint() helper used for the other control file fields, and reads the offset with it. Backpatch to v19, where MultiXactOffset was widened. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://postgr.es/m/CAD21AoCvzerscfU8o4ARQ793yAGHpQ72r2x5apeC_W2-k=SLCQ@mail.gmail.com Backpatch-through: 19
Commit bd8d9c9 widened MultiXactOffset to 64 bits, but pg_control_checkpoint() still handle checkPointCopy.nextMultiOffset as xid type and declared next_multi_offset column as xid. Since xid is 32 bits wide, an offset above 2^32 was reported truncated, while pg_controldata printed the full value of the same field. This commit reports the column as bigint instead. That matches pg_get_multixact_stats(), which already reports num_members and members_size, both derived from these same offsets, as int8. Backpatch to v19, where MultiXactOffset was widened. Bump catalog version. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://postgr.es/m/CAD21AoCvzerscfU8o4ARQ793yAGHpQ72r2x5apeC_W2-k=SLCQ@mail.gmail.com Backpatch-through: 19
When performing partition pruning with a RANGE partitioned table where the pruning quals are only present for a leading prefix of the partition key, it was possible that partition pruning would accidentally prune away some partitions which shouldn't be pruned and include some partitions that were not needed. This happened due to an incorrectly coded loop bound which was terminating the loop when the bound reached the first or last element in the partition bound array. This resulted in those end elements not being checked in cases where they should be checked. It appears that it might have been coded this way to avoid stepping off the array, but that was done incorrectly as it failed to take into account the direction of travel through the array (the loop can go forwards or backwards). I.e., it's valid to loop when 'off' is the last element if we're going backwards through the array, and valid to loop if 'off' is 0 and we're looping forward through the array, but the code as it was didn't allow that. Here we fix this by moving the loop condition check to after we've calculated the array element to process, and break from the loop if that element is beyond either end of the array. Example of accidentally pruned partition: p: partition by range (a, b); p1: for values from (1, 4) to (1, 7); p2: for values from (1, 7) to (3, 8); p3: for values from (4, 8) to (6, 9); def: default; select * from p where a <= 1; Here p2 was pruned by mistake. Example of accidentally not pruning a partition: p: partition by range (a, b); p1: for values from (7, 2) to (7, 7); def: default; select * from p where a > 7; No partitions would be pruned in this case, despite it being impossible for matching rows to exist in p1. Author: David Rowley <dgrowleyml@gmail.com> Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com> Reviewed-by: Tender Wang <tndrwang@gmail.com> Discussion: https://postgr.es/m/CAApHDvp5ne9AWaH-tG1Lke-USLz3NwWLWTUdP5NT7ypKtcFqcg@mail.gmail.com Backpatch-through: 14
create_append_path() skips cost_append() when an Append has exactly one child whose parallel awareness matches its own, since setrefs.c strips such an Append out entirely. In that case it copies the child's rowcount and costs directly, but it failed to copy disabled_nodes. An Append over a disabled child therefore claimed to contain no disabled nodes, letting a disabled path win over one that is not disabled. This is a regression in v18; before e222534, disable_cost was folded into a path's startup and total costs, so it rode along in the fields this shortcut already copies. Back-patch to v18. This can change plans in stable branches, but only for installations that have explicitly disabled a node type, and only to stop using the node they asked us to avoid. Reported-by: Man Zeng <zengman@halodbtech.com> Author: Tender Wang <tndrwang@gmail.com> Reviewed-by: Richard Guo <guofenglinux@gmail.com> Reviewed-by: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/CAHewXNm_Zx5EDoaD7wo7bq6cfRroNznS+RBCzT_p2-CWQXpgSw@mail.gmail.com Backpatch-through: 18
collate.linux.utf8 skips itself unless version() matches "linux-gnu", and infinite_recurse skips itself when version() matches "powerpc64[^,]*-linux-gnu". configure substitutes the GNU host triplet into that string, but the meson build composes it from host_machine.cpu_family() and host_system, which never carries the ABI suffix. So ever since meson support arrived in 16, collate.linux.utf8 has not run at all on a meson build, and infinite_recurse has been running on ppc64 Linux the very case it means to stay away from. Meson documentation says it reports 'ppc64' instead of 'powerpc64'. Fix by matching "-linux[-,]" and "p(ower)?pc64[^,]*-linux", which match all spellings. Keeping the punctuation on either side confines the match to the platform field. Neither pattern excludes musl, but collate.linux.utf8's other conditions already require a set of glibc locales to be present. Backpatch to 16, where the meson build was introduced. Discussion: https://postgr.es/m/a40b19da-9a02-47b4-8afd-2bbbde8db1e8@dunslane.net Reviewed-By: Jonathan Gonzalez V. <jonathan@abdiel.eu> Reviewed-By: Nazir Bilal Yavuz <byavuz81@gmail.com>
Reported-by: Masahiko Sawada Author: Masahiko Sawada Discussion: https://postgr.es/m/CAD21AoC1dJGqngg_cdT_ayQjOdw6gmSBfVOTtAWOq-5C+XeLZQ@mail.gmail.com Backpatch-through: 19 only
RegisterShmemCallbacks() left the backend in a bad state, if an error occurred in the callbacks or if an allocation failed. Firstly, 'shmem_request_state' was left in wrong state, causing a subsequent call to RegisterShmemCallbacks() to wrongly take the postmaster startup codepath or assertion failures in some other functions. Secondly, the 'pending_shmem_requests' list was not properly cleaned up, causing a subsequent RegisterShmemCallbacks() to try to process the stale, already-freed requests. To fix, add a PG_TRY() block to clean those things up on error. Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com> Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Discussion: https://www.postgresql.org/message-id/CAJTYsWVRRWH48=PcuAo_2Y4Ap6M0QRmzxgUfFkNRtdWK74LjBQ@mail.gmail.com Backpatch-through: 19
If SHMEM_CALLBACKS_ALLOW_AFTER_STARTUP is used to allocate shared memory after startup, but the initialization fails half-way through, the shmem area is left in an indeterminate state. Furthermore, if multiple shmem areas are registered in one RegisterShmemCallbacks() call, some might be allocated while others are not. This commit adds an explicit 'initialized' flag to each shmem area. We still leave behind an uninitialized area on error, but at least they are now clearly marked, and you get a slightly nicer error message if you try to re-register them. It'd be nice to clean up more thoroughly and support actually retrying the allocations, but in practice, the most likely reason for a shmem allocation or initialization to fail is that you are out of shared memory and retrying wouldn't help with that. This isn't exactly a new problem, the old ShmemInitStruct() interface had similar issues if the initialization code failed, or if you allocated multiple structs and some allocations failed. It was just left to the calling code to deal with it. Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com> Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Discussion: https://www.postgresql.org/message-id/CAJTYsWVRRWH48=PcuAo_2Y4Ap6M0QRmzxgUfFkNRtdWK74LjBQ@mail.gmail.com Backpatch-through: 19
analyzejoins.c decided which joins could be dropped by consulting the planner's derived data structures, but then implemented the removal by updating those structures in-place. That is a lot of fiddly work, and nothing keeps it in step with the rest of the planner: remove_leftjoinrel_from_query only bothered to update "parts of the planner's data structures that will actually be consulted later", with no good way to know what those are. Bug #19560 is one consequence. In that report, removing a join leaves an EquivalenceClass that now gives rise to a base restriction clause, but base restriction clauses have already been generated and nothing reconsiders them, so the WHERE condition disappears from the plan and we return wrong answers. The self-join elimination code has the same design and the same type of hazard. We have seen many related bugs over the years too, so it's time to do something drastic. To fix, do the removals by editing root->parse->jointree (which is a far simpler and more stable representation than the derived data), and then have query_planner() discard everything it computed from the jointree and derive it over again. This requires quite a bit less code, and doesn't require touching analyzejoins.c every time we change the data derived by query_planner(). For typical cases it can actually save a bit of planning time, though in cases where we have to iterate the derivation loop many times it does add some time. reduce_unique_semijoins() gets the same treatment: rather than deleting the semijoin's SpecialJoinInfo and relying on the jointree not being consulted again, it now changes the JoinExpr's jointype to JOIN_INNER and recalculates everything. Some plans change in the join regression test. Qual evaluation order shifts in a few cases, because the conditions now reach later planning in jointree order rather than in whatever order the removal code re-distributed them. A few plans improve, since the rebuilt relation targetlists no longer carry columns that only a removed join needed. We also detect a constant-false filter condition whose test used to carry a FIXME label. One plan gets marginally worse, because the old code recomputed attr_needed from equivalence classes after a join removal; that is more accurate than what deconstruct_jointree() derives from the original clauses, but we no longer do that. Making that recomputation happen anyway could be worth doing, but it should be considered independently and perhaps implemented differently. Back-patch to v16, on the grounds that the introduction of varnullingrels in v16 made the old approach significantly more complex and bug-prone; notably, bug #19560 does not manifest before v16. In released branches, do not remove externally-visible fixup functions such as remove_join_clause_from_rels, in case any extensions are relying on them; but they're no longer used by core code. But we must nonetheless break API/ABI for remove_useless_joins, reduce_unique_semijoins, and remove_useless_self_joins, as those now have different outputs and very different behavior than before. It seems unlikely that any extensions are calling those; but just in case, make the breakage more obvious by renaming remove_useless_joins to remove_useless_outer_joins, which is a more sensible name for it anyway since the addition of remove_useless_self_joins. Full disclosure: initial drafts of this patch were made with Claude Opus 4.8. Bug: #19560 Reported-by: Orestis Markou <orestis@orestis.gr> Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Richard Guo <guofenglinux@gmail.com> Reviewed-by: Thom Brown <thom@linux.com> Reviewed-by: Jacob Brazeal <jacob.brazeal@gmail.com> Discussion: https://postgr.es/m/1186816.1784573544@sss.pgh.pa.us Backpatch-through: 16
In v19, pg_stat_autovacuum_scores computes a TOAST table's scores from its own storage parameters alone. On the other hand, autovacuum falls back to the main table's parameters when the TOAST table has none. This means that the view may report scores that don't match what autovacuum would calculate. This contradicts the documented promise that the view generates its results the same way autovacuum workers do. To fix, teach the view to do the same fallback. As in do_autovacuum(), we must make a preliminary pass over pg_class to collect the main tables' parameters, since the pg_class scan may see TOAST tables before their main tables. Commit fad70a09ff for v20 improved autovacuum's handling of TOAST storage parameters and adjusted the view to match, but it was deemed too intrusive to back-patch. This fix is for v19 only. Oversight in commit 87f61f0. Reported-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Discussion: https://postgr.es/m/CAD21AoB1CJRVfCDh8qYuD3eueiygXxk7F3nybgjN0RZXSD-QUw%40mail.gmail.com Backpatch-through: 19 only
AutoVacuumUpdateCostLimit() runs after each nap in vacuum_delay_point() and follows av_nworkersForBalance, but the new limit never reached the shared cost params in the vacuum DSM: propagation only required config reload. Parallel workers computed their delays from the stale limit, so a parallel autovacuum could run at up to twice the configured budget (or half of it) until the next SIGHUP, contradicting the propagation promise in maintenance.sgml. Call parallel_vacuum_propagate_shared_delay_params() after rebalancing. Gated on the leader: parallel workers take the same nap path and must not overwrite the shared parameters. This also adds a test to validate the behaviour: pause the leader at the existing injection point, start a second autovacuum worker and hold it at a new injection point placed after it joined the balance, then check the first parameter load of the parallel workers reports the balanced limit. The hold is needed because a second worker left running can finish its own vacuum before the leader resumes, which puts the balance back where it started. Autovacuum is disabled for everything but the two test tables via thresholds, as a worker spawned by catalog churn would get trapped at the hold point and starve the test of its second worker slot. Backpatch to v19 where autovacuum gained the ability to use parallel vacuum workers. Author: Zsolt Parragi <zsolt.parragi@percona.com> Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://postgr.es/m/CAN4CZFOZtEPwGQ6oa9LvHvN522zEp8h_dW9hHExSh7pVXofoKQ@mail.gmail.com Backpatch-through: 19
The REPACK grammar accepts ONLY before the table name and * after the table name, but that's neither documented nor handled in the code. Perhaps REPACK should support that syntax, but for now let's just bring it in line with its documentation. Oversight in commit ac58465. Reviewed-by: Antonin Houska <ah@cybertec.at> Discussion: https://postgr.es/m/apBTsWGwkLXVh8Ow%40nathan Backpatch-through: 19
The compound flags collected from COMPOUNDFLAG and friends are stored in either the string or the integer member of a union, according to the flag mode that the affix file's FLAG line declares. NIImportOOAffixes() converted each flag as soon as it read it, using the mode in effect at that point, and recorded that mode in the entry. Since FLAG may appear anywhere in the file, including after the compound flags, entries written before and after it could disagree about which member of the union holds the flag. In assert-enabled builds, this would result in an assertion failure. Otherwise, cmpcmdflag() takes the mode from its first argument and applies it to both, so it can read an integer as a char pointer and pass that to strcmp(). Depending on which way the mismatch goes, the result is a segfault while sorting the array, a segfault in the bsearch() that later looks flags up (the lookup key is built with the final mode, so this happens even when the array itself is consistent), or, when both members happen to be readable, no crash at all and a compound flag that is never found, which silently disables compound word splitting. This isn't a security bug because we consider dictionary files to be trusted data, but it's still worth fixing. (In practice, dictionary files usually put the FLAG line first, which is why this went unreported for so long.) Fix by keeping the flags as strings while the file is read and converting them once it has been read in full, when the mode is final. This also makes the position of the FLAG line irrelevant, which is how the flags on AF, SFX and PFX lines are already treated: those are parsed in a second pass and so always use the final mode. That precedent is reason for behaving this way rather than throwing an error. The old ispell file format reaches addCompoundAffixFlagValue() too, from NIImportAffixes(), and returns without entering NIImportOOAffixes(), so it needs the conversion step as well. While we're here, also fix some integer width mismatches: store the result of strtol() into a "long", and cast to int only after we've done range checks. Typically a value too wide for int would fail the range checks anyway, but in some cases it would be silently accepted after truncation to int. Author: Ewan Young <kdbase.hack@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAON2xHN3QmsaySM6DGWa1gttcbJoFh0wjAE-_ZpSPo=LKN1hYw@mail.gmail.com Backpatch-through: 14
Oversight in commit a4f1265. Backpatch-through: 19
Oversight in commit b45137f. Backpatch-through: 19 only
Oversight in commit 38b602b. Backpatch-through: 19
Oversight in commit 283e823. Backpatch-through: 19
Oversight in ce207d2. Author: Corey Huinker <corey.huinker@gmail.com> Discussion: https://postgr.es/m/CADkLM=eo7MtuCE=YjovW+=ASw1=q39qQ3qarrsw+EKfU901ztA@mail.gmail.com Backpatch-through: 18
When to_char() formatted an integer value with a V pattern, it could
return an incorrect result instead of reporting an overflow. V shifts
the decimal point by multiplying the input value by a power of ten before
formatting it, so, for example,
to_char(3, '9V999999999')
requires computing 3 * 10^9. This result does not fit in int4, but
the integer variant of to_char() performed the multiplication using a
plain int32 expression. The intermediate result could therefore
overflow, causing the function to output incorrect digits instead of
raising "integer out of range".
Use dtoi4() and int4mul() for this calculation so that both an
out-of-range multiplier and an out-of-range product are detected, as
with ordinary integer arithmetic. This also matches the existing int8
implementation, which uses dtoi8() and int8mul() for the same
operation.
After this change, to_char() with V format either returns the
correctly formatted result when the scaled value fits in int4, or
raises "integer out of range" when it does not.
Backpatch to all supported versions.
Reported-by: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Reviewed-by: Miłosz Bieniek <bieniek.milosz@proton.me>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CAB8bMivEfqZxOVdzc3kZDN++XshmkEz2t7dfGBU8+oUm864EZg@mail.gmail.com
Backpatch-through: 14
The GRAPH_TABLE examples in ddl.sgml and queries.sgml were written as single long lines, making their structure harder to read in the generated documentation. Reformat these examples so that the graph name, MATCH clause, and COLUMNS clause appear on separate lines, making them easier to read. Author: Koshino Taiki <koshino@sraoss.co.jp> Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Reviewed-by: Fujii Masao <masao.fujii@gmail.com> Discussion: https://postgr.es/m/OS9P286MB64860BD6CD6D4B4B0E1CEDE894A32@OS9P286MB6486.JPNP286.PROD.OUTLOOK.COM Backpatch-through: 19
Commit b7b27eb added batching on top of the direct-index fast path for foreign key checks introduced by 2da86c1: FK rows are buffered and probed in groups using SK_SEARCHARRAY, rather than probed one at a time. This removes that layer and leaves the per-row fast path in place. The batching was proposed late in the v19 cycle and its transactional design was completed after feature freeze. The four most recent commits touching it -- 3b70fa6, f3a52a2, d2a710c and 268958a -- are not fixes to settled code; they are the state model itself, establishing how a batch relates to a subtransaction and to a trigger firing cycle. The open crash where SET CONSTRAINTS ... IMMEDIATE issued from a trigger body walks the after-trigger event list re-entrantly while an outer batch is live is a defect in the most recent of them. The concern is not the number of follow-up fixes but that new transaction and trigger states were still being identified weeks before release, in a code path whose failure mode is a foreign key check that is buffered and never performed. That produces an INSERT that succeeds and a row that violates its constraint permanently, with no error at any point. Testing can show that the states we have enumerated behave correctly; it cannot show the enumeration is complete, and the commit history suggests it is not yet. Removed: ri_FastPathBatchAdd(), ri_FastPathBatchFlush(), ri_FastPathFlushArray(), ri_FastPathFlushLoop(), ri_FastPathGetEntry(), ri_FastPathEndBatch(), ri_FastPathTeardown(), the RI_FastPathEntry and RI_FastPathKey structures, the fast-path entry cache and its in-flush flag, and AtEOSubXact_RI(), which existed only to drop cache entries belonging to an aborting subtransaction. Retained: the per-row fast path (ri_FastPathCheck(), ri_FastPathProbeOne(), ri_LockPKTuple(), recheck_matched_pk_tuple()) and every fix to it -- 68ace96 (domain-typed FK columns), 8c0aa08 (btree-only referenced indexes), 18a15b9 and abca128 (fast-path metadata lifetime), and the nullable-referenced-key handling from a05ece5, whose ri_FastPathFlushArray() site goes with the flush function while its recheck_matched_pk_tuple() site remains. AtEOXact_RI() is retained but reduced to releasing FastPathMeta objects detached by InvalidateConstraintCacheCallBack(); its cache-not-flushed warning has no subject once the cache is gone. In trigger.c, everything the batching used was introduced for it: at 2da86c1 -- the fast path without batching -- afterTriggers.firing_depth, AfterTriggerIsActive(), AfterTriggerCurrentQueryDepth(), the batch callback list and AfterTriggerBatchCallback do not exist, and none of them exist in v18 either. So all of it goes, including the subtransaction-end restore added by f3a52a2, which restores firing_depth and firing_batch_callbacks and has no subject once neither field exists. AfterTriggerFireDeferred()'s loop is restored to its pre-batching form. b7b27eb removed its "all fired" break so that events queued by a batch callback would be seen on the next iteration; with no callbacks to run inside the loop, the break comes back. Batching remains a reasonable optimisation and should be revisited for v20, developed over a full cycle.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.