...

View Full Version : ON DUPLICATE KEY UPDATE Conditions



bemore
03-05-2013, 01:16 AM
I've this


INSERT INTO opl_comp(gamename, gamecode, region, mode, vmc, smb, hdd, usb, notes, comp)VALUES('$gamename', '$cleangc', '$region', '$mode', '$vmc', '$smb', '$hdd', '$usb', '$notes', '$comp')
ON DUPLICATE KEY UPDATE gamename=('$gamename'), region=('$region'), mode=('$mode'), vmc=('$vmc'), smb=('$smb'), hdd=('$hdd'), usb=('$usb'), notes=('$notes'), comp=('$comp')";

Which is updating the existing row where the supplied gamecode value (foreign key) matches, though it also allows the row to be cleared and overwritten by submitting the same gamecode value again.

I'm trying to achieve the same thing as above, except only when (gamename)="".
I've tried a few things such as CASE WHEN & THEN VALUES, but must be doing it wrong.
Goal is to allow the existing row where the FK (gamecode) matches to be updated but only if the gamename column in that row is blank.

Shed of light appreciated :)

Arcticwarrio
03-05-2013, 07:08 AM
thats only looking to update a record, if it were blank then the row would be wmpty and you would need to INSERT it

or maybe try another blank key:



INSERT INTO opl_comp(gamename, gamecode, region, mode, vmc, smb, hdd, usb, notes, comp)VALUES('$gamename', '$cleangc', '$region', '$mode', '$vmc', '$smb', '$hdd', '$usb', '$notes', '$comp') WHERE gamename= ''
ON DUPLICATE KEY UPDATE gamename=('$gamename'), region=('$region'), mode=('$mode'), vmc=('$vmc'), smb=('$smb'), hdd=('$hdd'), usb=('$usb'), notes=('$notes'), comp=('$comp') ";

durangod
03-05-2013, 07:59 AM
Shouldnt this be in the MYSQL section?

Fou-Lu
03-05-2013, 02:20 PM
Yes it should be. Moving to mysql.
I'm not sure if you can use a case within an on update clause, but you can use an if:


INSERT INTO opl_comp(gamename, gamecode, region, mode, vmc, smb, hdd, usb, notes, comp) VALUES ('$gamename', '$cleangc', '$region', '$mode', '$vmc', '$smb', '$hdd', '$usb', '$notes', '$comp')
ON DUPLICATE KEY UPDATE
gamename = IF(gamename = '', $gamename, gamename),
region = IF(gamename = '', $region, region),
...

Old pedant may have some better ideas for this as well.

bemore
03-05-2013, 07:16 PM
Thanks for the suggestions, I will try these tips out when I am home this evening.

Old Pedant
03-05-2013, 09:06 PM
Personally, for something like this I would opt to make a SELECT query first, to check the existing conditions, and then do either an INSERT or UPDATE (or nothing at all!) as appropriate. Surely this doesn't happen often enough that doing two queries instead of one will make any real difference on the site's performance?

But I don't see why Fou-Lu's trick won't work.

bemore
03-06-2013, 12:42 AM
So, I tried the following



$sql="INSERT INTO $tbl_name(gamename, gamecode, region, mode, vmc, smb, hdd, usb, notes, comp)VALUES('$gamename', '$cleangc', '$region', '$mode', '$vmc', '$smb', '$hdd', '$usb', '$notes', '$comp')
ON DUPLICATE KEY UPDATE
gamename = IF(gamename = '', $gamename, gamename),
region = IF (gamename = '', $region, region),
mode = IF(gamename = '', $mode, mode),
vmc = IF(gamename = '', $vmc, vmc),
smb = IF(gamename = '', $smb, smb),
hdd = IF(gamename = '', $hdd, hdd),
usb = IF(gamename = '', $usb, usb),
notes = IF(gamename = '', $notes, notes),
comp = IF(gamename = '', $comp, comp)";


And it gives me


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 'Hack - Part 1 - Infection, gamename), region = IF (gamename = '', U, region), mo' at line 3

Same deal with Arcticwarrio's suggestion. I tried changing a few things, but unable to overcome the syntax error.

@Old_Pendant you're correct in that it may not happen very often, though I'd just prefer previous entries to be safe, as I cannot control whether or not a visitor checks to see if a game has been added before adding it again and overwriting the previous entry with possibly less compatibility information.

How can I incorporate conditions using a SELECT query before an INSERT and ON DUPLICATE KEY UPDATE? I've yet to try anything like this. My experience with SQL is limited.

Old Pedant
03-06-2013, 02:06 AM
Missing the apostrophes around the PHP variables. Why did you think you could omit them in the UPDATE section if you needed them in the INSERT section???

But I'd do this:


// ensure no sql injection and also put the apostrophes around each value
// so that you only have to do this once!
function clean( $val )
{
return "'" . mysql_real_escape_string( $val ) . "'"
}
// do it for each value:
$gamename = clean($gamename);
$cleangc = clean($cleangc);
$region = clean($region);
$mode = clean($mode);
$vmc = clean($vmc);
$smb = clean($smb);
$hdd = clean($hdd);
$usb = clean($usb);
$notes = clean($notes);
$comp = clean($comp);

$sql = "INSERT INTO $tbl_name(gamename, gamecode, region, mode, vmc, smb, hdd, usb, notes, comp)
VALUES($gamename,$cleangc,$region,$mode,$vmc,$smb,$hdd,$usb,$notes,$comp) "
ON DUPLICATE KEY UPDATE
gamename = IF(gamename = '', $gamename, gamename),
region = IF (gamename = '', $region, region),
mode = IF(gamename = '', $mode, mode),
vmc = IF(gamename = '', $vmc, vmc),
smb = IF(gamename = '', $smb, smb),
hdd = IF(gamename = '', $hdd, hdd),
usb = IF(gamename = '', $usb, usb),
notes = IF(gamename = '', $notes, notes),
comp = IF(gamename = '', $comp, comp)";

bemore
03-06-2013, 04:58 AM
That was indeed the problem.



$sql = "INSERT INTO $tbl_name(gamename, gamecode, region, mode, vmc, smb, hdd, usb, notes, comp)VALUES('$gamename', '$cleangc', '$region', '$mode', '$vmc', '$smb', '$hdd', '$usb', '$notes', '$comp')
ON DUPLICATE KEY UPDATE
gamename = IF(gamename = '', '$gamename', gamename),
region = IF (region = 'O', '$region', region),
mode = IF(mode = '', '$mode', mode),
vmc = IF(vmc = '', '$vmc', vmc),
smb = IF(smb = '', '$smb', smb),
hdd = IF(hdd = '', '$hdd', hdd),
usb = IF(usb = '', '$usb', usb),
notes = IF(notes = '', '$notes', notes),
comp = IF(comp = 'untested', '$comp', comp)";


Seems to be working as needed this way :thumbsup:
Very helpful.. silly to not include the apostrophes, my mistake. But at least it all worked out and I learned something new!
Using gamename column for every IF did allow it to UPDATE, but only the game name column would be updated. Rest of the submission form inputs are ignored, and the rest of the columns would be left blank.

Old Pedant
03-06-2013, 06:04 AM
Still say a SELECT followed by either INSERT, UPDATE, or nothing would be better.

If you did that all in a Stored Procedure, it would still only involve one call from PHP.

And why did you reject my idea about adding the mysql_real_escape_string??

bemore
03-06-2013, 08:36 AM
Still say a SELECT followed by either INSERT, UPDATE, or nothing would be better.

If you did that all in a Stored Procedure, it would still only involve one call from PHP.

And why did you reject my idea about adding the mysql_real_escape_string??

I'm not sure how to go about using SELECT to get the same results as IF gets me. The idea is to always UPDATE the existing row, but only if there is nothing in the row's gamename column.

I'm already using


if(isset($_POST['gamename'])) {
$gamename = mysqli_real_escape_string($link, $_POST['gamename']);
} else {
$gamename = "None";
}


I see that your method is cleaner and a lot less code, as I use the above for each input name, but


function clean( $val )
{
return "'" . mysql_real_escape_string( $val ) . "'"
}


gives me a syntax error on }

Old Pedant
03-06-2013, 03:56 PM
The semicolon on the end of the line is missing.


return "'" . mysql_real_escape_string( $val ) . "'";

I don't use PHP so I tend to make typos like that.

Fou-Lu
03-06-2013, 04:19 PM
Whoa whoa, you're mixing mysql and mysqli libraries here.
Since you are using mysqli, you can just bind the variables and be done with it. No need to escape them at all. Old Pedant's function is good and clean for mysql library, but it won't be as clean with the mysqli since the mysqli resource is (rightfully at that) not global, so it would need either globalization or passing to the function (preferable). So the end result is simply a function call that will be near identical to the use of the original code. Instead of
$gamename = mysqli_real_escape_string($link, $_POST['gamename']);, you'd have $gamename = clean($link, $_POST['gamename']);, which if the only purpose is to execute mysqli_real_escape_string, I'd beg question on the overall usefulness of breaking it down into a separate function.

Statement that up instead. I've never done a statement with an on duplicate update, but I can't see why it would be any different. I'll just pull the VALUES instead:


<?php
// pull your connection in here
// Gather ye' variables with a simple pull. Walk the $_POST through a stripslashes if get_magic_quotes_gpc() is enabled:
if (isset($_POST['gamename'], $_POST['gamecode'], /*...etc */))
{
$gamename = $_POST['gamename'];
$gamecode = $_POST['gamecode'];
// etc
$tbl_name = 'yourtable';
$sql="INSERT INTO $tbl_name (gamename, gamecode, region, mode, vmc, smb, hdd, usb, notes, comp)
VALUES
(?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
ON DUPLICATE KEY UPDATE
gamename = IF(gamename = '', VALUES(gamename), gamename),
region = IF (gamename = '', VALUES(region), region),
mode = IF(gamename = '', VALUES(mode), mode),
vmc = IF(gamename = '', VALUES(vmc), vmc),
smb = IF(gamename = '', VALUES(smb), smb),
hdd = IF(gamename = '', VALUES(hdd), hdd),
usb = IF(gamename = '', VALUES(usb), usb),
notes = IF(gamename = '', VALUES(notes), notes),
comp = IF(gamename = '', VALUES(comp), comp)";
if ($stmt = mysqli_stmt_prepare($link, $sql))
{
mysqli_stmt_bind_param($stmt, 'ssssssssss', $gamename, $gamecode, $region, $mode, $vmc, $smb, $hdd, $usb, $notes, $comp);
mysqli_stmt_execute($stmt);
mysqli_stmt_close($stmt);
}
else
{
printf("Failed to create a statement: %s" . PHP_EOL, mysqli_error($link));
}
}

Like that. Untested, and I wrote it in procedural to match what you have, so I can't be 100% sure that I got it right :P

I noticed as well that you have 2x different queries. I wrote this one with the assumption that it can go ahead and update so long as the gamename is empty, but you have a different one here that checks for each field as empty. Use whichever is correct.

bemore
03-06-2013, 08:43 PM
Whoa whoa, you're mixing mysql and mysqli libraries here.
Since you are using mysqli, you can just bind the variables and be done with it. No need to escape them at all. Old Pedant's function is good and clean for mysql library, but it won't be as clean with the mysqli since the mysqli resource is (rightfully at that) not global, so it would need either globalization or passing to the function (preferable). So the end result is simply a function call that will be near identical to the use of the original code. Instead of
$gamename = mysqli_real_escape_string($link, $_POST['gamename']);, you'd have $gamename = clean($link, $_POST['gamename']);, which if the only purpose is to execute mysqli_real_escape_string, I'd beg question on the overall usefulness of breaking it down into a separate function.

Statement that up instead. I've never done a statement with an on duplicate update, but I can't see why it would be any different. I'll just pull the VALUES instead:


<?php
// pull your connection in here
// Gather ye' variables with a simple pull. Walk the $_POST through a stripslashes if get_magic_quotes_gpc() is enabled:
if (isset($_POST['gamename'], $_POST['gamecode'], /*...etc */))
{
$gamename = $_POST['gamename'];
$gamecode = $_POST['gamecode'];
// etc
$tbl_name = 'yourtable';
$sql="INSERT INTO $tbl_name (gamename, gamecode, region, mode, vmc, smb, hdd, usb, notes, comp)
VALUES
(?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
ON DUPLICATE KEY UPDATE
gamename = IF(gamename = '', VALUES(gamename), gamename),
region = IF (gamename = '', VALUES(region), region),
mode = IF(gamename = '', VALUES(mode), mode),
vmc = IF(gamename = '', VALUES(vmc), vmc),
smb = IF(gamename = '', VALUES(smb), smb),
hdd = IF(gamename = '', VALUES(hdd), hdd),
usb = IF(gamename = '', VALUES(usb), usb),
notes = IF(gamename = '', VALUES(notes), notes),
comp = IF(gamename = '', VALUES(comp), comp)";
if ($stmt = mysqli_stmt_prepare($link, $sql))
{
mysqli_stmt_bind_param($stmt, 'ssssssssss', $gamename, $gamecode, $region, $mode, $vmc, $smb, $hdd, $usb, $notes, $comp);
mysqli_stmt_execute($stmt);
mysqli_stmt_close($stmt);
}
else
{
printf("Failed to create a statement: %s" . PHP_EOL, mysqli_error($link));
}
}

Like that. Untested, and I wrote it in procedural to match what you have, so I can't be 100% sure that I got it right :P

I noticed as well that you have 2x different queries. I wrote this one with the assumption that it can go ahead and update so long as the gamename is empty, but you have a different one here that checks for each field as empty. Use whichever is correct.

This is excellent.. as I have been wanting to get into prepared statements. As I understand it, the use of mysqli_real_escape_string is not needed with this method?

Though, currently each input requires an }else{ and also I use regex to change some of the inputs. To me, it seems easier to handle each one independently and just use the mysqli_real_escape_string.

Here is an example of how I use some of the input fields.


if(isset($_POST['region'])) {
$region = mysqli_real_escape_string($link, $_POST['region']);
} else {
$region = "O";
}
if(isset($_POST['mode'])) {
$mode = implode(",", $_POST['mode']);
} else {
$mode = " ";
}
if(isset($_POST['vmc'])) {
$vmc = mysqli_real_escape_string($link, $_POST['vmc']);
} else {
$vmc = "-";
}
if(isset($_POST['comp'])) {
$comp = mysqli_real_escape_string($link, $_POST['comp']);
} else {
$comp = "untested";
}


I run gamecode input thru regex to guarantee a specific character format, and I use the output variable for the INSERT VALUES ('$cleangc'). Can I simply apply this variable to your method? Example..



mysqli_stmt_bind_param($stmt, 'ssssssssss', $gamename, $cleangc, $region, $mode, $vmc, $smb, $hdd, $usb, $notes, $comp);
mysqli_stmt_execute($stmt);
mysqli_stmt_close($stmt);



Or would I also need to include it as

gamecode = IF(gamecode = '$cleangc', VALUES(cleangc), gamecode)

And a question, what is 'ssssssssss' doing?

Old Pedant
03-06-2013, 08:55 PM
I have to say, this strikes me as indicative of bad DB design:


$tbl_name = 'yourtable';
$sql="INSERT INTO $tbl_name (gamename, gamecode, region, mode, vmc, smb, hdd, usb, notes, comp)
VALUES ...

Why does the name of the table need to be a variable? Surely you don't have more than one table with the same fields??? Or did we discuss this before?

bemore
03-06-2013, 09:54 PM
I have to say, this strikes me as indicative of bad DB design:


$tbl_name = 'yourtable';
$sql="INSERT INTO $tbl_name (gamename, gamecode, region, mode, vmc, smb, hdd, usb, notes, comp)
VALUES ...

Why does the name of the table need to be a variable? Surely you don't have more than one table with the same fields??? Or did we discuss this before?

We've discussed this before :)
Though, I do have multiple tables now. I only ever UPDATE or INSERT into the one table, though, and I still have it set as a variable simply because that's what I did in my very first PHP/MySQL script so I just continue to do it that way. It could easily be INSERT INTO opl_comp, but why should it be? Honestly, I don't see any reason to assume bad DB design based off of calling a table with a variable, or not. I see no difference in the method or result no matter how it is done, and it'd all be personal opinion in the end.

Fou-Lu
03-06-2013, 11:45 PM
I'm actually with you on this one; the $tbl_name doesn't necessarily indicate a structural fault. Especially if you want to write a system where a prefix may be added, you may choose to do something of the sorts (or assemble with {$prefix}actual_table_name for example). What IS a problem though is this: $mode = implode(",", $_POST['mode']); which is definitely a structural problem as you are putting a collection into a single field.
'sssssssss' represents the datatype of each input variable. These are all strings. Other options are 'i' for integers, 'd' for doubles, and 'b' for binary.
You can't use each of these directly. The problem is that mysqli_real_escape_string isn't sensitive to magic_quotes_gpc, and that mysqli_stmt is not sensitive to mysqli_real_escape_string. So lets take my data as \\mymachine\Fou-Lu's share\.
If magic_quotes_gpc is enabled, that automatically becomes \\\\mymachine\\Fou-Lu\'s share\\. With mysqli_real_escape_string, that now becomes \\\\\\\\mymachine\\\\Fou-Lu\\\'s share\\\\. Insert this using mysqli_query and the stored result is \\\\mymachine\\Fou-Lu\'s share\\. If you insert it using prepared statements, you get \\\\\\\\mymachine\\\\Fou-Lu\\\'s share\\\\. So all of these are corrupting your data.
With a regular query you need to escape it. But only once. With a prepared statement you do not escape it as it becomes a part of the input. The data and structure are different pieces of the puzzle, so you cannot corrupt the structure with providing it data such as I want.
So this is why you must:
1. Check for magic_quotes_gpc. If enabled, issue stripslashes to input (\\\\mymachine\\Fou-Lu\'s share\\ now becomes \\mymachine\Fou-Lu's share\);
2. If you are using prepared statement, no other steps are necessary (data is still: \\mymachine\Fou-Lu's share\).
3. If you are not using prepared statements, issue mysqli_real_escape_string to escape it (ie: data is now: \\\\mymachine\\Fou-Lu\'s share\\)

Does that make more sense?

Old Pedant
03-07-2013, 12:29 AM
the $tbl_name doesn't necessarily indicate a structural fault. Especially if you want to write a system where a prefix may be added, you may choose to do something of the sorts (or assemble with {$prefix}actual_table_name for example).

But that should only apply if you are creating something meant to be installed on many different machines. For a "one off" there's no reason to have an adjustable prefix or any other reason to have multiple table names.

bemore
03-07-2013, 01:07 AM
But that should only apply if you are creating something meant to be installed on many different machines. For a "one off" there's no reason to have an adjustable prefix or any other reason to have multiple table names.

For the persistence surrounding it, I have changed every instance of $tbl_name to opl_comp and removed the $tbl_name = opl_comp variable :)


I'm actually with you on this one; the $tbl_name doesn't necessarily indicate a structural fault. Especially if you want to write a system where a prefix may be added, you may choose to do something of the sorts (or assemble with {$prefix}actual_table_name for example). What IS a problem though is this: $mode = implode(",", $_POST['mode']); which is definitely a structural problem as you are putting a collection into a single field.
'sssssssss' represents the datatype of each input variable. These are all strings. Other options are 'i' for integers, 'd' for doubles, and 'b' for binary.
You can't use each of these directly. The problem is that mysqli_real_escape_string isn't sensitive to magic_quotes_gpc, and that mysqli_stmt is not sensitive to mysqli_real_escape_string. So lets take my data as \\mymachine\Fou-Lu's share\.
If magic_quotes_gpc is enabled, that automatically becomes \\\\mymachine\\Fou-Lu\'s share\\. With mysqli_real_escape_string, that now becomes \\\\\\\\mymachine\\\\Fou-Lu\\\'s share\\\\. Insert this using mysqli_query and the stored result is \\\\mymachine\\Fou-Lu\'s share\\. If you insert it using prepared statements, you get \\\\\\\\mymachine\\\\Fou-Lu\\\'s share\\\\. So all of these are corrupting your data.
With a regular query you need to escape it. But only once. With a prepared statement you do not escape it as it becomes a part of the input. The data and structure are different pieces of the puzzle, so you cannot corrupt the structure with providing it data such as I want.
So this is why you must:
1. Check for magic_quotes_gpc. If enabled, issue stripslashes to input (\\\\mymachine\\Fou-Lu\'s share\\ now becomes \\mymachine\Fou-Lu's share\);
2. If you are using prepared statement, no other steps are necessary (data is still: \\mymachine\Fou-Lu's share\).
3. If you are not using prepared statements, issue mysqli_real_escape_string to escape it (ie: data is now: \\\\mymachine\\Fou-Lu\'s share\\)

Does that make more sense?

Yes it is making sense thank you for the detailed explanation. I'm now working on adjusting over to using prepared statements.

Old Pedant
03-07-2013, 01:24 AM
I thought we had discussed the table name stuff before, but wasn't sure.

I have no real object to using a variable for the table name under the circumstances.

It's just that about 90% of the time when I see that usage it means the programmer has set up multiple tables with the same structure in the mistaken belief the MySQL can 't handle more than a few dozen records per table or some other such nonsense.



EZ Archive Ads Plugin for vBulletin Copyright 2006 Computer Help Forum