Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

altering primary_key column generates alter_column with nullable dropped #788

Closed
sdspikes opened this issue Jan 28, 2021 · 3 comments
Closed
Labels
autogenerate - rendering bug Something isn't working great mcve an issue with a great mcve mysql

Comments

@sdspikes
Copy link

Describe the bug
After a table is created with a primary key, if that key is updated to have a comment (or presumably any other change) the resulting migration drops nullable=False, generating a sql command like: ALTER TABLE key_and_nullable MODIFY id INTEGER(11) NULL AUTO_INCREMENT COMMENT 'auto-incrementing key';

This occurs even if the column had nullable set explicitly.

Expected behavior
The generated migration should have nullable=False for autoincrementing columns (or at least for primary keys).

The generated SQL should be: ALTER TABLE key_and_nullable MODIFY id INTEGER(11) NOT NULL AUTO_INCREMENT COMMENT 'auto-incrementing key';

To Reproduce
Repo with a minimal example. In particular the second migration and the generated SQL as linked above.

Versions.

  • OS: Debian GNU/Linux rodete (x86-64)
  • Python: 3.8.6
  • Alembic: 1.4.3
  • SQLAlchemy: 1.3.22
  • Database: mysql Ver 15.1 Distrib 10.5.8-MariaDB, for debian-linux-gnu (x86_64) using EditLine wrapper
  • DBAPI: mysql

Additional context
This migration (both through alembic and running as the raw SQL) seems to run without any problem in my local environment and does not actually change the key to be nullable, but in my work dev environment, running the SQL errors out, which seems reasonable, a primary key shouldn't be nullable. I'm not sure what's different about the dev environment without further investigation, but this really should be generated as not nullable anyway.

@sdspikes sdspikes added the requires triage New issue that requires categorization label Jan 28, 2021
@zzzeek zzzeek added autogenerate - rendering bug Something isn't working great mcve an issue with a great mcve mysql and removed requires triage New issue that requires categorization labels Jan 29, 2021
@zzzeek
Copy link
Member

zzzeek commented Jan 29, 2021

this is local to the autogenerate feature

@sqla-tester
Copy link
Collaborator

Mike Bayer has proposed a fix for this issue in the master branch:

Remove obsolete SQLAlchemy pk issue workaround https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/2521

@zzzeek
Copy link
Member

zzzeek commented Jan 29, 2021

add existing_nullable=False to those alter_column() ops for now, this fix will be in the next release. thanks for the report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autogenerate - rendering bug Something isn't working great mcve an issue with a great mcve mysql
Projects
None yet
Development

No branches or pull requests

3 participants