feat: consolidate JoinType into a top-level enum

[vbarua]

No description provided.

[vbarua]

I noticed that we had 4 different definitions of the same enum, and thought it would be better to factor it out. I believe this is binary compatible, though I want to confirm that.

[yongchul]

@vbarua it is binary compatible. Any reason to hold back this change? This is a good cleanup IMO.

[jcamachor]

Thanks, @vbarua . Is there any remaining work or anything we can help with to get this PR merged? Any concerns about merging it? While this is a backwards-incompatible change, it'd be great to get this consolidation in place soon.

[yongchul]

Oh... I didn't realize that the enum values actually don't match! :( As @jcamachor said, I still think this is a necessary breaking change to keep things in consistent state.

[vbarua]

I'm focusing on substrait-io/substrait-go#128, which is actually somewhat related to my ability to check this.

didn't realize that the enum values actually don't match!

The other way to do this would to via slow migration, adding second field joint type field alongside the original ones (which we deprecate), have them co-exist for a number of versions, and then remove the old fields.

[jcamachor]

Thanks, @vbarua . I think the gradual migration approach you're suggesting is less disruptive and should help unblock this PR.

[benbellick]

This is a great cleanup and we should definitely try and get this in! 👍 on just doing a gentle migration by introducing a new field as long as we make a good effort to mark up everything old as deprecated.

[github-actions]

This PR has been automatically marked as stale because it has not had
recent activity. It will be closed in 7 days if no further activity occurs.

[github-actions]

This PR has been automatically closed due to inactivity.
If you believe this was closed in error, please reopen it.