Skip to content

feat: support ListView and LargeListView in ScalarValue#21669

Open
Jefffrey wants to merge 8 commits intoapache:mainfrom
Jefffrey:scalarvalue-listview
Open

feat: support ListView and LargeListView in ScalarValue#21669
Jefffrey wants to merge 8 commits intoapache:mainfrom
Jefffrey:scalarvalue-listview

Conversation

@Jefffrey
Copy link
Copy Markdown
Contributor

@Jefffrey Jefffrey commented Apr 16, 2026

Which issue does this PR close?

Rationale for this change

More support for listview types in the codebase

What changes are included in this PR?

Added ListView and LargeListView to ScalarValue with all accompanying changes

Support ListView and LargeListView in proto, both for the arrow datatype & the newly introduced scalarvalue variants.

Are these changes tested?

Yes, added tests

Are there any user-facing changes?

No

@github-actions github-actions bot added sql SQL Planner common Related to common crate proto Related to proto crate labels Apr 16, 2026
3,
),
)),
ScalarValue::ListView(Arc::new(ListViewArray::from_iter_primitive::<
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.

Only change in this file is adding cases for listview & largelistview; other changes is indent change. Would recommend reviewing with whitespace off

@Jefffrey Jefffrey marked this pull request as ready for review April 16, 2026 12:39
Comment on lines +3460 to +3471
ScalarValue::ListView(arr) => {
if size == 1 {
return Ok(Arc::clone(arr) as Arc<dyn Array>);
}
Self::list_to_array_of_size(arr.as_ref() as &dyn Array, size)?
}
ScalarValue::LargeListView(arr) => {
if size == 1 {
return Ok(Arc::clone(arr) as Arc<dyn Array>);
}
Self::list_to_array_of_size(arr.as_ref() as &dyn Array, size)?
}
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.

wondering if list arms can be combined into a single arm?

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.

I can't think of a way to do so, since arr in each case is &Arc<ListViewArray> and &Arc<LargeListViewArray> so we can't bind to a common name as they are distinct types

Copy link
Copy Markdown
Contributor

@comphead comphead left a comment

Choose a reason for hiding this comment

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

Thanks @Jefffrey one nit: to check if list/listview branches can be combined into a single branch, if possible

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

common Related to common crate proto Related to proto crate sql SQL Planner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support ListView, LargeListView in ScalarValue

3 participants