-
-
Notifications
You must be signed in to change notification settings - Fork 332
Add opt-in autogenerate support for CHECK constraint detection #1811
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
Open
bjorkbjork
wants to merge
2
commits into
sqlalchemy:main
Choose a base branch
from
bjorkbjork:feature/autogenerate-check-constraints
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,195 @@ | ||
| # mypy: allow-untyped-defs, allow-untyped-calls, allow-incomplete-defs | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import logging | ||
| from typing import Optional | ||
| from typing import TYPE_CHECKING | ||
| from typing import Union | ||
|
|
||
| from sqlalchemy import schema as sa_schema | ||
|
|
||
| from .util import _InspectorConv | ||
| from ...operations import ops | ||
| from ...util import PriorityDispatchResult | ||
| from ...util import sqla_compat | ||
|
|
||
| if TYPE_CHECKING: | ||
| from sqlalchemy.engine.interfaces import ReflectedCheckConstraint | ||
| from sqlalchemy.sql.elements import quoted_name | ||
| from sqlalchemy.sql.schema import CheckConstraint | ||
| from sqlalchemy.sql.schema import Table | ||
|
|
||
| from ...autogenerate.api import AutogenContext | ||
| from ...ddl.impl import DefaultImpl | ||
| from ...operations.ops import ModifyTableOps | ||
| from ...runtime.plugins import Plugin | ||
|
|
||
|
|
||
| log = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| def _make_check_constraint( | ||
| impl: DefaultImpl, | ||
| params: ReflectedCheckConstraint, | ||
| conn_table: Table, | ||
| ) -> CheckConstraint: | ||
| const = sa_schema.CheckConstraint( | ||
| params["sqltext"], | ||
| name=params["name"], | ||
| **impl.adjust_reflected_dialect_options(params, "check_constraint"), | ||
| ) | ||
| conn_table.append_constraint(const) | ||
| return const | ||
|
|
||
|
|
||
| def _compare_check_constraints( | ||
| autogen_context: AutogenContext, | ||
| modify_table_ops: ModifyTableOps, | ||
| schema: Optional[str], | ||
| tname: Union[quoted_name, str], | ||
| conn_table: Optional[Table], | ||
| metadata_table: Optional[Table], | ||
| ) -> PriorityDispatchResult: | ||
| if conn_table is None or metadata_table is None: | ||
| return PriorityDispatchResult.CONTINUE | ||
|
|
||
| inspector = autogen_context.inspector | ||
| impl = autogen_context.migration_context.impl | ||
|
|
||
| metadata_ck_constraints = { | ||
| ck | ||
| for ck in metadata_table.constraints | ||
| if isinstance(ck, sa_schema.CheckConstraint) | ||
| and not sqla_compat._is_type_bound(ck) | ||
| } | ||
|
|
||
| try: | ||
| conn_ck_list = _InspectorConv(inspector).get_check_constraints( | ||
| tname, schema=schema | ||
| ) | ||
| except NotImplementedError: | ||
| return PriorityDispatchResult.CONTINUE | ||
|
|
||
| conn_ck_list = [ | ||
| ck | ||
| for ck in conn_ck_list | ||
| if ck.get("name") is not None | ||
| and autogen_context.run_name_filters( | ||
| ck["name"], | ||
| "check_constraint", | ||
| {"table_name": tname, "schema_name": schema}, | ||
| ) | ||
| ] | ||
|
|
||
| conn_ck_objs = { | ||
| _make_check_constraint(impl, ck_def, conn_table) | ||
| for ck_def in conn_ck_list | ||
| } | ||
|
|
||
| metadata_ck_sig = { | ||
| impl._create_metadata_constraint_sig(ck) | ||
| for ck in metadata_ck_constraints | ||
| if sqla_compat._constraint_is_named(ck, autogen_context.dialect) | ||
| } | ||
|
|
||
| conn_ck_sig = { | ||
| impl._create_reflected_constraint_sig(ck) for ck in conn_ck_objs | ||
| } | ||
|
|
||
| metadata_ck_by_name = { | ||
| c.name: c | ||
| for c in metadata_ck_sig | ||
| if sqla_compat.constraint_name_string(c.name) | ||
| } | ||
| conn_ck_by_name = { | ||
| c.name: c | ||
| for c in conn_ck_sig | ||
| if sqla_compat.constraint_name_string(c.name) | ||
| } | ||
|
|
||
| for removed_name in sorted( | ||
| set(conn_ck_by_name).difference(metadata_ck_by_name) | ||
| ): | ||
| conn_obj = conn_ck_by_name[removed_name] | ||
| if autogen_context.run_object_filters( | ||
| conn_obj.const, | ||
| conn_obj.name, | ||
| "check_constraint", | ||
| True, | ||
| None, | ||
| ): | ||
| modify_table_ops.ops.append( | ||
| ops.DropConstraintOp.from_constraint(conn_obj.const) | ||
| ) | ||
| log.info( | ||
| "Detected removed check constraint %r on table %r", | ||
| conn_obj.name, | ||
| tname, | ||
| ) | ||
|
|
||
| for existing_name in sorted( | ||
| set(metadata_ck_by_name).intersection(conn_ck_by_name) | ||
| ): | ||
| metadata_obj = metadata_ck_by_name[existing_name] | ||
| conn_obj = conn_ck_by_name[existing_name] | ||
|
|
||
| comparison = metadata_obj.compare_to_reflected(conn_obj) | ||
|
|
||
| if comparison.is_different: | ||
| if autogen_context.run_object_filters( | ||
| metadata_obj.const, | ||
| metadata_obj.name, | ||
| "check_constraint", | ||
| False, | ||
| conn_obj.const, | ||
| ): | ||
| log.info( | ||
| "Detected changed check constraint %r on table %r: %s", | ||
| existing_name, | ||
| tname, | ||
| comparison.message, | ||
| ) | ||
| modify_table_ops.ops.append( | ||
| ops.DropConstraintOp.from_constraint(conn_obj.const) | ||
| ) | ||
| modify_table_ops.ops.append( | ||
| ops.AddConstraintOp.from_constraint(metadata_obj.const) | ||
| ) | ||
| elif comparison.is_skip: | ||
| log.info( | ||
| "Cannot compare check constraint %r, " | ||
| "assuming equal and skipping. %s", | ||
| existing_name, | ||
| comparison.message, | ||
| ) | ||
|
|
||
| for added_name in sorted( | ||
| set(metadata_ck_by_name).difference(conn_ck_by_name) | ||
| ): | ||
| metadata_obj = metadata_ck_by_name[added_name] | ||
| if autogen_context.run_object_filters( | ||
| metadata_obj.const, | ||
| metadata_obj.name, | ||
| "check_constraint", | ||
| False, | ||
| None, | ||
| ): | ||
| modify_table_ops.ops.append( | ||
| ops.AddConstraintOp.from_constraint(metadata_obj.const) | ||
| ) | ||
| log.info( | ||
| "Detected added check constraint %r on table %r", | ||
| metadata_obj.name, | ||
| tname, | ||
| ) | ||
|
|
||
| return PriorityDispatchResult.CONTINUE | ||
|
|
||
|
|
||
| def setup(plugin: Plugin) -> None: | ||
| plugin.add_autogenerate_comparator( | ||
| _compare_check_constraints, | ||
| "table", | ||
| "checkconstraints", | ||
| ) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ | |
| from typing import TypeVar | ||
| from typing import Union | ||
|
|
||
| from sqlalchemy.sql.schema import CheckConstraint | ||
| from sqlalchemy.sql.schema import Constraint | ||
| from sqlalchemy.sql.schema import ForeignKeyConstraint | ||
| from sqlalchemy.sql.schema import Index | ||
|
|
@@ -86,6 +87,7 @@ class _constraint_sig(Generic[_C]): | |
| _is_index: ClassVar[bool] = False | ||
| _is_fk: ClassVar[bool] = False | ||
| _is_uq: ClassVar[bool] = False | ||
| _is_ck: ClassVar[bool] = False | ||
|
|
||
| _is_metadata: bool | ||
|
|
||
|
|
@@ -325,5 +327,38 @@ def is_uq_sig(sig: _constraint_sig) -> TypeGuard[_uq_constraint_sig]: | |
| return sig._is_uq | ||
|
|
||
|
|
||
| class _ck_constraint_sig(_constraint_sig[CheckConstraint]): | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Federico Caselli (CaselIT) wrote: nit: let's move this before the other typeguards View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/6672 |
||
| _is_ck = True | ||
|
|
||
| @classmethod | ||
| def _register(cls) -> None: | ||
| _clsreg["check_constraint"] = cls | ||
| _clsreg["table_or_column_check_constraint"] = cls | ||
| _clsreg["column_check_constraint"] = cls | ||
|
|
||
| def __init__( | ||
| self, | ||
| is_metadata: bool, | ||
| impl: DefaultImpl, | ||
| const: CheckConstraint, | ||
| ) -> None: | ||
| self._is_metadata = is_metadata | ||
| self.impl = impl | ||
| self.const = const | ||
| self.name = sqla_compat.constraint_name_or_none(const.name) | ||
| self._sig = (self.name,) | ||
|
|
||
| def _compare_to_reflected( | ||
| self, other: _constraint_sig[_C] | ||
| ) -> ComparisonResult: | ||
| assert self._is_metadata | ||
| assert is_ck_sig(other) | ||
| return self.impl.compare_check_constraint(self.const, other.const) | ||
|
|
||
|
|
||
| def is_ck_sig(sig: _constraint_sig) -> TypeGuard[_ck_constraint_sig]: | ||
| return sig._is_ck | ||
|
|
||
|
|
||
| def is_fk_sig(sig: _constraint_sig) -> TypeGuard[_fk_constraint_sig]: | ||
| return sig._is_fk | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Federico Caselli (CaselIT) wrote:
I don't think this is needed.
View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/6672