> On June 25, 2013, 11:27 p.m., Abraham Elmahrek wrote:
> > Overall, looks good. I am mostly unsure about the use of 31 as the only
> > prime number separating hash code values.
>
> Jarek Cecho wrote:
> Hi Abe,
> thank you very much for the review, appreciated! All the hashCode methods
> were generated by my IDE (IntelliJ IDEA) including the magical constant 31. I
> assume that this is best practice, however please correct me if I'm wrong.
The number 31 seems arbitrary. My understanding is that hash codes with
multiple instance members involved require some level of separation. By
consistently using the number 31, there is a small chance that there will be
some overlap between the hash codes produced by the members. It should increase
the chance of a collision... the amount it increases it by is likely small
though if the members are strings. If the members are Boolean, the overlap is
guaranteed... IE: class A: member1 = Boolean, member1 = Boolean, hashCode =
func { result = 0; if member1 != null, then result = member1.hashCode(); if
member2 != null, then result = 31*result + member2.hashCode(); }. NOTE: Boolean
in java has two values for its hash code.
> On June 25, 2013, 11:27 p.m., Abraham Elmahrek wrote:
> > common/src/main/java/org/apache/sqoop/schema/Schema.java, lines 87-89
> > <https://reviews.apache.org/r/12089/diff/1/?file=310883#file310883line87>
> >
> > Doesn't seem necessary if this operation is idempotent.
>
> Jarek Cecho wrote:
> The operation is not meant to be idempotent, the addColumn is suppose to
> be called iteratively to build a list of columns (where the order is
> important) and adding two columns with the same name should not be valid
> operation.
Makes sense!
- Abraham
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12089/#review22380
-----------------------------------------------------------
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
>
>
