You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Ying Cao <an...@gmail.com> on 2017/02/06 03:19:55 UTC
Re: Review Request 55132: support for DB2 XML data type when
importing to hdfs
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55132/
-----------------------------------------------------------
(Updated \u4e8c\u6708 6, 2017, 3:19 a.m.)
Review request for Sqoop, Abraham Elmahrek and Jarek Cecho.
Changes
-------
Hi Bogi,
Thanks for your comments again, name of "JAVA_STRING_DATA_TYPE" is more clean than before.
Bugs: SQOOP-1904
https://issues.apache.org/jira/browse/SQOOP-1904
Repository: sqoop-trunk
Description
-------
SQOOP-1904: support for DB2 XML data type when importing to hdfs
Diffs (updated)
-----
src/java/org/apache/sqoop/manager/Db2Manager.java e39aa4c
src/test/org/apache/sqoop/manager/db2/DB2XmlTypeImportManualTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/55132/diff/
Testing
-------
UT passed by manually
Thanks,
Ying Cao
Re: Review Request 55132: support for DB2 XML data type when
importing to hdfs
Posted by Ying Cao <an...@gmail.com>.
> On \u4e8c\u6708 8, 2017, 10:25 p.m., Abraham Elmahrek wrote:
> >
Hi Abraham Elmahrek,
Thanks for your comments
Angela
- Ying
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55132/#review164793
-----------------------------------------------------------
On \u4e8c\u6708 9, 2017, 3:39 a.m., Ying Cao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55132/
> -----------------------------------------------------------
>
> (Updated \u4e8c\u6708 9, 2017, 3:39 a.m.)
>
>
> Review request for Sqoop, Abraham Elmahrek and Jarek Cecho.
>
>
> Bugs: SQOOP-1904
> https://issues.apache.org/jira/browse/SQOOP-1904
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> SQOOP-1904: support for DB2 XML data type when importing to hdfs
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/Db2Manager.java e39aa4c
> src/test/org/apache/sqoop/manager/db2/DB2XmlTypeImportManualTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55132/diff/
>
>
> Testing
> -------
>
> UT passed by manually
>
>
> Thanks,
>
> Ying Cao
>
>
Re: Review Request 55132: support for DB2 XML data type when
importing to hdfs
Posted by Ying Cao <an...@gmail.com>.
> On \u4e8c\u6708 8, 2017, 10:25 p.m., Abraham Elmahrek wrote:
> > src/java/org/apache/sqoop/manager/Db2Manager.java, line 176
> > <https://reviews.apache.org/r/55132/diff/4/?file=1624607#file1624607line176>
> >
> > Is this a Hive specific type? Or a Java specific type?
Hi Abraham Elmahrek,
Thanks for your comments.
This is Java specific type
> On \u4e8c\u6708 8, 2017, 10:25 p.m., Abraham Elmahrek wrote:
> > src/test/org/apache/sqoop/manager/db2/DB2XmlTypeImportManualTest.java, lines 218-220
> > <https://reviews.apache.org/r/55132/diff/4/?file=1624608#file1624608line218>
> >
> > Is this necessary?
Thanks for your comments.
> On \u4e8c\u6708 8, 2017, 10:25 p.m., Abraham Elmahrek wrote:
> > src/test/org/apache/sqoop/manager/db2/DB2XmlTypeImportManualTest.java, line 215
> > <https://reviews.apache.org/r/55132/diff/4/?file=1624608#file1624608line215>
> >
> > You could just move the contents of this method into the test case method above
Thanks for comments!
This method will help for resued if other more testing scenario is need.
- Ying
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55132/#review164793
-----------------------------------------------------------
On \u4e8c\u6708 6, 2017, 3:19 a.m., Ying Cao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55132/
> -----------------------------------------------------------
>
> (Updated \u4e8c\u6708 6, 2017, 3:19 a.m.)
>
>
> Review request for Sqoop, Abraham Elmahrek and Jarek Cecho.
>
>
> Bugs: SQOOP-1904
> https://issues.apache.org/jira/browse/SQOOP-1904
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> SQOOP-1904: support for DB2 XML data type when importing to hdfs
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/Db2Manager.java e39aa4c
> src/test/org/apache/sqoop/manager/db2/DB2XmlTypeImportManualTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55132/diff/
>
>
> Testing
> -------
>
> UT passed by manually
>
>
> Thanks,
>
> Ying Cao
>
>
Re: Review Request 55132: support for DB2 XML data type when
importing to hdfs
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55132/#review164793
-----------------------------------------------------------
src/java/org/apache/sqoop/manager/Db2Manager.java (line 176)
<https://reviews.apache.org/r/55132/#comment236570>
Is this a Hive specific type? Or a Java specific type?
src/test/org/apache/sqoop/manager/db2/DB2XmlTypeImportManualTest.java (line 190)
<https://reviews.apache.org/r/55132/#comment236574>
Why not just use a static string or static array of strings?
src/test/org/apache/sqoop/manager/db2/DB2XmlTypeImportManualTest.java (line 215)
<https://reviews.apache.org/r/55132/#comment236575>
You could just move the contents of this method into the test case method above
src/test/org/apache/sqoop/manager/db2/DB2XmlTypeImportManualTest.java (lines 218 - 220)
<https://reviews.apache.org/r/55132/#comment236578>
Is this necessary?
- Abraham Elmahrek
On Feb. 6, 2017, 3:19 a.m., Ying Cao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55132/
> -----------------------------------------------------------
>
> (Updated Feb. 6, 2017, 3:19 a.m.)
>
>
> Review request for Sqoop, Abraham Elmahrek and Jarek Cecho.
>
>
> Bugs: SQOOP-1904
> https://issues.apache.org/jira/browse/SQOOP-1904
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> SQOOP-1904: support for DB2 XML data type when importing to hdfs
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/Db2Manager.java e39aa4c
> src/test/org/apache/sqoop/manager/db2/DB2XmlTypeImportManualTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55132/diff/
>
>
> Testing
> -------
>
> UT passed by manually
>
>
> Thanks,
>
> Ying Cao
>
>
Re: Review Request 55132: support for DB2 XML data type when
importing to hdfs
Posted by Attila Szabo <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55132/#review166660
-----------------------------------------------------------
Ship it!
Hey Ying Cao,
Both implementation and testing looks good. Thanks for your contrubtion here! (Hope more will come in the future!)
Regards,
Attila
- Attila Szabo
On Feb. 24, 2017, 6:56 a.m., Ying Cao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55132/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2017, 6:56 a.m.)
>
>
> Review request for Sqoop, Abraham Elmahrek and Jarek Cecho.
>
>
> Bugs: SQOOP-1904
> https://issues.apache.org/jira/browse/SQOOP-1904
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> SQOOP-1904: support for DB2 XML data type when importing to hdfs
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/Db2Manager.java e39aa4c
> src/test/org/apache/sqoop/manager/db2/DB2XmlTypeImportManualTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55132/diff/
>
>
> Testing
> -------
>
> UT passed by manually
>
>
> Thanks,
>
> Ying Cao
>
>
Re: Review Request 55132: support for DB2 XML data type when
importing to hdfs
Posted by Ying Cao <an...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55132/
-----------------------------------------------------------
(Updated \u4e8c\u6708 24, 2017, 6:56 a.m.)
Review request for Sqoop, Abraham Elmahrek and Jarek Cecho.
Changes
-------
Fix issue base on Szabolcs comments
Bugs: SQOOP-1904
https://issues.apache.org/jira/browse/SQOOP-1904
Repository: sqoop-trunk
Description
-------
SQOOP-1904: support for DB2 XML data type when importing to hdfs
Diffs (updated)
-----
src/java/org/apache/sqoop/manager/Db2Manager.java e39aa4c
src/test/org/apache/sqoop/manager/db2/DB2XmlTypeImportManualTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/55132/diff/
Testing
-------
UT passed by manually
Thanks,
Ying Cao
Re: Review Request 55132: support for DB2 XML data type when
importing to hdfs
Posted by Szabolcs Vasas <va...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55132/#review166509
-----------------------------------------------------------
Hi Ying,
Thank you for the patch! I think the code you added works correctly, however there are a few minor things need to be fixed. See my comments below.
Regards,
Szabolcs
src/java/org/apache/sqoop/manager/Db2Manager.java (line 53)
<https://reviews.apache.org/r/55132/#comment238469>
I think this constant should have a more expressive name, as it contains the Java type the XML type is mapped to.
src/java/org/apache/sqoop/manager/Db2Manager.java (line 131)
<https://reviews.apache.org/r/55132/#comment238470>
Can we move this field declaration above the constructor? That would be inline with Java code style.
src/java/org/apache/sqoop/manager/Db2Manager.java (line 168)
<https://reviews.apache.org/r/55132/#comment238471>
I am not sure the debug message would be correct here. The keySet() method returns a set which is not necessarily ordered, so the debug message can be misleading.
src/java/org/apache/sqoop/manager/Db2Manager.java (line 175)
<https://reviews.apache.org/r/55132/#comment238472>
You use startsWith instead of equals here. Do we have more XML data types?
src/test/org/apache/sqoop/manager/db2/DB2XmlTypeImportManualTest.java (line 213)
<https://reviews.apache.org/r/55132/#comment238473>
The test looks good to me, however then I try to run it it fails, because test.build.data does not seem to be defined on my machine. Can you please fix it?
- Szabolcs Vasas
On Feb. 9, 2017, 3:39 a.m., Ying Cao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55132/
> -----------------------------------------------------------
>
> (Updated Feb. 9, 2017, 3:39 a.m.)
>
>
> Review request for Sqoop, Abraham Elmahrek and Jarek Cecho.
>
>
> Bugs: SQOOP-1904
> https://issues.apache.org/jira/browse/SQOOP-1904
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> SQOOP-1904: support for DB2 XML data type when importing to hdfs
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/Db2Manager.java e39aa4c
> src/test/org/apache/sqoop/manager/db2/DB2XmlTypeImportManualTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55132/diff/
>
>
> Testing
> -------
>
> UT passed by manually
>
>
> Thanks,
>
> Ying Cao
>
>
Re: Review Request 55132: support for DB2 XML data type when
importing to hdfs
Posted by Ying Cao <an...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55132/
-----------------------------------------------------------
(Updated \u4e8c\u6708 9, 2017, 3:39 a.m.)
Review request for Sqoop, Abraham Elmahrek and Jarek Cecho.
Changes
-------
Update patch SQOOP-1904.6.patch wtih Abraham's comments
Bugs: SQOOP-1904
https://issues.apache.org/jira/browse/SQOOP-1904
Repository: sqoop-trunk
Description
-------
SQOOP-1904: support for DB2 XML data type when importing to hdfs
Diffs (updated)
-----
src/java/org/apache/sqoop/manager/Db2Manager.java e39aa4c
src/test/org/apache/sqoop/manager/db2/DB2XmlTypeImportManualTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/55132/diff/
Testing
-------
UT passed by manually
Thanks,
Ying Cao