Re: Inconsistencies in model names in queries
Bharat Mediratta <[email protected]> Mon, 22 Apr 2013 10:10:07 -0700
| Newsgroups | gmane.comp.web.gallery.devel |
|---|---|
| Message-ID | <CAESa+_=hPec=CsOSKkhyumsUnzfnN-8Dw1CCdGRSpD1UwUY+CQ@mail.gmail.com> |
--===============0009759666778931985== Content-Type: multipart/alternative; boundary=0015175cfe8075b71d04daf62658 --0015175cfe8075b71d04daf62658 Content-Type: text/plain; charset=ISO-8859-1 Great - I'll try to give it a shot sometime today. On Sun, Apr 21, 2013 at 11:23 AM, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote: > 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 ] >>> >> >> > --0015175cfe8075b71d04daf62658 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr"><br><div style>Great - I'll try to give it a shot some= time today.</div></div><div class=3D"gmail_extra"><br><br><div class=3D"gma= il_quote">On Sun, Apr 21, 2013 at 11:23 AM, Shad Laws <span dir=3D"ltr"><= ;<a href=3D"mailto:shad-xpYdmXCiSuZWk0Htik3J/[email protected]" target=3D"_blank">shad-xpYdmXCiSuZWk0Htik3J/[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">Hey Bharat,<div><br></div><= div>Alright, I think it should be fixed. =A0Can you try it again?</div><div= ><br> </div><div>As discussed, I patched Kohana's ORM class to make it work. = =A0I also added a note to the vendor repo about the patch and sent a pull r= equest upstream:</div> <div><a href=3D"http://github.com/kohana/orm/pull/81" target=3D"_blank">htt= p://github.com/kohana/orm/pull/81</a><br></div><div><br></div><div>Take car= e,</div><div>Shad</div><div><br></div></div><div class=3D"HOEnZb"><div clas= s=3D"h5"> <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(). = =A0For setting _table_name and _object_plural, it only sets them if not alr= eady defined (i.e. if (empty($foo)) { $foo =3D "bar" }). =A0We ta= ke advantage of this in our ORM::_initialize() override by setting the _tab= le_name our way (e.g. "access_intents" instead of "accessint= ents"), which works. =A0The problem is that they don't do the same= for _object_name. =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. =A0This se= ems like a bad idea.</div><div>- Revamp all of our code and DB structure to= use nameswithoutspaces. =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. =A0This seems like the least bad idea.</div><div>= <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"). =A0Note that this is different from a fresh install= because the fresh install just unpacks pre-prepared SQL. =A0So 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= . =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">=A0 =A0545 = =A0 =A0 =A0$query =3D ORM::factory("AccessIntent") =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0=A0</font></div> <div><font face=3D"courier new, monospace">=A0 =A0546 =A0 =A0 =A0 =A0->s= elect("accessintent.$field", "items.left_ptr", "it= ems.right_ptr", "<a href=3D"http://items.id" target=3D"_blank">it= ems.id</a>") =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0</font></div> <div><font face=3D"courier new, monospace">=A0 =A0547 =A0 =A0 =A0 =A0->j= oin("items")->on("<a href=3D"http://items.id" target=3D"_= blank">items.id</a>", "=3D", "accessintent.item_id"= ;) =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0</font></div> <div><font face=3D"courier new, monospace">=A0 =A0548 =A0 =A0 =A0 =A0->w= here("left_ptr", ">=3D", $item->left_ptr) =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0</font></div><div> <font face=3D"courier new, monospace">=A0 =A0549 =A0 =A0 =A0 =A0->where(= "right_ptr", "<=3D", $item->right_ptr) =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0</f= ont></div><div><font face=3D"courier new, monospace">=A0 =A0550 =A0 =A0 =A0= =A0->where("type", "=3D", "album") =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=A0</font></div> <div><font face=3D"courier new, monospace">=A0 =A0551 =A0 =A0 =A0 =A0->w= here("accessintent.$field", "IS NOT", Access::INHERIT) = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=A0</font></div><div><f= ont face=3D"courier new, monospace">=A0 =A0552 =A0 =A0 =A0 =A0->order_by= ("level", "DESC") =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0=A0</font></div> <div><font face=3D"courier new, monospace">=A0 =A0553 =A0 =A0 =A0 =A0->f= ind_all(); =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0=A0</font></div><div><br></div></div><div> But what's frustrating about this is that we're mixing several diff= erent forms here. =A0Line 545 has our camel cased model name. =A0Line 546 h= as an intermediate form of the access intent table which is not camel cased= , but is not pluralized. =A0Then the items table <i>is</i>=A0pluralized. = =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. =A0In K2 there were only two forms - singular and plural and = we'd always use the plural in database forms. =A0In K3 we have this 3rd= form which doesn't make as much sense.</div> <div><br></div><div>any thoughts on this? =A0I'll try to circle back to= it later today..</div><span><font color=3D"#888888"><div><br></div><div>-B= harat</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> </div></div></blockquote></div><br></div> --0015175cfe8075b71d04daf62658-- --===============0009759666778931985== 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 --===============0009759666778931985== 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 ] --===============0009759666778931985==--