github-actions[bot] commented on code in PR #67520:
URL: https://github.com/apache/doris/pull/67520#discussion_r3932494001
##########
fe/fe-core/src/main/java/org/apache/doris/qe/ConnectProcessor.java:
##########
@@ -654,8 +654,23 @@ public void finalizeCommand() throws IOException {
&& ctx.getState().getStateType() !=
QueryState.MysqlStateType.ERR) {
ShowResultSet resultSet = executor.getShowResultSet();
if (resultSet == null) {
- executor.sendProxyQueryResult();
- packet = executor.getOutputPacket();
+ if (ctx.getMysqlChannel().clientDeprecatedEOF()
+ && !executor.isForwardedClientDeprecatedEofApplied()
+ && executor.getProxyStatusCode() == 0) {
Review Comment:
[P1] Classify successful old-master reads without widening this guard
A real forwarded SELECT cannot satisfy this gate: result producers finish
with `QueryState.setEof()`, while `proxyExecute` assigns status 0 only to `OK`
and maps successful EOF to 1105. The follower therefore replays the unsafe
cursor packets; the unit test mocks the impossible combination of query buffers
plus status 0. Simply accepting EOF here would also reject safe old-master
`COM_QUERY` and Connector/J 9.5+ results because this predicate never checks
cursor intent or the compatibility class. Please recognize real result-set
success, scope rejection to requests that need the cursor shim, test through
real `proxyExecute` construction, and ensure a non-final multi-statement sends
the local ERR only once.
##########
fe/fe-core/src/main/java/org/apache/doris/qe/ConnectProcessor.java:
##########
@@ -654,8 +654,23 @@ public void finalizeCommand() throws IOException {
&& ctx.getState().getStateType() !=
QueryState.MysqlStateType.ERR) {
ShowResultSet resultSet = executor.getShowResultSet();
if (resultSet == null) {
- executor.sendProxyQueryResult();
- packet = executor.getOutputPacket();
+ if (ctx.getMysqlChannel().clientDeprecatedEOF()
+ && !executor.isForwardedClientDeprecatedEofApplied()
+ && executor.getProxyStatusCode() == 0) {
+ if (executor.hasForwardedQueryResultPackets()) {
+
ctx.getState().setError(ErrorCode.ERR_NOT_SUPPORTED_YET,
+ "The master FE cannot preserve
CLIENT_DEPRECATE_EOF while forwarding this query. "
+ + "Connect to the master FE or finish
the FE rolling upgrade");
+ } else {
+ // An old master has already completed a DDL/DML
operation. Rebuild its final OK locally
+ // instead of returning an upgrade error that could
make the client retry side effects.
+
ctx.getState().setOk(executor.getForwardedAffectedRows(), 0, null);
Review Comment:
[P2] Preserve the old master's complete OK result
This rolling-upgrade branch rebuilds a successful DML response with only
`affectedRows`, discarding the warning count and info string already encoded in
the master's final OK packet. A normal forwarded INSERT calls
`OlapInsertExecutor.setReturnInfo()`, which reports `filteredRows` as warnings
and includes label/status/txnId in info; through an old master this branch
changes those to zero warnings and no message. Please preserve or decode all
protocol-visible OK fields (or safely reuse the ordinary OK packet) and cover a
response with nonzero warnings and nonempty info.
##########
fe/fe-core/src/main/java/org/apache/doris/mysql/MysqlOkPacket.java:
##########
@@ -58,16 +58,11 @@ public void writeTo(MysqlSerializer serializer) {
// TODO(zhaochun): STATUS_FLAGS
// if ((STATUS_FLAGS &
MysqlStatusFlag.SERVER_SESSION_STATE_CHANGED) != 0) {
// }
- } else {
- // 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)) {
- serializer.writeVInt(0);
- } else {
- serializer.writeLenEncodedString(infoMessage);
- }
+ } else if (!Strings.isNullOrEmpty(infoMessage)) {
+ serializer.writeLenEncodedString(infoMessage);
+ } else if (capability.isDeprecatedEOF()) {
Review Comment:
[P2] Read the negotiated EOF capability here
This condition is false in the new unit test, but it remains true for real
legacy-EOF connections. `MysqlProto.negotiate` records the client's bit only in
`MysqlChannel`, then sets the serializer capability to
`context.getServerCapability()`; that default mask always includes
`CLIENT_DEPRECATE_EOF`. `ProxyMysqlChannel` starts with the same default as
well. Consequently an authenticated client that did not negotiate the flag
still gets the trailing zero byte this change intends to remove. Please key
this from the negotiated/channel capability, and propagate it to proxy
serialization, or store the negotiated mask in the serializer, with a
handshake-level test.
##########
fe/fe-core/src/main/java/org/apache/doris/qe/ConnectProcessor.java:
##########
@@ -729,6 +744,8 @@ public TMasterOpResult proxyExecute(TMasterOpRequest
request) throws TException
if (request.isSetClientDeprecatedEOF() &&
request.isClientDeprecatedEOF()) {
ctx.getMysqlChannel().setClientDeprecatedEOF();
}
+ ctx.setCursorFetchRequested(request.isSetCursorFetchRequested()
Review Comment:
[P1] Handle cursor intent from old forwarding FEs
During a rolling upgrade an old follower cannot set the new optional
`cursor_fetch_requested` field, so this silently records `false`. It still
forwards `CLIENT_DEPRECATE_EOF`, Connector/J attributes, and, for parameterized
statements, the execute buffer; the new master then emits binary rows but omits
the compatibility metadata marker. An affected Connector/J cursor SELECT
forwarded through that old FE can therefore still consume the final marker and
hang. Please treat an absent cursor-intent field as an explicit
mixed-version/unknown execute mode and fail safely when the affected
combination cannot be disambiguated, with an old-sender/new-master
parameterized cursor test.
--
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]
