[CALCITE-3414] In calcite-core, use RexToLixTranslator.convert for type conversion code generation uniformly

[DonnyZone]

As illustrated in CALCITE-3414, we try to integrate Types.castIfNecessary's logic into RexToLixTranslator.cast.

[yanlin-Lynn]

can this ever happen for no toString method?

[DonnyZone]

Theoretically, a Class should contain toString method. However, there are some special cases in Calcite.
For example (QuidemTest#winagg.iq), if the operand is SqlFunctions.lesser(String.class, String.class), its type will be java.lang.Comparable, which has no toString method, leading to Exception in Expressions.call(operand, "toString").
Because for String.class, the method matched is:

  public static <T extends Comparable<T>> T lesser(T b0, T b1) {
    return b0 == null || b0.compareTo(b1) > 0 ? b1 : b0;
  }

[yanlin-Lynn]

OK, got that.

[danny0405]

Although the operand type class of SqlFunctions.lesser(String.class, String.class) does not implement toString method, the object should have implement the toString method, because the java Object already has that, did you mean the operand object is not a java Object ?

[DonnyZone]

Yes, the code ...lesser(..).toString will be ok in runtime. It just throws the exception during codegen in Expressions.call(operand, "toString")) through reflection. (String)lesser(...) skips this step.

[danny0405]

Do you think we can make an effort for Expressions.call(operand, "toString")) to get the right toString method ? Even though it uses the Java reflection.

[DonnyZone]

I investigate this problem, it may be complicated to implement this feature from beginning. However, relying on Guava's TypeResolver, we can easily infer a method's return type according to its argument types, as demonstrated below.
Is it necessary to open another JIRA for this feature?

public static Type inferRuntimeReturnType(final Method method,
      final List<Expression> arguments) {
    final Type returnType = method.getReturnType();
    final Type genericReturnType = method.getGenericReturnType();
    if (returnType == genericReturnType) {
      return returnType;
    }
    Type runtimeReturnType = genericReturnType;
    final Type[] parameterTypes = method.getParameterTypes();
    final Type[] genericParameterTypes = method.getGenericParameterTypes();
    for (int i = 0; i < parameterTypes.length; i++) {
      if (parameterTypes[i] != genericParameterTypes[i]) {
        final Type argType = arguments.get(i).getType();
        final TypeResolver resolver =
            new TypeResolver().where(genericParameterTypes[i], argType);
        runtimeReturnType = resolver.resolveType(runtimeReturnType);
      }
    }
    assert runtimeReturnType != genericReturnType;
    return runtimeReturnType;
  }

[danny0405]

I'm not sure how this code works, compared to this code, i'm more inclined with the try catch way, do we need to refine the specific exception instead of just Exception ?

[DonnyZone]

Ok, let me narrow it down to RuntimeException.

  public static MethodCallExpression call(Expression target, String methodName,
      Iterable<? extends Expression> arguments) {
    Method method;
    try {
      //noinspection unchecked
      method = Types.toClass(target.getType())
          .getMethod(methodName, Types.toClassArray(arguments));
    } catch (NoSuchMethodException e) {
      throw new RuntimeException("while resolving method '" + methodName
          + "' in class " + target.getType(), e);
    }
    return call(target, method, arguments);
  }

[danny0405]

Can we give more explain doc here, i understand this code is for the case Integer.valueOf((int) longVal). But i still need to go through the code for a while why this code snippet is needed.

[DonnyZone]

Sure, I will add doc for explanation and example here.

[danny0405]

Instead of try ... catch ... clause, can we use the java reflection to detect if the class has "toString" method first ?

[DonnyZone]

Do you have any advices on efficient code:)? I put some thought to this problem, but failed to get an elegant way, it seems that we should enumerate all methods to check their method names, argument types and return type.
Meanwhile, I found that in RexToLixTranslator, there is already a static function to find method in a class through reflection. But we still need to catch exception. Expressions.call also adopts the similar implementation.

  private static Method findMethod(
      Class<?> clazz, String name, Class... parameterTypes) {
    try {
      return clazz.getMethod(name, parameterTypes);
    } catch (NoSuchMethodException e) {
      throw new RuntimeException(e);
    }
  }

[danny0405]

Can we define a new method named needsCast(Type tpe1, Type tpe2) in Types ? Then Types.castIfNecessary and RexToLixTranslator.convert can both reuse this method. The method should include the content:

    if (fromType.equals(toType)) {
      return operand;
    }
    if (toType instanceof Types.RecordType) {
      // We can't extract Class from RecordType since mapping Java Class might not generated yet.
      return operand;
    }
    if (Types.isAssignableFrom(toType, fromType)) {
      return operand;
    }

[DonnyZone]

Good idea, I will put them into Types for reuse.