----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12089/#review22380 -----------------------------------------------------------
Overall, looks good. I am mostly unsure about the use of 31 as the only prime number separating hash code values. common/src/main/java/org/apache/sqoop/schema/Schema.java <https://reviews.apache.org/r/12089/#comment45913> Doesn't seem necessary if this operation is idempotent. common/src/main/java/org/apache/sqoop/schema/type/DateTime.java <https://reviews.apache.org/r/12089/#comment45907> Can we use a different base than 31 for this hashcode? DateTime's parent uses 31, which means there might be a common factor eventually. common/src/main/java/org/apache/sqoop/schema/type/Decimal.java <https://reviews.apache.org/r/12089/#comment45908> Same hash code comment as above. common/src/main/java/org/apache/sqoop/schema/type/FixedPoint.java <https://reviews.apache.org/r/12089/#comment45909> Same hash code comment as above common/src/main/java/org/apache/sqoop/schema/type/FloatingPoint.java <https://reviews.apache.org/r/12089/#comment45910> Same hash code comment as above common/src/main/java/org/apache/sqoop/schema/type/Map.java <https://reviews.apache.org/r/12089/#comment45911> Same hash code comment as above common/src/main/java/org/apache/sqoop/schema/type/Time.java <https://reviews.apache.org/r/12089/#comment45912> Same hash code comment as above - Abraham Elmahrek On June 25, 2013, 9:42 p.m., Jarek Cecho wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12089/ > ----------------------------------------------------------- > > (Updated June 25, 2013, 9:42 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-1101 > https://issues.apache.org/jira/browse/SQOOP-1101 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > This part of SQOOP-1073 is adding classes representing the data types > proposed in SQOOP-515. > > > Diffs > ----- > > common/pom.xml 2921800 > common/src/main/java/org/apache/sqoop/schema/Schema.java PRE-CREATION > common/src/main/java/org/apache/sqoop/schema/SchemaError.java PRE-CREATION > common/src/main/java/org/apache/sqoop/schema/type/AbstractComplexType.java > PRE-CREATION > common/src/main/java/org/apache/sqoop/schema/type/AbstractDateTime.java > PRE-CREATION > common/src/main/java/org/apache/sqoop/schema/type/AbstractNumber.java > PRE-CREATION > common/src/main/java/org/apache/sqoop/schema/type/AbstractString.java > PRE-CREATION > common/src/main/java/org/apache/sqoop/schema/type/Array.java PRE-CREATION > common/src/main/java/org/apache/sqoop/schema/type/Binary.java PRE-CREATION > common/src/main/java/org/apache/sqoop/schema/type/Bit.java PRE-CREATION > common/src/main/java/org/apache/sqoop/schema/type/Column.java PRE-CREATION > common/src/main/java/org/apache/sqoop/schema/type/Date.java PRE-CREATION > common/src/main/java/org/apache/sqoop/schema/type/DateTime.java > PRE-CREATION > common/src/main/java/org/apache/sqoop/schema/type/Decimal.java PRE-CREATION > common/src/main/java/org/apache/sqoop/schema/type/Enum.java PRE-CREATION > common/src/main/java/org/apache/sqoop/schema/type/FixedPoint.java > PRE-CREATION > common/src/main/java/org/apache/sqoop/schema/type/FloatingPoint.java > PRE-CREATION > common/src/main/java/org/apache/sqoop/schema/type/Map.java PRE-CREATION > common/src/main/java/org/apache/sqoop/schema/type/Set.java PRE-CREATION > common/src/main/java/org/apache/sqoop/schema/type/Text.java PRE-CREATION > common/src/main/java/org/apache/sqoop/schema/type/Time.java PRE-CREATION > common/src/main/java/org/apache/sqoop/schema/type/Type.java PRE-CREATION > common/src/main/java/org/apache/sqoop/schema/type/Unsupported.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/12089/diff/ > > > Testing > ------- > > > Thanks, > > Jarek Cecho > >
