Skip to content

[refactor](session) Bind a ConnectContext to one ProtocolAdapter instead of a Flight subclass - #67835

Merged
morningman merged 2 commits into
apache:masterfrom
morningman:session-protocol-adapter
Sep 11, 2026
Merged

morningman merged 2 commits into
apache:masterfrom
morningman:session-protocol-adapter

Conversation

@morningman

@morningman morningman commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: #67577 -- the tracking issue for the protocol-agnostic session and execution
layer. This is the second PR of its Stage 1 (after the golden baseline #67789) and does not close it.

The shape after this PR:

  +---------------------------+                             +-----------------------------+
  |       MySQL client        |                             |   Arrow Flight SQL client   |
  +-------------+-------------+                             +--------------+--------------+
                |                                                          |
                v                                                          v
  +---------------------------+                             +-----------------------------+
  | MysqlServer               |                             | DorisFlightSqlProducer      |
  |   AcceptListener          |                             |   every call goes through   |
  |   ReadListener            |                             |   adapter.runCommand()      |
  |   (xnio: one command      |                             |   (lock: one command        |
  |    at a time)             |                             |    at a time)               |
  +-------------+-------------+                             +--------------+--------------+
                |                                                          |
                v                                                          v
  +---------------------------+                             +-----------------------------+
  |   MysqlConnectProcessor   |                             |  FlightSqlConnectProcessor  |
  +-------------+-------------+                             +--------------+--------------+
                |                                                          |
                +----------------------------+-----------------------------+
                                             |
                                             v
  +-------------------------------------------------------------------------------------------+
  | ConnectContext  --  the session, one per connection                                       |
  |                                                                                           |
  |   user, catalog / db, SessionVariable, transaction, prepared statements,                  |
  |   queryId / stmtId, executor, audit, ...                                                  |
  |                                                                                           |
  |   protocolAdapter : ProtocolAdapter                                                       |
  |       bound once, by  forMysql() | forMysqlProxy() | forFlight() | new ConnectContext()   |
  |                                                                                           |
  |   getMysqlChannel()  getCapability()  getFlightSqlChannel()  isReturnResultFromLocal() .. |
  |       same signatures as before, now delegate to protocolAdapter                          |
  +---------------------------------------------+---------------------------------------------+
                                                |
                                                v
                       +-----------------------------------------------+
                       |  <<interface>>  qe.protocol.ProtocolAdapter   |
                       |                                               |
                       |   type()                                      |
                       |   remoteHostPortString(ctx)                   |
                       |   resultSinkType()                            |
                       |   connectPool(scheduler)                      |
                       |   afterStatement(ctx)                         |
                       |   closeConnection(ctx)                        |
                       +-----------------------+-----------------------+
                                               |
                          +--------------------+----------------------------+
                          |                                                 |
  +-----------------------+----------------------+  +-----------------------+----------------------+
  | mysql.protocol.MysqlProtocolAdapter          |  | service.arrowflight.protocol                 |
  |                                              |  |   .FlightProtocolAdapter                     |
  |   MysqlChannel:                              |  |                                              |
  |     a socket                                 |  |   peerIdentity (the bearer token)            |
  |     ProxyMysqlChannel (statement forwarded   |  |   FlightSqlChannel (FE-side result cache)    |
  |       to the master)                         |  |   endpoints of the last query                |
  |     DummyMysqlChannel (internal context)     |  |   returnResultFromLocal                      |
  |   server / negotiated MysqlCapability        |  |   prepared queries                           |
  |   handshake packet, MysqlSslContext          |  |   deferred executors + idle bound            |
  |   COM_STMT_EXECUTE packet, cursor flag       |  |     (#62259, #67503)                         |
  |   clientConsumesCursorMetadataTerminator()   |  |   per-session command lock:                  |
  |   accept-query loop                          |  |     runCommand() / callCommand()             |
  |     (start / suspend / resume / stop)        |  |                                              |
  +----------------------------------------------+  +----------------------------------------------+

  Not touched by this PR: ConnectProcessor / StmtExecutor / Coordinator keep calling the
  ConnectContext getters. The result-encoding half (ResultSender) and the capability
  predicates that replace the remaining ConnectType branches come in the next PRs.

In plain terms. A client session in the frontend is a ConnectContext. Today that one class
holds the state of both wire protocols at the same time: the MySQL socket, the capabilities
negotiated with the MySQL client, the handshake and SSL state, the prepared-statement packet being
executed -- and, next to them, the Arrow Flight SQL result cache, the backend endpoints of the last
Flight query, the prepared queries and the deferred coordinators. Which half is real is decided by a
subclass, FlightSqlConnectContext, that overrides six methods and leaves every other Flight member
sitting on the base class, where a MySQL connection carries it as dead weight and a Flight session
throws from the MySQL ones. This PR gives each protocol its own object, a ProtocolAdapter, and
binds a session to exactly one of them when it is created. Nothing a client sees changes: the golden
byte-for-byte baseline recorded in #67789 is identical before and after.

Problem Summary:

ConnectContext mixes three things: the session (user, catalog and database, session variables,
transaction, prepared statements, the running statement), the MySQL protocol state, and the Arrow
Flight SQL protocol state. The next steps of #67577 move the result path of both protocols onto one
shared implementation, which needs a place for "what only this protocol knows" that is not the
session itself. This PR creates that place and moves the state, without touching the execution layer
yet: StmtExecutor, ConnectProcessor and the coordinators still call the same ConnectContext
getters, which now delegate.

What is changed?

qe/protocol/ProtocolAdapter -- the wire-protocol half of a connection: type(), the client
address for processlist and the audit log, the result sink type the backend must use, the pool the
connection is registered in (there is still one per protocol), a per-statement cleanup hook and
closeConnection. qe/protocol holds only the interface; each front end implements it in a
protocol subpackage of its own package (mysql/protocol, service/arrowflight/protocol), which is
also where the result senders of the next step go.

mysql/protocol/MysqlProtocolAdapter -- owns the MysqlChannel (a socket, the
ProxyMysqlChannel of a forwarded statement on the master, or the DummyMysqlChannel of an
internal context), the server and negotiated capabilities, the handshake packet, the SSL context,
the COM_STMT_EXECUTE packet and its cursor flag, and the xnio accept-query loop that
AcceptListener / ReadListener drive. It also owns the decision StmtExecutor and FEOpExecutor
used to compute from ConnectContext fields -- whether the Connector/J release on the other end
consumes the metadata terminator of a cursor result (#67520) -- as
clientConsumesCursorMetadataTerminator.

service/arrowflight/protocol/FlightProtocolAdapter -- owns the peer identity (bearer token), the
FlightSqlChannel, the prepared queries, the endpoints of the last query, returnResultFromLocal
and the deferred executors of #62259 / #67503, together with their idle bound. ConnectContext
keeps checkTimeout and the idle reaper unchanged; only the list moved.

It also serializes the commands of a session. gRPC runs each call of a session on whatever thread it
likes and nothing in the Flight transport orders them, while ConnectContext is not thread-safe
(the existing DorisFlightSqlProducerTest spells that out). runCommand / callCommand take a
per-session lock, make the session the thread's current ConnectContext for the duration, restore
the previous one afterwards, and give up with UNAVAILABLE after the session's query timeout if
another command is still running. DorisFlightSqlProducer runs statement execution, prepared
statement creation and close, DoGet of a frontend-side result and the catalog / schema / table
metadata requests through it. DoGet of a frontend-side result streams under the lock on purpose: the
next statement of the session resets the channel, whose removal listener closes the
VectorSchemaRoot being streamed. Session teardown (token expiry, CloseSession, KILL) does not
take the lock; that path is reworked when the token becomes the session credential.

ConnectContext -- gets protocolAdapter and three factories: forMysql(StreamConnection),
forMysqlProxy(sessionId) (replaces the new ConnectContext(null, true, sessionId) call in
FrontendServiceImpl) and forFlight(peerIdentity) (replaces the subclass in
FlightSessionsManager). The existing constructors stay as thin wrappers, so the ~115 test files that
call new ConnectContext() are untouched. Every protocol-specific getter keeps its signature and
delegates: getMysqlChannel(), getCapability(), getFlightSqlChannel(), isReturnResultFromLocal()
and so on. A getter that only makes sense on the other protocol throws IllegalStateException
naming the actual protocol (the subclass used to throw a RuntimeException for getMysqlChannel();
the base class used to return null / an empty list for the Flight ones, which no caller relied on).
FlightSqlConnectContext is deleted: its getClientIP / getRemoteHostPortString /
closeChannel / setQueryId overrides are the adapter's remoteHostPortString / closeConnection
/ connectPool, and its kill override only differed in log text.

Removed as dead code while touching the class: isSend / setIsSend (nothing read them; the real
flag lives on MysqlChannel), cloneContext() (no caller, and it would have to share a channel
between two adapters), and the two lines of resetConnection() that cleared Flight-only fields
(COM_RESET_CONNECTION is only sent by MySQL clients).

Not in this PR, deliberately: the execution layer still branches on ConnectType, and an internal
context is still a MySQL context over a DummyMysqlChannel, exactly as before. Both go away in the
follow-up PRs that introduce the result sender and the capability bits.

Verification

  • Golden baseline of [test](protocol) Record a golden baseline of MySQL packets and Arrow Flight results #67789: MysqlPacketGoldenTest (27 cases, byte for byte) and
    FlightResultGoldenTest pass unchanged. Not a byte of the recorded traffic moved.
  • New unit tests: FlightProtocolAdapterTest (commands of one session run one at a time, a waiting
    command fails with UNAVAILABLE after the query timeout, the thread's current context is set and
    restored, a failing command releases the session, KILL unregisters the session from the Flight
    pool, the trace id lands in the Flight pool) and MysqlProtocolAdapterTest (internal and proxy
    contexts, the cursor-terminator decision and its per-statement reset, the accept-query loop and
    close going through the channel).
  • Existing tests adjusted to the factories: the ones that built a FlightSqlConnectContext, poked
    mysqlChannel / connectType through reflection, or used a plain new ConnectContext() as a
    Flight session (ShortCircuitPointQueryTest, AuditLogWorkloadGroupTest, StmtExecutorTest,
    ConnectContextTest, MysqlProtoTest, ConnectionExceedTest). 26 test classes around the
    session, the MySQL channel and the Flight producer: 166 tests, 0 failures.
  • Regression on a local cluster built from this branch: arrow_flight_sql_p0 (8 suites, including
    the forward-to-master, query-release, point-query, SQL cache and DatabaseMetaData.getColumns
    paths) and prepared_stmt_p0 (cursor fetch and server-side prepare over MySQL).
  • checkstyle:check on fe-core (main and test sources): 0 violations.

Release note

None.

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
  • Behavior changed:

    • No.
    • Yes.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

🤖 Generated with Claude Code

https://claude.ai/code/session_01QFwVuLmK8e7sEdKVKB6QZJ

…ead of a Flight subclass

ConnectContext carried the state of both wire protocols at once, and which of
them was live was decided by FlightSqlConnectContext overriding six methods.
Now a context is created with one ProtocolAdapter and keeps it for life:

- MysqlProtocolAdapter: the MysqlChannel (a socket, a ProxyMysqlChannel on the
  master, or a DummyMysqlChannel for an internal context), the negotiated
  capabilities, the handshake packet, the SSL context, the COM_STMT_EXECUTE
  packet and cursor flag, and the xnio accept-query loop. It also owns the
  Connector/J cursor-terminator decision that StmtExecutor and FEOpExecutor
  used to compute from ConnectContext fields.
- FlightProtocolAdapter: the peer identity, the FlightSqlChannel, the prepared
  queries, the endpoints, returnResultFromLocal and the deferred executors.
  It serializes the commands of a session under a per-session lock, which gRPC
  does not do and ConnectContext is not safe without: DorisFlightSqlProducer
  runs statements, prepared-statement actions, DoGet of frontend-side results
  and metadata requests through it.

The protocol-specific getters of ConnectContext keep their signatures and
delegate to the adapter, so the execution layer is untouched in this step.
ConnectContext.forMysql / forMysqlProxy / forFlight replace the subclass and
the (null, true, sessionId) constructor call. checkTimeout and the deferred
executor reaper stay where they were.

Verified against the golden baseline of apache#67789: neither MysqlPacketGoldenTest
nor FlightResultGoldenTest changed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QFwVuLmK8e7sEdKVKB6QZJ
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@morningman

Copy link
Copy Markdown
Contributor Author

run buildall

…otocol subpackage

qe.protocol defines the interface; each front end implements it in its own
protocol subpackage: mysql.protocol.MysqlProtocolAdapter and now
service.arrowflight.protocol.FlightProtocolAdapter. The result senders of the
next step land in the same two places.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QFwVuLmK8e7sEdKVKB6QZJ
@morningman

Copy link
Copy Markdown
Contributor Author

run buildall

1 similar comment
@morningman

Copy link
Copy Markdown
Contributor Author

run buildall

@morningman

Copy link
Copy Markdown
Contributor Author

Local pipeline review — ✅ PASS

schema: doris-repo-review/v1
status: PASS
pr: apache/doris#67835
commit: 22b9306171e097555946d8b9778d7cba13f7e036
base: b78724a75f704b059bf07cab4e7fa6601bdd8a79
reviewed_at: 2026-09-11T13:15+08:00
reviewer: morningman
model: claude-opus-5[1m]
effort: max
findings: {blocker: 0, major: 0, minor: 1, nit: 7}
rounds: 1
converged: true

Notes for maintainers

  • fe/fe-core/src/main/java/org/apache/doris/service/arrowflight/protocol/FlightProtocolAdapter.java:270 — the only Minor: a command waiting for the session lock gives up after query_timeout, while the command holding it may be a sync-load statement allowed getExecTimeoutS() = max(insert_timeout, query_timeout); a concurrent metadata/prepare call during a long INSERT gets UNAVAILABLE too early. One-line fix: bound the wait with ctx.getExecTimeoutS().
  • fe/fe-core/src/main/java/org/apache/doris/service/arrowflight/DorisFlightSqlProducer.java:320 — the busy-session UNAVAILABLE reaches clients as UNAVAILABLE, INTERNAL, or INTERNAL with an empty description depending on the entry point (getFlightInfoStatement and the three metadata streams re-wrap it); consider passing a FlightRuntimeException through unchanged.
  • fe/fe-core/src/main/java/org/apache/doris/service/arrowflight/protocol/FlightProtocolAdapter.java:120 — teardown (closeSession, token expiry, KILL) still runs unregisterConnection concurrently with a command holding the lock; pre-existing and stated in the javadoc, but the next PR of [Tracking] Protocol-agnostic session and execution layer: MySQL and Arrow Flight SQL as equal front ends #67577 should close it.
  • The deleted Flight kill override also changed the cancel reason a killed Flight query reports ("arrow flight query killed by user" -> "cancel query by user from "), not only the log text; nothing asserts either string.
  • Not verified locally: no build, unit test, regression or cluster run. CheckStyle is green; TeamCity FE UT build 1043145 on this exact commit was still running with 7118 passed / 0 failed when this comment was posted; the golden baselines under fe/fe-core/src/test/resources/protocol-golden/ are untouched by the diff.

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.

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 16798 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 22b9306171e097555946d8b9778d7cba13f7e036, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17626	3051	3023	3023
q2	2072	255	220	220
q3	10254	914	529	529
q4	4676	252	206	206
q5	7670	567	394	394
q6	141	118	95	95
q7	542	493	382	382
q8	9236	869	919	869
q9	3462	2417	2427	2417
q10	6496	858	703	703
q11	406	201	182	182
q12	614	262	202	202
q13	18135	1565	1180	1180
q14	163	149	143	143
q15	q16	451	405	382	382
q17	1310	903	756	756
q18	3108	2317	2318	2317
q19	1278	921	731	731
q20	394	285	205	205
q21	5583	1630	1879	1630
q22	332	269	232	232
Total cold run time: 93949 ms
Total hot run time: 16798 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	3421	3373	3335	3335
q2	502	396	380	380
q3	2239	2450	2199	2199
q4	1199	1187	896	896
q5	2232	2151	2139	2139
q6	168	129	88	88
q7	1031	924	864	864
q8	1582	1407	1407	1407
q9	3172	3138	3135	3135
q10	1883	1811	1654	1654
q11	357	269	251	251
q12	453	441	346	346
q13	1491	1543	1163	1163
q14	164	182	153	153
q15	q16	397	409	365	365
q17	3714	3351	3309	3309
q18	4902	4493	4913	4493
q19	1020	877	865	865
q20	1015	961	839	839
q21	3873	3203	3268	3203
q22	395	346	329	329
Total cold run time: 35210 ms
Total hot run time: 31413 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 83222 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 22b9306171e097555946d8b9778d7cba13f7e036, data reload: false

query5	4267	421	335	335
query6	376	132	126	126
query7	4952	440	228	228
query8	286	132	120	120
query9	8684	2920	2872	2872
query10	402	216	186	186
query11	5377	1049	936	936
query12	119	75	68	68
query13	1192	443	347	347
query14	6176	2221	2133	2133
query14_1	2007	1990	1980	1980
query15	180	127	111	111
query16	923	388	367	367
query17	807	478	364	364
query18	2335	350	228	228
query19	158	136	103	103
query20	80	67	69	67
query21	198	100	84	84
query22	5404	5347	5406	5347
query23	6980	6405	6241	6241
query23_1	6181	6318	6246	6246
query24	7299	1093	776	776
query24_1	774	783	778	778
query25	401	287	228	228
query26	1218	228	132	132
query27	2788	424	252	252
query28	4664	1507	1519	1507
query29	904	425	332	332
query30	251	155	132	132
query31	823	397	332	332
query32	129	72	70	70
query33	448	215	177	177
query34	985	842	496	496
query35	398	402	335	335
query36	564	562	531	531
query37	130	78	66	66
query38	1001	867	850	850
query39	505	476	500	476
query39_1	484	474	481	474
query40	198	88	76	76
query41	54	52	51	51
query42	76	70	71	70
query43	240	241	212	212
query44	971	535	544	535
query45	105	111	101	101
query46	779	847	554	554
query47	767	771	727	727
query48	337	314	235	235
query49	540	259	191	191
query50	745	273	199	199
query51	8376	8304	8160	8160
query52	65	72	58	58
query53	202	198	151	151
query54	239	178	162	162
query55	74	59	61	59
query56	196	166	158	158
query57	678	661	660	660
query58	199	161	156	156
query59	1218	1233	1149	1149
query60	253	211	188	188
query61	132	147	123	123
query62	372	212	185	185
query63	175	158	146	146
query64	2889	818	734	734
query65	1678	1584	1615	1584
query66	1787	317	196	196
query67	9904	9823	9713	9713
query68	2747	1232	738	738
query69	333	222	205	205
query70	670	641	613	613
query71	259	176	157	157
query72	2294	1709	1542	1542
query73	662	617	354	354
query74	1577	1235	1151	1151
query75	1171	1117	971	971
query76	2293	728	550	550
query77	263	273	214	214
query78	4135	3708	3282	3282
query79	2977	838	608	608
query80	1604	319	281	281
query81	517	161	148	148
query82	615	124	100	100
query83	271	210	193	193
query84	293	113	87	87
query85	794	364	288	288
query86	472	195	157	157
query87	1024	997	912	912
query88	3118	2122	2129	2122
query89	272	200	177	177
query90	2162	131	128	128
query91	134	119	101	101
query92	100	68	62	62
query93	2504	1051	713	713
query94	641	246	228	228
query95	518	317	230	230
query96	860	562	272	272
query97	1134	1071	1041	1041
query98	174	137	138	137
query99	414	349	308	308
Total cold run time: 180639 ms
Total hot run time: 83222 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 14.9 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 22b9306171e097555946d8b9778d7cba13f7e036, data reload: false

query1	0.00	0.00	0.00
query2	0.09	0.05	0.04
query3	0.25	0.10	0.11
query4	1.60	0.10	0.10
query5	0.18	0.16	0.16
query6	1.25	0.69	0.71
query7	0.03	0.01	0.00
query8	0.04	0.04	0.03
query9	0.30	0.22	0.22
query10	0.36	0.34	0.35
query11	0.16	0.12	0.12
query12	0.14	0.12	0.12
query13	0.32	0.32	0.32
query14	0.48	0.45	0.45
query15	0.36	0.35	0.36
query16	0.24	0.23	0.24
query17	0.68	0.71	0.69
query18	0.17	0.17	0.18
query19	1.21	1.11	1.15
query20	0.01	0.02	0.02
query21	15.45	0.16	0.14
query22	5.04	0.04	0.05
query23	16.16	0.26	0.10
query24	3.07	0.31	0.23
query25	0.09	0.05	0.04
query26	0.73	0.16	0.12
query27	0.04	0.04	0.03
query28	3.62	0.57	0.27
query29	12.45	3.23	2.63
query30	0.26	0.12	0.13
query31	2.76	0.39	0.17
query32	3.50	0.32	0.23
query33	1.41	1.55	1.42
query34	15.39	2.18	1.82
query35	1.79	1.79	1.76
query36	0.44	0.29	0.29
query37	0.06	0.04	0.04
query38	0.05	0.03	0.03
query39	0.04	0.02	0.02
query40	0.12	0.07	0.08
query41	0.08	0.03	0.02
query42	0.03	0.02	0.03
query43	0.03	0.03	0.03
Total cold run time: 90.48 s
Total hot run time: 14.9 s

@morningman
morningman merged commit 83ebddd into apache:master Sep 11, 2026
49 of 50 checks passed
morningman added a commit to morningman/doris that referenced this pull request Sep 11, 2026
…kage into org.apache.doris.arrowflight

Arrow Flight SQL was added under org.apache.doris.service.arrowflight in
2023 (apache#24772) because it was wired up next to the thrift FrontendServiceImpl.
It is now a full peer of the MySQL front end, whose code lives in the
top-level org.apache.doris.mysql package, and apache#67835 already had to mirror a
"protocol" sub-package on both sides. Move it up one level so the two front
ends are symmetric before the next PR adds ResultSender implementations to
that shape.

Pure rename: git mv of fe-core main/test service/arrowflight/** (sub-packages
kept), plus the manual Flight JDBC client FlightSqlJDBC that apache#27661 left in
the service test package. Inside the files only package and import lines
change, with the org.apache.doris import block re-sorted where "arrowflight"
now sorts before "catalog"/"common"/"qe". Nothing references the old package
name by string (no Class.forName, config key, pom, checkstyle suppression or
log4j entry).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QFwVuLmK8e7sEdKVKB6QZJ
morningman added a commit that referenced this pull request Sep 11, 2026
…kage into org.apache.doris.arrowflight (#67866)

### What problem does this PR solve?

Issue Number: #67577 -- the tracking issue for the protocol-agnostic
session and execution
layer. This is a preparatory, mechanical PR of its Stage 1 and does not
close it.

Arrow Flight SQL was added under `org.apache.doris.service.arrowflight`
in 2023 (#24772)
because it was wired up next to the thrift `FrontendServiceImpl`. It has
since grown into a full
peer of the MySQL front end (its own connect processor, sessions, auth,
result channel, and, as
of #67835, a `ProtocolAdapter`), while the MySQL side lives in the
top-level `org.apache.doris.mysql`
package. #67835 already had to mirror the `protocol` sub-package on both
sides, and the next PR
(`ResultSender`) would add one more implementation per side. Moving
Flight out of `service/` now
keeps the two front ends symmetric before more code lands on that shape:

```
  org.apache.doris.mysql                        org.apache.doris.arrowflight   (was service.arrowflight)
    .protocol.MysqlProtocolAdapter                .protocol.FlightProtocolAdapter
    .protocol.MysqlResultSender   (next PR)       .protocol.FlightResultSender   (next PR)
```

What moves (`git mv`, sub-packages kept as they were):

- `fe-core/src/main/java/org/apache/doris/service/arrowflight/**` ->
`.../doris/arrowflight/**`
(19 files: 4 top-level, `auth2/` 5, `protocol/` 1, `results/` 3,
`sessions/` 3, `tokens/` 3)
- `fe-core/src/test/java/org/apache/doris/service/arrowflight/**` ->
`.../doris/arrowflight/**`
  (7 tests)
- `fe-core/src/test/java/org/apache/doris/service/FlightSqlJDBC.java` ->
`.../doris/arrowflight/`
(the manual Flight JDBC client left behind by #27661; it only sat in
`service` because Flight did)

What changes inside files: `package` declarations and `import` lines
only, plus re-sorting the
`org.apache.doris.*` import block where `arrowflight` now sorts before
`catalog`/`common`/`qe`
(checkstyle `CustomImportOrder`). 9 files outside the moved tree only
update imports:
`ConnectContext`, `ConnectScheduler`, `Coordinator`,
`NereidsCoordinator`, `OssFeServerStarterProvider`,
and the tests `ConnectionExceedTest`, `MysqlProtocolAdapterTest`,
`AuditLogWorkloadGroupTest`,
`FlightResultGoldenTest`.

Nothing else references the old package name: no `Class.forName`, no
configuration key, no
`pom.xml` / checkstyle suppression / log4j entry, no regression-test or
`.github` path. The four
classes that are actually the thrift service (`ExecuteEnv`,
`FeDiskInfo`, `FrontendOptions`,
`FrontendServiceImpl`) stay in `org.apache.doris.service`.
morningman added a commit to morningman/doris that referenced this pull request Sep 12, 2026
Second step of the protocol-independent session work (apache#67577). The
result half of the two wire protocols is now behind qe.protocol.ResultSender,
implemented by mysql.protocol.MysqlResultSender (the former sendMetaData /
sendFields / sendTextResultRow / sendBinaryResultRow / sendStmtPrepareOK of
StmtExecutor and the COM_FIELD_LIST body of ConnectProcessor, byte layout
unchanged) and arrowflight.protocol.FlightResultSender (FlightSqlChannel).

StmtExecutor no longer holds a MysqlSerializer or takes a MysqlChannel:
executeAndSendResult, sendCachedValues and executeInternalQueryAndSend take
a ResultSender, and the internal executor of an IVM dry run gets the
caller's sender. handleExplainStmt / handleReplayStmt /
handleExplainPlanProcessStmt go through the one sendResultSet, which gives
EXPLAIN PLAN PROCESS a result on Arrow Flight SQL.

ConnectProcessor loses its connectType field: the per-statement protocol
work of a request is adapter.finishStatement (MySQL: SERVER_MORE_RESULTS_EXISTS
and the intermediate response; Flight: the forwarded outcome and the
single-result rule), finalizeCommand / getResultPacket move to
MysqlProtocolAdapter.finishCommand / responsePacket, COM_FIELD_LIST to
MysqlConnectProcessor, and the SQL-cache guard is supportsSqlCacheReplay().

Also folds in the leftovers of the apache#67835 review: the command lock waits
up to getExecTimeoutS() and logs when it gives up, FlightRuntimeException
passes through the producer's catch-alls, of() names a null adapter, and
two tests bind their channels through the adapter.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QFwVuLmK8e7sEdKVKB6QZJ
morningman added a commit that referenced this pull request Sep 12, 2026
### 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.
morningman added a commit that referenced this pull request Sep 14, 2026
…s with protocol capabilities (#67900)

### What problem does this PR solve?

Issue Number: #67577

Related PR: #67883 (`ResultSender`), #67835 (`ProtocolAdapter`), #67789
(protocol goldens)

Problem Summary:

Third and last refactor step of stage 1 of #67577. After #67835 and
#67883 the connection half and the result-encoding half of the two wire
protocols live behind `ProtocolAdapter` / `ResultSender`, but the
execution layer still asked `ctx.getConnectType()` in a dozen places:
five times in `StmtExecutor`, in `ConnectContext.supportHandleByFe`, in
nine insert / transaction `Command`s (each resetting the MySQL channel),
in both coordinators, in the short-circuit rewrite rule and in
`FEOpExecutor`; and the Flight-only `returnResultFromLocal` flag was
flipped from four places outside the adapter. This PR replaces every one
of them with a capability or a lifecycle hook on the adapter, one method
per use:

```
  +---------------------------+                             +-----------------------------+
  |       MySQL client        |                             |   Arrow Flight SQL client   |
  +-------------+-------------+                             +--------------+--------------+
                |                                                          |
                v                                                          v
  +---------------------------+                             +-----------------------------+
  | MysqlServer               |                             | DorisFlightSqlProducer      |
  |   AcceptListener          |                             |   every call goes through   |
  |   ReadListener            |                             |   adapter.runCommand()      |
  +-------------+-------------+                             |   executeQueryStatement:    |
                |                                           |   adapter.beginRequest() NEW|
                v                                           +--------------+--------------+
  +---------------------------+                                            |
  |   MysqlConnectProcessor   |                                            v
  |   COM_FIELD_LIST          |                             +-----------------------------+
  |   handleExecute:          |                             |  FlightSqlConnectProcessor  |
  |     adapter.beforeStatement  NEW                        |                             |
  |   finalizeCommand() =     |                             |                             |
  |   adapter.finishCommand() |                             |                             |
  +-------------+-------------+                             +--------------+--------------+
                |                                                          |
                +----------------------------+-----------------------------+
                                             |
                                             v
  +-------------------------------------------------------------------------------------------+
  | ConnectProcessor.executeQuery  --  parse, one StmtExecutor per statement, audit           |
  |     for each statement:  adapter.beforeStatement(ctx)                   <-- NEW           |
  |                          executor.execute()                                               |
  |                          adapter.finishStatement(ctx, executor, i, n)                     |
  |   proxyExecute (master side): MysqlProtocolAdapter.restoreFromForwardRequest  <-- NEW     |
  +---------------------------------------------+---------------------------------------------+
                                                |
                                                v
  +-------------------------------------------------------------------------------------------+
  | StmtExecutor  --  plans and runs one statement, protocol-agnostic                          |
  |                                                                                           |
  |   forwarding a query to the master:   adapter.canReplayForwardedQueryResult()   <-- NEW   |
  |   retry after a failed attempt:       adapter.canRetryQuery(ctx)                <-- NEW   |
  |   execute(queryId), every attempt:    adapter.beforeAttempt(ctx)                <-- NEW   |
  |   executeAndSendResult:               adapter.beforeQuery(ctx), then the coordinator;     |
  |                                       relay rows unless !ctx.isReturnResultFromLocal()    |
  |   FEOpExecutor.buildStmtForwardParams: adapter.fillForwardRequest(ctx, request) <-- NEW   |
  |                                                                                           |
  |   Commands (insert / txn):  no channel reset any more, beforeStatement did it             |
  |   Coordinator / NereidsCoordinator / QueryProcessor:  ctx.isReturnResultFromLocal()       |
  |       decides receivers vs. Flight endpoints, no ConnectType assertion                    |
  |   LogicalResultSinkToShortCircuitPointQuery:  adapter.supportsShortCircuitPointQuery() NEW|
  |   no ConnectType branch and no getMysqlChannel() left in qe/** and nereids/**             |
  +---------------------------------------------+---------------------------------------------+
                                                |
                                                v
  +-------------------------------------------------------------------------------------------+
  | ConnectContext  --  the session, one per connection                                       |
  |   protocolAdapter : ProtocolAdapter          getResultSender() = adapter.resultSender(this)|
  |   supportHandleByFe() = adapter.supportsFeSideResult() && command != COM_STMT_EXECUTE     |
  |   isReturnResultFromLocal() = adapter.returnsResultFromLocal(this)   (setter is gone)     |
  +---------------------------------------------+---------------------------------------------+
                                                |
                                                v
                       +-----------------------------------------------+
                       |  <<interface>>  qe.protocol.ProtocolAdapter   |
                       |   type() remoteHostPortString(ctx)            |
                       |   resultSinkType() resultSender(ctx)          |
                       |   connectPool(scheduler) closeConnection(ctx) |
                       |                                               |
                       |   capabilities:                               |
                       |     supportsSqlCacheReplay()                  |
                       |     canReplayForwardedQueryResult()  <-- NEW  |
                       |     supportsFeSideResult()           <-- NEW  |
                       |     supportsShortCircuitPointQuery() <-- NEW  |
                       |     canRetryQuery(ctx)               <-- NEW  |
                       |                                               |
                       |   statement lifecycle:                        |
                       |     beforeStatement(ctx)             <-- NEW  |
                       |     beforeAttempt(ctx)               <-- NEW  |
                       |     beforeQuery(ctx)                 <-- NEW  |
                       |     returnsResultFromLocal(ctx)      <-- NEW  |
                       |     finishStatement(ctx, executor, i, n)      |
                       |     afterStatement(ctx)                       |
                       |                                               |
                       |   forwarding:                                 |
                       |     fillForwardRequest(ctx, request) <-- NEW  |
                       +-----------------------+-----------------------+
                                               |
                          +--------------------+----------------------------+
                          |                                                 |
  +-----------------------+----------------------+  +-----------------------+----------------------+
  | mysql.protocol.MysqlProtocolAdapter          |  | arrowflight.protocol.FlightProtocolAdapter   |
  |   capabilities: all true;                    |  |   capabilities: all false (the reasons are   |
  |     canRetryQuery = nothing flushed yet      |  |     documented on each method)               |
  |   beforeStatement: channel.reset()  -- drops |  |   beforeStatement: the statement's result is |
  |     what the previous statement of the       |  |     on this frontend until beforeQuery says  |
  |     request left unsent                      |  |     a backend produces it; the coordinator   |
  |   beforeAttempt: nothing                     |  |   beforeAttempt: result on this frontend,    |
  |   beforeQuery: nothing, rows are relayed     |  |     endpoints of the failed attempt dropped  |
  |                                              |  |     then registers the endpoints             |
  |   returnsResultFromLocal: true               |  |   returnsResultFromLocal: that flag          |
  |   fillForwardRequest: capability flags,      |  |   fillForwardRequest: nothing                 |
  |     deprecate-EOF, execute packet + cursor   |  |   beginRequest(): deferred executors, result |
  |   restoreFromForwardRequest (master side)    |  |     cache, endpoints, flag  (Flight-private) |
  |   proxyResultPackets() (master side)         |  |                                              |
  +-----------------------+----------------------+  +-----------------------+----------------------+
                          |                                                 |
                          v                                                 v
  +----------------------------------------------+  +----------------------------------------------+
  | <<interface>> qe.protocol.ResultSender        |  |                                              |
  |   sendResultSet(rs, fieldInfos, binaryRows)   |  |                                              |
  |   sendFields(names, fieldInfos, types)        |  |                                              |
  |   sendRow(wireRow)                            |  |                                              |
  |   reset()  -- now only for a retried query    |  |                                              |
  +----------------------------------------------+  +----------------------------------------------+
  | mysql.protocol.MysqlResultSender (unchanged) |  | arrowflight.protocol.FlightResultSender      |
  |                                              |  |   sendResultSet no longer touches the        |
  |                                              |  |   result-location flag (EXPLAIN never marks  |
  |                                              |  |   the result as coming from a backend now)   |
  +----------------------------------------------+  +----------------------------------------------+
```

**Capabilities.** `canReplayForwardedQueryResult()` guards the refusal
to forward a query to the master on a Flight session (#67569; the
message is unchanged). `supportsFeSideResult()` is the protocol half of
`ConnectContext.supportHandleByFe()`; `supportsShortCircuitPointQuery()`
is the protocol half of
`LogicalResultSinkToShortCircuitPointQuery.scanMatchShortCircuitCondition`
(#67368); `canRetryQuery(ctx)` is the retry condition of
`handleQueryWithRetry`: for MySQL "nothing was flushed to the socket
yet", for Flight false (the client is not told which attempt its
endpoints belong to). The reason each Flight answer is what it is moves
onto the Flight implementation, out of the call sites.

**Statement lifecycle.** `beforeStatement(ctx)` is called by
`ConnectProcessor.executeQuery` before each statement (and by
`MysqlConnectProcessor.handleExecute`): the MySQL adapter resets the
channel there, which is what the query path and the nine `Command`s did
each on their own; the Flight adapter puts the statement's result on
this frontend. `beforeQuery(ctx)` is called at the top of
`executeAndSendResult`, before a coordinator is built: the Flight
adapter marks the result as staying on the backends, and `Coordinator` /
`NereidsCoordinator` / `QueryProcessor` keep reading
`ctx.isReturnResultFromLocal()` to register endpoints instead of
receivers (the `checkState(ARROW_FLIGHT_SQL)` assertions go, the
decision itself does not move). Because `beforeQuery` runs only where a
coordinator follows, an `EXPLAIN` -- handled earlier in
`handleQueryStmt` -- never marks its result as coming from a backend,
and the `setReturnResultFromLocal(true)` that
`FlightResultSender.sendResultSet` had to do in #67883 is gone with the
setter. The four flips in `StmtExecutor` / `FlightSqlConnectProcessor`
are gone; what a Flight request drops from its predecessor (deferred
executors, result cache, endpoints, the flag) is
`FlightProtocolAdapter.beginRequest()`.

`beforeAttempt(ctx)` is called at the top of
`StmtExecutor.execute(TUniqueId)`, i.e. for the first attempt and for
each replan retry of `queryRetry` (a cloud `NEED_REPLAN` error); it
replaces the `setReturnResultFromLocal(true)` that stood there before
this PR, which the first version of this PR had folded into
`beforeStatement` -- once per statement, outside the replan loop. A
Flight attempt that had reached `beforeQuery` and then failed with a
replan error left the next attempt in the "result on the backends"
state; if that attempt failed before its own `beforeQuery`,
`StatementContext.close` skipped the connector statement scope and the
`finalizeQuery` in `handleQueryWithRetry` was skipped, with no deferred
executor or DoGet ever releasing them. The Flight `beforeAttempt` puts
the result back on this frontend and withdraws exactly the endpoints the
failed attempt registered (the adapter records how many the request held
when the statement started and trims back to that mark, so an earlier
statement of the request keeps its own), which `queryRetry` -- unlike
`handleQueryWithRetry`, which asks `canRetryQuery` -- did not withdraw
before, so a replanned Flight query no longer hands the client the
endpoints of a cancelled attempt next to its own. Covered by
`FlightProtocolAdapterTest` (the attempt lifecycle) and
`ProtocolCapabilityWiringTest.testEveryAttemptOfAReplannedFlightQueryStartsOnTheFrontend`,
which runs a real Flight query whose first attempt fails on the mocked
backend with an `E-230` status after registering its endpoint and whose
second attempt fails while planned: the session ends with its result on
the frontend and no endpoint (the test fails on the flag without the
hook).

**Forwarding.** `fillForwardRequest(ctx, request)` adds to a
`TMasterOpRequest` what the master needs to know about the client: the
MySQL adapter writes the negotiated capability flags,
`CLIENT_DEPRECATE_EOF` and, for a `COM_STMT_EXECUTE`, the execute packet
and the cursor flag (formerly two blocks in
`FEOpExecutor.buildStmtForwardParams`, one of them behind `if (MYSQL)`);
the Flight adapter writes nothing, its session consumes the master's
status and rows rather than its packets (`carryForwardedOutcome`). The
master's side, `ConnectProcessor.restoreForwardedMysqlContext`, becomes
`MysqlProtocolAdapter.restoreFromForwardRequest`, and `proxyExecute`
reads the proxy channel's packets through
`MysqlProtocolAdapter.proxyResultPackets()` instead of
`StmtExecutor.getProxyQueryResultBufList()` casting the channel.

After this PR `grep -rn 'ConnectType\.\|getMysqlChannel()'
fe-core/src/main/java/org/apache/doris/{qe,nereids}` outside
`*/protocol/` finds only the `ConnectContext.getMysqlChannel()` delegate
itself and two lines of `MysqlConnectProcessor` (reading the client's
packet, the auth-switch handshake), which is MySQL protocol code by
definition.

**One behavior change, on the MySQL side, recorded in the golden.** The
channel used to be reset at the start of a *query* and inside the insert
/ transaction commands, and nowhere else. A client that did not
negotiate `CLIENT_MULTI_STATEMENTS` gets no intermediate response
between the statements of a request, so whatever a query wrote stayed in
the send buffer until the next query or insert reset it. When the next
statement was neither -- `select 1; set @A = 1` -- the buffered result
set of the `SELECT` went out together with the `OK` of the `SET`: a
result set terminated by a `0x00` OK packet, which no MySQL client
parses (it reads the OK as a row and waits for more). With the reset at
the start of every statement, such a request delivers only its last
statement's outcome, which is what
`MysqlProtocolAdapter.finishStatement` has documented as the intent all
along. The first commit makes `RecordingMysqlChannel` model the send
buffer (a reset drops what was written after the last flush, the way
`MysqlChannel.reset()` clears it) and records `select 1; set @A = 1`
with and without the capability as it is today; the second commit's
golden diff is exactly that: the three packets of `select 1` disappear
from `multi-statement-without-capability-query-then-set`, the `OK` keeps
its sequence id 4. The existing `multi-statement-without-capability`
case (`select 1; select 2`) loses the three packets of `select 1` in the
first commit only, because the recording channel now shows what reaches
the client -- its sequence ids, 4 to 7, already were the ones on the
wire. That the delivered response did not start at sequence id 1 was
pre-existing -- `MysqlChannel.reset()` cleared the buffer but did not
rewind the sequence id, so a client that checks sequence ids (pymysql,
libmysqlclient; Connector/J does not by default) already failed `select
1; select 2` without the capability with "Packet sequence number wrong -
got 5 expected 1" -- but the reset before every statement widened it: a
request without the capability whose later statement failed (`select 1;
select * from no_such_table`) used to deliver the buffered result set of
`select 1` followed by the ERR, and now delivered a lone ERR with that
hole, which pymysql rejects and go-sql-driver answers by closing the
connection. So it is fixed here: `MysqlChannel` tracks the sequence id
as of the last packet that reached the wire (the command packet
received, the last flush) and `reset()` rewinds to it,
`RecordingMysqlChannel` models the same, and the golden shows `select 1;
select 2` and `select 1; set @A = 1` numbered from 1 and gains `select
1; select * from no_such_table` with and without the capability (ERR at
5 after the intermediate response, ERR at 1 without one). The same reset
also fixes the retry of a forwarded query on the master:
`ProxyMysqlChannel` never flushes, so `canRetryQuery` held, but its
collected packets survived the reset and both attempts' packets
travelled to the client; it now drops them on reset.

Not in this PR: an internal adapter for the no-client context (it is
still a MySQL context over a `DummyMysqlChannel`, now with nothing in
the execution layer keyed on that), and the stage 1 performance
baseline.
morningman added a commit that referenced this pull request Sep 15, 2026
…on PRs from master in merge order (#67753 #67802 #67814 #67837 #67853 #67876) (#68017)

Cherry-picked from #67753, #67802, #67814, #67837, #67853, #67876

Batch pick of every merged PR carrying the `incremental-computation`
label that `branch-incremental-computation` does not have yet (no
`incremental-computation-picked` label), in the order they landed on
master (`git log --first-parent`). One commit per PR, each created with
`git cherry-pick -x` so the message ends with `(cherry picked from
commit <master sha>)`. Follows the same convention as #67830.

| # | Master commit | PR | Title |
|---|---|---|---|
| 1 | fe39f5b | #67753 | [fix](ivm) Answer FE-computable dry runs on
the frontend instead of a placeholder backend |
| 2 | f8ed33f | #67802 | [fix](ivm) Refresh the surviving partitions
after an IVM baseline rebuild |
| 3 | 7bd89a0 | #67814 | [fix](ivm) Stop the incremental delta from
reading partitions the MV dropped |
| 4 | 3050a9a | #67837 | [fix](ivm) Invalidate the baseline when a
column used by the MV is dropped |
| 5 | 22c95eb | #67853 | [fix](ivm) Carry the row-binlog hidden
columns in the analyzed MTMV schema |
| 6 | 3390a7a | #67876 | [test](ivm) Remove unnecessary cloud skips
from IVM suites |

Not included on purpose:
- The 11 labelled PRs that already carry
`incremental-computation-picked` (#62606 in the fork point, #67508 via
#67712, the nine of #67830).
- #67820 is still open on master; this branch already carries its
content via #67861.

### Prerequisite check

None of the six PRs declares a prerequisite, and none of them needs
another master PR for its behavior. The only master commits that touch
the same files and are not on this branch are unrelated to incremental
computation (#66761 TIMESTAMP_NS, #67545 DLF, #67569 / #67520 / #67835
MySQL-protocol and session refactors, #67186 Hive partition batching,
#67787 SQL cache user variables); they were left out, and two picks
needed a mechanical adaptation because of that:

- **#67753** conflicted in `StmtExecutor.sendMetaData`: master had
already extracted the post-metadata EOF into
`sendMetadataTerminatorIfNeeded(channel)` (#67520, a Connector/J
cursor-fetch fix). The branch keeps its inline EOF block and now sends
it on the given `channel` instead of `context.getMysqlChannel()`, which
is exactly what the extracted helper does on master. Everything else in
the pick is identical to the master commit.
- **#67814** applied cleanly but did not compile: the new
`MTMVPartitionUtil.generateRelatedBasePartitionIds()` returns an
`Optional`, and on master `import java.util.Optional;` came with #67186.
The import was added to the pick commit; that is the only difference
from the master commit.

The other four picks applied without conflicts and are byte-identical to
their master commits (diffs compared with `index`/`@@` lines stripped).
Both adaptations are recorded in the respective commit messages.

### Drift check against master

After the six picks, every touched file is byte-identical to master at
`3390a7a721f` except `MTMV.java`, `MTMVTask.java`,
`MTMVPartitionUtil.java`, `MTMVPartitionUtilTest.java`,
`MTMVTaskTest.java` (differences = #67186 + #67545 + #66761),
`CreateTableInfo.java` (= #67787) and `StmtExecutor.java` (= #67520 +
#67569 + #66761 + the later session refactors #67835 / #67883 + this
branch's #67861). For the first six files, applying those unrelated
master commits on top of the branch's versions reproduces master's files
exactly; for `StmtExecutor.java`, the diff against master right after
#67753 (`fe39f5b6a42`) consists only of #67520 / #67569 / #66761 /
#67861 hunks. So nothing IVM-related is missing. The regression
framework, plugins and the whole `mtmv_p0/ivm` suite/data directories
are identical to master.

### Verification

- FE: `run-fe-ut.sh --run` on this branch (regenerates thrift, compiles
fe-core main + test) with the 17 test classes touched by the picks or
extending the touched `IvmDeltaTestBase`:
17 classes, 404 tests, 0 failures, 0 errors, BUILD SUCCESS (5:19 min) —
`MTMVPlanUtilTest` 24, `IvmAggDeltaHandlerTest` 33,
`IvmDeltaRewriteHelperTest` 17, `IvmNormalizeMTMVJoinTest` 44,
`IvmJoinDeltaHandlerTest` 23, `IvmDeltaRewriteStateTest` 10,
`IvmPlanSignatureGeneratorTest` 22, `IvmBaselineRebuildTest` 28,
`IvmLinearDeltaHandlerTest` 39, `IvmDeltaRewriterTest` 23,
`IvmNormalizeMTMVUnionTest` 10, `MTMVTaskTest` 50,
`MTMVPropertyUtilTest` 13, `MTMVPartitionUtilTest` 16,
`SchemaChangeHandlerTest` 22, `StmtExecutorInternalQueryTest` 3,
`StmtExecutorTest` 27.
- FE checkstyle on fe-core: 0 violations.
- No BE, cloud or thrift changes in this batch.
- All 18 touched groovy files (framework `Suite.groovy`,
`plugin_planner.groovy`, 16 suites) parse cleanly (groovy parser check).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: yujun <yujun@selectdb.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants