fixed bug where if a many-to-many table mapped as "secondary" had other cols in it...

diff --git a/CHANGES b/CHANGES
index 020c5bf5075c4a1c0267f72242468837d76973f4..f23b43ab3569140e05cf6523f8536925115ccb4a 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -19,6 +19,8 @@ the MetaData object properly
 - fixed bug where Column with redefined "key" property wasnt getting
 type conversion happening in the ResultProxy [ticket:207]
 - fixed 'port' attribute of URL to be an integer if present
+- fixed old bug where if a many-to-many table mapped as "secondary"
+had extra columns, delete operations didnt work
 
 0.2.2
 - big improvements to polymorphic inheritance behavior, enabling it

diff --git a/lib/sqlalchemy/attributes.py b/lib/sqlalchemy/attributes.py
index a9b31340106b34543aa061936038db20c12eb13f..60cbecec14ed8aff05f42259b7035c78af0b993a 100644 (file)
--- a/lib/sqlalchemy/attributes.py
+++ b/lib/sqlalchemy/attributes.py
@@ -167,8 +167,7 @@ class InstrumentedAttribute(object):
                 trig()
             if self.uselist:
                 value = InstrumentedList(self, obj, value)
-            elif self.trackparent or len(self.extensions):
-                old = self.get(obj)
+            old = self.get(obj)
             obj.__dict__[self.key] = value
             state['modified'] = True
             if not self.uselist:
@@ -179,7 +178,10 @@ class InstrumentedAttribute(object):
                         self.sethasparent(old, False)
                 for ext in self.extensions:
                     ext.set(event or self, obj, value, old)
-            
+            else:
+                # set the deleted event for the old item
+                old[:] = []
+                
     def delete(self, event, obj):
         """deletes a value from the given object. 'event' is the InstrumentedAttribute that
         initiated the delete() operation and is used to control the depth of a circular delete
@@ -499,7 +501,7 @@ class AttributeHistory(object):
                 else:
                     self._deleted_items = []
                 self._unchanged_items = []
-
+        #print "orig", original, "current", current, "added", self._added_items, "unchanged", self._unchanged_items, "deleted", self._deleted_items
     def __iter__(self):
         return iter(self._current)
     def added_items(self):

diff --git a/lib/sqlalchemy/orm/dependency.py b/lib/sqlalchemy/orm/dependency.py
index 145ec5e9b6bd84a2a750e6b7236d9e9c4257913a..067e842c35b4c2fd4497278aa2135ff362ddf46c 100644 (file)
--- a/lib/sqlalchemy/orm/dependency.py
+++ b/lib/sqlalchemy/orm/dependency.py
@@ -281,7 +281,7 @@ class ManyToManyDP(DependencyProcessor):
         if len(secondary_delete):
             # TODO: precompile the delete/insert queries and store them as instance variables
             # on the PropertyLoader
-            statement = self.secondary.delete(sql.and_(*[c == sql.bindparam(c.key) for c in self.secondary.c]))
+            statement = self.secondary.delete(sql.and_(*[c == sql.bindparam(c.key) for c in self.secondary.c if c.key in associationrow]))
             connection.execute(statement, secondary_delete)
         if len(secondary_insert):
             statement = self.secondary.insert()

diff --git a/test/orm/objectstore.py b/test/orm/objectstore.py
index d267520f3d25e1369d38247a07d776b6bb3bb384..e897d84a8a448c16a7599a64771437dba228c630 100644 (file)
--- a/test/orm/objectstore.py
+++ b/test/orm/objectstore.py
@@ -920,54 +920,15 @@ class SaveTest(SessionTest):
         a1.user = None
         ctx.current.flush()
 
-    def _testalias(self):
-        """tests that an alias of a table can be used in a mapper. 
-        the mapper has to locate the original table and columns to keep it all straight."""
-        ualias = Alias(users, 'ualias')
-        m = mapper(User, ualias)
-        u = User()
-        u.user_name = 'testalias'
-        m.save(u)
-        
-        u2 = m.select(ualias.c.user_id == u.user_id)[0]
-        self.assert_(u2 is u)
-
-    def _testremove(self):
-        m = mapper(User, users, properties = dict(
-            addresses = relation(mapper(Address, addresses), lazy = True)
-        ))
-        u = User()
-        u.user_name = 'one2manytester'
-        u.addresses = []
-        a = Address()
-        a.email_address = 'one2many@test.org'
-        u.addresses.append(a)
-        a2 = Address()
-        a2.email_address = 'lala@test.org'
-        u.addresses.append(a2)
-        m.save(u)
-        addresstable = addresses.select(addresses.c.address_id.in_(a.address_id, a2.address_id)).execute().fetchall()
-        self.echo( repr(addresstable[0].values()))
-        self.assertEqual(addresstable[0].values(), [a.address_id, u.user_id, 'one2many@test.org'])
-        self.assertEqual(addresstable[1].values(), [a2.address_id, u.user_id, 'lala@test.org'])
-        del u.addresses[1]
-        m.save(u)
-        addresstable = addresses.select(addresses.c.address_id.in_(a.address_id, a2.address_id)).execute().fetchall()
-        self.echo( repr(addresstable))
-        self.assertEqual(addresstable[0].values(), [a.address_id, u.user_id, 'one2many@test.org'])
-        self.assertEqual(addresstable[1].values(), [a2.address_id, None, 'lala@test.org'])
-
     def testmanytomany(self):
         items = orderitems
 
         keywordmapper = mapper(Keyword, keywords)
 
-        items.select().execute()
         m = mapper(Item, items, properties = dict(
                 keywords = relation(keywordmapper, itemkeywords, lazy = False),
             ))
 
-
         data = [Item,
             {'item_name': 'mm_item1', 'keywords' : (Keyword,[{'name': 'big'},{'name': 'green'}, {'name': 'purple'},{'name': 'round'}])},
             {'item_name': 'mm_item2', 'keywords' : (Keyword,[{'name':'blue'}, {'name':'imnew'},{'name':'round'}, {'name':'small'}])},
@@ -1049,8 +1010,30 @@ class SaveTest(SessionTest):
         ctx.current.delete(objects[3])
         ctx.current.flush()
 
+    def testmanytomanyremove(self):
+        """tests that setting a list-based attribute to '[]' properly affects the history and allows
+        the many-to-many rows to be deleted"""
+        keywordmapper = mapper(Keyword, keywords)
+
+        m = mapper(Item, orderitems, properties = dict(
+                keywords = relation(keywordmapper, itemkeywords, lazy = False),
+            ))
+
+        i = Item()
+        k1 = Keyword()
+        k2 = Keyword()
+        i.keywords.append(k1)
+        i.keywords.append(k2)
+        ctx.current.flush()
+        
+        assert itemkeywords.count().scalar() == 2
+        i.keywords = []
+        ctx.current.flush()
+        assert itemkeywords.count().scalar() == 0
+
 
     def testmanytomanyupdate(self):
+        """tests some history operations on a many to many"""
         class Keyword(object):
             def __init__(self, name):
                 self.name = name
@@ -1075,9 +1058,6 @@ class SaveTest(SessionTest):
         item.keywords = []
         item.keywords.append(k1)
         item.keywords.append(k2)
-        print item.keywords.unchanged_items()
-        print item.keywords.added_items()
-        print item.keywords.deleted_items()
         ctx.current.flush()
         
         ctx.current.clear()
@@ -1086,6 +1066,7 @@ class SaveTest(SessionTest):
         assert item.keywords == [k1, k2]
         
     def testassociation(self):
+        """basic test of an association object"""
         class IKAssociation(object):
             def __repr__(self):
                 return "\nIKAssociation " + repr(self.item_id) + " " + repr(self.keyword)
@@ -1283,6 +1264,59 @@ class SaveTest2(SessionTest):
                         ]
         )
 
+class SaveTest3(SessionTest):
+
+    def setUpAll(self):
+        SessionTest.setUpAll(self)
+        global metadata, t1, t2, t3
+        metadata = testbase.metadata
+        t1 = Table('items', metadata,
+            Column('item_id', INT, Sequence('items_id_seq', optional=True), primary_key = True),
+            Column('item_name', VARCHAR(50)),
+        )
+
+        t3 = Table('keywords', metadata,
+            Column('keyword_id', Integer, Sequence('keyword_id_seq', optional=True), primary_key = True),
+            Column('name', VARCHAR(50)),
+
+        )
+        t2 = Table('assoc', metadata,
+            Column('item_id', INT, ForeignKey("items")),
+            Column('keyword_id', INT, ForeignKey("keywords")),
+            Column('foo', Boolean, default=True)
+        )
+        metadata.create_all()
+    def tearDownAll(self):
+        metadata.drop_all()
+        SessionTest.tearDownAll(self)
+
+    def setUp(self):
+        pass
+    def tearDown(self):
+        pass
+
+    def testmanytomanyxtracolremove(self):
+        """tests that a many-to-many on a table that has an extra column can properly delete rows from the table
+        without referencing the extra column"""
+        mapper(Keyword, t3)
+
+        mapper(Item, t1, properties = dict(
+                keywords = relation(Keyword, secondary=t2, lazy = False),
+            ))
+
+        i = Item()
+        k1 = Keyword()
+        k2 = Keyword()
+        i.keywords.append(k1)
+        i.keywords.append(k2)
+        ctx.current.flush()
+
+        assert t2.count().scalar() == 2
+        i.keywords = []
+        print i.keywords
+        ctx.current.flush()
+        assert t2.count().scalar() == 0
+
 
 if __name__ == "__main__":
     testbase.main()        

diff --git a/test/tables.py b/test/tables.py
index 7085e5a775867c2048706942e3f19542dff88fba..72870ec1eb7413a18846576c6ac844ffc65e82e0 100644 (file)
--- a/test/tables.py
+++ b/test/tables.py
@@ -53,7 +53,7 @@ userkeywords = Table('userkeywords', metadata,
 itemkeywords = Table('itemkeywords', metadata,
     Column('item_id', INT, ForeignKey("items")),
     Column('keyword_id', INT, ForeignKey("keywords")),
-    
+#    Column('foo', Boolean, default=True)
 )
 
 def create():