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

[5.5] Clear "select" bindings whenever select columns have been override. #21486

Closed
wants to merge 4 commits into from
Closed

Conversation

wanghanlin
Copy link
Contributor

@wanghanlin wanghanlin commented Oct 1, 2017

Sorry for not writing a detailed description in last PR #21485

This PR revert some changes in last fix #21468. And add the code call ->setBindings([], 'select') when select() being called to fix an issue for cases query builder originally have select bindings but later being override cause a binding mismatch issue that lead to error.

The quickest way to reproduce error is run this in tinker

$query = app('Illuminate\Database\Query\Builder')->selectRaw('(select count(*) from posts where status = ?)', [1])->from('users');

$query will have bindings [1].
if later call

$query->select()->where('foo', 'bar');

$query will have bindings [1, "bar"] while it supposed to only have ["bar"]

@themsaid let me know your thoughts, thanks!

reference: #21472

@themsaid
Copy link
Member

themsaid commented Oct 2, 2017

The case referenced is not a bug, you're overriding your select statement, I'd consider this is an edge case but not a bug.

The reason why we can't add this to select() is it might break applications in a patch release, if you want you can submit this to the master branch so we take the time to test if it's breaking or not.

@themsaid themsaid closed this Oct 2, 2017
@wanghanlin
Copy link
Contributor Author

ok! I will create a PR once 5.5 is merged to 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.

2 participants