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

Remove unused LogicalPlan::CrossJoin as it is unused #13076

Merged
merged 2 commits into from
Oct 24, 2024

Conversation

buraksenn
Copy link
Contributor

Which issue does this PR close?

Closes #13057

Rationale for this change

LogicalPlan::CrossJoin is not generated anymore so deleted all of its occurences

What changes are included in this PR?

  • delete occurences of crossjoin
  • simplify some match statements

Are these changes tested?

Existing tests

Are there any user-facing changes?

no

@github-actions github-actions bot added sql SQL Planner logical-expr Logical plan and expressions optimizer Optimizer rules core Core DataFusion crate substrait proto Related to proto crate labels Oct 23, 2024
@alamb alamb added the api change Changes the API exposed to users of the crate label Oct 24, 2024
@alamb alamb changed the title delete LogicalPlan::CrossJoin references since it is not generated in logicalplan anymore Remove unused LogicalPlan::CrossJoin as it is unused Oct 24, 2024
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.

THank you @buraksenn -- this looks like a very nice cleanup to me

cc @Dandandan who has been working in this area recently as well

@@ -220,10 +220,6 @@ pub enum LogicalPlan {
/// Join two logical plans on one or more join columns.
/// This is used to implement SQL `JOIN`
Join(Join),
/// Apply Cross Join to two logical plans.
/// This is used to implement SQL `CROSS JOIN`
/// Deprecated: use [LogicalPlan::Join] instead with empty `on` / no filter
Copy link
Contributor

Choose a reason for hiding this comment

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

nice for cleaning up a deprecated feature

Copy link
Contributor

@berkaysynnada berkaysynnada left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@Dandandan Dandandan left a comment

Choose a reason for hiding this comment

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

Let's remove it 🚀

@Dandandan Dandandan merged commit f2da32b into apache:main Oct 24, 2024
24 checks passed
Michael-J-Ward added a commit to Michael-J-Ward/datafusion-python that referenced this pull request Oct 28, 2024
Michael-J-Ward added a commit to apache/datafusion-python that referenced this pull request Nov 10, 2024
* patch datafusion deps

* migrate from deprecated RuntimeEnv::new to RuntimeEnv::try_new

Ref: apache/datafusion#12566

* remove Arc from create_udf call

Ref: apache/datafusion#12489

* doc typo

* migrage new UnnestOptions API

Ref: https://github.com/apache/datafusion/pull/12836/files

* update API for logical expr Limit

Ref: apache/datafusion#12836

* remove logical expr CrossJoin

It was removed upstream.

Ref: apache/datafusion#13076

* update PyWindowUDF

Ref: apache/datafusion#12803

* migrate window functions lead and lag to udwf

Ref: apache/datafusion#12802

* migrate window functions rank, dense_rank, and percent_rank to udwf

Ref: apache/datafusion#12648

* convert window function cume_dist to udwf

Ref: apache/datafusion#12695

* convert window function ntile to udwf

Ref: apache/datafusion#12694

* clean up functions_window invocation

* Only one column was being passed to udwf

* Update to DF 43.0.0

* Update tests to look for string_view type

* String view is now the default type for strings

* Making a variety of adjustments in wrappers and unit tests to account for the switch from string to string_view as default

* Resolve errors in doc building

---------

Co-authored-by: Tim Saucer <timsaucer@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api change Changes the API exposed to users of the crate core Core DataFusion crate logical-expr Logical plan and expressions optimizer Optimizer rules proto Related to proto crate sql SQL Planner substrait
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecate LogicalPlan::CrossJoin
4 participants