Merge pull request #5161 from citusdata/add-check-for-gucs-order

Add check for alphabetically sorted gucs

.circleci/config.yml

@@ -82,6 +82,9 @@ jobs:
       - run:
           name: 'Check if all CI scripts are actually run'
           command: ci/check_all_ci_scripts_are_run.sh
+      - run:
+          name: 'Check if all GUCs are sorted alphabetically'
+          command: ci/check_gucs_are_alphabetically_sorted.sh
 
   check-sql-snapshots:
     docker:

ci/README.md

@@ -358,3 +358,9 @@ This script checks and fixes issues with `.gitignore` rules:
 2. Makes sure we do not commit any generated files that should be ignored. If there is an
    ignored file in the git tree, the user is expected to review the files that are removed
    from the git tree and commit them.
+
+## `check_gucs_are_alphabetically_sorted.sh`
+
+This script checks the order of the GUCs defined in `shared_library_init.c`.
+To solve this failure, please check `shared_library_init.c` and make sure that the GUC
+definitions are in alphabetical order.

ci/check_gucs_are_alphabetically_sorted.sh

@@ -0,0 +1,10 @@
+#!/bin/bash
+
+set -euo pipefail
+# shellcheck disable=SC1091
+source ci/ci_helpers.sh
+
+# extract citus gucs in the form of "citus.X"
+grep -o -E "(\.*\"citus.\w+\")," src/backend/distributed/shared_library_init.c > gucs.out
+sort -c gucs.out
+rm gucs.out

src/backend/distributed/shared_library_init.c

@@ -614,19 +614,6 @@ RegisterCitusConfigVariables(void)
 		GUC_STANDARD,
 		NULL, NULL, NULL);
 
-	DefineCustomRealVariable(
-		"citus.count_distinct_error_rate",
-		gettext_noop("Desired error rate when calculating count(distinct) "
-					 "approximates using the postgresql-hll extension. "
-					 "0.0 disables approximations for count(distinct); 1.0 "
-					 "provides no guarantees about the accuracy of results."),
-		NULL,
-		&CountDistinctErrorRate,
-		0.0, 0.0, 1.0,
-		PGC_USERSET,
-		GUC_STANDARD,
-		NULL, NULL, NULL);
-
 	DefineCustomIntVariable(
 		"citus.copy_switchover_threshold",
 		gettext_noop("Sets the threshold for copy to be switched "
@@ -643,6 +630,19 @@ RegisterCitusConfigVariables(void)
 		GUC_UNIT_BYTE | GUC_NO_SHOW_ALL,
 		NULL, NULL, NULL);
 
+	DefineCustomRealVariable(
+		"citus.count_distinct_error_rate",
+		gettext_noop("Desired error rate when calculating count(distinct) "
+					 "approximates using the postgresql-hll extension. "
+					 "0.0 disables approximations for count(distinct); 1.0 "
+					 "provides no guarantees about the accuracy of results."),
+		NULL,
+		&CountDistinctErrorRate,
+		0.0, 0.0, 1.0,
+		PGC_USERSET,
+		GUC_STANDARD,
+		NULL, NULL, NULL);
+
 	DefineCustomBoolVariable(
 		"citus.defer_drop_after_shard_move",
 		gettext_noop("When enabled a shard move will mark the original shards "
@@ -691,18 +691,6 @@ RegisterCitusConfigVariables(void)
 		GUC_STANDARD,
 		NULL, NULL, NULL);
 
-	DefineCustomBoolVariable(
-		"citus.enable_manual_changes_to_shards",
-		gettext_noop("Enables dropping and truncating known shards."),
-		gettext_noop("Set to false by default. If set to true, enables "
-					 "dropping and truncating shards on the coordinator "
-					 "(or the workers with metadata)"),
-		&EnableManualChangesToShards,
-		false,
-		PGC_USERSET,
-		GUC_NO_SHOW_ALL,
-		NULL, NULL, NULL);
-
 	DefineCustomRealVariable(
 		"citus.distributed_deadlock_detection_factor",
 		gettext_noop("Sets the time to wait before checking for distributed "
@@ -716,6 +704,17 @@ RegisterCitusConfigVariables(void)
 		GUC_STANDARD,
 		ErrorIfNotASuitableDeadlockFactor, NULL, NULL);
 
+	DefineCustomBoolVariable(
+		"citus.enable_alter_database_owner",
+		gettext_noop("Enables propagating ALTER DATABASE ... OWNER TO ... statements to "
+					 "workers"),
+		NULL,
+		&EnableAlterDatabaseOwner,
+		false,
+		PGC_USERSET,
+		GUC_NO_SHOW_ALL,
+		NULL, NULL, NULL);
+
 	DefineCustomBoolVariable(
 		"citus.enable_alter_role_propagation",
 		gettext_noop("Enables propagating ALTER ROLE statements to workers (excluding "
@@ -737,17 +736,6 @@ RegisterCitusConfigVariables(void)
 		GUC_NO_SHOW_ALL,
 		NULL, NULL, NULL);
 
-	DefineCustomBoolVariable(
-		"citus.enable_alter_database_owner",
-		gettext_noop("Enables propagating ALTER DATABASE ... OWNER TO ... statements to "
-					 "workers"),
-		NULL,
-		&EnableAlterDatabaseOwner,
-		false,
-		PGC_USERSET,
-		GUC_NO_SHOW_ALL,
-		NULL, NULL, NULL);
-
 	DefineCustomBoolVariable(
 		"citus.enable_binary_protocol",
 		gettext_noop(
@@ -759,6 +747,18 @@ RegisterCitusConfigVariables(void)
 		GUC_STANDARD,
 		NULL, NULL, NULL);
 
+	DefineCustomBoolVariable(
+		"citus.enable_cost_based_connection_establishment",
+		gettext_noop("When enabled the connection establishment times "
+					 "and task execution times into account for deciding "
+					 "whether or not to establish new connections."),
+		NULL,
+		&EnableCostBasedConnectionEstablishment,
+		true,
+		PGC_USERSET,
+		GUC_NO_SHOW_ALL,
+		NULL, NULL, NULL);
+
 	DefineCustomBoolVariable(
 		"citus.enable_create_type_propagation",
 		gettext_noop("Enables propagating of CREATE TYPE statements to workers"),
@@ -845,6 +845,18 @@ RegisterCitusConfigVariables(void)
 		GUC_STANDARD,
 		NULL, NULL, NULL);
 
+	DefineCustomBoolVariable(
+		"citus.enable_manual_changes_to_shards",
+		gettext_noop("Enables dropping and truncating known shards."),
+		gettext_noop("Set to false by default. If set to true, enables "
+					 "dropping and truncating shards on the coordinator "
+					 "(or the workers with metadata)"),
+		&EnableManualChangesToShards,
+		false,
+		PGC_USERSET,
+		GUC_NO_SHOW_ALL,
+		NULL, NULL, NULL);
+
 	DefineCustomStringVariable(
 		"citus.enable_manual_metadata_changes_for_user",
 		gettext_noop("Enables some helper UDFs to modify metadata "
@@ -1005,18 +1017,6 @@ RegisterCitusConfigVariables(void)
 		0,
 		NULL, NULL, NULL);
 
-	DefineCustomBoolVariable(
-		"citus.enable_cost_based_connection_establishment",
-		gettext_noop("When enabled the connection establishment times "
-					 "and task execution times into account for deciding "
-					 "whether or not to establish new connections."),
-		NULL,
-		&EnableCostBasedConnectionEstablishment,
-		true,
-		PGC_USERSET,
-		GUC_NO_SHOW_ALL,
-		NULL, NULL, NULL);
-
 	DefineCustomBoolVariable(
 		"citus.explain_distributed_queries",
 		gettext_noop("Enables Explain for distributed queries."),
@@ -1029,20 +1029,6 @@ RegisterCitusConfigVariables(void)
 		GUC_NO_SHOW_ALL,
 		NULL, NULL, NULL);
 
-	DefineCustomBoolVariable(
-		"citus.function_opens_transaction_block",
-		gettext_noop("Open transaction blocks for function calls"),
-		gettext_noop("When enabled, Citus will always send a BEGIN to workers when "
-					 "running distributed queres in a function. When disabled, the "
-					 "queries may be committed immediately after the statemnent "
-					 "completes. Disabling this flag is dangerous, it is only provided "
-					 "for backwards compatibility with pre-8.2 behaviour."),
-		&FunctionOpensTransactionBlock,
-		true,
-		PGC_USERSET,
-		GUC_NO_SHOW_ALL,
-		NULL, NULL, NULL);
-
 	DefineCustomBoolVariable(
 		"citus.force_max_query_parallelization",
 		gettext_noop("Open as many connections as possible to maximize query "
@@ -1059,6 +1045,20 @@ RegisterCitusConfigVariables(void)
 		GUC_NO_SHOW_ALL,
 		NULL, NULL, NULL);
 
+	DefineCustomBoolVariable(
+		"citus.function_opens_transaction_block",
+		gettext_noop("Open transaction blocks for function calls"),
+		gettext_noop("When enabled, Citus will always send a BEGIN to workers when "
+					 "running distributed queres in a function. When disabled, the "
+					 "queries may be committed immediately after the statemnent "
+					 "completes. Disabling this flag is dangerous, it is only provided "
+					 "for backwards compatibility with pre-8.2 behaviour."),
+		&FunctionOpensTransactionBlock,
+		true,
+		PGC_USERSET,
+		GUC_NO_SHOW_ALL,
+		NULL, NULL, NULL);
+
 	DefineCustomIntVariable(
 		"citus.isolation_test_session_process_id",
 		NULL,