Code review feedback

Author: cashmand

parquet-avro/src/main/java/org/apache/parquet/avro/AvroWriteSupport.java

@@ -196,7 +196,7 @@ private void writeRecord(GroupType schema, Schema avroSchema, Object record) {
   private void writeVariantFields(GroupType schema, Schema avroSchema, Object record) {
     List<Type> fields = schema.getFields();
     List<Schema.Field> avroFields = avroSchema.getFields();
-    // Check if the avro schema is a value/metadata pair. If not, just read the Variant as if it was a normal record.
+    // Check if the avro schema is a value/metadata pair. If not, treat the value as a non-variant record.
     boolean binarySchema = true;
     int valueIndex = -1;
     int metadataIndex = -1;

parquet-avro/src/test/java/org/apache/parquet/avro/TestWriteVariant.java

@@ -79,7 +79,7 @@ private static ByteBuffer variant(String s) {
     return variant(b -> b.appendString(s));
   }
 
-  private static GroupType variantGroup = Types.buildGroup(Type.Repetition.REQUIRED)
+  private static final GroupType UNSHREDDED_GROUP = Types.buildGroup(Type.Repetition.REQUIRED)
       .as(LogicalTypeAnnotation.variantType((byte) 1))
       .required(PrimitiveTypeName.BINARY)
       .named("metadata")
@@ -95,10 +95,10 @@ private static MessageType parquetSchema(GroupType variantGroup) {
         .named("table");
   }
 
-  private static MessageType readSchema = parquetSchema(variantGroup);
+  private static final MessageType READ_SCHEMA = parquetSchema(UNSHREDDED_GROUP);
 
-  private static final Schema VARIANT_SCHEMA = new AvroSchemaConverter().convert(variantGroup);
-  private static final Schema SCHEMA = new AvroSchemaConverter().convert(readSchema);
+  private static final Schema VARIANT_SCHEMA = new AvroSchemaConverter().convert(UNSHREDDED_GROUP);
+  private static final Schema SCHEMA = new AvroSchemaConverter().convert(READ_SCHEMA);
 
   private ByteBuffer TEST_METADATA;
   private ByteBuffer TEST_OBJECT;
@@ -249,7 +249,7 @@ GenericRecord createRecord(int i, Variant v) {
   public void testUnshreddedValues() throws IOException {
     for (Variant v : VARIANTS) {
       GenericRecord record = createRecord(1, v);
-      TestSchema testSchema = new TestSchema(readSchema, readSchema);
+      TestSchema testSchema = new TestSchema(READ_SCHEMA, READ_SCHEMA);
 
       GenericRecord actual = writeAndRead(testSchema, record);
 
@@ -264,7 +264,7 @@ public void testShreddedValues() throws IOException {
     for (Variant v : VARIANTS) {
       GenericRecord record = createRecord(1, v);
       MessageType writeSchema = shreddingSchema(v);
-      TestSchema testSchema = new TestSchema(writeSchema, readSchema);
+      TestSchema testSchema = new TestSchema(writeSchema, READ_SCHEMA);
 
       GenericRecord actual = writeAndRead(testSchema, record);
       assertEquals(record.get(0), actual.get(0));
@@ -283,7 +283,7 @@ public void testMixedShredding() throws IOException {
       }
 
       MessageType writeSchema = shreddingSchema(valueForSchema);
-      TestSchema testSchema = new TestSchema(writeSchema, readSchema);
+      TestSchema testSchema = new TestSchema(writeSchema, READ_SCHEMA);
 
       List<GenericRecord> actual = writeAndRead(testSchema, expected);
       assertEquals(actual.size(), VARIANTS.length);