-2

im struggling with my code to update mysql entries using CRON. Maybe I just cant see what Im doing wrong, but maaan.. my head already hurts from overthinking this.

*This code should update table what shows up rounds for every user. I'd like it to add one round per 10mins for every user.

this is code, that is executed every 10mins:

$conn = mysql_connect($dbhost, $dbuser, $dbpass);
mysql_select_db('universe_domination');
$sql = mysql_query("SELECT * FROM rounds");
foreach (mysql_fetch_array($sql) as $row) {
    $id = $row['id'];
    $free = $row['free'];
    $saved = $row['saved'];
    $lost = $row['lost'];
    if ($free < 200) {
        $roundSql = "UPDATE rounds SET free = free + 1 WHERE id = " . $id;
    } elseif ($free == 200 && $saved < 300) {
        $roundSql = "UPDATE rounds SET saved = saved + 1 WHERE id = " . $id;
    } elseif ($free == 200 && $saved == 300) {
        $roundSql = "UPDATE rounds SET lost = lost + 1 WHERE id = " . $id;
    }
    mysql_query($roundSql);
}

problem is, that now i have 4 entries.

id  free  saved    lost
1    2      300   19568
2    200    250   19568
3    6      300   19568
4    7      300   19568

but after update it looks like this:

id  free  saved    lost
1    6      300   19568
2    200    250   19568
3    6      300   19568
4    9      300   19568

Can somebody tell me whats wrong in that code? I believe that it is some beginners mistake, but i cannot find whats wrong

**So far it edits it somehow wrong... one row adds 4 rounds, third none and fourth adds 2 rounds

****Still no luck after using this:

$conn = new PDO("mysql:host=$dbhost;dbname=$dbname", $dbuser, $dbpass);
    $sql = "UPDATE rounds SET free = free + (CASE WHEN free < 200 THEN 1 ELSE 0 END),
            SET saved = saved + (CASE WHEN saved < 300 THEN 1 ELSE 0 END),
            SET lost = lost + (CASE WHEN free = 200 AND saved = 300 THEN 1 ELSE 0 END)";
    $updateRound = $conn->prepare($sql);
    $updateRound->execute();

--table is not updated at all

SLUNlCKO
  • 21
  • 4
  • 1
    Why do you even need to do this in PHP? You can do this query entirely in MySQL, with one statement. – cdhowie Mar 06 '17 at 15:32
  • 2
    Please go read [ask] first of all. You completely neglected to tell us what your code is supposed to do, and for what reason the result you are getting is wrong. – CBroe Mar 06 '17 at 15:32
  • i cant use MySQL as my hosting provider doesn't allow me to use triggers – SLUNlCKO Mar 06 '17 at 15:35
  • 2
    @SLUNlCKO My point is that you can do this all in one single query, regardless of how you invoke the query. The way you are doing it right now (selecting all rows, analyzing the data with PHP, then running a bunch of queries to update the data) is fragile and will break if there are concurrent updates to the table. You can simply execute a single SQL statement (through PHP) and be done with it. – cdhowie Mar 06 '17 at 15:37
  • 2
    What @cdhowie is saying is something like: `UPDATE rounds SET free = case when free < 200 then free + 1 else free end, saved = case when ..... where id = ....` – Jorge Campos Mar 06 '17 at 15:40
  • @cdhowie problem is, that i want to let user use only 200 rounds at once.. when he doesnt use them store them for later usage or if all those containers (free = 200, saved = 200) are full, he loses ability to use new round at all – SLUNlCKO Mar 06 '17 at 15:40
  • @SLUNlCKO I don't see how that has anything to do with what I'm trying to tell you. – cdhowie Mar 06 '17 at 15:41
  • @cdhowie omfg im just an Idiot. Thank you for your help, I'll try this. – SLUNlCKO Mar 06 '17 at 15:44

2 Answers2

0

$roundSql is being run even if all if/elseif fails. The result is that depending on the data you may have one query - incrementing free for a particular id - run multiple times. There are a few ways to solve that, including:

1 - Have an else at the end of the if/elseif to make a query - even if it is a null MySQL statement - for when all other options fail.

2 - Initialize $roundSql before the if statement. Before the query, check to see if it is blank and only perform the query if it is not blank.

As a debugging tool, add some print statements to see what queries are being run.

0

Consider doing this all in a single query, which will be more reliable (particularly with concurrent table access), easier to reason about, and execute faster:

$conn = mysql_connect($dbhost, $dbuser, $dbpass);
mysql_select_db('universe_domination');

mysql_query(<<<SQL
UPDATE rounds
SET free = free + (CASE WHEN free < 200 THEN 1 ELSE 0 END),
    saved = saved + (CASE WHEN free = 200 AND saved < 300 THEN 1 ELSE 0 END),
    lost = lost + (CASE WHEN free = 200 AND saved = 300 THEN 1 ELSE 0 END);
SQL
);

As a side note, the mysql_* functions are deprecated in PHP 5.5 and they have some serious SQL injection vulnerability issues. Please consider using PDO and prepared statements instead.

cdhowie
  • 158,093
  • 24
  • 286
  • 300
  • using your solution with mysqli or PDO doesnt work.. table is not updated at all – SLUNlCKO Mar 06 '17 at 17:24
  • *SOLVED* for first ive noticed my hosting provider uses different port for MySQL so it wasn't working without defining port (obviously). And your solution is wrong as it uses SET for every column – SLUNlCKO Mar 06 '17 at 19:23
  • My bad, you are correct; I've removed the extra `SET`s and it should work now. – cdhowie Mar 06 '17 at 23:00