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

CHECK Constraint is not found when it is a per-column constraint #711

Closed
BarisSari opened this issue Jul 14, 2020 · 3 comments
Closed

CHECK Constraint is not found when it is a per-column constraint #711

BarisSari opened this issue Jul 14, 2020 · 3 comments

Comments

@BarisSari
Copy link

BarisSari commented Jul 14, 2020

Describe the bug
I have a migration file which basically creates a Check Constraint in upgrade() and drops it in downgrade(). When I define the constraint in the column, the downgrade throws ValueError: No such constraint: 'ck_Tasks_name'. When I define it in __table_args__, the downgrade works fine. upgrade() works in both definitions.

Expected behavior
I think Alembic should detect the per-column Check Constraints.

To Reproduce

# Model that throws an error during the downgrade
sqlite_naming_convention = {
    "ix": "ix_%(column_0_label)s",
    "uq": "uq_%(table_name)s_%(column_0_name)s",
    "ck": "ck_%(table_name)s_%(column_0_name)s",
    "fk": "fk_%(table_name)s_%(column_0_name)s_%(referred_table_name)s",
    "pk": "pk_%(table_name)s",
}

Metadata = MetaData(naming_convention=sqlite_naming_convention)
BaseSpatiaLite = declarative_base(metadata=Metadata)


class Task(BaseSpatiaLite):
    __tablename__ = "Tasks"

    task_id = Column(Integer, primary_key=True)
    # per-column constraint is below
    name = Column(String(150), CheckConstraint("name <> ''", name="ck_Tasks_name"), nullable=False)
# Model that works in both upgrade and downgrade
sqlite_naming_convention = {
    "ix": "ix_%(column_0_label)s",
    "uq": "uq_%(table_name)s_%(column_0_name)s",
    "ck": "ck_%(table_name)s_%(column_0_name)s",
    "fk": "fk_%(table_name)s_%(column_0_name)s_%(referred_table_name)s",
    "pk": "pk_%(table_name)s",
}

Metadata = MetaData(naming_convention=sqlite_naming_convention)
BaseSpatiaLite = declarative_base(metadata=Metadata)


class Task(BaseSpatiaLite):
    __tablename__ = "Tasks"

    task_id = Column(Integer, primary_key=True)
    # per-column constraint moved to __table_args
    name = Column(String(150), nullable=False)

    __table_args__ = (CheckConstraint("name <> ''", name="ck_Tasks_name"), )
# migration file
"""Add CheckConstraints for non-nullable string cols

Revision ID: b7fc1ab24c92
Revises: d5d740c76aa3
Create Date: 2020-07-10 13:24:56.007611

"""
from models import Task

# revision identifiers, used by Alembic.
revision = "b7fc1ab24c92"
down_revision = "d5d740c76aa3"
branch_labels = None
depends_on = None


def upgrade():
    with op.batch_alter_table("Tasks", schema=None, copy_from=Task.__table__) as batch_op:
        batch_op.create_check_constraint("ck_Tasks_name", condition="name <> ''")


def downgrade():
    with op.batch_alter_table("Tasks", schema=None, copy_from=Task.__table__) as batch_op:
        batch_op.drop_constraint("ck_Tasks_name", type_="check")

Error

INFO  [alembic.runtime.migration] Running downgrade b7fc1ab24c92 -> d5d740c76aa3, Add CheckConstraints for non-nullable string cols
-- Running downgrade b7fc1ab24c92 -> d5d740c76aa3

Traceback (most recent call last):
  File .../lib/python3.8/site-packages/alembic/operations/batch.py", line 508, in drop_constraint
    const = self.named_constraints.pop(const.name)
KeyError: 'ck_Tasks_name'

...
File ".../migrations/sqlite_versions/2020-07-10_b7fc1ab24c92_add_checkconstraints_for_non_nullable_.py", line 451, in downgrade
    batch_op.drop_constraint("ck_Tasks_name", type_="check")
  File "/Users/baris/.pyenv/versions/3.8.1/lib/python3.8/contextlib.py", line 120, in __exit__
    next(self.gen)
  File ".../lib/python3.8/site-packages/alembic/operations/base.py", line 354, in batch_alter_table
    impl.flush()
  File ".../lib/python3.8/site-packages/alembic/operations/batch.py", line 114, in flush
    fn(*arg, **kw)
  File ".../lib/python3.8/site-packages/alembic/operations/batch.py", line 516, in drop_constraint
    raise ValueError("No such constraint: '%s'" % const.name)
ValueError: No such constraint: 'ck_Tasks_name'

Versions.

  • OS: Mac OS 10.15
  • Python: 3.8.1
  • Alembic: 1.4.2
  • SQLAlchemy: 1.3.10
  • Database: SQLite
  • DBAPI:

Additional context
If this is reported before or is explained in the Alembic's documentation, I'm sorry that I couldn't find any information.

Have a nice day!

@BarisSari BarisSari added the requires triage label Jul 14, 2020
@BarisSari BarisSari changed the title CHECK Constraint's name not found when it is a per-column constraint CHECK Constraint is not found when it is a per-column constraint Jul 14, 2020
@zzzeek
Copy link
Member

zzzeek commented Jul 14, 2020

this still requires continued triage as it may be a dupe of an existing bug.

@sqla-tester
Copy link
Collaborator

sqla-tester commented Jul 16, 2020

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

Support DROP of named check constraint from column for batch https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/2094

@zzzeek
Copy link
Member

zzzeek commented Jul 16, 2020

separately I located sqlalchemy/sqlalchemy#5463 but that does not seem to be the immediate problem here.

I've added support for the DROP but I'm not sure if everything is doing the right thing here, there is currently no way to add the constraint to the column so I'm not sure how you are getting that result from your upgrade(), unless the table already exists with that column -level constraint.

if you are able to try the patch at https://gerrit.sqlalchemy.org/changes/sqlalchemy%2Falembic~2094/revisions/1/patch?zip that would help.

@zzzeek zzzeek removed the requires triage label Jul 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants