Skip to content

[refactor](be) Remove io_helper helper indirection from BE cast paths - #62179

Merged
zclllyybb merged 13 commits into
apache:masterfrom
zclllyybb:agent/task-a4cc5335-io-helper-refactor
Apr 15, 2026
Merged

[refactor](be) Remove io_helper helper indirection from BE cast paths#62179
zclllyybb merged 13 commits into
apache:masterfrom
zclllyybb:agent/task-a4cc5335-io-helper-refactor

Conversation

@zclllyybb

Copy link
Copy Markdown
Contributor

Problem Summary: Remove the legacy io_helper wrapper layer and move its parsing and formatting entry points onto the existing cast implementations so serde and conversion code depends on the real owners of that behavior.

@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?

@zclllyybb

Copy link
Copy Markdown
ContributorAuthor

run buildall

@zclllyybb
zclllyybb marked this pull request as draft April 7, 2026 14:09
@hello-stephen

Copy link
Copy Markdown
Contributor

BE UT Coverage Report

Increment line coverage 85.29% (116/136) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage52.99% (20101/37937)
Line Coverage36.53% (188919/517160)
Region Coverage32.77% (146508/447075)
Branch Coverage33.93% (64207/189229)

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 86.96% (140/161) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage71.82% (26681/37149)
Line Coverage54.76% (282318/515555)
Region Coverage51.81% (233773/451199)
Branch Coverage53.24% (101049/189805)

@zclllyybb
zclllyybbforce-pushed the agent/task-a4cc5335-io-helper-refactor branch from 26312d6 to 7b1a486CompareApril 8, 2026 14:41
@zclllyybb
zclllyybb marked this pull request as ready for review April 8, 2026 14:41
@zclllyybb

Copy link
Copy Markdown
ContributorAuthor

run buildall

@zclllyybb
zclllyybb requested a review from CopilotApril 8, 2026 14:46
@zclllyybb

Copy link
Copy Markdown
ContributorAuthor

/review

CopilotAI left a comment

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.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR removes the legacy util/io_helper wrapper layer and routes parsing/formatting through the corresponding cast implementations so serialization/conversion code depends on the real owners of that behavior.

Changes:

  • Deleted be/src/util/io_helper.h/.cpp and replaced helper call sites with CastTo* entry points.
  • Centralized numeric parsing helpers into cast_to_basic_number_common.h and decimal text parsing into CastToDecimal.
  • Updated tests and conversion/serde code paths to use CastToString::from_int128 and CastTo*::from_string*_mode(...).

Reviewed changes

Copilot reviewed 50 out of 50 changed files in this pull request and generated 3 comments.

Show a summary per file
FileDescription
be/test/exec/column_type_convert_test.cppUpdates tests to use cast-owned parsing/formatting APIs.
be/test/core/field_test.cppSwitches test dependency from io_helper to cast_to_string.
be/test/common/check_overflow.cppReplaces io_helper include with numeric cast parsing header.
be/src/util/io_helper.hRemoves legacy helper declarations (deleted file).
be/src/util/io_helper.cppRemoves legacy helper implementations (deleted file).
be/src/util/counts.hDrops io_helper include after refactor.
be/src/storage/olap_scan_common.hDrops io_helper include after refactor.
be/src/format/column_type_convert.hReplaces helper calls with CastTo* parsing/formatting implementations.
be/src/exprs/function/function_json.cppDrops io_helper include after refactor.
be/src/exprs/function/cast/cast_to_timestamptz.hDrops io_helper include after refactor.
be/src/exprs/function/cast/cast_to_string.hAdds from_int128 entry points and updates Int128 string conversion.
be/src/exprs/function/cast/cast_to_jsonb.hDrops io_helper include after refactor.
be/src/exprs/function/cast/cast_to_decimal.hMoves decimal text parsing helper into CastToDecimal.
be/src/exprs/function/cast/cast_to_boolean.hInlines boolean parsing via StringParser instead of io_helper.
be/src/exprs/function/cast/cast_to_basic_number_common.hMoves numeric text parsing helpers from io_helper into cast common header.
be/src/exprs/function/cast/cast_base.hDrops io_helper include after refactor.
be/src/exprs/aggregate/moments.hDrops io_helper include after refactor.
be/src/exprs/aggregate/aggregate_function_uniq.hDrops io_helper include and adjusts includes.
be/src/exprs/aggregate/aggregate_function_topn.hDrops io_helper include and adjusts includes.
be/src/exprs/aggregate/aggregate_function_sequence_match.hReplaces io_helper include with numeric cast parsing header.
be/src/exprs/aggregate/aggregate_function_rpc.hDrops io_helper include after refactor.
be/src/exprs/aggregate/aggregate_function_product.hFixes logging for int128_t product by using cast-owned formatting.
be/src/exprs/aggregate/aggregate_function_min_max.hDrops io_helper include and adjusts includes.
be/src/exprs/aggregate/aggregate_function_java_udaf.hDrops io_helper include after refactor.
be/src/exprs/aggregate/aggregate_function_histogram.hDrops io_helper include after refactor.
be/src/exprs/aggregate/aggregate_function_corr.hDrops io_helper include and adds required standard/core includes.
be/src/exprs/aggregate/aggregate_function_collect.hDrops io_helper include after refactor.
be/src/exprs/aggregate/aggregate_function_bit.hDrops io_helper include after refactor.
be/src/exprs/aggregate/aggregate_function_avg_weighted.hDrops io_helper include after refactor.
be/src/exprs/aggregate/aggregate_function_array_agg.hDrops io_helper include after refactor.
be/src/exec/common/histogram_helpers.hppDrops io_helper include after refactor.
be/src/exec/common/hash_table/hash_table.hDrops io_helper include after refactor.
be/src/core/field.cppDrops io_helper include after refactor.
be/src/core/data_type_serde/data_type_number_serde.cppUses cast-owned parsing/formatting instead of helper functions.
be/src/core/data_type_serde/data_type_ipv6_serde.cppUses CastToIPv6 parsing instead of io_helper helpers.
be/src/core/data_type_serde/data_type_ipv4_serde.cppUses CastToIPv4 parsing instead of io_helper helpers.
be/src/core/data_type_serde/data_type_decimal_serde.cppUses CastToDecimal::read_text instead of helper parsing.
be/src/core/data_type_serde/data_type_datev2_serde.cppUses CastToDateV2 parsing instead of helper parsing.
be/src/core/data_type_serde/data_type_datetimev2_serde.cppUses CastToDatetimeV2 parsing instead of helper parsing.
be/src/core/data_type_serde/data_type_date_or_datetime_serde.cppUses CastToDateOrDatetime parsing directly and normalizes types post-parse.
be/src/core/data_type/data_type_quantilestate.cppDrops io_helper include after refactor.
be/src/core/data_type/data_type_number_base.cppUses CastToString::from_int128 for 128-bit formatting.
be/src/core/data_type/data_type_ipv6.cppDrops io_helper include after refactor.
be/src/core/data_type/data_type_ipv4.cppDrops io_helper include after refactor.
be/src/core/data_type/data_type_hll.cppDrops io_helper include after refactor.
be/src/core/data_type/data_type_decimal.cppDrops io_helper include after refactor.
be/src/core/data_type/data_type_date_time.cppDrops io_helper include after refactor.
be/src/core/data_type/data_type_date_or_datetime_v2.cppDrops io_helper include after refactor.
be/src/core/data_type/data_type_date.cppDrops io_helper include after refactor.
be/src/core/data_type/data_type_bitmap.cppDrops io_helper include after refactor.
Comments suppressed due to low confidence (3)

be/src/format/column_type_convert.h:1

  • CastParameters params; is default-initialized here. If CastParameters contains fundamental fields (e.g. bool is_strict) without default member initializers, this can leave them uninitialized and cause nondeterministic parsing behavior. Prefer explicit initialization (e.g. CastParameters params{}; plus setting required fields, or use the same designated initialization pattern used elsewhere such as .status = Status::OK(), .is_strict = false).
    be/src/exprs/function/cast/cast_to_basic_number_common.h:1
  • The static_assert message refers to readFloatTextImpl, which doesn’t match the new function name (try_read_float_text). Updating the message to the current API name will make future failures clearer and reduce confusion during refactors.
    be/src/exprs/function/cast/cast_to_string.h:1
  • This switches 128-bit formatting from FMT_COMPILE(\"{}\") (previously used by int128_to_string) to a runtime format string. For hot formatting paths, consider using FMT_COMPILE with fmt::format_to (or an equivalent compiled-format approach) to avoid repeated runtime parsing of the format string.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadbe/src/core/data_type_serde/data_type_number_serde.cpp
Comment threadbe/src/core/data_type_serde/data_type_number_serde.cpp
Comment threadbe/src/core/data_type_serde/data_type_number_serde.cpp
@github-actions

github-actionsBot commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

No issues found in this refactor after reviewing the full BE cast/serde call chain.

Critical checkpoints:

  • Goal and correctness: Yes. The change removes io_helper indirection and replaces it with equivalent direct cast/parser helpers. The modified paths still normalize DATE/DATETIME values where required, and no behavior regression was identified in the touched call sites.
  • Scope/minimality: Yes. The functional changes are tightly scoped to parser/stringification helper relocation plus include cleanup.
  • Concurrency: Not applicable. The touched code is stateless parsing/serialization helpers with no new shared mutable state or lock interaction.
  • Lifecycle/static init: No issue. No new static/global lifecycle hazards were introduced.
  • Config/compatibility: Not applicable. No config, protocol, symbol-contract, or storage-format compatibility issue was introduced by this patch.
  • Parallel paths: Checked. Equivalent helper replacements were applied consistently across the touched cast, serde, field, and conversion paths.
  • Conditional checks: No problematic new special-case checks found.
  • Test coverage: Adequate for this refactor level. Existing touched BE unit tests cover relevant conversion paths; no obvious missing mandatory test was identified for this behavior-preserving change.
  • Observability: Not needed for this refactor.
  • Transaction/persistence/data-write concerns: Not applicable.
  • FE/BE variable passing: Not applicable.
  • Performance: Acceptable. The refactor removes an indirection layer without adding obvious extra work.
  • Other issues: None found.

Residual risk: compile-only fallout from transitive include changes is still possible, but I did not find orphaned references to the deleted helper in the repository.

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 76.00% (133/175) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage73.55% (27336/37168)
Line Coverage57.18% (295051/515966)
Region Coverage54.30% (245286/451734)
Branch Coverage56.02% (106446/190000)

@zclllyybb
zclllyybb requested a review from CopilotApril 9, 2026 04:40
@zclllyybb

Copy link
Copy Markdown
ContributorAuthor

/review

@github-actionsgithub-actionsBot left a comment

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.

Found 1 issue.

  1. Goal of the task: remove the legacy io_helper wrapper and route parsing/formatting through the real cast owners. The refactor mostly does that, but it does not fully preserve behavior because TYPE_TIME input parsing is now broken in the generic number serde path.
  2. Modification size/focus: mostly small and focused, but the new explicit DataTypeNumberSerDe<TYPE_TIME> instantiation unintentionally expands the affected surface beyond a pure wrapper removal.
  3. Concurrency: no concurrency-sensitive changes were involved.
  4. Lifecycle/static initialization: no special lifecycle or static initialization issues found.
  5. Configuration: no config changes.
  6. Compatibility/incompatible changes: no storage-format change was introduced, but there is a behavioral regression for deprecated TYPE_TIME text input paths.
  7. Parallel code paths: the wrapper removal was applied across most cast/serde/convert paths, but the parallel TYPE_TIME parse path in try_parse_impl() was left incomplete.
  8. Special conditional checks: no problematic new conditional checks beyond the missing TYPE_TIME branch.
  9. Test coverage: the added serde/converter tests are useful, but there is still no coverage for TYPE_TIME generic-serde text parsing (from_string/from_fe_string/batch variants), which is why this regression slipped through.
  10. Observability: no additional observability is needed for this refactor.
  11. Transaction/persistence: not applicable.
  12. Data write/atomicity: not applicable.
  13. FE-BE variable passing: not applicable.
  14. Performance: neutral overall; no clear performance regression found in the reviewed paths.
  15. Other issues: none beyond the regression below.

Critical checkpoints conclusion: the PR does not fully accomplish its goal yet because one reachable input path regresses.

Comment threadbe/src/core/data_type_serde/data_type_number_serde.cpp
@zclllyybb

Copy link
Copy Markdown
ContributorAuthor

run buildall

CopilotAI reviewed Apr 9, 2026

CopilotAI left a comment

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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@zclllyybb
zclllyybb requested a review from CopilotApril 9, 2026 07:35

CopilotAI left a comment

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.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (6)

be/test/exec/column_type_convert_test.cpp:1

  • reinterpret_cast<ColumnPtr&>(src_col) is undefined-behavior prone and can easily break if the smart-pointer types/layouts differ. Prefer creating a real ColumnPtr variable (e.g., ColumnPtr src = src_col;) and passing that to convert, or adjust convert to accept the concrete pointer type used by the tests.
    be/test/exec/column_type_convert_test.cpp:1
  • reinterpret_cast<ColumnPtr&>(src_col) is undefined-behavior prone and can easily break if the smart-pointer types/layouts differ. Prefer creating a real ColumnPtr variable (e.g., ColumnPtr src = src_col;) and passing that to convert, or adjust convert to accept the concrete pointer type used by the tests.
    be/test/exec/column_type_convert_test.cpp:1
  • reinterpret_cast<ColumnPtr&>(src_col) is undefined-behavior prone and can easily break if the smart-pointer types/layouts differ. Prefer creating a real ColumnPtr variable (e.g., ColumnPtr src = src_col;) and passing that to convert, or adjust convert to accept the concrete pointer type used by the tests.
    be/test/exec/column_type_convert_test.cpp:1
  • reinterpret_cast<ColumnPtr&>(src_col) is undefined-behavior prone and can easily break if the smart-pointer types/layouts differ. Prefer creating a real ColumnPtr variable (e.g., ColumnPtr src = src_col;) and passing that to convert, or adjust convert to accept the concrete pointer type used by the tests.
    be/src/format/column_type_convert.h:1
  • This header now directly includes several *_impl.hpp files. If column_type_convert.h is widely included, this can significantly increase compile times and may also increase the risk of ODR/link issues if any of those impl headers contain non-inline definitions. If feasible, keep column_type_convert.h depending on stable interface headers only and move impl-heavy dependencies behind a .cpp boundary (or introduce a smaller adapter header with forward declarations/inline-only glue).
    be/src/exprs/function/cast/cast_to_string.h:1
  • For a hot formatting path like int128→string, using a compile-time format string (e.g., FMT_COMPILE(\"{}\")) can reduce formatting overhead in fmt. This is optional, but worth considering if from_int128 is used frequently in tight loops.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadbe/src/core/data_type_serde/data_type_number_serde.cpp
@hello-stephen

Copy link
Copy Markdown
Contributor

BE UT Coverage Report

Increment line coverage 93.66% (133/142) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage53.01% (20122/37957)
Line Coverage36.55% (189191/517629)
Region Coverage32.82% (146915/447680)
Branch Coverage33.92% (64271/189486)

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 93.48% (129/138) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage73.69% (27387/37167)
Line Coverage57.27% (295515/516013)
Region Coverage54.39% (245750/451799)
Branch Coverage56.10% (106632/190060)

@zclllyybb

Copy link
Copy Markdown
ContributorAuthor

/review

@github-actionsgithub-actionsBot left a comment

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.

I found 1 issue in the current head.

  1. Critical checkpoints
  • Goal and correctness: Partially. The refactor does remove the io_helper indirection, but TYPE_TIME text parsing is now regressed on the generic number serde path, so the current code is not behavior-preserving for all touched owners.
  • Scope/minimality: Yes. The change set stays focused on helper removal and owner API consolidation.
  • Concurrency: Not applicable. I did not find new shared-state or lock interactions in the touched code.
  • Lifecycle/static init: No issue found. No new static/global lifetime hazards were introduced.
  • Config changes: Not applicable.
  • Incompatible changes: No storage/protocol format change found, but there is a behavioral incompatibility on TYPE_TIME string parsing.
  • Parallel paths: Mostly checked. The main remaining gap is that the generic serde/text path was updated without preserving the old TYPE_TIME handling.
  • Special conditional checks: No problematic new conditionals beyond the bug below.
  • Test coverage: Improved, but still missing coverage for DataTypeNumberSerDe<TYPE_TIME>::from_string()/from_olap_string(); that is why this regression survived.
  • Observability: Not needed for this refactor.
  • Transaction/persistence/data-write concerns: Not applicable.
  • FE/BE variable passing: Not applicable.
  • Performance: Acceptable overall; no blocking performance issue found.
  • Other issues: None beyond the finding below.
  1. Finding
  • be/src/core/data_type_serde/data_type_number_serde.cpp:728: try_parse_impl() still only supports float/double, bool, and integer primitive types, but this file now explicitly instantiates DataTypeNumberSerDe<TYPE_TIME> at the bottom. That makes from_string() and from_olap_string() for deprecated TIME columns go through try_parse_impl<TYPE_TIME, ...>(), which now throws NOT_IMPLEMENTED_ERROR instead of parsing the value. This is a real behavior regression for text-input paths that rely on the generic serde owner, including default values / schema-change style flows that call these APIs. Please either keep TYPE_TIME out of the generic DataTypeNumberSerDe text-parsing expansion, or add a PT == TYPE_TIME branch that parses with the same float-like path used elsewhere in this file.

Comment threadbe/src/core/data_type_serde/data_type_number_serde.cpp

CopilotAI left a comment

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.

Pull request overview

Copilot reviewed 58 out of 58 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +259 to +264
StringParser::ParseResult result;
duration =
StringParser::string_to_int<NativeType, false>(pos, end - pos, &result);
while (pos < end && *pos >= '0' && *pos <= '9') {
++pos;
}

CopilotAIApr 9, 2026

Copy link

Choose a reason for hiding this comment

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

StringParser::string_to_int is called with end - pos (the rest of the pattern), but then pos is advanced only over the leading digits. This can cause parsing to fail when the number is immediately followed by non-digit characters (e.g. ")"), leaving duration as an unspecified/incorrect value while pos != prev_pos so no error is reported. Parse using the digit-only slice ([prev_pos, digit_end)) and check the parse result (or at least silence the unused result variable).

Copilot uses AI. Check for mistakes.
Comment on lines +283 to +288
StringParser::ParseResult result;
event_number =
StringParser::string_to_int<UInt64, false>(pos, end - pos, &result);
while (pos < end && *pos >= '0' && *pos <= '9') {
++pos;
}

CopilotAIApr 9, 2026

Copy link

Choose a reason for hiding this comment

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

Same issue as the duration parsing above: string_to_int uses end - pos (including trailing pattern chars), but pos is later advanced only over digits. This can make event_number parse fail while still consuming digits, producing an incorrect event_number without triggering the pos == prev_pos check. Parse using the digit-only substring and check (or explicitly ignore) the parse result.

Copilot uses AI. Check for mistakes.
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: Add focused sequence_match unit coverage for time-condition parsing and malformed numeric tokens, and make parser failures clear parser state so malformed patterns fail closed instead of leaving partial matches behind.
### Release note
None
### Check List (For Author)
- Test: BE Unit Test
- ./run-be-ut.sh --run --filter='*FunctionCastToFloatTest.test_from_decimal*:*FunctionCastToFloatTest.test_decimalv2_keeps_scale_when_cast_to_float_and_double*:*FunctionCastToIntTest.test_from_decimal*:*FunctionCastToDecimalTest*:*ColumnTypeConvertTest*:*CastToStringTest*:*VSequenceMatchTest*' -j60
- Behavior changed: No
- Does this need documentation: No
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: Restore CastToDecimal as the owner-facing decimal-to-decimal API so external callers do not encode decimal storage families, and route decimal-to-int/float cast loops back through the stable public wrappers instead of underscored helpers.
### Release note
None
### Check List (For Author)
- Test: BE Unit Test
- ./run-be-ut.sh --run --filter='*FunctionCastToFloatTest.test_from_decimal*:*FunctionCastToFloatTest.test_decimalv2_keeps_scale_when_cast_to_float_and_double*:*FunctionCastToIntTest.test_from_decimal*:*FunctionCastToDecimalTest*:*ColumnTypeConvertTest*:*CastToStringTest*:*VSequenceMatchTest*' -j60
- Behavior changed: No
- Does this need documentation: No
@zclllyybb
zclllyybbforce-pushed the agent/task-a4cc5335-io-helper-refactor branch from 73d11be to 79669b5CompareApril 15, 2026 07:17
@zclllyybb

Copy link
Copy Markdown
ContributorAuthor

/review

@zclllyybb

Copy link
Copy Markdown
ContributorAuthor

run buildall

github-actions[bot]
github-actionsBot previously requested changes Apr 15, 2026

@github-actionsgithub-actionsBot left a comment

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.

Overall: Request changes. The refactor itself looks mostly behavior-preserving in the reviewed live paths, but the newly added Arrow string coverage is not checking the actual internal representation it claims to validate.

Critical checkpoints:

  • Goal of task: Remove io_helper indirection and route parsing/formatting to cast owners. Mostly achieved. The new coverage intended to prove Arrow STRING -> date-like parsing is currently incorrect, so the goal is not fully demonstrated by tests.
  • Small/clear/focused: Yes, mostly mechanical and focused.
  • Concurrency: No concurrency-sensitive changes in the reviewed paths.
  • Lifecycle/static init: No new lifecycle/SIOF issue found.
  • Configuration: None.
  • Compatibility/storage/protocol: No incompatible API/storage/protocol change found.
  • Parallel code paths: Reviewed the replaced io_helper wrappers; current live paths remain aligned.
  • Special conditionals/comments: No blocking issue found.
  • Test coverage: Affected paths have some BE test coverage, but the new Arrow date-like test is not validating the real packed DateV2/DateTimeV2 values.
  • Modified test results: One new test is incorrect and should be fixed before merge.
  • Observability: N/A for this refactor.
  • Transaction/persistence/data writes: N/A.
  • FE/BE variable passing: N/A.
  • Performance: No blocking performance issue found.
  • Other issues: None beyond the incorrect test.

Comment threadbe/test/core/data_type_serde/data_type_serde_number_test.cpp

CopilotAI left a comment

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.

Pull request overview

Copilot reviewed 59 out of 59 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadbe/src/format/column_type_convert.h
@github-actionsgithub-actionsBot added the approved Indicates a PR has been approved by one committer. label Apr 15, 2026
@github-actions

Copy link
Copy Markdown
Contributor

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

@hello-stephen

Copy link
Copy Markdown
Contributor

BE UT Coverage Report

Increment line coverage 83.91% (266/317) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage53.11% (20177/37993)
Line Coverage36.67% (190055/518222)
Region Coverage32.91% (147497/448164)
Branch Coverage34.06% (64610/189680)

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 86.41% (267/309) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage73.73% (27428/37202)
Line Coverage57.35% (296258/516604)
Region Coverage54.45% (246260/452274)
Branch Coverage56.15% (106826/190248)

@zclllyybb
zclllyybb merged commit 42bf5b2 into apache:masterApr 15, 2026
36 of 37 checks passed
@zclllyybb
zclllyybb deleted the agent/task-a4cc5335-io-helper-refactor branch April 15, 2026 12:07
@zclllyybb

Copy link
Copy Markdown
ContributorAuthor

wait #62865 to pick to 4.1

zclllyybb added a commit that referenced this pull request Apr 30, 2026
…#62179)
Problem Summary: Remove the legacy io_helper wrapper layer and move its
parsing and formatting entry points onto the existing cast
implementations so serde and conversion code depends on the real owners
of that behavior.
zclllyybb added a commit that referenced this pull request Apr 30, 2026
…#62179)
Problem Summary: Remove the legacy io_helper wrapper layer and move its
parsing and formatting entry points onto the existing cast
implementations so serde and conversion code depends on the real owners
of that behavior.
yiguolei pushed a commit that referenced this pull request May 1, 2026
@yiguoleiyiguolei mentioned this pull request May 20, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approvedIndicates a PR has been approved by one committer.dev/4.1.1-mergedreviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@zclllyybb@hello-stephen@jacktengg@Mryange@yiguolei