[
https://issues.apache.org/jira/browse/CALCITE-5864?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17747667#comment-17747667
]
Julian Hyde commented on CALCITE-5864:
--------------------------------------
Your PR looks good, and helped me understand the problem. But it made me
realize that we missed opportunities for abstraction in CALCITE-5495. One
indication of this is that PR adds constants {{DAYS_IN_WEEK}} and
{{MONTHS_IN_QUARTER}}, when that information is already available in
{{TimeUnit.multiplier}}.
I think that {{evaluateIntervalLiteralAsYear}} could be generalized to serve
any time unit that is a multiple of months - YEAR or QUARTER - in the process
of removing common code between year and quarter you will remove this bug.
There is probably a similar method for multiples of seconds.
A SQL test would be more useful and maintainable than a test in
SqlToRelConverterTest. Add something like this to {{misc.iq}}:
{code}
with t (i) as (values 1, null, -2)
select i,
i * interval('1' year) as y,
i * interval('1' quarter) as q,
i * interval('1' quarter) as m
from t
order by i;
{code}
> getValueAs reports the wrong number of milliseconds for WEEK and the wrong
> number of months for QUARTER
> -------------------------------------------------------------------------------------------------------
>
> Key: CALCITE-5864
> URL: https://issues.apache.org/jira/browse/CALCITE-5864
> Project: Calcite
> Issue Type: Bug
> Components: core
> Reporter: Jonathan A Sternberg
> Priority: Major
> Labels: pull-request-available
>
> The change that introduced WEEK and QUARTER processing here did not correctly
> implement the `evaluateIntervalLiteralAsX` functions for these two time units.
> For both of them, it used the `fillIntervalValueArray` with the week
> parameter being used for months and the quarter parameter being used for
> months with no modifications to the underlying values.
> For weeks, this results in the field being misused as the hour parameter so 2
> weeks becomes 2 hours. For quarters, this results in the field being misused
> as months so 2 quarters becomes 2 months.
> I believe the proper way to implement these is to perform the modification so
> weeks gets translated to 7 days and quarters gets translated to 3 months.
> This seems to only affect the string versions such as `INTERVAL '2' WEEK`.
> The integer variants don't seem to use the literal pathway so they aren't
> affected.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
