Friday, August 22, 2008

Show me your ID

One of the joys of working on software that has been outsourced to third parties, is fixing all the third party issues that only get discover after the support contract has expired.

Apart from having massive portions of duplicated code all over a payment system, I had to discover why some payments were going missing, and some were being assigned to the wrong businesses. After days of refactoring the code to remove all the duplication, I found this little nugget in one of the stored procedures...

insert into tbl_invoiceitems (invoice_id, list_id,voucher_id)

values (IDENT_CURRENT('tbl_invoice'), IDENT_CURRENT('tbl_directoryListing'), @vid)

Confused with is strange query, I decided to research this unusual Ident_Current. Basically it allows you to get the last identity insert into a table. That's fine if the stored procedure you are using has done that previous insert, but these stored procedures were called one after the other hoping to god that no other inserts were done between each other. They even returned the new inserted identities, but promptly forgot about them so they could hope that the identity was waiting in the right place in a queue in sql server. It's kind of like going to an airport baggae retrivval system, and grabbing the first case of the same colour. 9 times out of 10, it'll probably be yours and you can carry on your day. Occasionally however - someone runs off with the wrong case and everything goes tits up.

I even saw an attempt at rectifying the problem, where the developer had created the exact same procedure, adding a TBL_DircetoryListing identity property, but risking the invoice number column.

I recitified it by removing the IDENT_CURRENT clauses. Now I have to test the 30 combinations of the code repetition throughout the awful code (with slight changes that i'm almost too scared to fix)

Thursday, August 14, 2008

Optimizing SQL Queries

I was working on some code when I came across a behemoth of an inline SQL query nestled in the code behind of a page.

Select a.list_id, a.company_name, a.company_description, a.url,

( case when (Select upper(County_Name) from tbl_County where County_id=a.County_id) is null then

(Select upper(Country_Name) from tbl_description_Country where Country_id= a.Country_id)

else

(Select upper(County_Name) from tbl_County where County_id= a.County_id) end )

as County_name,

b.review_approval,

( case when (Select county_id from tbl_County where County_id = a.County_id)= 0 then

(Select Country_id from tbl_description_Country where Country_id = a.Country_id)

else

(Select county_id from tbl_County where County_id= a.County_id) end )

as CountryID,

dbo.funcGetRatingSum (a.list_id) as ratingSum,

dbo.funcGetRatingCount (a.list_id) as totalCount,

(Select case when (select top 1 banner_path from tbl_banners where list_id = a.list_id and a.subscription_id = 2) is null then

''

else

(select top 1 banner_path from tbl_banners where list_id = a.list_id and a.subscription_id = 2) end)

as bannerpath

from tbl_directorylisting a, tbl_preferences as b ,

tbl_companymaster c where a. status = 1 and(a.company_id =

b.company_id And a.status = 1) and c.category_id= @CategoryID

and a.company_id= c.company_id and a.subscription_id in (2,3) and subscription_date >= (dateadd(year,-1,getdate()))

Order by county_name, a.subscription_id asc, a.date_fee_paid

Actually it was within several lines of concatenated text values, and had no formatting whatsoever!. My first intention was try to make it a stored procedure, but doing so I couldn't help but try to optimize it a little. Here's my attempt....

Select

listing.list_id,

listing.company_name,

listing.company_description,

listing.url,

Coalesce(county.County_name, country.Country_Name) As County_name,

prefs.review_approval,

listing.Country_ID,

dbo.funcGetRatingSum (listing.list_id) as ratingSum,

dbo.funcGetRatingCount (listing.list_id) as totalCount,

Coalesce((select top 1 banner_path from tbl_banners where list_id = listing.list_id and listing.subscription_id = 2), '') as bannerpath

FROM

tbl_directorylisting listing

Inner Join tbl_preferences prefs

On listing.company_id = prefs.company_id

Inner Join tbl_companymaster company

On listing.company_id = company.company_id

Left Outer Join tbl_county county

On listing.county_id = county.county_id

Inner Join tbl_country country

On listing.country_id = country.country_id

where listing.status = 1

And company.category_id= @CategoryID

and listing.subscription_id in (2,3)

and listing.subscription_date >= (dateadd(year,-1,getdate()))

Order by county_name, listing.subscription_id asc, listing.date_fee_paid

Firstly you'll notice the second query is far easier to read (hopefully). I dont stand for the old skool method of joining tables, join with a join and it's obvious what is going on, plus it prepares isolation of results for the where clause afterwards.

Secondly there are fewer joins in the Column definition part of the select. I have included one of the original selects, because when optimizing the stored procedure it was siginificantly fast enough to use select top 1 ... than it was to Left Outer Join. I did however Coalesce so that the case statement didn't repeat iteself.

What do you think? Is that better or worse?

Wednesday, August 13, 2008

Welcome

Hi. I've been meaning to become a part of the online community for quite some time. I've had my anti-site online since 2000, and my Mum has been blogging for ages. I've helped out in the car industry, the property sector and now addressing weddings.

I am always trying to think up great ways of interacting and using the web like my online predictions league or, of course - noel's bum.

I have a big idea brewing at the moment, so watch this space for the announcement of a new code project in .NET and SQL Server.