launchpad-reviewers team mailing list archive

[Abel Deuring <abel.deuring@xxxxxxxxxxxxx>]

Abel Deuring has proposed merging lp:~adeuring/launchpad/bug-739052-8 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~adeuring/launchpad/bug-739052-8/+merge/75372

This branch is the stap preparatory step required to use StormRangeFactory. (At least I hope it...)

It increases the version number of lazr.batchnavigator to 1.2.10 (which I released and included in download-cache today.)

It also adds a new property StormRangeFactory.rough_length, which run an "EXPLAIN SELECT ..." query, as a surrogate of a sometimes very expensive "SELECT count(*)..." query.

lazr.batchnavigator uses this property only to display the last number in "6 -> 10 of 123 results", i.e., the exact value is not very important. (We might though consider to change the text to "6 -> 10 of approximately 123 results".)

The tests test_rough_length() and test_rough_length_first_sort_column_desc() would pass too with an assertEqual(5, estimated_length) (5 is the exact length of the result set), but the third test, test_rough_length_decorated_result_set(), is an example that rough_length is indeed not always excat: It return 4, while the real result length is 5.

test: ./bin/test canonical -vvt canonical.launchpad.webapp.tests.test_batching

no lint

-- 
https://code.launchpad.net/~adeuring/launchpad/bug-739052-8/+merge/75372
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~adeuring/launchpad/bug-739052-8 into lp:launchpad.

=== modified file 'lib/canonical/launchpad/webapp/batching.py'
--- lib/canonical/launchpad/webapp/batching.py	2011-08-25 15:13:59 +0000
+++ lib/canonical/launchpad/webapp/batching.py	2011-09-14 15:54:12 +0000
@@ -12,6 +12,7 @@
 import lazr.batchnavigator
 from lazr.batchnavigator.interfaces import IRangeFactory
 from operator import isSequenceType
+import re
 import simplejson
 from storm import Undef
 from storm.expr import (
@@ -23,7 +24,10 @@
     )
 from storm.properties import PropertyColumn
 from storm.zope.interfaces import IResultSet
-from zope.component import adapts
+from zope.component import (
+    adapts,
+    getUtility,
+    )
 from zope.interface import implements
 from zope.interface.common.sequence import IFiniteSequence
 from zope.security.proxy import (
@@ -33,15 +37,23 @@
     )
 
 from canonical.config import config
-from canonical.database.sqlbase import sqlvalues
+from canonical.database.sqlbase import (
+    convert_storm_clause_to_string,
+    sqlvalues,
+    )
+
 from canonical.launchpad.components.decoratedresultset import (
     DecoratedResultSet,
     )
 from canonical.launchpad.webapp.interfaces import (
+    IStoreSelector,
+    MAIN_STORE,
+    SLAVE_FLAVOR,
+    StormRangeFactoryError,
     ITableBatchNavigator,
-    StormRangeFactoryError,
     )
 from canonical.launchpad.webapp.publisher import LaunchpadView
+from lp.services.propertycache import cachedproperty
 
 
 class FiniteSequenceAdapter:
@@ -559,3 +571,25 @@
         """See `IRangeFactory."""
         sliced = self.resultset[start:end]
         return ShadowedList(list(sliced), list(sliced.get_plain_result_set()))
+
+    @cachedproperty
+    def rough_length(self):
+        """See `IRangeFactory."""
+        # get_select_expr() requires at least one column as a parameter.
+        # getorderBy() already knows about columns that can appear
+        # in the result set, let's take just the first one.
+        column = self.getOrderBy()[0]
+        if zope_isinstance(column, Desc):
+            column = column.expr
+        select = removeSecurityProxy(self.plain_resultset).get_select_expr(
+            column)
+        explain = 'EXPLAIN ' + convert_storm_clause_to_string(select)
+        store = getUtility(IStoreSelector).get(MAIN_STORE, SLAVE_FLAVOR)
+        result = store.execute(explain)
+        _rows_re = re.compile("rows=(\d+)\swidth=")
+        first_line = result.get_one()[0]
+        match = _rows_re.search(first_line)
+        if match is None:
+            raise RuntimeError(
+                "Unexpected EXPLAIN output %s" % repr(first_line))
+        return int(match.group(1))

=== modified file 'lib/canonical/launchpad/webapp/tests/test_batching.py'
--- lib/canonical/launchpad/webapp/tests/test_batching.py	2011-08-25 15:13:59 +0000
+++ lib/canonical/launchpad/webapp/tests/test_batching.py	2011-09-14 15:54:12 +0000
@@ -12,6 +12,10 @@
     compile,
     Desc,
     )
+from testtools.matchers import (
+    LessThan,
+    Not,
+    )
 from zope.security.proxy import isinstance as zope_isinstance
 
 from canonical.launchpad.components.decoratedresultset import (
@@ -715,3 +719,31 @@
         self.assertEqual(2, shadow_list[0])
         self.assertEqual(0, shadow_list.shadow_values[-1])
         self.assertEqual(2, shadow_list.shadow_values[0])
+
+    def test_rough_length(self):
+        # StormRangeFactory.rough_length returns an estimate of the
+        # length of the result set.
+        resultset = self.makeStormResultSet()
+        resultset.order_by(Person.id)
+        range_factory = StormRangeFactory(resultset)
+        estimated_length = range_factory.rough_length
+        self.assertThat(estimated_length, LessThan(10))
+        self.assertThat(estimated_length, Not(LessThan(1)))
+
+    def test_rough_length_first_sort_column_desc(self):
+        # StormRangeFactory.rough_length can handle result sets where
+        # the first sort column has descendig order.
+        resultset = self.makeStormResultSet()
+        resultset.order_by(Desc(Person.id))
+        range_factory = StormRangeFactory(resultset)
+        estimated_length = range_factory.rough_length
+        self.assertThat(estimated_length, LessThan(10))
+        self.assertThat(estimated_length, Not(LessThan(1)))
+
+    def test_rough_length_decorated_result_set(self):
+        # StormRangeFactory.rough_length can handle DecoratedResultSets.
+        resultset = self.makeDecoratedStormResultSet()
+        range_factory = StormRangeFactory(resultset)
+        estimated_length = range_factory.rough_length
+        self.assertThat(estimated_length, LessThan(10))
+        self.assertThat(estimated_length, Not(LessThan(1)))

=== modified file 'versions.cfg'
--- versions.cfg	2011-09-14 11:08:33 +0000
+++ versions.cfg	2011-09-14 15:54:12 +0000
@@ -30,7 +30,7 @@
 launchpadlib = 1.9.9
 lazr.amqp = 0.1
 lazr.authentication = 0.1.1
-lazr.batchnavigator = 1.2.9
+lazr.batchnavigator = 1.2.10
 lazr.config = 1.1.3
 lazr.delegates = 1.2.0
 lazr.enum = 1.1.3