-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 rewrite edge node filter. #4815
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ | |
#include <folly/json.h> | ||
|
||
#include <boost/algorithm/string/replace.hpp> | ||
#include <cstdint> | ||
|
||
#include "common/base/Base.h" | ||
#include "common/datatypes/DataSet.h" | ||
|
@@ -41,6 +42,7 @@ std::unordered_map<std::string, Value::Type> FunctionManager::variadicFunReturnT | |
{"concat_ws", Value::Type::STRING}, | ||
{"cos_similarity", Value::Type::FLOAT}, | ||
{"coalesce", Value::Type::__EMPTY__}, | ||
{"_any", Value::Type::__EMPTY__}, | ||
}; | ||
|
||
std::unordered_map<std::string, std::vector<TypeSignature>> FunctionManager::typeSignature_ = { | ||
|
@@ -2820,6 +2822,21 @@ FunctionManager::FunctionManager() { | |
} | ||
}; | ||
} | ||
// Get any argument which is not empty/null | ||
{ | ||
auto &attr = functions_["_any"]; | ||
attr.minArity_ = 1; | ||
attr.maxArity_ = INT64_MAX; | ||
attr.isAlwaysPure_ = true; | ||
attr.body_ = [](const auto &args) -> Value { | ||
for (const auto &arg : args) { | ||
if (!arg.get().isNull() && !arg.get().empty()) { | ||
return arg.get(); | ||
} | ||
} | ||
return Value::kNullValue; | ||
}; | ||
} | ||
Comment on lines
+2825
to
+2839
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think
Both neo4j and nebula support it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Better to resuse it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should ignore |
||
} // NOLINT | ||
|
||
// static | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
#include "graph/optimizer/rule/PushEFilterDownRule.h" | ||
|
||
#include "common/expression/Expression.h" | ||
#include "common/expression/FunctionCallExpression.h" | ||
#include "graph/optimizer/OptContext.h" | ||
#include "graph/optimizer/OptGroup.h" | ||
#include "graph/planner/plan/PlanNode.h" | ||
|
@@ -123,17 +124,15 @@ std::string PushEFilterDownRule::toString() const { | |
return nullptr; | ||
} | ||
} else { | ||
ret = LogicalExpression::makeOr(pool); | ||
std::vector<Expression *> operands; | ||
operands.reserve(edges.size()); | ||
auto args = ArgumentList::make(pool); | ||
for (auto &edge : edges) { | ||
auto reEdgeExp = rewriteStarEdge(propertyExpr, spaceId, edge, schemaMng, pool); | ||
if (reEdgeExp == nullptr) { | ||
return nullptr; | ||
} | ||
operands.emplace_back(reEdgeExp); | ||
args->addArgument(reEdgeExp); | ||
} | ||
static_cast<LogicalExpression *>(ret)->setOperands(std::move(operands)); | ||
ret = FunctionCallExpression::make(pool, "_any", args); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not just rewrite it as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about others? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. `like.start_year > xx' ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This filter could not be used in path pattern. I don't understand the purpose of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Storage don't support There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree with @czpmango comment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think what @Shylock-Hg wants is a generic way. In the path pattern alone, however, rewriting with == suffices. |
||
} | ||
return ret; | ||
}; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
# Copyright (c) 2022 vesoft inc. All rights reserved. | ||
# | ||
# This source code is licensed under Apache 2.0 License. | ||
Feature: Test match used in pipe | ||
|
||
Background: | ||
Given a graph with space named "nba" | ||
|
||
Scenario: Order by after match | ||
When executing query: | ||
""" | ||
match (v)-[e:like|teammate{start_year: 2010}]->() where id(v) == 'Tim Duncan' return e | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In openCypher, this edge filter means to find these edges which contain like or teammate labels and have the property start_year equal to 2010. So I think this filter should be converted to following format in NebulaGraph:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not only |
||
""" | ||
Then the result should be, in any order, with relax comparison: | ||
| e | | ||
| [:teammate "Tim Duncan"->"Danny Green" @0 {end_year: 2016, start_year: 2010}] | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the new added _any function, is it callable from query?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understood correctly, it seems to me that you mean
_all
, not_any
. If there are 10 edge types, all of which has the same prop:prop_x
, the semantics ofany
could be satisfied if only 1 suchprop_x
is found from all 10 edge types.all
means making sure and finding allprop_x
from all 10 edge types. I think you mean the latter one.