[BSE-524] Support Quarter Interval #137
Conversation
|
This is likely the wrong way to implement this. We should not add an additional type. If you notice, in order to implement this feature, we would have to modify 19 files in the fork which is not going to be maintainable. I see that You can very likely just simplify this entire thing by using |
| case QUARTER: | ||
| return evaluateIntervalLiteralAsQuarter(typeSystem, sign, value, value0, | ||
| pos); |
There was a problem hiding this comment.
Notice how this section, which is the only one that was required to implement this, uses TimeUnit.QUARTER and does not use INTERVAL_QUARTER at all.
| case "qtr": | ||
| case "qtrs": | ||
| case "quarters": | ||
| unit = TimeUnit.QUARTER; |
There was a problem hiding this comment.
Here we see TimeUnit.QUARTER being used.
| if (intervalQualifier.timeUnitRange.toString().equals("QUARTER")) { | ||
| long monthsInQuarter = 3; | ||
| return ret[0] * ret[2] * monthsInQuarter; | ||
| } |
There was a problem hiding this comment.
Do not use toString() to perform comparisons. You already have an enum. Just compare the enum. TimeUnitRange.QUARTER exists.
There was a problem hiding this comment.
Looked into this further and this entire area isn't needed. Since one quarter is 3 months, just perform that translation before calling fillIntervalValueArray.
| INTERVAL_QUARTER(PrecScale.NO_NO, false, Types.OTHER, | ||
| SqlTypeFamily.INTERVAL_YEAR_MONTH), |
There was a problem hiding this comment.
Just use INTERVAL_MONTH. There's no meaningful difference between how these are used.
Support
INTERVAL_QUARTER