Look for options at the last position in MBQL clauses (#30326)

Look for options at the last position in MBQL clauses

Fixes #30280.

src/metabase/lib/convert.cljc

@@ -70,8 +70,14 @@
   :hierarchy lib.hierarchy/hierarchy)
 
 (defn- default-MBQL-clause->pMBQL [mbql-clause]
-  (let [[clause-type options & args] (lib.options/ensure-uuid mbql-clause)]
-    (into [clause-type options] (map ->pMBQL) args)))
+  (let [last-elem (peek mbql-clause)
+        last-elem-option? (map? last-elem)
+        [clause-type & args] (cond-> mbql-clause
+                               last-elem-option? pop)
+        options (if last-elem-option?
+                  last-elem
+                  {})]
+    (lib.options/ensure-uuid (into [clause-type options] (map ->pMBQL) args))))
 
 (defmethod ->pMBQL :default
   [x]
@@ -140,6 +146,13 @@
                                              :type/*))]
     (lib.options/ensure-uuid [:value opts value])))
 
+(defmethod ->pMBQL :case
+  [[_tag pred-expr-pairs options]]
+  (let [default (:default options)]
+    (cond-> [:case (dissoc options :default) (mapv ->pMBQL pred-expr-pairs)]
+      :always lib.options/ensure-uuid
+      default (conj (->pMBQL default)))))
+
 (doseq [tag [:aggregation :expression]]
   (lib.hierarchy/derive tag ::aggregation-or-expression-ref))
 
@@ -191,7 +204,12 @@
          m)))
 
 (defn- aggregation->legacy-MBQL [[tag options & args]]
-  (let [inner (into [tag] (map ->legacy-MBQL args))]
+  (let [inner (into [tag] (map ->legacy-MBQL) args)
+        ;; the default value of the :case expression is in the options
+        ;; in legacy MBQL
+        inner (if (and (= tag :case) (next args))
+                (conj (pop inner) {:default (peek inner)})
+                inner)]
     (if-let [aggregation-opts (not-empty (options->legacy-MBQL options))]
       [:aggregation-options inner aggregation-opts]
       inner)))

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

@@ -50,17 +50,18 @@
    #_expr [:ref ::expression/expression]])
 
 
-(mbql-clause/define-tuple-mbql-clause :case
+(mbql-clause/define-catn-mbql-clause :case
   ;; TODO -- we should further constrain this so all of the exprs are of the same type
-  [:sequential {:min 1} [:ref ::case-subclause]])
+  [:pred-expr-pairs [:sequential {:min 1} [:ref ::case-subclause]]]
+  [:default [:? [:schema [:ref ::expression/expression]]]])
 
 (defmethod expression/type-of* :case
-  [[_tag _opts pred-expr-pairs]]
+  [[_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)))
-   nil
+   default
    pred-expr-pairs))
 
 ;;; TODO -- add constraint that these types have to be compatible

test/metabase/lib/convert_test.cljc

@@ -134,13 +134,13 @@
              :query {:source-table 1
                      :aggregation  [[:sum [:field 1 nil]]]
                      :breakout     [[:aggregation 0 {:display-name "Revenue"}]]}}
-          (lib.convert/->legacy-MBQL
-              {:lib/type :mbql/query
-               :stages   [{:lib/type     :mbql.stage/mbql
-                           :source-table 1
-                           :aggregation  [[:sum {:lib/uuid string?} [:field {:lib/uuid string?} 1]]]
-                           :breakout     [[:aggregation 0 {:display-name   "Revenue"
-                                                           :effective_type :type/Integer}]]}]})))))
+            (lib.convert/->legacy-MBQL
+             {:lib/type :mbql/query
+              :stages   [{:lib/type     :mbql.stage/mbql
+                          :source-table 1
+                          :aggregation  [[:sum {:lib/uuid string?} [:field {:lib/uuid string?} 1]]]
+                          :breakout     [[:aggregation 0 {:display-name   "Revenue"
+                                                          :effective_type :type/Integer}]]}]})))))
 
 (deftest ^:parallel round-trip-test
   ;; Miscellaneous queries that have caused test failures in the past, captured here for quick feedback.
@@ -260,6 +260,59 @@
                    :value [:param-value]}],
      :type :native}))
 
+(deftest ^:parallel round-trip-options-test
+  (testing "Round-tripping (p)MBQL caluses with options (#30280)"
+    (testing "starting with pMBQL"
+      (is (=? [:does-not-contain {:lib/uuid string?
+                                  :case-sensitive false}
+               [:field {:lib/uuid string?} 23]
+               "invite"]
+              (-> [:does-not-contain {:lib/uuid "b6a2ab24-bfb2-4b90-bd71-f96b1e025a5e"
+                                      :case-sensitive false}
+                   [:field {:lib/uuid "5d01e669-783f-40e0-9ae0-2b8098448390"} 23]
+                   "invite"]
+                  lib.convert/->legacy-MBQL lib.convert/->pMBQL))))
+    (testing "starting with MBQL"
+      (let [mbql-filter [:does-not-contain [:field 23 nil] "invite" {:case-sensitive false}]]
+        (is (= mbql-filter
+               (-> mbql-filter lib.convert/->pMBQL lib.convert/->legacy-MBQL)))))))
+
+(deftest ^:parallel case-expression-with-default-value-round-trip-test
+  (testing "Round trip of case expression with default value (#30280)"
+    (let [aggregation-options-clause
+          [:aggregation-options
+           [:distinct [:case [[[:> [:field 11 nil] 0] [:field 14 nil]]]
+                       {:default [:field 13 nil]}]]
+           {:name "CE"
+            :display-name "CE"}]
+          expected-pmbql-aggregation-options-clause
+          [:distinct {:name "CE"
+                      :display-name "CE"
+                      :lib/uuid string?}
+           [:case
+            {:lib/uuid string?}
+            [[[:> {:lib/uuid string?}
+               [:field {:lib/uuid string?} 11] 0]
+              [:field {:lib/uuid string?} 14]]]
+            [:field {:lib/uuid string?} 13]]]
+          mbql-query
+          {:database 1
+           :type :query
+           :query {:expressions {"CC" [:+ 1 1]}
+                   :limit 10
+                   :source-query {:source-table 2
+                                  :aggregation [aggregation-options-clause]
+                                  :breakout [[:field 15 {:temporal-unit :month}]]}}
+           :parameters []}
+          pmbql-aggregation-options-clause
+          (lib.convert/->pMBQL aggregation-options-clause)]
+      (is (=? expected-pmbql-aggregation-options-clause
+              pmbql-aggregation-options-clause))
+      (is (=? aggregation-options-clause
+              (lib.convert/->legacy-MBQL pmbql-aggregation-options-clause)))
+      (is (= (dissoc mbql-query :parameters [])
+             (-> mbql-query lib.convert/->pMBQL lib.convert/->legacy-MBQL))))))
+
 (deftest ^:parallel round-trip-preserve-metadata-test
   (testing "Round-tripping should not affect embedded metadata"
     (let [query {:database 2445