Jump to content

Why isn't this function updating table?


colap

Recommended Posts

function updateTable($dbh, $table, $columns, $val, $conditions) {		
			$set="";
			for($i=0;$i<count($columns);$i++) {
				$t=$columns[$i] . "=:" . $columns[$i];
				$set=$set . $t;
				if($i!=count($columns)-1)
					$set=$set . ",";
			}
			$cond=$conditions;
			$akeys=array_keys($conditions);
			$last_key=end($akeys);
			$where="";
			foreach($conditions as $k=>$v) {
				$where=$where . "$k=:$k";
				if($k != $last_key)
					$where=$where . " and ";
			}
			$sql="update $table set $set where $where";
			$stmt=$dbh->prepare($sql);			
			for($i=0;$i<count($columns);$i++) {
				$stmt->bindParam(":$columns[$i]",$val[$i]);
			}
			foreach($conditions as $k=>$v) {
				$stmt->bindParam(":$k",$v);
			}
			//exit;
			$stmt->execute();
			$stmt=null;			
	}

updateTable($dbh, "wd", ["sentence","meaning"], [$sentence,$meaning], ["word"=>$wd,"id"=>"1"]);

 

It's not updating table row and it's not showing any error too. Why is this?

 

But updateTable($dbh, "wd", ["sentence","meaning"], [$sentence,$meaning], ["word"=>$wd,"id"=>"1"]); this works.

 

Link to comment
Share on other sites

the two test cases are identical (i copied and compared them.) i suspect the second one only had a single term in the WHERE clause?

 

the reason for this is due to using bindParam() inside of a loop, which is binding the single variable, $v, both times, and then evaluates that variable at the ->execute() statement. This resulted in the last value in $v after the end of the loop, "1", being used for both the word and id columns, which resulted in a false WHERE clause.

 

bindParam() should only be used when you are going to execute the query multiple times, which you are not doing AND you need to specify the data type, which is rare.

 

you should instead forget about binding input data and just build an array of all the input parameters and supply it to the ->execute() call (note: this can be used when executing the query multiple times.)

 

next, if you build arrays of the set terms and the where terms and implode() them, which will work correctly for one or more terms, a lot of this code will disappear. you won't have to have logic to detect the last term.

Link to comment
Share on other sites

Archived

This topic is now archived and is closed to further replies.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.