I ran into an issue with when running update 6313 which updates the values on grid type fields when adding keys. The grid values I am working with are a rating system and are titled 5, 4, 3, 2, 1 (in that order). So, the values in the data field in webform_submitted_data are all 5, 4, 3, 2, or 1 before the update runs.

The update adds keys to this option list and then updates the values stored to the key instead of the option title. The keys added are 0, 1, 2, 3, 4 so that the key value options look like this:
0|5
1|4
2|3
3|2
4|1

Since the option titles are similar to the key values, this caused a problem in this way:

Fields with data value 5 were all updated to have a 0 instead. No problems here.
Then, fields with data value 4 were updated to have a 1 instead. No problems here.
Later when it got to data value 1, it updated the rows with a value of 1 to have a value of 4.
Unfortunately, since the opposite query had just been run, it ended up moving all rows with value 1 or 4 to having value 4. The same issue occurred with values 2 and 3 and hence the data which previously had values 1, 2, 3, 4 was no longer correct.

Attached is a patch that does all of the grid updates in a single query instead of doing them separately. This solved my problem.

CommentFileSizeAuthor
#2 webform-grid-update-fix-2021617-2.patch1.15 KBkleinmp
webform-update-6313-fix.patch1.06 KBkleinmp

Comments

quicksketch’s picture

quicksketch
he/him
commented
Status: Active » Needs work

Thanks for this extremely well-written summary and patch! It looks like this issue only applies to the 6.x-3.x version of the module. This update was written before the D7 version came into existence.

It looks like this patch has a problem though that it would run the query replacing all the keys once for every key, which would now be unnecessary since all the keys are done at once:

     // Convert the option values into keys if new ones were created.
     foreach ($new_keys as $delta => $new_key) {
-      db_query("UPDATE {webform_submitted_data} SET data = '%s' WHERE nid = %d AND cid = %d AND data = '%s'", $new_key, $row->nid, $row->cid, $values[$delta]);
+      if (!empty($new_keys)) {
+        $args = array();
+        $sql = "UPDATE {webform_submitted_data}
+                SET data = CASE data";
+        foreach ($new_keys as $delta => $new_key) {

As you can see the foreach ($new_keys as $delta => $new_key) { loop is executed twice. I don't think this is intentional.

kleinmp’s picture

kleinmp commented
Status: Needs work » Needs review
StatusFileSize
new webform-grid-update-fix-2021617-2.patch1.15 KB

Oh, whoops. I meant to remove the foreach too.

quicksketch’s picture

quicksketch
he/him
commented
Status: Needs review » Fixed

Thanks, committed!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.