well since create_version() is called within before_flush(), the net effect of 
any changes on local many-to-one relationships can only be taken into account 
by looking at these attributes.  The test in test_versioning called 
test_relationship illustrates this.

so the diff is like this, the test still passes:

diff --git a/examples/versioned_history/history_meta.py 
b/examples/versioned_history/history_meta.py
index 8cb5234..e8d3ed2 100644
--- a/examples/versioned_history/history_meta.py
+++ b/examples/versioned_history/history_meta.py
@@ -6,6 +6,7 @@ from sqlalchemy.orm.exc import UnmappedClassError, 
UnmappedColumnError
 from sqlalchemy import Table, Column, ForeignKeyConstraint, Integer
 from sqlalchemy import event
 from sqlalchemy.orm.properties import RelationshipProperty
+from sqlalchemy.orm.attributes import PASSIVE_NO_FETCH_RELATED
 
 def col_references_table(col, table):
     for fk in col.foreign_keys:
@@ -160,7 +161,7 @@ def create_version(obj, session, deleted = False):
         # check those too
         for prop in obj_mapper.iterate_properties:
             if isinstance(prop, RelationshipProperty) and \
-                attributes.get_history(obj, prop.key).has_changes():
+                attributes.get_history(obj, prop.key, 
passive=PASSIVE_NO_FETCH_RELATED).has_changes():
                 for p in prop.local_columns:
                     if p.foreign_keys:
                         obj_changed = True




On Jan 24, 2014, at 5:48 PM, Simon King <si...@simonking.org.uk> wrote:

> I see the same behaviour on 0.8.2 and 0.9.1.
> 
> Before making any changes I’d just like to understand what the code is trying 
> to do. As far as I can tell, the first loop works up the inheritance chain, 
> looking at each column to see if it changed, which is all fine.
> 
> But then, if it hasn’t found any columns that have changed, it goes into the 
> second loop, over obj_mapper.iterate_properties. I guess it is looking for 
> relationships which don’t correspond to columns on the object itself (eg. the 
> other side of a 1-to-many relationship). I can’t think of a situation where 
> this will produce something that can actually be written to the history table.
> 
> I removed the second loop altogether and in the very simple test it behaved 
> exactly as I would have expected, but perhaps it’ll fail in more complicate 
> scenarios?
> 
> Thanks again,
> 
> Simon
> 
> On 24 Jan 2014, at 18:00, Michael Bayer <mike...@zzzcomputing.com> wrote:
> 
>> without looking too deeply, you can modify that get_history() call to not 
>> emit any SQL, assuming you’re on 0.9 (maybe on 0.8 also but the API has been 
>> fixed up).
>> 
>> Take a look at passing the “passive” flag to get_history(), or alternatively 
>> using inspect().attrs.data.history:
>> 
>> http://docs.sqlalchemy.org/en/rel_0_9/changelog/migration_09.html#attributes-get-history-will-query-from-the-db-by-default-if-value-not-present
>> 
>> 
>> I’m not sure if you’re on 0.8 or 0.9 but I would think 0.9 if that’s 
>> emitting SQL?   again, haven’t looked closely.
>> 
>> if this fixes your issue we might want to update the recipe also, not sure.
>> 
>> 
>> 
>> On Jan 24, 2014, at 11:22 AM, Simon King <si...@simonking.org.uk> wrote:
>> 
>>> Hi again,
>>> 
>>> While testing the complicated relationship from the other thread, I
>>> noticed that I was generating a lot of queries that I couldn't
>>> immediately explain. After a bit of digging, it turned out that it was
>>> due to my use of code based on the "versioned objects" example at
>>> http://docs.sqlalchemy.org/en/rel_0_9/_modules/examples/versioned_history/history_meta.html.
>>> 
>>> In the test script below we have 3 classes. Users belong to Companies,
>>> and Companies reference a single Terms row. In the example, only the
>>> Company class is versioned.
>>> 
>>> The test script inserts a single user, company and terms row, then in
>>> new transaction loads the company and adds a new user. If you look at
>>> the SQL output, you should see that it loads the company.terms
>>> relationship, which doesn't seem like it should be necessary.
>>> 
>>> I think it's due to the following code from history_meta.py:
>>> 
>>>  if not obj_changed:
>>>      # not changed, but we have relationships.  OK
>>>      # check those too
>>>      for prop in obj_mapper.iterate_properties:
>>>          if isinstance(prop, RelationshipProperty) and \
>>>              attributes.get_history(obj, prop.key).has_changes():
>>>              for p in prop.local_columns:
>>>                  if p.foreign_keys:
>>>                      obj_changed = True
>>>                      break
>>>              if obj_changed is True:
>>>                  break
>>> 
>>> I can't figure out the circumstances when this should be necessary -
>>> can someone explain it to me?
>>> 
>>> Thanks a lot,
>>> 
>>> Simon
>>> 
>>> -- 
>>> You received this message because you are subscribed to the Google Groups 
>>> "sqlalchemy" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an 
>>> email to sqlalchemy+unsubscr...@googlegroups.com.
>>> To post to this group, send email to sqlalchemy@googlegroups.com.
>>> Visit this group at http://groups.google.com/group/sqlalchemy.
>>> For more options, visit https://groups.google.com/groups/opt_out.
>> 
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "sqlalchemy" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to sqlalchemy+unsubscr...@googlegroups.com.
> To post to this group, send email to sqlalchemy@googlegroups.com.
> Visit this group at http://groups.google.com/group/sqlalchemy.
> For more options, visit https://groups.google.com/groups/opt_out.

Attachment: signature.asc
Description: Message signed with OpenPGP using GPGMail

Reply via email to