On Dec 15, 2011, at 5:05 PM, Kent wrote:

> wait...that's where you lose me.  In this condition where user's
> changed someobject.some_related_id, then as soon as that is flushed,
> someobject.some_related by definition is no longer going to be the
> object we get with the currently committed id.... This is circling
> back to the automatic expiration of relationships dilemma.  it seems
> we are purposefully giving them the wrong object.  Directly after the
> flush, were we to query that same relationship, it would return a
> different object/collection.
> 
> I believe that is the other reason I needed to abandon trusting the
> relationship when within a flush.  I think I recall this exact problem
> occurring to me.  The id had changed and anywhere else within
> SQLAlchemy land (that isn't in a flush), I could trust the
> relationship to bring back the correct object given the current state
> of the id.  However, in that case (inside a flush) it was bringing
> back the wrong object (the one inconsistent with the current id) so
> inspecting that related object's values would give me the wrong
> answer.
> 
> You sure this logic (and therefore also test cases) aren't wrong?

The logic and test cases are definitely correct for the one-to-many case, it is 
attached as test_o2m.py and if you change line 417 of orm/strategies.py to not 
use committed_state, it fails.    We need to use what was loaded from the DB, 
not what is pending.

For the many-to-one case with use_get here there are actually no tests that 
cover the case where the two values are different - I can see this by changing 
get_attr() to call *both* functions and compare the values to see if they're 
different and there is no such test.   I can't really come up with a test that 
makes any difference here as the mechanics of things tends to "work out" the 
way m2o's currently work.

But to change it would mean it's inconsistent.    Within a flush, the 
dependency system needs to load relationships based on the last known database 
state, not the not-yet-flushed pending changes.   When this is called during a 
flush, we know we haven't yet updated the object since we're still looking at 
its attributes, which always results from one of the objects its dependent on, 
or from itself.

If the issue you're having is that you have a session handler that wants things 
to act as though we're outside of the flush, the internal APIs need to be 
changed such that dependency.py passes another flag in to use "committed state" 
without the need to check session._flushing.     Or perhaps it needs to somehow 
check if the target object has already been handled by the mapper - if your 
object has already been the subject of an UPDATE by the time your extension 
does something, then the committed value *is* wrong.    Try out the attached 
patch and if you can produce a test case which is repaired by it, then it's a 
candidate for inclusion.




> 
> 
>> There's still potentially a query run to get at some_related, just the 
>> foreign key value we use from the parent is different - it's looking at the 
>> persistent value.
>> 
>> To get more specific, comment out the conditional and just make it do the 
>> non-committed, then run the orm unit tests.  The tests that fail will be the 
>> ones that illustrate the use case.
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "sqlalchemy" group.
> To post to this group, send email to [email protected].
> To unsubscribe from this group, send email to 
> [email protected].
> For more options, visit this group at 
> http://groups.google.com/group/sqlalchemy?hl=en.
> 

from sqlalchemy import *
from sqlalchemy.orm import *

metadata = MetaData()
users = Table('users', metadata,
    Column('username', String(50), primary_key=True),
    Column('fullname', String(100)))

addresses = Table('addresses', metadata,
    Column('email', String(50), primary_key=True),
    Column('username', String(50), 
                    ForeignKey('users.username')))


class BasicEntity(object):
    def __init__(self, **kw):
        for key, value in kw.iteritems():
            setattr(self, key, value)

class User(BasicEntity):
    pass

class Address(BasicEntity):
    pass

mapper(User, users, properties={
    'addresses':relationship(Address, passive_updates=False)
})
mapper(Address, addresses)

e = create_engine("sqlite://", echo=True)
metadata.create_all(e)

sess = Session(e)
u1 = User(username='jack', fullname='jack')
u1.addresses.append(Address(email='jack1'))
u1.addresses.append(Address(email='jack2'))
sess.add(u1)
sess.commit()
u1.username = 'ed'
sess.commit()

assert sess.execute("select username from addresses").fetchall() == [
    ("ed",),
    ("ed",)]







Attachment: finer_grained_check.patch
Description: Binary data

-- 
You received this message because you are subscribed to the Google Groups 
"sqlalchemy" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/sqlalchemy?hl=en.

Reply via email to