Skip to content

Added failing test case for querying and joining on an associated inheri...#302

Closed
jonathaningram wants to merge 1 commit intodoctrine:masterfrom
jonathaningram:inheritance_assoc_query
Closed

Added failing test case for querying and joining on an associated inheri...#302
jonathaningram wants to merge 1 commit intodoctrine:masterfrom
jonathaningram:inheritance_assoc_query

Conversation

@jonathaningram
Copy link
Contributor

...tance hierarchy

I have written this (partial) failing unit test. When running the test, you'll receive the Semantical Exception as follows:

[Semantical Error] line 0, col 81 near 'idd WHERE idd': Error: Class Doctrine\Tests\Models\Document\ImageDocument has no association named data

I expect that either:

  1. This is expected behaviour. In which case, I'll update the test to expect the semantical exception.
  2. This is a bug (I do hope it is!). In which case, any thoughts on a fix?

Either way, such a query would be great to have in the documentation (including the semantical error), so that when searching for the error, the Doctrine docs appear.

FYI, this has come up from my recent work on trying to find a solution for large inheritance hierarchies (and join limits) and solving the problem by using single table inheritance and associating a "data" class on the base class of the large hierarchy.

P.S. I hope that I do not have a flawed understanding of the ORM and that I'm trying to construct an impossible query/mapping. If so, apologies :)

@asm89
Copy link
Member

asm89 commented Mar 13, 2012

Your mapping is not valid. If you want to have objects with inheritance mapped with the ORM, look at:
http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/reference/inheritance-mapping.html

@stof
Copy link
Member

stof commented Mar 13, 2012

your issue is that you have an entity (ImageDocument) extending another entity (Document) but without any mapping for the inheritance. So the mapping is not inherited

@stof
Copy link
Member

stof commented Mar 13, 2012

btw, the cli command validating your mapping will probably complain in such case

@asm89 asm89 closed this Mar 13, 2012
@jonathaningram
Copy link
Contributor Author

Yes my solution to the large hierarchies problem uses STI but I did not use that in these fixtures because it does not matter (I.e. both types failed).

Apart from explaining the different inheritance types, what's the part of that doc that explains the issue? Thanks.

@jonathaningram
Copy link
Contributor Author

Do you mean without any inheritance mapping for its "data" association? Is there no way for doctrine to know this? Because this is working when fetching/saving/updating deleting objects normally, it just fails when trying to write a custom query. Thanks.

@stof
Copy link
Member

stof commented Mar 13, 2012

@jonathaningram does your database contain a data field for ImageDocument when you don't map the inheritance ?

@jonathaningram
Copy link
Contributor Author

@asm89 can we please leave this open for the meantime?

@stof I actually forgot to add the inheritance mapping to the base class (it was added in my app, but forgot to add to these test fixtures). After that last commit, the error is still the same.

I updated my test fixtures to use STI on the Document hierarchy (CTI is meant to be used on the DocumentData classes). @stof I won't answer your last question because I had the missing mapping on the Document base class.

I also ran the schema validation tool (via Symfony2 console) on my app, and it validated with no errors:

$ ./app/console doctrine:schema:validate
[Mapping]  OK - The mapping files are correct.
[Database] OK - The database schema is in sync with the mapping files.

So:

  1. The validator tool does not complain about my mapping.
  2. All is working fine normally - only starting to use queries causes issues.
  3. If in the end I am still wrong, can this not be made into a test case anyway to help users find it and also since it is a valid test case? And, can the documentation be updated?

@jonathaningram
Copy link
Contributor Author

@stof, @asm89 that commit is here: jonathaningram@c6e08d6 but it doesn't show up in this PR (because it was closed?)

@jonathaningram
Copy link
Contributor Author

Ah! One more typo in the mapping is fixed: jonathaningram@d35cb3c

So the original error was incorrect. Now, the error is:

[Semantical Error] line 0, col 191 near 'url DESC': Error: Class Doctrine\Tests\Models\Document\DocumentData has no field or association named url

Which is wrong because it's only looking on the base DocumentData class for the field, and it should look in all subclasses (e.g. ImageDocumentData), no?

@stof
Copy link
Member

stof commented Mar 14, 2012

no, it is right. The target of the relation is DocumentData

@jonathaningram
Copy link
Contributor Author

Hmmm, have a look at: jonathaningram/doctrine2@doctrine:master...inheritance_assoc_query - I added a fix that looks in subclasses for the field.

It generates the SQL (not yet added to my test):

SELECT d0_.id AS id0, d0_.version AS version1, d1_.id AS id2, d0_.discr AS discr3, d1_.discr AS discr4 
FROM document d0_ INNER JOIN document_data d1_ ON d0_.data_id = d1_.id 
WHERE (d1_.discr IN ('image_document_data') AND d0_.version = 55) AND d0_.discr IN ('image_document') 
ORDER BY d1_.url DESC

But should it be allowed that the "targetEntity=DocumentData" be "any entity that is type DocumentData or any entity subclass of DocumentData"?

@stof
Copy link
Member

stof commented Mar 14, 2012

@jonathaningram the point is that "any subclass of DocumentData" does not have an url property

@jonathaningram
Copy link
Contributor Author

@stof fair enough, but these queries are runtime, so why can it not be checked at runtime for validity?

And did you mean "every subclass of DocumentData"?

@beberlei
Copy link
Member

DQL unlike PHP enforces strict OOP types.

@beberlei
Copy link
Member

Oh and we don't do this just to play with our users, but since relational databases are so strict about data we have to be strict about it in our query language. Otherwise inconsistencies would occur all the time.

@jonathaningram
Copy link
Contributor Author

Thanks. I thought the solution to large inheritance hierarchies would be to associate the data, but I guess that won't work either.

A colleague of mine suggested some sort of "cast" on the join as follows:

SELECT id, idd 
FROM Doctrine\Tests\Models\Document\ImageDocument id 
    JOIN (ImageDocumentData) id.data idd 
WHERE idd INSTANCE OF Doctrine\Tests\Models\Document\ImageDocumentData AND 
    id.version = 55 
ORDER BY idd.url DESC

Would that be a possible improvement to Doctrine? Doctrine would throw an exception if you try and cast to an invalid subclass.

@dvlpp
Copy link

dvlpp commented Nov 26, 2012

I'm facing the same problem as @jonathaningram, and this isn't the first time... To me, the main issue is that we can't "order by" a field owned by a particular subclass of the hierarchie, while we know (when we write the request) that this is this subclass which will be adressed by the join.
This is a typical OOP problem, and as @jonathaningram said in his last comment, the common resolution is cast (which can, obviously, throw a Runtime Exception is case of bad cast). I think it would be a great addition to Doctrine : casting in DQL.

@Ocramius
Copy link
Member

@jonathaningram @Laloutre this problem is usually addressed by selecting the correct leaf in the FROM clause (or by adding another level in the hierarchy to be able to filter multiple leafs based on that field).

I personally think it's an edge case, and it's a good thing that users keep thinking strict OOP even in DQL...

@dvlpp
Copy link

dvlpp commented Nov 26, 2012

@Ocramius but the point is I can't select the correct leaf in the FROM clause, because there's no relation between the correct leaf and the entity we and querying: the relation is managed by the top of the hierarchy. The leaf contains specific properties, including the one I want to use in the order by clause of the query for which we know it concerns only this subtype.

@Ocramius
Copy link
Member

@Laloutre my point was using the correct entity (always).

@dvlpp
Copy link

dvlpp commented Nov 26, 2012

@Ocramius OK, first to be clear, I'm a OOP evangelist, I'm not trying here to bypass poor design problems (I think).

Let's try to describe a simple case (all words between quotes are entities, and DB tables): a "project" contains a dynamic list of properties ("property"), which are typed: each could be "number", "text", "date".
Now a given type a project define a startdate (date), a enddate (date) and a budget (number). I can easily query all projects with a startdate > 2012:

select p from mymodels\Project p where exists (select d from mymodels\Date where d.project=p and d.name='startdate' and d.datevalue >= '2012-01-01 00:00:00')

BUT I can't list all projects order by startdate, because the datevalue field isn't accessible from my "attribute" (it's in the "date subclass), and the relation is made between "project" and "attribute". The @jonathaningram proposal seems elegant to me in this case, since I know I would only have "date" in this join:

select p from mymodels\Project left join (mymodels\Date) p.attributes d order by d.datevalue

@Ocramius
Copy link
Member

@Laloutre I wasn't assuming that you were bad at OOP :)

I see the limitation, but instead of a cast I'd prefer that the DQL parser recognizes p.attributes as mymodels\Date when INSTANCE OF mymodels\Date is used (otherwise it may also look like you're casting something random to a mymodels\Date).

This becomes real messy btw, and you're trying to fit EAV (in your example) into the problem of mapping. That's always a mess. If there's a clean solution for this that doesn't imply making parsing and result set mapping building a total mess, go on and implement!

But otherwise I'd just tell you to go NativeSQL and apply your own ResultSetMapping.

@dvlpp
Copy link

dvlpp commented Nov 26, 2012

@Ocramius I understand your point. We can probably agree on this: it's an OOP to SGBDR problem, and there is no perfect solution. The instance of solution could work too, while I think the query would be less readable.
Well, until a find a good way to do it in DQL, I'll stay with NativeSQL and (sometimes) database schema adaptations.

Thanks for your time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants