Code review comment for lp:~cr3/storm/reference_is_in

Revision history for this message
James Henstridge (jamesh) wrote :

The implementation looks wrong for compound key references when a list of objects is passed in:

    for i, other in enumerate(others):
        where = self._relation.get_where_for_local(other)
        others[i] = where.expr2

In such a case, get_where_for_local() will return an And() object, which has no expr2 attribute.

Probably the best you can do in such a situation would be something like:

    return Or(*[self._relation.get_where_for_local(other)
                for other in others])

You can do better for PostgreSQL and MySQL with "(key1, key2) in ((val1, val2), (val1', val2'), ...)" but that doesn't seem to work with SQLite.

You'd also want to make sure it does the right thing when passed an empty list.

review: Needs Fixing

« Back to merge proposal