bito-code-review[bot] commented on PR #44526:
URL: https://github.com/apache/superset/pull/44526#issuecomment-5814481340
<!-- Bito Reply -->
The flagged issue is correct. The logic added in
`superset/common/query_context_factory.py` correctly identifies the need to
recover `granularity` for legacy queries, but it does not account for the
scenario where a temporal filter exists in the query object's filters without a
corresponding `time_range`. In such cases, the subsequent logic in
`_apply_granularity` (which removes filters that match the granularity) may
inadvertently remove the only remaining temporal restriction, leaving the query
unfiltered.
To resolve this, you should ensure that the filter removal logic only
proceeds if the temporal filter is redundant or if a valid replacement is
guaranteed. I recommend adding a check to verify if a `time_range` or
equivalent temporal constraint exists before allowing the filter removal to
proceed.
There are no other comments on this PR to address.
**superset/common/query_context_factory.py**
```
if granularity := query_object.granularity:
# Ensure we do not remove the only temporal filter if no
time_range exists
if not query_object.time_range and not
self._has_temporal_filter(query_object):
return
filter_to_remove = None
```
--
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]
