diff --git a/src/graph/optimizer/rule/OptimizeTagIndexScanByFilterRule.cpp b/src/graph/optimizer/rule/OptimizeTagIndexScanByFilterRule.cpp index 77e75735485..b06a149c0aa 100644 --- a/src/graph/optimizer/rule/OptimizeTagIndexScanByFilterRule.cpp +++ b/src/graph/optimizer/rule/OptimizeTagIndexScanByFilterRule.cpp @@ -83,15 +83,7 @@ bool OptimizeTagIndexScanByFilterRule::match(OptContext* ctx, const MatchedResul } // Case2: logical AND expr - if (condition->kind() == ExprKind::kLogicalAnd) { - // for (auto operand : static_cast(condition)->operands()) { - // if (operand->kind() == ExprKind::kRelIn) { - // return false; - // } - // } - return true; - } - return false; + return condition->kind() == ExprKind::kLogicalAnd; } TagIndexScan* makeTagIndexScan(QueryContext* qctx, const TagIndexScan* scan, bool isPrefixScan) { diff --git a/src/graph/util/ExpressionUtils.cpp b/src/graph/util/ExpressionUtils.cpp index 240513c9e92..932a638d561 100644 --- a/src/graph/util/ExpressionUtils.cpp +++ b/src/graph/util/ExpressionUtils.cpp @@ -248,7 +248,7 @@ Expression *ExpressionUtils::rewriteLogicalAndToLogicalOr(const Expression *expr } // orExprOperands is a 2D vector where each sub-vector is the operands of AND expression. - // [[A, C], [A, D], [B, C], [B,D]] => (A and C) or (A and D) or (B and C) or (B or D) + // [[A, C], [A, D], [B, C], [B,D]] => (A and C) or (A and D) or (B and C) or (B and D) std::vector andExprList; andExprList.reserve(orExprOperands.size()); for (auto &operand : orExprOperands) { diff --git a/src/graph/util/test/ExpressionUtilsTest.cpp b/src/graph/util/test/ExpressionUtilsTest.cpp index 95c3033ad26..fa6963000b8 100644 --- a/src/graph/util/test/ExpressionUtilsTest.cpp +++ b/src/graph/util/test/ExpressionUtilsTest.cpp @@ -494,6 +494,81 @@ TEST_F(ExpressionUtilsTest, flattenInnerLogicalExpr) { } } +TEST_F(ExpressionUtilsTest, rewriteInExpr) { + auto elist1 = ExpressionList::make(pool); + (*elist1).add(ConstantExpression::make(pool, 10)).add(ConstantExpression::make(pool, 20)); + auto listExpr1 = ListExpression::make(pool, elist1); + + auto elist2 = ExpressionList::make(pool); + (*elist2).add(ConstantExpression::make(pool, "a")).add(ConstantExpression::make(pool, "b")); + auto listExpr2 = ListExpression::make(pool, elist2); + + auto elist3 = ExpressionList::make(pool); + (*elist3).add(ConstantExpression::make(pool, 100)); + auto listExpr3 = ListExpression::make(pool, elist3); + + // a IN [b,c] -> a==b OR a==c + { + auto inExpr1 = + RelationalExpression::makeIn(pool, ConstantExpression::make(pool, 10), listExpr1); + auto orExpr1 = ExpressionUtils::rewriteInExpr(inExpr1); + auto expected1 = LogicalExpression::makeOr( + pool, + RelationalExpression::makeEQ( + pool, ConstantExpression::make(pool, 10), ConstantExpression::make(pool, 10)), + RelationalExpression::makeEQ( + pool, ConstantExpression::make(pool, 10), ConstantExpression::make(pool, 20))); + ASSERT_EQ(*expected1, *orExpr1); + + auto inExpr2 = + RelationalExpression::makeIn(pool, ConstantExpression::make(pool, "abc"), listExpr2); + auto orExpr2 = ExpressionUtils::rewriteInExpr(inExpr2); + auto expected2 = LogicalExpression::makeOr( + pool, + RelationalExpression::makeEQ( + pool, ConstantExpression::make(pool, "abc"), ConstantExpression::make(pool, "a")), + RelationalExpression::makeEQ( + pool, ConstantExpression::make(pool, "abc"), ConstantExpression::make(pool, "b"))); + ASSERT_EQ(*expected2, *orExpr2); + } + + // a IN [b] -> a == b + { + auto inExpr = RelationalExpression::makeIn(pool, ConstantExpression::make(pool, 10), listExpr3); + auto expected = RelationalExpression::makeEQ( + pool, ConstantExpression::make(pool, 10), ConstantExpression::make(pool, 100)); + ASSERT_EQ(*expected, *ExpressionUtils::rewriteInExpr(inExpr)); + } +} + +TEST_F(ExpressionUtilsTest, rewriteLogicalAndToLogicalOr) { + auto orExpr1 = LogicalExpression::makeOr( + pool, ConstantExpression::make(pool, 10), ConstantExpression::make(pool, 20)); + auto orExpr2 = LogicalExpression::makeOr( + pool, ConstantExpression::make(pool, "a"), ConstantExpression::make(pool, "b")); + + // (a OR b) AND (c OR d) -> (a AND c) OR (a AND d) OR (b AND c) OR (b AND d) + { + auto andExpr = LogicalExpression::makeAnd(pool, orExpr1, orExpr2); + auto transformedExpr = ExpressionUtils::rewriteLogicalAndToLogicalOr(andExpr); + + std::vector orOperands = { + LogicalExpression::makeAnd( + pool, ConstantExpression::make(pool, 10), ConstantExpression::make(pool, "a")), + LogicalExpression::makeAnd( + pool, ConstantExpression::make(pool, 10), ConstantExpression::make(pool, "b")), + LogicalExpression::makeAnd( + pool, ConstantExpression::make(pool, 20), ConstantExpression::make(pool, "a")), + LogicalExpression::makeAnd( + pool, ConstantExpression::make(pool, 20), ConstantExpression::make(pool, "b"))}; + + auto expected = LogicalExpression::makeOr(pool); + expected->setOperands(orOperands); + + ASSERT_EQ(*expected, *transformedExpr); + } +} + TEST_F(ExpressionUtilsTest, splitFilter) { using Kind = Expression::Kind; { diff --git a/src/graph/validator/LookupValidator.cpp b/src/graph/validator/LookupValidator.cpp index 97df8d7a0bd..6f9b56cd1d2 100644 --- a/src/graph/validator/LookupValidator.cpp +++ b/src/graph/validator/LookupValidator.cpp @@ -129,10 +129,6 @@ StatusOr LookupValidator::handleLogicalExprOperands(LogicalExpressi auto& operands = lExpr->operands(); for (auto i = 0u; i < operands.size(); i++) { auto operand = lExpr->operand(i); - // if (operand->isLogicalExpr()) { - // // Not allow different logical expression to use: A AND B OR C - // return Status::SemanticError("Not supported filter: %s", lExpr->toString().c_str()); - // } auto ret = checkFilter(operand); NG_RETURN_IF_ERROR(ret);