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.

Sign up for GitHub

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

fix match index #3694

Merged
merged 9 commits into from
Jan 14, 2022
Merged

fix match index #3694

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9b73538
check scheam
nevermore3 Jan 10, 2022
68d79e7
add test case
nevermore3 Jan 11, 2022
e237126
fix graph crash
nevermore3 Jan 12, 2022
d84aab2
address comment'
nevermore3 Jan 13, 2022
dd93d51
add test case
nevermore3 Jan 13, 2022
34e2e7c
fix error
nevermore3 Jan 13, 2022
afa027c
fix vid select error
nevermore3 Jan 13, 2022
23832d8
fix unit test error
nevermore3 Jan 13, 2022
1c98514
address comment
nevermore3 Jan 14, 2022
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/graph/optimizer/rule/IndexScanRule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,11 +224,11 @@ inline bool verifyType(const Value& val) {
case Value::Type::SET:
case Value::Type::MAP:
case Value::Type::DATASET:
case Value::Type::DURATION:
case Value::Type::GEOGRAPHY: // TODO(jie)
case Value::Type::PATH: {
DLOG(FATAL) << "Not supported value type " << val.type() << "for index.";
return false;
} break;
}
default: {
return true;
}
Expand Down
11 changes: 6 additions & 5 deletions src/graph/planner/match/LabelIndexSeek.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,18 +89,20 @@ StatusOr<SubPlan> LabelIndexSeek::transformNode(NodeContext* nodeCtx) {
if (whereCtx && whereCtx->filter) {
auto* filter = whereCtx->filter;
const auto& nodeAlias = nodeCtx->info->alias;
const auto& schemaName = nodeCtx->scanInfo.schemaNames.back();

if (filter->kind() == Expression::Kind::kLogicalOr) {
auto exprs = ExpressionUtils::collectAll(filter, {Expression::Kind::kLabelTagProperty});
bool labelMatched = true;
bool matched = true;
for (auto* expr : exprs) {
auto tagPropExpr = static_cast<const LabelTagPropertyExpression*>(expr);
if (static_cast<const PropertyExpression*>(tagPropExpr->label())->prop() != nodeAlias) {
labelMatched = false;
if (static_cast<const PropertyExpression*>(tagPropExpr->label())->prop() != nodeAlias ||
tagPropExpr->sym() != schemaName) {
matched = false;
break;
}
}
if (labelMatched) {
if (matched) {
auto flattenFilter = ExpressionUtils::flattenInnerLogicalExpr(filter);
DCHECK_EQ(flattenFilter->kind(), Expression::Kind::kLogicalOr);
auto& filterItems = static_cast<LogicalExpression*>(flattenFilter)->operands();
Expand All @@ -120,7 +122,6 @@ StatusOr<SubPlan> LabelIndexSeek::transformNode(NodeContext* nodeCtx) {
storage::cpp2::IndexQueryContext ctx;
ctx.filter_ref() = Expression::encode(*flattenFilter);
scan->setIndexQueryContext({ctx});
whereCtx.reset();
}
}
}
Expand Down
52 changes: 29 additions & 23 deletions src/graph/visitor/VidExtractVisitor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ void VidExtractVisitor::visit(ConstantExpression *expr) {

void VidExtractVisitor::visit(UnaryExpression *expr) {
if (expr->kind() == Expression::Kind::kUnaryNot) {
// const auto *expr = static_cast<const UnaryExpression *>(expr);
expr->operand()->accept(this);
auto operandResult = moveVidPattern();
if (operandResult.spec == VidPattern::Special::kInUsed) {
Expand Down Expand Up @@ -119,14 +118,15 @@ void VidExtractVisitor::visit(LabelExpression *expr) {
}

void VidExtractVisitor::visit(LabelAttributeExpression *expr) {
if (expr->kind() == Expression::Kind::kLabelAttribute) {
const auto *labelExpr = static_cast<const LabelAttributeExpression *>(expr);
vidPattern_ =
VidPattern{VidPattern::Special::kInUsed,
{{labelExpr->left()->toString(), {VidPattern::Vids::Kind::kOtherSource, {}}}}};
} else {
vidPattern_ = VidPattern{};
}
const auto &label = expr->left()->toString();
vidPattern_ = VidPattern{VidPattern::Special::kInUsed,
{{label, {VidPattern::Vids::Kind::kOtherSource, {}}}}};
}

void VidExtractVisitor::visit(LabelTagPropertyExpression *expr) {
const auto &label = static_cast<const PropertyExpression *>(expr->label())->prop();
vidPattern_ = VidPattern{VidPattern::Special::kInUsed,
{{label, {VidPattern::Vids::Kind::kOtherSource, {}}}}};
}

void VidExtractVisitor::visit(ArithmeticExpression *expr) {
Expand All @@ -144,7 +144,13 @@ void VidExtractVisitor::visit(RelationalExpression *expr) {
{{label, {VidPattern::Vids::Kind::kOtherSource, {}}}}};
return;
}

if (expr->left()->kind() == Expression::Kind::kLabelTagProperty) {
const auto *tagPropExpr = static_cast<const LabelTagPropertyExpression *>(expr->left());
const auto &label = static_cast<const PropertyExpression *>(tagPropExpr->label())->prop();
vidPattern_ = VidPattern{VidPattern::Special::kInUsed,
{{label, {VidPattern::Vids::Kind::kOtherSource, {}}}}};
return;
}
if (expr->left()->kind() != Expression::Kind::kFunctionCall ||
expr->right()->kind() != Expression::Kind::kList ||
!ExpressionUtils::isEvaluableExpr(expr->right())) {
Expand All @@ -165,7 +171,6 @@ void VidExtractVisitor::visit(RelationalExpression *expr) {
VidPattern{VidPattern::Special::kInUsed,
{{fCallExpr->args()->args().front()->toString(),
{VidPattern::Vids::Kind::kIn, listExpr->eval(ctx(nullptr)).getList()}}}};
return;
} else if (expr->kind() == Expression::Kind::kRelEQ) {
// id(V) == vid
if (expr->left()->kind() == Expression::Kind::kLabelAttribute) {
Expand All @@ -175,6 +180,13 @@ void VidExtractVisitor::visit(RelationalExpression *expr) {
{{label, {VidPattern::Vids::Kind::kOtherSource, {}}}}};
return;
}
if (expr->left()->kind() == Expression::Kind::kLabelTagProperty) {
const auto *tagPropExpr = static_cast<const LabelTagPropertyExpression *>(expr->left());
const auto &label = static_cast<const PropertyExpression *>(tagPropExpr->label())->prop();
vidPattern_ = VidPattern{VidPattern::Special::kInUsed,
{{label, {VidPattern::Vids::Kind::kOtherSource, {}}}}};
return;
}
if (expr->left()->kind() != Expression::Kind::kFunctionCall ||
expr->right()->kind() != Expression::Kind::kConstant) {
vidPattern_ = VidPattern{};
Expand All @@ -194,10 +206,13 @@ void VidExtractVisitor::visit(RelationalExpression *expr) {
vidPattern_ = VidPattern{VidPattern::Special::kInUsed,
{{fCallExpr->args()->args().front()->toString(),
{VidPattern::Vids::Kind::kIn, List({constExpr->value()})}}}};
return;
} else {
vidPattern_ = VidPattern{};
return;
if (ExpressionUtils::isPropertyExpr(expr->left())) {
vidPattern_ = VidPattern{VidPattern::Special::kInUsed,
{{"", {VidPattern::Vids::Kind::kOtherSource, {}}}}};
} else {
vidPattern_ = VidPattern{};
}
}
}

Expand All @@ -213,11 +228,9 @@ void VidExtractVisitor::visit(AttributeExpression *expr) {

void VidExtractVisitor::visit(LogicalExpression *expr) {
if (expr->kind() == Expression::Kind::kLogicalAnd) {
// const auto *expr = static_cast<const LogicalExpression *>(expr);
std::vector<VidPattern> operandsResult;
operandsResult.reserve(expr->operands().size());
for (const auto &operand : expr->operands()) {
// operandsResult.emplace_back(reverseEvalVids(operand.get()));
operand->accept(this);
operandsResult.emplace_back(moveVidPattern());
}
Expand Down Expand Up @@ -273,8 +286,6 @@ void VidExtractVisitor::visit(LogicalExpression *expr) {
vidPattern_ = std::move(inResult);
return;
} else if (expr->kind() == Expression::Kind::kLogicalOr) {
// const auto *andExpr = static_cast<const LogicalExpression
// *>(expr);
std::vector<VidPattern> operandsResult;
operandsResult.reserve(expr->operands().size());
for (const auto &operand : expr->operands()) {
Expand Down Expand Up @@ -351,11 +362,6 @@ void VidExtractVisitor::visit(MapExpression *expr) {
}

// property Expression
void VidExtractVisitor::visit(LabelTagPropertyExpression *expr) {
UNUSED(expr);
vidPattern_ = VidPattern{};
}

void VidExtractVisitor::visit(TagPropertyExpression *expr) {
UNUSED(expr);
vidPattern_ = VidPattern{};
Expand Down
2 changes: 1 addition & 1 deletion src/storage/exec/IndexScanNode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ class QualifiedStrategy {
* [start,end) which is generated by `RangeIndex`. Therefore, it is necessary to make additional
* judgment on the truncated string type index
* For example:
* (ab)c meas that string is "abc" but index val has been truncated to "ab". (ab)c > ab is
* (ab)c means that string is "abc" but index val has been truncated to "ab". (ab)c > ab is
* `UNCERTAIN`, and (ab)c > aa is COMPATIBLE.
*
* Args:
Expand Down
79 changes: 65 additions & 14 deletions tests/tck/features/match/SeekById.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,42 +252,93 @@ Feature: Match seek by id
"""
Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down.

@skip
Scenario: test OR logic (reason = "or logic optimization error")
Scenario: test OR logic
nevermore3 marked this conversation as resolved.
Show resolved Hide resolved
When executing query:
"""
MATCH (v)
WHERE id(v) IN ['James Harden', 'Jonathon Simmons', 'Klay Thompson', 'Dejounte Murray']
OR v.player.age == 23
RETURN v.player.name AS Name
"""
Then the result should be, in any order:
| Name |
| 'James Harden' |
| 'Jonathon Simmons' |
| 'Klay Thompson' |
| 'Dejounte Murray' |
| 'Kristaps Porzingis' |
Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down.
When executing query:
"""
MATCH (v)
WHERE id(v) == 'James Harden'
OR v.player.age == 23
RETURN v.player.name AS Name
"""
Then the result should be, in any order:
| Name |
| 'James Harden' |
| 'Kristaps Porzingis' |
Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down.
When executing query:
"""
MATCH (v)
WHERE id(v) == 'James Harden'
OR v.player.age != 23
RETURN v.player.name AS Name
"""
Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down.
When executing query:
"""
MATCH (v:player)
WHERE v.player.name == "Tim Duncan"
OR v.player.age == 23
RETURN v
"""
Then the result should be, in any order:
| Name |
| v |
| ("Kristaps Porzingis" :player{age: 23, name: "Kristaps Porzingis"}) |
| ("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"}) |
When executing query:
"""
nevermore3 marked this conversation as resolved.
Show resolved Hide resolved
MATCH (v:player)
WHERE v.player.name == "Tim Duncan"
OR v.noexist.age == 23
RETURN v
"""
Then the result should be, in any order:
| v |
| ("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"}) |
When executing query:
"""
MATCH (v:player)
WHERE v.player.noexist == "Tim Duncan"
OR v.player.age == 23
RETURN v
"""
Then the result should be, in any order:
| v |
| ("Kristaps Porzingis" :player{age: 23, name: "Kristaps Porzingis"}) |
When executing query:
"""
MATCH (v:player)
WHERE v.player.noexist == "Tim Duncan"
OR v.noexist.age == 23
RETURN v
"""
Then the result should be, in any order:
| v |
When executing query:
nevermore3 marked this conversation as resolved.
Show resolved Hide resolved
"""
MATCH (v:player)
WHERE "Tim Duncan" == v.player.name
OR 23 + 1 == v.noexist.age - 3
RETURN v
"""
Then the result should be, in any order:
| v |
| ("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"}) |
When executing query:
"""
MATCH (v:player)-[e:like]->(t)
WHERE "Tim Duncan" == v.player.name
OR 25 - 2 == v.player.age
RETURN t
"""
Then the result should be, in any order:
| t |
| ("Tony Parker" :player{age: 36, name: "Tony Parker"}) |
| ("Luka Doncic" :player{age: 20, name: "Luka Doncic"}) |
| ("Manu Ginobili" :player{age: 41, name: "Manu Ginobili"}) |

Scenario: Start from end
When executing query:
Expand Down
67 changes: 53 additions & 14 deletions tests/tck/features/match/SeekById.intVid.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,42 +245,81 @@ Feature: Match seek by id
"""
Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down.

@skip
Scenario: test OR logic (reason = "or logic optimization error")
Scenario: test OR logic
When executing query:
"""
MATCH (v)
WHERE id(v) IN [hash('James Harden'), hash('Jonathon Simmons'), hash('Klay Thompson'), hash('Dejounte Murray')]
OR v.player.age == 23
RETURN v.player.name AS Name
"""
Then the result should be, in any order:
| Name |
| 'James Harden' |
| 'Jonathon Simmons' |
| 'Klay Thompson' |
| 'Dejounte Murray' |
| 'Kristaps Porzingis' |
Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down.
When executing query:
"""
MATCH (v)
WHERE id(v) == hash('James Harden')
OR v.player.age == 23
RETURN v.player.name AS Name
"""
Then the result should be, in any order:
| Name |
| 'James Harden' |
| 'Kristaps Porzingis' |
Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down.
When executing query:
"""
MATCH (v)
WHERE id(v) == hash('James Harden')
OR v.player.age != 23
RETURN v.player.name AS Name
"""
Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down.
When executing query:
"""
MATCH (v:player)
WHERE v.player.name == "Tim Duncan"
OR v.player.age == 23
RETURN v.player.name as name
"""
Then the result should be, in any order:
| Name |
| name |
| "Kristaps Porzingis" |
| "Tim Duncan" |
When executing query:
"""
MATCH (v:player)
WHERE v.player.name == "Tim Duncan"
OR v.noexist.age == 23
RETURN v.player.name as name
"""
Then the result should be, in any order:
| name |
| "Tim Duncan" |
When executing query:
"""
MATCH (v:player)
WHERE v.player.noexist == "Tim Duncan"
OR v.player.age == 23
RETURN v.player.name as name
"""
Then the result should be, in any order:
| name |
| "Kristaps Porzingis" |
When executing query:
"""
MATCH (v:player)
WHERE v.player.noexist == "Tim Duncan"
OR v.noexist.age == 23
RETURN v
"""
Then the result should be, in any order:
| v |
When executing query:
"""
MATCH (v:player)
WHERE "Tim Duncan" == v.player.name
OR 23 + 1 == v.noexist.age - 3
RETURN v.name as name
"""
Then the result should be, in any order:
| name |
| "Tim Duncan" |

Scenario: with arithmetic
When executing query:
Expand Down
7 changes: 4 additions & 3 deletions tests/tck/features/optimizer/IndexScanRule.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,10 @@ Feature: Match index selection
And the execution plan should be:
| id | name | dependencies | operator info |
| 6 | Project | 2 | |
| 2 | AppendVertices | 5 | |
| 5 | IndexScan | 0 | |
| 0 | Start | | |
| 9 | Filter | 3 | |
nevermore3 marked this conversation as resolved.
Show resolved Hide resolved
| 3 | AppendVertices | 7 | |
| 7 | IndexScan | 2 | |
| 2 | Start | | |

Scenario: degenerate to full tag scan
When profiling query:
Expand Down