Arrow: Fix valuesRead accumulation in VectorizedDeltaEncodedValuesReader#17121
Arrow: Fix valuesRead accumulation in VectorizedDeltaEncodedValuesReader#17121ebyhr wants to merge 1 commit into
Conversation
| assertThat(batch2).containsExactly(70, 80, 90, 100); | ||
|
|
||
| assertThat(reader.totalValueCount()).isEqualTo(10); | ||
| assertThatThrownBy(() -> reader.readIntegers(1, 0)) |
There was a problem hiding this comment.
reader.readIntegers(1, 0) returns [110] without this PR fix.
| remaining -= loadedRows; | ||
| } | ||
| valuesRead = total - remaining; | ||
| valuesRead += total - remaining; |
There was a problem hiding this comment.
I think remaining is always 0 here. The while loop exit criteria ensures that it is non-positive. InloadMiniBlockToOutput remaining is an upper bound for the return value loadedRows so remaining -= loadedRows cannot be negative either.
It could be only negative if valuesRead = 0 and total = 0. In this case it throws an exception now because we try to write the first value to a zero size array. So even if it's a valid use it's buggy anyway. I would add a check or early return if total is 0 or less.
There was a problem hiding this comment.
I just saw that the negative case is handled by #17121.
| int currentRowId = rowId; | ||
| // First value | ||
| if (valuesRead == 0) { | ||
| outputWriter.write(vec, ((long) (currentRowId + valuesRead) * typeWidth), firstValue); |
There was a problem hiding this comment.
Nit: I noticed that valuesRead is always 0, so the code can be simplified here.
2 participants
VectorizedDeltaEncodedValuesReader.readValuesresetsvaluesReadto the current batch size on every call (valuesRead = total - remaining) instead of accumulating it (+=). On a multi-batch page read the overflow guardvaluesRead + total > totalValueCountsees a stale count and silently allows reads past the logical end of the page.The bug does not throw - DELTA_BINARY_PACKED mini-blocks are zero-padded to a fixed size, so the reader decodes the padding zeros as valid deltas and emits plausible-looking but wrong values. For example, a page of [10, 20, ..., 100] read in batches of 6 and 4, then read once more, returns 110 - the next extrapolated delta step - with no error.