fix(query): handle Infinity in query results #16450
Conversation
| df[col] = pd.to_numeric( | ||
| df[col] | ||
| .replace(INFINITY_LITERALS, np.inf) | ||
| .replace(NEGATIVE_INFINITY_LITERALS, -np.inf), |
There was a problem hiding this comment.
Some DBAPI (e.g. Druid) returns Infinity as quoted strings "Infinity", we must manually replace them into Numpy infs.
There was a problem hiding this comment.
This logic should probably be moved into the Druid db_engine_spec to avoid running this logic on unaffected db engines. Something like
BaseEngineSpec.replace_literal_values(val: str) -> Any:
return valwhich then is implemented in the Druid spec with those specific literals.
| if self.enforce_numerical_metrics: | ||
| self.df_metrics_to_num(df, query_object) | ||
|
|
||
| df.replace([np.inf, -np.inf], np.nan, inplace=True) |
There was a problem hiding this comment.
Infs will be forced into nulls by JSON encoder anyway.
Codecov Report
@@ Coverage Diff @@
## master #16450 +/- ##
==========================================
- Coverage 76.63% 76.41% -0.22%
==========================================
Files 1002 1002
Lines 53635 53641 +6
Branches 6851 6851
==========================================
- Hits 41101 40992 -109
- Misses 12295 12410 +115
Partials 239 239
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
villebro
left a comment
villebro
left a comment
There was a problem hiding this comment.
Should we start considering moving to a binary format like msgpack for moving data over the wire to both decrease the payload size and add better support for these types of values?
| df[col] = pd.to_numeric( | ||
| df[col] | ||
| .replace(INFINITY_LITERALS, np.inf) | ||
| .replace(NEGATIVE_INFINITY_LITERALS, -np.inf), |
There was a problem hiding this comment.
This logic should probably be moved into the Druid db_engine_spec to avoid running this logic on unaffected db engines. Something like
BaseEngineSpec.replace_literal_values(val: str) -> Any:
return valwhich then is implemented in the Druid spec with those specific literals.
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue |
2 participants
SUMMARY
More explicitly handle
Infinityvalues in query results and improve type inference related to infinity values.Previously we treat
InfinityasN/Asince JSON cannot handle infinity values. This PR explicitly retains infinity values in Pandas dataframes and adds two fields in data response (infsand-infs) to return to the client side whichnulls in the data records are actually infinities. The client side can then convert them back into JavaScriptInfinity/-Infinity(not the scope of this PR).More unit tests to come.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION