Dataset with optional addition to where statement

Question

I am trying to generate a dataset from a query that has a where statement. If the user passes a value additional things need to be added to the where statement. Is this the best way to go about this?

class TableAdapters
{
    public class FindScriptByTargetVDN
    {
        public static DataSet Fill(SqlConnection connection, string parameter)
        {
            string conditional = string.Empty;
            if (parameter.Length > 0)
            {
                conditional = "and svce.PeripheralNumber= " + parameter + " ";
            }
            string query = "select svce.PeripheralNumber AS Service, " 
                + "ms.EnterpriseName AS [Script Name], "
                + "p.EnterpriseName AS Peripheral, " 
                + "sg.PeripheralNumber AS [Skill Mapping], " 
                + "s.Version AS [Latest Version], "
                + "s.DateTime AS Created, " 
                + "s.Author " 
                + "from Script_Cross_Reference scr " 
                + "LEFT OUTER JOIN Script s on s.ScriptID = scr.ScriptID " 
                + "LEFT OUTER JOIN Master_Script ms on s.MasterScriptID = ms.MasterScriptID "
                + "LEFT OUTER JOIN Service svce on svce.SkillTargetID = scr.ForeignKey " 
                + "LEFT OUTER JOIN Peripheral p on svce.PeripheralID = p.PeripheralID " 
                + "LEFT OUTER JOIN Service_Member sm on svce.SkillTargetID = sm.ServiceSkillTargetID " 
                + "LEFT OUTER JOIN Skill_Group sg on sg.SkillTargetID = sm.SkillGroupSkillTargetID " 
                + "where s.Version = ms.CurrentVersion and scr.TargetType = 1 "
                + conditional
                + "order by svce.PeripheralNumber, ms.EnterpriseName, p.EnterpriseName";
            SqlDataAdapter adapter = new SqlDataAdapter(query, connection);
            DataSet dataSet = new DataSet();
            dataSet.Tables.Add(new DataTable());
            adapter.Fill(dataSet.Tables[0]);
            return dataSet;
        }
    }
}

Here is an example of the call:

this.FindScriptsDataGrid.DataSource = TableAdapters.FindScriptByTargetVDN.Fill(connection, this.ValueTextBox.Text).Tables[0];

The connection is defined before the call. The reason for the some what round about method is the user has the option of defining what server they want to run this query on before they run it.

Answer 1

This:

+ "where s.Version = ms.CurrentVersion and scr.TargetType = 1 "
+ conditional
+ "order by svce.PeripheralNumber, ms.EnterpriseName, p.EnterpriseName";

Is poison.

conditional came from a string concatenation that involves a parameter string that the method cannot trust:

conditional = "and svce.PeripheralNumber= " + parameter.ToString() + " ";

..Uh, the ToString() call is superfluous here, parameter is already a string! ;)

Looking at the code in your other question, this is where the parameter came from:

string value = this.ValueTextBox.Text;

That is what you shouldn't be doing: concatenating user input into an SQL statement is asking for trouble, whether it's a web or a desktop application doesn't matter, SQL Injection is SQL injection. Not to mention that you just put the calling code in charge of knowing the types for all columns that might be used, whether or not the values should be enclosed in single quotes, etc.

You need to use a parameterized query instead.

Answer 2

Seems like a reasonable approach. It's similar to one that I've used when building a query completely from an XML file, except I used parameters rather than adding in the string.

selectString = string.Format("{0} WHERE {1}='{2}' {3};",
                selectString,
                dateColumn,
                dateSelected.ToString("yyyy-MM-dd"),
                additionalCommandsAfterWhereStatement);

Answer 3

In my opinion, the BEST way to do this is to use LINQ. Build the query in a modern language, and let the runtime automatically convert it safely to SQL using LINQ-To-SQL. It automatically takes care of putting the user-supplied data into the query safely.

Please check out http://www.linqpad.net/WhyLINQBeatsSQL.aspx