Skip to content

Commit ab75164

Browse files
committed
[core] fix NPE and ArrayIndexOutOfBoundsException for PartitionExpire
1 parent 4ed46aa commit ab75164

File tree

7 files changed

+56
-11
lines changed

7 files changed

+56
-11
lines changed

paimon-core/src/main/java/org/apache/paimon/operation/PartitionExpire.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,8 @@ private List<Map<String, String>> convertToPartitionString(
185185
return expiredPartValues.stream()
186186
.map(values -> String.join(DELIMITER, values))
187187
.sorted()
188-
.map(s -> s.split(DELIMITER))
188+
// Use split(DELIMITER, -1) to preserve trailing empty strings
189+
.map(s -> s.split(DELIMITER, -1))
189190
.map(strategy::toPartitionString)
190191
.limit(Math.min(expiredPartValues.size(), maxExpireNum))
191192
.collect(Collectors.toList());

paimon-core/src/main/java/org/apache/paimon/partition/PartitionExpireStrategy.java

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,13 @@
3939
public abstract class PartitionExpireStrategy {
4040

4141
protected final List<String> partitionKeys;
42+
protected final String partitionDefaultName;
4243
private final RowDataToObjectArrayConverter toObjectArrayConverter;
4344

44-
public PartitionExpireStrategy(RowType partitionType) {
45+
public PartitionExpireStrategy(RowType partitionType, String partitionDefaultName) {
4546
this.toObjectArrayConverter = new RowDataToObjectArrayConverter(partitionType);
4647
this.partitionKeys = partitionType.getFieldNames();
48+
this.partitionDefaultName = partitionDefaultName;
4749
}
4850

4951
public Map<String, String> toPartitionString(Object[] array) {
@@ -57,7 +59,11 @@ public Map<String, String> toPartitionString(Object[] array) {
5759
public List<String> toPartitionValue(Object[] array) {
5860
List<String> list = new ArrayList<>(partitionKeys.size());
5961
for (int i = 0; i < partitionKeys.size(); i++) {
60-
list.add(array[i].toString());
62+
if (array[i] != null) {
63+
list.add(array[i].toString());
64+
} else {
65+
list.add(partitionDefaultName);
66+
}
6167
}
6268
return list;
6369
}
@@ -76,13 +82,13 @@ public static PartitionExpireStrategy createPartitionExpireStrategy(
7682
@Nullable Identifier identifier) {
7783
switch (options.partitionExpireStrategy()) {
7884
case UPDATE_TIME:
79-
return new PartitionUpdateTimeExpireStrategy(partitionType);
85+
return new PartitionUpdateTimeExpireStrategy(options, partitionType);
8086
case VALUES_TIME:
8187
return new PartitionValuesTimeExpireStrategy(options, partitionType);
8288
case CUSTOM:
8389
return PartitionExpireStrategyFactory.INSTANCE
8490
.get()
85-
.create(catalogLoader, identifier, partitionType);
91+
.create(catalogLoader, identifier, options, partitionType);
8692
default:
8793
throw new IllegalArgumentException(
8894
"Unknown partitionExpireStrategy: " + options.partitionExpireStrategy());

paimon-core/src/main/java/org/apache/paimon/partition/PartitionExpireStrategyFactory.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
package org.apache.paimon.partition;
2020

21+
import org.apache.paimon.CoreOptions;
2122
import org.apache.paimon.catalog.CatalogLoader;
2223
import org.apache.paimon.catalog.Identifier;
2324
import org.apache.paimon.factories.FactoryUtil;
@@ -30,7 +31,10 @@
3031
public interface PartitionExpireStrategyFactory {
3132

3233
PartitionExpireStrategy create(
33-
CatalogLoader catalogLoader, Identifier identifier, RowType partitionType);
34+
CatalogLoader catalogLoader,
35+
Identifier identifier,
36+
CoreOptions options,
37+
RowType partitionType);
3438

3539
Supplier<PartitionExpireStrategyFactory> INSTANCE =
3640
Suppliers.memoize(

paimon-core/src/main/java/org/apache/paimon/partition/PartitionUpdateTimeExpireStrategy.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
package org.apache.paimon.partition;
2020

21+
import org.apache.paimon.CoreOptions;
2122
import org.apache.paimon.manifest.PartitionEntry;
2223
import org.apache.paimon.operation.FileStoreScan;
2324
import org.apache.paimon.types.RowType;
@@ -33,8 +34,8 @@
3334
*/
3435
public class PartitionUpdateTimeExpireStrategy extends PartitionExpireStrategy {
3536

36-
public PartitionUpdateTimeExpireStrategy(RowType partitionType) {
37-
super(partitionType);
37+
public PartitionUpdateTimeExpireStrategy(CoreOptions options, RowType partitionType) {
38+
super(partitionType, options.partitionDefaultName());
3839
}
3940

4041
@Override

paimon-core/src/main/java/org/apache/paimon/partition/PartitionValuesTimeExpireStrategy.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public class PartitionValuesTimeExpireStrategy extends PartitionExpireStrategy {
4848
private final PartitionTimeExtractor timeExtractor;
4949

5050
public PartitionValuesTimeExpireStrategy(CoreOptions options, RowType partitionType) {
51-
super(partitionType);
51+
super(partitionType, options.partitionDefaultName());
5252
String timePattern = options.partitionTimestampPattern();
5353
String timeFormatter = options.partitionTimestampFormatter();
5454
this.timeExtractor = new PartitionTimeExtractor(timePattern, timeFormatter);

paimon-core/src/test/java/org/apache/paimon/operation/PartitionExpireTest.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
import java.time.format.DateTimeFormatter;
6262
import java.time.temporal.ChronoUnit;
6363
import java.util.ArrayList;
64+
import java.util.Arrays;
6465
import java.util.Collections;
6566
import java.util.HashMap;
6667
import java.util.LinkedHashMap;
@@ -217,6 +218,34 @@ public void testBatchExpire() throws Exception {
217218
assertThat(overwriteSnapshotCnt).isEqualTo(3L);
218219
}
219220

221+
@Test
222+
public void testExpireWithNullOrEmptyPartition() throws Exception {
223+
SchemaManager schemaManager = new SchemaManager(LocalFileIO.create(), path);
224+
schemaManager.createTable(
225+
new Schema(
226+
RowType.of(VarCharType.STRING_TYPE, VarCharType.STRING_TYPE).getFields(),
227+
Arrays.asList("f0", "f1"),
228+
emptyList(),
229+
Collections.singletonMap(METASTORE_PARTITIONED_TABLE.key(), "true"),
230+
""));
231+
newTable();
232+
write("20230101", "11");
233+
write("20230101", "12");
234+
// sub partition is null
235+
write("20230101", null);
236+
// sub partition is empty string
237+
write("20230103", "");
238+
write("20230103", "32");
239+
write("20230105", "51");
240+
241+
PartitionExpire expire = newExpire();
242+
expire.setLastCheck(date(1));
243+
Assertions.assertDoesNotThrow(() -> expire.expire(date(6), Long.MAX_VALUE));
244+
245+
// null partition and empty string partition should be expired
246+
assertThat(read()).containsExactlyInAnyOrder("20230105:51");
247+
}
248+
220249
@Test
221250
public void test() throws Exception {
222251
SchemaManager schemaManager = new SchemaManager(LocalFileIO.create(), path);

paimon-core/src/test/java/org/apache/paimon/partition/CustomPartitionExpirationFactory.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
package org.apache.paimon.partition;
2020

21+
import org.apache.paimon.CoreOptions;
2122
import org.apache.paimon.catalog.Catalog;
2223
import org.apache.paimon.catalog.CatalogLoader;
2324
import org.apache.paimon.catalog.Identifier;
@@ -38,8 +39,11 @@ public class CustomPartitionExpirationFactory implements PartitionExpireStrategy
3839

3940
@Override
4041
public PartitionExpireStrategy create(
41-
CatalogLoader catalogLoader, Identifier identifier, RowType partitionType) {
42-
return new PartitionExpireStrategy(partitionType) {
42+
CatalogLoader catalogLoader,
43+
Identifier identifier,
44+
CoreOptions options,
45+
RowType partitionType) {
46+
return new PartitionExpireStrategy(partitionType, options.partitionDefaultName()) {
4347
@Override
4448
public List<PartitionEntry> selectExpiredPartitions(
4549
FileStoreScan scan, LocalDateTime expirationTime) {

0 commit comments

Comments
 (0)