aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
* meson: add install-{quiet, world} targetsAndres Freund2023-03-23
| | | | | | | To define our own install target, we need dependencies on the i18n targets, which we did not collect so far. Discussion: https://postgr.es/m/3fc3bb9b-f7f8-d442-35c1-ec82280c564a@enterprisedb.com
* Rename fields in pgstat structures for functions and relationsMichael Paquier2023-03-24
| | | | | | | | | | | | | | | | | This commit renames the members of a few pgstat structures related to functions and relations, by respectively removing their prefix "f_" and "t_". The statistics for functions and relations and handled in their own file, and pgstatfuncs.c associates each field in a structure variable named based on the object type handled, so no information is lost with this rename. This will help with some of the refactoring aimed for pgstatfuncs.c, as this makes more consistent the field names with the SQL functions retrieving them. Author: Bertrand Drouvot Reviewed-by: Michael Paquier, Melanie Plageman Discussion: https://postgr.es/m/9142f62a-a422-145c-bde0-b5bc498a4ada@gmail.com
* Count updates that move row to a new page.Peter Geoghegan2023-03-23
| | | | | | | | | | | | | | | | | | Add pgstat counter to track row updates that result in the successor version going to a new heap page, leaving behind an original version whose t_ctid points to the new version. The current count is shown by the n_tup_newpage_upd column of each of the pg_stat_*_tables views. The new n_tup_newpage_upd column complements the existing n_tup_hot_upd and n_tup_upd columns. Tables that have high n_tup_newpage_upd values (relative to n_tup_upd) are good candidates for tuning heap fillfactor. Corey Huinker, with small tweaks by me. Author: Corey Huinker <corey.huinker@gmail.com> Reviewed-By: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/CADkLM=ded21M9iZ36hHm-vj2rE2d=zcKpUQMds__Xm2pxLfHKA@mail.gmail.com
* Handle the "und" locale in ICU versions 54 and older.Jeff Davis2023-03-23
| | | | | | | | | | | | | | | | | | | | | | The "und" locale is an alternative spelling of the root locale, but it was not recognized until ICU 55. To maintain common behavior across all supported ICU versions, check for "und" and replace with "root" before opening. Previously, the lack of support for "und" was dangerous, because versions 54 and older fall back to the environment when a locale is not found. If the user specified "und" for the language (which is expected and documented), it could not only resolve to the wrong collator, but it could unexpectedly change (which could lead to corrupt indexes). This effectively reverts commit d72900bded, which worked around the problem for the built-in "unicode" collation, and is no longer necessary. Discussion: https://postgr.es/m/60da0cecfb512a78b8666b31631a636215d8ce73.camel@j-davis.com Discussion: https://postgr.es/m/0c6fa66f2753217d2a40480a96bd2ccf023536a1.camel@j-davis.com Reviewed-by: Peter Eisentraut
* Wrap ICU ucol_open().Jeff Davis2023-03-23
| | | | | | | | | | Hide details of supporting older ICU versions in a wrapper function. The current code only needs to handle icu_set_collation_attributes(), but a subsequent commit will add additional version-specific code. Discussion: https://postgr.es/m/7ee414ad-deb5-1144-8a0e-b34ae3b71cd5@enterprisedb.com Reviewed-by: Peter Eisentraut
* Ignore generated columns during apply of update/delete.Amit Kapila2023-03-23
| | | | | | | | | | | | We fail to apply updates and deletes when the REPLICA IDENTITY FULL is used for the table having generated columns. We didn't use to ignore generated columns while doing tuple comparison among the tuples from the publisher and subscriber during apply of updates and deletes. Author: Onder Kalaci Reviewed-by: Shi yu, Amit Kapila Backpatch-through: 12 Discussion: https://postgr.es/m/CACawEhVQC9WoofunvXg12aXtbqKnEgWxoRx3+v8q32AWYsdpGg@mail.gmail.com
* Allow logical replication to copy tables in binary format.Amit Kapila2023-03-23
| | | | | | | | | | | | | | | This patch allows copying tables in the binary format during table synchronization when the binary option for a subscription is enabled. Previously, tables are copied in text format even if the subscription is created with the binary option enabled. Copying tables in binary format may reduce the time spent depending on column types. A binary copy for initial table synchronization is supported only when both publisher and subscriber are v16 or later. Author: Melih Mutlu Reviewed-by: Peter Smith, Shi yu, Euler Taveira, Vignesh C, Kuroda Hayato, Osumi Takamichi, Bharath Rupireddy, Hou Zhijie Discussion: https://postgr.es/m/CAGPVpCQvAziCLknEnygY0v1-KBtg%2BOm-9JHJYZOnNPKFJPompw%40mail.gmail.com
* Improve the naming of Parallel Hash Join phases.Thomas Munro2023-03-23
| | | | | | | | | | | | | | | | | | | | | | | | | * Commit 3048898e dropped -ING from PHJ wait event names. Update the corresponding barrier phases names to match. * Rename the "DONE" phases to "FREE". That's symmetrical with "ALLOCATE", and names the activity that actually happens in that phase (as we do for the other phases) rather than a state. The bug fixed by commit 8d578b9b might have been more obvious with this name. * Rename the batch/bucket growth barriers' "ALLOCATE" phases to "REALLOCATE", a better description of what they do. * Update the high level comments about phases to highlight phases are executed by a single process with an asterisk (mostly memory management phases). No behavior change, as this is just improving internal identifiers. The only user-visible sign of this is that a couple of wait events' display names change from "...Allocate" to "...Reallocate" in pg_stat_activity, to stay in sync with the internal names. Reviewed-by: Melanie Plageman <melanieplageman@gmail.com> Discussion: https://postgr.es/m/CA%2BhUKG%2BMDpwF2Eo2LAvzd%3DpOh81wUTsrwU1uAwR-v6OGBB6%2B7g%40mail.gmail.com
* Allow locking updated tuples in tuple_update() and tuple_delete()Alexander Korotkov2023-03-23
| | | | | | | | | | | | | | | | | | | | | | | | | | Currently, in read committed transaction isolation mode (default), we have the following sequence of actions when tuple_update()/tuple_delete() finds the tuple updated by concurrent transaction. 1. Attempt to update/delete tuple with tuple_update()/tuple_delete(), which returns TM_Updated. 2. Lock tuple with tuple_lock(). 3. Re-evaluate plan qual (recheck if we still need to update/delete and calculate the new tuple for update). 4. Second attempt to update/delete tuple with tuple_update()/tuple_delete(). This attempt should be successful, since the tuple was previously locked. This patch eliminates step 2 by taking the lock during first tuple_update()/tuple_delete() call. Heap table access method saves some efforts by checking the updated tuple once instead of twice. Future undo-based table access methods, which will start from the latest row version, can immediately place a lock there. The code in nodeModifyTable.c is simplified by removing the nested switch/case. Discussion: https://postgr.es/m/CAPpHfdua-YFw3XTprfutzGp28xXLigFtzNbuFY8yPhqeq6X5kg%40mail.gmail.com Reviewed-by: Aleksander Alekseev, Pavel Borisov, Vignesh C, Mason Sharp Reviewed-by: Andres Freund, Chris Travers
* Evade extra table_tuple_fetch_row_version() in ExecUpdate()/ExecDelete()Alexander Korotkov2023-03-23
| | | | | | | | | | | When we lock tuple using table_tuple_lock() then we at the same time fetch the locked tuple to the slot. In this case we can skip extra table_tuple_fetch_row_version() thank to we've already fetched the 'old' tuple and nobody can change it concurrently since it's locked. Discussion: https://postgr.es/m/CAPpHfdua-YFw3XTprfutzGp28xXLigFtzNbuFY8yPhqeq6X5kg%40mail.gmail.com Reviewed-by: Aleksander Alekseev, Pavel Borisov, Vignesh C, Mason Sharp Reviewed-by: Andres Freund, Chris Travers
* Fix memory leak and inefficiency in CREATE DATABASE ... STRATEGY WAL_LOGAndres Freund2023-03-22
| | | | | | | | | | | | | | | RelationCopyStorageUsingBuffer() did not free the strategies used to access the source / target relation. They memory was released at the end of the transaction, but when using a template database with a lot of relations, the temporary leak can become big prohibitively big. RelationCopyStorageUsingBuffer() acquired the buffer for the target relation with RBM_NORMAL, therefore requiring a read of a block guaranteed to be zero. Use RBM_ZERO_AND_LOCK instead. Reviewed-by: Robert Haas <robertmhaas@gmail.com> Discussion: https://postgr.es/m/20230321070113.o2vqqxogjykwgfrr@awork3.anarazel.de Backpatch: 15-, where STRATEGY WAL_LOG was introduced
* Support language tags in older ICU versions (53 and earlier).Jeff Davis2023-03-21
| | | | | | | | | | | By calling uloc_canonicalize() before parsing the attributes, the existing locale attribute parsing logic works on language tags as well. Fix a small memory leak, too. Discussion: http://postgr.es/m/60da0cecfb512a78b8666b31631a636215d8ce73.camel@j-davis.com Reviewed-by: Peter Eisentraut
* Fix make maintainer-clean with queryjumblefuncs.*.c files in src/backend/nodes/Michael Paquier2023-03-22
| | | | | | | | | | | | | | | The files generated by gen_node_support.pl for query jumbling (queryjumblefuncs.funcs.c and queryjumblefuncs.switch.c) were not being removed on make maintainer-clean (they need to remain around after a simple "clean"). This commit makes the operation consistent with the copy, equal, out and read files. While on it, update a comment in the nodes'README where a reference to queryjumblefuncs.funcs.c was missing. Reported-by: Nathan Bossart Reviewed-by: Richard Guo, Daniel Gustafsson Discussion: https://postgr.es/m/ZBgAfTHcL6W7zGdW@paquier.xyz
* Fix incorrect comment in preptlist.cDavid Rowley2023-03-22
| | | | | | Author: Etsuro Fujita Reviewed-by: Richard Guo, Tom Lane Discussion: https://postgr.es/m/CAPmGK15V8dcVxL9vcgVWPHV6pw1qzM42LzoUkQDB7-e+1onnJw@mail.gmail.com
* Correct Memoize's estimated cache hit ratio calculationDavid Rowley2023-03-22
| | | | | | | | | | | | | | | | | | | | | | | As demonstrated by David Johnston, the Memoize cache hit ratio calculation wasn't quite correct. This change only affects the estimated hit ratio when the estimated number of entries to cache is estimated not to fit inside the cache. For example, if we expect 2000 distinct cache key values and only expect to be able to cache 1000 of those at once due to memory constraints, with an estimate of 10000 calls, if we could store all entries then the hit ratio should be 80% to account for the first 2000 of the 10000 calls to be a cache miss due to the value not being cached yet. If we can only store 1000 entries for each of the 2000 distinct possible values at once then the 80% should be reduced by half to make the final estimate of 40%. Previously, the calculation would have produced an estimated hit ratio of 30%, which wasn't correct. Apply to master only so as not to destabilize plans in the back branches. Reported-by: David G. Johnston Discussion: https://postgr.es/m/CAKFQuwZEmcNk3YQo2Xj4EDUOdY6qakad31rOD1Vc4q1_s68-Ew@mail.gmail.com Discussion: https://postgr.es/m/CAApHDvrV44LwiF4W_qf_RpbGYWSgp1kF=cZr+kTRRaALUfmXqw@mail.gmail.com
* Ignore dropped columns during apply of update/delete.Amit Kapila2023-03-21
| | | | | | | | | | | We fail to apply updates and deletes when the REPLICA IDENTITY FULL is used for the table having dropped columns. We didn't use to ignore dropped columns while doing tuple comparison among the tuples from the publisher and subscriber during apply of updates and deletes. Author: Onder Kalaci, Shi yu Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/CACawEhVQC9WoofunvXg12aXtbqKnEgWxoRx3+v8q32AWYsdpGg@mail.gmail.com
* Fix race in parallel hash join batch cleanup, take II.Thomas Munro2023-03-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | With unlucky timing and parallel_leader_participation=off (not the default), PHJ could attempt to access per-batch shared state just as it was being freed. There was code intended to prevent that by checking for a cleared pointer, but it was racy. Fix, by introducing an extra barrier phase. The new phase PHJ_BUILD_RUNNING means that it's safe to access the per-batch state to find a batch to help with, and PHJ_BUILD_DONE means that it is too late. The last to detach will free the array of per-batch state as before, but now it will also atomically advance the phase, so that late attachers can avoid the hazard. This mirrors the way per-batch hash tables are freed (see phases PHJ_BATCH_PROBING and PHJ_BATCH_DONE). An earlier attempt to fix this (commit 3b8981b6, later reverted) missed one special case. When the inner side is empty (the "empty inner optimization), the build barrier would only make it to PHJ_BUILD_HASHING_INNER phase before workers attempted to detach from the hashtable. In that case, fast-forward the build barrier to PHJ_BUILD_RUNNING before proceeding, so that our later assertions hold and we can still negotiate who is cleaning up. Revealed by build farm failures, where BarrierAttach() failed a sanity check assertion, because the memory had been clobbered by dsa_free(). In non-assert builds, the result could be a segmentation fault. Back-patch to all supported releases. Author: Thomas Munro <thomas.munro@gmail.com> Author: Melanie Plageman <melanieplageman@gmail.com> Reported-by: Michael Paquier <michael@paquier.xyz> Reported-by: David Geier <geidav.pg@gmail.com> Tested-by: David Geier <geidav.pg@gmail.com> Discussion: https://postgr.es/m/20200929061142.GA29096%40paquier.xyz
* Add @extschema:name@ and no_relocate options to extensions.Tom Lane2023-03-20
| | | | | | | | | | | | | | | | | | | | | | @extschema:name@ extends the existing @extschema@ feature so that we can also insert the schema name of some required extension, thus making cross-extension references robust even if they are in different schemas. However, this has the same hazard as @extschema@: if the schema name is embedded literally in an installed object, rather than being looked up once during extension script execution, then it's no longer safe to relocate the other extension to another schema. To deal with that without restricting things unnecessarily, add a "no_relocate" option to extension control files. This allows an extension to specify that it cannot handle relocation of some of its required extensions, even if in themselves those extensions are relocatable. We detect "no_relocate" requests of dependent extensions during ALTER EXTENSION SET SCHEMA. Regina Obe, reviewed by Sandro Santilli and myself Discussion: https://postgr.es/m/003001d8f4ae$402282c0$c0678840$@pcorp.us
* Ignore BRIN indexes when checking for HOT updatesTomas Vondra2023-03-20
| | | | | | | | | | | | | | | | | | | | | | | | When determining whether an index update may be skipped by using HOT, we can ignore attributes indexed by block summarizing indexes without references to individual tuples that need to be cleaned up. A new type TU_UpdateIndexes provides a signal to the executor to determine which indexes to update - no indexes, all indexes, or only the summarizing indexes. This also removes rd_indexattr list, and replaces it with rd_attrsvalid flag. The list was not used anywhere, and a simple flag is sufficient. This was originally committed as 5753d4ee32, but then got reverted by e3fcca0d0d because of correctness issues. Original patch by Josef Simanek, various fixes and improvements by Tomas Vondra and me. Authors: Matthias van de Meent, Josef Simanek, Tomas Vondra Reviewed-by: Tomas Vondra, Alvaro Herrera Discussion: https://postgr.es/m/05ebcb44-f383-86e3-4f31-0a97a55634cf@enterprisedb.com Discussion: https://postgr.es/m/CAFp7QwpMRGcDAQumN7onN9HjrJ3u4X3ZRXdGFT0K5G2JWvnbWg%40mail.gmail.com
* Fix netmask handling in inet_minmax_multi_opsTomas Vondra2023-03-20
| | | | | | | | | | | | | | | | | | | When calculating distance in brin_minmax_multi_distance_inet(), the netmask was applied incorrectly. This results in (seemingly) incorrect ordering of values, triggering an assert. For builds without asserts this is mostly harmless - we may merge other ranges, possibly resulting in slightly less efficient index. But it's still correct and the greedy algorithm doesn't guarantee optimality anyway. Backpatch to 14, where minmax-multi indexes were introduced. Reported by Dmitry Dolgov, investigation and fix by me. Reported-by: Dmitry Dolgov Backpatch-through: 14 Discussion: https://postgr.es/m/17774-c6f3e36dd4471e67@postgresql.org
* Have the planner account for the Memoize cache key memoryDavid Rowley2023-03-20
| | | | | | | | | | | | | | | | | | | | | The Memoize executor node stores the cache key values along with the tuple(s) which were found in the outer node which match each key value, however, when the planner tried to estimate how many entries could be stored in the cache, it didn't take into account that the cache key must also be stored. In many cases, this won't make a large difference as the key is likely small in comparison to the tuple(s) being stored, however, it's not impossible to craft cases where the key could take more memory than the tuple(s) stored for it. Here we adjust the planner so it takes into account the estimated amount of memory to store the cache key. Effectively, this change will reduce the estimated cache hit ratio when it's thought that not all items will fit in the cache, thus Memoize will become more expensive in such cases. The executor already takes into account the memory consumed by the cache key, so here we only need to adjust the planner. Discussion: https://postgr.es/m/CAApHDvqGErGuyBfQvBQrTCHDbzLTqoiW=_G9sOzeFxWEc_7auA@mail.gmail.com
* Fix memory leak in Memoize cache key evaluationDavid Rowley2023-03-20
| | | | | | | | | | | | | | | | | | When probing the Memoize cache to check if the current cache key values exist in the cache, we perform an evaluation of the expressions making up the cache key before probing the hash table for those values. This operation could leak memory as it is possible that the cache key is an expression which requires allocation of memory, as was the case in bug 17844. Here we fix this by correctly switching to the per tuple context before evaluating the cache expressions so that the memory is freed next time the per tuple context is reset. Bug: 17844 Reported-by: Alexey Ermakov Discussion: https://postgr.es/m/17844-d2f6f9e75a622bed@postgresql.org Backpatch-through: 14, where Memoize was introduced
* Avoid copying undefined data in _readA_Const().Tom Lane2023-03-19
| | | | | | | | | | | | | | nodeRead() will have created a Node struct that's only allocated big enough for the specific node type, so copying sizeof(union ValUnion) can be copying too much. This provokes valgrind complaints, and with very bad luck could perhaps result in SIGSEGV. While at it, tidy up _equalA_Const to avoid duplicate checks of isnull. Per report from Alexander Lakhin. This code is new as of a6bc33019, so no need to back-patch. Discussion: https://postgr.es/m/4995256b-cc65-170e-0b22-60ad2cd535f1@gmail.com
* Add functions to do timestamptz arithmetic in a non-default timezone.Tom Lane2023-03-18
| | | | | | | | | | | | | | | | | | Add versions of timestamptz + interval, timestamptz - interval, and generate_series(timestamptz, ...) in which a timezone can be specified explicitly instead of defaulting to the TimeZone GUC setting. The new functions for the first two are named date_add and date_subtract. This might seem too generic, but we could use overloading to add additional variants if that seems useful. Along the way, improve the docs' pretty inadequate explanation of how timestamptz +- interval works. Przemysław Sztoch and Gurjeet Singh; cosmetic changes and most of the docs work by me Discussion: https://postgr.es/m/01a84551-48dd-1359-bf7e-f6b0203a6bd0@sztoch.pl
* Refactor datetime functions' timezone lookup code to reduce duplication.Tom Lane2023-03-17
| | | | | | | | | | | We already had five copies of essentially the same logic, and an upcoming patch introduces yet another use-case. That's past my threshold of pain, so introduce a common subroutine. There's not that much net code savings, but the chance of typos should go down. Inspired by a patch from Przemysław Sztoch, but different in detail. Discussion: https://postgr.es/m/01a84551-48dd-1359-bf7e-f6b0203a6bd0@sztoch.pl
* Fix typoPeter Eisentraut2023-03-17
| | | | | | Introduced in de4d456b40. Reported-by: Erik Rijkers <er@xs4all.nl>
* Fix t_isspace(), etc., when datlocprovider=i and datctype=C.Jeff Davis2023-03-17
| | | | | | | | | | | | | Check whether the datctype is C to determine whether t_isspace() and related functions use isspace() or iswspace(). Previously, t_isspace() checked whether the database default collation was C; which is incorrect when the default collation uses the ICU provider. Discussion: https://postgr.es/m/79e4354d9eccfdb00483146a6b9f6295202e7890.camel@j-davis.com Reviewed-by: Peter Eisentraut Backpatch-through: 15
* Improve several permission-related error messages.Peter Eisentraut2023-03-17
| | | | | | | | | Mainly move some detail from errmsg to errdetail, remove explicit mention of superuser where appropriate, since that is implied in most permission checks, and make messages more uniform. Author: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://www.postgresql.org/message-id/20230316234701.GA903298@nathanxps13
* Add macros for ReorderBufferTXN toptxn.Amit Kapila2023-03-17
| | | | | | | | | | Currently, there are quite a few places in reorderbuffer.c that tries to access top-transaction for a subtransaction. This makes the code to access top-transaction consistent and easier to follow. Author: Peter Smith Reviewed-by: Vignesh C, Sawada Masahiko Discussion: https://postgr.es/m/CAHut+PuCznOyTqBQwjRUu-ibG-=KHyCv-0FTcWQtZUdR88umfg@mail.gmail.com
* Fix incorrect logic for determining safe WindowAgg run conditionsDavid Rowley2023-03-17
| | | | | | | | | | | | | | | The logic added in 9d9c02ccd to determine when a qual can be used as a WindowClause run condition failed to correctly check for subqueries in the qual. This was being done correctly for normal subquery qual pushdowns, it's just that 9d9c02ccd failed to follow the lead on that. This also fixes various other cases where transforming the qual into a WindowClause run condition in the subquery should have been disallowed. Bug: #17826 Reported-by: Anban Company Discussion: https://postgr.es/m/17826-7d8750952f19a5f5@postgresql.org Backpatch-through: 15, where 9d9c02ccd was introduced.
* Small tidyup for commit d41a178b, part II.Thomas Munro2023-03-17
| | | | | | | | | | | Further to commit 6a9229da, checking for NULL is now redundant. An "out of memory" error would have been thrown already by palloc() and treated as FATAL, so we can delete a few more lines. Back-patch to all releases, like those other commits. Reported-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/4040668.1679013388%40sss.pgh.pa.us
* Enable use of Memoize atop an Append that came from UNION ALL.Tom Lane2023-03-16
| | | | | | | | | | | | | | | | | create_append_path() would only apply get_baserel_parampathinfo when the path is for a partitioned table, but it's also potentially useful for paths for UNION ALL appendrels. Specifically, that supports building a Memoize path atop this one. While we're in the vicinity, delete some dead code in create_merge_append_plan(): there's no need for it to support parameterized MergeAppend paths, and it doesn't look like that is going to change anytime soon. It'll be easy enough to undo this when/if it becomes useful. Richard Guo Discussion: https://postgr.es/m/CAMbWs4_ABSu4PWG2rE1q10tJugEXHWgru3U8dAgkoFvgrb6aEA@mail.gmail.com
* Work around spurious compiler warning in inet operatorsAndres Freund2023-03-16
| | | | | | | | | | | | | | | | | | | | | | | | | | gcc 12+ has complaints like the following: ../../../../../pgsql/src/backend/utils/adt/network.c: In function 'inetnot': ../../../../../pgsql/src/backend/utils/adt/network.c:1893:34: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=] 1893 | pdst[nb] = ~pip[nb]; | ~~~~~~~~~^~~~~~~~~~ ../../../../../pgsql/src/include/utils/inet.h:27:23: note: at offset -1 into destination object 'ipaddr' of size 16 27 | unsigned char ipaddr[16]; /* up to 128 bits of address */ | ^~~~~~ ../../../../../pgsql/src/include/utils/inet.h:27:23: note: at offset -1 into destination object 'ipaddr' of size 16 This is due to a compiler bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104986 It has been a year since the bug has been reported without getting fixed. As the warnings are verbose and use of gcc 12 is becoming more common, it seems worth working around the bug. Particularly because a simple reformulation of the loop condition fixes the issue and isn't any less readable. Author: Tom Lane <tgl@sss.pgh.pa.us> Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/144536.1648326206@sss.pgh.pa.us Backpatch: 11-
* Small tidyup for commit d41a178b.Thomas Munro2023-03-17
| | | | | | | | | | | A comment was left behind claiming that we needed to use malloc() rather than palloc() because the corresponding free would run in another thread, but that's not true anymore. Remove that comment. And, with the reason being gone, we might as well actually use palloc(). Back-patch to supported releases, like d41a178b. Discussion: https://postgr.es/m/CA%2BhUKG%2BpdM9v3Jv4tc2BFx2jh_daY3uzUyAGBhtDkotEQDNPYw%40mail.gmail.com
* Tighten error checks in datetime input, and remove bogus "ISO" format.Tom Lane2023-03-16
| | | | | | | | | | | | | | | | | | | | | | | | DecodeDateTime and DecodeTimeOnly had support for date input in the style "Y2023M03D16", which the comments claimed to be an "ISO" format. However, so far as I can find there is no such format in ISO 8601; they write units before numbers in intervals, but not in datetimes. Furthermore, the lesser-known ISO 8601-2 spec actually defines an incompatible format "2023Y03M16D". None of our documentation mentions such a format either. So let's just drop it. That leaves us with only two cases for a prefix unit specifier in datetimes: Julian dates written as Jnnnn, and the "T" separator defined by ISO 8601. Add checks to catch misuse of these specifiers, that is consecutive specifiers or a dangling specifier at the end of the string. We do not however disallow a specifier that is separated from the field that it disambiguates (by noise words or unrelated fields). That being the case, remove some overly-aggressive error checks from the ISOTIME cases. Joseph Koshakow, editorialized a bit by me; thanks also to Peter Eisentraut for some standards-reading. Discussion: https://postgr.es/m/CAAvxfHf2Q1gKLiHGnuPOiyf0ASvKUM4BnMfsXuwgtYEb_Gx0Zw@mail.gmail.com
* Use "data directory" not "current directory" in error messages.Tom Lane2023-03-16
| | | | | | | | | | | The user receiving the message might not understand where the server's "current directory" is. "Data directory" seems clearer. (This would not be good for frontend code, but both of these messages are only issued in the backend.) Kyotaro Horiguchi Discussion: https://postgr.es/m/20230316.111646.1564684434328830712.horikyota.ntt@gmail.com
* Integrate superuser check into has_rolreplication()Peter Eisentraut2023-03-16
| | | | | | | | | This makes it consistent with similar functions like has_createrole_privilege() and allows removing some explicit superuser checks. Author: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://www.postgresql.org/message-id/20230310000313.GA3992372%40nathanxps13
* Small code simplificationPeter Eisentraut2023-03-16
| | | | | Author: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://www.postgresql.org/message-id/20230310000313.GA3992372%40nathanxps13
* Remove PgStat_BackendFunctionEntryMichael Paquier2023-03-16
| | | | | | | | | | This structure included only PgStat_FunctionCounts, and removing it facilitates some upcoming refactoring for pgstatfuncs.c to use more macros rather that mostly-duplicated functions. Author: Bertrand Drouvot Reviewed-by: Nathan Bossart Discussion: https://postgr.es/m/11d531fe-52fc-c6ea-7e8e-62f1b6ec626e@gmail.com
* Don't try to read default for a non-existent attributeAndrew Dunstan2023-03-15
| | | | | | Oversight in commit 9f8377f7a2 for COPY .. DEFAULT per report from Alexander Lakhin
* Support [NO] INDENT option in XMLSERIALIZE().Tom Lane2023-03-15
| | | | | | | | | | | | | | This adds the ability to pretty-print XML documents ... according to libxml's somewhat idiosyncratic notions of what's pretty, anyway. One notable divergence from a strict reading of the spec is that libxml is willing to collapse empty nodes "<node></node>" to just "<node/>", whereas SQL and the underlying XML spec say that this option should only result in whitespace tweaks. Nonetheless, it seems close enough to justify using the SQL-standard syntax. Jim Jones, reviewed by Peter Smith and myself Discussion: https://postgr.es/m/2f5df461-dad8-6d7d-4568-08e10608a69b@uni-muenster.de
* Add a hook for modifying the ldapbind passwordAndrew Dunstan2023-03-15
| | | | | | | | | | | The hook can be installed by a shared_preload library. A similar mechanism could be used for radius paswords, for example, and the type name auth_password_hook_typ has been shosen with that in mind. John Naylor and Andrew Dunstan Discussion: https://postgr.es/m/469b06ed-69de-ba59-c13a-91d2372e52a9@dunslane.net
* Support PlaceHolderVars in MERGE actions.Tom Lane2023-03-15
| | | | | | | | | | | | | preprocess_targetlist thought PHVs couldn't appear here. It was mistaken, as per report from Önder Kalacı. Surveying other pull_var_clause calls, I noted no similar errors, but I did notice that qual_is_pushdown_safe's assertion about !contain_window_function was pointless, because the following pull_var_clause call would complain about them anyway. In HEAD only, remove the redundant Assert and improve the commentary. Discussion: https://postgr.es/m/CACawEhUuum-gC_2S3sXLTcsk7bUSPSHOD+g1ZpfKaDK-KKPPWA@mail.gmail.com
* Allow the use of indexes other than PK and REPLICA IDENTITY on the subscriber.Amit Kapila2023-03-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Using REPLICA IDENTITY FULL on the publisher can lead to a full table scan per tuple change on the subscription when REPLICA IDENTITY or PK index is not available. This makes REPLICA IDENTITY FULL impractical to use apart from some small number of use cases. This patch allows using indexes other than PRIMARY KEY or REPLICA IDENTITY on the subscriber during apply of update/delete. The index that can be used must be a btree index, not a partial index, and it must have at least one column reference (i.e. cannot consist of only expressions). We can uplift these restrictions in the future. There is no smart mechanism to pick the index. If there is more than one index that satisfies these requirements, we just pick the first one. We discussed using some of the optimizer's low-level APIs for this but ruled it out as that can be a maintenance burden in the long run. This patch improves the performance in the vast majority of cases and the improvement is proportional to the amount of data in the table. However, there could be some regression in a small number of cases where the indexes have a lot of duplicate and dead rows. It was discussed that those are mostly impractical cases but we can provide a table or subscription level option to disable this feature if required. Author: Onder Kalaci, Amit Kapila Reviewed-by: Peter Smith, Shi yu, Hou Zhijie, Vignesh C, Kuroda Hayato, Amit Kapila Discussion: https://postgr.es/m/CACawEhVLqmAAyPXdHEPv1ssU2c=dqOniiGz7G73HfyS7+nGV4w@mail.gmail.com
* Fix fractional vacuum_cost_delay.Thomas Munro2023-03-15
| | | | | | | | | | | | | | | | | | | | | | | Commit 4753ef37 changed vacuum_delay_point() to use the WaitLatch() API, to fix the problem that vacuum could keep running for a very long time after the postmaster died. Unfortunately, that broke commit caf626b2's support for fractional vacuum_cost_delay, which shipped in PostgreSQL 12. WaitLatch() works in whole milliseconds. For now, revert the change from commit 4753ef37, but add an explicit check for postmaster death. That's an extra system call on systems other than Linux and FreeBSD, but that overhead doesn't matter much considering that we willingly went to sleep and woke up again. (In later work, we might add higher resolution timeouts to the latch API so that we could do this with our standard programming pattern, but that wouldn't be back-patched.) Back-patch to 14, where commit 4753ef37 arrived. Reported-by: Melanie Plageman <melanieplageman@gmail.com> Discussion: https://postgr.es/m/CAAKRu_b-q0hXCBUCAATh0Z4Zi6UkiC0k2DFgoD3nC-r3SkR3tg%40mail.gmail.com
* Fix waitpid() emulation on Windows.Thomas Munro2023-03-15
| | | | | | | | | | | | | | | | | | | | | | | | | Our waitpid() emulation didn't prevent a PID from being recycled by the OS before the call to waitpid(). The postmaster could finish up tracking more than one child process with the same PID, and confuse them. Fix, by moving the guts of pgwin32_deadchild_callback() into waitpid(), so that resources are released synchronously. The process and PID continue to exist until we close the process handle, which only happens once we're ready to adjust our book-keeping of running children. This seems to explain a couple of failures on CI. It had never been reported before, despite the code being as old as the Windows port. Perhaps Windows started recycling PIDs more rapidly, or perhaps timing changes due to commit 7389aad6 made it more likely to break. Thanks to Alexander Lakhin for analysis and Andres Freund for tracking down the root cause. Back-patch to all supported branches. Reported-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/20230208012852.bvkn2am4h4iqjogq%40awork3.anarazel.de
* Fix corner case bug in numeric to_char() some more.Tom Lane2023-03-14
| | | | | | | | | | | | | | | The band-aid applied in commit f0bedf3e4 turns out to still need some work: it made sure we didn't set Np->last_relevant too small (to the left of the decimal point), but it didn't prevent setting it too large (off the end of the partially-converted string). This could result in fetching data beyond the end of the allocated space, which with very bad luck could cause a SIGSEGV, though I don't see any hazard of interesting memory disclosure. Per bug #17839 from Thiago Nunes. The bug's pretty ancient, so back-patch to all supported versions. Discussion: https://postgr.es/m/17839-aada50db24d7b0da@postgresql.org
* Remove unnecessary code in dependency_is_compatible_expression().Tom Lane2023-03-14
| | | | | | | | | | | | | | | | Scanning the expression for compatible Vars isn't really necessary, because the subsequent match against StatisticExtInfo entries will eliminate expressions containing other Vars just fine. Moreover, this code hadn't stopped to think about what to do with PlaceHolderVars or Aggrefs in the clause; and at least for the PHV case, that demonstrably leads to failures. Rather than work out whether it's reasonable to ignore those, let's just remove the whole stanza. Per report from Richard Guo. Back-patch to v14 where this code was added. Discussion: https://postgr.es/m/CAMbWs48Mmvm-acGevXuwpB=g5JMqVSL6i9z5UaJyLGJqa-XPAA@mail.gmail.com
* Add support for the error functions erf() and erfc().Dean Rasheed2023-03-14
| | | | | | | | | | | | | | | | | | | | | Expose the standard error functions as SQL-callable functions. These are expected to be useful to people working with normal distributions, and we use them here to test the distribution from random_normal(). Since these functions are defined in the POSIX and C99 standards, they should in theory be available on all supported platforms. If that turns out not to be the case, more work will be needed. On all platforms tested so far, using extra_float_digits = -1 in the regression tests is sufficient to allow for variations between implementations. However, past experience has shown that there are almost certainly going to be additional unexpected portability issues, so these tests may well need further adjustments, based on the buildfarm results. Dean Rasheed, reviewed by Nathan Bossart and Thomas Munro. Discussion: https://postgr.es/m/CAEZATCXv5fi7+Vu-POiyai+ucF95+YMcCMafxV+eZuN1B-=MkQ@mail.gmail.com
* Fix JSON error reporting for many cases of erroneous string values.Tom Lane2023-03-13
| | | | | | | | | | | | | | | | | | | | | | | The majority of error exit cases in json_lex_string() failed to set lex->token_terminator, causing problems for the error context reporting code: it would see token_terminator less than token_start and do something more or less nuts. In v14 and up the end result could be as bad as a crash in report_json_context(). Older versions accidentally avoided that fate; but all versions produce error context lines that are far less useful than intended, because they'd stop at the end of the prior token instead of continuing to where the actually-bad input is. To fix, invent some macros that make it less notationally painful to do the right thing. Also add documentation about what the function is actually required to do; and in >= v14, add an assertion in report_json_context about token_terminator being sufficiently far advanced. Per report from Nikolay Shaplov. Back-patch to all supported versions. Discussion: https://postgr.es/m/7332649.x5DLKWyVIX@thinkpad-pgpro