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][Flink] Support Decimal Type with configurable precision and scale #5419

Merged
merged 1 commit into from
Sep 25, 2023

Conversation

CheneyYin
Copy link
Contributor

@CheneyYin CheneyYin commented Sep 2, 2023

Purpose of this pull request

Motivation

Flink has two type systems: TypeInformation of DataSet/DataStream API, and LogicType of Table & SQL API. TypeInformation has some shortcomings. For example, precision and scale can not be defined for DECIMALs.
SeaTunnel has implemented a translation layer based on DataStream, which used TypeInformation type system.

Solution

To support configurable decimal type, there are two solutions.

1st Solution

The 1st solution requires rewriting the translation layer based on Table & SQL API. But it will consume a lot of work and cause many code changes.

2nd Solution[Adopt]

The 2nd solution adopts the idea of type mapping and uses the String type as the carrier medium of Decimal. SeaTunnel restores the Decimal type field from String before processing the data, and converts the Decimal type field into String after completing the data processing. Because it cannot be inferred from the String type field of typeinformation whether it is a Decimal type field, this solution temporarily stores SeaTunnelRowType to bypass the type conversion between TypeInformation and SeaTunnelDataType.
Because this solution does not require too much work and can guarantee the stability of the source code, I will adopt it.

Check list

@CheneyYin CheneyYin changed the title [Feature][Flink] Support Decimal Type with configurable precison and scale [Feature][Flink] Support Decimal Type with configurable precision and scale Sep 2, 2023
@CheneyYin CheneyYin marked this pull request as ready for review September 2, 2023 08:02
@CheneyYin
Copy link
Contributor Author

SeaTunnel_Engine_Server_npe

@CheneyYin
Copy link
Contributor Author

SeaTunnel_E2E_Engine_Base

@CheneyYin
Copy link
Contributor Author

Pulsar

@CheneyYin
Copy link
Contributor Author

There are 3 failed jobs about zeta, please re-run failed jobs. @liugddx

@liugddx
Copy link
Member

liugddx commented Sep 4, 2023

There are 3 failed jobs about zeta, please re-run failed jobs. @liugddx

Done.

@liunaijie liunaijie mentioned this pull request Sep 4, 2023
5 tasks
@CheneyYin
Copy link
Contributor Author

@liugddx @TyrantLucifer PTAL

liugddx
liugddx previously approved these changes Sep 5, 2023
Copy link
Member

@liugddx liugddx left a comment

Choose a reason for hiding this comment

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

LGTM

@liugddx
Copy link
Member

liugddx commented Sep 5, 2023

@hailin0 @Hisoka-X PTAL

@Hisoka-X Hisoka-X added this to the 2.3.4 milestone Sep 5, 2023
@EricJoy2048
Copy link
Member

@s7monk PTAL

Copy link
Member

@Carl-Zhou-CN Carl-Zhou-CN left a comment

Choose a reason for hiding this comment

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

+1

@Hisoka-X
Copy link
Member

Hisoka-X commented Sep 11, 2023

This change not small in my view cc @ic4y @hailin0

@EricJoy2048
Copy link
Member

Please fix conflicts.

@CheneyYin
Copy link
Contributor Author

Please fix conflicts.

I have fixed confilcts. PTAL @EricJoy2048

@CheneyYin
Copy link
Contributor Author

all-connectors-it-7 build failure, please re-run failed jobs. @EricJoy2048

Copy link
Member

@liugddx liugddx left a comment

Choose a reason for hiding this comment

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

LGTM

@EricJoy2048 EricJoy2048 merged commit 7348be2 into apache:dev Sep 25, 2023
53 checks passed
gnehil pushed a commit to gnehil/seatunnel that referenced this pull request Oct 12, 2023
@CheneyYin CheneyYin deleted the flink-support-decimal-type branch January 11, 2024 02:42
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.

[Bug] [CI][E2E] connector-paimon-e2e failed randomly
5 participants