Copilot commented on code in PR #60743:
URL: https://github.com/apache/doris/pull/60743#discussion_r2803325883
##########
gensrc/thrift/PaloInternalService.thrift:
##########
@@ -425,14 +425,15 @@ struct TQueryOptions {
184: optional i32 cte_max_recursion_depth;
185: optional bool enable_parquet_file_page_cache = true;
- 186: optional bool enable_aggregate_function_null_v2 = false;
186: optional bool enable_streaming_agg_hash_join_force_passthrough;
187: optional bool enable_distinct_streaming_agg_force_passthrough;
188: optional bool enable_broadcast_join_force_passthrough;
+ 189: optional bool enable_aggregate_function_null_v2 = false;
Review Comment:
This change does more than reorder fields: it changes the Thrift field id
for `enable_aggregate_function_null_v2` from 186 to 189. That alters the wire
serialization contract, so the PR description claiming “no functionality is
affected” is inaccurate; please update the description (and release notes if
required) to reflect the compatibility impact and rationale for picking the new
id.
##########
gensrc/thrift/PaloInternalService.thrift:
##########
@@ -425,14 +425,15 @@ struct TQueryOptions {
184: optional i32 cte_max_recursion_depth;
185: optional bool enable_parquet_file_page_cache = true;
- 186: optional bool enable_aggregate_function_null_v2 = false;
186: optional bool enable_streaming_agg_hash_join_force_passthrough;
187: optional bool enable_distinct_streaming_agg_force_passthrough;
188: optional bool enable_broadcast_join_force_passthrough;
+ 189: optional bool enable_aggregate_function_null_v2 = false;
Review Comment:
Changing a `TQueryOptions` field id can break mixed-version compatibility:
any serialized payload that previously set field 186 for
`enable_aggregate_function_null_v2` will now be interpreted as
`enable_streaming_agg_hash_join_force_passthrough`. If this struct crosses
FE/BE boundaries across versions, consider keeping the existing id for the
already-released option (and renumbering the newer/conflicting one), or
otherwise document/enforce an upgrade constraint to avoid misinterpretation.
--
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]
