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

CommentFileSizeAuthor
#18 d7-backport-1827136-18.patch629 bytesposulliv
#7 severity_dbtng-1827136-7.patch702 bytespatrickd
#3 dblog-AddIndex-1827136-3.patch701 bytesamatzies
#1 dblog-AddIndex-1827136-1.patch376 bytesamatzies

Comments

amatzies’s picture

amatzies commented
StatusFileSize
new dblog-AddIndex-1827136-1.patch376 bytes

This patch adds the index.

Anonymous’s picture

Anonymous (not verified) commented
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

amatzies commented
StatusFileSize
new dblog-AddIndex-1827136-3.patch701 bytes

Thanks for the feedback. I added the hook_update_N now.

Anonymous’s picture

Anonymous (not verified) commented
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

patrickd commented
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

Anonymous (not verified) commented
Status: Reviewed & tested by the community » Needs work

Then let us be nit-picky.

patrickd’s picture

patrickd commented
Status: Needs work » Needs review
StatusFileSize
new severity_dbtng-1827136-7.patch702 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

Anonymous (not verified) commented
Issue tags: +Novice

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

Anonymous’s picture

Anonymous (not verified) commented
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

patrickd commented
Status: Needs work » Needs review

#7 now it's green, weird bot

Anonymous’s picture

Anonymous (not verified) commented
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

catch
he/him
Primary language English
commented
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

Anonymous (not verified) commented

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

Anonymous (not verified) commented
Status: Needs review » Reviewed & tested by the community
catch’s picture

catch
he/him
Primary language English
commented
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

posulliv commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new d7-backport-1827136-18.patch629 bytes

Patch to backport to D7.

droplet’s picture

droplet commented
Status: Needs review » Reviewed & tested by the community

Patch looks good.

David_Rothstein’s picture

David_Rothstein commented
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.