Is There A Better Way Of Writing This Stored Procedure?

Hi Experts,
I've just started learning to write Stored Procedure. The stored procedure listed below is working fine. I would like to know if there is anything that I should change to improve it?

The stored procedure will first construct a SQL string based on user input: Field1, Field2, Value1, Value2, Operator1, Condition1 and Condition2.

It then checks the total length of Value1 and Value2. If it equals 0, this means that this is the first time the page loads. So, it will execute the first SQL string. It length is more than 0 it will execute the second SQL string.

ALTER PROCEDURE dbo.test1
@Field1 varchar(50)='', /*Source = DropDownList1*/
@Field2 varchar(50)='', /*Source = DropDownList2*/
@Value1 varchar(50)='', /*Source = TextBox1*/
@Value2 varchar(50)='', /*Source = TextBox2*/
@Condition1 varchar(15)= ' OR ', /*Source = RadioButtons1*/
@Operator1 int=0, /*Source = DropDownList3*/
@Operator2 int=0, /*Source = DropDownList4*/
@startRowIndex varchar(5)='', /*Source = GridView1*/
@maximumRows varchar(5)='' /*Source = GridView1*/

AS
Begin
DECLARE @SqlLoad nvarchar(255)
DECLARE @SqlSearch nvarchar(2000)
DECLARE @Field1SQL varchar(255)
DECLARE @Field2SQL varchar(255)

      IF @Operator1=0
            BEGIN
            SET @Field1SQL=' (' + @Field1 + '  =''' + @Value1 + ''' OR ''' + @Value1 + ''' IS NULL) '
            END
      ELSE IF @Operator1=1
            BEGIN
            SET @Field1SQL=' ([' + @Field1 + '] LIKE ''%' + @Value1 + '%'') '
            END
      ELSE IF @Operator1=2
            BEGIN
            SET @Field1SQL=' ([' + @Field1 + '] NOT LIKE ''%' + @Value1 + '%'') '
      END

      IF @Operator2=0
            BEGIN
            SET @Field2SQL=' (' + @Field2 + '=''' + @Value2 + ''' OR ''' + @Value2 + ''' IS NULL)'
            END
      ELSE IF @Operator2=1
            BEGIN
            SET @Field2SQL=' ([' + @Field2 + '] LIKE ''%' + @Value2 + '%'') '
            END
      ELSE IF @Operator2=2
            BEGIN
            SET @Field2SQL=' ([' + @Field2 + '] NOT LIKE ''%' + @Value2 + '%'') '
      END

SET @SqlLoad = 'SELECT * FROM (SELECT *, ROW_NUMBER() OVER (ORDER BY FirstName) AS RowRank
 FROM Authors) As AuthorsWithRowNumbers
WHERE (RowRank >' + @startRowIndex + ') AND (RowRank <= (' + @startRowIndex + '+' + @maximumRows +'))'

SET @SqlSearch = 'SELECT * FROM (SELECT *, ROW_NUMBER() OVER (ORDER BY FirstName) AS RowRank
 FROM Authors WHERE ('  + @Field1SQL + @Condition1 + @Field2SQL + ')) As AuthorsWithRowNumbers
WHERE (RowRank >' + @startRowIndex + ') AND (RowRank <= (' + @startRowIndex + '+' + @maximumRows +'))'

IF (LEN(@Value1) + LEN(@Value2))=0
      EXEC sp_executesql @SqlLoad
ELSE
      EXEC sp_executesql @SqlSearch
END

Thanks
noobe1Asked:
Who is Participating?
 
dqmqConnect With a Mentor Commented:
Perhaps some cosmetics:

ALTER PROCEDURE dbo.test1
@Field1 varchar(50)='', /*Source = DropDownList1*/
@Field2 varchar(50)='', /*Source = DropDownList2*/
@Value1 varchar(50)='', /*Source = TextBox1*/
@Value2 varchar(50)='', /*Source = TextBox2*/
@Condition1 varchar(15)= ' OR ', /*Source = RadioButtons1*/
@Operator1 int=0, /*Source = DropDownList3*/
@Operator2 int=0, /*Source = DropDownList4*/
@startRowIndex varchar(5)='', /*Source = GridView1*/
@maximumRows varchar(5)='' /*Source = GridView1*/
 
AS
Begin
DECLARE @SqlLoad nvarchar(2000)
DECLARE @SqlSearch nvarchar(2000)
DECLARE @Field1SQL varchar(255)
DECLARE @Field2SQL varchar(255)
 
      IF @Operator1=0
            BEGIN
            SET @Field1SQL=' (' + @Field1 + '  =''' + @Value1 + ''' OR ''' + @Value1 + ''' IS NULL) '
            END
      ELSE IF @Operator1=1
            BEGIN
            SET @Field1SQL=' ([' + @Field1 + '] LIKE ''%' + @Value1 + '%'') '
            END
      ELSE IF @Operator1=2
            BEGIN
            SET @Field1SQL=' ([' + @Field1 + '] NOT LIKE ''%' + @Value1 + '%'') '
      END
 
      IF @Operator2=0
            BEGIN
            SET @Field2SQL=' (' + @Field2 + '=''' + @Value2 + ''' OR ''' + @Value2 + ''' IS NULL)' 
            END
      ELSE IF @Operator2=1
            BEGIN
            SET @Field2SQL=' ([' + @Field2 + '] LIKE ''%' + @Value2 + '%'') '
            END
      ELSE IF @Operator2=2
            BEGIN
            SET @Field2SQL=' ([' + @Field2 + '] NOT LIKE ''%' + @Value2 + '%'') '
      END
 
IF (LEN(@Value1) + LEN(@Value2))<>0
   SET @SqlSearch = 'WHERE ('  + @Field1SQL + @Condition1 + @Field2SQL + ')'
ELSE 
   SET @SqlSearch= = 'WHERE 1=1'
 
SET @SqlLoad =
'SELECT * FROM ' +
' (SELECT *, ROW_NUMBER() OVER (ORDER BY FirstName) AS RowRank ' +
'    FROM Authors ' + @SqlSearch + 
' ) As AuthorsWithRowNumbers ' +
'WHERE (RowRank >' + @startRowIndex + ') ' +
'  AND (RowRank <= (' + @startRowIndex + '+' + @maximumRows +')) '
 
EXEC sp_executesql @SqlLoad
 
END

Open in new window

0
 
ee_rleeConnect With a Mentor Commented:
hi

also put [] here
' (' + @Field1 + '  ='''

this statement
 ''' OR ''' + @Value1 + ''' IS NULL) '
will not work because '' is not = null, so it will always return false
compare it to '' or check if len = 0  if you want to check if @Value is empty
0
 
Guy Hengel [angelIII / a3]Connect With a Mentor Billing EngineerCommented:
as from sql 2005, you have the option to avoid one of the big problems with the dynamic sql, ie that the dynamic sql is executed, by default, under the permissions of the caller and NOT of the procedure owner...

read up on the EXECUTE AS SELF (vs EXECUTE AS CALLER) to be added to the procedure header  
0
 
brejkConnect With a Mentor Commented:
I would also suggest putting SET NOCOUNT ON in the beginning of the procedure to avoid sending useless messages to the applications.
0
 
noobe1Author Commented:
Thanks everyone for the comments. Points will be distributed evenly.
0
All Courses

From novice to tech pro — start learning today.