zeroshade commented on code in PR #1156:
URL: https://github.com/apache/arrow-go/pull/1156#discussion_r3833664836
##########
arrow/array/encoded.go:
##########
@@ -502,11 +502,15 @@ func (b *RunEndEncodedBuilder) NullN() int {
}
func (b *RunEndEncodedBuilder) AppendEmptyValue() {
- b.AppendNull()
+ b.finishRun()
Review Comment:
**Blocking:** After this call resets the unmarshalling state, the pending
non-null empty value is indistinguishable from a previously unmarshalled null
to `UnmarshalOne`. Calling `AppendEmptyValue()` followed by
`UnmarshalOne(null)` produces `runEnds=[2]`, `values=[""]`, and logical values
`["", ""]`; the appended null is silently converted to an empty string.
Expected run ends and values are `[1,2]` and `["", null]`. Please track this
state explicitly so a direct empty-value append cannot be mistaken for a null
run, and add this sequence as a regression test.
##########
arrow/array/encoded.go:
##########
@@ -502,11 +502,15 @@ func (b *RunEndEncodedBuilder) NullN() int {
}
func (b *RunEndEncodedBuilder) AppendEmptyValue() {
- b.AppendNull()
+ b.finishRun()
+ b.values.AppendEmptyValue()
Review Comment:
**Major:** This delegation creates an invalid result when the encoded type
is a dictionary. `dictionaryBuilder.AppendEmptyValue()` appends index `0`
without adding a dictionary entry. Consequently, `REE<int16, dictionary<int8,
string>>` contains an index into a zero-length dictionary, and
`GetOneForMarshal(0)` panics with `arrow/array: index out of range`. Please
provide valid dictionary empty-value behavior or explicitly reject this case,
with coverage.
--
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]
