Revert "Remove all UNKNOWN_PROP as a type of null. (vesoft-inc#4907)"

This reverts commit aa62416.
xtcyclist · Dec 29, 2022 · a4c09c2 · a4c09c2
1 parent 373a847
commit a4c09c2
Show file tree

Hide file tree

Showing 23 changed files with 74 additions and 62 deletions.
diff --git a/src/codec/RowReaderV1.cpp b/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:

diff --git a/src/codec/RowReaderV2.cpp b/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);

diff --git a/src/common/datatypes/Map.h b/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;
   }

diff --git a/src/common/datatypes/Value.cpp b/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__";
       }

diff --git a/src/common/datatypes/Value.h b/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;

diff --git a/src/common/datatypes/test/ValueTest.cpp b/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),

diff --git a/src/common/datatypes/test/ValueToJsonTest.cpp b/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),

diff --git a/src/common/expression/AttributeExpression.cpp b/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());

diff --git a/src/common/expression/test/AttributeExpressionTest.cpp b/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));

diff --git a/src/common/expression/test/SubscriptExpressionTest.cpp b/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]
   {

diff --git a/src/common/time/TimeUtils.h b/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;
     }
   }
 

diff --git a/src/common/utils/IndexKeyUtils.cpp b/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");

diff --git a/src/interface/common.thrift b/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")

diff --git a/src/storage/exec/IndexEdgeScanNode.cpp b/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()) {

diff --git a/src/storage/exec/IndexVertexScanNode.cpp b/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()) {

diff --git a/src/storage/exec/QueryUtils.h b/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;

diff --git a/src/tools/db-upgrade/DbUpgrader.cpp b/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";

diff --git a/tests/common/nebula_test_suite.py b/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)
 

diff --git a/tests/tck/features/expression/Attribute.feature b/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

diff --git a/tests/tck/features/match/With.feature b/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:

diff --git a/tests/tck/features/optimizer/PrunePropertiesRule.feature b/tests/tck/features/optimizer/PrunePropertiesRule.feature
@@ -756,35 +756,35 @@ Feature: Prune Properties rule
       """
     Then the result should be, in order, with relax comparison:
       | properties(src_v).age | properties(e).degree | name              | src_v.player.sex | e.start_year | dst_v.player.age |
-      | 41                    | __NULL__             | "Dejounte Murray" | "男"             | 2022         | 29               |
+      | 41                    | UNKNOWN_PROP         | "Dejounte Murray" | "男"             | 2022         | 29               |
       | 41                    | 88                   | "Spurs"           | "男"             | 2002         | NULL             |
-      | 41                    | __NULL__             | "Tiago Splitter"  | "男"             | 2022         | 34               |
+      | 41                    | UNKNOWN_PROP         | "Tiago Splitter"  | "男"             | 2022         | 34               |
     When executing query:
       """
       match (src_v:player{name:"Manu Ginobili"})-[e*2]-(dst_v)
         return properties(src_v).sex,properties(e[0]).degree as degree,properties(dst_v).name as name,src_v.player.age AS age, e[1].start_year,dst_v.player.age
         order by degree, name, age limit 5;
       """
     Then the result should be, in order, with relax comparison:
-      | properties(src_v).sex | degree | name          | age | e[1].start_year | dst_v.player.age |
-      | "男"                  | 88     | "Aron Baynes" | 41  | 2013            | 32               |
-      | "男"                  | 88     | "Boris Diaw"  | 41  | 2012            | 36               |
-      | "男"                  | 88     | "Cory Joseph" | 41  | 2011            | 27               |
-      | "男"                  | 88     | "Danny Green" | 41  | 2010            | 31               |
-      | "男"                  | 88     | "David West"  | 41  | 2015            | 38               |
+      | properties(src_v).sex | properties(e[0]).degree | properties(dst_v).name | age | e[1].start_year | dst_v.player.age |
+      | "男"                  | 88                      | "Danny Green"          | 41  | 2010            | 31               |
+      | "男"                  | UNKNOWN_PROP            | "Danny Green"          | 41  | 2022            | 31               |
+      | "男"                  | UNKNOWN_PROP            | "LeBron James"         | 41  | 2022            | 34               |
+      | "男"                  | 88                      | "Cory Joseph"          | 41  | 2011            | 27               |
+      | "男"                  | UNKNOWN_PROP            | "76ers"                | 41  | 2017            | NULL             |
     When executing query:
       """
       match (src_v:player{name:"Manu Ginobili"})-[e:like*2..3]-(dst_v)
         return properties(src_v).sex,properties(e[0]).degree as degree,properties(dst_v).name as name,src_v.player.age AS age, e[1].start_year,dst_v.player.age
         order by degree, name, age limit 5;
       """
     Then the result should be, in order, with relax comparison:
-      | properties(src_v).sex | degree   | name          | age | e[1].start_year | dst_v.player.age |
-      | "男"                  | __NULL__ | "Aron Baynes" | 41  | 2022            | 32               |
-      | "男"                  | __NULL__ | "Aron Baynes" | 41  | 2022            | 32               |
-      | "男"                  | __NULL__ | "Aron Baynes" | 41  | 2022            | 32               |
-      | "男"                  | __NULL__ | "Aron Baynes" | 41  | 2022            | 32               |
-      | "男"                  | __NULL__ | "Aron Baynes" | 41  | 2022            | 32               |
+      | properties(src_v).sex | properties(e[0]).degree | properties(dst_v).name | age | e[1].start_year | dst_v.player.age |
+      | "男"                  | UNKNOWN_PROP            | "Danny Green"          | 41  | 2022            | 31               |
+      | "男"                  | UNKNOWN_PROP            | "Danny Green"          | 41  | 2022            | 31               |
+      | "男"                  | UNKNOWN_PROP            | "Kyle Anderson"        | 41  | 2022            | 25               |
+      | "男"                  | UNKNOWN_PROP            | "LeBron James"         | 41  | 2022            | 34               |
+      | "男"                  | UNKNOWN_PROP            | "Kevin Durant"         | 41  | 2022            | 30               |
     When executing query:
       """
       match (v1)-->(v2)-->(v3) where id(v1)=="Manu Ginobili"
@@ -848,23 +848,23 @@ Feature: Prune Properties rule
         order by degree, degree1 limit 5;
       """
     Then the result should be, in order, with relax comparison:
-      | degree | degree1 |
-      | 88     | 88      |
-      | 88     | 88      |
-      | 88     | 88      |
-      | 88     | 88      |
-      | 88     | 88      |
+      | properties(e).degree | degree |
+      | 88                   | 88     |
+      | UNKNOWN_PROP         | 88     |
+      | 88                   | 88     |
+      | UNKNOWN_PROP         | 88     |
+      | 88                   | 88     |
     When executing query:
       """
       match (src_v)-[e:like|serve]->(dst_v)-[e2]-(dst_v2) where  id(src_v)=="Rajon Rondo" return properties(e).degree1,properties(e).degree1,e2.a,dst_v.p.name,dst_v.player.sex1,properties(src_v).name2 limit 5;
       """
     Then the result should be, in order, with relax comparison:
       | properties(e).degree1 | properties(e).degree1 | e2.a | dst_v.p.name | dst_v.player.sex1 | properties(src_v).name2 |
-      | __NULL__              | __NULL__              | NULL | NULL         | NULL              | __NULL__                |
-      | __NULL__              | __NULL__              | NULL | NULL         | NULL              | __NULL__                |
-      | __NULL__              | __NULL__              | NULL | NULL         | NULL              | __NULL__                |
-      | __NULL__              | __NULL__              | NULL | NULL         | NULL              | __NULL__                |
-      | __NULL__              | __NULL__              | NULL | NULL         | NULL              | __NULL__                |
+      | UNKNOWN_PROP          | UNKNOWN_PROP          | NULL | NULL         | NULL              | UNKNOWN_PROP            |
+      | UNKNOWN_PROP          | UNKNOWN_PROP          | NULL | NULL         | NULL              | UNKNOWN_PROP            |
+      | UNKNOWN_PROP          | UNKNOWN_PROP          | NULL | NULL         | NULL              | UNKNOWN_PROP            |
+      | UNKNOWN_PROP          | UNKNOWN_PROP          | NULL | NULL         | NULL              | UNKNOWN_PROP            |
+      | UNKNOWN_PROP          | UNKNOWN_PROP          | NULL | NULL         | NULL              | UNKNOWN_PROP            |
     Then drop the used space
 
   Scenario: Project on not exist tag