codeant-ai-for-open-source[bot] commented on code in PR #42895:
URL: https://github.com/apache/superset/pull/42895#discussion_r3816996079
##########
tests/unit_tests/models/helpers_test.py:
##########
@@ -2133,6 +2133,65 @@ def
test_adhoc_metric_to_sqla_invalid_simple_aggregate_raises_validation_error(
table.adhoc_metric_to_sqla(metric, {})
+def test_adhoc_metric_to_sqla_extended_aggregate_on_supported_engine() -> None:
+ """
+ MEDIAN/STDDEV_SAMP/VAR_SAMP compile correctly end-to-end on an engine that
+ supports them (Postgres), via the same `adhoc_metric_to_sqla` path every
+ other aggregate uses -- no pivot-table-specific code involved.
Review Comment:
**Suggestion:** The test claims to verify `MEDIAN`, `STDDEV_SAMP`, and
`VAR_SAMP`, but the metric uses only `MEDIAN` and the assertion checks only its
`percentile_cont` SQL. Regressions in either standard-deviation or variance
compilation would therefore pass this test; parameterize the test over all
three aggregates and assert each expected SQL expression. [incomplete
implementation]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ STDDEV_SAMP helper-dispatch regressions remain undetected.
- โ ๏ธ VAR_SAMP helper-dispatch regressions remain undetected.
- โ ๏ธ Existing PostgreSQL tests cover mappings, not integration dispatch.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/models/helpers_test.py
**Line:** 2138:2140
**Comment:**
*Incomplete Implementation: The test claims to verify `MEDIAN`,
`STDDEV_SAMP`, and `VAR_SAMP`, but the metric uses only `MEDIAN` and the
assertion checks only its `percentile_cont` SQL. Regressions in either
standard-deviation or variance compilation would therefore pass this test;
parameterize the test over all three aggregates and assert each expected SQL
expression.
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%2F42895&comment_hash=235729ebad51ae782d7247e630c85b8633fbe81f98074d932ec60db01c4f9188&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42895&comment_hash=235729ebad51ae782d7247e630c85b8633fbe81f98074d932ec60db01c4f9188&reaction=dislike'>๐</a>
##########
tests/unit_tests/db_engine_specs/test_duckdb.py:
##########
@@ -191,3 +191,26 @@ def test_fetch_data_preserves_cursor_description(mocker:
MockerFixture) -> None:
assert [col[0] for col in cursor.description] == ["col1", "col2"]
finally:
raw_conn.close()
+
+
+def test_extended_aggregation_func_median_stddev_var() -> None:
+ """
+ Verified against a live in-process duckdb instance, including under
+ GROUPING SETS: values match the same-input results from postgres/mysql
+ exactly.
+ """
Review Comment:
**Suggestion:** The test documentation claims live execution and GROUPING
SETS/value verification, but the test only compiles SQLAlchemy expressions and
never connects to DuckDB or executes a query. This can pass even if the
aggregate functions are unsupported or produce incorrect results; either
execute representative queries or narrow the docstring to describe
compilation-only coverage. [docstring mismatch]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ DuckDB aggregate runtime behavior is not validated by this unit test.
- โ ๏ธ GROUPING SETS result equivalence remains untested.
- โ ๏ธ Unsupported or semantically incorrect SQL could pass CI.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
<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_duckdb.py
**Line:** 197:201
**Comment:**
*Docstring Mismatch: The test documentation claims live execution and
GROUPING SETS/value verification, but the test only compiles SQLAlchemy
expressions and never connects to DuckDB or executes a query. This can pass
even if the aggregate functions are unsupported or produce incorrect results;
either execute representative queries or narrow the docstring to describe
compilation-only coverage.
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%2F42895&comment_hash=95f193700a36a5dacb73b9d795526e5e7bdd798e06c00482f73ab4e5fe25e3b3&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42895&comment_hash=95f193700a36a5dacb73b9d795526e5e7bdd798e06c00482f73ab4e5fe25e3b3&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]
