check vidType

vesoft-inc · Jul 23, 2022 · 9b53fbc · 9b53fbc
1 parent f8dd6b7
commit 9b53fbc
Show file tree

Hide file tree

Showing 8 changed files with 35 additions and 16 deletions.
diff --git a/src/graph/executor/StorageAccessExecutor.cpp b/src/graph/executor/StorageAccessExecutor.cpp
@@ -41,7 +41,8 @@ DataSet buildRequestDataSet(const SpaceInfo &space,
                             QueryExpressionContext &exprCtx,
                             Iterator *iter,
                             Expression *expr,
-                            bool dedup) {
+                            bool dedup,
+                            bool isCyper) {
   DCHECK(iter && expr) << "iter=" << iter << ", expr=" << expr;
   nebula::DataSet vertices({kVid});
   auto s = iter->size();
@@ -54,7 +55,13 @@ DataSet buildRequestDataSet(const SpaceInfo &space,
 
   for (; iter->valid(); iter->next()) {
     auto vid = expr->eval(exprCtx(iter));
+    if (vid.empty()) {
+      continue;
+    }
     if (!SchemaUtil::isValidVid(vid, vidType)) {
+      if (isCypher) {
+        continue;
+      }
       LOG(WARNING) << "Mismatched vid type: " << vid.type()
                    << ", space vid type: " << SchemaUtil::typeToString(vidType);
       continue;
@@ -80,9 +87,9 @@ DataSet StorageAccessExecutor::buildRequestDataSetByVidType(Iterator *iter,
   QueryExpressionContext exprCtx(qctx()->ectx());
 
   if (isIntVidType(space)) {
-    return internal::buildRequestDataSet<int64_t>(space, exprCtx, iter, expr, dedup);
+    return internal::buildRequestDataSet<int64_t>(space, exprCtx, iter, expr, dedup, isCypher);
   }
-  return internal::buildRequestDataSet<std::string>(space, exprCtx, iter, expr, dedup);
+  return internal::buildRequestDataSet<std::string>(space, exprCtx, iter, expr, dedup, isCypher);
 }
 
 std::string StorageAccessExecutor::getStorageDetail(

diff --git a/src/graph/executor/StorageAccessExecutor.h b/src/graph/executor/StorageAccessExecutor.h
@@ -158,7 +158,10 @@ class StorageAccessExecutor : public Executor {
 
   bool isIntVidType(const SpaceInfo &space) const;
 
-  DataSet buildRequestDataSetByVidType(Iterator *iter, Expression *expr, bool dedup);
+  DataSet buildRequestDataSetByVidType(Iterator *iter,
+                                       Expression *expr,
+                                       bool dedup,
+                                       bool isCypher = false);
 };
 
 }  // namespace graph

diff --git a/src/graph/executor/query/AppendVerticesExecutor.cpp b/src/graph/executor/query/AppendVerticesExecutor.cpp
@@ -21,7 +21,7 @@ DataSet AppendVerticesExecutor::buildRequestDataSet(const AppendVertices *av) {
     return nebula::DataSet({kVid});
   }
   auto valueIter = ectx_->getResult(av->inputVar()).iter();
-  return buildRequestDataSetByVidType(valueIter.get(), av->src(), av->dedup());
+  return buildRequestDataSetByVidType(valueIter.get(), av->src(), av->dedup(), true);
 }
 
 folly::Future<Status> AppendVerticesExecutor::appendVertices() {

diff --git a/src/graph/executor/query/GetEdgesExecutor.cpp b/src/graph/executor/query/GetEdgesExecutor.cpp
@@ -5,6 +5,7 @@
 #include "graph/executor/query/GetEdgesExecutor.h"
 
 #include "graph/planner/plan/Query.h"
+#include "graph/util/SchemaUtil.h"
 
 using nebula::storage::StorageClient;
 using nebula::storage::StorageRpcResponse;
@@ -53,6 +54,7 @@ folly::Future<Status> GetEdgesExecutor::getEdges() {
   }
 
   auto edges = buildRequestDataSet(ge);
+  NG_RETURN_IF_ERROR(res);
 
   if (edges.rows.empty()) {
     // TODO: add test for empty input.

diff --git a/src/graph/validator/FetchEdgesValidator.cpp b/src/graph/validator/FetchEdgesValidator.cpp
@@ -42,8 +42,7 @@ Status FetchEdgesValidator::validateEdgeName() {
 
 // Check validity of edge key(src, type, rank, dst)
 // from Input/Variable expression specified in sentence
-StatusOr<std::string> FetchEdgesValidator::validateEdgeRef(const Expression *expr,
-                                                           Value::Type type) {
+StatusOr<std::string> FetchEdgesValidator::validateEdgeRef(const Expression *expr) {
   const auto &kind = expr->kind();
   if (kind != Expression::Kind::kInputProperty && kind != EdgeExpression::Kind::kVarProperty) {
     return Status::SemanticError("`%s', only input and variable expression is acceptable",
@@ -72,13 +71,22 @@ Status FetchEdgesValidator::validateEdgeKey() {
   std::string inputVarName;
   if (sentence->isRef()) {  // edge keys from Input/Variable
     auto *srcExpr = sentence->ref()->srcid();
-    auto result = validateEdgeRef(srcExpr, vidType_);
+    auto result = validateEdgeRef(srcExpr);
     NG_RETURN_IF_ERROR(result);
     inputVarName = std::move(result).value();
 
     auto *rankExpr = sentence->ref()->rank();
     if (rankExpr->kind() != Expression::Kind::kConstant) {
-      result = validateEdgeRef(rankExpr, Value::Type::INT);
+      auto rankType = deduceExprType(rankExpr);
+      NG_RETURN_IF_ERROR(rankType);
+      if (rankType.value() != Value::Type::INT) {
+        std::stringstream ss;
+        ss << "`" << rankExpr->toString() << "' should be type of INT, but was "
+           << rankType.value();
+        return Status::SemanticError(ss.str());
+      }
+
+      result = validateEdgeRef(rankExpr);
       NG_RETURN_IF_ERROR(result);
       if (inputVarName != result.value()) {
         return Status::SemanticError(
@@ -88,7 +96,7 @@ Status FetchEdgesValidator::validateEdgeKey() {
     }
 
     auto *dstExpr = sentence->ref()->dstid();
-    result = validateEdgeRef(dstExpr, vidType_);
+    result = validateEdgeRef(dstExpr);
     NG_RETURN_IF_ERROR(result);
     if (inputVarName != result.value()) {
       return Status::SemanticError(

diff --git a/src/graph/validator/FetchEdgesValidator.h b/src/graph/validator/FetchEdgesValidator.h
@@ -22,7 +22,7 @@ class FetchEdgesValidator final : public Validator {
 
   Status validateEdgeName();
 
-  StatusOr<std::string> validateEdgeRef(const Expression* expr, Value::Type type);
+  StatusOr<std::string> validateEdgeRef(const Expression* expr);
 
   Status validateEdgeKey();
 

diff --git a/src/graph/validator/Validator.cpp b/src/graph/validator/Validator.cpp
@@ -457,7 +457,8 @@ Status Validator::validateStarts(const VerticesClause* clause, Starts& starts) {
       return type.status();
     }
     auto vidType = space_.spaceDesc.vid_type_ref().value().get_type();
-    if (type.value() != SchemaUtil::propTypeToValueType(vidType)) {
+    if (type != Value::Type::__EMPTY__ &&
+        type.value() != SchemaUtil::propTypeToValueType(vidType)) {
       std::stringstream ss;
       ss << "`" << src->toString() << "', the srcs should be type of "
          << apache::thrift::util::enumNameSafe(vidType) << ", but was`" << type.value() << "'";

diff --git a/src/graph/validator/test/GetSubgraphValidatorTest.cpp b/src/graph/validator/test/GetSubgraphValidatorTest.cpp
@@ -218,17 +218,15 @@ TEST_F(GetSubgraphValidatorTest, RefNotExist) {
         "PROP FROM $-.id YIELD vertices as nodes";
     auto result = checkResult(query);
     EXPECT_EQ(std::string(result.message()),
-              "SemanticError: `$-.id', the srcs should be type of "
-              "FIXED_STRING, but was`INT'");
+              "SemanticError: `$-.id', the srcs should be type of FIXED_STRING, but was`INT'");
   }
   {
     std::string query =
         "$a = GO FROM \"1\" OVER like YIELD $$.person.age AS ID; GET SUBGRAPH "
         "FROM $a.ID YIELD edges as relationships";
     auto result = checkResult(query);
     EXPECT_EQ(std::string(result.message()),
-              "SemanticError: `$a.ID', the srcs should be type of "
-              "FIXED_STRING, but was`INT'");
+              "SemanticError: `$a.ID', the srcs should be type of FIXED_STRING, but was`INT'");
   }
   {
     std::string query =