March 25, 2008 at 3:30 pm
Like the subject + description says - I have (at least one) stored procedure that isn't performing well and I am looking to rewrite it. I have come up with something but just want my logic checked out.
Notes
- this is an inherited system!
- can't use sp_executesql or EXEC() - it will break ownership chaining
- names have been changed to protect the innocent
- I also intend creating an index on ApplicationNumber
Table Definition (as scripted from SQL Server):
[font="Courier New"]CREATE TABLE [dbo].[Application] (
[ApplicationID] [bigint] IDENTITY (1,1) NOT NULL ,
[ApplicationNumber] [varchar] (7) COLLATE Latin1_General_CI_AS NULL ,
[DocumentNumber] [varchar] (8) COLLATE Latin1_General_CI_AS NULL ,
[PreviousDocumentNumber] [varchar] (8) COLLATE Latin1_General_CI_AS NULL ,
[Referred] [char] (1) COLLATE Latin1_General_CI_AS NOT NULL CONSTRAINT [DF_Application_Referred] DEFAULT ('N'),
[ARID] [bigint] NULL ,
[CertifiedDate] [datetime] NULL ,
[CertifiedBy] [varchar] (50) COLLATE Latin1_General_CI_AS NULL ,
[Version] [datetime] NOT NULL CONSTRAINT [DF_Application_Version] DEFAULT (getdate()),
CONSTRAINT [PK_Document] PRIMARY KEY CLUSTERED
(
[ApplicationID]
) WITH FILLFACTOR = 90 ON [PRIMARY] ,
CONSTRAINT [AR_Document_FK1] FOREIGN KEY
(
[ARID]
) REFERENCES [AR] (
[ARID]
),
CONSTRAINT [CK_Application_Referred] CHECK ([Referred] = 'Y' or [Referred] = 'N')
) ON [PRIMARY]
GO
CREATE INDEX [IX_Application1] ON [dbo].[Application]([PreviousDocumentNumber], [ApplicationNumber]) WITH FILLFACTOR = 90 ON [PRIMARY]
GO
CREATE INDEX [IX_Application2] ON [dbo].[Application]([DocumentNumber]) WITH FILLFACTOR = 90 ON [PRIMARY]
GO
CREATE INDEX [IX_Application_ARID] ON [dbo].[Application]([ARID]) WITH FILLFACTOR = 90 ON [PRIMARY]
GO[/font]
Original query (@ApplicationNumber & @DocumentNumber passed in):
[font="Courier New"]SELECT
ApplicationID
, ARID
, ApplicationNumber
, DocumentNumber
, PreviousDocumentNumber
, Referred
, CertifiedDate
, CertifiedBy
, (select TOP 1 ApplicationNumber from Application a2 where a2.PreviousDocumentNumber = a.DocumentNumber) as ChildApplicationNumber
, (select count(ApplicationNumber) from Application a2 where a.DocumentNumber = a2.PreviousDocumentNumber) as HasParentApplication
, Version
FROM Application a
WHERE ( (@ApplicationNumber IS NULL) OR (ApplicationNumber = @ApplicationNumber) )
AND ( (@DocumentNumber IS NULL) OR (DocumentNumber = @DocumentNumber) )[/font]
Proposed Query:
[font="Courier New"]SELECT
a.ApplicationID
, a.ARID
, a.ApplicationNumber
, a.DocumentNumber
, a.PreviousDocumentNumber
, a.Referred
, a.CertifiedDate
, a.CertifiedBy
, a2.ApplicationNumber as ChildApplicationNumber
, count(a2.ApplicationNumber) as HasParentApplication
, a.Version
FROM dbo.Application a LEFT OUTER JOIN dbo.Application a2 ON a2.PreviousDocumentNumber = a.DocumentNumber
WHERE a.ApplicationNumber = @ApplicationNumber
AND a.DocumentNumber = @DocumentNumber
GROUP BY a.ApplicationID
, a.ARID
, a.ApplicationNumber
, a.DocumentNumber
, a.PreviousDocumentNumber
, a.Referred
, a.CertifiedDate
, a.CertifiedBy
, a2.ApplicationNumber
, a.Version
[/font]
Now, in order to deal with NULL values for either of the input parameters, we would either have separate stored procedures or use IF-THEN-ELSE constructs. That's not my issue.
I am wanting to confirm that the LEFT OUTER JOIN in use is suitable and that I can drop the TOP clause. In my testing, I get the same results for both queries. From what I have seen of the data, for any given application there is no more than one child application anyway, potentially rendering the TOP statement redundant. Or should I include it in the new statement anyway (just not part of a sub-clause).
And I do want to get rid of the COUNT() statement as well - it will only ever return 1 or 0, so some sort of NULL comparison on a2.ApplicationNumber can be substituted instead. Just haven't got around to it.
Thoughts please.
Thanks.
MARCUS. Why dost thou laugh? It fits not with this hour.
TITUS. Why, I have not another tear to shed;
--Titus Andronicus, William Shakespeare
March 26, 2008 at 12:37 am
Scott Duncan (3/25/2008)
CREATE INDEX [IX_Application2] ON [dbo].[Application]([DocumentNumber]) WITH FILLFACTOR = 90 ON [PRIMARY]
GO
Ignoring the query for now (will look at it after I've had some coffee)..
I would suggest you widen this index to cover ApplicationNumber. SQL's more likely to use one wide NC index than 2 narrow ones.
p.s. Do you have any sample data for the Application Table?
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
March 26, 2008 at 2:16 am
Without data I can't test, but your version does look like it should work. I would recommend seperate procs for the cases where the parameters are null, to avoid potential parameter sniffng issues
I can suggest one change that may improve things even more. There's no need to group by all the columns if we use a subquery in the from clause.
SELECT
a.PassportApplicationID
, a.ARID
, a.ApplicationNumber
, a.DocumentNumber
, a.PreviousDocumentNumber
, a.Referred
, a.CertifiedDate
, a.CertifiedBy
, a2.ChildApplicationNumber
, ISNULL(a2.HasParentApplication,0) AS HasParentApplication
, a.Version
FROM dbo.Application a
LEFT OUTER JOIN (SELECT ApplicationNumber AS ChildApplicationNumber, PreviousDocumentNumber,
count(a2.ApplicationNumber) AS HasParentApplication
FROM dbo.Application GROUP BY PreviousDocumentNumber, ApplicationNumber) a2
ON a2.PreviousDocumentNumber = a.DocumentNumber
WHERE a.ApplicationNumber = @ApplicationNumber
AND a.DocumentNumber = @DocumentNumber
For that I would also suggst an index on PreviousDocumentNumber, ApplicationNumber
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
March 26, 2008 at 2:20 pm
Thanks for the feedback Gail.
I forgot you can do joins like that (can you tell I don't do this sort of thing too often?).
There already is an index for PreviousDocumentNumber, ApplicationNumber so no problem there.
Fiddled about with adding ApplicationNumber to IX_Application2, but if you are only querying on one or the other of ApplicationNumber or DocumentNumber (from studying the usage patterns with Profiler, it's very rare to get them both specified), you can end up with an index scan instead of a seek, so will probably go with the separate index.
MARCUS. Why dost thou laugh? It fits not with this hour.
TITUS. Why, I have not another tear to shed;
--Titus Andronicus, William Shakespeare
March 26, 2008 at 3:56 pm
Thanks for the feedback Gail.
I forgot you can do joins like that (can you tell I don't do this sort of thing too often?).
There already is an index for PreviousDocumentNumber, ApplicationNumber so no problem there.
Fiddled about with adding ApplicationNumber to IX_Application2, but if you are only querying on one or the other of ApplicationNumber or DocumentNumber (from studying the usage patterns with Profiler, it's very rare to get them both specified), you can end up with an index scan instead of a seek, so will probably go with the separate index.
MARCUS. Why dost thou laugh? It fits not with this hour.
TITUS. Why, I have not another tear to shed;
--Titus Andronicus, William Shakespeare
March 27, 2008 at 1:00 am
Scott Duncan (3/26/2008)
Fiddled about with adding ApplicationNumber to IX_Application2, but if you are only querying on one or the other of ApplicationNumber or DocumentNumber (from studying the usage patterns with Profiler, it's very rare to get them both specified), you can end up with an index scan instead of a seek, so will probably go with the separate index.
OK. In that case, I'd suggest one index on ApplicationNumber, DocumentNumber and a second on DocumentNumber
That way, whether the filter is on one of them or both, you have an index that will satify the query.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
Viewing 6 posts - 1 through 5 (of 5 total)
You must be logged in to reply to this topic. Login to reply