Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions superset/models/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -886,6 +886,60 @@ def _process_select_expression(
be properly parsed and validated.
"""
if expression:
# Fix for issue #35493: Quote column names with spaces to prevent SQLGlot
# from misinterpreting them as "column AS alias" syntax
# Only quote if it appears to be a simple identifier
# (no SQL operators or functions)
if (
" " in expression
and not any(
char in expression
for char in [
'"',
"'",
"`",
"[",
"(",
")",
"*",
"+",
"-",
"/",
"=",
"<",
">",
",",
]
)
and not any(
keyword in expression.upper()
for keyword in [
"SELECT",
"FROM",
"WHERE",
"AND",
"OR",
"AS",
"CASE",
"WHEN",
"THEN",
"ELSE",
"END",
"CAST",
"CONVERT",
"FUNCTION",
"SUM",
"COUNT",
"AVG",
"MAX",
"MIN",
"DISTINCT",
]
)
):
# This appears to be a simple column name with spaces, quote it
expression = self.database.quote_identifier(expression)

expression = f"SELECT {expression}"

if processed := self._process_sql_expression(
Expand Down
109 changes: 100 additions & 9 deletions tests/unit_tests/models/helpers_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -474,9 +474,9 @@ def condition_factory(col_name: str, expr):
has_single_quotes = "'Others'" in select_sql and "'Others'" in groupby_sql
has_double_quotes = '"Others"' in select_sql and '"Others"' in groupby_sql

assert has_single_quotes or has_double_quotes, (
"Others literal should be quoted with either single or double quotes"
)
assert (
has_single_quotes or has_double_quotes
), "Others literal should be quoted with either single or double quotes"

# Verify the structure of the generated SQL
assert "CASE WHEN" in select_sql
Expand Down Expand Up @@ -1118,10 +1118,101 @@ def test_process_select_expression_end_to_end(database: Database) -> None:
# sqlglot may normalize the SQL slightly, so we check the result exists
# and doesn't contain the SELECT prefix
assert result is not None, f"Failed to process: {expression}"
assert not result.upper().startswith("SELECT"), (
f"Result still has SELECT prefix: {result}"
)
assert not result.upper().startswith(
"SELECT"
), f"Result still has SELECT prefix: {result}"
# The result should contain the core expression (case-insensitive check)
assert expected.replace(" ", "").lower() in result.replace(" ", "").lower(), (
f"Expected '{expected}' to be in result '{result}' for input '{expression}'"
)
assert (
expected.replace(" ", "").lower() in result.replace(" ", "").lower()
), f"Expected '{expected}' to be in result '{result}' for input '{expression}'"


def test_process_select_expression_column_names_with_spaces(
database: Database,
) -> None:
"""
Test for issue #35493: Column names with spaces should be quoted
to prevent SQLGlot from misinterpreting them as "column AS alias" syntax.
"""
from superset.connectors.sqla.models import SqlaTable

table = SqlaTable(
table_name="test_table",
database=database,
)

# Test 1: Simple column name with spaces - should be quoted and not misinterpreted
result = table._process_select_expression(
expression="Test Column",
database_id=database.id,
engine="sqlite",
schema="",
template_processor=None,
)

# The result should be a quoted identifier, not "Test AS Column"
assert result is not None
assert (
"AS" not in result or result.count("AS") <= 1
) # Allow one AS if it's part of a proper alias
# Should contain the full column name in some quoted form
assert (
"Test Column" in result
or '"Test Column"' in result
or "'Test Column'" in result
)

# Test 2: Complex expression with spaces - should NOT be pre-quoted (let SQLGlot handle it)
result = table._process_select_expression(
expression="col1 * 10",
database_id=database.id,
engine="sqlite",
schema="",
template_processor=None,
)

# Should process the expression without breaking it
assert result is not None
assert "*" in result # The multiplication should be preserved

# Test 3: Expression with SQL keywords - should NOT be pre-quoted
result = table._process_select_expression(
expression="UPPER(name)",
database_id=database.id,
engine="sqlite",
schema="",
template_processor=None,
)

# Should process the function call correctly
assert result is not None
assert "UPPER" in result
assert "name" in result

# Test 4: Already quoted column - should NOT be quoted again
result = table._process_select_expression(
expression='"Already Quoted Column"',
database_id=database.id,
engine="sqlite",
schema="",
template_processor=None,
)

# Should preserve the original quoting
assert result is not None
assert '"Already Quoted Column"' in result

# Test 5: Column name with spaces that should trigger the fix
# This simulates the exact issue from #35493
result = table._process_select_expression(
expression="Customer Name",
database_id=database.id,
engine="sqlite",
schema="",
template_processor=None,
)

# The key test: should NOT result in "Customer AS Name"
assert result is not None
assert result != "Customer AS Name"
assert "Customer Name" in result or '"Customer Name"' in result
Loading