Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lifts an unnecessary restriction on ResultSetMappingBuilder #858

Merged
merged 3 commits into from
Feb 8, 2014

Conversation

schmittjoh
Copy link
Member

I've tested this locally, and it seems to work perfectly fine to use the rsm builder when querying for a concrete class (not a potentially abstract base class) in a single-table inheritance scheme.

@doctrinebot
Copy link

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

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

We use Jira to track the state of pull requests and the versions they got
included in.

@stof
Copy link
Member

stof commented Nov 24, 2013

Can you add a test covering this to avoid regressions ?

@schmittjoh
Copy link
Member Author

Sure, I was looking for some feedback first though.

On Sun, Nov 24, 2013 at 11:01 AM, Christophe Coevoet <
notifications@github.com> wrote:

Can you add a test covering this to avoid regressions ?


Reply to this email directly or view it on GitHubhttps://github.com//pull/858#issuecomment-29152266
.

{
if ($classMetadata->isInheritanceTypeSingleTable()
&& in_array($classMetadata->name, $classMetadata->discriminatorMap, true)) {
return true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not clear to me what case you are covering here. STI works when the selected entity is part of the discriminator map? Are there cases where that wouldn't be true?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, for the base class of the hierarchy.

On Tue, Nov 26, 2013 at 5:48 AM, Marco Pivetta notifications@github.51.alwrote:

In lib/Doctrine/ORM/Query/ResultSetMappingBuilder.php:

@@ -179,6 +179,16 @@ protected function addAllClassFields($class, $alias, $columnAliasMap = array())
}
}

  • private function isInheritanceSupported(ClassMetadataInfo $classMetadata)
  • {
  •    if ($classMetadata->isInheritanceTypeSingleTable()
    
  •        && in_array($classMetadata->name, $classMetadata->discriminatorMap, true)) {
    
  •        return true;
    

It is not clear to me what case you are covering here. STI works when the
selected entity is part of the discriminator map? Are there cases where
that wouldn't be true?


Reply to this email directly or view it on GitHubhttps://github.com//pull/858/files#r7915280
.

@beberlei
Copy link
Member

beberlei commented Jan 2, 2014

If this works fine and we can get a test I will merge it.

beberlei added a commit that referenced this pull request Feb 8, 2014
lifts an unnecessary restriction on ResultSetMappingBuilder
@beberlei beberlei merged commit 8e3f456 into doctrine:master Feb 8, 2014
@schmittjoh schmittjoh deleted the rsmBuilder branch February 8, 2014 17:10
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.

5 participants