ActiveRecord: findAll() doesn't work with operator format condition

Yii 2.0 General Discussions
1

Hi folks,

For example:

$models = Model::findAll(['>=', 'date', $date]);

will return an empty array, because the condition is in operator format.

You had to write like this:

$models = Model::find()->where(['>=', 'date', $date])->all();

OK, but what about this:

Model::deleteAll(['>=', 'date', $date]);

This behaves as expected.

I found that this is not a bug and is by design.
There’s a note on this pitfall in the API document.

Note: As this is a short-hand method only, using more complex conditions, like [’!=’, ‘id’, 1] will not work. If you need to specify more complex conditions, use find() in combination with where() instead.

https://www.yiiframework.com/doc/api/2.0/yii-db-baseactiverecord#findAll()-detail

But, at the first glance, it is not what you expect.

Is there any drawback in implementing findAll() by simply wrapping find()->where()->all() ?

2

It is the following internally:

protected static function findByCondition($condition)
{
    $query = static::find();

    if (!ArrayHelper::isAssociative($condition) && !$condition instanceof ExpressionInterface) {
        // query by primary key
        $primaryKey = static::primaryKey();
        if (isset($primaryKey[0])) {
            // if condition is scalar, search for a single primary key, if it is array, search for multiple primary key values
            $condition = [$primaryKey[0] => is_array($condition) ? array_values($condition) : $condition];
        } else {
            throw new InvalidConfigException('"' . get_called_class() . '" must have a primary key.');
        }
    }

    return $query->andWhere($condition);
}

As far as I can see, that is done to replace search by hash-format condition with a search by primary keys. These two syntaxes conflict. That is what’s meant by “shortcut”.

1 Like
3

Also, I don’t see it should be fixed since it is not safe to pass user-supplied condition to a method supporting hash-format without pre-filtering while it’s OK to do it for shortcut-method.

4

Thank you for the reply, @samdark

So, the following table shows the comparison of the condition parameter of the relevant methods.

Method Scalar Value Non-Associative Array Associative Array Expression
findOne() PK Search PK search Column Search Generic Search
findAll() PK Search PK search Column Search Generic Search
where() (not supported) Operator Format Search Column Search Generic Search

I hope it is correct.

A non-associative array will be treated as the search key for the multi-column primary key in findOne() and findAll(), while it is treated as an operator-format conditions in where().

findOne() and findAll() follow the same logic, by sharing findByCondition() internally.

But I don’t think the PK search for findAll() has much meaning, since it will not return more than 1 record in an ordinary scenario. I’d rather want it to behave like where()->all() does.

5

Yeah. Likely you’re right but we can’t fix it anyway, it will be a clear backwards compatibility break.

Could be fixed in standalone AR though so reporting it as an issue worth it: GitHub - yiisoft/active-record: Active Record database abstraction layer

1 Like
6

Yep, BC and consistency really matter. I’m OK with the decision. :+1: