Revert "Remove all UNKNOWN_PROP as a type of null. (#4907)" (#5149)

This reverts commit aa62416.

Co-authored-by: Sophie <[email protected]>

src/codec/RowReaderV1.cpp

@@ -199,11 +199,11 @@ Value RowReaderV1::getValueByName(const std::string& prop) const noexcept {
 
 Value RowReaderV1::getValueByIndex(const int64_t index) const noexcept {
   if (index < 0 || static_cast<size_t>(index) >= schema_->getNumFields()) {
-    return Value(NullType::__NULL__);
+    return Value(NullType::UNKNOWN_PROP);
   }
   auto vType = getSchema()->getFieldType(index);
   if (PropertyType::UNKNOWN == vType) {
-    return Value(NullType::__NULL__);
+    return Value(NullType::UNKNOWN_PROP);
   }
   switch (vType) {
     case PropertyType::BOOL:

src/codec/RowReaderV2.cpp

@@ -52,7 +52,7 @@ Value RowReaderV2::getValueByName(const std::string& prop) const noexcept {
 
 Value RowReaderV2::getValueByIndex(const int64_t index) const noexcept {
   if (index < 0 || static_cast<size_t>(index) >= schema_->getNumFields()) {
-    return Value(NullType::__NULL__);
+    return Value(NullType::UNKNOWN_PROP);
   }
 
   auto field = schema_->field(index);

src/common/datatypes/Map.h

@@ -69,7 +69,7 @@ struct Map {
   const Value& at(const std::string& key) const {
     auto iter = kvs.find(key);
     if (iter == kvs.end()) {
-      return Value::kNullValue;
+      return Value::kNullUnknownProp;
     }
     return iter->second;
   }

src/common/datatypes/Value.cpp

@@ -29,6 +29,7 @@ const Value Value::kNullNaN(NullType::NaN);
 const Value Value::kNullBadData(NullType::BAD_DATA);
 const Value Value::kNullBadType(NullType::BAD_TYPE);
 const Value Value::kNullOverflow(NullType::ERR_OVERFLOW);
+const Value Value::kNullUnknownProp(NullType::UNKNOWN_PROP);
 const Value Value::kNullDivByZero(NullType::DIV_BY_ZERO);
 const Value Value::kNullOutOfRange(NullType::OUT_OF_RANGE);
 
@@ -319,6 +320,7 @@ const std::string& Value::typeName() const {
       {NullType::BAD_DATA, "BAD_DATA"},
       {NullType::BAD_TYPE, "BAD_TYPE"},
       {NullType::ERR_OVERFLOW, "ERR_OVERFLOW"},
+      {NullType::UNKNOWN_PROP, "UNKNOWN_PROP"},
       {NullType::DIV_BY_ZERO, "DIV_BY_ZERO"},
   };
 
@@ -1574,6 +1576,8 @@ std::string Value::toString() const {
           return "__NULL_OVERFLOW__";
         case NullType::NaN:
           return "__NULL_NaN__";
+        case NullType::UNKNOWN_PROP:
+          return "__NULL_UNKNOWN_PROP__";
         case NullType::OUT_OF_RANGE:
           return "__NULL_OUT_OF_RANGE__";
       }

src/common/datatypes/Value.h

@@ -40,6 +40,7 @@ enum class NullType {
   BAD_DATA = 2,
   BAD_TYPE = 3,
   ERR_OVERFLOW = 4,
+  UNKNOWN_PROP = 5,
   DIV_BY_ZERO = 6,
   OUT_OF_RANGE = 7,
 };
@@ -51,6 +52,7 @@ struct Value {
   static const Value kNullBadData;
   static const Value kNullBadType;
   static const Value kNullOverflow;
+  static const Value kNullUnknownProp;
   static const Value kNullDivByZero;
   static const Value kNullOutOfRange;
 
@@ -155,8 +157,8 @@ struct Value {
     }
     auto& null = value_.nVal;
     return null == NullType::NaN || null == NullType::BAD_DATA || null == NullType::BAD_TYPE ||
-           null == NullType::ERR_OVERFLOW || null == NullType::DIV_BY_ZERO ||
-           null == NullType::OUT_OF_RANGE;
+           null == NullType::ERR_OVERFLOW || null == NullType::UNKNOWN_PROP ||
+           null == NullType::DIV_BY_ZERO || null == NullType::OUT_OF_RANGE;
   }
   bool isNumeric() const {
     return type_ == Type::INT || type_ == Type::FLOAT;

src/common/datatypes/test/ValueTest.cpp

@@ -1565,6 +1565,7 @@ TEST(Value, typeName) {
   EXPECT_EQ("BAD_DATA", Value::kNullBadData.typeName());
   EXPECT_EQ("BAD_TYPE", Value::kNullBadType.typeName());
   EXPECT_EQ("ERR_OVERFLOW", Value::kNullOverflow.typeName());
+  EXPECT_EQ("UNKNOWN_PROP", Value::kNullUnknownProp.typeName());
   EXPECT_EQ("DIV_BY_ZERO", Value::kNullDivByZero.typeName());
 }
 
@@ -1581,6 +1582,7 @@ TEST(Value, DecodeEncode) {
       Value(NullType::BAD_DATA),
       Value(NullType::ERR_OVERFLOW),
       Value(NullType::OUT_OF_RANGE),
+      Value(NullType::UNKNOWN_PROP),
 
       // int
       Value(0),

src/common/datatypes/test/ValueToJsonTest.cpp

@@ -225,7 +225,7 @@ TEST(ValueToJson, DecodeEncode) {
       Value(NullType::BAD_DATA),
       Value(NullType::ERR_OVERFLOW),
       Value(NullType::OUT_OF_RANGE),
-      Value(NullType::__NULL__),
+      Value(NullType::UNKNOWN_PROP),
 
       // int
       Value(0),

src/common/expression/AttributeExpression.cpp

@@ -67,7 +67,7 @@ const Value &AttributeExpression::eval(ExpressionContext &ctx) {
           }
         }
       }
-      return Value::kNullValue;
+      return Value::kNullUnknownProp;
     }
     case Value::Type::EDGE: {
       DCHECK(!rvalue.getStr().empty());

src/common/expression/test/AttributeExpressionTest.cpp

@@ -154,7 +154,7 @@ TEST_F(AttributeExpressionTest, DateTimeAttribute) {
     auto *right = LabelExpression::make(&pool, "not exist attribute");
     auto expr = AttributeExpression::make(&pool, left, right);
     auto value = Expression::eval(expr, gExpCtxt);
-    ASSERT_EQ(Value::kNullValue, value);
+    ASSERT_EQ(Value::kNullUnknownProp, value);
   }
   {
     auto *left = ConstantExpression::make(&pool, Value(dt));
@@ -168,7 +168,7 @@ TEST_F(AttributeExpressionTest, DateTimeAttribute) {
     auto *right = LabelExpression::make(&pool, "not exist attribute");
     auto expr = AttributeExpression::make(&pool, left, right);
     auto value = Expression::eval(expr, gExpCtxt);
-    ASSERT_EQ(Value::kNullValue, value);
+    ASSERT_EQ(Value::kNullUnknownProp, value);
   }
   {
     auto *left = ConstantExpression::make(&pool, Value(d));
@@ -182,7 +182,7 @@ TEST_F(AttributeExpressionTest, DateTimeAttribute) {
     auto *right = LabelExpression::make(&pool, "not exist attribute");
     auto expr = AttributeExpression::make(&pool, left, right);
     auto value = Expression::eval(expr, gExpCtxt);
-    ASSERT_EQ(Value::kNullValue, value);
+    ASSERT_EQ(Value::kNullUnknownProp, value);
   }
   {
     auto *left = ConstantExpression::make(&pool, Value(t));

src/common/expression/test/SubscriptExpressionTest.cpp

@@ -338,6 +338,7 @@ TEST_F(SubscriptExpressionTest, MapSubscript) {
     auto expr = SubscriptExpression::make(&pool, map, key);
     auto value = Expression::eval(expr, gExpCtxt);
     ASSERT_TRUE(value.isNull());
+    ASSERT_TRUE(value.isBadNull());
   }
   // {"key1":1,"key2":2, "key3":3}[0]
   {

src/common/time/TimeUtils.h

@@ -124,7 +124,7 @@ class TimeUtils {
     } else if (lowerProp == "microsecond") {
       return static_cast<int>(dt.microsec);
     } else {
-      return Value::kNullValue;
+      return Value::kNullUnknownProp;
     }
   }
 
@@ -160,7 +160,7 @@ class TimeUtils {
     } else if (lowerProp == "day") {
       return d.day;
     } else {
-      return Value::kNullValue;
+      return Value::kNullUnknownProp;
     }
   }
 
@@ -203,7 +203,7 @@ class TimeUtils {
     } else if (lowerProp == "microsecond") {
       return t.microsec;
     } else {
-      return Value::kNullValue;
+      return Value::kNullUnknownProp;
     }
   }
 

src/common/utils/IndexKeyUtils.cpp

@@ -204,7 +204,7 @@ StatusOr<Value> IndexKeyUtils::readValueWithLatestSche(RowReader* reader,
                                                        const std::string propName,
                                                        const meta::SchemaProviderIf* latestSchema) {
   auto value = reader->getValueByName(propName);
-  if (latestSchema == nullptr || !value.isNull() || value.getNull() != NullType::__NULL__) {
+  if (latestSchema == nullptr || !value.isNull() || value.getNull() != NullType::UNKNOWN_PROP) {
     return value;
   }
   auto field = latestSchema->field(propName);
@@ -230,6 +230,9 @@ Status IndexKeyUtils::checkValue(const Value& v, bool isNullable) {
   }
 
   switch (v.getNull()) {
+    case nebula::NullType::UNKNOWN_PROP: {
+      return Status::Error("Unknown prop");
+    }
     case nebula::NullType::__NULL__: {
       if (!isNullable) {
         return Status::Error("Not allowed to be null");

src/interface/common.thrift

@@ -95,7 +95,7 @@ enum NullType {
     BAD_DATA = 2,
     BAD_TYPE = 3,
     ERR_OVERFLOW = 4,
-    UNKNOWN_PROP = 5,  // deprecated but not to be deleted
+    UNKNOWN_PROP = 5,
     DIV_BY_ZERO = 6,
     OUT_OF_RANGE = 7,
 } (cpp.enum_strict, cpp.type = "nebula::NullType")

src/storage/exec/IndexEdgeScanNode.cpp

@@ -122,7 +122,7 @@ Map<std::string, Value> IndexEdgeScanNode::decodeFromBase(const std::string& key
       case QueryUtils::ReturnColType::kOther: {
         auto field = edge_.back()->field(col);
         if (field == nullptr) {
-          values[col] = Value::kNullValue;
+          values[col] = Value::kNullUnknownProp;
         } else {
           auto retVal = QueryUtils::readValue(reader.get(), col, field);
           if (!retVal.ok()) {

src/storage/exec/IndexVertexScanNode.cpp

@@ -99,7 +99,7 @@ Map<std::string, Value> IndexVertexScanNode::decodeFromBase(const std::string& k
       case QueryUtils::ReturnColType::kOther: {
         auto field = tag_.back()->field(col);
         if (field == nullptr) {
-          values[col] = Value::kNullValue;
+          values[col] = Value::kNullUnknownProp;
         } else {
           auto retVal = QueryUtils::readValue(reader.get(), col, field);
           if (!retVal.ok()) {

src/storage/exec/QueryUtils.h

@@ -80,7 +80,7 @@ class QueryUtils final {
       // read null value
       auto nullType = value.getNull();
 
-      if (nullType == NullType::__NULL__) {
+      if (nullType == NullType::UNKNOWN_PROP) {
         VLOG(1) << "Fail to read prop " << propName;
         if (!field) {
           return value;

src/tools/db-upgrade/DbUpgrader.cpp

@@ -867,7 +867,7 @@ std::string UpgraderSpace::encodeRowVal(const RowReader* reader,
           LOG(ERROR) << "Write rowWriterV2 failed";
           return "";
         }
-      } else if (nullType != NullType::__NULL__) {
+      } else if (nullType != NullType::UNKNOWN_PROP) {
         // nullType == NullType::kNullUnknownProp, indicates that the field is
         // only in the latest schema, maybe use default value or null value.
         LOG(ERROR) << "Data is illegal in " << name << " field";

tests/common/nebula_test_suite.py

@@ -34,8 +34,8 @@
 T_NULL_BAD_DATA.set_nVal(CommonTtypes.NullType.BAD_DATA)
 T_NULL_BAD_TYPE = CommonTtypes.Value()
 T_NULL_BAD_TYPE.set_nVal(CommonTtypes.NullType.BAD_TYPE)
-T_NULL___NULL__ = CommonTtypes.Value()
-T_NULL___NULL__.set_nVal(CommonTtypes.NullType.__NULL__)
+T_NULL_UNKNOWN_PROP = CommonTtypes.Value()
+T_NULL_UNKNOWN_PROP.set_nVal(CommonTtypes.NullType.UNKNOWN_PROP)
 T_NULL_UNKNOWN_DIV_BY_ZERO = CommonTtypes.Value()
 T_NULL_UNKNOWN_DIV_BY_ZERO.set_nVal(CommonTtypes.NullType.DIV_BY_ZERO)
 

tests/tck/features/expression/Attribute.feature

@@ -61,8 +61,8 @@ Feature: Attribute
       RETURN {k1 : 1, k2: true}.K1 AS k
       """
     Then the result should be, in any order:
-      | k        |
-      | __NULL__ |
+      | k            |
+      | UNKNOWN_PROP |
     When executing query:
       """
       MATCH (v) WHERE id(v) == 'Tim Duncan' RETURN v.player.name
@@ -101,28 +101,28 @@ Feature: Attribute
       """
     Then the result should be, in any order:
       | not_exists_attr |
-      | __NULL__        |
+      | UNKNOWN_PROP    |
     When executing query:
       """
       RETURN time("02:59:40").not_exists_attr AS not_exists_attr
       """
     Then the result should be, in any order:
       | not_exists_attr |
-      | __NULL__        |
+      | UNKNOWN_PROP    |
     When executing query:
       """
       RETURN datetime("2021-07-19T02:59:40").not_exists_attr AS not_exists_attr
       """
     Then the result should be, in any order:
       | not_exists_attr |
-      | __NULL__        |
+      | UNKNOWN_PROP    |
     When executing query:
       """
       RETURN {k1 : 1, k2: true}.not_exists_attr AS not_exists_attr
       """
     Then the result should be, in any order:
       | not_exists_attr |
-      | __NULL__        |
+      | UNKNOWN_PROP    |
     When executing query:
       """
       MATCH (v) WHERE id(v) == 'Tim Duncan' RETURN v.player.not_exists_attr

tests/tck/features/match/Base.feature

@@ -355,6 +355,7 @@ Feature: Basic match
       """
     Then a SemanticError should be raised at runtime: `like_not_exists': Unknown edge type
 
+  @hello
   Scenario: two steps
     When executing query:
       """
@@ -375,10 +376,10 @@ Feature: Basic match
       """
     Then the result should be, in any order:
       | Player           | Friend              | FoF            | NotExists |
-      | "Damian Lillard" | "LaMarcus Aldridge" | "Tim Duncan"   | __NULL__  |
-      | "Damian Lillard" | "LaMarcus Aldridge" | "Tony Parker"  | __NULL__  |
-      | "Paul George"    | "Russell Westbrook" | "James Harden" | __NULL__  |
-      | "Paul George"    | "Russell Westbrook" | "Paul George"  | __NULL__  |
+      | "Damian Lillard" | "LaMarcus Aldridge" | "Tim Duncan"   | NULL      |
+      | "Damian Lillard" | "LaMarcus Aldridge" | "Tony Parker"  | NULL      |
+      | "Paul George"    | "Russell Westbrook" | "James Harden" | NULL      |
+      | "Paul George"    | "Russell Westbrook" | "Paul George"  | NULL      |
     When executing query:
       """
       MATCH (v1) -[:like]-> (v2:player{age: 28}) -[:like]-> (v3)
@@ -435,8 +436,8 @@ Feature: Basic match
       """
     Then the result should be, in any order:
       | Player       | Friend          | TYPE    | FoF          | FoT       |
-      | "Paul Gasol" | "Marc Gasol"    | "like"  | "Paul Gasol" | __NULL__  |
-      | "Yao Ming"   | "Tracy McGrady" | "serve" | __NULL__     | "Rockets" |
+      | "Paul Gasol" | "Marc Gasol"    | "like"  | "Paul Gasol" | NULL      |
+      | "Yao Ming"   | "Tracy McGrady" | "serve" | NULL         | "Rockets" |
     When executing query:
       """
       MATCH (v1) -[e1:like]-> (v2) -[e2]-> (v3)

tests/tck/features/match/MultiQueryParts.feature

@@ -182,8 +182,8 @@ Feature: Multi Query Parts
     Then the result should be, in any order, with relax comparison:
       | v1              | v2                | e3                                            | v4                    | v3             | e5                                             |
       | ("Tony Parker") | ("Tony Parker")   | [:like "Tony Parker"->"LaMarcus Aldridge" @0] | ("LaMarcus Aldridge") | ("Tim Duncan") | [:like "LaMarcus Aldridge"->"Tony Parker" @0]  |
-      | ("Tony Parker") | ("Tim Duncan")    | [:like "Tim Duncan"->"Tony Parker" @0 ]       | ("Tony Parker")       | __NULL__       | __NULL__                                       |
-      | ("Tony Parker") | ("Tim Duncan")    | [:like "Tim Duncan"->"Tony Parker" @0]        | ("Tony Parker")       | __NULL__       | __NULL__                                       |
+      | ("Tony Parker") | ("Tim Duncan")    | [:like "Tim Duncan"->"Tony Parker" @0 ]       | ("Tony Parker")       | NULL           | NULL                                           |
+      | ("Tony Parker") | ("Tim Duncan")    | [:like "Tim Duncan"->"Tony Parker" @0]        | ("Tony Parker")       | NULL           | NULL                                           |
       | ("Tony Parker") | ("Tony Parker")   | [:like "Tony Parker"->"Tim Duncan" @0 ]       | ("Tim Duncan")        | ("Tim Duncan") | [:teammate "Tim Duncan"->"Tony Parker" @0]     |
       | ("Tony Parker") | ("Manu Ginobili") | [:like "Manu Ginobili"->"Tim Duncan" @0 ]     | ("Tim Duncan")        | ("Tim Duncan") | [:teammate "Tim Duncan"->"Tony Parker" @0]     |
       | ("Tony Parker") | ("Tony Parker")   | [:like "Tony Parker"->"Manu Ginobili" @0]     | ("Manu Ginobili")     | ("Tim Duncan") | [:teammate "Manu Ginobili"->"Tony Parker" @0]  |

tests/tck/features/match/With.feature

@@ -94,8 +94,8 @@ Feature: With clause
       RETURN x.c
       """
     Then the result should be, in any order:
-      | x.c      |
-      | __NULL__ |
+      | x.c          |
+      | UNKNOWN_PROP |
 
   Scenario: match with return
     When executing query: