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/12/30 17:35:02 UTC
[PR] [FLINK-36981][cdc] Fix the bug of transform merging with route in YAML task [flink-cdc]
aiwenmo opened a new pull request, #3826:
URL: https://github.com/apache/flink-cdc/pull/3826
(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-36981][cdc] Fix the bug of transform merging with route in YAML task [flink-cdc]
Posted by "aiwenmo (via GitHub)" <gi...@apache.org>.
aiwenmo commented on PR #3826:
URL: https://github.com/apache/flink-cdc/pull/3826#issuecomment-2566055712
PTAL @yuxiqian @lvyanquan
--
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-36981][cdc] Fix the bug of transform merging with route in YAML task [flink-cdc]
Posted by "leonardBang (via GitHub)" <gi...@apache.org>.
leonardBang merged PR #3826:
URL: https://github.com/apache/flink-cdc/pull/3826
--
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-36981][cdc] Fix the bug of transform merging with route in YAML task [flink-cdc]
Posted by "yuxiqian (via GitHub)" <gi...@apache.org>.
yuxiqian commented on code in PR #3826:
URL: https://github.com/apache/flink-cdc/pull/3826#discussion_r1906873445
##########
flink-cdc-runtime/src/main/java/org/apache/flink/cdc/runtime/operators/transform/PostTransformOperator.java:
##########
@@ -434,23 +436,36 @@ private Optional<DataChangeEvent> processDataChangeEvent(DataChangeEvent dataCha
&& transformProjectionOptional.get().isValid()) {
TransformProjection transformProjection = transformProjectionOptional.get();
if (!transformProjectionProcessorMap.containsKey(
- Tuple2.of(tableId, transformProjection))
- || !transformProjectionProcessorMap
- .get(Tuple2.of(tableId, transformProjection))
- .hasTableInfo()) {
+ Tuple2.of(tableId, transformProjection.getProjection()))) {
transformProjectionProcessorMap.put(
- Tuple2.of(tableId, transformProjection),
+ Tuple2.of(tableId, transformProjection.getProjection()),
TransformProjectionProcessor.of(
tableInfo,
transformProjection,
timezone,
udfDescriptors,
getUdfFunctionInstances(),
transform.getSupportedMetadataColumns()));
+ } else if (!transformProjectionProcessorMap
+ .get(Tuple2.of(tableId, transformProjection.getProjection()))
+ .hasTableInfo()) {
+ TransformProjectionProcessor transformProjectionProcessorWithoutTableInfo =
+ transformProjectionProcessorMap.get(
+ Tuple2.of(tableId, transformProjection.getProjection()));
+ transformProjectionProcessorMap.put(
+ Tuple2.of(tableId, transformProjection.getProjection()),
+ TransformProjectionProcessor.of(
Review Comment:
Dismissed for now, we can clean up transform module later.
--
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-36981][cdc] Fix the bug of transform merging with route in YAML task [flink-cdc]
Posted by "ChaomingZhangCN (via GitHub)" <gi...@apache.org>.
ChaomingZhangCN commented on code in PR #3826:
URL: https://github.com/apache/flink-cdc/pull/3826#discussion_r1904868819
##########
flink-cdc-runtime/src/main/java/org/apache/flink/cdc/runtime/operators/transform/PostTransformOperator.java:
##########
@@ -434,23 +436,36 @@ private Optional<DataChangeEvent> processDataChangeEvent(DataChangeEvent dataCha
&& transformProjectionOptional.get().isValid()) {
TransformProjection transformProjection = transformProjectionOptional.get();
if (!transformProjectionProcessorMap.containsKey(
- Tuple2.of(tableId, transformProjection))
- || !transformProjectionProcessorMap
- .get(Tuple2.of(tableId, transformProjection))
- .hasTableInfo()) {
+ Tuple2.of(tableId, transformProjection.getProjection()))) {
transformProjectionProcessorMap.put(
- Tuple2.of(tableId, transformProjection),
+ Tuple2.of(tableId, transformProjection.getProjection()),
TransformProjectionProcessor.of(
tableInfo,
transformProjection,
timezone,
udfDescriptors,
getUdfFunctionInstances(),
transform.getSupportedMetadataColumns()));
+ } else if (!transformProjectionProcessorMap
+ .get(Tuple2.of(tableId, transformProjection.getProjection()))
+ .hasTableInfo()) {
+ TransformProjectionProcessor transformProjectionProcessorWithoutTableInfo =
+ transformProjectionProcessorMap.get(
+ Tuple2.of(tableId, transformProjection.getProjection()));
+ transformProjectionProcessorMap.put(
+ Tuple2.of(tableId, transformProjection.getProjection()),
+ TransformProjectionProcessor.of(
Review Comment:
+1 Use TransformProjectionProcessor#copy() to create a non-shared object seems better.
--
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-36981][cdc] Fix the bug of transform merging with route in YAML task [flink-cdc]
Posted by "yuxiqian (via GitHub)" <gi...@apache.org>.
yuxiqian commented on PR #3826:
URL: https://github.com/apache/flink-cdc/pull/3826#issuecomment-2567525957
`1.Change Map<Tuple2<TableId, TransformProjection>, TransformProjectionProcessor> transformProjectionProcessorMap into Map<Tuple2<TableId, String>, TransformProjectionProcessor> transformProjectionProcessorMap.`
Seems you're changing the key of this map from "[TableId, TransformProjection]` to `[Transform, TransformProjection#projection]`. However the problem is Projection string is a field of TransformProjection. If `TransformProjection` has collision, how can the `ProjectionString` be OK?
--
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-36981][cdc] Fix the bug of transform merging with route in YAML task [flink-cdc]
Posted by "yuxiqian (via GitHub)" <gi...@apache.org>.
yuxiqian commented on code in PR #3826:
URL: https://github.com/apache/flink-cdc/pull/3826#discussion_r1900727807
##########
flink-cdc-runtime/src/main/java/org/apache/flink/cdc/runtime/operators/transform/PostTransformOperator.java:
##########
@@ -434,23 +436,36 @@ private Optional<DataChangeEvent> processDataChangeEvent(DataChangeEvent dataCha
&& transformProjectionOptional.get().isValid()) {
TransformProjection transformProjection = transformProjectionOptional.get();
if (!transformProjectionProcessorMap.containsKey(
- Tuple2.of(tableId, transformProjection))
- || !transformProjectionProcessorMap
- .get(Tuple2.of(tableId, transformProjection))
- .hasTableInfo()) {
+ Tuple2.of(tableId, transformProjection.getProjection()))) {
transformProjectionProcessorMap.put(
- Tuple2.of(tableId, transformProjection),
+ Tuple2.of(tableId, transformProjection.getProjection()),
TransformProjectionProcessor.of(
tableInfo,
transformProjection,
timezone,
udfDescriptors,
getUdfFunctionInstances(),
transform.getSupportedMetadataColumns()));
+ } else if (!transformProjectionProcessorMap
+ .get(Tuple2.of(tableId, transformProjection.getProjection()))
+ .hasTableInfo()) {
Review Comment:
`hasTableInfo` could be renamed to something like `hasChangeInfo` or `hasPostTransformChangeInfo`
##########
flink-cdc-runtime/src/main/java/org/apache/flink/cdc/runtime/operators/transform/PostTransformOperator.java:
##########
@@ -434,23 +436,36 @@ private Optional<DataChangeEvent> processDataChangeEvent(DataChangeEvent dataCha
&& transformProjectionOptional.get().isValid()) {
TransformProjection transformProjection = transformProjectionOptional.get();
if (!transformProjectionProcessorMap.containsKey(
- Tuple2.of(tableId, transformProjection))
- || !transformProjectionProcessorMap
- .get(Tuple2.of(tableId, transformProjection))
- .hasTableInfo()) {
+ Tuple2.of(tableId, transformProjection.getProjection()))) {
transformProjectionProcessorMap.put(
- Tuple2.of(tableId, transformProjection),
+ Tuple2.of(tableId, transformProjection.getProjection()),
TransformProjectionProcessor.of(
tableInfo,
transformProjection,
timezone,
udfDescriptors,
getUdfFunctionInstances(),
transform.getSupportedMetadataColumns()));
+ } else if (!transformProjectionProcessorMap
+ .get(Tuple2.of(tableId, transformProjection.getProjection()))
+ .hasTableInfo()) {
+ TransformProjectionProcessor transformProjectionProcessorWithoutTableInfo =
+ transformProjectionProcessorMap.get(
+ Tuple2.of(tableId, transformProjection.getProjection()));
+ transformProjectionProcessorMap.put(
+ Tuple2.of(tableId, transformProjection.getProjection()),
+ TransformProjectionProcessor.of(
Review Comment:
We may have a `copy` method in `TransformProjectionProcessor` to allow us overwriting specific field but keep others intact.
Otherwise, a lot of codes must be modified if we add one more field into it.
--
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-36981][cdc] Fix the bug of transform merging with route in YAML task [flink-cdc]
Posted by "ChaomingZhangCN (via GitHub)" <gi...@apache.org>.
ChaomingZhangCN commented on code in PR #3826:
URL: https://github.com/apache/flink-cdc/pull/3826#discussion_r1904868819
##########
flink-cdc-runtime/src/main/java/org/apache/flink/cdc/runtime/operators/transform/PostTransformOperator.java:
##########
@@ -434,23 +436,36 @@ private Optional<DataChangeEvent> processDataChangeEvent(DataChangeEvent dataCha
&& transformProjectionOptional.get().isValid()) {
TransformProjection transformProjection = transformProjectionOptional.get();
if (!transformProjectionProcessorMap.containsKey(
- Tuple2.of(tableId, transformProjection))
- || !transformProjectionProcessorMap
- .get(Tuple2.of(tableId, transformProjection))
- .hasTableInfo()) {
+ Tuple2.of(tableId, transformProjection.getProjection()))) {
transformProjectionProcessorMap.put(
- Tuple2.of(tableId, transformProjection),
+ Tuple2.of(tableId, transformProjection.getProjection()),
TransformProjectionProcessor.of(
tableInfo,
transformProjection,
timezone,
udfDescriptors,
getUdfFunctionInstances(),
transform.getSupportedMetadataColumns()));
+ } else if (!transformProjectionProcessorMap
+ .get(Tuple2.of(tableId, transformProjection.getProjection()))
+ .hasTableInfo()) {
+ TransformProjectionProcessor transformProjectionProcessorWithoutTableInfo =
+ transformProjectionProcessorMap.get(
+ Tuple2.of(tableId, transformProjection.getProjection()));
+ transformProjectionProcessorMap.put(
+ Tuple2.of(tableId, transformProjection.getProjection()),
+ TransformProjectionProcessor.of(
Review Comment:
+1 Use TransformProjectionProcessor#copy() to create a non-shared object seems better.
--
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