Skip to content

Commit

Permalink
Fix is null filter of scanVertices (#1402)
Browse files Browse the repository at this point in the history 
add test case

dispatch to null value

fix ut

fix scan edge

fix

fix tck

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>
  • Loading branch information
@czpmango @Sophie-Xie
czpmango and Sophie-Xie committed Oct 31, 2022
1 parent ecc7627 commit 5dd2054
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 46 deletions.
2 changes: 2 additions & 0 deletions src/common/datatypes/Value.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ struct Value {

friend class apache::thrift::Cpp2Ops<Value, void>;

// FIXME(czp): We should unify the semantics of NULL and EMPTY at both the semantic layer and
// the implementation layer
enum class Type : uint64_t {
__EMPTY__ = 1UL,
BOOL = 1UL << 1,
Expand Down
2 changes: 1 addition & 1 deletion src/graph/planner/match/ScanSeek.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ StatusOr<SubPlan> ScanSeek::transformNode(NodeContext *nodeCtx) {
Expression *prev = nullptr;
for (const auto &tag : nodeCtx->scanInfo.schemaNames) {
auto *tagPropExpr = TagPropertyExpression::make(pool, tag, kTag);
auto *notEmpty = UnaryExpression::makeIsNotEmpty(pool, tagPropExpr);
auto *notEmpty = UnaryExpression::makeIsNotNull(pool, tagPropExpr);
if (prev != nullptr) {
if (anyLabel) {
auto *orExpr = LogicalExpression::makeOr(pool, prev, notEmpty);
Expand Down
4 changes: 2 additions & 2 deletions src/storage/exec/ScanNode.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,10 @@ class ScanVertexPropNode : public QueryNode<Cursor> {
const std::vector<PropContext>* props) -> nebula::cpp2::ErrorCode {
for (const auto& prop : *props) {
if (prop.returned_) {
row.emplace_back(Value());
row.emplace_back(Value::kNullValue);
}
if (prop.filtered_ && expCtx_ != nullptr) {
expCtx_->setTagProp(tagNode->getTagName(), prop.name_, Value());
expCtx_->setTagProp(tagNode->getTagName(), prop.name_, Value::kNullValue);
}
}
return nebula::cpp2::ErrorCode::SUCCEEDED;
Expand Down
128 changes: 85 additions & 43 deletions src/storage/test/ScanVertexTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,19 +359,19 @@ TEST(ScanVertexTest, MultipleTagsTest) {
nebula::DataSet expect(
{"_vid", "1._vid", "1._tag", "1.name", "1.age", "1.avgScore", "2._tag", "2.name"});
expect.emplace_back(List({"Bulls",
Value::kEmpty,
Value::kEmpty,
Value::kEmpty,
Value::kEmpty,
Value::kEmpty,
Value::kNullValue,
Value::kNullValue,
Value::kNullValue,
Value::kNullValue,
Value::kNullValue,
2,
"Bulls"}));
expect.emplace_back(List({"Cavaliers",
Value::kEmpty,
Value::kEmpty,
Value::kEmpty,
Value::kEmpty,
Value::kEmpty,
Value::kNullValue,
Value::kNullValue,
Value::kNullValue,
Value::kNullValue,
Value::kNullValue,
2,
"Cavaliers"}));
expect.emplace_back(List({"Damian Lillard",
Expand All @@ -380,58 +380,88 @@ TEST(ScanVertexTest, MultipleTagsTest) {
"Damian Lillard",
29,
24,
Value::kEmpty,
Value::kEmpty}));
expect.emplace_back(List(
{"Jason Kidd", "Jason Kidd", 1, "Jason Kidd", 47, 12.6, Value::kEmpty, Value::kEmpty}));
expect.emplace_back(List(
{"Kevin Durant", "Kevin Durant", 1, "Kevin Durant", 31, 27, Value::kEmpty, Value::kEmpty}));
expect.emplace_back(List(
{"Kobe Bryant", "Kobe Bryant", 1, "Kobe Bryant", 41, 25, Value::kEmpty, Value::kEmpty}));
Value::kNullValue,
Value::kNullValue}));
expect.emplace_back(List({"Jason Kidd",
"Jason Kidd",
1,
"Jason Kidd",
47,
12.6,
Value::kNullValue,
Value::kNullValue}));
expect.emplace_back(List({"Kevin Durant",
"Kevin Durant",
1,
"Kevin Durant",
31,
27,
Value::kNullValue,
Value::kNullValue}));
expect.emplace_back(List({"Kobe Bryant",
"Kobe Bryant",
1,
"Kobe Bryant",
41,
25,
Value::kNullValue,
Value::kNullValue}));
expect.emplace_back(List({"Kristaps Porzingis",
"Kristaps Porzingis",
1,
"Kristaps Porzingis",
24,
18.1,
Value::kEmpty,
Value::kEmpty}));
expect.emplace_back(List(
{"Luka Doncic", "Luka Doncic", 1, "Luka Doncic", 21, 24.4, Value::kEmpty, Value::kEmpty}));
Value::kNullValue,
Value::kNullValue}));
expect.emplace_back(List({"Luka Doncic",
"Luka Doncic",
1,
"Luka Doncic",
21,
24.4,
Value::kNullValue,
Value::kNullValue}));
expect.emplace_back(List({"Mavericks",
Value::kEmpty,
Value::kEmpty,
Value::kEmpty,
Value::kEmpty,
Value::kEmpty,
Value::kNullValue,
Value::kNullValue,
Value::kNullValue,
Value::kNullValue,
Value::kNullValue,
2,
"Mavericks"}));
expect.emplace_back(List({"Nuggets",
Value::kEmpty,
Value::kEmpty,
Value::kEmpty,
Value::kEmpty,
Value::kEmpty,
Value::kNullValue,
Value::kNullValue,
Value::kNullValue,
Value::kNullValue,
Value::kNullValue,
2,
"Nuggets"}));
expect.emplace_back(List(
{"Paul George", "Paul George", 1, "Paul George", 30, 19.9, Value::kEmpty, Value::kEmpty}));
expect.emplace_back(List({"Paul George",
"Paul George",
1,
"Paul George",
30,
19.9,
Value::kNullValue,
Value::kNullValue}));
expect.emplace_back(List({"Tracy McGrady",
"Tracy McGrady",
1,
"Tracy McGrady",
41,
19.6,
Value::kEmpty,
Value::kEmpty}));
Value::kNullValue,
Value::kNullValue}));
expect.emplace_back(List({"Vince Carter",
"Vince Carter",
1,
"Vince Carter",
43,
16.7,
Value::kEmpty,
Value::kEmpty}));
Value::kNullValue,
Value::kNullValue}));
EXPECT_EQ(expect, *resp.props_ref());
}
}
Expand Down Expand Up @@ -468,8 +498,14 @@ TEST(ScanVertexTest, FilterTest) {
ASSERT_EQ(0, resp.result.failed_parts.size());
nebula::DataSet expect(
{"_vid", "1._vid", "1._tag", "1.name", "1.age", "1.avgScore", "2._tag", "2.name"});
expect.emplace_back(List(
{"Kobe Bryant", "Kobe Bryant", 1, "Kobe Bryant", 41, 25, Value::kEmpty, Value::kEmpty}));
expect.emplace_back(List({"Kobe Bryant",
"Kobe Bryant",
1,
"Kobe Bryant",
41,
25,
Value::kNullValue,
Value::kNullValue}));
EXPECT_EQ(expect, *resp.props_ref());
}
{
Expand All @@ -485,7 +521,7 @@ TEST(ScanVertexTest, FilterTest) {
filter = LogicalExpression::makeAnd(
&pool,
filter,
UnaryExpression::makeIsEmpty(&pool, TagPropertyExpression::make(&pool, "2", "name")));
UnaryExpression::makeIsNull(&pool, TagPropertyExpression::make(&pool, "2", "name")));
req.filter_ref() = filter->encode();
auto* processor = ScanVertexProcessor::instance(env, nullptr);
auto f = processor->getFuture();
Expand All @@ -495,8 +531,14 @@ TEST(ScanVertexTest, FilterTest) {
ASSERT_EQ(0, resp.result.failed_parts.size());
nebula::DataSet expect(
{"_vid", "1._vid", "1._tag", "1.name", "1.age", "1.avgScore", "2._tag", "2.name"});
expect.emplace_back(List(
{"Kobe Bryant", "Kobe Bryant", 1, "Kobe Bryant", 41, 25, Value::kEmpty, Value::kEmpty}));
expect.emplace_back(List({"Kobe Bryant",
"Kobe Bryant",
1,
"Kobe Bryant",
41,
25,
Value::kNullValue,
Value::kNullValue}));
EXPECT_EQ(expect, *resp.props_ref());
}
}
Expand Down
19 changes: 19 additions & 0 deletions tests/tck/features/match/IndexSelecting.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,4 +160,23 @@ Feature: Index selecting for match statement
| 2 | AppendVertices | 6 | |
| 6 | IndexScan | 0 | {"indexCtx": {"columnHints":{"scanType":"PREFIX"}}} |
| 0 | Start | | |
When profiling query:
"""
MATCH (v)
WHERE v.player.name IS NOT NULL
RETURN labels(v) AS l
LIMIT 300
"""
Then the result should be, in any order:
| l |
| ["player"] |
| ["player"] |
| ["player"] |
And the execution plan should be:
| id | name | dependencies | operator info |
| 10 | Project | 9 | |
| 9 | Limit | 2 | |
| 2 | AppendVertices | 6 | |
| 6 | ScanVertices | 0 | |
| 0 | Start | | |
Then drop the used space

0 comments on commit 5dd2054

Please sign in to comment.