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: support cross-db query in frontend and meta #20261

Merged
merged 23 commits into from
Feb 17, 2025
Merged

Conversation

yezizp2012
Copy link
Member

@yezizp2012 yezizp2012 commented Jan 22, 2025

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

What's changed and what's your intention?

Related #19631 .

  1. enable support for cross-database binding in batch operations, create_mv, and create_sink.
  2. check db CONNECT privilege for cross-db query.
  3. Integrated cross-db snapshot backfill executor, and address issues in metadata validation.
  4. reject drop mv/sink cascade with cross-db upstreams.
  5. verify the existence of log stores for all cross-db upstreams in the DDL.

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

Support cross-db query for both batch and streaming. The current requirements include:

  1. There is at least one subscription created on the upstream table or materialized view.

  2. The user is super or has connect permissions for the upstream databases and must also have the corresponding relation's query permissions.

@wenym1 wenym1 force-pushed the yiming/cross-db-backfill-executor branch from 008d2e7 to 745894e Compare February 11, 2025 07:56
Base automatically changed from yiming/cross-db-backfill-executor to main February 12, 2025 03:43
@yezizp2012 yezizp2012 marked this pull request as ready for review February 12, 2025 03:44
Copy link
Contributor

@wenym1 wenym1 left a comment

Choose a reason for hiding this comment

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

Rest LGTM!

We may need more tests for recovery, scale and background ddl. Not necessary to include in this PR.

@yezizp2012 yezizp2012 added the user-facing-changes Contains changes that are visible to users label Feb 12, 2025
Copy link
Contributor

Hi, there.

📝 Telemetry Reminder:
If you're implementing this feature, please consider adding telemetry metrics to track its usage. This helps us understand how the feature is being used and improve it further.
You can find the function report_event of telemetry reporting in the following files. Feel free to ask questions if you need any guidance!

  • src/frontend/src/telemetry.rs
  • src/meta/src/telemetry.rs
  • src/stream/src/telemetry.rs
  • src/storage/compactor/src/telemetry.rs
    Or calling report_event_common (src/common/telemetry_event/src/lib.rs) as if finding it hard to implement.
    ✨ Thank you for your contribution to RisingWave! ✨

This is an automated comment created by the peaceiris/actions-label-commenter. Responding to the bot or mentioning it won't have any effect.

Copy link
Contributor

@wenym1 wenym1 left a comment

Choose a reason for hiding this comment

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

LGTM!

Shall we add a release note to announce this new feature, and mention that currently user need to create a subscription on the table and should not drop the subscription?

@yezizp2012 yezizp2012 added this pull request to the merge queue Feb 17, 2025
Merged via the queue into main with commit 51c4809 Feb 17, 2025
31 of 32 checks passed
@yezizp2012 yezizp2012 deleted the zp/cross-db-frontend branch February 17, 2025 05:51
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.

2 participants