sadpandajoe opened a new pull request, #45317:
URL: https://github.com/apache/superset/pull/45317
### SUMMARY
The negated filter operators `!=`, `NOT LIKE` and `NOT ILIKE` on a
SQLAlchemy-backed dataset also removed rows whose column value is NULL, not
only the named values. For example `state != 'CA'` dropped the `CA` rows and
the rows where `state` is NULL. Only the named values should be excluded, which
is also how a `NOT IN` filter should behave (see #45305).
`ExploreMixin.get_sqla_query` emitted the bare SQLAlchemy operator for these
filters. Under SQL three-valued logic `NULL != 'CA'` and `NULL NOT LIKE '%C%'`
are not true, so the `WHERE` clause discarded the NULL rows. With this change a
negated predicate with a non-NULL operand is built as `<column> IS NULL OR
<negated predicate>`:
- scalar `!=`, `NOT LIKE` and `NOT ILIKE`; the NULL check is applied to the
column itself, not to the string cast used for pattern matching on non-string
columns;
- whole-array `!=` and `NOT IN` on multi-value columns (compile-level,
defensive);
- `!=` and `NOT IN` on advanced data type columns (internet address, port),
applied to the plugin's translated predicate;
- the `!=` clause built by the MCP dashboard scope SQL, so it agrees with
the dataset path.
The NULL check goes through the engine spec's `handle_null_filter`, so
engines with their own NULL syntax are honored. An explicit NULL operand keeps
its meaning: `!= NULL` is still `IS NOT NULL`, and a `NOT IN` list that
contains NULL still excludes NULL rows. `=`, `IN`, `LIKE`, `ILIKE` and the
comparison operators are unchanged, and so is plain `NOT IN`, which #45305
handles. Metric filters (HAVING) share the same code path, so `metric != x` now
also keeps rows whose aggregate is NULL.
`UPDATING.md` documents the behavior change, because saved charts and
reports that use these operators can now return additional NULL rows.
### Root Cause
- **Verified:** the final comparison/pattern branch of `get_sqla_query`
passed `!=` straight to `BaseEngineSpec.handle_comparison_filter` (`sqla_col !=
value`) and built `NOT LIKE` / `NOT ILIKE` with `sqla_col.not_like(eq)` /
`sqla_col.not_ilike(eq)`. Running real `SqlaTable.get_sqla_query` queries
against SQLite with rows `CA, NY, TX, NULL`: `!= 'CA'` returned `NY, TX`, `NOT
LIKE '%C%'` and `NOT ILIKE '%c%'` returned `NY, TX`; the NULL row was missing
in each. The same held for numeric, boolean, temporal, calculated and ad-hoc
SQL columns, for pattern matching on numeric and UUID columns, and for a metric
in HAVING.
- **Verified:** `handle_comparison_filter` is defined only in
`BaseEngineSpec` and has two callers (`get_sqla_query` and the MCP dashboard
scope SQL); no engine overrides it. `handle_null_filter` is overridden only by
the Kusto KQL spec.
- **History:** each operator was added as a 1:1 mapping to the SQLAlchemy
operator (`!=` in 2017, `NOT LIKE` in 2024, `NOT ILIKE` in 2026) without
considering NULL column values; no commit states an intent to drop NULL rows.
- **Not changed here:** plain `NOT IN` and the MCP `_in_list_clause`
(#45305); the Explore Custom SQL text generated for simple filters in the
frontend, which still excludes NULL rows and is left for a follow-up; the
semantic layer, whose NULL handling is separate and intentionally pinned by its
own tests.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable: backend query generation only, no UI change. Regression
guard output (real `get_sqla_query` executed on SQLite with a NULL row, plus
compile-level cases for array columns):
- Before the fix: 44 of the new guard cases fail with `Extra items in the
right set: None` (the NULL row is missing) or a missing NULL check in the
compiled SQL; the 29 pins for behavior that must not change pass.
- After the fix: all 73 guard cases pass; `tests/unit_tests/models` and
`tests/unit_tests/mcp_service` pass (11590 passed, 12 skipped).
### TESTING INSTRUCTIONS
1. Run the new tests:
`pytest tests/unit_tests/models/helpers_test.py
tests/unit_tests/models/test_multivalue_filter.py
tests/unit_tests/mcp_service/test_dashboard_scope_sql.py -k "negated or
unchanged_null or engine_spec_hook or advanced_data_type or metric_filters"`.
2. Manual check: create a chart on a dataset whose text column has some NULL
values, add the filter `state != 'CA'` (or `NOT LIKE '%C%'` through a custom
filter), and confirm only the matching values are excluded while NULL rows
remain. A `!= NULL` filter still returns only non-NULL rows.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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]
