codeant-ai-for-open-source[bot] commented on code in PR #41184:
URL: https://github.com/apache/superset/pull/41184#discussion_r3562164380
##########
superset/db_engine_specs/presto.py:
##########
@@ -166,6 +166,7 @@ class PrestoBaseEngineSpec(BaseEngineSpec,
metaclass=ABCMeta):
supports_dynamic_schema = True
supports_catalog = supports_dynamic_catalog =
supports_cross_catalog_queries = True
+ supports_grouping_sets = True
Review Comment:
**Suggestion:** Add an explicit type annotation for this newly introduced
class attribute so the boolean capability flag is type-safe and compliant with
the typing rule. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This newly added class attribute is a boolean configuration value that can
be annotated (for example, `supports_grouping_sets: bool = True`). Since it is
added without a type hint, it matches the Python type-hint rule.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2ec64881f13f4328964d8133c44e849b&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=2ec64881f13f4328964d8133c44e849b&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:** superset/db_engine_specs/presto.py
**Line:** 169:169
**Comment:**
*Custom Rule: Add an explicit type annotation for this newly introduced
class attribute so the boolean capability flag is type-safe and compliant with
the typing rule.
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%2F41184&comment_hash=b31ff590d8c4a17f3088e7e120798706475261d85a30e44de7e364b800d976ba&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41184&comment_hash=b31ff590d8c4a17f3088e7e120798706475261d85a30e44de7e364b800d976ba&reaction=dislike'>๐</a>
##########
superset/common/query_context_processor.py:
##########
@@ -252,9 +255,58 @@ def get_query_result(self, query_object: QueryObject) ->
QueryResult:
This method delegates to the datasource's get_query_result method,
which handles query execution, normalization, time offsets, and
post-processing.
+
+ When the query requests rollup ``grouping_sets`` but the engine does
not
+ support native ``GROUPING SETS``, fall back to one query per level and
+ concatenate the results with ``GROUPING()``-equivalent markers, so the
+ combined result matches the shape the native path produces (SIP.md,
+ phase 3b). Engines that support it run the single native query.
"""
+ if query_object.grouping_sets and not self._supports_grouping_sets():
+ return self._grouping_sets_fallback(query_object)
return self._qc_datasource.get_query_result(query_object)
+ def _supports_grouping_sets(self) -> bool:
+ engine_spec = getattr(self._qc_datasource, "db_engine_spec", None)
Review Comment:
**Suggestion:** Add an explicit type annotation for this local variable so
the engine spec contract is clear and statically checkable. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This is a new local variable in modified Python code and it is
type-annotatable. The code omits an explicit type hint, which matches the rule
requiring type hints for relevant variables that can be annotated.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d31ec908143245968aa170d490ac9ed4&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=d31ec908143245968aa170d490ac9ed4&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:** superset/common/query_context_processor.py
**Line:** 270:270
**Comment:**
*Custom Rule: Add an explicit type annotation for this local variable
so the engine spec contract is clear and statically checkable.
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%2F41184&comment_hash=2c3afc7744f09839ebc3eb11a02c1d073d948449e6fec8dd2cbb831db78e10a8&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41184&comment_hash=2c3afc7744f09839ebc3eb11a02c1d073d948449e6fec8dd2cbb831db78e10a8&reaction=dislike'>๐</a>
##########
superset/common/query_context_processor.py:
##########
@@ -252,9 +255,58 @@ def get_query_result(self, query_object: QueryObject) ->
QueryResult:
This method delegates to the datasource's get_query_result method,
which handles query execution, normalization, time offsets, and
post-processing.
+
+ When the query requests rollup ``grouping_sets`` but the engine does
not
+ support native ``GROUPING SETS``, fall back to one query per level and
+ concatenate the results with ``GROUPING()``-equivalent markers, so the
+ combined result matches the shape the native path produces (SIP.md,
+ phase 3b). Engines that support it run the single native query.
"""
+ if query_object.grouping_sets and not self._supports_grouping_sets():
+ return self._grouping_sets_fallback(query_object)
return self._qc_datasource.get_query_result(query_object)
+ def _supports_grouping_sets(self) -> bool:
+ engine_spec = getattr(self._qc_datasource, "db_engine_spec", None)
+ return bool(engine_spec and engine_spec.supports_grouping_sets)
+
+ def _grouping_sets_fallback(self, query_object: QueryObject) ->
QueryResult:
+ """
+ Emulate a GROUPING SETS query on engines without native support: run
one
+ query per rollup level and concatenate, tagging each level's rows with
+ the same per-column markers the native path emits.
+ """
+ levels = query_object.grouping_sets
Review Comment:
**Suggestion:** Add a concrete type annotation for this variable to document
the expected grouping set structure and improve type safety in downstream loop
logic. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This modified Python code introduces an unannotated local variable whose
structure is used in the loop below. Since the variable is clearly
type-annotatable, the omission aligns with the type-hints rule.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=58631b59279648debb4b482873c04677&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=58631b59279648debb4b482873c04677&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:** superset/common/query_context_processor.py
**Line:** 279:279
**Comment:**
*Custom Rule: Add a concrete type annotation for this variable to
document the expected grouping set structure and improve type safety in
downstream loop logic.
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%2F41184&comment_hash=f5f61e902db660b961ed5ef5b884a539e3ea13c6b390a008d807f7b006be6428&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41184&comment_hash=f5f61e902db660b961ed5ef5b884a539e3ea13c6b390a008d807f7b006be6428&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]
