Thread: Prevent conflicting SET options from being set

Prevent conflicting SET options from being set

From
"Qingqing Zhou"
Date:
This an item in the TODO list. But I am not sure how severe the problem is.
In my understanding, there are not so many conflicts but we may want to give
some hints to users of how to set proper GUC variables. For instance, if we
find "debug_print_parse" is set, we will suggest setting
"debug_pretty_print" as well.

To support:
1. Prevent conflicting set options;
2. Suggest correct/better options;

We make two modifications:
1. Add a member to current config_pool structure
struct config_bool
{   struct config_generic gen;    ...   char* test_cond;    /* Condition to be tested before the new value is
set */
};

Member test_cond saves a SQL command, which expresses the suggestions and
requirements of the setting.

2. Create a temp table pg_guc at the backend startup time. Each column of
this table is a guc variable. That is, the pg_guc will have the following
columns (debug_print_parse char(5), debug_pretty_print char(5), ...) - the
data type could be refined;

Now, we could define "test_cond" of debug_print_parse like this:
"SELECT   CASE WHEN debug_pretty_print='true' THEN 'ok'   ELSE    CASE WHEN debug_print_parse='true' THEN 'You\'d
betterset
 
debug_pretty_print as well'    -- otherwise, leave to assign_hook    END   END
AS result FROM pg_guc;"

When SET command is issued, PG will run test_cond as a SQL command, and
check the result. Of course, assign_hook will still be needed.

In this design, our ability to check the conflicting SET or give suggestions
is contributed by SQL's functionality. This is good I think. But there is
one thing: we can't execute a SQL command at any time. For example, at
server startup. Meanwhile, we will keep a redundant(to guc variables) temp
table to store these values, this is a kind of waste.

Regards,
Qingqing




Re: Prevent conflicting SET options from being set

From
Tom Lane
Date:
"Qingqing Zhou" <zhouqq@cs.toronto.edu> writes:
> In this design, our ability to check the conflicting SET or give suggestions
> is contributed by SQL's functionality. This is good I think.

This is out of the question, because we have to be able to execute SET
in environments where we don't have database access nor the ability to
run transactions (eg, postmaster startup).  Nor is it obvious that SQL
is the most useful language to make such checks in anyway.  For instance
a SQL command wouldn't have access to internal backend state, which it
might need to decide if a setting is valid.

We already have the ability to issue custom messages in assign_hooks,
and I think that's sufficient in practice.

A bigger problem with making cross-variable validity checks is that you
can't really do it per-variable without falling foul of ordering
considerations.  For instance, the existing checks in
assign_stage_log_stats and assign_log_stats are ill-considered because
they may reject a postgresql.conf file that turns one variable on before
turning the other off.
        regards, tom lane


Re: Prevent conflicting SET options from being set

From
"Qingqing Zhou"
Date:
"Tom Lane" <tgl@sss.pgh.pa.us> writes
>
> We already have the ability to issue custom messages in assign_hooks,
> and I think that's sufficient in practice.

Yes, I agree this is already sufficient - seems we need to remove that TODO
item in the list.

>
> A bigger problem with making cross-variable validity checks is that you
> can't really do it per-variable without falling foul of ordering
> considerations.  For instance, the existing checks in
> assign_stage_log_stats and assign_log_stats are ill-considered because
> they may reject a postgresql.conf file that turns one variable on before
> turning the other off.
>

This problem also applies to "debug_pretty_print" and "debug_print_parse". A
possible but costly solution is that after each SET, we have to validate
*all* the GUC variables again to see if there is any conflicts.

Regards,
Qingqing




Re: Prevent conflicting SET options from being set

From
Bruce Momjian
Date:
Qingqing Zhou wrote:
> 
> "Tom Lane" <tgl@sss.pgh.pa.us> writes
> >
> > We already have the ability to issue custom messages in assign_hooks,
> > and I think that's sufficient in practice.
> 
> Yes, I agree this is already sufficient - seems we need to remove that TODO
> item in the list.

Removed.  I think we have all the conflicting options fixed already:

<       o Prevent conflicting SET options from being set
<
<         This requires a checking function to be called after the server
<         configuration file is read.

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073
 


Re: Prevent conflicting SET options from being set

From
"Qingqing Zhou"
Date:
"Bruce Momjian" <pgman@candle.pha.pa.us> writes
> Removed.  I think we have all the conflicting options fixed already:
>

One more thing, there is a small typo in TODO list: duplidated "Allow a warm
standby system to also allow read-only queries ".

Regards,
Qingqing




Re: Prevent conflicting SET options from being set

From
Bruce Momjian
Date:
Thanks, fixed.

---------------------------------------------------------------------------

Qingqing Zhou wrote:
> 
> "Bruce Momjian" <pgman@candle.pha.pa.us> writes
> > Removed.  I think we have all the conflicting options fixed already:
> >
> 
> One more thing, there is a small typo in TODO list: duplidated "Allow a warm
> standby system to also allow read-only queries ".
> 
> Regards,
> Qingqing
> 
> 
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
> 
>                http://archives.postgresql.org
> 

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073