diff --git a/src/metabase/driver/bigquery.clj b/src/metabase/driver/bigquery.clj
index 56793fd3ed9a92199e7522a33c3e3fa012edf828..2cffced368c0136298bce3382b7a07d93ce4ad03 100644
--- a/src/metabase/driver/bigquery.clj
+++ b/src/metabase/driver/bigquery.clj
@@ -22,7 +22,9 @@
[metabase.mbql
[schema :as mbql.s]
[util :as mbql.u]]
- [metabase.models.table :as table]
+ [metabase.models
+ [field :refer [Field]]
+ [table :as table]]
[metabase.query-processor
[store :as qp.store]
[util :as qputil]]
@@ -317,7 +319,8 @@
;; TODO - this is totally unnecessary now, we can just override `->honeysql` for `Field` and `Table` instead. FIXME!
(defrecord ^:private BigQueryIdentifier [dataset-name ; optional; will use (dataset-name-for-current-query) otherwise
table-name
- field-name]
+ field-name
+ alias?]
honeysql.format/ToSql
(to-sql [{:keys [dataset-name table-name field-name], :as bq-id}]
;; Check to make sure the identifiers are valid and don't contain any sorts of escape characters since we are
@@ -333,9 +336,13 @@
(assert (valid-bigquery-identifier? field-name)
(tru "Invalid BigQuery identifier: ''{0}''" field-name)))
;; BigQuery identifiers should look like `dataset.table` or `dataset.table`.`field` (SAD!)
- (str (format "`%s.%s`" (or dataset-name (dataset-name-for-current-query)) table-name)
- (when (seq field-name)
- (format ".`%s`" field-name)))))
+ (let [dataset-name (or dataset-name (dataset-name-for-current-query))]
+ (str
+ (if alias?
+ (format "`%s`" table-name)
+ (format "`%s.%s`" dataset-name table-name))
+ (when (seq field-name)
+ (format ".`%s`" field-name))))))
(defn- honeysql-form->sql ^String [honeysql-form]
{:pre [(map? honeysql-form)]}
@@ -393,17 +400,14 @@
[driver [_ field unit]]
(sql/date driver unit (sqlqp/->honeysql driver field)))
-(defmethod sqlqp/->honeysql [BigQueryDriver :field-id]
- [_ [_ field-id]]
- (let [{field-name :name, special-type :special_type, table-id :table_id} (qp.store/field field-id)
- {table-name :name} (qp.store/table table-id)
- field (map->BigQueryIdentifier
- {:table-name table-name
- :field-name field-name})]
- (cond
- (isa? special-type :type/UNIXTimestampSeconds) (unix-timestamp->timestamp field :seconds)
- (isa? special-type :type/UNIXTimestampMilliseconds) (unix-timestamp->timestamp field :milliseconds)
- :else field)))
+(defmethod sqlqp/->honeysql [BigQueryDriver (class Field)]
+ [driver field]
+ (let [{table-name :name, :as table} (qp.store/table (:table_id field))
+ field-identifier (map->BigQueryIdentifier
+ {:table-name table-name
+ :field-name (:name field)
+ :alias? (:alias? table)})]
+ (sqlqp/cast-unix-timestamp-field-if-needed driver field field-identifier)))
(defn- field->identifier
"Generate appropriate identifier for a Field for SQL parameters. (NOTE: THIS IS ONLY USED FOR SQL PARAMETERS!)"
@@ -448,10 +452,10 @@
honeysql-form
(h/merge-left-join honeysql-form
[(map->BigQueryIdentifier {:table-name table-name})
- (map->BigQueryIdentifier {:table-name join-alias})]
+ (map->BigQueryIdentifier {:table-name join-alias, :alias? true})]
[:=
(map->BigQueryIdentifier {:table-name source-table-name, :field-name (:name source-field)})
- (map->BigQueryIdentifier {:table-name join-alias, :field-name (:name pk-field)})])]
+ (map->BigQueryIdentifier {:table-name join-alias, :field-name (:name pk-field), :alias? true})])]
(if (seq more)
(recur honeysql-form more)
honeysql-form)))))
diff --git a/src/metabase/driver/sparksql.clj b/src/metabase/driver/sparksql.clj
index a429e0a53ce698b6c0cf2391f470913ff4738fc9..67a2bdb0c8d15557f717718597efa13ace895df5 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/driver/bigquery_test.clj b/test/metabase/driver/bigquery_test.clj
index 06775218852696bdb8be727d11c7989cd574bd79..f0c22edefaf509049559981c79eec1273271c54b 100644
--- a/test/metabase/driver/bigquery_test.clj
+++ b/test/metabase/driver/bigquery_test.clj
@@ -151,10 +151,11 @@
;; alias, e.g. something like `categories__via__category_id`, which is considerably different from what other SQL
;; databases do. (#4218)
(expect-with-engine :bigquery
- (str "SELECT `test_data.categories__via__category_id`.`name` AS `name`,"
- " count(*) AS `count` FROM `test_data.venues` "
- "LEFT JOIN `test_data.categories` `test_data.categories__via__category_id`"
- " ON `test_data.venues`.`category_id` = `test_data.categories__via__category_id`.`id` "
+ (str "SELECT `categories__via__category_id`.`name` AS `name`,"
+ " count(*) AS `count` "
+ "FROM `test_data.venues` "
+ "LEFT JOIN `test_data.categories` `categories__via__category_id`"
+ " ON `test_data.venues`.`category_id` = `categories__via__category_id`.`id` "
"GROUP BY `name` "
"ORDER BY `name` ASC")
;; normally for test purposes BigQuery doesn't support foreign keys so override the function that checks that and
diff --git a/test/metabase/query_processor_test/nested_queries_test.clj b/test/metabase/query_processor_test/nested_queries_test.clj
index a7f6705523c6cbc6b1cc8619c86c37a4c1307cae..c657c0e6a1dcfe643642d69e594ecb9ef8d7c2f9 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]}}))))