aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* 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
* Fix capitalization of "Tcl"Peter Eisentraut2023-11-14
|
* 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
* Allow new role 'regress_dump_login_role' to log in under SSPI.Tom Lane2023-11-14
| | | | | Semi-blind attempt to fix a70f2a57f to work on Windows, along the same lines as 5253519b2. Per buildfarm.
* 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
* Don't try to dump RLS policies or security labels for extension objects.Tom Lane2023-11-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | checkExtensionMembership() set the DUMP_COMPONENT_SECLABEL and DUMP_COMPONENT_POLICY flags for extension member objects, even though we lack any infrastructure for tracking extensions' initial settings of these properties. This is not OK. The result was that a dump would always include commands to set these properties for extension objects that have them, with at least three negative consequences: 1. The restoring user might not have privilege to set these properties on these objects. 2. The properties might be incorrect/irrelevant for the version of the extension that's installed in the destination database. 3. The dump itself might fail, in the case of RLS properties attached to extension tables that the dumping user lacks privilege to LOCK. (That's because we must get at least AccessShareLock to ensure that we don't fail while trying to decompile the RLS expressions.) When and if somebody cares to invent initial-state infrastructure for extensions' RLS policies and security labels, we could think about finding another way around problem #3. But in the absence of such infrastructure, this whole thing is just wrong and we shouldn't do it. (Note: this applies only to ordinary dumps; binary-upgrade dumps still dump and restore extension member objects separately, with all properties.) Tom Lane and Jacob Champion. Back-patch to all supported branches. Discussion: https://postgr.es/m/00d46a48-3324-d9a0-49bf-e7f0f11d1038@timescale.com
* psql: improve description consistency of \dTS data typesBruce Momjian2023-11-13
| | | | | | | | | | | | This was done particularly for geometric data types. Reported-by: Christoph Berg Discussion: https://postgr.es/m/YGI8Leuk0WvmNWLr@msg.df7cb.de Co-authored-by: Kyotaro Horiguchi Backpatch-through: master
* Improve default and empty privilege outputs in psql.Tom Lane2023-11-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Default privileges are represented as NULL::aclitem[] in catalog ACL columns, while revoking all privileges leaves an empty aclitem[]. These two cases used to produce identical output in psql meta-commands like \dp. Using something like "\pset null '(default)'" as a workaround for spotting the difference did not work, because null values were always displayed as empty strings by describe.c's meta-commands. This patch improves that with two changes: 1. Print "(none)" for empty privileges so that the user is able to distinguish them from default privileges, even without special workarounds. 2. Remove the special handling of null values in describe.c, so that "\pset null" is honored like everywhere else. (This affects all output from these commands, not only ACLs.) The privileges shown by \dconfig+ and \ddp as well as the column privileges shown by \dp are not affected by change #1, because the respective aclitem[] is reset to NULL or deleted from the catalog instead of leaving an empty array. Erik Wienhold and Laurenz Albe Discussion: https://postgr.es/m/1966228777.127452.1694979110595@office.mailbox.org
* 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 inconsistencies for queries on pg_class in type_sanity.sqlMichael Paquier2023-11-12
| | | | | | | | | | | | Three queries did not consider partitioned indexes and tables, and surrounding comments have not been updated in a while. Like 4b9fbd6be442, this is only cosmetic currently as no such relkinds exist at this stage of the regression tests, but running these queries on existing clusters could lead to incorrect results. Author: Jian He, Michael Paquier Discussion: https://postgr.es/m/CACJufxGsB1ciahkNDccyxhw-Pfp_-_y+Wx+1BOdRyVVxKojAbg@mail.gmail.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 corner-case 64-bit integer subtraction bug on some platforms.Dean Rasheed2023-11-09
| | | | | | | | | | | | | | When computing "0 - INT64_MIN", most platforms would report an overflow error, which is correct. However, platforms without integer overflow builtins or 128-bit integers would fail to spot the overflow, and incorrectly return INT64_MIN. Back-patch to all supported branches. Patch be me. Thanks to Jian He for initial investigation, and Laurenz Albe and Tom Lane for review. Discussion: https://postgr.es/m/CAEZATCUNK-AZSD0jVdgkk0N%3DNcAXBWeAEX-QU9AnJPensikmdQ%40mail.gmail.com
* Fix uninitialized slot array access during the upgrade.Amit Kapila2023-11-09
| | | | | | | | | | Commit 29d0a77fa introduced fetching slot information from the old cluster but didn't initialize the required array in all the code paths. So when trying to access the array in verbose mode for the new cluster, it leads to an uninitialized memory access. Author: Vignesh C Discussion: http://postgr.es/m/CALDaNm1tntGP5=CtMz=v+k3_PGv7kE9t6iWSgX-QiurAaFkhZw@mail.gmail.com
* 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
* Call pqPipelineFlush from PQsendFlushRequestAlvaro Herrera2023-11-08
| | | | | | | | | | | | | | | | | | When PQsendFlushRequest() was added by commit 69cf1d5429d4, we argued against adding a PQflush() call in it[1]. This is still the right decision: if the user wants a flush to occur, they can just call that. However, we failed to realize that the message bytes could still be given to the kernel for transmitting when this can be made without blocking. That's what pqPipelineFlush() does, and it is done for every single other message type sent by libpq, so do that. (When the socket is in blocking mode this may indeed block, but that's what all the other libpq message-sending routines do, too.) [1] https://www.postgresql.org/message-id/202106252352.5ca4byasfun5%40alvherre.pgsql Author: Jelte Fennema-Nio <postgres@jeltef.nl> Discussion: https://postgr.es/m/CAGECzQTxZRevRWkKodE-SnJk1Yfm4eKT+8E4Cyq3MJ9YKTnNew@mail.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
* Don't install ldap_password_func in mesonPeter Eisentraut2023-11-08
| | | | It should be handled as a test module per commit b6a0d469ca.
* Fix use of OPENSSL in SSL tests if command is not foundMichael Paquier2023-11-08
| | | | | | | | | | | | | | `openssl` is an optional dependency in the meson build as it may not be installed in an environment even if SSL libraries are around. The meson scripts assume that, but the SSL tests thought that it was a hard dependency, causing a meson installation to fail if `openssl` could not be found. Like similar tests that depend on external commands, and to be consistent with ./configure for the SSL tests, this commit makes the command existence optional in the tests. Author: Tristan Partin Discussion: https://postgr.es/m/CWSX6P5OUUM5.N7B74KQ06ZP6@neon.tech Backpatch-through: 16
* 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
* Fix the test 003_logical_slots.Amit Kapila2023-11-07
| | | | | | | | | | | | | pg_upgrade test 003_logical_slots was leaving files like delete_old_cluster.sh in the source directory for VPATH and meson builds. The fix is to change the directory to tmp_check before running the test as is done in the similar test in 002_pg_upgrade. Reported-by: Peter Eisentraut, Andrew Dunstan Author: Hayato Kuroda based on a suggestion by Andrew Dunstan Reviewed-by: Peter Smith, Hou Zhijie Discussion: http://postgr.es/m/b4fb612d-ef0b-4db7-81b9-cf0701275491@eisentraut.org Discussion: http://postgr.es/m/TYAPR01MB5866D7B89DC5688256D980C2F5A9A@TYAPR01MB5866.jpnprd01.prod.outlook.com
* 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
* pg_resetwal: Add more tests and test coveragePeter Eisentraut2023-11-06
| | | | | | | | | | | | | pg_resetwal had poor test coverage. There are some TAP tests, but they all run with -n, so they don't actually test the full functionality. (There is a non-dry-run call of pg_resetwal in the recovery test suite, but that is incidental.) This adds a bunch of more tests to test all the different options and scenarios. Reviewed-by: Aleksander Alekseev <aleksander@timescale.com> Discussion: https://www.postgresql.org/message-id/flat/0f3ab4a1-ae80-56e8-3426-6b4a02507687@eisentraut.org
* Fix allocation of UniqueRelInfoAlexander Korotkov2023-11-06
| | | | | Reported-by: Richard Guo Discussion: https://postgr.es/m/CAMbWs4_STsG1PKQBuvQC8W4sPo3KvML3=jOTjKLUYQuK3g8cpQ@mail.gmail.com
* More consistent behavior of GetDataDirectoryCreatePerm on WindowsPeter Eisentraut2023-11-05
| | | | | | | | | | | | | | | | | | On Windows, GetDataDirectoryCreatePerm() just did nothing. The way the code in some callers is structured, this is the first function that tries to access the data directory. So it also ends up the place that is responsible for reporting that a data directory does not exist or similar. Therefore, on Windows, these scenarios end up on potentially completely different code paths. To unify this, to make testing more consistent across platforms, have GetDataDirectoryCreatePerm() run the stat() call on Windows as well, even though it won't do anything with the result. That way, file system errors are reporting to callers in the same way as on non-Windows. Reviewed-by: Aleksander Alekseev <aleksander@timescale.com> Discussion: https://www.postgresql.org/message-id/15a59bca-0383-183c-9383-0446da9b87e1%40eisentraut.org
* Try again to fix the MSVC buildDavid Rowley2023-11-04
| | | | | | | | My last attempt in 39c959ef2 mistakenly conditionally added the missing file based on some unrelated condition. Reported-by: Thomas Munro Discussion: https://postgr.es/m/CA+hUKGLovvAXim9Fytn=jxks9s=JhP5=8Oyy0cbxGG-ggALJtg@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