do not materialize entire collections using V8 (#4087)

arangod/Aql/Ast.cpp

@@ -172,6 +172,11 @@ AstNode* Ast::createNodeExample(AstNode const* variable,
 
   return node;
 }
+  
+/// @brief create subquery node
+AstNode* Ast::createNodeSubquery() {
+  return createNode(NODE_TYPE_SUBQUERY);
+}
 
 /// @brief create an AST for node
 AstNode* Ast::createNodeFor(char const* variableName, size_t nameLength,
@@ -192,6 +197,24 @@ AstNode* Ast::createNodeFor(char const* variableName, size_t nameLength,
   return node;
 }
 
+/// @brief create an AST for node, using an existing output variable 
+AstNode* Ast::createNodeFor(Variable* variable, AstNode const* expression) {
+  if (variable == nullptr) {
+    THROW_ARANGO_EXCEPTION(TRI_ERROR_OUT_OF_MEMORY);
+  }
+
+  AstNode* node = createNode(NODE_TYPE_FOR);
+  node->reserve(2);
+  
+  AstNode* v = createNode(NODE_TYPE_VARIABLE);
+  v->setData(static_cast<void*>(variable));
+
+  node->addMember(v);
+  node->addMember(expression);
+
+  return node;
+}
+
 /// @brief create an AST let node, without an IF condition
 AstNode* Ast::createNodeLet(char const* variableName, size_t nameLength,
                             AstNode const* expression,

arangod/Aql/Ast.h

@@ -149,8 +149,14 @@ class Ast {
   /// @brief create an AST example node
   AstNode* createNodeExample(AstNode const*, AstNode const*);
 
+  /// @brief create an AST subquery node
+  AstNode* createNodeSubquery();
+
   /// @brief create an AST for node
   AstNode* createNodeFor(char const*, size_t, AstNode const*, bool);
+  
+  /// @brief create an AST for node, using an existing output variable
+  AstNode* createNodeFor(Variable*, AstNode const*);
 
   /// @brief create an AST let node, without an IF condition
   AstNode* createNodeLet(char const*, size_t, AstNode const*, bool);

arangod/Aql/ExecutionPlan.cpp

@@ -381,15 +381,104 @@ ExecutionNode* ExecutionPlan::createCalculation(
     TRI_ASSERT(expression->numMembers() == 1);
     expression = expression->getMember(0);
   }
+ 
+  bool containsCollection = false;
+  // replace occurrences of collection names used as function call arguments
+  // (that are of type NODE_TYPE_COLLECTION) with their string equivalents
+  // for example, this will turn `WITHIN(collection, ...)` into
+  // `WITHIN("collection", ...)`
+  auto visitor = [this, &containsCollection](AstNode* node, void*) {
+    if (node->type == NODE_TYPE_FCALL) {
+      auto func = static_cast<Function*>(node->getData());
+
+      // check function arguments
+      auto args = node->getMember(0);
+      size_t const n = args->numMembers();
+
+      for (size_t i = 0; i < n; ++i) {
+        auto member = args->getMemberUnchecked(i);
+        auto conversion = func->getArgumentConversion(i);
+
+        if (member->type == NODE_TYPE_COLLECTION &&
+            (conversion == Function::CONVERSION_REQUIRED ||
+             conversion == Function::CONVERSION_OPTIONAL)) {
+          // collection attribute: no need to check for member simplicity
+          args->changeMember(i, _ast->createNodeValueString(member->getStringValue(), member->getStringLength()));
+        }
+      }
+    } else if (node->type == NODE_TYPE_COLLECTION) {
+      containsCollection = true;
+    }
+      
+    return node;
+  };
+
+  // replace NODE_TYPE_COLLECTION function call arguments in the expression
+  auto node = Ast::traverseAndModify(const_cast<AstNode*>(expression), visitor, nullptr);
+
+  if (containsCollection) {
+    // we found at least one occurence of NODE_TYPE_COLLECTION
+    // now replace them with proper (FOR doc IN collection RETURN doc) 
+    // subqueries
+    auto visitor = [this, &previous](AstNode* node, void*) {
+      if (node->type == NODE_TYPE_COLLECTION) {
+        // create an on-the-fly subquery for a full collection access
+        AstNode* rootNode = _ast->createNodeSubquery();
+
+        // FOR part
+        Variable* v = _ast->variables()->createTemporaryVariable();
+        AstNode* forNode = _ast->createNodeFor(v, node);
+        // RETURN part
+        AstNode* returnNode = _ast->createNodeReturn(_ast->createNodeReference(v));
+        
+        // add both nodes to subquery
+        rootNode->addMember(forNode);
+        rootNode->addMember(returnNode);
+
+        // produce the proper ExecutionNodes from the subquery AST 
+        auto subquery = fromNode(rootNode);
+        if (subquery == nullptr) {
+          THROW_ARANGO_EXCEPTION(TRI_ERROR_OUT_OF_MEMORY);
+        }
+
+        // and register a reference to the subquery result in the expression
+        v = _ast->variables()->createTemporaryVariable();
+        auto en = registerNode(new SubqueryNode(this, nextId(), subquery, v));
+        _subqueries[v->id] = en;
+        en->addDependency(previous);
+        previous = en;
+        return _ast->createNodeReference(v);
+      }
+      
+      return node;
+    };
+
+    // replace remaining NODE_TYPE_COLLECTION occurrences in the expression 
+    node = Ast::traverseAndModify(node, visitor, nullptr);
+  }
+
+  if (!isDistinct && node->type == NODE_TYPE_REFERENCE) {
+    // further optimize if we are only left with a reference to a subquery
+    auto subquery = getSubqueryFromExpression(node);
+
+    if (subquery != nullptr) {
+      // optimization: if the LET a = ... references a variable created by a
+      // subquery,
+      // change the output variable of the (anonymous) subquery to be the
+      // outvariable of
+      // the LET. and don't create the LET
+
+      subquery->replaceOutVariable(out);
+      return subquery;
+    }
+  }
 
   // generate a temporary calculation node
-  auto expr =
-      std::make_unique<Expression>(this, _ast, const_cast<AstNode*>(expression));
+  auto expr = std::make_unique<Expression>(this, _ast, node);
 
   CalculationNode* en;
   if (conditionVariable != nullptr) {
-    en =
-        new CalculationNode(this, nextId(), expr.get(), conditionVariable, out);
+    en = new CalculationNode(this, nextId(), expr.get(), conditionVariable, out);
   } else {
     en = new CalculationNode(this, nextId(), expr.get(), out);
   }
@@ -1995,13 +2084,7 @@ bool ExecutionPlan::isDeadSimple() const {
       return false;
     }
 
-    auto dep = current->getFirstDependency();
-
-    if (dep == nullptr) {
-      break;
-    }
-
-    current = dep;
+    current = current->getFirstDependency();
   }
 
   return true;

arangod/Aql/OptimizerRules.cpp

@@ -4342,7 +4342,7 @@ void arangodb::aql::inlineSubqueriesRule(Optimizer* opt,
 
     std::unordered_set<Variable const*> varsUsed;
 
-    current = n;
+    current = n->getFirstParent();
     // now check where the subquery is used
     while (current->hasParent()) {
       if (current->getType() == EN::ENUMERATE_LIST) {

arangod/Aql/V8Executor.cpp

@@ -698,9 +698,10 @@ void V8Executor::generateCodeCollection(AstNode const* node) {
   TRI_ASSERT(node != nullptr);
   TRI_ASSERT(node->numMembers() == 0);
 
-  _buffer->appendText(TRI_CHAR_LENGTH_PAIR("_AQL.GET_DOCUMENTS("));
-  generateCodeString(node->getStringValue(), node->getStringLength());
-  _buffer->appendChar(')');
+  // we should not get here anymore, as all collection accesses should
+  // have either been transformed to collection names (i.e. strings)
+  // or FOR ... RETURN ... subqueries beforehand
+  THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_INTERNAL, "unexpected node type 'collection' found in script generatin");
 }
 
 /// @brief generate JavaScript code for a full view access

js/server/tests/aql/aql-fail-on-warning.js

@@ -98,12 +98,12 @@ function ahuacatlFailOnWarningTestSuite () {
       }
     },
 
-    testEnabledWithCollectionInExpression : function () {
+    testEnabledArrayExpected : function () {
       try {
-        AQL_EXECUTE("RETURN _users + 1", null, { failOnWarning: true }).json;
+        AQL_EXECUTE("RETURN MEDIAN(7)", null, { failOnWarning: true }).json;
         fail();
       } catch (err) {
-        assertEqual(errors.ERROR_QUERY_COLLECTION_USED_IN_EXPRESSION.code, err.errorNum);
+        assertEqual(errors.ERROR_QUERY_ARRAY_EXPECTED.code, err.errorNum);
       }
     }
   };