This is an automated email from the ASF dual-hosted git repository.

JingsongLi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/paimon.git


The following commit(s) were added to refs/heads/master by this push:
     new 604771b8b4 [core] optimize column directives (#9304)
604771b8b4 is described below

commit 604771b8b46e47ff02c85d68617037bfb958b1c4
Author: Faiz <[email protected]>
AuthorDate: Thu Aug 20 09:18:52 2026 +0800

    [core] optimize column directives (#9304)
---
 .../apache/paimon/schema/ColumnDirectiveUtils.java | 17 +++++++++++-
 .../org/apache/paimon/schema/SchemaManager.java    |  6 ++++
 .../paimon/schema/ColumnDirectiveUtilsTest.java    | 14 ++++++++++
 .../apache/paimon/table/SchemaEvolutionTest.java   | 32 ++++++++++++++++++++++
 4 files changed, 68 insertions(+), 1 deletion(-)

diff --git 
a/paimon-core/src/main/java/org/apache/paimon/schema/ColumnDirectiveUtils.java 
b/paimon-core/src/main/java/org/apache/paimon/schema/ColumnDirectiveUtils.java
index d94f70f9de..6d9c77d2f0 100644
--- 
a/paimon-core/src/main/java/org/apache/paimon/schema/ColumnDirectiveUtils.java
+++ 
b/paimon-core/src/main/java/org/apache/paimon/schema/ColumnDirectiveUtils.java
@@ -34,9 +34,13 @@ import org.apache.paimon.utils.StringUtils;
 import javax.annotation.Nullable;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
 
 /** Utilities for column comment directives (BLOB / VECTOR type conversion via 
ADD COLUMN). */
 public final class ColumnDirectiveUtils {
@@ -310,7 +314,18 @@ public final class ColumnDirectiveUtils {
                 }
             }
         }
-        String newValue = existing == null ? fieldName : existing + "," + 
fieldName;
+
+        Set<String> values = new LinkedHashSet<>();
+        if (existing != null) {
+            values.addAll(
+                    Arrays.stream(existing.split(","))
+                            .map(String::trim)
+                            .filter(str -> !str.isEmpty())
+                            .collect(Collectors.toList()));
+        }
+        values.add(fieldName);
+
+        String newValue = StringUtils.join(values.iterator(), ",");
         options.put(optionKey, newValue);
     }
 
diff --git 
a/paimon-core/src/main/java/org/apache/paimon/schema/SchemaManager.java 
b/paimon-core/src/main/java/org/apache/paimon/schema/SchemaManager.java
index b55bff318b..9161d29337 100644
--- a/paimon-core/src/main/java/org/apache/paimon/schema/SchemaManager.java
+++ b/paimon-core/src/main/java/org/apache/paimon/schema/SchemaManager.java
@@ -107,6 +107,7 @@ import static 
org.apache.paimon.catalog.Identifier.UNKNOWN_DATABASE;
 import static 
org.apache.paimon.mergetree.compact.PartialUpdateMergeFunction.SEQUENCE_GROUP;
 import static 
org.apache.paimon.schema.ColumnDirectiveUtils.applyAddColumnDirective;
 import static org.apache.paimon.schema.ColumnDirectiveUtils.applyDirectives;
+import static 
org.apache.paimon.schema.ColumnDirectiveUtils.parseAddColumnComment;
 import static org.apache.paimon.types.BlobType.isBlobFileField;
 import static org.apache.paimon.utils.DefaultValueUtils.validateDefaultValue;
 import static org.apache.paimon.utils.FileUtils.listVersionedFiles;
@@ -577,6 +578,11 @@ public class SchemaManager implements Serializable {
                         lazyIdentifier);
             } else if (change instanceof UpdateColumnComment) {
                 UpdateColumnComment update = (UpdateColumnComment) change;
+                Preconditions.checkArgument(
+                        parseAddColumnComment(update.newDescription()) == null,
+                        "Should not alter existing field's type through column 
directives: %s",
+                        update.newDescription());
+
                 updateNestedColumn(
                         newFields,
                         update.fieldNames(),
diff --git 
a/paimon-core/src/test/java/org/apache/paimon/schema/ColumnDirectiveUtilsTest.java
 
b/paimon-core/src/test/java/org/apache/paimon/schema/ColumnDirectiveUtilsTest.java
index 8c866cf700..c8ab49525d 100644
--- 
a/paimon-core/src/test/java/org/apache/paimon/schema/ColumnDirectiveUtilsTest.java
+++ 
b/paimon-core/src/test/java/org/apache/paimon/schema/ColumnDirectiveUtilsTest.java
@@ -141,6 +141,20 @@ public class ColumnDirectiveUtilsTest {
         assertThat(opts).containsEntry(CoreOptions.BLOB_FIELD.key(), 
"existing,new_col");
     }
 
+    @Test
+    public void testBlobDirectiveDeduplicatesExistingOptionAndPreservesOrder() 
{
+        Map<String, String> opts = new HashMap<>();
+        opts.put(CoreOptions.BLOB_FIELD.key(), "first,pic");
+
+        ColumnDirectiveUtils.applyAddColumnDirective(
+                "__BLOB_FIELD", "pic", DataTypes.BYTES(), opts);
+        assertThat(opts).containsEntry(CoreOptions.BLOB_FIELD.key(), 
"first,pic");
+
+        ColumnDirectiveUtils.applyAddColumnDirective(
+                "__BLOB_FIELD", "last", DataTypes.BYTES(), opts);
+        assertThat(opts).containsEntry(CoreOptions.BLOB_FIELD.key(), 
"first,pic,last");
+    }
+
     @Test
     public void testBareDirectiveWithoutComment() {
         Map<String, String> opts = new HashMap<>();
diff --git 
a/paimon-core/src/test/java/org/apache/paimon/table/SchemaEvolutionTest.java 
b/paimon-core/src/test/java/org/apache/paimon/table/SchemaEvolutionTest.java
index 33531d8720..59acdec83e 100644
--- a/paimon-core/src/test/java/org/apache/paimon/table/SchemaEvolutionTest.java
+++ b/paimon-core/src/test/java/org/apache/paimon/table/SchemaEvolutionTest.java
@@ -524,6 +524,7 @@ public class SchemaEvolutionTest {
     @Test
     public void testCreateTableWithCommentDirectives() throws Exception {
         Map<String, String> options = blobEnabledOptions();
+        options.put(CoreOptions.BLOB_FIELD.key(), "pic");
         options.put(CoreOptions.VECTOR_FILE_FORMAT.key(), "json");
         schemaManager.createTable(
                 new Schema(
@@ -577,6 +578,37 @@ public class SchemaEvolutionTest {
         
assertThat(latest.options().get(CoreOptions.VECTOR_FIELD.key())).isEqualTo("embedding");
     }
 
+    @Test
+    public void testUpdateColumnCommentRejectsDirectives() throws Exception {
+        schemaManager.createTable(
+                Schema.newBuilder().column("pic", DataTypes.BYTES(), "original 
comment").build());
+
+        for (String directive :
+                Arrays.asList(
+                        "__BLOB_FIELD",
+                        "__BLOB_DESCRIPTOR_FIELD",
+                        "__BLOB_VIEW_FIELD",
+                        "__VECTOR_FIELD;64")) {
+            assertThatThrownBy(
+                            () ->
+                                    schemaManager.commitChanges(
+                                            
SchemaChange.updateColumnComment("pic", directive)))
+                    .isInstanceOf(IllegalArgumentException.class)
+                    .hasMessageContaining(
+                            "Should not alter existing field's type through 
column directives");
+        }
+
+        TableSchema latest = schemaManager.latest().get();
+        assertThat(latest.fields().get(0).type()).isEqualTo(DataTypes.BYTES());
+        assertThat(latest.fields().get(0).description()).isEqualTo("original 
comment");
+        assertThat(latest.options())
+                .doesNotContainKeys(
+                        CoreOptions.BLOB_FIELD.key(),
+                        CoreOptions.BLOB_DESCRIPTOR_FIELD.key(),
+                        CoreOptions.BLOB_VIEW_FIELD.key(),
+                        CoreOptions.VECTOR_FIELD.key());
+    }
+
     private static Map<String, String> blobEnabledOptions() {
         Map<String, String> options = new HashMap<>();
         options.put(CoreOptions.DATA_EVOLUTION_ENABLED.key(), "true");

Reply via email to