Fixing "fix version" as it was empty.

Integrated in Sqoop-ant-jdk-1.6 #120 (See https://builds.apache.org/job/Sqoop-ant-jdk-1.6/120/)
SQOOP-480 MS SQL server connector is incompatible with Sqoop-1.4 (Revision 1347112)

Result = SUCCESS
blee :
Files :

• /sqoop/trunk/build.xml
• /sqoop/trunk/src/java/com/cloudera/sqoop/manager/ImportJobContext.java
• /sqoop/trunk/src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java

Thanks, Cheolsoo! Patch is committed.

Thank you Bilung! Agreed, I removed the typecast and updated the patch.

This is because the setter method "setConnManager" actually has a slightly different argument (ConnManager under apache namespace) now than before (ConnManager under cloudera namespace). Since signature is bounded at compile time with the earlier version, the exception is thrown as a result when running against a newer version.

Your fix looks ok to me except you may want to remove the casting as it is unnecessary as you already mentioned.

After having a discussion with Bilung, I wrote a simpler test case that reproduces the issue of this jira. I'd like to see if anyone has better suggestions on this problem:

1. I define class Foo and Bar in package Name1 and Name2 and export them as Jar1.jar:

package Name1;

public class Foo {
  protected Bar b;

  public Name1.Bar getBar() {
    return b;
  }

  public void setBar(Name1.Bar b) {
    this.b = b;
  }
}

package Name1;

public class Bar {

}

package Name2;

public class Foo extends Name1.Foo {

  public Name2.Bar getBar() {
    return (Name2.Bar)b;
  }
}

package Name2;

public class Bar extends Name1.Bar {

}

2. I also define Foo and Bar in package Name2 and export them as Jar2.jar:

package Name2;

public class Foo {
  protected Name2.Bar b;

  public Name2.Bar getBar() {
    return b;
  }

  public void setBar(Name2.Bar b) {
    this.b = b;
  }
}

package Name2;

public class Bar {

}

3. I compile the following Main class against Jar2.jar and export it as Test.jar.

public class Main {

  public static void main(String[] args) {

    Name2.Foo name2Foo = new Name2.Foo();
    Name2.Bar name2Bar = new Name2.Bar();

    name2Foo.setBar(name2Bar);
    Name2.Bar b = name2Foo.getBar();

    System.out.println(b.toString());
  }

}

4. Now I run Main with the following command:

java -jar Test.jar

This works if Jar2.jar is in classpath (not surprising since it is compiled against Jar2.jar), but it doesn't work if Jar1.jar is in classpath. Specifically, it fails with the following error:

Exception in thread "main" java.lang.NoSuchMethodError: Name2.Foo.setBar(LName2/Bar;)V
	at Main.main(Main.java:9)

What's interesting is that Name2.Foo inherits setBar(LName2/Bar;)V from Name1.Foo, so setBar(LName2/Bar;)V should be visible in Name2.Foo. However, this doesn't seem to happen if Test.java is compiled against Jar2.jar and run with Jar1.jar.

Obviously, the easiest and safest way to fix is to re-compile Test.java against Jar1.jar. But it may be not always feasible if Test.java is 3rd-party software (In this jira, Test.java is Microsoft Sqoop connector).

Another workaround that I implemented in my patch is to explicitly define Name2.Foo.setBar(Name2.Bar b) in Jar1.jar as follows:

public void setBar(Name2.Bar b) {
  super.setBar((Name1.Bar)b);
}

Note that Name2.Foo.getBar() in Jar1.jar must be defined regardless because the return type of Name1.Foo.getBar() is Name1.Bar while that of Name2.Foo.getBar() is Name2.Bar, and Name1.Bar cannot be auto-cast to Name2.Bar. But Name2.Foo.setBar(Name2.Bar b) is somewhat redundant because Name2.Bar can be auto-cast to Name1.Bar.

Thoughts?

To test the patch, please follow the following instructions:

1) Set up MS SQL server.
2) Download MS connector to sqoop.thirdparty.lib.dir: http://www.microsoft.com/en-us/download/details.aspx?id=27584
3) Add the following line to build.properties: sqoop.test.msserver.connector.factory=com.microsoft.sqoop.SqlServer.MSSQLServerManagerFactory
4) ant test -Dtestcase=SQLServerManagerImportManualTest

If the property in #3 is not set, the test uses the built-in SQL manager.

I just found that this issue exists with OraOop connector as well.

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

(Updated 2012-05-02 22:33:28.094041)

Review request for Sqoop.

Summary
-------

Sqoop-1.4.* is incompatible with Microsoft SQL server connector due to the namespace migration from com.cloudera to org.apache.

The changes include:
1) Add a wrapper method setConnManager(com.cloudera.sqoop.manager.ConnManager) to ImportJobContext.
2) Enhance SQL server manager test so that it can be used with Microsoft connector as well.

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

Diffs

/build.xml 1333179
/src/java/com/cloudera/sqoop/manager/ImportJobContext.java 1333179
/src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java 1333179

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

Testing
-------

1) Ran SQLServerManagerImportTest using the Microsoft connector.
2) Ran ant test, ant test -Dthirdparty=true, and ant checkstyle.

Thanks,

Cheolsoo

Defining setConnManager(Lcom/cloudera/sqoop/manager/ConnManager;)V in com.cloudera.sqoop.manager.ImportJobContext fixes the problem:

public void setConnManager(ConnManager connManager) {
  super.setConnManager((org.apache.sqoop.manager.ConnManager)connManager);
}

I am running some tests with SQL server before posting my patch.