Since we provide the means to filter on severity the data column needs to be an index to prevent a whole table read.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

amatzies’s picture

FileSize
376 bytes

This patch adds the index.

Anonymous’s picture

Status: Active » Needs work

While the patch adds the index for new installations you didn't supply a hook_update_N to modify the existing tables.

amatzies’s picture

FileSize
701 bytes

Thanks for the feedback. I added the hook_update_N now.

Anonymous’s picture

Status: Needs work » Needs review

You need to set the status to "needs review" when adding a patch for the testbot to see it.

patrickd’s picture

Status: Needs review » Reviewed & tested by the community

works for me (there's a missing dot at the end of the comment though)

Anonymous’s picture

Status: Reviewed & tested by the community » Needs work

Then let us be nit-picky.

patrickd’s picture

Status: Needs work » Needs review
FileSize
702 bytes

same as #3, but with dot

Status: Needs review » Needs work

The last submitted patch, severity_dbtng-1827136-7.patch, failed testing.

Anonymous’s picture

Issue tags: +Novice

I think the testbot had a hiccup in its get-up-and-go.

Anonymous’s picture

Status: Needs work » Needs review
Issue tags: -Novice

#7: severity_dbtng-1827136-7.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, severity_dbtng-1827136-7.patch, failed testing.

patrickd’s picture

Status: Needs work » Needs review

#7 now it's green, weird bot

Anonymous’s picture

Status: Needs review » Reviewed & tested by the community

Yea, the testbot had an issue and the was requeued by the testbot admin. Unfortunately that doesn't reset the Status.

catch’s picture

Status: Reviewed & tested by the community » Needs review

Please post a before/after EXPLAIN on the query this index affects.

Since this needs backport to Drupal 7, I think we can actually skip adding the update to Drupal 8 (as long as it does actually get backported).

Anonymous’s picture

Without:

id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
1	SIMPLE	w	index	NULL	PRIMARY	4	NULL	11420	Using where
1	SIMPLE	u	eq_ref	PRIMARY	PRIMARY	4	d7port.w.uid	1	 

With:

id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
1	SIMPLE	w	ref	severity	severity	1	const	124	Using where; Using filesort
1	SIMPLE	u	eq_ref	PRIMARY	PRIMARY	4	d7port.w.uid	1	 
Anonymous’s picture

Status: Needs review » Reviewed & tested by the community
catch’s picture

Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)
Issue tags: +Needs backport to D7

Thanks!

I removed the update function from the patch and committed/pushed this one to 8.x. Moving to 7.x for backport. Tagging with 'revisit before release' so this doesn't get lost if it's never committed to 7.x for any reason.

posulliv’s picture

Status: Patch (to be ported) » Needs review
FileSize
629 bytes

Patch to backport to D7.

droplet’s picture

Status: Needs review » Reviewed & tested by the community

Patch looks good.

David_Rothstein’s picture

Status: Reviewed & tested by the community » Fixed

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

(Note that I fixed a couple small documentation issues on commit: The function was in the wrong docgroup, since this is not part of the Drupal 6-to-7 upgrade, and the grammar on the function PHPDoc needed a little work.)

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