Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.15.0
    • Component/s: None
    • Labels:

      Activity

      Hide
      julianhyde Julian Hyde added a comment -

      Resolved in release 1.15.0 (2017-12-11).

      Show
      julianhyde Julian Hyde added a comment - Resolved in release 1.15.0 (2017-12-11).
      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/3e97cff7 ; thanks for the PR, Dian Fu !
      Hide
      dian.fu Dian Fu added a comment -

      Julian Hyde I have left a few minor comments on your branch. The changes LGTM after these minor issues fixed.

      Show
      dian.fu Dian Fu added a comment - Julian Hyde I have left a few minor comments on your branch. The changes LGTM after these minor issues fixed.
      Hide
      julianhyde Julian Hyde added a comment -

      I have sketched out RelBuilder.patternConcat etc. in https://github.com/julianhyde/calcite/commit/e9be34774eb9078f859313b8a43afb3326fb9211. Please revise if we can make them easier to use.

      Show
      julianhyde Julian Hyde added a comment - I have sketched out RelBuilder.patternConcat etc. in https://github.com/julianhyde/calcite/commit/e9be34774eb9078f859313b8a43afb3326fb9211 . Please revise if we can make them easier to use.
      Hide
      julianhyde Julian Hyde added a comment -

      I will also change the `List<RexNode>` parameters to `Iterable<? extends RexNode>`, consistent with other methods in RelBuilder, and add documentation to http://calcite.apache.org/docs/algebra.html#relational-operators.

      Do we need methods RelBuilder.patternConcat, .patternAlternate, .patternQuantify? The test code to invoke these is quite verbose right now.

      Show
      julianhyde Julian Hyde added a comment - I will also change the `List<RexNode>` parameters to `Iterable<? extends RexNode>`, consistent with other methods in RelBuilder, and add documentation to http://calcite.apache.org/docs/algebra.html#relational-operators . Do we need methods RelBuilder.patternConcat, .patternAlternate, .patternQuantify? The test code to invoke these is quite verbose right now.
      Hide
      julianhyde Julian Hyde added a comment -

      I would rename the method RelBuilder.matchRecognize to match (since we're using the relational operator name, not the overly-verbose SQL keyword name). Otherwise, looks good; glad to see there was good javadoc plus a test. If there are no objections to renaming the method I will commit after 1.14 is released.

      Show
      julianhyde Julian Hyde added a comment - I would rename the method RelBuilder.matchRecognize to match (since we're using the relational operator name, not the overly-verbose SQL keyword name). Otherwise, looks good; glad to see there was good javadoc plus a test. If there are no objections to renaming the method I will commit after 1.14 is released.
      Hide
      ransom Zhiqiang He added a comment -

      It's OK. I'm already review it. I think this can be commit after Julian Hyde review.

      Show
      ransom Zhiqiang He added a comment - It's OK. I'm already review it. I think this can be commit after Julian Hyde review.
      Hide
      dian.fu Dian Fu added a comment -

      when do we need to create a matchrecoginize relNode used RelBuilder?

      This requirement comes from the cep support in Flink table API. After we have defined the match_recognize clauses such as define, measures, pattern using Flink table API, we need to a helper method to construct a matchRecognize RelNode from these RexNodes.
      Besides, I think this will also be helpful when we create optimization rules for match_recognize in the future.

      SqlToRel already have matchRecognize convert

      SqlToRelConverter.convertMatchRecognize is only used to convert SqlNode to RelNode. While with RelBuilder.matchRecognize we can construct RelNode from the RexNodes.

      Thoughts? Zhiqiang He

      Show
      dian.fu Dian Fu added a comment - when do we need to create a matchrecoginize relNode used RelBuilder? This requirement comes from the cep support in Flink table API. After we have defined the match_recognize clauses such as define , measures , pattern using Flink table API, we need to a helper method to construct a matchRecognize RelNode from these RexNodes . Besides, I think this will also be helpful when we create optimization rules for match_recognize in the future. SqlToRel already have matchRecognize convert SqlToRelConverter.convertMatchRecognize is only used to convert SqlNode to RelNode . While with RelBuilder.matchRecognize we can construct RelNode from the RexNodes . Thoughts? Zhiqiang He
      Hide
      ransom Zhiqiang He added a comment -

      Dian Fu your test case is only create. when do we need to create a matchrecoginize relNode used RelBuilder? SqlToRel already have matchRecognize convert ,
      is it for optimizer?

      Show
      ransom Zhiqiang He added a comment - Dian Fu your test case is only create. when do we need to create a matchrecoginize relNode used RelBuilder? SqlToRel already have matchRecognize convert , is it for optimizer?
      Hide
      dian.fu Dian Fu added a comment -

      Julian Hyde Could you help to review when it's convenient for you, thanks a lot.
      https://github.com/apache/calcite/pull/538

      Show
      dian.fu Dian Fu added a comment - Julian Hyde Could you help to review when it's convenient for you, thanks a lot. https://github.com/apache/calcite/pull/538

        People

        • Assignee:
          julianhyde Julian Hyde
          Reporter:
          dian.fu Dian Fu
        • Votes:
          0 Vote for this issue
          Watchers:
          3 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development