use only object_id() function for temp tables

Fixed issue with :meth:`.Inspector.has_table` when used against a temporary
table for the SQL Server dialect would fail an invalid object name error on
some Azure variants, due to an unnecessary information schema query that is
not supported on those server versions. Pull request courtesy Mike Barry.

the patch also fills out test support for has_table()
against temp tables, temp views, adding to the has_table() support just
added for views in #8700.

Fixes: #8714
Closes: #8716
Pull-request: https://github.com/sqlalchemy/sqlalchemy/pull/8716
Pull-request-sha: e2ac7a52e2b09a349a703ba1e1a2911f4d3c0912

Change-Id: Ia73e4e9e977a2d6b7e100abd2f81a8c8777dc9bb

diff --git a/doc/build/changelog/unreleased_14/8700.rst b/doc/build/changelog/unreleased_14/8700.rst
index 9e969c3e236b302b8619a4b12d4be7922730d575..205f251ef40053226cb665baa6cf0749e6076544 100644 (file)
--- a/doc/build/changelog/unreleased_14/8700.rst
+++ b/doc/build/changelog/unreleased_14/8700.rst
@@ -2,8 +2,9 @@
     :tags: bug, mssql, reflection
     :tickets: 8700
 
-    Fixed regression which occurred throughout the 1.4 series where the
-    :meth:`.Inspector.has_table` method, which historically reported on views
-    as well, stopped working for SQL Server. The issue is not present in the
-    2.0 series which uses a different reflection architecture. Test support is
-    added to ensure ``has_table()`` remains working per spec re: views.
+    Fixed issue with :meth:`.Inspector.has_table` when used against a view for
+    the SQL Server dialect would erroneously return ``False``, due to a
+    regression in the 1.4 series which removed support for this on SQL Server.
+    The issue is not present in the 2.0 series which uses a different
+    reflection architecture. Test support is added to ensure ``has_table()``
+    remains working per spec re: views.

diff --git a/doc/build/changelog/unreleased_14/8714.rst b/doc/build/changelog/unreleased_14/8714.rst
new file mode 100644 (file)
index 0000000..d75f257
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/8714.rst
@@ -0,0 +1,8 @@
+.. change::
+    :tags: bug, mssql
+    :tickets: 8714
+
+    Fixed issue with :meth:`.Inspector.has_table` when used against a temporary
+    table for the SQL Server dialect would fail an invalid object name error on
+    some Azure variants, due to an unnecessary information schema query that is
+    not supported on those server versions. Pull request courtesy Mike Barry.

diff --git a/lib/sqlalchemy/dialects/mssql/base.py b/lib/sqlalchemy/dialects/mssql/base.py
index fdb0704f6c6c365b03c179b30bd4ffc341ca9a4d..5d42d98e35bfd3be53a170efdb9e98dd706ce65d 100644 (file)
--- a/lib/sqlalchemy/dialects/mssql/base.py
+++ b/lib/sqlalchemy/dialects/mssql/base.py
@@ -3226,28 +3226,13 @@ class MSDialect(default.DefaultDialect):
         if tablename.startswith("#"):  # temporary table
             # mssql does not support temporary views
             # SQL Error [4103] [S0001]: "#v": Temporary views are not allowed
-            tables = ischema.mssql_temp_table_columns
-
-            s = sql.select(tables.c.table_name).where(
-                tables.c.table_name.like(
-                    self._temp_table_name_like_pattern(tablename)
+            return bool(
+                connection.scalar(
+                    # U filters on user tables only.
+                    text("SELECT object_id(:table_name, 'U')"),
+                    {"table_name": "tempdb.dbo.[{}]".format(tablename)},
                 )
             )
-
-            # #7168: fetch all (not just first match) in case some other #temp
-            #        table with the same name happens to appear first
-            table_names = connection.scalars(s).all()
-            # #6910: verify it's not a temp table from another session
-            for table_name in table_names:
-                if bool(
-                    connection.scalar(
-                        text("SELECT object_id(:table_name)"),
-                        {"table_name": "tempdb.dbo.[{}]".format(table_name)},
-                    )
-                ):
-                    return True
-            else:
-                return False
         else:
             tables = ischema.tables
 

diff --git a/lib/sqlalchemy/testing/requirements.py b/lib/sqlalchemy/testing/requirements.py
index 1d6f1103d7b3d690b8190ab5e9967f3eb3c7f742..8ceb2af2f6b78f7e7f7b46e5a1aa1276d95e29e3 100644 (file)
--- a/lib/sqlalchemy/testing/requirements.py
+++ b/lib/sqlalchemy/testing/requirements.py
@@ -709,6 +709,11 @@ class SuiteRequirements(Requirements):
         """target dialect supports listing of temporary table names"""
         return exclusions.closed()
 
+    @property
+    def has_temp_table(self):
+        """target dialect supports checking a single temp table name"""
+        return exclusions.closed()
+
     @property
     def temporary_tables(self):
         """target database supports temporary tables"""

diff --git a/lib/sqlalchemy/testing/suite/test_reflection.py b/lib/sqlalchemy/testing/suite/test_reflection.py
index dd7d38c5ac06c4189cbc38b32d7b719f66b4675e..68d1c13faaeb17883bd3a306c4df689a89472a17 100644 (file)
--- a/lib/sqlalchemy/testing/suite/test_reflection.py
+++ b/lib/sqlalchemy/testing/suite/test_reflection.py
@@ -45,7 +45,23 @@ from ...testing import mock
 metadata, users = None, None
 
 
-class HasTableTest(fixtures.TablesTest):
+class OneConnectionTablesTest(fixtures.TablesTest):
+    @classmethod
+    def setup_bind(cls):
+        # TODO: when temp tables are subject to server reset,
+        # this will also have to disable that server reset from
+        # happening
+        if config.requirements.independent_connections.enabled:
+            from sqlalchemy import pool
+
+            return engines.testing_engine(
+                options=dict(poolclass=pool.StaticPool, scope="class"),
+            )
+        else:
+            return config.db
+
+
+class HasTableTest(OneConnectionTablesTest):
     __backend__ = True
 
     @classmethod
@@ -67,6 +83,8 @@ class HasTableTest(fixtures.TablesTest):
 
         if testing.requires.view_reflection:
             cls.define_views(metadata)
+        if testing.requires.has_temp_table.enabled:
+            cls.define_temp_tables(metadata)
 
     @classmethod
     def define_views(cls, metadata):
@@ -87,6 +105,37 @@ class HasTableTest(fixtures.TablesTest):
                 DDL("DROP VIEW %s.vv" % (config.test_schema)),
             )
 
+    @classmethod
+    def temp_table_name(cls):
+        return get_temp_table_name(
+            config, config.db, f"user_tmp_{config.ident}"
+        )
+
+    @classmethod
+    def define_temp_tables(cls, metadata):
+        kw = temp_table_keyword_args(config, config.db)
+        table_name = cls.temp_table_name()
+        user_tmp = Table(
+            table_name,
+            metadata,
+            Column("id", sa.INT, primary_key=True),
+            Column("name", sa.VARCHAR(50)),
+            **kw,
+        )
+        if (
+            testing.requires.view_reflection.enabled
+            and testing.requires.temporary_views.enabled
+        ):
+            event.listen(
+                user_tmp,
+                "after_create",
+                DDL(
+                    "create temporary view user_tmp_v as "
+                    "select * from user_tmp_%s" % config.ident
+                ),
+            )
+            event.listen(user_tmp, "before_drop", DDL("drop view user_tmp_v"))
+
     def test_has_table(self):
         with config.db.begin() as conn:
             is_true(config.db.dialect.has_table(conn, "test_table"))
@@ -130,6 +179,19 @@ class HasTableTest(fixtures.TablesTest):
         insp = inspect(connection)
         is_true(insp.has_table("vv"))
 
+    @testing.requires.has_temp_table
+    def test_has_table_temp_table(self, connection):
+        insp = inspect(connection)
+        temp_table_name = self.temp_table_name()
+        is_true(insp.has_table(temp_table_name))
+
+    @testing.requires.has_temp_table
+    @testing.requires.view_reflection
+    @testing.requires.temporary_views
+    def test_has_table_temp_view(self, connection):
+        insp = inspect(connection)
+        is_true(insp.has_table("user_tmp_v"))
+
     @testing.requires.views
     @testing.requires.schemas
     def test_has_table_view_schema(self, connection):
@@ -393,22 +455,11 @@ def _multi_combination(fn):
     return schema(scope(kind(filter_names(fn))))
 
 
-class ComponentReflectionTest(ComparesTables, fixtures.TablesTest):
+class ComponentReflectionTest(ComparesTables, OneConnectionTablesTest):
     run_inserts = run_deletes = None
 
     __backend__ = True
 
-    @classmethod
-    def setup_bind(cls):
-        if config.requirements.independent_connections.enabled:
-            from sqlalchemy import pool
-
-            return engines.testing_engine(
-                options=dict(poolclass=pool.StaticPool, scope="class"),
-            )
-        else:
-            return config.db
-
     @classmethod
     def define_tables(cls, metadata):
         cls.define_reflected_tables(metadata, None)

diff --git a/test/requirements.py b/test/requirements.py
index 69eda37664f8a448775b6766d4fa7724faa15519..e56c944ffcce31fd546c97ce29ad90b76df3cd26 100644 (file)
--- a/test/requirements.py
+++ b/test/requirements.py
@@ -609,6 +609,18 @@ class DefaultRequirements(SuiteRequirements):
             self._sqlite_file_db
         )
 
+    @property
+    def has_temp_table(self):
+        """target dialect supports checking a single temp table name
+
+        unfortunately this is not the same as temp_table_names
+
+        """
+
+        return only_on(["sqlite", "oracle", "postgresql", "mssql"]) + skip_if(
+            self._sqlite_file_db
+        )
+
     @property
     def temporary_views(self):
         """target database supports temporary views"""