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

RexOptUtil does not support function table other than SqlStdOperatorTable

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 1.6.0
    • Fix Version/s: 1.9.0
    • Component/s: core
    • Labels:
      None

      Description

      RexOptUtil currently doesn't support function table other than SqlStdOperatorTable, which prevents third-party function implementations to reuse RexOptUtil.

        Activity

        Hide
        tedxu Ted Xu added a comment -

        Uploaded an initial patch to check operator SqlKind rather than compare attributes of SqlStdOperatorTable.

        Show
        tedxu Ted Xu added a comment - Uploaded an initial patch to check operator SqlKind rather than compare attributes of SqlStdOperatorTable.
        Hide
        julianhyde Julian Hyde added a comment -

        Ted Xu, I'm interested about your use case of using a different operator table. Are you intending to add some additional operators over and above the standard ones? For this, ChainedSqlOperatorTable is useful. You would still have the SqlStdOperatorTable underneath, so it would be the same instance of, say, the EQUALS operator.

        But if your use case is different, tell us what it is, let's figure out an official way to support it.

        Your patch looks basically good. And by the way, I just fixed CALCITE-1039, which provides SqlKind values for several more built-in operators. (It will be merged to master in the next day or two, but for now you can see it at https://github.com/julianhyde/calcite/commit/904c73da60b9f9deec61ea34d89ada3462381f93.)

        Have you found and fixed all places where we compare operators by identity, or just the ones that were causing you trouble?

        It would be really useful if your patch contained a test case. You could add a test to FrameworksTest that creates an operator table similar to the one in your project then calls

        FrameworkConfig config = Frameworks.newConfigBuilder().operatorTable(myTable).build();

        so that it gets used during parsing/validation/planning.

        I'll accept the patch when it has a test case and when you're fairly sure you've replaced all the operator identity comparisons you need.

        Show
        julianhyde Julian Hyde added a comment - Ted Xu , I'm interested about your use case of using a different operator table. Are you intending to add some additional operators over and above the standard ones? For this, ChainedSqlOperatorTable is useful. You would still have the SqlStdOperatorTable underneath, so it would be the same instance of, say, the EQUALS operator. But if your use case is different, tell us what it is, let's figure out an official way to support it. Your patch looks basically good. And by the way, I just fixed CALCITE-1039 , which provides SqlKind values for several more built-in operators. (It will be merged to master in the next day or two, but for now you can see it at https://github.com/julianhyde/calcite/commit/904c73da60b9f9deec61ea34d89ada3462381f93 .) Have you found and fixed all places where we compare operators by identity, or just the ones that were causing you trouble? It would be really useful if your patch contained a test case. You could add a test to FrameworksTest that creates an operator table similar to the one in your project then calls FrameworkConfig config = Frameworks.newConfigBuilder().operatorTable(myTable).build(); so that it gets used during parsing/validation/planning. I'll accept the patch when it has a test case and when you're fairly sure you've replaced all the operator identity comparisons you need.
        Hide
        julianhyde Julian Hyde added a comment -
        Show
        julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/calcite/commit/5c74b6b4 (thanks MinJi Kim !) and also in part by the fix for CALCITE-1200 .
        Hide
        jcamachorodriguez Jesus Camacho Rodriguez added a comment -

        Resolved in release 1.9.0 (2016-09-22)

        Show
        jcamachorodriguez Jesus Camacho Rodriguez added a comment - Resolved in release 1.9.0 (2016-09-22)

          People

          • Assignee:
            julianhyde Julian Hyde
            Reporter:
            tedxu Ted Xu
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development