You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Keegan Witt <ke...@gmail.com> on 2015/04/17 16:11:14 UTC
Review Request 33308: SQOOP-2296
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33308/
-----------------------------------------------------------
Review request for Sqoop.
Repository: sqoop-trunk
Description
-------
SQOOP-2296 1st patch
Diffs
-----
src/java/org/apache/sqoop/mapreduce/sqlserver/SqlServerUpsertOutputFormat.java 0cb2c78
src/test/org/apache/sqoop/mapreduce/sqlserver/SqlServerUpsertOutputFormatTest.java a144e30
Diff: https://reviews.apache.org/r/33308/diff/
Testing
-------
Thanks,
Keegan Witt
Re: Review Request 33308: SQOOP-2296
Posted by Jarek Cecho <ja...@apache.org>.
> On April 20, 2015, 11:15 p.m., Jarek Cecho wrote:
> > src/test/org/apache/sqoop/mapreduce/sqlserver/SqlServerUpsertOutputFormatTest.java, lines 86-87
> > <https://reviews.apache.org/r/33308/diff/1/?file=933806#file933806line86>
> >
> > After reading the linked documentation (referenced on the JIRA) [1] it seems to me that the proper place where to put the table hints is on this line in form of "INTO table WITH $TABLE_HINTS AS x", right?
> >
> > Links:
> > 1: https://msdn.microsoft.com/en-us/library/bb510625.aspx
>
> Keegan Witt wrote:
> My understanding (and I'm not a SQL Server expert) is that those table hints only apply if you are merging FROM another table. However, one hint that would apply is the "WITH <merge_hint>", but that doesn't support the entire table hint vocabulary. Since the argument we currently have for table hints might contain NOLOCK or READUNCOMMITTED (which are unsupported) I felt to be safe, we would need a separate argument to support that. Should I open a separate Jira for that?
Fair enough Keegan, let's cover that in separate JIRA then.
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33308/#review80856
-----------------------------------------------------------
On April 17, 2015, 2:11 p.m., Keegan Witt wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33308/
> -----------------------------------------------------------
>
> (Updated April 17, 2015, 2:11 p.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> SQOOP-2296 1st patch
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/mapreduce/sqlserver/SqlServerUpsertOutputFormat.java 0cb2c78
> src/test/org/apache/sqoop/mapreduce/sqlserver/SqlServerUpsertOutputFormatTest.java a144e30
>
> Diff: https://reviews.apache.org/r/33308/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Keegan Witt
>
>
Re: Review Request 33308: SQOOP-2296
Posted by Keegan Witt <ke...@gmail.com>.
> On April 20, 2015, 7:15 p.m., Jarek Cecho wrote:
> > src/test/org/apache/sqoop/mapreduce/sqlserver/SqlServerUpsertOutputFormatTest.java, lines 86-87
> > <https://reviews.apache.org/r/33308/diff/1/?file=933806#file933806line86>
> >
> > After reading the linked documentation (referenced on the JIRA) [1] it seems to me that the proper place where to put the table hints is on this line in form of "INTO table WITH $TABLE_HINTS AS x", right?
> >
> > Links:
> > 1: https://msdn.microsoft.com/en-us/library/bb510625.aspx
My understanding (and I'm not a SQL Server expert) is that those table hints only apply if you are merging FROM another table. However, one hint that would apply is the "WITH <merge_hint>", but that doesn't support the entire table hint vocabulary. Since the argument we currently have for table hints might contain NOLOCK or READUNCOMMITTED (which are unsupported) I felt to be safe, we would need a separate argument to support that. Should I open a separate Jira for that?
- Keegan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33308/#review80856
-----------------------------------------------------------
On April 17, 2015, 10:11 a.m., Keegan Witt wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33308/
> -----------------------------------------------------------
>
> (Updated April 17, 2015, 10:11 a.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> SQOOP-2296 1st patch
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/mapreduce/sqlserver/SqlServerUpsertOutputFormat.java 0cb2c78
> src/test/org/apache/sqoop/mapreduce/sqlserver/SqlServerUpsertOutputFormatTest.java a144e30
>
> Diff: https://reviews.apache.org/r/33308/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Keegan Witt
>
>
Re: Review Request 33308: SQOOP-2296
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33308/#review80856
-----------------------------------------------------------
Thank you for finding the issue Keegan. I have one question:
src/test/org/apache/sqoop/mapreduce/sqlserver/SqlServerUpsertOutputFormatTest.java
<https://reviews.apache.org/r/33308/#comment130980>
After reading the linked documentation (referenced on the JIRA) [1] it seems to me that the proper place where to put the table hints is on this line in form of "INTO table WITH $TABLE_HINTS AS x", right?
Links:
1: https://msdn.microsoft.com/en-us/library/bb510625.aspx
Jarcec
- Jarek Cecho
On April 17, 2015, 2:11 p.m., Keegan Witt wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33308/
> -----------------------------------------------------------
>
> (Updated April 17, 2015, 2:11 p.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> SQOOP-2296 1st patch
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/mapreduce/sqlserver/SqlServerUpsertOutputFormat.java 0cb2c78
> src/test/org/apache/sqoop/mapreduce/sqlserver/SqlServerUpsertOutputFormatTest.java a144e30
>
> Diff: https://reviews.apache.org/r/33308/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Keegan Witt
>
>
Re: Review Request 33308: SQOOP-2296
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33308/#review80890
-----------------------------------------------------------
Ship it!
Ship It!
- Jarek Cecho
On April 17, 2015, 2:11 p.m., Keegan Witt wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33308/
> -----------------------------------------------------------
>
> (Updated April 17, 2015, 2:11 p.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> SQOOP-2296 1st patch
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/mapreduce/sqlserver/SqlServerUpsertOutputFormat.java 0cb2c78
> src/test/org/apache/sqoop/mapreduce/sqlserver/SqlServerUpsertOutputFormatTest.java a144e30
>
> Diff: https://reviews.apache.org/r/33308/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Keegan Witt
>
>