What looks wrong in this?

Question

private int DBUpdate() {

DAL dal = new DAL();
string upd = "UPDATE [RPform] SET [ProjectName] = '@pname', [ProjectCode] = '@pcode', [Country] = @cnt, ";
upd += "[StartDate] = '@startdate', [FinishDate] = '@finishdate', [TotalParticipants] = @totpart, [ArrivalDate] = '@arrivedate', ";
upd += "[AirportTransfer] = @airtran, [AirportDate] = '@airdate', [AirportHours] = @airhour, [AirportMinutes] = @airmin, ";
upd += "[Problems] = '@problems', [FirstDayActivities] = '@fdayact' ";
upd += "WHERE [UserID]=@usid";

    OleDbParameter[] parm = new OleDbParameter[] { 
    new OleDbParameter("@pname",projname.Text),
    new OleDbParameter("@pcode",projcode.Text),
    new OleDbParameter("@cnt",countries.SelectedIndex),
    new OleDbParameter("@startdate",datestart.Text),
    new OleDbParameter("@finishdate",datefinish.Text),
    new OleDbParameter("@totpart",totalparticipants.Text),
    new OleDbParameter("@arrivedate",datearrival.Text),
    new OleDbParameter("@airtran",RadioButtonList1.SelectedValue),
    new OleDbParameter("@airdate",dateairport.Text),
    new OleDbParameter("@airhour",airporthours.SelectedIndex),
    new OleDbParameter("@airmin",airportminutes.SelectedIndex),
    new OleDbParameter("@problems",problems.Value),
    new OleDbParameter("@fdayact",firstday.Value),
    new OleDbParameter("@usid",user.ID)
    };
 return (dal.UpdateRow(upd,false,parm));
}

/// It causes no exceptions, but returns 0 rows affected. When same query executed from within MS Access it works fine. Hence I suppose the problem is sth with the handling of parameters ... but what? Thank you

Sergio: is this OK, for setting OleDbTypes explicitly?

///whatever ...
        new OleDbParameter("@problems",problems.Value),
        new OleDbParameter("@fdayact",firstday.Value),
        new OleDbParameter("@usid",user.ID)
        };
//then telling each one what they will be ... 
        parm[0].OleDbType = OleDbType.VarWChar;
        parm[1].OleDbType = OleDbType.VarWChar;

///
     return (dal.UpdateRow(upd,false,parm));

You're certain that user.ID has a value that corresponds to a UserID value in RPForm? Also, how come some of your parameters in the query (eg @problems) are wrapped in single quotes? — Matt Hamilton, Dec 23 '08 at 11:47
Are you sure your DAL object is completing the update? Did you step through the code and did you check your database to see if the update is completed as expected? — BenAlabaster, Dec 23 '08 at 15:58

score 6 · Answer 1 · answered Dec 23 '08 at 11:54

6

Check if the value of user.ID is being correctly set.

In your upd command string you are surrounding the parameters with quotes, like this:

[StartDate] = '@startdate'

Remove those quotes in all your parameters.

answered Dec 23 '08 at 11:54

Nelson Reis

4,780
9
43
61

Nelson Reis · Answer 2 · 2008-12-23T14:19:33.313

Sorry emre, I overlooked the OleDb Provider. Your SQL command syntax is correct if it was intended for an SQL provider.

So, your command should look like this:

string upd = "UPDATE [RPform] SET [ProjectName] = ?, [ProjectCode] = ?, [Country] = ?, ";
upd += "[StartDate] = ?, [FinishDate] = ?, [TotalParticipants] = ?, [ArrivalDate] = ?, ";
upd += "[AirportTransfer] = ?, [AirportDate] = ?, [AirportHours] = ?, [AirportMinutes] = ?, ";
upd += "[Problems] = ?, [FirstDayActivities] = ? ";
upd += "WHERE [UserID]=?";

Then you should add your OleDb parameters just like you already have, but you have to be careful to add them in the same order they appear in you SQL command.

One more thing, but that don't have to do with your problem: you shouldn't concatenate strings, because that operation isn't very good in terms of performance.

Instead, to lay out your SQL command nicely, try this:

StringBuilder upd = new StringBuilder();
upd.Append("UPDATE [RPform] SET [ProjectName] = ?, [ProjectCode] = ?, [Country] = ?, ");
upd.Append("[StartDate] = ?, [FinishDate] = ?, [TotalParticipants] = ?, [ArrivalDate] = ?, ");
upd.Append("[AirportTransfer] = ?, [AirportDate] = ?, [AirportHours] = ?, [AirportMinutes] = ?, ");
upd.Append("[Problems] = ?, [FirstDayActivities] = ? ");
upd.Append("WHERE [UserID]=?");

Then, to get your command string, you just need:

upd.ToString();

Hope that this could help you.

+1: OleDb needs positional parameters (and in any case SQL named parameters should not be enclosed in single quotes). Also you can declare a multi-line string constant for the sql statement as string upd = @"UPDATE ... "; A multiline string constant is easier to read and to copy/paste. — Joe, Dec 23 '08 at 14:57

Joe · Answer 3 · 2008-12-23T18:35:03.703

Just to comment that rather than concatenating, it's more readable (and easier to copy/paste) if you use a multiline string literal. Also you should not enclose parameter names in single quotes, even for string parameters (use single quotes for literals only). So your original SQL could look something like:

string upd = @"
UPDATE [RPform] SET [ProjectName] = @pname, [ProjectCode] = @pcode, [Country] = @cnt, 
[StartDate] = @startdate, [FinishDate] = @finishdate, [TotalParticipants] = @totpart, [ArrivalDate] = @arrivedate, 
[AirportTransfer] = @airtran, [AirportDate] = @airdate, [AirportHours] = @airhour, [AirportMinutes] = @airmin, 
[Problems] = @problems, [FirstDayActivities] = @fdayact 
WHERE [UserID]=@usid
";

As others have pointed out, OleDb does not use named parameters, so you should actually be using the following, ensuring that you add parameters to your OleDbCommand in the same order they appear in the SQL statement:

string upd = @"
UPDATE [RPform] SET [ProjectName] = ?, [ProjectCode] = ?, [Country] = ?, 
[StartDate] = ?, [FinishDate] = ?, [TotalParticipants] = ?, [ArrivalDate] = ?, 
[AirportTransfer] = ?, [AirportDate] = ?, [AirportHours] = ?, [AirportMinutes] = ?, 
[Problems] = ?, [FirstDayActivities] = ? 
WHERE [UserID]=?
";

+1. This form is much better than using a stringbuilder to make your query in just about every way. — Kibbee, Dec 23 '08 at 16:05

score 2 · Answer 4 · answered Dec 23 '08 at 11:52

2

If you set the parameter type in each new OleDbParameter you will not need to place single quotes on your sql making the instruction less prone to typing mistakes.

answered Dec 23 '08 at 11:52

Sergio

8,125
10
46
77

This is probably the problem. You should never quote parameters, the database driver does that itself. – Rune Grimstad Dec 23 '08 at 12:39

score 2 · Answer 5 · answered Dec 23 '08 at 16:38

Don't use the StringBuilder in this case. Use verbatim strings(@) with well formatted SQL code:

var updateStatement = @"
    UPDATE [RPform]
    SET     [ProjectName]        = @pname     ,
            [ProjectCode]        = @pcode     ,
            [Country]            = @cnt       ,
            [StartDate]          = @startdate ,
            [FinishDate]         = @finishdate,
            [TotalParticipants]  = @totpart   ,
            [ArrivalDate]        = @arrivedate,
            [AirportTransfer]    = @airtran   ,
            [AirportDate]        = @airdate   ,
            [AirportHours]       = @airhour   ,
            [AirportMinutes]     = @airmin    ,
            [Problems]           = @problems  ,
            [FirstDayActivities] = @fdayact
    WHERE   [UserID]             =@usid";

But I have to note that += will not be optimized! The simple + will be performed at compile time. For example

string s = "A" + "B" + C";

will result in "ABC" no perf hit However

string s = "A" + variable + C";

will be not optimized.

score 1 · Answer 6 · answered Dec 23 '08 at 12:35

By the way, you'll probably want to use a StringBuilder instead of a string to create upd. Every time you use += you're throwing away your old string a creating a new one.

Try

StringBuilder upd = new StringBuilder();
upd.Append("UPDATE [RPform] SET [ProjectName] = '@pname', [ProjectCode] = '@pcode', [Country] = @cnt, ");
upd.Append("[StartDate] = '@startdate', [FinishDate] = '@finishdate', [TotalParticipants] = @totpart, [ArrivalDate] = '@arrivedate', ");
upd.Append("[AirportTransfer] = @airtran, [AirportDate] = '@airdate', [AirportHours] = @airhour, [AirportMinutes] = @airmin, ");
upd.Append("[Problems] = '@problems', [FirstDayActivities] = '@fdayact' ");
upd.Append("WHERE [UserID]=@usid");

And to use it, simply

upd.ToString();

But string builders have a bit of overhead associated with them. In this case, with only 6 concatenations, it really doesn't make sense to use a string builder. You'd be most likely better off just removing the ;'s, and concating all the strings. The compiler may optimize this to a single string. — Kibbee, Dec 23 '08 at 15:55

score 0 · Answer 7 · answered Dec 23 '08 at 16:16

Aight! I solved it listening to Deltron 3030 .. it always works :)

This OleDbParameter object sets its oledbtype internally, according to the type I have passed (as object to the ctor) .. so I have passed an integer coming from a TextBox (client side validated but still text, so it crashes) .. also the bool one was radiobutton.Value .. which sends True but still text so it assumes as string and encapsulates it in single quotes ... But I cannot see what names to give these params since they are nameless ...

Sorry I hate reading long articles, instead I use one of the great virtues of a programmer which is lazyness .. So I ask here :)

Thank you all for your time and efforts .. Wish you guys all the success

What looks wrong in this?

7 Answers7