plan: fix a bug when 'only_full_group_by' is set in sql_mode #6734
Conversation
…onaggregated column in query without GROUP BY
|
/run-all-tests |
|
/run-all-tests |
plan/logical_plan_builder.go
Outdated
| @@ -1342,6 +1350,51 @@ func (b *planBuilder) checkOnlyFullGroupBy(p LogicalPlan, fields []*ast.SelectFi | |||
| } | |||
| } | |||
|
|
|||
| func (b *planBuilder) checkOnlyFullGroupByWithOutGroupClause(p LogicalPlan, fields []*ast.SelectField) { | |||
| resolver := colResolverForOnlyFullGroupBy{} | |||
| //for idx, field := range fields { | |||
plan/logical_plan_builder.go
Outdated
| } | ||
| } | ||
|
|
||
| type colResolverForOnlyFullGroupBy struct { |
|
@spongedu plz fix ci |
|
@zz-jason Done. PTAL |
|
/run-all-tests |
plan/logical_plan_builder.go
Outdated
| resolver.exprIdx = idx | ||
| field.Accept(&resolver) | ||
| if err := resolver.Check(); err != nil { | ||
| b.err = err |
plan/logical_plan_builder.go
Outdated
| } | ||
|
|
||
| func (c *colResolverForOnlyFullGroupBy) Enter(node ast.Node) (ast.Node, bool) { | ||
| switch node.(type) { |
There was a problem hiding this comment.
use x := node.(type), so that we don't need to do the type assertion again at line 1381.
|
|
||
| func (c *colResolverForOnlyFullGroupBy) Check() error { | ||
| if c.hasAggFunc && c.firstNonAggCol != nil { | ||
| return ErrMixOfGroupFuncAndFields.GenByArgs(c.firstNonAggColIdx+1, c.firstNonAggCol.Name.O) |
There was a problem hiding this comment.
MySQL(localhost:3306) > select count(a) + a from t;
ERROR 1140 (42000): In aggregated query without GROUP BY, expression #1 of SELECT list contains nonaggregated column 'test.t.a'; this is incompatible with sql_mode=only_full_group_bySeems MySQL use database_name + table_name + column_name to print the name of nonaggregated column
There was a problem hiding this comment.
It's not always go that way.... If the FROM clause is a subquery, table_name + column_name is used, no database_name.....
mysql> select max(a) + a from (select 1 as a, 2 as b , 3 as c) f;
ERROR 1140 (42000): In aggregated query without GROUP BY, expression #1 of SELECT list contains nonaggregated column 'f.a'; this is incompatible with sql_mode=only_full_group_byanother case, if FROM clause is a join, the error message depends on where the column coms from
mysql> select max(a) + a from (select 1 as a, 2 as b , 3 as c) f join t on f.a = t.c1;
ERROR 1140 (42000): In aggregated query without GROUP BY, expression #1 of SELECT list contains nonaggregated column 'f.a'; this is incompati
ble with sql_mode=only_full_group_by
mysql> select max(c1) + c1 from (select 1 as a, 2 as b , 3 as c) f join t on f.a = t.c1;
ERROR 1140 (42000): In aggregated query without GROUP BY, expression #1 of SELECT list contains nonaggregated column 'cdbagentmanager.t.c1';
this is incompatible with sql_mode=only_full_group_byso I wonder if it's necessary keep the error message as exactly the same as MySQL....
There was a problem hiding this comment.
@spongedu It's not necessary, we can leave it to the future work 😆
| } | ||
|
|
||
| func (c *colResolverForOnlyFullGroupBy) Check() error { | ||
| if c.hasAggFunc && c.firstNonAggCol != nil { |
There was a problem hiding this comment.
How about select min(c1) from t where c2 in (select c2 from t where c3 > 10); Could this checker handle the situation of subquery?
There was a problem hiding this comment.
No, this situation is not handled by this checker. ColumnRefs in predicates is not affected under this circumstance because predicates are processed prior to aggregations.
There was a problem hiding this comment.
At least, this PR is an improvement. @shenli
zz-jason
changed the title
executor/aggregate_test.go
Outdated
| @@ -444,6 +452,9 @@ func (s *testSuite) TestOnlyFullGroupBy(c *C) { | |||
| tk.MustQuery("select b like '%a' from t group by b") | |||
| tk.MustQuery("select c REGEXP '1.*' from t group by c") | |||
| tk.MustQuery("select -b from t group by b") | |||
| tk.MustQuery("select max(a+b) from t") | |||
| tk.MustQuery("select avg(a)+1 from t") | |||
| _, err = tk.Exec("select count(c), 5 from t") | |||
There was a problem hiding this comment.
Done. @zimulala PTAL :)
…o fix_only_full_group_by
|
@zimulala PTAL |
…o fix_only_full_group_by
|
@shenli PTAL :P |
|
LGTM |
In MySQL 5.7, when
only_full_group_byis set inSQL_MODE, The following query will fail, raise error for nonaggregated column in query without GROUP BY:This pr make TiDB compatible with MySQL 5.7 under this circustance.