aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Remove comment about xl_heap_inplace "AT END OF STRUCT".Noah Misch2024-06-27
| | | | | | | | Commit 2c03216d831160bedd72d45f712601b6f7d03f1c moved the tuple data from there to the buffer-0 data. Back-patch to v12 (all supported versions), the plan for the next change to this struct. Discussion: https://postgr.es/m/20240523000548.58.nmisch@google.com
* Cope with inplace update making catcache stale during TOAST fetch.Noah Misch2024-06-27
| | | | | | | | | | | | | | | | This extends ad98fb14226ae6456fbaed7990ee7591cbe5efd2 to invals of inplace updates. Trouble requires an inplace update of a catalog having a TOAST table, so only pg_database was at risk. (The other catalog on which core code performs inplace updates, pg_class, has no TOAST table.) Trouble would require something like the inplace-inval.spec test. Consider GRANT ... ON DATABASE fetching a stale row from cache and discarding a datfrozenxid update that vac_truncate_clog() has already relied upon. Back-patch to v12 (all supported versions). Reviewed (in an earlier version) by Robert Haas. Discussion: https://postgr.es/m/20240114201411.d0@rfd.leadboat.com Discussion: https://postgr.es/m/20240512232923.aa.nmisch@google.com
* AccessExclusiveLock new relations just after assigning the OID.Noah Misch2024-06-27
| | | | | | | | | | | | | | This has no user-visible, important consequences, since other sessions' catalog scans can't find the relation until we commit. However, this unblocks introducing a rule about locks required to heap_update() a pg_class row. CREATE TABLE has been acquiring this lock eventually, but it can heap_update() pg_class.relchecks earlier. create_toast_table() has been acquiring only ShareLock. Back-patch to v12 (all supported versions), the plan for the commit relying on the new rule. Reviewed (in an earlier version) by Robert Haas. Discussion: https://postgr.es/m/20240611024525.9f.nmisch@google.com
* Lock before setting relhassubclass on RELKIND_PARTITIONED_INDEX.Noah Misch2024-06-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 5b562644fec696977df4a82790064e8287927891 added a comment that SetRelationHasSubclass() callers must hold this lock. When commit 17f206fbc824d2b4b14480199ca9ff7dea417eda extended use of this column to partitioned indexes, it didn't take the lock. As the latter commit message mentioned, we currently never reset a partitioned index to relhassubclass=f. That largely avoids harm from the lock omission. The cause for fixing this now is to unblock introducing a rule about locks required to heap_update() a pg_class row. This might cause more deadlocks. It gives minor user-visible benefits: - If an ALTER INDEX SET TABLESPACE runs concurrently with ALTER TABLE ATTACH PARTITION or CREATE PARTITION OF, one transaction blocks instead of failing with "tuple concurrently updated". (Many cases of DDL concurrency still fail that way.) - Match ALTER INDEX ATTACH PARTITION in choosing to lock the index. While not user-visible today, we'll need this if we ever make something set the flag to false for a partitioned index, like ANALYZE does today for tables. Back-patch to v12 (all supported versions), the plan for the commit relying on the new rule. In back branches, add LockOrStrongerHeldByMe() instead of adding a LockHeldByMe() parameter. Reviewed (in an earlier version) by Robert Haas. Discussion: https://postgr.es/m/20240611024525.9f.nmisch@google.com
* Expand comments and add an assertion in nodeModifyTable.c.Noah Misch2024-06-27
| | | | | | | | | | | Most comments concern RELKIND_VIEW. One addresses the ExecUpdate() "tupleid" parameter. A later commit will rely on these facts, but they hold already. Back-patch to v12 (all supported versions), the plan for that commit. Reviewed (in an earlier version) by Robert Haas. Discussion: https://postgr.es/m/20240512232923.aa.nmisch@google.com
* Improve test coverage for changes to inplace-updated catalogs.Noah Misch2024-06-27
| | | | | | | | | | This covers both regular and inplace changes, since bugs arise at their intersection. Where marked, these witness extant bugs. Back-patch to v12 (all supported versions). Reviewed (in an earlier version) by Robert Haas. Discussion: https://postgr.es/m/20240512232923.aa.nmisch@google.com
* Avoid crashing when a JIT-inlined backend function throws an error.Tom Lane2024-06-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | errfinish() assumes that the __FUNC__ and __FILE__ arguments it's passed are compile-time constant strings that can just be pointed to rather than physically copied. However, it's possible for LLVM to generate code in which those pointers point into a dynamically loaded code segment. If that segment gets unloaded before we're done with the ErrorData struct, we have dangling pointers that will lead to SIGSEGV. In simple cases that won't happen, because we won't unload LLVM code before end of transaction. But it's possible to happen if the error is thrown within end-of-transaction code run by _SPI_commit or _SPI_rollback, because since commit 2e517818f those functions clean up by ending the transaction and starting a new one. Rather than fixing this by adding pstrdup() overhead to every elog/ereport sequence, let's fix it by copying the risky pointers in CopyErrorData(). That solves it for _SPI_commit/_SPI_rollback because they use that function to preserve the error data across the transaction end/restart sequence; and it seems likely that any other code doing something similar would need to do that too. I'm suspicious that this behavior amounts to an LLVM bug (or a bug in our use of it?), because it implies that string constant references that should be pointer-equal according to a naive understanding of C semantics will sometimes not be equal. However, even if it is a bug and someday gets fixed, we'll have to cope with the current behavior for a long time to come. Report and patch by me. Back-patch to all supported branches. Discussion: https://postgr.es/m/1565654.1719425368@sss.pgh.pa.us
* Fix MVCC bug with prepared xact with subxacts on standbyHeikki Linnakangas2024-06-27
| | | | | | | | | | | | | | | | | | We did not recover the subtransaction IDs of prepared transactions when starting a hot standby from a shutdown checkpoint. As a result, such subtransactions were considered as aborted, rather than in-progress. That would lead to hint bits being set incorrectly, and the subtransactions suddenly becoming visible to old snapshots when the prepared transaction was committed. To fix, update pg_subtrans with prepared transactions's subxids when starting hot standby from a shutdown checkpoint. The snapshots taken from that state need to be marked as "suboverflowed", so that we also check the pg_subtrans. Backport to all supported versions. Discussion: https://www.postgresql.org/message-id/6b852e98-2d49-4ca1-9e95-db419a2696e0@iki.fi
* tests: Trim newline from result returned by BackgroundPsql->queryHeikki Linnakangas2024-06-27
| | | | | | | | | | This went unnoticed, because only a few existing callers of BackgroundPsql->query used the result, and the ones that did were not bothered by an extra newline. I noticed because I was about to add a new test that checks the result. Backport to all supported versions, since I just backported the BackgroundPsql facility to all supported versions too.
* Backport BackgroundPsql perl test moduleHeikki Linnakangas2024-06-27
| | | | | | | | | | | | | | | Backport the new BackgroundPsql modules and the constructor functions, background_psql() and interactive_psql, to all supported branches. That makes it easier to backpatch tests that use it. BackgroundPsql was introduced in version 16. On version 16, this commit backports just the new timeout argument from master (commit 334f512f45). On older branches, the whole facility. This includes the change to `use warnings FATAL => 'all'`, which we haven't otherwise backported, but it seems good to keep the file identical across branches. Discussion: https://www.postgresql.org/message-id/b7c64f20-ea01-4f15-9088-0cd6832af149@iki.fi
* Remove redundant perl version checksAndrew Dunstan2024-06-26
| | | | | | | | | | | | | Commit 4c1532763a removed some redundant uses of 'use 5.008001;' in perl scripts, including in plperl's plc_perlboot.pl. Because it made other changes it wasn't backpatched. However, now this is causing a failure on back branches when built with bleeding edge perl. Therefore, backpatch just that part of it which removed those uses, from 15 all the way down to 9.2, which is the earliest version currently built in the buildfarm. per report from Alexander Lakhin Discussion: https://postgr.es/m/4cc2ee93-e03c-8e13-61ed-412e7e6ff19d@gmail.com
* Don't throw an error if a queued AFTER trigger no longer exists.Tom Lane2024-06-20
| | | | | | | | | | | | | | | | | | | | | afterTriggerInvokeEvents and AfterTriggerExecute have always treated it as an error if the trigger OID mentioned in a queued after-trigger event can't be found. However, that fails to account for the edge case where the trigger's been dropped in the current transaction since queueing the event. There seems no very good reason to disallow that case, so instead silently do nothing if the trigger OID can't be found. This does give up a little bit of bug-detection ability, but I don't recall that these error messages have ever actually revealed a bug, so it seems mostly theoretical. Alternatives such as marking pending events DONE at the time of dropping a trigger would be complicated and perhaps introduce bugs of their own. Per bug #18517 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/18517-af2d19882240902c@postgresql.org
* Fix insertion of SP-GiST REDIRECT tuples during REINDEX CONCURRENTLY.Tom Lane2024-06-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Reconstruction of an SP-GiST index by REINDEX CONCURRENTLY may insert some REDIRECT tuples. This will typically happen in a transaction that lacks an XID, which leads either to assertion failure in spgFormDeadTuple or to insertion of a REDIRECT tuple with zero xid. The latter's not good either, since eventually VACUUM will apply GlobalVisTestIsRemovableXid() to the zero xid, resulting in either an assertion failure or a garbage answer. In practice, since REINDEX CONCURRENTLY locks out index scans till it's done, it doesn't matter whether it inserts REDIRECTs or PLACEHOLDERs; and likewise it doesn't matter how soon VACUUM reduces such a REDIRECT to a PLACEHOLDER. So in non-assert builds there's no observable problem here, other than perhaps a little index bloat. But it's not behaving as intended. To fix, remove the failing Assert in spgFormDeadTuple, acknowledging that we might sometimes insert a zero XID; and guard VACUUM's GlobalVisTestIsRemovableXid() call with a test for valid XID, ensuring that we'll reduce such a REDIRECT the first time VACUUM sees it. (Versions before v14 use TransactionIdPrecedes here, which won't fail on zero xid, so they really have no bug at all in non-assert builds.) Another solution could be to not create REDIRECTs at all during REINDEX CONCURRENTLY, making the relevant code paths treat that case like index build (which likewise knows that no concurrent index scans can be happening). That would allow restoring the Assert in spgFormDeadTuple, but we'd still need the VACUUM change because redirection tuples with zero xid may be out there already. But there doesn't seem to be a nice way for spginsert() to tell that it's being called in REINDEX CONCURRENTLY without some API changes, so we'll leave that as a possible future improvement. In HEAD, also rename the SpGistState.myXid field to redirectXid, which seems less misleading (since it might not in fact be our transaction's XID) and is certainly less uninformatively generic. Per bug #18499 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/18499-8a519c280f956480@postgresql.org
* Clean out column-level pg_init_privs entries when dropping tables.Tom Lane2024-06-14
| | | | | | | | | | | | | | DeleteInitPrivs did not get the memo about how, when dropping a whole object (with subid == 0), you should drop entries relating to its sub-objects too. This is visible in the test_pg_dump test case if one drops the extension at the end: the entry for GRANT SELECT(col1) ON regress_pg_dump_table TO public; was still present in pg_init_privs afterwards, although it was pointing to a dangling table OID. Noted while fooling with a fix for REASSIGN OWNED for pg_init_privs entries. This bug is aboriginal in the pg_init_privs feature though, and there seems no reason not to back-patch the fix.
* Fix parsing of ignored operators in websearch_to_tsquery().Tom Lane2024-06-13
| | | | | | | | | | | | | | | | | | | | | | | | | The manual says clearly that punctuation in the input of websearch_to_tsquery() is ignored, except for the special cases of dashes and quotes. However, this failed for cases like "(foo bar) or something", or in general an ISOPERATOR character in front of the "or". We'd switch back to WAITOPERAND state, then ignore the operator character while remaining in that state, and then reach the "or" in WAITOPERAND state which (intentionally) makes us treat it as data. The fix is simple enough: if we see an ISOPERATOR character while in WAITOPERATOR state, we have to skip it while staying in that state. (We don't need to worry about other punctuation characters: those will be consumed as though they were words, but then rejected by lexizing.) In v14 and up (since commit eb086056f) we can simplify the code a bit more too, because there is no longer a reason for the WAITOPERAND state to distinguish between quoted and unquoted operands. Per bug #18479 from Manos Emmanouilidis. Back-patch to all supported branches. Discussion: https://postgr.es/m/18479-d9b46e2fc242c33e@postgresql.org
* When replanning a plpgsql "simple expression", check it's still simple.Tom Lane2024-06-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The previous coding here assumed that we didn't need to recheck any of the querytree tests made in exec_simple_check_plan(). I think we supposed that those properties were fully determined by the syntax of the source text and hence couldn't change. That is true for most of them, but at least hasTargetSRFs and hasAggs can change by dint of forcibly dropping an originally-referenced function and recreating it with new properties. That leads to "unexpected plan node type" or similar failures. These tests are pretty cheap compared to the cost of replanning, so rather than sweat over exactly which properties need to be rechecked, let's just recheck them all. Hence, factor out those tests into a new function exec_is_simple_query(), and rearrange callers as needed. A second problem in the same area was that if we failed during replanning or during exec_save_simple_expr(), we'd potentially leave behind now-dangling pointers to the old simple expression, potentially resulting in crashes later. To fix, clear those pointers before replanning. The v12 code looks quite different in this area but still has the bug about needing to recheck query simplicity. I chose to back-patch all of the plpgsql_simple.sql test script, which formerly didn't exist in this branch. Per bug #18497 from Nikita Kalinin. Back-patch to all supported branches. Discussion: https://postgr.es/m/18497-fe93b6da82ce31d4@postgresql.org
* Clamp result of MultiXactMemberFreezeThresholdHeikki Linnakangas2024-06-13
| | | | | | | | | | | | The purpose of the function is to reduce the effective autovacuum_multixact_freeze_max_age if the multixact members SLRU is approaching wraparound, to make multixid freezing more aggressive. The returned value should therefore never be greater than plain autovacuum_multixact_freeze_max_age. Reviewed-by: Robert Haas Discussion: https://www.postgresql.org/message-id/85fb354c-f89f-4d47-b3a2-3cbd461c90a3@iki.fi Backpatch-through: 12, all supported versions
* Skip some permissions checks on CygwinAndrew Dunstan2024-06-13
| | | | | | These are checks that are already skipped on other Windows systems. Backpatch to all live branches, as appropriate.
* Fix infer_arbiter_indexes() to not assume resultRelation is 1.Tom Lane2024-06-11
| | | | | | | | | | | | | | | | | | infer_arbiter_indexes failed to renumber varnos in index expressions or predicates that it got from the catalogs. This escaped detection up to now because the stored varnos in such trees will be 1, and an INSERT's result relation is usually the first rangetable entry, so that that was fine. However, in cases such as inserting through an updatable view, it's not fine, leading to failure to match the expressions to the query with ensuing "there is no unique or exclusion constraint matching the ON CONFLICT specification" errors. Fix by copy-and-paste from get_relation_info(). Per bug #18502 from Michael Wang. Back-patch to all supported versions. Discussion: https://postgr.es/m/18502-545b53f5b81e54e0@postgresql.org
* Tighten test_predtest's input checks, and improve error messages.Tom Lane2024-06-07
| | | | | | | | | | | | | | | | | test_predtest() neglected to consider the possibility that SPI_plan_get_cached_plan would return NULL. This led to a core dump if the input (incorrectly) contains more than one SQL command. While here, let's expend more than zero effort on the error message for this case and nearby ones. Per (half of) bug #18483 from Alexander Kozhemyakin. Back-patch to all supported branches, not because this is very significant (it's merely test scaffolding) but to make our world a bit safer for fuzz testing. Discussion: https://postgr.es/m/18483-30bfff42de238000@postgresql.org
* Reject modifying a temp table of another session with ALTER TABLE.Tom Lane2024-06-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | Normally this case isn't even reachable by non-superusers, since permissions checks prevent naming such a table. However, it is possible to make it happen by altering a parent table whose child is another session's temp table. We definitely can't support any such ALTER that requires modifying the contents of such a table, since we lack access to the other session's temporary-buffer pool. But there seems no good reason to allow it even if it'd only require changing catalog contents. One reason not to allow it is that we'd rather not expose the implementation-dependent behavior of whether a specific ALTER requires touching the table contents. Another is that there may be (in future, even if not today) optimizations that assume that a session's own temp tables won't be modified by other sessions. Hence, add a RELATION_IS_OTHER_TEMP() check to all the places where ALTER TABLE currently does CheckTableNotInUse(). (I looked through all other callers of CheckTableNotInUse(), and they seem OK already.) Per bug #18492 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/18492-c7a2634bf4968763@postgresql.org
* Fix behavior of stable functions called from a CALL's argument list.Tom Lane2024-06-07
| | | | | | | | | | | | | | | | | | | | | | | | | | If the CALL is within an atomic context (e.g. there's an outer transaction block), _SPI_execute_plan should acquire a fresh snapshot to execute any such functions with. We failed to do that and instead passed them the Portal snapshot, which had been acquired at the start of the current SQL command. This'd lead to seeing stale values of rows modified since the start of the command. This is arguably a bug in 84f5c2908: I failed to see that "are we in non-atomic mode" needs to be defined the same way as it is further down in _SPI_execute_plan, i.e. check !_SPI_current->atomic not just options->allow_nonatomic. Alternatively the blame could be laid on plpgsql, which is unconditionally passing allow_nonatomic = true for CALL/DO even when it knows it's in an atomic context. However, fixing it in spi.c seems like a better idea since that will also fix the problem for any extensions that may have copied plpgsql's coding pattern. While here, update an obsolete comment about _SPI_execute_plan's snapshot management. Per report from Victor Yegorov. Back-patch to all supported versions. Discussion: https://postgr.es/m/CAGnEboiRe+fG2QxuBO2390F7P8e2MQ6UyBjZSL_w1Cej+E4=Vw@mail.gmail.com
* Fix failure with SQL-procedure polymorphic output arguments in v12.Tom Lane2024-06-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Before the v13-era commit 913bbd88d, check_sql_fn_retval fails to resolve polymorphic output types and then just throws up its hands and assumes the check will be made at runtime. I think that's true for ordinary functions returning RECORD, but it doesn't happen in CALL, potentially resulting in crashes if the actual output of the SQL procedure's SELECT doesn't match the type inferred from polymorphism. With a little bit of rearrangement, we can use get_call_result_type instead of get_func_result_type and thereby infer the correct types. I'm still unwilling to back-patch all of 913bbd88d, so if the types don't match you'll get an error rather than perhaps silently inserting a cast as v13 and later can. That's consistent with prior behavior though, so it seems fine. Prior to 70ffb27b2, you'd typically get other errors due to other shortcomings of CALL's management of polymorphism. Nonetheless, this is an independent bug. Although there is no bug in v13 and up, it seems prudent to add the test case for this to the newer branches too. It's clearly an under-tested area. Per report from Andrew Bille. Discussion: https://postgr.es/m/CAJnzarw9EeWHAQRm76dXd=7j+rgw6ERqC=nCay8jeFqTwKwhqQ@mail.gmail.com
* Fix pl/tcl's handling of errors from Tcl_ListObjGetElements().Tom Lane2024-06-04
| | | | | | | | | | | | | | | | | | | | | | | | | In a procedure or function returning tuple, we use that function to parse the Tcl script's result, which is supposed to be a Tcl list. If it isn't, you get an error. Commit 26abb50c4 incautiously supposed that we could use throw_tcl_error() to report such an error. That doesn't actually work, because low-level functions like Tcl_ListObjGetElements() don't fill Tcl's errorInfo variable. The result is either a null-pointer-dereference crash or emission of misleading context information describing the previous Tcl error. Back off to just reporting the interpreter's result string, and improve throw_tcl_error()'s comment to explain when to use it. Also, although the similar code in pltcl_trigger_handler() avoided this mistake, it was using a fairly confusing wording of the error message. Improve that while we're here. Per report from A. Kozhemyakin. Back-patch to all supported branches. Erik Wienhold and Tom Lane Discussion: https://postgr.es/m/6a2a1c40-2b2c-4a33-8b72-243c0766fcda@postgrespro.ru
* Remove race conditions between ECPGdebug() and ecpg_log().Tom Lane2024-05-23
| | | | | | | | | | | | | | | | | | | | | | Coverity complains that ECPGdebug is accessing debugstream without holding debug_mutex, which is a fair complaint: we should take debug_mutex while changing the settings ecpg_log looks at. In some branches it also complains about unlocked use of simple_debug. I think it's intentional and safe to have a quick unlocked check of simple_debug at the start of ecpg_log, since that early exit will always be taken in non-debug cases. But we should recheck simple_debug after acquiring the mutex. In the worst case, calling ECPGdebug concurrently with ecpg_log in another thread could result in a null-pointer dereference due to debugstream transiently being NULL while simple_debug isn't 0. This is largely hypothetical, since it's unlikely anybody uses ECPGdebug() at all in the field, and our own regression tests don't seem to be hitting the theoretical race conditions either. Still, if we're going to the trouble of having mutexes here, we ought to be using them in a way that's actually safe not just almost safe. Hence, back-patch to all supported branches.
* Account for optimized MinMax aggregates during SS_finalize_plan.Tom Lane2024-05-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We are capable of optimizing MIN() and MAX() aggregates on indexed columns into subqueries that exploit the index, rather than the normal thing of scanning the whole table. When we do this, we replace the Aggref node(s) with Params referencing subquery outputs. Such Params really ought to be included in the per-plan-node extParam/allParam sets computed by SS_finalize_plan. However, we've never done so up to now because of an ancient implementation choice to perform that substitution during set_plan_references, which runs after SS_finalize_plan, so that SS_finalize_plan never sees these Params. The cleanest fix would be to perform a separate tree walk to do these substitutions before SS_finalize_plan runs. That seems unattractive, first because a whole-tree mutation pass is expensive, and second because we lack infrastructure for visiting expression subtrees in a Plan tree, so that we'd need a new function knowing as much as SS_finalize_plan knows about that. I also considered swapping the order of SS_finalize_plan and set_plan_references, but that fell foul of various assumptions that seem tricky to fix. So the approach adopted here is to teach SS_finalize_plan itself to check for such Aggrefs. I refactored things a bit in setrefs.c to avoid having three copies of the code that does that. Back-patch of v17 commits d0d44049d and 779ac2c74. When d0d44049d went in, there was no evidence that it was fixing a reachable bug, so I refrained from back-patching. Now we have such evidence. Per bug #18465 from Hal Takahara. Back-patch to all supported branches. Discussion: https://postgr.es/m/18465-2fae927718976b22@postgresql.org Discussion: https://postgr.es/m/2391880.1689025003@sss.pgh.pa.us
* Refuse upgrades from pre-9.0 clustersDaniel Gustafsson2024-05-17
| | | | | | | | | | | | | | Commit 695b4a113ab added a dependency on retrieving oldestxid from pg_control, which only exists in 9.0 and onwards, but the check for 8.4 as the oldest version was retained. Since there has been few if any complaints of 8.4 upgrades not working, fix by setting 9.0 as the oldest version supported rather than resurrecting 8.4 support. Backpatch to all supported versions. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/1973418.1657040382@sss.pgh.pa.us Backpatch-through: v12
* Fix handling of polymorphic output arguments for procedures.Tom Lane2024-05-14
| | | | | | | | | | | | | | | | | | | | | | | | | Most of the infrastructure for procedure arguments was already okay with polymorphic output arguments, but it turns out that CallStmtResultDesc() was a few bricks shy of a load here. It thought all it needed to do was call build_function_result_tupdesc_t, but that function specifically disclaims responsibility for resolving polymorphic arguments. Failing to handle that doesn't seem to be a problem for CALL in plpgsql, but CALL from plain SQL would get errors like "cannot display a value of type anyelement", or even crash outright. In v14 and later we can simply examine the exposed types of the CallStmt.outargs nodes to get the right type OIDs. But it's a lot more complicated to fix in v12/v13, because those versions don't have CallStmt.outargs, nor do they do expand_function_arguments until ExecuteCallStmt runs. We have to duplicatively run expand_function_arguments, and then re-determine which elements of the args list are output arguments. Per bug #18463 from Drew Kimball. Back-patch to all supported versions, since it's busted in all of them. Discussion: https://postgr.es/m/18463-f8cd77e12564d8a2@postgresql.org
* Fix pg_sequence_last_value() for unlogged sequences on standbys.Nathan Bossart2024-05-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Presently, when this function is called for an unlogged sequence on a standby server, it will error out with a message like ERROR: could not open file "base/5/16388": No such file or directory Since the pg_sequences system view uses pg_sequence_last_value(), it can error similarly. To fix, modify the function to return NULL for unlogged sequences on standby servers. Since this bug is present on all versions since v15, this approach is preferable to making the ERROR nicer because we need to repair the pg_sequences view without modifying its definition on released versions. For consistency, this commit also modifies the function to return NULL for other sessions' temporary sequences. The pg_sequences view already appropriately filters out such sequences, so there's no bug there, but we might as well offer some defense in case someone invokes this function directly. Unlogged sequences were first introduced in v15, but temporary sequences are much older, so while the fix for unlogged sequences is only back-patched to v15, the temporary sequence portion is back-patched to all supported versions. We could also remove the privilege check in the pg_sequences view definition in v18 if we modify this function to return NULL for sequences for which the current user lacks privileges, but that is left as a future exercise for when v18 development begins. Reviewed-by: Tom Lane, Michael Paquier Discussion: https://postgr.es/m/20240501005730.GA594666%40nathanxps13 Backpatch-through: 12
* Fix recursive RECORD-returning plpython functions.Tom Lane2024-05-09
| | | | | | | | | | | | | | | | | | | | If we recursed to a new call of the same function, with a different coldeflist (AS clause), it would fail because the inner call would overwrite the outer call's idea of what to return. This is vaguely like 1d2fe56e4 and c5bec5426, but it's not due to any API decisions: it's just that we computed the actual output rowtype at the start of the call, and saved it in the per-procedure data structure. We can fix it at basically zero cost by doing the computation at the end of each call instead of the start. It's not clear that there's any real-world use-case for such a function, but given that it doesn't cost anything to fix, it'd be silly not to. Per report from Andreas Karlsson. Back-patch to all supported branches. Discussion: https://postgr.es/m/1651a46d-3c15-4028-a8c1-d74937b54e19@proxel.se
* Ensure that "pg_restore -l" reports dependent TOC entries correctly.Tom Lane2024-05-07
| | | | | | | | | | | | | | If -l was specified together with selective-restore options such as -n or -N, dependent TOC entries such as comments would be omitted from the listing, even when an actual restore would have selected them. This happened because PrintTOCSummary neglected to update the te->reqs marking of the entry they depended on. Per report from Justin Pryzby. This has been wrong since 0d4e6ed30 taught _tocEntryRequired to sometimes look at the "reqs" marking of other TOC entries, so back-patch to all supported branches. Discussion: https://postgr.es/m/ZjoeirG7yxODdC4P@pryzbyj2023
* Don't corrupt plpython's "TD" dictionary in a recursive trigger call.Tom Lane2024-05-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | If a plpython-language trigger caused another one to be invoked, the "TD" dictionary created for the inner one would overwrite the outer one's "TD" dictionary. This is more or less the same problem that 1d2fe56e4 fixed for ordinary functions in plpython, so fix it the same way, by saving and restoring "TD" during a recursive invocation. This fix makes an ABI-incompatible change in struct PLySavedArgs. I'm not too worried about that because it seems highly unlikely that any extension is messing with those structs. We could imagine doing something weird to preserve nominal ABI compatibility in the back branches, like keeping the saved TD object in an extra element of namedargs[]. However, that would only be very nominal compatibility: if anything *is* touching PLySavedArgs, it would likely do the wrong thing due to not knowing about the additional value. So I judge it not worth the ugliness to do something different there. (I also changed struct PLyProcedure, but its added field fits into formerly-padding space, so that should be safe.) Per bug #18456 from Jacques Combrink. This bug is very ancient, so back-patch to all supported branches. Discussion: https://postgr.es/m/3008982.1714853799@sss.pgh.pa.us
* Stamp 12.19.REL_12_19Tom Lane2024-05-06
|
* Translation updatesPeter Eisentraut2024-05-06
| | | | | Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 9a37846122eee9aa9c8f8d1cea1bbe7afb28796b
* Ensure we allocate NAMEDATALEN bytes for names in Index Only ScansDavid Rowley2024-05-01
| | | | | | | | | | | | | | | As an optimization, we store "name" columns as cstrings in btree indexes. Here we modify it so that Index Only Scans convert these cstrings back to names with NAMEDATALEN bytes rather than storing the cstring in the tuple slot, as was happening previously. Bug: #17855 Reported-by: Alexander Lakhin Reviewed-by: Alexander Lakhin, Tom Lane Discussion: https://postgr.es/m/17855-5f523e0f9769a566@postgresql.org Backpatch-through: 12, all supported versions
* Disallow converting a table to a view within an outer SQL command.Tom Lane2024-04-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We have long disallowed all forms of ALTER TABLE if the table is already opened by some outer SQL command in the same session. This has the same purpose as obtaining AccessExclusiveLock, but since a session's own locks don't conflict the lock only blocks use of the table by other sessions, not our own. Without this check, the ALTER might confuse the outer SQL command since any previous inspection of the table would potentially become invalid. However, the RelisBecomingView code path in DefineQueryRewrite never got that memo, and assumed that AccessExclusiveLock is sufficient for performing something morally equivalent to a rather invasive ALTER TABLE. Unsurprisingly, this can confuse an outer command that is trying to do something with the table. This was submitted as a security issue, but the security team has been unable to identify any consequence worse than a null pointer dereference (from trying to access rd_tableam methods that the relation no longer has). Therefore, in accordance with our usual policy, it's not security material and should just be fixed as a routine bug. Fix by disallowing the operation if the table is open locally, exactly as ALTER TABLE does it. Per an anonymous security researcher, via Bundesamt für Sicherheit in der Informationstechnik. Patch v12-v15 only. In v16 and later, we removed this code altogether (cf. commit b23cd185f), so that there's no issue.
* Close race condition between datfrozen and relfrozen updates.Noah Misch2024-04-29
| | | | | | | | | | | | | vac_update_datfrozenxid() did multiple loads of relfrozenxid and relminmxid from buffer memory, and it assumed each would get the same value. Not so if a concurrent vac_update_relstats() did an inplace update. Commit 2d2e40e3befd8b9e0d2757554537345b15fa6ea2 fixed the same kind of bug in vac_truncate_clog(). Today's bug could cause the rel-level field and XIDs in the rel's rows to precede the db-level field. A cluster having such values should VACUUM affected tables. Back-patch to v12 (all supported versions). Discussion: https://postgr.es/m/20240423003956.e7.nmisch@google.com
* Detect more overflows in timestamp[tz]_pl_interval.Tom Lane2024-04-28
| | | | | | | | | | | | | | | | | In commit 25cd2d640 I (tgl) opined that "The additions of the months and microseconds fields could also overflow, of course. However, I believe we need no additional checks there; the existing range checks should catch such cases". This is demonstrably wrong however for the microseconds field, and given that discovery it seems prudent to be paranoid about the months addition as well. Report and patch by Joseph Koshakow. As before, back-patch to all supported branches. (However, the test case doesn't work before v15 because we didn't allow wider-than-int32 numbers in interval literals. A variant test could probably be built that fits within that restriction, but it didn't seem worth the trouble.) Discussion: https://postgr.es/m/CAAvxfHf77sRHKoEzUw9_cMYSpbpNS2C+J_+8Dq4+0oi8iKopeA@mail.gmail.com
* Fix MSVC recipe for ecpg regression tests, redux.Tom Lane2024-04-19
| | | | | | | | Forgot to inject -DCMDLINESYM=123 ... Per buildfarm. Discussion: https://postgr.es/m/4cc4dc47-ca2b-4129-8784-db69b5f82777@dunslane.net
* Fix MSVC recipe for ecpg regression tests.Tom Lane2024-04-18
| | | | | | | | | | | While back-patching commit 6f0cef935, I forgot that the MSVC build scripts would also need adjustment in the back branches. This is a blind attempt at a fix, but it's basically copying nearby code so I think it will work. Per buildfarm (via Andrew Dunstan) Discussion: https://postgr.es/m/4cc4dc47-ca2b-4129-8784-db69b5f82777@dunslane.net
* Fix assorted bugs in ecpg's macro mechanism.Tom Lane2024-04-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The code associated with EXEC SQL DEFINE was unreadable and full of bugs, notably: * It'd attempt to free a non-malloced string if the ecpg program tries to redefine a macro that was defined on the command line. * Possible memory stomp if user writes "-D=foo". * Undef'ing or redefining a macro defined on the command line would change the state visible to the next file, when multiple files are specified on the command line. (While possibly that could have been an intentional choice, the code clearly intends to revert to the original macro state; it's just failing to consider this interaction.) * Missing "break" in defining a new macro meant that redefinition of an existing name would cause an extra entry to be added to the definition list. While not immediately harmful, a subsequent undef would result in the prior entry becoming visible again. * The interactions with input buffering are subtle and were entirely undocumented. It's not that surprising that we hadn't noticed these bugs, because there was no test coverage at all of either the -D command line switch or multiple input files. This patch adds such coverage (in a rather hacky way I guess). In addition to the code bugs, the user documentation was confused about whether the -D switch defines a C macro or an ecpg one, and it failed to mention that you can write "-Dsymbol=value". These problems are old, so back-patch to all supported branches. Discussion: https://postgr.es/m/998011.1713217712@sss.pgh.pa.us
* Fix generation of EC join conditions at the wrong plan level.Tom Lane2024-04-16
| | | | | | | | | | | | | | | | | | | | | | | get_baserel_parampathinfo previously assumed without checking that the results of generate_join_implied_equalities "necessarily satisfy join_clause_is_movable_into". This turns out to be wrong in the presence of outer joins, because the generated clauses could include Vars that mustn't be evaluated below a relevant outer join. That led to applying clauses at the wrong plan level and possibly getting incorrect query results. We must check each clause's nullable_relids, and really the right thing to do is test join_clause_is_movable_into. However, trying to fix it that way exposes an oversight in equivclass.c: it wasn't careful about marking join clauses for appendrel children with the correct clause_relids. That caused the modified get_baserel_parampathinfo code to reject some clauses it still needs to accept. (See parallel commit for HEAD/v16 for more commentary about that.) Per bug #18429 from Benoît Ryder. This misbehavior existed for a long time before commit 2489d76c4, so patch v12-v15 this way. Discussion: https://postgr.es/m/18429-8982d4a348cc86c6@postgresql.org
* Fix type-checking of RECORD-returning functions in FROM, redux.Tom Lane2024-04-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 2ed8f9a01 intended to institute a policy that if a RangeTblFunction has a coldeflist, then the function return type is certainly RECORD, and we should use the coldeflist as the source of truth about what the columns of the record type are. When the original function has been folded to a constant, inspection of the constant might give a different answer. This situation will lead to a tuple-type-mismatch error at execution, but up until that point we need to consistently believe the coldeflist, or we'll have problems from different bits of code reaching different conclusions. expandRTE didn't get that memo though, and would try to produce a tupdesc based on the constant in this situation, leading to an assertion failure. (Desultory testing suggests that non-assert builds often manage to give the expected error, although I also saw a "cache lookup failed for type 0" error, and it seems at least possible that a crash could happen.) Some other callers of get_expr_result_type and get_expr_result_tupdesc were also being incautious about this. While none of them seem to have actual bugs, they're working harder than necessary in this case, besides which it seems safest to have an explicit policy of not using those functions on an RTE with a coldeflist. Adjust the code accordingly, and add commentary to funcapi.c about this policy. Also fix an obsolete comment that claimed "get_expr_result_type() doesn't know how to extract type info from a RECORD constant". That hasn't been true since commit d57534740. Per bug #18422 from Alexander Lakhin. As with the previous commit, back-patch to all supported branches. Discussion: https://postgr.es/m/18422-89ca86c8eac5246d@postgresql.org
* Fix WaitEventSet resource leak in WaitLatchOrSocket().Etsuro Fujita2024-04-11
| | | | | | | | | | | | | | | This function would have the same issue we solved in commit 501cfd07d: If an error is thrown after calling CreateWaitEventSet(), the file descriptor (on epoll- or kqueue-based systems) or handles (on Windows) that the WaitEventSet contains are leaked. Like that commit, use PG_TRY-PG_FINALLY (PG_TRY-PG_CATCH in v12) to make sure the WaitEventSet is freed properly. Back-patch to all supported versions, but as we do not have this issue in HEAD (cf. commit 50c67c201), no need to apply this patch to it. Discussion: https://postgr.es/m/CAPmGK16MqdDoD8oatp8SQWaEa4vS3nfQqDN_Sj9YRuu5J3Lj9g%40mail.gmail.com
* Fix plpgsql's handling of -- comments following expressions.Tom Lane2024-04-10
| | | | | | | | | | | | | | | | | | | | | | | | | | Up to now, read_sql_construct() has collected all the source text from the statement or expression's initial token up to the character just before the "until" token. It normally tries to strip trailing whitespace from that, largely for neatness. If there was a "-- text" comment after the expression, this resulted in removing the newline that terminates the comment, which creates a hazard if we try to paste the collected text into a larger SQL construct without inserting a newline after it. In particular this caused our handling of CASE constructs to fail if there's a comment after a WHEN expression. Commit 4adead1d2 noticed a similar problem with cursor arguments, and worked around it through the rather crude hack of suppressing the whitespace-trimming behavior for those. Rather than do that and leave the hazard open for future hackers to trip over, let's fix it properly. pl_scanner.c already has enough infrastructure to report the end location of the expression's last token, so we can copy up to that location and never collect any trailing whitespace or comment to begin with. Erik Wienhold and Tom Lane, per report from Michal Bartak. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAAVzF_FjRoi8fOVuLCZhQJx6HATQ7MKm=aFOHWZODFnLmjX-xA@mail.gmail.com
* Fix illegal attribute propagation in LLVM JIT.Thomas Munro2024-04-10
| | | | | | | | | | | | | | | | | | | Commit 72559438 started copying more attributes from AttributeTemplate to the functions we generate on the fly. In the case of deform functions, which return void, this meant that "noundef", from AttributeTemplate's return value (a Datum) was copied to a void type. Older LLVM releases were OK with that, but LLVM 18 crashes. Update our llvm_copy_attributes() function to skip copying the attribute for the return value, if the target function returns void. Thanks to Dmitry Dolgov for help chasing this down. Back-patch to all supported releases, like 72559438. Reported-by: Pavel Stehule <pavel.stehule@gmail.com> Reviewed-by: Dmitry Dolgov <9erthalion6@gmail.com> Discussion: https://postgr.es/m/CAFj8pRACpVFr7LMdVYENUkScG5FCYMZDDdSGNU-tch%2Bw98OxYg%40mail.gmail.com
* simplehash: Free collisions array in SH_STATAndres Freund2024-04-07
| | | | | | | | | | | | While SH_STAT() is only used for debugging, the allocated array can be large, and therefore should be freed. It's unclear why coverity started warning now. Reported-by: Tom Lane <tgl@sss.pgh.pa.us> Reported-by: Coverity Discussion: https://postgr.es/m/3005248.1712538233@sss.pgh.pa.us Backpatch: 12-
* Don't clobber test exit code at cleanup in LDAP/Kerberors testsHeikki Linnakangas2024-04-07
| | | | | | | | | | If the test script die()d before running the first test, the whole test was interpreted as SKIPped rather than failed. The PostgreSQL::Cluster module got this right. Backpatch to all supported versions. Discussion: https://www.postgresql.org/message-id/fb898a70-3a88-4629-88e9-f2375020061d@iki.fi
* Improve check in LDAP test to find the OpenLDAP installationHeikki Linnakangas2024-04-07
| | | | | | | | | | | | | | | | | | | | | | | If the OpenLDAP installation directory is not found, set $setup to 0 so that the LDAP tests are skipped. The macOS checks were already doing that, but the checks on other OS's were not. While we're at it, improve the error message when the tests are skipped, to specify whether the OS is supported at all, or if we just didn't find the installation directory. This was accidentally "working" without this, i.e. we were skipping the tests if the OpenLDAP installation was not found, because of a bug in the LdapServer test module: the END block clobbered the exit code so if the script die()s before running the first subtest, the whole test script was marked as SKIPped. The next commit will fix that bug, but we need to fix the setup code first. These checks should probably go into configure/meson, but this is better than nothing and allows fixing the bug in the END block. Backpatch to all supported versions. Discussion: https://www.postgresql.org/message-id/fb898a70-3a88-4629-88e9-f2375020061d@iki.fi
* Fix ecpg's mechanism for detecting unsupported cases in the grammar.Tom Lane2024-04-04
| | | | | | | | | | | | | | | | | | | | | | | | | ecpg wants to emit a warning if it parses a SQL construct that the backend can parse but will immediately throw a FEATURE_NOT_SUPPORTED error for. The way it was testing for this was to see if the string ERRCODE_FEATURE_NOT_SUPPORTED appeared anywhere in the gram.y code. This is, of course, not nearly good enough, as there are plenty of rules in gram.y that throw that error only conditionally. There was a hack dating to 2008 to suppress the warning in one rule that doesn't even exist anymore, but nothing for other cases we've created since then. End result was that you could get "unsupported feature will be passed to server" warnings while compiling perfectly good SQL code in ecpg. Somehow we'd not heard complaints about this, but it was exposed by the recent addition of an ecpg test for a SQL/JSON construct. To fix, suppress the warning if the rule contains any "if" statement. Manual comparison of gram.y with the generated preproc.y file shows that the warning is now emitted only in rules where it's sensible. This problem has existed for a long time, so back-patch to all supported branches. Discussion: https://postgr.es/m/603615.1712245382@sss.pgh.pa.us
generated by cgit v1.2.3 (git 2.25.1) at 2025-06-24 16:38:46 +0000