DDC-1043: Unit of Work - computeChangeSet #1302

New Issue

admin · 2026-01-22T13:09:02+01:00

admin commented

2026-01-22 13:09:02 +01:00

Originally created by @doctrinebot on GitHub (Feb 25, 2011).

Originally assigned to: @beberlei on GitHub.

Jira issue originally created by user vigor_bg:

In the unit of work in the compute Change Set function. When comparing the old and the new value of a field. In the case 'Entity is "fully" MANAGED'. And the values are strings it is used

$orgValue != $actualValue

comparison operator instead of

$orgValue !== $actualValue

Which lead to not detecting some changes. I know it is not something major, but it is really annoying. So please fix it for the next release.

Thanks in advance. :)

Originally created by @doctrinebot on GitHub (Feb 25, 2011). Originally assigned to: @beberlei on GitHub. Jira issue originally created by user vigor_bg: In the unit of work in the compute Change Set function. When comparing the old and the new value of a field. In the case 'Entity is "fully" MANAGED'. And the values are strings it is used ``` $orgValue != $actualValue ``` comparison operator instead of ``` $orgValue !== $actualValue ``` Which lead to not detecting some changes. I know it is not something major, but it is really annoying. So please fix it for the next release. Thanks in advance. :)

admin added the Bug label 2026-01-22 13:09:02 +01:00

admin closed this issue

2026-01-22 13:09:03 +01:00

admin commented

2026-01-22 13:09:04 +01:00

@doctrinebot commented on GitHub (Feb 25, 2011):

Comment created by @beberlei:

Can you give an example? The ones with null are already catched with the second part, so i cannot think of others.

($orgValue === null ^ $actualValue === null)

@doctrinebot commented on GitHub (Feb 25, 2011): Comment created by @beberlei: Can you give an example? The ones with null are already catched with the second part, so i cannot think of others. ``` ($orgValue === null ^ $actualValue === null) ```

admin commented

2026-01-22 13:09:04 +01:00

@doctrinebot commented on GitHub (Feb 26, 2011):

Comment created by vigor_bg:

well in my case I want to save a field with country code with lets say something like '+44'. But then i want to change it to just '44' it doesn't catch the difference and the column in the DB is varchar and in the entity it is set as string so it must use the strict comparison operator.

@doctrinebot commented on GitHub (Feb 26, 2011): Comment created by vigor_bg: well in my case I want to save a field with country code with lets say something like '+44'. But then i want to change it to just '44' it doesn't catch the difference and the column in the DB is varchar and in the entity it is set as string so it must use the strict comparison operator.

admin commented

2026-01-22 13:09:05 +01:00

@doctrinebot commented on GitHub (Feb 26, 2011):

Comment created by @beberlei:

What the fuck, i didn't know PHP had THIS strange conversion rules. The problem with using === is that for integer columns for example "44" vs 44 would determine a change and I cannot check for "string" column yes no at that point because this bit of code is very important for performance of UnitOfWork::commit().

Can we come up with a universal condition that detects this string change and keeps the comparison of all others equal?

@doctrinebot commented on GitHub (Feb 26, 2011): Comment created by @beberlei: What the fuck, i didn't know PHP had THIS strange conversion rules. The problem with using === is that for integer columns for example "44" vs 44 would determine a change and I cannot check for "string" column yes no at that point because this bit of code is very important for performance of UnitOfWork::commit(). Can we come up with a universal condition that detects this string change and keeps the comparison of all others equal?

admin commented

2026-01-22 13:09:06 +01:00

@doctrinebot commented on GitHub (Feb 27, 2011):

Comment created by vigor_bg:

To be honest I didn't know about the int problem I will think on it if I can find a solution I will tell you straight away.

@doctrinebot commented on GitHub (Feb 27, 2011): Comment created by vigor_bg: To be honest I didn't know about the int problem I will think on it if I can find a solution I will tell you straight away.

admin commented

2026-01-22 13:09:06 +01:00

@doctrinebot commented on GitHub (Mar 2, 2011):

Comment created by vigor_bg:

Well the best that i could come up with is

                else if((is_string($orgValue) && $orgValue !== $actualValue)
                            || ($orgValue === null <sup> $actualValue === null)) {
                    $changeSet[$propName] = array($orgValue, $actualValue);
                } else if ((!is_string($orgValue) && $orgValue != $actualValue)
                            || ($orgValue === null </sup> $actualValue === null)) {
                    $changeSet[$propName] = array($orgValue, $actualValue);
                }

Logically depending on the type of the old value to decide which operator to use as the old value was type cast by the DBAL Type class so we can depend on it.

@doctrinebot commented on GitHub (Mar 2, 2011): Comment created by vigor_bg: Well the best that i could come up with is ``` else if((is_string($orgValue) && $orgValue !== $actualValue) || ($orgValue === null <sup> $actualValue === null)) { $changeSet[$propName] = array($orgValue, $actualValue); } else if ((!is_string($orgValue) && $orgValue != $actualValue) || ($orgValue === null </sup> $actualValue === null)) { $changeSet[$propName] = array($orgValue, $actualValue); } ``` Logically depending on the type of the old value to decide which operator to use as the old value was type cast by the DBAL Type class so we can depend on it.

admin commented

2026-01-22 13:09:07 +01:00

@doctrinebot commented on GitHub (Apr 4, 2011):

Comment created by @beberlei:

Maybe we really have to change this:

diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php
index 682e906..8f68c3e 100644
--- a/lib/Doctrine/ORM/UnitOfWork.php
<ins></ins><ins> b/lib/Doctrine/ORM/UnitOfWork.php
@@ -467,9 </ins>467,7 @@ class UnitOfWork implements PropertyChangedListener
                     }
                 } else if ($isChangeTrackingNotify) {
                     continue;
-                } else if (is_object($orgValue) && $orgValue !== $actualValue) {
-                    $changeSet[$propName] = array($orgValue, $actualValue);
-                } else if ($orgValue != $actualValue || ($orgValue === null ^ $actualValue === null)) {
+                } else if ($orgValue !== $actualValue) {
                     $changeSet[$propName] = array($orgValue, $actualValue);
                 }
             }

Rather have more changes, which would be the developers need to take care of then ignoring these tricky casting issues.

@doctrinebot commented on GitHub (Apr 4, 2011): Comment created by @beberlei: Maybe we really have to change this: ``` diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 682e906..8f68c3e 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php <ins></ins><ins> b/lib/Doctrine/ORM/UnitOfWork.php @@ -467,9 </ins>467,7 @@ class UnitOfWork implements PropertyChangedListener } } else if ($isChangeTrackingNotify) { continue; - } else if (is_object($orgValue) && $orgValue !== $actualValue) { - $changeSet[$propName] = array($orgValue, $actualValue); - } else if ($orgValue != $actualValue || ($orgValue === null ^ $actualValue === null)) { + } else if ($orgValue !== $actualValue) { $changeSet[$propName] = array($orgValue, $actualValue); } } ``` Rather have more changes, which would be the developers need to take care of then ignoring these tricky casting issues.

admin commented

2026-01-22 13:09:07 +01:00

@doctrinebot commented on GitHub (May 1, 2011):

Comment created by @beberlei:

Fixed, but only merged into 2.1 / master so people can test this more.

@doctrinebot commented on GitHub (May 1, 2011): Comment created by @beberlei: Fixed, but only merged into 2.1 / master so people can test this more.

admin commented

2026-01-22 13:09:08 +01:00

@doctrinebot commented on GitHub (May 1, 2011):

Issue was closed with resolution "Fixed"

@doctrinebot commented on GitHub (May 1, 2011): Issue was closed with resolution "Fixed"

admin referenced this issue