What is wrong with my SQL query?

Question

I am trying to add in another table into an already working query but I am getting an error for some reason. I'm not sure if the query is enough to go on since it is being run by another function but I think it might just be my query..

Here is the original query that works:

$listing_sql = "select " . $select_column_list . " p.products_id, p.products_model,   

p.manufacturers_id, p.products_price, p.products_tax_class_id, IF(s.status, 
s.specials_new_products_price, NULL) as specials_new_products_price, IF(s.status, 
s.specials_new_products_price, p.products_price) as final_price 

from " . TABLE_PRODUCTS_DESCRIPTION . " pd, " . TABLE_PRODUCTS . " p 
left join " . TABLE_MANUFACTURERS . " m on p.manufacturers_id = m.manufacturers_id 
left join " . TABLE_SPECIALS . " s on p.products_id = s.products_id,
" . TABLE_PRODUCTS_TO_CATEGORIES . " p2c

where p.products_status = '1' and p.products_id = p2c.products_id and 
pd.products_id = p2c.products_id and pd.language_id = '" . (int)$languages_id . "' and 
p2c.categories_id = '" . (int)$current_category_id . "'";

And here is the new query with the the added table and where clause (TABLE_PRODUCTS_ATTRIBUTES is the new table pa):

$listing_sql = "select " . $select_column_list . " p.products_id, p.products_model, 
p.manufacturers_id, p.products_price, pa.products_values_id, p.products_tax_class_id, 
IF(s.status, s.specials_new_products_price, NULL) as specials_new_products_price, 
IF(s.status, s.specials_new_products_price, p.products_price) as final_price 

from " . TABLE_PRODUCTS_DESCRIPTION . " pd, " . TABLE_PRODUCTS . " p, " . 

TABLE_PRODUCTS_ATTRIBUTES . " pa
left join " . TABLE_MANUFACTURERS . " m on p.manufacturers_id = m.manufacturers_id 
left join " . TABLE_SPECIALS . " s on p.products_id = s.products_id,
" . TABLE_PRODUCTS_TO_CATEGORIES . " p2c

where p.products_status = '1' and p.products_id = p2c.products_id and p.products_id = 
pa.products_id and pd.products_id = p2c.products_id and pd.language_id = '" . 
(int)$languages_id . "' and p2c.categories_id = '" . (int)$current_category_id . "'";

Is there anything wrong with my query straight away?

What's the actual string look like after all that is done? Also, SQL injection - http://www.securiteam.com/securityreviews/5DP0N1P76E.html — McAden, Jun 29 '12 at 18:17
What are you joining on in the third table? The problem is with your join... There's the same amount of join statements but more tables, mysql is not going to know what you want...With the left join It is going to give you a huge results table should this actually work (column wise) Look at Mark's answer. — Hituptony, Jun 29 '12 at 18:22

Mark Byers · Accepted Answer · 2012-06-29T18:25:50.793

3

The problem is that you are mixing the comma-style join syntax with the JOIN keyword. Because they have different precedence, the joins are not being performed in the order you expected (you probably expected left-to-right order).

The MySQL manual specifically warns about mixing the two types of join:

However, the precedence of the comma operator is less than of INNER JOIN, CROSS JOIN, LEFT JOIN, and so on. If you mix comma joins with the other join types when there is a join condition, an error of the form Unknown column 'col_name' in 'on clause' may occur.

A simple change you could make to fix the error is to switch the order of TABLE_PRODUCTS_ATTRIBUTES and TABLE_PRODUCTS:

from " . TABLE_PRODUCTS_DESCRIPTION . " pd, "
. TABLE_PRODUCTS_ATTRIBUTES . " pa, "
. TABLE_PRODUCTS . " p

However this doesn't solve the real problem - that your query is unmaintainable. Better is to change all your comma-style joins to use the JOIN keyword. This will require rewriting your entire query from scratch but it will make it much easier to modify in the future.

edited Jun 29 '12 at 18:25

answered Jun 29 '12 at 18:19

Mark Byers

811,555
193
1,581
1,452

I'm a bit confused with how I am mixing the syntax, mostly because this query was already made with the mix I think. So If i were to remove the left joins basically I would just use the where clauses to match the products_id fields. or remove the where clauses relating to the comma join and use the join keyword? Using the join keyword is the better way? – Sackling Jun 29 '12 at 19:33
1

@Sackling: "Using the join keyword is the better way?" Yes. Extra reading: [Why isn't SQL ANSI-92 standard better adopted over ANSI-89?](http://stackoverflow.com/questions/334201/why-isnt-sql-ansi-92-standard-better-adopted-over-ansi-89) – Mark Byers Jun 29 '12 at 19:34
By the way, the simple fix of changing the table order still gives me an error. So I am going to try and rewrite it using only join – Sackling Jun 29 '12 at 19:41
@Sackling: There may also be a second and unrelated error that I haven't noticed. Rewriting it won't necessarily fix that error. You should post the full and exact error message if you want help. – Mark Byers Jun 29 '12 at 19:50

score 0 · Answer 2 · answered Jun 29 '12 at 18:27

Try changing that line in your query:

from " . TABLE_PRODUCTS_DESCRIPTION . " pd, " . TABLE_PRODUCTS . " p, " . TABLE_PRODUCTS_ATTRIBUTES . " pa

To:

from (" . TABLE_PRODUCTS_DESCRIPTION . " pd, " . TABLE_PRODUCTS . " p, " . TABLE_PRODUCTS_ATTRIBUTES . " pa)

The parentheses will unsure the order in which the tables are joined is the order you expect.

What is wrong with my SQL query?

2 Answers2