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 connector support SAP HANA Source & SAP HANA Sink. #3017

Merged
merged 1 commit into from
Jan 2, 2023

Conversation

FlechazoW
Copy link
Contributor

@FlechazoW FlechazoW commented Oct 8, 2022

Purpose of this pull request

[Connector-V2][JDBC-connector] support Jdbc dm

Check list

@liugddx
Copy link
Member

liugddx commented Oct 8, 2022

Is it necessary to have init-datasource as a separate file?I think the paperwork is getting a little messy,How about writing it straight into code?

@FlechazoW
Copy link
Contributor Author

Is it necessary to have init-datasource as a separate file?I think the paperwork is getting a little messy,How about writing it straight into code?

Nice thought.

Hisoka-X
Hisoka-X previously approved these changes Oct 9, 2022
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, Please take a look @ic4y

@Hisoka-X
Copy link
Member

Hisoka-X commented Oct 9, 2022

The code style should be fixed.

@FlechazoW
Copy link
Contributor Author

The code style should be fixed.

Done @Hisoka-X

@Hisoka-X
Copy link
Member

@FlechazoW The Integration Test have some problems should be fixed.

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.

LGTM

TyrantLucifer
TyrantLucifer previously approved these changes Oct 12, 2022
Copy link
Member

@TyrantLucifer TyrantLucifer 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
Copy link
Member

trigger CI

@CalvinKirs CalvinKirs closed this Oct 12, 2022
@CalvinKirs CalvinKirs reopened this Oct 12, 2022
@EricJoy2048 EricJoy2048 changed the title [Feature][Connector-V2] Jdbc connector support SAP HANA. [Feature][Connector-V2] Jdbc connector support SAP HANA Source & SAP HANA Sink. Oct 14, 2022
Copy link
Contributor

@ic4y ic4y left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution, please add e2e tests

case SAP_HANA_REAL:
case SAP_HANA_SMALLDECIMAL:
return BasicType.FLOAT_TYPE;
case SAP_HANA_BIGINT:
Copy link
Contributor

Choose a reason for hiding this comment

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

SAP_HANA_BIGINT can be use BasicType.LONG_TYPE

Copy link
Member

Choose a reason for hiding this comment

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

It seems unchanged?

return BasicType.FLOAT_TYPE;
case SAP_HANA_BIGINT:
case SAP_HANA_INTEGER:
case SAP_HANA_SMALLINT:
Copy link
Contributor

Choose a reason for hiding this comment

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

SAP_HANA_SMALLINT -> BasicType.SHORT_TYPE

case SAP_HANA_BIGINT:
case SAP_HANA_INTEGER:
case SAP_HANA_SMALLINT:
case SAP_HANA_TINYINT:
Copy link
Contributor

Choose a reason for hiding this comment

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

SAP_HANA_TINYINT -> BasicType.BYTE_TYPE

Hisoka-X
Hisoka-X previously approved these changes Nov 14, 2022
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 @ic4y PTAL.

TyrantLucifer
TyrantLucifer previously approved these changes Nov 22, 2022
Copy link
Member

@TyrantLucifer TyrantLucifer left a comment

Choose a reason for hiding this comment

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

LGTM, cc @ic4y

@ic4y
Copy link
Contributor

ic4y commented Nov 25, 2022

Are SAP_HANA_BIGINT, SAP_HANA_INTEGER, SAP_HANA_SMALLINT all tested with SHORT_TYPE? doesn't look right

Copy link
Member

@TyrantLucifer TyrantLucifer left a comment

Choose a reason for hiding this comment

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

@liugddx liugddx merged commit fe0180f into apache:dev Jan 2, 2023
lhyundeadsoul pushed a commit to lhyundeadsoul/incubator-seatunnel that referenced this pull request Jan 3, 2023
lhyundeadsoul pushed a commit to lhyundeadsoul/incubator-seatunnel that referenced this pull request Jan 3, 2023
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.

8 participants