sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Abraham Elmahrek" <...@cloudera.com>
Subject Re: Review Request 23201: Sqoop import code too large error
Date Mon, 07 Jul 2014 18:05:24 GMT


> 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 <abraham@elmahrek.com>
> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message