From 13500846e377b70be4c81b73f98477c8e215e8cb Mon Sep 17 00:00:00 2001 From: Ahmet Gedemenli Date: Wed, 4 Aug 2021 12:39:48 +0300 Subject: [PATCH] Add check for alphabetically sorted gucs --- .circleci/config.yml | 6 + ci/check_gucs_order.py | 16 +++ src/backend/distributed/shared_library_init.c | 124 +++++++++--------- 3 files changed, 84 insertions(+), 62 deletions(-) create mode 100644 ci/check_gucs_order.py diff --git a/.circleci/config.yml b/.circleci/config.yml index 55eaa51c4..a05c9852a 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -82,6 +82,12 @@ 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: | + set -euo pipefail + grep -o -E "(\.*\"citus.\w+\")," src/backend/distributed/shared_library_init.c > gucs.out + sort -c gucs.out check-sql-snapshots: docker: diff --git a/ci/check_gucs_order.py b/ci/check_gucs_order.py new file mode 100644 index 000000000..e15de71cf --- /dev/null +++ b/ci/check_gucs_order.py @@ -0,0 +1,16 @@ +import os + +fileDir = os.path.dirname(os.path.realpath('__file__')) +filename = os.path.join(fileDir, 'src/backend/distributed/shared_library_init.c') +shared_library_init = open(filename, 'r') +lines = shared_library_init.readlines() +getnextline = False +previous_guc = '' +for line in lines: + if getnextline: + if line < previous_guc: + exit(1) + previous_guc = line + getnextline = False + if 'DefineCustom' in line: + getnextline = True diff --git a/src/backend/distributed/shared_library_init.c b/src/backend/distributed/shared_library_init.c index dfcb05748..9d2f9113e 100644 --- a/src/backend/distributed/shared_library_init.c +++ b/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,