bug-fix/internal-issue-#6289 (#7249)

* improve troubleshooting

* fix tests

arangod/CMakeLists.txt

@@ -69,6 +69,7 @@ if (USE_IRESEARCH)
     IResearch/IResearchViewOptimizerRules.cpp IResearch/IResearchViewOptimizerRules.h
     IResearch/IResearchViewNode.cpp IResearch/IResearchViewNode.h
     IResearch/IResearchViewBlock.cpp IResearch/IResearchViewBlock.h
+    IResearch/IResearchExpressionContext.cpp IResearch/IResearchExpressionContext.h
     IResearch/IResearchViewMeta.cpp IResearch/IResearchViewMeta.h
     IResearch/IResearchFeature.cpp IResearch/IResearchFeature.h
     IResearch/IResearchDocument.cpp IResearch/IResearchDocument.h

arangod/IResearch/ExpressionFilter.cpp

@@ -23,9 +23,12 @@
 
 #include "ExpressionFilter.h"
 
+#include "Aql/AqlItemBlock.h"
 #include "Aql/AstNode.h"
 #include "Aql/AqlValue.h"
 
+#include "IResearch/IResearchViewBlock.h"
+
 #include "search/score_doc_iterators.hpp"
 #include "search/all_filter.hpp"
 #include "search/all_iterator.hpp"
@@ -34,7 +37,7 @@
 
 #include <type_traits>
 
-NS_LOCAL
+namespace {
 
 template<typename T>
 inline irs::filter::prepared::ptr compileQuery(
@@ -168,6 +171,9 @@ class NondeterministicExpressionQuery final : public irs::filter::prepared {
       return irs::doc_iterator::empty();
     }
 
+    // set expression for troubleshooting purposes
+    execCtx->ctx->_expr = _ctx.node.get();
+
     return irs::doc_iterator::make<NondeterministicExpressionIterator>(
       rdr,
       attributes(), // prepared_filter attributes
@@ -207,6 +213,9 @@ class DeterministicExpressionQuery final : public irs::filter::prepared {
       return irs::doc_iterator::empty();
     }
 
+    // set expression for troubleshooting purposes
+    execCtx->ctx->_expr = _ctx.node.get();
+
     arangodb::aql::Expression expr(_ctx.plan, _ctx.ast, _ctx.node.get());
     bool mustDestroy = false;
     auto value = expr.execute(execCtx->trx, execCtx->ctx, mustDestroy);
@@ -225,10 +234,10 @@ class DeterministicExpressionQuery final : public irs::filter::prepared {
   arangodb::iresearch::ExpressionCompilationContext _ctx;
 }; // DeterministicExpressionQuery
 
-NS_END // LOCAL
+}
 
-NS_BEGIN(arangodb)
+namespace arangodb {
-NS_BEGIN(iresearch)
+namespace iresearch {
 
 ///////////////////////////////////////////////////////////////////////////////
 /// --SECTION--                     ExpressionCompilationContext implementation
@@ -292,7 +301,7 @@ irs::filter::prepared::ptr ByExpression::prepare(
     );
   }
 
-  auto const* execCtx = ctx.get<arangodb::iresearch::ExpressionExecutionContext>().get();
+  auto* execCtx = ctx.get<arangodb::iresearch::ExpressionExecutionContext>().get();
 
   if (!execCtx || !static_cast<bool>(*execCtx)) {
     // no execution context provided, make deterministic query
@@ -301,6 +310,9 @@ irs::filter::prepared::ptr ByExpression::prepare(
     );
   }
 
+  // set expression for troubleshooting purposes
+  execCtx->ctx->_expr = _ctx.node.get();
+
   // evaluate expression
   bool mustDestroy = false;
   arangodb::aql::Expression expr(_ctx.plan, _ctx.ast, _ctx.node.get());
@@ -312,5 +324,5 @@ irs::filter::prepared::ptr ByExpression::prepare(
     : irs::filter::prepared::empty();
 }
 
-NS_END // iresearch
+} // iresearch
-NS_END // arangodb
+} // arangodb

arangod/IResearch/ExpressionFilter.h

@@ -25,6 +25,7 @@
 #define ARANGODB_IRESEARCH__IRESEARCH_EXPRESSION_FILTER 1
 
 #include "Aql/Expression.h"
+#include "IResearch/IResearchExpressionContext.h"
 
 #include "search/filter.hpp"
 
@@ -71,22 +72,27 @@ struct ExpressionExecutionContext : irs::attribute {
 
   ExpressionExecutionContext() = default;
 
-  explicit ExpressionExecutionContext(arangodb::transaction::Methods& trx)
+  explicit ExpressionExecutionContext(
+      arangodb::transaction::Methods& trx
+  ) noexcept
     : trx(&trx) {
   }
 
   ExpressionExecutionContext(
       arangodb::transaction::Methods& trx,
-      arangodb::aql::ExpressionContext& ctx
+      arangodb::iresearch::ViewExpressionContextBase& ctx
-  ) : trx(&trx), ctx(&ctx) {
+  ) noexcept : ctx(&ctx), trx(&trx) {
   }
 
   explicit operator bool() const noexcept {
     return trx && ctx;
   }
 
+  // FIXME change 'ctx' to be 'arangodb::aql::ExpressionContext'
+  // once IResearchView will be able to evaluate epxressions
+  // with loop variable in SEARCH expressions
+  arangodb::iresearch::ViewExpressionContextBase* ctx{};
   arangodb::transaction::Methods* trx{};
-  arangodb::aql::ExpressionContext* ctx{};
 }; // ExpressionFilterContext
 
 ///////////////////////////////////////////////////////////////////////////////

arangod/IResearch/IResearchExpressionContext.cpp

@@ -0,0 +1,110 @@
+////////////////////////////////////////////////////////////////////////////////
+/// DISCLAIMER
+///
+/// Copyright 2018 ArangoDB GmbH, Cologne, Germany
+///
+/// Licensed under the Apache License, Version 2.0 (the "License");
+/// you may not use this file except in compliance with the License.
+/// You may obtain a copy of the License at
+///
+///     http://www.apache.org/licenses/LICENSE-2.0
+///
+/// Unless required by applicable law or agreed to in writing, software
+/// distributed under the License is distributed on an "AS IS" BASIS,
+/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+/// See the License for the specific language governing permissions and
+/// limitations under the License.
+///
+/// Copyright holder is ArangoDB GmbH, Cologne, Germany
+///
+/// @author Andrey Abramov
+/// @author Vasiliy Nabatchikov
+////////////////////////////////////////////////////////////////////////////////
+
+#include "Aql/AqlItemBlock.h"
+#include "IResearch/IResearchExpressionContext.h"
+#include "IResearch/IResearchViewNode.h"
+
+namespace {
+
+inline arangodb::aql::RegisterId getRegister(
+    arangodb::aql::Variable const& var,
+    arangodb::aql::ExecutionNode const& node
+) noexcept {
+  auto const& vars = node.getRegisterPlan()->varInfo;
+  auto const it = vars.find(var.id);
+
+  return vars.end() == it
+    ? arangodb::aql::ExecutionNode::MaxRegisterId
+    : it->second.registerId;
+  }
+
+}
+
+namespace arangodb {
+namespace iresearch {
+
+using namespace arangodb::aql;
+
+// -----------------------------------------------------------------------------
+// --SECTION--                              ViewExpressionContext implementation
+// -----------------------------------------------------------------------------
+
+size_t ViewExpressionContext::numRegisters() const {
+  return _data->getNrRegs();
+}
+
+AqlValue ViewExpressionContext::getVariableValue(
+    Variable const* var, bool doCopy, bool& mustDestroy
+) const {
+  TRI_ASSERT(var);
+
+  if (var == &_node->outVariable()) {
+    // self-reference
+    if (_expr) {
+      std::string expr;
+
+      try {
+        expr = _expr->toString();
+      } catch (...) { }
+
+      if (!expr.empty()) {
+        THROW_ARANGO_EXCEPTION_FORMAT(
+          TRI_ERROR_NOT_IMPLEMENTED,
+          "Unable to evaluate loop variable '%s' as a part of ArangoSearch noncompliant expression '%s'",
+          var->name.c_str(),
+          expr.c_str()
+        );
+      }
+    }
+
+    THROW_ARANGO_EXCEPTION_FORMAT(
+      TRI_ERROR_NOT_IMPLEMENTED,
+      "Unable to evaluate loop variable '%s' as a part of ArangoSearch noncompliant expression",
+      var->name.c_str()
+    );
+  }
+
+  mustDestroy = false;
+  auto const reg = getRegister(*var, *_node);
+
+  if (reg == arangodb::aql::ExecutionNode::MaxRegisterId) {
+    THROW_ARANGO_EXCEPTION(TRI_ERROR_INTERNAL);
+  }
+
+  auto& value = _data->getValueReference(_pos, reg);
+
+  if (doCopy) {
+    mustDestroy = true;
+    return value.clone();
+  }
+
+  return value;
+}
+
+} // iresearch
+} // arangodb
+
+// -----------------------------------------------------------------------------
+// --SECTION--                                                       END-OF-FILE
+// -----------------------------------------------------------------------------

arangod/IResearch/IResearchExpressionContext.h

@@ -0,0 +1,89 @@
+////////////////////////////////////////////////////////////////////////////////
+/// DISCLAIMER
+///
+/// Copyright 2018 ArangoDB GmbH, Cologne, Germany
+///
+/// Licensed under the Apache License, Version 2.0 (the "License");
+/// you may not use this file except in compliance with the License.
+/// You may obtain a copy of the License at
+///
+///     http://www.apache.org/licenses/LICENSE-2.0
+///
+/// Unless required by applicable law or agreed to in writing, software
+/// distributed under the License is distributed on an "AS IS" BASIS,
+/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+/// See the License for the specific language governing permissions and
+/// limitations under the License.
+///
+/// Copyright holder is ArangoDB GmbH, Cologne, Germany
+///
+/// @author Andrey Abramov
+/// @author Vasiliy Nabatchikov
+////////////////////////////////////////////////////////////////////////////////
+
+#ifndef ARANGOD_IRESEARCH__IRESEARCH_EXPRESSION_CONTEXT_H
+#define ARANGOD_IRESEARCH__IRESEARCH_EXPRESSION_CONTEXT_H 1
+
+#include "Aql/QueryExpressionContext.h"
+#include "Basics/Exceptions.h"
+
+namespace arangodb {
+
+namespace aql {
+class AqlItemBlock;
+struct AstNode;
+class ExecutionEngine;
+} // aql
+
+namespace iresearch {
+
+class IResearchViewNode;
+
+///////////////////////////////////////////////////////////////////////////////
+/// @struct ViewExpressionContextBase
+/// @brief FIXME remove this struct once IResearchView will be able to evaluate
+///        epxressions with loop variable in SEARCH expressions
+///////////////////////////////////////////////////////////////////////////////
+struct ViewExpressionContextBase : public aql::QueryExpressionContext {
+  explicit ViewExpressionContextBase(arangodb::aql::Query* query)
+    : aql::QueryExpressionContext(query) {
+  }
+
+  aql::AstNode const* _expr{}; // for troubleshooting
+}; // ViewExpressionContextBase
+
+///////////////////////////////////////////////////////////////////////////////
+/// @struct ViewExpressionContext
+///////////////////////////////////////////////////////////////////////////////
+struct ViewExpressionContext final : public ViewExpressionContextBase {
+  ViewExpressionContext(arangodb::aql::Query* query, IResearchViewNode const& node)
+    : ViewExpressionContextBase(query),
+      _node(&node) {
+    TRI_ASSERT(_node);
+  }
+
+  virtual size_t numRegisters() const override;
+
+  virtual aql::AqlValue const& getRegisterValue(size_t i) const override {
+    THROW_ARANGO_EXCEPTION(TRI_ERROR_NOT_IMPLEMENTED);
+  }
+
+  virtual aql::Variable const* getVariable(size_t i) const override {
+    THROW_ARANGO_EXCEPTION(TRI_ERROR_NOT_IMPLEMENTED);
+  }
+
+  virtual aql::AqlValue getVariableValue(
+    aql::Variable const* variable,
+    bool doCopy,
+    bool& mustDestroy
+  ) const override;
+
+  aql::AqlItemBlock const* _data{};
+  IResearchViewNode const* _node;
+  size_t _pos{};
+}; // ViewExpressionContext
+
+} // iresearch 
+} // arangodb
+
+#endif // ARANGOD_IRESEARCH__IRESEARCH_EXPRESSION_CONTEXT_H 1

arangod/IResearch/IResearchViewBlock.cpp

@@ -48,62 +48,11 @@
 #include "search/boolean_filter.hpp"
 #include "search/score.hpp"
 
-NS_LOCAL
+namespace arangodb {
-
+namespace iresearch {
-inline arangodb::aql::RegisterId getRegister(
-    arangodb::aql::Variable const& var,
-    arangodb::aql::ExecutionNode const& node
-) noexcept {
-  auto const& vars = node.getRegisterPlan()->varInfo;
-  auto const it = vars.find(var.id);
-
-  return vars.end() == it
-    ? arangodb::aql::ExecutionNode::MaxRegisterId
-    : it->second.registerId;
-  }
-
-NS_END // NS_LOCAL
-
-NS_BEGIN(arangodb)
-NS_BEGIN(iresearch)
 
 using namespace arangodb::aql;
 
-// -----------------------------------------------------------------------------
-// --SECTION--                              ViewExpressionContext implementation
-// -----------------------------------------------------------------------------
-
-size_t ViewExpressionContext::numRegisters() const {
-  return _data->getNrRegs();
-}
-
-AqlValue ViewExpressionContext::getVariableValue(
-    Variable const* var, bool doCopy, bool& mustDestroy
-) const {
-  TRI_ASSERT(var);
-
-  if (var == &_node->outVariable()) {
-    // self-reference
-    THROW_ARANGO_EXCEPTION(TRI_ERROR_INTERNAL);
-  }
-
-  mustDestroy = false;
-  auto const reg = getRegister(*var, *_node);
-
-  if (reg == arangodb::aql::ExecutionNode::MaxRegisterId) {
-    THROW_ARANGO_EXCEPTION(TRI_ERROR_INTERNAL);
-  }
-
-  auto& value = _data->getValueReference(_pos, reg);
-
-  if (doCopy) {
-    mustDestroy = true;
-    return value.clone();
-  }
-
-  return value;
-}
-
 // -----------------------------------------------------------------------------
 // --SECTION--                             IResearchViewBlockBase implementation
 // -----------------------------------------------------------------------------

arangod/IResearch/IResearchViewBlock.h

@@ -27,61 +27,24 @@
 
 #include "Aql/ExecutionBlock.h"
 #include "Aql/ExecutionNode.h"
-#include "Aql/QueryExpressionContext.h"
 #include "Indexes/IndexIterator.h"
 #include "VocBase/LogicalView.h"
 #include "VocBase/ManagedDocumentResult.h"
 
 #include "utils/attributes.hpp"
 #include "IResearch/ExpressionFilter.h"
+#include "IResearch/IResearchExpressionContext.h"
 #include "IResearch/IResearchView.h"
 
-NS_BEGIN(iresearch)
+namespace iresearch {
 class score;
-NS_END // iresearch
+} // iresearch
 
-NS_BEGIN(arangodb)
+namespace arangodb {
-NS_BEGIN(aql)
+namespace iresearch {
-class AqlItemBlock;
-class ExecutionEngine;
-NS_END // aql
-
-NS_BEGIN(iresearch)
 
 class IResearchViewNode;
 
-///////////////////////////////////////////////////////////////////////////////
-/// @class ViewExpressionContext
-///////////////////////////////////////////////////////////////////////////////
-class ViewExpressionContext final : public aql::QueryExpressionContext {
- public:
-  explicit ViewExpressionContext(arangodb::aql::Query* query, IResearchViewNode const& node)
-    : QueryExpressionContext(query),
-      _node(&node) {
-    TRI_ASSERT(_node);
-  }
-
-  virtual size_t numRegisters() const override;
-
-  virtual aql::AqlValue const& getRegisterValue(size_t i) const override {
-    THROW_ARANGO_EXCEPTION(TRI_ERROR_NOT_IMPLEMENTED);
-  }
-
-  virtual aql::Variable const* getVariable(size_t i) const override {
-    THROW_ARANGO_EXCEPTION(TRI_ERROR_NOT_IMPLEMENTED);
-  }
-
-  virtual aql::AqlValue getVariableValue(
-    aql::Variable const* variable,
-    bool doCopy,
-    bool& mustDestroy
-  ) const override;
-
-  aql::AqlItemBlock const* _data{};
-  IResearchViewNode const* _node;
-  size_t _pos{};
-}; // ViewExpressionContext
-
 ///////////////////////////////////////////////////////////////////////////////
 /// @class IResearchViewBlockBase
 ///////////////////////////////////////////////////////////////////////////////
@@ -224,7 +187,7 @@ class IResearchViewOrderedBlock final : public IResearchViewBlockBase {
   size_t _skip{};
 }; // IResearchViewOrderedBlock
 
-NS_END // iresearch
+} // iresearch
-NS_END // arangodb
+} // arangodb
 
 #endif // ARANGOD_IRESEARCH__ENUMERATE_VIEW_BLOCK_H 

tests/IResearch/ExpressionContextMock.cpp

@@ -52,10 +52,10 @@ arangodb::aql::AqlValue ExpressionContextMock::getVariableValue(
   return it->second;
 }
   
-TRI_vocbase_t& ExpressionContextMock::vocbase() const {
+//TRI_vocbase_t& ExpressionContextMock::vocbase() const {
-  THROW_ARANGO_EXCEPTION(TRI_ERROR_NOT_IMPLEMENTED);
+//  THROW_ARANGO_EXCEPTION(TRI_ERROR_NOT_IMPLEMENTED);
-}
+//}
-
+//
-arangodb::aql::Query* ExpressionContextMock::query() const {
+//arangodb::aql::Query* ExpressionContextMock::query() const {
-  THROW_ARANGO_EXCEPTION(TRI_ERROR_NOT_IMPLEMENTED);
+//  THROW_ARANGO_EXCEPTION(TRI_ERROR_NOT_IMPLEMENTED);
-}
+//}

tests/IResearch/ExpressionContextMock.h

@@ -25,6 +25,7 @@
 #define ARANGODB_IRESEARCH__IRESEARCH_EXPRESSION_CONTEXT 1
 
 #include "Aql/FixedVarExpressionContext.h"
+#include "IResearch/IResearchExpressionContext.h"
 
 #include <unordered_map>
 
@@ -38,9 +39,13 @@ struct Variable; // forward decl
 } // aql
 } // arangodb
 
-struct ExpressionContextMock final : arangodb::aql::ExpressionContext {
+struct ExpressionContextMock final : arangodb::iresearch::ViewExpressionContextBase {
   static ExpressionContextMock EMPTY;
 
+  ExpressionContextMock() noexcept
+    : arangodb::iresearch::ViewExpressionContextBase(nullptr) {
+  }
+
   virtual ~ExpressionContextMock();
 
   virtual size_t numRegisters() const override {
@@ -88,14 +93,14 @@ struct ExpressionContextMock final : arangodb::aql::ExpressionContext {
     return nullptr;
   }
 
-  bool killed() const override { 
+//  bool killed() const override {
-    TRI_ASSERT(false);
+//    TRI_ASSERT(false);
-    return false;
+//    return false;
-  }
+//  }
-
+//
-  TRI_vocbase_t& vocbase() const override;
+//  TRI_vocbase_t& vocbase() const override;
-  
+//
-  arangodb::aql::Query* query() const override;
+//  arangodb::aql::Query* query() const override;
 
   std::unordered_map<std::string, arangodb::aql::AqlValue> vars;
 }; // ExpressionContextMock

tests/IResearch/IResearchQueryJoin-test.cpp

@@ -725,7 +725,7 @@ TEST_CASE("IResearchQueryTestJoin", "[iresearch][iresearch-query]") {
     };
 
     auto queryResult = arangodb::tests::executeQuery(vocbase, query);
-    REQUIRE(TRI_ERROR_INTERNAL == queryResult.code); // can't handle self-referenced variable now
+    REQUIRE(TRI_ERROR_NOT_IMPLEMENTED == queryResult.code); // can't handle self-referenced variable now
 
 //    auto result = queryResult.result->slice();
 //    CHECK(result.isArray());