RIAS
asked on
SQL query written in a better way
Hi,
Any suggestion on how to make this query look better.
Thanks in advance
Any suggestion on how to make this query look better.
Thanks in advance
ALTER PROCEDURE [dbo].[sp123]
(
@DocIDType int = 0
)
AS
BEGIN
-- SET NOCOUNT ON added to prevent extra result sets from
-- interfering with SELECT statements.
SET NOCOUNT ON;
declare @SQL varchar(500) = ''
declare @GroupBy varchar(500) = ''
SET @SQL = 'SELECT DISTINCT ApprovalDocDisplay '
IF @DocIdtype = 1
BEGIN
SET @SQL = @SQL + ' FROM [vvApp] '
SET @SQL = @SQL + ' WHERE Email = '+ '''' + '' +''''
END
IF @DocIdtype = 0
BEGIN
SET @SQL = @SQL + ' FROM [vvApp] '
SET @SQL = @SQL + ' WHERE Email <> '+ '''' + '' +''''
END
SET @SQL = @SQL + ISNULL(@GroupBy,'')
EXEC(@SQL)
END
ASKER
Thanks will try and brb
ASKER
Thanks. But when I try to set a datatabase role in SQL SERVER I get this error:
key cannot be null
Parameter name key:System
key cannot be null
Parameter name key:System
Hi RIAS,
Kindly let me know whether the error you have mentioned is for the above procedure or something else..
Kindly let me know whether the error you have mentioned is for the above procedure or something else..
See your last thread..
BUT: Is this your entire procedure?? Then why using dynamic SQL at all?
Eventually you need an WITH RECOMPILE on large datasets.
BUT: Is this your entire procedure?? Then why using dynamic SQL at all?
ALTER PROCEDURE [dbo].[sp123] (
@DocIDType INT = 0
)
AS
SET NOCOUNT ON;
SELECT DISTINCT ApprovalDocDisplay
FROM [vvApp]
WHERE @DocIDType = 1
AND Email = 'Literal'
OR @DocIDType = 0
AND Email <> 'Literal';
Eventually you need an WITH RECOMPILE on large datasets.
Back in a few.... Will type it ups and check it out for you....
Initial thought is why GROUP BY when selecting distinct ?
And how are you passing @Groupby - I can see a declare, but nothing to populate - or is this just a sample extract ?
Initial thought is why GROUP BY when selecting distinct ?
And how are you passing @Groupby - I can see a declare, but nothing to populate - or is this just a sample extract ?
ASKER
Yes, Mark this a sample extract.
You can correct the query wherever you feel like.
You can correct the query wherever you feel like.
You can correct the query wherever you feel like.No, we - well, at lease I - cannot, cause the sample does not show the reason for dynamic SQL. Thus it is a useless exercise.
SOLUTION
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
ASKER CERTIFIED SOLUTION
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
(1) Get rid of all of the blank lines unless it divides major sections, e.g. The first seven lines can be rewritten as one line >> ALTER PROCEDURE dbo.sp123 (@DocIDType int = 0)
(2) Lose the BEGIN and END around the procedure as that's unnecessary
(3) Keep all lines within a BEGIN and END on the same left indent, unless there are other decision structures.
(4) Lose all the redundant spaces, i.e. SET @SQL = @SQL + ' FROM [vvApp] ' << can be >> SET @SQL = @SQL + ' FROM vvApp'
(5) Good Lord add some frEEaking meaningful code comments so the next guy that supports this has a fighting change to figure out what this code is without having do dig through it all.
(2) Lose the BEGIN and END around the procedure as that's unnecessary
(3) Keep all lines within a BEGIN and END on the same left indent, unless there are other decision structures.
(4) Lose all the redundant spaces, i.e. SET @SQL = @SQL + ' FROM [vvApp] ' << can be >> SET @SQL = @SQL + ' FROM vvApp'
(5) Good Lord add some frEEaking meaningful code comments so the next guy that supports this has a fighting change to figure out what this code is without having do dig through it all.
Back in an hour or two - relocating mobile "office" :)
ALTER PROCEDURE [dbo].[sp123]
(
@DocIDType int = 0
)
AS
SET NOCOUNT ON;
SELECT DISTINCT ApprovalDocDisplay
FROM [vvApp]
WHERE (@DocIDType = 1 AND Email = '') OR
(@DocIDType <> 1 AND Email > '')
(
@DocIDType int = 0
)
AS
SET NOCOUNT ON;
SELECT DISTINCT ApprovalDocDisplay
FROM [vvApp]
WHERE (@DocIDType = 1 AND Email = '') OR
(@DocIDType <> 1 AND Email > '')
ASKER
Sorry Experts was away for couple of days.
Thanks for all the help.
Thanks for all the help.
FYI, there is no place to pass parameters to your Email column in the original query and hence not available in the code below as well.
Open in new window