From 333c73a53c3b9ecf24be88d7825e0983f6729b0e Mon Sep 17 00:00:00 2001 From: Burak Velioglu Date: Tue, 15 Mar 2022 13:50:05 +0300 Subject: [PATCH] Drop distributed table on worker with ProcessUtilityParseTree --- .../distributed/worker/worker_drop_protocol.c | 50 +++++++++++++------ .../regress/expected/aggregate_support.out | 11 ++++ src/test/regress/expected/check_mx.out | 10 ++++ src/test/regress/sql/aggregate_support.sql | 8 +++ src/test/regress/sql/check_mx.sql | 7 +++ 5 files changed, 72 insertions(+), 14 deletions(-) diff --git a/src/backend/distributed/worker/worker_drop_protocol.c b/src/backend/distributed/worker/worker_drop_protocol.c index 0f425583b..452de62e3 100644 --- a/src/backend/distributed/worker/worker_drop_protocol.c +++ b/src/backend/distributed/worker/worker_drop_protocol.c @@ -27,12 +27,16 @@ #include "distributed/listutils.h" #include "distributed/metadata_utility.h" #include "distributed/coordinator_protocol.h" +#include "distributed/commands/utility_hook.h" #include "distributed/metadata_cache.h" #include "distributed/metadata/distobject.h" #include "distributed/multi_partitioning_utils.h" +#include "distributed/worker_protocol.h" #include "foreign/foreign.h" +#include "tcop/utility.h" #include "utils/builtins.h" #include "utils/fmgroids.h" +#include "utils/lsyscache.h" PG_FUNCTION_INFO_V1(worker_drop_distributed_table); PG_FUNCTION_INFO_V1(worker_drop_shell_table); @@ -142,20 +146,11 @@ WorkerDropDistributedTable(Oid relationId) UnmarkObjectDistributed(&distributedTableObject); - if (!IsObjectAddressOwnedByExtension(&distributedTableObject, NULL)) - { - /* - * If the table is owned by an extension, we cannot drop it, nor should we - * until the user runs DROP EXTENSION. Therefore, we skip dropping the - * table and only delete the metadata. - * - * We drop the table with cascade since other tables may be referring to it. - */ - performDeletion(&distributedTableObject, DROP_CASCADE, - PERFORM_DELETION_INTERNAL); - } - - /* iterate over shardList to delete the corresponding rows */ + /* + * Remove metadata before object's itself to make functions no-op within + * drop event trigger for undistributed objects on worker nodes except + * removing pg_dist_object entries. + */ List *shardList = LoadShardList(relationId); uint64 *shardIdPointer = NULL; foreach_ptr(shardIdPointer, shardList) @@ -176,6 +171,33 @@ WorkerDropDistributedTable(Oid relationId) /* delete the row from pg_dist_partition */ DeletePartitionRow(relationId); + + /* + * If the table is owned by an extension, we cannot drop it, nor should we + * until the user runs DROP EXTENSION. Therefore, we skip dropping the + * table. + */ + if (!IsObjectAddressOwnedByExtension(&distributedTableObject, NULL)) + { + char *relName = get_rel_name(relationId); + Oid schemaId = get_rel_namespace(relationId); + char *schemaName = get_namespace_name(schemaId); + + StringInfo dropCommand = makeStringInfo(); + appendStringInfo(dropCommand, "DROP%sTABLE %s CASCADE", + IsForeignTable(relationId) ? " FOREIGN " : " ", + quote_qualified_identifier(schemaName, relName)); + + Node *dropCommandNode = ParseTreeNode(dropCommand->data); + + /* + * We use ProcessUtilityParseTree (instead of performDeletion) to make sure that + * we also drop objects that depend on the table and call the drop event trigger + * which removes them from pg_dist_object. + */ + ProcessUtilityParseTree(dropCommandNode, dropCommand->data, + PROCESS_UTILITY_QUERY, NULL, None_Receiver, NULL); + } } diff --git a/src/test/regress/expected/aggregate_support.out b/src/test/regress/expected/aggregate_support.out index 88fb7b539..e97634ea8 100644 --- a/src/test/regress/expected/aggregate_support.out +++ b/src/test/regress/expected/aggregate_support.out @@ -1189,6 +1189,17 @@ DROP TABLE dummy_tbl CASCADE; NOTICE: drop cascades to 2 other objects DETAIL: drop cascades to function dummy_fnc(dummy_tbl,double precision) drop cascades to function dependent_agg(double precision) +-- Show that after dropping a table on which functions and aggregates depending on +-- pg_dist_object is consistent on coordinator and worker node. +SELECT pg_identify_object_as_address(classid, objid, objsubid)::text +FROM pg_catalog.pg_dist_object + EXCEPT +SELECT unnest(result::text[]) AS unnested_result +FROM run_command_on_workers($$SELECT array_agg(pg_identify_object_as_address(classid, objid, objsubid)) from pg_catalog.pg_dist_object$$); + pg_identify_object_as_address +--------------------------------------------------------------------- +(0 rows) + SET citus.create_object_propagation TO automatic; begin; create type typ1 as (a int); diff --git a/src/test/regress/expected/check_mx.out b/src/test/regress/expected/check_mx.out index 6a030bc31..b19445229 100644 --- a/src/test/regress/expected/check_mx.out +++ b/src/test/regress/expected/check_mx.out @@ -10,3 +10,13 @@ SELECT bool_and(metadatasynced) FROM pg_dist_node WHERE noderole = 'primary'; t (1 row) +-- Show that pg_dist_object entities are same on all nodes +SELECT pg_identify_object_as_address(classid, objid, objsubid)::text +FROM pg_catalog.pg_dist_object + EXCEPT +SELECT unnest(result::text[]) AS unnested_result +FROM run_command_on_workers($$SELECT array_agg(pg_identify_object_as_address(classid, objid, objsubid)) from pg_catalog.pg_dist_object$$); + pg_identify_object_as_address +--------------------------------------------------------------------- +(0 rows) + diff --git a/src/test/regress/sql/aggregate_support.sql b/src/test/regress/sql/aggregate_support.sql index 5f235c4cf..ba3ef90d8 100644 --- a/src/test/regress/sql/aggregate_support.sql +++ b/src/test/regress/sql/aggregate_support.sql @@ -603,6 +603,14 @@ SELECT run_command_on_workers($$select aggfnoid from pg_aggregate where aggfnoid DROP TABLE dummy_tbl CASCADE; +-- Show that after dropping a table on which functions and aggregates depending on +-- pg_dist_object is consistent on coordinator and worker node. +SELECT pg_identify_object_as_address(classid, objid, objsubid)::text +FROM pg_catalog.pg_dist_object + EXCEPT +SELECT unnest(result::text[]) AS unnested_result +FROM run_command_on_workers($$SELECT array_agg(pg_identify_object_as_address(classid, objid, objsubid)) from pg_catalog.pg_dist_object$$); + SET citus.create_object_propagation TO automatic; begin; create type typ1 as (a int); diff --git a/src/test/regress/sql/check_mx.sql b/src/test/regress/sql/check_mx.sql index 6c9b2b664..628e86445 100644 --- a/src/test/regress/sql/check_mx.sql +++ b/src/test/regress/sql/check_mx.sql @@ -1,3 +1,10 @@ SHOW citus.enable_metadata_sync; SELECT bool_and(metadatasynced) FROM pg_dist_node WHERE noderole = 'primary'; + +-- Show that pg_dist_object entities are same on all nodes +SELECT pg_identify_object_as_address(classid, objid, objsubid)::text +FROM pg_catalog.pg_dist_object + EXCEPT +SELECT unnest(result::text[]) AS unnested_result +FROM run_command_on_workers($$SELECT array_agg(pg_identify_object_as_address(classid, objid, objsubid)) from pg_catalog.pg_dist_object$$);