Core: add variant type support #11831

aihuaxu · 2024-12-19T22:59:37Z

This is to add some required changes in API and core module for Variant support, including:

Add isVariantType() method for variant type
Add variant support in schema visitors such as AssignIdFreshIds, ReassignIds, PruneColumns, etc. and TypeUtil.
Add test coverage to test out schema visitors, TypeUtil.

Part of: #10392

aihuaxu · 2024-12-19T23:25:51Z

cc @rdblue, @RussellSpitzer, @flyrain and @JonasJ-ap. This is to add the changes in core to support variant type.

api/src/main/java/org/apache/iceberg/transforms/Identity.java

api/src/main/java/org/apache/iceberg/types/TypeUtil.java

core/src/main/java/org/apache/iceberg/SchemaParser.java

core/src/test/java/org/apache/iceberg/TestMetadataUpdateParser.java

core/src/test/java/org/apache/iceberg/avro/TestAvroSchemaProjection.java

shohamyamin · 2025-01-04T22:13:28Z

@aihuaxu very important feature that will allow a lot more options for iceberg, thank you for your contribution

api/src/main/java/org/apache/iceberg/types/Types.java

api/src/test/java/org/apache/iceberg/types/TestTypeUtil.java

core/src/main/java/org/apache/iceberg/avro/BuildAvroProjection.java

core/src/main/java/org/apache/iceberg/avro/TypeToSchema.java

core/src/test/java/org/apache/iceberg/TestMetadataUpdateParser.java

core/src/test/java/org/apache/iceberg/avro/TestBuildAvroProjection.java

api/src/test/java/org/apache/iceberg/types/TestTypeUtil.java

api/src/main/java/org/apache/iceberg/types/Types.java

core/src/main/java/org/apache/iceberg/SchemaParser.java

core/src/test/java/org/apache/iceberg/TestSchemaUnionByFieldName.java

core/src/test/java/org/apache/iceberg/avro/TestBuildAvroProjection.java

core/src/test/java/org/apache/iceberg/avro/TestSchemaConversions.java

data/src/test/java/org/apache/iceberg/data/avro/TestGenericData.java

api/src/main/java/org/apache/iceberg/types/TypeUtil.java

api/src/main/java/org/apache/iceberg/types/Types.java

api/src/main/java/org/apache/iceberg/types/TypeUtil.java

api/src/test/java/org/apache/iceberg/types/TestTypeUtil.java

api/src/test/java/org/apache/iceberg/types/TestTypes.java

rdblue · 2025-02-13T01:02:42Z

api/src/test/java/org/apache/iceberg/types/TestTypeUtil.java

+        new Schema(required(1, "v", testType), required(2, "A", Types.IntegerType.get()));
+
+    Schema assignedSchema =
+        TypeUtil.assignFreshIds(sourceSchema, new AtomicInteger(10)::incrementAndGet);


This doesn't use sourceSchema as intended. ID assignment with a source schema uses assigned IDs from that schema when names match and assigns missing/new IDs using the NextID supplier. When using the single schema version, the second schema should not be present.

That also raises the question about what we need to test for new types. In this case, because the type just needs to be handled by field and struct, I think it's fine to just have a case that validates that UnsupportedOperationException is not thrown and that the behavior is expected. That is, we only need on test with a variant here. Other tests with more logic for primitives may need more cases though.

You are right. I just need one schema. I may not get the following: seems we can test for unknown and other primitive types here as well to make sure the correct type is returned, not null.

we only need on test with a variant here. Other tests with more logic for primitives may need more cases though.

api/src/test/java/org/apache/iceberg/types/TestTypeUtil.java

rdblue · 2025-02-13T20:13:55Z

core/src/main/java/org/apache/iceberg/avro/TypeToSchema.java

@@ -187,6 +187,18 @@ public Schema map(Types.MapType map, Schema keySchema, Schema valueSchema) {
    return mapSchema;
  }

+  @Override
+  public Schema variant(Types.VariantType variant) {


I think this is included (and tested) in #12238. Can you remove it from here?

core/src/main/java/org/apache/iceberg/schema/SchemaWithPartnerVisitor.java

core/src/test/java/org/apache/iceberg/avro/TestSchemaConversions.java

rdblue · 2025-02-13T20:51:59Z

api/src/main/java/org/apache/iceberg/types/TypeUtil.java

@@ -712,6 +712,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");


Looks like this is not yet implemented by:

CheckCompatibility (tested by TestReadabilityChecks)

FixupTypes, SparkFixupTypes, or FlinkFixupTypes

PruneColumnsWithoutReordering (Spark, so maybe this can be done separately)

ReassignDoc

AssignIds

It is also not implemented by PruneColumnsWithReordering but that doesn't appear to be referenced anywhere. I opened #12258 to remove these.

rdblue · 2025-02-13T21:00:48Z

api/src/main/java/org/apache/iceberg/types/TypeUtil.java

-    public T variant() {
-      return null;
+    public T variant(Types.VariantType variant) {
+      throw new UnsupportedOperationException("Unsupported type: variant");


This still need to be implemented by:

CreateMapping

DescribeSchemaVisitor

IdToOrcName and other conversions

I think it's fine to implement the conversions later as part of Avro, Parquet, and ORC work, but the common ones should probably be implemented.

github-actions bot added API core labels Dec 19, 2024

aihuaxu marked this pull request as ready for review December 19, 2024 23:00

aihuaxu force-pushed the variant-type-core branch from 5a18496 to 68d690e Compare December 19, 2024 23:20

ebyhr mentioned this pull request Dec 20, 2024

Add support for variant type in Iceberg trinodb/trino#24538

Open

rdblue reviewed Dec 20, 2024

View reviewed changes

api/src/main/java/org/apache/iceberg/transforms/Identity.java Outdated Show resolved Hide resolved

rdblue reviewed Dec 21, 2024

View reviewed changes

api/src/main/java/org/apache/iceberg/types/TypeUtil.java Outdated Show resolved Hide resolved

rdblue reviewed Dec 21, 2024

View reviewed changes

core/src/main/java/org/apache/iceberg/SchemaParser.java Show resolved Hide resolved

rdblue reviewed Dec 21, 2024

View reviewed changes

core/src/main/java/org/apache/iceberg/SchemaParser.java Outdated Show resolved Hide resolved

rdblue reviewed Dec 21, 2024

View reviewed changes

core/src/main/java/org/apache/iceberg/SchemaParser.java Outdated Show resolved Hide resolved

rdblue reviewed Dec 21, 2024

View reviewed changes

core/src/test/java/org/apache/iceberg/TestMetadataUpdateParser.java Outdated Show resolved Hide resolved

rdblue reviewed Dec 21, 2024

View reviewed changes

core/src/test/java/org/apache/iceberg/avro/TestAvroSchemaProjection.java Outdated Show resolved Hide resolved

aihuaxu force-pushed the variant-type-core branch from 68d690e to 04958ca Compare December 21, 2024 04:43

aihuaxu requested a review from rdblue December 21, 2024 04:45

aihuaxu force-pushed the variant-type-core branch 2 times, most recently from b276d3f to fe6038a Compare December 21, 2024 16:42

rdblue reviewed Jan 24, 2025

View reviewed changes

api/src/main/java/org/apache/iceberg/types/Types.java Outdated Show resolved Hide resolved

rdblue reviewed Jan 24, 2025

View reviewed changes

api/src/test/java/org/apache/iceberg/types/TestTypeUtil.java Outdated Show resolved Hide resolved

rdblue reviewed Jan 24, 2025

View reviewed changes

api/src/test/java/org/apache/iceberg/types/TestTypeUtil.java Outdated Show resolved Hide resolved

rdblue reviewed Jan 24, 2025

View reviewed changes

api/src/test/java/org/apache/iceberg/types/TestTypeUtil.java Outdated Show resolved Hide resolved

rdblue reviewed Jan 24, 2025

View reviewed changes

api/src/test/java/org/apache/iceberg/types/TestTypeUtil.java Outdated Show resolved Hide resolved

rdblue reviewed Jan 24, 2025

View reviewed changes

core/src/main/java/org/apache/iceberg/avro/BuildAvroProjection.java Outdated Show resolved Hide resolved

rdblue reviewed Jan 24, 2025

View reviewed changes

core/src/main/java/org/apache/iceberg/avro/TypeToSchema.java Outdated Show resolved Hide resolved

rdblue reviewed Jan 24, 2025

View reviewed changes

core/src/test/java/org/apache/iceberg/TestMetadataUpdateParser.java Outdated Show resolved Hide resolved

rdblue reviewed Jan 24, 2025

View reviewed changes

core/src/test/java/org/apache/iceberg/avro/TestBuildAvroProjection.java Outdated Show resolved Hide resolved

aihuaxu force-pushed the variant-type-core branch 3 times, most recently from e0a430f to 1b56bf5 Compare January 30, 2025 05:12

rdblue reviewed Feb 5, 2025

View reviewed changes

api/src/test/java/org/apache/iceberg/types/TestTypeUtil.java Outdated Show resolved Hide resolved

rdblue reviewed Feb 5, 2025

View reviewed changes

api/src/main/java/org/apache/iceberg/types/Types.java Outdated Show resolved Hide resolved

rdblue reviewed Feb 5, 2025

View reviewed changes

core/src/main/java/org/apache/iceberg/SchemaParser.java Show resolved Hide resolved

rdblue reviewed Feb 5, 2025

View reviewed changes

core/src/test/java/org/apache/iceberg/TestSchemaUnionByFieldName.java Outdated Show resolved Hide resolved

rdblue reviewed Feb 5, 2025

View reviewed changes

core/src/test/java/org/apache/iceberg/avro/TestBuildAvroProjection.java Outdated Show resolved Hide resolved

rdblue reviewed Feb 5, 2025

View reviewed changes

core/src/test/java/org/apache/iceberg/avro/TestSchemaConversions.java Outdated Show resolved Hide resolved

rdblue reviewed Feb 5, 2025

View reviewed changes

data/src/test/java/org/apache/iceberg/data/avro/TestGenericData.java Outdated Show resolved Hide resolved

Update tests

f53769b

aihuaxu requested a review from rdblue February 10, 2025 17:36