-
Notifications
You must be signed in to change notification settings - Fork 719
refactor(sink): replace ForceAppendOnly with a separate ignore_delete field
#24310
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
Conversation
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
|
This change is part of the following stack:
Change managed by git-spice. |
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 refactors the sink type system by replacing SinkType::ForceAppendOnly with a separate ignore_delete boolean field on SinkType::AppendOnly. This change makes the concept of ignoring delete operations orthogonal to the sink type, enabling future support for combinations like SinkType::Upsert with ignore_delete to allow upsert sinks that ignore delete operations.
- Introduces a new
ignore_deleteboolean field in the sink model and protobuf definitions - Deprecates
SINK_TYPE_FORCE_APPEND_ONLYin proto definitions while maintaining backward compatibility - Migrates existing
FORCE_APPEND_ONLYsinks toAPPEND_ONLYwithignore_delete = true
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| proto/catalog.proto | Marks SINK_TYPE_FORCE_APPEND_ONLY as deprecated and adds ignore_delete field to Sink message |
| proto/stream_plan.proto | Adds ignore_delete field to SinkDesc message |
| proto/connector_service.proto | Adds ignore_delete field to SinkParam message |
| src/prost/src/lib.rs | Implements backward compatibility helper methods get_ignore_delete_compat() for multiple types |
| src/meta/model/src/sink.rs | Removes ForceAppendOnly enum variant and adds ignore_delete field, handles conversion with backward compatibility |
| src/meta/model/migration/src/m20251231_000000_sink_ignore_delete.rs | Database migration to add ignore_delete column and normalize deprecated FORCE_APPEND_ONLY rows |
| src/meta/model/migration/src/lib.rs | Registers the new migration |
| src/meta/src/controller/mod.rs | Updates PbSink conversion to include ignore_delete field |
| src/meta/src/manager/sink_coordination/manager.rs | Updates test fixtures with ignore_delete: false |
| src/frontend/src/optimizer/plan_node/stream_sink.rs | Refactors sink type derivation to return tuple of (SinkType, bool) and removes ForceAppendOnly handling |
| src/connector/src/sink/catalog/mod.rs | Removes ForceAppendOnly enum variant, updates conversion logic, adds ignore_delete field to SinkCatalog |
| src/connector/src/sink/catalog/desc.rs | Adds ignore_delete field to SinkDesc |
| src/connector/src/sink/mod.rs | Adds ignore_delete field to SinkParam and uses backward compatibility helper in conversions |
| src/stream/src/from_proto/sink.rs | Retrieves ignore_delete using backward compatibility helper |
| src/stream/src/executor/sink.rs | Refactors to use ignore_delete parameter instead of ForceAppendOnly type, updates compaction and filtering logic |
| src/bench/sink_bench/main.rs | Updates benchmark test fixture with ignore_delete: false |
Signed-off-by: Bugen Zhao <[email protected]>
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Previously we have
ForceAppendOnlyas aSinkType, which allows sinking non-append-only stream to append-only sink by droppingDeleterecords.However, whether to drop
Deleterecords can actually be orthogonal to whether the sink is append-only. For example, some users want an upsert sink to processInsertandUpdateoperations as usual while ignoringDeleteoperations to prevent accidental data deletion in downstream systems.This PR refactors
SinkType::ForceAppendOnlyto beSinkType::AppendOnlywith a newly introducedignore_deletefield set totruewhile maintaining backward compatibility, without actual logic changes. In future PRs, we may allow a combination ofSinkType::Upsertwithignore_deleteto unlock the mentioned use case.Checklist
Documentation
Release note