csun5285 commented on PR #70162:
URL: https://github.com/apache/doris/pull/70162#issuecomment-6627121352
<!-- doris-repo-review:v1:begin -->
```yaml
schema: doris-repo-review/v1
status: PASS
pr: apache/doris#70162
commit: f2d7c487dd3c671e3b1f78607310a865aa60dcd5
base: c0274bfb4accd3f013b320704d55919f7ae46c2e
reviewed_at: 2026-11-04T09:10:35+00:00
reviewer: csun5285
model: gpt-6-astra
effort: xhigh
findings: {blocker: 0, major: 0, minor: 0, nit: 0}
rounds: 2
converged: true
```
**Local code review: PASS.** Reviewed all four changed files and the
relevant binding, rewrite, prepared-execution, point-query, and normal
storage-reader paths at the commit above. Three independent review passes
covered execution semantics, tests, and slot/projection edge cases; the main
reviewer independently checked their evidence. The final convergence round
found no additional actionable findings. No existing inline findings required
deduplication.
The fix checks hidden **input dependencies** in both supported plan shapes:
```text
ResultSink ResultSink(hidden slots)
Project(hidden expression) Filter(key = literal)
Filter(key = literal) OlapScan
OlapScan
```
For hidden-column reads, neither shape sets the short-circuit flag, so
execution uses the normal reader that resolves singleton-rowset
version/commit-TSO placeholders. Checking the Project's input slots catches
hidden columns inside aliases and expressions. Checking the project-free Sink's
inputs does not include the internal delete-sign filter, preserving ordinary
visible-column point queries. Binding preserves column visibility, and the
analyzed shapes provide the `SlotReference` inputs required by the cast. The
guard runs before partition pruning; prepared execution cannot cache a
hidden-column shortcut rejected by this rule.
<details>
<summary>Required review checkpoints</summary>
| Checkpoint | Conclusion |
| --- | --- |
| Goal and proof | The FE guard addresses the JSONB placeholder limitation.
Added unit tests check routing; regression tests check routing and corrected
version/TSO results. |
| Scope and clarity | One shared predicate, two matching-rule conditions,
and focused tests; no unrelated production changes. |
| Concurrency and locks | No new threads, shared mutable production state,
lock operations, or I/O. |
| Lifecycle | Statement flags and prepared-context publication/reuse remain
consistent. No new owned resources or static initialization. |
| Configuration | No production configuration added. Test changelog setup
matches the P0/cloud-P0 environment; FE test classes use separate JVMs
(`reuseForks=false`). |
| Compatibility | No protocol, storage-format, or public API changes; this
changes FE execution-path selection. |
| Parallel paths | Both Project and project-free Sink patterns are covered.
Ordinary and prepared execution and shared normal-reader behavior were
inspected. |
| Conditional checks | Hidden-output dependency checks match the known
row-store limitation; filter-only version/TSO predicates already fail shortcut
admission. |
| Test coverage | Hidden version, TSO, arithmetic expression, delete-sign
projection, hidden-star expansion, and visible-column fast paths are covered.
No dedicated prepared-statement runtime case was added; that path was checked
statically. |
| Expected results | First insert version `2`, positive commit TSO, and
visible row `(1, 7)` match the implementation. Each result has at most one row,
so ordering is deterministic. |
| Observability | Existing EXPLAIN `SHORT-CIRCUIT` presence/absence exposes
the routing decision and is asserted by the regression. |
| Transactions and persistence | No EditLog, transaction, publish, or
persisted-state logic changed. |
| Writes and crash behavior | No data-write or ownership changes; no new
atomicity or cleanup path. |
| FE/BE parameter propagation | No transmitted fields or new sending paths. |
| Performance | Input-slot inspection runs during planning. Visible-column
queries retain the fast path; hidden-column queries intentionally use the
normal engine. |
| Remaining code concerns | No verified defects or unresolved code
candidates after the final sweep. |
</details>
**Validation and limits:** This was a local static review; no Doris build,
FE unit test, or regression test was rerun locally, and no source files were
changed. Existing status results for this exact head report success for
[COMPILE](http://43.132.222.7:8111/viewLog.html?buildId=1091587), [FE
UT](http://43.132.222.7:8111/viewLog.html?buildId=1091586), [P0
Regression](http://43.132.222.7:8111/viewLog.html?buildId=1091619), and
[cloud_p0](http://43.132.222.7:8111/viewLog.html?buildId=1091621). These status
results are not claimed as local test execution or proof of individual test
selection.
The separate [FE coverage
check](http://43.132.222.7:8111/viewLog.html?buildId=1092117) is still failed.
Its cause was not verified because the available TeamCity credential was
rejected. This PASS applies to code review and does not resolve that coverage
failure. The automated review failures reported model-capacity errors ([first
run](https://github.com/apache/doris/actions/runs/41765864196), [second
run](https://github.com/apache/doris/actions/runs/41783647830)).
<!-- doris-repo-review:v1:end -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
