do not scatter empty view (#5318)

* do not distribute scatter views without links

* simplify view downcasting

* refactor & add some tests

* adjust comments

arangod/Aql/BasicBlocks.h

@@ -170,7 +170,7 @@ class ReturnBlock final : public ExecutionBlock {
 
 class NoResultsBlock final : public ExecutionBlock {
  public:
-  NoResultsBlock(ExecutionEngine* engine, NoResultsNode const* ep)
+  NoResultsBlock(ExecutionEngine* engine, ExecutionNode const* ep)
       : ExecutionBlock(engine, ep) {}
 
   ~NoResultsBlock() {}

arangod/Aql/EngineInfoContainerCoordinator.cpp

@@ -123,10 +123,6 @@ void EngineInfoContainerCoordinator::addNode(ExecutionNode* node) {
       && node->getType() != ExecutionNode::ENUMERATE_COLLECTION
   );
 
-#ifdef USE_IRESEARCH
-  TRI_ASSERT(node->getType() != ExecutionNode::ENUMERATE_IRESEARCH_VIEW);
-#endif
-
   TRI_ASSERT(!_engines.empty());
   TRI_ASSERT(!_engineStack.empty());
   size_t idx = _engineStack.top();

arangod/IResearch/IResearchLink.cpp

@@ -168,11 +168,7 @@ int IResearchLink::drop() {
 
   if (arangodb::ServerState::instance()->isDBServer()) {
     // TODO FIXME find a better way to look up an iResearch View
-    #ifdef ARANGODB_ENABLE_MAINTAINER_MODE
-      auto* view = dynamic_cast<IResearchViewDBServer*>(_wiew.get());
-    #else
-      auto* view = static_cast<IResearchViewDBServer*>(_wiew.get());
-    #endif
+    auto* view = LogicalView::cast<IResearchViewDBServer>(_wiew.get());
 
     return view
       ? view->drop(_collection->id()).errorNumber()
@@ -235,11 +231,7 @@ bool IResearchLink::init(arangodb::velocypack::Slice const& definition) {
       // create the IResearchView for the specific collection (on DBServer)
       if (arangodb::ServerState::instance()->isDBServer()) {
         // TODO FIXME find a better way to look up an iResearch View
-        #ifdef ARANGODB_ENABLE_MAINTAINER_MODE
-          auto* view = dynamic_cast<IResearchViewDBServer*>(logicalView.get());
-        #else
-          auto* view = static_cast<IResearchViewDBServer*>(logicalView.get());
-        #endif
+        auto* view = LogicalView::cast<IResearchViewDBServer>(logicalView.get());
 
         if (view) {
           wiew = logicalView; // remeber the DBServer view instance
@@ -250,11 +242,7 @@ bool IResearchLink::init(arangodb::velocypack::Slice const& definition) {
       }
 
       // TODO FIXME find a better way to look up an iResearch View
-      #ifdef ARANGODB_ENABLE_MAINTAINER_MODE
-        auto* view = dynamic_cast<IResearchView*>(logicalView.get());
-      #else
-        auto* view = static_cast<IResearchView*>(logicalView.get());
-      #endif
+      auto* view = LogicalView::cast<IResearchView>(logicalView.get());
 
       if (!view) {
         LOG_TOPIC(WARN, arangodb::iresearch::TOPIC)

arangod/IResearch/IResearchViewDBServer.cpp

@@ -480,11 +480,7 @@ std::shared_ptr<arangodb::LogicalView> IResearchViewDBServer::ensure(
   }
 
   // TODO FIXME find a better way to look up an iResearch View
-  #ifdef ARANGODB_ENABLE_MAINTAINER_MODE
-    auto* view = dynamic_cast<IResearchViewDBServer*>(wiew.get());
-  #else
-    auto* view = static_cast<IResearchViewDBServer*>(wiew.get());
-  #endif
+  auto* view = LogicalView::cast<IResearchViewDBServer>(wiew.get());
 
   if (!view) {
     LOG_TOPIC(WARN, arangodb::iresearch::TOPIC)

arangod/IResearch/IResearchViewNode.cpp

@@ -22,15 +22,17 @@
 ////////////////////////////////////////////////////////////////////////////////
 
 #include "IResearchCommon.h"
+#include "IResearchViewCoordinator.h"
 #include "IResearchViewDBServer.h"
 #include "IResearchViewNode.h"
 #include "IResearchViewBlock.h"
 #include "IResearchOrderFactory.h"
 #include "IResearchView.h"
 #include "AqlHelper.h"
-#include "Aql/ExecutionPlan.h"
 #include "Aql/Ast.h"
+#include "Aql/BasicBlocks.h"
 #include "Aql/Condition.h"
+#include "Aql/ExecutionPlan.h"
 #include "Aql/SortCondition.h"
 #include "Aql/Query.h"
 #include "Aql/ExecutionEngine.h"
@@ -316,11 +318,14 @@ std::unique_ptr<aql::ExecutionBlock> IResearchViewNode::createBlock(
     std::unordered_set<std::string> const&
 ) const {
   if (ServerState::instance()->isCoordinator()) {
-    // coordinator in a cluster
-    THROW_ARANGO_EXCEPTION_MESSAGE(
-      TRI_ERROR_INTERNAL,
-      "IResearchView node is not intended to use on a coordinator"
-    );
+    // coordinator in a cluster: empty view case
+
+#ifdef ARANGODB_ENABLE_MAINTAINER_MODE
+    auto& view = LogicalView::cast<IResearchViewCoordinator>(this->view());
+    TRI_ASSERT(view.visitCollections([](TRI_voc_cid_t){ return false; }));
+#endif
+
+    return std::make_unique<aql::NoResultsBlock>(&engine, this);
   }
 
   auto* trx = engine.getQuery()->trx();
@@ -339,21 +344,9 @@ std::unique_ptr<aql::ExecutionBlock> IResearchViewNode::createBlock(
   PrimaryKeyIndexReader* reader;
 
   if (ServerState::instance()->isDBServer()) {
-    #ifdef ARANGODB_ENABLE_MAINTAINER_MODE
-      auto& view = dynamic_cast<IResearchViewDBServer const&>(this->view());
-    #else
-      auto& view = static_cast<IResearchViewDBServer const&>(this->view());
-    #endif
-
-    reader = view.snapshot(state);
+    reader = LogicalView::cast<IResearchViewDBServer>(this->view()).snapshot(state);
   } else {
-    #ifdef ARANGODB_ENABLE_MAINTAINER_MODE
-      auto& view = dynamic_cast<IResearchView const&>(this->view());
-    #else
-      auto& view = static_cast<IResearchView const&>(this->view());
-    #endif
-
-    reader = view.snapshot(state);
+    reader = LogicalView::cast<IResearchView>(this->view()).snapshot(state);
   }
 
   if (!reader) {

arangod/IResearch/IResearchViewOptimizerRules.cpp

@@ -492,6 +492,16 @@ void scatterViewInClusterRule(
 
   for (auto* node : nodes) {
     TRI_ASSERT(node);
+    auto& viewNode = static_cast<IResearchViewNode&>(*node);
+
+    if (viewNode.collections().empty()) {
+      // FIXME we have to invalidate plan cache (if exists)
+      // in case if corresponding view has been modified
+
+      // view has no associated collection, nothing to scatter
+      continue;
+    }
+
     auto const& parents = node->getParents();
     auto const& deps = node->getDependencies();
     TRI_ASSERT(deps.size() == 1);
@@ -508,13 +518,12 @@ void scatterViewInClusterRule(
       continue;
     }
 
-    bool const isRootNode = plan->isRoot(node);
-    plan->unlinkNode(node, true);
-
-    auto& viewNode = static_cast<IResearchViewNode&>(*node);
     auto& vocbase = viewNode.vocbase();
     auto& view = viewNode.view();
 
+    bool const isRootNode = plan->isRoot(node);
+    plan->unlinkNode(node, true);
+
     // insert a scatter node
     auto scatterNode = plan->registerNode(
       std::make_unique<IResearchViewScatterNode>(

arangod/VocBase/LogicalView.h

@@ -28,6 +28,7 @@
 #include "Basics/Common.h"
 #include "Basics/Result.h"
 #include "Basics/ReadWriteLock.h"
+#include "Meta/utility.h"
 #include "VocBase/voc-types.h"
 
 #include <velocypack/Buffer.h>
@@ -55,6 +56,53 @@ class LogicalView : public LogicalDataSource {
     std::shared_ptr<LogicalView>const& view // a pointer to the created view
   )> PreCommitCallback;
 
+
+  //////////////////////////////////////////////////////////////////////////////
+  /// @brief casts a specified 'LogicalView' to a provided Target type
+  //////////////////////////////////////////////////////////////////////////////
+  template<typename Target, typename Source>
+  inline static typename meta::adjustConst<Source, Target>::reference cast(
+      Source& view
+  ) noexcept {
+    typedef typename meta::adjustConst<
+      Source,
+      typename std::enable_if<
+        std::is_base_of<LogicalView, Target>::value
+          && std::is_same<typename std::remove_const<Source>::type,
+        LogicalView
+      >::value, Target>::type
+    >::reference target_type_t;
+
+  #ifdef ARANGODB_ENABLE_MAINTAINER_MODE
+    return dynamic_cast<target_type_t>(view);
+  #else
+    return static_cast<target_type_t>(view);
+  #endif
+  }
+
+  //////////////////////////////////////////////////////////////////////////////
+  /// @brief casts a specified 'LogicalView' to a provided Target type
+  //////////////////////////////////////////////////////////////////////////////
+  template<typename Target, typename Source>
+  inline static typename meta::adjustConst<Source, Target>::pointer cast(
+      Source* view
+  ) noexcept {
+    typedef typename meta::adjustConst<
+      Source,
+      typename std::enable_if<
+        std::is_base_of<LogicalView, Target>::value
+          && std::is_same<typename std::remove_const<Source>::type,
+        LogicalView
+      >::value, Target>::type
+    >::pointer target_type_t;
+
+  #ifdef ARANGODB_ENABLE_MAINTAINER_MODE
+    return dynamic_cast<target_type_t>(view);
+  #else
+    return static_cast<target_type_t>(view);
+  #endif
+  }
+
   //////////////////////////////////////////////////////////////////////////////
   /// @brief the category representing a logical view
   //////////////////////////////////////////////////////////////////////////////

lib/Meta/utility.h

@@ -29,12 +29,30 @@
 namespace arangodb {
 namespace meta {
 
+////////////////////////////////////////////////////////////////////////////////
+/// @brief adjusts constness of 'Out' according to 'In'
+////////////////////////////////////////////////////////////////////////////////
+template<typename In, typename Out>
+struct adjustConst {
+  typedef Out value_type;
+  typedef Out& reference;
+  typedef Out* pointer;
+};
+
+template<typename In, typename Out>
+struct adjustConst<const In, Out> {
+  typedef const Out value_type;
+  typedef const Out& reference;
+  typedef const Out* pointer;
+};
+
 template <class T, class U = T>
 T exchange(T& obj, U&& new_value) {
   T old_value = std::move(obj);
   obj = std::forward<U>(new_value);
   return old_value;
 }
+
 }
 }
 #endif

tests/IResearch/IResearchViewCoordinator-test.cpp

@@ -30,9 +30,12 @@
 #include "utils/log.hpp"
 
 #include "Aql/AqlFunctionFeature.h"
-#include "Aql/ExecutionPlan.h"
 #include "Aql/AstNode.h"
+#include "Aql/BasicBlocks.h"
+#include "Aql/ExecutionEngine.h"
+#include "Aql/ExecutionPlan.h"
 #include "Aql/Function.h"
+#include "Aql/OptimizerRulesFeature.h"
 #include "Aql/SortCondition.h"
 #include "Agency/Store.h"
 #include "Basics/ArangoGlobalContext.h"
@@ -54,6 +57,7 @@
 #include "IResearch/IResearchLinkHelper.h"
 #include "IResearch/IResearchLinkMeta.h"
 #include "IResearch/IResearchViewCoordinator.h"
+#include "IResearch/IResearchViewNode.h"
 #include "IResearch/SystemDatabaseFeature.h"
 #include "Logger/Logger.h"
 #include "Logger/LogTopic.h"
@@ -140,6 +144,7 @@ struct IResearchViewCoordinatorSetup {
     features.emplace_back(new arangodb::aql::AqlFunctionFeature(&server), true); // required for IResearchAnalyzerFeature
     features.emplace_back(new arangodb::iresearch::IResearchFeature(&server), true);
     features.emplace_back(new arangodb::iresearch::SystemDatabaseFeature(&server, system.get()), false); // required for IResearchAnalyzerFeature
+    features.emplace_back(new arangodb::aql::OptimizerRulesFeature(&server), true);
     features.emplace_back(new arangodb::FlushFeature(&server), false); // do not start the thread
     features.emplace_back(new arangodb::ClusterFeature(&server), false);
     features.emplace_back(new arangodb::AgencyFeature(&server), false);
@@ -3081,4 +3086,91 @@ SECTION("test_drop_link") {
   }
 }
 
+SECTION("IResearchViewNode::createBlock") {
+  auto* database = arangodb::DatabaseFeature::DATABASE;
+  REQUIRE(nullptr != database);
+
+  auto* ci = arangodb::ClusterInfo::instance();
+  REQUIRE(nullptr != ci);
+
+  std::string error;
+  TRI_vocbase_t* vocbase; // will be owned by DatabaseFeature
+
+  // create database
+  {
+    // simulate heartbeat thread
+    REQUIRE(TRI_ERROR_NO_ERROR == database->createDatabaseCoordinator(1, "testDatabase", vocbase));
+
+    REQUIRE(nullptr != vocbase);
+    CHECK("testDatabase" == vocbase->name());
+    CHECK(TRI_vocbase_type_e::TRI_VOCBASE_TYPE_COORDINATOR == vocbase->type());
+    CHECK(1 == vocbase->id());
+
+    CHECK(TRI_ERROR_NO_ERROR == ci->createDatabaseCoordinator(
+      vocbase->name(), VPackSlice::emptyObjectSlice(), error, 0.0
+    ));
+    CHECK("no error" == error);
+  }
+
+  // create and drop view (no id specified)
+  {
+    auto json = arangodb::velocypack::Parser::fromJson("{ \"name\": \"testView\", \"type\": \"arangosearch\" }");
+    arangodb::ViewID viewId;
+    error.clear(); // clear error message
+
+    CHECK(TRI_ERROR_NO_ERROR == ci->createViewCoordinator(
+      vocbase->name(), json->slice(), viewId, error
+    ));
+    CHECK(error.empty());
+
+    // get current plan version
+    auto planVersion = arangodb::tests::getCurrentPlanVersion();
+
+    auto view = ci->getView(vocbase->name(), viewId);
+    CHECK(nullptr != view);
+    CHECK(nullptr != std::dynamic_pointer_cast<arangodb::iresearch::IResearchViewCoordinator>(view));
+    CHECK(planVersion == view->planVersion());
+    CHECK("testView" == view->name());
+    CHECK(false == view->deleted());
+    CHECK(1 == view->id());
+    CHECK(arangodb::iresearch::DATA_SOURCE_TYPE == view->type());
+    CHECK(arangodb::LogicalView::category() == view->category());
+    CHECK(vocbase == &view->vocbase());
+
+    // dummy query
+    arangodb::aql::Query query(
+      false, *vocbase, arangodb::aql::QueryString("RETURN 1"),
+      nullptr, arangodb::velocypack::Parser::fromJson("{}"),
+      arangodb::aql::PART_MAIN
+    );
+    query.prepare(arangodb::QueryRegistryFeature::QUERY_REGISTRY, 42);
+
+    arangodb::aql::Variable const outVariable("variable", 0);
+
+    arangodb::iresearch::IResearchViewNode node(
+      *query.plan(),
+      42, // id
+      *vocbase, // database
+      *view, // view
+      outVariable,
+      nullptr, // no filter condition
+      {} // no sort condition
+    );
+
+    arangodb::aql::ExecutionEngine engine(&query);
+    std::unordered_map<arangodb::aql::ExecutionNode*, arangodb::aql::ExecutionBlock*> cache;
+    std::unordered_set<std::string> shards;
+    auto execBlock = node.createBlock(engine, cache, shards);
+    CHECK(nullptr != execBlock);
+    CHECK(nullptr != dynamic_cast<arangodb::aql::NoResultsBlock*>(execBlock.get()));
+
+    // drop view
+    CHECK(view->drop().ok());
+    CHECK(planVersion < arangodb::tests::getCurrentPlanVersion());
+
+    // check there is no more view
+    CHECK(nullptr == ci->getView(vocbase->name(), view->name()));
+  }
+}
+
 }

tests/IResearch/IResearchViewNode-test.cpp

@@ -70,13 +70,13 @@
 
 namespace {
 
-struct IResearchQuerySetup {
+struct IResearchViewNodeSetup {
   StorageEngineMock engine;
   arangodb::application_features::ApplicationServer server;
   std::unique_ptr<TRI_vocbase_t> system;
   std::vector<std::pair<arangodb::application_features::ApplicationFeature*, bool>> features;
 
-  IResearchQuerySetup(): server(nullptr, nullptr) {
+  IResearchViewNodeSetup(): server(nullptr, nullptr) {
     arangodb::EngineSelectorFeature::ENGINE = &engine;
 
     arangodb::tests::init(true);
@@ -124,7 +124,7 @@ struct IResearchQuerySetup {
     irs::logger::output_le(iresearch::logger::IRL_FATAL, stderr);
   }
 
-  ~IResearchQuerySetup() {
+  ~IResearchViewNodeSetup() {
     system.reset(); // destroy before reseting the 'ENGINE'
     arangodb::AqlFeature(&server).stop(); // unset singleton instance
     arangodb::LogTopic::setLogLevel(arangodb::iresearch::TOPIC.name(), arangodb::LogLevel::DEFAULT);
@@ -145,12 +145,12 @@ struct IResearchQuerySetup {
 
     arangodb::LogTopic::setLogLevel(arangodb::Logger::AUTHENTICATION.name(), arangodb::LogLevel::DEFAULT);
   }
-}; // IResearchQuerySetup
+}; // IResearchViewNodeSetup
 
 }
 
 TEST_CASE("IResearchViewNodeTest", "[iresearch][iresearch-view-node]") {
-  IResearchQuerySetup s;
+  IResearchViewNodeSetup s;
   UNUSED(s);
 
 SECTION("construct") {
@@ -422,7 +422,7 @@ SECTION("searialize") {
 }
 
 TEST_CASE("IResearchViewScatterNodeTest", "[iresearch][iresearch-view-node]") {
-  IResearchQuerySetup s;
+  IResearchViewNodeSetup s;
   UNUSED(s);
 
 SECTION("construct") {