----- "Colin Coe" <[email protected]> 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 <[email protected]>
> 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" <[email protected]> wrote:
> >
> >> OK, I think this patch is OK now.
> >>
> >> CC
> >>
> >> On Thu, Apr 8, 2010 at 9:10 PM, Colin Coe <[email protected]>
> >> 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 <[email protected]>
> >> 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
> >> <[email protected]> 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
> >> >>>> [email protected]
> >> >>>> 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
> >> >>> [email protected]
> >> >>> https://www.redhat.com/mailman/listinfo/spacewalk-devel
> >> >>>
> >> >>
> >> >
> >> >
> >> >
> >> > --
> >> > RHCE#805007969328369
> >> >
> >>
> >>
> >>
> >> --
> >> RHCE#805007969328369
> >>
> >> _______________________________________________
> >> Spacewalk-devel mailing list
> >> [email protected]
> >> https://www.redhat.com/mailman/listinfo/spacewalk-devel
> >
> > _______________________________________________
> > Spacewalk-devel mailing list
> > [email protected]
> > https://www.redhat.com/mailman/listinfo/spacewalk-devel
>
>
>
> --
> RHCE#805007969328369
>
> _______________________________________________
> Spacewalk-devel mailing list
> [email protected]
> https://www.redhat.com/mailman/listinfo/spacewalk-devel
_______________________________________________
Spacewalk-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/spacewalk-devel
