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

[9.x] Prevent triggering belongs to query if there are no relations to load #42564

Closed
wants to merge 2 commits into from

Conversation

pionl
Copy link
Contributor

@pionl pionl commented May 30, 2022

References issue #42493

  • At this moment add the protection only for BelongsTo relation (can be added to Relation if more use cases are found)
  • Prevents a query with empty whereIn value which reduces queries to DB.

@taylorotwell
Copy link
Member

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If possible, please consider releasing your code as a package so that the community can still take advantage of your contributions!

If you feel absolutely certain that this code corrects a bug in the framework, please "@" mention me in a follow-up comment with further explanation so that GitHub will send me a notification of your response.

@pionl
Copy link
Contributor Author

pionl commented May 30, 2022

Hi @taylorotwell, I truly believe this is a bug that bothers me from L5.4. Why would framework want to trigger a query on database that will return "nothing". This is a quite simple fix.

Of course I can make my "own" belongs to relation and use it, but I believe that hurts the quality of the framework that all the people like.

Here is a copy of explanation from issue. Thank you for your time.

  • Laravel Version: 9.13.0
  • PHP Version: 8.1.2

Description:

  • While using belongsTo relations (with/load) there can be situations when there are no relations (nullable relations) to be loaded and query is "invalid". -> select `id`, `name`, `public_name` from `options` where 0 = 1 .
  • This similar issue can be potentially found on other relations
  • This bug is not noticeable if the results contains at least 1 value in the relation (if you have pagination, some pages can contain this error).

Steps To Reproduce:

  1. Create a table with optional belongsTo relations
  2. Build a query to get the results (all results should contain null)
  3. Here a screenshot of a real queries

Snímek obrazovky 2022-05-24 v 14 22 56

@pionl
Copy link
Contributor Author

pionl commented Jul 6, 2022

HI @taylorotwell, any second thoughts? Thank you

@pionl
Copy link
Contributor Author

pionl commented Jan 10, 2023

Hi @taylorotwell as 10.X version is ahead of us, could we reconsider fixing this bug? This will prevent sending unnecessary query. We could say this makes Laravel more cost efficient, and we all love that, right? :)

I'm happy to update it to 10.x.

Thank you for everything

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