Skip to content

Commit

Permalink
Core: Fix null partitions in PartitionSet (#9248)
Browse files Browse the repository at this point in the history
  • Loading branch information
aokolnychyi authored Dec 8, 2023
1 parent 504c134 commit 62a23a3
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 2 deletions.
4 changes: 2 additions & 2 deletions core/src/main/java/org/apache/iceberg/util/PartitionSet.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public boolean contains(Object o) {
if (o instanceof Pair) {
Object first = ((Pair<?, ?>) o).first();
Object second = ((Pair<?, ?>) o).second();
if (first instanceof Integer && second instanceof StructLike) {
if (first instanceof Integer && (second == null || second instanceof StructLike)) {
return contains((Integer) first, (StructLike) second);
}
}
Expand Down Expand Up @@ -98,7 +98,7 @@ public boolean remove(Object o) {
if (o instanceof Pair) {
Object first = ((Pair<?, ?>) o).first();
Object second = ((Pair<?, ?>) o).second();
if (first instanceof Integer && second instanceof StructLike) {
if (first instanceof Integer && (second == null || second instanceof StructLike)) {
return remove((Integer) first, (StructLike) second);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,9 +274,11 @@ public void testKeyAndEntrySetEquality() {

map1.put(BY_DATA_SPEC.specId(), Row.of("aaa"), "v1");
map1.put(BY_DATA_SPEC.specId(), Row.of("bbb"), "v2");
map1.put(UNPARTITIONED_SPEC.specId(), null, "v3");

map2.put(BY_DATA_SPEC.specId(), CustomRow.of("aaa"), "v1");
map2.put(BY_DATA_SPEC.specId(), CustomRow.of("bbb"), "v2");
map2.put(UNPARTITIONED_SPEC.specId(), null, "v3");

assertThat(map1.keySet()).isEqualTo(map2.keySet());
assertThat(map1.entrySet()).isEqualTo(map2.entrySet());
Expand Down
83 changes: 83 additions & 0 deletions core/src/test/java/org/apache/iceberg/util/TestPartitionSet.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
* 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.
*/
package org.apache.iceberg.util;

import static org.apache.iceberg.types.Types.NestedField.required;
import static org.assertj.core.api.Assertions.assertThat;

import java.util.Map;
import org.apache.iceberg.PartitionSpec;
import org.apache.iceberg.Schema;
import org.apache.iceberg.TestHelpers.CustomRow;
import org.apache.iceberg.TestHelpers.Row;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
import org.apache.iceberg.types.Types;
import org.junit.jupiter.api.Test;

public class TestPartitionSet {
private static final Schema SCHEMA =
new Schema(
required(1, "id", Types.IntegerType.get()),
required(2, "data", Types.StringType.get()),
required(3, "category", Types.StringType.get()));
private static final PartitionSpec UNPARTITIONED_SPEC = PartitionSpec.unpartitioned();
private static final PartitionSpec BY_DATA_SPEC =
PartitionSpec.builderFor(SCHEMA).identity("data").withSpecId(1).build();
private static final PartitionSpec BY_DATA_CATEGORY_BUCKET_SPEC =
PartitionSpec.builderFor(SCHEMA).identity("data").bucket("category", 8).withSpecId(3).build();
private static final Map<Integer, PartitionSpec> SPECS =
ImmutableMap.of(
UNPARTITIONED_SPEC.specId(),
UNPARTITIONED_SPEC,
BY_DATA_SPEC.specId(),
BY_DATA_SPEC,
BY_DATA_CATEGORY_BUCKET_SPEC.specId(),
BY_DATA_CATEGORY_BUCKET_SPEC);

@Test
public void testGet() {
PartitionSet set = PartitionSet.create(SPECS);
set.add(BY_DATA_SPEC.specId(), Row.of("a"));
set.add(UNPARTITIONED_SPEC.specId(), null);
set.add(UNPARTITIONED_SPEC.specId(), Row.of());
set.add(BY_DATA_CATEGORY_BUCKET_SPEC.specId(), CustomRow.of("a", 1));

assertThat(set).hasSize(4);
assertThat(set.contains(BY_DATA_SPEC.specId(), CustomRow.of("a"))).isTrue();
assertThat(set.contains(UNPARTITIONED_SPEC.specId(), null)).isTrue();
assertThat(set.contains(UNPARTITIONED_SPEC.specId(), CustomRow.of())).isTrue();
assertThat(set.contains(BY_DATA_CATEGORY_BUCKET_SPEC.specId(), Row.of("a", 1))).isTrue();
}

@Test
public void testRemove() {
PartitionSet set = PartitionSet.create(SPECS);
set.add(BY_DATA_SPEC.specId(), Row.of("a"));
set.add(UNPARTITIONED_SPEC.specId(), null);
set.add(UNPARTITIONED_SPEC.specId(), Row.of());
set.add(BY_DATA_CATEGORY_BUCKET_SPEC.specId(), CustomRow.of("a", 1));

assertThat(set).hasSize(4);
assertThat(set.remove(BY_DATA_SPEC.specId(), CustomRow.of("a"))).isTrue();
assertThat(set.remove(UNPARTITIONED_SPEC.specId(), null)).isTrue();
assertThat(set.remove(UNPARTITIONED_SPEC.specId(), CustomRow.of())).isTrue();
assertThat(set.remove(BY_DATA_CATEGORY_BUCKET_SPEC.specId(), Row.of("a", 1))).isTrue();
assertThat(set).isEmpty();
}
}

0 comments on commit 62a23a3

Please sign in to comment.