Thread: UPDATE syntax problem
Hi,
I'm developing a C++ script to update postgresql database records. The
user interacts with the script via an html form. That is, the user is
presented with the data from a particular record in an html form and
asked to update any number of fields in that record.
To perform a multiple column update in postgres one does:
UPDATE tablename
SET column1 = 'blahblah',
column2 = 'moreblahblah',
column3 = 1234
WHERE id = 555;
Here's an excerpt from the script:
//sql statement
string sql;
sql = "UPDATE product SET ";
if (param["new_catid"] != param["old_catid"])
{
sql += "catid = " + param["new_catid"] + ",";
}
else if (param["new_catname"] != param["old_catname"])
{
sql += "prodname = '" + param["new_catname"] + "',";
}
else if (param["new_unitcnt"] != param["old_unitcnt"])
{
sql += "unitcnt = '" + param["new_unitcnt"] + "',";
}
else if (param["new_wprice"] != param["old_wprice"])
{
sql += "wprice = " + param["new_wprice"];
}
sql += "WHERE prodid = '" + param["prodid"] + "'";
int res = conn.Exec (sql.c_str()); //sql exec
Now the problem occurs when a user only wants to update certain columns,
which creates a syntax problem due to the comma (","). In other words
you cannot end a multiple column UPDATE statement with a comma followed by:
WHERE prodid = 'xyz';
Now I could probably solve this problem by building separate UPDATE
statements as such:
if (param["new_catid"] != param["old_catid"])
{
sql = "UPDATE product SET ";
sql += "catid = " + param["new_catid"];
sql += "WHERE prodid = '" + param["prodid"] + "'";
int res = conn.Exec (sql.c_str()); //sql exec
}
else if (param["new_catname"] != param["old_catname"])
{
sql = "UPDATE product SET ";
sql += "prodname = '" + param["new_catname"] + "'";
sql += "WHERE prodid = '" + param["prodid"] + "'";
int res = conn.Exec (sql.c_str()); //sql exec
}
This necessitates calling the database each time the if statement is
true. Is there perhaps a more efficient way of doing this? I'm assuming
that the fewer times you call the database the faster the program will run.
Thanks,
Mark Tessier
MT <mt@open2web.com> writes:
> Hi,
>
> I'm developing a C++ script to update postgresql database records. The
> user interacts with the script via an html form. That is, the user is
> presented with the data from a particular record in an html form and
> asked to update any number of fields in that record.
>
> To perform a multiple column update in postgres one does:
>
> UPDATE tablename
> SET column1 = 'blahblah',
> column2 = 'moreblahblah',
> column3 = 1234
> WHERE id = 555;
>
> Here's an excerpt from the script:
[snip]
A couple points:
1) You're wide open to an SQL injection attack. You'll need to
preprocess the parameter values to make sure single quotes are
properly escaped before building the SQL statement.
2) The code structure you're using is awkward--you have to add a new
clause if you add a parameter. I'd be more likely to make a list
of parameters, and loop through it checking for changed values and
adding clauses to the SQL statement (this would be a good time to
do the quote escaping). If no values have changed, just don't
execute the SQL at all (your code doesn't handle this case).
Try to think at a higher level of abstraction.
> Now I could probably solve this problem by building separate UPDATE
> statements as such:
>
> if (param["new_catid"] != param["old_catid"])
> {
> sql = "UPDATE product SET ";
> sql += "catid = " + param["new_catid"];
> sql += "WHERE prodid = '" + param["prodid"] + "'";
> int res = conn.Exec (sql.c_str()); //sql exec
> }
> else if (param["new_catname"] != param["old_catname"])
> {
> sql = "UPDATE product SET ";
> sql += "prodname = '" + param["new_catname"] + "'";
> sql += "WHERE prodid = '" + param["prodid"] + "'";
> int res = conn.Exec (sql.c_str()); //sql exec
> }
>
> This necessitates calling the database each time the if statement is
> true. Is there perhaps a more efficient way of doing this? I'm assuming
> that the fewer times you call the database the faster the program will run.
Surely. Whether it's an issue for you depends on how busy the site is
likely to be. You should also use transactions if you do it this way
to make sure other users don't see a half-updated record.
-Doug
Hello,
I would just like to follow up on what you suggested since it went a
little over my head.
> A couple points:
>
> 1) You're wide open to an SQL injection attack.
What's that?
> You'll need to
> preprocess the parameter values to make sure single quotes are
> properly escaped before building the SQL statement.
Do you mean:
string category = \'param["new_prodname"]\'
Does this prevent an sql injection attack?
> 2) The code structure you're using is awkward--you have to add a new
> clause if you add a parameter. I'd be more likely to make a list
> of parameters, and loop through it checking for changed values and
> adding clauses to the SQL statement (this would be a good time to
> do the quote escaping). If no values have changed, just don't
> execute the SQL at all (your code doesn't handle this case).
I'm not sure how this is done. I would appreciate it if you could
elaborate on this by perhaps providing a quick example.
The following is an excerpt from my script:
if (param["new_catid"] == param["old_catid"] && \
param["new_prodname"] == param["old_prodname"] && \
param["new_unitcnt"] == param["old_unitcnt"] && \
param["new_wprice"] == param["old_wprice"])
{
HTMLstream reply("goodbye.html");
reply.set_field("msg1", "No modification");
reply.set_field("msg2", "NO modification");
reply.set_field("msg3", "You didn't modify the select product");
reply.send();
return 0;
}
string new_catid = param["new_catid"];
if (param["new_catid"] == "")
{
new_catid = param["old_catid"];
}
//sql UPDATE statement
string sql;
sql = "UPDATE product SET ";
sql += "prodname = '" + param["new_prodname"] + "',";
sql += "wprice = " + param["new_wprice"] + ",";
sql += "unitcnt = '" + param["new_unitcnt"] + "',";
sql += "catid = " + new_catid;
sql += " WHERE prodid = '" + param["prodid"] + "'";
int res = conn.Exec (sql.c_str()); //sql exec
This works, but I'm always interested in finding better ways to do
things. Your way looks better. I realize this is more a programming
question than a postgres question. By the way, should I be using
transactions if I do it this way, or the way you have suggested?
Thanks,
Mark Tessier
MT <mt@open2web.com> writes: > Hello, > > I would just like to follow up on what you suggested since it went a > little over my head. > > > A couple points: > > > > 1) You're wide open to an SQL injection attack. > > What's that? STFW. http://www.google.com/search?hl=en&ie=ISO-8859-1&q=SQL+injection+attack > > > You'll need to > > preprocess the parameter values to make sure single quotes are > > properly escaped before building the SQL statement. > > Do you mean: > > string category = \'param["new_prodname"]\' > > Does this prevent an sql injection attack? Not quite--you need to look inside the string for single quote characters and escape them. It's a bit tedious but not hard. > > 2) The code structure you're using is awkward--you have to add a new > > clause if you add a parameter. I'd be more likely to make a list > > of parameters, and loop through it checking for changed values and > > adding clauses to the SQL statement (this would be a good time to > > do the quote escaping). If no values have changed, just don't > > execute the SQL at all (your code doesn't handle this case). > > I'm not sure how this is done. I would appreciate it if you could > elaborate on this by perhaps providing a quick example. Sure. I don't actually know C++ that well--I use Perl and Java generally, and I've been writing Perl tonight, so the below is mostly Perlish--but you should be able to follow along: $any_changed = 0; # false $sql = "UPDATE mytable SET "; @param_list = ('catid', 'prodname', 'unitcnt', 'price'); # create a list foreach $p (@param_list) { # iterate through it if ($param{"old_$p"} ne $param{"new_$p"}) { $any_changed = 1; if ($p ne $param_list[0]) { # if we're not on the first element $sql .= ", "; # put in a comma } $fixed_param = escape_param($param{"new_$p"}); # escape single quotes $sql .= "$p = '" . $fixed_param . "'"; } if ($any_changed) { $sql .= " WHERE prod_code = '4455GGF'"; exec_sql($sql); } } Here, '.' is the Perl string concatenation operator (instead of '+') and variable values are interpolated into double-quoted strings for you (so "new_$p" ends up being "new_catid", say). '#' denotes a comment just as '//' does in C++. You get the idea? This way, if you add a parameter, you just add it to the array, rather than copy/pasting a bunch of code and hacking it around. > This works, but I'm always interested in finding better ways to do > things. Your way looks better. I realize this is more a programming > question than a postgres question. By the way, should I be using > transactions if I do it this way, or the way you have suggested? Basically, you should use transactions any time you want to execute two or more SQL statements that should be seen as a unit by other database users. -Doug
On Sat, Dec 07, 2002 at 02:32:48PM -0500, MT wrote: > Hi, > > I'm developing a C++ script to update postgresql database records. The > user interacts with the script via an html form. That is, the user is > presented with the data from a particular record in an html form and > asked to update any number of fields in that record. > > To perform a multiple column update in postgres one does: > > UPDATE tablename > SET column1 = 'blahblah', > column2 = 'moreblahblah', > column3 = 1234 > WHERE id = 555; Heh, my cheap and hacky why is to end each column = value clause with a comma. Then i finish it off with a "id=id WHERE ...". That clause becomes a noop and the syntax is fine. Oh yeah, check out the SQL injection attacks. Nasty :) -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > Support bacteria! They're the only culture some people have.
Attachment
Using pltcl...
I just strip the comma, if it's there, when I'm all done with the " col = 'value' " bit.
string trimright $sql {,}
We all have our cheap hacks to bear....
The built-in [quote $value] in pltcl is handy for fending off injection attacks.
>>> Martijn van Oosterhout <kleptog@svana.org> 12/09/02 02:51AM >>>
On Sat, Dec 07, 2002 at 02:32:48PM -0500, MT wrote:
> Hi,
>
> I'm developing a C++ script to update postgresql database records. The
> user interacts with the script via an html form. That is, the user is
> presented with the data from a particular record in an html form and
> asked to update any number of fields in that record.
>
> To perform a multiple column update in postgres one does:
>
> UPDATE tablename
> SET column1 = 'blahblah',
> column2 = 'moreblahblah',
> column3 = 1234
> WHERE id = 555;
Heh, my cheap and hacky why is to end each column = value clause with a
comma. Then i finish it off with a "id=id WHERE ...". That clause becomes a
noop and the syntax is fine.
Oh yeah, check out the SQL injection attacks. Nasty :)
--
Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/
> Support bacteria! They're the only culture some people have.
Just to keep it clean, replace the last character with space before adding "WHERE ...". Martijn van Oosterhout wrote: > > On Sat, Dec 07, 2002 at 02:32:48PM -0500, MT wrote: > > Hi, > > > > I'm developing a C++ script to update postgresql database records. The > > user interacts with the script via an html form. That is, the user is > > presented with the data from a particular record in an html form and > > asked to update any number of fields in that record. > > > > To perform a multiple column update in postgres one does: > > > > UPDATE tablename > > SET column1 = 'blahblah', > > column2 = 'moreblahblah', > > column3 = 1234 > > WHERE id = 555; > > Heh, my cheap and hacky why is to end each column = value clause with a > comma. Then i finish it off with a "id=id WHERE ...". That clause becomes a > noop and the syntax is fine. > > Oh yeah, check out the SQL injection attacks. Nasty :) > -- > Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > > Support bacteria! They're the only culture some people have. > > ------------------------------------------------------------------------ > Part 1.2Type: application/pgp-signature
MT wrote:
> Hi,
>
> }
> else if (param["new_unitcnt"] != param["old_unitcnt"])
> {
> sql += "unitcnt = '" + param["new_unitcnt"] + "',";
> }
> else if (param["new_wprice"] != param["old_wprice"])
> {
> sql += "wprice = " + param["new_wprice"];
> }
>
> sql += "WHERE prodid = '" + param["prodid"] + "'";
>
> int res = conn.Exec (sql.c_str()); //sql exec
>
> Now the problem occurs when a user only wants to update certain columns,
> which creates a syntax problem due to the comma (","). In other words
> you cannot end a multiple column UPDATE statement with a comma
> followed by:
Before you process the WHERE clause....you need to backtrack and delete any
dangling ','. I'm not sure how you'd do it in pure C++ but in perl you'd say
$SQL =~ s/,$//
ie remove the last comma....
You'll also run into this problem when constructing WHERE clause items
where you
have have a 'AND' or similar operators at the end....
> er the program will run.
>
> Thanks,
>
> Mark Tessier
>
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)