0

I'm having a really strange issue with what I believe is a simple query. This was code I used to learn PHP a few years ago and was trying to repurpose it for a really simple time tracking application. When time permits, I'll work on making a new version with mysqli, but in the meantime I just want to get it working. I'm still learning, so it's not easy for me.

When a form posts, I'm basically gathering the data and trying to 1) update an existing value in one table based on submitted information and 2) create a new row in another table to store all entered values from the previous form.

Here's how I had written it:

<?php

    $uid = $_COOKIE["uid"];
    $cid = $_POST['customer_id'];
    $pid = $_POST['project_id'];
    $hoursAdded = $_POST['hoursAdded'];


    mysql_query("UPDATE projects SET hours=hours+'$hoursAdded' 
            WHERE id='$pid'") or die(mysql_error());

    mysql_query("INSERT INTO hours (id,uid,pid,cid,userhours) 
            VALUES ('','$uid','$pid','$cid','$hoursAdded')") 
            or die(mysql_error());        
 ?>

Everything works fine, but occasionally the SECOND mysql_query statement doesn't actually INSERT anything into the hours table. The First query always writes, but the second sometimes just doesn't do anything - no errors - nothing.

Essentially, the sum of the hours submitted in the HOURS table should equal the TOTAL hours updated in the projects table. Because the second query sometimes doesn't write, I end up with a project that has, for example, 200 TOTAL hours, and the sum of all hours for that project in the HOURS table totalling much less than 200.

Any ideas what's going on? Why would it sometimes write and other times not?

Thanks and sorry if the problem is obvious. This is still new to me.

Stangn99
  • 97
  • 1
  • 9
  • Your code is vulnerable to SQL injections. You should read on [how to prevent them in PHP](http://stackoverflow.com/q/60174/53114). – Gumbo Mar 26 '14 at 14:28
  • More importantly, you can cease using the deprecated `mysql_` syntax that is wide open to far more attacks than just SQL injenctions. Instead, please use the go-forward `mysqli_` syntax. – PlantTheIdea Mar 26 '14 at 14:30
  • no code between two queries? – mesutozer Mar 26 '14 at 14:31
  • 1
    Why do you store redundant information in your db scheme? If the number of hours of the project can always be calculated from the `hours` table, there's no need to store it in `projects`. – TheWolf Mar 26 '14 at 14:31
  • also if id is autoincrement field, just do not insert blank value into it, remove id from second query – mr. Pavlikov Mar 26 '14 at 14:35
  • What are the datatypes in your mysql table?. Post them as well – Tabby Mar 26 '14 at 14:37
  • How do you calculate sum of hours and compare with TOTAL? I guess the problem resides there.. – mesutozer Mar 26 '14 at 14:37
  • Is summing the hours the only way you check to see if the INSERT is working? If so, try checking after each execution so you can see what data is not getting inserted. – nick Mar 26 '14 at 14:37
  • Thanks all for the quick replies. I know this is pretty bad practise - as mentioned is old, and is only used on our local intranet for 6 individuals. It was a quick and dirty alternative to storing hours in Excel at the time. TheWold=f: The idea was to store the hours in another table to maintain a record of the hours inputting by each user. If I just added the hours to the projects table, there would be no record of who added what hours (hope that makes sense). – Stangn99 Mar 26 '14 at 14:44
  • Nick: how can I check after each execution? 90% of the time the insert command works fine after the UPDATE, but the other 10%, no information in INSERTED but the update query works fine. – Stangn99 Mar 26 '14 at 14:47
  • @Stangn99 for me to solve the error i need the datatypes of these values. Without it my efforts will be useless and waste of time. Should i assume all of them are of type `int` and in table `hours` ; `id` is set to autoincrement? – Tabby Mar 26 '14 at 14:48
  • @Tabby my apologies. ID is on auto_increment. All datatypes are of type int with the exception of userhours which is of type float – Stangn99 Mar 26 '14 at 14:57
  • Stangn99, yeah, I see that. But why not just drop `projects.hours` and calculate the total hours from the `hours` table? – TheWolf Mar 26 '14 at 15:00
  • @TheWolf good point. I guess 3 years ago I didn't think of that. But would that have anything to do with the second query (`INSERT`) not running at random times? – Stangn99 Mar 26 '14 at 15:05
  • Probably not, but it would make the first `INSERT` necessary... – TheWolf Mar 26 '14 at 15:07
  • @MarcusAdams no error. I learned this from LYNDA.COM php course. if autoincrement is on, the '' is just ignored i guess. – Stangn99 Mar 26 '14 at 15:09

1 Answers1

0

You have syntax errors. Try this out

<?php

    $uid = $_COOKIE["uid"];
    $cid = $_POST['customer_id'];
    $pid = $_POST['project_id'];
    $hoursAdded = $_POST['hoursAdded'];


    mysql_query("UPDATE projects SET hours=hours + $hoursAdded 
            WHERE id=$pid") or die(mysql_error());

    mysql_query("INSERT INTO hours (uid,pid,cid,userhours) 
            VALUES ($uid,$pid,$cid,$hoursAdded)") 
            or die(mysql_error());        
 ?>

I have removed single quotes from all the variables inside the mysql_query() . Adding single quotes to a variable makes the of type string. I would also suggest check if the php variables you have defined are empty or not before you insert them in the query. I have also remove id from hours table as its set to autoincrement therefore no use of it in the query. It will automatically increment when a new row is added

Tabby
  • 389
  • 1
  • 11
  • thank you very much. I'll make the changes and try running it again and see if a discrepancy shows up between the sum of hours in HOURS table and total hours in the projects table. I'm still wondering why it works MOST of the time though. Shouldn't it either work or not work? It's almost like it's skipping the insert query once in a while. – Stangn99 Mar 26 '14 at 15:12
  • I don't think that is possible. Did you cross check the values you inserted ? . My guess is that they were garbage values. Which php version were you using when this code ran as it should? – Tabby Mar 26 '14 at 15:19
  • Yes, I went into phpMyAdmin and made sure the values in the HOURS table were inserted...and they were. I also checked and the PROJECTS table had added the hours the users inputted. The times were the INSERT query didn't run, there was nothing stored in the database (as in no garbage data). The times that it did run the query, all correct values were stored. I actually thought it was a bug with mysql of the the PHP version I was using. So I updated to the latest MAMP (mysql 5.5.33, php 5.3). Really strange. – Stangn99 Mar 26 '14 at 15:25
  • @Stangn99 thats exactly why i'm asking you the php version which you used to use ;-) – Tabby Mar 26 '14 at 15:26
  • I've updated the code with the changes you provided and it seems to be running fine - although because the problem is intermittent, I won't know if it works or not until I find the totals in the two tables don't match up. I'll keep testing today and tomorrow and will post back the results. on an aside, is there a simple way to TEST if the query is successful in actually inserting a new row? – Stangn99 Mar 26 '14 at 15:31
  • Yes use a for loop and assign incremented values to all the variables you are using . If you dont want incremented valued do a basic math on these varables like adding , multiply, subtract but keep it simple. Simple enough for manual calculation. Then insert these variables into the database. This way you can insert 100s of values. I would stress on keeping the values incremental. – Tabby Mar 26 '14 at 15:38
  • Still no issues so far with your solution, but will continue testing. Thank you for the suggestion on using for loop and assigning incremental values - i'll give this a shot. Simple is key for now, as I eventually plan on writing a new/better way to achieve the same requirements. – Stangn99 Mar 26 '14 at 18:41
  • @Stangn99 how is the testing going ;-) – Tabby Mar 28 '14 at 06:07