From 62ae6d5babf1346a93f0173094d0fd6762ebef60 Mon Sep 17 00:00:00 2001
From: Ariya Hidayat <ariya@metabase.com>
Date: Thu, 24 Feb 2022 13:02:50 -0800
Subject: [PATCH] Upgrade MBQL of expression & aggregation dimension to include
 options (#20713)

---
 frontend/src/metabase-lib/lib/Dimension.ts    | 79 ++++++++++++++++---
 .../lib/queries/StructuredQuery.ts            |  2 +-
 .../lib/queries/structured/Aggregation.ts     |  2 +-
 .../metabase-lib/lib/Dimension.unit.spec.js   |  8 +-
 .../test/metabase/lib/dataset.unit.spec.js    |  2 +-
 5 files changed, 76 insertions(+), 17 deletions(-)

diff --git a/frontend/src/metabase-lib/lib/Dimension.ts b/frontend/src/metabase-lib/lib/Dimension.ts
index 17cfb818436..2069d6ddfdd 100644
--- a/frontend/src/metabase-lib/lib/Dimension.ts
+++ b/frontend/src/metabase-lib/lib/Dimension.ts
@@ -1128,7 +1128,7 @@ const isFieldDimension = dimension => dimension instanceof FieldDimension;
  * Expression reference, `["expression", expression-name]`
  */
 export class ExpressionDimension extends Dimension {
-  tag = "Custom";
+  _expressionName: ExpressionName;
 
   static parseMBQL(
     mbql: any,
@@ -1136,24 +1136,50 @@ export class ExpressionDimension extends Dimension {
     query?: StructuredQuery | null | undefined,
   ): Dimension | null | undefined {
     if (Array.isArray(mbql) && mbql[0] === "expression") {
-      return new ExpressionDimension(null, mbql.slice(1), metadata, query);
+      const [expressionName, options] = mbql.slice(1);
+      return new ExpressionDimension(expressionName, options, metadata, query);
     }
   }
 
+  constructor(
+    expressionName,
+    options = null,
+    metadata = null,
+    query = null,
+    additionalProperties = null,
+  ) {
+    super(
+      null,
+      [expressionName, options],
+      metadata,
+      query,
+      Object.freeze(Dimension.normalizeOptions(options)),
+    );
+    this._expressionName = expressionName;
+
+    if (additionalProperties) {
+      Object.keys(additionalProperties).forEach(k => {
+        this[k] = additionalProperties[k];
+      });
+    }
+
+    Object.freeze(this);
+  }
+
   mbql(): ExpressionReference {
-    return ["expression", this._args[0]];
+    return ["expression", this._expressionName, this._options];
   }
 
   name() {
-    return this._args[0];
+    return this._expressionName;
   }
 
   displayName(): string {
-    return this._args[0];
+    return this._expressionName;
   }
 
   columnName() {
-    return this._args[0];
+    return this._expressionName;
   }
 
   field() {
@@ -1176,7 +1202,7 @@ export class ExpressionDimension extends Dimension {
 
       type = infer(expressions[this.name()], env);
     } else {
-      type = infer(this._args[0]);
+      type = infer(this._expressionName);
     }
 
     let base_type = type;
@@ -1232,18 +1258,51 @@ const UNAGGREGATED_SEMANTIC_TYPES = new Set([TYPE.FK, TYPE.PK]);
  * Aggregation reference, `["aggregation", aggregation-index]`
  */
 export class AggregationDimension extends Dimension {
+  _aggregationIndex: number;
+
   static parseMBQL(
     mbql: any,
     metadata?: Metadata | null | undefined,
     query?: StructuredQuery | null | undefined,
   ): Dimension | null | undefined {
     if (Array.isArray(mbql) && mbql[0] === "aggregation") {
-      return new AggregationDimension(null, mbql.slice(1), metadata, query);
+      const [aggregationIndex, options] = mbql.slice(1);
+      return new AggregationDimension(
+        aggregationIndex,
+        options,
+        metadata,
+        query,
+      );
+    }
+  }
+
+  constructor(
+    aggregationIndex,
+    options = null,
+    metadata = null,
+    query = null,
+    additionalProperties = null,
+  ) {
+    super(
+      null,
+      [aggregationIndex, options],
+      metadata,
+      query,
+      Object.freeze(Dimension.normalizeOptions(options)),
+    );
+    this._aggregationIndex = aggregationIndex;
+
+    if (additionalProperties) {
+      Object.keys(additionalProperties).forEach(k => {
+        this[k] = additionalProperties[k];
+      });
     }
+
+    Object.freeze(this);
   }
 
   aggregationIndex(): number {
-    return this._args[0];
+    return this._aggregationIndex;
   }
 
   column(extra = {}) {
@@ -1314,7 +1373,7 @@ export class AggregationDimension extends Dimension {
   }
 
   mbql() {
-    return ["aggregation", this._args[0]];
+    return ["aggregation", this._aggregationIndex, this._options];
   }
 
   icon() {
diff --git a/frontend/src/metabase-lib/lib/queries/StructuredQuery.ts b/frontend/src/metabase-lib/lib/queries/StructuredQuery.ts
index 07460ac7eb2..c42845f8411 100644
--- a/frontend/src/metabase-lib/lib/queries/StructuredQuery.ts
+++ b/frontend/src/metabase-lib/lib/queries/StructuredQuery.ts
@@ -1288,8 +1288,8 @@ export default class StructuredQuery extends AtomicQuery {
     return Object.entries(this.expressions()).map(
       ([expressionName, expression]) => {
         return new ExpressionDimension(
+          expressionName,
           null,
-          [expressionName],
           this._metadata,
           this,
         );
diff --git a/frontend/src/metabase-lib/lib/queries/structured/Aggregation.ts b/frontend/src/metabase-lib/lib/queries/structured/Aggregation.ts
index c9fd526f016..b3f14a98e04 100644
--- a/frontend/src/metabase-lib/lib/queries/structured/Aggregation.ts
+++ b/frontend/src/metabase-lib/lib/queries/structured/Aggregation.ts
@@ -279,8 +279,8 @@ export default class Aggregation extends MBQLClause {
   // MISC
   aggregationDimension() {
     return new AggregationDimension(
+      this._index,
       null,
-      [this._index],
       this._query.metadata(),
       this._query,
     );
diff --git a/frontend/test/metabase-lib/lib/Dimension.unit.spec.js b/frontend/test/metabase-lib/lib/Dimension.unit.spec.js
index b0a46afe62f..3c901cbe163 100644
--- a/frontend/test/metabase-lib/lib/Dimension.unit.spec.js
+++ b/frontend/test/metabase-lib/lib/Dimension.unit.spec.js
@@ -664,7 +664,7 @@ describe("Dimension", () => {
     describe("INSTANCE METHODS", () => {
       describe("mbql()", () => {
         it('returns an "expression" clause', () => {
-          expect(dimension.mbql()).toEqual(["expression", "Hello World"]);
+          expect(dimension.mbql()).toEqual(["expression", "Hello World", null]);
         });
       });
       describe("displayName()", () => {
@@ -675,12 +675,12 @@ describe("Dimension", () => {
 
       describe("column()", () => {
         expect(dimension.column()).toEqual({
-          id: ["expression", "Hello World"],
+          id: ["expression", "Hello World", null],
           name: "Hello World",
           display_name: "Hello World",
           base_type: "type/Text",
           semantic_type: null,
-          field_ref: ["expression", "Hello World"],
+          field_ref: ["expression", "Hello World", null],
         });
       });
 
@@ -781,7 +781,7 @@ describe("Dimension", () => {
     describe("INSTANCE METHODS", () => {
       describe("mbql()", () => {
         it('returns an "aggregation" clause', () => {
-          expect(dimension.mbql()).toEqual(["aggregation", 1]);
+          expect(dimension.mbql()).toEqual(["aggregation", 1, null]);
         });
       });
 
diff --git a/frontend/test/metabase/lib/dataset.unit.spec.js b/frontend/test/metabase/lib/dataset.unit.spec.js
index d8479900fe2..76b6dfeed23 100644
--- a/frontend/test/metabase/lib/dataset.unit.spec.js
+++ b/frontend/test/metabase/lib/dataset.unit.spec.js
@@ -214,7 +214,7 @@ describe("metabase/util/dataset", () => {
               expression_name: "foo",
               field_ref: fieldRefEnabled ? ["expression", "foo"] : undefined,
             }),
-          ).toEqual(JSON.stringify(["ref", ["expression", "foo"]]));
+          ).toEqual(JSON.stringify(["ref", ["expression", "foo", null]]));
         });
         it("should return [name ...] for aggregation", () => {
           const col = {
-- 
GitLab