Skip to content

multipleStatements support #119

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

Closed
nmajor opened this issue Jun 20, 2023 · 2 comments
Closed

multipleStatements support #119

nmajor opened this issue Jun 20, 2023 · 2 comments

Comments

@nmajor
Copy link

nmajor commented Jun 20, 2023

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?

@nmajor
Copy link
Author

nmajor commented Jun 20, 2023

Found duplicate issue: #85

@mattrobenolt
Copy link
Member

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.

@dgraham dgraham closed this as completed Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants