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

Feng Zhu commented on CALCITE-3414:
-----------------------------------

[~danny0405], there are two reasons:

(1)some implementation in Types.castIfNecessary is not correct;

(2)we just need to copy a small part of unique logics, mainly 
{color:#FF0000}_returnType instanceof RecordType_{color} and{color:#FF0000} 
_Types.isAssignableFrom(returnType, type)_{color}, For other parts, 
RexToLixTranslator is more powerful.

> Unify Expression'type cast and conversion as a robust one
> ---------------------------------------------------------
>
>                 Key: CALCITE-3414
>                 URL: https://issues.apache.org/jira/browse/CALCITE-3414
>             Project: Calcite
>          Issue Type: Bug
>          Components: core
>    Affects Versions: 1.21.0
>            Reporter: Feng Zhu
>            Assignee: Feng Zhu
>            Priority: Major
>              Labels: pull-request-available
>         Attachments: RexToLixTranslator.png, TypeConversion.txt, Types.png
>
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
>  Current now, there are two functions in calcite that can be used to 
> cast/convert Expression to a specific Type.
>  *_Types.castIfNecessary_* and _*RexToLixTranslator.convert*_.
> We make a deep investigation on their implementations and demonstrate them as 
> below.
> {color:#ff0000}                                                               
>       !RexToLixTranslator.png!{color}
> {color:#ff0000}                                                               
>        *RexToLixTranslator.convert*{color}
>   !Types.png!
>                                            {color:#ff0000}                    
>        *Types.castIfNecessary*{color}
> It can be seen that: 
>  (1) They have a lot of overlaps; 
>  (2) *_RexToLixTranslator.cast_* can cover more cases with tools like 
> _SqlFunctions_ and etc.
>  (3) Both of them have limitations and may generate incorrect code, which is 
> listed in attachment(TypeConversion.txt).
> Multiple choices usually bring confusion to developers and resulting to the 
> misuse of them. 
>  For example, CALCITE-3245 exposes that Types.castIfNecessary cannot cast the 
> Expression to BigDecimal.class.
>  Fixing the issue in *_Types.castIfNecessary_* directly seems to be not a 
> good idea. 
>  On one hand, it is not convenient to call _SqlFunctions_ in linq4j. One the 
> other hand, it will brings duplicate with _*RexToLixTranslator.cast*_. 
> However, due to some unique logic in _*Types.castIfNecessary*_, we cannot 
> replace it as _*RexToLixTranslator.cast*_ neither.
> Therefore, it is a good idea to integrate implementations into 
> RexToLixTranslator.cast.



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

Reply via email to