Support for question references in native query template tags (#11835)

* Port tests from expectations to clojure.test

* Fix docstring

* Added :question template tag type with lookup

* s/question/card/

* Fix typo: s/substitue/substitute/g

* Wrap card query parameter data in a record

Just like `FieldFilter`s.

* Unrelated: Improve test layout

* Add substitution of card query parameters in queries

This implementation is incomplete.

* Finish substitution of card query in parent query

* Basic FE support for question template tags

* Update card ID on tag name update

Also, don't use `\d` in regexes.

* yarn prettier

* Add support for MBQL queries in template tag lookup

* Return query strings directly as tag value

* Fix query type lookup

* Initialize QP store for MBQL→native conversion

* Fix MBQL to native query conversion

* Port tests from expectations to clojure.test

* Add middleware to resolve tables/fields from referenced cards

This needs to happen before `substitute-parameters` in the QP pipeline,
so that `substitute-parameters` can convert any MBQL queries (from
referenced cards) into native queries. Maybe it should be consolidated
with the other resolution steps.

* Use mbql-query util instead of manual query def

* Add docstring to now public function

* Remove unused import

* Fix docstring positioning 



* Clean ns declaration

* Convert tests to clojure.test

* Be more specific about errors we're looking expecting

Invalid queries caused unexpected exceptions to be thrown when checking
permissions.

* Recursively check permissions of referenced queries

* Add incomplete test [WIP]

* Finish permission check test for referenced MBQL queries

* Add tests for referenced native queries perms check

* Clean ns

* Use existing mbql-to-native middleware

Rather than calling driver-specific `mbql->native` directly.

* Fix formatting

* Use more comprehensive qp func for converting query to native

* front end updates

* Check that all referenced queries are from the same db

* Add test for checking db id of referenced MBQL query

* Clean ns

* add metadata and better error states to UI

* move error message beneath the question picker

* Combine `vals`+`filter`+`map` into a single `keep`

* Rename `CardQuery`→`ReferencedCardQuery`

* Cosmetic: fix typo and formatting

* Test substitution of multiple sub-queries

* Test CTE syntax substitution of multiple sub-queries

* Clean ns

* Clean ns

* Simplify referenced card substitution tests

* Test recursive sub-query substitution

* Cosmetic: typos

* Fix parameter names in comments

* Check that queries don't include circular sub-queries

* Clean ns

* Give users a more user-friendly error for circular referencing sub-queries

* Test referencing queries with parameters

* Update src/metabase/query_processor/middleware/resolve_referenced.clj

Co-Authored-By: Maz Ameli <maz@metabase.com>

* Fix error message and combine card name queries

* Wrap sub-query error in a more user friendly explanation

* Cosmetic: function argument alignment

* Test error handling of referenced sub-queries

* Remove unused 1-arity version of `card-subquery-graph`

* Cosmetic: indentation

* Normalize l10n of error messages

* Reuse functions for looking up referenced cards from query

* implement FE PR feedback

* card -> card_id backend changes

* fix tests

* fix another test

* disallow space between # and number

* Align new middleware with newer QP architecture

* Align sub-query error handling with the new QP

It seems like exceptions are handled more consistently in the new QP, so
we can be less hacky.

* (Re)wrap sub-query errors in user-friendly expalanation

* Rename `xformf`→`rff` to align with QP changes

* force cypress click on select placeholder

* move popover attachement and cap its height

* icon spacing

* customize highlighting for template tags

Co-authored-by: Paul Rosenzweig <paulrosenzweig@users.noreply.github.com>
Co-authored-by: Maz Ameli <maz@metabase.com>

frontend/src/metabase-lib/lib/queries/NativeQuery.js

@@ -44,6 +44,21 @@ export const NATIVE_QUERY_TEMPLATE: NativeDatasetQuery = {
   },
 };
 
+// This regex needs to match logic in replaceCardId and _getUpdatedTemplateTags.
+const CARD_TAG_REGEX = /^#([0-9]*)$/;
+
+function cardTagCardId(name) {
+  const match = name.match(CARD_TAG_REGEX);
+  if (match && match[1].length > 0) {
+    return parseInt(match[1]);
+  }
+  return null;
+}
+
+function isCardQueryName(name) {
+  return CARD_TAG_REGEX.test(name);
+}
+
 export default class NativeQuery extends AtomicQuery {
   // For Flow type completion
   _nativeDatasetQuery: NativeDatasetQuery;
@@ -264,6 +279,14 @@ export default class NativeQuery extends AtomicQuery {
     return new NativeQuery(this._originalQuestion, datasetQuery);
   }
 
+  // `replaceCardId` updates the query text to reference a different card.
+  // Template tags are updated as a result of calling `setQueryText`.
+  replaceCardId(oldId, newId) {
+    const re = new RegExp(`{{\\s*#${oldId}\\s*}}`, "g");
+    const newQueryText = this.queryText().replace(re, () => `{{#${newId}}}`);
+    return this.setQueryText(newQueryText);
+  }
+
   dimensionOptions(
     dimensionFilter: DimensionFilter = () => true,
   ): DimensionOptions {
@@ -298,16 +321,16 @@ export default class NativeQuery extends AtomicQuery {
       // a variable name can optionally end with :start or :end which is not considered part of the actual variable name
       // expected pattern is like mustache templates, so we are looking for something like {{category}} or {{date:start}}
       // anything that doesn't match our rule is ignored, so {{&foo!}} would simply be ignored
+      // variables referencing other questions, by their card ID, are also supported: {{#123}} references question with ID 123
       let match;
-      const re = /\{\{\s*([A-Za-z0-9_]+?)\s*\}\}/g;
+      const re = /\{\{\s*([A-Za-z0-9_]+?|#[0-9]*)\s*\}\}/g;
       while ((match = re.exec(queryText)) != null) {
         tags.push(match[1]);
       }
 
       // eliminate any duplicates since it's allowed for a user to reference the same variable multiple times
-      const existingTemplateTags = this.templateTagsMap();
-
       tags = _.uniq(tags);
+      const existingTemplateTags = this.templateTagsMap();
       const existingTags = Object.keys(existingTemplateTags);
 
       // if we ended up with any variables in the query then update the card parameters list accordingly
@@ -318,13 +341,18 @@ export default class NativeQuery extends AtomicQuery {
         const templateTags = { ...existingTemplateTags };
         if (oldTags.length === 1 && newTags.length === 1) {
           // renaming
-          templateTags[newTags[0]] = { ...templateTags[oldTags[0]] };
+          const newTag = { ...templateTags[oldTags[0]] };
 
-          if (templateTags[newTags[0]].display_name === humanize(oldTags[0])) {
-            templateTags[newTags[0]].display_name = humanize(newTags[0]);
+          if (newTag.display_name === humanize(oldTags[0])) {
+            newTag.display_name = humanize(newTags[0]);
           }
 
-          templateTags[newTags[0]].name = newTags[0];
+          newTag.name = newTags[0];
+          if (isCardQueryName(newTag.name)) {
+            newTag.type = "card";
+            newTag.card_id = cardTagCardId(newTag.name);
+          }
+          templateTags[newTag.name] = newTag;
           delete templateTags[oldTags[0]];
         } else {
           // remove old vars
@@ -340,6 +368,14 @@ export default class NativeQuery extends AtomicQuery {
               display_name: humanize(tagName),
               type: "text",
             };
+
+            // parse card ID from tag name for card query template tags
+            if (isCardQueryName(tagName)) {
+              templateTags[tagName] = Object.assign(templateTags[tagName], {
+                type: "card",
+                card_id: cardTagCardId(tagName),
+              });
+            }
           }
         }
 

frontend/src/metabase/containers/QuestionPicker.jsx

@@ -3,9 +3,11 @@ import PropTypes from "prop-types";
 
 import ItemPicker from "./ItemPicker";
 
-const QuestionPicker = ({ value, onChange, ...props }) => (
+const QuestionPicker = ({ value, onChange, maxHeight, ...props }) => (
   <ItemPicker
     {...props}
+    // maxHeight is set when rendered in a popover
+    style={maxHeight != null ? { maxHeight } : {}}
     value={value === undefined ? undefined : { model: "card", id: value }}
     onChange={question => onChange(question ? question.id : undefined)}
     models={["card"]}

frontend/src/metabase/meta/Card.js

@@ -109,12 +109,14 @@ export function getTableMetadata(
 }
 
 export function getTemplateTags(card: ?Card): Array<TemplateTag> {
-  return card &&
+  const templateTags =
+    card &&
     card.dataset_query &&
     card.dataset_query.type === "native" &&
     card.dataset_query.native["template-tags"]
-    ? Object.values(card.dataset_query.native["template-tags"])
-    : [];
+      ? Object.values(card.dataset_query.native["template-tags"])
+      : [];
+  return templateTags.filter(tag => tag.type !== "card");
 }
 
 export function getParameters(card: ?Card): Parameter[] {

frontend/src/metabase/query_builder/components/NativeQueryEditor.css

@@ -17,6 +17,9 @@
 .NativeQueryEditor .ace_editor .ace_string {
   color: var(--color-saturated-green);
 }
+.NativeQueryEditor .ace_editor .ace_templateTag {
+  color: var(--color-brand);
+}
 
 .NativeQueryEditor .react-resizable {
   position: relative;

frontend/src/metabase/query_builder/components/NativeQueryEditor.jsx

@@ -186,11 +186,22 @@ export default class NativeQueryEditor extends Component {
       editorElement.classList.add("read-only");
     }
     const aceMode = query.aceMode();
-    if (this._editor.getSession().$modeId !== aceMode) {
-      this._editor.getSession().setMode(aceMode);
-      // monkey patch the mode to add our bracket/paren/braces-matching behavior
+    const session = this._editor.getSession();
+    if (session.$modeId !== aceMode) {
+      session.setMode(aceMode);
       if (aceMode.indexOf("sql") >= 0) {
-        this._editor.getSession().$mode.$behaviour = new SQLBehaviour();
+        // monkey patch the mode to add our bracket/paren/braces-matching behavior
+        session.$mode.$behaviour = new SQLBehaviour();
+
+        // add highlighting rule for template tags
+        session.$mode.$highlightRules.$rules.start.unshift({
+          token: "templateTag",
+          regex: "{{[^}]*}}",
+          onMatch: null,
+        });
+        session.$mode.$tokenizer = null;
+        session.bgTokenizer.setTokenizer(session.$mode.getTokenizer());
+        session.bgTokenizer.start(0);
       }
     }
 

frontend/src/metabase/query_builder/components/template_tags/CardTagEditor.jsx

+import React, { Component } from "react";
+import { Link } from "react-router";
+import { t } from "ttag";
+
+import Icon from "metabase/components/Icon";
+import Card from "metabase/components/Card";
+import QuestionPicker from "metabase/containers/QuestionPicker";
+import PopoverWithTrigger from "metabase/components/PopoverWithTrigger";
+import SelectButton from "metabase/components/SelectButton";
+import LoadingSpinner from "metabase/components/LoadingSpinner";
+
+import Questions from "metabase/entities/questions";
+import { question as questionUrl } from "metabase/lib/urls";
+import { formatDateTimeWithUnit } from "metabase/lib/formatting";
+import MetabaseSettings from "metabase/lib/settings";
+
+@Questions.load({
+  id: (state, { tag }) => tag.card_id,
+  loadingAndErrorWrapper: false,
+})
+export default class CardTagEditor extends Component {
+  handleQuestionSelection = id => {
+    const { question, query, setDatasetQuery } = this.props;
+    setDatasetQuery(
+      query.replaceCardId(question ? question.id : "", id).datasetQuery(),
+    );
+    this._popover && this._popover.close();
+  };
+
+  errorMessage() {
+    const { error, question, query } = this.props;
+
+    if (
+      question &&
+      // If this question was loaded by a search endpoint before fetching, it
+      // might not have a database_id set yet.
+      question.database_id != null &&
+      question.database_id !== query.databaseId()
+    ) {
+      return t`This question can't be used because it's based on a different database.`;
+    }
+    if (error) {
+      return error.status === 404
+        ? t`Couldn't find a saved question with that ID number.`
+        : error.data;
+    }
+    return null;
+  }
+
+  triggerElement() {
+    const { tag, question } = this.props;
+    return (
+      <SelectButton>
+        {tag.card_id == null ? (
+          <span className="text-medium">{t`Pick a saved question`}</span>
+        ) : this.errorMessage() ? (
+          <span className="text-medium">{t`Pick a different question`}</span>
+        ) : question ? (
+          question.name
+        ) : (
+          // we only hit this on the initial render before we fetch
+          t`Loading…`
+        )}
+      </SelectButton>
+    );
+  }
+
+  render() {
+    const {
+      tag: { card_id },
+      loading,
+      question,
+    } = this.props;
+
+    return (
+      <Card className="p2 mb2">
+        <h3 className="text-brand mb2">
+          {card_id == null ? (
+            t`Question #…`
+          ) : (
+            <Link to={questionUrl(card_id)}>{t`Question #${card_id}`}</Link>
+          )}
+        </h3>
+        {loading ? (
+          <LoadingSpinner />
+        ) : (
+          <PopoverWithTrigger
+            ref={ref => (this._popover = ref)}
+            triggerElement={this.triggerElement()}
+            verticalAttachments={["top", "bottom"]}
+            horizontalAttachments={["right", "left"]}
+            pinInitialAttachment
+          >
+            <QuestionPicker
+              className="p2"
+              value={question && question.id}
+              onChange={this.handleQuestionSelection}
+            />
+          </PopoverWithTrigger>
+        )}
+        {this.errorMessage() && (
+          <p className="text-error bg-light p2 mt2 mb0">
+            {this.errorMessage()}
+          </p>
+        )}
+        {question && !this.errorMessage() && (
+          <div className="bg-light text-medium py1 px2 mt2">
+            {question.collection && (
+              <div className="flex align-center">
+                <Icon name="all" size={12} mr={1} /> {question.collection.name}
+              </div>
+            )}
+            <div className="flex align-center">
+              <Icon name="calendar" size={12} mr={1} />{" "}
+              {t`Last edited ${formatDate(question.updated_at)}`}
+            </div>
+          </div>
+        )}
+      </Card>
+    );
+  }
+}
+
+// This formats a timestamp as a date using any custom formatting options.
+function formatDate(value) {
+  const options = MetabaseSettings.get("custom-formatting")["type/Temporal"];
+  return formatDateTimeWithUnit(value, "day", options);
+}

frontend/src/metabase/query_builder/components/template_tags/TagEditorSidebar.jsx

@@ -7,6 +7,7 @@ import { t } from "ttag";
 import cx from "classnames";
 
 import TagEditorParam from "./TagEditorParam";
+import CardTagEditor from "./CardTagEditor";
 import TagEditorHelp from "./TagEditorHelp";
 import SidebarContent from "metabase/query_builder/components/SidebarContent";
 
@@ -105,6 +106,8 @@ export default class TagEditorSidebar extends React.Component {
               databaseFields={databaseFields}
               database={database}
               databases={databases}
+              query={query}
+              setDatasetQuery={setDatasetQuery}
             />
           ) : (
             <TagEditorHelp
@@ -126,17 +129,27 @@ const SettingsPane = ({
   databaseFields,
   database,
   databases,
+  query,
+  setDatasetQuery,
 }) => (
   <div>
     {tags.map(tag => (
       <div key={tags.name}>
-        <TagEditorParam
-          tag={tag}
-          onUpdate={onUpdate}
-          databaseFields={databaseFields}
-          database={database}
-          databases={databases}
-        />
+        {tag.type === "card" ? (
+          <CardTagEditor
+            query={query}
+            setDatasetQuery={setDatasetQuery}
+            tag={tag}
+          />
+        ) : (
+          <TagEditorParam
+            tag={tag}
+            onUpdate={onUpdate}
+            databaseFields={databaseFields}
+            database={database}
+            databases={databases}
+          />
+        )}
       </div>
     ))}
   </div>
@@ -145,5 +158,7 @@ const SettingsPane = ({
 SettingsPane.propTypes = {
   tags: PropTypes.object.isRequired,
   onUpdate: PropTypes.func.isRequired,
+  setDatasetQuery: PropTypes.func.isRequired,
+  query: NativeQuery,
   databaseFields: PropTypes.array,
 };

frontend/test/metabase-lib/lib/queries/NativeQuery.unit.spec.js

@@ -212,6 +212,43 @@ describe("NativeQuery", () => {
       );
       expect(q.canRun()).toBe(true);
     });
+    describe("card template tags", () => {
+      it("should parse card tags", () => {
+        const q = makeQuery().setQueryText("{{#1}} {{ #2 }} {{ #1 }}");
+        expect(q.templateTags().map(v => v.card_id)).toEqual([1, 2]);
+      });
+    });
+    describe("replaceCardId", () => {
+      it("should update the query text", () => {
+        const query = makeQuery()
+          .setQueryText("SELECT * from {{ #123 }}")
+          .replaceCardId(123, 321);
+
+        expect(query.queryText()).toBe("SELECT * from {{#321}}");
+        const tags = query.templateTags();
+        expect(tags.length).toBe(1);
+        const [{ card_id, type, name }] = tags;
+        expect(card_id).toEqual(321);
+        expect(type).toEqual("card");
+        expect(name).toEqual("#321");
+      });
+
+      it("should perform multiple updates", () => {
+        const query = makeQuery()
+          .setQueryText("{{#123}} {{foo}} {{#1234}} {{ #123 }}")
+          .replaceCardId(123, 321);
+
+        expect(query.queryText()).toBe("{{#321}} {{foo}} {{#1234}} {{#321}}");
+      });
+
+      it("should replace a blank id", () => {
+        const query = makeQuery()
+          .setQueryText("{{#}} {{#123}}")
+          .replaceCardId("", 321);
+
+        expect(query.queryText()).toBe("{{#321}} {{#123}}");
+      });
+    });
   });
   describe("variables", () => {
     it("should return empty array if there are no tags", () => {

frontend/test/metabase/query_builder/components/NativeQueryEditor.cy.spec.js

-import { signInAsNormalUser } from "__support__/cypress";
+import { signInAsNormalUser, restore, popover } from "__support__/cypress";
 describe("NativeQueryEditor", () => {
+  before(restore);
   beforeEach(signInAsNormalUser);
 
   it("lets you create and run a SQL question", () => {
@@ -106,4 +107,46 @@ describe("NativeQueryEditor", () => {
       .last()
       .should("have.text", "bar");
   });
+
+  it("should show referenced cards in the template tag sidebar", () => {
+    cy.visit("/question/new");
+    cy.contains("Native query").click();
+
+    // start typing a question referenced
+    cy.get(".ace_content").type("select * from {{#}}", {
+      parseSpecialCharSequences: false,
+      delay: 0,
+    });
+
+    cy.contains("Question #…")
+      .parent()
+      .parent()
+      .contains("Pick a saved question")
+      .click({ force: true });
+
+    // selecting a question should update the query
+    popover()
+      .contains("Orders")
+      .click();
+
+    cy.contains("select * from {{#1}}");
+
+    // run query and see that a value from the results appears
+    cy.get(".NativeQueryEditor .Icon-play").click();
+    cy.contains("37.65");
+
+    // update the text of the query to reference question 2
+    // :visible is needed because there is an unused .ace_content present in the DOM
+    cy.get(".ace_content:visible").type("{leftarrow}{leftarrow}{backspace}2");
+
+    // sidebar should show updated question title and name
+    cy.contains("Question #2")
+      .parent()
+      .parent()
+      .contains("Orders, Count");
+
+    // run query again and see new result
+    cy.get(".NativeQueryEditor .Icon-play").click();
+    cy.contains("18,760");
+  });
 });

modules/drivers/mongo/src/metabase/driver/mongo.clj

@@ -214,7 +214,7 @@
     (with-mongo-connection [_ (qp.store/database)]
       (execute/execute-reducible-query query context respond))))
 
-(defmethod driver/substitue-native-parameters :mongo
+(defmethod driver/substitute-native-parameters :mongo
   [driver inner-query]
   (parameters/substitute-native-parameters driver inner-query))
 

modules/drivers/mongo/src/metabase/driver/mongo/parameters.clj

@@ -144,7 +144,7 @@
         (log/debug (tru "Substituted {0} -> {1}" (pr-str x) (pr-str <>)))))))
 
 (defn substitute-native-parameters
-  "Implementation of `driver/substitue-native-parameters` for MongoDB."
+  "Implementation of `driver/substitute-native-parameters` for MongoDB."
   [driver inner-query]
   (let [param->value (values/query->params-map inner-query)]
     (update inner-query :query (partial walk/postwalk (partial parse-and-substitute param->value)))))

src/metabase/driver.clj

@@ -563,7 +563,7 @@
 
 (defmethod current-db-time ::driver [_ _] nil)
 
-(defmulti substitue-native-parameters
+(defmulti substitute-native-parameters
   "For drivers that support `:native-parameters`. Substitute parameters in a normalized 'inner' native query.
 
     {:query \"SELECT count(*) FROM table WHERE id = {{param}}\"

src/metabase/driver/common/parameters.clj

@@ -27,6 +27,21 @@
   [x]
   (instance? FieldFilter x))
 
+;; A "ReferencedCardQuery" parameter expands to the native query of the referenced card.
+;;
+;; `card-id` is the ID of the Card instance whose query is the value for this parameter.
+;;
+;; `query` is the native query as stored in the Card
+(p.types/defrecord+ ReferencedCardQuery [card-id query]
+  PrettyPrintable
+  (pretty [this]
+    (list 'map->ReferencedCardQuery (into {} this))))
+
+(defn ReferencedCardQuery?
+  "Is `x` an instance of the `ReferencedCardQuery` record type?"
+  [x]
+  (instance? ReferencedCardQuery x))
+
 ;; as in a literal date, defined by date-string S
 ;;
 ;; TODO - why don't we just parse this into a Temporal type and let drivers handle it.

src/metabase/driver/common/parameters/values.clj

@@ -10,20 +10,25 @@
                                      :value  \"2015-01-01~2016-09-01\"}}}"
   (:require [clojure.string :as str]
             [metabase.driver.common.parameters :as i]
-            [metabase.models.field :refer [Field]]
+            [metabase.models
+             [card :refer [Card]]
+             [field :refer [Field]]]
+            [metabase.query-processor :as qp]
             [metabase.query-processor.error-type :as qp.error-type]
             [metabase.util
-             [i18n :as ui18n :refer [tru]]
+             [i18n :as ui18n :refer [deferred-tru]]
              [schema :as su]]
             [schema.core :as s]
             [toucan.db :as db])
-  (:import java.text.NumberFormat
+  (:import clojure.lang.ExceptionInfo
+           java.text.NumberFormat
            java.util.UUID
            [metabase.driver.common.parameters CommaSeparatedNumbers FieldFilter MultipleValues]))
 
 (def ^:private ParamType
   (s/enum :number
           :dimension                    ; Field Filter
+          :card
           :text
           :date))
 
@@ -43,6 +48,7 @@
     :display-name                 su/NonBlankString
     :type                         ParamType
     (s/optional-key :dimension)   [s/Any]
+    (s/optional-key :card-id)     su/IntGreaterThanZero
     (s/optional-key :widget-type) s/Keyword         ; type of the [default] value if `:type` itself is `dimension`
     (s/optional-key :required)    s/Bool
     (s/optional-key :default)     s/Any}
@@ -50,7 +56,7 @@
 
 (def ^:private ParsedParamValue
   "Schema for valid param value(s). Params can have one or more values."
-  (s/named (s/maybe (s/cond-pre i/SingleValue MultipleValues))
+  (s/named (s/maybe (s/cond-pre i/SingleValue MultipleValues su/Map))
            "Valid param value(s)"))
 
 (s/defn ^:private param-with-target
@@ -71,8 +77,9 @@
 ;;; FieldFilter Params (Field Filters) (e.g. WHERE {{x}})
 
 (defn- missing-required-param-exception [param-display-name]
-  (ex-info (tru "You''ll need to pick a value for ''{0}'' before this query can run." param-display-name)
-    {:type qp.error-type/missing-required-parameter}))
+  (ex-info (str (deferred-tru "You''ll need to pick a value for ''{0}'' before this query can run."
+                              param-display-name))
+           {:type qp.error-type/missing-required-parameter}))
 
 (s/defn ^:private default-value-for-field-filter
   "Return the default value for a FieldFilter (`:type` = `:dimension`) param defined by the map `tag`, if one is set."
@@ -97,8 +104,8 @@
      ;; TODO - shouldn't this use the QP Store?
      {:field (let [field-id (field-filter->field-id field-filter)]
                (or (db/select-one [Field :name :parent_id :table_id :base_type] :id field-id)
-                   (throw (ex-info (tru "Can''t find field with ID: {0}" field-id)
-                            {:field-id field-id, :type qp.error-type/invalid-parameter}))))
+                   (throw (ex-info (str (deferred-tru "Can''t find field with ID: {0}" field-id))
+                                   {:field-id field-id, :type qp.error-type/invalid-parameter}))))
       :value (if-let [value-info-or-infos (or
                                            ;; look in the sequence of params we were passed to see if there's anything
                                            ;; that matches
@@ -112,10 +119,29 @@
                ;;
                ;; (or it will be a vector of these maps for multiple values)
                (cond
-                 (map? value-info-or-infos)        (dissoc value-info-or-infos :target)
-                 (sequential? value-info-or-infos) (mapv #(dissoc % :target) value-info-or-infos))
+                (map? value-info-or-infos)        (dissoc value-info-or-infos :target)
+                (sequential? value-info-or-infos) (mapv #(dissoc % :target) value-info-or-infos))
                i/no-value)})))
 
+(s/defn ^:private card-query-for-tag :- (s/maybe (s/cond-pre su/Map (s/eq i/no-value)))
+  "Returns the native query for the `:card-id` referenced by the given tag."
+  [tag :- TagParam, _params :- (s/maybe [i/ParamValue])]
+  (when-let [card-id (:card-id tag)]
+    (when-let [query (db/select-one-field :dataset_query Card :id card-id)]
+      (try
+       (i/map->ReferencedCardQuery
+        {:card-id card-id
+         :query   (:query (qp/query->native query))})
+       (catch ExceptionInfo e
+         (throw (ex-info
+                 (str (deferred-tru
+                        "The sub-query from referenced question #{0} failed with the following error: {1}"
+                        (str card-id) (.getMessage e)))
+                 (merge (ex-data e)
+                        {:card-query-error? true
+                         :card-id           card-id
+                         :tag               tag}))))))))
+
 
 ;;; Non-FieldFilter Params (e.g. WHERE x = {{x}})
 
@@ -149,20 +175,20 @@
   converted to SQL as a simple comma-separated list.)"
   [value]
   (cond
-    ;; if not a string it's already been parsed
-    (number? value) value
-    ;; same goes for an instance of CommaSeperated values
-    (instance? CommaSeparatedNumbers value) value
-    ;; if the value is a string, then split it by commas in the string. Usually there should be none.
-    ;; Parse each part as a number.
-    (string? value)
-    (let [parts (for [part (str/split value #",")]
-                  (parse-number part))]
-      (if (> (count parts) 1)
-        ;; If there's more than one number return an instance of `CommaSeparatedNumbers`
-        (i/map->CommaSeparatedNumbers {:numbers parts})
-        ;; otherwise just return the single number
-        (first parts)))))
+   ;; if not a string it's already been parsed
+   (number? value) value
+   ;; same goes for an instance of CommaSeperated values
+   (instance? CommaSeparatedNumbers value) value
+   ;; if the value is a string, then split it by commas in the string. Usually there should be none.
+   ;; Parse each part as a number.
+   (string? value)
+   (let [parts (for [part (str/split value #",")]
+                 (parse-number part))]
+     (if (> (count parts) 1)
+       ;; If there's more than one number return an instance of `CommaSeparatedNumbers`
+       (i/map->CommaSeparatedNumbers {:numbers parts})
+       ;; otherwise just return the single number
+       (first parts)))))
 
 (s/defn ^:private parse-value-for-field-base-type :- s/Any
   "Do special parsing for value for a (presumably textual) FieldFilter (`:type` = `:dimension`) param (i.e., attempt
@@ -170,9 +196,9 @@
   handling types that do not have an associated parameter type (such as `date` or `number`), such as UUID fields."
   [base-type :- su/FieldType, value]
   (cond
-    (isa? base-type :type/UUID)   (UUID/fromString value)
-    (isa? base-type :type/Number) (value->number value)
-    :else                         value))
+   (isa? base-type :type/UUID)   (UUID/fromString value)
+   (isa? base-type :type/Number) (value->number value)
+   :else                         value))
 
 (s/defn ^:private parse-value-for-type :- ParsedParamValue
   "Parse a `value` based on the type chosen for the param, such as `text` or `number`. (Depending on the type of param
@@ -182,31 +208,31 @@
   base type Fields as UUIDs."
   [param-type :- ParamType, value]
   (cond
-    (= value i/no-value)
-    value
+   (= value i/no-value)
+   value
 
-    (= param-type :number)
-    (value->number value)
+   (= param-type :number)
+   (value->number value)
 
-    (= param-type :date)
-    (i/map->Date {:s value})
+   (= param-type :date)
+   (i/map->Date {:s value})
 
-    ;; Field Filters
-    (and (= param-type :dimension)
-         (= (get-in value [:value :type]) :number))
-    (update-in value [:value :value] value->number)
+   ;; Field Filters
+   (and (= param-type :dimension)
+        (= (get-in value [:value :type]) :number))
+   (update-in value [:value :value] value->number)
 
-    (sequential? value)
-    (i/map->MultipleValues {:values (for [v value]
-                                      (parse-value-for-type param-type v))})
+   (sequential? value)
+   (i/map->MultipleValues {:values (for [v value]
+                                     (parse-value-for-type param-type v))})
 
-    (and (= param-type :dimension)
-         (get-in value [:field :base_type])
-         (string? (get-in value [:value :value])))
-    (update-in value [:value :value] (partial parse-value-for-field-base-type (get-in value [:field :base_type])))
+   (and (= param-type :dimension)
+        (get-in value [:field :base_type])
+        (string? (get-in value [:value :value])))
+   (update-in value [:value :value] (partial parse-value-for-field-base-type (get-in value [:field :base_type])))
 
-    :else
-    value))
+   :else
+   value))
 
 (s/defn ^:private value-for-tag :- ParsedParamValue
   "Given a map `tag` (a value in the `:template-tags` dictionary) return the corresponding value from the `params`
@@ -214,6 +240,7 @@
   [tag :- TagParam, params :- (s/maybe [i/ParamValue])]
   (parse-value-for-type (:type tag) (or (param-value-for-tag tag params)
                                         (field-filter-value-for-tag tag params)
+                                        (card-query-for-tag tag params)
                                         (default-value-for-tag tag)
                                         i/no-value)))
 
@@ -225,15 +252,15 @@
     {:checkin_date #t \"2019-09-19T23:30:42.233-07:00\"}"
   [{tags :template-tags, params :parameters}]
   (try
-    (into {} (for [[k tag] tags
-                   :let    [v (value-for-tag tag params)]
-                   :when   v]
-               ;; TODO - if V is `nil` *on purpose* this still won't give us a query like `WHERE field = NULL`. That
-               ;; kind of query shouldn't be possible from the frontend anyway
-               {k v}))
-    (catch Throwable e
-      (throw (ex-info (.getMessage e)
-               {:type   (or (:type (ex-data e)) qp.error-type/invalid-parameter)
-                :tags   tags
-                :params params}
-               e)))))
+   (into {} (for [[k tag] tags
+                  :let    [v (value-for-tag tag params)]
+                  :when   v]
+              ;; TODO - if V is `nil` *on purpose* this still won't give us a query like `WHERE field = NULL`. That
+              ;; kind of query shouldn't be possible from the frontend anyway
+              {k v}))
+   (catch Throwable e
+     (throw (ex-info (.getMessage e)
+                     {:type   (or (:type (ex-data e)) qp.error-type/invalid-parameter)
+                      :tags   tags
+                      :params params}
+                     e)))))

src/metabase/driver/sql.clj

@@ -35,7 +35,7 @@
   [driver query]
   (sql.qp/mbql->native driver query))
 
-(defmethod driver/substitue-native-parameters :sql
+(defmethod driver/substitute-native-parameters :sql
   [_ {:keys [query] :as inner-query}]
   (let [[query params] (-> query
                            params.parse/parse

src/metabase/driver/sql/parameters/substitute.clj

@@ -14,6 +14,10 @@
     (let [{:keys [replacement-snippet prepared-statement-args]} (substitution/->replacement-snippet-info driver/*driver* v)]
       [(str sql replacement-snippet) (concat args prepared-statement-args) missing])))
 
+(defn- subsistute-card-query [[sql args missing] _in-optional? _k v]
+  (let [{:keys [replacement-snippet]} (substitution/->replacement-snippet-info driver/*driver* v)]
+    [(str sql replacement-snippet) args missing]))
+
 (defn- substitute-param [param->value [sql args missing] in-optional? {:keys [k]}]
   (if-not (contains? param->value k)
     [sql args (conj missing k)]
@@ -22,6 +26,9 @@
         (i/FieldFilter? v)
         (substitute-field-filter [sql args missing] in-optional? k v)
 
+        (i/ReferencedCardQuery? v)
+        (subsistute-card-query [sql args missing] in-optional? k v)
+
         (= i/no-value v)
         [sql args (conj missing k)]
 

src/metabase/driver/sql/parameters/substitution.clj

@@ -24,7 +24,8 @@
            honeysql.types.SqlCall
            java.time.temporal.Temporal
            java.util.UUID
-           [metabase.driver.common.parameters CommaSeparatedNumbers Date DateRange FieldFilter MultipleValues]))
+           [metabase.driver.common.parameters CommaSeparatedNumbers Date DateRange FieldFilter MultipleValues
+            ReferencedCardQuery]))
 
 ;;; ------------------------------------ ->prepared-substitution & default impls -------------------------------------
 
@@ -261,3 +262,11 @@
     :else
     (update (field-filter->replacement-snippet-info driver value)
             :replacement-snippet (partial str (field->identifier driver field (:type value)) " "))))
+
+
+;;; ------------------------------------- Field Filter replacement snippet info --------------------------------------
+
+(defmethod ->replacement-snippet-info [:sql ReferencedCardQuery]
+  [_driver {:keys [query]}]
+  {:prepared-statement-args nil
+   :replacement-snippet     (str "(" query ")")})

src/metabase/query_processor.clj

@@ -45,6 +45,7 @@
              [resolve-database-and-driver :as resolve-database-and-driver]
              [resolve-fields :as resolve-fields]
              [resolve-joins :as resolve-joins]
+             [resolve-referenced :as resolve-referenced]
              [resolve-source-table :as resolve-source-table]
              [results-metadata :as results-metadata]
              [splice-params-in-response :as splice-params-in-response]
@@ -83,6 +84,7 @@
    #'bucket-datetime/auto-bucket-datetimes
    #'resolve-source-table/resolve-source-tables
    #'parameters/substitute-parameters
+   #'resolve-referenced/resolve-referenced-card-resources
    #'expand-macros/expand-macros
    #'add-timezone-info/add-timezone-info
    #'splice-params-in-response/splice-params-in-response

src/metabase/query_processor/middleware/fetch_source_query.clj

@@ -216,7 +216,7 @@
     (recur (dissoc &match :database))))
 
 (s/defn ^:private resolve-all :- su/Map
-  "Recursively replace all Card ID source tables in `m` with resolved `:source-query` and `:source-metadata`. Since
+  "Recursively replace all Card ID source tables in `query` with resolved `:source-query` and `:source-metadata`. Since
   the `:database` is only useful for top-level source queries, we'll remove it from all other levels."
   [query :- su/Map]
   (-> query

src/metabase/query_processor/middleware/parameters/native.clj

@@ -36,6 +36,7 @@
     ;; Totally ridiculous, but top-level native queries use the key `:query` for SQL or equivalent, while native
     ;; source queries use `:native`. So we need to handle either case.
     (let [source-query?           (:native inner-query)
-          substituted-inner-query (driver/substitue-native-parameters driver/*driver* (set/rename-keys inner-query {:native :query}))]
+          substituted-inner-query (driver/substitute-native-parameters driver/*driver*
+                                                                       (set/rename-keys inner-query {:native :query}))]
       (cond-> (dissoc substituted-inner-query :parameters :template-tags)
         source-query? (set/rename-keys {:query :native})))))