Skip to content

Commit

Permalink
Fix replacement of columns in queries referencing a single model (#43)
Browse files Browse the repository at this point in the history
  • Loading branch information
crisptrutski authored May 30, 2024
1 parent ac5244c commit f5745c1
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 5 deletions.
4 changes: 2 additions & 2 deletions src/macaw/core.clj
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
(Specifically, it returns their fully-qualified names as strings, where 'fully-qualified' means 'as referred to in
the query'; this function doesn't do additional inference work to find out a table's schema.)"
[statement]
(collect/query->components statement))
[statement & {:as opts}]
(collect/query->components statement opts))

(defn replace-names
"Given a SQL query, apply the given table, column, and schema renames."
Expand Down
15 changes: 12 additions & 3 deletions src/macaw/util.clj
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
(ns macaw.util)
(ns macaw.util
(:require
[clojure.string :as str]))

(defn group-with
"Generalized `group-by`, where you can supply your own reducing function (instead of usual `conj`).
Expand All @@ -22,17 +24,24 @@
nil
coll))

(defn non-sentinel
"A hack around the fact that we don't (yet) track what columns are exposed by given sentinels."
[s]
(when s
(nil? (str/index-of s "_sentinel_"))))

(defn find-relevant
"Search the given map for the entry corresponding to [[map-key]], considering only the relevant keys.
The relevant keys are obtained by ignoring any suffix of [[ks]] for which [[map-key]] has nil or missing values.
We require that there is at least one relevant key to find a match."
[m map-key ks]
(when map-key
(if (every? map-key ks)
(if (every? (comp non-sentinel map-key) ks)
(find m (select-keys map-key ks))
;; Strip off keys from right-to-left where they are nil, and relax search to only consider these keys.
;; We need at least one non-generate key to remain for the search.
(when-let [ks-prefix (->> ks reverse (drop-while (comp nil? map-key)) reverse seq)]
;; NOTE: we could optimize away calling `non-sentinel` twice in this function, but for now just keeping it simple.
(when-let [ks-prefix (->> ks reverse (drop-while (comp not non-sentinel map-key)) reverse seq)]
(when (not= ks ks-prefix)
(seek (comp #{(select-keys map-key ks-prefix)}
#(select-keys % ks-prefix)
Expand Down
7 changes: 7 additions & 0 deletions test/macaw/core_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -342,3 +342,10 @@
(is (= "SELECT 1"
(m/replace-names "SELECT 1" {:tables {{:schema "public" :table "a"} "aa"}}
{:allow-unused? true}))))

(deftest model-reference-test
(is (= "SELECT subtotal FROM metabase_sentinel_table_154643 LIMIT 3"
(m/replace-names "SELECT total FROM metabase_sentinel_table_154643 LIMIT 3"
{:columns {{:table "orders" :column "total"} "subtotal"}
:tables {{:table "orders"} "purchases"}}
{:allow-unused? true}))))

0 comments on commit f5745c1

Please sign in to comment.