-
Notifications
You must be signed in to change notification settings - Fork 598
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: drop connector for table with connector #20023
base: main
Are you sure you want to change the base?
Conversation
replacement for #19871 |
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 you also test the behavior on tables with external-resolved schema (i.e., schema registry)? I believe #20131 will be a pre-requisite.
{"ID": 8}^{"ID": 8, "firstName": "Ava", "lastName": "Garcia", "age": 21, "height": 5.2, "weight": 115} | ||
EOF | ||
|
||
# the streaming job does not intake new data |
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 sleep a while before this check
// additional column name may have prefix _rw | ||
// When converting dropping the connector from table, the additional columns are converted to normal columns and keep the original name. | ||
// Under this case, we loosen the check for _rw prefix. | ||
if column_name.starts_with(RW_RESERVED_COLUMN_NAME_PREFIX) && !is_for_replace_plan { |
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 there is no need to enter this function (check_valid_column_name()
) at all when is_for_replace_plan == true
.
For exaample, is_row_id_column_name
may also need to be bypassed when you work on alter source alter connector
WITH ( | ||
${RISEDEV_KAFKA_WITH_OPTIONS_COMMON}, | ||
topic = 'test_alter_table_drop_connector') | ||
FORMAT PLAIN ENCODE JSON; |
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.
Is this shared source? Can you also add a test case for some non-shared source?
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Checklist
Documentation
Release note
new syntax for dropping connector part of a table
only allow for table with associate source, not works for plain table and CDC table.