Skip to content
Snippets Groups Projects
user avatar
dpsutton authored
* Speed up interger or string recognition

jeff helpfully pointed this out. I did some repl benchmarks

```clojure
(comment
  (doseq [x ["3" "bob"]]
    (dotimes [_ 3] (time (dotimes [_ 10000] (try (Integer/parseInt x)
                                                  (catch NumberFormatException _ x))))))
  (doseq [x ["3" "bob"]]
    (dotimes [_ 3] (time (dotimes [_ 10000] (if (re-matches #"\d+" x)
                                              (Integer/parseInt x)
                                              x))))))

tiles-test=> (doseq [x ["3" "bob"]]
               (dotimes [_ 3] (time (dotimes [_ 10000] (try (Integer/parseInt x)
                                                            (catch NumberFormatException _ x))))))
"Elapsed time: 5.72825 msecs"
"Elapsed time: 1.531042 msecs"
"Elapsed time: 0.797041 msecs"
"Elapsed time: 32.120875 msecs"
"Elapsed time: 27.848375 msecs"
"Elapsed time: 23.820542 msecs"
nil
tiles-test=> (doseq [x ["3" "bob"]]
               (dotimes [_ 3] (time (dotimes [_ 10000] (if (re-matches #"\d+" x)
                                                         (Integer/parseInt x)
                                                         x)))))
"Elapsed time: 5.356417 msecs"
"Elapsed time: 1.97225 msecs"
"Elapsed time: 1.868708 msecs"
"Elapsed time: 1.214666 msecs"
"Elapsed time: 1.185625 msecs"
"Elapsed time: 1.19025 msecs"
nil
tiles-test=>
```

* Only select lat/lon fields in api/tiles

We required the index of lat and long columns, ran the whole query, and
then just grabbed those columns in a `(for [row rows] [(nth row
lat-idx)..])`. But of course we can just update the fields we want to
select in the query.

MBQL and native queries flow through this codepath depending on the
source of the query of the card being mapped. For mbql queries, it is
straightforward, add a filter on the lat long and replace the fields
with just lat and long fields. For native, we nest the native query as a
nested query and then proceed on the resulting mbql query. The only
difference is requiring the type annotation for the field type.

mbql field: [:field 32 nil] vs [:field "latitude" {:base-type :type/Float}]

This addresses memory concerns. The query is not limited and we don't
need to select an entire row for these purposes. Dropping lots of text
fields, id fields, etc could save quite a bit of memory when resolving
the entire result set in memory.

* Add limit to number of coordinates per tile

these queries were unbounded in getting coordinates for each tile. But
in a visualization, can there really be more information provided? We
were leaving them unbounded and getting back millions of rows (reported
on #4844). So we were adding unnecessary detail at the price of OOM
errors.

Note tests have been changed to show that the limit of the original mbql
query is clobbered and the limit of the original native query is only
present in the nested query whereas the outer query selects its own
limit.

As always, should this number be exposed as a setting? Configurable in
the UI? Leaving as a hardcoded and documented number for the moment.

* docstring

* Remove col indices from api/tiles

since we just select the actual columns we want, we no longer need the
index in the results of those cols. they are always the first and second
columns

* Trim final slash on url
1c70882b
History
Code owners
Assign users and groups as approvers for specific file changes. Learn more.
Name Last commit Last update
..
metabase