Because {node}.language is used in the WHERE clause to generate the pager list on ?q=admin/content, the lack of index on the language column makes the query slower than it needs to be.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cafuego’s picture

FileSize
885 bytes

Attached patch adds an index in node.install and implements node_update_7011() to add the index on existing installations. See #778050: Add support for database hints and make PagerDefault properly pluggeable for a link to query speed tests.

cafuego’s picture

Status: Active » Needs review
brianV’s picture

Version: 7.x-dev » 8.x-dev
Issue tags: +Perfomance
FileSize
878 bytes
469 bytes

Patch attached for D8, and a reroll for D7.

Schema change only in D8, schema change and hook_update_n() in D7.

larowlan’s picture

Status: Needs review » Needs work

We also need an update hook to add the index for existing installs.

brianV’s picture

Status: Needs work » Needs review

There are two patches in #3. One for D8 which is just the schema change, and one for D7 that has schema change and update hook.

No update hook is needed in D8 since HEAD->HEAD upgrades are currently not supported.

YesCT’s picture

Issue tags: -Perfomance

Status: Needs review » Needs work
Issue tags: +Perfomance

The last submitted patch, 813634-add-node-language-index-D7.patch, failed testing.

claudinec’s picture

Rerolled new patch for D8 schema change.

claudinec’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 813634-add-node-language-index-8.patch, failed testing.

jlscott’s picture

Status: Needs work » Needs review
FileSize
489 bytes

The previous patch used the column name "language" instead of "langcode". Patch has been rerolled to include the correct column name.

cafuego’s picture

Looks good to me.

YesCT’s picture

Issue tags: +D8MI

maybe someone from d8mi can chime in here to rtbc it.

I dont see anything wrong, but I'm not knowledgable about performance and such.

YesCT’s picture

tag didn't stick.

Deciphered’s picture

If at all possible, can we get an interdiff for this to make the review process a little easier?

jlscott’s picture

Sorry for the delay, but I have now rerolled the patch and created an interdiff as requested.

This patch is so simple that the interdiff is actually larger than the patch :)

jair’s picture

Issue tags: +Needs reroll

Needs reroll

Nitesh Sethia’s picture

Assigned: Unassigned » Nitesh Sethia
FileSize
1.41 KB

Rerolling the patch #16

Status: Needs review » Needs work

The last submitted patch, add-node-language-index-813634-18.patch, failed testing.

wesleydv’s picture

Status: Needs work » Closed (won't fix)

Investigated this and the patch is no longer needed because current D8 head already has:

'node_langcode' => array('langcode'),

So I believe this patch is no longer needed.

Sutharsan’s picture

I agree, 'language' already has an index.. A bit more verbose:

In node.install you find:
$schema['node_field_revision'] = array(
'description' => 'Stores information about each saved version of a {node}.',
'fields' => array(
...
'indexes' => array(
'nid' => array('nid'),
'uid' => array('uid'),
'revision_uid' => array('revision_uid'),
'vid' => array('vid'),
'node_default_langcode' => array('default_langcode'),
'node_langcode' => array('langcode'),
),
...
'primary key' => array('nid', 'vid', 'langcode'),
);

cafuego’s picture

Version: 8.x-dev » 7.x-dev
Assigned: Nitesh Sethia » Unassigned
Status: Closed (won't fix) » Needs review
Issue tags: -D8MI, -Needs reroll
FileSize
720 bytes

Re-opening for 7.x-dev because it's still not fixed in D7, which is where I wanted it fixed to begin with :-P Re-attached an updated for-git version of the original patch from comment 1.

wesleydv’s picture

Status: Needs review » Reviewed & tested by the community

Looks ok to me

Anonymous’s picture

Assigned: Unassigned » David_Rothstein

this is for David to look at now.

David_Rothstein’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Fixed
David_Rothstein’s picture

Assigned: David_Rothstein » Unassigned

Status: Fixed » Closed (fixed)

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