Fixed change sets of array updates #246
Conversation
| @@ -653,6 +653,8 @@ public function computeChangeSet(ClassMetadata $class, $document) | |||
| } | |||
| } else if (is_object($orgValue) && $orgValue !== $actualValue) { | |||
| $changeSet[$propName] = array($orgValue, $actualValue); | |||
| } else if (is_array($orgValue) && is_array($actualValue) && ($diff = array_diff($actualValue, $orgValue))) { | |||
There was a problem hiding this comment.
Is this the best possible check we can do here? Are their any other situations we need to account for?
There was a problem hiding this comment.
maybe we can check with !==.
The problem is if i have a array with 'foo' => 'bar' and i will change this to 'foo' => true the != operator doesn't work.
|
Duplicate of #209. The solution there looks more complex but I am not sure it is needed. |
|
i think we should support better array updates, because in mongo we can update one single field in an array, but currently if i use the type |
|
@tecbot that is what we have @EmbedMany for. PersistentCollection wraps around the array and tracks what is actually being changed. With just a plain old PHP array we have no introspection in to what has actually changed in the array. We can make the array diff or whatever we use more intelligent but I don't think it can ever be perfect. |
|
@jwage yes i now, but if i want to store only a simple string in an array then i need to create a class like "StringEmbeddedDocument" and configure mappings, IMO its a overhead. I think we should support a more intelligent array update but not recursive. |
|
We can add support for it, but it should be optional. The default should be a basic array with no heavy comparison because the overhead of doing the complex array comparison will also have significant overhead. I'd be curious to see benchmarks and have it compared to the same thing as an EmbedMany. |
|
@jwage so what is the state of this pull request? can we merge this until we have found a better solution? |
|
Sure we can merge and revise. |
Fixed change sets of array updates
|
This commit broke the full test suite :( |
|
ping @tecbot can you run the full test suite with the change? |
all ok on my pc :). |
|
It is failing for PHP 5.4 http://travis-ci.org/#!/doctrine/mongodb-odm/jobs/737202 |
|
hmm this is very weird... Array to string conversion ?? wtf |
|
i have found that php 5.4 shows a notice now if you want to cast an array to string, so array_diff dosn't work for arrays in 5.4 if the values are an array, because he cast all entries to strings. |
|
I see, we need to come up with a more robust solution than array_diff. I don't know what exactly it is, but I know that array_diff won't work. |
|
I am going to revert this change until we have a better solution because the ODM is broken with php 5.4 as of right now. Unless you can send a PR to fix it today? |
|
That's for PHP 5.4 that I did not use array_diff in #209, implementing a "dirty check". |
|
@geraldcroes would you mind updating your PR and I will merge it? |
|
@jwage What do you wan't me to update in the PR ? (already fixed standards & stuff as requested) |
No description provided.