fix: Native filter dynamic numeric search #24418

john-bodley · 2023-06-16T06:34:03Z

SUMMARY

This PR provides a fix for #14710 in addition to being an alternative formulation for #24413. Additionally it provides LIKE and ILIKE operator support for SIMPLE ad-hoc filters irrespective of the column type, i.e., now all operators are valid.

This change came about give how #14710 implemented dynamic (backend) filtering of numerical values for native filters.

Hypothetically lets assume there was an id column which comprised of the set of integers from 0 to 100,000 and a user searched for 100, then:

A backend query was sent with a SIMPLE filter of the form {col: 'id', op: '>=', val: 100}.
The result set (limited to 1,000) would contain the values: 100, 101, 102, ..., 1,097, 1,098, 1,099
The frontend select filter would filter out any result not match the 100 substring resulting in only 10 matches: 100, 1,001, 1,002, 1,003, 1,004, 1,005, 1,006, 1,007, 1,008, 1,009.
Numerous valid matches like 1,100, 2,100, 10,000, etc. aren't shown as they're not part of the corpus of 1,000 results fetched from the backend.

The right way to see if a number is contained within another number is to treat the numbers as a string and provide a filter of the form {col: 'id', op: 'ILIKE', val: '%100%'}. This also remedies the BIGINT issue where in #24413 the numerical values were treated as a string.

The ad-hoc filters include the ILIKE and LIKE operator which work exclusively with STRING types. This PR simply provides "support" for using both these operators by casting the column (if necessary) to a STRING, i.e., it means that the ILIKE and LIKE operator work for all SIMPLE filters irrespective of the column type, which can then be leveraged by the native filter search functionality.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

BEFORE

AFTER

Dynamic casting

TESTING INSTRUCTIONS

CI.

ADDITIONAL INFORMATION

Has associated issue:
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

john-bodley · 2023-06-16T06:34:57Z

superset/models/helpers.py

@@ -1173,7 +1173,13 @@ def handle_single_value(value: Optional[FilterValue]) -> Optional[FilterValue]:
            if isinstance(value, str):
                value = value.strip("\t\n")

-                if target_generic_type == utils.GenericDataType.NUMERIC:
+                if (


I'm going to turn a blind eye to the fact that this function is copy-and-pasted in multiple places. Furthermore it's rather scary there's zero unit tests for the filter_values_handler method.

codecov · 2023-06-16T07:23:03Z

Codecov Report

Merging #24418 (16d23cd) into master (23bb1c4) will decrease coverage by 0.01%.
The diff coverage is 36.36%.

❗ Current head 16d23cd differs from pull request most recent head 729cc0e. Consider uploading reports for the commit 729cc0e to get more accurate results

@@            Coverage Diff             @@
##           master   #24418      +/-   ##
==========================================
- Coverage   68.91%   68.91%   -0.01%     
==========================================
  Files        1904     1904              
  Lines       73920    73920              
  Branches     8119     8118       -1     
==========================================
- Hits        50944    50941       -3     
- Misses      20865    20868       +3     
  Partials     2111     2111

Flag	Coverage Δ
hive	`53.92% <22.22%> (-0.01%)`	⬇️
javascript	`55.65% <50.00%> (-0.01%)`	⬇️
mysql	`79.19% <33.33%> (-0.01%)`	⬇️
postgres	`79.28% <33.33%> (-0.01%)`	⬇️
presto	`53.84% <22.22%> (-0.01%)`	⬇️
python	`83.27% <33.33%> (-0.01%)`	⬇️
sqlite	`77.78% <33.33%> (-0.01%)`	⬇️
unit	`54.62% <0.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
superset/models/helpers.py	`70.54% <25.00%> (-0.25%)`	⬇️
...ontend/src/filters/components/Select/buildQuery.ts	`88.88% <50.00%> (-1.12%)`	⬇️
superset/connectors/base/models.py	`87.57% <100.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

justinpark · 2023-06-16T16:35:05Z

superset-frontend/src/filters/components/Select/buildQuery.test.ts

@@ -120,6 +120,8 @@ describe('Select buildQuery', () => {
    });
    expect(queryContext.queries.length).toEqual(1);
    const [query] = queryContext.queries;
-    expect(query.filters).toEqual([{ col: 'my_col', op: '>=', val: 123 }]);
+    expect(query.filters).toEqual([
+      { col: 'my_col', op: 'ILIKE', val: '%123%' },


btw i think LIKE can be enough for since it matches only numbers, right?

nvm. I don't find any performance benefit of using LIKE over ILIKE.

I'm good with it

michael-s-molina

LGTM

(cherry picked from commit 652bf64)

pull-request-size bot added the size/M label Jun 16, 2023

john-bodley commented Jun 16, 2023

View reviewed changes

john-bodley force-pushed the john-bodley--filter-select-like branch from 66b970d to c4602ae Compare June 16, 2023 06:47

john-bodley requested review from villebro, justinpark and michael-s-molina June 16, 2023 06:47

john-bodley changed the title ~~fix: Native filter dynamic search~~ fix: Native filter dynamic numeric search Jun 16, 2023

john-bodley marked this pull request as ready for review June 16, 2023 07:16

john-bodley force-pushed the john-bodley--filter-select-like branch from c4602ae to 06c1168 Compare June 16, 2023 07:55

fix: Native filter dynamic search

729cc0e

john-bodley force-pushed the john-bodley--filter-select-like branch from 06c1168 to 729cc0e Compare June 16, 2023 08:26

john-bodley mentioned this pull request Jun 16, 2023

fix(dashboard): native filters do not preserve bigint #24413

Closed

9 tasks

justinpark reviewed Jun 16, 2023

View reviewed changes

justinpark approved these changes Jun 16, 2023

View reviewed changes

michael-s-molina approved these changes Jun 21, 2023

View reviewed changes

john-bodley merged commit 652bf64 into apache:master Jun 21, 2023

john-bodley added a commit to airbnb/superset-fork that referenced this pull request Jun 21, 2023

fix: Native filter dynamic numeric search (apache#24418)

9a438e8

(cherry picked from commit 652bf64)

john-bodley mentioned this pull request Jun 23, 2023

fix: Native filter dynamic numeric search #24506

Merged

9 tasks

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 3.0.0 labels Mar 8, 2024

vinothkumar66 pushed a commit to vinothkumar66/superset that referenced this pull request Nov 11, 2024

fix: Native filter dynamic numeric search (apache#24418)

80f1554

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Native filter dynamic numeric search #24418

fix: Native filter dynamic numeric search #24418

john-bodley commented Jun 16, 2023 •

edited

Loading

john-bodley Jun 16, 2023

codecov bot commented Jun 16, 2023 •

edited

Loading

justinpark Jun 16, 2023

justinpark Jun 16, 2023

michael-s-molina left a comment

fix: Native filter dynamic numeric search #24418

fix: Native filter dynamic numeric search #24418

Conversation

john-bodley commented Jun 16, 2023 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

BEFORE

AFTER

Dynamic casting

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

john-bodley Jun 16, 2023

Choose a reason for hiding this comment

codecov bot commented Jun 16, 2023 • edited Loading

Codecov Report

justinpark Jun 16, 2023

Choose a reason for hiding this comment

justinpark Jun 16, 2023

Choose a reason for hiding this comment

michael-s-molina left a comment

Choose a reason for hiding this comment

john-bodley commented Jun 16, 2023 •

edited

Loading

codecov bot commented Jun 16, 2023 •

edited

Loading