fixed #5622 - mysql 5.7 ONLY_FULL_GROUP_BY

[chihiro-adachi]

#5622

I fixed it with reference to #5973
please check.

[ruudk]

I just tested this in our project and it seems to solve the issues. Really awesome! 👍

Would be great if this could get merged.

[kissifrot]

Confirmed here too, +1 for a merge :)

[zluiten]

Looks good. Let's merge this!

[chihiro-adachi]

@Ocramius
this pr Is there something wrong?
anything else needed?

thank you.

[Ocramius]

Simply didn't get to it, sorry. Marco Pivetta http://twitter.com/Ocramius http://ocramius.github.com/

…

On Wed, Mar 15, 2017 at 1:18 PM, chihiro-adachi ***@***.***> wrote: @Ocramius <https://github.com/Ocramius> this pr Is there something wrong? anything else needed? thank you. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6143 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAJakJiQrYhcTpaQqIV6L6P8Xxi9ZjFcks5rl9cBgaJpZM4K5JxB> .

[mauriciogtaborda]

lgtm 👍

[nateevans]

is this on deck to be pulled in anytime soon?

[fefas]

+1

[paaacman]

+1

[Einenlum]

+1

[lcobucci]

@Ocramius I've synced this PR with master and added MySQL 5.7 to Travis. I'm just not fully convinced that this is the way to go (regex stuff and also the impact of wrapping the subquery on the other platforms).

[teohhanhui]

the impact of wrapping the subquery on the other platforms

Any SQL-compliant platform should require this fix.

[Arkemlar]

why still not merge?

[Ocramius]

@Arkemlar because I'd rather not have support for MySQL 5.7 than having security issues due to regex-based SQL manipulation.

The patch may be good, but it will need further checking for edge-cases in the newly added conditionals.

Also, tests were only modified so far, but none was added.

[Ocramius]

Looking good according to @lcobucci's changes: will likely merge today.

[Ocramius]

LGTM 👍 🚢

[teohhanhui]

Will this be merged into stable versions as a bugfix?

[Ocramius]

@chihiro-adachi this is already in 2.6.0

[teohhanhui]

Is 2.6.0 released yet? I can't find it anywhere. But I mean this should be treated as a bug, right? If so, might it be fixed in the currently supported versions?

[lcobucci]

No, mysql 5.7 is just officially supported in 2.6 (unstable version).

…

On Mon, 24 Jul 2017, 12:18 Teoh Han Hui, ***@***.***> wrote: Is 2.6.0 released yet? I can't find it anywhere. But I mean this should be treated as a bug, right? If so, might it be fixed in the currently supported versions? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6143 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAMU6w196JWvvHIDEAOkedRI0LTy9zlZks5sRG-NgaJpZM4K5JxB> .

[Ocramius]

@teohhanhui no, newer mysql versions broke BC and we simply support them in a newer version only.

[teohhanhui]

I thought this bug is also present when using other SQL-compliant platforms such as PostgreSQL... Because this bug only manifested itself when MySQL started becoming more SQL-compliant.

[Ocramius]

@teohhanhui yes, but only MySQL crashes hard. So far, the only issue with such queries was non-deterministic result due to ordering.

[ureimers]

@teohhanhui If you can't wait for Doctrine 2.6 to be released you still have the hackish way of overriding the Autoloader in your composer.json:

"autoload": {
        [...]
        "files": ["src/LimitSubqueryOutputWalkerPatch.php"]
    },

Just put the fixed source from this PR into the LimitSubqueryOutputWalkerPatch.php. When composer dumps your autoloader the class will take precedence over the Doctrine 2.5 one. Until Doctrine 2.6 is available you use this to temporarily fix the MySQL 5.7 incompatibility of the Paginator.

But keep in mind that this is pretty hackish. On the other hand if a project is blocked because of this you sometimes just don't have a choice.

[InfopactMLoos]

Is this not backported to 2.5 ? Since my company uses Ubuntu 16.04 LTS and the 2.6 branch does not support PHP 7.0 I cannot upgrade to 2.6. @ureimers solution works but I would prefer a cleaner solution if possible. I tried using the patch generated by this pull request but it is not compatible with 2.5. Now trying to strip down this patch so the changes can be applied to 2.5 but was wondering why this bugfix is 2.6 only.

[Ocramius]

@InfopactMLoos no, you will need to upgrade to 2.6. Upgrading PHP version is easy peasy via http://ppa.launchpad.net/ondrej/php/ubuntu/pool/main/p/php7.1/

why this bugfix is 2.6 only.

MySQL 5.7 support is an addition.

[InfopactMLoos]

Yeah I know how to do that through ppa it's easy indeed but company policy is to keep everything LTS compatible. We try to minimize customizations to base server images. Ok no worries I just completed rolling the patch and it seems to work fine. Was just wondering why, anyway thanks for the quick response!

[shkkmo]

It seems like there should be a version of Doctrine that supports both PHP 7.0 and MySQL given that those are the available versions in Ubuntu 16.04

[Ocramius]

Won't happen, as that is just pushing back the ecosystem with the excuse that the ecosystem is already being pushed back.

You can simply rely on the PPAs by ondrej, which reflect PHP core development, on which you can rely for security maintenance as well.