add try_new_with_length constructor to FixedSizeList
#8624
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
connortsui20
force-pushed
the
fsl-len
branch
2 times, most recently
from
844a573 to
6cd01dc
Compare
|
Would be great to get this moving @alamb |
|
Maybe merging in the latest main would fix CI? |
|
Does arrow-rs need to support Rust version 1.85? |
|
I suggest we don't try and change the MSRV as part of this PR |
alamb
reviewed
Nov 3, 2025
There was a problem hiding this comment.
This looks good to me -- thanks @connortsui20
Let's get the MSRV CI check sorted out and I think it will be good to go
connortsui20
commented
Nov 3, 2025
Comment on lines
230
to
231
| if let Some(null_buffer) = &nulls | ||
| && null_buffer.len() != len |
|
unfortunately the MSRV CI test is still failing: https://github.com/apache/arrow-rs/actions/runs/19050272042/job/54409016478?pr=8624 Here is a nicer output explaining what is going on andrewlamb@Andrews-MacBook-Pro-3:~/Software/arrow-rs/arrow$ cargo msrv verify
[Meta] cargo-msrv 0.18.4
Compatibility Check #1: Rust 1.85.0
[FAIL] Is incompatible
╭────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ Compiling libm v0.2.15 │
│ Compiling autocfg v1.5.0 │
│ Compiling proc-macro2 v1.0.103 │
│ Compiling quote v1.0.41 │
│ Compiling unicode-ident v1.0.22 │
│ Compiling zerocopy v0.8.27 │
│ Checking cfg-if v1.0.4 │
│ Compiling libc v0.2.177 │
│ Compiling version_check v0.9.5 │
│ Compiling getrandom v0.3.4 │
│ Checking bytes v1.10.1 │
│ Checking core-foundation-sys v0.8.7 │
│ Checking hashbrown v0.16.0 │
│ Checking arrow-schema v57.0.0 (/Users/andrewlamb/Software/arrow-rs/arrow-schema) │
│ Checking once_cell v1.21.3 │
│ Checking memchr v2.7.6 │
│ Checking lexical-util v1.0.7 │
│ Compiling serde_core v1.0.228 │
│ Compiling semver v1.0.27 │
│ Checking iana-time-zone v0.1.64 │
│ Checking ryu v1.0.20 │
│ Checking aho-corasick v1.1.4 │
│ Checking regex-syntax v0.8.8 │
│ Compiling ahash v0.8.12 │
│ Checking itoa v1.0.15 │
│ Compiling serde_json v1.0.145 │
│ Compiling num-traits v0.2.19 │
│ Checking lexical-write-integer v1.0.6 │
│ Checking lexical-parse-integer v1.0.6 │
│ Compiling rustc_version v0.4.1 │
│ Checking base64 v0.22.1 │
│ Checking csv-core v0.1.13 │
│ Checking equivalent v1.0.2 │
│ Checking lexical-parse-float v1.0.6 │
│ Checking lexical-write-float v1.0.6 │
│ Checking bitflags v2.10.0 │
│ Checking indexmap v2.12.0 │
│ Checking simdutf8 v0.1.5 │
│ Compiling flatbuffers v25.9.23 │
│ Checking lexical-core v1.0.6 │
│ Checking regex-automata v0.4.13 │
│ Compiling syn v2.0.108 │
│ Checking regex v1.12.2 │
│ Checking csv v1.4.0 │
│ Checking num-integer v0.1.46 │
│ Checking num-complex v0.4.6 │
│ Checking chrono v0.4.42 │
│ Checking atoi v2.0.0 │
│ Compiling zerocopy-derive v0.8.27 │
│ Checking num-bigint v0.4.6 │
│ Checking half v2.7.1 │
│ Checking arrow-buffer v57.0.0 (/Users/andrewlamb/Software/arrow-rs/arrow-buffer) │
│ Checking arrow-data v57.0.0 (/Users/andrewlamb/Software/arrow-rs/arrow-data) │
│ Checking arrow-array v57.0.0 (/Users/andrewlamb/Software/arrow-rs/arrow-array) │
│ error[E0658]: `let` expressions in this position are unstable │
│ --> arrow-array/src/array/fixed_size_list_array.rs:230:12 │
│ | │
│ 230 | if let Some(null_buffer) = &nulls │
│ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ │
│ | │
│ = note: see issue #53667 <https://github.com/rust-lang/rust/issues/53667> for more information │
│ │
│ warning: unknown lint: `unnecessary_transmutes` │
│ --> arrow-array/src/arithmetic.rs:424:17 │
│ | │
│ 424 | #[allow(unnecessary_transmutes)] │
│ | ^^^^^^^^^^^^^^^^^^^^^^ │
│ | │
│ = note: `#[warn(unknown_lints)]` on by default │
│ │
│ warning: unknown lint: `unnecessary_transmutes` │
│ --> arrow-array/src/arithmetic.rs:430:17 │
│ | │
│ 430 | #[allow(unnecessary_transmutes)] │
│ | ^^^^^^^^^^^^^^^^^^^^^^ │
│ │
│ warning: unknown lint: `unnecessary_transmutes` │
│ --> arrow-array/src/arithmetic.rs:441:17 │
│ | │
│ 441 | #[allow(unnecessary_transmutes)] │
│ | ^^^^^^^^^^^^^^^^^^^^^^ │
│ │
│ warning: unknown lint: `unnecessary_transmutes` │
│ --> arrow-array/src/arithmetic.rs:447:17 │
│ | │
│ 447 | #[allow(unnecessary_transmutes)] │
│ | ^^^^^^^^^^^^^^^^^^^^^^ │
│ │
│ For more information about this error, try `rustc --explain E0658`. │
│ warning: `arrow-array` (lib) generated 4 warnings │
│ error: could not compile `arrow-array` (lib) due to 1 previous error; 4 warnings emitted │
│ │
╰────────────────────────────────────────────────────────────────────────────────────────────────────╯
Crate source was found to be incompatible with Rust version '1.85.0' specified as MSRV in the Cargo manifest located at '/Users/andrewlamb/Software/arrow-rs/arrow/Cargo.toml' |
|
@alamb try again? I think the Edit: checked locally that this was indeed the issue. |
alamb
approved these changes
Nov 4, 2025
29 tasks
Also fixes existing tests and adds some more test cases for degenerate `FixedSizeList`s. Signed-off-by: Connor Tsui <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #8623
As a first step for fixing #8623, this PR introduces a
try_new_with_lengthconstructor forFixedSizeListArraythat takes in alenparameter for the specific case of non-nullable, degenerate arrays.To be honest, I think updating the existing constructors makes more sense (breaking change), but this is an easier first step because we can just make
try_new_with_lengththe newtry_newfrom here.I also fixed 2 cases of an existing unit test that were wrong, as well as added an extra test for the degenerate case.
Edit: I realized that the constructors also do not check if the list size correctly divides the input
values.len()intry_new, so added a check for that plus more tests.