From c2a15da44e66a546b05c2bebeb3919fd24c63257 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 c3c21e72c..9b1d9118b 100644 --- a/src/backend/distributed/executor/multi_executor.c +++ b/src/backend/distributed/executor/multi_executor.c @@ -585,7 +585,7 @@ IsLocalReferenceTableJoinPlan(PlannedStmt *plan) { bool hasReferenceTable = false; bool hasLocalTable = false; - ListCell *oidCell = NULL; + ListCell *rangeTableCell = NULL; bool hasReferenceTableReplica = false; /* @@ -622,12 +622,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 dca81da47..150d54d71 100644 --- a/src/test/regress/expected/replicate_reference_tables_to_coordinator.out +++ b/src/test/regress/expected/replicate_reference_tables_to_coordinator.out @@ -166,6 +166,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 5a2af41cf..37ec3675d 100644 --- a/src/test/regress/sql/replicate_reference_tables_to_coordinator.sql +++ b/src/test/regress/sql/replicate_reference_tables_to_coordinator.sql @@ -83,6 +83,10 @@ WITH t AS (SELECT *, random() x FROM dist) SELECT * FROM numbers, local_table 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;