Support Offset() as an expression with no breakouts [backend]

src/metabase/legacy_mbql/normalize.cljc

@@ -34,6 +34,7 @@
  [medley.core :as m]
  [metabase.legacy-mbql.schema :as mbql.s]
  [metabase.legacy-mbql.util :as mbql.u]
+ [metabase.lib.normalize :as lib.normalize]
  [metabase.lib.util.match :as lib.util.match]
  [metabase.shared.util.i18n :as i18n]
  [metabase.util :as u]
@@ -90,10 +91,11 @@
 
 (defn- normalize-ref-opts [opts]
  (let [opts (normalize-tokens opts :ignore-path)]
- (cond-> opts
- (:base-type opts) (update :base-type keyword)
- (:temporal-unit opts) (update :temporal-unit keyword)
- (:binning opts) (update :binning (fn [binning]
+ (cond-> opts
+ (:base-type opts) (update :base-type keyword)
+ (:effective-type opts) (update :effective-type keyword)
+ (:temporal-unit opts) (update :temporal-unit keyword)
+ (:binning opts) (update :binning (fn [binning]
  (cond-> binning
  (:strategy binning) (update :strategy keyword)))))))
 
@@ -207,7 +209,8 @@
 (defmethod normalize-mbql-clause-tokens :offset
  [[_tag opts expr n, :as clause]]
  {:pre [(= (count clause) 4)]}
- [:offset (or opts {}) (normalize-tokens expr :ignore-path) n])
+ (let [opts (lib.normalize/normalize :metabase.lib.schema.common/options (or opts {}))]
+ [:offset opts (normalize-tokens expr :ignore-path) n]))
 
 (defmethod normalize-mbql-clause-tokens :default
  ;; MBQL clauses by default are recursively normalized.

src/metabase/lib/schema.cljc

@@ -109,14 +109,23 @@
  (= ref-type (first x))
  (not (contains? valid-ids (get x 2)))))
 
+(defn- stage-with-joins-and-namespaced-keys-removed
+ "For ref validation purposes we should ignore `:joins` and any namespaced keys that might be used to record additional
+ info e.g. `:lib/metadata`."
+ [stage]
+ (select-keys stage (into []
+ (comp (filter simple-keyword?)
+ (remove (partial = :joins)))
+ (keys stage))))
+
 (defn- expression-ref-errors-for-stage [stage]
  (let [expression-names (into #{} (map (comp :lib/expression-name second)) (:expressions stage))]
- (mbql.u/matching-locations (dissoc stage :joins :lib/stage-metadata)
+ (mbql.u/matching-locations (stage-with-joins-and-namespaced-keys-removed stage)
  #(bad-ref-clause? :expression expression-names %))))
 
 (defn- aggregation-ref-errors-for-stage [stage]
  (let [uuids (into #{} (map (comp :lib/uuid second)) (:aggregation stage))]
- (mbql.u/matching-locations (dissoc stage :joins :lib/stage-metadata)
+ (mbql.u/matching-locations (stage-with-joins-and-namespaced-keys-removed stage)
  #(bad-ref-clause? :aggregation uuids %))))
 
 (defn ref-errors-for-stage

src/metabase/lib/schema/expression/window.cljc

@@ -13,7 +13,7 @@
 
 ;;; added 0.50.0
 (mbql-clause/define-tuple-mbql-clause :offset
- #_expr :any
+ #_expr [:ref ::expression/expression]
  #_n ::offset.n)
 
 (defmethod expression/type-of-method :offset

src/metabase/lib/schema/util.cljc

@@ -12,7 +12,7 @@
  [our-uuid]
  [])
  (comp (remove (fn [[k _v]]
- (#{:lib/metadata :lib/stage-metadata :lib/options} k)))
+ (qualified-keyword? k)))
  (mapcat (fn [[_k v]]
  (collect-uuids v))))
  m))

test/metabase/legacy_mbql/normalize_test.cljc

@@ -1510,3 +1510,18 @@
  k {"x" 1, "y" {"z" 2}, "a" nil}}]
  (t/is (= query
  (mbql.normalize/normalize query)))))))
+
+(t/deftest ^:parallel normalize-offset-test
+ (t/is (=? [:offset
+ {:effective-type :type/Float, :lib/uuid string?}
+ [:field
+ 1
+ {:base-type :type/Float, :effective-type :type/Float}]
+ -1]
+ (mbql.normalize/normalize
+ ["offset"
+ {"effective-type" "type/Float"}
+ ["field"
+ 1
+ {"base-type" "type/Float", "effective-type" "type/Float"}]
+ -1]))))

test/metabase/lib/schema/expression/window_test.cljc

@@ -0,0 +1,29 @@
+(ns metabase.lib.schema.expression.window-test
+ (:require
+ #?@(:cljs ([metabase.test-runner.assert-exprs.approximately-equal]))
+ [clojure.test :refer [deftest are]]
+ [metabase.lib.normalize :as lib.normalize]))
+
+#?(:cljs (comment metabase.test-runner.assert-exprs.approximately-equal/keep-me))
+
+(deftest ^:parallel normalize-offset-test
+ (are [x] (=? [:offset
+ {:lib/uuid string?, :effective-type :type/Float}
+ [:field
+ {:base-type :type/Float, :effective-type :type/Float, :lib/uuid string?}
+ 1]
+ -1]
+ (lib.normalize/normalize x))
+ [:offset
+ {:effective-type "type/Float"}
+ [:field
+ {:base-type "type/Float", :effective-type "type/Float"}
+ 1]
+ -1]
+
+ ["offset"
+ {"effective-type" "type/Float"}
+ ["field"
+ {"base-type" "type/Float", "effective-type" "type/Float"}
+ 1]
+ -1]))

test/metabase/lib/schema/expression_test.cljc

@@ -1,8 +1,9 @@
 (ns metabase.lib.schema.expression-test
- (:require [clojure.test :refer [deftest are testing]]
- [metabase.lib.core :as lib]
- [metabase.lib.schema.expression :as lib.schema.expression]
- [metabase.lib.test-metadata :as meta]))
+ (:require
+ [clojure.test :refer [deftest are testing]]
+ [metabase.lib.core :as lib]
+ [metabase.lib.schema.expression :as lib.schema.expression]
+ [metabase.lib.test-metadata :as meta]))
 
 (deftest ^:parallel comparable-expressions?-test
  (let [abs-datetime [:absolute-datetime {:lib/uuid (str (random-uuid))}

test/metabase/lib/schema_test.cljc

@@ -48,8 +48,8 @@
  [(str "Invalid :aggregation reference: no aggregation with uuid " bad-ref)]
 
  ;; if we forget to remove legacy ag refs from some part of the query make sure we get a useful error message.
- {:lib/type  :mbql.stage/mbql
- :metabase.lib.stage/cached-metadata {:field-ref [:aggregation 0]}}
+ {:lib/type :mbql.stage/mbql
+ :some-other-section {:field-ref [:aggregation 0]}}
  ["Invalid :aggregation reference: [:aggregation 0]"]
 
  ;; don't recurse into joins.

test/metabase/query_processor/test_util.clj

@@ -437,12 +437,12 @@
  when [[metabase.config/is-test?]] is true."
  false)
 
-(defn mock-fks-metadata-provider
+(defn mock-fks-application-database-metadata-provider
  "Impl for [[with-mock-fks-for-drivers-without-fk-constraints]]. A mock metadata provider composed with the application
  database metadata provider that adds FK relationships for Tables that would normally have them in drivers that have
  formal FK constraints."
  ([]
- (mock-fks-metadata-provider (lib.metadata.jvm/application-database-metadata-provider (data/id))))
+ (mock-fks-application-database-metadata-provider (lib.metadata.jvm/application-database-metadata-provider (data/id))))
 
  ([parent-metadata-provider]
  (lib.tu/merged-mock-metadata-provider
@@ -460,8 +460,8 @@
  (binding [qp.store/*TESTS-ONLY-allow-replacing-metadata-provider* true
  *enable-fk-support-for-disabled-drivers-in-tests* true]
  (qp.store/with-metadata-provider (if (qp.store/initialized?)
- (mock-fks-metadata-provider (qp.store/metadata-provider))
- (mock-fks-metadata-provider))
+ (mock-fks-application-database-metadata-provider (qp.store/metadata-provider))
+ (mock-fks-application-database-metadata-provider))
  (thunk))))
 
 (defmacro with-mock-fks-for-drivers-without-fk-constraints