Copilot commented on code in PR #3447:
URL: https://github.com/apache/parquet-java/pull/3447#discussion_r3370556916
##########
parquet-column/src/main/java/org/apache/parquet/io/api/Binary.java:
##########
@@ -268,15 +264,8 @@ public String toString() {
return "Binary{\"" + toStringUsingUTF8() + "\"}";
}
- private static final ThreadLocal<CharsetEncoder> ENCODER =
- ThreadLocal.withInitial(StandardCharsets.UTF_8::newEncoder);
-
private static ByteBuffer encodeUTF8(CharSequence value) {
- try {
- return ENCODER.get().encode(CharBuffer.wrap(value));
- } catch (CharacterCodingException e) {
- throw new ParquetEncodingException("UTF-8 not supported.", e);
- }
+ return
ByteBuffer.wrap(value.toString().getBytes(StandardCharsets.UTF_8));
}
Review Comment:
`value.toString().getBytes(UTF_8)` changes failure behavior for malformed
surrogate sequences: the previous `CharsetEncoder.encode(...)` path would throw
`CharacterCodingException` (wrapped as `ParquetEncodingException`) when the
`CharSequence` contains invalid UTF-16 (e.g., unpaired surrogates), while
`String#getBytes(UTF_8)` typically replaces malformed input. This is a
behavioral change and also makes the PR description claim of "identical"
encoding behavior inaccurate for such inputs. If you want to keep the new
stateless approach *and* preserve the prior strictness, consider creating a
fresh encoder per call (no `ThreadLocal`) and keep throwing on malformed input.
--
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]
