Relax concat args to allow any ExpressionArg (#48506) (#48597)

* Relax the arg types to ExpressionArg for concat expressions in the legacy schema

Relax the arg types to ExpressionArg for concat since many DBs allow to concatenate non-string types. This also aligns
with the corresponding MLv2 schema and with the reference docs we publish.

Fixes #39439

* Add nested concat schema tests

* Add nested-concat query-processor tests

Co-authored-by: appleby <86076+appleby@users.noreply.github.com>

src/metabase/legacy_mbql/schema.cljc

@@ -555,8 +555,10 @@
 (defclause ^{:requires-features #{:expressions}} replace
   s StringExpressionArg, match :string, replacement :string)
 
+;; Relax the arg types to ExpressionArg for concat since many DBs allow to concatenate non-string types. This also
+;; aligns with the corresponding MLv2 schema and with the reference docs we publish.
 (defclause ^{:requires-features #{:expressions}} concat
-  a StringExpressionArg, b StringExpressionArg, more (rest StringExpressionArg))
+  a ExpressionArg, b ExpressionArg, more (rest ExpressionArg))
 
 (defclause ^{:requires-features #{:expressions :regex}} regex-match-first
   s StringExpressionArg, pattern :string)

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

@@ -15,4 +15,32 @@
     [:concat
      {:lib/uuid "47cac41f-6240-4623-9a73-448addfdc735"}
      [:field {:lib/uuid "e47d33bc-c89c-48af-bffe-842c815f930b"} 1]
-     "2"]))
+     "2"]
+
+    [:concat
+     {:lib/uuid "47cac41f-6240-4623-9a73-448addfdc735"}
+     "concat simple nested expressions: "
+     [:get-year
+      {:lib/uuid "e47d33bc-c89c-48af-bffe-842c815f930a"}
+      [:field {:lib/uuid "e47d33bc-c89c-48af-bffe-842c815f930b"} 1]]
+     "Q"
+     [:get-quarter {:lib/uuid "e47d33bc-c89c-48af-bffe-842c815f930c"}
+      [:field {:lib/uuid "e47d33bc-c89c-48af-bffe-842c815f930d"} 2]]]
+
+    [:concat
+     {:lib/uuid "47cac41f-6240-4623-9a73-448addfdc735"}
+     "concat nested expressions of various types: "
+     [:/
+      {:lib/uuid "e47d33bc-c89c-48af-bffe-842c815f930a"}
+      3
+      [:+
+       {:lib/uuid "e47d33bc-c89c-48af-bffe-842c815f930b"}
+       1
+       [:field {:lib/uuid "e47d33bc-c89c-48af-bffe-842c815f930c"} 1]]]
+     [:and
+      {:lib/uuid "e47d33bc-c89c-48af-bffe-842c815f930a"}
+      [:or {:lib/uuid "e47d33bc-c89c-48af-bffe-842c815f930b"} 1 2]
+      3]
+     [:sum
+      {:lib/uuid "e47d33bc-c89c-48af-bffe-842c815f930a"}
+      [:field {:lib/uuid "e47d33bc-c89c-48af-bffe-842c815f930b"} 2]]]))

test/metabase/query_processor_test/string_extracts_test.clj

 (ns ^:mb/driver-tests metabase.query-processor-test.string-extracts-test
   (:require
    [clojure.test :refer :all]
+   [metabase.driver :as driver]
    [metabase.query-processor :as qp]
    [metabase.test :as mt]
    [metabase.test.data :as data]))
@@ -70,9 +71,30 @@
 
 (deftest ^:parallel test-concat
   (mt/test-drivers (mt/normal-drivers-with-feature :expressions)
-    (is (= "foobar" (test-string-extract [:concat "foo" "bar"])))
+    (testing "Does concat work with 2 strings"
+      (is (= "foobar" (test-string-extract [:concat "foo" "bar"]))))
     (testing "Does concat work with >2 args"
-      (is (= "foobar" (test-string-extract [:concat "f" "o" "o" "b" "a" "r"]))))))
+      (is (= "foobar" (test-string-extract [:concat "f" "o" "o" "b" "a" "r"]))))
+    (testing "Does concat work with nested concat expressions"
+      (is (= "foobar" (test-string-extract [:concat [:concat "f" "o" "o"] [:concat "b" "a" "r"]]))))))
+
+(defmethod driver/database-supports? [::driver/driver ::concat-non-string-args]
+  [_driver _feature _database]
+  true)
+
+;; These drivers do not support concat with non-string args
+(doseq [driver [:athena :mongo :presto-jdbc :vertica]]
+  (defmethod driver/database-supports? [driver ::concat-non-string-args]
+    [_driver _feature _database]
+    false))
+
+(deftest ^:parallel test-concat-non-string-args
+  (mt/test-drivers (mt/normal-drivers-with-feature :expressions ::concat-non-string-args)
+    (testing "Does concat work with non-string args"
+      (is (= "1234" (test-string-extract [:concat 123 [:+ 1 3]])))))
+  (mt/test-drivers (mt/normal-drivers-with-feature :expressions ::concat-non-string-args :temporal-extract)
+    (testing "Does concat work with nested temporal-extraction expressions"
+      (is (= "2024Q4" (test-string-extract [:concat [:get-year "2024-10-08"] "Q" [:get-quarter "2024-10-08"]]))))))
 
 (deftest ^:parallel test-regex-match-first
   (mt/test-drivers (mt/normal-drivers-with-feature :expressions :regex)