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 lookup in crash when the property has no corresponding filter #3986

Merged
merged 5 commits into from
Mar 15, 2022
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
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ StatusOr<TransformResult> OptimizeTagIndexScanByFilterRule::transform(
return TransformResult::noTransform();
} else {
// If the inner IN expr has only 1 element, rewrite it to an relEQ expression and there is
// no need to check wether it has a index
// no need to check whether it has a index
auto relEqExpr = graph::ExpressionUtils::rewriteInExpr(inExpr);
static_cast<LogicalExpression*>(transformedExpr)->setOperand(operandIndex, relEqExpr);
continue;
Expand Down
6 changes: 6 additions & 0 deletions src/graph/optimizer/rule/UnionAllIndexScanBaseRule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,12 @@ StatusOr<TransformResult> UnionAllIndexScanBaseRule::transform(OptContext* ctx,

// Reconstruct AND expr using distributive law
transformedExpr = graph::ExpressionUtils::rewriteLogicalAndToLogicalOr(transformedExpr);

// If there is no OR expr in the transformedExpr, only one index scan should be done.
// OptimizeTagIndexScanByFilterRule should be applied
if (!graph::ExpressionUtils::findAny(transformedExpr, {Expression::Kind::kLogicalOr})) {
return TransformResult::noTransform();
}
break;
}

Expand Down
6 changes: 6 additions & 0 deletions src/graph/util/ExpressionUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,12 @@ Expression *ExpressionUtils::rewriteInExpr(const Expression *expr) {

Expression *ExpressionUtils::rewriteLogicalAndToLogicalOr(const Expression *expr) {
DCHECK(expr->kind() == Expression::Kind::kLogicalAnd);

// If the given expression does not contain any inner logical OR expression, no need to rewrite
if (!findAny(expr, {Expression::Kind::kLogicalOr})) {
return const_cast<Expression *>(expr);
}

auto pool = expr->getObjPool();
auto logicalAndExpr = static_cast<LogicalExpression *>(expr->clone());
auto logicalAndExprSize = (logicalAndExpr->operands()).size();
Expand Down
3 changes: 2 additions & 1 deletion src/graph/util/ExpressionUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ class ExpressionUtils {
// Rewrites IN expression into OR expression or relEQ expression
static Expression* rewriteInExpr(const Expression* expr);

// Rewrites Logical AND expr to Logical OR expr using distributive law
// Rewrite Logical AND expr that contains Logical OR expr to Logical OR expr using distributive
// law
// Examples:
// A and (B or C) => (A and B) or (A and C)
// (A or B) and (C or D) => (A and C) or (A and D) or (B and C) or (B or D)
Expand Down
40 changes: 40 additions & 0 deletions tests/tck/features/bugfix/LookupIn.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# Copyright (c) 2022 vesoft inc. All rights reserved.
#
# This source code is licensed under Apache 2.0 License.
Feature: Lookup_In

# Lookup ... WHERE xxx IN xxx causes service crash
# Fix https://github.com/vesoft-inc/nebula/issues/3983
Scenario: lookup in where prop has no index
Given an empty graph
And load "nba" csv data to a new space
When executing query:
"""
DROP TAG INDEX IF EXISTS player_age_index
"""
Then the execution should be successful
When submit a job:
"""
REBUILD TAG INDEX
"""
Then wait the job to finish
When executing query:
"""
Show TAG INDEXES;
"""
Then the result should be, in any order:
| Index Name | By Tag | Columns |
| "bachelor_index" | "bachelor" | [] |
| "player_name_index" | "player" | ["name"] |
| "team_name_index" | "team" | ["name"] |
# player.age has no index
When executing query:
"""
LOOKUP ON player WHERE player.age IN [40, 20] AND player.name > "" YIELD id(vertex) as id, player.age
"""
Then the result should be, in any order:
| id | player.age |
| "Dirk Nowitzki" | 40 |
| "Luka Doncic" | 20 |
| "Kobe Bryant" | 40 |
Then drop the used space