`ListViewVector#copyFrom` Throws `IndexOutOfBoundsException` on Non-Empty Elements
- Lingua principale
- Java
- Stelle
- 94
- Fork
- 152
- Merge medio
- 3g 16h
- PR unite (30g)
- 11
Descrizione
`ListViewVector`'s `#copyFrom` is broken. Here is a test (that otherwise works for `List`):
```java
@Test
public void testListViewCopy() {
final Field childField = new Field("testChild",
new FieldType(false, new ArrowType.Int(32, true), null), null);
final Field listField = new Field("test",
new FieldType(false, ArrowType.ListView.INSTANCE, null), Collections.singletonList(childField));
try (final ListViewVector src = (ListViewVector) listField.createVector(allocator);
final ListViewVector dst = (ListViewVector) listField.createVector(allocator)) {
// init child vector
final int numValues = 10;
final IntVector childSrc = (IntVector) src.getDataVector();
childSrc.setValueCount(numValues);
for (int ii = 0; ii < numValues; ++ii) {
childSrc.set(ii, ii);
}
// init source vector
src.setValueCount(1);
src.startNewValue(0);
src.endValue(0, numValues);
assertEquals(List.of(0, 1, 2, 3, 4, 5, 6, 7, 8, 9), src.getObject(0));
dst.setValueCount(src.getValueCount());
dst.getDataVector().setValueCount(numValues);
dst.copyFrom(0, 0, src);
assertEquals(src.getObject(0), dst.getObject(0));
}
}
```
`ComplexCopier#writeValue` has impl:
```java
case LIST:
case LISTVIEW:
case LARGELIST:
case LARGELISTVIEW:
case FIXED_SIZE_LIST:
if (reader.isSet()) {
writer.startList();
while (reader.next()) {
FieldReader childReader = reader.reader();
FieldWriter childWriter = getListWriterForReader(childReader, writer);
if (childReader.isSet()) {
writeValue(childReader, childWriter);
} else {
childWriter.writeNull();
}
}
writer.endList();
} else {
writer.writeNull();
}
break;
```
Note that the implementation of `UnionListViewReader#next` will never ever return false:
```java
@Override
public boolean next() {
// Here, the currentOffSet keeps track of the current position in the vector inside the list at
// set position.
// And, size keeps track of the elements count in the list, so to make sure we traverse
// the full list, we need to check if the currentOffset is less than the currentOffset + size
if (currentOffset < currentOffset + size) {
data.getReader().setPosition(currentOffset++);
return true;
} else {
return false;
}
}
```
Notice how `currentOffset < currentOffset + size` can only ever be false if `size <= 0` -- but `size` is never modified.
I suspect the desired conditional is:
```
if (currentOffset < size) {
```
Please note that the embedded comment is also nonsense. It's not clear why the approach differs from `UnionListReader`, keeping a consistent approach would have prevented introducing a bug.
This issue exists in main as of [480e1be](https://github.com/apache/arrow-java/commit/480e1be6b7c5fa7bb0aab67c6f700059e4fcd3ea).
Guida per i contributori
Apri la guida per i contributori
Direzione di ricerca
Start with ListViewVector#copyFrom and the supplied test, then inspect UnionListViewReader#next alongside UnionListReader and ComplexCopier#writeValue. Verify iteration terminates for a non-empty list, run the test, and confirm the copied vector returns the same elements without an IndexOutOfBoundsException.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Valutazione
- Stack tecnologico
- java
- Ambito
- data-engineering
- Tipo di issue
- Bug
- Difficoltà
- 3/5
- Tempo stimato
- 1-2 giorni
- Stato di attività
- Ferma
- Chiarezza
- Specificata chiaramente
- Idoneità per principianti
- 52/100