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 967a8c9 commit 17504bb
Show file tree
Hide file tree
Showing 23 changed files with 74 additions and 62 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
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
50 changes: 25 additions & 25 deletions tests/tck/features/optimizer/PrunePropertiesRule.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit 17504bb

Please sign in to comment.