apache / apache/parquet-java

ProtoSchemaConverter renders invalid schema for oneof in unwrap mode

Open
#3,039 0 comments 0 reactions 0 assignees View on GitHub
Type: bug
Dominant language
Java
Stars
3.1k
Forks
1.6k
Avg merge
3d 12h
Merged PRs (30d)
33

Description

### Describe the bug, including details regarding any error messages, version, and platform.

When unwrap is enabled all fields of the `TestProto3.OneOfTestMessage` will be required.

```
message TestProto3.OneOfTestMessage {
required int32 first = 1;
required int32 second = 2;
}
```
https://github.com/apache/parquet-java/blob/73a4430af6c40f8eb246ad4911eb6d103c9a2abe/parquet-protobuf/src/test/resources/TestProto3.proto#L116

This will never work but tests are missing for unwrap of `TestProto3.OneOfTestMessage`

The required repetition is added here:
https://github.com/apache/parquet-java/blob/73a4430af6c40f8eb246ad4911eb6d103c9a2abe/parquet-protobuf/src/main/java/org/apache/parquet/proto/ProtoSchemaConverter.java#L278

This could be caught early by adding `withValidation(true)` here:
https://github.com/apache/parquet-java/blob/73a4430af6c40f8eb246ad4911eb6d103c9a2abe/parquet-protobuf/src/test/java/org/apache/parquet/proto/TestUtils.java#L221

If validation is disabled it will fail when trying to read one of the non-existing fields in the `oneof`.

Is there really a need for setting all primitive fields in unwrap to required?

### Component(s)

_No response_

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in parquet-protobuf/src/main/java/org/apache/parquet/proto/ProtoSchemaConverter.java at the unwrap repetition handling, then review the TestProto3.OneOfTestMessage fixture and its tests. Enable validation in parquet-protobuf/src/test/java/org/apache/parquet/proto/TestUtils.java and add coverage for unwrap mode. Done means the oneof conversion produces a valid schema and the relevant read path is tested.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
data
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.