[GitHub] [superset] john-bodley commented on a diff in pull request #24413: fix(dashboard): native filters do not preserve bigint

["john-bodley (via GitHub)" <gi...@apache.org>]

john-bodley commented on code in PR #24413:
URL: https://github.com/apache/superset/pull/24413#discussion_r1231648020


##########
superset-frontend/src/filters/components/Select/buildQuery.ts:
##########
@@ -49,12 +49,21 @@ const buildQuery: BuildQuery<PluginFilterSelectQueryFormData> = (
           coltypeMap[label] === GenericDataType.NUMERIC &&
           !Number.isNaN(Number(search))
         ) {
-          // for numeric columns we apply a >= where clause
-          extraFilters.push({
-            col: column,
-            op: '>=',
-            val: Number(search),
-          });
+          if (search === `${Number(search)}`) {
+            // for numeric columns we apply a >= where clause
+            extraFilters.push({
+              col: column,
+              op: '>=',
+              val: Number(search),

Review Comment:
   @justinpark do you know what additional value using `Number(...)` provides? [This](https://github.com/apache/superset/blob/23bb1c48a13236b47cdbff65f3de6658204e7168/superset/models/helpers.py#L1144-L1196) is how the backend handles the filtering logic which [casts](https://github.com/apache/superset/blob/23bb1c48a13236b47cdbff65f3de6658204e7168/superset/models/helpers.py#L1179) strings to numbers and thus maybe it's ok for the backend to handle both cases, i.e., we could remove lines 54–60.
   
   We could include a comment of the form,
   
   ```
   // for numeric columns the backend casts the search string to the appropriate type—preventing JavaScript overflow. Additionally we use the fairly crude(and insufficient)  >= operator as a proxy for detecting whether the search string is contained within the column given there is no trivial way of dynamically casting the the column type.



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org