[FEAT]: Sql joins with duplicate cols

[universalmind303]

adds support for union union all, and except set operations, as well as fixes an issue when performing joins with duplicate columns #3194

[codspeed-hq]

CodSpeed Performance Report

Merging #3241 will improve performances by ×2

_{Comparing universalmind303:sql-joins-with-duplicate-cols (ebf9644) with main (bdd25b6)}

Summary

⚡ 1 improvements
✅ 16 untouched benchmarks

Benchmarks breakdown

	Benchmark	main	universalmind303:sql-joins-with-duplicate-cols	Change
⚡	test_show[100 Small Files]	48.3 ms	24.1 ms	×2

[advancedxy]

@universalmind303 I think the Union/Except set operations support could be in a separate PR, and if wanted, I can help to contribute these support.

[universalmind303]

I think the Union/Except set operations support could be in a separate PR, and if wanted, I can help to contribute these support.

yeah that makes sense, will split them out!

[kevinzwang]

@universalmind303 I feel like join behavior diverge quite a bit between SQL and dataframe. Should we somehow split them up instead of sort of tacking the SQL parser on top of the dataframe one like we are currently doing?

Right now I create a project on the right table before the join to handle common column names for dataframe, we could move that out of the actual join op creation and have separate implementations of that project for each API.

[universalmind303]

@universalmind303 I feel like join behavior diverge quite a bit between SQL and dataframe. Should we somehow split them up instead of sort of tacking the SQL parser on top of the dataframe one like we are currently doing?

Right now I create a project on the right table before the join to handle common column names for dataframe, we could move that out of the actual join op creation and have separate implementations of that project for each API.

yeah I'm not a huge fan of all of the extra work needed to replicate sql style joins in our engine. I think this likely involves a larger refactor to how we handle joins and LogicalPlans though. The logicalPlan doesn't have a concept of a relation/table name which causes us to do all of this extra work.

[kevinzwang]

@universalmind303 I feel like join behavior diverge quite a bit between SQL and dataframe. Should we somehow split them up instead of sort of tacking the SQL parser on top of the dataframe one like we are currently doing?
Right now I create a project on the right table before the join to handle common column names for dataframe, we could move that out of the actual join op creation and have separate implementations of that project for each API.

yeah I'm not a huge fan of all of the extra work needed to replicate sql style joins in our engine. I think this likely involves a larger refactor to how we handle joins and LogicalPlans though. The logicalPlan doesn't have a concept of a relation/table name which causes us to do all of this extra work.

Would you like to start on that refactor in this PR or do you think it's worth merging this fix in for now? If the latter I can give this a review.

[advancedxy]

@universalmind303 I feel like join behavior diverge quite a bit between SQL and dataframe. Should we somehow split them up instead of sort of tacking the SQL parser on top of the dataframe one like we are currently doing?

I'm feeling the same way. The logical_plan::Join::try_new already passes join_suffix and join_prefix to distinguish non-common join keys, maybe we should leverage that to include common join keys as well? At least we can have consistent semantics between SQL and Dataframe API.

[kevinzwang]

LGTM. I think one thing we should do at some point is actually move the project/renaming logic out of Join::try_new, so that it can be tested independently and other places that create joins, such as LogicalPlan::with_new_children, do not need worry about that. But the code here is fine to merge in.