EnxDev commented on code in PR #44986:
URL: https://github.com/apache/superset/pull/44986#discussion_r4200869614
##########
superset/models/helpers.py:
##########
@@ -5396,7 +5396,17 @@ def get_sqla_query( # pylint:
disable=too-many-arguments,too-many-locals,too-ma
# CONTAINS_ANY/CONTAINS_ALL also produce a list ``eq``
(they
# are in ``is_list_target``), but are element-level array
ops
# handled by their own branch below — not IN.
- assert isinstance(eq, (tuple, list))
+ if not isinstance(eq, (tuple, list)):
+ # A missing or null value (``val`` absent or ``None``)
+ # leaves ``eq`` as ``None``. Reject it rather than
+ # dropping the filter, which would widen the results.
+ raise QueryObjectValidationError(
Review Comment:
The advanced data type branch above (around line 5330) still gets this same
`None`. With `ENABLE_ADVANCED_DATA_TYPES` on and an IN filter with no `val` on
an internet_address or port column, `filter_values_handler` returns `None`
before it wraps, so `translate_type` ends up doing `for val in None` and that's
a TypeError, still a 500.
Not blocking here. Could we hoist this check above the branch split, or add
it to the follow-ups next to the multi-value case?
--
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]
