Alembic test_variant_no_issue fails - not sure if it is relevant in my case

9 views
Skip to first unread message

gordon.d...@gmail.com

unread,
Jun 3, 2021, 6:18:04 PM6/3/21
to sqlalchemy-devel
An Alembic test named "test_variant_no_issue" in AutogenerateVariantCompareTest


is failing when run against CockroachDB. It looks like the test is trying to confirm that if you give it an empty upgrade

        uo = ops.UpgradeOps(ops=[])

and retrieve the net effect

        autogenerate._produce_net_changes(self.autogen_context, uo)

that it should be empty

        diffs = uo.as_diffs()
        eq_(diffs, [])

However what I'm getting is

AssertionError: [[('modify_type', None, 'sometable', 'id', {'existing_nullable': False, 'existing_server_default': DefaultClause(<sqlalchemy.sql.elements.TextClause object at 0x7fd7749c6c10>, for_update=False), 'existing_comment': None}, INTEGER(), Variant())]] != []
[[('modify_type',
   None,
   'sometable',
   'id',
   {'existing_comment': None,
    'existing_nullable': False,
    'existing_server_default': DefaultClause(<sqlalchemy.sql.elements.TextClause object at 0x7fd7749c6c10>, for_update=False)},
   INTEGER(),
   Variant())]] != []

What's not clear is the significance of the `.with_variant(Integer, "sqlite")` in the column definition. The test itself is not restricted to sqlite, and it does pass for postgresql, so I'm not quite sure if this is something that I should be concerned with.

Mike Bayer

unread,
Jun 3, 2021, 7:34:51 PM6/3/21
to sqlalche...@googlegroups.com
does the test pass if you remove the "variant" part of it?   it could be just the integer aspect of it not comparing correctly.  it looks like the base datatype is BigInteger so that's what's being compared.

This test is not one that is really relevant for 3rd party dialects though.    I dont think we should be trying to get the core test suite to pass for external dialects and we should be building out a new alembic/testing/suite kind of thing with new tests that are very targeted to just the specific things that 3rd party dialects need to worry about.
--
You received this message because you are subscribed to the Google Groups "sqlalchemy-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sqlalchemy-dev...@googlegroups.com.

gordon.d...@gmail.com

unread,
Jun 4, 2021, 7:48:36 AM6/4/21
to sqlalchemy-devel
> does the test pass if you remove the "variant" part of it?

Unfortunately, no. The `eq_()` failure just changes from …

[[('modify_type',
   None,
   'sometable',
   'id',
   {'existing_comment': None,
    'existing_nullable': False,
    'existing_server_default': DefaultClause(<sqlalchemy.sql.elements.TextClause object at 0x7fd7749c6c10>, for_update=False)},
   INTEGER(),
   Variant())]] != []

… to …

[[('modify_type',
   None,
   'sometable',
   'id',
   {'existing_comment': None,
    'existing_nullable': False,
    'existing_server_default': DefaultClause(<sqlalchemy.sql.elements.TextClause object at 0x7f00cc24f710>, for_update=False)},
   INTEGER(),
   BigInteger())]] != []

… so it may be something more fundamental.

> I dont think we should be trying to get the core test suite to pass for external dialects and we should be building out a new alembic/testing/suite kind of thing

Fair enough. (I asked about something like that on Gitter a week ago.) If you like I could open an Alembic issue on GitHub as a place to start.

gordon.d...@gmail.com

unread,
Jun 4, 2021, 8:13:47 AM6/4/21
to sqlalchemy-devel
However, the test does pass if I change

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

to

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

so it looks to me like the CockroachDB dialect is mapping BigInteger() to INTEGER() and that is what is getting reflected back to Alembic. Does that make sense?

Mike Bayer

unread,
Jun 4, 2021, 8:58:24 AM6/4/21
to sqlalche...@googlegroups.com
yup

gordon.d...@gmail.com

unread,
Jun 4, 2021, 9:02:34 AM6/4/21
to sqlalchemy-devel
The test also passes for cockroachdb, postgresql, and sqlite if I change

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

to

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

but would that alter the intent of the test?

Mike Bayer

unread,
Jun 4, 2021, 9:31:46 AM6/4/21
to sqlalche...@googlegroups.com
probably not
Reply all
Reply to author
Forward
0 new messages