fixed issue #2400

CHANGELOG

@@ -158,7 +158,13 @@ v3.2.alpha1 (2017-02-05)
 * generated Foxx services now use swagger tags
 
 
-v3.1.16 (2017-XX-XX)
+v3.1.17 (2017-XX-XX)
+--------------------
+
+* fixed issue #2400
+
+
+v3.1.16 (2017-03-27)
 --------------------
 
 * fixed issue #2392
@@ -173,6 +179,22 @@ v3.1.16 (2017-XX-XX)
 
 * avoid name resolution when given connection string is a valid ip address
 
+* helps with issue #1842, bug in COLLECT statement in connection with LIMIT.
+
+* fix locking bug in cluster traversals
+
+* increase lock timeout defaults
+
+* increase various cluster timeouts
+
+* limit default target size for revision cache to 1GB, which is better for
+  tight RAM situations (used to be 40% of (totalRAM - 1GB), use
+  --database.revision-cache-target-size <VALUEINBYTES> to get back the
+  old behaviour
+
+* fixed a bug with restarted servers indicating status as "STARTUP"
+  rather that "SERVING" in Nodes UI.
+
 
 v3.1.15 (2017-03-20)
 --------------------

arangod/Aql/AstNode.cpp

@@ -1878,7 +1878,7 @@ bool AstNode::isCacheable() const {
 /// user-defined function
 bool AstNode::callsUserDefinedFunction() const {
   if (isConstant()) {
-    return true;
+    return false;
   }
 
   // check sub-nodes first
@@ -1887,14 +1887,36 @@ bool AstNode::callsUserDefinedFunction() const {
   for (size_t i = 0; i < n; ++i) {
     auto member = getMemberUnchecked(i);
 
-    if (!member->callsUserDefinedFunction()) {
-      return false;
+    if (member->callsUserDefinedFunction()) {
+      return true;
     }
   }
 
   return (type == NODE_TYPE_FCALL_USER);
 }
 
+/// @brief whether or not a node (and its subnodes) may contain a call to a
+/// function or user-defined function
+bool AstNode::callsFunction() const {
+  if (isConstant()) {
+    return false;
+  }
+
+  // check sub-nodes first
+  size_t const n = numMembers();
+
+  for (size_t i = 0; i < n; ++i) {
+    auto member = getMemberUnchecked(i);
+
+    if (member->callsFunction()) {
+      // abort early
+      return true;
+    }
+  }
+
+  return (type == NODE_TYPE_FCALL || type == NODE_TYPE_FCALL_USER);
+}
+
 /// @brief whether or not the object node contains dynamically named attributes
 /// on its first level
 bool AstNode::containsDynamicAttributeName() const {

arangod/Aql/AstNode.h

@@ -481,6 +481,10 @@ struct AstNode {
   /// @brief whether or not a node (and its subnodes) may contain a call to a
   /// user-defined function
   bool callsUserDefinedFunction() const;
+  
+  /// @brief whether or not a node (and its subnodes) may contain a call to a
+  /// a function or a user-defined function
+  bool callsFunction() const;
 
   /// @brief whether or not the object node contains dynamically named
   /// attributes

arangod/Aql/OptimizerRules.cpp

@@ -1581,7 +1581,6 @@ void arangodb::aql::removeUnnecessaryCalculationsRule(
     TRI_ASSERT(outvars.size() == 1);
     auto varsUsedLater = n->getVarsUsedLater();
 
-
     if (varsUsedLater.find(outvars[0]) == varsUsedLater.end()) {
       // The variable whose value is calculated here is not used at
       // all further down the pipeline! We remove the whole
@@ -1669,7 +1668,6 @@ void arangodb::aql::removeUnnecessaryCalculationsRule(
         vars.clear();
       }
 
-
       if (usageCount == 1) {
         // our variable is used by exactly one other calculation
         // now we can replace the reference to our variable in the other
@@ -1687,6 +1685,15 @@ void arangodb::aql::removeUnnecessaryCalculationsRule(
           // expression types (V8 vs. non-V8) do not match. give up
           continue;
         }
+ 
+        if (!n->isInInnerLoop() && 
+            rootNode->callsFunction() && 
+            other->isInInnerLoop()) {
+          // original expression calls a function and is not contained in a loop
+          // we're about to move this expression into a loop, but we don't want
+          // to move (expensive) function calls into loops
+          continue;
+        }
 
         TRI_ASSERT(other != nullptr);
         otherExpression->replaceVariableReference(outvars[0], rootNode);

js/server/tests/aql/aql-optimizer-rule-remove-redundant-calculations.js

@@ -69,7 +69,9 @@ function optimizerRuleTestSuite () {
       var queries = [
         "FOR i IN [ { a: 1 }, { a: 2 }, { a: 3 } ] LET a = i.a LET b = i.a RETURN [ a, b ]",
         "FOR i IN [ { a: 1 }, { a: 2 }, { a: 3 } ] LET a = LENGTH(i), b = LENGTH(i) RETURN [ a, b ]",
-        "FOR i IN [ { a: 1 }, { a: 2 }, { a: 3 } ] LET a = MAX(i) RETURN MAX(i)"
+        "FOR i IN [ { a: 1 }, { a: 2 }, { a: 3 } ] LET a = MAX(i) RETURN MAX(i)",
+        "LET a = NOOPT(CONCAT('a', 'b')) FOR i IN [ 1, 2, 3 ] RETURN CONCAT(a, 'b')",
+        "FOR i IN [ 1, 2, 3 ] LET a = CONCAT(i, 'b') RETURN CONCAT(a, 'b')"
       ];
 
       queries.forEach(function(query) {
@@ -89,7 +91,8 @@ function optimizerRuleTestSuite () {
         ["FOR i IN [ { a: 1 }, { a: 2 }, { a: 3 } ] LET a = LENGTH(i), b = LENGTH(i) + 1 RETURN [ a, b ]", true],
         ["FOR i IN [ " + collection + " ] LET a = MAX(i.a) RETURN MIN(i.a)", true],
         ["FOR i IN [ { a: 1 }, { a: 2 }, { a: 3 } ] LET a = RAND(), b = RAND() RETURN [ a, b ]", false],
-        ["FOR i IN [ " + collection + " ] LET c = MAX(i.a) COLLECT d = c LET i = RAND() LET b = i.a RETURN d",true]
+        ["FOR i IN [ " + collection + " ] LET c = MAX(i.a) COLLECT d = c LET i = RAND() LET b = i.a RETURN d", true],
+        ["LET a = NOOPT(CONCAT('a', 'b')) FOR i IN [ 1, 2, 3 ] RETURN CONCAT(a, 'b')", true]
       ];
 
       queryList.forEach(function(query) {

js/server/tests/aql/aql-optimizer-rule-remove-unnecessary-calculations.js

@@ -82,7 +82,8 @@ function optimizerRuleTestSuite () {
         "FOR i IN [1] LET a = i, b = i RETURN [ a, b ]",
         "FOR i IN [1] LET a = i + 1, b = i - 1 RETURN [ a, b ]",
         "FOR i IN [1] LET a = i RETURN a.value",
-        "FOR i IN [1] LET a = i RETURN a[0]"
+        "FOR i IN [1] LET a = i RETURN a[0]",
+        "FOR i IN [ 1, 2, 3 ] LET a = CONCAT(i, 'b') RETURN CONCAT(a, 'b')"
       ];
 
       queries.forEach(function(query) {
@@ -169,7 +170,10 @@ function optimizerRuleTestSuite () {
         
         // v8 vs. non-v8 expression types
         "FOR doc IN [ { a: 1 }, { a: 2 } ] LET a = V8(ATTRIBUTES(doc)) RETURN KEEP(doc, a)",
-        "FOR doc IN [ { a: 1 }, { a: 2 } ] LET a = ATTRIBUTES(doc) RETURN V8(KEEP(doc, a))"
+        "FOR doc IN [ { a: 1 }, { a: 2 } ] LET a = ATTRIBUTES(doc) RETURN V8(KEEP(doc, a))",
+        
+        // different loop
+        "LET a = NOOPT(CONCAT('a', 'b')) FOR i IN [ 1, 2, 3 ] RETURN CONCAT(a, 'b')"
       ];
 
       queries.forEach(function(query) {
@@ -266,7 +270,10 @@ function optimizerRuleTestSuite () {
         
         // same expression types
         "FOR doc IN [ { a: 1 }, { a: 2 } ] LET a = V8(ATTRIBUTES(doc)) RETURN V8(KEEP(doc, a))",
-        "FOR doc IN [ { a: 1 }, { a: 2 } ] LET a = ATTRIBUTES(doc) RETURN KEEP(doc, a)"
+        "FOR doc IN [ { a: 1 }, { a: 2 } ] LET a = ATTRIBUTES(doc) RETURN KEEP(doc, a)",
+        
+        // same loop
+        "FOR i IN [ 1, 2, 3 ] LET a = CONCAT(i, 'b') RETURN CONCAT(a, 'b')"
       ];
 
       queries.forEach(function(query) {