tustvold commented on code in PR #5197:
URL: https://github.com/apache/arrow-rs/pull/5197#discussion_r1459415432
##########
arrow-json/src/writer.rs:
##########
@@ -469,11 +469,67 @@ fn set_column_for_json_rows(
row.insert(col_name.to_string(),
serde_json::Value::Object(obj));
}
}
+ DataType::Decimal128(_precision, _scale) |
DataType::Decimal256(_precision, _scale) => {
+ to_json_float(rows, array, col_name, explicit_nulls)?;
+ }
_ => {
return Err(ArrowError::JsonError(format!(
- "data type {:?} not supported in nested map for json writer",
+ "data type {:?} not supported for json writer",
array.data_type()
- )))
+ )));
+ }
+ }
+ Ok(())
+}
+
+fn to_json_float(
+ rows: &mut [Option<JsonMap<String, Value>>],
+ array: &ArrayRef,
+ col_name: &str,
+ explicit_nulls: bool,
+) -> Result<(), ArrowError> {
+ let options = FormatOptions::default();
+ let formatter = ArrayFormatter::try_new(array.as_ref(), &options)?;
Review Comment:
Writing non-native numeric quantities as string in JSON is a fairly standard
approach to workaround the limited precision many readers have by default,
serde_json included. Protobuf even serializes u64 to strings as part of its
JSON specification. This is not just because the JSON specification only
mandated double precision floating point, but because there are real
performance disadvantages to doing otherwise.
I agree using a string is not a good thing, ideally we wouldn't be relying
on serde_json to serialize, but aside from a fairly major rewrite of this code
to not usw serde_json value, it seems to me like a pragmatic way to get
something, which is better than not supporting anything?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
