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 2020/12/26 06:14:46 UTC

[GitHub] [arrow] jorgecarleitao opened a new pull request #9017: ARROW-11038: [Rust] Removed unused trait and Result.

jorgecarleitao opened a new pull request #9017:
URL: https://github.com/apache/arrow/pull/9017


   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] codecov-io edited a comment on pull request #9017: ARROW-11038: [Rust] Removed unused trait and Result.

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #9017:
URL: https://github.com/apache/arrow/pull/9017#issuecomment-751325986


   # [Codecov](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=h1) Report
   > Merging [#9017](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=desc) (bde093f) into [master](https://codecov.io/gh/apache/arrow/commit/a4f7c4a2acda874b3d6eb2eb4c986e7c3267c755?el=desc) (a4f7c4a) will **decrease** coverage by `0.00%`.
   > The diff coverage is `92.80%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/arrow/pull/9017/graphs/tree.svg?width=650&height=150&src=pr&token=LpTCFbqVT1)](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #9017      +/-   ##
   ==========================================
   - Coverage   82.87%   82.87%   -0.01%     
   ==========================================
     Files         201      201              
     Lines       49739    49733       -6     
   ==========================================
   - Hits        41220    41215       -5     
   + Misses       8519     8518       -1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [rust/arrow/src/array/builder.rs](https://codecov.io/gh/apache/arrow/pull/9017/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYnVpbGRlci5ycw==) | `84.05% <90.42%> (-0.07%)` | :arrow_down: |
   | [rust/arrow/src/compute/kernels/comparison.rs](https://codecov.io/gh/apache/arrow/pull/9017/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvY29tcHV0ZS9rZXJuZWxzL2NvbXBhcmlzb24ucnM=) | `96.28% <100.00%> (ø)` | |
   | [rust/arrow/src/tensor.rs](https://codecov.io/gh/apache/arrow/pull/9017/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvdGVuc29yLnJz) | `87.50% <100.00%> (ø)` | |
   | [rust/parquet/src/arrow/array\_reader.rs](https://codecov.io/gh/apache/arrow/pull/9017/diff?src=pr&el=tree#diff-cnVzdC9wYXJxdWV0L3NyYy9hcnJvdy9hcnJheV9yZWFkZXIucnM=) | `75.49% <100.00%> (ø)` | |
   | [rust/parquet/src/arrow/record\_reader.rs](https://codecov.io/gh/apache/arrow/pull/9017/diff?src=pr&el=tree#diff-cnVzdC9wYXJxdWV0L3NyYy9hcnJvdy9yZWNvcmRfcmVhZGVyLnJz) | `96.25% <100.00%> (ø)` | |
   | [rust/parquet/src/encodings/encoding.rs](https://codecov.io/gh/apache/arrow/pull/9017/diff?src=pr&el=tree#diff-cnVzdC9wYXJxdWV0L3NyYy9lbmNvZGluZ3MvZW5jb2RpbmcucnM=) | `95.43% <0.00%> (+0.19%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=footer). Last update [a4f7c4a...bde093f](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] alamb commented on a change in pull request #9017: ARROW-11038: [Rust] Removed unused trait and Result.

Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #9017:
URL: https://github.com/apache/arrow/pull/9017#discussion_r549113707



##########
File path: rust/arrow/src/array/builder.rs
##########
@@ -86,160 +80,200 @@ pub(crate) fn builder_to_mutable_buffer<T: ArrowPrimitiveType>(
 /// # }
 /// ```
 #[derive(Debug)]
-pub struct BufferBuilder<T: ArrowPrimitiveType> {
+pub struct BufferBuilder<T: ArrowNativeType> {
     buffer: MutableBuffer,
     len: usize,
     _marker: PhantomData<T>,
 }
 
-/// Trait for simplifying the construction of [`Buffers`](crate::buffer::Buffer).
-///
-/// This trait is used mainly to offer separate implementations for
-/// numeric types and boolean types, while still be able to call methods on buffer builder
-/// with generic primitive type.
-/// Separate implementations of this trait allow to add implementation-details,
-/// e.g. the implementation for boolean types uses bit-packing.
-pub trait BufferBuilderTrait<T: ArrowPrimitiveType> {
+impl<T: ArrowNativeType> BufferBuilder<T> {
     /// Creates a new builder with initial capacity for _at least_ `capacity`
     /// elements of type `T`.
     ///
     /// The capacity can later be manually adjusted with the
-    /// [`reserve()`](BufferBuilderTrait::reserve) method.
+    /// [`reserve()`](BufferBuilder::reserve) method.
     /// Also the
-    /// [`append()`](BufferBuilderTrait::append),
-    /// [`append_slice()`](BufferBuilderTrait::append_slice) and
-    /// [`advance()`](BufferBuilderTrait::advance)
+    /// [`append()`](BufferBuilder::append),
+    /// [`append_slice()`](BufferBuilder::append_slice) and
+    /// [`advance()`](BufferBuilder::advance)
     /// methods automatically increase the capacity if needed.
     ///
     /// # Example:
     ///
     /// ```
-    /// use arrow::array::{UInt8BufferBuilder, BufferBuilderTrait};
+    /// use arrow::array::UInt8BufferBuilder;
     ///
     /// let mut builder = UInt8BufferBuilder::new(10);
     ///
     /// assert!(builder.capacity() >= 10);
     /// ```
-    fn new(capacity: usize) -> Self;
+    #[inline]
+    pub fn new(capacity: usize) -> Self {
+        let buffer = MutableBuffer::new(capacity * mem::size_of::<T>());
+
+        Self {
+            buffer,
+            len: 0,
+            _marker: PhantomData,
+        }
+    }
 
     /// Returns the current number of array elements in the internal buffer.
     ///
     /// # Example:
     ///
     /// ```
-    /// use arrow::array::{UInt8BufferBuilder, BufferBuilderTrait};
+    /// use arrow::array::UInt8BufferBuilder;
     ///
     /// let mut builder = UInt8BufferBuilder::new(10);
     /// builder.append(42);
     ///
     /// assert_eq!(builder.len(), 1);
     /// ```
-    fn len(&self) -> usize;
+    pub fn len(&self) -> usize {
+        self.len
+    }
 
     /// Returns whether the internal buffer is empty.
     ///
     /// # Example:
     ///
     /// ```
-    /// use arrow::array::{UInt8BufferBuilder, BufferBuilderTrait};
+    /// use arrow::array::UInt8BufferBuilder;
     ///
     /// let mut builder = UInt8BufferBuilder::new(10);
     /// builder.append(42);
     ///
     /// assert_eq!(builder.is_empty(), false);
     /// ```
-    fn is_empty(&self) -> bool;
+    pub fn is_empty(&self) -> bool {
+        self.len == 0
+    }
 
     /// Returns the actual capacity (number of elements) of the internal buffer.
     ///
     /// Note: the internal capacity returned by this method might be larger than
     /// what you'd expect after setting the capacity in the `new()` or `reserve()`
     /// functions.
-    fn capacity(&self) -> usize;
+    pub fn capacity(&self) -> usize {
+        let byte_capacity = self.buffer.capacity();
+        byte_capacity / std::mem::size_of::<T>()
+    }
 
     /// Increases the number of elements in the internal buffer by `n`
     /// and resizes the buffer as needed.
     ///
-    /// The values of the newly added elements are undefined.
+    /// The values of the newly added elements are 0.

Review comment:
       maybe `T::default()`

##########
File path: rust/arrow/src/array/mod.rs
##########
@@ -179,18 +179,17 @@ pub use self::array_string::StringOffsetSizeTrait;
 
 pub use self::builder::BooleanBufferBuilder;
 pub use self::builder::BufferBuilder;
-pub use self::builder::BufferBuilderTrait;
 
-pub type Int8BufferBuilder = BufferBuilder<Int8Type>;
-pub type Int16BufferBuilder = BufferBuilder<Int16Type>;
-pub type Int32BufferBuilder = BufferBuilder<Int32Type>;
-pub type Int64BufferBuilder = BufferBuilder<Int64Type>;
-pub type UInt8BufferBuilder = BufferBuilder<UInt8Type>;
-pub type UInt16BufferBuilder = BufferBuilder<UInt16Type>;
-pub type UInt32BufferBuilder = BufferBuilder<UInt32Type>;
-pub type UInt64BufferBuilder = BufferBuilder<UInt64Type>;
-pub type Float32BufferBuilder = BufferBuilder<Float32Type>;
-pub type Float64BufferBuilder = BufferBuilder<Float64Type>;
+pub type Int8BufferBuilder = BufferBuilder<i8>;

Review comment:
       👍  This is a nice simplification




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] github-actions[bot] commented on pull request #9017: ARROW-11038: [Rust] Removed unused trait and Result.

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9017:
URL: https://github.com/apache/arrow/pull/9017#issuecomment-751325921


   https://issues.apache.org/jira/browse/ARROW-11038


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] alamb closed pull request #9017: ARROW-11038: [Rust] Removed unused trait and Result.

Posted by GitBox <gi...@apache.org>.
alamb closed pull request #9017:
URL: https://github.com/apache/arrow/pull/9017


   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] codecov-io commented on pull request #9017: ARROW-11038: [Rust] Removed unused trait and Result.

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #9017:
URL: https://github.com/apache/arrow/pull/9017#issuecomment-751325986


   # [Codecov](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=h1) Report
   > Merging [#9017](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=desc) (d0c6cea) into [master](https://codecov.io/gh/apache/arrow/commit/a4f7c4a2acda874b3d6eb2eb4c986e7c3267c755?el=desc) (a4f7c4a) will **increase** coverage by `0.00%`.
   > The diff coverage is `92.20%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/arrow/pull/9017/graphs/tree.svg?width=650&height=150&src=pr&token=LpTCFbqVT1)](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=tree)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master    #9017   +/-   ##
   =======================================
     Coverage   82.87%   82.87%           
   =======================================
     Files         201      201           
     Lines       49739    49736    -3     
   =======================================
   - Hits        41220    41219    -1     
   + Misses       8519     8517    -2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [rust/arrow/src/array/builder.rs](https://codecov.io/gh/apache/arrow/pull/9017/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYnVpbGRlci5ycw==) | `84.14% <90.32%> (+0.03%)` | :arrow_up: |
   | [rust/arrow/src/tensor.rs](https://codecov.io/gh/apache/arrow/pull/9017/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvdGVuc29yLnJz) | `87.50% <100.00%> (ø)` | |
   | [rust/parquet/src/arrow/array\_reader.rs](https://codecov.io/gh/apache/arrow/pull/9017/diff?src=pr&el=tree#diff-cnVzdC9wYXJxdWV0L3NyYy9hcnJvdy9hcnJheV9yZWFkZXIucnM=) | `75.49% <100.00%> (ø)` | |
   | [rust/parquet/src/arrow/record\_reader.rs](https://codecov.io/gh/apache/arrow/pull/9017/diff?src=pr&el=tree#diff-cnVzdC9wYXJxdWV0L3NyYy9hcnJvdy9yZWNvcmRfcmVhZGVyLnJz) | `96.25% <100.00%> (ø)` | |
   | [rust/parquet/src/encodings/encoding.rs](https://codecov.io/gh/apache/arrow/pull/9017/diff?src=pr&el=tree#diff-cnVzdC9wYXJxdWV0L3NyYy9lbmNvZGluZ3MvZW5jb2RpbmcucnM=) | `95.43% <0.00%> (+0.19%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=footer). Last update [a4f7c4a...1b2c4ad](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] codecov-io edited a comment on pull request #9017: ARROW-11038: [Rust] Removed unused trait and Result.

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #9017:
URL: https://github.com/apache/arrow/pull/9017#issuecomment-751325986


   # [Codecov](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=h1) Report
   > Merging [#9017](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=desc) (99db121) into [master](https://codecov.io/gh/apache/arrow/commit/a4f7c4a2acda874b3d6eb2eb4c986e7c3267c755?el=desc) (a4f7c4a) will **decrease** coverage by `0.00%`.
   > The diff coverage is `92.74%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/arrow/pull/9017/graphs/tree.svg?width=650&height=150&src=pr&token=LpTCFbqVT1)](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #9017      +/-   ##
   ==========================================
   - Coverage   82.87%   82.87%   -0.01%     
   ==========================================
     Files         201      201              
     Lines       49739    49733       -6     
   ==========================================
   - Hits        41220    41214       -6     
     Misses       8519     8519              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [rust/arrow/src/array/builder.rs](https://codecov.io/gh/apache/arrow/pull/9017/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYnVpbGRlci5ycw==) | `84.05% <90.42%> (-0.07%)` | :arrow_down: |
   | [rust/arrow/src/compute/kernels/comparison.rs](https://codecov.io/gh/apache/arrow/pull/9017/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvY29tcHV0ZS9rZXJuZWxzL2NvbXBhcmlzb24ucnM=) | `96.28% <100.00%> (ø)` | |
   | [rust/arrow/src/tensor.rs](https://codecov.io/gh/apache/arrow/pull/9017/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvdGVuc29yLnJz) | `87.50% <100.00%> (ø)` | |
   | [rust/parquet/src/arrow/array\_reader.rs](https://codecov.io/gh/apache/arrow/pull/9017/diff?src=pr&el=tree#diff-cnVzdC9wYXJxdWV0L3NyYy9hcnJvdy9hcnJheV9yZWFkZXIucnM=) | `75.49% <100.00%> (ø)` | |
   | [rust/parquet/src/arrow/record\_reader.rs](https://codecov.io/gh/apache/arrow/pull/9017/diff?src=pr&el=tree#diff-cnVzdC9wYXJxdWV0L3NyYy9hcnJvdy9yZWNvcmRfcmVhZGVyLnJz) | `96.25% <100.00%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=footer). Last update [a4f7c4a...99db121](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] alamb commented on pull request #9017: ARROW-11038: [Rust] Removed unused trait and Result.

Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #9017:
URL: https://github.com/apache/arrow/pull/9017#issuecomment-751468781


   I also merged from master and ran the tests locally to make sure this didn't logically conflict. Everything was good to go!


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] codecov-io edited a comment on pull request #9017: ARROW-11038: [Rust] Removed unused trait and Result.

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #9017:
URL: https://github.com/apache/arrow/pull/9017#issuecomment-751325986


   # [Codecov](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=h1) Report
   > Merging [#9017](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=desc) (1b2c4ad) into [master](https://codecov.io/gh/apache/arrow/commit/a4f7c4a2acda874b3d6eb2eb4c986e7c3267c755?el=desc) (a4f7c4a) will **decrease** coverage by `0.00%`.
   > The diff coverage is `92.74%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/arrow/pull/9017/graphs/tree.svg?width=650&height=150&src=pr&token=LpTCFbqVT1)](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #9017      +/-   ##
   ==========================================
   - Coverage   82.87%   82.87%   -0.01%     
   ==========================================
     Files         201      201              
     Lines       49739    49733       -6     
   ==========================================
   - Hits        41220    41215       -5     
   + Misses       8519     8518       -1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [rust/arrow/src/array/builder.rs](https://codecov.io/gh/apache/arrow/pull/9017/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYnVpbGRlci5ycw==) | `84.05% <90.42%> (-0.07%)` | :arrow_down: |
   | [rust/arrow/src/compute/kernels/comparison.rs](https://codecov.io/gh/apache/arrow/pull/9017/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvY29tcHV0ZS9rZXJuZWxzL2NvbXBhcmlzb24ucnM=) | `96.28% <100.00%> (ø)` | |
   | [rust/arrow/src/tensor.rs](https://codecov.io/gh/apache/arrow/pull/9017/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvdGVuc29yLnJz) | `87.50% <100.00%> (ø)` | |
   | [rust/parquet/src/arrow/array\_reader.rs](https://codecov.io/gh/apache/arrow/pull/9017/diff?src=pr&el=tree#diff-cnVzdC9wYXJxdWV0L3NyYy9hcnJvdy9hcnJheV9yZWFkZXIucnM=) | `75.49% <100.00%> (ø)` | |
   | [rust/parquet/src/arrow/record\_reader.rs](https://codecov.io/gh/apache/arrow/pull/9017/diff?src=pr&el=tree#diff-cnVzdC9wYXJxdWV0L3NyYy9hcnJvdy9yZWNvcmRfcmVhZGVyLnJz) | `96.25% <100.00%> (ø)` | |
   | [rust/parquet/src/encodings/encoding.rs](https://codecov.io/gh/apache/arrow/pull/9017/diff?src=pr&el=tree#diff-cnVzdC9wYXJxdWV0L3NyYy9lbmNvZGluZ3MvZW5jb2RpbmcucnM=) | `95.43% <0.00%> (+0.19%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=footer). Last update [a4f7c4a...99db121](https://codecov.io/gh/apache/arrow/pull/9017?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] jorgecarleitao commented on a change in pull request #9017: ARROW-11038: [Rust] Removed unused trait and Result.

Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on a change in pull request #9017:
URL: https://github.com/apache/arrow/pull/9017#discussion_r548950578



##########
File path: rust/arrow/src/array/builder.rs
##########
@@ -86,20 +80,13 @@ pub(crate) fn builder_to_mutable_buffer<T: ArrowPrimitiveType>(
 /// # }
 /// ```
 #[derive(Debug)]
-pub struct BufferBuilder<T: ArrowPrimitiveType> {
+pub struct BufferBuilder<T: ArrowNativeType> {

Review comment:
       This change was motivated by `T` not really requiring `ArrowPrimitiveType`, only `ArrowNativeType`, and is required for a major simplification on the Buillders, #9019 




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] alamb commented on pull request #9017: ARROW-11038: [Rust] Removed unused trait and Result.

Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #9017:
URL: https://github.com/apache/arrow/pull/9017#issuecomment-751468860


   This is a nice cleanup -- thank you @jorgecarleitao 


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] jorgecarleitao commented on a change in pull request #9017: ARROW-11038: [Rust] Removed unused trait and Result.

Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on a change in pull request #9017:
URL: https://github.com/apache/arrow/pull/9017#discussion_r548950578



##########
File path: rust/arrow/src/array/builder.rs
##########
@@ -86,20 +80,13 @@ pub(crate) fn builder_to_mutable_buffer<T: ArrowPrimitiveType>(
 /// # }
 /// ```
 #[derive(Debug)]
-pub struct BufferBuilder<T: ArrowPrimitiveType> {
+pub struct BufferBuilder<T: ArrowNativeType> {

Review comment:
       This change was motivated by `T` not really requiring `ArrowPrimitiveType`, only `ArrowNativeType`.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org