From adfed2fb310701f7887fa0fb010a36ce7bbde6d4 Mon Sep 17 00:00:00 2001 From: Jianhong Shi Date: Tue, 4 Aug 2026 21:17:36 -0500 Subject: [PATCH 1/3] add detect-and-bail guard for cyclic join queries --- src/optimizer/robust_optimizer.cpp | 40 ++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/optimizer/robust_optimizer.cpp b/src/optimizer/robust_optimizer.cpp index 547754b..5bb8c69 100644 --- a/src/optimizer/robust_optimizer.cpp +++ b/src/optimizer/robust_optimizer.cpp @@ -796,6 +796,42 @@ vector RobustOptimizerContextState::BuildPhysicalPlanDAG(Logi return all_nodes; } +bool ExistCycle(vector &all_nodes) { + unordered_map visited; + for (auto *node : all_nodes) { + if (visited.count(node->table_idx)) { + continue; + } + idx_t cntNodes = 0; + idx_t cntEdges = 0; + vector que; + auto discover = [&](PhysicalDAGNode *candidate) { + if (visited.count(candidate->table_idx)) { + return; + } + visited[candidate->table_idx] = true; + que.push_back(candidate); + cntNodes++; + cntEdges += candidate->parents.size() + candidate->children.size(); + }; + discover(node); + while (!que.empty()) { + auto cur = que.back(); + que.pop_back(); + for (auto &child : cur->children) { + discover(child); + } + for (auto &parent : cur->parents) { + discover(parent); + } + } + if (cntEdges / 2 >= cntNodes) { + return true; + } + } + return false; +} + void RobustOptimizerContextState::FlipRootsToLeaves(vector &all_nodes) { // step 1: find all roots vector roots; @@ -1679,6 +1715,10 @@ unique_ptr RobustOptimizerContextState::Optimize(unique_ptr uf_parent; auto all_nodes = BuildPhysicalPlanDAG(plan.get(), uf_parent); + if (ExistCycle(all_nodes)) { + D_PRINTF("Cycle Detected"); + return plan; + } // flip non-largest roots to leaves (default: on) Value flip_val; bool flip_roots = true; From 58941c9c42142aadb2996062a0451318e01d509a Mon Sep 17 00:00:00 2001 From: Jianhong Shi Date: Wed, 5 Aug 2026 01:22:18 -0500 Subject: [PATCH 2/3] fix incorrect cyclic join test --- test/sql/plan_positive.test | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/test/sql/plan_positive.test b/test/sql/plan_positive.test index 8dd1b04..7431a0f 100644 --- a/test/sql/plan_positive.test +++ b/test/sql/plan_positive.test @@ -84,26 +84,7 @@ WHERE t1.k12 = t2.k12 ---- physical_plan :.*PROBE_FILTER.* -# Correctness: triangle join -query II -EXPLAIN SELECT count(*) -FROM t2, t3, t4 -WHERE t2.k23 = t3.k23 - AND t3.k34 = t4.k34 - AND t2.k24 = t4.k24; ----- -physical_plan :.*CREATE_FILTER.* - -query II -EXPLAIN SELECT count(*) -FROM t2, t3, t4 -WHERE t2.k23 = t3.k23 - AND t3.k34 = t4.k34 - AND t2.k24 = t4.k24; ----- -physical_plan :.*PROBE_FILTER.* - -# Negative: four-table chain join +# Positive: four-table chain join query II EXPLAIN SELECT count(*) FROM t1, t2, t3, t4 From 536760c3966e20c62de98a5124f9070841bc204d Mon Sep 17 00:00:00 2001 From: Jianhong Shi Date: Fri, 28 Aug 2026 06:01:04 -0500 Subject: [PATCH 3/3] move detect-and-bail guard earlier --- src/optimizer/robust_optimizer.cpp | 86 ++++++++++++++++-------------- src/optimizer/robust_optimizer.hpp | 2 + 2 files changed, 48 insertions(+), 40 deletions(-) diff --git a/src/optimizer/robust_optimizer.cpp b/src/optimizer/robust_optimizer.cpp index 5bb8c69..ee603e4 100644 --- a/src/optimizer/robust_optimizer.cpp +++ b/src/optimizer/robust_optimizer.cpp @@ -175,8 +175,29 @@ ColumnBinding RobustOptimizerContextState::ResolveColumnBinding(const ColumnBind return current; } +static idx_t TableUFFind(unordered_map &parent, idx_t x) { + if (parent.find(x) == parent.end()) { + parent[x] = x; + } + while (parent[x] != x) { + parent[x] = parent[parent[x]]; + x = parent[x]; + } + return x; +} + +static void TableUFUnion(unordered_map &parent, idx_t a, idx_t b) { + a = TableUFFind(parent, a); + b = TableUFFind(parent, b); + if (a != b) { + parent[a] = b; + } +} + vector RobustOptimizerContextState::CreateJoinEdges(vector &join_ops) { vector edges; + unordered_map table_parent; + set> seen_pairs; for (auto &op : join_ops) { auto &join = op->Cast(); @@ -215,6 +236,26 @@ vector RobustOptimizerContextState::CreateJoinEdges(vector v) { + std::swap(u, v); + } + if (!seen_pairs.count({u, v})) { + if (TableUFFind(table_parent, u) == TableUFFind(table_parent, v)) { + exist_cycle = true; + break; + } else { + seen_pairs.insert({u, v}); + TableUFUnion(table_parent, u, v); + } + } + } + if (exist_cycle) { + break; } } } @@ -796,42 +837,6 @@ vector RobustOptimizerContextState::BuildPhysicalPlanDAG(Logi return all_nodes; } -bool ExistCycle(vector &all_nodes) { - unordered_map visited; - for (auto *node : all_nodes) { - if (visited.count(node->table_idx)) { - continue; - } - idx_t cntNodes = 0; - idx_t cntEdges = 0; - vector que; - auto discover = [&](PhysicalDAGNode *candidate) { - if (visited.count(candidate->table_idx)) { - return; - } - visited[candidate->table_idx] = true; - que.push_back(candidate); - cntNodes++; - cntEdges += candidate->parents.size() + candidate->children.size(); - }; - discover(node); - while (!que.empty()) { - auto cur = que.back(); - que.pop_back(); - for (auto &child : cur->children) { - discover(child); - } - for (auto &parent : cur->parents) { - discover(parent); - } - } - if (cntEdges / 2 >= cntNodes) { - return true; - } - } - return false; -} - void RobustOptimizerContextState::FlipRootsToLeaves(vector &all_nodes) { // step 1: find all roots vector roots; @@ -1698,6 +1703,11 @@ unique_ptr RobustOptimizerContextState::Optimize(unique_ptr RobustOptimizerContextState::Optimize(unique_ptr uf_parent; auto all_nodes = BuildPhysicalPlanDAG(plan.get(), uf_parent); - if (ExistCycle(all_nodes)) { - D_PRINTF("Cycle Detected"); - return plan; - } // flip non-largest roots to leaves (default: on) Value flip_val; bool flip_roots = true; diff --git a/src/optimizer/robust_optimizer.hpp b/src/optimizer/robust_optimizer.hpp index 10bb7be..11f743e 100644 --- a/src/optimizer/robust_optimizer.hpp +++ b/src/optimizer/robust_optimizer.hpp @@ -65,6 +65,8 @@ class RobustOptimizerContextState : public ClientContextState { unordered_map rename_col_bindings; + bool exist_cycle = false; + public: // extract all the join edges from the plan // vector ExtractOperators(LogicalOperator &plan, vector &join_ops);