changes

Author: richardc-db

kernel/kernel-api/src/main/java/io/delta/kernel/internal/data/GenericRow.java

@@ -137,7 +137,6 @@ public MapValue getMap(int ordinal) {
 
     @Override
     public VariantValue getVariant(int ordinal) {
-        // TODO(r.chen): test this path somehow?
         throwIfUnsafeAccess(ordinal, VariantType.class, "variant");
         return (VariantValue) getValue(ordinal);
     }

kernel/kernel-api/src/main/java/io/delta/kernel/types/VariantType.java

@@ -18,9 +18,7 @@
 import io.delta.kernel.annotation.Evolving;
 
 /**
- * A variant type.
- * <p>
- * todo: more comments
+ * A logical variant type.
  * @since 4.0.0
  */
 @Evolving

kernel/kernel-defaults/src/main/java/io/delta/kernel/defaults/internal/data/vector/DefaultVariantVector.java

@@ -37,12 +37,12 @@ public class DefaultVariantVector
      * Create an instance of {@link io.delta.kernel.data.ColumnVector} for array type.
      *
      * @param size          number of elements in the vector.
+     * @param type          {@code variant} datatype definition.
      * @param nullability   Optional array of nullability value for each element in the vector.
      *                      All values in the vector are considered non-null when parameter is
      *                      empty.
-     * @param offsets       Offsets into element vector on where the index of particular row
-     *                      values start and end.
-     * @param elementVector Vector containing the array elements.
+     * @param value         The child binary column vector representing each variant's values.
+     * @param metadata         The child binary column vector representing each variant's metadata.
      */
     public DefaultVariantVector(
         int size,
@@ -84,6 +84,13 @@ public byte[] getMetadata() {
         };
     }
 
+    /**
+     * Get the child column vector at the given {@code ordinal}. Variants should only have two
+     * child vectors, one for value and one for metadata.
+     *
+     * @param ordinal
+     * @return
+     */
     @Override
     public ColumnVector getChild(int ordinal) {
         checkArgument(ordinal >= 0 && ordinal < 2, "Invalid ordinal " + ordinal);

kernel/kernel-defaults/src/main/java/io/delta/kernel/defaults/internal/parquet/VariantConverter.java

@@ -34,10 +34,12 @@ class VariantConverter
     private final BinaryColumnConverter valueConverter;
     private final BinaryColumnConverter metadataConverter;
 
-    // Working state
-    private boolean isCurrentValueNull = true;
+    // working state
     private int currentRowIndex;
     private boolean[] nullability;
+    // If the value is null, start/end never get called which is a signal for null
+    // Set the initial state to true and when start() is called set it to false.
+    private boolean isCurrentValueNull = true;
 
     /**
      * Create converter for {@link VariantType} column.
@@ -46,10 +48,8 @@ class VariantConverter
      */
     VariantConverter(int initialBatchSize) {
         checkArgument(initialBatchSize > 0, "invalid initialBatchSize: %s", initialBatchSize);
-        // Initialize the working state
         this.nullability = ParquetConverters.initNullabilityVector(initialBatchSize);
 
-        int parquetOrdinal = 0;
         this.valueConverter = new BinaryColumnConverter(BinaryType.BINARY, initialBatchSize);
         this.metadataConverter = new BinaryColumnConverter(BinaryType.BINARY, initialBatchSize);
     }

kernel/kernel-defaults/src/test/scala/io/delta/kernel/defaults/internal/parquet/ParquetFileReaderSuite.scala

@@ -193,13 +193,10 @@ class ParquetFileReaderSuite extends AnyFunSuite
         parse_json(cast(id as string)),
         null
       ) as array_v""",
-      "map('test', cast(null as variant)) as map_value_v",
-      "map(cast(null as variant), parse_json(cast(id as string))) as map_key_v",
+      "map('test', cast(null as variant)) as map_value_v"
     )
   }
 
-  // TODO(richardc-db): Add nested variant tests once `parse_json` expression is implemented.
-
   /////////////////////////////////////////////////////////////////////////////////////////////////
   // Test compatibility with Parquet legacy format files                                         //
   /////////////////////////////////////////////////////////////////////////////////////////////////