Implement JOIN ... USING query

[dwickern]

Fixes #730

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
kysely	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 21, 2024 9:11am

[dwickern]

To implement JOIN ... USING, I added a new type argument to JoinBuilder, C extends string, representing the column names which are common to both tables being joined.

I see that JoinBuilder is also used for the MERGE query. Consequently this PR allows for a MERGE syntax which is not valid in Postgres and probably not valid on any DBMS:

// merge into person using pet using (id)
db.mergeInto('person').using('pet', (join) => join.using(['id']))

There are a couple ways to solve that (if maintainers here think it's worth solving):

1. Create a SelectJoinBuilder which extends JoinBuilder and adds the using function. Use SelectJoinBuilder for SELECT queries and use JoinBuilder for MERGE queries
2. MERGE queries could assign C = never as the type param to JoinBuilder

[dwickern]

Does it make sense to have an overload for a single column? I noticed that other APIs like select have this.

.innerJoin('pet', join => join.using('col'))
.innerJoin('pet', join => join.using(['col1', 'col2', 'col3']))

[igalklebanov]

Seems OK to me.
I'm not sure about enforcing non-empty with [C, ...C[]], I don't think we're THAT strict in other places.

[nikelborm]

Hi! This is very useful feature for me and I would love to see it in kysely, so thank you @dwickern!

Also are there any improvements needed to be done by the community to move this PR forward to be merged?

[igalklebanov]

That's a breaking change not worth doing, and also a surprise - how's on related to using?

This should make things quite simple, and no need for an extra node type:

Suggested change

	readonly on?: OnNode | JoinUsingNode
	readonly on?: OnNode
	readonly using?: ReadonlyArray<ColumnNode>

[igalklebanov]

To implement JOIN ... USING, I added a new type argument to JoinBuilder, C extends string, representing the column names which are common to both tables being joined.

Sounds like it's not crucial and could've been placed on the method/s. We're usually quite conservative when it comes to adding more generic parameters to existing builders. There has to be a good reason.. e.g. proving TypeScript compile performance is much better this way.

I see that JoinBuilder is also used for the MERGE query. Consequently this PR allows for a MERGE syntax which is not valid in Postgres and probably not valid on any DBMS:

// merge into person using pet using (id)
db.mergeInto('person').using('pet', (join) => join.using(['id']))

There are a couple ways to solve that (if maintainers here think it's worth solving):

1. Create a SelectJoinBuilder which extends JoinBuilder and adds the using function. Use SelectJoinBuilder for SELECT queries and use JoinBuilder for MERGE queries
2. MERGE queries could assign C = never as the type param to JoinBuilder

That's fine for now, not worth the extra builder/s IMHO. BYOSK (bring your own SQL knowledge).

[peterHakio]

@dwickern Hope you have time to finish the pull request and insure that it gets merged. Since it is a very nice feature and very nice work.