diff --git a/superset-frontend/packages/superset-ui-core/src/query/normalizeTimeColumn.ts b/superset-frontend/packages/superset-ui-core/src/query/normalizeTimeColumn.ts index 33c27f233763..b5bde0c88dbf 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/normalizeTimeColumn.ts +++ b/superset-frontend/packages/superset-ui-core/src/query/normalizeTimeColumn.ts @@ -67,6 +67,7 @@ export function normalizeTimeColumn( sqlExpression: formData.x_axis, label: formData.x_axis, expressionType: 'SQL', + isColumnReference: true, }; } diff --git a/superset-frontend/packages/superset-ui-core/src/query/types/Column.ts b/superset-frontend/packages/superset-ui-core/src/query/types/Column.ts index a12990dc7485..0fe1317e3dea 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/types/Column.ts +++ b/superset-frontend/packages/superset-ui-core/src/query/types/Column.ts @@ -27,6 +27,7 @@ export interface AdhocColumn { optionName?: string; sqlExpression: string; expressionType: 'SQL'; + isColumnReference?: boolean; columnType?: 'BASE_AXIS' | 'SERIES'; timeGrain?: string; datasourceWarning?: boolean; @@ -74,6 +75,10 @@ export function isAdhocColumn(column?: any): column is AdhocColumn { ); } +export function isAdhocColumnReference(column?: any): column is AdhocColumn { + return isAdhocColumn(column) && column?.isColumnReference === true; +} + export function isQueryFormColumn(column: any): column is QueryFormColumn { return isPhysicalColumn(column) || isAdhocColumn(column); } diff --git a/superset-frontend/packages/superset-ui-core/test/query/normalizeTimeColumn.test.ts b/superset-frontend/packages/superset-ui-core/test/query/normalizeTimeColumn.test.ts index e00fa683c028..b1d0eebb8c39 100644 --- a/superset-frontend/packages/superset-ui-core/test/query/normalizeTimeColumn.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/query/normalizeTimeColumn.test.ts @@ -86,6 +86,7 @@ test('should support different columns for x-axis and granularity', () => { { timeGrain: 'P1Y', columnType: 'BASE_AXIS', + isColumnReference: true, sqlExpression: 'time_column_in_x_axis', label: 'time_column_in_x_axis', expressionType: 'SQL', diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/buildQuery.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/buildQuery.test.ts index b97163336ce0..ffabe67df149 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/buildQuery.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/buildQuery.test.ts @@ -101,36 +101,35 @@ describe('queryObject conversion', () => { it('should convert queryObject', () => { const { queries } = buildQuery({ ...formData, x_axis: 'time_column' }); - expect(queries[0]).toEqual( - expect.objectContaining({ - granularity: 'time_column', - time_range: '1 year ago : 2013', - extras: { having: '', where: '', time_grain_sqla: 'P1Y' }, - columns: [ - { - columnType: 'BASE_AXIS', - expressionType: 'SQL', - label: 'time_column', - sqlExpression: 'time_column', - timeGrain: 'P1Y', - }, - 'col1', - ], - series_columns: ['col1'], - metrics: ['count(*)'], - post_processing: [ - { - operation: 'pivot', - options: { - aggregates: { 'count(*)': { operator: 'mean' } }, - columns: ['col1'], - drop_missing_columns: true, - index: ['time_column'], - }, + expect(queries[0]).toMatchObject({ + granularity: 'time_column', + time_range: '1 year ago : 2013', + extras: { having: '', where: '', time_grain_sqla: 'P1Y' }, + columns: [ + { + columnType: 'BASE_AXIS', + expressionType: 'SQL', + label: 'time_column', + sqlExpression: 'time_column', + timeGrain: 'P1Y', + isColumnReference: true, + }, + 'col1', + ], + series_columns: ['col1'], + metrics: ['count(*)'], + post_processing: [ + { + operation: 'pivot', + options: { + aggregates: { 'count(*)': { operator: 'mean' } }, + columns: ['col1'], + drop_missing_columns: true, + index: ['time_column'], }, - { operation: 'flatten' }, - ], - }), - ); + }, + { operation: 'flatten' }, + ], + }); }); }); diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index 0ef9572c5072..a4c12dd00cc8 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -1502,8 +1502,14 @@ def adhoc_column_to_sqla( # pylint: disable=too-many-locals """ label = utils.get_column_name(col) try: + sql_expression = col["sqlExpression"] + + # For column references, conditionally quote identifiers that need it + if col.get("isColumnReference"): + sql_expression = self.database.quote_identifier(sql_expression) + expression = self._process_select_expression( - expression=col["sqlExpression"], + expression=sql_expression, database_id=self.database_id, engine=self.database.backend, schema=self.schema, diff --git a/superset/superset_typing.py b/superset/superset_typing.py index 02a4a32f2498..c3460d80823d 100644 --- a/superset/superset_typing.py +++ b/superset/superset_typing.py @@ -59,6 +59,7 @@ class AdhocColumn(TypedDict, total=False): hasCustomLabel: Optional[bool] label: str sqlExpression: str + isColumnReference: Optional[bool] columnType: Optional[Literal["BASE_AXIS", "SERIES"]] timeGrain: Optional[str] diff --git a/tests/unit_tests/models/helpers_test.py b/tests/unit_tests/models/helpers_test.py index 8154e5013649..6f434c4f64ea 100644 --- a/tests/unit_tests/models/helpers_test.py +++ b/tests/unit_tests/models/helpers_test.py @@ -29,6 +29,8 @@ from sqlalchemy.orm.session import Session from sqlalchemy.pool import StaticPool +from superset.superset_typing import AdhocColumn + if TYPE_CHECKING: from superset.models.core import Database @@ -1125,3 +1127,34 @@ def test_process_select_expression_end_to_end(database: Database) -> None: assert expected.replace(" ", "").lower() in result.replace(" ", "").lower(), ( f"Expected '{expected}' to be in result '{result}' for input '{expression}'" ) + + +def test_adhoc_column_to_sqla_with_column_reference(database: Database) -> None: + """ + Test that adhoc_column_to_sqla + properly quotes column identifiers when isColumnReference is true. + + This tests the fix for column names with spaces being properly quoted + before being processed by SQLGlot to prevent "column AS alias" misinterpretation. + """ + from superset.connectors.sqla.models import SqlaTable + + table = SqlaTable( + table_name="test_table", + database=database, + ) + + # Test 1: Column reference with spaces should be quoted + col_with_spaces: AdhocColumn = { + "sqlExpression": "Customer Name", + "label": "Customer Name", + "isColumnReference": True, + } + + result = table.adhoc_column_to_sqla(col_with_spaces) + + # Should contain the quoted column name + assert result is not None + result_str = str(result) + + assert '"Customer Name"' in result_str