Support expression options and fields only case and coalasce exprs (#30395)

* Support expression options and fields only case and coalasce exprs

Fixes #29938.
Fixes #29950.

src/metabase/lib/aggregation.cljc

@@ -86,8 +86,6 @@
   [_query _stage-number _case]
   "case")
 
-(lib.hierarchy/derive :case ::aggregation)
-
 (lib.hierarchy/derive ::unary-aggregation ::aggregation)
 
 (doseq [tag [:avg

src/metabase/lib/convert.cljc

@@ -162,7 +162,7 @@
   (let [default (:default options)]
     (cond-> [:case (dissoc options :default) (mapv ->pMBQL pred-expr-pairs)]
       :always lib.options/ensure-uuid
-      default (conj (->pMBQL default)))))
+      (some? default) (conj (->pMBQL default)))))
 
 (defmethod ->pMBQL :expression
   [[tag value opts]]

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

@@ -55,14 +55,25 @@
   [:pred-expr-pairs [:sequential {:min 1} [:ref ::case-subclause]]]
   [:default [:? [:schema [:ref ::expression/expression]]]])
 
+(defn- allow-unkown-type
+  "A hack to allow case and coalesce expressions using fields only.
+  The real fix would pass in metadata so that the types of fields
+  can be determined."
+  [expr-type]
+  (if (= expr-type ::expression/type.unknown)
+    :type/*
+    expr-type))
+
 (defmethod expression/type-of* :case
   [[_tag _opts pred-expr-pairs default]]
-  (reduce
-   (fn [best-guess [_pred expr]]
-     (let [expr-type (expression/type-of expr)]
-       (best-return-type best-guess expr-type)))
-   default
-   pred-expr-pairs))
+  (-> (reduce
+       (fn [best-guess [_pred expr]]
+         (let [expr-type (expression/type-of expr)]
+           (best-return-type best-guess expr-type)))
+       (when (some? default)
+         (expression/type-of default))
+       pred-expr-pairs)
+      allow-unkown-type))
 
 ;;; TODO -- add constraint that these types have to be compatible
 (mbql-clause/define-tuple-mbql-clause :coalesce
@@ -71,4 +82,5 @@
 
 (defmethod expression/type-of* :coalesce
   [[_tag _opts expr null-value]]
-  (best-return-type (expression/type-of expr) (expression/type-of null-value)))
+  (-> (best-return-type (expression/type-of expr) (expression/type-of null-value))
+      allow-unkown-type))

test/metabase/lib/convert_test.cljc

@@ -186,6 +186,13 @@
 
     [:expression "expr" {:display-name "Iambic Diameter"}]
 
+    ;; (#29950)
+    [:starts-with [:field 133751 nil] "CHE" {:case-sensitive true}]
+
+    ;; (#29938)
+    {"First int"  [:case [[[:= [:field 133751 nil] 1] 1]]    {:default 0}]
+     "First bool" [:case [[[:= [:field 133751 nil] 1] true]] {:default false}]}
+
     [:case [[[:< [:field 1 nil] 10] [:value nil {:base_type :type/Number}]] [[:> [:field 2 nil] 2] 10]]]
 
     {:database 67

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

 (ns metabase.lib.schema.expression.conditional-test
   (:require
-   [clojure.test :refer [are deftest is]]
+   [clojure.test :refer [are deftest is testing]]
    [malli.core :as mc]
    [metabase.lib.schema :as lib.schema]
    [metabase.lib.schema.expression :as expression]
@@ -70,3 +70,33 @@
                                          {:base-type :type/Text, :lib/uuid "68443c43-f9de-45e3-9f30-8dfd5fef5af6"}
                                          (meta/id :venues :name)]
                                         "bar"]}}]})))
+
+(deftest ^:parallel case-type-of-with-fields-only-test
+  ;; Ideally expression/type-of should have enough information to determine the types of fields.
+  (testing "The type of a case expression can be determined even if it consists of fields only."
+    (is (= :type/*
+           (expression/type-of
+            [:case
+             {:lib/uuid "8c6e099e-b856-4aeb-a8f6-2266b5d3d1e3"}
+             [[[:>
+                {:lib/uuid "9c4cc3b0-f3c7-4d34-ab53-640ba6e911e5"}
+                [:field {:lib/uuid "435b08c8-9404-41a5-8c5a-00b415f14da6"} 25]
+                0]
+               [:field {:lib/uuid "1c93ba8b-6a39-4ef2-a9e6-e3bcff042800"} 32]]]
+             [:field
+              {:source-field 29
+               :lib/uuid "a5ab7f91-9826-40a7-9499-4a1a0184a450"}
+              23]])))))
+
+(deftest ^:parallel coalasce-type-of-with-fields-only-test
+  ;; Ideally expression/type-of should have enough information to determine the types of fields.
+  (testing "The type of a case expression can be determined even if it consists of fields only."
+    (is (= :type/*
+           (expression/type-of
+            [:coalesce
+             {:lib/uuid "8c6e099e-b856-4aeb-a8f6-2266b5d3d1e3"}
+             [:field {:lib/uuid "435b08c8-9404-41a5-8c5a-00b415f14da6"} 25]
+             [:field
+              {:source-field 29
+               :lib/uuid "a5ab7f91-9826-40a7-9499-4a1a0184a450"}
+              23]])))))

test/metabase/query_processor_test/test_mlv2.clj

@@ -35,12 +35,6 @@
   [legacy-query]
   (or
    *skip-conversion-tests*
-   ;; #29938: conversion for `:case` with default value does not work correctly
-   (mbql.u/match-one legacy-query
-     :case
-     (mbql.u/match-one &match
-       {:default _default}
-       "#29938"))
    ;; #29942: missing schema for `:cum-sum` and `:cum-count` aggregations
    (mbql.u/match-one legacy-query
      #{:cum-sum :cum-count}
@@ -57,10 +51,6 @@
    (mbql.u/match-one legacy-query
      :regex-match-first
      "#29949")
-   ;; #29950: string filter clauses with options like `:case-sensitive` option not handled correctly
-   (mbql.u/match-one legacy-query
-     {:case-sensitive _case-sensitive?}
-     "#29950")
    ;; #29958: `:convert-timezone` with 2 args is broken
    (mbql.u/match-one legacy-query
      [:convert-timezone _expr _source-timezone]