> On Nov. 18, 2014, 7:02 p.m., Abraham Elmahrek wrote:
> > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java,
> >  line 168
> > <https://reviews.apache.org/r/28139/diff/1/?file=766590#file766590line168>
> >
> >     Not a big deal, but I prefer "getFields". It makes more sense in the 
> > context of CSV parsing.

fields is generic, All of the CSV code is between strings to objects ( 
getObjectData ) and vice versa ( setObjectData ). So making it explicit so it 
is clear that every field is internally kept as a string.


- Veena


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28139/#review62115
-----------------------------------------------------------


On Nov. 18, 2014, 11:52 a.m., Veena Basavaraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28139/
> -----------------------------------------------------------
> 
> (Updated Nov. 18, 2014, 11:52 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1749
>     https://issues.apache.org/jira/browse/SQOOP-1749
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> see JIRA
> 
> Supporting Nested arrays is going to be dody logic
> 
> some of the assumptions here in parsing need more docs. Esp use of special 
> characters to parse nested arrays.
> 
> Welcome feedback if I can improve this more.
> 
> I am sure mores tests can be added.
> 
> 
> NOTE: WS will be addressed once the patch is reviewed for functionality and 
> this needs a rebase since I have other patches pending review that overlap.
> 
> 
> Diffs
> -----
> 
>   
> common/src/main/java/org/apache/sqoop/schema/type/AbstractComplexListType.java
>  5d8f28b 
>   common/src/main/java/org/apache/sqoop/schema/type/Array.java af13eb7 
>   
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java
>  39a01c1 
>   
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/IntermediateDataFormat.java
>  5ef6fc6 
>   
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/IntermediateDataFormatError.java
>  4d41679 
>   
> connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestCSVIntermediateDataFormat.java
>  fcf6c3c 
> 
> Diff: https://reviews.apache.org/r/28139/diff/
> 
> 
> Testing
> -------
> 
> yes unit tests added. 
> 
> 
> Thanks,
> 
> Veena Basavaraj
> 
>

Reply via email to