Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
septatrix opened this issue May 15, 2024 · 6 comments
Closed

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

septatrix opened this issue May 15, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@septatrix
Copy link

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...

@septatrix septatrix added the enhancement New feature or request label May 15, 2024
@ricardo-valero
Copy link

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:

  • db.read(table).select(cols)
  • db.read(table).pick(cols)
  • db.select(table).pick(cols)

These alternatives would align well with existing API patterns:

  • db.insert(table).values(values)
  • db.update(table).set(values).where(condition)
  • db.delete(table).where(condition)

@dankochetov
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Author

...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
Copy link
Author

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
Copy link
Contributor

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.

@drizzle-team drizzle-team locked and limited conversation to collaborators May 31, 2024
@dankochetov dankochetov converted this issue into discussion #2409 May 31, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants