betodealmeida commented on a change in pull request #4908: Replace NaN/Infinity
with null
URL:
https://github.com/apache/incubator-superset/pull/4908#discussion_r185292674
##########
File path: superset/views/core.py
##########
@@ -2435,7 +2436,7 @@ def sql_json(self):
resp = json_success(json.dumps(
{'query': query.to_dict()}, default=utils.json_int_dttm_ser,
- allow_nan=False), status=202)
+ ignore_nan=True), status=202)
Review comment:
Digging in a bit more, I found the first commit where this was introduced by
some guy called @mistercrunch:
https://github.com/apache/incubator-superset/commit/38b8db805 :-P
Initially both sync and async responses used `allow_nan=False`. Eventually
it got dropped from the sync response in
https://github.com/apache/incubator-superset/commit/269f55c29 when the
pessimistic encoder was introduced. The pessimistic encoder was not added to
the async response because the commit is trying to fix gigantic HTML error
messages, but IMHO it should've been added as well.
The problem is that the pessimistic encoder doesn't handle `NaN` and
`±Infinity` because they're floats, so the JSON encoder never calls the
`default` method on them.
I think the change above is safe — it will return _some_ result instead of
failing with a `ValueError` exception. But I think the best approach would be
using a more robust serialization to send data to the browser, like BSON.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
