adding None to collection fails ungracefully

[Mike Bayer]

from sqlalchemy import *
from sqlalchemy.orm import *

from sqlalchemy.ext.declarative import declarative_base

Base = declarative_base()

class Parent(Base):
    __tablename__ = 'p'
    id = Column(Integer, primary_key=True)


class Child(Base):
    __tablename__ = 'c'
    id = Column(Integer, primary_key=True)
    parent_id = Column(Integer, ForeignKey('p.id'))
    parent = relationship(Parent, backref="children")

e = create_engine('sqlite://')
Base.metadata.create_all(e)

s = Session(e)

s.add(Parent(children=[None](None)))




Traceback (most recent call last):
  File "test.py", line 25, in <module>
    s.add(Parent(children=[None](None)))
  File "/Users/classic/dev/sqlalchemy/lib/sqlalchemy/orm/session.py", line 1144, in add
    self._save_or_update_state(state)
  File "/Users/classic/dev/sqlalchemy/lib/sqlalchemy/orm/session.py", line 1159, in _save_or_update_state
    halt_on=self._contains_state):
  File "/Users/classic/dev/sqlalchemy/lib/sqlalchemy/orm/mapper.py", line 1688, in cascade_iterator
    parent_dict, visited_states, halt_on))
  File "/Users/classic/dev/sqlalchemy/lib/sqlalchemy/orm/properties.py", line 808, in cascade_iterator
    get_all_pending(state, dict_)
  File "/Users/classic/dev/sqlalchemy/lib/sqlalchemy/orm/attributes.py", line 725, in get_all_pending
    current_states = [c) for c in current]((instance_state(c),)
AttributeError: 'NoneType' object has no attribute '_sa_instance_state'

[Mike Bayer]

diff -r 57b91143ef74f0efb2c9b9b6caa858db7c0de2d5 lib/sqlalchemy/orm/attributes.py
--- a/lib/sqlalchemy/orm/attributes.py  Wed Jun 29 00:49:57 2011 -0400
+++ b/lib/sqlalchemy/orm/attributes.py  Wed Jun 29 19:31:34 2011 -0400
@@ -722,8 +722,12 @@
         if self.key in state.committed_state:
             original = state.committed_state[self.key](self.key)
             if original is not NO_VALUE:
-                current_states = [c) for c in current]((instance_state(c),)
-                original_states = [c) for c in original]((instance_state(c),)
+                current_states = [is not None) and 
+                                    instance_state(c) or None, c) 
+                                    for c in current](((c)
+                original_states = [is not None) and 
+                                    instance_state(c) or None, c) 
+                                    for c in original](((c)

                 current_set = dict(current_states)
                 original_set = dict(original_states)
@@ -1114,8 +1118,13 @@
         elif original is _NO_HISTORY:
             return cls((), list(current), ())
         else:
-            current_states = [c) for c in current]((instance_state(c),)
-            original_states = [c) for c in original]((instance_state(c),)
+
+            current_states = [is not None) and instance_state(c) or None, c) 
+                                for c in current 
+                                ](((c)
+            original_states = [is not None) and instance_state(c) or None, c) 
+                                for c in original 
+                                ](((c)

             current_set = dict(current_states)
             original_set = dict(original_states)
diff -r 57b91143ef74f0efb2c9b9b6caa858db7c0de2d5 lib/sqlalchemy/orm/properties.py
--- a/lib/sqlalchemy/orm/properties.py  Wed Jun 29 00:49:57 2011 -0400
+++ b/lib/sqlalchemy/orm/properties.py  Wed Jun 29 19:31:34 2011 -0400
@@ -818,6 +818,10 @@
             if instance_state in visited_states:
                 continue

+            if c is None:
+                raise sa_exc.InvalidRequestError(
+                            "Can't cascade %s operation to "
+                            "None present in collection %s" % (type_, self))
             instance_dict = attributes.instance_dict(c)

             if halt_on and halt_on(instance_state):

[Mike Bayer]

this skips the None silently in 0.6 so let's just keep this on 0.7.

[Mike Bayer]

#2229 proposes adding a warning in 0.8.

[Mike Bayer]

0cc357c6caf8f5d4e33e3dedd799beeca0304518

[Mike Bayer]

Removing milestone: 0.7.2 (automated comment)