Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature][Jdbc-Connector] Add OceanBase Connector #4626

Closed
wants to merge 8 commits into from

Conversation

changhuyan
Copy link
Contributor

@changhuyan changhuyan commented Apr 20, 2023

Purpose of this pull request

Add OceanBase Connector

Check list

  • Code changed are covered with tests, or it does not need tests for reason:
  • If any new Jar binary package adding in your PR, please add License Notice according
    New License Guide
  • If necessary, please update the documentation to describe the new feature. https://github.com/apache/incubator-seatunnel/tree/dev/docs
  • If you are contributing the connector code, please check that the following files are updated:
    1. Update change log that in connector document. For more details you can refer to [connector-v2]
      Examples are as follows:
      env {
      job.name = "SeaTunnel"
      spark.executor.instances = 1
      spark.executor.cores = 1
      spark.executor.memory = "1g"
      spark.master = local
      }

source {
jdbc{
url = "jdbc:oceanbase://xxxxx:2881/test"
driver = "com.alipay.oceanbase.jdbc.Driver"
user = "xxx"
password = "xxx"
query = "select id,name from test"
driver_type = "mysql"
}
}

transform {
}

sink {
jdbc {
url = "jdbc:oceanbase://xxx:2881/test"
driver = "com.alipay.oceanbase.jdbc.Driver"
user = "root"
password = "xxxxx"
driver_type = "oracle"
query = "insert into test3(id,name) values(?,?)"
}
}
3. Update plugin-mapping.properties and add new connector information in it
4. Update the pom file of seatunnel-dist

Copy link
Member

@hailin0 hailin0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -31,7 +31,7 @@ public class SeaTunnelEngineExample {

public static void main(String[] args)
throws FileNotFoundException, URISyntaxException, CommandException {
String configurePath = args.length > 0 ? args[0] : "/examples/fake_to_console.conf";
String configurePath = args.length > 0 ? args[0] : "/examples/oceabase.conf";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert

@@ -26,7 +26,7 @@ public class SeaTunnelApiExample {

public static void main(String[] args)
throws FileNotFoundException, URISyntaxException, CommandException {
String configurePath = args.length > 0 ? args[0] : "/examples/spark.batch.conf";
String configurePath = args.length > 0 ? args[0] : "/examples/oceabase.conf";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert

@hailin0
Copy link
Member

hailin0 commented Apr 20, 2023

@hailin0
Copy link
Member

hailin0 commented Apr 21, 2023

please check ci error

@@ -26,7 +26,7 @@ public class SeaTunnelApiExample {

public static void main(String[] args)
throws FileNotFoundException, URISyntaxException, CommandException {
String configurePath = args.length > 0 ? args[0] : "/examples/spark.batch.conf";
String configurePath = args.length > 0 ? args[0] : "/examples/fake_to_console.conf";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@@ -144,7 +151,10 @@
<groupId>mysql</groupId>
<artifactId>mysql-connector-java</artifactId>
</dependency>

<dependency>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

public boolean acceptsURL(String url, Optional<String> driverTye) {
return url.startsWith("jdbc:oceanbase:")
&& driverTye.isPresent()
&& driverTye.get().equalsIgnoreCase("mysql");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add example into docs

1、Modify ci and code style error
2、example into docs
3、add this drive into
@hailin0
Copy link
Member

hailin0 commented Apr 22, 2023

  1. check ci
  2. add e2e testcase

Comment on lines +58 to +59
+ " `age` varchar(255) DEFAULT NULL,\n"
+ " `name` varchar(255) DEFAULT NULL\n"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test all datatypes

}

@Override
void compareResult() {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

implement this logic

@@ -33,6 +33,7 @@ support `Xa transactions`. You can set `is_exactly_once=true` to enable it.
| user | String | No | - |
| password | String | No | - |
| query | String | No | - |
| driver_type | String | No | - |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import java.util.List;
import java.util.Map;

@Disabled("Disabled because it needs user's personal oceanbase account to run this test!")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NickCodeJourney
Copy link
Contributor

why not use mysql connector

@changhuyan
Copy link
Contributor Author

changhuyan commented May 29, 2023 via email

@@ -33,7 +35,7 @@ public interface JdbcDialectFactory {
* @return <code>true</code> if this dialect understands the given URL; <code>false</code>
* otherwise.
*/
boolean acceptsURL(String url);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the driverType param shoud be passed in the create method below, and we can use a new default method to keep the other dialects unchanged.

default JdbcDialect create(String driverType) {
    return create();
}


@Override
public JdbcDialect create() {
return new MysqlDialect();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can merge the two dialect classes of OceanBase and return the JdbcDialect by the driverType param here.

public JdbcDialect create(@Nonnull String driverType) {
    if ("mysql".equalsIgnoreCase(driverType)) {
        return new MysqlDialect();
    }
    return new OracleDialect();
}

<artifactId>oceanbase-client</artifactId>
<version>${oceanbase.version}</version>
<scope>provided</scope>
</dependency>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should not be imported directly due to the lgpl license https://github.com/oceanbase/obconnector-j

<artifactId>oceanbase-client</artifactId>
<version>${oceanbase.version}</version>
<scope>provided</scope>
</dependency>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should not use the previous internal version, just keep it same with seatunnel-connectors-v2/connector-jdbc/pom.xml


@Disabled("Disabled because it needs user's personal oceanbase account to run this test!")
public class JdbcOceanbaseIT extends AbstractJdbcIT {
private static final String OCEANBASE_IMAGE = "shihd/oceanbase:1.0";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use the docker image oceanbase/oceanbase-ce:4.0.0.0 with user root and empty password.

@changhuyan
Copy link
Contributor Author

changhuyan commented Jun 27, 2023 via email

@whhe
Copy link
Member

whhe commented Jun 27, 2023

@changhuyan Thanks for your contributation! I left some comments, and you can touch me at any time if you have any ideas on them.

For the reference to oceanbase-client, I think it may be an appropriate method to guide users to download it manually through the document, but I'm not sure how this affects the integration tests.

@whhe
Copy link
Member

whhe commented Jun 28, 2023

Yes, there's a better way. You can change it.

------------------ 原始邮件 ------------------ 发件人: "apache/seatunnel" @.>; 发送时间: 2023年6月27日(星期二) 晚上8:04 @.>; @.@.>; 主题: Re: [apache/seatunnel] [Feature][Jdbc-Connector] Add OceanBase Connector (PR #4626) @whhe requested changes on this pull request. In seatunnel-connectors-v2/connector-jdbc/src/main/java/org/apache/seatunnel/connectors/seatunnel/jdbc/internal/dialect/JdbcDialectFactory.java: > @@ -33,7 +35,7 @@ public interface JdbcDialectFactory { * @return <code>true</code> if this dialect understands the given URL; <code>false</code> * otherwise. / - boolean acceptsURL(String url); I think the driverType param shoud be passed in the create method below, and we can use a new default method to keep the other dialects unchanged. default JdbcDialect create(String driverType) { return create(); } In seatunnel-connectors-v2/connector-jdbc/src/main/java/org/apache/seatunnel/connectors/seatunnel/jdbc/internal/dialect/oceanbase/OceanBaseMySqlDialectFactory.java: > +import com.google.auto.service.AutoService; + +import java.util.Optional; + @.(JdbcDialectFactory.class) +public class OceanBaseMySqlDialectFactory implements JdbcDialectFactory { + @OverRide + public boolean acceptsURL(String url, Optional<String> driverTye) { + return url.startsWith("jdbc:oceanbase:") + && driverTye.isPresent() + && driverTye.get().equalsIgnoreCase("mysql"); + } + + @OverRide + public JdbcDialect create() { + return new MysqlDialect(); We can merge the two dialect classes of OceanBase and return the JdbcDialect by the driverType param here. public JdbcDialect @.* String driverType) { if ("mysql".equalsIgnoreCase(driverType)) { return new MysqlDialect(); } return new OracleDialect(); } In seatunnel-connectors-v2/connector-jdbc/pom.xml: > @@ -129,6 +130,12 @@ <version>${vertica.version}</version> <scope>provided</scope> </dependency> + <dependency> + <groupId>com.oceanbase</groupId> + <artifactId>oceanbase-client</artifactId> + <version>${oceanbase.version}</version> + <scope>provided</scope> + </dependency> It should not be imported directly due to the lgpl license https://github.com/oceanbase/obconnector-j In seatunnel-dist/pom.xml: > @@ -488,6 +489,12 @@ </dependency> <!-- jdbc driver --> + <dependency> + <groupId>com.alipay.oceanbase</groupId> + <artifactId>oceanbase-client</artifactId> + <version>${oceanbase.version}</version> + <scope>provided</scope> + </dependency> Should not use the previous internal version, just keep it same with seatunnel-connectors-v2/connector-jdbc/pom.xml In seatunnel-e2e/seatunnel-connector-v2-e2e/connector-jdbc-e2e/connector-jdbc-e2e-part-2/src/test/java/org/apache/seatunnel/connectors/seatunnel/jdbc/JdbcOceanbaseIT.java: > +import org.junit.jupiter.api.Disabled; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.containers.output.Slf4jLogConsumer; +import org.testcontainers.utility.DockerImageName; +import org.testcontainers.utility.DockerLoggerFactory; + +import com.google.common.collect.Lists; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + @.("Disabled because it needs user's personal oceanbase account to run this test!") +public class JdbcOceanbaseIT extends AbstractJdbcIT { + private static final String OCEANBASE_IMAGE = "shihd/oceanbase:1.0"; You can use the docker image oceanbase/oceanbase-ce:4.0.0.0 with user root and empty password. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.>

Do you have time to complete this work recently? If not, we can also find other developers in the community to continue development based on your work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants