Skip to content

Conversation

@findepi
Copy link
Member

@findepi findepi commented Oct 29, 2024

invoke_batch covers all needs, so let's deprecate and eventually remove the redundant variants.

@github-actions github-actions bot added the logical-expr Logical plan and expressions label Oct 29, 2024
@findepi findepi marked this pull request as draft October 29, 2024 20:27
@findepi
Copy link
Member Author

findepi commented Oct 30, 2024

This requires lots of changes, so let's do this incrementally.

@github-actions github-actions bot added the functions Changes to functions implementation label Oct 30, 2024
@findepi findepi force-pushed the findepi/deprecate-invoke-and-invoke-no-args-in-favor-of-invoke-batch-dd2bd8 branch from 04c8611 to 13e57b2 Compare November 2, 2024 21:08
@findepi findepi marked this pull request as ready for review November 2, 2024 21:15
@findepi
Copy link
Member Author

findepi commented Nov 2, 2024

#13179 deprecated functions in ScalarUDF, this PR deprecates equivalent functions in ScalarUDFImpl trait.

@comphead please take a look

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @findepi -- this looks great.

I think we could file a ticket (good first issue) for migrating the rest of the built in functions (removing the #allow_deprecated)

/// Invoke the function with `args` and the number of rows,
/// returning the appropriate result.
///
/// The function will be invoked with the slice of [`ColumnarValue`]
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

@findepi
Copy link
Member Author

findepi commented Nov 3, 2024

I think we could file a ticket (good first issue) for migrating the rest of the built in functions (removing the #allow_deprecated)

#13238

please add a label

@findepi
Copy link
Member Author

findepi commented Nov 4, 2024

Pushed a commit fixing logical conflict with #13130 that was merged recently.

@findepi
Copy link
Member Author

findepi commented Nov 4, 2024

cc @comphead

@alamb
Copy link
Contributor

alamb commented Nov 4, 2024

Looks good -- thanks @findepi

@alamb alamb merged commit 274b222 into apache:main Nov 4, 2024
24 checks passed
@findepi findepi deleted the findepi/deprecate-invoke-and-invoke-no-args-in-favor-of-invoke-batch-dd2bd8 branch November 4, 2024 19:46
@alamb alamb mentioned this pull request Nov 5, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

functions Changes to functions implementation logical-expr Logical plan and expressions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants