github-actions[bot] commented on code in PR #61050:
URL: https://github.com/apache/doris/pull/61050#discussion_r2886968213
##########
fe/fe-core/src/main/java/org/apache/doris/mysql/MysqlOkPacket.java:
##########
@@ -59,8 +59,13 @@ public void writeTo(MysqlSerializer serializer) {
// if ((STATUS_FLAGS &
MysqlStatusFlag.SERVER_SESSION_STATE_CHANGED) != 0) {
// }
} else {
- if (!Strings.isNullOrEmpty(infoMessage)) {
- // NOTE: in datasheet, use EOF string, but in the code, mysql
use length encoded string
+ // Always write the info field as a length-encoded string.
+ // When CLIENT_DEPRECATE_EOF is negotiated, the driver's
OkPacket.parse()
+ // unconditionally reads STRING_LENENC for info, so an empty
string must
+ // still be written (as a single 0x00 byte representing length 0).
+ if (Strings.isNullOrEmpty(infoMessage)) {
Review Comment:
[Nit] This change affects ALL clients, not just those that negotiated
`CLIENT_DEPRECATE_EOF`. Previously, when `infoMessage` was empty, nothing was
written to the packet. Now a `0x00` byte (empty lenenc string) is always
appended.
Per MySQL protocol spec, the info field is documented as `string<lenenc>`
and should always be present, so this is technically more correct. Standard
clients read until end-of-packet for any remaining fields, so the extra byte
should be harmless. Just flagging this behavioral change for awareness during
testing with various client drivers (not just MySQL Connector/J).
##########
fe/fe-core/src/main/java/org/apache/doris/qe/ConnectProcessor.java:
##########
@@ -553,7 +554,17 @@ protected void handleFieldList(String tableName) throws
ConnectionException {
// only Mysql protocol
Review Comment:
[Medium] **Proxy forwarding path does not propagate CLIENT_DEPRECATE_EOF.**
When `getResultPacket()` is called in the `proxyExecute()` context (line 734
of this file), `ctx.getMysqlChannel()` is a `ProxyMysqlChannel` where
`clientDeprecatedEOF` defaults to `false`. This means the final status packet
sent back to the follower will always be a traditional EOF packet, even if the
original client negotiated `CLIENT_DEPRECATE_EOF`.
Moreover, the intermediate data packets buffered by `ProxyMysqlChannel`
during query execution (via `sendMetaData`/`sendFields`) will **always
include** intermediate EOF packets (since the proxy channel's
`clientDeprecatedEOF()` returns false). The follower's `sendProxyQueryResult()`
relays these raw bytes directly to the client without adaptation.
If a `CLIENT_DEPRECATE_EOF` client connects to a follower FE that forwards
queries to master:
1. The client receives unexpected intermediate EOF packets after column
definitions
2. The client receives a traditional 5-byte EOF packet instead of a
ResultSet OK packet
This can cause protocol desync or parsing errors in the driver.
**Suggested fix:** Propagate the client's `clientDeprecatedEOF` flag through
`TMasterOpRequest` and call `setClientDeprecatedEOF()` on the
`ProxyMysqlChannel` in the master's `proxyExecute()` before query execution.
This way the master generates packets matching the original client's
capabilities.
Alternatively, if this is a pre-existing limitation of the proxy path (since
it also doesn't propagate other client capabilities), please document it as a
known issue.
--
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]
