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

reject valid aggregated query in only_full_group_by mode #22301

Closed
xuyifangreeneyes opened this issue Jan 8, 2021 · 10 comments
Closed

reject valid aggregated query in only_full_group_by mode #22301

xuyifangreeneyes opened this issue Jan 8, 2021 · 10 comments
Assignees
Labels
severity/minor sig/planner SIG: Planner type/bug The issue is confirmed as a bug.

Comments

@xuyifangreeneyes
Copy link
Contributor

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);
create table t2 (a int, b int);
select t1.a from t1 join t2 on t2.a=t1.a group by t2.a having min(t2.b) > 0;
select t2.a, count(t2.b) from t1 join t2 using (a) where t1.a = 1;

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

mysql> select t1.a from t1 join t2 on t2.a=t1.a group by t2.a having min(t2.b) > 0;
Empty set (0.00 sec)

mysql> select t2.a, count(t2.b) from t1 join t2 using (a) where t1.a = 1;
+------+-------------+
| a    | count(t2.b) |
+------+-------------+
| NULL |           0 |
+------+-------------+
1 row in set (0.00 sec)

3. What did you see instead (Required)

tidb> select t1.a from t1 join t2 on t2.a=t1.a group by t2.a having min(t2.b) > 0;
ERROR 1055 (42000): Expression #1 of SELECT list is not in GROUP BY clause and contains nonaggregated column 'test.t1.a' which is not functionally dependent on columns in GROUP BY clause; this is incompatible with sql_mode=only_full_group_by
tidb> select t2.a, count(t2.b) from t1 join t2 using (a) where t1.a = 1;
ERROR 8123 (HY000): In aggregated query without GROUP BY, expression #1 of SELECT list contains nonaggregated column 'a'; this is incompatible with sql_mode=only_full_group_by

4. What is your TiDB version? (Required)

tidb> select tidb_version()\G
*************************** 1. row ***************************
tidb_version(): Release Version: v4.0.0-beta.2-1990-gcec1a9265
Edition: Community
Git Commit Hash: cec1a926583b008f919430b35a032a38098ddcbc
Git Branch: master
UTC Build Time: 2021-01-08 07:26:23
GoVersion: go1.15.5
Race Enabled: false
TiKV Min Version: v3.0.0-60965b006877ca7234adaced7890d7b029ed1306
Check Table Before Drop: false
1 row in set (0.00 sec)
@xuyifangreeneyes xuyifangreeneyes added the type/bug The issue is confirmed as a bug. label Jan 8, 2021
@ichn-hu
Copy link
Contributor

ichn-hu commented Jan 11, 2021

/unlabel sig/execution

@ti-srebot ti-srebot removed the sig/execution SIG execution label Jan 11, 2021
@ichn-hu
Copy link
Contributor

ichn-hu commented Jan 11, 2021

/label sig/planner

@ti-srebot ti-srebot added the sig/planner SIG: Planner label Jan 11, 2021
@ichn-hu
Copy link
Contributor

ichn-hu commented Jan 11, 2021

I think this should be a planner bug

@xuyifangreeneyes
Copy link
Contributor Author

/assign

@Reminiscent
Copy link
Contributor

/assign

@Reminiscent
Copy link
Contributor

After discussing with @winoros offline. We think we should change the severity from major to minor. Because the implementation of the only-full-group-by checker is not good enough. It has many issues. We should refactor the code in the future.

@Reminiscent
Copy link
Contributor

These SQL statements can work in TiDB:

select t2.a, count(t2.b) from t2 where t2.a = 1;
select t1.a, count(t2.b) from t1 join t2 using (a) where t1.a = 1;

But these statements will get the wrong results:

select t1.a, count(t2.b) from t1 join t2 using (a) where t2.a = 1;
select t2.a, count(t2.b) from t1 join t2 using (a) where t2.a = 1;

@yudongusa
Copy link

@xuyifangreeneyes @Reminiscent would you please provide an update or verify if the issue still exists?

@xuyifangreeneyes
Copy link
Contributor Author

@xuyifangreeneyes @Reminiscent would you please provide an update or verify if the issue still exists?

The issue still exists on the current master. Maybe we can open an tracking issue to collect all the issues on only_full_group_by and see how to improve or refactor only-full-group-by-checker.

@AilinKid
Copy link
Contributor

winoros#10 in our latest feature branch, this has been fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
severity/minor sig/planner SIG: Planner type/bug The issue is confirmed as a bug.
Projects
None yet
Development

No branches or pull requests

8 participants