codeant-ai-for-open-source[bot] commented on code in PR #41961:
URL: https://github.com/apache/superset/pull/41961#discussion_r3565146110
##########
tests/unit_tests/models/helpers_test.py:
##########
@@ -3410,3 +3410,51 @@ 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
+
+ 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 flag missing explicit type annotations for test-local variables in
test files when the code is already type-checking cleanly (e.g. mypy is green).
**Applied to:**
- `**/test/**`
- `**/tests/**`
- `**/*test*.<ext>`
---
💡 *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,51 @@ 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
+
+ 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": "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 concrete type hints for test-local variables when the test
file already passes mypy and the code is otherwise clear.
**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]
