persistent-mysql: #641 introduced a bug for text/blob fields

[paul-rouse]

The change made in #641 created a problem: migration now attempts to alter the default value of text (and presumably blob) fields, but mysql does not allow these types to have defaults. Rather distressingly, the migration is actually trying to drop the default (even though there isn't one) but it is still a fatal error!

It's easy to reproduce this using an out-of-the-box yesod-mysql scaffolding:

stack --resolver lts-8.13 new foo yesod-mysql
cd foo
<create a clean mysql database, and edit config/settings.yml accordingly>
stack build
stack exec foo    # First run sets up the database without problem
stack exec foo    # Second run attempts a spurious migration and fails with an error from mysql

The error message is:

Migrating: ALTER TABLE `user` ALTER COLUMN `password` DROP DEFAULT
foo: ConnectionError {errFunction = "query", errNumber = 1101, errMessage = "BLOB/TEXT column 'password' can't have a default value"}

Reverting the two lines changed in #641 fixes this completely. The password field in the scaffolding is indeed nullable, so the new logic applies to it, but it is a text field, so the null in COLUMN_DEFAULT really does mean "unset", because it is not allowed to a default.

@qrilka I can see what you you wanted to achieve, but I am not sure how to fix it. What do you think?

[qrilka]

Imo NULL for text values is just a NULL.
The problem here seems to me to be in the detail what DEFAULT NULL could be set in 2 ways:

• explicitly as DEFAULT NULL
• implicitly for a field set by NULL as nullable or as for text/blob just as a default option.
  My only current idea how to deal with both correctly at the same time is to add some special case for default=NULL option

[paul-rouse]

It is worse than just text/blob fields. Even for other types, #641 just reversed the problem - now we get spurious migrations when the 'default null' is left implicit (ie a Maybe type with no explicit default) instead of when it is explicit!

A nullable field always has a default 'default null' in SQL, at least in mysql, so giving it explicitly makes no difference. Since it is an error to specify it for text/blob fields in mysql, it seems best never to pass "DEFAULT NULL" to the server, even if it is given in the Persistent table definition.

I am submitting PR #672 to do this - @qrilka can you check that it solves the problem you were originally trying to solve with #641?

[qrilka]

Thanks @paul-rouse
At first glance it looks to be a solution but I'd like to take deeper look a bit later.
Do you know how code for other DBs works with explicit NULLs or it's just another MySQL "feature"?
Also it makes sense to update wiki regarding default=NULL.

[paul-rouse]

AFAIK the implicit "default null" is standard SQL - certainly Postgresql does it too. I don't know whether other DBs retain the implicit/explicit distinction, but obviously they don't have to, as it does not affect the meaning. The only MySQL oddity is that text/blob columns cannot have an explicit default specified, even if it's the same as the implicit default null!

[paul-rouse]

Closed via #672