CodFusion

Dave Ferguson wrote on 01/28/1010:47 AM

Nice one here. This is most definitely a DBA challenge but I'll take a stab anyways.

First off.. the "select *" is a bad idea. Your queries should always only return the columns of data you need.

But, beyond that, it is a poorly constructed query. If I am reading this correctly the end result, if using default params of null, would return all rows where the joins succeed. I if the joins are a 1 to many you would end up with multiple rows for each customer. I am not a DBA so I don't know the precise reason why but I think it is because the where clauses cancel each other out.

--Dave

Katrina wrote on 01/28/1011:38 AM

The main thing change the where clause to:

where isnull(cu.customerid, 0) = @CustomerID and
isnull(cu.personID, 0) = @PersonID


Avoid select *

I would also consider breaking it up into separate results sets so you are not returning as much duplicate data

create procedure CustomerLookup
@CustomerID int = NULL
,@PersonID int = NULL

as


declare @count int

select [columns]
from Sales.Customer CU
where isnull(cu.customerid, 0) = @CustomerID
   and isnull(cu.personID, 0) = @PersonID
set @count = @@rowcount

if @count > 0
begin

   select [columns]
   from sales.salesorderheader sh
      inner join sales.salesorderdetail sd on sd.salesorderID = sh.salesorderid
   --no longer need the isnull because rowcount was > 0
   where sh.customerid = @CustomerID

end else begin
   select 'No records' as Result
end

spills wrote on 01/28/1012:26 PM

--- Comment Code Block
--- Added Use Nocount
--- BEGIN and END
--- no * unless good reason
--- added COALESCE to where and set value correctly as it does't make sense in orig. form
---
create procedure CustomerLookup

@CustomerID int = NULL,
@PersonID int = NULL

SET NOCOUNT ON
AS
BEGIN

select cu.col, sh.col
from Sales.Customer cu
inner join sales.salesorderheader sh on sh.customerid = cu.customerid
inner join sales.salesorderdetail sd on sd.salesorderID = sh.salesorderid
where (cu.customerid = COALESCE(@CustomerID,cu.cusomerID) and
(cu.personID = COALESCE(@PersonID,cu.personID )
END

Hope I didn't go to fast!
SET NOCOUNT OFF

Katrina wrote on 01/28/1012:52 PM

Also, likely the use of PersonId is superfluous. It seems the only use case to use it would be if there are customers who are not people (i.e. businesses).

Why are NULLS allowed for the Customer? Unless a person who is not a customer can have a sales order. This seems unlikely.

The name of the sp is poor. It implies a query on only the Customer table...better CustomerSalesOrderLookup

so I would change my first answer:

create procedure CustomerLookup
@CustomerID int --No default
,@PersonID int = NULL

declare @count int

select [columns]
from Sales.Customer CU
where cu.customerid = @CustomerID
and isnull(cu.personID, 0) = @PersonID
set @count = @@rowcount

Katrina wrote on 01/28/103:44 PM

@spills

COALESCE ... Good catch!

I have used that before. Which means my WHERE clause is COMPLETELY WRONG.