From 5a3e8a6e2479c429ab6075d888bdc55aefb116ef Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Sun, 17 Jan 2021 20:32:30 +0300 Subject: [PATCH] Skip postgres tables for UndistributeTable(cascadeViaFKeys) (#4530) The reason behind skipping postgres tables is that we support foreign keys between postgres tables and reference tables (without converting postgres tables to citus local tables) when enable_local_reference_table_foreign_keys is false or when coordinator is not added to metadata. --- ..._table_operation_for_connected_relations.c | 31 +++++++++--------- .../expected/undistribute_table_cascade.out | 32 +++++++++++++++++++ .../sql/undistribute_table_cascade.sql | 16 ++++++++++ 3 files changed, 64 insertions(+), 15 deletions(-) diff --git a/src/backend/distributed/commands/cascade_table_operation_for_connected_relations.c b/src/backend/distributed/commands/cascade_table_operation_for_connected_relations.c index 117eddfb3..1afc4598d 100644 --- a/src/backend/distributed/commands/cascade_table_operation_for_connected_relations.c +++ b/src/backend/distributed/commands/cascade_table_operation_for_connected_relations.c @@ -353,19 +353,28 @@ ExecuteCascadeOperationForRelationIdList(List *relationIdList, foreach_oid(relationId, relationIdList) { /* - * Caller already passed the relations that we should operate on, - * so we should not cascade here. + * The reason behind skipping certain table types in below loop is + * that we support some sort of foreign keys between postgres tables + * and citus tables when enable_local_reference_table_foreign_keys is + * false or when coordinator is not added to metadata. + * + * Also, as caller already passed the relations that we should operate + * on, we don't cascade via foreign keys here. */ bool cascadeViaForeignKeys = false; switch (cascadeOperationType) { case CASCADE_FKEY_UNDISTRIBUTE_TABLE: { - TableConversionParameters params = { - .relationId = relationId, - .cascadeViaForeignKeys = cascadeViaForeignKeys - }; - UndistributeTable(¶ms); + if (IsCitusTable(relationId)) + { + TableConversionParameters params = { + .relationId = relationId, + .cascadeViaForeignKeys = cascadeViaForeignKeys + }; + UndistributeTable(¶ms); + } + break; } @@ -373,14 +382,6 @@ ExecuteCascadeOperationForRelationIdList(List *relationIdList, { if (!IsCitusTable(relationId)) { - /* - * Normally, we wouldn't expect a postgres table connected - * to a citus local table via a foreign keys graph. But now - * this is possible as we allow foreign keys from postgres - * tables to reference tables when coordinator is not added - * to metadata. So instead of erroring out, we skip citus - * tables here. - */ CreateCitusLocalTable(relationId, cascadeViaForeignKeys); } diff --git a/src/test/regress/expected/undistribute_table_cascade.out b/src/test/regress/expected/undistribute_table_cascade.out index 6db239227..75c736324 100644 --- a/src/test/regress/expected/undistribute_table_cascade.out +++ b/src/test/regress/expected/undistribute_table_cascade.out @@ -4,6 +4,38 @@ SET citus.shard_replication_factor TO 1; CREATE SCHEMA undistribute_table_cascade; SET search_path TO undistribute_table_cascade; SET client_min_messages to ERROR; +-- remove coordinator if it is added to pg_dist_node +SELECT COUNT(master_remove_node(nodename, nodeport)) < 2 +FROM pg_dist_node WHERE nodename='localhost' AND nodeport=:master_port; + ?column? +--------------------------------------------------------------------- + t +(1 row) + +BEGIN; + CREATE TABLE reference_table(col_1 INT UNIQUE); + CREATE TABLE distributed_table(col_1 INT UNIQUE); + CREATE TABLE local_table (col_1 INT REFERENCES reference_table(col_1), FOREIGN KEY (col_1) REFERENCES distributed_table(col_1)); + SELECT create_reference_table('reference_table'); + create_reference_table +--------------------------------------------------------------------- + +(1 row) + + SELECT create_distributed_table('distributed_table', 'col_1'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + + -- show that we skip postgres tables when undistributing citus tables + SELECT undistribute_table('reference_table', cascade_via_foreign_keys=>true); + undistribute_table +--------------------------------------------------------------------- + +(1 row) + +ROLLBACK; -- ensure that coordinator is added to pg_dist_node SELECT 1 FROM master_add_node('localhost', :master_port, groupId => 0); ?column? diff --git a/src/test/regress/sql/undistribute_table_cascade.sql b/src/test/regress/sql/undistribute_table_cascade.sql index e1ac2fccf..b0c446838 100644 --- a/src/test/regress/sql/undistribute_table_cascade.sql +++ b/src/test/regress/sql/undistribute_table_cascade.sql @@ -8,6 +8,22 @@ SET search_path TO undistribute_table_cascade; SET client_min_messages to ERROR; +-- remove coordinator if it is added to pg_dist_node +SELECT COUNT(master_remove_node(nodename, nodeport)) < 2 +FROM pg_dist_node WHERE nodename='localhost' AND nodeport=:master_port; + +BEGIN; + CREATE TABLE reference_table(col_1 INT UNIQUE); + CREATE TABLE distributed_table(col_1 INT UNIQUE); + CREATE TABLE local_table (col_1 INT REFERENCES reference_table(col_1), FOREIGN KEY (col_1) REFERENCES distributed_table(col_1)); + + SELECT create_reference_table('reference_table'); + SELECT create_distributed_table('distributed_table', 'col_1'); + + -- show that we skip postgres tables when undistributing citus tables + SELECT undistribute_table('reference_table', cascade_via_foreign_keys=>true); +ROLLBACK; + -- ensure that coordinator is added to pg_dist_node SELECT 1 FROM master_add_node('localhost', :master_port, groupId => 0);