> On Nov. 18, 2014, 1:30 p.m., Abraham Elmahrek wrote:
> > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java,
> >  lines 183-189
> > <https://reviews.apache.org/r/28139/diff/1/?file=766590#file766590line183>
> >
> >     Should vary based on 'escaped'.
> 
> Jarek Cecho wrote:
>     I would advise not to. IDF is internal representation and I think that we 
> should be as strict as possible without a lot of conditional formating. The 
> "escaped" is more a matter of how it should look like on the output (or 
> input).
> 
> Abraham Elmahrek wrote:
>     Perhaps I don't fully understand this section of code or the argument. If 
> the IDF receives a CSV record, it some times has to parse it (hence the CSV 
> parser). The normal producer of CSV records are extractors. Isn't it entirely 
> possible that this particular of this section of code will be parsing data 
> provided by the extractors? In that case, not honoring quotes would be a 
> problem.

this is how I see it, 

if you see an example such as GenericJDBC and how it uses the IDF it is clear.

IDF has a schema and schema define column types, for a connector only this 
matters. 

How we represent the data representing each row is an internal impelentation. 
Today we have one of it that is storing a row as a comma separated list, and 
the schema maintains what each comman separated value is.

so  by csv records if you mean the objects array, yes connectors create it and 
call setObjectData. set obejct data encodes these obejcts when it is converting 
to a string.

so v''eena might be encoded to make sure we escape the quotes within.


the above  piece of code that we are commenting about is used to convert the 
string back to the objects. 

Now if we put all complex types inside the { } using the toJSONString then we 
should neither encode not decode what is inside it.


- Veena


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


On Nov. 19, 2014, 9:08 a.m., Veena Basavaraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28139/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2014, 9:08 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
> -----
> 
>   
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java
>  39a01c1b5b290903a6c93dbb8427af525515e2b2 
>   
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/IntermediateDataFormat.java
>  5ef6fc64ec695fbdf2f9b6acec1eb0235fd675be 
>   
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/IntermediateDataFormatError.java
>  4d41679d0b4b9e04d05eef4dd5280f201bbe58e2 
>   
> connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestCSVIntermediateDataFormat.java
>  fcf6c3c15c6558cf21a0d82c78a51903d8fb371c 
> 
> Diff: https://reviews.apache.org/r/28139/diff/
> 
> 
> Testing
> -------
> 
> yes unit tests added. 
> 
> 
> File Attachments
> ----------------
> 
> SQOOP-1749-v2.patch
>   
> https://reviews.apache.org/media/uploaded/files/2014/11/19/cfdfc0ed-57c9-40d3-959b-f482f1600c03__SQOOP-1749-v2.patch
> 
> 
> Thanks,
> 
> Veena Basavaraj
> 
>

Reply via email to