[SPARK-26739][SQL] Standardized Join Types for DataFrames

[agrawalpooja]

What changes were proposed in this pull request?

Tries the address the concern mentioned in SPARK-26739
To summarise, currently, in the join functions on DataFrames, the join types are defined via a string parameter called joinType. In order for a developer to know which joins are possible, they must look up the API call for join. While this works fine, it can cause the developer to make a typo resulting in improper joins and/or unexpected errors that aren't evident at compile time. The objective of this improvement would be to allow developers to use a common definition for join types (by enum or constants) called JoinTypes. This would contain the possible joins and remove the possibility of a typo. It would also allow Spark to alter the names of the joins in the future without impacting end-users.

How was this patch tested?

Tested via Unit tests

[agrawalpooja]

I have a quick question here:
Should the existing methods with string joinType be deprecated?
In this PR, I have deprecated it for now. But can keep it to maintain backward compatibility, if needed.

[dilipbiswal]

Isn't this a breaking change ? Are we allowed to do that ? cc @HyukjinKwon
I also have a question for Python and R APIs. Is there an equivalent mechanism for those APIs ?

[HyukjinKwon]

Yes, of course it does break, and we shouldn't do this.

[HyukjinKwon]

Technically mima check should fail. If we should do this, we should add an overriden definition and probably deprecate existing one.

[agrawalpooja]

@HyukjinKwon do you mean to maintain both the definitions for now (with string as well as JoinType enum)? and then eventually deprecate the string one?

[HyukjinKwon]

When I first read the JIRA, I thought you wanted to provide some kind of enums without fixing APIs at all. catalyst is an internal API. it's not supposed to be a user face interface as well.

[agrawalpooja]

@HyukjinKwon yep, initially I created an enum and was using that. But, later someone pointed out in JIRA that we already have a JoinType class which we can reuse here.
Is it fine if I use a enum here?
(The motive is to have a standardised join types and detect the invalid join types at compile time itself)

[maropu]

JoinType is a internal class now, so I think we should discuss whether its ok to expose JoinType first as @HyukjinKwon suggested .... cc: @gatorsmile @cloud-fan

[HyukjinKwon]

Probably, I guess this idea came from save modes. I don't have strong opinion on this too ..

[AmplabJenkins]

Can one of the admins verify this patch?

[HyukjinKwon]

ping @agrawalpooja to update or close

[github-actions]

We're closing this PR because it hasn't been updated in a while.
This isn't a judgement on the merit of the PR in any way. It's just
a way of keeping the PR queue manageable.

If you'd like to revive this PR, please reopen it!