Merge lp://staging/~iffy/storm/lazycolumns into lp://staging/storm

I'm glad you are working on this.

I have not done a full review at this point; I'd like to clarify
something though - it looks like you are making it so that all queries
will assume these columns are lazy. It is more efficient to grab the
column if needed in a single query rather than one-per-object. It
would be nice if users had the ability to control that.

For instance, say you have a changelog field in a table, which is a
big text field. Most pages in the website may not need it, but some
do. If its marked lazy, the pages that do need it will end up doing
(number of changelogs shown) separate queries to lazy-load the
changelog. If there is someway to say 'for this query, the changelog
field is needed', then that overhead can be eliminated.

And, if we have such a way to force laziness-into-eagerness, we
probably can do the reverse trivially - force eagerness into laziness
to prevent a bunch of fields being loaded on a query by query basis.

What do you think?

-Rob

Rob,

I know what you mean. I'm working on this to fix a problem we currently have with a big table (126 fields). 90% of the time, we need about 4 fields; 10% of the time we need some combination of the others.

My current patch allows you to define column laziness globally, which improves 90% of our code. What you're suggesting is that in addition to (instead of?) defining laziness globally, you can override laziness when using a store.find, store.get, Reference or ReferenceSet? This would allow one to avoid double queries for the other 10%.

What would the syntax look like?

store.find(Foo, eager=(Foo.name, Foo.kind))
bar = Reference(bar_id, 'Bar.id', eager=('Bar.name', 'Bar.kind'))

Or would you do some kind of state thing (this tastes bad)?

Bar.eagers.push('name', 'kind')
store.find(Bar)
Bar.eagers.pop()

Maybe this is better:

store.eager_load(Bar.name, Bar.kind)
store.find(Bar)
...
store.lazy_load(Bar.name, Bar.kind)

Also, is laziness overriding (your suggestion) something that should be built in from the beginning, or can it be added as a new feature after global laziness (my patch) is added? Maybe the answer depends on the implementation.

Thanks,

Matt

There are some notes in the TODO file that you might draw inspiration
from.

> There are some notes in the TODO file that you might draw inspiration
> from.

Yes, those are great notes! This branch implements laziness as described there.

On Wed, Jan 4, 2012 at 5:33 AM, Matt Haggard <email address hidden> wrote:
> Rob,
> Also, is laziness overriding (your suggestion) something that should be built in from the beginning, or can it be added as a new feature after global laziness (my patch) is added?  Maybe the answer depends on the implementation.

I think adding it later is fine, but we should consider what it needs
early, so that we don't conflict with the global laziness thing, which
will itself be useful.

As for syntax, perhaps
# do not load attribute1 or attribute2, and load attribute3 which is
globally set to lazy
store.find(Load(Foo, lazy=['attribute1', 'attribute2'], eager=['attribute3'])) ?

So what's the next step on this?

On Fri, Jan 6, 2012 at 11:19 AM, Matt Haggard <email address hidden> wrote:
> So what's the next step on this?

Well, I think for you to spend a little time coming up with an answer to
'does the patch need to change to gracefully permit per-field laziness
in future'

if the answer is no, then we review and land; if it is yes, you make
such changes as you consider necessary to future proof, then we review
and land.

-Rob

Rob,

No, this patch doesn't need to change to support per-query laziness in the future. Per-query laziness would use some of the stuff in this patch as it is. The overlap point is probably the LazyGroupMember class, which would probably be changed to Unloaded, NotLoadedYet, NotFetchedYet or Unfetched or something like that. But that's used internally and not in the public API.

Thanks for your help on this!

Matt

Thanks a lot for pushing this forward.

Given the question on how the syntax should look like, I'm concerned the
spirit described in the notes isn't being implemented. It doesn't seem
practical to define per field granularity for what should be loaded.

I'll have a look at the branch, hopefully next week.
On Jan 3, 2012 2:33 PM, "Matt Haggard" <email address hidden> wrote:

> Rob,
>
> I know what you mean. I'm working on this to fix a problem we currently
> have with a big table (126 fields). 90% of the time, we need about 4
> fields; 10% of the time we need some combination of the others.
>
> My current patch allows you to define column laziness globally, which
> improves 90% of our code. What you're suggesting is that in addition to
> (instead of?) defining laziness globally, you can override laziness when
> using a store.find, store.get, Reference or ReferenceSet? This would allow
> one to avoid double queries for the other 10%.
>
> What would the syntax look like?
>
> store.find(Foo, eager=(Foo.name, Foo.kind))
> bar = Reference(bar_id, 'Bar.id', eager=('Bar.name', 'Bar.kind'))
>
> Or would you do some kind of state thing (this tastes bad)?
>
> Bar.eagers.push('name', 'kind')
> store.find(Bar)
> Bar.eagers.pop()
>
> Maybe this is better:
>
> store.eager_load(Bar.name, Bar.kind)
> store.find(Bar)
> ...
> store.lazy_load(Bar.name, Bar.kind)
>
> Also, is laziness overriding (your suggestion) something that should be
> built in from the beginning, or can it be added as a new feature after
> global laziness (my patch) is added? Maybe the answer depends on the
> implementation.
>
>
> Thanks,
>
> Matt
> --
> https://code.launchpad.net/~magmatt/storm/lazycolumns/+merge/87092
> You are subscribed to branch lp:storm.
>

Gustavo,

I'm excited for your review.

To clarify, this branch implements laziness as described in the TODO. Rob mentioned that it would be nice to sometimes override that behavior per query. That (overriding per query) is not implemented here. So the syntax closely mirrors the TODO.

A single, lazy attr:

    class C(object):
        ...
        attr = Unicode(lazy=True)

or a group of lazy attrs (loaded together if either is accessed):

    class C(object):
        ...
        attr = Unicode(lazy=2)
        attr2 = Unicode(lazy=2)

    class D(object):
        ...
        attr = Int(lazy='group 1')
        attr2 = Int(lazy='group 2')

^ Is the syntax supported by this branch. I opted for a single "lazy" arg instead of having a "lazy" and "lazy_group." It seems simpler to me.