From e3e174f30fbc7cd01f6d7dcf17c67de8791c78a4 Mon Sep 17 00:00:00 2001 From: Hadi Moshayedi Date: Tue, 10 Dec 2019 14:25:47 -0800 Subject: [PATCH] Fix the way we check for local/reference table joins in the executor --- .../distributed/executor/multi_executor.c | 20 +++++++++++++++---- ...licate_reference_tables_to_coordinator.out | 3 +++ ...licate_reference_tables_to_coordinator.sql | 4 ++++ 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/backend/distributed/executor/multi_executor.c b/src/backend/distributed/executor/multi_executor.c index 466c1c023..691abf1ff 100644 --- a/src/backend/distributed/executor/multi_executor.c +++ b/src/backend/distributed/executor/multi_executor.c @@ -580,7 +580,7 @@ IsLocalReferenceTableJoinPlan(PlannedStmt *plan) { bool hasReferenceTable = false; bool hasLocalTable = false; - ListCell *oidCell = NULL; + ListCell *rangeTableCell = NULL; bool hasReferenceTableReplica = false; /* @@ -617,12 +617,24 @@ IsLocalReferenceTableJoinPlan(PlannedStmt *plan) return false; } - foreach(oidCell, plan->relationOids) + /* + * plan->rtable contains the flattened RTE lists of the plan tree, which + * includes rtes in subqueries, CTEs, ... + * + * It doesn't contain optimized away table accesses (due to join optimization), + * which is fine for our purpose. + */ + foreach(rangeTableCell, plan->rtable) { - Oid relationId = lfirst_oid(oidCell); + RangeTblEntry *rangeTableEntry = (RangeTblEntry *) lfirst(rangeTableCell); bool onlySearchPath = false; - if (RelationIsAKnownShard(relationId, onlySearchPath)) + if (rangeTableEntry->rtekind != RTE_RELATION) + { + continue; + } + + if (RelationIsAKnownShard(rangeTableEntry->relid, onlySearchPath)) { /* * We don't allow joining non-reference distributed tables, so we diff --git a/src/test/regress/expected/replicate_reference_tables_to_coordinator.out b/src/test/regress/expected/replicate_reference_tables_to_coordinator.out index f825e5a50..9ee24b477 100644 --- a/src/test/regress/expected/replicate_reference_tables_to_coordinator.out +++ b/src/test/regress/expected/replicate_reference_tables_to_coordinator.out @@ -244,6 +244,9 @@ HINT: Consider using an equality filter on the distributed table's partition co SELECT local_table.a, numbers.a FROM local_table NATURAL JOIN numbers FOR UPDATE; ERROR: could not run distributed query with FOR UPDATE/SHARE commands HINT: Consider using an equality filter on the distributed table's partition column. +-- verify that we can drop columns from reference tables replicated to the coordinator +-- see https://github.com/citusdata/citus/issues/3279 +ALTER TABLE squares DROP COLUMN b; -- clean-up SET client_min_messages TO ERROR; DROP SCHEMA replicate_ref_to_coordinator CASCADE; diff --git a/src/test/regress/sql/replicate_reference_tables_to_coordinator.sql b/src/test/regress/sql/replicate_reference_tables_to_coordinator.sql index b2e3667e0..0f52cec00 100644 --- a/src/test/regress/sql/replicate_reference_tables_to_coordinator.sql +++ b/src/test/regress/sql/replicate_reference_tables_to_coordinator.sql @@ -138,6 +138,10 @@ SELECT a FROM t NATURAL JOIN dist; SELECT local_table.a, numbers.a FROM local_table NATURAL JOIN numbers FOR SHARE; SELECT local_table.a, numbers.a FROM local_table NATURAL JOIN numbers FOR UPDATE; +-- verify that we can drop columns from reference tables replicated to the coordinator +-- see https://github.com/citusdata/citus/issues/3279 +ALTER TABLE squares DROP COLUMN b; + -- clean-up SET client_min_messages TO ERROR; DROP SCHEMA replicate_ref_to_coordinator CASCADE;