Reputation: 33
I keep getting an exception that reads:
"Invalid Column Name, 'IBM'."
The error is happening at: "' + @ticker + '"
even though @ticker
is declared in VALUES
. I suspect the error could be taking place at some other point in the query, but I'm pretty new to SQL/T-SQL, so I'm not sure how to figure out where.
private string InsertRecord(Indicator indicator)
{
try
{
if (!CheckIfColumnExists(indicator.GetType().Name))
{
AddColumn(indicator.GetType().Name, SqlDbType.Real);
}
const string query = @"
DECLARE @sql nvarchar(max) = '
INSERT INTO ' + QUOTENAME(@tableName) + '(
' + QUOTENAME(@indicator) + ', date, ticker)
VALUES(' + @indicatorValue + ', ' + @date + ', ' + @ticker + ')
';
EXEC sp_executesql @sql;
";
//checking if the record is already there
if (!CheckIfRecordExists(indicator))
{
using (SqlConnection conn = new SqlConnection(this.connectionstring))
{
conn.Open();
SqlCommand cmd = new SqlCommand(query , conn);
cmd.Parameters.AddWithValue("@tableName", tableName);
cmd.Parameters.AddWithValue("@indicator", indicator.GetType().Name);
cmd.Parameters.AddWithValue("@indicatorValue", indicator.Value.ToString());
cmd.Parameters.AddWithValue("@date", indicator.Date.ToString("yyyy-MM-dd"));
cmd.Parameters.AddWithValue("@ticker", indicator.Ticker);
var result = cmd.ExecuteNonQuery();
return "New Record Inserted";
}
}
else
{
return "Record Already Exists";
}
}
catch
{
return "Failure Inserting New Record";
}
}
EDIT: I'm accepting CharlieFace's answer, because it avoids breaches via SQL Injection, and explains the necessity for sp_executesql.
Upvotes: 0
Views: 217
Reputation: 71740
You should pass in the parameters which contain data (as opposed to column and table names) all the way to sp_executesql
const string query = @"
DECLARE @sql nvarchar(max) = '
INSERT INTO ' + QUOTENAME(@tableName) + '(
' + QUOTENAME(@indicator) + ', date, ticker)
VALUES(@indicatorValue, @date, @ticker)
';
EXEC sp_executesql @sql,
N'@indicatorValue nvarchar(100), @date date, @ticker nvarchar(100)',
@indicatorValue,
@date,
@ticker;
";
You should also pass the parameters as their real values (date, int) rather than ToString
. Also declare the parameter types and lengths explicitly
// table and column name should be NVARCHAR(128)
cmd.Parameters.Add("@tableName", SqlDbType.NVarchar, 128).Value = tableName;
cmd.Parameters.Add("@indicator", SqlDbType.NVarchar, 128).Value = indicator.GetType().Name;
cmd.Parameters.Add("@indicatorValue", SqlDbType.NVarchar, 100).Value = indicator.Value;
cmd.Parameters.Add("@date", SqlDbType.Date).Value = indicator.Date;
cmd.Parameters.Add("@ticker", SqlDbType.NVarchar, 100).Value = indicator.Ticker;
Upvotes: 1
Reputation: 51285
Concerns
You can't use parameters to append values for table name and column name. Instead, string concatenation is needed although this will lead to an open SQL Injection attack (Example: Bobby Tables). Hence, ensure that you have done enough validation for the tableName
or the string concatenation part.
You don't need to append single quotes '
for the parameters in the query. This will be done automatically when SQLCommand
appends the SQLParameter
value to query according to the parameter type.
I don't see there is a need to use EXEC sp_executesql
. While you can just straight-way execute the INSERT
query.
In StackOverflow community, normally would be suggested to use SqlCommand.Add("@Name", SqlDbType).Value
and specify the parameter type instead of SqlCommand.AddWithValue()
. Refer to Can we stop using AddWithValue() already?.
In conclusion, your SqlCommand
should be as:
string query = @"
INSERT INTO " + tableName +
"(" + indicator.GetType().Name + ", date, ticker)" +
" VALUES (@indicatorValue, @date, @ticker)";
SqlCommand cmd = new SqlCommand(query , conn);
cmd.Parameters.Add("@indicatorValue", SqlDbType.NVarchar).Value = indicator.Value.ToString();
cmd.Parameters.Add("@date", SqlDbType.NVarchar).Value = indicator.Date.ToString("yyyy-MM-dd");
cmd.Parameters.Add("@ticker", SqlDbType.NVarchar).Value = indicator.Ticker;
Upvotes: 1