aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* pg_dump: avoid unsafe function calls in getPolicies().Tom Lane2022-11-19
| | | | | | | | | | | | | | | | getPolicies() had the same disease I fixed in other places in commit e3fcbbd62, i.e., it was calling pg_get_expr() for expressions on tables that we don't necessarily have lock on. To fix, restrict the query to only collect interesting rows, rather than doing the filtering on the client side. Back-patch of commit 3e6e86abc. That's been in v15/HEAD long enough to have some confidence about it, so now let's fix the problem in older branches. Discussion: https://postgr.es/m/2273648.1634764485@sss.pgh.pa.us Discussion: https://postgr.es/m/7d7eb6128f40401d81b3b7a898b6b4de@W2012-02.nidsa.loc Discussion: https://postgr.es/m/45c93d57-9973-248e-d2df-e02ca9af48d4@darold.net
* Postpone calls of unsafe server-side functions in pg_dump.Tom Lane2022-11-19
| | | | | | | | | | | | | | | | | | | Avoid calling pg_get_partkeydef(), pg_get_expr(relpartbound), and regtypeout until we have lock on the relevant tables. The existing coding is at serious risk of failure if there are any concurrent DROP TABLE commands going on --- including drops of other sessions' temp tables. Back-patch of commit e3fcbbd62. That's been in v15/HEAD long enough to have some confidence about it, so now let's fix the problem in older branches. Original patch by me; thanks to Gilles Darold for back-patching legwork. Discussion: https://postgr.es/m/2273648.1634764485@sss.pgh.pa.us Discussion: https://postgr.es/m/7d7eb6128f40401d81b3b7a898b6b4de@W2012-02.nidsa.loc Discussion: https://postgr.es/m/45c93d57-9973-248e-d2df-e02ca9af48d4@darold.net
* 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
* Account for IPC::Run::result() Windows behavior change.Noah Misch2022-11-17
| | | | | | | | | This restores compatibility with the not-yet-released successor of version 20220807.0. Back-patch to 9.4, which introduced this code. Reviewed by Andrew Dunstan. Discussion: https://postgr.es/m/20221117061805.GA4020280@rfd.leadboat.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
* Fix alter_table.sql test case to test what it claims to.Tom Lane2022-11-10
| | | | | | | | | | | | | | The stanza "SET STORAGE may need to add a TOAST table" does not test what it's supposed to, and hasn't done so since we added the ability to store constant column default values as metadata. We need to use a non-constant default to get the expected table rewrite to actually happen. Fix that, and add the missing checks that would have exposed the problem to begin with. Noted while reviewing a patch that made changes in this test case. Back-patch to v11 where the problem came in.
* Re-allow building on Microsoft Visual Studio 2013.Tom Lane2022-11-10
| | | | | | | | | | | | | | | | | | In commit 450ee7012 I supposed that all platforms we now care about have snprintf(), since that's required by C99. Turns out that Microsoft did not get around to adding that until VS2015. We've dropped support for VS2013 as of HEAD (cf 6203583b7), but not in the back branches, so add a hack for this in the back branches only. There's no easy shortcut to an exact emulation of standard snprintf in VS2013, but fortunately we don't need one: this code was just fine with using sprintf before 450ee7012, so we can make it do so again on that platform (and any others where the problem might crop up). Per bug #17681 from Daisuke Higuchi. Back-patch to v12, like the previous patch. Discussion: https://postgr.es/m/17681-485ba2ec13e7f392@postgresql.org
* 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
* Stamp 12.13.REL_12_13Tom Lane2022-11-07
|
* Translation updatesPeter Eisentraut2022-11-07
| | | | | Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: ff92e39b5698b83b8f5290094153a59df3056a1a
* 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
* Allow use of __sync_lock_test_and_set for spinlocks on any machine.Tom Lane2022-11-02
| | | | | | | | | | | | | | | | | | | | | | | | | If we have no special-case code in s_lock.h for the current platform, but the compiler has __sync_lock_test_and_set, use that instead of failing. It's unlikely that anybody's __sync_lock_test_and_set would be so awful as to be worse than our semaphore-based fallback, but if it is, they can (continue to) use --disable-spinlocks. This allows removal of the RISC-V special case installed by commit c32fcac56, which generated exactly the same code but only on that platform. Usefully, the RISC-V buildfarm animals should now test at least the int variant of this patch. I've manually tested both variants on ARM by dint of removing the ARM-specific stanza. We don't want to drop that, because it already has some special knowledge and is likely to grow more over time. Likewise, this is not meant to preclude installing special cases for other arches if that proves worthwhile. Per discussion of a request to install the same code for loongarch64. Like the previous patch, we might as well back-patch to supported branches. Discussion: https://postgr.es/m/761ac43d44b84d679ba803c2bd947cc0@HSMAILSVR04.hs.handsome.com.cn
* Fix copy-and-pasteo in comment.Etsuro Fujita2022-11-02
|
* Update time zone data files to tzdata release 2022f.Tom Lane2022-11-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | DST law changes in Chile, Fiji, Iran, Jordan, Mexico, Palestine, and Syria. Historical corrections for Chile, Crimea, Iran, and Mexico. Also, the Europe/Kiev zone has been renamed to Europe/Kyiv (retaining the old name as a link). The following zones have been merged into nearby, more-populous zones whose clocks have agreed since 1970: Antarctica/Vostok, Asia/Brunei, Asia/Kuala_Lumpur, Atlantic/Reykjavik, Europe/Amsterdam, Europe/Copenhagen, Europe/Luxembourg, Europe/Monaco, Europe/Oslo, Europe/Stockholm, Indian/Christmas, Indian/Cocos, Indian/Kerguelen, Indian/Mahe, Indian/Reunion, Pacific/Chuuk, Pacific/Funafuti, Pacific/Majuro, Pacific/Pohnpei, Pacific/Wake and Pacific/Wallis. (This indirectly affects zones that were already links to one of these: Arctic/Longyearbyen, Atlantic/Jan_Mayen, Iceland, Pacific/Ponape, Pacific/Truk, and Pacific/Yap.) America/Nipigon, America/Rainy_River, America/Thunder_Bay, Europe/Uzhgorod, and Europe/Zaporozhye were also merged into nearby zones after discovering that their claimed post-1970 differences from those zones seem to have been errors. While the IANA crew have been working on merging zones that have no post-1970 differences for some time, this batch of changes affects some zones that are significantly more populous than those merged in the past, notably parts of Europe. The loss of pre-1970 timezone history for those zones may be troublesome for applications expecting consistency of timestamptz display. As an example, the stored value '1944-06-01 12:00 UTC' would previously display as '1944-06-01 13:00:00+01' if the Europe/Stockholm zone is selected, but now it will read out as '1944-06-01 14:00:00+02'. There exists a "packrat" option that will build the timezone data files with this old data preserved, but the problem is that it also resurrects a bunch of other, far less well-attested data; so much so that actually more zones' contents change from 2022a with that option than without it. I have chosen not to do that here, for that reason and because it appears that no major OS distributions are using the "packrat" option, so that doing so would cause Postgres' behavior to diverge significantly depending on whether it was built with --with-system-tzdata. However, for anyone for whom these changes pose significant problems, there is a solution: build a set of timezone files with the "packrat" option and use those with Postgres.
* 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
* pg_basebackup: Fix cross-platform tablespace relocation.Robert Haas2022-10-21
| | | | | | | | | | | | | | Specifically, when pg_basebackup is invoked with -Tx=y, don't error out if x could plausibly be an absolute path either on Windows or on non-Windows systems. We don't know whether the remote system is running the same OS as the local system, so it's not appropriate to assume that our local rule about absolute pathnames is the same as the rule on the remote system. Patch by me, reviewed by Tom Lane, Andrew Dunstan, and Davinder Singh. Discussion: http://postgr.es/m/CA+TgmoY+jC3YiskomvYKDPK3FbrmsDU7_8+wMHt02HOdJeRb0g@mail.gmail.com
* 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
* Use libc's snprintf, not sprintf, for special cases in snprintf.c.Tom Lane2022-10-16
| | | | | | | | | | | | | | | | | | | | | | | | | | snprintf.c has always fallen back on libc's *printf implementation when printing pointers (%p) and floats. When this code originated, we were still supporting some platforms that lacked native snprintf, so we used sprintf for that. That's not actually unsafe in our usage, but nonetheless builds on macOS are starting to complain about sprintf being unconditionally deprecated; and I wouldn't be surprised if other platforms follow suit. There seems little reason to believe that any platform supporting C99 wouldn't have standards-compliant snprintf, so let's just use that instead to suppress such warnings. Back-patch to v12, which is where we started to require C99. It's also where we started to use our snprintf.c everywhere, so this wouldn't be enough to suppress the warning in older branches anyway --- that is, in older branches these aren't necessarily all our usages of libc's sprintf. It is enough in v12+ because any deprecation annotation attached to libc's sprintf won't apply to pg_sprintf. (Whether all our usages of pg_sprintf are adequately safe is not a matter I intend to address here, but perhaps it could do with some review.) Per report from Andres Freund and local testing. Discussion: https://postgr.es/m/20221015211955.q4cwbsfkyk3c4ty3@awork3.anarazel.de
* 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
* Ensure all perl test modules are installedAlvaro Herrera2022-10-11
| | | | | | | | | | | | PostgreSQL::Test::Cluster and ::Utils were not being installed. This is very hard to notice, as it only seems to affect external modules that want to run tests from 15 back in earlier versions. Oversight in b235d41d9646. This applies only to branches 14 and back, because 15 had already been made correct in commit b3b4d8e68ae8. Discussion: https://postgr.es/m/20221010093415.poplkyn7pjeiv2y7@alvherre.pgsql
* 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
* Add missing source files to pg_waldump/nls.mkAlvaro Herrera2022-09-25
|
* 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
* Make check_usermap() parameter names consistent.Peter Geoghegan2022-09-17
| | | | | | | | | | | The function has a bool argument named "case_insensitive", but that was spelled "case_sensitive" in the declaration. Make them consistent now to avoid confusion in the future. Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Michael Paquiër <michael@paquier.xyz> Discussion: https://postgr.es/m/CAH2-WznJt9CMM9KJTMjJh_zbL5hD9oX44qdJ4aqZtjFi-zA3Tg@mail.gmail.com Backpatch: 10-
* Include c.h instead of postgres.h in src/port/*p{read,write}*.cAndres Freund2022-09-17
| | | | | | | | Frontend code shouldn't include postgres.h. Some files in src/port/ need to include postgres.h/postgres_fe.h, but these files don't. Discussion: https://postgr.es/m/20220915022626.5xx3ccgkzpkqw5mq@awork3.anarazel.de Backpatch: 12-, where 3fd2a7932ef introduced (some) of these files
* Improve plpgsql's ability to handle arguments declared as RECORD.Tom Lane2022-09-16
| | | | | | | | | | | | | | | | | Treat arguments declared as RECORD as if that were a polymorphic type (which it is, sort of), in that we substitute the actual argument type while forming the function cache lookup key. This allows the specific composite type to be known in some cases where it was not before, at the cost of making a separate function cache entry for each named composite type that's passed to the function during a session. The particular symptom discussed in bug #17610 could be solved in other more-efficient ways, but only at the cost of considerable development work, and there are other cases where we'd still fail without this. Per bug #17610 from Martin Jurča. Back-patch to v11 where we first allowed plpgsql functions to be declared as taking type RECORD. Discussion: https://postgr.es/m/17610-fb1eef75bf6c2364@postgresql.org
* Fix incorrect value for "strategy" with deflateParams() in walmethods.cMichael Paquier2022-09-14
| | | | | | | | | | | | | | The zlib documentation mentions the values supported for the compression strategy, but this code has been using a hardcoded value of 0 rather than Z_DEFAULT_STRATEGY. This commit adjusts the code to use Z_DEFAULT_STRATEGY. Backpatch down to where this code has been added to ease the backport of any future patch touching this area. Reported-by: Tom Lane Discussion: https://postgr.es/m/1400032.1662217889@sss.pgh.pa.us Backpatch-through: 10
* Expand palloc/pg_malloc API for more type safetyPeter Eisentraut2022-09-14
| | | | | | | | | | | | | | | | | | | | | This adds additional variants of palloc, pg_malloc, etc. that encapsulate common usage patterns and provide more type safety. Specifically, this adds palloc_object(), palloc_array(), and repalloc_array(), which take the type name of the object to be allocated as its first argument and cast the return as a pointer to that type. There are also palloc0_object() and palloc0_array() variants for initializing with zero, and pg_malloc_*() variants of all of the above. Inspired by the talloc library. This is backpatched from master so that future backpatchable code can make use of these APIs. This patch by itself does not contain any users of these APIs. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/bb755632-2a43-d523-36f8-a1e7a389a907@enterprisedb.com
* 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
* Fix possible omission of variable storage markers in ECPG.Tom Lane2022-09-09
| | | | | | | | | | | | | | | | | | | | | | | The ECPG preprocessor converted code such as static varchar str1[10], str2[20], str3[30]; into static struct varchar_1 { int len; char arr[ 10 ]; } str1 ; struct varchar_2 { int len; char arr[ 20 ]; } str2 ; struct varchar_3 { int len; char arr[ 30 ]; } str3 ; thus losing the storage attribute for the later variables. Repeat the declaration for each such variable. (Note that this occurred only for variables declared "varchar" or "bytea", which may help explain how it escaped detection for so long.) Andrey Sokolov Discussion: https://postgr.es/m/942241662288242@mail.yandex.ru
* 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)
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-03 23:22:18 +0000