Tomas and Jan, I'm grateful that I have the opportunity to contribute to software that I use and genuinely like. My apologies again because my response was unprofessional and unhelpful. As I'm learning that kind of feedback is what I need.
Thanks for the explanation of the SQL, I still don't really get it yet but I'll review it again and google up on the joins. CC On Tue, Apr 13, 2010 at 4:40 PM, Tomas Lestach <tlest...@redhat.com> wrote: > ----- "Colin Coe" <colin....@gmail.com> wrote: > >> Tomas, apologies for the original reply. > > Did you write something, you should apologize for? > >> My SQL skills are quite weak and doing the query wit joins did not >> occur to me. Is using the joins more efficient? I can't see a >> difference in the result returned when I run either of these queries. > > Colin, > my SQL skills are similar to yours, so do not worry. That's why I consulted > the patch with our DB guru, before I wrote you. :-) > The outcome was: > - result of the sql query is the same > - even if the join version looks more gracefully, oracle shall optimize both > variants the same, so it's not true one of the queries would run faster > - BUT - the query looks much readable, if you first list the data you'd like > to get and then the tables you want to get the data from, than to have nested > selects > - Jan's argument is part of that > > The main reason I wrote you wasn't I'd want to criticize you or your work. It > was just a comment. Please, take it as a friendly reference. You can consider > it or not. > > We know the spw code is not perfect and you can find many examples, where > nested sql commands are used, where another constructions would fit better, > but our intention is to constantly improve it. And a good start is to write > code, that doesn't have to be corrected later on for any reason. > > So please, do not take our comments wrong. I just wanted to put near our > coding conventions to you. > > Your work IS appreciated. > > > Best Regards, > Tomas > >> >> Thanks >> >> CC >> >> On Mon, Apr 12, 2010 at 4:21 PM, Tomas Lestach <tlest...@redhat.com> >> wrote: >> > Hey Colin, >> > >> > why don't you just list all the data you need in the SELECT section >> and the tables that are necessary for the select in the FROM section? >> > >> > What I mean is to write: >> > >> > SELECT DISTINCT C.id, >> > C.name, >> > C.parent_channel AS parent_id, >> > C.label AS channel_label, >> > (SELECT COUNT(P.package_id) >> > FROM rhnChannelPackage P >> > WHERE P.channel_id = C.id >> > ) AS package_count, >> > CA.name AS arch_name >> > FROM rhnChannel C >> > inner join rhnChannelArch CA ON CA.ID = C.channel_arch_id >> > inner join rhnUserChannel UC ON UC.channel_id = C.id >> > left join rhnChannel C2 ON C2.parent_channel = C.id >> > WHERE UC.user_id = :user_id AND >> > (C.org_id = :org_id OR ( C2.id IS NOT NULL AND C2.org_id >> = :org_id )) >> > >> > >> > instead of: >> > >> > select Distinct C.id, >> > C.name, >> > C.parent_channel as parent_id, >> > C.label as channel_label, >> > (SELECT COUNT(P.package_id) >> > FROM rhnChannelPackage P >> > WHERE P.channel_id = C.id >> > ) AS package_count, >> > (select CA.name from rhnChannelArch CA where CA.ID >> = C.channel_arch_id) arch_name >> > from rhnChannel C inner join >> > rhnUserChannel UC on UC.channel_id = C.id left join >> > rhnChannel C2 on C2.parent_channel = C.id >> > where UC.user_id = :user_id AND >> > (C.org_id = :org_id or ( C2.id is not null AND C2.org_id >> = :org_id )) >> > >> > >> > >> > Regards, >> > Tomas >> > >> > -- >> > Tomas Lestach >> > RHN Satellite Engineering, Red Hat >> > >> > ----- "Colin Coe" <colin....@gmail.com> wrote: >> > >> >> OK, I think this patch is OK now. >> >> >> >> CC >> >> >> >> On Thu, Apr 8, 2010 at 9:10 PM, Colin Coe <colin....@gmail.com> >> >> wrote: >> >> > Hi Justin >> >> > >> >> > Is this a bit better? http://fpaste.org/LJwb/ >> >> > >> >> > Thanks >> >> > >> >> > CC >> >> > >> >> > >> >> > >> >> > On Thu, Apr 8, 2010 at 5:49 AM, Colin Coe <colin....@gmail.com> >> >> wrote: >> >> >> Hi Justin >> >> >> >> >> >> Thanks for the feedback. I'll have another look at this and >> >> re-submit. >> >> >> >> >> >> CC >> >> >> >> >> >> On Thu, Apr 8, 2010 at 1:21 AM, Justin Sherrill >> >> <jsher...@redhat.com> wrote: >> >> >>> On 4/6/10 9:55 AM, Colin Coe wrote: >> >> >>>> Hi all >> >> >>>> >> >> >>>> As per the API Addition page, I've added 'arch' to the >> >> >>>> channel.list*Channels API calls. >> >> >>>> >> >> >>>> Comments/criticisms welcome >> >> >>>> >> >> >>>> CC >> >> >>>> >> >> >>>> >> >> >>>> >> >> >>>> >> >> >>>> _______________________________________________ >> >> >>>> Spacewalk-devel mailing list >> >> >>>> Spacewalk-devel@redhat.com >> >> >>>> https://www.redhat.com/mailman/listinfo/spacewalk-devel >> >> >>> Hey Colin, >> >> >>> >> >> >>> Having it do the lookups of package Arch in the Serializer >> really >> >> isn't >> >> >>> the right way to do it as Serializers are just meant to >> translate >> >> the >> >> >>> existing data. >> >> >>> >> >> >>> I would either add joins to rhnChannelArch to get the label in >> >> each of >> >> >>> the queries or write a elaborator that just gets the channel >> arch >> >> and >> >> >>> make each of those queries use the elaborator. >> >> >>> >> >> >>> Thanks, >> >> >>> >> >> >>> -Justin >> >> >>> >> >> >>> >> >> >>> -- >> >> >>> Justin Sherrill, RHCA 1801 Varsity Drive. >> >> >>> Software Engineer Raleigh, NC 27603 >> >> >>> Red Hat, Inc. >> >> >>> >> >> >>> _______________________________________________ >> >> >>> Spacewalk-devel mailing list >> >> >>> Spacewalk-devel@redhat.com >> >> >>> https://www.redhat.com/mailman/listinfo/spacewalk-devel >> >> >>> >> >> >> >> >> > >> >> > >> >> > >> >> > -- >> >> > RHCE#805007969328369 >> >> > >> >> >> >> >> >> >> >> -- >> >> RHCE#805007969328369 >> >> >> >> _______________________________________________ >> >> Spacewalk-devel mailing list >> >> Spacewalk-devel@redhat.com >> >> https://www.redhat.com/mailman/listinfo/spacewalk-devel >> > >> > _______________________________________________ >> > Spacewalk-devel mailing list >> > Spacewalk-devel@redhat.com >> > https://www.redhat.com/mailman/listinfo/spacewalk-devel >> >> >> >> -- >> RHCE#805007969328369 >> >> _______________________________________________ >> Spacewalk-devel mailing list >> Spacewalk-devel@redhat.com >> https://www.redhat.com/mailman/listinfo/spacewalk-devel > > _______________________________________________ > Spacewalk-devel mailing list > Spacewalk-devel@redhat.com > https://www.redhat.com/mailman/listinfo/spacewalk-devel -- RHCE#805007969328369 _______________________________________________ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
