Fix bug in StrippedQuery

Reviewers: buda, teon.banek

Reviewed By: buda

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D482

src/query/frontend/ast/ast.hpp

@@ -806,7 +806,12 @@ class NodeAtom : public PatternAtom {
   DEFVISITABLE(TreeVisitor<TypedValue>);
   bool Accept(HierarchicalTreeVisitor &visitor) override {
     if (visitor.PreVisit(*this)) {
-      identifier_->Accept(visitor);
+      bool cont = identifier_->Accept(visitor);
+      for (auto &property : properties_) {
+        if (cont) {
+          cont = property.second->Accept(visitor);
+        }
+      }
     }
     return visitor.PostVisit(*this);
   }
@@ -837,7 +842,12 @@ class EdgeAtom : public PatternAtom {
   DEFVISITABLE(TreeVisitor<TypedValue>);
   bool Accept(HierarchicalTreeVisitor &visitor) override {
     if (visitor.PreVisit(*this)) {
-      identifier_->Accept(visitor);
+      bool cont = identifier_->Accept(visitor);
+      for (auto &property : properties_) {
+        if (cont) {
+          cont = property.second->Accept(visitor);
+        }
+      }
     }
     return visitor.PostVisit(*this);
   }
@@ -1396,6 +1406,8 @@ class CachedAst {
     LiteralsPlugger(const Parameters &parameters) : parameters_(parameters) {}
 
     bool Visit(PrimitiveLiteral &literal) override {
+      // TODO: If literal is a part of NamedExpression then we need to change
+      // text in NamedExpression, otherwise wrong header will be returned.
       permanent_assert(
           literal.token_position_ != -1,
           "Use AstPlugLiteralsVisitor only on ast created by parsing queries");

src/query/frontend/semantic/symbol_generator.cpp

@@ -281,7 +281,8 @@ bool SymbolGenerator::PreVisit(NodeAtom &node_atom) {
     kv.second->Accept(*this);
   }
   scope_.in_property_map = false;
-  return true;
+  node_atom.identifier_->Accept(*this);
+  return false;
 }
 
 bool SymbolGenerator::PostVisit(NodeAtom &node_atom) {
@@ -310,7 +311,8 @@ bool SymbolGenerator::PreVisit(EdgeAtom &edge_atom) {
     kv.second->Accept(*this);
   }
   scope_.in_property_map = false;
-  return true;
+  edge_atom.identifier_->Accept(*this);
+  return false;
 }
 
 bool SymbolGenerator::PostVisit(EdgeAtom &edge_atom) {

src/query/frontend/stripped.cpp

@@ -42,7 +42,9 @@ StrippedQuery::StrippedQuery(const std::string &query) {
 
   // Convert tokens to strings, perform lowercasing and filtering.
   for (const auto *token : tokens) {
-    int position = token->getTokenIndex();
+    // Position is calculated in query after stripping and whitespace
+    // normalisation, not before.
+    int position = token_strings.size() * 2;
 
     switch (token->getType()) {
       case CypherLexer::UNION:

src/query/interpreter.hpp

@@ -32,9 +32,6 @@ class Interpreter : public Loggable {
     Context ctx(config, db_accessor);
     std::map<std::string, TypedValue> summary;
 
-    // query -> stripped query
-    StrippedQuery stripped(query);
-
     // stripped query -> high level tree
     AstTreeStorage ast_storage = [&]() {
       if (!FLAGS_ast_cache) {
@@ -48,8 +45,11 @@ class Interpreter : public Loggable {
         return std::move(visitor.storage());
       }
 
+      // query -> stripped query
+      StrippedQuery stripped(query);
+
       auto ast_cache_accessor = ast_cache_.access();
-      auto it = ast_cache_accessor.find(query::StrippedQuery(query).hash());
+      auto it = ast_cache_accessor.find(stripped.hash());
       if (it == ast_cache_accessor.end()) {
         // stripped query -> AST
         frontend::opencypher::Parser parser(stripped.query());

tests/unit/query_engine.cpp

@@ -59,5 +59,14 @@ TEST(QueryEngine, AstCache) {
     ASSERT_EQ(stream.GetResults()[0].size(), 1U);
     ASSERT_EQ(stream.GetResults()[0][0].Value<double>(), 11.5);
   }
+  {
+    // Cached ast, same literals, different whitespaces.
+    ResultStreamFaker stream;
+    auto dba = dbms.active();
+    engine.Run("RETURN 10.5+1", *dba, stream);
+    ASSERT_EQ(stream.GetResults().size(), 1U);
+    ASSERT_EQ(stream.GetResults()[0].size(), 1U);
+    ASSERT_EQ(stream.GetResults()[0][0].Value<double>(), 11.5);
+  }
 }
 }