You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@parquet.apache.org by "ASF GitHub Bot (JIRA)" <ji...@apache.org> on 2018/11/07 10:20:00 UTC
[jira] [Commented] (PARQUET-1305) Backward incompatible change
introduced in 1.8
[ https://issues.apache.org/jira/browse/PARQUET-1305?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16677977#comment-16677977 ]
ASF GitHub Bot commented on PARQUET-1305:
-----------------------------------------
gszadovszky closed pull request #483: PARQUET-1305: Backward incompatible change introduced in 1.8
URL: https://github.com/apache/parquet-mr/pull/483
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/parquet-thrift/src/main/java/org/apache/parquet/thrift/struct/ThriftType.java b/parquet-thrift/src/main/java/org/apache/parquet/thrift/struct/ThriftType.java
index 3d64e8dcf..5f8afe6d0 100644
--- a/parquet-thrift/src/main/java/org/apache/parquet/thrift/struct/ThriftType.java
+++ b/parquet-thrift/src/main/java/org/apache/parquet/thrift/struct/ThriftType.java
@@ -1,4 +1,4 @@
-/*
+/*
* 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
@@ -6,9 +6,9 @@
* 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
@@ -230,7 +230,7 @@ public StructType(List<ThriftField> children) {
public StructType(@JsonProperty("children") List<ThriftField> children,
@JsonProperty("structOrUnionType") StructOrUnionType structOrUnionType) {
super(STRUCT);
- this.structOrUnionType = structOrUnionType == null ? StructOrUnionType.UNKNOWN : structOrUnionType;
+ this.structOrUnionType = structOrUnionType == null ? StructOrUnionType.STRUCT : structOrUnionType;
this.children = children;
int maxId = 0;
if (children != null) {
diff --git a/parquet-thrift/src/test/java/org/apache/parquet/thrift/TestThriftSchemaConverter.java b/parquet-thrift/src/test/java/org/apache/parquet/thrift/TestThriftSchemaConverter.java
index cd5fc47eb..2cde15bb1 100644
--- a/parquet-thrift/src/test/java/org/apache/parquet/thrift/TestThriftSchemaConverter.java
+++ b/parquet-thrift/src/test/java/org/apache/parquet/thrift/TestThriftSchemaConverter.java
@@ -1,4 +1,4 @@
-/*
+/*
* 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
@@ -6,9 +6,9 @@
* 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
@@ -23,6 +23,7 @@
import org.apache.parquet.thrift.projection.StrictFieldProjectionFilter;
import org.apache.parquet.thrift.projection.ThriftProjectionException;
import org.apache.parquet.thrift.projection.deprecated.DeprecatedFieldProjectionFilter;
+import org.apache.parquet.thrift.struct.ThriftField;
import org.apache.parquet.thrift.struct.ThriftType;
import org.apache.parquet.thrift.struct.ThriftType.StructType;
import org.apache.parquet.thrift.test.compat.MapStructV2;
@@ -34,7 +35,10 @@
import com.twitter.data.proto.tutorial.thrift.Person;
import com.twitter.elephantbird.thrift.test.TestStructInMap;
+import java.util.Arrays;
+
import static org.apache.parquet.schema.MessageTypeParser.parseMessageType;
+import static org.apache.parquet.thrift.struct.ThriftField.Requirement.REQUIRED;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;
@@ -288,6 +292,26 @@ public void testSetPartialProjection() {
}
}
+ @Test
+ public void testConvertStructCreatedViaDeprecatedConstructor() {
+ String expected = "message ParquetSchema {\n" +
+ " required binary a (UTF8) = 1;\n" +
+ " required binary b (UTF8) = 2;\n" +
+ "}\n";
+
+ ThriftSchemaConverter converter = new ThriftSchemaConverter();
+
+ StructType structType = new StructType(
+ Arrays.asList(
+ new ThriftField("a", (short) 1, REQUIRED, new ThriftType.StringType()),
+ new ThriftField("b", (short) 2, REQUIRED, new ThriftType.StringType())
+ )
+ );
+
+ final MessageType converted = converter.convert(structType);
+ assertEquals(MessageTypeParser.parseMessageType(expected), converted);
+ }
+
public static void shouldGetProjectedSchema(String deprecatedFilterDesc, String strictFilterDesc, String expectedSchemaStr, Class<? extends TBase<?,?>> thriftClass) {
MessageType depRequestedSchema = getDeprecatedFilteredSchema(deprecatedFilterDesc, thriftClass);
MessageType strictRequestedSchema = getStrictFilteredSchema(strictFilterDesc, thriftClass);
diff --git a/parquet-thrift/src/test/java/org/apache/parquet/thrift/struct/TestThriftType.java b/parquet-thrift/src/test/java/org/apache/parquet/thrift/struct/TestThriftType.java
index e70004290..88e8bf1f1 100644
--- a/parquet-thrift/src/test/java/org/apache/parquet/thrift/struct/TestThriftType.java
+++ b/parquet-thrift/src/test/java/org/apache/parquet/thrift/struct/TestThriftType.java
@@ -1,4 +1,4 @@
-/*
+/*
* 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
@@ -6,9 +6,9 @@
* 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
@@ -35,7 +35,7 @@ public void testWriteUnionInfo() throws Exception {
assertEquals("{\n"
+" \"id\" : \"STRUCT\",\n"
+" \"children\" : [ ],\n"
- +" \"structOrUnionType\" : \"UNKNOWN\"\n"
+ +" \"structOrUnionType\" : \"STRUCT\"\n"
+"}", st.toJSON());
st = new StructType(new LinkedList<ThriftField>(), StructOrUnionType.UNION);
@@ -60,7 +60,7 @@ public void testParseUnionInfo() throws Exception {
st = (StructType) StructType.fromJSON("{\"id\": \"STRUCT\", \"children\":[], \"structOrUnionType\": \"STRUCT\"}");
assertEquals(st.getStructOrUnionType(), StructOrUnionType.STRUCT);
st = (StructType) StructType.fromJSON("{\"id\": \"STRUCT\", \"children\":[]}");
- assertEquals(st.getStructOrUnionType(), StructOrUnionType.UNKNOWN);
+ assertEquals(st.getStructOrUnionType(), StructOrUnionType.STRUCT);
st = (StructType) StructType.fromJSON("{\"id\": \"STRUCT\", \"children\":[], \"structOrUnionType\": \"UNKNOWN\"}");
assertEquals(st.getStructOrUnionType(), StructOrUnionType.UNKNOWN);
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
> Backward incompatible change introduced in 1.8
> ----------------------------------------------
>
> Key: PARQUET-1305
> URL: https://issues.apache.org/jira/browse/PARQUET-1305
> Project: Parquet
> Issue Type: Bug
> Reporter: Nandor Kollar
> Assignee: Nandor Kollar
> Priority: Major
> Labels: pull-request-available
> Fix For: 1.11.0
>
>
> ThriftSchemaConverter#convert(StructType struct) introduced a backward incompatible logic in Parquet 1.8, If StructType's [deprecated constructor|https://github.com/apache/parquet-mr/blob/master/parquet-thrift/src/main/java/org/apache/parquet/thrift/struct/ThriftType.java#L225] is used to create StructTypes, and this convert method is used, then the client receives the following exception:
> {code}
> org.apache.parquet.ShouldNeverHappenException: Encountered UNKNOWN StructOrUnionType
> at org.apache.parquet.thrift.ThriftSchemaConvertVisitor.isUnion(ThriftSchemaConvertVisitor.java:342)
> at org.apache.parquet.thrift.ThriftSchemaConvertVisitor.visit(ThriftSchemaConvertVisitor.java:218)
> at org.apache.parquet.thrift.ThriftSchemaConvertVisitor.visit(ThriftSchemaConvertVisitor.java:74)
> at org.apache.parquet.thrift.struct.ThriftType$StructType.accept(ThriftType.java:269)
> at org.apache.parquet.thrift.ThriftSchemaConvertVisitor.convert(ThriftSchemaConvertVisitor.java:93)
> at org.apache.parquet.thrift.ThriftSchemaConverter.convert(ThriftSchemaConverter.java:75)
> {code}
> To illustrate the issue, here's a test case, which passes on older versions, but fails on master:
> {code}
> @Test
> public void testIncompatibleThriftConverterChange() {
> ThriftSchemaConverter converter = new ThriftSchemaConverter();
> ThriftType.StructType structType = new ThriftType.StructType(
> asList(
> new ThriftField("a", (short)1, REQUIRED, new ThriftType.StringType()),
> new ThriftField("b", (short) 2, REQUIRED, new ThriftType.StringType())
> )
> );
> converter.convert(structType);
> }
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)