Storm

Merge lp:~jkakar/storm/is-empty-strips-order-by into lp:storm

  1. is-empty-strips-order-by
  2. Merge into trunk
Proposed by Jamu Kakar
Status: Merged
Approved by: Jamu Kakar
Approved revision: 362
Merged at revision: 362
Proposed branch: lp:~jkakar/storm/is-empty-strips-order-by
Merge into: lp:storm
Diff against target: 52 lines (+23/-1)
2 files modified
storm/store.py (+1/-0)
tests/store/base.py (+22/-1)
To merge this branch: bzr merge lp:~jkakar/storm/is-empty-strips-order-by
Reviewer Review Type Date Requested Status
Kevin McDermott (community) Approve
Robert Collins (community) Approve
Storm Developers Pending
Review via email: mp+30677@code.launchpad.net

Description of the change

This branch introduces the following changes:

- ResultSet.is_empty strips order by clauses, since they're
  unnecessary for this operation.

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

Looks good except for the typo ;)

review: Approve
lp:~jkakar/storm/is-empty-strips-order-by updated
362. By Jamu Kakar

- Fix a typo in a docstring.
- Unpack the list of captured statements to ensure there is only
  one.

Revision history for this message
Kevin McDermott (bigkevmcd) wrote :

As discussed "ORDER BY" could not appear in a whole load of SQL statements, can we check that there's only one query executed, and that "ORDER BY" doesn't appear.

Oh, and as Robert points out "natching" ;-)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'storm/store.py'
2--- storm/store.py 2010-02-10 11:29:39 +0000
3+++ storm/store.py 2010-07-22 16:18:43 +0000
4@@ -1055,6 +1055,7 @@
5 """Return true if this L{ResultSet} contains no results."""
6 subselect = self._get_select()
7 subselect.limit = 1
8+ subselect.order_by = Undef
9 select = Select(1, tables=Alias(subselect, "_tmp"), limit=1)
10 result = self._store._connection.execute(select)
11 return (not result.get_one())
12
13=== modified file 'tests/store/base.py'
14--- tests/store/base.py 2010-06-01 08:31:53 +0000
15+++ tests/store/base.py 2010-07-22 16:18:43 +0000
16@@ -30,7 +30,8 @@
17 from storm.properties import PropertyPublisherMeta, Decimal
18 from storm.variables import PickleVariable
19 from storm.expr import (
20- Asc, Desc, Select, LeftJoin, SQL, Count, Sum, Avg, And, Or, Eq, Lower)
21+ Asc, Desc, Select, LeftJoin, SQL, Count, Sum, Avg, And, Or, Eq, Lower,
22+ compile)
23 from storm.variables import Variable, UnicodeVariable, IntVariable
24 from storm.info import get_obj_info, ClassAlias
25 from storm.exceptions import (
26@@ -513,6 +514,26 @@
27 result = self.store.find(Foo, id=30)
28 self.assertEquals(result.is_empty(), False)
29
30+ def test_wb_is_empty_strips_order_by(self):
31+ """
32+ L{ResultSet.is_empty} strips the C{ORDER BY} clause, if one is
33+ present, since it isn't required to actually determine if a result set
34+ has any matching rows. This should provide some performance
35+ improvement when the ordered result set would be large.
36+ """
37+ statements = []
38+ original_execute = self.store._connection.execute
39+ def execute(expr):
40+ statements.append(compile(expr))
41+ return original_execute(expr)
42+ self.store._connection.execute = execute
43+
44+ result = self.store.find(Foo, Foo.id == 300)
45+ result.order_by(Foo.id)
46+ self.assertEqual(True, result.is_empty())
47+ [statement] = statements
48+ self.assertNotIn("ORDER BY", statement)
49+
50 def test_is_empty_with_composed_key(self):
51 result = self.store.find(Link, foo_id=300, bar_id=3000)
52 self.assertEquals(result.is_empty(), True)

Subscribers

People subscribed via source and target branches

to status/vote changes: