gabotorresruiz commented on code in PR #44899:
URL: https://github.com/apache/superset/pull/44899#discussion_r4190345209
##########
superset/common/query_object.py:
##########
@@ -494,6 +494,18 @@ def _is_valid_date_range(self, date_range: str) -> bool:
return False
def _sanitize_filters(self) -> None:
+ if (
+ self.datasource
+ and self.datasource.type == "semantic_view"
+ and (self.extras.get("where") or self.extras.get("having"))
+ ):
+ raise QueryObjectValidationError(
+ _(
+ "SQL WHERE/HAVING expressions are not supported for
semantic "
+ "views. Remove the SQL clause or use a semantic dimension
filter."
+ )
+ )
Review Comment:
Not a blocker, more of a direction question. The mapper still carries
`_get_filters_from_extras` (`superset/semantic_layers/mapper.py:500`), which
deliberately turns these same extras into `Operator.ADHOC` filters and has its
own cases in `mapper_test.py`, and `fetch_values_predicate` goes through the
same operator. With this check every `QueryObject` path rejects before the
mapper runs, so that conversion becomes unreachable. Is retiring it the intent,
or would the better shape be to skip only the SQLA sanitizer here and let the
provider accept or reject `ADHOC` filters, the way
`ADHOC_EXPRESSIONS_IN_ORDERBY` is already a per-provider capability? I can see
the argument for rejecting, since without `sanitize_clause` a pass-through
would hand unsanitized caller SQL to the provider. If rejecting is the call, a
follow-up that removes or annotates the now dead mapper branch would keep the
two from drifting.
--
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]
