panic: failed to create ListView views to the end of the elements array must have size 0

[asubiotto]

Describe the bug

Merging a couple of testing files with 0.56.0 + #5390 causes this panic:

thread 'bisect_opt::tests::test_minimal_repro' panicked at /Users/asubiotto/.cargo/git/checkouts/vortex-ea6777c0fea02158/3a714aa/vortex-error/src/lib.rs:346:33:
Failed to crate `ListViewArray`: views to the end of the elements array (length 3100) must have size 0
Backtrace:
   0: std::backtrace_rs::backtrace::libunwind::trace
             at /rustc/29483883eed69d5fb4db01964cdf2af4d86e9cb2/library/std/src/../../backtrace/src/backtrace/libunwind.rs:117:9
   1: std::backtrace_rs::backtrace::trace_unsynchronized
             at /rustc/29483883eed69d5fb4db01964cdf2af4d86e9cb2/library/std/src/../../backtrace/src/backtrace/mod.rs:66:14
   2: std::backtrace::Backtrace::create
             at /rustc/29483883eed69d5fb4db01964cdf2af4d86e9cb2/library/std/src/backtrace.rs:331:13
   3: vortex_array::arrays::listview::array::validate_offsets_and_sizes
             at /Users/asubiotto/.cargo/git/checkouts/vortex-ea6777c0fea02158/3a714aa/vortex-array/src/arrays/listview/array.rs:426:13
   4: vortex_array::arrays::listview::array::ListViewArray::validate
             at /Users/asubiotto/.cargo/git/checkouts/vortex-ea6777c0fea02158/3a714aa/vortex-array/src/arrays/listview/array.rs:243:17
   5: vortex_array::arrays::listview::array::ListViewArray::new_unchecked
             at /Users/asubiotto/.cargo/git/checkouts/vortex-ea6777c0fea02158/3a714aa/vortex-array/src/arrays/listview/array.rs:179:13
   6: vortex_array::arrays::listview::rebuild::<impl vortex_array::arrays::listview::array::ListViewArray>::rebuild_trim_elements
             at /Users/asubiotto/.cargo/git/checkouts/vortex-ea6777c0fea02158/3a714aa/vortex-array/src/arrays/listview/rebuild.rs:205:13
   7: vortex_array::arrays::listview::rebuild::<impl vortex_array::arrays::listview::array::ListViewArray>::rebuild_make_exact
             at /Users/asubiotto/.cargo/git/checkouts/vortex-ea6777c0fea02158/3a714aa/vortex-array/src/arrays/listview/rebuild.rs:217:18
   8: vortex_array::arrays::listview::rebuild::<impl vortex_array::arrays::listview::array::ListViewArray>::rebuild
             at /Users/asubiotto/.cargo/git/checkouts/vortex-ea6777c0fea02158/3a714aa/vortex-array/src/arrays/listview/rebuild.rs:56:52
   9: vortex_array::arrays::chunked::vtable::canonical::swizzle_list_chunks
             at /Users/asubiotto/.cargo/git/checkouts/vortex-ea6777c0fea02158/3a714aa/vortex-array/src/arrays/chunked/vtable/canonical.rs:128:39
  10: vortex_array::arrays::chunked::vtable::canonical::<impl vortex_array::vtable::canonical::CanonicalVTable<vortex_array::arrays::chunked::vtable::ChunkedVTable> for vortex_array::arrays::chunked::vtable::ChunkedVTable>::canonicalize
             at /Users/asubiotto/.cargo/git/checkouts/vortex-ea6777c0fea02158/3a714aa/vortex-array/src/arrays/chunked/vtable/canonical.rs:35:59
  11: <vortex_array::array::ArrayAdapter<V> as vortex_array::array::Array>::to_canonical
             at /Users/asubiotto/.cargo/git/checkouts/vortex-ea6777c0fea02158/3a714aa/vortex-array/src/array/mod.rs:541:25
  12: <alloc::sync::Arc<dyn vortex_array::array::Array> as vortex_array::array::Array>::to_canonical
             at /Users/asubiotto/.cargo/git/checkouts/vortex-ea6777c0fea02158/3a714aa/vortex-array/src/array/mod.rs:249:23
  13: vortex_btrblocks::BtrBlocksCompressor::compress
             at /Users/asubiotto/.cargo/git/checkouts/vortex-ea6777c0fea02158/3a714aa/vortex-btrblocks/src/lib.rs:358:31
  14: vortex_btrblocks::BtrBlocksCompressor::compress_canonical::{{closure}}

It is reproducible with the following test and three small vortex files I can provide. edit: I'm realizing this relies on merging and projecting missing columns which is specific to our code, so happy to share that code too if helpful.

    #[tokio::test]
    #[ignore]
    async fn test_minimal_repro() -> Result<()> {
        let workspace_root = std::env::var("CARGO_MANIFEST_DIR")
            .map(PathBuf::from)?
            .parent()
            .unwrap()
            .parent()
            .unwrap()
            .to_path_buf();
        let dir_path = workspace_root.join("minimal_repro");
        if !dir_path.exists() {
            return Err(anyhow!(
                "minimal_repro directory does not exist at {dir_path:?}. Run bisect-opt first to create it.",
            ));
        }

        let mut vortex_files: Vec<PathBuf> = WalkDir::new(&dir_path)
            .into_iter()
            .filter_map(|e| e.ok())
            .filter(|e| e.file_type().is_file())
            .map(|e| e.path().to_path_buf())
            .collect();
        if vortex_files.is_empty() {
            return Err(anyhow!("No vortex files found in minimal_repro/"));
        }

        vortex_files.sort();
        info!("Found {} files in minimal_repro/", vortex_files.len());

        let mut dtypes = Vec::new();
        for file_path in &vortex_files {
            let vortex_file = VortexSession::default()
                .open_options()
                .open(file_path.clone())
                .await?;
            dtypes.push(vortex_file.dtype().clone());
        }

        info!("Merging {} schemas", dtypes.len());
        let target_dtype = merge_dtypes(&dtypes)?;
        let target_schema = Arc::new(target_dtype.to_arrow_schema()?);
        info!("Target schema: {:?}", target_schema);

        let batch_stream = futures::stream::iter(vortex_files.clone())
            .then(move |file_path| {
                let target_schema = target_schema.clone();
                async move {
                    let vortex_file = VortexSession::default()
                        .open_options()
                        .open(file_path.clone())
                        .await?;
                    Ok::<_, VortexError>(vortex_file.scan()?.into_stream()?.and_then(move |a| {
                        futures::future::ready(project_and_fill(a, target_schema.clone()))
                    }))
                }
            })
            .try_flatten()
            .boxed();

        info!("Writing combined stream to buffer...");

        VortexSession::default()
            .with_tokio()
            .write_options()
            .write(
                Vec::new(),
                ArrayStreamAdapter::new(target_dtype.clone(), batch_stream),
            )
            .await?;
        Ok(())
    }

To Reproduce

No response

Expected behavior

No response

Additional context

No response

[asubiotto]

cc @connortsui20 / @a10y

[connortsui20]

I think I found the issue, in the new naive_rebuild, the if !self.is_valid check incorrectly appends the previous offset and size 0, where is should be appending the previous offset + previous size and size 0.

The bug (probably?) happens when there is a null list at the end, and instead of getting the actual end of the elements during slicing we lose the last list (because rebuild_trim_elements assumes that the last offset + last size is always the end if the array says it is zero-copyable to List)

My validation function missed this case because it just checks (separately) if the offsets are sorted and if there are no overlaps (and nulls don't create overlaps)

[asubiotto]

I will try out the fix to confirm!

[robert3005]

I assume this has been fixed by #5423. Please reopen if not

[asubiotto]

Yep! Just confirmed the fix. Thanks.