]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
use only object_id() function for temp tables
authorMike Barry <michael.barry@gmo.com>
Wed, 26 Oct 2022 17:40:32 +0000 (13:40 -0400)
committermike bayer <mike_mp@zzzcomputing.com>
Fri, 28 Oct 2022 13:09:07 +0000 (13:09 +0000)
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
(cherry picked from commit 2af33d79eddc696c0fb1ef749999fa5d0d33f214)

doc/build/changelog/unreleased_14/8700.rst
doc/build/changelog/unreleased_14/8714.rst [new file with mode: 0644]
lib/sqlalchemy/dialects/mssql/base.py
lib/sqlalchemy/testing/requirements.py
lib/sqlalchemy/testing/suite/test_reflection.py
test/requirements.py

index b9369e038bd5c93e9095a8ee880d2f11316a7521..205f251ef40053226cb665baa6cf0749e6076544 100644 (file)
@@ -2,14 +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 method never worked for
-    Oracle in this way, so for compatibility within the 1.4 series,
-    Oracle's dialect remains returning False for ``has_table()`` against a
-    view within the 1.4 series.
-
+    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, where has_table() reports True for views on all
-    backends including SQL Server and Oracle. Test support is added within the
-    1.4 series to ensure ``has_table()`` remains working per spec re: views.
+    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
@@ -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.
index 738ff7ce34aeb2ff3be996b8496e6cfb0f16349e..6c530b631c2559b4a1a83f9ed959c28cfe191642 100644 (file)
@@ -2933,29 +2933,18 @@ class MSDialect(default.DefaultDialect):
     @_db_plus_owner
     def has_table(self, connection, tablename, dbname, owner, schema):
         self._ensure_has_table_connection(connection)
-        if tablename.startswith("#"):  # temporary table
-            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)
+        if tablename.startswith("#"):  # temporary table
+            # mssql does not support temporary views
+            # SQL Error [4103] [S0001]: "#v": Temporary views are not allowed
+            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.execute(s).scalars().fetchall()
-            # #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
 
index 857d1fdef1e3e6a25a9f09d0f57fbb63d535b679..7e8a030e3224f2bb8c55ded0ab429e76669bb5d2 100644 (file)
@@ -663,6 +663,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"""
index ff98f18c0735e547bca9ec59c6c120eab3780fc7..3f234d2ea9c36738c5cf5996b61b4cefe00a75c7 100644 (file)
@@ -33,7 +33,23 @@ from ...testing import is_true
 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
@@ -55,6 +71,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):
@@ -75,6 +93,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, "user_tmp_%s" % (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"))
@@ -110,6 +159,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.fails_on(
         "oracle",
         "per #8700 this remains at its previous behavior of not "
@@ -326,22 +388,11 @@ class QuotedNameArgumentTest(fixtures.TablesTest):
         assert insp.get_check_constraints(name)
 
 
-class ComponentReflectionTest(fixtures.TablesTest):
+class ComponentReflectionTest(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)
index feb5e2a52251244994cc5f37fc0bec0b43c197d7..b5e9711115cb13d2daaf8ae9d69ae7b115524201 100644 (file)
@@ -608,6 +608,18 @@ class DefaultRequirements(SuiteRequirements):
         """
         return only_on(["postgresql"])
 
+    @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 default_schema_name_switch(self):
         return only_on(["postgresql", "oracle"])