[CI only] Bump PG minors to 16.15 / 17.11 / 18.6

.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: "-v1441896"
-      pg16_version: '{ "major": "16", "full": "16.14" }'
-      pg17_version: '{ "major": "17", "full": "17.10" }'
-      pg18_version: '{ "major": "18", "full": "18.4" }'
-      upgrade_pg_versions: "16.14-17.10-18.4"
+      sql_snapshot_pg_version: "18.6"
+      image_suffix: "-ved0dad9"
+      pg16_version: '{ "major": "16", "full": "16.15" }'
+      pg17_version: '{ "major": "17", "full": "17.11" }'
+      pg18_version: '{ "major": "18", "full": "18.6" }'
+      upgrade_pg_versions: "16.15-17.11-18.6"
     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.pg17_version }},
           ${{ needs.params.outputs.pg18_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:

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,73 @@ 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(btrim(name, ' \"') = %s), false) "
+					 "FROM unnest(string_to_array(coalesce("
+					 "current_setting('output_plugin_libraries', true), %s), ',')) name",
+					 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 +1497,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 +1573,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

@@ -142,6 +142,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

@@ -284,6 +284,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/multi_follower_dml.out

@@ -227,26 +227,18 @@ COPY the_table (a, b, z) FROM STDIN WITH CSV;
 ERROR:  COPY command to Citus tables is not allowed in read-only mode
 DETAIL:  the database is read-only
 HINT:  All COPY commands to citus tables happen via 2PC, and 2PC requires the database to be in a writable state.
-\.
-invalid command \.
 COPY the_replicated_table (a, b, z) FROM STDIN WITH CSV;
 ERROR:  writing to worker nodes is not currently allowed for replicated tables such as reference tables or hash distributed tables with replication factor greater than 1.
 DETAIL:  the database is read-only
 HINT:  All modifications to replicated tables happen via 2PC, and 2PC requires the database to be in a writable state.
-\.
-invalid command \.
 COPY reference_table (a, b, z) FROM STDIN WITH CSV;
 ERROR:  writing to worker nodes is not currently allowed for replicated tables such as reference tables or hash distributed tables with replication factor greater than 1.
 DETAIL:  the database is read-only
 HINT:  All modifications to replicated tables happen via 2PC, and 2PC requires the database to be in a writable state.
-\.
-invalid command \.
 COPY citus_local_table (a, b, z) FROM STDIN WITH CSV;
 ERROR:  COPY command to Citus tables is not allowed in read-only mode
 DETAIL:  the database is read-only
 HINT:  All COPY commands to citus tables happen via 2PC, and 2PC requires the database to be in a writable state.
-\.
-invalid command \.
 -- all multi-shard modifications require 2PC hence not supported
 INSERT INTO the_table (a, b, z) VALUES (2, 3, 4), (5, 6, 7);
 ERROR:  cannot assign TransactionIds during recovery
@@ -299,20 +291,14 @@ COPY the_table (a, b, z) FROM STDIN WITH CSV;
 ERROR:  COPY command to Citus tables is not allowed in read-only mode
 DETAIL:  the database is read-only
 HINT:  All COPY commands to citus tables happen via 2PC, and 2PC requires the database to be in a writable state.
-\.
-invalid command \.
 COPY reference_table (a, b, z) FROM STDIN WITH CSV;
 ERROR:  writing to worker nodes is not currently allowed for replicated tables such as reference tables or hash distributed tables with replication factor greater than 1.
 DETAIL:  the database is read-only
 HINT:  All modifications to replicated tables happen via 2PC, and 2PC requires the database to be in a writable state.
-\.
-invalid command \.
 COPY citus_local_table (a, b, z) FROM STDIN WITH CSV;
 ERROR:  COPY command to Citus tables is not allowed in read-only mode
 DETAIL:  the database is read-only
 HINT:  All COPY commands to citus tables happen via 2PC, and 2PC requires the database to be in a writable state.
-\.
-invalid command \.
 SELECT * FROM the_table ORDER BY a;
  a | b | z
 ---------------------------------------------------------------------

src/test/regress/expected/multi_multiuser_copy.out

@@ -25,27 +25,16 @@ COPY customer_copy_hash (c_custkey,c_name) FROM STDIN;
 -- COPY FROM as user with ALL access
 SET ROLE full_access;
 COPY customer_copy_hash (c_custkey,c_name) FROM STDIN;
-;
 RESET ROLE;
 -- COPY FROM as user with SELECT access, should fail
 SET ROLE read_access;
 COPY customer_copy_hash (c_custkey,c_name) FROM STDIN;
 ERROR:  permission denied for table customer_copy_hash
-3	customer3
-\.
-invalid command \.
-;
-ERROR:  syntax error at or near "3"
 RESET ROLE;
 -- COPY FROM as user with no access, should fail
 SET ROLE no_access;
 COPY customer_copy_hash (c_custkey,c_name) FROM STDIN;
 ERROR:  permission denied for table customer_copy_hash
-4	customer4
-\.
-invalid command \.
-;
-ERROR:  syntax error at or near "4"
 RESET ROLE;
 -- COPY TO as superuser
 COPY (SELECT * FROM customer_copy_hash ORDER BY 1) TO STDOUT;

src/test/regress/expected/pg12.out

@@ -88,15 +88,12 @@ select create_distributed_table('cptest', 'id');
 
 copy cptest from STDIN with csv where val < 4;
 ERROR:  Citus does not support COPY FROM with WHERE
-1,6
-2,3
-3,2
-4,9
-5,4
-\.
-invalid command \.
 select sum(id), sum(val) from cptest;
-ERROR:  syntax error at or near "1"
+ sum | sum
+---------------------------------------------------------------------
+     |
+(1 row)
+
 -- CTE materialized/not materialized
 CREATE TABLE single_hash_repartition_first (id int, sum int, avg float);
 CREATE TABLE single_hash_repartition_second (id int primary key, sum int, avg float);

src/test/regress/expected/replicated_partitioned_table.out

@@ -126,8 +126,6 @@ HINT:  Run the query on the parent table "collections" instead.
 COPY collections_1 FROM STDIN;
 ERROR:  modifications on partitions when replication factor is greater than 1 is not supported
 HINT:  Run the query on the parent table "collections" instead.
-\.
-invalid command \.
 -- DDLs are not allowed
 CREATE INDEX index_on_partition ON collections_1(key);
 ERROR:  modifications on partitions when replication factor is greater than 1 is not supported

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/pg_regress_multi.pl

@@ -584,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 and 18.6 restrict logical decoding to
+# the output 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=0");

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

src/test/regress/sql/failure_copy_on_hash.sql

@@ -142,6 +142,7 @@ SELECT create_distributed_table('test_table_2','id');
 SELECT citus.mitmproxy('conn.kill()');
 
 \COPY test_table_2 FROM stdin delimiter ',';
+\.
 
 SELECT citus.mitmproxy('conn.allow()');
 SELECT pds.logicalrelid, pdsd.shardid, pdsd.shardstate

src/test/regress/sql/failure_copy_to_reference.sql

@@ -25,20 +25,23 @@ CREATE VIEW unhealthy_shard_count AS
 -- response we get from the worker
 SELECT citus.mitmproxy('conn.kill()');
 \copy test_table FROM STDIN DELIMITER ','
+\.
 SELECT citus.mitmproxy('conn.allow()');
 SELECT * FROM unhealthy_shard_count;
 SELECT count(*) FROM test_table;
 
 -- kill as soon as the coordinator sends begin
 SELECT citus.mitmproxy('conn.onQuery(query="^BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED").kill()');
 \copy test_table FROM STDIN DELIMITER ','
+\.
 SELECT citus.mitmproxy('conn.allow()');
 SELECT * FROM unhealthy_shard_count;
 SELECT count(*) FROM test_table;
 
 -- cancel as soon as the coordinator sends begin
 SELECT citus.mitmproxy('conn.onQuery(query="^BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED").cancel(' ||  pg_backend_pid() || ')');
 \copy test_table FROM STDIN DELIMITER ','
+\.
 SELECT citus.mitmproxy('conn.allow()');
 SELECT * FROM unhealthy_shard_count;
 SELECT count(*) FROM test_table;