-
Notifications
You must be signed in to change notification settings - Fork 701
feat(sink): support redis sink stream #23412
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR adds support for Redis Stream data type to the Redis sink connector in RisingWave, expanding beyond the existing support for strings, geospatial data, and pub/sub channels.
- Added
REDIS_VALUE_TYPE_STREAMsupport for Redis Streams - Implemented stream configuration with
streamandstream_columnparameters - Added template encoders for Redis Stream key-value pairs
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/connector/src/sink/redis.rs | Added stream constants and pipeline operations for Redis streams |
| src/connector/src/sink/formatter/mod.rs | Added stream value type handling in template encoder build logic |
| src/connector/src/sink/encoder/template.rs | Implemented stream encoders and renamed PubSub encoders for consistency |
| src/connector/Cargo.toml | Added "streams" feature to redis dependency |
Comments suppressed due to low confidence (1)
src/connector/src/sink/formatter/mod.rs:1
- Missing space after comma in error messages. Should be 'Cannot find '{VALUE_FORMAT}', please set it.' and 'Cannot find '{KEY_FORMAT}', please set it.'
// Copyright 2025 RisingWave Labs
Co-authored-by: Copilot <[email protected]>
src/connector/src/sink/redis.rs
Outdated
| || (stream.is_some() && stream_column.is_some()) | ||
| { | ||
| return Err(SinkError::Config(anyhow!( | ||
| "`{STREAM}` and `{STREAM_COLUMN}` only one can be set" |
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.
nits:
Please specific either `{STREAM}` or `{STREAM_COLUMN}`. They are mutually exclusive options.
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 expected? Should we add .vscode in gitignore?
d7b4ad4 to
0d91bea
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
|
Please update the PR description. Also, please mention that this is a license feature in the PR description and the release note. |
|
also request review from redis expert @shanicky :doge |
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.
No functionality tests on CI?
@xxhZs can we add redis stream e2e test? It seems that this feature is available in open source edition. |
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
We can use pubsub like
or
value_formatRequired: Format of the value passed to streamkey_formatRequired: Format of the key passed to streamstreamstream_columnboth must set at least one.streamThe name of the message stream to send tostream_columnUse the values in this column as the name of the message stream to send to (type must be varchar)