v2-0001-Fix-GUC_NO_SHOW_ALL-test-scenarios.patch

From 7353219d2da63f6a780fa95b3742a27233f54f3a Mon Sep 17 00:00:00 2001
From: Nitin Jadhav <nitinjadhav@microsoft.com>
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