0

Hi there trying to do srcipt like Vote

So Row bitwa_glosy are setted to 0

and when i press button post method should incrase value from 0 to 1. and should grown up everytime i pressed the button.

Reading the value from DB 

$Query="SELECT * FROM tentego_img WHERE id='$aid'";
$QueryResult=mysql_query($Query);
while($Kol_a=mysql_fetch_array($QueryResult)){
$a_glos =$Kol_a['bitwa_glosy']; // Values= 0

Now im know that value from bitwa_glosy are 0

and trying to incrase 

if(isset($_POST['glos_a']))

    {
        $plus = 1;
        $a_glos = $a_glos+$plus;
    $dodaj_glos_a = "UPDATE `tentego_img` SET `bitwa_glosy` = $plus WHERE `id`=$aid"; 
    $idzapytania = mysql_query($dodaj_glos_a);
    }

But it does not work and i dont know why

Theres the Post Form

<form method="post"><input type="submit" name="glos_a" value="Głosuj +"/></form>

***EDITED FINALL CODE from your sugestions and answers And still does not work, i give up with them Thanks everybody trying to help me

cheeers

Thanks everybody for attention, i give up with them, code sometimes works and sometimes not, and tried all things from answers.

theres code btw some modified with your suggestions but still does not work

Cheers!!

<!-- Begin Block -->
<?php
$ilosc= 1;
$typ= 'img';




#Lewa
$Query="SELECT * FROM tentego_img WHERE type='img' ORDER BY RAND() LIMIT ".$ilosc;
$QueryResult=mysql_query($Query);
while($Kol=mysql_fetch_array($QueryResult)){
$atytul =$Kol['title'];
$asrc =$Kol['src'];
$aid =$Kol['id'];
}


#Prawa
$QueryResult=mysql_query($Query);
while($Kol1=mysql_fetch_array($QueryResult)){
$btytul =$Kol1['title'];
$bsrc =$Kol1['src'];
$bid =$Kol1['id'];
}



echo '<table border="0"><tr>';
echo '<td><span style="color:green">#1#</span> '.$atytul.'</td><td></td><td><span style="color:red">#2#</span> '.$btytul.'</td><tr>';
echo '<td><a href="/img/'.$aid.'/'.$atytul.'/"><img src="/upload/'.$asrc.'" alt="'.$atytul.'" title="'.$atytul.'" width="370px" height="370px" /></a>Liczba głósów:0<form method="post"><input type="submit" name="glos_a" value="Głosuj +"/></form></td><td><img src="./_themes/fajna/bitwa/vs.png" /></td>';
echo '<td><a href="/img/'.$bid.'/'.$btytul.'/"><img src="/upload/'.$bsrc.'" alt="'.$btytul.'" title="'.$btytul.'" width="370px" height="370px" /></a>Liczba głosów:0<form method="post"><input type="submit" name="glos_b" value="Głosuj +"/></form></td>';
echo '</tr></table>';
$plus = 1;
if(isset($_POST['glos_a']))
{
$a_glos = $a_glos+$plus;
     $dodaj_glos_a = "UPDATE `tentego_img` SET `bitwa_glosy` = `bitwa_glosy` + $plus WHERE `id`=$aid"; 
$idzapytania = mysql_query($dodaj_glos_a);
}
if(isset($_POST['glos_b']))
{

     $dodaj_glos_b = "UPDATE `tentego_img` SET `bitwa_glosy` = `bitwa_glosy` + $plus WHERE `id`=$bid"; 
$idzapytania2 = mysql_query($dodaj_glos_b);
}

?>
   </div>   
<!-- End Block -->
Improve this question
5
  • 2
    bitwa_glosy=bitwa_glosy+1 is a more simple way to do it. Commented Jul 18, 2013 at 20:20
  • 2
    You are currently seting $plus = 1 and then you update the record and set the bitwa_glossy value to the value of $plus which is 1. Instead you should be setting it to $a_glos using your current code. Commented Jul 18, 2013 at 20:21
  • This is WAY more code than you need. It overly confuses the purpose, and is subject to (a more problematic) concurrency issue. When two (or more) are running at the same time, the counter may only get incremented by 1. Each process can run the select, and get the SAME value, and then each process updates the table. The later update overwrites the first. So the counter gets incremented by 1 rather than by 2 (or more). (If there is a big delay between the SELECT and the UPDATE, the counter could actually be set LOWER. Avoid this with an atomic operation, as demonstrated in the answers. Commented Jul 18, 2013 at 20:50
  • @Mike: Actually, no. Setting the column to a value of $a_glos is an anti-pattern, and should be avoided, if the intent is to "increment the current value by one". There is no need to retrieve the column value in a separate statement. Much better to reference the CURRENT value in the column IN THE SAME STATEMENT that updates the column.To further what @phpisuber01 said, not only is there a "more simple" way to do this, but that way is much better as well. Commented Jul 18, 2013 at 21:15
  • @spencer7593 That's why I added it as a comment and not an answer. My intent was to show the mistake that was in current code. Commented Jul 19, 2013 at 1:33

3 Answers 3

Reset to default
5

Instead of querying and then updating, why not try this?

 UPDATE `tentego_img` SET `bitwa_glosy` = `bitwa_glosy` + 1  WHERE `id`=$aid

(And, please be careful about SQL injection. What will happen to your system if your user manages to give you an $aid value of "7;DROP TABLE TENTEGO_IMG;" ?)

Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

Indeed - would have added a code segment for a prepared statement using PDO/Mysqli for security to prevent as such
1

It "doesn't work" because you are assigning the value of $plus to the column, rather than ADDING the value of $plus to the CURRENT value stored in the column.

To fix this, change the UPDATE query from this:

"UPDATE `tentego_img` SET `bitwa_glosy` = $plus WHERE `id`=$aid";

To this: 

"UPDATE `tentego_img` SET `bitwa_glosy` = `bitwa_glosy` + $plus WHERE `id`=$aid";
                                           ^^^^^^^^^^^^^^

Your question mentions a value of 0. It's not clear if you only want to increment this row if the current value is zero. You also mention that this value should be incremented with each button push, which makes it sound that you want it to be incremented, not just from zero, but from whatever the current value is.

Do NOT do THIS:

"UPDATE `tentego_img` SET `bitwa_glosy` = $a_glos WHERE `id`=$aid";
                                          ^^^^^^^

It looks as if you were calculating the $a_glos and meaning to assign that to the column. But don't do that because that would introduce a concurrency issue, a "last update wins" anti-pattern.

Consider what happens when two (or more) copies of this are running concurrently (at the same time). That counter may get incremented a lower number of times than it should, and end up with a value lower than is expected.

Consider that each copy of the process can run the SELECT, and happen to retrieve the SAME value, if the timing is right. Then, each process is going to update the table. The later update will overwrite the first. The end result is that the counter gets incremented by 1 rather than by 2, as we would expect.

And, if there happens to be some big delay between the SELECT and the UPDATE in one of the processes, the counter could actually be get set LOWER than its current value when the UPDATE runs.

To avoid this, use an atomic operation. Use a single statement which retrieves the current value, adds $plus to it, and then sets the column, all in one operation.

(This approach is demonstrated in the other good answers here.)

As a demonstration of the concurrency issue, consider this when four processes are running at the same time:

process 1    : select gets 0
process  2   : select gets 0
process   3  : select gets 0
process 1    : update set to 1
process    4 : select gets 1
process   3  : update set to 1
process    4 : update set to 2
process  2   : update set to 1

At the completion of four executions, we'd expect the value to be set to 4.

But the only way we are guaranteed to get is if there is NO OVERLAP between processes. And that is impossible to guarantee in a multi-user, multi-threaded environment, without introducing "serialized" access to a resource, and that unnecessarily increases complexity and impacts scalability.

This problem is difficult to catch in single user development testing, without adding some sort of significant delay in the code between the SELECT and the UPDATE, and running two or more processes. But even on a "low volume, low probability" of this happening, you really do want to avoid the "last update wins" anti-pattern.

So, there's no need for that section of code that retrieves the current value of the column, unless of course, you are retrieving that to display on the page, to show the current vote total.

Improve this answer

Comments

1

Change your query to

UPDATE tentego_img SET bitwa_glosy = 'bitwa_glosy'+1 WHERE id=$aid

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.