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

feat: tvf with cdc source #20439

Merged
merged 8 commits into from
Feb 11, 2025
Merged

feat: tvf with cdc source #20439

merged 8 commits into from
Feb 11, 2025

Conversation

tabVersion
Copy link
Contributor

@tabVersion tabVersion commented Feb 10, 2025

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

allow postgres_query and mysql_query by reusing the params of cdc-source

part of #18583

Checklist

  • I have written necessary rustdoc comments.
  • I have added necessary unit tests and integration tests.
  • I have added test labels as necessary.
  • I have added fuzzing tests or opened an issue to track them.
  • My PR contains breaking changes.
  • My PR changes performance-critical code, so I will run (micro) benchmarks and present the results.
  • My PR contains critical fixes that are necessary to be merged into the latest release.

Documentation

  • My PR needs documentation updates.
Release note
create source some_cdc_source with (
connector='postgres-cdc' / 'mysql-cdc',
hostname = 'aaa',
port = 1122,
username = 'bbb',
password = 'ccc',
database.name = 'dddd',
...
);

select * from postgres_query('some_cdc_source', 'select * from upstream table')

Note that we only take the (hostname, port, username, password, database_name) from the source, ignoring the slot.name and server.id

@tabVersion tabVersion marked this pull request as ready for review February 10, 2025 09:20
@tabVersion tabVersion requested a review from Gogomoe February 10, 2025 09:23
@@ -117,6 +117,11 @@ pub fn generate_risedev_env(services: &Vec<ServiceConfig>) -> String {
writeln!(env, r#"PGUSER="{user}""#,).unwrap();
writeln!(env, r#"PGPASSWORD="{password}""#,).unwrap();
writeln!(env, r#"PGDATABASE="{database}""#,).unwrap();
writeln!(
env,
r#"RISEDEV_POSTGRES_WITH_OPTIONS_COMMON="connector='postgres-cdc',hostname='{host}',port='{port}'""#,
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think test specific env vars should be set in risedev_env. How about setting them inside the e2e-source-test script?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be acceptable as we already have it for Mysql tests

writeln!(env, r#"RISEDEV_MYSQL_WITH_OPTIONS_COMMON="connector='mysql-cdc',hostname='{host}',port='{port}'""#,).unwrap();

Copy link
Contributor

@kwannoel kwannoel left a comment

Choose a reason for hiding this comment

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

I like this approach, just need to improve the readability. Thanks for the work!

Copy link
Contributor

@Gogomoe Gogomoe left a comment

Choose a reason for hiding this comment

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

LGTM~!

@tabVersion tabVersion enabled auto-merge February 10, 2025 12:51
@tabVersion tabVersion added this pull request to the merge queue Feb 11, 2025
Merged via the queue into main with commit deb98b5 Feb 11, 2025
28 of 29 checks passed
@tabVersion tabVersion deleted the tab/tvf-pg branch February 11, 2025 05:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants