jarektkaczyk / eloquence

Extensions for the Eloquent ORM
http://softonsofa.com
MIT License
1.09k stars 142 forks source link

ManyToMany Relationship search wrong type used #246

Closed craax closed 5 years ago

craax commented 5 years ago

Hello,

I seem to have noticed a bug in the 5.6.3 version:

I have a table named "Addresses" and other tables named "Pages","Associations","Resellers", "Posts", etc. Events, associations, resellers, posts can have the same address, but also can have several addresses, so we're in a n/n relationship. The table "Localizables" will help us in that way, with these columns: "address_id", "localizable_id" (id of Association/event/reseller or post) and "localizable_type" (default laravel values: "App\Model\Event", "App\Model\Association", "App\Model\Reseller", "App\Model\Post") Note: here my models are in the Model folder (by default they're in App folder)

Address model has these relationships:

// App\Model\Address.php
public function pages()
    {
        return $this->morphedByMany(Page::class, 'localizable');
    }

    public function resellers()
    {
        return $this->morphedByMany(Reseller::class, 'localizable');
    }

    public function posts()
    {
        return $this->morphedByMany(Post::class, 'localizable');
    }

    public function associations()
    {
        return $this->morphedByMany(Association::class, 'localizable');
    }

Page, Reseller, Association and Post models share a "Localizable" trait granting them the following relationship:

<?php
// App\Traits\Localizable.php
namespace App\Traits;

use Illuminate\Http\Request;
use App\Model\Address;

trait Localizable
{
    public function address()
    {
        return $this->morphToMany(Address::class, 'localizable');
    }
}

Note: this relationship should be called "addresses" instead of "address" as it gives a collection of models, but that would mean a lot of refactoring so i'm keeping it that way for now.

-- Now, i'm trying to search on addresses fields, but also on the fields of linked objects to addresses , i use the Eloquence trait on "Address" model, and searchableColumns property:

// App\Model\Address.php
protected $searchableColumns = [
        'name' => 30,
        'street' => 10,
        'zip' => 5,
        'city' => 5,
        'resellers.name' => 10,
        'resellers.description' => 8,
        'associations.name' => 10,
        'associations.summary' => 9,
        'associations.description' => 8,
        'events.title' => 9,
        'events.summary' => 8,
        'events.content' => 7,
        'posts.title' => 9,
        'posts.summary' => 8,
        'posts.content' => 7,
        'pages.title' => 9,
        'pages.summary' => 8,
        'pages.content' => 7,
    ];

Here's the obtained SQL request on a $addresses->search($term) with $term == 'danse':

Version 5.6.3 :

select count(*) as aggregate from (
    select `addresses`.*, max(
        case when `addresses`.`name` = 'danse' then 450 else 0 end + 
        case when `addresses`.`name` like 'danse%' then 150 else 0 end + 
        case when `addresses`.`name` like '%danse%' then 30 else 0 end + 
        case when `addresses`.`street` = 'danse' then 150 else 0 end + 
        case when `addresses`.`street` like 'danse%' then 50 else 0 end + 
        case when `addresses`.`street` like '%danse%' then 10 else 0 end + 
        case when `addresses`.`zip` = 'danse' then 75 else 0 end + 
        case when `addresses`.`zip` like 'danse%' then 25 else 0 end + 
        case when `addresses`.`zip` like '%danse%' then 5 else 0 end + 
        case when `addresses`.`city` = 'danse' then 75 else 0 end + 
        case when `addresses`.`city` like 'danse%' then 25 else 0 end + 
        case when `addresses`.`city` like '%danse%' then 5 else 0 end + 
        case when `resellers`.`name` = 'danse' then 150 else 0 end + 
        case when `resellers`.`name` like 'danse%' then 50 else 0 end + 
        case when `resellers`.`name` like '%danse%' then 10 else 0 end + 
        case when `resellers`.`description` = 'danse' then 120 else 0 end + 
        case when `resellers`.`description` like 'danse%' then 40 else 0 end + 
        case when `resellers`.`description` like '%danse%' then 8 else 0 end + 
        case when `associations`.`name` = 'danse' then 150 else 0 end + 
        case when `associations`.`name` like 'danse%' then 50 else 0 end + 
        case when `associations`.`name` like '%danse%' then 10 else 0 end + 
        case when `associations`.`summary` = 'danse' then 135 else 0 end + 
        case when `associations`.`summary` like 'danse%' then 45 else 0 end + 
        case when `associations`.`summary` like '%danse%' then 9 else 0 end + 
        case when `associations`.`description` = 'danse' then 120 else 0 end + 
        case when `associations`.`description` like 'danse%' then 40 else 0 end + 
        case when `associations`.`description` like '%danse%' then 8 else 0 end + 
        case when `events`.`title` = 'danse' then 135 else 0 end + 
        case when `events`.`title` like 'danse%' then 45 else 0 end + 
        case when `events`.`title` like '%danse%' then 9 else 0 end + 
        case when `events`.`summary` = 'danse' then 120 else 0 end + 
        case when `events`.`summary` like 'danse%' then 40 else 0 end + 
        case when `events`.`summary` like '%danse%' then 8 else 0 end + 
        case when `events`.`content` = 'danse' then 105 else 0 end + 
        case when `events`.`content` like 'danse%' then 35 else 0 end + 
        case when `events`.`content` like '%danse%' then 7 else 0 end + 
        case when `posts`.`title` = 'danse' then 135 else 0 end + 
        case when `posts`.`title` like 'danse%' then 45 else 0 end + 
        case when `posts`.`title` like '%danse%' then 9 else 0 end + 
        case when `posts`.`summary` = 'danse' then 120 else 0 end + 
        case when `posts`.`summary` like 'danse%' then 40 else 0 end + 
        case when `posts`.`summary` like '%danse%' then 8 else 0 end + 
        case when `posts`.`content` = 'danse' then 105 else 0 end + 
        case when `posts`.`content` like 'danse%' then 35 else 0 end + 
        case when `posts`.`content` like '%danse%' then 7 else 0 end + 
        case when `pages`.`title` = 'danse' then 135 else 0 end + 
        case when `pages`.`title` like 'danse%' then 45 else 0 end + 
        case when `pages`.`title` like '%danse%' then 9 else 0 end + 
        case when `pages`.`summary` = 'danse' then 120 else 0 end + 
        case when `pages`.`summary` like 'danse%' then 40 else 0 end + 
        case when `pages`.`summary` like '%danse%' then 8 else 0 end + 
        case when `pages`.`content` = 'danse' then 105 else 0 end + 
        case when `pages`.`content` like 'danse%' then 35 else 0 end + 
        case when `pages`.`content` like '%danse%' then 7 else 0 end
    ) as relevance from `addresses` 
    left join `localizables` on `localizables`.`address_id` = `addresses`.`id` 
    left join `resellers` on `localizables`.`localizable_id` = `resellers`.`id` and `localizable_type` = 'App\Model\Address' 
    left join `associations` on `localizables`.`localizable_id` = `associations`.`id` and `localizable_type` = 'App\Model\Address' 
    left join `events` on `localizables`.`localizable_id` = `events`.`id` and `localizable_type` = 'App\Model\Address' 
    left join `posts` on `localizables`.`localizable_id` = `posts`.`id` and `localizable_type` = 'App\Model\Address' 
    left join `pages` on `localizables`.`localizable_id` = `pages`.`id` and `localizable_type` = 'App\Model\Address' where (
        `addresses`.`name` like '%danse%' or
         `addresses`.`street` like '%danse%' or
         `addresses`.`zip` like '%danse%' or
         `addresses`.`city` like '%danse%' or
         `resellers`.`name` like '%danse%' or
         `resellers`.`description` like '%danse%' or
         `associations`.`name` like '%danse%' or
         `associations`.`summary` like '%danse%' or
         `associations`.`description` like '%danse%' or
         `events`.`title` like '%danse%' or
         `events`.`summary` like '%danse%' or
         `events`.`content` like '%danse%' or
         `posts`.`title` like '%danse%' or
         `posts`.`summary` like '%danse%' or
         `posts`.`content` like '%danse%' or
         `pages`.`title` like '%danse%' or
         `pages`.`summary` like '%danse%' or
         `pages`.`content` like '%danse%'
     ) group by `addresses`.`id`
) as `addresses` where 
    (
        (exists (select * from `associations` inner join `localizables` on `associations`.`id` = `localizables`.`localizable_id` where `addresses`.`id` = `localizables`.`address_id` and `localizables`.`localizable_type` = 'App\Model\Association' and `associations`.`published` = 1) or
         exists (select * from `resellers` inner join `localizables` on `resellers`.`id` = `localizables`.`localizable_id` where `addresses`.`id` = `localizables`.`address_id` and `localizables`.`localizable_type` = 'App\Model\Reseller') or
         exists (select * from `pages` inner join `localizables` on `pages`.`id` = `localizables`.`localizable_id` where `addresses`.`id` = `localizables`.`address_id` and `localizables`.`localizable_type` = 'App\Model\Page' and `pages`.`published` = 1)) or
         (not exists (select * from `associations` inner join `localizables` on `associations`.`id` = `localizables`.`localizable_id` where `addresses`.`id` = `localizables`.`address_id` and `localizables`.`localizable_type` = 'App\Model\Association') and not exists (select * from `resellers` inner join `localizables` on `resellers`.`id` = `localizables`.`localizable_id` where `addresses`.`id` = `localizables`.`address_id` and `localizables`.`localizable_type` = 'App\Model\Reseller') and not exists (select * from `pages` inner join `localizables` on `pages`.`id` = `localizables`.`localizable_id` where `addresses`.`id` = `localizables`.`address_id` and `localizables`.`localizable_type` = 'App\Model\Page'))
    ) and `relevance` >= 41.75

In the SQL request, the left joins part seems wrong to me. We should have localizable_type = 'Post' or 'Page' or 'Reseller' or 'Association' here.

I also noticed that in 5.6.2 version, the given SQL request is the same without the localizable_type conditions.

So i am staying on the 5.6.2 version at the moment as I'd rather have more results (good and wrong ones) than none.

Thank you !

P.S: Please forgive my english

jarektkaczyk commented 5 years ago

In general I'd suggest using search services or custom algos for such cases, while searchable only for simple use-cases.