How to refactor my nested 3 deep foreach loop

Question

I am using a laravel composer package that allows for a triple pivot table and it does not support batch inserts. So I am stuck looping through each array to make sure each id from the three tables have a match in my triple pivot table. What I have works but I would like some suggestions on how to refactor my function so I don't have to have so many nested loops. I was thinking about using an array map.

public static function addKeyPhraseToPivotTable($ids) {

    $locations = Location::where('account_id', self::$items[0]['account_id'])->get();
    $urls = Url::where('account_id', self::$items[0]['account_id'])->get();

    if($locations->count() > 0 && $urls->count() > 0) {
        foreach($ids as $keyPhraseId) {
            foreach($locations as $location) {
                foreach($urls as $url) {
                    $url->keywords()->attach([$keyPhraseId, $location->id]);
                }
            }
        }
    }
}

Answer 1

This is a tough issue to solve using Eloquent, maybe currently impossible. First as you found out, it doesn't support 3 way pivot tables out of the box. Second, the package you are using to add support looks like it will be continually attaching items to that record creating enormous amounts of data duplication in the database because there is no sync. And third, if you were to enforce unique records by adding a unique index on those fields in your table, there is no support for insert ignore queries.

With all that in mind, it might be useful to use raw sql for this. Since there is nothing crazy going on in the SQL however, I think it would be fine in this case.

First, you will want to enforce data integrity on the database level. The way you do this would depend on how your pivot table is already structured. If you already have an id column for this table which is an auto incrementing primary key, you would need to add a unique key to your 3 columns which contains the IDs of your 3 tables you are trying to link. If you don't have an id here or some other primary key already, you can set a composite primary key which contains those 3 columns.

It's possible this will fail if there are already duplicate records. All you should need to do is remove those duplicates and re-add your key.

With all that said, I'd then refactor your function so that it uses a bulk insert.

function addKeyPhraseToPivotTable($ids) {

    $locations = Location::where('account_id', self::$items[0]['account_id'])->get();
    $urls = Url::where('account_id', self::$items[0]['account_id'])->get();

    $deletes = [];
    if ($locations->count() > 0 && $urls->count() > 0) {
        foreach ($ids as $keyPhraseId) {
            foreach ($locations as $location) {
                foreach ($urls as $url) {
                    $deletes[] = [
                        'location' => $location->id,
                        'key' => $keyPhraseId,
                        'url' => $url->id, // Not sure what the ID is of this item, might need to change it.
                    ];
                }
            }
        }
    }

    $sql = 'insert ignore into accounts (location_id, key_phrase_id, url_id) values ';

    foreach ($deletes as $i => $delete) {
        $sql .= $i == 0 ? '' : ',';
        $sql .= sprintf('(%s, %s, %s)', $delete['location'], $delete['key'], $delete['url']);
    }

    DB::unprepared($sql);
}

This should try and insert each record you have and because you are using insert ignore, it will not break when trying to insert a duplicate.

I believe DB::unprepared() will return the number of rows which were created so you can get a good idea of how many items you tried to insert were duplicates if you needed that as well. If you add on duplicate key update however, keep in mind MySQL will return 2 rows affected for each row which was updated and not inserted so in that case, that number would probably be useless for you.