From 0de3b72814be73c246c4f0e76cde7802b77eebd2 Mon Sep 17 00:00:00 2001 From: BohuTANG Date: Thu, 14 Dec 2023 10:45:26 +0800 Subject: [PATCH] chore: merge settings possible_values to range --- Cargo.lock | 1 + src/query/settings/Cargo.toml | 1 + src/query/settings/src/settings.rs | 3 - src/query/settings/src/settings_default.rs | 226 ++++++++---------- .../settings/src/settings_getter_setter.rs | 54 +++-- src/query/settings/src/settings_global.rs | 37 ++- 6 files changed, 160 insertions(+), 162 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dbb9d411f8a5..2a52c49de3b0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2663,6 +2663,7 @@ dependencies = [ "dashmap", "itertools 0.10.5", "log", + "num", "num_cpus", "once_cell", "serde", diff --git a/src/query/settings/Cargo.toml b/src/query/settings/Cargo.toml index db1cd9ced37c..ca484350ba67 100644 --- a/src/query/settings/Cargo.toml +++ b/src/query/settings/Cargo.toml @@ -24,3 +24,4 @@ log = { workspace = true } num_cpus = "1.13.1" once_cell = { workspace = true } sys-info = "0.9" +num = "0.4.1" diff --git a/src/query/settings/src/settings.rs b/src/query/settings/src/settings.rs index 551357851dbc..afa8b9a4fc15 100644 --- a/src/query/settings/src/settings.rs +++ b/src/query/settings/src/settings.rs @@ -115,7 +115,6 @@ pub struct SettingsItem { pub desc: &'static str, pub user_value: UserSettingValue, pub default_value: UserSettingValue, - pub possible_values: Option>, } pub struct SettingsIter<'a> { @@ -156,7 +155,6 @@ impl<'a> Iterator for SettingsIter<'a> { desc: default_value.desc, user_value: default_value.value.clone(), default_value: default_value.value, - possible_values: default_value.possible_values, }, Some(change_value) => SettingsItem { name: key, @@ -164,7 +162,6 @@ impl<'a> Iterator for SettingsIter<'a> { desc: default_value.desc, user_value: change_value.value.clone(), default_value: default_value.value, - possible_values: default_value.possible_values, }, }), }; diff --git a/src/query/settings/src/settings_default.rs b/src/query/settings/src/settings_default.rs index a85bf395f43c..4c1d83879605 100644 --- a/src/query/settings/src/settings_default.rs +++ b/src/query/settings/src/settings_default.rs @@ -37,13 +37,56 @@ pub enum SettingMode { Write, } +#[derive(Clone, Debug)] +pub enum SettingRange { + Numeric(Range), + String(Vec<&'static str>), +} + +impl SettingRange { + /// Checks if an integer value is within the numeric range. + pub fn is_within_numeric_range(&self, value: u64) -> Result<()> { + match self { + SettingRange::Numeric(range) => { + if range.contains(&value) { + Ok(()) + } else { + Err(ErrorCode::BadArguments(format!( + "Value {} is not within the range {:?}", + value, range + ))) + } + } + _ => Err(ErrorCode::BadArguments( + "Expected numeric range".to_string(), + )), + } + } + + /// Checks if a string value is within the string range. + pub fn is_within_string_range(&self, value: &str) -> Result<()> { + match self { + SettingRange::String(values) => { + if values.iter().any(|s| s.eq_ignore_ascii_case(value)) { + Ok(()) + } else { + Err(ErrorCode::BadArguments(format!( + "Value {} is not within the allowed values {:?}", + value, values + ))) + } + } + _ => Err(ErrorCode::BadArguments("Expected string range".to_string())), + } + } +} + #[derive(Clone, Debug)] pub struct DefaultSettingValue { pub(crate) value: UserSettingValue, pub(crate) desc: &'static str, - pub(crate) possible_values: Option>, pub(crate) mode: SettingMode, - pub(crate) range: Option>, + pub(crate) range: Option, } #[derive(Clone)] @@ -64,28 +107,24 @@ impl DefaultSettings { ("max_block_size", DefaultSettingValue { value: UserSettingValue::UInt64(65536), desc: "Sets the maximum byte size of a single data block that can be read.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("parquet_max_block_size", DefaultSettingValue { value: UserSettingValue::UInt64(8192), desc: "Max block size for parquet reader", - possible_values: None, mode: SettingMode::Both, range: None, }), ("max_threads", DefaultSettingValue { value: UserSettingValue::UInt64(num_cpus), desc: "Sets the maximum number of threads to execute a request.", - possible_values: None, mode: SettingMode::Both, - range: Some(1..1024), + range: Some(SettingRange::Numeric(1..1024)), }), ("max_memory_usage", DefaultSettingValue { value: UserSettingValue::UInt64(max_memory_usage), desc: "Sets the maximum memory usage in bytes for processing a single query.", - possible_values: None, mode: SettingMode::Both, range: None, }), @@ -93,36 +132,31 @@ impl DefaultSettings { // unit of retention_period is hour value: UserSettingValue::UInt64(12), desc: "Sets the retention period in hours.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("max_storage_io_requests", DefaultSettingValue { value: UserSettingValue::UInt64(default_max_storage_io_requests), desc: "Sets the maximum number of concurrent I/O requests.", - possible_values: None, mode: SettingMode::Both, - range: Some(1..1024), + range: Some(SettingRange::Numeric(1..1024)), }), ("storage_io_min_bytes_for_seek", DefaultSettingValue { value: UserSettingValue::UInt64(48), desc: "Sets the minimum byte size of data that must be read from storage in a single I/O operation \ when seeking a new location in the data file.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("storage_io_max_page_bytes_for_read", DefaultSettingValue { value: UserSettingValue::UInt64(512 * 1024), desc: "Sets the maximum byte size of data pages that can be read from storage in a single I/O operation.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("flight_client_timeout", DefaultSettingValue { value: UserSettingValue::UInt64(60), desc: "Sets the maximum time in seconds that a flight client request can be processed.", - possible_values: None, mode: SettingMode::Both, range: None, }), @@ -133,182 +167,160 @@ impl DefaultSettings { UserSettingValue::UInt64(result_timeout_secs) }, desc: "Set the timeout in seconds that a http query session expires without any polls.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("storage_read_buffer_size", DefaultSettingValue { value: UserSettingValue::UInt64(1024 * 1024), desc: "Sets the byte size of the buffer used for reading data into memory.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("input_read_buffer_size", DefaultSettingValue { value: UserSettingValue::UInt64(4 * 1024 * 1024), desc: "Sets the memory size in bytes allocated to the buffer used by the buffered reader to read data from storage.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("timezone", DefaultSettingValue { value: UserSettingValue::String("UTC".to_owned()), desc: "Sets the timezone.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("group_by_two_level_threshold", DefaultSettingValue { value: UserSettingValue::UInt64(20000), desc: "Sets the number of keys in a GROUP BY operation that will trigger a two-level aggregation.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("max_inlist_to_or", DefaultSettingValue { value: UserSettingValue::UInt64(3), desc: "Sets the maximum number of values that can be included in an IN expression to be converted to an OR operator.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("unquoted_ident_case_sensitive", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Determines whether Databend treats unquoted identifiers as case-sensitive.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("quoted_ident_case_sensitive", DefaultSettingValue { value: UserSettingValue::UInt64(1), desc: "Determines whether Databend treats quoted identifiers as case-sensitive.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("sql_dialect", DefaultSettingValue { value: UserSettingValue::String("PostgreSQL".to_owned()), desc: "Sets the SQL dialect. Available values include \"PostgreSQL\", \"MySQL\", \"Experimental\", and \"Hive\".", - possible_values: Some(vec!["PostgreSQL", "MySQL", "Experimental", "Hive"]), mode: SettingMode::Both, - range: None, + range: Some(SettingRange::String(vec!["PostgreSQL", "MySQL", "Experimental", "Hive"])), }), ("enable_dphyp", DefaultSettingValue { value: UserSettingValue::UInt64(1), desc: "Enables dphyp join order algorithm.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("enable_cbo", DefaultSettingValue { value: UserSettingValue::UInt64(1), desc: "Enables cost-based optimization.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("disable_join_reorder", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Disable join reorder optimization.", - possible_values: None, + mode: SettingMode::Both, range: None, }), ("join_spilling_threshold", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Maximum amount of memory can use for hash join, 0 is unlimited.", - possible_values: None, + mode: SettingMode::Both, range: None, }), ("enable_runtime_filter", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Enables runtime filter optimization for JOIN.", - possible_values: None, + mode: SettingMode::Both, range: None, }), ("max_execute_time_in_seconds", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Sets the maximum query execution time in seconds. Setting it to 0 means no limit.", - possible_values: None, + mode: SettingMode::Both, range: None, }), ("collation", DefaultSettingValue { value: UserSettingValue::String("binary".to_owned()), desc: "Sets the character collation. Available values include \"binary\" and \"utf8\".", - possible_values: Some(vec!["binary", "utf8"]), mode: SettingMode::Both, - range: None, + range: Some(SettingRange::String(vec!["binary", "utf8"])), }), ("max_result_rows", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Sets the maximum number of rows that can be returned in a query result when no specific row count is specified. Setting it to 0 means no limit.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("prefer_broadcast_join", DefaultSettingValue { value: UserSettingValue::UInt64(1), desc: "Enables broadcast join.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("storage_fetch_part_num", DefaultSettingValue { value: UserSettingValue::UInt64(2), desc: "Sets the number of partitions that are fetched in parallel from storage during query execution.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("load_file_metadata_expire_hours", DefaultSettingValue { value: UserSettingValue::UInt64(24 * 7), desc: "Sets the hours that the metadata of files you load data from with COPY INTO will expire in.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("hide_options_in_show_create_table", DefaultSettingValue { value: UserSettingValue::UInt64(1), desc: "Hides table-relevant information, such as SNAPSHOT_LOCATION and STORAGE_FORMAT, at the end of the result of SHOW TABLE CREATE.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("sandbox_tenant", DefaultSettingValue { value: UserSettingValue::String("".to_string()), desc: "Injects a custom 'sandbox_tenant' into this session. This is only for testing purposes and will take effect only when 'internal_enable_sandbox_tenant' is turned on.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("parquet_uncompressed_buffer_size", DefaultSettingValue { value: UserSettingValue::UInt64(2 * 1024 * 1024), desc: "Sets the byte size of the buffer used for reading Parquet files.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("enable_bushy_join", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Enables generating a bushy join plan with the optimizer.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("enable_query_result_cache", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Enables caching query results to improve performance for identical queries.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("query_result_cache_max_bytes", DefaultSettingValue { value: UserSettingValue::UInt64(1048576), // 1MB desc: "Sets the maximum byte size of cache for a single query result.", - possible_values: None, mode: SettingMode::Both, range: None, }), @@ -316,84 +328,72 @@ impl DefaultSettings { value: UserSettingValue::UInt64(300), // seconds desc: "Sets the time-to-live (TTL) in seconds for cached query results. \ Once the TTL for a cached result has expired, the result is considered stale and will not be used for new queries.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("query_result_cache_allow_inconsistent", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Determines whether Databend will return cached query results that are inconsistent with the underlying data.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("enable_hive_parquet_predict_pushdown", DefaultSettingValue { value: UserSettingValue::UInt64(1), desc: "Enable hive parquet predict pushdown by setting this variable to 1, default value: 1", - possible_values: None, mode: SettingMode::Both, range: None, }), ("hive_parquet_chunk_size", DefaultSettingValue { value: UserSettingValue::UInt64(16384), desc: "the max number of rows each read from parquet to databend processor", - possible_values: None, mode: SettingMode::Both, range: None, }), ("aggregate_spilling_bytes_threshold_per_proc", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Sets the maximum amount of memory in bytes that an aggregator can use before spilling data to storage during query execution.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("aggregate_spilling_memory_ratio", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Sets the maximum memory ratio in bytes that an aggregator can use before spilling data to storage during query execution.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("sort_spilling_bytes_threshold_per_proc", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Sets the maximum amount of memory in bytes that a sorter can use before spilling data to storage during query execution.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("sort_spilling_memory_ratio", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Sets the maximum memory ratio in bytes that a sorter can use before spilling data to storage during query execution.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("group_by_shuffle_mode", DefaultSettingValue { value: UserSettingValue::String(String::from("before_merge")), desc: "Group by shuffle mode, 'before_partial' is more balanced, but more data needs to exchange.", - possible_values: Some(vec!["before_partial", "before_merge"]), mode: SettingMode::Both, - range: None, + range: Some(SettingRange::String(vec!["before_partial", "before_merge"])), }), ("efficiently_memory_group_by", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Memory is used efficiently, but this may cause performance degradation.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("lazy_read_threshold", DefaultSettingValue { value: UserSettingValue::UInt64(1000), desc: "Sets the maximum LIMIT in a query to enable lazy read optimization. Setting it to 0 disables the optimization.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("parquet_fast_read_bytes", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Parquet file with smaller size will be read as a whole file, instead of column by column.", - possible_values: None, mode: SettingMode::Both, range: None, }), @@ -402,7 +402,6 @@ impl DefaultSettings { ("enterprise_license", DefaultSettingValue { value: UserSettingValue::String("".to_owned()), desc: "License key for use enterprise features", - possible_values: None, // license key should not be reported mode: SettingMode::Write, range: None, @@ -410,231 +409,199 @@ impl DefaultSettings { ("enable_table_lock", DefaultSettingValue { value: UserSettingValue::UInt64(1), desc: "Enables table lock if necessary (enabled by default).", - possible_values: None, mode: SettingMode::Both, range: None, }), ("table_lock_expire_secs", DefaultSettingValue { value: UserSettingValue::UInt64(10), desc: "Sets the seconds that the table lock will expire in.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("acquire_lock_timeout", DefaultSettingValue { value: UserSettingValue::UInt64(15), desc: "Sets the maximum timeout in seconds for acquire a lock.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("deduplicate_label", DefaultSettingValue { value: UserSettingValue::String("".to_owned()), desc: "Sql duplicate label for deduplication.", - possible_values: None, mode: SettingMode::Write, range: None, }), ("enable_distributed_copy_into", DefaultSettingValue { value: UserSettingValue::UInt64(1), desc: "Enable distributed execution of copy into.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("enable_experimental_merge_into", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Enable experimental merge into.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("enable_distributed_merge_into", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Enable distributed merge into.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("merge_into_static_filter_partition_threshold", DefaultSettingValue { value: UserSettingValue::UInt64(1500), desc: "Max number of partitions allowed for static filtering of merge into statement", - possible_values: None, mode: SettingMode::Both, range: None, }), ("enable_distributed_replace_into", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Enable distributed execution of replace into.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("enable_distributed_compact", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Enable distributed execution of table compaction.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("enable_aggregating_index_scan", DefaultSettingValue { value: UserSettingValue::UInt64(1), desc: "Enable scanning aggregating index data while querying.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("enable_recluster_after_write", DefaultSettingValue { value: UserSettingValue::UInt64(1), desc: "Enables re-clustering after write(copy/replace-into).", - possible_values: None, mode: SettingMode::Both, range: None, }), ("use_parquet2", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Use parquet2 instead of parquet_rs when infer_schema().", - possible_values: None, mode: SettingMode::Both, range: None, }), ("enable_replace_into_partitioning", DefaultSettingValue { value: UserSettingValue::UInt64(1), desc: "Enables partitioning for replace-into statement (if table has cluster keys).", - possible_values: None, mode: SettingMode::Both, range: None, }), ("enable_replace_into_bloom_pruning", DefaultSettingValue { value: UserSettingValue::UInt64(1), desc: "Enables bloom pruning for replace-into statement.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("replace_into_bloom_pruning_max_column_number", DefaultSettingValue { value: UserSettingValue::UInt64(4), desc: "Max number of columns used by bloom pruning for replace-into statement.", - possible_values: None, + mode: SettingMode::Both, range: None, }), ("replace_into_shuffle_strategy", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "0 for Block level shuffle, 1 for segment level shuffle", - possible_values: None, mode: SettingMode::Both, range: None, }), ("recluster_timeout_secs", DefaultSettingValue { value: UserSettingValue::UInt64(12 * 60 * 60), desc: "Sets the seconds that recluster final will be timeout.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("enable_refresh_aggregating_index_after_write", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Refresh aggregating index after new data written", - possible_values: None, mode: SettingMode::Both, range: None, }), ("ddl_column_type_nullable", DefaultSettingValue { value: UserSettingValue::UInt64(1), desc: "If columns are default nullable when create or alter table", - possible_values: None, mode: SettingMode::Both, range: None, }), ("enable_query_profiling", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Enables recording query profile", - possible_values: None, mode: SettingMode::Both, range: None, }), ("recluster_block_size", DefaultSettingValue { value: UserSettingValue::UInt64(recluster_block_size), desc: "Sets the maximum byte size of blocks for recluster", - possible_values: None, mode: SettingMode::Both, range: None, }), ("enable_distributed_recluster", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Enable distributed execution of table recluster.", - possible_values: None, mode: SettingMode::Both, range: None, }), ("enable_parquet_page_index", DefaultSettingValue { value: UserSettingValue::UInt64(1), desc: "Enables parquet page index", - possible_values: None, mode: SettingMode::Both, range: None, }), ("enable_parquet_rowgroup_pruning", DefaultSettingValue { value: UserSettingValue::UInt64(1), desc: "Enables parquet rowgroup pruning", - possible_values: None, mode: SettingMode::Both, range: None, }), ("external_server_connect_timeout_secs", DefaultSettingValue { value: UserSettingValue::UInt64(10), desc: "Connection timeout to external server", - possible_values: None, mode: SettingMode::Both, range: None, }), ("external_server_request_timeout_secs", DefaultSettingValue { value: UserSettingValue::UInt64(180), desc: "Request timeout to external server", - possible_values: None, mode: SettingMode::Both, range: None, }), ("enable_parquet_prewhere", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Enables parquet prewhere", - possible_values: None, mode: SettingMode::Both, range: None, }), ("numeric_cast_option", DefaultSettingValue { value: UserSettingValue::String("rounding".to_string()), desc: "Set numeric cast mode as \"rounding\" or \"truncating\".", - possible_values: Some(vec!["rounding", "truncating"]), mode: SettingMode::Both, - range: None, + range: Some(SettingRange::String(vec!["rounding", "truncating"])), }), ("enable_experimental_rbac_check", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "experiment setting disables stage and udf privilege check(disable by default).", - possible_values: None, mode: SettingMode::Both, range: None, }), ("create_query_flight_client_with_current_rt", DefaultSettingValue { value: UserSettingValue::UInt64(1), desc: "create query flight client with current runtime", - possible_values: None, mode: SettingMode::Both, range: None, }), ("query_flight_compression", DefaultSettingValue { value: UserSettingValue::String(String::from("LZ4")), desc: "flight compression method", - possible_values: Some(vec!["None", "LZ4", "ZSTD"]), mode: SettingMode::Both, - range: None, + range: Some(SettingRange::String(vec!["None", "LZ4", "ZSTD"])), }), ("enable_refresh_virtual_column_after_write", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "Refresh virtual column after new data written", - possible_values: None, mode: SettingMode::Both, range: None, }), @@ -711,56 +678,55 @@ impl DefaultSettings { Ok(Self::instance()?.settings.contains_key(key)) } - pub fn convert_value(k: String, mut v: String) -> Result<(String, Option)> { + /// Converts and validates a setting value based on its key. + /// + /// # Arguments + /// * `k` - The key of the setting. + /// * `v` - The value of the setting to be validated and converted. + /// + /// # Returns + /// * A result containing a tuple of the key and an optional `UserSettingValue` if valid, or an error. + pub fn convert_value(k: String, v: String) -> Result<(String, Option)> { + // Retrieve the default settings instance let default_settings = DefaultSettings::instance()?; + // Match the setting key with the settings definition match default_settings.settings.get(&k) { + // Return None for keys that are not found in the settings None => Ok((k, None)), + + // Process the setting value for found keys Some(setting_value) => { - if let Some(possible_values) = &setting_value.possible_values { - let mut checked_value = false; + match &setting_value.range { + None => Ok((k, None)), + Some(range) => { + match range { + // Numeric range. + SettingRange::Numeric(_) => { + // Attempt to parse the value as an unsigned 64-bit integer + let u64_val = v.parse::().map_err(|_| { + ErrorCode::BadArguments(format!( + "{} is not a valid u64 value", + v + )) + })?; + range.is_within_numeric_range(u64_val)?; + + // Return the key and value if valid + Ok((k, Some(UserSettingValue::UInt64(u64_val)))) + } + // String range. + SettingRange::String(_) => { + // Convert the value to lowercase for case-insensitive comparison + let val_lower = v.to_lowercase(); - for possible_value in possible_values { - if possible_value.eq_ignore_ascii_case(&v) { - checked_value = true; - v = possible_value.to_string(); - } - } + range.is_within_string_range(&val_lower)?; - if !checked_value { - return Err(ErrorCode::WrongValueForVariable(format!( - "Invalid setting value: {:?} for variable {:?}, possible values: {:?}", - v, k, possible_values - ))); - } - } - match setting_value.value { - UserSettingValue::UInt64(_) => { - // decimal 10 * 1.5 to string may result in string like "15.0" - let val = if let Some(p) = v.find('.') { - if v[(p + 1)..].chars().all(|x| x == '0') { - &v[..p] - } else { - return Err(ErrorCode::BadArguments("not a integer")); - } - } else { - &v[..] - }; - - let u64_val = val.parse::()?; - - if let Some(range) = &setting_value.range { - if !range.contains(&u64_val) { - return Err(ErrorCode::BadArguments(format!( - "{} value should in range: {:?}", - k, range - ))); + // Return the key and value if valid + Ok((k, Some(UserSettingValue::String(v)))) } } - - Ok((k, Some(UserSettingValue::UInt64(u64_val)))) } - UserSettingValue::String(_) => Ok((k, Some(UserSettingValue::String(v)))), } } } diff --git a/src/query/settings/src/settings_getter_setter.rs b/src/query/settings/src/settings_getter_setter.rs index 53b45cbeeb67..d27a2c239e5b 100644 --- a/src/query/settings/src/settings_getter_setter.rs +++ b/src/query/settings/src/settings_getter_setter.rs @@ -64,35 +64,47 @@ impl Settings { unsafe { self.unchecked_try_set_u64(key, val) } } + /// Sets a u64 value for a given key in the settings. + /// Ensures that the key exists, the setting type is UInt64, and the value is within any defined numeric range. unsafe fn unchecked_try_set_u64(&self, key: &str, val: u64) -> Result<()> { - match DefaultSettings::instance()?.settings.get(key) { + // Retrieve the instance of default settings + let default_settings = DefaultSettings::instance()?; + + // Check if the key exists in the default settings + match default_settings.settings.get(key) { + // If the key does not exist, return an error indicating an unknown variable None => Err(ErrorCode::UnknownVariable(format!( "Unknown variable: {:?}", key ))), - Some(default_val) => { - if !matches!(&default_val.value, UserSettingValue::UInt64(_)) { - return Err(ErrorCode::BadArguments(format!( - "Set a integer({}) into {:?}.", - val, key - ))); - } - if let Some(range) = &default_val.range { - if !range.contains(&val) { - return Err(ErrorCode::BadArguments(format!( - "{} value should in range: {:?}", - key, range - ))); + // If the key exists, proceed to validate and set the value + Some(default_val) => { + // Ensure the setting type is UInt64 + match &default_val.value { + UserSettingValue::UInt64(_) => { + // If a numeric range is defined, validate the value against this range + if let Some(range) = &default_val.range { + // Check if the value falls within the numeric range + range.is_within_numeric_range(val).map_err(|err| { + ErrorCode::BadArguments(format!("{}: {}", key, err)) + })?; + } + + // Insert the value into changes with a session scope + self.changes.insert(key.to_string(), ChangeValue { + level: ScopeLevel::Session, + value: UserSettingValue::UInt64(val), + }); + + Ok(()) } + // If the setting type is not UInt64, return an error + _ => Err(ErrorCode::BadArguments(format!( + "Set an integer ({}) into {:?}.", + val, key + ))), } - - self.changes.insert(key.to_string(), ChangeValue { - level: ScopeLevel::Session, - value: UserSettingValue::UInt64(val), - }); - - Ok(()) } } } diff --git a/src/query/settings/src/settings_global.rs b/src/query/settings/src/settings_global.rs index 75c754084ba0..61aee9a4200b 100644 --- a/src/query/settings/src/settings_global.rs +++ b/src/query/settings/src/settings_global.rs @@ -25,6 +25,7 @@ use log::warn; use crate::settings::ChangeValue; use crate::settings::Settings; use crate::settings_default::DefaultSettings; +use crate::settings_default::SettingRange; use crate::ScopeLevel; impl Settings { @@ -96,14 +97,34 @@ impl Settings { continue; } Some(default_setting_value) => { - if !default_setting_value - .possible_values - .as_ref() - .map(|values| values.iter().any(|v| v.eq_ignore_ascii_case(&val))) - .unwrap_or(true) - { - // the settings may be deprecated - warn!("Ignore invalid global setting {} = {}", name, val); + // Check if the setting value is valid based on the defined range + let validation_result = match &default_setting_value.range { + Some(SettingRange::Numeric(_)) => match val.parse::() { + Ok(num) => default_setting_value + .range + .as_ref() + .ok_or_else(|| "Range not set correctly".to_string()) + .and_then(|range| { + range + .is_within_numeric_range(num) + .map_err(|err| err.to_string()) + }), + Err(_) => Err("Value is not a valid u64".to_string()), + }, + Some(SettingRange::String(_)) => default_setting_value + .range + .as_ref() + .ok_or_else(|| "Range not set correctly".to_string()) + .and_then(|range| { + range + .is_within_string_range(&val) + .map_err(|err| err.to_string()) + }), + None => Ok(()), + }; + + if let Err(err) = validation_result { + warn!("Ignore invalid global setting {} = {}: {}", name, val, err); continue; }