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

does not work with hybridrelations between mysql and mongodb relationships #30

Closed
vesper8 opened this issue May 10, 2017 · 11 comments
Closed

Comments

@vesper8
Copy link

vesper8 commented May 10, 2017

I know this is probably outside the scope of your project but I think it would be really cool if I could get it to work.

I'm using https://github.com/jenssegers/laravel-mongodb which supports hybrid relations between mysql and mongodb models.

I am able to do this no problem User::with('profile')->all();

Where User is a mysql model and Profile is a mongodb model

Furthermore your Filterable trait works out of the box on the mongodb model which is great and I am having a lot of enjoyment using it!

so $users = User::filter($request->get()) and $profiles = Profile::filter($request->get()) both work great.

But if I try to do $users = User::filter(['gender' => 'male'])->get(); then I get errors and the errors are basically showing that it is looking for the profile fields inside mysql instead of making those queries on the mongodb collection.

I've set up my models properly

My User model uses a UserFilter which has

    public function gender($value)
    {
        return $this->where('gender', $value);
    }

And my ProfileFilter has

    public function gender($value)
    {
        return $this->where('gender', $value);
    }

And I've confirmed that the gender function above DOES get called. But when it does I get this error

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 'exists ? and `user_id` <> ? and `main_picture` is not null and `gender` = ?)' at line 1 (SQL: select * from `users` where exists (select `user_id` from `profile` where `user_id` exists 1 and `user_id` <> 201 and `main_picture` is not null and `gender` = male))

Basically it's ignoring my HybridRelations setup and assuming that Profile is another mysql model.

The fact that your EloquentFilter DOES work on my mongodb model when used without a relation tells me that this can probably be made to work without too much difficulty.

Is there any chance you could add this functionality or put me in the right direction?

I have a feeling part of the answer might lie in https://github.com/jenssegers/laravel-mongodb/blob/master/src/Jenssegers/Mongodb/Eloquent/HybridRelations.php

Many thanks!

@Tucker-Eric
Copy link
Owner

Skimming over the other library it looks like the issue may be in the addHasWhere() method that library overwrites returns a whereIn() instead of a whereExists() or whereCountQuery().

What version of Laravel are you using?

Do you get the same error running the following without the filters?

User::whereHas('profile', function($query) {
    return $query->where('gender', 'male');
})->get();

@vesper8
Copy link
Author

vesper8 commented May 11, 2017

I think you're on the right track! I'm using Laravel 5.4. And yes I do get the same error running the code you supplied

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 'exists ? and `gender` = ?)' at line 1 (SQL: select * from `users` where exists (select `user_id` from `profile` where `user_id` exists 1 and `gender` = male))

@Tucker-Eric
Copy link
Owner

It looks like they should override the has() method instead because that's where the exists constraint is being used. That needs to be a whereIn() or nested whereIn() if using whereHas() instead. If that is done correctly the filters would work between databases. All non-joined relations in the filters are queried using the whereHas() method which in turn uses the has method under the hood.

@vesper8
Copy link
Author

vesper8 commented May 13, 2017

well.. upon doing a little more digging on the very popular package https://github.com/jenssegers/laravel-mongodb/issues?utf8=%E2%9C%93&q=wherehas

It looks like having the "wherehas" working properly for hybrid relations has been a repeatedly requested feature going all the way back to 2014.. and the owner himself has said that implementing this seems very difficult.. and people continue to ask for it.. myself included.

I think unfortunately it must not be easy to make it work at all.. or in any case no one seems to know how to do it. If you think you have an idea of how to make it work and wanted to give it a try at adding a PR there.. I think that would make you a real hero ;)

@Tucker-Eric
Copy link
Owner

I threw together a rough patch for it. You wanna give it a test spin before I submit the pr?

https://github.com/Tucker-Eric/laravel-mongodb/commit/266305caeed70cf8ee787bcd0140c9fca2f0bcd6

@vesper8
Copy link
Author

vesper8 commented May 16, 2017

absolutely! will test it today and let you know if I run into any troubles.

wow thanks for taking this on!! I will support your PR as I'm sure others will.. it's been a long time coming!

Does using this branch also automagically makes your EloquentFilter work on mysql->mongo hybrid relations?

@Tucker-Eric
Copy link
Owner

Yup! Although I haven't tested the 2 together, there isn't anything that should break it in this branch. This will also work mongo->mysql relationships too. It just won't support belongsToMany() because that's​ queried with an inner join.

@vesper8
Copy link
Author

vesper8 commented May 17, 2017

Alright alright! I pulled in your branch and am glad I did because I found a problem.

On the one hand, this query which used to fail now works!

                    $query = User::whereHas('profile',
                        function ($query) {
                            $query->where('someBoolField', true);
                        })->with([
                        'profile' => function ($query) use ($profileFields) {
                            $query->select($profileFields);
                        },
                    ]);

So that's a win! Awesome!

On the other hand, this query which worked before, now fails!

        $this->data['matches'] = User::whereHas('matches',
            function ($query) {
                $query->where('user_id_1', \Auth::id())
                    ->orwhere('user_id_2', \Auth::id());
            })->with([
            'profile' => function ($query) use ($profileFields) {
                $query->select($profileFields);
            },
        ])->get($userFields)
            ->except(\Auth::id());

Where User and Match are mysql models and Profile is a mongodb model

So it seems that your fix has corrected the mysql->whereHas(mongodb)->with(mongodb) but is now interfering with the scenario mysql->whereHas(mysql)->with(mongodb)

this is my user model in case it's helpful

<?php

namespace App;

use EloquentFilter\Filterable;
use Jenssegers\Mongodb\Eloquent\HybridRelations;
use Laravel\Spark\User as SparkUser;

class User extends SparkUser
{
    use HybridRelations, Filterable;

    public function modelFilter()
    {
        return $this->provideFilter(\Omitted\Models\Filters\UserFilter::class);
    }

    public function profile()
    {
        return $this->hasOne('\Omitted\Models\Profile');
    }

    public function matches()
    {
        return $this->hasMany('\Omitted\Models\Match', 'user_id_1');
    }
}

Let me know if you need me to provide more info

@Tucker-Eric
Copy link
Owner

Ok, it should be good. I added some tests for hybrid whereHas with and everything is passing as of this commit. https://github.com/Tucker-Eric/laravel-mongodb/commit/818d7f66e1468b4c5ba8601474d585a082958dbf

@vesper8
Copy link
Author

vesper8 commented May 18, 2017

Seems to work beautifully!!

Tried the eloquent filter as well


                    $input = [
                        'gender' => 'female',
                    ];

                    $users = User::filter($input)->with('profile');

That works perfectly too!

And re-tried the two scenarios that I mentioned above and they now both work as expected!

That's some really fantastic work you've done here! Thank you so much! I'll be using your branch for now.

I hope @jenssegers approves your PR quickly! I know many of us have been waiting for has() and whereHas() to be added to his library for a long time so you're going to make a lot of happy people with this!

You rock :)

@Tucker-Eric
Copy link
Owner

I'm going to close this for now. We can reopen if there are issues with that pr.

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

2 participants