mapper - pks_by_table should store keys in order even tho we dont have a failure...

lazyloader can lazyload using mapper.get() if it is appropriate, saves a lot of queries
a few more assertions in properties in prep for relations against select statement mappers
mapper get() clause is determined upfront to avoid re-generating it

diff --git a/lib/sqlalchemy/mapping/mapper.py b/lib/sqlalchemy/mapping/mapper.py
index 78bc8ee708ccf533f42cc5229f2762ce6022b0c9..ee8f8ddc76f32390ddcbbdf50f903bedd71e7931 100644 (file)
--- a/lib/sqlalchemy/mapping/mapper.py
+++ b/lib/sqlalchemy/mapping/mapper.py
@@ -87,16 +87,16 @@ class Mapper(object):
         self.pks_by_table = {}
         if primary_key is not None:
             for k in primary_key:
-                self.pks_by_table.setdefault(k.table, util.HashSet()).append(k)
+                self.pks_by_table.setdefault(k.table, util.HashSet(ordered=True)).append(k)
                 if k.table != self.table:
                     # associate pk cols from subtables to the "main" table
-                    self.pks_by_table.setdefault(self.table, util.HashSet()).append(k)
+                    self.pks_by_table.setdefault(self.table, util.HashSet(ordered=True)).append(k)
         else:
             for t in self.tables + [self.table]:
                 try:
                     l = self.pks_by_table[t]
                 except KeyError:
-                    l = self.pks_by_table.setdefault(t, util.HashSet())
+                    l = self.pks_by_table.setdefault(t, util.HashSet(ordered=True))
                 if not len(t.primary_key):
                     raise "Table " + t.name + " has no primary key columns. Specify primary_key argument to mapper."
                 for k in t.primary_key:
@@ -153,6 +153,10 @@ class Mapper(object):
             proplist = self.columntoproperty.setdefault(column.original, [])
             proplist.append(prop)
 
+        self._get_clause = sql.and_()
+        for primary_key in self.pks_by_table[self.primarytable]:
+            self._get_clause.clauses.append(primary_key == sql.bindparam("pk_"+primary_key.key))
+
         if (
                 (not hasattr(self.class_, '_mapper') or not mapper_registry.has_key(self.class_._mapper))
                 or self.is_primary 
@@ -254,15 +258,14 @@ class Mapper(object):
         try:
             return objectstore.uow()._get(key)
         except KeyError:
-            clause = sql.and_()
             i = 0
+            params = {}
             for primary_key in self.pks_by_table[self.primarytable]:
-                # appending to the and_'s clause list directly to skip
-                # typechecks etc.
-                clause.clauses.append(primary_key == ident[i])
+                params["pk_"+primary_key.key] = ident[i]
                 i += 1
+            print str(self._get_clause)
             try:
-                return self.select(clause)[0]
+                return self.select(self._get_clause, params=params)[0]
             except IndexError:
                 return None
 

diff --git a/lib/sqlalchemy/mapping/properties.py b/lib/sqlalchemy/mapping/properties.py
index 8cdf5aa95e12c2737e74a00801385a435cd3a1a1..2e1b7929d96c22a8c58164330ba7d7aaa75b0643 100644 (file)
--- a/lib/sqlalchemy/mapping/properties.py
+++ b/lib/sqlalchemy/mapping/properties.py
@@ -235,7 +235,9 @@ class PropertyLoader(MapperProperty):
             return PropertyLoader.LEFT
         elif self.foreignkey.table == self.parent.primarytable:
             return PropertyLoader.RIGHT
-
+        else:
+            raise "Cant determine relation direction"
+            
     def _find_dependent(self):
         dependent = [None]
         def foo(binary):
@@ -573,7 +575,10 @@ class LazyLoader(PropertyLoader):
     def init(self, key, parent):
         PropertyLoader.init(self, key, parent)
         (self.lazywhere, self.lazybinds) = create_lazy_clause(self.parent.table, self.primaryjoin, self.secondaryjoin, self.foreignkey)
-
+        # determine if our "lazywhere" clause is the same as the mapper's
+        # get() clause.  then we can just use mapper.get()
+        self.use_get = not self.uselist and self.mapper._get_clause.compare(self.lazywhere)
+        
     def _set_class_attribute(self, class_, key):
         # establish a class-level lazy loader on our class
         #print "SETCLASSATTR LAZY", repr(class_), key
@@ -590,7 +595,14 @@ class LazyLoader(PropertyLoader):
                     allparams = False
                     break
             if allparams:
-                if self.order_by is not False:
+                # if we have a simple straight-primary key load, use mapper.get()
+                # to possibly save a DB round trip
+                if self.use_get:
+                    ident = []
+                    for primary_key in self.mapper.pks_by_table[self.mapper.primarytable]:
+                        ident.append(params[self.mapper.primarytable.name + "_" + primary_key.key])
+                    return self.mapper.get(*ident)
+                elif self.order_by is not False:
                     order_by = self.order_by
                 elif self.secondary is not None and self.secondary.default_order_by() is not None:
                     order_by = self.secondary.default_order_by()
@@ -630,12 +642,12 @@ def create_lazy_clause(table, primaryjoin, secondaryjoin, foreignkey):
         circular = isinstance(binary.left, schema.Column) and isinstance(binary.right, schema.Column) and binary.left.table is binary.right.table
         if isinstance(binary.left, schema.Column) and ((not circular and binary.left.table is table) or (circular and foreignkey is binary.right)):
             binary.left = binds.setdefault(binary.left,
-                    sql.BindParamClause(table.name + "_" + binary.left.name, None, shortname = binary.left.name))
+                    sql.BindParamClause(binary.right.table.name + "_" + binary.right.name, None, shortname = binary.left.name))
             binary.swap()
 
         if isinstance(binary.right, schema.Column) and ((not circular and binary.right.table is table) or (circular and foreignkey is binary.left)):
             binary.right = binds.setdefault(binary.right,
-                    sql.BindParamClause(table.name + "_" + binary.right.name, None, shortname = binary.right.name))
+                    sql.BindParamClause(binary.left.table.name + "_" + binary.left.name, None, shortname = binary.right.name))
                     
     if secondaryjoin is not None:
         lazywhere = sql.and_(primaryjoin, secondaryjoin)
@@ -660,7 +672,10 @@ class EagerLoader(PropertyLoader):
         [self.to_alias.append(f) for f in self.primaryjoin._get_from_objects()]
         if self.secondaryjoin is not None:
             [self.to_alias.append(f) for f in self.secondaryjoin._get_from_objects()]
-        del self.to_alias[parent.primarytable]
+        try:
+            del self.to_alias[parent.primarytable]
+        except KeyError:
+            pass
     
         # if this eagermapper is to select using an "alias" to isolate it from other
         # eager mappers against the same table, we have to redefine our secondary