Skip to content

Make the constant metadata a Scalar#6751

Merged
gatesn merged 1 commit intodevelopfrom
ct/constant-scalar
Mar 2, 2026
Merged

Make the constant metadata a Scalar#6751
gatesn merged 1 commit intodevelopfrom
ct/constant-scalar

Conversation

@connortsui20
Copy link
Contributor

Summary

Changes the Metadata for ConstantArray to be a Scalar. Note that this is not a breaking change since the Metadata is purely an in-memory construct.

Testing

All of the existing tests should be sufficient.

Comment on lines 118 to 135
@@ -145,7 +134,17 @@ impl VTable for ConstantVTable {
let scalar_value = ScalarValue::from_proto_bytes(bytes, dtype)?;
let scalar = Scalar::try_new(dtype.clone(), scalar_value)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be on the GPU and there still very slow to copy

Copy link
Contributor Author

@connortsui20 connortsui20 Mar 2, 2026

Choose a reason for hiding this comment

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

I believe we talked about this before, I think we don't care right now? cc @gatesn @onursatici

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we care right now.

We need to figure out the right flat layout logic for GPU + host behavior. Small constant arrays should store their values in the metadata. Larger constant arrays and list arrays should store their values in an array.

This would be using ScalarValue::Array.

The thing we need to figure out is a way for ConstantArray to store this array value in a child array, rather than serialized into its own metadata. If we do this, then list-constants and large constant scalars will remain on the GPU all the way through deserialization.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that ScalarValue::Array does not exist yet (we want to have it), and this is blocking that

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@gatesn gatesn merged commit 909deb5 into develop Mar 2, 2026
56 checks passed
@gatesn gatesn deleted the ct/constant-scalar branch March 2, 2026 20:43
gatesn pushed a commit that referenced this pull request Mar 2, 2026
## Summary

Similar in nature to #6751. We
want to be able to work with in-memory metadata more easily, and we also
want to make sure all scalar deserialization happens is `deserialize`
instead of `build`.

## API Changes

Renames the constructors to be in line with the rest of the encodings.

## Testing

Existing tests should be sufficient.

---------

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
connortsui20 added a commit that referenced this pull request Mar 2, 2026
## Summary

Similar in nature to #6751 and
#6759. We want to be able to
work with in-memory metadata more easily, and we also want to make sure
all scalar deserialization happens is `deserialize` instead of `build`.
This is the last of the changes to metadata needed to unblock
array-valued scalars in metadata.

## Testing

Existing tests should be sufficient.

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@connortsui20 connortsui20 mentioned this pull request Mar 2, 2026
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.

3 participants