sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jarek Cecho" <jar...@apache.org>
Subject Re: Review Request 32588: SQOOP-2213: Sqoop2: toCSVFixedPoint ClassCastException
Date Thu, 02 Apr 2015 20:04:59 GMT


> 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?
> 
> Abraham Elmahrek wrote:
>     https://issues.apache.org/jira/browse/SQOOP-2280

Thanks!


- Jarek


-----------------------------------------------------------
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 <abe@apache.org>
> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message