Don't format non-temporal columns as datetimes in XLSX exports (#21048)

* Don't format non-temporal columns as datetimes in XLSX exports

* Test fixes 

* Oops uncomment something

src/metabase/query_processor/streaming/xlsx.clj

@@ -204,15 +204,16 @@
 (defn- format-settings->format-strings
   "Returns a vector of format strings for a datetime column or number column, corresponding
   to the provided format settings."
-  [format-settings semantic-type]
+  [format-settings {semantic-type :semantic_type, effective-type :effective_type, :as _col}]
   (u/one-or-many
    (cond
      ;; Primary key or foreign key
      (isa? semantic-type :Relation/*)
      "0"
 
-     (or (some #(contains? datetime-setting-keys %) (keys format-settings))
-         (isa? semantic-type :type/Temporal))
+     (and (or (some #(contains? datetime-setting-keys %) (keys format-settings))
+              (isa? semantic-type :type/Temporal))
+          (isa? effective-type :type/Temporal))
      (datetime-format-string format-settings)
 
      (or (some #(contains? number-setting-keys %) (keys format-settings))
@@ -264,8 +265,7 @@
                                    {::mb.viz/column-name (:name col)})
                    id-or-name    (first (vals settings-key))
                    settings      (get col-settings settings-key)
-                   semantic-type (:semantic_type col)
-                   format-strings (format-settings->format-strings settings semantic-type)]
+                   format-strings (format-settings->format-strings settings col)]
                (when (seq format-strings)
                  {id-or-name
                   (map

test/metabase/query_processor/streaming/xlsx_test.clj

@@ -20,7 +20,8 @@
    (format-string format-settings nil))
 
   ([format-settings semantic-type]
-   (let [format-strings (@#'xlsx/format-settings->format-strings format-settings semantic-type)]
+   (let [format-strings (@#'xlsx/format-settings->format-strings format-settings {:semantic_type  semantic-type
+                                                                                  :effective_type :type/Temporal})]
      ;; If only one format string is returned (for datetimes) or both format strings
      ;; are equal, just return a single value to make tests more readable.
      (cond
@@ -241,8 +242,9 @@
 (defn parse-cell-content
   "Parses an XLSX sheet and returns the raw data in each row"
   [sheet]
-  (for [row (spreadsheet/into-seq sheet)]
-    (map spreadsheet/read-cell row)))
+  (mapv (fn [row]
+          (mapv spreadsheet/read-cell row))
+        (spreadsheet/into-seq sheet)))
 
 (defn parse-xlsx-results
   "Given a byte array representing an XLSX document, parses the query result sheet using the provided `parse-fn`"
@@ -293,7 +295,7 @@
                                 [[1.23]]
                                 parse-format-strings))))
     (is (= ["yyyy.m.d, h:mm:ss am/pm"]
-           (second (xlsx-export [{:id 0, :name "Col"}]
+           (second (xlsx-export [{:id 0, :name "Col", :effective_type :type/Temporal}]
                                 {::mb.viz/column-settings {{::mb.viz/field-id 0}
                                                            {::mb.viz/date-style "YYYY/M/D",
                                                             ::mb.viz/date-separator ".",
@@ -499,3 +501,19 @@
       (i/finish! results-writer {:row_count 0})
       ;; No additional files should exist in the temp directory
       (is (= expected-poifiles-count (count (file-seq poifiles-directory)))))))
+
+(deftest dont-format-non-temporal-columns-as-temporal-columns-test
+  (testing "Don't format columns with temporal semantic type as datetime unless they're actually datetimes (#18729)"
+    (mt/dataset sample-dataset
+      (is (= [["CREATED_AT"]
+              [1.0]
+              [2.0]]
+             (xlsx-export [{:id             0
+                            :semantic_type  :type/CreationTimestamp
+                            :unit           :month-of-year
+                            :name           "CREATED_AT"
+                            :effective_type :type/Integer
+                            :base_type      :type/Integer}]
+                          {}
+                          [[1]
+                           [2]]))))))