1

Been redoing the site. Upgraded to mysqli and now I'm tidying up the code and securty with prepared statements. I understand that statements should be prepared outside of foreach loops but I'm wondering about conditional statements.

[code that decides $table]
foreach ($_POST[$lastvar] as $key => $value) {
 [code not relevant to Q]
 $sql3 = "SELECT * from $table WHERE titlesid=? and peopleid=?";
 $stmt3 = $mysqli->prepare($sql3);
 $stmt3->bind_param("ii", $titlesid,$peopleid);
 $stmt3->execute();
if ($stmt3->num_rows == 0) {
  if ($table == "dhereviewers") {
  $sql = "INSERT into $table (titlesid,peopleid) VALUES (?,?)";
  } else {
  $sql = "INSERT into $table (titlesid,peopleid,billing) VALUES (?,?,?)";
  }
$billing++;
[prepare/execute one of the last two statements]
 } 
 }
 } 

So depending on the 'if' I'm going to execute one or the other of the last two inserts. Because they are conditional, do I only prepare them if they're "chosen"?

Hope I'm clear. :-)

Still learning the ropes of prepared statements.

Ian
  • 509
  • 12
  • 36

2 Answers2

2

You can determine your prepared statement conditionally like you propose. There is no problem here. The only thing is that in your case, you would need to understand which option was selected so you know how many parameters to bind.

That being said, looking at your code, you might consider doing a INSERT .. SELECT query like this:

INSERT INTO table_1 (field_1, field_2)
SELECT field_1, field_2 FROM table_2
WHERE field_x = ?

so you don't need to do a whole bunch of different queries in a loop. You should be able to do what you want with one single query.

See MySQL documentation on INSERT .. SELECT syntax here: http://dev.mysql.com/doc/refman/5.5/en/insert-select.html

Mike Brant
  • 70,514
  • 10
  • 99
  • 103
  • thanks...I'll look into that. Wondering if it would work though as option one has two fields and option 2 three. – Ian Dec 13 '13 at 21:36
  • @Ian Sure you would just make your determination based on table to decide whether you need to do the three filed insert or two field insert. This determination should likely not be any different than what you are currently doing. – Mike Brant Dec 13 '13 at 22:43
0

Because the number of parameters is different between the two statements, it would probably be more clear to prepare and bind inside the if/then/else blocks.

if ($table == "dhereviewers") {
  $sql = "INSERT into $table (titlesid,peopleid) VALUES (?,?)";
  if ($stmt = $mysqli->prepare($sql)) {
    $stmt->bind_param("ii", $titlesid, $peopleid);
  }
} else {
  $sql = "INSERT into $table (titlesid,peopleid,billing) VALUES (?,?,?)";
  if ($stmt = $mysqli->prepare($sql)) {
    $stmt->bind_param("iii", $titlesid, $peopleid, $billing);
  }
}
$stmt->execute();

@MikeBrant has a good point, that in this example, you may be able to do it more simply. But this may depend on some of the code that you have excluded for this question.

PS: num_rows always reports zero rows until you fetch all the rows. You can do this with fetch() or else use $mysqli->store_result(). See http://www.php.net/manual/en/mysqli-stmt.num-rows.php

Bill Karwin
  • 538,548
  • 86
  • 673
  • 828
  • thanks for the heads up on num_rows. Still wrapping my head 'round the new methods. – Ian Dec 13 '13 at 21:34
  • @Ian, that behavior of num_rows is exactly the same as mysql_num_rows() -- it only works with a buffered query. – Bill Karwin Dec 14 '13 at 08:49
  • Yes, sorry...my bigger problem I'm having now is "Commands out of sync". Trying to find out how to get around that. – Ian Dec 14 '13 at 20:45
  • @Ian, http://stackoverflow.com/questions/614671/commands-out-of-sync-you-cant-run-this-command-now – Bill Karwin Dec 14 '13 at 23:30
  • Thanks for that pointer. Gonna have to do some more reading. Right now I can't even seem to get values despite bound results so I'm probably doing something wrong right under my nose. – Ian Dec 15 '13 at 00:49
  • Got the results...now reading up on the "Undefined variable: mysqli" issue with an included function. Like learning to ride a bike all over again. :) – Ian Dec 15 '13 at 02:10