Database driver and ORM Model update() inconsistency

[WanWizard]

I noticed that the MySQL and MySQLI drivers query() method return affected_rows() after execution, which means

• a value of -1 when the query had an error
• a value of 0 when the query was ok but no rows were affected
• or the number of affected rows.

The PDO driver only returns rowCount(), so no indication if the query executed correctly or not.

Then, the ORM Model update() method uses this return value, and determines that the update was correct if the rowcount returned is greater than zero. Which in turn means that if no rows were affected, Model::save() returns false.

I don't think this is correct.

• the DB drivers should be made consistent, which means the PDO driver must return -1 in case of a query error
• the Model::update() method should only return false if the result of the query was -1

[jschreuder]

Was away most of today. No time to fully look into this tonight but it sounds logical. On the other hand, the affected row count from Model::update() should always be 1. If it's not something went wrong (Model::update() only updates the current instance, no others)

[WanWizard]

If you (with MySQL) fetch a record using Model::find(1) and then directly save it, affected rows is zero (update went ok, but no rows were updates since all values were the same).

If your controller contains a

if ($model->save())
{
    // success
}
else
{
    // failure
}

Your application will generate an error if the user requests a record, and clicks save without changing anything.

[jschreuder]

Actually, is_changed() is run before save() is executed and when no change is detected it will always return true

[WanWizard]

Issue is in typing. MySQL stores everything as string, so when you retrieve a record, all column values are strings, However, the fields on my forms and the validation routines behind it aren't, so an int column which was retrieved as "10", will be set to 10 (int!) before calling save(), and I assume that is_changed() will then return true, although for MySQL still nothing has changed.

[jschreuder]

I just changed Orm\Query::update() to return true when affected rows is 0. fuel/orm@42e17df6

[WanWizard]

Thanks. This can be closed then.