[Dbix-class] order_by interfering with has_many fetch

[Darin McBride]

On Saturday November 14 2015 11:39:04 PM Peter Rabbitson wrote:
> On 11/14/2015 10:46 PM, Darin McBride wrote:
> > DBIx::Class::ResultSet::_construct_results(): Unable to properly collapse
> > has_many results in iterator mode due to order criteria - performed an
> > eager cursor slurp underneath. Consider using ->all() instead at
> > /data/Lacuna- Server/lib/Lacuna/DB/Result/Empire.pm line 622
...
> 
> It is (I suspect) a different issue, and if the case - the warning (it
> is not an error) needs better wording. If 'name' does NOT have a unique
> non-nullable constraint declared on it - i.e. you can have several 'me's
> with identical 'name's - then DBIC can not rely on the results coming
> from the DBI cursor to be "grouped in slabs" representing every object
> hierarchy "hanging off" every individual 'me' instance. This is why it
> needs to exhaust the cursor to give you a "certifiably correct" answer.

Maybe it's just my non-DBIC background, I just figured the DB was giving me 
everything in the right order already ;)  But I guess DBIC is trying to 
collapse objects so that multiple rows with those prefetches will return the 
same actual object reference?  Or something else entirely, which I don't 
understand :)

> > If it helps, Map::Body has 0 or 1 empires (can be null)
> 
> Your analysis is correct - the order on 'empire.id' ought to work, but
> this is extra (rather complicated) logic that currently is not
> implemented: note this TODO
> https://metacpan.org/source/RIBASUSHI/DBIx-Class-0.082820/t/prefetch/multipl
> e_hasmany_torture.t#L131-132

In my case, it doesn't really matter whether I sort on me.id or empire.id in 
addition, so I can live without it for now.

> > Any guidance on how to update this order_by to get DBIC to only fetch one
> > row at a time would be appreciated.
> 
> Simply order by [ me.name, me.<primary_key> ] <--- this will ensure
> things are ordered in a *stable* manner on the leftmost side, and the
> collapse will proceed correctly. A PR/patch amending the warning text
> and/or the prefetch documentation to make this clearer will be *very*
> much appreciated.

To me, me.name is a unique key, so that should indicate stability, no?  Now, I 
can fully appreciate that the actual definition of the table may not indicate 
to DBIC that it's a unique key, though I tried using add_unique_constraint for 
name, and that didn't help any.  (There is both an integer primary key and the 
name is also unique across all rows.)  So, other than primary_key, of which I 
already have one, what other method is used to determine that collapsing 
doesn't require a full scan?

As to the documentation, I'm not sure I understand it sufficiently myself to 
start proposing changes :)

> Please note that depending on your DBD, DBI may *still* be loading the
> entire resultset into memory even if DBIC uses ->next on the surface.
> For example DBD::Pg does this invariably, and DBD::mysql does it by
> default unless you set https://metacpan.org/pod/DBD::mysql#mysql_use_result

This is useful information.  Since we're using mysql, this may be a separate 
issue that I'll have to take up.  However, in the meantime, what I can do is 
what I can do, and adding 'me.id' does seem to get past this warning, which, I 
assume, means fewer perl objects created at a time.

Thanks,