fix(trino/presto): use equality for boolean filters to support computed columns#39500
Conversation
…ed columns Selecting IS TRUE / IS FALSE on a boolean chart filter raised an error on Trino/Presto when the column was derived from a cast like `(expiration = 1) AS expiration`. Switch these engines to generate `col = true`/`col = false` instead of `col IS true`/`col IS false`, matching the existing Athena behavior. The flag is set on PrestoBaseEngineSpec so both TrinoEngineSpec and PrestoEngineSpec inherit the fix.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #39500 +/- ##
=======================================
Coverage 64.44% 64.44%
=======================================
Files 2560 2560
Lines 133574 133575 +1
Branches 31017 31017
=======================================
+ Hits 86082 86083 +1
Misses 45999 45999
Partials 1493 1493
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
The original bug (sc-97540) was specifically about computed boolean expressions like `(expiration = 1) AS expiration`, not plain boolean columns. Add a case using `literal_column` to pin that scenario.
Code Review Agent Run #e1dd96Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
aminghadersohi
left a comment
There was a problem hiding this comment.
Straightforward fix — sets use_equality_for_boolean_filters = True on PrestoBaseEngineSpec, which covers both Presto and Trino. Matches the existing pattern in Athena (#34598) and Snowflake. Tests cover IS_TRUE, IS_FALSE, and the computed-column regression case.
SUMMARY
Selecting
IS TRUE(orIS FALSE) on a boolean chart filter raised an error on Trino/Presto when the column was derived from a cast such as(expiration = 1) AS expiration. The backend generatedcol IS true, which Trino/Presto reject on these computed boolean expressions.This PR sets
use_equality_for_boolean_filters = TrueonPrestoBaseEngineSpec, so bothTrinoEngineSpecandPrestoEngineSpecemitcol = true/col = falseinstead. This matches the existing Athena behavior introduced in #34598 (Athena is Presto-based and hit the same issue).Fixes internal ticket sc-97540.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — purely a SQL generation fix.
Before:
SELECT ... WHERE expiration IS true→ query error on Trino/Presto.After:
SELECT ... WHERE expiration = true→ succeeds.TESTING INSTRUCTIONS
(expiration = 1) AS expiration.expirationcolumn with operatorIS TRUE(orIS FALSE).expiration = trueinstead ofexpiration IS true.Unit tests added:
tests/unit_tests/db_engine_specs/test_trino.py::test_handle_boolean_filtertests/unit_tests/db_engine_specs/test_presto.py::test_handle_boolean_filterADDITIONAL INFORMATION