deferred with_polymorphic(), declarative support for concrete bases

[Mike Bayer]

support with_polymorphic as a callable. the patch below illustrates that all tests continue to pass. This would obviate the need for the awkward UsageRecipes/DeclarativeAbstractConcreteBase , and will also allow us to rewrite the "inheritance" documentation using declarative.

diff -r 086ae95614b4848c486eeb7acf2bc1b03b2a6a37 lib/sqlalchemy/orm/mapper.py
--- a/lib/sqlalchemy/orm/mapper.py  Thu Jul 28 11:53:18 2011 -0400
+++ b/lib/sqlalchemy/orm/mapper.py  Fri Jul 29 11:11:17 2011 -0400
@@ -150,31 +150,8 @@

         self.allow_partial_pks = allow_partial_pks

-        if with_polymorphic == '*':
-            self.with_polymorphic = ('*', None)
-        elif isinstance(with_polymorphic, (tuple, list)):
-            if isinstance(with_polymorphic[0](0), (basestring, tuple, list)):
-                self.with_polymorphic = with_polymorphic
-            else:
-                self.with_polymorphic = (with_polymorphic, None)
-        elif with_polymorphic is not None:
-            raise sa_exc.ArgumentError("Invalid setting for with_polymorphic")
-        else:
-            self.with_polymorphic = None
-
-        if isinstance(self.local_table, expression._SelectBase):
-            raise sa_exc.InvalidRequestError(
-                "When mapping against a select() construct, map against "
-                "an alias() of the construct instead."
-                "This because several databases don't allow a "
-                "SELECT from a subquery that does not have an alias."
-                )
-
-        if self.with_polymorphic and \
-                    isinstance(self.with_polymorphic[1](1),
-                                expression._SelectBase):
-            self.with_polymorphic = (self.with_polymorphic[0](0),
-                                self.with_polymorphic[1](1).alias())
+        self.with_polymorphic = lambda: with_polymorphic
+

         # our 'polymorphic identity', a string name that when located in a
         #  result set row indicates this Mapper should be used to construct
@@ -831,13 +808,13 @@
                     setter = False
                     instrument = False
                     col = self.polymorphic_on
-                    if self.with_polymorphic is None \
-                        or self.with_polymorphic[1](1).corresponding_column(col) \
-                        is None:
-                        raise sa_exc.InvalidRequestError("Could not map polymorphic_on column "
-                                  "'%s' to the mapped table - polymorphic "
-                                  "loads will not function properly"
-                                  % col.description)
+                    #if self.with_polymorphic is None \
+                    #    or self.with_polymorphic[1](1).corresponding_column(col) \
+                    #    is None:
+                    #    raise sa_exc.InvalidRequestError("Could not map polymorphic_on column "
+                    #              "'%s' to the mapped table - polymorphic "
+                    #              "loads will not function properly"
+                    #              % col.description)
                 else:
                     instrument = True

@@ -864,6 +841,33 @@
         else:
             self._set_polymorphic_identity = None

+    def _init_with_polymorphic(self):
+        self.with_polymorphic = with_polymorphic = self.with_polymorphic()
+        if with_polymorphic == '*':
+            self.with_polymorphic = ('*', None)
+        elif isinstance(with_polymorphic, (tuple, list)):
+            if isinstance(with_polymorphic[0](0), (basestring, tuple, list)):
+                self.with_polymorphic = with_polymorphic
+            else:
+                self.with_polymorphic = (with_polymorphic, None)
+        elif with_polymorphic is not None:
+            raise sa_exc.ArgumentError("Invalid setting for with_polymorphic")
+        else:
+            self.with_polymorphic = None
+
+        if isinstance(self.local_table, expression._SelectBase):
+            raise sa_exc.InvalidRequestError(
+                "When mapping against a select() construct, map against "
+                "an alias() of the construct instead."
+                "This because several databases don't allow a "
+                "SELECT from a subquery that does not have an alias."
+                )
+
+        if self.with_polymorphic and \
+                    isinstance(self.with_polymorphic[1](1),
+                                expression._SelectBase):
+            self.with_polymorphic = (self.with_polymorphic[0](0),
+                                self.with_polymorphic[1](1).alias())


     def _adapt_inherited_property(self, key, prop, init):
@@ -1029,6 +1033,9 @@
         """

         self._log("_post_configure_properties() started")
+
+        self._init_with_polymorphic()
+
         l = [prop) for key, prop in self._props.iteritems()]((key,)
         for key, prop in l:
             self._log("initialize prop %s", key)

[Mike Bayer]

abstract base is more difficult than this - the mapping still needs to be deferred. the concrete base can be added after the fact per the patch below, but the declarative case is not working yet.

diff -r 086ae95614b4848c486eeb7acf2bc1b03b2a6a37 lib/sqlalchemy/ext/declarative.py
--- a/lib/sqlalchemy/ext/declarative.py Thu Jul 28 11:53:18 2011 -0400
+++ b/lib/sqlalchemy/ext/declarative.py Fri Jul 29 12:55:31 2011 -0400
@@ -1042,7 +1042,10 @@
                 del our_stuff[key](key)
     cols = sorted(cols, key=lambda c:c._creation_order)
     table = None
-    if '__table__' not in dict_:
+    if '__abstract_base_table__' in cls.__dict__:
+        table = cls.__abstract_base_table__
+
+    elif '__table__' not in dict_:
         if tablename is not None:

             if isinstance(table_args, dict):
@@ -1156,13 +1159,25 @@
                                 table, 
                                 properties=our_stuff, 
                                 **mapper_args)
+    if '__abstract_base_table__' in cls.__dict__:
+        for scls in cls.__subclasses__():
+            if hasattr(scls, '__mapper__') and \
+                scls.__mapper__.concrete and \
+                cls in scls.__bases__:
+                scls.__mapper__._set_concrete_base(cls.__mapper__)

 class DeclarativeMeta(type):
     def __init__(cls, classname, bases, dict_):
         if '_decl_class_registry' in cls.__dict__:
             return type.__init__(cls, classname, bases, dict_)

-        _as_declarative(cls, classname, cls.__dict__)
+        if '__abstract_base_table__' in cls.__dict__:
+            def decl():
+                if '__mapper__' not in cls.__dict__:
+                    _as_declarative(cls, classname, cls.__dict__)
+            cls.go = staticmethod(decl)
+        else:
+            _as_declarative(cls, classname, cls.__dict__)
         return type.__init__(cls, classname, bases, dict_)

     def __setattr__(cls, key, value):
diff -r 086ae95614b4848c486eeb7acf2bc1b03b2a6a37 lib/sqlalchemy/orm/mapper.py
--- a/lib/sqlalchemy/orm/mapper.py  Thu Jul 28 11:53:18 2011 -0400
+++ b/lib/sqlalchemy/orm/mapper.py  Fri Jul 29 12:55:31 2011 -0400
@@ -543,6 +543,21 @@
                     "Mapper '%s' does not have a mapped_table specified." 
                     % self)

+    def _set_concrete_base(self, mapper):
+        assert self.concrete
+        assert not self.inherits
+        self.inherits = mapper
+        self.inherits.polymorphic_map.update(self.polymorphic_map)
+        self.polymorphic_map = self.inherits.polymorphic_map
+        for mapper in self.iterate_to_root():
+            if mapper.polymorphic_on is not None:
+                mapper._requires_row_aliasing = True
+        self.batch = self.inherits.batch
+        self.base_mapper = self.inherits.base_mapper
+        self.inherits._inheriting_mappers.add(self)
+        self.passive_updates = self.inherits.passive_updates
+        self._all_tables = self.inherits._all_tables
+
     def _configure_legacy_instrument_class(self):

         if self.inherits:

[Mike Bayer]

the attached patch moves several mapping settings to be settable after the fact. Test illustrates new extensions for declarative for abstract and concrete use cases. Would be nice however if _set_with_polymorphic() and _set_polymorphic_on() worked for joined inh and/or multi-level inh as well, doesn't seem to pass in test_abc_inheritance if these are moved to the "configure" step.

[Mike Bayer]

polymorphic_on set later is not that big a deal, just needs to be in order of descent:

    def _post_configure_properties(self):

        if self is self.base_mapper:
            for m in self.self_and_descendants:
                m._configure_polymorphic_setter(init=m.configured)

[Mike Bayer]

seems to work, we'll see how it goes.

bb62d80217e584acf3f899804bb66b19f71205e2

[Mike Bayer]

Removing milestone: 0.7.3 (automated comment)