Save a cloned row updates the original one

[gpoehl]

I cloned a row, altered a value in a column and saved the cloned row.
Instead of inserting this row the original row was updated. Is this the expected behavior or is this a bug?

Calling the insert function also doesn't work because the columns in the new row aren't marked as dirty. So they will not be saved to the table.

$data = Kontoplan::find()->where(['mandantID'=>1])->all();
        $attr = array_keys(Kontoplan::getTableSchema()->columns);
        unset($attr[0]);
        foreach ($data as $row){
            $kontoplan = clone $row;
            $kontoplan->mandantID =$id;
            $kontoplan->save(false, $attr);
        }

[klimov-paul]

This is not surprising since all original rows attributes and fields including isNewRecord have being copied.
Still I am unsure: perhaps cloning of the Active Record instance should actually behave differently.
Lets wait for other opinions.

[lynicidn]

it depended from unique validators and primary keys

[samdark]

Primary key value is the same in cloned instance as in original so the behavior is expected.

[gpoehl]

The problem described above is twofold.

1. The original model has been updated instead of inserting the cloned one. This doesn't follow the PHP logic of an cloned object! The clone is an independent instance of the original. So saving a clone should not affect the original!

2. The attributes of the cloned model are not marked as dirty so that the values are not used
   even with the $model->insert() function. This follows PHP logic but makes imho not much sence for active records.
   How about an function to mark all columns dirty or implementing a copy() function or something similar?

@lynicidn, the primary key column (autoindexed) has been excluded from the list of attributes to be saved. I didn't made it very clear in my example. The unset command removes the key from the attributes array while the save(false, $attr) command says that only this limited number of attributes should be saved.

[samdark]

@twiNNi there's ID and it's not empty. Do you exect AR to ignore it?

[gpoehl]

@samdark, I did some more tests and figured out that the model update, insert and save statements for the a cloned model creates sql commands containing only changed attributes. The parameter 'attributes' within these statements is just an additional limitation and not the list of attributes you want to see in the sql statement!

As a cloned model has no changed attributes only my modified column $kontoplan->mandantID will be inserted or updated.

The decision between insert or update for an `savestatement is based on the value ofisNewRecord``. For cloned models it's the same as for the original (false in my case).

Now its clear why my first example $kontoplan->save(false, $attr); updates the original row. isNewRecordis false and the key, no matter if it is an element in the attributes parameter list, stays the same and will be used for the update statement.

Everything what happend can be explained but is far away from what one would expect.

I see some ways to solve the situation but all of them requires some design decisions. Maybe the sipliest one would be to see a clone as a new model and to set attributes like isNewRecord , old_attributesor dirty_attributesaccordingly.

Ingnoring of id's (I assume your're talking about keys) could make the clone process less transparent as there could by compound keys and unique indexes. I also don't see how this could solve the problem.

If you're not going to implement the suggest changes you should at least place a big warning into the doc's.

[samdark]

I don't think it's really expected behavior from PHP perspective, especially cleaning out primary key value.

@yiisoft/core-developers opinions?

[gpoehl]

@samdark, I haven't asked for cleanig primary key values but to treat a clone as a new data (model or row). Managing key values should be the task for the application developer. As mentioned before there might be compound keys and unique indizes as well which need also to be considered.

One additional comment: Assume you change the internal procedure how to figure out if a active record is a new one by reading the row (with modified keys) again from the database then the result is different.
I think that an application should be independent as much as possible from internal logic of the framework.

[cebe]

Cloning a record should be possible by doing the following:

// assuming  id  is the primary key attribute
$model = User::findOne(1);
$model->isNewRecord = true;
$model->id = null; // set to another value if it is not auto incremental key
$model->save();

[lynicidn]

hm

$user = User::findOne(1);
$model = new User([
    'attributes' => $user->getAttributes() //id not in rules or set params - getAttributes(['username', 'email?', 'other'])
]);
$model->save();

[lynicidn]

cloning with uniq columns it do bug's

[lynicidn]

as idea create saveAs($id = null) method

[gpoehl]

@cebe, this works perfect.
The alternative way, remove the primary key columns from the attribute list without setting a new value, works as well.

@lynicidn, your idea works also very well. Just the syntax is a bit different.

$user = User::findOne(1);
$model = new User(
    $user->getAttributes() // get all attributes and copy them to the new instance
);
$model->id = null;
$model->save();

Thanks for your answers.

[jogerq]

In the commen of "cebe" of 4 Mar 2015 he wrote:

Cloning a record should be possible by doing the following:

// assuming id is the primary key attribute
$model = User::findOne(1);
$model->isNewRecord = true;
$model->id = null; // set to another value if it is not auto incremental key
$model->save();"

For me this "$model->id = null" didn't work. I had to write unset($model->id) and then it worked