From: Mike Bayer Date: Mon, 7 Jan 2008 18:52:02 +0000 (+0000) Subject: - fixed an attribute history bug whereby assigning a new collection X-Git-Tag: rel_0_4_2b~2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=e8feacf1db658ecccf7bb1d1688662e701ad37f5;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - fixed an attribute history bug whereby assigning a new collection to a collection-based attribute which already had pending changes would generate incorrect history [ticket:922] - fixed delete-orphan cascade bug whereby setting the same object twice to a scalar attribute could log it as an orphan [ticket:925] - generative select.order_by(None) / group_by(None) was not managing to reset order by/group by criterion, fixed [ticket:924] --- diff --git a/CHANGES b/CHANGES index c9922fc34a..539b57a7f8 100644 --- a/CHANGES +++ b/CHANGES @@ -10,10 +10,21 @@ CHANGES when no length is present is also deprecated until 0.5; will issue a warning when used for CREATE TABLE statements (String with no length for SQL expression purposes is still fine) [ticket:912] - + + - generative select.order_by(None) / group_by(None) was not managing to + reset order by/group by criterion, fixed [ticket:924] + - orm - suppressing *all* errors in InstanceState.__cleanup() now. - + + - fixed an attribute history bug whereby assigning a new collection + to a collection-based attribute which already had pending changes + would generate incorrect history [ticket:922] + + - fixed delete-orphan cascade bug whereby setting the same + object twice to a scalar attribute could log it as an orphan + [ticket:925] + - Fixed cascades on a += assignment to a list-based relation. - synonyms can now be created against props that don't exist yet, diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py index a95ae05002..84c9dfbb63 100644 --- a/lib/sqlalchemy/orm/attributes.py +++ b/lib/sqlalchemy/orm/attributes.py @@ -251,8 +251,7 @@ class AttributeImpl(object): def set_committed_value(self, state, value): """set an attribute value on the given instance and 'commit' it.""" - if state.committed_state is not None: - state.commit_attr(self, value) + state.commit_attr(self, value) # remove per-instance callable, if any state.callables.pop(self.key, None) state.dict[self.key] = value @@ -395,7 +394,7 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl): if self.trackparent: if value is not None: self.sethasparent(value._state, True) - if previous is not None: + if previous is not value and previous is not None: self.sethasparent(previous._state, False) instance = state.obj() @@ -544,7 +543,8 @@ class CollectionAttributeImpl(AttributeImpl): if old is value: return - state.committed_state[self.key] = self.copy(old) + if self.key not in state.committed_state: + state.committed_state[self.key] = self.copy(old) old_collection = self.get_collection(state, old) diff --git a/lib/sqlalchemy/sql/expression.py b/lib/sqlalchemy/sql/expression.py index b3b04b678c..3dc7242d7a 100644 --- a/lib/sqlalchemy/sql/expression.py +++ b/lib/sqlalchemy/sql/expression.py @@ -2815,7 +2815,7 @@ class _SelectBaseMixin(object): form of this method instead to prevent this issue. """ - if clauses == [None]: + if len(clauses) == 1 and clauses[0] is None: self._order_by_clause = ClauseList() else: if getattr(self, '_order_by_clause', None): @@ -2833,7 +2833,7 @@ class _SelectBaseMixin(object): form of this method instead to prevent this issue. """ - if clauses == [None]: + if len(clauses) == 1 and clauses[0] is None: self._group_by_clause = ClauseList() else: if getattr(self, '_group_by_clause', None): diff --git a/test/dialect/oracle.py b/test/dialect/oracle.py index 64f86329ed..cff96c21fb 100644 --- a/test/dialect/oracle.py +++ b/test/dialect/oracle.py @@ -59,7 +59,7 @@ class CompileTest(SQLCompileTest): s = select([t]).limit(10).offset(20).order_by(t.c.col2) self.assert_compile(s, "SELECT col1, col2 FROM (SELECT sometable.col1 AS col1, " - "sometable.col2 AS col2, ROW_NUMBER() OVER (ORDER BY sometable.col2) AS ora_rn FROM sometable ORDER BY sometable.col2) WHERE ora_rn>20 AND ora_rn<=30") + "sometable.col2 AS col2, ROW_NUMBER() OVER (ORDER BY sometable.col2) AS ora_rn FROM sometable) WHERE ora_rn>20 AND ora_rn<=30") def test_outer_join(self): table1 = table('mytable', diff --git a/test/orm/attributes.py b/test/orm/attributes.py index a03123c9d8..7ec838458c 100644 --- a/test/orm/attributes.py +++ b/test/orm/attributes.py @@ -337,6 +337,14 @@ class AttributesTest(PersistTest): b2.element = None assert not attributes.has_parent(Bar, f2, 'element') + + # test that double assignment doesn't accidentally reset the 'parent' flag. + b3 = Bar() + f4 = Foo() + b3.element = f4 + assert attributes.has_parent(Bar, f4, 'element') + b3.element = f4 + assert attributes.has_parent(Bar, f4, 'element') def test_mutablescalars(self): """test detection of changes on mutable scalar items""" @@ -450,11 +458,6 @@ class BackrefTest(PersistTest): c.students = [s1, s2, s3] self.assert_(s2.courses == [c]) self.assert_(s1.courses == [c]) - print "--------------------------------" - print s1 - print s1.courses - print c - print c.students s1.courses.remove(c) self.assert_(c.students == [s2,s3]) @@ -766,6 +769,12 @@ class HistoryTest(PersistTest): f._state.commit(['someattr']) self.assertEquals(attributes.get_history(f._state, 'someattr'), ([], [there], [])) + + f.someattr = [hi] + self.assertEquals(attributes.get_history(f._state, 'someattr'), ([hi], [], [there])) + + f.someattr = [old, new] + self.assertEquals(attributes.get_history(f._state, 'someattr'), ([old, new], [], [there])) # case 2. object with direct settings (similar to a load operation) f = Foo() @@ -808,8 +817,6 @@ class HistoryTest(PersistTest): f._state.commit(['someattr']) self.assertEquals(tuple([set(x) for x in attributes.get_history(f._state, 'someattr')]), (set([]), set([hi, there]), set([]))) - - def test_object_collections_mutate(self): class Foo(fixtures.Base): pass @@ -872,13 +879,21 @@ class HistoryTest(PersistTest): collection = attributes.init_collection(f, 'someattr') collection.append_without_event(new) f._state.commit_all() - print f._state.dict self.assertEquals(attributes.get_history(f._state, 'someattr'), ([], [new], [])) f.id = 1 f.someattr.remove(new) self.assertEquals(attributes.get_history(f._state, 'someattr'), ([], [], [new])) + # case 3. mixing appends with sets + f = Foo() + f.someattr.append(hi) + self.assertEquals(attributes.get_history(f._state, 'someattr'), ([hi], [], [])) + f.someattr.append(there) + self.assertEquals(attributes.get_history(f._state, 'someattr'), ([hi, there], [], [])) + f.someattr = [there] + self.assertEquals(attributes.get_history(f._state, 'someattr'), ([there], [], [])) + def test_collections_via_backref(self): class Foo(fixtures.Base): pass diff --git a/test/orm/cascade.py b/test/orm/cascade.py index 7583507b2c..8b416fafb7 100644 --- a/test/orm/cascade.py +++ b/test/orm/cascade.py @@ -263,6 +263,15 @@ class M2OCascadeTest(AssertMixin): assert p not in ctx.current assert e not in ctx.current + def testorphan3(self): + """test that double assignment doesn't accidentally reset the 'parent' flag.""" + + jack = ctx.current.query(User).get_by(user_name='jack') + newpref = Pref("newpref") + jack.pref = newpref + jack.pref = newpref + ctx.current.flush() + class M2MCascadeTest(AssertMixin): diff --git a/test/sql/select.py b/test/sql/select.py index a5d45a6b8b..0df575c7c0 100644 --- a/test/sql/select.py +++ b/test/sql/select.py @@ -487,6 +487,17 @@ sq.myothertable_othername AS sq_myothertable_othername FROM (" + sqstring + ") A table2.select(order_by = [table2.c.otherid, table2.c.othername.desc()]), "SELECT myothertable.otherid, myothertable.othername FROM myothertable ORDER BY myothertable.otherid, myothertable.othername DESC" ) + + # generative order_by + self.assert_compile( + table2.select().order_by(table2.c.otherid).order_by(table2.c.othername.desc()), + "SELECT myothertable.otherid, myothertable.othername FROM myothertable ORDER BY myothertable.otherid, myothertable.othername DESC" + ) + + self.assert_compile( + table2.select().order_by(table2.c.otherid).order_by(table2.c.othername.desc()).order_by(None), + "SELECT myothertable.otherid, myothertable.othername FROM myothertable" + ) def testgroupby(self): self.assert_compile( @@ -494,6 +505,17 @@ sq.myothertable_othername AS sq_myothertable_othername FROM (" + sqstring + ") A "SELECT myothertable.othername, count(myothertable.otherid) AS count_1 FROM myothertable GROUP BY myothertable.othername" ) + # generative group by + self.assert_compile( + select([table2.c.othername, func.count(table2.c.otherid)]).group_by(table2.c.othername), + "SELECT myothertable.othername, count(myothertable.otherid) AS count_1 FROM myothertable GROUP BY myothertable.othername" + ) + + self.assert_compile( + select([table2.c.othername, func.count(table2.c.otherid)]).group_by(table2.c.othername).group_by(None), + "SELECT myothertable.othername, count(myothertable.otherid) AS count_1 FROM myothertable" + ) + def testgroupby_and_orderby(self): self.assert_compile(