Skip to content

Add ColumnarValue::to_array_variant method (follow-up to #22784)#22934

Open
nathanb9 wants to merge 1 commit into
apache:mainfrom
nathanb9:columnarvalue-to-array-variant
Open

Add ColumnarValue::to_array_variant method (follow-up to #22784)#22934
nathanb9 wants to merge 1 commit into
apache:mainfrom
nathanb9:columnarvalue-to-array-variant

Conversation

@nathanb9

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Follow-up to #22784, addressing a review comment from @alamb suggesting the expand_if_scalar helper would make a good method on ColumnarValue.

Rationale for this change

map.rs had a private expand_if_scalar helper that expands a Scalar to an array while leaving an Array as is, keeping the value wrapped as a ColumnarValue. This is generally useful for functions that thread a ColumnarValue but need to assume a non-scalar input, so it belongs on ColumnarValue rather than in one function file.

What changes are included in this PR?

  • Add ColumnarValue::to_array_variant(&self, num_rows), which returns a ColumnarValue guaranteed to be the Array variant. It reuses the existing to_array, so a Scalar is expanded to num_rows and an Array is returned unchanged (no length validation).
  • Remove the private expand_if_scalar from map.rs and call the new method instead.

Are these changes tested?

Yes. Added a unit test for to_array_variant covering scalar expansion, array pass-through, and that an existing array is not length validated. Existing map unit tests and sqllogictests continue to pass.

Are there any user-facing changes?

Adds one public method to ColumnarValue. No behavior changes.

@github-actions github-actions Bot added logical-expr Logical plan and expressions functions Changes to functions implementation labels Jun 12, 2026
@nathanb9 nathanb9 marked this pull request as ready for review June 12, 2026 21:55

@Jefffrey Jefffrey left a comment

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.

personally i'm not sure about introducing this method, as it seems to goes against the purpose of a ColumnarValue in representing scalars and arrays distinctly 🤔

it would be better for the use case to manually unwrap to just an array if it can only operate on arrays instead of doing this conversion

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

Labels

functions Changes to functions implementation logical-expr Logical plan and expressions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants