Fix expression issues when using ES_QUERY in Lookup clause (#5597)

src/common/datatypes/Edge.cpp

@@ -82,9 +82,8 @@ const Value& Edge::value(const std::string& key) const {
   auto find = props.find(key);
   if (find != props.end()) {
     return find->second;
-  } else {
-    return Value::kNullValue;
   }
+  return Value::kNullValue;
 }
 
 bool Edge::operator<(const Edge& rhs) const {

src/common/datatypes/Vertex.cpp

@@ -61,6 +61,18 @@ const Value& Vertex::value(const std::string& key) const {
   return Value::kNullValue;
 }
 
+const Value& Vertex::getTagProp(const std::string& tagName, const std::string& prop) const {
+  for (const auto& tag : tags) {
+    if (tag.name == tagName) {
+      auto find = tag.props.find(prop);
+      if (find != tag.props.end()) {
+        return find->second;
+      }
+    }
+  }
+  return Value::kNullValue;
+}
+
 std::string Vertex::toString() const {
   std::stringstream os;
   os << "(" << vid << ")";

src/common/datatypes/Vertex.h

@@ -107,6 +107,7 @@ struct Vertex {
   bool contains(const Value& key) const;
 
   const Value& value(const std::string& key) const;
+  const Value& getTagProp(const std::string& tagName, const std::string& prop) const;
 };
 
 inline void swap(Vertex& a, Vertex& b) {

src/graph/context/iterator/SequentialIter.cpp

@@ -126,6 +126,25 @@ StatusOr<std::size_t> SequentialIter::getColumnIndex(const std::string& col) con
   return index->second;
 }
 
+const Value& SequentialIter::getTagProp(const std::string& tag, const std::string& prop) const {
+  const auto& val = this->getColumn("VERTEX");
+  if (val.isVertex()) {
+    const auto& vertex = val.getVertex();
+    return vertex.getTagProp(tag, prop);
+  }
+  return Value::kNullValue;
+}
+
+const Value& SequentialIter::getEdgeProp(const std::string& edge, const std::string& prop) const {
+  DCHECK(!edge.empty());
+  const auto& val = this->getColumn("EDGE");
+  if (val.isEdge()) {
+    const auto& e = val.getEdge();
+    return e.value(prop);
+  }
+  return Value::kNullValue;
+}
+
 Value SequentialIter::getVertex(const std::string& name) {
   return getColumn(name);
 }

src/graph/context/iterator/SequentialIter.h

@@ -78,6 +78,10 @@ class SequentialIter : public Iterator {
 
   StatusOr<std::size_t> getColumnIndex(const std::string& col) const override;
 
+  const Value& getTagProp(const std::string& tag, const std::string& prop) const override;
+
+  const Value& getEdgeProp(const std::string& edge, const std::string& prop) const override;
+
   Value getVertex(const std::string& name = "") override;
 
   Value getEdge() const override;

src/graph/executor/query/FulltextIndexScanExecutor.cpp

@@ -84,10 +84,12 @@ StatusOr<plugin::ESQueryResult> FulltextIndexScanExecutor::accessFulltextIndex(
       auto arg = tsExpr->arg();
       auto index = arg->index();
       auto query = arg->query();
-      int64_t offset = ftIndexScan->offset();
-      int64_t count = ftIndexScan->limit() > std::numeric_limits<int32_t>::max()
-                          ? std::numeric_limits<int32_t>::max()
-                          : ftIndexScan->limit();
+      int64_t offset = ftIndexScan->getValidOffset();
+      auto limit = ftIndexScan->limit();
+      if (limit > std::numeric_limits<int32_t>::max()) {
+        limit = std::numeric_limits<int32_t>::max();
+      }
+      int64_t count = limit - offset;
       execFunc = [=, &esAdapter]() { return esAdapter.queryString(index, query, offset, count); };
       break;
     }

src/graph/executor/query/GetEdgesExecutor.cpp

@@ -93,7 +93,7 @@ folly::Future<Status> GetEdgesExecutor::getEdges() {
                  ge->exprs(),
                  ge->dedup(),
                  ge->orderBy(),
-                 ge->limit(qctx()),
+                 ge->getValidLimit(),
                  ge->filter())
       .via(runner())
       .ensure([this, getPropsTime]() {

src/graph/executor/query/GetVerticesExecutor.cpp

@@ -43,7 +43,7 @@ folly::Future<Status> GetVerticesExecutor::getVertices() {
                  gv->exprs(),
                  gv->dedup(),
                  gv->orderBy(),
-                 gv->limit(qctx()),
+                 gv->getValidLimit(),
                  gv->filter())
       .via(runner())
       .ensure([this, getPropsTime]() {

src/graph/planner/ngql/LookupPlanner.cpp

@@ -65,9 +65,13 @@ StatusOr<SubPlan> LookupPlanner::transform(AstContext* astCtx) {
       auto rank = FunctionCallExpression::make(pool, "rank", {ColumnExpression::make(pool, 0)});
       auto dst = FunctionCallExpression::make(pool, "dst", {ColumnExpression::make(pool, 0)});
 
-      plan.root =
+      auto ge =
           GetEdges::make(qctx, plan.root, spaceId, src, type, rank, dst, std::move(edgeProps));
 
+      auto yieldColumns = pool->makeAndAdd<YieldColumns>();
+      yieldColumns->addColumn(new YieldColumn(EdgeExpression::make(pool)));
+      plan.root = Project::make(qctx, ge, yieldColumns);
+
       hashKeys = {ColumnExpression::make(pool, 0)};
       probeKeys = {EdgeExpression::make(pool)};
     } else {
@@ -76,9 +80,13 @@ StatusOr<SubPlan> LookupPlanner::transform(AstContext* astCtx) {
       vertexProp.props_ref() = lookupCtx->idxReturnCols;
       auto vertexProps = std::make_unique<std::vector<storage::cpp2::VertexProp>>();
       vertexProps->emplace_back(std::move(vertexProp));
-      plan.root = GetVertices::make(
+      auto gv = GetVertices::make(
           qctx, plan.root, spaceId, ColumnExpression::make(pool, 0), std::move(vertexProps));
 
+      auto yieldColumns = pool->makeAndAdd<YieldColumns>();
+      yieldColumns->addColumn(new YieldColumn(VertexExpression::make(pool)));
+      plan.root = Project::make(qctx, gv, yieldColumns);
+
       hashKeys = {ColumnExpression::make(pool, 0)};
       probeKeys = {FunctionCallExpression::make(pool, "id", {VertexExpression::make(pool)})};
     }

src/graph/planner/plan/Query.cpp

@@ -19,10 +19,18 @@ using folly::stringPrintf;
 namespace nebula {
 namespace graph {
 
+int64_t Explore::getValidLimit() const {
+  auto limit = this->limit();
+  if (limit < 0) {
+    limit = std::numeric_limits<int64_t>::max();
+  }
+  return limit;
+}
+
 int64_t Explore::limit(QueryContext* qctx) const {
   DCHECK(ExpressionUtils::isEvaluableExpr(limit_, qctx));
   QueryExpressionContext ctx(qctx ? qctx->ectx() : nullptr);
-  return DCHECK_NOTNULL(limit_)->eval(ctx).getInt();
+  return limit(ctx);
 }
 
 std::unique_ptr<PlanNodeDescription> Explore::explain() const {

src/graph/planner/plan/Query.h

@@ -22,6 +22,7 @@ namespace graph {
 //  GetVertices,
 //  GetEdges,
 //  IndexScan
+//  FulltextIndexScan
 class Explore : public SingleInputNode {
  public:
   GraphSpaceID space() const {
@@ -36,6 +37,8 @@ class Explore : public SingleInputNode {
     return dedup_;
   }
 
+  int64_t getValidLimit() const;
+
   // Get the constant limit value
   int64_t limit(QueryContext* qctx = nullptr) const;
 
@@ -441,7 +444,7 @@ class GetVertices : public Explore {
                            std::unique_ptr<std::vector<Expr>>&& exprs = nullptr,
                            bool dedup = false,
                            std::vector<storage::cpp2::OrderBy> orderBy = {},
-                           int64_t limit = std::numeric_limits<int64_t>::max(),
+                           int64_t limit = -1,
                            Expression* filter = nullptr) {
     return qctx->objPool()->makeAndAdd<GetVertices>(qctx,
                                                     Kind::kGetVertices,
@@ -547,7 +550,7 @@ class GetEdges final : public Explore {
                         std::unique_ptr<std::vector<EdgeProp>>&& props = nullptr,
                         std::unique_ptr<std::vector<Expr>>&& exprs = nullptr,
                         bool dedup = false,
-                        int64_t limit = std::numeric_limits<int64_t>::max(),
+                        int64_t limit = -1,
                         std::vector<storage::cpp2::OrderBy> orderBy = {},
                         Expression* filter = nullptr) {
     return qctx->objPool()->makeAndAdd<GetEdges>(qctx,
@@ -767,6 +770,10 @@ class FulltextIndexScan : public Explore {
     return isEdge_;
   }
 
+  int64_t getValidOffset() const {
+    return offset_ >= 0 ? offset_ : 0;
+  }
+
   int64_t offset() const {
     return offset_;
   }

src/graph/util/ExpressionUtils.cpp

@@ -190,6 +190,13 @@ Expression *ExpressionUtils::rewriteEdgePropFunc2LabelAttribute(
   return RewriteVisitor::transform(expr, std::move(matcher), std::move(rewriter));
 }
 
+Expression *ExpressionUtils::rewriteLabelAttr2PropExpr(const Expression *expr, bool isEdge) {
+  if (isEdge) {
+    return rewriteLabelAttr2EdgeProp(expr);
+  }
+  return rewriteLabelAttr2TagProp(expr);
+}
+
 Expression *ExpressionUtils::rewriteLabelAttr2TagProp(const Expression *expr) {
   ObjectPool *pool = expr->getObjPool();
   auto matcher = [](const Expression *e) -> bool {

src/graph/util/ExpressionUtils.h

@@ -71,6 +71,9 @@ class ExpressionUtils {
   static Expression* rewriteEdgePropFunc2LabelAttribute(
       const Expression* expr, const std::unordered_map<std::string, AliasType>& aliasTypeMap);
 
+  // rewrite LabelAttr expr to property expr
+  static Expression* rewriteLabelAttr2PropExpr(const Expression* expr, bool isEdge);
+
   // rewrite LabelAttr to tagProp
   static Expression* rewriteLabelAttr2TagProp(const Expression* expr);
 

src/graph/validator/LookupValidator.cpp

@@ -573,7 +573,7 @@ Status LookupValidator::validateYieldColumn(YieldColumn* col, bool isEdge) {
     return Status::SemanticError("illegal yield clauses `%s'", col->toString().c_str());
   }
 
-  bool scoreCol = false;
+  bool isScoreCol = false;
   switch (col->expr()->kind()) {
     case Expression::Kind::kLabelAttribute: {
       auto expr = static_cast<LabelAttributeExpression*>(col->expr());
@@ -589,7 +589,8 @@ Status LookupValidator::validateYieldColumn(YieldColumn* col, bool isEdge) {
         if (col->alias().empty()) {
           return Status::SemanticError("Yield column should have an alias for score()");
         }
-        scoreCol = true;
+        isScoreCol = true;
+        lookupCtx_->hasScore = true;
       }
       break;
     }
@@ -598,32 +599,24 @@ Status LookupValidator::validateYieldColumn(YieldColumn* col, bool isEdge) {
   }
 
   Expression* colExpr = nullptr;
-  if (scoreCol) {
+  if (isScoreCol) {
     // Rewrite score() to $score
     colExpr = VariablePropertyExpression::make(qctx_->objPool(), "", kScore);
-    col->setExpr(colExpr);
-    outputs_.emplace_back(col->name(), Value::Type::FLOAT);
-    lookupCtx_->yieldExpr->addColumn(col->clone().release());
-    lookupCtx_->hasScore = true;
   } else {
-    if (isEdge) {
-      colExpr = ExpressionUtils::rewriteLabelAttr2EdgeProp(col->expr());
-    } else {
-      colExpr = ExpressionUtils::rewriteLabelAttr2TagProp(col->expr());
-    }
-
-    col->setExpr(colExpr);
-    NG_RETURN_IF_ERROR(ValidateUtil::invalidLabelIdentifiers(colExpr));
-
-    auto typeStatus = deduceExprType(colExpr);
-    NG_RETURN_IF_ERROR(typeStatus);
-    outputs_.emplace_back(col->name(), typeStatus.value());
-    lookupCtx_->yieldExpr->addColumn(col->clone().release());
-    if (isEdge) {
-      NG_RETURN_IF_ERROR(deduceProps(colExpr, exprProps_, nullptr, &schemaIds_));
-    } else {
-      NG_RETURN_IF_ERROR(deduceProps(colExpr, exprProps_, &schemaIds_));
-    }
+    colExpr = ExpressionUtils::rewriteLabelAttr2PropExpr(col->expr(), isEdge);
+  }
+  col->setExpr(colExpr);
+  NG_RETURN_IF_ERROR(ValidateUtil::invalidLabelIdentifiers(colExpr));
+
+  auto typeStatus = deduceExprType(colExpr);
+  NG_RETURN_IF_ERROR(typeStatus);
+  auto type = isScoreCol ? Value::Type::FLOAT : typeStatus.value();
+  outputs_.emplace_back(col->name(), type);
+  lookupCtx_->yieldExpr->addColumn(col->clone().release());
+  if (isEdge) {
+    NG_RETURN_IF_ERROR(deduceProps(colExpr, exprProps_, nullptr, &schemaIds_));
+  } else {
+    NG_RETURN_IF_ERROR(deduceProps(colExpr, exprProps_, &schemaIds_));
   }
 
   return Status::OK();

src/graph/visitor/DeducePropsVisitor.cpp

@@ -153,8 +153,13 @@ void DeducePropsVisitor::visit(InputPropertyExpression *expr) {
 }
 
 void DeducePropsVisitor::visit(VariablePropertyExpression *expr) {
-  exprProps_->insertVarProp(expr->sym(), expr->prop());
-  userDefinedVarNameList_->emplace(expr->sym());
+  auto sym = expr->sym();
+  // If the variable is not defined, it is an expression which get the expr->prop() column
+  // and skip it
+  if (!sym.empty()) {
+    exprProps_->insertVarProp(sym, expr->prop());
+    userDefinedVarNameList_->emplace(sym);
+  }
 }
 
 void DeducePropsVisitor::visit(DestPropertyExpression *expr) {

src/graph/visitor/DeduceTypeVisitor.cpp

@@ -575,6 +575,10 @@ void DeduceTypeVisitor::visit(InputPropertyExpression *expr) {
 
 void DeduceTypeVisitor::visit(VariablePropertyExpression *expr) {
   const auto &var = expr->sym();
+  if (var.empty()) {
+    // Could not deduce the actual type of the property and do nothing
+    return;
+  }
   if (!vctx_->existVar(var) && !qctx_->existParameter(var)) {
     status_ = Status::SemanticError(
         "`%s', not exist variable `%s'", expr->toString().c_str(), var.c_str());