4370 Commits (82f99688240fa4a870e332e05d564ca6e4cca075)

Author	SHA1	Message	Date
Peter Eisentraut	f0d55b68ef	Fix incorrect lack of Datum conversion in _int_matchsel() ... The code used return (Selectivity) 0.0; where PG_RETURN_FLOAT8(0.0); would be correct. On 64-bit systems, these are pretty much equivalent, but on 32-bit systems, PG_RETURN_FLOAT8() correctly produces a pointer, but the old wrong code would return a null pointer, possibly leading to a crash elsewhere. We think this code is actually not reachable because bqarr_in won't accept an empty query, and there is no other function that will create query_int values. But better be safe and not let such incorrect code lie around. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/8246d7ff-f4b7-4363-913e-827dadfeb145%40eisentraut.org	1 month ago
Etsuro Fujita	d642d23064	Disallow collecting transition tuples from child foreign tables. ... Commit 9e6104c66 disallowed transition tables on foreign tables, but failed to account for cases where a foreign table is a child table of a partitioned/inherited table on which transition tables exist, leading to incorrect transition tuples collected from such foreign tables for queries on the parent table triggering transition capture. This occurred not only for inherited UPDATE/DELETE but for partitioned INSERT later supported by commit 3d956d956, which should have handled it at least for the INSERT case, but didn't. To fix, modify ExecAR*Triggers to throw an error if the given relation is a foreign table requesting transition capture. Also, this commit fixes make_modifytable so that in case of an inherited UPDATE/DELETE triggering transition capture, FDWs choose normal operations to modify child foreign tables, not DirectModify; which is needed because they would otherwise skip the calls to ExecAR*Triggers at execution, causing unexpected behavior. Author: Etsuro Fujita <etsuro.fujita@gmail.com> Reviewed-by: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/CAPmGK14QJYikKzBDCe3jMbpGENnQ7popFmbEgm-XTNuk55oyHg%40mail.gmail.com Backpatch-through: 13	1 month ago
Nathan Bossart	f79ca73d75	Allow resetting unknown custom GUCs with reserved prefixes. ... Currently, ALTER DATABASE/ROLE/SYSTEM RESET [ALL] with an unknown custom GUC with a prefix reserved by MarkGUCPrefixReserved() errors (unless a superuser runs a RESET ALL variant). This is problematic for cases such as an extension library upgrade that removes a GUC. To fix, simply make sure the relevant code paths explicitly allow it. Note that we require superuser or privileges on the parameter to reset it. This is perhaps a bit more restrictive than is necessary, but it's not clear whether further relaxing the requirements is safe. Oversight in commit 88103567cb. The ALTER SYSTEM fix is dependent on commit 2d870b4aef, which first appeared in v17. Unfortunately, back-patching that commit would introduce ABI breakage, and while that breakage seems unlikely to bother anyone, it doesn't seem worth the risk. Hence, the ALTER SYSTEM part of this commit is omitted on v15 and v16. Reported-by: Mert Alev <mert@futo.org> Reviewed-by: Laurenz Albe <laurenz.albe@cybertec.at> Discussion: https://postgr.es/m/18964-ba09dea8c98fccd6%40postgresql.org Backpatch-through: 15	1 month ago
Michael Paquier	75c1eb60b2	Fix incompatibility with libxml2 >= 2.14 ... libxml2 has deprecated the members of xmlBuffer, and it is recommended to access them with dedicated routines. We have only one case in the tree where this shows an impact: xml2/xpath.c where "content" was getting directly accessed. The rest of the code looked fine, checking the PostgreSQL code with libxml2 close to the top of its "2.14" branch. xmlBufferContent() exists since year 2000 based on a check of the upstream libxml2 tree, so let's switch to it. Like 400928b83b, backpatch all the way down as this can have an impact on all the branches already released once newer versions of libxml2 get more popular. Reported-by: Walid Ibrahim <walidib@amazon.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/aGdSdcR4QTjEHX6s@paquier.xyz Backpatch-through: 13	2 months ago
Masahiko Sawada	fc0fb77c55	Fix re-distributing previously distributed invalidation messages during logical decoding. ... Commit 4909b38af0 introduced logic to distribute invalidation messages from catalog-modifying transactions to all concurrent in-progress transactions. However, since each transaction distributes not only its original invalidation messages but also previously distributed messages to other transactions, this leads to an exponential increase in allocation request size for invalidation messages, ultimately causing memory allocation failure. This commit fixes this issue by tracking distributed invalidation messages separately per decoded transaction and not redistributing these messages to other in-progress transactions. The maximum size of distributed invalidation messages that one transaction can store is limited to MAX_DISTR_INVAL_MSG_PER_TXN (8MB). Once the size of the distributed invalidation messages exceeds this threshold, we invalidate all caches in locations where distributed invalidation messages need to be executed. Back-patch to all supported versions where we introduced the fix by commit 4909b38af0. Note that this commit adds two new fields to ReorderBufferTXN to store the distributed transactions. This change breaks ABI compatibility in back branches, affecting third-party extensions that depend on the size of the ReorderBufferTXN struct, though this scenario seems unlikely. Additionally, it adds a new flag to the txn_flags field of ReorderBufferTXN to indicate distributed invalidation message overflow. This should not affect existing implementations, as it is unlikely that third-party extensions use unused bits in the txn_flags field. Bug: #18938 #18942 Author: vignesh C <vignesh21@gmail.com> Reported-by: Duncan Sands <duncan.sands@deepbluecap.com> Reported-by: John Hutchins <john.hutchins@wicourts.gov> Reported-by: Laurence Parry <greenreaper@hotmail.com> Reported-by: Max Madden <maxmmadden@gmail.com> Reported-by: Braulio Fdo Gonzalez <brauliofg@gmail.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com> Discussion: https://postgr.es/m/680bdaf6-f7d1-4536-b580-05c2760c67c6@deepbluecap.com Discussion: https://postgr.es/m/18942-0ab1e5ae156613ad@postgresql.org Discussion: https://postgr.es/m/18938-57c9a1c463b68ce0@postgresql.org Discussion: https://postgr.es/m/CAD1FGCT2sYrP_70RTuo56QTizyc+J3wJdtn2gtO3VttQFpdMZg@mail.gmail.com Discussion: https://postgr.es/m/CANO2=B=2BT1hSYCE=nuuTnVTnjidMg0+-FfnRnqM6kd23qoygg@mail.gmail.com Backpatch-through: 13	3 months ago
Robert Haas	d59ff3be24	pg_prewarm: Allow autoprewarm to use more than 1GB to dump blocks. ... Reported-by: Daria Shanina <vilensipkdm@gmail.com> Author: Daria Shanina <vilensipkdm@gmail.com> Author: Robert Haas <robertmhaas@gmail.com> Backpatch-through: 13	3 months ago
Tom Lane	e76097124f	Disallow "=" in names of reloptions and foreign-data options. ... We store values for these options as array elements with the syntax "name=value", hence a name containing "=" confuses matters when it's time to read the array back in. Since validation of the options is often done (long) after this conversion to array format, that leads to confusing and off-point error messages. We can improve matters by rejecting names containing "=" up-front. (Probably a better design would have involved pairs of array elements, but it's too late now --- and anyway, there's no evident use-case for option names like this. We already reject such names in some other contexts such as GUCs.) Reported-by: Chapman Flack <jcflack@acm.org> Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Chapman Flack <jcflack@acm.org> Discussion: https://postgr.es/m/6830EB30.8090904@acm.org Backpatch-through: 13	3 months ago
Tom Lane	3c31594f55	Fix memory leakage in postgres_fdw's DirectModify code path. ... postgres_fdw tries to use PG_TRY blocks to ensure that it will eventually free the PGresult created by the remote modify command. However, it's fundamentally impossible for this scheme to work reliably when there's RETURNING data, because the query could fail in between invocations of postgres_fdw's DirectModify methods. There is at least one instance of exactly this situation in the regression tests, and the ensuing session-lifespan leak is visible under Valgrind. We can improve matters by using a memory context reset callback attached to the ExecutorState context. That ensures that the PGresult will be freed when the ExecutorState context is torn down, even if control never reaches postgresEndDirectModify. I have little faith that there aren't other potential PGresult leakages in the backend modules that use libpq. So I think it'd be a good idea to apply this concept universally by creating infrastructure that attaches a reset callback to every PGresult generated in the backend. However, that seems too invasive for v18 at this point, let alone the back branches. So for the moment, apply this narrow fix that just makes DirectModify safe. I have a patch in the queue for the more general idea, but it will have to wait for v19. Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Matheus Alcantara <matheusssilv97@gmail.com> Discussion: https://postgr.es/m/2976982.1748049023@sss.pgh.pa.us Backpatch-through: 13	3 months ago
Tom Lane	09c9ae8f6d	Avoid resource leaks when a dblink connection fails. ... If we hit out-of-memory between creating the PGconn and inserting it into dblink's hashtable, we'd lose track of the PGconn, which is quite bad since it represents a live connection to a remote DB. Fix by rearranging things so that we create the hashtable entry first. Also reduce the number of states we have to deal with by getting rid of the separately-allocated remoteConn object, instead allocating it in-line in the hashtable entries. (That incidentally removes a session-lifespan memory leak observed in the regression tests.) There is an apparently-irreducible remaining OOM hazard, which is that if the connection fails at the libpq level (ie it's CONNECTION_BAD) then we have to pstrdup the PGconn's error message before we can release it, and theoretically that could fail. However, in such cases we're only leaking memory not a live remote connection, so I'm not convinced that it's worth sweating over. This is a pretty low-probability failure mode of course, but losing a live connection seems bad enough to justify back-patching. Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Matheus Alcantara <matheusssilv97@gmail.com> Discussion: https://postgr.es/m/1346940.1748381911@sss.pgh.pa.us Backpatch-through: 13	4 months ago
Michael Paquier	130300a154	pg_stat_statements: Fix parameter number gaps in normalized queries ... pg_stat_statements anticipates that certain constant locations may be recorded multiple times and attempts to avoid calculating a length for these locations in fill_in_constant_lengths(). However, during generate_normalized_query() where normalized query strings are generated, these locations are not excluded from consideration. This could increment the parameter number counter for every recorded occurrence at such a location, leading to an incorrect normalization in certain cases with gaps in the numbers reported. For example, take this query: SELECT WHERE '1' IN ('2'::int, '3'::int::text) Before this commit, it would be normalized like that, with gaps in the parameter numbers: SELECT WHERE $1 IN ($3::int, $4::int::text) However the correct, less confusing one should be like that: SELECT WHERE $1 IN ($2::int, $3::int::text) This commit fixes the computation of the parameter numbers to track the number of constants replaced with an $n by a separate counter instead of the iterator used to loop through the list of locations. The underlying query IDs are not changed, neither are the normalized strings for existing PGSS hash entries. New entries with fresh normalized queries would automatically get reshaped based on the new parameter numbering. Issue discovered while discussing a separate problem for HEAD, but this affects all the stable branches. Author: Sami Imseih <samimseih@gmail.com> Discussion: https://postgr.es/m/CAA5RZ0tzxvWXsacGyxrixdhy3tTTDfJQqxyFBRFh31nNHBQ5qA@mail.gmail.com Backpatch-through: 13	4 months ago
Amit Kapila	f6429bd7db	Fix xmin advancement during fast_forward decoding. ... During logical decoding, we advance catalog_xmin of logical too early in fast_forward mode, resulting in required catalog data being removed by vacuum. This mode is normally used to advance the slot without processing the changes, but we still can't let the slot's xmin to advance to an incorrect value. Commit f49a80c481 fixed a similar issue where the logical slot's catalog_xmin was getting advanced prematurely during non-fast-forward mode. During xl_running_xacts processing, instead of directly advancing the slot's xmin to the oldest running xid in the record, it allowed the xmin to be held back for snapshots that can be used for not-yet-replayed transactions, as those might consider older txns as running too. However, it missed the fact that the same problem can happen during fast_forward mode decoding, as we won't build a base snapshot in that mode, and the future call to get_changes from the same slot can miss seeing the required catalog changes leading to incorrect reslts. This commit allows building the base snapshot even in fast_forward mode to prevent the early advancement of xmin. Reported-by: Amit Kapila <amit.kapila16@gmail.com> Author: Zhijie Hou <houzj.fnst@fujitsu.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: shveta malik <shveta.malik@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Backpatch-through: 13 Discussion: https://postgr.es/m/CAA4eK1LqWncUOqKijiafe+Ypt1gQAQRjctKLMY953J79xDBgAg@mail.gmail.com Discussion: https://postgr.es/m/OS0PR01MB57163087F86621D44D9A72BF94BB2@OS0PR01MB5716.jpnprd01.prod.outlook.com	5 months ago
Amit Kapila	90bc4523fd	Fix typo in test file name added in commit 4909b38af0. ... Author: Shlok Kyal <shlok.kyal.oss@gmail.com> Backpatch-through: 13 Discussion: https://postgr.es/m/CANhcyEXsObdjkjxEnq10aJumDpa5J6aiPzgTh_w4KCWRYHLw6Q@mail.gmail.com	5 months ago
Tom Lane	90a3fd811e	Be more wary of corrupt data in pageinspect's heap_page_items(). ... The original intent in heap_page_items() was to return nulls, not throw an error or crash, if an item was sufficiently corrupt that we couldn't safely extract data from it. However, commit d6061f83a utterly missed that memo, and not only put in an un-length-checked copy of the tuple's data section, but also managed to break the check on sane nulls-bitmap length. Either mistake could possibly lead to a SIGSEGV crash if the tuple is corrupt. Bug: #18896 Reported-by: Dmitry Kovalenko <d.kovalenko@postgrespro.ru> Author: Dmitry Kovalenko <d.kovalenko@postgrespro.ru> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/18896-add267b8e06663e3@postgresql.org Backpatch-through: 13	5 months ago
Tom Lane	9a8c16aecc	Fix GIN's shimTriConsistentFn to not corrupt its input. ... Commit 0f21db36d made an assumption that GIN triConsistentFns would not modify their input entryRes[] arrays. But in fact, the "shim" triConsistentFn that we use for opclasses that don't supply their own did exactly that, potentially leading to wrong answers from a GIN index search. Through bad luck, none of the test cases that we have for such opclasses exposed the bug. One response to this could be that the assumption of consistency check functions not modifying entryRes[] arrays is a bad one, but it still seems reasonable to me. Notably, shimTriConsistentFn is itself assuming that with respect to the underlying boolean consistentFn, so it's sure being self-centered in supposing that it gets to do so. Fortunately, it's quite simple to fix shimTriConsistentFn to restore the entry-time state of entryRes[], so let's do that instead. This issue doesn't affect any core GIN opclasses, since they all supply their own triConsistentFns. It does affect contrib modules btree_gin, hstore, and intarray. Along the way, I (tgl) noticed that shimTriConsistentFn failed to pick up on a "recheck" flag returned by its first call to the boolean consistentFn. This may be only a latent problem, since it would be unlikely for a consistentFn to set recheck for the all-false case and not any other cases. (Indeed, none of our contrib modules do that.) Nonetheless, it's formally wrong. Reported-by: Vinod Sridharan <vsridh90@gmail.com> Author: Vinod Sridharan <vsridh90@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAFMdLD7XzsXfi1+DpTqTgrD8XU0i2C99KuF=5VHLWjx4C1pkcg@mail.gmail.com Backpatch-through: 13	5 months ago
Amit Kapila	9f21be08e8	Fix data loss in logical replication. ... Data loss can happen when the DDLs like ALTER PUBLICATION ... ADD TABLE ... or ALTER TYPE ... that don't take a strong lock on table happens concurrently to DMLs on the tables involved in the DDL. This happens because logical decoding doesn't distribute invalidations to concurrent transactions and those transactions use stale cache data to decode the changes. The problem becomes bigger because we keep using the stale cache even after those in-progress transactions are finished and skip the changes required to be sent to the client. This commit fixes the issue by distributing invalidation messages from catalog-modifying transactions to all concurrent in-progress transactions. This allows the necessary rebuild of the catalog cache when decoding new changes after concurrent DDL. We observed performance regression primarily during frequent execution of *publication DDL* statements that modify the published tables. The regression is minor or nearly nonexistent for DDLs that do not affect the published tables or occur infrequently, making this a worthwhile cost to resolve a longstanding data loss issue. An alternative approach considered was to take a strong lock on each affected table during publication modification. However, this would only address issues related to publication DDLs (but not the ALTER TYPE ...) and require locking every relation in the database for publications created as FOR ALL TABLES, which is impractical. The bug exists in all supported branches, but we are backpatching till 14. The fix for 13 requires somewhat bigger changes than this fix, so the fix for that branch is still under discussion. Reported-by: hubert depesz lubaczewski <depesz@depesz.com> Reported-by: Tomas Vondra <tomas.vondra@enterprisedb.com> Author: Shlok Kyal <shlok.kyal.oss@gmail.com> Author: Hayato Kuroda <kuroda.hayato@fujitsu.com> Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Tested-by: Benoit Lobréau <benoit.lobreau@dalibo.com> Backpatch-through: 14 Discussion: https://postgr.es/m/de52b282-1166-1180-45a2-8d8917ca74c6@enterprisedb.com Discussion: https://postgr.es/m/CAD21AoAenVqiMjpN-PvGHL1N9DWnHSq673bfgr6phmBUzx=kLQ@mail.gmail.com	5 months ago
Noah Misch	63f6ecb6b0	Make dblink interruptible, via new libpqsrv APIs. ... This replaces dblink's blocking libpq calls, allowing cancellation and allowing DROP DATABASE (of a database not involved in the query). Apart from explicit dblink_cancel_query() calls, dblink still doesn't cancel the remote side. The replacement for the blocking calls consists of new, general-purpose query execution wrappers in the libpqsrv facility. Out-of-tree extensions should adopt these. The original commit d3c5f37dd5 did not back-patch. Back-patch now to v16-v13, bringing coverage to all supported versions. This back-patch omits the orignal's refactoring in postgres_fdw. Discussion: https://postgr.es/m/20231122012945.74@rfd.leadboat.com	5 months ago
Tom Lane	0e86bad380	Prevent assertion failure in contrib/pg_freespacemap. ... Applying pg_freespacemap() to a relation lacking storage (such as a view) caused an assertion failure, although there was no ill effect in non-assert builds. Add an error check for that case. Bug: #18866 Reported-by: Robins Tharakan <tharakan@gmail.com> Author: Tender Wang <tndrwang@gmail.com> Reviewed-by: Euler Taveira <euler@eulerto.com> Discussion: https://postgr.es/m/18866-d68926d0f1c72d44@postgresql.org Backpatch-through: 13	6 months ago
Tom Lane	bb85d09355	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	8 months ago
Tom Lane	cdeed4de77	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	8 months ago
Thomas Munro	3181befdca	Restore smgrtruncate() prototype in back-branches. ... It's possible that external code is calling smgrtruncate(). Any external callers might like to consider the recent changes to RelationTruncate(), but commit 38c579b0 should not have changed the function prototype in the back-branches, per ABI stability policy. Restore smgrtruncate()'s traditional argument list in the back-branches, but make it a wrapper for a new function smgrtruncate2(). The three callers in core can use smgrtruncate2() directly. In master (18-to-be), smgrtruncate2() is effectively renamed to smgrtruncate(), so this wart is cleaned up. Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/CA%2BhUKG%2BThae6x6%2BjmQiuALQBT2Ae1ChjMh1%3DkMvJ8y_SBJZrvA%40mail.gmail.com	8 months ago
Thomas Munro	fb540b6aa5	Fix corruption when relation truncation fails. ... RelationTruncate() does three things, while holding an AccessExclusiveLock and preventing checkpoints: 1. Logs the truncation. 2. Drops buffers, even if they're dirty. 3. Truncates some number of files. Step 2 could previously be canceled if it had to wait for I/O, and step 3 could and still can fail in file APIs. All orderings of these operations have data corruption hazards if interrupted, so we can't give up until the whole operation is done. When dirty pages were discarded but the corresponding blocks were left on disk due to ERROR, old page versions could come back from disk, reviving deleted data (see pgsql-bugs #18146 and several like it). When primary and standby were allowed to disagree on relation size, standbys could panic (see pgsql-bugs #18426) or revive data unknown to visibility management on the primary (theorized). Changes: * WAL is now unconditionally flushed first * smgrtruncate() is now called in a critical section, preventing interrupts and causing PANIC on file API failure * smgrtruncate() has a new parameter for existing fork sizes, because it can't call smgrnblocks() itself inside a critical section The changes apply to RelationTruncate(), smgr_redo() and pg_truncate_visibility_map(). That last is also brought up to date with other evolutions of the truncation protocol. The VACUUM FileTruncate() failure mode had been discussed in older reports than the ones referenced below, with independent analysis from many people, but earlier theories on how to fix it were too complicated to back-patch. The more recently invented cancellation bug was diagnosed by Alexander Lakhin. Other corruption scenarios were spotted by me while iterating on this patch and earlier commit 75818b3a. Back-patch to all supported releases. Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Robert Haas <robertmhaas@gmail.com> Reported-by: rootcause000@gmail.com Reported-by: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/18146-04e908c662113ad5%40postgresql.org Discussion: https://postgr.es/m/18426-2d18da6586f152d6%40postgresql.org	9 months ago
Michael Paquier	c1c9df3159	Replace durable_rename_excl() by durable_rename(), take two ... durable_rename_excl() attempts to avoid overwriting any existing files by using link() and unlink(), and it falls back to rename() on some platforms (aka WIN32), which offers no such overwrite protection. Most callers use durable_rename_excl() just in case there is an existing file, but in practice there shouldn't be one (see below for more details). Furthermore, failures during durable_rename_excl() can result in multiple hard links to the same file. As per Nathan's tests, it is possible to end up with two links to the same file in pg_wal after a crash just before unlink() during WAL recycling. Specifically, the test produced links to the same file for the current WAL file and the next one because the half-recycled WAL file was re-recycled upon restarting, leading to WAL corruption. This change replaces all the calls of durable_rename_excl() to durable_rename(). This removes the protection against accidentally overwriting an existing file, but some platforms are already living without it and ordinarily there shouldn't be one. The function itself is left around in case any extensions are using it. It will be removed on HEAD via a follow-up commit. Here is a summary of the existing callers of durable_rename_excl() (see second discussion link at the bottom), replaced by this commit. First, basic_archive used it to avoid overwriting an archive concurrently created by another server, but as mentioned above, it will still overwrite files on some platforms. Second, xlog.c uses it to recycle past WAL segments, where an overwrite should not happen (origin of the change at f0e37a8) because there are protections about the WAL segment to select when recycling an entry. The third and last area is related to the write of timeline history files. writeTimeLineHistory() will write a new timeline history file at the end of recovery on promotion, so there should be no such files for the same timeline. What remains is writeTimeLineHistoryFile(), that can be used in parallel by a WAL receiver and the startup process, and some digging of the buildfarm shows that EEXIST from a WAL receiver can happen with an error of "could not link file \"pg_wal/xlogtemp.NN\" to \"pg_wal/MM.history\", which would cause an automatic restart of the WAL receiver as it is promoted to FATAL, hence this should improve the stability of the WAL receiver as rename() would overwrite an existing TLI history file already fetched by the startup process at recovery. This is the second time this change is attempted, ccfbd92 being the first one, but this time no assertions are added for the case of a TLI history file written concurrently by the WAL receiver or the startup process because we can expect one to exist (some of the TAP tests are able to trigger with a proper timing). This commit has been originally applied on v16~ as of dac1ff3090, and we have received more reports of this issue, where clusters can become corrupted at replay in older stable branches with multiple links pointing to the same physical WAL segment file. This backpatch addresses the problem for the v13~v15 range. Author: Nathan Bossart Reviewed-by: Robert Haas, Kyotaro Horiguchi, Michael Paquier Discussion: https://postgr.es/m/20220407182954.GA1231544@nathanxps13 Discussion: https://postgr.es/m/Ym6GZbqQdlalSKSG@paquier.xyz Discussion: https://postgr.es/m/CAJhEC04tBkYPF4q2uS_rCytauvNEVqdBAzasBEokfceFhF=KDQ@mail.gmail.com	9 months ago
Michael Paquier	1df1e1e787	Fix invalidation of local pgstats references for entry reinitialization ... 818119afcc has introduced the "generation" concept in pgstats entries, incremented a counter when a pgstats entry is reinitialized, but it did not count on the fact that backends still holding local references to such entries need to be refreshed if the cache age is outdated. The previous logic only updated local references when an entry was dropped, but it needs also to consider entries that are reinitialized. This matters for replication slot stats (as well as custom pgstats kinds in 18~), where concurrent drops and creates of a slot could cause incorrect stats to be locally referenced. This would lead to an assertion failure at shutdown when writing out the stats file, as the backend holding an outdated local reference would not be able to drop during its shutdown sequence the stats entry that should be dropped, as the last process holding a reference to the stats entry. The checkpointer was then complaining about such an entry late in the shutdown sequence, after the shutdown checkpoint is finished with the control file updated, causing the stats file to not be generated. In non-assert builds, the entry would just be skipped with the stats file written. Note that only logical replication slots use statistics. A test case based on TAP is added to test_decoding, where a persistent connection peeking at a slot's data is kept with concurrent drops and creates of the same slot. This is based on the isolation test case that Anton has sent. As it requires a node shutdown with a check to make sure that the stats file is written with this specific sequence of events, TAP is used instead. Reported-by: Anton A. Melnikov Reviewed-by: Bertrand Drouvot Discussion: https://postgr.es/m/56bf8ff9-dd8c-47b2-872a-748ede82af99@postgrespro.ru Backpatch-through: 15	9 months ago
Peter Geoghegan	16a2bb0793	Count contrib/bloom index scans in pgstat view. ... Maintain the pg_stat_user_indexes.idx_scan pgstat counter during contrib/Bloom index scans. Oversight in commit 9ee014fc, which added the Bloom index contrib module. Author: Masahiro Ikeda <ikedamsh@oss.nttdata.com> Reviewed-By: Peter Geoghegan <pg@bowt.ie> Discussion: https://postgr.es/m/c48839d881388ee401a01807c686004d@oss.nttdata.com Backpatch: 13- (all supported branches).	10 months ago
Alexander Korotkov	713b8546ab	Fix arrays comparison in CompareOpclassOptions() ... The current code calls array_eq() and does not provide FmgrInfo. This commit provides initialization of FmgrInfo and uses C collation as the safe option for text comparison because we don't know anything about the semantics of opclass options. Backpatch to 13, where opclass options were introduced. Reported-by: Nicolas Maus Discussion: https://postgr.es/m/18692-72ea398df3ec6712%40postgresql.org Backpatch-through: 13	10 months ago
Amit Kapila	5ce0dcc99a	Stabilize the test added by commit 022564f60c. ... The test was unstable in branches 14 and 15 as we were relying on the number of changes in the table having a toast column to start streaming. On branches >= 16, we have a GUC debug_logical_replication_streaming which can stream each change, so the test was stable in those branches. Change the test to use PREPARE TRANSACTION as that should make the result consistent and test the code changed in 022564f60c. Reported-by: Daniel Gustafsson as per buildfarm Author: Hou Zhijie, Amit Kapila Backpatch-through: 14 Discussion: https://postgr.es/m/8C2F86AA-981E-4803-B14D-E264C0255330@yesql.se	11 months ago
Nathan Bossart	01731eeeaa	Fix Y2038 issues with MyStartTime. ... Several places treat MyStartTime as a "long", which is only 32 bits wide on some platforms. In reality, MyStartTime is a pg_time_t, i.e., a signed 64-bit integer. This will lead to interesting bugs on the aforementioned systems in 2038 when signed 32-bit integers are no longer sufficient to store Unix time (e.g., "pg_ctl start" hanging). To fix, ensure that MyStartTime is handled as a 64-bit value everywhere. (Of course, users will need to ensure that time_t is 64 bits wide on their system, too.) Co-authored-by: Max Johnson Discussion: https://postgr.es/m/CO1PR07MB905262E8AC270FAAACED66008D682%40CO1PR07MB9052.namprd07.prod.outlook.com Backpatch-through: 12	11 months ago
Amit Kapila	8175a7d11f	Fix fetching default toast value during decoding of in-progress transactions. ... During logical decoding of in-progress transactions, we perform the toast table scan while fetching the default toast value for an attribute. We forgot to initialize the flag during this scan to indicate that the system table scan is in progress. We need this flag to ensure that during logical decoding we never directly access the tableam or heap APIs because we check for concurrent aborts only in systable_* APIs. Reported-by: Alexander Lakhin Author: Takeshi Ideriha, Hou Zhijie Reviewed-by: Amit Kapila, Hou Zhijie Backpatch-through: 14 Discussion: https://postgr.es/m/18641-6687273b7f15269d@postgresql.org	11 months ago
Tom Lane	634804885f	Replace usages of xmlXPathCompile() with xmlXPathCtxtCompile(). ... In existing releases of libxml2, xmlXPathCompile can be driven to stack overflow because it fails to protect itself against too-deeply-nested input. While there is an upstream fix as of yesterday, it will take years for that to propagate into all shipping versions. In the meantime, we can protect our own usages basically for free by calling xmlXPathCtxtCompile instead. (The actual bug is that libxml2 keeps its nesting counter in the xmlXPathContext, and its parsing code was willing to just skip counting nesting levels if it didn't have a context. So if we supply a context, all is well. It seems odd actually that it works at all to not supply a context, because this means that XPath parsing does not have access to XML namespace info. Apparently libxml2 never checks namespaces until runtime? Anyway, this seems like good future-proofing even if its only immediate effect is to dodge a bug.) Sadly, this hack only offers protection with libxml2 2.9.11 and newer. Before that there are multiple similar problems, so if you are processing untrusted XML it behooves you to get a newer version. But we have some pretty old libxml2 in the buildfarm, so it seems impractical to add a regression test to verify this fix. Per bug #18617 from Jingzhou Fu. Back-patch to all supported versions. Discussion: https://postgr.es/m/18617-1cee4d2ed1f4e7ae@postgresql.org Discussion: https://gitlab.gnome.org/GNOME/libxml2/-/issues/799	1 year ago
Nathan Bossart	a63aef5e49	Fix contrib/pageinspect's test for sequences. ... I managed to break this test in two different ways in commit 05036a3155. First, the output of the new call to tuple_data_split() on the test sequence is dependent on endianness. This is fixed by setting a special start value for the test sequence that produces the same output regardless of the endianness of the machine. Second, on versions older than v15, the new test case fails under "force_parallel_mode = regress" with the following error: ERROR: cannot access temporary tables during a parallel operation This is because pageinspect's disk-accessing functions are incorrectly marked PARALLEL SAFE on versions older than v15 (see commit aeaaf520f4 for details). This one is fixed by changing the test sequence to be permanent. The only reason it was previously marked temporary was to avoid needing a DROP SEQUENCE command at the end of the test. Unlike some other tests in this file, the use of a permanent sequence here shouldn't result in any test instability like what was fixed by commit e2933a6e11. Reviewed-by: Tom Lane Discussion: https://postgr.es/m/ZuOKOut5hhDlf_bP%40nathan Backpatch-through: 12	1 year ago
Nathan Bossart	e03042a700	Reintroduce support for sequences in pgstattuple and pageinspect. ... Commit 4b82664156 restricted a number of functions provided by contrib modules to only relations that use the "heap" table access method. Sequences always use this table access method, but they do not advertise as such in the pg_class system catalog, so the aforementioned commit also (presumably unintentionally) removed support for sequences from some of these functions. This commit reintroduces said support for sequences to these functions and adds a couple of relevant tests. Co-authored-by: Ayush Vatsa Reviewed-by: Robert Haas, Michael Paquier, Matthias van de Meent Discussion: https://postgr.es/m/CACX%2BKaP3i%2Bi9tdPLjF5JCHVv93xobEdcd_eB%2B638VDvZ3i%3DcQA%40mail.gmail.com Backpatch-through: 12	1 year ago
Masahiko Sawada	e81e53a0c1	Restrict accesses to non-system views and foreign tables during pg_dump. ... When pg_dump retrieves the list of database objects and performs the data dump, there was possibility that objects are replaced with others of the same name, such as views, and access them. This vulnerability could result in code execution with superuser privileges during the pg_dump process. This issue can arise when dumping data of sequences, foreign tables (only 13 or later), or tables registered with a WHERE clause in the extension configuration table. To address this, pg_dump now utilizes the newly introduced restrict_nonsystem_relation_kind GUC parameter to restrict the accesses to non-system views and foreign tables during the dump process. This new GUC parameter is added to back branches too, but these changes do not require cluster recreation. Back-patch to all supported branches. Reviewed-by: Noah Misch Security: CVE-2024-7348 Backpatch-through: 12	1 year ago
Thomas Munro	e52a44b8a4	Skip citext_utf8 test on Windows. ... Back-patch of commit cff4e5a3 to 15 and 16, per request from Oleg Tselebrovskiy. Original commit message: On other Windows build farm animals it is already skipped because they don't use UTF-8 encoding. On "hamerkop", UTF-8 is used, and then the test fails. It is not clear to me (a non-Windows person looking only at buildfarm evidence) whether Windows is less sophisticated than other OSes and doesn't know how to downcase Turkish İ with the standard Unicode database, or if it is more sophisticated than other systems and uses locale-specific behavior like ICU does. Whichever the reason, the result is the same: we need to skip the test on Windows, just as we already do for ICU, at least until a Windows-savvy developer comes up with a better idea. The technique for detecting the OS is borrowed from collate.windows.win1252.sql. This was anticipated by commit c2e8bd27, but the problem only surfaced when Windows build farm animals started using Meson. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CA%2BhUKGJ1LeC3aE2qQYTK95rFVON3ZVoTQpTKJqxkHdtEyawH4A%40mail.gmail.com	1 year ago
Etsuro Fujita	f39f3e0fbe	postgres_fdw: Avoid "cursor can only scan forward" error. ... Commit d844cd75a disallowed rewind in a non-scrollable cursor to resolve anomalies arising from such a cursor operation. However, this failed to take into account the assumption in postgres_fdw that when rescanning a foreign relation, it can rewind the cursor created for scanning the foreign relation without specifying the SCROLL option, regardless of its scrollability, causing this error when it tried to do such a rewind in a non-scrollable cursor. Fix by modifying postgres_fdw to instead recreate the cursor, regardless of its scrollability, when rescanning the foreign relation. (If we had a way to check its scrollability, we could improve this by rewinding it if it is scrollable and recreating it if not, but we do not have it, so this commit modifies it to recreate it in any case.) Per bug #17889 from Eric Cyr. Devrim Gunduz also reported this problem. Back-patch to v15 where that commit enforced the prohibition. Reviewed by Tom Lane. Discussion: https://postgr.es/m/17889-e8c39a251d258dda%40postgresql.org Discussion: https://postgr.es/m/b415ac3255f8352d1ea921cf3b7ba39e0587768a.camel%40gunduz.org	1 year ago
Masahiko Sawada	aee8c2b954	Fix possibility of logical decoding partial transaction changes. ... When creating and initializing a logical slot, the restart_lsn is set to the latest WAL insertion point (or the latest replay point on standbys). Subsequently, WAL records are decoded from that point to find the start point for extracting changes in the DecodingContextFindStartpoint() function. Since the initial restart_lsn could be in the middle of a transaction, the start point must be a consistent point where we won't see the data for partial transactions. Previously, when not building a full snapshot, serialized snapshots were restored, and the SnapBuild jumps to the consistent state even while finding the start point. Consequently, the slot's restart_lsn and confirmed_flush could be set to the middle of a transaction. This could lead to various unexpected consequences. Specifically, there were reports of logical decoding decoding partial transactions, and assertion failures occurred because only subtransactions were decoded without decoding their top-level transaction until decoding the commit record. To resolve this issue, the changes prevent restoring the serialized snapshot and jumping to the consistent state while finding the start point. On v17 and HEAD, a flag indicating whether snapshot restores should be skipped has been added to the SnapBuild struct, and SNAPBUILD_VERSION has been bumpded. On backbranches, the flag is stored in the LogicalDecodingContext instead, preserving on-disk compatibility. Backpatch to all supported versions. Reported-by: Drew Callahan Reviewed-by: Amit Kapila, Hayato Kuroda Discussion: https://postgr.es/m/2444AA15-D21B-4CCE-8052-52C7C2DAFE5C%40amazon.com Backpatch-through: 12	1 year ago
Heikki Linnakangas	d5fd7865f0	Backport BackgroundPsql perl test module ... Backport the new BackgroundPsql modules and the constructor functions, background_psql() and interactive_psql, to all supported branches. That makes it easier to backpatch tests that use it. BackgroundPsql was introduced in version 16. On version 16, this commit backports just the new timeout argument from master (commit 334f512f45). On older branches, the whole facility. This includes the change to `use warnings FATAL => 'all'`, which we haven't otherwise backported, but it seems good to keep the file identical across branches. Discussion: https://www.postgresql.org/message-id/b7c64f20-ea01-4f15-9088-0cd6832af149@iki.fi	1 year ago
Etsuro Fujita	b33c141cc5	postgres_fdw: Refuse to send FETCH FIRST WITH TIES to remote servers. ... Previously, when considering LIMIT pushdown, postgres_fdw failed to check whether the query has this clause, which led to pushing false LIMIT clauses, causing incorrect results. This clause has been supported since v13, so we need to do a remote-version check before deciding that it will be safe to push such a clause, but we do not currently have a way to do the check (without accessing the remote server); disable pushing such a clause for now. Oversight in commit 357889eb1. Back-patch to v13, where that commit added the support. Per bug #18467 from Onder Kalaci. Patch by Japin Li, per a suggestion from Tom Lane, with some changes to the comments by me. Review by Onder Kalaci, Alvaro Herrera, and me. Discussion: https://postgr.es/m/18467-7bb89084ff03a08d%40postgresql.org	1 year ago
Tom Lane	6c85e3359b	Make postgres_fdw request remote time zone 'GMT' not 'UTC'. ... This should have the same results for all practical purposes. The advantage of selecting 'GMT' is that it's guaranteed to work even when the remote system's timezone database is missing entries, because pg_tzset() hard-wires handling of that, at least in 9.2 and later. (It seems like it would be a good idea to similarly hard-wire correct handling of 'UTC', but that'll be a little more invasive than I want to consider back-patching. Leave that for another day when we're not in feature freeze.) Per trouble report from Adnan Dautovic. Back-patch to all supported branches. Discussion: https://postgr.es/m/465248.1712211585@sss.pgh.pa.us	1 year ago
Michael Paquier	689ba4f1c4	xml2: Replace deprecated routines with recommended ones ... Some functions are used in the tree and are currently marked as deprecated by upstream. This commit refreshes the code to use the recommended functions, leading to the following changes: - xmlSubstituteEntitiesDefault() is gone, and needs to be replaced with XML_PARSE_NOENT for the paths doing the parsing. - xmlParseMemory() -> xmlReadMemory(). These functions, as well as more functions setting global states, have been officially marked as deprecated by upstream in August 2022. Their replacements exist since the 2001-ish area, as far as I have checked, so that should be safe. This has been originally applied as 65c5864d7f without a backpatch, and this has come up as well when working on 400928b83. Per request from Tom Lane, for new buildfarm member indri that is able to see deprecation warnings with xmlSubstituteEntitiesDefault() in 16 and older stable branches. Author: Dmitry Koval Discussion: https://postgr.es/m/18274-98d16bc03520665f@postgresql.org Discussion: https://postgr.es/m/1012981.1713222862@sss.pgh.pa.us Bakpatch-through: 12	1 year ago
Etsuro Fujita	3f96d113ff	Fix bogus coding in ExecAppendAsyncEventWait(). ... No configured-by-FDW events would result in "return" directly out of a PG_TRY block, making the exception stack dangling. Repair. Oversight in commit 501cfd07d; back-patch to v14, like that commit, but as we do not have this issue in HEAD (cf. commit 50c67c201), no need to apply this patch to it. In passing, improve a comment about the handling of in-process requests in a postgres_fdw.c function called from this function. Alexander Pyhalov, with comment adjustment/improvement by me. Discussion: https://postgr.es/m/425fa29a429b21b0332737c42a4fdc70%40postgrespro.ru	1 year ago
Alexander Korotkov	0d466bce9e	amcheck: Normalize index tuples containing uncompressed varlena ... It might happen that the varlena value wasn't compressed by index_form_tuple() due to current storage parameters. If compression is currently enabled, we need to compress such values to match index tuple coming from the heap. Backpatch to all supported versions. Discussion: https://postgr.es/m/flat/7bdbe559-d61a-4ae4-a6e1-48abdf3024cc%40postgrespro.ru Author: Andrey Borodin Reviewed-by: Alexander Lakhin, Michael Zhilin, Jian He, Alexander Korotkov Backpatch-through: 12	1 year ago
Alexander Korotkov	54e6184db3	amcheck: Support for different header sizes of short varlena datum ... In the heap, tuples may contain short varlena datum with both 1B header and 4B headers. But the corresponding index tuple should always have such varlena's with 1B headers. So, for fingerprinting, we need to convert. Backpatch to all supported versions. Discussion: https://postgr.es/m/flat/7bdbe559-d61a-4ae4-a6e1-48abdf3024cc%40postgrespro.ru Author: Michael Zhilin Reviewed-by: Alexander Lakhin, Andrey Borodin, Jian He, Alexander Korotkov Backpatch-through: 12	1 year ago
David Rowley	ab64b275ad	Fix deparsing of Consts in postgres_fdw ORDER BY ... For UNION ALL queries where a union child query contained a foreign table, if the targetlist of that query contained a constant, and the top-level query performed an ORDER BY which contained the column for the constant value, then postgres_fdw would find the EquivalenceMember with the Const and then try to produce an ORDER BY containing that Const. This caused problems with INT typed Consts as these could appear to be requests to order by an ordinal column position rather than the constant value. This could lead to either an error such as: ERROR: ORDER BY position <int const> is not in select list or worse, if the constant value is a valid column, then we could just sort by the wrong column altogether. Here we fix this issue by just not including these Consts in the ORDER BY clause. In passing, add a new section for testing ORDER BY in the postgres_fdw tests and move two existing tests which were misplaced in the WHERE clause testing section into it. Reported-by: Michał Kłeczek Reviewed-by: Ashutosh Bapat, Richard Guo Bug: #18381 Discussion: https://postgr.es/m/0714C8B8-8D82-4ABB-9F8D-A0C3657E7B6E%40kleczek.org Discussion: https://postgr.es/m/18381-137456acd168bf93%40postgresql.org Backpatch-through: 12, oldest supported version	2 years ago
Daniel Gustafsson	f74b5c5bc6	pgcrypto: Fix check for buffer size ... The code copying the PGP block into the temp buffer failed to account for the extra 2 bytes in the buffer which are needed for the prefix. If the block was oversized, subsequent checks of the prefix would have exceeded the buffer size. Since the block sizes are hardcoded in the list of supported ciphers it can be verified that there is no live bug here. Backpatch all the way for consistency though, as this bug is old. Author: Mikhail Gribkov <youzhick@gmail.com> Discussion: https://postgr.es/m/CAMEv5_uWvcMCMdRFDsJLz2Q8g16HEa9xWyfrkr+FYMMFJhawOw@mail.gmail.com Backpatch-through: v12	2 years ago
Tom Lane	3f8ac13b19	Fix incompatibilities with libxml2 >= 2.12.0. ... libxml2 changed the required signature of error handler callbacks to make the passed xmlError struct "const". This is causing build failures on buildfarm member caiman, and no doubt will start showing up in the field quite soon. Add a version check to adjust the declaration of xml_errorHandler() according to LIBXML_VERSION. 2.12.x also produces deprecation warnings for contrib/xml2/xpath.c's assignment to xmlLoadExtDtdDefaultValue. I see no good reason for that to still be there, seeing that we disabled external DTDs (at a lower level) years ago for security reasons. Let's just remove it. Back-patch to all supported branches, since they might all get built with newer libxml2 once it gets a bit more popular. (The back branches produce another deprecation warning about xpath.c's use of xmlSubstituteEntitiesDefault(). We ought to consider whether to back-patch all or part of commit 65c5864d7 to silence that. It's less urgent though, since it won't break the buildfarm.) Discussion: https://postgr.es/m/1389505.1706382262@sss.pgh.pa.us	2 years ago
Amit Kapila	b793a416bf	Fix catalog lookup due to wrong snapshot for subtransactions during decoding. ... In commit 272248a0c, we fixed the catalog lookup due to the wrong snapshot for transactions and subtransactions during decoding. We failed to consider the case where top-level xact is already marked as containing catalog change but its subtransaction is not yet marked as containing catalog change even though it contained such a change. This can happen when during decoding, none of the WAL records from the subtransaction was decoded and top-level xact contains a DDL. We fix it by marking the transaction and all its subtransactions as containing catalog changes if the top-level xact contains any catalog change and it is present in the initial running xacts array. This fix is required only for 14 and 15 because in prior branches we already always mark the transaction and all its subtransactions as containing catalog changes in the same case. In 16 and above, we preserve the list of transaction IDs and sub-transaction IDs, that have modified catalogs and are running during snapshot serialization, to the serialized snapshot (see commit 7f13ac8123). Author: Fei Changhong Reviewed-by: Amit Kapila, Hayato Kuroda, Andy Fan Discussion: https://postgr.es/m/18280-4c8060178cb41750@postgresql.org	2 years ago
Tom Lane	940ab02b53	Fix integer-overflow problem in intarray's g_int_decompress(). ... An array element equal to INT_MAX gave this code indigestion, causing an infinite loop that surely ended in SIGSEGV. We fixed some nearby problems awhile ago (cf 757c5182f) but missed this. Report and diagnosis by Alexander Lakhin (bug #18273); patch by me Discussion: https://postgr.es/m/18273-9a832d1da122600c@postgresql.org	2 years ago
Michael Paquier	2e08440d61	pageinspect: Fix failure with hash_bitmap_info() for partitioned indexes ... This function reads directly a page from a relation, relying on index_open() to open the index to read from. Unfortunately, this would crash when using partitioned indexes, as these can be opened with index_open() but they have no physical pages. Alexander has fixed the module, while I have written the test. Author: Alexander Lakhin, Michael Paquier Discussion: https://postgr.es/m/18246-f4d9ff7cb3af77e6@postgresql.org Backpatch-through: 12	2 years ago
Michael Paquier	b745f16804	pgstattuple: Fix failure with pgstathashindex() for partitioned indexes ... As coded, the function relied on index_open() when opening an index relation, allowing partitioned indexes to be processed by pgstathashindex(). This was leading to a "could not open file" error because partitioned indexes have no physical files, or to a crash with an assertion failure (like on HEAD). This issue is fixed by applying the same checks as the other stat functions for indexes, with a lookup at both RELKIND_INDEX and the index AM expected. Author: Alexander Lakhin Discussion: https://postgr.es/m/18246-f4d9ff7cb3af77e6@postgresql.org Backpatch-through: 12	2 years ago
Heikki Linnakangas	481d7d1c01	Fix resource leak when a FDW's ForeignAsyncRequest function fails ... If an error is thrown after calling CreateWaitEventSet(), the memory of a WaitEventSet is free'd as it's allocated in the short-lived memory context, but the file descriptor (on epoll- or kqueue-based systems) or handles (on Windows) that it contains are leaked. Use PG_TRY-FINALLY to ensure it gets freed. (On master, I will apply a better fix, using ResourceOwners to track the WaitEventSet, but that's not backpatchable.) The added test doesn't check for leaking resources, so it passed even before this commit. But at least it covers the code path. In the passing, fix misleading comment on what the 'nevents' argument to WaitEventSetWait means. Report by Alexander Lakhin, analysis and suggestion for the fix by Tom Lane. Fixes bug #17828. Backpatch to v14 where async execution was introduced, but master gets a different fix. Discussion: https://www.postgresql.org/message-id/17828-122da8cba23236be@postgresql.org Discussion: https://www.postgresql.org/message-id/472235.1678387869@sss.pgh.pa.us	2 years ago