Storm

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

  1. resultselect
  2. Merge into trunk
Proposed by Jamu Kakar
Status: Rejected
Rejected by: Jamu Kakar
Proposed branch: lp://staging/~jkakar/storm/resultselect
Merge into: lp://staging/storm
Diff against target: 117 lines
To merge this branch: bzr merge lp://staging/~jkakar/storm/resultselect
Reviewer Review Type Date Requested Status
James Henstridge Abstain
Review via email: mp+9870@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Jamu Kakar (jkakar) wrote :

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.

Revision history for this message
Jamu Kakar (jkakar) wrote :

> 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.

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

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

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'storm/expr.py'
2--- storm/expr.py 2009-07-31 01:53:08 +0000
3+++ storm/expr.py 2009-08-13 20:20:20 +0000
4@@ -24,7 +24,7 @@
5 from copy import copy
6 import re
7
8-from storm.exceptions import CompileError, NoTableError, ExprError
9+from storm.exceptions import CompileError, NoTableError, ExprError, FeatureError
10 from storm.variables import (
11 Variable, RawStrVariable, UnicodeVariable, LazyValue,
12 DateTimeVariable, DateVariable, TimeVariable, TimeDeltaVariable,
13@@ -667,6 +667,30 @@
14 return "".join(tokens)
15
16
17+class ResultSelect(Select):
18+ __slots__ = ()
19+
20+ def __init__(self, result, *columns):
21+ """
22+ Transform a L{ResultSet} into a L{Select} expression that returns
23+ specified C{columns}.
24+
25+ @param result: A L{ResultSet} to transform into a L{Select} expression.
26+ @param columns: One or more L{Column} objects whose values will be
27+ included in the subselect column list.
28+ """
29+ if not columns:
30+ raise FeatureError("At least one column must be provided.")
31+ if result._select is not Undef:
32+ raise FeatureError("Can't be used with set expressions.")
33+ select = result._get_select()
34+ super(ResultSelect, self).__init__(
35+ columns, where=select.where, tables=select.tables,
36+ default_tables=select.default_tables, order_by=select.order_by,
37+ group_by=select.group_by, limit=select.limit, offset=select.offset,
38+ distinct=select.distinct, having=select.having)
39+
40+
41 class Insert(Expr):
42 """Expression representing an insert statement.
43
44
45=== modified file 'storm/store.py'
46--- storm/store.py 2009-07-31 01:53:08 +0000
47+++ storm/store.py 2009-08-13 20:20:20 +0000
48@@ -31,7 +31,7 @@
49 from storm.info import get_cls_info, get_obj_info, set_obj_info
50 from storm.variables import Variable, LazyValue
51 from storm.expr import (
52- Expr, Select, Insert, Update, Delete, Column, Count, Max, Min,
53+ Expr, Select, ResultSelect, Insert, Update, Delete, Column, Count, Max, Min,
54 Avg, Sum, Eq, And, Asc, Desc, compile_python, compare_columns, SQLRaw,
55 Union, Except, Intersect, Alias, SetExpr)
56 from storm.exceptions import (
57
58=== modified file 'tests/expr.py'
59--- tests/expr.py 2009-07-23 22:47:10 +0000
60+++ tests/expr.py 2009-08-13 20:20:20 +0000
61@@ -24,6 +24,7 @@
62
63 from storm.variables import *
64 from storm.expr import *
65+from storm.exceptions import FeatureError
66
67
68 class Func1(NamedFunc):
69@@ -52,6 +53,12 @@
70 return [TrackContext() for i in range(n)]
71
72
73+class FakeResultSet(object):
74+
75+ def __init__(self, **kwargs):
76+ self.__dict__.update(kwargs)
77+
78+
79 class ExprTest(TestHelper):
80
81 def test_select_default(self):
82@@ -683,6 +690,35 @@
83 self.assertEquals(order_by.context, EXPR)
84 self.assertEquals(group_by.context, EXPR)
85
86+ def test_result_select(self):
87+ select_expr = Select([column1, column2], Undef, [table1], distinct=True)
88+ result = FakeResultSet(_select=Undef, _get_select=lambda: select_expr)
89+ result_select_expr = ResultSelect(result, column1)
90+ state = State()
91+ statement = compile(result_select_expr, state)
92+ self.assertEquals(statement, 'SELECT DISTINCT column1 FROM "table 1"')
93+ self.assertEquals(state.parameters, [])
94+
95+ def test_result_select_without_columns(self):
96+ select_expr = Select([column1, column2], Undef, [table1], distinct=True)
97+ result = FakeResultSet(_select=Undef, _get_select=lambda: select_expr)
98+ self.assertRaises(FeatureError, ResultSelect, result)
99+
100+ def test_result_select_with_non_undef_select(self):
101+ select_expr = Select([column1, column2], Undef, [table1], distinct=True)
102+ result = FakeResultSet(_select=None, _get_select=lambda: select_expr)
103+ self.assertRaises(FeatureError, ResultSelect, result, column1)
104+
105+ def test_result_select_with_many_columns(self):
106+ select_expr = Select([column1, column2], Undef, [table1], distinct=True)
107+ result = FakeResultSet(_select=Undef, _get_select=lambda: select_expr)
108+ result_select_expr = ResultSelect(result, column1, column2)
109+ state = State()
110+ statement = compile(result_select_expr, state)
111+ self.assertEquals(statement,
112+ 'SELECT DISTINCT column1, column2 FROM "table 1"')
113+ self.assertEquals(state.parameters, [])
114+
115 def test_insert(self):
116 expr = Insert({column1: elem1, Func1(): Func2()}, Func2())
117 state = State()

Subscribers

People subscribed via source and target branches

to status/vote changes: