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

Support match from id Set and fix some related issues #5360

Merged
merged 3 commits into from
Feb 22, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
14 changes: 14 additions & 0 deletions src/common/datatypes/Set.cpp
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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use camelcase naming.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consistent with std::set_intersection naming.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep the consistence in the project

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
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
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
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
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
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -1097,9 +1097,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
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
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
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
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
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