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

planner: resolve order by item from select fields first when checking FULL_GROUP_BY constraint (#16332) #16620

Merged
merged 3 commits into from
Jun 15, 2020

Conversation

sre-bot
Copy link
Contributor

@sre-bot sre-bot commented Apr 20, 2020

cherry-pick #16332 to release-3.0


What problem does this PR solve?

Problem Summary:

mysql> create table t(a int, b int);
Query OK, 0 rows affected (0.01 sec)

mysql> select count(a) as b from t group by a order by b;
ERROR 1055 (42000): Expression #1 of ORDER BY is not in GROUP BY clause and contains nonaggregated column '' which is not functionally dependent on columns in GROUP BY clause; this is incompatible with sql_mode=only_full_group_by

MySQL does not report error for this query.

What is changed and how it works?

What's Changed:

Resolve ByItem of ORDER BY using select fields first, if the ByItem refers to the field, skip check for it since it has been checked already; if not, resolve it from input plan.

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test

Side effects

N/A

Release note

  • Fix error of query when only_full_group_by SQL mode is set

@sre-bot
Copy link
Contributor Author

sre-bot commented Apr 20, 2020

/run-all-tests

Signed-off-by: sre-bot <sre-bot@pingcap.com>
Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zz-jason zz-jason added the status/LGT1 Indicates that a PR has LGTM 1. label Apr 23, 2020
Copy link
Contributor

@lzmhhh123 lzmhhh123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lzmhhh123 lzmhhh123 added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels May 8, 2020
@sre-bot
Copy link
Contributor Author

sre-bot commented May 12, 2020

@bb7133 bb7133 modified the milestones: v3.0.14, v3.0.15 May 19, 2020
@bb7133 bb7133 modified the milestones: v3.0.15, 3.0.16, v3.0.16 Jun 6, 2020
@eurekaka
Copy link
Contributor

/run-all-tests

@eurekaka
Copy link
Contributor

/run-mybatis-test
/run-unit-test

@eurekaka eurekaka merged commit bf529ac into pingcap:release-3.0 Jun 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/planner SIG: Planner status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug. type/compatibility type/3.0-cherry-pick
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants