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

Problem with Database BaseBuilder binds #1226

Closed
Martin-4Spaces opened this issue Sep 18, 2018 · 3 comments
Closed

Problem with Database BaseBuilder binds #1226

Martin-4Spaces opened this issue Sep 18, 2018 · 3 comments
Labels
bug Verified issues on the current code behavior or pull requests that will fix them database Issues or pull requests that affect the database layer
Milestone

Comments

@Martin-4Spaces
Copy link

Imagine this query.

$model = new UserModel();
$user = $model
        ->where('id', 2)
        ->orWhereIn('id', [3, 4])
        ->find(); 

where('id', 2) will generate a bind in BaseBuilder with key id.
orWhereIn('id', [3, 4]) will do the same.

We will end up with one bind, when we should have two.
The final query will end up like this

SELECT *
FROM `users`
WHERE `users`.`id` = (3,4)
OR `users`.`id` IN (3,4)

I see that you have already tried to deal with this issue by adding a setBind-method in BaseBuilder. But whereIn() does not use this method. I've tried replacing
$this->binds[$ok] = $where_in;
With
$this->setBind($ok, $where_in);
But this does not solve the problem. I still end up with a query like this

SELECT * 
FROM `users` 
WHERE `users`.`id` = 2 
OR `users`.`id` IN 2

Unfortunately I do not have more time to look at this. I hope someone else with take a look :-)

@Martin-4Spaces
Copy link
Author

I found a fix anyway. Look at this.
4spacesdarktemplar____phpstormprojects_4spacesdarktemplar__-_____system_database_basebuilder_php__4spacesdarktemplar_

@tada5hi
Copy link
Contributor

tada5hi commented Sep 23, 2018

having the same issue. Your solution fixed mine as well

@lonnieezell
Copy link
Member

Will have to dig a little deeper when I have a little more time, but that's not a perfect fix. It needs to be fixed in the setBind method, since that helps to ensure that name collisions don't happen, etc.

@jim-parry jim-parry added the bug Verified issues on the current code behavior or pull requests that will fix them label Oct 19, 2018
@jim-parry jim-parry added this to the 4.0.0 milestone Oct 19, 2018
@jim-parry jim-parry added the database Issues or pull requests that affect the database layer label Dec 10, 2018
lonnieezell added a commit that referenced this issue Jan 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them database Issues or pull requests that affect the database layer
Projects
None yet
Development

No branches or pull requests

4 participants