[ 
https://issues.apache.org/jira/browse/CALCITE-5919?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17753443#comment-17753443
 ] 

Mihai Budiu commented on CALCITE-5919:
--------------------------------------

[~julianhyde]

The problem is that there is a lot of code already in Calcite which assumes 
that time values are represented as int values encoding milliseconds since 
midnight. This choice is made in JavaTypeFactoryImpl, and it looks to me like 
both compile-time optimizations and the runtime depend on this choice. 

Here is an example from SqlFunctions:

{code:java}
/** SQL {@code DATETIME(DATE, TIME)} function; returns a Calcite TIMESTAMP. */
  public static long datetime(int daysSinceEpoch, int millisSinceMidnight) {
    // BigQuery's DATETIME function returns a Calcite TIMESTAMP,
    // represented internally as milliseconds since epoch UTC.
    return daysSinceEpoch * DateTimeUtils.MILLIS_PER_DAY + millisSinceMidnight;
  }
{code}

This function assumes that a time value is represented as an integer which 
contains the millisSinceMidnight. There may be hundreds such locations in the 
code. 

Even more insidious, there is code in RexImpTable 
ExtractImplementor.implementSafe which looks like this: 
{code:java}
case MILLISECOND:
      case MICROSECOND:
      case NANOSECOND:
        if (sqlTypeName == SqlTypeName.DATE) {
          return Expressions.constant(0L);
        }
        operand = mod(operand, TimeUnit.MINUTE.multiplier.longValue(), 
!isIntervalType);
        return Expressions.multiply(
            operand, Expressions.constant((long) (1 / 
unit.multiplier.doubleValue())));
{code}

This code implicitly assumes that time values can be arguments to functions 
like "mod" and "multiply", which is not true for BigDecimal values.

This is a failure of abstraction: the fact that TIME values are represented as 
'int' values in the compiler has leaked in many places.

The problem would be slightly more tractable if the compile-time and run-time 
representations were independent, as you suggest, but I am not sure that is the 
case.

I may make an effort to see if this particular change is tractable, but if it's 
too much work I will give up. 
The only alternative solution I can foresee is to disallow precision >= 3 for 
time values, because otherwise some optimizations produce wrong results.



(There is at least one other bug here, it shouldn't do a floating point 
division when it could do a precise division, but forget about that.)


> Compile-time implementation of EXTRACT ignores sub-millisecond values
> ---------------------------------------------------------------------
>
>                 Key: CALCITE-5919
>                 URL: https://issues.apache.org/jira/browse/CALCITE-5919
>             Project: Calcite
>          Issue Type: Bug
>          Components: core
>    Affects Versions: 1.35.0
>            Reporter: Mihai Budiu
>            Priority: Minor
>
> When enabling the optimization rule PROJECT_REDUCE_EXPRESSIONS the 
> compile-time evaluation of an expression like EXTRACT(MICROSECONDS FROM TIME 
> '13:30:25.575401') will produce a result up to milliseconds only, 
> irrespective of the type system provided. (This query will evaluate to 
> 25575000 instead of 25575401).
> The bug is in RexImpTable.ExtractImplementor, here:
> {code:java}
> case MILLISECOND:
>       case MICROSECOND:
>       case NANOSECOND:
>         if (sqlTypeName == SqlTypeName.DATE) {
>           return Expressions.constant(0L);
>         }
>         operand = mod(operand, TimeUnit.MINUTE.multiplier.longValue(), 
> !isIntervalType); // << BUG
>         return Expressions.multiply(
>             operand, Expressions.constant((long) (1 / 
> unit.multiplier.doubleValue())));
> {code}
> The mod operation uses a multiplier for MINUTE that is expressed in 
> milliseconds, so it always truncates away values below milliseconds. The 
> multiplier seems to assume that the type system precision for TIME is set to 
> 3, which is the default value, but may be overridden.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to