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

Support BroadcastNestedLoopJoinExec #198

Open
singhpk234 opened this issue Mar 12, 2024 · 7 comments
Open

Support BroadcastNestedLoopJoinExec #198

singhpk234 opened this issue Mar 12, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@singhpk234
Copy link

What is the problem the feature request solves?

Datafusion supports
Cross and NestedLoop joins as well : https://docs.rs/datafusion-physical-plan/36.0.0/datafusion_physical_plan/joins/index.html

It will really nice if we can add support for it like Hash and SortMergeJoin.

Describe the potential solution

No response

Additional context

SHJ pr

@singhpk234 singhpk234 added the enhancement New feature or request label Mar 12, 2024
@singhpk234
Copy link
Author

@viirya can i pick this up ?

@viirya
Copy link
Member

viirya commented Mar 12, 2024

Sure. I've not begun working on this yet. Which one you will work, NestedLoop or Cross Join? In Spark, they are two difference join operators, I think we should have different tickets for them instead of one.

@viirya viirya changed the title Support NestedLoop and Cross Join Support BroadcastNestedLoopJoinExec Mar 12, 2024
@singhpk234
Copy link
Author

I can start with BNLJ :) !

@viirya viirya mentioned this issue Mar 12, 2024
10 tasks
@singhpk234
Copy link
Author

still working on it, will publish a pr by early next week, apologies for the delay.

@viirya
Copy link
Member

viirya commented Mar 28, 2024

No problem. Thank you for working on this.

@viirya
Copy link
Member

viirya commented Apr 5, 2024

Note that I found several bugs in current broadcast implementation when trying to enable broadcast by default in #213. Since BroadcastNestedLoopJoinExec uses broadcast, I suggest that you can work on the operator after #213 is merged.

@singhpk234
Copy link
Author

I suggest that you can work on the operator after #213 is merged

Ack, let me rebase with this branch meanwhile

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

No branches or pull requests

2 participants