-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Make AsyncScalarUDFImpl::invoke_async_with_args
consistent with ScalarUDFImpl::invoke_with_args
#16902
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.
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
Conversation
738c72d
to
522ea9a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there aren't any special backgrounds, this change lgtm
Thank you @geetanshjuneja 🙏 This is technically an API change so should have a entry in the upgrade guide I think -- I'll push a commit to this PR to do so |
@@ -24,6 +24,48 @@ | |||
**Note:** DataFusion `50.0.0` has not been released yet. The information provided in this section pertains to features and changes that have already been merged to the main branch and are awaiting release in this version. | |||
You can see the current [status of the `50.0.0 `release here](https://github.com/apache/datafusion/issues/16799) | |||
|
|||
### `AsyncScalarUDFImpl::invoke_async_with_args` returns `ColumnarValue` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this note here
I plan to merge this Monday unless anyone would like additional time to comment |
I merged up to resolve a conflict |
I merged up to fix a merge conflict with this PR and plan to merge this PR when the CI passes |
Thanks again @geetanshjuneja and @xudong963 |
…alarUDFImpl::invoke_with_args` (apache#16902) * Change AsyncScalarUDFImpl::invoke_async_with_args return type to ColumnarValue * fix docs * cargo fmt * cargo clippy * Add a note in the upgrade guide * Fix merge blunder --------- Co-authored-by: Andrew Lamb <[email protected]>
Which issue does this PR close?
AsyncScalarUDFImpl::invoke_async_with_args
consistent withScalarUDFImpl::invoke_with_args
#16896.Rationale for this change
What changes are included in this PR?
Changed
AsyncScalarUDFImpl::invoke_async_with_args
return type toColumnarValue
.Are these changes tested?
Are there any user-facing changes?