You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "aiwenmo (via GitHub)" <gi...@apache.org> on 2024/11/18 15:12:39 UTC
[PR] [FLINK-36741][cdc][transform] Fix the precision of decimal has been lost when use transform [flink-cdc]
aiwenmo opened a new pull request, #3740:
URL: https://github.com/apache/flink-cdc/pull/3740
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36741][cdc][transform] Fix the precision of decimal has been lost when use transform [flink-cdc]
Posted by "yuxiqian (via GitHub)" <gi...@apache.org>.
yuxiqian commented on PR #3740:
URL: https://github.com/apache/flink-cdc/pull/3740#issuecomment-2564977198
Could @aiwenmo also summarise what issue does this PR fixes, by changing PR title and description? Obviously this fixes more than just DecimalType precision issue, and most part of description is irrelevant.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36741][cdc][transform] Fix the precision of decimal has been lost when use transform [flink-cdc]
Posted by "aiwenmo (via GitHub)" <gi...@apache.org>.
aiwenmo commented on PR #3740:
URL: https://github.com/apache/flink-cdc/pull/3740#issuecomment-2483443793
1. Add convertCalciteRelDataType in DataTypeConverter.
2. Modify testGenerateProjectionColumns in TransformParserTest to verify the repair.
3. This PR fixed all data type which has precision or length.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36741][cdc][transform] Fix the precision of decimal has been lost when use transform [flink-cdc]
Posted by "aiwenmo (via GitHub)" <gi...@apache.org>.
aiwenmo commented on code in PR #3740:
URL: https://github.com/apache/flink-cdc/pull/3740#discussion_r1855858665
##########
flink-cdc-runtime/src/test/java/org/apache/flink/cdc/runtime/parser/TransformParserTest.java:
##########
@@ -316,12 +316,13 @@ public void testGenerateProjectionColumns() {
Column.physicalColumn("name", DataTypes.STRING(), "string"),
Column.physicalColumn("age", DataTypes.INT(), "age"),
Column.physicalColumn("address", DataTypes.STRING(), "address"),
+ Column.physicalColumn("deposit", DataTypes.DECIMAL(10, 2), "deposit"),
Column.physicalColumn("weight", DataTypes.DOUBLE(), "weight"),
Column.physicalColumn("height", DataTypes.DOUBLE(), "height"));
List<ProjectionColumn> result =
TransformParser.generateProjectionColumns(
- "id, upper(name) as name, age + 1 as newage, weight / (height * height) as bmi",
+ "id, upper(name) as name, age + 1 as newage, deposit as deposits, weight / (height * height) as bmi",
Review Comment:
I'll handle it tonight.
##########
flink-cdc-runtime/src/main/java/org/apache/flink/cdc/runtime/parser/TransformParser.java:
##########
@@ -123,10 +123,7 @@ private static RelNode sqlToRel(
if (udf.getReturnTypeHint() != null) {
// This UDF has return type hint annotation
returnTypeInference =
- o ->
- o.getTypeFactory()
- .createSqlType(
- convertCalciteType(udf.getReturnTypeHint()));
+ o -> convertCalciteType(o.getTypeFactory(), udf.getReturnTypeHint());
Review Comment:
I'll handle it tonight.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36741][cdc][transform] Fix the precision of decimal has been lost when use transform [flink-cdc]
Posted by "yuxiqian (via GitHub)" <gi...@apache.org>.
yuxiqian commented on PR #3740:
URL: https://github.com/apache/flink-cdc/pull/3740#issuecomment-2483423451
I remember that similar things happen to VARCHAR (and other types with variable precisions). Any chance this PR has fixed these cases, too?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36741][cdc][transform] Fix the precision of decimal has been lost when use transform and support DecimalData [flink-cdc]
Posted by "aiwenmo (via GitHub)" <gi...@apache.org>.
aiwenmo commented on PR #3740:
URL: https://github.com/apache/flink-cdc/pull/3740#issuecomment-2564984725
1.Add convertCalciteRelDataType in DataTypeConverter.
2.Add the convert of precision in DataTypeConverter.convertCalciteType.
3.Use DecimalData instead of BigDecimal in YAML task.
4.Optimize the strategy of SqlToRelConverter.
5.Fix the precision and length of data type has been lost in TransformTable.getRowType.
6.Modify testGenerateProjectionColumns in TransformParserTest to verify the repair.
7.Add testComplicatedUdfReturnTypes to verify the repair when using udf.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36741][cdc][transform] Fix the precision of decimal has been lost when use transform [flink-cdc]
Posted by "aiwenmo (via GitHub)" <gi...@apache.org>.
aiwenmo commented on PR #3740:
URL: https://github.com/apache/flink-cdc/pull/3740#issuecomment-2483456151
@yuxiqian @leonardBang @lvyanquan PTAL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36741][cdc][transform] Fix the precision of and length of data type has been lost when use transform and support DecimalData [flink-cdc]
Posted by "leonardBang (via GitHub)" <gi...@apache.org>.
leonardBang merged PR #3740:
URL: https://github.com/apache/flink-cdc/pull/3740
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36741][cdc][transform] Fix the precision of decimal has been lost when use transform [flink-cdc]
Posted by "yuxiqian (via GitHub)" <gi...@apache.org>.
yuxiqian commented on code in PR #3740:
URL: https://github.com/apache/flink-cdc/pull/3740#discussion_r1855853213
##########
flink-cdc-runtime/src/main/java/org/apache/flink/cdc/runtime/parser/TransformParser.java:
##########
@@ -588,7 +585,8 @@ public static String normalizeFilter(String projection, String filter) {
public static boolean hasAsterisk(@Nullable String projection) {
if (isNullOrWhitespaceOnly(projection)) {
- return false;
+ // Providing an empty projection expression is equivalent to writing `*` explicitly.
+ return true;
Review Comment:
Please rebase with `master` as this has been resolved
##########
flink-cdc-runtime/src/main/java/org/apache/flink/cdc/runtime/parser/TransformParser.java:
##########
@@ -123,10 +123,7 @@ private static RelNode sqlToRel(
if (udf.getReturnTypeHint() != null) {
// This UDF has return type hint annotation
returnTypeInference =
- o ->
- o.getTypeFactory()
- .createSqlType(
- convertCalciteType(udf.getReturnTypeHint()));
+ o -> convertCalciteType(o.getTypeFactory(), udf.getReturnTypeHint());
Review Comment:
Could you please add some test cases on UDF return type inferences?
##########
flink-cdc-runtime/src/test/java/org/apache/flink/cdc/runtime/parser/TransformParserTest.java:
##########
@@ -316,12 +316,13 @@ public void testGenerateProjectionColumns() {
Column.physicalColumn("name", DataTypes.STRING(), "string"),
Column.physicalColumn("age", DataTypes.INT(), "age"),
Column.physicalColumn("address", DataTypes.STRING(), "address"),
+ Column.physicalColumn("deposit", DataTypes.DECIMAL(10, 2), "deposit"),
Column.physicalColumn("weight", DataTypes.DOUBLE(), "weight"),
Column.physicalColumn("height", DataTypes.DOUBLE(), "height"));
List<ProjectionColumn> result =
TransformParser.generateProjectionColumns(
- "id, upper(name) as name, age + 1 as newage, weight / (height * height) as bmi",
+ "id, upper(name) as name, age + 1 as newage, deposit as deposits, weight / (height * height) as bmi",
Review Comment:
Please add some tests to verify if `VARCHAR`, `TIMESTAMP`-series types with precision works, too
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36741][cdc][transform] Fix the precision of and length of data type has been lost when use transform and support DecimalData [flink-cdc]
Posted by "aiwenmo (via GitHub)" <gi...@apache.org>.
aiwenmo commented on PR #3740:
URL: https://github.com/apache/flink-cdc/pull/3740#issuecomment-2577571598
PTAL @yuxiqian
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36741][cdc][transform] Fix the precision of decimal has been lost when use transform [flink-cdc]
Posted by "aiwenmo (via GitHub)" <gi...@apache.org>.
aiwenmo commented on code in PR #3740:
URL: https://github.com/apache/flink-cdc/pull/3740#discussion_r1855859241
##########
flink-cdc-runtime/src/main/java/org/apache/flink/cdc/runtime/parser/TransformParser.java:
##########
@@ -588,7 +585,8 @@ public static String normalizeFilter(String projection, String filter) {
public static boolean hasAsterisk(@Nullable String projection) {
if (isNullOrWhitespaceOnly(projection)) {
- return false;
+ // Providing an empty projection expression is equivalent to writing `*` explicitly.
+ return true;
Review Comment:
I'll rebase it tonight.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36741][cdc][transform] Fix the precision of decimal has been lost when use transform [flink-cdc]
Posted by "aiwenmo (via GitHub)" <gi...@apache.org>.
aiwenmo commented on PR #3740:
URL: https://github.com/apache/flink-cdc/pull/3740#issuecomment-2550079055
Resolved conflicts. CC @yuxiqian
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org