This is an automated email from the ASF dual-hosted git repository.
beto pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 22de26cd77 fix: `metric.currency` should be JSON, not string (#33303)
22de26cd77 is described below
commit 22de26cd77199495ff1cd50672f584d7e45f5a9c
Author: Beto Dealmeida <[email protected]>
AuthorDate: Thu May 1 18:16:51 2025 -0400
fix: `metric.currency` should be JSON, not string (#33303)
---
docs/static/resources/openapi.json | 10 --
.../superset-ui-chart-controls/src/fixtures.ts | 1 -
.../superset-ui-chart-controls/src/types.ts | 1 -
.../test/utils/columnChoices.test.tsx | 2 -
.../test/utils/defineSavedMetrics.test.tsx | 1 -
superset-frontend/src/dashboard/constants.ts | 1 -
.../src/explore/actions/datasourcesActions.test.ts | 2 -
.../src/explore/controlUtils/controlUtils.test.tsx | 1 -
...etControlValuesCompatibleWithDatasource.test.ts | 1 -
superset-frontend/src/explore/fixtures.tsx | 2 -
superset-frontend/src/features/datasets/types.ts | 1 -
.../src/utils/getDatasourceUid.test.ts | 1 -
superset/connectors/sqla/models.py | 17 +--
superset/dashboards/schemas.py | 1 -
superset/datasets/api.py | 1 -
superset/explore/schemas.py | 1 -
superset/migrations/shared/utils.py | 125 ++++++++++++++++++++-
..._f1edd4a4d4f2_metric_currency_should_be_json.py | 46 ++++++++
18 files changed, 171 insertions(+), 44 deletions(-)
diff --git a/docs/static/resources/openapi.json
b/docs/static/resources/openapi.json
index 6389dec60d..9c360c71e4 100644
--- a/docs/static/resources/openapi.json
+++ b/docs/static/resources/openapi.json
@@ -3565,9 +3565,6 @@
},
"type": "array"
},
- "currency_formats": {
- "type": "object"
- },
"database": {
"$ref": "#/components/schemas/Database"
},
@@ -5030,10 +5027,6 @@
},
"type": "array"
},
- "currency_formats": {
- "description": "Currency formats.",
- "type": "object"
- },
"database": {
"description": "Database associated with the dataset.",
"type": "object"
@@ -5559,9 +5552,6 @@
"created_on_humanized": {
"readOnly": true
},
- "currency_formats": {
- "readOnly": true
- },
"database": {
"$ref": "#/components/schemas/DatasetRestApi.get.Database"
},
diff --git
a/superset-frontend/packages/superset-ui-chart-controls/src/fixtures.ts
b/superset-frontend/packages/superset-ui-chart-controls/src/fixtures.ts
index 05b4d4df3d..ead279c0e7 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/fixtures.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/fixtures.ts
@@ -21,7 +21,6 @@ import { Dataset } from './types';
export const TestDataset: Dataset = {
column_formats: {},
- currency_formats: {},
columns: [
{
advanced_data_type: undefined,
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts
b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts
index 62ac944688..71517884da 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts
@@ -69,7 +69,6 @@ export interface Dataset {
columns: ColumnMeta[];
metrics: Metric[];
column_formats: Record<string, string>;
- currency_formats: Record<string, Currency>;
verbose_map: Record<string, string>;
main_dttm_col: string;
// eg. ['["ds", true]', 'ds [asc]']
diff --git
a/superset-frontend/packages/superset-ui-chart-controls/test/utils/columnChoices.test.tsx
b/superset-frontend/packages/superset-ui-chart-controls/test/utils/columnChoices.test.tsx
index f27365e956..646ae093fb 100644
---
a/superset-frontend/packages/superset-ui-chart-controls/test/utils/columnChoices.test.tsx
+++
b/superset-frontend/packages/superset-ui-chart-controls/test/utils/columnChoices.test.tsx
@@ -53,7 +53,6 @@ describe('columnChoices()', () => {
],
verbose_map: {},
column_formats: { fiz: 'NUMERIC', about: 'STRING', foo: 'DATE' },
- currency_formats: {},
datasource_name: 'my_datasource',
description: 'this is my datasource',
}),
@@ -105,7 +104,6 @@ describe('columnChoices()', () => {
],
verbose_map: {},
column_formats: { fiz: 'NUMERIC', about: 'STRING', foo: 'DATE' },
- currency_formats: {},
datasource_name: 'my_datasource',
description: 'this is my datasource',
}),
diff --git
a/superset-frontend/packages/superset-ui-chart-controls/test/utils/defineSavedMetrics.test.tsx
b/superset-frontend/packages/superset-ui-chart-controls/test/utils/defineSavedMetrics.test.tsx
index 682075b5c1..f1c64ad4a7 100644
---
a/superset-frontend/packages/superset-ui-chart-controls/test/utils/defineSavedMetrics.test.tsx
+++
b/superset-frontend/packages/superset-ui-chart-controls/test/utils/defineSavedMetrics.test.tsx
@@ -41,7 +41,6 @@ describe('defineSavedMetrics', () => {
columns: [],
verbose_map: {},
column_formats: {},
- currency_formats: {},
datasource_name: 'my_datasource',
description: 'this is my datasource',
};
diff --git a/superset-frontend/src/dashboard/constants.ts
b/superset-frontend/src/dashboard/constants.ts
index d512c2a8a6..60c8847513 100644
--- a/superset-frontend/src/dashboard/constants.ts
+++ b/superset-frontend/src/dashboard/constants.ts
@@ -29,7 +29,6 @@ export const PLACEHOLDER_DATASOURCE: Datasource = {
column_types: [],
metrics: [],
column_formats: {},
- currency_formats: {},
verbose_map: {},
main_dttm_col: '',
description: '',
diff --git a/superset-frontend/src/explore/actions/datasourcesActions.test.ts
b/superset-frontend/src/explore/actions/datasourcesActions.test.ts
index cf599c6b4f..ace4913c9b 100644
--- a/superset-frontend/src/explore/actions/datasourcesActions.test.ts
+++ b/superset-frontend/src/explore/actions/datasourcesActions.test.ts
@@ -41,7 +41,6 @@ const CURRENT_DATASOURCE = {
columns: [],
metrics: [],
column_formats: {},
- currency_formats: {},
verbose_map: {},
main_dttm_col: '__timestamp',
// eg. ['["ds", true]', 'ds [asc]']
@@ -55,7 +54,6 @@ const NEW_DATASOURCE = {
columns: [],
metrics: [],
column_formats: {},
- currency_formats: {},
verbose_map: {},
main_dttm_col: '__timestamp',
// eg. ['["ds", true]', 'ds [asc]']
diff --git a/superset-frontend/src/explore/controlUtils/controlUtils.test.tsx
b/superset-frontend/src/explore/controlUtils/controlUtils.test.tsx
index e623fcb3f7..24fb753b8f 100644
--- a/superset-frontend/src/explore/controlUtils/controlUtils.test.tsx
+++ b/superset-frontend/src/explore/controlUtils/controlUtils.test.tsx
@@ -56,7 +56,6 @@ describe('controlUtils', () => {
{ metric_name: 'second', uuid: '2' },
],
column_formats: {},
- currency_formats: {},
verbose_map: {},
main_dttm_col: '',
datasource_name: '1__table',
diff --git
a/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.test.ts
b/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.test.ts
index b26ba8c1b4..cd8444346f 100644
---
a/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.test.ts
+++
b/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.test.ts
@@ -35,7 +35,6 @@ const sampleDatasource: Dataset = {
],
metrics: [{ metric_name: 'saved_metric_2', uuid: '1' }],
column_formats: {},
- currency_formats: {},
verbose_map: {},
main_dttm_col: '',
datasource_name: 'Sample Dataset',
diff --git a/superset-frontend/src/explore/fixtures.tsx
b/superset-frontend/src/explore/fixtures.tsx
index 211519590e..1e4afe9b61 100644
--- a/superset-frontend/src/explore/fixtures.tsx
+++ b/superset-frontend/src/explore/fixtures.tsx
@@ -138,7 +138,6 @@ export const exploreInitialData: ExplorePageInitialData = {
{ metric_name: 'second', uuid: '2' },
],
column_formats: {},
- currency_formats: {},
verbose_map: {},
main_dttm_col: '',
datasource_name: '8__table',
@@ -157,7 +156,6 @@ export const fallbackExploreInitialData:
ExplorePageInitialData = {
columns: [],
metrics: [],
column_formats: {},
- currency_formats: {},
verbose_map: {},
main_dttm_col: '',
owners: [],
diff --git a/superset-frontend/src/features/datasets/types.ts
b/superset-frontend/src/features/datasets/types.ts
index 63f1678d79..d343ad153d 100644
--- a/superset-frontend/src/features/datasets/types.ts
+++ b/superset-frontend/src/features/datasets/types.ts
@@ -78,7 +78,6 @@ export type DatasetObject = {
always_filter_main_dttm: boolean;
type: DatasourceType;
column_formats: Record<string, string>;
- currency_formats: Record<string, Currency>;
datasource_name: string | null;
verbose_map: Record<string, string>;
};
diff --git a/superset-frontend/src/utils/getDatasourceUid.test.ts
b/superset-frontend/src/utils/getDatasourceUid.test.ts
index ed7ec6256b..d3a629efcf 100644
--- a/superset-frontend/src/utils/getDatasourceUid.test.ts
+++ b/superset-frontend/src/utils/getDatasourceUid.test.ts
@@ -26,7 +26,6 @@ const TEST_DATASOURCE = {
columns: [],
metrics: [],
column_formats: {},
- currency_formats: {},
verbose_map: {},
main_dttm_col: '__timestamp',
// eg. ['["ds", true]', 'ds [asc]']
diff --git a/superset/connectors/sqla/models.py
b/superset/connectors/sqla/models.py
index d74b2ef5c0..275cc0cdbf 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -313,10 +313,6 @@ class BaseDatasource(AuditMixinNullable,
ImportExportMixin): # pylint: disable=
def column_formats(self) -> dict[str, str | None]:
return {m.metric_name: m.d3format for m in self.metrics if m.d3format}
- @property
- def currency_formats(self) -> dict[str, dict[str, str | None] | None]:
- return {m.metric_name: m.currency_json for m in self.metrics if
m.currency_json}
-
def add_missing_metrics(self, metrics: list[SqlMetric]) -> None:
existing_metrics = {m.metric_name for m in self.metrics}
for metric in metrics:
@@ -379,7 +375,6 @@ class BaseDatasource(AuditMixinNullable,
ImportExportMixin): # pylint: disable=
"id": self.id,
"uid": self.uid,
"column_formats": self.column_formats,
- "currency_formats": self.currency_formats,
"description": self.description,
"database": self.database.data, # pylint: disable=no-member
"default_endpoint": self.default_endpoint,
@@ -1046,7 +1041,7 @@ class SqlMetric(AuditMixinNullable, ImportExportMixin,
CertificationMixin, Model
metric_type = Column(String(32))
description = Column(utils.MediumText())
d3format = Column(String(128))
- currency = Column(String(128))
+ currency = Column(JSON, nullable=True)
warning_text = Column(Text)
table_id = Column(Integer, ForeignKey("tables.id", ondelete="CASCADE"))
expression = Column(utils.MediumText(), nullable=False)
@@ -1101,16 +1096,6 @@ class SqlMetric(AuditMixinNullable, ImportExportMixin,
CertificationMixin, Model
def get_perm(self) -> str | None:
return self.perm
- @property
- def currency_json(self) -> dict[str, str | None] | None:
- try:
- return json.loads(self.currency or "{}") or None
- except (TypeError, json.JSONDecodeError) as exc:
- logger.error(
- "Unable to load currency json: %r. Leaving empty.", exc,
exc_info=True
- )
- return None
-
@property
def data(self) -> dict[str, Any]:
attrs = (
diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py
index db581f42df..c9fa1e3617 100644
--- a/superset/dashboards/schemas.py
+++ b/superset/dashboards/schemas.py
@@ -259,7 +259,6 @@ class DashboardDatasetSchema(Schema):
id = fields.Int()
uid = fields.Str()
column_formats = fields.Dict()
- currency_formats = fields.Dict()
database = fields.Nested(DatabaseSchema)
default_endpoint = fields.String()
filter_select = fields.Bool()
diff --git a/superset/datasets/api.py b/superset/datasets/api.py
index 85cc075667..9b84885b4c 100644
--- a/superset/datasets/api.py
+++ b/superset/datasets/api.py
@@ -221,7 +221,6 @@ class DatasetRestApi(BaseSupersetModelRestApi):
"datasource_name",
"name",
"column_formats",
- "currency_formats",
"granularity_sqla",
"time_grain_sqla",
"order_by_choices",
diff --git a/superset/explore/schemas.py b/superset/explore/schemas.py
index 5149f98ff5..0c2ffd9386 100644
--- a/superset/explore/schemas.py
+++ b/superset/explore/schemas.py
@@ -25,7 +25,6 @@ class DatasetSchema(Schema):
}
)
column_formats = fields.Dict(metadata={"description": "Column formats."})
- currency_formats = fields.Dict(metadata={"description": "Currency
formats."})
columns = fields.List(fields.Dict(), metadata={"description": "Columns
metadata."})
database = fields.Dict(
metadata={"description": "Database associated with the dataset."}
diff --git a/superset/migrations/shared/utils.py
b/superset/migrations/shared/utils.py
index 293dae59a0..a0a096168a 100644
--- a/superset/migrations/shared/utils.py
+++ b/superset/migrations/shared/utils.py
@@ -22,7 +22,17 @@ from typing import Any, Callable, Optional, Union
from uuid import uuid4
from alembic import op
-from sqlalchemy import Column, inspect
+from sqlalchemy import (
+ Column,
+ inspect,
+ JSON,
+ MetaData,
+ select,
+ String,
+ Table,
+ text,
+ update,
+)
from sqlalchemy.dialects.mysql.base import MySQLDialect
from sqlalchemy.dialects.postgresql.base import PGDialect
from sqlalchemy.dialects.sqlite.base import SQLiteDialect # noqa: E402
@@ -468,3 +478,116 @@ def create_fks_for_table(
remote_cols,
ondelete=ondelete,
)
+
+
+def cast_text_column_to_json(
+ table: str,
+ column: str,
+ pk: str = "id",
+ nullable: bool = True,
+ suffix: str = "_tmp",
+) -> None:
+ """
+ Cast a text column to JSON.
+
+ SQLAlchemy now has a nice abstraction for JSON columns, even if the
underlying
+ database doesn't support the type natively. We should always use it when
storing
+ JSON payloads.
+
+ :param table: The name of the table.
+ :param column: The name of the column to be cast.
+ :param pk: The name of the primary key column.
+ :param nullable: Whether the new column should be nullable.
+ :param suffix: The suffix to be added to the temporary column name.
+ """
+ conn = op.get_bind()
+
+ if isinstance(conn.dialect, PGDialect):
+ conn.execute(
+ text(
+ f"""
+ ALTER TABLE {table}
+ ALTER COLUMN {column} TYPE jsonb
+ USING {column}::jsonb
+ """
+ )
+ )
+ return
+
+ tmp_column = column + suffix
+ op.add_column(
+ table,
+ Column(tmp_column, JSON(), nullable=nullable),
+ )
+
+ meta = MetaData()
+ t = Table(table, meta, autoload_with=conn)
+ stmt_select = select(t.c[pk], t.c[column]).where(t.c[column].is_not(None))
+
+ for row_pk, value in conn.execute(stmt_select):
+ stmt_update = update(t).where(t.c[pk] == row_pk).values({tmp_column:
value})
+ conn.execute(stmt_update)
+
+ op.drop_column(table, column)
+ op.alter_column(table, tmp_column, existing_type=JSON(),
new_column_name=column)
+
+ return
+
+
+def cast_json_column_to_text(
+ table: str,
+ column: str,
+ pk: str = "id",
+ nullable: bool = True,
+ suffix: str = "_tmp",
+ length: int = 128,
+) -> None:
+ """
+ Cast a JSON column back to text.
+
+ :param table: The name of the table.
+ :param column: The name of the column to be cast.
+ :param pk: The name of the primary key column.
+ :param nullable: Whether the new column should be nullable.
+ :param suffix: The suffix to be added to the temporary column name.
+ :param length: The length of the text column.
+ """
+ conn = op.get_bind()
+
+ if isinstance(conn.dialect, PGDialect):
+ conn.execute(
+ text(
+ f"""
+ ALTER TABLE {table}
+ ALTER COLUMN {column} TYPE text
+ USING {column}::text
+ """
+ )
+ )
+ return
+
+ tmp_column = column + suffix
+ op.add_column(
+ table,
+ Column(tmp_column, String(length=length), nullable=nullable),
+ )
+
+ meta = MetaData()
+ t = Table(table, meta, autoload_with=conn)
+ stmt_select = select(t.c[pk], t.c[column]).where(t.c[column].is_not(None))
+
+ for row_pk, value in conn.execute(stmt_select):
+ stmt_update = (
+ update(t).where(t.c[pk] == row_pk).values({tmp_column:
json.dumps(value)})
+ )
+ conn.execute(stmt_update)
+
+ op.drop_column(table, column)
+ op.alter_column(
+ table,
+ tmp_column,
+ existing_type=String(length=length),
+ new_column_name=column,
+ )
+
+ return
diff --git
a/superset/migrations/versions/2025-04-30_11-04_f1edd4a4d4f2_metric_currency_should_be_json.py
b/superset/migrations/versions/2025-04-30_11-04_f1edd4a4d4f2_metric_currency_should_be_json.py
new file mode 100644
index 0000000000..c01f2a33e9
--- /dev/null
+++
b/superset/migrations/versions/2025-04-30_11-04_f1edd4a4d4f2_metric_currency_should_be_json.py
@@ -0,0 +1,46 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""metric currency should be JSON
+
+Revision ID: f1edd4a4d4f2
+Revises: 378cecfdba9f
+Create Date: 2025-04-30 11:04:39.105229
+
+"""
+
+from superset.migrations.shared.utils import (
+ cast_json_column_to_text,
+ cast_text_column_to_json,
+)
+
+# revision identifiers, used by Alembic.
+revision = "f1edd4a4d4f2"
+down_revision = "378cecfdba9f"
+
+
+def upgrade():
+ """
+ Convert the currency column to JSON.
+ """
+ cast_text_column_to_json("sql_metrics", "currency")
+
+
+def downgrade():
+ """
+ Convert the currency column back to text.
+ """
+ cast_json_column_to_text("sql_metrics", "currency")
