Skip to content

Conversation

@qidaye
Copy link
Contributor

@qidaye qidaye commented Nov 6, 2024

What problem does this PR solve?

This PR is a followup repair on the basis of #42882.
Building index on renamed column will produce an empty index file, when column_idx is not found.
The query is successful because match without inverted index.
Move test_match_without_index to fault_injection folder and make it non-concurrent to avoid concurrency issues.

Check List (For Committer)

  • 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 colde files have been changed.
      • Other reason
  • Behavior changed:

    • No.
    • Yes.
  • Does this need documentation?

    • No.
    • Yes.
  • Release note

    Fix build index empty index file on renamed column

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@doris-robot
Copy link

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

@qidaye
Copy link
Contributor Author

qidaye commented Nov 6, 2024

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2024

clang-tidy review says "All clean, LGTM! 👍"

airborne12
airborne12 previously approved these changes Nov 6, 2024
Copy link
Member

@airborne12 airborne12 left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Nov 6, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2024

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

@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2024

PR approved by anyone and no changes requested.

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 37.89% (9848/25989)
Line Coverage: 29.05% (81909/281913)
Region Coverage: 28.28% (42190/149187)
Branch Coverage: 24.85% (21401/86112)
Coverage Report: http://coverage.selectdb-in.cc/coverage/1153202c6600c1950557cce4dce1c6ea2b75eb52_1153202c6600c1950557cce4dce1c6ea2b75eb52/report/index.html

@github-actions github-actions bot removed the approved Indicates a PR has been approved by one committer. label Nov 6, 2024
@qidaye
Copy link
Contributor Author

qidaye commented Nov 6, 2024

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2024

clang-tidy review says "All clean, LGTM! 👍"

airborne12
airborne12 previously approved these changes Nov 6, 2024
Copy link
Member

@airborne12 airborne12 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@csun5285 csun5285 left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2024

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

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Nov 6, 2024
@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 37.88% (9849/26000)
Line Coverage: 29.04% (81909/282037)
Region Coverage: 28.27% (42191/149244)
Branch Coverage: 24.85% (21404/86140)
Coverage Report: http://coverage.selectdb-in.cc/coverage/93582f9f8df92d1815fa84aa9862174b0de34772_93582f9f8df92d1815fa84aa9862174b0de34772/report/index.html

@qidaye
Copy link
Contributor Author

qidaye commented Nov 6, 2024

run buildall

@github-actions github-actions bot removed the approved Indicates a PR has been approved by one committer. label Nov 6, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2024

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 37.88% (9848/25998)
Line Coverage: 29.03% (81887/282031)
Region Coverage: 28.26% (42170/149241)
Branch Coverage: 24.84% (21396/86142)
Coverage Report: http://coverage.selectdb-in.cc/coverage/50c01d693196f50e2388bc8b62129bcc6aa8f797_50c01d693196f50e2388bc8b62129bcc6aa8f797/report/index.html

@qidaye
Copy link
Contributor Author

qidaye commented Nov 6, 2024

run buildall

@qidaye
Copy link
Contributor Author

qidaye commented Nov 6, 2024

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2024

clang-tidy review says "All clean, LGTM! 👍"

1 similar comment
@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2024

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 37.88% (9848/25998)
Line Coverage: 29.02% (81855/282038)
Region Coverage: 28.25% (42169/149248)
Branch Coverage: 24.84% (21395/86148)
Coverage Report: http://coverage.selectdb-in.cc/coverage/f3a5cf660c52065cc0bf82cf7c7d589028e982d6_f3a5cf660c52065cc0bf82cf7c7d589028e982d6/report/index.html

Copy link
Contributor

@csun5285 csun5285 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@airborne12 airborne12 left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Nov 7, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2024

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

@airborne12 airborne12 merged commit 585759a into apache:master Nov 7, 2024
github-actions bot pushed a commit that referenced this pull request Nov 7, 2024
…43336)

### What problem does this PR solve?

This PR is a followup repair on the basis of #42882.
Building index on renamed column will produce an empty index file, when
`column_idx` is not found.
The query is successful because match without inverted index.
Move test_match_without_index to fault_injection folder and make it
non-concurrent to avoid concurrency issues.
@qidaye qidaye deleted the fix_build_index_empty_index branch November 7, 2024 03:32
qidaye added a commit to qidaye/incubator-doris that referenced this pull request Nov 7, 2024
…pache#43336)

### What problem does this PR solve?

This PR is a followup repair on the basis of apache#42882.
Building index on renamed column will produce an empty index file, when
`column_idx` is not found.
The query is successful because match without inverted index.
Move test_match_without_index to fault_injection folder and make it
non-concurrent to avoid concurrency issues.
qidaye added a commit that referenced this pull request Nov 7, 2024
…amed column (#43392)

Cherry-picked from #43336

Co-authored-by: qiye <jianliang5669@gmail.com>
qidaye added a commit that referenced this pull request Nov 7, 2024
…43246)(#43336) (#43266)

bp #42882 #43246 #43336
We pick these PRs together because they all fix one problem and if not
do so, the regression test won't be passed.

---------

Co-authored-by: Sun Chenyang <csun5285@gmail.com>
Co-authored-by: qidaye <luen@selectdb.com>
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. dev/2.1.8-merged dev/3.0.3-merged reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants