From 7353219d2da63f6a780fa95b3742a27233f54f3a Mon Sep 17 00:00:00 2001 From: Nitin Jadhav Date: Sat, 4 Feb 2023 19:16:57 +0000 Subject: [PATCH] Fix GUC_NO_SHOW_ALL test scenarios The existing test does not validates GUC_NO_SHOW_ALL scenarios properly as pg_show_all_settings() does not fetch GUC_NO_SHOW_ALL parameters. Added a policy to verify GUC_NO_SHOW_ALL requires GUC_NOT_IN_SAMPLE in guc.c. Removed a policy which checks for GUC_NO_SHOW_ALL AND NOT GUC_NO_RESET_ALL as there are few parameters which matches this criteria. --- src/backend/utils/misc/guc.c | 22 ++++++++++++++++++---- src/test/regress/expected/guc.out | 23 ++++------------------- src/test/regress/sql/guc.sql | 15 ++++----------- 3 files changed, 26 insertions(+), 34 deletions(-) diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 978b385568..ec04d50a9c 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -1383,11 +1383,14 @@ check_GUC_name_for_parameter_acl(const char *name) } /* - * Routine in charge of checking that the initial value of a GUC is the - * same when declared and when loaded to prevent anybody looking at the - * C declarations of these GUCS from being fooled by mismatched values. + * Routine in charge of checking various states of a GUC. * - * The following validation rules apply: + * This performs two sanity checks. First, it checks that the initial + * value of a GUC is the same when declared and when loaded to prevent + * anybody looking at the C declarations of these GUCS from being fooled by + * mismatched values. Second, it checks for incorrect flag combinations. + * + * The following validation rules apply for the values: * bool - can be false, otherwise must be same as the boot_val * int - can be 0, otherwise must be same as the boot_val * real - can be 0.0, otherwise must be same as the boot_val @@ -1398,6 +1401,7 @@ check_GUC_name_for_parameter_acl(const char *name) static bool check_GUC_init(struct config_generic *gconf) { + /* Checks on values */ switch (gconf->vartype) { case PGC_BOOL: @@ -1462,6 +1466,16 @@ check_GUC_init(struct config_generic *gconf) } } + /* Flag combinations */ + /* GUC_NO_SHOW_ALL requires GUC_NOT_IN_SAMPLE */ + if ((gconf->flags & GUC_NO_SHOW_ALL) && + !(gconf->flags & GUC_NOT_IN_SAMPLE)) + { + elog(LOG, "GUC %s flags: NO_SHOW_ALL and !NOT_IN_SAMPLE", + gconf->name); + return false; + } + return true; } #endif diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out index 2914b950b4..36ccbbd220 100644 --- a/src/test/regress/expected/guc.out +++ b/src/test/regress/expected/guc.out @@ -841,7 +841,6 @@ CREATE TABLE tab_settings_flags AS SELECT name, category, 'EXPLAIN' = ANY(flags) AS explain, 'NO_RESET' = ANY(flags) AS no_reset, 'NO_RESET_ALL' = ANY(flags) AS no_reset_all, - 'NO_SHOW_ALL' = ANY(flags) AS no_show_all, 'NOT_IN_SAMPLE' = ANY(flags) AS not_in_sample, 'RUNTIME_COMPUTED' = ANY(flags) AS runtime_computed FROM pg_show_all_settings() AS psas, @@ -880,17 +879,11 @@ SELECT name FROM tab_settings_flags ------ (0 rows) --- NO_SHOW_ALL implies NO_RESET_ALL, and vice-versa. +-- 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 no_show_all AND NOT no_reset_all - ORDER BY 1; - name ------- -(0 rows) - --- Exceptions are transaction_*. -SELECT name FROM tab_settings_flags - WHERE NOT no_show_all AND no_reset_all + WHERE no_reset_all ORDER BY 1; name ------------------------ @@ -899,14 +892,6 @@ SELECT name FROM tab_settings_flags transaction_read_only (3 rows) --- NO_SHOW_ALL implies NOT_IN_SAMPLE. -SELECT name FROM tab_settings_flags - WHERE no_show_all AND NOT not_in_sample - ORDER BY 1; - name ------- -(0 rows) - -- NO_RESET implies NO_RESET_ALL. SELECT name FROM tab_settings_flags WHERE no_reset AND NOT no_reset_all diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql index d40d0c178f..907cc1e7f7 100644 --- a/src/test/regress/sql/guc.sql +++ b/src/test/regress/sql/guc.sql @@ -326,7 +326,6 @@ CREATE TABLE tab_settings_flags AS SELECT name, category, 'EXPLAIN' = ANY(flags) AS explain, 'NO_RESET' = ANY(flags) AS no_reset, 'NO_RESET_ALL' = ANY(flags) AS no_reset_all, - 'NO_SHOW_ALL' = ANY(flags) AS no_show_all, 'NOT_IN_SAMPLE' = ANY(flags) AS not_in_sample, 'RUNTIME_COMPUTED' = ANY(flags) AS runtime_computed FROM pg_show_all_settings() AS psas, @@ -349,17 +348,11 @@ SELECT name FROM tab_settings_flags SELECT name FROM tab_settings_flags WHERE category = 'Preset Options' AND NOT not_in_sample ORDER BY 1; --- NO_SHOW_ALL implies NO_RESET_ALL, and vice-versa. +-- 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 no_show_all AND NOT no_reset_all - ORDER BY 1; --- Exceptions are transaction_*. -SELECT name FROM tab_settings_flags - WHERE NOT no_show_all AND no_reset_all - ORDER BY 1; --- NO_SHOW_ALL implies NOT_IN_SAMPLE. -SELECT name FROM tab_settings_flags - WHERE no_show_all AND NOT not_in_sample + WHERE no_reset_all ORDER BY 1; -- NO_RESET implies NO_RESET_ALL. SELECT name FROM tab_settings_flags -- 2.25.1