[fix](be) Throw on invalid function return type inference#64050
Open
Mryange wants to merge 1 commit into
Open
Conversation
### 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
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
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.
Contributor
Author
|
run buildall |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)