apache / apache/arrow-java

`ListViewVector#copyFrom` Throws `IndexOutOfBoundsException` on Non-Empty Elements

Abierto
#471 0 comentarios 0 reacciones 0 asignados Ver en GitHub
help wanted Type: bug
Lenguaje dominante
Java
Estrellas
94
Forks
152
Merge medio
3 d 16 h
PR fusionados (30 d)
11

Descripción

`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).

Guía de contribución

Abrir la guía de contribución

Línea de trabajo

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.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
java
Área
data-engineering
Tipo de issue
Error
Dificultad
3/5
Tiempo estimado
1-2 días
Estado de actividad
Estancado
Claridad
Bien especificado
Aptitud para principiantes
52/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.