----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1946/#review2034 -----------------------------------------------------------
Jarek, the changes look good. Regarding the test case problem you have mentioned, I don't think that it is really a test case issue. On the contrary, it seems to be a bug to me. Here is why: the purpose of boundary query is to limit the overall size of ingest from the database. It does not and should not matter how many mappers are used to perform the ingest. You can fix this by modifying the DataDrivenDBInputFormat.getSplits() method and ensuring that the single split it generates for the one mapper case uses the boundary query if specified. /src/docs/user/import.txt <https://reviews.apache.org/r/1946/#comment4582> nit:The BOUNDARY need not be all capitalized. /src/docs/user/import.txt <https://reviews.apache.org/r/1946/#comment4583> Please remove the trailing whitespace. /src/docs/user/import.txt <https://reviews.apache.org/r/1946/#comment4584> Trailing whitespace. - Arvind On 2011-09-20 08:29:02, Jarek Jarcec wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/1946/ > ----------------------------------------------------------- > > (Updated 2011-09-20 08:29:02) > > > Review request for Sqoop and Arvind Prabhakar. > > > Summary > ------- > > I've incorporated all Arvind's suggestions (hopefully :-)). > > > This addresses bug SQOOP-331. > https://issues.apache.org/jira/browse/SQOOP-331 > > > Diffs > ----- > > /src/docs/man/import-args.txt 1171925 > /src/docs/user/import.txt 1171925 > /src/java/com/cloudera/sqoop/SqoopOptions.java 1171925 > /src/java/com/cloudera/sqoop/manager/SqlManager.java 1171925 > /src/java/com/cloudera/sqoop/mapreduce/DataDrivenImportJob.java 1171925 > /src/java/com/cloudera/sqoop/tool/BaseSqoopTool.java 1171925 > /src/java/com/cloudera/sqoop/tool/ImportTool.java 1171925 > /src/test/com/cloudera/sqoop/TestSqoopOptions.java 1171925 > > Diff: https://reviews.apache.org/r/1946/diff > > > Testing > ------- > > I'm still having troubles to create meaningful tests for this patch. I've > came up with two different approaches, but I wasn't able to get running > either of them: > > 1) Use boundary query for limiting import data (like "select 1, 2"). This is > totally wrong usage of this parameter, but I was thinking that It might be > fine for the testing purpose. Unfortunately underlying code is using this > query only in case that is creating more than one map task and I was not able > to forced it create more than one. Which make sense because the -m parameter > is also only a hint. > > 2) Parse logs. Fortunately class responsible for creating splits is printing > used boundary query, so there is possibility to parse those logs and look for > used boundary query. But I'm not sure how this can be done in proper fashion. > > Any ideas will be welcomed. > > Jarcec > > > Thanks, > > Jarek > >
