Skip to content

Using Multi Statement Files with MySQL & LoadFile #88

Open

Description

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

Activity

  1. freeeve commented on Sep 24, 2014

    @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.

  2. freeeve commented on Sep 24, 2014

    @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)

  3. derekperkins commented on Sep 24, 2014

    @derekperkins
    Author

    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.

  4. freeeve commented on Sep 24, 2014

    @freeeve

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

  5. freeeve commented on Sep 24, 2014

    @freeeve

    See #89

  6. jmoiron commented on Sep 24, 2014

    @jmoiron
    Owner

    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.

  7. derekperkins commented on Sep 24, 2014

    @derekperkins
    Author

    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.

  8. jmoiron commented on Sep 24, 2014

    @jmoiron
    Owner

    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.

  9. derekperkins commented on Sep 24, 2014

    @derekperkins
    Author

    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.

  10. jmoiron commented on Sep 24, 2014

    @jmoiron
    Owner

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

  11. derekperkins commented on Sep 24, 2014

    @derekperkins
    Author

    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.

  12. smthpickboy commented on Nov 1, 2016

    @smthpickboy

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions