From 77647b8c4aaa7653df1fb83fa1f0071070d3f4da Mon Sep 17 00:00:00 2001
From: Cam Saul <cam@geotip.com>
Date: Wed, 1 Jul 2015 20:15:46 -0700
Subject: [PATCH] more work on nested field support
---
src/metabase/driver/mongo.clj | 31 ++++++++-----------
src/metabase/driver/mongo/query_processor.clj | 8 +++--
src/metabase/driver/mongo/util.clj | 3 --
src/metabase/driver/sync.clj | 13 ++++----
4 files changed, 24 insertions(+), 31 deletions(-)
diff --git a/src/metabase/driver/mongo.clj b/src/metabase/driver/mongo.clj
index 5987980f087..e4d70283f59 100644
--- a/src/metabase/driver/mongo.clj
+++ b/src/metabase/driver/mongo.clj
@@ -99,11 +99,8 @@
ISyncDriverFieldSubFields
(active-subfield-names->type [this field]
- ;; shared part of driver should be enforcing this actually !
- (assert (= (:base_type field) :UnknownField))
;; Build a map of nested-field-key -> type -> count
- ;; TODO - using an atom isn't the *fastest* thing in the world (but is the easiest); this takes ~50ms for a smallish Field in the Geographic Tips dataset;
- ;; if I run out of important things to do re-write this recursively or with transient collections
+ ;; TODO - using an atom isn't the *fastest* thing in the world (but is the easiest); consider alternate implementation
(let [field->type->count (atom {})]
;; Look at the first 1000 values
(doseq [val (take 1000 (field-values-lazy-seq this field))]
@@ -155,17 +152,15 @@
(with-mongo-connection [_ db]
(active-subfield-names->type driver &tips.venue)))))
-{:id 100
- :name "address"
- :parent_id nil}
-
-{:parent_id 100
- :name "street"
- :type :CharField
- :special_type :name}
-
-{:name "Tempest"
- :address {:street "430 Natoma"
- :city "San Francisco"
- :state "CA"
- :country "US"}}
+;; TODO
+;; 1. Sync
+;; 1A. Create new Field objects for each Nested Field during Sync
+;; 1B. Recursively call sync-field! for each nested field
+;; 2. QX
+;; 2A. Recursively resolve nested fields
+;; 2B. Add qualified-path-components fieldd to nested fields
+;; 3. Mongo
+;; 3A. Mongo QP handle nested Fields appropriately
+;; 4. API
+;; 4A. API Tweaks as Needed
+;; 5. Cleanup + Tests
diff --git a/src/metabase/driver/mongo/query_processor.clj b/src/metabase/driver/mongo/query_processor.clj
index ab540d770cc..7cd9fe88081 100644
--- a/src/metabase/driver/mongo/query_processor.clj
+++ b/src/metabase/driver/mongo/query_processor.clj
@@ -2,6 +2,7 @@
(:refer-clojure :exclude [find sort])
(:require [clojure.core.match :refer [match]]
[clojure.tools.logging :as log]
+ [clojure.walk :as walk]
[colorize.core :as color]
(monger [collection :as mc]
[core :as mg]
@@ -38,9 +39,10 @@
(case (keyword query-type)
:query (let [generated-query (process-structured (:query query))]
(when-not qp/*disable-qp-logging*
- (log/debug (color/magenta "\n\n******************** Generated Monger Query: ********************\n"
- (u/pprint-to-str generated-query)
- "\n*****************************************************************\n")))
+ (log/debug (u/format-color 'green "\nMONGER FORM:\n\n%s\n"
+ (->> generated-query
+ (walk/postwalk #(if (symbol? %) (symbol (name %)) %)) ; strip namespace qualifiers from Monger form
+ u/pprint-to-str) "\n"))) ; so it's easier to read
{:results (eval generated-query)})
:native (let [results (eval-raw-command (:query (:native query)))]
{:results (if (sequential? results) results
diff --git a/src/metabase/driver/mongo/util.clj b/src/metabase/driver/mongo/util.clj
index 46cee85ce8d..142a950efab 100644
--- a/src/metabase/driver/mongo/util.clj
+++ b/src/metabase/driver/mongo/util.clj
@@ -93,8 +93,5 @@
(sort-by second)
first
first
- ((fn [v]
- (println (metabase.util/format-color 'green "TYPE: %s" v))
- v))
driver/class->base-type)
:UnknownField))
diff --git a/src/metabase/driver/sync.clj b/src/metabase/driver/sync.clj
index 4d6effbf4a6..de312f2b5a4 100644
--- a/src/metabase/driver/sync.clj
+++ b/src/metabase/driver/sync.clj
@@ -24,7 +24,7 @@
sync-table-active-fields-and-pks!
sync-table-fks!
sync-table-fields-metadata!
- update-field-subfields!
+ sync-field-subfields!
update-table-row-count!)
;; ## sync-database! and sync-table!
@@ -152,7 +152,7 @@
;; Now do the syncing for Table's Fields
(log/debug (format "Determining active Fields for Table '%s'..." (:name table)))
(let [active-column-names->type (active-column-names->type driver table)
- field-name->id (sel :many :field->id [Field :name] :table_id (:id table) :active true)]
+ field-name->id (sel :many :field->id [Field :name] :table_id (:id table) :active true)]
(assert (map? active-column-names->type) "active-column-names->type should return a map.")
(assert (every? string? (keys active-column-names->type)) "The keys of active-column-names->type should be strings.")
(assert (every? (partial contains? field/base-types) (vals active-column-names->type)) "The vals of active-column-names->type should be valid Field base types.")
@@ -246,13 +246,12 @@
{:pre [driver
field]}
(log/debug (format "Syncing field '%s.%s'..." (:name @(:table field)) (:name field)))
- (println (u/format-color 'red "FIELD TYPE: %s" field))
(sync-field->> field
(mark-url-field! driver)
mark-category-field!
(mark-no-preview-display-field! driver)
auto-assign-field-special-type-by-name!
- (update-field-subfields! driver)))
+ (sync-field-subfields! driver)))
;; Each field-syncing function below should return FIELD with any updates that we made, or nil.
@@ -429,9 +428,9 @@
(assoc field :special_type special-type))))
-(defn- update-field-subfields! [driver field]
- (when (and (= (:base_type field) :UnknownField)
+(defn- sync-field-subfields! [driver field]
+ (when (and (= (:base_type field) :DictionaryField)
(supports? driver :nested-fields) ; if one of these is true
(satisfies? ISyncDriverFieldSubFields driver)) ; the other should be :wink:
(let [subfield-name->type (active-subfield-names->type driver field)]
- (log/info "Syncing subfields for '%s.%s': %s" (:name @(:table field)) (:name field) (keys subfield-name->type)))))
+ (log/info (u/format-color 'green "Syncing subfields for '%s.%s': %s" (:name @(:table field)) (:name field) (keys subfield-name->type))))))
--
GitLab