aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
* WAL-log the creation of the init fork of unlogged indexes.Heikki Linnakangas2023-07-06
| | | | | | | | | | | | | | | | | | | We create a file, so we better WAL-log it. In practice, all the built-in index AMs and all extensions that I'm aware of write a metapage to the init fork, which is WAL-logged, and replay of the metapage implicitly creates the fork too. But if ambuildempty() didn't write any page, we would miss it. This can be seen with dummy_index_am. Set up replication, create a 'dummy_index_am' index on an unlogged table, and look at the files created in the replica: the init fork is not created on the replica. Dummy_index_am doesn't do anything with the relation files, however, so it doesn't lead to any user-visible errors. Backpatch to all supported versions. Reviewed-by: Robert Haas Discussion: https://www.postgresql.org/message-id/6e5bbc08-cdfc-b2b3-9e23-1a914b9850a9%40iki.fi
* Revert the commits related to allowing page lock to conflict among parallel ↵Amit Kapila2023-07-06
| | | | | | | | | | | | | | | | | | | | group members. This commit reverts the work done by commits 3ba59ccc89 and 72e78d831a. Those commits were incorrect in asserting that we never acquire any other heavy-weight lock after acquring page lock other than relation extension lock. We can acquire a lock on catalogs while doing catalog look up after acquring page lock. This won't impact any existing feature but we need to think some other way to achieve this before parallelizing other write operations or even improving the parallelism in vacuum (like allowing multiple workers for an index). Reported-by: Jaime Casanova Author: Amit Kapila Backpatch-through: 13 Discussion: https://postgr.es/m/CAJKUy5jffnRKNvRHKQ0LynRb0RJC-o4P8Ku3x9vGAVLwDBWumQ@mail.gmail.com
* Fix leak of LLVM "fatal-on-oom" section counter.Heikki Linnakangas2023-07-05
| | | | | | | | | | | | | | | | llvm_release_context() called llvm_enter_fatal_on_oom(), but was missing the corresponding llvm_leave_fatal_on_oom() call. As a result, if JIT was used at all, we were almost always in the "fatal-on-oom" state. It only makes a difference if you use an extension written in C++, and run out of memory in a C++ 'new' call. In that case, you would get a PostgreSQL FATAL error, instead of the default behavior of throwing a C++ exception. Back-patch to all supported versions. Reviewed-by: Daniel Gustafsson Discussion: https://www.postgresql.org/message-id/54b78cca-bc84-dad8-4a7e-5b56f764fab5@iki.fi
* pgstat: fix subscription stats entry leak.Masahiko Sawada2023-07-05
| | | | | | | | | | | | | | | | | | Commit 7b64e4b3 taught DropSubscription() to drop stats entry of subscription that is not associated with a replication slot for apply worker at DROP SUBSCRIPTION but missed covering the case where the subscription is not associated with replication slots for both apply worker and tablesync worker. Also add a test to verify that the stats for slot-less subscription is removed at DROP SUBSCRIPTION time. Backpatch down to 15. Author: Masahiko Sawada Reviewed-by: Nathan Bossart, Hayato Kuroda, Melih Mutlu, Amit Kapila Discussion: https://postgr.es/m/CAD21AoB71zkP7uPT7JDPsZcvp0749ExEQnOJxeNKPDFisHar+w@mail.gmail.com Backpatch-through: 15
* Fix assertion failure in snapshot buildingDaniel Gustafsson2023-07-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Clear any potential stale next_phase_at value from the snapshot builder which otherwise may trip an assertion check ensuring that there is no next_phase_at value. This can be reproduced by running 80 concurrent sessions like the below where $c is a loop counter (assumes there has been 1..$c databases created) : echo " CREATE TABLE replication_example(id SERIAL PRIMARY KEY, somedata int, text varchar(120)); SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot_$c', 'test_decoding'); SELECT data FROM pg_logical_slot_get_changes('regression_slot_$c', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1'); " | psql -d regress_$c >>psql.log & Backpatch down to v16. Bug: #17695 Author: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Alexander Lakhin <exclusion@gmail.com> Reported-by: bowenshi <zxwsbg@qq.com> Discussion: https://postgr.es/m/17695-6be9277c9295985f@postgresql.org Backpatch-through: v16
* Ensure that creation of an empty relfile is fsync'd at checkpoint.Heikki Linnakangas2023-07-04
| | | | | | | | | | | | | | | | | If you create a table and don't insert any data into it, the relation file is never fsync'd. You don't lose data, because an empty table doesn't have any data to begin with, but if you crash and lose the file, subsequent operations on the table will fail with "could not open file" error. To fix, register an fsync request in mdcreate(), like we do for mdwrite(). Per discussion, we probably should also fsync the containing directory after creating a new file. But that's a separate and much wider issue. Backpatch to all supported versions. Reviewed-by: Andres Freund, Thomas Munro Discussion: https://www.postgresql.org/message-id/d47d8122-415e-425c-d0a2-e0160829702d%40iki.fi
* Re-bin segment when memory pages are freed.Thomas Munro2023-07-04
| | | | | | | | | | | | | | | | | | | | | It's OK to be lazy about re-binning memory segments when allocating, because that can only leave segments in a bin that's too high. We'll search higher bins if necessary while allocating next time, and also eventually re-bin, so no memory can become unreachable that way. However, when freeing memory, the largest contiguous range of free pages might go up, so we should re-bin eagerly to make sure we don't leave the segment in a bin that is too low for get_best_segment() to find. The re-binning code is moved into a function of its own, so it can be called whenever free pages are returned to the segment's free page map. Back-patch to all supported releases. Author: Dongming Liu <ldming101@gmail.com> Reviewed-by: Robert Haas <robertmhaas@gmail.com> (earlier version) Reviewed-by: Thomas Munro <thomas.munro@gmail.com> Discussion: https://postgr.es/m/CAL1p7e8LzB2LSeAXo2pXCW4%2BRya9s0sJ3G_ReKOU%3DAjSUWjHWQ%40mail.gmail.com
* Fix race in SSI interaction with gin fast path.Thomas Munro2023-07-04
| | | | | | | | | | | | | | | | | | The ginfast.c code previously checked for conflicts in before locking the relevant buffer, leaving a window where a RW conflict could be missed. Re-order. There was also a place where buffer ID and block number were confused while trying to predicate-lock a page, noted by visual inspection. Back-patch to all supported releases. Fixes one more problem discovered with the reproducer from bug #17949, in this case when Dmitry tried other index types. Reported-by: Artem Anisimov <artem.anisimov.255@gmail.com> Reported-by: Dmitry Dolgov <9erthalion6@gmail.com> Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://postgr.es/m/17949-a0f17035294a55e2%40postgresql.org
* Fix race in SSI interaction with bitmap heap scan.Thomas Munro2023-07-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | When performing a bitmap heap scan, we don't want to miss concurrent writes that occurred after we observed the heap's rs_nblocks, but before we took predicate locks on index pages. Therefore, we can't skip fetching any heap tuples that are referenced by the index, because we need to test them all with CheckForSerializableConflictOut(). The old optimization that would ignore any references to blocks >= rs_nblocks gets in the way of that requirement, because it means that concurrent writes in that window are ignored. Removing that optimization shouldn't affect correctness at any isolation level, because any new tuples shouldn't be visible to an MVCC snapshot. There also shouldn't be any error-causing references to heap blocks past the end, because we should have held at least an AccessShareLock on the table before the index scan. It can't get smaller while our transaction is running. For now, though, we'll keep the optimization at lower levels to avoid making unnecessary changes in a bug fix. Back-patch to all supported releases. In release 11, the code is in a different place but not fundamentally different. Fixes one aspect of bug #17949. Reported-by: Artem Anisimov <artem.anisimov.255@gmail.com> Reviewed-by: Dmitry Dolgov <9erthalion6@gmail.com> Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://postgr.es/m/17949-a0f17035294a55e2%40postgresql.org
* Fix race in SSI interaction with empty btrees.Thomas Munro2023-07-04
| | | | | | | | | | | | | | | | | When predicate-locking btrees, we have a special case for completely empty btrees, since there is no page to lock. This was racy, because, without buffer lock held, a matching key could be inserted between the _bt_search() and the PredicateLockRelation() calls. Fix, by rechecking _bt_search() after taking the relation-level SIREAD lock, if using SERIALIZABLE isolation and an empty btree is discovered. Back-patch to all supported releases. Fixes one aspect of bug #17949. Reported-by: Artem Anisimov <artem.anisimov.255@gmail.com> Reviewed-by: Dmitry Dolgov <9erthalion6@gmail.com> Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://postgr.es/m/17949-a0f17035294a55e2%40postgresql.org
* Silence "missing contrecord" error.Thomas Munro2023-07-03
| | | | | | | | | | | | | | | | | Commit dd38ff28ad added a new error message "missing contrecord" when we fail to reassemble a record. Unfortunately that caused noisy messages to be logged by pg_waldump at end of segment, and by walsender when asked to shut down on a segment boundary. Remove the new error message, so that this condition signals end-of- WAL without a message. It's arguably a reportable condition that should not be silenced while performing crash recovery, but fixing that without introducing noise in the other cases will require more research. Back-patch to 15. Reported-by: Tomas Vondra <tomas.vondra@enterprisedb.com> Discussion: https://postgr.es/m/6a1df56e-4656-b3ce-4b7a-a9cb41df8189%40enterprisedb.com
* Fix oversight in handling of modifiedCols since f24523672dTomas Vondra2023-07-02
| | | | | | | | | | | | | | | | | | | Commit f24523672d fixed a memory leak by moving the modifiedCols bitmap into the per-row memory context. In the case of AFTER UPDATE triggers, the bitmap is however referenced from an event kept until the end of the query, resulting in a use-after-free bug. Fixed by copying the bitmap into the AfterTriggerEvents memory context, which is the one where we keep the trigger events. There's only one place that needs to do the copy, but the memory context may not exist yet. Doing that in a separate function seems more readable. Report by Alexander Pyhalov, fix by me. Backpatch to 13, where the bitmap was added to the event by commit 71d60e2aa0. Reported-by: Alexander Pyhalov Backpatch-through: 13 Discussion: https://postgr.es/m/acddb17c89b0d6cb940eaeda18c08bbe@postgrespro.ru
* Fix memory leak in Incremental Sort rescansTomas Vondra2023-07-02
| | | | | | | | | | | | | | | | | | | | | | | | The Incremental Sort had a couple issues, resulting in leaking memory during rescans, possibly triggering OOM. The code had a couple of related flaws: 1. During rescans, the sort states were reset but then also set to NULL (despite the comment saying otherwise). ExecIncrementalSort then sees NULL and initializes a new sort state, leaking the memory used by the old one. 2. Initializing the sort state also automatically rebuilt the info about presorted keys, leaking the already initialized info. presorted_keys was also unnecessarily reset to NULL. Patch by James Coleman, based on patches by Laurenz Albe and Tom Lane. Backpatch to 13, where Incremental Sort was introduced. Author: James Coleman, Laurenz Albe, Tom Lane Reported-by: Laurenz Albe, Zu-Ming Jiang Backpatch-through: 13 Discussion: https://postgr.es/m/b2bd02dff61af15e3526293e2771f874cf2a3be7.camel%40cybertec.at Discussion: https://postgr.es/m/db03c582-086d-e7cd-d4a1-3bc722f81765%40inf.ethz.ch
* Fix marking of indisvalid for partitioned indexes at creationMichael Paquier2023-06-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The logic that introduced partitioned indexes missed a few things when invalidating a partitioned index when these are created, still the code is written to handle recursions: 1) If created from scratch because a mapping index could not be found, the new index created could be itself invalid, if for example it was a partitioned index with one of its leaves invalid. 2) A CCI was missing when indisvalid is set for a parent index, leading to inconsistent trees when recursing across more than one level for a partitioned index creation if an invalidation of the parent was required. This could lead to the creation of a partition index tree where some of the partitioned indexes are marked as invalid, but some of the parents are marked valid, which is not something that should happen (as validatePartitionedIndex() defines, indisvalid is switched to true for a partitioned index iff all its partitions are themselves valid). This patch makes sure that indisvalid is set to false on a partitioned index if at least one of its partition is invalid. The flag is set to true if *all* its partitions are valid. The regression test added in this commit abuses of a failed concurrent index creation, marked as invalid, that maps with an index created on its partitioned table afterwards. Reported-by: Alexander Lakhin Reviewed-by: Alexander Lakhin Discussion: https://postgr.es/m/14987634-43c0-0cb3-e075-94d423607e08@gmail.com Backpatch-through: 11
* Arm gen_node_support.pl's nodetag ABI stability check in v16.Tom Lane2023-06-29
| | | | Per RELEASE_CHANGES checklist.
* Fix pg_depend entry to AMs after ALTER TABLE .. SET ACCESS METHODMichael Paquier2023-06-30
| | | | | | | | | | | | | | | | | | | | | | ALTER TABLE .. SET ACCESS METHOD was not registering a dependency to the new access method with the relation altered in its rewrite phase, making possible the drop of an access method even if there are relations that depend on it. During the rewrite, a temporary relation is created to build the new relation files before swapping the new and old files, and, while the temporary relation was registering a correct dependency to the new AM, the old relation did not do that. A dependency on the access method is added when the relation files are swapped, which is the point where pg_class is updated. Materialized views and tables use the same code path, hence both were impacted. Backpatch down to 15, where this command has been introduced. Reported-by: Alexander Lakhin Reviewed-by: Nathan Bossart, Andres Freund Discussion: https://postgr.es/m/18000-9145c25b1af475ca@postgresql.org Backpatch-through: 15
* Defend against bogus parameterization of join input paths.Tom Lane2023-06-29
| | | | | | | | | | | | | | | | | | | An outer join cannot be formed using an input path that is parameterized by a value that is supposed to be nulled by the outer join. This is obviously nonsensical, and it could lead to a bad plan being selected; although currently it seems that we'll hit various sanity-check assertions first. I think that such cases were formerly prevented by the delay_upper_joins mechanism, but now that that's gone we need an explicit check. (Perhaps we should avoid generating baserel paths that could lead to this situation in the first place; but it seems like having a defense at the join level would be a good idea anyway.) Richard Guo and Tom Lane, per report from Jaime Casanova Discussion: https://postgr.es/m/CAJKUy5g2uZRrUDZJ8p-=giwcSHVUn0c9nmdxPSY0jF0Ov8VoEA@mail.gmail.com
* Fix order of operations in ExecEvalFieldStoreDeForm().Tom Lane2023-06-29
| | | | | | | | | | | | | | | | | | | | | | | | | If the given composite datum is toasted out-of-line, DatumGetHeapTupleHeader will perform database accesses to detoast it. That can invalidate the result of get_cached_rowtype, as documented (perhaps not plainly enough) in that function's API spec; which leads to strange errors or crashes when we try to use the TupleDesc to read the tuple. In short then, trying to update a field of a composite column could fail intermittently if the overall column value is wide enough to require toasting. We can fix the bug at no cost by just changing the order of operations, since we don't need the TupleDesc until after detoasting. (Other callers of get_cached_rowtype appear to get this right already, so there's only one bug.) Note that the added regression test case reveals this bug reliably only with debug_discard_caches/CLOBBER_CACHE_ALWAYS. Per bug #17994 from Alexander Lakhin. Sadly, this patch does not fix the missing-values issue revealed in the bug discussion; we'll need some more work to cover that. Discussion: https://postgr.es/m/17994-5c7100b51b4790e9@postgresql.org
* Remove inappropriate raw_expression_tree_walker() codePeter Eisentraut2023-06-29
| | | | | | | | It was walking into the ColumnDef->compression field, which is not a node but a string. This code is currently not reachable (because the compression field is only set in situations that don't go through raw_expression_tree_walker()), but if it had been, this could have behaved erratically.
* Error message wording improvementsPeter Eisentraut2023-06-29
|
* Reword error messages for consistencyPeter Eisentraut2023-06-28
|
* Ignore invalid indexes when enforcing index rules in ALTER TABLE ATTACH ↵Michael Paquier2023-06-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | PARTITION A portion of ALTER TABLE .. ATTACH PARTITION is to ensure that the partition being attached to the partitioned table has a correct set of indexes, so as there is a consistent index mapping between the partitioned table and its new-to-be partition. However, as introduced in 8b08f7d, the current logic could choose an invalid index as a match, which is something that can exist when dealing with more than two levels of partitioning, like attaching a partitioned table (that has partitions, with an index created by CREATE INDEX ON ONLY) to another partitioned table. A partitioned index with indisvalid set to false is equivalent to an incomplete partition tree, meaning that an invalid partitioned index does not have indexes defined in all its partitions. Hence, choosing an invalid partitioned index can create inconsistent partition index trees, where the parent attaching to is valid, but its partition may be invalid. In the report from Alexander Lakhin, this showed up as an assertion failure when validating an index. Without assertions enabled, the partition index tree would be actually broken, as indisvalid should be switched to true for a partitioned index once all its partitions are themselves valid. With two levels of partitioning, the top partitioned table used a valid index and was able to link to an invalid index stored on its partition, itself a partitioned table. I have studied a few options here (like the possibility to switch indisvalid to false for the parent), but came down to the conclusion that we'd better rely on a simple rule: invalid indexes had better never be chosen, so as the partition attached uses and creates indexes that the parent expects. Some regression tests are added to provide some coverage. Note that the existing coverage is not impacted. This is a problem since partitioned indexes exist, so backpatch all the way down to v11. Reported-by: Alexander Lakhin Discussion: https://postgr.es/14987634-43c0-0cb3-e075-94d423607e08@gmail.com Backpatch-through: 11
* Remove dependency to query text in JumbleQuery()Michael Paquier2023-06-28
| | | | | | | | | | | Since 3db72eb, the query ID of utilities is generated using the Query structure, making the use of the query string in JumbleQuery() unnecessary. This commit removes the argument "querytext" from JumbleQuery(). Reported-by: Joe Conway Reviewed-by: Nathan Bossart Discussion: https://postgr.es/m/ZJlQAWE4COFqHuAV@paquier.xyz
* Translation updatesPeter Eisentraut2023-06-26
| | | | | Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: ab77975e9d2cde44da796c18af3ec1a66f0df7ae
* Change "..." to cstring in old input/output function comments.Heikki Linnakangas2023-06-26
| | | | | | | It was not clear what the "..." meant. Author: Steve Chavez Discussion: https://www.postgresql.org/message-id/CAGRrpzZzeh7zC3yaVG9di%3DydJ%2BiHwdXnFPB3evGFKvC1zf6ajA@mail.gmail.com
* Check for interrupts and stack overflow in TParserGet().Tom Lane2023-06-24
| | | | | | | | | | | | | | | TParserGet() recurses for some token types, meaning it's possible to drive it to stack overflow. Since this is a minority behavior, I chose to add the check_stack_depth() call to the two places that recurse rather than doing it during every single call. While at it, add CHECK_FOR_INTERRUPTS(), because this can run unpleasantly long for long inputs. Per bug #17995 from Zuming Jiang. This is old, so back-patch to all supported branches. Discussion: https://postgr.es/m/17995-9f20ff3e6389db4c@postgresql.org
* Error message refactoringPeter Eisentraut2023-06-23
| | | | | | Take some untranslatable things out of the message and replace by format placeholders, to reduce translatable strings and reduce translation mistakes.
* Fix cache lookup hazards introduced by ff9618e82a.Nathan Bossart2023-06-22
| | | | | | | | | | | | | | | | | | | | | | | | | ff9618e82a introduced has_partition_ancestor_privs(), which is used to check whether a user has MAINTAIN on any partition ancestors. This involves syscache lookups, and presently this function does not take any relation locks, so it is likely subject to the same kind of cache lookup failures that were fixed by 19de0ab23c. To fix this problem, this commit partially reverts ff9618e82a. Specifically, it removes the partition-related changes, including the has_partition_ancestor_privs() function mentioned above. This means that MAINTAIN on a partitioned table is no longer sufficient to perform maintenance commands on its partitions. This is more like how privileges for maintenance commands work on supported versions. Privileges are checked for each partition, so a command that flows down to all partitions might refuse to process them (e.g., if the current user doesn't have MAINTAIN on the partition). In passing, adjust a few related comments and error messages, and add a test for the privilege checks for CLUSTER on a partitioned table. Reviewed-by: Michael Paquier, Jeff Davis Discussion: https://postgr.es/m/20230613211246.GA219055%40nathanxps13
* nbtree VACUUM: cope with topparent inconsistencies.Peter Geoghegan2023-06-21
| | | | | | | | | | | | | | | | | | Avoid "right sibling %u of block %u is not next child" errors when vacuuming a corrupt nbtree index. Just LOG the issue and press on. That way VACUUM will have a decent chance of finishing off all required processing for the index (and for the table as a whole). This is similar to recent work from commit 5abff197, as well as work from commit 5b861baa (later backpatched as commit 43e409ce), which taught nbtree VACUUM to keep going when its "re-find" check fails. The hardening added by this commit takes place directly after the "re-find" check, right before the critical section for the first stage of page deletion. Author: Peter Geoghegan <pg@bowt.ie> Discussion: https://postgr.es/m/CAH2-Wz=dayg0vjs4+er84TS9ami=csdzjpuiCGbEw=idhwqhzQ@mail.gmail.com Backpatch: 11- (all supported versions).
* ICU: do not convert locale 'C' to 'en-US-u-va-posix'.Jeff Davis2023-06-21
| | | | | | | | | | | | | | | | | | | | | Older versions of ICU canonicalize "C" to "en-US-u-va-posix"; but starting in ICU version 64, the "C" locale is considered obsolete. Postgres commit ea1db8ae70 introduced code to always canonicalize "C" to "en-US-u-va-posix" for consistency and convenience, but it was deemed too confusing. This commit removes that code, so that "C" is treated like other ICU locale names: canonicalization is attempted, and if it fails, the behavior is controlled by icu_validation_level. A similar change was previously committed as f7faa9976c, then reverted due to an ICU-version-dependent test failure. This commit un-reverts it, omitting the test because we now expect the behavior to depend on the version of ICU being used. Discussion: https://postgr.es/m/3a200aca-4672-4b37-fc91-5d198a323503%40eisentraut.org Discussion: https://postgr.es/m/f83f089ee1e9acd5dbbbf3353294d24e1f196e95.camel@j-davis.com Discussion: https://postgr.es/m/37520ec1ae9591f83132f82dbd625f3fc2d69c16.camel@j-davis.com
* Avoid Assert failure when processing empty statement in aborted xact.Tom Lane2023-06-21
| | | | | | | | | | | | | | | | | | | | | exec_parse_message() wants to create a cached plan in all cases, including for empty input. The empty-input path does not have a test for being in an aborted transaction, making it possible that plancache.c will fail due to trying to do database lookups even though there's no real work to do. One solution would be to throw an aborted-transaction error in this path too, but it's not entirely clear whether the lack of such an error was intentional or whether some clients might be relying on non-error behavior. Instead, let's hack plancache.c so that it treats empty statements with the same logic it already had for transaction control commands, ensuring that it can soldier through even in an already-aborted transaction. Per bug #17983 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/17983-da4569fcb878672e@postgresql.org
* Fix the errhint message and docs for drop subscription failure.Amit Kapila2023-06-21
| | | | | | | | | | The existing errhint message and docs were missing the fact that we can't disassociate from the slot unless the subscription is disabled. Author: Robert Sjöblom, Peter Smith Reviewed-by: Peter Eisentraut, Amit Kapila Backpatch-through: 11 Discussion: https://postgr.es/m/807bdf85-61ea-88e2-5712-6d9fcd4eabff@fortnox.se
* Move bool parameter for vacuum_rel() to option bits.Nathan Bossart2023-06-20
| | | | | | | | | | | ff9618e82a introduced the skip_privs parameter, which is used to skip privilege checks when recursing to a relation's TOAST table. This parameter should have been added as a flag bit in VacuumParams->options instead. Suggested-by: Michael Paquier Reviewed-by: Michael Paquier, Jeff Davis Discussion: https://postgr.es/m/ZIj4v1CwqlDVJZfB%40paquier.xyz
* Fix hash join when inner hashkey expressions contain Params.Tom Lane2023-06-20
| | | | | | | | | | | | | | | | | | | | | | | | If the inner-side expressions contain PARAM_EXEC Params, we must re-hash whenever the values of those Params change. The executor mechanism for that exists already, but we failed to invoke it because finalize_plan() neglected to search the Hash.hashkeys field for Params. This allowed a previous scan's hash table to be re-used when it should not be, leading to rows missing from the join's output. (I believe incorrectly-included join rows are impossible however, since checking the real hashclauses would reject false matches.) This bug is very ancient, dating probably to d24d75ff1 of 7.4. Sadly, this simple fix depends on the plan representational changes made by 2abd7ae9b, so it will only work back to v12. I thought about trying to make some kind of hack for v11, but I'm leery of putting code significantly different from what is used in the newer branches into a nearly-EOL branch. Seeing that the bug escaped detection for a full twenty years, problematic cases must be rare; so I don't feel too awful about leaving v11 as-is. Per bug #17985 from Zuming Jiang. Back-patch to v12. Discussion: https://postgr.es/m/17985-748b66607acd432e@postgresql.org
* Fix another cause of "wrong varnullingrels" planner failures.Tom Lane2023-06-20
| | | | | | | | | | | | | | | | | | | I removed the delay_upper_joins mechanism in commit b448f1c8d, reasoning that it was only needed when we have a single-table (SELECT ... WHERE) as the immediate RHS child of a left join, and we could get rid of that by hoisting the WHERE condition into the parent join's quals. However that new code missed a case: we could have "foo LEFT JOIN ((SELECT ... WHERE) LEFT JOIN bar)", and if the two left joins can be commuted then we now have the problematic query shape. We can fix this too easily enough, by allowing the syntactically-lower left join to pass through its parent qual location pointer recursively. That lets prepjointree.c discard the SELECT by temporarily hoisting the WHERE condition into the ancestor join's qual. Per bug #17978 from Zuming Jiang. Discussion: https://postgr.es/m/17978-12f3d93a55297266@postgresql.org
* Don't include outer join relids in lateral_relids bitmapsets.Tom Lane2023-06-20
| | | | | | | | | | | | | | This avoids an assertion failure when outer joins are rearranged per identity 3. Listing only the baserels from a PlaceHolderVar's ph_lateral set should be enough to ensure that the required values are available when we need to compute the PHV --- it's what we did before inventing nullingrel sets, after all. It's a bit unsatisfying; but with beta2 hard upon us, there's not time to look for an aesthetically cleaner fix. Richard Guo and Tom Lane Discussion: https://postgr.es/m/CAMbWs48Jcw-NvnxT23WiHP324wG44DvzcH1j4hc0Zn+3sR9cfg@mail.gmail.com
* Centralize fixups for mismatched nullingrels in nestloop params.Tom Lane2023-06-20
| | | | | | | | | | | | | | | | | | | | It turns out that the fixes we applied in commits bfd332b3f and 63e4f13d2 were not nearly enough to solve the problem. We'd focused narrowly on subquery RTEs with lateral references, but lateral references can occur in several other RTE kinds such as function RTEs. Putting the same hack into half a dozen code paths seems quite unattractive. Hence, revert the code changes (but not the test cases) from those commits and instead solve it centrally in identify_current_nestloop_params(), as Richard proposed originally. This is a bit annoying because it could mask erroneous nullingrels in nestloop params that are generated from non-LATERAL parameterized paths; but on balance I don't see a better way. Maybe at some future time we'll be motivated to find a more rigorous approach to nestloop params, but that's not happening for beta2. Richard Guo and Tom Lane Discussion: https://postgr.es/m/CAMbWs48Jcw-NvnxT23WiHP324wG44DvzcH1j4hc0Zn+3sR9cfg@mail.gmail.com
* Pre-beta2 mechanical code beautification.Tom Lane2023-06-20
| | | | | | | | | Run pgindent and pgperltidy. It seems we're still some ways away from all committers doing this automatically. Now that we have a buildfarm animal that will whine about poorly-indented code, we'll try to keep the tree more tidy. Discussion: https://postgr.es/m/3156045.1687208823@sss.pgh.pa.us
* Enable archiving in recovery TAP test 009_twophase.plMichael Paquier2023-06-20
| | | | | | | | | | | | | | | | | This is a follow-up of f663b00, that has been committed to v13 and v14, tweaking the TAP test for two-phase transactions so as it provides coverage for the bug that has been fixed. This change is done in its own commit for clarity, as v15 and HEAD did not show the problematic behavior, still missed coverage for it. While on it, this adds a comment about the dependency of the last partial segment rename and RecoverPreparedTransactions() at the end of recovery, as that can be easy to miss. Author: Michael Paquier Reviewed-by: Kyotaro Horiguchi Discussion: https://postgr.es/m/743b9b45a2d4013bd90b6a5cba8d6faeb717ee34.camel@cybertec.at Backpatch-through: 13
* fd.c: Retry after EINTR in more placesAndres Freund2023-06-19
| | | | | | | | | | | | | | | | | | | | | | | Starting with 4d330a61bb1 we can use posix_fallocate() to extend files. Unfortunately in some situation, e.g. on tmpfs filesystems, EINTR may be returned. See also 4518c798b2b. To fix, add a retry path to FileFallocate(). In contrast to 4518c798b2b the amount we extend by is limited and the extending may happen at a high frequency, so disabling signals does not appear to be the correct path here. Also add retry paths to other file operations currently lacking them (around fdatasync(), fsync(), ftruncate(), posix_fadvise(), sync_file_range(), truncate()) - they are all documented or have been observed to return EINTR. Even though most of these functions used in the back branches, it does not seem worth the risk to backpatch - outside of the new-to-16 case of posix_fallocate() I am not aware of problem reports due to the lack of retries. Reported-by: Christoph Berg <myon@debian.org> Discussion: https://postgr.es/m/ZEZDj1H61ryrmY9o@msg.df7cb.de Backpatch: -
* Don't use partial unique indexes for unique proofs in the plannerDavid Rowley2023-06-19
| | | | | | | | | | | | | | | | | | | Here we adjust relation_has_unique_index_for() so that it no longer makes use of partial unique indexes as uniqueness proofs. It is incorrect to use these as the predicates used by check_index_predicates() to set predOK makes use of not only baserestrictinfo quals as proofs, but also qual from join conditions. For relation_has_unique_index_for()'s case, we need to know the relation is unique for a given set of columns before any joins are evaluated, so if predOK was only set to true due to some join qual, then it's unsafe to use such indexes in relation_has_unique_index_for(). The final plan may not even make use of that index, which could result in reading tuples that are not as unique as the planner previously expected them to be. Bug: #17975 Reported-by: Tor Erik Linnerud Backpatch-through: 11, all supported versions Discussion: https://postgr.es/m/17975-98a90c156f25c952%40postgresql.org
* CREATE DATABASE: make LOCALE apply to all collation providers.Jeff Davis2023-06-16
| | | | | | | | | | | For CREATE DATABASE, make LOCALE parameter apply regardless of the provider used. Also affects initdb and createdb --locale arguments. Previously, LOCALE (and --locale) only affected the database default collation when using the libc provider. Discussion: https://postgr.es/m/1a63084d-221e-4075-619e-6b3e590f673e@enterprisedb.com Reviewed-by: Peter Eisentraut
* Fix typo in comment.Amit Langote2023-06-16
| | | | | | | Back-patch down to 11. Author: Sho Kato (<kato-sho@fujitsu.com>) Discussion: https://postgr.es/m/TYCPR01MB68499042A33BC32241193AAF9F5BA%40TYCPR01MB6849.jpnprd01.prod.outlook.com
* When removing a left join, clean out references in EquivalenceClasses.Tom Lane2023-06-15
| | | | | | | | | | | | | | | | | | | | Since commit b448f1c8d, we've been able to remove left joins (that are otherwise removable) even when they are underneath other left joins, a case that was previously prevented by a delay_upper_joins check. This is a clear improvement, but it has a surprising side-effect: it's now possible that there are EquivalenceClasses whose relid sets mention the removed baserel and/or outer join. If we fail to clean those up, we may drop essential join quals due to not having any join level that appears to satisfy their relid sets. (It's not quite 100% clear that this was impossible before. But the lack of complaints since we added join removal a dozen years ago strongly suggests that it was impossible.) Richard Guo and Tom Lane, per bug #17976 from Zuming Jiang Discussion: https://postgr.es/m/17976-4b638b525e9a983b@postgresql.org
* Remove outdated reference to a removed fileAmit Langote2023-06-15
| | | | | | | parse_jsontable.c was removed as part of 2f2b18bd3f55, though its mention in src/backend/parser/README was not. Fix that. Discussion: https://postgr.es/m/CA%2BHiwqHDzw8AP8p_dEkFr0xg458ZTf58zbivAHhK4UeNrx9Tdg%40mail.gmail.com
* Replace GUC_UNIT_MEMORY|GUC_UNIT_TIME with GUC_UNIT.Masahiko Sawada2023-06-15
| | | | | | | | | | We used (GUC_UNIT_MEMORY | GUC_UNIT_TIME) instead of GUC_UNIT some places but we already define it in guc.h. This commit replaces them with GUC_UNIT for better consistency with their surrounding code. Author: Japin Li Reviewed-by: Richard Guo, Michael Paquier, Masahiko Sawada Discussion: https://postgr.es/m/MEYP282MB1669EC0FED922F7A151673ACB65AA@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
* Fix possible crash in tablesync worker.Amit Kapila2023-06-15
| | | | | | | | | | | | Commit c3afe8cf5a added a new password_required option but forgot that you need database access to check whether an arbitrary role ID is a superuser. Commit e7e7da2f8d fixed a similar bug in apply worker, and this patch fixes a similar bug in tablesync worker. Author: Hou Zhijie Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/OS0PR01MB571607F5A9D723755268D36294759@OS0PR01MB5716.jpnprd01.prod.outlook.com
* Make parseNodeString() C idiom compatible with Visual Studio 2015.Noah Misch2023-06-14
| | | | | | | | | | | | Between v15 and now, this function's "else if" chain grew from 252 lines to 592 lines, exceeding a compiler limit that manifests as "fatal error C1026: parser stack overflow, program too complex (compiling source file src/backend/nodes/readfuncs.c)". Use "if (...) return ...;" instead. Reviewed by Tom Lane, Peter Eisentraut and Michael Paquier. Not all reviewers endorse this. Discussion: https://postgr.es/m/20230607185458.GA1334487@rfd.leadboat.com
* Fix typo in comment.Masahiko Sawada2023-06-14
| | | | | | | | Introduced in 4d330a61bb1. Author: Masahiko Sawada Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/CAD21AoDg8rTWJkrNJg9UTP89vS8smfib2c55DVqKrCn8zR-GYA@mail.gmail.com
* Retain relkind too in RTE_SUBQUERY entries for views.Amit Langote2023-06-14
| | | | | | | | | | | | | | | | | | 47bb9db75 modified the ApplyRetrieveRule()'s conversion of a view's original RTE_RELATION entry into an RTE_SUBQUERY one to retain relid, rellockmode, and perminfoindex so that the executor can lock the view and check its permissions. It seems better to also retain relkind for cross-checking that the exception of an RTE_SUBQUERY entry being allowed to carry relation details only applies to views, so do so. Bump catversion because this changes the output format of RTE_SUBQUERY RTEs. Suggested-by: David Steele <david@pgmasters.net> Reviewed-by: David Steele <david@pgmasters.net> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/3953179e-9540-e5d1-a743-4bef368785b0%40pgmasters.net
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-02 15:44:02 +0000