Fix SQL alias generation regression for simple inheritance

[rogregoire]

As asked by @ostrolucky in the #8229 issue - I'm created a PR trying to fix a edge case of what I think may be a regression in the effort of supporting Joined table inheritance with AttributeOverride.

Context:

• Entity1 extends Entity0 through ResolveTargetEntities mecanism
• Entity1 has all its fieldsMapping marked as inherited=true
• Entity1 inheritanceType is ClassMetadata::INHERITANCE_TYPE_NONE
• Some code calls $entityManager->getRepository($entity1)->findOneBy()
• The SQL generated in the where part of the query has a bad alias because it uses Entity0 as classname for getSQLTableAlias call in BasicEntityPersister::getSelectConditionStatementSQL method
• The query crash

Proposal:
Excluding INHERITANCE_TYPE_NONE entities to use inherited classname as parameter for getSQLTableAlias generation in BasicEntityPersister::getSelectConditionStatementSQL

As said in original comment, I'm really not sure of what I'm doing, I'm new to doctrine 2

Referenced Issue comments: #8229 (comment)

[ostrolucky]

Test case is also needed

[ostrolucky]

There is lot of problems in this PR. Please edit this PR and check this checkbox, so I can help you.

[ostrolucky]

Have you checked the checkbox yet?

[rogregoire]

That's all I found and it's already checked, Am I missing something ?

[ostrolucky]

I have cleaned up git history, removed unnecessary try catch in a test, unnecessary usage of ResolveTargetEntityListener and renamed entities you created. The listener has no effect on reproducing the issue.

If you plan to push some more commits, you need to pull with rebase your branch: git pull --rebase

[rogregoire]

Thanks,
Well, I didn't test without ResolveTargetEntityListener because that wasn't my bug case, but I guess that mean this bug must have impacted a lot more of people that I initially thought ?

[ostrolucky]

yes

[cziegenberg]

Thanks @rogregoire!

FYI: I just found two other untested use cases for which my #8229 fix doesn't work yet. I'm currently trying to create tests for it and will provide a PR soon.

[ostrolucky]

Thanks! Ping me in a PR afterwads

[cziegenberg]

@ostrolucky After some testing everything works fine. The problem was an outdated composer.lock file not matching the composer.json file and pointing to an old version.

[ostrolucky]

👍

[beberlei]

@cziegenberg can you please explain your use case? This looks like invalid mapping to me. But maybe I am wrong, are these two entities stored in one or two distinct tables in your case?

I am pretty sure this fails in many other places that only check for 'inherited' and nothing more. I believe this use-case is not actually supported by Doctrine reliably. Not sure if we should support this... The better approach would be to have a mapped superclass as the base, and then have two entities without properties extend from it.

[beberlei]

@rogregoire The ResolveEntityTargetListener does not work when you extend another entity with. Its specifically for implementing an interface. Your use-case of Entity1 extends Entity0 is only supported if you do Inheritence Mapping.

[cziegenberg]

@beberlei:

This isn't my PR, so do you really mean my usecase? My use case is explained in #8229. The goal is a "everything is a resource"-structure:

• a global resource table "app_resource" with the columns "resource_id" and "resource_type"
• an own table for each specific resource type (i.e. "app_user") with additional, type-specific columns, and an adjusted name for the identifier column to clearly name it and differ references
• other tables referencing either any type of resource (-> "app_resource") or a specific one (i.e. "app_user")

A mapped super class wouldn't allow to reference "any type of resource", right?