Re: Inconsistencies in model names in queries
Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> Sun, 21 Apr 2013 20:23:23 +0200
| Newsgroups | gmane.comp.web.gallery.devel |
|---|---|
| Message-ID | <CA+z51A6n50Oqye7Q6c1hWUFrAvYJv=NJO1c_ZOgud9Q0SyHzrw@mail.gmail.com> |
--===============4528994074196416153== Content-Type: multipart/alternative; boundary=001a11c329e8623b5c04dae30d48 --001a11c329e8623b5c04dae30d48 Content-Type: text/plain; charset=UTF-8 Hey Bharat, Alright, I think it should be fixed. Can you try it again? As discussed, I patched Kohana's ORM class to make it work. I also added a note to the vendor repo about the patch and sent a pull request upstream: http://github.com/kohana/orm/pull/81 Take care, Shad On 21 April 2013 11:12, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote: > Hey gang, > > Good news: I found the issue. > > Bad news: it's not easily amended by making an override... > > The issue is in ORM::_initialize(). For setting _table_name and > _object_plural, it only sets them if not already defined (i.e. if > (empty($foo)) { $foo = "bar" }). We take advantage of this in our > ORM::_initialize() override by setting the _table_name our way (e.g. > "access_intents" instead of "accessintents"), which works. The problem is > that they don't do the same for _object_name. Drat. > > It looks like we have three options: > - Make an overriding _initialize() class that totally replaces this one. > This seems like a bad idea. > - Revamp all of our code and DB structure to use nameswithoutspaces. This > seems like a really bad idea. > - Patch the main ORM class for now and then submit it to the Kohana team > so it can go upstream. This seems like the least bad idea. > > I'm going to go ahead and get option 3 going. > > Take care, > Shad > > > > > On 20 April 2013 21:11, Bharat Mediratta <[email protected]> wrote: > >> Shad, >> >> I'm trying to plumb a line through the test code which means getting all >> the installer code working properly when you call >> Module::install("gallery"). Note that this is different from a fresh >> install because the fresh install just unpacks pre-prepared SQL. So >> eventually we call Access::_update_access_view_cache which bombs around >> line 553 when it's trying to find all albums that don't inherit access >> permission. I can get the query to work if I change it to this: >> >> 545 $query = ORM::factory("AccessIntent") >> >> 546 ->select("accessintent.$field", "items.left_ptr", >> "items.right_ptr", "items.id") >> 547 ->join("items")->on("items.id", "=", >> "accessintent.item_id") >> >> 548 ->where("left_ptr", ">=", $item->left_ptr) >> >> 549 ->where("right_ptr", "<=", $item->right_ptr) >> >> 550 ->where("type", "=", "album") >> >> 551 ->where("accessintent.$field", "IS NOT", Access::INHERIT) >> >> 552 ->order_by("level", "DESC") >> >> 553 ->find_all(); >> >> >> But what's frustrating about this is that we're mixing several different >> forms here. Line 545 has our camel cased model name. Line 546 has an >> intermediate form of the access intent table which is not camel cased, but >> is not pluralized. Then the items table *is* pluralized. Same thing on >> line 547. >> >> I haven't had time to dig into this, but my general feeling is that we're >> missing something and/or we've outsmarted ourselves here. In K2 there were >> only two forms - singular and plural and we'd always use the plural in >> database forms. In K3 we have this 3rd form which doesn't make as much >> sense. >> >> any thoughts on this? I'll try to circle back to it later today.. >> >> -Bharat >> >> >> ------------------------------------------------------------------------------ >> Precog is a next-generation analytics platform capable of advanced >> analytics on semi-structured data. The platform includes APIs for building >> apps and a phenomenal toolset for data science. Developers can use >> our toolset for easy data analysis & visualization. Get a free account! >> http://www2.precog.com/precogplatform/slashdotnewsletter >> __[ g a l l e r y - d e v e l ]_________________________ >> >> [ list info/archive --> http://gallery.sf.net/lists.php ] >> [ gallery info/FAQ/download --> http://gallery.sf.net ] >> > > --001a11c329e8623b5c04dae30d48 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr">Hey Bharat,<div><br></div><div style>Alright, I think it s= hould be fixed. =C2=A0Can you try it again?</div><div style><br></div><div = style>As discussed, I patched Kohana's ORM class to make it work. =C2= =A0I also added a note to the vendor repo about the patch and sent a pull r= equest upstream:</div> <div style><a href=3D"http://github.com/kohana/orm/pull/81">http://github.c= om/kohana/orm/pull/81</a><br></div><div style><br></div><div style>Take car= e,</div><div style>Shad</div><div style><br></div></div><div class=3D"gmail= _extra"> <br><br><div class=3D"gmail_quote">On 21 April 2013 11:12, Shad Laws <span = dir=3D"ltr"><<a href=3D"mailto:shad-xpYdmXCiSuZWk0Htik3J/[email protected]" target=3D"_blank">shad= @shadlaws.com</a>></span> wrote:<br><blockquote class=3D"gmail_quote" st= yle=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div dir=3D"ltr">Hey gang,<div><br></div><div>Good news: I found the issue.= </div><div><br></div><div>Bad news: it's not easily amended by making a= n override...</div><div><br></div><div>The issue is in ORM::_initialize(). = =C2=A0For setting _table_name and _object_plural, it only sets them if not = already defined (i.e. if (empty($foo)) { $foo =3D "bar" }). =C2= =A0We take advantage of this in our ORM::_initialize() override by setting = the _table_name our way (e.g. "access_intents" instead of "a= ccessintents"), which works. =C2=A0The problem is that they don't = do the same for _object_name. =C2=A0Drat.</div> <div><br></div><div>It looks like we have three options:</div><div>- Make a= n overriding _initialize() class that totally replaces this one. =C2=A0This= seems like a bad idea.</div><div>- Revamp all of our code and DB structure= to use nameswithoutspaces. =C2=A0This seems like a really bad idea.</div> <div>- Patch the main ORM class for now and then submit it to the Kohana te= am so it can go upstream. =C2=A0This seems like the least bad idea.</div><d= iv><br></div><div>I'm going to go ahead and get option 3 going.</div> <div><br></div><div>Take care,</div><div>Shad</div><div><br></div><div><br>= </div></div><div class=3D"gmail_extra"><br><br><div class=3D"gmail_quote">O= n 20 April 2013 21:11, Bharat Mediratta <span dir=3D"ltr"><<a href=3D"ma= ilto:[email protected]" target=3D"_blank">[email protected]</a>></span= > wrote:<br> <blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1p= x #ccc solid;padding-left:1ex"><div dir=3D"ltr"><div>Shad,</div><div><br></= div><div>I'm trying to plumb a line through the test code which means g= etting all the installer code working properly when you call Module::instal= l("gallery"). =C2=A0Note that this is different from a fresh inst= all because the fresh install just unpacks pre-prepared SQL. =C2=A0So event= ually we call Access::_update_access_view_cache which bombs around line 553= when it's trying to find all albums that don't inherit access perm= ission. =C2=A0I can get the query to work if I change it to this:</div> <div><br></div><div><div><font face=3D"courier new, monospace">=C2=A0 =C2= =A0545 =C2=A0 =C2=A0 =C2=A0$query =3D ORM::factory("AccessIntent"= ) =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0=C2=A0</font></div> <div><font face=3D"courier new, monospace">=C2=A0 =C2=A0546 =C2=A0 =C2=A0 = =C2=A0 =C2=A0->select("accessintent.$field", "items.left_= ptr", "items.right_ptr", "<a href=3D"http://items.id" t= arget=3D"_blank">items.id</a>") =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0</font></div> <div><font face=3D"courier new, monospace">=C2=A0 =C2=A0547 =C2=A0 =C2=A0 = =C2=A0 =C2=A0->join("items")->on("<a href=3D"http://it= ems.id" target=3D"_blank">items.id</a>", "=3D", "access= intent.item_id") =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0</font></div> <div><font face=3D"courier new, monospace">=C2=A0 =C2=A0548 =C2=A0 =C2=A0 = =C2=A0 =C2=A0->where("left_ptr", ">=3D", $item-&g= t;left_ptr) =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0</font= ></div><div> <font face=3D"courier new, monospace">=C2=A0 =C2=A0549 =C2=A0 =C2=A0 =C2=A0= =C2=A0->where("right_ptr", "<=3D", $item->rig= ht_ptr) =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0</font></div><di= v><font face=3D"courier new, monospace">=C2=A0 =C2=A0550 =C2=A0 =C2=A0 =C2= =A0 =C2=A0->where("type", "=3D", "album") = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0</font></div> <div><font face=3D"courier new, monospace">=C2=A0 =C2=A0551 =C2=A0 =C2=A0 = =C2=A0 =C2=A0->where("accessintent.$field", "IS NOT"= , Access::INHERIT) =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0=C2=A0</font></div><div><font face=3D"courier new, mono= space">=C2=A0 =C2=A0552 =C2=A0 =C2=A0 =C2=A0 =C2=A0->order_by("leve= l", "DESC") =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0</font></d= iv> <div><font face=3D"courier new, monospace">=C2=A0 =C2=A0553 =C2=A0 =C2=A0 = =C2=A0 =C2=A0->find_all(); =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0</font></div><div><br></div></d= iv><div> But what's frustrating about this is that we're mixing several diff= erent forms here. =C2=A0Line 545 has our camel cased model name. =C2=A0Line= 546 has an intermediate form of the access intent table which is not camel= cased, but is not pluralized. =C2=A0Then the items table <i>is</i>=C2=A0pl= uralized. =C2=A0Same thing on line 547.</div> <div><br></div><div>I haven't had time to dig into this, but my general= feeling is that we're missing something and/or we've outsmarted ou= rselves here. =C2=A0In K2 there were only two forms - singular and plural a= nd we'd always use the plural in database forms. =C2=A0In K3 we have th= is 3rd form which doesn't make as much sense.</div> <div><br></div><div>any thoughts on this? =C2=A0I'll try to circle back= to it later today..</div><span><font color=3D"#888888"><div><br></div><div= >-Bharat</div></font></span></div> <br>-----------------------------------------------------------------------= -------<br> Precog is a next-generation analytics platform capable of advanced<br> analytics on semi-structured data. The platform includes APIs for building<= br> apps and a phenomenal toolset for data science. Developers can use<br> our toolset for easy data analysis & visualization. Get a free account!= <br> <a href=3D"http://www2.precog.com/precogplatform/slashdotnewsletter" target= =3D"_blank">http://www2.precog.com/precogplatform/slashdotnewsletter</a><br= >__[ g a l l e r y - d e v e l ]_________________________<br> <br> [ list info/archive --> <a href=3D"http://gallery.sf.net/lists.php" targ= et=3D"_blank">http://gallery.sf.net/lists.php</a> ]<br> [ gallery info/FAQ/download --> <a href=3D"http://gallery.sf.net" target= =3D"_blank">http://gallery.sf.net</a> ]<br></blockquote></div><br></div> </blockquote></div><br></div> --001a11c329e8623b5c04dae30d48-- --===============4528994074196416153== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline ------------------------------------------------------------------------------ Precog is a next-generation analytics platform capable of advanced analytics on semi-structured data. The platform includes APIs for building apps and a phenomenal toolset for data science. Developers can use our toolset for easy data analysis & visualization. Get a free account! http://www2.precog.com/precogplatform/slashdotnewsletter --===============4528994074196416153== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline __[ g a l l e r y - d e v e l ]_________________________ [ list info/archive --> http://gallery.sf.net/lists.php ] [ gallery info/FAQ/download --> http://gallery.sf.net ] --===============4528994074196416153==--