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