drizzle-team / drizzle-orm

Headless TypeScript ORM with a head. Runs on Node, Bun and Deno. Lives on the Edge and yes, it's a JavaScript ORM too 😅
https://orm.drizzle.team
Apache License 2.0
21.57k stars 490 forks source link

[FEATURE]: Fully type-safe query builder #2329

Closed septatrix closed 1 month ago

septatrix commented 1 month ago

Describe what you want

Drizzle already has decent typescript support but many pitfalls can currently not be checked due to the API. Using select first and then from means that the former cannot rely and type information from the latter. This makes autocompletion worse and makes it impossible to check bugs where the user selected a column from a table which is not included in a from/join statement.

Other query builders¹ use a syntax where one first writes the from (then join) and then the select. This has the advantage of knowing which tables will be available when writing the select statement. Typescripts powerful type system can utilize this (as some of the below query builders demonstrate) and fully check statement as well as provide better autocompletion hints to the editor.

Drizzle is now at the stage where it has catched up to the other players in the JS-SQL world and providing a properly type-safe query builder will likely help it rise further. This will especially help Drizzle in corporate environments where correctness is vital as can be seen be the adoption of Typescript. I am aware that this will be a breaking change but with proper communication to the community I expect that this change will be welcomed by the community due to the advantages it brings.

¹ E.g. kysely, TypeORM (to a certain degree), Knex, Lucid, even outside of the Typescript world with e.g. sqlkata, diesel.rs - admittedly I do not know the latter in depth but the db.from(table).select(cols) seems to be quite widespread in mature and popular query builders. I could have searched further and likely found more...

ricardo-valero commented 1 month ago

I fully agree with this proposal. Even though it deviates from the traditional SQL syntax, it is a more than appropriate change.

The design of SQL has a lexical, not a logical order.

Another example of this approach can be seen in Linq, which also emphasizes logical flow over traditional syntax order.

Additional Syntax Considerations: The proposed db.from(table).select(cols) looks fine, but if maintaining verb-first API consistency is a priority, consider these alternatives:

These alternatives would align well with existing API patterns:

dankochetov commented 1 month ago

We deliberately switched to our current API from the one proposed here, to reduce the cognitive load on the library users, because we decided it will provide better DX than huge type errors that are quite hard to understand for average TS developers. Another reason is, even if we were to switch to this API, it still won't support selecting with joins, as they would have to come before the selection as well for full type safety, deviating the API from SQL even more.

dankochetov commented 1 month ago

Also, I think this conversation should be moved to discussions rather than issues, so I'll close this one. Feel free to create a discussion on this topic, if there isn't one already.

septatrix commented 1 month ago

...we decided it will provide better DX than huge type errors that are quite hard to understand for average TS developers.

The current behavior in such a case would be huge runtime errors which provide no context whatsoever so this would still be an improvement over the status quo. Additionally, having this properly encoded in the type system would improve autocompletion such that developers wouldn't even be tempted to select columns which are not returned by the query. I fail to see how this would degrade DX.

Another reason is, even if we were to switch to this API, it still won't support selecting with joins, as they would have to come before the selection as well for full type safety, deviating the API from SQL even more.

Sure, I only gave a simplified example. Joins would of course also have to be moved further ahead. The linked reference about lexical vs logical order is the key point I'm getting at here. One pretty good example given by Ricardo is also Linq which uses the logical ordering. While I have not personally worked with it, I have only ever heard good about it and no complaints about it's unusual ordering compared to structured queries. So DX seems to be no concern here...

Also, I think this conversation should be moved to discussions rather than issues, so I'll close this one. Feel free to create a discussion on this topic, if there isn't one already.

As a contributor you should have the permissions to convert this to a discussion. Would you mind doing so such that this is correctly shown/tracked by GitHub?

septatrix commented 1 month ago

We deliberately switched to our current API from the one proposed here

I only ever know the current API. Can I find information about the previous version somewhere? Assuming there was a discussion about that change then it would be very insightful to read through the arguments at that time

dankochetov commented 1 month ago

Can I find information about the previous version somewhere?

There was no discussion on GitHub at the time, only internally between the team members, as it was on quite an early stage of the library.

Actually, I've realized that an issue can be converted to a discussion, so I'll do that instead of opening a new one.