LazyLoader._load_for_state doesn't handle NEVER_SET correctly

Issue #2224 resolved
Former user created an issue

Attached is a test script that causes an error under 0.7, but works fine under 0.6 (1da499a838cb3cc50c98f7b32de6d35a09f00fe6 appears to be the earliest commit where it occurs). It looks to me like the error is caused by LazyLoader._load_for_state() attempting to do a query using NEVER_SET as the value of the primary key, resulting in an InterfaceError.

''(Disclaimer: the test script has a somewhat odd setup, but I believe it's exposing an unrelated underlying bug, which I'm not sure how to reproduce any other way. If this is in fact not a bug, and just caused by my use of really weird schema & relationship options, my apologies. )''

I tried to track this down myself, so I could submit a patch, but don't know enough about the internals to say for sure where the actual bug is. In case it helps point someone in the right direction, the following is what I was able to figure out...

  • The test script causes the error by flushing a change to an already flushed (but not committed) object; though the error will only occur if the mapper is set up a certain way.

  • The problem seems to start when UOWTransaction.get_attribute_history() is called with PASSIVE_OFF.

  • That calls ScalarObjectAttributeImpl.get_history(). As of 1da499a838cb3cc50c98f7b32de6d35a09f00fe6, this function treats PASSIVE_OFF like PASSIVE_RETURN_NEVER_SET.

  • get_history() then calls AttributeImpl.get(), which calls LazyLoader.load_for_state(), as stored in self.callable.

  • Once the call gets into LazyLoader._load_for_state(), it builds up a list of pk values in 'ident' (line 500 of orm/strategies.py as of bd45f22e171c4bdde037e6a03b6102347d9b224b). There's a check for ident containing PASSIVE_NO_RESULT, but not a check for NEVER_SET; so instead of returning NEVER_SET, it tries to query database, resulting in an interface error.

Adding a check for NEVER_SET where _load_for_state checks for PASSIVE_NO_RESULT (line 508 as of bd45f22e171c4bdde037e6a03b6102347d9b224b) seems to make the test script's error go away, but it didn't look to me like the callers (AttributeImpl.get, ScalarObjectAttributeImpl.get_history) were prepared for NEVER_SET as a return value, so more comprehensive fix is probably needed.

Hope the extra information was helpful.

Comments (6)

  1. Mike Bayer repo owner

    pretty bad regression, and its quite basic so sort of surprising this isn't coming up more. will review patch for a commit soon.

  2. Former user Account Deleted

    Wow, thanks for the quick turnaround on this!

    I guess I'll be using the default branch until 0.7.2 :)

  3. Log in to comment