You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "vbhanuchander-lang (via GitHub)" <gi...@apache.org> on 2026/08/11 00:15:35 UTC
[PR] API, Spark: Add direct UUID string-to-ByteBuffer conversion [iceberg]
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Spark: Add direct UUID string-to-ByteBuffer conversion [iceberg]
Posted by "vbhanuchander-lang (via GitHub)" <gi...@apache.org>.
vbhanuchander-lang commented on PR #17603:
URL: https://github.com/apache/iceberg/pull/17603#issuecomment-5518540496
Re-checked against `main` (`8ea7d00`) today: still `MERGEABLE`, 38 checks green, no conflicts. This
closes #16003, which @abhishek593 filed against the `// TODO: direct conversion from string to byte
buffer` that was already sitting in `SparkValueWriters.UUIDWriter`.
There is one decision here that needs a maintainer rather than more review time, so to make it easy
to answer: **the new path is stricter than `UUID.fromString`.** `UUID.fromString("1-2-3-4-5")` parses
today by zero-extending short groups; the byte-level parser accepts only canonical 8-4-4-4-12, upper
or lower case. I chose strict because these bytes are persisted into a data file, so a non-canonical
string would be written as a value the writer never really validated — but it is a narrowing of
accepted input on a write path, and there is a test pinning whichever way you want it.
If the lenient behaviour should be preserved exactly, say so and I will change it in one commit. If
strict is right, this is ready.
@RussellSpitzer @pvary you have both merged in `api/util` and the Spark writers recently — would one
of you be willing to make that call?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Spark: Add direct UUID string-to-ByteBuffer conversion [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #17603: API, Spark: Add direct UUID string-to-ByteBuffer conversion
URL: https://github.com/apache/iceberg/pull/17603
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Spark: Add direct UUID string-to-ByteBuffer conversion [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #17603:
URL: https://github.com/apache/iceberg/pull/17603#issuecomment-6115644121
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Spark: Add direct UUID string-to-ByteBuffer conversion [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #17603:
URL: https://github.com/apache/iceberg/pull/17603#issuecomment-5975252646
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Spark: Add direct UUID string-to-ByteBuffer conversion [iceberg]
Posted by "vbhanuchander-lang (via GitHub)" <gi...@apache.org>.
vbhanuchander-lang commented on PR #17603:
URL: https://github.com/apache/iceberg/pull/17603#issuecomment-5274290087
Gentle bump — this one is waiting on two things a committer can unblock.
The workflow runs are all sitting at `action_required`, so CI has never executed on this
branch. Could someone approve the run? Locally the new `TestUUIDUtil` cases pass, including a
1000-random-UUID equivalence check against `UUID.fromString`, and the Spark modules build on
v3.5, v4.0 and v4.1.
cc @nastra (you have the most history on `UUIDUtil`) and @manuzhang (you last touched
`SparkValueWriters` on v4.1) — the change resolves the existing
`// TODO: direct conversion from string to byte buffer` in `SparkValueWriters.UUIDWriter` by
parsing the 36-byte UUID text straight into the reusable `ByteBuffer`, instead of allocating a
`UUID` per value. Happy to split the API and Spark parts if you would rather review them
separately.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org