Many to many relations in conrete inherited classes produces duplicates

[Former user]

If we define many-to-many relation in base class with secondary table s1, and then re-define it with secondary table s2 in inherited class, it will try to use both relations and insert records in both s1 and s2. So in provided example we get deleted_posts -> deleted_post_files <- files AND deleted_posts -> post_files <- files, so object from base class with same primary index gets incorrectly associated to files table.

[Mike Bayer]

overriding of relation()'s functionality is not supported in any case, regardless of inheritance type. i will test later but I am likely to mark this as "wontfix".

[Mike Bayer]

it is in fact a bug. we have no many-to-many tests for concrete inheritance at this time and it is concrete where "overrides" are necessary for relations.

[Mike Bayer]

filtering the dependency processors to stop on concrete begins to address this problem:

Index: lib/sqlalchemy/orm/unitofwork.py
===================================================================
--- lib/sqlalchemy/orm/unitofwork.py    (revision 6121)
+++ lib/sqlalchemy/orm/unitofwork.py    (working copy)
@@ -692,6 +692,9 @@
         else:
             elements = self.targettask.polymorphic_tosave_elements

+        elements = [for e in elements if _state_mapper(e.state) is self.targettask.mapper or not _state_mapper(e.state).concrete](e) 
+            
+
         self.processor.process_dependencies(
             self.targettask, 
             [for elem in elements](elem.state),

but this is just a hack.

Changing the dependency procs to not use "polymorphic_elements" at all in favor of just "elements" breaks many tests - the behavior needs to be limited to stop at concrete subclasses.

1. why can't the dependency processors consider only non-polymorhic elements ? don't subclass mappers have the same collection of mapperproperties as the parent class, and therefore their own dependency procs? this question must be answered.

[Mike Bayer]

answer: yes, subclass mappers also register the dependency processors associated with the base class. but its the same processor, so that processor must process the full descendant mapper chain.

UOWDependencyProcessor.preexecute and execute() must be able to ask the question for each entity, "do I handle this entity?". right now the assumption is they handle the full depth of entity, and that is where the mistaken assumption is. the presence of ConcreteInheritedProperty may be something we can look for when asking this question. along the lines of:

Index: lib/sqlalchemy/orm/unitofwork.py
===================================================================
--- lib/sqlalchemy/orm/unitofwork.py    (revision 6121)
+++ lib/sqlalchemy/orm/unitofwork.py    (working copy)
@@ -652,6 +652,16 @@
     def __hash__(self):
         return hash((self.processor, self.targettask))

+    def _do_we_handle_this(self, elem):
+        state = elem.state
+        mapper = _state_mapper(state)
+        if self.processor.key not in mapper._props or \
+            not hasattr(mapper._props[self.processor.key](self.processor.key), '_dependency_processor') or \
+            mapper._props[self.processor.key](self.processor.key)._dependency_processor is self.processor:
+            return True
+        else:
+            return False
+
     def preexecute(self, trans):
         """preprocess all objects contained within this ``UOWDependencyProcessor``s target task.

@@ -673,12 +683,13 @@
             return elem.state

         ret = False
-        elements = [for elem in self.targettask.polymorphic_tosave_elements if self not in elem.preprocessed](getobj(elem))
+        elements = [for elem in self.targettask.polymorphic_tosave_elements if self not in elem.preprocessed and self._do_we_handle_this(elem)](getobj(elem))
+
         if elements:
             ret = True
             self.processor.preprocess_dependencies(self.targettask, elements, trans, delete=False)

-        elements = [for elem in self.targettask.polymorphic_todelete_elements if self not in elem.preprocessed](getobj(elem))
+        elements = [for elem in self.targettask.polymorphic_todelete_elements if self not in elem.preprocessed and self._do_we_handle_this(elem)](getobj(elem))
         if elements:
             ret = True
             self.processor.preprocess_dependencies(self.targettask, elements, trans, delete=True)
@@ -694,7 +705,7 @@

         self.processor.process_dependencies(
             self.targettask, 
-            [for elem in elements](elem.state), 
+            [for elem in elements if self._do_we_handle_this(elem)](elem.state), 
             trans, 
             delete=delete)

the above would be a generic way to block super-relations from hitting sub-relations that have been overridden regardless of concrete or not. but the navigation needs to be improved so that this check becomes more succinct, and should probably involve adding information to each DependencyProcessor as to what mapped classes they should handle.

[Mike Bayer]

this is fixed in b9b62b2369e00be2f344dd96aec94e88c9210fb0.

[Mike Bayer]

Removing milestone: 0.5.6 (automated comment)