Fix: disable index scan for NOT_EQUAL operator (Fixes #620)#631
Fix: disable index scan for NOT_EQUAL operator (Fixes #620)#631zywoote3ture wants to merge 1 commit intooceanbase:mainfrom
Conversation
Signed-off-by: zywoote3ture <3355324252@qq.com>
There was a problem hiding this comment.
Pull request overview
This pull request fixes issue #620 by disabling index scan for the NOT_EQUAL (<>) operator in query optimization. The change ensures that queries using NOT_EQUAL operations fall back to table scan instead of using index scan, which was producing incorrect results.
Key Changes:
- Modified the comparison operator filter to exclude NOT_EQUAL from index optimization consideration
- Added tracking of the target comparison operator to detect NOT_EQUAL operations
- Added conditional check to prevent index usage when NOT_EQUAL is detected
- Added diagnostic logging for NOT_EQUAL operations that use table scan
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| auto comparison_expr = static_cast<ComparisonExpr *>(expr.get()); | ||
| // 简单处理，就找等值查询 | ||
| if (comparison_expr->comp() != EQUAL_TO && comparison_expr->comp() != NOT_EQUAL) { | ||
| if (comparison_expr->comp() != EQUAL_TO ) { |
There was a problem hiding this comment.
The logic here is problematic. The code now only allows EQUAL_TO comparisons to proceed, but then later checks if target_op == NOT_EQUAL (line 165). This creates a logical inconsistency: if NOT_EQUAL comparisons are filtered out here with continue, then target_op can never be set to NOT_EQUAL at line 165.
The original code allowed both EQUAL_TO and NOT_EQUAL to proceed, which made sense if the intent is to detect NOT_EQUAL operations and disable index usage for them. This change breaks that detection logic.
| if (comparison_expr->comp() != EQUAL_TO ) { | |
| if (comparison_expr->comp() != EQUAL_TO && comparison_expr->comp() != NOT_EQUAL) { |
| LOG_TRACE("use table scan"); | ||
| if(target_op == NOT_EQUAL) | ||
| { | ||
| LOG_TRACE("not equal operation,use table scan instead of index scan"); | ||
| } | ||
| else | ||
| { | ||
| LOG_TRACE("use table scan"); | ||
| } |
There was a problem hiding this comment.
Duplicated and redundant logging logic. This code logs "use table scan" unconditionally at line 196, then has a conditional block (lines 197-204) that logs either a specific message for NOT_EQUAL or "use table scan" again. This results in duplicate "use table scan" messages being logged for most cases.
Recommend removing the LOG_TRACE at line 196 and keeping only the conditional logging block, or simplifying to just use the conditional logging.
| return RC::SUCCESS; | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
[nitpick] Unnecessary blank line added. This extra blank line is inconsistent with the rest of the file's formatting and should be removed.
| LOG_TRACE("use table scan"); | ||
| if(target_op == NOT_EQUAL) | ||
| { | ||
| LOG_TRACE("not equal operation,use table scan instead of index scan"); |
There was a problem hiding this comment.
Missing space after comma in log message. Should be "not equal operation, use table scan instead of index scan" for proper formatting.
| LOG_TRACE("not equal operation,use table scan instead of index scan"); | |
| LOG_TRACE("not equal operation, use table scan instead of index scan"); |
1 participant
What problem were solved in this pull request?
Issue Number: close #620
Problem:
What is changed and how it works?
Other information