david-mollitor-db opened a new pull request, #58887:
URL: https://github.com/apache/spark/pull/58887
### What changes were proposed in this pull request?
`VectorizedPlainValuesReader.readGeoData` (the vectorized PLAIN-encoding
read path for
GEOMETRY/GEOGRAPHY columns) buffered every converted value for the whole
page into a
`ByteBufferOutputStream` created with no initial capacity -- so it started
at the JDK default of 32
bytes and grew by repeatedly doubling its backing array -- then copied the
result out via
`toByteArray()` into `WritableColumnVector.arrayData().appendBytes(...)` in
a single operation. It
also wrote a redundant 4-byte length prefix before each value.
This rewrites the method to append each converted value directly to the
column vector's
`arrayData()` and record it with `putArray(...)`, matching what
`VectorizedDeltaByteArrayReader.readGeoData` already does for the same types:
```java
WritableColumnVector arrayData = v.arrayData();
for (int i = 0; i < total; i++) {
int len = readInteger();
byte[] physicalValue = converter.convert(in.readNBytes(len), srid);
int offset = arrayData.getElementsAppended();
arrayData.appendBytes(physicalValue.length, physicalValue, 0);
v.putArray(rowId + i, offset, physicalValue.length);
}
```
### Why are the changes needed?
The intermediate `ByteBufferOutputStream` starts at 32 bytes and reallocates
+ copies its backing
array on the order of `log2(pageBytes / 32)` times as it accumulates the
page; `toByteArray()` then
copies it again, and the final `appendBytes` copies it a third time into the
column vector.
`arrayData()` is the value's final destination and already auto-grows
(amortized) through `reserve`,
so appending directly removes the intermediate buffer and the extra copies.
The 4-byte length prefix
was redundant -- the element length is already recorded by `putArray`'s
third argument, and
`putArray` pointed past the prefix, so those bytes were never read -- and is
dropped as well, saving
4 bytes per value. This also re-converges `readGeoData` with the
delta-encoding reader, which
already used this pattern.
### Does this PR introduce _any_ user-facing change?
No. The decoded column is byte-for-byte identical: each element's array is
the same physical value
at the same length.
### How was this patch tested?
Existing `ParquetGeoSuite` (writes and reads Geometry and Geography columns
through the vectorized
reader), plus `ParquetDeltaByteArrayEncodingSuite` and
`ParquetDeltaLengthByteArrayEncodingSuite`
(which exercise `readGeometry` / `readGeography`), pass. This is a
behavior-preserving change, so no
new tests were added.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Isaac
This pull request and its description were written by Isaac.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
