Re: [Dbix-class] order_by interfering with has_many fetch
On 01/28/2018 09:51 PM, Darin McBride wrote: tl;dr version: we got past it, but I'm not sure if there is a DBIC metadata problem or not, and the whole project is moot now. Right. Re-reading your earlier debug info, I am now almost certain that this is where things went off the rails: https://github.com/Perl5/DBIx-Class/commit/f064a2abb158 This will probably take another 2~3 months to appear in its full glory on CPAN, but what you were observing fits neatly into that description. The related ticket is https://rt.cpan.org/Ticket/Display.html?id=107462, subscribe to it to get flagged when it is finally marked resolved :/ Thanks, though. And glad to see you're back :) Eh... it's all relative ;) Hope Lacuna 2.0 is just as cool as the other one ;) ___ 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@lists.scsys.co.uk
Re: [Dbix-class] order_by interfering with has_many fetch
On Sunday, January 28, 2018 8:36:36 AM MST Peter Rabbitson wrote: > On 11/16/2015 09:18 PM, Darin McBride wrote: > > On Monday November 16 2015 7:20:17 PM Peter Rabbitson wrote: > >> 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. > > Hi Darin! > > My notes around this time are a bit jumbled. I was wondering if you > could refresh my memory: did we ever fix this problem you were having? > > Thanks! tl;dr version: we got past it, but I'm not sure if there is a DBIC metadata problem or not, and the whole project is moot now. Basically, I have the 'me.id' in the order_by, and we shipped with it, though it looked like it may have been a major cause in some slowdowns (probably not DBIC, but just the whole model). And then I left the project, and then the project died (I'm not saying those two were related ;) ). I'm going to be trying to resurrect the project under a new name now that I have a new job, and the part of the code that was being affected by all this is going to be ripped out and not replaced, it was for a feature we're going to explicitly disallow in the resurrected project. Thanks, though. And glad to see you're back :) Thanks, Darin ___ 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@lists.scsys.co.uk
Re: [Dbix-class] order_by interfering with has_many fetch
On 11/16/2015 09:18 PM, Darin McBride wrote: On Monday November 16 2015 7:20:17 PM Peter Rabbitson wrote: 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. Hi Darin! My notes around this time are a bit jumbled. I was wondering if you could refresh my memory: did we ever fix this problem you were having? 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@lists.scsys.co.uk
Re: [Dbix-class] order_by interfering with has_many fetch
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@lists.scsys.co.uk
Re: [Dbix-class] order_by interfering with has_many fetch
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... (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. ___ 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@lists.scsys.co.uk
Re: [Dbix-class] order_by interfering with has_many fetch
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@lists.scsys.co.uk
Re: [Dbix-class] order_by interfering with has_many fetch
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 ) ] ___ 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@lists.scsys.co.uk
Re: [Dbix-class] order_by interfering with has_many fetch
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. ] <--- 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@lists.scsys.co.uk
Re: [Dbix-class] order_by interfering with has_many fetch
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@lists.scsys.co.uk
Re: [Dbix-class] order_by interfering with has_many fetch
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... ___ 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@lists.scsys.co.uk
Re: [Dbix-class] order_by interfering with has_many fetch
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@lists.scsys.co.uk
Re: [Dbix-class] order_by interfering with has_many fetch
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. ] <--- 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@lists.scsys.co.uk