On Thu, May 30, 2013 at 3:45 PM, Michael Bayer <mike...@zzzcomputing.com> wrote:
>
> On May 30, 2013, at 2:28 PM, Claudio Freire <klaussfre...@gmail.com> wrote:
>
>>
>>> 4. it would have a super crapload of very complete and clean unit tests.
>>
>> Ehm... I would imagine all the current tests involving Query would
>> cover most of it. A few more cache-specific tests could be added
>> surely, but only to check caching is indeed happening, correctness
>> should be checked by existing tests already.
>
> well the current tests suffer very much from years of being integration tests 
> and not unit tests.   These tests would actually do a terrible job of testing 
> this cache, as almost all tests use a brand new mapping, a brand new session, 
> and emit just one query.  none of the things that can go wrong with caching, 
> such as the multiple result issue you're describing with the bake() recipe, 
> would be exercised by current tests.    Memory leaks, subtle changes in 
> queries, all that stuff.    It also sort of depends on how the feature comes 
> out, how hard it will be to verify its correctness.

Um... that might make the task a lot bigger than it should be. I'll
have to look into it.

>>> Otherwise, the bake() recipe as it is can be enhanced or augmented with 
>>> __hash__() methods and all that but I'm not aware of anything regarding it 
>>> that would require changes to SQLAlchemy itself, since it uses a Query 
>>> subclass.
>>
>> Well, yeah, I guess so. But that subclass would have to step on all of
>> Query methods to be able to compute, cache and update the hash
>> (computing it always would be almost as costly as compiling, so it has
>> to be cached in an instance attribute). That'd be a chore, and it
>> would break every other release.
>
> Can't a Query generate its hash from its current state, without generative 
> methods being called ?     Otherwise, the generative methods do run through a 
> common system, which is the @_generative decorator.

I'm not so familiar with Query internals yet to answer this. But I'll
look into it. I've been thinking, that if caching is conditional on
nothing structural about the query changing, and if we just want to
support that pattern I mentioned above (where you have a global query
object from which you build session-bound ones with "with_session"),
it could be as cheap as taking the internals' identity as hash. That
wouldn't work for the usual query building patterns, but then again,
when you build a new object, you're already paying a cost similar to
compiling, so caching would only really benefit the case where you
cache the expression externally.

In case I'm not clear, this would not be cached if I were to take id(internals)

query(Blah).filter(blah).join(blah).first()

But I don't care, because that's "expensive" on its own. This would:

q = query(Blah).filter(blah).join(blah)

...
q2 = q.with_session(S).params(blah).first()


On Thu, May 30, 2013 at 4:05 PM, Michael Bayer <mike...@zzzcomputing.com> wrote:
> On May 30, 2013, at 2:28 PM, Claudio Freire <klaussfre...@gmail.com> wrote:
>
>> On Thu, May 30, 2013 at 2:25 PM, Michael Bayer <mike...@zzzcomputing.com> 
>> wrote:
>>
>>> If you want to work on a feature that is actually going to change 
>>> SQLAlchemy, (and would that be before or after you finish #2720? :) ), it 
>>> would be:
>>
>> After, I didn't forget, just real life real work priorities made me
>> veer away from it. Since it was for 0.9, I judged I could safely delay
>> 2720 a bit while I take care of work related priorities ;-)
>
> also, I find an overhaul to Query such that it's self-hashing a lot more 
> interesting than #2720.  It would be a much bigger performance savings and it 
> would apply to other interpreters like pypy too.    Replacements of tiny 
> sections of code with C, not that interesting :) (redoing all the C in pyrex 
> is more interesting but not necessarily a priority).

The C extension is already done, and I think I sent the latest
version, haven't I?

The only thing remaining of 2720 is turning it all into pyrex code.

-- 
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?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to