launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/better-bulk into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/better-bulk/+merge/93772

This branch extends some database infrastructure to support additional bulk operations.

 - A BulkInsert Storm expression has been added. This is stolen from lp:~wgrant/storm/bulk-insert until that lands, to support bulk and query inserts.
 - lp.services.database.bulk.load now supports compound primary keys.
-- 
https://code.launchpad.net/~wgrant/launchpad/better-bulk/+merge/93772
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/better-bulk into lp:launchpad.

=== modified file 'lib/lp/services/database/bulk.py'
--- lib/lp/services/database/bulk.py	2011-12-21 14:58:31 +0000
+++ lib/lp/services/database/bulk.py	2012-02-20 07:20:26 +0000
@@ -16,7 +16,10 @@
 from functools import partial
 from operator import attrgetter
 
-from storm.expr import Or
+from storm.expr import (
+    And,
+    Or,
+    )
 from storm.info import get_cls_info
 from storm.store import Store
 from zope.security.proxy import removeSecurityProxy
@@ -67,28 +70,35 @@
         list(query)
 
 
-def _primary_key(object_type):
+def _primary_key(object_type, allow_compound=False):
     """Get a primary key our helpers can use.
-    
+
     :raises AssertionError if the key is missing or unusable.
     """
     primary_key = get_cls_info(object_type).primary_key
-    if len(primary_key) != 1:
-        raise AssertionError(
-            "Compound primary keys are not supported: %s." %
-            object_type.__name__)
-    return primary_key[0]
+    if len(primary_key) == 1:
+        return primary_key[0]
+    else:
+        if not allow_compound:
+            raise AssertionError(
+                "Compound primary keys are not supported: %s." %
+                object_type.__name__)
+        return primary_key
 
 
 def load(object_type, primary_keys, store=None):
     """Load a large number of objects efficiently."""
-    primary_key = _primary_key(object_type)
-    primary_key_column = primary_key
+    primary_key = _primary_key(object_type, allow_compound=True)
     primary_keys = set(primary_keys)
     primary_keys.discard(None)
     if not primary_keys:
         return []
-    condition = primary_key_column.is_in(primary_keys)
+    if isinstance(primary_key, tuple):
+        condition = Or(*(
+            And(*(key == value for (key, value) in zip(primary_key, values)))
+            for values in primary_keys))
+    else:
+        condition = primary_key.is_in(primary_keys)
     if store is None:
         store = IStore(object_type)
     return list(store.find(object_type, condition))
@@ -96,7 +106,7 @@
 
 def load_referencing(object_type, owning_objects, reference_keys):
     """Load objects of object_type that reference owning_objects.
-    
+
     Note that complex types like Person are best loaded through dedicated
     helpers that can eager load other related things (e.g. validity for
     Person).

=== modified file 'lib/lp/services/database/stormexpr.py'
--- lib/lp/services/database/stormexpr.py	2011-02-04 09:07:36 +0000
+++ lib/lp/services/database/stormexpr.py	2012-02-20 07:20:26 +0000
@@ -3,6 +3,7 @@
 
 __metaclass__ = type
 __all__ = [
+    'BulkInsert',
     'Concatenate',
     'CountDistinct',
     'Greatest',
@@ -10,10 +11,14 @@
 
 from storm.expr import (
     BinaryOper,
+    build_tables,
+    COLUMN_NAME,
     compile,
     EXPR,
     Expr,
     NamedFunc,
+    TABLE,
+    Undef,
     )
 
 
@@ -48,3 +53,57 @@
     """Storm operator for string concatenation."""
     __slots__ = ()
     oper = " || "
+
+
+class BulkInsert(Expr):
+    """Expression representing an insert statement.
+
+    This is storm.expr.Insert from lp:~wgrant/launchpad/bulk-insert,
+    duplicated here until the Storm branch is merged.
+
+    @ivar map: Dictionary mapping columns to values, or a sequence of columns
+        for a bulk insert.
+    @ivar table: Table where the row should be inserted.
+    @ivar default_table: Table to use if no table is explicitly provided, and
+        no tables may be inferred from provided columns.
+    @ivar primary_columns: Tuple of columns forming the primary key of the
+        table where the row will be inserted.  This is a hint used by backends
+        to process the insertion of rows.
+    @ivar primary_variables: Tuple of variables with values for the primary
+        key of the table where the row will be inserted.  This is a hint used
+        by backends to process the insertion of rows.
+    @ivar expr: Expression or sequence of tuples of values for bulk insertion.
+    """
+    __slots__ = ("map", "table", "default_table", "primary_columns",
+                 "primary_variables", "expr")
+
+    def __init__(self, map, table=Undef, default_table=Undef,
+                 primary_columns=Undef, primary_variables=Undef,
+                 expr=Undef):
+        self.map = map
+        self.table = table
+        self.default_table = default_table
+        self.primary_columns = primary_columns
+        self.primary_variables = primary_variables
+        self.expr = expr
+
+
+@compile.when(BulkInsert)
+def compile_bulkinsert(compile, insert, state):
+    state.push("context", COLUMN_NAME)
+    columns = compile(tuple(insert.map), state, token=True)
+    state.context = TABLE
+    table = build_tables(compile, insert.table, insert.default_table, state)
+    state.context = EXPR
+    expr = insert.expr
+    if expr is Undef:
+        expr = [tuple(insert.map.itervalues())]
+    if isinstance(expr, Expr):
+        compiled_expr = compile(expr, state)
+    else:
+        compiled_expr = (
+            "VALUES (%s)" %
+            "), (".join(compile(values, state) for values in expr))
+    state.pop()
+    return "".join(
+        ["INSERT INTO ", table, " (", columns, ") ", compiled_expr])

=== modified file 'lib/lp/services/database/tests/test_bulk.py'
--- lib/lp/services/database/tests/test_bulk.py	2012-01-01 02:58:52 +0000
+++ lib/lp/services/database/tests/test_bulk.py	2012-02-20 07:20:26 +0000
@@ -23,6 +23,10 @@
     ISlaveStore,
     IStore,
     )
+from lp.services.features.model import (
+    FeatureFlag,
+    getFeatureStore,
+    )
 from lp.soyuz.model.component import Component
 from lp.testing import (
     TestCase,
@@ -170,10 +174,17 @@
 
     def test_load_with_compound_primary_keys(self):
         # load() does not like compound primary keys.
-        self.assertRaisesWithContent(
-            AssertionError,
-            'Compound primary keys are not supported: BugAffectsPerson.',
-            bulk.load, BugAffectsPerson, [])
+        flags = [
+            FeatureFlag(u'foo', 0, u'bar', u'true'),
+            FeatureFlag(u'foo', 0, u'baz', u'false'),
+            ]
+        other_flag = FeatureFlag(u'notfoo', 0, u'notbar', u'true')
+        for flag in flags + [other_flag]:
+            getFeatureStore().add(flag)
+
+        self.assertContentEqual(
+            flags,
+            bulk.load(FeatureFlag, [(ff.scope, ff.flag) for ff in flags]))
 
     def test_load_with_store(self):
         # load() can use an alternative store.

=== added file 'lib/lp/services/database/tests/test_stormexpr.py'
--- lib/lp/services/database/tests/test_stormexpr.py	1970-01-01 00:00:00 +0000
+++ lib/lp/services/database/tests/test_stormexpr.py	2012-02-20 07:20:26 +0000
@@ -0,0 +1,55 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from storm.expr import (
+    Column,
+    compile,
+    Select,
+    SQLToken,
+    State,
+    )
+from testtools import TestCase
+
+from lp.services.database.stormexpr import BulkInsert
+
+
+# Create columnN, tableN, and elemN variables.
+# Stolen from Storm's tests.expr.
+for i in range(10):
+    for name in ["column", "elem"]:
+        exec "%s%d = SQLToken('%s%d')" % (name, i, name, i)
+    for name in ["table"]:
+        exec "%s%d = '%s %d'" % (name, i, name, i)
+
+
+class TestBulkInsert(TestCase):
+    """Storm Insert expression which supports bulk operations.
+
+    These are parts of tests.expr from lp:~wgrant/launchpad/bulk-insert,
+    duplicated here until the Storm branch is merged.
+    """
+
+    def test_insert_bulk(self):
+        expr = BulkInsert(
+            (Column(column1, table1), Column(column2, table1)),
+            expr=[(elem1, elem2), (elem3, elem4)])
+        state = State()
+        statement = compile(expr, state)
+        self.assertEquals(
+            statement,
+            'INSERT INTO "table 1" (column1, column2) '
+            'VALUES (elem1, elem2), (elem3, elem4)')
+        self.assertEquals(state.parameters, [])
+
+    def test_insert_select(self):
+        expr = BulkInsert(
+            (Column(column1, table1), Column(column2, table1)),
+            expr=Select((Column(column3, table3), Column(column4, table4))))
+        state = State()
+        statement = compile(expr, state)
+        self.assertEquals(
+            statement,
+            'INSERT INTO "table 1" (column1, column2) '
+            '(SELECT "table 3".column3, "table 4".column4 '
+            'FROM "table 3", "table 4")')
+        self.assertEquals(state.parameters, [])