[ https://issues.apache.org/jira/browse/CALCITE-3414?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16952595#comment-16952595 ]
Danny Chen commented on CALCITE-3414: ------------------------------------- For (1) i think we can make a fix, for (2) could we make another method for the unique logic in linq4j then Types.castIfNecessary and RexToLixTranslator.convert can both reuse them ? > 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)