skip sync transforms temp table

enterprise/backend/src/metabase_enterprise/transforms/util.clj

@@ -8,7 +8,7 @@
    [metabase-enterprise.transforms.settings :as transforms.settings]
    [metabase.driver :as driver]
    [metabase.lib.schema.common :as lib.schema.common]
-   [metabase.premium-features.core :as premium-features]
+   [metabase.premium-features.core :as premium-features :refer [defenterprise]]
    [metabase.query-processor.compile :as qp.compile]
    [metabase.query-processor.pipeline :as qp.pipeline]
    [metabase.sync.core :as sync]
@@ -24,6 +24,10 @@
 
 (set! *warn-on-reflection* true)
 
+(def ^:const transform-temp-table-prefix
+  "Prefix used for temporary tables created during transform execution."
+  "mb_transform_temp_table")
+
 (defn qualified-table-name
   "Return the name of the target table of a transform as a possibly qualified symbol."
   [_driver {:keys [schema name]}]
@@ -250,17 +254,18 @@
 
 (defn temp-table-name
   "Generate a temporary table name with the given suffix and current timestamp in seconds.
-   If table name would exceed max table name length for the driver, fallback to using a shorter base-name."
-  [driver base-table-name suffix]
-  {:pre [(keyword? base-table-name)]}
-  (let [suffix (str "_" suffix "_" (quot (System/currentTimeMillis) 1000))
-        max-len (or (driver/table-name-length-limit driver) Integer/MAX_VALUE)
-        max-prefix-len (- max-len (count suffix))
-        table-name (name base-table-name)
-        prefix (subs table-name 0 (min (count table-name) max-prefix-len))]
-    (assert (pos? max-prefix-len))
-    (keyword (namespace base-table-name)
-             (str prefix suffix))))
+  If table name would exceed max table name length for the driver, fallback to using a shorter base-name"
+  [driver schema]
+  (let [max-len   (max 1 (or (driver/table-name-length-limit driver) Integer/MAX_VALUE))
+        timestamp (str (System/currentTimeMillis))
+        prefix    (str transform-temp-table-prefix "_")
+        available (- max-len (count prefix))
+        ;; If we don't have enough space, take the later digits of the timestamp
+        suffix    (if (>= available (count timestamp))
+                    timestamp
+                    (subs timestamp (- (count timestamp) available)))
+        table-name (str prefix suffix)]
+    (keyword schema table-name)))
 
 (defn rename-tables!
   "Rename multiple tables atomically within a transaction using the new driver/rename-tables method.
@@ -269,6 +274,13 @@
   (log/infof "Renaming tables: %s" (pr-str rename-map))
   (driver/rename-tables! driver database-id rename-map))
 
+(defenterprise is-temp-transform-table?
+  "Return true when `table` matches the transform temporary table naming pattern and transforms are enabled."
+  :feature :transforms
+  [table]
+  (when-let [table-name (:name table)]
+    (str/starts-with? (u/lower-case-en table-name) transform-temp-table-prefix)))
+
 (defn db-routing-enabled?
   "Returns whether or not the given database is either a router or destination database"
   [db-or-id]

enterprise/backend/src/metabase_enterprise/transforms_python/execute.clj

@@ -21,14 +21,6 @@
 
 (set! *warn-on-reflection* true)
 
-(def ^:const temp-table-suffix-new
-  "Suffix used for temporary tables containing new data during swap operations."
-  "new")
-
-(def ^:const temp-table-suffix-old
-  "Suffix used for temporary tables containing old data during swap operations."
-  "old")
-
 (defn- empty-message-log
   "Returns a new message log.
 
@@ -162,8 +154,9 @@
   "Transfer data using the rename-tables*! multimethod with atomicity guarantees.
    Creates new table, then atomically renames target->old and new->target, then drops old."
   [driver db-id table-name metadata data-source]
-  (let [source-table-name (transforms.util/temp-table-name driver table-name temp-table-suffix-new)
-        temp-table-name (transforms.util/temp-table-name driver table-name temp-table-suffix-old)]
+  (let [source-table-name (transforms.util/temp-table-name driver (namespace table-name))
+        temp-table-name (u/poll {:thunk #(transforms.util/temp-table-name driver (namespace table-name))
+                                 :done? #(not= source-table-name %)})]
     (log/info "Using rename-tables strategy with atomicity guarantees")
     (try
 
@@ -183,7 +176,7 @@
   "Transfer data using create + drop + rename to minimize time without data.
    Creates new table, drops old table, then renames new->target."
   [driver db-id table-name metadata data-source]
-  (let [source-table-name (transforms.util/temp-table-name driver table-name temp-table-suffix-new)]
+  (let [source-table-name (transforms.util/temp-table-name driver (namespace table-name))]
     (log/info "Using create-drop-rename strategy to minimize downtime")
     (try
 

enterprise/backend/test/metabase_enterprise/transforms/util_test.clj

@@ -16,24 +16,17 @@
       (let [driver driver/*driver*]
 
         (testing "Basic table name generation"
-          (let [result (transforms.util/temp-table-name driver :base_table "test")]
+          (let [result (transforms.util/temp-table-name driver nil)
+                table-name (name result)]
             (is (keyword? result))
             (is (nil? (namespace result)))
-            (is (str/starts-with? (name result) "base_table_test_"))))
-
-        (testing "Table name with very large base name respects driver limits"
-          (let [very-long-base-name (keyword (apply str (repeat 200 "a")))
-                result (transforms.util/temp-table-name driver very-long-base-name "test")
-                max-len (or (driver/table-name-length-limit driver) Integer/MAX_VALUE)
-                actual-name (name result)]
-            (is (keyword? result))
-            (is (<= (count actual-name) max-len))
-            (is (re-find #"_test_\d+$" actual-name))))
+            (is (str/starts-with? table-name "mb_transform_temp_table_"))
+            (is (re-matches #"mb_transform_temp_table_\d+" table-name))))
 
         (testing "Table name preserves namespace when present"
-          (let [result (transforms.util/temp-table-name driver :schema/base_table "test")]
+          (let [result (transforms.util/temp-table-name driver "schema")]
             (is (= "schema" (namespace result)))
-            (is (str/starts-with? (name result) "base_table_test_"))))))))
+            (is (str/starts-with? (name result) "mb_transform_temp_table_"))))))))
 
 (deftest temp-table-name-creates-table-test
   (testing "temp-table-name produces names that can actually create tables"
@@ -41,9 +34,7 @@
       (let [driver driver/*driver*
             db-id (mt/id)
 
-            long-base-name (keyword (apply str (repeat 1000 "test")))
-
-            table-name (transforms.util/temp-table-name driver long-base-name "temp")
+            table-name (transforms.util/temp-table-name driver nil)
             schema-name (when (get-method sql.tx/session-schema driver)
                           (sql.tx/session-schema driver))
             qualified-table-name (if schema-name
@@ -62,3 +53,19 @@
                 (catch Exception _e
                   ;; Ignore cleanup errors
                   nil)))))))))
+
+(deftest is-temp-transform-tables-test
+  (testing "tables with shcema"
+    (let [table-with-schema    {:name (name (transforms.util/temp-table-name :postgres "schema"))}
+          table-without-schema {:name (name (transforms.util/temp-table-name :postgres "schema"))}]
+      (mt/with-premium-features #{}
+        (is (false? (transforms.util/is-temp-transform-table? table-with-schema)))
+        (is (false? (transforms.util/is-temp-transform-table? table-without-schema))))
+      (mt/with-premium-features #{:transforms}
+        (is (transforms.util/is-temp-transform-table? table-without-schema))
+        (is (transforms.util/is-temp-transform-table? table-with-schema)))))
+
+  (testing "Ignores non-transform tables"
+    (mt/with-premium-features #{:transforms}
+      (is (false? (transforms.util/is-temp-transform-table? {:name :orders})))
+      (is (false? (transforms.util/is-temp-transform-table? {:name :public/orders}))))))

enterprise/backend/test/metabase_enterprise/transforms_python/execute_test.clj

@@ -86,12 +86,9 @@
                   (transforms-python.execute/execute-python-transform! transform {:run-method :manual})
 
                   (let [db-id (mt/id)
-                        tables (t2/select :model/Table :db_id db-id :active true)
-                        new-table-pattern (re-pattern (str ".*" table-name "_" transforms-python.execute/temp-table-suffix-new "_.*"))
-                        old-table-pattern (re-pattern (str ".*" table-name "_" transforms-python.execute/temp-table-suffix-old "_.*"))]
-                    (is (not-any? #(or (re-matches new-table-pattern (:name %))
-                                       (re-matches old-table-pattern (:name %))) tables)
-                        (str "No temp tables (_" transforms-python.execute/temp-table-suffix-new "_ or _" transforms-python.execute/temp-table-suffix-old "_) should remain after successful Python transform"))
+                        tables (t2/select :model/Table :db_id db-id :active true)]
+                    (is (not-any? transforms.util/is-temp-transform-table? tables)
+                        (str "No temp tables should remain after successful Python transform"))
 
                     (is (= [[1 "a"] [2 "b"] [3 "c"]] (transforms.tu/table-rows table-name))
                         "Table should contain the expected data after swap")))))))))))

src/metabase/sync/sync_metadata/tables.clj

@@ -11,6 +11,7 @@
    [metabase.lib.schema.common :as lib.schema.common]
    [metabase.models.humanization :as humanization]
    [metabase.models.interface :as mi]
+   [metabase.premium-features.core :refer [defenterprise]]
    [metabase.sync.fetch-metadata :as fetch-metadata]
    [metabase.sync.interface :as i]
    [metabase.sync.sync-metadata.crufty :as crufty]
@@ -81,6 +82,12 @@
     ;; MSSQL
     #"^syncobj_0x.*"})
 
+(defenterprise is-temp-transform-table?
+  "Return true if `table` references a temporary transform table created during transforms execution."
+  metabase-enterprise.transforms.util
+  [_table]
+  false)
+
 ;;; ---------------------------------------------------- Syncing -----------------------------------------------------
 
 (mu/defn- update-database-metadata!
@@ -243,7 +250,9 @@
   Get set of user tables only, excluding metabase metadata tables."
   [db-metadata :- i/DatabaseMetadata]
   (into #{}
-        (remove metabase-metadata/is-metabase-metadata-table?)
+        (remove (fn [table]
+                  (or (metabase-metadata/is-metabase-metadata-table? table)
+                      (is-temp-transform-table? table))))
         (:tables db-metadata)))
 
 (mu/defn- select-tables :- [:set (ms/InstanceOf :model/Table)]

test/metabase/sync/sync_metadata/tables_test.clj

@@ -42,6 +42,22 @@
              (set (for [table (t2/select [:model/Table :name :visibility_type :initial_sync_status] :db_id (mt/id))]
                     (into {} table))))))))
 
+(deftest transform-temp-tables-are-skipped-test
+  (mt/when-ee-evailable
+   (let [temp-table   {:name   "mb_transform_temp_table_temp_123"
+                       :schema "public"}
+         normal-table {:name   "orders"
+                       :schema "public"}
+         db-metadata  {:tables #{temp-table normal-table}}]
+     (testing "table-set excludes transform temporary tables when flagged and has transforms feature"
+       (mt/with-premium-features #{:transforms}
+         (is (= #{normal-table}
+                (#'sync-tables/table-set db-metadata)))))
+     (testing "ignroe it if transform feature is disabled"
+       (mt/with-premium-features #{}
+         (is (= #{normal-table temp-table}
+                (#'sync-tables/table-set db-metadata))))))))
+
 (deftest retire-tables-test
   (testing "`retire-tables!` should retire the Table(s) passed to it, not all Tables in the DB -- see #9593"
     (mt/with-temp [:model/Database db {}