Copilot commented on code in PR #35848:
URL: https://github.com/apache/superset/pull/35848#discussion_r2470431757
##########
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 ",
Review Comment:
The expected expression for `TimeGrain.HOUR` includes a trailing space at
the end of the string (`" - SECOND(my_col) SECONDS - MICROSECOND(my_col)
MICROSECONDS "`). This matches the current implementation in
`superset/db_engine_specs/db2.py` line 50, but the trailing space appears to be
unintentional. Consider removing the trailing space from both the source code
and this test to improve consistency and maintainability.
```suggestion
" - SECOND(my_col) SECONDS - MICROSECOND(my_col) MICROSECONDS",
```
##########
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:
Review Comment:
The type hint for `grain` parameter should be `TimeGrain | None` or
`Optional[TimeGrain]` since one of the test cases passes `None` as the value
(line 89).
--
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]
