aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
* Explore alternative orderings of group-by pathkeys during optimization.Alexander Korotkov2024-01-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | When evaluating a query with a multi-column GROUP BY clause, we can minimize sort operations or avoid them if we synchronize the order of GROUP BY clauses with the ORDER BY sort clause or sort order, which comes from the underlying query tree. Grouping does not imply any ordering, so we can compare the keys in arbitrary order, and a Hash Agg leverages this. But for Group Agg, we simply compared keys in the order specified in the query. This commit explores alternative ordering of the keys, trying to find a cheaper one. The ordering of group keys may interact with other parts of the query, some of which may not be known while planning the grouping. For example, there may be an explicit ORDER BY clause or some other ordering-dependent operation higher up in the query, and using the same ordering may allow using either incremental sort or even eliminating the sort entirely. The patch always keeps the ordering specified in the query, assuming the user might have additional insights. This introduces a new GUC enable_group_by_reordering so that the optimization may be disabled if needed. Discussion: https://postgr.es/m/7c79e6a5-8597-74e8-0671-1c39d124c9d6%40sigaev.ru Author: Andrei Lepikhov, Teodor Sigaev Reviewed-by: Tomas Vondra, Claudio Freire, Gavin Flower, Dmitry Dolgov Reviewed-by: Robert Haas, Pavel Borisov, David Rowley, Zhihong Yu Reviewed-by: Tom Lane, Alexander Korotkov, Richard Guo, Alena Rybakina
* Generalize the common code of adding sort before processing of groupingAlexander Korotkov2024-01-21
| | | | | | | Extract the repetitive code pattern into a new function make_ordered_path(). Discussion: https://postgr.es/m/CAPpHfdtzaVa7S4onKy3YvttF2rrH5hQNHx9HtcSTLbpjx%2BMJ%2Bw%40mail.gmail.com Author: Andrei Lepikhov
* Add hint about not qualifying UPDATE...SET target with relation name.Tom Lane2024-01-20
| | | | | | | | | | | | | | | Target columns in UPDATE ... SET must not be qualified with the target table; we disallow this because it'd create ambiguity about which name is the column name in case of field-qualified names. However, newbies have been seen to expect that they could qualify a target name just like other names. The error message when they do is confusing: "column "foo" of relation "foo" does not exist". To improve matters, issue a HINT if the invalid name is qualified and matches the relation's alias. James Coleman (editorialized a bit by me) Discussion: https://postgr.es/m/CAAaqYe8S2Qa060UV-YF5GoSd5PkEhLV94x-fEi3=TOtpaXCV+w@mail.gmail.com
* Add planner support functions for range operators <@ and @>.Tom Lane2024-01-20
| | | | | | | | | | | | | | | | These support functions will transform expressions with constant range values into direct comparisons on the range bound values, which are frequently better-optimizable. The transformation is skipped however if it would require double evaluation of a volatile or expensive element expression. Along the way, add the range opfamily OID to range typcache entries, since load_rangetype_info has to compute that anyway and it seems silly to duplicate the work later. Kim Johan Andersson and Jian He, reviewed by Laurenz Albe Discussion: https://postgr.es/m/94f64d1f-b8c0-b0c5-98bc-0793a34e0851@kimmet.dk
* Introduce the dynamic shared memory registry.Nathan Bossart2024-01-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | Presently, the most straightforward way for a shared library to use shared memory is to request it at server startup via a shmem_request_hook, which requires specifying the library in shared_preload_libraries. Alternatively, the library can create a dynamic shared memory (DSM) segment, but absent a shared location to store the segment's handle, other backends cannot use it. This commit introduces a registry for DSM segments so that these other backends can look up existing segments with a library-specified string. This allows libraries to easily use shared memory without needing to request it at server startup. The registry is accessed via the new GetNamedDSMSegment() function. This function handles allocating the segment and initializing it via a provided callback. If another backend already created and initialized the segment, it simply attaches the segment. GetNamedDSMSegment() locks the registry appropriately to ensure that only one backend initializes the segment and that all other backends just attach it. The registry itself is comprised of a dshash table that stores the DSM segment handles keyed by a library-specified string. Reviewed-by: Michael Paquier, Andrei Lepikhov, Nikita Malakhov, Robert Haas, Bharath Rupireddy, Zhang Mingli, Amul Sul Discussion: https://postgr.es/m/20231205034647.GA2705267%40nathanxps13
* Fix name collision in c64086b79dbaAlexander Korotkov2024-01-19
| | | | | Reported-by: Erik Rijkers, Tom Lane Discussion: https://postgr.es/m/E1rQqeS-002A0s-Qm%40gemulon.postgresql.org
* Reorder actions in ProcArrayApplyRecoveryInfo()Alexander Korotkov2024-01-19
| | | | | | | | | | | | | | Since 5a1dfde8334b, 2PC filenames use FullTransactionId. Thus, it needs to convert TransactionId to FullTransactionId in StandbyTransactionIdIsPrepared() using TransamVariables->nextXid. However, ProcArrayApplyRecoveryInfo() first releases locks with usage StandbyTransactionIdIsPrepared(), then advances TransamVariables->nextXid. This sequence of actions could cause errors. This commit makes ProcArrayApplyRecoveryInfo() advance TransamVariables->nextXid before releasing locks. Reported-by: Thomas Munro, Michael Paquier Discussion: https://postgr.es/m/CA%2BhUKGLj_ve1_pNAnxwYU9rDcv7GOhsYXJt7jMKSA%3D5-6ss-Cw%40mail.gmail.com Discussion: https://postgr.es/m/Zadp9f4E1MYvMJqe%40paquier.xyz
* Add stratnum GiST support functionPeter Eisentraut2024-01-19
| | | | | | | | | | | | | | | | | | This is support function 12 for the GiST AM and translates "well-known" RT*StrategyNumber values into whatever strategy number is used by the opclass (since no particular numbers are actually required). We will use this to support temporal PRIMARY KEY/UNIQUE/FOREIGN KEY/FOR PORTION OF functionality. This commit adds two implementations, one for internal GiST opclasses (just an identity function) and another for btree_gist opclasses. It updates btree_gist from 1.7 to 1.8, adding the support function for all its opclasses. Author: Paul A. Jungwirth <pj@illuminatedcomputing.com> Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Reviewed-by: jian he <jian.universality@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com
* Rename COPY option from SAVE_ERROR_TO to ON_ERRORAlexander Korotkov2024-01-19
| | | | | | | | | The option names now are "stop" (default) and "ignore". The future options could be "file 'filename.log'" and "table 'tablename'". Discussion: https://postgr.es/m/20240117.164859.2242646601795501168.horikyota.ntt%40gmail.com Author: Jian He Reviewed-by: Atsushi Torikoshi
* Add optimized C string hashingJohn Naylor2024-01-19
| | | | | | | | | | | | | | | | | | | | | | | | | | Given an already-initialized hash state and a NUL-terminated string, accumulate the hash of the string into the hash state and return the length for the caller to (optionally) save for the finalizer. This avoids a strlen call. If the string pointer is aligned, we can use a word-at-a-time algorithm for NUL lookahead. The aligned case is only used on 64-bit platforms, since it's not worth the extra complexity for 32-bit. Handling the tail of the string after finishing the word-wise loop was inspired by NetBSD's strlen(), but no code was taken since that is written in assembly language. As demonstration, use this in the search path cache. This brings the general case performance closer to the special case optimization done in commit a86c61c9ee. There are other places that could benefit, but that is left for future work. Jeff Davis and John Naylor Reviewed by Heikki Linnakangas, Jian He, Junwang Zhao Discussion: https://postgr.es/m/3820f030fd008ff14134b3e9ce5cc6dd623ed479.camel%40j-davis.com Discussion: https://postgr.es/m/b40292c99e623defe5eadedab1d438cf51a4107c.camel%40j-davis.com
* Fix incorrect placeholder in walreceiver.cMichael Paquier2024-01-19
| | | | | Author: Yongtao Huang Discussion: https://postgr.es/m/CAOe1Go3H7CgrSceO+HBhnoptk-mJhii-YT8D19CikKintjwumQ@mail.gmail.com
* Improve some documentation about the bootstrap superuser.Nathan Bossart2024-01-18
| | | | | | | | | | | | | | This commit adds some notes about the inability to remove superuser privileges from the bootstrap superuser. This has been blocked since commit e530be2c5c, but it wasn't intended be a supported feature before that, either. In passing, change "bootstrap user" to "bootstrap superuser" in a couple places. Author: Yurii Rashkovskii Reviewed-by: Vignesh C, David G. Johnston Discussion: https://postgr.es/m/CA%2BRLCQzSx_eTC2Fch0EzeNHD3zFUcPvBYOoB%2BpPScFLch1DEQw%40mail.gmail.com
* Fix broken Bitmapset optimization in DiscreteKnapsack()David Rowley2024-01-19
| | | | | | | | | | | | | | | | | | | | | | Some code in DiscreteKnapsack() attempted to zero all words in a Bitmapset by performing bms_del_members() to delete all the members from itself before replacing those members with members from another set. When that code was written, this was a valid way to manipulate the set in such a way to save from palloc having to be called to allocate a new Bitmapset. However, 00b41463c modified Bitmapsets so that an empty set is *always* represented as NULL and this breaks the optimization as the Bitmapset code will always pfree the memory when the set becomes empty. Since DiscreteKnapsack() has been coded to avoid as many unneeded allocations as possible, it seems risky to not fix this. Here we add bms_replace_members() to effectively perform an in-place copy of another set, reusing the memory of the existing set, when possible. This got broken in v16, but no backpatch for now as there've been no complaints. Reviewed-by: Richard Guo Discussion: https://postgr.es/m/CAApHDvoTCBkBU2PJghNOFUiO0q=QP4WAWHi5sJP6_4=b2WodrA@mail.gmail.com
* Remove LVPagePruneState.Robert Haas2024-01-18
| | | | | | | | | | | | | | Commit cb970240f13df2b63f0410f81f452179a2b78d6f moved some code from lazy_scan_heap() to lazy_scan_prune(), and now some things that used to need to be passed back and forth are completely local to lazy_scan_prune(). Hence, this struct is mostly obsolete. The only thing that still needs to be passed back to the caller is has_lpdead_items, and that's also passed back by lazy_scan_noprune(), so do it the same way in both cases. Melanie Plageman, reviewed and slightly revised by me. Discussion: http://postgr.es/m/CAAKRu_aM=OL85AOr-80wBsCr=vLVzhnaavqkVPRkFBtD0zsuLQ@mail.gmail.com
* Move VM update code from lazy_scan_heap() to lazy_scan_prune().Robert Haas2024-01-18
| | | | | | | | | | | | | | Most of the output parameters of lazy_scan_prune() were being used to update the VM in lazy_scan_heap(). Moving that code into lazy_scan_prune() simplifies lazy_scan_heap() and requires less communication between the two. This change permits some further code simplification, but that is left for a separate commit. Melanie Plageman, reviewed by me. Discussion: http://postgr.es/m/CAAKRu_aM=OL85AOr-80wBsCr=vLVzhnaavqkVPRkFBtD0zsuLQ@mail.gmail.com
* Optimize vacuuming of relations with no indexes.Robert Haas2024-01-18
| | | | | | | | | | | | If there are no indexes on a relation, items can be marked LP_UNUSED instead of LP_DEAD when pruning. This significantly reduces WAL volume, since we no longer need to emit one WAL record for pruning and a second to change the LP_DEAD line pointers thus created to LP_UNUSED. Melanie Plageman, reviewed by Andres Freund, Peter Geoghegan, and me Discussion: https://postgr.es/m/CAAKRu_bgvb_k0gKOXWzNKWHt560R0smrGe3E8zewKPs8fiMKkw%40mail.gmail.com
* Error message capitalisationPeter Eisentraut2024-01-18
| | | | | | | per style guidelines Author: Peter Smith <peter.b.smith@fujitsu.com> Discussion: https://www.postgresql.org/message-id/flat/CAHut%2BPtzstExQ4%3DvFH%2BWzZ4g4xEx2JA%3DqxussxOdxVEwJce6bw%40mail.gmail.com
* Improve handling of dropped partitioned indexes for REINDEX INDEXMichael Paquier2024-01-18
| | | | | | | | | | | | | | | | | | | A REINDEX INDEX done on a partitioned index builds a list of the indexes to work on before processing its partitions in individual transactions. When combined with a DROP of the partitioned index, there was a window where it was possible to see some unexpected "could not open relation with OID", synonym of relation lookup error. The code was robust enough to handle the case where the parent relation is missing, but not the case where an index would be gone missing. This is similar to 1d65416661bb. Support for REINDEX on partitioned relations has been introduced in a6642b3ae060, so backpatch down to 14. Author: Fei Changhong Discussion: https://postgr.es/m/tencent_6A52106095ACDE55333E3AD33F304C0C3909@qq.com Backpatch-through: 14
* Add try_index_open(), conditional variant of index_open()Michael Paquier2024-01-18
| | | | | | | | | | | | | | try_index_open() is able to open an index if its relkind fits, except that it would return NULL instead of generated an error if the relation does not exist. This new routine will be used by an upcoming patch to make REINDEX on partitioned relations more robust when an index in a partition tree is dropped. Extracted from a larger patch by the same author. Author: Fei Changhong Discussion: https://postgr.es/m/tencent_6A52106095ACDE55333E3AD33F304C0C3909@qq.com Backpatch-through: 14
* Fix spelling in noticeAlexander Korotkov2024-01-17
| | | | | Reported-by: Atsushi Torikoshi Discussion: https://postgr.es/m/762d7dd4d5aa9e5ecffec2ae6a255a28%40oss.nttdata.com
* Fix incorrect comment on how BackendStatusArray is indexedHeikki Linnakangas2024-01-17
| | | | | | | | | | | The comment was copy-pasted from the call to ProcSignalInit() in AuxiliaryProcessMain(), which uses a similar scheme of having reserved slots for aux processes after MaxBackends slots for backends. However, ProcSignalInit() indexing starts from 1, whereas BackendStatusArray starts from 0. The code is correct, but the comment was wrong. Discussion: https://www.postgresql.org/message-id/f3ecd4cb-85ee-4e54-8278-5fabfb3a4ed0@iki.fi Backpatch-through: v14
* 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
* Fix description of DecodeInsert() in decode.cMichael Paquier2024-01-17
| | | | | | | | This incorrectly referred to deletes. Author: Yongtao Huang Reviewed-by: Richard Guo Description: https://postgr.es/m/CAOe1Go0Czgvo9eiDqeFpaABwJu=gBK6qjrYzZGZLn=tKDX8AUw@mail.gmail.com
* Add support for parsing of large XML data (>= 10MB)Michael Paquier2024-01-17
| | | | | | | | | | | | | | | | | | | This commit adds XML_PARSE_HUGE to the libxml2 functions used in core for the parsing of XML objects, raising up the original limit of 10MB supported by libxml2. In most code paths of upstream, XML_MAX_TEXT_LENGTH (10^7) is the historical limit that gets upgraded to XML_MAX_HUGE_LENGTH (10^9) once XML_PARSE_HUGE is given to the parser calls. These are still limited by any palloc() calls for text, up to 1GB. This offers the possibility to handle within the backend XML objects larger than 10MB in general, with also a higher depth limit. This change affects the contrib module xml2, the xml data type and SQL/XML. Author: Dmitry Koval Reviewed-by: Tom Lane, Michael Paquier Discussion: https://postgr.es/m/18274-98d16bc03520665f@postgresql.org
* Fix format specifier for NOTICE in copyfrom.cAlexander Korotkov2024-01-17
| | | | It's incorrect to use %lz for 64-bit numbers on 32-bit machine.
* Add new COPY option SAVE_ERROR_TOAlexander Korotkov2024-01-16
| | | | | | | | | | | | | | | | | | | | | Currently, when source data contains unexpected data regarding data type or range, the entire COPY fails. However, in some cases, such data can be ignored and just copying normal data is preferable. This commit adds a new option SAVE_ERROR_TO, which specifies where to save the error information. When this option is specified, COPY skips soft errors and continues copying. Currently, SAVE_ERROR_TO only supports "none". This indicates error information is not saved and COPY just skips the unexpected data and continues running. Later works are expected to add more choices, such as 'log' and 'table'. Author: Damir Belyalov, Atsushi Torikoshi, Alex Shulgin, Jian He Discussion: https://postgr.es/m/87k31ftoe0.fsf_-_%40commandprompt.com Reviewed-by: Pavel Stehule, Andres Freund, Tom Lane, Daniel Gustafsson, Reviewed-by: Alena Rybakina, Andy Fan, Andrei Lepikhov, Masahiko Sawada Reviewed-by: Vignesh C, Atsushi Torikoshi
* Fix REALLOCATE_BITMAPSETS codeDavid Rowley2024-01-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 7d58f2342 added a compile-time option to have bitmapset.c reallocate the set before returning when a set is modified. That commit failed to do its job in various cases and returned the input set when it shouldn't have in these cases. Here we fix those missing cases. This commit also adds some documentation about what REALLOCATE_BITMAPSETS is for. This is important as future functions that go inside bitmapset.c need to know if they need to do anything special when this compile-time option is defined. Also, between 71a3e8c43 and 7d58f2342 some Asserts seem to have become duplicated. Tidy these up. Rather than having the Assert check each aspect of what makes a set invalid, here we introduce a helper function which returns false when a set is invalid and have the Asserts use this instead. Also, make a pass on improving the comments in bitmapset.c. Various comments mentioned the input sets being "recycled". This could be interpreted to mean that the output set will always point to the same memory as the given input parameter. Here we try to make it clear that this must not be relied upon and that callers must ensure that all references to a given set are updated on each modification. In passing, improve comments for bms_union(), bms_intersect() and bms_difference() to detail what they do. I (David) have too often had to remind myself by reading the code each time to find out if I need, for example, to use bms_union() or bms_join(). I also removed some low-value comments that were trying to convey information about "these operations" without mentioning which operations it was talking about. It seems better to document these things in the function header comment instead. Author: Richard Guo, David Rowley Discussion: https://postgr.es/m/CAMbWs4-djy9qYux2gZrtmxA0StrYXJjvB-oqLxn-d7J88t=PQQ@mail.gmail.com
* Be more consistent about whether to update the FSM while vacuuming.Robert Haas2024-01-16
| | | | | | | | | | | | | | | | | | | Previously, when lazy_scan_noprune() was called and returned true, we would update the FSM immediately if the relation had no indexes or if the page contained no dead items. On the other hand, when lazy_scan_prune() was called, we would update the FSM if either of those things was true or if index vacuuming was disabled. Eliminate that behavioral difference by considering vacrel->do_index_vacuuming in both cases. Also, make lazy_scan_heap() responsible for deciding whether to update the FSM, instead of doing it inside lazy_scan_noprune(). This is more consistent with the lazy_scan_prune() case. lazy_scan_noprune() still needs an output parameter for whether there are LP_DEAD items on the page, but the real decision-making now happens in the caller. Patch by me, reviewed by Peter Geoghegan and Melanie Plageman. Discussion: http://postgr.es/m/CA+TgmoaOzvN1TcHd9iej=PR3fY40En1USxzOnXSR2CxCLaRM0g@mail.gmail.com
* Support identity columns in partitioned tablesPeter Eisentraut2024-01-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, identity columns were disallowed on partitioned tables. (The reason was mainly that no one had gotten around to working through all the details to make it work.) This makes it work now. Some details on the behavior: * A newly created partition inherits identity property The partitions of a partitioned table are integral part of the partitioned table. A partition inherits identity columns from the partitioned table. An identity column of a partition shares the identity space with the corresponding column of the partitioned table. In other words, the same identity column across all partitions of a partitioned table share the same identity space. This is effected by sharing the same underlying sequence. When INSERTing directly into a partition, the sequence associated with the topmost partitioned table is used to calculate the value of the corresponding identity column. In regular inheritance, identity columns and their properties in a child table are independent of those in its parent tables. A child table does not inherit identity columns or their properties automatically from the parent. (This is unchanged.) * Attached partition inherits identity column A table being attached as a partition inherits the identity property from the partitioned table. This should be fine since we expect that the partition table's column has the same type as the partitioned table's corresponding column. If the table being attached is a partitioned table, the identity properties are propagated down its partition hierarchy. An identity column in the partitioned table is also marked as NOT NULL. The corresponding column in the partition needs to be marked as NOT NULL for the attach to succeed. * Drop identity property when detaching partition A partition's identity column shares the identity space (i.e. underlying sequence) as the corresponding column of the partitioned table. If a partition is detached it can longer share the identity space as before. Hence the identity columns of the partition being detached loose their identity property. When identity of a column of a regular table is dropped it retains the NOT NULL constraint that came with the identity property. Similarly the columns of the partition being detached retain the NOT NULL constraints that came with identity property, even though the identity property itself is lost. The sequence associated with the identity property is linked to the partitioned table (and not the partition being detached). That sequence is not dropped as part of detach operation. * Partitions with their own identity columns are not allowed. * The usual ALTER operations (add identity column, add identity property to existing column, alter properties of an indentity column, drop identity property) are supported for partitioned tables. Changing a column only in a partitioned table or a partition is not allowed; the change needs to be applied to the whole partition hierarchy. Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Discussion: https://www.postgresql.org/message-id/flat/CAExHW5uOykuTC+C6R1yDSp=o8Q83jr8xJdZxgPkxfZ1Ue5RRGg@mail.gmail.com
* struct XmlTableRoutine: use C99 designated initializersAlvaro Herrera2024-01-16
| | | | | | | | As in c27f8621eed et al. Not as critical as other cases we've handled, but I figure if we're going to add JsonbTableRoutine using TableFuncRoutine, this makes it easier to jump around the code.
* Assert that partition inherits from only one parent in MergeAttributes()Peter Eisentraut2024-01-16
| | | | | | | | | | | A partition inherits only from one partitioned table and thus inherits a column definition only once. Assert the same in MergeAttributes() and simplify a condition accordingly. Similar definition exists about line 3068 in the same function. Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CAExHW5uOykuTC+C6R1yDSp=o8Q83jr8xJdZxgPkxfZ1Ue5RRGg@mail.gmail.com
* Fix 'negative bitmapset member' errorAlexander Korotkov2024-01-15
| | | | | | | | | | | | | | | | | | | | | When removing a useless join, we'd remove PHVs that are not used at join partner rels or above the join. A PHV that references the join's relid in ph_eval_at is logically "above" the join and thus should not be removed. We have the following check for that: !bms_is_member(ojrelid, phinfo->ph_eval_at) However, in the case of SJE removing a useless inner join, 'ojrelid' is set to -1, which would trigger the "negative bitmapset member not allowed" error in bms_is_member(). Fix it by skipping examining ojrelid for inner joins in this check. Reported-by: Zuming Jiang Bug: #18260 Discussion: https://postgr.es/m/18260-1b6a0c4ae311b837%40postgresql.org Author: Richard Guo Reviewed-by: Andrei Lepikhov
* Avoid useless ReplicationOriginExitCleanup lockingAlvaro Herrera2024-01-15
| | | | | | | | When session_replication_state is NULL, we can know there's nothing to do with no lock acquisition. Do that. Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Discussion: https://postgr.es/m/CALj2ACX+YaeRU5xJqR4C7kLsTO_F7DBRNF8WgeHvJZcKtNuK_A@mail.gmail.com
* Prevent access to an unpinned buffer in BEFORE ROW UPDATE triggers.Tom Lane2024-01-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When ExecBRUpdateTriggers switches to a new target tuple as a result of the EvalPlanQual logic, it must form a new proposed update tuple. Since commit 86dc90056, that tuple (the result of ExecGetUpdateNewTuple) has been a virtual tuple that might contain pointers to by-ref fields of the new target tuple (in "oldslot"). However, immediately after that we materialize oldslot, causing it to drop its buffer pin, whereupon the by-ref pointers are unsafe to use. This is a live bug only when the new target tuple is in a different page than the original target tuple, since we do still hold a pin on the original one. (Before 86dc90056, there was no bug because the EPQ plantree would hold a pin on the new target tuple; but now that's not assured.) To fix, forcibly materialize the new tuple before we materialize oldslot. This costs nothing since we would have done that shortly anyway. The real-world impact of this is probably minimal. A visible failure could occur if the new target tuple's buffer were recycled for some other page in the short interval before we materialize newslot within the trigger-calling loop; but that's quite unlikely given that we'd just touched that page. There's a larger hazard that some other process could prune and repack that page within the window. We have lock on the new target tuple, but that wouldn't prevent it being moved on the page. Alexander Lakhin and Tom Lane, per bug #17798 from Alexander Lakhin. Back-patch to v14 where 86dc90056 came in. Discussion: https://postgr.es/m/17798-0907404928dcf0dd@postgresql.org
* Remove useless AssertPeter Eisentraut2024-01-14
| | | | | It's already included in the subsequent intVal() call. Fixup for 4f622503d6.
* 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
* Make attstattarget nullablePeter Eisentraut2024-01-13
| | | | | | | | | | | | | | | | | | | | | | | | This changes the pg_attribute field attstattarget into a nullable field in the variable-length part of the row. If no value is set by the user for attstattarget, it is now null instead of previously -1. This saves space in pg_attribute and tuple descriptors for most practical scenarios. (ATTRIBUTE_FIXED_PART_SIZE is reduced from 108 to 104.) Also, null is the semantically more correct value. The ANALYZE code internally continues to represent the default statistics target by -1, so that that code can avoid having to deal with null values. But that is now contained to the ANALYZE code. Only the DDL code deals with attstattarget possibly null. For system columns, the field is now always null. The ANALYZE code skips system columns anyway. To set a column's statistics target to the default value, the new command form ALTER TABLE ... SET STATISTICS DEFAULT can be used. (SET STATISTICS -1 still works.) Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://www.postgresql.org/message-id/flat/4da8d211-d54d-44b9-9847-f2a9f1184c76@eisentraut.org
* Fix memory leak in connection string validation.Jeff Davis2024-01-12
| | | | | | | | Introduced in commit c3afe8cf5a. Discussion: https://postgr.es/m/066a65233d3cb4ea27a9e0778d2f1d0dc764b222.camel@j-davis.com Reviewed-by: Nathan Bossart, Tom Lane Backpatch-through: 16
* Re-validate connection string in libpqrcv_connect().Jeff Davis2024-01-12
| | | | | | | | | | | | | | | | | | | A superuser may create a subscription with password_required=true, but which uses a connection string without a password. Previously, if the owner of such a subscription was changed to a non-superuser, the non-superuser was able to utilize a password from another source (like a password file or the PGPASSWORD environment variable), which should not have been allowed. This commit adds a step to re-validate the connection string before connecting. Reported-by: Jeff Davis Author: Vignesh C Reviewed-by: Peter Smith, Robert Haas, Amit Kapila Discussion: https://www.postgresql.org/message-id/flat/e5892973ae2a80a1a3e0266806640dae3c428100.camel%40j-davis.com Backpatch-through: 16
* Refactor ATExecAddColumn() to use BuildDescForRelation()Peter Eisentraut2024-01-12
| | | | | | | | | | | | BuildDescForRelation() has all the knowledge for converting a ColumnDef into pg_attribute/tuple descriptor. ATExecAddColumn() can make use of that, instead of duplicating all that logic. We just pass a one-element list of ColumnDef and we get back exactly the data structure we need. Note that we don't even need to touch BuildDescForRelation() to make this work. Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://www.postgresql.org/message-id/flat/52a125e4-ff9a-95f5-9f61-b87cf447e4da@eisentraut.org
* Refactor code checking for file existenceMichael Paquier2024-01-12
| | | | | | | | | | jit.c and dfgr.c had a copy of the same code to check if a file exists or not, with a twist: jit.c did not check for EACCES when failing the stat() call for the path whose existence is tested. This refactored routine will be used by an upcoming patch. Reviewed-by: Ashutosh Bapat Discussion: https://postgr.es/m/ZTiV8tn_MIb_H2rE@paquier.xyz
* Rework how logirep launchers are stopped during pg_upgradeMichael Paquier2024-01-12
| | | | | | | | | | | | | | | | | This is a rework of 7021d3b17664, where we relied on forcing max_logical_replication_workers to 0 in the postgres command. This commit now prevents logical replication launchers to start using -b and a backend-side check based on IsBinaryUpgrade, effective when upgrading from 17 and newer versions. This commit improves the comments explaining why this restriction is necessary. This discussion was on hold until we were sure how to add support for subscribers in pg_upgrade, something now done thanks to 9a17be1e244a. Reviewed-by: Álvaro Herrera, Amit Kapila, Tom Lane Discussion: https://postgr.es/m/ZU2TeVkUg5qEi7Oy@paquier.xyz
* 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
* Remove hastup from LVPagePruneState.Robert Haas2024-01-11
| | | | | | | | | | | Instead, just have lazy_scan_prune() and lazy_scan_noprune() update LVRelState->nonempty_pages directly. This makes the two functions more similar and also removes makes lazy_scan_noprune need one fewer output parameters. Melanie Plageman, reviewed by Andres Freund, Michael Paquier, and me Discussion: http://postgr.es/m/CAAKRu_btji_wQdg=ok-5E4v_bGVxKYnnFFe7RA6Frc1EcOwtSg@mail.gmail.com
* Reindent after commit d9ef650fca7bc574586f4171cd929cfd5240326e.Robert Haas2024-01-11
|
* Add new function pg_get_wal_summarizer_state().Robert Haas2024-01-11
| | | | | | | | | | This makes it possible to access information about the progress of WAL summarization from SQL. The previously-added functions pg_available_wal_summaries() and pg_wal_summary_contents() only examine on-disk state, but this function exposes information from the server's shared memory. Discussion: http://postgr.es/m/CA+Tgmobvqqj-DW9F7uUzT-cQqs6wcVb-Xhs=w=hzJnXSE-kRGw@mail.gmail.com
* Restore initdb's old behavior of always setting the lc_xxx GUCs.Tom Lane2024-01-10
| | | | | | | | | | | | | | | | | | | | | | | In commit 3e51b278d I (tgl) caused initdb to leave lc_messages and other lc_xxx GUCs commented-out in the installed postgresql.conf file if they were going to be set to 'C'. This was a hack for cosmetic purposes, and it was buggy because lc_messages' wired-in default is not 'C' but '' (empty string). That led to --no-locale not having the expected effect, since the postmaster would then obtain lc_messages from its startup environment. Let's just revert to the prior behavior of always de-commenting the lc_xxx entries; the argument for changing that longstanding behavior was weak in the first place. Also, fix postgresql.conf.sample's erroneous claim that the default value of lc_messages is 'C'. I suspect that was what misled me into making this mistake in the first place. Report and patch by Kyotaro Horiguchi. Back-patch to v16 where the problem was introduced. Discussion: https://postgr.es/m/20231122.162700.1995154567625541112.horikyota.ntt@gmail.com
* Fix Asserts in calc_non_nestloop_required_outer().Tom Lane2024-01-10
| | | | | | | | | | | | | | These were not testing the same thing as the comparable Assert in calc_nestloop_required_outer(), because we neglected to map the given Paths' relids to top-level relids. When considering a partition child join the latter is the correct thing to do. This oversight is old, but since it's only an overly-weak Assert check there doesn't seem to be much value in back-patching. Richard Guo (with cosmetic changes and comment updates by me) Discussion: https://postgr.es/m/CAMbWs49sqbe9GBZ8sy8dSfKRNURgicR85HX8vgzcgQsPF0XY1w@mail.gmail.com
* Handle WindowClause.runCondition in tree walker/mutator functions.Tom Lane2024-01-10
| | | | | | | | | | | | | | | | | Commit 9d9c02ccd, which added the notion of a "run condition" for window functions, neglected to teach nodeFuncs.c to process the new field. Remarkably, that doesn't seem to have had any ill effects before we invented Var.varnullingrels, but now it can cause visible failures in join-removal scenarios. I have no faith that there's not reachable problems in v15 too, so back-patch the code change to v15 where 9d9c02ccd came in. The test case seems irrelevant to v15, though. Per bug #18277 from Zuming Jiang. Diagnosis and patch by Richard Guo. Discussion: https://postgr.es/m/18277-089ead83b329a2fd@postgresql.org
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-07 13:04:42 +0000