You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "phillipleblanc (via GitHub)" <gi...@apache.org> on 2024/10/21 02:43:11 UTC
[PR] Implement arrow_json encoder for Decimal128 & Decimal256 [arrow-rs]
phillipleblanc opened a new pull request, #6606:
URL: https://github.com/apache/arrow-rs/pull/6606
# Which issue does this PR close?
Closes #6605
# Rationale for this change
Makes the arrow_json crate able to serialize more record batches.
# What changes are included in this PR?
Implements a Decimal128Encoder and Decimal256Encoder and uses that within the `make_encoder_impl` function for the Decimal128 & Decimal256 types.
# Are there any user-facing changes?
More record batches will be able to be serialized to JSON, and this isn't backwards incompatible.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement arrow_json encoder for Decimal128 & Decimal256 [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold merged PR #6606:
URL: https://github.com/apache/arrow-rs/pull/6606
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement arrow_json encoder for Decimal128 & Decimal256 [arrow-rs]
Posted by "phillipleblanc (via GitHub)" <gi...@apache.org>.
phillipleblanc commented on code in PR #6606:
URL: https://github.com/apache/arrow-rs/pull/6606#discussion_r1808287639
##########
arrow-json/src/writer/encoder.rs:
##########
@@ -545,3 +555,37 @@ where
out.push(b'"');
}
}
+
+struct Decimal128Encoder<'a> {
+ array: &'a Decimal128Array,
+}
+
+impl<'a> Decimal128Encoder<'a> {
+ fn new(array: &'a Decimal128Array) -> Self {
+ Self { array }
+ }
+}
+
+impl<'a> Encoder for Decimal128Encoder<'a> {
+ fn encode(&mut self, idx: usize, out: &mut Vec<u8>) {
+ let formatted = self.array.value_as_string(idx);
Review Comment:
I tried this, but it encoded them as strings not numbers, and I don't see any option that is relevant.
```rust
DataType::Decimal128(_, _) | DataType::Decimal256(_, _)=> {
let options = FormatOptions::new();
let formatter = ArrayFormatter::try_new(array, &options)?;
(Box::new(formatter) as _, array.nulls().cloned())
}
```
```console
assertion `left == right` failed
left: [Some(Object {"decimal": Number(12.34)}), Some(Object {"decimal": Number(56.78)}), Some(Object {"decimal": Number(90.12)}), None]
right: [Some(Object {"decimal": String("12.34")}), Some(Object {"decimal": String("56.78")}), Some(Object {"decimal": String("90.12")}), None]
```
I'm not sure why though, it seems to call the same `format_decimal` method that `value_as_string` does?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement arrow_json encoder for Decimal128 & Decimal256 [arrow-rs]
Posted by "phillipleblanc (via GitHub)" <gi...@apache.org>.
phillipleblanc commented on code in PR #6606:
URL: https://github.com/apache/arrow-rs/pull/6606#discussion_r1808263249
##########
arrow-json/src/writer/encoder.rs:
##########
@@ -545,3 +555,37 @@ where
out.push(b'"');
}
}
+
+struct Decimal128Encoder<'a> {
+ array: &'a Decimal128Array,
+}
+
+impl<'a> Decimal128Encoder<'a> {
+ fn new(array: &'a Decimal128Array) -> Self {
+ Self { array }
+ }
+}
+
+impl<'a> Encoder for Decimal128Encoder<'a> {
+ fn encode(&mut self, idx: usize, out: &mut Vec<u8>) {
+ let formatted = self.array.value_as_string(idx);
Review Comment:
Let me try this
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement arrow_json encoder for Decimal128 & Decimal256 [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #6606:
URL: https://github.com/apache/arrow-rs/pull/6606#discussion_r1808418521
##########
arrow-json/src/writer/encoder.rs:
##########
@@ -545,3 +555,37 @@ where
out.push(b'"');
}
}
+
+struct Decimal128Encoder<'a> {
+ array: &'a Decimal128Array,
+}
+
+impl<'a> Decimal128Encoder<'a> {
+ fn new(array: &'a Decimal128Array) -> Self {
+ Self { array }
+ }
+}
+
+impl<'a> Encoder for Decimal128Encoder<'a> {
+ fn encode(&mut self, idx: usize, out: &mut Vec<u8>) {
+ let formatted = self.array.value_as_string(idx);
Review Comment:
I took the liberty of doing this in https://github.com/apache/arrow-rs/pull/6606/commits/c912fa1e498622b4a9d1439893497ff4c5710171
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement arrow_json encoder for Decimal128 & Decimal256 [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #6606:
URL: https://github.com/apache/arrow-rs/pull/6606#discussion_r1808298785
##########
arrow-json/src/writer/encoder.rs:
##########
@@ -545,3 +555,37 @@ where
out.push(b'"');
}
}
+
+struct Decimal128Encoder<'a> {
+ array: &'a Decimal128Array,
+}
+
+impl<'a> Decimal128Encoder<'a> {
+ fn new(array: &'a Decimal128Array) -> Self {
+ Self { array }
+ }
+}
+
+impl<'a> Encoder for Decimal128Encoder<'a> {
+ fn encode(&mut self, idx: usize, out: &mut Vec<u8>) {
+ let formatted = self.array.value_as_string(idx);
Review Comment:
Yes you will need to keep the custom encoder as a newtype as unlike the implementation for ArrayFormatter above, you don't want to write the quote characters
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement arrow_json encoder for Decimal128 & Decimal256 [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #6606:
URL: https://github.com/apache/arrow-rs/pull/6606#discussion_r1808260914
##########
arrow-json/src/writer/encoder.rs:
##########
@@ -545,3 +555,37 @@ where
out.push(b'"');
}
}
+
+struct Decimal128Encoder<'a> {
+ array: &'a Decimal128Array,
+}
+
+impl<'a> Decimal128Encoder<'a> {
+ fn new(array: &'a Decimal128Array) -> Self {
+ Self { array }
+ }
+}
+
+impl<'a> Encoder for Decimal128Encoder<'a> {
+ fn encode(&mut self, idx: usize, out: &mut Vec<u8>) {
+ let formatted = self.array.value_as_string(idx);
Review Comment:
FWIW you can avoid this allocation by using https://docs.rs/arrow-cast/latest/arrow_cast/display/struct.ArrayFormatter.html
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement arrow_json encoder for Decimal128 & Decimal256 [arrow-rs]
Posted by "phillipleblanc (via GitHub)" <gi...@apache.org>.
phillipleblanc commented on code in PR #6606:
URL: https://github.com/apache/arrow-rs/pull/6606#discussion_r1808421582
##########
arrow-json/src/writer/encoder.rs:
##########
@@ -545,3 +555,37 @@ where
out.push(b'"');
}
}
+
+struct Decimal128Encoder<'a> {
+ array: &'a Decimal128Array,
+}
+
+impl<'a> Decimal128Encoder<'a> {
+ fn new(array: &'a Decimal128Array) -> Self {
+ Self { array }
+ }
+}
+
+impl<'a> Encoder for Decimal128Encoder<'a> {
+ fn encode(&mut self, idx: usize, out: &mut Vec<u8>) {
+ let formatted = self.array.value_as_string(idx);
Review Comment:
Thanks!
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org