Framework: $instance->increment(<column>) increments <column> on all rows

[Obbi89]

• Laravel Framework Lumen (5.6.3) (Laravel Components 5.6.*)
• PHP Version: 7.2.6
• Database Driver & Version:
  
  
  
  • mysqlnd 5.0.12-dev - 20150407
  
  
  • mysqld Ver 10.3.7-MariaDB-1:10.3.7+maria~jessie for debian-linux-gnu on x86_64 (mariadb.org binary distribution)
  
  

Description:

Calls to increment will update all rows even though it is called on an instance.
One would either expect that it modifies that column on the instance directly or throw an Exception that increments cannot be run on non-persisted Instances and one should either
use save()/push() first to persist the model or increment the value by hand using $model->column++ or similar.

partially fixed in #890
see comment in last change.

Steps To Reproduce:

1. Create Model instance
2. Use increment before committing the model.

$model = new Model();
$model->increment('column');

Expected:

Increment happening on model instance or Exception.

Observed:

Increment is happening on column for every row.

[staudenmeir]

The line you commented on in the commit is necessary for static calls like Post::increment('views').

AFAICS, we could change that by adding this to Model::__callStatic() (taken from __call()):

if (in_array($method, ['increment', 'decrement'])) {
    return static::query()->$method(...$parameters);
}

Then we can provide a different functionality for non-persistent models. Throwing an exception makes the most sense to me.

[devcircus]

This is expected. The same could be said for any situation in which you call the query builder from an instance. I depend on this functionality for many apps, as I'm sure others do.

[Obbi89]

@devcircus You are using this?!
Why would you ever call a static method on an instance. That makes no sense to me.
Especially since calling increment on a model changes it's behavior after calling save()

meaning

$model = new Model();
$model->increment('column'); // changes all rows.
$model->save();
$model->increment('column'); // changes one row.

To me that is everything else than logical.

[OwenMelbz]

@themsaid any explanation as to why this has been closed? Would be good so people are aware why this happens and how to avoid the problem?

[themsaid]

It's closed because it's the expected behaviour, if you check the code inModel::incrementOrDecrement() you'll see that if no model exists on the instance the methods are run on an instance of Eloquent/Builder for that model which will affect all rows in the DB.

[OwenMelbz]

Hey @themsaid, Thanks for replying - Would this be the same for other methods?

let's say I was to do Model::touch() does every row in the database get touched?

Additionally what would be the work around to enable the inc/dec methods without it instantly hitting the database?

let's say you span up a model, various things happened which incremented a value before it was saved how would you achieve this? or could you simply not use the inc/dec methods?

Thanks :) I'm sure you can appreciated the benefits to understanding things fully!

[themsaid]

Nope increment() and decrement() are special in this case, in your case I'd just update the attributes manually without using increment and decrement, or persist too early before any operations.

I see your point though, but these methods are intended to be used in this way, maybe propose new methods via a PR?