From 32b49375d31fbad4937d8b5f8b65c86b3c6b36e3 Mon Sep 17 00:00:00 2001 From: Paul Becotte Date: Mon, 24 Feb 2020 09:39:55 -0500 Subject: [PATCH] Include post-parenthesis tokens when tokenizing type string Fixed regression caused by the new "type comparison" logic introduced in 1.4 as part of :ticket:`605` where comparisons of MySQL "unsigned integer" datatypes would produce false positives, as the regular expression logic was not correctly parsing the "unsigned" token when MySQL's default display width would be returned by the database. Pull request courtesy Paul Becotte. Fixes: #661 Closes: #662 Pull-request: https://github.com/sqlalchemy/alembic/pull/662 Pull-request-sha: 28f181247ac475069eb8cd3669331e689fc78792 Change-Id: Ie1ba69fe0b3c2084026a51b1f835b3aab66aba6a --- alembic/ddl/impl.py | 26 ++++++++++++++++++++++---- docs/build/unreleased/661.rst | 10 ++++++++++ tests/test_autogen_diffs.py | 20 ++++++++++++++++++++ 3 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 docs/build/unreleased/661.rst diff --git a/alembic/ddl/impl.py b/alembic/ddl/impl.py index 72fc2947..655d8c94 100644 --- a/alembic/ddl/impl.py +++ b/alembic/ddl/impl.py @@ -330,11 +330,29 @@ class DefaultImpl(with_metaclass(ImplMeta)): def _tokenize_column_type(self, column): definition = self.dialect.type_compiler.process(column.type).lower() - # py27 - py36 - col_type, *param_terms = re.findall... - matches = re.findall("[^(),]+", definition) - col_type, param_terms = matches[0], matches[1:] + + # tokenize the SQLAlchemy-generated version of a type, so that + # the two can be compared. + # + # examples: + # NUMERIC(10, 5) + # TIMESTAMP WITH TIMEZONE + # INTEGER UNSIGNED + # INTEGER (10) UNSIGNED + # + # "ext" are the words after the parenthesis, if any, but if there + # are no parenthesis, then these are part of "col". so they are + # moved together for normalization purposes + matches = re.search( + r"^(?P[^()]*)(?:\((?P.*?)\))?(?P[^()]*)?$", + definition, + ).groupdict(default="") + col_type = matches["col"] + if matches["ext"]: + col_type = col_type.strip() + " " + matches["ext"].strip() + params = Params(col_type, [], {}) - for term in param_terms: + for term in re.findall("[^(),]+", matches["params"] or ""): if "=" in term: key, val = term.split("=") params.kwargs[key.strip()] = val.strip() diff --git a/docs/build/unreleased/661.rst b/docs/build/unreleased/661.rst new file mode 100644 index 00000000..d0520183 --- /dev/null +++ b/docs/build/unreleased/661.rst @@ -0,0 +1,10 @@ +.. change:: + :tags: bug, autogenerate + :tickets: 661 + + Fixed regression caused by the new "type comparison" logic introduced in + 1.4 as part of :ticket:`605` where comparisons of MySQL "unsigned integer" + datatypes would produce false positives, as the regular expression logic + was not correctly parsing the "unsigned" token when MySQL's default display + width would be returned by the database. Pull request courtesy Paul + Becotte. diff --git a/tests/test_autogen_diffs.py b/tests/test_autogen_diffs.py index 7a7702f7..e1e5c8d4 100644 --- a/tests/test_autogen_diffs.py +++ b/tests/test_autogen_diffs.py @@ -31,6 +31,7 @@ from sqlalchemy import TypeDecorator from sqlalchemy import Unicode from sqlalchemy import UniqueConstraint from sqlalchemy import VARCHAR +from sqlalchemy.dialects import mysql from sqlalchemy.dialects import sqlite from sqlalchemy.types import NULLTYPE from sqlalchemy.types import VARBINARY @@ -842,6 +843,25 @@ class CompareMetadataToInspectorTest(TestBase): True, config.requirements.integer_subtype_comparisons, ), + ( # note that the mysql.INTEGER tests only use these params + # if the dialect is "mysql". however we also test that their + # dialect-agnostic representation compares by running this + # against other dialects. + mysql.INTEGER(unsigned=True, display_width=10), + mysql.INTEGER(unsigned=True, display_width=10), + False, + ), + (mysql.INTEGER(unsigned=True), mysql.INTEGER(unsigned=True), False), + ( + mysql.INTEGER(unsigned=True, display_width=10), + mysql.INTEGER(unsigned=True), + False, + ), + ( + mysql.INTEGER(unsigned=True), + mysql.INTEGER(unsigned=True, display_width=10), + False, + ), ) def test_numeric_comparisons(self, cola, colb, expect_changes): is_(self._compare_columns(cola, colb), expect_changes) -- 2.47.2