In most cases, I've used the replace_columns() helper in the store code where I need to fiddle with the column list for a query.
It'd probably be better to do the same in ResultSet.values(). The current code looks like it could leak changes outside of the method call, which could be problematic.
Secondly, I don't believe this patch will result in correct behaviour from ResultSet.values() for set expressions. Consider a table Foo containing the following data:
id | name
---+------
10 | value
20 | value
And then do the following:
result = store.find(Foo, id=10).union(store.find(Foo, id=20))
print list(result.values(Foo.name))
This will print ['value'] when it should print ['value', 'value']. The result set clearly has two rows, but if we rewrite the column lists to Foo.name then the union will only give us one row.
In most cases, I've used the replace_columns() helper in the store code where I need to fiddle with the column list for a query.
It'd probably be better to do the same in ResultSet.values(). The current code looks like it could leak changes outside of the method call, which could be problematic.
Secondly, I don't believe this patch will result in correct behaviour from ResultSet.values() for set expressions. Consider a table Foo containing the following data:
id | name
---+------
10 | value
20 | value
And then do the following:
result = store.find(Foo, id=10). union(store. find(Foo, id=20)) values( Foo.name) )
print list(result.
This will print ['value'] when it should print ['value', 'value']. The result set clearly has two rows, but if we rewrite the column lists to Foo.name then the union will only give us one row.