Full support for all dimension types in expression editor (#10981)

* Full support for all dimension types in expression editor (field-id, field-literal, fk->, joined-field, expression)

* Fix various bugs in metabase-lib

* Limit 'card-has-ambiguous-columns?' to native queries + use real DB id in composeThisQuery

* replace metadata.database[...] etc with metadata.database(...)

* Fix and add to tests

* Revert "replace metadata.database[...] etc with metadata.database(...)"

This reverts commit 0f5aa69be184a1a767675e253eace47a419d970a.

* Fix joined dimension options, e.x. custom field from a saved query

frontend/src/metabase-lib/lib/Dimension.js

@@ -2,7 +2,7 @@ import { t, ngettext, msgid } from "ttag";
 import _ from "underscore";
 
 import { stripId, FK_SYMBOL } from "metabase/lib/formatting";
-import { getFriendlyName } from "metabase/visualizations/lib/utils";
+import { TYPE } from "metabase/lib/types";
 
 import Field from "./metadata/Field";
 import Metadata from "./metadata/Metadata";
@@ -608,6 +608,7 @@ export class FKDimension extends FieldDimension {
 }
 
 import { DATETIME_UNITS, formatBucketing } from "metabase/lib/query_time";
+import type Aggregation from "./queries/structured/Aggregation";
 
 const isFieldDimension = dimension =>
   dimension instanceof FieldIDDimension || dimension instanceof FKDimension;
@@ -808,8 +809,6 @@ export class ExpressionDimension extends Dimension {
   }
 }
 
-const INTEGER_AGGREGATIONS = new Set(["count", "cum-count", "distinct"]);
-
 /**
  * Aggregation reference, `["aggregation", aggregation-index]`
  */
@@ -828,30 +827,11 @@ export class AggregationDimension extends Dimension {
     return this._args[0];
   }
 
-  displayName(): string {
-    const name = this.columnName();
-    return name
-      ? getFriendlyName({ name: name, display_name: name })
-      : `[${t`Unknown`}]`;
-  }
-
-  fieldDimension() {
-    const aggregation = this.aggregation();
-    if (aggregation.length === 2 && aggregation[1]) {
-      return this.parseMBQL(aggregation[1]);
-    }
-    return null;
-  }
-
   column(extra = {}) {
-    const [short] = this.aggregation() || [];
+    const aggregation = this.aggregation();
     return {
       ...super.column(),
-      base_type: INTEGER_AGGREGATIONS.has(short)
-        ? "type/Integer"
-        : "type/Float",
-      display_name: short,
-      name: short,
+      base_type: aggregation ? aggregation.baseType() : TYPE.Float,
       source: "aggregation",
       ...extra,
     };
@@ -859,36 +839,40 @@ export class AggregationDimension extends Dimension {
 
   field() {
     // FIXME: it isn't really correct to return the unaggregated field. return a fake Field object?
-    const dimension = this.fieldDimension();
+    const dimension = this.aggregation().dimension();
     return dimension ? dimension.field() : super.field();
   }
 
-  // MBQL of the underlying aggregation
+  /**
+   * Raw aggregation
+   */
+  _aggregation(): Aggregation {
+    return this._query && this._query.aggregations()[this.aggregationIndex()];
+  }
+
+  /**
+   * Underlying aggregation, with aggregation-options removed
+   */
   aggregation() {
-    const aggregation =
-      this._query && this._query.aggregations()[this.aggregationIndex()];
+    const aggregation = this._aggregation();
     if (aggregation) {
-      return aggregation[0] === "aggregation-options"
-        ? aggregation[1]
-        : aggregation;
+      return aggregation.aggregation();
+    }
+    return null;
+  }
+
+  displayName(): string {
+    const aggregation = this._aggregation();
+    if (aggregation) {
+      return aggregation.displayName();
     }
     return null;
   }
 
   columnName() {
-    const aggregation =
-      this._query && this._query.aggregations()[this.aggregationIndex()];
+    const aggregation = this._aggregation();
     if (aggregation) {
-      // FIXME: query lib
-      if (aggregation[0] === "aggregation-options") {
-        const { "display-name": displayName } = aggregation[2];
-        if (displayName) {
-          return displayName;
-        }
-      }
-      const short = aggregation[0];
-      // NOTE: special case for "distinct"
-      return short === "distinct" ? "count" : short;
+      return aggregation.columnName();
     }
     return null;
   }

frontend/src/metabase-lib/lib/DimensionOptions.js

@@ -15,12 +15,12 @@ export default class DimensionOptions {
     Object.assign(this, o);
   }
 
-  allDimensions() {
+  all() {
     return [].concat(this.dimensions, ...this.fks.map(fk => fk.dimensions));
   }
 
   hasDimension(dimension: Dimension): boolean {
-    for (const d of this.allDimensions()) {
+    for (const d of this.all()) {
       if (dimension.isSameBaseDimension(d)) {
         return true;
       }

frontend/src/metabase-lib/lib/Question.js

@@ -464,14 +464,12 @@ export default class Question {
   }
 
   composeThisQuery(): ?Question {
-    const SAVED_QUESTIONS_FAUX_DATABASE = -1337;
-
     if (this.id()) {
       const card = {
         display: "table",
         dataset_query: {
           type: "query",
-          database: SAVED_QUESTIONS_FAUX_DATABASE,
+          database: this.databaseId(),
           query: {
             "source-table": "card__" + this.id(),
           },

frontend/src/metabase-lib/lib/metadata/Metric.js

@@ -23,6 +23,14 @@ export default class Metric extends Base {
     return ["metric", this.id];
   }
 
+  definitionQuery() {
+    return this.table.query().setQuery(this.definition);
+  }
+
+  aggregation() {
+    return this.definitionQuery().aggregations()[0];
+  }
+
   isActive(): boolean {
     return !this.archived;
   }

frontend/src/metabase-lib/lib/queries/structured/Aggregation.js

@@ -6,6 +6,8 @@ import { t } from "ttag";
 
 import * as A_DEPRECATED from "metabase/lib/query_aggregation";
 
+import { TYPE } from "metabase/lib/types";
+
 import type { Aggregation as AggregationObject } from "metabase/meta/types/Query";
 import type StructuredQuery from "../StructuredQuery";
 import type Dimension from "../../Dimension";
@@ -13,6 +15,8 @@ import type { AggregationOption } from "metabase/meta/types/Metadata";
 import type { MetricId } from "metabase/meta/types/Metric";
 import type { FieldId } from "metabase/meta/types/Field";
 
+const INTEGER_AGGREGATIONS = new Set(["count", "cum-count", "distinct"]);
+
 export default class Aggregation extends MBQLClause {
   /**
    * Replaces the aggregation in the parent query and returns the new StructuredQuery
@@ -50,20 +54,23 @@ export default class Aggregation extends MBQLClause {
    * Returns the display name for the aggregation
    */
   displayName() {
-    if (this.hasOptions()) {
-      return this.name() || this.aggregation().displayName();
-    } else if (this.isCustom()) {
-      return this._query.formatExpression(this);
-    } else if (this.isMetric()) {
-      const metric = this.metric();
+    const displayName = this.options()["display-name"];
+    if (displayName) {
+      return displayName;
+    }
+    const aggregation = this.aggregation();
+    if (aggregation.isCustom()) {
+      return aggregation._query.formatExpression(aggregation);
+    } else if (aggregation.isMetric()) {
+      const metric = aggregation.metric();
       if (metric) {
         return metric.displayName();
       }
-    } else if (this.isStandard()) {
-      const option = this.getOption();
+    } else if (aggregation.isStandard()) {
+      const option = aggregation.getOption();
       if (option) {
         const aggregationName = option.name.replace(" of ...", "");
-        const dimension = this.dimension();
+        const dimension = aggregation.dimension();
         if (dimension) {
           return t`${aggregationName} of ${dimension.displayName()}`;
         } else {
@@ -74,6 +81,52 @@ export default class Aggregation extends MBQLClause {
     return null;
   }
 
+  /**
+   * Returns the column name (non-deduplicated)
+   */
+  columnName() {
+    const displayName = this.options()["display-name"];
+    if (displayName) {
+      return displayName;
+    }
+    const aggregation = this.aggregation();
+    if (aggregation.isCustom()) {
+      return "expression";
+    } else if (aggregation.isMetric()) {
+      const metric = aggregation.metric();
+      if (metric) {
+        // delegate to the metric's definition
+        return metric.aggregation().columnName();
+      }
+    } else if (aggregation.isStandard()) {
+      const short = this.short();
+      if (short) {
+        // NOTE: special case for "distinct"
+        return short === "distinct" ? "count" : short;
+      }
+    }
+    return null;
+  }
+
+  short() {
+    const aggregation = this.aggregation();
+    // FIXME: if metric, this should be the underlying metric's short name?
+    if (aggregation.isMetric()) {
+      const metric = aggregation.metric();
+      if (metric) {
+        // delegate to the metric's definition
+        return metric.aggregation().short();
+      }
+    } else if (aggregation.isStandard()) {
+      return aggregation[0];
+    }
+  }
+
+  baseType() {
+    const short = this.short();
+    return INTEGER_AGGREGATIONS.has(short) ? TYPE.Integer : TYPE.Float;
+  }
+
   /**
    * Predicate function to test if a given aggregation clause is valid
    */
@@ -197,19 +250,6 @@ export default class Aggregation extends MBQLClause {
     }
   }
 
-  // NAMED
-
-  /**
-   * Returns true if this a named aggregation
-   */
-  isNamed() {
-    return !!this.options()["display-name"];
-  }
-
-  name() {
-    return this.options()["display-name"];
-  }
-
   /**
    * Returns the aggregation without "aggregation-options" clause, if any
    */

frontend/src/metabase-lib/lib/queries/structured/Join.js

@@ -5,6 +5,7 @@ import { t } from "ttag";
 
 import StructuredQuery from "../StructuredQuery";
 import Dimension, { JoinedDimension } from "metabase-lib/lib/Dimension";
+import DimensionOptions from "metabase-lib/lib/DimensionOptions";
 
 import { TableId } from "metabase/meta/types/Table";
 import type {
@@ -239,7 +240,7 @@ export default class Join extends MBQLObjectClause {
       options.count += fkOptions.count;
       options.fks.push(fkOptions);
     }
-    return options;
+    return new DimensionOptions(options);
   }
   setParentDimension(dimension: Dimension | ConcreteField): Join {
     if (dimension instanceof Dimension) {
@@ -274,11 +275,11 @@ export default class Join extends MBQLObjectClause {
   }
   joinDimensionOptions() {
     const dimensions = this.joinedDimensions();
-    return {
+    return new DimensionOptions({
       count: dimensions.length,
       dimensions: dimensions,
       fks: [],
-    };
+    });
   }
 
   // HELPERS
@@ -337,22 +338,21 @@ export default class Join extends MBQLObjectClause {
     dimensionFilter: (d: Dimension) => boolean = () => true,
   ) {
     const dimensions = this.joinedDimensions().filter(dimensionFilter);
-    return {
+    return new DimensionOptions({
       name: this.displayName(),
       icon: "join_left_outer",
       dimensions: dimensions,
       fks: [],
       count: dimensions.length,
-    };
+    });
   }
 
   joinedDimension(dimension: Dimension) {
-    return new JoinedDimension(
-      dimension,
-      [this.alias],
-      this.metadata(),
-      this.query(),
-    );
+    return this.query().parseFieldReference([
+      "joined-field",
+      this.alias,
+      dimension.mbql(),
+    ]);
   }
 
   dependentTableIds() {

frontend/src/metabase/lib/expressions/formatter.js

@@ -8,7 +8,7 @@ import {
   isMetric,
   isAggregation,
   formatMetricName,
-  formatIdentifier,
+  formatDimensionName,
 } from "../expressions";
 
 // convert a MBQL expression back into an expression string
@@ -52,7 +52,8 @@ function formatLiteral(expr) {
 
 function formatFieldReference(fieldRef, { query }) {
   if (query) {
-    return formatIdentifier(query.parseFieldReference(fieldRef).displayName());
+    const dimension = query.parseFieldReference(fieldRef);
+    return formatDimensionName(dimension);
   } else {
     throw new Error("`query` is a required parameter to format expressions");
   }

frontend/src/metabase/lib/expressions/index.js

@@ -17,6 +17,13 @@ export function getAggregationFromName(name) {
   return AGG_NAMES_MAP.get(name.toLowerCase());
 }
 
+export function getDimensionFromName(name, query) {
+  return query
+    .dimensionOptions()
+    .all()
+    .find(d => getDimensionName(d) === name);
+}
+
 export function isReservedWord(word) {
   return !!getAggregationFromName(word);
 }
@@ -35,12 +42,12 @@ export function formatMetricName(metric) {
   return formatIdentifier(metric.name);
 }
 
-export function formatFieldName(field) {
-  return formatIdentifier(field.display_name);
+export function formatDimensionName(dimension) {
+  return formatIdentifier(getDimensionName(dimension));
 }
 
-export function formatExpressionName(name) {
-  return formatIdentifier(name);
+export function getDimensionName(dimension) {
+  return dimension.render();
 }
 
 // move to query lib

frontend/src/metabase/lib/expressions/parser.js

@@ -3,12 +3,14 @@ import { Lexer, Parser, getImage } from "chevrotain";
 import _ from "underscore";
 import { t } from "ttag";
 import {
-  formatFieldName,
-  formatExpressionName,
+  // aggregations:
   formatAggregationName,
   getAggregationFromName,
+  // dimensions:
+  getDimensionFromName,
+  getDimensionName,
+  formatDimensionName,
 } from "../expressions";
-import { isNumeric } from "metabase/lib/schema_metadata";
 
 import {
   allTokens,
@@ -25,6 +27,8 @@ import {
   Identifier,
 } from "./tokens";
 
+import { ExpressionDimension } from "metabase-lib/lib/Dimension";
+
 const ExpressionsLexer = new Lexer(allTokens);
 
 class ExpressionsParser extends Parser {
@@ -124,21 +128,17 @@ class ExpressionsParser extends Parser {
       return this._unknownMetric(metricName);
     });
 
-    $.RULE("fieldExpression", () => {
-      const fieldName = $.OR([
+    $.RULE("dimensionExpression", () => {
+      const dimensionName = $.OR([
         { ALT: () => $.SUBRULE($.stringLiteral) },
         { ALT: () => $.SUBRULE($.identifier) },
       ]);
 
-      const field = this.getFieldForName(this._toString(fieldName));
-      if (field != null) {
-        return this._fieldReference(fieldName, field.id);
-      }
-      const expression = this.getExpressionForName(this._toString(fieldName));
-      if (expression != null) {
-        return this._expressionReference(fieldName, expression);
+      const dimension = this.getDimensionForName(this._toString(dimensionName));
+      if (dimension != null) {
+        return this._dimensionReference(dimensionName, dimension);
       }
-      return this._unknownField(fieldName);
+      return this._unknownField(dimensionName);
     });
 
     $.RULE("identifier", () => {
@@ -169,10 +169,10 @@ class ExpressionsParser extends Parser {
           // NOTE: DISABLE METRICS
           // {GATE: () => outsideAggregation, ALT: () => $.SUBRULE($.metricExpression) },
 
-          // fields are not allowed outside aggregations
+          // dimensions are not allowed outside aggregations
           {
             GATE: () => !outsideAggregation,
-            ALT: () => $.SUBRULE($.fieldExpression),
+            ALT: () => $.SUBRULE($.dimensionExpression),
           },
 
           {
@@ -180,8 +180,9 @@ class ExpressionsParser extends Parser {
           },
           { ALT: () => $.SUBRULE($.numberLiteral) },
         ],
-        (outsideAggregation ? "aggregation" : "field name") +
-          ", number, or expression",
+        outsideAggregation
+          ? "aggregation, number, or expression"
+          : "field name, number, or expression",
       );
     });
 
@@ -195,24 +196,16 @@ class ExpressionsParser extends Parser {
     Parser.performSelfAnalysis(this);
   }
 
-  getFieldForName(fieldName) {
-    const fields =
-      this._options.tableMetadata && this._options.tableMetadata.fields;
-    return _.findWhere(fields, { display_name: fieldName });
-  }
-
-  getExpressionForName(expressionName) {
-    const customFields = this._options && this._options.customFields;
-    return customFields[expressionName];
+  getDimensionForName(dimensionName) {
+    return getDimensionFromName(dimensionName, this._options.query);
   }
 
   getMetricForName(metricName) {
-    const metrics =
-      this._options.tableMetadata && this._options.tableMetadata.metrics;
-    return _.find(
-      metrics,
-      metric => metric.name.toLowerCase() === metricName.toLowerCase(),
-    );
+    return this._options.query
+      .table()
+      .metrics.find(
+        metric => metric.name.toLowerCase() === metricName.toLowerCase(),
+      );
   }
 }
 
@@ -235,11 +228,8 @@ class ExpressionsParserMBQL extends ExpressionsParser {
   _metricReference(metricName, metricId) {
     return ["metric", metricId];
   }
-  _fieldReference(fieldName, fieldId) {
-    return Array.isArray(fieldId) ? fieldId : ["field-id", fieldId];
-  }
-  _expressionReference(fieldName) {
-    return ["expression", fieldName];
+  _dimensionReference(dimensionName, dimension) {
+    return dimension.mbql();
   }
   _unknownField(fieldName) {
     throw new Error('Unknown field "' + fieldName + '"');
@@ -296,11 +286,8 @@ class ExpressionsParserSyntax extends ExpressionsParser {
   _metricReference(metricName, metricId) {
     return syntax("metric", metricName);
   }
-  _fieldReference(fieldName, fieldId) {
-    return syntax("field", fieldName);
-  }
-  _expressionReference(fieldName) {
-    return syntax("expression-reference", token(fieldName));
+  _dimensionReference(dimensionName, dimension) {
+    return syntax("field", dimensionName);
   }
   _unknownField(fieldName) {
     return syntax("unknown", fieldName);
@@ -378,7 +365,7 @@ export function parse(source, options = {}) {
 const parserInstance = new ExpressionsParser([]);
 export function suggest(
   source,
-  { tableMetadata, customFields, startRule, index = source.length } = {},
+  { query, startRule, index = source.length, expressionName } = {},
 ) {
   const partialSource = source.slice(0, index);
   const lexResult = ExpressionsLexer.tokenize(partialSource);
@@ -456,37 +443,31 @@ export function suggest(
       nextTokenType === StringLiteral
     ) {
       if (!outsideAggregation) {
-        let fields = [];
+        let dimensions = [];
         if (startRule === "aggregation" && currentAggregationToken) {
           const aggregationShort = getAggregationFromName(
             getImage(currentAggregationToken),
           );
-          const aggregationOption = _.findWhere(
-            tableMetadata.aggregation_options,
-            { short: aggregationShort },
-          );
-          fields =
-            (aggregationOption &&
-              aggregationOption.fields &&
-              aggregationOption.fields[0]) ||
-            [];
+          dimensions = query.aggregationFieldOptions(aggregationShort).all();
         } else if (startRule === "expression") {
-          fields = tableMetadata.fields.filter(isNumeric);
+          dimensions = query
+            .dimensionOptions(
+              d =>
+                // numeric
+                d.field().isNumeric() &&
+                // not itself
+                !(
+                  d instanceof ExpressionDimension &&
+                  d.name() === expressionName
+                ),
+            )
+            .all();
         }
         finalSuggestions.push(
-          ...fields.map(field => ({
+          ...dimensions.map(dimension => ({
             type: "fields",
-            name: field.display_name,
-            text: formatFieldName(field) + " ",
-            prefixTrim: /\w+$/,
-            postfixTrim: /^\w+\s*/,
-          })),
-        );
-        finalSuggestions.push(
-          ...Object.keys(customFields || {}).map(expressionName => ({
-            type: "fields",
-            name: expressionName,
-            text: formatExpressionName(expressionName) + " ",
+            name: getDimensionName(dimension),
+            text: formatDimensionName(dimension) + " ",
             prefixTrim: /\w+$/,
             postfixTrim: /^\w+\s*/,
           })),
@@ -501,7 +482,8 @@ export function suggest(
     ) {
       if (outsideAggregation) {
         finalSuggestions.push(
-          ...tableMetadata.aggregation_options
+          ...query
+            .aggregationOptionsWithoutRows()
             .filter(a => formatAggregationName(a))
             .map(aggregationOption => {
               const arity = aggregationOption.fields.length;
@@ -536,18 +518,19 @@ export function suggest(
   // throw away any suggestion that is not a suffix of the last partialToken.
   if (partialSuggestionMode) {
     const partial = getImage(lastInputToken).toLowerCase();
-    finalSuggestions = _.filter(
-      finalSuggestions,
-      suggestion =>
-        (suggestion.text &&
-          suggestion.text.toLowerCase().startsWith(partial)) ||
-        (suggestion.name && suggestion.name.toLowerCase().startsWith(partial)),
-    );
-
-    const prefixLength = partial.length;
     for (const suggestion of finalSuggestions) {
-      suggestion.prefixLength = prefixLength;
+      suggestion: for (const text of [suggestion.name, suggestion.text]) {
+        let index = 0;
+        for (const part of (text || "").toLowerCase().split(/\b/g)) {
+          if (part.startsWith(partial)) {
+            suggestion.range = [index, index + partial.length];
+            break suggestion;
+          }
+          index += part.length;
+        }
+      }
     }
+    finalSuggestions = finalSuggestions.filter(suggestion => suggestion.range);
   }
   for (const suggestion of finalSuggestions) {
     suggestion.index = index;

frontend/src/metabase/query_builder/actions.js

@@ -441,6 +441,15 @@ export const initializeQB = (location, params) => {
 
     /**** All actions are dispatched here ****/
 
+    // Fetch alerts for the current question if the question is saved
+    if (card && card.id != null) {
+      dispatch(fetchAlertsForQuestion(card.id));
+    }
+    // Fetch the question metadata (blocking)
+    if (card) {
+      await dispatch(loadMetadataForCard(card));
+    }
+
     // Update the question to Redux state together with the initial state of UI controls
     dispatch.action(INITIALIZE_QB, {
       card,
@@ -448,12 +457,6 @@ export const initializeQB = (location, params) => {
       uiControls,
     });
 
-    // Fetch alerts for the current question if the question is saved
-    card && card.id && dispatch(fetchAlertsForQuestion(card.id));
-
-    // Fetch the question metadata
-    card && dispatch(loadMetadataForCard(card));
-
     const question = card && new Question(getMetadata(getState()), card);
 
     // if we have loaded up a card that we can run then lets kick that off as well

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

@@ -345,8 +345,6 @@ export default class AggregationPopover extends Component {
               startRule="aggregation"
               expression={aggregation}
               query={query}
-              tableMetadata={tableMetadata}
-              customFields={customFields}
               onChange={parsedExpression =>
                 this.setState({
                   aggregation: A_DEPRECATED.setContent(

frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield.jsx

@@ -49,8 +49,6 @@ export default class ExpressionEditorTextfield extends Component {
 
   static propTypes = {
     expression: PropTypes.array, // should be an array like [parsedExpressionObj, expressionString]
-    tableMetadata: PropTypes.object.isRequired,
-    customFields: PropTypes.object,
     onChange: PropTypes.func.isRequired,
     onError: PropTypes.func.isRequired,
     startRule: PropTypes.string.isRequired,
@@ -66,8 +64,6 @@ export default class ExpressionEditorTextfield extends Component {
   _getParserInfo(props = this.props) {
     return {
       query: props.query,
-      tableMetadata: props.tableMetadata,
-      customFields: props.customFields || {},
       startRule: props.startRule,
     };
   }
@@ -77,12 +73,8 @@ export default class ExpressionEditorTextfield extends Component {
   }
 
   componentWillReceiveProps(newProps) {
-    // we only refresh our state if we had no previous state OR if our expression or table has changed
-    if (
-      !this.state ||
-      this.props.expression != newProps.expression ||
-      this.props.tableMetadata != newProps.tableMetadata
-    ) {
+    // we only refresh our state if we had no previous state OR if our expression changed
+    if (!this.state || this.props.expression != newProps.expression) {
       const parserInfo = this._getParserInfo(newProps);
       const parsedExpression = newProps.expression;
       const expressionString = format(newProps.expression, parserInfo);
@@ -348,19 +340,21 @@ export default class ExpressionEditorTextfield extends Component {
                     )}
                     onMouseDownCapture={e => this.onSuggestionMouseDown(e, i)}
                   >
-                    {suggestion.prefixLength ? (
+                    {suggestion.range ? (
                       <span>
+                        {suggestion.name.slice(0, suggestion.range[0])}
                         <span
                           className={cx("text-brand text-bold", {
                             "text-white bg-brand":
                               i === this.state.highlightedSuggestion,
                           })}
                         >
-                          {suggestion.name.slice(0, suggestion.prefixLength)}
-                        </span>
-                        <span>
-                          {suggestion.name.slice(suggestion.prefixLength)}
+                          {suggestion.name.slice(
+                            suggestion.range[0],
+                            suggestion.range[1],
+                          )}
                         </span>
+                        {suggestion.name.slice(suggestion.range[1])}
                       </span>
                     ) : (
                       suggestion.name

frontend/test/metabase/lib/expressions/parser.unit.spec.js

 import { compile, suggest, parse } from "metabase/lib/expressions/parser";
 import _ from "underscore";
+
 import { TYPE } from "metabase/lib/types";
+import {
+  ORDERS,
+  REVIEWS,
+  makeMetadata,
+} from "__support__/sample_dataset_fixture";
 
-const mockMetadata = {
-  tableMetadata: {
-    fields: [
-      { id: 1, display_name: "A", base_type: TYPE.Float },
-      { id: 2, display_name: "B", base_type: TYPE.Float },
-      { id: 3, display_name: "C", base_type: TYPE.Float },
-      { id: 10, display_name: "Toucan Sam", base_type: TYPE.Float },
-      { id: 11, display_name: "count", base_type: TYPE.Float },
-    ],
-    metrics: [{ id: 1, name: "foo bar" }],
-    aggregation_options: [
-      { short: "count", fields: [] },
-      { short: "sum", fields: [[]] },
-    ],
+const metadata = makeMetadata({
+  databases: {
+    1: {
+      name: "db",
+      tables: [1],
+      features: [
+        "basic-aggregations",
+        "standard-deviation-aggregations",
+        "expression-aggregations",
+        "foreign-keys",
+        "native-parameters",
+        "expressions",
+        "right-join",
+        "left-join",
+        "inner-join",
+        "nested-queries",
+      ],
+    },
+  },
+  tables: {
+    1: {
+      db: 1,
+      fields: [1, 2, 3, 10, 11],
+    },
+  },
+  fields: {
+    1: { id: 1, table: 1, display_name: "A", base_type: TYPE.Float },
+    2: { id: 2, table: 1, display_name: "B", base_type: TYPE.Float },
+    3: { id: 3, table: 1, display_name: "C", base_type: TYPE.Float },
+    10: { id: 10, table: 1, display_name: "Toucan Sam", base_type: TYPE.Float },
+    11: { id: 11, table: 1, display_name: "count", base_type: TYPE.Float },
   },
-};
+});
+
+const query = metadata.table(1).query();
 
-const expressionOpts = { ...mockMetadata, startRule: "expression" };
-const aggregationOpts = { ...mockMetadata, startRule: "aggregation" };
+const expressionOpts = { query, startRule: "expression" };
+const aggregationOpts = { query, startRule: "aggregation" };
 
 describe("lib/expressions/parser", () => {
   describe("compile()", () => {
@@ -157,10 +182,15 @@ describe("lib/expressions/parser", () => {
   describe("suggest()", () => {
     it("should suggest aggregations and metrics after an operator", () => {
       expect(cleanSuggestions(suggest("1 + ", aggregationOpts))).toEqual([
+        { type: "aggregations", text: "Average(" },
         { type: "aggregations", text: "Count " },
+        { type: "aggregations", text: "CumulativeCount " },
+        { type: "aggregations", text: "CumulativeSum(" },
+        { type: "aggregations", text: "Distinct(" },
+        { type: "aggregations", text: "Max(" },
+        { type: "aggregations", text: "Min(" },
+        { type: "aggregations", text: "StandardDeviation(" },
         { type: "aggregations", text: "Sum(" },
-        // NOTE: metrics support currently disabled
-        // { type: 'metrics',     text: '"foo bar"' },
         { type: "other", text: " (" },
       ]);
     });
@@ -179,6 +209,8 @@ describe("lib/expressions/parser", () => {
     it("should suggest partial matches in aggregation", () => {
       expect(cleanSuggestions(suggest("1 + C", aggregationOpts))).toEqual([
         { type: "aggregations", text: "Count " },
+        { type: "aggregations", text: "CumulativeCount " },
+        { type: "aggregations", text: "CumulativeSum(" },
       ]);
     });
     it("should suggest partial matches in expression", () => {
@@ -193,6 +225,52 @@ describe("lib/expressions/parser", () => {
         { type: "fields", text: "C " },
       ]);
     });
+    it("should suggest foreign fields", () => {
+      expect(
+        cleanSuggestions(
+          suggest("User", { query: ORDERS.query(), startRule: "expression" }),
+        ),
+      ).toEqual([
+        { text: '"User ID" ', type: "fields" },
+        { text: '"User → ID" ', type: "fields" },
+        { text: '"User → Latitude" ', type: "fields" },
+        { text: '"User → Longitude" ', type: "fields" },
+      ]);
+    });
+    it("should suggest joined fields", () => {
+      expect(
+        cleanSuggestions(
+          suggest("Foo", {
+            query: ORDERS.query().join({
+              alias: "Foo",
+              "source-table": REVIEWS.id,
+            }),
+            startRule: "expression",
+          }),
+        ),
+      ).toEqual([
+        { text: '"Foo → ID" ', type: "fields" },
+        { text: '"Foo → Product ID" ', type: "fields" },
+        { text: '"Foo → Rating" ', type: "fields" },
+      ]);
+    });
+    it("should suggest nested query fields", () => {
+      expect(
+        cleanSuggestions(
+          suggest("", {
+            query: ORDERS.query()
+              .aggregate(["count"])
+              .breakout(ORDERS.TOTAL)
+              .nest(),
+            startRule: "expression",
+          }),
+        ),
+      ).toEqual([
+        { text: '"Count of rows" ', type: "fields" },
+        { text: "Total ", type: "fields" },
+        { text: " (", type: "other" },
+      ]);
+    });
   });
 
   describe("compile() in syntax mode", () => {

src/metabase/api/database.clj

@@ -94,9 +94,10 @@
    would be ambiguous. Too many things break when attempting to use a query like this. In the future, this may be
    supported, but it will likely require rewriting the source SQL query to add appropriate aliases (this is even
    trickier if the source query uses `SELECT *`)."
-  [{result-metadata :result_metadata}]
-  (some (partial re-find #"_2$")
-        (map (comp name :name) result-metadata)))
+  [{result-metadata :result_metadata, dataset-query :dataset_query}]
+  (and (= (:type dataset-query) :native)
+       (some (partial re-find #"_2$")
+             (map (comp name :name) result-metadata))))
 
 (defn- card-uses-unnestable-aggregation?
   "Since cumulative count and cumulative sum aggregations are done in Clojure-land we can't use Cards that