UPSERT using InnoDB engine?

Question

Here is the solution for an UPSERT that uses primary key idfill to check for duplicates. I'm just not sure if it's sql injection proof or even efficient?

$idq="SELECT idafill FROM afillInfo, actorsInfo
WHERE (actorsInfo.id = afillInfo.id_actor) AND email = '$_SESSION[email]'" or die    (mysql_error());



$sql = "INSERT INTO afillInfo (idfill, agency, agentPhone, afillChoice, id_actor)
VALUES ( ?,?,?,?, ( select id FROM actorsInfo WHERE email = ?))
ON DUPLICATE KEY UPDATE
`id_actor` = VALUES(`id_actor`),
`agency` = VALUES(`agency`),
`agentPhone` = VALUES(`agentPhone`),
`afillChoice` = VALUES(`afillChoice`)
";


if (($stmt = $con->prepare($sql)) === false) {
trigger_error($con->error, E_USER_ERROR);
}

$result= mysqli_query($con, $idq);
$row_number = 1;
while ($row = mysqli_fetch_array($result)) {

$idfill= $row["idafill"];
}

if ($stmt->bind_param("sssss",
$idfill,
$_POST["agency"], $_POST["agentPhone"],
$_POST["afillChoice"], $_SESSION["email"]) === false) {
trigger_error($stmt->error, E_USER_ERROR);
}


if (($stmt->execute()) === false) {
trigger_error($stmt->error, E_USER_ERROR);
}

Not related to your question, but it is unsafe to interpolate $_POST variables directly into your SQL statement. See http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php — Bill Karwin, Oct 04 '13 at 01:32

Bill Karwin · Accepted Answer · 2013-10-07T23:55:33.437

INSERT adds a new row if it can.

When you use INSERT...ON DUPLICATE KEY UPDATE, it performs an UPDATE only if your INSERT would create a duplicate value in a primary key or unique key column.

Thank you for posting your table definition. I see now that you have no UNIQUE column besides idfill, the primary key.

So if you don't specify a value for idfill, it'll generate a new value in a new row. There's no way this will trigger the duplicate key. It makes no sense to run the query as you are doing and not expect it to create a new row.

You must specify an existing value in your INSERT statement for a PRIMARY or UNIQUE KEY, in order to cause the INSERT to fail and fall through to do the UPDATE. Otherwise the INSERT will succeed, by creating a new row with a distinct value for the primary key.

So you must add the idfill column to your INSERT, and specify a value that conflicts with one already existing in the database.

INSERT INTO afillInfo (idfill, agency, agentPhone, afillChoice, id_actor)
VALUES (?, ?, ?, ?, ( SELECT id FROM actorsInfo WHERE email = ?))
   ...

Apologies that I didn't notice this immediately, but another problem is that the UPDATE part of your statement isn't changing anything.

        ... UPDATE
`id_actor` = `id_actor`,
`agency` = `agency`,
`agentPhone` = `agentPhone`,
`afillChoice` = `afillChoice`

This sets the columns to exactly the same values they had before. It's a no-op. It's the equivalent of doing this in PHP:

$sql = $sql;

You can work around this by using the VALUES() function to re-use the values you tried to insert. Here's an example:

        ... UPDATE
`id_actor` = VALUES(`id_actor`),
`agency` = VALUES(`agency`),
`agentPhone` = VALUES(`agentPhone`),
`afillChoice` = VALUES(`afillChoice`)

I made some changes can you take a look at what I'm doing wrong? — user2714558, Oct 05 '13 at 06:29
Thanks. Aside from a missing parantheses I managed to get it to insert but the update is still behaving the same way as before. you mention to add idfill into the on duplicate key update. How do I add this primary key reference? — user2714558, Oct 05 '13 at 23:49
Thanks. I tried the new version and after deleting sss params to fit the latest modification, it still adds another line even with all of the fields duplicated. — user2714558, Oct 06 '13 at 23:01
Please edit your question above to include the output of `SHOW CREATE TABLE aFillInfo`. — Bill Karwin, Oct 07 '13 at 01:07
I included the SHOW CREATE TABLE afillInfo. Hope this helps. Thanks. — user2714558, Oct 07 '13 at 23:24
Thanks for the solution. It finaly works but I'm not sure if it's SQL injection proof? — user2714558, Oct 08 '13 at 21:41
In the first query, use a parameter instead of interpolating $_SESSION[email] into the string. — Bill Karwin, Oct 08 '13 at 21:58

UPSERT using InnoDB engine?

1 Answers1