Help with SqlCommand

[Gospp]

I've a problem when trying to convert my sql-query to a SqlCommand.

The query:

SELECT      L1.[App_Name],
DATEDIFF(ss,MAX(L1.MESG_DATESTAMP),GETDATE()) as Seconds
FROM sbyn_csf.CSF_CME_LOG    AS L1
JOIN sbyn_csf.CSF_LOGGER_LOG AS L2 ON L2.MESG_ID = L1.MESG_ID
WHERE logger_code = 101
AND [App_name] in ('002','010','022','025','036','037','040')
GROUP BY l1.[APP_NAME], L2.LOGGER_CODE
ORDER BY 1, 2

The problem is with the "in" operator, I want to build it dynamically in the program, whilst I'm able to change the query via a config-file.

private SqlCommand CreateSQLCommand(int i)
{ 
	SqlCommand SqlCmd = new SqlCommand();
	
	//SqlCmd.CommandText = SqlStrSelectTime;
	
	SqlCmd.CommandText =	
	"SELECT      L1.[App_Name]," + Environment.NewLine +
	"DATEDIFF(ss,MAX(L1.MESG_DATESTAMP),GETDATE()) as Seconds" + Environment.NewLine +
	"FROM sbyn_csf.CSF_CME_LOG    AS L1" + Environment.NewLine +
	"JOIN sbyn_csf.CSF_LOGGER_LOG AS L2 ON L2.MESG_ID = L1.MESG_ID" + Environment.NewLine +
	"WHERE logger_code = @logger_code" + Environment.NewLine +
	"AND [App_name] in (@inAppId)" + Environment.NewLine +
	"GROUP BY l1.[APP_NAME], L2.LOGGER_CODE" + Environment.NewLine +
	"ORDER BY 1, 2 ";

	SqlCmd.Parameters.Add("@logger_code", SqlDbType.VarChar).Value = i;
	List<string> inAppId = new List<string>();
	//string inAppId = "''";
	inAppId.Add("''");
	for (int j = 0; j < ServicesList.Count; j++)
	{
		
		inAppId.Add(",'" + ServicesList[j].App_Name + "'");
		//inAppId +=  ",'" + ServicesList[j].App_Name + "'";
	}

	SqlCmd.Parameters.Add("@inAppId", SqlDbType.VarChar).Value = inAppId;

	return SqlCmd;
}

I'm grateful for any help!

[Sodabread]

I'm not 100% on this, but shouldn't your inAppId variable be a stringbuilder or something similar? Right now, you're trying to add a List as a varchar and I didn't think that would implicitly convert correctly.

[Gospp]

I'm not 100% on this, but shouldn't your inAppId variable be a stringbuilder or something similar? Right now, you're trying to add a List as a varchar and I didn't think that would implicitly convert correctly.

Well, I don't get a error, but a don't get a query response. :(

private SqlCommand CreateSQLCommand(int i)
{ 
	SqlCommand SqlCmd = new SqlCommand();
	
	//SqlCmd.CommandText = SqlStrSelectTime;

	SqlCmd.CommandText =
	"SELECT      L1.[App_Name]," + Environment.NewLine +
	"DATEDIFF(ss,MAX(L1.MESG_DATESTAMP),GETDATE()) as Seconds" + Environment.NewLine +
	"FROM sbyn_csf.CSF_CME_LOG    AS L1" + Environment.NewLine +
	"JOIN sbyn_csf.CSF_LOGGER_LOG AS L2 ON L2.MESG_ID = L1.MESG_ID" + Environment.NewLine +
	"WHERE logger_code = @logger_code" + Environment.NewLine +
	"AND [App_name] in (@inAppId)" + Environment.NewLine +
	"GROUP BY l1.[APP_NAME], L2.LOGGER_CODE" + Environment.NewLine +
	"ORDER BY 1, 2 ";
	
	SqlCmd.Parameters.Add("@logger_code", SqlDbType.VarChar).Value = i;

	StringBuilder inAppId = new StringBuilder();
	inAppId.Append("''");
	for (int j = 0; j < ServicesList.Count; j++)
	{
		inAppId.Append(",'" + ServicesList[j].App_Name + "'");
	}

	SqlCmd.Parameters.Add("@inAppId", SqlDbType.VarChar).Value = inAppId.ToString();

	return SqlCmd;
}

[Sodabread]

Hmm. When you return that SQL command, are you associating it with a connection object? Also, how are you executing the query?

[Gospp]

Hmm. When you return that SQL command, are you associating it with a connection object? Also, how are you executing the query?

Yes I do, if I remove the "in" part of the query everything works fine...

sqlcmd.Connection = SQLConnection;

SqlDataAdapter SQLDataAdapter = new SqlDataAdapter(sqlcmd);

DataTable dtResult = new DataTable();
SQLDataAdapter.Fill(dtResult);

foreach (DataRow drRow in dtResult.Rows)
{
	//Update objects
}
dtResult.Dispose();
SQLDataAdapter.Dispose();

With the "in" part active the dtResult.Rows is always 0 if i add more than one object.

[Sodabread]

Let's try this. When you create your stringbuilder, you immediately append a double apostrophe and then a comma. I'm wondering if an empty set of apostrophes is causing issues. For curiosity's sake, try this:

StringBuilder inAppId = new StringBuilder();

for(int j = 0; j < ServiceList.Count; j++)
{
    inAppId.Append("'" + ServiceList[j].App_Name + "'");
    if(j < ServiceList.Count - 1)
        inAppId.Append(",");
}

I'm kind of fishing for a solution here, but I've come across some row return issues when things look like they should work, but there's non-necessary text in the query string.

[Gospp]

Let's try this. When you create your stringbuilder, you immediately append a double apostrophe and then a comma. I'm wondering if an empty set of apostrophes is causing issues. For curiosity's sake, try this:

StringBuilder inAppId = new StringBuilder();

for(int j = 0; j < ServiceList.Count; j++)
{
    inAppId.Append("'" + ServiceList[j].App_Name + "'");
    if(j < ServiceList.Count - 1)
        inAppId.Append(",");
}

I'm kind of fishing for a solution here, but I've come across some row return issues when things look like they should work, but there's non-necessary text in the query string.

Close but no cigar, as long as I only add one value to the stringbuilder it work, but not with two. :(

I think there might be possible to use SqlDbType.Structured and a DataTable instead. But I can't get that to work either.

[privatevoid]

I think I see the problem:

If you have a argument that is a string you would assign it like this string str = "'001', '002', '003'"; now if you added that to your query like this string.format("Select * from ... where appName in {0};", str); it will work as you are adding the code directly though you shouldn't do this as it opens up an SQL injection possibility. The reason your code does not work is because you are putting multiple parameters into one parameter as a string so what you are actually saying is this:

...
WHERE logger_code = 101
AND [App_name] in (''002','010','022','025','036','037','040'')
...
// Note the extra ' at each end so in effect you are comparing 
// App_name to ''002','010','022','025','036','037','040'' and 
// not each individual element.

If I where you I would think about using a list of SQL Parameters to build up the in block.

Treat the below as an idea rather than a perfect execution but somthing like this:

private DataTable GetData()
{
    string insert = "";
    string sql = "Select ... IN ({0}) ... ;";

    foreach(SqlParameter param in parameters)
    {
          insert += param.ParameterName + " ,";  
    } 

    insert = insert.Remove(insert.Length - 2);
    sql = String.Format(sql, insert);

    return myQuery( sql, parameters);
}

private static DataTable myQuery(string sql, params SqlParameter[] args)
{
    SqlConnection connection = new SqlConnection(Settings.Default.ConnectionString);

    SqlCommand command = new SqlCommand(sql, connection);
    if (args != null) command.Parameters.AddRange(args);

    SqlDataAdapter adapter = new SqlDataAdapter(command);
    DataTable returnData = new DataTable();
    adapter.Fill(returnData);

    return returnData;
}

[Gospp]

I think I see the problem:

If you have a argument that is a string you would assign it like this string str = "'001', '002', '003'"; now if you added that to your query like this string.format("Select * from ... where appName in {0};", str); it will work as you are adding the code directly though you shouldn't do this as it opens up an SQL injection possibility. The reason your code does not work is because you are putting multiple parameters into one parameter as a string so what you are actually saying is this:

...
WHERE logger_code = 101
AND [App_name] in (''002','010','022','025','036','037','040'')
...
// Note the extra ' at each end so in effect you are comparing 
// App_name to ''002','010','022','025','036','037','040'' and 
// not each individual element.

If I where you I would think about using a list of SQL Parameters to build up the in block.

Treat the below as an idea rather than a perfect execution but somthing like this:

private DataTable GetData()
{
    string insert = "";
    string sql = "Select ... IN ({0}) ... ;";

    foreach(SqlParameter param in parameters)
    {
          insert += param.ParameterName + " ,";  
    } 

    insert = insert.Remove(insert.Length - 2);
    sql = String.Format(sql, insert);

    return myQuery( sql, parameters);
}

private static DataTable myQuery(string sql, params SqlParameter[] args)
{
    SqlConnection connection = new SqlConnection(Settings.Default.ConnectionString);

    SqlCommand command = new SqlCommand(sql, connection);
    if (args != null) command.Parameters.AddRange(args);

    SqlDataAdapter adapter = new SqlDataAdapter(command);
    DataTable returnData = new DataTable();
    adapter.Fill(returnData);

    return returnData;
}

I've done something else, it' not that pretty but it works.
I ended up dividing my query in two different variables, split by the in parameter. Now I'm free to append my parameters.

private SqlCommand CreateSQLCommand(int i)
{
	StringBuilder sb = new StringBuilder();

	sb.Append(SqlStrSelectTimePart1);

	sb.Append("'',");
	for (int j = 0; j < ServicesList.Count; j++)
	{
		sb.Append("'" + ServicesList[j].App_Name + "'");
		if (j < ServicesList.Count - 1)
		{
			sb.Append(",");
		}
	}
	
	sb.Append(SqlStrSelectTimePart2);

	SqlCommand SqlCmd = new SqlCommand();
	SqlCmd.CommandText = sb.ToString();
	
	SqlCmd.Parameters.Add("@logger_code", SqlDbType.VarChar).Value = i;

	return SqlCmd;
}

[privatevoid]

Thats fine aslong as the app_names are not editable outside the code as this will leave a security vulnerability.

[Gospp]

Thats fine aslong as the app_names are not editable outside the code as this will leave a security vulnerability.

I fetch them from a database, so they should be safe.

[Sodabread]

Good catch, void. I think I need to up my C# knowledge a bit.

[Gospp]

I think we've come as far as we are going to get.

Thanks for your help, both of you!! :)