betodealmeida opened a new pull request, #32701:
URL: https://github.com/apache/superset/pull/32701

   <!---
   Please write the PR title following the conventions at 
https://www.conventionalcommits.org/en/v1.0.0/
   Example:
   fix(dashboard): load charts correctly
   -->
   
   ### SUMMARY
   <!--- Describe the change below, including rationale and design decisions -->
   
   When filtering boolean columns in the chart builder we're using the operator 
`==` for `IS TRUE` and `IS FALSE`, instead of the native operators:
   
   ```typescript
     [Operators.IsTrue]: { display: t('Is true'), operation: '==' },
     [Operators.IsFalse]: { display: t('Is false'), operation: '==' },
   ```
   
   There are two problems with this approach. First, the rendered filtered 
looks ugly, showing `VALUE = true` instead of `VALUE IS TRUE`:
   
   <img width="386" alt="Screenshot 2025-03-17 at 10 44 33 AM" 
src="https://github.com/user-attachments/assets/ea704e1e-abee-4694-90da-2591061e72ef";
 />
   
   <img width="322" alt="Screenshot 2025-03-17 at 10 44 43 AM" 
src="https://github.com/user-attachments/assets/32b12173-3aae-480e-9cf9-3301688b9672";
 />
   
   Second, the filter in the request payload looks like this:
   
   ```json
   {col: "value", op: "==", val: true}
   ```
   
   When the SQLAlchemy query is generated, the filter will be built like this:
   
   ```python
   where_clause_and.append(sqla_col == eq)
   ```
   
   Which is iffy and error prone, specially because many databases don't 
support native boolean types, using integers for them. This could potentially 
result in broken comparisons between a boolean and an integer, for example.
   
   It's much better to map the `IS TRUE` and `IS FALSE` to the `IS TRUE` and 
`IS FALSE` operators, since then the SQLAlchemy filter is generated like this:
   
   ```python
   where_clause_and.append(sqla_col.is_(True))
   ```
   
   This PR fixes the problem by mapping `IS TRUE` and `IS FALSE` to the correct 
operators (and cleaning up some code). With these changes, the UI looks correct:
   
   <img width="319" alt="Screenshot 2025-03-17 at 11 11 26 AM" 
src="https://github.com/user-attachments/assets/a78e86ae-3f3b-44ca-996a-24848e2a9565";
 />
   
   And the request payload:
   
   ```json
   {col: "value", op: "IS TRUE"}
   ```
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   
   See discussion above.
   
   ### TESTING INSTRUCTIONS
   <!--- Required! What steps can be taken to manually verify the changes? -->
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] 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]

Reply via email to