simplify optimization rule for ArangoSearch views

arangodb · gnusi · Jan 10, 2019 · Dec 21, 2018 · Dec 23, 2018 · Dec 24, 2018
commit 30a9e290ec499e2f1f6f27fd6f33f6f1e619206c
diff --git a/arangod/IResearch/AqlHelper.cpp b/arangod/IResearch/AqlHelper.cpp
@@ -296,6 +296,25 @@ size_t hash(aql::AstNode const* node, size_t hash /*= 0*/) noexcept {
   }
 }
 
+irs::string_ref getFuncName(aql::AstNode const& node) {
+  irs::string_ref fname;
+
+  switch (node.type) {
+    case aql::NODE_TYPE_FCALL:
+      fname = reinterpret_cast<aql::Function const*>(node.getData())->name;
+      break;
+
+    case aql::NODE_TYPE_FCALL_USER:
+      parseValue(fname, node);
+      break;
+
+    default:
+      TRI_ASSERT(false);
+  }
+
+  return fname;
+}
+
 // ----------------------------------------------------------------------------
 // --SECTION--                                    AqlValueTraits implementation
 // ----------------------------------------------------------------------------

diff --git a/arangod/IResearch/AqlHelper.h b/arangod/IResearch/AqlHelper.h
@@ -80,6 +80,12 @@ inline irs::string_ref getStringRef(aql::AstNode const& node) {
   return irs::string_ref(node.getStringValue(), node.getStringLength());
 }
 
+//////////////////////////////////////////////////////////////////////////////
+/// @returns name of function denoted by a specified AstNode
+/// @note applicable for nodes of type NODE_TYPE_FCALL, NODE_TYPE_FCALL_USER
+//////////////////////////////////////////////////////////////////////////////
+irs::string_ref getFuncName(aql::AstNode const& node);
+
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief tries to extract 'size_t' value from the specified AstNode 'node'
 /// @returns true on success, false otherwise

diff --git a/arangod/IResearch/IResearchOrderFactory.h b/arangod/IResearch/IResearchOrderFactory.h
@@ -127,6 +127,26 @@ class ScorerReplacer {
   ////////////////////////////////////////////////////////////////////////////////
   void extract(aql::Variable const& var, std::vector<Scorer>& scorers);
 
+  ////////////////////////////////////////////////////////////////////////////////
+  /// @returns true if no scorers were replaced
+  ////////////////////////////////////////////////////////////////////////////////
+  bool empty() const noexcept {
+    return _dedup.empty();
+  }
+
+  ////////////////////////////////////////////////////////////////////////////////
+  /// @brief visits all replaced scorer entries
+  ////////////////////////////////////////////////////////////////////////////////
+  template<typename Visitor>
+  bool visit(Visitor visitor) const {
+    for (auto& entry : _dedup) {
+      if (!visitor(entry.first)) {
+        return false;
+      }
+    }
+    return true;
+  }
+
  private:
   struct ScorerHash {
     size_t operator()(Scorer const& key) const noexcept {

diff --git a/arangod/IResearch/IResearchViewOptimizerRules.cpp b/arangod/IResearch/IResearchViewOptimizerRules.cpp
@@ -30,6 +30,7 @@
 #include "Aql/ExecutionPlan.h"
 #include "Aql/ClusterNodes.h"
 #include "Aql/Condition.h"
+#include "Aql/Function.h"
 #include "Aql/Query.h"
 #include "Aql/SortNode.h"
 #include "Aql/Optimizer.h"
@@ -38,6 +39,8 @@
 #include "Utils/CollectionNameResolver.h"
 #include "VocBase/LogicalCollection.h"
 
+#include "utils/misc.hpp"
+
 using namespace arangodb::iresearch;
 using namespace arangodb::aql;
 using EN = arangodb::aql::ExecutionNode;
@@ -84,160 +87,74 @@ bool addView(
   return view.visitCollections(visitor);
 }
 
-///////////////////////////////////////////////////////////////////////////////
-/// @class IResearchViewConditionFinder
-///////////////////////////////////////////////////////////////////////////////
-class IResearchViewConditionHandler final
-    : public arangodb::aql::WalkerWorker<ExecutionNode> {
- public:
-  IResearchViewConditionHandler(
-      ExecutionPlan& plan,
-      std::set<arangodb::iresearch::IResearchViewNode const *>& processedViewNodes) noexcept
-   : _plan(&plan),
-     _processedViewNodes(&processedViewNodes) {
-  }
-
-  virtual bool before(ExecutionNode*) override;
-
-  virtual bool enterSubquery(ExecutionNode*, ExecutionNode*) override {
-    return false;
-  }
-
- private:
-  bool handleFilterCondition(
-    ExecutionNode* en,
-    Condition& condition
-  );
-
-  ScorerReplacer _scorerReplacer;
-  std::vector<Scorer> _scorers;
-  ExecutionPlan* _plan;
-  std::set<arangodb::iresearch::IResearchViewNode const*>* _processedViewNodes;
-}; // IResearchViewConditionFinder
-
-bool IResearchViewConditionHandler::before(ExecutionNode* en) {
-  TRI_ASSERT(_plan && _plan->getAst() && _plan->getAst()->query());
-  auto& query = *_plan->getAst()->query();
+bool optimizeSearchCondition(
+    IResearchViewNode& viewNode,
+    Query& query,
+    ExecutionPlan& plan
+) {
+  auto view = viewNode.view();
 
-  if (_processedViewNodes->size() >= query.views().size()) {
-    // all views were processed
-    return false;
+  // add view and linked collections to the query
+  if (!addView(*view, query)) {
+    THROW_ARANGO_EXCEPTION_MESSAGE(
+      TRI_ERROR_QUERY_PARSE,
+      "failed to process all collections linked with the view '" + view->name() + "'"
+    );
   }
 
-  switch (en->getType()) {
-    case EN::SINGLETON:
-    case EN::NORESULTS:
-      // in all these cases we better abort
-      return true;
+  // build search condition
+  Condition searchCondition(plan.getAst());
 
-    case EN::CALCULATION: {
-      auto* calcNode = EN::castTo<CalculationNode*>(en);
-      TRI_ASSERT(calcNode);
+  if (!viewNode.filterConditionIsEmpty()) {
+    searchCondition.andCombine(&viewNode.filterCondition());
+    searchCondition.normalize(&plan); // normalize the condition
 
-      _scorerReplacer.replace(*calcNode);
-      break;
-    }
-
-    case EN::ENUMERATE_IRESEARCH_VIEW: {
-      auto node = EN::castTo<IResearchViewNode*>(en);
-      auto& view = *node->view();
-
-      // add view and linked collections to the query
-      if (!addView(view, query)) {
-        THROW_ARANGO_EXCEPTION_MESSAGE(
-          TRI_ERROR_QUERY_PARSE,
-          "failed to process all collections linked with the view '" + view.name() + "'"
+    if (searchCondition.isEmpty()) {
+      // condition is always false
+      for (auto const& x : viewNode.getParents()) {
+        plan.insertDependency(
+          x,
+          plan.registerNode(std::make_unique<NoResultsNode>(&plan, plan.nextId()))
         );
       }
+      return false;
+    }
 
-      if (_processedViewNodes->find(node) != _processedViewNodes->end()) {
-        // already optimized this node
-        break;
-      }
-
-      // build filter condition
-      Condition filterCondition(_plan->getAst());
-
-      if (!node->filterConditionIsEmpty()) {
-        filterCondition.andCombine(&node->filterCondition());
-
-        if (!handleFilterCondition(en, filterCondition)) {
-          break;
-        }
-      }
-
-      // check filter condition
-      auto const canUseView = !filterCondition.root() || FilterFactory::filter(
-        nullptr,
-        { nullptr, nullptr, nullptr, nullptr, &node->outVariable() },
-        *filterCondition.root()
-      );
-
-      if (!canUseView) {
-        THROW_ARANGO_EXCEPTION_MESSAGE(
-          TRI_ERROR_QUERY_PARSE,
-          "unsupported SEARCH condition"
-        );
-      }
-
-      if (!filterCondition.isEmpty()) {
-        node->filterCondition(filterCondition.root());
-      }
-
-      // find scorers that have to be evaluated by a view
-      _scorerReplacer.extract(node->outVariable(), _scorers);
-      node->scorers(std::move(_scorers));
-
-      _processedViewNodes->insert(node);
+    auto const& varsValid = viewNode.getVarsValid();
 
-      break;
+    // remove all invalid variables from the condition
+    if (searchCondition.removeInvalidVariables(varsValid)) {
+      // removing left a previously non-empty OR block empty...
+      // this means we can't use the index to restrict the results
+      return false;
     }
-
-    default:
-      // in these cases we simply ignore the intermediate nodes, note
-      // that we have taken care of nodes that could throw exceptions
-      // above.
-      break;
   }
 
-  return false;
-}
-
-bool IResearchViewConditionHandler::handleFilterCondition(
-    ExecutionNode* en,
-    Condition& condition) {
-  // normalize the condition
-  condition.normalize(_plan);
-  TRI_IF_FAILURE("ConditionFinder::normalizePlan") {
-    THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG);
-  }
+  // check filter condition
+  auto const conditionValid = !searchCondition.root() || FilterFactory::filter(
+    nullptr,
+    { nullptr, nullptr, nullptr, nullptr, &viewNode.outVariable() },
+    *searchCondition.root()
+  );
 
-  if (condition.isEmpty()) {
-    // condition is always false
-    for (auto const& x : en->getParents()) {
-      auto noRes = new NoResultsNode(_plan, _plan->nextId());
-      _plan->registerNode(noRes);
-      _plan->insertDependency(x, noRes);
-    }
-    return false;
+  if (!conditionValid) {
+    THROW_ARANGO_EXCEPTION_MESSAGE(
+      TRI_ERROR_QUERY_PARSE,
+      "unsupported SEARCH condition"
+    );
   }
 
-  auto const& varsValid = en->getVarsValid();
-
-  // remove all invalid variables from the condition
-  if (condition.removeInvalidVariables(varsValid)) {
-    // removing left a previously non-empty OR block empty...
-    // this means we can't use the index to restrict the results
-    return false;
+  if (!searchCondition.isEmpty()) {
+    viewNode.filterCondition(searchCondition.root());
   }
 
   return true;
 }
 
 }
 
-NS_BEGIN(arangodb)
-NS_BEGIN(iresearch)
+namespace arangodb {
+namespace iresearch {
 
 /// @brief move filters and sort conditions into views
 void handleViewsRule(
@@ -246,55 +163,65 @@ void handleViewsRule(
     arangodb::aql::OptimizerRule const* rule
 ) {
   TRI_ASSERT(plan && plan->getAst() && plan->getAst()->query());
+  auto& query = *plan->getAst()->query();
 
-  if (plan->getAst()->query()->views().empty()) {
+  // ensure 'Optimizer::addPlan' will be called
+  bool modified = false;
+  auto addPlan = irs::make_finally([opt, &plan, rule, modified](){
+    opt->addPlan(std::move(plan), rule, modified);
+  });
+
+  if (query.views().empty()) {
     // nothing to do in absence of views
-    opt->addPlan(std::move(plan), rule, false);
     return;
   }
 
   SmallVector<ExecutionNode*>::allocator_type::arena_type a;
   SmallVector<ExecutionNode*> nodes{a};
 
-  // try to find `EnumerateViewNode`s and process corresponding filters and sorts
-  plan->findEndNodes(nodes, true);
-
-  // set of processed view nodes
-  std::set<IResearchViewNode const*> processedViewNodes;
+  // replace scorers in all calculation nodes with references
+  plan->findNodesOfType(nodes, EN::CALCULATION, true);
 
-  IResearchViewConditionHandler handler(*plan, processedViewNodes);
-  for (auto const& n : nodes) {
-    n->walk(handler);
-  }
+  ScorerReplacer scorerReplacer;
 
-  if (!processedViewNodes.empty()) {
-    std::unordered_set<ExecutionNode*> toUnlink;
+  for (auto* node : nodes) {
+    TRI_ASSERT(node && EN::CALCULATION == node->getType());
 
-    // remove sort setters covered by a view internally
-    for (auto* viewNode : processedViewNodes) {
-      TRI_ASSERT(viewNode);
+    scorerReplacer.replace(*EN::castTo<CalculationNode*>(node));
+  }
 
-      for (auto const& sort : viewNode->scorers()) {
-        auto const* var = sort.var;
+  modified = !scorerReplacer.empty();
 
-        if (!var) {
-          continue;
-        }
  // register replaced scorers to be evaluated by corresponding view nodes
+  nodes.clear();
+  plan->findNodesOfType(nodes, EN::ENUMERATE_IRESEARCH_VIEW, true);
 
-        auto* setter = plan->getVarSetBy(var->id);
+  std::vector<Scorer> scorers;
 
-        if (!setter || EN::CALCULATION != setter->getType()) {
-          continue;
-        }
+  for (auto* node : nodes) {
+    TRI_ASSERT(node && EN::ENUMERATE_IRESEARCH_VIEW == node->getType());
+    auto& viewNode = *EN::castTo<IResearchViewNode*>(node);
 
-        toUnlink.emplace(setter);
-      }
+    if (!optimizeSearchCondition(viewNode, query, *plan)) {
+      continue;
     }
 
-    plan->unlinkNodes(toUnlink);
+    // find scorers that have to be evaluated by a view
+    scorerReplacer.extract(viewNode.outVariable(), scorers);
+    viewNode.scorers(std::move(scorers));
   }
 
-  opt->addPlan(std::move(plan), rule, !processedViewNodes.empty());
+  scorerReplacer.visit([](Scorer const& scorer) -> bool {
+    TRI_ASSERT(scorer.node);
+    auto const funcName = iresearch::getFuncName(*scorer.node);
+
+    THROW_ARANGO_EXCEPTION_FORMAT(
+      TRI_ERROR_QUERY_PARSE,
+      "Non ArangoSearch view variable '%s' is used in scorer function '%s'",
+      scorer.var->name.c_str(),
+      funcName.c_str()
+    );
+  });
 }
 
 void scatterViewInClusterRule(
@@ -434,8 +361,8 @@ void scatterViewInClusterRule(
   opt->addPlan(std::move(plan), rule, wasModified);
 }
 
-NS_END // iresearch
-NS_END // arangodb
+} // iresearch
+} // arangodb
 
 // -----------------------------------------------------------------------------
 // --SECTION--                                                       END-OF-FILE