Skip to content

[refine](code) remove dead code across core types and utilities#62994

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

[refine](code) remove dead code across core types and utilities#62994
Mryange wants to merge 2 commits intoapache:masterfrom
Mryange:dead-code-cleanup-part-1

Conversation

@Mryange
Copy link
Copy Markdown
Contributor

@Mryange Mryange commented May 4, 2026

What problem does this PR solve?

Issue Number: N/A

Problem Summary:

This PR cleans up dead code accumulated across BE core files — unused unions, methods, type
traits, free functions, forward declarations, and includes that are never referenced in the
codebase. Specific removals include:

  • binary_cast.hpp: 5 unreferenced type-punning unions (TypeConverter, DecimalInt128Union,
    VecDateTimeInt64Union, DateV2UInt32Union, DateTimeV2UInt64Union)
  • block.cpp/h: unused columns_bytes() and clone_with_columns() methods, plus stale includes
    (sys/types.h, iterator, config.h)
  • string_ref.cpp/h: unused single-char start_with/end_with overloads and the
    min_string_val/max_string_val/MIN_CHAR/MAX_CHAR scaffolding that served type_limit
    specializations
  • type_limit.h: type_limit and type_limitstd::string specializations that depended
    on the removed string_ref helpers
  • number_traits.h: 6 unused type-trait structs (ResultOfAdditionMultiplication,
    ResultOfSubtraction, ResultOfFloatingPointDivision, ResultOfIntegerDivision, ResultOfModulo,
    ResultOfBit, BinaryOperatorTraits) and stale includes
  • column_variant.cpp/h: unused get_last_field() and is_doc_mode() methods
  • nested_utils.cpp/h: unused extract_table_name() and declaration of validate_array_sizes()
  • map_value.cpp, struct_value.cpp: entire files deleted — each contained only a single unused
    shallow_copy method
  • call_on_type_index.h: stale forward declaration of DataTypeEnum
  • data_type_array_serde.h: stale forward declaration of JsonWriter
  • function_array_enumerate_uniq.cpp, cgroup_util.cpp: added missing transitive includes exposed
    after dead-code removal (column_decimal.h, boost/algorithm/string.hpp)

Corresponding tests that exercised only the deleted functions were also removed (block_test.cpp,
column_variant_test.cpp, data_type_serde_test.cpp, variant_util_test.cpp).

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 4, 2026

run buildall

@Mryange Mryange force-pushed the dead-code-cleanup-part-1 branch from 939c37f to b42a76b Compare May 4, 2026 14:39
@Mryange
Copy link
Copy Markdown
Contributor Author

Mryange commented May 4, 2026

run buildall

@Mryange Mryange force-pushed the dead-code-cleanup-part-1 branch from b42a76b to de64b1d Compare May 4, 2026 14:46
@Mryange
Copy link
Copy Markdown
Contributor Author

Mryange commented May 4, 2026

run buildall

@Mryange Mryange force-pushed the dead-code-cleanup-part-1 branch from de64b1d to 9f7c8c9 Compare May 5, 2026 03:00
@Mryange
Copy link
Copy Markdown
Contributor Author

Mryange commented May 5, 2026

run buildall

@Mryange
Copy link
Copy Markdown
Contributor Author

Mryange commented May 5, 2026

run buildall

@Mryange
Copy link
Copy Markdown
Contributor Author

Mryange commented May 5, 2026

run p0

@Mryange
Copy link
Copy Markdown
Contributor Author

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

I found two blocking build/API regressions in this cleanup. The deleted helpers are still referenced by the benchmark target, and one removed Block method still has a public declaration without a definition.

Critical checkpoint conclusions:

  • Goal/test proof: the goal is dead-code cleanup. It is only partially accomplished because references remain in active source/build paths; no test/build evidence is provided in the PR checklist.
  • Scope: the change is mostly focused, but missed dependent benchmark/header cleanup.
  • Concurrency/lifecycle/config/compatibility/data writes/transactions: not applicable; this is BE API/source cleanup only.
  • Parallel paths: benchmark code and public headers are parallel consumers of the removed helpers and must be updated together.
  • Testing: existing tests that only covered deleted APIs were removed, but there is no demonstrated BE build, BE UT, or benchmark build coverage; BUILD_BENCHMARK would catch one issue.
  • Observability/performance: not applicable; no runtime behavior path is intentionally changed.
  • User focus: no additional user-provided focus was present.

Please fix the stale references/declaration and run an appropriate BE build/test target before merge.

#include "core/value/vdatetime_value.h"

namespace doris {
union TypeConverter {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Removing these union helpers breaks the benchmark target. be/benchmark/benchmark_main.cpp still includes binary_cast_benchmark.hpp, and that header's old_binary_cast() still references TypeConverter, VecDateTimeInt64Union, DateV2UInt32Union, DateTimeV2UInt64Union, and DecimalInt128Union. Since be/CMakeLists.txt builds this file when BUILD_BENCHMARK is enabled, -DBUILD_BENCHMARK=ON will no longer compile. Please either update/remove the benchmark's old implementation or keep local benchmark-only definitions.

return res;
}

std::string Block::columns_bytes() const {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The implementation is removed, but be/src/core/block/block.h still declares std::string columns_bytes() const;. That leaves a public Block API with no definition, so any caller that includes the header and calls this method will fail at link time. Please remove the declaration as well, or keep the definition if the API should remain available.

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