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

Zuozhi Wang edited comment on CALCITE-4713 at 8/3/21, 1:47 AM:
---------------------------------------------------------------

[~julianhyde] Thanks for the reply. As you said, indeed things will break if 
this behavior is changed. We've tried different fixes and they break some test 
cases. I assume some existing logic already rely on this particular behavior.

To provide a context on this problem, we are working on integrating the Tempura 
incremental query optimizer 
[CALCITE-4569|https://issues.apache.org/jira/browse/CALCITE-4568] into a newer 
version of Calcite. 
We wrote some test cases that uses the existing `CalciteAssert.HR` schema, 
which is a reflective schema based on Java objects. 

There is a rule about merging a snapshot and a delta that does this: 
{code}
snapshot(t1) UNION delta(t1-t2) = snapshot(t2).
{code}

The union operator's `deriveRowType` function calls `leastRestrictive`. If the 
table has a `JavaType(String)` column, then union's row type becomes `VARCHAR`, 
which causes type assertion to fail.

Ultimately we want to add some test cases to show how Tempura works. 
Any help on how to workaround this issue without breaking changes are much 
appreciated. 
We are also considering to only use SQL type system in the test cases to avoid 
potential issues. Any pointers on how to use SQL types in test cases are also 
very helpful. 





was (Author: zuozhiw):
[~julianhyde] Thanks for the reply. As you said, indeed things will break if 
this behavior is changed. We've tried different fixes and they break some test 
cases. I assume some existing logic already rely on this particular behavior.

To provide a context on this problem, we are working on integrating the Tempura 
incremental query optimizer 
[CALCITE-4569|https://issues.apache.org/jira/browse/CALCITE-4568] into a newer 
version of Calcite. 
We wrote some test cases that uses the existing `CalciteAssert.HR` schema, 
which is a reflective schema based on Java objects. 

There is a rule about merging a snapshot and a delta that does this: 
{code}
snapshot(t1) UNION delta(t1-t2) = snapshot(t2).
{code}

The union operator's `deriveRowType` function calls `leastRestrictive`. If the 
table has a `JavaType(String)` column, then union's row type becomes `VARCHAR`, 
which causes the type assertion to fail.

Ultimately we want to add some test cases to show how Tempura works. 
Any help on how to workaround this issue without breaking changes are much 
appreciated. 
We are also considering to only use SQL type system in the test cases to avoid 
potential issues. Any pointers on how to use SQL types in test cases are also 
very helpful. 




> JavaTypeFactory#leastRestrictive returns wrong type for JavaType(String)
> ------------------------------------------------------------------------
>
>                 Key: CALCITE-4713
>                 URL: https://issues.apache.org/jira/browse/CALCITE-4713
>             Project: Calcite
>          Issue Type: Bug
>          Components: core
>    Affects Versions: 1.27.0
>            Reporter: Zuozhi Wang
>            Priority: Minor
>
> When using JavaTypeFactoryImpl, calling `leastRestrictive` with two 
> `JavaType(String)` types gives a wrong return type of `VARCHAR` (SqlType). 
> The expected return type should still be `JavaType(String)` 
> See the following test case:
> {code:java}
>   @Test void testLeastRestrictiveWithWithJavaString() {
>     JavaTypeFactoryImpl javaTypeFactory = new JavaTypeFactoryImpl();
>     RelDataType javaStringType = javaTypeFactory.createJavaType(String.class);
>     // leastRestrictive of two same JavaType(String) types
>     RelDataType leastRestrictive = javaTypeFactory.leastRestrictive(
>         Arrays.asList(javaStringType, javaStringType));
>     // expect leastRestrictive to also be JavaType(String)
>     // but actual value is VARCHAR (BasicSqlType)
>     assertEquals(javaStringType, leastRestrictive);
>   }
> {code}
> We are currently investigating a fix of this issue.
> JavaTypeFactoryImpl extends SqlTypeFactoryImpl, and the `leastRestrictive` is 
> not overridden. In `SqlTypeFactoryImpl`, `leastRestrictive` will first try to 
> treat the input types as sql types, and call `leastRestrictiveSqlType`, 
> therefore `JavaType(String)` is treated as SqlType `VARCHAR`.
> One possible fix of this issue is to override the `leastRestrictive` function 
> in `JavaTypeFactoryImpl` to call `leastRestrictiveByCast` in 
> SqlTypeFactoryImpl instead. 
> {code:java}
>   // in JavaTypeFactoryImpl 
>   // also need to change SqlTypeFactoryImpl#leastRestrictiveByCast from 
> private to protected
>   @Override
>   public @Nullable RelDataType leastRestrictive(List<RelDataType> types) {
>     return super.leastRestrictiveByCast(types);
>   }
> {code}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to