Skip to content

Commit

Permalink
change the string setting range return the value
Browse files Browse the repository at this point in the history
  • Loading branch information
BohuTANG committed Dec 14, 2023
1 parent c29d77c commit fb0003a
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 10 deletions.
18 changes: 8 additions & 10 deletions src/query/settings/src/settings_default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,15 @@ impl SettingRange {
}

/// Checks if a string value is within the string range.
pub fn is_within_string_range(&self, value: &str) -> Result<()> {
pub fn is_within_string_range(&self, value: &str) -> Result<String> {
match self {
SettingRange::String(values) => {
if values.iter().any(|s| s.eq_ignore_ascii_case(value)) {
Ok(())
} else {
Err(ErrorCode::BadArguments(format!(
match values.iter().find(|&s| s.eq_ignore_ascii_case(value)) {
Some(s) => Ok(s.to_string()),
None => Err(ErrorCode::BadArguments(format!(
"Value {} is not within the allowed values {:?}",
value, values
)))
))),
}
}
_ => Err(ErrorCode::BadArguments("Expected string range".to_string())),
Expand Down Expand Up @@ -723,11 +722,10 @@ impl DefaultSettings {
}
// String range.
SettingRange::String(_) => {
// Convert the value to lowercase for case-insensitive comparison
let val_lower = v.to_lowercase();
range.is_within_string_range(&val_lower)?;
// value is the standard value of the setting.
let value = range.is_within_string_range(&v)?;

Ok((k, UserSettingValue::String(v)))
Ok((k, UserSettingValue::String(value)))
}
}
}
Expand Down
13 changes: 13 additions & 0 deletions src/query/settings/tests/it/setting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,24 @@ fn test_set_settings() {
.set_setting("query_flight_compression".to_string(), "LZ4".to_string())
.unwrap();

// Ok
settings
.set_setting("query_flight_compression".to_string(), "lz4".to_string())
.unwrap();

// Error
let result = settings.set_setting("query_flight_compression".to_string(), "xx".to_string());
let expect = "BadArguments. Code: 1006, Text = Value xx is not within the allowed values [\"None\", \"LZ4\", \"ZSTD\"].";
assert_eq!(expect, format!("{}", result.unwrap_err()));
}

// String without range.
{
// Ok
settings
.set_setting("sandbox_tenant".to_string(), "xx".to_string())
.unwrap();
}
}

#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
Expand Down

0 comments on commit fb0003a

Please sign in to comment.