[I] parquet-protobuf: recursion truncation breaks repeated and map fields — ClassCastException in specs-compliant mode, file corruption in the old style [parquet-java]

["puskarpeter (via GitHub)" <gi...@apache.org>]

puskarpeter opened a new issue, #3751:
URL: https://github.com/apache/parquet-java/issues/3751

   ### Describe the bug
   
   `ProtoSchemaConverter` (PARQUET-1711) terminates recursive proto message fields at
   `parquet.proto.maxRecursion` depth by replacing them with the serialized proto bytes. The
   truncation hardcodes the replacement column as `optional binary`, ignoring the original field's
   repetition:
   
   ```java
   // ProtoSchemaConverter.addMessageField
   if (seen.get(typeName).size() > maxRecursion) {
     return builder.primitive(BINARY, Type.Repetition.OPTIONAL).as((LogicalTypeAnnotation) null);
   }
   ```
   
   `ProtoWriteSupport.MessageWriter` however still wraps every repeated field's writer in
   `ArrayWriter` (specs-compliant) or `RepeatedWriter` (old style), and map fields in `MapWriter` —
   all of which emit record structure the `optional binary` column cannot hold. The mock-based unit
   tests (`ProtoWriteSupportTest.testRepeatedRecursion` / `testMapRecursion`) never validate against a
   real `MessageColumnIO`, so the mismatch was never caught.
   
   Consequences, each reproduced by an end-to-end write test included in the follow-up PR:
   
   1. **Specs-compliant mode, repeated recursive field, data deeper than maxRecursion:** the write
      crashes with
   
      ```
      java.lang.ClassCastException: class org.apache.parquet.io.PrimitiveColumnIO cannot be cast to
      class org.apache.parquet.io.GroupColumnIO
      ```
   
      (`ArrayWriter` calls `startGroup()`/`startField("list", 0)` against a primitive column). This is
      a data-dependent landmine: schema creation and shallow rows succeed; the job dies only when a
      row's data actually nests past the limit.
   
   2. **Old style (writeSpecsCompliant=false), repeated recursive field with more than one element at
      the truncation depth:** no exception — the write emits inconsistent repetition levels for the
      second and following elements, **corrupting the file**. Depending on the data, reading it back
      either fails with `ParquetDecodingException` (`Can not read value at ... in block`,
      EOF/BufferUnderflow underneath) or silently returns a wrong tree (elements lost or attached to
      phantom duplicate parent nodes).
   
   3. **Specs-compliant map field at which the recursion budget runs out** (e.g.
      `google.protobuf.Struct` maps reached through `list_value` branches): the whole MAP — including
      its keys — collapses into a single unreadable binary in the schema, and writing data through
      that branch crashes with the same `ClassCastException` (`MapWriter` navigating `key_value`
      groups over a primitive column). On map paths where the budget happens to trip at a singular
      field first (like `Struct`'s main `fields → struct_value` chain), the schema was already fine —
      the collapse is branch-dependent.
   
   ### Reproducer
   
   Any repeated self-recursive message nested deeper than maxRecursion, e.g. the existing test proto
   `Trees.WideTree`:
   
   ```java
   Trees.WideTree deep = ...; // chain of children 5 levels deep, 2 children per node
   Configuration conf = new Configuration();
   ProtoWriteSupport.setWriteSpecsCompliant(conf, true);
   ProtoSchemaConverter.setMaxRecursion(conf, 2);
   try (ParquetWriter<Message> w = ProtoParquetWriter.<Message>builder(path)
       .withMessage(Trees.WideTree.class).withConf(conf).build()) {
     w.write(deep); // ClassCastException
   }
   ```
   
   Affects all released versions since 1.13.0 (PARQUET-1711) through current master (verified on
   1.17.1 and master @ e02f65e2).
   
   ### Proposed fix (PR follows)
   
   Preserve the field's shape when truncating, mirroring how ordinary repeated primitives are handled:
   
   - `ProtoSchemaConverter.addMessageField`:
     - repeated + specs-compliant → LIST-wrapped binary via the existing `addRepeatedPrimitive`
       (`optional group x (LIST) { repeated group list { required binary element } }`);
     - otherwise `builder.primitive(BINARY, getRepetition(descriptor))` (`repeated binary` in the old
       style; truncated optional fields unchanged, proto2 required fields now keep `required`);
     - specs-compliant map fields keep their MAP structure unconditionally; a recursive **value** type
       is truncated to `optional binary` inside `key_value` when `addMapField` recurses into the value
       field (same recursion budget, applied one level deeper where it belongs).
   - `ProtoWriteSupport.createMessageWriter`: look through the LIST/MAP wrapper when detecting a
     truncated-to-binary message field (`getContentType`, introduced by the fix for #2142, which
     terminates empty message types through the same mechanism) so `BinaryWriter` is selected for
     truncated elements/values; the existing `ArrayWriter`/`RepeatedWriter`/`MapWriter` wrapping then
     lines up with the schema.
   
   With the fix, each repeated element / map value at the truncation depth round-trips as one binary
   containing the serialized subtree (`X.parseFrom(bytes)` reconstructs it), keys of truncated-value
   maps stay queryable, and truncated optional fields are byte-for-byte unchanged.
   
   Existing expected-schema tests were regenerated (`WideTree.par`, `Value.par`, `Struct.par`,
   inline schemas, and the `testDeepRecursion` Struct fan-out series changes from `2n+4` to `2n+5`
   because a truncated map now retains its key column). New `ProtoRecursionTruncationTest` (5 tests)
   writes through a real `MessageColumnIO`: repeated recursion in both modes, a map field exhausting
   the recursion budget (fails with the ClassCastException before the fix), and the already-working
   map-main-path and optional cases as regression guards.
   
   Note on schema compatibility: files previously written with a truncated *optional* field are
   unchanged. A repeated/map truncated field changes its schema shape — but writing more than one
   element at the truncation depth crashed (specs) or corrupted the file (old style) before, so no
   valid existing files carry the old shape with meaningful multi-element data.
   
   ### Related
   
   - PARQUET-1711 / #995 — introduced maxRecursion truncation (optional-field case only).
   - #2708 / PARQUET-2181 — read-side ClassCastException in parquet-cli on proto files; note that
     `ProtoParquetReader` itself also cannot read back **any** truncated field (including the optional
     case that writes fine): `ProtoMessageConverter.newScalarConverter` has no binary→message path and
     throws `ClassCastException` at converter-tree construction. That read-side gap is orthogonal to
     this write-side fix and probably deserves its own issue.
   - #2142 — empty message types cannot be written at all; the companion PR fixes it with the same
     terminate-as-proto-bytes mechanism, and this fix builds on it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org

Re: [I] parquet-protobuf: recursion truncation breaks repeated and map fields — ClassCastException in specs-compliant mode, file corruption in the old style [parquet-java]

["wgtmac (via GitHub)" <gi...@apache.org>]

wgtmac closed issue #3751: parquet-protobuf: recursion truncation breaks repeated and map fields — ClassCastException in specs-compliant mode, file corruption in the old style
URL: https://github.com/apache/parquet-java/issues/3751


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org