You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/07/04 08:30:22 UTC

[GitHub] [arrow-rs] viirya opened a new pull request, #2000: Add Decimal256Builder and Decimal256Array

viirya opened a new pull request, #2000:
URL: https://github.com/apache/arrow-rs/pull/2000

   # Which issue does this PR close?
   
   <!---
   We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
   -->
   
   Closes #1999.
   
   # Rationale for this change
    
    <!---
    Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
    Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
   -->
   
   # What changes are included in this PR?
   
   <!---
   There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
   -->
   
   # Are there any user-facing changes?
   
   
   <!---
   If there are user-facing changes then we may require documentation to be updated before approving the PR.
   -->
   
   <!---
   If there are any breaking changes to public APIs, please add the `breaking change` label.
   -->
   


-- 
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


[GitHub] [arrow-rs] tustvold commented on pull request #2000: Add Decimal256Builder and Decimal256Array

Posted by GitBox <gi...@apache.org>.
tustvold commented on PR #2000:
URL: https://github.com/apache/arrow-rs/pull/2000#issuecomment-1177616401

   Should I be reviewing this or #2006, they seem to implement the same functionality?


-- 
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


[GitHub] [arrow-rs] alamb merged pull request #2000: Add Decimal256Builder and Decimal256Array

Posted by GitBox <gi...@apache.org>.
alamb merged PR #2000:
URL: https://github.com/apache/arrow-rs/pull/2000


-- 
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


[GitHub] [arrow-rs] alamb commented on pull request #2000: Add `Decimal256Builder` and `Decimal256Array`; Decimal arrays now implement `BasicDecimalArray` trait

Posted by GitBox <gi...@apache.org>.
alamb commented on PR #2000:
URL: https://github.com/apache/arrow-rs/pull/2000#issuecomment-1179214132

   I see now that @liukun4515  asked to be pinged on this PR for review, which @viirya did but then I merged it prior to @liukun4515 having a chance to review. 
   
   @liukun4515  I will make the 18.0.0 release candidate but if you find issues with the API or some other reason we should not release 18.0.0 with this change in it, please let me know and we can make changes prior to release.
   
   


-- 
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


[GitHub] [arrow-rs] liukun4515 commented on pull request #2000: Add `Decimal256Builder` and `Decimal256Array`; Decimal arrays now implement `BasicDecimalArray` trait

Posted by GitBox <gi...@apache.org>.
liukun4515 commented on PR #2000:
URL: https://github.com/apache/arrow-rs/pull/2000#issuecomment-1180234222

   > I see now that @liukun4515 asked to be pinged on this PR for review, which @viirya did but then I merged it prior to @liukun4515 having a chance to review.
   > 
   > @liukun4515 I will make the 18.0.0 release candidate but if you find issues with the API or some other reason we should not release 18.0.0 with this change in it, please let me know and we can make changes prior to release.
   
   
   
   > I see now that @liukun4515 asked to be pinged on this PR for review, which @viirya did but then I merged it prior to @liukun4515 having a chance to review.
   > 
   > @liukun4515 I will make the 18.0.0 release candidate but if you find issues with the API or some other reason we should not release 18.0.0 with this change in it, please let me know and we can make changes prior to release.
   
   Sorry for the later reply, I am busy with other task.
   This pr looks great for me.
   And from https://github.com/apache/arrow-rs/pull/2000#discussion_r916386516, it's better to refactor the builder and generate the array data from `fixed list builder` to `fixed binary builder`.


-- 
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


[GitHub] [arrow-rs] HaoYang670 commented on a diff in pull request #2000: Add Decimal256Builder and Decimal256Array

Posted by GitBox <gi...@apache.org>.
HaoYang670 commented on code in PR #2000:
URL: https://github.com/apache/arrow-rs/pull/2000#discussion_r916800674


##########
arrow/src/array/builder/decimal_builder.rs:
##########
@@ -42,8 +44,18 @@ pub struct DecimalBuilder {
     value_validation: bool,
 }
 
+/// Array Builder for [`Decimal256Array`]
+///
+/// See [`Decimal256Array`] for example.
+#[derive(Debug)]
+pub struct Decimal256Builder {
+    builder: FixedSizeListBuilder<UInt8Builder>,

Review Comment:
   Create https://github.com/apache/arrow-rs/issues/2026 to track 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


[GitHub] [arrow-rs] codecov-commenter commented on pull request #2000: Add Decimal256Builder and Decimal256Array

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #2000:
URL: https://github.com/apache/arrow-rs/pull/2000#issuecomment-1175427049

   # [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/2000?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#2000](https://codecov.io/gh/apache/arrow-rs/pull/2000?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8b2e387) into [master](https://codecov.io/gh/apache/arrow-rs/commit/230915703471e9bf1403af657a447fcab971d530?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2309157) will **decrease** coverage by `0.02%`.
   > The diff coverage is `72.64%`.
   
   > :exclamation: Current head 8b2e387 differs from pull request most recent head 1dd13ea. Consider uploading reports for the commit 1dd13ea to get more accurate results
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #2000      +/-   ##
   ==========================================
   - Coverage   83.58%   83.56%   -0.03%     
   ==========================================
     Files         222      222              
     Lines       57529    57621      +92     
   ==========================================
   + Hits        48088    48149      +61     
   - Misses       9441     9472      +31     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/2000?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [arrow/src/array/equal\_json.rs](https://codecov.io/gh/apache/arrow-rs/pull/2000/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L2VxdWFsX2pzb24ucnM=) | `87.89% <0.00%> (-1.81%)` | :arrow_down: |
   | [arrow/src/array/iterator.rs](https://codecov.io/gh/apache/arrow-rs/pull/2000/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L2l0ZXJhdG9yLnJz) | `96.11% <ø> (ø)` | |
   | [arrow/src/array/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/2000/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L21vZC5ycw==) | `100.00% <ø> (ø)` | |
   | [arrow/src/array/ord.rs](https://codecov.io/gh/apache/arrow-rs/pull/2000/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L29yZC5ycw==) | `69.17% <ø> (ø)` | |
   | [arrow/src/compute/kernels/cast.rs](https://codecov.io/gh/apache/arrow-rs/pull/2000/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2NvbXB1dGUva2VybmVscy9jYXN0LnJz) | `95.79% <ø> (ø)` | |
   | [arrow/src/compute/kernels/sort.rs](https://codecov.io/gh/apache/arrow-rs/pull/2000/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2NvbXB1dGUva2VybmVscy9zb3J0LnJz) | `95.67% <ø> (ø)` | |
   | [arrow/src/compute/kernels/take.rs](https://codecov.io/gh/apache/arrow-rs/pull/2000/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2NvbXB1dGUva2VybmVscy90YWtlLnJz) | `95.27% <ø> (ø)` | |
   | [arrow/src/csv/reader.rs](https://codecov.io/gh/apache/arrow-rs/pull/2000/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2Nzdi9yZWFkZXIucnM=) | `89.89% <ø> (ø)` | |
   | [arrow/src/util/decimal.rs](https://codecov.io/gh/apache/arrow-rs/pull/2000/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL3V0aWwvZGVjaW1hbC5ycw==) | `91.50% <ø> (ø)` | |
   | [arrow/src/util/display.rs](https://codecov.io/gh/apache/arrow-rs/pull/2000/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL3V0aWwvZGlzcGxheS5ycw==) | `73.17% <ø> (ø)` | |
   | ... and [5 more](https://codecov.io/gh/apache/arrow-rs/pull/2000/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/2000?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/2000?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [2309157...1dd13ea](https://codecov.io/gh/apache/arrow-rs/pull/2000?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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


[GitHub] [arrow-rs] viirya commented on pull request #2000: Add Decimal256Builder and Decimal256Array

Posted by GitBox <gi...@apache.org>.
viirya commented on PR #2000:
URL: https://github.com/apache/arrow-rs/pull/2000#issuecomment-1175790339

   cc @alamb @tustvold @liukun4515 


-- 
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


[GitHub] [arrow-rs] liukun4515 commented on pull request #2000: Add Decimal256Builder and Decimal256Array

Posted by GitBox <gi...@apache.org>.
liukun4515 commented on PR #2000:
URL: https://github.com/apache/arrow-rs/pull/2000#issuecomment-1174779034

   If it is ready, please at me. @viirya 


-- 
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


[GitHub] [arrow-rs] alamb commented on a diff in pull request #2000: Add Decimal256Builder and Decimal256Array

Posted by GitBox <gi...@apache.org>.
alamb commented on code in PR #2000:
URL: https://github.com/apache/arrow-rs/pull/2000#discussion_r915181100


##########
arrow/src/array/builder/decimal_builder.rs:
##########
@@ -154,10 +166,59 @@ impl ArrayBuilder for DecimalBuilder {
     }
 }
 
+impl Decimal256Builder {
+    /// Creates a new `Decimal256Builder`, `capacity` is the number of bytes in the values
+    /// array
+    pub fn new(capacity: usize, precision: usize, scale: usize) -> Self {
+        let values_builder = UInt8Builder::new(capacity);
+        let byte_width = 32;
+        Self {
+            builder: FixedSizeListBuilder::new(values_builder, byte_width),
+            precision,
+            scale,
+        }
+    }
+
+    /// Appends a byte slice into the builder.
+    ///
+    /// Automatically calls the `append` method to delimit the slice appended in as a
+    /// distinct array element.
+    #[inline]
+    pub fn append_value(&mut self, value: &Decimal256) -> Result<()> {
+        let value_as_bytes = value.raw_value();
+
+        if self.builder.value_length() != value_as_bytes.len() as i32 {

Review Comment:
   I wonder if we also need to check the `precision` and `scale` of `value` (and either if they don't match the builder or do conversion if required) 🤔 



##########
arrow/src/array/builder/decimal_builder.rs:
##########
@@ -197,4 +258,42 @@ mod tests {
         assert_eq!(32, decimal_array.value_offset(2));
         assert_eq!(16, decimal_array.value_length());
     }
+
+    #[test]
+    fn test_decimal256_builder() {
+        let mut builder = Decimal256Builder::new(30, 40, 6);
+
+        let mut bytes = vec![0; 32];
+        bytes[0..16].clone_from_slice(&8_887_000_000_i128.to_le_bytes());
+        let value = Decimal256::try_new_from_bytes(40, 6, bytes.as_slice()).unwrap();
+        builder.append_value(&value).unwrap();
+
+        builder.append_null().unwrap();
+
+        bytes = vec![255; 32];
+        let value = Decimal256::try_new_from_bytes(40, 6, bytes.as_slice()).unwrap();
+        builder.append_value(&value).unwrap();
+
+        bytes = vec![0; 32];
+        bytes[0..16].clone_from_slice(&0_i128.to_le_bytes());
+        bytes[15] = 128;
+        let value = Decimal256::try_new_from_bytes(40, 6, bytes.as_slice()).unwrap();

Review Comment:
   I think we should add a test for appending a `value` that has a precision/scale other than `40. 6`, as I mentioned above



-- 
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


[GitHub] [arrow-rs] viirya commented on pull request #2000: Add Decimal256Builder and Decimal256Array

Posted by GitBox <gi...@apache.org>.
viirya commented on PR #2000:
URL: https://github.com/apache/arrow-rs/pull/2000#issuecomment-1173526854

   Marked as draft now and I will add some tests and dedup some codes.


-- 
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


[GitHub] [arrow-rs] alamb commented on a diff in pull request #2000: Add Decimal256Builder and Decimal256Array

Posted by GitBox <gi...@apache.org>.
alamb commented on code in PR #2000:
URL: https://github.com/apache/arrow-rs/pull/2000#discussion_r915181100


##########
arrow/src/array/builder/decimal_builder.rs:
##########
@@ -154,10 +166,59 @@ impl ArrayBuilder for DecimalBuilder {
     }
 }
 
+impl Decimal256Builder {
+    /// Creates a new `Decimal256Builder`, `capacity` is the number of bytes in the values
+    /// array
+    pub fn new(capacity: usize, precision: usize, scale: usize) -> Self {
+        let values_builder = UInt8Builder::new(capacity);
+        let byte_width = 32;
+        Self {
+            builder: FixedSizeListBuilder::new(values_builder, byte_width),
+            precision,
+            scale,
+        }
+    }
+
+    /// Appends a byte slice into the builder.
+    ///
+    /// Automatically calls the `append` method to delimit the slice appended in as a
+    /// distinct array element.
+    #[inline]
+    pub fn append_value(&mut self, value: &Decimal256) -> Result<()> {
+        let value_as_bytes = value.raw_value();
+
+        if self.builder.value_length() != value_as_bytes.len() as i32 {

Review Comment:
   I wonder if we also need to check the `precision` and `scale` of `value` (and either error if they don't match the builder or do conversion if required) 🤔 



-- 
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


[GitHub] [arrow-rs] liukun4515 commented on a diff in pull request #2000: Add `Decimal256Builder` and `Decimal256Array`; Decimal arrays now implement `BasicDecimalArray` trait

Posted by GitBox <gi...@apache.org>.
liukun4515 commented on code in PR #2000:
URL: https://github.com/apache/arrow-rs/pull/2000#discussion_r917719589


##########
arrow/src/array/builder/decimal_builder.rs:
##########
@@ -42,8 +44,18 @@ pub struct DecimalBuilder {
     value_validation: bool,
 }
 
+/// Array Builder for [`Decimal256Array`]
+///
+/// See [`Decimal256Array`] for example.
+#[derive(Debug)]
+pub struct Decimal256Builder {
+    builder: FixedSizeListBuilder<UInt8Builder>,

Review Comment:
   I also confused about that we use the the `FixedSizeListBuilder` to store decimal data in the decimal builder.



-- 
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


[GitHub] [arrow-rs] viirya commented on pull request #2000: Add Decimal256Builder and Decimal256Array

Posted by GitBox <gi...@apache.org>.
viirya commented on PR #2000:
URL: https://github.com/apache/arrow-rs/pull/2000#issuecomment-1177709419

   @tustvold I'd prefer this. #2006 is just for showing the alternative concept. But there is some issues. Please review this, 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


[GitHub] [arrow-rs] viirya commented on a diff in pull request #2000: Add Decimal256Builder and Decimal256Array

Posted by GitBox <gi...@apache.org>.
viirya commented on code in PR #2000:
URL: https://github.com/apache/arrow-rs/pull/2000#discussion_r915198508


##########
arrow/src/array/builder/decimal_builder.rs:
##########
@@ -154,10 +166,59 @@ impl ArrayBuilder for DecimalBuilder {
     }
 }
 
+impl Decimal256Builder {
+    /// Creates a new `Decimal256Builder`, `capacity` is the number of bytes in the values
+    /// array
+    pub fn new(capacity: usize, precision: usize, scale: usize) -> Self {
+        let values_builder = UInt8Builder::new(capacity);
+        let byte_width = 32;
+        Self {
+            builder: FixedSizeListBuilder::new(values_builder, byte_width),
+            precision,
+            scale,
+        }
+    }
+
+    /// Appends a byte slice into the builder.
+    ///
+    /// Automatically calls the `append` method to delimit the slice appended in as a
+    /// distinct array element.
+    #[inline]
+    pub fn append_value(&mut self, value: &Decimal256) -> Result<()> {
+        let value_as_bytes = value.raw_value();
+
+        if self.builder.value_length() != value_as_bytes.len() as i32 {

Review Comment:
   Good point. I think we should. It is a cheap check too so I think okay.



-- 
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


[GitHub] [arrow-rs] HaoYang670 commented on a diff in pull request #2000: Add Decimal256Builder and Decimal256Array

Posted by GitBox <gi...@apache.org>.
HaoYang670 commented on code in PR #2000:
URL: https://github.com/apache/arrow-rs/pull/2000#discussion_r916386516


##########
arrow/src/array/builder/decimal_builder.rs:
##########
@@ -42,8 +44,18 @@ pub struct DecimalBuilder {
     value_validation: bool,
 }
 
+/// Array Builder for [`Decimal256Array`]
+///
+/// See [`Decimal256Array`] for example.
+#[derive(Debug)]
+pub struct Decimal256Builder {
+    builder: FixedSizeListBuilder<UInt8Builder>,

Review Comment:
   It may be more straightforward to build decimal array based on `FixedSizeBinaryBuilder`. I will file a followup issue.



-- 
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


[GitHub] [arrow-rs] alamb commented on pull request #2000: Add Decimal256Builder and Decimal256Array

Posted by GitBox <gi...@apache.org>.
alamb commented on PR #2000:
URL: https://github.com/apache/arrow-rs/pull/2000#issuecomment-1179144097

   @tustvold  is this one good to go (for arrow 18?)


-- 
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