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

Review request for Sqoop.
Summary (updated)
-----------------

SQOOP-540: Microsoft SQL Connector doesn't support custom schemas
Description (updated)
-------

I've chosen similar implementation path as was used in PostgreSQL Connector.
This addresses bug SQOOP-540.
    https://issues.apache.org/jira/browse/SQOOP-540
Diffs
-----

  src/java/org/apache/sqoop/manager/SQLServerManager.java 7ce1edd
  src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBRecordReader.java a56b93d
  src/test/com/cloudera/sqoop/manager/SQLServerManagerExportManualTest.java PRE-CREATION
  src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java 0f949f4

Diff: https://reviews.apache.org/r/7369/diff/
Testing (updated)
-------

I've test my changes (both import and export) against Microsoft SQL Server Express 2012.
Thanks,

Jarek Cecho

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7369/#review12071
-----------------------------------------------------------
Looks good. I have one question regarding CONNECTOR_FACTORY as below.

I also found "ant checkstyle" fails with an error. Although the error is not introduced by this patch, can you please address it in this patch too?

-      if((Boolean)isSecurityEnabled.invoke(null)) {
+      if ((Boolean)isSecurityEnabled.invoke(null)) {

Thanks!
src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java
<https://reviews.apache.org/r/7369/#comment25722>

    This was added to make it easier to test the MS connector (http://www.microsoft.com/en-us/download/details.aspx?id=27584). Are you removing this because testing the MS connector is not that useful anymore?
    
    If that's the intention, can you please remove relevant code in build.xml (line 261 - 271) too?
- Cheolsoo Park
On Oct. 1, 2012, 12:19 a.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7369/
> -----------------------------------------------------------
>
> (Updated Oct. 1, 2012, 12:19 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've chosen similar implementation path as was used in PostgreSQL Connector.
>
>
> This addresses bug SQOOP-540.
>     https://issues.apache.org/jira/browse/SQOOP-540
>
>
> Diffs
> -----
>
>   src/java/org/apache/sqoop/manager/SQLServerManager.java 7ce1edd
>   src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBRecordReader.java a56b93d
>   src/test/com/cloudera/sqoop/manager/SQLServerManagerExportManualTest.java PRE-CREATION
>   src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java 0f949f4
>
> Diff: https://reviews.apache.org/r/7369/diff/
>
>
> Testing
> -------
>
> I've test my changes (both import and export) against Microsoft SQL Server Express 2012.
>
>
> Thanks,
>
> Jarek Cecho
>
>

> On Oct. 1, 2012, 6:39 p.m., Cheolsoo Park wrote:
> > Looks good. I have one question regarding CONNECTOR_FACTORY as below.
> >
> > I also found "ant checkstyle" fails with an error. Although the error is not introduced by this patch, can you please address it in this patch too?
> >
> > -      if((Boolean)isSecurityEnabled.invoke(null)) {
> > +      if ((Boolean)isSecurityEnabled.invoke(null)) {
> >
> > Thanks!

Thank you very much for your input sir. I appreciate your time.
> On Oct. 1, 2012, 6:39 p.m., Cheolsoo Park wrote:
> > src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java, line 85
> > <https://reviews.apache.org/r/7369/diff/1/?file=167761#file167761line85>
> >
> >     This was added to make it easier to test the MS connector (http://www.microsoft.com/en-us/download/details.aspx?id=27584). Are you removing this because testing the MS connector is not that useful anymore?
> >    
> >     If that's the intention, can you please remove relevant code in build.xml (line 261 - 271) too?

I've actually misunderstand the purpose of this part of the code and removed it under impressions that it's outdated. I'll put it back and I'll also add it to newly introduced Export test case.
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7369/#review12071
-----------------------------------------------------------
On Oct. 1, 2012, 12:19 a.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7369/
> -----------------------------------------------------------
>
> (Updated Oct. 1, 2012, 12:19 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've chosen similar implementation path as was used in PostgreSQL Connector.
>
>
> This addresses bug SQOOP-540.
>     https://issues.apache.org/jira/browse/SQOOP-540
>
>
> Diffs
> -----
>
>   src/java/org/apache/sqoop/manager/SQLServerManager.java 7ce1edd
>   src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBRecordReader.java a56b93d
>   src/test/com/cloudera/sqoop/manager/SQLServerManagerExportManualTest.java PRE-CREATION
>   src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java 0f949f4
>
> Diff: https://reviews.apache.org/r/7369/diff/
>
>
> Testing
> -------
>
> I've test my changes (both import and export) against Microsoft SQL Server Express 2012.
>
>
> Thanks,
>
> Jarek Cecho
>
>

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

(Updated Oct. 1, 2012, 10:18 p.m.)
Review request for Sqoop.
Changes
-------

I've incorporated Cheolsoo's comments.
Description
-------

I've chosen similar implementation path as was used in PostgreSQL Connector.
This addresses bug SQOOP-540.
    https://issues.apache.org/jira/browse/SQOOP-540
Diffs (updated)
-----

  src/java/org/apache/sqoop/manager/SQLServerManager.java 7ce1edd
  src/java/org/apache/sqoop/mapreduce/HBaseImportJob.java a6e5546
  src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBRecordReader.java a56b93d
  src/test/com/cloudera/sqoop/manager/SQLServerManagerExportManualTest.java PRE-CREATION
  src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java 0f949f4

Diff: https://reviews.apache.org/r/7369/diff/
Testing
-------

I've test my changes (both import and export) against Microsoft SQL Server Express 2012.
Thanks,

Jarek Cecho

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

Ship it!
Ship It!

- Cheolsoo Park
On Oct. 1, 2012, 10:18 p.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7369/
> -----------------------------------------------------------
>
> (Updated Oct. 1, 2012, 10:18 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've chosen similar implementation path as was used in PostgreSQL Connector.
>
>
> This addresses bug SQOOP-540.
>     https://issues.apache.org/jira/browse/SQOOP-540
>
>
> Diffs
> -----
>
>   src/java/org/apache/sqoop/manager/SQLServerManager.java 7ce1edd
>   src/java/org/apache/sqoop/mapreduce/HBaseImportJob.java a6e5546
>   src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBRecordReader.java a56b93d
>   src/test/com/cloudera/sqoop/manager/SQLServerManagerExportManualTest.java PRE-CREATION
>   src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java 0f949f4
>
> Diff: https://reviews.apache.org/r/7369/diff/
>
>
> Testing
> -------
>
> I've test my changes (both import and export) against Microsoft SQL Server Express 2012.
>
>
> Thanks,
>
> Jarek Cecho
>
>