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

Add LogicalPlanBuilder::join_on #7766

Closed
alamb opened this issue Oct 7, 2023 · 4 comments · Fixed by #7805
Closed

Add LogicalPlanBuilder::join_on #7766

alamb opened this issue Oct 7, 2023 · 4 comments · Fixed by #7805
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@alamb
Copy link
Contributor

alamb commented Oct 7, 2023

Is your feature request related to a problem or challenge?

While working on #7612 with @nseekhao I think the LogicalPlanBuilder::join* interfaces are confusing:

Specifically, they all have a space to put parallel lists of join columns

join_keys: (Vec<impl Into<Column>, Global>, Vec<impl Into<Column>, Global>),
filter: Option<Expr>

However, the the ExtractEquijoinPredicate optimizer pass already splits up join predicates into equijoin predicates and "other" predicates

DataFrame::join_on uses this interface to nice effect:

pub fn join_on(
    self,
    right: DataFrame,
    join_type: JoinType,
    on_exprs: impl IntoIterator<Item = Expr>
) -> Result<DataFrame>

Describe the solution you'd like

I would like someone to

  1. add LogicalPlanBuilder::join_on, that does the same thing as DataFrame::join_on
  2. Add documentation that explains subsequent optimizer passes will split apart expressions, so there is no need to do so if you don't want to
  3. Add a doc example to LogicalPlanBuilder::join_on that shows how to use it

Something like

impl LogicalPlanBuilder {

pub fn join_on(
    self,
    right: DataFrame,
    join_type: JoinType,
    on_exprs: impl IntoIterator<Item = Expr>
) -> Result<DataFrame> {
...
}

Describe alternatives you've considered

No response

Additional context

No response

@haohuaijin
Copy link
Contributor

I want to do it.

@alamb
Copy link
Contributor Author

alamb commented Oct 8, 2023

Thank you @haohuaijin

@haohuaijin
Copy link
Contributor

Hi @alamb ,

impl LogicalPlanBuilder {

pub fn join_on(
    self,
    right: DataFrame,
    join_type: JoinType,
    on_exprs: impl IntoIterator<Item = Expr>
) -> Result<DataFrame> {
...
}

Shouldn't the return type of LogicalPlanBuilder::join_on be Result<Self>?

@alamb
Copy link
Contributor Author

alamb commented Oct 11, 2023

Shouldn't the return type of LogicalPlanBuilder::join_on be Result?

Yes you are right -- sorry @haohuaijin -- I think this was a typo

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

Successfully merging a pull request may close this issue.

2 participants