-
Notifications
You must be signed in to change notification settings - Fork 590
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
feat(cdc): map upstream table schema automatically for cdc table #16986
Conversation
…bs/risingwave into siyuan/cdc-metadata-columns
36087b2
to
3117fac
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.
Generally LGTM
It is recommended to split "code movement and refactoring" and "functional logic changes" into two separate PRs next time
...e-connector-service/src/main/java/com/risingwave/connector/source/common/MySqlValidator.java
Show resolved
Hide resolved
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.
Btw, Can you show an error message indicating that the database cannot be connected? I guess it would be very helpful for users to know that it is the frontend which need to be connected to the source database.
Here is an example
|
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.
Can we adopt the new inline source style introduced in #16449?
hostname = '${MYSQL_HOST:localhost}', | ||
port = '${MYSQL_TCP_PORT:8306}', | ||
username = 'root', | ||
password = '${MYSQL_PWD:}', |
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.
We can use ${RISEDEV_MYSQL_WITH_OPTIONS_COMMON}
.
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.
Better to organize it under the new folder e2e_test/source_inline
.
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.
Will refactor in another pr.
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
b32370c
to
38ff10e
Compare
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Automatically map the upstream table schema when creating a CDC table from
mysql-cdc
orpostgres-cdc
source.We use the sea-schema crate to read the upstream schema.
ExternalTableImpl
as an interface to read external table schemamysql.rs
.*
cannot be used with column definitions, for examplecreate table t (v1 int, *)
is prohibited.Example:
close #16443
Checklist
./risedev check
(or alias,./risedev c
)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.