Skip to content

feat: support ListView and LargeListView in ScalarValue#21669

Merged
Jefffrey merged 11 commits intoapache:mainfrom
Jefffrey:scalarvalue-listview
Apr 22, 2026
Merged

feat: support ListView and LargeListView in ScalarValue#21669
Jefffrey merged 11 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 thread datafusion/common/src/scalar/mod.rs
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

Comment thread datafusion/common/src/scalar/mod.rs
Comment thread datafusion/common/src/scalar/mod.rs
Comment thread datafusion/common/src/scalar/mod.rs
Comment thread datafusion/common/src/scalar/mod.rs Outdated
}
ScalarValue::ListView(arr) => {
let array = copy_array_data(&arr.to_data());
*Arc::make_mut(arr) = ListViewArray::from(array);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
*Arc::make_mut(arr) = ListViewArray::from(array);
*Arc::make_mut(arr) = ListViewArray::from(array)

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 don't see a value add here; it's valid syntax

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Consistency with the other match arms

Comment thread datafusion/common/src/scalar/mod.rs Outdated
Comment thread datafusion/common/src/utils/mod.rs Outdated
Comment thread datafusion/common/src/scalar/mod.rs Outdated
@alamb
Copy link
Copy Markdown
Contributor

alamb commented Apr 22, 2026

Let's do it!

@Jefffrey Jefffrey added this pull request to the merge queue Apr 22, 2026
@Jefffrey
Copy link
Copy Markdown
Contributor Author

Thanks all

Merged via the queue into apache:main with commit 8a45d02 Apr 22, 2026
37 of 39 checks passed
@Jefffrey Jefffrey deleted the scalarvalue-listview branch April 22, 2026 05:33
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

5 participants