launchpad-dev team mailing list archive

[Jamu Kakar <jamshed.kakar@xxxxxxxxxxxxx>]

Hi,

On Mon, Aug 2, 2010 at 6:12 PM, Jeroen Vermeulen <jtv@xxxxxxxxxxxxx> wrote:
> On 2010-08-02 22:52, Jamu Kakar wrote:
>> The way we implement the pattern in Landscape is, I think, a bit
>> simpler.  We don't end up with a very general solution like you have
>> with the implementation in Launchpad, but we don't have that many
>> objects we want collections for, so it's not really a problem.
>> Anyway, the pattern we use is to store the primitive values needed
>> to construct the query.  When collection.with_computer_ids(ids) is
>> invoked we store 'ids', instead of creating a Computer.id.is_in(ids)
>> expression and storing that.  Each collection must implement a
>> _get_result method that uses the primitive data that has been
>> gathered to generate a query.
>
> The idea with our Collection was that you can do this if you really want to,
> but it should be a distant second place to the simple, straightforward case.
>
> The simple case does not require anything but a one-liner with_computer_ids
> implementation.  But you pay a higher price if you do want to optimize the
> SQL: you'd make with_computer_ids store the ids list, and override select to
> take the ids list into account somehow.  I think that's similar to the
> Landscape approach.

We started by trying to do something like what you describe and it
quickly became a mess of tricky logic that was hard to understand,
so we stripped it down to what I described above.  In general, we've
benefited from being able to generate special-case queries in most
of the collection we have, so I'm not sure I'd consider it a "second
place" case.  The reality seems to be that you need to spell queries
differently, depending on the filtering need, to keep performance
up.

I'm not convinced that "override select to take ids list into
account somehow" is a great idea.  I think it'll lead to mixture of
models: you'll have a list of expressions, sometimes, with special
cases, sometimes, and then some glue to put it all back together.  I
suspect it'll be hard to understand.  Even though it isn't very
sophisticated I feel the model we use in Landscape is quite clean:
Everything before Collection.find() is invoked is about capturing
the values needed to generate the query.  Nothing more, nothing
less.  Everything that happens in Collection.find() is about
generating the query.  Once you understand that simple pattern you
can easily figure out how any collection in Landscape works, pretty
much by just reading the _get_result method.

Anyway, as I said, it's interesting to see different strategies
emerge for the pattern.  I'm curious to know how dealing with
special cases turns out.

Thanks,
J.