You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "AdamGS (via GitHub)" <gi...@apache.org> on 2026/04/14 12:07:49 UTC

[PR] Fix clippy warning in fixed_size_binary_array.rs [arrow-rs]

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

   # Which issue does this PR close?
   
   - Closes #NNN.
   
   # Rationale for this change
   
   This is a future lint, so I took to the opportunity to make this it clear that the length validation only happens in one case.
   
   # What changes are included in this PR?
   
   Minor changes to validation in `FixedSizeBinaryArray::try_new`.
   
   # Are these changes tested?
   
   Added some assertions to existing tests.
   
   # Are there any user-facing changes?
   
   None
   


-- 
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] Fix clippy warning in fixed_size_binary_array.rs [arrow-rs]

Posted by "AdamGS (via GitHub)" <gi...@apache.org>.
AdamGS commented on code in PR #9712:
URL: https://github.com/apache/arrow-rs/pull/9712#discussion_r3085854414


##########
arrow-array/src/array/fixed_size_binary_array.rs:
##########
@@ -94,27 +94,31 @@ impl FixedSizeBinaryArray {
             ArrowError::InvalidArgumentError(format!("Size cannot be negative, got {size}"))
         })?;
 
-        let len = if s == 0 {
-            if !values.is_empty() {
-                return Err(ArrowError::InvalidArgumentError(
-                    "Buffer cannot have non-zero length if the item size is zero".to_owned(),
-                ));
+        let len = match values.len().checked_div(s) {

Review Comment:
   obviously this is a really nitpicky thing, glad to close this PR and just punt on it to whenever it becomes an actual 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


Re: [PR] Fix clippy warning in fixed_size_binary_array.rs [arrow-rs]

Posted by "AdamGS (via GitHub)" <gi...@apache.org>.
AdamGS commented on code in PR #9712:
URL: https://github.com/apache/arrow-rs/pull/9712#discussion_r3085827178


##########
arrow-array/src/array/fixed_size_binary_array.rs:
##########
@@ -94,27 +94,31 @@ impl FixedSizeBinaryArray {
             ArrowError::InvalidArgumentError(format!("Size cannot be negative, got {size}"))
         })?;
 
-        let len = if s == 0 {
-            if !values.is_empty() {
-                return Err(ArrowError::InvalidArgumentError(
-                    "Buffer cannot have non-zero length if the item size is zero".to_owned(),
-                ));
+        let len = match values.len().checked_div(s) {

Review Comment:
   its going to be `clippy::manual_checked_ops` which I think will be part of 1.95, I just ran into it because I was building with nightly.



-- 
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] Fix clippy warning in fixed_size_binary_array.rs [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #9712:
URL: https://github.com/apache/arrow-rs/pull/9712


-- 
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] Fix clippy warning in fixed_size_binary_array.rs [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #9712:
URL: https://github.com/apache/arrow-rs/pull/9712#discussion_r3082359882


##########
arrow-array/src/array/fixed_size_binary_array.rs:
##########
@@ -94,27 +94,31 @@ impl FixedSizeBinaryArray {
             ArrowError::InvalidArgumentError(format!("Size cannot be negative, got {size}"))
         })?;
 
-        let len = if s == 0 {
-            if !values.is_empty() {
-                return Err(ArrowError::InvalidArgumentError(
-                    "Buffer cannot have non-zero length if the item size is zero".to_owned(),
-                ));
+        let len = match values.len().checked_div(s) {

Review Comment:
   what is the clippy lint that is triggered?
   
   To be honest I found this formulation harder to read than the previous one



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