Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Core: add variant type support #11831

Merged
merged 11 commits into from
Feb 18, 2025
5 changes: 5 additions & 0 deletions api/src/main/java/org/apache/iceberg/Accessors.java
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,11 @@ public Map<Integer, Accessor<StructLike>> struct(
return accessors;
}

@Override
public Map<Integer, Accessor<StructLike>> variant(Types.VariantType variant) {
return null;
}

@Override
public Map<Integer, Accessor<StructLike>> field(
Types.NestedField field, Map<Integer, Accessor<StructLike>> fieldResult) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,11 @@ public Type map(Types.MapType map, Supplier<Type> keyFuture, Supplier<Type> valu
}
}

@Override
public Type variant(Types.VariantType variant) {
return variant;
}

@Override
public Type primitive(Type.PrimitiveType primitive) {
return primitive;
Expand Down
5 changes: 5 additions & 0 deletions api/src/main/java/org/apache/iceberg/types/AssignIds.java
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ public Type map(Types.MapType map, Supplier<Type> keyFuture, Supplier<Type> valu
}
}

@Override
public Type variant(Types.VariantType variant) {
return variant;
}

@Override
public Type primitive(Type.PrimitiveType primitive) {
return primitive;
Expand Down
13 changes: 13 additions & 0 deletions api/src/main/java/org/apache/iceberg/types/CheckCompatibility.java
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,19 @@ public List<String> map(
}
}

@Override
public List<String> variant(Types.VariantType readVariant) {
if (currentType.isVariantType()) {
return NO_ERRORS;
}

// Currently promotion is not allowed to variant type
return ImmutableList.of(
String.format(
": %s cannot be read as a %s",
currentType.typeId().toString().toLowerCase(Locale.ENGLISH), readVariant));
}

@Override
public List<String> primitive(Type.PrimitiveType readPrimitive) {
if (currentType.equals(readPrimitive)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,9 @@ public Type map(Types.MapType map, Type keyResult, Type valueResult) {
}

@Override
public Type variant() {
if (predicate.test(Types.VariantType.get())) {
return Types.VariantType.get();
public Type variant(Types.VariantType variant) {
if (predicate.test(variant)) {
return variant;
}

return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ public Set<Integer> struct(Types.StructType struct, List<Set<Integer>> fieldResu

@Override
public Set<Integer> field(Types.NestedField field, Set<Integer> fieldResult) {
if ((includeStructIds && field.type().isStructType()) || field.type().isPrimitiveType()) {
if ((includeStructIds && field.type().isStructType())
|| field.type().isPrimitiveType()
|| field.type().isVariantType()) {
fieldIds.add(field.fieldId());
}
return fieldIds;
Expand All @@ -72,4 +74,9 @@ public Set<Integer> map(Types.MapType map, Set<Integer> keyResult, Set<Integer>
}
return fieldIds;
}

@Override
public Set<Integer> variant(Types.VariantType variant) {
return null;
}
}
5 changes: 5 additions & 0 deletions api/src/main/java/org/apache/iceberg/types/IndexById.java
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,9 @@ public Map<Integer, Types.NestedField> map(
}
return null;
}

@Override
public Map<Integer, Types.NestedField> variant(Types.VariantType variant) {
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ public Map<String, Integer> map(
}

@Override
public Map<String, Integer> variant() {
public Map<String, Integer> variant(Types.VariantType variant) {
return nameToId;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public Map<Integer, Integer> map(
}

@Override
public Map<Integer, Integer> variant() {
public Map<Integer, Integer> variant(Types.VariantType variant) {
return idToParent;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@ class PrimitiveHolder implements Serializable {
}

Object readResolve() throws ObjectStreamException {
return Types.fromPrimitiveString(typeAsString);
return Types.fromTypeName(typeAsString);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is tested by TestSerializableTypes. Can you add new cases for variant (and the other new types, while you're updating it)?

Copy link
Contributor Author

@aihuaxu aihuaxu Feb 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new type tests have been added in TestSerializableTypes. But let me refactor a little bit for testUnknown() which should be tested with isSameAs(), same as other primitive types. testVariant() is testing with isEqualTo().

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the test for this is missing variant.

}
}
5 changes: 5 additions & 0 deletions api/src/main/java/org/apache/iceberg/types/PruneColumns.java
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,11 @@ public Type map(Types.MapType map, Type ignored, Type valueResult) {
return null;
}

@Override
public Type variant(Types.VariantType variant) {
return null;
}

@Override
public Type primitive(Type.PrimitiveType primitive) {
return null;
Expand Down
5 changes: 5 additions & 0 deletions api/src/main/java/org/apache/iceberg/types/ReassignDoc.java
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ public Type map(Types.MapType map, Supplier<Type> keyTypeFuture, Supplier<Type>
}
}

@Override
public Type variant(Types.VariantType variant) {
return variant;
}

@Override
public Type primitive(Type.PrimitiveType primitive) {
return primitive;
Expand Down
5 changes: 5 additions & 0 deletions api/src/main/java/org/apache/iceberg/types/ReassignIds.java
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@ public Type map(Types.MapType map, Supplier<Type> keyTypeFuture, Supplier<Type>
}
}

@Override
public Type variant(Types.VariantType variant) {
return variant;
}

@Override
public Type primitive(Type.PrimitiveType primitive) {
return primitive; // nothing to reassign
Expand Down
8 changes: 8 additions & 0 deletions api/src/main/java/org/apache/iceberg/types/Type.java
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ default Types.MapType asMapType() {
throw new IllegalArgumentException("Not a map type: " + this);
}

default Types.VariantType asVariantType() {
throw new IllegalArgumentException("Not a variant type: " + this);
}

default boolean isNestedType() {
return false;
}
Expand All @@ -98,6 +102,10 @@ default boolean isMapType() {
return false;
}

default boolean isVariantType() {
return false;
}

default NestedType asNestedType() {
throw new IllegalArgumentException("Not a nested type: " + this);
}
Expand Down
19 changes: 17 additions & 2 deletions api/src/main/java/org/apache/iceberg/types/TypeUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -616,8 +616,16 @@ public T map(Types.MapType map, T keyResult, T valueResult) {
return null;
}

/**
* @deprecated will be removed in 2.0.0; use {@link #variant(Types.VariantType)} instead.
*/
@Deprecated
public T variant() {
return null;
return variant(Types.VariantType.get());
}

public T variant(Types.VariantType variant) {
throw new UnsupportedOperationException("Unsupported type: variant");
}

public T primitive(Type.PrimitiveType primitive) {
Expand Down Expand Up @@ -684,7 +692,7 @@ public static <T> T visit(Type type, SchemaVisitor<T> visitor) {
return visitor.map(map, keyResult, valueResult);

case VARIANT:
return visitor.variant();
return visitor.variant(type.asVariantType());

default:
return visitor.primitive(type.asPrimitiveType());
Expand Down Expand Up @@ -712,6 +720,10 @@ public T map(Types.MapType map, Supplier<T> keyResult, Supplier<T> valueResult)
return null;
}

public T variant(Types.VariantType variant) {
throw new UnsupportedOperationException("Unsupported type: variant");
}

public T primitive(Type.PrimitiveType primitive) {
return null;
}
Expand Down Expand Up @@ -788,6 +800,9 @@ public static <T> T visit(Type type, CustomOrderSchemaVisitor<T> visitor) {
new VisitFuture<>(map.keyType(), visitor),
new VisitFuture<>(map.valueType(), visitor));

case VARIANT:
return visitor.variant(type.asVariantType());

default:
return visitor.primitive(type.asPrimitiveType());
}
Expand Down
26 changes: 23 additions & 3 deletions api/src/main/java/org/apache/iceberg/types/Types.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ public class Types {

private Types() {}

private static final ImmutableMap<String, PrimitiveType> TYPES =
ImmutableMap.<String, PrimitiveType>builder()
private static final ImmutableMap<String, Type> TYPES =
ImmutableMap.<String, Type>builder()
.put(BooleanType.get().toString(), BooleanType.get())
.put(IntegerType.get().toString(), IntegerType.get())
.put(LongType.get().toString(), LongType.get())
Expand All @@ -56,13 +56,14 @@ private Types() {}
.put(UUIDType.get().toString(), UUIDType.get())
.put(BinaryType.get().toString(), BinaryType.get())
.put(UnknownType.get().toString(), UnknownType.get())
.put(VariantType.get().toString(), VariantType.get())
.buildOrThrow();

private static final Pattern FIXED = Pattern.compile("fixed\\[\\s*(\\d+)\\s*\\]");
private static final Pattern DECIMAL =
Pattern.compile("decimal\\(\\s*(\\d+)\\s*,\\s*(\\d+)\\s*\\)");

public static PrimitiveType fromPrimitiveString(String typeString) {
public static Type fromTypeName(String typeString) {
String lowerTypeString = typeString.toLowerCase(Locale.ROOT);
if (TYPES.containsKey(lowerTypeString)) {
return TYPES.get(lowerTypeString);
Expand All @@ -81,6 +82,15 @@ public static PrimitiveType fromPrimitiveString(String typeString) {
throw new IllegalArgumentException("Cannot parse type string to primitive: " + typeString);
}

public static PrimitiveType fromPrimitiveString(String typeString) {
Type type = fromTypeName(typeString);
if (type.isPrimitiveType()) {
return type.asPrimitiveType();
}

throw new IllegalArgumentException("Cannot parse type string: variant is not a primitive type");
}

public static class BooleanType extends PrimitiveType {
private static final BooleanType INSTANCE = new BooleanType();

Expand Down Expand Up @@ -430,6 +440,16 @@ public String toString() {
return "variant";
}

@Override
public boolean isVariantType() {
return true;
}

@Override
public VariantType asVariantType() {
return this;
}

@Override
public boolean equals(Object o) {
if (this == o) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import static org.apache.iceberg.types.Types.NestedField.required;
import static org.assertj.core.api.Assertions.assertThat;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import org.apache.iceberg.Schema;
import org.apache.iceberg.types.Type.PrimitiveType;
Expand Down Expand Up @@ -112,6 +114,34 @@ private void testDisallowPrimitiveToStruct(PrimitiveType from, Schema fromSchema
.contains("cannot be read as a struct");
}

@Test
public void testVariantType() {
Schema fromSchema = new Schema(required(1, "from_field", Types.VariantType.get()));
List<String> errors =
CheckCompatibility.writeCompatibilityErrors(
new Schema(required(1, "to_field", Types.VariantType.get())), fromSchema);
assertThat(errors).as("Should produce 0 error messages").isEmpty();

List<Type> incompatibleTypes = new ArrayList<>();
incompatibleTypes.addAll(
List.of(
Types.StructType.of(required(1, "from", Types.IntegerType.get())),
Types.MapType.ofRequired(1, 2, Types.StringType.get(), Types.IntegerType.get()),
Types.ListType.ofRequired(1, Types.StringType.get())));
incompatibleTypes.addAll(Arrays.asList(PRIMITIVES));

for (Type from : incompatibleTypes) {
fromSchema = new Schema(required(3, "from_field", from));
errors =
CheckCompatibility.writeCompatibilityErrors(
new Schema(required(3, "to_field", Types.VariantType.get())), fromSchema);
assertThat(errors).hasSize(1);
assertThat(errors.get(0))
.as("Should complain that other type to variant is not allowed")
.contains("cannot be read as a variant");
}
}

@Test
public void testRequiredSchemaField() {
Schema write = new Schema(optional(1, "from_field", Types.IntegerType.get()));
Expand Down
Loading
Loading