GH-3671: Keep files readable when they use future logical types#3669
GH-3671: Keep files readable when they use future logical types#3669kevinjqliu wants to merge 1 commit into
Conversation
| import org.junit.Test; | ||
|
|
||
| public class TestInterOpReadUnknownLogicalType { | ||
| private static final String REFERENCE_FILE = "unknown-logical-type.parquet"; |
There was a problem hiding this comment.
Added a test using the parquet-testing artifact
unknown-logical-type.parquet
A file containing a column annotated with a LogicalType whose identifier has been set to an abitrary high value to check the behaviour of an old reader reading a file written by a new writer containing an unsupported type (see related issue).
-- from https://github.com/apache/parquet-testing/blob/master/data/README.md
ref: https://github.com/apache/parquet-testing/blob/master/data/unknown-logical-type.parquet
4aa0e2e to
f9fba56
Compare
f9fba56 to
33d2f58
Compare
wgtmac
left a comment
wgtmac
left a comment
There was a problem hiding this comment.
Thanks @kevinjqliu! LGTM. Let's create a Github issue and link it to the PR title?
kevinjqliu
changed the title
etseidl
left a comment
etseidl
left a comment
There was a problem hiding this comment.
Looks good, thanks for fixing this. Just one non-blocking nit.
| LogicalTypeAnnotation getLogicalTypeAnnotation(LogicalType type) { | ||
| switch (type.getSetField()) { | ||
| LogicalType._Fields setField = type.getSetField(); | ||
| if (setField == null) { |
There was a problem hiding this comment.
Would it be simpler to just change the default case to return null rather than throwing an exception?
4 participants
This is part of the effort to ensure that new logical types are forward compatible (See apache/parquet-format#599)
Closes #3671
Description
This makes parquet-java more tolerant of Parquet files written by newer implementations.
A newer writer may use a logical type that this version of parquet-java does not know about yet. The file can still have a perfectly valid physical column type, but today the reader fails while trying to interpret the unknown logical-type metadata.
The fix is to handle that case in
ParquetMetadataConverter, where file metadata is translated into parquet-java schema annotations. If the logical type cannot be understood, the converter now leaves the column as its physical type instead of failing. That is the right behavior for forward compatibility: readers should preserve the data they can understand, and only apply logical annotations when they are known.This also keeps the existing fallback behavior intact for files that include older
converted_typemetadata.Tests
Added tests cover reading an unknown logical type both with and without a
converted_typefallback.