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

Rename functions-array to functions-nested #11602

Merged
merged 10 commits into from
Jul 24, 2024

Conversation

goldmedal
Copy link
Contributor

Which issue does this PR close?

Closes #11598 .

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 documentation Improvements or additions to documentation development-process Related to development process of DataFusion logical-expr Logical plan and expressions core Core DataFusion crate labels Jul 22, 2024
@goldmedal goldmedal marked this pull request as ready for review July 22, 2024 16:27
pub mod functions_array {
#[cfg(feature = "array_expressions")]
pub use datafusion_functions_array::*;
/// re-export of [`datafusion_functions_nested`] crate, if "nested_expressions" feature is enabled
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can leave a pub use functions_array here to help migration

Something like

    #[deprecated(since = "41.0.0", note = "use datafusion-functions-nested instead")]
pub mod functions_array {
    #[cfg(feature = "nested_expressions")]
    pub use datafusion_functions_nested::*;
}

We could do something similar with the feature flags, but maybe that is too complicated to be worth it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. Thanks. I'll try to do it for the crate and feature.

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.

Thanks @goldmedal -- I think this is an improvement, though I left a suggestion to help people upgrade

Comment on lines +44 to +45
# This feature is deprecated. Use the `nested_expressions` feature instead.
array_expressions = ["nested_expressions"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no better way to annotate a feature that is deprecated. Just leave a comment for it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is ok -- thank you

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool! I didn't know we have crate-deps graph

Copy link
Contributor

@jayzhan211 jayzhan211 left a comment

Choose a reason for hiding this comment

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

👍

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.

Thanks @goldmedal and @jayzhan211

pub use datafusion_functions_nested::*;
}

/// re-export of [`datafusion_functions_nested`] crate as [`functions_array`] for backward compatibility, if "nested_expressions" feature is enabled
Copy link
Contributor

Choose a reason for hiding this comment

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

💯

@alamb alamb merged commit 1e06b91 into apache:main Jul 24, 2024
26 checks passed
@goldmedal goldmedal deleted the chore/11598-rename-array-to-nested branch July 24, 2024 15:33
@goldmedal
Copy link
Contributor Author

Thanks @alamb and @jayzhan211 !

Michael-J-Ward added a commit to Michael-J-Ward/datafusion-python that referenced this pull request Aug 10, 2024
Michael-J-Ward added a commit to Michael-J-Ward/datafusion-python that referenced this pull request Aug 20, 2024
andygrove pushed a commit to apache/datafusion-python that referenced this pull request Aug 23, 2024
* update datafusion deps to point to githuc.com/apache/datafusion

Datafusion 41 is not yet released on crates.io.

* update TableProvider::scan

Ref: apache/datafusion#11516

* use SessionStateBuilder

The old constructor is deprecated.

Ref: apache/datafusion#11403

* update AggregateFunction

Upstream Changes:
- The field name was switched from `func_name` to func.
- AggregateFunctionDefinition was removed

Ref: apache/datafusion#11803

* update imports in catalog

Catlog API was extracted to a separate crate.

Ref: apache/datafusion#11516

* use appropriate path for approx_distinct

Ref: apache/datafusion#11644

* migrate AggregateExt to ExprFunctionExt

Also removed `sqlparser` dependency since it's re-exported upstream.

Ref: apache/datafusion#11550

* update regr_count tests for new return type

Ref: apache/datafusion#11731

* migrate from function-array to functions-nested

The package was renamed upstream.

Ref: apache/datafusion#11602

* cargo fmt

* lock datafusion deps to 41

* remove todo from cargo.toml

All the datafusion dependencies are re-exported, but I still need to figure out *why*.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core DataFusion crate development-process Related to development process of DataFusion documentation Improvements or additions to documentation logical-expr Logical plan and expressions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename functions-array to functions-nested to collect all nested-type functions
3 participants