LuciferYang commented on code in PR #3447:
URL: https://github.com/apache/parquet-java/pull/3447#discussion_r3370740291
##########
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:
Confirmed. `getBytes(UTF_8)` replaces malformed input instead of throwing,
so it wasn't behavior-preserving and the `catch` wasn't dead code.
Went with your suggestion and now use a fresh `CharsetEncoder` per call: it
still drops the `ThreadLocal`/lambda behind the leak, while `newEncoder()`
keeps the default `CodingErrorAction.REPORT`, so unpaired surrogates still fail
fast with a `ParquetEncodingException`.
Also added tests (valid encoding + malformed-UTF-16 rejection), corrected
the misleading "UTF-8 not supported." message, and updated the PR description
to match.
--
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]
