This is an automated email from the ASF dual-hosted git repository.
JingsongLi pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/paimon-rust.git
The following commit(s) were added to refs/heads/main by this push:
new 23656594 fix(spec): match Java whitespace for CHAR/VARCHAR partition
values (#985)
23656594 is described below
commit 236565941a07f29e6193fb27a414a0b6ec4547b0
Author: jackylee <[email protected]>
AuthorDate: Sun Oct 4 22:23:58 2026 +0800
fix(spec): match Java whitespace for CHAR/VARCHAR partition values (#985)
---
.../datafusion/src/format_partition_ddl.rs | 15 +++--
.../tests/format_table_partition_filters.rs | 51 +++++++++++++++
.../datafusion/tests/rest_format_partition_sql.rs | 14 +++-
crates/paimon/src/spec/mod.rs | 1 +
crates/paimon/src/spec/partition_utils.rs | 74 ++++++++++++++++++++--
crates/paimon/src/table/format_partition.rs | 73 +++++++++++++++++++--
6 files changed, 212 insertions(+), 16 deletions(-)
diff --git a/crates/integrations/datafusion/src/format_partition_ddl.rs
b/crates/integrations/datafusion/src/format_partition_ddl.rs
index 7d0c5bed..c2d4eeac 100644
--- a/crates/integrations/datafusion/src/format_partition_ddl.rs
+++ b/crates/integrations/datafusion/src/format_partition_ddl.rs
@@ -34,7 +34,7 @@ use datafusion::sql::sqlparser::keywords::Keyword;
use datafusion::sql::sqlparser::parser::Parser;
use datafusion::sql::sqlparser::tokenizer::{Token, Tokenizer};
use paimon::catalog::{Catalog, Identifier};
-use paimon::spec::{CoreOptions, DataType as PaimonDataType};
+use paimon::spec::{is_java_whitespace_only, CoreOptions, DataType as
PaimonDataType};
use paimon::table::{
format_partition_value, parse_format_partition_value,
FormatTablePartitionPaths,
};
@@ -487,10 +487,17 @@ pub(crate) fn parse_format_partition_spec(
Some(text) => {
let text = match data_type {
PaimonDataType::Char(_) | PaimonDataType::VarChar(_) => {
- // A blank string would address the default partition;
Java
- // `PaimonFormatTable.requireNameablePartitionValues`
refuses it too.
+ // A value that folds to the default partition would
address
+ // the NULL partition, so Java `PaimonFormatTable.
+ // requireNameablePartitionValues` refuses it for a
mutation.
+ // Classify with the same Java-whitespace predicate the
+ // formatter folds by (`is_java_whitespace_only`), not
Rust
+ // `str::trim`, whose set differs (trim keeps
U+001C-U+001F,
+ // which Java folds): a bare `trim().is_empty()` would
let
+ // `DROP PARTITION (c = '<U+001C>')` through and
destructively
+ // address the default partition.
if let Some(operation) = mutating_operation {
- if text.trim().is_empty() {
+ if is_java_whitespace_only(&text) {
return Err(DataFusionError::Plan(format!(
"{operation} does not support an empty or
whitespace-only \
string for partition column '{column}' of
Format Table {}. \
diff --git
a/crates/integrations/datafusion/tests/format_table_partition_filters.rs
b/crates/integrations/datafusion/tests/format_table_partition_filters.rs
index fc302776..26e4410d 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=<U+180E>` 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/integrations/datafusion/tests/rest_format_partition_sql.rs
b/crates/integrations/datafusion/tests/rest_format_partition_sql.rs
index 78c2cdc5..12fabbfa 100644
--- a/crates/integrations/datafusion/tests/rest_format_partition_sql.rs
+++ b/crates/integrations/datafusion/tests/rest_format_partition_sql.rs
@@ -305,8 +305,12 @@ async fn test_partition_literals() {
}
}
-/// A blank string for a string partition column is written to the default
partition, so ADD and
-/// DROP refuse it rather than address the NULL partition, as Java does.
+/// A value that folds to the default partition for a string column is written
to the default
+/// partition, so ADD and DROP refuse it rather than address the NULL
partition, as Java does.
+/// This covers Rust-blank strings and the control characters U+001C-U+001F,
which Java's
+/// `Character.isWhitespace` folds but Rust `str::trim` keeps: the mutating
guard must classify
+/// with the same Java-whitespace predicate the formatter folds by, or `DROP
PARTITION
+/// (label = '<U+001C>')` would silently delete the default partition's
registration and files.
#[cfg(not(windows))]
#[tokio::test]
async fn test_partition_ddl_refuses_blank_string_values() {
@@ -326,6 +330,12 @@ async fn test_partition_ddl_refuses_blank_string_values() {
"DROP IF EXISTS PARTITION (label = ' ')",
"ADD PARTITION (label = '')",
"ADD IF NOT EXISTS PARTITION (label = ' ')",
+ // Control characters that are Java whitespace but not Rust `str::trim`
+ // whitespace: a `trim().is_empty()` guard would let these address the
+ // default partition and (for DROP) destroy it.
+ "DROP PARTITION (label = '\u{001C}')",
+ "DROP IF EXISTS PARTITION (label = '\u{001F}')",
+ "ADD PARTITION (label = '\u{001E}')",
] {
common::assert_sql_error(
&context,
diff --git a/crates/paimon/src/spec/mod.rs b/crates/paimon/src/spec/mod.rs
index 2c8be11b..e7b2a9af 100644
--- a/crates/paimon/src/spec/mod.rs
+++ b/crates/paimon/src/spec/mod.rs
@@ -104,6 +104,7 @@ pub use types::*;
mod partition;
pub use partition::Partition;
mod partition_utils;
+pub use partition_utils::is_java_whitespace_only;
pub(crate) use partition_utils::{
bucket_path, bucket_path_under, data_file_path, escape_path_name,
relative_bucket_path,
unescape_path_name, PartitionComputer,
diff --git a/crates/paimon/src/spec/partition_utils.rs
b/crates/paimon/src/spec/partition_utils.rs
index 7c3dda8c..d219d928 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,26 @@ 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=<U+180E>` directory written by a modern
JVM is
+/// then preserved and matched by an equality filter instead of being folded to
+/// the default partition.
+///
+/// Public so the DataFusion partition-DDL layer validates mutating partition
+/// values with the same classification that this crate folds by, matching Java
+/// `PaimonFormatTable.requireNameablePartitionValues`.
+pub 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 +1186,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 +1557,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 748c8b28..8b1879e8 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=<U+180E>` 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())
+ );
+ }
}
