You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@parquet.apache.org by "kevinjqliu (via GitHub)" <gi...@apache.org> on 2026/07/16 17:58:20 UTC
[PR] Keep files readable when they use future logical types [parquet-java]
kevinjqliu opened a new pull request, #3669:
URL: https://github.com/apache/parquet-java/pull/3669
## 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_type` metadata.
## Tests
Added tests cover reading an unknown logical type both with and without a `converted_type` fallback.
--
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
Re: [PR] GH-3671: Keep files readable when they use future logical types [parquet-java]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko merged PR #3669:
URL: https://github.com/apache/parquet-java/pull/3669
--
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
Re: [PR] GH-3671: Keep files readable when they use future logical types [parquet-java]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on PR #3669:
URL: https://github.com/apache/parquet-java/pull/3669#issuecomment-5014583483
Let's move this in, thanks @kevinjqliu for working on this, and thanks @wgtmac and @etseidl for the review ๐
--
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
Re: [PR] GH-3671: Keep files readable when they use future logical types [parquet-java]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #3669:
URL: https://github.com/apache/parquet-java/pull/3669#discussion_r3605908975
##########
parquet-hadoop/src/main/java/org/apache/parquet/format/converter/ParquetMetadataConverter.java:
##########
@@ -1343,7 +1343,12 @@ LogicalTypeAnnotation getLogicalTypeAnnotation(ConvertedType type, SchemaElement
}
LogicalTypeAnnotation getLogicalTypeAnnotation(LogicalType type) {
- switch (type.getSetField()) {
+ LogicalType._Fields setField = type.getSetField();
+ if (setField == null) {
Review Comment:
It would and i wanted to do that, but unfortunately `switch` statement doesnt work with `null`
> A switch statement is executed by first evaluating the Expression. If the Expression evaluates to null, a NullPointerException is thrown and the entire switch statement completes abruptly for that reason. Otherwise, if the result is of type Character, Byte, Short, or Integer, it is subject to unboxing conversion ([ยง5.1.8](https://docs.oracle.com/javase/specs/jls/se11/html/jls-5.html#jls-5.1.8)).
-- https://docs.oracle.com/javase/specs/jls/se11/html/jls-14.html#jls-SwitchStatement
--
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
Re: [PR] Keep files readable when they use future logical types [parquet-java]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #3669:
URL: https://github.com/apache/parquet-java/pull/3669#discussion_r3597873889
##########
parquet-hadoop/src/test/java/org/apache/parquet/hadoop/TestInterOpReadUnknownLogicalType.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.parquet.hadoop;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+import java.io.IOException;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.apache.parquet.example.data.Group;
+import org.apache.parquet.hadoop.example.GroupReadSupport;
+import org.apache.parquet.hadoop.util.HadoopInputFile;
+import org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName;
+import org.apache.parquet.schema.Type;
+import org.junit.Test;
+
+public class TestInterOpReadUnknownLogicalType {
+ private static final String REFERENCE_FILE = "unknown-logical-type.parquet";
Review 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
--
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
Re: [PR] GH-3671: Keep files readable when they use future logical types [parquet-java]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #3669:
URL: https://github.com/apache/parquet-java/pull/3669#issuecomment-4999101669
thanks for the review. added a new github issue: https://github.com/apache/parquet-java/issues/3671
and also linked to the pr title and description
i previously opened https://github.com/apache/parquet-java/issues/3633. it is similar but slightly different. To fully resolve #3633, we might need to consider backports. Hence opened a new github issue
--
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
Re: [PR] GH-3671: Keep files readable when they use future logical types [parquet-java]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #3669:
URL: https://github.com/apache/parquet-java/pull/3669#issuecomment-5012746909
cc @Fokko do you mind taking a look? this should unblock the [1.18.0 milestone](https://github.com/apache/parquet-java/milestone/34)
--
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
Re: [PR] GH-3671: Keep files readable when they use future logical types [parquet-java]
Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on code in PR #3669:
URL: https://github.com/apache/parquet-java/pull/3669#discussion_r3605408178
##########
parquet-hadoop/src/main/java/org/apache/parquet/format/converter/ParquetMetadataConverter.java:
##########
@@ -1343,7 +1343,12 @@ LogicalTypeAnnotation getLogicalTypeAnnotation(ConvertedType type, SchemaElement
}
LogicalTypeAnnotation getLogicalTypeAnnotation(LogicalType type) {
- switch (type.getSetField()) {
+ LogicalType._Fields setField = type.getSetField();
+ if (setField == null) {
Review Comment:
Would it be simpler to just change the `default` case to return null rather than throwing an exception?
--
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