Merge lp:~jkakar/storm/resultselect into lp:storm

[Jamu Kakar]

[Jamu Kakar (jkakar)]

This is another crack at the problem, but using an expression
instead of modifying the ResultSet API. I think this will solve the
use cases we have without compromising the ResultSet API.

[Jamu Kakar (jkakar)]

> This is another crack at the problem, but using an expression
> instead of modifying the ResultSet API. I think this will solve the
> use cases we have without compromising the ResultSet API.

I'm planning to add more tests to make sure all the parameters
passed to super(ResultSelect, self).__init__ are handled correctly.
Before I do that I'd like to know if this basic approach is going to
be acceptable.

[James Henstridge (jamesh)]

I was happy with the previous interface to be honest, so I'm marking this abstain: Gustavo is the one to convince here.

I will note that this could just as easily be a factory function producing a Select instead of a new class. That would also remove the possibility of not transcribing all the Select attributes correctly.

review: Abstain

Unmerged revisions

328. By Jamu Kakar on 2009-08-08

- Added new ResultSelect expression that transforms a ResultSet into
  a Select expression.

=== modified file 'storm/expr.py'
--- storm/expr.py	2009-07-31 01:53:08 +0000
+++ storm/expr.py	2009-08-13 20:20:20 +0000
@@ -24,7 +24,7 @@
 from copy import copy
 import re
 
-from storm.exceptions import CompileError, NoTableError, ExprError
+from storm.exceptions import CompileError, NoTableError, ExprError, FeatureError
 from storm.variables import (
     Variable, RawStrVariable, UnicodeVariable, LazyValue,
     DateTimeVariable, DateVariable, TimeVariable, TimeDeltaVariable,
@@ -667,6 +667,30 @@
     return "".join(tokens)
 
 
+class ResultSelect(Select):
+    __slots__ = ()
+
+    def __init__(self, result, *columns):
+        """
+        Transform a L{ResultSet} into a L{Select} expression that returns
+        specified C{columns}.
+
+        @param result: A L{ResultSet} to transform into a L{Select} expression.
+        @param columns: One or more L{Column} objects whose values will be
+            included in the subselect column list.
+        """
+        if not columns:
+            raise FeatureError("At least one column must be provided.")
+        if result._select is not Undef:
+            raise FeatureError("Can't be used with set expressions.")
+        select = result._get_select()
+        super(ResultSelect, self).__init__(
+            columns, where=select.where, tables=select.tables,
+            default_tables=select.default_tables, order_by=select.order_by,
+            group_by=select.group_by, limit=select.limit, offset=select.offset,
+            distinct=select.distinct, having=select.having)
+
+
 class Insert(Expr):
     """Expression representing an insert statement.
 

=== modified file 'storm/store.py'
--- storm/store.py	2009-07-31 01:53:08 +0000
+++ storm/store.py	2009-08-13 20:20:20 +0000
@@ -31,7 +31,7 @@
 from storm.info import get_cls_info, get_obj_info, set_obj_info
 from storm.variables import Variable, LazyValue
 from storm.expr import (
-    Expr, Select, Insert, Update, Delete, Column, Count, Max, Min,
+    Expr, Select, ResultSelect, Insert, Update, Delete, Column, Count, Max, Min,
     Avg, Sum, Eq, And, Asc, Desc, compile_python, compare_columns, SQLRaw,
     Union, Except, Intersect, Alias, SetExpr)
 from storm.exceptions import (

=== modified file 'tests/expr.py'
--- tests/expr.py	2009-07-23 22:47:10 +0000
+++ tests/expr.py	2009-08-13 20:20:20 +0000
@@ -24,6 +24,7 @@
 
 from storm.variables import *
 from storm.expr import *
+from storm.exceptions import FeatureError
 
 
 class Func1(NamedFunc):
@@ -52,6 +53,12 @@
     return [TrackContext() for i in range(n)]
 
 
+class FakeResultSet(object):
+
+    def __init__(self, **kwargs):
+        self.__dict__.update(kwargs)
+
+
 class ExprTest(TestHelper):
 
     def test_select_default(self):
@@ -683,6 +690,35 @@
         self.assertEquals(order_by.context, EXPR)
         self.assertEquals(group_by.context, EXPR)
 
+    def test_result_select(self):
+        select_expr = Select([column1, column2], Undef, [table1], distinct=True)
+        result = FakeResultSet(_select=Undef, _get_select=lambda: select_expr)
+        result_select_expr = ResultSelect(result, column1)
+        state = State()
+        statement = compile(result_select_expr, state)
+        self.assertEquals(statement, 'SELECT DISTINCT column1 FROM "table 1"')
+        self.assertEquals(state.parameters, [])
+
+    def test_result_select_without_columns(self):
+        select_expr = Select([column1, column2], Undef, [table1], distinct=True)
+        result = FakeResultSet(_select=Undef, _get_select=lambda: select_expr)
+        self.assertRaises(FeatureError, ResultSelect, result)
+
+    def test_result_select_with_non_undef_select(self):
+        select_expr = Select([column1, column2], Undef, [table1], distinct=True)
+        result = FakeResultSet(_select=None, _get_select=lambda: select_expr)
+        self.assertRaises(FeatureError, ResultSelect, result, column1)
+
+    def test_result_select_with_many_columns(self):
+        select_expr = Select([column1, column2], Undef, [table1], distinct=True)
+        result = FakeResultSet(_select=Undef, _get_select=lambda: select_expr)
+        result_select_expr = ResultSelect(result, column1, column2)
+        state = State()
+        statement = compile(result_select_expr, state)
+        self.assertEquals(statement,
+                          'SELECT DISTINCT column1, column2 FROM "table 1"')
+        self.assertEquals(state.parameters, [])
+
     def test_insert(self):
         expr = Insert({column1: elem1, Func1(): Func2()}, Func2())
         state = State()