Обсуждение: Re: [Util] Warn and Remove Invalid GUCs
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.
> 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 <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:
> In my patch, I currently warn and remove invalid GUCs from the hashtable. However, as you rightly pointed out, some
ofthese 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
unregisteredprefixes.
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 ofthese 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 unregisteredprefixes. > > I do understand that not everyone may prefer seeing such warnings during PG server restart. To address this, we could introducea 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 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 namedwarn_on_unregistered_guc_prefix), which defaults tofalse, 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 Robert,
> I think you might be missing the point of the comments from Tom and
> 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
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
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
<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