From: Georg Wicke-Arndt Date: Mon, 22 Jan 2024 15:22:43 +0000 (-0500) Subject: Parse NOT NULL for MySQL generated columns X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=14f30b85c7bea7839111bbe54576b290457e3a8d;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Parse NOT NULL for MySQL generated columns Fixed issue where NULL/NOT NULL would not be properly reflected from a MySQL column that also specified the VIRTUAL or STORED directives. Pull request courtesy Georg Wicke-Arndt. Fixes: #10850 Closes: #10851 Pull-request: https://github.com/sqlalchemy/sqlalchemy/pull/10851 Pull-request-sha: fb9a81020c393231ca90a1e88342b11cf64414a1 Change-Id: I9a80d0db722c15682e18f0390a7b58e5979e73a1 --- diff --git a/doc/build/changelog/unreleased_20/10850.rst b/doc/build/changelog/unreleased_20/10850.rst new file mode 100644 index 0000000000..6b6b323ce8 --- /dev/null +++ b/doc/build/changelog/unreleased_20/10850.rst @@ -0,0 +1,7 @@ +.. change:: + :tags: bug, mysql + :tickets: 10850 + + Fixed issue where NULL/NOT NULL would not be properly reflected from a + MySQL column that also specified the VIRTUAL or STORED directives. Pull + request courtesy Georg Wicke-Arndt. diff --git a/lib/sqlalchemy/dialects/mysql/reflection.py b/lib/sqlalchemy/dialects/mysql/reflection.py index 74c60f07b5..c764e8ccc7 100644 --- a/lib/sqlalchemy/dialects/mysql/reflection.py +++ b/lib/sqlalchemy/dialects/mysql/reflection.py @@ -290,6 +290,9 @@ class MySQLTableDefinitionParser: # this can be "NULL" in the case of TIMESTAMP if spec.get("notnull", False) == "NOT NULL": col_kw["nullable"] = False + # For generated columns, the nullability is marked in a different place + if spec.get("notnull_generated", False) == "NOT NULL": + col_kw["nullable"] = False # AUTO_INCREMENT if spec.get("autoincr", False): @@ -452,7 +455,9 @@ class MySQLTableDefinitionParser: r"(?: +ON UPDATE [\-\w\.\(\)]+)?)" r"))?" r"(?: +(?:GENERATED ALWAYS)? ?AS +(?P\(" - r".*\))? ?(?PVIRTUAL|STORED)?)?" + r".*\))? ?(?PVIRTUAL|STORED)?" + r"(?: +(?P(?:NOT )?NULL))?" + r")?" r"(?: +(?PAUTO_INCREMENT))?" r"(?: +COMMENT +'(?P(?:''|[^'])*)')?" r"(?: +COLUMN_FORMAT +(?P\w+))?" diff --git a/test/dialect/mysql/test_reflection.py b/test/dialect/mysql/test_reflection.py index f3d1f34599..79e7198ef3 100644 --- a/test/dialect/mysql/test_reflection.py +++ b/test/dialect/mysql/test_reflection.py @@ -764,103 +764,152 @@ class ReflectionTest(fixtures.TestBase, AssertsCompiledSQL): view_names = dialect.get_view_names(connection, "information_schema") self.assert_("TABLES" in view_names) - def test_nullable_reflection(self, metadata, connection): - """test reflection of NULL/NOT NULL, in particular with TIMESTAMP - defaults where MySQL is inconsistent in how it reports CREATE TABLE. - - """ - meta = metadata - - # this is ideally one table, but older MySQL versions choke - # on the multiple TIMESTAMP columns - row = connection.exec_driver_sql( - "show variables like '%%explicit_defaults_for_timestamp%%'" - ).first() - explicit_defaults_for_timestamp = row[1].lower() in ("on", "1", "true") - - reflected = [] - for idx, cols in enumerate( + @testing.combinations( + ( [ - [ - "x INTEGER NULL", - "y INTEGER NOT NULL", - "z INTEGER", - "q TIMESTAMP NULL", - ], - ["p TIMESTAMP NULL DEFAULT CURRENT_TIMESTAMP"], - ["r TIMESTAMP NOT NULL"], - ["s TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP"], - ["t TIMESTAMP"], - ["u TIMESTAMP DEFAULT CURRENT_TIMESTAMP"], - ] - ): - Table("nn_t%d" % idx, meta) # to allow DROP - - connection.exec_driver_sql( - """ - CREATE TABLE nn_t%d ( - %s - ) - """ - % (idx, ", \n".join(cols)) - ) - - reflected.extend( - { - "name": d["name"], - "nullable": d["nullable"], - "default": d["default"], - } - for d in inspect(connection).get_columns("nn_t%d" % idx) - ) - - if connection.dialect._is_mariadb_102: - current_timestamp = "current_timestamp()" - else: - current_timestamp = "CURRENT_TIMESTAMP" - - eq_( - reflected, + "x INTEGER NULL", + "y INTEGER NOT NULL", + "z INTEGER", + "q TIMESTAMP NULL", + ], [ {"name": "x", "nullable": True, "default": None}, {"name": "y", "nullable": False, "default": None}, {"name": "z", "nullable": True, "default": None}, {"name": "q", "nullable": True, "default": None}, - {"name": "p", "nullable": True, "default": current_timestamp}, + ], + ), + ( + ["p TIMESTAMP NULL DEFAULT CURRENT_TIMESTAMP"], + [ + { + "name": "p", + "nullable": True, + "default": "CURRENT_TIMESTAMP", + } + ], + ), + ( + ["r TIMESTAMP NOT NULL"], + [ { "name": "r", "nullable": False, - "default": None - if explicit_defaults_for_timestamp - else ( - "%(current_timestamp)s " - "ON UPDATE %(current_timestamp)s" - ) - % {"current_timestamp": current_timestamp}, - }, - {"name": "s", "nullable": False, "default": current_timestamp}, + "default": None, + "non_explicit_defaults_for_ts_default": ( + "CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP" + ), + } + ], + ), + ( + ["s TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP"], + [ + { + "name": "s", + "nullable": False, + "default": "CURRENT_TIMESTAMP", + } + ], + ), + ( + ["t TIMESTAMP"], + [ { "name": "t", - "nullable": True - if explicit_defaults_for_timestamp - else False, - "default": None - if explicit_defaults_for_timestamp - else ( - "%(current_timestamp)s " - "ON UPDATE %(current_timestamp)s" - ) - % {"current_timestamp": current_timestamp}, - }, + "nullable": True, + "default": None, + "non_explicit_defaults_for_ts_nullable": False, + "non_explicit_defaults_for_ts_default": ( + "CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP" + ), + } + ], + ), + ( + ["u TIMESTAMP DEFAULT CURRENT_TIMESTAMP"], + [ { "name": "u", - "nullable": True - if explicit_defaults_for_timestamp - else False, - "default": current_timestamp, - }, + "nullable": True, + "non_explicit_defaults_for_ts_nullable": False, + "default": "CURRENT_TIMESTAMP", + } ], - ) + ), + ( + ["v INTEGER GENERATED ALWAYS AS (4711) VIRTUAL NOT NULL"], + [ + { + "name": "v", + "nullable": False, + "default": None, + } + ], + testing.requires.mysql_notnull_generated_columns, + ), + argnames="ddl_columns,expected_reflected", + ) + def test_nullable_reflection( + self, metadata, connection, ddl_columns, expected_reflected + ): + """test reflection of NULL/NOT NULL, in particular with TIMESTAMP + defaults where MySQL is inconsistent in how it reports CREATE TABLE. + + """ + row = connection.exec_driver_sql( + "show variables like '%%explicit_defaults_for_timestamp%%'" + ).first() + explicit_defaults_for_timestamp = row[1].lower() in ("on", "1", "true") + + def get_expected_default(er): + if ( + not explicit_defaults_for_timestamp + and "non_explicit_defaults_for_ts_default" in er + ): + default = er["non_explicit_defaults_for_ts_default"] + else: + default = er["default"] + + if default is not None and connection.dialect._is_mariadb_102: + default = default.replace( + "CURRENT_TIMESTAMP", "current_timestamp()" + ) + + return default + + def get_expected_nullable(er): + if ( + not explicit_defaults_for_timestamp + and "non_explicit_defaults_for_ts_nullable" in er + ): + return er["non_explicit_defaults_for_ts_nullable"] + else: + return er["nullable"] + + expected_reflected = [ + { + "name": er["name"], + "nullable": get_expected_nullable(er), + "default": get_expected_default(er), + } + for er in expected_reflected + ] + + Table("nullable_refl", metadata) + + cols_ddl = ", \n".join(ddl_columns) + connection.exec_driver_sql(f"CREATE TABLE nullable_refl ({cols_ddl})") + + reflected = [ + { + "name": d["name"], + "nullable": d["nullable"], + "default": d["default"], + } + for d in inspect(connection).get_columns("nullable_refl") + ] + eq_(reflected, expected_reflected) def test_reflection_with_unique_constraint(self, metadata, connection): insp = inspect(connection) diff --git a/test/requirements.py b/test/requirements.py index cb6ceeb265..e5692a83f7 100644 --- a/test/requirements.py +++ b/test/requirements.py @@ -1694,6 +1694,10 @@ class DefaultRequirements(SuiteRequirements): def mysql_fsp(self): return only_if(["mysql >= 5.6.4", "mariadb"]) + @property + def mysql_notnull_generated_columns(self): + return only_if(["mysql >= 5.7"]) + @property def mysql_fully_case_sensitive(self): return only_if(self._has_mysql_fully_case_sensitive)