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

Variant base not taken into account when comparing types #433

Closed
sqlalchemy-bot opened this Issue Jun 2, 2017 · 9 comments

Comments

Projects
None yet
1 participant
@sqlalchemy-bot

sqlalchemy-bot commented Jun 2, 2017

Migrated issue, originally created by Dheeraj Gupta

I have a set of migration tests for my model which has a lot of autoincrement primary keys not all of which are Integer.

To support sqlite, my models have id lines like so

id_ = Column("id", BigInteger().with_variant(Integer, "sqlite"),
                 primary_key=True)

I use alembic for migrations and have written tests which compare the types of columns too. For versions of alembic <= 0.8.3, all tests pass. However, for versions newer than 0.8.4, I get the following differences from metadata (leading to test failure)

[('modify_type', None, 'ip_checkpoint', 'id', {'existing_server_default': False, 'existing_nullable': None}, BIGINT(display_width=20), Variant())]

I understand that Issue 341 had added additional type comparison and this change probably broke my tests.

Is it possible to fine-tune type checking for variants or should I just rework my tests to ignore such differences?

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jun 2, 2017

Michael Bayer (@zzzeek) wrote:

seeing that failure against SQLite or against other backends? current observation is the SQLite comparison will succeed because it compares only Integer. There seems to be a problem with the BigInteger() part of it, however, e.g. on other backends.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jun 2, 2017

Changes by Michael Bayer (@zzzeek):

  • removed labels: low priority
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jun 2, 2017

Changes by Michael Bayer (@zzzeek):

  • changed title from "Alembic upgrade from 0.8.3 to 0.8.4 or later break" to "Variant base not taken into account when comparing"
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jun 2, 2017

Changes by Michael Bayer (@zzzeek):

  • set milestone to "fasttrack"
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jun 2, 2017

Michael Bayer (@zzzeek) wrote:

even though we do dialect_impl() in the comparison, apparently for Variant if you are on a default dialect it gives you back itself (because it's a TypeDecorator). need to adjust for this.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jun 2, 2017

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jun 2, 2017

Michael Bayer (@zzzeek) wrote:

Adjust for Variant returning itself as impl

Fixed bug where autogen comparison of a :class:.Variant datatype
would not compare to the dialect level type for the "default"
implementation of the :class:.Variant, returning the type as changed
between database and table metadata.

Change-Id: Ie94779ece9f1c768375cdbdc4124c98f9c11bb86
Fixes: #433

54c5abb

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jun 2, 2017

Changes by Michael Bayer (@zzzeek):

  • changed status to closed
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jun 5, 2017

Dheeraj Gupta wrote:

@zzzeek Yes you are correct....tests succeed for SQLite backend but fail for MySQL backend.

Thanks for the quick-fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment