Skip to content
Snippets Groups Projects
Unverified Commit 54109eeb authored by Braden Shepherdson's avatar Braden Shepherdson Committed by GitHub
Browse files

Use `:database` to check for a Mongo native query (#32189)

Previously this was done by checking if the FE had included
`:collection "some_table"` on the native query, but that's been
included for SQL queries lately, and broke "Explore Results".

Includes new e2e tests: one for this issue itself, and another
that the fix doesn't break real Mongo native queries.

Fixes #32121.
parent 78f82236
No related branches found
No related tags found
No related merge requests found
import { restore, saveQuestion, startNewQuestion } from "e2e/support/helpers";
const MONGO_DB_NAME = "QA Mongo4";
describe("issue 32121", () => {
describe("on SQL questions", () => {
beforeEach(() => {
restore();
cy.signInAsAdmin();
});
it("clicking 'Explore results' works (metabase#32121)", () => {
// Stepping all the way through the QB because I couldn't repro with canned `createNativeQuestion` JSON.
startNewQuestion();
// Query the entire Orders table, then convert to SQL.
cy.get("#DataPopover").findByText("Sample Database").click();
cy.get("#DataPopover").findByText("Orders").click();
cy.findByTestId("qb-header").find(".Icon-sql").click();
cy.get(".Modal").findByText("Convert this question to SQL").click();
// Run the query.
cy.intercept("POST", "/api/dataset").as("dataset");
cy.get(".NativeQueryEditor .Icon-play").click();
cy.wait("@dataset");
cy.findByTestId("question-row-count").contains(
"Showing first 2,000 rows",
);
// Save it.
saveQuestion("all Orders");
cy.findByTestId("qb-header").findByText("Explore results").click();
cy.findByTestId("question-row-count").contains(
"Showing first 2,000 rows",
);
});
});
describe("on native Mongo questions", { tags: "@external" }, () => {
before(() => {
restore("mongo-4");
cy.signInAsAdmin();
startNewQuestion();
// eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
cy.findByText(MONGO_DB_NAME).click();
// eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
cy.findByText("Orders").click();
});
it("convert GUI question to native query, and 'Explore results' works (metabase#32121)", () => {
cy.get(".QueryBuilder .Icon-sql").click();
cy.get(".Modal")
.findByText("Convert this question to a native query")
.click();
cy.get(".Modal").should("not.exist");
cy.intercept("POST", "/api/dataset").as("dataset");
cy.get(".NativeQueryEditor .Icon-play").click();
cy.wait("@dataset");
saveQuestion("all Orders");
cy.findByTestId("question-row-count").contains(
"Showing first 2,000 rows",
);
cy.findByTestId("qb-header").findByText("Explore results").click();
cy.findByTestId("question-row-count").contains(
"Showing first 2,000 rows",
);
});
});
});
......@@ -26,6 +26,7 @@
[clojure.string :as str]
[medley.core :as m]
[metabase.driver.ddl.interface :as ddl.i]
[metabase.driver.util :as driver.u]
[metabase.mbql.normalize :as mbql.normalize]
[metabase.mbql.schema :as mbql.s]
[metabase.mbql.util :as mbql.u]
......@@ -86,7 +87,6 @@
(s/constrained query-has-resolved-database-id?
"Query where source-query virtual `:database` has been replaced with actual Database ID")))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | Resolving card__id -> source query |
;;; +----------------------------------------------------------------------------------------------------------------+
......@@ -106,21 +106,22 @@
(defn- source-query
"Get the query to be run from the card"
[{dataset-query :dataset_query card-id :id :as card}]
(let [{mbql-query :query
(let [{db-id :database
mbql-query :query
{template-tags :template-tags :as native-query} :native} dataset-query]
(or
mbql-query
;; rename `:query` to `:native` because source queries have a slightly different shape
(when-some [native-query (set/rename-keys native-query {:query :native})]
(let [collection (:collection native-query)]
(let [mongo? (= (driver.u/database->driver db-id) :mongo)]
(cond-> native-query
;; MongoDB native queries consist of a collection and a pipelne (query)
collection
(update :native (fn [pipeline] {:collection collection
mongo?
(update :native (fn [pipeline] {:collection (:collection native-query)
:query pipeline}))
;; trim trailing comments from SQL, but not other types of native queries
(and (nil? collection)
(and (not mongo?)
(string? (:native native-query)))
(update :native (partial trim-sql-query card-id))
......@@ -171,7 +172,6 @@
(when-let [[_ card-id-str] (re-find #"^card__(\d+)$" source-table-str)]
(Integer/parseInt card-id-str)))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | Logic for traversing the query |
;;; +----------------------------------------------------------------------------------------------------------------+
......@@ -290,7 +290,7 @@
extract-resolved-card-id))
(s/defn resolve-card-id-source-tables* :- {:card-id (s/maybe su/IntGreaterThanZero)
:query FullyResolvedQuery}
:query FullyResolvedQuery}
"Resolve `card__n`-style `:source-tables` in `query`."
[{inner-query :query, :as outer-query} :- mbql.s/Query]
(if-not inner-query
......
......@@ -2,6 +2,7 @@
(:require
[cheshire.core :as json]
[clojure.test :refer :all]
[metabase.driver.util :as driver.u]
[metabase.mbql.schema :as mbql.s]
[metabase.models :refer [Card]]
[metabase.query-processor :as qp]
......@@ -344,16 +345,17 @@
:collection "checkins"
:mbql? true}
:database (mt/id)}]
(t2.with-temp/with-temp [Card {card-id :id} {:dataset_query query}]
(is (= {:source-metadata nil
:source-query {:projections ["_id" "user_id" "venue_id"],
:native {:collection "checkins"
:query [{:$project {:_id "$_id"}}
{:$limit 1048575}]}
:collection "checkins"
:mbql? true}
:database (mt/id)}
(#'fetch-source-query/card-id->source-query-and-metadata card-id))))))
(with-redefs [driver.u/database->driver (constantly :mongo)]
(t2.with-temp/with-temp [Card {card-id :id} {:dataset_query query}]
(is (= {:source-metadata nil
:source-query {:projections ["_id" "user_id" "venue_id"],
:native {:collection "checkins"
:query [{:$project {:_id "$_id"}}
{:$limit 1048575}]}
:collection "checkins"
:mbql? true}
:database (mt/id)}
(#'fetch-source-query/card-id->source-query-and-metadata card-id)))))))
(testing "card-id->source-query-and-metadata-test should preserve mongodb native queries in string format (#30112)"
(let [query-str (str "[{\"$project\":\n"
" {\"_id\":\"$_id\",\n"
......@@ -364,10 +366,11 @@
:native {:query query-str
:collection "checkins"}
:database (mt/id)}]
(t2.with-temp/with-temp [Card {card-id :id} {:dataset_query query}]
(is (= {:source-metadata nil
:source-query {:native {:collection "checkins"
:query query-str}
:collection "checkins"}
:database (mt/id)}
(#'fetch-source-query/card-id->source-query-and-metadata card-id)))))))
(with-redefs [driver.u/database->driver (constantly :mongo)]
(t2.with-temp/with-temp [Card {card-id :id} {:dataset_query query}]
(is (= {:source-metadata nil
:source-query {:native {:collection "checkins"
:query query-str}
:collection "checkins"}
:database (mt/id)}
(#'fetch-source-query/card-id->source-query-and-metadata card-id))))))))
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment