Remove master_drop_sequences

src/backend/distributed/operations/delete_protocol.c

@@ -252,86 +252,15 @@ master_drop_all_shards(PG_FUNCTION_ARGS)
 
 
 /*
- * master_drop_sequences attempts to drop a list of sequences on worker nodes.
+ * master_drop_sequences was previously used to drop sequences on workers
- * The "IF EXISTS" clause is used to permit dropping sequences even if they may not
+ * when using metadata syncing.
- * exist. If the commands fail on the workers, the operation is rolled back.
+ *
- * If ddl propagation (citus.enable_ddl_propagation) is set to off, then the function
+ * It may still be called when dropping objects during CREATE EXTENSION,
- * returns without doing anything.
+ * hence the function remains in place.
  */
 Datum
 master_drop_sequences(PG_FUNCTION_ARGS)
 {
-	ArrayType *sequenceNamesArray = PG_GETARG_ARRAYTYPE_P(0);
-	Datum sequenceNameDatum = 0;
-	bool isNull = false;
-	StringInfo dropSeqCommand = makeStringInfo();
-
-	if (!CitusHasBeenLoaded())
-	{
-		/* ignore calls during CREATE EXTENSION citus */
-		PG_RETURN_VOID();
-	}
-
-	CheckCitusVersion(ERROR);
-
-	/*
-	 * Do nothing if DDL propagation is switched off or we're not on
-	 * the coordinator. Here we prefer to not error out on the workers
-	 * because this function is called on every dropped sequence and
-	 * we don't want to mess up the sequences that are not associated
-	 * with distributed tables.
-	 */
-	if (!EnableDDLPropagation || !IsCoordinator())
-	{
-		PG_RETURN_VOID();
-	}
-
-	/* iterate over sequence names to build single command to DROP them all */
-	ArrayIterator sequenceIterator = array_create_iterator(sequenceNamesArray, 0, NULL);
-	while (array_iterate(sequenceIterator, &sequenceNameDatum, &isNull))
-	{
-		if (isNull)
-		{
-			ereport(ERROR, (errmsg("unexpected NULL sequence name"),
-							errcode(ERRCODE_INVALID_PARAMETER_VALUE)));
-		}
-
-		text *sequenceNameText = DatumGetTextP(sequenceNameDatum);
-		Oid sequenceOid = ResolveRelationId(sequenceNameText, true);
-		if (OidIsValid(sequenceOid))
-		{
-			/*
-			 * This case (e.g., OID is valid) could only happen when a user manually calls
-			 * the UDF. So, ensure that the user has right to drop the sequence.
-			 *
-			 * In case the UDF is called via the DROP trigger, the OID wouldn't be valid since
-			 * the trigger is called after DROP happens.
-			 */
-			EnsureSequenceOwner(sequenceOid);
-		}
-
-		/* append command portion if we haven't added any sequence names yet */
-		if (dropSeqCommand->len == 0)
-		{
-			appendStringInfoString(dropSeqCommand, "DROP SEQUENCE IF EXISTS");
-		}
-		else
-		{
-			/* otherwise, add a comma to separate subsequent sequence names */
-			appendStringInfoChar(dropSeqCommand, ',');
-		}
-
-		appendStringInfo(dropSeqCommand, " %s", TextDatumGetCString(sequenceNameText));
-	}
-
-	if (dropSeqCommand->len != 0)
-	{
-		appendStringInfoString(dropSeqCommand, " CASCADE");
-
-		SendCommandToWorkersWithMetadata(DISABLE_DDL_PROPAGATION);
-		SendCommandToWorkersWithMetadata(dropSeqCommand->data);
-	}
-
 	PG_RETURN_VOID();
 }
 

src/backend/distributed/sql/citus--9.4-1--9.5-1.sql

@@ -3,6 +3,7 @@
 -- bump version to 9.5-1
 #include "udfs/undistribute_table/9.5-1.sql"
 #include "udfs/create_citus_local_table/9.5-1.sql"
+#include "udfs/citus_drop_trigger/9.5-1.sql"
 
 SET search_path = 'pg_catalog';
 
@@ -14,5 +15,6 @@ DROP FUNCTION worker_execute_sql_task(bigint, integer, text, bool);
 DROP TRIGGER dist_authinfo_task_tracker_cache_invalidate ON pg_catalog.pg_dist_authinfo;
 DROP TRIGGER dist_poolinfo_task_tracker_cache_invalidate ON pg_catalog.pg_dist_poolinfo;
 DROP FUNCTION task_tracker_conninfo_cache_invalidate();
+DROP FUNCTION master_drop_sequences(text[]);
 
 RESET search_path;

src/backend/distributed/sql/downgrades/citus--9.5-1--9.4-1.sql

@@ -2,6 +2,8 @@
 
 SET search_path = 'pg_catalog';
 
+#include "../udfs/citus_drop_trigger/9.0-1.sql"
+
 -- Check if user has any citus local tables.
 -- If not, DROP create_citus_local_table UDF and continue safely.
 -- Otherwise, raise an exception to stop the downgrade process.
@@ -78,6 +80,13 @@ CREATE TRIGGER dist_authinfo_task_tracker_cache_invalidate
     ON pg_catalog.pg_dist_authinfo
     FOR EACH STATEMENT EXECUTE PROCEDURE task_tracker_conninfo_cache_invalidate();
 
+CREATE FUNCTION master_drop_sequences(sequence_names text[])
+    RETURNS void
+    LANGUAGE C STRICT
+    AS 'MODULE_PATHNAME', $$master_drop_sequences$$;
+COMMENT ON FUNCTION master_drop_sequences(text[])
+    IS 'drop specified sequences from the cluster';
+
 RESET search_path;
 
 DROP FUNCTION pg_catalog.undistribute_table(table_name regclass);

src/backend/distributed/sql/udfs/citus_drop_trigger/9.5-1.sql

@@ -0,0 +1,33 @@
+CREATE OR REPLACE FUNCTION pg_catalog.citus_drop_trigger()
+    RETURNS event_trigger
+    LANGUAGE plpgsql
+    SET search_path = pg_catalog
+    AS $cdbdt$
+DECLARE
+    v_obj record;
+    sequence_names text[] := '{}';
+    table_colocation_id integer;
+    propagate_drop boolean := false;
+BEGIN
+    FOR v_obj IN SELECT * FROM pg_event_trigger_dropped_objects()
+                 WHERE object_type IN ('table', 'foreign table')
+    LOOP
+        -- first drop the table and metadata on the workers
+        -- then drop all the shards on the workers
+        -- finally remove the pg_dist_partition entry on the coordinator
+        PERFORM master_remove_distributed_table_metadata_from_workers(v_obj.objid, v_obj.schema_name, v_obj.object_name);
+        PERFORM master_drop_all_shards(v_obj.objid, v_obj.schema_name, v_obj.object_name);
+        PERFORM master_remove_partition_metadata(v_obj.objid, v_obj.schema_name, v_obj.object_name);
+    END LOOP;
+
+    -- remove entries from citus.pg_dist_object for all dropped root (objsubid = 0) objects
+    FOR v_obj IN SELECT * FROM pg_event_trigger_dropped_objects()
+    LOOP
+        PERFORM master_unmark_object_distributed(v_obj.classid, v_obj.objid, v_obj.objsubid);
+    END LOOP;
+END;
+$cdbdt$;
+COMMENT ON FUNCTION pg_catalog.citus_drop_trigger()
+    IS 'perform checks and actions at the end of DROP actions';
+
+

src/backend/distributed/sql/udfs/citus_drop_trigger/latest.sql

@@ -9,11 +9,6 @@ DECLARE
     table_colocation_id integer;
     propagate_drop boolean := false;
 BEGIN
-    -- collect set of dropped sequences to drop on workers later
-    SELECT array_agg(object_identity) INTO sequence_names
-    FROM pg_event_trigger_dropped_objects()
-    WHERE object_type = 'sequence';
-
     FOR v_obj IN SELECT * FROM pg_event_trigger_dropped_objects()
                  WHERE object_type IN ('table', 'foreign table')
     LOOP
@@ -25,10 +20,6 @@ BEGIN
         PERFORM master_remove_partition_metadata(v_obj.objid, v_obj.schema_name, v_obj.object_name);
     END LOOP;
 
-    IF cardinality(sequence_names) > 0 THEN
-        PERFORM master_drop_sequences(sequence_names);
-    END IF;
-
     -- remove entries from citus.pg_dist_object for all dropped root (objsubid = 0) objects
     FOR v_obj IN SELECT * FROM pg_event_trigger_dropped_objects()
     LOOP

src/test/regress/expected/multi_extension.out

@@ -441,6 +441,7 @@ ALTER EXTENSION citus UPDATE TO '9.5-1';
 SELECT * FROM print_extension_changes();
                            previous_object                           |               current_object
 ---------------------------------------------------------------------
+ function master_drop_sequences(text[])                              |
  function task_tracker_assign_task(bigint,integer,text)              |
  function task_tracker_cleanup_job(bigint)                           |
  function task_tracker_conninfo_cache_invalidate()                   |
@@ -449,7 +450,7 @@ SELECT * FROM print_extension_changes();
  function worker_merge_files_and_run_query(bigint,integer,text,text) |
                                                                      | function create_citus_local_table(regclass)
                                                                      | function undistribute_table(regclass)
-(8 rows)
+(9 rows)
 
 DROP TABLE prev_objects, extension_diff;
 -- show running version

src/test/regress/expected/multi_mx_metadata.out

@@ -334,42 +334,9 @@ SELECT raise_failed_aclcheck($$
 $$);
 ERROR:  must be owner of the object
 CONTEXT:  PL/pgSQL function raise_failed_aclcheck(text) line 6 at RAISE
-SELECT raise_failed_aclcheck($$
-    SELECT master_drop_sequences(ARRAY['public.distributed_mx_table_some_val_seq']);
-$$);
-ERROR:  must be owner of the object
-CONTEXT:  PL/pgSQL function raise_failed_aclcheck(text) line 6 at RAISE
-SELECT raise_failed_aclcheck($$
-    SELECT master_drop_sequences(ARRAY['distributed_mx_table_some_val_seq']);
-$$);
-ERROR:  must be owner of the object
-CONTEXT:  PL/pgSQL function raise_failed_aclcheck(text) line 6 at RAISE
-SELECT master_drop_sequences(ARRAY['non_existing_schema.distributed_mx_table_some_val_seq']);
- master_drop_sequences
----------------------------------------------------------------------
-
-(1 row)
-
-SELECT master_drop_sequences(ARRAY['']);
-ERROR:  invalid name syntax
-SELECT master_drop_sequences(ARRAY['public.']);
-ERROR:  invalid name syntax
-SELECT master_drop_sequences(ARRAY['public.distributed_mx_table_some_val_seq_not_existing']);
- master_drop_sequences
----------------------------------------------------------------------
-
-(1 row)
-
 -- make sure that we can drop unrelated tables/sequences
 CREATE TABLE unrelated_table(key serial);
 DROP TABLE unrelated_table;
--- doesn't error out but it has no effect, so no need to error out
-SELECT master_drop_sequences(NULL);
- master_drop_sequences
----------------------------------------------------------------------
-
-(1 row)
-
 \c - postgres - :master_port
 -- finally make sure that the sequence remains
 SELECT "Column", "Type", "Modifiers" FROM table_desc WHERE relid='distributed_mx_table'::regclass;

src/test/regress/sql/multi_mx_metadata.sql

@@ -210,25 +210,11 @@ $$);
 SELECT raise_failed_aclcheck($$
     SELECT master_remove_partition_metadata('distributed_mx_table'::regclass, 'public', 'distributed_mx_table');
 $$);
-SELECT raise_failed_aclcheck($$
-    SELECT master_drop_sequences(ARRAY['public.distributed_mx_table_some_val_seq']);
-$$);
-SELECT raise_failed_aclcheck($$
-    SELECT master_drop_sequences(ARRAY['distributed_mx_table_some_val_seq']);
-$$);
-
-SELECT master_drop_sequences(ARRAY['non_existing_schema.distributed_mx_table_some_val_seq']);
-SELECT master_drop_sequences(ARRAY['']);
-SELECT master_drop_sequences(ARRAY['public.']);
-SELECT master_drop_sequences(ARRAY['public.distributed_mx_table_some_val_seq_not_existing']);
 
 -- make sure that we can drop unrelated tables/sequences
 CREATE TABLE unrelated_table(key serial);
 DROP TABLE unrelated_table;
 
--- doesn't error out but it has no effect, so no need to error out
-SELECT master_drop_sequences(NULL);
-
 \c - postgres - :master_port
 
 -- finally make sure that the sequence remains