morningman commented on PR #67900:
URL: https://github.com/apache/doris/pull/67900#issuecomment-5652620644
<!-- doris-repo-review:v1:begin -->
### Local pipeline review — ✅ PASS
```yaml
schema: doris-repo-review/v1
status: PASS
pr: apache/doris#67900
commit: 5181cbac469aa22570ed33fa6f099a165c24816a
base: efe7dec93d048e27b84edb0a3e2506e4cd872b53
reviewed_at: 2026-09-13T18:08+08:00
reviewer: morningman
model: claude-opus-5
effort: max
findings: {blocker: 0, major: 0, minor: 3, nit: 7}
rounds: 1
converged: true
```
**Notes for maintainers**
- `fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:626-632` —
F-02 (Minor): the base's
per-attempt `setReturnResultFromLocal(true)` at the top of
`execute(queryId)` is gone, and `queryRetry`
re-enters `execute(queryId)` without `beforeStatement`, so a replan
re-attempt on a Flight session that
fails before its coordinator runs with a stale `false` and
`StatementContext.close()` skips the connector
statement-scope close. One-line fix:
`context.getProtocolAdapter().beforeStatement(context)` before each re-attempt.
-
`fe/fe-core/src/main/java/org/apache/doris/mysql/protocol/MysqlProtocolAdapter.java:133-140`
— F-01 (Minor,
pre-existing): the `canRetryQuery` javadoc's "packets of a failed attempt
are dropped by the next reset" is
false for the master's `ProxyMysqlChannel` (`isSend` is never set,
`proxyResultBuffer` is never cleared), so a
forwarded query retried on the master after a mid-stream rpc failure
relays both attempts' packets. Same
condition in the base; the adapter is now the natural place to fix it
(clear the proxy buffer in a
`ProxyMysqlChannel.reset()` override).
-
`regression-test/suites/query_p0/test_mysql_forward_to_master.groovy:31-36` —
F-03 (Minor): the 2-FE docker
suite is skipped by every pipeline (`excludeDockerTest` defaults to true;
P0 "passed" it in 2 ms) and the PR
reports no local run, so the forwarded `COM_STMT_EXECUTE` cursor round
trip has no executed evidence. Please
run it once with `excludeDockerTest = false` and say so.
-
`fe/fe-core/src/main/java/org/apache/doris/qe/protocol/ProtocolAdapter.java:97-102`
— Nits: the
`beforeStatement` / `returnsResultFromLocal` contracts promise more than
the implementations deliver (2 MB
auto-flush; Flight keeps the previous statement's endpoints; registration
is released right away unless
deferred); details and the remaining test/description Nits are in the
local review documents.
<sub>Reviewed locally with the `doris-repo-review` pipeline. Repository
policy may accept this receipt for the matching commit; it is not a human
Apache approval.</sub>
<!-- 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]
