> On July 3, 2014, 2:16 a.m., Jarek Cecho wrote: > > Nice patch Abe, thank you! > > > > Quick question - do you think that it would make sense to add an > > integration test case that will try to create table with enough columns to > > actually cause the exception without this patch, just to ensure that we > > won't regress here in the future?
Yep. > On July 3, 2014, 2:16 a.m., Jarek Cecho wrote: > > src/java/org/apache/sqoop/orm/ClassWriter.java, line 165 > > <https://reviews.apache.org/r/23201/diff/2/?file=621396#file621396line165> > > > > Nit: Can we make the default value a constant defined at the begging of > > the class? Doesn't it make sense to make this configurable in case 500 is too few or too many? > On July 3, 2014, 2:16 a.m., Jarek Cecho wrote: > > src/java/org/apache/sqoop/orm/ClassWriter.java, lines 577-589 > > <https://reviews.apache.org/r/23201/diff/2/?file=621396#file621396line577> > > > > Do you think that it would make sense for case where numberOfMethods = > > 1 (majority of cases I assume) to bypass the second method call? Indeed it does. - Abraham ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23201/#review47279 ----------------------------------------------------------- On July 1, 2014, 5:09 p.m., Abraham Elmahrek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23201/ > ----------------------------------------------------------- > > (Updated July 1, 2014, 5:09 p.m.) > > > Review request for Sqoop, Gwen Shapira and Jarek Cecho. > > > Bugs: SQOOP-1239 > https://issues.apache.org/jira/browse/SQOOP-1239 > > > Repository: sqoop-trunk > > > Description > ------- > > commit 4cb6a0406b33272b97265a29d56fdb4f2194bcbe > Author: Abraham Elmahrek <[email protected]> > Date: Mon Jun 30 19:24:47 2014 -0700 > > SQOOP-1239 Sqoop import code too large error > > Too many columns baloon the size of the method. > Splitting methods up will enable these use cases. > Here are a few limitations that someone could run into: > - Max # of methods: 65535. > - Max # of fields: 65535. > > :100644 100644 df1ab72... a56e167... M > src/java/org/apache/sqoop/orm/ClassWriter.java > > > Diffs > ----- > > src/java/org/apache/sqoop/orm/ClassWriter.java df1ab72 > > Diff: https://reviews.apache.org/r/23201/diff/ > > > Testing > ------- > > + Tested manually by importing a 2000 column table from MySQL MyISAM. > + Ran tests. > > > Thanks, > > Abraham Elmahrek > >
