Trim trailing comments from native nested queries [ci drivers]

Having a trailing comment works fine when the query is executed
directly, but when in a nested query the original query is wrapped in
another query and the trailing comment causes the tail end of the
query to be "missing" due to the comment causing the query to fail.

Fixes #7363

src/metabase/query_processor/middleware/fetch_source_query.clj

 (ns metabase.query-processor.middleware.fetch-source-query
   "Middleware responsible for 'hydrating' the source query for queries that use another query as their source."
-  (:require [clojure.tools.logging :as log]
+  (:require [clojure.string :as str]
+            [clojure.tools.logging :as log]
             [metabase.query-processor
              [interface :as i]
              [util :as qputil]]
             [metabase.util :as u]
+            [puppetlabs.i18n.core :refer [trs]]
             [toucan.db :as db]))
 
+(defn- trim-query
+  "Native queries can have trailing SQL comments. This works when executed directly, but when we use the query in a
+  nested query, we wrap it in another query, which can cause the last part of the query to be unintentionally
+  commented out, causing it to fail. This function removes any trailing SQL comment."
+  [card-id query-str]
+  (let [trimmed-string (str/replace query-str #"--.*(\n|$)" "")]
+    (if (= query-str trimmed-string)
+      query-str
+      (do
+        (log/info (trs "Trimming trailing comment from card with id {0}" card-id))
+        trimmed-string))))
+
 (defn- card-id->source-query
   "Return the source query info for Card with CARD-ID."
   [card-id]
@@ -14,7 +28,7 @@
         card-query (:dataset_query card)]
     (assoc (or (:query card-query)
                (when-let [native (:native card-query)]
-                 {:native        (:query native)
+                 {:native        (trim-query card-id (:query native))
                   :template_tags (:template_tags native)})
                (throw (Exception. (str "Missing source query in Card " card-id))))
       ;; include database ID as well; we'll pass that up the chain so it eventually gets put in its spot in the

test/metabase/query_processor_test/nested_queries_test.clj

@@ -191,6 +191,32 @@
             :aggregation [:count]
             :breakout    [[:field-literal (keyword (data/format-name :price)) :type/Integer]]))))))
 
+;; Ensure trailing comments are trimmed and don't cause a wrapping SQL query to fail
+(expect
+  breakout-results
+  (tt/with-temp Card [card {:dataset_query {:database (data/id)
+                                            :type     :native
+                                            :native   {:query "SELECT * FROM VENUES -- small comment here"}}}]
+    (rows+cols
+      (format-rows-by [int int]
+        (qp/process-query
+          (query-with-source-card card
+            :aggregation [:count]
+            :breakout    [[:field-literal (keyword (data/format-name :price)) :type/Integer]]))))))
+
+;; Ensure trailing comments followed by a newline are trimmed and don't cause a wrapping SQL query to fail
+(expect
+  breakout-results
+  (tt/with-temp Card [card {:dataset_query {:database (data/id)
+                                            :type     :native
+                                            :native   {:query "SELECT * FROM VENUES -- small comment here\n"}}}]
+    (rows+cols
+      (format-rows-by [int int]
+        (qp/process-query
+          (query-with-source-card card
+            :aggregation [:count]
+            :breakout    [[:field-literal (keyword (data/format-name :price)) :type/Integer]]))))))
+
 
 ;; make sure we can filter by a field literal
 (expect