0

My code:

int total = Convert.ToInt32(form["REC-tr-total"]); 
int value = 0;
string insValOutput = ""; 

if (total > 0)
{
  for (int i = 1; i <= total; i++) 
  {
    value = Convert.ToInt32(form["inventoryID[" + i + "]"]); 
    insValOutput = $"(" + vCAS_capture_payments.Id + ", value{i + 1}),"; 
  }

db.Database.ExecuteSqlCommand(String.Format(@"INSERT INTO dbo.VCAS_capture_payments__REF_items (FK_capture_paymentsId, FK_inventoryId)                 VALUES {0};", insValOutput));             
}

The error message:

enter image description here

This seems like a pretty straight-forward loop, but error keeps coming up... Experts please advise.

UPDATE: for those asking, I've updated this line as follows:

insValOutput = String.Format(@"({0},{1}),", vCAS_capture_payments.Id, value);  // (1, value1),(2, value2) ...

But I get the same results.

UPDATE No.2:

Thanks for all the comments, really appreciate it. I know the code is not the best, but I have been fighting with how best to implement multi-insert via LinQ and Entity Framework in my current codebase.

However from the answer provided, I was able to amend the code as follows, which works.

The culprit was the SQL statement and the concatenation.

int total = Convert.ToInt32(form["REC-tr-total"]); // -- Total Receipt item rows
int value = 0;
string insValOutput = ""; // -- Insert VALUES variable

if (total > 0)
{
    for (int i = 1; i <= total; i++)
    {
        value = Convert.ToInt32(form["inventoryID["+i+"]"]); // -- Fetch the inventory ID for index i   
        insValOutput += String.Format(@"({0},{1}),", vCAS_capture_payments.Id, value);  // (1, value1),(2, value2) ...
    }

    db.Database.ExecuteSqlCommand(String.Format(@"
                    INSERT INTO dbo.VCAS_capture_payments__REF_items (FK_capture_paymentsId, FK_inventoryId)
                    VALUES {0};", insValOutput.TrimEnd(',', ' ')));
}
Improve this question
8
  • 2
    insValOutput = $"(" + vCAS_capture_payments.Id + ", value{i + 1}),"; - this certainly doesn't do what you expect it to. And I guess it's the cause for the error at hand. But there are a couple more issues with this code. Commented Nov 27, 2024 at 14:50
  • 2
    ^^ the string will evaluate to (<whatever vCAS_capture_payments.Id is>, value{i + 1}), Commented Nov 27, 2024 at 14:52
  • 3
    Have you checked what the error says? It's a SqlException. The SQL query is invalid. It's fully vulnerable to SQL injection and that's exactly what happened here. Use parameterized queries instead of string formatting and concatenation. It could be worse. One of the fields could contain '); drop table Users; -- Commented Nov 27, 2024 at 14:52
  • 1
    The error is pointing to the wrong line. It’s not a c# syntax error but one with your sql Commented Nov 27, 2024 at 14:52
  • 3
    What is db? If it's a DbContext, why execute raw SQL like this? In any case, if you don't want to create a SqlCommand with parameters, consider using Dapper which automatically parameterizes queries based on object properties. You can even insert a list of items connection.Execute(@"insert MyTable(colA, colB) values (@a, @b)", items); Commented Nov 27, 2024 at 14:57

2 Answers 2

Reset to default
3

It looks as though you are trying to build the list of values passed to your insert statement. You want the result to be Values (itemA1, itemA2), (itemB2, itemB2), ... If this is incorrect the rest of the answer might not be relevant.

The error is in the line where you are structuring this collection:

insValOutput = $"(" + vCAS_capture_payments.Id + ", value{i + 1}),";

There are a few problems in your conditional where you are building the collection of values...

The first problem is that you will only ever have 1 (itema, itemb) collection because you do not append to your insValOutput variable. You're actually overwriting it with each execution of your loop.

The second is once you clear the error for '{' you'll get an error for a comma because you add a comma to the end of the text, and your last element needs to omit the comma.

The third, is the specific error you are receiving right now.

insValOutput = $"(" + vCAS_capture_payments.Id + ", value{i + 1}),";

Currently, you're actually putting the string "value{i + 1}" into your insValOutput, and the error is stating that the curly bracket is incorrect syntax.

I think what you were looking for is this:

insValOutput += $"({vCAS_capture_payments.Id}, {value}),"

But even with that corrected, you'll still have the issues stated above. Also, it looks hideous, find a way without mixing the string interpolation and the "+" for concatenation.

You should revisit how you are building this Values collection, then you should capture the entire statement you are passing so you can both proof read it, and then execute it in an SMS session to check for errors.

As others have pointed out, this is also not a great way to build queries in your application. You should definitely look into restructuring your code and how it builds queries.

Improve this answer
Sign up to request clarification or add additional context in comments.

7 Comments

insValOutput += $"("(" + {vCAS_capture_payments.Id} + ", " + value{i + 1} + "),")" - that cannot be right ...
you're right, there are too many quotes going on...trying to straighten it out
I suspect it should be: insValOutput = $"({vCAS_capture_payments.Id}, value{i + 1}),";
^^ Me, too. But I am not sure if this is worth fixing since the whole ordeal shouldn't be done as it is in the original post. But if we concentrate on this string, then the only addition I'd have is that there is probably still an "off-by-1" because the loop is 1-based.
needs a little more because he's trying to include the open parenthesis, the comma between the two values, the close parethesis, and the comma after that collection. I think I got it in the edits
|
1

try this:

int total = Convert.ToInt32(form["REC-tr-total"]); 
int value = 0;
string insValOutput = ""; 

if (total > 0)
    {
        List<string> valuesList = new List<string>();
        
        for (int i = 1; i <= total; i++) 
        {
            value = Convert.ToInt32(form["inventoryID[" + i + "]"]); 
            valuesList.Add($"({vCAS_capture_payments.Id}, {value})"); 
        }
    
        insValOutput = string.Join(", ", valuesList);
    
        db.Database.ExecuteSqlCommand(
            string.Format(@"INSERT INTO dbo.VCAS_capture_payments__REF_items 
                            (FK_capture_paymentsId, FK_inventoryId) 
                            VALUES {0};", insValOutput));             
    }
Improve this answer

2 Comments

Thank you for contributing to the Stack Overflow community. This may be a correct answer, but it’d be really useful to provide additional explanation of your code so developers can understand your reasoning. This is especially useful for new developers who aren’t as familiar with the syntax or struggling to understand the concepts. Would you kindly edit your answer to include additional details for the benefit of the community?
Sure, will do so...

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.