You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/08/04 18:15:22 UTC

[GitHub] [arrow-datafusion] ovr opened a new pull request, #3038: feat: Support week, decade, century for Interval literal

ovr opened a new pull request, #3038:
URL: https://github.com/apache/arrow-datafusion/pull/3038

   # Which issue does this PR close?
   
   A lot of BIs uses `+ INTERVAL '1 week'`. This PR introduces support for parsing new types for intervals: `week`, `decades` and `century`. (last two were added to be similar with Postgres behaviour)
   
   # Are there any user-facing changes?
   
   New functionality without breaking changes.


-- 
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


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

Posted by 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


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

Posted by GitBox <gi...@apache.org>.
andygrove commented on code in PR #3038:
URL: https://github.com/apache/arrow-datafusion/pull/3038#discussion_r938187722


##########
datafusion/core/tests/sql/expr.rs:
##########
@@ -769,6 +777,18 @@ async fn test_interval_expressions() -> Result<()> {
         "interval '1 year'",
         "1 years 0 mons 0 days 0 hours 0 mins 0.00 secs"
     );
+    test_expression!(
+        "interval '1 decade'",
+        "10 years 0 mons 0 days 0 hours 0 mins 0.00 secs"
+    );
+    test_expression!(
+        "interval '1 decade'",
+        "10 years 0 mons 0 days 0 hours 0 mins 0.00 secs"
+    );

Review Comment:
   nit: this is a duplicate of the previous test



-- 
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


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

Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #3038:
URL: https://github.com/apache/arrow-datafusion/pull/3038#issuecomment-1206376746

   Benchmark runs are scheduled for baseline = 07b7314d8769da3567998b5fa3385c04a645a80d and contender = 581934d73dfca7b99e6cc66767b3af3bbad7755f. 581934d73dfca7b99e6cc66767b3af3bbad7755f is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/e4497bf40507475bae7ce4664a647fce...c20a38a33061413b98cd84ce6921ab7e/)
   [Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] [test-mac-arm](https://conbench.ursa.dev/compare/runs/7b074880c7184140ba76dd1568d4a3a6...b3323a029eef4f068bee88dc23396044/)
   [Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/bee86df4353c47d18011251df0c6741a...5d09b95bcec743e2a1b1ef1f9bd99686/)
   [Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/3d78991e83594d9dae11a472626e59d8...4204e99761574c218b690084c29b18fe/)
   Buildkite builds:
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
   test-mac-arm: Supported benchmark langs: C++, Python, R
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
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


[GitHub] [arrow-datafusion] alamb merged pull request #3038: feat: Support `week`, `decade`, `century` for Interval literal

Posted by GitBox <gi...@apache.org>.
alamb merged PR #3038:
URL: https://github.com/apache/arrow-datafusion/pull/3038


-- 
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