Skip to content

Fix panic when attempting to set default_timestamp_interval to 0#36853

Open
peterdukelarsen wants to merge 3 commits into
MaterializeInc:mainfrom
peterdukelarsen:pl/ss-147-thread-coordinator-panic
Open

Fix panic when attempting to set default_timestamp_interval to 0#36853
peterdukelarsen wants to merge 3 commits into
MaterializeInc:mainfrom
peterdukelarsen:pl/ss-147-thread-coordinator-panic

Conversation

@peterdukelarsen
Copy link
Copy Markdown
Contributor

@peterdukelarsen peterdukelarsen commented Jun 1, 2026

Motivation

Closes ss-147

Description

Adds a non-zero constraint to the "default_timestamp_interval" system parameter. This prevents an issue where setting it to 0 causes a panic and the database won't start up until you go in and update the parameter by different means.

Verification

  1. Reproduced the issue locally, updated code and confirmed we received an error instead of crashing when the ALTER ... statement is issued
  2. Added basic sqllogictest tests

@peterdukelarsen peterdukelarsen requested a review from a team as a code owner June 1, 2026 20:03
@peterdukelarsen peterdukelarsen requested a review from ublubu June 1, 2026 20:08
Copy link
Copy Markdown
Contributor

@ublubu ublubu left a comment

Choose a reason for hiding this comment

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

Looks good. It does give me a new question, though. (inline)

Comment on lines +127 to +131
if d.is_zero() {
Err(VarError::InvalidParameterValue {
name: var.name(),
invalid_values: vec![format!("{:?}", d)],
reason: "only supports durations greater than zero".to_string(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks a little weird to is_zero() and then say greater than zero. I had to double-check that Duration can't be negative 😅

simple conn=mz_system,user=mz_system
ALTER SYSTEM SET default_timestamp_interval = '-1';
----
db error: ERROR: parameter "default_timestamp_interval" requires a "duration" value
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I realize this isn't a regression, but this error message is interesting, especially given that the above tests use values like 1 and 0, which don't appear to be durations either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants