| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Parquet forbids empty groups, so a message containing a field whose type is an empty proto message converted into a schema that writer construction rejects with "Cannot write a schema with an empty group" (InvalidSchemaException) - a single such field made the whole message type unwritable. Terminate such fields as BINARY holding the serialized message (zero bytes when the field is set), the same mechanism PARQUET-1711 uses for recursion beyond maxRecursion, preserving the field's repetition: LIST-wrapped binary in parquet-specs mode, repeated binary in the old style, optional binary for map values inside key_value. Field presence and cardinality round-trip; only a message that is empty at the root is still rejected. ProtoWriteSupport's truncated-field detection now looks through the LIST/MAP wrapper (getContentType) so BinaryWriter lines up with these schemas. Signed-off-by: Puškár, Peter <peter.puskar@firma.seznam.cz>
…oto fields The maxRecursion truncation (PARQUET-1711) replaced recursive fields with a hardcoded optional binary, while ProtoWriteSupport still wraps repeated/map fields' writers in ArrayWriter/RepeatedWriter/MapWriter. Writing data that nests deeper than maxRecursion through a repeated recursive field crashed with a ClassCastException in parquet-specs mode and corrupted the file in the old style (inconsistent repetition levels: reads fail with ParquetDecodingException or return a wrong tree). A map field exhausting the recursion budget collapsed entirely - keys included - into one binary, and writing data through it crashed the same way. Truncate to proto bytes preserving the field's shape instead, reusing the terminate-as-bytes path introduced for empty message types (apacheGH-2142): LIST-wrapped binary for repeated fields in specs mode, repeated binary in the old style, and the MAP structure kept with the recursive value truncated inside key_value (the map branch now runs before the recursion check). Truncated optional fields are unchanged; proto2 required fields now keep their required repetition. Each truncated cell round-trips as the serialized subtree. Signed-off-by: Puškár, Peter <peter.puskar@firma.seznam.cz>
| Back | FazBrowse Home | New Git URL |
Note
Draft — stacked on #3750. This fix reuses the terminate-as-proto-bytes machinery introduced
by the empty-message fix (GH-2142), so only the top commit belongs to this PR; the base commit
is #3750's. I will rebase onto master and mark this ready for review once #3750 merges.
Rationale for this change
PARQUET-1711 truncates recursive proto fields at parquet.proto.maxRecursion depth by replacing
them with the serialized proto bytes, but hardcodes the replacement column as optional binary,
ignoring the field's actual repetition. ProtoWriteSupport still wraps repeated fields' writers in
ArrayWriter/RepeatedWriter and map fields in MapWriter, which emit record structure the
optional binary column cannot hold. As a result (see #3751):
recursive field crashes with ClassCastException: PrimitiveColumnIO cannot be cast to GroupColumnIO — a data-dependent failure that passes schema creation and shallow rows;
repetition levels and corrupts the file — depending on the data, reading it back either
fails with ParquetDecodingException or silently returns a wrong tree (elements lost or
attached to phantom duplicate nodes);
reached through list_value branches), the whole MAP — including its keys — collapses into one
binary in the schema, and writing data through it crashes with the same ClassCastException.
The existing mock-based tests (ProtoWriteSupportTest.testRepeatedRecursion/testMapRecursion)
never validate against a real MessageColumnIO, which is why the mismatch went unnoticed.
What changes are included in this PR?
ProtoSchemaConverter.addMessageField keeps the field's shape when truncating, reusing the
terminate-as-proto-bytes path introduced for empty message types (#3750, which this builds on):
style; truncated optional fields stay optional binary, byte-for-byte identical to before
(proto2 required fields in a recursion cycle now keep their required repetition instead of
being forced optional);
(typed key) is always preserved and a recursive value type is truncated to optional binary
inside key_value when addMapField recurses into the value field — same recursion budget,
applied at the level where the recursion actually is.
The writer side needs no further changes: the getContentType check from #3750 already selects
BinaryWriter behind LIST/MAP wrappers, so the existing
ArrayWriter/RepeatedWriter/MapWriter wrapping then lines up with the schema.
Are these changes tested?
Yes. New ProtoRecursionTruncationTest (5 tests) writes recursive data deeper than maxRecursion
through the real write path (ProtoParquetWriter → MessageColumnIO) and reads it back:
ClassCastException and the file-corrupting write, respectively; now every element at the
truncation depth round-trips as the serialized subtree;
previously the whole-MAP collapse plus the same ClassCastException when data reached it; now
keys stay typed and queryable, values round-trip as serialized protos;
regression guards for the shapes that already worked.
Expected-schema fixtures were regenerated for the new truncation shape: WideTree.par,
Value.par, Struct.par, the inline schemas in ProtoSchemaConverterTest, and the
testDeepRecursion Struct fan-out series (now 2n+5 — a truncated map keeps its key column). The
full parquet-protobuf suite passes.
Are there any user-facing changes?
Schemas containing repeated or map recursive fields change shape at the truncation depth
(LIST-of-binary / MAP-with-binary-value instead of a single optional binary) — but writing more
than one element at that depth previously crashed (specs mode) or corrupted the file (old style),
so no valid existing files carry meaningful multi-element data in the old shape. Truncated
optional fields are unchanged; proto2 required fields in a recursion cycle now map to required binary (previously forced optional). Data past the truncation depth now round-trips losslessly:
each binary cell is the serialized subtree, recoverable with X.parseFrom(bytes).
Closes #3751