Skip to content

minor: add higher-order function methods to SessionContext#21950

Open
gstvg wants to merge 2 commits intoapache:mainfrom
gstvg:session_ctx_higher_order_function_methods
Open

minor: add higher-order function methods to SessionContext#21950
gstvg wants to merge 2 commits intoapache:mainfrom
gstvg:session_ctx_higher_order_function_methods

Conversation

@gstvg
Copy link
Copy Markdown
Contributor

@gstvg gstvg commented Apr 30, 2026

Which issue does this PR close?

Follow-up of #21679 that forgot to add these methods to SessionContext

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions Bot added the core Core DataFusion crate label Apr 30, 2026
@gstvg
Copy link
Copy Markdown
Contributor Author

gstvg commented Apr 30, 2026

cc @rluvaton @comphead

Comment on lines 1446 to 1457
match function {
RegisterFunction::Scalar(f) => {
self.state.write().register_udf(f)?;
}
RegisterFunction::Aggregate(f) => {
self.state.write().register_udaf(f)?;
}
RegisterFunction::Window(f) => {
self.state.write().register_udwf(f)?;
}
RegisterFunction::Table(name, f) => self.register_udtf(&name, f),
};
Copy link
Copy Markdown
Member

@rluvaton rluvaton Apr 30, 2026

Choose a reason for hiding this comment

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

If you added it to drop_function, shouldn't we also add it here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

216542e
I think that drop_function works on all functions not only those registered via a FunctionFactory, and I wrongly thought that adding that variant would require a lot of changes and tests, so I left it for a future PR, but I checked it now and it's all user defined and nothing had to be changed/added (this match is the only usage of the Aggregate, Window and Table variants, for example), thanks

@github-actions
Copy link
Copy Markdown

Thank you for opening this pull request!

Reviewer note: 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
     Cloning origin/main
    Building datafusion v53.1.0 (current)
       Built [  86.524s] (current)
     Parsing datafusion v53.1.0 (current)
      Parsed [   0.036s] (current)
    Building datafusion v53.1.0 (baseline)
       Built [  88.067s] (baseline)
     Parsing datafusion v53.1.0 (baseline)
      Parsed [   0.035s] (baseline)
    Checking datafusion v53.1.0 -> v53.1.0 (no change; assume patch)
     Checked [   0.638s] 222 checks: 221 pass, 1 fail, 0 warn, 30 skip

--- failure enum_variant_added: enum variant added on exhaustive enum ---

Description:
A publicly-visible enum without #[non_exhaustive] has a new variant.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#enum-variant-new
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.47.0/src/lints/enum_variant_added.ron

Failed in:
  variant RegisterFunction:HigherOrder in /home/runner/work/datafusion/datafusion/datafusion/core/src/execution/context/mod.rs:2169

     Summary semver requires new major version: 1 major and 0 minor checks failed
    Finished [ 176.936s] datafusion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants