add time_zone into ConfigOptions #3485
Conversation
| pub fn new_string( | ||
| key: impl Into<String>, | ||
| description: impl Into<String>, | ||
| default_value: String, | ||
| ) -> Self { | ||
| Self::new( | ||
| key, | ||
| description, | ||
| DataType::UInt64, | ||
| ScalarValue::Utf8(Some(default_value)), | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
add a new_string method for adding String Type configuration
| } else if variable_lower == "timezone" || variable_lower == "time.zone" { | ||
| // we could introduce alias in OptionDefinition if this string matching thing grows | ||
| String::from("SELECT name, setting FROM information_schema.df_settings WHERE name = 'datafusion.execution.time_zone'") |
There was a problem hiding this comment.
if we have more and more alias like this, we could consider add aliases into ConfigDefinition
Codecov Report
@@ Coverage Diff @@
## master #3485 +/- ##
=======================================
Coverage 85.75% 85.76%
=======================================
Files 299 299
Lines 55273 55311 +38
=======================================
+ Hits 47402 47435 +33
- Misses 7871 7876 +5
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Other than the errant println! this looks good to me -- thanks @waitingkuo
datafusion/sql/src/planner.rs
Outdated
|
|
||
| let query = if variable.to_lowercase() == "all" { | ||
| let variable_lower = variable.to_lowercase(); | ||
| println!("{}", variable_lower); |
There was a problem hiding this comment.
Perhaps is this a left over debugging println!?
There was a problem hiding this comment.
my fault, let me push the fix
|
Thanks @waitingkuo |
Co-authored-by: Kun Liu <liukun@apache.org>
|
thank you @liukun4515 |
|
Benchmark runs are scheduled for baseline = 6ffda68 and contender = 3319220. 3319220 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
5 participants
Which issue does this PR close?
Closes #3255
Rationale for this change
some of the function requires session timezone to work
e.g.
EXTRACT(HOUR FROM SOME_TIME)need thisas
SHOWstatement is done in #3455we could do this now
I also added the shortcut to align with postgrseql
What changes are included in this PR?
Are there any user-facing changes?