cleanup some of the internal APIs a bit (#10152)

arangod/Aql/Condition.cpp

@@ -556,7 +556,7 @@ void Condition::toVelocyPack(arangodb::velocypack::Builder& builder, bool verbos
 }
 
 /// @brief create a condition from VPack
-Condition* Condition::fromVPack(ExecutionPlan* plan, arangodb::velocypack::Slice const& slice) {
+std::unique_ptr<Condition> Condition::fromVPack(ExecutionPlan* plan, arangodb::velocypack::Slice const& slice) {
   auto condition = std::make_unique<Condition>(plan->getAst());
 
   if (slice.isObject() && slice.length() != 0) {
@@ -568,7 +568,7 @@ Condition* Condition::fromVPack(ExecutionPlan* plan, arangodb::velocypack::Slice
   condition->_isNormalized = true;
   condition->_isSorted = false;
 
-  return condition.release();
+  return condition;
 }
 
 /// @brief clone the condition

arangod/Aql/Condition.h

@@ -131,7 +131,7 @@ class Condition {
   void toVelocyPack(arangodb::velocypack::Builder&, bool) const;
 
   /// @brief create a condition from VPack
-  static Condition* fromVPack(ExecutionPlan*, arangodb::velocypack::Slice const&);
+  static std::unique_ptr<Condition> fromVPack(ExecutionPlan*, arangodb::velocypack::Slice const&);
 
   /// @brief clone the condition
   Condition* clone() const;

arangod/Aql/Functions.cpp

@@ -1077,7 +1077,7 @@ AqlValue dateFromParameters(
     months m{extractFunctionParameterValue(parameters, 1).toInt64()};
     days d{extractFunctionParameterValue(parameters, 2).toInt64()};
 
-    if ((y < years{0}) || (m < months{0}) || (d < days{0})) {
+    if ((y < years{0} || y > years{9999}) || (m < months{0}) || (d < days{0})) {
       registerWarning(expressionContext, AFN, TRI_ERROR_QUERY_INVALID_DATE_VALUE);
       return AqlValue(AqlValueHintNull());
     }
@@ -1121,7 +1121,6 @@ AqlValue dateFromParameters(
   }
 
   if (asTimestamp) {
-    // TODO: validate
     return AqlValue(AqlValueHintInt(time.count()));
   }
   return ::timeAqlValue(expressionContext, AFN, tp);
@@ -4187,7 +4186,7 @@ AqlValue Functions::Sleep(ExpressionContext* expressionContext,
   }
 
   auto& server = application_features::ApplicationServer::server();
-
+  
   double const sleepValue = value.toDouble();
   auto now = std::chrono::steady_clock::now();
   auto const endTime = now + std::chrono::milliseconds(static_cast<int64_t>(sleepValue * 1000.0));

arangod/Aql/IndexNode.cpp

@@ -107,7 +107,7 @@ IndexNode::IndexNode(ExecutionPlan* plan, arangodb::velocypack::Slice const& bas
         TRI_ERROR_BAD_PARAMETER, "\"condition\" attribute should be an object");
   }
 
-  _condition.reset(Condition::fromVPack(plan, condition));
+  _condition = Condition::fromVPack(plan, condition);
 
   TRI_ASSERT(_condition != nullptr);
 

arangod/Aql/Query.cpp

@@ -83,19 +83,19 @@ Query::Query(bool contextOwnedByExterior, TRI_vocbase_t& vocbase,
       _vocbase(vocbase),
       _context(nullptr),
       _queryString(queryString),
-      _queryBuilder(),
       _bindParameters(bindParameters),
       _options(options),
       _collections(&vocbase),
-      _state(QueryExecutionState::ValueType::INVALID_STATE),
       _trx(nullptr),
-      _warnings(),
       _startTime(TRI_microtime()),
-      _part(part),
+      _queryHash(DontCache),
       _contextOwnedByExterior(contextOwnedByExterior),
       _killed(false),
       _isModificationQuery(false),
       _preparedV8Context(false),
+      _queryHashCalculated(false),
+      _state(QueryExecutionState::ValueType::INVALID_STATE),
+      _part(part),
       _executionPhase(ExecutionPhase::INITIALIZE),
       _sharedState(std::make_shared<SharedQueryState>()) {
   if (_contextOwnedByExterior) {
@@ -163,15 +163,16 @@ Query::Query(bool contextOwnedByExterior, TRI_vocbase_t& vocbase,
       _queryBuilder(queryStruct),
       _options(options),
       _collections(&vocbase),
-      _state(QueryExecutionState::ValueType::INVALID_STATE),
       _trx(nullptr),
-      _warnings(),
       _startTime(TRI_microtime()),
-      _part(part),
+      _queryHash(DontCache),
       _contextOwnedByExterior(contextOwnedByExterior),
       _killed(false),
       _isModificationQuery(false),
       _preparedV8Context(false),
+      _queryHashCalculated(false),
+      _state(QueryExecutionState::ValueType::INVALID_STATE),
+      _part(part),
       _executionPhase(ExecutionPhase::INITIALIZE),
       _sharedState(std::make_shared<SharedQueryState>()) {
   // populate query options
@@ -408,7 +409,7 @@ void Query::prepare(QueryRegistry* registry, SerializationFormat format) {
 #endif
 
   if (plan == nullptr) {
-    plan.reset(preparePlan());
+    plan = preparePlan();
 
     TRI_ASSERT(plan != nullptr);
 
@@ -450,7 +451,7 @@ void Query::prepare(QueryRegistry* registry, SerializationFormat format) {
 /// execute calls it internally. The purpose of this separate method is
 /// to be able to only prepare a query from VelocyPack and then store it in the
 /// QueryRegistry.
-ExecutionPlan* Query::preparePlan() {
+std::unique_ptr<ExecutionPlan> Query::preparePlan() {
   LOG_TOPIC("9625e", DEBUG, Logger::QUERIES) << TRI_microtime() - _startTime << " "
                                              << "Query::prepare"
                                              << " this: " << (uintptr_t)this;
@@ -561,7 +562,7 @@ ExecutionPlan* Query::preparePlan() {
   // return the V8 context if we are in one
   exitContext();
 
-  return plan.release();
+  return plan;
 }
 
 /// @brief execute an AQL query

arangod/Aql/Query.h

@@ -26,6 +26,7 @@
 
 #include "Aql/BindParameters.h"
 #include "Aql/Collections.h"
+#include "Aql/ExecutionPlan.h"
 #include "Aql/ExecutionState.h"
 #include "Aql/Graphs.h"
 #include "Aql/QueryExecutionState.h"
@@ -38,8 +39,6 @@
 #include "Aql/SharedQueryState.h"
 #include "Aql/types.h"
 #include "Basics/Common.h"
-#include "Basics/ConditionLocker.h"
-#include "Basics/ConditionVariable.h"
 #include "V8Server/V8Context.h"
 #include "VocBase/voc-types.h"
 
@@ -69,7 +68,6 @@ namespace aql {
 struct AstNode;
 class Ast;
 class ExecutionEngine;
-class ExecutionPlan;
 class Query;
 struct QueryCacheResultEntry;
 struct QueryProfile;
@@ -309,7 +307,7 @@ class Query {
   CollectionNameResolver const& resolver();
 
 #ifdef ARANGODB_USE_GOOGLE_TESTS
-  ExecutionPlan* stealPlan() { return std::move(preparePlan()); }
+  std::unique_ptr<ExecutionPlan> stealPlan() { return preparePlan(); }
 #endif
 
  private:
@@ -323,7 +321,7 @@ class Query {
   /// execute calls it internally. The purpose of this separate method is
   /// to be able to only prepare a query from VelocyPack and then store it in
   /// the QueryRegistry.
-  ExecutionPlan* preparePlan();
+  std::unique_ptr<ExecutionPlan> preparePlan();
 
   /// @brief log a query
   void log();
@@ -401,10 +399,6 @@ class Query {
   /// @brief query execution profile
   std::unique_ptr<QueryProfile> _profile;
 
-  /// @brief current state the query is in (used for profiling and error
-  /// messages)
-  QueryExecutionState::ValueType _state;
-
   /// @brief the ExecutionPlan object, if the query is prepared
   std::shared_ptr<ExecutionPlan> _plan;
 
@@ -426,8 +420,8 @@ class Query {
   /// @brief query start time
   double _startTime;
 
-  /// @brief the query part
+  /// @brief hash for this query. will be calculated only once when needed
-  QueryPart const _part;
+  mutable uint64_t _queryHash = DontCache;
 
   /// @brief whether or not someone else has acquired a V8 context for us
   bool const _contextOwnedByExterior;
@@ -442,6 +436,9 @@ class Query {
   /// once for this expression
   /// it needs to be run once before any V8-based function is called
   bool _preparedV8Context;
+  
+  /// @brief whether or not the hash was already calculated
+  mutable bool _queryHashCalculated;
 
   /// Create the result in this builder. It is also used to determine
   /// if we are continuing the query or of we called
@@ -450,6 +447,13 @@ class Query {
   /// Options for _resultBuilder. Optimally, its lifetime should be linked to
   /// it, but this is hard to do.
   std::shared_ptr<arangodb::velocypack::Options> _resultBuilderOptions;
+  
+  /// @brief current state the query is in (used for profiling and error
+  /// messages)
+  QueryExecutionState::ValueType _state;
+  
+  /// @brief the query part
+  QueryPart const _part;
 
   /// Track in which phase of execution we are, in order to implement
   /// repeatability.
@@ -462,12 +466,6 @@ class Query {
   /// only populated when the query has generated its result(s) and before
   /// storing the cache entry in the query cache
   std::unique_ptr<QueryCacheResultEntry> _cacheEntry;
-
-  /// @brief hash for this query. will be calculated only once when needed
-  mutable uint64_t _queryHash = DontCache;
-
-  /// @brief whether or not the hash was already calculated
-  mutable bool _queryHashCalculated = false;
 };
 
 }  // namespace aql

arangod/Aql/RestAqlHandler.cpp

@@ -270,7 +270,7 @@ void RestAqlHandler::setupClusterQuery() {
 
 bool RestAqlHandler::registerSnippets(
     VPackSlice const snippetsSlice, VPackSlice const collectionSlice,
-    VPackSlice const variablesSlice, std::shared_ptr<VPackBuilder> options,
+    VPackSlice const variablesSlice, std::shared_ptr<VPackBuilder> const& options,
     std::shared_ptr<transaction::Context> const& ctx, double const ttl,
     SerializationFormat format, bool& needToLock, VPackBuilder& answerBuilder) {
   TRI_ASSERT(answerBuilder.isOpenObject());

arangod/Aql/RestAqlHandler.h

@@ -102,7 +102,7 @@ class RestAqlHandler : public RestVocbaseBaseHandler {
   bool registerSnippets(arangodb::velocypack::Slice const snippets,
                         arangodb::velocypack::Slice const collections,
                         arangodb::velocypack::Slice const variables,
-                        std::shared_ptr<arangodb::velocypack::Builder> options,
+                        std::shared_ptr<arangodb::velocypack::Builder> const& options,
                         std::shared_ptr<transaction::Context> const& ctx,
                         double const ttl, aql::SerializationFormat format,
                         bool& needToLock, arangodb::velocypack::Builder& answer);

arangod/Aql/TraversalConditionFinder.cpp

@@ -712,9 +712,8 @@ bool TraversalConditionFinder::before(ExecutionNode* en) {
       }
 
       if (!isEmpty) {
-        // node->setCondition(_condition.release());
         originalFilterConditions->normalize();
-        node->setCondition(originalFilterConditions.release());
+        node->setCondition(std::move(originalFilterConditions));
         // We restart here with an empty condition.
         // All Filters that have been collected thus far
         // depend on sth issued by this traverser or later

arangod/Aql/TraversalNode.cpp

@@ -100,7 +100,6 @@ TraversalNode::TraversalNode(ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocb
     : GraphNode(plan, id, vocbase, direction, graph, std::move(options)),
       _pathOutVariable(nullptr),
       _inVariable(nullptr),
-      _condition(nullptr),
       _fromCondition(nullptr),
       _toCondition(nullptr),
       _pruneExpression(std::move(pruneExpression)) {
@@ -169,7 +168,6 @@ TraversalNode::TraversalNode(ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocb
       _pathOutVariable(nullptr),
       _inVariable(inVariable),
       _vertexId(vertexId),
-      _condition(nullptr),
       _fromCondition(nullptr),
       _toCondition(nullptr) {}
 
@@ -177,7 +175,6 @@ TraversalNode::TraversalNode(ExecutionPlan* plan, arangodb::velocypack::Slice co
     : GraphNode(plan, base),
       _pathOutVariable(nullptr),
       _inVariable(nullptr),
-      _condition(nullptr),
       _fromCondition(nullptr),
       _toCondition(nullptr) {
   // In Vertex
@@ -273,11 +270,7 @@ TraversalNode::TraversalNode(ExecutionPlan* plan, arangodb::velocypack::Slice co
 #endif
 }
 
-TraversalNode::~TraversalNode() {
+TraversalNode::~TraversalNode() {}
-  if (_condition != nullptr) {
-    delete _condition;
-  }
-}
 
 int TraversalNode::checkIsOutVariable(size_t variableId) const {
   if (_vertexOutVariable != nullptr && _vertexOutVariable->id == variableId) {
@@ -699,7 +692,7 @@ void TraversalNode::prepareOptions() {
 }
 
 /// @brief remember the condition to execute for early traversal abortion.
-void TraversalNode::setCondition(arangodb::aql::Condition* condition) {
+void TraversalNode::setCondition(std::unique_ptr<arangodb::aql::Condition> condition) {
   arangodb::HashSet<Variable const*> varsUsedByCondition;
 
   Ast::getReferencedVariables(condition->root(), varsUsedByCondition);
@@ -713,7 +706,7 @@ void TraversalNode::setCondition(arangodb::aql::Condition* condition) {
     }
   }
 
-  _condition = condition;
+  _condition = std::move(condition);
 }
 
 void TraversalNode::registerCondition(bool isConditionOnEdge, uint64_t conditionLevel,

arangod/Aql/TraversalNode.h

@@ -158,10 +158,10 @@ class TraversalNode : public GraphNode {
   std::string const getStartVertex() const { return _vertexId; }
 
   /// @brief remember the condition to execute for early traversal abortion.
-  void setCondition(Condition* condition);
+  void setCondition(std::unique_ptr<Condition> condition);
 
   /// @brief return the condition for the node
-  Condition* condition() const { return _condition; }
+  Condition* condition() const { return _condition.get(); }
 
   /// @brief which variable? -1 none, 0 Edge, 1 Vertex, 2 path
   int checkIsOutVariable(size_t variableId) const;
@@ -210,7 +210,7 @@ class TraversalNode : public GraphNode {
   std::string _vertexId;
 
   /// @brief early abort traversal conditions:
-  Condition* _condition;
+  std::unique_ptr<Condition> _condition;
 
   /// @brief variables that are inside of the condition
   arangodb::HashSet<Variable const*> _conditionVariables;

lib/Basics/AttributeNameParser.cpp

@@ -22,6 +22,7 @@
 ////////////////////////////////////////////////////////////////////////////////
 
 #include <stddef.h>
+#include <algorithm>
 #include <memory>
 #include <ostream>
 
@@ -166,7 +167,7 @@ void arangodb::basics::TRI_AttributeNamesToString(std::vector<AttributeName> con
   TRI_ASSERT(result.empty());
 
   bool isFirst = true;
-  for (auto& it : input) {
+  for (auto const& it : input) {
     if (!isFirst) {
       result += ".";
     }
@@ -179,12 +180,9 @@ void arangodb::basics::TRI_AttributeNamesToString(std::vector<AttributeName> con
 }
 
 bool arangodb::basics::TRI_AttributeNamesHaveExpansion(std::vector<AttributeName> const& input) {
-  for (auto& it : input) {
+  return std::any_of(input.begin(), input.end(), [](AttributeName const& value) {
-    if (it.shouldExpand) {
+    return value.shouldExpand; 
-      return true;
+  });
-    }
-  }
-  return false;
 }
 
 ////////////////////////////////////////////////////////////////////////////////