Skip to content

[Chore](build) fix conflict field id#60743

Merged
hello-stephen merged 1 commit intoapache:masterfrom
BiteTheDDDDt:fix_0213_2
Feb 13, 2026
Merged

[Chore](build) fix conflict field id#60743
hello-stephen merged 1 commit intoapache:masterfrom
BiteTheDDDDt:fix_0213_2

Conversation

@BiteTheDDDDt
Copy link
Contributor

What problem does this PR solve?

This pull request makes a minor update to the TQueryOptions struct in gensrc/thrift/PaloInternalService.thrift. The change simply reorders the declaration of the enable_aggregate_function_null_v2 option, moving it a few lines down in the struct definition. No functionality is affected.

(gensrc/thrift/PaloInternalService.thriftL428-R436)

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:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason
  • 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

Copilot AI review requested due to automatic review settings February 13, 2026 09:52
@BiteTheDDDDt
Copy link
Contributor Author

run buildall

@hello-stephen
Copy link
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?

@hello-stephen
Copy link
Contributor

skip buildall

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Feb 13, 2026
@github-actions
Copy link
Contributor

PR approved by at least one committer and no changes requested.

@github-actions
Copy link
Contributor

PR approved by anyone and no changes requested.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the TQueryOptions struct in gensrc/thrift/PaloInternalService.thrift to resolve a Thrift field-id conflict by moving enable_aggregate_function_null_v2 to a new field id and position in the struct.

Changes:

  • Remove enable_aggregate_function_null_v2 from Thrift field id 186.
  • Re-add enable_aggregate_function_null_v2 as field id 189 after the other 186/187/188 passthrough options.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 433 to +435
188: optional bool enable_broadcast_join_force_passthrough;

189: optional bool enable_aggregate_function_null_v2 = false;
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Comment on lines 429 to +435
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;
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Copy link
Member

@mrhhsg mrhhsg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@hello-stephen
Copy link
Contributor

FE UT Coverage Report

Increment line coverage `` 🎉
Increment coverage report
Complete coverage report

@hello-stephen
Copy link
Contributor

skip buildall

@hello-stephen hello-stephen merged commit f7d49d5 into apache:master Feb 13, 2026
38 of 40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants