When editing a product with several attributes (8 in total), I get a "Page not found" error when clicking on the Adjustments tab. I'm using Drupal 6.9.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cha0s’s picture

I'm not sure the real issue, but there is definitely a performance issue. I have tried twice to reproduce the bug and had my laptop overheat. I noticed that as soon as I try to navigate to the adjustments page, mysql is taking 100% cpu until it brings my poor lappy down. I haven't been able to reproduce the actual 'page not found' though.

I'd appreciate it if someone else can try to reproduce this bug.

cbrody’s picture

I've asked my ISP to increase the PHP max_execution_time setting, as it could be a timeout which results in the 404 error (it's currently set to 60 seconds).

cha0s’s picture

That may be the case. However, if we're hitting mysql for 60 seconds straight at full CPU, something is definitely wrong. I mean if we're talking 800 attributes maybe, but 8...

cbrody’s picture

n.b. perhaps there is a better way to collect info from subscribers on signup (which is what I'm using Ubercart for: to assign new Drupal roles on purchase of an annual membership). I couldn't find another way of ensuring that new subscribers enter the relevant information.

rszrama’s picture

Issue tags: +Scaling

Was chatting w/ cha0s about this, and the problem is it's trying to generate a page with a form of some number of thousands of rows, one for each possible option combination. Basically, the product of the number of options for each of those 8 attributes... could be quite large indeed. We've known the adjustments tab doesn't scale well, but I don't even know how you could try to manage it for this many options. You'd need more like adjustment rules or something... I suppose we should just have that page catch if it's about to display an insane number of combinations and not try to build the form.

cbrody’s picture

I only need to adjust the SKU (and the price) for one of the eight binary attributes. Is it possible to collect the other information in another way during signup, and/or remove the attributes from the Adjustments page? Ideally I would like to force the user to complete a short questionnaire prior to making an order.

Island Usurper’s picture

Title: Adjustments page not found » Adjustments page can't handle too many attributes
mikejoconnor’s picture

I would suggest we allow for an alternative method of handling sku adjustments, such as token replacement. Instead of having 16 rows for 4 color options, and 4 size options we would have one field with the following.

[basesku]-[attibute-color]-[attribute-size]

Possibly with an "advanced" option that allows either php, or a per combination setting by selecting color and size, both from a drop down, then entering the custom sku and hitting save.

Based on my experience the token field would work 75%+ of the time.

cha0s’s picture

IMHO, we should *always* be generating unique SKUs. I think that allowing more than one object to share the same SKU is a mistake, and it makes things like proper stock handling a real pain.

I realize maybe this is a bit broader than the original scope of the topic, but I think it's fair to mention anyways.

As an interim solution, it may work out to use tokens or even some simple form of regex to mass update SKUs. As I said though, if we auto-generated these off the bat, I think this'd be almost a non-issue.

cha0s’s picture

Status: Active » Postponed
misterZ’s picture

A client of mine is having this same problem. They need the product they sell to have different SKUs for when the customer selects specific attributes. They have 5 attributes, each with a few options. The total number of options then is 7 * 3 * 2 * 2 * 4 = 336 different SKUs.

This had caused problems for mySQL, and Drupal caused the site to go offline with an error from mysql saying Too many connections. I had to get the guy managing the server to restart it, which seemed to fix the site, but we cannot get into the Adjustments section (it'll just hang while trying to load the page).

As you were saying, if there was a way to auto-generate the SKUs based on Attributes, that would solve a lot of problems and save a lot of time. Is there a way to figure out the max number or attributes they can use? Or is there something in the php settings I can edit, like memory limit?

slantview’s picture

Status: Postponed » Needs review

I am seeing this same issue. Here is the query that is causing mine to hang. I have 746 unique SKUs in the {uc_product_adjustments} table.

Here is the output from show full processlist

| 267 | root | localhost       | xxx | Query   |  302 | Copying to tmp table | SELECT DISTINCT ao1.aid AS aid1, upa1.display AS display1, ao1.name AS name1, ao1.oid AS oid1, po1.ordering, ao2.aid AS aid2, upa2.display AS display2, ao2.name AS name2, ao2.oid AS oid2, po2.ordering, ao3.aid AS aid3, upa3.display AS display3, ao3.name AS name3, ao3.oid AS oid3, po3.ordering, ao4.aid AS aid4, upa4.display AS display4, ao4.name AS name4, ao4.oid AS oid4, po4.ordering, ao5.aid AS aid5, upa5.display AS display5, ao5.name AS name5, ao5.oid AS oid5, po5.ordering, ao6.aid AS aid6, upa6.display AS display6, ao6.name AS name6, ao6.oid AS oid6, po6.ordering FROM (uc_product_options AS po1 LEFT JOIN uc_attribute_options AS ao1 ON po1.oid = ao1.oid AND po1.nid = 293 LEFT JOIN uc_product_attributes AS upa1 ON upa1.aid = ao1.aid), (uc_product_options AS po2 LEFT JOIN uc_attribute_options AS ao2 ON po2.oid = ao2.oid AND po2.nid = 293 LEFT JOIN uc_product_attributes AS upa2 ON upa2.aid = ao2.aid), (uc_product_options AS po3 LEFT JOIN uc_attribute_options AS ao3 ON po3.oid = ao3.oid AND po3.nid = 293 LEFT JOIN uc_product_attributes AS upa3 ON upa3.aid = ao3.aid), (uc_product_options AS po4 LEFT JOIN uc_attribute_options AS ao4 ON po4.oid = ao4.oid AND po4.nid = 293 LEFT JOIN uc_product_attributes AS upa4 ON upa4.aid = ao4.aid), (uc_product_options AS po5 LEFT JOIN uc_attribute_options AS ao5 ON po5.oid = ao5.oid AND po5.nid = 293 LEFT JOIN uc_product_attributes AS upa5 ON upa5.aid = ao5.aid), (uc_product_options AS po6 LEFT JOIN uc_attribute_options AS ao6 ON po6.oid = ao6.oid AND po6.nid = 293 LEFT JOIN uc_product_attributes AS upa6 ON upa6.aid = ao6.aid) WHERE ao1.aid = 11 AND ao2.aid = 12 AND ao3.aid = 13 AND ao4.aid = 14 AND ao5.aid = 15 AND ao6.aid = 17 ORDER BY po1.ordering, ao1.name, po2.ordering, ao2.name, po3.ordering, ao3.name, po4.ordering, ao4.name, po5.ordering, ao5.name, po6.ordering, ao6.name LIMIT 0, 20 |

http://drupalbin.com/12319?nocache=1

I imported these products via custom import script, if you are interested, you can see it here:

http://drupalbin.com/12318?nocache=1

TR’s picture

Version: 6.x-2.0-beta4 » 6.x-2.x-dev
Status: Needs review » Active

"Needs review" is only for when there's a patch that needs review. There's no patch here yet, so moving back to "active".

Yuri’s picture

Just to confirm this issue, here is an email from my shared host. In my case, I have one product with 6 attributes and hundreds of options. I still have not seen an alternative way to offer my product combinations. My temporary solution is to non-touch the adjustment tab/button and use PhpMyadmin to edit options.

Dear Customer,
You seem to have an issue with a module or your installation which creates queries which approach a level of never ending CPU usage. These queries seem to run at 100% CPU usage for eternity. This is completely unacceptable.
The queries in question are shown below. If you start these queries again your account will be suspended due to impact on other users. We would suggest that you debug your problems on your local computer.
Thank You.

44868 carpartt_yuri localhost carpartt_carpartt Query 47434 Sending data SELECT COUNT(*) FROM (drup_uc_product_options AS po1 LEFT JOIN drup_uc_attribute_options AS ao1 ON po1.oid = ao1.oid AND po1.nid = 3159), (drup_uc_product_options AS po2 LEFT JOIN drup_uc_attribute_options AS ao2 ON po2.oid = ao2.oid AND po2.nid = 3159), (drup_uc_product_options AS po3 LEFT JOIN drup_uc_attribute_options AS ao3 ON po3.oid = ao3.oid AND po3.nid = 3159), (drup_uc_product_options AS po4 LEFT JOIN drup_uc_attribute_options AS ao4 ON po4.oid = ao4.oid AND po4.nid = 3159), (drup_uc_product_options AS po5 LEFT JOIN drup_uc_attribute_options AS ao5 ON po5.oid = ao5.oid AND po5.nid = 3159) WHERE ao1.aid = 3 AND ao2.aid = 2 AND ao3.aid = 1 AND ao4.aid = 4 AND ao5.aid = 5

Would be nice if Ubercart could prevent this from happening somehow.

tika’s picture

Subscribing... I'm getting this exact problem the Prod server just hangs forever when trying to deactivate options and save. I have many colour options for a cosmetics website. Localhost is working fine just live shared server.

Les Lim’s picture

Title: Adjustments page can't handle too many attributes » Adjustments page breaks MySQL on too many attribute option combinations
Assigned: Unassigned » Les Lim
Priority: Normal » Major

So with certain non-edge-case configurations, the Adjustments form can cause MySQL to get stuck while maxing out the CPU until the stuck processes are killed. That seems to warrant raising the priority a notch.

Some kind of token-based SKU adjustment makes sense to me, but I don't have enough of an understanding about all the places where adjustments are used to consider the implications. In the meantime, we should prevent the adjustments form from attempting impossibly large queries. Working on this now.

Les Lim’s picture

Status: Active » Needs review
FileSize
2.14 KB

Here's a patch to check if there are more than 1000 configurable combinations before continuing with the adjustments form.

Mixologic’s picture

Oh boy.. just had a fan spinnning laptop meltdown as well.

The problem here is trying to get all the information out of the database in one fell swoop in order to use the pager query.
A loop that adds self joining LEFT JOIN's to a query is bound to be non performant. n! algorithm if Im not mistaken. However, the problem isnt intractable. We simply need to do multiple queries, and marry all the results together. And then provide the pager mechanism with all the variables it needs to page through the results.

Attached is a patch that factors out the cartesian join, keeps pager results and in general makes it so that my product with nine attributes can have adjustments without a mysql meltdown.

Mixologic’s picture

Assigned: Les Lim » Unassigned

and unassigning this..

Island Usurper’s picture

Ooh, I think I like this idea. It's a little disconcerting that you have to hold the whole array in memory, but I don't think that should really be a problem since it's only a bunch of integers.

Mixologic’s picture

I wonder what it would take to exhaust memory? 10 attributes with 10 options each with a 10 character name average would probably only use about 1.6TB of memory. So maybe we should still have a limiter in there to prevent implosion? In any case its a *lot* faster than the cartesian join, which was probably forcing mysql to try and do the same thing by thrashing the disk.

rszrama’s picture

But really, who's running Drupal on a host with less than 4TB RAM these days anyways? : P

amariotti’s picture

Having this issue as well. Running the patch on the latest 2.4 version gave me the following rej file. Help?

lphm’s picture

Issue tags: +serialize, +database schema

The structure of tables on this module is not very database friendly because :

  • Use of "serialize" in uc_product_combinations is not always a good choice : If you add new attribute or delete any attribute option, adjustments are broken and so all adjustments are deleted - it is actually the process of uc_attribute. Because it so complicated to alter "Array".
  • All "LEFT JOIN" in first query on adjustements form use a lot of resources.

So, in my company, in a fork of ubercart, I changed the database structure of uc_product_combinations, to be more DB regular.

uc_product_adjustments

+------+--------------------------------------+-------------------------------------------+-----------+-----------+
| nid  | cid                                  | model                                     | cost      | price     |
+------+--------------------------------------+-------------------------------------------+-----------+-----------+
|   93 | 3b7c35f2-b00d-66d4-cde0-207d0898f87d | PACK-TELEPHONIE-MOBILE-ILLIMITE-2         | 744.00000 | 200.00000 |
|   93 | b992a827-d60a-6514-a52c-ef79ee91a72c | PACK-TELEPHONIE-MOBILE-ILLIMITE-2-POSTES  |   0.00000 | 400.00000 |
|    4 | bd48ee67-3bbb-cb14-4561-941f87f99669 | LDW-W12-0002-1                            |  27.50000 |  55.00000 |
|    4 | bbe63800-8ee5-d0f4-3988-62069b0f99df | LDW-W12-0003-1                            |  18.77000 |  81.60000 |
|    4 | df9917ee-1bd6-f994-3d4f-968507642d07 | LDW-W24-0001-1                            |  22.57500 |  45.15000 |
|    4 | 941fb951-07a2-5be4-1143-bc4b509e6bd5 | LDW-W24-0002-1                            |  40.32000 |  80.64000 |

uc_product_combinations

To store combinations, in replacement of serialize(). I have taken the first lines of this table.

mysql> SELECT * FROM uc_product_combinations;
+--------------------------------------+-----+-----+
| cid                                  | aid | oid |
+--------------------------------------+-----+-----+
| 00404a71-7c1b-4d14-9132-1567738e3847 |   1 |   1 |
| 00404a71-7c1b-4d14-9132-1567738e3847 |   2 |   8 |
| 00404a71-7c1b-4d14-9132-1567738e3847 |   8 |  12 |
| 02571cc6-3bdc-0e04-6dc3-e408889227b5 |   1 |   3 |
| 02571cc6-3bdc-0e04-6dc3-e408889227b5 |   2 |   7 |
| 02571cc6-3bdc-0e04-6dc3-e408889227b5 |   8 |  13 |
| 0b025c9e-d155-3214-ad6a-94406e491702 |   1 |   2 |
| 0b025c9e-d155-3214-ad6a-94406e491702 |   2 |   8 |
| 0b025c9e-d155-3214-ad6a-94406e491702 |   8 |  12 |
| 12ceb7e9-470a-b874-75eb-d3ff79dcd219 |   1 |   3 |
| 12ceb7e9-470a-b874-75eb-d3ff79dcd219 |   2 |   8 |
| 12ceb7e9-470a-b874-75eb-d3ff79dcd219 |   8 |  12 |
| 144eaf80-e0de-7da4-791a-9d5aadab2aca |   1 |   3 |
| 144eaf80-e0de-7da4-791a-9d5aadab2aca |   2 |   6 |

With this table schema, we need few additional lines :

  • A code to generate a GUID (the new CID - combination ID)
  • A code to retrieve GUID with a list of combination AID => OID

The new adjustments form

/**
 * Form builder to associate option combinations with mutations of a product's model number.
 *
 * @ingroup forms
 * @see product_adjustments_form_submit()
 */
function product_adjustments_form($form_state, $node) {
  drupal_set_title(check_plain($node->title));
  $nid = $node->nid;


  //Populate table and such.
  $model = $node->model;
  $product_attributes = product_get_attributes($node->nid);
  // Filter attributes on REQUIRED
  $product_attributes_buffer = array();
  foreach($product_attributes as $aid => $attribute) {
    if($attribute->required) {
      $product_attributes_buffer[$aid] = $attribute;
    }
  }
  $product_attributes = $product_attributes_buffer;

  if (count($product_attributes) > 0) {

    $sub_select_attributes = array();
    $attributes_headers = array();
    $cid_sql_filter = '';
    foreach($product_attributes as $aid => $attribute) {
      $sub_select_attributes[] = "(SELECT po.oid AS ".$aid."_oid, ao.name AS ".$aid."_name FROM {uc_product_options} po, {uc_attribute_options} ao WHERE ao.oid = po.oid
   and nid = " . $node->nid . " AND ao.aid = $aid) AS tmp_$aid";

      if(!empty($cid_sql_filter)) {
        $cid_sql_filter .= " OR ";
      }
      $cid_sql_filter .= " (c.aid = ".$aid." AND c.oid = ".$aid."_oid) ";

      $attributes_headers[] = '<th>' . $attribute->label . '</th>';
    }

    $sub_combination_find_select = " SELECT c.cid FROM {uc_product_combinations} c WHERE $cid_sql_filter GROUP BY c.cid
      HAVING COUNT(c.cid) = " . count($product_attributes);

    $sub_select_sql = implode(', ', $sub_select_attributes);

    $result = pager_query("SELECT * FROM " . $sub_select_sql, 25, 0, NULL, array());


    //Get previous values
    $adj_result = db_query("SELECT * FROM {uc_product_adjustments} WHERE nid = %d", $nid);
    $old_vals = array();

    while ($obj = db_fetch_object($adj_result)) {
      $old_vals[$obj->cid] = $obj;
    }

    $form['original'] = array(
      '#value' => '<div><br /><b>'. t('Default product SKU: @sku', array('@sku' => $model)) .'</b></div>',
    );
    $form['default'] = array(
      '#type' => 'value', '#value' => $model,
    );
    $form['table'] = array(
      '#prefix' => '<table class="combinations">',
      '#suffix' => '</table>',
    );
    $form['table']['head'] = array(
      '#prefix' => '<thead><tr>',
      '#suffix' => '</tr></thead>',
      '#value' => implode('', $attributes_headers) .'<th>Référence SKU</th>' .'<th>'.t('Cost').'</th>' .'<th>'.t('Price').'</th>',
      '#weight' => 0,
    );
    $form['table']['body'] = array(
      '#prefix' => '<tbody>',
      '#suffix' => '</tbody>',
      '#weight' => 1,
      '#tree' => TRUE,
    );

    $i = 0;
    while ($combo = db_fetch_object($result)) {

      $cells = '';
      $row_title = '';
      $comb_array = array();
      foreach ($product_attributes as $aid => $attribute) {
        $cells .= '<td>'. check_plain($combo->{$aid . '_name'}) .'</td>';
        $row_title .= check_plain($combo->{$aid . '_name'}) .', ';
        $comb_array[$aid] = $combo->{$aid . '_oid'};
      }

      ksort($comb_array);
      $row_title = substr($row_title, 0, strlen($row_title) - 2);
      $default_model = $model;
      $default_cost = $node->cost;
      $default_price = $node->sell_price;
      $default = TRUE;

      $combination_id = attribute_find_adjustment_by_combination($node->nid, $comb_array);
      if (isset($old_vals[$combination_id])) {
        $ov = $old_vals[$combination_id];

          $default_model = $ov->model;
          $default_cost = $ov->cost;
          $default_price = $ov->price;
          $default = FALSE;
      }

      $form['table']['body'][$i] = array(
        '#prefix' => '<tr title="'. $row_title .'">',
        '#suffix' => '</tr>',
      );
      $form['table']['body'][$i]['combo'] = array(
        '#value' => $cells,
      );
      $form['table']['body'][$i]['combo_array'] = array(
        '#type' => 'value',
        '#value' => serialize($comb_array),
      );
      $form['table']['body'][$i]['cid'] = array(
        '#type' => 'value',
        '#value' => $combination_id,
      );
      $form['table']['body'][$i]['model'] = array(
        '#type' => 'textfield',
        '#default_value' => $default_model,
        '#prefix' => '<td>',
        '#suffix' => '</td>',
        '#attributes' => array('class' => $default ? 'default' : ''),
      );
      $form['table']['body'][$i]['cost'] = array(
        '#type' => 'textfield',
        '#default_value' => uc_format_price_field_value($default_cost),
        '#size' => 6,
        '#prefix' => '<td>',
        '#suffix' => '</td>',
      );
      $form['table']['body'][$i]['price'] = array(
        '#type' => 'textfield',
        '#default_value' => uc_format_price_field_value($default_price),
        '#size' => 6,
        '#prefix' => '<td>',
        '#suffix' => '</td>',
      );
      ++$i;
    }

    $form['nid'] = array(
      '#type' => 'hidden',
      '#value' => $nid,
    );
    $form['submit'] = array(
      '#type' => 'submit',
      '#value' => t('Submit'),
    );
  }
  else {
    $form['error'] = array(
      '#value' => '<div><br />'. t('This product does not have any attributes.') .'</div>',
    );
  }

  $form['pager'] = array(
    '#value' => theme('pager', array(), 25),
  );

  return $form;
}

/**
 * @see product_adjustments_form()
 */
function product_adjustments_form_submit($form, &$form_state) {
  foreach ($form_state['values']['body'] as $value) {
    if (!empty($value['model']) && $value['model'] != $form_state['values']['default']) {
      db_query("UPDATE {uc_product_adjustments} SET model = '%s', cost = %f, price = %f WHERE nid = %d AND cid = '%s'",
        $value['model'], $value['cost'], $value['price'], $form_state['values']['nid'], $value['cid']);

      if (!db_affected_rows()) {
        // Generate a new UUID
        $uuid = uuid('uc_product_adjustments', 'cid');
        db_query("INSERT INTO {uc_product_adjustments} (nid, cid, model, cost, price) VALUES (%d, '%s', '%s', %f, %f)",
          $form_state['values']['nid'], $uuid, $value['model'], $value['cost'], $value['price']);

        // Insert combination
        $combo_array = unserialize($value['combo_array']);
        foreach ($combo_array as $aid => $oid) {
          db_query("INSERT INTO {uc_product_combinations} (cid, aid, oid) VALUES ('%s', %d, %d)",
            $uuid, $aid, $oid);
        }
      }
    }
    else {
      db_query("DELETE FROM {uc_product_adjustments} WHERE nid = %d AND cid = '%s'",
        $form_state['values']['nid'], $value['cid']);
      db_query("DELETE FROM {uc_product_combinations} WHERE cid = '%s'", $value['cid']);
    }
  }
  drupal_set_message(t('Product adjustments have been saved.'));
  $goto = array($_GET['q']);
  if ($_GET['page']) {
    $goto[] = 'page='. $_GET['page'];
  }
  $form_state['redirect'] = $goto;
}

/**
 * Return an array of properties for this combination.
 *
 * @param String $cid
 */
function _attribute_get_combination($cid) {
  static $combinations_id_cache;

  if (isset($combinations_id_cache[$cid])) {
    return $combinations_id_cache[$cid];
  }

  $buffer = db_query("SELECT * FROM {uc_product_combinations} WHERE cid = '%s'", $cid);
  $comb = array();
  while ($field = db_fetch_object($buffer)) {
    $comb[$field->aid] = $field->oid;
  }
  $combinations_id_cache[$cid] = $comb;
  return $comb;
}

/**
 * Generates a Universally Unique IDentifier, version 4.
 *
 * RFC 4122 (http://www.ietf.org/rfc/rfc4122.txt) defines a special type of Globally
 * Unique IDentifiers (GUID), as well as several methods for producing them. One
 * such method, described in section 4.4, is based on truly random or pseudo-random
 * number generators, and is therefore implementable in a language like PHP.
 *
 * We choose to produce pseudo-random numbers with the Mersenne Twister, and to always
 * limit single generated numbers to 16 bits (ie. the decimal value 65535). That is
 * because, even on 32-bit systems, PHP's RAND_MAX will often be the maximum *signed*
 * value, with only the equivalent of 31 significant bits. Producing two 16-bit random
 * numbers to make up a 32-bit one is less efficient, but guarantees that all 32 bits
 * are random.
 *
 * The algorithm for version 4 UUIDs (ie. those based on random number generators)
 * states that all 128 bits separated into the various fields (32 bits, 16 bits, 16 bits,
 * 8 bits and 8 bits, 48 bits) should be random, except : (a) the version number should
 * be the last 4 bits in the 3rd field, and (b) bits 6 and 7 of the 4th field should
 * be 01. We try to conform to that definition as efficiently as possible, generating
 * smaller values where possible, and minimizing the number of base conversions.
 *
 * @copyright  Copyright (c) CFD Labs, 2006. This function may be used freely for
 *              any purpose ; it is distributed without any form of warranty whatsoever.
 * @author      David Holmes <dholmes@cfdsoftware.net>
 *
 * @return  string  A UUID, made up of 32 hex digits and 4 hyphens.
 */

function uuid($table = NULL, $col = NULL) {
   if (is_null($table) || is_null($col)) {
     return uuid_generate();
   }

   $uuid = NULL;
   do {
     $uuid = uuid_generate();
     $count = db_result(db_query("SELECT COUNT(*) FROM {$table} WHERE $col = '%s'", $uuid));
   } while (intval($count) > 0);

   return $uuid;
}


function uuid_generate() {
  // The field names refer to RFC 4122 section 4.1.2
   return sprintf('%04x%04x-%04x-%03x4-%04x-%04x%04x%04x',
       mt_rand(0, 65535), mt_rand(0, 65535), // 32 bits for "time_low"
       mt_rand(0, 65535), // 16 bits for "time_mid"
       mt_rand(0, 4095),  // 12 bits before the 0100 of (version) 4 for "time_hi_and_version"
       bindec(substr_replace(sprintf('%016b', mt_rand(0, 65535)), '01', 6, 2)),
           // 8 bits, the last two of which (positions 6 and 7) are 01, for "clk_seq_hi_res"
           // (hence, the 2nd hex digit after the 3rd hyphen can only be 1, 5, 9 or d)
           // 8 bits for "clk_seq_low"
       mt_rand(0, 65535), mt_rand(0, 65535), mt_rand(0, 65535) // 48 bits for "node"
   );
}

Advantages

All adjustments are not deleted when you add or remove any attribute. Just corresponding combinations are deleted or altered. Check the code below.

/**
 * @see object_options_form()
 */
function object_options_form_submit($form, &$form_state) {
  if ($form_state['values']['type'] == 'product') {
    $attr_table = '{uc_product_attributes}';
    $opt_table = '{uc_product_options}';
    $id = 'nid';
    $sql_type = '%d';
  }
  elseif ($form_state['values']['type'] == 'class') {
    $attr_table = '{uc_class_attributes}';
    $opt_table = '{uc_class_attribute_options}';
    $id = 'pcid';
    $sql_type = "'%s'";
  }

  foreach ($form_state['values']['attributes'] as $attribute) {
    db_query("UPDATE $attr_table SET default_option = %d WHERE $id = $sql_type AND aid = %d", $attribute['default'], $form_state['values']['id'], $attribute['aid']);

    if (is_array($attribute['options'])) {
      foreach ($attribute['options'] as $oid => $option) {
        db_query("DELETE FROM $opt_table WHERE $id = $sql_type AND oid = %d", $form_state['values']['id'], $oid);

        if ($option['select']) {
          db_query("INSERT INTO $opt_table ($id, oid, cost, price, weight, ordering) VALUES ($sql_type, %d, %f, %f, %f, %d)",
                   $form_state['values']['id'], $oid, $option['cost'], $option['price'], $option['weight'], $option['ordering']);
        }
        elseif ($form_state['values']['type'] == 'product') {
          $aid = $attribute['aid'];
          db_query("DELETE FROM {uc_product_combinations} WHERE aid = %d AND oid = %d", $aid, $oid);
        }
      }
    }
  }

  drupal_set_message(t('The !type options have been saved.', array('!type' => $form_state['values']['type'] == 'product' ? t('product') : t('product class'))));
}

I hope this message will help you.

amariotti’s picture

Thanks for your work on this. Although I tried to past this in to my uc_attribute.admin.inc file and got an error.

warning: call_user_func_array() [function.call-user-func-array]: First argument is expected to be a valid callback, 'uc_product_adjustments_form' was given in /var/www/host/site.com/httpdocs/includes/form.inc on line 376.

Might be missing something though...let me know.

lphm’s picture

Hi!
To be "Ubercart compatible", rename functions product_adjustments_form to uc_product_adjustments_form() and product_adjustments_form_submit to uc_product_adjustments_form_submit() and try again.

Best regards.

Ludovic M.

amariotti’s picture

FileSize
47.93 KB

Did that, and now I get this:

Fatal error: Call to undefined function uc_attribute_find_adjustment_by_combination() in /var/www/vhosts/timelesstapestry.com/httpdocs/sites/all/modules/ubercart/uc_attribute/uc_attribute.admin.inc on line 1127

I tried to ad "uc_" but it still gives me the error. Sorry to be a pain. I'm attaching my current uc_attribute.admin.inc file. One thing to note, I am on the 2.4 version and not the 2.x-dev version, should I be updating to the dev?

lphm’s picture

Ok, I have ommited an helper function :


/**
 * Return a combination object properties with given values AID => OID.
 */
function uc_attribute_find_adjustment_by_combination($product_id, $values = array(), $return_type = 'cid', $default_sku = NULL) {
  static $adjustment_combinations;

  if(count($values) == 0) {
    return NULL;
  }

  if (isset($adjustment_combinations[$product_id . serialize($values)])) {
    $comb_object = $adjustment_combinations[$product_id . serialize($values)];

    if ($return_type == 'cid') {
      return $comb_object->cid;
    }
    else if($return_type == 'sku') {
      return $comb_object->model;
    }
    else if($return_type == 'object') {
      return $comb_object;
    }
  }

  // Build SQL filter
  $sql_filter = '';
  foreach ($values as $aid => $oid) {
    if(!empty($sql_filter)) {
      $sql_filter .= " OR ";
    }
    $sql_filter .= " (c.aid = ".$aid." AND c.oid = ".intval($oid).") ";
  }

  $query = db_query("SELECT j.*, COUNT(c.cid) AS nb_options
    FROM {uc_product_combinations} c
    INNER JOIN {uc_product_adjustments} j ON (j.cid = c.cid AND j.nid = ".$product_id.")
    WHERE ".$sql_filter."
      GROUP BY c.cid
      HAVING COUNT(c.cid) = " . count($values));

  if($sku = db_fetch_object($query)) {
    
    $adjustment_combinations[$product_id . serialize($values)] = $sku;

    if ($return_type == 'cid') {
      $result = $sku->cid;
    }
    else if($return_type == 'sku') {
      $result = $sku->model;
    }
    else if($return_type == 'object') {
      $result = $sku;
    }
  } else {
    if ($return_type == 'cid') {
      $result = NULL;
    }
    else if($return_type == 'sku') {
      $result = $default_sku;
    }
    else if($return_type == 'object') {
      $result = NULL;
    }
  }
  
  return $result;
}

Put this function into uc_attributes.module and tell me if that works.

lphm’s picture

PS: I will try this week to provide a patch with my changes, so that it is less "complicated" to alter your code...
Best regards,
lphm

TR’s picture

Status: Needs review » Needs work

@lphm: Can you please open a new issue for the proposed changes to the database structure outlined in #24 and following? That's a subject that will have to be dealt with separately from the original issue of this thread.

I would like to get back to the proposed patch in #18 and get a review of that to fix the narrow issue of combinatorics.

For the patch at #18, why does it remove t() around 'Alternate SKU', reversing #924140: 'Alternate SKU' is not translatable in uc_attribute.admin.inc ? It looks like maybe you made the patch against 2.4 instead of -dev.

Mixologic’s picture

Status: Needs work » Needs review
FileSize
5.27 KB

Ugh..looks like I had checked out my dev version before the latest commits went in.. so it appears this patch was against 1.1.2.27, and not 1.1.2.29.

Attached is a patch that applies cleanly agains 6.x-2.x-dev head.

theamoeba’s picture

Hey Mixologic,

I used your patch against 6.x-2.x-dev.
It applied beautifully :), but I now get this:

Fatal error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 45 bytes) in /Users/jonathanwagener/development/uc_dev/sites/all/modules/ubercart/uc_attribute/uc_attribute.admin.inc on line 1109

My PHP memory limit is set to 128MB.

The line in question is merging a huge array:

...
$bigger_combo[] = array_merge($existing_combo, $retrieved_combo);
...
Ddroid’s picture

Priority: Major » Critical

Same problem here. Any ideas how to make this work without going to the adjustments tab?

I need a static SKU number only on certain attributes. What if we could include the SKU under the "global" attributes settings:
admin/store/attributes/31/options/238/edit? Under the "Default adjustments" fileds would be perfect.

Any ideas would be awesome, this is keeping my site from going live!

TR’s picture

Priority: Critical » Major
Issue tags: -Scaling, -serialize, -database schema

@Ddroid: Did you try the patch?

Ddroid’s picture

I did yes. Running on localhost makes the laptop die. Upped the memory to 2gig.

stewart.adam’s picture

I'm having the same problem as Ddroid, maybe with enough memory this patch works but on my product (15 or so attributes mostly with 2 options each ("Yes/No"), with two or three having 5+ options (for colours) it pretty much hangs the server while processing the adjustments page request and then eventually runs out of memory. I tried upping it to 512MB which was no help.

dorys’s picture

I too am having this problem and it's a huge issue for my client. She has to launch her site at the end of this month. Her site has 100s of attributes.

I tried patching my software with all of the above, one by one, none of it worked. Is anyone working on this issue and will they be providing us with a new version or patch that works?

This is a HUGE issue for me.

Thanks

dorys’s picture

Any word on this?

dorys’s picture

Priority: Major » Critical
Status: Needs review » Active

OK, this issue is becoming critical to my customer's website. I just downloaded the latest dev dated 8/29... the issue still seems to be present.

I have to have the site LIVE in two weeks and I can't get through the ordering workflow due to this issue. I have a very pissed off customer right now so please... is anyone working on this actively?

TR’s picture

Priority: Critical » Major
Mixologic’s picture

Status: Active » Needs work

dorys: the patch was never committed, thus as it stands probably wont apply against the latest dev. As pointed out by theAmoeba, this patch also has the potential knock down your server due to php memory constraints. If your client has hundreds of attributes *on the same product* then, well, ubercart's attribute system just cant swing that. If your client is on their own dedicated box and you can up the memory limit, then I suggest you download the patch I posted, study it and roll your own version that applies against the current dev. All it does is replace a horrorificly terrible MySQL killer with a not quite as awful gigantic in memory array.

The "fix" unfortunately for this is architectural.. we're simply bumping up against an n! problem. Displaying every possible combination of attributes is not the right way about this.

Also, please dont change the priority of a bug to *your* priority, or mark it as 'active'. "needs review" means there is a patch that somebody other than the person who wrote it (me) should look at. Nobody did, therefore it never made it into a release. If its marked "active" then the patch I wrote might be assumed to be invalid in theory or in concept and the days that *I* spent struggling with the same problem you are having, that I *fixed* for my client, might not make it into ubercart. Though looks like various scenarios other than what I experienced are killing peoples machines.

dominict’s picture

Is there any reason this cannot be handled by setting each of the attributes and then on initial select AJAX selecting (similar to hierarchical select module approach) the options for that attribute so that each product's complete possible combinations need not be loaded up front, only the list of attributes and default options?

niklaz’s picture

Current restriction for attributes to be considered into combinations for adjustments is just to be type of display 1 or 2 - radiobox or select, the one that has numeric value and single selected option.

1. I would suggest having possibility to set in attribute's settings if some attribute should or should not be included in generating possible combinations.

This way execution time should be reduced, and we point to attributes we only want to be included.

2. I started writting tiny module, as addition to having alternative SKU auto generated.
This is the concept:

  • retrieve attributes for particular product
  • create iteration for retrieved attributes to build array for attributes only of type radiobox or select ( 1 or 2 )

    note: in addition to restricting certain attributes, to get included in combinations, check for that too.
  • create function that does recursively permutations for multidimensional array
  • get combinations with help of written function for permutations
  • iterate through combinations, and check if some exist in db and do insertion
msielski’s picture

Any updates on this issue?

Forget cases with hundreds of attributes, all you need is a handful of attributes each with a handful of options and your DB server becomes a space heater.

dorys’s picture

Unfortunately, my client hit the adjustment button last night and caused her site to go down basically until her hosting company could fix it.

A year later and I don't see a fix for this. Has it been forgotten?

Anonymous’s picture

I've just had this problem crop up in a Drupal 7 / Ubercart 7.x-3.2 install, which I've reported here - http://drupal.org/node/1806054

The site doesn't crash, but when submitting 2 attributes with around 250 combined options it just refreshes the options page without updating. I tried adding ini_set('max_execution_time', 0); to the settings.php but this doesn't help.

It only happens when trying to submit attributes with a large number of options. Can anyone help...???

Anonymous’s picture

This is a major issue. Can anyone help?

longwave’s picture

It hasn't been forgotten; just nobody has stepped up to work on it. If this is an issue for you and you have no workaround, perhaps consider paying someone to develop a fix for this problem.

Anonymous’s picture

I have spoken to my client and they are happy to pay someone to investigate this for us. Anyone out there interested?

Dan Z’s picture

You're unlikely to find someone looking here. There are lots of pros doing Drupal work, though. Try http://www.ubercart.org/forum/bounties, https://drupal.org/paid-services, and Google, for starters.

Anonymous’s picture

I just found this in the error logs.

[Fri Oct 19 08:26:55 2012] [alert] [client 122.201.109.120] /home/mysite/public_html/.htaccess: Invalid command 'SecFilterEngine', perhaps misspelled or defined by a module not included in the server configuration, referer: http://122.201.91.152/~mysite/node/624/edit/options

Anyone know what this could mean?

longwave’s picture

@afestein: That is nothing to do with Ubercart; SecFilterEngine is an Apache mod_security command in your .htaccess file, but mod_security is not enabled on your server.

Anonymous’s picture

Thanks for the assistance here. With a little paid Drupal help I discovered that max_input_vars was set to 1000 on the problem server, which was causing the form to fail when trying to post. So my problem had nothing to do with MySQL at all.

mesch’s picture

In helping @afestein resolve his issue, I had a look at the adjustment page code and thought a bit about how the current scalability issue could be addressed. I think a module could be created to replace the default adjustment page with a token-based mass assignment form, complimented with a second form to assign custom SKU's if desired.

Token page:
- uses batch api to deal with potential scalability issues
- option to not override custom SKU (custom/not custom flag stored in new field in {uc_product_adjustments})
- option token stored in new field in{uc_product_options}
- default token separator is a single underscore ("_")
- use option # if no token provided
- displays all SKUs using a pager
- displays the number of possible combinations, and the number of SKUs already set

Custom page:
- uses multiple select elements, one per attribute
- displays textfield form element for each possible combination of selected options, provided scalability criteria (tbd) are not violated (in other words, input fields are not displayed until the user has sufficiently refined their selection of possible combinations).
- displays the number possible combinations, and the number of current custom SKUs already set

Both pages:
- ensures each SKU combination is unique
- ensures SKU is not larger than permitted by uc_product_adjustments schema

I've no plans to write this at the moment, as ultimately this wasn't @afestein's issue, but thought I'd share my thoughts anyways...

Jonesgr’s picture

Hallo all. New to Drupal and Web Design for the sake of a new company. I am building a travel portal with ubercart (latest version) and Drupal 7. I know this post is about 6.x but I think its the right place to share my thoughts. I stumbled upon the same problem, as I have several attributes not affecting the SKU. I just tried a very experimental solution and I need your opinion.
First of all, I have to say that I know the $aid of the only attribute affecting the SKU. As a result,in uc_attribute.admin.inc, I add the following line: $query->condition('pa.aid', 7, '='); under line 1205 in function uc_product_adjustments_form($form, &$form_state, $node).

$query = db_select('uc_product_attributes', 'pa');
  <strong>$query->condition('pa.aid', 7, '=');</strong>
  $query->leftJoin('uc_attributes', 'a', 'pa.aid = a.aid');

This line limits the attributes that affect the SKU. Is this acceptable? Can I make such a modification? Is this secure?

Next, because I use the uc_out_of_stock.module, I had to make some modifications to this module as well.
Under Line 122:

$combination = unserialize($row->combination);
        // Apparently, on D6, one entry of the stock table has always the main SKU regardless the adjustments settings
        // Therefor, if the join gives a null record for the stock table, the main sku will be queried as well
               $position=7;
		$second[$position] = $attrs[7];
		

		if ( isset($row->sku) && $combination == $second ) {


	   // ( isset($row->sku) && $combination == $attrs ) {

Combination is an array with the attribute id (aid) and the option selected id (oid), As a result, we have to take only the option id of the attribute we are interested in. Again we know the aid which is 7.

Finally, I add three lines (in bold) in function uc_attribute_uc_product_alter(&$node) in uc_attribute.module

$position=7;
$second[$position] = $combination[7];
$combination=$second;

function uc_attribute_uc_product_alter(&$node) {
  if (isset($node->data['attributes']) && is_array($node->data['attributes'])) {
    $options = _uc_cart_product_get_options($node);
    foreach ($options as $option) {
      $node->cost += $option['cost'];
      $node->price += $option['price'];
      $node->weight += $option['weight'];
    }

    $combination = array();
    foreach ($node->data['attributes'] as $aid => $value) {
      if (is_numeric($value)) {
        $attribute = uc_attribute_load($aid, $node->nid, 'product');
        if ($attribute && ($attribute->display == 1 || $attribute->display == 2)) {
          $combination[$aid] = $value;
        }
      }
    }
	    $position=7;
		$second[$position] = $combination[7];
		$combination=$second;
    ksort($combination);

    $model = db_query("SELECT model FROM {uc_product_adjustments} WHERE nid = :nid AND combination LIKE :combo", array(':nid' => $node->nid, ':combo' => serialize($combination)))->fetchField();

    if (!empty($model)) {
      $node->model = $model;
    }
  }
}

I added these lines so I would be able to get the right sku in the checkout screen.

Sorry for the long message. I know its highly unacceptable to change modules this way but i was kind of desparate to find a solution. Waiting for your comments. Thanks.

millenniumtree’s picture

8 custom text input boxes on a product...

Two choices:
1) Multiple attribute sets, each with a single custom input box.
Result: Hideous, database crushing MySQL query, requiring more database records than atoms in the earth.
2) One attribute set with 8 text input options
Result: 8 times as many SKU variant entry boxes as necessary (text boxes should not require their own SKU!!)

Issue reported in 2009, no solution yet in 2013, and incredibly, the same buggy code is now in the D7 version.

Possible solutions:
* I like the token solution, but it may not work for everyone.
* How about a checkbox for attribute sets "no SKU adjustment".
* Disable SKU adjustments for text input options.
* Then of course, we could use an iterative approach to building the adjustments tab instead of relying on the infinite database query of doom.

The problem is not that there are trillions of SKU options, as there could be less than 100 actual SKU variants or even just 1 or 2 SKUs with a bunch of single text entry attribute sets.
If you have 16 total records in each uc_attribute_options and uc_product_options, the database is LEFT JOINing on both tables for every attribute set on the product, whether it has 1 option or 100.
DB_RECORDS ^ (ATTRIBUTE_SETS * 2)
16 ^ 16 = 18 QUINTILLION records, and that's before MySQL even has a chance to start using those primary keys.

Thankfully our site doesn't require more than 3 or 4 non-text attribute sets. I'm just going to encode all required custom text fields into a single text attribute and code around the problem for now. But that means I won't be the one to fix this either.

TR’s picture

Version: 6.x-2.x-dev » 8.x-4.x-dev
Priority: Major » Normal
Issue summary: View changes