DDC-280: UnitOfWork changeSet population should take advantage of Comparable technique #347

New Issue

admin · 2026-01-22T12:35:33+01:00

admin commented

2026-01-22 12:35:33 +01:00

Originally created by @doctrinebot on GitHub (Jan 27, 2010).

Originally assigned to: @Ocramius on GitHub.

Jira issue originally created by user @guilhermeblanco:

Currently our UnitOfWork computes the changeset by checking actual instances of Objects.

// UnitOfWork, lines 501-507 

            foreach ($actualData as $propName => $actualValue) { 
                $orgValue = isset($originalData[$propName]) ? $originalData[$propName] : null; 
                if (is_object($orgValue) && $orgValue !== $actualValue) { 
                    $changeSet[$propName] = array($orgValue, $actualValue); 
                } else if ($orgValue != $actualValue || ($orgValue === null <sup> $actualValue === null))  
                        $changeSet[$propName] = array($orgValue, $actualValue);
                }

While this is ok when you do new object assignments, it just bypass same instances of same object, since the hash is the same.
A user on IRC (post-o-matic) has a quite complex object logic that he would like to avoid clone and even instantiate another class.
I agree with him that cloning is not the ideal technique, mainly because the changeset would always compute the object (since then hashs would be different).

He implemented this datatype:

class EffortGraphType extends Type 
{ 
    public function getSqlDeclaration(array $fieldDeclaration, AbstractPlatform $platform) 
    { 
        return $platform->getClobTypeDeclarationSql($fieldDeclaration); 
    } 

    public function convertToPHPValue($value, AbstractPlatform $platform) 
    { 
        return new EffortGraph(unserialize($value)); 
    } 

    public function convertToDatabaseValue($value, AbstractPlatform $platform) 
    { 
        return serialize($value->getGraphPoints()); 
    } 

    public function getName() 
    { 
        return 'effort_graph'; 
    } 
}

I was thinking in a possible alternative and it came up to me the same basic idea we have with operators overloading OR Comparable interface of Java. I know in Java it supports way more things, but at least for this situation (as a start point) it would make developer's life easier.

Basic idea is to have an interface in Doctrine\Common:

namespace Doctrine\Common;

interface Comparable
{
    public function compareTo($value);
}

And update our UnitOfWork to take advantage of it:

// UnitOfWork, lines 501-507 

            foreach ($actualData as $propName => $actualValue) { 
                $orgValue = isset($originalData[$propName]) ? $originalData[$propName] : null; 

                if (is_object($orgValue)) { 
                    $isDiff = ($orgValue instanceof Doctrine\Common\Comparable) 
                        ? $orgValue->compareTo($actualValue) :  ($orgValue !== $actualValue);

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

In this user's usecase, it'd require him to update the EffortGraph class and implement Comparable interface.
For his specific situation, he'd need to store original value and updated value, just like we do internally in UnitOfWork for Entities.

Originally created by @doctrinebot on GitHub (Jan 27, 2010). Originally assigned to: @Ocramius on GitHub. Jira issue originally created by user @guilhermeblanco: Currently our UnitOfWork computes the changeset by checking actual instances of Objects. ``` php // UnitOfWork, lines 501-507 foreach ($actualData as $propName => $actualValue) { $orgValue = isset($originalData[$propName]) ? $originalData[$propName] : null; if (is_object($orgValue) && $orgValue !== $actualValue) { $changeSet[$propName] = array($orgValue, $actualValue); } else if ($orgValue != $actualValue || ($orgValue === null <sup> $actualValue === null)) $changeSet[$propName] = array($orgValue, $actualValue); } ``` While this is ok when you do new object assignments, it just bypass same instances of same object, since the hash is the same. A user on IRC (post-o-matic) has a quite complex object logic that he would like to avoid clone and even instantiate another class. I agree with him that cloning is not the ideal technique, mainly because the changeset would always compute the object (since then hashs would be different). He implemented this datatype: ``` php class EffortGraphType extends Type { public function getSqlDeclaration(array $fieldDeclaration, AbstractPlatform $platform) { return $platform->getClobTypeDeclarationSql($fieldDeclaration); } public function convertToPHPValue($value, AbstractPlatform $platform) { return new EffortGraph(unserialize($value)); } public function convertToDatabaseValue($value, AbstractPlatform $platform) { return serialize($value->getGraphPoints()); } public function getName() { return 'effort_graph'; } } ``` I was thinking in a possible alternative and it came up to me the same basic idea we have with operators overloading OR Comparable interface of Java. I know in Java it supports way more things, but at least for this situation (as a start point) it would make developer's life easier. Basic idea is to have an interface in Doctrine\Common: ``` php namespace Doctrine\Common; interface Comparable { public function compareTo($value); } ``` And update our UnitOfWork to take advantage of it: ``` php // UnitOfWork, lines 501-507 foreach ($actualData as $propName => $actualValue) { $orgValue = isset($originalData[$propName]) ? $originalData[$propName] : null; if (is_object($orgValue)) { $isDiff = ($orgValue instanceof Doctrine\Common\Comparable) ? $orgValue->compareTo($actualValue) : ($orgValue !== $actualValue); if ($isDiff) { $changeSet[$propName] = array($orgValue, $actualValue); } } else if ($orgValue != $actualValue || ($orgValue === null </sup> $actualValue === null)) $changeSet[$propName] = array($orgValue, $actualValue); } ``` In this user's usecase, it'd require him to update the EffortGraph class and implement Comparable interface. For his specific situation, he'd need to store original value and updated value, just like we do internally in UnitOfWork for Entities.

admin added the Improvement Won't Fix Can't Fix labels 2026-01-22 12:35:33 +01:00

admin closed this issue

2026-01-22 12:35:33 +01:00

admin commented

2026-01-22 12:35:35 +01:00

@doctrinebot commented on GitHub (May 19, 2010):

Comment created by @guilhermeblanco:

What's the final status of this?

IMHO this should be incorporated, since it adds a powerful support that users can take advantage in our Types.

Cheers,

@doctrinebot commented on GitHub (May 19, 2010): Comment created by @guilhermeblanco: What's the final status of this? IMHO this should be incorporated, since it adds a powerful support that users can take advantage in our Types. Cheers,

admin commented

2026-01-22 12:35:35 +01:00

@doctrinebot commented on GitHub (May 19, 2010):

Comment created by @guilhermeblanco:

You're the main guy that can give a final word in this subject.

I'm +1 for this

@doctrinebot commented on GitHub (May 19, 2010): Comment created by @guilhermeblanco: You're the main guy that can give a final word in this subject. I'm +1 for this

admin commented

2026-01-22 12:35:35 +01:00

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

Comment created by rv4wd:

+1 for this...

if you have datetimes in a table and are using the DateTime object, you end up with useless update queries, if you persist an unchanged object...

@doctrinebot commented on GitHub (Feb 4, 2011): Comment created by rv4wd: +1 for this... if you have datetimes in a table and are using the DateTime object, you end up with useless update queries, if you persist an unchanged object...

admin commented

2026-01-22 12:35:36 +01:00

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

Comment created by @beberlei:

That is not true.

@doctrinebot commented on GitHub (Feb 4, 2011): Comment created by @beberlei: That is not true.

admin commented

2026-01-22 12:35:37 +01:00

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

Comment created by rv4wd:

Sorry, I wasn't clear.

It does not happen in all cases.

I have a simple object, that I save to the session. After merging it and flushing the entitiy manager, an update query is generated, which sets all the datetime fields of the object to their current value:

$item = $_SESSION['item'];
$item = $this->_em->merge($item);
$this->_em->persist($item);
$this->_em->flush();

This code results in the expected SELECT query, which refreshes the item from DB, but it also results in an update query which sets the datetime of the object to the same value.
This query could be omitted, if I could use a comparable interface and a custom type for datetimes, which implement it.

@doctrinebot commented on GitHub (Feb 4, 2011): Comment created by rv4wd: Sorry, I wasn't clear. It does not happen in all cases. I have a simple object, that I save to the session. After merging it and flushing the entitiy manager, an update query is generated, which sets all the datetime fields of the object to their current value: ``` php $item = $_SESSION['item']; $item = $this->_em->merge($item); $this->_em->persist($item); $this->_em->flush(); ``` This code results in the expected SELECT query, which refreshes the item from DB, but it also results in an update query which sets the datetime of the object to the same value. This query could be omitted, if I could use a comparable interface and a custom type for datetimes, which implement it.

admin commented

2026-01-22 12:35:37 +01:00

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

Comment created by @beberlei:

This rather seems like a bug with the merging. Can you open up a new ticket describing this? Thank you

@doctrinebot commented on GitHub (Feb 4, 2011): Comment created by @beberlei: This rather seems like a bug with the merging. Can you open up a new ticket describing this? Thank you

admin commented

2026-01-22 12:35:37 +01:00

@AlexeyKosov commented on GitHub (Jul 26, 2017):

Related to #5542
Any advance on this?

@AlexeyKosov commented on GitHub (Jul 26, 2017): Related to #5542 Any advance on this?

admin commented

2026-01-22 12:35:38 +01:00

@Ocramius commented on GitHub (Aug 3, 2017):

This is a won't fix until we do data comparison post DBAL type conversion, and that's not going to happen for now.

Closing.

@Ocramius commented on GitHub (Aug 3, 2017): This is a `won't fix` until we do data comparison post DBAL type conversion, and that's not going to happen for now. Closing.

admin referenced this issue

2026-01-22 15:58:11 +01:00

[PR #347] [CLOSED] Hydrator hotfix - time to think about keeping types for columns #8056

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: doctrine/archived-orm#347