andygrove commented on code in PR #3522: URL: https://github.com/apache/arrow-datafusion/pull/3522#discussion_r975767764
########## datafusion/core/src/config.rs: ########## @@ -288,26 +288,37 @@ impl ConfigOptions { } /// get a boolean configuration option - pub fn get_bool(&self, key: &str) -> bool { + pub fn get_bool(&self, key: &str) -> Option<bool> { match self.get(key) { - Some(ScalarValue::Boolean(Some(b))) => b, - _ => false, + Some(ScalarValue::Boolean(b)) => b, + Some(b) => Some( + b.to_string() + .parse::<bool>() + .unwrap_or_else(|_| panic!("Cannot parse bool from {:?}", &b)), + ), + None => None, } } /// get a u64 configuration option - pub fn get_u64(&self, key: &str) -> u64 { + pub fn get_u64(&self, key: &str) -> Option<u64> { match self.get(key) { - Some(ScalarValue::UInt64(Some(n))) => n, - _ => 0, + Some(ScalarValue::UInt64(n)) => n, + Some(n) => Some( + n.to_string() + .parse::<u64>() + .unwrap_or_else(|_| panic!("Cannot parse u64 from {:?}", &n)), + ), + _ => None, } } /// get a string configuration option - pub fn get_string(&self, key: &str) -> String { + pub fn get_string(&self, key: &str) -> Option<String> { match self.get(key) { - Some(ScalarValue::Utf8(Some(s))) => s, - _ => "".into(), + Some(ScalarValue::Utf8(s)) => s, + Some(s) => Some(s.to_string()), Review Comment: I'm not sure this is what we want. What does this return for a u64 value? Could you add a test for this? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org