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 783a0e3f fix(spec): reject non-primitive primary-key and partition-key 
types (#976)
783a0e3f is described below

commit 783a0e3fd1f00d69e8e6b60daa363e369990d343
Author: jackylee <[email protected]>
AuthorDate: Sat Oct 3 18:07:52 2026 +0800

    fix(spec): reject non-primitive primary-key and partition-key types (#976)
---
 crates/paimon/src/spec/schema.rs | 137 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 120 insertions(+), 17 deletions(-)

diff --git a/crates/paimon/src/spec/schema.rs b/crates/paimon/src/spec/schema.rs
index cfe2df5a..d6ba4cec 100644
--- a/crates/paimon/src/spec/schema.rs
+++ b/crates/paimon/src/spec/schema.rs
@@ -1202,7 +1202,7 @@ impl Schema {
         // Create and alter share this, so an unparsable type never lands.
         CoreOptions::new(options).table_type()?;
         validate_no_reserved_field_names(fields)?;
-        Self::validate_key_field_types(fields, primary_keys, options)?;
+        Self::validate_key_field_types(fields, partition_keys, primary_keys, 
options)?;
         Self::validate_row_tracking(primary_keys, options)?;
         Self::validate_blob_fields(fields, partition_keys, primary_keys, 
options)?;
         Self::validate_primary_key_blob_configuration(fields, primary_keys, 
options)?;
@@ -1459,34 +1459,57 @@ impl Schema {
         Ok(())
     }
 
-    /// Reject types that cannot serve as a key (primary key or explicit
-    /// `bucket-key`). Currently only `VECTOR` is rejected here: it is densely
-    /// stored and has no key ordering, so it cannot be used as a key column.
+    /// Reject types that cannot serve as a key. Primary and partition keys 
mirror
+    /// Java `SchemaValidation.validateOnlyContainPrimitiveType`, which forbids
+    /// nested/complex types (`MAP`, `ARRAY`, `ROW`, `MULTISET`, `VECTOR`,
+    /// `VARIANT`): they have no key ordering, and a partition value is encoded
+    /// into a directory path. A bucket key only hashes a `BinaryRow`, so it 
allows
+    /// the one extra type that encodes into that row without ordering — 
`VARIANT`
+    /// — mirroring Java's narrower nested-type bucket check 
(`ARRAY`/`MULTISET`/
+    /// `MAP`/`ROW`). `VECTOR` stays rejected everywhere.
     fn validate_key_field_types(
         fields: &[DataField],
+        partition_keys: &[String],
         primary_keys: &[String],
         options: &HashMap<String, String>,
     ) -> crate::Result<()> {
-        let reject = |key_kind: &str, name: &str| -> crate::Result<()> {
-            let field = fields.iter().find(|f| f.name() == name);
-            if let Some(field) = field {
-                if matches!(field.data_type(), DataType::Vector(_)) {
-                    return Err(crate::Error::ConfigInvalid {
-                        message: format!(
-                            "The VECTOR type of {key_kind} field '{name}' is 
unsupported."
-                        ),
-                    });
-                }
+        let reject = |key_kind: &str, name: &str, for_bucket: bool| -> 
crate::Result<()> {
+            let Some(field) = fields.iter().find(|f| f.name() == name) else {
+                return Ok(());
+            };
+            let unsupported = match field.data_type() {
+                DataType::Map(_) => Some("MAP"),
+                DataType::Array(_) => Some("ARRAY"),
+                DataType::Multiset(_) => Some("MULTISET"),
+                DataType::Row(_) => Some("ROW"),
+                DataType::Vector(_) => Some("VECTOR"),
+                // VARIANT is the one non-primitive allowed as a bucket key: it
+                // encodes into the hashed BinaryRow and needs no ordering, so
+                // only primary/partition keys reject it (Java's bucket check
+                // lists ARRAY/MULTISET/MAP/ROW, not VARIANT; VECTOR stays 
rejected
+                // everywhere).
+                DataType::Variant(_) if !for_bucket => Some("VARIANT"),
+                _ => None,
+            };
+            if let Some(type_name) = unsupported {
+                return Err(crate::Error::ConfigInvalid {
+                    message: format!(
+                        "The {type_name} type of {key_kind} field '{name}' is 
unsupported."
+                    ),
+                });
             }
             Ok(())
         };
 
         for pk in primary_keys {
-            reject("primary key", pk)?;
+            reject("primary key", pk, false)?;
+        }
+        for partition_key in partition_keys {
+            reject("partition key", partition_key, false)?;
         }
         if let Some(bucket_keys) = CoreOptions::new(options).bucket_key() {
             for bk in &bucket_keys {
-                reject("bucket key", bk)?;
+                reject("bucket key", bk, true)?;
             }
         }
         Ok(())
@@ -2377,7 +2400,9 @@ impl Default for SchemaBuilder {
 
 #[cfg(test)]
 mod tests {
-    use crate::spec::{BlobType, CharType, FloatType, IntType, VarCharType, 
VectorType};
+    use crate::spec::{
+        BlobType, CharType, FloatType, IntType, VarCharType, VariantType, 
VectorType,
+    };
 
     use super::*;
 
@@ -2948,6 +2973,84 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_schema_validation_rejects_non_primitive_partition_key() {
+        // A partition value is encoded into a directory path, so a partition 
key
+        // must be primitive. Java validateOnlyContainPrimitiveType rejects 
ARRAY;
+        // before this guard the partition-key column type was never checked.
+        let err = Schema::builder()
+            .column("id", DataType::Int(IntType::new()))
+            .column(
+                "tags",
+                DataType::Array(ArrayType::new(DataType::Int(IntType::new()))),
+            )
+            .partition_keys(["tags"])
+            .option("bucket", "1")
+            .build()
+            .unwrap_err();
+        assert!(
+            matches!(err, crate::Error::ConfigInvalid { ref message } if 
message.contains("ARRAY") && message.contains("partition key")),
+            "ARRAY partition key must be rejected, got {err:?}"
+        );
+    }
+
+    #[test]
+    fn test_schema_validation_rejects_non_primitive_primary_key() {
+        // A key has no ordering when its type is a MAP; Java rejects it, and 
this
+        // guard now covers more than the previous VECTOR-only check.
+        let err = Schema::builder()
+            .column(
+                "k",
+                DataType::Map(MapType::new(
+                    DataType::Int(IntType::new()),
+                    DataType::Int(IntType::new()),
+                )),
+            )
+            .primary_key(["k"])
+            .option("bucket", "1")
+            .build()
+            .unwrap_err();
+        assert!(
+            matches!(err, crate::Error::ConfigInvalid { ref message } if 
message.contains("MAP") && message.contains("primary key")),
+            "MAP primary key must be rejected, got {err:?}"
+        );
+    }
+
+    #[test]
+    fn test_variant_bucket_key_is_accepted() {
+        // A bucket key only hashes a BinaryRow, which already encodes 
VARIANT, so
+        // a VARIANT bucket key stays valid (Java's bucket check lists only
+        // ARRAY/MULTISET/MAP/ROW). Primary/partition keys still reject 
VARIANT.
+        let schema = Schema::builder()
+            .column("id", DataType::Int(IntType::new()))
+            .column("v", DataType::Variant(VariantType::new()))
+            .option("bucket", "2")
+            .option("bucket-key", "v")
+            .build()
+            .unwrap();
+        assert_eq!(schema.fields().len(), 2);
+    }
+
+    #[test]
+    fn test_schema_validation_rejects_non_primitive_bucket_key() {
+        // Nested types still have no hash-key semantics as a bucket key; Java
+        // rejects ARRAY/MULTISET/MAP/ROW there even though VARIANT is allowed.
+        let err = Schema::builder()
+            .column("id", DataType::Int(IntType::new()))
+            .column(
+                "tags",
+                DataType::Array(ArrayType::new(DataType::Int(IntType::new()))),
+            )
+            .option("bucket", "2")
+            .option("bucket-key", "tags")
+            .build()
+            .unwrap_err();
+        assert!(
+            matches!(err, crate::Error::ConfigInvalid { ref message } if 
message.contains("ARRAY") && message.contains("bucket key")),
+            "ARRAY bucket key must be rejected, got {err:?}"
+        );
+    }
+
     #[test]
     fn test_blob_schema_validation_accepts_valid_blob_table() {
         let schema = Schema::builder()

Reply via email to