Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/sql/src/session/vars/constraints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

use std::fmt::Debug;
use std::ops::{RangeBounds, RangeFrom, RangeInclusive};
use std::time::Duration;

use mz_repr::adt::numeric::Numeric;
use mz_repr::bytes::ByteSize;
Expand All @@ -19,6 +20,8 @@ use super::{Value, Var, VarError};

pub static NUMERIC_NON_NEGATIVE: NumericNonNegNonNan = NumericNonNegNonNan;

pub static NON_ZERO_DURATION: NonZeroDuration = NonZeroDuration;

pub static NUMERIC_BOUNDED_0_1_INCLUSIVE: NumericInRange<RangeInclusive<f64>> =
NumericInRange(0.0f64..=1.0);

Expand Down Expand Up @@ -114,6 +117,25 @@ impl DomainConstraint for NumericNonNegNonNan {
}
}

#[derive(Debug, Clone, Eq, PartialEq)]
pub struct NonZeroDuration;

impl DomainConstraint for NonZeroDuration {
type Value = Duration;

fn check(&self, var: &dyn Var, d: &Duration) -> Result<(), VarError> {
if d.is_zero() {
Err(VarError::InvalidParameterValue {
name: var.name(),
invalid_values: vec![format!("{:?}", d)],
reason: "only supports durations greater than zero".to_string(),
Comment on lines +127 to +131
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 😅

})
} else {
Ok(())
}
}
}

#[derive(Debug, Clone, Eq, PartialEq)]
pub struct NumericInRange<R>(pub R);

Expand Down
7 changes: 4 additions & 3 deletions src/sql/src/session/vars/definitions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ use uncased::UncasedStr;

use crate::session::user::{SUPPORT_USER, SYSTEM_USER, User};
use crate::session::vars::constraints::{
BYTESIZE_AT_LEAST_1MB, DomainConstraint, NUMERIC_BOUNDED_0_1_INCLUSIVE, NUMERIC_NON_NEGATIVE,
ValueConstraint,
BYTESIZE_AT_LEAST_1MB, DomainConstraint, NON_ZERO_DURATION, NUMERIC_BOUNDED_0_1_INCLUSIVE,
NUMERIC_NON_NEGATIVE, ValueConstraint,
};
use crate::session::vars::errors::VarError;
use crate::session::vars::polyfill::{LazyValueFn, lazy_value, value};
Expand Down Expand Up @@ -1441,7 +1441,8 @@ pub static DEFAULT_TIMESTAMP_INTERVAL: VarDefinition = VarDefinition::new(
value!(Duration; Duration::from_millis(1000)),
"The interval at which timestamps are assigned to data from sources and tables.",
false,
);
)
.with_constraint(&NON_ZERO_DURATION);

pub static MIN_TIMESTAMP_INTERVAL: VarDefinition = VarDefinition::new(
"min_timestamp_interval",
Expand Down
15 changes: 15 additions & 0 deletions test/sqllogictest/alter.slt
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,21 @@ ALTER SYSTEM SET max_timestamp_interval = '10s';
----
COMPLETE 0

simple conn=mz_system,user=mz_system
ALTER SYSTEM SET default_timestamp_interval = '1';
----
COMPLETE 0

simple conn=mz_system,user=mz_system
ALTER SYSTEM SET default_timestamp_interval = '0';
----
db error: ERROR: parameter "default_timestamp_interval" cannot have value "0ns": only supports durations greater than zero

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.


statement ok
ALTER SOURCE s SET (TIMESTAMP INTERVAL '2s')

Expand Down
Loading