GH-2142: Write fields of empty message types as proto bytes - #3750
Open
puskarpeter wants to merge 1 commit into
Open
GH-2142: Write fields of empty message types as proto bytes#3750puskarpeter wants to merge 1 commit into
puskarpeter wants to merge 1 commit into
Conversation
Copy link
Copy Markdown
Sorry, something went wrong.
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
Protobuf allows empty message definitions, but Parquet forbids empty groups. Converting a message
that merely contains a field of an empty message type produces a schema with an empty group,
which writer construction rejects with InvalidSchemaException: Cannot write a schema with an empty group. Such fields appear in real-world schemas (deprecated stubs, marker/placeholder
messages), and a single one makes the whole message type unwritable.
What changes are included in this PR?
ProtoSchemaConverter.addMessageField terminates a field whose message type has no fields as a
BINARY column holding the serialized message — the same mechanism PARQUET-1711 uses for
recursion beyond maxRecursion. Since an empty message serializes to zero bytes, the column is
cheap, and field presence still round-trips (null = unset vs empty bytes = set):
addRepeatedPrimitive, so element cardinality survives;
ProtoWriteSupport.createMessageWriter's existing truncated-field check (primitive BINARY where a
message field was declared → BinaryWriter) is generalized to look through the LIST/MAP wrapper
(getGroupType → getContentType), so the writer tree lines up with these schemas.
A message that is empty at the root is still rejected — there is no parent field to hold the
bytes, and a Parquet file with zero columns is not representable.
Are these changes tested?
Yes. New ProtoEmptyMessageTest (new test messages Stub/StubBox in Trees.proto) writes
through the real write path (ProtoParquetWriter → MessageColumnIO, both specs-compliant and
old style) and reads back with GroupReadSupport:
zero-byte values;
empty group").
ProtoSchemaConverterTest.testEmptyMessageFields pins the converted schema. The full
parquet-protobuf suite passes (114 tests).
Are there any user-facing changes?
Message types that previously could not be written to Parquet at all now can; fields of empty
message types appear as (possibly LIST/MAP-wrapped) binary columns. No change for schemas that
were previously writable. Error behavior for an empty root message is unchanged.
Closes #2142