> On Nov. 18, 2014, 5 p.m., Jarek Cecho wrote: > > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java, > > lines 78-79 > > <https://reviews.apache.org/r/28139/diff/1/?file=766590#file766590line78> > > > > Can you plase document how is array expected to be coded up in the IDF? > > It seems that we don't have this in the code yet, so I would simply update > > the wiki: > > > > > > https://cwiki.apache.org/confluence/display/SQOOP/Sqoop2+Intermediate+representation#Sqoop2Intermediaterepresentation-Intermediateformatrepresentationproposal > > > > (We can move the IDF proposal from wiki to code in subsequent JIRA)
I have a ticket for that. I would rather keep this in one place https://issues.apache.org/jira/browse/SQOOP-1706 > On Nov. 18, 2014, 5 p.m., Jarek Cecho wrote: > > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java, > > line 99 > > <https://reviews.apache.org/r/28139/diff/1/?file=766590#file766590line99> > > > > I'm wondering if this one really needs to be public? it is default access so we can use it in unit tests, there is no need to be public yes > On Nov. 18, 2014, 5 p.m., Jarek Cecho wrote: > > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java, > > lines 21-62 > > <https://reviews.apache.org/r/28139/diff/1/?file=766590#file766590line21> > > > > Nit: Seems as unncecessary change in ordering. I used the IDE ordering, I am not sure if there a standard for this. > On Nov. 18, 2014, 5 p.m., Jarek Cecho wrote: > > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java, > > line 321 > > <https://reviews.apache.org/r/28139/diff/1/?file=766590#file766590line321> > > > > Why space as a separator for nested data types when we are already > > using comma as a separator? this is for the elements inside the array. We use comma between the 2 columns. Another alternative would be encode this as JSON string, which I might try as Abe suggested below. ['A' 'B'], 'text' > On Nov. 18, 2014, 5 p.m., Jarek Cecho wrote: > > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java, > > line 102 > > <https://reviews.apache.org/r/28139/diff/1/?file=766590#file766590line102> > > > > Based on the intermediate format proposal the correct Time format > > should be: HH:MM:DD[.ZZZZZZ][+/-XX] > > > > > > https://cwiki.apache.org/confluence/display/SQOOP/Sqoop2+Intermediate+representation#Sqoop2Intermediaterepresentation-Intermediateformatrepresentationproposal > > > > Can we add the missing parts? that is support this as Time? so what are some of the examples you would like to be tested.? would be easy for me to add those to test cases - Veena ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28139/#review62081 ----------------------------------------------------------- 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 > >
