Skip to content

[10.x] Convert Expression to string for from in having subqueries - #48525

Merged
taylorotwell merged 1 commit into
laravel:10.xfrom
ikari7789:get-from-expressions-string
Sep 28, 2023
Merged

taylorotwell merged 1 commit into
laravel:10.xfrom
ikari7789:get-from-expressions-string

Conversation

@ikari7789

@ikari7789 ikari7789 commented Sep 25, 2023

Copy link
Copy Markdown
Contributor

Attempting to override the having from inside of the whereHaving() callback will fail due to the removal of __toString() logic with Laravel 10.x.

This adds back the ability to override the local table with a subquery in the having, which worked on previous versions of Laravel before 10.x.

Comment thread src/Illuminate/Database/Eloquent/Concerns/QueriesRelationships.php Outdated
Comment thread tests/Database/DatabaseEloquentBuilderTest.php Outdated
@ikari7789
ikari7789 force-pushed the get-from-expressions-string branch from fc01137 to 2c31616 Compare September 26, 2023 06:07
@ikari7789
ikari7789 force-pushed the get-from-expressions-string branch from 2c31616 to 2591bbf Compare September 26, 2023 06:08
@taylorotwell
taylorotwell marked this pull request as draft September 26, 2023 21:29
@ikari7789
ikari7789 requested a review from tpetry September 27, 2023 03:11

@tpetry tpetry left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now but didn't run it myself.

@ikari7789

Copy link
Copy Markdown
Contributor Author

@tpetry thank you for your check. If there's anything more for me to help, please let me know. I haven't contributed to Laravel much so not too familiar with the typical flow.

@ikari7789
ikari7789 marked this pull request as ready for review September 28, 2023 00:18
@taylorotwell
taylorotwell merged commit 4b22e39 into laravel:10.x Sep 28, 2023
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