aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
...
* Fix handling of CREATE DOMAIN with GENERATED constraint syntaxPeter Eisentraut2024-12-03
| | | | | | | | | | | | | | | | | | | | Stuff like CREATE DOMAIN foo AS int CONSTRAINT cc GENERATED ALWAYS AS (2) STORED is not supported for domains, but the parser allows it, because it's the same syntax as for table constraints. But CreateDomain() did not explicitly handle all ConstrType values, so the above would get an internal error like ERROR: unrecognized constraint subtype: 4 Fix that by providing a user-facing error message for all ConstrType values. Also, remove the switch default case, so future additions to ConstrType are caught. Reported-by: Jian He <jian.universality@gmail.com> Discussion: https://www.postgresql.org/message-id/CACJufxF8fmM=Dbm4pDFuV_nKGz2-No0k4YifhrF3-rjXTWJM3w@mail.gmail.com
* Fix temporary memory leak in system table index scansPeter Eisentraut2024-12-03
| | | | | | | | | | | | | | | | Commit 811af9786b introduced palloc() calls into systable_beginscan() and systable_beginscan_ordered(). But there was no pfree(), as is the usual style. It turns out that an ANALYZE of a partitioned table can invoke many thousand system table index scans, and this memory is not cleaned up until the end of the command, so this can temporarily leak quite a bit of memory. Maybe there are improvements to be made at a higher level about this, but for now, insert a couple of corresponding pfree() calls to fix this particular issue. Reported-by: Justin Pryzby <pryzby@telsasoft.com> Discussion: https://www.postgresql.org/message-id/Z0XTfIq5xUtbkiIh@pryzbyj2023
* Perform provider-specific initialization in new functions.Jeff Davis2024-12-02
| | | | | Reviewed-by: Andreas Karlsson Discussion: https://postgr.es/m/4548a168-62cd-457b-8d06-9ba7b985c477@proxel.se
* Fix unintentional behavior change in commit e9931bfb75.Jeff Davis2024-12-02
| | | | | | | | | Prior to that commit, there was special case to use ASCII case mapping behavior for the libc provider with a single-byte encoding when that's the default collation. Commit e9931bfb75 mistakenly eliminated that special case; this commit restores it. Discussion: https://postgr.es/m/01a104f0d2179d756261e90d96fd65c36ad6fcf0.camel@j-davis.com
* Revert "Introduce CompactAttribute array in TupleDesc"David Rowley2024-12-03
| | | | | | | | | | This reverts commit d28dff3f6cd6a7562fb2c211ac0fb74a33ffd032. Quite a large number of buildfarm members didn't like this commit and it's not yet clear why. Reverting this before too many animals turn red. Discussion: https://postgr.es/m/CAApHDvr9i6T5=iAwQCxFDgMsthr_obVxgwBaEJkC8KUH6yM3Hw@mail.gmail.com
* Introduce CompactAttribute array in TupleDescDavid Rowley2024-12-03
| | | | | | | | | | | | | | | | | | | | | | | | | | The new compact_attrs array stores a few select fields from FormData_pg_attribute in a more compact way, using only 16 bytes per column instead of the 104 bytes that FormData_pg_attribute uses. Using CompactAttribute allows performance-critical operations such as tuple deformation to be performed without looking at the FormData_pg_attribute element in TupleDesc which means fewer cacheline accesses. With this change, NAMEDATALEN could be increased with a much smaller negative impact on performance. For some workloads, tuple deformation can be the most CPU intensive part of processing the query. Some testing with 16 columns on a table where the first column is variable length showed around a 10% increase in transactions per second for an OLAP type query performing aggregation on the 16th column. However, in certain cases, the increases were much higher, up to ~25% on one AMD Zen4 machine. This also makes pg_attribute.attcacheoff redundant. A follow-on commit will remove it, thus shrinking the FormData_pg_attribute struct by 4 bytes. Author: David Rowley Discussion: https://postgr.es/m/CAApHDvrBztXP3yx=NKNmo3xwFAFhEdyPnvrDg3=M0RhDs+4vYw@mail.gmail.com Reviewed-by: Andres Freund, Victor Yegorov
* RelationTruncate() must set DELAY_CHKPT_START.Thomas Munro2024-12-03
| | | | | | | | | | | | | | | | | | | | | | Previously, it set only DELAY_CHKPT_COMPLETE. That was important, because it meant that if the XLOG_SMGR_TRUNCATE record preceded a XLOG_CHECKPOINT_ONLINE record in the WAL, then the truncation would also happen on disk before the XLOG_CHECKPOINT_ONLINE record was written. However, it didn't guarantee that the sync request for the truncation was processed before the XLOG_CHECKPOINT_ONLINE record was written. By setting DELAY_CHKPT_START, we guarantee that if an XLOG_SMGR_TRUNCATE record is written to WAL before the redo pointer of a concurrent checkpoint, the sync request queued by that operation must be processed by that checkpoint, rather than being left for the following one. This is a refinement of commit 412ad7a5563. Back-patch to all supported releases, like that commit. Author: Robert Haas <robertmhaas@gmail.com> Reported-by: Thomas Munro <thomas.munro@gmail.com> Discussion: https://postgr.es/m/CA%2BhUKG%2B-2rjGZC2kwqr2NMLBcEBp4uf59QT1advbWYF_uc%2B0Aw%40mail.gmail.com
* Deprecate MD5 passwords.Nathan Bossart2024-12-02
| | | | | | | | | | | | | | | | | | | MD5 has been considered to be unsuitable for use as a cryptographic hash algorithm for some time. Furthermore, MD5 password hashes in PostgreSQL are vulnerable to pass-the-hash attacks, i.e., knowing the username and hashed password is sufficient to authenticate. The SCRAM-SHA-256 method added in v10 is not subject to these problems and is considered to be superior to MD5. This commit marks MD5 password support in PostgreSQL as deprecated and to be removed in a future release. The documentation now contains several deprecation notices, and CREATE ROLE and ALTER ROLE now emit deprecation warnings when setting MD5 passwords. The warnings can be disabled by setting the md5_password_warnings parameter to "off". Reviewed-by: Greg Sabino Mullane, Jim Nasby Discussion: https://postgr.es/m/ZwbfpJJol7lDWajL%40nathan
* Add a planner support function for numeric generate_series().Dean Rasheed2024-12-02
| | | | | | | | | | | This allows the planner to estimate the number of rows returned by generate_series(numeric, numeric[, numeric]), when the input values can be estimated at plan time. Song Jinzhou, reviewed by Dean Rasheed and David Rowley. Discussion: https://postgr.es/m/tencent_F43E7F4DD50EF5986D1051DE8DE547910206%40qq.com Discussion: https://postgr.es/m/tencent_1F6D5B9A1545E02FD7D0EE508DFD056DE50A%40qq.com
* Fix #include order in timestamp.c.Dean Rasheed2024-12-02
| | | | Oversight in 036bdcec9f.
* Fix error code for referential action RESTRICTPeter Eisentraut2024-12-02
| | | | | | | | | | | | According to the SQL standard, if the referential action RESTRICT is triggered, it has its own error code. We previously didn't use that, we just used the error code for foreign key violation. But RESTRICT is not necessarily an actual foreign key violation. The foreign key might still be satisfied in theory afterwards, but the RESTRICT setting prevents the action even then. So it's a separate kind of error condition. Discussion: https://www.postgresql.org/message-id/ea5b2777-266a-46fa-852f-6fca6ec480ad@eisentraut.org
* Avoid mislabeling of lateral references, redux.Tom Lane2024-11-30
| | | | | | | | | | | | | | | | | | As I'd feared, commit 5c9d8636d was still a few bricks shy of a load. We can't just leave pulled-up lateral-reference Vars with no new nullingrels: we have to carefully compute what subset of the to-be-replaced Var's nullingrels apply to them, else we still get "wrong varnullingrels" errors. This is a bit tedious, but it looks like we can use the nullingrel data this patch computes for other purposes, enabling better optimization. We don't want to inject unnecessary plan changes into stable branches though, so leave that idea for a later HEAD-only patch. Patch by me, but thanks to Richard Guo for devising a test case that broke 5c9d8636d, and for preliminary investigation about how to fix it. As before, back-patch to v16. Discussion: https://postgr.es/m/E1tGn4j-0003zi-MP@gemulon.postgresql.org
* Small indenting fixes in jsonpath_scan.lPeter Eisentraut2024-11-29
| | | | | | Some lines were indented by an inconsistent number of spaces. While we're here, also fix some code that used the newline after left parenthesis style, which is obsolete.
* Skip not SOAP-supported indexes while transforming an OR clause into SAOPAlexander Korotkov2024-11-29
| | | | | | | | | | | | There is no point in transforming OR-clauses into SAOP's if the target index doesn't support SAOP scans anyway. This commit adds corresponding checks to match_orclause_to_indexcol() and group_similar_or_args(). The first check fixes the actual bug, while the second just saves some cycles. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/8174de69-9e1a-0827-0e81-ef97f56a5939%40gmail.com Author: Alena Rybakina Reviewed-by: Ranier Vilela, Alexander Korotkov, Andrei Lepikhov
* Fix typo in header comment for set_operation_ordered_results_usefulDavid Rowley2024-11-29
| | | | | Reported-by: Richard Guo Discussion: https://postgr.es/m/CAMbWs492vMy3XNjDZRtqtHfFTK6HVeDwhrEQH7eXGgF_h5Jnzw@mail.gmail.com
* Avoid mislabeling of lateral references when pulling up a subquery.Tom Lane2024-11-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | If we are pulling up a subquery that's under an outer join, and the subquery's target list contains a strict expression that uses both a subquery variable and a lateral-reference variable, it's okay to pull up the expression without wrapping it in a PlaceHolderVar. That's safe because if the subquery variable is forced to NULL by the outer join, the expression result will come out as NULL too, so we don't have to force that outcome by evaluating the expression below the outer join. It'd be correct to wrap in a PHV, but that can lead to very significantly worse plans, since we'd then have to use a nestloop plan to pass down the lateral reference to where the expression will be evaluated. However, when we do that, we should not mark the lateral reference variable as being nulled by the outer join, because it isn't after we pull up the expression in this way. So the marking logic added by cb8e50a4a was incorrect in this detail, leading to "wrong varnullingrels" errors from the consistency-checking logic in setrefs.c. It seems to be sufficient to just not mark lateral references at all in this case. (I have a nagging feeling that more complexity may be needed in cases where there are several levels of outer join, but some attempts to break it with that didn't succeed.) Per report from Bertrand Mamasam. Back-patch to v16, as the previous patch was. Discussion: https://postgr.es/m/CACZ67_UA_EVrqiFXJu9XK50baEpH=ofEPJswa2kFxg6xuSw-ww@mail.gmail.com
* Remove useless casts to (void *)Peter Eisentraut2024-11-28
| | | | | | | | Many of them just seem to have been copied around for no real reason. Their presence causes (small) risks of hiding actual type mismatches or silently discarding qualifiers Discussion: https://www.postgresql.org/message-id/flat/461ea37c-8b58-43b4-9736-52884e862820@eisentraut.org
* Require sizeof(bool) == 1.Thomas Munro2024-11-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The C standard says that sizeof(bool) is implementation-defined, but we know of no current systems where it is not 1. The last known systems seem to have been Apple macOS/PowerPC 10.5 and Microsoft Visual C++ 4, both long defunct. PostgreSQL has always required sizeof(bool) == 1 for the definition of bool that it used, but previously it would define its own type if the system-provided bool had a different size. That was liable to cause memory layout problems when interacting with system and third-party libraries on (by now hypothetical) computers with wider _Bool, and now C23 has introduced a new problem by making bool a built-in datatype (like C++), so the fallback code doesn't even compile. We could probably work around that, but then we'd be writing new untested code for a computer that doesn't exist. Instead, delete the unreachable and C23-uncompilable fallback code, and let existing static assertions fail if the system-provided bool is too wide. If we ever get a problem report from a real system, then it will be time to figure out what to do about it in a way that also works on modern compilers. Note on C++: Previously we avoided including <stdbool.h> or trying to define a new bool type in headers that might be included by C++ code. These days we might as well just include <stdbool.h> unconditionally: it should be visible to C++11 but do nothing, just as in C23. We already include <stdint.h> without C++ guards in c.h, and that falls under the same C99-compatibility section of the C++11 standard as <stdbool.h>, so let's remove the guards here too. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/3198438.1731895163%40sss.pgh.pa.us
* Make GUC_check_errdetail messages full sentencesÁlvaro Herrera2024-11-27
| | | | | | | They were all missing punctuation, one was missing initial capital. Per our message style guidelines. No backpatch, to avoid breaking existing translations.
* Remove redundant relam initializationÁlvaro Herrera2024-11-27
| | | | | | | | This struct member is initialized again a few lines below in the same function. This is cosmetic, so no backpatch. Reported-by: Jingtang Zhang <mrdrivingduck@gmail.com> Discussion: https://postgr.es/m/AFF74506-B925-46BB-B875-CF5A946170EB@gmail.com
* Look up backend type in pg_signal_backend() more cheaply.Nathan Bossart2024-11-27
| | | | | | | | | | | | | | | | | | Commit ccd38024bc, which introduced the pg_signal_autovacuum_worker role, added a call to pgstat_get_beentry_by_proc_number() for the purpose of determining whether the process is an autovacuum worker. This function calls pgstat_read_current_status(), which can be fairly expensive and may return cached, out-of-date information. Since we just need to look up the target backend's BackendType, and we already know its ProcNumber, we can instead inspect the BackendStatusArray directly, which is much less expensive and possibly more up-to-date. There are some caveats with this approach (which are documented in the code), but it's still substantially better than before. Reported-by: Andres Freund Reviewed-by: Andres Freund Discussion: https://postgr.es/m/ujenaa2uabzfkwxwmfifawzdozh3ljr7geozlhftsuosgm7n7q%40g3utqqyyosb6
* postmaster: Reduce verbosity of environment dump debug messageAndres Freund2024-11-27
| | | | | | | | | | | | | Emitting each variable separately is unnecessarily verbose / hard to skim over. Emit the whole thing in one ereport() to address that. Also remove program name and function reference from the message. The former doesn't seem particularly helpful and the latter is provided by the elog.c infrastructure these days. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Discussion: https://postgr.es/m/leouteo5ozcrux3fepuhtbp6c56tbfd4naxeokidbx7m75cabz@hhw6g4urlowt
* Require ucrt if using MinGW.Thomas Munro2024-11-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Historically we tolerated the absence of various C runtime library features for the benefit of the MinGW tool chain, because it used ancient msvcrt.dll for a long period of time. It now uses ucrt by default (like Windows 10+, Visual Studio 2015+), and that's the only configuration we're testing. In practice, we effectively required ucrt already in PostgreSQL 17, when commit 8d9a9f03 required _create_locale etc, first available in msvcr120.dll (Visual Studio 2013, the last of the pre-ucrt series of runtimes), and for MinGW users that practically meant ucrt because it was difficult or impossible to use msvcr120.dll. That may even not have been the first such case, but old MinGW configurations had already dropped off our testing radar so we weren't paying much attention. This commit formalizes the requirement. It also removes a couple of obsolete comments that discussed msvcrt.dll limitations, and some tests of !defined(_MSC_VER) to imply msvcrt.dll. There are many more anachronisms, but it'll take some time to figure out how to remove them all. APIs affected relate to locales, UTF-8, threads, large files and more. Thanks to Peter Eisentraut for the documentation change. It's not really necessary to talk about ucrt explicitly in such a short section, since it's the default for MinGW-w64 and MSYS2. It's enough to prune references and broken links to much older tools. Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Discussion: https://postgr.es/m/d9e7731c-ca1b-477c-9298-fa51e135574a%40eisentraut.org
* Improve slightly misleading internal error messagePeter Eisentraut2024-11-27
| | | | | | | | The error message was talking about RowCompareType but was actually checking strategy numbers. While those are closely related, it is better to be accurate. Discussion: https://www.postgresql.org/message-id/flat/E72EAA49-354D-4C2E-8EB9-255197F55330@enterprisedb.com
* Support LIKE with nondeterministic collationsPeter Eisentraut2024-11-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This allows for example using LIKE with case-insensitive collations. There was previously no internal implementation of this, so it was met with a not-supported error. This adds the internal implementation and removes the error. The implementation follows the specification of the SQL standard for this. Unlike with deterministic collations, the LIKE matching cannot go character by character but has to go substring by substring. For example, if we are matching against LIKE 'foo%bar', we can't start by looking for an 'f', then an 'o', but instead with have to find something that matches 'foo'. This is because the collation could consider substrings of different lengths to be equal. This is all internal to MatchText() in like_match.c. The changes in GenericMatchText() in like.c just pass through the locale information to MatchText(), which was previously not needed. This matches exactly Generic_Text_IC_like() below. ILIKE is not affected. (It's unclear whether ILIKE makes sense under nondeterministic collations.) This also updates match_pattern_prefix() in like_support.c to support optimizing the case of an exact pattern with nondeterministic collations. This was already alluded to in the previous code. (includes documentation examples from Daniel Vérité and test cases from Paul A Jungwirth) Reviewed-by: Jian He <jian.universality@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/700d2e86-bf75-4607-9cf2-f5b7802f6e88@eisentraut.org
* Improve error message for replication of generated columns.Amit Kapila2024-11-27
| | | | | | | | | | | | | Currently, logical replication produces a generic error message when targeting a subscriber-side table column that is either missing or generated. The error message can be misleading for generated columns. This patch introduces a specific error message to clarify the issue when generated columns are involved. Author: Shubham Khanna Reviewed-by: Peter Smith, Vignesh C, Amit Kapila Discussion: https://postgr.es/m/CAHv8RjJBvYtqU7OAofBizOmQOK2Q8h+w9v2_cQWxT_gO7er3Aw@mail.gmail.com
* Handle better implicit transaction state of pipeline modeMichael Paquier2024-11-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When using a pipeline, a transaction starts from the first command and is committed with a Sync message or when the pipeline ends. Functions like IsInTransactionBlock() or PreventInTransactionBlock() were already able to understand a pipeline as being in a transaction block, but it was not the case of CheckTransactionBlock(). This function is called for example to generate a WARNING for SET LOCAL, complaining that it is used outside of a transaction block. The current state of the code caused multiple problems, like: - SET LOCAL executed at any stage of a pipeline issued a WARNING, even if the command was at least second in line where the pipeline is in a transaction state. - LOCK TABLE failed when invoked at any step of a pipeline, even if it should be able to work within a transaction block. The pipeline protocol assumes that the first command of a pipeline is not part of a transaction block, and that any follow-up commands is considered as within a transaction block. This commit changes the backend so as an implicit transaction block is started each time the first Execute message of a pipeline has finished processing, with this implicit transaction block ended once a sync is processed. The checks based on XACT_FLAGS_PIPELINING in the routines checking if we are in a transaction block are not necessary: it is enough to rely on the existing ones. Some tests are added to pgbench, that can be backpatched down to v17 when \syncpipeline is involved and down to v14 where \startpipeline and \endpipeline are available. This is unfortunately limited regarding the error patterns that can be checked, but it provides coverage for various pipeline combinations to check if these succeed or fail. These tests are able to capture the case of SET LOCAL's WARNING. The author has proposed a different feature to improve the coverage by adding similar meta-commands to psql where error messages could be checked, something more useful for the cases where commands cannot be used in transaction blocks, like REINDEX CONCURRENTLY or VACUUM. This is considered as future work for v18~. Author: Anthonin Bonnefoy Reviewed-by: Jelte Fennema-Nio, Michael Paquier Discussion: https://postgr.es/m/CAO6_XqrWO8uNBQrSu5r6jh+vTGi5Oiyk4y8yXDORdE2jbzw8xw@mail.gmail.com Backpatch-through: 13
* Distinguish between AcquireExternalFD and epoll_create1 / kqueue failingAndres Freund2024-11-26
| | | | | | | | | | | | | | The error messages in CreateWaitEventSet() made it hard to know whether the syscall or AcquireExternalFD() failed. This is particularly relevant because AcquireExternalFD() imposes a lower limit than what would cause syscalls fail with EMFILE. I did not change the message in libpqsrv_connect_prepare(), which is the one other use of AcquireExternalFD() in our codebase, as the error message already is less ambiguous. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/xjjx7r4xa7beixuu4qtkdhnwdbchrrpo3gaeb3jsbinvvdiat5@cwjw55mna5of
* Clean up newlines following left parenthesesÁlvaro Herrera2024-11-26
| | | | | | | | Most came in during the 17 cycle, so backpatch there. Some (particularly reorderbuffer.h) are very old, but backpatching doesn't seem useful. Like commits c9d297751959, c4f113e8fef9.
* Improve InitShmemAccess() prototypePeter Eisentraut2024-11-26
| | | | | | | | | The code comment said, 'the argument should be declared "PGShmemHeader *seghdr", but we use void to avoid having to include ipc.h in shmem.h.' We can achieve the original goal with a struct forward declaration. (ipc.h was also not the correct header file.) Discussion: https://www.postgresql.org/message-id/flat/cnthxg2eekacrejyeonuhiaezc7vd7o2uowlsbenxqfkjwgvwj@qgzu6eoqrglb
* Remove dead code in get_param_path_clause_serials()Richard Guo2024-11-26
| | | | | | | | | | | | | | | | The function get_param_path_clause_serials() is used to get the set of pushed-down clauses enforced within a parameterized Path. Since we don't currently support parameterized MergeAppend paths, and it doesn't look like that is going to change anytime soon (as explained in the comments for generate_orderedappend_paths), we don't need to consider MergeAppendPath in this function. This change won't make any measurable difference in performance; it's just for clarity's sake. Author: Richard Guo Reviewed-by: Andrei Lepikhov Discussion: https://postgr.es/m/CAMbWs4_Puie4DQ2ODvjQB_3CxYkUODnrJm8jn_ObMAcrjYNW7Q@mail.gmail.com
* Reordering DISTINCT keys to match input path's pathkeysRichard Guo2024-11-26
| | | | | | | | | | | | | | | | | | | | | | | | The ordering of DISTINCT items is semantically insignificant, so we can reorder them as needed. In fact, in the parser, we absorb the sorting semantics of the sortClause as much as possible into the distinctClause, ensuring that one clause is a prefix of the other. This can help avoid a possible need to re-sort. In this commit, we attempt to adjust the DISTINCT keys to match the input path's pathkeys. This can likewise help avoid re-sorting, or allow us to use incremental-sort to save efforts. For DISTINCT ON expressions, the parser already ensures that they match the initial ORDER BY expressions. When reordering the DISTINCT keys, we must ensure that the resulting pathkey list matches the initial distinctClause pathkeys. This introduces a new GUC, enable_distinct_reordering, which allows the optimization to be disabled if needed. Author: Richard Guo Reviewed-by: Andrei Lepikhov Discussion: https://postgr.es/m/CAMbWs48dR26cCcX0f=8bja2JKQPcU64136kHk=xekHT9xschiQ@mail.gmail.com
* Fix NULLIF()'s handling of read-write expanded objects.Tom Lane2024-11-25
| | | | | | | | | | | | | | | | | | | | | | | If passed a read-write expanded object pointer, the EEOP_NULLIF code would hand that same pointer to the equality function and then (unless equality was reported) also return the same pointer as its value. This is no good, because a function that receives a read-write expanded object pointer is fully entitled to scribble on or even delete the object, thus corrupting the NULLIF output. (This problem is likely unobservable with the equality functions provided in core Postgres, but it's easy to demonstrate with one coded in plpgsql.) To fix, make sure the pointer passed to the equality function is read-only. We can still return the original read-write pointer as the NULLIF result, allowing optimization of later operations. Per bug #18722 from Alexander Lakhin. This has been wrong since we invented expanded objects, so back-patch to all supported branches. Discussion: https://postgr.es/m/18722-fd9e645448cc78b4@postgresql.org
* Avoid "you don't own a lock of type ExclusiveLock" in GRANT TABLESPACE.Noah Misch2024-11-25
| | | | | | | | | | | | | | | | | | | | | This WARNING appeared because SearchSysCacheLocked1() read cc_relisshared before catcache initialization, when the field is false unconditionally. On the basis of reading false there, it constructed a locktag as though pg_tablespace weren't relisshared. Only shared catalogs could be affected, and only GRANT TABLESPACE was affected in practice. SearchSysCacheLocked1() callers use one other shared-relation syscache, DATABASEOID. DATABASEOID is initialized by the end of CheckMyDatabase(), making the problem unreachable for pg_database. Back-patch to v13 (all supported versions). This has no known impact before v16, where ExecGrant_common() first appeared. Earlier branches avoid trouble by having a separate ExecGrant_Tablespace() that doesn't use LOCKTAG_TUPLE. However, leaving this unfixed in v15 could ensnare a future back-patch of a SearchSysCacheLocked1() call. Reported by Aya Iwata. Discussion: https://postgr.es/m/OS7PR01MB11964507B5548245A7EE54E70EA212@OS7PR01MB11964.jpnprd01.prod.outlook.com
* Remove the wrong assertion from match_orclause_to_indexcol()Alexander Korotkov2024-11-25
| | | | | | | | Obviously, the constant could be zero. Also, add the relevant check to regression tests. Reported-by: Richard Guo Discussion: https://postgr.es/m/CAMbWs4-siKJdtWhcbqk4Y-xG12do2Ckm1qw672GNsSnDqL9FQg%40mail.gmail.com
* Doc: Clarify the `inactive_since` field description.Amit Kapila2024-11-25
| | | | | | | | | | | Updated to specify that it represents the exact time a slot became inactive, rather than the period of inactivity. Reported-by: Peter Smith Author: Bruce Momjian, Nisha Moond Reviewed-by: Amit Kapila, Peter Smith Backpatch-through: 17 Discussion: https://postgr.es/m/CAHut+PuvsyA5v8y7rYoY9mkDQzUhwaESM05yCByTMaDoRh30tA@mail.gmail.com
* Teach bitmap path generation about transforming OR-clauses to SAOP'sAlexander Korotkov2024-11-24
| | | | | | | | | | | | | | | | | | | | | When optimizer generates bitmap paths, it considers breaking OR-clause arguments one-by-one. But now, a group of similar OR-clauses can be transformed into SAOP during index matching. So, bitmap paths should keep up. This commit teaches bitmap paths generation machinery to group similar OR-clauses into dedicated RestrictInfos. Those RestrictInfos are considered both to match index as a whole (as SAOP), or to match as a set of individual OR-clause argument one-by-one (the old way). Therefore, bitmap path generation will takes advantage of OR-clauses to SAOP's transformation. The old way of handling them is also considered. So, there shouldn't be planning regression. Discussion: https://postgr.es/m/CAPpHfdu5iQOjF93vGbjidsQkhHvY2NSm29duENYH_cbhC6x%2BMg%40mail.gmail.com Author: Alexander Korotkov, Andrey Lepikhov Reviewed-by: Alena Rybakina, Andrei Lepikhov, Jian he, Robert Haas Reviewed-by: Peter Geoghegan
* Transform OR-clauses to SAOP's during index matchingAlexander Korotkov2024-11-24
| | | | | | | | | | | | | | | | | | | | This commit makes match_clause_to_indexcol() match "(indexkey op C1) OR (indexkey op C2) ... (indexkey op CN)" expression to the index while transforming it into "indexkey op ANY(ARRAY[C1, C2, ...])" (ScalarArrayOpExpr node). This transformation allows handling long OR-clauses with single IndexScan avoiding diving them into a slower BitmapOr. We currently restrict Ci to be either Const or Param to apply this transformation only when it's clearly beneficial. However, in the future, we might switch to a liberal understanding of constants, as it is in other cases. Discussion: https://postgr.es/m/567ED6CA.2040504%40sigaev.ru Author: Alena Rybakina, Andrey Lepikhov, Alexander Korotkov Reviewed-by: Peter Geoghegan, Ranier Vilela, Alexander Korotkov, Robert Haas Reviewed-by: Jian He, Tom Lane, Nikolay Shaplov
* Disallow modifying statistics on system columns.Jeff Davis2024-11-22
| | | | | Reported-by: Heikki Linnakangas Discussion: https://postgr.es/m/df3e1c41-4e6c-40ad-9636-98deefe488cd@iki.fi
* Add INT64_HEX_FORMAT and UINT64_HEX_FORMAT to c.h.Nathan Bossart2024-11-22
| | | | | | | | | | | | | | | | | | Like INT64_FORMAT and UINT64_FORMAT, these macros produce format strings for 64-bit integers. However, INT64_HEX_FORMAT and UINT64_HEX_FORMAT generate the output in hexadecimal instead of decimal. Besides introducing these macros, this commit makes use of them in several places. This was originally intended to be part of commit 5d6187d2a2, but I left it out because I felt there was a nonzero chance that back-patching these new macros into c.h could cause problems with third-party code. We tend to be less cautious with such changes in new major versions. Note that UINT64_HEX_FORMAT was originally added in commit ee1b30f128, but it was placed in test_radixtree.c, so it wasn't widely available. This commit moves UINT64_HEX_FORMAT to c.h. Discussion: https://postgr.es/m/ZwQvtUbPKaaRQezd%40nathan
* Fix data loss when restarting the bulk_write facilityHeikki Linnakangas2024-11-22
| | | | | | | | | | | | | | | | | | | If a user started a bulk write operation on a fork with existing data to append data in bulk, the bulk_write machinery would zero out all previously written pages up to the last page written by the new bulk_write operation. This is not an issue for PostgreSQL itself, because we never use the bulk_write facility on a non-empty fork. But there are use cases where it makes sense. TimescaleDB extension is known to do that to merge partitions, for example. Backpatch to v17, where the bulk_write machinery was introduced. Author: Matthias van de Meent <boekewurm+postgres@gmail.com> Reported-By: Erik Nordström <erik@timescale.com> Reviewed-by: Erik Nordström <erik@timescale.com> Discussion: https://www.postgresql.org/message-id/CACAa4VJ%2BQY4pY7M0ECq29uGkrOygikYtao1UG9yCDFosxaps9g@mail.gmail.com
* Add write_to_file to PgStat_KindInfo for pgstats kindsMichael Paquier2024-11-22
| | | | | | | | | | | | | | | | | This new field controls if entries of a stats kind should be written or not to the on-disk pgstats file when shutting down an instance. This affects both fixed and variable-numbered kinds. This is useful for custom statistics by itself, and a patch is under discussion to add a new builtin stats kind where the write of the stats is not necessary. All the built-in stats kinds, as well as the two custom stats kinds in the test module injection_points, set this flag to "true" for now, so as stats entries are written to the on-disk pgstats file. Author: Bertrand Drouvot Reviewed-by: Nazir Bilal Yavuz Discussion: https://postgr.es/m/Zz7T47nHwYgeYwOe@ip-10-97-1-34.eu-west-3.compute.internal
* pgindent runPeter Eisentraut2024-11-21
| | | | for commit 79b575d3bc0
* Fix outdated bit in README.tuplockÁlvaro Herrera2024-11-21
| | | | | | | | | | | | Apparently this information has been outdated since first committed, because we adopted a different implementation during development per reviews and this detail was not updated in the README. This has been wrong since commit 0ac5ad5134f2 introduced the file in 2013. Backpatch to all live branches. Reported-by: Will Mortensen <will@extrahop.com> Discussion: https://postgr.es/m/CAMpnoC6yEQ=c0Rdq-J7uRedrP7Zo9UMp6VZyP23QMT68n06cvA@mail.gmail.com
* Fix ALTER TABLE / REPLICA IDENTITY for temporal tablesPeter Eisentraut2024-11-21
| | | | | | | | REPLICA IDENTITY USING INDEX did not accept a GiST index. This should be allowed when used as a temporal primary key. Author: Paul Jungwirth <pj@illuminatedcomputing.com> Discussion: https://www.postgresql.org/message-id/04579cbf-b134-45e1-8f2d-8c54c849c1ee@illuminatedcomputing.com
* Unify repetitive error messagesÁlvaro Herrera2024-11-21
|
* Fix memory leak in pgoutput for the WAL senderMichael Paquier2024-11-21
| | | | | | | | | | | | | | | | | | | | | RelationSyncCache, the hash table in charge of tracking the relation schemas sent through pgoutput, was forgetting to free the TupleDesc associated to the two slots used to store the new and old tuples, causing some memory to be leaked each time a relation is invalidated when the slots of an existing relation entry are cleaned up. This is rather hard to notice as the bloat is pretty minimal, but a long-running WAL sender would be in trouble over time depending on the workload. sysbench has proved to be pretty good at showing the problem, coupled with some memory monitoring of the WAL sender. Issue introduced in 52e4f0cd472d, that has added row filters for tables logically replicated. Author: Boyu Yang Reviewed-by: Michael Paquier, Hou Zhijie Discussion: https://postgr.es/m/DM3PR84MB3442E14B340E553313B5C816E3252@DM3PR84MB3442.NAMPRD84.PROD.OUTLOOK.COM Backpatch-through: 15
* Refine nbtree = redundancy preprocessing comment.Peter Geoghegan2024-11-20
| | | | | | | | Spell out how a = key associated with a SAOP array renders a > key against the same index column redundant at the relevant point inside _bt_preprocess_keys. Follow-up to commit 5bf748b8.
* Avoid assertion failure if a setop leaf query contains setops.Tom Lane2024-11-20
| | | | | | | | | | | | | | | | | | | | | | Ordinarily transformSetOperationTree will collect all UNION/ INTERSECT/EXCEPT steps into the setOperations tree of the topmost Query, so that leaf queries do not contain any setOperations. However, it cannot thus flatten a subquery that also contains WITH, ORDER BY, FOR UPDATE, or LIMIT. I (tgl) forgot that in commit 07b4c48b6 and wrote an assertion in rule deparsing that a leaf's setOperations would always be empty. If it were nonempty then we would want to parenthesize the subquery to ensure that the output represents the setop nesting correctly (e.g. UNION below INTERSECT had better get parenthesized). So rather than just removing the faulty Assert, let's change it into an additional case to check to decide whether to add parens. We don't expect that the additional case will ever fire, but it's cheap insurance. Man Zeng and Tom Lane Discussion: https://postgr.es/m/tencent_7ABF9B1F23B0C77606FC5FE3@qq.com
* file_fdw: Add REJECT_LIMIT option to file_fdw.Fujii Masao2024-11-20
| | | | | | | | | | | | | | | | | | | | Commit 4ac2a9bece introduced the REJECT_LIMIT option for the COPY command. This commit extends the support for this option to file_fdw. As well as REJECT_LIMIT option for COPY, this option limits the maximum number of erroneous rows that can be skipped. If the number of data type conversion errors exceeds this limit, accessing the file_fdw foreign table will fail with an error, even when on_error = 'ignore' is specified. Since the CREATE/ALTER FOREIGN TABLE commands require foreign table options to be single-quoted, this commit updates defGetCopyRejectLimitOption() to handle also string value for them, in addition to int64 value for COPY command option. Author: Atsushi Torikoshi Reviewed-by: Fujii Masao, Yugo Nagata, Kirill Reshke Discussion: https://postgr.es/m/bab68a9fc502b12693f0755b6f35f327@oss.nttdata.com