From b710a5286310710d5d0a6d5170696e8fe4d4a0d8 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Wed, 25 Jan 2023 08:58:03 -0500 Subject: [PATCH] Make comment support conditional on fn_listextendedproperty availability The newly added comment reflection and rendering capability of the MSSQL dialect, added in :ticket:`7844`, will now be disabled by default if it cannot be determined that an unsupported backend such as Azure Synapse may be in use; this backend does not support table and column comments and does not support the SQL Server routines in use to generate them as well as to reflect them. A new parameter ``supports_comments`` is added to the dialect which defaults to ``None``, indicating that comment support should be auto-detected. When set to ``True`` or ``False``, the comment support is either enabled or disabled unconditionally. Fixes: #9142 Change-Id: Ib5cac31806185e7353e15b3d83b580652d304b3b --- doc/build/changelog/unreleased_20/9142.rst | 18 ++++++ lib/sqlalchemy/dialects/mssql/base.py | 50 +++++++++++++++ test/dialect/mssql/test_engine.py | 71 ++++++++++++++++++++++ test/dialect/mssql/test_reflection.py | 34 ++++++++++- 4 files changed, 171 insertions(+), 2 deletions(-) create mode 100644 doc/build/changelog/unreleased_20/9142.rst diff --git a/doc/build/changelog/unreleased_20/9142.rst b/doc/build/changelog/unreleased_20/9142.rst new file mode 100644 index 0000000000..11abff6056 --- /dev/null +++ b/doc/build/changelog/unreleased_20/9142.rst @@ -0,0 +1,18 @@ +.. change:: + :tags: bug, mssql, regression + :tickets: 9142 + + The newly added comment reflection and rendering capability of the MSSQL + dialect, added in :ticket:`7844`, will now be disabled by default if it + cannot be determined that an unsupported backend such as Azure Synapse may + be in use; this backend does not support table and column comments and does + not support the SQL Server routines in use to generate them as well as to + reflect them. A new parameter ``supports_comments`` is added to the dialect + which defaults to ``None``, indicating that comment support should be + auto-detected. When set to ``True`` or ``False``, the comment support is + either enabled or disabled unconditionally. + + .. seealso:: + + :ref:`mssql_comment_support` + diff --git a/lib/sqlalchemy/dialects/mssql/base.py b/lib/sqlalchemy/dialects/mssql/base.py index eabea88db6..a30c57c7f6 100644 --- a/lib/sqlalchemy/dialects/mssql/base.py +++ b/lib/sqlalchemy/dialects/mssql/base.py @@ -435,6 +435,29 @@ Note that when using LIMIT and/or OFFSET, whether using the older or newer SQL Server syntaxes, the statement must have an ORDER BY as well, else a :class:`.CompileError` is raised. +.. _mssql_comment_support: + +DDL Comment Support +-------------------- + +Comment support, which includes DDL rendering for attributes such as +:paramref:`_schema.Table.comment` and :paramref:`_schema.Column.comment`, as +well as the ability to reflect these comments, is supported assuming a +supported version of SQL Server is in use. If a non-supported version such as +Azure Synapse is detected at first-connect time (based on the presence +of the ``fn_listextendedproperty`` SQL function), comment support including +rendering and table-comment reflection is disabled, as both features rely upon +SQL Server stored procedures and functions that are not available on all +backend types. + +To force comment support to be on or off, bypassing autodetection, set the +parameter ``supports_comments`` within :func:`_sa.create_engine`:: + + e = create_engine("mssql+pyodbc://u:p@dsn", supports_comments=False) + +.. versionadded:: 2.0 Added support for table and column comments for + the SQL Server dialect, including DDL generation and reflection. + .. _mssql_isolation_level: Transaction Isolation Level @@ -3039,6 +3062,7 @@ class MSDialect(default.DefaultDialect): use_scope_identity=True, schema_name="dbo", deprecate_large_types=None, + supports_comments=None, json_serializer=None, json_deserializer=None, legacy_schema_aliasing=None, @@ -3053,6 +3077,9 @@ class MSDialect(default.DefaultDialect): self.ignore_no_transaction_on_rollback = ( ignore_no_transaction_on_rollback ) + self._user_defined_supports_comments = uds = supports_comments + if uds is not None: + self.supports_comments = uds if legacy_schema_aliasing is not None: util.warn_deprecated( @@ -3160,6 +3187,7 @@ class MSDialect(default.DefaultDialect): super().initialize(connection) self._setup_version_attributes() self._setup_supports_nvarchar_max(connection) + self._setup_supports_comments(connection) def _setup_version_attributes(self): if self.server_version_info[0] not in list(range(8, 17)): @@ -3193,6 +3221,23 @@ class MSDialect(default.DefaultDialect): else: self._supports_nvarchar_max = True + def _setup_supports_comments(self, connection): + if self._user_defined_supports_comments is not None: + return + + try: + connection.scalar( + sql.text( + "SELECT 1 FROM fn_listextendedproperty" + "(default, default, default, default, " + "default, default, default)" + ) + ) + except exc.DBAPIError: + self.supports_comments = False + else: + self.supports_comments = True + def _get_default_schema_name(self, connection): query = sql.text("SELECT schema_name()") default_schema_name = connection.scalar(query) @@ -3432,6 +3477,11 @@ class MSDialect(default.DefaultDialect): @reflection.cache def get_table_comment(self, connection, table_name, schema=None, **kw): + if not self.supports_comments: + raise NotImplementedError( + "Can't get table comments on current SQL Server version in use" + ) + schema_name = schema if schema else self.default_schema_name COMMENT_SQL = """ SELECT cast(com.value as nvarchar(max)) diff --git a/test/dialect/mssql/test_engine.py b/test/dialect/mssql/test_engine.py index 57c6274e72..70fd1a6431 100644 --- a/test/dialect/mssql/test_engine.py +++ b/test/dialect/mssql/test_engine.py @@ -5,6 +5,7 @@ from unittest.mock import Mock from sqlalchemy import Column from sqlalchemy import event from sqlalchemy import exc +from sqlalchemy import inspect from sqlalchemy import Integer from sqlalchemy import Numeric from sqlalchemy import select @@ -620,6 +621,76 @@ class VersionDetectionTest(fixtures.TestBase): eq_(dialect._get_server_version_info(conn), expected) +class MiscTest(fixtures.TestBase): + __only_on__ = "mssql" + __backend__ = True + + @testing.variation("enable_comments", [True, False]) + def test_comments_enabled_disabled( + self, testing_engine, metadata, enable_comments + ): + Table( + "tbl_with_comments", + metadata, + Column( + "id", + Integer, + primary_key=True, + comment="pk comment", + ), + Column("no_comment", Integer), + Column( + "has_comment", + String(20), + comment="has the comment", + ), + comment="table comment", + ) + + eng = testing_engine( + options={"supports_comments": bool(enable_comments)} + ) + metadata.create_all(eng) + + insp = inspect(testing.db) + if enable_comments: + eq_( + insp.get_table_comment("tbl_with_comments"), + {"text": "table comment"}, + ) + + cols = { + col["name"]: col["comment"] + for col in insp.get_columns("tbl_with_comments") + } + eq_( + cols, + { + "id": "pk comment", + "no_comment": None, + "has_comment": "has the comment", + }, + ) + else: + eq_( + insp.get_table_comment("tbl_with_comments"), + {"text": None}, + ) + + cols = { + col["name"]: col["comment"] + for col in insp.get_columns("tbl_with_comments") + } + eq_( + cols, + { + "id": None, + "no_comment": None, + "has_comment": None, + }, + ) + + class RealIsolationLevelTest(fixtures.TestBase): __only_on__ = "mssql" __backend__ = True diff --git a/test/dialect/mssql/test_reflection.py b/test/dialect/mssql/test_reflection.py index 59b88c6ef3..d1cade7b08 100644 --- a/test/dialect/mssql/test_reflection.py +++ b/test/dialect/mssql/test_reflection.py @@ -28,6 +28,7 @@ from sqlalchemy.testing import AssertsCompiledSQL from sqlalchemy.testing import ComparesTables from sqlalchemy.testing import eq_ from sqlalchemy.testing import expect_raises +from sqlalchemy.testing import expect_raises_message from sqlalchemy.testing import fixtures from sqlalchemy.testing import in_ from sqlalchemy.testing import is_ @@ -784,7 +785,8 @@ class ReflectionTest(fixtures.TestBase, ComparesTables, AssertsCompiledSQL): is_(col["type"].length, None) in_("max", str(col["type"].compile(dialect=connection.dialect))) - def test_comments(self, metadata, connection): + @testing.fixture + def comment_table(self, metadata): Table( "tbl_with_comments", metadata, @@ -802,7 +804,9 @@ class ReflectionTest(fixtures.TestBase, ComparesTables, AssertsCompiledSQL): ), comment="table comment çòé 🐍", ) - metadata.create_all(connection) + metadata.create_all(testing.db) + + def test_comments(self, connection, comment_table): insp = inspect(connection) eq_( insp.get_table_comment("tbl_with_comments"), @@ -822,6 +826,32 @@ class ReflectionTest(fixtures.TestBase, ComparesTables, AssertsCompiledSQL): }, ) + def test_comments_not_supported(self, testing_engine, comment_table): + eng = testing_engine(options={"supports_comments": False}) + insp = inspect(eng) + + with expect_raises_message( + NotImplementedError, + "Can't get table comments on current SQL Server version in use", + ): + insp.get_table_comment("tbl_with_comments") + + # currently, column comments still reflect normally since we + # aren't using an fn/sp for that + + cols = { + col["name"]: col["comment"] + for col in insp.get_columns("tbl_with_comments") + } + eq_( + cols, + { + "id": "pk comment 🔑", + "no_comment": None, + "has_comment": "has the comment § méil 📧", + }, + ) + class InfoCoerceUnicodeTest(fixtures.TestBase, AssertsCompiledSQL): def test_info_unicode_cast_no_2000(self): -- 2.47.2