You should use a parameterized query for several reasons:
- eliminate the possibility of an embedded apos breaking your query
- protecting your database/website from sql injection
Also, if the cmd returns 1 then the row was updated. You may need to examine 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 that look much cleaner? And when you understand why, it will give you a warm fuzzy feeling and let you sleep well at night. ;-)
Clarification:
Your stated question was "What is wrong with this query". The answer is nothing. Nothing is wrong with the query.
The problem is with your page code, which you did not post even after 5 people suggested that there is nothing wrong with the query.
What I meant by 'You may need to examine your expectations...' and 'you need to examine the data going into the parameters BEFORE executing the query to ensure they are what you want' is more clearly described by John, but let me briefly point it out again:
The query as shown is ostensibly correct and should perform as expected. What you are likely experiencing is a flaw in the logic of the surrounding code.
Most likely you are databinding in page_load without an IsPostback guard thus overwriting your input values with the original data.
You need to load and bind your controls/page once only upon the first load. Thereafter the state of the controls are persisted in viewstate, which is stored in a hidden field in the html. If you simply bind to the data source on each page load you will overwrite any input
So lets examine how this works.
Here is a proper 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' into the textbox and click button one, this is what Trace shows:
Page_Load
Bind TextBox1
TextBox1.Text = Initial Value
Page_Load
TextBox1.Text = New Value
Button1_Click
TextBox1.Text = New Value
But without the guard:
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 get the results that you are likely experiencing...
Page_Load
Bind TextBox1
TextBox1.Text = Initial Value
Page_Load
Bind TextBox1
TextBox1.Text = Initial Value
Button1_Click
TextBox1.Text = Initial Value
But again, a practical debugging technique that would help you is to break in the update method just before the query is executed and examine the values to verify that they are what you think they are and then go from there.