Writing a dreaded SQL search query (2nd phase)

Question

I am working on a search query (with an asp.net 3.5 front end) which seems quite simple, but is quite complex. The complete query is:

set ANSI_NULLS ON
set QUOTED_IDENTIFIER ON
go

ALTER PROCEDURE [dbo].[usp_Item_Search]
    @Item_Num varchar(30) = NULL
    ,@Search_Type int = NULL
    ,@Vendor_Num varchar(10) = NULL
    ,@Search_User_ID int = 0
    ,@StartDate smalldatetime = NULL
    ,@EndDate smalldatetime = NULL
AS
DECLARE @SQLstr as nvarchar(4000)

Set @SQLstr = 'SELECT RecID, Vendor_Num, Vendor_Name, InvoiceNum, Item_Num, 
(SELECT CONVERT(VARCHAR(11), RecDate, 106) AS [DD MON YYYY]) As RecDate, NeedsUpdate, RecAddUserID FROM [tbl_ItemLog] where 1=1 '

IF (@Item_Num IS NOT NULL and LTRIM(@Item_Num) <> '')
    Begin
        If @Search_Type = 0
            BEGIN
                Set @SQLstr = @SQLstr +  'AND Item_Num LIKE ''' + @Item_Num + '%'''
            END
        If @Search_Type = 1
            BEGIN
                Set @SQLstr = @SQLstr + 'AND Item_Num LIKE ''%' + @Item_Num + '%'''
            END
        If @Search_Type = 2
            BEGIN
                Set @SQLstr = @SQLstr + 'AND Item_Num LIKE ''%' + @Item_Num + ''''
            END
    End

IF (@Vendor_Num IS NOT NULL and LTRIM(@Vendor_Num) <> '')
    Begin
        Set @SQLstr = @SQLstr + ' AND Vendor_Num = ''' + @Vendor_Num + ''''
    End

IF (@Search_User_ID IS NOT NULL and @Search_User_ID > 0)
    Begin
        Set @SQLstr = @SQLstr + ' AND RecAddUserID = ' + convert(nvarchar(20),@Search_User_ID)
    End

Set @SQLstr = @SQLstr + ' AND (RecDate BETWEEN ''' + convert(nvarchar(10),@StartDate,106) + ''' AND ''' + convert(nvarchar(10),@EndDate,106) + ''')'

PRINT (@SQLstr)
--Execute (@SQLstr)

When I pass all empty parameter values, I get an error:

"Failed to convert parameter value from a String to a Int32."

The asp.net code that is calling the stored proc is:

        //Display search results in GridView;
        SqlConnection con = new SqlConnection(strConn);
        //string sqlItemSearch = "usp_Item_Search";
        SqlCommand cmdItemSearch = new SqlCommand(sqlItemSearch, con);
        cmdItemSearch.CommandType = CommandType.StoredProcedure;

        cmdItemSearch.Parameters.Add(new SqlParameter("@Item_Num", SqlDbType.VarChar, 30));
        cmdItemSearch.Parameters["@Item_Num"].Value = txtItemNumber.Text.Trim();

        cmdItemSearch.Parameters.Add(new SqlParameter("@Search_Type", SqlDbType.Int));
        cmdItemSearch.Parameters["@Search_Type"].Value = ddlSearchType.SelectedItem.Value;

        cmdItemSearch.Parameters.Add(new SqlParameter("@Vendor_Num", SqlDbType.VarChar, 10));
        cmdItemSearch.Parameters["@Vendor_Num"].Value = txtVendorNumber.Text.Trim();

        cmdItemSearch.Parameters.Add(new SqlParameter("@Search_User_ID", SqlDbType.Int));
        cmdItemSearch.Parameters["@Search_User_ID"].Value = ddlSeachUser.SelectedItem.Value;

        if (!string.IsNullOrEmpty(txtStartDate.Text))
        {
            cmdItemSearch.Parameters.Add(new SqlParameter("@StartDate", SqlDbType.DateTime));
            cmdItemSearch.Parameters["@StartDate"].Value = Convert.ToDateTime(txtStartDate.Text.Trim());
        }
        else
        {
            cmdItemSearch.Parameters.Add(new SqlParameter("@StartDate", SqlDbType.DateTime));
            cmdItemSearch.Parameters["@StartDate"].Value = Convert.ToDateTime("01/01/1996");
        }

        if (!string.IsNullOrEmpty(txtEndDate.Text))
        {
            cmdItemSearch.Parameters.Add(new SqlParameter("@EndDate", SqlDbType.DateTime));
            cmdItemSearch.Parameters["@EndDate"].Value = Convert.ToDateTime(txtEndDate.Text.Trim());
        }
        else
        {
            cmdItemSearch.Parameters.Add(new SqlParameter("@EndDate", SqlDbType.DateTime));
            cmdItemSearch.Parameters["@EndDate"].Value = Convert.ToDateTime(DateTime.Now);
        }
        con.Open();

        SqlDataAdapter ada = new SqlDataAdapter(cmdItemSearch);
        DataSet ds = new DataSet();
        ada.Fill(ds);

            gvSearchDetailResults.DataSource = ds;
            gvSearchDetailResults.DataBind();
            pnlSearchResults.Visible = true;

How can I resolve this?

Answer 1

IF (Search_User_ID IS NOT NULL) 

needs an @ symbol infront of the variable

You say you are passing empty string in for all variables but one is an int, it can't take an empty string that is not int data. Can't believe I didn;t notice that the first time.

Answer 2

Why don't you use single parameterized query like this:

select 
   recdid,
   Vendor_Num,
   Vendor_Name,
   InvoiceNum,
   Item_Num, 
   CONVERT(VARCHAR(11), RecDate, 106) as RecDate,
   NeedsUpdate, 
   RecAddUserID 
FROM 
  [tbl_ItemLog] as t
where
   (((Item_num like @Item_Num + '%' and @Search_Type = 0) OR
    (Item_num like '%' + @Item_Num + '%' and @Search_Type =  1) OR
    (Item_num like '%' + @Item_Num + '%' and @Search_Type = 2))  
        OR
    @Item_Num IS NULL) AND
   (Vendor_Num = @Vendor_Num OR @Vendor_Num IS NULL) AND
   (RecAddUserId = @Search_User_Id OR @Search_User_Id IS NULL) AND
   (RecDate BETWEEN @StartDate AND @EndDate)

Answer 3

You're not quite building the string correctly as far as I can tell. If no @Item_Num is passed in, you'll end up with no WHERE key word... you'll just have "FROM [tblItem_Log] AND..."

I would make all of the criteria appends be "AND ..." and as your initial statement use:

FROM [tbl_Item_Log] WHERE (1=1)

Since you have code to return the generated string, why not put that into SSMS and try to run it?

I also just noticed that if you don't pass in date values that you will end up executing a NULL string, because your final concatenation will end up causing a NULL. These are the kinds of things that you need to pay very close attention to if you're going to be using dynamic SQL to build queries.

Once I corrected that I was able to run the stored procedure without any errors (at least to generate what looks like a valid SQL statement). That leads me to believe that it may be a problem with data types in the underlying table. Can you provide the definition for that?

One last note: Personally, I would use

CONVERT(VARCHAR(11), RecDate, 106) AS RecDate

instead of the seemingly unnecessary subquery that you have.

Yet another edit: You may want to remove the code that checks LTRIM(@Search_User_ID) <> ''. It's a pointless bit of code and perhaps a setting particular to your server/connection is causing it to fail because of the type mismatch.

Answer 4

You really have several different stored procedures here. Why not just write them separately? Everything that's controlled by switch statements could be easily in procedural code. Same for the LTRIM calls.

You could call them all from a single SP with switch statements; but I think it's generally better to not merge them in the first place. The SP queries will optimize more easily, and the code will be simpified. There's not much you gain by consolidating them.

Not sure of your business rules, but you could simplify this outside SQL with

switch(search_type) {    
case 1:  
    do_query_type_1(args);  
    break;  
case 2:  
    do_query_type_2(args);  
    break;  
case 3:  
    do_query_type_3(args);  
    break;  
default:  
    whatever ...  
}

Also it looks like you have separate logic for cases where the item number is provided or not. Same for other fields. Each of your use cases looks like it resolves to a pretty simple query.