Copilot commented on code in PR #35848:
URL: https://github.com/apache/superset/pull/35848#discussion_r2470563695
##########
tests/unit_tests/db_engine_specs/test_db2.py:
##########
@@ -78,3 +81,70 @@ def test_get_prequeries(mocker: MockerFixture) -> None:
assert Db2EngineSpec.get_prequeries(database, schema="my_schema") == [
'set current_schema "my_schema"'
]
+
+
[email protected](
+ ("grain", "expected_expression"),
+ [
+ (None, "my_col"),
+ (
+ TimeGrain.SECOND,
+ "CAST(my_col as TIMESTAMP) - MICROSECOND(my_col) MICROSECONDS",
+ ),
+ (
+ TimeGrain.MINUTE,
+ "CAST(my_col as TIMESTAMP)"
+ " - SECOND(my_col) SECONDS - MICROSECOND(my_col) MICROSECONDS",
+ ),
+ (
+ TimeGrain.HOUR,
+ "CAST(my_col as TIMESTAMP)"
+ " - MINUTE(my_col) MINUTES"
+ " - SECOND(my_col) SECONDS - MICROSECOND(my_col) MICROSECONDS",
+ ),
+ (TimeGrain.DAY, "DATE(my_col)"),
+ (TimeGrain.WEEK, "my_col - (DAYOFWEEK(my_col)) DAYS"),
+ (TimeGrain.MONTH, "my_col - (DAY(my_col)-1) DAYS"),
+ (
+ TimeGrain.QUARTER,
+ "my_col - (DAY(my_col)-1) DAYS"
+ " - (MONTH(my_col)-1) MONTHS + ((QUARTER(my_col)-1) * 3) MONTHS",
+ ),
+ (
+ TimeGrain.YEAR,
+ "my_col - (DAY(my_col)-1) DAYS - (MONTH(my_col)-1) MONTHS",
+ ),
+ ],
+)
+def test_time_grain_expressions(grain: TimeGrain, expected_expression: str) ->
None:
+ """
+ Test that time grain expressions generate the expected SQL.
+ """
+ from superset.db_engine_specs.db2 import Db2EngineSpec
+
+ actual = Db2EngineSpec._time_grain_expressions[grain].format(col="my_col")
+ assert actual == expected_expression
+
+
+def test_time_grain_day_parseable() -> None:
+ """
+ Test that the DAY time grain expression generates valid SQL
+ that can be parsed by sqlglot.
+
+ This test addresses the bug where the previous expression
+ "CAST({col} as TIMESTAMP) - HOUR({col}) HOURS - ..."
+ could not be parsed by sqlglot.
+ """
+ from superset.db_engine_specs.db2 import Db2EngineSpec
+
+ expression = Db2EngineSpec._time_grain_expressions[TimeGrain.DAY].format(
+ col="my_timestamp_col",
+ )
Review Comment:
The `noqa: S608` comment suppresses a security warning for SQL injection.
While the expression comes from a trusted source (engine specs), consider using
a safer approach like sqlglot's builder API to construct the query, or add a
comment explaining why this is safe in this specific test context.
```suggestion
)
# The interpolated value in this SQL string comes from a trusted, static
source
# (Db2EngineSpec._time_grain_expressions), and this query is only
parsed, not executed.
# Therefore, this usage is safe in this test context.
```
--
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]
