-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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] FakeSource support MultipleTable #5766
[Feature] FakeSource support MultipleTable #5766
Conversation
efbcb92
to
fe14cfa
Compare
8edcb52
to
61d3371
Compare
61d3371
to
86788c0
Compare
86788c0
to
1d9e779
Compare
1d9e779
to
7de401b
Compare
7e9094a
to
7075e19
Compare
parseFromConfig(fakeSourceRootConfig); | ||
} | ||
// validate | ||
assert fakeConfigs != null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the assert
will not work by default. Please use com.google.common.base.Preconditions.checkNotNull
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -209,9 +209,9 @@ public SeaTunnelDataType<?> toSeaTunnelType( | |||
case DATE: | |||
return LocalTimeType.LOCAL_DATE_TYPE; | |||
case DATETIME: | |||
return LocalTimeType.LOCAL_DATE_TIME_TYPE; | |||
case TIMESTAMP: | |||
return LocalTimeType.LOCAL_TIME_TYPE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mapping DATETIME to LOCAL_DATE_TIME_TYPE ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The DATETIME should convert to LocalTime and TIMESTAMP should convert to LOCAL_DATE_TIME_TYPE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I revert this change.
8fabd05
to
4afca2c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4afca2c
to
a1dd591
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Purpose of this pull request
FakeSource support generate multiple different CatalogTable. This will be used for testing multiple table feature for sink.
Does this PR introduce any user-facing change?
How was this patch tested?
Tested by UT and e2e case
Check list
New License Guide
release-note
.