Skip to content

[improvement](topn) check multiget result rows against request row id count and add be UT (#61758)#61781

Open
jacktengg wants to merge 2 commits intoapache:branch-4.0from
jacktengg:260326-pick-4.0
Open

[improvement](topn) check multiget result rows against request row id count and add be UT (#61758)#61781
jacktengg wants to merge 2 commits intoapache:branch-4.0from
jacktengg:260326-pick-4.0

Conversation

@jacktengg
Copy link
Contributor

Pick #61758

Check multiget result rows matches request row id count when doing merge_multi_response:

  1. A BE may return an empty block event if request.request_block_descs(i).row_id_size() != 0: If the id_file_map was GC'd on the BE before it could process the request, refer 'if (!id_file_map)' in
    RowIdStorageReader::read_by_rowids.
  2. Report error in any case where the row count doesn't match, even if it's not empty,
    since that indicates a bug in BE's row fetching logic or serialization logic.

Also add comments.

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

None

  • 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.

  • Confirm the release note

  • Confirm test cases

  • Confirm document

  • Add branch pick label

What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

Release note

None

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

… count and add be UT (apache#61758)

Check multiget result rows matches request row id count when doing
merge_multi_response:
1. A BE may return an empty block event if
request.request_block_descs(i).row_id_size() != 0:
If the id_file_map was GC'd on the BE before it could process the
request, refer 'if (!id_file_map)' in
RowIdStorageReader::read_by_rowids.
2. Report error in any case where the row count doesn't match, even if
it's not empty,
since that indicates a bug in BE's row fetching logic or serialization
logic.

Also add comments.

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

None

- Test <!-- At least one of them must be included. -->
    - [ ] 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 <!-- Add your reason?  -->

- Behavior changed:
    - [ ] No.
    - [ ] Yes. <!-- Explain the behavior change -->

- Does this need documentation?
    - [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg:
apache/doris-website#1214 -->

- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->
@jacktengg jacktengg requested a review from yiguolei as a code owner March 26, 2026 14:36
@Thearas
Copy link
Contributor

Thearas commented Mar 26, 2026

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?

@jacktengg
Copy link
Contributor Author

run buildall

@jacktengg
Copy link
Contributor Author

run buildall

@hello-stephen
Copy link
Contributor

BE UT Coverage Report

Increment line coverage 45.45% (10/22) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 52.94% (19208/36284)
Line Coverage 36.11% (178947/495553)
Region Coverage 32.74% (138732/423755)
Branch Coverage 33.71% (60269/178804)

@hello-stephen
Copy link
Contributor

BE Regression && UT Coverage Report

Increment line coverage 95.45% (21/22) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 71.36% (25349/35521)
Line Coverage 54.03% (267291/494670)
Region Coverage 51.56% (220675/428031)
Branch Coverage 53.08% (95243/179447)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants