Whats the status here? Do we have any?

Since we moved to Symfony Console I don't think this is needed anymore.
The purpose of this ticket was actually to test our own CLI support, which was dropped.

I'm closing the ticket due to this. Reopen if you have any other comment.

I think we do need some basic functional tests of our Commands, they have been subject to many bugs in the past becaues they are not tested.

Fixed another fatal error in the command due to missing namespace dependency. We need tests for all the commands, there have been dozens of issues on these things so far.

This commit shows a simple approach on how testing is easily possible for symfony commands:

http://github.com/doctrine/doctrine2/commit/51e6681934a7cf4448b85c5670c04045f66c6056

Can we expect some more tests for beta4 or is it unlikely that you find the time? Should we move this further back or does someone else want to step in?

Increasing the severity and adding a fix version since this MUST be fixed before next release.

I find this to be of rather minor importance. You're talking of compatibility breaks, but thats only the case if we intend to start being very nitpicky about the naming in the future. We are currently not, and we wont be, so not much risk of later breakage.

We have a rule of thumb already: Acronyms up to 4 characters all uppercase, otherwise normal camelcase.

Now, it is often a matter of taste what is an acronym and what not and also not all acronyms have a clear casing, prime example mysql.

Being too nit-picky here is just a pain in the ass and we won't reach a common consensus anyway.

Oh and we better dont start arguing about whats better to read because there is no chance of agreement. If you ask me I find SQLException better than SqlException. So here we go. Lets better not run down this path.

No need to get upset, I'm only trying to help.

I just thought that consistency is usually a good idea, either way.

I have to disagree in that an acronym is an acronym, it's not a matter of taste, the letters either stand for something, or they don't.

As for "mysql", only the SQL part is an acronym. So, MySQL or MySql.

Close if you disagree.

I'm not upset. What made you think so? Maybe I should introduce a every now and then.

There's just no point in arguing about readability.

Like I said, we do have a naming standard even if its not adhered everywhere. The standard is also not something we've made up ourselves because we try to avoid that. When we introduced namespaces, we talked about adopting either the Java or .NET naming standards. We opted for the .NET standards. And there its recommended to write acronyms with up to 4 characters all uppercase.

I dont think there are too many violations of that in the code, probably Cli => CLI and some others but most of it looks ok to me.

UPDATE: Maybe we can gather a list here in this ticket of violations? Cli => CLI would be one. MySql => MySQL another. "Id" is rather an abbreviation of Identifier and not an acronym, to me at least.

It's not a case of getting anyone upset...

But we should fix it and keep consistency of our codebase.

@romanb We should all talk and reach a common sense.
That's our last chance (last alpha) to get this consistency in.

@Guilherme: We do have a naming standard since a year. You want to change the standard now?

@Roman,

Just a feeling I got.

This issue was more about consistency (indicated in the title) than moving to proper-case naming.

I think it might be up to 3 characters in .NET, HtmlElement, System.Linq, etc. Anyway, not important.

I agree that Id. is an abbreviation.

There are some more violations. If you decide you want to change them, let me know and I'll compile a list.

@Glen: Yes, a list would be great. I find it hard to be 100% consistent sometimes though because my taste conflicts with the rule. For example, I would prefer "Id" over "ID", especially since it comes directly after ORM "Doctrine\ORM\ID\..." would be a bit too much. But I would not like "Orm" or "Dbal" either. But I think in most cases we can clearly fix the inconsistency. Like CLI or MySQL or MSSQL (or should it be MsSQL?).

Thanks for your help!

We should probably create updated coding standards for Doctrine 2 also, since they differ quite some from Doctrine 1 due to the introduction of namespaces and such. I'll create a ticket for that.

I'll do the list for you by tomorrow at the latest, just running out of time ATM.

Id is correct, as mentioned above, so that would be fine. MsSQL is correct (Ms = Microsoft = abbreviation).

Found some time ...

Doctrine\Common\Cache\ApcCache -> APCCache
Doctrine\Common\Cli -> CLI
Doctrine\Common\Cli\Printers\AnsiColorPrinter -> ANSIColorPrinter
Doctrine\Common\Cli\CliController -> CLIController
Doctrine\Common\Cli\CliException -> CLIException

Doctrine\DBAL\Driver\PDOMsSql -> PDOMsSQL
Doctrine\DBAL\Driver\PDOMySql -> PDOMySQL
Doctrine\DBAL\Driver\PDOPgSql -> PDOPgSQL
Doctrine\DBAL\Driver\PDOSqlite -> PDOSQLite
Doctrine\DBAL\Logging\EchoSqlLogger -> EchoSQLLogger
Doctrine\DBAL\Logging\SqlLogger -> SQLLogger
Doctrine\DBAL\Platforms\MsSqlPlatform -> MsSQLPlatform
Doctrine\DBAL\Platforms\MySqlPlatform -> MySQLPlatform
Doctrine\DBAL\Platforms\PostgreSqlPlatform -> PostgreSQLPlatform
Doctrine\DBAL\Platforms\SqlitePlatform -> SQLitePlatform
Doctrine\DBAL\Schema\Visitor\CreateSchemaSqlCollector -> CreateSchemaSQLCollector
Doctrine\DBAL\Schema\Visitor\DropSchemaSqlCollector -> DropSchemaSQLCollector
Doctrine\DBAL\Schema\MsSqlSchemaManager -> MsSQLSchemaManager
Doctrine\DBAL\Schema\MySqlSchemaManager -> MySQLSchemaManager
Doctrine\DBAL\Schema\PostgreSqlSchemaManager -> PostgreSQLSchemaManager
Doctrine\DBAL\Schema\SqliteSchemaManager -> SQLiteSchemaManager
Doctrine\DBAL\Tools\Cli -> CLI
Doctrine\DBAL\Tools\Cli\Tasks\RunSqlTask -> RunSQLTask

Doctrine\ORM\Mapping\Driver\PhpDriver -> PHPDriver
Doctrine\ORM\Mapping\Driver\XmlDriver -> XMLDriver
Doctrine\ORM\Mapping\Driver\YamlDriver -> YAMLDriver
Doctrine\ORM\Query\Exec\AbstractSqlExecutor -> AbstractSQLExecutor
(Should Doctrine\ORM\Query\Expr\Andx and Orx be AndX and OrX?)
Doctrine\ORM\Query\SqlWalker -> SQLWalker
Doctrine\ORM\Tools\Cli -> CLI
Doctrine\ORM\Tools\Cli\Tasks\RunDqlTask -> RunDQLTask
Doctrine\ORM\Tools\Export\Driver\PhpExporter -> PHPExporter
Doctrine\ORM\Tools\Export\Driver\XmlExporter -> XMLExporter
Doctrine\ORM\Tools\Export\Driver\YamlExporter -> YAMLExporter

... I think that's it. More than you expected? I did say: "There is more proper-casing than upper-casing."

No, not more than I expected. It's mostly SQL and CLI basically. Thanks for the list!

We should try to go through that list before beta.

Methods should be adjusted accordingly also (even though they're case insensitive). I already started with that. More to come.

Most of the Cli => CLI changes seem to be complete now.

Pushing the rest of the name changes towards beta2.

More renamings have been done but still more to do. Pushing remaining work to beta3.

Final name changes should be done prior to going into RC1.

there is much to be done yet, however most of it is also public API class names and might cause quite some BC breaks (i.e. DBAL Platforms and Schema Manager, Mapping Driver Names). I am not sure how to proceed here.

Since PHP is mostly case-insensitive on class and method names it shouldn't be much of an issue.

Here is the way I've implement the solution on my side: (for oracle)

into Doctrine/DBAL/Statement.php, I've add this method:

/**
     * Binds a parameter value to the statement.
     * This is implemented this way for oracle only. Other drivers are redirected to bindValue method.
     *
     * The value will be bound with to the type provided (that required to be a table type).
     *
     * @param String $name The name or position of the parameter.
     * @param Array $value The value of the parameter.
     * @param String $type The name of the type to use to bind.
     * @return boolean TRUE on success, FALSE on failure.
     */
    public function bindList($name, Array $value, $type)
    {
        if ('oracle' !== $this->platform->getName())
        {
            $this->bindValue($name, $value, $type);
        }
        else
        {
            return $this->stmt->bindList($name, $value, $type);
        }
    }

into Doctrine/DBAL/Driver/Statement.php I've add:

/**
     * @TODO: docs
     */
    function bindList($param, Array $values, $type);

into Doctrine/DBAL/Driver/OCI8/OCI8Statement.php I've add this method:

/**
     * {@inheritdoc}
     */
    public function bindList($param, Array $value, $type)
    {
        if (!($list = oci_new_collection($this->_dbh, $type)))
        {
            //throw new OCI8Exception::fromErrorInfo($this->errorInfo());
        }

        foreach ($value as $entry)
        {
            $list->append($entry);
        }
        if (!oci_bind_by_name($this->_sth, $param, $list, -1, OCI_B_NTY))
        {
            //throw new OCI8Exception::fromErrorInfo($this->errorInfo());
        }
    }

// NOTE: we should probably add the bindList to all driver Statement object.

into your code you can use it this way:

$sql = "
    SELECT *
    FROM test
    WHERE id IN
    (
        SELECT *
        FROM
        (
            CAST (: p_ids AS list_int_type)
        )
    )
";
$stmt = connection->prepare($sql);
$stmt->bindList(': p_ids', $ids, 'list_int_type');
$stmt->execute();
$rs = $stmt->fetchAll(PDO::FETCH_ASSOC);

NOTE:
list_int_type need to be a valid oracle data type. You can create one with the name you want.
example:
you can have 2 type of accepted array of values: integer and string
let's say we create one for string named: list_str_type and one for integer list_int_type

create or replace type list_str_type as table of varchar2(4000);
create or replace type list_int_type as table of number;

Hey Marc Drolet

thanks for the feedback and the solution, however i would like to have something generic that is working independent of the database driver. This code is very specific.

Can you point me to some documentation why oci collection works with more than 1000 elements and how it works in PHP?

Hi Benjamin,

The limitation is not from the oci driver, it's an oracle limitation. There are a couple of possible solution/implementation that can be done but the one I've provide is the one that perform better for the test I've done and from what I can found over the blogs I've read.

I can't find the exact documentation of oracle. oracle doc is so poor.
Here is the best description link I can provide that describe some possible implementation.
http://vsadilovskiy.wordpress.com/substituting-a-collection-for-in-list-performance-study/

I don't know if there is similar limitation with other database. With the implementation I've provided, It will be possible to implement the proper solution depending on the database limitation you face otherwise it will execute the generic IN. What's bad, we need to create the type into the database.

NOTE: In my case, I can not perform a sub-query, I get the my collection from a web service call.

Symfony 2.1.8, php 5.4.7 and php 5.4.12, Windows 7

Please provide either a code example or a test case. As it stands, this issue is incomplete

Are you calling EntityManager#clear() inbetween? Because PHP reuses the hashes. The ORM accounts for this.

This is not a reproduce case, i don't want to execute your whole project.

I want to know, what is the actual bug that you see? Can you just print a list of all the hashes? Because the hashes dont differ at the end, bu tjust somewhere in the middle.

I attached a hashlogs.txt file. The last Task class hash is 0000000050ab4aba0000000058e1cb12 ( line 3 129 )

This is not unique, view the line 2 760 . The Task is not being saved and the program don't call the prePersist listener. The "UnitOfWork" believe the entity has been saved because the isset($this->entityStates[$oid]) is true. But it is an other entity.

The EntityManager::clear() fix the problem, but this is not "good" and "beautiful" solution. Shows no sign of that conflicts were and this is causing the problem. I was looking for the problem 7 hours.

If this is to be included in 2.0 it needs to happen for RC1. However, it is not clear yet whether it will be done in time.

How would you solve this Roman? I thought of adding a query hint so that the postLoad inside unit of work is not triggered and gathering all the entities that have a post load event in an array inside the object hydrator, then iterating it after taking the snapshots of all collections inside hydrateAll

@Benjamin: Not sure what you would use the query hint for but in general that is the approach I had in mind, yes. You can't get around iterating over the entities after the actual hydration.

The query hint would do something like:

        //TODO: These should be invoked later, after hydration, because associations may not yet be loaded here.
        if (isset($class->lifecycleCallbacks[Events::postLoad]) && !isset($hints['hydrationPostLoad'])) {
            $class->invokeLifecycleCallbacks(Events::postLoad, $entity);
        }
        if ($this->evm->hasListeners(Events::postLoad) && !isset($hints['hydrationPostLoad'])) {
            $this->evm->dispatchEvent(Events::postLoad, new LifecycleEventArgs($entity, $this->em));
        }

another way would be to move that code out of UoW::createEntity completly and have the persisters call it when they use that method.

Leaving that code in UoW does not make sense to me, if it is moved, it needs to be moved completely. Why do you think the persisters should do it? Initially I thought collecting the affected entities during hydration and then when hydration is done iterating over them and triggering the postLoad events.

Yes but postLoad has to be triggered for non Hydrated entities (i.e. Persister) also

Moved back

Hi Gyus, I need access to associations in postLoad or similar event, and my idea is to dispatch new event after full initialisation of object, what do You think about it? If I can help please let me know It's important for me.

Now in my PostLoad access to associations is work fine. Why this issue is still in Unresolved status?

What do you (Roman and Benjamin) think about adding postHydrate event which would be called within ObjectHydrator::hydrateAllData() on every entity collected during hydration? I could prepare a patch for this. I personally think this would be better than adding a hint that changes behaviour of postLoad event.

Just stumbled upon this issue. Seems like my associated entities aren't yet loaded in PostLoad event handler.
Doctrine version "doctrine/orm": "2.3.1"

I've noticed that I have two different paths of execution:
1. When the find() method is used to retrieve the entity, SimpleObjectHydrator is used to perform a hydration and the associations are made available in PostLoad event:
bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php.Doctrine\ORM\Mapping\ClassMetadataInfo->invokeLifecycleCallbacks : lineno 2312() bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php at line 2312
bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php.Doctrine\ORM\UnitOfWork->createEntity : lineno 2610() bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php at line 2610
bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/Internal/Hydration/SimpleObjectHydrator.php.Doctrine\ORM\Internal\Hydration\SimpleObjectHydrator->hydrateRowData : lineno 135() bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/Internal/Hydration/SimpleObjectHydrator.php at line 135
bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/Internal/Hydration/SimpleObjectHydrator.php.Doctrine\ORM\Internal\Hydration\SimpleObjectHydrator->hydrateAllData : lineno 50() bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/Internal/Hydration/SimpleObjectHydrator.php at line 50
bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php.Doctrine\ORM\Internal\Hydration\AbstractHydrator->hydrateAll : lineno 111() bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php at line 111
bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php.Doctrine\ORM\Persisters\BasicEntityPersister->load : lineno 678() bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php at line 678
bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/EntityManager.php.Doctrine\ORM\EntityManager->find : lineno 413() bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/EntityManager.php at line 413
bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/EntityRepository.php.Doctrine\ORM\EntityRepository->find : lineno 131() bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/EntityRepository.php at line 131

2. When the DQL is used, ObjectHydrator is used to perform a hydration, and the associations aren't made ready in PostLoad event:
bvdpetroleum/src/BVD/PetroleumBundle/Entity/FuelCard.php.BVD\PetroleumBundle\Entity\FuelCard->retrieveNumbers : lineno 304() bvdpetroleum/src/BVD/PetroleumBundle/Entity/FuelCard.php at line 304
bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php.Doctrine\ORM\Mapping\ClassMetadataInfo->invokeLifecycleCallbacks : lineno 2312() bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php at line 2312
bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php.Doctrine\ORM\UnitOfWork->createEntity : lineno 2610() bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php at line 2610
bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php.Doctrine\ORM\Internal\Hydration\ObjectHydrator->_getEntity : lineno 245() bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php at line 245
bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php.Doctrine\ORM\Internal\Hydration\ObjectHydrator->hydrateRowData : lineno 478() bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php at line 478
bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php.Doctrine\ORM\Internal\Hydration\ObjectHydrator->hydrateAllData : lineno 149() bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php at line 149
bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php.Doctrine\ORM\Internal\Hydration\AbstractHydrator->hydrateAll : lineno 111() bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php at line 111
bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/AbstractQuery.php.Doctrine\ORM\AbstractQuery->execute : lineno 747() bvdpetroleum/vendor/doctrine/orm/lib/Doctrine/ORM/AbstractQuery.php at line 747

Hope this helps to solve the issue.

formated snippets nicely

I need this feature too.

But I would suggest using the same annotation used by JPA

@Embeddable

+1

You should also take into consideration different storage strategies of ValueObjects.

Martin Fowler points out - in „PoEAA" - two approaches: Embedded Value (which is the one presented above) and Serialized LOB .
Both have their pros and cons, that's why Doctrine2 should give developers choice of selecting the fittest solution.

Of course technically we can similate a serialized LOB with a new Doctrine 2 type.

I don't like that idea - Its so not generic.

VO as a pattern is important building block of domain model, which clearly indicates that VO as a feature of Doctrine2 should be tailor-made.

To anyone of dev-team reading this issue: without VOs Doctrine is not yet DDD-ready, please hurry

Serialized LOB is not very useful IMHO and has lots of problems (many mentioned in PoEEA already).

@Alan: I appreciate your nice reminder and I'm sure you mean it in a friendly way, but please keep in mind that noone is paid to work on this project. It all happens in free/spare time and the current state of the project already consumed at least 1 1/2 years spending many hours weekly on this project from me alone. Not to speak of the others.

Thus, there is no point in demanding something or telling us to hurry. The best way to get a feature in is to provide a (good) patch that we find worth including.

I started to add notes to this issue to collect all the things that need to be done for this feature.

In the meantime, its not too hard/ugly to get a half-way decent embedded value yourself:

/** @Entity @HasLifecycleCallbacks */
class Foo {
    // annotations not shown
    private $id;
    private $embedded;
    private $value1; // never reveal to public
    private $value2; // never reveal to public
    private $value3; // never reveal to public

   public function getEmbedded() {
       return $this->embedded;
   }

   public function setEmbedded($embedded) {
       $this->embedded = $embedded;
   }
   
   /** @PrePersist @PreUpdate */
   function _destructEmbedded() {
       // destruct $embedded into $value1, $value2, $value3
   }

   /** @PostLoad */
   function _constructEmbedded() {
      // construct $embedded from $value1, $value2, $value3 
   }
}

Several variations of this are possible, also with an external event listener instead of callbacks but in that case you might need to use reflection to get at the values.

I want to share my thoughts on possible VOs collections implementations.

1. As it was mentioned earlier serialized (C)LOB is one solution. Implementation of storing/retrieving object graphs alone is quite simple, but it's complex in terms of SELECTs with conditions.
Composing SQL condition would result in some nasty constructions e.g. vo_collection_column LIKE '%foo%bar%' which output format would depend on serialization target (CSV, XML, YAML, PHP serialized objects etc.). Also in most cases it would be impossible to obtain eligible result.

I'm not taking Regexp or XPath operators into consideration as only few RDBMS support them.

2. The second solution is to break VOs graph into separate related table... or tables if we consider that VO can contain another VO(s). It's not so fast as serialized LOB but more flexible and it utilize power of RDMS,
But there is one catch: Doctrine2 must preserve nature of VO. To make it happen during Entities persisting - if any change in dependant VOs graph has been made - all associated VOs rows in database should be deleted and the new/changed VOs graph should be inserted in their place.
I know it could be inefficient while dealing with large object graphs, yet faster than comparing VOs one-by-one.

In conclusion:
serialized LOB is extremely fast in CRUD-like operations on aggregates, however very search unfriendly.
Separate ValueObjects tables are better where serialized LOB lacks, but slower in exploitation.

I can't tell which approach is superior, because each of them is valid under different circumstances.

Hope this helps.

@Alan: I appreciate your nice reminder and I'm sure you mean it in a friendly way [...]

Of course I do.

It would be easy to implement value objects in userland using the XML capabilities of many RDBMS:

1. Implement an Xpath function on the Dql Parser
2. Implement a User-Defined Type for each value object that handles the translation from and to XML.

The second point can be heavily optimized when value objects are immutable with an own identiy map of value types inside the Type flyweight instance.

I more or less suggested something similar above.

ah, my bad - i must have overseen this

+1

This would be awesome.

Don't forget (especially with regard to SLOBs) that values might in turn contain references to Entities.

Example: An "Order" might be an @Entity and might have a field (an array) of OrderLineItems as value. Each OrderLineItem might e. g. carry quantity or disconunt and references a Product (@Entity).

So even if you don't need the traversal from Product to all the Orders it is contained in, serializing the OrderLineItems needs a way to "cut off" the object graph at the transition towards the Product but must place some kind of referral there so that upon unserialization (of the OrderLineItem list, that is, during Order load) the Product references in every OrderLineItem are at least initialized with proxies again.

Don't know whether/how referential integrity (OrderLineItems <-> Products) would make sense or could be implemented here.

Pushed back to 2.x, this feature is probably the largest feature request we have and we'd rather focus on small improvements for 2.1

Several thinks to consider/not to oversee here:

1) There are value objects with identity. I know that is not DDD-conform but only at first sight. It means they are technically entities but are treated like VOs.
Common examples are Zipcode or country. As they have identity (e.g. Zipcode: de-40723) they are entities but are created and interchanged like normal VOs.
On the google DDD-List they were often referenced aS Lookup Entities.

2) In virtually all (business) cases a collection of VO is an Entity. How else could you reference (add or remove) single elements of that list?
There are exceptions here like a undefinded number of VOs in a collection, but in that case you can only add or remove a quantity of it.
As a true collection (say 3 addresses for a client = Entity ClientAdresses) you would have to give them some kind of identity, even if it is only having
a sequential number in that collection.

@Matthias: OrderLineItems is an example of actually being an Entity.

Hi guys. I face this in my own way. Hope you won't wake up your neighbours with loud laugh.

Every @Entity extends my BaseEntity object which provide kind of wrap for value with ValueBase object. So when want to get/set value from entity you call $entity->getData() where you won't get value "data" but wrapping ValueBase for value "data". Then you can get bare value by getValue(). Name of value class is in annotation and would be child of ValueBase.

There's also parent class Base for EntityBase and ValueBase. In my case class Base is something like HTML element. So in the end you can use $entity->renderHtml() or $value->renderHtml() no matter if you're rendering value or @Entity. There's more features like validation, filtering and hydration value/entity from HTML forms, but it's extra.

Implementation:

 
 /* @MappedSuperclass */
abstract class Base {
  /* there're methods like _getParent(), _getPropertyName(), etc. used in code behind */
}

 
abstract class ValueBase extends Base { 
   public function getValue() {
        return $this->_getParent()->{$this->_getPropertyName()};
   }
   
   public function setValue($value) {
        $this->_getParent()->{$this->_getPropertyName()} = $value;
   }
}

 
/** @MappedSuperclass */
abstract class EntityBase extends Base {
    public function __call($name, $arguments) {
        /* get property object */
        $pattern = '/^get(.*)$/u';
        preg_match($pattern, $name, $matches);
        if (isset($matches[1])) {
            $propertyName = lcfirst($matches[1]);
            return $this->get($propertyName);
        }

        /* set entity */
        $pattern = '/^set(.*)$/u';
        preg_match($pattern, $name, $matches);
        if (isset($matches[1])) {
            $propertyName = lcfirst($matches[1]);
            return $this->set($propertyName, $arguments[0]);
        }
    }

    public function get($propertyName) {
	    $property = $this->_getElementProperty($propertyName);

	    if ($property == null)
		throw new Exception(sprintf("There isn't property like '%s'.", $propertyName));

	    /* for collections and entities */
	    if ($property["type"] == "collection" || $property["type"] == "entity") {
		$element = $this->{$propertyName};
		if ($element != null) {
		    $element->_setParent($this);
		    $element->_setPropertyName($propertyName);
		} elseif ($property["type"] == "entity") {
		    $element = new $property["class"];
		    $element->_setParent($this);
		    $element->_setPropertyName($propertyName);
		    $element->_setNullEntity();
		    $this->{$propertyName} = $element;
		}
		return $element;
	    }
	    else {
	    /* for values */
		if (!isset($this->_loadedEntities[$propertyName])) {
		    $this->_loadedEntities[$propertyName] = new $property["class"]($this, $propertyName);
		}
		return $this->_loadedEntities[$propertyName];
	    }
    }

    public function set($propertyName, $value) {
        $property = $this->_getElementProperty($propertyName);

        if ($property == null)
            throw new Exception(sprintf("There isn't property like '%s'.", $propertyName));

        /* for collections and entities */
        if ($property["type"] == "collection" || $property["type"] == "entity") {
            $this->{$propertyName} = $value;
        }
        /* for values */ else {
            throw new Exception(sprintf("Can't call set on value property '%s'.", $propertyName));
        }

        return $this;
    }
}

Note that there's something i call "NullEntity". Instead of getting bare "null" you'll get @Entity child of EntityBase, where is set property nullEntity. Then there's posibility to work with null entity (for example renderHtml with empty inputs).

It would be nice, if this is support by Doctrine natively, because i have some performace problems with my implementation. If it's interest in my whole code i can send you. But of course there's some security holes so i'll send it privetely. Thanks for understand and for Doctrine of course.

Note that Roman's workaround presented here does not work.

   /** @PrePersist @PreUpdate */
   function _destructEmbedded() {
       // destruct $embedded into $value1, $value2, $value3
   }

Doctrine tracks changes and does not perform updates when no changes are found. $embedded is not mapped, so it's not tracked and won't be taken into account by Doctrine when updating. Therefore, if $embedded is the only value that was changed, the PreUpdate event won't be triggered.

The easiest thing to do is to simply destruct the VO on every mutation:

   public function setEmbedded($embedded) {
       $this->embedded = $embedded;
       $this->_destructEmbedded();
   }

The downside is that you need to remember to call the method in every setter, but apart from that, there are no side effects, it always works and it's just one line of code

_constructEmbedded() keeps working as is, postLoad will always be triggered.

Hi,

This feature would be awesome !
VOs are really essential in a good domain design.

If you plan to implement this, please remember that you can have nested VOs.
Take the design for a Booking process for instance, you would have a DateRange object embedding two DateTime objects (in the simplest case).

I have no doubts that you've already took this in consideration, but i prefer pointing this out, just in case

work has been started, https://github.com/doctrine/doctrine2/pull/265

Does the new "complex sql types" feature help here - I mean, could that be used to map a value object to more than one column in the database?

@Benjamin Eberlei The request seems to be closed in the link you provided! Does that mean that this feature won't be implemented?!

songoko songowan no, it just probably wasn't the correct way of implementing this

I'm curious if any effort is currently being put into this. I would really love to have this feature available.

Daniel Pitts this is being developed in DDC-2374 ( https://github.com/doctrine/doctrine2/pull/634 )

Questions

• I suppose link and unlinked entities would then handled by UnitOfwork commit also?
• Since the collection is not initialized, one does not know upfront if the action will be successful, what happens if:
  • an entity is linked with a collection, although they are already connected.
  • an entity is unlinked from a collection it is not in.

Regarding the naming, i like link/unlink.

What do you mean by "handled by UnitOfWork commit" ? Whether the SQL is "scheduled" or executed immediately? Interesting question.
Scheduling would probably be better but also more difficult.

As far as usage is concerned, I currently imagine it as follows:

// EntityManager#link($sourceObj, $field, $targetObj)
$user = $em->getReference($userId); // $userId probably from request parameters
$address = $em->getReference($addressId); // $addressId probably from request parameters
$em->link($user, 'addresses', $address);

"What happens if: an entity is linked with a collection, although they are already connected."

Probably an SQL error which results in an exception from the driver. Depends on the database constraints though.

"What happens if: an entity is unlinked from a collection it is not in"

Probably nothing, at least not from the SQL side. An exception could be thrown from Doctrine itself if the update affected 0 rows.

Thanks for these initial questions. Thats definitely food for thought. Keep it coming.

Pushed back.

Roman, I'd like to do this one as I have currently a use case for this. Do you have any idea of how to do this? What I'm wondering is whether it is possible to implement this without user intervention. (This would simply mean "store the entities as they were added"). But this would need another column in DB that we'd have to add within oneToMany / manyToMany relationships, but in this case one could save a serialized array holding "entityId => position" key / value pairs.

Afterwards, one could easily rebuild / reorder the collection via $collection->set($entity, $order[$entity->identifier]);

If you've got another thought about this, please don't hesitate to point me into the right direction!

this won't be implemented until 2.1, since its a pretty complex feature.

Changes are probably required in:

1. CollectionPersister - Add a new collection persister that takes the position into account
2. SchemaTool - Add a 'col_position' column to either the many-to-many or the one-to-many tables.
3. EntityPersister - Use and extend current order-by support to make the sorting happen

You can implement this already though with some performance hit in update scenarios. If you use the ORDER BY support and implement an API around your entity that abstracts those changes and always sets a "position" field on the many entity that is supposed to be sorted.

I don't think we necessarily need a new collection persister. Simply adjusting the ManyToManyPersister to be able to deal with it might be sufficient.

For OneToMany, that is always persisted from the "many" side, thus there is no collection persister, we would need to adjust the normal persisters.

They key element for the user should be a new annotation (or corresponding xml/yaml element) @OrderColumn. By default the order should not be persistent, only when an @OrderColumn annotation is present. The name of the order column can have a default, i.e. "position". Thus this enhancement of persisting the order should be fully backwards compatible.

On another note, the getInsertDiff/getDeleteDiff methods of PersistentCollection should already be "ready" for this. That is, when an element in the collection changed only its position, this is already tracked as a change. However the ManyToManyPersister issues no "UPDATE" queries, it simply deletes and inserts. A position change may be more effectively persisted with an UPDATE.

From a mailinglist entry, required check/changepoints:

1. ClassMetadata of Many-To-Many associations have to be extended to publish the required datastructure to the ORM.
2. All Metadata Mapping Drivers have to be extended
3. Persisters\ManyToManyCollectionPersister has to be extended to save the key in the many to many table if desired by the user.
4. Schema-Tool has to be extended to create the additional column.
5. PersistentCollection has to be extended so that lazy loading of collections with additional key works.
6. Array- and ObjectHydrator have to be extended to allow fetch join of collections with key column.
7. Discuss wheather to support this for One-To-Many also with the key-column on the many side. This is much more tricky internally though.

Push back to 2.x, we will have support for DDC-250 first and for this at a later release.

Hi there,
I'm looking for this feature.

Benjamin Eberlei said that : "You can implement this already", but I don't understand the "how to".

Also,
The problem should be solve if RDBMS had a "natural" order. An order based on item position inside table.

To get this feature without any change on Doctrine, I have remplace the PK defined by the target & mapped field identifier. The new PK is a new field with type "integer" and with auto-increment enable.
In this configuration, Doctrine use the "natural" order of the RDBMS. And I can change order of my item inside Collection and persist it.

It's an very bad solution, but It work before an official support.

Waiting for advices, and solutions,

Thomas.

Answering to Benjamin Eberlei on the "7. Discuss wheather to support this for One-To-Many also with the key-column on the many side. This is much more tricky internally though.".

I think that for One-To-Many relations, if user want to store the collection order, Doctrine can store the One-To-Many as Many-To-Many with a "model" limitation.
In that case, if storing order collection for Many-To-Many work, it should work for One-To-Many.

What do you think about it ?

I think that it must be possible to have two keys ordering : the order isn't obligatory reversible.

For exemple with user and group :

• You can order groups for one user : with preference by exemple, or importance.
• And with a different order, users for a group : rank by example.

And maybe more, if you decide to add multi-order : an user show group by his rank in it, if his rank is identical, the order is make by love preference, and after by the importance given by the user (not necessary a number, if we imagine filter on them).

So a default order can be choice with parametized fields and could be :

 
@ManyToMany(targetEntity="Group")
...
@JoinFields ( rank: { type: int} , preference:{type:int}, importance:{type: string, length: 40} )
@OrderByJoinFields({"rank" = "ASC", "preference"="ASC", "importance"="ASC" } )

In this case the order must be optional and would be clean if another order appears in the same scope (DQL...). And manytomany became virtual entities act as other entities except they don't appears permetting in the same time a better conception.

So if the solution take in DDC-181 will become the only solution. This would a good idea to document this. Because, this seems to me a very important point.

My last point is even an unique ordering field created in the join table will be a big and usefull improvement.

Thank a lot for your beautiful work.

In my point of view, a collection can be order in a single way only.
If you want to add more than one order between User & Group, it's a new collection, a new relation.
Like :

• User.memberOf() : Group[]
• Group.members() : User[]
• Group.importantMembers() : User[]

And it's your role to keep a consistency between members & importantMembers array.

Because ManyToMany join table is the reflection of a state of an ArrayCollection. It's not a usefull feature to be able to store all of the state of an ArrayCollection, even the order of this Array. It's just a normal feature that is really missing

Thomas.

I don't think:

If you have three collection, you duplicate one relation 3 times and it's easy in consequence to lost the data integrity and unicity.

By example :

• Thomas have rank 10 in Admin
• Thomas think the admin group has importance noted 3 on all of his groups.
• If a responsable of admin group decide to delete Thomas from it.
• Thomas, in his ordered list of groups, think always to be in group admin.

So in my idea, the many to many relation isn't just an array collection, but should be an virtual entity. In UML or in Merise method this is a common problem to have a parametized relation. I think an orm should just implement this.

Hum,
I agree with you.. In a SQL Schema, it's a good choice to add many fields in a ManyToMany join table to description "order".

I just want to add a piece of Doctrine ORM Document :

"When working with collections, keep in mind that a Collection is essentially an ordered map (just like a PHP array). That is why the remove operation accepts an index/key. removeElement is a separate method that has O ( n) complexity using array_search, where n is the size of the map."

Hi there,
After several discussions. on IRC, I have changed my point of view.

Doctrine Documentation says : "When working with collections, keep in mind that a Collection is essentially an ordered map (just like a PHP array)".
So, I think that Doctrine have to be able to store or not the order of a Collection. By adding a new field on the Joined table to store the position of each elements.

But I not agree with @Nicolas. Because in his case, he's talking about Association Class : http://etutorials.org/Programming/UML/Chapter+6.+Class+Diagrams+Advanced+Concepts/Association+Class/
Because he's talking of a business logic, he's talking of a dedicated Entity class.

What do you think about it ?

Thomas;

Any news ?

Hi, any news on this?

If I may add any info to this feature request, maybe something like JPA/Hibernate could be a good start?

See http://docs.oracle.com/javaee/6/api/javax/persistence/OrderColumn.html or http://docs.jboss.org/hibernate/core/3.6/reference/en-US/html/collections.html#collections-indexed

The idea in Hibernate is that you persist the order of the list in an extra column. This column is not a field of the entity however.

The DataObject you describe is a no-go for Doctrine 2. Its just a very bad practice.

Inheritance Mapping is for REAL inheritance only, otherwise you shouldnt go with a relational database in the first place.

You should use the Event system for such changes, it offers you roughly the same possibilities and keeps you from having to use inheritance mapping. You could still create an abstract data object and define the fields that will be used in each "implementation" and then in events do something like:

if ($entity instanceof DataObject) {
     $entity->updated();
     $archiver->makeSnapshot($entity);
}

With this patch I think you could setup a nice similar model where you can introduce new children of this parent class and have it added to the discriminator map from the child instead of having to modify the parents mapping information.

http://www.doctrine-project.org/jira/browse/DDC-447

In this implementation Schema-Tool would generate a table:

elementTable (entity_id-1, ..., entity_id-n, key, value) and using the Platform Type Generation of keyType and valueType

Column Names should be Change-able also since there could be people who name their primary keys "key" and "value" o_O

Ordering could be implemented on top of this using the @OrderColumn JPA implementation by adding another column to the table with a numeric order that will be "order by"'d on select time.

Pushed back

This seems to be a valid issue to me.

This implementation is the actual solution to associations retrieval that are inherited (type joined).

Example:

/** Joined */
class Base {}

class Foo extends Base {}

class Bar {
    public $foo;
}

// This causes the CTI to link as INNER JOIN, which makes the result become 0
// il if you have no Foo's defined (although it should ignore this)
$q = $this->_em->createQuery('SELECT b, f FROM Bar b LEFT JOIN b.foo f'); 

Yes, this is a possible solution for DDC-512 but on the SQL level. I still don't see this as appropriate for DQL, it just doesnt make sense to me, DQL joins object associations, there is no precedence.

So, no, this has nothing to do with DDC-512. DDC-512 can even be fixed differently as outlined in my comments there.

On a side note I would still like to know/see the following for this issue:

• Some realisitic DQL examples where this feature would be essential, i.e. there is no other way to do it.
  This also means explaining what the impact on the resulting object graph is and why it makes sense.
• Which other ORMs support this on the OQL/Criteria level?

So far, my stance on this issue is:

1) It doesnt make sense (semantically) in DQL
2) Its rarely needed
3) When you really need it you can use a NativeQuery anyway and use this nesting in SQL, where it probably belongs and makes more sense
4) It would (unnecessarily) complicate DQL

Thus I am currently leaning towards "Wont fix" for this issue.

Hi Roman. I understand your doubts, and I have been breaking my head over
creating a realistic example the last few hours that would hopefully convince
you for implementing this feature. But actually I cannot find one that you wouldn't
consider to be trivial. I do have a number of very complex optimized queries written
for sportskickoff dot com (using Doctrine 1.2) but they are probably hard to understand
because they may not be selfdescribing. Below is one example literally ripped from
the application. Still they often can be broken down to my example query in this
ticket's description, but applied grouping, additional other joins on the root component
and/or other criteria made them impossible to rewrite using subselects or choosing
another root component. Most often they just performed way best using the nested
syntax and saved me a number of additional queries.

SELECT A.id, A.username, A.balance, COALESCE(SUM(B.stake), 0) AS sumstake, COUNT(B.id) AS nrbets
FROM account A
LEFT JOIN (
bet B
INNER JOIN game G ON G.id = :GAMEID AND B.timestampcompletion BETWEEN G.timestampstart AND G.timestampend
) ON B.accountid = A.id AND B.timestampcompletion IS NOT NULL
WHERE A.Status & :ACTIVEORDISQUALIFIED = :ACTIVE
GROUP BY A.id, A.username, A.balance
ORDER BY A.balance DESC, sumstake ASC, nrbets ASC, A.username ASC

But let's put it another way. I would also like this feature to be supported in DQL
because I just do not want to use native queries. Why would I want to use native
queries if it can be done using DQL? In DQL I work with class names and field
names, and they may differ from the underlying table and column names. Doctrine
takes care of that mapping based on my schema/annotations and I do not
have to "know" these mappings. In native queries I suddenly do have to "know"
these mappings. I use Doctrine because it makes my application portable and
enables me to work with my database in an OOP way like I do in my model,
abstracting things. The need for native queries partly reverts the benefits Doctrine
offers in the first place.

Btw, I recall to have successfully used the nested join syntax in HQL (.NET Hibernate)
but I cannot find examples on the web or a BNF notation.

Furthermore, in reply to your stances:
1) It indeed doesnt make sense (semantically) in DQL, it only makes the result
set different, but not the way data is hydrated into objects;
2) Its indeed rarely needed for inserting, updating and populating basic lists but
it allows you to better select what combinations of associated rows are joined
and which not in more optimized queries without having to use native queries,
or because they perform better than using subseletcs and alike.
3) Not having to use native queries is just an extra reason for using Doctrine and
maintains the abstraction the ORM provides througout on'es whole application
4) Why would it complicate DQL, if people do not know about or understand
the feature it wouldn't matter because not using parenthesises is the default
way to specify joins?

Well, this is it, can't find any more words to promote and make you enthusiastic.... lol.

Ok, I have not given up yet... , here's a "stupid" example.

Imagine a book store that sells books of various authors and keeps track of those sales.
Let's say you would have an admin page that lists all authors, and for each author
its also shows the books and their sales dates since january 1st, but only for those
books that were actually sold and contain an A in its name. An optimized SQL query
to fetch all the information at once would be something like:

SELECT A., B., S.*
FROM author A
LEFT JOIN (
book B
INNER JOIN sale S ON S.book_id = B.id AND S.dt >= '2010-01-01'
) ON B.author_id = A.id AND A.name LIKE '%A%'

In DQL it would then be something like:

SELECT A., B., S.*
FROM author A
LEFT JOIN (
book B
INNER JOIN sale S WITH S.dt >= '2010-01-01'
) WITH A.name LIKE '%A%'

If the database would contain thousands of books, but sales for just a
few books, this will definitely perform better than using subselects.
Off course one would like to fetch array graphs instead of objects for
further optimization, but this hopefully shows my point.

I have attached a test casefor a similar query, though without the additional
join constraints for clarity. I surely hope you can consider it.

One last note, you shouldn't be afraid that nesting joins is not in the
ansi SQL spec. Select queries are about record sets and products
between these sets, tables are just the basic means of providing record
sets to the query. This is an important terminological difference to think about.
Specifying precedence with parenthesis around joins is a logical and
natural evolution of the ansi sql standard. For example views are a good
proof of this concept, I could define book B INNER JOIN sale S as a view
and LEFT JOIN that to authors to get effectively the same result
set as the above example. The database server would internally perform the
same query (though may additionally take indexes on the view into account).
That said, rdbm's that support this syntax would certainly never drop the
feature, as its not a feature but just plain logical and smart querying!

P.S. I had a hard time finding out how to run the test cases, I could not find
it in the Doctrine 2 documentation, development wiki, cookbook or any other
place, while finally it was as easy as running phpunit Doctrine_Tests_AllTests
from within the tests/ directory, or just phpunit Doctrine_Tests_ORM_Functional_Ticket_DDC349Test
for my test. Could you please add some info about this somewhere, it might
save others some googling.

Test case as SVN patch using a parenthesized join.
Just remove the parenthesises from the query to have it fail...

@"The need for native queries partly reverts the benefits Doctrine offers in the first place."

That is something I hugely disagree with. Neither SQL abstraction, nor database vendor independence is the main purpose of an ORM like Doctrine 2.
It is the state management of your objects, the transparent change tracking, lazy-loading and synchronization of the object state with the database state and nothing of this gets lost when using native queries.

We could rip out DQL and any other querying mechanism except a basic find() (and lazy-loading, of course), only providing the native query facility and even only supporting MySQL and would still retain all the core ORM functionality.

NativeQuery is one of the best and core "features" of the project. It is even the foundation for DQL. A DQL query is nothing more than an additional (beautiful) abstraction but what comes out is a native query + a ResultSetMapping, the same thing you can build yourself in the first place, even using the mapping metadata to construct the query. Nothing forces you to hardcode table and column names in native queries if you don't want that. Just use the mapping metadata, DQL does the same.

SQL abstraction and database vendor independence is icing on the cake, not the heart of the ORM.

i just found a possible BC issue with this.

EntityRepository is allowed to be extended by us, it has several variables that are underscore prefixed. How to proceed in this case?

I know but its not really a problem I think. We should just decide whether we make them private in the first place and provide getters instead (which would have avoided this problem in the first place).

Leaving the prefixes on the repository class only is also an option... but I dont think thats necessary.

can we commit getters for Beta 1 then? We could give everyone a period until Beta 2 to fix their code and then make the change.

EntityRepository is the only class that is meant to be userland extendable to my knowledge, so this should be the only problem to adress

Yes, you can add getters and commit right away if you want. Plus adding a note on the upgrade document that direct access of these properties is deprecated.

Persisters will be also extensible some day in userland but they need more polish for that, I've already started with it

Is this still planned? Searching the code base finds this is not being implemented. It would be a good idea to implement the change sooner than later if it will be done at all. Also, +1 for the change. It makes complete sense.

Has there been any work on a coding standards document yet?
I'm currently working on fixing documentation on this project, and it might be a good time to define a standard.
I've started compiling a few recommendations based on various feedbacks I've got in my pull requests, and I can post them here.
Please let me know if there have been previous attempts so far!

Benjamin Morel Guilherme Blanco may have a CS ruleset, but it's not ready yet. Perfect timing btw, we really need to automate this to avoid having all these useless CS fix comments in pull requests

Ok, I'll post my document here once ready, and Guilherme Blanco will be able to compare it with his ruleset!

Here is a first draft: https://gist.github.com/4676670

Please comment!

Guilherme Blanco, if you don't have time to compare your ruleset with my draft, maybe you could publish your current ruleset so that others can have a look?

Hi,

you mention a good point, however, this currently only affects findBy queries made through a repository. A DQL query already triggers a flush when there are pending insertions but this still has its own problems. First of, querying against the objects in the UoW is not a viable solution in my eyes.

For a regular find() (by identifier) the situation is clear anyway, you must flush prior to a lookup on an entity you previously persisted in the same request because, by definition, generated primary key values are only guaranteed to be available after the next flush.

Automatic flushing if the UoW has pending inserts (new objects) and a query is executed (either through DQL or a repository) currently has its own set of problems, namely that it is still subject to infinite recursion if such a query is triggered in an event (listener) that executes during commit of a UoW, and secondly, that it will easily lead to double-flushes that cause unnecessary overhead (currently a flush() even if nothing needs to be done is not free because the UoW actually has to check whether nothing needs to be done). Both of these problems could be addressed with some sort of flags, but the question still is whether its not better to flush manually in the first place.
That would mean, in your example, you should flush after persisting the new objects, irrespectively of what code comes next, you persisted (a) new object(s) and you want to make sure these are fully available to the rest of the script.

Furthermore, automatic flushing when there is no transaction active is probably also not a great idea, as it may split a single unit of work (that was supposed to be atomic) into 2 without the user knowing about it. So auto-flushing should better only happen when a transaction is active (i.e. explicit transaction demarcation is used).

That would mean, in every example, you should flush after persisting new objects, period. If I flush in some cases and not in others, I'm asking for issues that may not be caught by tests. It's an inconsistency that I personally am not comfortable with.

Could be that I'm overlooking something, I've just started playing with D2.

Why is querying against registered objects not viable? It's not easy, granted, but it doesn't seem impossible. There should probably be a layer between the UoW and the "persisters" (Data Mappers?).

RE: the UoW double flush: state management on the UoW as a whole should prevent that. i.e. after a commit the whole UoW is clean? Just a suggestion, as I said, still getting my bearings.

On a side note I just want to say that what I've seen so far, for the better part, pleases me greatly. Kudos.

@"That would mean, in every example, you should flush after persisting new objects, period."

Yes, if you want the objects to be visible to queries in the same request. Generally, you should flush when you complete a unit of work and that is usually not the whole request (but can be).

I don't want to "query" against registered objects because it is a) not easy b) likely a lot of code and c) very likely error-prone. And in addition I don't see this helping with solving any inconsistency. If you want to use find() you have to flush anyway because you can not find() without having the identifier in the first place, which is only available after a flush.

@RE: the UoW double flush:

Yes, like I said, it can be done but it is a compromise. Having a "clean/dirty" flag in addition to calculating the changesets of the work to do (which implicitly tells us whether the UoW is dirty) adds more code and more potential for errors. Forget to update the flag in one location and you get flushes that don't do anything, because the flag was not updated. A dirty-flag for the UoW is not really required for proper working. It is similar to the approach of maintaining a separate counter for the number of elements in a collection implementation: can make many size/count requests faster but complicates the internal implementation and increases the likelihood for errors (and lock contention for the counter in a thread-safe/concurrent implementation, an interesting case where performance goes against scalability, but I digress and that does not apply to php obviously). That said, I am not strongly opposed to doing this.

If you're interested in how this is specified by "big brother", take a look at section 3.8.7 of the JPA 2 specification. Shortly, with the default behavior it requires the implementation to ensure that unflushed changes are visible to queries which can be achieved by flushing these to the database automatically but only if a transaction is active, otherwise the implementation must not flush to the database. There is alternatively also a "MANUAL" flush mode, in that case the effect of updates made to entities in the UoW upon queries is unspecified. We do not have different flush modes anymore, however, in Doctrine.

So I see two possible ways to go here:

1) More effort, more code, (really better?)

• Maintaining a dirty flag in the UoW (this could be done anyway at some point, even if 2) is chosen)
• Maintaining a flag to avoid infinite recursion triggered from events within a UoW commit/flush
• Flushing automatically when querying while there are pending inserts and a transaction is active

2) No effort, less code

• Removing the current auto-flush on DQL queries which is still subject to infinite recursion
• No automatic flushes, anywhere (less magic, so to speak?)
• Clearly documenting that new, unflushed entities are not visible in subsequent queries issued in the same request, and if this is desired, a flush should be issued.

That's how I see it. Now we need some votes and volunteers for the implementation Personally, I am not sure yet about which version I prefer, 2) does not sound too bad for me.

In Nr. 1) the case with the infinite recursion may actually be more problematic. I think you simply can not see unflushed new objects in queries made during a UoW commit.

When there's no in-memory objects inclusion, I'd say 2) as well. Again, I have no idea how this is implemented currently, but I would prefer something like this:

$repo->start();

$repo->register($object);

$repo->commit();

Why?

• Commit instead of flush: "flush" has little semantic value IMO, "commit" leaves no questions: you're committing your changes (which implies that they are not, before)
• Operating on the repo leaves no question to what you are committing: changes of the associated type and relations configured to cascade, made after start()
• Register instead of persist: "persist" is misleading as the object is not immediately persisted, and as my example shows, may not be.

The way I see it "start" would create a UoW associated with the repo, "commit" would calculate changes and write (the enitity manager would make sure references in other UoWs are removed).

Because the way it is currently implemented (or so it seems), it's unclear when to flush and when not to flush, and unclear what I'm flushing at any one point in the code (because it is not locally isolated). If I have to decide whether to flush in some bit of client code, I am apparently making an assumption about the target entity, i.e. coupling.

I know, you already went beta, so it's unlikely you would consider such a large change, but anyway, for your consideration.

Finally, I realize I'm borderline nagging now as you've made it clear you see nothing it, but a Repository (as in the PoEAA pattern, p 322) may provide a method of fetching native in-memory objects using criteria, acting as a "buffer" between code and database. The Repository in D2 does effectively nothing but delegate to the UoW (or mostly to the underlying persister). Ref PoEAA 327 for an example of an in-memory strategy. As a final point of critique, the Repository does not always seem to be used as entry point for data requests, which is the whole point of the pattern. Most of what's in EntityManager, should be in EntityRepository ("manager" is a bit to abstract a concept to expect clear responsibilities anyway). EntityManager::find() delegates to EntityRepository, but pretty much everything else is the other way around. EntityManager would be better off named DataGateway, as that accurately describes its intended function.

I admit, it would be very difficult to use DQL on in memory objects, but it would be far superior and if it work lead to much more predictable behaviour. It's the ONLY way the data store is ever going to be truly transparent. A few examples (DQL from the docs):

SELECT
u,
UPPER(u.name) nameUpper
FROM MyProject\Model\User u

• Fetch everything from the db
• Select all objects from the User UoW
• Iterate over the in memory ones and modify the name property to upper case
• Merge the results and return

SELECT u FROM User u WHERE u.id = ?1 OR u.nickname LIKE ?2 ORDER BY u.surname DESC

• Execute against database
• Iterate over the User UoW, indexing by "surname", adding items that match the criteria
• Merge the results and return

With joins it could get more complex, provided you want to intelligently merge results into existing objects. Question is whether that is really needed, but there's obviously a performance benefit. Actually this may already be implemented.

I suspect there are edge cases, rooted in DQL still being based on SQL, but in theory it should be possible. Likely you would still want to do start(), and delegate to the driver to start an actual transaction to prevent inconsistent reads... The only way to find out if it's truly feasible is to to try it, I think.

Ramble, ramble, ramble, I'm done. I know I seem critical, but it's positive critique, I love the direction you went with D2.

Maybe I was not clear, with approach Nr.1 there would be in-memory objects inclusion (of new objects), in fact, there always is, due to the identity map. When you query for objects and some of them are already in memory, these are used, not again reconstructed.

The EntityRepository provided by Doctrine is just a convenient mechanism for writing your own repositories. There are many different understandings for what a repository is, you can make it whatever you want it to be. Is a PoEAA repository the same as a DDD repository? Anyway, the repository could be stripped of the project, it is optional, the state management is handled by the EntityManager and UnitOfWork. These are the core components. I agree that the delegation from EntityManager#find to the repository is suboptimal in this regard and should be the other way around.

Now to your question: "When should I flush?". Generally, you should flush at the end of a transaction, which in turn is a unit of work. That means, use explicit transaction demarcation. begin() ... flush() commit(). I've added some control abstractions recently that should make this even easier. I can only recommend to explicitly demarcate your transaction boundaries. As you probably know, you can not talk to your database outside of a transaction anyway. The default behavior (flush() wrapping all its stuff in a transaction) is for convenience mostly and so as not to alienate or confuse people even more who are used to autocommit mode.

Concerning the naming, we mostly stick with the JPA specification and I, for one, really like the naming and I don't want to invent new names. PoEAA is far more abstract (and the examples far too specific) than what is specified in JPA, so I recommend giving that a read. The patterns in PoEAA obviously and intentionally leave a lot of room for different variants of implementation and also leave open a lot of open questions (many of the difficult questions especially, it is for a reason that the author recommends using an existing tool instead of writing your own). In my opinion it is just not feasible to query in-memory objects in a generic way, all the examples in PoEAA do not have generic but rather concrete code examples, which is obviously a lot easier. The feasible strategy, and that is what we do, is to do in-memory lookups only when querying by PK, otherwise the query is executed and afterwards nevertheless any objects reused that are already in memory (based on the PK) and not reconstructed. This is the approach we use.

Thanks for your input, I do see that you are an experienced fellow in object-relational persistence, maybe we can see you as a committer some day?

@ "SELECT u, UPPER(u.name) nameUpper FROM MyProject\Model\User u"

This selects all users and their names in uppercase, the uppercase names are scalar values, the users are not modified! Scalar values are separate from objects.

@ "... and unclear what I'm flushing at any one point in the code"

flush() means: Synchronize the in-memory state of my objects with the database, making any changes that are only in-memory persistent. Nothing more, nothing less.

Again, objects are always reused based on the identity map and the state that is in-memory prevails, unless you use refresh() or execute a query with the Query::HINT_REFRESH query hint. All objects you fetch from DQL, be it as a root object or as a joined association, are first looked up in-memory (but after the SQL query has been issued!).

Maybe we have been talking past each other here, what I refer to as not feasible is querying the in-memory objects first in some way, even before the SQL query. This is just too complicated and error-prone, except for the simple case of a PK lookup and that is where we do it already.

> Scalar values are separate from objects.

Right. Bad example.

> flush() means: Synchronize the in-memory state of my objects with the database, making any changes that are only in-memory persistent. Nothing more, nothing less.

I realize that it means that, but commit() would be more obvious.

> Maybe we have been talking past each other here, what I refer to as not feasible is querying the in-memory objects first in some way, even before the SQL query. This is just too complicated and error-prone, except for the simple case of a PK lookup and that is where we do it already.

Fair enough, you don't think it's feasible, so we'll keep it at that. Maybe I'll give it a shot some time.

Might need to be pushed back to a 2.0.x / 2.x.x bugfix release. Not clear yet.

Ubuntu 10.04 + Oracle 11g Entreprise + PHP 5.3.2 + Doctrine2 Git (up-to-date)

This is the continuation of http://www.doctrine-project.org/jira/browse/DDC-616. Only the schema is different.

just for understanding this scenario:

Is this a One-To-One relation and the TABLE_TEST2 "inherits" the primary key from its parent TABLE_TEST1?

If yes, this construct is not yet supported by Doctrine 2, we still need to include an ID-Generator that supports this kind of schema.

Change for a more understandable use case. Note that it's not my real use case and that I work on legacy database on which I can't change the structure.

updated the issue topic to get a better grasp of what needs to be done here.

I have the same error with Mysql whit the same condition.

More details on the work to be done:

The relevant code is in Doctrine/ORM/Mapping/Driver/DatabaseDriver.php only.

The idea is currently many-to-many tables are detected by checking that the table has foreign keys on all the primary key columns (no additional columns!)

Now with the 2.1 feature of foreign key/primary key entities this is not necessarily true anymore. You can have the primary keys being foreign keys BUT have additional columns that are not part of the primary key. This has to be detected.

If a foreign key-primary-key entity is found that has additional columns a ClassMetadata has to be created and the associations have to be created with the "id" => true flag in mapManyToOne().

For what it's worth, I'm getting this error when I have a PK that is a single column and not a FK.

PRIMARY KEY (`id`),
UNIQUE KEY `cycle_station_id` (`cycle`,`station_id`),
KEY `station_id_idx` (`station_id`),
KEY `readings` (`readings`),
KEY `source` (`source`),
KEY `temperature_min_max` (`temperature_max`,`temperature_min`),
KEY `station_id_cycle` (`station_id`,`cycle`,`updated_at`),
CONSTRAINT `compiled_1_station_id_stations_id` FOREIGN KEY (`station_id`) REFERENCES `stations` (`id`),
CONSTRAINT `compiled_1_station_id_stations_id_1` FOREIGN KEY (`station_id`) REFERENCES `stations` (`id`) ON DELETE CASCADE
) ENGINE=InnoDB AUTO_INCREMENT=160833690 DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci

If this is to be implemented for 2.0, it needs to happen for RC1, therefore rescheduling to RC1. Feel free to reschedule to 2.x if necessary.

Only oracle supports lock timeouts and no other vendor seems to plan to support it. I move to 2.x, but i guess this would rather be an issue of user extension.

Doctrine_Record defines a replace() method.

In the MySQL Doctrine implementation, however, it is not the same as INSERT .. ON DUPLICATE KEY UPDATE. The replace() method implemented in Doctrine_Connection_Mysql uses the REPLACE INTO syntax, which is a DELETE and then INSERT when the key exists. This is fine, except for tables that use auto-increment fields. The delete-then-insert operation yields a new auto-incremented value, whereas INSERT .. ON DUPLICTATE KEY UPDATE would not.

MySQL (and SQLite) REPLACE is a no go. It causes way too much disc I/O and worse yet totally screws up the on disk data structures because of the deleting.

Scheduled for 2.2

Evaluating this makes me sad, except MySQL support for this is rather non-existant, and the oracle merge is aiming at batch operations.

Should this be done with

1. Select first, then insert
2. Catch and evaluate exception then update

I am leaning towards 1.

Updating fix version

I think to preserve the semantics the following has to happen:

"on-delete" => "cascade" has to implicitly set cascade = remove. This hurts performance of course vs just using the on-delete, however it won't corrupt the UoW.

Not entirely would it hurt performance, you could check if on-delete => cascade is set. If this is the case you wouldnt need to do an explicit remove using the UnitOfWorks cascade.

Changed to improvement

ORACLE db SYMFONY

Ok this is what refresh() with LOCK support is actually needed for:

    public function lock($entity, $lockMode, $lockVersion = null)
    {
        if ($this->getEntityState($entity) != self::STATE_MANAGED) {
            throw new InvalidArgumentException("Entity is not MANAGED.");
        } else if ($entity instanceof Proxy && $entity->__isInitialized__) {
            $this->refresh(....); // with LOCK!
        }
        ...
    }

What about an @IdGetter annotation. A function instrumented like this would not trigger the lazy load within the proxy.

Something like

class Entity {
    /** @Id **/
    private $id;

    /** @IdGetter **/
    public function getId() {
       return $this->id;
    }
}

would then result in the proxy implementation

class EntityProxy extends Entity {
  
    public function getId() {
        if (!$this->__isInitialized__) {
            return $this->_identifier;
         } else {
             return parent::getId();
        }
    }
}

After reading the original post I realized that it proposed nearly the same thing. Nevertheless I'll leave it here for clarity. I still think that an annotation on a function would be better, than an annotation which gets the function name as a parameter.

Regards Stefan

$this->_identifier is an array.

Mac OS X 10.6; PHP 5.3.2; MySQL 5.1.44

This is actually currently expected. You can not get access to the original data in the onFlush event right now. I'm not saying that this will never be possible but it is simply the way it works at the moment.

Does this mean that it is currently impossible to implement a Versionable mechanism using snapshots?

You can hold a map of them yourself if your listener also implements the "postLoad" event:

$entity = $args->getentity();
$this->originalData[spl_object_hash($entity)] = $args->getEntityManager()->getUnitOfWork()->getOriginalData($entity);

Changed into possible improvement for the future

This very minimal patch removes the use of these temporary files:

--- library/Doctrine/ORM/Proxy/ProxyFactory.php	(revision 2)
+++ library/Doctrine/ORM/Proxy/ProxyFactory.php	(working copy)
@@ -78,9 +78,8 @@
         $fqn = $this->_proxyNamespace . '\\' . $proxyClassName;
 
         if ($this->_autoGenerate && ! class_exists($fqn, false)) {
-            $fileName = $this->_proxyDir . DIRECTORY_SEPARATOR . $proxyClassName . '.php';
-            $this->_generateProxyClass($this->_em->getClassMetadata($className), $proxyClassName, $fileName, self::$_proxyClassTemplate);
-            require $fileName;
+            $file = $this->_generateProxyClass($this->_em->getClassMetadata($className), $proxyClassName, null, self::$_proxyClassTemplate);
+            eval('?>'.$file);
         }
 
         if ( ! $this->_em->getMetadataFactory()->hasMetadataFor($fqn)) {
@@ -144,6 +143,9 @@
 
         $file = str_replace($placeholders, $replacements, $file);
 
+        if ($fileName === null)
+            return $file;
+
         file_put_contents($fileName, $file);
     }
 

The proxy dir is used for the "doctrine orm:generate-proxies" command in the case of "autogenerate= false", so you need to define it anyways.

You have to use proxies, the option is not for Proxy yes/no. If you have autogenerate=false and doctrine requires a proxy for a use case but can't find it you will get a fatal error.

I just saw the eval() keyword, ieeks It could maybe be a convenience option for those that don't want to use proxy direcotiries, however i am not sure.

eval() is no different than writing code to a file and using require().

When using runtime-generated proxies, there are no benefits (that I know of) from writing them to a file. The disadvantages are:

• slower because of write disk access
• has problems with high concurrency, unless special care is taken (DDC-716)
• potentially has permission problems if code is executed by different users (e.g. nobody for a daemon and www-data for http)
• requires setup of a writable directory

This is a nicer patch, which makes _generateProxyClass() return a string, just like other _generate* methods:

--- library/Doctrine/ORM/Proxy/ProxyFactory.php	(revision 2)
+++ library/Doctrine/ORM/Proxy/ProxyFactory.php	(working copy)
@@ -78,9 +78,8 @@
         $fqn = $this->_proxyNamespace . '\\' . $proxyClassName;
 
         if ($this->_autoGenerate && ! class_exists($fqn, false)) {
-            $fileName = $this->_proxyDir . DIRECTORY_SEPARATOR . $proxyClassName . '.php';
-            $this->_generateProxyClass($this->_em->getClassMetadata($className), $proxyClassName, $fileName, self::$_proxyClassTemplate);
-            require $fileName;
+            $code = $this->_generateProxyClass($this->_em->getClassMetadata($className), $proxyClassName);
+            eval($code);
         }
 
         if ( ! $this->_em->getMetadataFactory()->hasMetadataFor($fqn)) {
@@ -107,19 +106,19 @@
         foreach ($classes as $class) {
             $proxyClassName = str_replace('\\', '', $class->name) . 'Proxy';
             $proxyFileName = $proxyDir . $proxyClassName . '.php';
-            $this->_generateProxyClass($class, $proxyClassName, $proxyFileName, self::$_proxyClassTemplate);
+            $code = $this->_generateProxyClass($class, $proxyClassName);
+            file_put_contents($proxyFileName, "<?php\n" . $code);
         }
     }
 
     /**
      * Generates a proxy class file.
      *
-     * @param $class
-     * @param $originalClassName
+     * @param ClassMetadata $class
      * @param $proxyClassName
-     * @param $file The path of the file to write to.
+     * @return string The code of the generated methods.
      */
-    private function _generateProxyClass($class, $proxyClassName, $fileName, $file)
+    private function _generateProxyClass(ClassMetadata $class, $proxyClassName)
     {
         $methods = $this->_generateMethods($class);
         $sleepImpl = $this->_generateSleep($class);
@@ -142,9 +141,9 @@
             $methods, $sleepImpl
         );
 
-        $file = str_replace($placeholders, $replacements, $file);
+        $file = str_replace($placeholders, $replacements, self::$_proxyClassTemplate);
 
-        file_put_contents($fileName, $file);
+        return $file;
     }
 
     /**
@@ -244,8 +243,7 @@
 
     /** Proxy class code template */
     private static $_proxyClassTemplate =
-'<?php
-
+'
 namespace <namespace>;
 
 /**

Scheduled usage of eval() for 2.1, if the following conditions exist:

1. Autogenerate is set to TRUE
2. No Proxy Directory is configured.

Great, this is even better. This way you can have 1) autogenerated in ram-only, 2) autogenerated in files and 3) pregenerated.

And the minimal amount of config needed to get up and running is reduced, which is always nice.

you should know though, eval is dead slow. It generates the necessary proxies on EACH request and that cannot be cached in APC.

It's no slower than current autogeneration (file_put_contents+require). TBH, I don't know why anyone would want to use that over eval(), but I don't mind it being there.

Pre-generation is, of course, a different thing. Seems like a valid tradeoff to offer: build/a bit of config/better perfomance vs. no build/no config/potentially slower.

Yes, that is because file_put_contents + require is a development only strategy. The manual clearly states that autogenerate has to be false in production.

Using eval() instead of producing and requiring the file in the case of enabled auto-generation of proxy classes sounds like a good improvement for 2.1 to make proxies more transparent during deveopment and for anyone for whom performance is no issue.

I'm increasing the priority as I think it is easy to implement for 2.1 and a good enhancement.

A note on why having the proxies written to a file can be useful even with autogenerate being on: it makes it really easy to check the proxy code being generated. I use that a lot currently.

The solution suggested, giving three possibilities is cool, though.

@Roman A possible fix for this in my opinion is another map in UnitOfWork $mergedEntities = array(); and a patch like this:

diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php
index 242d84b..1d0d8b3 100644
--- a/lib/Doctrine/ORM/UnitOfWork.php
+++ b/lib/Doctrine/ORM/UnitOfWork.php
@@ -1340,6 +1340,10 @@ class UnitOfWork implements PropertyChangedListener
             return; // Prevent infinite recursion
         }
 
+        if (isset($this->mergedEntities[$oid])) {
+            return $this->mergedEntities[$oid];
+        }
+
         $visited[$oid] = $entity; // mark visited
 
         $class = $this->em->getClassMetadata(get_class($entity));
@@ -1468,6 +1472,8 @@ class UnitOfWork implements PropertyChangedListener
 
         $this->cascadeMerge($entity, $managedCopy, $visited);
 
+        $this->mergedEntities[$oid] = $managedCopy;
+
         return $managedCopy;
     }

I have tested this patch with my application and it fixes the problem in all my relevant test cases apart from one. The test case that's failing is one that persists a bi-directional many to many relationship, so the associations interweave with each other (if you know what I mean).

I wonder if perhaps doMerge need to continue cascading even if it finds an item in $this->mergedEntities

This is the Flextrine code that fails - it results in no entries in movie_artist. This might also be related to DDC-758?

m1 = new Movie();
m1.title = "Movie 1";

m2 = new Movie();
m2.title = "Movie 2";

a1 = new Artist();
a1.name = "Artist 1";

a2 = new Artist();
a2.name = "Artist 2";

m1.artists.addItem(a1); a1.movies.addItem(m1);
m1.artists.addItem(a2); a2.movies.addItem(m1);

m2.artists.addItem(a1); a1.movies.addItem(m2);
m2.artists.addItem(a2); a2.movies.addItem(m2);

// These translate to cascade merges on the server
em.persist(m1);
em.persist(m2);
em.persist(a1);
em.persist(a2);

// Now flush
em.flush();

P.S. This test passes if I translate em.persist() to $em->persist() (not cascading) on the server instead of translating it to a cascade merge; not sure if that helps

I'd really like to avoid introducing an additional instance variable just to solve this issue but I did not find the time yet to really look into it.

Does someone have a unit test for this already and can attach it to the issue?

Rescheduling for RC1.

Here is a functional test case containing three tests:

testMultiMerge tests basic merging of two new entities, checking that only a single entity ends up in the database. This passes with Benjamin's patch.

testMultiCascadeMerge tests the more complex case of merging a OneToMany association. This also passes with Benjamin's patch.

testManyToManyPersistByReachability tests the ManyToMany case described above and this fails with Benjamin's patch, probably because doMerge doesn't cascade down entities that it has already merged and some ManyToMany associations are being ignored. Its a bit hard to be certain what is causing this as even without Benjamin's patch this test would fail due to DDC-758.

@Roman i thought about this issue, its not possible without that additional map of merged entities. There is no way we can get that information from other sources.

Problem is rather that the use-case probably only applies in mass-merging scenarios and client-server serialization.

Added another failing test case - adding the same entity from different ends of a many to many bi-directional association to check that there isn't an integrity constraint violation caused by Doctrine trying to add the same row twice.

Attached a patch for this issue.

can you comment why all the additionall stuff is necessary compared to my patch?

It fixes the two additional test cases - testManyToManyPersistByReachability and testManyToManyDuplicatePersistByReachability.

testManyToManyPersistByReachability was failing with your original patch because there are ManyToMany cases where an entity may have already been merged, but its still necessary to add it to an association and continue to cascade. Running the following with the original patch will miss out some of the associations.

$m1 = new Movie();
$m1->title = "Movie 1";

$m2 = new Movie();
$m2->title = "Movie 2";

$a1 = new Artist();
$a1->name = "Artist 1";

$a2 = new Artist();
$a2->name = "Artist 2";

$m1->artists->add($a1); $a1->movies->add($m1);
$m1->artists->add($a2); $a2->movies->add($m1);
$m2->artists->add($a1); $a1->movies->add($m2);
$m2->artists->add($a2); $a2->movies->add($m2);

$em->merge($a1);
$em->merge($a2);
$em->flush();

The other change in my patch is to protect against this case. It ensures that the following code doesn't add the same entity twice to a collection.

$em->merge($m1);
$em->merge($m2);
$em->merge($a2);
$em->merge($a2);
$em->flush();

I am not sure if the issue here is rather multiple calls to merge that contain different parts of the same object-graph.

There should be a very simple fix for this, call ->clear() after each merge.

I am not sure if this patch drags us into a blackhole of issues with merging.

Calling ->clear() and ->flush() after each merge is a workaround for the simple case, but unless I am misunderstanding I don't think its a solution for cases where the merging is happening automatically in cascadeMerge. I've actually encountered this issue in another project and scenario to do with creating REST APIs and merging JSON objects into entities, and applying the patch fixed it so a) I think this issue might be a more common that we first thought and b) the patch basically seems to work (plus it doesn't introduce any failing cases in the existing test suite). I can actually still find one edge case to do with cascading merging interlinked many to many associations that this doesn't fix, but I was planning to open that as a new ticket after this My feeling is that the current merge already has issues and this definitely improves it.

It cannot happen inside a single merge, single merges use the $visited to avoid infinite recursions, each entity can only be merged once inside a single merge operation.

Added a note into the documentation about using EntityManager#clear between merging of entities which share subgraphs and cascade merge.

Handling this issue in UnitOfwork will be declared an improvement, not a bug anymore and be scheduled for later releases. The required changes to the core are to dangerous and big.

Where in the docs is that?

Just to summarize, the equivalent operation to having multiple merges and a single flush is to call merge followed by flush each time, with the whole thing surrounded by a transaction? Does this have a big impact on performance?

Ben - even given the decision not to implement this (and I do understand your thinking, as it is a major change), is there any reason not to implement the bit that ensures that the same entity isn't added to a collection twice during a merge? I can't think of a situation where this should be allowed, and I have a use case where I get 'DUPLICATE KEY' errors if this isn't there.

Please see attached patch.

What bit of that huge patch is that? Can you extract it into another ticket if thats possible?

I added it to "Working with Objects" and the descripton of Merge. Its not yet live on the site.

Using this current workaround has a performance impact, since more SELECT statements have to be issued against the database.

Apologies for not being clear - only the 3rd patch (multipleaddmerge.diff) is relevant to the 'DUPLICATE KEY' error I am now talking about, but I'll put it in a nother ticket if you prefer.

please add a new ticket, patch looks good.

Created as DDC-875

That would obviously produce wrong results. Maybe you can elaborate more with an example.

I use ENUM("foo","bar") as discriminator columns. That means, the column will contain the right values out of the box, no further result set limiting required with WHERE.

Windows 7, Apache 2.2, MSSQL Server, PHP 5.3.3

Hi, i try to make my custom type for binary field in MSSQL.
I don't find own, can you please show me your custom type binary?
Thks a lot.

That is documented and for perfomance reasons we cannot move the preUpdate/postUpdate/prePersist/postPersist events to other locations inside the UnitOfWork.

There is an onFlush event that allows for more flexibility and is triggered before any update/insert/delete is done by the UnitOfWork.

Thanks.

I understand that. Is there any chance of getting some onPostFlush or similar, which would be triggered like onFlush, but after all update/insert/delete ? Or just some post-something event which is allowed to issue db queries.

onFlush you can store your entity for furher processing and on postPersist you can check if there are no more insertions and process the entity if it needs additional query
I have faced all these issues and you can check http://github.com/l3pp4rd/DoctrineExtensions/tree/master/lib/DoctrineExtensions/Translatable/
for a solution to your problem

I think this issue should be closed since the main reason of opening it was the possibility to execute additional queries when inserts were pending in unit of work. In current release it does not cause a flush during an additional query execution anymore.

From reading the issue i know what the bug is, indeed this sucks.

@Jon: Any more information coming?

@Benjamin: Can you summarize the essence of the issue shortly?

@Roman: The UnitOfWork (may) still be pushed as a listener into that entity, and still recieve noticies of update. Which may throw notices because the oid hashes are removed everywhere. Additionally you cant serialize the thing because you still got the UoW inside there.

I don't have anymore information currently. The issue was relayed to me. I will try and find some more information and report back.

There is no way to "fix" this issue, i am turning it into a feature request. There needs to be a "postDetach" event that is triggered where the developer can detach the change notification objects.

There is another approach for this using several subqueries to build an IN clause, the Paginator extension supports this: http://github.com/beberlei/DoctrineExtensions

I rather go the extension approach than changing the DQL for this feature.

I just saw your second example, that is rather cool though and gets +1 from me.

I had the same idea for "not initialized proxies", i.e.

$em->getUnitOfWork()->initializeProxies('Customer');

Second example is a duplicate of DDC-734

I have to note that the code I listed above turned out to be broken. There is nothing that guarantees that a data object just merged will not become detached again after being merged on assignment, unless the object is immediately persisted afterwards.

The correct solution would be to merge all data objects found through relationships for a given data object, right from the persistence manager, immediately before calling persist() on the data object.

I am currently using this solution (save() saves a data object safely for use within long-running batch jobs):

   public static function save(DataObject $dataObject) {
      
      self::mergeRelatedDataObjectsIfDetached($dataObject);
      
      self::$doctrineEntityManager->persist($dataObject);
   }
   

   public static function merge(DataObject $dataObject) {
      
      return self::$doctrineEntityManager->merge($dataObject);
   }


  protected static function mergeRelatedDataObjectsIfDetached(DataObject $dataObject) {
      
      $reflectionClass = new ReflectionClass($dataObject);
      
      $properties = $reflectionClass->getProperties();
      
      foreach ($properties as $property) {

         $propertyName = $property->getName();
         
         $propertyValue = $dataObject->__get($propertyName);
         
         
         if (MetadataReader::isToOneRelationship($property)) {
            
            if (   $propertyValue !== null
                && ! $propertyValue instanceof Proxy
                && self::isDetached($propertyValue)) {
               
               $relatedDataObject = self::merge($propertyValue);
               
               $dataObject->__set($propertyName, $relatedDataObject);
            }
         }
         else {
            
            if (MetadataReader::isToManyRelationship($property)) {
               
               $relatedDataObjects = $propertyValue->toArray();
               
               foreach ($relatedDataObjects as $index => $relatedDataObject) {
                  
                  if ( ! $relatedDataObject instanceof Proxy
                      && self::isDetached($relatedDataObject)) {
                     
                     $relatedDataObject = self::merge($relatedDataObject);
                     
                     
                     // Replace the entry in the collection with the merged copy.
                     
                     $propertyValue->set($index, $relatedDataObject);
                  }
               }
            }
         }
      }
   }
   
   
   protected static function isDetached(DataObject $dataObject) {
      
      return self::$doctrineEntityManager->getUnitOfWork()->getEntityState($dataObject) == UnitOfWork::STATE_DETACHED;
   }

I still wish there would be an automerge feature, kind of Hibernate's "update".

Wrapped the code sections into proper code blocks...

Subselect as columns or FROM clause should have mor examples.

FreeBSD, PostgreSQL 8.4

http://ilia.ws/archives/211-Igbinary,-The-great-serializer.html#extended

Descheduled to 2.x

err sorry .. this is of course a DBAL doc issue not and ORM issue.

Requirements for batching of eager loads:

1. Since we are using an IN() query for this we can only support this feature for entities that have a single column primary key.
2. If we want to support composite keys we need to build it as WHERE ( (id1 = ? AND id2 = ?) OR (id1 = ? AND id2 = ?)) but this is currently not possible with the way how internally the $criteria array is used.

Next item to think about: What if an exception or event breaks the flow and "triggerEagerLoads()" is never called?

All

I have the same problem and I didn't found documentation to select the engine for a given table.

I tried to understand the code by myself and made modifications of my Doctrine v2.0.5, perhaps this is not the best one but here is a beginning of a solution :

-------------------------------------------------------------------------------

@ ORM\Mapping\Driver\DoctrineAnnotations.php:100

final class Table extends Annotation

@ ORM\Mapping\Driver\AnnotationDriver.php:144

$primaryTable = array(
'name' => $tableAnnot->name,
'schema' => $tableAnnot->schema,
'engine' => $tableAnnot->engine
);

@ ORM\Tools\SchemaTool.php:133

$table = $schema->createTable($class->getQuotedTableName($this->_platform));

if ( isset($class->table["engine"]) )
$table->addOption("engine",$class->table["engine"]);

-------------------------------------------------------------------------------

...and to define for instance the MyISAM engine in annotations :

/**

• @Entity
• @Table(engine="MyISAM")
  */

Because there is no foreign key on MyISAM tables, there are still problems on the schema creation/update when Doctrine executes the corresponding "alter table" SQL commands.

in Doctrine 2.2.2 will be

Doctrine/ORM/Tools/SchemaTool.php-149- /// PATCH ------------------------------------
Doctrine/ORM/Tools/SchemaTool.php-150- if ( isset($class->table["engine"]) )
Doctrine/ORM/Tools/SchemaTool.php-151- $table->addOption("engine",$class->table["engine"]);
Doctrine/ORM/Tools/SchemaTool.php-152- /// PATCH ------------------------------------

Doctrine/ORM/Mapping/ClassMetadataInfo.php-1719- /// PATCH ------------------------------------
Doctrine/ORM/Mapping/ClassMetadataInfo.php-1720- if (isset($table['engine']))

Doctrine/ORM/Mapping/ClassMetadataInfo.php-1723- /// PATCH ------------------------------------

Doctrine/ORM/Mapping/Table.php-42- /// PATCH ------------------------------------
Doctrine/ORM/Mapping/Table.php-43- /** @var string */
Doctrine/ORM/Mapping/Table.php-44- public $engine;
Doctrine/ORM/Mapping/Table.php-45- /// PATCH ------------------------------------

Doctrine/ORM/Mapping/Driver/AnnotationDriver.php-181- /// PATCH ------------------------------------
Doctrine/ORM/Mapping/Driver/AnnotationDriver.php-182- $primaryTable['engine'] = $tableAnnot->engine;
Doctrine/ORM/Mapping/Driver/AnnotationDriver.php-183- /// PATCH ------------------------------------

i found a problem in ManyToMany relations, the intermediate table will be InnoDB

You can do all tables as MyISAM already through metadata using @Table(options=

) except Many-To-Many Join Tables. But you could create a listener to the "postSchemaGenerate" event and accept the schema instance there, modify everything accordingly.

should we add the support of the options for the @JoinTable annotation ?

Yes, that is probably what this ticket boils down to

Hi, this doesn't seem to work for me. I have written a small database export / import utility. As long as I use the automatic ID generation, everything works flawlessly, but I'm trying to preserve the existing IDs. I do exactly what you've suggested in your post. It works for @OneToOne relations, but I get the following error messages when persisting entities that are parts of @ManyToOne relations:
Notice: Undefined index: [....] in [...]Doctrine/ORM/UnitOfWork.php on line 2655
I'm using version 2.2.2
Am I doing something wrong?

Never mind. I've upgraded to Doctrine 2.3.0 and it works as expected.