[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #3038: feat: Support `week`, `decade`, `century` for Interval literal

[GitBox <gi...@apache.org>]

alamb commented on code in PR #3038:
URL: https://github.com/apache/arrow-datafusion/pull/3038#discussion_r938745517


##########
datafusion/sql/src/interval.rs:
##########
@@ -64,17 +64,28 @@ pub(crate) fn parse_interval(leading_field: &str, value: &str) -> Result<ScalarV
             }
 
             match interval_type.to_lowercase().as_str() {
-                "year" => Ok(align_interval_parts(interval_period * 12_f32, 0.0, 0.0)),
-                "month" => Ok(align_interval_parts(interval_period, 0.0, 0.0)),
+                "century" | "centuries" => {
+                    Ok(align_interval_parts(interval_period * 1200_f32, 0.0, 0.0))

Review Comment:
   It might be nice in the future to name constants like `1200_f32` with some symbolic names
   
   like 
   
   ```
   let YEARS_PER_CENTRY: f32 = 1200_f32;
   ```
   
   But that is just a stylistic thing -- it isn't like the number of years in a century is going to be changed at any point in our lifetimes 🤣 



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org