Skip to content

Commit

Permalink
Support match from id Set and fix some related issues (vesoft-inc#2454)
Browse files Browse the repository at this point in the history 
* Fix match by ids

small delete

* Delete some code comments

* fix ut

Co-authored-by: kyle.cao <[email protected]>
  • Loading branch information
@nebula-bots @czpmango
nebula-bots and czpmango authored Feb 23, 2023
1 parent 2c8d0ff commit 44f0f40
Show file tree
Hide file tree
Showing 14 changed files with 101 additions and 116 deletions.
14 changes: 14 additions & 0 deletions src/common/datatypes/Set.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,20 @@

namespace nebula {

Set Set::set_intersection(const Set& lhs, const Set& rhs) {
if (lhs.size() <= rhs.size()) {
Set iset;
for (const Value& key : lhs.values) {
if (rhs.values.count(key) > 0) {
iset.values.insert(key);
}
}
return iset;
} else {
return set_intersection(rhs, lhs);
}
}

std::string Set::toString() const {
std::vector<std::string> value(values.size());
std::transform(values.begin(), values.end(), value.begin(), [](const auto& v) -> std::string {
Expand Down
1 change: 1 addition & 0 deletions src/common/datatypes/Set.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ struct Set {
folly::dynamic toJson() const;
// Extract the metadata of each element
folly::dynamic getMetaData() const;
static Set set_intersection(const Set& lhs, const Set& rhs);

Set& operator=(const Set& rhs) {
if (this == &rhs) {
Expand Down
17 changes: 0 additions & 17 deletions src/common/function/FunctionManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1581,23 +1581,6 @@ FunctionManager::FunctionManager() {
return std::find(args.begin() + 1, args.end(), args[0].get()) != args.end();
};
}
{
auto &attr = functions_["near"];
attr.minArity_ = 2;
attr.maxArity_ = 2;
attr.isAlwaysPure_ = true;
attr.body_ = [](const auto &args) -> Value {
// auto result = geo::GeoFilter::near(args);
// if (!result.ok()) {
// return std::string("");
// } else {
// return std::move(result).value();
// }
// TODO
UNUSED(args);
return std::string("");
};
}
{
auto &attr = functions_["cos_similarity"];
attr.minArity_ = 2;
Expand Down
2 changes: 1 addition & 1 deletion src/graph/context/ast/CypherAstContext.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ struct NodeContext final : PatternContext {

// Output fields
ScanInfo scanInfo;
List ids;
Set ids;
// initialize start expression in project node
Expression* initialExpr{nullptr};
};
Expand Down
10 changes: 0 additions & 10 deletions src/graph/executor/query/FulltextIndexScanExecutor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,16 +114,6 @@ StatusOr<plugin::ESQueryResult> FulltextIndexScanExecutor::accessFulltextIndex(
}
}

// auto retryCnt = FLAGS_ft_request_retry_times > 0 ? FLAGS_ft_request_retry_times : 1;
// StatusOr<nebula::plugin::ESQueryResult> result;
// while (retryCnt-- > 0) {
// result = execFunc();
// if (!result.ok()) {
// continue;
// }
// break;
// }

return execFunc();
}

Expand Down
14 changes: 13 additions & 1 deletion src/graph/planner/match/VertexIdSeek.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,23 @@ std::string VertexIdSeek::listToAnnoVarVid(QueryContext *qctx, const List &list)
return input;
}

std::string VertexIdSeek::setToAnnoVarVid(QueryContext *qctx, const Set &set) {
auto input = qctx->vctx()->anonVarGen()->getVar();
DataSet vids({kVid});
for (auto &v : set.values) {
vids.emplace_back(Row({std::move(v)}));
}

qctx->ectx()->setResult(input, ResultBuilder().value(Value(std::move(vids))).build());

return input;
}

StatusOr<SubPlan> VertexIdSeek::transformNode(NodeContext *nodeCtx) {
SubPlan plan;
auto *qctx = nodeCtx->qctx;

std::string inputVar = listToAnnoVarVid(qctx, nodeCtx->ids);
std::string inputVar = setToAnnoVarVid(qctx, nodeCtx->ids);

auto *passThrough = PassThroughNode::make(qctx, nullptr);
passThrough->setOutputVar(inputVar);
Expand Down
1 change: 1 addition & 0 deletions src/graph/planner/match/VertexIdSeek.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class VertexIdSeek final : public StartVidFinder {
StatusOr<SubPlan> transformEdge(EdgeContext* edgeCtx) override;

std::string listToAnnoVarVid(QueryContext* qctx, const List& list);
std::string setToAnnoVarVid(QueryContext* qctx, const Set& list);

const char* name() const override {
return "VertexIdSeekFinder";
Expand Down
3 changes: 0 additions & 3 deletions src/graph/validator/MatchValidator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1128,9 +1128,6 @@ Status MatchValidator::validateMatchPathExpr(
expr->accept(&visitor);
auto matchPathExprs = ExpressionUtils::collectAll(expr, {Expression::Kind::kMatchPathPattern});
for (auto &matchPathExpr : matchPathExprs) {
// auto matchClauseCtx = getContext<MatchClauseContext>();
// matchClauseCtx->aliasesAvailable = availableAliases;
// matchClauseCtx->isOptional = true;
DCHECK_EQ(matchPathExpr->kind(), Expression::Kind::kMatchPathPattern);
auto *matchPathExprImpl = const_cast<MatchPathPatternExpression *>(
static_cast<const MatchPathPatternExpression *>(matchPathExpr));
Expand Down
33 changes: 0 additions & 33 deletions src/graph/visitor/FoldConstantExprVisitor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,27 +61,6 @@ void FoldConstantExprVisitor::visit(ArithmeticExpression *expr) {

void FoldConstantExprVisitor::visit(RelationalExpression *expr) {
visitBinaryExpr(expr);
if (expr->kind() == Expression::Kind::kRelIn) {
auto rhs = static_cast<RelationalExpression *>(expr)->right();
if (rhs->kind() == Expression::Kind::kConstant) {
auto ce = static_cast<ConstantExpression *>(rhs);
auto v = ce->value();
// Convert to more efficient C++ data structure based on the size of the container expression
if (v.isList()) {
auto &list = v.getList().values;
if (list.size() >= 16) {
ce->setValue(Set(std::unordered_set<Value>{std::make_move_iterator(list.begin()),
std::make_move_iterator(list.end())}));
}
} else if (v.isSet()) {
auto &set = v.getSet().values;
if (set.size() < 16) {
ce->setValue(List(std::vector<Value>{std::make_move_iterator(set.begin()),
std::make_move_iterator(set.end())}));
}
}
}
}
}

void FoldConstantExprVisitor::visit(SubscriptExpression *expr) {
Expand All @@ -96,29 +75,17 @@ void FoldConstantExprVisitor::visit(AttributeExpression *expr) {
void FoldConstantExprVisitor::visit(LogicalExpression *expr) {
auto &operands = expr->operands();
auto foldable = true;
// auto shortCircuit = false;
for (auto i = 0u; i < operands.size(); i++) {
auto *operand = operands[i];
operand->accept(this);
if (canBeFolded_) {
auto *newExpr = fold(operand);
if (!ok()) return;
expr->setOperand(i, newExpr);
/*
if (newExpr->value().isBool()) {
auto value = newExpr->value().getBool();
if ((value && expr->kind() == Expression::Kind::kLogicalOr) ||
(!value && expr->kind() == Expression::Kind::kLogicalAnd)) {
shortCircuit = true;
break;
}
}
*/
} else {
foldable = false;
}
}
// canBeFolded_ = foldable || shortCircuit;
canBeFolded_ = foldable;
}

Expand Down
65 changes: 25 additions & 40 deletions src/graph/visitor/VidExtractVisitor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,8 @@ namespace graph {
if (find == v.nodes.end()) {
v.nodes.emplace(std::move(*find));
} else {
std::sort(find->second.vids.values.begin(), find->second.vids.values.end());
std::sort(node.second.vids.values.begin(), node.second.vids.values.end());
std::vector<Value> intersection;
std::set_intersection(find->second.vids.values.begin(),
find->second.vids.values.end(),
node.second.vids.values.begin(),
node.second.vids.values.end(),
std::back_inserter(intersection));
find->second.vids.values = std::move(intersection);
Set intersection = Set::set_intersection(find->second.vids, node.second.vids);
find->second.vids = std::move(intersection);
}
}
return v;
Expand All @@ -41,15 +34,8 @@ namespace graph {
if (find == left.nodes.end()) {
left.nodes.emplace(std::move(right));
} else {
std::sort(find->second.vids.values.begin(), find->second.vids.values.end());
std::sort(right.second.vids.values.begin(), right.second.vids.values.end());
std::vector<Value> values;
std::set_intersection(find->second.vids.values.begin(),
find->second.vids.values.end(),
right.second.vids.values.begin(),
right.second.vids.values.end(),
std::back_inserter(values));
find->second.vids.values = std::move(values);
Set intersection = Set::set_intersection(find->second.vids, right.second.vids);
find->second.vids = std::move(intersection);
}
return std::move(left);
}
Expand All @@ -62,18 +48,10 @@ namespace graph {
v.nodes.emplace(std::move(left));
v.nodes.emplace(std::move(right));
} else {
std::sort(left.second.vids.values.begin(), left.second.vids.values.end());
std::sort(right.second.vids.values.begin(), right.second.vids.values.end());
std::vector<Value> values;
std::set_intersection(left.second.vids.values.begin(),
left.second.vids.values.end(),
right.second.vids.values.begin(),
right.second.vids.values.end(),
std::back_inserter(values));
Set intersection = Set::set_intersection(left.second.vids, right.second.vids);
v.nodes[left.first].kind = VidPattern::Vids::Kind::kIn;
v.nodes[left.first].vids.values.insert(v.nodes[left.first].vids.values.end(),
std::make_move_iterator(values.begin()),
std::make_move_iterator(values.end()));
v.nodes[left.first].vids.values.insert(std::make_move_iterator(intersection.values.begin()),
std::make_move_iterator(intersection.values.end()));
}
return v;
}
Expand Down Expand Up @@ -166,14 +144,23 @@ void VidExtractVisitor::visit(RelationalExpression *expr) {
return;
}

auto rightListValue = expr->right()->eval(graph::QueryExpressionContext(qctx_->ectx())());
if (!rightListValue.isList()) {
auto vs = expr->right()->eval(graph::QueryExpressionContext(qctx_->ectx())());
if (vs.isList()) {
Set s;
for (auto &v : vs.getList().values) {
s.values.emplace(v);
}
vidPattern_ = VidPattern{
VidPattern::Special::kInUsed,
{{fCallExpr->args()->args().front()->toString(), {VidPattern::Vids::Kind::kIn, s}}}};
} else if (vs.isSet()) {
vidPattern_ = VidPattern{VidPattern::Special::kInUsed,
{{fCallExpr->args()->args().front()->toString(),
{VidPattern::Vids::Kind::kIn, vs.getSet()}}}};
} else {
vidPattern_ = VidPattern{};
return;
}
vidPattern_ = VidPattern{VidPattern::Special::kInUsed,
{{fCallExpr->args()->args().front()->toString(),
{VidPattern::Vids::Kind::kIn, rightListValue.getList()}}}};
} else if (expr->kind() == Expression::Kind::kRelEQ) {
// id(V) == vid
if (expr->left()->kind() == Expression::Kind::kLabelAttribute) {
Expand Down Expand Up @@ -211,15 +198,15 @@ void VidExtractVisitor::visit(RelationalExpression *expr) {
}
vidPattern_ = VidPattern{VidPattern::Special::kInUsed,
{{fCallExpr->args()->args().front()->toString(),
{VidPattern::Vids::Kind::kIn, List({constExpr->value()})}}}};
{VidPattern::Vids::Kind::kIn, Set({constExpr->value()})}}}};
} else if ((expr->right()->kind() == Expression::Kind::kVar ||
expr->right()->kind() == Expression::Kind::kSubscript) &&
ExpressionUtils::isEvaluableExpr(expr->right(), qctx_)) {
auto rValue = expr->right()->eval(graph::QueryExpressionContext(qctx_->ectx())());
if (SchemaUtil::isValidVid(rValue)) {
vidPattern_ = VidPattern{VidPattern::Special::kInUsed,
{{fCallExpr->args()->args().front()->toString(),
{VidPattern::Vids::Kind::kIn, List({rValue})}}}};
{VidPattern::Vids::Kind::kIn, Set({rValue})}}}};
return;
} else {
vidPattern_ = VidPattern{};
Expand Down Expand Up @@ -261,7 +248,6 @@ void VidExtractVisitor::visit(LogicalExpression *expr) {
for (auto &node : operandResult.nodes) {
if (node.second.kind == VidPattern::Vids::Kind::kNotIn) {
notInResult.nodes[node.first].vids.values.insert(
notInResult.nodes[node.first].vids.values.end(),
std::make_move_iterator(node.second.vids.values.begin()),
std::make_move_iterator(node.second.vids.values.end()));
}
Expand Down Expand Up @@ -293,11 +279,11 @@ void VidExtractVisitor::visit(LogicalExpression *expr) {
for (auto &node : inResult.nodes) {
auto find = notInResult.nodes.find(node.first);
if (find != notInResult.nodes.end()) {
List removeNotIn;
Set removeNotIn;
for (auto &v : node.second.vids.values) {
if (std::find(find->second.vids.values.begin(), find->second.vids.values.end(), v) ==
find->second.vids.values.end()) {
removeNotIn.emplace_back(std::move(v));
removeNotIn.values.insert(std::move(v));
}
}
node.second.vids = std::move(removeNotIn);
Expand Down Expand Up @@ -327,7 +313,6 @@ void VidExtractVisitor::visit(LogicalExpression *expr) {
case VidPattern::Vids::Kind::kIn: {
inResult.nodes[node.first].kind = VidPattern::Vids::Kind::kIn;
inResult.nodes[node.first].vids.values.insert(
inResult.nodes[node.first].vids.values.end(),
std::make_move_iterator(node.second.vids.values.begin()),
std::make_move_iterator(node.second.vids.values.end()));
}
Expand Down
2 changes: 1 addition & 1 deletion src/graph/visitor/VidExtractVisitor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class VidExtractVisitor final : public ExprVisitor {
kIn,
kNotIn,
} kind{Kind::kOtherSource};
List vids;
Set vids;
};
std::unordered_map<std::string, Vids> nodes;
};
Expand Down
2 changes: 0 additions & 2 deletions src/graph/visitor/test/ExtractFilterExprVisitorTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,6 @@ TEST_F(ExtractFilterExprVisitorTest, TestCanBePushSomeAndOr) {
auto rmexpr = std::move(visitor).remainedExpr();
ASSERT_EQ(rmexpr->kind(), Expression::Kind::kDstProperty);
ASSERT_EQ(expr->kind(), Expression::Kind::kLogicalAnd);
// auto newExpr = ExpressionUtils::foldConstantExpr(expr, &pool_);
// ASSERT_EQ(newExpr->kind(), Expression::Kind::kLogicalOr);
}

// let A indicates (can be pushed expr), and B denotes (can not push expr) in the following.
Expand Down
19 changes: 11 additions & 8 deletions src/graph/visitor/test/FoldConstantExprVisitorTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,9 @@ TEST_F(FoldConstantExprVisitorTest, TestRelIn) {
auto inSetExpr = RelationalExpression::makeIn(pool, lhs, setExpr);
inSetExpr->accept(&foldVisitor);
auto expectInSetExpr = RelationalExpression::makeIn(
pool, lhs, ConstantExpression::make(pool, List(std::vector<Value>{5, "3", false, 4, 3})));
pool,
lhs,
ConstantExpression::make(pool, Set(std::unordered_set<Value>{5, "3", false, 4, 3})));
EXPECT_EQ(*inSetExpr, *expectInSetExpr);

for (int i = 10u; i < 20; ++i) {
Expand All @@ -253,19 +255,20 @@ TEST_F(FoldConstantExprVisitorTest, TestRelIn) {
auto expectInBigListExpr = RelationalExpression::makeIn(
pool,
lhs,
ConstantExpression::make(pool,
Set(std::unordered_set<Value>{
19, 18, 3, 16, 4, 17, false, 13, "3", 5, 10, 11, 12, 14, 15})));
EXPECT_EQ(*inBigListExpr, *expectInBigListExpr);
ConstantExpression::make(
pool,
List(
std::vector<Value>{3, 4, false, "3", 5, 3, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19})));
EXPECT_EQ(*inBigListExpr, *expectInBigListExpr) << inBigListExpr->toString();
auto bigSetExpr = SetExpression::make(pool, items);
auto inBigSetExpr = RelationalExpression::makeIn(pool, lhs, bigSetExpr);
inBigSetExpr->accept(&foldVisitor);
auto expectInBigSetExpr = RelationalExpression::makeIn(
pool,
lhs,
ConstantExpression::make(
pool,
List(std::vector<Value>{19, 18, 16, 15, 14, 12, 11, 10, 5, 13, "3", 17, false, 4, 3})));
ConstantExpression::make(pool,
Set(std::unordered_set<Value>{
19, 18, 16, 15, 14, 12, 11, 10, 5, 13, "3", 17, false, 4, 3})));
EXPECT_EQ(*inBigSetExpr, *expectInBigSetExpr);
}

Expand Down
Loading

0 comments on commit 44f0f40

Please sign in to comment.