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

[11.x] Fully qualify morph columns when using WHERE clauses #52227

Merged

Conversation

maartenpaauw
Copy link
Contributor

When using multiple tables with the same morph columns, the query could be ambitious. This pull request fix that by qualifying all morph columns.

@maartenpaauw maartenpaauw changed the title fix(database): fully qualify morph columns when using WHERE clauses [11.x] Fully qualify morph columns when using WHERE clauses Jul 22, 2024
@maartenpaauw
Copy link
Contributor Author

I'm not sure why the testItCanTravelByMicroseconds test is failing. I think the changes I've made didn't affect any of that logic.

@driesvints driesvints marked this pull request as draft July 23, 2024 06:54
@driesvints
Copy link
Member

Hi there. Please rebase this PR as we've fixed the test suite on the 11.x branch. Then mark your PR as ready again.

@maartenpaauw maartenpaauw force-pushed the bugfix/fully-qualify-morph-columns branch from 1ffff8b to 25e0d98 Compare July 23, 2024 06:58
@maartenpaauw
Copy link
Contributor Author

@driesvints alright! That you for the clarification. I've rebased this branch and all tests are passing.

@maartenpaauw maartenpaauw marked this pull request as ready for review July 23, 2024 07:05
@maartenpaauw maartenpaauw force-pushed the bugfix/fully-qualify-morph-columns branch from 25e0d98 to 0f13977 Compare July 24, 2024 14:20
@taylorotwell
Copy link
Member

How do you have two tables with the exact same morph column names?

@taylorotwell taylorotwell merged commit c2eb318 into laravel:11.x Jul 29, 2024
29 checks passed
@maartenpaauw
Copy link
Contributor Author

Thanks for merging this PR. I’m sorry for my delayed response. I couldn’t find any time earlier, but I still want to explain why this is needed.

What I meant by multiple tables having the same morph column names is the following situation.

Let’s assume we have a parent and a child model. Both models have a morph to a contact. Let’s also assume the parent and child models aren’t always morphed to the same contact. Therefore, both models have contactable_type and contactable_id columns.

If we want to scope the parent relationship and include the child relationships, we’ll get an overly ambitious MySQL error.

The same column qualification is used when calling the whereBelongsTo method.

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.

3 participants