zeroshade commented on code in PR #34986:
URL: https://github.com/apache/arrow/pull/34986#discussion_r1164259577


##########
go/arrow/array/encoded.go:
##########
@@ -192,6 +193,15 @@ func (r *RunEndEncoded) GetPhysicalLength() int {
        return encoded.GetPhysicalLength(r.data)
 }
 
+func (r *RunEndEncoded) ValueStr(i int) string {

Review Comment:
   I think that's actually the difference here between `GetOneForMarshal` and 
`ValueStr`. `GetOneForMarshal` would return the whole run-mapping etc. While I 
think `ValueStr` should just return the actual "value" for the given "logical" 
index, so that a consumer doesn't need to do that themselves if they are 
actually trying to retrieve the values from the run-end-encoded array. It also 
maintains consistency with the Dictionary encoded arrays, where `ValueStr` 
returns the actual dictionary value, not the dictionary index. Does that make 
sense?
   
   We should also explicitly document this in a doc string on the function.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to