1

I have a class file where I declare readonly string of my query to be used in a method. I met the error of

Must declare the scalar variable "@DBID"

May I know if I declare my variables wrongly?

Below are the code snippets:


Class file:

private static readonly string QUERY_GETMATCHEDRECORD = "SELECT [Title], [ItemLink], [RecordDocID] FROM [ERMS].[dbo].[Records] WHERE [ID] = @DBID AND [V1RecordID] = @recID AND [V1RecordDocID] = @recDocID";

public DataTable GetMatchedRecord(string DBID, string recID, string recDocID)
{
    string Method = System.Reflection.MethodBase.GetCurrentMethod().Name;
    DataTable dt = new DataTable();
    try
    {
        using (DB db = new DB(_datasource, _initialCatalog))
        {
            db.OpenConnection();
            using (SqlCommand command = new SqlCommand())
            {
                string commandText = QUERY_GETMATCHEDRECORD .FormatWith(DBID,recID,recDocID);
                _log.LogDebug(Method, "Command|{0}".FormatWith(commandText));
                command.CommandText = commandText;
                dt = db.ExecuteDataTable(command);
            }
            db.CloseConnection();
        }
    }
    catch (Exception ex)
    {
        _log.LogError(Method, "Error while retrieving matching records |{0}".FormatWith(ex.Message));
        _log.LogError(ex);
    }
    return dt;
}

Program .cs file:

MatchedRecords = oDB.GetMatchedRecord(DBID, RecID, RecDocID);
gymcode
  • 4,431
  • 15
  • 72
  • 128
  • What does the line in `_log.LogDebug` look like? Shouldn't your constant look more like `WHERE [ID] = {0} AND [V1RecordID] = {1} AND [V1RecordDocID] = {2}`? – Rafalon Jun 11 '19 at 07:09
  • It logged the error message and method encountered: `GetMatchedRecord | Error while retrieving matched record | Must declare the scalar variable "@DBID". .Net SqlClient Data Provider | Must declare the scalar variable "@DBID"` – gymcode Jun 11 '19 at 07:13
  • 1
    Where are you setting your variables? `FormatWith` replaces `{0}, {1}` etc. (assuming you're using James Newton-Kings extension method). – HoneyBadger Jun 11 '19 at 07:14
  • 2
    By the way, you should really use [parameterization](https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php/60496#60496) – HoneyBadger Jun 11 '19 at 07:16
  • @gymcode, I was talking about this line `_log.LogDebug(Method, "Command|{0}".FormatWith(commandText));`, not the one in the `catch` block – Rafalon Jun 11 '19 at 07:19
  • @gymcode the error complains that you tried to use a variable or parameter but never actually declared id. Whatever `FormatWith` does, it doesn't add parameters to the command. Most likely, it breaks the query by trying (and failing) to replace placeholders with strings. That's how SQL injection attacks happen. Imagine what would happen if `RecDocID` contained `1; drop table records;--`. Quoting doesn't help either, `1'; drop table records;--` would still work – Panagiotis Kanavos Jun 11 '19 at 07:22
  • It would be awesome if you could provide a [mcve]. Also, what is the **exact** value of `commandText`? – mjwills Jun 11 '19 at 07:25
  • @gymcode if you want to simplify your code, use a microORM like Dapper. Creating aliases like `FormatWith` for common functions like `String.Format` doesn't help. You save 4 characters and get code that nobody else can understand. You could use string interpolation instead of `FormatWith` to make creating strings easier – Panagiotis Kanavos Jun 11 '19 at 07:26

1 Answers1

2

using '@'-notated variables will only work if you add the parameters to the commands parameter-collection.

try the following:

    using (DB db = new DB(_datasource, _initialCatalog))
    {
        db.OpenConnection();
        using (SqlCommand command = new SqlCommand())
        {
            command.CommandText = QUERY_GETMATCHEDRECORD;
            command.Parameters.AddWithValue("@DBID", DBID);
            command.Parameters.AddWithValue("@recID", recID);
            command.Parameters.AddWithValue("@recDocID",recDocID);
            dt = db.ExecuteDataTable(command);
        }
        db.CloseConnection();
    }
Sancho Panza
  • 670
  • 4
  • 11
  • 1
    Except you shouldn't be using [AddWithValue](https://blogs.msmvps.com/jcoehoorn/blog/2014/05/12/can-we-stop-using-addwithvalue-already/) – HoneyBadger Jun 11 '19 at 07:18
  • 1
    :D I really dont think that is of any concern to OP at this stage of his code. Nevertheless a valid point! Since the actual types cannot be inferred given OPs code-snippet I will not edit my answer. – Sancho Panza Jun 11 '19 at 07:26
  • 1
    @HoneyBadger any "don't use X" drive-by comment is irresponsible if it doesn't also take the time to explain why X shouldn't be used. In this case, `.AddWithValue()` infers database types and sometimes gets it wrong, which can have bad implications for your database. If you're just dealing with small datasets and known datatypes, e.g. strings, there's no issue with using .AddWithValue()`. By the way, here's an archive link for the now-dead page you linked to: https://web.archive.org/web/20230206195816/https://blogs.msmvps.com/jcoehoorn/blog/2014/05/12/can-we-stop-using-addwithvalue-already/ – TylerH Jun 28 '23 at 13:59