Storm

Merge lp://qastaging/~jamesh/storm/bug-349482 into lp://qastaging/storm

  1. bug-349482
  2. Merge into trunk
Proposed by James Henstridge
Status: Merged
Merged at revision: not available
Proposed branch: lp://qastaging/~jamesh/storm/bug-349482
Merge into: lp://qastaging/storm
Diff against target: 303 lines
To merge this branch: bzr merge lp://qastaging/~jamesh/storm/bug-349482
Reviewer Review Type Date Requested Status
Gustavo Niemeyer Approve
Thomas Herve (community) Approve
Review via email: mp+6693@code.qastaging.launchpad.net
To post a comment you must log in.
Revision history for this message
James Henstridge (jamesh) wrote :

Change the way compile_python works to pass constants in directly rather than trying to stringify them. Among other things, this makes it possible to handle variables whose values do not have a repr() that is valid Python code.

I also updated the get_column() callback to take a column object rather than going column -> name -> column conversions when getting column values in the matcher.

Revision history for this message
Thomas Herve (therve) wrote :

+ exec code in namespace
+ return namespace['closure'](state.parameters, bool)

Can you add a local dict and do this:

+ local = {}
+ exec code in namespace, local
+ return local['closure'](state.parameters, bool)

This is it will be even cleaner.

+1!

review: Approve
Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

Nice improvements! Thank you!

James, I recall something about this change being motivated by an actual failing expression. Is it easy to reproduce this case in a test just so that we ensure this case won't ever come back again?

+1!

review: Approve
lp://qastaging/~jamesh/storm/bug-349482 updated
310. By James Henstridge

Add a test to show that get_matcher() works for values whose repr() is
not a valid Python expression.

311. By James Henstridge

Add a NEWS item about changes.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2009-06-13 12:18:15 +0000
3+++ NEWS 2009-06-18 15:20:10 +0000
4@@ -28,6 +28,8 @@
5 - The test suite now passes when run with Python 2.6.
6 - ListVariable now converts its elements to database representation
7 correctly (bug #136806 reported by Marc Tardif).
8+ - compile_python now works for values that don't produce valid Python
9+ expressions with repr().
10
11
12 0.14 (2009-01-09)
13
14=== modified file 'storm/expr.py'
15--- storm/expr.py 2008-06-27 16:44:28 +0000
16+++ storm/expr.py 2009-06-18 15:20:10 +0000
17@@ -206,8 +206,18 @@
18 class CompilePython(Compile):
19
20 def get_matcher(self, expr):
21- exec "def match(get_column): return bool(%s)" % self(expr)
22- return match
23+ state = State()
24+ source = self(expr, state)
25+ namespace = {}
26+ code = ("def closure(parameters, bool):\n"
27+ " [%s] = parameters\n"
28+ " def match(get_column):\n"
29+ " return bool(%s)\n"
30+ " return match" %
31+ (",".join("_%d" % i for i in range(len(state.parameters))),
32+ source))
33+ exec code in namespace
34+ return namespace['closure'](state.parameters, bool)
35
36
37 class State(object):
38@@ -348,12 +358,18 @@
39 return "NULL"
40
41
42-@compile_python.when(str, unicode, bool, int, long, float,
43- datetime, date, time, timedelta, type(None))
44+@compile_python.when(str, unicode, int, long, float, type(None))
45 def compile_python_builtin(compile, expr, state):
46 return repr(expr)
47
48
49+@compile_python.when(bool, datetime, date, time, timedelta)
50+def compile_python_bool_and_dates(compile, expr, state):
51+ index = len(state.parameters)
52+ state.parameters.append(expr)
53+ return "_%d" % index
54+
55+
56 @compile.when(Variable)
57 def compile_variable(compile, variable, state):
58 state.parameters.append(variable)
59@@ -361,7 +377,9 @@
60
61 @compile_python.when(Variable)
62 def compile_python_variable(compile, variable, state):
63- return repr(variable.get())
64+ index = len(state.parameters)
65+ state.parameters.append(variable.get())
66+ return "_%d" % index
67
68
69 # --------------------------------------------------------------------
70@@ -776,7 +794,9 @@
71
72 @compile_python.when(Column)
73 def compile_python_column(compile, column, state):
74- return "get_column(%s)" % repr(column.name)
75+ index = len(state.parameters)
76+ state.parameters.append(column)
77+ return "get_column(_%d)" % index
78
79
80 # --------------------------------------------------------------------
81
82=== modified file 'storm/store.py'
83--- storm/store.py 2009-02-16 10:44:31 +0000
84+++ storm/store.py 2009-06-18 15:20:10 +0000
85@@ -1349,11 +1349,8 @@
86 match = None
87 else:
88 match = compile_python.get_matcher(self._where)
89- name_to_column = dict(
90- (column.name, column)
91- for column in self._find_spec.default_cls_info.columns)
92- def get_column(name, name_to_column=name_to_column):
93- return obj_info.variables[name_to_column[name]].get()
94+ def get_column(column):
95+ return obj_info.variables[column].get()
96 objects = []
97 cls = self._find_spec.default_cls_info.cls
98 for obj_info in self._store._iter_alive():
99
100=== modified file 'tests/expr.py'
101--- tests/expr.py 2008-10-02 15:47:18 +0000
102+++ tests/expr.py 2009-06-18 15:20:10 +0000
103@@ -1937,8 +1937,10 @@
104
105 def test_compile_sequence(self):
106 expr = [elem1, Variable(1), (Variable(2), None)]
107- py_expr = compile_python(expr)
108- self.assertEquals(py_expr, "elem1, 1, 2, None")
109+ state = State()
110+ py_expr = compile_python(expr, state)
111+ self.assertEquals(py_expr, "elem1, _0, _1, None")
112+ self.assertEquals(state.parameters, [1, 2])
113
114 def test_compile_invalid(self):
115 self.assertRaises(CompileError, compile_python, object())
116@@ -1965,8 +1967,10 @@
117 self.assertEquals(py_expr, "1L")
118
119 def test_bool(self):
120- py_expr = compile_python(True)
121- self.assertEquals(py_expr, "True")
122+ state = State()
123+ py_expr = compile_python(True, state)
124+ self.assertEquals(py_expr, "_0")
125+ self.assertEquals(state.parameters, [True])
126
127 def test_float(self):
128 py_expr = compile_python(1.1)
129@@ -1974,23 +1978,31 @@
130
131 def test_datetime(self):
132 dt = datetime(1977, 5, 4, 12, 34)
133- py_expr = compile_python(dt)
134- self.assertEquals(py_expr, repr(dt))
135+ state = State()
136+ py_expr = compile_python(dt, state)
137+ self.assertEquals(py_expr, "_0")
138+ self.assertEquals(state.parameters, [dt])
139
140 def test_date(self):
141 d = date(1977, 5, 4)
142- py_expr = compile_python(d)
143- self.assertEquals(py_expr, repr(d))
144+ state = State()
145+ py_expr = compile_python(d, state)
146+ self.assertEquals(py_expr, "_0")
147+ self.assertEquals(state.parameters, [d])
148
149 def test_time(self):
150 t = time(12, 34)
151- py_expr = compile_python(t)
152- self.assertEquals(py_expr, repr(t))
153+ state = State()
154+ py_expr = compile_python(t, state)
155+ self.assertEquals(py_expr, "_0")
156+ self.assertEquals(state.parameters, [t])
157
158 def test_timedelta(self):
159 td = timedelta(days=1, seconds=2, microseconds=3)
160- py_expr = compile_python(td)
161- self.assertEquals(py_expr, repr(td))
162+ state = State()
163+ py_expr = compile_python(td, state)
164+ self.assertEquals(py_expr, "_0")
165+ self.assertEquals(state.parameters, [td])
166
167 def test_none(self):
168 py_expr = compile_python(None)
169@@ -1998,63 +2010,87 @@
170
171 def test_column(self):
172 expr = Column(column1)
173- py_expr = compile_python(expr)
174- self.assertEquals(py_expr, "get_column('column1')")
175+ state = State()
176+ py_expr = compile_python(expr, state)
177+ self.assertEquals(py_expr, "get_column(_0)")
178+ self.assertEquals(state.parameters, [expr])
179
180 def test_column_table(self):
181 expr = Column(column1, table1)
182- py_expr = compile_python(expr)
183- self.assertEquals(py_expr, "get_column('column1')")
184+ state = State()
185+ py_expr = compile_python(expr, state)
186+ self.assertEquals(py_expr, "get_column(_0)")
187+ self.assertEquals(state.parameters, [expr])
188
189 def test_variable(self):
190 expr = Variable("value")
191- py_expr = compile_python(expr)
192- self.assertEquals(py_expr, "'value'")
193+ state = State()
194+ py_expr = compile_python(expr, state)
195+ self.assertEquals(py_expr, "_0")
196+ self.assertEquals(state.parameters, ["value"])
197
198 def test_eq(self):
199 expr = Eq(Variable(1), Variable(2))
200- py_expr = compile_python(expr)
201- self.assertEquals(py_expr, "1 == 2")
202+ state = State()
203+ py_expr = compile_python(expr, state)
204+ self.assertEquals(py_expr, "_0 == _1")
205+ self.assertEquals(state.parameters, [1, 2])
206
207 def test_ne(self):
208 expr = Ne(Variable(1), Variable(2))
209- py_expr = compile_python(expr)
210- self.assertEquals(py_expr, "1 != 2")
211+ state = State()
212+ py_expr = compile_python(expr, state)
213+ self.assertEquals(py_expr, "_0 != _1")
214+ self.assertEquals(state.parameters, [1, 2])
215
216 def test_gt(self):
217 expr = Gt(Variable(1), Variable(2))
218- py_expr = compile_python(expr)
219- self.assertEquals(py_expr, "1 > 2")
220+ state = State()
221+ py_expr = compile_python(expr, state)
222+ self.assertEquals(py_expr, "_0 > _1")
223+ self.assertEquals(state.parameters, [1, 2])
224
225 def test_ge(self):
226 expr = Ge(Variable(1), Variable(2))
227- py_expr = compile_python(expr)
228- self.assertEquals(py_expr, "1 >= 2")
229+ state = State()
230+ py_expr = compile_python(expr, state)
231+ self.assertEquals(py_expr, "_0 >= _1")
232+ self.assertEquals(state.parameters, [1, 2])
233
234 def test_lt(self):
235 expr = Lt(Variable(1), Variable(2))
236- py_expr = compile_python(expr)
237- self.assertEquals(py_expr, "1 < 2")
238+ state = State()
239+ py_expr = compile_python(expr, state)
240+ self.assertEquals(py_expr, "_0 < _1")
241+ self.assertEquals(state.parameters, [1, 2])
242
243 def test_le(self):
244 expr = Le(Variable(1), Variable(2))
245- py_expr = compile_python(expr)
246- self.assertEquals(py_expr, "1 <= 2")
247+ state = State()
248+ py_expr = compile_python(expr, state)
249+ self.assertEquals(py_expr, "_0 <= _1")
250+ self.assertEquals(state.parameters, [1, 2])
251
252 def test_lshift(self):
253 expr = LShift(Variable(1), Variable(2))
254- py_expr = compile_python(expr)
255- self.assertEquals(py_expr, "1<<2")
256+ state = State()
257+ py_expr = compile_python(expr, state)
258+ self.assertEquals(py_expr, "_0<<_1")
259+ self.assertEquals(state.parameters, [1, 2])
260
261 def test_rshift(self):
262 expr = RShift(Variable(1), Variable(2))
263- py_expr = compile_python(expr)
264- self.assertEquals(py_expr, "1>>2")
265+ state = State()
266+ py_expr = compile_python(expr, state)
267+ self.assertEquals(py_expr, "_0>>_1")
268+ self.assertEquals(state.parameters, [1, 2])
269
270 def test_in(self):
271 expr = In(Variable(1), Variable(2))
272- py_expr = compile_python(expr)
273- self.assertEquals(py_expr, "1 in (2,)")
274+ state = State()
275+ py_expr = compile_python(expr, state)
276+ self.assertEquals(py_expr, "_0 in (_1,)")
277+ self.assertEquals(state.parameters, [1, 2])
278
279 def test_and(self):
280 expr = And(elem1, elem2, And(elem3, elem4))
281@@ -2109,8 +2145,20 @@
282
283 match = compile_python.get_matcher((col1 > 10) & (col2 < 10))
284
285- self.assertTrue(match({"column1": 15, "column2": 5}.get))
286- self.assertFalse(match({"column1": 5, "column2": 15}.get))
287+ self.assertTrue(match({col1: 15, col2: 5}.get))
288+ self.assertFalse(match({col1: 5, col2: 15}.get))
289+
290+ def test_match_bad_repr(self):
291+ """The get_matcher() works for expressions containing values
292+ whose repr is not valid Python syntax."""
293+ class BadRepr(object):
294+ def __repr__(self):
295+ return "$Not a valid Python expression$"
296+
297+ value = BadRepr()
298+ col1 = Column(column1)
299+ match = compile_python.get_matcher(col1 == Variable(value))
300+ self.assertTrue(match({col1: value}.get))
301
302
303 class LazyValueExprTest(TestHelper):

Subscribers

People subscribed via source and target branches

to status/vote changes: