Slow query on page creation - Patch
mehmeta - April 22, 2009 - 16:17
| Project: | Wikitools |
| Version: | 6.x-1.1 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
Just made a improvements to a couple queries which reduced the query time from ~5000ms to ~25ms for me. When handling requests, wikitools looks for a page that has the same name, this patch limits that query to the specified wiki nodetypes.
| Attachment | Size |
|---|---|
| wikitoolsPerfPatch.patch | 1.35 KB |
#1
This seems like a good idea, but it is also a minor security hole as the node type names could potentially contain SQL injection text (as I said, minor, because few people can change node type names). Please reroll the patch using db_placeholders() to escape the text properly.
Thanks!
#2
Here's the updated patch.
#3
Thanks, mehmeta!
Just one minor problem - the patch doesn't comply with Drupal's coding standards - we use two spaces for indentation rather than a tab, and leave a space between the concatenation operator (.) and any string encompassed by quotation marks ("string" or 'string' but not $string or get_string()). But just fix that up and it will be ready to commit! Thanks!
#4
Here's the updated patch, let me know if there are further changes needed, thank you.
#5
Sorry to set this back to needs work, normally I'd just fix the coding standards myself and commit this, but you want to be a maintainer so I'd like you to learn Drupal coding standards yourself for the long term:
- The indentation is off - I don't see why the first three lines should have been touched at all.
- You got the string concatenation rules backwards: you put
<?php$result = db_query("SELECT nid, type FROM {node} WHERE type IN (" .db_placeholders($node_types, 'varchar'). ") AND LOWER(title) = LOWER('%s')", $params);
?>
when you should have put
<?php$result = db_query("SELECT nid, type FROM {node} WHERE type IN (". db_placeholders($node_types, 'varchar') .") AND LOWER(title) = LOWER('%s')", $params);
?>
Again, sorry to be picky. :( When you make these two changes, however, I will give you commit access, and you'll be free to commit your patch yourself. :) I'm looking forward to having you as a co-maintainer.
#6
Oh, my bad. Sorry for making this kinda painful.
#7
Beautiful! I've granted you commit access. Thanks so much!
#8
thank you, it's committed.
#9
Automatically closed -- issue fixed for 2 weeks with no activity.