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][Connector-V2][Jdbc] support gbase 8a #3026

Merged
merged 27 commits into from
Oct 19, 2022

Conversation

liugddx
Copy link
Member

@liugddx liugddx commented Oct 8, 2022

close #3016

Purpose of this pull request

gbase 8a source connector.

Check list

@liugddx liugddx marked this pull request as draft October 8, 2022 15:28
@liugddx liugddx marked this pull request as ready for review October 11, 2022 09:18
@liugddx
Copy link
Member Author

liugddx commented Oct 11, 2022

please run CI @EricJoy2048

@liugddx
Copy link
Member Author

liugddx commented Oct 11, 2022

gbase8a must use sql date java.sql.Date,java.sql.Time,java.sql.Timestamp,otherwise it will be abnormal.

Exception in thread "main" com.gbase.jdbc.GBaseDataTruncation: Data truncation: Incorrect date value: '******'
	at com.gbase.jdbc.GBaseIO.checkErrorPacket(GBaseIO.java:3631)
	at com.gbase.jdbc.GBaseIO.checkErrorPacket(GBaseIO.java:3565)
	at com.gbase.jdbc.GBaseIO.sendCommand(GBaseIO.java:1996)
	at com.gbase.jdbc.GBaseIO.sqlQueryDirect(GBaseIO.java:2150)
	at com.gbase.jdbc.ConnectionImpl.execSQL(ConnectionImpl.java:2652)
	at com.gbase.jdbc.PreparedStatement.executeInternal(PreparedStatement.java:2133)
	at com.gbase.jdbc.PreparedStatement.executeUpdate(PreparedStatement.java:2429)
	at com.gbase.jdbc.PreparedStatement.executeUpdate(PreparedStatement.java:2347)
	at com.gbase.jdbc.PreparedStatement.executeUpdate(PreparedStatement.java:2332)

@hailin0
Copy link
Member

hailin0 commented Oct 12, 2022

please fix ci error

@liugddx
Copy link
Member Author

liugddx commented Oct 12, 2022 via email


@Override
public SeaTunnelRow toInternal(ResultSet rs, ResultSetMetaData metaData, SeaTunnelRowType typeInfo) throws SQLException {
List<Object> fields = new ArrayList<>();
Copy link
Member

Choose a reason for hiding this comment

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

gave a init length is better.


for (int i = 1; i <= seaTunnelDataTypes.length; i++) {
Object seatunnelField;
SeaTunnelDataType<?> seaTunnelDataType = seaTunnelDataTypes[i - 1];
Copy link
Member

Choose a reason for hiding this comment

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

use seaTunnelDataType.getSqlType() is better.

case ORACLE_NUMBER:
if (precision < 38) {
return new DecimalType(precision, scale);
}
Copy link
Member

Choose a reason for hiding this comment

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

can you add a comment about why delete this?

@liugddx
Copy link
Member Author

liugddx commented Oct 14, 2022

@EricJoy2048 @Hisoka-X @hailin0 please code review,thanks

@liugddx
Copy link
Member Author

liugddx commented Oct 15, 2022

rerun CI and help to review thanks. @CalvinKirs

…e/seatunnel/connectors/seatunnel/jdbc/internal/dialect/gbase8a/Gbase8aJdbcRowConverter.java

Co-authored-by: Eric <gaojun2048@gmail.com>
EricJoy2048
EricJoy2048 previously approved these changes Oct 18, 2022
Copy link
Member

@EricJoy2048 EricJoy2048 left a comment

Choose a reason for hiding this comment

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

LGTM @CalvinKirs @ic4y PTAL

import java.sql.Timestamp;
import java.util.Objects;

public class SqlDateType<T> implements SeaTunnelDataType<T> {
Copy link
Member

Choose a reason for hiding this comment

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

@ashulin PTAL about add new SqlDateType.

Copy link
Member

@ashulin ashulin Oct 18, 2022

Choose a reason for hiding this comment

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

IMO, this is not necessary.
The same effect can be achieved using LocalTimeType.
Multiple time types can confuse developers/users

@Hisoka-X
Copy link
Member

Why we need add SqlDateType? It's same with LocalTimeType.

@liugddx
Copy link
Member Author

liugddx commented Oct 18, 2022

Why we need add SqlDateType? It's same with LocalTimeType.

gbase8a must use sql date java.sql.Date,java.sql.Time,java.sql.Timestamp,otherwise it will be abnormal.

Exception in thread "main" com.gbase.jdbc.GBaseDataTruncation: Data truncation: Incorrect date value: '******'
	at com.gbase.jdbc.GBaseIO.checkErrorPacket(GBaseIO.java:3631)
	at com.gbase.jdbc.GBaseIO.checkErrorPacket(GBaseIO.java:3565)
	at com.gbase.jdbc.GBaseIO.sendCommand(GBaseIO.java:1996)
	at com.gbase.jdbc.GBaseIO.sqlQueryDirect(GBaseIO.java:2150)
	at com.gbase.jdbc.ConnectionImpl.execSQL(ConnectionImpl.java:2652)
	at com.gbase.jdbc.PreparedStatement.executeInternal(PreparedStatement.java:2133)
	at com.gbase.jdbc.PreparedStatement.executeUpdate(PreparedStatement.java:2429)
	at com.gbase.jdbc.PreparedStatement.executeUpdate(PreparedStatement.java:2347)
	at com.gbase.jdbc.PreparedStatement.executeUpdate(PreparedStatement.java:2332)

@ashulin @Hisoka-X Gbase does not support localTimeType type and i don't know what better way to do it.

@liugddx
Copy link
Member Author

liugddx commented Oct 18, 2022

Gbase8a test is ok,however, the sink does not support localTimeType type.Now I'm using the Assert test results . @EricJoy2048 @Hisoka-X @ashulin

@Hisoka-X
Copy link
Member

The CI not passed, please check

@liugddx
Copy link
Member Author

liugddx commented Oct 19, 2022

The CI not passed, please check

Done.

Copy link
Member

@Hisoka-X Hisoka-X left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@EricJoy2048 EricJoy2048 left a comment

Choose a reason for hiding this comment

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

+1

@EricJoy2048 EricJoy2048 merged commit dc6e85d into apache:dev Oct 19, 2022
@liugddx liugddx deleted the jdbc-gbase8a branch October 20, 2022 05:27
Carl-Zhou-CN pushed a commit to Carl-Zhou-CN/incubator-seatunnel that referenced this pull request Oct 27, 2022
* gbase 8a connector

* add gbase8a e2e test
Carl-Zhou-CN pushed a commit to Carl-Zhou-CN/incubator-seatunnel that referenced this pull request Oct 31, 2022
* gbase 8a connector

* add gbase8a e2e test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature][Connector-V2] gbase 8a connector
6 participants