aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
...
* Remove NOT_USED BufFileTellBlock() from buffile.cMichael Paquier2023-11-17
| | | | | | | | | | | | | This routine has been marked as NOT_USED since 20ad43b576d9 from 2000, and a patch is planned to switch the logtape/tuplestore APIs to rely on int64 rather than long for the block nunbers, which is more portable. Keeping it is more confusing than anything at this stage, so let's get rid of it entirely. Thanks for Heikki Linnakangas for the poke on this one. Discussion: https://postgr.es/m/5047be8c-7ee6-4dd5-af76-6c916c3103b4@iki.fi
* Ensure we preprocess expressions before checking their volatility.Tom Lane2023-11-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | contain_mutable_functions and contain_volatile_functions give reliable answers only after expression preprocessing (specifically eval_const_expressions). Some places understand this, but some did not get the memo --- which is not entirely their fault, because the problem is documented only in places far away from those functions. Introduce wrapper functions that allow doing the right thing easily, and add commentary in hopes of preventing future mistakes from copy-and-paste of code that's only conditionally safe. Two actual bugs of this ilk are fixed here. We failed to preprocess column GENERATED expressions before checking mutability, so that the code could fail to detect the use of a volatile function default-argument expression, or it could reject a polymorphic function that is actually immutable on the datatype of interest. Likewise, column DEFAULT expressions weren't preprocessed before determining if it's safe to apply the attmissingval mechanism. A false negative would just result in an unnecessary table rewrite, but a false positive could allow the attmissingval mechanism to be used in a case where it should not be, resulting in unexpected initial values in a new column. In passing, re-order the steps in ComputePartitionAttrs so that its checks for invalid column references are done before applying expression_planner, rather than after. The previous coding would not complain if a partition expression contains a disallowed column reference that gets optimized away by constant folding, which seems to me to be a behavior we do not want. Per bug #18097 from Jim Keener. Back-patch to all supported versions. Discussion: https://postgr.es/m/18097-ebb179674f22932f@postgresql.org
* Add target "slru" to pg_stat_reset_shared()Michael Paquier2023-11-16
| | | | | | | | | | | | | | | | Currently, pg_stat_reset_shared() cannot reset the counters in the view pg_stat_slru even if it is a type of shared stats. This patch adds support for a new value in pg_stat_reset_shared(), called "slru", able to do that. Note that pg_stat_reset_shared(NULL) also resets SLRU counters. There may be a point in removing pg_stat_reset_slru() that was introduced in 28cac71bd368 (v13~) as the new option overlaps with this function, but we would lose the ability to reset individual SLRU counters. This is left for future reconsideration. Author: Atsushi Torikoshi Discussion: https://postgr.es/m/e3c25d72e81378e7b64f3c52e0306fc9@oss.nttdata.com
* Retire MemoryContextResetAndDeleteChildren() macro.Nathan Bossart2023-11-15
| | | | | | | | | | | | | | | | | As of commit eaa5808e8e, MemoryContextResetAndDeleteChildren() is just a backwards compatibility macro for MemoryContextReset(). Now that some time has passed, this macro seems more likely to create confusion. This commit removes the macro and replaces all remaining uses with calls to MemoryContextReset(). Any third-party code that use this macro will need to be adjusted to call MemoryContextReset() instead. Since the two have behaved the same way since v9.5, such adjustments won't produce any behavior changes for all currently-supported versions of PostgreSQL. Reviewed-by: Amul Sul, Tom Lane, Alvaro Herrera, Dagfinn Ilmari Mannsåker Discussion: https://postgr.es/m/20231113185950.GA1668018%40nathanxps13
* Clear CurrentResourceOwner earlier in CommitTransaction.Heikki Linnakangas2023-11-15
| | | | | | | | | | | Alexander reported a crash with repeated create + drop database, after the ResourceOwner rewrite (commit b8bff07daa). That was fixed by the previous commit, but it nevertheless seems like a good idea clear CurrentResourceOwner earlier, because you're not supposed to use it for anything after we start releasing it. Reviewed-by: Alexander Lakhin Discussion: https://www.postgresql.org/message-id/11b70743-c5f3-3910-8e5b-dd6c115ff829%40gmail.com
* Fix dsa.c with different resource owners.Heikki Linnakangas2023-11-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The comments in dsa.c suggested that areas were owned by resource owners, but it was not in fact tracked explicitly. The DSM attachments held by the dsa were owned by resource owners, but not the area itself. That led to confusion if you used one resource owner to attach or create the area, but then switched to a different resource owner before allocating or even just accessing the allocations in the area with dsa_get_address(). The additional DSM segments associated with the area would get owned by a different resource owner than the initial segment. To fix, add an explicit 'resowner' field to dsa_area. It replaces the 'mapping_pinned' flag; resowner == NULL now indicates that the mapping is pinned. This is arguably a bug fix, but I'm not backpatching because it doesn't seem to be a live bug in the back branches. In 'master', it is a bug because commit b8bff07daa made ResourceOwners more strict so that you are no longer allowed to remember new resources in a ResourceOwner after you have started to release it. Merely accessing a dsa pointer might need to attach a new DSM segment, and before this commit it was temporarily remembered in the current owner for a very brief period even if the DSA was pinned. And that could happen in AtEOXact_PgStat(), which is called after the owner is already released. Reported-by: Alexander Lakhin Reviewed-by: Alexander Lakhin, Thomas Munro, Andres Freund Discussion: https://www.postgresql.org/message-id/11b70743-c5f3-3910-8e5b-dd6c115ff829%40gmail.com
* Add cache for recomputeNamespacePath().Jeff Davis2023-11-14
| | | | | | | | | | | | | | | | When search_path is changed to something that was previously set, and no invalidation happened in between, use the cached list of namespace OIDs rather than recomputing them. This avoids syscache lookups and ACL checks. Important when the search_path changes frequently, such as when set in proconfig. An earlier version of this patch was reviewd by Nathan Bossart. This version simplifies a few things and is safer in case of OOM. Discussion: https://www.postgresql.org/message-id/abf4ce8804e0e05dff8c1725ae6a8ed28b7d66e0.camel%40j-davis.com Reviewed-by: Nathan Bossart
* Change how a base backup decides which files have checksums.Robert Haas2023-11-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, it thought that any plain file located under global, base, or a tablespace directory had checksums unless it was in a short list of excluded files. Now, it thinks that files in those directories have checksums if parse_filename_for_nontemp_relation says that they are relation files. (Temporary relation files don't matter because they're excluded from the backup anyway.) This changes the behavior if you have stray files not managed by PostgreSQL in the relevant directories. Previously, you'd get some kind of checksum-related complaint if such files existed, assuming that the cluster had checksums enabled and that the base backup wasn't run with NOVERIFY_CHECKSUMS. Now, you won't get those complaints any more. That seems like an improvement to me, because those files were presumably not created by PostgreSQL and so there is no reason to think that they would be checksummed like a PostgreSQL relation file. (If we want to complain about such files, we should complain about them existing at all, not just about their checksums.) The point of this change is to make the code more consistent. sendDir() was already calling parse_filename_for_nontemp_relation() as part of an effort to determine which files to include in the backup. So, it already had the information about whether a certain file was a relation file. sendFile() then used a separate method, embodied in is_checksummed_file(), to make what is essentially the same determination. It's better not to make the same decision using two different methods, especially in closely-related code. Patch by me. Reviewed by Dilip Kumar and Álvaro Herrera. Thanks also to Jakub Wartak and Peter Eisentraut for comments, suggestions, and testing on the larger patch set of which this is a part. Discussion: http://postgr.es/m/CAFiTN-snhaKkWhi2Gz5i3cZeKefun6sYL==wBoqqnTXxX4_mFA@mail.gmail.com Discussion: http://postgr.es/m/202311141312.u4qx5gtpvfq3@alvherre.pgsql
* Support +/- infinity in the interval data type.Dean Rasheed2023-11-14
| | | | | | | | | | | | | | | | | | | | | | | | | This adds support for infinity to the interval data type, using the same input/output representation as the other date/time data types that support infinity. This allows various arithmetic operations on infinite dates, timestamps and intervals. The new values are represented by setting all fields of the interval to INT32/64_MIN for -infinity, and INT32/64_MAX for +infinity. This ensures that they compare as less/greater than all other interval values, without the need for any special-case comparison code. Note that, since those 2 values were formerly accepted as legal finite intervals, pg_upgrade and dump/restore from an old database will turn them from finite to infinite intervals. That seems OK, since those exact values should be extremely rare in practice, and they are outside the documented range supported by the interval type, which gives us a certain amount of leeway. Bump catalog version. Joseph Koshakow, Jian He, and Ashutosh Bapat, reviewed by me. Discussion: https://postgr.es/m/CAAvxfHea4%2BsPybKK7agDYOMo9N-Z3J6ZXf3BOM79pFsFNcRjwA%40mail.gmail.com
* Replace Gen_dummy_probes.sed with Gen_dummy_probes.plPeter Eisentraut2023-11-14
| | | | | | | | | | | | | | | | | | | | | | | | To generate a dummy probes.h file when dtrace is not available, we had two different scripts: A sed version, which is the original version, and a Perl version, which was generated by s2p. This split was necessary because Perl was not a mandatory build dependency on Unix, but sed was not guaranteed to be available on Windows. (The Meson build system used the sed version even on Windows, which was probably incorrect and probably would have had to be fixed before elevating that build system from experimental status.) As of 721856ff24, Perl is a required build dependency, so this split is no longer necessary. We can just use the Perl script in all build environments and remove a whole bunch of infrastructure to keep the two variants in sync. The new Gen_dummy_probes.pl is not the version generated by s2p but a new implementation written by hand by adapting the sed version to Perl syntax. Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/3fd0f1bc-4483-4ba9-8aa0-64765b052039@eisentraut.org
* Add support for pg_stat_reset_slru without argumentMichael Paquier2023-11-14
| | | | | | | | | | | | | | | | | pg_stat_reset_slru currently requires an input argument, either: - NULL to reset the SLRU counters of everything. - A specific value to reset a single SLRU cache. This commit adds support for a new pattern: pg_stat_reset_slru without any argument works the same way as pg_stat_reset_slru(NULL), relying on a DEFAULT in the function definition to handle this case. This makes the function more consistent with 23c8c0c8f472. Bump catalog version. Author: Bharath Rupireddy Reviewed-by: Atsushi Torikoshi Discussion: https://postgr.es/m/CALj2ACW1VizYg01EeH_cA-7qA+4NzWVAoZ5Lw9_XYO1RRHAZbA@mail.gmail.com
* Improve readability and error detection of array_in().Tom Lane2023-11-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Rewrite array_in() and its subroutines so that we make only one pass over the input text, rather than two. This requires potentially re-pallocing the working arrays values[] and nulls[] larger than our initial guess, but that cost will hopefully be made up by avoiding duplicate parsing. In any case this coding seems much clearer and more straightforward than what we had before. This also fixes array_in() to reject non-rectangular input (that is, different brace depths in different parts of the input) more reliably than before, and to give a better error message when it does so. This is analogous to the plpython and plperl fixes in 0553528e7 and f47004add. Like those PLs, we now accept input such as '{{},{}}' as a valid representation of an empty array, which we did not before. Additionally, reject explicit array subscripts that are outside the integer range (previously you just got whatever atoi() converted them to), and make some other minor improvements in error reporting. Although this is arguably a bug fix, it's also a behavioral change that might trip somebody up, so no back-patch. Tom Lane, Heikki Linnakangas, and Jian He. Thanks to Alexander Lakhin for the initial report and for review/testing. Discussion: https://postgr.es/m/2794005.1683042087@sss.pgh.pa.us
* Add error about the use of FREEZE in COPY TOBruce Momjian2023-11-13
| | | | | | | | | | Also clarify some other error wording. Reported-by: Kyotaro Horiguchi Discussion: https://postgr.es/m/20220802.133046.1941977979333284049.horikyota.ntt@gmail.com Backpatch-through: master
* Don't release index root page pin in ginFindParents().Tom Lane2023-11-13
| | | | | | | | | | | | | | | | | | | | | It's clearly stated in the comments that ginFindParents() must keep the pin on the index's root page that's associated with the topmost GinBtreeStack item. However, the code path for the case that the desired downlink has been pushed down to the next index level ignored this proviso, and would release the pin anyway if we were still examining the root level. That led to an assertion failure or "buffer NNNN is not owned by resource owner" error later, when we try to release the pin again at the end of the insertion. This is quite hard to reproduce, since it can only happen if an index root page split occurs concurrently with our own insertion. Thanks to Jeff Janes for finding a test case that triggers it often enough to allow investigation. This has been there since the beginning of GIN, so back-patch to all supported branches. Discussion: https://postgr.es/m/CAMkU=1yCAKtv86dMrD__Ja-7KzjE=uMeKX8y__cx5W-OEWy2ow@mail.gmail.com
* Remove incorrect file reference in comment.Etsuro Fujita2023-11-13
| | | | | | | | | | | | | | | Commit b7eda3e0e moved XidInMVCCSnapshot() from tqual.c into snapmgr.c, but follow-up commit c91560def incorrectly updated this reference. We could fix it, but as pointed out by Daniel Gustafsson, 1) the reader can easily find the file that contains the definition of that function, e.g. by grepping, and 2) this kind of reference is prone to going stale; so let's just remove it. Back-patch to all supported branches. Reviewed by Daniel Gustafsson. Discussion: https://postgr.es/m/CAPmGK145VdKkPBLWS2urwhgsfidbSexwY-9zCL6xSUJH%2BBTUUg%40mail.gmail.com
* Use REGBUF_NO_CHANGE at one more place in the hash index.Amit Kapila2023-11-13
| | | | | | | | | | Commit 00d7fb5e2e started to use REGBUF_NO_CHANGE at a few places in the code where we register the buffer before marking it dirty but missed updating one of the code flows in the hash index where we free the overflow page without any live tuples on it. Author: Amit Kapila and Hayato Kuroda Discussion: http://postgr.es/m/f045c8f7-ee24-ead6-3679-c04a43d21351@gmail.com
* Extend sendFileWithContent() to handle custom content length in basebackup.cMichael Paquier2023-11-13
| | | | | | | | | | | | | | | | | | | sendFileWithContent() previously got the content length by using strlen(), assuming that the content given is always a string. Some patches are under discussion to pass binary contents to a base backup stream, where an arbitrary length needs to be given by the caller instead. The patch extends sendFileWithContent() to be able to handle this case, where len < 0 can be used to indicate an arbitrary length rather than rely on strlen() for the content length. A comment in sendFileWithContent() mentioned the backup_label file. However, this routine is used by more file types, like the tablespace map, so adjust it in passing. Author: David Steele Discussion: https://postgr.es/m/2daf8adc-8db7-4204-a7f2-a7e94e2bfa4b@pgmasters.net
* Add ability to reset all shared stats types in pg_stat_reset_shared()Michael Paquier2023-11-12
| | | | | | | | | | | | | | | | | | | | | | Currently, pg_stat_reset_shared() can use an argument to specify the target of statistics to reset, doing nothing for NULL as it is strict. This patch adds to pg_stat_reset_shared() the possibility to reset all the stats types already handled in this function rather than do nothing if the argument value given is NULL or if nothing is specified (proisstrict is switched to false). Like previously, SLRUs are not included in what gets reset. The idea to use NULL or no argument to control if all the shared stats already covered by this function should be reset has been proposed by Andres Freund. Bump catalog version. Author: Atsushi Torikoshi Reviewed-by: Kyotaro Horiguchi, Michael Paquier, Bharath Rupireddy, Matthias van de Meent Discussion: https://postgr.es/m/4291a55137ddda77cf7cc5f46e846daf@oss.nttdata.com
* Fix how SJE checks against PHVsAlexander Korotkov2023-11-10
| | | | | | | | | | It seems that a PHV evaluated/needed at or below the self join should not have a problem if we remove the self join. But this requires further investigation. For now, we just do not remove self joins if the rel to be removed is laterally referenced by PHVs. Discussion: https://postgr.es/m/CAMbWs4-ns73VF9gi37q61G3dS6Xuos+HtryMaBh37WQn=BsaJw@mail.gmail.com Author: Richard Guo
* Prohibit max_slot_wal_keep_size to value other than -1 during upgrade.Amit Kapila2023-11-10
| | | | | | | | | | | We don't want existing slots in the old cluster to get invalidated during the upgrade. During an upgrade, we set this variable to -1 via the command line in an attempt to prevent such invalidations, but users have ways to override it. This patch ensures the value is not overridden by the user. Author: Kyotaro Horiguchi Reviewed-by: Peter Smith, Alvaro Herrera Discussion: http://postgr.es/m/20231027.115759.2206827438943188717.horikyota.ntt@gmail.com
* Fix computation of varnullingrels when const-folding field selection.Tom Lane2023-11-09
| | | | | | | | | | | | | We can simplify FieldSelect on a whole-row Var into a plain Var for the selected field. However, we should copy the whole-row Var's varnullingrels when we do so, because the new Var is clearly nullable by exactly the same rels as the original. Failure to do this led to errors like "wrong varnullingrels (b) (expected (b 3)) for Var 2/2". Richard Guo, per bug #18184 from Marian Krucina. Back-patch to v16 where varnullingrels was introduced. Discussion: https://postgr.es/m/18184-5868dd258782058e@postgresql.org
* Fix the way SJE removes references from PHVsAlexander Korotkov2023-11-09
| | | | | | | | | | | Add missing replacement of relids in phv->phexpr. Also, remove extra replace_relid() over phv->phrels. Reported-by: Zuming Jiang Bug: #18187 Discussion: https://postgr.es/m/flat/18187-831da249cbd2ff8e%40postgresql.org Author: Richard Guo Reviewed-by: Andrei Lepikhov
* Avoid integer overflow hazard in interval_time().Dean Rasheed2023-11-09
| | | | | | | | | | | | When casting an interval to a time, the original code suffered from 64-bit integer overflow for inputs with a sufficiently large negative "time" field, leading to bogus results. Fix by rewriting the algorithm in a simpler form, that more obviously cannot overflow. While at it, improve the test coverage to include negative interval inputs. Discussion: https://postgr.es/m/CAEZATCXoUKHkcuq4q63hkiPsKZJd0kZWzgKtU%2BNT0aU4wbf_Pw%40mail.gmail.com
* Fix AFTER ROW trigger execution in MERGE cross-partition update.Dean Rasheed2023-11-09
| | | | | | | | | | | | | | | | | | | | | When executing a MERGE UPDATE action, if the UPDATE is turned into a cross-partition DELETE then INSERT, do not attempt to invoke AFTER UPDATE ROW triggers, or any of the other post-update actions in ExecUpdateEpilogue(). For consistency with a plain UPDATE command, such triggers should not be fired (and typically fail anyway), and similarly, other post-update actions, such as WCO/RLS checks should not be executed, and might also lead to unexpected failures. Therefore, as with ExecUpdate(), make ExecMergeMatched() return immediately if ExecUpdateAct() reports that a cross-partition update was done, to be sure that no further processing is done for that tuple. Back-patch to v15, where MERGE was introduced. Discussion: https://postgr.es/m/CAEZATCWjBgagyNZs02vgDF0DvASYj-iHTFtXG2-nP3orZhmtcw%40mail.gmail.com
* Ensure we use the correct spelling of "ensure"David Rowley2023-11-10
| | | | | | | | | We seem to have accidentally used "insure" in a few places. Correct that. Author: Peter Smith Discussion: https://postgr.es/m/CAHut+Pv0biqrhA3pMhu40aDsj343mTsD75khKnHsLqR8P04f=Q@mail.gmail.com Backpatch-through: 12, oldest supported version
* Fix bug in the new ResourceOwner implementation.Heikki Linnakangas2023-11-09
| | | | | | | | | | | | | | | When the hash table is in use, ResoureOwnerSort() moves any elements from the small fixed-size array to the hash table, and sorts it. When the hash table is not in use, it sorts the elements in the small fixed-size array directly. However, ResourceOwnerSort() and ResourceOwnerReleaseAll() had different idea on when the hash table is in use: ResourceOwnerSort() checked owner->nhash != 0, and ResourceOwnerReleaseAll() checked owner->hash != NULL. If the hash table was allocated but was currently empty, you hit an assertion failure. Reported-by: Alexander Lakhin <exclusion@gmail.com> Discussion: https://www.postgresql.org/message-id/be58d565-9e95-d417-4e47-f6bd408dea4b@gmail.com
* Check stack depth in new recursive functionsAlvaro Herrera2023-11-08
| | | | | | | | | | Commit b0e96f311985 introduced a bunch of recursive functions, but failed to make them check for stack depth. This can cause the backend to crash when operating on inheritance hierarchies several thousands deep. Protect the code by adding the missing stack depth checks. Reported-by: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/b2ac2392-9727-5f76-e890-721ac80c1615@gmail.com
* Use a faster hash function in resource owners.Heikki Linnakangas2023-11-08
| | | | | | | | | | This buys back some of the performance loss that we otherwise saw from the previous commit. Reviewed-by: Aleksander Alekseev, Michael Paquier, Julien Rouhaud Reviewed-by: Kyotaro Horiguchi, Hayato Kuroda, Álvaro Herrera, Zhihong Yu Reviewed-by: Peter Eisentraut, Andres Freund Discussion: https://www.postgresql.org/message-id/d746cead-a1ef-7efe-fb47-933311e876a3%40iki.fi
* Make ResourceOwners more easily extensible.Heikki Linnakangas2023-11-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Instead of having a separate array/hash for each resource kind, use a single array and hash to hold all kinds of resources. This makes it possible to introduce new resource "kinds" without having to modify the ResourceOwnerData struct. In particular, this makes it possible for extensions to register custom resource kinds. The old approach was to have a small array of resources of each kind, and if it fills up, switch to a hash table. The new approach also uses an array and a hash, but now the array and the hash are used at the same time. The array is used to hold the recently added resources, and when it fills up, they are moved to the hash. This keeps the access to recent entries fast, even when there are a lot of long-held resources. All the resource-specific ResourceOwnerEnlarge*(), ResourceOwnerRemember*(), and ResourceOwnerForget*() functions have been replaced with three generic functions that take resource kind as argument. For convenience, we still define resource-specific wrapper macros around the generic functions with the old names, but they are now defined in the source files that use those resource kinds. The release callback no longer needs to call ResourceOwnerForget on the resource being released. ResourceOwnerRelease unregisters the resource from the owner before calling the callback. That needed some changes in bufmgr.c and some other files, where releasing the resources previously always called ResourceOwnerForget. Each resource kind specifies a release priority, and ResourceOwnerReleaseAll releases the resources in priority order. To make that possible, we have to restrict what you can do between phases. After calling ResourceOwnerRelease(), you are no longer allowed to remember any more resources in it or to forget any previously remembered resources by calling ResourceOwnerForget. There was one case where that was done previously. At subtransaction commit, AtEOSubXact_Inval() would handle the invalidation messages and call RelationFlushRelation(), which temporarily increased the reference count on the relation being flushed. We now switch to the parent subtransaction's resource owner before calling AtEOSubXact_Inval(), so that there is a valid ResourceOwner to temporarily hold that relcache reference. Other end-of-xact routines make similar calls to AtEOXact_Inval() between release phases, but I didn't see any regression test failures from those, so I'm not sure if they could reach a codepath that needs remembering extra resources. There were two exceptions to how the resource leak WARNINGs on commit were printed previously: llvmjit silently released the context without printing the warning, and a leaked buffer io triggered a PANIC. Now everything prints a WARNING, including those cases. Add tests in src/test/modules/test_resowner. Reviewed-by: Aleksander Alekseev, Michael Paquier, Julien Rouhaud Reviewed-by: Kyotaro Horiguchi, Hayato Kuroda, Álvaro Herrera, Zhihong Yu Reviewed-by: Peter Eisentraut, Andres Freund Discussion: https://www.postgresql.org/message-id/cbfabeb0-cd3c-e951-a572-19b365ed314d%40iki.fi
* Move a few ResourceOwnerEnlarge() calls for safety and clarity.Heikki Linnakangas2023-11-08
| | | | | | | | | | | | | | | | | | | | | | These are functions where a lot of things happen between the ResourceOwnerEnlarge and ResourceOwnerRemember calls. It's important that there are no unrelated ResourceOwnerRemember calls in the code in between, otherwise the reserved entry might be used up by the intervening ResourceOwnerRemember and not be available at the intended ResourceOwnerRemember call anymore. I don't see any bugs here, but the longer the code path between the calls is, the harder it is to verify. In bufmgr.c, there is a function similar to ResourceOwnerEnlarge, ReservePrivateRefCountEntry(), to ensure that the private refcount array has enough space. The ReservePrivateRefCountEntry() calls were made at different places than the ResourceOwnerEnlargeBuffers() calls. Move the ResourceOwnerEnlargeBuffers() and ReservePrivateRefCountEntry() calls together for consistency. Reviewed-by: Aleksander Alekseev, Michael Paquier, Julien Rouhaud Reviewed-by: Kyotaro Horiguchi, Hayato Kuroda, Álvaro Herrera, Zhihong Yu Reviewed-by: Peter Eisentraut, Andres Freund Discussion: https://www.postgresql.org/message-id/cbfabeb0-cd3c-e951-a572-19b365ed314d%40iki.fi
* Enlarge assertion in bloom_init() for false_positive_rateMichael Paquier2023-11-08
| | | | | | | | | | | | | | | | | | false_positive_rate is a parameter that can be set with the bloom opclass in BRIN, and setting it to a value of exactly 0.25 would trigger an assertion in the first INSERT done on the index with value set. The assertion changed here relied on BLOOM_{MIN|MAX}_FALSE_POSITIVE_RATE that are somewhat arbitrary values, and specifying an out-of-range value would also trigger a failure when defining such an index. So, as-is, the assertion was just doubling on the min-max check of the reloption. This is now enlarged to check that it is a correct percentage value, instead, based on a suggestion by Tom Lane. Author: Alexander Lakhin Reviewed-by: Tom Lane, Shihao Zhong Discussion: https://postgr.es/m/17969-a6c54de48026d694@postgresql.org Backpatch-through: 14
* Stop including parsenodes.h in plannodes.hAlvaro Herrera2023-11-07
| | | | | | | | | | | | | | | | | | | | I added it by mistake in commit 7103ebb7aae8. To clean up, struct MergeAction needs to be moved to primnodes.h from parsenodes.h. (This forces us to also move OverridingKind to primnodes.h). Having to add parsenodes.h to bootstrap.h as fallout is a bit surprising, since nothing nominally needs it there. However, per comments in bootscanner.l, it is needed so that YYSTYPE can be declared. I think this only started with commit dac048f71ebb, but I didn't actually verify that. In passing, stop including parsenodes.h in tcopprot.h. Nothing needs it there. Per discussion on a patch by Ashutosh Bapat. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/202311071106.6y7b2ascqjlz@alvherre.pgsql
* Reorder two functions in inval.cMichael Paquier2023-11-07
| | | | | | | | | | This file separates public and static functions with a separator comment, but two routines were not defined in a location reflecting that, so reorder them. Author: Aleksander Alekseev Reviewed-by: Álvaro Herrera, Michael Paquier Discussion: https://postgr.es/m/CAJ7c6TMX2dd0g91UKvcC+CVygKQYJkKJq1+ZzT4rOK42+b53=w@mail.gmail.com
* Make use of initReadOnlyStringInfo() in more placesDavid Rowley2023-11-07
| | | | | | | | | | | | | | | | | | | | | | f0efa5aec introduced the concept of "read-only" StringInfos which makes use of an existing, possibly not NUL terminated, buffer. Here we adjust two places that make use of StringInfos to receive data to avoid using appendBinaryStringInfo() in cases where a NUL termination character is not required. This saves a possible palloc() and saves having to needlessly memcpy() from one buffer to another. Here we adjust two places which were using appendBinaryStringInfo(). Neither of these cases seem particularly performance-critical. In the case of XLogWalRcvProcessMsg(), the appendBinaryStringInfo() was only appending 24 bytes. The change made here does mean that we can get rid of the incoming_message global variable and make that local instead. The apply_spooled_messages() case applies in logical decoding when applying (possibly large) changes which have been serialized to a file. Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/CAApHDvoxYUDHwqPf-ShvchsERf1RzmkGoLwg63JNvHCkDCuyKQ@mail.gmail.com
* Detect integer overflow while computing new array dimensions.Tom Lane2023-11-06
| | | | | | | | | | | | | | | | | | | | | | | | | | array_set_element() and related functions allow an array to be enlarged by assigning to subscripts outside the current array bounds. While these places were careful to check that the new bounds are allowable, they neglected to consider the risk of integer overflow in computing the new bounds. In edge cases, we could compute new bounds that are invalid but get past the subsequent checks, allowing bad things to happen. Memory stomps that are potentially exploitable for arbitrary code execution are possible, and so is disclosure of server memory. To fix, perform the hazardous computations using overflow-detecting arithmetic routines, which fortunately exist in all still-supported branches. The test cases added for this generate (after patching) errors that mention the value of MaxArraySize, which is platform-dependent. Rather than introduce multiple expected-files, use psql's VERBOSITY parameter to suppress the printing of the message text. v11 psql lacks that parameter, so omit the tests in that branch. Our thanks to Pedro Gallegos for reporting this problem. Security: CVE-2023-5869
* Compute aggregate argument types correctly in transformAggregateCall().Tom Lane2023-11-06
| | | | | | | | | | | | | | | | | | | | | | | | transformAggregateCall() captures the datatypes of the aggregate's arguments immediately to construct the Aggref.aggargtypes list. This seems reasonable because the arguments have already been transformed --- but there is an edge case where they haven't been. Specifically, if we have an unknown-type literal in an ANY argument position, nothing will have been done with it earlier. But if we also have DISTINCT, then addTargetToGroupList() converts the literal to "text" type, resulting in the aggargtypes list not matching the actual runtime type of the argument. The end result is that the aggregate tries to interpret a "text" value as being of type "unknown", that is a zero-terminated C string. If the text value contains no zero bytes, this could result in disclosure of server memory following the text literal value. To fix, move the collection of the aggargtypes list to the end of transformAggregateCall(), after DISTINCT has been handled. This requires slightly more code, but not a great deal. Our thanks to Jingzhou Fu for reporting this problem. Security: CVE-2023-5868
* Remove distprepPeter Eisentraut2023-11-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A PostgreSQL release tarball contains a number of prebuilt files, in particular files produced by bison, flex, perl, and well as html and man documentation. We have done this consistent with established practice at the time to not require these tools for building from a tarball. Some of these tools were hard to get, or get the right version of, from time to time, and shipping the prebuilt output was a convenience to users. Now this has at least two problems: One, we have to make the build system(s) work in two modes: Building from a git checkout and building from a tarball. This is pretty complicated, but it works so far for autoconf/make. It does not currently work for meson; you can currently only build with meson from a git checkout. Making meson builds work from a tarball seems very difficult or impossible. One particular problem is that since meson requires a separate build directory, we cannot make the build update files like gram.h in the source tree. So if you were to build from a tarball and update gram.y, you will have a gram.h in the source tree and one in the build tree, but the way things work is that the compiler will always use the one in the source tree. So you cannot, for example, make any gram.y changes when building from a tarball. This seems impossible to fix in a non-horrible way. Second, there is increased interest nowadays in precisely tracking the origin of software. We can reasonably track contributions into the git tree, and users can reasonably track the path from a tarball to packages and downloads and installs. But what happens between the git tree and the tarball is obscure and in some cases non-reproducible. The solution for both of these issues is to get rid of the step that adds prebuilt files to the tarball. The tarball now only contains what is in the git tree (*). Getting the additional build dependencies is no longer a problem nowadays, and the complications to keep these dual build modes working are significant. And of course we want to get the meson build system working universally. This commit removes the make distprep target altogether. The make dist target continues to do its job, it just doesn't call distprep anymore. (*) - The tarball also contains the INSTALL file that is built at make dist time, but not by distprep. This is unchanged for now. The make maintainer-clean target, whose job it is to remove the prebuilt files in addition to what make distclean does, is now just an alias to make distprep. (In practice, it is probably obsolete given that git clean is available.) The following programs are now hard build requirements in configure (they were already required by meson.build): - bison - flex - perl Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://www.postgresql.org/message-id/flat/e07408d9-e5f2-d9fd-5672-f53354e9305e@eisentraut.org
* Set GUC "is_superuser" in all processes that set AuthenticatedUserId.Noah Misch2023-11-06
| | | | | | | | | It was always false in single-user mode, in autovacuum workers, and in background workers. This had no specifically-identified security consequences, but non-core code or future work might make it security-relevant. Back-patch to v11 (all supported versions). Jelte Fennema-Nio. Reported by Jelte Fennema-Nio.
* Ban role pg_signal_backend from more superuser backend types.Noah Misch2023-11-06
| | | | | | | | | | | | | | | | | | | Documentation says it cannot signal "a backend owned by a superuser". On the contrary, it could signal background workers, including the logical replication launcher. It could signal autovacuum workers and the autovacuum launcher. Block all that. Signaling autovacuum workers and those two launchers doesn't stall progress beyond what one could achieve other ways. If a cluster uses a non-core extension with a background worker that does not auto-restart, this could create a denial of service with respect to that background worker. A background worker with bugs in its code for responding to terminations or cancellations could experience those bugs at a time the pg_signal_backend member chooses. Back-patch to v11 (all supported versions). Reviewed by Jelte Fennema-Nio. Reported by Hemanth Sandrana and Mahendrakar Srinivasarao. Security: CVE-2023-5870
* Add XMLText function (SQL/XML X038)Daniel Gustafsson2023-11-06
| | | | | | | | | | | | | | | | | | | This function implements the standard XMLTest function, which converts text into xml text nodes. It uses the libxml2 function xmlEncodeSpecialChars to escape predefined entities (&"<>), so that those do not cause any conflict when concatenating the text node output with existing xml documents. This also adds a note in features.sgml about not supporting XML(SEQUENCE). The SQL specification defines a RETURNING clause to a set of XML functions, where RETURNING CONTENT or RETURNING SEQUENCE can be defined. Since PostgreSQL doesn't support XML(SEQUENCE) all of these functions operate with an implicit RETURNING CONTENT. Author: Jim Jones <jim.jones@uni-muenster.de> Reviewed-by: Vik Fearing <vik@postgresfriends.org> Discussion: https://postgr.es/m/86617a66-ec95-581f-8d54-08059cca8885@uni-muenster.de
* Fix allocation of UniqueRelInfoAlexander Korotkov2023-11-06
| | | | | Reported-by: Richard Guo Discussion: https://postgr.es/m/CAMbWs4_STsG1PKQBuvQC8W4sPo3KvML3=jOTjKLUYQuK3g8cpQ@mail.gmail.com
* Fix usage of the parse tree for estimate_num_groups() in set operationsAlexander Korotkov2023-11-04
| | | | | | | | | | | | | | | | | | | | | recurse_set_operations() uses the parse tree for the group number estimation, because of the "varno 0" hack. At the same time 2489d76c49 made root->parse and corresponding parent_root->simple_rte_array[]->subquery distinct copies of the parse tree, while d3d55ce571 introduced self-join removal replacing relid of removed relation only in one of the copies. The present commit fixes this bug by making recurse_set_operations() call estimate_num_groups() with the copy of the parse tree processed by self-join removal. In future, we may think about maintaining just one copy of the parse tree and/or keeping removed relids as aliases. Reported-by: Zuming Jiang Bug: #18170 Discussion: https://postgr.es/m/flat/18170-f1d17bf9a0d58b24%40postgresql.org Author: Richard Guo, Alexander Korotkov Reviewed-by: Andrei Lepikhov
* Make GetConfigOption/GetConfigOptionResetString return "" for NULL.Tom Lane2023-11-02
| | | | | | | | | | | | | | | As per the preceding commit, GUC APIs generally expose NULL-valued string variables as empty strings. Extend that policy to GetConfigOption() and GetConfigOptionResetString(), eliminating a crash hazard for unwary callers, as well as a fundamental ambiguity in GetConfigOption()'s API. No back-patch, since this is an API change and conceivably somebody somewhere is depending on this corner case. Xing Guo, Aleksander Alekseev, Tom Lane Discussion: https://postgr.es/m/CACpMh+AyDx5YUpPaAgzVwC1d8zfOL4JoD-uyFDnNSa1z0EsDQQ@mail.gmail.com
* Be more wary about NULL values for GUC string variables.Tom Lane2023-11-02
| | | | | | | | | | | | | get_explain_guc_options() crashed if a string GUC marked GUC_EXPLAIN has a NULL boot_val. Nosing around found a couple of other places that seemed insufficiently cautious about NULL string values, although those are likely unreachable in practice. Add some commentary defining the expectations for NULL values of string variables, in hopes of forestalling future additions of more such bugs. Xing Guo, Aleksander Alekseev, Tom Lane Discussion: https://postgr.es/m/CACpMh+AyDx5YUpPaAgzVwC1d8zfOL4JoD-uyFDnNSa1z0EsDQQ@mail.gmail.com
* Additional unicode primitive functions.Jeff Davis2023-11-01
| | | | | | | | | | | | Introduce unicode_version(), icu_unicode_version(), and unicode_assigned(). The latter requires introducing a new lookup table for the Unicode General Category, which is generated along with the other Unicode lookup tables. Discussion: https://postgr.es/m/CA+TgmoYzYR-yhU6k1XFCADeyj=Oyz2PkVsa3iKv+keM8wp-F_A@mail.gmail.com Reviewed-by: Peter Eisentraut
* Fix function name in commentDaniel Gustafsson2023-11-01
| | | | | | | | | | | The name of the function resulting from the macro expansion was incorrectly stated. Backpatch to 16 where it was introduced. Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Discussion: https://postgr.es/m/20231101.172308.1740861597185391383.horikyota.ntt@gmail.com Backpatch-through: v16
* doc: 1-byte varlena headers can be used for user PLAIN storageBruce Momjian2023-10-31
| | | | | | | | | | | | This also updates some C comments. Reported-by: suchithjn22@gmail.com Discussion: https://postgr.es/m/167336599095.2667301.15497893107226841625@wrigleys.postgresql.org Author: Laurenz Albe (doc patch) Backpatch-through: 11
* improve alignment of postgresql.conf commentsBruce Momjian2023-10-31
| | | | | | | | Discussion: https://postgr.es/m/CAHut+Ps5MdQ1b4jp9rd63zfE2X25mV58y1W+hm2v53svtGDxBQ@mail.gmail.com Author: Peter Smith Backpatch-through: master
* Diagnose !indisvalid in more SQL functions.Noah Misch2023-10-30
| | | | | | | | | | | | | pgstatindex failed with ERRCODE_DATA_CORRUPTED, of the "can't-happen" class XX. The other functions succeeded on an empty index; they might have malfunctioned if the failed index build left torn I/O or other complex state. Report an ERROR in statistics functions pgstatindex, pgstatginindex, pgstathashindex, and pgstattuple. Report DEBUG1 and skip all index I/O in maintenance functions brin_desummarize_range, brin_summarize_new_values, brin_summarize_range, and gin_clean_pending_list. Back-patch to v11 (all supported versions). Discussion: https://postgr.es/m/20231001195309.a3@google.com
* pgindent run to fix commits de64268561 and 5ae2087202aBruce Momjian2023-10-30
| | | | | | | | Reported-by: Michael Paquier Discussion: https://postgr.es/m/ZT9YH7-TTx27V3yW@paquier.xyz Backpatch-through: master