> On March 29, 2015, 8:29 p.m., Jarek Cecho wrote: > > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopIDFUtils.java, > > lines 130-142 > > <https://reviews.apache.org/r/32588/diff/1/?file=908480#file908480line130> > > > > I'm not particularly familiar with this code, so forgive me if my > > question doesn't make much sense here. It seems to me that we're doing > > quite a lot if-elses and castings to simply get Number object to i'ts > > corresponding string representation. Wouldn't be simpler to just do: > > > > return obj.toString()? > > Abraham Elmahrek wrote: > Great question. This is one I wasn't 100% sure about either > unfortunately. I think it depends on what we want to expose to connector > developers. I've assumed the connector developers want the framework to make > sure the data is correct. This may not be right... > > Here's the different scenarios I see: > 1. Object value is an integer. > 2. Object value is a long. > 3. Object value is a number, but neither a long nor integer. > 4. Object value is not a number. > > The resulting code would be affected by the following variables: > 1. How we want to handle Long values being passed to Integer sized columns > 2. How we want to handle non-numeric values being passed > > This code will do the following: > 1. Column(Integer), Value(Integer) => value as string > 2. Column(Integer), Value(Long) => integer bits selected then transformed > to string (e.g. Java.lang.Long.MAX_VALUE.intValue() should be -1). > 3. Column(Integer), Value(String) => validate string is a number (perform > #1 or #2 casts) or throw an exception. > 4. Column(Long), Value(Long) => value as string > 5. Column(Long), Value(Integer) => value as string > 6. Column(Long), Value(String) => validate string is a number (perform #4 > or #5 casts) or throw an exception. > > I do see your point and this logic might be misplaced. Perhaps we can > move this to a validation layer in due time. > > Jarek Cecho wrote: > Fair enough, let's create a subsequent JIRA to discuss it further?
https://issues.apache.org/jira/browse/SQOOP-2280 - Abraham ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32588/#review78164 ----------------------------------------------------------- On March 27, 2015, 10:14 p.m., Abraham Elmahrek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32588/ > ----------------------------------------------------------- > > (Updated March 27, 2015, 10:14 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-2213 > https://issues.apache.org/jira/browse/SQOOP-2213 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > commit 4bee6231efff20cad14303628fc3a18db38ecc6b > Author: Abraham Elmahrek <[email protected]> > Date: Fri Mar 27 12:13:47 2015 -0700 > > SQOOP-2213: Sqoop2: toCSVFixedPoint ClassCastException > > :100644 100644 c460f80... 2a7aa1b... M > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopIDFUtils.java > :100644 100644 f99d1af... 0819f92... M > connector/connector-sdk/src/test/java/org/apache/sqoop/connector/common/TestSqoopIDFUtils.java > > > Diffs > ----- > > > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopIDFUtils.java > c460f80 > > connector/connector-sdk/src/test/java/org/apache/sqoop/connector/common/TestSqoopIDFUtils.java > f99d1af > > Diff: https://reviews.apache.org/r/32588/diff/ > > > Testing > ------- > > mvn test > > > Thanks, > > Abraham Elmahrek > >
