[Development] Products Specifications

♥kymation · August 19, 2009

The specs section should be enough to allow me to recreate the data. I'll have to do some mods since I don't have your products, but I can do that.

Regards

Jim

August 20, 2009

Am getting an SQL error when creating a product with attributes, adding specifications for that product and then using the "Copy To" feature (duplicate product) in admin/categories.

The error is:

1054 - Unknown column 'specification_description_id' in 'field list'

select specification_description_id, language_id, specification from products_specifications where products_id = 'xxx'

Can't see any other posts about this problem on the forum so posting it in case it's not already on the bugs list. Would the fix be to change the SQL query to read from the specification_description table instead of the products_specification one? Or is there extra code required to pull other things as well to complete the product copy?

(Just to be sure it wasn't because of my admin skin like the last error I reported, I double-checked the error on a clean install of rc2a with just product specs as the only mod. The error still occurred.)

♥kymation · August 20, 2009

Good catch! I missed that one. You need to change specification_description_id to specifications_id on Line 393 and Line 402. I'll fix that in the master copy.

Thanks for the bug report.

Regards

Jim

Yarhajile · August 20, 2009

Ran into something this evening that I thought would be worth asking about. I noticed that the tep_get_filter_sql() function generates sql strings using sql sub-selects. I was curious to whether consideration had been made to using INNER JOIN's instead of the notoriously ill-performing sub-selects? I see a pretty substantial difference in the query plans between the two methods...

Method 1 (sub-selects)

explain select 
				count(p.products_id) as count
			  from (products p) 
			  	join (products_to_categories p2c ) on (p.products_id = p2c.products_id) 
			  where p.products_status = '1' and p2c.categories_id = '1668' and p.products_id in (select products_id
                                                      from products_specifications where specification = 'Boys' and specifications_id = '29' and language_id = '1'
                                                     ) and p.products_id in (select products_id
                                                      from products_specifications where specification = 'Preschool' and specifications_id = '31' and language_id = '1'
                                                     );

output copied from phpMyAdmin, rows returned are bolded:

1 PRIMARY p2c ref PRIMARY,products_id,categories_id categories_id 3 const 3594

1 PRIMARY p eq_ref PRIMARY,products_status PRIMARY 5 partyworks_ms2max.p2c.products_id,const 1 Using where; Using index

3 DEPENDENT SUBQUERY products_specifications ref products_id,specification specification 258 const 4 Using where

2 DEPENDENT SUBQUERY products_specifications ref products_id,specification specification 258 const 2 Using where

Method 2 (Inner join)

explain select 
				count(p.products_id) as count
			  from (products p) 
			  	join (products_to_categories p2c ) on (p.products_id = p2c.products_id) 
			  	INNER JOIN products_specifications ps1 ON p.products_id = ps1.products_id
			  	INNER JOIN products_specifications ps2 ON p.products_id = ps2.products_id
			  where p.products_status = '1' and p2c.categories_id = '1668' and ps1.specification = 'Boys' and ps1.specifications_id = '29' and ps1.language_id = '1' and ps2.specification = 'Preschool' and ps2.specifications_id = '31' and ps2.language_id = '1'

output copied from phpMyAdmin, rows returned are bolded:

1 SIMPLE ps1 ref products_id,specification specification 258 const 2 Using where

1 SIMPLE p eq_ref PRIMARY,products_status PRIMARY 5 partyworks_ms2max.ps1.products_id,const 1 Using index

1 SIMPLE p2c ref PRIMARY,products_id,categories_id products_id 3 partyworks_ms2max.p.products_id 4 Using where

1 SIMPLE ps2 ref products_id,specification specification 258 const 4 Using where

Stacking filters on top of each-other like that would need a bit more work in that function to keep the inner join aliases unique (ps1, ps2, etc), but I think the streamlined queries would be worth it. Just my $.02

Elijah

♥kymation · August 20, 2009

Yes, there is a real need to optimize the filter queries. The sub-selects were used for simplicity, not speed. As you noted, the inner joins needed for a complex set of filters would require some additional work to keep things separate. Maybe use the Specification ID as the number part of the alias?

I think I had an action item on the bug list to work on this. Feel free if you want to play with the SQL.

Regards

Jim

Yarhajile · August 20, 2009

I'll see what I can do to get that hammered out tomorrow. I'm hoping that reworking how the queries are performed will reduce the slower page load times associated with the filter counting when caching isn't involved.

I still don't see a GOOD way to cache every possible filter combination for a given category under the test (and likely live) scenario I'm throwing at it. I have a simple recursive function that generates a list of every possible combination for a group of 8 spec groups with 4 having 5 filters and 4 having between 20 and 100 filters. Somewhere around 52 million possible combination's that the filters could be arranged in (and that's with no duplicates like a+b & b+a as they produce the same results). Something doesn't feel right about performing 52M db selects & then another 52M inserts into whatever caching engine is used. Plus the fact that the index would need to be refreshed each time a product or filter was added or removed.

I wonder how the sites like newegg.com and bestbuy.com do it?

Elijah

Yarhajile · August 20, 2009

Rewrote the tep_get_filter_sql() function to use inner joins instead of sub-selects and I see a HUGE performance gain, such that caching the counts wont be necessary. I haven't tested all of the filter classes, but they should be functional. The "multiple" class would be the one that has any problems since it's more complicated.

I currently have it named tep_get_filter_sql2() since everything that makes a call to that function will need to be re-worked to use the different return value (array instead of string). I had to do this since the current implementation simply appends the generated $sql_string to the end of the query in its "where" clause and in order to use INNER JOIN's, we need to also append the join to the "from" clause. This function spits out an array containing the 'from' and 'where' strings so the code requesting the functions output can put those two clauses where they need to be in the query.

/////
// Generate the SQL to return the filtered values
 function tep_get_filter_sql2 ($filter_class, $specifications_id, $filter_array = array(), $products_column_name, $languages_id) {

   $sql_array = array('from' => '', 'where' => '');

   if (isset($_GET['f' . $specifications_id])){
 	  return $sql_array;
 	}

   $filter_array = (is_array ($filter_array) ) ? $filter_array : array ($filter_array);
   // If the Show All option is set, return a blank string

   if (isset ($filter_array[0]) && ($filter_array[0] == '0' || $filter_array[0] == '') ) {
     return $sql_array;
   } else {
     // Scrub the filter array so apostrophes in filters don't error out.
     foreach($filter_array as $filterKey => $filterValue)
     {
     	$filter_array[$filterKey] = tep_db_input($filterValue);
     }

     // The Manufacturer's column contains an ID and not the name, so we have to change it
     if ($products_column_name == 'manufacturers_id') {
       $filter_array = tep_get_manufacturer_id ($filter_array, $products_column_name);
       $products_column_name = 'p.' . $products_column_name;
     }

     switch ($filter_class) {
       case 'exact':
         foreach ($filter_array as $filter) {
           if (isset ($filter) && $filter != '0' && $filter != '') {
             if (strlen ($products_column_name) > 1) { // Use an existing column
               $sql_array['where'] .= " AND " . $products_column_name . " = '" . $filter . "' ";
             } else {
             	$sql_array['from'] .= " INNER JOIN " . TABLE_PRODUCTS_SPECIFICATIONS . " ps" . $specifications_id . " ON p.products_id = ps" . $specifications_id . ".products_id ";
             	$sql_array['where'] .= " AND ps" . $specifications_id . ".specification = '" . $filter ."'
             	                         AND ps" . $specifications_id . ".specifications_id = '" . $specifications_id . "'
             	                         AND ps" . $specifications_id . ".language_id = '" . (int) $languages_id . "'
             	                          ";
             } // if (strlen ($products_column_name ... else ...
           } // if (isset ($filter
         } // foreach ($filter_array
         break;

       case 'multiple':          
      	  if (strlen ($products_column_name) > 1) {
           $sql_array['where'] .= " and " . $products_column_name . " in (";
           $first = true;
           foreach ($filter_array as $filter) {
             if ($first == true) {
               $first = false;
               $sql_array['where'] .= " '" . $filter . "' ";
             } else {
               $sql_array['where'] .= ", '" . $filter . "' ";
             }
           }
           $sql_array['where'] .= ") ";

         } else {
        	$sql_array['from'] .= " INNER JOIN " . TABLE_PRODUCTS_SPECIFICATIONS . " ps" . $specifications_id . " ON p.products_id = ps" . $specifications_id . ".products_id ";
           $first = true;
           foreach ($filter_array as $filter) {
             if ($filter != '0') {
               if ($first == true) {
                 $first = false;
                 $sql_array['where'] .= "                   AND ps" . $specifications_id . ".specification in ('" . $filter . "'
                                                   ";
               } else {
                 $sql_array['where'] .= ", '" . $filter . "'
                                                    ";
               }
             }
           }

           $sql_array['where'] .= ") AND ps" . $specifications_id . ".specifications_id = '" . $specifications_id . "'
             	                      AND ps" . $specifications_id . ".language_id = '" . (int) $languages_id . "'
             	                   ";

           foreach ($filter_array as $filter) {
             if ($filter == '0') {
               $sql_array = array('from' => '', 'where' => '');
             }
           }
         }
         break;

       case 'range':
         $filters_range = explode ('-', $filter_array[0]);
         if (strlen ($products_column_name) > 1) {
           if (count ($filters_range) < 2) {
             $sql_array['where'] .= " and " . $products_column_name . " > '" . $filters_range[0] . "'";
           } else {
             $sql_array['where'] .= " and (" . $products_column_name . " between '" . $filters_range[0] . "' and '" . $filters_range[1] . "')";
           }
         } else {
           if (count ($filters_range) < 2) { // There is only one parameter, so it is a minimum
             	$sql_array['from'] .= " INNER JOIN " . TABLE_PRODUCTS_SPECIFICATIONS . " ps" . $specifications_id . " ON p.products_id = ps" . $specifications_id . ".products_id ";
             	$sql_array['where'] .= " AND ps" . $specifications_id . ".specification > '" . $filters_range[0] ."'
             	                          AND ps" . $specifications_id . ".specifications_id = '" . $specifications_id . "'
             	                          AND ps" . $specifications_id . ".language_id = '" . (int) $languages_id . "'
             	                          ";

         } else { // There are two parameters, so treat them as minimum and maximum
         	  $sql_array['from'] .= " INNER JOIN " . TABLE_PRODUCTS_SPECIFICATIONS . " ps" . $specifications_id . " ON p.products_id = ps" . $specifications_id . ".products_id ";
             $sql_array['where'] .= " AND (ps" . $specifications_id . ".specification between '" . $filters_range[0] ."' and '" . $filters_range[1] . "')
                                       AND ps" . $specifications_id . ".specifications_id = '" . $specifications_id . "'
             	                        AND ps" . $specifications_id . ".language_id = '" . (int) $languages_id . "'
             	                        ";
           }
         }
         break;

       case 'reverse':
         // No existing columns are set up as a reverse range, so this filter class has no provision for existing columns
             	$sql_array['from'] .= " INNER JOIN " . TABLE_PRODUCTS_SPECIFICATIONS . " ps" . $specifications_id . " ON p.products_id = ps" . $specifications_id . ".products_id ";
             	$sql_array['where'] .= " AND '" . $filter_array[0] . "' BETWEEN SUBSTRING_INDEX(ps.specification,'-',1) AND SUBSTRING_INDEX(ps.specification,'-',-1)
             	                          AND ps" . $specifications_id . ".specifications_id = '" . $specifications_id . "'
             	                          AND ps" . $specifications_id . ".language_id = '" . (int) $languages_id . "'
             	                          ";
         break;

       case 'start':
         if (strlen ($products_column_name) > 1) {
           $sql_array['where'] .= " and " . $products_column_name . " like '" . $filter_array[0] . "%' ";
         } else {
           $sql_array['from'] .= " INNER JOIN " . TABLE_PRODUCTS_SPECIFICATIONS . " ps" . $specifications_id . " ON p.products_id = ps" . $specifications_id . ".products_id ";
           $sql_array['where'] .= " AND ps" . $specifications_id . ".specification LIKE '" . $filters_array[0] ."%'
                                     AND ps" . $specifications_id . ".specifications_id = '" . $specifications_id . "'
                                     AND ps" . $specifications_id . ".language_id = '" . (int) $languages_id . "'
                                     ";
         }
         break;

       case 'partial':
         if (strlen ($products_column_name) > 1) {
           $sql_array['where'] .= " and " . $products_column_name . " like '%" . $filter_array[0] . "%' ";
         } else {
           $sql_array['from'] .= " INNER JOIN " . TABLE_PRODUCTS_SPECIFICATIONS . " ps" . $specifications_id . " ON p.products_id = ps" . $specifications_id . ".products_id ";
           $sql_array['where'] .= " AND ps" . $specifications_id . ".specification like '%" . $filters_array[0] ."%'
          	                          AND ps" . $specifications_id . ".specifications_id = '" . $specifications_id . "'
          	                          AND ps" . $specifications_id . ".language_id = '" . (int) $languages_id . "'
             	                          ";
         }
         break;

       case 'like':
         // Function currently uses 'sounds like' to do a soundex match
         if (strlen ($products_column_name) > 1) {
           $sql_array['where'] .= " and " . $products_column_name . " sounds like '%" . $filter_array[0] . "%' ";
         } else {
           $sql_array['from'] .= " INNER JOIN " . TABLE_PRODUCTS_SPECIFICATIONS . " ps" . $specifications_id . " ON p.products_id = ps" . $specifications_id . ".products_id ";
           $sql_array['where'] .= " AND ps" . $specifications_id . ".specification sounds like '" . $filters_array[0] ."'
          	                          AND ps" . $specifications_id . ".specifications_id = '" . $specifications_id . "'
          	                          AND ps" . $specifications_id . ".language_id = '" . (int) $languages_id . "'
             	                          ";
         }
         break;

       case 'none':
       case '':
       default:
         break;
     } // switch ($filter_class
   } // if (count ($filter_array) ... else ...
   return $sql_array;
 }

Elijah

♥kymation · August 20, 2009

Wow. I expected a noticeable performance gain, but nothing like that. That's impressive. Good job!

I expect that the big sites have a lot of resources involved. They can afford to have an entire server dedicated to caching if they want to. The little sites running osCommerce don't have those resources, so we do what we can with what we have. In this case, it appears that getting rid of the sub-selects might be aall that's needed.

Thanks for all the work you've done on this. It will certainly benefit everyone.

Regards

Jim

Yarhajile · August 20, 2009

You've started something here with this project that will fix a lot of the navigation struggles sites have when they have lots of categories & products and I'm glad to help!

Fixed a couple of bugs in the tep_get_filter_sql2() function. This one should be used instead. The 'exact' and probably the 'multiple' filter classes still need work so they skip specs that already have filters applied instead of the hack job I did for the sql table aliases. You may have a better idea for that!

/////
// Generate the SQL to return the filtered values
 function tep_get_filter_sql2 ($filter_class, $specifications_id, $filter_array = array(), $products_column_name, $languages_id) {

   $sql_array = array('from' => '', 'where' => '');

   $filter_array = (is_array ($filter_array) ) ? $filter_array : array ($filter_array);

   // If the Show All option is set, return a blank string

   if (isset ($filter_array[0]) && ($filter_array[0] == '0' || $filter_array[0] == '') ) {
     return $sql_array;
   } else {
     // Scrub the filter array so apostrophes in filters don't error out.
     foreach($filter_array as $filterKey => $filterValue)
     {
     	$filter_array[$filterKey] = tep_db_input($filterValue);
     }

     // The Manufacturer's column contains an ID and not the name, so we have to change it
     if ($products_column_name == 'manufacturers_id') {
       $filter_array = tep_get_manufacturer_id ($filter_array, $products_column_name);
       $products_column_name = 'p.' . $products_column_name;
     }

     switch ($filter_class) {
       case 'exact':

         foreach ($filter_array as $filter) {
         	// This might not be necessary if we can skip filters for already applied specs, 
         	// but until then this will keep the query aliases from overlapping
           $key = $specification_id . '_' . crc32($filter);

           if (isset ($filter) && $filter != '0' && $filter != '') {
             if (strlen ($products_column_name) > 1) { // Use an existing column
               $sql_array['where'] .= " AND " . $products_column_name . " = '" . $filter . "' ";
             } else {
             	$sql_array['from'] .= " INNER JOIN " . TABLE_PRODUCTS_SPECIFICATIONS . " ps" . $key . " ON p.products_id = ps" . $key . ".products_id ";
             	$sql_array['where'] .= " AND ps" . $key . ".specification = '" . $filter ."'
             	                         AND ps" . $key . ".specifications_id = '" . $specifications_id . "'
             	                         AND ps" . $key . ".language_id = '" . (int) $languages_id . "'
             	                          ";
             } // if (strlen ($products_column_name ... else ...
           } // if (isset ($filter
         } // foreach ($filter_array
         break;

       case 'multiple':          
      	  if (strlen ($products_column_name) > 1) {
           $sql_array['where'] .= " and " . $products_column_name . " in (";
           $first = true;
           foreach ($filter_array as $filter) {
             if ($first == true) {
               $first = false;
               $sql_array['where'] .= " '" . $filter . "' ";
             } else {
               $sql_array['where'] .= ", '" . $filter . "' ";
             }
           }
           $sql_array['where'] .= ") ";

         } else {
        	$sql_array['from'] .= " INNER JOIN " . TABLE_PRODUCTS_SPECIFICATIONS . " ps" . $specifications_id . " ON p.products_id = ps" . $specifications_id . ".products_id ";
           $first = true;
           foreach ($filter_array as $filter) {
             if ($filter != '0') {
               if ($first == true) {
                 $first = false;
                 $sql_array['where'] .= "                   AND ps" . $specifications_id . ".specification in ('" . $filter . "'
                                                   ";
               } else {
                 $sql_array['where'] .= ", '" . $filter . "'
                                                    ";
               }
             }
           }

           $sql_array['where'] .= ") AND ps" . $specifications_id . ".specifications_id = '" . $specifications_id . "'
             	                      AND ps" . $specifications_id . ".language_id = '" . (int) $languages_id . "'
             	                   ";

           foreach ($filter_array as $filter) {
             if ($filter == '0') {
               $sql_array = array('from' => '', 'where' => '');
             }
           }
         }
         break;

       case 'range':
         $filters_range = explode ('-', $filter_array[0]);
         if (strlen ($products_column_name) > 1) {
           if (count ($filters_range) < 2) {
             $sql_array['where'] .= " and " . $products_column_name . " > '" . $filters_range[0] . "'";
           } else {
             $sql_array['where'] .= " and (" . $products_column_name . " between '" . $filters_range[0] . "' and '" . $filters_range[1] . "')";
           }
         } else {
           if (count ($filters_range) < 2) { // There is only one parameter, so it is a minimum
             	$sql_array['from'] .= " INNER JOIN " . TABLE_PRODUCTS_SPECIFICATIONS . " ps" . $specifications_id . " ON p.products_id = ps" . $specifications_id . ".products_id ";
             	$sql_array['where'] .= " AND ps" . $specifications_id . ".specification > '" . $filters_range[0] ."'
             	                          AND ps" . $specifications_id . ".specifications_id = '" . $specifications_id . "'
             	                          AND ps" . $specifications_id . ".language_id = '" . (int) $languages_id . "'
             	                          ";

         } else { // There are two parameters, so treat them as minimum and maximum
         	  $sql_array['from'] .= " INNER JOIN " . TABLE_PRODUCTS_SPECIFICATIONS . " ps" . $specifications_id . " ON p.products_id = ps" . $specifications_id . ".products_id ";
             $sql_array['where'] .= " AND (ps" . $specifications_id . ".specification between '" . $filters_range[0] ."' and '" . $filters_range[1] . "')
                                       AND ps" . $specifications_id . ".specifications_id = '" . $specifications_id . "'
             	                        AND ps" . $specifications_id . ".language_id = '" . (int) $languages_id . "'
             	                        ";
           }
         }
         break;

       case 'reverse':
         // No existing columns are set up as a reverse range, so this filter class has no provision for existing columns
             	$sql_array['from'] .= " INNER JOIN " . TABLE_PRODUCTS_SPECIFICATIONS . " ps" . $specifications_id . " ON p.products_id = ps" . $specifications_id . ".products_id ";
             	$sql_array['where'] .= " AND '" . $filter_array[0] . "' BETWEEN SUBSTRING_INDEX(ps.specification,'-',1) AND SUBSTRING_INDEX(ps.specification,'-',-1)
             	                          AND ps" . $specifications_id . ".specifications_id = '" . $specifications_id . "'
             	                          AND ps" . $specifications_id . ".language_id = '" . (int) $languages_id . "'
             	                          ";
         break;

       case 'start':
         if (strlen ($products_column_name) > 1) {
           $sql_array['where'] .= " and " . $products_column_name . " like '" . $filter_array[0] . "%' ";
         } else {
           $sql_array['from'] .= " INNER JOIN " . TABLE_PRODUCTS_SPECIFICATIONS . " ps" . $specifications_id . " ON p.products_id = ps" . $specifications_id . ".products_id ";
           $sql_array['where'] .= " AND ps" . $specifications_id . ".specification LIKE '" . $filters_array[0] ."%'
                                     AND ps" . $specifications_id . ".specifications_id = '" . $specifications_id . "'
                                     AND ps" . $specifications_id . ".language_id = '" . (int) $languages_id . "'
                                     ";
         }
         break;

       case 'partial':
         if (strlen ($products_column_name) > 1) {
           $sql_array['where'] .= " and " . $products_column_name . " like '%" . $filter_array[0] . "%' ";
         } else {
           $sql_array['from'] .= " INNER JOIN " . TABLE_PRODUCTS_SPECIFICATIONS . " ps" . $specifications_id . " ON p.products_id = ps" . $specifications_id . ".products_id ";
           $sql_array['where'] .= " AND ps" . $specifications_id . ".specification like '%" . $filters_array[0] ."%'
          	                          AND ps" . $specifications_id . ".specifications_id = '" . $specifications_id . "'
          	                          AND ps" . $specifications_id . ".language_id = '" . (int) $languages_id . "'
             	                          ";
         }
         break;

       case 'like':
         // Function currently uses 'sounds like' to do a soundex match
         if (strlen ($products_column_name) > 1) {
           $sql_array['where'] .= " and " . $products_column_name . " sounds like '%" . $filter_array[0] . "%' ";
         } else {
           $sql_array['from'] .= " INNER JOIN " . TABLE_PRODUCTS_SPECIFICATIONS . " ps" . $specifications_id . " ON p.products_id = ps" . $specifications_id . ".products_id ";
           $sql_array['where'] .= " AND ps" . $specifications_id . ".specification sounds like '" . $filters_array[0] ."'
          	                          AND ps" . $specifications_id . ".specifications_id = '" . $specifications_id . "'
          	                          AND ps" . $specifications_id . ".language_id = '" . (int) $languages_id . "'
             	                          ";
         }
         break;

       case 'none':
       case '':
       default:
         break;
     } // switch ($filter_class
   } // if (count ($filter_array) ... else ...
   return $sql_array;
 }

Here's how I currently have the tep_show_filter_count() function setup, it still needs to be cleaned up & re-factored, but it gets the job done.

 function tep_show_filter_count($current_category_id, $specification, $specification_id, $filter_class, $products_column_name, $languages_id = '1')
 {
 	$raw_query_start = "select 
				      count(p.products_id) as count
	               ";

 	$raw_query_from = " from 
			  	     (products p) 
			  	     join (products_to_categories p2c) on (p.products_id = p2c.products_id)
			  	   ";

 	$raw_query_where = " where 
     			  	  p.products_status = '1' and 
			   	      p2c.categories_id = '" . $current_category_id . "' 
			  	   ";

   $raw_query_addon = tep_get_filter_sql2($filter_class, $specification_id, $specification, $products_column_name, '1');

   $raw_query_from .= $raw_query_addon['from'];

   $raw_query_where .= $raw_query_addon['where'];

   // Loop through the $_GET vars looking for any set filters.

foreach($_GET as $k => $v) 
{
	if (preg_match('/^f[0-9]+$/', $k)) 
	{
		// We found a var that smells like a filter; append the filtered sql so we can count it.
		$k = str_replace('f', '', $k);

		if ($v == $specification)
		{
			continue;
		}			

		// Ugly hack job to store the specs array since we don't need to query it over and over again for each filter.
		//  Eventually move this whole function into an object to store states like this.
   		if (!isset($GLOBALS['specs'][$k]))
   		{
   		    $specs_query_raw = "select   s.specifications_id,
                              s.products_column_name,
                              s.filter_class,
                              s.filter_show_all,
                              s.filter_display,
                              sd.specification_name,
                              sd.specification_prefix,
                              sd.specification_suffix
                       from " . TABLE_SPECIFICATION . " s,
                            " . TABLE_SPECIFICATION_DESCRIPTION . " sd,
                            " . TABLE_SPECIFICATION_GROUPS . " sg,
                            " . TABLE_SPECIFICATIONS_TO_CATEGORIES . " s2c
                       where s.specification_group_id = sg.specification_group_id
                         and sg.specification_group_id = s2c.specification_group_id
                         and sd.specifications_id = s.specifications_id
                         and s2c.categories_id = '" . $current_category_id . "'
                         and s.show_filter = 'True'
                         and sg.show_filter = 'True'
                         and sd.language_id = '" . $languages_id . "'
                         and s.specifications_id = '" . $k . "'
                     ";

   			$specs_query = tep_db_query ($specs_query_raw);

	    	$specs_array = tep_db_fetch_array($specs_query);

	    	$GLOBALS['specs'][$k] = $specs_array;
   		}
   		else
   		{
   			$specs_array = $GLOBALS['specs'][$k];
   		}

	    $raw_query_addon = tep_get_filter_sql2($specs_array['filter_class'], $specs_array['specifications_id'], $v, $specs_array['products_column_name'], $languages_id);

           $raw_query_from .= $raw_query_addon['from'];
           $raw_query_where .= $raw_query_addon['where'];
	} // if (preg_match
} // foreach($_GET

$raw_query = $raw_query_start . $raw_query_from . $raw_query_where;

$filter_count_query = tep_db_query($raw_query);

$filter_count_results = tep_db_fetch_array($filter_count_query);

$count = (string)$filter_count_results['count'];

 	return $count;
 }

sanam · August 20, 2009

Hello Jim, thanks for taking initiative for such a big project. I know its going to take lot of effort but a great contribution lacking in oscommerce. Kudos goes to all who have been adding their time and effort in this. I have been looking for something like this for a while and very glad to find it.I am still trying to wrap my head around the code and functionality. So for start-up I have tried my hand on by adding filter bread crumbs from feature list.

73.Filters in the Breadcrumb trail, with an X to remove each one. CF Newegg.

I am not going to upload my moded file to the contrib section because i will rather have you guys go over it and may be Jim can add in next release.

All modifications are in one page catalog->products_filter.php

1) Changes to $specs_query_raw.

I rewrote this sql and instead of comparing id added joins to the table. If you want you can keep the original but add one field (sd.specification_name) and table to from (" . TABLE_SPECIFICATION_DESCRIPTION . " sd) and compare to where (and sd.specifications_id = s.specifications_id)

Edit to Original Query

  // Check for filters on each applicable Specification
 $specs_query_raw = "select s.specifications_id,
                            s.filter_class,
                            s.products_column_name, sd.specification_name
                     from " . TABLE_SPECIFICATION . " s,
                          " . TABLE_SPECIFICATION_GROUPS . " sg,
                          " . TABLE_SPECIFICATIONS_TO_CATEGORIES . " s2c,
                          " . TABLE_SPECIFICATION_DESCRIPTION . " sd
                     where s.specification_group_id = sg.specification_group_id
                       and sg.specification_group_id = s2c.specification_group_id
                       and sd.specifications_id = s.specifications_id
                       " . $category_sql . "
                       and s.show_filter = 'True'
                       and sg.show_filter = 'True'

New Query with joins

 $specs_query_raw = "SELECT s.specifications_id, s.filter_class, s.products_column_name, sd.specification_name "
                    ." FROM ".TABLE_SPECIFICATION." AS s Inner Join ".TABLE_SPECIFICATION_GROUPS." AS sg ON s.specification_group_id = sg.specification_group_id "
                    ." Inner Join ".TABLE_SPECIFICATIONS_TO_CATEGORIES." AS s2c ON sg.specification_group_id = s2c.specification_group_id "
                    ." Inner Join ".TABLE_SPECIFICATION_DESCRIPTION." sd ON sd.specifications_id = s.specifications_id "
                    ." WHERE s.show_filter =  'True' AND sg.show_filter =  'True' ".$category_sql ;

2) Add after line 53: $specs_query = tep_db_query ($specs_query_raw);

//breadcrumbs : preserve the result of the specs_query
$specs_array_breadcrumb = array();

3) Add after Line 62 : $$var = tep_clean_get__recursive ($_GET[$var]);

 //breadcrumbs : add to breadcumb array current array & sanitized $_GET value
    $specs_array_breadcrumb[] = array_merge($specs_array, array("value" => $_GET[$var]));

4) Add after Line 210, Before closing ?> <!doctype ...> :

$image = $image['categories_image'];

}

// Start of Filter Breadcrumbs 
$total_crumbs = count($specs_array_breadcrumb);
 	for($master=0;$master<$total_crumbs;$master++){ // start master: go through array and build the links
 	    $filter_link = "";
 		$master_array = $specs_array_breadcrumb[$master];
 		for($link=0;$link<$total_crumbs;$link++){ // start link: ignore the current array value from the master and add rest
 		   if($link != $master){
 		   	 $filter_link .= '&f'.$specs_array_breadcrumb[$link]['specifications_id'].'='.$specs_array_breadcrumb[$link]['value'];
 		   }
 		} // if end link
 	$breadcrumb->add('[<sup>X</sup> '.$master_array['specification_name'].' : '.$master_array['value'].']',  tep_href_link(FILENAME_PRODUCTS_FILTERS, 'cPath=' . $cPath . $filter_link));

 	}// if end master
 // End of Filter Breadcrumbs

This will add filters to the breadcrumb trail on top. The displaying part is easy and can modified for individual taste. Hope it works, any suggestions are welcomed.

♥kymation · August 20, 2009

Thanks for the code. I'll play with that as soon as I get some time. And of course I'll add it to the package.

I really appreciate everyone who's helping out here, whether by adding code, testing, or making comments on the features. You're all making this addon so much better for everyone.

Regards

Jim

Yarhajile · August 21, 2009

Sanam,

Your breadcrumb code worked straight out of the box on my setup over here. Thanks for the work on that!

I did modify the crumb generation loop just a bit to shrink it down.

This code

// Start of Filter Breadcrumbs
$total_crumbs = count($specs_array_breadcrumb);

for($master=0;$master<$total_crumbs;$master++){ // start master: go through array and build the links
 $filter_link = "";
 $master_array = $specs_array_breadcrumb[$master];

 for($link=0;$link<$total_crumbs;$link++){ // start link: ignore the current array value from the master and add rest
   if($link != $master){
     $filter_link .= '&f'.$specs_array_breadcrumb[$link]['specifications_id'].'='.$specs_array_breadcrumb[$link]['value'];
   }
 } // if end link

 $breadcrumb->add('[<sup>X</sup> '.$master_array['specification_name'].' : '.$master_array['value'].']', tep_href_link(FILENAME_PRODUCTS_FILTERS, 'cPath=' . $cPath . $filter_link));
}// if end master
// End of Filter Breadcrumbs

can be accomplished with this instead for a smaller footprint. The tep_get_all_get_params() function is the biggest player here as it does the work your loop does..

// Start of Filter Breadcrumbs
 foreach($specs_array_breadcrumb as $crumb){
   $breadcrumb->add('<sup>X</sup>' . $crumb['specification_name'] . ' : ' . $crumb['value'], tep_href_link(FILENAME_PRODUCTS_FILTERS, tep_get_all_get_params(array('f' . $crumb['specifications_id']))));
 }
// End of Filter Breadcrumbs

Elijah

Yarhajile · August 21, 2009

It also looks like the change in step 3

		//breadcrumbs : add to breadcumb array current array & sanitized $_GET value
   $specs_array_breadcrumb[] = array_merge($specs_array, array("value" => $_GET[$var]));

should be

		//breadcrumbs : add to breadcumb array current array & sanitized $_GET value
	  $specs_array_breadcrumb[] = array_merge($specs_array, array("value" => $$var));

in order to use the sanitized $$var

Elijah

sanam · August 21, 2009

It also looks like the change in step 3

		//breadcrumbs : add to breadcumb array current array & sanitized $_GET value
   $specs_array_breadcrumb[] = array_merge($specs_array, array("value" => $_GET[$var]));

should be

		//breadcrumbs : add to breadcumb array current array & sanitized $_GET value
	  $specs_array_breadcrumb[] = array_merge($specs_array, array("value" => $$var));

in order to use the sanitized $$var

Elijah

Thanks Elijah, I had $$var but during cut and paste trying to get right line numbers kind of mistyped it.

The loop was a good call, keep it going.

Sukhbir

jonathanbastin · August 21, 2009

Jim did you get my sql dump in a PM?

♥kymation · August 21, 2009

Yes, I got it. Sorry, I should have replied. I've been busy here. Elijah is working on an optimized SQL that will change the way filters are applied, so I need to wait for that before doing any more testing anyway.

Regards

Jim

Yarhajile · August 21, 2009

♥kymation · August 21, 2009

Thanks again for doing all of this work. I can hardly wait to finish what I'm doing so I can go play with this. Soon, I hope.

Regards

Jim

Yarhajile · August 21, 2009

You're welcome!

Yarhajile · August 22, 2009

I've moved the counting function into an object so that a some of the more repetitive queries can be stored in the object rather than in $GLOBALS. No doubt this can be built out even further to incorporate some of the other features this mod provides, but at a more modular level.

The functionality is the same, just a little cleaner delivery.

New file: catalog/includes/classes/Specifications.php

<?php
class Specifications
{
private $specs = array();
private $applied_filters = array();
private $current_category_id;
private $languages_id;

public function __construct()
{
	global $current_category_id, $languages_id;

	$this->current_category_id = $current_category_id;
	$this->languages_id = $languages_id;

	$this->setAppliedFilters();
}

private function setAppliedFilters()
{
	$category_sql = $this->current_category_id != 0 ? "and s2c.categories_id = '" . $this->current_category_id . "'" : '';

	// Check for filters on each applicable Specification
	$specs_query_raw = "SELECT 
 						s.specifications_id, 
 						s.filter_class, 
 						s.products_column_name, 
 						sd.specification_name
 					  FROM 
 					  	" . TABLE_SPECIFICATION . " AS s 
 					  	INNER JOIN " . TABLE_SPECIFICATION_GROUPS . " AS sg 
 					  		ON s.specification_group_id = sg.specification_group_id
 					  	INNER JOIN " . TABLE_SPECIFICATIONS_TO_CATEGORIES . " AS s2c 
 					  		ON sg.specification_group_id = s2c.specification_group_id
 					  	INNER JOIN " . TABLE_SPECIFICATION_DESCRIPTION . " sd ON sd.specifications_id = s.specifications_id
 					  WHERE 
 					  	s.show_filter = 'True' 
 					  	AND sg.show_filter = 'True' " . $category_sql; 

	$specs_query = tep_db_query ($specs_query_raw);

	while ($specs_array = tep_db_fetch_array ($specs_query) ) 
	{
		// Retrieve the GET vars used as filters
		// Variable names are the letter "f" followed by the specifications_id for that spec.
		$var = $specs_array['specifications_id'];
		$$var = '0';

		if (isset ($_GET['f' . $var]) && $_GET['f' . $var] != '') 
		{
			// Sanitize variables to prevent hacking
			$$var = tep_clean_get__recursive ($_GET['f' . $var]);

			// Set the cporrect variable type (All _GET variables are strings by default)
			$$var = tep_set_type ($$var);

			$this->applied_filters[$var] = $$var;
		} // if (isset ($_GET[$var]

	} // while ($specs_array
}

private function getAppliedFilters()
{
	return $this->applied_filters;
}

public function getFilterCount($specification, $specifications_id, $filter_class, $products_column_name)
{
  	$raw_query_start = "select count(p.products_id) as count ";

  	$raw_query_from = " from (products p) join (products_to_categories p2c) on (p.products_id = p2c.products_id) ";

 		$raw_query_where = " where p.products_status = '1' and p2c.categories_id = '" . $this->current_category_id . "' ";

   	$raw_query_addon_array = tep_get_filter_sql($filter_class, $specifications_id, $specification, $products_column_name, '1');

   	$raw_query_from .= $raw_query_addon_array['from'];

   	$raw_query_where .= $raw_query_addon_array['where'];

   	$applied_filters = $this->getAppliedFilters();

	foreach($applied_filters as $k => $v) 
	{
		if ($v == $specification)
		{
			continue;
		}			

		$specs_array = $this->getSpecification($k);

	    $raw_query_addon_array = tep_get_filter_sql($specs_array['filter_class'], $specs_array['specifications_id'], $v, $specs_array['products_column_name'], $this->languages_id);

           $raw_query_from .= $raw_query_addon_array['from'];
           $raw_query_where .= $raw_query_addon_array['where'];
	} // foreach($_GET

	$raw_query = $raw_query_start . $raw_query_from . $raw_query_where;

	$filter_count_query = tep_db_query($raw_query);

	$filter_count_results = tep_db_fetch_array($filter_count_query);

	$count = (string)$filter_count_results['count'];

  	return $count;
 	}

private function getSpecification($id)
{
	if (!isset($this->specs[$id]))
	{
		$specs_query_raw = "SELECT 
							s.specifications_id,
							s.products_column_name,
							s.filter_class,
							s.filter_show_all,
							s.filter_display,
							sd.specification_name,
							sd.specification_prefix,
							sd.specification_suffix
						FROM 
							" . TABLE_SPECIFICATION . " s
							JOIN " . TABLE_SPECIFICATION_DESCRIPTION . " sd
								ON s.specifications_id = sd.specifications_id
							JOIN " . TABLE_SPECIFICATION_GROUPS . " sg
								ON s.specification_group_id = sg.specification_group_id
							JOIN " . TABLE_SPECIFICATIONS_TO_CATEGORIES . " s2c
								ON sg.specification_group_id = s2c.specification_group_id
						WHERE 
							s.specifications_id = '" . $id . "'
							and s.show_filter = 'True'
							and sg.show_filter = 'True'
							and sd.language_id = '" . $this->languages_id . "'
							and s2c.categories_id = '" . $this->current_category_id . "'
						";

		$specs_query = tep_db_query($specs_query_raw);

		$this->specs[$id] = tep_db_fetch_array($specs_query);
    }

    return $this->specs[$id];
 	}
}

In catalog/includes/boxes/products_filter.php around line 47 before the 'while' loop for the $specs_array, add this.

~SNIP~
         $count = 1;

         if ( !isset($_GET['f' . $specs_array['specifications_id']]))
         {
			$count = $spec_object->getFilterCount($filter_id, $specs_array['specifications_id'], $specs_array['filter_class'], $specs_array['products_column_name']);
         }

         if (SPECIFICATION_FILTER_NO_RESULT != 'normal') 
         {
         	if ($count == 0)
         	{
          	continue;
         	}
         }

         $filters_select_array[$filter_index] = array ('id' => $filter_id,
                                                       'text' => $filter_text,
                                                       'count' => $count
                                                      );
         $filter_index++;
       } // while ($filters_array

       if ($specs_array['filter_class'] == 'range') 
       {
         if ($specs_array['products_column_name'] == 'products_price') 
         {
           $previous_filter = $currencies->format ($previous_filter);
         }

	  $count = $spec_object->getFilterCount($previous_filter_id, $specs_array['specifications_id'], $specs_array['filter_class'], $specs_array['products_column_name']);

         if (SPECIFICATION_FILTER_NO_RESULT != 'normal') 
         {
         	if ($count > 0)
         	{
           	$filters_select_array[$filter_index] = array ('id' => $previous_filter_id,
               	                                          'text' => $previous_filter . '+',
                    		                                  'count' => $count
                           		                          );
         	}
         }
       }
~SNIP~

I'll be working on implementing some of the other features of the mod into this object to consolidate the code for our store even more, but will post any of the updates I make here so that whomever chooses to use them can do so.

Elijah

♥kymation · August 22, 2009

Thanks again for all of your hard work. I was just starting to put together an updated package to try out.

Regards

Jim

Yarhajile · August 22, 2009

Thanks again for all of your hard work. I was just starting to put together an updated package to try out.

Regards

Jim

I think I'm done for now (the weekend at least!) making any further changes. Our current setup is in an eye-brow raising satisfactory condition, so as long as the security checks out ok, we'll be moving forward in getting this to a heavy testing stage this next week.

Thanks for providing all of the work you've done on this!

Yarhajile · August 22, 2009

I've looked through the queries being performed and everything seems to look OK as far as indexing goes. The specification_description table's indexes don't get used in the query plans since there are so few rows in the table on my end. The indexes are there though on the specifications_id, so I wouldn't expect any problems.

If you're going to be creating the package using the counting code, you'll notice the deficient area of what I've been working on is on the SPECIFICATION_FILTER_NO_RESULT conditions. I simply disable the filters if there aren't any results, but it should be easy enough to implement the "greyed out" feature here to pass on to the tep_get_filter_string() function. I'd have done it myself if I felt a need to use it. :)

Just wanted to point that out since it's the only incomplete item that I could think of off the top of my head.

♥kymation · August 22, 2009

I noticed that. I'll add in that option as soon as I get done playing with this.

Regards

Jim

jonathanbastin · August 23, 2009

Yes, I got it. Sorry, I should have replied. I've been busy here. Elijah is working on an optimized SQL that will change the way filters are applied, so I need to wait for that before doing any more testing anyway.

Regards

Jim

No worries just wanted to check as it didn't register me sending it.

[Development] Products Specifications

Recommended Posts

Link to comment

Share on other sites

Top Posters In This Topic

Popular Days

Top Posters In This Topic

Popular Days

Popular Posts

akbareshghi

kymation

kymation

Posted Images

Guest

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Join the conversation