Venkat Ranganathan
2013-03-13, 19:18
Jarek Cecho
2013-03-15, 04:38
Venkat Ranganathan
2013-03-15, 04:46
Venkat Ranganathan
2013-03-19, 21:25
Jarek Cecho
2013-03-20, 21:30
Venkat Ranganathan
2013-03-19, 16:45
Venkat Ranganathan
2013-03-19, 18:06
Venkat Ranganathan
2013-03-21, 00:08
Jarek Cecho
2013-03-22, 03:22
Jarek Cecho
2013-03-22, 03:24
Jarek Cecho
2013-03-20, 21:16
Venkat Ranganathan
2013-03-21, 03:41
Jarek Cecho
2013-03-21, 02:17
Venkat Ranganathan
2013-03-21, 01:38
Jarek Cecho
2013-03-21, 00:41
Venkat Ranganathan
2013-03-20, 23:48
Venkat Ranganathan
2013-03-20, 23:43
Jarek Cecho
2013-03-22, 03:21
Venkat Ranganathan
2013-03-21, 18:14
Venkat Ranganathan
2013-03-22, 03:29
Venkat Ranganathan
2013-03-21, 04:34
Venkat Ranganathan
2013-03-21, 04:34
Venkat Ranganathan
2013-03-22, 03:40
Jarek Cecho
2013-03-22, 03:52
Venkat Ranganathan
2013-04-15, 14:46
-
Review Request: Fix for SQOOP-937Venkat Ranganathan 2013-03-13, 19:18
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9909/
-----------------------------------------------------------
Review request for Sqoop and Jarek Cecho.
Description
-------
SQOOP generates an ORM file that represents a record in a table for a
given connector implememntation. The generated class has methods to
read field values off ResultSet, set bind values in PreparedStatements,
handle LOB objects in a DB specific way (that the connector represents),
parse input fields etc.
This file is then compiled and archived using jar and used with the SQOOP job.
This ORM instance is generated in all cases, and used to make sure that
the data is read and processed in a more or less uniform way.
Unfortunately, this generated class is not used by a class of connectors which
manage the reading and processing of records themselves. In essence, the
whole ORM class is unusable in these instances and is simply ignored. These
are typically "direct" connectors which use a DB specific highspeed path.
The generation of the ORM class in these cases causes confusion to users.
This patch tries to solve this by
1) Providing a capability for the connection managers to declare that
they don't depend on the ORM jar file.
2) Generating a dummy ORM jar file with explicit message during generation
so that
a) users are aware that the class generated is a dummy one
b) there is a record that this jar file was generated explicitly
c) we don't have to change a whole lot of the codebase to disable
the generaration and loading of the jar file.
This patch also adds one test.
Diffs
-----
src/java/org/apache/sqoop/manager/ConnManager.java 1b32dc9
src/java/org/apache/sqoop/manager/DirectNetezzaManager.java 0a1e605
src/java/org/apache/sqoop/orm/ClassWriter.java 136982c
src/java/org/apache/sqoop/tool/CodeGenTool.java 8a4aa42
src/test/com/cloudera/sqoop/orm/TestClassWriter.java 3b77571
Diff: https://reviews.apache.org/r/9909/diff/
Testing
-------
Added one tests. All unit tests and check style tests passed with no new checkstyle issues
Thanks,
Venkat Ranganathan
+
Venkat Ranganathan 2013-03-13, 19:18
-
Re: Review Request: Fix for SQOOP-937Jarek Cecho 2013-03-15, 04:38
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9909/#review17947
-----------------------------------------------------------
Hi Venkat,
thank you very much for cleaning this up. What about taking the idea even further by skipping entire class generation and compilation in this case? I think that some of the classes requires valid class name, but we can put dummy class to the source code directly right? There shouldn't be need to generate and compile it every time. What do you think?
Jarcec
- Jarek Cecho
On March 13, 2013, 7:18 p.m., Venkat Ranganathan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9909/
> -----------------------------------------------------------
>
> (Updated March 13, 2013, 7:18 p.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Description
> -------
>
> SQOOP generates an ORM file that represents a record in a table for a
> given connector implememntation. The generated class has methods to
> read field values off ResultSet, set bind values in PreparedStatements,
> handle LOB objects in a DB specific way (that the connector represents),
> parse input fields etc.
>
> This file is then compiled and archived using jar and used with the SQOOP job.
>
> This ORM instance is generated in all cases, and used to make sure that
> the data is read and processed in a more or less uniform way.
>
> Unfortunately, this generated class is not used by a class of connectors which
> manage the reading and processing of records themselves. In essence, the
> whole ORM class is unusable in these instances and is simply ignored. These
> are typically "direct" connectors which use a DB specific highspeed path.
>
> The generation of the ORM class in these cases causes confusion to users.
>
> This patch tries to solve this by
>
> 1) Providing a capability for the connection managers to declare that
> they don't depend on the ORM jar file.
> 2) Generating a dummy ORM jar file with explicit message during generation
> so that
> a) users are aware that the class generated is a dummy one
> b) there is a record that this jar file was generated explicitly
> c) we don't have to change a whole lot of the codebase to disable
> the generaration and loading of the jar file.
>
>
> This patch also adds one test.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/ConnManager.java 1b32dc9
> src/java/org/apache/sqoop/manager/DirectNetezzaManager.java 0a1e605
> src/java/org/apache/sqoop/orm/ClassWriter.java 136982c
> src/java/org/apache/sqoop/tool/CodeGenTool.java 8a4aa42
> src/test/com/cloudera/sqoop/orm/TestClassWriter.java 3b77571
>
> Diff: https://reviews.apache.org/r/9909/diff/
>
>
> Testing
> -------
>
> Added one tests. All unit tests and check style tests passed with no new checkstyle issues
>
>
> Thanks,
>
> Venkat Ranganathan
>
>
+
Jarek Cecho 2013-03-15, 04:38
-
Re: Review Request: Fix for SQOOP-937Venkat Ranganathan 2013-03-15, 04:46
> On March 15, 2013, 4:38 a.m., Jarek Cecho wrote:
> > Hi Venkat,
> > thank you very much for cleaning this up. What about taking the idea even further by skipping entire class generation and compilation in this case? I think that some of the classes requires valid class name, but we can put dummy class to the source code directly right? There shouldn't be need to generate and compile it every time. What do you think?
> >
> > Jarcec
Good point Jarcec. The reason I did not go the full step was there were a bunch of places we expect the jar (even if I say no jar file by returning null from ORM generator), the loadJar files used in a few places expect non-null strings. I initially went down that path and decided against it as this has a record of why we generated this.
I think your optimization makes sense (keep it generated in the src tree). That should be good to do and may be we can then get rid of the loadJar calls if the Jar file is null.
Thanks
Venkat
- Venkat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9909/#review17947
-----------------------------------------------------------
On March 13, 2013, 7:18 p.m., Venkat Ranganathan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9909/
> -----------------------------------------------------------
>
> (Updated March 13, 2013, 7:18 p.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Description
> -------
>
> SQOOP generates an ORM file that represents a record in a table for a
> given connector implememntation. The generated class has methods to
> read field values off ResultSet, set bind values in PreparedStatements,
> handle LOB objects in a DB specific way (that the connector represents),
> parse input fields etc.
>
> This file is then compiled and archived using jar and used with the SQOOP job.
>
> This ORM instance is generated in all cases, and used to make sure that
> the data is read and processed in a more or less uniform way.
>
> Unfortunately, this generated class is not used by a class of connectors which
> manage the reading and processing of records themselves. In essence, the
> whole ORM class is unusable in these instances and is simply ignored. These
> are typically "direct" connectors which use a DB specific highspeed path.
>
> The generation of the ORM class in these cases causes confusion to users.
>
> This patch tries to solve this by
>
> 1) Providing a capability for the connection managers to declare that
> they don't depend on the ORM jar file.
> 2) Generating a dummy ORM jar file with explicit message during generation
> so that
> a) users are aware that the class generated is a dummy one
> b) there is a record that this jar file was generated explicitly
> c) we don't have to change a whole lot of the codebase to disable
> the generaration and loading of the jar file.
>
>
> This patch also adds one test.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/ConnManager.java 1b32dc9
> src/java/org/apache/sqoop/manager/DirectNetezzaManager.java 0a1e605
> src/java/org/apache/sqoop/orm/ClassWriter.java 136982c
> src/java/org/apache/sqoop/tool/CodeGenTool.java 8a4aa42
> src/test/com/cloudera/sqoop/orm/TestClassWriter.java 3b77571
>
> Diff: https://reviews.apache.org/r/9909/diff/
>
>
> Testing
> -------
>
> Added one tests. All unit tests and check style tests passed with no new checkstyle issues
>
>
> Thanks,
>
> Venkat Ranganathan
>
>
+
Venkat Ranganathan 2013-03-15, 04:46
-
Re: Review Request: Fix for SQOOP-937Venkat Ranganathan 2013-03-19, 21:25
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9909/
-----------------------------------------------------------
(Updated March 19, 2013, 9:25 p.m.)
Review request for Sqoop and Jarek Cecho.
Changes
-------
Removed the generation of the ORM files completely for the cases mentioned. Right now DirectNetezzaManager is the only exercising it as a sample. Once this is goes in, we can update all the similar managers to avoid the generation of unnecessary ORM files. The earlier approach of generating a dummy file had its merits in some cases, but this one elmininates the generation completely.
Ran all tests, Netezza direct mode tests to validate the functionality and added unit tests for new functionalty
Description
-------
SQOOP generates an ORM file that represents a record in a table for a
given connector implememntation. The generated class has methods to
read field values off ResultSet, set bind values in PreparedStatements,
handle LOB objects in a DB specific way (that the connector represents),
parse input fields etc.
This file is then compiled and archived using jar and used with the SQOOP job.
This ORM instance is generated in all cases, and used to make sure that
the data is read and processed in a more or less uniform way.
Unfortunately, this generated class is not used by a class of connectors which
manage the reading and processing of records themselves. In essence, the
whole ORM class is unusable in these instances and is simply ignored. These
are typically "direct" connectors which use a DB specific highspeed path.
The generation of the ORM class in these cases causes confusion to users.
This patch tries to solve this by
1) Providing a capability for the connection managers to declare that
they don't depend on the ORM jar file.
2) Generating a dummy ORM jar file with explicit message during generation
so that
a) users are aware that the class generated is a dummy one
b) there is a record that this jar file was generated explicitly
c) we don't have to change a whole lot of the codebase to disable
the generaration and loading of the jar file.
This patch also adds one test.
Diffs (updated)
-----
src/java/org/apache/sqoop/manager/ConnManager.java 1b32dc9
src/java/org/apache/sqoop/manager/DirectNetezzaManager.java 0a1e605
src/java/org/apache/sqoop/manager/ExportJobContext.java 5699e2f
src/java/org/apache/sqoop/manager/ImportJobContext.java 09a7abe
src/java/org/apache/sqoop/mapreduce/ExportJobBase.java ff84974
src/java/org/apache/sqoop/mapreduce/ImportJobBase.java f766532
src/java/org/apache/sqoop/mapreduce/JobBase.java 4e7723f
src/java/org/apache/sqoop/orm/ClassWriter.java 982e444
src/java/org/apache/sqoop/tool/CodeGenTool.java 8a4aa42
src/test/com/cloudera/sqoop/orm/TestClassWriter.java 3b77571
Diff: https://reviews.apache.org/r/9909/diff/
Testing
-------
Added one tests. All unit tests and check style tests passed with no new checkstyle issues
Thanks,
Venkat Ranganathan
+
Venkat Ranganathan 2013-03-19, 21:25
-
Re: Review Request: Fix for SQOOP-937Jarek Cecho 2013-03-20, 21:30
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9909/#review18176
-----------------------------------------------------------
Ship it!
Ship It!
- Jarek Cecho
On March 19, 2013, 9:25 p.m., Venkat Ranganathan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9909/
> -----------------------------------------------------------
>
> (Updated March 19, 2013, 9:25 p.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Description
> -------
>
> SQOOP generates an ORM file that represents a record in a table for a
> given connector implememntation. The generated class has methods to
> read field values off ResultSet, set bind values in PreparedStatements,
> handle LOB objects in a DB specific way (that the connector represents),
> parse input fields etc.
>
> This file is then compiled and archived using jar and used with the SQOOP job.
>
> This ORM instance is generated in all cases, and used to make sure that
> the data is read and processed in a more or less uniform way.
>
> Unfortunately, this generated class is not used by a class of connectors which
> manage the reading and processing of records themselves. In essence, the
> whole ORM class is unusable in these instances and is simply ignored. These
> are typically "direct" connectors which use a DB specific highspeed path.
>
> The generation of the ORM class in these cases causes confusion to users.
>
> This patch tries to solve this by
>
> 1) Providing a capability for the connection managers to declare that
> they don't depend on the ORM jar file.
> 2) Generating a dummy ORM jar file with explicit message during generation
> so that
> a) users are aware that the class generated is a dummy one
> b) there is a record that this jar file was generated explicitly
> c) we don't have to change a whole lot of the codebase to disable
> the generaration and loading of the jar file.
>
>
> This patch also adds one test.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/ConnManager.java 1b32dc9
> src/java/org/apache/sqoop/manager/DirectNetezzaManager.java 0a1e605
> src/java/org/apache/sqoop/manager/ExportJobContext.java 5699e2f
> src/java/org/apache/sqoop/manager/ImportJobContext.java 09a7abe
> src/java/org/apache/sqoop/mapreduce/ExportJobBase.java ff84974
> src/java/org/apache/sqoop/mapreduce/ImportJobBase.java f766532
> src/java/org/apache/sqoop/mapreduce/JobBase.java 4e7723f
> src/java/org/apache/sqoop/orm/ClassWriter.java 982e444
> src/java/org/apache/sqoop/tool/CodeGenTool.java 8a4aa42
> src/test/com/cloudera/sqoop/orm/TestClassWriter.java 3b77571
>
> Diff: https://reviews.apache.org/r/9909/diff/
>
>
> Testing
> -------
>
> Added one tests. All unit tests and check style tests passed with no new checkstyle issues
>
>
> Thanks,
>
> Venkat Ranganathan
>
>
+
Jarek Cecho 2013-03-20, 21:30
-
Review Request: Fix for SQOOP-932Venkat Ranganathan 2013-03-19, 16:45
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10018/
-----------------------------------------------------------
Review request for Sqoop and Jarek Cecho.
Description
-------
Fixes for SQOOP-932
Diffs
-----
src/java/org/apache/sqoop/lib/DelimiterSet.java ef62ba0
src/java/org/apache/sqoop/manager/DirectNetezzaManager.java 0a1e605
src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java 410a569
src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java 9e6cab6
src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableExportJob.java 2a702d9
src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableImportJob.java 7ee6f70
src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java 938ffc5
src/test/com/cloudera/sqoop/manager/NetezzaExportManualTest.java 50d27fe
src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java 3482dd8
Diff: https://reviews.apache.org/r/10018/diff/
Testing
-------
All unit tests pass - added more tests for testing this functionality
Thanks,
Venkat Ranganathan
+
Venkat Ranganathan 2013-03-19, 16:45
-
Re: Review Request: Fix for SQOOP-932Venkat Ranganathan 2013-03-19, 18:06
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10018/
-----------------------------------------------------------
(Updated March 19, 2013, 6:06 p.m.)
Review request for Sqoop and Jarek Cecho.
Changes
-------
This time with checkstyle fixes! Sorry about that
Description
-------
Fixes for SQOOP-932
Diffs (updated)
-----
src/java/org/apache/sqoop/lib/DelimiterSet.java ef62ba0
src/java/org/apache/sqoop/manager/DirectNetezzaManager.java 0a1e605
src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java 410a569
src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java 9e6cab6
src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableExportJob.java 2a702d9
src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableImportJob.java 7ee6f70
src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java 938ffc5
src/test/com/cloudera/sqoop/manager/NetezzaExportManualTest.java 50d27fe
src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java 3482dd8
Diff: https://reviews.apache.org/r/10018/diff/
Testing
-------
All unit tests pass - added more tests for testing this functionality
Thanks,
Venkat Ranganathan
+
Venkat Ranganathan 2013-03-19, 18:06
-
Re: Review Request: Fix for SQOOP-932Venkat Ranganathan 2013-03-21, 00:08
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10018/
-----------------------------------------------------------
(Updated March 21, 2013, 12:08 a.m.)
Review request for Sqoop and Jarek Cecho.
Changes
-------
Updated patch with trunk changes
Description
-------
Fixes for SQOOP-932
Diffs (updated)
-----
src/java/org/apache/sqoop/lib/DelimiterSet.java ef62ba0
src/java/org/apache/sqoop/manager/DirectNetezzaManager.java ef98936
src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java 410a569
src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java 9e6cab6
src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableExportJob.java 2a702d9
src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableImportJob.java 7ee6f70
src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java 938ffc5
src/test/com/cloudera/sqoop/manager/NetezzaExportManualTest.java 50d27fe
src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java 3482dd8
Diff: https://reviews.apache.org/r/10018/diff/
Testing
-------
All unit tests pass - added more tests for testing this functionality
Thanks,
Venkat Ranganathan
+
Venkat Ranganathan 2013-03-21, 00:08
-
Re: Review Request: Fix for SQOOP-932Jarek Cecho 2013-03-22, 03:22
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10018/#review18236
-----------------------------------------------------------
Ship it!
Ship It!
- Jarek Cecho
On March 21, 2013, 12:08 a.m., Venkat Ranganathan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10018/
> -----------------------------------------------------------
>
> (Updated March 21, 2013, 12:08 a.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Description
> -------
>
> Fixes for SQOOP-932
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/lib/DelimiterSet.java ef62ba0
> src/java/org/apache/sqoop/manager/DirectNetezzaManager.java ef98936
> src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java 410a569
> src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java 9e6cab6
> src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableExportJob.java 2a702d9
> src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableImportJob.java 7ee6f70
> src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java 938ffc5
> src/test/com/cloudera/sqoop/manager/NetezzaExportManualTest.java 50d27fe
> src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java 3482dd8
>
> Diff: https://reviews.apache.org/r/10018/diff/
>
>
> Testing
> -------
>
> All unit tests pass - added more tests for testing this functionality
>
>
> Thanks,
>
> Venkat Ranganathan
>
>
+
Jarek Cecho 2013-03-22, 03:22
-
Re: Review Request: Fix for SQOOP-932Jarek Cecho 2013-03-22, 03:24
> On March 22, 2013, 3:22 a.m., Jarek Cecho wrote:
> > Ship It!
Would you mind attaching the final patch to the JIRA, Venkat?
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10018/#review18236
-----------------------------------------------------------
On March 21, 2013, 12:08 a.m., Venkat Ranganathan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10018/
> -----------------------------------------------------------
>
> (Updated March 21, 2013, 12:08 a.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Description
> -------
>
> Fixes for SQOOP-932
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/lib/DelimiterSet.java ef62ba0
> src/java/org/apache/sqoop/manager/DirectNetezzaManager.java ef98936
> src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java 410a569
> src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java 9e6cab6
> src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableExportJob.java 2a702d9
> src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableImportJob.java 7ee6f70
> src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java 938ffc5
> src/test/com/cloudera/sqoop/manager/NetezzaExportManualTest.java 50d27fe
> src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java 3482dd8
>
> Diff: https://reviews.apache.org/r/10018/diff/
>
>
> Testing
> -------
>
> All unit tests pass - added more tests for testing this functionality
>
>
> Thanks,
>
> Venkat Ranganathan
>
>
+
Jarek Cecho 2013-03-22, 03:24
-
Re: Review Request: Fix for SQOOP-932Jarek Cecho 2013-03-20, 21:16
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10018/#review18174
-----------------------------------------------------------
Hi Venkat,
good job on this patch. I do have one question:
src/java/org/apache/sqoop/manager/DirectNetezzaManager.java
<https://reviews.apache.org/r/10018/#comment38344>
My Netezza knowledge is a bit rusty these day, but I do have feeling that the external table parameter "NULLVALUE" is used only for string based columns (varchar, ...). For all other column types (int, float, ...) empty string is used to encode NULL value. On precondition that this is still the case, shouldn't the condition be more if(nullNonStrValue != null) { error; }?
Jarcec
- Jarek Cecho
On March 19, 2013, 6:06 p.m., Venkat Ranganathan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10018/
> -----------------------------------------------------------
>
> (Updated March 19, 2013, 6:06 p.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Description
> -------
>
> Fixes for SQOOP-932
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/lib/DelimiterSet.java ef62ba0
> src/java/org/apache/sqoop/manager/DirectNetezzaManager.java 0a1e605
> src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java 410a569
> src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java 9e6cab6
> src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableExportJob.java 2a702d9
> src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableImportJob.java 7ee6f70
> src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java 938ffc5
> src/test/com/cloudera/sqoop/manager/NetezzaExportManualTest.java 50d27fe
> src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java 3482dd8
>
> Diff: https://reviews.apache.org/r/10018/diff/
>
>
> Testing
> -------
>
> All unit tests pass - added more tests for testing this functionality
>
>
> Thanks,
>
> Venkat Ranganathan
>
>
+
Jarek Cecho 2013-03-20, 21:16
-
Re: Review Request: Fix for SQOOP-932Venkat Ranganathan 2013-03-21, 03:41
> On March 20, 2013, 9:16 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/DirectNetezzaManager.java, lines 78-83
> > <https://reviews.apache.org/r/10018/diff/2/?file=271931#file271931line78>
> >
> > My Netezza knowledge is a bit rusty these day, but I do have feeling that the external table parameter "NULLVALUE" is used only for string based columns (varchar, ...). For all other column types (int, float, ...) empty string is used to encode NULL value. On precondition that this is still the case, shouldn't the condition be more if(nullNonStrValue != null) { error; }?
>
> Venkat Ranganathan wrote:
> Thanks Jarcec
>
> The Netezza external table feature supports NULLVALUE for nonstring columns also. The unit tests tests it with an Integer column if you see.
>
> Thanks
> Venkat
>
> Jarek Cecho wrote:
> Hi Venkat,
> thank you for your feedback. I was actually looking at that particular test before, but I came to a conclusion that it's passing simply because "\N" is invalid value for integer based column. To verify my theory, I've added another test to DirectNetezzaExportManualTest that is using string "1" for null escape character (e.g. value that is valid for integer based column):
>
> @Test
> public void testNullStringExport2() throws Exception {
>
> String [] extraArgs = {
> "--input-null-string", "1",
> "--input-null-non-string", "1",
> "--input-escaped-by", "\\",
> };
> ColumnGenerator[] extraCols = new ColumnGenerator[] {
> new ColumnGenerator() {
> @Override
> public String getExportText(int rowNum) {
> return "1";
> }
>
> @Override
> public String getVerifyText(int rowNum) {
> return null;
> }
>
> @Override
> public String getType() {
> return "INTEGER";
> }
> },
> };
>
> String[] argv = getArgv(true, 10, 10, extraArgs);
> runNetezzaTest(getTableName(), argv, extraCols);
> }
>
> And this particular test is failing for me:
>
> Testcase: testNullStringExport2 took 2.528 sec
> FAILED
> Got unexpected column value expected:<null> but was:<1>
> junit.framework.ComparisonFailure: Got unexpected column value expected:<null> but was:<1>
> at com.cloudera.sqoop.TestExport.assertColValForRowId(TestExport.java:380)
> at com.cloudera.sqoop.TestExport.assertColMinAndMax(TestExport.java:398)
> at com.cloudera.sqoop.manager.DirectNetezzaExportManualTest.runNetezzaTest(DirectNetezzaExportManualTest.java:131)
> at com.cloudera.sqoop.manager.DirectNetezzaExportManualTest.testNullStringExport2(DirectNetezzaExportManualTest.java:205)
>
>
> I've also tried similar test for direct import by adding following test to NetezzaImportManualTest:
>
> @Test
> public void testDirectNullStringValue() throws Exception {
>
>
> String [] extraArgs = {
> "--null-string", "\\\\N",
> "--null-non-string", "\\\\N",
> };
>
> String[] expectedResultsWithNulls > getExpectedResultsWithNulls();
> String tableNameWithNull = getTableName() + "_W_N";
>
> runNetezzaTest(true, tableNameWithNull, expectedResultsWithNulls,
> extraArgs);
> }
>
> Generated output seems to be suggesting that the substitution character is not being used for the integer column:
>
> 22218 [main] INFO com.cloudera.sqoop.manager.NetezzaImportManualTest - Line read from file = 1,Aaron,2009-05-14,1000000,T,engineering,,1
> 22218 [main] INFO com.cloudera.sqoop.manager.NetezzaImportManualTest - Line read from file = 3,Fred,2009-01-23,15,F,marketing,,3
> 22218 [main] INFO com.cloudera.sqoop.manager.NetezzaImportManualTest - Line read from file = 2,Bob,2009-04-20,400,T,sales,,2
Hi Jarcec
I tried it with NPS 6 and 7. The char/varchar columns do get converted and int column was not. I tested it with my 6 and 7 emulators. But if you go by the Data loading guide for Netezza, then the option applies to all columns in both load and unload. So, it is more of a bug I would assume. But it is good to document it.
Thanks for bringing this up. I will create a JIRA to update the doc
- Venkat
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10018/#review18174
On March 21, 2013, 12:08 a.m., Venkat Ranganathan wrote:
+
Venkat Ranganathan 2013-03-21, 03:41
-
Re: Review Request: Fix for SQOOP-932Jarek Cecho 2013-03-21, 02:17
> On March 20, 2013, 9:16 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/DirectNetezzaManager.java, lines 78-83
> > <https://reviews.apache.org/r/10018/diff/2/?file=271931#file271931line78>
> >
> > My Netezza knowledge is a bit rusty these day, but I do have feeling that the external table parameter "NULLVALUE" is used only for string based columns (varchar, ...). For all other column types (int, float, ...) empty string is used to encode NULL value. On precondition that this is still the case, shouldn't the condition be more if(nullNonStrValue != null) { error; }?
>
> Venkat Ranganathan wrote:
> Thanks Jarcec
>
> The Netezza external table feature supports NULLVALUE for nonstring columns also. The unit tests tests it with an Integer column if you see.
>
> Thanks
> Venkat
>
> Jarek Cecho wrote:
> Hi Venkat,
> thank you for your feedback. I was actually looking at that particular test before, but I came to a conclusion that it's passing simply because "\N" is invalid value for integer based column. To verify my theory, I've added another test to DirectNetezzaExportManualTest that is using string "1" for null escape character (e.g. value that is valid for integer based column):
>
> @Test
> public void testNullStringExport2() throws Exception {
>
> String [] extraArgs = {
> "--input-null-string", "1",
> "--input-null-non-string", "1",
> "--input-escaped-by", "\\",
> };
> ColumnGenerator[] extraCols = new ColumnGenerator[] {
> new ColumnGenerator() {
> @Override
> public String getExportText(int rowNum) {
> return "1";
> }
>
> @Override
> public String getVerifyText(int rowNum) {
> return null;
> }
>
> @Override
> public String getType() {
> return "INTEGER";
> }
> },
> };
>
> String[] argv = getArgv(true, 10, 10, extraArgs);
> runNetezzaTest(getTableName(), argv, extraCols);
> }
>
> And this particular test is failing for me:
>
> Testcase: testNullStringExport2 took 2.528 sec
> FAILED
> Got unexpected column value expected:<null> but was:<1>
> junit.framework.ComparisonFailure: Got unexpected column value expected:<null> but was:<1>
> at com.cloudera.sqoop.TestExport.assertColValForRowId(TestExport.java:380)
> at com.cloudera.sqoop.TestExport.assertColMinAndMax(TestExport.java:398)
> at com.cloudera.sqoop.manager.DirectNetezzaExportManualTest.runNetezzaTest(DirectNetezzaExportManualTest.java:131)
> at com.cloudera.sqoop.manager.DirectNetezzaExportManualTest.testNullStringExport2(DirectNetezzaExportManualTest.java:205)
>
>
> I've also tried similar test for direct import by adding following test to NetezzaImportManualTest:
>
> @Test
> public void testDirectNullStringValue() throws Exception {
>
>
> String [] extraArgs = {
> "--null-string", "\\\\N",
> "--null-non-string", "\\\\N",
> };
>
> String[] expectedResultsWithNulls > getExpectedResultsWithNulls();
> String tableNameWithNull = getTableName() + "_W_N";
>
> runNetezzaTest(true, tableNameWithNull, expectedResultsWithNulls,
> extraArgs);
> }
>
> Generated output seems to be suggesting that the substitution character is not being used for the integer column:
>
> 22218 [main] INFO com.cloudera.sqoop.manager.NetezzaImportManualTest - Line read from file = 1,Aaron,2009-05-14,1000000,T,engineering,,1
> 22218 [main] INFO com.cloudera.sqoop.manager.NetezzaImportManualTest - Line read from file = 3,Fred,2009-01-23,15,F,marketing,,3
> 22218 [main] INFO com.cloudera.sqoop.manager.NetezzaImportManualTest - Line read from file = 2,Bob,2009-04-20,400,T,sales,,2
Export:
Yeah agreed, I would also expect this particular row to be moved as bad one. But I'm not extremely surprised to be honest. I've tried the same test with VARCHAR based column and everything worked. Thus my assumption that the NULLVALUE is used only for string based columns.
Import:
I've tried the same example with VARCHAR based column and I got the NULL substitution character instead of empty string. Which is source of my feeling that NULLVALUE is used only for string based columns. Would you mind taking a look how it behave in your environment?
- Jarek
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10018/#review18174
On March 21, 2013, 12:08 a.m., Venkat Ranganathan wrote:
+
Jarek Cecho 2013-03-21, 02:17
-
Re: Review Request: Fix for SQOOP-932Venkat Ranganathan 2013-03-21, 01:38
> On March 20, 2013, 9:16 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/DirectNetezzaManager.java, lines 78-83
> > <https://reviews.apache.org/r/10018/diff/2/?file=271931#file271931line78>
> >
> > My Netezza knowledge is a bit rusty these day, but I do have feeling that the external table parameter "NULLVALUE" is used only for string based columns (varchar, ...). For all other column types (int, float, ...) empty string is used to encode NULL value. On precondition that this is still the case, shouldn't the condition be more if(nullNonStrValue != null) { error; }?
>
> Venkat Ranganathan wrote:
> Thanks Jarcec
>
> The Netezza external table feature supports NULLVALUE for nonstring columns also. The unit tests tests it with an Integer column if you see.
>
> Thanks
> Venkat
>
> Jarek Cecho wrote:
> Hi Venkat,
> thank you for your feedback. I was actually looking at that particular test before, but I came to a conclusion that it's passing simply because "\N" is invalid value for integer based column. To verify my theory, I've added another test to DirectNetezzaExportManualTest that is using string "1" for null escape character (e.g. value that is valid for integer based column):
>
> @Test
> public void testNullStringExport2() throws Exception {
>
> String [] extraArgs = {
> "--input-null-string", "1",
> "--input-null-non-string", "1",
> "--input-escaped-by", "\\",
> };
> ColumnGenerator[] extraCols = new ColumnGenerator[] {
> new ColumnGenerator() {
> @Override
> public String getExportText(int rowNum) {
> return "1";
> }
>
> @Override
> public String getVerifyText(int rowNum) {
> return null;
> }
>
> @Override
> public String getType() {
> return "INTEGER";
> }
> },
> };
>
> String[] argv = getArgv(true, 10, 10, extraArgs);
> runNetezzaTest(getTableName(), argv, extraCols);
> }
>
> And this particular test is failing for me:
>
> Testcase: testNullStringExport2 took 2.528 sec
> FAILED
> Got unexpected column value expected:<null> but was:<1>
> junit.framework.ComparisonFailure: Got unexpected column value expected:<null> but was:<1>
> at com.cloudera.sqoop.TestExport.assertColValForRowId(TestExport.java:380)
> at com.cloudera.sqoop.TestExport.assertColMinAndMax(TestExport.java:398)
> at com.cloudera.sqoop.manager.DirectNetezzaExportManualTest.runNetezzaTest(DirectNetezzaExportManualTest.java:131)
> at com.cloudera.sqoop.manager.DirectNetezzaExportManualTest.testNullStringExport2(DirectNetezzaExportManualTest.java:205)
>
>
> I've also tried similar test for direct import by adding following test to NetezzaImportManualTest:
>
> @Test
> public void testDirectNullStringValue() throws Exception {
>
>
> String [] extraArgs = {
> "--null-string", "\\\\N",
> "--null-non-string", "\\\\N",
> };
>
> String[] expectedResultsWithNulls > getExpectedResultsWithNulls();
> String tableNameWithNull = getTableName() + "_W_N";
>
> runNetezzaTest(true, tableNameWithNull, expectedResultsWithNulls,
> extraArgs);
> }
>
> Generated output seems to be suggesting that the substitution character is not being used for the integer column:
>
> 22218 [main] INFO com.cloudera.sqoop.manager.NetezzaImportManualTest - Line read from file = 1,Aaron,2009-05-14,1000000,T,engineering,,1
> 22218 [main] INFO com.cloudera.sqoop.manager.NetezzaImportManualTest - Line read from file = 3,Fred,2009-01-23,15,F,marketing,,3
> 22218 [main] INFO com.cloudera.sqoop.manager.NetezzaImportManualTest - Line read from file = 2,Bob,2009-04-20,400,T,sales,,2
Thanks Jarcec for the examples
On the export - I think it may be a bug in NZ load that some valid values for NULL representation (like 1 in your case) are passed as is. If \N is not treated as NULL, then we will have the records treated as bad and written to bad records, right?
On the import, I saw the issue, but it was with any columns. I think it might also be tied to the JDBC driver version and the NZ version. For example, when I was researching Netezza forums, I saw reports that the NULL string should be 0-4 chars and only ASCII chars and no special chars, but then it has been changed in the later documentation to UTF-8 chars.
I agree it is not consistent.
Thanks
Venkat
- Venkat
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10018/#review18174
On March 21, 2013, 12:08 a.m., Venkat Ranganathan wrote:
+
Venkat Ranganathan 2013-03-21, 01:38
-
Re: Review Request: Fix for SQOOP-932Jarek Cecho 2013-03-21, 00:41
> On March 20, 2013, 9:16 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/DirectNetezzaManager.java, lines 78-83
> > <https://reviews.apache.org/r/10018/diff/2/?file=271931#file271931line78>
> >
> > My Netezza knowledge is a bit rusty these day, but I do have feeling that the external table parameter "NULLVALUE" is used only for string based columns (varchar, ...). For all other column types (int, float, ...) empty string is used to encode NULL value. On precondition that this is still the case, shouldn't the condition be more if(nullNonStrValue != null) { error; }?
>
> Venkat Ranganathan wrote:
> Thanks Jarcec
>
> The Netezza external table feature supports NULLVALUE for nonstring columns also. The unit tests tests it with an Integer column if you see.
>
> Thanks
> Venkat
Hi Venkat,
thank you for your feedback. I was actually looking at that particular test before, but I came to a conclusion that it's passing simply because "\N" is invalid value for integer based column. To verify my theory, I've added another test to DirectNetezzaExportManualTest that is using string "1" for null escape character (e.g. value that is valid for integer based column):
@Test
public void testNullStringExport2() throws Exception {
String [] extraArgs = {
"--input-null-string", "1",
"--input-null-non-string", "1",
"--input-escaped-by", "\\",
};
ColumnGenerator[] extraCols = new ColumnGenerator[] {
new ColumnGenerator() {
@Override
public String getExportText(int rowNum) {
return "1";
}
@Override
public String getVerifyText(int rowNum) {
return null;
}
@Override
public String getType() {
return "INTEGER";
}
},
};
String[] argv = getArgv(true, 10, 10, extraArgs);
runNetezzaTest(getTableName(), argv, extraCols);
}
And this particular test is failing for me:
Testcase: testNullStringExport2 took 2.528 sec
FAILED
Got unexpected column value expected:<null> but was:<1>
junit.framework.ComparisonFailure: Got unexpected column value expected:<null> but was:<1>
at com.cloudera.sqoop.TestExport.assertColValForRowId(TestExport.java:380)
at com.cloudera.sqoop.TestExport.assertColMinAndMax(TestExport.java:398)
at com.cloudera.sqoop.manager.DirectNetezzaExportManualTest.runNetezzaTest(DirectNetezzaExportManualTest.java:131)
at com.cloudera.sqoop.manager.DirectNetezzaExportManualTest.testNullStringExport2(DirectNetezzaExportManualTest.java:205)
I've also tried similar test for direct import by adding following test to NetezzaImportManualTest:
@Test
public void testDirectNullStringValue() throws Exception {
String [] extraArgs = {
"--null-string", "\\\\N",
"--null-non-string", "\\\\N",
};
String[] expectedResultsWithNulls getExpectedResultsWithNulls();
String tableNameWithNull = getTableName() + "_W_N";
runNetezzaTest(true, tableNameWithNull, expectedResultsWithNulls,
extraArgs);
}
Generated output seems to be suggesting that the substitution character is not being used for the integer column:
22218 [main] INFO com.cloudera.sqoop.manager.NetezzaImportManualTest - Line read from file = 1,Aaron,2009-05-14,1000000,T,engineering,,1
22218 [main] INFO com.cloudera.sqoop.manager.NetezzaImportManualTest - Line read from file = 3,Fred,2009-01-23,15,F,marketing,,3
22218 [main] INFO com.cloudera.sqoop.manager.NetezzaImportManualTest - Line read from file = 2,Bob,2009-04-20,400,T,sales,,2
Hope it helps!
Jarcec
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10018/#review18174
-----------------------------------------------------------
On March 21, 2013, 12:08 a.m., Venkat Ranganathan wrote:
>
> ----------------------------------------------------
+
Jarek Cecho 2013-03-21, 00:41
-
Re: Review Request: Fix for SQOOP-932Venkat Ranganathan 2013-03-20, 23:48
On March 20, 2013, 9:16 p.m., Venkat Ranganathan wrote:
> > Jarcec
Hi Jarcec
With 937 checked in, I don't think this patch will apply cleanly. Let me rebase and regenerate this patch
Thanks
- Venkat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10018/#review18174
-----------------------------------------------------------
On March 19, 2013, 6:06 p.m., Venkat Ranganathan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10018/
> -----------------------------------------------------------
>
> (Updated March 19, 2013, 6:06 p.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Description
> -------
>
> Fixes for SQOOP-932
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/lib/DelimiterSet.java ef62ba0
> src/java/org/apache/sqoop/manager/DirectNetezzaManager.java 0a1e605
> src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java 410a569
> src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java 9e6cab6
> src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableExportJob.java 2a702d9
> src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableImportJob.java 7ee6f70
> src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java 938ffc5
> src/test/com/cloudera/sqoop/manager/NetezzaExportManualTest.java 50d27fe
> src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java 3482dd8
>
> Diff: https://reviews.apache.org/r/10018/diff/
>
>
> Testing
> -------
>
> All unit tests pass - added more tests for testing this functionality
>
>
> Thanks,
>
> Venkat Ranganathan
>
>
+
Venkat Ranganathan 2013-03-20, 23:48
-
Re: Review Request: Fix for SQOOP-932Venkat Ranganathan 2013-03-20, 23:43
> On March 20, 2013, 9:16 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/DirectNetezzaManager.java, lines 78-83
> > <https://reviews.apache.org/r/10018/diff/2/?file=271931#file271931line78>
> >
> > My Netezza knowledge is a bit rusty these day, but I do have feeling that the external table parameter "NULLVALUE" is used only for string based columns (varchar, ...). For all other column types (int, float, ...) empty string is used to encode NULL value. On precondition that this is still the case, shouldn't the condition be more if(nullNonStrValue != null) { error; }?
Thanks Jarcec
The Netezza external table feature supports NULLVALUE for nonstring columns also. The unit tests tests it with an Integer column if you see.
Thanks
Venkat
- Venkat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10018/#review18174
-----------------------------------------------------------
On March 19, 2013, 6:06 p.m., Venkat Ranganathan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10018/
> -----------------------------------------------------------
>
> (Updated March 19, 2013, 6:06 p.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Description
> -------
>
> Fixes for SQOOP-932
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/lib/DelimiterSet.java ef62ba0
> src/java/org/apache/sqoop/manager/DirectNetezzaManager.java 0a1e605
> src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java 410a569
> src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java 9e6cab6
> src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableExportJob.java 2a702d9
> src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableImportJob.java 7ee6f70
> src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java 938ffc5
> src/test/com/cloudera/sqoop/manager/NetezzaExportManualTest.java 50d27fe
> src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java 3482dd8
>
> Diff: https://reviews.apache.org/r/10018/diff/
>
>
> Testing
> -------
>
> All unit tests pass - added more tests for testing this functionality
>
>
> Thanks,
>
> Venkat Ranganathan
>
>
+
Venkat Ranganathan 2013-03-20, 23:43
-
Re: Review Request: Fix for SQOOP-932Jarek Cecho 2013-03-22, 03:21
> On March 20, 2013, 9:16 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/DirectNetezzaManager.java, lines 78-83
> > <https://reviews.apache.org/r/10018/diff/2/?file=271931#file271931line78>
> >
> > My Netezza knowledge is a bit rusty these day, but I do have feeling that the external table parameter "NULLVALUE" is used only for string based columns (varchar, ...). For all other column types (int, float, ...) empty string is used to encode NULL value. On precondition that this is still the case, shouldn't the condition be more if(nullNonStrValue != null) { error; }?
>
> Venkat Ranganathan wrote:
> Thanks Jarcec
>
> The Netezza external table feature supports NULLVALUE for nonstring columns also. The unit tests tests it with an Integer column if you see.
>
> Thanks
> Venkat
>
> Jarek Cecho wrote:
> Hi Venkat,
> thank you for your feedback. I was actually looking at that particular test before, but I came to a conclusion that it's passing simply because "\N" is invalid value for integer based column. To verify my theory, I've added another test to DirectNetezzaExportManualTest that is using string "1" for null escape character (e.g. value that is valid for integer based column):
>
> @Test
> public void testNullStringExport2() throws Exception {
>
> String [] extraArgs = {
> "--input-null-string", "1",
> "--input-null-non-string", "1",
> "--input-escaped-by", "\\",
> };
> ColumnGenerator[] extraCols = new ColumnGenerator[] {
> new ColumnGenerator() {
> @Override
> public String getExportText(int rowNum) {
> return "1";
> }
>
> @Override
> public String getVerifyText(int rowNum) {
> return null;
> }
>
> @Override
> public String getType() {
> return "INTEGER";
> }
> },
> };
>
> String[] argv = getArgv(true, 10, 10, extraArgs);
> runNetezzaTest(getTableName(), argv, extraCols);
> }
>
> And this particular test is failing for me:
>
> Testcase: testNullStringExport2 took 2.528 sec
> FAILED
> Got unexpected column value expected:<null> but was:<1>
> junit.framework.ComparisonFailure: Got unexpected column value expected:<null> but was:<1>
> at com.cloudera.sqoop.TestExport.assertColValForRowId(TestExport.java:380)
> at com.cloudera.sqoop.TestExport.assertColMinAndMax(TestExport.java:398)
> at com.cloudera.sqoop.manager.DirectNetezzaExportManualTest.runNetezzaTest(DirectNetezzaExportManualTest.java:131)
> at com.cloudera.sqoop.manager.DirectNetezzaExportManualTest.testNullStringExport2(DirectNetezzaExportManualTest.java:205)
>
>
> I've also tried similar test for direct import by adding following test to NetezzaImportManualTest:
>
> @Test
> public void testDirectNullStringValue() throws Exception {
>
>
> String [] extraArgs = {
> "--null-string", "\\\\N",
> "--null-non-string", "\\\\N",
> };
>
> String[] expectedResultsWithNulls > getExpectedResultsWithNulls();
> String tableNameWithNull = getTableName() + "_W_N";
>
> runNetezzaTest(true, tableNameWithNull, expectedResultsWithNulls,
> extraArgs);
> }
>
> Generated output seems to be suggesting that the substitution character is not being used for the integer column:
>
> 22218 [main] INFO com.cloudera.sqoop.manager.NetezzaImportManualTest - Line read from file = 1,Aaron,2009-05-14,1000000,T,engineering,,1
> 22218 [main] INFO com.cloudera.sqoop.manager.NetezzaImportManualTest - Line read from file = 3,Fred,2009-01-23,15,F,marketing,,3
> 22218 [main] INFO com.cloudera.sqoop.manager.NetezzaImportManualTest - Line read from file = 2,Bob,2009-04-20,400,T,sales,,2
Thank you for creating the documentation JIRA Venkat! I've tried yet another test case:
@Test
public void testNullStringExport3() throws Exception {
String [] extraArgs = {
"--input-null-string", "N",
"--input-null-non-string", "N",
"--input-escaped-by", "\\",
};
ColumnGenerator[] extraCols = new ColumnGenerator[] {
new ColumnGenerator() {
@Override
public String getExportText(int rowNum) {
return "B";
}
@Override
public String getVerifyText(int rowNum) {
return null;
}
@Override
public String getType() {
return "INTEGER";
}
},
};
String[] argv = getArgv(true, 10, 10, extraArgs);
runNetezzaTest(getTableName(), argv, extraCols);
}
Interestingly this test case actually fails. I found the behavior of the external table feature quite buggy on Netezza side. I think that your change to documentation explains it correctly, so I'll go ahead and commit it.
- Jarek
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10018/#review18174
On March 21, 2013, 12:08 a.m., Venkat Ranganathan wrote:
+
Jarek Cecho 2013-03-22, 03:21
-
Re: Review Request: Fix for SQOOP-932Venkat Ranganathan 2013-03-21, 18:14
> On March 20, 2013, 9:16 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/DirectNetezzaManager.java, lines 78-83
> > <https://reviews.apache.org/r/10018/diff/2/?file=271931#file271931line78>
> >
> > My Netezza knowledge is a bit rusty these day, but I do have feeling that the external table parameter "NULLVALUE" is used only for string based columns (varchar, ...). For all other column types (int, float, ...) empty string is used to encode NULL value. On precondition that this is still the case, shouldn't the condition be more if(nullNonStrValue != null) { error; }?
>
> Venkat Ranganathan wrote:
> Thanks Jarcec
>
> The Netezza external table feature supports NULLVALUE for nonstring columns also. The unit tests tests it with an Integer column if you see.
>
> Thanks
> Venkat
>
> Jarek Cecho wrote:
> Hi Venkat,
> thank you for your feedback. I was actually looking at that particular test before, but I came to a conclusion that it's passing simply because "\N" is invalid value for integer based column. To verify my theory, I've added another test to DirectNetezzaExportManualTest that is using string "1" for null escape character (e.g. value that is valid for integer based column):
>
> @Test
> public void testNullStringExport2() throws Exception {
>
> String [] extraArgs = {
> "--input-null-string", "1",
> "--input-null-non-string", "1",
> "--input-escaped-by", "\\",
> };
> ColumnGenerator[] extraCols = new ColumnGenerator[] {
> new ColumnGenerator() {
> @Override
> public String getExportText(int rowNum) {
> return "1";
> }
>
> @Override
> public String getVerifyText(int rowNum) {
> return null;
> }
>
> @Override
> public String getType() {
> return "INTEGER";
> }
> },
> };
>
> String[] argv = getArgv(true, 10, 10, extraArgs);
> runNetezzaTest(getTableName(), argv, extraCols);
> }
>
> And this particular test is failing for me:
>
> Testcase: testNullStringExport2 took 2.528 sec
> FAILED
> Got unexpected column value expected:<null> but was:<1>
> junit.framework.ComparisonFailure: Got unexpected column value expected:<null> but was:<1>
> at com.cloudera.sqoop.TestExport.assertColValForRowId(TestExport.java:380)
> at com.cloudera.sqoop.TestExport.assertColMinAndMax(TestExport.java:398)
> at com.cloudera.sqoop.manager.DirectNetezzaExportManualTest.runNetezzaTest(DirectNetezzaExportManualTest.java:131)
> at com.cloudera.sqoop.manager.DirectNetezzaExportManualTest.testNullStringExport2(DirectNetezzaExportManualTest.java:205)
>
>
> I've also tried similar test for direct import by adding following test to NetezzaImportManualTest:
>
> @Test
> public void testDirectNullStringValue() throws Exception {
>
>
> String [] extraArgs = {
> "--null-string", "\\\\N",
> "--null-non-string", "\\\\N",
> };
>
> String[] expectedResultsWithNulls > getExpectedResultsWithNulls();
> String tableNameWithNull = getTableName() + "_W_N";
>
> runNetezzaTest(true, tableNameWithNull, expectedResultsWithNulls,
> extraArgs);
> }
>
> Generated output seems to be suggesting that the substitution character is not being used for the integer column:
>
> 22218 [main] INFO com.cloudera.sqoop.manager.NetezzaImportManualTest - Line read from file = 1,Aaron,2009-05-14,1000000,T,engineering,,1
> 22218 [main] INFO com.cloudera.sqoop.manager.NetezzaImportManualTest - Line read from file = 3,Fred,2009-01-23,15,F,marketing,,3
> 22218 [main] INFO com.cloudera.sqoop.manager.NetezzaImportManualTest - Line read from file = 2,Bob,2009-04-20,400,T,sales,,2
Hi Jarcec
BTW, I created the JIRA and updated a patch for that also
Thanks
- Venkat
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10018/#review18174
On March 21, 2013, 12:08 a.m., Venkat Ranganathan wrote:
+
Venkat Ranganathan 2013-03-21, 18:14
-
Re: Review Request: Fix for SQOOP-932Venkat Ranganathan 2013-03-22, 03:29
> On March 20, 2013, 9:16 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/DirectNetezzaManager.java, lines 78-83
> > <https://reviews.apache.org/r/10018/diff/2/?file=271931#file271931line78>
> >
> > My Netezza knowledge is a bit rusty these day, but I do have feeling that the external table parameter "NULLVALUE" is used only for string based columns (varchar, ...). For all other column types (int, float, ...) empty string is used to encode NULL value. On precondition that this is still the case, shouldn't the condition be more if(nullNonStrValue != null) { error; }?
>
> Venkat Ranganathan wrote:
> Thanks Jarcec
>
> The Netezza external table feature supports NULLVALUE for nonstring columns also. The unit tests tests it with an Integer column if you see.
>
> Thanks
> Venkat
>
> Jarek Cecho wrote:
> Hi Venkat,
> thank you for your feedback. I was actually looking at that particular test before, but I came to a conclusion that it's passing simply because "\N" is invalid value for integer based column. To verify my theory, I've added another test to DirectNetezzaExportManualTest that is using string "1" for null escape character (e.g. value that is valid for integer based column):
>
> @Test
> public void testNullStringExport2() throws Exception {
>
> String [] extraArgs = {
> "--input-null-string", "1",
> "--input-null-non-string", "1",
> "--input-escaped-by", "\\",
> };
> ColumnGenerator[] extraCols = new ColumnGenerator[] {
> new ColumnGenerator() {
> @Override
> public String getExportText(int rowNum) {
> return "1";
> }
>
> @Override
> public String getVerifyText(int rowNum) {
> return null;
> }
>
> @Override
> public String getType() {
> return "INTEGER";
> }
> },
> };
>
> String[] argv = getArgv(true, 10, 10, extraArgs);
> runNetezzaTest(getTableName(), argv, extraCols);
> }
>
> And this particular test is failing for me:
>
> Testcase: testNullStringExport2 took 2.528 sec
> FAILED
> Got unexpected column value expected:<null> but was:<1>
> junit.framework.ComparisonFailure: Got unexpected column value expected:<null> but was:<1>
> at com.cloudera.sqoop.TestExport.assertColValForRowId(TestExport.java:380)
> at com.cloudera.sqoop.TestExport.assertColMinAndMax(TestExport.java:398)
> at com.cloudera.sqoop.manager.DirectNetezzaExportManualTest.runNetezzaTest(DirectNetezzaExportManualTest.java:131)
> at com.cloudera.sqoop.manager.DirectNetezzaExportManualTest.testNullStringExport2(DirectNetezzaExportManualTest.java:205)
>
>
> I've also tried similar test for direct import by adding following test to NetezzaImportManualTest:
>
> @Test
> public void testDirectNullStringValue() throws Exception {
>
>
> String [] extraArgs = {
> "--null-string", "\\\\N",
> "--null-non-string", "\\\\N",
> };
>
> String[] expectedResultsWithNulls > getExpectedResultsWithNulls();
> String tableNameWithNull = getTableName() + "_W_N";
>
> runNetezzaTest(true, tableNameWithNull, expectedResultsWithNulls,
> extraArgs);
> }
>
> Generated output seems to be suggesting that the substitution character is not being used for the integer column:
>
> 22218 [main] INFO com.cloudera.sqoop.manager.NetezzaImportManualTest - Line read from file = 1,Aaron,2009-05-14,1000000,T,engineering,,1
> 22218 [main] INFO com.cloudera.sqoop.manager.NetezzaImportManualTest - Line read from file = 3,Fred,2009-01-23,15,F,marketing,,3
> 22218 [main] INFO com.cloudera.sqoop.manager.NetezzaImportManualTest - Line read from file = 2,Bob,2009-04-20,400,T,sales,,2
Yes. I found it buggy. The documentation seems to be evolving also. I will update the patch to the JIRA. We can keep an eye on this to see if there is any JDBC/Netezza changes that changes the behavior in future.
Thanks Jarcec
Venkat
- Venkat
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10018/#review18174
On March 21, 2013, 12:08 a.m., Venkat Ranganathan wrote:
+
Venkat Ranganathan 2013-03-22, 03:29
-
Review Request: Fix for SQOOP-932 documentation subtaskVenkat Ranganathan 2013-03-21, 04:34
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10057/
-----------------------------------------------------------
Review request for Sqoop and Jarek Cecho.
Description
-------
Documentation for the null string handling
Diffs
-----
src/docs/user/connectors.txt c172c4b
Diff: https://reviews.apache.org/r/10057/diff/
Testing
-------
Ran docs target - successfully built and verified output
Thanks,
Venkat Ranganathan
+
Venkat Ranganathan 2013-03-21, 04:34
-
Re: Review Request: Fix for SQOOP-932 documentation subtaskVenkat Ranganathan 2013-03-21, 04:34
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10057/
-----------------------------------------------------------
(Updated March 21, 2013, 4:34 a.m.)
Review request for Sqoop and Jarek Cecho.
Description
-------
Documentation for the null string handling
Diffs
-----
src/docs/user/connectors.txt c172c4b
Diff: https://reviews.apache.org/r/10057/diff/
Testing
-------
Ran docs target - successfully built and verified output
Thanks,
Venkat Ranganathan
+
Venkat Ranganathan 2013-03-21, 04:34
-
Re: Review Request: Fix for SQOOP-932 documentation subtaskVenkat Ranganathan 2013-03-22, 03:40
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10057/
-----------------------------------------------------------
(Updated March 22, 2013, 3:40 a.m.)
Review request for Sqoop and Jarek Cecho.
Changes
-------
Updated patch with removing trailing blanks in a couple of lines. Run ant docs and verified html documentation to be correctly formatted
Thanks
Venkat
Description
-------
Documentation for the null string handling
Diffs (updated)
-----
src/docs/user/connectors.txt c172c4b
Diff: https://reviews.apache.org/r/10057/diff/
Testing
-------
Ran docs target - successfully built and verified output
Thanks,
Venkat Ranganathan
+
Venkat Ranganathan 2013-03-22, 03:40
-
Re: Review Request: Fix for SQOOP-932 documentation subtaskJarek Cecho 2013-03-22, 03:52
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10057/#review18240
-----------------------------------------------------------
Ship it!
Ship It!
- Jarek Cecho
On March 22, 2013, 3:40 a.m., Venkat Ranganathan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10057/
> -----------------------------------------------------------
>
> (Updated March 22, 2013, 3:40 a.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Description
> -------
>
> Documentation for the null string handling
>
>
> Diffs
> -----
>
> src/docs/user/connectors.txt c172c4b
>
> Diff: https://reviews.apache.org/r/10057/diff/
>
>
> Testing
> -------
>
> Ran docs target - successfully built and verified output
>
>
> Thanks,
>
> Venkat Ranganathan
>
>
+
Jarek Cecho 2013-03-22, 03:52
-
Review Request: Fix for SQOOP-981Venkat Ranganathan 2013-04-15, 14:46
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10478/
-----------------------------------------------------------
Review request for Sqoop and Jarek Cecho.
Description
-------
Recognize --hadoop-home also as backward compatible option
Diffs
-----
src/java/org/apache/sqoop/tool/BaseSqoopTool.java 684d4a5
src/test/com/cloudera/sqoop/TestSqoopOptions.java fcc38eb
Diff: https://reviews.apache.org/r/10478/diff/
Testing
-------
Added two tests (one to recognize hadoop-home) and one to test hadoop-mapred-home overriding hadoop-home)
All tests pass
Thanks,
Venkat Ranganathan
+
Venkat Ranganathan 2013-04-15, 14:46
