This is an automated email from the ASF dual-hosted git repository. gopalv pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/hive.git
commit bfc44ff83b38d3ad4ce3cfadcc69ebbaff23e974 Author: David Mollitor <dam6...@gmail.com> AuthorDate: Sun Mar 10 22:45:56 2019 -0700 HIVE-21264: Improvements Around CharTypeInfo (David Mollitor, reviewed by Gopal V) Signed-off-by: Gopal V <gop...@apache.org> --- .../hive/serde2/typeinfo/BaseCharTypeInfo.java | 39 ++++++++++++++--- .../hadoop/hive/serde2/typeinfo/BaseCharUtils.java | 2 - .../hadoop/hive/serde2/typeinfo/CharTypeInfo.java | 22 ---------- .../hive/serde2/typeinfo/PrimitiveTypeInfo.java | 41 ++++++++++-------- .../hive/serde2/typeinfo/TypeInfoFactory.java | 6 +-- .../hive/serde2/typeinfo/VarcharTypeInfo.java | 22 ---------- .../hive/serde2/typeinfo/TestBaseCharTypeInfo.java | 50 ++++++++++++++++++++++ 7 files changed, 109 insertions(+), 73 deletions(-) diff --git a/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/BaseCharTypeInfo.java b/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/BaseCharTypeInfo.java index 820fb4e..18de5a0 100644 --- a/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/BaseCharTypeInfo.java +++ b/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/BaseCharTypeInfo.java @@ -54,12 +54,16 @@ public abstract class BaseCharTypeInfo extends PrimitiveTypeInfo { return getQualifiedName(typeName, length); } - public static String getQualifiedName(String typeName, int length) { - StringBuilder sb = new StringBuilder(typeName); - sb.append("("); - sb.append(length); - sb.append(")"); - return sb.toString(); + /** + * Utility method to build the fully qualified data type. For example: + * (char,16) becomes char(16). + * + * @param typeName The name of the data type (char or varchar) + * @param length The maximum length of the data type + * @return A fully qualified field name + */ + protected String getQualifiedName(String typeName, int length) { + return typeName + '(' + length + ')'; } @Override @@ -67,4 +71,27 @@ public abstract class BaseCharTypeInfo extends PrimitiveTypeInfo { // type name should already be set by subclass return; } + + @Override + public int hashCode() { + final int prime = 31; + int result = super.hashCode(); + result = prime * result + length; + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!super.equals(obj)) { + return false; + } + BaseCharTypeInfo other = (BaseCharTypeInfo) obj; + if (length != other.length) { + return false; + } + return true; + } } diff --git a/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/BaseCharUtils.java b/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/BaseCharUtils.java index 259e642..d3c53ff 100644 --- a/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/BaseCharUtils.java +++ b/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/BaseCharUtils.java @@ -22,8 +22,6 @@ import org.apache.hadoop.hive.common.type.HiveBaseChar; import org.apache.hadoop.hive.common.type.HiveChar; import org.apache.hadoop.hive.common.type.HiveVarchar; import org.apache.hadoop.hive.serde2.io.HiveBaseCharWritable; -import org.apache.hadoop.hive.serde2.io.HiveCharWritable; -import org.apache.hadoop.hive.serde2.io.HiveVarcharWritable; public class BaseCharUtils { diff --git a/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/CharTypeInfo.java b/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/CharTypeInfo.java index ef9b945..9b91317 100644 --- a/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/CharTypeInfo.java +++ b/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/CharTypeInfo.java @@ -39,28 +39,6 @@ public class CharTypeInfo extends BaseCharTypeInfo { } @Override - public boolean equals(Object other) { - if (this == other) { - return true; - } - if (other == null || getClass() != other.getClass()) { - return false; - } - - CharTypeInfo pti = (CharTypeInfo) other; - - return this.typeName.equals(pti.typeName) && this.getLength() == pti.getLength(); - } - - /** - * Generate the hashCode for this TypeInfo. - */ - @Override - public int hashCode() { - return getQualifiedName().hashCode(); - } - - @Override public String toString() { return getQualifiedName(); } diff --git a/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/PrimitiveTypeInfo.java b/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/PrimitiveTypeInfo.java index 97af49a..0394ffa 100644 --- a/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/PrimitiveTypeInfo.java +++ b/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/PrimitiveTypeInfo.java @@ -19,6 +19,7 @@ package org.apache.hadoop.hive.serde2.typeinfo; import java.io.Serializable; +import java.util.Objects; import org.apache.hadoop.hive.common.classification.InterfaceAudience; import org.apache.hadoop.hive.common.classification.InterfaceStability; @@ -52,6 +53,7 @@ public class PrimitiveTypeInfo extends TypeInfo implements Serializable { * For TypeInfoFactory use only. */ PrimitiveTypeInfo(String typeName) { + Objects.requireNonNull(typeName); this.typeName = typeName; } @@ -90,29 +92,34 @@ public class PrimitiveTypeInfo extends TypeInfo implements Serializable { } @Override - public boolean equals(Object other) { - if (this == other) { - return true; - } - if (other == null || getClass() != other.getClass()) { - return false; - } - - PrimitiveTypeInfo pti = (PrimitiveTypeInfo) other; - - return this.typeName.equals(pti.typeName); + public String toString() { + return typeName; } - /** - * Generate the hashCode for this TypeInfo. - */ @Override public int hashCode() { - return typeName.hashCode(); + final int prime = 31; + int result = 1; + result = prime * result + ((typeName == null) ? 0 : typeName.hashCode()); + return result; } @Override - public String toString() { - return typeName; + public boolean equals(Object obj) { + if (obj == null) { + return false; + } + if (getClass() != obj.getClass()) { + return false; + } + PrimitiveTypeInfo other = (PrimitiveTypeInfo) obj; + if (typeName == null) { + if (other.typeName != null) { + return false; + } + } else if (!typeName.equals(other.typeName)) { + return false; + } + return true; } } diff --git a/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/TypeInfoFactory.java b/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/TypeInfoFactory.java index 77d60c5..baafb75 100644 --- a/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/TypeInfoFactory.java +++ b/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/TypeInfoFactory.java @@ -174,13 +174,11 @@ public final class TypeInfoFactory { } public static CharTypeInfo getCharTypeInfo(int length) { - String fullName = BaseCharTypeInfo.getQualifiedName(serdeConstants.CHAR_TYPE_NAME, length); - return (CharTypeInfo) getPrimitiveTypeInfo(fullName); + return new CharTypeInfo(length); } public static VarcharTypeInfo getVarcharTypeInfo(int length) { - String fullName = BaseCharTypeInfo.getQualifiedName(serdeConstants.VARCHAR_TYPE_NAME, length); - return (VarcharTypeInfo) getPrimitiveTypeInfo(fullName); + return new VarcharTypeInfo(length); } public static DecimalTypeInfo getDecimalTypeInfo(int precision, int scale) { diff --git a/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/VarcharTypeInfo.java b/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/VarcharTypeInfo.java index edf12a2..f8d41e4 100644 --- a/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/VarcharTypeInfo.java +++ b/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/VarcharTypeInfo.java @@ -39,28 +39,6 @@ public class VarcharTypeInfo extends BaseCharTypeInfo { } @Override - public boolean equals(Object other) { - if (this == other) { - return true; - } - if (other == null || getClass() != other.getClass()) { - return false; - } - - VarcharTypeInfo pti = (VarcharTypeInfo) other; - - return this.getLength() == pti.getLength(); - } - - /** - * Generate the hashCode for this TypeInfo. - */ - @Override - public int hashCode() { - return getLength(); - } - - @Override public String toString() { return getQualifiedName(); } diff --git a/serde/src/test/org/apache/hadoop/hive/serde2/typeinfo/TestBaseCharTypeInfo.java b/serde/src/test/org/apache/hadoop/hive/serde2/typeinfo/TestBaseCharTypeInfo.java new file mode 100644 index 0000000..19a2bcb --- /dev/null +++ b/serde/src/test/org/apache/hadoop/hive/serde2/typeinfo/TestBaseCharTypeInfo.java @@ -0,0 +1,50 @@ +/* + * 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.hadoop.hive.serde2.typeinfo; + +import org.junit.Assert; +import org.junit.Test; + +/** + * Test suite for BaseCharTypes. + */ +public class TestBaseCharTypeInfo { + + /** + * Test that VARCHAR and CHAR are not considered to be the same. + */ + @Test + public void testVarCharToCharEqualityDefault() { + VarcharTypeInfo vcharTypeInfo = new VarcharTypeInfo(); + CharTypeInfo charTypeInfo = new CharTypeInfo(); + + Assert.assertNotEquals(vcharTypeInfo, charTypeInfo); + } + + /** + * Test that VARCHAR(10) and CHAR(10) are not considered to be the same. + */ + @Test + public void testVarCharToCharEqualitySize() { + VarcharTypeInfo vcharTypeInfo = new VarcharTypeInfo(10); + CharTypeInfo charTypeInfo = new CharTypeInfo(10); + + Assert.assertNotEquals(vcharTypeInfo, charTypeInfo); + } +}