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

Add Interval suppport #1752

Merged
merged 4 commits into from
Sep 10, 2021
Merged

Add Interval suppport #1752

merged 4 commits into from
Sep 10, 2021

Conversation

junli1026
Copy link
Contributor

@junli1026 junli1026 commented Sep 7, 2021

I hereby agree to the terms of the CLA available at: https://datafuse.rs/policies/cla/

Summary

Add toInterval support. The usage can be like "select interval '1' day". with the pattern of "Interval '[num]' [unit]".

Changelog

  • New Feature

Related Issues

Fixes #issue

Test Plan

Unit Tests

Manual tested with mysql and clickhouse client. Test sql includes:
"select interval '1' year"
"select now() + interval '1' year" // this will hit to arithmetic, which is not implemented yet.

@databend-bot
Copy link
Member

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

@databend-bot databend-bot added the pr-feature this PR introduces a new feature to the codebase label Sep 7, 2021
@codecov-commenter
Copy link

codecov-commenter commented Sep 7, 2021

Codecov Report

Merging #1752 (94fb8e3) into master (c41fcfa) will decrease coverage by 0%.
The diff coverage is 56%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1752   +/-   ##
======================================
- Coverage      72%     72%   -1%     
======================================
  Files         610     610           
  Lines       36452   36549   +97     
======================================
+ Hits        26357   26410   +53     
- Misses      10095   10139   +44     
Impacted Files Coverage Δ
common/datavalues/src/data_value.rs 44% <0%> (-1%) ⬇️
common/datavalues/src/types/physical_data_type.rs 81% <0%> (-2%) ⬇️
common/datavalues/src/types/serializations/mod.rs 33% <0%> (-4%) ⬇️
common/datavalues/src/types/data_type_coercion.rs 60% <17%> (-4%) ⬇️
common/datavalues/src/types/data_type.rs 59% <37%> (-7%) ⬇️
...on/functions/src/scalars/arithmetics/arithmetic.rs 68% <66%> (-1%) ⬇️
query/src/sql/plan_parser.rs 75% <88%> (+<1%) ⬆️
query/src/sql/plan_parser_test.rs 97% <100%> (+<1%) ⬆️
...pelines/transforms/transform_aggregator_partial.rs 89% <0%> (-2%) ⬇️
store/src/meta_service/network.rs 73% <0%> (-2%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c41fcfa...94fb8e3. Read the comment docs.

@jorgecarleitao
Copy link

some extra info:

In arrow there are 3 interval formats, all of which arrow2 supports: Interval in months (i32), interval in (days,milliseconds) ([i32,i32]) and interval in (month,day,nanoseconds) [i32,i32,i64]. These are defined because math in time-aware timezones is often not sufficient in making everything seconds, because a day is not necessarily 24h*3600 sec/h due to leap seconds and others.

There is also the "duration" with 4 units (sec, milli, micro, nano) stored as i64 which is not time-aware.

@junli1026
Copy link
Contributor Author

some extra info:

In arrow there are 3 interval formats, all of which arrow2 supports: Interval in months (i32), interval in (days,milliseconds) ([i32,i32]) and interval in (month,day,nanoseconds) [i32,i32,i64]. These are defined because math in time-aware timezones is often not sufficient in making everything seconds, because a day is not necessarily 24h*3600 sec/h due to leap seconds and others.

There is also the "duration" with 4 units (sec, milli, micro, nano) stored as i64 which is not time-aware.

Thanks Jorge, is the Interval(Year) in arrows based on real life calander ? For example the one @sundy-li mentioned:

"03-31 interval -1 month ===> 02-29 rather than 03-31 sub 30 days." 

@jorgecarleitao
Copy link

Thanks Jorge, is the Interval(Year) in arrows based on real life calander ? For example the one @sundy-li mentioned:

"03-31 interval -1 month ===> 02-29 rather than 03-31 sub 30 days." 

Exactly, Interval(YearMonth) is an i32 representing months. Arrow2 will support ops over intervals; we recently added support to read time-aware timestamps, the next step is the compute :)

@junli1026 junli1026 changed the title Add toInterval suppport Add Interval suppport Sep 10, 2021
Copy link
Member

@sundy-li sundy-li left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, we can add arithmetic ops for the interval type in another pr.

@databend-bot
Copy link
Member

CI Passed
Reviewer Approved
Let's Merge

@databend-bot databend-bot merged commit f9aab56 into datafuselabs:master Sep 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-feature this PR introduces a new feature to the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants