You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Venkat Ranganathan <n....@live.com> on 2013/02/21 17:49:34 UTC

Review Request: SQOOP-846 Provide Netezza connector

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

Review request for Sqoop and Jarek Cecho.


Description
-------

This addresses SQOOP-846 (provide a Netezza connector)


This addresses bug SQOOP-846.
    https://issues.apache.org/jira/browse/SQOOP-846


Diffs
-----

  src/java/org/apache/sqoop/lib/DelimiterSet.java 4e9bcab 
  src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 54eb258 
  src/java/org/apache/sqoop/manager/DirectNetezzaManager.java PRE-CREATION 
  src/java/org/apache/sqoop/manager/MySQLUtils.java ef18818 
  src/java/org/apache/sqoop/manager/NetezzaManager.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaDBDataSliceSplitter.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaDataDrivenDBInputFormat.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableExportJob.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableImportJob.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableImportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableInputFormat.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableInputSplitter.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableRecordExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableTextExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaJDBCStatementRunner.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java PRE-CREATION 

Diff: https://reviews.apache.org/r/9543/diff/


Testing
-------

Ran all sqoop tests.   Ran Netezza manual tests against Netezza VMs version 6 and 7


Thanks,

Venkat Ranganathan


Re: Review Request: SQOOP-846 Provide Netezza connector

Posted by Venkat Ranganathan <n....@live.com>.

> On Feb. 24, 2013, 10:16 p.m., Jarek Cecho wrote:
> > src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java, line 17
> > <https://reviews.apache.org/r/9543/diff/1/?file=260691#file260691line17>
> >
> >     I'm not sure that you want to have your private IP publicly available in the source code.
> 
> Venkat Ranganathan wrote:
>     Fixed in second revision.  But will create a new combo patch

Sorry, the comment was for previous issue.  This is  a non-routable private IP so security issue is not a concern, but still I understand your point and have fixed it in latest revision


- Venkat


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


On Feb. 25, 2013, 10:02 p.m., Venkat Ranganathan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9543/
> -----------------------------------------------------------
> 
> (Updated Feb. 25, 2013, 10:02 p.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Description
> -------
> 
> This addresses SQOOP-846 (provide a Netezza connector)
> 
> 
> This addresses bug SQOOP-846.
>     https://issues.apache.org/jira/browse/SQOOP-846
> 
> 
> Diffs
> -----
> 
>   src/java/org/apache/sqoop/lib/DelimiterSet.java 4e9bcab 
>   src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 54eb258 
>   src/java/org/apache/sqoop/manager/DirectNetezzaManager.java PRE-CREATION 
>   src/java/org/apache/sqoop/manager/MySQLUtils.java ef18818 
>   src/java/org/apache/sqoop/manager/NetezzaManager.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaDBDataSliceSplitter.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaDataDrivenDBInputFormat.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportJob.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportJob.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableInputFormat.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableInputSplitter.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableRecordExportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableTextExportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java PRE-CREATION 
>   src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java PRE-CREATION 
>   src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java PRE-CREATION 
>   src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/9543/diff/
> 
> 
> Testing
> -------
> 
> Ran all sqoop tests.   Ran Netezza manual tests against Netezza VMs version 6 and 7
> 
> 
> Thanks,
> 
> Venkat Ranganathan
> 
>


Re: Review Request: SQOOP-846 Provide Netezza connector

Posted by Venkat Ranganathan <n....@live.com>.

> On Feb. 24, 2013, 10:16 p.m., Jarek Cecho wrote:
> > Hi Venkat,
> > thank you very much for putting Netezza connector together. I greatly appreciate your time and effort. I've noticed that updated patch number 2 have only 12kb and is not the same as was uploaded into review board. Would you mind uploading the latest patch to the review board? It's much more easier for reviewer (well, it's much more easier for me).
> > 
> > Nevertheless, I've done high level review:
> > 
> > 1) Would you mind removing trailing white space characters?
> > 
> > 2) Would you mind adding appropriate section(s) into user guide?

Thanks Jarcec for reviewing the changes.     I removed the trailing blanks with additional changes and updated a second patch.   Unfortunately, I was not able to upload a new patch as the review board was claiming that some files were not available (because I added a few new files).   Hence I created a patch of only differences from patch 1.   I will try again.   I understand it is difficult to review like this and sorry about that.   Do you have suggestions on I might have done wrong in updating a revision 2?

Yes, I will be adding a section to the User guide (tracked by a separate JIRA) after the review so that I can be clear about options etc.


> On Feb. 24, 2013, 10:16 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/DirectNetezzaManager.java, lines 34-36
> > <https://reviews.apache.org/r/9543/diff/1/?file=260675#file260675line34>
> >
> >     I felt that org.apache.sqoop.mapreduce packages is becoming a big mess, so I've started adding new connector-specific code into their own packages. For example:
> >     
> >     org.apache.sqoop.mapreduce.mysql
> >     org.apache.sqoop.mapreduce.sqlserver
> >     
> >     I would suggest to put the mapreduce files for Netezza into their own respective package:
> >     
> >     org.apache.sqoop.mapreduce.netezza
> >     
> >     What do you think?
> >

Sure,  I moved all Netezza change to mapreduce.db.   But mapreduce.db.netezza is also good.  Since there were not any db specific packages currently, I was not doing that.  But will move the netezza changes  to netezza package.


> On Feb. 24, 2013, 10:16 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/DirectNetezzaManager.java, lines 94-99
> > <https://reviews.apache.org/r/9543/diff/1/?file=260675#file260675line94>
> >
> >     Would you mind introducing finally block and closing the PreparedStatement and ResultSet? Something like
> >     
> >     } finally {
> >       if(rc != null) {
> >         rc.close();
> >       }
> >       if(ps != null) {
> >         ps.close();
> >       }
> >     }

Thanks for catching this!


> On Feb. 24, 2013, 10:16 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/DirectNetezzaManager.java, lines 126-128
> > <https://reviews.apache.org/r/9543/diff/1/?file=260675#file260675line126>
> >
> >     I can see that you're parsing extra arguments on several places (exportTable, importTable). What about moving this code into constructor?

Good observation, but if we have an invalid option, do you suggest throwing IllegalArgumentException from the constructor, as the constructor does not throw an exception.   


> On Feb. 24, 2013, 10:16 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/MySQLUtils.java, line 61
> > <https://reviews.apache.org/r/9543/diff/1/?file=260676#file260676line61>
> >
> >     Nit: Trailing white space characters.

Fixed in second revision.  But will create a new combo patch


> On Feb. 24, 2013, 10:16 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/MySQLUtils.java, line 72
> > <https://reviews.apache.org/r/9543/diff/1/?file=260676#file260676line72>
> >
> >     Nit: Trailing white space characters.

Fixed in second revision.  But will create a new combo patch


> On Feb. 24, 2013, 10:16 p.m., Jarek Cecho wrote:
> > src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java, line 17
> > <https://reviews.apache.org/r/9543/diff/1/?file=260691#file260691line17>
> >
> >     I'm not sure that you want to have your private IP publicly available in the source code.

Fixed in second revision.  But will create a new combo patch


- Venkat


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


On Feb. 22, 2013, 7:58 a.m., Venkat Ranganathan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9543/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2013, 7:58 a.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Description
> -------
> 
> This addresses SQOOP-846 (provide a Netezza connector)
> 
> 
> This addresses bug SQOOP-846.
>     https://issues.apache.org/jira/browse/SQOOP-846
> 
> 
> Diffs
> -----
> 
>   src/java/org/apache/sqoop/manager/DirectNetezzaManager.java PRE-CREATION 
>   src/java/org/apache/sqoop/manager/MySQLUtils.java ef18818 
>   src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableExportJob.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableExportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableImportMapper.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/9543/diff/
> 
> 
> Testing
> -------
> 
> Ran all sqoop tests.   Ran Netezza manual tests against Netezza VMs version 6 and 7
> 
> 
> Thanks,
> 
> Venkat Ranganathan
> 
>


Re: Review Request: SQOOP-846 Provide Netezza connector

Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9543/#review17010
-----------------------------------------------------------


Hi Venkat,
thank you very much for putting Netezza connector together. I greatly appreciate your time and effort. I've noticed that updated patch number 2 have only 12kb and is not the same as was uploaded into review board. Would you mind uploading the latest patch to the review board? It's much more easier for reviewer (well, it's much more easier for me).

Nevertheless, I've done high level review:

1) Would you mind removing trailing white space characters?

2) Would you mind adding appropriate section(s) into user guide?


src/java/org/apache/sqoop/manager/DirectNetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36052>

    I felt that org.apache.sqoop.mapreduce packages is becoming a big mess, so I've started adding new connector-specific code into their own packages. For example:
    
    org.apache.sqoop.mapreduce.mysql
    org.apache.sqoop.mapreduce.sqlserver
    
    I would suggest to put the mapreduce files for Netezza into their own respective package:
    
    org.apache.sqoop.mapreduce.netezza
    
    What do you think?
    



src/java/org/apache/sqoop/manager/DirectNetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36059>

    Tests are suggesting that staging table in Direct mode is not supported, though this class do not override supportsStagingForExport method that is returning true in parent class.



src/java/org/apache/sqoop/manager/DirectNetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36053>

    Would you mind introducing finally block and closing the PreparedStatement and ResultSet? Something like
    
    } finally {
      if(rc != null) {
        rc.close();
      }
      if(ps != null) {
        ps.close();
      }
    }



src/java/org/apache/sqoop/manager/DirectNetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36054>

    Nit: Would you mind putting the catch keyword on the same line as closing "}". The rest of code is following this code style, so it would be great to remain consistent.



src/java/org/apache/sqoop/manager/DirectNetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36055>

    I can see that you're parsing extra arguments on several places (exportTable, importTable). What about moving this code into constructor?



src/java/org/apache/sqoop/manager/MySQLUtils.java
<https://reviews.apache.org/r/9543/#comment36056>

    Nit: Trailing white space characters.



src/java/org/apache/sqoop/manager/MySQLUtils.java
<https://reviews.apache.org/r/9543/#comment36057>

    Nit: Trailing white space characters.



src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java
<https://reviews.apache.org/r/9543/#comment36058>

    I'm not sure that you want to have your private IP publicly available in the source code.


Jarcec

- Jarek Cecho


On Feb. 22, 2013, 7:58 a.m., Venkat Ranganathan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9543/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2013, 7:58 a.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Description
> -------
> 
> This addresses SQOOP-846 (provide a Netezza connector)
> 
> 
> This addresses bug SQOOP-846.
>     https://issues.apache.org/jira/browse/SQOOP-846
> 
> 
> Diffs
> -----
> 
>   src/java/org/apache/sqoop/manager/DirectNetezzaManager.java PRE-CREATION 
>   src/java/org/apache/sqoop/manager/MySQLUtils.java ef18818 
>   src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableExportJob.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableExportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableImportMapper.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/9543/diff/
> 
> 
> Testing
> -------
> 
> Ran all sqoop tests.   Ran Netezza manual tests against Netezza VMs version 6 and 7
> 
> 
> Thanks,
> 
> Venkat Ranganathan
> 
>


Re: Review Request: SQOOP-846 Provide Netezza connector

Posted by Venkat Ranganathan <n....@live.com>.
Hi Jarcec


I have to spin one more patch    I inadvertently added DirectMySQLTest when I was preparing this patch.   Will respin without that file.  In the meanwhile if you have review comments on all the other files, please continue with the review.   The only new change that I am planning to do is to remove this file

Venkat

From: Venkat Ranganathan 
Sent: Wednesday, February 27, 2013 9:04 PM
To: Jarek Cecho 
Cc: Venkat Ranganathan ; Sqoop 
Subject: Re: Review Request: SQOOP-846 Provide Netezza connector

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


  On February 27th, 2013, 1:01 a.m., Jarek Cecho wrote:

Hi Venkat,
thank you very much for working on this. I really appreciate your time and effort. I've started diving into your patch. I'm not quite done yet, but let me share my thoughts to unblock you.

1) Would you mind running "ant checkstyle" and cleaning up the errors? Some of the errors are already present in trunk, so please feel free to leave them out. Just it would be great if would not include any new errors.

2) Tests do not seem passing for me against Netezza Simulator 6.0. I'm not yet sure why, so I'll investigate that on my end.

3) It seems that you prefer creating chained exceptions in form NewException("String").initCause(oldException). The rest of the code seems to be following different approach and is creating exceptions in form NewException("String", oldException). Do you feel comfortable with change that to have unified code style?
Thanks for the review Jarek.   
1) Ant checkstyle - would do.  Good point.   I missed doing that the last time around
2) Let me know if you find something - it runs fine with my NPS simulators.   
3) Old habit working on WLS Server core in the pre 1.4 days :)   Yes, will switch to the other style

Will update another diff after running tests with new changes

  On February 27th, 2013, 1:01 a.m., Jarek Cecho wrote:

          src/java/org/apache/sqoop/manager/DirectNetezzaManager.java (Diff revision 5)  
None 
          34 import org.apache.sqoop.mapreduce.db.netezza.NetezzaDataDrivenDBInputFormat; 
          35 import org.apache.sqoop.mapreduce.db.netezza.NetezzaExternalTableExportJob; 
          36 import org.apache.sqoop.mapreduce.db.netezza.NetezzaExternalTableImportJob; 

It seems that I've confused you, so please accept my apologies for that. Would you mind putting the mapreduce specific classes into package "org.apache.sqoop.mapreduce.netezza"? (e.g leaving out the "db" package). You can notice that there are already packages for MySQL and Microsoft SQL Server:

org.apache.sqoop.mapreduce.mysql
org.apache.sqoop.mapreduce.sqlserverSure - would do

  On February 27th, 2013, 1:01 a.m., Jarek Cecho wrote:

          src/java/org/apache/sqoop/manager/DirectNetezzaManager.java (Diff revision 5)  
None 
          131     try { 
          132       handleNetezzaExtraArgs(options); 
          133     } catch (SqoopOptions.InvalidOptionsException ioe) { 
          134       throw (ExportException) new ExportException(ioe.getMessage()) 
          135           .initCause(ioe); 
          136     } 

I can see similar code fragment in both exportTable() and importTable() methods (just returning different exception class). I would suggest to put it into constructor instead. The other connectors seems to be doing that already.I am not a big fan of doing this and making the parsing exception as  a child ( sometimes chained exceptions are eaten while logging etc).  But for consistency will change it

  On February 27th, 2013, 1:01 a.m., Jarek Cecho wrote:

          src/java/org/apache/sqoop/manager/DirectNetezzaManager.java (Diff revision 5)  
None 
          211     if (extraArgs != null && extraArgs.length == 0 

Shouldn't this condition be something like:

...extraArgs.length != 0 ...Yes!

  On February 27th, 2013, 1:01 a.m., Jarek Cecho wrote:

          src/java/org/apache/sqoop/manager/DirectNetezzaManager.java (Diff revision 5)  
None 
          212         && ConfigurationHelper.getConfNumMaps(conf) > 1) { 

I'm not quite sure what is the reasoning for skipping extra parameter checking when there is only one mapper?For DirectNetezzaManager, it does not make sense to restrict as the log dir and max errors also apply to one mapper case.   For NetezzaManager, with one mapper, there is no point in having a dataslice aligned import, so that check was there.   Removing it for DirectNetezzaManager

  On February 27th, 2013, 1:01 a.m., Jarek Cecho wrote:

          src/java/org/apache/sqoop/manager/DirectNetezzaManager.java (Diff revision 5)  
None 
          229         SqoopOptions.InvalidOptionsException ioe = new SqoopOptions.InvalidOptionsException( 
          230             pe.getMessage()); 
          231         ioe.initCause(pe); 
          232         throw ioe; 

Would you mind simplifying this by doing something like:

throw new SqoopOptions.InvalidOptionsException(pe.getMessage(), pe);SqoopOptions does not provide a constructor to initialize the cause (only messages).  I will ignore the cause and just provide the exception msg to the constructor

  On February 27th, 2013, 1:01 a.m., Jarek Cecho wrote:

          src/java/org/apache/sqoop/manager/DirectNetezzaManager.java (Diff revision 5)  
None 
          234     } else { 
          235       conf.setBoolean(NETEZZA_DATASLICE_ALIGNED_ACCESS_OPT, true); 
          236     } 

It seems to me that this property is set in both branches of the if-else statement. Would it make sense to set it at one place then?Yes, good point

  On February 27th, 2013, 1:01 a.m., Jarek Cecho wrote:

          src/java/org/apache/sqoop/manager/NetezzaManager.java (Diff revision 5)  
None {'text': '  private void handleNetezzaImportExtraArgs(ImportJobContext context)', 'line': 199} 
          208         && ConfigurationHelper.getConfNumMaps(conf) > 1) { 

I'm not quite sure what is the reasoning for skipping extra parameter checking when there is only one mapper?I am not sure about this line.   For updates the number of mappers more than 1 causes inconsistencies.   I added a link to the doc but did not reference it specifically.   Will update the doc.   Essentially Netezza does an delete and insert for updates and because of the architecture, updates from multiple mappers to the same table can cause issues

  On February 27th, 2013, 1:01 a.m., Jarek Cecho wrote:

          src/java/org/apache/sqoop/manager/NetezzaManager.java (Diff revision 5)  
None {'text': '  private void handleNetezzaImportExtraArgs(ImportJobContext context)', 'line': 199} 
          209       RelatedOptions netezzaOpts = new RelatedOptions("Netezza options"); 
          210       netezzaOpts.addOption(OptionBuilder 
          211           .withArgName(NETEZZA_DATASLICE_ALIGNED_ACCESS_OPT).hasArg() 
          212           .withDescription("Data slice aligned import") 
          213           .withLongOpt(NETEZZA_DATASLICE_ALIGNED_ACCESS_LONG_ARG).create()); 

Can we put this into standalone method getNetezzaExtraOpts() similarly as we've done in NetezzaDirectManager?Yes

  On February 27th, 2013, 1:01 a.m., Jarek Cecho wrote:

          src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportJob.java (Diff revision 5)  
None 
          39  * Class that runs an export job using mysqlimport in the mapper. 

That comment seems little bit off :-)I used the mysql file as the starting point :)

  On February 27th, 2013, 1:01 a.m., Jarek Cecho wrote:

          src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java (Diff revision 5)  
None 
          44 public class NetezzaExternalTableExportMapper<K, V> extends 

Shouldn't this be abstract class? It seems to me that it do not make sense when it would be used directly.Yes good point.  

  On February 27th, 2013, 1:01 a.m., Jarek Cecho wrote:

          src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java (Diff revision 5)  
None 
          45     Mapper<K, V, NullWritable, NullWritable> { 

Can we inherit from SqoopMapper?Yes, that provides the ability to handle verbose setting also

  On February 27th, 2013, 1:01 a.m., Jarek Cecho wrote:

          src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java (Diff revision 5)  
None 
          101           LOG.warn("Unsupported enclosed by character: " + qc + " - ignoring."); 

Can we make similar check on the Sqoop client side? I'm afraid that this warning will get lost as it's printed in the mapper task and I'm not expecting users to read it up there.Great point.   I updated this for both direct mode import and export table so that exception is printed early.

  On February 27th, 2013, 1:01 a.m., Jarek Cecho wrote:

          src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableInputSplitter.java (Diff revision 5)  
None 
          30 public class NetezzaExternalTableInputSplitter extends InputSplit implements 

I would suggest to rename this into NetezzaExternalTableInputSplit instead of "..Splitter". Done

  On February 27th, 2013, 1:01 a.m., Jarek Cecho wrote:

          src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java (Diff revision 5)  
None {'text': '  public Throwable getExceptonChain() {', 'line': 58} 
          64       t.initCause(exceptionList.get(i)); 

This chaining of exceptions seems problematic to me. The initCause() might not be allowed to call as the exception cause might be already initialized.

It seems that there might be maximally two exceptions. If that is correct then I suggest to return only one instead of the chain. I would personally prioritized the one catched on line 88 over the one on line 95.Fixed.  Good catch

  On February 27th, 2013, 1:01 a.m., Jarek Cecho wrote:

          src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java (Diff revision 5)  
None {'text': '  public void testAuthExport() throws IOException, SQLException {', 'line': 193} 
          233       conf.set("fs.default.name", "file:///"); 

I would vote for not doing this. The file:// should be default value and by explicitly specifying this, we will loose the ability to run the tests against real Hadoop cluster.Good point - I removed it

  On February 27th, 2013, 1:01 a.m., Jarek Cecho wrote:

          src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java (Diff revision 5)  
None {'text': '  public static String getNZConnectString() {', 'line': 54} 
          72  
          73     return url.toString(); 

Can we print out used Netezza URL, something like:

LOG.info("Using Netezza URL: " + url.toString());

My motivation here is to help users running the test to see what exactly is Sqoop trying to do. It helped me, so I can imagine that it might help others as well.Good point

- Venkat



On February 28th, 2013, 1:53 a.m., Venkat Ranganathan wrote:

      Review request for Sqoop and Jarek Cecho.
      By Venkat Ranganathan.
      Updated Feb. 28, 2013, 1:53 a.m.

      Description 
This addresses SQOOP-846 (provide a Netezza connector) 

      Testing 
Ran all sqoop tests.   Ran Netezza manual tests against Netezza VMs version 6 and 7 

      Bugs: SQOOP-846 
      Diffs 
        a.. src/docs/user/connectors.txt (7dd2a2e) 
        b.. src/java/org/apache/sqoop/lib/DelimiterSet.java (4e9bcab) 
        c.. src/java/org/apache/sqoop/manager/DefaultManagerFactory.java (54eb258) 
        d.. src/java/org/apache/sqoop/manager/DirectNetezzaManager.java (PRE-CREATION) 
        e.. src/java/org/apache/sqoop/manager/MySQLUtils.java (ef18818) 
        f.. src/java/org/apache/sqoop/manager/NetezzaManager.java (PRE-CREATION) 
        g.. src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaDBDataSliceSplitter.java (PRE-CREATION) 
        h.. src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java (PRE-CREATION) 
        i.. src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java (PRE-CREATION) 
        j.. src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableRecordExportMapper.java (PRE-CREATION) 
        k.. src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableTextExportMapper.java (PRE-CREATION) 
        l.. src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java (PRE-CREATION) 
        m.. src/java/org/apache/sqoop/mapreduce/netezza/NetezzaDataDrivenDBInputFormat.java (PRE-CREATION) 
        n.. src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableExportJob.java (PRE-CREATION) 
        o.. src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableImportJob.java (PRE-CREATION) 
        p.. src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableInputFormat.java (PRE-CREATION) 
        q.. src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableInputSplit.java (PRE-CREATION) 
        r.. src/test/com/cloudera/sqoop/manager/DirectMySQLExportTest.java (eac7836) 
        s.. src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java (PRE-CREATION) 
        t.. src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java (PRE-CREATION) 
        u.. src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java (PRE-CREATION) 
      View Diff
     

Re: Review Request: SQOOP-846 Provide Netezza connector

Posted by Venkat Ranganathan <n....@live.com>.

> On Feb. 27, 2013, 1:01 a.m., Jarek Cecho wrote:
> > Hi Venkat,
> > thank you very much for working on this. I really appreciate your time and effort. I've started diving into your patch. I'm not quite done yet, but let me share my thoughts to unblock you.
> > 
> > 1) Would you mind running "ant checkstyle" and cleaning up the errors? Some of the errors are already present in trunk, so please feel free to leave them out. Just it would be great if would not include any new errors.
> > 
> > 2) Tests do not seem passing for me against Netezza Simulator 6.0. I'm not yet sure why, so I'll investigate that on my end.
> > 
> > 3) It seems that you prefer creating chained exceptions in form NewException("String").initCause(oldException). The rest of the code seems to be following different approach and is creating exceptions in form NewException("String", oldException). Do you feel comfortable with change that to have unified code style?
> >

Thanks for the review Jarek.   
1) Ant checkstyle - would do.  Good point.   I missed doing that the last time around
2) Let me know if you find something - it runs fine with my NPS simulators.   
3) Old habit working on WLS Server core in the pre 1.4 days :)   Yes, will switch to the other style

Will update another diff after running tests with new changes


> On Feb. 27, 2013, 1:01 a.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/DirectNetezzaManager.java, lines 34-36
> > <https://reviews.apache.org/r/9543/diff/5/?file=261951#file261951line34>
> >
> >     It seems that I've confused you, so please accept my apologies for that. Would you mind putting the mapreduce specific classes into package "org.apache.sqoop.mapreduce.netezza"? (e.g leaving out the "db" package). You can notice that there are already packages for MySQL and Microsoft SQL Server:
> >     
> >     org.apache.sqoop.mapreduce.mysql
> >     org.apache.sqoop.mapreduce.sqlserver

Sure - would do


> On Feb. 27, 2013, 1:01 a.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/DirectNetezzaManager.java, lines 131-136
> > <https://reviews.apache.org/r/9543/diff/5/?file=261951#file261951line131>
> >
> >     I can see similar code fragment in both exportTable() and importTable() methods (just returning different exception class). I would suggest to put it into constructor instead. The other connectors seems to be doing that already.

I am not a big fan of doing this and making the parsing exception as  a child ( sometimes chained exceptions are eaten while logging etc).  But for consistency will change it


> On Feb. 27, 2013, 1:01 a.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/DirectNetezzaManager.java, line 211
> > <https://reviews.apache.org/r/9543/diff/5/?file=261951#file261951line211>
> >
> >     Shouldn't this condition be something like:
> >     
> >     ...extraArgs.length != 0 ...

Yes!


> On Feb. 27, 2013, 1:01 a.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/DirectNetezzaManager.java, line 212
> > <https://reviews.apache.org/r/9543/diff/5/?file=261951#file261951line212>
> >
> >     I'm not quite sure what is the reasoning for skipping extra parameter checking when there is only one mapper?

For DirectNetezzaManager, it does not make sense to restrict as the log dir and max errors also apply to one mapper case.   For NetezzaManager, with one mapper, there is no point in having a dataslice aligned import, so that check was there.   Removing it for DirectNetezzaManager


> On Feb. 27, 2013, 1:01 a.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/DirectNetezzaManager.java, lines 229-232
> > <https://reviews.apache.org/r/9543/diff/5/?file=261951#file261951line229>
> >
> >     Would you mind simplifying this by doing something like:
> >     
> >     throw new SqoopOptions.InvalidOptionsException(pe.getMessage(), pe);

SqoopOptions does not provide a constructor to initialize the cause (only messages).  I will ignore the cause and just provide the exception msg to the constructor


> On Feb. 27, 2013, 1:01 a.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/DirectNetezzaManager.java, lines 234-236
> > <https://reviews.apache.org/r/9543/diff/5/?file=261951#file261951line234>
> >
> >     It seems to me that this property is set in both branches of the if-else statement. Would it make sense to set it at one place then?

Yes, good point


> On Feb. 27, 2013, 1:01 a.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/NetezzaManager.java, line 208
> > <https://reviews.apache.org/r/9543/diff/5/?file=261953#file261953line208>
> >
> >     I'm not quite sure what is the reasoning for skipping extra parameter checking when there is only one mapper?

I am not sure about this line.   For updates the number of mappers more than 1 causes inconsistencies.   I added a link to the doc but did not reference it specifically.   Will update the doc.   Essentially Netezza does an delete and insert for updates and because of the architecture, updates from multiple mappers to the same table can cause issues


> On Feb. 27, 2013, 1:01 a.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/NetezzaManager.java, lines 209-213
> > <https://reviews.apache.org/r/9543/diff/5/?file=261953#file261953line209>
> >
> >     Can we put this into standalone method getNetezzaExtraOpts() similarly as we've done in NetezzaDirectManager?

Yes


> On Feb. 27, 2013, 1:01 a.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportJob.java, line 39
> > <https://reviews.apache.org/r/9543/diff/5/?file=261956#file261956line39>
> >
> >     That comment seems little bit off :-)

I used the mysql file as the starting point :)


> On Feb. 27, 2013, 1:01 a.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java, line 44
> > <https://reviews.apache.org/r/9543/diff/5/?file=261957#file261957line44>
> >
> >     Shouldn't this be abstract class? It seems to me that it do not make sense when it would be used directly.

Yes good point.  


> On Feb. 27, 2013, 1:01 a.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java, line 45
> > <https://reviews.apache.org/r/9543/diff/5/?file=261957#file261957line45>
> >
> >     Can we inherit from SqoopMapper?

Yes, that provides the ability to handle verbose setting also


> On Feb. 27, 2013, 1:01 a.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java, line 101
> > <https://reviews.apache.org/r/9543/diff/5/?file=261957#file261957line101>
> >
> >     Can we make similar check on the Sqoop client side? I'm afraid that this warning will get lost as it's printed in the mapper task and I'm not expecting users to read it up there.

Great point.   I updated this for both direct mode import and export table so that exception is printed early.


> On Feb. 27, 2013, 1:01 a.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableInputSplitter.java, line 30
> > <https://reviews.apache.org/r/9543/diff/5/?file=261961#file261961line30>
> >
> >     I would suggest to rename this into NetezzaExternalTableInputSplit instead of "..Splitter".

Done


> On Feb. 27, 2013, 1:01 a.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java, line 64
> > <https://reviews.apache.org/r/9543/diff/5/?file=261964#file261964line64>
> >
> >     This chaining of exceptions seems problematic to me. The initCause() might not be allowed to call as the exception cause might be already initialized.
> >     
> >     It seems that there might be maximally two exceptions. If that is correct then I suggest to return only one instead of the chain. I would personally prioritized the one catched on line 88 over the one on line 95.

Fixed.  Good catch


> On Feb. 27, 2013, 1:01 a.m., Jarek Cecho wrote:
> > src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java, line 233
> > <https://reviews.apache.org/r/9543/diff/5/?file=261965#file261965line233>
> >
> >     I would vote for not doing this. The file:// should be default value and by explicitly specifying this, we will loose the ability to run the tests against real Hadoop cluster.

Good point - I removed it


> On Feb. 27, 2013, 1:01 a.m., Jarek Cecho wrote:
> > src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java, lines 72-73
> > <https://reviews.apache.org/r/9543/diff/5/?file=261967#file261967line72>
> >
> >     Can we print out used Netezza URL, something like:
> >     
> >     LOG.info("Using Netezza URL: " + url.toString());
> >     
> >     My motivation here is to help users running the test to see what exactly is Sqoop trying to do. It helped me, so I can imagine that it might help others as well.

Good point


- Venkat


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


On Feb. 28, 2013, 1:53 a.m., Venkat Ranganathan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9543/
> -----------------------------------------------------------
> 
> (Updated Feb. 28, 2013, 1:53 a.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Description
> -------
> 
> This addresses SQOOP-846 (provide a Netezza connector)
> 
> 
> This addresses bug SQOOP-846.
>     https://issues.apache.org/jira/browse/SQOOP-846
> 
> 
> Diffs
> -----
> 
>   src/docs/user/connectors.txt 7dd2a2e 
>   src/java/org/apache/sqoop/lib/DelimiterSet.java 4e9bcab 
>   src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 54eb258 
>   src/java/org/apache/sqoop/manager/DirectNetezzaManager.java PRE-CREATION 
>   src/java/org/apache/sqoop/manager/MySQLUtils.java ef18818 
>   src/java/org/apache/sqoop/manager/NetezzaManager.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaDBDataSliceSplitter.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableRecordExportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableTextExportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaDataDrivenDBInputFormat.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableExportJob.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableImportJob.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableInputFormat.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableInputSplit.java PRE-CREATION 
>   src/test/com/cloudera/sqoop/manager/DirectMySQLExportTest.java eac7836 
>   src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java PRE-CREATION 
>   src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java PRE-CREATION 
>   src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/9543/diff/
> 
> 
> Testing
> -------
> 
> Ran all sqoop tests.   Ran Netezza manual tests against Netezza VMs version 6 and 7
> 
> 
> Thanks,
> 
> Venkat Ranganathan
> 
>


Re: Review Request: SQOOP-846 Provide Netezza connector

Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9543/#review17076
-----------------------------------------------------------


Hi Venkat,
thank you very much for working on this. I really appreciate your time and effort. I've started diving into your patch. I'm not quite done yet, but let me share my thoughts to unblock you.

1) Would you mind running "ant checkstyle" and cleaning up the errors? Some of the errors are already present in trunk, so please feel free to leave them out. Just it would be great if would not include any new errors.

2) Tests do not seem passing for me against Netezza Simulator 6.0. I'm not yet sure why, so I'll investigate that on my end.

3) It seems that you prefer creating chained exceptions in form NewException("String").initCause(oldException). The rest of the code seems to be following different approach and is creating exceptions in form NewException("String", oldException). Do you feel comfortable with change that to have unified code style?



src/java/org/apache/sqoop/manager/DirectNetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36230>

    It seems that I've confused you, so please accept my apologies for that. Would you mind putting the mapreduce specific classes into package "org.apache.sqoop.mapreduce.netezza"? (e.g leaving out the "db" package). You can notice that there are already packages for MySQL and Microsoft SQL Server:
    
    org.apache.sqoop.mapreduce.mysql
    org.apache.sqoop.mapreduce.sqlserver



src/java/org/apache/sqoop/manager/DirectNetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36161>

    I can see similar code fragment in both exportTable() and importTable() methods (just returning different exception class). I would suggest to put it into constructor instead. The other connectors seems to be doing that already.



src/java/org/apache/sqoop/manager/DirectNetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36211>

    I've noticed that this parameter is being used by both Direct and JDBC manager. What about creating method getNetezzaExtraOpts() in JDBC  Manager that will create this single argument and in this method simply add direct specific arguments. Something like:
    
    RelatedOptions netezzaOpts = super.getNetezzaExtraOpts();
    // Add the direct mode specific arguments



src/java/org/apache/sqoop/manager/DirectNetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36193>

    Shouldn't this condition be something like:
    
    ...extraArgs.length != 0 ...



src/java/org/apache/sqoop/manager/DirectNetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36196>

    I'm not quite sure what is the reasoning for skipping extra parameter checking when there is only one mapper?



src/java/org/apache/sqoop/manager/DirectNetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36200>

    Would you mind simplifying this by doing something like:
    
    throw new SqoopOptions.InvalidOptionsException(pe.getMessage(), pe);



src/java/org/apache/sqoop/manager/DirectNetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36195>

    It seems to me that this property is set in both branches of the if-else statement. Would it make sense to set it at one place then?



src/java/org/apache/sqoop/manager/NetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36202>

    I didn't see any reference to this constructor, is it used?



src/java/org/apache/sqoop/manager/NetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36205>

    Would you mind sharing more details about why is the parallel JDBC update dangerous and how it can lead to inconsistencies?



src/java/org/apache/sqoop/manager/NetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36206>

    Shouldn't this condition be something like:
    
    ... extraArgs.length != 0 ...



src/java/org/apache/sqoop/manager/NetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36207>

    I'm not quite sure what is the reasoning for skipping extra parameter checking when there is only one mapper?



src/java/org/apache/sqoop/manager/NetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36210>

    Can we put this into standalone method getNetezzaExtraOpts() similarly as we've done in NetezzaDirectManager?



src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportJob.java
<https://reviews.apache.org/r/9543/#comment36213>

    That comment seems little bit off :-)



src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java
<https://reviews.apache.org/r/9543/#comment36220>

    Shouldn't this be abstract class? It seems to me that it do not make sense when it would be used directly.



src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java
<https://reviews.apache.org/r/9543/#comment36219>

    Can we inherit from SqoopMapper?



src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java
<https://reviews.apache.org/r/9543/#comment36214>

    Can we make similar check on the Sqoop client side? I'm afraid that this warning will get lost as it's printed in the mapper task and I'm not expecting users to read it up there.



src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableInputSplitter.java
<https://reviews.apache.org/r/9543/#comment36223>

    I would suggest to rename this into NetezzaExternalTableInputSplit instead of "..Splitter". 



src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java
<https://reviews.apache.org/r/9543/#comment36255>

    This chaining of exceptions seems problematic to me. The initCause() might not be allowed to call as the exception cause might be already initialized.
    
    It seems that there might be maximally two exceptions. If that is correct then I suggest to return only one instead of the chain. I would personally prioritized the one catched on line 88 over the one on line 95.



src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java
<https://reviews.apache.org/r/9543/#comment36224>

    I would vote for not doing this. The file:// should be default value and by explicitly specifying this, we will loose the ability to run the tests against real Hadoop cluster.



src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java
<https://reviews.apache.org/r/9543/#comment36225>

    It seems to me that both tests are doing exactly the same, is it really necessary?



src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java
<https://reviews.apache.org/r/9543/#comment36231>

    Would you mind fixing this to the proper call:
    
    return args.toArray(new String[args.size()]);



src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java
<https://reviews.apache.org/r/9543/#comment36229>

    I believe that the second semicolon is not needed there :-)



src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java
<https://reviews.apache.org/r/9543/#comment36228>

    Can we print out used Netezza URL, something like:
    
    LOG.info("Using Netezza URL: " + url.toString());
    
    My motivation here is to help users running the test to see what exactly is Sqoop trying to do. It helped me, so I can imagine that it might help others as well.


Jarcec

- Jarek Cecho


On Feb. 25, 2013, 10:02 p.m., Venkat Ranganathan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9543/
> -----------------------------------------------------------
> 
> (Updated Feb. 25, 2013, 10:02 p.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Description
> -------
> 
> This addresses SQOOP-846 (provide a Netezza connector)
> 
> 
> This addresses bug SQOOP-846.
>     https://issues.apache.org/jira/browse/SQOOP-846
> 
> 
> Diffs
> -----
> 
>   src/java/org/apache/sqoop/lib/DelimiterSet.java 4e9bcab 
>   src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 54eb258 
>   src/java/org/apache/sqoop/manager/DirectNetezzaManager.java PRE-CREATION 
>   src/java/org/apache/sqoop/manager/MySQLUtils.java ef18818 
>   src/java/org/apache/sqoop/manager/NetezzaManager.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaDBDataSliceSplitter.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaDataDrivenDBInputFormat.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportJob.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportJob.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableInputFormat.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableInputSplitter.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableRecordExportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableTextExportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java PRE-CREATION 
>   src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java PRE-CREATION 
>   src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java PRE-CREATION 
>   src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/9543/diff/
> 
> 
> Testing
> -------
> 
> Ran all sqoop tests.   Ran Netezza manual tests against Netezza VMs version 6 and 7
> 
> 
> Thanks,
> 
> Venkat Ranganathan
> 
>


Re: Review Request: SQOOP-846 Provide Netezza connector

Posted by Venkat Ranganathan <n....@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9543/
-----------------------------------------------------------

(Updated March 3, 2013, 5:16 a.m.)


Review request for Sqoop and Jarek Cecho.


Changes
-------

New patch - Fixed documentation, more review changes


Description
-------

This addresses SQOOP-846 (provide a Netezza connector)


This addresses bug SQOOP-846.
    https://issues.apache.org/jira/browse/SQOOP-846


Diffs (updated)
-----

  src/docs/user/connectors.txt 7dd2a2e 
  src/java/org/apache/sqoop/lib/DelimiterSet.java 4e9bcab 
  src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 54eb258 
  src/java/org/apache/sqoop/manager/DirectNetezzaManager.java PRE-CREATION 
  src/java/org/apache/sqoop/manager/MySQLUtils.java ef18818 
  src/java/org/apache/sqoop/manager/NetezzaManager.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaDBDataSliceSplitter.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableRecordExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableTextExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/netezza/NetezzaDataDrivenDBInputFormat.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableExportJob.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableImportJob.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableInputFormat.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableInputSplit.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java PRE-CREATION 

Diff: https://reviews.apache.org/r/9543/diff/


Testing
-------

Ran all sqoop tests.   Ran Netezza manual tests against Netezza VMs version 6 and 7


Thanks,

Venkat Ranganathan


Re: Review Request: SQOOP-846 Provide Netezza connector

Posted by Venkat Ranganathan <n....@live.com>.

> On March 2, 2013, 7:26 p.m., Jarek Cecho wrote:
> > Hi Venkat,
> > thank you very much for incorporating all my suggestions. I believe that we're almost there! I do have just couple of additional high level notes and few comments (mostly just nits):
> > 
> > 1) Tests are passing for me - it was something specific to my environment. Thank you for checking though!
> > 
> > 2) The docs are not compiling for me:
> > 
> >      [exec] asciidoc: ERROR: connectors.txt: line 288: [blockdef-listing] missing closing delimiter
> > 
> > 3) I would like to see more tests, especially about testing the various extra arguments. But I'm open to resolve that in follow up jira. No need to postpone this task even further.
> 
> Venkat Ranganathan wrote:
>     Thanks Jarek for reviewing and feedback.
>     
>     1)  Great - I could run it with NPS 6 and 7.  And glad that it is now passing for you.   
>     2)  Fixed the docs.
>     3)  I agree.   I have couple of manual tests but will integrate them by filing followon JIRAs
>     
>     Will update new patch after testing and making sure the documentation is in order also
>     
>     Thanks
>     Venkat
> 
> Jarek Cecho wrote:
>     Thank you for your quick feedback sir!

Hi Jarek

Sorry I missed marking couple of the changes as fixed.   But the patch has not changed (it was already part of the patch)

Thanks

Venkat


- Venkat


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


On March 3, 2013, 5:16 a.m., Venkat Ranganathan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9543/
> -----------------------------------------------------------
> 
> (Updated March 3, 2013, 5:16 a.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Description
> -------
> 
> This addresses SQOOP-846 (provide a Netezza connector)
> 
> 
> This addresses bug SQOOP-846.
>     https://issues.apache.org/jira/browse/SQOOP-846
> 
> 
> Diffs
> -----
> 
>   src/docs/user/connectors.txt 7dd2a2e 
>   src/java/org/apache/sqoop/lib/DelimiterSet.java 4e9bcab 
>   src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 54eb258 
>   src/java/org/apache/sqoop/manager/DirectNetezzaManager.java PRE-CREATION 
>   src/java/org/apache/sqoop/manager/MySQLUtils.java ef18818 
>   src/java/org/apache/sqoop/manager/NetezzaManager.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaDBDataSliceSplitter.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableRecordExportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableTextExportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaDataDrivenDBInputFormat.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableExportJob.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableImportJob.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableInputFormat.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableInputSplit.java PRE-CREATION 
>   src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java PRE-CREATION 
>   src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java PRE-CREATION 
>   src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/9543/diff/
> 
> 
> Testing
> -------
> 
> Ran all sqoop tests.   Ran Netezza manual tests against Netezza VMs version 6 and 7
> 
> 
> Thanks,
> 
> Venkat Ranganathan
> 
>


Re: Review Request: SQOOP-846 Provide Netezza connector

Posted by Jarek Cecho <ja...@apache.org>.

> On March 2, 2013, 7:26 p.m., Jarek Cecho wrote:
> > Hi Venkat,
> > thank you very much for incorporating all my suggestions. I believe that we're almost there! I do have just couple of additional high level notes and few comments (mostly just nits):
> > 
> > 1) Tests are passing for me - it was something specific to my environment. Thank you for checking though!
> > 
> > 2) The docs are not compiling for me:
> > 
> >      [exec] asciidoc: ERROR: connectors.txt: line 288: [blockdef-listing] missing closing delimiter
> > 
> > 3) I would like to see more tests, especially about testing the various extra arguments. But I'm open to resolve that in follow up jira. No need to postpone this task even further.
> 
> Venkat Ranganathan wrote:
>     Thanks Jarek for reviewing and feedback.
>     
>     1)  Great - I could run it with NPS 6 and 7.  And glad that it is now passing for you.   
>     2)  Fixed the docs.
>     3)  I agree.   I have couple of manual tests but will integrate them by filing followon JIRAs
>     
>     Will update new patch after testing and making sure the documentation is in order also
>     
>     Thanks
>     Venkat

Thank you for your quick feedback sir!


> On March 2, 2013, 7:26 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/NetezzaManager.java, line 96
> > <https://reviews.apache.org/r/9543/diff/8/?file=263540#file263540line96>
> >
> >     Shouldn't be safer operation to rollback instead of commit here? I would expect that the rest of code will commit any changes that it needed to do prior calling close method.
> 
> Venkat Ranganathan wrote:
>     Yes.  That is true.  It seems that this is a carry over from the PostgresqlManager.   I will file a JIRA to fix the PostgresqlManager also

Agreed, please go ahead and file that JIRA for PostgreSQL.


- Jarek


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


On Feb. 28, 2013, 8:06 a.m., Venkat Ranganathan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9543/
> -----------------------------------------------------------
> 
> (Updated Feb. 28, 2013, 8:06 a.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Description
> -------
> 
> This addresses SQOOP-846 (provide a Netezza connector)
> 
> 
> This addresses bug SQOOP-846.
>     https://issues.apache.org/jira/browse/SQOOP-846
> 
> 
> Diffs
> -----
> 
>   src/docs/user/connectors.txt 7dd2a2e 
>   src/java/org/apache/sqoop/lib/DelimiterSet.java 4e9bcab 
>   src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 54eb258 
>   src/java/org/apache/sqoop/manager/DirectNetezzaManager.java PRE-CREATION 
>   src/java/org/apache/sqoop/manager/MySQLUtils.java ef18818 
>   src/java/org/apache/sqoop/manager/NetezzaManager.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaDBDataSliceSplitter.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableRecordExportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableTextExportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaDataDrivenDBInputFormat.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableExportJob.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableImportJob.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableInputFormat.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableInputSplit.java PRE-CREATION 
>   src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java PRE-CREATION 
>   src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java PRE-CREATION 
>   src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/9543/diff/
> 
> 
> Testing
> -------
> 
> Ran all sqoop tests.   Ran Netezza manual tests against Netezza VMs version 6 and 7
> 
> 
> Thanks,
> 
> Venkat Ranganathan
> 
>


Re: Review Request: SQOOP-846 Provide Netezza connector

Posted by Venkat Ranganathan <n....@live.com>.

> On March 2, 2013, 7:26 p.m., Jarek Cecho wrote:
> > Hi Venkat,
> > thank you very much for incorporating all my suggestions. I believe that we're almost there! I do have just couple of additional high level notes and few comments (mostly just nits):
> > 
> > 1) Tests are passing for me - it was something specific to my environment. Thank you for checking though!
> > 
> > 2) The docs are not compiling for me:
> > 
> >      [exec] asciidoc: ERROR: connectors.txt: line 288: [blockdef-listing] missing closing delimiter
> > 
> > 3) I would like to see more tests, especially about testing the various extra arguments. But I'm open to resolve that in follow up jira. No need to postpone this task even further.

Thanks Jarek for reviewing and feedback.

1)  Great - I could run it with NPS 6 and 7.  And glad that it is now passing for you.   
2)  Fixed the docs.
3)  I agree.   I have couple of manual tests but will integrate them by filing followon JIRAs

Will update new patch after testing and making sure the documentation is in order also

Thanks
Venkat


> On March 2, 2013, 7:26 p.m., Jarek Cecho wrote:
> > src/docs/user/connectors.txt, line 257
> > <https://reviews.apache.org/r/9543/diff/8/?file=263535#file263535line257>
> >
> >     Nit: Copy paste from PostgreSQL :-)

Thanks :)


> On March 2, 2013, 7:26 p.m., Jarek Cecho wrote:
> > src/docs/user/connectors.txt, lines 259-276
> > <https://reviews.apache.org/r/9543/diff/8/?file=263535#file263535line259>
> >
> >     It seems that the start of second column in the header is somewhere else than rest of the rows. This seems to be causing very weird typesetting of the table. Would you mind taking a look?

Yes.   Fixing it


> On March 2, 2013, 7:26 p.m., Jarek Cecho wrote:
> > src/docs/user/connectors.txt, line 262
> > <https://reviews.apache.org/r/9543/diff/8/?file=263535#file263535line262>
> >
> >     Nit: Would you mind prepending the argument with "\--"

I based on the Postgresql connector doc which was not prepending the --.  But adding it as it seems more natural for users to expect


> On March 2, 2013, 7:26 p.m., Jarek Cecho wrote:
> > src/docs/user/connectors.txt, lines 294-296
> > <https://reviews.apache.org/r/9543/diff/8/?file=263535#file263535line294>
> >
> >     Would you mind enclosing the execution example in "---"? It will be then typeset differently in the HTML docs.

Yes.  Good thing to do


> On March 2, 2013, 7:26 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/DirectNetezzaManager.java, lines 134-135
> > <https://reviews.apache.org/r/9543/diff/8/?file=263538#file263538line134>
> >
> >     Nit: Those two variables seems to be unused in the method.

Fixed.  It was on exportTable.


> On March 2, 2013, 7:26 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/DirectNetezzaManager.java, lines 141-142
> > <https://reviews.apache.org/r/9543/diff/8/?file=263538#file263538line141>
> >
> >     Nit: Return value of the methods is already char, is there a reason for the explicit retyping?

When I moved the check from execution to client side I did not change it where the method used was typing it to int


> On March 2, 2013, 7:26 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/NetezzaManager.java, line 96
> > <https://reviews.apache.org/r/9543/diff/8/?file=263540#file263540line96>
> >
> >     Shouldn't be safer operation to rollback instead of commit here? I would expect that the rest of code will commit any changes that it needed to do prior calling close method.

Yes.  That is true.  It seems that this is a carry over from the PostgresqlManager.   I will file a JIRA to fix the PostgresqlManager also


> On March 2, 2013, 7:26 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/manager/NetezzaManager.java, line 223
> > <https://reviews.apache.org/r/9543/diff/8/?file=263540#file263540line223>
> >
> >     I found this quite confusing. The if(cmdLine.hasOption()) do not have else and thus that path of the execution will leave this property unset. It seems that the default value is false (per NetezzaDataDrivenDBInputFormat). I would suggest to either set this property in all branches or depend on the default value.

Sorry about the confusion.  I cleaned it up and removed the else clause with the default value set unconditionally and optionally reset


> On March 2, 2013, 7:26 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java, line 89
> > <https://reviews.apache.org/r/9543/diff/8/?file=263543#file263543line89>
> >
> >     I believe that Sqoop default NULL substitution string is "null" (lowercase). It might be good idea to stay consistent.

Good point.   Changing it to be consistent


> On March 2, 2013, 7:26 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java, line 47
> > <https://reviews.apache.org/r/9543/diff/8/?file=263546#file263546line47>
> >
> >     Can we rename this method to "printException"?

Yes.  Makes sense.


- Venkat


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


On Feb. 28, 2013, 8:06 a.m., Venkat Ranganathan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9543/
> -----------------------------------------------------------
> 
> (Updated Feb. 28, 2013, 8:06 a.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Description
> -------
> 
> This addresses SQOOP-846 (provide a Netezza connector)
> 
> 
> This addresses bug SQOOP-846.
>     https://issues.apache.org/jira/browse/SQOOP-846
> 
> 
> Diffs
> -----
> 
>   src/docs/user/connectors.txt 7dd2a2e 
>   src/java/org/apache/sqoop/lib/DelimiterSet.java 4e9bcab 
>   src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 54eb258 
>   src/java/org/apache/sqoop/manager/DirectNetezzaManager.java PRE-CREATION 
>   src/java/org/apache/sqoop/manager/MySQLUtils.java ef18818 
>   src/java/org/apache/sqoop/manager/NetezzaManager.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaDBDataSliceSplitter.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableRecordExportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableTextExportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaDataDrivenDBInputFormat.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableExportJob.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableImportJob.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableInputFormat.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableInputSplit.java PRE-CREATION 
>   src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java PRE-CREATION 
>   src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java PRE-CREATION 
>   src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/9543/diff/
> 
> 
> Testing
> -------
> 
> Ran all sqoop tests.   Ran Netezza manual tests against Netezza VMs version 6 and 7
> 
> 
> Thanks,
> 
> Venkat Ranganathan
> 
>


Re: Review Request: SQOOP-846 Provide Netezza connector

Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9543/#review17303
-----------------------------------------------------------


Hi Venkat,
thank you very much for incorporating all my suggestions. I believe that we're almost there! I do have just couple of additional high level notes and few comments (mostly just nits):

1) Tests are passing for me - it was something specific to my environment. Thank you for checking though!

2) The docs are not compiling for me:

     [exec] asciidoc: ERROR: connectors.txt: line 288: [blockdef-listing] missing closing delimiter

3) I would like to see more tests, especially about testing the various extra arguments. But I'm open to resolve that in follow up jira. No need to postpone this task even further.


src/docs/user/connectors.txt
<https://reviews.apache.org/r/9543/#comment36773>

    Nit: Copy paste from PostgreSQL :-)



src/docs/user/connectors.txt
<https://reviews.apache.org/r/9543/#comment36776>

    It seems that the start of second column in the header is somewhere else than rest of the rows. This seems to be causing very weird typesetting of the table. Would you mind taking a look?



src/docs/user/connectors.txt
<https://reviews.apache.org/r/9543/#comment36777>

    Nit: Would you mind prepending the argument with "\--"



src/docs/user/connectors.txt
<https://reviews.apache.org/r/9543/#comment36778>

    Nit: Would you mind prepending the argument with "\--"



src/docs/user/connectors.txt
<https://reviews.apache.org/r/9543/#comment36779>

    Nit: Would you mind prepending the argument with "\--"



src/docs/user/connectors.txt
<https://reviews.apache.org/r/9543/#comment36771>

    Nit: Please remove the trailing whitespace.



src/docs/user/connectors.txt
<https://reviews.apache.org/r/9543/#comment36774>

    Would you mind enclosing the execution example in "---"? It will be then typeset differently in the HTML docs.



src/docs/user/connectors.txt
<https://reviews.apache.org/r/9543/#comment36772>

    Nit: Please remove the trailing whitespace.



src/docs/user/connectors.txt
<https://reviews.apache.org/r/9543/#comment36775>

    Would you mind enclosing the execution example in "---"? It will be then typeset differently in the HTML docs.



src/java/org/apache/sqoop/manager/DirectNetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36780>

    Nit: Can we put the cause into the constructor?



src/java/org/apache/sqoop/manager/DirectNetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36788>

    Nit: Those two variables seems to be unused in the method.



src/java/org/apache/sqoop/manager/DirectNetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36789>

    Nit: Return value of the methods is already char, is there a reason for the explicit retyping?



src/java/org/apache/sqoop/manager/DirectNetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36790>

    Nit: Return value of the methods is already char, is there a reason for the explicit retyping?



src/java/org/apache/sqoop/manager/DirectNetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36783>

    Reading current code, I've realized that that I've misunderstood the purpose last time. The super.getNetezzaExtraOpts() is adding only NETEZZA_DATASLICE_ALIGNED_ACCESS_OPT, but that is always set to true in the Direct connector. Thus I believe that we should not get the super here as we would offer extra argument that is not used in the direct connector. Sorry for the confusion!



src/java/org/apache/sqoop/manager/NetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36781>

    It seems to me that this method just call the parent one, but that will be done by java automatically, right?



src/java/org/apache/sqoop/manager/NetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36782>

    Shouldn't be safer operation to rollback instead of commit here? I would expect that the rest of code will commit any changes that it needed to do prior calling close method.



src/java/org/apache/sqoop/manager/NetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36784>

    I found this quite confusing. The if(cmdLine.hasOption()) do not have else and thus that path of the execution will leave this property unset. It seems that the default value is false (per NetezzaDataDrivenDBInputFormat). I would suggest to either set this property in all branches or depend on the default value.



src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java
<https://reviews.apache.org/r/9543/#comment36791>

    This variable seems to be unused.



src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java
<https://reviews.apache.org/r/9543/#comment36792>

    This variables seems to be unused.



src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java
<https://reviews.apache.org/r/9543/#comment36785>

    I believe that Sqoop default NULL substitution string is "null" (lowercase). It might be good idea to stay consistent.



src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java
<https://reviews.apache.org/r/9543/#comment36786>

    I believe that Sqoop default NULL substitution string is "null" (lowercase). It might be good idea to stay consistent.



src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java
<https://reviews.apache.org/r/9543/#comment36787>

    Can we rename this method to "printException"?


Jarcec

- Jarek Cecho


On Feb. 28, 2013, 8:06 a.m., Venkat Ranganathan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9543/
> -----------------------------------------------------------
> 
> (Updated Feb. 28, 2013, 8:06 a.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Description
> -------
> 
> This addresses SQOOP-846 (provide a Netezza connector)
> 
> 
> This addresses bug SQOOP-846.
>     https://issues.apache.org/jira/browse/SQOOP-846
> 
> 
> Diffs
> -----
> 
>   src/docs/user/connectors.txt 7dd2a2e 
>   src/java/org/apache/sqoop/lib/DelimiterSet.java 4e9bcab 
>   src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 54eb258 
>   src/java/org/apache/sqoop/manager/DirectNetezzaManager.java PRE-CREATION 
>   src/java/org/apache/sqoop/manager/MySQLUtils.java ef18818 
>   src/java/org/apache/sqoop/manager/NetezzaManager.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaDBDataSliceSplitter.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableRecordExportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableTextExportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaDataDrivenDBInputFormat.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableExportJob.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableImportJob.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableInputFormat.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableInputSplit.java PRE-CREATION 
>   src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java PRE-CREATION 
>   src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java PRE-CREATION 
>   src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/9543/diff/
> 
> 
> Testing
> -------
> 
> Ran all sqoop tests.   Ran Netezza manual tests against Netezza VMs version 6 and 7
> 
> 
> Thanks,
> 
> Venkat Ranganathan
> 
>


Re: Review Request: SQOOP-846 Provide Netezza connector

Posted by Venkat Ranganathan <n....@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9543/
-----------------------------------------------------------

(Updated Feb. 28, 2013, 8:06 a.m.)


Review request for Sqoop and Jarek Cecho.


Changes
-------

Removed the DirectMySQLExportTest changes from this work which was inadvertently added


Description
-------

This addresses SQOOP-846 (provide a Netezza connector)


This addresses bug SQOOP-846.
    https://issues.apache.org/jira/browse/SQOOP-846


Diffs (updated)
-----

  src/docs/user/connectors.txt 7dd2a2e 
  src/java/org/apache/sqoop/lib/DelimiterSet.java 4e9bcab 
  src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 54eb258 
  src/java/org/apache/sqoop/manager/DirectNetezzaManager.java PRE-CREATION 
  src/java/org/apache/sqoop/manager/MySQLUtils.java ef18818 
  src/java/org/apache/sqoop/manager/NetezzaManager.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaDBDataSliceSplitter.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableRecordExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableTextExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/netezza/NetezzaDataDrivenDBInputFormat.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableExportJob.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableImportJob.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableInputFormat.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableInputSplit.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java PRE-CREATION 

Diff: https://reviews.apache.org/r/9543/diff/


Testing
-------

Ran all sqoop tests.   Ran Netezza manual tests against Netezza VMs version 6 and 7


Thanks,

Venkat Ranganathan


Re: Review Request: SQOOP-846 Provide Netezza connector

Posted by Venkat Ranganathan <n....@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9543/
-----------------------------------------------------------

(Updated Feb. 28, 2013, 1:53 a.m.)


Review request for Sqoop and Jarek Cecho.


Changes
-------

Sorry about the previous patch.  I had missed the newly refactored files when building a consolidated patch.   Now included it.

All checkstyle issues fixed
All other comments and suggestions from Jarcec in place (Thanks Jarcec)


Description
-------

This addresses SQOOP-846 (provide a Netezza connector)


This addresses bug SQOOP-846.
    https://issues.apache.org/jira/browse/SQOOP-846


Diffs (updated)
-----

  src/docs/user/connectors.txt 7dd2a2e 
  src/java/org/apache/sqoop/lib/DelimiterSet.java 4e9bcab 
  src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 54eb258 
  src/java/org/apache/sqoop/manager/DirectNetezzaManager.java PRE-CREATION 
  src/java/org/apache/sqoop/manager/MySQLUtils.java ef18818 
  src/java/org/apache/sqoop/manager/NetezzaManager.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaDBDataSliceSplitter.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableRecordExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableTextExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/netezza/NetezzaDataDrivenDBInputFormat.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableExportJob.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableImportJob.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableInputFormat.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableInputSplit.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/DirectMySQLExportTest.java eac7836 
  src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java PRE-CREATION 

Diff: https://reviews.apache.org/r/9543/diff/


Testing
-------

Ran all sqoop tests.   Ran Netezza manual tests against Netezza VMs version 6 and 7


Thanks,

Venkat Ranganathan


Re: Review Request: SQOOP-846 Provide Netezza connector

Posted by Venkat Ranganathan <n....@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9543/
-----------------------------------------------------------

(Updated Feb. 27, 2013, 11:48 p.m.)


Review request for Sqoop and Jarek Cecho.


Changes
-------

This takes care of the latest comments from Jarcec.   Checkstyle violations have been fixed.   Validated the runs of unit tests.    
Connector Document has been updated and uploaded to the JIRA.   That explains the update table restriction.

Thanks Jarcec for a thorough review. 


Description
-------

This addresses SQOOP-846 (provide a Netezza connector)


This addresses bug SQOOP-846.
    https://issues.apache.org/jira/browse/SQOOP-846


Diffs (updated)
-----

  src/docs/user/connectors.txt 7dd2a2e 
  src/java/org/apache/sqoop/lib/DelimiterSet.java 4e9bcab 
  src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 54eb258 
  src/java/org/apache/sqoop/manager/DirectNetezzaManager.java PRE-CREATION 
  src/java/org/apache/sqoop/manager/MySQLUtils.java ef18818 
  src/java/org/apache/sqoop/manager/NetezzaManager.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaDBDataSliceSplitter.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableRecordExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableTextExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/DirectMySQLExportTest.java eac7836 
  src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java PRE-CREATION 

Diff: https://reviews.apache.org/r/9543/diff/


Testing
-------

Ran all sqoop tests.   Ran Netezza manual tests against Netezza VMs version 6 and 7


Thanks,

Venkat Ranganathan


Re: Review Request: SQOOP-846 Provide Netezza connector

Posted by Venkat Ranganathan <n....@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9543/
-----------------------------------------------------------

(Updated Feb. 25, 2013, 10:02 p.m.)


Review request for Sqoop and Jarek Cecho.


Changes
-------

Updated a new patch with all changes in one


Description
-------

This addresses SQOOP-846 (provide a Netezza connector)


This addresses bug SQOOP-846.
    https://issues.apache.org/jira/browse/SQOOP-846


Diffs (updated)
-----

  src/java/org/apache/sqoop/lib/DelimiterSet.java 4e9bcab 
  src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 54eb258 
  src/java/org/apache/sqoop/manager/DirectNetezzaManager.java PRE-CREATION 
  src/java/org/apache/sqoop/manager/MySQLUtils.java ef18818 
  src/java/org/apache/sqoop/manager/NetezzaManager.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaDBDataSliceSplitter.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaDataDrivenDBInputFormat.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportJob.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportJob.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableInputFormat.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableInputSplitter.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableRecordExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableTextExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java PRE-CREATION 

Diff: https://reviews.apache.org/r/9543/diff/


Testing
-------

Ran all sqoop tests.   Ran Netezza manual tests against Netezza VMs version 6 and 7


Thanks,

Venkat Ranganathan


Re: Review Request: SQOOP-846 Provide Netezza connector

Posted by Venkat Ranganathan <n....@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9543/
-----------------------------------------------------------

(Updated Feb. 25, 2013, 8:53 p.m.)


Review request for Sqoop and Jarek Cecho.


Description
-------

This addresses SQOOP-846 (provide a Netezza connector)


This addresses bug SQOOP-846.
    https://issues.apache.org/jira/browse/SQOOP-846


Diffs (updated)
-----

  src/java/org/apache/sqoop/lib/DelimiterSet.java 4e9bcab 
  src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 54eb258 
  src/java/org/apache/sqoop/manager/DirectNetezzaManager.java PRE-CREATION 
  src/java/org/apache/sqoop/manager/MySQLUtils.java ef18818 
  src/java/org/apache/sqoop/manager/NetezzaManager.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaDBDataSliceSplitter.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaDataDrivenDBInputFormat.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportJob.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportJob.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableInputFormat.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableInputSplitter.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableRecordExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableTextExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java PRE-CREATION 

Diff: https://reviews.apache.org/r/9543/diff/


Testing
-------

Ran all sqoop tests.   Ran Netezza manual tests against Netezza VMs version 6 and 7


Thanks,

Venkat Ranganathan


Re: Review Request: SQOOP-846 Provide Netezza connector

Posted by Venkat Ranganathan <n....@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9543/
-----------------------------------------------------------

(Updated Feb. 25, 2013, 7:50 p.m.)


Review request for Sqoop and Jarek Cecho.


Changes
-------

New patch with changes - will upload the patch to the JIRA also as I am forced to use a parent diff that messes up reviewing capabilities


Description
-------

This addresses SQOOP-846 (provide a Netezza connector)


This addresses bug SQOOP-846.
    https://issues.apache.org/jira/browse/SQOOP-846


Diffs (updated)
-----

  src/java/org/apache/sqoop/lib/DelimiterSet.java 4e9bcab 
  src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 54eb258 
  src/java/org/apache/sqoop/manager/DirectNetezzaManager.java PRE-CREATION 
  src/java/org/apache/sqoop/manager/DirectNetezzaManager.java PRE-CREATION 
  src/java/org/apache/sqoop/manager/DirectNetezzaManager.java PRE-CREATION 
  src/java/org/apache/sqoop/manager/DirectNetezzaManager.java PRE-CREATION 
  src/java/org/apache/sqoop/manager/MySQLUtils.java ef18818 
  src/java/org/apache/sqoop/manager/MySQLUtils.java ef18818 
  src/java/org/apache/sqoop/manager/NetezzaManager.java PRE-CREATION 
  src/java/org/apache/sqoop/manager/NetezzaManager.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaDBDataSliceSplitter.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaDBDataSliceSplitter.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaDataDrivenDBInputFormat.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaDataDrivenDBInputFormat.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableExportJob.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableExportJob.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableExportJob.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableImportJob.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableImportJob.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableImportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableImportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableImportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableInputFormat.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableInputFormat.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableInputSplitter.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableInputSplitter.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableRecordExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableRecordExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableTextExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableTextExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaJDBCStatementRunner.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaJDBCStatementRunner.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaDBDataSliceSplitter.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaDataDrivenDBInputFormat.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportJob.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportJob.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableInputFormat.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableInputSplitter.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableRecordExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableTextExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java PRE-CREATION 

Diff: https://reviews.apache.org/r/9543/diff/


Testing
-------

Ran all sqoop tests.   Ran Netezza manual tests against Netezza VMs version 6 and 7


Thanks,

Venkat Ranganathan


Re: Review Request: SQOOP-846 Provide Netezza connector

Posted by Venkat Ranganathan <n....@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9543/
-----------------------------------------------------------

(Updated Feb. 25, 2013, 7:46 p.m.)


Review request for Sqoop and Jarek Cecho.


Description
-------

This addresses SQOOP-846 (provide a Netezza connector)


This addresses bug SQOOP-846.
    https://issues.apache.org/jira/browse/SQOOP-846


Diffs
-----

  src/java/org/apache/sqoop/manager/DirectNetezzaManager.java PRE-CREATION 
  src/java/org/apache/sqoop/manager/MySQLUtils.java ef18818 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableExportJob.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableImportMapper.java PRE-CREATION 

Diff: https://reviews.apache.org/r/9543/diff/


Testing
-------

Ran all sqoop tests.   Ran Netezza manual tests against Netezza VMs version 6 and 7


Thanks,

Venkat Ranganathan


Re: Review Request: SQOOP-846 Provide Netezza connector

Posted by Venkat Ranganathan <n....@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9543/
-----------------------------------------------------------

(Updated Feb. 25, 2013, 7:43 p.m.)


Review request for Sqoop and Jarek Cecho.


Description
-------

This addresses SQOOP-846 (provide a Netezza connector)


This addresses bug SQOOP-846.
    https://issues.apache.org/jira/browse/SQOOP-846


Diffs
-----

  src/java/org/apache/sqoop/manager/DirectNetezzaManager.java PRE-CREATION 
  src/java/org/apache/sqoop/manager/MySQLUtils.java ef18818 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableExportJob.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableImportMapper.java PRE-CREATION 

Diff: https://reviews.apache.org/r/9543/diff/


Testing
-------

Ran all sqoop tests.   Ran Netezza manual tests against Netezza VMs version 6 and 7


Thanks,

Venkat Ranganathan


Re: Review Request: SQOOP-846 Provide Netezza connector

Posted by Venkat Ranganathan <n....@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9543/
-----------------------------------------------------------

(Updated Feb. 22, 2013, 7:58 a.m.)


Review request for Sqoop and Jarek Cecho.


Changes
-------

Updates from review

Removed additional options.  Fixed whitespace issues and some comments
Fixed exportjob for NZ external tables to use the input delimiters


Description
-------

This addresses SQOOP-846 (provide a Netezza connector)


This addresses bug SQOOP-846.
    https://issues.apache.org/jira/browse/SQOOP-846


Diffs (updated)
-----

  src/java/org/apache/sqoop/manager/DirectNetezzaManager.java PRE-CREATION 
  src/java/org/apache/sqoop/manager/MySQLUtils.java ef18818 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableExportJob.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableExportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableImportMapper.java PRE-CREATION 

Diff: https://reviews.apache.org/r/9543/diff/


Testing
-------

Ran all sqoop tests.   Ran Netezza manual tests against Netezza VMs version 6 and 7


Thanks,

Venkat Ranganathan