[Util] Warn and Remove Invalid GUCs
Hi Hackers,
Currently, if there's a typo in an extension name while adding a GUC to postgresql.conf, PostgreSQL server starts up silently without any warning. This can be misleading, as one might assume the configuration has been correctly applied, when in fact the value hasn’t been set due to the typo.
To improve this experience, I’m proposing a patch that issues a warning for such invalid GUC entries. This can serve as a helpful acknowledgment, especially in cases where the mistake is unintentional (e.g., a simple typo).
For example, if I mistakenly add pg_stut_statements.max = 15000 (observe the typo in the extension name) to postgresql.conf and restart PostgreSQL, the server currently starts without indicating any issue. However, with this patch, the startup output will include a warning like:
WARNING: invalid configuration parameter name "pg_stut_statements.max", removing it
2025-05-21 17:10:45 GMT::@:[44158]:DETAIL: "pg_stut_statements.max" doesn't has a reserved prefix.
I believe providing such feedback during startup can be quite helpful for users, especially to catch and correct typos early.
Please find the attached patch for your consideration.
Thanks & Regards,
Shaik Mohammad Mujeeb
Member Technical Staff
Zoho Corp
Attachments:
warn_and_remove_invalid_prefix_gucs.patchapplication/octet-stream; name=warn_and_remove_invalid_prefix_gucs.patchDownload
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index 43b4dbccc3d..f68128650ac 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -1891,6 +1891,8 @@ load_libraries(const char *libraries, const char *gucname, bool restricted)
pfree(expanded);
}
+ WarnAndRemoveInvalidGUCs();
+
list_free_deep(elemlist);
pfree(rawstring);
}
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 667df448732..7dd8d07c7c2 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -254,6 +254,7 @@ static bool validate_option_array_item(const char *name, const char *value,
static void write_auto_conf_file(int fd, const char *filename, ConfigVariable *head);
static void replace_auto_config_value(ConfigVariable **head_p, ConfigVariable **tail_p,
const char *name, const char *value);
+static bool has_valid_class_prefix(const char *name);
static bool valid_custom_variable_name(const char *name);
static bool assignable_custom_variable_name(const char *name, bool skip_errors,
int elevel);
@@ -1065,6 +1066,38 @@ add_guc_variable(struct config_generic *var, int elevel)
return true;
}
+/*
+ * Check if the given GUC name has a valid class prefix.
+ *
+ * A valid class prefix is defined as a known reserved prefix
+ * (e.g., "pg_stat_statements") that appears
+ * before the qualifier separator ('.') in the GUC name.
+ *
+ * This function extracts the portion of the name before the
+ * first '.' and compares it against the list of reserved prefixes.
+ *
+ * Returns true if a valid prefix is found; false otherwise.
+ */
+static bool
+has_valid_class_prefix(const char *name){
+ /* If there's no separator, it can't be a custom variable */
+ const char *sep = strchr(name, GUC_QUALIFIER_SEPARATOR);
+ size_t class_len = sep - name;
+ ListCell *lc;
+ foreach(lc, reserved_class_prefix)
+ {
+ const char *rcprefix = lfirst(lc);
+
+ if (strlen(rcprefix) == class_len &&
+ strncmp(name, rcprefix, class_len) == 0)
+ {
+ return true;
+ }
+ }
+
+ return false;
+}
+
/*
* Decide whether a proposed custom variable name is allowed.
*
@@ -5268,6 +5301,23 @@ DefineCustomEnumVariable(const char *name,
define_custom_variable(&var->gen);
}
+/*
+ * remove_gucvar
+ *
+ * Removes a GUC variable from the GUC hash table,
+ * and also from any associated internal lists.
+ */
+static inline void remove_gucvar(struct config_generic *gucvar)
+{
+ /* Remove it from the hash table */
+ hash_search(guc_hashtab,
+ &gucvar->name,
+ HASH_REMOVE,
+ NULL);
+ /* Remove it from any lists it's in, too */
+ RemoveGUCFromLists(gucvar);
+}
+
/*
* Mark the given GUC prefix as "reserved".
*
@@ -5305,13 +5355,7 @@ MarkGUCPrefixReserved(const char *className)
var->name),
errdetail("\"%s\" is now a reserved prefix.",
className)));
- /* Remove it from the hash table */
- hash_search(guc_hashtab,
- &var->name,
- HASH_REMOVE,
- NULL);
- /* Remove it from any lists it's in, too */
- RemoveGUCFromLists(var);
+ remove_gucvar(var);
}
}
@@ -5321,6 +5365,39 @@ MarkGUCPrefixReserved(const char *className)
MemoryContextSwitchTo(oldcontext);
}
+/*
+ * WarnAndRemoveInvalidGUCs
+ *
+ * Iterates over the GUC hash table and identifies any custom placeholders
+ * (i.e., parameters not known at compile time) that do not follow the expected
+ * naming convention (i.e., do not have a valid reserved prefix).
+ *
+ * For each such invalid GUC, a warning is emitted and the GUC is removed from
+ * the system. This helps catch configuration mistakes such as typos in
+ * extension-specific GUC names, which would otherwise be silently ignored.
+ */
+void WarnAndRemoveInvalidGUCs(){
+ HASH_SEQ_STATUS status;
+ GUCHashEntry *hentry;
+
+ /* Warn and remove invalid placeholders. */
+ hash_seq_init(&status, guc_hashtab);
+ while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
+ {
+ struct config_generic *var = hentry->gucvar;
+
+ if((var->flags & GUC_CUSTOM_PLACEHOLDER) != 0 && !has_valid_class_prefix(var->name)){
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_NAME),
+ errmsg("invalid configuration parameter name \"%s\", removing it",
+ var->name),
+ errdetail("\"%s\" doesn't has a reserved prefix.",
+ var->name)));
+ remove_gucvar(var);
+ }
+ }
+}
+
/*
* Return an array of modified GUC options to show in EXPLAIN.
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index f619100467d..7de00f63def 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -390,6 +390,7 @@ extern void DefineCustomEnumVariable(const char *name,
GucShowHook show_hook) pg_attribute_nonnull(1, 4);
extern void MarkGUCPrefixReserved(const char *className);
+extern void WarnAndRemoveInvalidGUCs(void);
/* old name for MarkGUCPrefixReserved, for backwards compatibility: */
#define EmitWarningsOnPlaceholders(className) MarkGUCPrefixReserved(className)
Import Notes
Reply to msg id not found:
On Wednesday, May 21, 2025, Shaik Mohammad Mujeeb <mujeeb.sk@zohocorp.com>
wrote:
Currently, if there's a typo in an extension name while adding a GUC to
postgresql.conf, PostgreSQL server starts up silently without any warning.
Because any such setting name is perfectly valid (it serves as a
placeholder) and whether it is a typo or just some valid unregistered
prefix is not something the system can know.
David J.
Shaik Mohammad Mujeeb <mujeeb.sk@zohocorp.com> writes:
Currently, if there's a typo in an extension name while adding a GUC to postgresql.conf, PostgreSQL server starts up silently without any warning. This can be misleading, as one might assume the configuration has been correctly applied, when in fact the value hasn’t been set due to the typo.
Well, yeah, because the core server has no way to identify bogus
extension GUCs if the relevant extension isn't loaded. We do
already complain about such things at extension load time.
To improve this experience, I’m proposing a patch that issues a
warning for such invalid GUC entries.
How will you know they are invalid? All I see in the patch is
a syntactic check, which looks quite redundant with
assignable_custom_variable_name().
regards, tom lane
Hi,
On Thu, May 22, 2025 at 2:09 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Shaik Mohammad Mujeeb <mujeeb.sk@zohocorp.com> writes:
Currently, if there's a typo in an extension name while adding a GUC to
postgresql.conf, PostgreSQL server starts up silently without any warning.
This can be misleading, as one might assume the configuration has been
correctly applied, when in fact the value hasn’t been set due to the typo.Well, yeah, because the core server has no way to identify bogus
extension GUCs if the relevant extension isn't loaded. We do
already complain about such things at extension load time.
the extension is loaded and then i entered the bogus extension GUC into
postgresql.conf and restarted, i did not observe any complain/warning .
To improve this experience, I’m proposing a patch that issues a
warning for such invalid GUC entries.How will you know they are invalid? All I see in the patch is
a syntactic check, which looks quite redundant with
assignable_custom_variable_name().
after the extension is loaded, MarkGUCPrefixReserved() appends
reserved_class_prefix with extension name ,so this patch has code to check
if the the GUC's prefix from the postgresql.conf is in the
reserved_class_prefix or not ,if not it invalidates and throws
warning.Please correct me if i am wrong.
+static bool
+has_valid_class_prefix(const char *name){
+ /* If there's no separator, it can't be a custom variable */
+ const char *sep = strchr(name, GUC_QUALIFIER_SEPARATOR);
+ size_t class_len = sep - name;
+ ListCell *lc;
+ foreach(lc, reserved_class_prefix)
+ {
+ const char *rcprefix = lfirst(lc);
+
+ if (strlen(rcprefix) == class_len &&
+ strncmp(name, rcprefix, class_len) == 0)
+ {
+ return true;
+ }
+ }
+
+ return false;
+}+void WarnAndRemoveInvalidGUCs(){
+ HASH_SEQ_STATUS status;
+ GUCHashEntry *hentry;
+
+ /* Warn and remove invalid placeholders. */
+ hash_seq_init(&status, guc_hashtab);
+ while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
+ {
+ struct config_generic *var = hentry->gucvar;
+
+ if((var->flags & GUC_CUSTOM_PLACEHOLDER) != 0 &&
!has_valid_class_prefix(var->name)){
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_NAME),
+ errmsg("invalid configuration parameter name \"%s\", removing it",
+ var->name),
+ errdetail("\"%s\" doesn't has a reserved prefix.",
+ var->name)));
+ remove_gucvar(var);
+ }
+ }
+}
+--
Thanks,
Srinath Reddy Sadipiralla
EDB: https://www.enterprisedb.com/
Srinath Reddy Sadipiralla <srinath2133@gmail.com> writes:
the extension is loaded and then i entered the bogus extension GUC into
postgresql.conf and restarted, i did not observe any complain/warning .
Were you looking in the right place? I experimented with adding
shared_preload_libraries = 'plpgsql' # (change requires restart)
plpgsql.bogus = 1
to postgresql.conf. Restarting the server, I see in the
postmaster log:
2025-05-22 11:16:45.724 EDT [1526138] WARNING: invalid configuration parameter name "plpgsql.bogus", removing it
2025-05-22 11:16:45.724 EDT [1526138] DETAIL: "plpgsql" is now a reserved prefix.
2025-05-22 11:16:45.728 EDT [1526138] LOG: starting PostgreSQL 18beta1 on x86_64-pc-linux-gnu, compiled by gcc (GCC) 8.5.0 20210514 (Red Hat 8.5.0-26), 64-bit
... etc etc ...
(I also tried the other order of these settings, to see if that made
any difference, but it didn't.) I also tried
session_preload_libraries = 'plpgsql'
plpgsql.bogus = 1
and then the complaint comes out during any session start:
$ psql
WARNING: invalid configuration parameter name "plpgsql.bogus", removing it
DETAIL: "plpgsql" is now a reserved prefix.
psql (18beta1)
Type "help" for help.
That's all operating as intended, and I'm not seeing how the proposed
patch isn't completely redundant with it.
regards, tom lane
Hi David J,
Because any such setting name is perfectly valid (it serves as a placeholder) and whether it is a typo or just some valid unregistered prefix is not something the system can know.
In my patch, I currently warn and remove invalid GUCs from the hashtable. However, as you rightly pointed out, some of these could belong to valid but unregistered prefixes. In such cases, it might not be ideal to remove them outright. Instead, it could be more helpful to simply warn the user - covering both potential typos and GUCs with valid yet unregistered prefixes.
I do understand that not everyone may prefer seeing such warnings during PG server restart. To address this, we could introduce a new GUC (perhaps named warn_on_unregistered_guc_prefix), which defaults to false, preserving the existing behaviour. If explicitly enabled, it would emit warnings for these cases, giving users the choice to opt in to this feedback.
Thoughts on this approach?
Thanks & Regards,
Shaik Mohammad Mujeeb
Member Technical Staff
Zoho Corp
---- On Thu, 22 May 2025 02:01:25 +0530 David G. Johnston <david.g.johnston@gmail.com> wrote ---
On Wednesday, May 21, 2025, Shaik Mohammad Mujeeb <mailto:mujeeb.sk@zohocorp.com> wrote:
Currently, if there's a typo in an extension name while adding a GUC to postgresql.conf, PostgreSQL server starts up silently without any warning.
Because any such setting name is perfectly valid (it serves as a placeholder) and whether it is a typo or just some valid unregistered prefix is not something the system can know.
David J.
On Thu, May 22, 2025 at 9:00 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Srinath Reddy Sadipiralla <srinath2133@gmail.com> writes:
the extension is loaded and then i entered the bogus extension GUC into
postgresql.conf and restarted, i did not observe any complain/warning .Were you looking in the right place? I experimented with adding
shared_preload_libraries = 'plpgsql' # (change requires restart)
plpgsql.bogus = 1to postgresql.conf. Restarting the server, I see in the
postmaster log:2025-05-22 11:16:45.724 EDT [1526138] WARNING: invalid configuration
parameter name "plpgsql.bogus", removing it
2025-05-22 11:16:45.724 EDT [1526138] DETAIL: "plpgsql" is now a reserved
prefix.
2025-05-22 11:16:45.728 EDT [1526138] LOG: starting PostgreSQL 18beta1 on
x86_64-pc-linux-gnu, compiled by gcc (GCC) 8.5.0 20210514 (Red Hat
8.5.0-26), 64-bit
... etc etc ...
Tom, the problem is when the prefix is a typo ,my bad i should have
specified as bogus prefix instead of bogus GUC ,can you please try again
with
"lpgsql.bogus = 1" .
--
Thanks,
Srinath Reddy Sadipiralla
EDB: https://www.enterprisedb.com/
Hi Tom Lane,
Well, yeah, because the core server has no way to identify bogus
extension GUCs if the relevant extension isn't loaded. We do
already complain about such things at extension load time.
Yes, currently we do issue warnings at extension load time if a GUC with a valid prefix contains an invalid or misspelled suffix (e.g., plpgsql.bogus, plpgsql.abc, plpgsql.xyz). In such cases, only the part after the dot is invalid, so the warning helps users catch typos early.
However, if the typo exists in the extension name itself (e.g., plppsql.extra_errors, plpgaql.extra_errors, plpgsqi.extra_errors), we currently don’t provide any acknowledgment. In these examples, the prefix is invalid or misspelled, and it would be helpful to notify the user so they have a chance to correct it.
I do agree that in cases of unrecognised prefixes, it’s possible they may either be typos or valid but unregistered prefixes. So instead of removing such GUCs from the hashtable - as my current patch does - it would be more appropriate to just issue a warning.
Of course, I understand that not all users may want to see these warnings. To make this behaviour optional, we could introduce a new GUC (e.g., warn_on_unregistered_guc_prefix), which defaults to false to preserve existing behaviour. When set to true, it would enable warnings for unregistered or potentially misspelled GUC prefixes, giving users the flexibility to opt in.
How will you know they are invalid? All I see in the patch is
a syntactic check, which looks quite redundant with
assignable_custom_variable_name().
After all the shared preload libraries are loaded, the reserved_class_prefix list contains the valid prefixes which we can refer to for this validation. I’m performing this check only after all such libraries are loaded. While this might look similar to the logic in assignable_custom_variable_name(), the purpose here is different.
I hope this helps clarify the intention behind the proposal, and I’d appreciate any thoughts or feedback on this approach.
Thanks & Regards,
Shaik Mohammad Mujeeb
Member Technical Staff
Zoho Corp
---- On Thu, 22 May 2025 02:08:58 +0530 Tom Lane <tgl@sss.pgh.pa.us> wrote ---
Shaik Mohammad Mujeeb <mailto:mujeeb.sk@zohocorp.com> writes:
Currently, if there's a typo in an extension name while adding a GUC to postgresql.conf, PostgreSQL server starts up silently without any warning. This can be misleading, as one might assume the configuration has been correctly applied, when in fact the value hasn’t been set due to the typo.
Well, yeah, because the core server has no way to identify bogus
extension GUCs if the relevant extension isn't loaded. We do
already complain about such things at extension load time.
To improve this experience, I’m proposing a patch that issues a
warning for such invalid GUC entries.
How will you know they are invalid? All I see in the patch is
a syntactic check, which looks quite redundant with
assignable_custom_variable_name().
regards, tom lane
Srinath Reddy Sadipiralla <srinath2133@gmail.com> writes:
Tom, the problem is when the prefix is a typo ,my bad i should have
specified as bogus prefix instead of bogus GUC ,can you please try again
with
"lpgsql.bogus = 1" .
[ shrug... ] How do you know that's a bogus prefix? It could
perfectly well be a fully valid setting for an extension that
the installation doesn't choose to preload.
regards, tom lane
Shaik Mohammad Mujeeb <mujeeb.sk@zohocorp.com> writes:
In my patch, I currently warn and remove invalid GUCs from the hashtable. However, as you rightly pointed out, some of these could belong to valid but unregistered prefixes. In such cases, it might not be ideal to remove them outright. Instead, it could be more helpful to simply warn the user - covering both potential typos and GUCs with valid yet unregistered prefixes.
I kind of doubt that warnings at startup are all that helpful.
People don't look at the postmaster log all that often, and
by the time they start wondering why something isn't acting
as they expect, the log file that had the info may have been
rotated out of existence.
I doubt even more that removing GUCs just because they are
not registered prefixes is sane.
Let me suggest a different way of thinking about this: what
say we extend the pg_file_settings view to mark custom GUCs
that don't correspond to any reserved extension prefix?
"Not a known extension" could be one of the "error" messages
that that view provides.
regards, tom lane
On Thu, May 22, 2025 at 12:10 PM Shaik Mohammad Mujeeb
<mujeeb.sk@zohocorp.com> wrote:
In my patch, I currently warn and remove invalid GUCs from the hashtable. However, as you rightly pointed out, some of these could belong to valid but unregistered prefixes. In such cases, it might not be ideal to remove them outright. Instead, it could be more helpful to simply warn the user - covering both potential typos and GUCs with valid yet unregistered prefixes.
I do understand that not everyone may prefer seeing such warnings during PG server restart. To address this, we could introduce a new GUC (perhaps named warn_on_unregistered_guc_prefix), which defaults to false, preserving the existing behaviour. If explicitly enabled, it would emit warnings for these cases, giving users the choice to opt in to this feedback.
I think you might be missing the point of the comments from Tom and
David. To the extent that it is possible to give warnings, we already
do. So this proposal just doesn't really make sense. It either warns
in cases where there is no actual problem, or it gives a duplicate
warning in cases where there is. Changing the details of the proposal
doesn't address that fundamental problem.
I would really encourage you to spend a bit more time trying to
understand the current design intention and behavior before proposing
changes. It actually makes a lot of sense. It is not perfect, but if
there were a simple way to do better we would have likely done that a
long time ago.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Thu, May 22, 2025 at 12:45 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
"lpgsql.bogus = 1" .
[ shrug... ] How do you know that's a bogus prefix? It could perfectly
well be a fully valid setting for an extension that
the installation doesn't choose to preload.
Well, we do have ways to view all *potential* extensions. I find myself
more sympathetic to the OP than the others here, as I think it's a good
idea to display a warning for a potentially misspelled GUC prefix (but do
nothing else - certainly not remove it!). Will such a warning be seen?
Perhaps not, but that's a risk any warning message has to face. And
obviously there could be false positives if an extension decides to name
their GUCs something other than pg_available_extensions()->name but
hopefully that's a rare case (and shame on them if so.)
Cheers,
Greg
--
Crunchy Data - https://www.crunchydata.com
Enterprise Postgres Software Products & Tech Support
On Thu, May 22, 2025 at 1:20 PM Greg Sabino Mullane <htamfids@gmail.com>
wrote:
On Thu, May 22, 2025 at 12:45 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
"lpgsql.bogus = 1" .
[ shrug... ] How do you know that's a bogus prefix? It could perfectly
well be a fully valid setting for an extension that
the installation doesn't choose to preload.Well, we do have ways to view all *potential* extensions. I find myself
more sympathetic to the OP than the others here, as I think it's a good
idea to display a warning for a potentially misspelled GUC prefix (but do
nothing else - certainly not remove it!). Will such a warning be seen?
Perhaps not, but that's a risk any warning message has to face. And
obviously there could be false positives if an extension decides to name
their GUCs something other than pg_available_extensions()->name but
hopefully that's a rare case (and shame on them if so.)
We don't have proper server variables for applications and non-extensions
to use in things like RLS so choosing a non-extension-registered prefix is
a perfectly valid thing to do. Maybe it would seldom be done in the config
file, and be nominally reserved for SET/set_config, but it is still valid.
IMO, an improvement in this area would revolve around not making people
type out setting names manually at all. If they don't type them they
cannot typo them. Again, this also only improves the config file setup,
but why not add pre-made configuration samples in a subdirectory that
people can either copy/paste individually; or setup a default include_dir
directive for contrib and people can place the "sample" files into the live
"contrib.d" directory making them no longer just samples but live
configuration that can then be edited without having to type names - just
remove comment symbols (maybe) and change values.
David J.
On Thu, May 22, 2025 at 8:43 AM Shaik Mohammad Mujeeb <
mujeeb.sk@zohocorp.com> wrote:
I do understand that not everyone may prefer seeing such warnings during
PG server restart. To address this, we could introduce a new GUC (perhaps
named *warn_on_unregistered_guc_prefix*), which defaults to false,
preserving the existing behaviour. If explicitly enabled, it would emit
warnings for these cases, giving users the choice to opt in to this
feedback.Thoughts on this approach?
The need for yet another GUC makes this considerably less appealing than it
already is.
I see and agree with the problem statement posed here but would prefer an
approach that improves the UX to minimize such mistakes or encourages
people to check their settings more easily to ensure that they didn't type
100 when they meant to type 10 for the correct setting name. In short,
warning in limited places for a subset of potential errors when doing so
involves false positives is just an unappealing change.
David J.
Hi Greg,
Well, we do have ways to view all *potential* extensions. I find myself more sympathetic to the OP than the others here, as I think it's a good idea to display a warning for a potentially misspelled GUC prefix (but do nothing else - certainly not remove it!).
I'm really glad that you understood the problem statement clearly - thank you for the positive response.
Will such a warning be seen? Perhaps not, but that's a risk any warning message has to face. And obviously there could be false positives if an extension decides to name their GUCs something other than pg_available_extensions()->name but hopefully that's a rare case (and shame on them if so.)
I do agree that such warnings might sometimes go unnoticed in the logs. However, by configuring specific log-related GUCs, it's possible to surface Postmaster warnings directly on the terminal during a PostgreSQL restart, rather than just logging them to a file. We've been using this setup in our production environments, and it's proven helpful in identifying typos right at server startup.
For example,
~ pgrestart
waiting for server to shut down.... done
server stopped
waiting for server to start....2025-05-23 13:48:20 GMT::@:[29146]:WARNING: invalid configuration parameter name "pg_stut_statements.max"
2025-05-23 13:48:20 GMT::@:[29146]:DETAIL: "pg_stut_statements.max" doesn't has a reserved prefix.
2025-05-23 13:48:21 GMT::@:[29146]:LOG: redirecting log output to logging collector process
2025-05-23 13:48:21 GMT::@:[29146]:HINT: Future log output will appear in directory "pg_log".
done
server started
So I felt this approach might be beneficial to others as well.
Regards,
Shaik Mohammad Mujeeb
Member Technical Staff
Zoho Corp
---- On Fri, 23 May 2025 01:49:53 +0530 Greg Sabino Mullane <htamfids@gmail.com> wrote ---
On Thu, May 22, 2025 at 12:45 PM Tom Lane <mailto:tgl@sss.pgh.pa.us> wrote:
"lpgsql.bogus = 1" .
[ shrug... ] How do you know that's a bogus prefix? It could perfectly well be a fully valid setting for an extension that
the installation doesn't choose to preload.
Well, we do have ways to view all *potential* extensions. I find myself more sympathetic to the OP than the others here, as I think it's a good idea to display a warning for a potentially misspelled GUC prefix (but do nothing else - certainly not remove it!). Will such a warning be seen? Perhaps not, but that's a risk any warning message has to face. And obviously there could be false positives if an extension decides to name their GUCs something other than pg_available_extensions()->name but hopefully that's a rare case (and shame on them if so.)
Cheers,
Greg
--
Crunchy Data - https://www.crunchydata.com
Enterprise Postgres Software Products & Tech Support
Hi Robert,
I think you might be missing the point of the comments from Tom and
David. To the extent that it is possible to give warnings, we already
do. So this proposal just doesn't really make sense. It either warns
in cases where there is no actual problem, or it gives a duplicate
warning in cases where there is.
As far as I know, there shouldn’t be any duplicate warnings. But if you happen to come across a case where duplicates do occur, please do let me know - I’d be happy to revisit and correct my understanding.
I would really encourage you to spend a bit more time trying to
understand the current design intention and behavior before proposing
changes. It actually makes a lot of sense. It is not perfect, but if
there were a simple way to do better we would have likely done that a
long time ago.
Thank you for the feedback - I do appreciate the depth of the current design and the effort that has gone into shaping it over the years. That said, I'm trying to understand it better by exploring edge cases and practical pain points we've encountered in real deployments. My intention is not to disregard the existing design, but to see if there's any room for incremental improvement or at least discussion.
I understand that if there were an easy fix, it might have already been considered - but sometimes fresh perspectives can help uncover new angles. I’ll definitely take your suggestion to spend more time with the current behaviour seriously and will make sure any future proposals are better informed.
Regards,
Shaik Mohammad Mujeeb
Member Technical Staff
Zoho Corp
---- On Thu, 22 May 2025 22:22:41 +0530 Robert Haas <robertmhaas@gmail.com> wrote ---
On Thu, May 22, 2025 at 12:10 PM Shaik Mohammad Mujeeb
<mailto:mujeeb.sk@zohocorp.com> wrote:
In my patch, I currently warn and remove invalid GUCs from the hashtable. However, as you rightly pointed out, some of these could belong to valid but unregistered prefixes. In such cases, it might not be ideal to remove them outright. Instead, it could be more helpful to simply warn the user - covering both potential typos and GUCs with valid yet unregistered prefixes.
I do understand that not everyone may prefer seeing such warnings during PG server restart. To address this, we could introduce a new GUC (perhaps named warn_on_unregistered_guc_prefix), which defaults to false, preserving the existing behaviour. If explicitly enabled, it would emit warnings for these cases, giving users the choice to opt in to this feedback.
I think you might be missing the point of the comments from Tom and
David. To the extent that it is possible to give warnings, we already
do. So this proposal just doesn't really make sense. It either warns
in cases where there is no actual problem, or it gives a duplicate
warning in cases where there is. Changing the details of the proposal
doesn't address that fundamental problem.
I would really encourage you to spend a bit more time trying to
understand the current design intention and behavior before proposing
changes. It actually makes a lot of sense. It is not perfect, but if
there were a simple way to do better we would have likely done that a
long time ago.
--
Robert Haas
EDB: http://www.enterprisedb.com