You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by go...@apache.org on 2019/03/11 05:53:06 UTC

[hive] 01/02: HIVE-21264: Improvements Around CharTypeInfo (David Mollitor, reviewed by Gopal V)

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 <da...@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 <go...@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);
+  }
+}