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

mssql dialect not anticipating existing_type + type sent at the same time #812

Closed
ALMP-SallaH opened this issue Mar 4, 2021 · 4 comments
Labels
bug Something isn't working Microsoft SQL Server microsoft SQL Server, e.g. mssql op directives

Comments

@ALMP-SallaH
Copy link

Hello,

I have the following table in my MSSQL database:

Table('NewClients', metadata,
    Column('id', Integer, nullable=False, primary_key=True),
    Column('name', String(255), nullable=False, primary_key=True),
    Column('email', String(255), nullable=True),
    Column('phone', String(255), nullable=True),
    Column('address', String(255), nullable=True),
    Column('zip_code', String(255), nullable=True),
    Column('country', String(255), nullable=True),
    Column('date_of_birth', Date, nullable=True),
    schema='store')

I wish to alter the data type of column id to VARCHAR(15) NOT NULL. I expected the following commands to produce the same result:

op.alter_column(table_name='NewClients', column_name='id', nullable=False, type_=sa.String(15), schema='store')
op.alter_column(table_name='NewClients', column_name='id', nullable=False, type_=sa.String(15), existing_type=sa.Integer, schema='store')

The first one works perfectly

op.drop_constraint('pk_NewClients', table_name='NewClients', schema='store')
op.alter_column(table_name='NewClients', column_name='id', nullable=False, type_=sa.String(15), schema='store')
op.create_primary_key('pk_NewClients', table_name='NewClients', columns=['id', 'name'], schema='store')

However, if I add the existing_type parameter to alter_column operation, things become confusing...

op.drop_constraint('pk_NewClients', table_name='NewClients', schema='store')
op.alter_column(table_name='NewClients', column_name='id', nullable=False, type_=sa.String(15), existing_type=sa.Integer, schema='store')
op.create_primary_key('pk_NewClients', table_name='NewClients', columns=['id', 'name'], schema='store')

This produces

INFO [sqlalchemy.engine.base.Engine] ()
2021-03-04 08:55:50,868 INFO sqlalchemy.engine.base.Engine ALTER TABLE store.[NewClients] ALTER COLUMN id INTEGER NOT NULL
INFO [sqlalchemy.engine.base.Engine] ALTER TABLE store.[NewClients] ALTER COLUMN id INTEGER NOT NULL
2021-03-04 08:55:50,869 INFO sqlalchemy.engine.base.Engine ()
INFO [sqlalchemy.engine.base.Engine] ()
2021-03-04 08:55:50,873 INFO sqlalchemy.engine.base.Engine ALTER TABLE store.[NewClients] ALTER COLUMN id VARCHAR(15)
INFO [sqlalchemy.engine.base.Engine] ALTER TABLE store.[NewClients] ALTER COLUMN id VARCHAR(15)
2021-03-04 08:55:50,873 INFO sqlalchemy.engine.base.Engine ()

and as a result the column id has now type VARCHAR(15) NULL and the creation of primary key fails (because primary key columns cannot be NULL).

Is this the intended behavior? If it is, would it be possible to add some sort of notice or warning regarding the use of parameter existing_type in alter_column with MSSQL to documentation?

Versions.

  • OS: Win 10
  • Python: 3.8.3
  • Alembic: 1.5.5
  • SQLAlchemy: 1.3.23
  • Database: SQL Server 14.0.3281.6 (2017, Linux)
  • DBAPI: pyodbc
@ALMP-SallaH ALMP-SallaH added the requires triage New issue that requires categorization label Mar 4, 2021
@sqla-tester
Copy link
Collaborator

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

Adjust mssql accommodate existing_type and type https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/2615

@zzzeek zzzeek changed the title Confusing behavior with existing_type parameter in alter_column operation with MSSQL mssql dialect not anticipating existing_type + type sent at the same time Mar 4, 2021
@zzzeek zzzeek added bug Something isn't working Microsoft SQL Server microsoft SQL Server, e.g. mssql op directives and removed requires triage New issue that requires categorization labels Mar 4, 2021
@zzzeek
Copy link
Member

zzzeek commented Mar 4, 2021

that's just a simple bug, MSSQL can alter the type + nullability in one statement, which is distinct from the default logic that can only set them in separate statements, and the logic it uses to detect this simply didn't anticipate existing_type being present. thanks for the bug report.

@zzzeek
Copy link
Member

zzzeek commented Mar 4, 2021

I can release now unless you want to first try out github master to see if it resolves your issue.

@ALMP-SallaH
Copy link
Author

Tested and works great. Thank you very much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Microsoft SQL Server microsoft SQL Server, e.g. mssql op directives
Projects
None yet
Development

No branches or pull requests

3 participants