-
-
Notifications
You must be signed in to change notification settings - Fork 393
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
Support sql`` JS template strings that create cached prepared statements #1157
Comments
I'm -1. @JoshuaWise has done a great job in keeping this library focused and simple, and yet there are still a ton of unresolved issues on this project. This would be a nice feature, but string templating, by design, doesn't have the same SQL-aware expressiveness, composability, and vendor-specific syntax translation as something like knex. In other words, not all users of this library will want to (or need to) switch to this new API. If this feature could not be added as an additional layer, but had to be incorporated in this library, I might change my mind, but IMHO, what @leafac has done with a layer on top of this library is the correct way to do it. Whether that code is maintained or documented sufficiently is a wholly different matter. |
That's fine. You're not required to use it. But many dev like me absolutely need it. A library that doesn't offer this is a no-go from the get-go for me. Just because you don't need it, or it could theoretically be done elsewhere, is not a good reason to reject the feature. For me, this is an essential, fundamental feature of any SQL API.
As you can see, that library is no longer maintained. If I base my app on it, I need to have confidence that the lib and feature is going to be there in the future. Also, there's the issue of discoverability. If I find better-sqlite3, I won't know that there is a better way than the "better sqlite3" (forgive the pun, but that's my point). |
Instead you'd put the burden of maintaining it on better-sqlite3?
Then maintain it? I don't mean to sound rude, but better-sqlite3 has essentially been in maintenance mode for quite some time. I don't remember the last real bug report and most activity is around support for newer Node.js/Electron versions. This is not about "dev like me absolutely need it" or "discoverability" or "this is a no-go" it's simply that there is no capacity here to implement and support this feature. And since it can easily be implemented in user-land (and better-sqlite3 was always meant as a very low level wrapper anyway) then I recommend doing that. We'd rather have low level things like N-API or being able to cooperatively kill a worker mid transaction then a feature that can be implemented outside of better-sqlite3. Because these are at the core of it. |
That's fair enough. Thanks for letting me know. |
The library that adds tagged template support to better-sqlite3 is here: https://github.com/radically-straightforward/radically-straightforward/tree/main/sqlite It used be called I hope you have a good time using it. |
+
straight-forward-
vulnerable to SQL injection security attacks.-
slow, because the DB engine has to do the query optimization on every execution.+
avoid SQL injection.+
By re-using the prepared statement, they allow the optimization to happen only once, making the runtime much faster. However,-
They separate the SQL statement from the data, and the variables from their place in the SQL string, making them very cumbersome to use in practice.+
make it comfortable for the dev to create SQL statements+
can avoid SQL injection security bugs. But:-
most SQL template string implementations create direct SQL strings, not prepared statements, so they are still slow. This is the reason why feature: Add support for sql-template-strings #178 was closed wontfix.However, it's possible to combine all the advantages, by letting the SQL template string function (JS template strings are just normal JS functions and can do anything they want) create the prepared statement first, cache it internally, and then use the cached prepared statement to do the variable replacement.
SQL template strings that create cached prepared statements gives
There are implementations of that idea. @leafac/sqlite/, now replaced by radically-straightforward, implements exactly that. Demo, direct link to the relevant time (13:10): https://youtu.be/3PCpXOPcVlM?t=793 However, I am uncertain about the stability of this project, given the short notice deprecation, complete lack of documentation, etc.
This is a request to add this feature directly to better-sqlite3 .
The text was updated successfully, but these errors were encountered: