4

I have to make a string by using the values which the user selects on the webpage,

Suppose I need to display files for multiple machines with different search criteria...

I currently use this code:

DataTable dt = new DataTable();
SqlConnection connection = new SqlConnection();
connection.ConnectionString = ConfigurationManager
               .ConnectionStrings["DBConnectionString"].ConnectionString;
connection.Open();
SqlCommand sqlCmd = new SqlCommand
  ("SELECT FileID FROM Files
    WHERE MachineID=@machineID and date= @date", connection);
SqlDataAdapter sqlDa = new SqlDataAdapter(sqlCmd);

sqlCmd.Parameters.AddWithValue("@machineID", machineID);
sqlCmd.Parameters.AddWithValue("@date", date);

sqlDa.Fill(dt);

Now this is a fixed query where the user just has one machine and just selects one date...

I want to make a query in which the user has multiple search options like type or size if he/she wants depending on what he/she selects.

Also if he/she can select multiple machines...

SELECT FileID FROM Files
WHERE (MachineID=@machineID1 or MachineID = @machineID2...)
AND (date= @date and size=@size and type=@type... )

All of this happens at runtime... otherwise I have to create a for loop to put multiple machines one by one... and have multiple queries depending on the case the user selected...

This is quite interesting and I could use some help...

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
user175084
  • 4,550
  • 28
  • 114
  • 169
  • your inputs are great but what about the other search criteria to add like date, type and so on depending on user choice....ok i am not good with stored procedures so was looking for an easy way out... but as allu of you are suggesting i go that way i need to kno how to implement it.. can some1 tell me where i can find good information on stored procedures so that i can understand and use it... – user175084 Apr 16 '10 at 22:23
  • 1
    Hi, I've extended my answer to include a stored procedure that would work for your problem. – amelvin Apr 16 '10 at 23:39

6 Answers6

2

You can use WHERE MachineID IN ('Machine1', 'Machine2', 'Machine3', ... 'MachineN')

Then in your loop you would just add the 1..n machines. The IN clause works with 1 element or n elements, so it should be fine.

However, I'd look at using a stored procedure to do it rather than hardcoding the SQL into your application.

dcp
  • 54,410
  • 22
  • 144
  • 164
  • 1
    this is potentially vulnerable to a SQL injection, since you're no longer using a parameterized query (at least, I've never seen a parameterized query interface that allows parameterizing the "in" clause) – rmeador Apr 16 '10 at 22:03
  • @meador - I agree 100%, that's why I made the recommendation to use a stored procedure. – dcp Apr 16 '10 at 22:14
2

Build a real table and load the machine ids into it.

Then your SQL would be:

where MachineID in ( select MachineID from userMachine where userID = x)

When you are done, remove all rows for the userID:

delete from userMachine where userID = x.
Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
sparkkkey
  • 158
  • 1
  • 10
2

If you are going to do this via dynamic SQL, you need to build a call to the IN function. (e.g. In(id1, id2, id3...)

private string GetSql( IList<int> machineIds )
{
    var sql = new StringBuilder( "SELECT FileID FROM Files Where MachineID In(" );
    for( var i = 0; i < machineIds.Count; i++ )
    {
        if ( i > 0 )
            sql.Append(", ")
        sql.Append("@MachineId{0}", i);
    }

    sql.Append(" ) ");

    //additional parameters to query
    sql.AppendLine(" And Col1 = @Col1" );
    sql.AppendLine(" And Col2 = @Col2 ");
    ...

    return sql.ToString();
}

private DataTable GetData( IList<int> machineIds, string col1, int col2... )
{
    var dt = new DataTable();
    var sql = GetSql( machineIds );
    using ( var conn = new SqlConnection() )
    {
        conn.ConnectionString = ConfigurationManager.ConnectionStrings["DBConnectionString"].ConnectionString;
        using ( var cmd = new SqlCommand( sql, conn ) )
        {
            conn.Open();

            for( var i = 0; i < machineIds.Count; i++ )
            {
                var parameterName = string.Format("@MachineId{0}", i );
                cmd.Parameters.AddWithValue( parameterName, machineIds[i] );
            }

            cmd.Parameters.AddWithValue( "@Col1", col1 ); 
            cmd.Parameters.AddWithValue( "@Col2", col2 ); 
            ...

            using ( var da = new SqlDataAdapter( cmd ) )
            {
                da.Fill( dt );
            }
        }
    }

    return dt;
}
Thomas
  • 63,911
  • 12
  • 95
  • 141
  • ok i think i understood your code but have not implemented it,, but what do for the other search items.. simply append them as u did for "," and "machineID"??? – user175084 Apr 16 '10 at 22:34
  • @user175084 - If by "other search items" you mean other columns in the query, you would build those into your Select statement just like you did when you had `MachineId = @MachineId`. So, "...And Col1 = @Col1 And Col2 = @Col2...". If by "other" you mean other machine ids, you pass a list into the function of Ids you want returned. – Thomas Apr 16 '10 at 22:41
  • 1
    @user175084 - I have edited my response to show how you might pass additional columns into the query. You need to append parameters to your SQL statement for each of those additional values you wish to drop into the query and then add SqlParameters for each one. – Thomas Apr 16 '10 at 22:46
  • ok things look good let me try this and hope it works... thanks man – user175084 Apr 16 '10 at 22:52
  • i tried a lot of stuff but im getting this error Must declare the scalar variable "@MachineId" – user175084 Apr 21 '10 at 18:58
  • @user175084 - You shouldn't have a parameter called @MachineId. They should all be named @MachineId0, @MachineId1, @MachineId2 etc. – Thomas Apr 21 '10 at 19:16
1

Ideally you are trying to arrive at a solution similar to creating "MachineID in (1, 2, 3, 4)" dynamically.

Option 1

There are many ways to complete this task from passing in a comma separated string into the stored proc and dynamically build the sql string and then calling "EXEC sp_executesql @sql" WHERE IN (array of IDs)

Option 2

You can pass in a string of comma separated values and then parse out the values into their own temp-table and then join on to it http://vyaskn.tripod.com/passing_arrays_to_stored_procedures.htm

Option 3 - my choice

You can now pass in the array of values using XML and then select the array items easily. http://support.microsoft.com/kb/555266

.

Community
  • 1
  • 1
Glennular
  • 17,827
  • 9
  • 58
  • 77
1

I would also recommend using a stored procedure because otherwise you will leave yourself open to an SQL injection attack - especially where you are building up a string based on user input.

Something like:

a' or 1=1; -- Do bad things

You can use sp_executesql in SQL to run an SQL statement that is built up with a where clause like @dcp suggests and although it wouldn't optimize well it is probably a quick command to run anyway.

SQL Injection attacks by example

One way to achieve this would be using charindex. This example demonstrates how a stored procedure could be run when passed a space separated list of ids:

declare @machine table (machineId int, machineName varchar(20))
declare @files table (fileId int, machineId int)

insert into @machine (machineId, machineName) values (1, 'machine')
insert into @machine (machineId, machineName) values (2, 'machine 2.0')
insert into @machine (machineId, machineName) values (3, 'third machine')
insert into @machine (machineId, machineName) values (4, 'machine goes forth')
insert into @machine (machineId, machineName) values (5, 'machine V')

insert into @files (fileId, machineId) values (1, 3)
insert into @files (fileId, machineId) values (2, 3)
insert into @files (fileId, machineId) values (3, 2)
insert into @files (fileId, machineId) values (4, 1)
insert into @files (fileId, machineId) values (5, 3)
insert into @files (fileId, machineId) values (6, 5)

declare @machineText1 varchar(100)
declare @machineText2 varchar(100)
declare @machineText3 varchar(100)

set @machineText1 = '1 3 4'
set @machineText2 = '1'
set @machineText3 = '5 6'

select * from @files where charindex(rtrim(machineId), @machineText1, 1) > 0
-- returns files 1, 2, 4 and 5

select * from @files where charindex(rtrim(machineId), @machineText2, 1) > 0
-- returns file 4

select * from @files where charindex(rtrim(machineId), @machineText3, 1) > 0
--returns file 6

So you can create this stored procedure to achieve your aim:

create procedure FilesForMachines (@machineIds varchar(1000))
as
select * from [Files] where charindex(rtrim(machineId), @machineIds, 1) > 0

The charindex tip is from BugSplat.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
amelvin
  • 8,919
  • 4
  • 38
  • 59
1

Normally when I want to create a "search" type query, I use optional parameters. This allows me to send something or nothing to the parameter, making the query go from vague to very specific.

Example:

SELECT
  COL1,
  COL2,
  COL3
FROM TABLE
WHERE (@COL1 IS NULL OR @COL1 = '' OR @COL1 = COL1)

As you'll notice above, if you pass in NULL or BLANK it won't add the parameter to the query. If you do enter a value, then it'll be used in the comparison.

Zachary
  • 6,522
  • 22
  • 34