aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Add TAP tests for include directives in HBA end ident filesMichael Paquier2022-11-28
| | | | | | | | | | | | | | | | | | | | | This commit adds a basic set of authentication tests to check after the new keywords added by a54b658 for the HBA and ident files, aka "include", "include_if_exists" and "include_dir". This includes checks for all the positive cases originally proposed, where valid contents are generated for the HBA and ident files without any errors happening in the server, checking as well the contents of their respective system views. The error handling will be evaluated separately (-DEXEC_BACKEND makes that trickier), and what we have here covers most of the ground I would like to see covered if one manipulates the tokenization logic of hba.c in the future. While on it, some coverage is added for files included with '@' for database or user name lists. Author: Julien Rouhaud Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya@jrouhaud
* Fix binary mismatch for MSVC plperl vs gcc built perl libsAndrew Dunstan2022-11-27
| | | | | | | | | | | | | | | | When loading plperl built against Strawberry perl or the msys2 ucrt perl that have been built with gcc, a binary mismatch has been encountered which looks like this: loadable library and perl binaries are mismatched (got handshake key 0000000012800080, needed 0000000012900080) To cure this we bring the handshake keys into sync by adding NO_THREAD_SAFE_LOCALE to the defines used to build plperl. Discussion: https://postgr.es/m/20211005004334.tgjmro4kuachwiuc@alap3.anarazel.de Discussion: https://postgr.es/m/c2da86a0-2906-744c-923d-16da6047875e@dunslane.net Backpatch to all live branches.
* Mark two signal flags as sig_atomic_t in pgbench and pg_test_fsyncMichael Paquier2022-11-26
| | | | | | | | | | Two booleans used for timeout tracking were used within some SIGALRM signal handlers, but they were not declared as sig_atomic_t, so mark them as such. This has no consequence on WIN32 for both tools. Author: Ranier Vilela Reviewed-by: Michael Paquier, Masahiko Sawada Discussion: https://postgr.es/m/CAEudQArCDQQiPiFR16=yu9k5s2tp4tgEe1U1ZbkW4ofx81AWWQ@mail.gmail.com
* Fix typo in hba.cMichael Paquier2022-11-26
| | | | Spotted while reading through the git history.
* Allow building with MSVC and Strawberry perlAndrew Dunstan2022-11-25
| | | | | | | Strawberry uses __builtin_expect which Visual C doesn't have. For this case define it as a noop. Solution taken from vim sources. Backpatch to all live branches
* Correct some SQL feature namesPeter Eisentraut2022-11-25
| | | | | Maybe these were initially typos or they have been changed upstream since we first added them. This brings it all in line with SQL:2016.
* Fix gen_node_support.pl for changed AclMode sizeAndrew Dunstan2022-11-25
| | | | | | omitted from 7b378237aa, mea culpa. Complaint and fix from Amit Langote.
* Fix rule-detection code for MERGE.Dean Rasheed2022-11-25
| | | | | | | | | | | Use the relation's rd_rules structure to test whether it has rules, rather than the relhasrules flag, which might be out of date. Reviewed by Tom Lane. Backpatch to 15, where MERGE was added. Discussion: https://postgr.es/m/CAEZATCVkBVZABfw71sYvkcPf6tarcOFST5Bc6AOi-LFT9YdccQ%40mail.gmail.com
* Fix handling of pending inserts in nodeModifyTable.c.Etsuro Fujita2022-11-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | Commit b663a4136, which allowed FDWs to INSERT rows in bulk, added to nodeModifyTable.c code to flush pending inserts to the foreign-table result relation(s) before completing processing of the ModifyTable node, but the code failed to take into account the case where the INSERT query has modifying CTEs, leading to incorrect results. Also, that commit failed to flush pending inserts before firing BEFORE ROW triggers so that rows are visible to such triggers. In that commit we scanned through EState's es_tuple_routing_result_relations or es_opened_result_relations list to find the foreign-table result relations to which pending inserts are flushed, but that would be inefficient in some cases. So to fix, 1) add a List member to EState to record the insert-pending result relations, and 2) modify nodeModifyTable.c so that it adds the foreign-table result relation to the list in ExecInsert() if appropriate, and flushes pending inserts properly using the list where needed. While here, fix a copy-and-pasteo in a comment in ExecBatchInsert(), which was added by that commit. Back-patch to v14 where that commit appeared. Discussion: https://postgr.es/m/CAPmGK16qutyCmyJJzgQOhfBq%3DNoGDqTB6O0QBZTihrbqre%2BoxA%40mail.gmail.com
* Skip TAP test for peer authentication if there are no unix-domain socketsMichael Paquier2022-11-25
| | | | | | | | | | Peer connections require support for local connections to work, but the test missed the same check as the other ones in this suite. The buildfarm does not run the authentication tests on Windows, and, more surprisingly, the CI with meson was already able to skip it. Author: Anton A. Melnikov Discussion: https://postgr.es/m/28b9d685-9590-45b1-fe87-358d61c6950a@inbox.ru
* Introduce variables for initial and max nesting depth on configuration filesMichael Paquier2022-11-25
| | | | | | | | | | The code has been assuming already in a few places that the initial recursion nesting depth is 0, and the recent changes in hba.c (mainly 783e8c6) have relies on this assumption in more places. The maximum recursion nesting level is assumed to be 10 for hba.c and GUCs. Author: Julien Rouhaud Discussion: https://postgr.es/m/20221124090724.n7amf5kpdhx6vb76@jrouhaud
* Fix some 32-bit shift warnings in MSVCDavid Rowley2022-11-25
| | | | | | | | | 7b378237a widened AclMode to 64 bits which resulted in 3 new additional warnings on MSVC. Here we make use of UINT64CONST to reassure the compiler that we do intend the bit shift expression to yield a 64-bit result. Discussion: https://postgr.es/m/CAApHDvo=pn01Y_3zASZZqn+cotF1c4QFCwWgk6MiF0VscaE5ug@mail.gmail.com
* Improve indenting in _hash_pgaddtupDavid Rowley2022-11-25
| | | | | | | | | The Assert added in d09dbeb9b came out rather ugly after having run pgindent on that code. Here we adjust things to use some local variables so that the Assert remains within the 80-character margin. Author: Ted Yu Discussion: https://postgr.es/m/CALte62wLSir1=x93Jf0xZvHaO009FEJfhVMFwnaR8q=csPP8kQ@mail.gmail.com
* Make multixact error message more explicitAlvaro Herrera2022-11-24
| | | | | | | | | | | | | | There are recent reports involving a very old error message that we have no history of hitting -- perhaps a recently introduced bug. Improve the error message in an attempt to improve our chances of investigating the bug. Per reports from Dimos Stamatakis and Bob Krier. Backpatch to 11. Discussion: https://postgr.es/m/CO2PR0801MB2310579F65529380A4E5EDC0E20A9@CO2PR0801MB2310.namprd08.prod.outlook.com Discussion: https://postgr.es/m/17518-04e368df5ad7f2ee@postgresql.org
* Add the database name to the ps display of logical WAL sendersMichael Paquier2022-11-24
| | | | | | | | | | | | | | | | Logical WAL senders display now as follows, gaining a database name: postgres: walsender USER DATABASE HOST(PORT) STATE Physical WAL senders show up the same, as of: postgres: walsender USER HOST(PORT) STATE This information was missing, hence it was not possible to know from ps if a WAL sender was a logical or a physical one, and on which database it is connected when it is logical. Author: Tatsuhiro Nakamori Reviewed-by: Fujii Masao, Bharath Rupireddy Discussion: https://postgr.es/m/36a3b137e82e0ea9fe7e4234f03b64a1@oss.nttdata.com
* Add support for file inclusions in HBA and ident configuration filesMichael Paquier2022-11-24
| | | | | | | | | | | | | | | | | | | | | | | | | pg_hba.conf and pg_ident.conf gain support for three record keywords: - "include", to include a file. - "include_if_exists", to include a file, ignoring it if missing. - "include_dir", to include a directory of files. These are classified by name (C locale, mostly) and need to be prefixed by ".conf", hence following the same rules as GUCs. This commit relies on the refactoring pieces done in efc9816, ad6c528, 783e8c6 and 1b73d0b, adding a small wrapper to build a list of TokenizedAuthLines (tokenize_include_file), and the code is shaped to offer some symmetry with what is done for GUCs with the same options. pg_hba_file_rules and pg_ident_file_mappings gain a new field called file_name, to track from which file a record is located, taking advantage of the addition of rule_number in c591300 to offer an organized view of the HBA or ident records loaded. Bump catalog version. Author: Julien Rouhaud Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya@jrouhaud
* Speedup hash index builds by skipping needless binary searchesDavid Rowley2022-11-24
| | | | | | | | | | | | | | | | | When building hash indexes using the spool method, tuples are added to the index page in hashkey order. Because of this, we can safely skip performing the binary search on the existing tuples on the page to find the location to insert the tuple based on its hashkey value. For this case, we can just always put the tuple at the end of the item array as the tuples will always arrive in hashkey order. Testing has shown that this can improve hash index build speeds by 5-15% with a unique set of integer values. Author: Simon Riggs Reviewed-by: David Rowley Tested-by: David Zhang, Tomas Vondra Discussion: https://postgr.es/m/CANbhV-GBc5JoG0AneUGPZZW3o4OK5LjBGeKe_icpC3R1McrZWQ@mail.gmail.com
* Create memory context for tokenization after opening top-level file in hba.cMichael Paquier2022-11-24
| | | | | | | | | | | | | | The memory context was created before attempting to open the first HBA or ident file, which would cause it to leak. This had no consequences for the system views for HBA and ident files, but this would cause memory leaks in the postmaster on reload if the initial HBA and/or ident files are missing, which is a valid behavior while the backend is running. Oversight in efc9816. Author: Ted Yu Discussion: https://postgr.es/m/CALte62xH6ivgiKKzPRJgfekPZC6FKLB3xbnf3=tZmc_gKj78dw@mail.gmail.com
* Add missing initialization in tokenize_expand_file() for output listMichael Paquier2022-11-24
| | | | | | | | This should have been added in efc9816, but it looks like I have found a way to mess up a bit a patch split. This should have no consequence in practice, but let's be clean. Discussion: https://postgr.es/m/Y324HvGKiWxW2yxe@paquier.xyz
* Rework memory contexts in charge of HBA/ident tokenizationMichael Paquier2022-11-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The list of TokenizedAuthLines generated at parsing for the HBA and ident files is now stored in a static context called tokenize_context, where only all the parsed tokens are stored. This context is created when opening the first authentication file of a HBA/ident set (hba_file or ident_file), and is cleaned up once we are done all the work around it through a new routine called free_auth_file(). One call of open_auth_file() should have one matching call of free_auth_file(), the creation and deletion of the tokenization context is controlled by the recursion depth of the tokenization. Rather than having tokenize_auth_file() return a memory context that includes all the records, the tokenization logic now creates and deletes one memory context each time this function is called. This will simplify recursive calls to this routine for the upcoming inclusion record logic. While on it, rename tokenize_inc_file() to tokenize_expand_file() as this would conflict with the upcoming patch that will add inclusion records for HBA/ident files. An '@' file has its tokens added to an existing list. Reloading HBA/indent configuration in a tight loop shows no leaks, as of one type of test done (with and without -DEXEC_BACKEND). Author: Michael Paquier Reviewed-by: Julien Rouhaud Discussion: https://postgr.es/m/Y324HvGKiWxW2yxe@paquier.xyz
* Support for custom slots in the custom executor nodesAlexander Korotkov2022-11-24
| | | | | | | | | | | Some custom table access method may have their tuple format and use custom executor nodes for their custom scan types. The ability to set a custom slot would save them from tuple format conversion. Other users of custom executor nodes may also benefit. Discussion: https://postgr.es/m/CAPpHfduJUU6ToecvTyRE_yjxTS80FyPpct4OHaLFk3OEheMTNA@mail.gmail.com Author: Alexander Korotkov Reviewed-by: Pavel Borisov
* Simplify WARNING messages from skipped vacuum/analyze on a tableAndrew Dunstan2022-11-23
| | | | | | | | | This will more easily accomodate adding new permissions for vacuum and analyze. Nathan Bossart following a suggestion from Kyotaro Horiguchi Discussion: https://postgr.es/m/20220726.104712.912995710251150228.horikyota.ntt@gmail.com
* Expand AclMode to 64 bitsAndrew Dunstan2022-11-23
| | | | | | | | | | | | | | We're running out of bits for new permissions. This change doubles the number of permissions we can accomodate from 16 to 32, so the forthcoming new ones for vacuum/analyze don't exhaust the pool. Nathan Bossart Reviewed by: Bharath Rupireddy, Kyotaro Horiguchi, Stephen Frost, Robert Haas, Mark Dilger, Tom Lane, Corey Huinker, David G. Johnston, Michael Paquier. Discussion: https://postgr.es/m/20220722203735.GB3996698@nathanxps13
* Simplify vacuum_set_xid_limits() signature.Peter Geoghegan2022-11-23
| | | | | | | | | | | | Pass VACUUM parameters (VacuumParams state) to vacuum_set_xid_limits() directly, rather than passing most individual VacuumParams fields as separate arguments. Also make vacuum_set_xid_limits() output parameter symbol names match those used by its vacuumlazy.c caller. Author: Peter Geoghegan <pg@bowt.ie> Discussion: https://postgr.es/m/CAH2-Wz=TE7gW5DgSahDkf0UEZigFGAoHNNN6EvSrdzC=Kn+hrA@mail.gmail.com
* Don't test HEAP_XMAX_INVALID when freezing xmax.Peter Geoghegan2022-11-23
| | | | | | | | | | | | | | | | | | | | We shouldn't ever need to rely on whether HEAP_XMAX_INVALID is set in t_infomask when considering whether or not an xmax should be deemed already frozen, since that status flag is just a hint. The only acceptable representation for an "xmax_already_frozen" raw xmax field is the transaction ID value zero (also known as InvalidTransactionId). Adjust code that superficially appeared to rely on HEAP_XMAX_INVALID to make the rule about xmax_already_frozen clear. Also avoid needlessly rereading the tuple's raw xmax. Oversight in bugfix commit d2599ecf. There is no evidence that this ever led to incorrect behavior, so no backpatch. The worst consequence of this bug was that VACUUM could hypothetically fail to notice and report on certain kinds of corruption, which seems fairly benign. Author: Peter Geoghegan <pg@bowt.ie> Discussion: https://postgr.es/m/CAH2-Wzkh3DMCDRPfhZxj9xCq9v3WmzvmbiCpf1dNKUBPadhCbQ@mail.gmail.com
* Fix perl warning from commit 9b4eafcaf4Andrew Dunstan2022-11-23
| | | | | | per gripe from Andres Freund and Tom Lane Backpatch to all live branches.
* Give better hints for ambiguous or unreferenceable columns.Tom Lane2022-11-22
| | | | | | | | | | | | | | | | | | | | | | | | | Examine ParseNamespaceItem flags to detect whether a column name is unreferenceable for lack of LATERAL, or could be referenced if a qualified name were used, and give better hints for such cases. Also, don't phrase the message to imply that there's only one matching column when there is really more than one. Many of the regression test output changes are not very interesting, but just reflect reclassifying the "There is a column ... but it cannot be referenced from this part of the query" messages as DETAIL rather than HINT. They are details per our style guide, in the sense of being factual rather than offering advice; and this change provides room to offer actual HINTs about what to do. While here, adjust the fuzzy-name-matching code to be a shade less impenetrable. It was overloading the meanings of FuzzyAttrMatchState fields way too much IMO, so splitting them into multiple fields seems to make it clearer. It's not like we need to shave bytes in that struct. Per discussion of bug #17233 from Alexander Korolev. Discussion: https://postgr.es/m/17233-afb9d806aaa64b17@postgresql.org
* YA attempt at taming worst-case behavior of get_actual_variable_range.Tom Lane2022-11-22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | We've made multiple attempts at preventing get_actual_variable_range from taking an unreasonable amount of time (3ca930fc3, fccebe421). But there's still an issue for the very first planning attempt after deletion of a large number of extremal-valued tuples. While that planning attempt will set "killed" bits on the tuples it visits and thereby reduce effort for next time, there's still a lot of work it has to do to visit the heap and then set those bits. It's (usually?) not worth it to do that much work at plan time to have a slightly better estimate, especially in a context like this where the table contents are known to be mutating rapidly. Therefore, let's bound the amount of work to be done by giving up after we've visited 100 heap pages. Giving up just means we'll fall back on the extremal value recorded in pg_statistic, so it shouldn't mean that planner estimates suddenly become worthless. Note that this means we'll still gradually whittle down the problem by setting a few more index "killed" bits in each planning attempt; so eventually we'll reach a good state (barring further deletions), even in the absence of VACUUM. Simon Riggs, per a complaint from Jakub Wartak (with cosmetic adjustments by me). Back-patch to all supported branches. Discussion: https://postgr.es/m/CAKZiRmznOwi0oaV=4PHOCM4ygcH4MgSvt8=5cu_vNCfc8FSUug@mail.gmail.com
* Remove useless MERGE testAlvaro Herrera2022-11-22
| | | | | | | | | This was trying to exercise an ERROR we don't actually have. Backpatch to 15. Reported by Teja Mupparti <Tejeswar.Mupparti@microsoft.com> Discussion: https://postgr.es/m/SN6PR2101MB1040BDAF740EA4389484E92BF0079@SN6PR2101MB1040.namprd21.prod.outlook.com
* Improve comments atop pg_get_replication_slots.Amit Kapila2022-11-22
| | | | | | | | Update comments atop pg_get_replication_slots to make it clear that it shows all replication slots that currently exist on the database cluster. Author: sirisha chamarthi Discussion: https://postgr.es/m/CAKrAKeXRuFpeiWS+STGFm-RFfW19sUDxju66JkyRi13kdQf94Q@mail.gmail.com
* Ignore invalidated slots while computing oldest catalog XminAlvaro Herrera2022-11-22
| | | | | | | | | | | | | | | | | Once a logical slot has acquired a catalog_xmin, it doesn't let go of it, even when invalidated by exceeding the max_slot_wal_keep_size, which means that dead catalog tuples are not removed by vacuum anymore since the point is invalidated, until the slot is dropped. This could be catastrophic if catalog churn is high. Change the computation of Xmin to ignore invalidated slots, to prevent dead rows from accumulating. Backpatch to 13, where slot invalidation appeared. Author: Sirisha Chamarthi <sirichamarthi22@gmail.com> Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Discussion: https://postgr.es/m/CAKrAKeUEDeqquN9vwzNeG-CN8wuVsfRYbeOUV9qKO_RHok=j+g@mail.gmail.com
* Add wait event for pg_usleep() in perform_spin_delay()Andres Freund2022-11-21
| | | | | | | | | | | | | | | | | | | | The lwlock wait queue scalability issue fixed in a4adc31f690 was quite hard to find because of the exponential backoff and because we adjust spins_per_delay over time within a backend. To make it easier to find similar issues in the future, add a wait event for the pg_usleep() in perform_spin_delay(). Showing a wait event while spinning without sleeping would increase the overhead of spinlocks, which we do not want. We may at some later point want to have more granular wait events, but that'd be a substantial amount of work. This provides at least some insights into something currently hard to observe. Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Robert Haas <robertmhaas@gmail.com> Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com> https://postgr.es/m/20221120204310.xywrhyxyytsajuuq@awork3.anarazel.de
* Replace link to Hunspell with the current homepageDaniel Gustafsson2022-11-21
| | | | | | | | | The Hunspell project moved from Sourceforge to Github sometime in 2016, so update our links to match the new URL. Backpatch the doc changes to all supported versions. Discussion: https://postgr.es/m/DC9A662A-360D-4125-A453-5A6CB9C6C4B4@yesql.se Backpatch-through: v11
* Add comments and a missing CHECK_FOR_INTERRUPTS in ts_headline.Tom Lane2022-11-21
| | | | | | | | | | | | | | | | I just spent an annoying amount of time reverse-engineering the 100%-undocumented API between ts_headline and the text search parser's prsheadline function. Add some commentary about that while it's fresh in mind. Also remove some unused macros in wparser_def.c. While at it, I noticed that when commit 78e73e875 added a CHECK_FOR_INTERRUPTS call in TS_execute_recurse, it missed doing so in the parallel function TS_phrase_execute, which surely needs one just as much. Back-patch because of the missing CHECK_FOR_INTERRUPTS. Might as well back-patch the rest of this too.
* Add workaround to make ubsan and ps_status.c compatibleAndres Freund2022-11-21
| | | | | | | | | | | | | | | | | | | | At least on linux, set_ps_display() breaks /proc/$pid/environ. The sanitizer's helper library uses /proc/$pid/environ to implement getenv(), as it wants to work independent of libc. When just using undefined and alignment sanitizers, the sanitizer library is only initialized when the first error occurs, by which time we've often already called set_ps_display(), preventing the sanitizer libraries from seeing the options. We can work around that by defining __ubsan_default_options, a weak symbol libsanitizer uses to get defaults from the application, and return getenv("UBSAN_OPTIONS"). But only if main already was reached, so that we don't end up relying on a not-yet-working getenv(). As it's just a function that won't get called when not running a sanitizer, it doesn't seem necessary to make compilation of the function conditional. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/20220323173537.ll7klrglnp4gn2um@alap3.anarazel.de
* Provide options for postmaster to kill child processes with SIGABRT.Tom Lane2022-11-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The postmaster normally sends SIGQUIT to force-terminate its child processes after a child crash or immediate-stop request. If that doesn't result in child exit within a few seconds, we follow it up with SIGKILL. This patch provides GUC flags that allow either of these signals to be replaced with SIGABRT. On typically-configured Unix systems, that will result in a core dump being produced for each such child. This can be useful for debugging problems, although it's not something you'd want to have on in production due to the risk of disk space bloat from lots of core files. The old postmaster -T switch, which sent SIGSTOP in place of SIGQUIT, is changed to be the same as send_abort_for_crash. As far as I can tell from the code comments, the intent of that switch was just to block things for long enough to force core dumps manually, which seems like an unnecessary extra step. (Maybe at the time, there was no way to get most kernels to produce core files with per-PID names, requiring manual core file renaming after each one. But now it's surely the hard way.) I also took the opportunity to remove the old postmaster -n (skip shmem reinit) switch, which hasn't actually done anything in decades, though the documentation still claimed it did. Discussion: https://postgr.es/m/2251016.1668797294@sss.pgh.pa.us
* Replace SQLValueFunction by COERCE_SQL_SYNTAXMichael Paquier2022-11-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This switch impacts 9 patterns related to a SQL-mandated special syntax for function calls: - LOCALTIME [ ( typmod ) ] - LOCALTIMESTAMP [ ( typmod ) ] - CURRENT_TIME [ ( typmod ) ] - CURRENT_TIMESTAMP [ ( typmod ) ] - CURRENT_DATE Five new entries are added to pg_proc to compensate the removal of SQLValueFunction to provide backward-compatibility and making this change transparent for the end-user (for example for the attribute generated when a keyword is specified in a SELECT or in a FROM clause without an alias, or when specifying something else than an Iconst to the parser). The parser included a set of checks coming from the files in charge of holding the C functions used for the SQLValueFunction calls (as of transformSQLValueFunction()), which are now moved within each function's execution path, so this reduces the dependencies between the execution and the parsing steps. As of this change, all the SQL keywords use the same paths for their work, relying only on COERCE_SQL_SYNTAX. Like fb32748, no performance difference has been noticed, while the perf profiles get reduced with ExecEvalSQLValueFunction() gone. Bump catalog version. Reviewed-by: Corey Huinker, Ted Yu Discussion: https://postgr.es/m/YzaG3MoryCguUOym@paquier.xyz
* Add additional checks while creating the initial decoding snapshot.Amit Kapila2022-11-21
| | | | | | | | | | | | As per one of the CI reports, there is an assertion failure which indicates that we were trying to use an unenforced xmin horizon for decoding snapshots. Though, we couldn't figure out the reason for assertion failure these checks would help us in finding the reason if the problem happens again in the future. Author: Amit Kapila based on suggestions by Andres Freund Reviewd by: Andres Freund Discussion: https://postgr.es/m/CAA4eK1L8wYcyTPxNzPGkhuO52WBGoOZbT0A73Le=ZUWYAYmdfw@mail.gmail.com
* lwlock: Fix quadratic behavior with very long wait listsAndres Freund2022-11-20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Until now LWLockDequeueSelf() sequentially searched the list of waiters to see if the current proc is still is on the list of waiters, or has already been removed. In extreme workloads, where the wait lists are very long, this leads to a quadratic behavior. #backends iterating over a list #backends long. Additionally, the likelihood of needing to call LWLockDequeueSelf() in the first place also increases with the increased length of the wait queue, as it becomes more likely that a lock is released while waiting for the wait list lock, which is held for longer during lock release. Due to the exponential back-off in perform_spin_delay() this is surprisingly hard to detect. We should make that easier, e.g. by adding a wait event around the pg_usleep() - but that's a separate patch. The fix is simple - track whether a proc is currently waiting in the wait list or already removed but waiting to be woken up in PGPROC->lwWaiting. In some workloads with a lot of clients contending for a small number of lwlocks (e.g. WALWriteLock), the fix can substantially increase throughput. As the quadratic behavior arguably is a bug, we might want to decide to backpatch this fix in the future. Author: Andres Freund <andres@anarazel.de> Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Discussion: https://postgr.es/m/20221027165914.2hofzp4cvutj6gin@awork3.anarazel.de Discussion: https://postgr.es/m/CALj2ACXktNbG=K8Xi7PSqbofTZozavhaxjatVc14iYaLu4Maag@mail.gmail.com
* pgstat: replace double lookup with IsSharedRelation()Andres Freund2022-11-20
| | | | | | | | | | | As the list of shared relations is fixed, we can just dispatch based IsSharedRelation(), instead of first trying to look up stats for a non-shared rel and falling back to shared stats. Author: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com> Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Reviewed-by: Andres Freund <andres@anarazel.de Discussion: https://postgr.es/m/8c1851a2-a98e-e1bc-7729-37b0b95f66ec@gmail.com
* Fix sloppy cleanup of roles in privileges.sql.Tom Lane2022-11-20
| | | | | | | Commit 3d14e171e dropped regress_roleoption_donor twice and regress_roleoption_protagonist not at all. Leaving roles behind after "make installcheck" is unfriendly in its own right, plus it causes repeated runs of "make installcheck" to fail.
* Fix long-obsolete comment.Tom Lane2022-11-20
| | | | | | Commit c94959d41 fixed DROP OPERATOR to reset oprcom/oprnegate links to the dropped operator; but it missed updating this old comment that claimed we allow such links to dangle.
* Prevent port collisions between concurrent TAP testsAndrew Dunstan2022-11-20
| | | | | | | | | | | | | | | | | | | | | | | | Currently there is a race condition where if concurrent TAP tests both test that they can open a port they will assume that it is free and use it, causing one of them to fail. To prevent this we record a reservation using an exclusive lock, and any TAP test that discovers a reservation checks to see if the reserving process is still alive, and looks for another free port if it is. Ports are reserved in a directory set by the environment setting PG_TEST_PORT_DIR, or if that doesn't exist a subdirectory of the top build directory as set by meson or Makefile.global, or its own tmp_check directory. The prove_check recipe in Makefile.global.in is extended to export top_builddir to the TAP tests. This was already exported by the prove_installcheck recipes. Per complaint from Andres Freund This will be backpatched in due course after some testing. Discussion: https://postgr.es/m/20221002164931.d57hlutrcz4d2zi7@awork3.anarazel.de
* Switch SQLValueFunction on "name" to use COERCE_SQL_SYNTAXMichael Paquier2022-11-20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit changes six SQL keywords to use COERCE_SQL_SYNTAX rather than relying on SQLValueFunction: - CURRENT_ROLE - CURRENT_USER - USER - SESSION_USER - CURRENT_CATALOG - CURRENT_SCHEMA Among the six, "user", "current_role" and "current_catalog" require specific SQL functions to allow ruleutils.c to map them to the SQL keywords these require when using COERCE_SQL_SYNTAX. Having pg_proc.proname match with the keyword ensures that the compatibility remains the same when projecting any of these keywords in a FROM clause to an attribute name when an alias is not specified. This is covered by the tests added in 2e0d80c, making sure that a correct mapping happens with each SQL keyword. The three others (current_schema, session_user and current_user) already have pg_proc entries for this job, so this brings more consistency between the way such keywords are treated in the parser, the executor and ruleutils.c. SQLValueFunction is reduced to half its contents after this change, simplifying its logic a bit as there is no need to enforce a C collation anymore for the entries returning a name as a result. I have made a few performance tests, with a million-ish calls to these keywords without seeing a difference in run-time or in perf profiles (ExecEvalSQLValueFunction() is removed from the profiles). The remaining SQLValueFunctions are now related to timestamps and dates. Bump catalog version. Reviewed-by: Corey Huinker Discussion: https://postgr.es/m/YzaG3MoryCguUOym@paquier.xyz
* Fix catversionJoe Conway2022-11-19
| | | | | Commit 2fb6154fc didn't quite get the catversion correct per usual norms. Fix it. Reported by Rishu Bagga.
* Fix mislabeling of PROC_QUEUE->links as PGPROC, fixing UBSan on 32bitAndres Freund2022-11-19
| | | | | | | | | | | | | | | | | | | | | | | ProcSleep() used a PGPROC* variable to point to PROC_QUEUE->links.next, because that does "the right thing" with SHMQueueInsertBefore(). While that largely works, it's certainly not correct and unnecessary - we can just use SHM_QUEUE* to point to the insertion point. Noticed when testing a 32bit of postgres with undefined behavior sanitizer. UBSan noticed that sometimes the supposed PGPROC wasn't sufficiently aligned (required since 46d6e5f5679, ensured indirectly, via ShmemAllocRaw() guaranteeing cacheline alignment). For now fix this by using a SHM_QUEUE* for the insertion point. Subsequently we should replace all the use of PROC_QUEUE and SHM_QUEUE with ilist.h, but that's a larger change that we don't want to backpatch. Backpatch to all supported versions - it's useful to be able to run postgres under UBSan. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/20221117014230.op5kmgypdv2dtqsf@awork3.anarazel.de Backpatch: 11-
* Disable debug_discard_caches in test_oat_hooks test.Tom Lane2022-11-19
| | | | | | | | | | | | The test output varies when debug_discard_caches is enabled, because that causes extra executions of recomputeNamespacePath. Maybe putting a hook in that was a bad idea, but as a stopgap, just turn off debug_discard_caches in this test. Per buildfarm (now that we have debug_discard_caches coverage again). Back-patch to v15 where this module was added. Discussion: https://postgr.es/m/2267406.1668804934@sss.pgh.pa.us
* Doc: sync src/tutorial/basics.source with SGML documentation.Tom Lane2022-11-19
| | | | | | | | | | | basics.source is supposed to be pretty closely in step with the examples in chapter 2 of the tutorial, but I forgot to update it in commit f05a5e000. Fix that, and adjust a couple of other discrepancies that had crept in over time. (I notice that advanced.source is nowhere near being in sync with chapter 3, but I lack the ambition to do something about that right now.)
* Fix typos and bump catversion.Robert Haas2022-11-18
| | | | | | | Typos reported by Álvaro Herrera and Erik Rijkers. Catversion bump for 3d14e171e9e2236139e8976f3309a588bcc8683b was inadvertently omitted.
* Add a SET option to the GRANT command.Robert Haas2022-11-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Similar to how the INHERIT option controls whether or not the permissions of the granted role are automatically available to the grantee, the new SET permission controls whether or not the grantee may use the SET ROLE command to assume the privileges of the granted role. In addition, the new SET permission controls whether or not it is possible to transfer ownership of objects to the target role or to create new objects owned by the target role using commands such as CREATE DATABASE .. OWNER. We could alternatively have made this controlled by the INHERIT option, or allow it when either option is given. An advantage of this approach is that if you are granted a predefined role with INHERIT TRUE, SET FALSE, you can't go and create objects owned by that role. The underlying theory here is that the ability to create objects as a target role is not a privilege per se, and thus does not depend on whether you inherit the target role's privileges. However, it's surely something you could do anyway if you could SET ROLE to the target role, and thus making it contingent on whether you have that ability is reasonable. Design review by Nathan Bossat, Wolfgang Walther, Jeff Davis, Peter Eisentraut, and Stephen Frost. Discussion: http://postgr.es/m/CA+Tgmob+zDSRS6JXYrgq0NWdzCXuTNzT5eK54Dn2hhgt17nm8A@mail.gmail.com