leaves12138 commented on code in PR #958:
URL: https://github.com/apache/paimon-rust/pull/958#discussion_r4106219107
##########
crates/paimon/src/spec/partition_utils.rs:
##########
@@ -244,6 +244,16 @@ fn format_partition_value(
s.to_string()
}
+ DataType::Binary(_) | DataType::VarBinary(_) if !legacy => {
+ // Java's BinaryToStringCastRule wraps the raw bytes in a
+ // BinaryString, whose toString decodes them as UTF-8.
+ let value =
String::from_utf8_lossy(row.get_binary(pos)?).into_owned();
Review Comment:
[P2] Match Java's replacement behavior for malformed UTF-8
`String::from_utf8_lossy` does not always match
`BinaryString.fromBytes(...).toString()` for arbitrary binary values. For the
three bytes `ED A0 80`, Java's decoder emits one U+FFFD, whereas Rust emits
three. Thus Java computes `bin=<U+FFFD>/`, but this writer creates
`bin=<U+FFFD><U+FFFD><U+FFFD>/`. I verified the Java output and a persisted
Rust table reproduction: the Rust file exists, but the Java-derived file path
does not. BINARY/VARBINARY does not require valid UTF-8, so this is a supported
input rather than an invalid string column. Please reproduce Java's
malformed-sequence replacement semantics (and test this byte sequence) instead
of using Rust's default lossy decoding for a persisted cross-language path.
##########
crates/paimon/src/spec/partition_utils.rs:
##########
@@ -244,6 +244,16 @@ fn format_partition_value(
s.to_string()
}
+ DataType::Binary(_) | DataType::VarBinary(_) if !legacy => {
+ // Java's BinaryToStringCastRule wraps the raw bytes in a
+ // BinaryString, whose toString decodes them as UTF-8.
+ let value =
String::from_utf8_lossy(row.get_binary(pos)?).into_owned();
+ if value.trim().is_empty() {
Review Comment:
[P2] Use Java's whitespace predicate for binary partition values
For the newly supported BINARY/VARBINARY partition path, Rust `str::trim` is
not equivalent to Java `StringUtils.isNullOrWhitespaceOnly`, which uses
`Character.isWhitespace`. With `partition.legacy-name=false`, bytes `C2 A0`
(U+00A0, non-breaking space) produce `bin=<U+00A0>/` in Java but
`bin=__DEFAULT_PARTITION__/` here. Conversely, byte `1C` produces the default
partition in Java but `bin=%1C/` here. Both inputs are valid UTF-8. I verified
both using Java's partition computer and real Rust table writes followed by
commit/scan: the file exists under Rust's directory, not the one Java
reconstructs from its manifest partition bytes. Please use a Java-compatible
blank predicate and cover these values; Rust-only round trips do not detect
this cross-engine missing-file problem. This finding is about the newly added
binary branch, not the pre-existing VARCHAR branch.
--
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]
