You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by jo...@apache.org on 2023/06/21 16:45:22 UTC

[superset] branch master updated: fix: Native filter dynamic numeric search (#24418)

This is an automated email from the ASF dual-hosted git repository.

johnbodley pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 652bf6454e fix: Native filter dynamic numeric search (#24418)
652bf6454e is described below

commit 652bf6454e6e10f5986f1aee36d6d3dcad601453
Author: John Bodley <45...@users.noreply.github.com>
AuthorDate: Wed Jun 21 09:45:15 2023 -0700

    fix: Native filter dynamic numeric search (#24418)
---
 .../filters/components/Select/buildQuery.test.ts   |  8 +++---
 .../src/filters/components/Select/buildQuery.ts    | 16 ++++--------
 superset/connectors/base/models.py                 |  9 ++++++-
 superset/models/helpers.py                         | 30 ++++++++++++++--------
 4 files changed, 38 insertions(+), 25 deletions(-)

diff --git a/superset-frontend/src/filters/components/Select/buildQuery.test.ts b/superset-frontend/src/filters/components/Select/buildQuery.test.ts
index c89719dc83..c643ab927b 100644
--- a/superset-frontend/src/filters/components/Select/buildQuery.test.ts
+++ b/superset-frontend/src/filters/components/Select/buildQuery.test.ts
@@ -97,7 +97,7 @@ describe('Select buildQuery', () => {
     expect(query.orderby).toEqual([['my_col', false]]);
   });
 
-  it('should add text search parameter to query filter', () => {
+  it('should add text search parameter for string to query filter', () => {
     const queryContext = buildQuery(formData, {
       ownState: {
         search: 'abc',
@@ -111,7 +111,7 @@ describe('Select buildQuery', () => {
     ]);
   });
 
-  it('should add numeric search parameter to query filter', () => {
+  it('should add text search parameter for numeric to query filter', () => {
     const queryContext = buildQuery(formData, {
       ownState: {
         search: '123',
@@ -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%' },
+    ]);
   });
 });
diff --git a/superset-frontend/src/filters/components/Select/buildQuery.ts b/superset-frontend/src/filters/components/Select/buildQuery.ts
index d9a5b3c229..45d05cffd0 100644
--- a/superset-frontend/src/filters/components/Select/buildQuery.ts
+++ b/superset-frontend/src/filters/components/Select/buildQuery.ts
@@ -39,22 +39,16 @@ const buildQuery: BuildQuery<PluginFilterSelectQueryFormData> = (
     if (search) {
       columns.filter(isPhysicalColumn).forEach(column => {
         const label = getColumnLabel(column);
-        if (coltypeMap[label] === GenericDataType.STRING) {
+        if (
+          coltypeMap[label] === GenericDataType.STRING ||
+          (coltypeMap[label] === GenericDataType.NUMERIC &&
+            !Number.isNaN(Number(search)))
+        ) {
           extraFilters.push({
             col: column,
             op: 'ILIKE',
             val: `%${search}%`,
           });
-        } else if (
-          coltypeMap[label] === GenericDataType.NUMERIC &&
-          !Number.isNaN(Number(search))
-        ) {
-          // for numeric columns we apply a >= where clause
-          extraFilters.push({
-            col: column,
-            op: '>=',
-            val: Number(search),
-          });
         }
       });
     }
diff --git a/superset/connectors/base/models.py b/superset/connectors/base/models.py
index 8396da911c..647df374c9 100644
--- a/superset/connectors/base/models.py
+++ b/superset/connectors/base/models.py
@@ -439,7 +439,14 @@ class BaseDatasource(
             if isinstance(value, str):
                 value = value.strip("\t\n")
 
-                if target_generic_type == utils.GenericDataType.NUMERIC:
+                if (
+                    target_generic_type == utils.GenericDataType.NUMERIC
+                    and operator
+                    not in {
+                        utils.FilterOperator.ILIKE,
+                        utils.FilterOperator.LIKE,
+                    }
+                ):
                     # For backwards compatibility and edge cases
                     # where a column data type might have changed
                     return utils.cast_to_num(value)
diff --git a/superset/models/helpers.py b/superset/models/helpers.py
index 42d5a24174..dcffe24a48 100644
--- a/superset/models/helpers.py
+++ b/superset/models/helpers.py
@@ -1173,7 +1173,14 @@ class ExploreMixin:  # pylint: disable=too-many-public-methods
             if isinstance(value, str):
                 value = value.strip("\t\n")
 
-                if target_generic_type == utils.GenericDataType.NUMERIC:
+                if (
+                    target_generic_type == utils.GenericDataType.NUMERIC
+                    and operator
+                    not in {
+                        utils.FilterOperator.ILIKE,
+                        utils.FilterOperator.LIKE,
+                    }
+                ):
                     # For backwards compatibility and edge cases
                     # where a column data type might have changed
                     return utils.cast_to_num(value)
@@ -1744,10 +1751,7 @@ class ExploreMixin:  # pylint: disable=too-many-public-methods
                         tbl_column=col_obj, template_processor=template_processor
                     )
                 col_type = col_obj.type if col_obj else None
-                col_spec = db_engine_spec.get_column_spec(
-                    native_type=col_type,
-                    #                    db_extra=self.database.get_extra(),
-                )
+                col_spec = db_engine_spec.get_column_spec(native_type=col_type)
                 is_list_target = op in (
                     utils.FilterOperator.IN.value,
                     utils.FilterOperator.NOT_IN.value,
@@ -1766,7 +1770,6 @@ class ExploreMixin:  # pylint: disable=too-many-public-methods
                     target_native_type=col_type,
                     is_list_target=is_list_target,
                     db_engine_spec=db_engine_spec,
-                    #                     db_extra=self.database.get_extra(),
                 )
                 if (
                     col_advanced_data_type != ""
@@ -1848,10 +1851,17 @@ class ExploreMixin:  # pylint: disable=too-many-public-methods
                         where_clause_and.append(sqla_col >= eq)
                     elif op == utils.FilterOperator.LESS_THAN_OR_EQUALS.value:
                         where_clause_and.append(sqla_col <= eq)
-                    elif op == utils.FilterOperator.LIKE.value:
-                        where_clause_and.append(sqla_col.like(eq))
-                    elif op == utils.FilterOperator.ILIKE.value:
-                        where_clause_and.append(sqla_col.ilike(eq))
+                    elif op in {
+                        utils.FilterOperator.ILIKE,
+                        utils.FilterOperator.LIKE,
+                    }:
+                        if target_generic_type != GenericDataType.STRING:
+                            sqla_col = sa.cast(sqla_col, sa.String)
+
+                        if utils.FilterOperator.LIKE.value:
+                            where_clause_and.append(sqla_col.like(eq))
+                        else:
+                            where_clause_and.append(sqla_col.ilike(eq))
                     elif (
                         op == utils.FilterOperator.TEMPORAL_RANGE.value
                         and isinstance(eq, str)