refactor/cleanup to mapper options methodology to allow for incoming defer/undefer options

mapper/relations are stricter about class attributes and primary mapper - is_primary flag
on relations fixed (wasnt working before). new primary mappers clear off old class attributes,
secondary mappers insure that their property was set up by the primary; otherwise secondary
mappers can add behavior to properties that are unmanaged by the primary mapper
added "group" option to deferred loaders so a group of properties can be loaded at once
mapper adds the "oid" column to the select list if "distinct" is set to true and its
using the default "order by oid" ordering (mysql benefits from ansisql fix to only print out unique
columns in the select list since its oid is the same as the pk column)
fixed unittests to comply with stricter primary mapper rules

4 files changed, 94 insertions, 33 deletions

diff --git a/lib/sqlalchemy/attributes.py b/lib/sqlalchemy/attributes.py
index ed5da3284..2cc617612 100644
--- a/lib/sqlalchemy/attributes.py
+++ b/lib/sqlalchemy/attributes.py
@@ -374,7 +374,21 @@ class AttributeManager(object):
             class_._attribute_manager = self
         return attr
 
+    def reset_class_managed(self, class_):
+        try:
+            attr = getattr(class_, '_class_managed_attributes')
+            for key in attr.keys():
+                delattr(class_, key)
+            delattr(class_, '_class_managed_attributes')
+        except AttributeError:
+            pass
 
+    def is_class_managed(self, class_, key):
+        try:
+            return class_._class_managed_attributes.has_key(key)
+        except AttributeError:
+            return False
+            
     def create_history_container(self, obj, key, uselist, callable_ = None, **kwargs):
         """creates a new history container for the given attribute on the given object."""
         if callable_ is not None:
diff --git a/lib/sqlalchemy/mapping/__init__.py b/lib/sqlalchemy/mapping/__init__.py
index 2cab0b0da..3217e283e 100644
--- a/lib/sqlalchemy/mapping/__init__.py
+++ b/lib/sqlalchemy/mapping/__init__.py
@@ -63,11 +63,11 @@ def _relation_mapper(class_, table=None, secondary=None,
                     live=live, association=association, lazy=lazy, 
                     selectalias=selectalias, order_by=order_by, attributeext=attributeext)
 
-def column(*columns):
-    return ColumnProperty(*columns)
+def column(*columns, **kwargs):
+    return ColumnProperty(*columns, **kwargs)
     
-def deferred(*columns):
-    return DeferredColumnProperty(*columns)
+def deferred(*columns, **kwargs):
+    return DeferredColumnProperty(*columns, **kwargs)
     
     
 class assignmapper(object):
diff --git a/lib/sqlalchemy/mapping/mapper.py b/lib/sqlalchemy/mapping/mapper.py
index 451967374..9f7cc7177 100644
--- a/lib/sqlalchemy/mapping/mapper.py
+++ b/lib/sqlalchemy/mapping/mapper.py
@@ -154,15 +154,16 @@ class Mapper(object):
             proplist = self.columntoproperty.setdefault(column.original, [])
             proplist.append(prop)
 
+        if not hasattr(self.class_, '_mapper') or self.is_primary or not mapper_registry.has_key(self.class_._mapper) or (inherits is not None and inherits._is_primary_mapper()):
+            objectstore.global_attributes.reset_class_managed(self.class_)
+            self._init_class()
+            
         if inherits is not None:
             for key, prop in inherits.props.iteritems():
                 if not self.props.has_key(key):
                     self.props[key] = prop._copy()
                 
-        if not hasattr(self.class_, '_mapper') or self.is_primary or not mapper_registry.has_key(self.class_._mapper) or (inherits is not None and inherits._is_primary_mapper()):
-            self._init_class()
-       
-        
+
     engines = property(lambda s: [t.engine for t in s.tables])
 
     def add_property(self, key, prop):
@@ -275,6 +276,17 @@ class Mapper(object):
         compiling or executing it"""
         return self._compile(whereclause, **options)
 
+    def copy(self, hashkey=None):
+        # TODO: at the moment, we are re-using the properties from the original mapper
+        # which stay connected to that first mapper.  if we start making copies of 
+        # mappers where the primary attributes of the mapper change, we might want 
+        # to look into copying all the property objects too.
+        if hashkey is None:
+            hashkey = hash_key(self) + "->copy" 
+        mapper = Mapper(hashkey, **self.copyargs)
+        mapper._init_properties()
+        return mapper
+        
     def options(self, *options):
         """uses this mapper as a prototype for a new mapper with different behavior.
         *options is a list of options directives, which include eagerload(), lazyload(), and noload()"""
@@ -283,8 +295,7 @@ class Mapper(object):
         try:
             return mapper_registry[hashkey]
         except KeyError:
-            mapper = Mapper(hashkey, **self.copyargs)
-            mapper._init_properties()
+            mapper = self.copy(hashkey)
 
             for option in options:
                 option.process(mapper)
@@ -563,10 +574,18 @@ class Mapper(object):
                 statement.order_by(order_by)
         else:
             statement = sql.select([], whereclause, from_obj=[self.table], use_labels=True, **kwargs)
-            if not kwargs.get('distinct', False) and order_by is not None and kwargs.get('order_by', None) is None:
+            if order_by is not None and kwargs.get('order_by', None) is None:
                 statement.order_by(order_by)
+            # for a DISTINCT query, you need the columns explicitly specified in order
+            # to use it in "order_by" - in the case we added the rowid column in,
+            # add that to the column list
+            # TODO: this idea should be handled by the SELECT statement itself, insuring
+            # that order_by cols are in the select list if DISTINCT is selected
+            if kwargs.get('distinct', False) and order_by is self.table.rowid_column:
+                statement.append_column(self.table.rowid_column)
         # plugin point
         
+            
         # give all the attached properties a chance to modify the query
         for key, value in self.props.iteritems():
             value.setup(key, statement, **kwargs) 
diff --git a/lib/sqlalchemy/mapping/properties.py b/lib/sqlalchemy/mapping/properties.py
index 922aab453..a0ac84dc4 100644
--- a/lib/sqlalchemy/mapping/properties.py
+++ b/lib/sqlalchemy/mapping/properties.py
@@ -67,7 +67,7 @@ class DeferredColumnProperty(ColumnProperty):
     will "lazy load" its value from the table.  this is per-column lazy loading."""
 
     def __init__(self, *columns, **kwargs):
-        self.isoption = kwargs.get('isoption', False)
+        self.group = kwargs.get('group', None)
         ColumnProperty.__init__(self, *columns)
     
     def hash_key(self):
@@ -84,14 +84,25 @@ class DeferredColumnProperty(ColumnProperty):
                 if not attr:
                     return None
                 clause.clauses.append(primary_key == attr)
-            return sql.select([self.parent.table.c[self.key]], clause).scalar()
+            
+            if self.group is not None:
+                groupcols = [p for p in self.parent.props.values() if isinstance(p, DeferredColumnProperty) and p.group==self.group]
+                row = sql.select([g.columns[0] for g in groupcols], clause).execute().fetchone()
+                for prop in groupcols:
+                    if prop is self:
+                        continue
+                    instance.__dict__[prop.key] = row[prop.columns[0]]
+                    objectstore.global_attributes.create_history(instance, prop.key, uselist=False)
+                return row[self.columns[0]]    
+            else:
+                return sql.select([self.columns[0]], clause).scalar()
         return lazyload
 
     def _is_primary(self):
         """a return value of True indicates we are the primary MapperProperty for this loader's
         attribute on our mapper's class.  It means we can set the object's attribute behavior
         at the class level.  otherwise we have to set attribute behavior on a per-instance level."""
-        return self.parent._is_primary_mapper and not self.isoption
+        return self.parent._is_primary_mapper()
 
     def setup(self, key, statement, **options):
         pass
@@ -120,7 +131,7 @@ class PropertyLoader(MapperProperty):
 
     """describes an object property that holds a single item or list of items that correspond
     to a related database table."""
-    def __init__(self, argument, secondary, primaryjoin, secondaryjoin, foreignkey=None, uselist=None, private=False, live=False, isoption=False, association=None, selectalias=None, order_by=None, attributeext=None, backref=None, is_backref=False):
+    def __init__(self, argument, secondary, primaryjoin, secondaryjoin, foreignkey=None, uselist=None, private=False, live=False, association=None, selectalias=None, order_by=None, attributeext=None, backref=None, is_backref=False):
         self.uselist = uselist
         self.argument = argument
         self.secondary = secondary
@@ -129,7 +140,6 @@ class PropertyLoader(MapperProperty):
         self.foreignkey = foreignkey
         self.private = private
         self.live = live
-        self.isoption = isoption
         self.association = association
         self.selectalias = selectalias
         self.order_by=util.to_list(order_by)
@@ -210,12 +220,14 @@ class PropertyLoader(MapperProperty):
                     # else set one of us as the "backreference"
                     if not self.mapper.props[self.backref].is_backref:
                         self.is_backref=True
-                    
+        elif not objectstore.global_attributes.is_class_managed(parent.class_, key):
+            raise "Non-primary property created for attribute '%s' on class '%s', but that attribute is not managed! Insure that the primary mapper for this class defines this property" % (key, parent.class_.__name__)
+
     def _is_primary(self):
         """a return value of True indicates we are the primary PropertyLoader for this loader's
         attribute on our mapper's class.  It means we can set the object's attribute behavior
         at the class level.  otherwise we have to set attribute behavior on a per-instance level."""
-        return self.parent._is_primary_mapper and not self.isoption
+        return self.parent._is_primary_mapper()
         
     def _set_class_attribute(self, class_, key):
         """sets attribute behavior on our target class."""
@@ -744,6 +756,8 @@ class EagerLoader(PropertyLoader):
             result_list = h
         else:
             result_list = getattr(instance, self.key)
+            if not hasattr(result_list, 'append_nohistory'):
+                raise "hi2"
     
         self._instance(row, imap, result_list)
 
@@ -760,7 +774,31 @@ class EagerLoader(PropertyLoader):
             row = fakerow
         return self.mapper._instance(row, imap, result_list)
 
-class EagerLazyOption(MapperOption):
+class GenericOption(MapperOption):
+    """a mapper option that can handle dotted property names,
+    descending down through the relations of a mapper until it
+    reaches the target."""
+    def __init__(self, key):
+        self.key = key
+    def process(self, mapper):
+        self.process_by_key(mapper, self.key)
+    def process_by_key(self, mapper, key):
+        tokens = key.split('.', 1)
+        if len(tokens) > 1:
+            oldprop = mapper.props[tokens[0]]
+            kwargs = util.constructor_args(oldprop)
+            kwargs['argument'] = self.process_by_key(oldprop.mapper.copy(), tokens[1])
+            newprop = oldprop.__class__(**kwargs)
+            mapper.set_property(tokens[0], newprop)
+        else:
+            self.create_prop(mapper, tokens[0])
+        return mapper
+        
+    def create_prop(self, mapper, key):
+        kwargs = util.constructor_args(oldprop)
+        mapper.set_property(key, class_(**kwargs ))
+            
+class EagerLazyOption(GenericOption):
     """an option that switches a PropertyLoader to be an EagerLoader or LazyLoader"""
     def __init__(self, key, toeager = True, **kwargs):
         self.key = key
@@ -770,17 +808,7 @@ class EagerLazyOption(MapperOption):
     def hash_key(self):
         return "EagerLazyOption(%s, %s)" % (repr(self.key), repr(self.toeager))
 
-    def process(self, mapper):
-        tup = self.key.split('.', 1)
-        key = tup[0]
-        oldprop = mapper.props[key]
-
-        if len(tup) > 1:
-            submapper = mapper.props[key].mapper
-            submapper = submapper.options(EagerLazyOption(tup[1], self.toeager))
-        else:
-            submapper = oldprop.mapper
-
+    def create_prop(self, mapper, key):
         if self.toeager:
             class_ = EagerLoader
         elif self.toeager is None:
@@ -789,9 +817,9 @@ class EagerLazyOption(MapperOption):
             class_ = LazyLoader
 
         # create a clone of the class using mostly the arguments from the original
-        self.kwargs['isoption'] = True
-        self.kwargs['argument'] = submapper
-        kwargs = util.constructor_args(oldprop, **self.kwargs)
+        submapper = mapper.props[key].mapper
+        #self.kwargs['argument'] = submapper
+        kwargs = util.constructor_args(mapper.props[key], **self.kwargs)
         mapper.set_property(key, class_(**kwargs ))
 
 class Aliasizer(sql.ClauseVisitor):