MySQL or PHP syntax oversight when trying to perform conditional update

Question

I think this is an escaping issue or something. When I execute the query and populate all variables, everything is peachy and all row is updated properly in the DB.

I looked on StackOverflow to get me rolling with these dynamic/contructed on the fly queries and I'm at the end of my rope.

My stuff looks like this:

$sql="UPDATE users SET ";

if (!empty($fname)) { "fname = '$fname', ";}

if (!empty($lname)) { "lname = '$lname', ";}

if (!empty($location)) { "location = '$location', ";}

if (!empty($url)) { "url = '$url', ";}

"WHERE id = '$id' LIMIT 1";

When I break the query to insert the "IFs" I keep getting the following: Error: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 1

I ECHO'd the query and for some odd reason it's nto complete and the variables are coming in before the query start like so

fname = 'Rob', lname = 'Smith', location = 'Jersey City, NJ', url = 'http://somesite.com', UPDATE users SET Error: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 1

Sorry if I am not clear. I will clarify where needed. I am new at all this. Thank you!

Answer 1

echo out your query and take a look at the commas in your SET caluse. Do you have too many? Not enough? I think you'll find that you have one extra comma. You'll probably want to use the implode() function to build up your SET clause. This will insert the appropriate number of commas in the appropriate places.

Answer 2

I see two problems, there is no space before WHERE which means it could turn out "url=http://www.stackoverflow.com"WHERE" and maybe cause a problem.

Also, there is a comma at the end of every SET clause, the last one in the list should not have a comma.

Answer 3

You're not allowed to have a comma after the last thing you SET.

One easy solution is this:

$set = array();
if (!empty($fname)) { $set[] = "fname = '$fname'";}
if (!empty($lname)) { $set[] = "lname = '$lname'";}
if (!empty($location)) { $set[] = "location = '$location'";}
if (!empty($url)) { $set[] = "url = '$url'";}

if(!empty($set)) {
  $sql = "UPDATE users SET ";
  $sql .= implode(', ', $set)
  $sql .= " WHERE id = '$id' LIMIT 1";
}

Oh, and make sure the variables you're shoving in the query are SQL safe; otherwise you've got a SQL injection issue.

Answer 4

Remember in these programming languages, each statement (text ending with a ;) is much like a complete sentence. You need a subject-object-verb for it to make sense. I can't just say

 doggy;

I have to say

 feed the doggy;

Similarly, I can't just say

 "fname = '$fname', "

when I mean "Append this string to the query I started earlier". I have to be explicit:

 $sql .= "fname = '$fname', ";

I'm saying "Append this text to $sql". Its a complete sentence.

Answer 5

better to put all your SETs into an array and implode them into a string. That way you can be sure there are no dangling commas. Something like:

if (!empty($fname)) $sets[]="fname = '$fname' ";
if (!empty($lname)) sets[]= "lname = '$lname' ";
if (!empty($location)) sets[]= "location = '$location' ";
if (!empty($url)) sets[]= "url = '$url' ";

$setstring= implode(',',$sets);
if($setstring) {
  $query="UPDATE users SET $sets WHERE id = '$id' LIMIT 1";
  //run query, etc.
}

Answer 6

Not really a direct answer but for dynamic queries i suggest using PDO. That way you can specify optional parameters more secure, elegant and easier.

<?php
$stmt = $dbh->prepare("INSERT INTO REGISTRY (name, value) VALUES (:name, :value)");
$stmt->bindParam(':name', $name);
$stmt->bindParam(':value', $value);

// insert one row
$name = 'one';
$value = 1;
$stmt->execute();

// insert another row with different values
$name = 'two';
$value = 2;
$stmt->execute();
?>

If your queries become larger, the way you are doing things now will be pretty complicated to maintain.

Answer 7

Is it better to use !empty($fname) vs. isset($fname) or does it not matter? Is there a different between the 2 functions in the case of your recommended solution?