Skip to content
Snippets Groups Projects
Unverified Commit 9f6d45ff authored by metabase-bot[bot]'s avatar metabase-bot[bot] Committed by GitHub
Browse files

Fix wrong aggregation field's `base_type` (#28741) (#28835)


* Fix wrong aggregation field's `base_type`

It should be the same as the underlying field, not a hard-coded number
typ (`type/Integer` or `type/Float`)

* Add unit tests

Co-authored-by: default avatarMahatthana (Kelvin) Nomsawadi <me@bboykelvin.dev>
parent 20510888
No related branches found
No related tags found
No related merge requests found
......@@ -23,7 +23,7 @@ const questionDetails = {
},
};
describe.skip("issue 25994", () => {
describe("issue 25994", () => {
beforeEach(() => {
restore();
cy.signInAsAdmin();
......
......@@ -12,7 +12,10 @@ import Metric from "metabase-lib/metadata/Metric";
import StructuredQuery from "../StructuredQuery";
import Dimension, { AggregationDimension } from "../../Dimension";
import MBQLClause from "./MBQLClause";
const INTEGER_AGGREGATIONS = new Set(["count", "cum-count", "distinct"]);
const ORIGINAL_FIELD_TYPE_AGGREGATIONS = new Set(["min", "max"]);
export default class Aggregation extends MBQLClause {
/**
* Replaces the aggregation in the parent query and returns the new StructuredQuery
......@@ -134,7 +137,16 @@ export default class Aggregation extends MBQLClause {
baseType() {
const short = this.short();
return INTEGER_AGGREGATIONS.has(short) ? TYPE.Integer : TYPE.Float;
if (INTEGER_AGGREGATIONS.has(short)) {
return TYPE.Integer;
}
const field = this.dimension()?.field();
if (ORIGINAL_FIELD_TYPE_AGGREGATIONS.has(short) && field) {
return field.base_type;
}
return TYPE.Float;
}
/**
......
......@@ -801,6 +801,62 @@ describe("Dimension", () => {
expect(base_type).toBe("type/Integer");
});
it.each([
{
field: ["field", PRODUCTS.CATEGORY.id, null],
fieldName: "category",
expectedType: "type/Text",
},
{
field: ["field", PRODUCTS.PRICE.id, null],
fieldName: "price",
expectedType: "type/Float",
},
{
field: [
"field",
PRODUCTS.CREATED_AT.id,
{ "temporal-unit": "day" },
],
fieldName: "created_at",
expectedType: "type/DateTime",
},
])(
"should return $expectedType for min of $fieldName",
({ field, expectedType }) => {
const { base_type } = aggregation(["min", field]).field();
expect(base_type).toBe(expectedType);
},
);
it.each([
{
field: ["field", PRODUCTS.CATEGORY.id, null],
fieldName: "category",
expectedType: "type/Text",
},
{
field: ["field", PRODUCTS.PRICE.id, null],
fieldName: "price",
expectedType: "type/Float",
},
{
field: [
"field",
PRODUCTS.CREATED_AT.id,
{ "temporal-unit": "day" },
],
fieldName: "created_at",
expectedType: "type/DateTime",
},
])(
"should return $expectedType for max of $fieldName",
({ field, expectedType }) => {
const { base_type } = aggregation(["max", field]).field();
expect(base_type).toBe(expectedType);
},
);
it("should return an int field for count", () => {
const { base_type } = aggregation(["count"]).field();
expect(base_type).toBe("type/Integer");
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment