multipleStatements support
nmajor opened this issue · 2 comments
I'm trying to use database-js as my sole connection library for a project. But I'm running into an issue when running schema changes like migration files. It seems to be a problem with including multiple statements.
For example if I try to run an sql file like this:
ALTER TABLE `posts` MODIFY COLUMN `org_id` varchar(50) NOT NULL;
ALTER TABLE `posts` MODIFY COLUMN `updated_at` timestamp NOT NULL DEFAULT (now()) ON UPDATE CURRENT_TIMESTAMP;
It errors after the first statement.
Its possible I'm missing something obvious, but this seems to be an issue with having multiple statements. I can imagine this might be a security issue, but is there any plan to support multiple statements?
Yeah, correct that it's not currently supported, and once we do, it'll be expected to pass an array of statements, and I have no intention for the driver or the API to support implicitly splitting the input and being more intelligent about it. There are libraries that do this that we'd suggest pairing with, such as: https://www.npmjs.com/package/@verycrazydog/mysql-parser which supports a split() method.
You could use this now if you like and you'd just sequentially send each statement to us.
The improvements we're making to the API to support this would simply eliminate the extra round trip latency by going back and forth. But you'll still be required to split the statements yourself.
The reason for this is sorta simple but counter intuitive. It's not trivial to split statements without knowledge of SQL grammar. You can't blindly split on ; for example, and counter-intuitively, it's not common that clients use the actual mysql MULTI_STATEMENT mode. Even the mysql CLI tool kinda masks over this for you and splits the statements first, then sends them sequentially to the server.
I personally don't want to do any implicit behaviors around this, since as you kinda pointed out, it's potentially unexpected behavior which also means potentially a security issue. If you or your application wants to send explicitly more than 1 statement, you'll be able to send them, just they much each be well formed.
So maybe this helps at least get you going since there's nothing stopping you from using some SQL parser library to split the statements from your input data and send them to us sequentially. The API changes will just be performance improvement, and you'll still need to do the same thing.
I hope that helps.