aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
...
* Add comments and a missing CHECK_FOR_INTERRUPTS in ts_headline.Tom Lane2022-11-21
| | | | | | | | | | | | | | | | I just spent an annoying amount of time reverse-engineering the 100%-undocumented API between ts_headline and the text search parser's prsheadline function. Add some commentary about that while it's fresh in mind. Also remove some unused macros in wparser_def.c. While at it, I noticed that when commit 78e73e875 added a CHECK_FOR_INTERRUPTS call in TS_execute_recurse, it missed doing so in the parallel function TS_phrase_execute, which surely needs one just as much. Back-patch because of the missing CHECK_FOR_INTERRUPTS. Might as well back-patch the rest of this too.
* Fix mislabeling of PROC_QUEUE->links as PGPROC, fixing UBSan on 32bitAndres Freund2022-11-19
| | | | | | | | | | | | | | | | | | | | | | | ProcSleep() used a PGPROC* variable to point to PROC_QUEUE->links.next, because that does "the right thing" with SHMQueueInsertBefore(). While that largely works, it's certainly not correct and unnecessary - we can just use SHM_QUEUE* to point to the insertion point. Noticed when testing a 32bit of postgres with undefined behavior sanitizer. UBSan noticed that sometimes the supposed PGPROC wasn't sufficiently aligned (required since 46d6e5f5679, ensured indirectly, via ShmemAllocRaw() guaranteeing cacheline alignment). For now fix this by using a SHM_QUEUE* for the insertion point. Subsequently we should replace all the use of PROC_QUEUE and SHM_QUEUE with ilist.h, but that's a larger change that we don't want to backpatch. Backpatch to all supported versions - it's useful to be able to run postgres under UBSan. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/20221117014230.op5kmgypdv2dtqsf@awork3.anarazel.de Backpatch: 11-
* Replace RelationOpenSmgr() with RelationGetSmgr().Tom Lane2022-11-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is a back-patch of the v15-era commit f10f0ae42 into older supported branches. The idea is to design out bugs in which an ill-timed relcache flush clears rel->rd_smgr partway through some code sequence that wasn't expecting that. We had another report today of a corner case that reliably crashes v14 under debug_discard_caches (nee CLOBBER_CACHE_ALWAYS), and therefore would crash once in a blue moon in the field. We're unlikely to get rid of all such code paths unless we adopt the more rigorous coding rules instituted by f10f0ae42. Therefore, even though this is a bit invasive, it's time to back-patch. Some comfort can be taken in the fact that f10f0ae42 has been in v15 for 16 months without problems. I left the RelationOpenSmgr macro present in the back branches, even though no core code should use it anymore, in order to not break third-party extensions in minor releases. Such extensions might opt to start using RelationGetSmgr instead, to reduce their code differential between v15 and earlier branches. This carries a hazard of failing to compile against headers from existing minor releases. However, once compiled the extension should work fine even with such releases, because RelationGetSmgr is a "static inline" function so it creates no link-time dependency. So depending on distribution practices, that might be an OK tradeoff. Per report from Spyridon Dimitrios Agathos. Original patch by Amul Sul. Discussion: https://postgr.es/m/CAFM5RaqdgyusQvmWkyPYaWMwoK5gigdtW-7HcgHgOeAw7mqJ_Q@mail.gmail.com Discussion: https://postgr.es/m/CANiYTQsU7yMFpQYnv=BrcRVqK_3U3mtAzAsJCaqtzsDHfsUbdQ@mail.gmail.com
* Fix cleanup lock acquisition in SPLIT_ALLOCATE_PAGE replay.Amit Kapila2022-11-14
| | | | | | | | | | | | | | | | During XLOG_HASH_SPLIT_ALLOCATE_PAGE replay, we were checking for a cleanup lock on the new bucket page after acquiring an exclusive lock on it and raising a PANIC error on failure. However, it is quite possible that checkpointer can acquire the pin on the same page before acquiring a lock on it, and then the replay will lead to an error. So instead, directly acquire the cleanup lock on the new bucket page during XLOG_HASH_SPLIT_ALLOCATE_PAGE replay operation. Reported-by: Andres Freund Author: Robert Haas Reviewed-By: Amit Kapila, Andres Freund, Vignesh C Backpatch-through: 11 Discussion: https://postgr.es/m/20220810022617.fvjkjiauaykwrbse@awork3.anarazel.de
* Fix theoretical torn page hazard.Jeff Davis2022-11-11
| | | | | | | | | | | | | | | | | | | | | | | | The original report was concerned with a possible inconsistency between the heap and the visibility map, which I was unable to confirm. The concern has been retracted. However, there did seem to be a torn page hazard when using checksums. By not setting the heap page LSN during redo, the protections of minRecoveryPoint were bypassed. Fixed, along with a misleading comment. It may have been impossible to hit this problem in practice, because it would require a page tear between the checksum and the flags, so I am marking this as a theoretical risk. But, as discussed, it did violate expectations about the page LSN, so it may have other consequences. Backpatch to all supported versions. Reported-by: Konstantin Knizhnik Reviewed-by: Konstantin Knizhnik Discussion: https://postgr.es/m/fed17dac-8cb8-4f5b-d462-1bb4908c029e@garret.ru Backpatch-through: 11
* Doc: add comments about PreventInTransactionBlock/IsInTransactionBlock.Tom Lane2022-11-09
| | | | | | | | | | | | | Add a little to the header comments for these functions to make it clearer what guarantees about commit behavior are provided to callers. (See commit f92944137 for context.) Although this is only a comment change, it's really documentation aimed at authors of extensions, so it seems appropriate to back-patch. Yugo Nagata and Tom Lane, per further discussion of bug #17434. Discussion: https://postgr.es/m/17434-d9f7a064ce2a88a3@postgresql.org
* Translation updatesPeter Eisentraut2022-11-07
| | | | | Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 93ab2efcf9d6fb34739c57e61d57ae37e1fb03d3
* Correct error message for row-level triggers with transition tables on ↵Etsuro Fujita2022-11-04
| | | | | | | | | | | | | | | partitioned tables. "Triggers on partitioned tables cannot have transition tables." is incorrect as we allow statement-level triggers on partitioned tables to have transition tables. This has been wrong since commit 86f575948; back-patch to v11 where that commit came in. Reviewed by Tom Lane. Discussion: https://postgr.es/m/CAPmGK17gk4vXLzz2iG%2BG4LWRWCoVyam70nZ3OuGm1hMJwDrhcg%40mail.gmail.com
* Create FKs properly when attaching table as partitionAlvaro Herrera2022-11-03
| | | | | | | | | | | | | | | | | | | | | | Commit f56f8f8da6af added some code in CloneFkReferencing that's way too lax about a Constraint node it manufactures, not initializing enough struct members -- initially_valid in particular was forgotten. This causes some FKs in partitions added by ALTER TABLE ATTACH PARTITION to be marked as not validated. Set initially_valid true, which fixes the bug. While at it, make the struct initialization more complete. Very similar code was added in two other places by the same commit; make them all follow the same pattern for consistency, though no bugs are apparent there. This bug has never been reported: I only happened to notice while working on commit 614a406b4ff1. The test case that was added there with the improper result is repaired. Backpatch to 12. Discussion: https://postgr.es/m/20221005105523.bhuhkdx4olajboof@alvherre.pgsql
* Avoid crash after function syntax error in a replication worker.Tom Lane2022-11-03
| | | | | | | | | | | | | | | | | | | | | | | If a syntax error occurred in a SQL-language or PL/pgSQL-language CREATE FUNCTION or DO command executed in a logical replication worker, we'd suffer a null pointer dereference or assertion failure. That seems like a rather contrived case, but nonetheless worth fixing. The cause is that function_parse_error_transpose assumes it must be executing within the context of a Portal, but logical/worker.c doesn't create a Portal since it's not running the standard executor. We can just back off the hard Assert check and make it fail gracefully if there's not an ActivePortal. (I have a feeling that the aggressive check here was my fault originally, probably because I wasn't sure if the case would always hold and wanted to find out. Well, now we know.) The hazard seems to exist in all branches that have logical replication, so back-patch to v10. Maxim Orlov, Anton Melnikov, Masahiko Sawada, Tom Lane Discussion: https://postgr.es/m/b570c367-ba38-95f3-f62d-5f59b9808226@inbox.ru Discussion: https://postgr.es/m/adf0452f-8c6b-7def-d35e-ab516c80088e@inbox.ru
* Defend against unsupported partition relkind in logical replication worker.Tom Lane2022-11-02
| | | | | | | | | | | | | | | | | | | | Since partitions can be foreign tables not only plain tables, but logical replication only supports plain tables, we'd better check the relkind of a partition after we find it. (There was some discussion of checking this when adding a partitioned table to a subscription; but that would be inadequate since the troublesome partition could be added later.) Without this, the situation leads to a segfault or assertion failure. In passing, add a separate variable for the target Relation of a cross-partition UPDATE; reusing partrel seemed mighty confusing and error-prone. Shi Yu and Tom Lane, per report from Ilya Gladyshev. Back-patch to v13 where logical replication into partitioned tables became a thing. Discussion: https://postgr.es/m/6b93e3748ba43298694f376ca8797279d7945e29.camel@gmail.com
* Fix copy-and-pasteo in comment.Etsuro Fujita2022-11-02
|
* Fix ordering issue with WAL operations in GIN fast insert pathMichael Paquier2022-10-26
| | | | | | | | | | | | | | | | | | | | | | | | Contrary to what is documented in src/backend/access/transam/README, ginHeapTupleFastInsert() had a few ordering issues with the way it does its WAL operations when inserting items in its fast path. First, when using a separate list, XLogBeginInsert() was being always called before START_CRIT_SECTION(), and in this case a second thing was wrong when merging lists, as an exclusive lock was taken on the tail page *before* calling XLogBeginInsert(). Finally, when inserting items into a tail page, the order of XLogBeginInsert() and START_CRIT_SECTION() was reversed. This commit addresses all these issues by moving the calls of XLogBeginInsert() after all the pages logged are locked and pinned, within a critical section. This has been applied first only on HEAD as of 56b6625, but as per discussion with Tom Lane and Álvaro Herrera, a backpatch is preferred to keep all the branches consistent and to respect the transam's README where we can. Author: Matthias van de Meent, Zhang Mingli Discussion: https://postgr.es/m/CAEze2WhL8uLMqynnnCu1LAPwxD5RKEo0nHV+eXGg_N6ELU88HQ@mail.gmail.com Backpatch-through: 10
* Add CHECK_FOR_INTERRUPTS while restoring changes during decoding.Amit Kapila2022-10-21
| | | | | | | | | | | Previously in commit 42681dffaf, we added CFI during decoding changes but missed another similar case that can happen while restoring changes spilled to disk back into memory in a loop. Reported-by: Robert Haas Author: Amit Kapila Backpatch-through: 10 Discussion: https://postgr.es/m/CA+TgmoaLObg0QbstbC8ykDwOdD1bDkr4AbPpB=0DPgA2JW0mFg@mail.gmail.com
* Fix executing invalidation messages generated by subtransactions during ↵Amit Kapila2022-10-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | decoding. This problem has been introduced by commit 272248a0c1 where we started assigning the subtransactions to the top-level transaction when we mark both the top-level transaction and its subtransactions as containing catalog changes. After we assign subtransactions to the top-level transaction, we were not allowed to execute any invalidations associated with it when we decide to skip the transaction. The reason to assign the subtransactions to the top-level transaction was to avoid the assertion failure in AssertTXNLsnOrder() as they have the same LSN when we sometimes start accumulating transaction changes for partial transactions after the restart. Now that with commit 64ff0fe4e8, we skip this assertion check until we reach the LSN at which we start decoding the contents of the transaction, so, there is no reason for such an assignment anymore. The assignment change was introduced in 15 and prior versions but this bug doesn't exist in branches prior to 14 since we don't add invalidation messages to subtransactions. We decided to backpatch through 11 for consistency but not for 10 since its final release is near. Reported-by: Kuroda Hayato Author: Masahiko Sawada Reviewed-by: Amit Kapila Backpatch-through: 11 Discussion: https://postgr.es/m/TYAPR01MB58660803BCAA7849C8584AA4F57E9%40TYAPR01MB5866.jpnprd01.prod.outlook.com Discussion: https://postgr.es/m/a89b46b6-0239-2fd5-71a9-b19b1f7a7145%40enterprisedb.com
* Fix assertion failures while processing NEW_CID record in logical decoding.Amit Kapila2022-10-20
| | | | | | | | | | | | | | | | | | | | | | | | | | | When the logical decoding restarts from NEW_CID, since there is no association between the top transaction and its subtransaction, both are created as top transactions and have the same LSN. This caused the assertion failure in AssertTXNLsnOrder(). This patch skips the assertion check until we reach the LSN at which we start decoding the contents of the transaction, specifically start_decoding_at LSN in SnapBuild. This is okay because we don't guarantee to make the association between top transaction and subtransaction until we try to decode the actual contents of transaction. The ordering of the records prior to the start_decoding_at LSN should have been checked before the restart. The other assertion failure is due to the reason that we forgot to track that we have considered top-level transaction id in the list of catalog changing transactions that were committed when one of its subtransactions is marked as containing catalog change. Reported-by: Tomas Vondra, Osumi Takamichi Author: Masahiko Sawada, Kuroda Hayato Reviewed-by: Amit Kapila, Dilip Kumar, Kuroda Hayato, Kyotaro Horiguchi, Masahiko Sawada Backpatch-through: 10 Discussion: https://postgr.es/m/a89b46b6-0239-2fd5-71a9-b19b1f7a7145%40enterprisedb.com Discussion: https://postgr.es/m/TYCPR01MB83733C6CEAE47D0280814D5AED7A9%40TYCPR01MB8373.jpnprd01.prod.outlook.com
* Track LLVM 15 changes.Thomas Munro2022-10-19
| | | | | | | | | | | | Per https://llvm.org/docs/OpaquePointers.html, support for non-opaque pointers still exists and we can request that on our context. We have until LLVM 16 to move to opaque pointers, a much larger change. Back-patch to 11, where LLVM support arrived. Author: Thomas Munro <thomas.munro@gmail.com> Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/CAMHz58Sf_xncdyqsekoVsNeKcruKootLtVH6cYXVhhUR1oKPCg%40mail.gmail.com
* Reject non-ON-SELECT rules that are named "_RETURN".Tom Lane2022-10-17
| | | | | | | | | | | | DefineQueryRewrite() has long required that ON SELECT rules be named "_RETURN". But we overlooked the converse case: we should forbid non-ON-SELECT rules that are named "_RETURN". In particular this prevents using CREATE OR REPLACE RULE to overwrite a view's _RETURN rule with some other kind of rule, thereby breaking the view. Per bug #17646 from Kui Liu. Back-patch to all supported branches. Discussion: https://postgr.es/m/17646-70c93cfa40365776@postgresql.org
* Guard against table-AM-less relations in planner.Tom Lane2022-10-17
| | | | | | | | | | | | | | | The executor will dump core if it's asked to execute a seqscan on a relation having no table AM, such as a view. While that shouldn't really happen, it's possible to get there via catalog corruption, such as a missing ON SELECT rule. It seems worth installing a defense against that. There are multiple plausible places for such a defense, but I picked the planner's get_relation_info(). Per discussion of bug #17646 from Kui Liu. Back-patch to v12 where the tableam APIs were introduced; in older versions you won't get a SIGSEGV, so it seems less pressing. Discussion: https://postgr.es/m/17646-70c93cfa40365776@postgresql.org
* Rename parser token REF to REF_P to avoid a symbol conflict.Tom Lane2022-10-16
| | | | | | | | | | | | | | | | | | | | | | | In the latest version of Apple's macOS SDK, <sys/socket.h> fails to compile if "REF" is #define'd as something. Apple may or may not agree that this is a bug, and even if they do accept the bug report I filed, they probably won't fix it very quickly. In the meantime, our back branches will all fail to compile gram.y. v15 and HEAD currently escape the problem thanks to the refactoring done in 98e93a1fc, but that's purely accidental. Moreover, since that patch removed a widely-visible inclusion of <netdb.h>, back-patching it seems too likely to break third-party code. Instead, change the token's code name to REF_P, following our usual convention for naming parser tokens that are likely to have symbol conflicts. The effects of that should be localized to the grammar and immediately surrounding files, so it seems like a safer answer. Per project policy that we want to keep recently-out-of-support branches buildable on modern systems, back-patch all the way to 9.2. Discussion: https://postgr.es/m/1803927.1665938411@sss.pgh.pa.us
* Harden pmsignal.c against clobbered shared memory.Tom Lane2022-10-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | The postmaster is not supposed to do anything that depends fundamentally on shared memory contents, because that creates the risk that a backend crash that trashes shared memory will take the postmaster down with it, preventing automatic recovery. In commit 969d7cd43 I lost sight of this principle and coded AssignPostmasterChildSlot() in such a way that it could fail or even crash if the shared PMSignalState structure became corrupted. Remarkably, we've not seen field reports of such crashes; but I managed to induce one while testing the recent changes around palloc chunk headers. To fix, make a semi-duplicative state array inside the postmaster so that we need consult only local state while choosing a "child slot" for a new backend. Ensure that other postmaster-executed routines in pmsignal.c don't have critical dependencies on the shared state, either. Corruption of PMSignalState might now lead ReleasePostmasterChildSlot() to conclude that backend X failed, when actually backend Y was the one that trashed things. But that doesn't matter, because we'll force a cluster-wide reset regardless. Back-patch to all supported branches, since this is an old bug. Discussion: https://postgr.es/m/3436789.1665187055@sss.pgh.pa.us
* Yet further fixes for multi-row VALUES lists for updatable views.Tom Lane2022-10-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | DEFAULT markers appearing in an INSERT on an updatable view could be mis-processed if they were in a multi-row VALUES clause. This would lead to strange errors such as "cache lookup failed for type NNNN", or in older branches even to crashes. The cause is that commit 41531e42d tried to re-use rewriteValuesRTE() to remove any SetToDefault nodes (that hadn't previously been replaced by the view's own default values) appearing in "product" queries, that is DO ALSO queries. That's fundamentally wrong because the DO ALSO queries might not even be INSERTs; and even if they are, their targetlists don't necessarily match the view's column list, so that almost all the logic in rewriteValuesRTE() is inapplicable. What we want is a narrow focus on replacing any such nodes with NULL constants. (That is, in this context we are interpreting the defaults as being strictly those of the view itself; and we already replaced any that aren't NULL.) We could add still more !force_nulls tests to further lobotomize rewriteValuesRTE(); but it seems cleaner to split out this case to a new function, restoring rewriteValuesRTE() to the charter it had before. Per bug #17633 from jiye_sw. Patch by me, but thanks to Richard Guo and Japin Li for initial investigation. Back-patch to all supported branches, as the previous fix was. Discussion: https://postgr.es/m/17633-98cc85e1fa91e905@postgresql.org
* Fix self-referencing foreign keys with partitioned tablesAlvaro Herrera2022-10-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | There are a number of bugs in this area. Two of them are fixed here, namely: 1. get_relation_idx_constraint_oid does not restrict the type of constraint that's returned, so with sufficient bad luck it can return the OID of a foreign key constraint. This has the effect that a primary key in a partition can end up as a child of a foreign key, which makes no sense (it needs to be the child of the equivalent primary key.) Change the API contract so that only index-backed constraints are returned, mimicking get_constraint_index(). 2. Both CloneFkReferenced and CloneFkReferencing clone a self-referencing foreign key, so the partition ends up with a duplicate foreign key. Change the former function to ignore such constraints. Add some tests to verify that things are better now. (However, these new tests show some additional misbehavior that will be fixed later -- namely that there's a constraint marked NOT VALID.) Backpatch to 12, where these constraints are possible at all. Author: Jehan-Guillaume de Rorthais <jgdr@dalibo.com> Discussion: https://postgr.es/m/20220603154232.1715b14c@karst
* Avoid improbable PANIC during heap_update, redux.Tom Lane2022-09-30
| | | | | | | | | | | | | | | | | | | | Commit 34f581c39 intended to ensure that RelationGetBufferForTuple would acquire a visibility-map page pin in case the otherBuffer's all-visible bit had become set since we last had lock on that page. But I missed a case: when we're extending the relation, VM concerns were dealt with only in the relatively-less-likely case that we fail to conditionally lock the otherBuffer. I think I'd believed that we couldn't need to worry about it if the conditional lock succeeds, which is true for the target buffer; but the otherBuffer was unlocked for awhile so its bit might be set anyway. So we need to do the GetVisibilityMapPins dance, and then also recheck the page's free space, in both cases. Per report from Jaime Casanova. Back-patch to v12 as the previous patch was (although there's still no evidence that the bug is reachable pre-v14). Discussion: https://postgr.es/m/E1lWLjP-00006Y-Ml@gemulon.postgresql.org
* Change some errdetail() to errdetail_internal()Alvaro Herrera2022-09-28
| | | | | | | | | | | | This prevents marking the argument string for translation for gettext, and it also prevents the given string (which is already translated) from being translated at runtime. Also, mark the strings used as arguments to check_rolespec_name for translation. Backpatch all the way back as appropriate. None of this is caught by any tests (necessarily so), so I verified it manually.
* Fix tupdesc lifespan bug with AfterTriggersTableData.storeslot.Tom Lane2022-09-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 25936fd46 adjusted things so that the "storeslot" we use for remapping trigger tuples would have adequate lifespan, but it neglected to consider the lifespan of the tuple descriptor that the slot depends on. It turns out that in at least some cases, the tupdesc we are passing is a refcounted tupdesc, and the refcount for the slot's reference can get assigned to a resource owner having different lifespan than the slot does. That leads to an error like "tupdesc reference 0x7fdef236a1b8 is not owned by resource owner SubTransaction". Worse, because of a second oversight in the same commit, we'd try to free the same tupdesc refcount again while cleaning up after that error, leading to recursive errors and an "ERRORDATA_STACK_SIZE exceeded" PANIC. To fix the initial problem, let's just make a non-refcounted copy of the tupdesc we're supposed to use. That seems likely to guard against additional problems, since there's no strong reason for this code to assume that what it's given is a refcounted tupdesc; in which case there's an independent hazard of the tupdesc having shorter lifespan than the slot does. (I didn't bother trying to free said copy, since it should go away anyway when the (sub) transaction context is cleaned up.) The other issue can be fixed by making the code added to AfterTriggerFreeQuery work like the rest of that function, ie be sure that it doesn't try to free the same slot twice in the event of recursive error cleanup. While here, also clean up minor stylistic issues in the test case added by 25936fd46: don't use "create or replace function", as any name collision within the tests is likely to have ill effects that that won't mask; and don't use function names as generic as trigger_function1, especially if you're not going to drop them at the end of the test stanza. Per bug #17607 from Thomas Mc Kay. Back-patch to v12, as the previous fix was. Discussion: https://postgr.es/m/17607-bd8ccc81226f7f80@postgresql.org
* Fix race condition where heap_delete() fails to pin VM page.Jeff Davis2022-09-22
| | | | | | | | | | Similar to 5f12bc94dc, the code must re-check PageIsAllVisible() after buffer lock is re-acquired. Backpatching to the same version, 12. Discussion: https://postgr.es/m/CAEP4nAw9jYQDKd_5Y+-s2E4YiUJq1vqiikFjYGpLShtp-K3gag@mail.gmail.com Reported-by: Robins Tharakan Reviewed-by: Robins Tharakan Backpatch-through: 12
* Fix thinko in comment.Etsuro Fujita2022-09-22
| | | | | | | This comment has been wrong since its introduction in commit 0d5f05cde; backpatch to v12 where that came in. Discussion: https://postgr.es/m/CAPmGK14VGf-xQjGQN4o1QyAbXAaxugU5%3DqfcmTDh1iufUDnV_w%40mail.gmail.com
* Suppress more variable-set-but-not-used warnings from clang 15.Tom Lane2022-09-21
| | | | | | | | | | | | | | | | | | | Mop up assorted set-but-not-used warnings in the back branches. This includes back-patching relevant fixes from commit 152c9f7b8 the rest of the way, but there are also several cases that did not appear in HEAD. Some of those we'd fixed in a retail way but not back-patched, and others I think just got rewritten out of existence during nearby refactoring. While here, also back-patch b1980f6d0 (PL/Tcl: Fix compiler warnings with Tcl 8.6) into 9.2, so that that branch compiles warning-free with modern Tcl. Per project policy, this is a candidate for back-patching into out-of-support branches: it suppresses annoying compiler warnings but changes no behavior. Hence, back-patch all the way to 9.2. Discussion: https://postgr.es/m/514615.1663615243@sss.pgh.pa.us
* Suppress variable-set-but-not-used warnings from clang 15.Tom Lane2022-09-20
| | | | | | | | | | | | | | | | | | | | | | | | clang 15+ will issue a set-but-not-used warning when the only use of a variable is in autoincrements (e.g., "foo++;"). That's perfectly sensible, but it detects a few more cases that we'd not noticed before. Silence the warnings with our usual methods, such as PG_USED_FOR_ASSERTS_ONLY, or in one case by actually removing a useless variable. One thing that we can't nicely get rid of is that with %pure-parser, Bison emits "yynerrs" as a local variable that falls foul of this warning. To silence those, I inserted "(void) yynerrs;" in the top-level productions of affected grammars. Per recently-established project policy, this is a candidate for back-patching into out-of-support branches: it suppresses annoying compiler warnings but changes no behavior. Hence, back-patch to 9.5, which is as far as these patches go without issues. (A preliminary check shows that the prior branches need some other set-but-not-used cleanups too, so I'll leave them for another day.) Discussion: https://postgr.es/m/514615.1663615243@sss.pgh.pa.us
* Future-proof the recursion inside ExecShutdownNode().Tom Lane2022-09-19
| | | | | | | | | | | | | | | | | | | | | | | The API contract for planstate_tree_walker() callbacks is that they take a PlanState pointer and a context pointer. Somebody figured they could save a couple lines of code by ignoring that, and passing ExecShutdownNode itself as the walker even though it has but one argument. Somewhat remarkably, we've gotten away with that so far. However, it seems clear that the upcoming C2x standard means to forbid such cases, and compilers that actively break such code likely won't be far behind. So spend the extra few lines of code to do it honestly with a separate walker function. In HEAD, we might as well go further and remove ExecShutdownNode's useless return value. I left that as-is in back branches though, to forestall complaints about ABI breakage. Back-patch, with the thought that this might become of practical importance before our stable branches are all out of service. It doesn't seem to be fixing any live bug on any currently known platform, however. Discussion: https://postgr.es/m/208054.1663534665@sss.pgh.pa.us
* Fix NaN comparison in circle_same testDaniel Gustafsson2022-09-12
| | | | | | | | | | | | | | | | | | | | | | Commit c4c340088 changed geometric operators to use float4 and float8 functions, and handle NaN's in a better way. The circle sameness test had a typo in the code which resulted in all comparisons with the left circle having a NaN radius considered same. postgres=# select '<(0,0),NaN>'::circle ~= '<(0,0),1>'::circle; ?column? ---------- t (1 row) This fixes the sameness test to consider the radius of both the left and right circle. Backpatch to v12 where this was introduced. Author: Ranier Vilela <ranier.vf@gmail.com> Discussion: https://postgr.es/m/CAEudQAo8dK=yctg2ZzjJuzV4zgOPBxRU5+Kb+yatFiddtQk6Rw@mail.gmail.com Backpatch-through: v12
* Choose FK name correctly during partition attachmentAlvaro Herrera2022-09-08
| | | | | | | | | | | | | | | During ALTER TABLE ATTACH PARTITION, if the name of a parent's foreign key constraint is already used on the partition, the code tries to choose another one before the FK attributes list has been populated, so the resulting constraint name was "<relname>__fkey" instead of "<relname>_<attrs>_fkey". Repair, and add a test case. Backpatch to 12. In 11, the code to attach a partition was not smart enough to cope with conflicting constraint names, so the problem doesn't exist there. Author: Jehan-Guillaume de Rorthais <jgdr@dalibo.com> Discussion: https://postgr.es/m/20220901184156.738ebee5@karst
* Further fixes for MULTIEXPR_SUBLINK fix.Tom Lane2022-09-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Some more things I didn't think about in commits 3f7323cbb et al: MULTIEXPR_SUBLINK subplans might have been converted to initplans instead of regular subplans, in which case they won't show up in the modified targetlist. Fortunately, this would only happen if they have no input parameters, which means that the problem we originally needed to fix can't happen with them. Therefore, there's no need to clone their output parameters, and thus it doesn't hurt that we'll fail to see them in the first pass over the targetlist. Nonetheless, this complicates matters greatly, because now we have to distinguish output Params of initplans (which shouldn't get renumbered) from those of regular subplans (which should). This also breaks the simplistic scheme I used of assuming that the subplans found in the targetlist have consecutive subLinkIds. We really can't avoid the need to know the subplans' subLinkIds in this code. To fix that, add subLinkId as the last field of SubPlan. We can get away with that change in back branches because SubPlan nodes will never be stored in the catalogs, and there's no ABI break for external code that might be looking at the existing fields of SubPlan. Secondly, rewriteTargetListIU might have rolled up multiple FieldStores or SubscriptingRefs into one targetlist entry, breaking the assumption that there's at most one Param to fix per targetlist entry. (That assumption is OK I think in the ruleutils.c code I stole the logic from in 18f51083c, because that only deals with pre-rewrite query trees. But it's definitely not OK here.) Abandon that shortcut and just do a full tree walk on the targetlist to ensure we find all the Params we have to change. Per bug #17606 from Andre Lin. As before, only v10-v13 need the patch. Discussion: https://postgr.es/m/17606-e5c8ad18d31db96a@postgresql.org
* Backpatch nbtree page deletion hardening.Peter Geoghegan2022-09-05
| | | | | | | | | | | | | | | | | | | Postgres 14 commit 5b861baa taught nbtree VACUUM to tolerate buggy opclasses. VACUUM's inability to locate a to-be-deleted page's downlink in the parent page was logged instead of throwing an error. VACUUM could just press on with vacuuming the index, and vacuuming the table as a whole. There are now anecdotal reports of this error causing problems that were much more disruptive than the underlying index corruption ever could be. Anything that makes VACUUM unable to make forward progress against one table/index ultimately risks making the system enter xidStopLimit mode. There is no good reason to take any chances here, so backpatch the hardening commit. Author: Peter Geoghegan <pg@bowt.ie> Discussion: https://postgr.es/m/CAH2-Wzm9HR6Pow=t-iQa57zT8qmX6_M4h14F-pTtb=xFDW5FBA@mail.gmail.com Backpatch: 10-13 (all supported versions that lacked the hardening)
* Fix oversight in recent MULTIEXPR_SUBLINK fix.Tom Lane2022-09-02
| | | | | | | | | | | | | | | Commits 3f7323cbb et al missed the possibility that the Params they are looking for could be buried under implicit coercions, as well as other stuff that processIndirection() could add to the original targetlist entry. Copy the code in ruleutils.c that deals with such cases. (I thought about refactoring so that there's just one copy; but seeing that we only need this in old back branches, it seems not worth the trouble.) Per off-list report from Andre Lin. As before, only v10-v13 need the patch. Discussion: https://postgr.es/m/17596-c5357f61427a81dc@postgresql.org
* Fix some possibly latent bugs in slab.cDavid Rowley2022-09-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | Primarily, this fixes an incorrect calculation in SlabCheck which was looking in the wrong byte for the sentinel check. The reason that we've never noticed this before in the form of a failing sentinel check is because the pre-check to this always fails because all current core users of slab contexts have a chunk size which is already MAXALIGNed, therefore there's never any space for the sentinel byte. It is possible that an extension needs to use a slab context and if they do with a chunk size that's not MAXALIGNed, then they'll likely get errors about overwritten sentinel bytes. Additionally, this patch changes various calculations which are being done based on the sizeof(SlabBlock). Currently, sizeof(SlabBlock) is a multiple of 8, therefore sizeof(SlabBlock) is the same as MAXALIGN(sizeof(SlabBlock)), however, if we were to ever have to add any fields to that struct as part of a bug fix, then SlabAlloc could end up returning a non-MAXALIGNed pointer. To be safe, let's ensure we always MAXALIGN sizeof(SlabBlock) before using it in any calculations. This patch has already been applied to master in d5ee4db0e. Diagnosed-by: Tomas Vondra, Tom Lane Author: Tomas Vondra, David Rowley Discussion: https://postgr.es/m/CAA4eK1%2B1JyW5TiL%3DyV-3Uq1CrfnTyn0Xrk5uArt31Z%3D8rgPhXQ%40mail.gmail.com Backpatch-through: 10
* Prevent long-term memory leakage in autovacuum launcher.Tom Lane2022-08-31
| | | | | | | | | | | | | | | | | | | | | get_database_list() failed to restore the caller's memory context, instead leaving current context set to TopMemoryContext which is how CommitTransactionCommand() leaves it. The callers both think they are using short-lived contexts, for the express purpose of not having to worry about cleaning up individual allocations. The net effect therefore is that supposedly short-lived allocations could accumulate indefinitely in the launcher's TopMemoryContext. Although this has been broken for a long time, it seems we didn't have any obvious memory leak here until v15's rearrangement of the stats logic. I (tgl) am not entirely convinced that there's no other leak at all, though, and we're surely at risk of adding one in future back-patched fixes. So back-patch to all supported branches, even though this may be only a latent bug in pre-v15. Reid Thompson Discussion: https://postgr.es/m/972a4e12b68b0f96db514777a150ceef7dcd2e0f.camel@crunchydata.com
* In the Snowball dictionary, don't try to stem excessively-long words.Tom Lane2022-08-31
| | | | | | | | | | | | | | | | | | | | | If the input word exceeds 1000 bytes, don't pass it to the stemmer; just return it as-is after case folding. Such an input is surely not a word in any human language, so whatever the stemmer might do to it would be pretty dubious in the first place. Adding this restriction protects us against a known recursion-to-stack-overflow problem in the Turkish stemmer, and it seems like good insurance against any other safety or performance issues that may exist in the Snowball stemmers. (I note, for example, that they contain no CHECK_FOR_INTERRUPTS calls, so we really don't want them running for a long time.) The threshold of 1000 bytes is arbitrary. An alternative definition could have been to treat such words as stopwords, but that seems like a bigger break from the old behavior. Per report from Egor Chindyaskin and Alexander Lakhin. Thanks to Olly Betts for the recommendation to fix it this way. Discussion: https://postgr.es/m/1661334672.728714027@f473.i.mail.ru
* Prevent WAL corruption after a standby promotion.Robert Haas2022-08-29
| | | | | | | | | | | | | | | | | | | | | | | | | | | When a PostgreSQL instance performing archive recovery but not using standby mode is promoted, and the last WAL segment that it attempted to read ended in a partial record, the previous code would create invalid WAL on the new timeline. The WAL from the previously timeline would be copied to the new timeline up until the end of the last valid record, but instead of beginning to write WAL at immediately afterwards, the promoted server would write an overwrite contrecord at the beginning of the next segment. The end of the previous segment would be left as all-zeroes, resulting in failures if anything tried to read WAL from that file. The root of the issue is that ReadRecord() decides whether to set abortedRecPtr and missingContrecPtr based on the value of StandbyMode, but ReadRecord() switches to a new timeline based on the value of ArchiveRecoveryRequested. We shouldn't try to write an overwrite contrecord if we're switching to a new timeline, so change the test in ReadRecod() to check ArchiveRecoveryRequested instead. Code fix by Dilip Kumar. Comments by me incorporating suggested language from Álvaro Herrera. Further review from Kyotaro Horiguchi and Sami Imseih. Discussion: http://postgr.es/m/CAFiTN-t7umki=PK8dT1tcPV=mOUe2vNhHML6b3T7W7qqvvajjg@mail.gmail.com Discussion: http://postgr.es/m/FB0DEA0B-E14E-43A0-811F-C1AE93D00FF3%40amazon.com
* Repair rare failure of MULTIEXPR_SUBLINK subplans in inherited updates.Tom Lane2022-08-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Prior to v14, if we have a MULTIEXPR SubPlan (that is, use of the syntax UPDATE ... SET (c1, ...) = (SELECT ...)) in an UPDATE with an inherited or partitioned target table, inheritance_planner() will clone the targetlist and therefore also the MULTIEXPR SubPlan and the Param nodes referencing it for each child target table. Up to now, we've allowed all the clones to share the underlying subplan as well as the output parameter IDs -- that is, the runtime ParamExecData slots. That technique is borrowed from the far older code that supports initplans, and it works okay in that case because the cloned SubPlan nodes are essentially identical. So it doesn't matter which one of the clones the shared ParamExecData.execPlan field might point to. However, this fails to hold for MULTIEXPR SubPlans, because they can have nonempty "args" lists (values to be passed into the subplan), and those lists could get mutated to different states in the various clones. In the submitted reproducer, as well as the test case added here, one clone contains Vars with varno OUTER_VAR where another has INNER_VAR, because the child tables are respectively on the outer or inner side of the join. Sharing the execPlan pointer can result in trying to evaluate an args list that doesn't match the local execution state, with mayhem ensuing. The result often is to trigger consistency checks in the executor, but I believe this could end in a crash or incorrect updates. To fix, assign new Param IDs to each of the cloned SubPlans, so that they don't share ParamExecData slots at runtime. It still seems fine for the clones to share the underlying subplan, and extra ParamExecData slots are cheap enough that this fix shouldn't cost much. This has been busted since we invented MULTIEXPR SubPlans in 9.5. Probably the lack of previous reports is because query plans in which the different clones of a MULTIEXPR mutate to effectively-different states are pretty rare. There's no issue in v14 and later, because without inheritance_planner() there's never a reason to clone MULTIEXPR SubPlans. Per bug #17596 from Andre Lin. Patch v10-v13 only. Discussion: https://postgr.es/m/17596-c5357f61427a81dc@postgresql.org
* Fix typo in comment.Etsuro Fujita2022-08-26
|
* Defend against stack overrun in a few more places.Tom Lane2022-08-24
| | | | | | | | | | | | | | | | | | | SplitToVariants() in the ispell code, lseg_inside_poly() in geo_ops.c, and regex_selectivity_sub() in selectivity estimation could recurse until stack overflow; fix by adding check_stack_depth() calls. So could next() in the regex compiler, but that case is better fixed by converting its tail recursion to a loop. (We probably get better code that way too, since next() can now be inlined into its sole caller.) There remains a reachable stack overrun in the Turkish stemmer, but we'll need some advice from the Snowball people about how to fix that. Per report from Egor Chindyaskin and Alexander Lakhin. These mistakes are old, so back-patch to all supported branches. Richard Guo and Tom Lane Discussion: https://postgr.es/m/1661334672.728714027@f473.i.mail.ru
* Doc: prefer sysctl to /proc/sys in docs and comments.Tom Lane2022-08-23
| | | | | | | | | sysctl is more portable than Linux's /proc/sys file tree, and often easier to use too. That's why most of our docs refer to sysctl when talking about how to adjust kernel parameters. Bring the few stragglers into line. Discussion: https://postgr.es/m/361175.1661187463@sss.pgh.pa.us
* Add CHECK_FOR_INTERRUPTS while decoding changes.Amit Kapila2022-08-23
| | | | | | | | | | | | | While decoding changes in a loop, if we skip all the changes there is no CFI making the loop uninterruptible. Reported-by: Whale Song and Andrey Borodin Bug: 17580 Author: Masahiko Sawada Reviwed-by: Amit Kapila Backpatch-through: 10 Discussion: https://postgr.es/m/17580-849c1d5b6d7eb422@postgresql.org Discussion: https://postgr.es/m/B319ECD6-9A28-4CDF-A8F4-3591E0BF2369@yandex-team.ru
* Fix subtly-incorrect matching of parent and child partitioned indexes.Tom Lane2022-08-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | When creating a partitioned index, DefineIndex tries to identify any existing indexes on the partitions that match the partitioned index, so that it can absorb those as child indexes instead of building new ones. Part of the matching is to compare IndexInfo structs --- but that wasn't done quite right. We're comparing the IndexInfo built within DefineIndex itself to one made from existing catalog contents by BuildIndexInfo. Notably, while BuildIndexInfo will run index expressions and predicates through expression preprocessing, that has not happened to DefineIndex's struct. The result is failure to match and subsequent creation of duplicate indexes. The easiest and most bulletproof fix is to build a new IndexInfo using BuildIndexInfo, thereby guaranteeing that the processing done is identical. While here, let's also extract the opfamily and collation data from the new partitioned index, removing ad-hoc logic that duplicated knowledge about how those are constructed. Per report from Christophe Pettus. Back-patch to v11 where we invented partitioned indexes. Richard Guo and Tom Lane Discussion: https://postgr.es/m/8864BFAA-81FD-4BF9-8E06-7DEB8D4164ED@thebuild.com
* Fix replica identity check for a partitioned table.Amit Kapila2022-08-16
| | | | | | | | | | | | | | The current publisher code checks if UPDATE or DELETE can be executed with the replica identity of the table even if it's a partitioned table. We can skip checking the replica identity for partitioned tables because the operations are actually performed on the leaf partitions (not the partitioned table). Reported-by: Brad Nicholson Author: Hou Zhijie Reviewed-by: Peter Smith, Amit Kapila Backpatch-through: 13 Discussion: https://postgr.es/m/CAMMnM%3D8i5DohH%3DYKzV0_wYuYSYvuOJoL9F5nzXTc%2ByzsG1f6rg%40mail.gmail.com
* Fix outdated --help message for postgres -fMichael Paquier2022-08-15
| | | | | | | | | | This option switch supports a total of 8 values, as told by set_plan_disabling_options() and the documentation, but this was not reflected in the output generated by --help. Author: Junwang Zhao Discussion: https://postgr.es/m/CAEG8a3+pT3cWzyjzKs184L1XMNm8NDnoJLiSjAYSO7XqpRh_vA@mail.gmail.com Backpatch-through: 10
* Preserve memory context of VarStringSortSupport buffers.Tom Lane2022-08-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When enlarging the work buffers of a VarStringSortSupport object, varstrfastcmp_locale was careful to keep them in the ssup_cxt memory context; but varstr_abbrev_convert just used palloc(). The latter creates a hazard that the buffers could be freed out from under the VarStringSortSupport object, resulting in stomping on whatever gets allocated in that memory later. In practice, because we only use this code for ICU collations (cf. 3df9c374e), the problem is confined to use of ICU collations. I believe it may have been unreachable before the introduction of incremental sort, too, as traditional sorting usually just uses one context for the duration of the sort. We could fix this by making the broken stanzas in varstr_abbrev_convert match the non-broken ones in varstrfastcmp_locale. However, it seems like a better idea to dodge the issue altogether by replacing the pfree-and-allocate-anew coding with repalloc, which automatically preserves the chunk's memory context. This fix does add a few cycles because repalloc will copy the chunk's content, which the existing coding assumes is useless. However, we don't expect that these buffer enlargement operations are performance-critical. Besides that, it's far from obvious that copying the buffer contents isn't required, since these stanzas make no effort to mark the buffers invalid by resetting last_returned, cache_blob, etc. That seems to be safe upon examination, but it's fragile and could easily get broken in future, which wouldn't get revealed in testing with short-to-moderate-size strings. Per bug #17584 from James Inform. Whether or not the issue is reachable in the older branches, this code has been broken on its own terms from its introduction, so patch all the way back. Discussion: https://postgr.es/m/17584-95c79b4a7d771f44@postgresql.org
* Avoid misbehavior when hash_table_bytes < bucket_size.Tom Lane2022-08-13
| | | | | | | | | | | | | | | It's possible to reach this case when work_mem is very small and tupsize is (relatively) very large. In that case ExecChooseHashTableSize would get an assertion failure, or with asserts off it'd compute nbuckets = 0, which'd likely cause misbehavior later (I've not checked). To fix, clamp the number of buckets to be at least 1. This is due to faulty conversion of old my_log2() coding in 28d936031. Back-patch to v13, as that was. Zhang Mingli Discussion: https://postgr.es/m/beb64ca0-91e2-44ac-bf4a-7ea36275ec02@Spark
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-05 06:47:50 +0000