GH-3039: Keep protobuf oneof fields optional when unwrapping wrappers

parquet-protobuf/src/main/java/org/apache/parquet/proto/ProtoSchemaConverter.java

@@ -149,7 +149,8 @@ public ProtoSchemaConverter(boolean parquetSpecsCompliant, int maxRecursion) {
    * @param maxRecursion          The maximum recursion depth messages are allowed to go before terminating as
    *                              bytes instead of their actual schema.
    * @param unwrapProtoWrappers   If set to true, unwrap common Proto wrappers like Timestamp and DoubleValue
-   *                              with corresponding OPTIONAL logical annotations. Primitive types become REQUIRED.
+   *                              with corresponding OPTIONAL logical annotations. Primitive types become REQUIRED,
+   *                              except oneof members, which remain optional because only one can be set.
    */
   public ProtoSchemaConverter(boolean parquetSpecsCompliant, int maxRecursion, boolean unwrapProtoWrappers) {
     this.parquetSpecsCompliant = parquetSpecsCompliant;
@@ -275,7 +276,11 @@ private <T> Builder<? extends Builder<?, GroupBuilder<T>>, GroupBuilder<T>> addF
       // the old schema style did not include the LIST wrapper around repeated fields
       return addRepeatedPrimitive(parquetType.primitiveType, parquetType.logicalTypeAnnotation, builder);
     }
-    Repetition repetition = unwrapProtoWrappers ? Repetition.REQUIRED : getRepetition(descriptor);
+    // Unwrap mode marks scalars REQUIRED (an unwrapped wrapper value is always present),
+    // but oneof members must stay optional: at most one alternative is written.
+    Repetition repetition = unwrapProtoWrappers && descriptor.getContainingOneof() == null
+        ? Repetition.REQUIRED
+        : getRepetition(descriptor);
     return builder.primitive(parquetType.primitiveType, repetition).as(parquetType.logicalTypeAnnotation);
   }
 

parquet-protobuf/src/test/java/org/apache/parquet/proto/ProtoSchemaConverterTest.java

@@ -393,6 +393,29 @@ public void testProto3ConvertDateTimeMessageUnwrapped() throws Exception {
     testConversion(TestProto3.DateTimeMessage.class, expectedSchema, false, true);
   }
 
+  @Test
+  public void testProto3ConvertOneOf() {
+    String expectedSchema = JOINER.join(
+        "message TestProto3.OneOfTestMessage {",
+        "  optional int32 first = 1;",
+        "  optional int32 second = 2;",
+        "}");
+
+    testConversion(TestProto3.OneOfTestMessage.class, expectedSchema);
+  }
+
+  @Test
+  public void testProto3ConvertOneOfUnwrapped() {
+    String expectedSchema = JOINER.join(
+        "message TestProto3.OneOfTestMessage {",
+        "  optional int32 first = 1;",
+        "  optional int32 second = 2;",
+        "}");
+
+    testConversion(TestProto3.OneOfTestMessage.class, expectedSchema, true, true);
+    testConversion(TestProto3.OneOfTestMessage.class, expectedSchema, false, true);
+  }
+
   @Test
   public void testProto3ConvertWrappedMessageUnwrapped() throws Exception {
     String expectedSchema = "message TestProto3.WrappedMessage {\n" + "  optional double wrappedDouble = 1;\n"

parquet-protobuf/src/test/java/org/apache/parquet/proto/ProtoWriteSupportTest.java

@@ -37,6 +37,7 @@
 import org.apache.parquet.proto.test.TestProtobuf;
 import org.apache.parquet.proto.test.Trees;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
 import org.mockito.InOrder;
 import org.mockito.Mockito;
 
@@ -1051,6 +1052,38 @@ public void testMessageOneOfRoundTrip() throws IOException {
     assertThat(gotBackThird.getTheOneofCase()).isEqualTo(TestProto3.OneOfTestMessage.TheOneofCase.FIRST);
   }
 
+  @Test
+  public void testMessageOneOfUnwrappedRoundTrip(@TempDir java.nio.file.Path tempDir) throws IOException {
+    TestProto3.OneOfTestMessage secondSet =
+        TestProto3.OneOfTestMessage.newBuilder().setSecond(99).build();
+    TestProto3.OneOfTestMessage nothingSet =
+        TestProto3.OneOfTestMessage.newBuilder().build();
+    TestProto3.OneOfTestMessage firstSet =
+        TestProto3.OneOfTestMessage.newBuilder().setFirst(42).build();
+
+    Path tmpFilePath = new Path(tempDir.resolve("oneof-unwrap.parquet").toUri());
+    try (ParquetWriter<MessageOrBuilder> writer = ProtoParquetWriter.<MessageOrBuilder>builder(tmpFilePath)
+        .withMessage(TestProto3.OneOfTestMessage.class)
+        .config(ProtoWriteSupport.PB_UNWRAP_PROTO_WRAPPERS, "true")
+        .withValidation(true)
+        .build()) {
+      writer.write(secondSet);
+      writer.write(nothingSet);
+      writer.write(firstSet);
+    }
+
+    List<TestProto3.OneOfTestMessage> gotBack =
+        TestUtils.readMessages(tmpFilePath, TestProto3.OneOfTestMessage.class);
+
+    assertThat(gotBack).hasSize(3);
+    assertThat(gotBack.get(0).getSecond()).isEqualTo(99);
+    assertThat(gotBack.get(0).getTheOneofCase()).isEqualTo(TestProto3.OneOfTestMessage.TheOneofCase.SECOND);
+    assertThat(gotBack.get(1).getTheOneofCase())
+        .isEqualTo(TestProto3.OneOfTestMessage.TheOneofCase.THEONEOF_NOT_SET);
+    assertThat(gotBack.get(2).getFirst()).isEqualTo(42);
+    assertThat(gotBack.get(2).getTheOneofCase()).isEqualTo(TestProto3.OneOfTestMessage.TheOneofCase.FIRST);
+  }
+
   @Test
   public void testMessageRecursion() {
     RecordConsumer readConsumerMock = Mockito.mock(RecordConsumer.class);