Core: Update variant class visibility

[rdblue]

This updates variants with changes needed for Parquet readers.

• Add numFields to objects, numElements to arrays, and dictionarySize to metadata
• Use VariantMetadata instead of direct references to SerializedMetadata
• Support any VariantObject for unshredded fields in ShreddedObject to avoid leaking SerializedObject
• Support suppressing fields in ShreddedObject to handle shredded fields that are missing, not overridden
• For boolean variants, alter the type based on the value to simplify creating booleans
• Rename Variants.from

[aihuaxu]

nit: seems I prefer the existing implementation which is cleaner and consistent with other types.

[rdblue]

The trade-off is that this would require a separate reader just for boolean values. I'm open to that if there are strong objections, but it seems to me that allowing the actual type to be fixed up depending on the value is a reasonable change that saves quite a bit of code. If you object to this, I can revert it and add the boolean-specific reader.

[aihuaxu]

Yeah. You are right. BOOLEAN_TRUE and BOOLEAN_FALSE physical types need special handling. When we shred them, they should be grouped together, considered as one type.

And also we may need to have a type list {NULL, BOOLEAN, INT8, INT16, etc}, which is almost same as physical type list with BOOLEAN for BOOLEAN_TRUE and BOOLEAN_FALSE. Otherwise, we can't represent a shredded column type for true/false.

I'm fine to keep it simple for now. We can revisit if needed.

[aihuaxu]

Thanks for addressing it. I was trying to make Parquet change in (https://github.com/apache/iceberg/pull/11653/files#diff-b8e8443fcec3843e538dbc702d4c131ff58359cb83ccdb211d8679c1d77c16bd) and we need to expose this.

[rdblue]

I needed it for the readers and for the updates here to ShreddedObject, too.

[aihuaxu]

What does this remove() try to support or to be used?

[rdblue]

The purpose of this class is to create objects from an unshredded, serialized variant in value and the fields in its corresponding typed_value. The serialized object is used to construct the ShreddedObject instance and then the shredded fields are set through put.

This is intended to handle fields that are "missing" because the field's value and typed_value are null. In those cases, we need to basically add a null value to the shreddedFields map. We could do that, but the map implementations that we use (from Guava) don't allow null values. Even if we used a map that could handle null, we would have to handle those nulls in places like nameSet and in serialization. That way we correctly store that the field was missing according to the shredding spec, rather than defined and equal to a Variant null.

I thought it was cleaner to handle missing fields by calling remove for the field name to show that it is not present in the shredded fields. I also think that using a separate set of field names makes the most sense for handling these instead of using null as a sentinel value in the shreddedFields map.

[aihuaxu]

Thanks for explanation.

If a field is missing and we remove the field from shreddedFields, why do we still need removedFields to keep track of it? Would the following get the correct field list?

  private Set<String> nameSet() {
    Set<String> names = Sets.newHashSet(shreddedFields.keySet());

    if (unshredded != null) {
      Iterables.addAll(names, unshredded.fieldNames());
    }

    return names;
  }

[rdblue]

That doesn't handle the case where unshredded incorrectly includes the field. We need to keep track of the shredded fields, whether present or missing, so that the shredded fields are always used.

[amogh-jahagirdar]

Thanks @rdblue , and @aihuaxu for reviewing!