You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Using Multi Statement Files with MySQL & LoadFile #88
FIXME: this does not really work with multi-statement files for mattn/go-sqlite3 or the go-mysql-driver/mysql drivers; pq seems to be an exception here. Detecting this by requiring something with DriverName() and then attempting to split the queries will be difficult to get right, and its current driver-specific behavior is deemed at least not complex in its incorrectness.
I'm using the go-mysql-driver and having this functionality work would really facilitate unit testing. Is this a sqlx issue or a driver related issue?
sqlx just tries to do db.Exec() with the contents of the file provided. If the driver can handle multiple statements in Exec() it works, otherwise it doesn't. It doesn't work with my driver, either.
Seems like that would work for me. You'd also have to filter out comments, but it seems like a decent regex could handle most situations without too much difficulty.
The problem was I originally developed sqlx against postgresql and I had some misconceptions about what was part of database/sql and what was part of the driver. As far as I know, lib/pq is the only driver that will actually execute what you send to it, and even then I'm not sure if there's a maximum size.
A simplistic attempt at parsing will be fraught with all the same dangers as #85 but given how useless this function is now for most non-psql users and the fact that sqlx now knows which driver we're dealing with thanks to the addition of the driverName on dbs, I think I'm rather more willing in this case to go part of the way towards supporting this a little better. For a lot of simplistic uses (like loading schema from a file for integration tests) this could still be useful.
Still, I'm worried that this will open sqlx users up to some really bad errors. Consider loading a database dump that has sql in it; getting that wrong could be catastrophic. The other problem is that the error conditions for when this fails leave your database in a potentially undefined, broken state, and that there's no way to test whether or not it would work on any particular file in isolation as a parsing error could lead to a transaction being committed.
As far as I know, here is what a hypothetical statement separator/parser has to deal with:
; can occur inside identifiers, eg ";hello" (and of course by default MySQL will use ` for the delimiter)
; can occur inside strings, which differ between drivers:
In the SQL standard, ' is the string delimiter
in MySQL and SQLite, " is usable either by default or via configuration
; can occur inside comments; as far as I know, all databases support the -- comment here style; are there others to be aware of?
I feel like the most common use case will be for unit testing (correct me if I'm wrong). For me, it's as simple as exporting my model from MySQL Workbench and having that in my repo as a mockup.
I think it's alright to put the responsibility of making sure that the file works on whoever is using it. I think it could be useful even if the caveats were as restrictive as not providing support for any of the scenarios you mentioned. If it did work out of the box, great, but no promises.
People can wrap their own files in a transaction, so it won't be committed if there is a parse error. As long as it errors out properly, let users do their own debugging.
It's possible to craft (and I think, if you're storing SQL in your database, probably possible to accidentally create) files which would execute SQL that are embedded in strings or in other novel datatypes if the parser is insufficiently sophisticated. The parse errors would come from the server after we mistakenly sent it something that was not an SQL statement, which means that If you used it like:
tx:=db.MustBegin()
// as an API, the `path string` is a mistake, but lets disregard that_, err:=LoadFile(tx, path)
iferr!=nil {
tx.Rollback()
} else {
tx.Commit()
}
But due to a parser error, you've executed commit; in there, the transaction would be committed.
These are "failure" scenarios, ordered by desirability:
It works in some safe way
It works for most scenarios, but doesn't work in a safe way for some obvious inputs
It doesn't work in some safe way
It works for most scenarios, but doesn't work in a safe way for non-obvious inputs
It doesn't work at all, with unsafe failures
It works for most scenarios, but doesn't work in an unsafe way for some obvious inputs
It works for most scenarios, but doesn't work in an unsafe way for non-obvious inputs
So, I prefer safe failure scenarios over unsafe failure scenarios for everything. Note that I consider not working at all in a safe way to be better than working most of the time but failing unexpectedly. I think that it results in better software for everyone if instead of me trying to handle things I can't people just deal with their own situations in a way that works for them.
What if you just exposed a processing function inside of LoadFile? It might expect a function signature something like: func processFile(fileContents []byte) (sqlStatements []string)
and then runs all of those statements inside of a transaction or something. You could provide some sample parsing functions in the docs, but if any code fails, that's the fault of the parser that they built for their own purposes.
The value is that you can ignore all the file handling and statement execution. It's basically plug and play for simple use cases by copying a sample parse function from the documentation, but you don't have to support tons of use cases.
Activity
freeeve commentedon Sep 24, 2014
sqlx just tries to do
db.Exec()with the contents of the file provided. If the driver can handle multiple statements inExec()it works, otherwise it doesn't. It doesn't work with my driver, either.freeeve commentedon Sep 24, 2014
Maybe we could try to split on semicolons that aren't in quotes and get it right most of the time? (that would work for mine)
derekperkins commentedon Sep 24, 2014
Seems like that would work for me. You'd also have to filter out comments, but it seems like a decent regex could handle most situations without too much difficulty.
freeeve commentedon Sep 24, 2014
I think you could send comments and it would be ok for most drivers.
freeeve commentedon Sep 24, 2014
See #89
jmoiron commentedon Sep 24, 2014
I'm waffling a bit on this.
The problem was I originally developed sqlx against postgresql and I had some misconceptions about what was part of
database/sqland what was part of the driver. As far as I know,lib/pqis the only driver that will actually execute what you send to it, and even then I'm not sure if there's a maximum size.A simplistic attempt at parsing will be fraught with all the same dangers as #85 but given how useless this function is now for most non-psql users and the fact that sqlx now knows which driver we're dealing with thanks to the addition of the driverName on dbs, I think I'm rather more willing in this case to go part of the way towards supporting this a little better. For a lot of simplistic uses (like loading schema from a file for integration tests) this could still be useful.
Still, I'm worried that this will open sqlx users up to some really bad errors. Consider loading a database dump that has sql in it; getting that wrong could be catastrophic. The other problem is that the error conditions for when this fails leave your database in a potentially undefined, broken state, and that there's no way to test whether or not it would work on any particular file in isolation as a parsing error could lead to a transaction being committed.
As far as I know, here is what a hypothetical statement separator/parser has to deal with:
;can occur inside identifiers, eg";hello"(and of course by default MySQL will use ` for the delimiter);can occur inside strings, which differ between drivers:'is the string delimiter"is usable either by default or via configuration;can occur inside comments; as far as I know, all databases support the-- comment herestyle; are there others to be aware of?This sounds simple, but I suspect that it's not.
Feedback is appreciated.
derekperkins commentedon Sep 24, 2014
I feel like the most common use case will be for unit testing (correct me if I'm wrong). For me, it's as simple as exporting my model from MySQL Workbench and having that in my repo as a mockup.
I think it's alright to put the responsibility of making sure that the file works on whoever is using it. I think it could be useful even if the caveats were as restrictive as not providing support for any of the scenarios you mentioned. If it did work out of the box, great, but no promises.
People can wrap their own files in a transaction, so it won't be committed if there is a parse error. As long as it errors out properly, let users do their own debugging.
jmoiron commentedon Sep 24, 2014
It's possible to craft (and I think, if you're storing SQL in your database, probably possible to accidentally create) files which would execute SQL that are embedded in strings or in other novel datatypes if the parser is insufficiently sophisticated. The parse errors would come from the server after we mistakenly sent it something that was not an SQL statement, which means that If you used it like:
But due to a parser error, you've executed
commit;in there, the transaction would be committed.These are "failure" scenarios, ordered by desirability:
So, I prefer safe failure scenarios over unsafe failure scenarios for everything. Note that I consider not working at all in a safe way to be better than working most of the time but failing unexpectedly. I think that it results in better software for everyone if instead of me trying to handle things I can't people just deal with their own situations in a way that works for them.
derekperkins commentedon Sep 24, 2014
What if you just exposed a processing function inside of LoadFile? It might expect a function signature something like:
func processFile(fileContents []byte) (sqlStatements []string)and then runs all of those statements inside of a transaction or something. You could provide some sample parsing functions in the docs, but if any code fails, that's the fault of the parser that they built for their own purposes.
jmoiron commentedon Sep 24, 2014
This would be a good compromise, but I wonder at that point how much value
LoadFileis really providing.derekperkins commentedon Sep 24, 2014
The value is that you can ignore all the file handling and statement execution. It's basically plug and play for simple use cases by copying a sample parse function from the documentation, but you don't have to support tons of use cases.
smthpickboy commentedon Nov 1, 2016
Use multiStatements = true connection parameter for go-mysql-driver