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

QueryBuilder does not add params for Expression in where case #9452

Closed
Tigrov opened this issue Aug 18, 2015 · 3 comments
Closed

QueryBuilder does not add params for Expression in where case #9452

Tigrov opened this issue Aug 18, 2015 · 3 comments
Assignees
Labels
Milestone

Comments

@Tigrov
Copy link
Member

Tigrov commented Aug 18, 2015

It does not work if I pass params to new Expression()

AR::find()->where(new Expression(':item = ANY (array)', ['item' => 'value']))->all();

but it works if I pass params to where()

AR::find()->where(new Expression(':item = ANY (array)'), ['item' => 'value'])->all();

sometimes it is not comfortable when I pass where expression as parameter of function or method

function getAll($where) {
    return AR::find()->where($where)->all();
}

// it does not work
$models = getAll(new Expression(':item = ANY (array)', ['item' => 'value']));

This can be fixed if to add one more condition in QueryBuilder::buildCondition()
https://github.com/yiisoft/yii2/blob/master/framework/db/QueryBuilder.php#L910

if ($condition instanceof Expression) {
    foreach ($condition->params as $n => $v) {
        $params[$n] = $v;
    }
    return (string) $condition;
}

Also it requires for QueryBuilder::buildAndCondition() and QueryBuilder::buildNotCondition()
https://github.com/yiisoft/yii2/blob/master/framework/db/QueryBuilder.php#L975
https://github.com/yiisoft/yii2/blob/master/framework/db/QueryBuilder.php#L1004
to replace if (is_array($operand)) { on if (is_array($operand) || $operand instanceof Expression) {

@klimov-paul
Copy link
Member

I don't get it: what is the sense of passing Expression instance to Query::where(), while you can use a plain string for this?

AR::find()->where(':item = ANY (array)', ['item' => 'value'])->all();

@mnvx
Copy link

mnvx commented Aug 20, 2015

This situation described by Tigrov:

sometimes it is not comfortable when I pass where expression as parameter of function or method

function getAll($where) {
return AR::find()->where($where)->all();
}

And I agree with him. Sometimes it is handy to prepare where condition dynamically as a single object. In this case code where($expression) is more handy when where($expression->expression, $expression->params).
And if Expression using in some methods of QueryBuilder, it is good decision go use it in where(). Isn't it?

@klimov-paul
Copy link
Member

Resolved by commit bea90e1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants