matthewgapp commented on code in PR #5197:
URL: https://github.com/apache/arrow-rs/pull/5197#discussion_r1460183809
##########
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:
Thanks for the context. 5314 which builds on top of the impressive
https://github.com/apache/arrow-rs/pull/3479 looks like a great step forward.
In the meantime, I appreciate that writing to string is the correct thing to
do, but I recommend that we balance the ideal technical solution with how it
will be used in the real world and that user experience.
For practical purposes, I strongly suggest that we provide the user an
option to write decimals as json numbers. We could do this in a non-breaking
way by adding a pub `JsonWriter` that provides a method to set the decimal
decoding as number (the default being string). This would ensure that by
default, the correct thing is performed but the user can opt in for the
practical parsing to number, saving them a ton of headache downstream.
For example, our downstream use case would require this feature; otherwise
we'd have to maintain a fork of the crate that lets us serialize decimals as
number; otherwise we'd have to layer in additional parsing on each field to see
if string fields could actually be parsed to numbers (which would be commonly
the case since we're dealing with small decimal-type numbers at the source).
--
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]
