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

fix match index #3694

Merged
merged 9 commits into from
Jan 14, 2022
Merged
Show file tree
Hide file tree
Changes from 8 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/graph/optimizer/rule/IndexScanRule.cpp
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
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
4 changes: 4 additions & 0 deletions src/graph/planner/match/VertexIdSeek.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ bool VertexIdSeek::matchNode(NodeContext *nodeCtx) {
if (vidResult.spec != VidExtractVisitor::VidPattern::Special::kInUsed) {
return false;
}
if (vidResult.nodes.size() > 1) {
// where id(v) == 'xxx' or id(t) == 'yyy'
return false;
}
for (auto &nodeVid : vidResult.nodes) {
if (nodeVid.second.kind == VidExtractVisitor::VidPattern::Vids::Kind::kIn) {
if (nodeVid.first == node.alias) {
Expand Down
22 changes: 11 additions & 11 deletions src/graph/util/ExpressionUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -570,32 +570,32 @@ Expression *ExpressionUtils::rewriteRelExprHelper(const Expression *expr,
}

StatusOr<Expression *> ExpressionUtils::filterTransform(const Expression *filter) {
// If the filter contains more than one different LabelAttribute expr, this filter cannot be
// pushed down
auto propExprs = ExpressionUtils::collectAll(filter, {Expression::Kind::kLabelTagProperty});
// Check if any overflow happen before filter tranform
auto initialConstFold = foldConstantExpr(filter);
NG_RETURN_IF_ERROR(initialConstFold);
auto newFilter = initialConstFold.value();

// If the filter contains more than one different Label expr, this filter cannot be
// pushed down, such as where v1.player.name == 'xxx' or v2.player.age == 20
auto propExprs = ExpressionUtils::collectAll(newFilter, {Expression::Kind::kLabel});
// Deduplicate the list
std::unordered_set<std::string> dedupPropExprSet;
for (auto &iter : propExprs) {
dedupPropExprSet.emplace(iter->toString());
}
if (dedupPropExprSet.size() > 1) {
return const_cast<Expression *>(filter);
return const_cast<Expression *>(newFilter);
}

// Check if any overflow happen before filter tranform
auto initialConstFold = foldConstantExpr(filter);
NG_RETURN_IF_ERROR(initialConstFold);

// Rewrite relational expression
auto rewrittenExpr = initialConstFold.value();
rewrittenExpr = rewriteRelExpr(rewrittenExpr);
auto rewrittenExpr = rewriteRelExpr(newFilter->clone());
nevermore3 marked this conversation as resolved.
Show resolved Hide resolved

// Fold constant expression
auto constantFoldRes = foldConstantExpr(rewrittenExpr);
// If errors like overflow happened during the constant fold, stop transferming and return the
// original expression
if (!constantFoldRes.ok()) {
return const_cast<Expression *>(filter);
return const_cast<Expression *>(newFilter);
}
rewrittenExpr = constantFoldRes.value();

Expand Down
52 changes: 29 additions & 23 deletions src/graph/visitor/VidExtractVisitor.cpp
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
14 changes: 9 additions & 5 deletions src/graph/visitor/test/FilterTransformTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,17 @@ TEST_F(FilterTransformTest, TestCalculationOverflow) {
auto res = ExpressionUtils::filterTransform(expr);
ASSERT(res.ok());
auto expected = expr;
ASSERT_EQ(res.value(), expected) << res.value() << " vs. " << expected->toString();
ASSERT_EQ(res.value()->toString(), expected->toString())
<< res.value()->toString() << " vs. " << expected->toString();
}
// (v.age + 1 < -9223372036854775808) => unchanged
{
auto expr = ltExpr(addExpr(laExpr("v", "age"), constantExpr(1)), constantExpr(INT64_MIN));
auto res = ExpressionUtils::filterTransform(expr);
ASSERT(res.ok());
auto expected = expr;
ASSERT_EQ(res.value(), expected) << res.value() << " vs. " << expected->toString();
ASSERT_EQ(res.value()->toString(), expected->toString())
<< res.value()->toString() << " vs. " << expected->toString();
}
// (v.age - 1 < 9223372036854775807 + 1) => overflow
{
Expand Down Expand Up @@ -71,7 +73,8 @@ TEST_F(FilterTransformTest, TestCalculationOverflow) {
auto res = ExpressionUtils::filterTransform(expr);
ASSERT(res.ok());
auto expected = expr;
ASSERT_EQ(res.value(), expected) << res.value()->toString() << " vs. " << expected->toString();
ASSERT_EQ(res.value()->toString(), expected->toString())
<< res.value()->toString() << " vs. " << expected->toString();
}
// !!!(v.age + 1 < -9223372036854775808) => unchanged
{
Expand All @@ -80,12 +83,13 @@ TEST_F(FilterTransformTest, TestCalculationOverflow) {
auto res = ExpressionUtils::filterTransform(expr);
ASSERT(res.ok());
auto expected = expr;
ASSERT_EQ(res.value(), expected) << res.value()->toString() << " vs. " << expected->toString();
ASSERT_EQ(res.value()->toString(), expected->toString())
<< res.value()->toString() << " vs. " << expected->toString();
}
}

TEST_F(FilterTransformTest, TestNoRewrite) {
// Do not rewrite if the filter contains more than one different LabelAttribute expr
// Do not rewrite if the filter contains more than one different Label expr
{
// (v.age - 1 < v2.age + 2) => Unchanged
auto expr = ltExpr(minusExpr(laExpr("v", "age"), constantExpr(1)),
Expand Down
2 changes: 1 addition & 1 deletion src/storage/exec/IndexScanNode.h
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
Loading