diff --git a/resources/migrations/001_update_migrations.yaml b/resources/migrations/001_update_migrations.yaml
index 3000db23459b5dd0a15f2dcaadc24834e2d5b6b6..74de023f826ba32b8866fcb146589b869db20913 100644
--- a/resources/migrations/001_update_migrations.yaml
+++ b/resources/migrations/001_update_migrations.yaml
@@ -6542,14 +6542,6 @@ databaseChangeLog:
- customChange:
class: "metabase.db.custom_migrations.CreateSampleContent"
- - changeSet:
- id: v50.2024-04-09T15:55:23
- author: piranha
- comment: Backfill query_field for native queries
- changes:
- - customChange:
- class: "metabase.db.custom_migrations.BackfillQueryField"
-
- changeSet:
id: v50.2024-04-12T12:33:09
author: piranha
@@ -6569,6 +6561,14 @@ databaseChangeLog:
relativeToChangelogFile: true
rollback: # no rollback necessary, we're not removing the columns yet
+ - changeSet:
+ id: v50.2024-04-18T14:33:19
+ author: piranha
+ comment: Backfill query_field
+ changes:
+ - customChange:
+ class: "metabase.db.custom_migrations.BackfillQueryField"
+
- changeSet:
id: v50.2024-04-19T17:04:04
author: noahmoss
diff --git a/src/metabase/api/dashboard.clj b/src/metabase/api/dashboard.clj
index fc987517da9f1e07e5345d1de7411f859db85648..db22d302fe18de38f359915aabea77fef6d1af2e 100644
--- a/src/metabase/api/dashboard.clj
+++ b/src/metabase/api/dashboard.clj
@@ -541,7 +541,7 @@
(defn- do-update-dashcards!
[dashboard current-cards new-cards]
- (let [{:keys [to-create to-update to-delete]} (u/classify-changes current-cards new-cards)]
+ (let [{:keys [to-create to-update to-delete]} (u/row-diff current-cards new-cards)]
(when (seq to-update)
(update-dashcards! dashboard to-update))
{:deleted-dashcards (when (seq to-delete)
diff --git a/src/metabase/db/custom_migrations.clj b/src/metabase/db/custom_migrations.clj
index 383b6fac9737334d59143d27a0558386209a6d41..fc9958b77065b3dc1ee142c9688a830cbda8d948 100644
--- a/src/metabase/db/custom_migrations.clj
+++ b/src/metabase/db/custom_migrations.clj
@@ -1259,13 +1259,39 @@
(qs/shutdown scheduler)))
(define-migration BackfillQueryField
- (let [update-query-fields! (requiring-resolve 'metabase.native-query-analyzer/update-query-fields-for-card!)
- cards (t2/select :model/Card :id [:in {:from [[:report_card :c]]
- :left-join [[:query_field :f] [:= :f.card_id :c.id]]
- :select [:c.id]
- :where [:and
- [:not :c.archived]
- [:= :c.query_type "native"]
- [:= :f.id nil]]}])]
- (doseq [card cards]
- (update-query-fields! card))))
+ (let [field-ids-for-sql (requiring-resolve 'metabase.native-query-analyzer/field-ids-for-sql)
+ ;; practically #'metabase.models.query-field/field-ids-for-mbql
+ mbql-fields-xf (comp
+ (map #(match %
+ ["field" (id :guard integer?) opts] [id (:source-field opts)]
+ :else nil))
+ cat
+ (remove nil?))
+ field-ids-for-mbql (fn [query]
+ {:direct (->> (tree-seq coll? seq query)
+ (into #{} mbql-fields-xf)
+ not-empty)})
+ cards (t2/select :report_card :id [:in {:from [[:report_card :c]]
+ :left-join [[:query_field :f] [:= :f.card_id :c.id]]
+ :select [:c.id]
+ :where [:and
+ [:not :c.archived]
+ [:= :f.id nil]]}])]
+ (doseq [{card-id :id query :dataset_query :as card} cards]
+ (let [query (json/parse-string query true)
+ {:keys [direct indirect] :as res} (if (= (:type query) "native")
+ (field-ids-for-sql query)
+ (field-ids-for-mbql query))
+ id->record (fn [direct? field-id]
+ {:card_id card-id
+ :field_id field-id
+ :direct_reference direct?})
+ records (concat
+ (map (partial id->record true) direct)
+ (map (partial id->record false) indirect))
+ known (set
+ (when (seq records)
+ (t2/select-fn-set :id :metabase_field :id [:in (map :field_id records)])))
+ records (filterv (comp known :field_id) records)]
+ (when (seq records)
+ (t2/insert! :query_field records))))))
diff --git a/src/metabase/lib/util.cljc b/src/metabase/lib/util.cljc
index 783b1e90b0fdddd890bc9da767daee6cabd0b2e3..0985e0a80afafc4459c0429ac04fcb048e768e75 100644
--- a/src/metabase/lib/util.cljc
+++ b/src/metabase/lib/util.cljc
@@ -584,7 +584,10 @@
(when (#{:mbql/query :query :native :internal} query-type)
query-type))))
-(mu/defn referenced-field-ids :- [:maybe [:sequential ::lib.schema.id/field]]
- "Find all the integer field IDs in ``, Which can arbitrarily be anything that is part of MLv2 query schema."
+(mu/defn referenced-field-ids :- [:maybe [:set ::lib.schema.id/field]]
+ "Find all the integer field IDs in `coll`, Which can arbitrarily be anything that is part of MLv2 query schema."
[coll]
- (lib.util.match/match coll [:field _ (id :guard int?)] id))
+ (not-empty
+ (into #{}
+ (comp cat (filter some?))
+ (lib.util.match/match coll [:field opts (id :guard int?)] [id (:source-field opts)]))))
diff --git a/src/metabase/models/card.clj b/src/metabase/models/card.clj
index 72f11d09316f574522493fe9eb8eed0412a4e211..2eb9bbbd72ad5fd6c2ab948be4033ceabf5fc0ee 100644
--- a/src/metabase/models/card.clj
+++ b/src/metabase/models/card.clj
@@ -29,10 +29,10 @@
[metabase.models.permissions :as perms]
[metabase.models.pulse :as pulse]
[metabase.models.query :as query]
+ [metabase.models.query-field :as query-field]
[metabase.models.revision :as revision]
[metabase.models.serialization :as serdes]
[metabase.moderation :as moderation]
- [metabase.native-query-analyzer :as query-analyzer]
[metabase.public-settings :as public-settings]
[metabase.public-settings.premium-features
:as premium-features
@@ -527,7 +527,7 @@
(log/info "Card references Fields in params:" field-ids)
(field-values/update-field-values-for-on-demand-dbs! field-ids))
(parameter-card/upsert-or-delete-from-parameters! "card" (:id card) (:parameters card))
- (query-analyzer/update-query-fields-for-card! card)))
+ (query-field/update-query-fields-for-card! card)))
(t2/define-before-update :model/Card
[{:keys [verified-result-metadata?] :as card}]
@@ -555,7 +555,7 @@
[card]
(u/prog1 card
(when (contains? (t2/changes card) :dataset_query)
- (query-analyzer/update-query-fields-for-card! card))))
+ (query-field/update-query-fields-for-card! card))))
;; Cards don't normally get deleted (they get archived instead) so this mostly affects tests
(t2/define-before-delete :model/Card
diff --git a/src/metabase/models/dashboard.clj b/src/metabase/models/dashboard.clj
index 827e63c03f76b74169649f3efddbb511fcaf5990..7ee1091dd95883fb2824b8d998b6dc054aa0bb06 100644
--- a/src/metabase/models/dashboard.clj
+++ b/src/metabase/models/dashboard.clj
@@ -268,7 +268,7 @@
:model/DashboardCard
:dashboard_id dashboard-id)
id->current-card (zipmap (map :id current-cards) current-cards)
- {:keys [to-create to-update to-delete]} (u/classify-changes current-cards serialized-cards)]
+ {:keys [to-create to-update to-delete]} (u/row-diff current-cards serialized-cards)]
(when (seq to-delete)
(dashboard-card/delete-dashboard-cards! (map :id to-delete)))
(when (seq to-create)
diff --git a/src/metabase/models/dashboard_tab.clj b/src/metabase/models/dashboard_tab.clj
index 1ec24497a4e1da2d3ff50d4e94ae51213125d550..91a07c59ca8311b6fb9860bb20b0da5b96893d74 100644
--- a/src/metabase/models/dashboard_tab.clj
+++ b/src/metabase/models/dashboard_tab.clj
@@ -118,7 +118,7 @@
[dashboard-id current-tabs new-tabs]
(let [{:keys [to-create
to-update
- to-delete]} (u/classify-changes current-tabs new-tabs)
+ to-delete]} (u/row-diff current-tabs new-tabs)
to-delete-ids (map :id to-delete)
_ (when-let [to-delete-ids (seq to-delete-ids)]
(delete-tabs! to-delete-ids))
diff --git a/src/metabase/models/field_usage.clj b/src/metabase/models/field_usage.clj
index c2819a6dc7d300cc58b819374eb1c507d995a815..1a4aa78c222c080810b58b910bf6b1719e2c2da7 100644
--- a/src/metabase/models/field_usage.clj
+++ b/src/metabase/models/field_usage.clj
@@ -56,8 +56,8 @@
:breakout_binning_num_bins (:num-bins binning-option)}))))
(defn- expression->field-usage
- [expresison-clause]
- (when-let [field-ids (seq (lib.util/referenced-field-ids expresison-clause))]
+ [expression-clause]
+ (when-let [field-ids (seq (lib.util/referenced-field-ids expression-clause))]
(for [field-id field-ids]
{:field_id field-id
:used_in :expression})))
@@ -66,7 +66,7 @@
(defn- join->field-usages
[query join]
- (let [join-query (fetch-source-query/resolve-source-cards (assoc query :stages (get join :stages)))]
+ (let [join-query (fetch-source-query/resolve-source-cards (assoc query :stages (:stages join)))]
;; treat the source query as a :mbql/query
(pmbql->field-usages join-query)))
diff --git a/src/metabase/models/query_field.clj b/src/metabase/models/query_field.clj
index 0d243ba1ad39a396d0cb6e1926e39f80d237b1e1..c40c2b01e8b691c1b9b3523dfe85aca4535ae94f 100644
--- a/src/metabase/models/query_field.clj
+++ b/src/metabase/models/query_field.clj
@@ -1,5 +1,9 @@
(ns metabase.models.query-field
(:require
+ [metabase.legacy-mbql.util :as mbql.u]
+ [metabase.native-query-analyzer :as query-analyzer]
+ [metabase.util :as u]
+ [metabase.util.log :as log]
[methodical.core :as methodical]
[toucan2.core :as t2]))
@@ -7,3 +11,61 @@
(doto :model/QueryField
(derive :metabase/model))
+
+;;; Updating QueryField from card
+
+(defn- field-ids-for-mbql
+ "Find out ids of all fields used in a query. Conforms to the same protocol as [[query-analyzer/field-ids-for-sql]],
+ so returns `{:direct #{...int ids}}` map.
+
+ Does not track wildcards for queries rendered as tables afterwards."
+ [query]
+ {:direct (mbql.u/referenced-field-ids query)})
+
+(defn update-query-fields-for-card!
+ "Clears QueryFields associated with this card and creates fresh, up-to-date-ones.
+
+ Any card is accepted, but this functionality only works for ones with a native query.
+
+ If you're invoking this from a test, be sure to turn on [[*parse-queries-in-test?*]].
+
+ Returns `nil` (and logs the error) if there was a parse error."
+ [{card-id :id, query :dataset_query}]
+ (when query
+ (try
+ (let [{:keys [direct indirect] :as res} (case (:type query)
+ :native (try
+ (query-analyzer/field-ids-for-sql query)
+ (catch Exception e
+ (log/error e "Error parsing SQL" query)))
+ :query (field-ids-for-mbql query)
+ nil nil)
+ id->row (fn [direct? field-id]
+ {:card_id card-id
+ :field_id field-id
+ :direct_reference direct?})
+ query-field-rows (concat
+ (map (partial id->row true) direct)
+ (map (partial id->row false) indirect))]
+ ;; when response is `nil`, it's a disabled parser, not unknown columns
+ (when (some? res)
+ (t2/with-transaction [_conn]
+ (let [existing (t2/select :model/QueryField :card_id card-id)
+ {:keys [to-update
+ to-create
+ to-delete]} (u/row-diff existing query-field-rows
+ {:id-fn :field_id
+ :cleanup #(dissoc % :id :card_id :field_id)})]
+ (when (seq to-delete)
+ ;; this delete seems to break transaction (implicit commit or something) on MySQL, and this `diff`
+ ;; algo drops its frequency by a lot - which should help with transactions affecting each other a
+ ;; lot. Parallel tests in `metabase.models.query.permissions-test` were breaking when delete was
+ ;; executed unconditionally on every query change.
+ (t2/delete! :model/QueryField :card_id card-id :field_id [:in (map :field_id to-delete)]))
+ (when (seq to-create)
+ (t2/insert! :model/QueryField to-create))
+ (doseq [item to-update]
+ (t2/update! :model/QueryField {:card_id card-id :field_id (:field_id item)}
+ (select-keys item [:direct_reference])))))))
+ (catch Exception e
+ (log/error e "Error parsing native query")))))
diff --git a/src/metabase/native_query_analyzer.clj b/src/metabase/native_query_analyzer.clj
index 81696d5fcacd9b2ab2e7cf9ca3b1ca251e62a976..e43b69011995bc55c8dd06487ea5bb5bde228002 100644
--- a/src/metabase/native_query_analyzer.clj
+++ b/src/metabase/native_query_analyzer.clj
@@ -17,7 +17,6 @@
[metabase.native-query-analyzer.parameter-substitution :as nqa.sub]
[metabase.public-settings :as public-settings]
[metabase.util :as u]
- [metabase.util.log :as log]
[toucan2.core :as t2]))
(def ^:dynamic *parse-queries-in-test?*
@@ -94,53 +93,25 @@
;; limit to the named tables
(seq table-wildcards) (active-fields-from-tables table-wildcards))))
-(defn- field-ids-for-card
+(defn field-ids-for-sql
"Returns a `{:direct #{...} :indirect #{...}}` map with field IDs that (may) be referenced in the given cards's
query. Errs on the side of optimism: i.e., it may return fields that are *not* in the query, and is unlikely to fail
to return fields that are in the query.
Direct references are columns that are named in the query; indirect ones are from wildcards. If a field could be
both direct and indirect, it will *only* show up in the `:direct` set."
- [card]
- (let [query (:dataset_query card)
- db-id (:database query)
- sql-string (:query (nqa.sub/replace-tags query))
- parsed-query (macaw/query->components (macaw/parsed-query sql-string))
- direct-ids (direct-field-ids-for-query parsed-query db-id)
- indirect-ids (set/difference
- (indirect-field-ids-for-query parsed-query db-id)
- direct-ids)]
- {:direct direct-ids
- :indirect indirect-ids}))
-
-(defn update-query-fields-for-card!
- "Clears QueryFields associated with this card and creates fresh, up-to-date-ones.
-
- Any card is accepted, but this functionality only works for ones with a native query.
-
- If you're invoking this from a test, be sure to turn on [[*parse-queries-in-test?*]].
-
- Returns `nil` (and logs the error) if there was a parse error."
- [{card-id :id, query :dataset_query :as card}]
+ [query]
(when (and (active?)
- (= :native (:type query)))
- (try
- (let [{:keys [direct indirect]} (field-ids-for-card card)
- id->record (fn [direct? field-id]
- {:card_id card-id
- :field_id field-id
- :direct_reference direct?})
- query-field-records (concat
- (map (partial id->record true) direct)
- (map (partial id->record false) indirect))]
- ;; This feels inefficient at first glance, but the number of records should be quite small and doing some sort
- ;; of upsert-or-delete would involve comparisons in Clojure-land that are more expensive than just "killing and
- ;; filling" the records.
- (t2/with-transaction [_conn]
- (t2/delete! :model/QueryField :card_id card-id)
- (t2/insert! :model/QueryField query-field-records)))
- (catch Exception e
- (log/error e "Error parsing native query")))))
+ (:native query))
+ (let [db-id (:database query)
+ sql-string (:query (nqa.sub/replace-tags query))
+ parsed-query (macaw/query->components (macaw/parsed-query sql-string))
+ direct-ids (direct-field-ids-for-query parsed-query db-id)
+ indirect-ids (set/difference
+ (indirect-field-ids-for-query parsed-query db-id)
+ direct-ids)]
+ {:direct direct-ids
+ :indirect indirect-ids})))
;; TODO: does not support template tags
(defn replace-names
diff --git a/src/metabase/util.cljc b/src/metabase/util.cljc
index 2f1234ab9131dc1fd4a9287f4be61044e17c990d..0d13b09e3de8a32434b86c3c6abaf63521cd013c 100644
--- a/src/metabase/util.cljc
+++ b/src/metabase/util.cljc
@@ -841,20 +841,38 @@
{:kvs kvs})))
ret))))
-(defn classify-changes
+(defn row-diff
"Given 2 lists of seq maps of changes, where each map an has an `id` key,
return a map of 3 keys: `:to-create`, `:to-update`, `:to-delete`.
Where:
- :to-create is a list of maps that ids in `new-items`
- :to-update is a list of maps that has ids in both `current-items` and `new-items`
- :to delete is a list of maps that has ids only in `current-items`"
- [current-items new-items]
- (let [[delete-ids create-ids update-ids] (diff (set (map :id current-items))
- (set (map :id new-items)))]
- {:to-create (when (seq create-ids) (filter #(create-ids (:id %)) new-items))
- :to-delete (when (seq delete-ids) (filter #(delete-ids (:id %)) current-items))
- :to-update (when (seq update-ids) (filter #(update-ids (:id %)) new-items))}))
+ - `:to-create` is a list of maps that ids in `new-rows`
+ - `:to-delete` is a list of maps that has ids only in `current-rows`
+ - `:to-skip` is a list of identical maps that has ids in both lists
+ - `:to-update` is a list of different maps that has ids in both lists
+
+ Optional arguments:
+ - `id-fn` - function to get row-matching identifiers
+ - `cleanup` - function to get rows into a comparable state
+ "
+ [current-rows new-rows & {:keys [id-fn cleanup]
+ :or {id-fn :id
+ cleanup identity}}]
+ (let [[delete-ids
+ create-ids
+ update-ids] (diff (set (map id-fn current-rows))
+ (set (map id-fn new-rows)))
+ known-map (m/index-by id-fn current-rows)
+ {to-update false
+ to-skip true} (when (seq update-ids)
+ (group-by (fn [x]
+ (let [y (get known-map (id-fn x))]
+ (= (cleanup x) (cleanup y))))
+ (filter #(update-ids (id-fn %)) new-rows)))]
+ {:to-create (when (seq create-ids) (filter #(create-ids (id-fn %)) new-rows))
+ :to-delete (when (seq delete-ids) (filter #(delete-ids (id-fn %)) current-rows))
+ :to-update to-update
+ :to-skip to-skip}))
(defn empty-or-distinct?
"True if collection `xs` is either [[empty?]] or all values are [[distinct?]]."
diff --git a/test/metabase/db/custom_migrations_test.clj b/test/metabase/db/custom_migrations_test.clj
index b3f1f0911797a059ee2317cd3d4ec590f153fcb1..41f9c157cfc69873c61242242ef8f5ec180261ee 100644
--- a/test/metabase/db/custom_migrations_test.clj
+++ b/test/metabase/db/custom_migrations_test.clj
@@ -1675,16 +1675,22 @@
(is (nil? (:cache_field_values_schedule db))))))))))
(deftest backfill-query-field-test
- (impl/test-migrations "v50.2024-04-09T15:55:23" [migrate!]
+ (impl/test-migrations "v50.2024-04-18T14:33:19" [migrate!]
(let [user-id (:id (new-instance-with-default :core_user))
- ;; it is already `false`, but binding it anyway to indicate it's important
- card-id (binding [query-analyzer/*parse-queries-in-test?* false]
- (:id (new-instance-with-default
- :report_card
- {:creator_id user-id
- :database_id (mt/id)
- :query_type "native"
- :dataset_query (json/generate-string (mt/native-query {:query "SELECT id FROM venues"}))})))
+ c1-id (:id (new-instance-with-default
+ :report_card
+ {:creator_id user-id
+ :database_id (mt/id)
+ :query_type "native"
+ :dataset_query (json/generate-string
+ (mt/native-query {:query "SELECT id FROM venues"}))}))
+ c2-id (:id (new-instance-with-default
+ :report_card
+ {:creator_id user-id
+ :database_id (mt/id)
+ :query_type "query"
+ :dataset_query (json/generate-string
+ (mt/mbql-query venues {:aggregation [[:distinct $name]]}))}))
archived-id (binding [query-analyzer/*parse-queries-in-test?* false]
(:id (new-instance-with-default
:report_card
@@ -1692,15 +1698,20 @@
:creator_id user-id
:database_id (mt/id)
:query_type "native"
- :dataset_query (json/generate-string (mt/native-query {:query "SELECT id FROM venues"}))})))
+ :dataset_query (json/generate-string
+ (mt/native-query {:query "SELECT id FROM venues"}))})))
;; (first (vals %)) are necessary since h2 generates :count(id) as name for column
get-count #(t2/select-one-fn (comp first vals) [:model/QueryField [[:count :id]]] :card_id %)]
(testing "QueryField is empty - queries weren't analyzed"
- (is (zero? (get-count card-id)))
+ ;; they were not analyzed since `new-instance-with-default` inserts directly into the table, omitting model
+ ;; hooks
+ (is (zero? (get-count c1-id)))
+ (is (zero? (get-count c2-id)))
(is (zero? (get-count archived-id))))
(binding [query-analyzer/*parse-queries-in-test?* true]
(migrate!))
(testing "QueryField is filled now"
- (is (pos? (get-count card-id)))
+ (is (pos? (get-count c1-id)))
+ (is (pos? (get-count c2-id)))
(testing "but not for archived card"
(is (zero? (get-count archived-id))))))))
diff --git a/test/metabase/models/query_field_test.clj b/test/metabase/models/query_field_test.clj
index af97442b5549ec4d52a8b3c3213b334dd576cf3c..9cffad1b2dae1e60f9f7b7f7c9421bd575b55837 100644
--- a/test/metabase/models/query_field_test.clj
+++ b/test/metabase/models/query_field_test.clj
@@ -2,6 +2,8 @@
(:require
[clojure.set :as set]
[clojure.test :refer :all]
+ [metabase.models :refer [Card]]
+ [metabase.models.query-field :as query-field]
[metabase.native-query-analyzer :as query-analyzer]
[metabase.test :as mt]
[toucan2.core :as t2]
@@ -129,3 +131,24 @@
;; subset since it also includes the PKs/FKs
(is (set/subset? #{total-qf tax-qf}
(t2/select-fn-set qf->map :model/QueryField :card_id card-id :direct_reference true))))))))
+
+(deftest parse-mbql-test
+ (testing "Parsing MBQL query returns correct used fields"
+ (mt/with-temp [Card c1 {:dataset_query (mt/mbql-query venues
+ {:aggregation [[:distinct $name]
+ [:distinct $price]]
+ :limit 5})}
+ Card c2 {:dataset_query {:query {:source-table (str "card__" (:id c1))}
+ :database (:id (mt/db))
+ :type :query}}
+ Card c3 {:dataset_query (mt/mbql-query checkins
+ {:joins [{:source-table (str "card__" (:id c2))
+ :alias "Venues"
+ :condition [:= $checkins.venue_id $venues.id]}]})}]
+ (mt/$ids
+ (is (= {:direct #{%venues.name %venues.price}}
+ (#'query-field/field-ids-for-mbql (:dataset_query c1))))
+ (is (= {:direct nil}
+ (#'query-field/field-ids-for-mbql (:dataset_query c2))))
+ (is (= {:direct #{%venues.id %checkins.venue_id}}
+ (#'query-field/field-ids-for-mbql (:dataset_query c3))))))))
diff --git a/test/metabase/pulse/pulse_integration_test.clj b/test/metabase/pulse/pulse_integration_test.clj
index 52e212caa64520840da88dd104350d3f7b83b332..4dea84c50335fee40ffe171dd7565ae228a9d0ae 100644
--- a/test/metabase/pulse/pulse_integration_test.clj
+++ b/test/metabase/pulse/pulse_integration_test.clj
@@ -292,7 +292,7 @@
[date-str n]
(format
"WITH T AS (SELECT CAST('%s' AS TIMESTAMP) AS example_timestamp),
- SAMPLE AS (SELECT T.example_timestamp AS full_datetime_utc,
+ \"SAMPLE\" AS (SELECT T.example_timestamp AS full_datetime_utc,
T.example_timestamp AT TIME ZONE 'US/Pacific' AS full_datetime_pacific,
CAST(T.example_timestamp AS TIMESTAMP) AS example_timestamp,
CAST(T.example_timestamp AS TIMESTAMP WITH TIME ZONE) AS example_timestamp_with_time_zone,
@@ -308,7 +308,7 @@
EXTRACT(SECOND FROM T.example_timestamp) AS example_second
FROM T)
SELECT *
- FROM SAMPLE
+ FROM \"SAMPLE\"
CROSS JOIN
generate_series(1, %s);"
date-str n))
diff --git a/test/metabase/util_test.cljc b/test/metabase/util_test.cljc
index 4a7eac5fb04b1bcd5c20b1339eb8051b9364d8b3..2f5532631e210a6fc8453d45d8b6189dc0a22050 100644
--- a/test/metabase/util_test.cljc
+++ b/test/metabase/util_test.cljc
@@ -426,14 +426,27 @@
(is (= {:m true}
(meta (u/assoc-default ^:m {:x 0} :y 1 :z 2 :a nil))))))
-(deftest ^:parallel classify-changes-test
+(deftest ^:parallel row-diff-test
(testing "classify correctly"
- (is (= {:to-update [{:id 2 :name "c3"} {:id 4 :name "c4"}]
+ (is (= {:to-update [{:id 2 :name "c3"}]
:to-delete [{:id 1 :name "c1"} {:id 3 :name "c3"}]
- :to-create [{:id -1 :name "-c1"}]}
- (u/classify-changes
- [{:id 1 :name "c1"} {:id 2 :name "c2"} {:id 3 :name "c3"} {:id 4 :name "c4"}]
- [{:id -1 :name "-c1"} {:id 2 :name "c3"} {:id 4 :name "c4"}])))))
+ :to-create [{:id -1 :name "-c1"}]
+ :to-skip [{:id 4 :name "c4"}]}
+ (u/row-diff
+ [{:id 1 :name "c1"} {:id 2 :name "c2"} {:id 3 :name "c3"} {:id 4 :name "c4"}]
+ [{:id -1 :name "-c1"} {:id 2 :name "c3"} {:id 4 :name "c4"}])))
+ (is (= {:to-skip [{:god_id 10, :name "Zeus", :job "God of Thunder"}]
+ :to-delete [{:id 2, :god_id 20, :name "Odin", :job "God of Thunder"}]
+ :to-update [{:god_id 30, :name "Osiris", :job "God of Afterlife"}]
+ :to-create [{:god_id 40, :name "Perun", :job "God of Thunder"}]}
+ (u/row-diff [{:id 1 :god_id 10 :name "Zeus" :job "God of Thunder"}
+ {:id 2 :god_id 20 :name "Odin" :job "God of Thunder"}
+ {:id 3 :god_id 30 :name "Osiris" :job "God of Fertility"}]
+ [{:god_id 10 :name "Zeus" :job "God of Thunder"}
+ {:god_id 30 :name "Osiris" :job "God of Afterlife"}
+ {:god_id 40 :name "Perun" :job "God of Thunder"}]
+ {:id-fn :god_id
+ :cleanup #(dissoc % :id :god_id)})))))
(deftest ^:parallel empty-or-distinct?-test
(are [xs expected] (= expected