Skip to content

Feature: ListVector#5190

Merged
connortsui20 merged 4 commits intodevelopfrom
ct/vv-list
Nov 5, 2025
Merged

Feature: ListVector#5190
connortsui20 merged 4 commits intodevelopfrom
ct/vv-list

Conversation

@connortsui20
Copy link
Copy Markdown
Contributor

@connortsui20 connortsui20 commented Nov 4, 2025

Tracking Issue: #5028

This PR adds the ListVector type and the basic operations EXCEPT split_off and unsplit (as we probably should figure that out with binaryview).

More docs can come later once more methods are added (we probably also want a get_list kind of method).

@connortsui20 connortsui20 requested a review from gatesn November 4, 2025 20:27
@connortsui20 connortsui20 added the changelog/feature A new feature label Nov 4, 2025
@connortsui20 connortsui20 mentioned this pull request Nov 4, 2025
39 tasks
) -> VortexResult<()> {
let len = offsets.len();

match_each_integer_pvector!(&offsets, |offsets_vector| {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can actually just match_each_native_ptype!(offsets.ptype(), |T| offsets.into::<PVector<T>>()) I think

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we keep the macros I wrote for completeness?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't need them, then no

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I banged my head for a while and basically if we want to do it this way then we need to have a downcast that doesnt take ownership. I think it might be worth adding as_ methods to the primitive and decimal vectors

@codecov
Copy link
Copy Markdown

codecov bot commented Nov 4, 2025

Codecov Report

❌ Patch coverage is 68.67647% with 213 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.85%. Comparing base (28f5b3d) to head (84b0ad6).
⚠️ Report is 4 commits behind head on develop.

Files with missing lines Patch % Lines
vortex-vector/src/listview/vector_mut.rs 49.18% 93 Missing ⚠️
vortex-vector/src/listview/vector.rs 62.50% 54 Missing ⚠️
vortex-vector/src/vector_mut.rs 0.00% 13 Missing ⚠️
vortex-vector/src/vector.rs 0.00% 12 Missing ⚠️
vortex-vector/src/listview/tests.rs 95.67% 10 Missing ⚠️
vortex-buffer/src/bit/buf_mut.rs 0.00% 9 Missing ⚠️
vortex-mask/src/mask_mut.rs 33.33% 8 Missing ⚠️
vortex-compute/src/mask/mod.rs 30.00% 7 Missing ⚠️
vortex-mask/src/lib.rs 94.33% 3 Missing ⚠️
vortex-compute/src/arrow/list.rs 0.00% 2 Missing ⚠️
... and 1 more

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@connortsui20 connortsui20 marked this pull request as ready for review November 5, 2025 15:28
@connortsui20 connortsui20 merged commit c6e025a into develop Nov 5, 2025
39 checks passed
@connortsui20 connortsui20 deleted the ct/vv-list branch November 5, 2025 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/feature A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants