]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
Correct for MySQL 8.0 table and schema names in FK reflection
authorMike Bayer <mike_mp@zzzcomputing.com>
Mon, 29 Jul 2019 18:07:21 +0000 (14:07 -0400)
committerMike Bayer <mike_mp@zzzcomputing.com>
Mon, 29 Jul 2019 18:12:58 +0000 (14:12 -0400)
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

doc/build/changelog/unreleased_13/4751.rst [new file with mode: 0644]
lib/sqlalchemy/dialects/mysql/base.py
test/dialect/mysql/test_reflection.py

diff --git a/doc/build/changelog/unreleased_13/4751.rst b/doc/build/changelog/unreleased_13/4751.rst
new file mode 100644 (file)
index 0000000..086defe
--- /dev/null
@@ -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
+
index f5e974be00417390c95f1fa69728e4bf66da18d2..13012a4f20954845aaf66675467e1aa275045b37 100644 (file)
@@ -2386,7 +2386,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,)
         )
 
@@ -2559,16 +2559,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,
@@ -2623,19 +2627,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
index 790d629d64a74f9cb75fd24c59994a2a6d1221e7..1ab646f3c6eae14e5ed861efd44390426f6ae4bd 100644 (file)
@@ -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": {},
                     },