You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "LiaCastaneda (via GitHub)" <gi...@apache.org> on 2026/05/28 15:12:15 UTC
[PR] refactor: wrap HigherOrderUDFImpl in a concrete HigherOrderUDF struct [datafusion]
LiaCastaneda opened a new pull request, #22593:
URL: https://github.com/apache/datafusion/pull/22593
## Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
- Closes #.
## Rationale for this change
`HigherOrderUDF` was the only UDF kind defined as a trait that callers used directly via `Arc<dyn HigherOrderUDF>`. The other UDFs: `ScalarUDF`, `AggregateUDF`, `WindowUDF` — are concrete structs that wrap their respective *Impl trait, which makes inherent methods like with_aliases ergonomic to call on the function object. With the trait-only setup, adding aliases to an existing higher-order function required an extension trait import or a free helper function.
This PR brings higher order functions in line with the other UDFs so the same with_aliases pattern works.
## What changes are included in this PR?
Rename the `HigherOrderUDF` trait to `HigherOrderUDFImpl`, matching `ScalarUDFImpl`/`AggregateUDFImpl`.
Add a concrete `HigherOrderUDF` struct wrapping `Arc<dyn HigherOrderUDFImpl>`, with the same shape as `ScalarUDF`: new_from_impl, new_from_shared_impl, inner, with_aliases, From<F: HigherOrderUDFImpl>, and delegate methods for every trait method.
`with_aliases` is backed by a private `AliasedHigherOrderUDFImpl` decorator (same pattern as `AliasedScalarUDFImpl`).
Update `Expr::HigherOrderFunction`, `FunctionRegistry`, the create_higher_order! singleton macro, and all consumer files ( across several crates) to use `Arc<HigherOrderUDF>` instead of `Arc<dyn HigherOrderUDF>`.
Existing impls (`ArrayFilter`, `ArrayTransform`, `ArrayAnyMatch`) now implement `HigherOrderUDFImpl`; their public constructors continue to return `Arc<HigherOrderUDF>` so external call sites need no changes.
Callers can now write:
`array_filter_higher_order_function().with_aliases(["filter"])
`
exactly like the existing scalar pattern:
`make_array_udf().as_ref().clone().with_aliases(["array_construct"])
`
## Are these changes tested?
Covered by existing tests
## Are there any user-facing changes?
Yes, any code referring to Arc<dyn HigherOrderUDF> needs to become Arc<HigherOrderUDF>, and any code that wrote impl HigherOrderUDF for MyType needs to write impl HigherOrderUDFImpl for MyType. Constructing a HigherOrderUDF from an impl is HigherOrderUDF::new_from_impl(my_impl) (or my_impl.into()).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] refactor: wrap HigherOrderUDFImpl in a concrete HigherOrderUDF struct [datafusion]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #22593:
URL: https://github.com/apache/datafusion/pull/22593#issuecomment-4566888016
<!-- semver-check-comment -->
Thank you for opening this pull request!
Reviewer note: [cargo-semver-checks](https://github.com/obi1kenobi/cargo-semver-checks) reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch).
<details>
<summary>Details</summary>
```
Cloning apache/main
Building datafusion v53.1.0 (current)
Built [ 97.714s] (current)
Parsing datafusion v53.1.0 (current)
Parsed [ 0.034s] (current)
Building datafusion v53.1.0 (baseline)
Built [ 94.754s] (baseline)
Parsing datafusion v53.1.0 (baseline)
Parsed [ 0.036s] (baseline)
Checking datafusion v53.1.0 -> v53.1.0 (no change; assume patch)
Checked [ 0.573s] 222 checks: 222 pass, 30 skip
Summary no semver update required
Finished [ 194.616s] datafusion
Building datafusion-datasource v53.1.0 (current)
Built [ 37.051s] (current)
Parsing datafusion-datasource v53.1.0 (current)
Parsed [ 0.032s] (current)
Building datafusion-datasource v53.1.0 (baseline)
Built [ 36.647s] (baseline)
Parsing datafusion-datasource v53.1.0 (baseline)
Parsed [ 0.032s] (baseline)
Checking datafusion-datasource v53.1.0 -> v53.1.0 (no change; assume patch)
Checked [ 0.241s] 222 checks: 222 pass, 30 skip
Summary no semver update required
Finished [ 75.232s] datafusion-datasource
Building datafusion-datasource-arrow v53.1.0 (current)
Built [ 36.230s] (current)
Parsing datafusion-datasource-arrow v53.1.0 (current)
Parsed [ 0.011s] (current)
Building datafusion-datasource-arrow v53.1.0 (baseline)
Built [ 37.046s] (baseline)
Parsing datafusion-datasource-arrow v53.1.0 (baseline)
Parsed [ 0.012s] (baseline)
Checking datafusion-datasource-arrow v53.1.0 -> v53.1.0 (no change; assume patch)
Checked [ 0.072s] 222 checks: 222 pass, 30 skip
Summary no semver update required
Finished [ 74.470s] datafusion-datasource-arrow
Building datafusion-execution v53.1.0 (current)
Built [ 29.987s] (current)
Parsing datafusion-execution v53.1.0 (current)
Parsed [ 0.025s] (current)
Building datafusion-execution v53.1.0 (baseline)
Built [ 30.129s] (baseline)
Parsing datafusion-execution v53.1.0 (baseline)
Parsed [ 0.027s] (baseline)
Checking datafusion-execution v53.1.0 -> v53.1.0 (no change; assume patch)
Checked [ 0.242s] 222 checks: 222 pass, 30 skip
Summary no semver update required
Finished [ 61.610s] datafusion-execution
Building datafusion-expr v53.1.0 (current)
Built [ 26.449s] (current)
Parsing datafusion-expr v53.1.0 (current)
Parsed [ 0.074s] (current)
Building datafusion-expr v53.1.0 (baseline)
Built [ 26.550s] (baseline)
Parsing datafusion-expr v53.1.0 (baseline)
Parsed [ 0.071s] (baseline)
Checking datafusion-expr v53.1.0 -> v53.1.0 (no change; assume patch)
Checked [ 1.192s] 222 checks: 221 pass, 1 fail, 0 warn, 30 skip
--- failure trait_changed_kind: pub trait changed kind ---
Description:
A public trait was replaced by a struct, enum, or union at the same import path.
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.47.0/src/lints/trait_changed_kind.ron
Failed in:
trait datafusion_expr::HigherOrderUDF became struct in /home/runner/work/datafusion/datafusion/datafusion/expr/src/higher_order_function.rs:829
Summary semver requires new major version: 1 major and 0 minor checks failed
Finished [ 55.343s] datafusion-expr
Building datafusion-ffi v53.1.0 (current)
Built [ 59.398s] (current)
Parsing datafusion-ffi v53.1.0 (current)
Parsed [ 0.059s] (current)
Building datafusion-ffi v53.1.0 (baseline)
Built [ 59.613s] (baseline)
Parsing datafusion-ffi v53.1.0 (baseline)
Parsed [ 0.061s] (baseline)
Checking datafusion-ffi v53.1.0 -> v53.1.0 (no change; assume patch)
Checked [ 0.248s] 222 checks: 222 pass, 30 skip
Summary no semver update required
Finished [ 120.852s] datafusion-ffi
Building datafusion-functions-nested v53.1.0 (current)
Built [ 35.396s] (current)
Parsing datafusion-functions-nested v53.1.0 (current)
Parsed [ 0.036s] (current)
Building datafusion-functions-nested v53.1.0 (baseline)
Built [ 35.547s] (baseline)
Parsing datafusion-functions-nested v53.1.0 (baseline)
Parsed [ 0.038s] (baseline)
Checking datafusion-functions-nested v53.1.0 -> v53.1.0 (no change; assume patch)
Checked [ 0.195s] 222 checks: 222 pass, 30 skip
Summary no semver update required
Finished [ 72.343s] datafusion-functions-nested
Building datafusion-optimizer v53.1.0 (current)
Built [ 27.688s] (current)
Parsing datafusion-optimizer v53.1.0 (current)
Parsed [ 0.029s] (current)
Building datafusion-optimizer v53.1.0 (baseline)
Built [ 26.747s] (baseline)
Parsing datafusion-optimizer v53.1.0 (baseline)
Parsed [ 0.030s] (baseline)
Checking datafusion-optimizer v53.1.0 -> v53.1.0 (no change; assume patch)
Checked [ 0.160s] 222 checks: 222 pass, 30 skip
Summary no semver update required
Finished [ 55.570s] datafusion-optimizer
Building datafusion-physical-expr v53.1.0 (current)
Built [ 28.487s] (current)
Parsing datafusion-physical-expr v53.1.0 (current)
Parsed [ 0.046s] (current)
Building datafusion-physical-expr v53.1.0 (baseline)
Built [ 28.987s] (baseline)
Parsing datafusion-physical-expr v53.1.0 (baseline)
Parsed [ 0.049s] (baseline)
Checking datafusion-physical-expr v53.1.0 -> v53.1.0 (no change; assume patch)
Checked [ 0.350s] 222 checks: 222 pass, 30 skip
Summary no semver update required
Finished [ 58.817s] datafusion-physical-expr
Building datafusion-proto v53.1.0 (current)
Built [ 57.223s] (current)
Parsing datafusion-proto v53.1.0 (current)
Parsed [ 0.022s] (current)
Building datafusion-proto v53.1.0 (baseline)
Built [ 57.602s] (baseline)
Parsing datafusion-proto v53.1.0 (baseline)
Parsed [ 0.018s] (baseline)
Checking datafusion-proto v53.1.0 -> v53.1.0 (no change; assume patch)
Checked [ 0.257s] 222 checks: 222 pass, 30 skip
Summary no semver update required
Finished [ 116.484s] datafusion-proto
Building datafusion-session v53.1.0 (current)
Built [ 36.625s] (current)
Parsing datafusion-session v53.1.0 (current)
Parsed [ 0.008s] (current)
Building datafusion-session v53.1.0 (baseline)
Built [ 36.712s] (baseline)
Parsing datafusion-session v53.1.0 (baseline)
Parsed [ 0.008s] (baseline)
Checking datafusion-session v53.1.0 -> v53.1.0 (no change; assume patch)
Checked [ 0.084s] 222 checks: 222 pass, 30 skip
Summary no semver update required
Finished [ 74.606s] datafusion-session
Building datafusion-spark v53.1.0 (current)
Built [ 56.472s] (current)
Parsing datafusion-spark v53.1.0 (current)
Parsed [ 0.058s] (current)
Building datafusion-spark v53.1.0 (baseline)
Built [ 56.542s] (baseline)
Parsing datafusion-spark v53.1.0 (baseline)
Parsed [ 0.060s] (baseline)
Checking datafusion-spark v53.1.0 -> v53.1.0 (no change; assume patch)
Checked [ 0.300s] 222 checks: 222 pass, 30 skip
Summary no semver update required
Finished [ 114.857s] datafusion-spark
Building datafusion-sql v53.1.0 (current)
Built [ 42.914s] (current)
Parsing datafusion-sql v53.1.0 (current)
Parsed [ 0.030s] (current)
Building datafusion-sql v53.1.0 (baseline)
Built [ 42.661s] (baseline)
Parsing datafusion-sql v53.1.0 (baseline)
Parsed [ 0.032s] (baseline)
Checking datafusion-sql v53.1.0 -> v53.1.0 (no change; assume patch)
Checked [ 0.252s] 222 checks: 222 pass, 30 skip
Summary no semver update required
Finished [ 87.067s] datafusion-sql
Building datafusion-substrait v53.1.0 (current)
Built [ 361.220s] (current)
Parsing datafusion-substrait v53.1.0 (current)
Parsed [ 0.018s] (current)
Building datafusion-substrait v53.1.0 (baseline)
Built [ 365.523s] (baseline)
Parsing datafusion-substrait v53.1.0 (baseline)
Parsed [ 0.018s] (baseline)
Checking datafusion-substrait v53.1.0 -> v53.1.0 (no change; assume patch)
Checked [ 0.226s] 222 checks: 222 pass, 30 skip
Summary no semver update required
Finished [ 729.158s] datafusion-substrait
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] refactor: wrap HigherOrderUDFImpl in a concrete HigherOrderUDF struct [datafusion]
Posted by "gstvg (via GitHub)" <gi...@apache.org>.
gstvg commented on code in PR #22593:
URL: https://github.com/apache/datafusion/pull/22593#discussion_r3319297358
##########
datafusion/expr/src/higher_order_function.rs:
##########
@@ -815,6 +819,304 @@ pub trait HigherOrderUDF: Debug + DynEq + DynHash + Send + Sync + Any {
}
}
+/// Logical representation of a Higher Order User Defined Function.
+///
+/// A higher order function takes one or more lambda arguments in addition to
+/// regular value arguments. This struct contains the information DataFusion
+/// needs to plan and invoke functions you supply such as name, type signature,
+/// return type, and actual implementation.
+///
+/// For advanced use cases, implement [`HigherOrderUDFImpl`] which provides the
+/// full API.
Review Comment:
Is this part necessary?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] refactor: wrap HigherOrderUDFImpl in a concrete HigherOrderUDF struct [datafusion]
Posted by "LiaCastaneda (via GitHub)" <gi...@apache.org>.
LiaCastaneda commented on PR #22593:
URL: https://github.com/apache/datafusion/pull/22593#issuecomment-4566480724
> yep makes sense, I think we can drop the API note here (it was carry over from claude) .
About this, I see all the UDF traits follow this struct + inner trait pattern, including the most recent async UDF -- but I think the pattern exists in the first place for backwards compatibility. See #8578 and #8713 (rewrites `ScalarUDF` to wrap `Arc<dyn ScalarUDFImpl>`). The wrapper was kept all along to avoid breaking users of the older API, not because the shape was preferred. I think it's still worth keeping for consistency here, but flagging in case simplifying this across UDFs is something we want to do in the future.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] refactor: wrap HigherOrderUDFImpl in a concrete HigherOrderUDF struct [datafusion]
Posted by "LiaCastaneda (via GitHub)" <gi...@apache.org>.
LiaCastaneda commented on code in PR #22593:
URL: https://github.com/apache/datafusion/pull/22593#discussion_r3319452615
##########
datafusion/expr/src/higher_order_function.rs:
##########
@@ -815,6 +819,304 @@ pub trait HigherOrderUDF: Debug + DynEq + DynHash + Send + Sync + Any {
}
}
+/// Logical representation of a Higher Order User Defined Function.
+///
+/// A higher order function takes one or more lambda arguments in addition to
+/// regular value arguments. This struct contains the information DataFusion
+/// needs to plan and invoke functions you supply such as name, type signature,
+/// return type, and actual implementation.
+///
+/// For advanced use cases, implement [`HigherOrderUDFImpl`] which provides the
+/// full API.
Review Comment:
yep makes sense, I think we can drop the API note here (it was carry over from claude) .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] refactor: wrap HigherOrderUDFImpl in a concrete HigherOrderUDF struct [datafusion]
Posted by "gabotechs (via GitHub)" <gi...@apache.org>.
gabotechs merged PR #22593:
URL: https://github.com/apache/datafusion/pull/22593
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] refactor: wrap HigherOrderUDFImpl in a concrete HigherOrderUDF struct [datafusion]
Posted by "LiaCastaneda (via GitHub)" <gi...@apache.org>.
LiaCastaneda commented on PR #22593:
URL: https://github.com/apache/datafusion/pull/22593#issuecomment-4565717487
@gstvg @rluvaton wdyt about this change? after bringing the higher order functions in our service I noticed we can't add custom aliases the way we can for the other kinds of UDFs. Since `HigherOrderUDF` was a trait there wasn't a natural place to put `with_aliases`. This mirrors the `ScalarUDF`/`ScalarUDFImpl` split so you can do `array_filter_higher_order_function().with_aliases(["filter"]) `like the scalar case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] refactor: wrap HigherOrderUDFImpl in a concrete HigherOrderUDF struct [datafusion]
Posted by "gstvg (via GitHub)" <gi...@apache.org>.
gstvg commented on code in PR #22593:
URL: https://github.com/apache/datafusion/pull/22593#discussion_r3319297358
##########
datafusion/expr/src/higher_order_function.rs:
##########
@@ -815,6 +819,304 @@ pub trait HigherOrderUDF: Debug + DynEq + DynHash + Send + Sync + Any {
}
}
+/// Logical representation of a Higher Order User Defined Function.
+///
+/// A higher order function takes one or more lambda arguments in addition to
+/// regular value arguments. This struct contains the information DataFusion
+/// needs to plan and invoke functions you supply such as name, type signature,
+/// return type, and actual implementation.
+///
+/// For advanced use cases, implement [`HigherOrderUDFImpl`] which provides the
+/// full API.
Review Comment:
Is this part necessary? I see that ScalarUDF has this comment too, but for scalar udf there's `create_udf` (the simple use case) which avoid implemeting the full trait (the advanced use case)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] refactor: wrap HigherOrderUDFImpl in a concrete HigherOrderUDF struct [datafusion]
Posted by "gabotechs (via GitHub)" <gi...@apache.org>.
gabotechs commented on PR #22593:
URL: https://github.com/apache/datafusion/pull/22593#issuecomment-4572220651
Nice, I'm merging this one now
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] refactor: wrap HigherOrderUDFImpl in a concrete HigherOrderUDF struct [datafusion]
Posted by "LiaCastaneda (via GitHub)" <gi...@apache.org>.
LiaCastaneda commented on PR #22593:
URL: https://github.com/apache/datafusion/pull/22593#issuecomment-4572211520
yeah I agree, I'll leave a comment in the release issue
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] refactor: wrap HigherOrderUDFImpl in a concrete HigherOrderUDF struct [datafusion]
Posted by "gstvg (via GitHub)" <gi...@apache.org>.
gstvg commented on PR #22593:
URL: https://github.com/apache/datafusion/pull/22593#issuecomment-4566199966
Nice, this will allow adding `HigherOrderUDF::call` in the future, similar to [ScalarUDF::call](https://docs.rs/datafusion/latest/datafusion/logical_expr/struct.ScalarUDF.html#method.call), and make some code less verbose, specially tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org