From cab00648345fa3c476ac083754f3f63c2f3fce63 Mon Sep 17 00:00:00 2001
From: Tom Robinson <tlrobinson@gmail.com>
Date: Sat, 10 Dec 2016 13:35:13 -0800
Subject: [PATCH] Only show valid aggregations and fields
---
.../src/metabase/lib/expressions/formatter.js | 4 +-
.../src/metabase/lib/expressions/index.js | 21 +++--
.../src/metabase/lib/expressions/parser.js | 78 +++++++++----------
.../expressions/ExpressionEditorTextfield.jsx | 2 +-
4 files changed, 53 insertions(+), 52 deletions(-)
diff --git a/frontend/src/metabase/lib/expressions/formatter.js b/frontend/src/metabase/lib/expressions/formatter.js
index 2e3a2f670c1..ae88983bdfb 100644
--- a/frontend/src/metabase/lib/expressions/formatter.js
+++ b/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]) {
diff --git a/frontend/src/metabase/lib/expressions/index.js b/frontend/src/metabase/lib/expressions/index.js
index 05b2e156484..c6d40065628 100644
--- a/frontend/src/metabase/lib/expressions/index.js
+++ b/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
diff --git a/frontend/src/metabase/lib/expressions/parser.js b/frontend/src/metabase/lib/expressions/parser.js
index 1b49afef099..3b5004c569e 100644
--- a/frontend/src/metabase/lib/expressions/parser.js
+++ b/frontend/src/metabase/lib/expressions/parser.js
@@ -1,18 +1,7 @@
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+\(\)?/
})))
diff --git a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield.jsx b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield.jsx
index dc5e8585aae..72fc3ccf5e0 100644
--- a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield.jsx
+++ b/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) &&
--
GitLab