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.

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

Implement trait based API for defining WindowUDF #8711

Closed
alamb opened this issue Jan 1, 2024 · 1 comment · Fixed by #8719
Closed

Implement trait based API for defining WindowUDF #8711

alamb opened this issue Jan 1, 2024 · 1 comment · Fixed by #8719
Labels
enhancement New feature or request

Comments

@alamb
Copy link
Contributor

alamb commented Jan 1, 2024

Similarly to #8568

Is your feature request related to a problem or challenge?

The current way a user implements a WindowUDF is awkward and very hard to extend in backwards compatible ways:

They must wade through several Arc<dyn<...> typedefs to figure out how to provide the type signature and implementation

impl WindowUDF {

/// Create a new WindowUDF 
pub fn new(
    name: &str,
    signature: &Signature,
    return_type: &Arc<dyn Fn(&[DataType]) -> Result<Arc<DataType>, DataFusionError> + Send + Sync>,
    partition_evaluator_factory: &Arc<dyn Fn() -> Result<Box<dyn PartitionEvaluator>, DataFusionError> + Send + Sync>
) -> WindowUDF

Describe the solution you'd like

Follow the pattern in #8578

  1. Create a WindowUDFImpl trait, and WindowUDF::new_from_impl that creates an WindowUDF from the impl
  2. Add an example in datafusion-examples/examples/advanced_udwf.rs of using this API

I am not sure why this API is implemented like it is (other than it was consistent with ScalarUDF). As a user I would expect to be able to use a trait object like this

like

struct MyWindowFunction { 
..
}

impl WindowUDFImpl for MyWindowFunction {
  fn name(&self) -> &str, 
  fn return_type(&self) -> &DataType, 
...
}

Describe alternatives you've considered

No response

Additional context

@guojidan
Copy link
Contributor

guojidan commented Jan 2, 2024

hi @alamb I will try to implement this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants