sadpandajoe commented on code in PR #45305:
URL: https://github.com/apache/superset/pull/45305#discussion_r4301341721


##########
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:
   Fixed in a8eeb458bc — the NULL check that keeps NULL rows for `NOT IN` is 
now built by `db_engine_spec.handle_null_filter(sqla_col, IS_NULL)` in the 
dataset path and in the dashboard scope SQL mirror, so KQL gets `isnull(col)` 
and SQL engines still get `col IS NULL`. Tests cover both paths with the Kusto 
spec.



-- 
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]

Reply via email to