PHP - Save to database outside foreach loop

Question

I have below foreach loop, which iterates over a bunch of different custom rules my users can apply.

public function parse(Document $document)
{
        $content = $document->text;
        $rules = $document->stream->fieldrules;

        foreach ($rules as $rule) {
            $class = $this->getClass($rule);
            $content = $class->apply($content);

            //Save to database.
            $fieldresult = new FieldRuleResult();
            $fieldresult->create([
                'field_rule_id' => $rule->field_id,
                'document_id' => $document->id,
                'content' => $content
            ]);
        }
}

As you can see, I am calling the database on each iteration. Some users may have up to 50 rules defined, which will then result in 50 insert queries. So I believe I may have encountered a n+1 problem.

I was wondering - what is the best practice here? I've tried searching the Laravel documentation, and found the createMany method. So I tried to refactor the iteration to below:

$result = [];
foreach($rules as $rule)
{
  $class = $this->getClass($rule);
  $content = $class->apply($content);
  $fieldresult = new FieldRuleResult();

  $result[] = [
     'field_rule_id' => $rule->field_id, 
     'document_id' => $document->id, 
     'content' => $content];
}

return $rules->createMany($result);

Which gives me below error:

Method Illuminate\Database\Eloquent\Collection::createMany does not exist.

Now I imagine that is because $rules returns a collection. I tried to alter it to:

return $document->stream->fieldrules()->createMany($result);

Which gives me below error:

Call to undefined method Illuminate\Database\Eloquent\Relations\HasManyThrough::createMany()

maybe this will help stackoverflow.com/questions/12702812/… — shushu304
– shushu304, Commented Mar 26, 2019 at 19:09

oliverbj · Accepted Answer · 2019-03-27 07:32:25Z

3

There is a method available in the Illuminate/Database/Query/Builder named insert. This method allows you to insert many models in one query. Noted is that this will not, as all multiple in one query methods do, trigger eloquent events.

Like you have already done, store the data in an array and execute the query after your loop is done. However this time you could use insert.

$content = 'your content here...';

$data = [];

foreach($rules as $rule) {

    $class = $this->getClass($rule);

    $content = $class->apply($content);

    $data[] = [
        // ...
        'content' => $content
    ];
}

FieldRuleResult::insert($data);

Not 100% sure this works directly on the model. However in the documentation you can find an example with the DB facade.

edited Mar 27, 2019 at 7:32

oliverbj

6,11031 gold badges102 silver badges198 bronze badges

answered Mar 26, 2019 at 19:17

Thomas Van der Veen

3,2362 gold badges24 silver badges38 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

oliverbj Over a year ago

Thanks Thomas! However, is there a way where I can insert the data more elegantly (without the n+1 issue), and still fire off events?

The Alpha Over a year ago

I think, this is a good way to go. Before you prepare your $data just apply the filter.

Fabián Montero Rodríguez Over a year ago

As @TheAlpha comments, is only for use with the DB Facade. DB::table('table')->insert([ [$data1], [$data2] ]);

Thomas Van der Veen Over a year ago

@oliverbj We can fire eloquent events manually, butI am not sure if we should do this. However, depending on your needs, we can create a custom event and listener.

Pepijn Olivier · Accepted Answer · 2019-03-26 20:23:12Z

2

If Eloquent events need to be triggered for whichever reason, and you would like to optimize for the n+1 problem, I would suggest using a transaction. This will still yield n queries, but the database can optimize this via bulk insert (because of the transaction). It has the extra benefit of data consistency (either all inserts succeed, or none of them do).

$result = getResult();

\DB::transaction(function() use ($result) {
    foreach($result as $item) {
        FieldRuleResult::create($item);
    }
});

If you do not care about Eloquent events being triggered, and you just want to "bulk insert", you can use the insert method, as mentioned by Fabian. I would still advise to use a database transaction though, for data consistency.

$result = getResult();

\DB::transaction(function() use ($result) {
    FieldRuleResult::insert($result);
});

In this latter case, if the amount of entries you're about to insert can be "large", you might also want to chunk these inserts. (eg. 100 at a time, rather than all at once)

answered Mar 26, 2019 at 20:23

Pepijn Olivier

9761 gold badge19 silver badges32 bronze badges

2 Comments

oliverbj Over a year ago

Thanks, I will try with the transaction as adviced. Quick question: would it make sense to further move the actual DB insert into a job and then queue it?

Pepijn Olivier Over a year ago

Depending on your project, that might make sense. Especially if the database can be under heavy load, or there is a lot of data to be inserted in that job. Just be aware that having it queued in a job is usually async, and thus if the user can view his submissions somewhere, he might be confused as to why they aren't there (yet!)

Collectives™ on Stack Overflow

PHP - Save to database outside foreach loop

2 Answers 2

4 Comments

2 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

4 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related