Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Commit

Permalink
fix: Native filter dynamic numeric search (apache#24418)
Browse files Browse the repository at this point in the history
  • Loading branch information
@john-bodley
john-bodley authored Jun 21, 2023
1 parent 92e2ee9 commit 652bf64
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 25 deletions.
8 changes: 5 additions & 3 deletions superset-frontend/src/filters/components/Select/buildQuery.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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',
Expand All @@ -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%' },
]);
});
});
16 changes: 5 additions & 11 deletions superset-frontend/src/filters/components/Select/buildQuery.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
});
}
});
}
Expand Down
9 changes: 8 additions & 1 deletion superset/connectors/base/models.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,14 @@ def handle_single_value(value: FilterValue | None) -> FilterValue | None:
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)
Expand Down
30 changes: 20 additions & 10 deletions superset/models/helpers.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1173,7 +1173,14 @@ 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 (
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)
Expand Down Expand Up @@ -1744,10 +1751,7 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma
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,
Expand All @@ -1766,7 +1770,6 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma
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 != ""
Expand Down Expand Up @@ -1848,10 +1851,17 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma
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)
Expand Down

0 comments on commit 652bf64

Please sign in to comment.