codeant-ai-for-open-source[bot] commented on code in PR #38617:
URL: https://github.com/apache/superset/pull/38617#discussion_r3562900268
##########
tests/unit_tests/db_engine_specs/test_starrocks.py:
##########
@@ -359,3 +360,24 @@ def
test_get_prequeries_with_email_prefix_from_user_email_when_effective_user_di
assert StarRocksEngineSpec.get_prequeries(database) == [
'EXECUTE AS "alice.doe" WITH NO REVERT;'
]
+
+
+def test_time_grain_expressions_inherit_mysql() -> None:
+ """
+ Test that StarRocksEngineSpec inherits MySQL time grain expressions and
+ that the HOUR grain produces the correct SQL with DATE() preserved.
Review Comment:
**Suggestion:** The test docstring says the HOUR expression preserves
`DATE()`, but the assertion validates a `DATE_FORMAT(...)` expression with no
`DATE()` call. Update the docstring to match what the test actually verifies,
otherwise future maintainers will be misled about the regression being covered.
[comment mismatch]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
โ ๏ธ Test docstring misrepresents actual MySQL HOUR expression behavior.
โ ๏ธ Future maintainers may misunderstand regression coverage in StarRocks
tests.
```
</details>
<details>
<summary><b>Steps of Reproduction โ
</b></summary>
```mdx
1. Open `tests/unit_tests/db_engine_specs/test_starrocks.py` and locate
`test_time_grain_expressions_inherit_mysql()` at lines 26-32 (as shown by
the Read tool);
note the docstring text: \"Test that StarRocksEngineSpec inherits MySQL time
grain
expressions and that the HOUR grain produces the correct SQL with DATE()
preserved.\".
2. In the same test function, inspect the assertion at lines 36-44 (per Read
output):
`assert actual == "DATE_FORMAT(my_col, '%Y-%m-%d %H:00:00')"` which verifies
a
`DATE_FORMAT(...)` expression that does not contain any `DATE()` call.
3. Open `superset/db_engine_specs/mysql.py` and inspect
`_time_grain_expressions` at lines
246-260 (per Read output); confirm that `TimeGrain.HOUR` is defined as
`"DATE_FORMAT({col}, '%Y-%m-%d %H:00:00')"` with no `DATE()` wrapper,
matching the
StarRocks assertion string.
4. From these observations, conclude that the test docstring claims the HOUR
grain
preserves `DATE()`, but the actual behavior under test is that StarRocks
inherits MySQL's
HOUR time grain expression, which is a plain `DATE_FORMAT(...)` without
`DATE()`. The
mismatch can mislead future maintainers about what regression this test is
actually
covering.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8da19a85632e445e9ab8b090a3af398c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=8da19a85632e445e9ab8b090a3af398c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/db_engine_specs/test_starrocks.py
**Line:** 367:368
**Comment:**
*Comment Mismatch: The test docstring says the HOUR expression
preserves `DATE()`, but the assertion validates a `DATE_FORMAT(...)` expression
with no `DATE()` call. Update the docstring to match what the test actually
verifies, otherwise future maintainers will be misled about the regression
being covered.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38617&comment_hash=d182dbcc4f435b56717a0cee6f471c766b845a19acd3524f7c630405b410c419&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38617&comment_hash=d182dbcc4f435b56717a0cee6f471c766b845a19acd3524f7c630405b410c419&reaction=dislike'>๐</a>
--
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]
