mirror of
https://github.com/apache/superset.git
synced 2026-04-17 07:05:04 +00:00
fix(sql): quote column names with spaces to prevent SQLGlot parsing errors (#35553)
This commit is contained in:
committed by
GitHub
parent
310dcd7b94
commit
306f4c14cf
@@ -67,6 +67,7 @@ export function normalizeTimeColumn(
|
||||
sqlExpression: formData.x_axis,
|
||||
label: formData.x_axis,
|
||||
expressionType: 'SQL',
|
||||
isColumnReference: true,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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',
|
||||
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'],
|
||||
},
|
||||
'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' },
|
||||
],
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1502,8 +1502,14 @@ class SqlaTable(
|
||||
"""
|
||||
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,
|
||||
|
||||
@@ -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]
|
||||
|
||||
|
||||
@@ -29,6 +29,8 @@ from sqlalchemy import create_engine
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user