RongtongJin commented on code in PR #10536:
URL: https://github.com/apache/rocketmq/pull/10536#discussion_r3449490317
##########
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/header/PullMessageRequestHeader.java:
##########
@@ -85,13 +85,13 @@ public void encode(ByteBuf out) {
writeIfNotNull(out, "topic", topic);
writeIfNotNull(out, "liteTopic", liteTopic);
writeIfNotNull(out, "queueId", queueId);
- writeIfNotNull(out, "queueOffset", queueOffset);
+ writeLong(out, "queueOffset", queueOffset);
writeIfNotNull(out, "maxMsgNums", maxMsgNums);
writeIfNotNull(out, "sysFlag", sysFlag);
- writeIfNotNull(out, "commitOffset", commitOffset);
- writeIfNotNull(out, "suspendTimeoutMillis", suspendTimeoutMillis);
+ writeLong(out, "commitOffset", commitOffset);
+ writeLong(out, "suspendTimeoutMillis", suspendTimeoutMillis);
writeIfNotNull(out, "subscription", subscription);
- writeIfNotNull(out, "subVersion", subVersion);
+ writeLong(out, "subVersion", subVersion);
Review Comment:
This changes the missing-field behavior for `subVersion`: the old `Long` +
`writeIfNotNull` path omitted it when unset, but the new primitive path always
serializes `0`. There are proxy paths that set `subscription`/`expressionType`
without setting `subVersion` (`PullMessageActivity` when it injects a
subscription, and `ConsumerProcessor#pullMessage` when building the header).
The broker uses `requestHeader.getSubVersion()` as the client filter version
for non-tag filters, so defaulting to `0` can hide a missing required field and
register/compare stale SQL filter data. Please either keep `subVersion`
nullable/omitted here, or set it from `SubscriptionData#getSubVersion()` in
those proxy paths and add a regression 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]
