PHP - Best price?

Hi, I have many products which are the same in my database, but want to show only the *lowest* priced... at the moment, only the 1st result in the database is shown..

$sql = "SELECT * FROM $table WHERE make='$getmake' GROUP BY handset";

Cheers..

Quote:

Originally Posted by nikomou

Hi, I have many products which are the same in my database, but want to show only the *lowest* priced... at the moment, only the 1st result in the database is shown..

$sql = "SELECT * FROM $table WHERE make='$getmake' GROUP BY handset";

When the GROUP BY clause is used, only expressions within that clause can appear in SELECT expression list unless an aggregate function is used. That is, only handset can be used by itself; including * should raise an error.

Luckly, there is quite an appropriate aggregate function:

  SELECT handset, MIN(price) WHERE make='...' GROUP BY handset

The result set should contain one instance of each handset value with the appropriate make, and the lowest price within each group.

Mike

ORDER BY is also helpful.
If you took the first result of the list ordered by price, then that would be the lowest price. (ASC [ascending] is the default. For descending, use DESC)
Ex. of desc:
....ORDER BY `price` DESC

came up with an error.. there any other ways to do this?

Error means fix, not give up.
Post the code that gave the error.

Quote:

Originally Posted by djr33

Post the code that gave the error.

Also, please provide more information about the database structure, including what data you're actually looking for in each result.

Mike

Yeah, that too.

code used:

PHP Code:

$sql = "SELECT handset, MIN(price) WHERE make='$getmake' GROUP BY handset";

error msg:

HTML Code:

Warning: mysql_fetch_array(): supplied argument is not a valid MySQL result resource in /home/xxxx/public_html/test.php on line 18

database example:

ID | Handset | Make | Network | Tariff | Total Cost| Line Rental | Mins | Texts | URL

There are on average 20 records with the same handset name.

I want to show every handset determaned by its make, but only show the ones with the min "Total Cost"

e.g. of code

PHP Code:

<?php $getmake = $_GET['make']; $address = "localhost"; $username = "xxxx"; $password = "xxxx"; $db = "xxxx"; $table = "xxxx"; $conn = mysql_connect($address, $username, $password); $rs = mysql_select_db($db, $conn); $sql = "SELECT handset, MIN(price) WHERE make='$getmake' GROUP BY handset"; $rs = mysql_query($sql, $conn); $j = 0; echo("<table width='100%' border='0' cellpadding='0' cellspacing='0' id='$make'><tr valign='top'>"); while($row = mysql_fetch_array($rs)) { $handset = $row[handset]; $make = $row[make]; $tariff = $row[tariff]; $tariffid = $row[tariffid]; $id = $row[handsetid]; $price = $row[price]; $url = $row[url]; $gift = $row[gift]; $texts = $row[texts]; $xnet = $row[xnet]; $offpeak = $row[offpeak]; $merchant = $row[merchant]; $contractlength = $row[contractlength]; $montlycost = $row[monthlycost]; $cashback = $row[cashback]; $freerental = $row[freerental]; $halfrental = $row[halfrental]; $total = $row[total]; if ($xnet == "0") $xnetmin = (""); if ($xnet != "0") $xnetmin = ("$xnet Anytime Minutes"); if ($offpeak == "0") $offpeakmin = (""); if ($offpeak != "0") $offpeakmin = ("$offpeak Off Peak Minutes"); if ($texts == "0") $textsmin = ("Text Bundles Available"); if ($texts != "0") $textsmin = ("$texts FREE Texts"); echo("<th class='menusmall' align='center' width='16%'> "); echo("<table border='0' width='200' id='$handset' cellspacing='5' cellpadding='0'><tr><td colspan='2' width='0'><a href='http://www.xxxx.com/handsets.php?hs=$id.'>$handset</a></td><td width='9%'><a class='tditem' href='http://www.xxxx.com/handsets.php?hs=$id'><img src='http://www.xxxx.com/images/network$network.gif' border='0' align='right'></a></td></tr><tr><td width='10%' valign='top'><a href='http://www.xxxx.com/handsets.php?hs=$id'><img alt='$handset' src='http://www.xxxx.com/images/handset_$id.jpg' border='0'></a></td><td width='83%' colspan='2'>$gift$tariff $xnetmin$offpeakmin $textsmin</td></tr></table>"); echo(" </th>"); $j++; if(($j % 4) == 0) echo("</tr>\n<tr>"); } echo("</tr></table>"); mysql_close(); ?>

Hmm.
It all looks fine to me.
Are you sure that the $getmake variable is set? Did you have (eg) ?make=xxxxxx on the end of the url?

yeah, used ?make=nokia

also tried a few others, but still get the error

echo various checks along the way, like echo $rs; after it's set.
Just see if you're looking for the right thing.
Maybe there's something we're not seeing without running it.

i tried this..

get no error this way, but only the "handset" name appears, all other info is blank..

PHP Code:

$sql = "SELECT handset, MIN(total) FROM xxxxx WHERE make='$getmake' GROUP BY handset";

Quote:

Originally Posted by nikomou

PHP Code:

$sql = "SELECT handset, MIN(price) WHERE make='$getmake' GROUP BY handset";

It would have helped somewhat if you altered the statement to reflect your table structure: you don't have a "price" column and the table name needed to be added using a FROM clause. However, a GROUP BY clause isn't particularly appropriate with the amount of information you seem to want. I did mention that this might have been a possibility.

No simple, one-query solution comes to mind. Instead, obtain all handsets (adding or ORDER BY clause to sort them, if you like):

  SELECT DISTINCT handset FROM ... WHERE make='$make'

then, for each handset (loop through the results), order the options by cost and take the first one:

  SELECT * FROM ... WHERE make='$make' AND handset='$handset' ORDER BY `Total Cost` LIMIT 1

There's a possibility that there are two handsets with the same cost. I don't know what you'd want to do in that case.

Remember this time to edit the SQL statements to use the correct column/table/variable names. :) Neither of them should be used verbatim.

Quote:

$getmake = $_GET['make'];
...
$sql = "SELECT handset, MIN(price) WHERE make='$getmake' GROUP BY handset";

I really hope that you omitted the call to mysql_real_escape_string (or mysqli_real_escape_string), otherwise you're opening yourself up to SQL injection.

Mike

Hey, thanks Mike...

the correct collumn was used in the script - price is used for the cost of the handset, and total is the total cost of a contract...

How would i go about to do what your saying? i would still like the results to show in 4 collumns, could i still do that?

as for "SQL injection" - what is it? and what should i do to prevent it?!

Cheers.

A lillte helpfull debug tip:

Code:

//Try and run this after your query: if(mysql_errno() > 0){ echo mysql_error(); exit; }

Quote:

Originally Posted by nikomou

as for "SQL injection" - what is it? and what should i do to prevent it?!

SQL Injection is a way that hackers retrieve data/delete data etc in a database, this can usually be prevented by making sure that your query fits a certain format before you execute it, or that you make sure the input from the form generating the query is encoded to prevent it from making a code execute... Just need to be careful about what you let the user try to search for...

Quote:

Originally Posted by nikomou

the correct collumn was used in the script - price is used for the cost of the handset, and total is the total cost of a contract...

Oh, OK. The database "sample" you posted used different names than what I'd posted beforehand (which was just a guess). Still, an appropriate FROM clause needed to be added. I assume that it wasn't, which was why the error occurred.

Quote:

How would i go about to do what your saying?

A bare-bones example show the sort of thing I was envisioning. Obviously it isn't tested (I don't have your database to play with), but it should be a start.

If you look at the getCheapestHandset function, the SQL statement only lists a small number of column names. These were the ones I could find in your previous post. If you need others, add them. One that I wasn't sure about was the $network variable that you use when generating one of the image URIs; it wasn't defined anywhere in the code you posted.

How you handle database errors is up to you. You could use exception handling, raising appropriate exceptions where "/* Handle error */" appears in the code. An alternative would be to return null or false. Either approach will require some minor restructuring of the code around the loop.

The make query parameter isn't checked in the code below, so you should write code to make sure that it exists and is valid before using it.

PHP Code:

function getCheapestHandset($make, $handset, $dbConnection) { $statement = 'SELECT handsetid, handset, gift, tariff, xnet, offpeak, texts FROM ...' . " WHERE make='{$make}' AND handset='{$handset}' ORDER BY total LIMIT 1"; $resultSet = mysql_query($statement, $dbConnection); if (!$resultSet) { /* Handle error */ } $result = mysql_fetch_assoc($resultSet); mysql_free_result($resultSet); return $result; } function getHandsets($make, $dbConnection) { $result = array(); $statement = "SELECT DISTINCT handset FROM ... WHERE make='{$make}'" $resultSet = mysql_query($statement, $dbConnection); if (!$resultSet) { /* Handle error */ } while (($row = mysql_fetch_row($resultSet))) $result[] = $row[0]; mysql_free_result($resultSet); return $result; } $dbConnection = mysql_connect( /* ... */ ); $make = mysql_real_escape_string($_GET['make'], $dbConnection); foreach (getHandsets($make, $dbConnection) as $handset) { $cheapestHandset = getCheapestHandset($make, mysql_real_escape_string($handset), $dbConnection); /* Generate output from the elements of $cheapestHandset */ }

Quote:

i would still like the results to show in 4 collumns, could i still do that?

You could. The foreach statement is analogous to your while loop in the code you posted. However, such a rigid layout might not be a good idea.

Constructing the table you would generate seems to produce a very wide element that takes up the entire width. It might be better to create floated "cells". These would reflow based on the available width, forming a single column in a small viewport, and perhaps as many as five or six in a larger one. You could always limit the maximum width used, if necessary.

The second issue is that your markup is very bloated and invalid. In one cell, there were as many as three nested font elements. There shouldn't a single occurrence of that element anywhere in modern markup.

Quote:

as for "SQL injection" - what is it? and what should i do to prevent it?!

SQL injection involves inserting values into an SQL statement that changes how that query operates. Consider the simple query:

"SELECT `First name`, `Last name` FROM users WHERE `Last name` = '{$_GET['name']}'"

With a name query parameter value like "Jones", the resulting statement would be:

"SELECT `First name`, `Last name` FROM user WHERE `Last name` = 'Jones'"

Harmless. However, consider a value like "Jones'; UPDATE user SET password='foo' WHERE username='admin". This would produce:

"SELECT `First name`, `Last name` FROM user WHERE `Last name` = 'Jones';
UPDATE user SET password='foo' WHERE username='admin'"

It ends the first statement cleanly, then adds a completely new one. Disastrous.

Validating input is crucial, as is escaping strings that are to added to a query. Never trust anything that comes from the user, including hidden fields and the like. You can find more information on-line.

Mike

Thanks mwinter,

Thats way to complex for me! I'm having trouble putting it all together, where would i put the connection details, and all of these:

Code:

$id = $row[handsetid]; $handset = $row[handset]; $make = $row[make];

Also, what would i have to do to "Generate output from the elements of $cheapestHandset"

Could you give me an example? cheers

Quote:

Originally Posted by nikomou

... where would i put the connection details,

The variable, $dbConnection, is defined after the function definitions in the code I posted. Initialisation can go there.

Quote:

and all of these:

Code:

$id = $row[handsetid]; $handset = $row[handset]; $make = $row[make];

As I wrote previously, the foreach loop is analogous to the while loop in the code you posted. Much of what you do in that can be included in what I posted.

The return value from the getCheapestHandset function (assigned to $cheapestHandset) is the result from the database. It is an associative array that contains the column names as listed in the query. Therefore, the id, for example, can be obtained from $cheapestHandset['handsetid']. Remember that not all of data has been returned from the database; you seemed to be using only a small subset and there's no point in returning twenty values if you're only using five. Modify the query as necessary to add more values to the returned array.

As a point of syntax, don't write things like $arrayName[elementName]. For instance, in the code you posted, you had expression statements such as:

  $handset = $row[handset];

where handset is the literal name of the key. This is currently tolerated, but it's wrong. PHP will initially thing that handset is a constant, and by constant I mean something like PHP_VERSION or E_NOTICE. If it can't find a constant by that name, it assumes that you really meant to use:

  $handset = $row['handset'];

That is, a string with the value 'handset'. This tolerance may disappear in future versions of PHP, so get out of the habit of using it. This sort of thing currently generates E_NOTICE messages in the error log. The PHP manual describes this in more detail.

Quote:

Also, what would i have to do to "Generate output from the elements of $cheapestHandset"

Again, you could just do what you posted in your own code: output the contents of a table cell. You'd have to output the table start- and end-tags outside of the loop, and generate rows as you did previously. As an alternative, you could write code to generate "cells", as I described them previously. It's up to you.

Mike

PHP Code:

<?php $getmake = $_GET['make']; $address = "localhost"; $username = "xxxx"; $password = "xxxx"; $db = "xxxx"; $table = "xxxx"; $dbConnection = mysql_connect($address, $username, $password); function getCheapestHandset($make, $handset, $dbConnection) { $statement = 'SELECT handsetid, handset, gift, tariff, xnet, offpeak, texts FROM $table' . " WHERE make='{$getmake}' AND handset='{$handset}' ORDER BY total LIMIT 1"; $resultSet = mysql_query($statement, $dbConnection); if (!$resultSet) { /* Handle error */ } $result = mysql_fetch_assoc($resultSet); mysql_free_result($resultSet); return $result; } function getHandsets($make, $dbConnection) { $result = array(); $statement = "SELECT DISTINCT handset FROM $table WHERE make='{$getmake}'" $resultSet = mysql_query($statement, $dbConnection); if (!$resultSet) { /* Handle error */ } while (($row = mysql_fetch_row($resultSet))) $result[] = $row[0]; mysql_free_result($resultSet); return $result; } $dbConnection = mysql_connect($address, $username, $password); $make = mysql_real_escape_string($_GET['make'], $dbConnection); foreach (getHandsets($make, $dbConnection) as $handset) { $cheapestHandset = getCheapestHandset($make, mysql_real_escape_string($handset), $dbConnection); echo("$cheapestHandset['handsetid']") }

Quote:

Originally Posted by nikomou

$dbConnection = mysql_connect($address, $username, $password);

You only need this once (it also occurs later). Choose which one you want to keep. You'll also need to select the database, or edit the SQL statements to qualify table names.

Quote:

function getCheapestHandset($make, $handset, $dbConnection) {
    $statement = 'SELECT handsetid, handset, gift, tariff, xnet, offpeak, texts FROM $table'

Variables are not expanded in single quotes. Change them to double quotes.

Quote:

echo("$cheapestHandset['handsetid']")

When accessing an array element during variable expansion, the expression must be surrounded by braces:

  "{$cheapestHandset['handsetid']}"

Mike

Code:

Parse error: syntax error, unexpected T_VARIABLE in /home/crazy4/public_html/m4e5.php on line 24

PHP Code:

<?php $getmake = $_GET['make']; $address = "localhost"; $username = "xxxx"; $password = "xxxx"; $db = "xxxx"; $table = "xxxx"; $dbConnection = mysql_connect($address, $username, $password); function getCheapestHandset($make, $handset, $dbConnection) { $statement = "SELECT handsetid, handset, gift, tariff, xnet, offpeak, texts FROM $table WHERE make='{$make}' AND handset='{$handset}' ORDER BY total LIMIT 1"; $resultSet = mysql_query($statement, $dbConnection); if (!$resultSet) { /* Handle error */ } $result = mysql_fetch_assoc($resultSet); mysql_free_result($resultSet); return $result; } function getHandsets($make, $dbConnection) { $result = array(); $statement = "SELECT DISTINCT handset FROM $table WHERE make='{$getmake}'" $resultSet = mysql_query($statement, $dbConnection); if (!$resultSet) { /* Handle error */ } while (($row = mysql_fetch_row($resultSet))) $result[] = $row[0]; mysql_free_result($resultSet); return $result; } $make = mysql_real_escape_string($_GET['make'], $dbConnection); foreach (getHandsets($make, $dbConnection) as $handset) { $cheapestHandset = getCheapestHandset($make, mysql_real_escape_string($handset), $dbConnection); echo("{$cheapestHandset['handset']}") }

Quote:

Originally Posted by nikomou

Code:

Parse error: syntax error, unexpected T_VARIABLE in /home/crazy4/public_html/m4e5.php on line 24

Quote:

function getHandsets($make, $dbConnection) {
$result = array();
$statement = "SELECT DISTINCT handset FROM $table WHERE make='{$getmake}'"

Missing semi-colon at the end of the expression statement.

Quote:

foreach (getHandsets($make, $dbConnection) as $handset) {
/* ... */
echo("{$cheapestHandset['handset']}")

Again, a missing semi-colon.

Mike

Made the semi colon ammendments, now get...

Code:

Warning: mysql_fetch_row(): supplied argument is not a valid MySQL result resource in /home/crazy4/public_html/m4e5.php on line 27 Warning: mysql_free_result(): supplied argument is not a valid MySQL result resource in /home/crazy4/public_html/m4e5.php on line 28

PHP Code:

function getCheapestHandset($make, $handset, $dbConnection) { $statement = "SELECT handsetid, handset, gift, tariff, xnet, offpeak, texts FROM $table WHERE make='{$make}' AND handset='{$handset}' ORDER BY total LIMIT 1"; $resultSet = mysql_query($statement, $dbConnection); if (!$resultSet) { /* Handle error */ } $result = mysql_fetch_assoc($resultSet); mysql_free_result($resultSet); return $result; } function getHandsets($make, $dbConnection) { $result = array(); $statement = "SELECT DISTINCT handset FROM $table WHERE make='{$getmake}'"; $resultSet = mysql_query($statement, $dbConnection); if (!$resultSet) { /* Handle error */ } while (($row = mysql_fetch_row($resultSet))) $result[] = $row[0]; mysql_free_result($resultSet); return $result; } $make = mysql_real_escape_string($_GET['make'], $dbConnection); foreach (getHandsets($make, $dbConnection) as $handset) { $cheapestHandset = getCheapestHandset($make, mysql_real_escape_string($handset), $dbConnection); echo("{$cheapestHandset['handset']}"); }

Quote:

Originally Posted by nikomou

Made the semi colon ammendments, now get...

Do you plan on making an effort to debug your own mistakes? So far, the only issue with the code I originally posted is a single missing semicolon.

Quote:

Warning: mysql_fetch_row(): supplied argument is not a valid MySQL result resource in /home/crazy4/public_html/m4e5.php on line 27

Warning: mysql_free_result(): supplied argument is not a valid MySQL result resource in /home/crazy4/public_html/m4e5.php on line 28

If you'd looked at the SQL statements after variable expansion you'd have found out why.

Quote:

PHP Code:

function getCheapestHandset($make, $handset, $dbConnection) {       $statement = "SELECT handsetid, handset, gift, tariff, xnet, offpeak, texts FROM $table WHERE make='{$make}' AND handset='{$handset}' ORDER BY total LIMIT 1";

The variable, $table, is not defined locally, so its value will be undefined. You have four options:

Use the $GLOBALS superglobal. Replace $table with {$GLOBALS['table']}.
Include a global declaration statement.
Include the table name literally.
Change the definition of the function to include a fourth argument and pass the table name in the call.

Quote:

PHP Code:

    if (!$resultSet) { /* Handle error */ }

I do hope you plan to add error handling at some point, both here and in the other function.

Quote:

PHP Code:

function getHandsets($make, $dbConnection) {       $result = array();       $statement = "SELECT DISTINCT handset FROM $table WHERE make='{$getmake}'";

The same undefined variable issue applies here to both $table and $getmake, yet the latter is worse - much worse. You define $getmake to be the 'make' element value of the $_GET superglobal despite my warnings about SQL injection. The code I posted passes a sanitised representation of the 'make' query string parameter to the function (you can see the argument in the function declaration, above), and you choose to ignore it.

Mike