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 multiple match joined on edge produce wrong result #4923

Merged
merged 12 commits into from
Nov 24, 2022
8 changes: 8 additions & 0 deletions src/common/datatypes/Edge.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,14 @@ bool Edge::keyEqual(const Edge& rhs) const {
return src == rhs.dst && dst == rhs.src && ranking == rhs.ranking;
}

std::string Edge::id() const {
if (type > 0) {
return src.toString() + dst.toString() + std::to_string(type) + std::to_string(ranking);
codesigner marked this conversation as resolved.
Show resolved Hide resolved
} else {
return dst.toString() + src.toString() + std::to_string(-type) + std::to_string(ranking);
}
}

} // namespace nebula

namespace std {
Expand Down
3 changes: 3 additions & 0 deletions src/common/datatypes/Edge.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ struct Edge {
const Value& value(const std::string& key) const;

bool keyEqual(const Edge& rhs) const;

// Return this edge's id encoded in string
std::string id() const;
};

inline std::ostream& operator<<(std::ostream& os, const Edge& v) {
Expand Down
8 changes: 6 additions & 2 deletions src/common/function/FunctionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1828,12 +1828,16 @@ FunctionManager::FunctionManager() {
attr.maxArity_ = 1;
attr.isAlwaysPure_ = true;
attr.body_ = [](const auto &args) -> Value {
switch (args[0].get().type()) {
const Value &value = args[0].get();
switch (value.type()) {
case Value::Type::NULLVALUE: {
return Value::kNullValue;
}
case Value::Type::VERTEX: {
return args[0].get().getVertex().vid;
return value.getVertex().vid;
}
case Value::Type::EDGE: {
codesigner marked this conversation as resolved.
Show resolved Hide resolved
return value.getEdge().id();
}
default: {
return Value::kNullBadType;
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 @@ -55,7 +55,7 @@ struct EdgeInfo {
Expression* filter{nullptr};
};

enum class AliasType : int8_t { kNode, kEdge, kPath, kDefault };
enum class AliasType : int8_t { kNode, kEdge, kPath, kEdgeList, kDefault };

struct ScanInfo {
Expression* filter{nullptr};
Expand Down
13 changes: 12 additions & 1 deletion src/graph/planner/match/MatchPlanner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,18 @@ Status MatchPlanner::connectMatchPlan(SubPlan& queryPlan, MatchClauseContext* ma
}
std::unordered_set<std::string> intersectedAliases;
for (auto& alias : matchCtx->aliasesGenerated) {
if (matchCtx->aliasesAvailable.find(alias.first) != matchCtx->aliasesAvailable.end()) {
auto it = matchCtx->aliasesAvailable.find(alias.first);
if (it != matchCtx->aliasesAvailable.end()) {
// Joined type should be same
if (it->second != alias.second) {
return Status::SemanticError(fmt::format(
"{} binding to different type: {} vs {}", alias.first, alias.second, it->second));
}
// Joined On EdgeList is not supported
if (alias.second == AliasType::kEdgeList) {
return Status::SemanticError(alias.first +
" defined with type EdgeList, which cannot be joined on");
}
intersectedAliases.insert(alias.first);
}
}
Expand Down
7 changes: 6 additions & 1 deletion src/graph/validator/MatchValidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -283,16 +283,21 @@ Status MatchValidator::buildEdgeInfo(const MatchPath *path,
edgeInfos[i].types.emplace_back(typeName.value());
}
}
AliasType aliasType = AliasType::kEdge;
auto *stepRange = edge->range();
if (stepRange != nullptr) {
NG_RETURN_IF_ERROR(validateStepRange(stepRange));
edgeInfos[i].range = stepRange;
// if step range is more than 1, then the typ of e binding to is edge list
if (stepRange->max() > stepRange->min()) {
aliasType = AliasType::kEdgeList;
}
}
if (alias.empty()) {
anonymous = true;
alias = vctx_->anonVarGen()->getVar();
} else {
if (!aliases.emplace(alias, AliasType::kEdge).second) {
if (!aliases.emplace(alias, aliasType).second) {
return Status::SemanticError("`%s': Redefined alias", alias.c_str());
}
}
Expand Down
28 changes: 28 additions & 0 deletions tests/tck/features/bugfix/MatchJoinOnEdge.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Copyright (c) 2020 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: Multiple Match joined on edge
When executing query:
"""
MATCH (v:player)-[e:follow*1..2]->(u) WHERE v.player.name=="Tim Duncan" MATCH (vv:player)-[e]->() WHERE vv.player.name=="Tony Parker"return v, u
"""
Then a SemanticError should be raised at runtime: e binding to different type: 1 vs 3
codesigner marked this conversation as resolved.
Show resolved Hide resolved
codesigner marked this conversation as resolved.
Show resolved Hide resolved
When executing query:
"""
MATCH (v:player)-[e]->() WHERE v.player.name=="Tim Duncan" MATCH (u:player)-[e]->() WHERE u.player.name=="Tony Parker"return v
"""
Then the result should be, in any order:
| v |
| |
When executing query:
"""
MATCH (v:player)-[e]->() WHERE v.player.name=="Tim Duncan" MATCH ()-[e]->(u:player) WHERE u.player.name=="Tony Parker"return v, u
"""
Then the result should be, in any order:
| v | u |
| ("player100" :player{age: 42, name: "Tim Duncan"}) | ("player101" :player{age: 36, name: "Tony Parker"}) |