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

Nesting LAST within PREV is not parsed correctly for MATCH_RECOGNIZE

    Details

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

      Description

      For table Ticker:

      symbol: string
      tstamp: long
      price: int
      

      , the UP in the following sql

      SELECT *
      FROM Ticker
      MATCH_RECOGNIZE (
        MEASURES
          STRT.tstamp AS start_tstamp,
          LAST(DOWN.tstamp) AS bottom_tstamp,
          LAST(UP.tstamp) AS end_tstamp
        ONE ROW PER MATCH
        PATTERN (STRT DOWN+ UP+)
        DEFINE
          DOWN AS DOWN.price < PREV(DOWN.price),
          UP AS UP.price > PREV(LAST(DOWN.price, 1), 1)
      ) AS T
      

      is parsed as follows:

      ">(PREV(UP.$2, 0), LAST(DOWN.$2, 1))"
      

      I think this is not as expected. According to the documentation about MATCH_RECOGNIZE, it supports nesting LAST/FIRST within PREV/NEXT and we should not just ignore the PREV.

        Issue Links

          Activity

          Hide
          julianhyde Julian Hyde added a comment -

          Zhiqiang He, can you take a look at this?

          Show
          julianhyde Julian Hyde added a comment - Zhiqiang He , can you take a look at this?
          Hide
          windpicker Angus Xiao added a comment -

          I've checked this issue, and found that it was the problem of match-recognize validation. It seems some work is unfinished here.

          Show
          windpicker Angus Xiao added a comment - I've checked this issue, and found that it was the problem of match-recognize validation. It seems some work is unfinished here .
          Hide
          ransom Zhiqiang He added a comment -

          Julian Hyde OK, I will test it.

          Show
          ransom Zhiqiang He added a comment - Julian Hyde OK, I will test it.
          Hide
          ransom Zhiqiang He added a comment -
          Show
          ransom Zhiqiang He added a comment - https://github.com/apache/calcite/pull/494 please review it, thanks.
          Hide
          julianhyde Julian Hyde added a comment -

          Reviewing and testing now.

          Show
          julianhyde Julian Hyde added a comment - Reviewing and testing now.
          Show
          julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/calcite/commit/65d95e67 .
          Hide
          dian.fu Dian Fu added a comment -

          Hi Zhiqiang He, just found that this patch disabled NavigationExpander and NavigationReplacer, will we add them back in the future?

          Show
          dian.fu Dian Fu added a comment - Hi Zhiqiang He , just found that this patch disabled NavigationExpander and NavigationReplacer , will we add them back in the future?
          Hide
          ransom Zhiqiang He added a comment -

          Dian Fu No, It would not add back in future. It was delete for calcite-1871.
          if you have some question about it, you can create a new issues. we can discuess in the new issues.

          Show
          ransom Zhiqiang He added a comment - Dian Fu No, It would not add back in future. It was delete for calcite-1871. if you have some question about it, you can create a new issues. we can discuess in the new issues.
          Hide
          dian.fu Dian Fu added a comment -

          Hi Zhiqiang He, thanks for your reply. I have created an issue calcite-1943 and we can discuss there.

          Show
          dian.fu Dian Fu added a comment - Hi Zhiqiang He , thanks for your reply. I have created an issue calcite-1943 and we can discuss there.
          Hide
          michaelmior Michael Mior added a comment -

          Resolved in release 1.14.0 (2017-10-01)

          Show
          michaelmior Michael Mior added a comment - Resolved in release 1.14.0 (2017-10-01)

            People

            • Assignee:
              ransom Zhiqiang He
              Reporter:
              dian.fu Dian Fu
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development