datetime fields in nested queries should get auto-bucketed the same way as top-level ones (#15555)

* datetime fields in nested queries should get auto-bucketed the same way as top-level ones

* Test fix 

enterprise/backend/test/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions_test.clj

@@ -163,7 +163,7 @@
                   :query      {:source-query {:source-table $$checkins
                                               :fields       [$id !default.$date $user_id $venue_id]
                                               :filter       [:and
-                                                             [:> $date [:absolute-datetime #t "2014-01-01T00:00Z[UTC]" :default]]
+                                                             [:>= !default.date [:absolute-datetime #t "2014-01-02T00:00Z[UTC]" :default]]
                                                              [:=
                                                               $user_id
                                                               [:value 5 {:base_type         :type/Integer

src/metabase/query_processor/middleware/auto_bucket_datetimes.clj

@@ -3,6 +3,7 @@
   bucketing. Applies to any unbucketed Field in a breakout, or fields in a filter clause being compared against
   `yyyy-MM-dd` format datetime strings."
   (:require [clojure.set :as set]
+            [clojure.walk :as walk]
             [medley.core :as m]
             [metabase.mbql.predicates :as mbql.preds]
             [metabase.mbql.schema :as mbql.s]
@@ -86,12 +87,12 @@
   `field-id->type-info` to see if we should do so."
   ;; we only want to wrap clauses in `:breakout` and `:filter` so just make a 3-arg version of this fn that takes the
   ;; name of the clause to rewrite and call that twice
-  ([query field-id->type-info :- FieldIDOrName->TypeInfo]
-   (-> query
+  ([inner-query field-id->type-info :- FieldIDOrName->TypeInfo]
+   (-> inner-query
        (wrap-unbucketed-fields field-id->type-info :breakout)
        (wrap-unbucketed-fields field-id->type-info :filter)))
 
-  ([query field-id->type-info clause-to-rewrite]
+  ([inner-query field-id->type-info clause-to-rewrite]
    (let [datetime-but-not-time? (comp date-or-datetime-field? field-id->type-info)]
      (letfn [(wrap-fields [x]
                (mbql.u/replace x
@@ -103,10 +104,10 @@
                  ;; `:type/Time`), then go ahead and replace it
                  [:field (id-or-name :guard datetime-but-not-time?) opts]
                  [:field id-or-name (assoc opts :temporal-unit :day)]))]
-       (m/update-existing-in query [:query clause-to-rewrite] wrap-fields)))))
+       (m/update-existing inner-query clause-to-rewrite wrap-fields)))))
 
-(s/defn ^:private auto-bucket-datetimes*
-  [{{breakouts :breakout, filter-clause :filter} :query, :as query}]
+(s/defn ^:private auto-bucket-datetimes-this-level
+  [{breakouts :breakout, filter-clause :filter, :as inner-query}]
   ;; find any breakouts or filters in the query that are just plain `[:field-id ...]` clauses (unwrapped by any other
   ;; clause)
   (if-let [unbucketed-fields (mbql.u/match (cons filter-clause breakouts)
@@ -116,9 +117,22 @@
     ;; breakouts/filters...
     (let [field-id->type-info (unbucketed-fields->field-id->type-info unbucketed-fields)]
       ;; ...and then update each breakout/filter by wrapping it if appropriate
-      (wrap-unbucketed-fields query field-id->type-info))
-    ;; otherwise if there are no unbuketed breakouts/filters return the query as-is
-    query))
+      (wrap-unbucketed-fields inner-query field-id->type-info))
+    ;; otherwise if there are no unbucketed breakouts/filters return the query as-is
+    inner-query))
+
+(defn- auto-bucket-datetimes-all-levels [{query-type :type, :as query}]
+  (if (not= query-type :query)
+    query
+    ;; walk query, looking for inner-query forms that have a `:filter` key
+    (walk/postwalk
+     (fn [form]
+       (if (and (map? form)
+                (or (seq (:filter form))
+                    (seq (:breakout form))))
+         (auto-bucket-datetimes-this-level form)
+         form))
+     query)))
 
 (defn auto-bucket-datetimes
   "Middleware that automatically adds `:temporal-unit` to breakout and filter `:field` clauses if the Field they refer
@@ -130,4 +144,4 @@
   format datetime strings."
   [qp]
   (fn [query rff context]
-    (qp (auto-bucket-datetimes* query) rff context)))
+    (qp (auto-bucket-datetimes-all-levels query) rff context)))

test/metabase/query_processor/middleware/auto_bucket_datetimes_test.clj

@@ -262,3 +262,23 @@
                                  :source-table $$checkins}]})]
           (is (= filter-clause
                  (get-in (auto-bucket query) [:query :filter]))))))))
+
+(deftest nested-queries-test
+  (testing "Datetime fields inside nested MBQL queries should get auto-bucketed the same way as at the top-level (#15352)"
+    (mt/dataset sample-dataset
+      (let [q1 (mt/mbql-query orders
+                 {:aggregation [[:count]]
+                  :filter      [:between $created_at "2020-02-01" "2020-02-29"]})]
+        (testing "original query"
+          (is (= (mt/mbql-query orders
+                   {:aggregation [[:count]]
+                    :filter      [:between !day.created_at "2020-02-01" "2020-02-29"]})
+                 (auto-bucket q1))))
+        (testing "nested query"
+          (let [q2 (mt/mbql-query nil
+                     {:source-query (:query q1)})]
+            (is (= (mt/mbql-query orders
+                     {:source-query {:source-table $$orders
+                                     :aggregation  [[:count]]
+                                     :filter       [:between !day.created_at "2020-02-01" "2020-02-29"]}})
+                   (auto-bucket q2)))))))))

test/metabase/query_processor_test/nested_queries_test.clj

@@ -1126,3 +1126,34 @@
                                    :aggregation  [[:sum *sum/Float]]
                                    :breakout     [*products.title]}
                     :filter       [:> *sum/Float 100]}))))))))
+
+(deftest date-range-test
+  (mt/test-drivers (disj (mt/normal-drivers-with-feature :foreign-keys :nested-queries) :redshift) ; sample-dataset doesn't work on Redshift yet -- see #14784
+    (testing "Date ranges should work the same in nested queries as is regular queries (#15352)"
+      (mt/dataset sample-dataset
+        (let [q1        (mt/mbql-query orders
+                          {:aggregation [[:count]]
+                           :filter      [:between $created_at "2020-02-01" "2020-02-29"]})
+              q1-native {:query  (str "SELECT count(*) AS \"count\" "
+                                      "FROM \"PUBLIC\".\"ORDERS\" "
+                                      "WHERE (\"PUBLIC\".\"ORDERS\".\"CREATED_AT\" >= ?"
+                                      " AND \"PUBLIC\".\"ORDERS\".\"CREATED_AT\" < ?)")
+                         :params [#t "2020-02-01T00:00Z[UTC]" #t "2020-03-01T00:00Z[UTC]"]}]
+          (testing "original query"
+            (when (= driver/*driver* :h2)
+              (is (= q1-native
+                     (qp/query->native q1))))
+            (is (= [[543]]
+                   (mt/formatted-rows [int] (qp/process-query q1)))))
+          (testing "nested query"
+            (let [q2 (mt/mbql-query nil
+                       {:source-query (:query q1)})]
+              (when (= driver/*driver* :h2)
+                (is (= (update q1-native :query (fn [s]
+                                                  (format (str "SELECT \"source\".\"count\" AS \"count\" "
+                                                               "FROM (%s) \"source\" "
+                                                               "LIMIT 1048575")
+                                                          s)))
+                       (qp/query->native q2))))
+              (is (= [[543]]
+                     (mt/formatted-rows [int] (qp/process-query q2)))))))))))