Skip to content

[fix](be) Throw on invalid function return type inference#64050

Open
Mryange wants to merge 1 commit into
apache:masterfrom
Mryange:fixreturntypethrow
Open

[fix](be) Throw on invalid function return type inference#64050
Mryange wants to merge 1 commit into
apache:masterfrom
Mryange:fixreturntypethrow

Conversation

@Mryange
Copy link
Copy Markdown
Contributor

@Mryange Mryange commented Jun 3, 2026

What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary: Some BE function return type inference paths could return nullptr after detecting an invalid return type or unsupported implementation. For struct_element, an invalid field name or index produced a Status but get_return_type_impl returned nullptr, which let the outer return-type check report a generic nullptr mismatch and masked the original error. This change makes these paths throw exceptions directly and keeps the base builder from propagating nullptr return types.

Release note

None

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

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

### What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary: Some BE function return type inference paths could return nullptr after detecting an invalid return type or unsupported implementation. For struct_element, an invalid field name or index produced a Status but get_return_type_impl returned nullptr, which let the outer return-type check report a generic nullptr mismatch and masked the original error. This change makes these paths throw exceptions directly and keeps the base builder from propagating nullptr return types.

### Release note

None

### Check List (For Author)

- Test: Unit Test
    - sh run-be-ut.sh --run --filter="FunctionStructElementTest*"
- Behavior changed: No
- Does this need documentation: No
@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 Jun 3, 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 conclusion: no blocking issues found.

Critical checkpoint conclusions:

  • Goal and tests: The PR makes invalid/unsupported BE function return-type inference fail with explicit exceptions instead of propagating nullptr; the changed code accomplishes that for the touched paths, and the added BE unit test covers invalid struct_element field lookup.
  • Scope and clarity: The change is small and focused on return-type inference error handling.
  • Concurrency: No new concurrency or shared mutable lifecycle behavior is introduced.
  • Lifecycle/static initialization: No new static/global initialization dependencies are introduced.
  • Configuration/compatibility: No new configs, storage formats, or FE-BE protocol fields are introduced.
  • Parallel paths: CAST and struct_element touched paths are consistent with the existing builder exception model; no additional required parallel-path changes were identified in this PR scope.
  • Error handling: New exceptions are in vectorized function preparation/type-resolution paths and are covered by existing VExprContext exception-to-Status boundaries.
  • Tests: Unit coverage was added for the struct_element invalid-field case. I did not run tests in this review runner.
  • Observability/performance: No new observability requirement or meaningful performance regression was identified.

User focus points: No additional user-provided review focus was specified.

@Mryange
Copy link
Copy Markdown
Contributor Author

Mryange commented Jun 3, 2026

run buildall

@hello-stephen
Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 12.50% (1/8) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 72.11% (27540/38191)
Line Coverage 55.52% (294517/530477)
Region Coverage 52.30% (245917/470207)
Branch Coverage 53.43% (106208/198764)

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