Repository navigation
[fix](load) Restrict load counter updates to load source scanners - #82
Conversation
### What problem does this PR solve? Issue Number: None Related PR: apache#63781 Problem Summary: Query-side scanner predicates could update load filtered and unselected counters when profiling was enabled, causing INSERT INTO SELECT to report incorrect affected rows or fail the filter ratio check. Restrict load counter updates to load-source scanners while preserving FILE_STREAM accounting for HTTP stream and group commit loads. ### Release note Fix incorrect affected rows reported by INSERT INTO SELECT when enable_profile is enabled. ### Check List (For Author) - Test: Regression test added but not run per requested validation scope - Behavior changed: Yes. Query-side filtered rows no longer pollute load counters. - Does this need documentation: No
xuchenhao
left a comment
xuchenhao
left a comment
There was a problem hiding this comment.
审查结论：未发现阻断性问题（no blocking findings），也没有确认的 P0–P3 finding。
已检查的关键点：
- 追踪了
VScanner的全部直接子类、构造与关闭/profile 收集路径。经典 load 仍由VFileScanner的_is_load判定计数；NewOlapScanner等 query-side scanner 不再污染 load counter，且其 override 仍正确调用基类。 - 核对了
VFileScanner::_should_update_load_counters()的FILE_STREAM分支：保留http_stream/ group commit load source 在_is_load == false时的num_rows_unselected语义，同时普通 file TVF/query scan 不会被误计入。 - 追踪了
RuntimeState的 total/filtered/unselected/success 计算及 stream load、group commit、tablet writer、fragment report 等消费方；新的 gate 与这些消费方对 total = success + filtered + unselected 的约定一致。 - 与已合并的上游
apache/doris#63781最终实现逐项对照，生产代码语义一致；回归用例按 branch-3.1 的测试约定适配，并覆盖 profile-enabledINSERT INTO ... SELECT的原始触发路径以及 DELETE/UPDATE no-op 路径。
静态验证：git diff --check origin/branch-3.1...HEAD 通过；四处 C++ 变更行的 clang-format --dry-run --Werror 通过；测试查询名与 .out 段落一一对应。按本次审查范围未编译 Doris，也未运行需要集群的 regression test。
|
Reviewed the load-counter gating. Query-side scanners no longer contribute filtered/unselected rows to load accounting, while VFileScanner preserves FILE_STREAM accounting for stream/group-commit load sources. The regression coverage matches the INSERT/DELETE/UPDATE no-op cases. No blocking issue found; please run the added regression suite before merge. |
Ryan19929
left a comment
Ryan19929
left a comment
There was a problem hiding this comment.
No P1/P2 issues found. LGTM.
Reviewed PR #82 at head e32a83ccc2 against branch-3.1, comparing against the upstream fix (apache#63781). The backport gates RuntimeState load filtered/unselected counter updates behind _should_update_load_counters(), so query-side scanners (OlapScan, plain file/TVF scans) in INSERT-SELECT/DELETE/UPDATE no longer drive num_rows_load_success() negative when enable_profile=true, while classic loads (_is_load) and FILE_STREAM TVF loads (http_stream/group commit) keep reporting. Verified the _is_load/_params initialization, the thrift optional/__isset usage, all _collect_profile_before_close overrides and its single invocation site, the FE FILE_STREAM producers, and the sink-side total/filtered/unselected accounting convention — the backport is faithful to upstream and internally consistent. The regression suite deterministically reproduces the original failure and now also asserts exact affected-row counts; prior review comments are addressed, and the post-review delta is test-only.
xuchenhao
left a comment
xuchenhao
left a comment
There was a problem hiding this comment.
LGTM。基于当前 head e32a83ccc2476311da77264f1867022f665df725 复审完整 diff，并单独核对旧 review 后新增的 24 行 affected-rows 断言，未发现需要修复的 P0–P3 问题。
按 code-review Part 1.3 的检查结论：
- 目标：仅真实 load source scanner 更新 filtered/unselected counters，query 过滤不再污染 INSERT 结果；profile-enabled no-op INSERT 覆盖原触发路径。
- 范围：四处生产文件的计数 gate 聚焦，虚方法/override 职责明确。
- 并发：scanner 私有 counter/range、RuntimeState 原子累加及现有调度锁不变，没有新增锁顺序。
- 生命周期：params 随 query/split source 存活，EOS 汇总后不重调度，close 不重复汇总；无新资源分配。
- 配置：无新增配置，profile 不再决定 load counter 的业务归属。
- 兼容性：仅 BE 内部方法，无协议/存储格式变化；兼容 params/range 两处 FILE_STREAM 标记。
- 平行路径：所有 VScanner 子类、常规/TVF load、HTTP stream、独立 group-commit 队列、两版 writer 及 FE 消费公式已核对。
- 条件：
_is_load || FILE_STREAM有明确 planner/StreamLoad 来源，普通 file TVF 不命中例外。 - 测试：SQL 返回值确实来自 JDBC getUpdateCount，落表行数另行检查；四个 qt 标签与 out 对应。部分过滤 5 行及 DELETE/UPDATE 在 profile=false 下属于兼容性覆盖，仍可补 profile=true 组合；未执行 HTTP/group-commit 集成测试。
- 可观测性：scan profile 仍正常更新，load counters 恢复业务含义。
- 持久化：事务状态机、EditLog、failover 不变，仅影响既有校验消费的计数。
- 写入/原子性：不改变行内容、tablet 写入/提交和资源释放。
- FE/BE 传参：无新跨端变量。
- 性能：scanner 收尾 O(1) 判断，无逐行虚调用、重复扫描或新分配。
- 其他：未发现遗漏 Status、异常边界或其他高置信缺陷。
本轮完成完整/增量 git diff --check、clang-format 20.1.8 对四处 C++ 修改范围的 dry-run/Werror 检查及调用链静态审查，工作树干净。独立 worktree 缺少初始化脚本/标记、独立依赖和构建产物，未编译或运行 BE UT/集群 regression；作者的 integration 记录不计作本轮执行。当前 5 个 checks 成功，2 个 Sonar 失败：C++ 无法解析 action repository，Java 在 Setup thrift exit 100，分析阶段均未执行。
静态 Code Review：fix Restrict load counter updates to load source scanners
总体结论✅ LGTM，建议合入（附 1 个低优先级边界说明）。 Bug 机理确认基线 修复将门控下沉为虚函数 VFileScanner 覆盖的特例http_stream / group commit relay 等 TVF 型 load 以查询 scan 形态规划、 边界说明（低）
测试
其他检查点
|
Proposed changes
Issue Number: None
Related PR: apache#63781
Query-side scanner predicates could update RuntimeState load filtered/unselected counters when profiling was enabled. For INSERT INTO SELECT, this polluted loadedRows and could produce an incorrect affected-row count or a negative success-row count.
This PR backports the upstream fix to the branch-3.1 VScanner/VFileScanner hierarchy:
Release note
Fix incorrect affected rows reported by INSERT INTO SELECT when enable_profile is enabled.
Check List (For Author)
Integration Test Evidence (2026-08-24/25)
All PRs in the 3.1.4 bugfix batch were validated together on integration branch
test/3.1.4-bugfix-integration(commit94ad56237b5): full BE+FE Release build, single-node cluster deploy, version verifieddoris-3.1.4-hydcp-1.2-rc02-94ad56237b5with BE Alive=true.PR-specific results:
load_p0/insert/test_scan_filtered_rows_not_pollute_load_counterexecuted and passed on the integration cluster.load_p0/insertdirectory suites passed (the only failure,test_insert_move_memtable, is a pre-existing broken suite unrelated to this PR; removal proposed in [test](regression) Remove broken test_insert_move_memtable suite #87).Remaining gap: returned affected rows not directly asserted; FILE_STREAM/http_stream/group-commit paths not covered.
Review Follow-up (2026-08-25)
Extended
test_scan_filtered_rows_not_pollute_load_counter(commite32a83ccc2) with direct affected-rows assertions: a partially filtered insert must report exactly the loaded row count (5), and a no-op insert with profile enabled must report 0 affected rows. Verified passing on the integration branch cluster. FILE_STREAM/http_stream/group-commit paths remain uncovered.