[DDC-1069] setParameter[s]() examples on DQL page uses ":" even though that is not supported Created: 13/Mar/11  Updated: 18/Nov/11  Resolved: 29/Mar/11

Status: Resolved
Project: Doctrine 2 - ORM
Component/s: ORM
Affects Version/s: 2.0.2
Fix Version/s: 2.0.4
Security Level: All

Type: Documentation Priority: Minor
Reporter: André R. Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None


 Description   

http://www.doctrine-project.org/docs/orm/2.0/en/reference/dql-doctrine-query-language.html

Among others:
$query = $em->createQuery('SELECT u from ForumUser u WHERE (u.username = :name OR u.username = :name2) AND u.id = :id');
$query->setParameters(array(
':name' => 'Bob',
':name2' => 'Alice',
':id' => 321,
));

Despite this does not work and doc says "When referencing the parameters in Query#setParameter($param, $value) both named and positional parameters are used without their prefixies." on same page.
(side note: prefixies => prefixes)

Fresh on doctrine 2.0 you typically copy past examples to try out stuff, and when exception then says that ":id" is not a valid param on this query you end up not knowing what went wrong.



 Comments   
Comment by Michael Ridgway [ 21/Mar/11 ]

I posted a simple patch for this at https://github.com/doctrine/orm-documentation/pull/20

Comment by André R. [ 22/Mar/11 ]

Looks good!

Comment by Benjamin Eberlei [ 29/Mar/11 ]

Resolved

Comment by Christian Stoller [ 18/Nov/11 ]

You can find the same error in the PHPDoc of the methods setParameter(...) and setParameters(..) in class Doctrine\ORM\QueryBuilder.

* <code>
*     $qb = $em->createQueryBuilder()
*         ->select('u')
*         ->from('User', 'u')
*         ->where('u.id = :user_id')
*         ->setParameter(':user_id', 1);
* </code>

It would be nice if this will be fixed, because it is confusing, if you look at the autocomplete notice of the IDEs.





[DDC-1096] "You may have observed that in a mixed result, the object always ends up on index 0 of a result row." Created: 03/Apr/11  Updated: 14/Nov/11  Resolved: 31/Oct/11

Status: Resolved
Project: Doctrine 2 - ORM
Component/s: ORM
Affects Version/s: 2.0.2
Fix Version/s: 2.0.4
Security Level: All

Type: Improvement Priority: Critical
Reporter: Daniel Alvarez Arribas Assignee: Guilherme Blanco
Resolution: Duplicate Votes: 0
Labels: None

Issue Links:
Reference
relates to DDC-1424 Make order in DQL SELECT clause matte... Resolved

 Description   

The DQL reference at

http://www.doctrine-project.org/docs/orm/2.0/en/reference/dql-doctrine-query-language.html

defines that

"You may have observed that in a mixed result, the object always ends up on index 0 of a result row."

I find this too important to state it in such a parenthetic way.

The reordering is counterintuitive behaviour at its worst, and grossly in violation of the principle of least astonishment. I bet noone would even guess that this could be intended behaviour. I would not consider this a classic bug, as it is documented behaviour, but as a way of deliberately doing it wrong, making mixed results totally counterintuitive to use. From a usability standpoint, this is as bad as it gets.

I cannot think of any convincing reason why the ORM should decide on its own, and without any notice, to change the explicitly specified ordering from the select list, because it is absolutely confusing and IMO there is no advantage at all in doing so.

I explicitly told Doctrine 2 to put the entity element on position 3, so why should it end up at position 0 without further notice? If position 0 is a must (which I do not think it should be), then the user should not be able to specify entity elements at any other position than that. As long as other positions are permissible, I would rather have the element stay at the exact position declared, and not be shuffled around.



 Comments   
Comment by Daniel Alvarez Arribas [ 03/Apr/11 ]

And: Is the entry in the manual bad humor? I mean the way the DQL reference puts it ("You may have obverved...") - it even assumes it already took the user some experimentation. Really funny. I wish I had the time back I spent debugging.

Comment by Benjamin Eberlei [ 03/Apr/11 ]

I don't understand your confusion with this. How would you expect it to work? Scalar values have a name, so they are named. Objects dont have a "name" or alias and therefore key-based. Its just an convention.

Yes the sentence may be formulated strangely, but its still stating how it works.

Comment by Daniel Alvarez Arribas [ 03/Apr/11 ]

I would expect this to work using regular numeric indexing. If Doctrine can put something at position 0, it should as well be able to put it at position 3. I do not see where this requires string keys.

Comment by Daniel Alvarez Arribas [ 03/Apr/11 ]

The confusion is due to the total deviation from regular query semantics. The behaviour to be expected is that when selecting Elements (A, B, C), A is listed first in the result rows, B comes second, and then C. Now, in case of mixed results, it can end up (A, C, B), (C, A, B), or (B, A, C), depending on the types of A, B, and C. Now, tell me this isn' confusing.

Comment by Daniel Alvarez Arribas [ 03/Apr/11 ]

To support my point, Scalar expressions or Literals do not have a "name" either.

Comment by Benjamin Eberlei [ 03/Apr/11 ]

Its a requirement to put it on 0 the way how the hydration algorithm works. You would have to add some additional variables to the ResultSetMapping to save the name of an alias or the order with regards to scalar values, which adds additional variables that need to be cached and serialized/unserialized and going by a convention is just easier here.

Scalar expressions and literals can use AS name, and start by a scalar counter of 1 per the requirement that objects start at 0. Multiple objects on the main level come in multiple result rows, so there is no clash possible here.

This is quite different of how sql works with regards to conventions, but fetching an alias which is actually a set of many columns required this shift in conventions in our opinion.

Comment by Daniel Alvarez Arribas [ 03/Apr/11 ]

Multiple rows... geez, I did not think it would get any worse

In a word: counterintuitive. I am getting what you are explaining, nevertheless I perceive it as a way-too-complicated approach, without recompensating the user with advantages.

I would suggest not relying on names at all for positioning. Names should be names and positions should be positions. In fact, names are not even unique.

Consider this query:

SELECT A.somefield, A.somefield, A.somefield FROM \A

I would expect it to return any number of rows with three elements each. It does not. It returns any number of rows with only one element each. That's because the implicit "names" clash internally, and are aliased to the same value, which apparently is not only used to identify the content, but also the whole output field. So in fact the query shown above is equivalent to

SELECT A.somefield FROM \A

which is clearly not what the user asked for. Maybe it is a matter of "hydration", but in this case it would be more straightforward to give the fields unique indexes automatically like "somefield2", "somefield3".

I run into peculiarities like this one several times a week with Doctrine 2. I am not claiming that there is anything buggy about Doctrine 2 in this case. But I definitely have to say Doctrine 2 could behave much more intuitively. Even if it is intended behaviour, it is a nightmare to understand, and it is not something anyone would come up with intuitively by himself.

In the original query example, I did not ask Doctrine to change the order specified, and would never ever ask for the result being split up into multiple rows. From a user perspective, the behavior is in no way desirable.

From what I would expect, I would simply propose to always keep the ordering of fields in the result just as the user specified it, and always return one row for one match. I do not see any advantages in doing it any different, if not for purely technical reasons related to the existing implementation. What can I say - embrace change

Comment by Benjamin Eberlei [ 03/Apr/11 ]

1. The example with a.somefield, a.somefield, a.somefield breaks down with standard SQL aswell leading to exactly the same result, just one "somefield". This is why i would expect everyone coming from an SQL background working with scalar fields to alias everything, which ends with you getting all named fields and a zero key for the object.

2. You can write your own hydrator if you wish to handle the multiple rows per object in the from statement. In that hydrator you could even handle positioning differently (I don't know if thats possible with the current information though).

You can register it with the Configuration and pass its name to "getResult()" on any AbstractQuery. The default hydration has to stay this way obviously for BC reasons. Only negative thing about writing your own hydrator is that you have to copy paste lots of code because all the logic is inlined into one big method for performance reasons.

I will update the docs about mixed results as they clearly lack detail and statements with regard to the conventions.

Comment by Benjamin Eberlei [ 03/Apr/11 ]

Clarified in the docs.

Comment by Daniel Alvarez Arribas [ 04/Apr/11 ]

If I select the same field multiple times in SQL, I get multiple elements per row in the result set. That's an of-course.

Here's the result from PostgreSQL 8.4.7

NameOfMyDatabase=# select dbID, dbID From WPOMOrder limit 1;
 dbid | dbid
------+------
  363 |  363
(1 row)

And here's the result from MySQL Server 5.0

mysql> select dbID, dbID From WPOMOrder limit 1;
+------+------+
| dbID | dbID |
+------+------+
|  363 |  363 |
+------+------+
1 row in set (0.00 sec)

There is no intuitiveness whatsoever about the merge of multiple columns into a single one, neither in SQL, nor by any reasonable common-sense standard. If I specify it twice, I get it twice. And that's the only thing to expect, having explicitly defined it in the query. Now, imagine a user has to "register with the configuration" or "write his own "hydrator""...

Even using Aliasing, the result will still have two columns:

Again, PostgreSQL 8.4.7:

NameOfMyDatabase=# select dbID as A, dbID as A From WPOMOrder limit 1;
  a  |  a
-----+-----
 363 | 363
(1 row)

and here's the result from MySQL Server 5.0:

mysql> select dbID as a, dbID as a From WPOMOrder limit 1;
+-----+-----+
| a   | a   |
+-----+-----+
| 363 | 363 |
+-----+-----+
1 row in set (0.00 sec)
Comment by Benjamin Eberlei [ 04/Apr/11 ]

Well yes, but using mysql_fetch_assoc/pg_fetch_assoc you will getone field again of course. ORM has no concept of numeric fetching at least not with the current hydrators.

Comment by Daniel Alvarez Arribas [ 05/Apr/11 ]

Well, that's what this issue is all about: That a concept of numeric fetching would be highly desirable. That there currently is no such concept is obvious.

In case of the db drivers, there is almost always an option to simply return the row as-is, using numeric indexing, e. g. mysql_fetch_row, or pg_fetch_row. This avoids any unwanted merging of columns by name. If the user chooses the *_fetch_assoc functions instead, name clashes are to be expected. But you do not have to, by design, contrary to Doctrine 2, where this IMO highly broken behaviour is currently inherent.

Comment by Daniel Alvarez Arribas [ 05/Apr/11 ]

Not only that, what bugs me primarily is that it reorders the resulting columns at its own liking.

Comment by Benjamin Eberlei [ 05/Apr/11 ]

It seems that everything that does not work as you want is broken. However there is a perfectly legitmate way to work with the hydrator and youre bloating a problem that is just not there, since DQL is primarily about fetching Objects the numerical fetch mechanism is not nearly as important as in the RMDBS world.

Quoting a sentence from the docs that has nothing to do with numerical fetching as ticket title, saying nothing about that you want this as a feature in the ticket description imho doesnt constitute "this is what is this issue about", i.e. a feature request about adding a numerical fetch mode. You can add a feature request ticket for this and I'll look into it.

Additionally there is a consistent way how Doctrine names "unnamed" columns, its was just not documented good enough yet.

Comment by Daniel Alvarez Arribas [ 06/Apr/11 ]

I changed the issue type to "improvement" to not cause offence. The recent discussion was clearly heading in the wrong direction.

Seriously, if the notion of "index 0" has nothing to do with numerical indexing, I don't know what does. The statements "As long as other positions are permissible, I would rather have the element stay at the exact position declared, and not be shuffled around." and "From what I would expect, I would simply propose to always keep the ordering of fields in the result just as the user specified it" are, to me, as clear as it gets about desiring the described characteristic.

I certainly do not mean any offense. All I want is provide you with feedback. Of course, this is not always possible without voicing a different opinion. Issues are, by nature, mostly things that are wanted differently than they are. As I said right up-front, I did and do not mean to report this as a classical bug, because I knew from the documentation that it was intended, but rather as a severely couterintuitive behaviour, which I still insist it is. I did not enter it as an explicit feature request here, because maintaining the ordering specified is already implemented (in case of non-"mixed" results), and the solution of this issue would rather be to simply always maintain the ordering given, and getting rid of a special case of deliberately overriding the given order for "mixed" results. So I could not see the new feature here. Now, considering it, I think "improvement" nails it.

Getting back to what I originally wished would work differently, I wish that selecting the columns (A, B, C) would return result rows of the form (A, B, C) always, and under no circumstances of the form (A, C, B), (C A B) or (B A C). Column names would be just that, and a column having no name would still be rendered at the column position specified by the ordering in the select expression.

Comment by Benjamin Eberlei [ 06/Apr/11 ]

Do you mean the order regardless of keys?

Say SELECT A B C=> $a = array_shift($result); $b = array_shift($result); $c = array_shift($result) ?

Comment by Daniel Alvarez Arribas [ 07/Apr/11 ]

Yes, that's exactly what I mean (ignoring the shifting side effect on the result of course).

It would be:

$A = reset($result);
$B = next($result);
$C = next($result);

And never, e. g.:

$C = reset($result);
$A = next($result);
$B = next($result);

If you'd rely on numeric indexes, it would be:

Basically: SELECT A, B, C -> $A = $result[0], $B = $result[1], $C = $result[2].

Or: list ($A, $B, $C) = $result;

And never: SELECT A, B, C -> $A = $result[2], $B = $result[0], $C = $result[1].

Or: list ($C, $A, $B) = $result;

Numeric indexing is, though, of course not guaranteed to correctly identify positioning under all circumstances due to the associative-table nature of PHP arrays. E. g. if you populate an array with non-continuous numeric keys, say array (3 = > 'someValue', 5 = 'someOtherValue'), you end up with an array with numeric key 3 at position 0, where the key is clearly not suitable to identify the element's position. But provided you assign numeric keys to the elements in such a way that the numeric keys coincide with the element positions in the array (i. e. the element at position 0 has numeric key 0, and the element at position 1 has numeric key 1), then, of course, you could use numeric keys internally to identify element positions. Then again, you do not need to, the approach of using the de-facto element positions should be more fail-safe.

Comment by Daniel Alvarez Arribas [ 07/Apr/11 ]

Which reminds me... I will have to double-check that it is not the list() calls I do on the result that are screwing things up.

Comment by Daniel Alvarez Arribas [ 08/May/11 ]

Sorry for the late update.

I double checked that the effect is not caused by using the list construct on the result in application code.

For DQL queries of the following form:

SELECT someEntityReference.someStringValuedField, someEntityReference
    FROM \SomeEntity someEntityReference

the fields in the result rows will have got shuffled around as soon as the result is returned by Doctrine 2, i. e. before any list() function or the like gets called on it in application code.

Calling gettype() on the result elements, like:

$result = $aQueryOfTheFormShownAbove->getResult();

$firstResultElement = reset($result);

foreach ($firstResultElement as $field) {
  
   echo gettype($field) . "\n";
}

will output:

object
string

whereas the DQL query explicitly asked for a result of the form (string, object), not (object, string).

To my best knowledge, the foreach construct will rely only on the actual positioning regardless of the key values in the input array.

So the situation is just as I have previously described.

Comment by Benjamin Eberlei [ 31/Oct/11 ]

Result keys Will be implemented as part of DDC-1424

Comment by Guilherme Blanco [ 14/Nov/11 ]

Improvement implemented in https://github.com/doctrine/doctrine2/commit/81cc6d9da83d217ea62bd467053fd9885d1083cd





[DDC-1079] Behavior of Doctrine 2 is not as defined by EBNF definition for SimpleSelectExpression Created: 25/Mar/11  Updated: 29/Oct/11  Resolved: 27/Mar/11

Status: Resolved
Project: Doctrine 2 - ORM
Component/s: None
Affects Version/s: 2.0.2
Fix Version/s: 2.0.4
Security Level: All

Type: Bug Priority: Minor
Reporter: Daniel Alvarez Arribas Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None


 Description   

Queries like

SELECT COUNT(a)
  FROM \a a
 WHERE EXISTS (SELECT 0
      		 FROM \b b
                WHERE b.a = a)

will not work, whereas they should.

Assume that there are potentially millions of b's per a, so this should not be joined, just simply left as-is as a correlated subquery within an exists expression. Actually, a "LIMIT 1" at the end of the subquery would come in handy here, too.

Doctrine 2 will give an error message

[Syntax Error] line 0, col [...]: Error: Expected known function, got '0''

when parsing the DQL statement.

According to the EBNF definition, 0 qualifies as a Literal, which qualifies as an ArithmeticPrimary, which qualifies as an ArithmeticFactor, which qualifies as an ArithmeticTerm, which qualifies as a SimpleArithmeticExpression, which qualifies as a ScalarExpression, which qualifies as a SimpleSelectExpression, which should be perfectly legitimate in that position.

This is yet another annoying case of the Doctrine 2 documentation not matching the behavior of the actual implementation.

Fortunately, it is easy to work around this defect by selecting some field and sacrificing a bit of performance, but then again, it sucks to have to continuously implement workarounds.



 Comments   
Comment by Benjamin Eberlei [ 27/Mar/11 ]

Fixed, didn't work because of a shortcut that was implemented in SimpleSelectExpression. I removed that now and fixed the problem the shortcut was trying to fix at a better location.

Comment by Benjamin Eberlei [ 29/Oct/11 ]

Some more stuff is fixed in regard to this in some lsat ocmmits.





[DDC-1040] Using the same named parameter at various places within a DQL query will result in a PHP fatal error Created: 21/Feb/11  Updated: 05/Apr/11  Resolved: 03/Apr/11

Status: Resolved
Project: Doctrine 2 - ORM
Component/s: None
Affects Version/s: None
Fix Version/s: 2.0.4
Security Level: All

Type: Bug Priority: Major
Reporter: Daniel Alvarez Arribas Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 1
Labels: None

Attachments: File ddc1040.diff    

 Description   

There are perfectly legitimate situations where the same value needs to be used at various places within a single query (e. g. to compare various versioned artifacts against a common revision number, while joining them). However, specifying the same parameter name twice within a DQL query currently results in a fatal error:

Warning: array_combine(): Both parameters should have an equal number of elements in /var/www/invoiceCreator/src/thirdPartyComponents/doctrine/Doctrine/ORM/Query.php on line 252 Fatal error: Unsupported operand types in /var/www/invoiceCreator/src/thirdPartyComponents/doctrine/Doctrine/ORM/Query.php on line 252

It is possible to work around this by just giving each parameter a unique name. Unfortunately, it becomes an impossibility when using generated DQL statements, e.g. when a correlated subquery is replicated multiple times within the same DQL query.

I can not think of a convincing reason to allow each parameter to appear only once in a query, as the number of parameters could be determined as well by counting the number of unique names rather than simply all names.

Would it be possible to relax the restriction? It would remove a massive burden on dynamically building queries, as currently all parameter names have to be adapted to every single situation, and that would be no longer an issue, if multiple parameters with the same name would be allowed and correctly handled by Doctrine 2.

Apart from the functional requirement, I don't like the crashing effect. If such use is forbidden, it should be officially so (with the documentation at least mentioning the restriction and Doctrine 2 giving an understandable error message when used wrongly, instead of crashing).



 Comments   
Comment by Daniel Alvarez Arribas [ 21/Feb/11 ]

Currently, I automatically post-process all queries so that all parameters are assigned new (unique) names. This way, even the multiple replicated subqueries within the same DQL query will end up having unique parameter names. While this works, it is clumsy as a general approach.

Comment by Benjamin Eberlei [ 26/Feb/11 ]

Works for me, see patch attached.

The error occuring to you is not related to named parameters but to objects as parameters. Can you give a reproducable test-case to fix this?

Comment by Daniel Alvarez Arribas [ 26/Feb/11 ]

OK, I will see what I can do. I will prepare an example of a working scenario and a non-working scenario, and let you know.

Comment by Daniel Alvarez Arribas [ 26/Feb/11 ]

I tried to reduce the non-working query as much as possible to get a schematic idea of what provokes the error.

It seems that queries like this one will break:

{{$doctrineQuery = $doctrineEntityManager->createQuery('SELECT a
FROM \persistentData\model\import\A a,
WHERE a.a = :anEntityInstance
AND a.b = :anEntityInstance
AND a.c = :aString');

$doctrineQuery->setParameter('anEntityInstance', $anEntityInstance); // Whatever entity instance
$doctrineQuery->setParameter('aString', $aString); // Some string-valued variable

$doctrineQuery->getResult();}}

Note that the first argument is an entity instance, and the second one a string. Both are non-null.

This gives:

Warning: array_combine(): Both parameters should have an equal number of elements in /var/www/invoiceCreator/src/thirdPartyComponents/doctrine/Doctrine/ORM/Query.php on line 252 Fatal error: Unsupported operand types in /var/www/invoiceCreator/src/thirdPartyComponents/doctrine/Doctrine/ORM/Query.php on line 252

This is a basic form of a working scenario (all parameters have unique names):

{{$doctrineQuery = $doctrineEntityManager->createQuery('SELECT a
FROM \persistentData\model\import\A a,
WHERE a.a = :anEntityInstance1
AND a.b = :anEntityInstance2
AND a.c = :aString');

$doctrineQuery->setParameter('anEntityInstance1', $anEntityInstance); // Whatever entity instance
$doctrineQuery->setParameter('anEntityInstance2', $anEntityInstance); // Whatever entity instance
$doctrineQuery->setParameter('aString', $aString); // Some string-valued variable

$doctrineQuery->getResult();}}

Comment by Benjamin Eberlei [ 03/Apr/11 ]

Fixed.

Comment by Daniel Alvarez Arribas [ 05/Apr/11 ]

Great, thanks.





[DDC-1068] ClassMetadata should use Class name from reflection Created: 13/Mar/11  Updated: 05/Apr/11  Resolved: 20/Mar/11

Status: Resolved
Project: Doctrine 2 - ORM
Component/s: ORM
Affects Version/s: None
Fix Version/s: 2.0.4
Security Level: All

Type: Bug Priority: Major
Reporter: Benjamin Eberlei Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None


 Description   

Currently ClassMetadata uses the class name that is passed from loadMetadata(). This one could have case-sensitivity problems (not the same case as the class actually has), which can lead to problems later in the code.



 Comments   
Comment by Benjamin Eberlei [ 20/Mar/11 ]

Fixed





[DDC-1087] Repository::findBy and Repository::findOneBy NULL invalid query Created: 29/Mar/11  Updated: 05/Apr/11  Resolved: 03/Apr/11

Status: Resolved
Project: Doctrine 2 - ORM
Component/s: Mapping Drivers
Affects Version/s: Git Master
Fix Version/s: 2.0.4
Security Level: All

Type: Bug Priority: Major
Reporter: Patrik Votoček Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None
Environment:

MySQL



 Description   
$repository->findBy(array('foo' => NULL)); // or findOneBy

convert to invalid query:

SELECT f.* FROM foo f  WHERE f.foo = NULL;

in MySQL doesn't same as valid:

SELECT f.* FROM foo f  WHERE f.foo IS NULL;


 Comments   
Comment by Benjamin Eberlei [ 03/Apr/11 ]

Fixed





[DDC-1077] Specifying a constant value in the SELECT clause causes parser error Created: 24/Mar/11  Updated: 05/Apr/11  Resolved: 27/Mar/11

Status: Resolved
Project: Doctrine 2 - ORM
Component/s: ORM
Affects Version/s: 2.0.2
Fix Version/s: 2.0.4
Security Level: All

Type: Bug Priority: Minor
Reporter: Kevin McBride Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None

Issue Links:
Reference
is referenced by DDC-1095 Selecting Literals will not work, cra... Resolved

 Description   

User group post: http://groups.google.com/group/doctrine-user/browse_thread/thread/7f49cf9ff36d0750

Specifying a value in the select clause, such as the following, causes a parser error:

queryBuilder->select('p.name as name, \'foo\' as bar')>from>('Person p');

The equivalent SQL would be:

SELECT p.name as name, 'foo' as bar FROM person;

And would result in something like the following:

name bar
---------------
steve foo
john foo
sally foo

The error output:

Doctrine\ORM\Query\QueryException : [Syntax Error] line 0, col 136: Error:

Expected IdentificationVariable | StateFieldPathExpression | AggregateExpression | "(" Subselect ")" | ScalarExpression, got 'foo'

Doctrine/ORM/Query/Parser.php(393): syntaxError()



 Comments   
Comment by Benjamin Eberlei [ 27/Mar/11 ]

Fixed





[DDC-992] ManyToMany self referencing association bug with lazy loading Created: 19/Jan/11  Updated: 05/Apr/11  Resolved: 20/Mar/11

Status: Resolved
Project: Doctrine 2 - ORM
Component/s: ORM
Affects Version/s: 2.0
Fix Version/s: 2.0.4
Security Level: All

Type: Bug Priority: Major
Reporter: victor Velkov Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None
Environment:

Doctrine 2.0.0



 Description   

Hi there I encountered a bug. When was trying to get ManyToMany self referencing association via lazy loading.

Here is my Entity.

use Doctrine\Common\Collections\ArrayCollection;

/**
 *
 * @Table(name="Roles")
 * @entity(repositoryClass="Users_Model_RoleRepo")
 */

class Users_Model_Entity_Role extends Viscomp_Doctrine_Entity_Abstract
{
/**
	 *  @Id  @Column(name="roleID", type="integer")
	 *  @GeneratedValue(strategy="AUTO")
	 *
	 */
	protected $roleID;

	/**
	 * @Column (name="name", type="string", length="45")
	 *
	 *
	 */
	protected $name;

	/**
	 * @Column (name="shortName", type="string", length="45")
	 *
	 *
	 */
	protected $shortName;

 	/**
 	 * @ManyToMany (targetEntity="Users_Model_Entity_Role", mappedBy="extends")
  	 */
	protected $extendedBy;

	/**
	 * @ManyToMany (targetEntity="Users_Model_Entity_Role", inversedBy="extendedBy")
	 * @JoinTable (name="RoleRelations",
	 *      joinColumns={@JoinColumn(name="roleID", referencedColumnName="roleID")},
	 *      inverseJoinColumns={@JoinColumn(name="extendsRoleID", referencedColumnName="roleID")}
	 *      )
	 */
	protected $extends;

	public function __construct() {
		$this->extends = new ArrayCollection;
		$this->extendedBy = new ArrayCollection;
   }

}

When I call

$test = $this->getEntityManager()->getRepository('Users_Model_Entity_Role')->findOneBy(array('roleID' => 3));
Doctrine\Common\Util\Debug::dump($test); die;

The SQL that is generated is :

SELECT t0.roleID AS roleID1, t0.name AS name2, t0.shortName AS shortName3 FROM Roles t0 WHERE t0.roleID = ? array(1) { [0]=>  int(3) } 

SELECT t0.roleID AS roleID1, t0.name AS name2, t0.shortName AS shortName3 FROM Roles t0 INNER JOIN RoleRelations ON t0.roleID = RoleRelations.roleID WHERE RoleRelations.extendsRoleID = ?
array(1) {
  [0]=>
  int(3)
}
SELECT t0.roleID AS roleID1, t0.name AS name2, t0.shortName AS shortName3 FROM Roles t0 INNER JOIN RoleRelations ON t0.roleID = RoleRelations.extendsRoleID WHERE t0.roleID = ?
array(1) {
  [0]=>
  int(3)
}

In the last SQL query there is a mistake in the where clause it should be RoleRelations.roleID = ? and not t0.roleID = ?. And because of that it is not returning correct result.



 Comments   
Comment by victor Velkov [ 19/Jan/11 ]

The solution that I found is in the class BasicEntityPersister on line 1123 we have

 $conditionSql .= $this->_getSQLTableAlias($this->_class->name) . '.';

and i replace it with

if ($assoc !== null
    && $assoc['targetEntity'] == $assoc['sourceEntity']
    && $assoc['type'] == ClassMetadata::MANY_TO_MANY) {
        $owningAssoc = $assoc['isOwningSide'] ? $assoc : $this->_em->getClassMetadata($assoc['targetEntity'])
                                                                   ->associationMappings[$assoc['mappedBy']];
	$conditionSql .= $this->_class->getQuotedJoinTableName($owningAssoc, $this->_platform) . '.';

} else {
         $conditionSql .= $this->_getSQLTableAlias($this->_class->name) . '.';
}

I am not sure if that is the correct answer but as far as i have tested it doesn't mess up with the normal many to many association via join table.

Comment by Benjamin Eberlei [ 23/Jan/11 ]

There is a simple workaround if you rename the "roleID" from the join table to something else, childRoleID for example. This should work for now. I am looking into a way to fix this issue.

Comment by victor Velkov [ 23/Jan/11 ]

Ok thanks

Comment by Benjamin Eberlei [ 20/Mar/11 ]

Fixed, merged into 2.0.x branch.





[DDC-1052] Class table inheritance + OptimisticLocking issue Created: 28/Feb/11  Updated: 05/Apr/11  Resolved: 20/Mar/11

Status: Resolved
Project: Doctrine 2 - ORM
Component/s: ORM
Affects Version/s: None
Fix Version/s: 2.0.4
Security Level: All

Type: Bug Priority: Major
Reporter: Gustavo Falco Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None
Environment:

MySQL 5.1 + Ubuntu server 64 bits + PHP 5.3.2



 Description   

Hi everyone!

I'm having a problem with the optimistic locking feature. I have this (simplified) model tree:

  • BaseElement (abstract)
    • Element (abstract)
      • Entity (abstract)
        • Person (abstract)
          • Contact
        • BusinessPartner (abstract)
          • Customer
          • Lead

Now, with MySQL Logging active, when I update a Lead I have these queries executed:

 
Query	START TRANSACTION
Query	SELECT ...Lead fields... WHERE id = 123
Query       UPDATE business_partner SET ...BusinessPartner fields... WHERE id = 123
Query       UPDATE element SET version = version + 1 WHERE id = 123 AND version = 1
Query       commit

It works perfectly. The same goes when I update a Customer. The problem comes when I want to update a Contact:

 
Query	START TRANSACTION
Query	SELECT ...Contact fields... WHERE id = 123
Query       UPDATE element SET version = version + 1 WHERE id = 123 AND version = 1
Query       UPDATE person SET ...Person fields... WHERE id = 123
Query       commit

As you can see, the element row (and the version checking) occurs before the update of the person row. I think this is the reason I'm getting an OptimisticLockingException. Maybe after the Person row is updated, version is being checked again? I can't figure out why. My simplified model basically is like this:

BaseElement:

 
/**
 * @orm:MappedSuperclass
 */
abstract class BaseElement
{
      // ID, Version and other fields
}

Element:

 
/**
 * @orm:Entity(repositoryClass="ENC\Bundle\ElementModuleBundle\Entity\ElementRepository")
 * @orm:Table(name="element")
 * @orm:InheritanceType("JOINED")
 * @orm:DiscriminatorColumn(name="discriminator", type="string")
 * @orm:DiscriminatorMap({
      "contact" 			= "CNE\Bundle\ContactModuleBundle\Entity\Contact",
      "lead" 				= "CNE\Bundle\LeadModuleBundle\Entity\Lead",
      "customer" 			= "CNE\Bundle\CustomerModuleBundle\Entity\Customer",
   })
abstract class Element extends BaseElement
{
      // Other fields
}

Entity:

 
/**
 * @orm:Entity(repositoryClass="ENC\Bundle\EntityModuleBundle\Entity\EntityRepository")
 * @orm:Table(name="entity")
 */
abstract class Entity extends Element
{
     // Fields
}

BusinessPartner:

 
/**
 * @orm:Entity(repositoryClass="ENC\Bundle\BusinessPartnerModuleBundle\Entity\BusinessPartnerRepository")
 * @orm:Table(name="business_partner")
 */
abstract class BusinessPartner extends Entity
{
    // Fields
}

Person

/**
 * @orm:Entity(repositoryClass="ENC\Bundle\PersonModuleBundle\Entity\PersonRepository")
 * @orm:Table(name="person")
 */
abstract class Person extends Entity
{
     // Fields
}

Contact:

/**
 * @orm:Entity(repositoryClass="ENC\Bundle\ContactModuleBundle\Entity\ContactRepository")
 * @orm:Table(name="contact")
 * @orm:HasLifecycleCallbacks
 */
class Contact extends Person
{
    // Fields
}

Lead:

/**
 * @orm:Entity(repositoryClass="ENC\Bundle\LeadModuleBundle\Entity\LeadRepository")
 * @orm:Table(name="lead")
 * @orm:HasLifecycleCallbacks
 */
class Lead extends BusinessPartner
{
    // Fields
}

Customer:

/**
 * @orm:Entity(repositoryClass="ENC\Bundle\CustomerModuleBundle\Entity\CustomerRepository")
 * @orm:Table(name="customer")
 * @orm:HasLifecycleCallbacks
 */
class Customer extends BusinessPartner
{
    // Fields
}

I don't post the annotations of all my models because they're too many and I think they don't have nothing to do with the problem. But in case you need them, please, tell me and I post them

Thanks in advance!



 Comments   
Comment by Benjamin Eberlei [ 01/Mar/11 ]

Can you post the stack trace of the optimistic version exception?

In general This inheritance hierachy scares the hell out of me, why do you need 5 levels of inheritance? Cant you just drop BaseElement, element and Entity into one? Plus this model really fails What happens if a person moves from a lead to customer or contact or back?

Comment by Benjamin Eberlei [ 01/Mar/11 ]

Can you try to change lib/Doctrine/ORM/Persisters/JoinedEntityPersister.php line 368

if ($this->_class->isVersioned && ! $result) {

to

if ($versioned && ! $result) {
Comment by Gustavo Falco [ 02/Mar/11 ]

Thanks for your answer Benjamin! your change indeed solved the problem But the file wasn't JoinedEntityPersister (it doesn't exist at least in the latest doctrine copy from "git://github.com/doctrine/doctrine2.git doctrine"). It was on the file BasicEntityPersister.php, located in the same dir you mention.

About my model, I didn't show the entire model because it didn't make sense for this issue. But one reason I have for this hierarchy is that I have lots of models that need to be related with all the instances of my model "Entity". For instance, all Users, Contacts, Leads, Customers and Suppliers need to have Activities, and I need to provide a way to select one between all these models when you, for example, create an Activity. Then I have a model "Opportunity" that needs to be associated only with BusinessPartner's. And, lastly, I have a couple of models that need to be associated with all my "Element"s. BaseElement is a mapped superclass that has only the basic fields for ALL my models (related to Element or not), that have in common all the entities in my model (don't confuse with the model "Entity". Yes, it's a little bit confuse but I couldn't think in another name to relate People and Business Partners ).

The other point: People can't be a customer or a lead. My requirements say that only business partners can be customers, leads or suppliers. Business partners are companies. They can't be individuals. Business partners can have contacts. And if a user needs to "transform" a lead to a customer, it can create a customer from a lead, and have a reference back to it. I thought I need them to have them separated because this won't be only a CRM. It's going to be a sales system too (I don't know if this is the correct english term for this type of system), so I need to have them in two different entities for a couple of reasons. If I only have a status that describes a customer or a lead, I can't, for example, enforce some business rules at database level I'll have (in a DB portable way at least that I know of). I need to enforce, if it's possible and portable, this type of business rules because there will be other apps that will interact with my DB. For instance, if I need to associate an invoice to a lead_or_customer, how can I enforce this on the DB if I only have a status field that determines if it's a lead or a contact? Separating the entities is the only way I could think of to solve this type of problems. Maybe is not the best way. I'll need to rethink the trade-offs of this decision. Thanks for pointing this out

For now it's a CRM. But it's only the beginning. It will have lots of additional models when I'll add modules for a sales system, voIP vinculation, etc. That's why I tried to make a reusable tree on my model. I know the performance risks having too many entities related to each other, having lots of joins and one table in common that will be always touched could be a bottleneck if there are lots of users doing lots of operations. But I'm in the hurry and I need to save development time. I'm the only person that will work on this app, and I have only 6 months from scratch to finish the CRM, which is a really big app in very little time for me (having to work on a pretty big ACL module and a Workflow module too). Inheritance gave me a DRY way in lots of aspects of my app. I can sacrifice some performance to save development time. Then I can optimize if I need it

THANKS A LOT for your fix and for quickly look this issue! and for your comments too. I'm not an expert yet and constructive comments like these help me to rethink some points on my decisions.

Best regards.

Comment by Benjamin Eberlei [ 20/Mar/11 ]

Fixed, merged into 2.0.x branch

Comment by Gustavo Falco [ 20/Mar/11 ]

Great! thanks a lot.





[DDC-1053] GROUP BY crashes Doctrine 2 Created: 01/Mar/11  Updated: 05/Apr/11  Resolved: 20/Mar/11

Status: Resolved
Project: Doctrine 2 - ORM
Component/s: DQL
Affects Version/s: 2.0.1
Fix Version/s: 2.0.4
Security Level: All

Type: Bug Priority: Major
Reporter: Daniel Alvarez Arribas Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None

Issue Links:
Reference
relates to DDC-1073 Refactor SqlWalker::walkGroupByItem n... Resolved

 Description   

This query will crash Doctrine 2:

SELECT wpomOrder.documentDate
                           FROM \persistentData\model\import\webProductionOrderManager\WPOMOrder wpomOrder
                         GROUP BY wpomOrder

Here is the hard error I get:

Catchable fatal error: Argument 1 passed to Doctrine\ORM\Query\SqlWalker::walkGroupByItem() must be an instance of Doctrine\ORM\Query\AST\PathExpression, string given in /var/www/invoiceCreator/src/thirdPartyComponents/doctrine/Doctrine/ORM/Query/SqlWalker.php on line 1203

Of course, the query shown is pointless. I reduced it from a more complex statement to illustrate that it is actually the GROUP BY that breaks Doctrine. Even in a meaningful scenario with aggregates, it would not work.



 Comments   
Comment by Daniel Alvarez Arribas [ 01/Mar/11 ]

The query shown above could as well be invalid, because it groups the results by something which is not in the select clause. I do not know if this should be a requirement, though, since I can think of use cases where you would want to group by something, but still not select it. Anyway, this query will work:

SELECT wpomOrder.documentDate
                           FROM \persistentData\model\import\webProductionOrderManager\WPOMOrder wpomOrder
                         GROUP BY wpomOrder.documentDate

Note that it groups the results by the expression used in the select clause rather than just the entity like in the example above.

Comment by Benjamin Eberlei [ 20/Mar/11 ]

Fixed, group by identification variable is now supported. Merged into 2.0.x

Added follow up task for 3.0 cleanup DDC-1073





[DDC-1054] Lazy loading problem Created: 02/Mar/11  Updated: 05/Apr/11  Resolved: 25/Mar/11

Status: Resolved
Project: Doctrine 2 - ORM
Component/s: ORM
Affects Version/s: 2.0.1
Fix Version/s: 2.0.4
Security Level: All

Type: Bug Priority: Major
Reporter: victor Velkov Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None

Attachments: File Doctrine LazyLoad.php    

 Description   

I have a problem when using lazy loading in entity that was initialized before that through eager loading. I am attaching a file with my entities and the 2 queries that i make whit which i have the problem.



 Comments   
Comment by victor Velkov [ 02/Mar/11 ]

There is a fix that i made to fix the problem, but i am not sure how it will reflect on the performance. But as far as i can see and understand the logic i don't see where else it could be.

in the class ObjectHydrator in privet function _initRelatedCollection
on line 164 you make a check if collection is set to be refreshed or it is fetched but not initialized.


else if (isset($this->_hints[Query::HINT_REFRESH]) ||
                isset($this->_hints['fetched'][$class->name][$fieldName]) &&
                ! $value->isInitialized()) {
            // Is already PersistentCollection, but either REFRESH or FETCH-JOIN and UNINITIALIZED!
            $value->setDirty(false);
            $value->setInitialized(true);
            $value->unwrap()->clear();
            $this->_initializedCollections[$oid . $fieldName] = $value;
        }

The problem that i see here is in the second part when the collection is not initialized and i think that is the reason for the problem so my solution si


else if (isset($this->_hints[Query::HINT_REFRESH]) ||
                isset($this->_hints['fetched'][$class->name][$fieldName]) &&
                ! $value->isInitialized()) {
            // Is already PersistentCollection, but either REFRESH or FETCH-JOIN and UNINITIALIZED!
            $value->setDirty(false);
	   /**
            *  $value->setInitialized(true);
            *  $value->unwrap()->clear();
            * Fix by Victor on 02/03/2011
            * With only that code the collection doesn't get initialized
            */
            $value->initialize();
            $this->_initializedCollections[$oid . $fieldName] = $value;
        }
Comment by Benjamin Eberlei [ 04/Mar/11 ]

"I am having a problem".

Can you describe a little bit more what your problem actually is?

Comment by victor Velkov [ 05/Mar/11 ]

well that is why i have attached the php file. Inside you can see my entities and the 2 queries whit which i have the problem. So in general when i call entity and use eager join to get related entity. And then call that related entity on it's own the lazy loading is not working.

Comment by Benjamin Eberlei [ 20/Mar/11 ]

In your use-case do you execute Q1 and then Q2 after each other in the same request? I just don't get why in your case a query with a Refresh Hint is executed.

The code is not very helpful as i cannot execute it out of the box. I would prefer a PHPUnit test-case with one of the Doctrine2 Test Models for such a complex issue.

Comment by victor Velkov [ 25/Mar/11 ]

After i updated to the latest version the bug was fixed. So sorry for the bother. Looks like it was connected with the problem of ticket DDC-992.
Thanks for the great work





[DDC-1070] AbstractQuery::iterate() do not use $hydrationMode argument Created: 15/Mar/11  Updated: 05/Apr/11  Resolved: 20/Mar/11

Status: Resolved
Project: Doctrine 2 - ORM
Component/s: ORM
Affects Version/s: None
Fix Version/s: 2.0.4
Security Level: All

Type: Bug Priority: Major
Reporter: Eugene Leonovich Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None


 Description   

In addition, a call $this->_doExecute($params, $hydrationMode) inside the iterate()
is not compatible with the declaration:

abstract protected function _doExecute();


 Comments   
Comment by Benjamin Eberlei [ 20/Mar/11 ]

Fixed, merged into 2.0.x branch.





[DDC-1093] EntityManager "ExpressionBuilder" instead of "Query\Expr" Created: 31/Mar/11  Updated: 05/Apr/11  Resolved: 03/Apr/11

Status: Resolved
Project: Doctrine 2 - ORM
Component/s: Documentation
Affects Version/s: 2.0.2
Fix Version/s: 2.0.4
Security Level: All

Type: Documentation Priority: Minor
Reporter: Matt Wells Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None
Environment:

WampServer 2.1e on Windows XP Home SP3



 Description   

The EntityManager's method getExpressionBuilder() DocBlock states that it returns a class of "ExpressionBuilder" when it actually returns an instance of "Query\Expr". The expressionBuilder property on the EntityManager also has the "ExpressionBuilder" type.

The ExpressionBuilder class does not exist and is making my PHP IDE (Netbeans 7 beta 2) code suggestion/completion functionality from working.



 Comments   
Comment by Benjamin Eberlei [ 03/Apr/11 ]

Fixed, you can use QueryBuilder->expr() aswell btw, which type hints this correctly.





Generated at Sun Apr 20 11:30:33 UTC 2014 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.