Skip to content

Conversation

schauder
Copy link
Contributor

@schauder schauder commented Feb 2, 2023

No description provided.

schauder added a commit that referenced this pull request Feb 2, 2023
@schauder schauder force-pushed the issue/full-outer-joins branch from cb4773b to b0a9ca0 Compare February 2, 2023 10:31
@schauder schauder force-pushed the issue/full-outer-joins branch from b0a9ca0 to 2dce16c Compare February 2, 2023 10:33
@schauder schauder requested a review from mp911de February 2, 2023 10:33
Copy link
Member

@mp911de mp911de left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know what you think, looks good otherwise.

* @see Join
* @see SQL#table(String)
*/
SelectOn fullOuterJoin(TableLike table);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether we should introduce a JoinType type to be more flexible and not always require a new method per join type. That type could describe INNER JOIN, LEFT/RIGHT joins, outer, cross, natural and many more of those.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, I will replace fullOuterJoin with join with an additional parameter.

@mp911de mp911de added the type: enhancement A general enhancement label Feb 23, 2023
@mp911de mp911de added this to the 3.1 M3 (2023.0.0) milestone Feb 23, 2023
schauder added a commit that referenced this pull request Mar 7, 2023
@schauder
Copy link
Contributor Author

schauder commented Mar 7, 2023

As discussed I added a generic join method and merged it into main.

@schauder schauder closed this Mar 7, 2023
@schauder schauder deleted the issue/full-outer-joins branch March 7, 2023 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants