fix(frontend): reject TTL watermarks on refreshable tables by yashaggarwal85d · Pull Request #27624 · risingwavelabs/risingwave

yashaggarwal85d · 2026-10-23T04:53:30Z

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

What's changed and what's your intention?

Creating a refreshable table (refresh_mode = 'FULL_RELOAD') with a WATERMARK ... WITH TTL clause panics today:

TTL column not in the primary key: the frontend panics in StreamMaterialize::derive_staging_table_catalog (mapping.try_map_all(stream_key).unwrap()), because the stream key includes the TTL column but the pk-only staging table does not.
TTL column in the primary key: the DDL succeeds, but the compute node later panics in state_table.rs with an index out of bounds, because the staging catalog copies clean_watermark_indices without remapping them to the staging table's columns.

As suggested in the issue, properly supporting this needs work in the MaterializeExecutor refresh merge path (feed TTL watermarks into the table and skip expired rows during the merge). Until then, this PR rejects the combination in the frontend with a clear error, right next to the existing "refreshable tables must have a PRIMARY KEY" check in gen_table_plan:

Not supported: Watermark `WITH TTL` is not supported on refreshable tables (`refresh_mode = 'FULL_RELOAD'`).
HINT: Remove `WITH TTL` from the watermark definition, or create the table without `refresh_mode = 'FULL_RELOAD'`.

Tests:

Planner tests in watermark_ttl.yaml cover both cases (TTL column outside and inside the pk).
e2e_test/source_inline/refresh/refresh_table.slt asserts both CREATE TABLE statements fail with the new error.
Closes Refreshable tables with a TTL watermark panic in the frontend or the compute node #27509

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.
I have checked the Release Timeline and Currently Supported Versions to determine which release branches I need to cherry-pick this PR into.

Documentation

My PR needs documentation updates.

Release note

Creating a refreshable table (refresh_mode = 'FULL_RELOAD') with a WATERMARK ... WITH TTL now returns a "not supported" error instead of crashing the frontend or the compute node.

This solution was AI-assisted (Claude Code); I reviewed and tested the changes.

Summary by CodeRabbit

Bug Fixes
- Refreshable tables using WITH TTL watermarks are now rejected during planning, including when the TTL column is part of a single-column or composite primary key.
- The planning error explains that this configuration is unsupported and suggests removing the TTL watermark or disabling FULL_RELOAD.

coderabbitai · 2026-10-23T04:53:51Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 30c61fe1-6ed3-4960-9778-8c9032ef40cf

📥 Commits

Reviewing files that changed from the base of the PR and between 7d26b00 and 01a486c.

📒 Files selected for processing: 4

e2e_test/source_inline/refresh/refresh_table.slt
src/frontend/planner_test/tests/testdata/input/watermark_ttl.yaml
src/frontend/planner_test/tests/testdata/output/watermark_ttl.yaml
src/frontend/src/optimizer/mod.rs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

The planner now rejects refreshable tables that use WITH TTL watermarks. Planner and end-to-end tests cover cases where the TTL column is outside or inside the primary key.

Changes: Refreshable table TTL watermark validation

Layer / File(s)	Summary
Planner rejection and test coverage `src/frontend/src/optimizer/mod.rs`, `src/frontend/planner_test/tests/testdata/{input,output}/watermark_ttl.yaml`, `e2e_test/source_inline/refresh/refresh_table.slt`	`gen_table_plan` returns `NotSupported` when a refreshable table has a TTL watermark. Planner and end-to-end tests verify the error for single-column and composite primary keys.

Priority: ⚪ Pending latest changes

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested labels: type/fix, user-facing-changes, ci/run-sqlsmith-fuzzing-tests

Suggested reviewers: chenzl25, yuhao-su

Merge Risk: ⚪ Minimal · up to 01a48

The change provides the intended frontend error without affecting streaming or source-less table planning.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: rejecting TTL watermarks on refreshable tables.
Description check		The description is complete and follows the repository template. It explains the panic scenarios, the implemented frontend rejection, the error guidance, test coverage, issue link, checklist, and user…
Linked Issues check		Issue [`#27509`] requires that refreshable tables with `WATERMARK ... WITH TTL` do not panic for either primary-key case. The change rejects `refreshable && !ttl_watermark_indices.is_empty()` in `gen_ta…
Out of Scope Changes check		The changes are limited to the frontend rejection and tests for issue [`#27509`]. The planner fixtures and refresh E2E cases directly verify the new behavior. No unrelated implementation or behavior cha…
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (3 skipped: 3 …

Full details: Description check

Explanation

The description is complete and follows the repository template. It explains the panic scenarios, the implemented frontend rejection, the error guidance, test coverage, issue link, checklist, and user impact.

Full details: Linked Issues check

Explanation

Issue [#27509] requires that refreshable tables with WATERMARK ... WITH TTL do not panic for either primary-key case. The change rejects refreshable && !ttl_watermark_indices.is_empty() in gen_table_plan with the specified NotSupported error and actionable hint. Planner tests cover the TTL column outside and inside the primary key. The refresh E2E test covers both DDL cases.

Full details: Out of Scope Changes check

Explanation

The changes are limited to the frontend rejection and tests for issue [#27509]. The planner fixtures and refresh E2E cases directly verify the new behavior. No unrelated implementation or behavior change appears in the reviewed diff.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (3 skipped: 3 unsupported.)

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

Creating a FULL_RELOAD refreshable table with a `WATERMARK ... WITH TTL` panicked in the frontend (TTL column outside the pk) or on the compute node (TTL column inside the pk), because the pk-only staging table cannot hold the TTL watermark column. Reject the combination with a clear error until the refresh merge supports TTL. Fixes risingwavelabs#27509

yashaggarwal85d · 2026-10-24T04:45:38Z

Hi @yuhao-su, this is the interim frontend check you suggested in #27509: it rejects refresh_mode = 'FULL_RELOAD' combined with WATERMARK ... WITH TTL instead of panicking, and adds planner and SLT tests. Since the issue is on the release-3.1 milestone, could you take a look when you have a moment? A /approve-run-bk would also be appreciated so Buildkite can run on the fork. Thanks!

yashaggarwal85d · 2026-10-27T13:14:52Z

Hi @tabVersion @BugenZhao, would one of you mind reviewing this? It’s a small frontend check so FULL_RELOAD tables with a TTL watermark get a clear “not supported” error instead of panicking (#27509). Also needs a /approve-run-bk for CI. Thanks!

yashaggarwal85d force-pushed the claude/project-thread-tm63bd branch from 01a486c to 298067b Compare October 23, 2026 04:55

Navigation Menu

Repository navigation

fix(frontend): reject TTL watermarks on refreshable tables - #27624

fix(frontend): reject TTL watermarks on refreshable tables#27624
yashaggarwal85d wants to merge 1 commit into
risingwavelabs:mainfrom
yashaggarwal85d:claude/project-thread-tm63bd

yashaggarwal85d commented Oct 23, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 23, 2026 •

edited

Loading

Uh oh!

yashaggarwal85d commented Oct 24, 2026

Uh oh!

yashaggarwal85d commented Oct 27, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

yashaggarwal85d commented Oct 23, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's changed and what's your intention?

Checklist

Documentation

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yashaggarwal85d commented Oct 24, 2026

Uh oh!

yashaggarwal85d commented Oct 27, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

yashaggarwal85d commented Oct 23, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 23, 2026 •

edited

Loading