> On Aug. 8, 2015, 6:42 p.m., Jarek Cecho wrote:
> > Hi Thomas ,
> > thank you for the patch! I have two high level notes and several nits:
> > 
> > 1) I believe that we should document the new arguments in our user guide 
> > [1]. I would not hesitate to explain the use case with the WITH UR on DB2 
> > connector that you hit.
> > 2) I'm missing tests for the new functionality, would you mind adding some?
> > 
> > Links:
> > 1: https://github.com/apache/sqoop/tree/trunk/src/docs/user
> 
> Filippo Causa wrote:
>     thanks for your tips. Sorry but my name is not Thomas but Filippo. In the 
> next  days i will work to improve the patch to your specification.
>     
>     Filippo
> 
> Jarek Cecho wrote:
>     So embarassing, my huge apologies for messing your name Filippo!
> 
> Venkat Ranganathan wrote:
>     Just to point out, WITH UR is WITH UNCOMMITTED READ option and the same 
> effect can be had with --relaxed-isolation option.   Also this was specific 
> to DB2, supporting this syntax might be good to do in the DB2Manager?
> 
> Jarek Cecho wrote:
>     I was thinking about proposing change only to DB2Manager for the "WITH 
> UR", but then I've realized that adding custom prefixes and suffixes might be 
> useful to specify various flags in other databases as well. I do not have a 
> concerete example, but MySQL hints that are specified in /* */ can be entered 
> this way. Hence I feel that albeit this is super advanced feature, it could 
> be generally useful. What do you think Venkat?
> 
> Filippo Causa wrote:
>     I think --relaxed-isoltaion is good idea but working only in the mapper 
> step, so it would be made available also in the process of calculation of the 
> split and "select max()" . We could add both features ( isolation , suffix 
> and alias arguments) to  provide global capability. What  do you think?
> 
> Venkat Ranganathan wrote:
>     Hi Jarcec,  not sure if  MySQL Hints require that they have to be at the 
> end only (unlike the DB2 syntax).  This feature may generally be useful, but 
> not sure for this?  WITH UR option is specific to DB2 and may be easily 
> solved in DB2 connection manager or better yet we tell people not to use WITH 
> UR option but used --relaxed-isolation for queries?   --relaxed-isolation 
> also works with table imports (you don't have to craft a query for that).
>     Filippo Causa, good point about --relaxed-isolation not working with max 
> queries.  We should definitely fix those but generally DBs would handle this 
> but I agree,  better to fix Sqoop code to make sure this feature also works 
> for queries executed in the client
> 
> Filippo Causa wrote:
>     If we all agree isolation-level approach, i am fixing the code to add 
> relaxed-isolation in max queries and split queries. So we can close the jira 
> and suggest the use of relaxed-isolation parameters to work with db2 instead 
> of WITH UR. Then, we can open a new jira to add prefix,suffix and alias 
> arguments as new features. Prefix arguments can be useful for  statement like 
> "with v as (select * from t1) select a,b from t2 inner join v".
> 
> Jarek Cecho wrote:
>     Seems reasonable to me Filippo.
> 
> Filippo Causa wrote:
>     I have uploaded a new patch. Sorry if it's not right way to update the 
> review request.
> 
> Venkat Ranganathan wrote:
>     Agreed - that is the reasonable approach.  Thanks

Thanks Filippo for the updated patch. I believe that the agreement is to make 
this change specific to DB2 connector (e.g. add extra argument --with-ur or 
something) and take the general approach to separate JIRA, correct?


- Jarek


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


On Aug. 17, 2015, 9:48 a.m., Filippo Causa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36729/
> -----------------------------------------------------------
> 
> (Updated Aug. 17, 2015, 9:48 a.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> propagated  relaxed-isolation parameter setting to connection used for 
> calculating max incremental value and split range. Use --relaxed-isoltaion 
> inbstead of "WITH UR" clause.
> 
> 
> Diffs
> -----
> 
>   src/java/com/cloudera/sqoop/mapreduce/db/DBConfiguration.java 89f2b4f 
>   src/java/com/cloudera/sqoop/tool/BaseSqoopTool.java a5f72f7 
>   src/java/org/apache/sqoop/SqoopOptions.java 9405605 
>   src/java/org/apache/sqoop/hive/TableDefWriter.java c9962e9 
>   src/java/org/apache/sqoop/manager/MySQLManager.java e1d5a36 
>   src/java/org/apache/sqoop/manager/OracleManager.java 69b613f 
>   src/java/org/apache/sqoop/mapreduce/DataDrivenImportJob.java 260bc29 
>   src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 93d438a 
>   src/java/org/apache/sqoop/mapreduce/db/DBConfiguration.java a9b7e42 
>   src/java/org/apache/sqoop/mapreduce/db/DBInputFormat.java 3a8e5d0 
>   src/java/org/apache/sqoop/mapreduce/db/DBRecordReader.java a78eb06 
>   src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBInputFormat.java db96e41 
>   src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBRecordReader.java 
> b734e05 
>   src/java/org/apache/sqoop/mapreduce/db/OracleDBRecordReader.java 9058a55 
>   src/java/org/apache/sqoop/mapreduce/hcat/SqoopHCatUtilities.java 4070c24 
>   src/java/org/apache/sqoop/mapreduce/sqlserver/SqlServerRecordReader.java 
> bc101c5 
>   src/java/org/apache/sqoop/orm/ClassWriter.java 1c6f7f4 
>   src/java/org/apache/sqoop/tool/BaseSqoopTool.java 4e2e66d 
>   src/java/org/apache/sqoop/tool/ImportTool.java 39af42c 
> 
> Diff: https://reviews.apache.org/r/36729/diff/
> 
> 
> Testing
> -------
> 
> Test done with jdbc type 4
> 
> 
> File Attachments
> ----------------
> 
> Transaction-Isolation Patch
>   
> https://reviews.apache.org/media/uploaded/files/2015/08/13/9ebd4663-592d-448f-a888-94ee00ca84de__1096.patch
> 
> 
> Thanks,
> 
> Filippo Causa
> 
>

Reply via email to