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][Transform] Transform supports multi-table scenarios #5646

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from

Conversation

ic4y
Copy link
Contributor

@ic4y ic4y commented Oct 17, 2023

Purpose of this pull request

1、supports multi-table for transform
2、supports multi-table for assert sink

Does this PR introduce any user-facing change?

Multi-table transfrom is supported. But the original logic is perfectly compatible

How was this patch tested?

add some e2e test

Check list

@ic4y ic4y marked this pull request as ready for review October 18, 2023 12:51
ic4y and others added 4 commits October 19, 2023 11:37
# Conflicts:
#	seatunnel-engine/seatunnel-engine-server/src/main/java/org/apache/seatunnel/engine/server/dag/execution/ExecutionPlanGenerator.java
@hailin0
Copy link
Member

hailin0 commented Oct 25, 2023

check ci error

Comment on lines +47 to +61
inputCatalogTables.forEach(
inputCatalogTable -> {
String tableId = inputCatalogTable.getTableId().toTablePath().toString();
transformMap.put(tableId, buildTransform(inputCatalogTable, config));
});

this.outputCatalogTables =
inputCatalogTables.stream()
.map(
inputCatalogTable -> {
String tableName =
inputCatalogTable.getTableId().toTablePath().toString();
return transformMap.get(tableName).getProducedCatalogTable();
})
.collect(Collectors.toList());
Copy link
Member

Choose a reason for hiding this comment

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

This processing method contains serious potential problems. If there are 10 tables in the upstream, but only one of the tables is need be transformed, the job will report an error because the transform of the remaining 9 tables cannot be constructed normally, there is no corresponding configuration, and it stands to reason that the transforms of the remaining 9 tables should not be constructed, because no modifications have been made at all. We should not foreach the upstream catalogtables, but foreach the user's transform configuration.

@hailin0 hailin0 added this to the 2.3.4 milestone Dec 17, 2023
@EricJoy2048 EricJoy2048 removed this from the 2.3.4 milestone Jan 11, 2024
@hailin0 hailin0 added transform-v2 feature New feature no update The owner doesn't provide further feedback. labels Feb 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature no update The owner doesn't provide further feedback. transform-v2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants