morningman opened a new pull request, #67883:
URL: https://github.com/apache/doris/pull/67883
### What problem does this PR solve?
Issue Number: #67577
Related PR: #67835 (`ProtocolAdapter`), #67866 (package move), #67789
(protocol goldens)
Problem Summary:
Second step of the protocol-independent session work (#67577, stage 1).
#67835 moved the *connection* half of the two wire protocols behind
`ProtocolAdapter`; the *result* half was still spread over `StmtExecutor` and
`ConnectProcessor` as `ConnectType` branches, a `MysqlSerializer` field,
`MysqlChannel` parameters and three copies of "send a text result set".
```
+---------------------------+
+-----------------------------+
| MySQL client | | Arrow Flight
SQL client |
+-------------+-------------+
+--------------+--------------+
| |
v v
+---------------------------+
+-----------------------------+
| MysqlServer | |
DorisFlightSqlProducer |
| AcceptListener | | every call
goes through |
| ReadListener | |
adapter.runCommand() |
+-------------+-------------+
+--------------+--------------+
| |
v v
+---------------------------+
+-----------------------------+
| MysqlConnectProcessor | |
FlightSqlConnectProcessor |
| COM_FIELD_LIST, and | |
|
| finalizeCommand() = | |
|
| adapter.finishCommand() | |
|
+-------------+-------------+
+--------------+--------------+
| |
+----------------------------+-----------------------------+
|
v
+-------------------------------------------------------------------------------------------+
| ConnectProcessor.executeQuery -- parse, one StmtExecutor per
statement, audit |
| for each statement: executor.execute()
|
| adapter.finishStatement(ctx, executor, i, n)
<-- NEW |
+---------------------------------------------+---------------------------------------------+
|
v
+-------------------------------------------------------------------------------------------+
| StmtExecutor -- plans and runs one statement, protocol-agnostic result
path |
|
|
| FE-side result (SHOW / EXPLAIN / REPLAY / dry run / FE-computable
SELECT / forwarded): |
| sendResultSet(rs) -> sender.sendResultSet(rs, fieldInfos,
binaryRows) |
| BE result stream:
|
| sender.sendFields(...) then sender.sendRow(row) per row
|
| internal executor streaming to the caller's client:
|
| executeInternalQueryAndSend(plan, callerCtx.getResultSender())
|
|
|
| no MysqlSerializer field, no MysqlChannel parameter, no ConnectType
branch on this path |
+---------------------------------------------+---------------------------------------------+
|
v
+-------------------------------------------------------------------------------------------+
| ConnectContext -- the session, one per connection
|
| protocolAdapter : ProtocolAdapter getResultSender() =
adapter.resultSender(this)|
+---------------------------------------------+---------------------------------------------+
|
v
+-----------------------------------------------+
| <<interface>> qe.protocol.ProtocolAdapter |
| type() remoteHostPortString(ctx) |
| resultSinkType() connectPool(scheduler) |
| afterStatement(ctx) closeConnection(ctx) |
| resultSender(ctx) <-- NEW |
| supportsSqlCacheReplay() <-- NEW |
| finishStatement(ctx, executor, i, n) NEW |
+-----------------------+-----------------------+
|
+--------------------+----------------------------+
| |
+-----------------------+----------------------+
+-----------------------+----------------------+
| mysql.protocol.MysqlProtocolAdapter | |
arrowflight.protocol.FlightProtocolAdapter |
| (unchanged state: channel, capability, | | (unchanged state:
result cache, endpoints, |
| handshake, SSL, execute packet, cursor) | | deferred executors,
command lock) |
| finishStatement: SERVER_MORE_RESULTS_EXISTS| | finishStatement:
carry the outcome of a |
| + flush the intermediate response when | | statement
forwarded to the master; only |
| CLIENT_MULTI_STATEMENTS | | the last statement
may return a result |
| finishCommand: OK/EOF/ERR or the master's | |
|
| packets; responsePacket(ctx) | |
|
+-----------------------+----------------------+
+-----------------------+----------------------+
| |
v v
+----------------------------------------------+
+----------------------------------------------+
| <<interface>> qe.protocol.ResultSender (NEW) | |
|
| sendResultSet(rs, fieldInfos, binaryRows) | |
|
| sendFields(names, fieldInfos, types) | |
|
| sendRow(wireRow) | |
|
| reset() | |
|
+----------------------------------------------+
+----------------------------------------------+
| mysql.protocol.MysqlResultSender | |
arrowflight.protocol.FlightResultSender |
| column count + column definitions + | | caches the ResultSet
as Utf8 vectors under |
| terminator (EOF / cursor OK) + text or | | the query id for
the client's DoGet |
| binary rows, through the channel's | | sendFields/sendRow:
never called, the |
| serializer; a raw row passes through | | client pulls BE
results from the BE |
| MySQL-only: sendStmtPrepareOK, | | reset: nothing
pending |
| sendFieldList | |
|
+----------------------------------------------+
+----------------------------------------------+
```
**`ResultSender`** (`qe.protocol`, interface; implementations in
`mysql.protocol` and `arrowflight.protocol`, mirroring the adapters): how a
statement's result reaches the client. Four operations, all of them already
used: `sendResultSet` for a result the frontend materialized, `sendFields` +
`sendRow` for a backend result stream, `reset` for what `MysqlChannel.reset()`
did at the start of a query. `MysqlResultSender` is the old `sendMetaData /
sendFields / sendTextResultRow / sendBinaryResultRow /
sendMetadataTerminatorIfNeeded / sendStmtPrepareOK` of `StmtExecutor` plus the
`COM_FIELD_LIST` body of `ConnectProcessor`, moved without changes to the byte
layout; it uses the channel's serializer, so the executor's `serializer` field
is gone. `FlightResultSender` wraps `FlightSqlChannel.addResult` (every column
still `Utf8`, typing them is stage 2).
**`StmtExecutor`** no longer takes a `MysqlChannel`: `executeAndSendResult`,
`sendCachedValues` and `executeInternalQueryAndSend` take a `ResultSender`. The
three "text result" methods (`handleExplainStmt`, `handleReplayStmt`,
`handleExplainPlanProcessStmt`) build a `ShowResultSet` and go through the one
`sendResultSet`, which fixes `EXPLAIN PLAN PROCESS` returning nothing on an
Arrow Flight SQL session (it had no Flight branch). The `MysqlChannel`
overloads #67753 added for the IVM dry run become "hand the internal executor
the caller's sender": `RefreshMTMVCommand` passes `ctx.getResultSender()`, and
the internal executor's rows are encoded with the caller's negotiated
capabilities instead of the internal context's defaults.
**`ConnectProcessor`** loses its `connectType` field and every branch on it.
The per-statement protocol work of a multi-statement request is one call,
`adapter.finishStatement(ctx, executor, i, n)`: for MySQL it sets
`SERVER_MORE_RESULTS_EXISTS` and flushes the intermediate response when the
client negotiated `CLIENT_MULTI_STATEMENTS`; for Flight it carries a forwarded
statement's outcome into the session (the former
`carryForwardedOutcomeToFlightSession`) and enforces "only the last statement
may return a result". `finalizeCommand` / `getResultPacket` move to
`MysqlProtocolAdapter.finishCommand` / `responsePacket` and `COM_FIELD_LIST` to
`MysqlConnectProcessor`, the only processor that dispatches it. The SQL-cache
guard is `adapter.supportsSqlCacheReplay()` (true only for MySQL, whose wire
rows the cache stores).
Also folded in, from the local review of #67835:
`FlightProtocolAdapter.acquireCommandLock` waits up to `getExecTimeoutS()` (a
sync load may legitimately run past `query_timeout`) and logs when it gives up;
`getFlightInfoStatement` / `streamMetadata` pass a `FlightRuntimeException`
through instead of re-wrapping `UNAVAILABLE` as `INTERNAL`; `of(ctx)` names a
null adapter instead of throwing NPE from the error path;
`testFailedCommandReleasesTheSession` checks the lock from a second thread;
`ConnectProcessorForwardProtocolTest` binds its recording channel through the
adapter instead of overriding `getMysqlChannel()`.
Not in this PR (next one, PR-1.3): the remaining `ConnectType` branches
outside the result path (`returnResultFromLocal`, the Flight early return in
`executeAndSendResult`, the retry condition, `supportHandleByFe`, the nine
`getMysqlChannel().reset()` in insert / transaction commands, `FEOpExecutor`,
the coordinators), which become capability bits on the adapter.
### Release note
`EXPLAIN PLAN PROCESS ...` over Arrow Flight SQL now returns the Rule /
Before / After rows like it does over MySQL, instead of an empty
`StatusResult=0`.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
`MysqlPacketGoldenTest` (28 cases, byte for byte) is unchanged;
`FlightResultGoldenTest` changes in exactly one entry, `explain plan process
select 1`, from "no result" to the three-column result (first commit records
the baseline, second commit changes it). New `MysqlResultSenderTest` (the six
encoding cases moved out of `StmtExecutorTest` plus prepare / field list / raw
row / reset), `FlightResultSenderTest`, `FlightForwardedOutcomeTest` (moved
from `ConnectProcessorFlightForwardOutcomeTest`), and multi-statement /
response-packet / client-address cases in the two adapter tests. Local
regression (single FE + BE built from this branch): `arrow_flight_sql_p0` 8/8,
`prepared_stmt_p0` 6/6, `load_p0/mysql_load` 7/7 (LOAD DATA LOCAL still reads
the channel directly), `point_query_p0` 16/16 (short-circuit `sendFields` with
the pre-serialized column definitions), `query_p0/cache` 12/12 (SQL cache
replay, `explain plan process`), `query_p0/dry_run` 1/1, `query_p0/explain`
9/9, `query
_p0/system` 14/14, `mtmv_p0/ivm/test_ivm_refresh_dry_run` (the internal
executor streaming through the caller's sender), `insert_p0/test_jdbc` and
`unique_with_mow_p0/partial_update/test_partial_update_multi_stmt`
(multi-statement requests) -- all green.
- Behavior changed:
- [ ] No.
- [x] Yes. <!-- Explain the behavior change -->
1. `EXPLAIN PLAN PROCESS` on Arrow Flight SQL returns rows (release note
above).
2. `EXPLAIN` / `EXPLAIN PLAN PROCESS` / `REPLAY` now count their lines
in `ReturnRows` of the audit log (they went through their own send path before
and reported 0), same as `SHOW` always did.
3. `StmtExecutor.setMoreStmtExists` is set for every non-last statement
of a request, not only on MySQL. It only reaches the master through
`TMasterOpRequest.moreResultExists` and only affects the status flags of the
packet the master returns, which a Flight session does not use.
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01QFwVuLmK8e7sEdKVKB6QZJ
--
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]
