vbhanuchander-lang opened a new pull request, #17603:
URL: https://github.com/apache/iceberg/pull/17603
Closes #16003
### What
`SparkValueWriters.UUIDWriter` carried a TODO for exactly this:
```java
// TODO: direct conversion from string to byte buffer
UUID uuid = UUID.fromString(s.toString());
encoder.writeFixed(UUIDUtil.convertToByteBuffer(uuid, BUFFER.get()).array());
```
Two intermediate objects per row, on a write path: `s.toString()` allocates
a `String` and decodes UTF-8, then `UUID.fromString` parses it with a regex,
splits and `Long.parseLong` to build a `UUID` that is immediately destructured
back into two longs for the buffer.
This adds `UUIDUtil.convertToByteBuffer(byte[] uuidText, int offset, int
length, ByteBuffer reuse)`, which reads the canonical text straight from its
ASCII bytes into the caller's buffer, plus a two-argument overload. The Spark
writer now calls it with `s.getBytes()`. No `String`, no `UUID`, and the
thread-local buffer is still reused.
### One deliberate behaviour difference
The new path is **stricter** than `UUID.fromString`, which zero-extends
short groups — `UUID.fromString("1-2-3-4-5")` parses successfully today. Only
the canonical 8-4-4-4-12 form is accepted, upper or lower case.
I chose strict because these bytes land in a data file, so accepting a
non-canonical string would persist a value the writer never really validated.
It is a narrowing, though, so if you would rather preserve the lenient
behaviour exactly I am happy to change it — there is a test pinning the current
choice either way.
### Tests
`TestUUIDUtil`, 14 tests, all passing:
- agreement with `UUID.fromString` over **1000 random UUIDs** — this is the
property that matters, since the method replaces it on the write path
- the all-zero, all-ones and sign-bit boundaries (`80000000-...`,
`7fffffff-...`), where a sloppy shift or a sign-extension bug would show up
- case insensitivity, buffer reuse returning the same instance, and reading
at an offset
- seven non-canonical inputs rejected (too short, too long, no dashes, wrong
separator, non-hex digit, trailing space, empty) rather than silently producing
different bytes
- an explicit test documenting that `"1-2-3-4-5"` parses via
`UUID.fromString` and is rejected here
```
TestUUIDUtil: tests=14 failures=0 errors=0
```
`:iceberg-api:checkstyleMain`, `:iceberg-api:checkstyleTest`,
`:iceberg-api:spotlessCheck`,
`:iceberg-spark:iceberg-spark-4.1_2.13:checkstyleMain` and `spotlessCheck` all
pass, as do the Spark 4.1 Avro write tests.
### Spark versions
Applied to v3.5, v4.0 and v4.1, matching #17482. Only one Spark version
configures in a local Gradle build at a time, so I verified the gates against
4.1; the change is byte-identical in all three.
### Not included
No JMH benchmark. The allocation reduction is structural — two objects and a
regex parse removed per value — so I did not want to attach a synthetic number
to it without a harness you would trust. Happy to add one if you would like the
figure.
--
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]
