Only show valid aggregations and fields

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

@@ -48,7 +48,7 @@ function formatField([, fieldId], { tableMetadata: { fields } }) {
     if (!field) {
         throw 'field with ID does not exist: ' + fieldId;
     }
-    return formatFieldName(field.display_name);
+    return formatFieldName(field);
 }
 
 function formatMetric([, metricId], { tableMetadata: { metrics } }) {
@@ -56,7 +56,7 @@ function formatMetric([, metricId], { tableMetadata: { metrics } }) {
     if (!metric) {
         throw 'metric with ID does not exist: ' + metricId;
     }
-    return formatMetricName(metric.name) + "()";
+    return formatMetricName(metric) + "()";
 }
 
 function formatExpressionReference([, expressionName]) {

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

@@ -16,23 +16,30 @@ export const VALID_AGGREGATIONS = new Map(Object.entries({
     "sum": "Sum",
     "cum_sum": "CumulativeSum",
     "distinct": "Distinct",
+    "stddev": "StandardDeviation",
     "avg": "Average",
     "min": "Min",
     "max": "Max"
 }));
 
-export function formatMetricName(metricName) {
-    return titleize(metricName).replace(/\W+/g, "")
+export function formatAggregationName(aggregationOption) {
+    return VALID_AGGREGATIONS.get(aggregationOption.short);
 }
 
-export function formatFieldName(fieldName) {
-    return /^\w+$/.test(fieldName) ?
-        fieldName :
-        JSON.stringify(fieldName);
+export function formatMetricName(metric) {
+    return titleize(metric.name).replace(/\W+/g, "")
+}
+
+export function formatFieldName(field) {
+    return /^\w+$/.test(field.display_name) ?
+        field.display_name :
+        JSON.stringify(field.display_name);
 }
 
 export function formatExpressionName(name) {
-    return formatFieldName(name);
+    return /^\w+$/.test(name) ?
+        name :
+        JSON.stringify(name);
 }
 
 // move to query lib

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

 const { Lexer, Parser, extendToken, getImage } = require("chevrotain");
 const _ = require("underscore");
 
-import { VALID_AGGREGATIONS, formatFieldName, formatMetricName, formatExpressionName } from "../expressions";
-
-export const AGGREGATION_ARITY = new Map([
-    ["Count", 0],
-    ["CumulativeCount", 0],
-    ["Sum", 1],
-    ["CumulativeSum", 1],
-    ["Distinct", 1],
-    ["Average", 1],
-    ["Min", 1],
-    ["Max", 1]
-]);
+import { VALID_AGGREGATIONS, formatFieldName, formatMetricName, formatExpressionName, formatAggregationName } from "../expressions";
 
 const AdditiveOperator = extendToken("AdditiveOperator", Lexer.NA);
 const Plus = extendToken("Plus", /\+/, AdditiveOperator);
@@ -190,7 +179,7 @@ class ExpressionsParser extends Parser {
 
     getMetricForName(metricName) {
         const metrics = this._options.tableMetadata && this._options.tableMetadata.metrics;
-        return _.find(metrics, (metric) => formatMetricName(metric.name) === metricName);
+        return _.find(metrics, (metric) => formatMetricName(metric) === metricName);
     }
 }
 
@@ -245,6 +234,9 @@ export function suggest(source, {
 
     let finalSuggestions = []
 
+    // TODO: is there a better way to figure out which aggregation we're inside of?
+    const currentAggregationToken = _.find(assistanceTokenVector.slice().reverse(), (t) => t instanceof Aggregation);
+
     for (const suggestion of syntacticSuggestions) {
         const { nextTokenType, ruleStack } = suggestion;
         // no nesting of aggregations or field references outside of aggregations
@@ -258,8 +250,8 @@ export function suggest(source, {
                 type: "operators",
                 name: getTokenSource(token),
                 text: " " + getTokenSource(token) + " ",
-                prefixTrim: /\s+$/,
-                postfixTrim: /^\s+/
+                prefixTrim: /\s*$/,
+                postfixTrim: /^\s*[*/+-]?\s*/
             })))
         } else if (nextTokenType === LParen) {
             finalSuggestions.push({
@@ -267,53 +259,55 @@ export function suggest(source, {
                 name: "(",
                 text: " (",
                 postfixText: ")",
-                prefixTrim: /\s+$/,
-                postfixTrim: /^\s+/
+                prefixTrim: /\s*$/,
+                postfixTrim: /^\s*\(?\s*/
             });
         } else if (nextTokenType === RParen) {
             finalSuggestions.push({
                 type: "other",
                 name: ")",
                 text: ") ",
-                prefixTrim: /\s+$/,
-                postfixTrim: /^\s+/
+                prefixTrim: /\s*$/,
+                postfixTrim: /^\s*\)?\s*/
             });
         } else if (nextTokenType === Identifier || nextTokenType === StringLiteral) {
-            if (!outsideAggregation) {
-                finalSuggestions.push(...tableMetadata.fields.map(field => ({
-                    type: "fields",
-                    name: field.display_name,
-                    text: formatFieldName(field.display_name) + " ",
-                    prefixTrim: /\w+$/,
-                    postfixTrim: /^\w+\s*/
-                })))
-                finalSuggestions.push(...Object.keys(customFields || {}).map(expressionName => ({
-                    type: "fields",
-                    name: expressionName,
-                    text: formatExpressionName(expressionName) + " ",
-                    prefixTrim: /\w+$/,
-                    postfixTrim: /^\w+\s*/
-                })))
+            if (!outsideAggregation && currentAggregationToken) {
+                let aggregationShort = aggregationsMap.get(getImage(currentAggregationToken));
+                let aggregationOption = _.findWhere(tableMetadata.aggregation_options, { short: aggregationShort });
+                if (aggregationOption && aggregationOption.fields.length > 0) {
+                    finalSuggestions.push(...aggregationOption.fields[0].map(field => ({
+                        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) + " ",
+                        prefixTrim: /\w+$/,
+                        postfixTrim: /^\w+\s*/
+                    })))
+                }
             }
         } else if (nextTokenType === Aggregation) {
             if (outsideAggregation) {
-                let tokens = getSubTokenTypes(nextTokenType);
-                finalSuggestions.push(...tokens.map(token => {
-                    let text = getTokenSource(token);
-                    let arity = AGGREGATION_ARITY.get(text);
+                finalSuggestions.push(...tableMetadata.aggregation_options.filter(a => formatAggregationName(a)).map(aggregationOption => {
+                    const arity = aggregationOption.fields.length;
                     return {
                         type: "aggregations",
-                        name: text,
-                        text: text + "(" + (arity > 0 ? "" : ")"),
+                        name: formatAggregationName(aggregationOption),
+                        text: formatAggregationName(aggregationOption) + "(" + (arity > 0 ? "" : ")"),
                         postfixText: arity > 0 ? ")" : "",
                         prefixTrim: /\w+$/,
                         postfixTrim: /^\w+\(\)?/
-                    }
+                    };
                 }));
                 finalSuggestions.push(...tableMetadata.metrics.map(metric => ({
                     type: "metrics",
                     name: metric.name,
-                    text: formatMetricName(metric.name) + "() ",
+                    text: formatMetricName(metric) + "() ",
                     prefixTrim: /\w+$/,
                     postfixTrim: /^\w+\(\)?/
                 })))

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

@@ -245,7 +245,7 @@ export default class ExpressionEditorTextfield extends Component {
                             targetAttachment: 'bottom left'
                         }}
                     >
-                        <ul style={{minWidth: 150, maxHeight: 342, overflow: "hidden"}}>
+                        <ul style={{minWidth: 150, overflow: "hidden"}}>
                             {suggestions.map((suggestion, i) =>
                                 // insert section title. assumes they're sorted by type
                                 [(i === 0 || suggestion.type !== suggestions[i - 1].type) &&