MySQL test success depends on using InnoDB table type

[knex]

MySQL test success depends on using InnoDB table type

[tgriesser]

@jblebrun - thanks for the heads up... I just pushed some changes to be able to add the engine on createTable... let me know if this looks good.

[jblebrun]

This does the trick, although it does break the abstraction a bit. Since only MySQL bindings support the engine function for now, it's not a huge deal, but it might be worth thinking through how to cleanly separate implementation-specific details further down the road.

[tgriesser]

Well, it's a no-op if used when it's not a MySQL connection, since it's only used in the compileCreateTable for the MySQL builder... Do you think that'd be alright?

[tgriesser]

@jblebrun - The other I was thinking is maybe if the engine type could be set in the config settings in Knex.Initialize? Then it wouldn't be per-table though...

[jblebrun]

I think doing it in the Initialize step would be better. Otherwise implementation details start to leak into what is supposed to be an implementation-independent means of specifying a schema.

[jblebrun]

For example, what if a database is later added that also supports engines, but different kinds? You couldn't simply change the initializer and expect things to work - the new database might break when it encounters the unexpected engine name that was intended for MySQL.

That said, it's pretty hard to write a schema that is truly implementation independent, if you expect to use all of the features of the database. However, keeping a separation between implementation-specific details and common schema detail makes it easier to migrate from one platform to another - it's easy to detect which parts need to be changed and which don't.

[jblebrun]

As an aside, your latest change breaks the tests, since the sql is expected to contain the "engine = " for the mysql statements.

[tgriesser]

The engine should be set in the tests in the latest...

I agree with the point that you might not be able to simply change the Initializer, though I feel like ultimately the engine should be set on a per-table basis... Also, InnoDB is the default engine with MySQL so there shouldn't normally be a need specify this anyway. I think I'm going to keep it per-table for now and re-visit if there's any conflict down the line.

[jblebrun]

I see, the problem was that the key name in the config file for charset encoding changed from "encoding" to "charset". Since I'm using a different config file, I didn't get this change.