Skip to content

Commit

Permalink
Revert "Remove all UNKNOWN_PROP as a type of null. (vesoft-inc#4907)"
Browse files Browse the repository at this point in the history
This reverts commit aa62416.
  • Loading branch information
xtcyclist committed Dec 29, 2022
1 parent 075a7bc commit 3d8050d
Show file tree
Hide file tree
Showing 25 changed files with 77 additions and 64 deletions.
4 changes: 2 additions & 2 deletions src/codec/RowReaderV1.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion src/codec/RowReaderV2.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/common/datatypes/Map.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
4 changes: 4 additions & 0 deletions src/common/datatypes/Value.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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"},
};

Expand Down Expand Up @@ -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__";
}
Expand Down
6 changes: 4 additions & 2 deletions src/common/datatypes/Value.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand All @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions src/common/datatypes/test/ValueTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

Expand All @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion src/common/datatypes/test/ValueToJsonTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion src/common/expression/AttributeExpression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ const Value &AttributeExpression::eval(ExpressionContext &ctx) {
}
}
}
return Value::kNullValue;
return Value::kNullUnknownProp;
}
case Value::Type::EDGE: {
DCHECK(!rvalue.getStr().empty());
Expand Down
6 changes: 3 additions & 3 deletions src/common/expression/test/AttributeExpressionTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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));
Expand All @@ -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));
Expand Down
1 change: 1 addition & 0 deletions src/common/expression/test/SubscriptExpressionTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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]
{
Expand Down
6 changes: 3 additions & 3 deletions src/common/time/TimeUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ class TimeUtils {
} else if (lowerProp == "microsecond") {
return static_cast<int>(dt.microsec);
} else {
return Value::kNullValue;
return Value::kNullUnknownProp;
}
}

Expand Down Expand Up @@ -160,7 +160,7 @@ class TimeUtils {
} else if (lowerProp == "day") {
return d.day;
} else {
return Value::kNullValue;
return Value::kNullUnknownProp;
}
}

Expand Down Expand Up @@ -203,7 +203,7 @@ class TimeUtils {
} else if (lowerProp == "microsecond") {
return t.microsec;
} else {
return Value::kNullValue;
return Value::kNullUnknownProp;
}
}

Expand Down
5 changes: 4 additions & 1 deletion src/common/utils/IndexKeyUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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");
Expand Down
2 changes: 1 addition & 1 deletion src/interface/common.thrift
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion src/storage/exec/IndexEdgeScanNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
2 changes: 1 addition & 1 deletion src/storage/exec/IndexVertexScanNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
2 changes: 1 addition & 1 deletion src/storage/exec/QueryUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/tools/db-upgrade/DbUpgrader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
4 changes: 2 additions & 2 deletions tests/common/nebula_test_suite.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
12 changes: 6 additions & 6 deletions tests/tck/features/expression/Attribute.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
13 changes: 7 additions & 6 deletions tests/tck/features/match/Base.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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:
"""
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions tests/tck/features/match/MultiQueryParts.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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] |
Expand Down
4 changes: 2 additions & 2 deletions tests/tck/features/match/With.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading

0 comments on commit 3d8050d

Please sign in to comment.