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

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


The following commit(s) were added to refs/heads/master by this push:
     new ef61f7b5298 [fix](partition)fix default partitionkey not persistent 
and keep default on some operator (#50489)
ef61f7b5298 is described below

commit ef61f7b5298d76fce879b08bc9e0396e201e3452
Author: koarz <li...@selectdb.com>
AuthorDate: Tue Apr 29 14:17:21 2025 +0800

    [fix](partition)fix default partitionkey not persistent and keep default on 
some operator (#50489)
    
    ### What problem does this PR solve?
    
    1. The default key's isD attribute was not persisted to disk, and the
    serialization was not done with the default Gson but with the overloaded
    PartitionKeySerializer, so after restarting the fe the partitionKey's
    isD attribute was lost
    2. The show create table The default partition in show create table has
    been modified to no longer show values in
    4. The sql in addpartitionrecord no longer records the values of the
    default key.
---
 .../apache/doris/binlog/AddPartitionRecord.java    |  8 ++++--
 .../apache/doris/catalog/ListPartitionInfo.java    | 18 ++++++++----
 .../apache/doris/catalog/ListPartitionItem.java    |  3 ++
 .../org/apache/doris/catalog/PartitionKey.java     | 26 +++++++++++++++--
 .../org/apache/doris/catalog/PartitionKeyTest.java | 15 +++++++++-
 .../test_list_default_partition_show_create.groovy | 33 ++++++++++++++++++++++
 6 files changed, 91 insertions(+), 12 deletions(-)

diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/binlog/AddPartitionRecord.java 
b/fe/fe-core/src/main/java/org/apache/doris/binlog/AddPartitionRecord.java
index bb5485b2624..4ca3213ec12 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/binlog/AddPartitionRecord.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/binlog/AddPartitionRecord.java
@@ -90,8 +90,12 @@ public class AddPartitionRecord {
             sb.append("\");");
         } else if 
(!this.listPartitionItem.equals(ListPartitionItem.DUMMY_ITEM)) {
             // list
-            sb.append("VALUES IN ");
-            sb.append(((ListPartitionItem) listPartitionItem).toSql());
+            String partitionSql = ((ListPartitionItem) 
listPartitionItem).toSql();
+            // the partition may default partition
+            if (!partitionSql.isEmpty()) {
+                sb.append("VALUES IN ");
+                sb.append(partitionSql);
+            }
             sb.append(" (\"version_info\" = \"");
             sb.append(partition.getVisibleVersion());
             sb.append("\");");
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/catalog/ListPartitionInfo.java 
b/fe/fe-core/src/main/java/org/apache/doris/catalog/ListPartitionInfo.java
index 3134d7d3b3f..51b40c9f85b 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/ListPartitionInfo.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/ListPartitionInfo.java
@@ -207,21 +207,27 @@ public class ListPartitionInfo extends PartitionInfo {
             String partitionName = partition.getName();
             List<PartitionKey> partitionKeys = entry.getValue().getItems();
 
-            sb.append("PARTITION ").append(partitionName).append(" VALUES IN 
");
-            sb.append("(");
+            sb.append("PARTITION ").append(partitionName);
+            StringBuilder partitionKeysBuilder = new StringBuilder();
             int idxInternal = 0;
             for (PartitionKey partitionKey : partitionKeys) {
                 String partitionKeyStr = partitionKey.toSql();
-                if (!isMultiColumnPartition) {
+                if (!isMultiColumnPartition && !partitionKeyStr.isEmpty()) {
                     partitionKeyStr = partitionKeyStr.substring(1, 
partitionKeyStr.length() - 1);
                 }
-                sb.append(partitionKeyStr);
+                partitionKeysBuilder.append(partitionKeyStr);
                 if (partitionKeys.size() > 1 && idxInternal != 
partitionKeys.size() - 1) {
-                    sb.append(",");
+                    partitionKeysBuilder.append(",");
                 }
                 idxInternal++;
             }
-            sb.append(")");
+
+            // length == 0 means it is a default partition
+            if (partitionKeysBuilder.length() > 0) {
+                sb.append(" VALUES IN ").append("(");
+                sb.append(partitionKeysBuilder.toString());
+                sb.append(")");
+            }
 
             if (!"".equals(getStoragePolicy(entry.getKey()))) {
                 sb.append("(\"storage_policy\" = 
\"").append(getStoragePolicy(entry.getKey())).append("\")");
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/catalog/ListPartitionItem.java 
b/fe/fe-core/src/main/java/org/apache/doris/catalog/ListPartitionItem.java
index 6edabea442e..eeb82b9bf8d 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/ListPartitionItem.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/ListPartitionItem.java
@@ -188,6 +188,9 @@ public class ListPartitionItem extends PartitionItem {
     }
 
     public String toSql() {
+        if (isDefaultPartition) {
+            return "";
+        }
         StringBuilder sb = new StringBuilder();
         sb.append("(");
 
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/catalog/PartitionKey.java 
b/fe/fe-core/src/main/java/org/apache/doris/catalog/PartitionKey.java
index 29bfda8b201..56e50bd21ba 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/PartitionKey.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/PartitionKey.java
@@ -40,6 +40,7 @@ import org.apache.doris.qe.SessionVariable;
 import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Lists;
+import com.google.gson.Gson;
 import com.google.gson.JsonArray;
 import com.google.gson.JsonDeserializationContext;
 import com.google.gson.JsonDeserializer;
@@ -69,6 +70,7 @@ public class PartitionKey implements 
Comparable<PartitionKey>, Writable {
     private List<String> originHiveKeys;
     @SerializedName("ts")
     private List<PrimitiveType> types;
+    // the isD not serialize before because of Gson using 
PartitionKeySerializer
     @SerializedName("isD")
     private boolean isDefaultListPartitionKey = false;
 
@@ -366,6 +368,9 @@ public class PartitionKey implements 
Comparable<PartitionKey>, Writable {
 
     // return: ("100", "200", "300")
     public String toSql() {
+        if (isDefaultListPartitionKey) {
+            return "";
+        }
         StringBuilder sb = new StringBuilder("(");
         int i = 0;
         for (LiteralExpr expr : keys) {
@@ -594,8 +599,13 @@ public class PartitionKey implements 
Comparable<PartitionKey>, Writable {
                 jsonArray.add(typeAndKey);
             }
 
-            // for compatibility in the future
-            jsonArray.add(new JsonPrimitive("unused"));
+            // use extend as a json to record some information
+            JsonArray extend = new JsonArray();
+            
extend.add(context.serialize(partitionKey.isDefaultListPartitionKey()));
+
+            // for compatibility in the future add item before unused
+            extend.add("unused");
+            jsonArray.add(new JsonPrimitive(GsonUtils.GSON.toJson(extend)));
 
             return jsonArray;
         }
@@ -640,8 +650,18 @@ public class PartitionKey implements 
Comparable<PartitionKey>, Writable {
                     partitionKey.types.add(type);
                     partitionKey.keys.add(key);
                 }
+                JsonPrimitive extend = jsonArray.get(jsonArray.size() - 
1).getAsJsonPrimitive();
+                String extendStr = extend.getAsString();
+                // for compatibility, extend takes up the previous "unused" 
position
+                // so the last element needs to be checked here, if it is 
unused ignore it
+                if (!extendStr.equals("unused")) {
+                    // parse extend record
+                    Gson gson = new Gson();
+                    JsonArray pExtend = gson.fromJson(extendStr, 
JsonArray.class);
+                    
partitionKey.setDefaultListPartition(pExtend.get(0).getAsBoolean());
+                    // ignore the last element
+                }
 
-                // ignore the last element
                 return partitionKey;
             }
         }
diff --git 
a/fe/fe-core/src/test/java/org/apache/doris/catalog/PartitionKeyTest.java 
b/fe/fe-core/src/test/java/org/apache/doris/catalog/PartitionKeyTest.java
index c6b45c0b136..1c936da18bc 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/catalog/PartitionKeyTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/catalog/PartitionKeyTest.java
@@ -219,7 +219,9 @@ public class PartitionKeyTest {
         FakeEnv.setMetaVersion(FeConstants.meta_version);
 
         // 1. Write objects to file
-        Path path = Files.createFile(Paths.get("./keyRangePartition"));
+        Path dir = Paths.get("./keyRangePartition");
+        Files.deleteIfExists(dir);
+        Path path = Files.createFile(dir);
         DataOutputStream dos = new 
DataOutputStream(Files.newOutputStream(path));
 
         PartitionKey keyEmpty = new PartitionKey();
@@ -251,6 +253,13 @@ public class PartitionKeyTest {
         PartitionKey key = PartitionKey.createPartitionKey(keys, columns);
         key.write(dos);
 
+        keys.clear();
+        List<Type> types = new ArrayList<Type>();
+        types.add(ScalarType.createType(PrimitiveType.INT));
+        PartitionKey defaultKey = 
PartitionKey.createListPartitionKeyWithTypes(keys, types, false);
+        Assert.assertTrue(defaultKey.isDefaultListPartitionKey());
+        defaultKey.write(dos);
+
         dos.flush();
         dos.close();
 
@@ -264,6 +273,10 @@ public class PartitionKeyTest {
         Assert.assertEquals(key, key);
         Assert.assertNotEquals(key, this);
 
+        PartitionKey rDefaultKey = PartitionKey.read(dis);
+        Assert.assertEquals(defaultKey, rDefaultKey);
+        Assert.assertTrue(rDefaultKey.isDefaultListPartitionKey());
+
         // 3. delete files
         dis.close();
         Files.deleteIfExists(path);
diff --git 
a/regression-test/suites/partition_p0/list_partition/test_list_default_partition_show_create.groovy
 
b/regression-test/suites/partition_p0/list_partition/test_list_default_partition_show_create.groovy
new file mode 100644
index 00000000000..9021b06c3b6
--- /dev/null
+++ 
b/regression-test/suites/partition_p0/list_partition/test_list_default_partition_show_create.groovy
@@ -0,0 +1,33 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+suite("test_list_default_partition_show_create") {
+    sql "drop table if exists list_default"
+    sql """
+        CREATE TABLE IF NOT EXISTS list_default ( 
+            k1 int NOT NULL, 
+            k2 int NOT NULL) 
+        UNIQUE KEY(k1)
+        PARTITION BY LIST(k1) ( 
+            PARTITION p1 ) 
+        DISTRIBUTED BY HASH(k1) BUCKETS 5 properties("replication_num" = "1")
+        """
+
+    def res = sql "show create table list_default"
+    logger.info(res[0][1])
+    assertTrue(res[0][1].contains("(PARTITION p1)"))
+}


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to