[
https://issues.apache.org/jira/browse/CALCITE-3414?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16952470#comment-16952470
]
Feng Zhu edited comment on CALCITE-3414 at 10/16/19 3:09 AM:
-------------------------------------------------------------
Hi, [~julianhyde] , thanks for your clarification on the role of these two
functions.
Current _Types.castIfNecessary_ implementation has no constraint on what it
can do and consequently it "can" handle everything, even the result is wrong.
Hence, from my personal view, it is not suitable to call Types.castIfNecessary
in Calcite-core, because it involves types like BigDecimal/Date and etc.
The main approach of this PR is:
(1) copy some unique logic for special cases in Types.castIfNecessary into
RexToLixTranslator.convert, making it .
(2) unify the entrance of type cast/convert in Calcite-core as
RexToLixTranslator.convert.
was (Author: donnyzone):
Hi, [~julianhyde] , thanks for your clarification on the role of these two
functions.
Current _Types.castIfNecessary_ implementation has no constraint on what it can
do and consequently it can handle everything, even the result is wrong.
Hence, from my personal view, it is not suitable to call Types.castIfNecessary
in Calcite-core, because it involves types like BigDecimal/Date and etc.
The main approach of this PR is:
(1) copy some unique logic for special cases in Types.castIfNecessary into
RexToLixTranslator.convert, making it .
(2) unify the entrance of type cast/convert in Calcite-core as
RexToLixTranslator.convert.
> 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)
