codeant-ai-for-open-source[bot] commented on code in PR #41961:
URL: https://github.com/apache/superset/pull/41961#discussion_r3564826418


##########
tests/unit_tests/models/helpers_test.py:
##########
@@ -3410,3 +3410,52 @@ def fake_engine(*args, **kwargs):
     # ```forecasts.original`.`total_cost``` (the regression), so this negative
     # assertion catches the actual failure mode, not just an exact-string 
match.
     assert "`forecasts.original`" not in sql
+
+
+def test_simple_metric_quotes_column_requiring_quoting(database: Database) -> 
None:
+    """
+    Regression for #30637: a SIMPLE adhoc metric that aggregates a column whose
+    name requires identifier quoting (e.g. it contains a space) must render the
+    column with the dialect's quote characters, otherwise the emitted SQL is
+    invalid for dialects that need quoting.
+
+    This exercises the metric compilation path named in the issue
+    (``adhoc_metric_to_sqla`` -> ``TableColumn.get_sqla_col`` -> ``column()``),
+    asserting the identifier is quoted rather than emitted bare.
+    """
+    from superset.connectors.sqla.models import SqlaTable, TableColumn
+    from superset.utils.core import AdhocMetricExpressionType
+
+    column_name = "Amount HT"
+    table = SqlaTable(
+        database=database,
+        schema=None,
+        table_name="test_table",
+        columns=[TableColumn(column_name=column_name, type="INTEGER")],
+    )
+    columns_by_name = {col.column_name: col for col in table.columns}

Review Comment:
   ✅ **Customized review instruction saved!**
   
   **Instruction:**
   > Do not require explicit type annotations for local variables inside tests; 
only flag missing annotations in production code or where the type hint is 
genuinely necessary.
   
   **Applied to:**
     - `**/test/**`
     - `**/tests/**`
     - `**/*test*.py`
   
   ---
   💡 *To manage or update this instruction, visit: [CodeAnt AI 
Settings](https://app.codeant.ai/org/settings/learnings)*



##########
tests/unit_tests/models/helpers_test.py:
##########
@@ -3410,3 +3410,52 @@ def fake_engine(*args, **kwargs):
     # ```forecasts.original`.`total_cost``` (the regression), so this negative
     # assertion catches the actual failure mode, not just an exact-string 
match.
     assert "`forecasts.original`" not in sql
+
+
+def test_simple_metric_quotes_column_requiring_quoting(database: Database) -> 
None:
+    """
+    Regression for #30637: a SIMPLE adhoc metric that aggregates a column whose
+    name requires identifier quoting (e.g. it contains a space) must render the
+    column with the dialect's quote characters, otherwise the emitted SQL is
+    invalid for dialects that need quoting.
+
+    This exercises the metric compilation path named in the issue
+    (``adhoc_metric_to_sqla`` -> ``TableColumn.get_sqla_col`` -> ``column()``),
+    asserting the identifier is quoted rather than emitted bare.
+    """
+    from superset.connectors.sqla.models import SqlaTable, TableColumn
+    from superset.utils.core import AdhocMetricExpressionType
+
+    column_name = "Amount HT"
+    table = SqlaTable(
+        database=database,
+        schema=None,
+        table_name="test_table",
+        columns=[TableColumn(column_name=column_name, type="INTEGER")],
+    )
+    columns_by_name = {col.column_name: col for col in table.columns}
+
+    metric: AdhocMetric = {
+        "expressionType": AdhocMetricExpressionType.SIMPLE,
+        "aggregate": "SUM",
+        "column": {"column_name": column_name},
+        "label": "total",
+    }
+
+    sqla_metric = table.adhoc_metric_to_sqla(metric, columns_by_name)

Review Comment:
   ✅ **Customized review instruction saved!**
   
   **Instruction:**
   > Do not require explicit type annotations for test-local dicts or other 
local test fixtures; rely on existing type checking in test files.
   
   **Applied to:**
     - `**/test/**`
     - `**/tests/**`
     - `**/*test*.py`
   
   ---
   💡 *To manage or update this instruction, visit: [CodeAnt AI 
Settings](https://app.codeant.ai/org/settings/learnings)*



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to