What's wrong with this simple update request?

You must use a parameterized query for several reasons:

• exclude the possibility of intrusion into your request
• protecting your database / website from SQL injection

Also, if cmd returns 1, then the row has been updated. You may need to check your expectations ...

String query = @"
    UPDATE contacts 
    SET contact_name = @contact_name, contact_phone = @contact_phone, contact_fax = @contact_fax, 
        contact_direct = @contact_direct , company_id = @company_id, contact_address1 = @contact_address1, 
        contact_address2 =@contact_address2, contact_city = @contact_city , contact_state = @contact_state,  
        contact_zip = @contact_zip 
    WHERE contact_id = @contact_id";

String cs = Lib.GetConnectionString(null);
using (SqlConnection conn = new SqlConnection(cs))
{
    using (SqlCommand cmd = conn.CreateCommand())
    {
        cmd.Parameters.AddWithValue("@contact_name", ContactName.Text.Trim());
        cmd.Parameters.AddWithValue("@contact_phone", Phone.Text.Trim());
        cmd.Parameters.AddWithValue("@contact_fax", Fax.Text.Trim());
        cmd.Parameters.AddWithValue("@contact_direct", Direct.Text.Trim());
        cmd.Parameters.AddWithValue("@company_id", Company.SelectedValue);
        cmd.Parameters.AddWithValue("@contact_address1", Address1.Text.Trim());
        cmd.Parameters.AddWithValue("@contact_address2", Address2.Text.Trim());
        cmd.Parameters.AddWithValue("@contact_city", City.Text.Trim());
        cmd.Parameters.AddWithValue("@contact_state", State.SelectedValue);
        cmd.Parameters.AddWithValue("@contact_zip", Zip.Text.Trim());
        cmd.Parameters.AddWithValue("@contact_id", contact_id);
        cmd.CommandText = query;
        cmd.Connection.Open();
        int rows = cmd.ExecuteNonQuery();
    }
}

      

        
        
        
      

    

Now, doesn't it look much cleaner? And when you understand why, it will give you a warm fuzzy feeling and allow you to sleep well at night .; -)

Clarification

Your stated question is "What is wrong with this request." The answer is nothing. Nothing happens in the request.

The problem is your page code that you haven't posted even after 5 people suggested there was nothing wrong with the request.

What I mean by "You might need to check your expectations ..." and "You need to examine the data that goes into the parameters before executing the request to make sure you need them" is more clearly described by John. but let me briefly point it out again:

The request, as shown, is ostensibly correct and should perform as expected. What you are probably experiencing is a flaw in the logic of the surrounding code.

Chances are you are binding data in page_load without an IsPostback guard, thereby overwriting your input values ​​with the original data.

You only need to load and bind the controls / page once on first load. After that, the state of the controls is stored in the viewstate, which is stored in a hidden field in html. If you just bind to the datasource on every page load, you will overwrite any input

So let's take a look at how it works.

Here is the correct logical flow

protected void Page_Load(object sender, EventArgs e)
{
    System.Diagnostics.Trace.WriteLine("Page_Load");
    if (!IsPostBack)
    {
        System.Diagnostics.Trace.WriteLine("\tBind TextBox1");
        TextBox1.Text = "Initial Value";
    }
    System.Diagnostics.Trace.WriteLine("\tTextBox1.Text = " + TextBox1.Text);
}

protected void Button1_Click(object sender, EventArgs e)
{
    System.Diagnostics.Trace.WriteLine("Button1_Click");
    System.Diagnostics.Trace.WriteLine("\tTextBox1.Text = " + TextBox1.Text);
}

      

        
        
        
      

    

If you load this page, enter "New value" in the text box and click the "one" button, this shows the trace:

    Page_Load
        Bind TextBox1
        TextBox1.Text = Initial Value

    Page_Load
        TextBox1.Text = New Value

    Button1_Click
        TextBox1.Text = New Value

But without protection:

protected void Page_Load(object sender, EventArgs e)
{
    System.Diagnostics.Trace.WriteLine("Page_Load");
    System.Diagnostics.Trace.WriteLine("\tBind TextBox1");
    TextBox1.Text = "Initial Value";
    System.Diagnostics.Trace.WriteLine("\tTextBox1.Text = " + TextBox1.Text);
}

        protected void Button1_Click(object sender, EventArgs e)
{
    System.Diagnostics.Trace.WriteLine("Button1_Click");
    System.Diagnostics.Trace.WriteLine("\tTextBox1.Text = " + TextBox1.Text);
}

      

        
        
        
      

    

You are getting the results that you are probably experiencing ...

    Page_Load
        Bind TextBox1
        TextBox1.Text = Initial Value

    Page_Load
        Bind TextBox1
        TextBox1.Text = Initial Value

    Button1_Click
        TextBox1.Text = Initial Value

But then again, a practical debugging technique to help you is to break down the update method just before executing the query and check the values ​​to make sure they are what you think they are, and then from there.

First, it is wildly susceptible to injection.

For more than a day, any input containing a single apostrophe will break the syntax. Does your data have apostrophe data?

EDIT - second idea Be aware that you MUST call conn.Close();

to close the database connection as soon as you are done with it, otherwise the connection will remain open on the database server until garbage collection starts, t21> instance. Alternatively, see Construction using () {}

.

Slightly grabbing at straws here, but try an explicit transaction.

SqlTransaction trans;
conn.Open(); 
try 
{
    trans = conn.BeginTransaction();
    int rows = cmd.ExecuteNonQuery(); 
    trans.Commit();
}
catch 
{
     trans.Rollback();
}
conn.Close();

      

        
        
        
      

    

Looking at the other discussion in this question, it appears that your query works, but something unrelated is wrong here. Some things to check that bite my ass sometimes:

• Are you really sure you are fulfilling the request that you think you are? Go to the debugger before clicking cmd.ExecuteNonQuery()
  
  . Are the parameters what you think they are? Is the command text still what you think it should be?

• Are you updating the correct database? Break into the debugger and check your connection string ( cs
  
  ). Perhaps you are still connected to the production database if you want to develop one, or vice versa?

• What happens if you execute a query manually in SQL Management Studio? Has the line updated what you think it should?