Skip to content

fix: Fixed numeric filtering issue in Superset - #33222

Closed
Sayan199 wants to merge 2 commits into
apache:masterfrom
Sayan199:fix-numeric-filtering
Closed

fix: Fixed numeric filtering issue in Superset#33222
Sayan199 wants to merge 2 commits into
apache:masterfrom
Sayan199:fix-numeric-filtering

Conversation

@Sayan199

@Sayan199 Sayan199 commented Apr 23, 2025

Copy link
Copy Markdown

SUMMARY

Fixes #33206

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

In order to test this, we need to

  1. Add a filter for a numeric field that contains both integer and decimal values (e.g., 21, 21.8, 25.35, etc.).
  2. First select an integer value (for example, 21) in the filter, and then fractional values.
    This should now be able to filter and give results that do match with 21 but also with 21.8 and 25.35

ADDITIONAL INFORMATION

  • Has associated issue: Incorrect filtering for numeric values with different decimal precision #33206
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • 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

@Sayan199 Sayan199 closed this Apr 23, 2025
@Sayan199 Sayan199 reopened this Apr 23, 2025

@korbit-ai korbit-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.
Category Issue Status
Documentation Missing temporal range value handling explanation ▹ view 🧠 Not in standard
Files scanned
File Path Reviewed
superset/models/helpers.py

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

Comment on lines 1162 to 1164
def handle_single_value(value: Optional[FilterValue]) -> Optional[FilterValue]:
if operator == utils.FilterOperator.TEMPORAL_RANGE:
return value

This comment was marked as resolved.

@rusackas rusackas changed the title Fixed numeric filtering issue in Superset fix: Fixed numeric filtering issue in Superset Apr 23, 2025
@rusackas

Copy link
Copy Markdown
Member

Looks like there are a bunch of CI tasks failing, but we should probably start by running pre-commit to fix the linting/formatting issues, and see if anything else gets unblocked.

Superset uses Git pre-commit hooks courtesy of pre-commit. To install run the following:

pip3 install -r requirements/development.txt
pre-commit install
A series of checks will now run when you make a git commit.

Alternatively it is possible to run pre-commit by running pre-commit manually:

pre-commit run --all-files

@michael-s-molina

Copy link
Copy Markdown
Member

Possible duplicate of #33230

@rusackas

rusackas commented Aug 5, 2025

Copy link
Copy Markdown
Member

@Sayan199 curious if you're able to fix the CI issues, or if you think this is indeed a duplicate of #33230 (where you're also welcome to review or add thoughts)

@bito-code-review

bito-code-review Bot commented Mar 11, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #73fd41

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: db19474..15172eb
    • superset/models/helpers.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@rusackas

rusackas commented Mar 11, 2026

Copy link
Copy Markdown
Member

Thanks for the contribution. After reviewing the diff more carefully, there's a structural bug that would need to be addressed: the if target_generic_type == utils.GenericDataType.BOOLEAN block and final return value at the bottom of handle_single_value were accidentally dedented out of that nested function into the outer filter_values_handler scope, which would break boolean filter handling.

The possible duplicate (#33230) was also closed without merging, so that concern is moot, but the underlying fix here needs another look before it would be safe to merge.

@rusackas rusackas closed this Mar 11, 2026
@rusackas rusackas reopened this Mar 11, 2026
@rusackas
rusackas marked this pull request as draft March 11, 2026 21:59
@rusackas

Copy link
Copy Markdown
Member

Marking as draft until it an get further attention.

@rusackas

Copy link
Copy Markdown
Member

Thanks for the contribution, @Sayan199. As noted in review, the diff has a structural bug — the BOOLEAN branch and the final return value in handle_single_value got dedented out of the intended block — and it's been idle since. Closing this draft; the underlying issue #33206 stays open and we'll pick it back up there (ideally with a regression test). Feel free to reopen a fresh PR if you'd like to continue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect filtering for numeric values with different decimal precision

3 participants