-
Notifications
You must be signed in to change notification settings - Fork 63
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
Adds support for the interval type #1741
base: main
Are you sure you want to change the base?
Conversation
cc667bf
to
fb00a50
Compare
@@ -51,4 +55,89 @@ class OpArithmeticTest : PartiQLTyperTestBase() { | |||
|
|||
return super.testGenPType("arithmetic", tests, argsMap) | |||
} | |||
|
|||
@TestFactory | |||
fun plus(): Stream<DynamicContainer> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Self-review: I copied the arithmetic
test from above to split out the plus/minus planner tests, since we can now add intervals to datetimes and vice-versa.
I would rather see end-to-end tests for this, but I digress.
fb00a50
to
1222709
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1741 +/- ##
=========================================
Coverage 80.03% 80.03%
Complexity 47 47
=========================================
Files 19 19
Lines 506 506
Branches 23 23
=========================================
Hits 405 405
Misses 88 88
Partials 13 13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
a0a2c8a
to
0cfd212
Compare
Adds several functions pertaining to INTERVAL Adds coercions to/from INTERVAL
0cfd212
to
e451ef9
Compare
Relevant Issues
Overview
SQL
If you are reviewing this PR, I highly recommend that you read:
a. This gives insight into why we use
INTERVAL_YM
andINTERVAL_DT
.a. This gives insight into the coercion of intervals
PType Design
This PR adds two new PTypes,
INTERVAL_YM
andINTERVAL_DT
, to represent the year-month intervals and day-time intervals, respectively. We have taken this approach due to practical implementation reasons. The main reason is that all of the existing cast/coercion logic is done via the PType codes -- and, year-month intervals are mutually assignable and comparable to other year-month intervals. Same goes for day-time intervals.We have added
getIntervalCode()
. This is mostly how PostgreSQL does it.We have also added
getFractionalPrecision()
. Adding a new method to the interface does NOT slow down the implementation. We could have usedgetScale()
, however, this might have led to confusion.We have opted to use
getPrecision()
for getting the leading field precision. Though, if there is pushback, we can create a new API more similarly named to how it is called in the SQL Spec. MaybegetLeadingFieldPrecision()
. It doesn't matter to me. I've documented howgetPrecision()
should be used when the type's code isINTERVAL_YM
orINTERVAL_DT
.Datum Design
SQL describes intervals as a contiguous subset of the allowable interval types. For year-month intervals, it can be years or months (or a contiguous subset). For day-times, it can be days, hours, minutes, and seconds (or a contiguous subset). Therefore, year-month intervals may call
getYears()
andgetMonths()
. For day-time intervals, you may callgetDays()
,getHours()
,getMinutes()
,getSeconds()
, andgetNanos()
. This way, we don't have Java'sDuration
orPeriod
in our public API. We only deal with primitives, and if we really want to use some of the standard Java library's logic, we can under-the-hood convert them. By only dealing with primitives, we set up our APIs for future optimization without sacrificing any of Java's provided standard library logic. These are not mutually exclusive.I only wish we took this approach for
TIME
,TIMEZ
,TIMESTAMP
, andTIMESTAMPZ
. If we want to do this, I don't think it'd be too difficult.Operations Added
This PR has added the following operations involving intervals:
For the above, there are many permutations of the datetimes and intervals for each of these operations (consider DATE, TIME, TIMEZ, TIMESTAMP, TIMESTAMPZ operating with INTERVAL YEAR/MONTH/YEAR_MONTH/DAY/etc)
Other operations that SQL specifies and are not implemented by this PR:
If these are considered critical, I will add them to the PR. Or, if I find time before this is fully reviewed, I will add them.
License Information
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.