Merge lp:~allenap/storm/get-set-expr-columns into lp:storm

[Gavin Panella]

ResultSet.values() can now return values from set expressions.

Enables ResultSet.values() to operate on set expressions. It does this with a pass-through property, SetExpr.columns. An alternative implementation could be to deal specially with SetExprs in values() itself.

[James Henstridge (jamesh)]

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.

review: Needs Fixing

Unmerged revisions

358. By Gavin Panella on 2010-04-15

Tests to show ResultSet.values() working on a set expression.

357. By Gavin Panella on 2010-04-15

Create a pass-through columns property on SetExpr.

356. By Gavin Panella on 2010-04-15

ResultSet.values() can now accept column names as well as columns themselves.

=== modified file 'storm/expr.py'
--- storm/expr.py	2009-11-02 11:11:20 +0000
+++ storm/expr.py	2010-04-22 15:57:27 +0000
@@ -1136,6 +1136,20 @@
                 first.offset is Undef):
                 self.exprs = first.exprs + self.exprs[1:]
 
+    def _get_columns(self):
+        if len(self.exprs) > 0:
+            # The find specs of all sub-expressions should be
+            # compatible, so simply return the first.
+            return self.exprs[0].columns
+        else:
+            return []
+
+    def _set_columns(self, columns):
+        for expr in self.exprs:
+            expr.columns = columns
+
+    columns = property(_get_columns, _set_columns)
+
 
 @compile.when(SetExpr)
 def compile_set_expr(compile, expr, state):

=== modified file 'tests/expr.py'
--- tests/expr.py	2009-11-02 11:11:20 +0000
+++ tests/expr.py	2010-04-22 15:57:27 +0000
@@ -294,6 +294,16 @@
         self.assertEquals(expr.expr, objects[0])
         self.assertEquals(expr.name, objects[1])
 
+    def test_set_expr_columns(self):
+        select1 = Select((elem1, elem2))
+        select2 = Select((elem2, elem1))
+        expr = SetExpr(select1, select2)
+        self.assertEquals((elem1, elem2), expr.columns)
+        expr.columns = []
+        self.assertEquals([], expr.columns)
+        self.assertEquals([], select1.columns)
+        self.assertEquals([], select2.columns)
+
     def test_union(self):
         expr = Union(elem1, elem2, elem3)
         self.assertEquals(expr.exprs, (elem1, elem2, elem3))

=== modified file 'tests/store/base.py'
--- tests/store/base.py	2010-04-22 15:57:27 +0000
+++ tests/store/base.py	2010-04-22 15:57:27 +0000
@@ -31,7 +31,7 @@
 from storm.variables import PickleVariable
 from storm.expr import (
     Alias, And, Asc, Avg, Count, Desc, Eq, LeftJoin, Lower, Or, SQL, Select,
-    Sum)
+    Sum, Union)
 from storm.variables import Variable, UnicodeVariable, IntVariable
 from storm.info import get_obj_info, ClassAlias
 from storm.exceptions import *
@@ -1047,6 +1047,16 @@
         values = self.store.find(Foo).order_by(Foo.id)[1:2].values(Foo.id)
         self.assertEquals(list(values), [20])
 
+    def test_find_set_expr_values(self):
+        result = self.store.find(Foo).union(self.store.find(Foo), all=True)
+        values = result.values(Foo.id)
+        self.assertEquals(sorted(values), [10, 10, 20, 20, 30, 30])
+
+    def test_find_set_expr_values_by_column_name(self):
+        result = self.store.find(Foo).union(self.store.find(Foo), all=True)
+        values = result.values('id')
+        self.assertEquals(sorted(values), [10, 10, 20, 20, 30, 30])
+
     def test_find_remove(self):
         self.store.find(Foo, Foo.id == 20).remove()
         self.assertEquals(self.get_items(), [