You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by be...@apache.org on 2025/11/20 23:29:35 UTC
(superset) branch fix-column-quoting created (now 804a5eb7cc)
This is an automated email from the ASF dual-hosted git repository.
beto pushed a change to branch fix-column-quoting
in repository https://gitbox.apache.org/repos/asf/superset.git
at 804a5eb7cc Add test
This branch includes the following new commits:
new 695987e8c7 fix: adhoc column quoting
new 804a5eb7cc Add test
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 02/02: Add test
Posted by be...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
beto pushed a commit to branch fix-column-quoting
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 804a5eb7cc36207c5f52f826b6ed3d6b68b7e519
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Thu Nov 20 18:29:24 2025 -0500
Add test
---
tests/unit_tests/models/helpers_test.py | 113 +++++++++++++++++++++++++++++---
1 file changed, 105 insertions(+), 8 deletions(-)
diff --git a/tests/unit_tests/models/helpers_test.py b/tests/unit_tests/models/helpers_test.py
index dfbcd9a278..5783cf0489 100644
--- a/tests/unit_tests/models/helpers_test.py
+++ b/tests/unit_tests/models/helpers_test.py
@@ -1395,20 +1395,23 @@ def test_reapply_query_filters_with_empty_filters(database: Database) -> None:
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.
+ Test that adhoc_column_to_sqla properly handles column references
+ by looking up the column in metadata instead of quoting and processing through SQLGlot.
- This tests the fix for column names with spaces being properly quoted
- before being processed by SQLGlot to prevent "column AS alias" misinterpretation.
+ This tests the fix for column names with spaces being properly handled
+ without going through SQLGlot which could misinterpret "column AS alias" patterns.
"""
- from superset.connectors.sqla.models import SqlaTable
+ from superset.connectors.sqla.models import SqlaTable, TableColumn
table = SqlaTable(
table_name="test_table",
database=database,
+ columns=[
+ TableColumn(column_name="Customer Name", type="TEXT"),
+ ],
)
- # Test 1: Column reference with spaces should be quoted
+ # Test: Column reference with spaces should be found in metadata
col_with_spaces: AdhocColumn = {
"sqlExpression": "Customer Name",
"label": "Customer Name",
@@ -1417,8 +1420,102 @@ def test_adhoc_column_to_sqla_with_column_reference(database: Database) -> None:
result = table.adhoc_column_to_sqla(col_with_spaces)
- # Should contain the quoted column name
+ # Should return a valid SQLAlchemy column
+ assert result is not None
+ result_str = str(result)
+
+ # The column name should be present (may or may not be quoted depending on dialect)
+ assert "Customer Name" in result_str or '"Customer Name"' in result_str
+
+
+def test_adhoc_column_to_sqla_preserves_column_type_for_time_grain(
+ database: Database,
+) -> None:
+ """
+ Test that adhoc_column_to_sqla preserves column type information when using column references.
+
+ This tests the fix where column references now look up metadata first, preserving
+ type information needed for time grain operations. Previously, quoting the column name
+ before metadata lookup would cause the column to not be found, resulting in NULL type
+ and failing to apply time grain transformations properly.
+
+ The test verifies that:
+ 1. Column metadata is found by looking up the unquoted column name
+ 2. The column type (DATE) is preserved when creating the SQLAlchemy column
+ 3. The get_timestamp_expr method is properly called with the column type info
+ """
+ from superset.connectors.sqla.models import SqlaTable, TableColumn
+
+ # Create a table with a temporal column
+ table = SqlaTable(
+ table_name="test_table",
+ database=database,
+ columns=[
+ TableColumn(
+ column_name="local_date",
+ type="DATE",
+ is_dttm=True,
+ )
+ ],
+ )
+
+ # Test with a DATE column reference with time grain
+ date_col: AdhocColumn = {
+ "sqlExpression": "local_date",
+ "label": "local_date",
+ "isColumnReference": True,
+ "timeGrain": "P1D", # Daily time grain
+ "columnType": "BASE_AXIS",
+ }
+
+ # Should not raise ColumnNotFoundException
+ result = table.adhoc_column_to_sqla(date_col)
+
assert result is not None
result_str = str(result)
- assert '"Customer Name"' in result_str
+ # Verify the column name is present (may be quoted depending on dialect)
+ assert "local_date" in result_str
+
+
+def test_adhoc_column_to_sqla_with_temporal_column_types(database: Database) -> None:
+ """
+ Test that adhoc_column_to_sqla correctly handles different temporal column types.
+
+ This verifies that for different temporal types (DATE, DATETIME, TIMESTAMP),
+ the column metadata is properly found and the column type is preserved,
+ allowing time grain operations to work correctly.
+ """
+ from superset.connectors.sqla.models import SqlaTable, TableColumn
+
+ # Test different temporal types
+ temporal_types = ["DATE", "DATETIME", "TIMESTAMP"]
+
+ for type_name in temporal_types:
+ table = SqlaTable(
+ table_name="test_table",
+ database=database,
+ columns=[
+ TableColumn(
+ column_name="time_col",
+ type=type_name,
+ is_dttm=True,
+ )
+ ],
+ )
+
+ time_col: AdhocColumn = {
+ "sqlExpression": "time_col",
+ "label": "time_col",
+ "isColumnReference": True,
+ "timeGrain": "P1D",
+ "columnType": "BASE_AXIS",
+ }
+
+ result = table.adhoc_column_to_sqla(time_col)
+
+ assert result is not None
+ result_str = str(result)
+
+ # Verify the column name is present
+ assert "time_col" in result_str
(superset) 01/02: fix: adhoc column quoting
Posted by be...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
beto pushed a commit to branch fix-column-quoting
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 695987e8c7da3c40c1053ee9d8c2357e2d6c55d0
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Thu Nov 20 18:04:15 2025 -0500
fix: adhoc column quoting
---
superset/connectors/sqla/models.py | 39 ++++++++++++++++++++------------------
1 file changed, 21 insertions(+), 18 deletions(-)
diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py
index 3afe9fabea..a4d6ca75ae 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -1509,33 +1509,34 @@ class SqlaTable(
:rtype: sqlalchemy.sql.column
"""
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=sql_expression,
- database_id=self.database_id,
- engine=self.database.backend,
- schema=self.schema,
- template_processor=template_processor,
- )
- except SupersetSecurityException as ex:
- raise QueryObjectValidationError(ex.message) from ex
+ sql_expression = col["sqlExpression"]
time_grain = col.get("timeGrain")
has_timegrain = col.get("columnType") == "BASE_AXIS" and time_grain
is_dttm = False
pdf = None
- if col_in_metadata := self.get_column(expression):
+ if col.get("isColumnReference"):
+ col_in_metadata = self.get_column(sql_expression)
+ if col_in_metadata is None:
+ # should not happen
+ raise ColumnNotFoundException("Column not found")
+
sqla_column = col_in_metadata.get_sqla_col(
template_processor=template_processor
)
is_dttm = col_in_metadata.is_temporal
pdf = col_in_metadata.python_date_format
else:
+ try:
+ expression = self._process_select_expression(
+ expression=sql_expression,
+ database_id=self.database_id,
+ engine=self.database.backend,
+ schema=self.schema,
+ template_processor=template_processor,
+ )
+ except SupersetSecurityException as ex:
+ raise QueryObjectValidationError(ex.message) from ex
+
sqla_column = literal_column(expression)
if has_timegrain or force_type_check:
try:
@@ -1866,7 +1867,9 @@ class SqlaTable(
def default_query(qry: Query) -> Query:
return qry.filter_by(is_sqllab_view=False)
- def has_extra_cache_key_calls(self, query_obj: QueryObjectDict) -> bool: # noqa: C901
+ def has_extra_cache_key_calls(
+ self, query_obj: QueryObjectDict
+ ) -> bool: # noqa: C901
"""
Detects the presence of calls to `ExtraCache` methods in items in query_obj that
can be templated. If any are present, the query must be evaluated to extract