From: Mike Bayer Date: Mon, 29 Jul 2019 18:07:21 +0000 (-0400) Subject: Correct for MySQL 8.0 table and schema names in FK reflection X-Git-Tag: rel_1_3_7~16 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4cc40b0ad698245ec7d6ad6d9ee081fcf16d33ec;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Correct for MySQL 8.0 table and schema names in FK reflection Added another fix for an upstream MySQL 8 issue where a case sensitive table name is reported incorrectly in foreign key constraint reflection, this is an extension of the fix first added for :ticket:`4344` which affects a case sensitive column name. The new issue occurs through MySQL 8.0.17, so the general logic of the 88718 fix remains in place. .. seealso:: https://bugs.mysql.com/bug.php?id=96365 - upstream bug Fixes: #4751 Change-Id: I391903565db919b85b6b3c62c28f4b90ee596135 (cherry picked from commit 9a6654e3af74710b55feb6b5b0218dc767d7013b) --- diff --git a/doc/build/changelog/unreleased_13/4751.rst b/doc/build/changelog/unreleased_13/4751.rst new file mode 100644 index 0000000000..086defea80 --- /dev/null +++ b/doc/build/changelog/unreleased_13/4751.rst @@ -0,0 +1,14 @@ +.. change:: + :tags: bug, mysql + :tickets: 4751 + + Added another fix for an upstream MySQL 8 issue where a case sensitive + table name is reported incorrectly in foreign key constraint reflection, + this is an extension of the fix first added for :ticket:`4344` which + affects a case sensitive column name. The new issue occurs through MySQL + 8.0.17, so the general logic of the 88718 fix remains in place. + + .. seealso:: + + https://bugs.mysql.com/bug.php?id=96365 - upstream bug + diff --git a/lib/sqlalchemy/dialects/mysql/base.py b/lib/sqlalchemy/dialects/mysql/base.py index d8013eb70a..e02476e0e2 100644 --- a/lib/sqlalchemy/dialects/mysql/base.py +++ b/lib/sqlalchemy/dialects/mysql/base.py @@ -2384,7 +2384,7 @@ class MySQLDialect(default.DefaultDialect): default.DefaultDialect.initialize(self, connection) - self._needs_correct_for_88718 = ( + self._needs_correct_for_88718_96365 = ( not self._is_mariadb and self.server_version_info >= (8,) ) @@ -2557,16 +2557,20 @@ class MySQLDialect(default.DefaultDialect): } fkeys.append(fkey_d) - if self._needs_correct_for_88718: - self._correct_for_mysql_bug_88718(fkeys, connection) + if self._needs_correct_for_88718_96365: + self._correct_for_mysql_bugs_88718_96365(fkeys, connection) return fkeys - def _correct_for_mysql_bug_88718(self, fkeys, connection): + def _correct_for_mysql_bugs_88718_96365(self, fkeys, connection): # Foreign key is always in lower case (MySQL 8.0) # https://bugs.mysql.com/bug.php?id=88718 # issue #4344 for SQLAlchemy + # table name also for MySQL 8.0 + # https://bugs.mysql.com/bug.php?id=96365 + # issue #4751 for SQLAlchemy + # for lower_case_table_names=2, information_schema.columns # preserves the original table/schema casing, but SHOW CREATE # TABLE does not. this problem is not in lower_case_table_names=1, @@ -2621,19 +2625,24 @@ class MySQLDialect(default.DefaultDialect): # is necessary d = defaultdict(dict) for schema, tname, cname in correct_for_wrong_fk_case: + d[(lower(schema), lower(tname))]["SCHEMANAME"] = schema + d[(lower(schema), lower(tname))]["TABLENAME"] = tname d[(lower(schema), lower(tname))][cname.lower()] = cname for fkey in fkeys: + rec = d[ + ( + lower(fkey["referred_schema"] or default_schema_name), + lower(fkey["referred_table"]), + ) + ] + + fkey["referred_table"] = rec["TABLENAME"] + if fkey["referred_schema"] is not None: + fkey["referred_schema"] = rec["SCHEMANAME"] + fkey["referred_columns"] = [ - d[ - ( - lower( - fkey["referred_schema"] or default_schema_name - ), - lower(fkey["referred_table"]), - ) - ][col.lower()] - for col in fkey["referred_columns"] + rec[col.lower()] for col in fkey["referred_columns"] ] @reflection.cache diff --git a/test/dialect/mysql/test_reflection.py b/test/dialect/mysql/test_reflection.py index 790d629d64..1ab646f3c6 100644 --- a/test/dialect/mysql/test_reflection.py +++ b/test/dialect/mysql/test_reflection.py @@ -759,12 +759,12 @@ class ReflectionTest(fixtures.TestBase, AssertsCompiledSQL): [{"name": "foo_idx", "column_names": ["x"], "unique": False}], ) - def _bug_88718_casing_0(self): + def _bug_88718_96365_casing_0(self): fkeys_casing_0 = [ { "name": "FK_PlaylistTTrackId", "constrained_columns": ["TTrackID"], - "referred_schema": "test_schema", + "referred_schema": "Test_Schema", "referred_table": "Track", "referred_columns": ["trackid"], "options": {}, @@ -779,17 +779,17 @@ class ReflectionTest(fixtures.TestBase, AssertsCompiledSQL): }, ] ischema_casing_0 = [ - ("test", "Track", "TrackID"), - ("test_schema", "Track", "TrackID"), + ("Test", "Track", "TrackID"), + ("Test_Schema", "Track", "TrackID"), ] return fkeys_casing_0, ischema_casing_0 - def _bug_88718_casing_1(self): + def _bug_88718_96365_casing_1(self): fkeys_casing_1 = [ { "name": "FK_PlaylistTTrackId", "constrained_columns": ["TTrackID"], - "referred_schema": "test_schema", + "referred_schema": "Test_Schema", "referred_table": "Track", "referred_columns": ["trackid"], "options": {}, @@ -804,18 +804,20 @@ class ReflectionTest(fixtures.TestBase, AssertsCompiledSQL): }, ] ischema_casing_1 = [ - (util.u("test"), util.u("Track"), "TrackID"), - (util.u("test_schema"), util.u("Track"), "TrackID"), + (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): + def _bug_88718_96365_casing_2(self): fkeys_casing_2 = [ { "name": "FK_PlaylistTTrackId", "constrained_columns": ["TTrackID"], + # I haven't tested schema name but since col/table both + # do it, assume schema name also comes back wrong "referred_schema": "test_schema", - "referred_table": "Track", + "referred_table": "track", "referred_columns": ["trackid"], "options": {}, }, @@ -823,38 +825,40 @@ class ReflectionTest(fixtures.TestBase, AssertsCompiledSQL): "name": "FK_PlaylistTrackId", "constrained_columns": ["TrackID"], "referred_schema": None, - "referred_table": "Track", + # table name also comes back wrong (probably schema also) + # with casing=2, see https://bugs.mysql.com/bug.php?id=96365 + "referred_table": "track", "referred_columns": ["trackid"], "options": {}, }, ] ischema_casing_2 = [ - ("test", "Track", "TrackID"), - ("test_schema", "Track", "TrackID"), + ("Test", "Track", "TrackID"), + ("Test_Schema", "Track", "TrackID"), ] return fkeys_casing_2, ischema_casing_2 - def test_correct_for_mysql_bug_88718(self): + def test_correct_for_mysql_bugs_88718_96365(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()), + (0, self._bug_88718_96365_casing_0()), + (1, self._bug_88718_96365_casing_1()), + (2, self._bug_88718_96365_casing_2()), ]: dialect._casing = casing - dialect.default_schema_name = "test" + dialect.default_schema_name = "Test" connection = mock.Mock( dialect=dialect, execute=lambda stmt, **params: ischema ) - dialect._correct_for_mysql_bug_88718(fkeys, connection) + dialect._correct_for_mysql_bugs_88718_96365(fkeys, connection) eq_( fkeys, [ { "name": "FK_PlaylistTTrackId", "constrained_columns": ["TTrackID"], - "referred_schema": "test_schema", + "referred_schema": "Test_Schema", "referred_table": "Track", "referred_columns": ["TrackID"], "options": {}, @@ -910,6 +914,14 @@ class ReflectionTest(fixtures.TestBase, AssertsCompiledSQL): m1.create_all() if testing.db.dialect._casing in (1, 2): + # the original test for the 88718 fix here in [ticket:4344] + # actually set referred_table='track', with the wrong casing! + # this test was never run. with [ticket:4751], I've gone through + # the trouble to create docker containers with true + # lower_case_table_names=2 and per + # https://bugs.mysql.com/bug.php?id=96365 the table name being + # lower case is also an 8.0 regression. + eq_( inspect(testing.db).get_foreign_keys("PlaylistTrack"), [ @@ -917,7 +929,7 @@ class ReflectionTest(fixtures.TestBase, AssertsCompiledSQL): "name": "FK_PlaylistTTrackId", "constrained_columns": ["TTrackID"], "referred_schema": testing.config.test_schema, - "referred_table": "track", + "referred_table": "Track", "referred_columns": ["TrackID"], "options": {}, }, @@ -925,7 +937,7 @@ class ReflectionTest(fixtures.TestBase, AssertsCompiledSQL): "name": "FK_PlaylistTrackId", "constrained_columns": ["TrackID"], "referred_schema": None, - "referred_table": "track", + "referred_table": "Track", "referred_columns": ["TrackID"], "options": {}, },