tags:

views:

1091

answers:

10

I have a very simple Update statement that will update mail server settings and network credentials info... Query works fine when I run it in Access but C# keeps giving me the error stating that my SQL Syntax is wrong ... I have a dataaccess layer (dal class) and Update instance method pasted belows ... But the problem must be sth else cuz I have updated lots of stuff this way but this time it just won't do .. any clues will be greatly appreciated. Thx in advance.

Update instance method in DAL class .. (this is supposed to be a Data Access Layer :) I'm just a management graduate :P

public int UpdateRow(string Query, bool isSP, params OleDbParameter[] args)
{
    int affectedRows = -1;
    using (con = new OleDbConnection(connStr))
    {
        using (cmd = con.CreateCommand())
        {
            cmd.CommandText = Query;
            if (isSP)
            {
                cmd.CommandType = CommandType.StoredProcedure;
            }
            if (args != null)
            {
                foreach (OleDbParameter prm in args)
                {
                    cmd.Parameters.Add(prm);
                }
            }

            try
            {
            con.Open();
            affectedRows = cmd.ExecuteNonQuery();
            }
            catch(OleDbException ex)
            {
            throw ex;
            }
            catch (Exception ex)
            {
            throw ex;
            }
        }
    }
    return affectedRows;
}

And the ASP.NEt codebehjind that will do the updating =

protected void Update_Click(object sender, EventArgs e) {
DAL dal = new DAL();
string upt = string.Format("UPDATE [MailConfig] SET Server='{0}', Username='{1}', Password='{2}', AddressFrom='{3}', DisplayName='{4}'",server.Text,username.Text,password.Text,replyto.Text,displayname.Text);
dal.UpdateRow(upt,false,null);
LoadData();
}

peace!

A: 

First off - don't use string.Format here. Use parameters, and add parameters to the command. Right now, you are wide open to SQL injection attacks. Think "Bobby Tables".

Re "stating that my SQL Syntax is wrong" - can you please quote the exact error?

Marc Gravell
Oh, and you don't need to catch an exception just to throw it - just let it bubble itself upwards.
Marc Gravell
A: 

First of all, you have no where clause in your Update, so it will update all rows, and violate key constraints causing an error, if you have any.

Second, running that kind of code makes you very vunerable to SQL Injection, if someone enters a username that has a sql command embedded in it, you could lose all your data.

You should use parameterized queries. You specify your parameters in the sql command with @paramname instead of using {4}, and then with the command object do accessCommand.parameters.AddWithValue("@paramname", value)

Tony Peterson
A: 

I know there is only one row on that table .. just keeping username pasword etc ... I have also added where statement no change ...

Ok when it comes to parameters, you are right, but I always parameterize my stuff later at the end, first it gots to work properly then I parameterize cuz you know its messy and harder to debug then ... This is just lazyness ..

About the details of my exception, there are two catch blocks as you see, and the last one catches it ... the Exception .. not the OleDBException .. and it only says Syntax error in UPDATE statement. Thats it ... While I debug, get the sql statement from visual studio, paste in Access and it works fine

A: 

You are using a CommandType of StoredProcedure, but your query is not a stored procedure name, its a sql query without a where clause.

"UPDATE [MailConfig] SET Server='{0}', Username='{1}', Password='{2}', AddressFrom='{3}', DisplayName='{4}'"

So you need to remove the command type line, or change it to a correct command type CommandType.Text, and add a Where clause specifying what rows are to be affected.

I don't think Access even has Stored Procedures, so there's no using to use that command type with it.

An example of a command that does use stored procedures would be something like:

string sqlCommString = "QCApp.dbo.ColumnSeek";
SqlCommand metaDataComm = new SqlCommand(sqlCommString, sqlConn);
metaDataComm.CommandType = CommandType.StoredProcedure;

The command string for that type is just the name of the stored proc.

Tony Peterson
A: 

No my Update method takes 3 parameters, like string Query, boll IsSp , params OleDBParameter[] .. and i usually do it the easy way, pass false for sp, so it doesnot fall into if(isSP){cmd.CommandType = storedproc etc..} .. it doesnot do that .. theres also a null check for params collection .. so I send null when I'm lazy, so it also passes that takes only the Query .. :) genius right

if you are truly calling a stored proc, then the command text needs to be the name of that proc, not a query string.
Tony Peterson
A: 

This is a small asp.net app for a dude I know .. this dude is going to send Mail messages around, and this is just a screen for him to define his mail server, user and pass so he can send emails around .. there is only one row so no need for a where statement .. I will put it anyway for sake of "best practices" :) Yes there is no StoredProcedure thing in access, but hopefully santa clause will bring LINQ to OleDB in 2009 :) So this query works fine, it does update the row but just C# keeps giving me this exception ... I dunno why really .. no clue

Oh I see now that the command type is only set to stored proc if the param passed to it is true.
Tony Peterson
+2  A: 

Trying wrapping your field names in [ ]. I have had problems in the past with certain field names such as a username and password and count, etc, being recognized as reserved words and screwing up the sql giving me an error.

Tony Peterson
A: 

yes I have initially made the AddressFrom field as only From, until I have realised that FROM was that famous FROM :) but yeah .. got that corrected too ...let me try that for all fields .. ı got no hope tough .. can sth like this happen? First time I got errors because I have used a reserved word From as a field name, then I got this error .. I fixed it but keep getting the error ... maybe somewhere deepdown in the asp.net engine sth got twisted ... wish I knew some pure C :)

A: 

hmmm... thanks Tony I really appreciate it ... I hate being this lazy , I have tought of doing this but sth prevents me ... I say no nuttin is wrong .. but [ and ] where the two key symbols to this problem .. it is solved thanks a lot

by the way I have tought of writing a static method somewhere that will return me a sql Parameters array in return of, say, a hashtable ... any best practices to recommend?

thanks in advance ...

A: 

Oh sure, this is my first time here, so I didn't want to pollute it actually .. You're right I will pay big attention to that from now on ... site is great by the way thanks for all ...