4

I am trying create an average rating for a store listing using MySQL's AVG and then displaying the average next to each store on a directory list page.

Every time someone leaves a review for a store, they give a rating of 1-5, which is inserted into TABLE reviews in column rating.

The store data is coming from TABLE reviews and the table relationship is coming from a column in both tables named store_id. I trying to use a join statement to get the average to display next to each store.

This is the current code in the loop for the rating with join statement:

<?php echo "<table>\n";
echo "<tr><th>Store ID</th><th>Rating</th></tr>\n";
$result1 = mysql_query("SELECT s.store_id AVG(r.rating) AS avg_rating
                       FROM stores as s
                       JOIN reviews as r ON s.store_id = r.store_name
                       GROUP BY s.store_id");
while ($row1 = mysql_fetch_assoc($result1)) {
    echo "<tr><td>$row1[store_id]</td><td>$row1[avg_rating]</td></tr>\n";
}
echo "</table>\n";?>
echo "</table>\n";?>

This is the SQL error I am getting

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '(r.rating) AS avg_rating FROM stores'

This is the PHP error I am getting:

Warning: mysql_fetch_assoc() expects parameter 1 to be resource, boolean given in /home/shopping/public_html/retailers.php on line 227

Line 227 is this line from the code above:

while ($row1 = mysql_fetch_assoc($result1)) {

These are my tables' two structures:

CREATE TABLE `stores` (
  `store_id` int(11) unsigned NOT NULL AUTO_INCREMENT,
  `store_name` varchar(255) NOT NULL DEFAULT '',

CREATE TABLE `reviews` (
  `rating_id` int(11) unsigned NOT NULL AUTO_INCREMENT,
  `store_id` int(11) NOT NULL DEFAULT '0',
  `rating` tinyint(1) NOT NULL DEFAULT '0',
Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Michael Falciglia
  • 1,046
  • 4
  • 20
  • 36
  • 3
    *sidenote:* stop using deprecated `mysql_*` functions. use [MySQLi](http://php.net/manual/en/book.mysqli.php) or [PDO](http://php.net/manual/en/book.pdo.php) instead. – Raptor Dec 27 '13 at 03:49
  • Do you have an r.store_name? Perhaps you mean r.retailer_id? – tabstop Dec 27 '13 at 03:50
  • Please, do not use the truly awful `mysql_query` interface. The problem here is you're getting `false` back instead of a resource, your query didn't work, so you can't fetch it. PDO can emit exceptions which are a lot harder to miss. Stop using `mysql_query` before you create even more legacy code you'll have to re-write. – tadman Dec 27 '13 at 03:50
  • You should check whether the query is successful. If not, you should call `die(mysql_error())` to see the error my SQL. – Barmar Dec 27 '13 at 03:51
  • @barmar Ugh. `or die` is the worst thing in PHP. That's not error handling. That's giving up. – tadman Dec 27 '13 at 03:52
  • @tadman it's better than continuing on with the bad data. If he uses PDO and it emits an exception, won't that also just give up? – Barmar Dec 27 '13 at 03:54
  • The idea is you present an error to the user that makes sense, not some low-level MySQL error they should not be seeing. **DO NOT** ever expose your database schema or queries to the user. PDO is slightly better because you can catch exceptions and render a proper error page, the flow control is way easier. Even better is using a [development framework](http://codegeekz.com/best-php-frameworks-for-developers/) that does a lot of this for you out of the box. – tadman Dec 27 '13 at 03:57
  • @Barmar I got a lot of original errors from last night fixed, the other store data is not showing up. I will try the `die(mysql_error())` do I place it after the query statement? – Michael Falciglia Dec 27 '13 at 03:59
  • @MichaelFalciglia I strongly encourage you to spend the small amount of time required to learn a proper [development framework](http://codegeekz.com/best-php-frameworks-for-developers/) because the mistakes you're making here are related to the fact that you're writing against the least capable, lowest-level, most obsolete driver available in PHP. Having a proper database layer largely eliminates a lot of this drudgery. You are creating an extraordinary amount of pointless work for yourself by following this path, the same one that fell out of style ten years ago. – tadman Dec 27 '13 at 04:03
  • @tabstop I actually copied it from an old table and I meant to edit it on the question, I just fixed it – Michael Falciglia Dec 27 '13 at 04:05
  • @tadman I don't disagree with you, but this really what I am doing with this project, trying to learn. I did not create the script, I am just modifying it to add features. I have been spending my days going through tutorials and trying to learn and whatever I can't figure out I have been coming here at night asking questions to help me. – Michael Falciglia Dec 27 '13 at 04:07
  • Frameworks can be great, I will agree. However, if someone doesn't understand how classes & functions etc. work, and need to fix something if/when something goes wrong, then it makes much more difficult to fix. Frameworks are built on such elements. It's like throwing someone into a Trapeze (Cirque du Soleil for instance) and is afraid of heights or has depth perception problems, will surely cause bigger problems when **The Big Show must go on** and people are waiting to see the Opening act without seeing bloodshed. @tadman It's easy for many, but not for many others as well. My 2 cents. – Funk Forty Niner Dec 27 '13 at 04:21
  • 1
    @Fred-ii- I think you make a valid point, I was using codeigniter to learn, but there was a lot of stuff I did not understand that I wanted t. I feel like I can do much more with straight php and I am learning more about MySQL this way. – Michael Falciglia Dec 27 '13 at 04:44
  • @MichaelFalciglia "Straight PHP" is the wrong way to do it. That's extremely tedious and error prone. Using a framework is not hard, it provides a sort of guidance on how to do it *correctly*, and provides a lot of community code that will work with your application without modification. `mysql_query` code will have to be thrown in the garbage when it's pulled from PHP in the near future so you're doing yourself a massive disservice by learning this antiquated technique. – tadman Dec 27 '13 at 18:49
  • @Fred-ii- Why does the PHP community have such animosity towards frameworks when the Python, Ruby and Perl communities have no problems whatsoever, where frameworks are the *de-facto* why to approach these problems? This anti-framework attitude needs to stop. Suggesting it's easier to learn super low-level database calls is like saying it's easier to learn how to drive by building your own car out of raw materials. What you'll end up with will be non-standard, will have all kinds of design flaws, and will be extremely difficult to maintain. – tadman Dec 27 '13 at 18:53
  • @tadman Why don't you help me figure out this question I posted :) And I am for frameworks for faster development reasons, I just have always liked to learn as much as I can, so trying to learn as much about php as I can, which will only make me better when I use frameworks. – Michael Falciglia Dec 27 '13 at 20:01
  • My comment wasn't based on animosities, just noting about the complexity of frameworks and if someone doesn't know their way around them, that it could potentially cause confusion, that's all. @tadman I for one do appreciate your suggestions. – Funk Forty Niner Dec 27 '13 at 20:05
  • @MichaelFalciglia Frameworks first, PHP second. Don't do this the other way around. A good framework will naturally guide your learning, and you'll have much better examples to work from. As to your MySQL problem you already have an answer here that tackles that part. If you insist on using `mysql_query`, you're in for a very hard time. – tadman Dec 27 '13 at 20:05
  • @Fred-ii- If frameworks cause confusion, you're using the wrong frameworks. Seriously, what is the big deal here? People using frameworks don't have these sorts of elementary questions because their code is already working and they've moved on to more challenging concerns. It's a lot easier to pick up a framework with great examples and lots of modular code than to throw someone the PHP manual and leave them to fend for themselves. – tadman Dec 27 '13 at 20:06
  • *"you're using the wrong frameworks"* --- I wasn't talking about "me"; do take the time to actually read and understand my wording. And there is no "big deal" here, you're making it a big deal. @tadman Just stop. – Funk Forty Niner Dec 27 '13 at 20:09
  • @tadman I have a friend who does Ruby-On-Rails, and he is really good at it, but he sucked in the beginning and other than learning C in college, it was his first real language he learned. It took him almost 2 years to get decent at it. I can do more with PHP from the last few months of learning than he could do during his first two years. Now today he is really good at it and making good money, constantly being recruited by companies, but my point is I think knowing the foundation of something can only make you excel at it in the long run. – Michael Falciglia Dec 27 '13 at 20:24
  • @tadman FYI - I do appreciate your input, I am willing to learn whatever, I just have a lot of clients who have php sites. Up till now I always hired programmers in India to do work or hacked scripts, and focused on the business end, but recently I thought it would be nice to be able to fix a simple problem without having to contact a programmer to do it, so since the majority of my projects are in non-frameworked php I need to learn how to fix them. Also I don't know – Michael Falciglia Dec 27 '13 at 20:31
  • possible duplicate of [mysql\_fetch\_array() expects parameter 1 to be resource, boolean given in select](http://stackoverflow.com/questions/2973202/mysql-fetch-array-expects-parameter-1-to-be-resource-boolean-given-in-select) – John Conde Dec 28 '13 at 00:44

1 Answers1

3

Your SQL query is invalid. You need to either omit s.store_name from the query or include it in the GROUP BY

SELECT 
    s.store_id, 
    s.store_name, 
    AVG(r.rating) AS     
FROM stores as s
    JOIN reviews as r ON s.store_id = r.store_id
GROUP BY s.store_id, s.store_name
Dave Zych
  • 21,581
  • 7
  • 51
  • 66
  • Are you saying to remove the `avg_rating` from `AVG(r.rating) AS avg_rating` and use `rating` as the variable in the echo statement? – Michael Falciglia Dec 27 '13 at 04:18
  • No, I'm saying that your SQL query doesn't run. When using a GROUP BY, all of the items in your select need to be aggregate functions or specified in the grouping clause. Review the SQL query in my answer which shows this. – Dave Zych Dec 27 '13 at 04:23
  • I understand what you were saying how it was omitted. I was just asking because I saw you did not have that written on your example, so I was just checking. – Michael Falciglia Dec 27 '13 at 04:26
  • Also I just fixed it and it is still giving me the same error. I would think the query was working because it is in a loop pulling from the same database and all the data is showing up in the loop from the array that I have. – Michael Falciglia Dec 27 '13 at 04:28
  • I just took your first advice and omitted it and now the error is showing this `You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near (r.rating) AS avg_rating FROM stores at line 1` – Michael Falciglia Dec 27 '13 at 04:40
  • This query looks fine. Have you tried running it independent of your PHP code? – tadman Dec 27 '13 at 20:10