Skip to content
Snippets Groups Projects
Unverified Commit b44f1530 authored by Noah Moss's avatar Noah Moss Committed by GitHub
Browse files

Fix pin map lat/lon column selection when query uses :breakout (#20227)

parent cf247b68
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
src/metabase/api/tiles.clj
+ 32
− 21
View file @ b44f1530
......@@ -7,13 +7,14 @@
[metabase.mbql.normalize :as normalize]
[metabase.mbql.util :as mbql.u]
[metabase.query-processor :as qp]
[metabase.query-processor.util :as qp.util]
[metabase.util :as u]
[metabase.util.i18n :refer [tru]]
[metabase.util.schema :as su]
[schema.core :as s])
(:import java.awt.Color
java.awt.image.BufferedImage
[java.io ByteArrayInputStream ByteArrayOutputStream]
java.io.ByteArrayOutputStream
javax.imageio.ImageIO))
;;; --------------------------------------------------- CONSTANTS ----------------------------------------------------
......@@ -143,7 +144,7 @@
(let [id-or-name' (int-or-string id-or-name)]
[:field id-or-name' (when (string? id-or-name') {:base-type :type/Float})]))
(defn query->tiles-query
(defn- query->tiles-query
"Transform a card's query into a query finding coordinates in a particular region.
- transform native queries into nested mbql queries from that native query
......@@ -151,16 +152,14 @@
- limit query results to `tile-coordinate-limit` number of results
- only select lat and lon fields rather than entire query's fields"
[query {:keys [zoom x y lat-field lon-field]}]
(let [lat-ref (field-ref lat-field)
lon-ref (field-ref lon-field)]
(-> query
native->source-query
(update :query query-with-inside-filter
lat-ref lon-ref
x y zoom)
(assoc-in [:query :fields] [lat-ref lon-ref])
(assoc-in [:query :limit] tile-coordinate-limit)
(assoc :async? false))))
(-> query
native->source-query
(update :query query-with-inside-filter
lat-field lon-field
x y zoom)
(assoc-in [:query :fields] [lat-field lon-field])
(assoc-in [:query :limit] tile-coordinate-limit)
(assoc :async? false)))
;; TODO - this can be reworked to be `defendpoint-async` instead
;;
......@@ -178,26 +177,38 @@
lat-field s/Str
lon-field s/Str
query su/JSONString}
(let [zoom (Integer/parseInt zoom)
x (Integer/parseInt x)
y (Integer/parseInt y)
(let [zoom (Integer/parseInt zoom)
x (Integer/parseInt x)
y (Integer/parseInt y)
lat-field-ref (field-ref lat-field)
lon-field-ref (field-ref lon-field)
query
(normalize/normalize (json/parse-string query keyword))
updated-query (query->tiles-query query {:zoom zoom :x x :y y
:lat-field lat-field
:lon-field lon-field})
:lat-field lat-field-ref
:lon-field lon-field-ref})
{:keys [status], {:keys [rows]} :data, :as result}
{:keys [status], {:keys [rows cols]} :data, :as result}
(qp/process-query-and-save-execution! updated-query
{:executed-by api/*current-user-id*
:context :map-tiles})]
;; manual ring response here. we simply create an inputstream from the byte[] of our image
:context :map-tiles})
lat-key (qp.util/field-ref->key lat-field-ref)
lon-key (qp.util/field-ref->key lon-field-ref)
find-fn (fn [lat-or-lon-key]
(first (keep-indexed
(fn [idx col] (when (= (qp.util/field-ref->key (:field_ref col)) lat-or-lon-key) idx))
cols)))
lat-idx (find-fn lat-key)
lon-idx (find-fn lon-key)
points (for [row rows]
[(nth row lat-idx) (nth row lon-idx)])]
(if (= status :completed)
{:status 200
:headers {"Content-Type" "image/png"}
:body (ByteArrayInputStream. (tile->byte-array (create-tile zoom rows)))}
:body (tile->byte-array (create-tile zoom points))}
(throw (ex-info (tru "Query failed")
;; `result` might be a `core.async` channel or something we're not expecting
(assoc (when (map? result) result) :status-code 400))))))
......
test/metabase/api/tiles_test.clj
+ 38
− 10
View file @ b44f1530
......@@ -46,8 +46,8 @@
:query {:source-table 88
:fields [[:field 562 nil]
[:field 574 nil] ; lat
[:field 576 nil] ; lon
]
[:field 576 nil]] ; lon
:limit 50000}
:type :query}]
(is (= {:database 19
......@@ -58,10 +58,10 @@
:filter [:inside [:field 574 nil] [:field 576 nil]]}
:type :query
:async? false}
(clean (tiles/query->tiles-query query
{:zoom 2 :x 3 :y 1
:lat-field "574"
:lon-field "576"})))))))
(clean (#'tiles/query->tiles-query query
{:zoom 2 :x 3 :y 1
:lat-field [:field 574 nil]
:lon-field [:field 576 nil]})))))))
(testing "native"
(testing "nests the query, selects fields"
(let [query {:type :native
......@@ -78,10 +78,31 @@
:limit 2000}
:type :query
:async? false}
(clean (tiles/query->tiles-query query
{:zoom 2 :x 2 :y 1
:lat-field "latitude"
:lon-field "longitude"})))))))))
(clean (@#'tiles/query->tiles-query query
{:zoom 2 :x 2 :y 1
:lat-field [:field "latitude" {:base-type :type/Float}]
:lon-field [:field "longitude" {:base-type :type/Float}]})))))))))
(deftest breakout-query-test
(testing "the appropriate lat/lon fields are selected from the results, if the query contains a :breakout clause (#20182)"
(mt/dataset sample-dataset
(with-redefs [tiles/create-tile (fn [_ points] points)
tiles/tile->byte-array identity]
(let [result (mt/user-http-request
:rasta :get 200 (format "tiles/7/30/49/%d/%d"
(mt/id :people :latitude)
(mt/id :people :longitude))
:query (json/generate-string
{:database (mt/id)
:type :query
:query {:source-table (mt/id :people)
:breakout [[:field (mt/id :people :latitude)]
[:field (mt/id :people :longitude)]]
:aggregation [[:count]]}}))]
(is (= [[36.6163612 -94.5197949]
[36.8177783 -93.8447328]
[36.8311004 -95.0253779]]
(take 3 result))))))))
(deftest failure-test
(testing "if the query fails, don't attempt to generate a map without any points -- the endpoint should return a 400"
......@@ -104,3 +125,10 @@
:type :query
:query {:source-table (mt/id :venues)}
:async? true}))))))
(deftest field-ref-test
(testing "Field refs can be constructed from strings representing integer field IDs or field names"
(is (= [:field 1 nil]
(@#'tiles/field-ref "1")))
(is (= [:field "Latitude" {:base-type :type/Float}]
(@#'tiles/field-ref "Latitude")))))
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment