Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(query): ignore ascii case for flight compression setting #13957

Merged
merged 6 commits into from
Dec 8, 2023
Merged
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
13 changes: 11 additions & 2 deletions src/query/settings/src/settings_default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -610,14 +610,23 @@ impl DefaultSettings {
Ok(Self::instance()?.settings.contains_key(key))
}

pub fn convert_value(k: String, v: String) -> Result<(String, Option<UserSettingValue>)> {
pub fn convert_value(k: String, mut v: String) -> Result<(String, Option<UserSettingValue>)> {
let default_settings = DefaultSettings::instance()?;

match default_settings.settings.get(&k) {
None => Ok((k, None)),
Some(setting_value) => {
if let Some(possible_values) = &setting_value.possible_values {
if !possible_values.iter().any(|x| x.eq_ignore_ascii_case(&v)) {
let mut checked_value = false;

for possible_value in possible_values {
if possible_value.eq_ignore_ascii_case(&v) {
checked_value = true;
v = possible_value.to_string();
}
}

if !checked_value {
return Err(ErrorCode::WrongValueForVariable(format!(
"Invalid setting value: {:?} for variable {:?}, possible values: {:?}",
v, k, possible_values
Expand Down
12 changes: 8 additions & 4 deletions src/query/settings/src/settings_getter_setter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ impl Settings {
}

pub fn get_sql_dialect(&self) -> Result<Dialect> {
match self.try_get_string("sql_dialect")?.as_str() {
match self.try_get_string("sql_dialect")?.to_lowercase().as_str() {
"hive" => Ok(Dialect::Hive),
"mysql" => Ok(Dialect::MySQL),
"experimental" => Ok(Dialect::Experimental),
Expand All @@ -267,7 +267,7 @@ impl Settings {
}

pub fn get_collation(&self) -> Result<&str> {
match self.try_get_string("collation")?.as_str() {
match self.try_get_string("collation")?.to_lowercase().as_str() {
"utf8" => Ok("utf8"),
_ => Ok("binary"),
}
Expand Down Expand Up @@ -503,8 +503,12 @@ impl Settings {
}

pub fn get_query_flight_compression(&self) -> Result<Option<FlightCompression>> {
match self.try_get_string("query_flight_compression")?.as_str() {
"None" => Ok(None),
match self
.try_get_string("query_flight_compression")?
.to_uppercase()
.as_str()
{
"NONE" => Ok(None),
"LZ4" => Ok(Some(FlightCompression::Lz4)),
"ZSTD" => Ok(Some(FlightCompression::Zstd)),
_ => unreachable!("check possible_values in set variable"),
Expand Down
16 changes: 16 additions & 0 deletions tests/sqllogictests/suites/base/06_show/06_0003_show_settings.test
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,19 @@ unset max_memory_usage

statement ok
unset max_threads

statement ok
set query_flight_compression = 'Lz4';

query TT
select value from system.settings where name = 'query_flight_compression'
----
LZ4

statement ok
set query_flight_compression = 'lz4';

query TT
select value from system.settings where name = 'query_flight_compression'
----
LZ4
Loading