rusackas opened a new pull request, #45773:
URL: https://github.com/apache/superset/pull/45773
### SUMMARY
T-SQL's `IS` keyword only supports NULL checks (`IS [NOT] NULL`); there is
no generic `IS <literal>` predicate. `BaseEngineSpec.handle_boolean_filter`'s
default path compiles a boolean filter with SQLAlchemy's `column.is_(value)`,
which under the real `mssql` dialect emits invalid T-SQL:
```python
>>> col.is_(True).compile(dialect=mssql.dialect(),
compile_kwargs={"literal_binds": True})
'test_col IS 1'
>>> col.is_(False).compile(dialect=mssql.dialect(),
compile_kwargs={"literal_binds": True})
'test_col IS 0'
```
`AthenaEngineSpec`, `ClickHouseEngineSpec`, `SnowflakeEngineSpec`, and
`PrestoBaseEngineSpec` (→ `TrinoEngineSpec`) already work around the same class
of dialect behavior by setting `use_equality_for_boolean_filters = True`, which
makes `handle_boolean_filter` emit `col = true`/`col = false` instead.
`MssqlEngineSpec` never set this flag, so a SQL Server `BIT` column (reflected
as `mssql.BIT`, a direct subclass of `sqltypes.Boolean`) hits Superset's full
`IS_TRUE`/`IS_FALSE` filter UI today and generates invalid SQL against a real
SQL Server instance.
This PR sets `use_equality_for_boolean_filters = True` on `MssqlEngineSpec`.
`SybaseEngineSpec` subclasses `MssqlEngineSpec` directly and shares T-SQL's `IS
NULL`-only grammar, so it inherits the fix with no separate change needed.
### TESTING INSTRUCTIONS
```
pytest tests/unit_tests/db_engine_specs/test_mssql.py -v
pytest tests/unit_tests/db_engine_specs/test_sybase.py -v
```
All 53 `test_mssql.py` tests pass (including the two new ones), and the
existing `test_sybase.py` test still passes. The new
`test_handle_boolean_filter` compiles the result against the real `mssql`
dialect and asserts it now reads `test_col = 1` / `test_col = 0` instead of the
invalid `test_col IS 1` / `test_col IS 0`. `pre-commit run` is clean on both
changed files.
### 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
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
