Add naming convention

[kigawas]

Rationale:

This is basically a poka-yoke design, for

1. Disabling constraints without names
2. Generating constraints with names in alembic migrations
   • Alembic will not generate names for constraints defined in columns, like unique=True
   • Alembic will complain when it's downgrading a constraint without name

FYI:
https://alembic.sqlalchemy.org/en/latest/naming.html

[fantix]

Thanks for the PR! I think I get your case and I'm with you on making this default, with only one concern: would it break existing applications? Say I remove a unique=True and auto-generate a new Alembic revision with this PR, would the new revision work properly?

[kigawas]

With the old version, actually you cannot downgrade constraints like removing unique, if you didn't set names in alembic migration files.

The reason is simple: removing a unique constraint needs to know its name.

This pr doesn't actually break anything, it just makes the old pitfall obvious.

[fantix]

This pr doesn't actually break anything

Well, this is partially true - I got some time today and did some testing, looks like this PR will lead to an extra auto-generated Alembic revision to rename constraints for the new naming convention even the application code is unchanged at all, unless e.g. the existing unique constraint in the database (mapping to unique=True in the model) matches exactly the name defined in the convention, which is usually not the case. I see people leaving the names of the unique constraints etc. in generated revisions None (or give them random names, and some people just don't test downgrade which is not working), and the actual names in the database are different than the suggested naming convention. Also, there're people not using Alembic or create_all()/drop_all() at all.

After thinking about alternative approaches (warn about new naming conventions in GINO 1.1 and actually add them in GINO 1.2), at the moment I'm leaning towards not adding default naming conventions in GINO, for the same reason why SQLAlchemy provides only default naming convention for indexes in the code, and other conventions in the documentation. Please see also sqlalchemy/sqlalchemy#4784. The naming may not be trivial and should be decided by the user.

Though, I think we could definitely add naming_convention recommendation in GINO docs. That would be quite helpful.

[kigawas]

Let the DB decide the constraint names is a bad idea in most cases - as far as I know. I'd rather see some consistent errors than not aware of hidden magic played by some database's some specific version.

Say you didn't test downgrading and have brought it onto production, it'd be a total disaster if you'd like to downgrade the migration as long as unique=True included in some revision.

The default naming convention is not a trivial API change. I agree to add it in the following version, but a warning for not defining naming convention should be released ASAP.

By the way, alembic migration files should be reviewed and might be deleted, so it's not a big problem for me to generate index name changes - you can delete them anyway.