Skip to content

[refine](code) remove dead runtime helpers and deprecated MaxCompute fields#63026

Open
Mryange wants to merge 1 commit intoapache:masterfrom
Mryange:dead-code-cleanup-part-2
Open

[refine](code) remove dead runtime helpers and deprecated MaxCompute fields#63026
Mryange wants to merge 1 commit intoapache:masterfrom
Mryange:dead-code-cleanup-part-2

Conversation

@Mryange
Copy link
Copy Markdown
Contributor

@Mryange Mryange commented May 6, 2026

What problem does this PR solve?

Issue Number: N/A

Problem Summary:

This commit cleans up dead BE-side runtime and scan code that remained after earlier refactors.

GenericReader no longer has any out-of-line logic, several CollectionValue, ResultQueueMgr, RuntimeState, and descriptor helpers have no remaining callers, and MaxComputeTableDescriptor still cached deprecated TMCTable fields and rebuilt authentication properties from them even though FE already sends authentication settings through TMCTable.properties.

Root cause: deprecated compatibility code and empty translation units were left behind after the MaxCompute path moved to the properties-based contract and after reader/runtime logic was moved or inlined elsewhere. This change removes the empty source files, drops the unused helpers and accessors, and keeps MaxComputeTableDescriptor focused on the fields that are still consumed at runtime.

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

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

@Mryange
Copy link
Copy Markdown
Contributor Author

Mryange commented May 6, 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.

Requesting changes for one compatibility regression in the BE cleanup.

Critical checkpoint conclusions:

  • Goal/test proof: the PR aims to remove dead code; most removed accessors/helpers appear unused, but the MaxCompute deprecated-field fallback is still part of the FE-BE thrift compatibility surface and is not covered by tests here.
  • Scope/focus: generally focused on BE cleanup, except it removes behavior needed for old TMCTable payloads.
  • Concurrency: no concurrency changes found in the actual GitHub PR diff.
  • Lifecycle/static init: no new static initialization or lifecycle issue found.
  • Config/protocol compatibility: issue found; removing deprecated MaxCompute credential fallback can break mixed-version or old serialized descriptors.
  • Parallel paths: no missing parallel code path found in the reviewed diff.
  • Testing: no test coverage was added for MaxCompute descriptor compatibility.
  • Observability/performance/transaction/data-write concerns: not materially involved by the cleanup.
  • User focus: no additional user-provided focus points were supplied.

Comment thread be/src/runtime/descriptors.cpp
@Mryange Mryange force-pushed the dead-code-cleanup-part-2 branch from 5dc8594 to b82ab83 Compare May 6, 2026 09:57
@Mryange
Copy link
Copy Markdown
Contributor Author

Mryange commented May 6, 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.

Review summary:

I reviewed the current GitHub PR diff and the existing inline review context. I did not find additional issues in the currently reviewable BE runtime cleanup files (, , , , and related removed empty translation units). I am not duplicating the already-open MaxCompute descriptor compatibility thread.

Critical checkpoint conclusions:

  • Goal and tests: The current reviewable diff is a dead-code/runtime-helper cleanup. The removed helpers appear unused in the current tree; no new functional test is required for those removals, but the existing MaxCompute compatibility concern remains tracked separately.
  • Scope/focus: The reviewable changes are small and focused on unused BE helpers/empty source files.
  • Concurrency/lifecycle: No new concurrency or non-trivial lifecycle management is introduced in the current reviewable diff.
  • Config/compatibility/protocol: No new config or protocol changes are present in the current reviewable diff. Compatibility concern for MaxCompute deprecated fields is already raised in the existing thread.
  • Parallel paths/data correctness/memory safety: No additional data correctness, memory tracking, or parallel-path issues found in the reviewed BE cleanup files.
  • Observability/performance: No new observability requirement or performance concern found.

User focus: No additional user-provided review focus was supplied.

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.

Review summary:

I reviewed the current GitHub PR diff and the existing inline review context. I did not find additional issues in the currently reviewable BE runtime cleanup files (GenericReader, CollectionValue, ResultQueueMgr, RuntimeState, and related removed empty translation units). I am not duplicating the already-open MaxCompute descriptor compatibility thread.

Critical checkpoint conclusions:

  • Goal and tests: The current reviewable diff is a dead-code/runtime-helper cleanup. The removed helpers appear unused in the current tree; no new functional test is required for those removals, but the existing MaxCompute compatibility concern remains tracked separately.
  • Scope/focus: The reviewable changes are small and focused on unused BE helpers/empty source files.
  • Concurrency/lifecycle: No new concurrency or non-trivial lifecycle management is introduced in the current reviewable diff.
  • Config/compatibility/protocol: No new config or protocol changes are present in the current reviewable diff. Compatibility concern for MaxCompute deprecated fields is already raised in the existing thread.
  • Parallel paths/data correctness/memory safety: No additional data correctness, memory tracking, or parallel-path issues found in the reviewed BE cleanup files.
  • Observability/performance: No new observability requirement or performance concern found.

User focus: No additional user-provided review focus was supplied.

@Mryange
Copy link
Copy Markdown
Contributor Author

Mryange commented May 6, 2026

run buildall

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.

2 participants