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

Invalid discriminator maps should be detected during schema/mapping validation #6558

Closed
jeroenvrooij opened this issue Jul 18, 2017 · 8 comments · Fixed by #10411
Closed
Assignees
Labels

Comments

@jeroenvrooij
Copy link

jeroenvrooij commented Jul 18, 2017

Given the following structure of entities, which are discriminated over two levels, fetching a Developer using the repository from the root entity (Person) results in exceptions.

doctrine2

The issue is that an Employee object is being instantiated, which fails since that class is abstract. So it seems that Doctrine is not following the tree until the leaf nodes. Fetching the Developer using the repository from it's direct parent (so the Employee) works fine.

This may, or may not, give some insights as well:
Persisting a Developer works ok, except that the order of insert queries executed are wrong (in my opinion). The Person is inserted first, followed by the Developer and the Employee is inserted last. (So: root -> leaf -> 2nd layer). This forces us to not have foreign keys on the id columns of the Employee, Staff and Developer.

Note:
I will submit some test cases which test the two scenarios of fetching using the different repositories.

Edit:
PR with the test case: #6559

@Ocramius
Copy link
Member

Closing as invalid as per #6559 (comment)

The issue is that the schema validation tools didn't discover a duplicate inheritance definition, but that's for a separate issue to fix.

@jeroenvrooij
Copy link
Author

Could you please go in to a bit more detail? Because at the moment I do not understand if I made a mistake in the inheritance mapping or that multilevel discrimination is not even supported in the first place.
We also tried having the same structure of entities, but only have to inheritance mapping on the top level like you said. But that leads to different errors: during the fetching of a Developer the Employee properties would not get populated.

@Ocramius
Copy link
Member

You basically need to define the inheritance mapping only at the very top of the inheritance.

@jeroenvrooij
Copy link
Author

Yeah I get that. But like I said, that is also not a valid mapping. We tried that as well. Please check this PR with the updated test case which hopefully clears things up :). #6578

@Ocramius
Copy link
Member

@jeroenvrooij re-opening then, thanks for clarifying with a new test case 👍

@Ocramius
Copy link
Member

As discussed in #6578 (comment), this issue is a schema validator problem: some discriminator values are missing in the discriminator map.

@Ocramius Ocramius changed the title Fetching entities which are discriminated over multiple levels does not work. Invalid discriminator maps should be detected during schema/mapping validation Aug 19, 2017
@mstefan21
Copy link

HI, i have a same problem with multiple inheritance, but i had different discriminator column names. It's ok? or i must have same discriminator column name?

Our structure is same as image, but i in Image is discriminator column name "PersonType" and in Employee is discriminator column name is "EmployeeType", if i call findAll() on Person, i get only Managers and Employes and doctrine not resolve Epmployees inheritance.

@przemyslaw-leczycki-valueadd
Copy link

przemyslaw-leczycki-valueadd commented Nov 30, 2017

Hi @Ocramius I've got related problem to this.
My entity structure is

Item -> Document
        Deviation
        Media -> Image
                 ...

I've got discriminators specified in Item class.
Item is an abstract class as well as Media.

If I use $mediaRepository->find($id) everything works fine

SELECT t1.name AS name_3, t1.deleted_at AS deleted_at_4, t1.id AS id_5, t1.created_at AS created_at_6, t1.updated_at AS updated_at_7, t0.description AS description_8, t0.photographer AS photographer_9, t0.expired AS expired_10, t0.filePath_path AS filePath_path_11, t1.folder_id AS folder_id_12, t1.site_id AS site_id_13, t1.archived_by_id AS archived_by_id_14, t1.created_by_id AS created_by_id_15, t1.updated_by_id AS updated_by_id_16, t1.type, t2.zoom_level AS zoom_level_17, t2.position_y AS position_y_18, t2.position_x AS position_x_19, t2.rotation_value AS rotation_value_20 
FROM items_media t0 
INNER JOIN items t1 ON t0.id = t1.id 
LEFT JOIN items_media_type_image t2 
ON t0.id = t2.id 
WHERE t1.id = ? AND ((t1.deleted_at IS NULL) AND (t1.site_id = 'DE09FACA-7210-11E7-8CF7-A6006AD3DBA0') AND (t1.deleted_at IS NULL))

if I use $itemRepository->find($id) none of the values from Media Entity are populated to leaf entity.

SELECT t0.name AS name_4, t0.deleted_at AS deleted_at_5, t0.id AS id_6, t0.created_at AS created_at_7, t0.updated_at AS updated_at_8, t0.folder_id AS folder_id_9, t0.site_id AS site_id_10, t0.archived_by_id AS archived_by_id_11, t0.created_by_id AS created_by_id_12, t0.updated_by_id AS updated_by_id_13, t0.type, t1.description AS description_14, t1.numeric_identifier AS numeric_identifier_15, t1.status_status AS status_status_16, t2.numeric_identifier AS numeric_identifier_17, t2.status_status AS status_status_18, t2.impacts_impacts_health AS impacts_impacts_health_19, t2.impacts_impacts_environment AS impacts_impacts_environment_20, t2.impacts_impacts_safety AS impacts_impacts_safety_21, t2.impacts_impacts_quality AS impacts_impacts_quality_22, t3.zoom_level AS zoom_level_23, t3.position_y AS position_y_24, t3.position_x AS position_x_25, t3.rotation_value AS rotation_value_26 
FROM items t0 
LEFT JOIN items_documents t1 ON t0.id = t1.id 
LEFT JOIN items_deviations t2 ON t0.id = t2.id 
LEFT JOIN items_media_type_image t3 ON t0.id = t3.id 
WHERE t0.id = ? AND ((t0.deleted_at IS NULL) AND (t0.site_id = 'DE09FACA-7210-11E7-8CF7-A6006AD3DBA0') AND (t0.deleted_at IS NULL))

As you see on attached SQL dumps in second example doctrine didn't join items_media table at all which cause all properties from Media Entity that should be in Image Entity are null or empty string.
I saw it's reproduced in PR #6578 method testEmployeeIsPopulated().
I didn't find explicitly related issue to this. Should I create new one or it's related to this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants