diff --git a/backend/mbql/src/metabase/mbql/util.clj b/backend/mbql/src/metabase/mbql/util.clj index 472697db82dbf0c6260384d327357b4a67ab176b..3fa6c4848cdd2a46a216d6cd5386ff4e733067c5 100644 --- a/backend/mbql/src/metabase/mbql/util.clj +++ b/backend/mbql/src/metabase/mbql/util.clj @@ -442,7 +442,7 @@ "Unwrap a Field `clause`, if it's something that can be unwrapped (i.e. something that is, or wraps, a `:field-id` or `:field-literal`). Otherwise return `clause` as-is." [clause] - (if (is-clause? #{:field-id :fk-> :field-literal :datetime-field :binning-strategy} clause) + (if (is-clause? #{:field-id :fk-> :field-literal :datetime-field :binning-strategy :joined-field} clause) (unwrap-field-clause clause) clause)) @@ -453,10 +453,9 @@ (field-clause->id-or-literal [:datetime-field [:field-id 100] ...]) ; -> 100 (field-clause->id-or-literal [:field-id 100]) ; -> 100 - For expressions (or any other clauses) this returns the clause as-is, so as to facilitate the primary use case of - comparing Field clauses." + For expressions returns the expression name." [clause :- mbql.s/Field] - (second (unwrap-field-clause clause))) + (second (maybe-unwrap-field-clause clause))) (s/defn add-order-by-clause :- mbql.s/MBQLQuery "Add a new `:order-by` clause to an MBQL `inner-query`. If the new order-by clause references a Field that is diff --git a/backend/mbql/test/metabase/mbql/util_test.clj b/backend/mbql/test/metabase/mbql/util_test.clj index 44737e9e3c7ce9dec2ce7e462709e41027c78387..d6d4d5ae0ebbaaf51abe4d7bfa353795fbe2df62 100644 --- a/backend/mbql/test/metabase/mbql/util_test.clj +++ b/backend/mbql/test/metabase/mbql/util_test.clj @@ -1059,3 +1059,15 @@ :query {:source-query {:expressions {:two [:+ 1 1]} :source-table 1}}} "two")) + +(expect + 1 + (mbql.u/field-clause->id-or-literal [:field-id 1])) + +(expect + "foo" + (mbql.u/field-clause->id-or-literal [:field-literal "foo" :type/Integer])) + +(expect + "foo" + (mbql.u/field-clause->id-or-literal [:expression "foo"])) diff --git a/src/metabase/cmd.clj b/src/metabase/cmd.clj index b1c56553cfeba19c862896efbd4c1420fa09a989..c01468aaf795e7bed976993a5ccc6f4c4389e389 100644 --- a/src/metabase/cmd.clj +++ b/src/metabase/cmd.clj @@ -37,11 +37,14 @@ ((resolve 'metabase.cmd.load-from-h2/load-from-h2!) h2-connection-string)))) (defn ^:command dump-to-h2 - "Transfer data from existing database to newly created H2 DB." - [h2-filename] + "Transfer data from existing database to newly created H2 DB with specified filename. + + Target H2 file is deleted before dump, unless the --keep-existing flag is given." + [h2-filename & opts] (classloader/require 'metabase.cmd.dump-to-h2) (binding [mdb/*disable-data-migrations* true] - (let [return-code ((resolve 'metabase.cmd.dump-to-h2/dump-to-h2!) h2-filename)] + (let [keep-existing (boolean (some #{"--keep-existing"} opts)) + return-code ((resolve 'metabase.cmd.dump-to-h2/dump-to-h2!) h2-filename keep-existing)] (when (pos-int? return-code) (System/exit return-code))))) diff --git a/src/metabase/cmd/dump_to_h2.clj b/src/metabase/cmd/dump_to_h2.clj index 4e06aeb2e5e9181b34694da10eab68bde32b03bf..1fb7369438c8bc7db7737b5719f91f1800e8d2a0 100644 --- a/src/metabase/cmd/dump_to_h2.clj +++ b/src/metabase/cmd/dump_to_h2.clj @@ -78,7 +78,7 @@ (println-ok)) (defn- load-data! [target-db-conn] - (println "Source db:" (mdb/jdbc-spec)) + (println "Source db:" (dissoc (mdb/jdbc-spec) :password)) (jdbc/with-db-connection [db-conn (mdb/jdbc-spec)] (doseq [{table-name :table, :as e} entities :let [rows (jdbc/query db-conn [(str "SELECT * FROM " (name table-name))])] @@ -91,16 +91,19 @@ ;;; --------------------------------------------------- Public Fns --------------------------------------------------- (defn dump-to-h2! - "Transfer data from existing database specified by connection string - to the H2 DB specified by env vars. Intended as a tool for migrating - from one instance to another using H2 as serialization target. + "Transfer data from existing database specified by connection string to the H2 DB specified by env vars. Intended as a + tool for migrating from one instance to another using H2 as serialization target. - Defaults to using `@metabase.db/db-file` as the connection string." - [h2-filename] + Defaults to using `@metabase.db/db-file` as the connection string. + + Target H2 DB will be deleted if it exists, unless `keep-existing` is truthy." + [h2-filename keep-existing] (let [h2-filename (or h2-filename "metabase_dump.h2")] - (println "Dumping to" h2-filename) - (doseq [filename [h2-filename (str h2-filename ".mv.db")]] - (when (.exists (io/file filename)) + (println "Dumping to " h2-filename) + (doseq [filename [h2-filename + (str h2-filename ".mv.db")]] + (when (and (.exists (io/file filename)) + (not keep-existing)) (io/delete-file filename) (println (u/format-color 'red (trs "Output H2 database already exists: %s, removing.") filename)))) diff --git a/src/metabase/query_processor/middleware/add_dimension_projections.clj b/src/metabase/query_processor/middleware/add_dimension_projections.clj index 2895097e56ef191f5eeac60ce1474618ca473ed3..7299390dfaf1c8932744f94811bb55586a87255e 100644 --- a/src/metabase/query_processor/middleware/add_dimension_projections.clj +++ b/src/metabase/query_processor/middleware/add_dimension_projections.clj @@ -87,19 +87,22 @@ the newly remapped column. This should order by the text of the remapped column vs. the id of the source column before the remapping" [field->remapped-col :- {mbql.s/field-id, mbql.s/fk->}, order-by-clauses :- [mbql.s/OrderBy]] - (vec - (for [[direction field, :as order-by-clause] order-by-clauses] - (if-let [remapped-col (get field->remapped-col field)] - [direction remapped-col] - order-by-clause)))) + (->> (for [[direction field, :as order-by-clause] order-by-clauses] + (if-let [remapped-col (get field->remapped-col field)] + [direction remapped-col] + order-by-clause)) + distinct + vec)) (defn- update-remapped-breakout [field->remapped-col breakout-clause] - (vec (mapcat (fn [field] - (if-let [remapped-col (get field->remapped-col field)] - [remapped-col field] - [field])) - breakout-clause))) + (->> breakout-clause + (mapcat (fn [field] + (if-let [remapped-col (get field->remapped-col field)] + [remapped-col field] + [field]))) + distinct + vec)) (s/defn ^:private add-fk-remaps :- [(s/one (s/maybe [ExternalRemappingDimension]) "external remapping dimensions") (s/one mbql.s/Query "query")] @@ -114,7 +117,11 @@ ;; fetch remapping column pairs if any exist... (if-let [remap-col-tuples (seq (create-remap-col-tuples (concat fields breakout)))] ;; if they do, update `:fields`, `:order-by` and `:breakout` clauses accordingly and add to the query - (let [new-fields (into fields (map second) remap-col-tuples) + (let [new-fields (->> remap-col-tuples + (map second) + (concat fields) + distinct + vec) ;; make a map of field-id-clause -> fk-clause from the tuples field->remapped-col (into {} (for [[field-clause fk-clause] remap-col-tuples] [field-clause fk-clause])) diff --git a/test/metabase/query_processor/middleware/add_dimension_projections_test.clj b/test/metabase/query_processor/middleware/add_dimension_projections_test.clj index dfb89cc90001294c08c1457c37a0c3a540744afd..8a30cbba62a16dc2029ce4ca2e050babf9de1f3a 100644 --- a/test/metabase/query_processor/middleware/add_dimension_projections_test.clj +++ b/test/metabase/query_processor/middleware/add_dimension_projections_test.clj @@ -50,9 +50,20 @@ (testing "make sure FK remaps add an entry for the FK field to `:fields`, and returns a pair of [dimension-info updated-query]" (is (= [[remapped-field] (update-in example-query [:query :fields] - conj [:fk-> [:field-id (mt/id :venues :category_id)] [:field-id (mt/id :categories :name)]])] + conj [:fk-> [:field-id (mt/id :venues :category_id)] + [:field-id (mt/id :categories :name)]])] (#'add-dim-projections/add-fk-remaps example-query)))) + (testing "make sure we don't duplicate remappings" + (is (= [[remapped-field] + (update-in example-query [:query :fields] + conj [:fk-> [:field-id (mt/id :venues :category_id)] + [:field-id (mt/id :categories :name)]])] + (#'add-dim-projections/add-fk-remaps + (update-in example-query [:query :fields] + conj [:fk-> [:field-id (mt/id :venues :category_id)] + [:field-id (mt/id :categories :name)]]))))) + (testing "adding FK remaps should replace any existing order-bys for a field with order bys for the FK remapping Field" (is (= [[remapped-field] (-> example-query