Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Dec 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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