diff --git a/crates/integrations/datafusion/tests/format_table_partition_filters.rs b/crates/integrations/datafusion/tests/format_table_partition_filters.rs index fc3027766..26e4410d6 100644 --- a/crates/integrations/datafusion/tests/format_table_partition_filters.rs +++ b/crates/integrations/datafusion/tests/format_table_partition_filters.rs @@ -137,3 +137,54 @@ async fn test_filter_on_a_partition_column_reads_the_directory_value() { } assert!(mismatches.is_empty(), "{mismatches:#?}"); } + +/// U+180E (MONGOLIAN VOWEL SEPARATOR) is `Character.isWhitespace` only on legacy +/// JDK 8 (Unicode 6.2); on the supported modern JVMs (Java 11/17, Unicode >= 6.3) +/// it is a format character. A modern JVM therefore keeps it as a literal +/// partition value and writes a `dt=` directory. The reader must format +/// the query literal the same way; folding it to the default-partition name (the +/// legacy JDK 8 behavior) prunes to the wrong directory and misses the row. +#[tokio::test] +async fn test_u180e_partition_value_is_read_as_a_modern_jvm_literal() { + let tmp = TempDir::new().expect("Failed to create temp dir"); + let mut options = Options::new(); + options.set( + CatalogOptions::WAREHOUSE, + format!("file:{}", tmp.path().display()), + ); + let catalog = Arc::new(FileSystemCatalog::new(options).expect("Failed to create catalog")); + catalog + .create_database(DATABASE, false, Default::default()) + .await + .expect("CREATE DATABASE failed"); + let schema = Schema::builder() + .column("dt", DataType::VarChar(VarCharType::new(32).unwrap())) + .column("id", DataType::BigInt(BigIntType::new())) + .partition_keys(vec!["dt".to_string()]) + .option("type", "format-table") + .option("file.format", "parquet") + .build() + .unwrap(); + catalog + .create_table(&Identifier::new(DATABASE, TABLE), schema, false) + .await + .expect("CREATE TABLE failed"); + let table_dir = tmp.path().join(format!("{DATABASE}.db")).join(TABLE); + // U+180E is not escaped by the Hive path-escaping rules, so a modern JVM + // writes the raw code point into the directory name. + write_ids(&table_dir.join("dt=\u{180E}"), &[7]); + + let mut context = SQLContext::new(); + context.register_catalog("paimon", catalog).await.unwrap(); + + let got = ids( + &context, + &format!("SELECT id FROM paimon.{DATABASE}.{TABLE} WHERE dt = '\u{180E}'"), + ) + .await; + assert_eq!( + got, + vec![7], + "an equality filter on a U+180E partition value must find the literal modern-JVM directory" + ); +} diff --git a/crates/paimon/src/spec/mod.rs b/crates/paimon/src/spec/mod.rs index 5fe8faa11..97be205c9 100644 --- a/crates/paimon/src/spec/mod.rs +++ b/crates/paimon/src/spec/mod.rs @@ -104,8 +104,8 @@ mod partition; pub use partition::Partition; mod partition_utils; pub(crate) use partition_utils::{ - bucket_path, bucket_path_under, data_file_path, escape_path_name, relative_bucket_path, - unescape_path_name, PartitionComputer, + bucket_path, bucket_path_under, data_file_path, escape_path_name, is_java_whitespace_only, + relative_bucket_path, unescape_path_name, PartitionComputer, }; mod predicate; pub(crate) use predicate::datum_cmp; diff --git a/crates/paimon/src/spec/partition_utils.rs b/crates/paimon/src/spec/partition_utils.rs index 7c3dda8c1..c24abc687 100644 --- a/crates/paimon/src/spec/partition_utils.rs +++ b/crates/paimon/src/spec/partition_utils.rs @@ -393,7 +393,12 @@ fn format_partition_value( DataType::Char(_) | DataType::VarChar(_) => { let s = row.get_string(pos)?; - if s.trim().is_empty() { + // Java folds a partition value to the default name when + // `StringUtils.isNullOrWhitespaceOnly` holds. Rust `str::trim` uses + // a different whitespace set (e.g. it trims NBSP / U+2007 / U+202F, + // which Java does not, and keeps U+001C-U+001F, which Java trims), + // so reuse the Java-matching predicate the Binary arm already uses. + if is_java_whitespace_only(s) { return Ok(default_partition_name.to_string()); } s.to_string() @@ -519,14 +524,22 @@ fn decode_java_utf8(mut bytes: &[u8]) -> String { /// Java `StringUtils.isNullOrWhitespaceOnly` checks each UTF-16 code unit with /// `Character.isWhitespace`; its whitespace set differs from Rust `str::trim`. -fn is_java_whitespace_only(value: &str) -> bool { +/// +/// The set targets the supported modern JVMs (Java 11/17). The only code point +/// whose `Character.isWhitespace` result is JVM-version dependent is U+180E +/// (MONGOLIAN VOWEL SEPARATOR): it was a space separator under the Unicode 6.2 +/// bundled with JDK 8, but Unicode 6.3 reclassified it as a format character, so +/// Java 9+ returns `false`. We follow the modern result and keep U+180E as a +/// literal partition value — a `dt=` directory written by a modern JVM is +/// then preserved and matched by an equality filter instead of being folded to +/// the default partition. +pub(crate) fn is_java_whitespace_only(value: &str) -> bool { value.chars().all(|ch| { matches!( ch, '\u{0009}'..='\u{000D}' | '\u{001C}'..='\u{0020}' | '\u{1680}' - | '\u{180E}' | '\u{2000}'..='\u{2006}' | '\u{2008}'..='\u{200A}' | '\u{2028}'..='\u{2029}' @@ -1169,6 +1182,49 @@ mod tests { ); } + #[test] + fn test_string_partition_whitespace_matches_java() { + // U+001C (file separator) is whitespace to Java's `Character.isWhitespace` + // but not to Rust `str::trim`; Java folds such a value to the default + // partition name, so we must too. + assert_single_partition( + "dt", + DataType::VarChar(VarCharType::default()), + |b| b.write_string(0, "\u{001C}"), + "dt=__DEFAULT_PARTITION__/", + true, + ); + + // A non-breaking space (U+00A0) is whitespace to Rust `str::trim` but not + // to Java, so Java keeps it as the partition value rather than folding it. + let fields = vec![make_field("dt", DataType::VarChar(VarCharType::default()))]; + let keys = vec!["dt".to_string()]; + let computer = + PartitionComputer::new(&keys, &fields, TEST_DEFAULT_PARTITION_NAME, true).unwrap(); + let mut builder = TestRowBuilder::new(1); + builder.write_string(0, "\u{00A0}"); + let row = builder.build(); + let result = computer.generate_partition_path(&row).unwrap(); + assert_ne!( + result, "dt=__DEFAULT_PARTITION__/", + "a non-breaking space must not fold to the default partition (Java keeps it)" + ); + + // U+180E is whitespace only on legacy JDK 8 (Unicode 6.2); a modern JVM + // (Java 11/17) treats it as a format character and keeps it as a literal + // partition value, so we must not fold it to the default partition. + let computer = + PartitionComputer::new(&keys, &fields, TEST_DEFAULT_PARTITION_NAME, true).unwrap(); + let mut builder = TestRowBuilder::new(1); + builder.write_string(0, "\u{180E}"); + let row = builder.build(); + let result = computer.generate_partition_path(&row).unwrap(); + assert_eq!( + result, "dt=\u{180E}/", + "U+180E must stay a literal partition value on modern JVMs" + ); + } + #[test] fn test_boolean_partition() { assert_single_partition( @@ -1497,11 +1553,13 @@ mod tests { #[test] fn test_binary_partition_matches_java_whitespace() { - // JDK 8 Character.isWhitespace includes U+001C and U+180E, but not - // U+00A0 or U+2007. Rust str::trim differs for controls and NBSP. + // Modern JVMs (Java 11/17) treat U+001C as whitespace but U+180E as a + // format character, and never fold U+00A0 or U+2007. Rust str::trim + // differs for controls and NBSP, so we match Java explicitly. for (value, expected) in [ (b"\x1c".as_slice(), "bin=__DEFAULT_PARTITION__/"), - ("\u{180E}".as_bytes(), "bin=__DEFAULT_PARTITION__/"), + // U+180E is whitespace only on legacy JDK 8; a modern JVM keeps it. + ("\u{180E}".as_bytes(), "bin=\u{180E}/"), ("\u{00A0}".as_bytes(), "bin=\u{00A0}/"), ("\u{2007}".as_bytes(), "bin=\u{2007}/"), ] { diff --git a/crates/paimon/src/table/format_partition.rs b/crates/paimon/src/table/format_partition.rs index 748c8b284..8b1879e8d 100644 --- a/crates/paimon/src/table/format_partition.rs +++ b/crates/paimon/src/table/format_partition.rs @@ -23,7 +23,7 @@ use std::collections::HashMap; use chrono::NaiveDate; use crate::io::FileIO; -use crate::spec::{escape_path_name, unescape_path_name, DataType, Datum}; +use crate::spec::{escape_path_name, is_java_whitespace_only, unescape_path_name, DataType, Datum}; const UNIX_EPOCH_DAYS_FROM_CE: i32 = 719_163; @@ -78,7 +78,13 @@ impl FormatTablePartitionPaths { } let mut segments = Vec::with_capacity(leading_values.len()); for (key, value) in self.partition_keys.iter().zip(leading_values) { - if value.trim().is_empty() { + // A value that folds to the default partition name (Java + // `StringUtils.isNullOrWhitespaceOnly`) is not stored under its + // literal spelling, so a prefix pattern built from it would miss the + // data; skip pushdown and let the caller list every partition. + // Mirrors Java `buildPartitionNamePrefixPattern`, which returns null + // for such a value; `str::trim` folds a different whitespace set. + if is_java_whitespace_only(value) { return None; } segments.push(format!( @@ -309,7 +315,11 @@ pub fn format_partition_value( (Datum::Int(value), DataType::Int(_)) => Some(value.to_string()), (Datum::Long(value), DataType::BigInt(_)) => Some(value.to_string()), (Datum::String(value), DataType::Char(_) | DataType::VarChar(_)) => { - if value.trim().is_empty() { + // Fold to the default partition name exactly when Java + // `InternalRowPartitionComputer` does (`isNullOrWhitespaceOnly`), so + // the directory matches cross-engine; `str::trim` uses a different + // whitespace set (folds NBSP, keeps U+001C-U+001F). + if is_java_whitespace_only(value) { Some(default_partition_name.to_string()) } else { Some(value.clone()) @@ -378,7 +388,7 @@ fn last_path_segment(path: &str) -> Option<&str> { #[cfg(test)] mod tests { use super::*; - use crate::spec::{BooleanType, DateType}; + use crate::spec::{BooleanType, DateType, VarCharType}; #[test] fn test_parse_format_partition_value() { @@ -524,4 +534,59 @@ mod tests { assert_eq!(is_storage_not_found(&error), expected); } } + + #[test] + fn test_format_partition_value_folds_java_whitespace_only() { + let varchar = DataType::VarChar(VarCharType::string_type()); + let default = "__DEFAULT_PARTITION__"; + + // U+00A0 (non-breaking space) is not `Character.isWhitespace` in Java, so + // it stays a real partition value. `str::trim` would wrongly fold it to + // the default and diverge from a Java-written directory. + assert_eq!( + format_partition_value( + &Datum::String("\u{00A0}".to_string()), + &varchar, + default, + false + ), + Some("\u{00A0}".to_string()) + ); + + // U+001C (file separator) is `Character.isWhitespace` in Java, so it folds + // to the default. `str::trim` keeps it, which would produce a literal + // directory Java never writes. + assert_eq!( + format_partition_value( + &Datum::String("\u{001C}".to_string()), + &varchar, + default, + false + ), + Some(default.to_string()) + ); + + // U+180E is whitespace only on legacy JDK 8 (Unicode 6.2); a modern JVM + // (Java 11/17) treats it as a format character and keeps it literal, so a + // `dt=` directory written by a modern JVM stays matchable. + assert_eq!( + format_partition_value( + &Datum::String("\u{180E}".to_string()), + &varchar, + default, + false + ), + Some("\u{180E}".to_string()) + ); + + // An ASCII-blank value folds under both rules; a normal value is kept. + assert_eq!( + format_partition_value(&Datum::String(" ".to_string()), &varchar, default, false), + Some(default.to_string()) + ); + assert_eq!( + format_partition_value(&Datum::String("dt".to_string()), &varchar, default, false), + Some("dt".to_string()) + ); + } }