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

The schema tool now doesn't add a foreign constraint when subclassess of... #440

Closed
wants to merge 2 commits into from

Conversation

sroddy
Copy link

@sroddy sroddy commented Sep 8, 2012

... a STI use the same field to map relations with entities of different classes

It seems that there are no particular side-effects in mapping different relationship using the same table column name in different subclasses of a STI hierarchy.
Eg. Tag superclass of BookTag (id, object_id, comment) and MovieTag (id, object_id, comment), so having two attributes $book and $movie mapped to the same object_id field with @joincolumn annotation

I wasn't sure about this so I tested to see if there were errors. And I was surprised that everything runs without any particular problem.
Moreover this is a really interesting (wanted? unwanted?) feature, very useful if you have huge tables that need to be joined with other table using the discriminator and object_id columns, or if you have some kind of logging class and you need to keep the table efficient and small. The only problem is that the schema generator always inserts a foreign column constraint (probably the last specified) when generating these kind of Entities.

This PR ensures that the foreign key is not added if there are at least two different classes mapped on the same field of a db table.

@stof
Copy link
Member

stof commented Sep 8, 2012

This needs tests

Stefano Rodriguez added 2 commits September 8, 2012 15:10
… of a STI use the same field to map relations with entities of different classes
@sroddy
Copy link
Author

sroddy commented Sep 8, 2012

@stof the PR should be ready now.

@sroddy
Copy link
Author

sroddy commented Sep 13, 2012

@beberlei @guilhermeblanco what do you think about this?

beberlei pushed a commit that referenced this pull request Nov 12, 2012
@beberlei
Copy link
Member

Merged into master.

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.

None yet

3 participants