From: Mike Bayer Date: Wed, 26 Jun 2019 15:45:50 +0000 (-0400) Subject: Reflect "NO ACTION" as None; support "RESTRICT" X-Git-Tag: rel_1_4_0b1~824 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5cf8e14d58c89fdb94c60bf5e94d8b13d296da25;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git 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 index 0000000000..afc94bddbb --- /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 9aa80a5f4b..da76201f2a 100644 --- 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 b089483d7c..3e5cc7b111 100644 --- 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 2bc48c53ee..5aea02cfca 100644 --- 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 d6a7139880..b6ca8fe3c9 100644 --- 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 3a21617406..04537905b2 100644 --- 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 e529a0753c..53e1599b35 100644 --- 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 7e7a82d469..ea72d57105 100644 --- 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 1ec1d5d6f4..6166523896 100644 --- 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 059ce01871..6385b0d877 100644 --- 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")