Reflect "NO ACTION" as None; support "RESTRICT"

The "NO ACTION" keyword for foreign key "ON UPDATE" is now considered to be
the default cascade for a foreign key on all supporting backends (SQlite,
MySQL, PostgreSQL) and when detected is not included in the reflection
dictionary; this is already the behavior for PostgreSQL and MySQL for all
previous SQLAlchemy versions in any case.   The "RESTRICT" keyword is
positively stored when detected; PostgreSQL does report on this keyword,
and MySQL as of version 8.0 does as well.  On earlier MySQL versions, it is
not reported by the database.

Fixes: #4741
Change-Id: I6becf1f2450605c1991158bb8a04d954dcc7396c

diff --git a/doc/build/changelog/unreleased_14/4741.rst b/doc/build/changelog/unreleased_14/4741.rst
new file mode 100644 (file)
index 0000000..afc94bd
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/4741.rst
@@ -0,0 +1,12 @@
+.. change::
+    :tags: sql, reflection
+    :tickets: 4741
+
+    The "NO ACTION" keyword for foreign key "ON UPDATE" is now considered to be
+    the default cascade for a foreign key on all supporting backends (SQlite,
+    MySQL, PostgreSQL) and when detected is not included in the reflection
+    dictionary; this is already the behavior for PostgreSQL and MySQL for all
+    previous SQLAlchemy versions in any case.   The "RESTRICT" keyword is
+    positively stored when detected; PostgreSQL does report on this keyword,
+    and MySQL as of version 8.0 does as well.  On earlier MySQL versions, it is
+    not reported by the database.

diff --git a/lib/sqlalchemy/dialects/mysql/base.py b/lib/sqlalchemy/dialects/mysql/base.py
index 9aa80a5f4b46719917f38c839262c5df03d7fac4..da76201f2acf5ba97aa789b415b283f2b88f518f 100644 (file)
--- a/lib/sqlalchemy/dialects/mysql/base.py
+++ b/lib/sqlalchemy/dialects/mysql/base.py
@@ -2541,7 +2541,7 @@ class MySQLDialect(default.DefaultDialect):
 
             con_kw = {}
             for opt in ("onupdate", "ondelete"):
-                if spec.get(opt, False):
+                if spec.get(opt, False) not in ("NO ACTION", None):
                     con_kw[opt] = spec[opt]
 
             fkey_d = {

diff --git a/lib/sqlalchemy/dialects/mysql/reflection.py b/lib/sqlalchemy/dialects/mysql/reflection.py
index b089483d7c9ace0bbdba2d3defc16e422e299944..3e5cc7b11110fe277b667897b8e54b64e531e53b 100644 (file)
--- a/lib/sqlalchemy/dialects/mysql/reflection.py
+++ b/lib/sqlalchemy/dialects/mysql/reflection.py
@@ -423,7 +423,7 @@ class MySQLTableDefinitionParser(object):
         #
         # unique constraints come back as KEYs
         kw = quotes.copy()
-        kw["on"] = "RESTRICT|CASCADE|SET NULL|NOACTION"
+        kw["on"] = "RESTRICT|CASCADE|SET NULL|NO ACTION"
         self._re_fk_constraint = _re_compile(
             r"  "
             r"CONSTRAINT +"

diff --git a/lib/sqlalchemy/dialects/postgresql/base.py b/lib/sqlalchemy/dialects/postgresql/base.py
index 2bc48c53ee619e4f349f7dc7e98b5a0d053d8902..5aea02cfcaa3adb26a4931d909b054669ddc4386 100644 (file)
--- a/lib/sqlalchemy/dialects/postgresql/base.py
+++ b/lib/sqlalchemy/dialects/postgresql/base.py
@@ -3150,19 +3150,24 @@ class PGDialect(default.DefaultDialect):
                 preparer._unquote_identifier(x)
                 for x in re.split(r"\s*,\s", referred_columns)
             ]
+            options = {
+                k: v
+                for k, v in [
+                    ("onupdate", onupdate),
+                    ("ondelete", ondelete),
+                    ("initially", initially),
+                    ("deferrable", deferrable),
+                    ("match", match),
+                ]
+                if v is not None and v != "NO ACTION"
+            }
             fkey_d = {
                 "name": conname,
                 "constrained_columns": constrained_columns,
                 "referred_schema": referred_schema,
                 "referred_table": referred_table,
                 "referred_columns": referred_columns,
-                "options": {
-                    "onupdate": onupdate,
-                    "ondelete": ondelete,
-                    "deferrable": deferrable,
-                    "initially": initially,
-                    "match": match,
-                },
+                "options": options,
             }
             fkeys.append(fkey_d)
         return fkeys

diff --git a/lib/sqlalchemy/dialects/sqlite/base.py b/lib/sqlalchemy/dialects/sqlite/base.py
index d6a7139880b875c31d75cfbc77cc381bf327def6..b6ca8fe3c95260217ef84079bbd99eb829217a0a 100644 (file)
--- a/lib/sqlalchemy/dialects/sqlite/base.py
+++ b/lib/sqlalchemy/dialects/sqlite/base.py
@@ -1810,9 +1810,13 @@ class SQLiteDialect(default.DefaultDialect):
 
                 for token in re.split(r" *\bON\b *", onupdatedelete.upper()):
                     if token.startswith("DELETE"):
-                        options["ondelete"] = token[6:].strip()
+                        ondelete = token[6:].strip()
+                        if ondelete and ondelete != "NO ACTION":
+                            options["ondelete"] = ondelete
                     elif token.startswith("UPDATE"):
-                        options["onupdate"] = token[6:].strip()
+                        onupdate = token[6:].strip()
+                        if onupdate and onupdate != "NO ACTION":
+                            options["onupdate"] = onupdate
                 yield (
                     constraint_name,
                     constrained_columns,

diff --git a/lib/sqlalchemy/testing/requirements.py b/lib/sqlalchemy/testing/requirements.py
index 3a216174065086fddfdd108f0a63478562dfdd15..04537905b298ba3851a3088163849fbf3fdc6f5e 100644 (file)
--- a/lib/sqlalchemy/testing/requirements.py
+++ b/lib/sqlalchemy/testing/requirements.py
@@ -426,10 +426,22 @@ class SuiteRequirements(Requirements):
     def foreign_key_constraint_option_reflection_ondelete(self):
         return exclusions.closed()
 
+    @property
+    def fk_constraint_option_reflection_ondelete_restrict(self):
+        return exclusions.closed()
+
+    @property
+    def fk_constraint_option_reflection_ondelete_noaction(self):
+        return exclusions.closed()
+
     @property
     def foreign_key_constraint_option_reflection_onupdate(self):
         return exclusions.closed()
 
+    @property
+    def fk_constraint_option_reflection_onupdate_restrict(self):
+        return exclusions.closed()
+
     @property
     def temp_table_reflection(self):
         return exclusions.open()

diff --git a/lib/sqlalchemy/testing/suite/test_reflection.py b/lib/sqlalchemy/testing/suite/test_reflection.py
index e529a0753c7d1be422b509f4bc9a63b33d1d8145..53e1599b3533e5905ed7a0c39eda057b27667cbf 100644 (file)
--- a/lib/sqlalchemy/testing/suite/test_reflection.py
+++ b/lib/sqlalchemy/testing/suite/test_reflection.py
@@ -671,10 +671,29 @@ class ComponentReflectionTest(fixtures.TablesTest):
     def test_get_foreign_key_options_onupdate(self):
         self._test_get_foreign_key_options(onupdate="SET NULL")
 
+    @testing.requires.foreign_key_constraint_option_reflection_onupdate
+    def test_get_foreign_key_options_onupdate_noaction(self):
+        self._test_get_foreign_key_options(onupdate="NO ACTION", expected={})
+
+    @testing.requires.fk_constraint_option_reflection_ondelete_noaction
+    def test_get_foreign_key_options_ondelete_noaction(self):
+        self._test_get_foreign_key_options(ondelete="NO ACTION", expected={})
+
+    @testing.requires.fk_constraint_option_reflection_onupdate_restrict
+    def test_get_foreign_key_options_onupdate_restrict(self):
+        self._test_get_foreign_key_options(onupdate="RESTRICT")
+
+    @testing.requires.fk_constraint_option_reflection_ondelete_restrict
+    def test_get_foreign_key_options_ondelete_restrict(self):
+        self._test_get_foreign_key_options(ondelete="RESTRICT")
+
     @testing.provide_metadata
-    def _test_get_foreign_key_options(self, **options):
+    def _test_get_foreign_key_options(self, expected=None, **options):
         meta = self.metadata
 
+        if expected is None:
+            expected = options
+
         Table(
             "x",
             meta,
@@ -714,7 +733,8 @@ class ComponentReflectionTest(fixtures.TablesTest):
         eq_(dict((k, opts[k]) for k in opts if opts[k]), {})
 
         opts = insp.get_foreign_keys("user")[0]["options"]
-        eq_(dict((k, opts[k]) for k in opts if opts[k]), options)
+        eq_(opts, expected)
+        # eq_(dict((k, opts[k]) for k in opts if opts[k]), expected)
 
     def _assert_insp_indexes(self, indexes, expected_indexes):
         index_names = [d["name"] for d in indexes]

diff --git a/test/dialect/postgresql/test_reflection.py b/test/dialect/postgresql/test_reflection.py
index 7e7a82d46974edad5f4ba7e424124f9f6bbd738e..ea72d571056f29101015a3cec9036014515cbb03 100644 (file)
--- a/test/dialect/postgresql/test_reflection.py
+++ b/test/dialect/postgresql/test_reflection.py
@@ -1147,13 +1147,7 @@ class ReflectionTest(fixtures.TestBase):
                 "referred_columns": ["id"],
                 "referred_table": "industry",
                 "referred_schema": None,
-                "options": {
-                    "onupdate": "CASCADE",
-                    "deferrable": None,
-                    "ondelete": "CASCADE",
-                    "initially": None,
-                    "match": None,
-                },
+                "options": {"onupdate": "CASCADE", "ondelete": "CASCADE"},
             },
         }
         metadata.create_all()

diff --git a/test/dialect/test_sqlite.py b/test/dialect/test_sqlite.py
index 1ec1d5d6f440aa7588343e6971182fdb6808b43b..616652389675a23c51af985846026b67e6d67ca8 100644 (file)
--- a/test/dialect/test_sqlite.py
+++ b/test/dialect/test_sqlite.py
@@ -1914,7 +1914,7 @@ class ConstraintReflectionTest(fixtures.TestBase):
                     "referred_schema": None,
                     "name": "fk4",
                     "constrained_columns": ["c4"],
-                    "options": {"onupdate": "NO ACTION"},
+                    "options": {},
                 },
             ],
         )

diff --git a/test/requirements.py b/test/requirements.py
index 059ce01871923ecde2de315773ba94afae72810f..6385b0d8779f8ace2e1fcdb2ca27414d68ad53eb 100644 (file)
--- a/test/requirements.py
+++ b/test/requirements.py
@@ -110,10 +110,22 @@ class DefaultRequirements(SuiteRequirements):
     def foreign_key_constraint_option_reflection_ondelete(self):
         return only_on(["postgresql", "mysql", "sqlite", "oracle"])
 
+    @property
+    def fk_constraint_option_reflection_ondelete_restrict(self):
+        return only_on(["postgresql", "sqlite", self._mysql_80])
+
+    @property
+    def fk_constraint_option_reflection_ondelete_noaction(self):
+        return only_on(["postgresql", "mysql", "sqlite"])
+
     @property
     def foreign_key_constraint_option_reflection_onupdate(self):
         return only_on(["postgresql", "mysql", "sqlite"])
 
+    @property
+    def fk_constraint_option_reflection_onupdate_restrict(self):
+        return only_on(["postgresql", "sqlite", self._mysql_80])
+
     @property
     def comment_reflection(self):
         return only_on(["postgresql", "mysql", "oracle"])
@@ -1307,6 +1319,13 @@ class DefaultRequirements(SuiteRequirements):
 
         return only_if(check)
 
+    def _mysql_80(self, config):
+        return (
+            against(config, "mysql")
+            and config.db.dialect._is_mysql
+            and config.db.dialect.server_version_info >= (8,)
+        )
+
     def _mariadb_102(self, config):
         return (
             against(config, "mysql")