extjac's avatar

Avoiding N+1

I am having a bit of a challenge with N+1. I am trying to calculate the balance of a member but I am not able to avoid the N+1 issues. This is basically a table with all the members and i am trying to show many things, and one of those things is the outstanding balance.

// model
// Member.php 
	
    public function orders()
    {
        return $this->hasMany(Order::class);
    }
   
  //Paymetns, Discounts, Refunds
    public function transactions()
    {
        return $this->hasMany(Order::Transaction );
    }    

    public function sumOrders()
    {
        return $this->orders->sum('amount');
    }    

    public function sumPayments()
    {
        return $this->transactions->where('type', 'payment')->sum('amount');
    }  

    public function sumDiscounts()
    {
        return $this->transactions->where('type', 'discount')->sum('amount');
    }  

    public function balance()
    {
        return $this->sumOrders() - $this->sumPayments() - $this->sumDiscounts() ;
    }  
// Contriller

public function index()
{
  
	$members = Member::query()->with(['transactions','orders'])->get();

	return view('members.index')->with('members', $members );

}

In my view would like to use something like below lets say in a table....

@foreach($members as $member )
    $member->balance();
@endforeach
    ~~~
0 likes
11 replies
Glenn 's avatar
public function index()
{
	return view('memebers.index')->with([
       'members',
       'members.balance', //<< Will load balance with members
       'orders',
       'transactions'
]);
}

or

public function index()
{
	return view('memebers.index')->with([
       'members' => function($query){
          return  $query->with['bakance']
      },
       'orders',
       'transactions'
]);
}
Glenn 's avatar

@extjac

What about at the beginning of your Report, Running a query to sum all payment transactions in a single query, then save as into array keyed by the member-id, then pass that to the blade, In your blade, just lookup the array - $payments[$member->memberID] - with appropriate isset checks for missing keys in the array

Off the top of my head... $payments = Tranactions::where('type', 'payment')->sum('amount')->groupBy('memberID')->keyBy('memberID')->ToArray()

[ memberID => sum, memberID => sum, memberID => sum, ]

Run for each Orders, Payments, Discounts

Or even map into members in PHP

$members = $members->map(function($member) use ($prders, $payments, $discounts) { $members->calcBalance = $orders[$member->memberID] - $payments[$member->memberID] return $memebers })

SilenceBringer's avatar

@extjac you need to load the data before attaching it to the view

public function index()
{
	$members =  Member::with('orders', 'transactions', 'balance')->get();
	return view('memebers.index', compact('members'));
}

when you're calling with to the view - it just attach the data to the view. When you're calling with on Eloquent/QueryBuilder - it uses eager loading to avoid N+1 problem

and access the data like

@foreach ($members as $member)
	// $member->orders
	// $member->balance
@endforeach
kokoshneta's avatar

@SilenceBringer Eh? You don’t need to load the data before attaching it to the view. Whether you load the data, save it to a variable and then attach that variable to the view, or load the data when defining the array to attach to the view doesn’t make any difference.

Also, balance is not a relationship, so it won’t work with the with() function on the Eloquent builder.

extjac's avatar

@SilenceBringer sorry, forgot to add this to my example. Didnt copy/paste properly. I do it like below....

$members =  Member::with('orders', 'transactions')->get();

but when i put Balance

$members =  Member::with('orders', 'transactions', 'balance')->get();

will show an error because balance is not a relationship.

Call to a member function addEagerConstraints() on float
kokoshneta's avatar

I don’t understand how your code is running at all. It’s full of what looks like syntax errors and invalid method calls.

  • What is Order::Tranactions? Do you have a static constant called Tranactions in your Order model, which contains a class name? If you don’t, your tranactions() relationship will never load anything, but throw an error. I suspect it should say Tran(s?)action::class.
  • The with() call to the view() function (in your index() method) should have an array as its first argument, and is not meant to be chained. It looks like you’re trying to load the view with eager-loaded relationships on the members, but if so, your syntax is all wrong.
  • You use tranactions in some places and transactions elsewhere – the correct word is transactions, and I would suggest using that everywhere: in method and property names, in your model class name, in your database, etc.
  • Similarly, your view should be named members.index, not memebers.index.

Since your index method doesn’t actually eager-load anything on the models at all, naturally each call to $member->tran(s)actions will lazy-load the relationship, hence the N+1 issue.

Moreover, your sum methods all recalculate the sums every time you call them, which is inefficient. You should use accessors to cache the results so you can refer to them multiple times without recalculation:

Member model

namespace App\Models;
use Illuminate\Database\Eloquent\Casts\Attribute;

class Member extends Model {
	public function orders()
	{
		return $this->hasMany(Order::class);
	}

	public function transactions()
	{
		return $this->hasMany(Transaction::class);
	}

	public function sumOrders() : Attribute
	{
		return Attribute::make(fn() => $this->orders->sum('amount'));
	}    

	public function sumPayments() : Attribute
	{
		return Attribute::make(fn() => $this->transactions->where('type', 'payment')->sum('amount'));
	}

	public function sumDiscounts() : Attribute
	{
		return Attribute::make(fn() => $this->transactions->where('type', 'discount')->sum('amount'));
	}  

	public function balance() : Attribute
	{
		return Attribute::make(fn() => $this->sumOrders - $this->sumPayments - $this->sumDiscounts);
	}
}

Controller

public function index()
{
	return view('members.index')->with([
		'members' => Member::with('orders', 'transactions')->get()
	]);
}

And then in your view:

@foreach ($members as $member)
	$member->balance
@endforeach
extjac's avatar

@kokoshneta sorry. I didnt copy/paste properly. Was actually typing on the laracast editor rather copy/paste. I dont use Attribute; but thanks for the tip. The transactions can be payments, discounts, refunds. Everything works on the code, the issue is that when using the "Balance" in the view, it will do the N+1 because needs to call again the relationships. Anyway...thanks for your reply.

kokoshneta's avatar

@extjac No, balance does not need to load the relationship again. If it does that, it’s because the relationships are not eager-loaded to begin with, probably because there is an error in your code somewhere.

What happens if you do this in your controller:

public function index()
{
	$members = Member::query()->with(['transactions','orders'])->get();
	dd($members->first());
	return view('memebers.index')->with('members', $members );
}

If you look under relationships in the object in the data-dump, are both transactions and orders loaded and have contents?

extjac's avatar

@kokoshneta yes they load ok. If i remove the balance from the table, there is not N+1.

kokoshneta's avatar

@extjac Then there shouldn’t be if you run $model->balance() either. If the relationships are loaded, the balance function will simply retrieve them from the object, not reload them from the database.

Which ‘table’ do you mean?

Please or to participate in this conversation.