fix(sql): quote column names with spaces to prevent SQLGlot parsing errors (apache#35553)

Author: msyavuz

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,
       };
     }
 

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);
 }

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',

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' },
+      ],
+    });
   });
 });

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,

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]
 

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