Default to empty string for String based fields

[jacobsvante]

If both default and server_default are None then set them to empty strings as default. As we’re already forcing nullable=False on the field this feels pretty sensible.

This commit is backwards incompatible for MySQL users as they now need to set the option text_server_defaults=False if they use Text columns in their models. Added the ability to pass in options directly to make_lazy_configured to ease the pain somewhat. I've added some documentation on this. While I was at it I also added documentation about auto_now=True not working below MySQL 5.6.5.

[jacobsvante]

Travis gives error "Invalid default value for 'created_at'" for MySQL. Can't recreate this locally when I run DB=mysql py.test...

I'm on OS X Yosemite with Python 3.4 and MySQL 5.6.25 installed locally if that helps. PostgreSQL and SQLite works just fine also.

Edit: I fixed this by adding a pytest.mark.skipif that checks the mysqld version.

[jacobsvante]

I've fixed the Travis issue. Do you think this could be merged now @kvesteri?

[kvesteri]

Thanks for the PR. Could you split this to multiple commits (and even multiple PRs)? As for the string defaults I'm not sure if its a good idea: How can you override this behaviour on column level? This is a common use case for me as on some projects I've generated forms using WTForms-Alchemy and their it makes all the difference if a string column is not nullable but has a default or if it doesn't have default.

[jacobsvante]

I'll look into it splitting up the PR when I have some time over.

I pushed a new version that allows setting whether server_default for strings through the setting string_server_defaults. Calling make_lazy_configured(sa.orm.mapper, string_server_defaults=False, text_server_defaults=False) disables this behavior in the application for all string types. I see no reason why passing it to __lazy_options__ wouldn't work. But I could add a test for that when I have time.

[kvesteri]

Hmm I'm not sure about adding two different options for text and string. Since text in sqlalchemy world is just a subclass of string:

In [7]: issubclass(sa.Text, sa.String)
Out[7]: True

Also to make it possible for user to be able to override this behaviour on column level I would like this option to be more explicit. For example we could have an option string_server_default which the user can set to empty string thus enforcing all string columns to have empty string default.

The more I think about in the future we could introduce some kind of simple rule system for this Rule(condition, action) where both condition and action would be some kind of callables. But I have to think about that more. Just food for thought :)

[jacobsvante]

The reason I added a different option for text is to help out MySQL users as server defaults for TEXT is not supported (not even in 5.7).

Interesting idea about adding Rule. Perhaps we could start out simple for this case though by defaulting to None for {string/text}_server_default which would mean that no default is added.

DEFAULT_OPTIONS {
    ...
    'string_server_default': None,
    'text_server_default': None,
}

Because of my OCD I re-did the tests to use pytest fixtures. Removed the DB env var and added DSN instead to make it more explicit. I'll add as a new PR. When that has been merged I'll add another PR for passing in default settings to make_lazy_configured, then finally for the originally intended PR 😉

[kvesteri]

Good stuff. I would still leave out text_server_default option and just make a special rule for not applying string_server_default option on MySQL text fields (this should be documented of course).

[jacobsvante]

Okay so their only choice would be to completely turn off server defaults for strings on the entire table / mapper? Maybe it's not such a big deal. 😅

[kvesteri]

No, what I meant is that let's say I'm MySQL user and I have string_server_defaults=''. This rule would be then applied to all string fields except TEXT fields.

[jacobsvante]

Okay but how do we know that the user is a MySQL user when we're setting up the columns? We don't have access to any sort of engine bind when make_lazy_configured is called.

[kvesteri]

Could we have some kind of SQLAlchemy expression that compiles itself to None for MySQL text type?

[jacobsvante]

Interesting idea. I haven't worked too much with custom expressions to know if it's possible. I would guess that you're about a million times more suitable to find out 😉

[kvesteri]

It should be pretty straight-forward for you by following the guidelines found here: http://docs.sqlalchemy.org/en/latest/core/compiler.html#further-examples

[jacobsvante]

Hmm I'll see if I have some time to do that some day.

[jacobsvante]

I'm going to go ahead and close this PR. After much consideration I think not having this "feature" in SQLAlchemy-Defaults matches much better with how Flask-Admin and your WTForms-Alchemy handles defaulted-columns as not required fields.