Repository navigation
Change ScalarValue::List to store ArrayRef #7352
Description
Is your feature request related to a problem or challenge?
Currently ScalarValue::List stores a Vec<Option<ScalarValue>>. This is incredibly inefficient as it stores nullability and type information per nested value. It also makes the type very cumbersome to use, as the data cannot easily be fed into the normal array kernels.
Describe the solution you'd like
I would like to change the type of ScalarValue::List to store ArrayRef
Describe alternatives you've considered
No response
Additional context
See #7353
Reacted by yujie.zhang (he/him)
Activity
Dandandan commentedon Aug 21, 2023
#996 seems to be a similar suggestion
Dandandan commentedon Aug 21, 2023
Also see discussions here:
#6837 (comment)
#6837 (comment)
tustvold commentedon Aug 21, 2023
Happy to close this in favor if you prefer, mainly just hoping to bring more attention to this. I know there is a decent amount of ongoing work relating to lists at the moment
alamb commentedon Aug 21, 2023
I think
Change ScalarValue::List to store ArrayRefis a great ideaAn ArrayRef is a pretty high overhead way of representing other scalar types but for lists it is different
I also think doing this would allow us to work on / improve the ergonomics of using ListArray in general and set some patterns (like using take and manipulating the list indexes themselves) as well
So 👍 from me
jayzhan211 commentedon Sep 4, 2023
Is anyone working on this? If not, I would like to help with this.
Dandandan commentedon Sep 4, 2023
I don't think so. I see @tustvold already assigned you on this.
jayzhan211 commentedon Sep 16, 2023
tustvold commentedon Sep 18, 2023
I'm not sure I follow what you're suggesting but this seems like a fairly fundamental property
alamb commentedon Oct 13, 2023
FYI I think the PR from @jayzhan211, #7629, is ready to merge. Would anyone else on this chain like a chance to reivew it?