taxonomy_update_7005() is a batched update, running

<?php
    $query
= 'SELECT td.vid AS vocab_id, td.tid, tn.nid, tn.vid, n.type, n2.created, n2.sticky, n2.nid AS is_current FROM {taxonomy_term_data} td INNER JOIN {taxonomy_term_node} tn ON td.tid = tn.tid INNER JOIN {node} n ON tn.nid = n.nid LEFT JOIN {node} n2 ON tn.vid = n2.vid ORDER BY tn.vid, td.weight ASC, tn.tid';
?>

for every batch. This query is slow and I think adds up to a lot of time. Drupal.org uses 1418 batches.

We should do this query once and store the data.

Files: 
CommentFileSizeAuthor
#11 1549390.patch6.27 KBBTMash
PASSED: [[SimpleTest]]: [MySQL] 38,769 pass(es).
[ View ]
#10 1549390.patch6.32 KBBTMash
FAILED: [[SimpleTest]]: [MySQL] 37,564 pass(es), 1,147 fail(s), and 5 exception(s).
[ View ]
#3 1549390.patch6.37 KBdrumm
FAILED: [[SimpleTest]]: [MySQL] 38,728 pass(es), 2,424 fail(s), and 1,232 exception(s).
[ View ]
#1 1549390.patch6.72 KBdrumm
FAILED: [[SimpleTest]]: [MySQL] 37,546 pass(es), 1,147 fail(s), and 5 exception(s).
[ View ]

Comments

Status:Active» Needs review
Issue tags:+Drupal.org D7
StatusFileSize
new6.72 KB
FAILED: [[SimpleTest]]: [MySQL] 37,546 pass(es), 1,147 fail(s), and 5 exception(s).
[ View ]

Let's see if this untested patch works.

Status:Needs review» Needs work

The last submitted patch, 1549390.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new6.37 KB
FAILED: [[SimpleTest]]: [MySQL] 38,728 pass(es), 2,424 fail(s), and 1,232 exception(s).
[ View ]

Fix whitespace.

Status:Needs review» Needs work

+++ b/modules/taxonomy/taxonomy.installundefined
@@ -592,36 +592,107 @@ function taxonomy_update_7005(&$sandbox) {
+    $result = db_query_range('SELECT vocab_id, tid, nid, vid, type, created, sticky, is_current FROM {taxonomy_update_7005} ORDER BY n', $sandbox['last'], $batch);

"Order by n"? Typo?

Status:Needs work» Needs review

Alright, drumm clarified this in IRC. n is here:

+++ b/modules/taxonomy/taxonomy.installundefined
@@ -592,36 +592,107 @@ function taxonomy_update_7005(&$sandbox) {
+        'n' => array(
+          'description' => 'Preserve order.',
+          'type' => 'serial',
+          'unsigned' => TRUE,
+          'not null' => TRUE,
+        ),

Status:Needs review» Needs work
Issue tags:-Drupal.org D7

The last submitted patch, 1549390.patch, failed testing.

Status:Needs work» Needs review

#3: 1549390.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+Drupal.org D7

The last submitted patch, 1549390.patch, failed testing.

I think the problem is that the taxonomy update exits unexpectedly somehow and leaves the rest of the updates undone.

StatusFileSize
new6.32 KB
FAILED: [[SimpleTest]]: [MySQL] 37,564 pass(es), 1,147 fail(s), and 5 exception(s).
[ View ]

I went through this to see what is going on and what I found is that DBTNG is being strict about the query and there being null values for created, is_current, sticky. I moved the query out just so it was easier to debug which led me to the following error:

Failed: PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'created' cannot be null: INSERT INTO {taxonomy_update_7005} (vocab_id, tid, nid, vid, type, created, sticky, is_current) SELECT td.vid AS vocab_id, td.tid AS tid, tn.nid AS nid, tn.vid AS vid, n.type AS type, n2.created AS created, n2.sticky AS sticky, n2.nid AS is_current FROM {taxonomy_term_data} td INNER JOIN {taxonomy_term_node} tn ON td.tid = tn.tid INNER JOIN {node} n ON tn.nid = n.nid LEFT OUTER JOIN {node} n2 ON tn.vid = n2.vid ORDER BY tn.vid ASC, td.weight ASC, tn.tid ASC; Array ( ) in taxonomy_update_7005() (line 689 of /Users/amodi/Sites/d7_dev/modules/taxonomy/taxonomy.install).

Attaching revised patch which still fails (thus not changing status).

Status:Needs work» Needs review
StatusFileSize
new6.27 KB
PASSED: [[SimpleTest]]: [MySQL] 38,769 pass(es).
[ View ]

This patch is with created, sticky, is_current being allowed to be null values. The test still fails thought efails are now on the taxonomy checks (so atleast it goes through the upgrade). Maybe testbot will give even more info?

Great, I can try this out next time we try a fresh upgrade of Drupal.org's DB. One small thing I noticed is the "This query must return a consistent ordering across multiple calls." comment can be removed or edited; we no longer run that slow query across multiple calls, and the order is still important.

Wow...this is really strange. I had a massive number of errors popping up on my local installation so this *shouldn't* be working...I'll try and make some time to look into this over the afternoon.

I took a look to see what was going on locally and its again due to #1504994: Upgrade tests for taxonomy are failing. So this patch is actually working correctly.

Issue tags:+porting

tag

Status:Needs review» Reviewed & tested by the community

I tested this on http://7.devdrupal.org/. Plenty of other stuff is broken, but posts still have taxonomy terms, like http://7.devdrupal.org/node/1548238. And the update finished approximately 20% faster, down to 7 hr 44 min. Details about the upgrade process as a whole are at #1559652: Get update.php running smoothly on 7.devdrupal.org

Thanks for the patch. Aside from the uneasiness coming from changing an existing update function, this does seem to be OK and it's tested automatically and manually so I hope we do not open a can of worms again. Let's go with it.

All the logic should be the same, just rearranging so a slow query only needs to be done once. The riskiest part is the DBTNG conversion of $query.

Status:Reviewed & tested by the community» Fixed

A little scary, but looks good to me too. And the fact that it's been well-tested is a good thing :)

Committed to 7.x - thanks! http://drupalcode.org/project/drupal.git/commit/a80c7e8

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