diff --git a/backend/mbql/src/metabase/mbql/normalize.clj b/backend/mbql/src/metabase/mbql/normalize.clj index 9a39b41ae750f07c46ceb6cd3d514481109436c5..841b32da7a432aaeab86a89dc32cf1e5922c534c 100644 --- a/backend/mbql/src/metabase/mbql/normalize.clj +++ b/backend/mbql/src/metabase/mbql/normalize.clj @@ -140,7 +140,7 @@ (defn- aggregation-subclause? [x] (or (when ((some-fn keyword? string?) x) - (#{:avg :count :cum-count :distinct :stddev :sum :min :max :+ :- :/ :* :sum-where :count-where :share} (mbql.u/normalize-token x))) + (#{:avg :count :cum-count :distinct :stddev :sum :min :max :+ :- :/ :* :sum-where :count-where :share :var :median :percentile} (mbql.u/normalize-token x))) (when (mbql-clause? x) (aggregation-subclause? (first x))))) diff --git a/backend/mbql/src/metabase/mbql/schema.clj b/backend/mbql/src/metabase/mbql/schema.clj index 19d02b18da11f589b206fc69ecf7f926b83bdda5..7f3b076e3b0da0c61ef68454ee08a6380386d5af 100644 --- a/backend/mbql/src/metabase/mbql/schema.clj +++ b/backend/mbql/src/metabase/mbql/schema.clj @@ -277,7 +277,7 @@ (def string-expressions "String functions" - #{:substring :trim :rtrim :ltrim :upper :lower :replace :concat :regex-match-first :coalesce :length}) + #{:substring :trim :rtrim :ltrim :upper :lower :replace :concat :regex-match-first :coalesce}) (declare StringExpression) @@ -295,9 +295,12 @@ :else Field)) -(def ^:private arithmetic-expressions #{:+ :- :/ :* :coalesce}) +(def ^:private arithmetic-expressions #{:+ :- :/ :* :coalesce :length :round :ceil :floor :abs :power :sqrt :log :exp}) + +(def ^:private aggregations #{:sum :avg :stddev :var :median :percentile :min :max :cum-count :cum-sum :count-where :sum-where :share :distinct :metric :aggregation-options :count}) (declare ArithmeticExpression) +(declare Aggregation) (def ^:private NumericExpressionArg (s/conditional @@ -307,6 +310,9 @@ (partial is-clause? arithmetic-expressions) (s/recursive #'ArithmeticExpression) + (partial is-clause? aggregations) + (s/recursive #'Aggregation) + (partial is-clause? :value) value @@ -342,7 +348,7 @@ a ExpressionArg, b ExpressionArg, more (rest ExpressionArg)) (defclause ^{:requires-features #{:expressions}} substring - s StringExpressionArg, start s/Int, length (optional s/Int)) + s StringExpressionArg, start NumericExpressionArg, length (optional NumericExpressionArg)) (defclause ^{:requires-features #{:expressions}} length s StringExpressionArg) @@ -372,7 +378,7 @@ s StringExpressionArg, pattern s/Str) (def ^:private StringExpression* - (one-of substring trim ltrim rtrim replace lower upper concat regex-match-first coalesce length)) + (one-of substring trim ltrim rtrim replace lower upper concat regex-match-first coalesce)) (def ^:private StringExpression "Schema for the definition of an string expression." @@ -385,10 +391,35 @@ x NumericExpressionArg, y NumericExpressionArgOrInterval, more (rest NumericExpressionArgOrInterval)) (defclause ^{:requires-features #{:expressions}} /, x NumericExpressionArg, y NumericExpressionArg, more (rest NumericExpressionArg)) + (defclause ^{:requires-features #{:expressions}} *, x NumericExpressionArg, y NumericExpressionArg, more (rest NumericExpressionArg)) +(defclause ^{:requires-features #{:expressions}} floor + x NumericExpressionArg) + +(defclause ^{:requires-features #{:expressions}} ceil + x NumericExpressionArg) + +(defclause ^{:requires-features #{:expressions}} round + x NumericExpressionArg) + +(defclause ^{:requires-features #{:expressions}} abs + x NumericExpressionArg) + +(defclause ^{:requires-features #{:advanced-math-expressions}} power + x NumericExpressionArg, y NumericExpressionArg) + +(defclause ^{:requires-features #{:advanced-math-expressions}} sqrt + x NumericExpressionArg) + +(defclause ^{:requires-features #{:advanced-math-expressions}} exp + x NumericExpressionArg) + +(defclause ^{:requires-features #{:advanced-math-expressions}} log + x NumericExpressionArg) + (def ^:private ArithmeticExpression* - (one-of + - / * coalesce)) + (one-of + - / * coalesce length floor ceil round abs power sqrt exp log)) (def ^:private ArithmeticExpression "Schema for the definition of an arithmetic expression." @@ -425,6 +456,7 @@ s/Str DatetimeLiteral FieldOrRelativeDatetime + ExpressionArg value))) (def ^:private OrderComparible @@ -435,6 +467,7 @@ s/Num s/Str DatetimeLiteral + ExpressionArg FieldOrRelativeDatetime))) ;; For all of the non-compound Filter clauses below the first arg is an implicit Field ID @@ -448,20 +481,20 @@ ;; `!=` works like SQL `NOT IN` with more than 2 args ;; [:!= [:field-id 1] 2 3] --[DESUGAR]--> [:and [:!= [:field-id 1] 2] [:!= [:field-id 1] 3]] -(defclause =, field Field, value-or-field EqualityComparible, more-values-or-fields (rest EqualityComparible)) -(defclause !=, field Field, value-or-field EqualityComparible, more-values-or-fields (rest EqualityComparible)) +(defclause =, field EqualityComparible, value-or-field EqualityComparible, more-values-or-fields (rest EqualityComparible)) +(defclause !=, field EqualityComparible, value-or-field EqualityComparible, more-values-or-fields (rest EqualityComparible)) -(defclause <, field Field, value-or-field OrderComparible) -(defclause >, field Field, value-or-field OrderComparible) -(defclause <=, field Field, value-or-field OrderComparible) -(defclause >=, field Field, value-or-field OrderComparible) +(defclause <, field OrderComparible, value-or-field OrderComparible) +(defclause >, field OrderComparible, value-or-field OrderComparible) +(defclause <=, field OrderComparible, value-or-field OrderComparible) +(defclause >=, field OrderComparible, value-or-field OrderComparible) -(defclause between field Field, min OrderComparible, max OrderComparible) +(defclause between field OrderComparible, min OrderComparible, max OrderComparible) ;; SUGAR CLAUSE: This is automatically written as a pair of `:between` clauses by the `:desugar` middleware. (defclause ^:sugar inside - lat-field Field - lon-field Field + lat-field OrderComparible + lon-field OrderComparible lat-max OrderComparible lon-min OrderComparible lat-min OrderComparible @@ -585,6 +618,16 @@ (defclause ^{:requires-features #{:standard-deviation-aggregations}} stddev field-or-expression FieldOrExpressionDef) +(defclause ^{:requires-features #{:standard-deviation-aggregations}} var + field-or-expression FieldOrExpressionDef) + +(defclause ^{:requires-features #{:percentile-aggregations}} median + field-or-expression FieldOrExpressionDef) + +(defclause ^{:requires-features #{:percentile-aggregations}} percentile + field-or-expression FieldOrExpressionDef, percentile NumericExpressionArg) + + ;; Metrics are just 'macros' (placeholders for other aggregations with optional filter and breakout clauses) that get ;; expanded to other aggregations/etc. in the expand-macros middleware ;; @@ -594,30 +637,11 @@ ;; the following are definitions for expression aggregations, e.g. [:+ [:sum [:field-id 10]] [:sum [:field-id 20]]] -(declare Aggregation) - -(def ^:private ExpressionAggregationArg - (s/if number? - s/Num - (s/recursive #'Aggregation))) - -(defclause [^{:requires-features #{:expression-aggregations}} ag:+ +] - x ExpressionAggregationArg, y ExpressionAggregationArg, more (rest ExpressionAggregationArg)) - -(defclause [^{:requires-features #{:expression-aggregations}} ag:- -] - x ExpressionAggregationArg, y ExpressionAggregationArg, more (rest ExpressionAggregationArg)) - -(defclause [^{:requires-features #{:expression-aggregations}} ag:* *] - x ExpressionAggregationArg, y ExpressionAggregationArg, more (rest ExpressionAggregationArg)) - -(defclause [^{:requires-features #{:expression-aggregations}} ag:div /] - x ExpressionAggregationArg, y ExpressionAggregationArg, more (rest ExpressionAggregationArg)) -;; ag:/ isn't a valid token - - (def ^:private UnnamedAggregation* - (one-of count avg cum-count cum-sum distinct stddev sum min max ag:+ ag:- ag:* ag:div metric share count-where - sum-where case)) + (s/if (partial is-clause? arithmetic-expressions) + ArithmeticExpression + (one-of count avg cum-count cum-sum distinct stddev sum min max metric share count-where + sum-where case median percentile var))) (def ^:private UnnamedAggregation (s/recursive #'UnnamedAggregation*)) diff --git a/backend/mbql/test/metabase/mbql/normalize_test.clj b/backend/mbql/test/metabase/mbql/normalize_test.clj index e7006cf768a5cc745661a0436e38f2e0536de156..ac2f8697701270d67c34b4e8858e14f1f90f5080 100644 --- a/backend/mbql/test/metabase/mbql/normalize_test.clj +++ b/backend/mbql/test/metabase/mbql/normalize_test.clj @@ -157,6 +157,18 @@ [["=" ["field-id" 2] 1] "foo"]] {:default ["field-id" 2]}]]}})) +(expect + {:query {:aggregation [:median [:field-id 13]]}} + (#'normalize/normalize-tokens {:query {:aggregation ["median" ["field-id" 13]]}})) + +(expect + {:query {:aggregation [:var [:field-id 13]]}} + (#'normalize/normalize-tokens {:query {:aggregation ["var" ["field-id" 13]]}})) + +(expect + {:query {:aggregation [:percentile [:field-id 13] 0.9]}} + (#'normalize/normalize-tokens {:query {:aggregation ["percentile" ["field-id" 13] 0.9]}})) + ;;; ---------------------------------------------------- order-by ---------------------------------------------------- diff --git a/frontend/src/metabase-lib/lib/metadata/Database.js b/frontend/src/metabase-lib/lib/metadata/Database.js index 3dea8825ffca375292d5477ca16df0d3edb212d9..cf75c14d2f4c9dca8b7169e49fee6faef558a336 100644 --- a/frontend/src/metabase-lib/lib/metadata/Database.js +++ b/frontend/src/metabase-lib/lib/metadata/Database.js @@ -41,7 +41,12 @@ export default class Database extends Base { return this.schemas.map(s => s.name).sort((a, b) => a.localeCompare(b)); } - hasFeature(feature: DatabaseFeature | VirtualDatabaseFeature): boolean { + hasFeature( + feature: null | DatabaseFeature | VirtualDatabaseFeature, + ): boolean { + if (!feature) { + return true; + } const set = new Set(this.features); if (feature === "join") { return ( diff --git a/frontend/src/metabase-lib/lib/queries/StructuredQuery.js b/frontend/src/metabase-lib/lib/queries/StructuredQuery.js index ba0cab01d1ee06f6539ca199011f3e9b59b6989b..5ef28c49530f0d7ce919f698ce19d181c1ed63cd 100644 --- a/frontend/src/metabase-lib/lib/queries/StructuredQuery.js +++ b/frontend/src/metabase-lib/lib/queries/StructuredQuery.js @@ -63,7 +63,6 @@ import { augmentDatabase } from "metabase/lib/table"; import { TYPE } from "metabase/lib/types"; -import { isSegmentFilter } from "metabase/lib/query/filter"; import { fieldRefForColumn } from "metabase/lib/dataset"; type DimensionFilter = (dimension: Dimension) => boolean; @@ -693,15 +692,6 @@ export default class StructuredQuery extends AtomicQuery { return !this.hasAggregations() && !this.hasBreakouts(); } - /** - * @deprecated use this.aggregations()[index].displayName() directly - * @returns the formatted named of the aggregation at the provided index. - */ - aggregationName(index: number = 0): ?string { - const aggregation = this.aggregations()[index]; - return aggregation && aggregation.displayName(); - } - formatExpression(expression, { quotes = DISPLAY_QUOTES, ...options } = {}) { return formatExpression(expression, { quotes, ...options, query: this }); } @@ -886,8 +876,7 @@ export default class StructuredQuery extends AtomicQuery { if (filter && !(filter instanceof FilterWrapper)) { filter = new FilterWrapper(filter, null, this); } - const currentSegmentId = - filter && filter.isSegmentFilter() && filter.segmentId(); + const currentSegmentId = filter && filter.isSegment() && filter.segmentId(); return this.table().segments.filter( segment => (currentSegmentId != null && currentSegmentId === segment.id) || @@ -900,13 +889,8 @@ export default class StructuredQuery extends AtomicQuery { */ segments() { return this.filters() - .filter(f => isSegmentFilter(f)) - .map(segmentFilter => { - // segment id is stored as the second part of the filter clause - // e.x. ["segment", 1] - const segmentId = segmentFilter[1]; - return this.metadata().segment(segmentId); - }); + .filter(filter => filter.isSegment()) + .map(filter => filter.segment()); } /** diff --git a/frontend/src/metabase-lib/lib/queries/structured/Aggregation.js b/frontend/src/metabase-lib/lib/queries/structured/Aggregation.js index 0c731e44bd4ca07c18dff7df0d33d8501c8648dd..34226a6776f1bd624cb7f9b609cfe28f1c339aea 100644 --- a/frontend/src/metabase-lib/lib/queries/structured/Aggregation.js +++ b/frontend/src/metabase-lib/lib/queries/structured/Aggregation.js @@ -4,10 +4,10 @@ import MBQLClause from "./MBQLClause"; import { t } from "ttag"; -import * as A_DEPRECATED from "metabase/lib/query_aggregation"; - import { TYPE } from "metabase/lib/types"; +import { isStandard, isMetric, isCustom } from "metabase/lib/query/aggregation"; + import type { Aggregation as AggregationObject } from "metabase/meta/types/Query"; import type StructuredQuery from "../StructuredQuery"; import type Dimension from "../../Dimension"; @@ -154,15 +154,31 @@ export default class Aggregation extends MBQLClause { } } - // STANDARD AGGREGATION + // There are currently 3 "classes" of aggregations that are handled differently, "standard", "segment", and "custom" /** * Returns true if this is a "standard" metric */ isStandard(): boolean { - return A_DEPRECATED.isStandard(this); + return isStandard(this); + } + + /** + * Returns true if this is a metric + */ + isMetric(): boolean { + return isMetric(this); + } + + /** + * Returns true if this is custom expression created with the expression editor + */ + isCustom(): boolean { + return isCustom(this); } + // STANDARD AGGREGATION + /** * Gets the aggregation option matching this aggregation * Returns `null` if the clause isn't a "standard" metric @@ -209,13 +225,6 @@ export default class Aggregation extends MBQLClause { // METRIC AGGREGATION - /** - * Returns true if this is a metric - */ - isMetric(): boolean { - return this[0] === "metric"; - } - /** * Get metricId from a metric aggregation clause * Returns `null` if the clause doesn't represent a metric @@ -232,15 +241,6 @@ export default class Aggregation extends MBQLClause { } } - // CUSTOM - - /** - * Returns true if this is custom expression created with the expression editor - */ - isCustom(): boolean { - return A_DEPRECATED.isCustom(this); - } - // OPTIONS hasOptions() { diff --git a/frontend/src/metabase-lib/lib/queries/structured/Filter.js b/frontend/src/metabase-lib/lib/queries/structured/Filter.js index 7778731ab9be3fca26b6cc9a6d64fc361358948b..d7d898aa7693fa58cb6baac8d5bbfcf0974887a9 100644 --- a/frontend/src/metabase-lib/lib/queries/structured/Filter.js +++ b/frontend/src/metabase-lib/lib/queries/structured/Filter.js @@ -13,11 +13,14 @@ import type Dimension from "../../Dimension"; import { generateTimeFilterValuesDescriptions } from "metabase/lib/query_time"; import { - isSegmentFilter, - isCompoundFilter, + isStandard, + isSegment, + isCustom, isFieldFilter, hasFilterOptions, } from "metabase/lib/query/filter"; + +import { isExpression } from "metabase/lib/expressions"; import { getFilterArgumentFormatOptions } from "metabase/lib/schema_metadata"; import { t, ngettext, msgid } from "ttag"; @@ -54,10 +57,10 @@ export default class Filter extends MBQLClause { * Returns the display name for the filter */ displayName() { - if (this.isSegmentFilter()) { + if (this.isSegment()) { const segment = this.segment(); return segment ? segment.displayName() : t`Unknown Segment`; - } else if (this.isFieldFilter()) { + } else if (this.isStandard()) { const dimension = this.dimension(); const operator = this.operator(); const dimensionName = dimension && dimension.displayName(); @@ -75,9 +78,12 @@ export default class Filter extends MBQLClause { * Returns true if the filter is valid */ isValid() { - if (this.isFieldFilter()) { - // has an operator name and dimension + if (this.isStandard()) { + // has an operator name and dimension or expression const dimension = this.dimension(); + if (!dimension && isExpression(this[1])) { + return true; + } const query = this.query(); if ( !dimension || @@ -98,39 +104,60 @@ export default class Filter extends MBQLClause { } } return true; - } else if (this.isSegmentFilter()) { + } else if (this.isSegment()) { return !!this.segment(); - } else if (this.isCompoundFilter()) { - // TODO: compound filters + } else if (this.isCustom()) { return true; } return false; } - // SEGMENT FILTERS + // There are currently 3 "classes" of filters that are handled differently, "standard", "segment", and "custom" + + /** + * Returns true if this is a "standard" filter + */ + isStandard(): boolean { + return isStandard(this); + } + + /** + * Returns true if this is a segment + */ + isSegment(): boolean { + return isSegment(this); + } + + /** + * Returns true if this is custom filter created with the expression editor + */ + isCustom(): boolean { + return isCustom(this); + } - isSegmentFilter() { - return isSegmentFilter(this); + /** + * Returns true for filters where the first argument is a field + */ + isFieldFilter(): boolean { + return isFieldFilter(this); } + // SEGMENT FILTERS + segmentId() { - if (this.isSegmentFilter()) { + if (this.isSegment()) { return this[1]; } } segment() { - if (this.isSegmentFilter()) { + if (this.isSegment()) { return this.metadata().segment(this.segmentId()); } } // FIELD FILTERS - isFieldFilter() { - return isFieldFilter(this); - } - dimension(): ?Dimension { if (this.isFieldFilter()) { return this._query.parseFieldReference(this[1]); @@ -298,14 +325,4 @@ export default class Filter extends MBQLClause { : otherOperator && otherOperator.name; return operator && operator.name === operatorName; } - - // COMPOUND FILTER - - isCompoundFilter() { - return isCompoundFilter(this); - } - - isCustom() { - return this.isCompoundFilter(); - } } diff --git a/frontend/src/metabase/lib/expressions/config.js b/frontend/src/metabase/lib/expressions/config.js index 13aa8b9ac8f3a4a6c5210bb712a53578ffbcbf40..5eb511e5032d7e308d52e696f4fc2e821bf69aff 100644 --- a/frontend/src/metabase/lib/expressions/config.js +++ b/frontend/src/metabase/lib/expressions/config.js @@ -98,6 +98,7 @@ export const MBQL_CLAUSES = { displayName: `StandardDeviation`, type: "aggregation", args: ["number"], + requiresFeature: "standard-deviation-aggregations", }, avg: { displayName: `Average`, @@ -129,7 +130,25 @@ export const MBQL_CLAUSES = { type: "aggregation", args: ["number", "boolean"], }, - // expression functions + var: { + displayName: `Variance`, + type: "aggregation", + args: ["number"], + requiresFeature: "standard-deviation-aggregations", + }, + median: { + displayName: `Median`, + type: "aggregation", + args: ["number"], + requiresFeature: "percentile-aggregations", + }, + percentile: { + displayName: `Percentile`, + type: "aggregation", + args: ["number"], + requiresFeature: "percentile-aggregations", + }, + // string functions lower: { displayName: `lower`, type: "string", @@ -149,7 +168,7 @@ export const MBQL_CLAUSES = { displayName: `regexextract`, type: "string", args: ["string", "string"], - requiredFeatures: ["regex"], + requiresFeature: "regex", }, concat: { displayName: `concat`, @@ -157,17 +176,16 @@ export const MBQL_CLAUSES = { args: ["expression"], multiple: true, }, - coalesce: { - displayName: `coalesce`, - type: "expression", - args: ["expression", "expression"], - multiple: true, - }, replace: { displayName: `substitute`, type: "string", args: ["string", "string", "string"], }, + length: { + displayName: `length`, + type: "number", + args: ["string"], + }, trim: { displayName: `trim`, type: "string", @@ -183,13 +201,56 @@ export const MBQL_CLAUSES = { type: "string", args: ["string"], }, - case: { - displayName: `case`, - type: "expression", - args: ["expression", "expression"], // ideally we'd alternate boolean/expression - multiple: true, + // numeric functions + abs: { + displayName: `abs`, + type: "number", + args: ["number"], + requiresFeature: "expressions", + }, + floor: { + displayName: `floor`, + type: "number", + args: ["number"], + requiresFeature: "expressions", + }, + ceil: { + displayName: `ceil`, + type: "number", + args: ["number"], + requiresFeature: "expressions", + }, + round: { + displayName: `round`, + type: "number", + args: ["number"], + requiresFeature: "expressions", + }, + sqrt: { + displayName: `sqrt`, + type: "number", + args: ["number"], + requiresFeature: "advanced-math-expressions", + }, + power: { + displayName: `power`, + type: "number", + args: ["number", "number"], + requiresFeature: "advanced-math-expressions", }, - // filters functions + log: { + displayName: `log`, + type: "number", + args: ["number"], + requiresFeature: "advanced-math-expressions", + }, + exp: { + displayName: `exp`, + type: "number", + args: ["number"], + requiresFeature: "advanced-math-expressions", + }, + // boolean functions contains: { displayName: `contains`, type: "boolean", @@ -215,6 +276,19 @@ export const MBQL_CLAUSES = { type: "boolean", args: ["expression", "number", "string"], }, + // other expression functions + coalesce: { + displayName: `coalesce`, + type: "expression", + args: ["expression", "expression"], + multiple: true, + }, + case: { + displayName: `case`, + type: "expression", + args: ["expression", "expression"], // ideally we'd alternate boolean/expression + multiple: true, + }, // boolean operators and: { displayName: `AND`, @@ -231,7 +305,7 @@ export const MBQL_CLAUSES = { type: "boolean", args: ["boolean"], }, - // expression operators + // numeric operators "*": { displayName: "*", tokenName: "Multi", @@ -324,19 +398,6 @@ export function getMBQLName(expressionName) { return EXPRESSION_TO_MBQL_NAME.get(expressionName.toLowerCase()); } -export const EXPRESSION_FUNCTIONS = new Set([ - "lower", // concrete-field - "upper", // concrete-field - "substring", // concrete-field start length - "regex-match-first", // concrete-field regex - "concat", // & expression - "coalesce", // & expression - "replace", // concrete-field from to - "trim", // concrete-field - "rtrim", // concrete-field - "ltrim", // concrete-field -]); - export const AGGREGATION_FUNCTIONS = new Set([ // count-where/sum-where must come before count/sum "count-where", @@ -351,14 +412,40 @@ export const AGGREGATION_FUNCTIONS = new Set([ "min", "max", "share", + "var", + "median", + "percentile", ]); -export const FILTER_FUNCTIONS = new Set([ +export const EXPRESSION_FUNCTIONS = new Set([ + // string + "lower", + "upper", + "substring", + "regex-match-first", + "concat", + "replace", + "trim", + "rtrim", + "ltrim", + "length", + // number + "abs", + "floor", + "ceil", + "round", + "sqrt", + "power", + "log", + "exp", + // boolean "contains", "ends-with", "starts-with", "between", "time-interval", + // other + "coalesce", ]); export const EXPRESSION_OPERATORS = new Set(["+", "-", "*", "/"]); @@ -370,7 +457,6 @@ export const BOOLEAN_BINARY_OPERATORS = new Set(["and", "or"]); export const FUNCTIONS = new Set([ ...EXPRESSION_FUNCTIONS, ...AGGREGATION_FUNCTIONS, - ...FILTER_FUNCTIONS, ]); export const OPERATORS = new Set([ @@ -379,3 +465,35 @@ export const OPERATORS = new Set([ ...BOOLEAN_UNARY_OPERATORS, ...BOOLEAN_BINARY_OPERATORS, ]); + +// "standard" filters, can be edited using UI +export const STANDARD_FILTERS = new Set([ + "!=", + "<=", + ">=", + "<", + ">", + "=", + "contains", + "does-not-contain", + "ends-with", + "starts-with", + "between", + "time-interval", + "is-null", + "not-null", + "inside", +]); + +// "standard" aggregations, can be edited using UI +export const STANDARD_AGGREGATIONS = new Set([ + "count", + "cum-count", + "sum", + "cum-sum", + "distinct", + "stddev", + "avg", + "min", + "max", +]); diff --git a/frontend/src/metabase/lib/expressions/lexer.js b/frontend/src/metabase/lib/expressions/lexer.js index 33e984f607391f63be89bc7bada129fefafa1d7b..4be53be37b45b0a322421233d62858cb328b03f1 100644 --- a/frontend/src/metabase/lib/expressions/lexer.js +++ b/frontend/src/metabase/lib/expressions/lexer.js @@ -8,7 +8,6 @@ import { BOOLEAN_UNARY_OPERATORS, AGGREGATION_FUNCTIONS, EXPRESSION_FUNCTIONS, - FILTER_FUNCTIONS, MBQL_CLAUSES, EDITOR_QUOTES, } from "./config"; @@ -154,18 +153,6 @@ for (const clause of Array.from(EXPRESSION_FUNCTIONS)) { // special-case Case since it uses different syntax export const Case = createClauseToken("case"); -// FILTERS - -export const FilterFunctionName = createToken({ - name: "FilterFunctionName", - pattern: Lexer.NA, - categories: [FunctionName], -}); - -for (const clause of Array.from(FILTER_FUNCTIONS)) { - createClauseToken(clause, { categories: [FilterFunctionName] }); -} - // MISC export const Comma = createToken({ @@ -246,7 +233,6 @@ export const allTokens = [ FunctionName, AggregationFunctionName, ExpressionFunctionName, - FilterFunctionName, // all clauses ...CLAUSE_TOKENS.keys(), // literals diff --git a/frontend/src/metabase/lib/expressions/suggest.js b/frontend/src/metabase/lib/expressions/suggest.js index 5183be6b40173cccbbb1f2557e6337f011e627ae..609925846cd810c00b8e4a66800941329217f0e5 100644 --- a/frontend/src/metabase/lib/expressions/suggest.js +++ b/frontend/src/metabase/lib/expressions/suggest.js @@ -241,18 +241,28 @@ export function suggest({ isTokenType(nextTokenType, FunctionName) || nextTokenType === Case ) { + const database = query.database(); let functions = []; if (isExpressionType(expectedType, "aggregation")) { // special case for aggregation finalSuggestions.push( - ...query - .aggregationOperatorsWithoutRows() - .filter(a => getExpressionName(a.short)) - .map(aggregationOperator => + // ...query + // .aggregationOperatorsWithoutRows() + // .filter(a => getExpressionName(a.short)) + // .map(aggregationOperator => + // functionSuggestion( + // "aggregations", + // aggregationOperator.short, + // aggregationOperator.fields.length > 0, + // ), + // ), + ...FUNCTIONS_BY_TYPE["aggregation"] + .filter(clause => database.hasFeature(clause.requiresFeature)) + .map(clause => functionSuggestion( "aggregations", - aggregationOperator.short, - aggregationOperator.fields.length > 0, + clause.name, + clause.args.length > 0, ), ), ); @@ -265,14 +275,9 @@ export function suggest({ } else { functions = FUNCTIONS_BY_TYPE[expectedType]; } - const database = query.database(); finalSuggestions.push( ...functions - .filter( - clause => - !clause.requiredFeatures || - clause.requiredFeatures.every(f => database.hasFeature(f)), - ) + .filter(clause => database.hasFeature(clause.requiresFeature)) .map(clause => functionSuggestion("functions", clause.name)), ); } else if (nextTokenType === LParen) { diff --git a/frontend/src/metabase/lib/query/aggregation.js b/frontend/src/metabase/lib/query/aggregation.js index 955fc0274f1a1aa0cc5ff4772f331c8535bcdd92..3dc99e50d6cf38c45ac50f5a0f57b21b4e0de3c4 100644 --- a/frontend/src/metabase/lib/query/aggregation.js +++ b/frontend/src/metabase/lib/query/aggregation.js @@ -1,6 +1,9 @@ /* @flow */ import { noNullValues, add, update, remove, clear } from "./util"; +import { isValidField } from "./field_ref"; +import { STANDARD_AGGREGATIONS } from "metabase/lib/expressions"; + import _ from "underscore"; import type { AggregationClause, Aggregation } from "metabase/meta/types/Query"; @@ -71,3 +74,21 @@ export function hasEmptyAggregation(ac: ?AggregationClause): boolean { export function hasValidAggregation(ac: ?AggregationClause): boolean { return _.all(getAggregations(ac), aggregation => noNullValues(aggregation)); } + +// AGGREGATION TYPES + +export function isStandard(aggregation: AggregationClause): boolean { + return ( + Array.isArray(aggregation) && + STANDARD_AGGREGATIONS.has(aggregation[0]) && + (aggregation[1] === undefined || isValidField(aggregation[1])) + ); +} + +export function isMetric(aggregation: AggregationClause): boolean { + return Array.isArray(aggregation) && aggregation[0] === "metric"; +} + +export function isCustom(aggregation: AggregationClause): boolean { + return !isStandard(aggregation) && !isMetric(aggregation); +} diff --git a/frontend/src/metabase/lib/query/filter.js b/frontend/src/metabase/lib/query/filter.js index 2be5db243e0b7cd9697afe808bf9209b0aec3dd4..d85ee86c7ad0464a9cc585865ab212cd503fd160 100644 --- a/frontend/src/metabase/lib/query/filter.js +++ b/frontend/src/metabase/lib/query/filter.js @@ -1,6 +1,8 @@ /* @flow */ import { op, args, noNullValues, add, update, remove, clear } from "./util"; +import { isValidField } from "./field_ref"; +import { STANDARD_FILTERS } from "metabase/lib/expressions"; import type { FilterClause, @@ -63,21 +65,30 @@ export function canAddFilter(filter: ?FilterClause): boolean { return true; } -export function isSegmentFilter(filter: FilterClause): boolean { - return Array.isArray(filter) && filter[0] === "segment"; -} +// FILTER TYPES -export function isCompoundFilter(filter: FilterClause): boolean { +export function isStandard(filter: FilterClause): boolean { return ( Array.isArray(filter) && - (filter[0] === "and" || filter[0] === "or" || filter[0] === "not") + STANDARD_FILTERS.has(filter[0]) && + (filter[1] === undefined || isValidField(filter[1])) ); } +export function isSegment(filter: FilterClause): boolean { + return Array.isArray(filter) && filter[0] === "segment"; +} + +export function isCustom(filter: FilterClause): boolean { + return !isStandard(filter) && !isSegment(filter); +} + export function isFieldFilter(filter: FilterClause): boolean { - return !isSegmentFilter(filter) && !isCompoundFilter(filter); + return !isSegment(filter) && isValidField(filter[1]); } +// FILTER OPTIONS + // TODO: is it safe to assume if the last item is an object then it's options? export function hasFilterOptions(filter: Filter): boolean { const o = filter[filter.length - 1]; diff --git a/frontend/src/metabase/lib/query_aggregation.js b/frontend/src/metabase/lib/query_aggregation.js index fb881e0be0dcf7919d8f23f93821650fda17442b..18c74e7bd6b04490abe989d612b160d4473c069c 100644 --- a/frontend/src/metabase/lib/query_aggregation.js +++ b/frontend/src/metabase/lib/query_aggregation.js @@ -1,9 +1,14 @@ import { isValidField } from "./query/field_ref"; +// these are aggregations that can't only be added via custom aggregations export const SPECIAL_AGGREGATIONS = new Set([ "share", "sum-where", "count-where", + // TODO: add these to schema_metadata.js and remove from here + "var", + "median", + "percentile", ]); export function hasOptions(clause) { diff --git a/frontend/src/metabase/reference/selectors.js b/frontend/src/metabase/reference/selectors.js index a0ab2251e4f8ab68c5cfdb3f916f07b22aa375af..3f9f84f1303e3232876f2b45e7e82e4e3dc388df 100644 --- a/frontend/src/metabase/reference/selectors.js +++ b/frontend/src/metabase/reference/selectors.js @@ -152,7 +152,7 @@ export const getSegmentQuestions = createSelector( question => question.dataset_query.type === "query" && Query.getFilters(question.dataset_query.query).some( - filter => Filter.isSegmentFilter(filter) && filter[1] === segmentId, + filter => Filter.isSegment(filter) && filter[1] === segmentId, ), ) .reduce((map, question) => assoc(map, question.id, question), {}), diff --git a/frontend/test/__support__/sample_dataset_fixture.json b/frontend/test/__support__/sample_dataset_fixture.json index 87eb8fb931bf60de3b7cf45ae080978494057b55..866b0711fc7f9a019e0659d1a1e3be4902840c59 100644 --- a/frontend/test/__support__/sample_dataset_fixture.json +++ b/frontend/test/__support__/sample_dataset_fixture.json @@ -96,9 +96,11 @@ "basic-aggregations", "standard-deviation-aggregations", "expression-aggregations", + "percentile-aggregations", "foreign-keys", "native-parameters", "expressions", + "advanced-math-expressions", "right-join", "left-join", "inner-join", diff --git a/frontend/test/metabase-lib/lib/queries/StructuredQuery.unit.spec.js b/frontend/test/metabase-lib/lib/queries/StructuredQuery.unit.spec.js index 28d5b352a99ed997b83e3cd6878ebbd0958fc1f1..6cb3f49538796d1dc3324f3885d174d430777214 100644 --- a/frontend/test/metabase-lib/lib/queries/StructuredQuery.unit.spec.js +++ b/frontend/test/metabase-lib/lib/queries/StructuredQuery.unit.spec.js @@ -232,26 +232,26 @@ describe("StructuredQuery", () => { }); }); - describe("aggregationName", () => { + describe("aggregation name", () => { it("returns a saved metric's name", () => { expect( - makeQueryWithAggregation([ - "metric", - MAIN_METRIC_ID, - ]).aggregationName(), + makeQueryWithAggregation(["metric", MAIN_METRIC_ID]) + .aggregations()[0] + .displayName(), ).toBe("Total Order Value"); }); it("returns a standard aggregation name", () => { - expect(makeQueryWithAggregation(["count"]).aggregationName()).toBe( - "Count", - ); + expect( + makeQueryWithAggregation(["count"]) + .aggregations()[0] + .displayName(), + ).toBe("Count"); }); it("returns a standard aggregation name with field", () => { expect( - makeQueryWithAggregation([ - "sum", - ["field-id", ORDERS.TOTAL.id], - ]).aggregationName(), + makeQueryWithAggregation(["sum", ["field-id", ORDERS.TOTAL.id]]) + .aggregations()[0] + .displayName(), ).toBe("Sum of Total"); }); it("returns a standard aggregation name with fk field", () => { @@ -259,7 +259,9 @@ describe("StructuredQuery", () => { makeQueryWithAggregation([ "sum", ["fk->", ORDERS.PRODUCT_ID.id, PRODUCTS.TITLE.id], - ]).aggregationName(), + ]) + .aggregations()[0] + .displayName(), ).toBe("Sum of Product → Title"); }); it("returns a custom expression description", () => { @@ -268,7 +270,9 @@ describe("StructuredQuery", () => { "+", 1, ["sum", ["field-id", ORDERS.TOTAL.id]], - ]).aggregationName(), + ]) + .aggregations()[0] + .displayName(), ).toBe("1 + Sum(Total)"); }); it("returns a named expression name", () => { @@ -277,7 +281,9 @@ describe("StructuredQuery", () => { "aggregation-options", ["sum", ["field-id", ORDERS.TOTAL.id]], { "display-name": "Named" }, - ]).aggregationName(), + ]) + .aggregations()[0] + .displayName(), ).toBe("Named"); }); }); diff --git a/frontend/test/metabase-lib/lib/queries/structured/Filter.unit.spec.js b/frontend/test/metabase-lib/lib/queries/structured/Filter.unit.spec.js index 9b5b0165dd42c55c27a6c3aedd9650d6a984f950..734d0525fc07abee9a47545c1543bb16ab2c41d5 100644 --- a/frontend/test/metabase-lib/lib/queries/structured/Filter.unit.spec.js +++ b/frontend/test/metabase-lib/lib/queries/structured/Filter.unit.spec.js @@ -4,7 +4,7 @@ import { ORDERS } from "__support__/sample_dataset_fixture"; const query = ORDERS.query(); -function filterForMBQL(mbql) { +function filter(mbql) { return new Filter(mbql, 0, query); } @@ -12,34 +12,65 @@ describe("Filter", () => { describe("displayName", () => { it("should return the correct string for an = filter", () => { expect( - filterForMBQL(["=", ["field-id", ORDERS.TOTAL.id], 42]).displayName(), + filter(["=", ["field-id", ORDERS.TOTAL.id], 42]).displayName(), ).toEqual("Total is equal to 42"); }); it("should return the correct string for a segment filter", () => { - expect(filterForMBQL(["segment", 1]).displayName()).toEqual( - "Expensive Things", - ); + expect(filter(["segment", 1]).displayName()).toEqual("Expensive Things"); }); }); describe("isValid", () => { describe("with a field filter", () => { it("should return true for a field that exists", () => { - expect( - filterForMBQL(["=", ["field-id", ORDERS.TOTAL.id], 42]).isValid(), - ).toBe(true); + expect(filter(["=", ["field-id", ORDERS.TOTAL.id], 42]).isValid()).toBe( + true, + ); }); it("should return false for a field that doesn't exists", () => { - expect(filterForMBQL(["=", ["field-id", 12341234], 42]).isValid()).toBe( - false, - ); + expect(filter(["=", ["field-id", 12341234], 42]).isValid()).toBe(false); + }); + it("should return true for a filter with an expression for the field", () => { + expect( + filter(["=", ["/", ["field-id", 12341234], 43], 42]).isValid(), + ).toBe(true); }); }); }); describe("operator", () => { it("should return the correct FilterOperator", () => { expect( - filterForMBQL(["=", ["field-id", ORDERS.TOTAL.id], 42]).operator().name, + filter(["=", ["field-id", ORDERS.TOTAL.id], 42]).operator().name, ).toBe("="); }); }); + describe("setDimension", () => { + it("should set the dimension for existing filter clause", () => { + expect( + filter(["=", ["field-id", ORDERS.SUBTOTAL.id], 42]).setDimension( + ["field-id", ORDERS.TOTAL.id], + { + useDefaultOperator: true, + }, + ), + ).toEqual(["=", ["field-id", ORDERS.TOTAL.id], 42]); + }); + it("should set the dimension for new filter clause", () => { + expect(filter([]).setDimension(["field-id", ORDERS.TOTAL.id])).toEqual([ + null, + ["field-id", ORDERS.TOTAL.id], + ]); + }); + it("should set the dimension and default operator for empty filter clauses", () => { + expect( + filter([]).setDimension(["field-id", ORDERS.TOTAL.id], { + useDefaultOperator: true, + }), + ).toEqual(["=", ["field-id", ORDERS.TOTAL.id], undefined]); + }); + it("should set the dimension correctly when changing from segment", () => { + expect( + filter(["segment", 1]).setDimension(["field-id", ORDERS.TOTAL.id]), + ).toEqual([null, ["field-id", ORDERS.TOTAL.id]]); + }); + }); }); diff --git a/frontend/test/metabase/lib/expressions/__support__/expressions.js b/frontend/test/metabase/lib/expressions/__support__/expressions.js index e35be60b914b6cf366d0512fcab094e7d47967db..6bdd5bcc35945b433b62f8ada617b469bf733d7d 100644 --- a/frontend/test/metabase/lib/expressions/__support__/expressions.js +++ b/frontend/test/metabase/lib/expressions/__support__/expressions.js @@ -10,13 +10,16 @@ const metadata = makeMetadata({ "basic-aggregations", "standard-deviation-aggregations", "expression-aggregations", + "percentile-aggregations", "foreign-keys", "native-parameters", "expressions", + "advanced-math-expressions", "right-join", "left-join", "inner-join", "nested-queries", + "advanced-math-expressions", ], }, }, diff --git a/frontend/test/metabase/lib/expressions/__support__/shared.js b/frontend/test/metabase/lib/expressions/__support__/shared.js index ec6edcdc08d694798a524ceb691a5688c5be05c0..0c1f8723f7009388698b9a2f40b3a3dce3f03697 100644 --- a/frontend/test/metabase/lib/expressions/__support__/shared.js +++ b/frontend/test/metabase/lib/expressions/__support__/shared.js @@ -110,6 +110,17 @@ const aggregation = [ const filter = [ ["[Total] < 10", ["<", total, 10], "filter operator"], + // [ + // "floor([Total]) < 10", + // ["<", ["floor", total], 10], + // "filter operator with number function", + // ], + ["between([Subtotal], 1, 2)", ["between", subtotal, 1, 2], "filter function"], + [ + "between([Subtotal] - [Tax], 1, 2)", + ["between", ["-", subtotal, tax], 1, 2], + "filter function with math", + ], ["NOT [Total] < 10", ["not", ["<", total, 10]], "filter with not"], [ "[Total] < 10 AND [Tax] >= 1", diff --git a/frontend/test/metabase/lib/expressions/suggest.unit.spec.js b/frontend/test/metabase/lib/expressions/suggest.unit.spec.js index c5d21de72dc66213dbed25cf3639551c09d0d872..69366fd5c8bceebfd11cea26da351d6a59109f83 100644 --- a/frontend/test/metabase/lib/expressions/suggest.unit.spec.js +++ b/frontend/test/metabase/lib/expressions/suggest.unit.spec.js @@ -20,11 +20,14 @@ const AGGREGATION_FUNCTIONS = [ { type: "aggregations", text: "CumulativeSum(" }, { type: "aggregations", text: "Distinct(" }, { type: "aggregations", text: "Max(" }, + { type: "aggregations", text: "Median(" }, { type: "aggregations", text: "Min(" }, + { type: "aggregations", text: "Percentile(" }, { type: "aggregations", text: "Share(" }, { type: "aggregations", text: "StandardDeviation(" }, { type: "aggregations", text: "Sum(" }, { type: "aggregations", text: "SumIf(" }, + { type: "aggregations", text: "Variance(" }, ]; const STRING_FUNCTIONS = [ { text: "concat(", type: "functions" }, @@ -37,6 +40,18 @@ const STRING_FUNCTIONS = [ { text: "trim(", type: "functions" }, { text: "upper(", type: "functions" }, ]; +const NUMERIC_FUNCTIONS = [ + { text: "abs(", type: "functions" }, + { text: "ceil(", type: "functions" }, + { text: "exp(", type: "functions" }, + { text: "floor(", type: "functions" }, + { text: "length(", type: "functions" }, + { text: "log(", type: "functions" }, + { text: "power(", type: "functions" }, + { text: "round(", type: "functions" }, + { text: "sqrt(", type: "functions" }, +]; + const STRING_FUNCTIONS_EXCLUDING_REGEX = STRING_FUNCTIONS.filter( ({ text }) => text !== "regexextract(", ); @@ -141,8 +156,11 @@ describe("metabase/lib/expression/suggest", () => { expect(suggest({ source: "", ...expressionOpts })).toEqual([ ...FIELDS_CUSTOM, ...FIELDS_CUSTOM_NON_NUMERIC, - { type: "functions", text: "coalesce(" }, - ...STRING_FUNCTIONS_EXCLUDING_REGEX, + ...[ + { type: "functions", text: "coalesce(" }, + ...NUMERIC_FUNCTIONS, + ...STRING_FUNCTIONS_EXCLUDING_REGEX, + ].sort(suggestionSort), OPEN_PAREN, ]); }); @@ -150,6 +168,7 @@ describe("metabase/lib/expression/suggest", () => { it("should suggest numeric fields after an aritmetic", () => { expect(suggest({ source: "1 + ", ...expressionOpts })).toEqual([ ...FIELDS_CUSTOM, + ...NUMERIC_FUNCTIONS, OPEN_PAREN, ]); }); @@ -157,6 +176,7 @@ describe("metabase/lib/expression/suggest", () => { expect(suggest({ source: "1 + C", ...expressionOpts })).toEqual([ { type: "fields", text: "[C] " }, { type: "fields", text: "[count] " }, + { type: "functions", text: "ceil(" }, ]); }); it("should suggest partial matches in unterminated quoted string", () => { @@ -218,13 +238,16 @@ describe("metabase/lib/expression/suggest", () => { .nest(), startRule: "expression", }), - ).toEqual([ - { text: "[Count] ", type: "fields" }, - { text: "[Total] ", type: "fields" }, - { text: "coalesce(", type: "functions" }, - ...STRING_FUNCTIONS, - OPEN_PAREN, - ]); + ).toEqual( + [ + { text: "[Count] ", type: "fields" }, + { text: "[Total] ", type: "fields" }, + { text: "coalesce(", type: "functions" }, + ...STRING_FUNCTIONS, + ...NUMERIC_FUNCTIONS, + OPEN_PAREN, + ].sort(suggestionSort), + ); }); it("should suggest numeric operators after field in an expression", () => { expect( @@ -284,11 +307,13 @@ describe("metabase/lib/expression/suggest", () => { { type: "fields", text: "[count] " }, // { text: "case(", type: "functions" }, // { text: "coalesce(", type: "functions" }, + { text: "ceil(", type: "functions" }, ]); }); it("should suggest aggregations and metrics after an operator", () => { expect(suggest({ source: "1 + ", ...aggregationOpts })).toEqual([ ...AGGREGATION_FUNCTIONS, + ...NUMERIC_FUNCTIONS, ...METRICS_CUSTOM, OPEN_PAREN, ]); @@ -296,6 +321,7 @@ describe("metabase/lib/expression/suggest", () => { it("should suggest fields after an aggregation without closing paren", () => { expect(suggest({ source: "Average(", ...aggregationOpts })).toEqual([ ...FIELDS_CUSTOM, + ...NUMERIC_FUNCTIONS, OPEN_PAREN, CLOSE_PAREN, ]); @@ -303,7 +329,12 @@ describe("metabase/lib/expression/suggest", () => { it("should suggest fields after an aggregation with closing paren", () => { expect( suggest({ source: "Average()", ...aggregationOpts, targetOffset: 8 }), - ).toEqual([...FIELDS_CUSTOM, OPEN_PAREN, CLOSE_PAREN]); + ).toEqual([ + ...FIELDS_CUSTOM, + ...NUMERIC_FUNCTIONS, + OPEN_PAREN, + CLOSE_PAREN, + ]); }); it("should suggest partial matches in aggregation", () => { expect(suggest({ source: "1 + C", ...aggregationOpts })).toEqual([ @@ -311,6 +342,7 @@ describe("metabase/lib/expression/suggest", () => { { type: "aggregations", text: "CountIf(" }, { type: "aggregations", text: "CumulativeCount " }, { type: "aggregations", text: "CumulativeSum(" }, + { type: "functions", text: "ceil(" }, ]); }); @@ -321,7 +353,12 @@ describe("metabase/lib/expression/suggest", () => { query: ORDERS.query(), startRule: "aggregation", }), - ).toEqual([...AGGREGATION_FUNCTIONS, ...METRICS_ORDERS, OPEN_PAREN]); + ).toEqual([ + ...AGGREGATION_FUNCTIONS, + ...NUMERIC_FUNCTIONS, + ...METRICS_ORDERS, + OPEN_PAREN, + ]); }); it("should show help text in an aggregation functiom", () => { @@ -497,3 +534,6 @@ function cleanSuggestions(suggestions) { .sortBy("type") .value(); } + +const suggestionSort = (a, b) => + a.type.localeCompare(b.type) || a.text.localeCompare(b.text); diff --git a/modules/drivers/bigquery/src/metabase/driver/bigquery.clj b/modules/drivers/bigquery/src/metabase/driver/bigquery.clj index b7fe7e5224c2cf372b55a8ab5a4e4bb80a9819bf..e83de7ed394a5fef18b51b6b604b3547ab4938c8 100644 --- a/modules/drivers/bigquery/src/metabase/driver/bigquery.clj +++ b/modules/drivers/bigquery/src/metabase/driver/bigquery.clj @@ -27,6 +27,9 @@ (driver/register! :bigquery, :parent #{:google :sql}) +(defmethod driver/supports? [:bigquery :percentile-aggregations] [_ _] false) + + ;;; +----------------------------------------------------------------------------------------------------------------+ ;;; | Client | ;;; +----------------------------------------------------------------------------------------------------------------+ diff --git a/modules/drivers/bigquery/src/metabase/driver/bigquery/query_processor.clj b/modules/drivers/bigquery/src/metabase/driver/bigquery/query_processor.clj index 34c0b21da3dfb24e0dc1bbb3fff6352f2b5ce36b..f52bf0e40856d6e966912ecad7c8a597198977dd 100644 --- a/modules/drivers/bigquery/src/metabase/driver/bigquery/query_processor.clj +++ b/modules/drivers/bigquery/src/metabase/driver/bigquery/query_processor.clj @@ -351,11 +351,30 @@ [_ value] (hx/cast :float64 value)) - (defmethod sql.qp/->honeysql [:bigquery :regex-match-first] [driver [_ arg pattern]] (hsql/call :regexp_extract (sql.qp/->honeysql driver arg) (sql.qp/->honeysql driver pattern))) +(defn- percentile->quantile + [x] + (loop [x (double x) + power (int 0)] + (if (zero? (- x (Math/floor x))) + [(Math/round x) (Math/round (Math/pow 10 power))] + (recur (* 10 x) (inc power))))) + +(defmethod sql.qp/->honeysql [:bigquery :percentile] + [driver [_ arg p]] + (let [[offset quantiles] (percentile->quantile p)] + (hsql/raw (format "APPROX_QUANTILES(%s, %s)[OFFSET(%s)]" + (hformat/to-sql (sql.qp/->honeysql driver arg)) + quantiles + offset)))) + +(defmethod sql.qp/->honeysql [:bigquery :median] + [driver [_ arg]] + (sql.qp/->honeysql driver [:percentile arg 0.5])) + ;;; +----------------------------------------------------------------------------------------------------------------+ ;;; | Query Processor | diff --git a/modules/drivers/presto/src/metabase/driver/presto.clj b/modules/drivers/presto/src/metabase/driver/presto.clj index cd4888c8f2f875b1adc4db2e24401a8a2dba5905..8c946a67d303dad65eff73ac8da82974ea0efef3 100644 --- a/modules/drivers/presto/src/metabase/driver/presto.clj +++ b/modules/drivers/presto/src/metabase/driver/presto.clj @@ -260,10 +260,6 @@ [_ bool] (hsql/raw (if bool "TRUE" "FALSE"))) -(defmethod sql.qp/->honeysql [:presto :stddev] - [driver [_ field]] - (hsql/call :stddev_samp (sql.qp/->honeysql driver field))) - (defmethod sql.qp/->honeysql [:presto :time] [_ [_ t]] (hx/cast :time (u.date/format-sql (t/local-time t)))) @@ -276,6 +272,13 @@ [driver [_ arg pattern]] (hsql/call :regexp_extract (sql.qp/->honeysql driver arg) (sql.qp/->honeysql driver pattern))) +(defmethod sql.qp/->honeysql [:presto :median] + [driver [_ arg]] + (hsql/call :approx_percentile (sql.qp/->honeysql driver arg) 0.5)) + +(defmethod sql.qp/->honeysql [:presto :percentile] + [driver [_ arg p]] + (hsql/call :approx_percentile (sql.qp/->honeysql driver arg) (sql.qp/->honeysql driver p))) ;; See https://prestodb.io/docs/current/functions/datetime.html diff --git a/modules/drivers/snowflake/src/metabase/driver/snowflake.clj b/modules/drivers/snowflake/src/metabase/driver/snowflake.clj index 4af47b5ee157ea5347bda563d8a8c50914d66bba..bd568540de5a9c860a7732e84d797eabdd6cf1d7 100644 --- a/modules/drivers/snowflake/src/metabase/driver/snowflake.clj +++ b/modules/drivers/snowflake/src/metabase/driver/snowflake.clj @@ -137,6 +137,10 @@ [driver [_ arg pattern]] (hsql/call :regexp_substr (sql.qp/->honeysql driver arg) (sql.qp/->honeysql driver pattern))) +(defmethod sql.qp/->honeysql [:snowflake :median] + [driver [_ arg]] + (sql.qp/->honeysql driver [:percentile arg 0.5])) + (defn- db-name "As mentioned above, old versions of the Snowflake driver used `details.dbname` to specify the physical database, but tests (and Snowflake itself) expected `details.db`. This has since been fixed, but for legacy support we'll still diff --git a/modules/drivers/sparksql/src/metabase/driver/hive_like.clj b/modules/drivers/sparksql/src/metabase/driver/hive_like.clj index dbfb2bcff85ca2c4aa1c3600a593e696f17366ee..0a1a83e5fcc900f9ae3f4644c1ab848e764817bb 100644 --- a/modules/drivers/sparksql/src/metabase/driver/hive_like.clj +++ b/modules/drivers/sparksql/src/metabase/driver/hive_like.clj @@ -117,6 +117,14 @@ [driver [_ arg pattern]] (hsql/call :regexp_extract (sql.qp/->honeysql driver arg) (sql.qp/->honeysql driver pattern))) +(defmethod sql.qp/->honeysql [:hive-like :median] + [driver [_ arg]] + (hsql/call :percentile (sql.qp/->honeysql driver arg) 0.5)) + +(defmethod sql.qp/->honeysql [:hive-like :percentile] + [driver [_ arg p]] + (hsql/call :percentile (sql.qp/->honeysql driver arg) (sql.qp/->honeysql driver p))) + (defmethod sql.qp/add-interval-honeysql-form :hive-like [_ hsql-form amount unit] (hx/+ (hx/->timestamp hsql-form) (hsql/raw (format "(INTERVAL '%d' %s)" (int amount) (name unit))))) diff --git a/modules/drivers/sqlite/src/metabase/driver/sqlite.clj b/modules/drivers/sqlite/src/metabase/driver/sqlite.clj index e719fb5aa0d2197501be3f10020c7c4fa13ff9ab..e388507562f854169a74c0562b97869b3cb765f8 100644 --- a/modules/drivers/sqlite/src/metabase/driver/sqlite.clj +++ b/modules/drivers/sqlite/src/metabase/driver/sqlite.clj @@ -28,6 +28,8 @@ (driver/register! :sqlite, :parent :sql-jdbc) (defmethod driver/supports? [:sqlite :regex] [_ _] false) +(defmethod driver/supports? [:sqlite :percentile-aggregations] [_ _] false) +(defmethod driver/supports? [:sqlite :advanced-math-expressions] [_ _] false) (defmethod sql-jdbc.conn/connection-details->spec :sqlite [_ {:keys [db] @@ -228,6 +230,14 @@ (hx/literal arg) arg))))))) +(defmethod sql.qp/->honeysql [:sqlite :floor] + [driver [_ arg]] + (hsql/call :round (hsql/call :- arg 0.5))) + +(defmethod sql.qp/->honeysql [:sqlite :ceil] + [driver [_ arg]] + (hsql/call :round (hsql/call :+ arg 0.5))) + ;; See https://sqlite.org/lang_datefunc.html diff --git a/modules/drivers/sqlserver/src/metabase/driver/sqlserver.clj b/modules/drivers/sqlserver/src/metabase/driver/sqlserver.clj index b5f5632462ede4da76989051189ea1167008c881..cc28113aa4f324ded5e7854b1ca8aeefdff6b043 100644 --- a/modules/drivers/sqlserver/src/metabase/driver/sqlserver.clj +++ b/modules/drivers/sqlserver/src/metabase/driver/sqlserver.clj @@ -24,6 +24,7 @@ (driver/register! :sqlserver, :parent :sql-jdbc) (defmethod driver/supports? [:sqlserver :regex] [_ _] false) +(defmethod driver/supports? [:sqlserver :percentile-aggregations] [_ _] false) ;; See the list here: https://docs.microsoft.com/en-us/sql/connect/jdbc/using-basic-data-types (defmethod sql-jdbc.sync/database-type->base-type :sqlserver @@ -229,7 +230,11 @@ (defmethod sql.qp/->honeysql [:sqlserver :stddev] [driver [_ field]] - (hsql/call :stdev (sql.qp/->honeysql driver field))) + (hsql/call :stdevp (sql.qp/->honeysql driver field))) + +(defmethod sql.qp/->honeysql [:sqlserver :var] + [driver [_ field]] + (hsql/call :varp (sql.qp/->honeysql driver field))) (defmethod sql.qp/->honeysql [:sqlserver :substring] [driver [_ arg start length]] @@ -241,6 +246,22 @@ [driver [_ arg]] (hsql/call :len (sql.qp/->honeysql driver arg))) +(defmethod sql.qp/->honeysql [:sqlserver :ceil] + [driver [_ arg]] + (hsql/call :ceiling (sql.qp/->honeysql driver arg))) + +(defmethod sql.qp/->honeysql [:sqlserver :round] + [driver [_ arg]] + (hsql/call :round (hx/cast :float (sql.qp/->honeysql driver arg)) 0)) + +(defmethod sql.qp/->honeysql [:sqlserver :power] + [driver [_ arg power]] + (hsql/call :power (hx/cast :float (sql.qp/->honeysql driver arg)) (sql.qp/->honeysql driver power))) + +(defmethod sql.qp/->honeysql [:sqlserver :median] + [driver [_ arg]] + (sql.qp/->honeysql driver [:percentile arg 0.5])) + (defmethod driver.common/current-db-time-date-formatters :sqlserver [_] (driver.common/create-db-time-formatters "yyyy-MM-dd'T'HH:mm:ss.SSSSSSSSZ")) diff --git a/modules/drivers/vertica/src/metabase/driver/vertica.clj b/modules/drivers/vertica/src/metabase/driver/vertica.clj index 30c5aad930cd8f608eae9b8bbafbab565bd7ffa8..b09383fd4e60eeb1d24bc0cae47bb7783acc0043 100644 --- a/modules/drivers/vertica/src/metabase/driver/vertica.clj +++ b/modules/drivers/vertica/src/metabase/driver/vertica.clj @@ -2,7 +2,9 @@ (:require [clojure.java.jdbc :as jdbc] [clojure.set :as set] [clojure.tools.logging :as log] - [honeysql.core :as hsql] + [honeysql + [core :as hsql] + [format :as hformat]] [metabase.driver :as driver] [metabase.driver.common :as driver.common] [metabase.driver.sql-jdbc @@ -20,6 +22,9 @@ (driver/register! :vertica, :parent #{:sql-jdbc ::legacy/use-legacy-classes-for-read-and-set}) +(defmethod driver/supports? [:vertica :percentile-aggregations] [_ _] false) + + (defmethod sql-jdbc.sync/database-type->base-type :vertica [_ database-type] ({:Boolean :type/Boolean @@ -99,6 +104,16 @@ [driver [_ arg pattern]] (hsql/call :regexp_substr (sql.qp/->honeysql driver arg) (sql.qp/->honeysql driver pattern))) +(defmethod sql.qp/->honeysql [:vertica :percentile] + [driver [_ arg p]] + (hsql/raw (format "APPROXIMATE_PERCENTILE(%s USING PARAMETERS percentile=%s)" + (hformat/to-sql (sql.qp/->honeysql driver arg)) + (hformat/to-sql (sql.qp/->honeysql driver p))))) + +(defmethod sql.qp/->honeysql [:vertica :median] + [driver [_ arg]] + (hsql/call :approximate_median (sql.qp/->honeysql driver arg))) + (defmethod sql.qp/add-interval-honeysql-form :vertica [_ hsql-form amount unit] (hx/+ (hx/->timestamp hsql-form) (hsql/raw (format "(INTERVAL '%d %s')" (int amount) (name unit))))) diff --git a/src/metabase/driver.clj b/src/metabase/driver.clj index 284d17f3afd651d0c009f0f68790d9aefd46d609..a1dfbe42748bf2d9f847791d2eddf150997f75ab 100644 --- a/src/metabase/driver.clj +++ b/src/metabase/driver.clj @@ -355,7 +355,7 @@ ;; DEFAULTS TO TRUE. :basic-aggregations - ;; Does this driver support standard deviation aggregations? + ;; Does this driver support standard deviation and variance aggregations? :standard-deviation-aggregations ;; Does this driver support expressions (e.g. adding the values of 2 columns together)? @@ -393,7 +393,13 @@ :inner-join :full-join - :regex}) + :regex + + ;; Does the driver support advanced math expressions such as log, power, ... + :advanced-math-expressions + + ;; Does the driver support percentile calculations (including median) + :percentile-aggregations}) (defmulti supports? "Does this driver support a certain `feature`? (A feature is a keyword, and can be any of the ones listed above in diff --git a/src/metabase/driver/h2.clj b/src/metabase/driver/h2.clj index 2453a52ece1abf9cb190ce1b321e64bae04e90d9..25c657a96a8931c90961556207470297af5f48ce 100644 --- a/src/metabase/driver/h2.clj +++ b/src/metabase/driver/h2.clj @@ -33,6 +33,7 @@ (defmethod driver/supports? [:h2 :full-join] [_ _] false) (defmethod driver/supports? [:h2 :regex] [_ _] false) +(defmethod driver/supports? [:h2 :percentile-aggregations] [_ _] false) (defmethod driver/connection-properties :h2 [_] @@ -179,6 +180,10 @@ 3) 2)))) +(defmethod sql.qp/->honeysql [:h2 :log] + [driver [_ field]] + (hsql/call :log10 (sql.qp/->honeysql driver field))) + ;;; +----------------------------------------------------------------------------------------------------------------+ ;;; | metabase.driver.sql-jdbc impls | diff --git a/src/metabase/driver/mysql.clj b/src/metabase/driver/mysql.clj index 7cf14890c7aa30816337e7a8e5f885d92b08afd4..7b5b888bec225e76db6a7a13a5094e74c3cf43f7 100644 --- a/src/metabase/driver/mysql.clj +++ b/src/metabase/driver/mysql.clj @@ -35,6 +35,8 @@ (defmethod driver/display-name :mysql [_] "MySQL") (defmethod driver/supports? [:mysql :regex] [_ _] false) +(defmethod driver/supports? [:mysql :percentile-aggregations] [_ _] false) + ;;; +----------------------------------------------------------------------------------------------------------------+ diff --git a/src/metabase/driver/postgres.clj b/src/metabase/driver/postgres.clj index c3025b48e8db875803b886917768dd6f92d06a39..d24ab97ee541feb883ae8b6701ef5e4b0cf7edb8 100644 --- a/src/metabase/driver/postgres.clj +++ b/src/metabase/driver/postgres.clj @@ -161,6 +161,10 @@ :type/PostgresEnum (hx/quoted-cast database-type value) (sql.qp/->honeysql driver value))))) +(defmethod sql.qp/->honeysql [:postgres :median] + [driver [_ arg]] + (sql.qp/->honeysql driver [:percentile arg 0.5])) + (defmethod sql.qp/->honeysql [:postgres :regex-match-first] [driver [_ arg pattern]] (hsql/call :substring (hsql/raw (str (hformat/to-sql (sql.qp/->honeysql driver arg)) " FROM '" pattern "'")))) diff --git a/src/metabase/driver/sql.clj b/src/metabase/driver/sql.clj index 72c35cffc3fe2d9140394c3637c931ee004f62b4..69706957e7ae188a5bbfd9fcb3509cbd9f2c9a12 100644 --- a/src/metabase/driver/sql.clj +++ b/src/metabase/driver/sql.clj @@ -21,6 +21,8 @@ :native-parameters :nested-queries :binning + :advanced-math-expressions + :percentile-aggregations :regex]] (defmethod driver/supports? [:sql feature] [_ _] true)) diff --git a/src/metabase/driver/sql/query_processor.clj b/src/metabase/driver/sql/query_processor.clj index 6ca33f3721ea9b9adca04fe9b030a653e9caf207..ea873453114c2369420ffbd663912878ef1835bc 100644 --- a/src/metabase/driver/sql/query_processor.clj +++ b/src/metabase/driver/sql/query_processor.clj @@ -300,12 +300,26 @@ (hsql/call :count (->honeysql driver field)) :%count.*)) -(defmethod ->honeysql [:sql :avg] [driver [_ field]] (hsql/call :avg (->honeysql driver field))) -(defmethod ->honeysql [:sql :distinct] [driver [_ field]] (hsql/call :distinct-count (->honeysql driver field))) -(defmethod ->honeysql [:sql :stddev] [driver [_ field]] (hsql/call :stddev (->honeysql driver field))) -(defmethod ->honeysql [:sql :sum] [driver [_ field]] (hsql/call :sum (->honeysql driver field))) -(defmethod ->honeysql [:sql :min] [driver [_ field]] (hsql/call :min (->honeysql driver field))) -(defmethod ->honeysql [:sql :max] [driver [_ field]] (hsql/call :max (->honeysql driver field))) +(defmethod ->honeysql [:sql :avg] [driver [_ field]] (hsql/call :avg (->honeysql driver field))) +(defmethod ->honeysql [:sql :median] [driver [_ field]] (hsql/call :median (->honeysql driver field))) +(defmethod ->honeysql [:sql :percentile] [driver [_ field p]] (hsql/call :percentile-cont (->honeysql driver field) (->honeysql driver p))) +(defmethod ->honeysql [:sql :distinct] [driver [_ field]] (hsql/call :distinct-count (->honeysql driver field))) +(defmethod ->honeysql [:sql :stddev] [driver [_ field]] (hsql/call :stddev_pop (->honeysql driver field))) +(defmethod ->honeysql [:sql :var] [driver [_ field]] (hsql/call :var_pop (->honeysql driver field))) +(defmethod ->honeysql [:sql :sum] [driver [_ field]] (hsql/call :sum (->honeysql driver field))) +(defmethod ->honeysql [:sql :min] [driver [_ field]] (hsql/call :min (->honeysql driver field))) +(defmethod ->honeysql [:sql :max] [driver [_ field]] (hsql/call :max (->honeysql driver field))) + +(defmethod ->honeysql [:sql :floor] [driver [_ field]] (hsql/call :floor (->honeysql driver field))) +(defmethod ->honeysql [:sql :ceil] [driver [_ field]] (hsql/call :ceil (->honeysql driver field))) +(defmethod ->honeysql [:sql :round] [driver [_ field]] (hsql/call :round (->honeysql driver field))) +(defmethod ->honeysql [:sql :abs] [driver [_ field]] (hsql/call :abs (->honeysql driver field))) + +(defmethod ->honeysql [:sql :log] [driver [_ field]] (hsql/call :log 10 (->honeysql driver field))) +(defmethod ->honeysql [:sql :exp] [driver [_ field]] (hsql/call :exp (->honeysql driver field))) +(defmethod ->honeysql [:sql :sqrt] [driver [_ field]] (hsql/call :sqrt (->honeysql driver field))) +(defmethod ->honeysql [:sql :power] [driver [_ field power]] + (hsql/call :power (->honeysql driver field) (->honeysql driver power))) (defmethod ->honeysql [:sql :+] [driver [_ & args]] diff --git a/src/metabase/query_processor/middleware/annotate.clj b/src/metabase/query_processor/middleware/annotate.clj index d4d8e259c30c991412f31921c0f4ff7bd1df9f59..56a1eb7386c412ced4ba2f7e41f50dfc49b3a1f1 100644 --- a/src/metabase/query_processor/middleware/annotate.clj +++ b/src/metabase/query_processor/middleware/annotate.clj @@ -338,16 +338,19 @@ [:count] (tru "Count") [:case] (tru "Case") - [:distinct arg] (tru "Distinct values of {0}" (aggregation-arg-display-name inner-query arg)) - [:count arg] (tru "Count of {0}" (aggregation-arg-display-name inner-query arg)) - [:avg arg] (tru "Average of {0}" (aggregation-arg-display-name inner-query arg)) + [:distinct arg] (tru "Distinct values of {0}" (aggregation-arg-display-name inner-query arg)) + [:count arg] (tru "Count of {0}" (aggregation-arg-display-name inner-query arg)) + [:avg arg] (tru "Average of {0}" (aggregation-arg-display-name inner-query arg)) ;; cum-count and cum-sum get names for count and sum, respectively (see explanation in `aggregation-name`) - [:cum-count arg] (tru "Count of {0}" (aggregation-arg-display-name inner-query arg)) - [:cum-sum arg] (tru "Sum of {0}" (aggregation-arg-display-name inner-query arg)) - [:stddev arg] (tru "SD of {0}" (aggregation-arg-display-name inner-query arg)) - [:sum arg] (tru "Sum of {0}" (aggregation-arg-display-name inner-query arg)) - [:min arg] (tru "Min of {0}" (aggregation-arg-display-name inner-query arg)) - [:max arg] (tru "Max of {0}" (aggregation-arg-display-name inner-query arg)) + [:cum-count arg] (tru "Count of {0}" (aggregation-arg-display-name inner-query arg)) + [:cum-sum arg] (tru "Sum of {0}" (aggregation-arg-display-name inner-query arg)) + [:stddev arg] (tru "SD of {0}" (aggregation-arg-display-name inner-query arg)) + [:sum arg] (tru "Sum of {0}" (aggregation-arg-display-name inner-query arg)) + [:min arg] (tru "Min of {0}" (aggregation-arg-display-name inner-query arg)) + [:max arg] (tru "Max of {0}" (aggregation-arg-display-name inner-query arg)) + [:var arg] (tru "Variance of {0}" (aggregation-arg-display-name inner-query arg)) + [:median arg] (tru "Median of {0}" (aggregation-arg-display-name inner-query arg)) + [:percentile arg p] (tru "{0}th percentile of {1}" p (aggregation-arg-display-name inner-query arg)) ;; until we have a way to generate good names for filters we'll just have to say 'matching condition' for now [:sum-where arg _] (tru "Sum of {0} matching condition" (aggregation-arg-display-name inner-query arg)) diff --git a/src/metabase/util/honeysql_extensions.clj b/src/metabase/util/honeysql_extensions.clj index 61ede48659ef7d2887cae307d4965be4f1232436..a7d22291f67e279f5502c093a88552285e199bed 100644 --- a/src/metabase/util/honeysql_extensions.clj +++ b/src/metabase/util/honeysql_extensions.clj @@ -31,11 +31,16 @@ (defmethod hformat/fn-handler "extract" [_ unit expr] (str "extract(" (name unit) " from " (hformat/to-sql expr) ")")) -;; register the function "distinct-count" with HoneySQL +;; register the function `distinct-count` with HoneySQL ;; (hsql/format :%distinct-count.x) -> "count(distinct x)" (defmethod hformat/fn-handler "distinct-count" [_ field] (str "count(distinct " (hformat/to-sql field) ")")) +;; register the function `percentile` with HoneySQL +;; (hsql/format (hsql/call :percentile-cont :a 0.9)) -> "percentile_cont(0.9) within group (order by a)" +(defmethod hformat/fn-handler "percentile-cont" [_ field p] + (str "PERCENTILE_CONT(" (hformat/to-sql p) ") within group (order by " (hformat/to-sql field) ")")) + ;; HoneySQL 0.7.0+ parameterizes numbers to fix issues with NaN and infinity -- see ;; https://github.com/jkk/honeysql/pull/122. However, this broke some of Metabase's behavior, specifically queries diff --git a/test/metabase/query_processor_test/advanced_math_test.clj b/test/metabase/query_processor_test/advanced_math_test.clj new file mode 100644 index 0000000000000000000000000000000000000000..08b62948b7afb3c474ff21bb1cbf7bf39958c26e --- /dev/null +++ b/test/metabase/query_processor_test/advanced_math_test.clj @@ -0,0 +1,93 @@ +(ns metabase.query-processor-test.advanced-math-test + (:require [clojure.test :refer :all] + [metabase + [query-processor-test :refer :all] + [test :as mt] + [util :as u]] + [metabase.test.data :as data])) + +(defn- test-math-expression + [expr] + (->> {:expressions {"test" expr} + :fields [[:expression "test"]] + ;; To ensure stable ordering + :order-by [[:asc [:field-id (data/id :venues :id)]]] + :limit 1} + (mt/run-mbql-query venues) + rows + ffirst + double + ;; Round to prevent minute differences across DBs due to differences in how float point math is handled + (u/round-to-decimals 2))) + +(deftest test-round + (mt/test-drivers (mt/normal-drivers-with-feature :expressions) + (is (= 1.0 (test-math-expression [:round 0.7]))))) + +(deftest test-floor + (mt/test-drivers (mt/normal-drivers-with-feature :expressions) + (is (= 0.0 (test-math-expression [:floor 0.7]))))) + +(deftest test-ceil + (mt/test-drivers (mt/normal-drivers-with-feature :expressions) + (is (= 1.0 (test-math-expression [:ceil 0.3]))))) + +(deftest test-abs + (mt/test-drivers (mt/normal-drivers-with-feature :expressions) + (is (= 2.0 (test-math-expression [:abs -2]))))) + + +(deftest test-power + (mt/test-drivers (mt/normal-drivers-with-feature :advanced-math-expressions) + (is (= 4.0 (test-math-expression [:power 2.0 2]))) + (is (= 2.0 (test-math-expression [:power 4.0 0.5]))) + (is (= 0.25 (test-math-expression [:power 2.0 -2]))))) + +(deftest test-sqrt + (mt/test-drivers (mt/normal-drivers-with-feature :advanced-math-expressions) + (is (= 2.0 (test-math-expression [:sqrt 4.0]))))) + +(deftest test-exp + (mt/test-drivers (mt/normal-drivers-with-feature :advanced-math-expressions) + (is (= 7.39 (test-math-expression [:exp 2.0]))))) + +(deftest test-log + (mt/test-drivers (mt/normal-drivers-with-feature :advanced-math-expressions) + (is (= 1.0 (test-math-expression [:log 10.0]))))) + + +(deftest test-filter + (mt/test-drivers (mt/normal-drivers-with-feature :advanced-math-expressions) + (is (= 59 (->> {:aggregation [[:count]] + :filter [:between [:- [:round [:power [:field-id (data/id :venues :price)] 2]] 1] 1 5]} + (mt/run-mbql-query venues) + rows + ffirst + int))))) + + +(defn- test-aggregation + [agg] + (->> {:aggregation [agg]} + (mt/run-mbql-query venues) + rows + ffirst + double + (u/round-to-decimals 2))) + +(deftest test-variance + (mt/test-drivers (mt/normal-drivers-with-feature :standard-deviation-aggregations) + (is (= 0.59 (test-aggregation [:var [:field-id (data/id :venues :price)]]))))) + +(deftest test-median + (mt/test-drivers (mt/normal-drivers-with-feature :percentile-aggregations) + (is (= 2.0 (test-aggregation [:median [:field-id (data/id :venues :price)]]))))) + +(deftest test-percentile + (mt/test-drivers (mt/normal-drivers-with-feature :percentile-aggregations) + (is (= 3.0 (test-aggregation [:percentile [:field-id (data/id :venues :price)] 0.9]))))) + +(deftest test-nesting + (mt/test-drivers (mt/normal-drivers-with-feature :advanced-math-expressions) + (is (= 2.0 (test-math-expression [:sqrt [:power 2.0 2]]))) + (is (= 59.0 (test-aggregation [:count-where [:between [:- [:round [:power [:field-id (data/id :venues :price)] 2]] 1] 1 5]]))))) diff --git a/test/metabase/query_processor_test/case_test.clj b/test/metabase/query_processor_test/case_test.clj index b1943ec9011014db26a191058abd89b2a65af87b..87eb1fda537b3d4fe407eedba5f5e09ed96c89ab 100644 --- a/test/metabase/query_processor_test/case_test.clj +++ b/test/metabase/query_processor_test/case_test.clj @@ -43,7 +43,7 @@ :definition {:source-table (data/id :venues) :filter [:< [:field-id (data/id :venues :price)] 4]}}]] (is (= 179.0 (test-case [:sum [:case [[[:segment segment-id] [:field-id (data/id :venues :price)]]]]]))))) - (testing "Can we use case in metricw" + (testing "Can we use case in metric" (tt/with-temp* [Metric [{metric-id :id} {:table_id (data/id :venues) :definition {:source-table (data/id :venues) :aggregation [:sum diff --git a/test/metabase/query_processor_test/nested_queries_test.clj b/test/metabase/query_processor_test/nested_queries_test.clj index df2082b3b7917490d67b81658ffd62a03acf4ed3..f0469929dea5a9eac70864f8755d507451fc6edc 100644 --- a/test/metabase/query_processor_test/nested_queries_test.clj +++ b/test/metabase/query_processor_test/nested_queries_test.clj @@ -287,7 +287,7 @@ ;; e.g. the ORDER BY in the source-query should refer the 'stddev' aggregation, NOT the 'avg' aggregation (expect {:query (str "SELECT avg(\"source\".\"stddev\") AS \"avg\" FROM (" - "SELECT \"PUBLIC\".\"VENUES\".\"PRICE\" AS \"PRICE\", stddev(\"PUBLIC\".\"VENUES\".\"ID\") AS \"stddev\" " + "SELECT \"PUBLIC\".\"VENUES\".\"PRICE\" AS \"PRICE\", stddev_pop(\"PUBLIC\".\"VENUES\".\"ID\") AS \"stddev\" " "FROM \"PUBLIC\".\"VENUES\" " "GROUP BY \"PUBLIC\".\"VENUES\".\"PRICE\" " "ORDER BY \"stddev\" DESC, \"PUBLIC\".\"VENUES\".\"PRICE\" ASC" diff --git a/test/metabase/query_processor_test/order_by_test.clj b/test/metabase/query_processor_test/order_by_test.clj index 4dbcefd0968b9b5e767f6b4c78ad9160550f8e76..793ddebd975f514c969d455c92c9355283344d8a 100644 --- a/test/metabase/query_processor_test/order_by_test.clj +++ b/test/metabase/query_processor_test/order_by_test.clj @@ -77,10 +77,10 @@ (mt/test-drivers (mt/normal-drivers-with-feature :standard-deviation-aggregations) ;; standard deviation calculations are always NOT EXACT (normal behavior) so round results to nearest whole ;; number. - (is (= [[3 (if (= driver/*driver* :mysql) 25.0 26.0)] + (is (= [[3 25.0] [1 24.0] [2 21.0] - [4 (if (= driver/*driver* :mysql) 14.0 15.0)]] + [4 14.0]] (mt/formatted-rows [int 0.0] (mt/run-mbql-query venues {:aggregation [[:stddev $category_id]] diff --git a/test/metabase/query_processor_test/string_extracts_test.clj b/test/metabase/query_processor_test/string_extracts_test.clj index 9af9b56e45a4b46e1c094e055087fec954cb02b3..77b6f0b8129a6a84d04c722f225ef610d0f069ee 100644 --- a/test/metabase/query_processor_test/string_extracts_test.clj +++ b/test/metabase/query_processor_test/string_extracts_test.clj @@ -43,7 +43,9 @@ (deftest test-substring (mt/test-drivers (mt/normal-drivers-with-feature :expressions) (is (= "Red" (test-string-extract [:substring [:field-id (data/id :venues :name)] 1 3]))) - (is (= "ed Medicine" (test-string-extract [:substring [:field-id (data/id :venues :name)] 2]))))) + (is (= "ed Medicine" (test-string-extract [:substring [:field-id (data/id :venues :name)] 2]))) + (is (= "Red Medicin" (test-string-extract [:substring [:field-id (data/id :venues :name)] + 1 [:- [:length [:field-id (data/id :venues :name)]] 1]]))))) (deftest test-replacea (mt/test-drivers (mt/normal-drivers-with-feature :expressions)