From fb6d683af9e9d8d3fa023a3218e13f82f1b7d595 Mon Sep 17 00:00:00 2001 From: Dmitry Yemanov Date: Sat, 1 Feb 2025 18:54:59 +0300 Subject: [PATCH 1/2] Fix #8109: Plan/Performance regression when using special construct for IN --- src/dsql/BoolNodes.cpp | 64 ++++++++++++++++++++++++++++++++++++++++++ src/dsql/BoolNodes.h | 3 ++ 2 files changed, 67 insertions(+) diff --git a/src/dsql/BoolNodes.cpp b/src/dsql/BoolNodes.cpp index 7b79c298069..dd6ade8d98d 100644 --- a/src/dsql/BoolNodes.cpp +++ b/src/dsql/BoolNodes.cpp @@ -1339,8 +1339,72 @@ BoolExprNode* InListBoolNode::copy(thread_db* tdbb, NodeCopier& copier) const return node; } +BoolExprNode* InListBoolNode::decompose(CompilerScratch* csb) +{ + // Collect list items depending on record streams + + HalfStaticArray splitItems; + + for (auto item : list->items) + { + SortedStreamList streams; + item->collectStreams(streams); + + if (streams.hasData()) + splitItems.add(item); + } + + if (splitItems.isEmpty()) + return nullptr; + + // Decompose expression: IN (, , , ...) + // into: IN (, , ...) OR = OR = ... + // where the ORed booleans are known to be stream-based (i.e. contain fields inside) + // and thus could use an index, if possible. + // + // See #8109 in the tracker, example: + // + // SELECT e.* + // FROM Employees e + // WHERE :SomeID IN (e.LeaderID, e.DispEmpID) + + auto& pool = csb->csb_pool; + BoolExprNode* boolNode = nullptr; + + for (const auto item : splitItems) + { + list->items.findAndRemove(item); + + const auto cmpNode = + FB_NEW_POOL(pool) ComparativeBoolNode(pool, blr_eql, arg, item); + + if (boolNode) + boolNode = FB_NEW_POOL(pool) BinaryBoolNode(pool, blr_or, boolNode, cmpNode); + else + boolNode = cmpNode; + } + + if (const auto count = list->items.getCount()) + { + BoolExprNode* priorNode = this; + + if (count == 1) + { + // Convert A IN (B) into A = B + priorNode = FB_NEW_POOL(pool) ComparativeBoolNode(pool, blr_eql, arg, list->items.front()); + } + + boolNode = FB_NEW_POOL(pool) BinaryBoolNode(pool, blr_or, boolNode, priorNode); + } + + return boolNode; +} + BoolExprNode* InListBoolNode::pass1(thread_db* tdbb, CompilerScratch* csb) { + if (const auto node = decompose(csb)) + return node->pass1(tdbb, csb); + doPass1(tdbb, csb, arg.getAddress()); nodFlags |= FLAG_INVARIANT; diff --git a/src/dsql/BoolNodes.h b/src/dsql/BoolNodes.h index 43ad33fcd04..5cceab24e42 100644 --- a/src/dsql/BoolNodes.h +++ b/src/dsql/BoolNodes.h @@ -187,6 +187,9 @@ class InListBoolNode : public TypedNode process) override; bool execute(thread_db* tdbb, Request* request) const override; +private: + BoolExprNode* decompose(CompilerScratch* csb); + public: NestConst arg; NestConst list; From d87dfed38474d3d3ae9ce50c5a0fbf4cc9ff9a56 Mon Sep 17 00:00:00 2001 From: Dmitry Yemanov Date: Mon, 3 Feb 2025 12:38:07 +0300 Subject: [PATCH 2/2] Optimize the code a bit: use one loop instead of two loops --- src/dsql/BoolNodes.cpp | 45 ++++++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/src/dsql/BoolNodes.cpp b/src/dsql/BoolNodes.cpp index dd6ade8d98d..fe6fb4fbd3e 100644 --- a/src/dsql/BoolNodes.cpp +++ b/src/dsql/BoolNodes.cpp @@ -1341,24 +1341,11 @@ BoolExprNode* InListBoolNode::copy(thread_db* tdbb, NodeCopier& copier) const BoolExprNode* InListBoolNode::decompose(CompilerScratch* csb) { - // Collect list items depending on record streams - - HalfStaticArray splitItems; - - for (auto item : list->items) - { - SortedStreamList streams; - item->collectStreams(streams); - - if (streams.hasData()) - splitItems.add(item); - } - - if (splitItems.isEmpty()) - return nullptr; - - // Decompose expression: IN (, , , ...) - // into: IN (, , ...) OR = OR = ... + // Search for list items depending on record streams. + // If found, decompose expression: + // IN (, , , ...) + // into: + // IN (, , ...) OR = OR = ... // where the ORed booleans are known to be stream-based (i.e. contain fields inside) // and thus could use an index, if possible. // @@ -1371,12 +1358,22 @@ BoolExprNode* InListBoolNode::decompose(CompilerScratch* csb) auto& pool = csb->csb_pool; BoolExprNode* boolNode = nullptr; - for (const auto item : splitItems) + for (auto iter = list->items.begin(); iter != list->items.end();) { - list->items.findAndRemove(item); + ValueExprNode* const item = *iter; + + SortedStreamList streams; + item->collectStreams(streams); + + if (streams.isEmpty()) + { + iter++; + continue; + } + + list->items.remove(iter); - const auto cmpNode = - FB_NEW_POOL(pool) ComparativeBoolNode(pool, blr_eql, arg, item); + const auto cmpNode = FB_NEW_POOL(pool) ComparativeBoolNode(pool, blr_eql, arg, item); if (boolNode) boolNode = FB_NEW_POOL(pool) BinaryBoolNode(pool, blr_or, boolNode, cmpNode); @@ -1384,11 +1381,11 @@ BoolExprNode* InListBoolNode::decompose(CompilerScratch* csb) boolNode = cmpNode; } - if (const auto count = list->items.getCount()) + if (boolNode && list->items.hasData()) { BoolExprNode* priorNode = this; - if (count == 1) + if (list->items.getCount() == 1) { // Convert A IN (B) into A = B priorNode = FB_NEW_POOL(pool) ComparativeBoolNode(pool, blr_eql, arg, list->items.front());