Skip to content

Deprecate Expr::Wildcard #14959

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

Merged
merged 4 commits into from
Mar 3, 2025
Merged

Conversation

linhr
Copy link
Contributor

@linhr linhr commented Mar 1, 2025

Which issue does this PR close?

N/A

Rationale for this change

This is discussed as part of #14123.

What changes are included in this PR?

Expr::Wildcard is marked as deprecated.

Are these changes tested?

N/A

Are there any user-facing changes?

N/A

@github-actions github-actions bot added sql SQL Planner logical-expr Logical plan and expressions optimizer Optimizer rules substrait Changes to the substrait crate proto Related to proto crate functions Changes to functions implementation labels Mar 1, 2025
@linhr linhr mentioned this pull request Mar 1, 2025
26 tasks
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.

Looks good to me -- thank you @linhr and @shehabgamin

Comment on lines 106 to 107
// TODO: remove the next line after `Expr::Wildcard` is removed
#[allow(deprecated)]
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 if you use expect(deprecated) here and below rather than allow(deprecated) the compiler will warn / error once we remove Expr::Wildcard (this is a trick I learned from @itsjunetime )

Suggested change
// TODO: remove the next line after `Expr::Wildcard` is removed
#[allow(deprecated)]
#[expect(deprecated)]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @alamb! I've changed #[allow(deprecated)] to #[expect(deprecated)] in this PR.

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 kept the TODO comment in a few places to provide more context, when Expr::Wildcard is not directly referred to in the line following #[expect(deprecated)].

@alamb
Copy link
Contributor

alamb commented Mar 2, 2025

I plan to make a PR to backport this change to branch-46 so we can make a release candidate shortly

Copy link
Member

@xudong963 xudong963 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!

@xudong963 xudong963 merged commit 717b615 into apache:main Mar 3, 2025
24 checks passed
xudong963 pushed a commit to xudong963/arrow-datafusion that referenced this pull request Mar 3, 2025
* Deprecate `Expr::Wildcard`

* Update

Co-authored-by: Andrew Lamb <[email protected]>

* Update

---------

Co-authored-by: Andrew Lamb <[email protected]>
xudong963 added a commit that referenced this pull request Mar 3, 2025
* Deprecate `Expr::Wildcard`

* Update



* Update

---------

Co-authored-by: Heran Lin <[email protected]>
Co-authored-by: Andrew Lamb <[email protected]>
@linhr linhr deleted the wildcard-expr-deprecation branch March 15, 2025 02:56
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 optimizer Optimizer rules proto Related to proto crate sql SQL Planner substrait Changes to the substrait crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants