[DDC-2452] Additional `WITH` condition in joins between JTI roots cause invalid SQL to be produced Created: 16/May/13 Updated: 16/May/13 |
|
| Status: | Open |
| Project: | Doctrine 2 - ORM |
| Component/s: | DQL, ORM |
| Affects Version/s: | Git Master |
| Fix Version/s: | 2.4 |
| Security Level: | All |
| Type: | Bug | Priority: | Major |
| Reporter: | Marco Pivetta | Assignee: | Marco Pivetta |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | dql, sql-walker | ||
| Environment: |
irrelevant |
||
| Description |
|
Given a simple Joined Table Inheritance like following: /** * @Entity @Table(name="foo") @InheritanceType("JOINED") * @DiscriminatorColumn(name="discr", type="string") * @DiscriminatorMap({"foo" = "DDC2452Foo", "bar" = "DDC2452Bar"}) */ class DDC2452Foo { /** @Id @Column(type="integer") @GeneratedValue */ public $id; } /** @Entity @Table(name="bar") */ class DDC2452Bar extends DDC2452Foo { } Following DQL SELECT foo1 FROM DDC2452Foo foo1 JOIN DDC2452Foo foo2 WITH 1=1 Will produce broken SQL: SELECT
f0_.id AS id0, f0_.discr AS discr1
FROM
foo f0_
LEFT JOIN bar b1_
ON f0_.id = b1_.id
LEFT JOIN foo f2_
LEFT JOIN bar b3_
ON f2_.id = b3_.id
ON (1 = 1)
(please note the duplicate `ON` in the SQL) That is caused because of the SQL walker producing the JTI filter with already the `ON` clause in it. That happens because the JTI join conditions are added in https://github.com/doctrine/doctrine2/blob/2.4.0-BETA2/lib/Doctrine/ORM/Query/SqlWalker.php#L823-L825 (`walkRangeVariableDeclaration`), while the additional defined `WITH` conditions are considered in `walkJoinAssociationDeclaration` later on. Added a test case and fix at https://github.com/doctrine/doctrine2/pull/668 |
[DDC-2052] Custom tree walkers are not allowed to add new components to the query Created: 02/Oct/12 Updated: 14/May/13 |
|
| Status: | Reopened |
| Project: | Doctrine 2 - ORM |
| Component/s: | DQL |
| Affects Version/s: | 2.3 |
| Fix Version/s: | 2.4 |
| Type: | Improvement | Priority: | Major |
| Reporter: | Łukasz Cybula | Assignee: | Benjamin Eberlei |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | dql | ||
| Description |
|
Custom tree walkers have freedom in modifying the AST but when you try to add a new query component (i.e. new join in walkSelectStatement() ) to the AST then the SqlWalker throws an exception because it does not has the new component in its _queryComponents array. I see two possible ways to resolve this: |
| Comments |
| Comment by Benjamin Eberlei [ 06/Oct/12 ] |
|
Ok this is much more complicated to allow then i thought. The problem is that the QueryComponents are passed by value, as an array, not by reference. That prevents changing them because this change wouldn't be visible in the output walker. I can add a method to allow this in the OutputWalker for now, but generally this requires a bigger refactoring on the Query Components. |
| Comment by Benjamin Eberlei [ 06/Oct/12 ] |
|
Added setQueryComponent() in SQL Walker to allow modification in output walker. |
| Comment by Łukasz Cybula [ 08/Oct/12 ] |
|
I'm afraid that this doesn't solve the initial problem at all. I'll try to describe it in more details to show what I mean. Suppose we have two doctrine extensions each of which contain its own tree walker. Each of these tree walkers need to modify AST and add new component to it (joined with some component already existing in the query). The first problem is that each tree walker has its own queryComponents array which is not passed between them, although they not necessary need to use queryComponents - they could use only AST. The second, bigger problem is that the Parser class does not know anything about modifications of queryComponents in tree walkers and cannot pass modified version to the OutputWalker. The goal of submitting this issue was to allow adding new components to the query in tree walkers which is not achievable by your fix. I think it may be the first step in the right direction. Maybe TreeWalkerAdapter should have public method getQueryComponents() which would be used by the Parser to pass modified queryComponents between different tree walkers and finally to the OutputWalker ? This would not break backward compatibility and solve this issue. What do you think about it? |
| Comment by Łukasz Cybula [ 08/Oct/12 ] |
|
I've tried to implement the solution mentioned in previous comment but it's also not so clean and easy as I thought. Each tree walker (including TreeWalkerChain) would have to implement getQueryComponents() and setQueryComponent($alias, array $component) methods. The same with SqlWalker, so the TreeWalker interface should have these methods, which would break BC in some way (walkers that do not inherit from SqlWalker or TreeWalkerAdapter will fail to compile). So maybe my first solution (PR #464) is not so bad for now? In the future queryComponents could be replaced by a special object or could be passed by a reference to allow modifications. |
| Comment by Benjamin Eberlei [ 09/May/13 ] |
|
Marked as improvement as its not a bug. A solution might probably implement an object holding all the QueryComponent, implementing ArrayAccess. So that way the state can be shared. |
| Comment by Marco Pivetta [ 14/May/13 ] |
|
Just hit this while developing an ast walker... Will look into it too since I need it more than soon. |
| Comment by Marco Pivetta [ 14/May/13 ] |
|
As a VERY UGLY workaround, I used a static variable and a custom sql walker in combination with my AST walker. namespace Comcom\Versioning\ORM\Query; use Doctrine\ORM\Query\SqlWalker; class WorkaroundSqlWalker extends SqlWalker { public function __construct($query, $parserResult, array $queryComponents) { parent::__construct($query, $parserResult, $queryComponents); foreach (VersionWalker::$additionalAliases as $alias => $value) { $this->setQueryComponent($alias, $value); } } } |
[DDC-2390] Remove Parser and SQLWalker dependency on Query Created: 04/Apr/13 Updated: 04/Apr/13 |
|
| Status: | Open |
| Project: | Doctrine 2 - ORM |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 2.4 |
| Security Level: | All |
| Type: | Improvement | Priority: | Major |
| Reporter: | Benjamin Eberlei | Assignee: | Benjamin Eberlei |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | None | ||
| Description |
|
Query is too powerful to be available in Parser and SQLWalker, because it may lead to accessing data that changes on subsequent runs of a query that is cached. Idea is to introduce a MetadataBag that contains only the values that are allowed to be accessed. |
[DDC-2203] add EntityManager->getFilters()->isEnabled('filterName'') Created: 17/Dec/12 Updated: 01/Apr/13 |
|
| Status: | Open |
| Project: | Doctrine 2 - ORM |
| Component/s: | ORM |
| Affects Version/s: | Git Master |
| Fix Version/s: | 2.4 |
| Security Level: | All |
| Type: | Improvement | Priority: | Minor |
| Reporter: | Enea Bette | Assignee: | Benjamin Eberlei |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | None | ||
| Comments |
| Comment by Paweł Nowak [ 10/Jan/13 ] |
|
My pull request (https://github.com/doctrine/doctrine2/pull/548) contains an implementation of the method. Note that no exception is thrown if you query for the state of a non-existing filter - in such a case, false is returned as for disabled filters. |
[DDC-1621] Add support for FROM Class1 a1 JOIN Class2 a2 WITH cond queries Created: 25/Jan/12 Updated: 30/Dec/12 |
|
| Status: | In Progress |
| Project: | Doctrine 2 - ORM |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 2.4 |
| Security Level: | All |
| Type: | New Feature | Priority: | Major |
| Reporter: | Benjamin Eberlei | Assignee: | Alexander |
| Resolution: | Unresolved | Votes: | 1 |
| Labels: | None | ||
| Description |
|
Check feasibility of this kind of query different from FROM Class1 a1, Class2 a2 to allow arbitrary joins between classes. |
| Comments |
| Comment by Alex [ 30/Nov/12 ] |
|
Hi all! |
[DDC-2089] Modify OneToMany to allow unidirectional associations without the need of a JoinTable Created: 19/Oct/12 Updated: 16/Dec/12 |
|
| Status: | Open |
| Project: | Doctrine 2 - ORM |
| Component/s: | ORM |
| Affects Version/s: | 2.x |
| Fix Version/s: | 2.4, 3.0 |
| Security Level: | All |
| Type: | Improvement | Priority: | Major |
| Reporter: | Enea Bette | Assignee: | Benjamin Eberlei |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | onetomany, persister, unidirectional | ||
| Environment: |
Debian Wheezy, Mysql 5.1, Apache2, PHP 5.4 |
||
| Description |
|
As I sayd in the title, it would be nice if the ORM layer could permit to map a 1:n association in the db as an unidirectional OneToMany in the classes, without using a JoinTable in the database. Is it possible? |
| Comments |
| Comment by Enea Bette [ 16/Dec/12 ] |
|
A little up... for inspiration from JPA |
[DDC-1590] Fix Inheritance in Code-Generation Created: 09/Jan/12 Updated: 10/Dec/12 |
|
| Status: | Open |
| Project: | Doctrine 2 - ORM |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 2.4 |
| Security Level: | All |
| Type: | Improvement | Priority: | Major |
| Reporter: | Benjamin Eberlei | Assignee: | Benjamin Eberlei |
| Resolution: | Unresolved | Votes: | 3 |
| Labels: | None | ||
| Issue Links: |
|
||||||||
| Comments |
| Comment by Lukas Domnick [ 10/Dec/12 ] |
|
(I have no Link Privileges, but this one # |
[DDC-1283] Possible issue with PersistentCollection#getDelete/InsertDiff() Created: 21/Jul/11 Updated: 20/Sep/12 |
|
| Status: | Reopened |
| Project: | Doctrine 2 - ORM |
| Component/s: | None |
| Affects Version/s: | 2.1 |
| Fix Version/s: | 2.4 |
| Security Level: | All |
| Type: | Improvement | Priority: | Minor |
| Reporter: | Glen Ainscow | Assignee: | Guilherme Blanco |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | None | ||
| Description |
|
Using the following code, when you go from (1, 2) to (1), (2) is deleted as expected. However, if you go from (1, 2) to (2), (1) and (2) are deleted and (2) is then inserted. Is this the desired behaviour? (i.e. 2 extra queries) $bracket->getTournamentLocations()->takeSnapshot();
$col = $bracket->getTournamentLocations()->unwrap();
$col->clear();
foreach ($form->getValue('tournamentLocations') as $id) {
$col->add($em->getReference('Tournaments_Model_TournamentLocation', $id));
}
$bracket->getTournamentLocations()->setDirty(true);
|
| Comments |
| Comment by Benjamin Eberlei [ 26/Jul/11 ] |
|
First, you are using internal API therefore you are on your own anyways. This is marked as improvment now, the functionality works, it may just be inefficient. |
| Comment by Guilherme Blanco [ 09/Dec/11 ] |
|
Hi, I'm marking issue as invalid because you're conceptually wrong. Correct code would be you to regenerate the collection (a new ArrayCollection) and just assign it to setTournamentLocations($newCollection); Does this explanation is enough for you? Cheers, |
| Comment by Glen Ainscow [ 23/Dec/11 ] |
|
Hi Guilherme, If I do this: $locations = new ArrayCollection();
foreach ($form->getValue('tournamentLocations') as $id) {
$locations->add($em->getReference('Tournaments_Model_TournamentLocation', $id));
}
$bracket->setTournamentLocations($locations);
... then all the records are deleted, before adding the new records. This is inefficient and causes extra, unnecessary write operations. Can't Doctrine perform diffs when persisting the collection, so that only the necessary deletes and inserts are executed? |
| Comment by Guilherme Blanco [ 13/Jan/12 ] |
|
We could add it, but I don't think it worth the effort. I'd rather consider that it's not possible to be done at the moment, but I need much more investigation for that. This will be something that I'll probably only do when I look at this issue again with a lot of time (which is really hard to happen). If you have some spare time, feel free to make some attempts. |
[DDC-1599] OnFlush event in transaction Created: 14/Jan/12 Updated: 20/Sep/12 |
|
| Status: | Open |
| Project: | Doctrine 2 - ORM |
| Component/s: | ORM |
| Affects Version/s: | Git Master |
| Fix Version/s: | 2.4 |
| Type: | Improvement | Priority: | Major |
| Reporter: | Gediminas Morkevicius | Assignee: | Benjamin Eberlei |
| Resolution: | Unresolved | Votes: | 2 |
| Labels: | None | ||
| Description |
|
Is there any particular reason why onFlush event is not triggered when the transaction is allready open? https://github.com/doctrine/doctrine2/blob/master/lib/Doctrine/ORM/UnitOfWork.php#L290 It would help a lot developing listeners since this event is the mostly used one and since theres preFlush now it seems a logical solution if onFlush would be a start of transaction in general |
| Comments |
| Comment by Benjamin Eberlei [ 14/Jan/12 ] |
|
onFluish is not the start of a transaction. It has nothing to do with this. |
| Comment by Marco Pivetta [ 31/Mar/12 ] |
|
Is a third event needed? Or is this to be marked as "won't fix"? |
| Comment by Benjamin Eberlei [ 31/Mar/12 ] |
|
Maybe onBeginTransaction, onCommit and onRollback. However since you can start transactions manually using $em->beginTransaction(), the Flush events are somehwat independent of transactions anyways. |
| Comment by Gediminas Morkevicius [ 31/Mar/12 ] |
|
Well, user can start transaction anytime, but the fact is that if we think ORM we do not know nothing about the database. we just persist and flush objects. Yes I think these would be very useful, from how I see it, if you use event listeners, is: loadClassMetadata: you can apply extra mapping onFlush: you can modify entity changesets, or persist recalculate new ones, without triggering the database, since it is not used to begin the database modifications yet. onBeginTransaction: could use the database modifications keeping in sync the entity changesets. the thing about this event is that usually in behavioral way atomic updates are required. for example nestedset tree sync lft rgt columns, sortable sync the sort index, materialized path, all these requires atomic updates, and the best place is the start of transaction. onCommit: could be useful to execute right before commit, finalizing database modifications could be done. onRollback: this one is really something, since if you go far, there might be something like files uploaded during the entity processing, and you may want to remove them if transaction fails. |
| Comment by Guilherme Blanco [ 21/May/12 ] |
|
This situation was barely documented here: http://www.doctrine-project.org/jira/browse/DDC-1443 We need a better Transaction API that completely fixes the computation of changesets and also allow more fine grained control over Entities and their corresponding information. I'd postpone this one until 3.0. |
[DDC-1624] Locking CTI doesnt work on SQL Server Created: 29/Jan/12 Updated: 20/Sep/12 |
|
| Status: | Open |
| Project: | Doctrine 2 - ORM |
| Component/s: | ORM |
| Affects Version/s: | 2.2 |
| Fix Version/s: | 2.4 |
| Security Level: | All |
| Type: | Bug | Priority: | Major |
| Reporter: | Benjamin Eberlei | Assignee: | Benjamin Eberlei |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | None | ||
| Description |
|
The WITH Keyowrd is appended to the whole FROM .. JOIN .. block instead of behind the FROM block. |
[DDC-1852] Doctrine\ORM\Tools\SchemaValidator should check validity of lifecycle callbacks Created: 04/Jun/12 Updated: 20/Sep/12 |
|
| Status: | Open |
| Project: | Doctrine 2 - ORM |
| Component/s: | ORM |
| Affects Version/s: | Git Master |
| Fix Version/s: | 2.4, 2.x, Git Master |
| Security Level: | All |
| Type: | Improvement | Priority: | Major |
| Reporter: | Marco Pivetta | Assignee: | Benjamin Eberlei |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | None | ||
| Description |
|
The schema validator should analyze mapped lifecycle callbacks and: a) if some lifecycle callbacks were defined, but no @HasLifecycleCallbacks annotation/mapping was set, warn the user |
| Comments |
| Comment by Marco Pivetta [ 04/Jun/12 ] |
|
Existing PR at https://github.com/doctrine/doctrine2/pull/361 |
[DDC-1840] Create ParameterCollection indexed and implement it on AbstractQuery and QueryBuilder Created: 26/May/12 Updated: 20/Sep/12 |
|
| Status: | Open |
| Project: | Doctrine 2 - ORM |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 2.4 |
| Security Level: | All |
| Type: | Improvement | Priority: | Major |
| Reporter: | Guilherme Blanco | Assignee: | Benjamin Eberlei |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | None | ||
| Description |
|
Currently, method setParameters in AbstractQuery and QueryBuilder only appends new parameters to the list. It should actually override the existing ones. |
| Comments |
| Comment by Benjamin Eberlei [ 27/May/12 ] |
|
Not a bug |
[DDC-1738] Allow multiple Generators per class Created: 29/Mar/12 Updated: 20/Sep/12 |
|
| Status: | Open |
| Project: | Doctrine 2 - ORM |
| Component/s: | Mapping Drivers |
| Affects Version/s: | Git Master |
| Fix Version/s: | 2.4 |
| Security Level: | All |
| Type: | Improvement | Priority: | Major |
| Reporter: | Guilherme Blanco | Assignee: | Benjamin Eberlei |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | None | ||
| Description |
|
We should be able to support multiple generators per class. Currently we only support 1 generator per class. |
[DDC-1723] Custom ID Generators Created: 22/Mar/12 Updated: 20/Sep/12 |
|
| Status: | Open |
| Project: | Doctrine 2 - ORM |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 2.4 |
| Security Level: | All |
| Type: | New Feature | Priority: | Major |
| Reporter: | Benjamin Eberlei | Assignee: | Benjamin Eberlei |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | None | ||
| Description |
|
Allow specify custom id generators, pull request is GH-206 https://github.com/doctrine/doctrine2/pull/206 |