HUGE BUG: update() function updates all records if id is empty

[thewhistler1]

Example code:

$params = array(
     'guid' => 12345
);
$employerModel->update($employer_id,$params);

The variable $employer_id was null for whatever reason and so the resulting query was this:

update employers set guid = '12345', updated_at = '2021-04-26 06:34:13';

This resulted in the function updating ALL records instead of no records.

CodeIgniter version: 4.1.1

Expected behavior -
I would expect the same behavior that results when you write the query manually:
update employers set guid = '12345', updated_at = '2021-04-26 06:34:13' WHERE employer_id = '';

The above query would not update any records unless you indeed had a employer with a null id. So I think the appropriate outcome would be to not update any records.

Context

• OS: Ubuntu 20.04
• Web server Apache/2.4.41 (Ubuntu)
• PHP version PHP 7.4.3

[musmanikram]

Indeed it's a HUGE bug and by mistake, but I assume it's intentional to update all records at once.
But on the other hand, if some passes a variable with a null value by mistake can mess up the whole table.

To make it defensive I think it should throw an exception. Will create a PR and let see what others think about this.

[iRedds]

This is not a bug. You want the framework to read your mind.

The update($id, $data) method of the model is equivalent to code

->whereIn('primary_key', [$id])->set($data)->update();

The code was made universal. Therefore, when $id = null, the condition is not added so that all records can be updated.

Your expected behavior will break working code for other developers.

You can rewrite the update method to achieve the behavior you want.
Or add conditions to the request like

->whereIn('employer_id', [$employer_id])->update(null, $params);

Or add a condition to the code

if ($employer_id !== null)
{
    ->update($employer_id, $params);
}

[musmanikram]

@thewhistler1 I think @paulbalandan have a pretty much summarised why it's not a bug.
Please read this .

@paulbalandan Thanks again mate

[lonnieezell]

I would also point out that is what regression tests are for - keeping us from shooting ourselves in the foot with dangerous code :)

This is not a code as pointed out, and is functioning as intended. Closing.