iemejia commented on code in PR #55924:
URL: https://github.com/apache/spark/pull/55924#discussion_r3925088973
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetDeltaByteArrayEncodingSuite.scala:
##########
@@ -100,6 +214,117 @@ class ParquetDeltaByteArrayEncodingSuite extends
ParquetCompatibilityTest with S
geoType)
}
+ testGeo("geo interleaves skipBinary with readGeoData (null/skipped rows)") {
geoType =>
+ // A geometry column with null or skipped rows alternates skipBinary (used
for the
+ // skipped rows) with readGeometry/readGeography on the same reader. Both
paths now
+ // share the reusable prevBuf, so the shared prefix carried across a
skipped value must
+ // still be honored by the following read. The values also exceed
prevBuf's initial
+ // 64-byte capacity, exercising the grow-and-preserve branch under
interleaving.
+ assertGeoReadWriteWithSkip(writer, reader, sharedPrefixPolygons(6),
geoType)
Review Comment:
You're right -- the old shape couldn't tell a bug from working code. Fixed
in cee762f73f2 by giving the geo interleave test the same shape as
`longPrefixValues` on the non-geo side, via a new `growingPrefixPolygons()`
with point counts `(8, 12, 12, 16, 30, 30)`:
- **Grow branch with non-zero prefix:** the values now differ in length, so
the 30-point polygon at an even (read) index crosses `prevBuf`'s grown capacity
and `readGeoData` takes the grow-and-preserve branch with a non-zero
`prefixLength` (the 9-byte WKB header shared with the shorter predecessor) --
not just the first value at prefix 0.
- **Detects a broken skip:** each equal-length pair (indices 1/2 and 4/5)
differs only in a late coordinate, so an even row reads with a long prefix
against the odd row `skipBinary` just processed, while the even row two
positions back has a different point count and diverges at the `numPoints`
field (byte 9, a 9-byte prefix). So a read after a skip genuinely depends on
the prefix `skipBinary` left in `prevBuf`; if `skipBinary` stopped maintaining
it, that read decodes wrong bytes or trips the prefix-length guard rather than
passing.
I left `sharedPrefixPolygons` untouched, so `corrupt first geo value with a
non-zero prefix fails fast` (:266) keeps its long shared prefix -- the reshape
is confined to the interleave test. All three geo types pass.
--
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]
