diff --git a/src/metabase/query_processor/streaming.clj b/src/metabase/query_processor/streaming.clj index 4e40dd7ed73a1077ee76480e21e04f065dfd7e40..8ed184b00b9e5fbdb18d70234bc535d2475752a6 100644 --- a/src/metabase/query_processor/streaming.clj +++ b/src/metabase/query_processor/streaming.clj @@ -34,13 +34,24 @@ cols (mbql.u/uniquify-names (map :name cols)))) +(defn- validate-table-columms + "Validate that all of the field refs in `table-columns` correspond to actual columns in `cols`, if `cols` contains + field refs. Returns `nil` if any do not, so that we fall back to using `cols` directly for the export (#19465). + Otherwise returns `table-columns`." + [table-columns cols] + (let [col-field-refs (set (remove nil? (map :field_ref cols)))] + ;; If there are no field refs in `cols` (e.g. for native queries), we should use `table-columns` as-is + (when (or (empty? col-field-refs) + (every? (fn [table-col] (col-field-refs (::mb.viz/table-column-field-ref table-col))) table-columns)) + table-columns))) + (defn- export-column-order "For each entry in `table-columns` that is enabled, finds the index of the corresponding entry in `cols` by name or id. If a col has been remapped, uses the index of the new column. The resulting list of indices determines the order of column names and data in exports." [cols table-columns] - (let [table-columns' (or table-columns + (let [table-columns' (or (validate-table-columms table-columns cols) ;; If table-columns is not provided (e.g. for saved cards), we can construct a fake one ;; that retains the original column ordering in `cols` (for [col cols] diff --git a/test/metabase/query_processor/streaming_test.clj b/test/metabase/query_processor/streaming_test.clj index 1650dfc85bbb9e4ff03b350a69a7834dd97f65fe..2d6ed8677c274a393d2248817c3bd8744bb94652 100644 --- a/test/metabase/query_processor/streaming_test.clj +++ b/test/metabase/query_processor/streaming_test.clj @@ -470,12 +470,13 @@ {:id 1, :name "Col2", :remapped_from "Col1", :field_ref ["field" 1 nil]}] [{::mb.viz/table-column-field-ref ["field" 0 nil], ::mb.viz/table-column-enabled true}])))) - (testing "entries in table-columns without corresponding entries in cols are ignored" + (testing "if table-columns contains a column without a corresponding entry in cols, table-columns is ignored and + cols is used as the source of truth for column order (#19465)" (is (= [0] (@#'qp.streaming/export-column-order [{:id 0, :name "Col1" :field_ref [:field 0 nil]}] - [{::mb.viz/table-column-field-ref [:field 0 nil], ::mb.viz/table-column-enabled true} - {::mb.viz/table-column-field-ref [:field 1 nil], ::mb.viz/table-column-enabled true}])))) + [{::mb.viz/table-column-field-ref [:field 1 nil], ::mb.viz/table-column-enabled true} + {::mb.viz/table-column-field-ref [:field 2 nil], ::mb.viz/table-column-enabled true}])))) (testing "if table-columns is nil, original order of cols is used" (is (= [0 1]