Copilot commented on code in PR #45305:
URL: https://github.com/apache/superset/pull/45305#discussion_r4297360200
##########
superset/models/helpers.py:
##########
@@ -5489,11 +5489,17 @@ def get_sqla_query( # pylint:
disable=too-many-arguments,too-many-locals,too-ma
cond = or_(is_null_cond,
sqla_col.in_(eq_without_none))
else:
cond = is_null_cond
+ if op == utils.FilterOperator.NOT_IN:
+ cond = ~cond
else:
cond = sqla_col.in_(eq)
-
- if op == utils.FilterOperator.NOT_IN:
- cond = ~cond
+ if op == utils.FilterOperator.NOT_IN:
+ # ``NULL NOT IN (...)`` is not true (three-valued
+ # logic), so a plain negation would also drop NULL
+ # rows. Only the listed values are meant to be
+ # excluded, so keep NULL rows unless NULL itself is
+ # in the list (handled above).
+ cond = or_(sqla_col.is_(None), ~cond)
Review Comment:
Use `db_engine_spec.handle_null_filter(sqla_col,
utils.FilterOperator.IS_NULL)` for this new NULL check, as the explicit `IS
NULL` branch below does. Native KQL requires `isnull(column)`, supplied by
`KustoKqlEngineSpec.handle_null_filter`
(`superset/db_engine_specs/kusto.py:290–309`). Calling `.is_(None)` directly
adds an unsupported SQL-style NULL check to previously working `NOT IN` filters
without NULL operands. The engine hook preserves the intended behavior for SQL
backends while producing the required KQL expression.
--
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]