Merge pull request #7449 from metabase/trailing-comment-on-nested-queries

Trim trailing comments from native nested queries [ci drivers]

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