bito-code-review[bot] commented on PR #43578:
URL: https://github.com/apache/superset/pull/43578#issuecomment-5435311769
<!-- Bito Reply -->
The flagged issue is correct. When `FORCE_LIMIT` wraps a query in a
subquery, it can break ClickHouse-specific top-level semantics like `WITH
TOTALS`, which only apply to the outermost query.
To resolve this, you should ensure that queries containing `WITH TOTALS` are
not wrapped in a subquery when applying a limit. You can modify
`set_limit_value` in `superset/sql/parse.py` to check for the presence of `WITH
TOTALS` in the parsed query and force the use of `WRAP_SQL` only when it is
safe to do so, or otherwise handle the limit at the outer level.
Here is a concise implementation to check for `WITH TOTALS`:
```python
def _has_with_totals(self) -> bool:
"""
Check if the statement has a ClickHouse `WITH TOTALS` clause.
"""
return bool(self._parsed.args.get("with_totals"))
```
Then, update the logic in `set_limit_value` to avoid wrapping if
`_has_with_totals()` is true. Would you like me to fetch all other comments on
this PR to validate and implement fixes for them as well?
**superset/sql/parse.py**
```
def _has_with_totals(self) -> bool:
"""
Check if the statement has a ClickHouse `WITH TOTALS` clause.
"""
return bool(self._parsed.args.get("with_totals"))
```
--
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]
