Problem getting array values to update in table

PHP
1

I’m trying to get some array data to update via a form and for some reason, although it doesn’t throw an error, it doesn’t update- it’s as if the whole code is ignored.

The code below is part of a form, and shows the existing data for the record.

$getmotdetails = $mysqli->query('SELECT * FROM mothistorytable WHERE carid='.$refnumber); 
while ($row = $getmotdetails->fetch_assoc()) {
	  $numberofrows = $getmotdetails->num_rows;
	  $motcarid = $row["carid"];
      $motdate = $row["motdate"];
      $motodometer = $row["motodometer"];
      $customorder = $row["CustomOrder"];
      $id = $row["id"];
	  
	  $output = ""; 
	  
	  for ($i=0; $i<$numberofrows; $i++) { 
     $output = ' <div class="addnewcar-wide"><input type="hidden" name="_ID['.$i.']" value='.$id.' /><label for="_MotDate">Date</label><input name="_MotDate['.$i.']" type="text" size="10" value="'.$motdate.'" /><label for="_MotOdometer">Odometer</label><input name="_MotOdometer['.$i.']" type="text" size="20" value="'.$motodometer.'" /></div>'; 
    } 
	echo $output;
	$count++ ;
}

Below is the code that should make any updates to the fields in the form:

$motdate = $_POST['_MotDate'];    
$motodometer = $_POST['_MotOdometer']; 
$id = $_POST['_ID']; 
   
for ($i=0; $i<count($motdate); $i++) {    
    if ($motdate[$i]!="" && $motodometer[$i]!="") {   
		
		$updatemotdetails = $mysqli->query('UPDATE mothistorytable SET motdate = "'.$motdate.'", motodometer = "'.$motodometer.'" WHERE `id` = '.$id);
    }  
}

If I echo the $motdate, $motodometer and $id variables I get nothing, so it’s as if the form just isn’t sending anything through (but I don’t get any error). But the form displays the existing values ok in the inputs.

The weird thing is that a similar query, for adding some new values from blank inputs, works fine:

echo '<p><strong>Add any new MOT details required in the slots below.</strong></p>';

    $newoutput = ""; 
    $desired_row_count = 20;  
     
    for ($i=0; $i<$desired_row_count; $i++) { 
     $newoutput .= ' <div class="addnewcar-wide"><label for="_NewMotDate">Date</label><input name="_NewMotDate['.$i.']" type="text" size="10" /><label for="_NewMotOdometer">Odometer</label><input name="_NewMotOdometer['.$i.']" type="text" size="20" /></div>'; 
    } 
    echo $newoutput;

And then:

//MOT ADD NEW RECORDS FORM
$newmotdate = $_POST['_NewMotDate'];    
$newmotodometer = $_POST['_NewMotOdometer']; 

echo $newmotdate;
   
for ($i=0; $i<count($newmotdate); $i++) {    
    if ($newmotdate[$i]!="" && $newmotodometer[$i]!="") {  
       $addmotdetails = $mysqli->query('INSERT INTO mothistorytable (`carid`,`motdate`,`motodometer`) VALUES ('.$carid.',"'.$newmotdate[$i].'","'.$newmotodometer[$i].'")');    
    }  
}
2

Spot the difference:

$getmotdetails = $mysqli->query('SELECT * FROM mothistorytable WHERE carid='.$refnumber);

in your selection code, but

$updatemotdetails = $mysqli->query('UPDATE mothistorytable SET motdate = "'.$motdate.'", motodometer = "'.$motodometer.'" WHERE `id` = '.$id);

in your update code. The id column seems to have changed from carid to id.

That first bit of code doesn’t seem correct to me, though, even if the above minor typo is the reason your updates aren’t working. It looks to me as if you’re running a query, opening a while() loop to read each row returned by the query, but within each iteration of that loop you run a for() loop up to the number of rows returned and set the $output variable. But it works OK because you only echo $output after the for() loop is finished - what’s the reason for that for() loop in there?

3

That’s fine- the first SELECT pulls in all records which have a carid of a certain value (these are MOT records for a particular car). The UPDAT query is for updating specific MOT records that have changed, which is why I refer to id which is the primary key for the table.

The “for” loop was to basically run the update for all the records that were outputted in the form so that if they’ve changed then the changes are put into the table- but it doesn’t seem to work.

The INSERT query works fine however. So the user can add new records, but I want them to be able to view and amend existing ones if they so choose, as well…

4

That code is wide open to SQL injection attack as you’re letting user submitted data near the database without sanitizing and escaping it. You should use prepared statements when sending data in a query to the database and be sanitizing the input to make sure that it’s what you’re expecting, eg; type, size, etc

5

I was originally using mysqli_real_escape_string but for some reason it wouldn’t work with these array variables (presumably because they’re not strings) which is why they’re just POST values.

The code is actually on a restricted access page. I’ve heard of using prepared statements but I haven’t yet found a tutorial that makes sense to someone who hasn’t used them before… if you know of one that makes sense to a newcomer on the topic, I’d be grateful!

Still can’t figure why the UPDATE query isn’t working, or rather why the values aren’t going through to it…

6

Sorry, I meant the for loop in your first bit of code, where you output the form details in the first place. As I read it, you loop through the recordset, and for each row you get the total number of rows returned, then run through a loop from 0 to totalrows-1, and in that loop you assign a value to $output (assign, note, not concatenate as you do in the code that does work) then after the loop has ended, you echo $output to the browser. To me, that will display a set of inputs that have the correct data but always have a value for $i (which you use for the array element numbers) equal to the number of rows returned by your query. If you display the html produced for your first form, are those values correct or not? I’ve added comments and questions in your code below:

$getmotdetails = $mysqli->query('SELECT * FROM mothistorytable WHERE carid='.$refnumber); 
while ($row = $getmotdetails->fetch_assoc()) {
   $numberofrows = $getmotdetails->num_rows;   // why get this every time we read a row? It won't change.
   $motcarid = $row["carid"];
   $motdate = $row["motdate"];
   $motodometer = $row["motodometer"];
   $customorder = $row["CustomOrder"];
   $id = $row["id"];
   $output = ""; 
   for ($i=0; $i<$numberofrows; $i++) { // why loop for the number of rows here? Surely just output the current one
      $output = ' <div class="addnewcar-wide"><input type="hidden" name="_ID['.$i.']" value='.$id.' /><label for="_MotDate">Date</label><input name="_MotDate['.$i.']" type="text" size="10" value="'.$motdate.'" /><label for="_MotOdometer">Odometer</label><input name="_MotOdometer['.$i.']" type="text" size="20" value="'.$motodometer.'" /></div>'; 
    } 
   echo $output; // at this point, this just contains the current row details, with $i = $numberofrows - 1
   $count++ ;
}
7

Hi, thanks for the reply, yes the form that displays the current values (for the user to change if they wish to) shows them correctly- this particular car has four MOT dates and readings, so there are four records outputted which is correct.

So as far as I can tell, the form values are being passed through ok (certainly they’re displaying correctly). If I echo them in the UPDATE query then I get “Array” outputted so something is clearly going through- but the UPDATE query is just being ignored…

I commented out the FOR loop and tried again, exactly the same result.

8

I’d expect it to display the correct data values from your code, but I’d expect it to have an incorrect value for the array element number in $i. When the form is displayed with the correct details in it, what do you see when you right-click and ‘view source’ for the form html? If you comment out the for loop but leave in the line where you assign $output, surely it will always have a value for $i of zero? Shouldn’t you use $count instead?

9

This is what I get in the page source code:

<p>MOT DETAILS</p>
<div class="addnewcar-wide">
<input type="hidden" name="_ID[]" value=2350 />
<label for="_MotDate">Date</label>
<input name="_MotDate[]" type="text" size="10" value="15.09.1980" />
<label for="_MotOdometer">Odometer</label>
<input name="_MotOdometer[]" type="text" size="20" value="45,893" /></div>
<div class="addnewcar-wide">
<input type="hidden" name="_ID[]" value=2351 />
<label for="_MotDate">Date</label>
<input name="_MotDate[]" type="text" size="10" value="17.09.1981" />
<label for="_MotOdometer">Odometer</label>
<input name="_MotOdometer[]" type="text" size="20" value="56,987" /></div>
<div class="addnewcar-wide">
<input type="hidden" name="_ID[]" value=2348 />
<label for="_MotDate">Date</label>
<input name="_MotDate[]" type="text" size="10" value="22.02.1976" />
<label for="_MotOdometer">Odometer</label>
<input name="_MotOdometer[]" type="text" size="20" value="34,897" />
</div><div class="addnewcar-wide">
<input type="hidden" name="_ID[]" value=2349 />
<label for="_MotDate">Date</label>
<input name="_MotDate[]" type="text" size="10" value="24.02.1977" />
<label for="_MotOdometer">Odometer</label>
<input name="_MotOdometer[]" type="text" size="20" value="36,984" /></div>

Not sure how I’d use $count instead of $I as I’m using it at the moment to just go through the WHILE loop.

10

Your problem is here in your first code block:

while ($row = $getmotdetails->fetch_assoc()) {
    // ...
    $output = ""; 
    for ($i=0; $i<$numberofrows; $i++) { 
        $output = '...'; 
    } 
    echo $output;
}

The for loop is overwriting $output on each iteration, meaning that although you get the correct number of outputs, the array index for each one is set to the same value (the last value of $i).

When you try to iterate over the $_POST values in your update script, your for loop starts with $i = 0, but the array doesn’t have a value for index zero and so throws an ‘undefined index’ error, which you won’t see unless you have display_errors turned on or you check the error log.

11

Yes, I was thinking that you needed to specify the array element as something like “_MotDate[0]” and “_MotDate[1]” when of course you can leave that blank. I think in the earlier code, though, before you removed the for-loop, it would always have been using “_MotDate[20]” if you had 21 rows returned by the query, because you were putting $i in the html code in your loop. Using $count would mean that the first tow specified “_MotDate[0]”, the second “_MotDate[1]” and so on.

I can’t see the issue, so the next step is to add echo statements into your update script - so create a variable for your query and echo that, complete with each piece of data, then copy/paste it into phpmyadmin and see if there’s an SQL issue with quotes or something. You said that if you echo the variables it says ‘array’, did you use print_r to see what was in the array?

12

I asked about that, but OP has taken the for loop out now, so all the array indices are blank.

13

Droopsnoot is right about not needing the inner loop, you can rewrite your code like this:

$getmotdetails = $mysqli->query('SELECT * FROM mothistorytable WHERE carid='.$refnumber); 
while ($row = $getmotdetails->fetch_assoc()) {
    $numberofrows = $getmotdetails->num_rows;
    $motcarid = $row["carid"];
    $motdate = $row["motdate"];
    $motodometer = $row["motodometer"];
    $customorder = $row["CustomOrder"];
    $i = $row["id"];
    echo ' <div class="addnewcar-wide"><input type="hidden" name="_ID['.$i.']" value="'.$i.'"" /><label for="_MotDate">Date</label><input name="_MotDate['.$i.']" type="text" size="10" value="'.$motdate.'" /><label for="_MotOdometer">Odometer</label><input name="_MotOdometer['.$i.']" type="text" size="20" value="'.$motodometer.'" /></div>'; 
}

Notice that I’m using the row ID as the array index for the form elements. This allows us to simplify the code in the receiving script:

$motdate     = $_POST['_MotDate'];    
$motodometer = $_POST['_MotOdometer']; 
$id = $_POST['_ID'];

foreach ($id as $rowId) {
    if ($motdate[$rowId] && $motodometer[$rowId]) {   
        $stmt = $mysqli->prepare('UPDATE mothistorytable SET motdate = ?, motodometer = ? WHERE `id` = ?');
        $stmt->bind_param("ssi", $motdate[$rowId], $motodometer[$rowId] ,$rowId);
        $stmt->execute();
    }  
}

As was suggested above, I’m using a prepared statement to avoid the risk of an SQL injection attack.

14

Ah, sorry, it’s easy to miss things when you come late to a thread :slight_smile: The example code I posted should sort the problem with the indices though.

15

Too true, especially when most of this mornings posts have been about contacting Google or Yahoo.

16

Slightly going off-topic for a minute @fretburner, I’ve been using prepared statements with PDO, am I correct that you could also arrange the code like this instead?

$motdate     = $_POST['_MotDate'];    
$motodometer = $_POST['_MotOdometer']; 
$id = $_POST['_ID'];
 
$stmt = $mysqli->prepare('UPDATE mothistorytable SET motdate = ?, motodometer = ? WHERE `id` = ?');
$stmt->bind_param("ssi", $motdate[$rowId], $motodometer[$rowId] ,$rowId);
 
foreach ($id as $rowId) {
    if ($motdate[$rowId] && $motodometer[$rowId]) {   
        $stmt->execute();
    }  
}

So we prepare the relationship between the parameters and the query, but just execute it each time when the variables we’ve bound have different values? Would there be a problem in that $rowId doesn’t exist at the point of bind_param()?

17

Thanks both. That’s finally updating. It would be great to know a way of getting it to work without using the prepared statement though (not good practice I know, but I can’t really figure how they work, so if it can be done in a procedural way as well, that would help me understand)

Anyway it seems to be working now so thanks for the help :slight_smile:

18

I think you’d have to pull out the values from the arrays first, as I don’t think you can bind to array values in that way. This is untested, but should work:

$id = $_POST['_ID'];
$stmt = $mysqli->prepare('UPDATE mothistorytable SET motdate = ?, motodometer = ? WHERE `id` = ?');
$stmt->bind_param("ssi", $motdate, $motodometer, $rowId);

foreach ($id as $rowId) {
    $motdate     = $_POST['_MotDate'][$rowId];    
    $motodometer = $_POST['_MotOdometer'][$rowId]; 
    if ($motdate && $motodomete) {   
        $stmt->execute();
    }  
}

Here’s an example of using the mysqli procedural functions for prepared statements in the PHP manual.

1 Like
19

I’ve just tried the exact same thing on a separate table (for Service details) in the same form, and for some reason it comes up with an error:

Fatal error: Call to a member function bind_param() on a non-object

Form:

echo '<p>SERVICE DETAILS</p>';
$getservicedetails = $mysqli->query('SELECT * FROM servicehistorytable WHERE carid='.$refnumber); 
while ($row = $getservicedetails->fetch_assoc()) {
	  $numberofrows = $getservicedetails->num_rows;
      $servicecarid = $row["carid"];
      $servicedate = $row["servicedate"];
      $serviceodometer = $row["odometer"];
      $serviceagent = $row["serviceagent"];
      $servicehistory = $row["servicehistory"];
      $customorder = $row["CustomOrder"];
      $i = $row["id"];
     echo '<div class="addnewcar-wide"><input type="hidden" name="_ServiceID['.$i.']" value="'.$i.'"" /><label for="_ServiceDate">Date</label><input name="_ServiceDate['.$i.']" type="text" size="10" value="'.$servicedate.'" /><label for="_ServiceOdometer">Odometer</label><input name="_ServiceOdometer['.$i.']" type="text" size="20" value="'.$serviceodometer.'" /><label for="_ServiceAgent">Service Agent</label><input name="_ServiceAgent['.$i.']" type="text" size="20" value="'.$serviceagent.'" /></div>
<div class="addnewcar-service"><label for="_ServiceHistory">Service History</label><textarea name="_ServiceHistory['.$i.']" rows="3" cols="100">'.$servicehistory.'</textarea></div>'; 
    }

The code that accepts the service details:

//SERVICE UPDATE EXISTING RECORDS FORM 
$servicedate = $_POST['_ServiceDate'];   
$serviceodometer = $_POST['_ServiceOdometer'];   
$serviceagent = $_POST['_ServiceAgent'];   
$servicedesc = $_POST['_ServiceHistory']; 
$serviceid = $_POST['_ServiceID']; 

foreach ($serviceid as $rowId) {
    if ($servicedate[$rowId] && $serviceodometer[$rowId]) {   
        $stmt = $mysqli->prepare('UPDATE servicehistorytable SET servicedate = ?, odometer = ?, serviceagent = ?, servicedesc = ? WHERE `id` = ?');
        $stmt->bind_param("ssi", $servicedate[$rowId], $serviceodometer[$rowId], $serviceagent[$rowId], $servicedesc[$rowId], $rowId);
        $stmt->execute();
    }  
}

I thought I could just apply the same principle but sadly not. The table fields have been checked and are correct. My IF statement in the above covers just the date and odometer fields as they’re the only 2 that are required. The table is very similar, just a few more fields…???

I echoed the posted variables and it looks as if the values come through:

echo "0: serviceid={$serviceid[0]}, servicedate={$servicedate[0]}, serviceodometer={$serviceodometer[0]}, serviceagent={$serviceagent[0]}, , servicedesc={$servicedesc[0]}<hr/>\n";
echo "1: serviceid={$serviceid[1]}, servicedate={$servicedate[1]}, serviceodometer={$serviceodometer[1]}, serviceagent={$serviceagent[1]}, , servicedesc={$servicedesc[1]}<hr/>\n";

result:

0: serviceid=6611, servicedate=12.04.71, serviceodometer=21,897, serviceagent=SW Renovations, , servicedesc=test jgjkgjk

If I do a var_dump on the posted variables for date and odometer I do get values, however:

array(4) { [6611]=> string(8) "12.04.71" [6612]=> string(8) "23.05.85" [6613]=> string(8) "12.04.79" [6614]=> string(8) "23.05.85" } array(4) { [6611]=> string(6) "21,897" [6612]=> string(6) "34,908" [6613]=> string(6) "26,897" [6614]=> string(6) "34,908" }

I changed the date and odometer values for record id 6611 so these seem to carry through ok- but then the error happens.

20

If the prepare() method fails for any reason, it returns FALSE, which is why you get the above error message - because $stmt is a boolean value rather than a mysqli statement object.

You can find out the reason why prepare() failed by adding the following code directly beneath it:

echo "Failed to run query: (" . $mysqli->errno . ") " . $mysqli->error;

It may be down to something like a misspelled table name.