see if we can enable CHECK constraint reflection in batch mode, perhaps w/ flag if it interferes w/ boolean/enum

[progamesigner]

Describe the bug
With Alembic 1.6.5 and SQLite backend, dropping check constraints will cause ValueError: No such constraint error.

Expected behavior
It should drop check constraint in batch mode if supported.

To Reproduce

from alembic import op
from sqlalchemy import Column, Integer


revision = 'a8f24ce8f819'
down_revision = None
branch_labels = None
depends_on = None


def upgrade() -> None:
    op.create_table('tname', Column('id', Integer, nullable=False))

    with op.batch_alter_table('tname') as operation:
        operation.create_check_constraint('ck', 'id <> NULL')


def downgrade() -> None:
    with op.batch_alter_table('tname') as operation:
        operation.drop_constraint('ck', 'check')

    op.drop_table('tname')

Error

Traceback (most recent call last):
  File "/workspaces/sample/.venv/lib/python3.9/site-packages/alembic/operations/batch.py", line 569, in drop_constraint
    const = self.named_constraints.pop(const.name)
KeyError: 'ck'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/local/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/workspaces/sample/.venv/lib/python3.9/site-packages/alembic/__main__.py", line 4, in <module>
    main(prog="alembic")
  File "/workspaces/sample/.venv/lib/python3.9/site-packages/alembic/config.py", line 559, in main
    CommandLine(prog=prog).main(argv=argv)
  File "/workspaces/sample/.venv/lib/python3.9/site-packages/alembic/config.py", line 553, in main
    self.run_cmd(cfg, options)
  File "/workspaces/sample/.venv/lib/python3.9/site-packages/alembic/config.py", line 530, in run_cmd
    fn(
  File "/workspaces/sample/.venv/lib/python3.9/site-packages/alembic/command.py", line 335, in downgrade
    script.run_env()
  File "/workspaces/sample/.venv/lib/python3.9/site-packages/alembic/script/base.py", line 490, in run_env
    util.load_python_file(self.dir, "env.py")
  File "/workspaces/sample/.venv/lib/python3.9/site-packages/alembic/util/pyfiles.py", line 97, in load_python_file
    module = load_module_py(module_id, path)
  File "/workspaces/sample/.venv/lib/python3.9/site-packages/alembic/util/compat.py", line 184, in load_module_py
    spec.loader.exec_module(module)
  File "<frozen importlib._bootstrap_external>", line 850, in exec_module
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File "database/env.py", line 56, in <module>
    asyncio.run(run_migrations_online())
  File "/usr/local/lib/python3.9/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/usr/local/lib/python3.9/asyncio/base_events.py", line 642, in run_until_complete
    return future.result()
  File "database/env.py", line 46, in run_migrations_online
    await connection.run_sync(run_migrations)
  File "/workspaces/sample/.venv/lib/python3.9/site-packages/sqlalchemy/ext/asyncio/engine.py", line 466, in run_sync
    return await greenlet_spawn(fn, conn, *arg, **kw)
  File "/workspaces/sample/.venv/lib/python3.9/site-packages/sqlalchemy/util/_concurrency_py3k.py", line 129, in greenlet_spawn
    result = context.switch(value)
  File "database/env.py", line 41, in run_migrations
    do_run_migrations(
  File "database/env.py", line 18, in do_run_migrations
    context.run_migrations()
  File "<string>", line 8, in run_migrations
  File "/workspaces/sample/.venv/lib/python3.9/site-packages/alembic/runtime/environment.py", line 813, in run_migrations
    self.get_context().run_migrations(**kw)
  File "/workspaces/sample/.venv/lib/python3.9/site-packages/alembic/runtime/migration.py", line 561, in run_migrations
    step.migration_fn(**kw)
  File "/workspaces/sample/database/migrations/a8f24ce8f819.py", line 20, in downgrade
    operation.drop_constraint('ck', 'check')
  File "/usr/local/lib/python3.9/contextlib.py", line 124, in __exit__
    next(self.gen)
  File "/workspaces/sample/.venv/lib/python3.9/site-packages/alembic/operations/base.py", line 336, in batch_alter_table
    impl.flush()
  File "/workspaces/sample/.venv/lib/python3.9/site-packages/alembic/operations/batch.py", line 117, in flush
    fn(*arg, **kw)
  File "/workspaces/sample/.venv/lib/python3.9/site-packages/alembic/operations/batch.py", line 577, in drop_constraint
    raise ValueError("No such constraint: '%s'" % const.name)
ValueError: No such constraint: 'ck'

Versions.

• OS: Debian 10 (buster)
• Python: 3.9.6
• Alembic: 1.6.5
• SQLAlchemy: 1.4.22
• Database: SQLite
• DBAPI: aiosqlite 0.17.0

Additional context
After my investigation, alembic/operations/batch.py#220 checks CheckConstraint and skips them. If I comment out L220 ~ L223, the migration works as expected.

...
elif self.reflected and isinstance(const, CheckConstraint):
    # TODO: we are skipping reflected CheckConstraint because
    # we have no way to determine _is_type_bound() for these.
    pass
...

However, I knew previous SQLAlchemy could not reflect CheckConstraint in SQLite backend. Is this still valid for newer SQLAlchemy? Any suggestions to make dropping CheckConstraint work with SQLite?

[zzzeek]

well, it looks like the last time i documented things on batch, SQLAlchemy didn't reflect check constraints at all: "SQLAlchemy currently doesn’t reflect CHECK constraints on any backend." from https://alembic.sqlalchemy.org/en/latest/batch.html#including-check-constraints , but then on SQLAlchemy it looks like we added them in 1.1.

However, the comment above is actually referring to a different issue which is that we can't distinguish these CHECK constraints from the ones SQLAlchemy creates automatically for Boolean and Enum datatypes. SQLAlchemy 1.4 turns off that default behavior as it has been nothing but trouble for alembic.

Basically it seems like we should look into enabling the above block, perhaps with some conditional. If devs want to work on this or people have PRs, that would help. In the meantime, you can send the CheckConstraint object to the directive manually as illustrated at https://alembic.sqlalchemy.org/en/latest/batch.html#including-check-constraints.

[zzzeek]

so your case passes if i take that code out, but the tests dont. seeing if i can disinguish these cases.

[zzzeek]

yeah it is exactly this: "we have no way to determine _is_type_bound() for these." still looking

[zzzeek]

OK i have something that will work, it will be for 1.7 and will have a bit of a backwards incompatible change for users renaming or dropping columns that refer to named check constraints; they'll have to refer to these. we will see what the impact is and if we need to add more of an upgrade path.

[sqla-tester]

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

support named CHECK constraints in batch https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/3017