This should fix MySQL 5.7 issues caused by ONLY_FULL_GROUP_BY #5973
Conversation
Should fix MySQL 5.7 issues caused by ONLY_FULL_GROUP_BY
wucdbm
changed the title
|
CI currently fails because of tests expect string equality. PR will probably need to contain the updated tests in order to get merged. |
|
I am aware. However, I lack the time to dig into it and change tests accordingly. I also have no clue if and if so, how this would affect other DBMS, so I'll simply leave my code here and hope it would help you guys get a decent solution out of it :) |
| return $sql; | ||
| } | ||
|
|
||
| // Rebuild the order by clause to work in the scope of the new select statement | ||
| /* @var array $orderBy an array of rebuilt order by items */ | ||
| $orderBy = $this->rebuildOrderByClauseForOuterScope($orderByClause); | ||
| $orderByFields = str_replace([' DESC', ' ASC'], ['', ''], $orderBy); |
There was a problem hiding this comment.
You may have something like ORDER BY ASCII ASC - that would be replaced with ORDER BYII here - no-go.
There was a problem hiding this comment.
This calls for additional test scenarios for both descending and ascending sorting clauses
There was a problem hiding this comment.
You probably want to match a series of ((\S+)\s+(ASC|DESC)\s*,?)* here (or similar)
| // If the sql statement has an order by clause, we need to wrap it in a new select distinct | ||
| // statement | ||
| if (! $orderByClause instanceof OrderByClause) { | ||
| if (!$orderByClause instanceof OrderByClause) { |
|
I ran into the same problem. The problem is still not fixed? @Ocramius |
|
@Shandur doesn't look like it. Maybe you can finish that PR? it doesn't look like much is missing from it. |
|
@theofidry I'll try to do it in few days. |
|
hello guys, any progress with this? |
|
@lukasneubauer Hey. May was really busy for me, I didn't have any time to work with it. Gonna try to finish it the next week, I'll have some free time. I'll keep you posted. |
|
Hello, We are all waiting for your PR in our company and I guess we are not the only ones :) |
|
@bourdeau then maybe on waiting on it someone could finish this up ;) |
|
@lcobucci yeap. |
|
Handled in #6143 |
First of all, I have to say that this is my first pull request ever created
This should fix MySQL 5.7 issues caused by ONLY_FULL_GROUP_BY
The proposed code works for me, maybe others could share opinions on a fix for this, it is very annoying when working with new versions of MySQL