Thread: Prevent conflicting SET options from being set
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
"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
"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
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
"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
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