sqlalchemy
sqlalchemy copied to clipboard
sqlalchemy
Some tests for dialect ignores lack of support of CHECK CONSTRAINT
Describe the bug
Faced during the development of a new Dialect https://github.com/caretdev/sqlalchemy-iris/
for instance sqlalchemy/testing/suite/test_reflection.py - QuotedNameArgumentTest
Uses CheckConstraint unconditionally
To Reproduce
@property
def check_constraint_reflection(self):
"""target dialect supports reflection of check constraints"""
return exclusions.closed()
def visit_check_constraint(self, constraint, **kw):
raise exc.CompileError("Check CONSTRAINT is not supported")
### Error
Check CONSTRAINT is not supported
### Versions
- OS:
- Python:
- SQLAlchemy:
- Database:
- DBAPI `intersystems-iris`
https://pypi.org/project/sqlalchemy-iris/
### Additional context
_No response_
So, this is a dialect that has no concept of CHECK constraints at all. The test suite does not have the notion of dialects that raise an exception when a Table contains a CheckConstraint is generated, and I think as this is an extremely common construct that is also database-level, it might be best if your dialect simply ignored CheckConstraint() objects in tables rather than raising an exceptoin; that way one can have Table metadata that contains CheckConstraint objects for some backends but on your backend they'd be ignored.
Currently you can have CHECK constraint ignored as follows:
def visit_check_constraint(self, constraint):
return None
the above directive is only invoked in the context of a "CREATE TABLE" directive, or an "ADD CONSTRAINT" directive.
to block "ADD CONSTRAINT" explicitly, which would apply only to Alembic migrations or other tools that use the AddConstraint object directly, you can do this:
def visit_add_constraint(self, create, **kw):
if isinstance(create.element, schema.CheckConstraint):
raise exc.CompileError("Can't add CHECK constraint")
return super().visit_add_constraint(create, **kw)
that part should likely be more automatic on the SQLAlchemy side; otherwise it does "ADD CONSTRAINT None".
@zzzeek so the idea is not to place under a requirement flag all check constraint used in the test suite, correct?
we have a lot of constraints and other things that are not supported by some backends, like unique constraints, im sure a lot of the key/value store types of DBs dont have actual ForeignKeyConstraints, etc. Deciding that we would water down the DDL model used in the testing suite to not assume anything beyond tables and columns would be a big step to take, and if we are going to take that step (I think it's too late / unnecessary), we should take it for real rather than being inconsistent. Historically, for DDL things that aren't supported on backends that don't impact how SQLAlchemy talks to that object, we have the construct be a no-op. Precedence for this includes MySQL not having real CHECK constraints at all, even though the database server itself allows the syntax to pass through. Database backends that don't actually have primary key constraints, but we still put "primary_key=True" on columns. The Oracle dialect, more dramatically, ignores ForeignKey ON UPDATE directives, and doesn't render them, though in that case we emit a warning as that can change a program's behavior. SQLite and many other backends not supporting SQL comments, we just ignore them for those backends.