Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

set: shouldn't support 'select ... into outfile ... union select ... ' and 'select ... union select SQL_BUFFER_RESULT ... ' statements #19097

Open
ChenPeng2013 opened this issue Aug 10, 2020 · 16 comments
Assignees
Labels
challenge-program component/parser good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. severity/minor sig/sql-infra SIG: SQL Infra type/compatibility

Comments

@ChenPeng2013
Copy link
Contributor

ChenPeng2013 commented Aug 10, 2020

Description

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

drop table if exists t1, t2;
CREATE TABLE t1 (a int not null, b char (10) not null);
insert into t1 values(1,'a'),(2,'b'),(3,'c'),(3,'c');
CREATE TABLE t2 (a int not null, b char (10) not null);
insert into t2 values (3,'c'),(4,'d'),(5,'f'),(6,'e');
select a,b from t1 into outfile 'skr' union select a,b from t2;
select * from t1 union select SQL_BUFFER_RESULT * from t2;

2. What did you expect to see? (Required)

in MySQL or MariaDB

mysql> select a,b from t1 into outfile 'skr' union select a,b from t2;
ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'union select a,b from t2' at line 1
mysql> select * from t1 union select SQL_BUFFER_RESULT * from t2;
ERROR 1234 (42000): Incorrect usage/placement of 'SQL_BUFFER_RESULT'

3. What did you see instead (Required)

mysql> select a,b from t1 into outfile 'skr' union select a,b from t2;
+---+------+
| a | b    |
+---+------+
| 3 | c    |
| 4 | d    |
| 5 | f    |
| 6 | e    |
| 2 | b    |
| 1 | a    |
+---+------+
6 rows in set (0.00 sec)

mysql> select * from t1 union select SQL_BUFFER_RESULT * from t2;
+---+------+
| a | b    |
+---+------+
| 1 | a    |
| 2 | b    |
| 4 | d    |
| 5 | f    |
| 6 | e    |
| 3 | c    |
+---+------+
6 rows in set (0.00 sec)

4. What is your TiDB version? (Required)

Release Version: v4.0.0-beta.2-921-gb75a30fda
Edition: Community
Git Commit Hash: b75a30fda8eef5962e2fde1c6254ff02f330fd21
Git Branch: master
UTC Build Time: 2020-08-10 02:44:14
GoVersion: go1.14.6
Race Enabled: false
TiKV Min Version: v3.0.0-60965b006877ca7234adaced7890d7b029ed1306
Check Table Before Drop: false

SIG slack channel

#sig-exec

Score

  • 300

Mentor

@ChenPeng2013 ChenPeng2013 added the type/bug The issue is confirmed as a bug. label Aug 10, 2020
@liuzix
Copy link
Contributor

liuzix commented Aug 10, 2020

/label sig/execution

@ti-srebot ti-srebot added the sig/execution SIG execution label Aug 10, 2020
@ghost
Copy link

ghost commented Aug 10, 2020

I am not sure if SQL_BUFFER_RESULT does anything in TiDB? I think this is just a simple parser compatibility issue. Fixing it will help ensure applications can migrate back to MySQL if needed.

@lzmhhh123
Copy link
Contributor

It's a compatibility issue about the parser. So I move the sig/exec to sig/ddl and change type/bug to type/compatibility.

@AilinKid
Copy link
Contributor

AilinKid commented Nov 23, 2020

SQL_BUFFER_RESULT is introduced in pingcap/parser#304 for compatibility, and it has nothing to do with the TiDB side.

So the second SQL in the issue select * from t1 union select SQL_BUFFER_RESULT * from t2; will be run as select * from t1 union select * from t2;. With respect to this, this is as expected as MySQL does.

@wjhuang2016
Copy link
Member

For the second SQL, we can document that SQL_BUFFER_RESULT is ignored.

@AilinKid
Copy link
Contributor

AilinKid commented Nov 23, 2020

Reason:
This is a difference between TiDB and MySQL. For lock-clause and into-clause, MySQL will parse them as the suffix of the query_expression which is an embedded loop of the select statement (union included), while TiDB can recognize them as the suffix for every single select-statement.

syntax

So SQL like select a,b from t1 into outfile 'skr' union select a,b from t2 is invalid in MySQL, because after the into-clause is parsed, the union clause will be unrecognized, which leads to error.

However, it will be parsed successfully in TIDB because every single select-statement can be recognized with an into-clause suffix. The reason why the output is shown on the result rather than be stored in the file is: building an internal select for union statement will ignore the into-option.

As a conclusion above, we only support the into-option for single select-statement, better doc it on our website. If we wanna fully be compatible with this, it will take some time.

@AilinKid
Copy link
Contributor

PTAL @wjhuang2016 @bb7133

@wjhuang2016
Copy link
Member

I think we should make TiDB be compatible with MySQL

@bb7133
Copy link
Member

bb7133 commented Nov 24, 2020

Sounds that is not 'difficult/easy'.

What's the estimated work time to change the parser?

@wjhuang2016
Copy link
Member

@AilinKid Any progress?

@AilinKid
Copy link
Contributor

AilinKid commented Nov 30, 2020

@AilinKid Any progress?

@tangenta is asked to help with this.

@wjhuang2016
Copy link
Member

@tangenta Any progress?

@tangenta
Copy link
Contributor

tangenta commented Dec 4, 2020

I tried to extract lock-clause and into-clause to an upper level, but the shift/reduce conflicts is hard to resolve. Can we throw the syntax error in the semantic action?

@wjhuang2016
Copy link
Member

I tried to extract lock-clause and into-clause to an upper level, but the shift/reduce conflicts is hard to resolve. Can we throw the syntax error in the semantic action?

You can ask amao or kenny for help.

@AilinKid
Copy link
Contributor

AilinKid commented Dec 9, 2020

I tried to extract lock-clause and into-clause to an upper level, but the shift/reduce conflicts is hard to resolve. Can we throw the syntax error in the semantic action?

yes, it's quite hard to solve. I suspect there will be some other work after the parser is finished.

@morgo
Copy link
Contributor

morgo commented Jul 30, 2021

Changing it to minor since it's a minor compatibility issue, not a bug.

@tisonkun tisonkun added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. and removed difficulty/easy labels Sep 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
challenge-program component/parser good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. severity/minor sig/sql-infra SIG: SQL Infra type/compatibility
Projects
None yet
Development

No branches or pull requests