From: Mike Bayer Date: Mon, 11 Feb 2019 14:07:03 +0000 (-0500) Subject: Add complete coverage and fix lower() for MySQL 88718 workaround X-Git-Tag: rel_1_3_0~20^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=49197c7b36573d91b015019d4635071f9da1c216;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Add complete coverage and fix lower() for MySQL 88718 workaround Fixed a second regression caused by :ticket:`4344` (the first was :ticket:`4361`), which works around MySQL issue 88718, where the lower casing function used was not correct for Python 2 with OSX/Windows casing conventions, which would then raise ``TypeError``. Full coverage has been added to this logic so that every codepath is exercised in a mock style for all three casing conventions on all versions of Python. MySQL 8.0 has meanwhile fixed issue 88718 so the workaround is only applies to a particular span of MySQL 8.0 versions. Fixes: #4492 Change-Id: I14e7237e0be4a9c21c58c921066304ae99ac4dc6 --- diff --git a/doc/build/changelog/unreleased_12/4492.rst b/doc/build/changelog/unreleased_12/4492.rst new file mode 100644 index 0000000000..7c7c901f32 --- /dev/null +++ b/doc/build/changelog/unreleased_12/4492.rst @@ -0,0 +1,12 @@ +.. change:: + :tags: bug, mysql + :tickets: 4492 + + Fixed a second regression caused by :ticket:`4344` (the first was + :ticket:`4361`), which works around MySQL issue 88718, where the lower + casing function used was not correct for Python 2 with OSX/Windows casing + conventions, which would then raise ``TypeError``. Full coverage has been + added to this logic so that every codepath is exercised in a mock style for + all three casing conventions on all versions of Python. MySQL 8.0 has + meanwhile fixed issue 88718 so the workaround is only applies to a + particular span of MySQL 8.0 versions. diff --git a/lib/sqlalchemy/dialects/mysql/base.py b/lib/sqlalchemy/dialects/mysql/base.py index 76b52e0d7d..1b89a3ac6e 100644 --- a/lib/sqlalchemy/dialects/mysql/base.py +++ b/lib/sqlalchemy/dialects/mysql/base.py @@ -2543,8 +2543,10 @@ class MySQLDialect(default.DefaultDialect): # preserves the original table/schema casing, but SHOW CREATE # TABLE does not. this problem is not in lower_case_table_names=1, # but use case-insensitive matching for these two modes in any case. + if self._casing in (1, 2): - lower = str.lower + def lower(s): + return s.lower() else: # if on case sensitive, there can be two tables referenced # with the same name different casing, so we need to use diff --git a/test/dialect/mysql/test_reflection.py b/test/dialect/mysql/test_reflection.py index c42a3be402..790d629d64 100644 --- a/test/dialect/mysql/test_reflection.py +++ b/test/dialect/mysql/test_reflection.py @@ -27,6 +27,7 @@ from sqlalchemy import TIMESTAMP from sqlalchemy import Unicode from sqlalchemy import UnicodeText from sqlalchemy import UniqueConstraint +from sqlalchemy import util from sqlalchemy.dialects.mysql import base as mysql from sqlalchemy.dialects.mysql import reflection as _reflection from sqlalchemy.schema import CreateIndex @@ -36,6 +37,7 @@ from sqlalchemy.testing import eq_ from sqlalchemy.testing import expect_warnings from sqlalchemy.testing import fixtures from sqlalchemy.testing import is_ +from sqlalchemy.testing import mock class TypeReflectionTest(fixtures.TestBase): @@ -757,6 +759,117 @@ class ReflectionTest(fixtures.TestBase, AssertsCompiledSQL): [{"name": "foo_idx", "column_names": ["x"], "unique": False}], ) + def _bug_88718_casing_0(self): + fkeys_casing_0 = [ + { + "name": "FK_PlaylistTTrackId", + "constrained_columns": ["TTrackID"], + "referred_schema": "test_schema", + "referred_table": "Track", + "referred_columns": ["trackid"], + "options": {}, + }, + { + "name": "FK_PlaylistTrackId", + "constrained_columns": ["TrackID"], + "referred_schema": None, + "referred_table": "Track", + "referred_columns": ["trackid"], + "options": {}, + }, + ] + ischema_casing_0 = [ + ("test", "Track", "TrackID"), + ("test_schema", "Track", "TrackID"), + ] + return fkeys_casing_0, ischema_casing_0 + + def _bug_88718_casing_1(self): + fkeys_casing_1 = [ + { + "name": "FK_PlaylistTTrackId", + "constrained_columns": ["TTrackID"], + "referred_schema": "test_schema", + "referred_table": "Track", + "referred_columns": ["trackid"], + "options": {}, + }, + { + "name": "FK_PlaylistTrackId", + "constrained_columns": ["TrackID"], + "referred_schema": None, + "referred_table": "Track", + "referred_columns": ["trackid"], + "options": {}, + }, + ] + ischema_casing_1 = [ + (util.u("test"), util.u("Track"), "TrackID"), + (util.u("test_schema"), util.u("Track"), "TrackID"), + ] + return fkeys_casing_1, ischema_casing_1 + + def _bug_88718_casing_2(self): + fkeys_casing_2 = [ + { + "name": "FK_PlaylistTTrackId", + "constrained_columns": ["TTrackID"], + "referred_schema": "test_schema", + "referred_table": "Track", + "referred_columns": ["trackid"], + "options": {}, + }, + { + "name": "FK_PlaylistTrackId", + "constrained_columns": ["TrackID"], + "referred_schema": None, + "referred_table": "Track", + "referred_columns": ["trackid"], + "options": {}, + }, + ] + ischema_casing_2 = [ + ("test", "Track", "TrackID"), + ("test_schema", "Track", "TrackID"), + ] + return fkeys_casing_2, ischema_casing_2 + + def test_correct_for_mysql_bug_88718(self): + dialect = mysql.dialect() + + for casing, (fkeys, ischema) in [ + (0, self._bug_88718_casing_0()), + (1, self._bug_88718_casing_1()), + (2, self._bug_88718_casing_2()), + ]: + dialect._casing = casing + dialect.default_schema_name = "test" + connection = mock.Mock( + dialect=dialect, execute=lambda stmt, **params: ischema + ) + dialect._correct_for_mysql_bug_88718(fkeys, connection) + eq_( + fkeys, + [ + { + "name": "FK_PlaylistTTrackId", + "constrained_columns": ["TTrackID"], + "referred_schema": "test_schema", + "referred_table": "Track", + "referred_columns": ["TrackID"], + "options": {}, + }, + { + "name": "FK_PlaylistTrackId", + "constrained_columns": ["TrackID"], + "referred_schema": None, + "referred_table": "Track", + "referred_columns": ["TrackID"], + "options": {}, + }, + ], + ) + @testing.provide_metadata def test_case_sensitive_column_constraint_reflection(self): # test for issue #4344 which works around