Skip to content

[fix](snapshot) drop index should set db_id (#58401)#62770

Merged
yiguolei merged 1 commit intoapache:branch-4.1from
w41ter:pick-60738-b41
May 1, 2026
Merged

[fix](snapshot) drop index should set db_id (#58401)#62770
yiguolei merged 1 commit intoapache:branch-4.1from
w41ter:pick-60738-b41

Conversation

@w41ter
Copy link
Copy Markdown
Contributor

@w41ter w41ter commented Apr 24, 2026

cherry-pick #58401

recycle_indexes require db_id to recycle multi versioned keys, but drop index does not set db_id

@w41ter w41ter requested a review from yiguolei as a code owner April 24, 2026 02:40
@Thearas
Copy link
Copy Markdown
Contributor

Thearas commented Apr 24, 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?

@w41ter
Copy link
Copy Markdown
Contributor Author

w41ter commented Apr 24, 2026

run buildall

@w41ter
Copy link
Copy Markdown
Contributor Author

w41ter commented Apr 24, 2026

/review

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

No blocking issues found.

Critical checkpoints:

  • Correctness: Pass. The patch consistently threads through the FE call chain that reaches cloud , including table erase, rollup/schema-change cancel, MV/rollup drop, restore, recycle-bin cleanup, and replay.
  • Completeness: Pass. I did not find remaining FE-side callers of , , , or in this branch that still omit .
  • Compatibility: Pass for the touched paths. The replay/recycle metadata used here already carries (, ), so the new argument is available where this PR consumes it.
  • Tests: No new automated coverage was added in this PR. Existing cloud/meta-service tests already exercise with , but FE-side propagation still relies on code inspection rather than a dedicated regression test.

User focus:

  • No additional user-provided review focus was supplied.

Residual risk:

  • I did not run FE unit tests in this review session.

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

No blocking issues found.

Critical checkpoints:

  • Correctness: Pass. The patch consistently threads dbId through the FE call chain that reaches cloud drop_index, including table erase, rollup/schema-change cancel, MV/rollup drop, restore, recycle-bin cleanup, and replay.
  • Completeness: Pass. I did not find remaining FE-side callers of onEraseOlapTable, eraseTableDropBackendReplicas, eraseDroppedIndex, or dropMaterializedIndex in this branch that still omit dbId.
  • Compatibility: Pass for the touched paths. The replay/recycle metadata used here already carries dbId (DropInfo, RecycleTableInfo), so the new argument is available where this PR consumes it.
  • Tests: No new automated coverage was added in this PR. Existing cloud/meta-service tests already exercise drop_index with db_id, but FE-side propagation still relies on code inspection rather than a dedicated regression test.

User focus:

  • No additional user-provided review focus was supplied.

Residual risk:

  • I did not run FE unit tests in this review session.

@hello-stephen
Copy link
Copy Markdown
Contributor

FE UT Coverage Report

Increment line coverage 37.50% (6/16) 🎉
Increment coverage report
Complete coverage report

@hello-stephen
Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 75.00% (12/16) 🎉
Increment coverage report
Complete coverage report

@w41ter
Copy link
Copy Markdown
Contributor Author

w41ter commented Apr 24, 2026

run buildall

@hello-stephen
Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 75.00% (12/16) 🎉
Increment coverage report
Complete coverage report

@morningman
Copy link
Copy Markdown
Contributor

run buildall

@hello-stephen
Copy link
Copy Markdown
Contributor

FE UT Coverage Report

Increment line coverage 37.50% (6/16) 🎉
Increment coverage report
Complete coverage report

@hello-stephen
Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 87.50% (14/16) 🎉
Increment coverage report
Complete coverage report

@hello-stephen
Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 4.88% (14/287) 🎉
Increment coverage report
Complete coverage report

@hello-stephen
Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 3.46% (14/405) 🎉
Increment coverage report
Complete coverage report

@yiguolei yiguolei merged commit 7398d4b into apache:branch-4.1 May 1, 2026
28 of 31 checks passed
@w41ter w41ter deleted the pick-60738-b41 branch May 6, 2026 02:42
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.

5 participants