aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
* 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 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 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
* Avoid deadlock during orphan temp table removal.Tom Lane2024-04-02
| | | | | | | | | | | | | | | | | | | | | | | If temp tables have dependencies (such as sequences) then it's possible for autovacuum's cleanup of orphan temp tables to deadlock against an incoming backend that's trying to clean out the temp namespace for its own use. That can happen because RemoveTempRelations' performDeletion call can visit objects within the namespace in an order different from the order in which a per-table deletion will visit them. To fix, observe that performDeletion will begin by taking an exclusive lock on the temp namespace (even though it won't actually delete it). So, if we can get a shared lock on the namespace, we can be sure we're not running concurrently with RemoveTempRelations, while also not conflicting with ordinary use of the namespace. This requires introducing a conditional version of LockDatabaseObject, but that's no big deal. (It's surprising we've got along without that this long.) Report and patch by Mikhail Zhilin. Back-patch to all supported branches. Discussion: https://postgr.es/m/c43ce028-2bc2-4865-9b89-3f706246eed5@postgrespro.ru
* Fix unnecessary use of moving-aggregate mode with non-moving frame.Tom Lane2024-03-27
| | | | | | | | | | | | | | | | | | | | When a plain aggregate is used as a window function, and the window frame start is specified as UNBOUNDED PRECEDING, the frame's head cannot move so we do not need to use moving-aggregate mode. The check for that was put into initialize_peragg(), failing to notice that ExecInitWindowAgg() calls that function before it's filled in winstate->frameOptions. Since makeNode() would have zeroed the field, this didn't provoke uninitialized-value complaints, nor would the erroneous decision have resulted in more than a little inefficiency. Still, it's wrong, so move the initialization of winstate->frameOptions earlier to make it work properly. While here, also fix a thinko in a comment. Both errors crept in in commit a9d9acbf2 which introduced the moving-aggregate mode. Spotted by Vallimaharajan G. Back-patch to all supported branches. Discussion: https://postgr.es/m/18e7f2a5167.fe36253866818.977923893562469143@zohocorp.com
* Fix failure of ALTER FOREIGN TABLE SET SCHEMA to move sequences.Tom Lane2024-03-26
| | | | | | | | | | | | | | | | | | Ordinary ALTER TABLE SET SCHEMA will also move any owned sequences into the new schema. We failed to do likewise for foreign tables, because AlterTableNamespaceInternal believed that only certain relkinds could have indexes, owned sequences, or constraints. We could simply add foreign tables to that relkind list, but it seems likely that the same oversight could be made again in future. Instead let's remove the relkind filter altogether. These functions shouldn't cost much when there are no objects that they need to process, and surely this isn't an especially performance-critical case anyway. Per bug #18407 from Vidushi Gupta. Back-patch to all supported branches. Discussion: https://postgr.es/m/18407-4fd07373d252c6a0@postgresql.org
* Allow "make check"-style testing to work with musl C library.Tom Lane2024-03-26
| | | | | | | | | | | | | | | | | | | | | The musl dynamic linker saves a pointer to the process' environment value of LD_LIBRARY_PATH very early in startup. When we move/clobber the environment to make more room for ps status strings, we clobber that value and thereby prevent libraries from being found via LD_LIBRARY_PATH, which breaks the use of a temporary installation for testing purposes. To fix, stop collecting usable space for ps status if we notice that the variable we are about to clobber is LD_LIBRARY_PATH. This will result in some reduction in how long the ps status can be, but it's only likely to occur in temporary test contexts, so it doesn't seem like a big problem. In any case, we don't have to do it if we see we are on glibc, which surely is where the majority of our Linux testing is done. Thomas Munro, Bruce Momjian, and Tom Lane, per report from Wolfgang Walther. Back-patch to all supported branches, with the hope that we'll set up a buildfarm animal to test on this platform. Discussion: https://postgr.es/m/fddd1cd6-dc16-40a2-9eb5-d7fef2101488@technowledgy.de
* Review wording on tablespaces w.r.t. partitioned tablesAlvaro Herrera2024-03-20
| | | | | | | | | | | Remove a redundant comment, and document pg_class.reltablespace properly in catalogs.sgml. After commits a36c84c3e4a9, 87259588d0ab and others. Backpatch to 12. Discussion: https://postgr.es/m/202403191013.w2kr7wqlamqz@alvherre.pgsql
* Fix EXPLAIN Bitmap heap scan to count pages with no visible tuplesHeikki Linnakangas2024-03-18
| | | | | | | | | | | | | | | | | Previously, bitmap heap scans only counted lossy and exact pages for explain when there was at least one visible tuple on the page. heapam_scan_bitmap_next_block() returned true only if there was a "valid" page with tuples to be processed. However, the lossy and exact page counters in EXPLAIN should count the number of pages represented in a lossy or non-lossy way in the constructed bitmap, regardless of whether or not the pages ultimately contained visible tuples. Backpatch to all supported versions. Author: Melanie Plageman Discussion: https://www.postgresql.org/message-id/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA@mail.gmail.com Discussion: https://www.postgresql.org/message-id/CAAKRu_bxrXeZ2rCnY8LyeC2Ls88KpjWrQ%2BopUrXDRXdcfwFZGA@mail.gmail.com
* Make INSERT-from-multiple-VALUES-rows handle domain target columns.Tom Lane2024-03-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | Commit a3c7a993d fixed some cases involving target columns that are arrays or composites by applying transformAssignedExpr to the VALUES entries, and then stripping off any assignment ArrayRefs or FieldStores that the transformation added. But I forgot about domains over arrays or composites :-(. Such cases would either fail with surprising complaints about mismatched datatypes, or insert unexpected coercions that could lead to odd results. To fix, extend the stripping logic to get rid of CoerceToDomain if it's atop an ArrayRef or FieldStore. While poking at this, I realized that there's a poorly documented and not-at-all-tested behavior nearby: we coerce each VALUES column to the domain type separately, and rely on the rewriter to merge those operations so that the domain constraints are checked only once. If that merging did not happen, it's entirely possible that we'd get unexpected domain constraint failures due to checking a partially-updated container value. There's no bug there, but while we're here let's improve the commentary about it and add some test cases that explicitly exercise that behavior. Per bug #18393 from Pablo Kharo. Back-patch to all supported branches. Discussion: https://postgr.es/m/18393-65fedb1a0de9260d@postgresql.org
* Fix confusion about the return rowtype of SQL-language procedures.Tom Lane2024-03-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | There is a very ancient hack in check_sql_fn_retval that allows a single SELECT targetlist entry of composite type to be taken as supplying all the output columns of a function returning composite. (This is grotty and fundamentally ambiguous, but it's really hard to do nested composite-returning functions without it.) As far as I know, that doesn't cause any problems in ordinary functions. It's disastrous for procedures however. All procedures that have any output parameters are labeled with prorettype RECORD, and the CALL code expects it will get back a record with one column per output parameter, regardless of whether any of those parameters is composite. Doing something else leads to an assertion failure or core dump. This is simple enough to fix: we just need to not apply that rule when considering procedures. However, that requires adding another argument to check_sql_fn_retval, which at least in principle might be getting called by external callers. Therefore, in the back branches convert check_sql_fn_retval into an ABI-preserving wrapper around a new function check_sql_fn_retval_ext. Per report from Yahor Yuzefovich. This has been broken since we implemented procedures, so back-patch to all supported branches. Discussion: https://postgr.es/m/CABz5gWHSjj2df6uG0NRiDhZ_Uz=Y8t0FJP-_SVSsRsnrQT76Gg@mail.gmail.com
* Disconnect if socket cannot be put into non-blocking modeHeikki Linnakangas2024-03-12
| | | | | | | | | | | | | | | | | | | | Commit 387da18874 moved the code to put socket into non-blocking mode from socket_set_nonblocking() into the one-time initialization function, pq_init(). In socket_set_nonblocking(), there indeed was a risk of recursion on failure like the comment said, but in pq_init(), ERROR or FATAL is fine. There's even another elog(FATAL) just after this, if setting FD_CLOEXEC fails. Note that COMMERROR merely logged the error, it did not close the connection, so if putting the socket to non-blocking mode failed we would use the connection anyway. You might not immediately notice, because most socket operations in a regular backend wait for the socket to become readable/writable anyway. But e.g. replication will be quite broken. Backpatch to all supported versions. Discussion: https://www.postgresql.org/message-id/d40a5cd0-2722-40c5-8755-12e9e811fa3c@iki.fi
* Backpatch missing check_stack_depth() to some recursive functionsAlexander Korotkov2024-03-11
| | | | | | | Backpatch changes from d57b7cc333, 75bcba6cbd to all supported branches per proposal of Egor Chindyaskin. Discussion: https://postgr.es/m/DE5FD776-A8CD-4378-BCFA-3BF30F1F6D60%40mail.ru
* Cope with a deficiency in OpenSSL 3.x's error reporting.Tom Lane2024-03-07
| | | | | | | | | | | | | | | | In OpenSSL 3.0.0 and later, ERR_reason_error_string randomly refuses to provide a string for error codes representing system errno values (e.g., "No such file or directory"). There is a poorly-documented way to extract the errno from the SSL error code in this case, so do that and apply strerror, rather than falling back to reporting the error code's numeric value as we were previously doing. Problem reported by David Zhang, although this is not his proposed patch; it's instead based on a suggestion from Heikki Linnakangas. Back-patch to all supported branches, since any of them are likely to be used with recent OpenSSL. Discussion: https://postgr.es/m/b6fb018b-f05c-4afd-abd3-318c649faf18@highgo.ca
* Revert "Fix parallel-safety check of expressions and predicate for index builds"Michael Paquier2024-03-07
| | | | | | | | | | | This reverts commit eae7be600be7, following a discussion with Tom Lane, due to concerns that this impacts the decisions made by the planner for the number of workers spawned based on the inlining and const-folding of index expressions and predicate for cases that would have worked until this commit. Discussion: https://postgr.es/m/162802.1709746091@sss.pgh.pa.us Backpatch-through: 12
* Fix type-checking of RECORD-returning functions in FROM.Tom Lane2024-03-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In the corner case where a function returning RECORD has been simplified to a RECORD constant or an inlined ROW() expression, ExecInitFunctionScan failed to cross-check the function's result rowtype against the coldeflist provided by the calling query. That happened because get_expr_result_type is able to extract a tupdesc from such expressions, which led ExecInitFunctionScan to ignore the coldeflist. (Instead, it used the extracted tupdesc to check the function's output, which of course always succeeds.) I have not been able to demonstrate any really serious consequences from this, because if some column of the result is of the wrong type and is directly referenced by a Var of the calling query, CheckVarSlotCompatibility will catch it. However, we definitely do fail to report the case where the function returns more columns than the coldeflist expects, and in the converse case where it returns fewer columns, we get an assert failure (but, seemingly, no worse results in non-assert builds). To fix, always build the expected tupdesc from the coldeflist if there is one, and consult get_expr_result_type only when there isn't one. Also remove the failing Assert, even though it is no longer reached after this fix. It doesn't seem to be adding anything useful, since later checking will deal with cases with the wrong number of columns. The only other place I could find that is doing something similar is inline_set_returning_function. There's no live bug there because we cannot be looking at a Const or RowExpr, but for consistency change that code to agree with ExecInitFunctionScan. Per report from PetSerAl. After some debate I've concluded that this should be back-patched. There is a small risk that somebody has been relying on such a case not throwing an error, but I judge this outweighed by the risk that I've missed some way in which the failure to cross-check has worse consequences than sketched above. Discussion: https://postgr.es/m/CAKygsHSerA1eXsJHR9wft3Gn3wfHQ5RfP8XHBzF70_qcrrRvEg@mail.gmail.com
* Fix parallel-safety check of expressions and predicate for index buildsMichael Paquier2024-03-06
| | | | | | | | | | | | | | | | | | | | | | | | As coded, the planner logic that calculates the number of parallel workers to use for a parallel index build uses expressions and predicates from the relcache, which are flattened for the planner by eval_const_expressions(). As reported in the bug, an immutable parallel-unsafe function flattened in the relcache would become a Const, which would be considered as parallel-safe, even if the predicate or the expressions including the function are not safe in parallel workers. Depending on the expressions or predicate used, this could cause the parallel build to fail. Tests are included that check parallel index builds with parallel-unsafe predicate and expressions. Two routines are added to lsyscache.h to be able to retrieve expressions and predicate of an index from its pg_index data. Reported-by: Alexander Lakhin Author: Tender Wang Reviewed-by: Jian He, Michael Paquier Discussion: https://postgr.es/m/CAHewXN=UaAaNn9ruHDH3Os8kxLVmtWqbssnf=dZN_s9=evHUFA@mail.gmail.com Backpatch-through: 12
* Fix incorrectly reported stats kind in "can't happen" ERRORDavid Rowley2024-03-05
| | | | | | | | | | The error message(s) were reporting the stats kind of 'f', which is not correct as that's for the "dependencies" statistics kind. Reported-by: Horst Reiterer Reviewed-by: Richard Guo Discussion: https://postgr.es/m/18375-ba99383eb9062d6a@postgresql.org Backpatch-through: 12, where MCV extended stats were added.
* Fix integer underflow in shared memory debuggingDaniel Gustafsson2024-02-29
| | | | | | | | | | | dsa_dump would print a large negative number instead of zero for segment bin 0. Fix by explicitly checking for underflow and add special case for bin 0. Backpatch to all supported versions. Author: Ian Ilyasov <ianilyasov@outlook.com> Reviewed-by: Robert Haas <robertmhaas@gmail.com> Discussion: https://postgr.es/m/GV1P251MB1004E0D09D117D3CECF9256ECD502@GV1P251MB1004.EURP251.PROD.OUTLOOK.COM Backpatch-through: v12
* Promote assertion about !ReindexIsProcessingIndex to runtime error.Tom Lane2024-02-25
| | | | | | | | | | | | | | | | | | | | | When this assertion was installed (in commit d2f60a3ab), I thought it was only for catching server logic errors that caused accesses to catalogs that were undergoing index rebuilds. However, it will also fire in case of a user-defined index expression that attempts to access its own table. We occasionally see reports of people trying to do that, and typically getting unintelligible low-level errors as a result. We can provide a more on-point message by making this a regular runtime check. While at it, adjust the similar error check in systable_beginscan_ordered to use the same message text. That one is (probably) not reachable without a coding bug, but we might as well use a translatable message if we have one. Per bug #18363 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/18363-e3598a5a572d0699@postgresql.org
* Avoid dangling-pointer problem with partitionwise joins under GEQO.Tom Lane2024-02-23
| | | | | | | | | | | | | | | | build_child_join_sjinfo creates a derived SpecialJoinInfo in the short-lived GEQO context, but afterwards the semi_rhs_exprs from that may be used in a UniquePath for a child base relation. This breaks the expectation that all base-relation-level structures are in the planning-lifespan context, leading to use of a dangling pointer with probable ensuing crash later on in create_unique_plan. To fix, copy the expression trees when making a UniquePath. Per bug #18360 from Alexander Lakhin. This has been broken since partitionwise joins were added, so back-patch to all supported branches. Discussion: https://postgr.es/m/18360-a23caf3157f34e62@postgresql.org
* Fix incorrect pruning of NULL partition for boolean IS NOT clausesDavid Rowley2024-02-20
| | | | | | | | | | | | | | | | | | | | | | | | Partition pruning wrongly assumed that, for a table partitioned on a boolean column, a clause in the form "boolcol IS NOT false" and "boolcol IS NOT true" could be inverted to correspondingly become "boolcol IS true" and "boolcol IS false". These are not equivalent as the NOT version matches the opposite boolean value *and* NULLs. This incorrect assumption meant that partition pruning pruned away partitions that could contain NULL values. Here we fix this by correctly not pruning partitions which could store NULLs. To be affected by this, the table must be partitioned by a NULLable boolean column and queries would have to contain "boolcol IS NOT false" or "boolcol IS NOT true". This could result in queries filtering out NULL values with a LIST partitioned table and "ERROR: invalid strategy number 0" for RANGE and HASH partitioned tables. Reported-by: Alexander Lakhin Bug: #18344 Discussion: https://postgr.es/m/18344-8d3f00bada6d09c6@postgresql.org Backpatch-through: 12
* Doc: improve a couple of comments in postgresql.conf.sample.Tom Lane2024-02-15
| | | | | | | | | Clarify comments associated with max_parallel_workers and related settings. Per bug #18343 from Christopher Kline. Discussion: https://postgr.es/m/18343-3a5e903d1d3692ab@postgresql.org
* Fix 'mmap' DSM implementation with allocations larger than 4 GBHeikki Linnakangas2024-02-13
| | | | | | Fixes bug #18341. Backpatch to all supported versions. Discussion: https://www.postgresql.org/message-id/18341-ce16599e7fd6228c@postgresql.org
* Revert "Skip .DS_Store files in server side utils"Daniel Gustafsson2024-02-13
| | | | | This reverts commit 76bb6dd2e56c14e947196e638f86982424c51254. Per failure reports from the buildfarm.
* Skip .DS_Store files in server side utilsDaniel Gustafsson2024-02-13
| | | | | | | | | | | | | | | | | | The macOS Finder application creates .DS_Store files in directories when opened, which creates problems for serverside utilities which expect all files to be PostgreSQL specific files. Skip these files when encountered in pg_checksums, pg_rewind and pg_basebackup. This was extracted from a larger patchset for skipping hidden files and system files, where the concencus was to just skip these. Since this is equally likely to happen in every version, backpatch to all supported versions. Reported-by: Mark Guertin <markguertin@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Tobias Bussmann <t.bussmann@gmx.net> Discussion: https://postgr.es/m/E258CE50-AB0E-455D-8AAD-BB4FE8F882FB@gmail.com Backpatch-through: v12
* Remove race condition in pg_get_expr().Tom Lane2024-02-09
| | | | | | | | | | | | | | | | | | | | | | | Since its introduction, pg_get_expr() has intended to silently return NULL if called with an invalid relation OID, as can happen when scanning the catalogs concurrently with relation drops. However, there is a race condition: we check validity of the OID at the start, but it could get dropped just afterward, leading to failures. This is the cause of some intermittent instability we're seeing in a proposed new test case, and presumably it's a hazard in the field as well. We can fix this by AccessShareLock-ing the target relation for the duration of pg_get_expr(). Since we don't require any permissions on the target relation, this is semantically a bit undesirable. But it turns out that the set_relation_column_names() subroutine already takes a transient AccessShareLock on that relation, and has done since commit 2ffa740be in 2012. Given the lack of complaints about that, it seems like there should be no harm in holding the lock a bit longer. Back-patch to all supported branches. Discussion: https://postgr.es/m/31ddcc01-a71b-4e8c-9948-01d1c47293ca@eisentraut.org
* Fix wrong logic in TransactionIdInRecentPast()Alexander Korotkov2024-02-09
| | | | | | | | | | | | | | | | | | | The TransactionIdInRecentPast() should return false for all the transactions older than TransamVariables->oldestClogXid. However, the function contains a bug in comparison FullTransactionId to TransactionID allowing full transactions between nextXid - 2^32 and oldestClogXid - 2^31. This commit fixes TransactionIdInRecentPast() by turning the oldestClogXid into FullTransactionId first, then performing the comparison. Backpatch to all supported versions. Reported-by: Egor Chindyaskin Bug: 18212 Discussion: https://postgr.es/m/18212-547307f8adf57262%40postgresql.org Author: Karina Litskevich Reviewed-by: Kyotaro Horiguchi Backpatch-through: 12
* Translation updatesPeter Eisentraut2024-02-05
| | | | | Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 25eaf29cbb9ee022c0e5f7a4dc4e217bc8a40dfb
* Fix assertion if index is dropped during REFRESH CONCURRENTLYHeikki Linnakangas2024-02-05
| | | | | | | | | | When assertions are disabled, the built SQL statement is invalid and you get a "syntax error". So this isn't a serious problem, but let's avoid the assertion failure. Backpatch to all supported versions. Reviewed-by: Noah Misch
* Run REFRESH MATERIALIZED VIEW CONCURRENTLY in right security contextHeikki Linnakangas2024-02-05
| | | | | | | | | | | | | | | | | | | | | | | | | The internal commands in REFRESH MATERIALIZED VIEW CONCURRENTLY are correctly executed in SECURITY_RESTRICTED_OPERATION mode, except for creating the temporary "diff" table, because you cannot create temporary tables in SRO mode. But creating the temporary "diff" table is a pretty complex CTAS command that selects from another temporary table created earlier in the command. If you can cajole that CTAS command to execute code defined by the table owner, the table owner can run code with the privileges of the user running the REFRESH command. The proof-of-concept reported to the security team relied on CREATE RULE to convert the internally-built temp table to a view. That's not possible since commit b23cd185fd, and I was not able to find a different way to turn the SELECT on the temp table into code execution, so as far as I know this is only exploitable in v15 and below. That's a fiddly assumption though, so apply this patch to master and all stable versions. Thanks to Pedro Gallegos for the report. Security: CVE-2023-5869 Reviewed-by: Noah Misch
* Translate ENOMEM to ERRCODE_OUT_OF_MEMORY in errcode_for_file_access().Tom Lane2024-02-02
| | | | | | | | | | Previously you got ERRCODE_INTERNAL_ERROR, which seems inappropriate, especially given that we're trying to avoid emitting that in reachable cases. Alexander Kuzmenkov Discussion: https://postgr.es/m/CALzhyqzgQph0BY8-hFRRGdHhF8CoqmmDHW9S=hMZ-HMzLxRqDQ@mail.gmail.com
* Apply band-aid fix for an oversight in reparameterize_path_by_child.Tom Lane2024-02-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The path we wish to reparameterize is not a standalone object: in particular, it implicitly references baserestrictinfo clauses in the associated RelOptInfo, and if it's a SampleScan path then there is also the TableSampleClause in the RTE to worry about. Both of those could contain lateral references to the join partner relation, which would need to be modified to refer to its child. Since we aren't doing that, affected queries can give wrong answers, or odd failures such as "variable not found in subplan target list", or executor crashes. But we can't just summarily modify those expressions, because they are shared with other paths for the rel. We'd break things if we modify them and then end up using some non-partitioned-join path. In HEAD, we plan to fix this by postponing reparameterization until create_plan(), when we know that those other paths are no longer of interest, and then adjusting those expressions along with the ones in the path itself. That seems like too big a change for stable branches however. In the back branches, let's just detect whether any troublesome lateral references actually exist in those expressions, and fail reparameterization if so. This will result in not performing a partitioned join in such cases. Given the lack of field complaints, nobody's likely to miss the optimization. Report and patch by Richard Guo. Apply to 12-16 only, since the intended fix for HEAD looks quite different. We're not quite ready to push the HEAD fix, but with back-branch releases coming up soon, it seems wise to get this stopgap fix in place there. Discussion: https://postgr.es/m/CAMbWs496+N=UAjOc=rcD3P7B6oJe4rZw08e_TZRUsWbPxZW3Tw@mail.gmail.com
* Fix various issues with ALTER TEXT SEARCH CONFIGURATIONMichael Paquier2024-01-31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit addresses a set of issues when changing token type mappings in a text search configuration when using duplicated token names: - ADD MAPPING would fail on insertion because of a constraint failure after inserting the same mapping. - ALTER MAPPING with an "overridden" configuration failed with "tuple already updated by self" when the token mappings are removed. - DROP MAPPING failed with "tuple already updated by self", like previously, but in a different code path. The code is refactored so the token names (with their numbers) are handled as a List with unique members rather than an array with numbers, ensuring that no duplicates mess up with the catalog inserts, updates and deletes. The list is generated by getTokenTypes(), with the same error handling as previously while duplicated tokens are discarded from the list used to work on the catalogs. Regression tests are expanded to cover much more ground for the cases fixed by this commit, as there was no coverage for the code touched in this commit. A bit more is done regarding the fact that a token name not supported by a configuration's parser should result in an error even if IF EXISTS is used in a DROP MAPPING clause. This is implied in the code but there was no coverage for that, and it was very easy to miss. These issues exist since at least their introduction in core with 140d4ebcb46e, so backpatch all the way down. Reported-by: Alexander Lakhin Author: Tender Wang, Michael Paquier Discussion: https://postgr.es/m/18310-1eb233c5908189c8@postgresql.org Backpatch-through: 12
* Doc: mention foreign keys can reference unique indexesDavid Rowley2024-01-30
| | | | | | | | | | | | | | | | We seem to have only documented a foreign key can reference the columns of a primary key or unique constraint. Here we adjust the documentation to mention columns in a non-partial unique index can be mentioned too. The header comment for transformFkeyCheckAttrs() also didn't mention unique indexes, so fix that too. In passing make that header comment reflect reality in the various other aspects where it deviated from it. Bug: 18295 Reported-by: Gilles PARC Author: Laurenz Albe, David Rowley Discussion: https://www.postgresql.org/message-id/18295-0ed0fac5c9f7b17b%40postgresql.org Backpatch-through: 12
* Fix incompatibilities with libxml2 >= 2.12.0.Tom Lane2024-01-29
| | | | | | | | | | | | | | | | | | | | | | libxml2 changed the required signature of error handler callbacks to make the passed xmlError struct "const". This is causing build failures on buildfarm member caiman, and no doubt will start showing up in the field quite soon. Add a version check to adjust the declaration of xml_errorHandler() according to LIBXML_VERSION. 2.12.x also produces deprecation warnings for contrib/xml2/xpath.c's assignment to xmlLoadExtDtdDefaultValue. I see no good reason for that to still be there, seeing that we disabled external DTDs (at a lower level) years ago for security reasons. Let's just remove it. Back-patch to all supported branches, since they might all get built with newer libxml2 once it gets a bit more popular. (The back branches produce another deprecation warning about xpath.c's use of xmlSubstituteEntitiesDefault(). We ought to consider whether to back-patch all or part of commit 65c5864d7 to silence that. It's less urgent though, since it won't break the buildfarm.) Discussion: https://postgr.es/m/1389505.1706382262@sss.pgh.pa.us
* Fix locking when fixing an incomplete split of a GIN internal pageHeikki Linnakangas2024-01-29
| | | | | | | | | | | | | | | | | ginFinishSplit() expects the caller to hold an exclusive lock on the buffer, but when finishing an earlier "leftover" incomplete split of an internal page, the caller held a shared lock. That caused an assertion failure in MarkBufferDirty(). Without assertions, it could lead to corruption if two backends tried to complete the split at the same time. On master, add a test case using the new injection point facility. Report and analysis by Fei Changhong. Backpatch the fix to all supported versions. Reviewed-by: Fei Changhong, Michael Paquier Discussion: https://www.postgresql.org/message-id/tencent_A3CE810F59132D8E230475A5F0F7A08C8307@qq.com
* Detect Julian-date overflow in timestamp[tz]_pl_interval.Tom Lane2024-01-26
| | | | | | | | | | | | | | | | | | | | We perform addition of the days field of an interval via arithmetic on the Julian-date representation of the timestamp's date. This step is subject to int32 overflow, and we also should not let the Julian date become very negative, for fear of weird results from j2date. (In the timestamptz case, allow a Julian date of -1 to pass, since it might convert back to zero after timezone rotation.) 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. The difficulty here is that j2date's magic modular arithmetic could produce something that looks like it's in-range. Per bug #18313 from Christian Maurer. This has been wrong for a long time, so back-patch to all supported branches. Discussion: https://postgr.es/m/18313-64d2c8952d81e84b@postgresql.org
* Track LLVM 18 changes.Thomas Munro2024-01-25
| | | | | | | | | | A function was given a newly standard name from C++20 in LLVM 16. Then LLVM 18 added a deprecation warning for the old name, and it is about to ship, so it's time to adjust that. Back-patch to all supported releases. Discussion: https://www.postgresql.org/message-id/CA+hUKGLbuVhH6mqS8z+FwAn4=5dHs0bAWmEMZ3B+iYHWKC4-ZA@mail.gmail.com
* Fix ALTER TABLE .. ADD COLUMN with complex inheritance treesMichael Paquier2024-01-24
| | | | | | | | | | | | | | | | This command, when used to add a column on a parent table with a complex inheritance tree, tried to update multiple times the same tuple in pg_attribute for a child table when incrementing attinhcount, causing failures with "tuple already updated by self" because of a missing CommandCounterIncrement() between two updates. This exists for a rather long time, so backpatch all the way down. Reported-by: Alexander Lakhin Author: Tender Wang Reviewed-by: Richard Guo Discussion: https://postgr.es/m/18297-b04cd83a55b51e35@postgresql.org Backpatch-through: 12
* lwlock: Fix quadratic behavior with very long wait listsAndres Freund2024-01-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Until now LWLockDequeueSelf() sequentially searched the list of waiters to see if the current proc is still is on the list of waiters, or has already been removed. In extreme workloads, where the wait lists are very long, this leads to a quadratic behavior. #backends iterating over a list #backends long. Additionally, the likelihood of needing to call LWLockDequeueSelf() in the first place also increases with the increased length of the wait queue, as it becomes more likely that a lock is released while waiting for the wait list lock, which is held for longer during lock release. Due to the exponential back-off in perform_spin_delay() this is surprisingly hard to detect. We should make that easier, e.g. by adding a wait event around the pg_usleep() - but that's a separate patch. The fix is simple - track whether a proc is currently waiting in the wait list or already removed but waiting to be woken up in PGPROC->lwWaiting. In some workloads with a lot of clients contending for a small number of lwlocks (e.g. WALWriteLock), the fix can substantially increase throughput. This has been originally fixed for 16~ with a4adc31f6902 without a backpatch, and we have heard complaints from users impacted by this quadratic behavior in older versions as well. Author: Andres Freund <andres@anarazel.de> Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Discussion: https://postgr.es/m/20221027165914.2hofzp4cvutj6gin@awork3.anarazel.de Discussion: https://postgr.es/m/CALj2ACXktNbG=K8Xi7PSqbofTZozavhaxjatVc14iYaLu4Maag@mail.gmail.com Backpatch-through: 12
* Close socket in case of errors in setting non-blockingDaniel Gustafsson2024-01-17
| | | | | | | | | | | | | If configuring the newly created socket non-blocking fails we error out and return INVALID_SOCKET, but the socket that had been created wasn't closed. Fix by issuing closesocket in the errorpath. Backpatch to all supported branches. Author: Ranier Vilela <ranier.vf@gmail.com> Discussion: https://postgr.es/m/CAEudQApmU5CrKefH85VbNYE2y8H=-qqEJbg6RAPU65+vCe+89A@mail.gmail.com Backpatch-through: v12
* Re-pgindent catcache.c after previous commit.Tom Lane2024-01-13
| | | | | Discussion: https://postgr.es/m/1393953.1698353013@sss.pgh.pa.us Discussion: https://postgr.es/m/CAGjhLkOoBEC9mLsnB42d3CO1vcMx71MLSEuigeABbQ8oRdA6gw@mail.gmail.com
* Cope with catcache entries becoming stale during detoasting.Tom Lane2024-01-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We've long had a policy that any toasted fields in a catalog tuple should be pulled in-line before entering the tuple in a catalog cache. However, that requires access to the catalog's toast table, and we'll typically do AcceptInvalidationMessages while opening the toast table. So it's possible that the catalog tuple is outdated by the time we finish detoasting it. Since no cache entry exists yet, we can't mark the entry stale during AcceptInvalidationMessages, and instead we'll press forward and build an apparently-valid cache entry. The upshot is that we have a race condition whereby an out-of-date entry could be made in a backend's catalog cache, and persist there indefinitely causing indeterminate misbehavior. To fix, use the existing systable_recheck_tuple code to recheck whether the catalog tuple is still up-to-date after we finish detoasting it. If not, loop around and restart the process of searching the catalog and constructing cache entries from the top. The case is rare enough that this shouldn't create any meaningful performance penalty, even in the SearchCatCacheList case where we need to tear down and reconstruct the whole list. Indeed, the case is so rare that AFAICT it doesn't occur during our regression tests, and there doesn't seem to be any easy way to build a test that would exercise it reliably. To allow testing of the retry code paths, add logic (in USE_ASSERT_CHECKING builds only) that randomly pretends that the recheck failed about one time out of a thousand. This is enough to ensure that we'll pass through the retry paths during most regression test runs. By adding an extra level of looping, this commit creates a need to reindent most of SearchCatCacheMiss and SearchCatCacheList. I'll do that separately, to allow putting those changes in .git-blame-ignore-revs. Patch by me; thanks to Alexander Lakhin for having built a test case to prove the bug is real, and to Xiaoran Wang for review. Back-patch to all supported branches. Discussion: https://postgr.es/m/1393953.1698353013@sss.pgh.pa.us Discussion: https://postgr.es/m/CAGjhLkOoBEC9mLsnB42d3CO1vcMx71MLSEuigeABbQ8oRdA6gw@mail.gmail.com
* Allow subquery pullup to wrap a PlaceHolderVar in another one.Tom Lane2024-01-11
| | | | | | | | | | | | | | | | | | | | | | | The code for wrapping subquery output expressions in PlaceHolderVars believed that if the expression already was a PlaceHolderVar, it was never necessary to wrap that in another one. That's wrong if the expression is underneath an outer join and involves a lateral reference to outside that scope: failing to add an additional PHV risks evaluating the expression at the wrong place and hence not forcing it to null when the outer join should do so. This is an oversight in commit 9e7e29c75, which added logic to forcibly wrap lateral-reference Vars in PlaceHolderVars, but didn't see that the adjacent case for PlaceHolderVars needed the same treatment. The test case we have for this doesn't fail before 4be058fe9, but now that I see the problem I wonder if it is possible to demonstrate related errors before that. That's moot though, since all such branches are out of support. Per bug #18284 from Holger Reise. Back-patch to all supported branches. Discussion: https://postgr.es/m/18284-47505a20c23647f8@postgresql.org
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-04 14:23:42 +0000