Re: Fix GUC_NO_SHOW_ALL test scenario in 003_check_guc.pl
| От | Michael Paquier |
|---|---|
| Тема | Re: Fix GUC_NO_SHOW_ALL test scenario in 003_check_guc.pl |
| Дата | |
| Msg-id | Y+Cq1pkhZjfQXKQh@paquier.xyz обсуждение исходный текст |
| Ответ на | Re: Fix GUC_NO_SHOW_ALL test scenario in 003_check_guc.pl (Nitin Jadhav <nitinjadhavpostgres@gmail.com>) |
| Ответы |
Re: Fix GUC_NO_SHOW_ALL test scenario in 003_check_guc.pl
|
| Список | pgsql-hackers |
On Sun, Feb 05, 2023 at 12:56:58AM +0530, Nitin Jadhav wrote: > Ok. Understood the other problems. I have attached the v2 patch which > uses the idea present in Michael's patch. In addition, I have removed > fetching NO_SHOW_ALL parameters while creating tab_settings_flags > table in guc.sql and adjusted the test which checks for (NO_RESET_ALL > AND NOT NO_SHOW_ALL) as this was misleading the developer who thinks > that tab_settings_flags table has NO_SHOW_ALL parameters which is > incorrect. Okay, the part to add an initialization check for GUC_NO_SHOW_ALL and GUC_NOT_IN_SAMPLE looked fine by me, so applied after more comment polishing. +-- NO_RESET_ALL can be specified without NO_SHOW_ALL, like transaction_*. +-- tab_settings_flags does not contain NO_SHOW_ALL flags. Just checking for +-- NO_RESET_ALL implies NO_RESET_ALL AND NOT NO_SHOW_ALL. SELECT name FROM tab_settings_flags - WHERE NOT no_show_all AND no_reset_all + WHERE no_reset_all ORDER BY 1; Removing entirely no_show_all is fine by me, but keeping this SQL has little sense, then, because it would include any GUCs loaded by an external source when they define NO_RESET_ALL. I think that 0001 should be like the attached, instead, backpatched down to 15 (release week, so it cannot be touched until the next version is stamped), where we just remove all the checks based on no_show_all. On top of that, I have noticed an extra combination that would not make sense and that could be checked with the SQL queries: GUC_DISALLOW_IN_FILE implies GUC_NOT_IN_SAMPLE. The opposite may not be true, though, as some developer GUCs are marked as GUC_NOT_IN_SAMPLE but they are allowed in a file. The only exception to that currently is config_file. It is just a special case whose value is enforced at startup and it can be passed down as an option switch via the postgres binary, still it seems like it would be better to also mark it as GUC_NOT_IN_SAMPLE? This is done in 0002, only for HEAD, as that would be a new check. Thoughts? -- Michael
Вложения
В списке pgsql-hackers по дате отправления: