Uploaded image for project: 'Calcite'
  1. Calcite
  2. CALCITE-3414

In calcite-core, use RexToLixTranslator.convert for type conversion code generation uniformly

VotersWatch issueWatchersLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    Description

       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.

                                                                          

                                                                            RexToLixTranslator.convert

       

                                                                            Types.castIfNecessary

      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.

      Attachments

        1. RexToLixTranslator.png
          158 kB
          Feng Zhu
        2. TypeConversion.txt
          108 kB
          Feng Zhu
        3. Types.png
          132 kB
          Feng Zhu

        Issue Links

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            donnyzone Feng Zhu
            donnyzone Feng Zhu
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Time Tracking

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0h
                0h
                Logged:
                Time Spent - 3h
                3h

                Slack

                  Issue deployment