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

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

(1)According to the figures posted above, there are 19 approaches in 
RexToLixTranslator while Types.castIfNecessary only contains 4 approaches.

(2)It may be trivial to make a common method. To summarize, we only need to add 
three places, two precondition checks below, and (*_xxx->String.class_*, choose 
{color:#FF0000}_xxx.toString_{color} _or_ {color:#FF0000}_(String)xxx_{color})
{code:java}
if (toType instanceof Types.RecordType) {
   return operand;
 }
 if (Types.isAssignableFrom(toType, fromType)) {
   return operand;
 }{code}

> 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