knex / knex

A query builder for PostgreSQL, MySQL, CockroachDB, SQL Server, SQLite3 and Oracle, designed to be flexible, portable, and fun to use.
https://knexjs.org/
MIT License
19.22k stars 2.12k forks source link

Immutable Joins #259

Open ericclemmons opened 10 years ago

ericclemmons commented 10 years ago

Howdy!

My project uses composition to build the query depending on the requested tables/columns.

Because of this, each "term" modifies the query solely based on what it needs to get that single column. As expected, several columns may depend on the same "join", but, when they both compose the same builder, there's an error in the SQL because of a non-unique alias.

I would expect that joining on the same table with the same clause would effectively be a no-op (unless the join type differed, in which case I'd throw an exception).

Thoughts?

tgriesser commented 10 years ago

Interesting, that makes sense. I can look into adding this functionality in the coming 0.6 release.

So just to make sure - all that you're saying would need to happen is to check at compilation time here that there aren't duplicates? I don't think it'd be necessary to check if the join type differed because if there's an error it should be thrown by the database library, right?

ericclemmons commented 10 years ago

The checks I'm doing currently look like this (thanks Javascript for not having private variables!):

if (!_.find(qb.joins, { table: 'program' })) {
  qb.join('program', 'school.id', '=', 'program.school.id');
}

I was going to replace this by using hooker to pre-empt .join() and only do it if the join hasn't already been performed.

Because of how the qb is being composed, I figured that joins should only be saved once for a particular table + clause. My question would be whether or not to throw an error because one file composed an inner-join while another composed an outer-join, or let one override the other.

tgriesser commented 10 years ago

Eh, actually thinking about it again, I don't know if we'd want to make too many assumptions around this. Don't want it to try to be too smart and not alert an error to a possible mistake they're making, probably better to do it as you're doing and deal with it in the application logic.

ericclemmons commented 10 years ago

Too smart = too clever = too confusing = magic :sparkles: :)

kamholz commented 9 years ago

I would really like to see some way to make this possible. There are some code paths where it's just inconvenient and messy to keep track of whether a table has been joined already. gesundheit has the method .ensureJoin, which makes it less magical, since the method name makes it explicit that the table is not to be joined twice.

Maybe the new introspection API will make this easier. Still, it would be nice to have a wrapper method that takes care of the introspection for you, so you don't have to write the same code to check whether a table has already been joined, or factor it out into a method (both of which would break chaining).

tgriesser commented 9 years ago

Agreed, this would be neat. I've been working on something at the schema definition layer that might help make this possible in one way or another.