Bug fix 3.4/internal issue #633 (#9916)

* Ported fix for internal issue #633

* fixed indentation

* Another indent fixed

* Update CHANGELOG

CHANGELOG

@@ -1,6 +1,10 @@
 v3.4.9 (XXXX-XX-XX)
 -------------------
 
+* Fixed internal issue #633: made ArangoSearch functions BOOST, ANALYZER, MIN_MATCH
+  callable with constant arguments. This will allow running queries where all arguments
+  for these functions are deterministic and do not depend on loop variables.
+
 * Added UI support to create documents in a collection using smartGraphAttribute
   and/or smartJoinAttribute.
 

arangod/IResearch/IResearchFeature.cpp

@@ -118,7 +118,6 @@ NS_END  // aql
   }
 };  // IResearchLogTopic
 
-// template <char const *name>
 arangodb::aql::AqlValue dummyFilterFunc(arangodb::aql::Query*,
                                         arangodb::transaction::Methods*,
                                         arangodb::SmallVector<arangodb::aql::AqlValue> const&) {
@@ -131,6 +130,45 @@ arangodb::aql::AqlValue dummyFilterFunc(arangodb::aql::Query*,
       " Please ensure function signature is correct.");
 }
 
+/// function body for ArangoSearchContext functions ANALYZER/BOOST. 
+/// Just returns its first argument as outside ArangoSearch context
+/// there is nothing to do with search stuff, but optimization could roll.
+arangodb::aql::AqlValue dummyContextFunc(arangodb::aql::Query*,
+                                        arangodb::transaction::Methods*,
+                                        arangodb::SmallVector<arangodb::aql::AqlValue> const& args) {
+  TRI_ASSERT(!args.empty()); //ensured by function signature
+  return args[0];
+}
+
+/// Executes MIN_MATCH function with const parameters locally the same way it will be done in ArangoSearch on runtime
+/// This will allow optimize out MIN_MATCH call if all arguments are const
+arangodb::aql::AqlValue dummyMinMatchContextFunc(arangodb::aql::Query*,
+                                        arangodb::transaction::Methods* trx,
+                                        arangodb::SmallVector<arangodb::aql::AqlValue> const& args) {
+  TRI_ASSERT(args.size() > 1); // ensured by function signature
+  auto& minMatchValue = args.back();
+  if (ADB_LIKELY(minMatchValue.isNumber())) {
+    auto matchesLeft = minMatchValue.toInt64(trx);
+    const auto argsCount = args.size() - 1;
+    for (size_t i = 0; i < argsCount && matchesLeft > 0; ++i) {
+      auto& currValue = args[i];
+      if (currValue.toBoolean()) {
+        matchesLeft--;
+      }
+    }
+    return arangodb::aql::AqlValue(arangodb::aql::AqlValueHintBool(matchesLeft == 0));
+  } else {
+    auto message = std::string("'MIN_MATCH' AQL function: ")
+                     .append(" last argument has invalid type '") 
+                     .append(minMatchValue.getTypeString())
+                     .append("' (numeric expected)");
+    THROW_ARANGO_EXCEPTION_MESSAGE(
+      TRI_ERROR_BAD_PARAMETER,
+      message);
+  }
+}
+
+
 arangodb::aql::AqlValue dummyScorerFunc(arangodb::aql::Query*,
                                         arangodb::transaction::Methods*,
                                         arangodb::SmallVector<arangodb::aql::AqlValue> const&) {
@@ -190,9 +228,9 @@ void registerFilters(arangodb::aql::AqlFunctionFeature& functions) {
   addFunction(functions, {"STARTS_WITH", ".,.|.", flags, &dummyFilterFunc});  // (attribute, prefix, scoring-limit)
   addFunction(functions, {"PHRASE", ".,.|.+", flags, &dummyFilterFunc});  // (attribute, input [, offset, input... ] [, analyzer])
   addFunction(functions, {"IN_RANGE", ".,.,.,.,.", flags, &dummyFilterFunc});  // (attribute, lower, upper, include lower, include upper)
-  addFunction(functions, {"MIN_MATCH", ".,.|.+", flags, &dummyFilterFunc});  // (filter expression [, filter expression, ... ], min match count)
+  addFunction(functions, {"MIN_MATCH", ".,.|.+", flags, &dummyMinMatchContextFunc});  // (filter expression [, filter expression, ... ], min match count)
-  addFunction(functions, {"BOOST", ".,.", flags, &dummyFilterFunc});  // (filter expression, boost)
+  addFunction(functions, {"BOOST", ".,.", flags, &dummyContextFunc});  // (filter expression, boost)
-  addFunction(functions, {"ANALYZER", ".,.", flags, &dummyFilterFunc});  // (filter expression, analyzer)
+  addFunction(functions, {"ANALYZER", ".,.", flags, &dummyContextFunc});  // (filter expression, analyzer)
 }
 
 void registerIndexFactory() {
@@ -358,7 +396,9 @@ NS_BEGIN(arangodb)
 NS_BEGIN(iresearch)
 
 bool isFilter(arangodb::aql::Function const& func) noexcept {
-  return func.implementation == &dummyFilterFunc;
+  return func.implementation == &dummyFilterFunc ||
+         func.implementation == &dummyContextFunc ||
+         func.implementation == &dummyMinMatchContextFunc;
 }
 
 bool isScorer(arangodb::aql::Function const& func) noexcept {

arangod/IResearch/IResearchViewNode.cpp

@@ -228,10 +228,6 @@ bool parseOptions(aql::AstNode const* optionsNode,
 bool hasDependecies(aql::ExecutionPlan const& plan, aql::AstNode const& node,
                     aql::Variable const& ref,
                     std::unordered_set<aql::Variable const*>& vars) {
-  if (!node.isDeterministic()) {
-    return false;
-  }
-
   vars.clear();
   aql::Ast::getReferencedVariables(&node, vars);
   vars.erase(&ref);  // remove "our" variable
@@ -488,8 +484,7 @@ std::pair<bool, bool> IResearchViewNode::volatility(bool force /*=false*/) const
   }
 
   return std::make_pair(irs::check_bit<0>(_volatilityMask),  // filter
-                        irs::check_bit<1>(_volatilityMask)   // sort
+                        irs::check_bit<1>(_volatilityMask));   // sort
-  );
 }
 
 /// @brief toVelocyPack, for EnumerateViewNode

tests/IResearch/IResearchFilterBoolean-test.cpp

@@ -364,8 +364,26 @@ SECTION("UnaryNot") {
     assertFilterSuccess("LET c=41 FOR d IN collection FILTER ANALYZER(not (TO_STRING(c+1) == d.a['b'][23].c), 'test_analyzer') RETURN d", expected, &ctx);
     assertFilterSuccess("LET c=41 FOR d IN collection FILTER ANALYZER(not (TO_STRING(c+1) == d['a']['b'][23]['c']), 'test_analyzer') RETURN d", expected, &ctx);
     assertFilterSuccess("LET c=41 FOR d IN collection FILTER not ANALYZER(TO_STRING(c+1) == d['a']['b'][23]['c'], 'test_analyzer') RETURN d", expected, &ctx);
+  }
 
-    assertFilterExecutionFail("LET c=41 FOR d IN collection FILTER not (ANALYZER(TO_STRING(c+1), 'test_analyzer') == d['a']['b'][23]['c']) RETURN d", &ctx);
+  // filter with constexpr analyzer
+  {
+    arangodb::aql::Variable var("c", 0);
+    arangodb::aql::AqlValue value(arangodb::aql::AqlValueHintInt{41});
+    arangodb::aql::AqlValueGuard guard(value, true);
+
+    ExpressionContextMock ctx;
+    ctx.vars.emplace(var.name, value);
+    irs::Or expected;
+    expected.add<irs::Not>()
+      .filter<irs::And>()
+      .add<irs::by_term>()
+      .field(mangleStringIdentity("a.b[23].c"))
+      .term("42");
+    assertFilterSuccess(
+      "LET c=41 FOR d IN collection FILTER not (ANALYZER(TO_STRING(c+1), "
+      "'test_analyzer') == d['a']['b'][23]['c']) RETURN d",
+      expected, &ctx);
   }
 
   // dynamic complex attribute name

tests/IResearch/IResearchQueryComplexBoolean-test.cpp

@@ -476,15 +476,62 @@ TEST_CASE("IResearchQueryTestComplexBoolean", "[iresearch][iresearch-query]") {
     auto slice = result.result->slice();
     CHECK(slice.isArray());
     size_t i = 0;
-
     for (arangodb::velocypack::ArrayIterator itr(slice); itr.valid(); ++itr) {
       auto const resolved = itr.value().resolveExternals();
       CHECK((i < expected.size()));
       CHECK((0 == arangodb::basics::VelocyPackHelper::compare(expected[i++], resolved, true)));
     }
-
     CHECK((i == expected.size()));
   }
+
+  // constexpr BOOST (true)
+  {
+    std::string const query =
+      "FOR d IN testView SEARCH BOOST(1 == 1, 42) "
+      "LIMIT 1 "
+      "RETURN { d }";
+    auto queryResult = arangodb::tests::executeQuery(vocbase, query);
+    REQUIRE(TRI_ERROR_NO_ERROR == queryResult.code);
+    REQUIRE(queryResult.result->slice().isArray());
+    CHECK(1 == queryResult.result->slice().length());
+  }
+  // constexpr BOOST (false)
+  {
+    std::string const query =
+      "FOR d IN testView SEARCH BOOST(1==2, 42) "
+      "LIMIT 1 "
+      "RETURN { d }";
+    auto queryResult = arangodb::tests::executeQuery(vocbase, query);
+    REQUIRE(TRI_ERROR_NO_ERROR == queryResult.code);
+    REQUIRE(queryResult.result->slice().isArray());
+    CHECK(0 == queryResult.result->slice().length());
+  }
+
+  // constexpr min match (true)
+  {
+    std::string const query =
+      "FOR d IN testView SEARCH MIN_MATCH(1==1, 2==2, 3==3, 2) "
+      "SORT d.seq "
+      "RETURN d";
+    auto queryResult = arangodb::tests::executeQuery(vocbase, query);
+    REQUIRE(TRI_ERROR_NO_ERROR == queryResult.code);
+    auto slice = queryResult.result->slice();
+    REQUIRE(slice.isArray());
+    CHECK(insertedDocs.size() == slice.length());
+  }
+
+  // constexpr min match (false)
+  {
+    std::string const query =
+      "FOR d IN testView SEARCH MIN_MATCH(1==5, 2==6, 3==3, 2) "
+      "SORT d.seq "
+      "RETURN d";
+    auto queryResult = arangodb::tests::executeQuery(vocbase, query);
+    REQUIRE(TRI_ERROR_NO_ERROR == queryResult.code);
+    auto slice = queryResult.result->slice();
+    REQUIRE(slice.isArray());
+    CHECK(0 == slice.length());
+  }
 }
 
 // -----------------------------------------------------------------------------

tests/IResearch/IResearchQueryPhrase-test.cpp

@@ -601,6 +601,45 @@ TEST_CASE("IResearchQueryTestPhrase", "[iresearch][iresearch-query]") {
     REQUIRE(TRI_ERROR_QUERY_PARSE == result.code);
   }
 
+  // constexpr ANALYZER function (true)
+  {
+    std::vector<arangodb::velocypack::Slice> expected = {
+        insertedDocs[7].slice(),  insertedDocs[8].slice(),
+        insertedDocs[13].slice(), insertedDocs[19].slice(),
+        insertedDocs[22].slice(), insertedDocs[24].slice(),
+        insertedDocs[29].slice(),
+    };
+    auto result = arangodb::tests::executeQuery(
+        vocbase,
+        "FOR d IN testView SEARCH ANALYZER(1==1, 'test_analyzer') && ANALYZER(PHRASE(d.duplicated, 'z'), "
+        "'test_analyzer') SORT BM25(d) ASC, TFIDF(d) DESC, d.seq RETURN d");
+    REQUIRE(TRI_ERROR_NO_ERROR == result.code);
+    auto slice = result.result->slice();
+    REQUIRE(slice.isArray());
+    size_t i = 0;
+
+    for (arangodb::velocypack::ArrayIterator itr(slice); itr.valid(); ++itr) {
+      auto const resolved = itr.value().resolveExternals();
+      CHECK((i < expected.size()));
+      CHECK((0 == arangodb::basics::VelocyPackHelper::compare(expected[i++],
+                                                                    resolved, true)));
+    }
+    CHECK((i == expected.size()));
+  }
+
+  // constexpr ANALYZER function (false)
+  {
+    auto result = arangodb::tests::executeQuery(
+        vocbase,
+        "FOR d IN testView SEARCH ANALYZER(1==2, 'test_analyzer') && ANALYZER(PHRASE(d.duplicated, 'z'), "
+        "'test_analyzer') SORT BM25(d) ASC, TFIDF(d) DESC, d.seq RETURN d");
+    REQUIRE(TRI_ERROR_NO_ERROR == result.code);
+    auto slice = result.result->slice();
+    REQUIRE(slice.isArray());
+    CHECK(0 == slice.length());
+  }
+
+
   // test custom analyzer
   {
     std::vector<arangodb::velocypack::Slice> expected = {

tests/js/common/aql/aql-view-arangosearch-cluster.js

@@ -487,6 +487,30 @@ function IResearchAqlTestSuite(args) {
       });
     },
 	
+    testViewInInnerLoopSortByAttributeWithNonDeterministic : function() {
+      var expected = [];
+      expected.push({ a: "bar", b: "foo", c: 1 });
+      expected.push({ a: "baz", b: "foo", c: 1 });
+      expected.push({ a: "foo", b: "bar", c: 0 });
+      expected.push({ a: "foo", b: "baz", c: 0 });
+
+      var result = db._query(
+        "FOR adoc IN AnotherUnitTestsCollection " +
+        "FOR doc IN UnitTestsView SEARCH RAND() != -10 && STARTS_WITH(doc['a'], adoc.a) && adoc.id == doc.c OPTIONS { waitForSync : true } " +
+        "SORT doc.c DESC, doc.a, doc.b " +
+        "RETURN doc"
+      ).toArray();
+
+      assertEqual(result.length, expected.length);
+      var i = 0;
+      result.forEach(function(res) {
+        var doc = expected[i++];
+        assertEqual(doc.a, res.a);
+        assertEqual(doc.b, res.b);
+        assertEqual(doc.c, res.c);
+      });
+    },
+
     testWithKeywordForViewInGraph : function() {
       var results = [];
 
@@ -778,7 +802,33 @@ function IResearchAqlTestSuite(args) {
 
       assertEqual(result.length, 0);
     },
-
+    testAnalyzerFunctionPrematureCall : function () {
+      assertEqual(
+        db._query("FOR d in UnitTestsView SEARCH ANALYZER(d.a IN TOKENS('#', 'text_en'), 'text_en') OPTIONS { waitForSync : true } RETURN d").toArray().length,
+        0);
+      assertEqual(
+        db._query("FOR d in UnitTestsView SEARCH ANALYZER(d.a NOT IN TOKENS('#', 'text_en'), 'text_en') OPTIONS { waitForSync : true } RETURN d").toArray().length,
+        28);
+    },
+    testBoostFunctionPrematureCall : function () {
+      assertEqual(
+        db._query("FOR d in UnitTestsView SEARCH BOOST(d.a IN TOKENS('#', 'text_en'), 2) OPTIONS { waitForSync : true }  SORT BM25(d) RETURN d").toArray().length,
+        0);
+      assertEqual(
+        db._query("FOR d in UnitTestsView SEARCH BOOST(d.a NOT IN TOKENS('#', 'text_en'), 2) OPTIONS { waitForSync : true }  SORT BM25(d) RETURN d").toArray().length,
+        28);
+    },
+    testMinMatchFunctionPrematureCall : function () {
+      assertEqual(
+        db._query("FOR d in UnitTestsView SEARCH MIN_MATCH(d.a IN TOKENS('#', 'text_en'), d.a IN TOKENS('#', 'text_de'), 1) OPTIONS { waitForSync : true } RETURN d").toArray().length,
+        0);
+      assertEqual(
+        db._query("FOR d in UnitTestsView SEARCH MIN_MATCH(false, true, true, 2) OPTIONS { waitForSync : true }  RETURN d").toArray().length,
+        28);
+      assertEqual(
+        db._query("FOR d in UnitTestsView SEARCH MIN_MATCH(false, false, false, 0) OPTIONS { waitForSync : true }  RETURN d").toArray().length,
+        28);
+    },
     testViewWithInterruptedInserts : function() {
       let docsCollectionName = "docs";
       let docsViewName  = "docs_view";

tests/js/common/aql/aql-view-arangosearch-noncluster.js

@@ -497,6 +497,30 @@ function iResearchAqlTestSuite () {
       });
     },
 	
+    testViewInInnerLoopSortByAttributeWithNonDeterministic : function() {
+      var expected = [];
+      expected.push({ a: "bar", b: "foo", c: 1 });
+      expected.push({ a: "baz", b: "foo", c: 1 });
+      expected.push({ a: "foo", b: "bar", c: 0 });
+      expected.push({ a: "foo", b: "baz", c: 0 });
+
+      var result = db._query(
+        "FOR adoc IN AnotherUnitTestsCollection " +
+        "FOR doc IN UnitTestsView SEARCH RAND() != -10 && STARTS_WITH(doc['a'], adoc.a) && adoc.id == doc.c OPTIONS { waitForSync : true } " +
+        "SORT doc.c DESC, doc.a, doc.b " +
+        "RETURN doc"
+      ).toArray();
+
+      assertEqual(result.length, expected.length);
+      var i = 0;
+      result.forEach(function(res) {
+        var doc = expected[i++];
+        assertEqual(doc.a, res.a);
+        assertEqual(doc.b, res.b);
+        assertEqual(doc.c, res.c);
+      });
+    },
+
     testViewInInnerLoopSortByTFIDF_BM25_Attribute : function() {
       var expected = [];
       expected.push({ a: "baz", b: "foo", c: 1 });
@@ -803,6 +827,33 @@ function iResearchAqlTestSuite () {
 
       assertEqual(result.length, 0);
     },
+    testAnalyzerFunctionPrematureCall : function () {
+      assertEqual(
+        db._query("FOR d in UnitTestsView SEARCH ANALYZER(d.a IN TOKENS('#', 'text_en'), 'text_en') OPTIONS { waitForSync : true } RETURN d").toArray().length,
+        0);
+      assertEqual(
+        db._query("FOR d in UnitTestsView SEARCH ANALYZER(d.a NOT IN TOKENS('#', 'text_en'), 'text_en') OPTIONS { waitForSync : true } RETURN d").toArray().length,
+        28);
+    },
+    testBoostFunctionPrematureCall : function () {
+      assertEqual(
+        db._query("FOR d in UnitTestsView SEARCH BOOST(d.a IN TOKENS('#', 'text_en'), 2) OPTIONS { waitForSync : true }  SORT BM25(d) RETURN d").toArray().length,
+        0);
+      assertEqual(
+        db._query("FOR d in UnitTestsView SEARCH BOOST(d.a NOT IN TOKENS('#', 'text_en'), 2) OPTIONS { waitForSync : true }  SORT BM25(d) RETURN d").toArray().length,
+        28);
+    },
+    testMinMatchFunctionPrematureCall : function () {
+      assertEqual(
+        db._query("FOR d in UnitTestsView SEARCH MIN_MATCH(d.a IN TOKENS('#', 'text_en'), d.a IN TOKENS('#', 'text_de'), 1) OPTIONS { waitForSync : true } RETURN d").toArray().length,
+        0);
+      assertEqual(
+        db._query("FOR d in UnitTestsView SEARCH MIN_MATCH(false, true, true, 2) OPTIONS { waitForSync : true }  RETURN d").toArray().length,
+        28);
+      assertEqual(
+        db._query("FOR d in UnitTestsView SEARCH MIN_MATCH(false, false, false, 0) OPTIONS { waitForSync : true }  RETURN d").toArray().length,
+        28);
+    },
     testViewWithInterruptedInserts : function() {
       let docsCollectionName = "docs";
       let docsViewName  = "docs_view";