parser: fix the compatibility problem of UNION statement
#6335
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -949,6 +949,8 @@ func (s *testSuite) TestUnion(c *C) { | |
| tk.MustExec("create table t(a int)") | ||
| tk.MustExec("insert into t value(0),(0)") | ||
| tk.MustQuery("select 1 from (select a from t union all select a from t) tmp").Check(testkit.Rows("1", "1", "1", "1")) | ||
| tk.MustQuery("select 10 as a from dual union select a from t order by a desc limit 1 ").Check(testkit.Rows("10")) | ||
| tk.MustQuery("select -10 as a from dual union select a from t order by a limit 1 ").Check(testkit.Rows("-10")) | ||
|
There was a problem hiding this comment. This test will cause a CI error. There was a problem hiding this comment. @ciscoxll The test failure seems not caused by this test. I can pass test on my dev machine using |
||
| tk.MustQuery("select count(1) from (select a from t union all select a from t) tmp").Check(testkit.Rows("4")) | ||
|
|
||
| _, err := tk.Exec("select 1 from (select a from t limit 1 union all select a from t limit 1) tmp") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -699,6 +699,9 @@ import ( | |
| SelectStmtFieldList "SELECT statement field list" | ||
| SelectStmtLimit "SELECT statement optional LIMIT clause" | ||
| SelectStmtOpts "Select statement options" | ||
| SelectStmtPart1 "SELECT statement part 1" | ||
|
There was a problem hiding this comment. What's the difference between part1/2/3? There was a problem hiding this comment. They are syntax segments that abstracted from There was a problem hiding this comment. How about changing to a better name? |
||
| SelectStmtPart2 "SELECT statement part 2" | ||
| SelectStmtPart3 "SELECT statement part 3" | ||
| SelectStmtGroup "SELECT statement optional GROUP BY clause" | ||
| ShowTargetFilterable "Show target that can be filtered by WHERE or LIKE" | ||
| ShowDatabaseNameOpt "Show tables/columns statement database name option" | ||
|
|
@@ -4075,96 +4078,107 @@ RollbackStmt: | |
| $$ = &ast.RollbackStmt{} | ||
| } | ||
|
|
||
| SelectStmtPart1: | ||
| "SELECT" SelectStmtOpts SelectStmtFieldList | ||
| { | ||
| st := &ast.SelectStmt { | ||
| SelectStmtOpts: $2.(*ast.SelectStmtOpts), | ||
| Distinct: $2.(*ast.SelectStmtOpts).Distinct, | ||
| Fields: $3.(*ast.FieldList), | ||
| } | ||
| $$ = st | ||
| } | ||
|
|
||
| SelectStmtPart2: | ||
| SelectStmtPart1 FromDual WhereClauseOptional | ||
| { | ||
| st := $1.(*ast.SelectStmt) | ||
| lastField := st.Fields.Fields[len(st.Fields.Fields)-1] | ||
| if lastField.Expr != nil && lastField.AsName.O == "" { | ||
| lastEnd := yyS[yypt-1].offset-1 | ||
| lastField.SetText(parser.src[lastField.Offset:lastEnd]) | ||
| } | ||
| if $3 != nil { | ||
| st.Where = $3.(ast.ExprNode) | ||
| } | ||
| } | ||
|
|
||
|
|
||
| SelectStmtPart3: | ||
| SelectStmtPart1 "FROM" | ||
| TableRefsClause WhereClauseOptional SelectStmtGroup HavingClause | ||
| { | ||
| st := $1.(*ast.SelectStmt) | ||
| st.From = $3.(*ast.TableRefsClause) | ||
| if st.SelectStmtOpts.TableHints != nil { | ||
| st.TableHints = st.SelectStmtOpts.TableHints | ||
| } | ||
| lastField := st.Fields.Fields[len(st.Fields.Fields)-1] | ||
| if lastField.Expr != nil && lastField.AsName.O == "" { | ||
| lastEnd := parser.endOffset(&yyS[yypt-4]) | ||
| lastField.SetText(parser.src[lastField.Offset:lastEnd]) | ||
| } | ||
| if $4 != nil { | ||
| st.Where = $4.(ast.ExprNode) | ||
| } | ||
| if $5 != nil { | ||
| st.GroupBy = $5.(*ast.GroupByClause) | ||
| } | ||
| if $6 != nil { | ||
| st.Having = $6.(*ast.HavingClause) | ||
| } | ||
| $$ = st | ||
| } | ||
|
|
||
| SelectStmt: | ||
| SelectStmtPart1 OrderByOptional SelectStmtLimit SelectLockOpt | ||
| { | ||
| st := $1.(*ast.SelectStmt) | ||
| st.LockTp = $4.(ast.SelectLockType) | ||
| lastField := st.Fields.Fields[len(st.Fields.Fields)-1] | ||
| if lastField.Expr != nil && lastField.AsName.O == "" { | ||
| src := parser.src | ||
| var lastEnd int | ||
| if $2 != nil { | ||
| lastEnd = yyS[yypt-2].offset-1 | ||
| } else if $3 != nil { | ||
| lastEnd = yyS[yypt-1].offset-1 | ||
| } else if $4 != ast.SelectLockNone { | ||
| lastEnd = yyS[yypt].offset-1 | ||
| } else { | ||
| lastEnd = len(src) | ||
| if src[lastEnd-1] == ';' { | ||
| lastEnd-- | ||
| } | ||
| } | ||
| lastField.SetText(src[lastField.Offset:lastEnd]) | ||
| } | ||
| if $2 != nil { | ||
| st.OrderBy = $2.(*ast.OrderByClause) | ||
| } | ||
| if $3 != nil { | ||
| st.Limit = $3.(*ast.Limit) | ||
| } | ||
| $$ = st | ||
| } | ||
| | SelectStmtPart2 SelectStmtLimit SelectLockOpt | ||
| { | ||
| st := $1.(*ast.SelectStmt) | ||
| st.LockTp = $3.(ast.SelectLockType) | ||
| if $2 != nil { | ||
| st.Limit = $2.(*ast.Limit) | ||
| } | ||
| $$ = st | ||
| } | ||
| | SelectStmtPart3 OrderByOptional SelectStmtLimit SelectLockOpt | ||
| { | ||
| st := $1.(*ast.SelectStmt) | ||
| st.LockTp = $4.(ast.SelectLockType) | ||
| if $2 != nil { | ||
| st.OrderBy = $2.(*ast.OrderByClause) | ||
| } | ||
| if $3 != nil { | ||
| st.Limit = $3.(*ast.Limit) | ||
| } | ||
| $$ = st | ||
| } | ||
|
|
@@ -4591,14 +4605,53 @@ SelectLockOpt: | |
|
|
||
| // See https://dev.mysql.com/doc/refman/5.7/en/union.html | ||
|
|
||
| UnionStmt: | ||
| UnionClauseList "UNION" UnionOpt SelectStmtPart1 OrderByOptional SelectStmtLimit SelectLockOpt | ||
|
There was a problem hiding this comment. Shall we extract a common There was a problem hiding this comment.
|
||
| { | ||
| st := $4.(*ast.SelectStmt) | ||
| union := $1.(*ast.UnionStmt) | ||
| union.Distinct = union.Distinct || $3.(bool) | ||
| lastSelect := union.SelectList.Selects[len(union.SelectList.Selects)-1] | ||
| endOffset := parser.endOffset(&yyS[yypt-5]) | ||
| parser.setLastSelectFieldText(lastSelect, endOffset) | ||
| union.SelectList.Selects = append(union.SelectList.Selects, st) | ||
| if $5 != nil { | ||
| union.OrderBy = $5.(*ast.OrderByClause) | ||
| } | ||
| if $6 != nil { | ||
| union.Limit = $6.(*ast.Limit) | ||
| } | ||
| $$ = union | ||
|
There was a problem hiding this comment. yes. Since the There was a problem hiding this comment. I think we need to add SelectLockOpt on both side of the union in this case. There was a problem hiding this comment. @tiancaiamao what if the left There was a problem hiding this comment. begin txn1 begin txn2 commit txn1 (If for update works on union, this operation should fail in TiDB) There was a problem hiding this comment. We should promote to lock mode either The select lock have something to do with transaction isolation level. There was a problem hiding this comment.
@tiancaiamao what this mean ? Fail to get your point..... @.@ BTW, I test these cases in MySQL 5.7. mysql> begin;
Query OK, 0 rows affected (0.00 sec)
mysql> select * from t union select * from t2 for update;
+----+----+---------+
| id | tn | type |
+----+----+---------+
| 1 | 5 | cat |
| 2 | 6 | dog |
| 3 | 7 | duck |
| 4 | 29 | chicken |
| 4 | 99 | chicken |
+----+----+---------+
5 rows in set (0.01 sec)Then, execute this in the second tx: mysql> update t set tn = 31 where id = 4; Query OK, 1 row affected (0.00 sec)
Rows matched: 1 Changed: 1 Warnings: 0
mysql> update t2 set tn = 31 where id = 4;
ERROR 1205 (HY000): Lock wait timeout exceeded; try restarting transactionthe Another condition: mysql> begin;
Query OK, 0 rows affected (0.00 sec)
mysql> select * from t union select * from t2 limit 100 for update; +----+-----+---------+
| id | tn | type |
+----+-----+---------+
| 1 | 5 | cat |
| 2 | 6 | dog |
| 3 | 7 | duck |
| 4 | 187 | chicken |
+----+-----+---------+
4 rows in set (0.01 sec)Then, execute this in the second tx: mysql> begin;
Query OK, 0 rows affected (0.00 sec)
mysql> update t2 set tn = 1 where id = 1; Query OK, 1 row affected (0.00 sec)
Rows matched: 1 Changed: 1 Warnings: 0
mysql> update t set tn = 1 where id = 1;
Query OK, 1 row affected (0.00 sec)
Rows matched: 1 Changed: 1 Warnings: 0Both Update works. The There was a problem hiding this comment. Okay, there is some subtle difference about the transaction isolation between TiDB and MySQL. |
||
| } | ||
| | UnionClauseList "UNION" UnionOpt SelectStmtPart2 SelectStmtLimit SelectLockOpt | ||
| { | ||
| st := $4.(*ast.SelectStmt) | ||
| union := $1.(*ast.UnionStmt) | ||
| union.Distinct = union.Distinct || $3.(bool) | ||
| lastSelect := union.SelectList.Selects[len(union.SelectList.Selects)-1] | ||
| endOffset := parser.endOffset(&yyS[yypt-4]) | ||
| parser.setLastSelectFieldText(lastSelect, endOffset) | ||
| union.SelectList.Selects = append(union.SelectList.Selects, st) | ||
| if $5 != nil { | ||
| union.Limit = $5.(*ast.Limit) | ||
| } | ||
| $$ = union | ||
| } | ||
| | UnionClauseList "UNION" UnionOpt SelectStmtPart3 OrderByOptional | ||
| SelectStmtLimit SelectLockOpt | ||
| { | ||
| st := $4.(*ast.SelectStmt) | ||
| union := $1.(*ast.UnionStmt) | ||
| union.Distinct = union.Distinct || $3.(bool) | ||
| lastSelect := union.SelectList.Selects[len(union.SelectList.Selects)-1] | ||
| endOffset := parser.endOffset(&yyS[yypt-5]) | ||
| parser.setLastSelectFieldText(lastSelect, endOffset) | ||
| union.SelectList.Selects = append(union.SelectList.Selects, st) | ||
| if $5 != nil { | ||
| union.OrderBy = $5.(*ast.OrderByClause) | ||
| } | ||
| if $6 != nil { | ||
| union.Limit = $6.(*ast.Limit) | ||
| } | ||
| $$ = union | ||
| } | ||
| | UnionClauseList "UNION" UnionOpt '(' SelectStmt ')' OrderByOptional SelectStmtLimit | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bug this PR fixed is that,
order by a desc limit 1belongs to union statement, but our old code thoughtit belongs to the second select statement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the last
order by a desc limit 1is expected to belong to the whole union statement.