Skip to content

Fix: disable index scan for NOT_EQUAL operator (Fixes #620) #631

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

Open
zywoote3ture wants to merge 1 commit into
base: main
Choose a base branch
from
+20 −3
Open

Fix: disable index scan for NOT_EQUAL operator (Fixes #620) #631

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 20 additions & 3 deletions src/observer/sql/optimizer/physical_plan_generator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,12 @@ RC PhysicalPlanGenerator::create_plan(TableGetLogicalOperator &table_get_oper, u

Index *index = nullptr;
ValueExpr *value_expr = nullptr;
CompOp target_op = NO_OP;
for (auto &expr : predicates) {
if (expr->type() == ExprType::COMPARISON) {
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 ) {
Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

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

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.

Suggested change
if (comparison_expr->comp() != EQUAL_TO ) {
if (comparison_expr->comp() != EQUAL_TO && comparison_expr->comp() != NOT_EQUAL) {

Copilot uses AI. Check for mistakes.
continue;
}

Expand Down Expand Up @@ -161,12 +162,17 @@ RC PhysicalPlanGenerator::create_plan(TableGetLogicalOperator &table_get_oper, u
const Field &field = field_expr->field();
index = table->find_index_by_field(field.field_name());
if (nullptr != index) {
target_op = comparison_expr->comp();
// 找到索引把操作符记录下来
break;
}
}
}

if (index != nullptr) {

if (index != nullptr && target_op != NOT_EQUAL)
{
//修改条件：不等于操作不使用索引
ASSERT(value_expr != nullptr, "got an index but value expr is null ?");

const Value &value = value_expr->get_value();
Expand All @@ -181,16 +187,27 @@ RC PhysicalPlanGenerator::create_plan(TableGetLogicalOperator &table_get_oper, u
index_scan_oper->set_predicates(std::move(predicates));
oper = unique_ptr<PhysicalOperator>(index_scan_oper);
LOG_TRACE("use index scan");
} else {
} else
{
//对于不等于操作或者没有索引的情况，使用表扫描
auto table_scan_oper = new TableScanPhysicalOperator(table, table_get_oper.read_write_mode());
table_scan_oper->set_predicates(std::move(predicates));
oper = unique_ptr<PhysicalOperator>(table_scan_oper);
LOG_TRACE("use table scan");
if(target_op == NOT_EQUAL)
{
LOG_TRACE("not equal operation,use table scan instead of index scan");
Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

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

Missing space after comma in log message. Should be "not equal operation, use table scan instead of index scan" for proper formatting.

Suggested change
LOG_TRACE("not equal operation,use table scan instead of index scan");
LOG_TRACE("not equal operation, use table scan instead of index scan");

Copilot uses AI. Check for mistakes.
}
else
{
LOG_TRACE("use table scan");
}
Comment on lines 196 to +204
Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
}

return RC::SUCCESS;
}


Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

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

[nitpick] Unnecessary blank line added. This extra blank line is inconsistent with the rest of the file's formatting and should be removed.

Suggested change

Copilot uses AI. Check for mistakes.
RC PhysicalPlanGenerator::create_plan(PredicateLogicalOperator &pred_oper, unique_ptr<PhysicalOperator> &oper, Session* session)
{
vector<unique_ptr<LogicalOperator>> &children_opers = pred_oper.children();
Expand Down