jbonofre commented on code in PR #1309:
URL: https://github.com/apache/arrow-java/pull/1309#discussion_r4173064020
##########
vector/src/main/java/org/apache/arrow/vector/dictionary/DictionaryEncoder.java:
##########
@@ -165,7 +165,7 @@ static void retrieveIndexVector(
for (int i = start; i < end; i++) {
if (!indices.isNull(i)) {
int indexAsInt = (int) indices.getValueAsLong(i);
- if (indexAsInt > dictionaryCount) {
+ if (indexAsInt < 0 || indexAsInt >= dictionaryCount) {
Review Comment:
This bound is correct, but one of the three callers doesn't pass the
dictionary size. `StructSubfieldEncore.decode` does:
```java
DictionaryEncoder.retrieveIndexVector(indices, transfer, valueCount, 0,
valueCount);
```
where `valueCount` is the struct row count. So for the struct path:
- a 5-row strict with a 2-entry dictionary and an index of 4 still passes
the check and reads past the dictionary, which is the case #1261 describes
- a 1-row struct with index 1 into a 2-entry dictionary used to decode (`1 >
1` is false) and now throws (`1 >= 1`), although the index is valid
Could you pass the dictionary count there?
```java
DictionaryEncoder.retrieveIndexVector(
indices, transfer, dictionary.getVector().getValueCount(), 0,
valueCount);
```
Alternatively, `retrieveIndexVector` could take the dictionary vector and
call `getValueCount()` itself, so callers can't pass the wrong number.
@lidavidm thoughts?
--
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]
