order_by interfering with has_many fetch

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

order_by interfering with has_many fetch

Darin McBride
I'm getting an error message similar to an earlier message,
http://lists.scsys.co.uk/pipermail/dbix-class/2014-August/011746.html 

I've got code like this:

        $planet_rs =
            Lacuna->db->resultset('Map::Body')->
            search(
                   {
                       'sitterauths.sitter_id' => $real_empire->id,
                       'me.class' => { '!=' =>
'Lacuna::DB::Result::Map::Body::Planet::Station' },
                   },
                   {
                       join => { empire => 'sitterauths' },
                       prefetch => 'empire',
                       order_by => ['me.name'],
                   });

It's giving me the same error:

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

The earlier message implied that it would be fixed in 0.082700_05, but I'm
using 0.082810 at the moment.  So, either it wasn't fixed, or I'm encountering
something different.

Further searching uncovers https://www.mail-archive.com/dbix-class@.../msg07082.html - in which Peter Rabbitson indicates
that I would 'need to order_by "leftmost part" for this message to go away' --
but I don't understand what "leftmost part" means in this context.

If it helps, Map::Body has 0 or 1 empires (can be null), and sitterauths has a
dual-column primary key, referring to two empire through their IDs.  So, while
ordering by empire is neither helpful nor hurtful to me, ordering by
sitterauth makes no sense (though it will likely not harm me, either, if I
sort in there).  However, I've tried an order_by of [ 'empire.id', 'me.name'
], or even [ 'empire.id','sitterauths.sitter_id','me.name' ], and still end up
with the error message.

I'm expecting that once I roll this out into production and it gets wide use,
we'll have a thousand rows ("Map::Body" objects) being returned, and up to 40
or 50 empires, so I'd rather not resort to ->all to save some memory in the
Plack processes.

Any guidance on how to update this order_by to get DBIC to only fetch one row
at a time would be appreciated.

_______________________________________________
List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
IRC: irc.perl.org#dbix-class
SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
Searchable Archive: http://www.grokbase.com/group/dbix-class@...
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: order_by interfering with has_many fetch

Peter Rabbitson-2
On 11/14/2015 10:46 PM, Darin McBride wrote:
> ...
>   order_by => ['me.name'],
> ...
>
...
> 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
>
> The earlier message implied that it would be fixed in 0.082700_05, but I'm
> using 0.082810 at the moment.  So, either it wasn't fixed, or I'm encountering
> something different.

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.

> 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/multiple_hasmany_torture.t#L131-132

> 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.

> I'm expecting that once I roll this out into production and it gets wide use,
> we'll have a thousand rows ("Map::Body" objects) being returned, and up to 40
> or 50 empires, so I'd rather not resort to ->all to save some memory in the
> Plack processes.

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

Hope this is sufficient to get you going

Cheers!


_______________________________________________
List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
IRC: irc.perl.org#dbix-class
SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
Searchable Archive: http://www.grokbase.com/group/dbix-class@...
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: 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,

_______________________________________________
List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
IRC: irc.perl.org#dbix-class
SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
Searchable Archive: http://www.grokbase.com/group/dbix-class@...
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: order_by interfering with has_many fetch

Darin McBride
On Monday November 16 2015 1:17:55 AM Peter Rabbitson wrote:

> On 11/15/2015 11:23 PM, Darin McBride wrote:
> > 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 :)
>
> I will explain this separately, however:
> > 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.
>
> Are you saying that you did:
>
> __PACKAGE__->add-unique_constraint( ... => [ 'name' ]);
>
>     AND
>
> The 'name' column is *NOT* marked as is_nullable => 1 in the DBIC metadata
>
>     AND
>
>   you *still* got the warning?
>
> If this is the case - this is in fact a rather serious bug and I need to
> investigate this further...

Please don't.  I must have done something wrong the first time.  When I try
again, because I have to reverify everything when I'm not 100% sure, I can add
that unique constraint and remove the me.id on the order, and suddenly it
works without a warning.  Sorry for the distress.

_______________________________________________
List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
IRC: irc.perl.org#dbix-class
SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
Searchable Archive: http://www.grokbase.com/group/dbix-class@...
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: order_by interfering with has_many fetch

Darin McBride
On Sunday November 15 2015 7:09:05 PM Darin McBride wrote:

> > Are you saying that you did:
> >
> >
> > __PACKAGE__->add-unique_constraint( ... => [ 'name' ]);
> >
> >
> >     AND
> >
> >
> > The 'name' column is *NOT* marked as is_nullable => 1 in the DBIC metadata
> >
> >
> >     AND
> >
> >   you *still* got the warning?
> >
> >
> > If this is the case - this is in fact a rather serious bug and I need to
> > investigate this further...
>
> Please don't.  I must have done something wrong the first time.  When I try
> again, because I have to reverify everything when I'm not 100% sure, I can
> add that unique constraint and remove the me.id on the order, and suddenly
> it works without a warning.  Sorry for the distress.

And I keep testing and I got this - helps if I turn off DBIC_TRACE, makes the
other warnings show up better (i.e., not hide behind excess noise).

So, the column is:

__PACKAGE__->add_columns(
    name                    => { data_type => 'varchar', size => 30,
is_nullable => 0 },

So, explicitly not nullable.

I've added this code as the next executable line after all the columns:

__PACKAGE__->add_unique_constraint(name => ['name']);

And now my search (which has grown since last time - I'm now prefetching
sitterauths, too, since I'm going to need a field from there for each empire
returned) looks like this:

        $planet_rs =
            Lacuna->db->resultset('Map::Body')->
            search(
                   {
                       'sitterauths.sitter_id' => $real_empire->id,
                       'me.class' => { '!=' =>
'Lacuna::DB::Result::Map::Body::Planet::Station' },
                   },
                   {
                       join => { empire => 'sitterauths' },
                       prefetch => { 'empire', 'sitterauths' },
                       order_by => ['me.name'],
                   });

And I still get the unable to properly collapse error.  Adding the 'me.id'
back in as a secondary order_by key resolves that again.  (Perl 5.12.1, DBIC
0.082810.)

I'm completely expecting that I'm doing something wrong here, though I do
wonder what.

Thanks,

_______________________________________________
List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
IRC: irc.perl.org#dbix-class
SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
Searchable Archive: http://www.grokbase.com/group/dbix-class@...
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: order_by interfering with has_many fetch

Darin McBride
On Monday November 16 2015 10:08:13 AM Peter Rabbitson wrote:

> On 11/16/2015 06:20 AM, Darin McBride wrote:
> > On Sunday November 15 2015 7:09:05 PM Darin McBride wrote:
> > So, the column is:
> >
> > __PACKAGE__->add_columns(
> >
> >      name                    => { data_type => 'varchar', size => 30,
> >
> > is_nullable => 0 },
> >
> > So, explicitly not nullable.
> >
> > I've added this code as the next executable line after all the columns:
> >
> > __PACKAGE__->add_unique_constraint(name => ['name']);
> >
> > And now my search (which has grown since last time - I'm now prefetching
> > sitterauths, too, since I'm going to need a field from there for each
> > empire>
> > returned) looks like this:
> >          $planet_rs =
> >          
> >              Lacuna->db->resultset('Map::Body')->
> >              search(
> >              
> >                     {
> >                    
> >                         'sitterauths.sitter_id' => $real_empire->id,
> >                         'me.class' => { '!=' =>
> >
> > 'Lacuna::DB::Result::Map::Body::Planet::Station' },
> >
> >                     },
> >                     {
> >                    
> >                         join => { empire => 'sitterauths' },
> >                         prefetch => { 'empire', 'sitterauths' },
> >                         order_by => ['me.name'],
> >                    
> >                     });
>
> Please use the resultset exactly as defined above, execute the following
> and get me its result:
>
>
> use Devel::Dwarn;
> $Data::Dumper::Maxdepth = 3;
> Dwarn [
>   $planet_rs->result_source
>              ->schema
>               ->storage
>                ->_extract_colinfo_of_stable_main_source_order_by_portion(
>                  $planet_rs->_resolved_attrs
>                )
> ]

[]

If I put me.id back in, then it gives me a hash of the two keys and other
interesting things, but without me.id, so exactly as defined above, it's empty.

Just double checking that I still have the unique constraint defined, and I do.  
(It's not defined as a unique constraint in the db though, I was going to add
that later, since I suspect we'll get some minor speed improvement from it.)

Thanks,

_______________________________________________
List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
IRC: irc.perl.org#dbix-class
SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
Searchable Archive: http://www.grokbase.com/group/dbix-class@...
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: order_by interfering with has_many fetch

Darin McBride
On Monday November 16 2015 7:20:17 PM Peter Rabbitson wrote:

> On 11/16/2015 07:16 PM, Darin McBride wrote:
> > On Monday November 16 2015 10:08:13 AM Peter Rabbitson wrote:
> >> On 11/16/2015 06:20 AM, Darin McBride wrote:
> >>> On Sunday November 15 2015 7:09:05 PM Darin McBride wrote:
> >>> So, the column is:
> >>>
> >>> __PACKAGE__->add_columns(
> >>>
> >>>       name                    => { data_type => 'varchar', size => 30,
> >>>
> >>> is_nullable => 0 },
> >>>
> >>> So, explicitly not nullable.
> >>>
> >>> I've added this code as the next executable line after all the columns:
> >>>
> >>> __PACKAGE__->add_unique_constraint(name => ['name']);
> >>>
> >>> And now my search (which has grown since last time - I'm now prefetching
> >>> sitterauths, too, since I'm going to need a field from there for each
> >>> empire>
> >>>
> >>> returned) looks like this:
> >>>           $planet_rs =
> >>>          
> >>>               Lacuna->db->resultset('Map::Body')->
> >>>               search(
> >>>              
> >>>                      {
> >>>                      
> >>>                          'sitterauths.sitter_id' => $real_empire->id,
> >>>                          'me.class' => { '!=' =>
> >>>
> >>> 'Lacuna::DB::Result::Map::Body::Planet::Station' },
> >>>
> >>>                      },
> >>>                      {
> >>>                      
> >>>                          join => { empire => 'sitterauths' },
> >>>                          prefetch => { 'empire', 'sitterauths' },
> >>>                          order_by => ['me.name'],
> >>>                      
> >>>                      });
> >>
> >> Please use the resultset exactly as defined above, execute the following
> >> and get me its result:
> >>
> >>
> >> use Devel::Dwarn;
> >> $Data::Dumper::Maxdepth = 3;
> >> Dwarn [
> >>
> >>    $planet_rs->result_source
> >>    
> >>               ->schema
> >>              
> >>                ->storage
> >>                
> >>                 ->_extract_colinfo_of_stable_main_source_order_by_portion
> >>                 (
> >>                
> >>                   $planet_rs->_resolved_attrs
> >>                
> >>                 )
> >>
> >> ]
>
> What does this say then:
>
> Dwarn { $planet_rs->result_source->unique_constraints }
>
> Something screwy is going on with the metadata layer...


{
  primary => [
    "id"
  ]
}

This makes it look like this line:

# tell DBIC that this is unique so it can be sorted properly when dealing
# with has_many, prefetch, and order_by
__PACKAGE__->add_unique_constraint(name => ['name']);

isn't doing anything.  Though, when I change it so the second parameter isn't
an array reference anymore, I do get a nasty error message telling me I messed
it up, and nothing else happens (it dies), so I'm pretty sure it's being
called before the above Dwarn :)

As I'm reading the docs more, I also notice that prefetch is another name for
join, so I'm removing the explicit join to just rely on the prefetch for
telling DBIC to perform the join.  Not that this seems to affect the output at
all, I'm just being clear so that if I paste the code in again in the future,
you'll know why it has changed.

I also updated to DBIC 0.082820 on my dev system just in case it made any
difference.  You probably knew it wouldn't, but it can't hurt.  (If it does
make a difference, I'll have to upgrade the servers, too.)

>
> > (It's not defined as a unique constraint in the db though, I was going to
> > add that later, since I suspect we'll get some minor speed improvement
> > from it.)
> It is not relevant whether the DB itself has it. It is perfectly fine to
> "lie" to DBIC as long as the thing exists logically.

That's what I figured, which is why I wasn't going to worry about it quite yet.  
I like consistency, but things like this are regularly not consistent while
still developing :)


_______________________________________________
List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
IRC: irc.perl.org#dbix-class
SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
Searchable Archive: http://www.grokbase.com/group/dbix-class@...
Loading...