[MLv2] Fix NULL FK values in the QP and drill-thru (#35567)

The QP returns "large int" IDs like FKs as string, since JS doubles only
guarantee 51 bits of mantissa and a 64-bit ID might get rounded.
However NULL IDs were getting returned as `""` empty strings.

This fixes the QP middleware to return `nil` (JS `null`).

In addition, the drill-thru logic in the FE and MLv2 was trying to
filter incorrectly with a NULL value. This PR prevents the FK filter
drill from returning anything in that case.

I filed #35561 to track the fact that would be useful to drill thru to
all rows with a NULL for some FK, but it's low priority.

Fixes #13957.

src/metabase/lib/drill_thru/foreign_key.cljc

@@ -21,6 +21,7 @@
   (when (and (lib.drill-thru.common/mbql-stage? query stage-number)
              column
              (some? value)
+             (not= value :null) ; If the FK is null, don't show this option.
              (not (lib.types.isa/primary-key? column))
              (lib.types.isa/foreign-key? column))
     {:lib/type  :metabase.lib.drill-thru/drill-thru

src/metabase/query_processor/middleware/large_int_id.clj

@@ -5,10 +5,14 @@
    [metabase.mbql.util :as mbql.u]
    [metabase.query-processor.store :as qp.store]))
 
+(defn- ->string [x]
+  (when x
+    (str x)))
+
 (defn- result-int->string
   [field-indexes rf]
   ((map (fn [row]
-          (reduce #(update (vec %1) %2 str) row field-indexes)))
+          (reduce #(update (vec %1) %2 ->string) row field-indexes)))
    rf))
 
 (defn- should-convert-to-string? [field]
@@ -38,7 +42,7 @@
   "Converts any ID (:type/PK and :type/FK) in a result to a string to handle a number > 2^51
   or < -2^51, the JavaScript float mantissa. This will allow proper display of large numbers,
   like IDs from services like social media. All ID numbers are converted to avoid the performance
-  penalty of a comparison based on size."
+  penalty of a comparison based on size. NULLs are converted to Clojure nil/JS null."
   [{{:keys [js-int-to-string?] :or {js-int-to-string? false}} :middleware, :as query} rff]
   ;; currently, this excludes `:field` w/ name clauses, aggregations, etc.
   ;;

test/metabase/lib/drill_thru/foreign_key_test.cljc

@@ -44,3 +44,16 @@
                            (lib/available-drill-thrus query context))]
           (testing (str "\nAvailable drills =\n" (u/pprint-to-str drills))
             (is (not (contains? drills :drill-thru/fk-filter)))))))))
+
+(deftest ^:parallel do-not-return-fk-filter-for-null-fk-test
+  (testing "#13957 if this is an FK column but the value clicked is NULL, don't show the FK filter drill"
+    (let [test-case            {:click-type  :cell
+                                :query-type  :unaggregated
+                                :column-name "PRODUCT_ID"}
+          {:keys [query row]}  (lib.drill-thru.tu/query-and-row-for-test-case test-case)
+          context              (lib.drill-thru.tu/test-case-context query row test-case)
+          drill-types          #(->> % (lib/available-drill-thrus query) (map :type) set)]
+      (is (contains? (drill-types context)
+                     :drill-thru/fk-filter))
+      (is (not (contains? (drill-types (assoc context :value :null))
+                          :drill-thru/fk-filter))))))

test/metabase/query_processor/middleware/large_int_id_test.clj

@@ -125,3 +125,12 @@
         (is (= [["1"]
                 ["2147483647"]]
                (convert-id-to-string rows)))))))
+
+(deftest ^:parallel null-ids-as-strings
+  (testing "Middleware should convert NULL IDs to nil (#13957)"
+    (is (= [["1"]
+            ["2147483647"]
+            [nil]]
+           (convert-id-to-string [[1]
+                                  [Integer/MAX_VALUE]
+                                  [nil]])))))