aboutsummaryrefslogtreecommitdiff
path: root/src/backend/utils/adt
Commit message (Collapse)AuthorAge
* Add doubly linked count list implementationDavid Rowley2022-11-02
| | | | | | | | | | | | | | | | | | | | | | | | | | We have various requirements when using a dlist_head to keep track of the number of items in the list. This, traditionally, has been done by maintaining a counter variable in the calling code. Here we tidy this up by adding "dclist", which is very similar to dlist but also keeps track of the number of items stored in the list. Callers may use the new dclist_count() function when they need to know how many items are stored. Obtaining the count is an O(1) operation. For simplicity reasons, dclist and dlist both use dlist_node as their node type and dlist_iter/dlist_mutable_iter as their iterator type. dclists have all of the same functionality as dlists except there is no function named dclist_delete(). To remove an item from a list dclist_delete_from() must be used. This requires knowing which dclist the given item is stored in. Additionally, here we also convert some dlists where additional code exists to keep track of the number of items stored and to make these use dclists instead. Author: David Rowley Reviewed-by: Bharath Rupireddy, Aleksander Alekseev Discussion: https://postgr.es/m/CAApHDvrtVxr+FXEX0VbViCFKDGxA3tWDgw9oFewNXCJMmwLjLg@mail.gmail.com
* Fix planner failure with extended statistics on partitioned tables.Tom Lane2022-11-01
| | | | | | | | | | Some cases would result in "cache lookup failed for statistics object", due to trying to fetch inherited statistics when only non-inherited ones are available or vice versa. Richard Guo and Justin Pryzby Discussion: https://postgr.es/m/20221030170520.GM16921@telsasoft.com
* Clean up some inconsistencies with GUC declarationsMichael Paquier2022-10-31
| | | | | | | | | | | | | | | | | | | | This is similar to 7d25958, and this commit takes care of all the remaining inconsistencies between the initial value used in the C variable associated to a GUC and its default value stored in the GUC tables (as of pg_settings.boot_val). Some of the initial values of the GUCs updated rely on a compile-time default. These are refactored so as the GUC table and its C declaration use the same values. This makes everything consistent with other places, backend_flush_after, bgwriter_flush_after, port, checkpoint_flush_after doing so already, for example. Extracted from a larger patch by Peter Smith. The spots updated in the modules are from me. Author: Peter Smith, Michael Paquier Reviewed-by: Nathan Bossart, Tom Lane, Justin Pryzby Discussion: https://postgr.es/m/CAHut+PtHE0XSfjjRQ6D4v7+dqzCw=d+1a64ujra4EX8aoc_Z+w@mail.gmail.com
* Remove AssertArg and AssertStatePeter Eisentraut2022-10-28
| | | | | | | | | These don't offer anything over plain Assert, and their usage had already been declared obsolescent. Author: Nathan Bossart <nathandbossart@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/20221009210148.GA900071@nathanxps13
* Allow nodeSort to perform Datum sorts for byref typesDavid Rowley2022-10-28
| | | | | | | | | | | | | | | | | | | | | | | | Here we add a new 'copy' parameter to tuplesort_getdatum so that we can instruct the function not to datumCopy() byref Datums before returning. Similar to 91e9e89dc, this can provide significant performance improvements in nodeSort when sorting by a single byref column and the sort's targetlist contains only that column. This allows us to re-enable Datum sorts for byref types which was disabled in 3a5817695 due to a reported memory leak. Additionally, here we slightly optimize DISTINCT aggregates so that we no longer perform any datumCopy() when we find the current value not to be distinct from the previous value. Previously the code would always take a copy of the most recent Datum and pfree the previous value, even when the values were the same. Testing shows a small but noticeable performance increase when aggregate transitions are skipped due to the current transition value being the same as the prior one. Author: David Rowley Discussion: https://postgr.es/m/CAApHDvqS6wC5U==k9Hd26E4EQXH3QR67-T4=Q1rQ36NGvjfVSg@mail.gmail.com Discussion: https://postgr.es/m/CAApHDvqHonfe9G1cVaKeHbDx70R_zCrM3qP2AGXpGrieSKGnhA@mail.gmail.com
* Add rule_number to pg_hba_file_rules and map_number to pg_ident_file_mappingsMichael Paquier2022-10-26
| | | | | | | | | | | | | | | | | | | | These numbers are strictly-monotone identifiers assigned to each rule of pg_hba_file_rules and each map of pg_ident_file_mappings when loading the HBA and ident configuration files, indicating the order in which they are checked at authentication time, until a match is found. With only one file loaded currently, this is equivalent to the line numbers assigned to the entries loaded if one wants to know their order, but this becomes mandatory once the inclusion of external files is added to the HBA and ident files to be able to know in which order the rules and/or maps are applied at authentication. Note that NULL is used when a HBA or ident entry cannot be parsed or validated, aka when an error exists, contrary to the line number. Bump catalog version. Author: Julien Rouhaud Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya@jrouhaud
* Improve the accuracy of numeric power() for integer exponents.Dean Rasheed2022-10-20
| | | | | | | | | | | | | | | | | | | | | | | | | | This makes the choice of result scale of numeric power() for integer exponents consistent with the choice for non-integer exponents, and with the result scale of other numeric functions. Specifically, the result scale will be at least as large as the scale of either input, and sufficient to ensure that the result has at least 16 significant digits. Formerly, the result scale was based only on the scale of the first input, without taking into account the weight of the result. For results with negative weight, that could lead to results with very few or even no non-zero significant digits (e.g., 10.0 ^ (-18) produced 0.0000000000000000). Fix this by moving responsibility for the choice of result scale into power_var_int(), which already has code to estimate the result weight. Per report by Adrian Klaver and suggested fix by Tom Lane. No back-patch -- arguably this is a bug fix, but one which is easy to work around, so it doesn't seem worth the risk of changing query results in stable branches. Discussion: https://postgr.es/m/12a40226-70ac-3a3b-3d3a-fdaf9e32d312%40aklaver.com
* Refactor regular expression handling in hba.cMichael Paquier2022-10-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | AuthToken gains a regular expression, and IdentLine is changed so as it uses an AuthToken rather than tracking separately the ident user string used for the regex compilation and its generated regex_t. In the case of pg_ident.conf, a set of AuthTokens is built in the pre-parsing phase of the file, and an extra regular expression is compiled when building the list of IdentLines, after checking the sanity of the fields in a pre-parsed entry. The logic in charge of computing and executing regular expressions is now done in a new set of routines called respectively regcomp_auth_token() and regexec_auth_token() that are wrappers around pg_regcomp() and pg_regexec(), working on AuthTokens. While on it, this patch adds a routine able to free an AuthToken, free_auth_token(), to simplify a bit the logic around the requirement of using a specific free routine for computed regular expressions. Note that there are no functional or behavior changes introduced by this commit. The goal of this patch is to ease the use of regular expressions with more items of pg_hba.conf (user list, database list, potentially hostnames) where AuthTokens are used extensively. This will be tackled later in a separate patch. Author: Bertrand Drouvot, Michael Paquier Discussion: https://postgr.es/m/fff0d7c1-8ad4-76a1-9db3-0ab6ec338bf7@amazon.com
* Rename SetSingleFuncCall() to InitMaterializedSRF()Michael Paquier2022-10-18
| | | | | | | | | | | | | | | | | | Per discussion, the existing routine name able to initialize a SRF function with materialize mode is unpopular, so rename it. Equally, the flags of this function are renamed, as of: - SRF_SINGLE_USE_EXPECTED -> MAT_SRF_USE_EXPECTED_DESC - SRF_SINGLE_BLESS -> MAT_SRF_BLESS The previous function and flags introduced in 9e98583 are kept around for compatibility purposes, so as any extension code already compiled with v15 continues to work as-is. The declarations introduced here for compatibility will be removed from HEAD in a follow-up commit. The new names have been suggested by Andres Freund and Melanie Plageman. Discussion: https://postgr.es/m/20221013194820.ciktb2sbbpw7cljm@awork3.anarazel.de Backpatch-through: 15
* Fix EXPLAIN of SEARCH BREADTH FIRST with a constant initial value.Tom Lane2022-10-16
| | | | | | | | | | | | | | | | | | | | | | If the non-recursive term of a SEARCH BREADTH FIRST recursive query has only constants in its target list, the planner will fold the starting RowExpr added by rewrite into a simple Const of type RECORD. The executor doesn't have any problem with that --- but EXPLAIN VERBOSE will encounter the Const as the ultimate source of truth about what the field names of the SET column are, and it didn't know what to do with that. Fortunately, we can pull the identifying typmod out of the Const, in much the same way that record_out would. For reasons that remain a bit obscure to me, this only fails with SEARCH BREADTH FIRST, not SEARCH DEPTH FIRST or CYCLE. But I added regression test cases for both of those options too, just to make sure we don't break it in future. Per bug #17644 from Matthijs van der Vleuten. Back-patch to v14 where these constructs were added. Discussion: https://postgr.es/m/17644-3bd1f3036d6d7a16@postgresql.org
* pgstat: Track time of the last scan of a relationAndres Freund2022-10-14
| | | | | | | | | | | | | | | | | | | | | It can be useful to know when a relation has last been used, e.g., when evaluating whether an index is still required. It was already possible to infer the time of the last usage by tracking, e.g., pg_stat_all_indexes.idx_scan over time. But far from everybody does so. To make it easier to detect the last time a relation has been scanned, track that time in each relation's pgstat entry. To minimize overhead a) the timestamp is updated only when the backend pending stats entry is flushed to shared stats b) the last transaction's stop timestamp is used as the timestamp. Bumps catalog and stats format versions. Author: Dave Page <dpage@pgadmin.org> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Bruce Momjian <bruce@momjian.us> Reviewed-by: Vik Fearing <vik@postgresfriends.org> Discussion: https://postgr.es/m/CA+OCxozrVHNFVEPkweUHMZje+t1tfY816d9MZYc6eZwOOusOaQ@mail.gmail.com
* Store GUC data in a memory context, instead of using malloc().Tom Lane2022-10-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | The only real argument for using malloc directly was that we needed the ability to not throw error on OOM; but mcxt.c grew that feature awhile ago. Keeping the data in a memory context improves accountability and debuggability --- for example, without this it's almost impossible to detect memory leaks in the GUC code with anything less costly than valgrind. Moreover, the next patch in this series will add a hash table for GUC lookup, and it'd be pretty silly to be using palloc-dependent hash facilities alongside malloc'd storage of the underlying data. This is a bit invasive though, in particular causing an API break for GUC check hooks that want to modify the GUC's value or use an "extra" data structure. They must now use guc_malloc() and guc_free() instead of malloc() and free(). Failure to change affected code will result in assertion failures or worse; but thanks to recent effort in the mcxt infrastructure, it shouldn't be too hard to diagnose such oversights (at least in assert-enabled builds). One note is that this changes ParseLongOption() to return short-lived palloc'd not malloc'd data. There wasn't any caller for which the previous definition was better. Discussion: https://postgr.es/m/2982579.1662416866@sss.pgh.pa.us
* Use C library functions instead of Abs() for int64Peter Eisentraut2022-10-10
| | | | | | | | | | Instead of Abs() for int64, use the C standard functions labs() or llabs() as appropriate. Define a small wrapper around them that matches our definition of int64. (labs() is C90, llabs() is C99.) Reviewed-by: Zhang Mingli <zmlpostgres@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/4beb42b5-216b-bce8-d452-d924d5794c63%40enterprisedb.com
* Use fabsf() instead of Abs() or fabs() where appropriatePeter Eisentraut2022-10-08
| | | | | | | | This function is new in C99. Reviewed-by: Zhang Mingli <zmlpostgres@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/4beb42b5-216b-bce8-d452-d924d5794c63%40enterprisedb.com
* Remove unnecessary uses of Abs()Peter Eisentraut2022-10-07
| | | | | | | | Use C standard abs() or fabs() instead. Reviewed-by: Zhang Mingli <zmlpostgres@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/4beb42b5-216b-bce8-d452-d924d5794c63%40enterprisedb.com
* Introduce t_isalnum() to replace t_isalpha() || t_isdigit() tests.Tom Lane2022-10-06
| | | | | | | | | | | | ts_locale.c omitted support for "isalnum" tests, perhaps on the grounds that there were initially no use-cases for that. However, both ltree and pg_trgm need such tests, and we do also have one use-case now in the core backend. The workaround of testing isalpha and isdigit separately seems quite inefficient, especially when dealing with multibyte characters; so let's fill in the missing support. Discussion: https://postgr.es/m/2548310.1664999615@sss.pgh.pa.us
* Rename shadowed local variablesDavid Rowley2022-10-05
| | | | | | | | | | | | In a similar effort to f01592f91, here we mostly rename shadowed local variables to remove the warnings produced when compiling with -Wshadow=compatible-local. This fixes 63 warnings and leaves just 5. Author: Justin Pryzby, David Rowley Reviewed-by: Justin Pryzby Discussion https://postgr.es/m/20220817145434.GC26426%40telsasoft.com
* Revert "Optimize order of GROUP BY keys".Tom Lane2022-10-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This reverts commit db0d67db2401eb6238ccc04c6407a4fd4f985832 and several follow-on fixes. The idea of making a cost-based choice of the order of the sorting columns is not fundamentally unsound, but it requires cost information and data statistics that we don't really have. For example, relying on procost to distinguish the relative costs of different sort comparators is pretty pointless so long as most such comparator functions are labeled with cost 1.0. Moreover, estimating the number of comparisons done by Quicksort requires more than just an estimate of the number of distinct values in the input: you also need some idea of the sizes of the larger groups, if you want an estimate that's good to better than a factor of three or so. That's data that's often unknown or not very reliable. Worse, to arrive at estimates of the number of calls made to the lower-order-column comparison functions, the code needs to make estimates of the numbers of distinct values of multiple columns, which are necessarily even less trustworthy than per-column stats. Even if all the inputs are perfectly reliable, the cost algorithm as-implemented cannot offer useful information about how to order sorting columns beyond the point at which the average group size is estimated to drop to 1. Close inspection of the code added by db0d67db2 shows that there are also multiple small bugs. These could have been fixed, but there's not much point if we don't trust the estimates to be accurate in-principle. Finally, the changes in cost_sort's behavior made for very large changes (often a factor of 2 or so) in the cost estimates for all sorting operations, not only those for multi-column GROUP BY. That naturally changes plan choices in many situations, and there's precious little evidence to show that the changes are for the better. Given the above doubts about whether the new estimates are really trustworthy, it's hard to summon much confidence that these changes are better on the average. Since we're hard up against the release deadline for v15, let's revert these changes for now. We can always try again later. Note: in v15, I left T_PathKeyInfo in place in nodes.h even though it's unreferenced. Removing it would be an ABI break, and it seems a bit late in the release cycle for that. Discussion: https://postgr.es/m/TYAPR01MB586665EB5FB2C3807E893941F5579@TYAPR01MB5866.jpnprd01.prod.outlook.com
* Use actual backend IDs in pg_stat_get_backend_idset() and friends.Tom Lane2022-09-29
| | | | | | | | | | | | | | | | | | | | | | | Up to now, the ID values returned by pg_stat_get_backend_idset() and used by pg_stat_get_backend_activity() and allied functions were just indexes into a local array of sessions seen by the last stats refresh. This is problematic for a few reasons. The "ID" of a session can vary over its existence, which is surprising. Also, while these numbers often match the "backend ID" used for purposes like temp schema assignment, that isn't reliably true. We can fairly cheaply switch things around to make these numbers actually be the sessions' backend IDs. The added test case illustrates that with this definition, the temp schema used by a given session can be obtained given its PID. While here, delete some dead code that guarded against getting a NULL return from pgstat_fetch_stat_local_beentry(). That can't happen as long as the caller is careful to pass an in-range array index, as all the callers are. (This code may not have been dead when written, but it surely is now.) Nathan Bossart Discussion: https://postgr.es/m/20220815205811.GA250990@nathanxps13
* Introduce SYSTEM_USERMichael Paquier2022-09-29
| | | | | | | | | | | | | | | | | | | | | | | | SYSTEM_USER is a reserved keyword of the SQL specification that, roughly described, is aimed at reporting some information about the system user who has connected to the database server. It may include implementation-specific information about the means by the user connected, like an authentication method. This commit implements SYSTEM_USER as of auth_method:identity, where "auth_method" is a keyword about the authentication method used to log into the server (like peer, md5, scram-sha-256, gss, etc.) and "identity" is the authentication identity as introduced by 9afffcb (peer sets authn to the OS user name, gss to the user principal, etc.). This format has been suggested by Tom Lane. Note that thanks to d951052, SYSTEM_USER is available to parallel workers. Bump catalog version. Author: Bertrand Drouvot Reviewed-by: Jacob Champion, Joe Conway, Álvaro Herrera, Michael Paquier Discussion: https://postgr.es/m/7e692b8c-0b11-45db-1cad-3afc5b57409f@amazon.com
* Change some errdetail() to errdetail_internal()Alvaro Herrera2022-09-28
| | | | | | | | | | | | This prevents marking the argument string for translation for gettext, and it also prevents the given string (which is already translated) from being translated at runtime. Also, mark the strings used as arguments to check_rolespec_name for translation. Backpatch all the way back as appropriate. None of this is caught by any tests (necessarily so), so I verified it manually.
* Revert 56-bit relfilenode change and follow-up commits.Robert Haas2022-09-28
| | | | | | | | There are still some alignment-related failures in the buildfarm, which might or might not be able to be fixed quickly, but I've also just realized that it increased the size of many WAL records by 4 bytes because a block reference contains a RelFileLocator. The effect of that hasn't been studied or discussed, so revert for now.
* Increase width of RelFileNumbers from 32 bits to 56 bits.Robert Haas2022-09-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | RelFileNumbers are now assigned using a separate counter, instead of being assigned from the OID counter. This counter never wraps around: if all 2^56 possible RelFileNumbers are used, an internal error occurs. As the cluster is limited to 2^64 total bytes of WAL, this limitation should not cause a problem in practice. If the counter were 64 bits wide rather than 56 bits wide, we would need to increase the width of the BufferTag, which might adversely impact buffer lookup performance. Also, this lets us use bigint for pg_class.relfilenode and other places where these values are exposed at the SQL level without worrying about overflow. This should remove the need to keep "tombstone" files around until the next checkpoint when relations are removed. We do that to keep RelFileNumbers from being recycled, but now that won't happen anyway. However, this patch doesn't actually change anything in this area; it just makes it possible for a future patch to do so. Dilip Kumar, based on an idea from Andres Freund, who also reviewed some earlier versions of the patch. Further review and some wordsmithing by me. Also reviewed at various points by Ashutosh Sharma, Vignesh C, Amul Sul, Álvaro Herrera, and Tom Lane. Discussion: http://postgr.es/m/CA+Tgmobp7+7kmi4gkq7Y+4AM9fTvL+O1oQ4-5gFTT+6Ng-dQ=g@mail.gmail.com
* Harmonize more lexer function parameter names.Peter Geoghegan2022-09-22
| | | | | | | | | Make sure that function declarations use names that exactly match the corresponding names from function definitions for several "lexer adjacent" backend functions. These were missed by commit aab06442. Author: Peter Geoghegan <pg@bowt.ie> Discussion: https://postgr.es/m/CAH2-WznJt9CMM9KJTMjJh_zbL5hD9oX44qdJ4aqZtjFi-zA3Tg@mail.gmail.com
* meson: Add initial version of meson based build systemAndres Freund2022-09-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Autoconf is showing its age, fewer and fewer contributors know how to wrangle it. Recursive make has a lot of hard to resolve dependency issues and slow incremental rebuilds. Our home-grown MSVC build system is hard to maintain for developers not using Windows and runs tests serially. While these and other issues could individually be addressed with incremental improvements, together they seem best addressed by moving to a more modern build system. After evaluating different build system choices, we chose to use meson, to a good degree based on the adoption by other open source projects. We decided that it's more realistic to commit a relatively early version of the new build system and mature it in tree. This commit adds an initial version of a meson based build system. It supports building postgres on at least AIX, FreeBSD, Linux, macOS, NetBSD, OpenBSD, Solaris and Windows (however only gcc is supported on aix, solaris). For Windows/MSVC postgres can now be built with ninja (faster, particularly for incremental builds) and msbuild (supporting the visual studio GUI, but building slower). Several aspects (e.g. Windows rc file generation, PGXS compatibility, LLVM bitcode generation, documentation adjustments) are done in subsequent commits requiring further review. Other aspects (e.g. not installing test-only extensions) are not yet addressed. When building on Windows with msbuild, builds are slower when using a visual studio version older than 2019, because those versions do not support MultiToolTask, required by meson for intra-target parallelism. The plan is to remove the MSVC specific build system in src/tools/msvc soon after reaching feature parity. However, we're not planning to remove the autoconf/make build system in the near future. Likely we're going to keep at least the parts required for PGXS to keep working around until all supported versions build with meson. Some initial help for postgres developers is at https://wiki.postgresql.org/wiki/Meson With contributions from Thomas Munro, John Naylor, Stone Tickle and others. Author: Andres Freund <andres@anarazel.de> Author: Nazir Bilal Yavuz <byavuz81@gmail.com> Author: Peter Eisentraut <peter@eisentraut.org> Reviewed-By: Peter Eisentraut <peter.eisentraut@enterprisedb.com> Discussion: https://postgr.es/m/20211012083721.hvixq4pnh2pixr3j@alap3.anarazel.de
* Fix recent cpluspluscheck issue in selfuncs.h.Peter Geoghegan2022-09-20
| | | | | | | | | | | Fix selfuncs.h cpluspluscheck complaint, without reintroducing a parameter name inconsistency (restore the original declaration names, and then make corresponding function definitions consistent with that). Oversight in commit a601366a. Author: Peter Geoghegan <pg@bowt.ie> Reported-By: Andres Freund <andres@anarazel.de>
* Harmonize more parameter names in bulk.Peter Geoghegan2022-09-20
| | | | | | | | | | | | | | | | Make sure that function declarations use names that exactly match the corresponding names from function definitions in optimizer, parser, utility, libpq, and "commands" code, as well as in remaining library code. Do the same for all code related to frontend programs (with the exception of pg_dump/pg_dumpall related code). Like other recent commits that cleaned up function parameter names, this commit was written with help from clang-tidy. Later commits will handle ecpg and pg_dump/pg_dumpall. Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/CAH2-WznJt9CMM9KJTMjJh_zbL5hD9oX44qdJ4aqZtjFi-zA3Tg@mail.gmail.com
* Suppress variable-set-but-not-used warnings from clang 15.Tom Lane2022-09-20
| | | | | | | | | | | | | | | | | | | | | | | | clang 15+ will issue a set-but-not-used warning when the only use of a variable is in autoincrements (e.g., "foo++;"). That's perfectly sensible, but it detects a few more cases that we'd not noticed before. Silence the warnings with our usual methods, such as PG_USED_FOR_ASSERTS_ONLY, or in one case by actually removing a useless variable. One thing that we can't nicely get rid of is that with %pure-parser, Bison emits "yynerrs" as a local variable that falls foul of this warning. To silence those, I inserted "(void) yynerrs;" in the top-level productions of affected grammars. Per recently-established project policy, this is a candidate for back-patching into out-of-support branches: it suppresses annoying compiler warnings but changes no behavior. Hence, back-patch to 9.5, which is as far as these patches go without issues. (A preliminary check shows that the prior branches need some other set-but-not-used cleanups too, so I'll leave them for another day.) Discussion: https://postgr.es/m/514615.1663615243@sss.pgh.pa.us
* Consistently use named parameters in regex code.Peter Geoghegan2022-09-19
| | | | | | | | | | | | Make regex code consistently use named parameters in function declarations. Also make sure that parameter names from each function's declaration match corresponding definition parameter names. This makes Henry Spencer's regex code follow Postgres coding standards. Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/CAH2-WznJt9CMM9KJTMjJh_zbL5hD9oX44qdJ4aqZtjFi-zA3Tg@mail.gmail.com
* Adjust assorted hint messages that list all valid options.Peter Eisentraut2022-09-16
| | | | | | | | | | Instead of listing all valid options, we now try to provide one that looks similar. Since this may be useful elsewhere, this change introduces a new set of functions that can be reused for similar purposes. Author: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/b1f9f399-3a1a-b554-283f-4ae7f34608e2@enterprisedb.com
* Split up guc.c for better build speed and ease of maintenance.Tom Lane2022-09-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | guc.c has grown to be one of our largest .c files, making it a bottleneck for compilation. It's also acquired a bunch of knowledge that'd be better kept elsewhere, because of our not very good habit of putting variable-specific check hooks here. Hence, split it up along these lines: * guc.c itself retains just the core GUC housekeeping mechanisms. * New file guc_funcs.c contains the SET/SHOW interfaces and some SQL-accessible functions for GUC manipulation. * New file guc_tables.c contains the data arrays that define the built-in GUC variables, along with some already-exported constant tables. * GUC check/assign/show hook functions are moved to the variable's home module, whenever that's clearly identifiable. A few hard- to-classify hooks ended up in commands/variable.c, which was already a home for miscellaneous GUC hook functions. To avoid cluttering a lot more header files with #include "guc.h", I also invented a new header file utils/guc_hooks.h and put all the GUC hook functions' declarations there, regardless of their originating module. That allowed removal of #include "guc.h" from some existing headers. The fallout from that (hopefully all caught here) demonstrates clearly why such inclusions are best minimized: there are a lot of files that, for example, were getting array.h at two or more levels of remove, despite not having any connection at all to GUCs in themselves. There is some very minor code beautification here, such as renaming a couple of inconsistently-named hook functions and improving some comments. But mostly this just moves code from point A to point B and deals with the ensuing needs for #include adjustments and exporting a few functions that previously weren't exported. Patch by me, per a suggestion from Andres Freund; thanks also to Michael Paquier for the idea to invent guc_funcs.c. Discussion: https://postgr.es/m/587607.1662836699@sss.pgh.pa.us
* Fix NaN comparison in circle_same testDaniel Gustafsson2022-09-12
| | | | | | | | | | | | | | | | | | | | | | Commit c4c340088 changed geometric operators to use float4 and float8 functions, and handle NaN's in a better way. The circle sameness test had a typo in the code which resulted in all comparisons with the left circle having a NaN radius considered same. postgres=# select '<(0,0),NaN>'::circle ~= '<(0,0),1>'::circle; ?column? ---------- t (1 row) This fixes the sameness test to consider the radius of both the left and right circle. Backpatch to v12 where this was introduced. Author: Ranier Vilela <ranier.vf@gmail.com> Discussion: https://postgr.es/m/CAEudQAo8dK=yctg2ZzjJuzV4zgOPBxRU5+Kb+yatFiddtQk6Rw@mail.gmail.com Backpatch-through: v12
* Bump minimum version of Bison to 2.3John Naylor2022-09-09
| | | | | | | | | | | | Since the retirement of some older buildfarm members, the oldest Bison that gets regular testing is 2.3. MacOS ships that version, and will continue doing so for the forseeable future because of Apple's policy regarding GPLv3. While Mac users could use a package manager to install a newer version, there is no compelling reason to force them do so at this time. Reviewed by Andres Freund Discussion: https://www.postgresql.org/message-id/1097762.1662145681@sss.pgh.pa.us
* Fix an assortment of improper usages of string functionsDavid Rowley2022-09-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | In a similar effort to f736e188c and 110d81728, fixup various usages of string functions where a more appropriate function is available and more fit for purpose. These changes include: 1. Use cstring_to_text_with_len() instead of cstring_to_text() when working with a StringInfoData and the length can easily be obtained. 2. Use appendStringInfoString() instead of appendStringInfo() when no formatting is required. 3. Use pstrdup(...) instead of psprintf("%s", ...) 4. Use pstrdup(...) instead of psprintf(...) (with no formatting) 5. Use appendPQExpBufferChar() instead of appendPQExpBufferStr() when the length of the string being appended is 1. 6. appendStringInfoChar() instead of appendStringInfo() when no formatting is required and string is 1 char long. 7. Use appendPQExpBufferStr(b, .) instead of appendPQExpBuffer(b, "%s", .) 8. Don't use pstrdup when it's fine to just point to the string constant. I (David) did find other cases of #8 but opted to use #4 instead as I wasn't certain enough that applying #8 was ok (e.g in hba.c) Author: Ranier Vilela, David Rowley Discussion: https://postgr.es/m/CAApHDvo2j2+RJBGhNtUz6BxabWWh2Jx16wMUMWKUjv70Ver1vg@mail.gmail.com
* Fix incorrect uses of Datum conversion macrosPeter Eisentraut2022-09-05
| | | | | | | | | | | Since these macros just cast whatever you give them to the designated output type, and many normal uses also cast the output type further, a number of incorrect uses go undiscovered. The fixes in this patch have been discovered by changing these macros to inline functions, which is the subject of a future patch. Reviewed-by: Aleksander Alekseev <aleksander@timescale.com> Discussion: https://www.postgresql.org/message-id/flat/8528fb7e-0aa2-6b54-85fb-0c0886dbd6ed%40enterprisedb.com
* Build all Flex files standaloneJohn Naylor2022-09-04
| | | | | | | | | | | | | The proposed Meson build system will need a way to ignore certain generated files in order to coexist with the autoconf build system, and C files generated by Flex which are #include'd into .y files make this more difficult. In similar vein to 72b1e3a21, arrange for all Flex C files to compile to their own .o targets. Reviewed by Andres Freund Discussion: https://www.postgresql.org/message-id/20220810171935.7k5zgnjwqzalzmtm%40awork3.anarazel.de Discussion: https://www.postgresql.org/message-id/CAFBsxsF8Gc2StS3haXofshHCzqNMRXiSxvQEYGwnFsTmsdwNeg@mail.gmail.com
* Revert SQL/JSON featuresAndrew Dunstan2022-09-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The reverts the following and makes some associated cleanups: commit f79b803dc: Common SQL/JSON clauses commit f4fb45d15: SQL/JSON constructors commit 5f0adec25: Make STRING an unreserved_keyword. commit 33a377608: IS JSON predicate commit 1a36bc9db: SQL/JSON query functions commit 606948b05: SQL JSON functions commit 49082c2cc: RETURNING clause for JSON() and JSON_SCALAR() commit 4e34747c8: JSON_TABLE commit fadb48b00: PLAN clauses for JSON_TABLE commit 2ef6f11b0: Reduce running time of jsonb_sqljson test commit 14d3f24fa: Further improve jsonb_sqljson parallel test commit a6baa4bad: Documentation for SQL/JSON features commit b46bcf7a4: Improve readability of SQL/JSON documentation. commit 112fdb352: Fix finalization for json_objectagg and friends commit fcdb35c32: Fix transformJsonBehavior commit 4cd8717af: Improve a couple of sql/json error messages commit f7a605f63: Small cleanups in SQL/JSON code commit 9c3d25e17: Fix JSON_OBJECTAGG uniquefying bug commit a79153b7a: Claim SQL standard compliance for SQL/JSON features commit a1e7616d6: Rework SQL/JSON documentation commit 8d9f9634e: Fix errors in copyfuncs/equalfuncs support for JSON node types. commit 3c633f32b: Only allow returning string types or bytea from json_serialize commit 67b26703b: expression eval: Fix EEOP_JSON_CONSTRUCTOR and EEOP_JSONEXPR size. The release notes are also adjusted. Backpatch to release 15. Discussion: https://postgr.es/m/40d2c882-bcac-19a9-754d-4299e1d87ac7@postgresql.org
* Fix a bug in roles_is_member_of.Robert Haas2022-08-31
| | | | | | | | | | | | | | Commit e3ce2de09d814f8770b2e3b3c152b7671bcdb83f rearranged this function to be able to identify which inherited role had admin option on the target role, but it got the order of operations wrong, causing the function to return wrong answers in the presence of non-inherited grants. Fix that, and add a test case that verifies the correct behavior. Patch by me, reviewed by Nathan Bossart Discussion: http://postgr.es/m/CA+TgmoYamnu-xt-u7CqjYWnRiJ6BQaSpYOHXP=r4QGTfd1N_EA@mail.gmail.com
* Remove configure probe for sockaddr_in6 and require AF_INET6.Thomas Munro2022-08-26
| | | | | | | | | | | | | | | | | SUSv3 <netinet/in.h> defines struct sockaddr_in6, and all targeted Unix systems have it. Windows has it in <ws2ipdef.h>. Remove the configure probe, the macro and a small amount of dead code. Also remove a mention of IPv6-less builds from the documentation, since there aren't any. This is similar to commits f5580882 and 077bf2f2 for Unix sockets. Even though AF_INET6 is an "optional" component of SUSv3, there are no known modern operating system without it, and it seems even less likely to be omitted from future systems than AF_UNIX. Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/CA+hUKGKErNfhmvb_H0UprEmp4LPzGN06yR2_0tYikjzB-2ECMw@mail.gmail.com
* More -Wshadow=compatible-local warning fixesDavid Rowley2022-08-26
| | | | | | | | | | | | In a similar effort to f01592f91, here we're targetting fixing the warnings where we've deemed the shadowing variable to serve a close enough purpose to the shadowed variable just to reuse the shadowed version and not declare the shadowing variable at all. By my count, this takes the warning count from 106 down to 71. Author: Justin Pryzby Discussion: https://postgr.es/m/20220825020839.GT2342@telsasoft.com
* Allow grant-level control of role inheritance behavior.Robert Haas2022-08-25
| | | | | | | | | | | | | | | | | | | | The GRANT statement can now specify WITH INHERIT TRUE or WITH INHERIT FALSE to control whether the member inherits the granted role's permissions. For symmetry, you can now likewise write WITH ADMIN TRUE or WITH ADMIN FALSE to turn ADMIN OPTION on or off. If a GRANT does not specify WITH INHERIT, the behavior based on whether the member role is marked INHERIT or NOINHERIT. This means that if all roles are marked INHERIT or NOINHERIT before any role grants are performed, the behavior is identical to what we had before; otherwise, it's different, because ALTER ROLE [NO]INHERIT now only changes the default behavior of future grants, and has no effect on existing ones. Patch by me. Reviewed and testing by Nathan Bossart and Tushar Ahuja, with design-level comments from various others. Discussion: http://postgr.es/m/CA+Tgmoa5Sf4PiWrfxA=sGzDKg0Ojo3dADw=wAHOhR9dggV=RmQ@mail.gmail.com
* Defend against stack overrun in a few more places.Tom Lane2022-08-24
| | | | | | | | | | | | | | | | | | | SplitToVariants() in the ispell code, lseg_inside_poly() in geo_ops.c, and regex_selectivity_sub() in selectivity estimation could recurse until stack overflow; fix by adding check_stack_depth() calls. So could next() in the regex compiler, but that case is better fixed by converting its tail recursion to a loop. (We probably get better code that way too, since next() can now be inlined into its sole caller.) There remains a reachable stack overrun in the Turkish stemmer, but we'll need some advice from the Snowball people about how to fix that. Per report from Egor Chindyaskin and Alexander Lakhin. These mistakes are old, so back-patch to all supported branches. Richard Guo and Tom Lane Discussion: https://postgr.es/m/1661334672.728714027@f473.i.mail.ru
* Further -Wshadow=compatible-local warning fixesDavid Rowley2022-08-24
| | | | | | | | | | | | | These should have been included in 421892a19 as these shadowed variable warnings can also be fixed by adjusting the scope of the shadowed variable to put the declaration for it in an inner scope. This is part of the same effort as f01592f91. By my count, this takes the warning count from 114 down to 106. Author: David Rowley and Justin Pryzby Discussion: https://postgr.es/m/CAApHDvrwLGBP%2BYw9vriayyf%3DXR4uPWP5jr6cQhP9au_kaDUhbA%40mail.gmail.com
* Make role grant system more consistent with other privileges.Robert Haas2022-08-22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, membership of role A in role B could be recorded in the catalog tables only once. This meant that a new grant of role A to role B would overwrite the previous grant. For other object types, a new grant of permission on an object - in this case role A - exists along side the existing grant provided that the grantor is different. Either grant can be revoked independently of the other, and permissions remain so long as at least one grant remains. Make role grants work similarly. Previously, when granting membership in a role, the superuser could specify any role whatsoever as the grantor, but for other object types, the grantor of record must be either the owner of the object, or a role that currently has privileges to perform a similar GRANT. Implement the same scheme for role grants, treating the bootstrap superuser as the role owner since roles do not have owners. This means that attempting to revoke a grant, or admin option on a grant, can now fail if there are dependent privileges, and that CASCADE can be used to revoke these. It also means that you can't grant ADMIN OPTION on a role back to a user who granted it directly or indirectly to you, similar to how you can't give WITH GRANT OPTION on a privilege back to a role which granted it directly or indirectly to you. Previously, only the superuser could specify GRANTED BY with a user other than the current user. Relax that rule to allow the grantor to be any role whose privileges the current user posseses. This doesn't improve compatibility with what we do for other object types, where support for GRANTED BY is entirely vestigial, but it makes this feature more usable and seems to make sense to change at the same time we're changing related behaviors. Along the way, fix "ALTER GROUP group_name ADD USER user_name" to require the same privileges as "GRANT group_name TO user_name". Previously, CREATEROLE privileges were sufficient for either, but only the former form was permissible with ADMIN OPTION on the role. Now, either CREATEROLE or ADMIN OPTION on the role suffices for either spelling. Patch by me, reviewed by Stephen Frost. Discussion: http://postgr.es/m/CA+TgmoaFr-RZeQ+WoQ5nKPv97oT9+aDgK_a5+qWHSgbDsMp1Vg@mail.gmail.com
* Remove shadowed local variables that are new in v15David Rowley2022-08-20
| | | | | | | | | | | | | | | | | | | | | | | | | Compiling with -Wshadow=compatible-local yields quite a few warnings about local variables being shadowed by compatible local variables in an inner scope. Of course, this is perfectly valid in C, but we have had bugs in the past as a result of developers failing to notice this. af7d270dd is a recent example. Here we do a cleanup of warnings we receive from -Wshadow=compatible-local for code which is new to PostgreSQL 15. We've yet to have the discussion about if we actually ever want to run that as a standard compilation flag. We'll need to at least get the number of warnings down to something easier to manage before we can realistically consider if we want this or not. This commit is the first step towards reducing the warnings. The changes being made here are all fairly trivial. Because of that, and the fact that v15 is still in beta, this is being back-patched into 15. It seems more risky not to do this as the risk of future bugs is increased by the additional conflicts that this commit could cause for any future bug fixes touching the same areas as this commit. Author: Justin Pryzby Discussion: https://postgr.es/m/20220817145434.GC26426%40telsasoft.com Backpatch-through: 15
* Avoid using list_length() to test for empty list.Tom Lane2022-08-17
| | | | | | | | | | | | | | | | | | | | | | | | The standard way to check for list emptiness is to compare the List pointer to NIL; our list code goes out of its way to ensure that that is the only representation of an empty list. (An acceptable alternative is a plain boolean test for non-null pointer, but explicit mention of NIL is usually preferable.) Various places didn't get that memo and expressed the condition with list_length(), which might not be so bad except that there were such a variety of ways to check it exactly: equal to zero, less than or equal to zero, less than one, yadda yadda. In the name of code readability, let's standardize all those spellings as "list == NIL" or "list != NIL". (There's probably some microscopic efficiency gain too, though few of these look to be at all performance-critical.) A very small number of cases were left as-is because they seemed more consistent with other adjacent list_length tests that way. Peter Smith, with bikeshedding from a number of us Discussion: https://postgr.es/m/CAHut+PtQYe+ENX5KrONMfugf0q6NHg4hR5dAhqEXEc2eefFeig@mail.gmail.com
* Preserve memory context of VarStringSortSupport buffers.Tom Lane2022-08-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When enlarging the work buffers of a VarStringSortSupport object, varstrfastcmp_locale was careful to keep them in the ssup_cxt memory context; but varstr_abbrev_convert just used palloc(). The latter creates a hazard that the buffers could be freed out from under the VarStringSortSupport object, resulting in stomping on whatever gets allocated in that memory later. In practice, because we only use this code for ICU collations (cf. 3df9c374e), the problem is confined to use of ICU collations. I believe it may have been unreachable before the introduction of incremental sort, too, as traditional sorting usually just uses one context for the duration of the sort. We could fix this by making the broken stanzas in varstr_abbrev_convert match the non-broken ones in varstrfastcmp_locale. However, it seems like a better idea to dodge the issue altogether by replacing the pfree-and-allocate-anew coding with repalloc, which automatically preserves the chunk's memory context. This fix does add a few cycles because repalloc will copy the chunk's content, which the existing coding assumes is useless. However, we don't expect that these buffer enlargement operations are performance-critical. Besides that, it's far from obvious that copying the buffer contents isn't required, since these stanzas make no effort to mark the buffers invalid by resetting last_returned, cache_blob, etc. That seems to be safe upon examination, but it's fragile and could easily get broken in future, which wouldn't get revealed in testing with short-to-moderate-size strings. Per bug #17584 from James Inform. Whether or not the issue is reachable in the older branches, this code has been broken on its own terms from its introduction, so patch all the way back. Discussion: https://postgr.es/m/17584-95c79b4a7d771f44@postgresql.org
* Fix function-defined-but-not-used warning.Tom Lane2022-08-06
| | | | | | | | | | | | Buildfarm member jacana (MinGW) has been complaining that get_iso_localename is defined but not used. This is evidently fallout from the recent removal of VS2013 support in pg_locale.c. Rearrange the #ifs so that get_iso_localename and its subroutine search_locale_enum won't get built on MinGW. I also noticed that a comment in get_iso_localename cross- referenced a comment in IsoLocaleName that isn't there anymore. Put back what I think is the referenced material.
* Replace pgwin32_is_junction() with lstat().Thomas Munro2022-08-06
| | | | | | | | | | | | | | | | | | | Now that lstat() reports junction points with S_IFLNK/S_ISLINK(), and unlink() can unlink them, there is no need for conditional code for Windows in a few places. That was expressed by testing for WIN32 or S_ISLNK, which we can now constant-fold. The coding around pgwin32_is_junction() was a bit suspect anyway, as we never checked for errors, and we also know that errors can be spuriously reported because of transient sharing violations on this OS. The lstat()-based code has handling for that. This also reverts 4fc6b6ee on master only. That was done because lstat() didn't previously work for symlinks (junction points), but now it does. Tested-by: Andrew Dunstan <andrew@dunslane.net> Discussion: https://postgr.es/m/CA%2BhUKGLfOOeyZpm5ByVcAt7x5Pn-%3DxGRNCvgiUPVVzjFLtnY0w%40mail.gmail.com
* Remove configure probes for symlink/readlink, and dead code.Thomas Munro2022-08-05
| | | | | | | | | | | | | | | | | | | symlink() and readlink() are in SUSv2 and all targeted Unix systems have them. We have partial emulation on Windows. Code that raised runtime errors on systems without it has been dead for years, so we can remove that and also references to such systems in the documentation. Define HAVE_READLINK and HAVE_SYMLINK macros on Unix. Our Windows replacement functions based on junction points can't be used for relative paths or for non-directories, so the macros can be used to check for full symlink support. The places that deal with tablespaces can just use symlink functions without checking the macros. (If they did check the macros, they'd need to provide an #else branch with a runtime or compile time error, and it'd be dead code.) Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/CA+hUKGJ3LHeP9w5Fgzdr4G8AnEtJ=z=p6hGDEm4qYGEUX5B6fQ@mail.gmail.com