Repository: tobymao/sqlglot

Issue:
bug in match recognize implementation
I'm using sqlglot to parse some oracle SQL and noticed a parseError is raised with some sample code from Oracle docs.

source: https://oracle-base.com/articles/12c/pattern-matching-in-oracle-database-12cr1
```
SELECT *
FROM   sales_history MATCH_RECOGNIZE (
         PARTITION BY product
         ORDER BY tstamp
         MEASURES  STRT.tstamp AS start_tstamp,
                   LAST(UP.tstamp) AS peak_tstamp,
                   LAST(DOWN.tstamp) AS end_tstamp,
                   MATCH_NUMBER() AS mno
         ONE ROW PER MATCH
         AFTER MATCH SKIP TO LAST DOWN
         PATTERN (STRT UP+ FLAT* DOWN+)
         DEFINE
           UP AS UP.units_sold > PREV(UP.units_sold),
           FLAT AS FLAT.units_sold = PREV(FLAT.units_sold),
           DOWN AS DOWN.units_sold < PREV(DOWN.units_sold)
       ) MR
```

on parsing this statement i get 
```
Exception has occurred: ParseError
Expecting ). Line 5, Col: 47.
          PARTITION BY product
         ORDER BY tstamp
         MEASURES  STRT.tstamp AS start_tstamp[4m,[0m
                   LAST(UP.tstamp) AS peak_tstamp,
                   LAST(DOWN.tstamp) AS end_tsta
  File "C:\gitRepos\parse\main.py", line 12, in <module>
    stmnt = parser.parse(sql=sql)
sqlglot.errors.ParseError: Expecting ). Line 5, Col: 47.
          PARTITION BY product
         ORDER BY tstamp
         MEASURES  STRT.tstamp AS start_tstamp[4m,[0m
                   LAST(UP.tstamp) AS peak_tstamp,
                   LAST(DOWN.tstamp) AS end_tsta
```

after some digging in the codebase i figured there is an implementation error in both parsing MEASURES and DEFINE,
where they both dont take into account several measures or definitions.

the fix for measures seems simple enough:
```
def _parse_match_recognize(self) -> t.Optional[exp.Expression]:
    if not self._match(TokenType.MATCH_RECOGNIZE):
        return None
    self._match_l_paren()

    partition = self._parse_partition_by()
    order = self._parse_order()
    measures = (
        #self._parse_alias(self._parse_conjunction())
        self._parse_csv(self._parse_expression)
        if self._match_text_seq("MEASURES")
        else None
    )...
```

measures parses successfully (while assuming UP, DOWN and STRT are tables which might be a misnomer? maybe a new symbol token should be introduced?)
but now define has kind of the same bug:
```
  File "C:\gitRepos\parse\main.py", line 12, in <module>
    stmnt = parser.parse(sql=sql)
sqlglot.errors.ParseError: Expecting ). Line 13, Col: 20.
   MATCH SKIP TO LAST DOWN
         PATTERN (STRT UP+ FLAT* DOWN+)
         DEFINE
           UP AS UP[4m.[0munits_sold > PREV(UP.units_sold),
           FLAT AS FLAT.units_sold = PREV(FLAT.units_sold),
```

but for the DEFINE clause, self._parse_expression wont cut it because the symbol(alias) comes before the expression,
so i guess a new function should be defined.



Discussion:
GeorgeSittas: Simplified the implementation by moving the alias reversing logic inside of `matchrecognize_sql`. I also refactored `Generator.expressions`, so that it can also work with lists of strings instead of a single expression. This has the benefit of allowing us to reuse the prettifying logic for lists of sqls we construct manually, like in this example.

Patch:
diff --git a/sqlglot/dialects/oracle.py b/sqlglot/dialects/oracle.py
index c9533af7..1bbe6b96 100644
--- a/sqlglot/dialects/oracle.py
+++ b/sqlglot/dialects/oracle.py
@@ -152,9 +152,9 @@ class Oracle(Dialect):
 
         def xmltable_sql(self, expression: exp.XMLTable) -> str:
             this = self.sql(expression, "this")
-            passing = self.expressions(expression, "passing")
+            passing = self.expressions(expression, key="passing")
             passing = f"{self.sep()}PASSING{self.seg(passing)}" if passing else ""
-            columns = self.expressions(expression, "columns")
+            columns = self.expressions(expression, key="columns")
             columns = f"{self.sep()}COLUMNS{self.seg(columns)}" if columns else ""
             by_ref = (
                 f"{self.sep()}RETURNING SEQUENCE BY REF" if expression.args.get("by_ref") else ""
diff --git a/sqlglot/expressions.py b/sqlglot/expressions.py
index 14b082e9..2071e1e8 100644
--- a/sqlglot/expressions.py
+++ b/sqlglot/expressions.py
@@ -1476,6 +1476,7 @@ class MatchRecognize(Expression):
         "after": False,
         "pattern": False,
         "define": False,
+        "alias": False,
     }
 
 
@@ -3167,7 +3168,6 @@ class Neg(Unary):
     pass
 
 
-# Special Functions
 class Alias(Expression):
     arg_types = {"this": True, "alias": False}
 
diff --git a/sqlglot/generator.py b/sqlglot/generator.py
index b2b2182f..3cbbee55 100644
--- a/sqlglot/generator.py
+++ b/sqlglot/generator.py
@@ -703,9 +703,7 @@ class Generator:
                 nested = f"{self.STRUCT_DELIMITER[0]}{interior}{self.STRUCT_DELIMITER[1]}"
                 if expression.args.get("values") is not None:
                     delimiters = ("[", "]") if type_value == exp.DataType.Type.ARRAY else ("(", ")")
-                    values = (
-                        f"{delimiters[0]}{self.expressions(expression, 'values')}{delimiters[1]}"
-                    )
+                    values = f"{delimiters[0]}{self.expressions(expression, key='values')}{delimiters[1]}"
             else:
                 nested = f"({interior})"
 
@@ -721,7 +719,7 @@ class Generator:
         this = self.sql(expression, "this")
         this = f" FROM {this}" if this else ""
         using_sql = (
-            f" USING {self.expressions(expression, 'using', sep=', USING ')}"
+            f" USING {self.expressions(expression, key='using', sep=', USING ')}"
             if expression.args.get("using")
             else ""
         )
@@ -1329,16 +1327,20 @@ class Generator:
     def matchrecognize_sql(self, expression: exp.MatchRecognize) -> str:
         partition = self.partition_by_sql(expression)
         order = self.sql(expression, "order")
-        measures = self.sql(expression, "measures")
-        measures = self.seg(f"MEASURES {measures}") if measures else ""
+        measures = self.expressions(expression, key="measures")
+        measures = self.seg(f"MEASURES{self.seg(measures)}") if measures else ""
         rows = self.sql(expression, "rows")
         rows = self.seg(rows) if rows else ""
         after = self.sql(expression, "after")
         after = self.seg(after) if after else ""
         pattern = self.sql(expression, "pattern")
         pattern = self.seg(f"PATTERN ({pattern})") if pattern else ""
-        define = self.sql(expression, "define")
-        define = self.seg(f"DEFINE {define}") if define else ""
+        definition_sqls = [
+            f"{self.sql(definition, 'alias')} AS {self.sql(definition, 'this')}"
+            for definition in expression.args.get("define", [])
+        ]
+        definitions = self.expressions(sqls=definition_sqls)
+        define = self.seg(f"DEFINE{self.seg(definitions)}") if definitions else ""
         body = "".join(
             (
                 partition,
@@ -1350,7 +1352,9 @@ class Generator:
                 define,
             )
         )
-        return f"{self.seg('MATCH_RECOGNIZE')} {self.wrap(body)}"
+        alias = self.sql(expression, "alias")
+        alias = f" {alias}" if alias else ""
+        return f"{self.seg('MATCH_RECOGNIZE')} {self.wrap(body)}{alias}"
 
     def query_modifiers(self, expression: exp.Expression, *sqls: str) -> str:
         limit = expression.args.get("limit")
@@ -1371,7 +1375,7 @@ class Generator:
             self.sql(expression, "group"),
             self.sql(expression, "having"),
             self.sql(expression, "qualify"),
-            self.seg("WINDOW ") + self.expressions(expression, "windows", flat=True)
+            self.seg("WINDOW ") + self.expressions(expression, key="windows", flat=True)
             if expression.args.get("windows")
             else "",
             self.sql(expression, "distribute"),
@@ -1604,7 +1608,7 @@ class Generator:
 
     def primarykey_sql(self, expression: exp.ForeignKey) -> str:
         expressions = self.expressions(expression, flat=True)
-        options = self.expressions(expression, "options", flat=True, sep=" ")
+        options = self.expressions(expression, key="options", flat=True, sep=" ")
         options = f" {options}" if options else ""
         return f"PRIMARY KEY ({expressions}){options}"
 
@@ -1688,7 +1692,7 @@ class Generator:
         this = self.sql(expression, "this")
         expressions = self.expressions(expression, flat=True)
         expressions = f"({expressions})" if expressions else ""
-        options = self.expressions(expression, "options", flat=True, sep=" ")
+        options = self.expressions(expression, key="options", flat=True, sep=" ")
         options = f" {options}" if options else ""
         return f"REFERENCES {this}{expressions}{options}"
 
@@ -1714,9 +1718,9 @@ class Generator:
         return f"NOT {self.sql(expression, 'this')}"
 
     def alias_sql(self, expression: exp.Alias) -> str:
-        to_sql = self.sql(expression, "alias")
-        to_sql = f" AS {to_sql}" if to_sql else ""
-        return f"{self.sql(expression, 'this')}{to_sql}"
+        alias = self.sql(expression, "alias")
+        alias = f" AS {alias}" if alias else ""
+        return f"{self.sql(expression, 'this')}{alias}"
 
     def aliases_sql(self, expression: exp.Aliases) -> str:
         return f"{self.sql(expression, 'this')} AS ({self.expressions(expression, flat=True)})"
@@ -1825,13 +1829,13 @@ class Generator:
         actions = expression.args["actions"]
 
         if isinstance(actions[0], exp.ColumnDef):
-            actions = self.expressions(expression, "actions", prefix="ADD COLUMN ")
+            actions = self.expressions(expression, key="actions", prefix="ADD COLUMN ")
         elif isinstance(actions[0], exp.Schema):
-            actions = self.expressions(expression, "actions", prefix="ADD COLUMNS ")
+            actions = self.expressions(expression, key="actions", prefix="ADD COLUMNS ")
         elif isinstance(actions[0], exp.Delete):
-            actions = self.expressions(expression, "actions", flat=True)
+            actions = self.expressions(expression, key="actions", flat=True)
         else:
-            actions = self.expressions(expression, "actions")
+            actions = self.expressions(expression, key="actions")
 
         exists = " IF EXISTS" if expression.args.get("exists") else ""
         return f"ALTER TABLE{exists} {self.sql(expression, 'this')} {actions}"
@@ -1994,14 +1998,15 @@ class Generator:
 
     def expressions(
         self,
-        expression: exp.Expression,
+        expression: t.Optional[exp.Expression] = None,
         key: t.Optional[str] = None,
+        sqls: t.Optional[t.List[str]] = None,
         flat: bool = False,
         indent: bool = True,
         sep: str = ", ",
         prefix: str = "",
     ) -> str:
-        expressions = expression.args.get(key or "expressions")
+        expressions = expression.args.get(key or "expressions") if expression else sqls
 
         if not expressions:
             return ""
diff --git a/sqlglot/parser.py b/sqlglot/parser.py
index 7df407da..08cb3f2d 100644
--- a/sqlglot/parser.py
+++ b/sqlglot/parser.py
@@ -1912,14 +1912,13 @@ class Parser(metaclass=_Parser):
     def _parse_match_recognize(self) -> t.Optional[exp.Expression]:
         if not self._match(TokenType.MATCH_RECOGNIZE):
             return None
+
         self._match_l_paren()
 
         partition = self._parse_partition_by()
         order = self._parse_order()
         measures = (
-            self._parse_alias(self._parse_conjunction())
-            if self._match_text_seq("MEASURES")
-            else None
+            self._parse_csv(self._parse_expression) if self._match_text_seq("MEASURES") else None
         )
 
         if self._match_text_seq("ONE", "ROW", "PER", "MATCH"):
@@ -1973,8 +1972,17 @@ class Parser(metaclass=_Parser):
             pattern = None
 
         define = (
-            self._parse_alias(self._parse_conjunction()) if self._match_text_seq("DEFINE") else None
+            self._parse_csv(
+                lambda: self.expression(
+                    exp.Alias,
+                    alias=self._parse_id_var(any_token=True),
+                    this=self._match(TokenType.ALIAS) and self._parse_conjunction(),
+                )
+            )
+            if self._match_text_seq("DEFINE")
+            else None
         )
+
         self._match_r_paren()
 
         return self.expression(
@@ -1986,6 +1994,7 @@ class Parser(metaclass=_Parser):
             after=after,
             pattern=pattern,
             define=define,
+            alias=self._parse_table_alias(),
         )
 
     def _parse_lateral(self) -> t.Optional[exp.Expression]:


Test patch:
diff --git a/tests/dialects/test_oracle.py b/tests/dialects/test_oracle.py
index 6b4a32b1..92f0e5ce 100644
--- a/tests/dialects/test_oracle.py
+++ b/tests/dialects/test_oracle.py
@@ -108,3 +108,28 @@ FROM XMLTABLE(
             },
             pretty=True,
         )
+
+    def test_match_recognize(self):
+        self.validate_identity(
+            """SELECT
+  *
+FROM sales_history
+MATCH_RECOGNIZE (
+  PARTITION BY product
+  ORDER BY
+    tstamp
+  MEASURES
+    STRT.tstamp AS start_tstamp,
+    LAST(UP.tstamp) AS peak_tstamp,
+    LAST(DOWN.tstamp) AS end_tstamp,
+    MATCH_NUMBER() AS mno
+  ONE ROW PER MATCH
+  AFTER MATCH SKIP TO LAST DOWN
+  PATTERN (STRT UP+ FLAT* DOWN+)
+  DEFINE
+    UP AS UP.units_sold > PREV(UP.units_sold),
+    FLAT AS FLAT.units_sold = PREV(FLAT.units_sold),
+    DOWN AS DOWN.units_sold < PREV(DOWN.units_sold)
+) MR""",
+            pretty=True,
+        )
diff --git a/tests/dialects/test_snowflake.py b/tests/dialects/test_snowflake.py
index 6a9483b2..c883d13e 100644
--- a/tests/dialects/test_snowflake.py
+++ b/tests/dialects/test_snowflake.py
@@ -842,11 +842,13 @@ MATCH_RECOGNIZE (
   PARTITION BY a, b
   ORDER BY
     x DESC
-  MEASURES y AS b
+  MEASURES
+    y AS b
   {row}
   {after}
   PATTERN (^ S1 S2*? ( {{- S3 -}} S4 )+ | PERMUTE(S1, S2){{1,2}} $)
-  DEFINE x AS y
+  DEFINE
+    x AS y
 )""",
                     pretty=True,
                 )