You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@orc.apache.org by om...@apache.org on 2019/12/11 18:31:06 UTC
[orc] branch master updated: ORC-556: Do not check for attribute
equality during schema evolution check
This is an automated email from the ASF dual-hosted git repository.
omalley pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/orc.git
The following commit(s) were added to refs/heads/master by this push:
new bf6fc4f ORC-556: Do not check for attribute equality during schema evolution check
bf6fc4f is described below
commit bf6fc4ffab026e74557c50de9f2a827ddf535bdb
Author: Shardul Mahadik <sm...@linkedin.com>
AuthorDate: Mon Dec 2 13:36:18 2019 -0800
ORC-556: Do not check for attribute equality during schema evolution check
Fixes #457
Signed-off-by: Owen O'Malley <om...@apache.org>
---
java/core/src/findbugs/exclude.xml | 5 ++++
.../src/java/org/apache/orc/TypeDescription.java | 32 ++++++++++++++++------
.../org/apache/orc/impl/TreeReaderFactory.java | 4 ++-
.../test/org/apache/orc/TestTypeDescription.java | 19 +++++++++++++
.../org/apache/orc/impl/TestSchemaEvolution.java | 27 ++++++++++++++++++
5 files changed, 77 insertions(+), 10 deletions(-)
diff --git a/java/core/src/findbugs/exclude.xml b/java/core/src/findbugs/exclude.xml
index 9e21433..73ac38b 100644
--- a/java/core/src/findbugs/exclude.xml
+++ b/java/core/src/findbugs/exclude.xml
@@ -65,4 +65,9 @@
<Bug pattern="RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE"/>
<Class name="org.apache.orc.TestStringDictionary"/>
</Match>
+ <Match>
+ <Bug pattern="EQ_UNUSUAL"/>
+ <Class name="org.apache.orc.TypeDescription"/>
+ <Method name="equals" />
+ </Match>
</FindBugsFilter>
diff --git a/java/core/src/java/org/apache/orc/TypeDescription.java b/java/core/src/java/org/apache/orc/TypeDescription.java
index 1533d33..b0dc2af 100644
--- a/java/core/src/java/org/apache/orc/TypeDescription.java
+++ b/java/core/src/java/org/apache/orc/TypeDescription.java
@@ -402,6 +402,19 @@ public class TypeDescription
@Override
public boolean equals(Object other) {
+ return equals(other, true);
+ }
+
+ /**
+ * Determines whether the two object are equal.
+ * This function can either compare or ignore the type attributes as
+ * desired.
+ * @param other the reference object with which to compare.
+ * @param checkAttributes should the type attributes be considered?
+ * @return {@code true} if this object is the same as the other
+ * argument; {@code false} otherwise.
+ */
+ public boolean equals(Object other, boolean checkAttributes) {
if (other == null || !(other instanceof TypeDescription)) {
return false;
}
@@ -415,16 +428,17 @@ public class TypeDescription
precision != castOther.precision) {
return false;
}
- // make sure the attributes are the same
- List<String> attributeNames = getAttributeNames();
- if (castOther.getAttributeNames().size() != attributeNames.size()) {
- return false;
- }
- for(String attribute: attributeNames) {
- if (!getAttributeValue(attribute).equals(
- castOther.getAttributeValue(attribute))) {
+ if (checkAttributes) {
+ // make sure the attributes are the same
+ List<String> attributeNames = getAttributeNames();
+ if (castOther.getAttributeNames().size() != attributeNames.size()) {
return false;
}
+ for (String attribute : attributeNames) {
+ if (!getAttributeValue(attribute).equals(castOther.getAttributeValue(attribute))) {
+ return false;
+ }
+ }
}
// check the children
if (children != null) {
@@ -432,7 +446,7 @@ public class TypeDescription
return false;
}
for (int i = 0; i < children.size(); ++i) {
- if (!children.get(i).equals(castOther.children.get(i))) {
+ if (!children.get(i).equals(castOther.children.get(i), checkAttributes)) {
return false;
}
}
diff --git a/java/core/src/java/org/apache/orc/impl/TreeReaderFactory.java b/java/core/src/java/org/apache/orc/impl/TreeReaderFactory.java
index 3f66eaf..8019afd 100644
--- a/java/core/src/java/org/apache/orc/impl/TreeReaderFactory.java
+++ b/java/core/src/java/org/apache/orc/impl/TreeReaderFactory.java
@@ -2370,7 +2370,9 @@ public class TreeReaderFactory {
return new NullTreeReader(0);
}
TypeDescription.Category readerTypeCategory = readerType.getCategory();
- if (!fileType.equals(readerType) &&
+ // We skip attribute checks when comparing types since they are not used to
+ // create the ConvertTreeReaders
+ if (!fileType.equals(readerType, false) &&
(readerTypeCategory != TypeDescription.Category.STRUCT &&
readerTypeCategory != TypeDescription.Category.MAP &&
readerTypeCategory != TypeDescription.Category.LIST &&
diff --git a/java/core/src/test/org/apache/orc/TestTypeDescription.java b/java/core/src/test/org/apache/orc/TestTypeDescription.java
index e2e722c..3295073 100644
--- a/java/core/src/test/org/apache/orc/TestTypeDescription.java
+++ b/java/core/src/test/org/apache/orc/TestTypeDescription.java
@@ -19,6 +19,7 @@ package org.apache.orc;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertTrue;
@@ -377,6 +378,24 @@ public class TestTypeDescription {
assertEquals(null, street.getAttributeValue("foobar"));
}
+ @Test
+ public void testAttributesEquality() {
+ TypeDescription schema = TypeDescription.fromString(
+ "struct<" +
+ "name:struct<first:string,last:string>," +
+ "address:struct<street:string,city:string,country:string,post_code:string>," +
+ "credit_cards:array<struct<card_number:string,expire:date,ccv:string>>>");
+ // set some attributes
+ schema.findSubtype("name").setAttribute("iceberg.id", "12");
+ schema.findSubtype("address.street").setAttribute("mask", "nullify")
+ .setAttribute("context", "pii");
+
+ TypeDescription clone = schema.clone();
+ assertEquals(3, clearAttributes(clone));
+ assertFalse(clone.equals(schema));
+ assertTrue(clone.equals(schema, false));
+ }
+
static int clearAttributes(TypeDescription schema) {
int result = 0;
for(String attribute: schema.getAttributeNames()) {
diff --git a/java/core/src/test/org/apache/orc/impl/TestSchemaEvolution.java b/java/core/src/test/org/apache/orc/impl/TestSchemaEvolution.java
index fa96fb8..b9e9f13 100644
--- a/java/core/src/test/org/apache/orc/impl/TestSchemaEvolution.java
+++ b/java/core/src/test/org/apache/orc/impl/TestSchemaEvolution.java
@@ -1637,6 +1637,33 @@ public class TestSchemaEvolution {
}
@Test
+ public void testTypeConversionShouldIgnoreAttributes() throws IOException {
+ TypeDescription fileType = TypeDescription.fromString("struct<x:int,y:smallint>");
+ TypeDescription readType = TypeDescription.fromString("struct<x:int,y:int>");
+ readType.findSubtype("x").setAttribute("iceberg.id", "12");
+ readType.findSubtype("y").setAttribute("iceberg.id", "13");
+ SchemaEvolution evo = new SchemaEvolution(fileType, readType, options);
+
+ // check to make sure the fields are mapped correctly
+ assertEquals(1, evo.getFileType(1).getId());
+ assertEquals(2, evo.getFileType(2).getId());
+
+ TreeReaderFactory.Context treeContext =
+ new TreeReaderFactory.ReaderContext().setSchemaEvolution(evo);
+ TreeReaderFactory.TreeReader reader =
+ TreeReaderFactory.createTreeReader(readType, treeContext);
+
+ // check to make sure the tree reader is built right
+ assertEquals(TreeReaderFactory.StructTreeReader.class, reader.getClass());
+ TreeReaderFactory.TreeReader[] children =
+ ((TreeReaderFactory.StructTreeReader) reader).getChildReaders();
+ assertEquals(2, children.length);
+ assertEquals(TreeReaderFactory.IntTreeReader.class, children[0].getClass());
+ assertEquals(ConvertTreeReaderFactory.AnyIntegerFromAnyIntegerTreeReader.class,
+ children[1].getClass());
+ }
+
+ @Test
public void testPositionalEvolution() throws IOException {
options.forcePositionalEvolution(true);
TypeDescription file = TypeDescription.fromString("struct<x:int,y:int,z:int>");