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.

Sign up for GitHub

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

Jump to bottom

enhance match attribute filter #3272

Merged
merged 6 commits into from
Nov 22, 2021
Merged

enhance match attribute filter #3272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
744fd5b
enhance match attribute filter
nevermore3 Nov 4, 2021
b9af8ce
add test case
nevermore3 Nov 4, 2021
a54bf40
fix test error
nevermore3 Nov 5, 2021
f6a2e54
delete some null test
nevermore3 Nov 5, 2021
c02c2ca
fix test error
nevermore3 Nov 5, 2021
f2d46e4
fix storage crash
nevermore3 Nov 22, 2021
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
58 changes: 36 additions & 22 deletions src/graph/validator/MatchValidator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,7 @@ Status MatchValidator::validateImpl() {
return Status::OK();
}

Status MatchValidator::validatePath(const MatchPath *path,
MatchClauseContext &matchClauseCtx) const {
Status MatchValidator::validatePath(const MatchPath *path, MatchClauseContext &matchClauseCtx) {
NG_RETURN_IF_ERROR(
buildNodeInfo(path, matchClauseCtx.nodeInfos, matchClauseCtx.aliasesGenerated));
NG_RETURN_IF_ERROR(
Expand All @@ -122,8 +121,7 @@ Status MatchValidator::validatePath(const MatchPath *path,
return Status::OK();
}

Status MatchValidator::buildPathExpr(const MatchPath *path,
MatchClauseContext &matchClauseCtx) const {
Status MatchValidator::buildPathExpr(const MatchPath *path, MatchClauseContext &matchClauseCtx) {
auto *pathAlias = path->alias();
if (pathAlias == nullptr) {
return Status::OK();
Expand All @@ -148,7 +146,7 @@ Status MatchValidator::buildPathExpr(const MatchPath *path,

Status MatchValidator::buildNodeInfo(const MatchPath *path,
std::vector<NodeInfo> &nodeInfos,
std::unordered_map<std::string, AliasType> &aliases) const {
std::unordered_map<std::string, AliasType> &aliases) {
auto *sm = qctx_->schemaMng();
auto steps = path->steps();
auto *pool = qctx_->objPool();
Expand Down Expand Up @@ -182,7 +180,7 @@ Status MatchValidator::buildNodeInfo(const MatchPath *path,
}
Expression *filter = nullptr;
if (props != nullptr) {
auto result = makeNodeSubFilter(props, "*");
auto result = makeNodeSubFilter(const_cast<MapExpression *>(props), "*");
NG_RETURN_IF_ERROR(result);
filter = result.value();
} else if (node->labels() != nullptr && !node->labels()->labels().empty()) {
Expand All @@ -204,7 +202,7 @@ Status MatchValidator::buildNodeInfo(const MatchPath *path,

Status MatchValidator::buildEdgeInfo(const MatchPath *path,
std::vector<EdgeInfo> &edgeInfos,
std::unordered_map<std::string, AliasType> &aliases) const {
std::unordered_map<std::string, AliasType> &aliases) {
auto *sm = qctx_->schemaMng();
auto steps = path->steps();
edgeInfos.resize(steps);
Expand Down Expand Up @@ -250,7 +248,7 @@ Status MatchValidator::buildEdgeInfo(const MatchPath *path,
}
Expression *filter = nullptr;
if (props != nullptr) {
auto result = makeEdgeSubFilter(props);
auto result = makeEdgeSubFilter(const_cast<MapExpression *>(props));
NG_RETURN_IF_ERROR(result);
filter = result.value();
}
Expand Down Expand Up @@ -521,32 +519,40 @@ Status MatchValidator::validateUnwind(const UnwindClause *unwindClause,
return Status::OK();
}

StatusOr<Expression *> MatchValidator::makeEdgeSubFilter(const MapExpression *map) const {
StatusOr<Expression *> MatchValidator::makeEdgeSubFilter(MapExpression *map) const {
auto *pool = qctx_->objPool();
DCHECK(map != nullptr);
auto &items = map->items();
DCHECK(!items.empty());

if (!ExpressionUtils::isEvaluableExpr(items[0].second)) {
return Status::SemanticError("Props must be constant: `%s'",
auto foldStatus = ExpressionUtils::foldConstantExpr(items[0].second);
NG_RETURN_IF_ERROR(foldStatus);
auto foldExpr = foldStatus.value();
if (!ExpressionUtils::isEvaluableExpr(foldExpr)) {
return Status::SemanticError("Props must be evaluable: `%s'",
items[0].second->toString().c_str());
}
map->setItem(0, std::make_pair(items[0].first, foldExpr));
Expression *root = RelationalExpression::makeEQ(
pool, EdgePropertyExpression::make(pool, "*", items[0].first), items[0].second->clone());
pool, EdgePropertyExpression::make(pool, "*", items[0].first), foldExpr);
for (auto i = 1u; i < items.size(); i++) {
if (!ExpressionUtils::isEvaluableExpr(items[i].second)) {
return Status::SemanticError("Props must be constant: `%s'",
foldStatus = ExpressionUtils::foldConstantExpr(items[i].second);
NG_RETURN_IF_ERROR(foldStatus);
foldExpr = foldStatus.value();
if (!ExpressionUtils::isEvaluableExpr(foldExpr)) {
return Status::SemanticError("Props must be evaluable: `%s'",
items[i].second->toString().c_str());
}
map->setItem(0, std::make_pair(items[i].first, foldExpr));
auto *left = root;
auto *right = RelationalExpression::makeEQ(
pool, EdgePropertyExpression::make(pool, "*", items[i].first), items[i].second->clone());
pool, EdgePropertyExpression::make(pool, "*", items[i].first), foldExpr);
root = LogicalExpression::makeAnd(pool, left, right);
}
return root;
}

StatusOr<Expression *> MatchValidator::makeNodeSubFilter(const MapExpression *map,
StatusOr<Expression *> MatchValidator::makeNodeSubFilter(MapExpression *map,
const std::string &label) const {
auto *pool = qctx_->objPool();
// Node has tag without property
Expand All @@ -562,20 +568,28 @@ StatusOr<Expression *> MatchValidator::makeNodeSubFilter(const MapExpression *ma
auto &items = map->items();
DCHECK(!items.empty());

if (!ExpressionUtils::isEvaluableExpr(items[0].second)) {
return Status::SemanticError("Props must be constant: `%s'",
auto foldStatus = ExpressionUtils::foldConstantExpr(items[0].second);
NG_RETURN_IF_ERROR(foldStatus);
auto foldExpr = foldStatus.value();
if (!ExpressionUtils::isEvaluableExpr(foldExpr)) {
return Status::SemanticError("Props must be evaluable: `%s'",
items[0].second->toString().c_str());
}
map->setItem(0, std::make_pair(items[0].first, foldExpr));
Expression *root = RelationalExpression::makeEQ(
pool, TagPropertyExpression::make(pool, label, items[0].first), items[0].second->clone());
pool, TagPropertyExpression::make(pool, label, items[0].first), foldExpr);
for (auto i = 1u; i < items.size(); i++) {
if (!ExpressionUtils::isEvaluableExpr(items[i].second)) {
return Status::SemanticError("Props must be constant: `%s'",
foldStatus = ExpressionUtils::foldConstantExpr(items[i].second);
NG_RETURN_IF_ERROR(foldStatus);
foldExpr = foldStatus.value();
if (!ExpressionUtils::isEvaluableExpr(foldExpr)) {
return Status::SemanticError("Props must be evaluable: `%s'",
items[i].second->toString().c_str());
}
map->setItem(i, std::make_pair(items[i].first, foldExpr));
auto *left = root;
auto *right = RelationalExpression::makeEQ(
pool, TagPropertyExpression::make(pool, label, items[i].first), items[i].second->clone());
pool, TagPropertyExpression::make(pool, label, items[i].first), foldExpr);
root = LogicalExpression::makeAnd(pool, left, right);
}
return root;
Expand Down
13 changes: 6 additions & 7 deletions src/graph/validator/MatchValidator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class MatchValidator final : public Validator {

AstContext *getAstContext() override;

Status validatePath(const MatchPath *path, MatchClauseContext &matchClauseCtx) const;
Status validatePath(const MatchPath *path, MatchClauseContext &matchClauseCtx);

Status validateFilter(const Expression *filter, WhereClauseContext &whereClauseCtx) const;

Expand Down Expand Up @@ -68,13 +68,13 @@ class MatchValidator final : public Validator {

Status buildNodeInfo(const MatchPath *path,
std::vector<NodeInfo> &edgeInfos,
std::unordered_map<std::string, AliasType> &aliases) const;
std::unordered_map<std::string, AliasType> &aliases);

Status buildEdgeInfo(const MatchPath *path,
std::vector<EdgeInfo> &nodeInfos,
std::unordered_map<std::string, AliasType> &aliases) const;
std::unordered_map<std::string, AliasType> &aliases);

Status buildPathExpr(const MatchPath *path, MatchClauseContext &matchClauseCtx) const;
Status buildPathExpr(const MatchPath *path, MatchClauseContext &matchClauseCtx);

Status combineAliases(std::unordered_map<std::string, AliasType> &curAliases,
const std::unordered_map<std::string, AliasType> &lastAliases) const;
Expand All @@ -89,10 +89,9 @@ class MatchValidator final : public Validator {

Status buildOutputs(const YieldColumns *yields);

StatusOr<Expression *> makeEdgeSubFilter(const MapExpression *map) const;
StatusOr<Expression *> makeEdgeSubFilter(MapExpression *map) const;

StatusOr<Expression *> makeNodeSubFilter(const MapExpression *map,
const std::string &label) const;
StatusOr<Expression *> makeNodeSubFilter(MapExpression *map, const std::string &label) const;

private:
std::unique_ptr<MatchAstContext> matchCtx_;
Expand Down
20 changes: 14 additions & 6 deletions src/graph/visitor/FoldConstantExprVisitor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,17 +346,25 @@ Expression *FoldConstantExprVisitor::fold(Expression *expr) {
auto value = expr->eval(ctx(nullptr));
if (value.type() == Value::Type::NULLVALUE) {
switch (value.getNull()) {
case NullType::DIV_BY_ZERO:
case NullType::DIV_BY_ZERO: {
canBeFolded_ = false;
status_ = Status::Error("/ by zero");
status_ = Status::SemanticError("Divide by 0");
break;
case NullType::ERR_OVERFLOW:
}
case NullType::ERR_OVERFLOW: {
canBeFolded_ = false;
status_ = Status::SemanticError("result of %s cannot be represented as an integer",
expr->toString().c_str());
break;
}
case NullType::BAD_TYPE: {
canBeFolded_ = false;
status_ = Status::Error("result of %s cannot be represented as an integer",
expr->toString().c_str());
status_ = Status::SemanticError("Type error `%s'", expr->toString().c_str());
break;
default:
}
default: {
break;
}
}
} else {
status_ = Status::OK();
Expand Down
12 changes: 6 additions & 6 deletions src/graph/visitor/test/FilterTransformTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ TEST_F(FilterTransformTest, TestCalculationOverflow) {
auto expr =
ltExpr(minusExpr(laExpr("v", "age"), constantExpr(1)), constantExpr(9223372036854775807));
auto res = ExpressionUtils::filterTransform(expr);
auto expected = Status::Error(
auto expected = Status::SemanticError(
"result of (9223372036854775807+1) cannot be represented as an "
"integer");
ASSERT(!res.status().ok());
Expand All @@ -39,7 +39,7 @@ TEST_F(FilterTransformTest, TestCalculationOverflow) {
{
auto expr = ltExpr(addExpr(laExpr("v", "age"), constantExpr(1)), constantExpr(INT64_MIN));
auto res = ExpressionUtils::filterTransform(expr);
auto expected = Status::Error(
auto expected = Status::SemanticError(
"result of (-9223372036854775808-1) cannot be represented as an "
"integer");
ASSERT(!res.status().ok());
Expand All @@ -50,7 +50,7 @@ TEST_F(FilterTransformTest, TestCalculationOverflow) {
auto expr = ltExpr(minusExpr(laExpr("v", "age"), constantExpr(1)),
addExpr(constantExpr(9223372036854775807), constantExpr(1)));
auto res = ExpressionUtils::filterTransform(expr);
auto expected = Status::Error(
auto expected = Status::SemanticError(
"result of (9223372036854775807+1) cannot be represented as an "
"integer");
ASSERT(!res.status().ok());
Expand All @@ -61,7 +61,7 @@ TEST_F(FilterTransformTest, TestCalculationOverflow) {
auto expr = ltExpr(addExpr(laExpr("v", "age"), constantExpr(1)),
minusExpr(constantExpr(INT64_MIN), constantExpr(1)));
auto res = ExpressionUtils::filterTransform(expr);
auto expected = Status::Error(
auto expected = Status::SemanticError(
"result of (-9223372036854775808-1) cannot be represented as an "
"integer");
ASSERT(!res.status().ok());
Expand All @@ -72,7 +72,7 @@ TEST_F(FilterTransformTest, TestCalculationOverflow) {
auto expr = notExpr(notExpr(notExpr(ltExpr(minusExpr(laExpr("v", "age"), constantExpr(1)),
constantExpr(9223372036854775807)))));
auto res = ExpressionUtils::filterTransform(expr);
auto expected = Status::Error(
auto expected = Status::SemanticError(
"result of (9223372036854775807+1) cannot be represented as an "
"integer");
ASSERT(!res.status().ok());
Expand All @@ -83,7 +83,7 @@ TEST_F(FilterTransformTest, TestCalculationOverflow) {
auto expr = notExpr(notExpr(
notExpr(ltExpr(addExpr(laExpr("v", "age"), constantExpr(1)), constantExpr(INT64_MIN)))));
auto res = ExpressionUtils::filterTransform(expr);
auto expected = Status::Error(
auto expected = Status::SemanticError(
"result of (-9223372036854775808-1) cannot be represented as an "
"integer");
ASSERT(!res.status().ok());
Expand Down
2 changes: 2 additions & 0 deletions src/parser/MatchSentence.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ class MatchNode final {

const MapExpression* props() const { return props_; }

MapExpression* props() { return props_; }

Comment on lines +168 to +169
Copy link
Contributor

Choose a reason for hiding this comment

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

Why delete const?

std::string toString() const;

private:
Expand Down
36 changes: 23 additions & 13 deletions src/storage/exec/IndexScanNode.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,20 +61,30 @@ std::string Path::encodeValue(const Value& value,
std::string& key) {
std::string val;
bool isNull = false;
if (colDef.get_type() == ::nebula::cpp2::PropertyType::GEOGRAPHY) {
CHECK_EQ(value.type(), Value::Type::STRING);
val = value.getStr();
} else if (value.type() == Value::Type::STRING) {
val = IndexKeyUtils::encodeValue(value, *colDef.get_type_length());
if (val.back() != '\0') {
strategySet_.insert(QualifiedStrategy::constant<QualifiedStrategy::UNCERTAIN>());
switch (colDef.get_type()) {
case ::nebula::cpp2::PropertyType::STRING:
case ::nebula::cpp2::PropertyType::FIXED_STRING: {
if (value.type() == Value::Type::NULLVALUE) {
val = IndexKeyUtils::encodeNullValue(Value::Type::STRING, colDef.get_type_length());
isNull = true;
} else {
val = IndexKeyUtils::encodeValue(value, *colDef.get_type_length());
if (val.back() != '\0') {
strategySet_.insert(QualifiedStrategy::constant<QualifiedStrategy::UNCERTAIN>());
}
}
break;
}
default: {
if (value.type() == Value::Type::NULLVALUE) {
auto vType = IndexKeyUtils::toValueType(colDef.get_type());
val = IndexKeyUtils::encodeNullValue(vType, colDef.get_type_length());
isNull = true;
} else {
val = IndexKeyUtils::encodeValue(value);
}
break;
}
} else if (value.type() == Value::Type::NULLVALUE) {
auto vtype = IndexKeyUtils::toValueType(colDef.get_type());
val = IndexKeyUtils::encodeNullValue(vtype, colDef.get_type_length());
isNull = true;
} else {
val = IndexKeyUtils::encodeValue(value);
}
// If the current colDef can be null, then it is necessary to additionally determine whether the
// corresponding value under a nullable is null when parsing the key (the encoding of the maximum
Expand Down
8 changes: 2 additions & 6 deletions tests/tck/features/expression/EndsWith.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,7 @@ Feature: Ends With Expression
"""
YIELD 123 ENDS WITH 3
"""
Then the result should be, in any order:
| (123 ENDS WITH 3) |
| BAD_TYPE |
Then a SemanticError should be raised at runtime: Type error `(123 ENDS WITH 3)'

Scenario: yield not ends with
When executing query:
Expand Down Expand Up @@ -118,9 +116,7 @@ Feature: Ends With Expression
"""
YIELD 123 NOT ENDS WITH 3
"""
Then the result should be, in any order:
| (123 NOT ENDS WITH 3) |
| BAD_TYPE |
Then a SemanticError should be raised at runtime: Type error `(123 NOT ENDS WITH 3)'

Scenario: ends with go
When executing query:
Expand Down
24 changes: 12 additions & 12 deletions tests/tck/features/expression/Null.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ Feature: NULL related operations
| NULL | NULL | NULL | NULL | NULL |
When executing query:
"""
RETURN cbrt(NULL) AS value1, hypot(NULL, NULL) AS value2, pow(NULL, NULL) AS value3, exp(NULL) AS value4, exp2(NULL) AS value5
RETURN cbrt(NULL) AS value1, exp(NULL) AS value4, exp2(NULL) AS value5
"""
Then the result should be, in any order:
| value1 | value2 | value3 | value4 | value5 |
| NULL | BAD_TYPE | BAD_TYPE | NULL | NULL |
| value1 | value4 | value5 |
| NULL | NULL | NULL |
When executing query:
"""
RETURN log(NULL) AS value1, log2(NULL) AS value2, log10(NULL) AS value3, sin(NULL) AS value4, asin(NULL) AS value5
Expand All @@ -38,18 +38,18 @@ Feature: NULL related operations
| NULL | NULL | NULL | NULL | NULL |
When executing query:
"""
RETURN cos(NULL) AS value1, acos(NULL) AS value2, tan(NULL) AS value3, atan(NULL) AS value4, rand32(NULL) AS value5
RETURN cos(NULL) AS value1, acos(NULL) AS value2, tan(NULL) AS value3, atan(NULL) AS value4
"""
Then the result should be, in any order:
| value1 | value2 | value3 | value4 | value5 |
| NULL | NULL | NULL | NULL | BAD_TYPE |
| value1 | value2 | value3 | value4 |
| NULL | NULL | NULL | NULL |
When executing query:
"""
RETURN collect(NULL) AS value1, avg(NULL) AS value2, count(NULL) AS value3, max(NULL) AS value4, rand64(NULL,NULL) AS value5
RETURN collect(NULL) AS value1, avg(NULL) AS value2, count(NULL) AS value3, max(NULL) AS value4
"""
Then the result should be, in any order:
| value1 | value2 | value3 | value4 | value5 |
| [] | NULL | 0 | NULL | BAD_TYPE |
| value1 | value2 | value3 | value4 |
| [] | NULL | 0 | NULL |
When executing query:
"""
RETURN min(NULL) AS value1, std(NULL) AS value2, sum(NULL) AS value3, bit_and(NULL) AS value4, bit_or(NULL,NULL) AS value5
Expand All @@ -59,8 +59,8 @@ Feature: NULL related operations
| NULL | NULL | 0 | NULL | NULL |
When executing query:
"""
RETURN bit_xor(NULL) AS value1, size(NULL) AS value2, range(NULL,NULL) AS value3, sign(NULL) AS value4, radians(NULL) AS value5
RETURN bit_xor(NULL) AS value1, size(NULL) AS value2, sign(NULL) AS value4, radians(NULL) AS value5
"""
Then the result should be, in any order:
| value1 | value2 | value3 | value4 | value5 |
| NULL | NULL | BAD_TYPE | NULL | NULL |
| value1 | value2 | value4 | value5 |
| NULL | NULL | NULL | NULL |
Loading