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

query that worked in Laravel 5.2 gives me error in Laravel 5.3 #14997

Closed
nikocraft opened this issue Aug 24, 2016 · 37 comments
Closed

query that worked in Laravel 5.2 gives me error in Laravel 5.3 #14997

nikocraft opened this issue Aug 24, 2016 · 37 comments

Comments

@nikocraft
Copy link

nikocraft commented Aug 24, 2016

This query works in 5.2:
my gallery table looks like this
id, title, hash, user_id, published, views, created_at, updated_at

$galleries = Gallery::with(array(
            'images' => function ($query) {
                $query->orderBy('order', 'asc');
            }
        ))
        ->with('votes')
        ->leftJoin('votes', 'votes.votable_id', '=', 'gallery.id')
        ->selectRaw(
            'gallery.*, count(case votes.status when "upvote" then 1 else null end) - count(case votes.status when "downvote" then 1 else null end) as points'
        )
        ->where('votes.votable_type','App\Gallery')
        ->groupBy('gallery.id')
        ->orderBy('points', 'desc')
        ->published()->orderBy('gallery.created_at', 'desc')->paginate(30);

I am trying to select all galleries that have votes, when I run this in 5.3 I get this

1/2 PDOException in Connection.php line 333: SQLSTATE[42000]: 
Syntax error or access violation: 1055 'images.gallery.title' isn't in GROUP BY 
SQLSTATE[42000]: Syntax error or access violation: 1055 'images.gallery.title' isn't in 
GROUP BY (SQL: select gallery.*, count(case votes.status when "upvote" then 1 else null end) - count(case votes.status when "downvote" then 1 else null end) 
as points from `gallery` left join `votes` on `votes`.`votable_id` = `gallery`.`id`
where `votes`.`votable_type` = App\Gallery and `published` = 1 group by `gallery`.`id` 
order by `points` desc, `gallery`.`created_at` desc limit 30 offset 0)
@Codetimeup
Copy link

Codetimeup commented Aug 24, 2016

I'm having the same issue after upgrading from 5.2 to 5.3. Everything works fine on 5.2.

@nikocraft
Copy link
Author

Codetimeup post your query here so Talyor can look at it

@fernandobandeira
Copy link
Contributor

Could you post the result of the method toSql() of this query, both on 5.2 and 5.3 instead of the paginate(30)?

Seems that the 5.3 isn't including all the columns on the groupBy, maybe it doesn't include the ones from the selectRaw only, i wanna see how the 5.2 does.

@nikocraft
Copy link
Author

nikocraft commented Aug 24, 2016

running toSql on 5.2 I get this:
'"select gallery.*, count(case votes.status when "upvote" then 1 else null end) - count(case votes.status when "downvote" then 1 else null end) as points from 'gallery' left join 'votes' on 'votes'.'votable_id' = 'gallery'.'id' where 'votes'.'votable_type' = ? and 'published' = ? group by 'gallery'.'id' order by 'points' desc, 'created_at' desc"'

and exactly the same sql query on 5.3 they are identical:
'"select gallery.*, count(case votes.status when "upvote" then 1 else null end) - count(case votes.status when "downvote" then 1 else null end) as points from 'gallery' left join 'votes' on 'votes'.'votable_id' = 'gallery'.'id' where 'votes'.'votable_type' = ? and 'published' = ? group by 'gallery'.'id' order by 'points' desc, 'created_at' desc"'

@nikocraft
Copy link
Author

nikocraft commented Aug 24, 2016

I posted about this on stackoverflow and someone suggested that I run the join according to new 5.3 documentation. Here is new query which also does not work.

            $galleries = Gallery::with(array(
                    'images' => function ($query) {
                        $query->orderBy('order', 'asc');
                    }
                ))
                ->with('votes')
                ->leftJoin('votes',function($query){
                             $query->on('votes','gallery')->where('votes', 'votes.votable_id', '=', 'gallery.id');
                 })->selectRaw(
                    'gallery.*, count(case votes.status when "upvote" then 1 else null end) - count(case votes.status when "downvote" then 1 else null end) as points'
                )
                ->where('votes.votable_type','App\Gallery')
                ->groupBy('gallery.id')
                ->orderBy('points', 'desc')
                ->published()->orderBy('gallery.created_at', 'desc')->paginate(30);

this new query fails and gives me

SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'gallery.id `votes` = ? where `votes`.`votable_type` = ? and `published` = ? grou' at line 1 (SQL: select count(*) as aggregate from `gallery` left join `votes` on `votes` = `gallery` gallery.id `votes` = votes.votable_id where `votes`.`votable_type` = App\Gallery and `published` = 1 group by `gallery`.`id`)

if I run toSql on it output is this:
"select gallery.*, count(case votes.status when "upvote" then 1 else null end) - count(case votes.status when "downvote" then 1 else null end) as points from 'gallery' left join 'votes' on 'votes' = 'gallery' gallery.id 'votes' = ? where 'votes'.'votable_type' = ? and 'published' = ? group by 'gallery'.'id' order by 'points' desc, 'gallery'.'created_at' desc"

@nikocraft
Copy link
Author

I updated the query so its now according to documentation and it looks like this:

            $galleries = Gallery::with(array(
                    'images' => function ($query) {
                        $query->orderBy('order', 'asc');
                    }
                ))
                ->with('votes')
                ->leftJoin('votes',function($query){
                             $query->on('votes.votable_id','gallery.id');
                 })->selectRaw(
                    'gallery.*, count(case votes.status when "upvote" then 1 else null end) - count(case votes.status when "downvote" then 1 else null end) as points'
                )
                ->where('votes.votable_type','App\Gallery')
                ->groupBy('gallery.id')
                ->orderBy('points', 'desc')
                ->published()->orderBy('gallery.created_at', 'desc')->paginate(20);

it produces this sql:

"select gallery.*, count(case votes.status when "upvote" then 1 else null end) - count(case votes.status when "downvote" then 1 else null end) as points from 'gallery' left join 'votes' on 'votes'.'votable_id' = 'gallery'.'id' where 'votes'.'votable_type' = ? and 'published' = ? group by 'gallery'.'id' order by 'points' desc, 'gallery'.'created_at' desc"

and if I put paginate(20) back into query I get the same error as before:
SQLSTATE[42000]: Syntax error or access violation: 1055 'images.gallery.title' isn't in GROUP BY

@Codetimeup
Copy link

All queries that have selectRaw or DB::Raw is having this issue for me. toSql method gave the same output in both versions of Laravel(5.2 and 5.3), I'll post an example of my code when i get home.

@fernandobandeira
Copy link
Contributor

This is pretty strange actually, because this is a sql error but both 5.2 and 5.3 are generating the same query, you probably have ONLY_FULL_GROUP_BY inside the SQL_MODE variable in your DB.

And this causes the MariaDB in your case to request that all the columns that are in the select (galeries.*) must also be in your group by.

Of course either adding manually all of these columns to your groupBy(), or removing ONLY_FULL_GROUP_BY from your SQL_MODE should fix your query but it's strange that 5.2 is behaving differently as you stated.

@nikocraft
Copy link
Author

I had to do this but its not eleqant and I dont like it, however it works: groupBy('gallery.id', 'gallery.title', 'gallery.hash', 'gallery.user_id', 'gallery.published', 'gallery.views' , 'gallery.created_at', 'gallery.updated_at')

how do I turn of ONLY_FULL_GROUP_BY, if it's on it's strange that it's working for 5.2 since it's connecting to the same database as 5.3

@nikocraft
Copy link
Author

I run this SELECT @@sql_mode in my database and got this NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION, as you can see ONLY_FULL_GROUP_BY is not there.

@fernandobandeira
Copy link
Contributor

fernandobandeira commented Aug 24, 2016

Ok, Laravel can also enable this mode when querying, check your config/database.php

And check if inside the mysql settings there is one that is like:

'strict' => false,

If this is set to true then it'll add the ONLY_FULL_GROUP_BY when querying.

Update
Just some trivia, strict enables these modes when querying:

ONLY_FULL_GROUP_BY
STRICT_TRANS_TABLES
NO_ZERO_IN_DATE
NO_ZERO_DATE
ERROR_FOR_DIVISION_BY_ZERO
NO_AUTO_CREATE_USER
NO_ENGINE_SUBSTITUTION

Instead of disabling strict what you could do is pass an array to the config, enabling only the modes that you want:

'modes' => [
    'NO_ZERO_DATE',
     'ONLY_FULL_GROUP_BY',
],

The above example enables both the group by and no zero date when querying.

Briefly with strict mode on you will have to:
1 - Add all columns to group by.
2 - Won't be able to use date's such as 0000-00-00 00:00:00
3 - Fields like boolean will throw fatal if you pass something that isn't a boolean value, like 1, before it would convert it to true and save, with strict it fails.
4 - You'll get an error if you divide a field by 0 (or another field that has 0 as value).

And etc, notice that in previous versions of Laravel when creating timestamps the migration added the default value 0000-00-00 00:00:00 to the fields, so remember to check that if you enable this mode after upgrading from an older version, also some packages may not work with it enabled yet since it's quite a new thing...

@fernandobandeira
Copy link
Contributor

Also i checked now and on the master branch this is set to true on laravel/laravel. I'll create a PR there an link with this issue.

@nikocraft
Copy link
Author

damn it was so simple :) well great it solved now

@Codetimeup
Copy link

Solved it, thanks fernandobandeira.

@abdullahnaseer
Copy link

Same problem. It was working on my machine with mysql but not on cloud with maria db. Anyways its solved by solution of fernandobandeira. Thank you.

@obonyojimmy
Copy link

This was very helpfull thread !

@nikodtbVf
Copy link

A lot of thanks!! <3

@giovannipds
Copy link

Thanks @fernandobandeira, solved it quickly thanks to you. =) Cheers!

@ghost
Copy link

ghost commented Jan 16, 2017

Thanks @fernandobandeira

@chadlinden
Copy link

What is the context for this change ?laravel/laravel@f237656

@giovannipds
Copy link

@chadlinden ask @Adam14Four.

@Adam14Four
Copy link

To be completely honest, I don't remember exactly what the details were, but it was some sort of data-loss problem. With strict mode disabled it was possible for operations to fail silently and end up with incorrect data stored in the database with no error or warning to the calling code.

It was determined that the default should be set to the safest setting since it is easy for devs to change if necessary. However, I would recommend leaving it on and updating code to work with strict mode enabled rather than just disable it.

@fernandobandeira
Copy link
Contributor

I've updated my comment to add more info about this mode,

I recommend that if you encounter this issue you should check which mode is causing your issue and then decide if you should leave it on or not,

@chadlinden
Copy link

Thanks @fernandobandeira & @Adam14Four

@sparkison
Copy link

@fernandobandeira thanks for the tip! Helped me as well ;)
Was running into this issue using Eloquence full text search on related models

@emadzz
Copy link

emadzz commented Jan 30, 2017

I'm having a similar issue, but in a different project than Laravel (it uses PDO). And it seems that it only raises the error when there are no matched records found. I'm still looking into this, but thought that it might help to solve the main problem, rather than disabling the strict mode.

@raylight75
Copy link

Same, I'm having a similar issue too changing config/database.php strict to false did the job,

@andyunleashed
Copy link

Yep changing strict to false fixed my error also, thanks @fernandobandeira

@gracefullight
Copy link

@fernandobandeira you saved my life

@taufiqnugraha
Copy link

go to folder > config/database.php > set 'strict' => false,

@TakesTheBiscuit
Copy link

Just caught me out! ended up just adding to GROUP BY like @nikocraft on #14997 (comment)

@jfhernandeze
Copy link

This actually worked for me in L5.5 go to folder > config/database.php > set 'strict' => false,

@sdkcodes
Copy link

sdkcodes commented Jun 4, 2018

I changed config/database.php 'strict' to false, but now, no records are returned

@justRau
Copy link

justRau commented Jul 13, 2018

If you want to leave strict checking on, but remove the check for ONLY_FULL_GROUP_BY, Illuminate\Database\Connectors\MySqlConnector should be overridden. It is initiated (and hard-coded) in Illuminate\Database\Connectors\ConnectionFactory, therefore, this class should also be overridden. Fortunately, it is bound to container as db.factory and we can change the binding.

Full instructions:

  1. Create App\Database\Connectors\MySqlConnector that extends Illuminate\Database\Connectors\MySqlConnector and override method strictMode - remove ONLY_FULL_GROUP_BY from the return statements.
  2. Create App\Database\Connectors\ConnectionFactory that extends Illuminate\Database\Connectors\ConnectionFactory and override method createConnector - make sure case 'mysql': returns an instance of your MySqlConnector class created in step 1.
  3. Create App\Providers\DatabaseServiceProvider:
    /**
     * Register services.
     *
     * @return void
     */
    public function register()
    {
        $this->registerConnectionServices();
    }

    /**
     * Register the primary database bindings.
     *
     * @return void
     */
    protected function registerConnectionServices()
    {
        // The connection factory is used to create the actual connection instances on
        // the database. We will inject the factory into the manager so that it may
        // make the connections while they are actually needed and not of before.
        $this->app->singleton('db.factory', function ($app) {
            return new ConnectionFactory($app);
        });
    }

and register it in config/app.php.

@Dapshos
Copy link

Dapshos commented Jan 1, 2019

@fernandobandeira
Hi
I did your orders
but . not work's
++++

@JYvman
Copy link

JYvman commented Feb 27, 2022

Hi, initially i had done strict => false, and it worked. When i changed i tried it in a different file, the same code does not work. What could be the problem?

@giovannipds
Copy link

@JYvman I do not know, please try to improve your question giving more context, versions used, repo for testing, etc. Also try to keep your versions up-to-date as much as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests