UPDATE syntax problem

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&amp;ie=ISO-8859-1&amp;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/

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/

Just to keep it clean, replace the last character with space before
adding "WHERE ...".

Martijn van Oosterhout wrote:

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....