0
\$\begingroup\$

Can this query be improved? Is there a way to eliminate the duplicate function call?

-- a Special Group has many Items which have many Bookings
create function f_BookingsForSpecialGroup(@specialGroupId varchar(99))
returns table as
    return select * from v_Booking where ITEM_CODE in
        (select ITEM_CODE from v_Item i where i.SPECIAL_GROUP_ID = @specialGroupId);
go


create view v_SpecialGroup as
select (NON_BAD_BOOKINGS - PAID_BOOKINGS) as PENDING_BOOKINGS, * from
(
select
    (select count(*) from f_BookingsForSpecialGroup(g.SPECIAL_GROUP_ID) where IS_BAD=0) as NON_BAD_BOOKINGS,
    (select count(*) from f_BookingsForSpecialGroup(g.SPECIAL_GROUP_ID) where IS_PAID=1) as PAID_BOOKINGS,
    *
from SPECIAL_GROUP g
) g
go

The view v_SpecialGroup is never queried directly by the application; it is used to build other views which select individual columns as needed. (You can think of v_SpecialGroup as a "base view" which exists solely to augment the SPECIAL_GROUP table. I have profiled this strategy and it seems that if you don't select the more expensive columns you don't pay for them, but I could be wrong of course...)

\$\endgroup\$
1
\$\begingroup\$

It looks to me like the view returns all of the columns from a SPECIAL_GROUP table, along with an additional column counting bookings that are neither is_bad nor is_paid.

If so, you could use a common table expression* to simplify the logic a bit:

create view v_SpecialGroup as

with bookings_by_group as (
    select  i.SPECIAL_GROUP_ID,
            count(case when is_bad = 0 then 1 end) as NON_BAD_BOOKINGS,
            count(case when is_paid = 1 then 1 end) as PAID_BOOKINGS
    from    v_booking b join v_item i on b.ITEM_CODE = i.ITEM_CODE
    group by i.SPECIAL_GROUP_ID)
select  NON_BAD_BOOKINGS - PAID_BOOKINGS as PENDING_BOOKINGS,
        g.*
from    bookings_by_group b join SPECIAL_GROUP g on b.SPECIAL_GROUP_ID = g.SPECIAL_GROUP_ID

ps. It might also be possible to condense the logic further:

count(case when is_bad = 0 and is_paid != 1 then 1 end) as PENDING_BOOKINGS

it would depend on whether you had any bookings that were both is_bad and is_paid

*SQL Server must be >= 2008R2

\$\endgroup\$
  • \$\begingroup\$ This looks good. I came up with something similar using a cross join instead of a CTE. I will have to research the significance of using one vs the other. \$\endgroup\$ – default.kramer Jan 27 '16 at 2:07
  • \$\begingroup\$ You could compare the execution plans, you may well find that the optimiser executes both syntaxes in exactly the same way. \$\endgroup\$ – paul Jan 27 '16 at 15:01
  • \$\begingroup\$ It turns out the CTE actually performed better whether or not I used the table-valued function. Very nice! \$\endgroup\$ – default.kramer Jan 28 '16 at 14:47
  • \$\begingroup\$ CTEs were introduced in SQL Server 2005. \$\endgroup\$ – Martin Smith Jan 31 '16 at 12:44
2
\$\begingroup\$

Yes, there is a way to eliminate the repeated function call, but as to whether or not it would improve performance you need to benchmark it. This method writes the resulting data into a temporary table, which means it will call the function only once when populating it, but in turn will mean that it will have to write the table to tempdb for that session. Here is how it could be written that way:

if object_id('tempdb..#SpecialBookings') is not null
    drop table tempdb..#SpecialBookings;

select *
into #SpecialBookings
from SPECIAL_GROUP as grp
    cross apply f_BookingsForSpecialGroup(grp.SPECIAL_GROUP_ID) as bookings
where (bookings.IS_BAD = 0 or bookings.IS_PAID = 1);

select (grpCounted.NON_BAD_BOOKINGS - grpCounted.PAID_BOOKINGS) as PENDING_BOOKINGS, * 
from 
(
    select
        (select count(*) from #SpecialBookings where IS_BAD=0) as NON_BAD_BOOKINGS,
        (select count(*) from #SpecialBookings where IS_PAID=1) as PAID_BOOKINGS,
        *
    from #SpecialBookings
) as grpCounted;

if object_id('tempdb..#SpecialBookings') is not null
    drop table tempdb..#SpecialBookings;

select *

Do you really need all the fields from both the SPECIAL_GROUP table and the f_BookingsForSpecialGroup? If you do really need them, then fair enough, but otherwise, you should not use select *.


Aliases

As you can see I renamed your table aliases a bit to make your code easier to read. Whenever possible, avoid single-letter or otherwise meaningless aliases as they make the code less clear.

\$\endgroup\$
  • \$\begingroup\$ Thanks, I think a cross apply is exactly what I need. And I think I can get by without a temp table... \$\endgroup\$ – default.kramer Jan 26 '16 at 22:17
  • \$\begingroup\$ OK good, glad I could help! \$\endgroup\$ – Phrancis Jan 26 '16 at 23:15
2
\$\begingroup\$

One way is to have a function that returns a table composed of two INTs, corresponding to each of your COUNTs:

BadCount INT, PaidCount INT

This function will compute both COUNTs based on a SPECIAL_GROUP_ID, so it should like this:

CREATE FUNCTION dbo.ufnGetContactInformation(@SPECIAL_GROUP_ID INT)
RETURNS @CountInfo TABLE 
(

    ContactID int PRIMARY KEY NOT NULL, 
    FirstName nvarchar(50) NULL, 
)
-- function body comes here

However, keep in mind that your function will be called for each row returned from SPECIAL_GROUP (clearly visible in profiler, estimated/actual plan does not show function calls, if I remember correctly), so performance might be affected.

Also, * should be avoided since it can lead to performance problems (selecting all columns may inhibit indexes usage) and also may also lead to unexpected results (changing table structure without procedure recompilation, means that * does not actually brings you all the columns).

If possible, please provide code from f_BookingsForSpecialGroup - maybe it can be rewritten to be more set based.

\$\endgroup\$
  • \$\begingroup\$ Good point, I have updated the question to show f_BookingsForSpecialGroup \$\endgroup\$ – default.kramer Jan 26 '16 at 21:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.