0

I am a few issues with my sql query that used to get the total of pop of a player combining all the towns they have but for some reason it is giving the wrong total pop I am also told to use bound parameters but i am unsure what they are and how to use them. any help would be great thank you.

<!DOCTYPE html>
<html>
<head>
<style>
table {
    width: 100%;
    border-collapse: collapse;
}

table, td, th {
    border: 1px solid black;
    padding: 5px;
}

th {text-align: left;}
</style>
</head>
<body>

<?php
$q = intval($_GET['q']);
$latitude = -838;
$longitude = -493;
$con = mysqli_connect('localhost','User','Password','Database');
if (!$con) {
    die('Could not connect: ' . mysqli_error($con));
}

mysqli_select_db($con,"Database");
$sql="SELECT towns.location_x,  towns.location_y,  players.*, 
            town_deltas.*, town_deltas.town_id, 
            sum( town_deltas.population ) as total_population  
      FROM `town_deltas` 
        LEFT JOIN players ON town_deltas.owner_id = players.player_id 
        LEFT JOIN towns ON town_deltas.town_id = towns.town_id 
      WHERE town_deltas.owner_id = '".$q."' 
      GROUP BY owner_id, data_timestamp 
      ORDER BY `town_deltas`.`town_id` ASC, 
               `town_deltas`.`data_timestamp` ASC";



$result = mysqli_query($con,$sql); 
echo "<table>
<tr>

<th>data_timestamp</th>

<th>name</th>


<td>population</td>
<td>location_x</td>
<td>location_y</td>
<td>distance</td>
</tr>";
while($row = mysqli_fetch_array($result)) {
    echo "<tr>";


    echo "<td>" . $row['data_timestamp'] . "</td>";

    echo "<td>" . $row['Player_name'] . "</td>";


    echo "<td>" . $row['total_population'] . "</td>";

    echo "<td>" . $row['location_x'] . "</td>";
    echo "<td>" . $row['location_y'] . "</td>";
    echo "<td>" . $row['distance'] . "</td>";

    echo "</tr>";
}

echo "</table>";
mysqli_close($con);
?>
</body>
</html>

broken total pop page

data_timestamp      name         population
2016-09-25 00:03:29 Kodabear    50315100

WHat is should show

data_timestamp       name    population
2016-09-25 00:03:29 Kodabear  335,434

Player table

player_id   race_id alliance_id alliance_role_id    Player_name 
237186        1     78           381                 Kodabear

town_deltas table

5198444 2016-09-21 00:03:47 282609  237186  01 Smokey The Bear  29634   0 0
5198445 2016-09-21 00:03:47 289820  237186  02 Littlebears  28557   0   0   
5198446 2016-09-21 00:03:47 292694  237186  03 Panda-bear   28068   0   0
5198447 2016-09-21 00:03:47 296040  237186  04 Humphrey 29420   1   1
5198448 2016-09-21 00:03:47 296593  237186  05 Winnie-the-Pooh  29527   0   0
5198449 2016-09-21 00:03:47 296914  237186  06 Shayna The Bear  29287   0   0
5198450 2016-09-21 00:03:47 301887  237186  07 Basil the Bear   27684   0   0
5198451 2016-09-21 00:03:47 316315  237186  08 Gummybear    27546   0   0
5198452 2016-09-21 00:03:47 350576  237186  09 Paddington   28919   0   0
5198453 2016-09-21 00:03:47 365385  237186  10 Walkingbear  26369   0   0
5198454 2016-09-21 00:03:47 377604  237186  11 Panserbjørne    25636   0   0
5198455 2016-09-21 00:03:47 404089  237186  12 YogiBear 19672   0   0
5198456 2016-09-21 00:03:47 405583  237186  13 Baloo    3938    0   0
RiggsFolly
  • 93,638
  • 21
  • 103
  • 149
kodabear
  • 340
  • 1
  • 14
  • 2
    I removed the sql-server tag. You have enough rep that you should know how to tag a question with the right database. And, if you want three columns (`data_timestamp`, `name`, `population`), why are you selecting a zillion other ones? – Gordon Linoff Sep 25 '16 at 01:30
  • 2
    Your script is at risk of [SQL Injection Attack](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) Have a look at what happened to [Little Bobby Tables](http://bobby-tables.com/) Even [if you are escaping inputs, its not safe!](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) Use [prepared parameterized statements](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) – RiggsFolly Sep 25 '16 at 01:32
  • 1
    oops sry. It has been while since i posted a question and I just clicked the thing that showed up on the tags – kodabear Sep 25 '16 at 01:32
  • 1
    location_x and location_Y are unneed and were left over from code that i used for a sql query to get a list of each town that the a player has. Player and town_deltas are needed to get the player's name and the data need for other bits of code. reading up on Prepared Statements and bound parameters – kodabear Sep 25 '16 at 01:41
  • tbh @RiggsFolly, I'd like to see an sql injection get through `intval()` in this case Lol ... Not saying you're wrong about the prepared statements though, one should always use them :D – Just Lucky Really Sep 25 '16 at 01:43

1 Answers1

2

The town population is going to be repeated over-and-over. You don't want the sum(). Just take the max():

SELECT towns.location_x,  towns.location_y,  players.*, 
       town_deltas.*, town_deltas.town_id, 
       max( town_deltas.population ) as total_population 

Your query has lots of other problems -- notably, including columns in the SELECT that are not properly aggregated. However, this might fix your immediate issue.

Gordon Linoff
  • 1,242,037
  • 58
  • 646
  • 786