aboutsummaryrefslogtreecommitdiff
path: root/src/backend/utils/misc/guc.c
Commit message (Collapse)AuthorAge
* Fix assorted pretty-trivial memory leaks in the backend.Tom Lane42 hours
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In the current system architecture, none of these are worth obsessing over; most are once-per-process leaks. However, Valgrind complains about all of them, and if we get to using threads rather than processes for backend sessions, it will become more interesting to avoid per-session leaks. * Fix leaks in StartupXLOG() and ShutdownWalRecovery(). * Fix leakage of pq_mq_handle in a parallel worker. While at it, move mq_putmessage's "Assert(pq_mq_handle != NULL)" to someplace where it's not trivially useless. * Fix leak in logicalrep_worker_detach(). * Don't leak the startup-packet buffer in ProcessStartupPacket(). * Fix leak in evtcache.c's DecodeTextArrayToBitmapset(). If the presented array is toasted, this neglected to free the detoasted copy, which was then leaked into EventTriggerCacheContext. * I'm distressed by the amount of code that BuildEventTriggerCache is willing to run while switched into a long-lived cache context. Although the detoasted array is the only leak that Valgrind reports, let's tighten things up while we're here. (DecodeTextArrayToBitmapset is still run in the cache context, so doing this doesn't remove the need for the detoast fix. But it reduces the surface area for other leaks.) * load_domaintype_info() intentionally leaked some intermediate cruft into the long-lived DomainConstraintCache's memory context, reasoning that the amount of leakage will typically not be much so it's not worth doing a copyObject() of the final tree to avoid that. But Valgrind knows nothing of engineering tradeoffs and complains anyway. On the whole, the copyObject doesn't cost that much and this is surely not a performance-critical code path, so let's do it the clean way. * MarkGUCPrefixReserved didn't bother to clean up removed placeholder GUCs at all, which shows up as a leak in one regression test. It seems appropriate for it to do as much cleanup as define_custom_variable does when replacing placeholders, so factor that code out into a helper function. define_custom_variable's logic was one brick shy of a load too: it forgot to free the separate allocation for the placeholder's name. Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Richard Guo <guofenglinux@gmail.com> Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us
* Allow resetting unknown custom GUCs with reserved prefixes.Nathan Bossart3 days
| | | | | | | | | | | | | | | | | | | | | | | | Currently, ALTER DATABASE/ROLE/SYSTEM RESET [ALL] with an unknown custom GUC with a prefix reserved by MarkGUCPrefixReserved() errors (unless a superuser runs a RESET ALL variant). This is problematic for cases such as an extension library upgrade that removes a GUC. To fix, simply make sure the relevant code paths explicitly allow it. Note that we require superuser or privileges on the parameter to reset it. This is perhaps a bit more restrictive than is necessary, but it's not clear whether further relaxing the requirements is safe. Oversight in commit 88103567cb. The ALTER SYSTEM fix is dependent on commit 2d870b4aef, which first appeared in v17. Unfortunately, back-patching that commit would introduce ABI breakage, and while that breakage seems unlikely to bother anyone, it doesn't seem worth the risk. Hence, the ALTER SYSTEM part of this commit is omitted on v15 and v16. Reported-by: Mert Alev <mert@futo.org> Reviewed-by: Laurenz Albe <laurenz.albe@cybertec.at> Discussion: https://postgr.es/m/18964-ba09dea8c98fccd6%40postgresql.org Backpatch-through: 15
* Fix guc_malloc calls for consistency and OOM checksDaniel Gustafsson2025-03-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | check_createrole_self_grant and check_synchronized_standby_slots were allocating memory on a LOG elevel without checking if the allocation succeeded or not, which would have led to a segfault on allocation failure. On top of that, a number of callsites were using the ERROR level, relying on erroring out rather than returning false to allow the GUC machinery handle it gracefully. Other callsites used WARNING instead of LOG. While neither being not wrong, this changes all check_ functions do it consistently with LOG. init_custom_variable gets a promoted elevel to FATAL to keep the guc_malloc error handling in line with the rest of the error handling in that function which already call FATAL. If we encounter an OOM in this callsite there is no graceful handling to be had, better to error out hard. Backpatch the fix to check_createrole_self_grant down to v16 and the fix to check_synchronized_standby_slots down to v17 where they were introduced. Author: Daniel Gustafsson <daniel@yesql.se> Reported-by: Nikita <pm91.arapov@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Bug: #18845 Discussion: https://postgr.es/m/18845-582c6e10247377ec@postgresql.org Backpatch-through: 16
* Use "ssize_t" not "long" in max_stack_depth-related code.Tom Lane2025-01-30
| | | | | | | | | | | | | | | | | | | | | This change adapts these functions to the machine's address width without depending on "long" to be the right size. (It isn't on Win64, for example.) While it seems unlikely anyone would care to run with a stack depth limit exceeding 2GB, this is part of a general push to avoid using type "long" to represent memory sizes. It's convenient to use ssize_t rather than the perhaps-more-obvious choice of size_t/Size, because the code involved depends on working with a signed data type. Our MAX_KILOBYTES limit already ensures that ssize_t will be sufficient to represent the maximum value of max_stack_depth. Extracted from a larger patch by Vladlen, plus additional hackery by me. Author: Vladlen Popolitov <v.popolitov@postgrespro.ru> Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/1a01f0-66ec2d80-3b-68487680@27595217
* Update copyright for 2025Bruce Momjian2025-01-01
| | | | Backpatch-through: 13
* Fix per-session activation of ALTER {ROLE|DATABASE} SET role.Noah Misch2024-11-15
| | | | | | | | | | | | | | | After commit 5a2fed911a85ed6d8a015a6bafe3a0d9a69334ae, the catalog state resulting from these commands ceased to affect sessions. Restore the longstanding behavior, which is like beginning the session with a SET ROLE command. If cherry-picking the CVE-2024-10978 fixes, default to including this, too. (This fixes an unintended side effect of fixing CVE-2024-10978.) Back-patch to v12, like that commit. The release team decided to include v12, despite the original intent to halt v12 commits earlier this week. Tom Lane and Noah Misch. Reported by Etienne LAFARGE. Discussion: https://postgr.es/m/CADOZwSb0UsEr4_UTFXC5k7=fyyK8uKXekucd+-uuGjJsGBfxgw@mail.gmail.com
* Fix improper interactions between session_authorization and role.Tom Lane2024-11-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The SQL spec mandates that SET SESSION AUTHORIZATION implies SET ROLE NONE. We tried to implement that within the lowest-level functions that manipulate these settings, but that was a bad idea. In particular, guc.c assumes that it doesn't matter in what order it applies GUC variable updates, but that was not the case for these two variables. This problem, compounded by some hackish attempts to work around it, led to some security-grade issues: * Rolling back a transaction that had done SET SESSION AUTHORIZATION would revert to SET ROLE NONE, even if that had not been the previous state, so that the effective user ID might now be different from what it had been. * The same for SET SESSION AUTHORIZATION in a function SET clause. * If a parallel worker inspected current_setting('role'), it saw "none" even when it should see something else. Also, although the parallel worker startup code intended to cope with the current role's pg_authid row having disappeared, its implementation of that was incomplete so it would still fail. Fix by fully separating the miscinit.c functions that assign session_authorization from those that assign role. To implement the spec's requirement, teach set_config_option itself to perform "SET ROLE NONE" when it sets session_authorization. (This is undoubtedly ugly, but the alternatives seem worse. In particular, there's no way to do it within assign_session_authorization without incompatible changes in the API for GUC assign hooks.) Also, improve ParallelWorkerMain to directly set all the relevant user-ID variables instead of relying on some of them to get set indirectly. That allows us to survive not finding the pg_authid row during worker startup. In v16 and earlier, this includes back-patching 9987a7bf3 which fixed a violation of GUC coding rules: SetSessionAuthorization is not an appropriate place to be throwing errors from. Security: CVE-2024-10978
* Support configuring multiple ECDH curvesDaniel Gustafsson2024-10-24
| | | | | | | | | | | | | | | | | | | The ssl_ecdh_curve GUC only accepts a single value, but the TLS handshake can list multiple curves in the groups extension (the extension has been renamed to contain more than elliptic curves). This changes the GUC to accept a colon-separated list of curves. This commit also renames the GUC to ssl_groups to match the new nomenclature for the TLS extension. Original patch by Erica Zhang with additional hacking by me. Author: Erica Zhang <ericazhangy2021@qq.com> Author: Daniel Gustafsson <daniel@yesql.se> Reviewed-by: Jacob Champion <jacob.champion@enterprisedb.com> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Reviewed-by: Jelte Fennema-Nio <postgres@jeltef.nl> Discussion: https://postgr.es/m/tencent_063F89FA72CCF2E48A0DF5338841988E9809@qq.com
* Apply GUC name from central table in more places of guc.cMichael Paquier2024-10-09
| | | | | | | | | | | | | | | | | | | The name extracted from the record of the GUC tables is applied to more internal places of guc.c. This change has the advantage to simplify parse_and_validate_value(), where the "name" was only used in elog messages, while it was required to match with the name from the GUC record. pg_parameter_aclcheck() now passes the name of the GUC from its record in two places rather than the caller's argument. The value given to this function goes through convert_GUC_name_for_parameter_acl() that does a simple ASCII downcasing. Few GUCs mix character casing in core; one test is added for one of these code paths with "IntervalStyle". Author: Peter Smith, Michael Paquier Discussion: https://postgr.es/m/ZwNh4vkc2NHJHnND@paquier.xyz
* Allow adjusting session_authorization and role in parallel workers.Tom Lane2024-08-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | The code intends to allow GUCs to be set within parallel workers via function SET clauses, but not otherwise. However, doing so fails for "session_authorization" and "role", because the assign hooks for those attempt to set the subsidiary "is_superuser" GUC, and that call falls foul of the "not otherwise" prohibition. We can't switch to using GUC_ACTION_SAVE for this, so instead add a new GUC variable flag GUC_ALLOW_IN_PARALLEL to mark is_superuser as being safe to set anyway. (This is okay because is_superuser has context PGC_INTERNAL and thus only hard-wired calls can change it. We'd need more thought before applying the flag to other GUCs; but maybe there are other use-cases.) This isn't the prettiest fix perhaps, but other alternatives we thought of would be much more invasive. While here, correct a thinko in commit 059de3ca4: when rejecting a GUC setting within a parallel worker, we should return 0 not -1 if the ereport doesn't longjmp. (This seems to have no consequences right now because no caller cares, but it's inconsistent.) Improve the comments to try to forestall future confusion of the same kind. Despite the lack of field complaints, this seems worth back-patching. Thanks to Nathan Bossart for the idea to invent a new flag, and for review. Discussion: https://postgr.es/m/2833457.1723229039@sss.pgh.pa.us
* Move all extern declarations for GUC variables to header filesPeter Eisentraut2024-07-24
| | | | | | | | | | | | | | | | | | Add extern declarations in appropriate header files for global variables related to GUC. In many cases, this was handled quite inconsistently before, with some GUC variables declared in a header file and some only pulled in via ad-hoc extern declarations in various .c files. Also add PGDLLIMPORT qualifications to those variables. These were previously missing because src/tools/mark_pgdllimport.pl has only been used with header files. This also fixes -Wmissing-variable-declarations warnings for GUC variables (not yet part of the standard warning options). Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://www.postgresql.org/message-id/flat/e0a62134-83da-4ba4-8cdb-ceb0111c95ce@eisentraut.org
* Fix bad indentation introduced in 43cd30bcd1cAndres Freund2024-07-15
| | | | | | | | Oops. Reported-by: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://postgr.es/m/ZpVZB9rH5tHllO75@nathan Backpatch: 12-, like 43cd30bcd1c
* Fix type confusion in guc_var_compare()Andres Freund2024-07-15
| | | | | | | | | | | | | | | | | Before this change guc_var_compare() cast the input arguments to const struct config_generic *. That's not quite right however, as the input on one side is often just a char * on one side. Instead just use char *, the first field in config_generic. This fixes a -Warray-bounds warning with some versions of gcc. While the warning is only known to be triggered for <= 15, the issue the warning points out seems real, so apply the fix everywhere. Author: Nazir Bilal Yavuz <byavuz81@gmail.com> Reported-by: Erik Rijkers <er@xs4all.nl> Suggested-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/a74a1a0d-0fd2-3649-5224-4f754e8f91aa%40xs4all.nl
* Revise GUC names quoting in messages againPeter Eisentraut2024-05-17
| | | | | | | | | | | | | | | After further review, we want to move in the direction of always quoting GUC names in error messages, rather than the previous (PG16) wildly mixed practice or the intermittent (mid-PG17) idea of doing this depending on how possibly confusing the GUC name is. This commit applies appropriate quotes to (almost?) all mentions of GUC names in error messages. It partially supersedes a243569bf65 and 8d9978a7176, which had moved things a bit in the opposite direction but which then were abandoned in a partial state. Author: Peter Smith <smithpb2250@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CAHut%2BPv-kSN8SkxSdoHano_wPubqcg5789ejhCDZAcLFceBR-w%40mail.gmail.com
* Improve "out of range" error messages for GUCs.Tom Lane2024-04-23
| | | | | | | If the GUC has a unit, label the minimum and maximum values with the unit explicitly. Per suggestion from Jian He. Discussion: https://postgr.es/m/CACJufxFJo6FyVg9W8yvNAxbjP+EJ9wieE9d9vw5LpPzyLnLLOQ@mail.gmail.com
* Add allow_alter_system GUC.Robert Haas2024-03-29
| | | | | | | | | | | | | | | | | | This is marked PGC_SIGHUP, so it can only be set in a configuration file, not anywhere else; and it is also marked GUC_DISALLOW_IN_AUTO_FILE, so it can't be set using ALTER SYSTEM. When set to false, the ALTER SYSTEM command is disallowed. There was considerable concern that this would be misinterpreted as a security feature, which it is not, because a determined superuser has various ways of bypassing it. Hence, a lot of work has gone into wordsmithing the documentation, in the hopes of avoiding any such confusion. Jelte Fennemia-Nio and Gabriele Bartolini, with wording suggestions for the documentation from many others. Discussion: http://postgr.es/m/CA%2BVUV5rEKt2%2BCdC_KUaPoihMu%2Bi5ChT4WVNTr4CD5-xXZUfuQw%40mail.gmail.com
* Use printf's %m format instead of strerror(errno) in more placesMichael Paquier2024-03-12
| | | | | | | | | | | | | | | | | Most callers of strerror() are removed from the backend code. The remaining callers require special handling with a saved errno from a previous system call. The frontend code still needs strerror() where error states need to be handled outside of fprintf. Note that pg_regress is not changed to use %m as the TAP output may clobber errno, since those functions call fprintf() and friends before evaluating the format string. Support for %m in src/port/snprintf.c has been added in d6c55de1f99a, hence all the stable branches currently supported include it. Author: Dagfinn Ilmari Mannsåker Discussion: https://postgr.es/m/87sf13jhuw.fsf@wibble.ilmari.org
* Fix buildfarm failures from 2af07e2f74.Jeff Davis2024-03-04
| | | | | | | | | | Use GUC_ACTION_SAVE rather than GUC_ACTION_SET, necessary for working with parallel query. Now that the call requires more arguments, wrap the call in a new function to avoid code duplication and offer a place for a comment. Discussion: https://postgr.es/m/E1rhJpO-0027Wf-9L@gemulon.postgresql.org
* Remove unused #include's from backend .c filesPeter Eisentraut2024-03-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | as determined by include-what-you-use (IWYU) While IWYU also suggests to *add* a bunch of #include's (which is its main purpose), this patch does not do that. In some cases, a more specific #include replaces another less specific one. Some manual adjustments of the automatic result: - IWYU currently doesn't know about includes that provide global variable declarations (like -Wmissing-variable-declarations), so those includes are being kept manually. - All includes for port(ability) headers are being kept for now, to play it safe. - No changes of catalog/pg_foo.h to catalog/pg_foo_d.h, to keep the patch from exploding in size. Note that this patch touches just *.c files, so nothing declared in header files changes in hidden ways. As a small example, in src/backend/access/transam/rmgr.c, some IWYU pragma annotations are added to handle a special case there. Discussion: https://www.postgresql.org/message-id/flat/af837490-6b2f-46df-ba05-37ea6a6653fc%40eisentraut.org
* Update copyright for 2024Bruce Momjian2024-01-03
| | | | | | | | Reported-by: Michael Paquier Discussion: https://postgr.es/m/ZZKTDPxBBMt3C0J9@paquier.xyz Backpatch-through: 12
* Fix failure to verify PGC_[SU_]BACKEND GUCs in pg_file_settings view.Tom Lane2023-12-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | set_config_option() bails out early if it detects that the option to be set is PGC_BACKEND or PGC_SU_BACKEND class and we're reading the config file in a postmaster child; we don't want to apply any new value in such a case. That's fine as far as it goes, but it fails to consider the requirements of the pg_file_settings view: for that, we need to check validity of the value even though we have no intention to apply it. Because we didn't, even very silly values for affected GUCs would be reported as valid by the view. There are only half a dozen such GUCs, which perhaps explains why this got overlooked for so long. Fix by continuing when changeVal is false; this parallels the logic in some other early-exit paths. Also, the check added by commit 924bcf4f1 to prevent GUC changes in parallel workers seems a few bricks shy of a load: it's evidently assuming that ereport(elevel, ...) won't return. Make sure we bail out if it does. The lack of trouble reports suggests that this is only a latent bug, i.e. parallel workers don't actually reach here with elevel < ERROR. (Per the code coverage report, we never reach here at all in the regression suite.) But we clearly don't want to risk proceeding if that does happen. Per report from Rıdvan Korkmaz. These are ancient bugs, so back-patch to all supported branches. Discussion: https://postgr.es/m/2089235.1703617353@sss.pgh.pa.us
* Cache opaque handle for GUC option to avoid repeasted lookups.Jeff Davis2023-12-08
| | | | | | | | | | When setting GUCs from proconfig, performance is important, and hash lookups in the GUC table are significant. Per suggestion from Robert Haas. Discussion: https://postgr.es/m/CA+TgmoYpKxhR3HOD9syK2XwcAUVPa0+ba0XPnwWBcYxtKLkyxA@mail.gmail.com Reviewed-by: John Naylor
* Apply quotes more consistently to GUC names in logsMichael Paquier2023-11-30
| | | | | | | | | | | | | | Quotes are applied to GUCs in a very inconsistent way across the code base, with a mix of double quotes or no quotes used. This commit removes double quotes around all the GUC names that are obviously referred to as parameters with non-English words (use of underscore, mixed case, etc). This is the result of a discussion with Álvaro Herrera, Nathan Bossart, Laurenz Albe, Peter Eisentraut, Tom Lane and Daniel Gustafsson. Author: Peter Smith Discussion: https://postgr.es/m/CAHut+Pv-kSN8SkxSdoHano_wPubqcg5789ejhCDZAcLFceBR-w@mail.gmail.com
* Make GetConfigOption/GetConfigOptionResetString return "" for NULL.Tom Lane2023-11-02
| | | | | | | | | | | | | | | As per the preceding commit, GUC APIs generally expose NULL-valued string variables as empty strings. Extend that policy to GetConfigOption() and GetConfigOptionResetString(), eliminating a crash hazard for unwary callers, as well as a fundamental ambiguity in GetConfigOption()'s API. No back-patch, since this is an API change and conceivably somebody somewhere is depending on this corner case. Xing Guo, Aleksander Alekseev, Tom Lane Discussion: https://postgr.es/m/CACpMh+AyDx5YUpPaAgzVwC1d8zfOL4JoD-uyFDnNSa1z0EsDQQ@mail.gmail.com
* Be more wary about NULL values for GUC string variables.Tom Lane2023-11-02
| | | | | | | | | | | | | get_explain_guc_options() crashed if a string GUC marked GUC_EXPLAIN has a NULL boot_val. Nosing around found a couple of other places that seemed insufficiently cautious about NULL string values, although those are likely unreachable in practice. Add some commentary defining the expectations for NULL values of string variables, in hopes of forestalling future additions of more such bugs. Xing Guo, Aleksander Alekseev, Tom Lane Discussion: https://postgr.es/m/CACpMh+AyDx5YUpPaAgzVwC1d8zfOL4JoD-uyFDnNSa1z0EsDQQ@mail.gmail.com
* Allow ALTER SYSTEM to set unrecognized custom GUCs.Tom Lane2023-10-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, ALTER SYSTEM failed if the target GUC wasn't present in the session's GUC hashtable. That is a reasonable behavior for core (single-part) GUC names, and for custom GUCs for which we have loaded an extension that's reserved the prefix. But it's unnecessarily restrictive otherwise, and it also causes inconsistent behavior: you can "ALTER SYSTEM SET foo.bar" only if you did "SET foo.bar" earlier in the session. That's fairly silly. Hence, refactor things so that we can execute ALTER SYSTEM even if the variable doesn't have a GUC hashtable entry, as long as the name meets the custom-variable naming requirements and does not have a reserved prefix. (It's safe to do this even if the variable belongs to an extension we currently don't have loaded. A bad value will at worst cause a WARNING when the extension does get loaded.) Also, adjust GRANT ON PARAMETER to have the same opinions about whether to allow an unrecognized GUC name, and to throw the same errors if not (it previously used a one-size-fits-all message for several distinguishable conditions). By default, only a superuser will be allowed to do ALTER SYSTEM SET on an unrecognized name, but it's possible to GRANT the ability to do it. Patch by me, pursuant to a documentation complaint from Gavin Panella. Arguably this is a bug fix, but given the lack of other complaints I'll refrain from back-patching. Discussion: https://postgr.es/m/2617358.1697501956@sss.pgh.pa.us Discussion: https://postgr.es/m/169746329791.169914.16613647309012285391@wrigleys.postgresql.org
* Add some const qualifiersPeter Eisentraut2023-09-26
| | | | | | | | | | | There was a mismatch between the const qualifiers for excludeDirContents in src/backend/backup/basebackup.c and src/bin/pg_rewind/filemap.c, which led to a quick search for similar cases. We should make excludeDirContents match, but the rest of the changes seem like a good idea as well. Author: David Steele <david@pgmasters.net> Discussion: https://www.postgresql.org/message-id/flat/669a035c-d23d-2f38-7ff0-0cb93e01d610@pgmasters.net
* Introduce macros for protocol characters.Nathan Bossart2023-08-22
| | | | | | | | | | | This commit introduces descriptively-named macros for the identifiers used in wire protocol messages. These new macros are placed in a new header file so that they can be easily used by third-party code. Author: Dave Cramer Reviewed-by: Alvaro Herrera, Tatsuo Ishii, Peter Smith, Robert Haas, Tom Lane, Peter Eisentraut, Michael Paquier Discussion: https://postgr.es/m/CADK3HHKbBmK-PKf1bPNFoMC%2BoBt%2BpD9PH8h5nvmBQskEHm-Ehw%40mail.gmail.com
* Fix code indentation violations introduced by recent commitMichael Paquier2023-08-11
| | | | | | | The two culprit commits are 5765cfe and 5e0c761. Per buildfarm member koel for the first commit, while I have noticed the second one in passing.
* Transform proconfig for faster execution.Jeff Davis2023-08-10
| | | | | | | | | | | | Store function config settings in lists to avoid the need to parse and allocate for each function execution. Speedup is modest but significant. Additionally, this change also seems cleaner and supports some other performance improvements under discussion. Discussion: https://postgr.es/m/04c8592dbd694e4114a3ed87139a7a04e4363030.camel@j-davis.com Reviewed-by: Nathan Bossart
* Replace GUC_UNIT_MEMORY|GUC_UNIT_TIME with GUC_UNIT.Masahiko Sawada2023-06-15
| | | | | | | | | | We used (GUC_UNIT_MEMORY | GUC_UNIT_TIME) instead of GUC_UNIT some places but we already define it in guc.h. This commit replaces them with GUC_UNIT for better consistency with their surrounding code. Author: Japin Li Reviewed-by: Richard Guo, Michael Paquier, Masahiko Sawada Discussion: https://postgr.es/m/MEYP282MB1669EC0FED922F7A151673ACB65AA@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
* Pre-beta mechanical code beautification.Tom Lane2023-05-19
| | | | | | | | | | | | | | | Run pgindent, pgperltidy, and reformat-dat-files. This set of diffs is a bit larger than typical. We've updated to pg_bsd_indent 2.1.2, which properly indents variable declarations that have multi-line initialization expressions (the continuation lines are now indented one tab stop). We've also updated to perltidy version 20230309 and changed some of its settings, which reduces its desire to add whitespace to lines to make assignments etc. line up. Going forward, that should make for fewer random-seeming changes to existing code. Discussion: https://postgr.es/m/20230428092545.qfb3y5wcu4cm75ur@alvherre.pgsql
* Revert "Add USER SET parameter values for pg_db_role_setting"Alexander Korotkov2023-05-17
| | | | | | | | | This reverts commit 096dd80f3ccc and its fixups beecbe8e5001, afdd9f7f0e00, 529da086ba, db93e739ac61. Catversion is bumped. Discussion: https://postgr.es/m/d46f9265-ff3c-6743-2278-6772598233c2%40pgmasters.net
* Fix wrong construct_array_builtin() call in GUCArrayDelete()Alexander Korotkov2023-04-27
| | | | | | | | | | The current code unintentionally uses the wrong datum to construct an array. The bug was introduced by 096dd80f3c, so no backpatching is needed. Reported-by: David Steele Discussion: https://postgr.es/m/d46f9265-ff3c-6743-2278-6772598233c2%40pgmasters.net Author: Nathan Bossart Reviewed-by: David Steele, Tom Lane
* Fix various typosDavid Rowley2023-04-18
| | | | | | | | | | | | This fixes many spelling mistakes in comments, but a few references to invalid parameter names, function names and option names too in comments and also some in string constants Also, fix an #undef that was undefining the incorrect definition Author: Alexander Lakhin Reviewed-by: Justin Pryzby Discussion: https://postgr.es/m/d5f68d19-c0fc-91a9-118d-7c6a5a3f5fad@gmail.com
* Remove old GUC name mapping for "force_parallel_mode"David Rowley2023-04-14
| | | | | | | | | | | | | | | | This GUC was renamed to debug_parallel_query in 5352ca22e. That commit added an entry into map_old_guc_names[] to allow the old name still to work. That was done to allow a transition time where the buildfarm configs could be swapped over to use debug_parallel_query instead. That work is now complete. Here we remove the old name with the intention of breaking any user code which is using force_parallel_query. As mentioned in the commit message for 5352ca22e, it appeared many users were misled into thinking that setting this GUC was doing something useful for them to make queries run more quickly. Discussion: https://postgr.es/m/CAApHDvoR7EOz7Tvyzrd18FO-Dw2Cp4Uyq25TEWguK+XyCJtzOw@mail.gmail.com
* 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
* Rename force_parallel_mode to debug_parallel_queryDavid Rowley2023-02-15
| | | | | | | | | | | | | | | | | | | | force_parallel_mode is meant to be used to allow us to exercise the parallel query infrastructure to ensure that it's working as we expect. It seems some users think this GUC is for forcing the query planner into picking a parallel plan regardless of the costs. A quick look at the documentation would have made them realize that they were wrong, but the GUC is likely too conveniently named which, evidently, seems to often result in users expecting that it forces the planner into usefully parallelizing queries. Here we rename the GUC to something which casual users are less likely to mistakenly think is what they need to make their query run more quickly. For now, the old name can still be used. We'll revisit if the old name mapping can be removed once the buildfarm configs are all updated. Reviewed-by: John Naylor Discussion: https://postgr.es/m/CAApHDvrsOi92_uA7PEaHZMH-S4Xv+MGhQWA+GrP8b1kjpS1HjQ@mail.gmail.com
* Extend check_GUC_init() with checks on flag combinations when loading GUCsMichael Paquier2023-02-06
| | | | | | | | | | | | | | | | | | | | | | | | This extends the work begun by a73952b, with the addition of a GUC check for flag combinations in check_GUC_init(), making sure that anything defined with GUC_NO_SHOW_ALL also includes GUC_NOT_IN_SAMPLE, as first step. There has never been any GUCs of this kind in the core code, and this combination makes little sense as a parameter marked as not fit for SHOW ALL should not be hidden in postgresql.conf.sample. Note that GUCs marked with GUC_NO_SHOW_ALL are not listed under pg_settings or SHOW ALL (still they can be queried individually), making them unfit for checks via SQL queries in the regression tests that do a full scan of the parameters available. The SQL tests are still a bit incorrect about that, and will be cleaned up in a separate commit. We have also discussed the possibility to extend the SQL functions for GUCs so as they could show more information about parameters defined with GUC_NO_SHOW_ALL, though it has been concluded that this is not worth the extra complication in the long run, an enforced policy at initialization time being enough to do the same job. Per discussion with Nitin Jadhav and Tom Lane. Discussion: https://postgr.es/m/CAMm1aWaYe0muu3ABo7iSAgK+OWDS9yNe8GGRYnCyeEpScYKa+g@mail.gmail.com
* Minor GUC code refactoring.Tom Lane2023-01-27
| | | | | | | | | | | | | | | | | | | | | Split out "ConfigOptionIsVisible" to perform the privilege check for GUC_SUPERUSER_ONLY GUCs (which these days can also be read by pg_read_all_settings role members), and move the should-we-show-it checks from GetConfigOptionValues to its sole caller. This commit also removes get_explain_guc_options's check of GUC_NO_SHOW_ALL, which seems to have got cargo-culted in there. While there's no obvious use-case for marking a GUC both GUC_EXPLAIN and GUC_NO_SHOW_ALL, if it were set up that way one would expect EXPLAIN to show it --- if that's not what you want, then don't set GUC_EXPLAIN. In passing, simplify the loop logic in show_all_settings. Nitin Jadhav, Bharath Rupireddy, Tom Lane Discussion: https://postgr.es/m/CAMm1aWYgfekpRK-Jz5=pM_bV+Om=ktGq1vxTZ_dr1Z6MV-qokA@mail.gmail.com
* Update copyright for 2023Bruce Momjian2023-01-02
| | | | Backpatch-through: 11
* Add USER SET parameter values for pg_db_role_settingAlexander Korotkov2022-12-09
| | | | | | | | | | | | | | | | The USER SET flag specifies that the variable should be set on behalf of an ordinary role. That lets ordinary roles set placeholder variables, which permission requirements are not known yet. Such a value wouldn't be used if the variable finally appear to require superuser privileges. The new flags are stored in the pg_db_role_setting.setuser array. Catversion is bumped. This commit is inspired by the previous work by Steve Chavez. Discussion: https://postgr.es/m/CAPpHfdsLd6E--epnGqXENqLP6dLwuNZrPMcNYb3wJ87WR7UBOQ%40mail.gmail.com Author: Alexander Korotkov, Steve Chavez Reviewed-by: Pavel Borisov, Steve Chavez
* 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
* Add check on initial and boot values when loading GUCsMichael Paquier2022-10-31
| | | | | | | | | | | | | | | | | | | | | | | | | | This commit adds a function to perform a cross-check between the initial value of the C declaration associated to a GUC and its actual boot value in assert-enabled builds. The purpose of this is to prevent anybody reading these C declarations from being fooled by mismatched values before they are loaded at program startup. The following rules apply depending on the GUC type: * bool - can be false, or same as boot_val. * int - can be 0, or same as the boot_val. * real - can be 0.0, or same as the boot_val. * string - can be NULL, or strcmp'd equal to the boot_val. * enum - equal to the boot_val. This is done for the system as well custom GUCs loaded by external modules, which may require extension developers to adapt the C declaration of the variables used by these GUCs (testing this change with some of my own modules has allowed me to catch some stupid typos, FWIW). This may finish by being a bad experiment depending on the feedbcak received, but let's see how it goes. Author: Peter Smith Reviewed-by: Nathan Bossart, Tom Lane, Michael Paquier, 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
* Add auxiliary lists to GUC data structures for better performance.Tom Lane2022-10-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | The previous patch made addition of new GUCs cheap, but other GUC operations aren't improved and indeed get a bit slower, because hash_seq_search() is slower than just scanning a pointer array. However, most performance-critical GUC operations only need to touch a relatively small fraction of the GUCs; especially so for AtEOXact_GUC(). We can improve matters at the cost of a bit more space by adding dlist or slist links to the GUC data structures. This patch invents lists that track (1) all GUCs with non-default "source"; (2) all GUCs with nonempty state stack (implying they've been changed in the current transaction); (3) all GUCs due for reporting to the client. All of guc.c's performance-critical cases can make use of one or another of these lists to avoid searching the whole hash table. In particular, the stack list means that transaction end doesn't take time proportional to the number of GUCs, but only to the number changed in the current transaction. Discussion: https://postgr.es/m/2982579.1662416866@sss.pgh.pa.us
* Replace the sorted array of GUC variables with a hash table.Tom Lane2022-10-14
| | | | | | | | | | | | | | | | This gets rid of bsearch() in favor of hashed lookup. The main advantage is that it becomes far cheaper to add new GUCs, since we needn't re-sort the pointer array. Adding N new GUCs had been O(N^2 log N), but now it's closer to O(N). We need to sort only in SHOW ALL and equivalent functions, which are hopefully not performance-critical to anybody. Also, merge GetNumConfigOptions() into get_guc_variables(), because in a world where the set of GUCs isn't fairly static you really want to consider those two results as tied together not independent. Discussion: https://postgr.es/m/2982579.1662416866@sss.pgh.pa.us
* 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
* 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
* Cleanup useless assignments and checksMichael Paquier2022-10-04
| | | | | | | | | | | | | | This cleans up a couple of areas: - Remove XLogSegNo calculation for the last WAL segment in backup in xlog.c (7d70809 has moved this logic entirely to xlogbackup.c when building the contents of the backup history file). - Remove check on log_min_duration in analyze.c, as it is already true where this code path is reached. - Simplify call to find_option() in guc.c. Author: Ranier Vilela Reviewed-by: Masahiko Sawada Discussion: https://postgr.es/m/CAEudQArCDQQiPiFR16=yu9k5s2tp4tgEe1U1ZbkW4ofx81AWWQ@mail.gmail.com