Quote EXTRACT fields and bump PostgreSQL minors

.github/workflows/build_and_test.yml

@@ -35,12 +35,12 @@ jobs:
       pgupgrade_image_name: "ghcr.io/citusdata/pgupgradetester"
       style_checker_image_name: "ghcr.io/citusdata/stylechecker"
       style_checker_tools_version: "0.8.33"
-      sql_snapshot_pg_version: "18.4"
-      image_suffix: "-v609733f"
-      pg17_version: '{ "major": "17", "full": "17.10" }'
-      pg18_version: '{ "major": "18", "full": "18.4" }'
+      sql_snapshot_pg_version: "18.6"
+      image_suffix: "-vdba9cbb"
+      pg17_version: '{ "major": "17", "full": "17.11" }'
+      pg18_version: '{ "major": "18", "full": "18.6" }'
       pg19_version: '{ "major": "19", "full": "19beta3" }'
-      upgrade_pg_versions: "17.10-18.4-19beta3"
+      upgrade_pg_versions: "17.11-18.6-19beta3"
     steps:
       # Since GHA jobs need at least one step we use a noop step here.
       - name: Set up parameters
@@ -148,7 +148,7 @@ jobs:
           ${{ needs.params.outputs.pg18_version }},
           ${{ needs.params.outputs.pg19_version }}
         ]
-      make_targets: '["check-split", "check-multi", "check-multi-1", "check-multi-1-create-citus", "check-multi-mx", "check-vanilla", "check-isolation", "check-operations", "check-follower-cluster", "check-add-backup-node", "check-columnar", "check-columnar-isolation", "check-enterprise", "check-enterprise-isolation", "check-enterprise-isolation-logicalrep-1", "check-enterprise-isolation-logicalrep-2", "check-enterprise-isolation-logicalrep-3", "check-tap"]'
+      make_targets: '["check-split", "check-split-output-plugin-denied", "check-multi", "check-multi-1", "check-multi-1-create-citus", "check-multi-mx", "check-vanilla", "check-isolation", "check-operations", "check-follower-cluster", "check-add-backup-node", "check-columnar", "check-columnar-isolation", "check-enterprise", "check-enterprise-isolation", "check-enterprise-isolation-logicalrep-1", "check-enterprise-isolation-logicalrep-2", "check-enterprise-isolation-logicalrep-3", "check-tap"]'
       image_suffix: ${{ needs.params.outputs.image_suffix }}
       image_name: ${{ needs.params.outputs.test_image_name }}
     secrets:
@@ -314,6 +314,7 @@ jobs:
       matrix:
         pg_version:
           - ${{ needs.params.outputs.pg17_version }}
+          - ${{ needs.params.outputs.pg18_version }}
     steps:
     - uses: actions/checkout@v5
     - uses: "./.github/actions/setup_extension"
@@ -418,15 +419,19 @@ jobs:
         detected_changes=$(git diff origin/main... --name-only --diff-filter=AM | (grep 'src/test/regress/sql/.*\.sql\|src/test/regress/spec/.*\.spec\|src/test/regress/citus_tests/test/test_.*\.py' || true))
         tests=${detected_changes}
 
-        # split the tests to be skipped --today we only skip upgrade tests
-        # and snapshot based node addition tests.
+        # split the tests to be skipped --today we only skip upgrade tests,
+        # snapshot based node addition tests and the output plugin allowlist
+        # test.
+        # split_output_plugin_denied only reports its expected error when the
+        # cluster starts without "citus" in output_plugin_libraries, which only
+        # the check-split-output-plugin-denied target arranges.
         # snapshot based node addition tests are not flaky, as they promote
         # the streaming replica (clone) to a PostgreSQL primary node that is one way
         # operation
         skipped_tests=""
         not_skipped_tests=""
         for test in $tests; do
-            if [[ $test =~ ^src/test/regress/sql/upgrade_ ]] || [[ $test =~ ^src/test/regress/sql/multi_add_node_from_backup ]]; then
+            if [[ $test =~ ^src/test/regress/sql/upgrade_ ]] || [[ $test =~ ^src/test/regress/sql/multi_add_node_from_backup ]] || [[ $test =~ ^src/test/regress/sql/split_output_plugin_denied ]]; then
                 skipped_tests="$skipped_tests $test"
             else
                 not_skipped_tests="$not_skipped_tests $test"

src/backend/distributed/deparser/ruleutils_17.c

@@ -7644,7 +7644,8 @@ get_func_sql_syntax(FuncExpr *expr, deparse_context *context)
 				Assert(IsA(con, Const) &&
 					   con->consttype == TEXTOID &&
 					   !con->constisnull);
-				appendStringInfoString(buf, TextDatumGetCString(con->constvalue));
+				appendStringInfoString(buf,
+									   quote_identifier(TextDatumGetCString(con->constvalue)));
 			}
 			appendStringInfoString(buf, " FROM ");
 			get_rule_expr((Node *) lsecond(expr->args), context, false);

src/backend/distributed/deparser/ruleutils_18.c

@@ -8078,7 +8078,8 @@ get_func_sql_syntax(FuncExpr *expr, deparse_context *context)
 				Assert(IsA(con, Const) &&
 					   con->consttype == TEXTOID &&
 					   !con->constisnull);
-				appendStringInfoString(buf, TextDatumGetCString(con->constvalue));
+				appendStringInfoString(buf,
+									   quote_identifier(TextDatumGetCString(con->constvalue)));
 			}
 			appendStringInfoString(buf, " FROM ");
 			get_rule_expr((Node *) lsecond(expr->args), context, false);

src/backend/distributed/operations/shard_split.c

@@ -69,6 +69,9 @@ typedef struct GroupedDummyShards
 	List *shardIntervals;
 } GroupedDummyShards;
 
+/* name of the logical decoding output plugin used by non-blocking splits */
+#define CITUS_SPLIT_DECODER_PLUGIN "citus"
+
 /* Function declarations */
 static void ErrorIfCannotSplitShard(SplitOperation splitOperation,
 									ShardInterval *sourceShard);
@@ -92,6 +95,8 @@ static void CreateAuxiliaryStructuresForShardGroup(List *shardGroupSplitInterval
 												   List *workersForPlacementList,
 												   bool includeReplicaIdentity);
 static void CreateReplicaIdentitiesForDummyShards(HTAB *mapOfPlacementToDummyShardList);
+static void ErrorIfOutputPluginNotAllowed(MultiConnection *connection,
+										  char *outputPlugin);
 static void CreateObjectOnPlacement(List *objectCreationCommandList,
 									WorkerNode *workerNode);
 static List *    CreateSplitIntervalsForShardGroup(List *sourceColocatedShardList,
@@ -1367,6 +1372,76 @@ AcquireNonblockingSplitLock(Oid relationId)
 }
 
 
+/*
+ * ErrorIfOutputPluginNotAllowed errors out if the given logical decoding output
+ * plugin is not listed in the "output_plugin_libraries" setting on the node
+ * behind the given connection, which is where the replication slot is created.
+ *
+ * "output_plugin_libraries" was introduced in PostgreSQL 14.24, 15.19, 16.15,
+ * 17.11 and 18.6. On older minor versions current_setting() returns NULL and we
+ * skip the check, because there is nothing to enforce.
+ */
+static void
+ErrorIfOutputPluginNotAllowed(MultiConnection *connection, char *outputPlugin)
+{
+	StringInfo command = makeStringInfo();
+	appendStringInfo(command,
+					 "SELECT current_setting('output_plugin_libraries', true), "
+					 "coalesce(bool_or(CASE WHEN name LIKE '\"%%\"' THEN "
+					 "replace(substring(name, 2, length(name) - 2), '\"\"', '\"') "
+					 "ELSE name END = %s), false) "
+					 "FROM (SELECT btrim(elem, E' \\t\\n\\r\\f') AS name FROM unnest("
+					 "string_to_array(coalesce(current_setting("
+					 "'output_plugin_libraries', true), %s), ',')) elem) s",
+					 quote_literal_cstr(outputPlugin),
+					 quote_literal_cstr(outputPlugin));
+
+	PGresult *result = NULL;
+	int queryResult = ExecuteOptionalRemoteCommand(connection, command->data, &result);
+
+	if (queryResult != RESPONSE_OKAY || !IsResponseOK(result) || PQntuples(result) != 1)
+	{
+		ReportResultError(connection, result, ERROR);
+	}
+
+	char *allowedPlugins = "";
+	if (!PQgetisnull(result, 0, 0))
+	{
+		allowedPlugins = pstrdup(PQgetvalue(result, 0, 0));
+	}
+
+	bool isAllowed = (strcmp(PQgetvalue(result, 0, 1), "t") == 0);
+
+	PQclear(result);
+	ForgetResults(connection);
+
+	if (isAllowed)
+	{
+		return;
+	}
+
+	StringInfo newValue = makeStringInfo();
+	if (allowedPlugins[0] != '\0')
+	{
+		appendStringInfo(newValue, "%s, ", allowedPlugins);
+	}
+	appendStringInfoString(newValue, outputPlugin);
+
+	ereport(ERROR, (errmsg("output plugin \"%s\" is not allowed on the source node",
+						   outputPlugin),
+					errdetail("Non-blocking shard splits replicate data with the \"%s\" "
+							  "logical decoding output plugin, but PostgreSQL on node "
+							  "%s:%d only allows the plugins listed in "
+							  "\"output_plugin_libraries\", which is set to \"%s\".",
+							  outputPlugin, connection->hostname, connection->port,
+							  allowedPlugins),
+					errhint("Allow the plugin on every node and reload the "
+							"configuration, for example: ALTER SYSTEM SET "
+							"output_plugin_libraries = %s; SELECT pg_reload_conf();",
+							newValue->data)));
+}
+
+
 /*
  * SplitShard API to split a given shard (or shard group) in non-blocking fashion
  * based on specified split points to a set of destination nodes.
@@ -1425,6 +1500,12 @@ NonBlockingShardSplit(SplitOperation splitOperation,
 		databaseName);
 	ClaimConnectionExclusively(sourceConnection);
 
+	/*
+	 * Fail before creating any shards, publications or replication slots if
+	 * the source node does not allow our output plugin.
+	 */
+	ErrorIfOutputPluginNotAllowed(sourceConnection, CITUS_SPLIT_DECODER_PLUGIN);
+
 	MultiConnection *sourceReplicationConnection =
 		GetReplicationConnection(sourceShardToCopyNode->workerName,
 								 sourceShardToCopyNode->workerPort);
@@ -1495,7 +1576,7 @@ NonBlockingShardSplit(SplitOperation splitOperation,
 		groupedLogicalRepTargetsHash,
 		superUser, databaseName);
 
-	char *logicalRepDecoderPlugin = "citus";
+	char *logicalRepDecoderPlugin = CITUS_SPLIT_DECODER_PLUGIN;
 
 	/*
 	 * 6) Create replication slots and keep track of their snapshot.

src/test/cdc/t/cdctestlib.pm

@@ -146,6 +146,16 @@ max_wal_senders = 100
 max_replication_slots = 100
 ";
     $node->init(allows_streaming => 'logical');
+
+    # PostgreSQL 14.24, 15.19, 16.15, 17.11 and 18.6 restrict logical decoding
+    # to the output plugins listed in output_plugin_libraries. Setting an
+    # unknown GUC is fatal, so only set it when initdb wrote it out.
+    if (open(my $conf_fh, '<', $node->data_dir . "/postgresql.conf")) {
+        $citus_config_options = $citus_config_options .
+            "\noutput_plugin_libraries = 'pgoutput, test_decoding, wal2json, citus'"
+            if grep { /output_plugin_libraries/ } <$conf_fh>;
+        close($conf_fh);
+    }
     if ($node_type == $NODE_TYPE_COORDINATOR || $node_type == $NODE_TYPE_WORKER) {
         $node->append_conf("postgresql.conf",$citus_config_options);
     } else {

src/test/regress/Makefile

@@ -294,6 +294,12 @@ check-split: all
 	$(pg_regress_multi_check) --load-extension=citus \
 	-- $(MULTI_REGRESS_OPTS) --schedule=$(citus_abs_srcdir)/split_schedule $(EXTRA_TESTS)
 
+# Runs the split negative test against a cluster that deliberately does not
+# allow the "citus" output plugin in "output_plugin_libraries".
+check-split-output-plugin-denied: all
+	CITUS_TEST_SKIP_OUTPUT_PLUGIN_ALLOWLIST=1 $(pg_regress_multi_check) --load-extension=citus \
+	-- $(MULTI_REGRESS_OPTS) --schedule=$(citus_abs_srcdir)/split_output_plugin_denied_schedule $(EXTRA_TESTS)
+
 check-failure: all
 	$(pg_regress_multi_check) --load-extension=citus --mitmproxy \
 	-- $(MULTI_REGRESS_OPTS) --schedule=$(citus_abs_srcdir)/failure_schedule $(EXTRA_TESTS)

src/test/regress/expected/extract_deparse.out

@@ -0,0 +1,40 @@
+--
+-- EXTRACT_DEPARSE
+--
+CREATE SCHEMA extract_deparse;
+SET search_path TO extract_deparse;
+CREATE TABLE extract_deparse_source (id int, ts timestamp);
+SELECT create_distributed_table('extract_deparse_source', 'id',
+								shard_count => 1, colocate_with => 'none');
+ create_distributed_table
+---------------------------------------------------------------------
+
+(1 row)
+
+INSERT INTO extract_deparse_source VALUES (1, timestamp '2026-01-01');
+-- EXTRACT accepts any string as its field name. Verify Citus quotes that field
+-- when deparsing task SQL, so text resembling a second statement remains data.
+-- PG19 has equivalent coverage in pg19.sql.
+DO $$
+BEGIN
+	IF current_setting('server_version_num')::int < 190000 THEN
+		PERFORM EXTRACT('year FROM timestamp ''2000-01-01''); CREATE TABLE injected(); --' FROM ts)
+		FROM extract_deparse_source
+		WHERE id = 1;
+	END IF;
+EXCEPTION
+	WHEN invalid_parameter_value THEN NULL;
+END
+$$;
+SELECT bool_and(result::boolean) AS extract_field_injection_blocked
+FROM run_command_on_workers($$
+	SELECT to_regclass('extract_deparse.injected') IS NULL
+$$);
+ extract_field_injection_blocked
+---------------------------------------------------------------------
+ t
+(1 row)
+
+SET client_min_messages TO ERROR;
+DROP SCHEMA extract_deparse CASCADE;
+RESET client_min_messages;

src/test/regress/expected/split_output_plugin_denied.out

@@ -0,0 +1,51 @@
+-- Negative coverage for the "output_plugin_libraries" allowlist that PostgreSQL
+-- 14.24, 15.19, 16.15, 17.11 and 18.6 introduced. This test is only run by the
+-- check-split-output-plugin-denied target, which deliberately starts the cluster
+-- without adding "citus" to the allowlist.
+CREATE SCHEMA split_output_plugin_denied;
+SET search_path TO split_output_plugin_denied;
+SET citus.shard_count TO 2;
+SET citus.shard_replication_factor TO 1;
+SET citus.next_shard_id TO 8990000;
+CREATE TABLE table_to_split (id bigint PRIMARY KEY, value char);
+SELECT create_distributed_table('table_to_split', 'id');
+ create_distributed_table
+---------------------------------------------------------------------
+
+(1 row)
+
+SELECT nodeid AS worker_1_node FROM pg_dist_node WHERE nodeport=:worker_1_port \gset
+SELECT nodeid AS worker_2_node FROM pg_dist_node WHERE nodeport=:worker_2_port \gset
+-- The split must be rejected up front. Terse verbosity keeps the node address
+-- and the current allowlist value out of the expected output.
+\set VERBOSITY terse
+SELECT citus_split_shard_by_split_points(
+	8990000,
+	ARRAY['-1073741826'],
+	ARRAY[:worker_1_node, :worker_2_node],
+	'force_logical');
+ERROR:  output plugin "citus" is not allowed on the source node
+\set VERBOSITY default
+-- Nothing must have been created before the split was rejected.
+SELECT count(*) FROM pg_dist_shard WHERE logicalrelid = 'table_to_split'::regclass;
+ count
+---------------------------------------------------------------------
+     2
+(1 row)
+
+SELECT run_command_on_workers($$SELECT count(*) FROM pg_replication_slots$$);
+ run_command_on_workers
+---------------------------------------------------------------------
+ (localhost,57637,t,0)
+ (localhost,57638,t,0)
+(2 rows)
+
+SELECT run_command_on_workers($$SELECT count(*) FROM pg_publication$$);
+ run_command_on_workers
+---------------------------------------------------------------------
+ (localhost,57637,t,0)
+ (localhost,57638,t,0)
+(2 rows)
+
+DROP SCHEMA split_output_plugin_denied CASCADE;
+NOTICE:  drop cascades to table table_to_split

src/test/regress/multi_1_create_citus_schedule

@@ -90,6 +90,7 @@ test: stat_counters
 test: function_propagation
 test: citus_internal_distribute_object
 test: drop_database
+test: extract_deparse
 test: pg16
 test: pg18
 test: pg19

src/test/regress/pg_regress_multi.pl

@@ -570,10 +570,6 @@ sub should_defer_libdir_swap
 }
 push(@pgOptions, "shared_preload_libraries='${sharedPreloadLibraries}'");
 
-if ($majorversion >= "19") {
-    push(@pgOptions, "output_plugin_libraries='pgoutput,citus'");
-}
-
 if ($vanillatest) {
     # use the default used in vanilla tests
     push(@pgOptions, "max_parallel_workers_per_gather=2");
@@ -588,6 +584,19 @@ sub should_defer_libdir_swap
 # Allow CREATE SUBSCRIPTION to work
 push(@pgOptions, "wal_level='logical'");
 
+# PostgreSQL 14.24, 15.19, 16.15, 17.11, 18.6 and 19 restrict logical decoding
+# to the plugins listed in output_plugin_libraries. Citus uses "citus" for
+# non-blocking shard splits and "wal2json" in some CDC tests. Setting an unknown
+# GUC is fatal, so only set it when the installed PostgreSQL knows about it.
+if (!$ENV{CITUS_TEST_SKIP_OUTPUT_PLUGIN_ALLOWLIST} &&
+	open(my $confSample, '<', catfile($sharedir, "postgresql.conf.sample")))
+{
+	push(@pgOptions,
+		 "output_plugin_libraries='pgoutput, test_decoding, wal2json, citus'")
+		if grep { /output_plugin_libraries/ } <$confSample>;
+	close($confSample);
+}
+
 # Faster logical replication status update so tests with logical replication
 # run faster
 push(@pgOptions, "wal_receiver_status_interval=1");

src/test/regress/split_output_plugin_denied_schedule

@@ -0,0 +1,8 @@
+# Verifies that a non-blocking shard split fails with an actionable error when
+# the "citus" logical decoding output plugin is not allowed by PostgreSQL.
+# Include tests from 'split_schedule' for setup.
+test: multi_test_helpers multi_test_helpers_superuser
+test: multi_cluster_management
+test: remove_coordinator_from_metadata
+test: multi_test_catalog_views
+test: split_output_plugin_denied