puskarpeter commented on PR #3750:
URL: https://github.com/apache/parquet-java/pull/3750#issuecomment-5759772445
@wgtmac Thanks for looking into it :) Both are fair points.
Non-protobuf readers just see an unannotated `BINARY` column with the
field's own repetition (or inside the usual `LIST`/`MAP` wrappers), so there is
no logical type to misinterpret. For an empty message the value is either zero
bytes (set) or null (unset). The tests were already reading the files back
through
`GroupReadSupport`, which knows nothing about protobuf, but that was
implicit, so I added tests that assert the footer schema of the written file in
both schema styles. Readers that want to interpret the bytes can use the
descriptor the writer already stores in the footer, which is what
`ProtoParquetReader` does.
Regarding the contract, PARQUET-1711 has been writing truncated recursion
this way since 1.13.0, so this PR reuses an encoding that already exists in
files out there rather than adding a new one. You are right that it was only
mentioned in the `maxRecursion` Javadoc though, so I added a section to the
parquet-protobuf README and a note in the `ProtoSchemaConverter` Javadoc
covering both cases.
One caveat I put in the docs as well: if an empty message type later gains
fields, new files will have a group where old files have `BINARY`, so schema
merging tools will report a type conflict. That is the same thing that already
happens today when `maxRecursion` changes.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
