You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "tustvold (via GitHub)" <gi...@apache.org> on 2023/08/21 18:13:17 UTC

[GitHub] [arrow-datafusion] tustvold opened a new issue, #7352: Change ScalarValue::List to store ArrayRef

tustvold opened a new issue, #7352:
URL: https://github.com/apache/arrow-datafusion/issues/7352

   ### 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
   
   _No response_


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] tustvold commented on issue #7352: Change ScalarValue::List to store ArrayRef

Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on issue #7352:
URL: https://github.com/apache/arrow-datafusion/issues/7352#issuecomment-1686909908

   > https://github.com/apache/arrow-datafusion/issues/996 seems to be a similar suggestion
   
   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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] alamb commented on issue #7352: Change ScalarValue::List to store ArrayRef

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #7352:
URL: https://github.com/apache/arrow-datafusion/issues/7352#issuecomment-1686980417

   I think `Change ScalarValue::List to store ArrayRef` is a great idea
   
   An 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 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] Dandandan commented on issue #7352: Change ScalarValue::List to store ArrayRef

Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan commented on issue #7352:
URL: https://github.com/apache/arrow-datafusion/issues/7352#issuecomment-1686810184

   https://github.com/apache/arrow-datafusion/issues/996


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Change ScalarValue::List to store ArrayRef [arrow-datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed issue #7352: Change ScalarValue::List to store ArrayRef
URL: https://github.com/apache/arrow-datafusion/issues/7352


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] jayzhan211 commented on issue #7352: Change ScalarValue::List to store ArrayRef

Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on issue #7352:
URL: https://github.com/apache/arrow-datafusion/issues/7352#issuecomment-1704462811

   Is anyone working on this? If not, I would like to help with this.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] jayzhan211 commented on issue #7352: Change ScalarValue::List to store ArrayRef

Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on issue #7352:
URL: https://github.com/apache/arrow-datafusion/issues/7352#issuecomment-1722224133

   Is it ok to wrap any ArrayRef at the first step?
   
   Not only ListArray(PrimitiveArray(1)) but also PrimitiveArray(1) are wrapped in ScalarValue::List.
   
   In the current implementation.
   ScalarValue::Int64(1) can transform to PrimitiveArray(1) and it is reversible.
   
   In my current design,
   After ScalarValue::Int64(1) transform to PrimitiveArray(1), it will becomes ScalarValue::List and cant transform back to ScalarValue::Int64(1) via `try_from_array()`. 
   
   I think it is fine since we do not need to guarantee the ScalarValue and Array mapping is one-to-one.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] Dandandan commented on issue #7352: Change ScalarValue::List to store ArrayRef

Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan commented on issue #7352:
URL: https://github.com/apache/arrow-datafusion/issues/7352#issuecomment-1686815513

   Also see discussions here:
   
   https://github.com/apache/arrow-datafusion/pull/6837#discussion_r1251736845
   https://github.com/apache/arrow-datafusion/pull/6837#discussion_r1252006197


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] tustvold commented on issue #7352: Change ScalarValue::List to store ArrayRef

Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on issue #7352:
URL: https://github.com/apache/arrow-datafusion/issues/7352#issuecomment-1723016831

   > we do not need to guarantee the ScalarValue and Array mapping is one-to-one
   
   I'm not sure I follow what you're suggesting but this seems like a fairly fundamental property


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Change ScalarValue::List to store ArrayRef [arrow-datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #7352:
URL: https://github.com/apache/arrow-datafusion/issues/7352#issuecomment-1761901340

   FYI I think the PR from @jayzhan211, https://github.com/apache/arrow-datafusion/pull/7629, is ready to merge. Would anyone else on this chain like a chance to reivew it?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] Dandandan commented on issue #7352: Change ScalarValue::List to store ArrayRef

Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan commented on issue #7352:
URL: https://github.com/apache/arrow-datafusion/issues/7352#issuecomment-1705384978

   > Is anyone working on this? If not, I would like to help with this.
   
   I don't think so. I see @tustvold already assigned you on this.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org