Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Public API ScalarUDFImpl.return_type returns internal error in some cases #13716

Open
andygrove opened this issue Dec 10, 2024 · 1 comment · May be fixed by #13717
Open

Public API ScalarUDFImpl.return_type returns internal error in some cases #13716

andygrove opened this issue Dec 10, 2024 · 1 comment · May be fixed by #13717
Labels
bug Something isn't working

Comments

@andygrove
Copy link
Member

andygrove commented Dec 10, 2024 •
edited
Loading

Describe the bug

I am trying to upgrade the version of DataFusion that Comet uses. Comet currently relies on ScalarUDFImpl.return_type to get the return type of a scalar UDF. This now returns an internal error when called for DatePartFunc.

    fn return_type(&self, _arg_types: &[DataType]) -> Result<DataType> {
        internal_err!("return_type_from_exprs shoud be called instead")
    }

Should we deprecate this function if it is no longer the correct approach for determining UDF return types? It looks like return_type_from_exprs should now be used instead?

To Reproduce

No response

Expected behavior

No response

Additional context

No response
@andygrove andygrove added the bug Something isn't working label Dec 10, 2024
@andygrove andygrove linked a pull request Dec 10, 2024 that will close this issue
Open
@findepi
Copy link
Member

findepi commented Dec 11, 2024

Should we deprecate this function if it is no longer the correct approach for determining UDF return types? It looks like return_type_from_exprs should now be used instead?

Short term yes

Long term -- the UDF weaved into the plan should know it's return type. It's wasteful to recalculate it over again. #12604

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Deprecate ScalarUDFImpl::return_type andygrove/datafusion
2 participants
@findepi @andygrove