From 604da7e263b3f7ad5d9f848498f7c0670a6c5aed Mon Sep 17 00:00:00 2001
From: Cam Saul <cammsaul@gmail.com>
Date: Tue, 6 Nov 2018 17:57:25 -0800
Subject: [PATCH] Simplify SparkSQL driver; fix nested query support [ci
drivers]
---
src/metabase/driver/sparksql.clj | 62 ++++---------------
.../nested_queries_test.clj | 19 ++++++
2 files changed, 31 insertions(+), 50 deletions(-)
diff --git a/src/metabase/driver/sparksql.clj b/src/metabase/driver/sparksql.clj
index a429e0a53ce..67a2bdb0c8d 100644
--- a/src/metabase/driver/sparksql.clj
+++ b/src/metabase/driver/sparksql.clj
@@ -15,7 +15,6 @@
[generic-sql :as sql]
[hive-like :as hive-like]]
[metabase.driver.generic-sql.query-processor :as sqlqp]
- [metabase.mbql.util :as mbql.u]
[metabase.models.field :refer [Field]]
[metabase.query-processor
[store :as qp.store]
@@ -34,54 +33,18 @@
;;; ------------------------------------------ Custom HoneySQL Clause Impls ------------------------------------------
-(def ^:private source-table-alias "t1")
-
-(defn- resolve-table-alias [{field-name :name, special-type :special_type, table-id :table_id, :as field}]
- (let [{schema-name :schema, table-name :name} (qp.store/table table-id)
- source-table (qp.store/table (mbql.u/query->source-table-id sqlqp/*query*))
- matching-join-table (some (fn [{:keys [table-id]}]
- (let [join-table (qp.store/table table-id)]
- (when (and (= schema-name (:schema join-table))
- (= table-name (:name join-table)))
- join-table)))
- (get-in sqlqp/*query* [:query :join-tables]))]
- (cond
- (and (= schema-name (:schema source-table))
- (= table-name (:name source-table)))
- (assoc field :schema nil, :table source-table-alias)
-
- matching-join-table
- (assoc field :schema nil, :table (:join-alias matching-join-table))
-
- :else
- field)))
-
-(defmethod sqlqp/->honeysql [SparkSQLDriver (class Field)]
- [driver field-before-aliasing]
- (let [{schema-name :schema
- table-name :table
- special-type :special_type
- field-name :name} (resolve-table-alias field-before-aliasing)
- field (keyword (hx/qualify-and-escape-dots schema-name table-name field-name))]
- (cond
- (isa? special-type :type/UNIXTimestampSeconds) (sql/unix-timestamp->timestamp driver field :seconds)
- (isa? special-type :type/UNIXTimestampMilliseconds) (sql/unix-timestamp->timestamp driver field :milliseconds)
- :else field)))
-
-(defn- apply-join-tables
- [honeysql-form {join-tables :join-tables}]
- (loop [honeysql-form honeysql-form, [{:keys [table-id pk-field-id fk-field-id schema join-alias]} & more] join-tables]
- (let [{table-name :name} (qp.store/table table-id)
- source-field (qp.store/field fk-field-id)
- pk-field (qp.store/field pk-field-id)
- honeysql-form (h/merge-left-join honeysql-form
- [(hx/qualify-and-escape-dots schema table-name) (keyword join-alias)]
- [:=
- (hx/qualify-and-escape-dots source-table-alias (:field-name source-field))
- (hx/qualify-and-escape-dots join-alias (:field-name pk-field))])]
- (if (seq more)
- (recur honeysql-form more)
- honeysql-form))))
+(def ^:private source-table-alias
+ "Default alias for all source tables. (Not for source queries; those still use the default SQL QP alias of `source`.)"
+ "t1")
+
+(defmethod sqlqp/->honeysql [SparkSQLDriver (class Field)]
+ [driver field]
+ (let [table (qp.store/table (:table_id field))
+ table-name (if (:alias? table)
+ (:name table)
+ source-table-alias)
+ field-identifier (keyword (hx/qualify-and-escape-dots table-name (:name field)))]
+ (sqlqp/cast-unix-timestamp-field-if-needed driver field field-identifier)))
(defn- apply-page-using-row-number-for-offset
"Apply `page` clause to HONEYSQL-FROM, using row_number() for drivers that do not support offsets"
@@ -203,7 +166,6 @@
(merge (sql/ISQLDriverDefaultsMixin)
{:apply-page (u/drop-first-arg apply-page-using-row-number-for-offset)
:apply-source-table (u/drop-first-arg apply-source-table)
- :apply-join-tables (u/drop-first-arg apply-join-tables)
:column->base-type (u/drop-first-arg hive-like/column->base-type)
:connection-details->spec (u/drop-first-arg connection-details->spec)
:date (u/drop-first-arg hive-like/date)
diff --git a/test/metabase/query_processor_test/nested_queries_test.clj b/test/metabase/query_processor_test/nested_queries_test.clj
index a7f6705523c..c657c0e6a1d 100644
--- a/test/metabase/query_processor_test/nested_queries_test.clj
+++ b/test/metabase/query_processor_test/nested_queries_test.clj
@@ -646,3 +646,22 @@
Collection [dest-card-collection]]
(perms/grant-collection-read-permissions! (group/all-users) source-card-collection)
(save-card-via-API-with-native-source-query! 403 db source-card-collection dest-card-collection)))))
+
+;; make sure that if we refer to a Field that is actually inside the source query, the QP is smart enough to figure
+;; out what you were referring to and behave appropriately
+(datasets/expect-with-engines (non-timeseries-engines-with-feature :nested-queries)
+ [[10]]
+ (format-rows-by [int]
+ (rows
+ (qp/process-query
+ {:database (data/id)
+ :type :query
+ :query {:source-query {:source-table (data/id :venues)
+ :fields [(data/id :venues :id)
+ (data/id :venues :name)
+ (data/id :venues :category_id)
+ (data/id :venues :latitude)
+ (data/id :venues :longitude)
+ (data/id :venues :price)]}
+ :aggregation [[:count]]
+ :filter [:= [:field-id (data/id :venues :category_id)] 50]}}))))
--
GitLab