Sqlx: Using Multi Statement Files with MySQL & LoadFile

[derekperkins]

_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?

[freeeve]

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.

[freeeve]

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]

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]

I think you could send comments and it would be ok for most drivers.

[freeeve]

See #89

[jmoiron]

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/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?

This sounds simple, but I suspect that it's not.

Feedback is appreciated.

[derekperkins]

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]

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)
if err != 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.

[derekperkins]

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]

This would be a good compromise, but I wonder at that point how much value LoadFile is really providing.

[derekperkins]

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]

Use multiStatements = true connection parameter for go-mysql-driver