apache / apache/arrow-java

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

オープン
#471 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る
help wanted Type: bug
主要言語
Java
スター
94
フォーク
152
平均マージ
3日 16時間
マージ済み PR(30日)
11

説明

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

コントリビューションガイド

コントリビューションガイドを開く

調査の方向性

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.

索引モデルが issue の本文から書いたものです。

評価

技術スタック
java
領域
data-engineering
issue の種類
バグ
難易度
3/5
見積もり時間
1〜2日
活発さ
停滞
明瞭さ
明確に書かれている
初心者へのやさしさ
52/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。