I was noticing some slowness on my Commerce installation, and thanks to xhprof was able to trace it to the absence of an index on {commerce_payment_transaction}.order_id. I don't know a whole lot about how foreign keys are supposed to work, but as far as I can tell, indexes are required on foreign key columns (at least in InnoDB) but if the Schema API example is any guide, they still have to be defined explicitly.

The attached patch adds indexes to four foreign keyed columns that lack them.

CommentFileSizeAuthor
Indexes-on-foreign-keys.patch3.17 KBserialjaywalker

Comments

serialjaywalker’s picture

Status: Active » Needs review
rszrama’s picture

Status: Needs review » Fixed

Great, thanks for doing the research on this. I added return values to the update functions so the admin will see a confirmation message after the update and will follow this up with another patch to add a missing foreign key (commerce_line_item's order_id column) and then to deal with #1388868: Add index to the commerce_customer_profile table.

Committed!

amateescu’s picture

Issue tags: +Performance

Tagging.

serialjaywalker’s picture

For what it's worth, I mentioned InnoDB's requirement of indices on foreign key columns, but this is irrelevant because Drupal doesn't actually create foreign keys in the database. I still think this fix is worthwhile for performance reasons, though.

damien tournoud’s picture

Status: Fixed » Needs work

In commerce_payment the update function adds those:

+  db_add_index('commerce_payment_transaction', 'order_id', array('order_id'));
+  db_add_index('commerce_payment_transaction', 'uid', array('uid'));

But the schema only adds the uid one?

     ),
     'indexes' => array(
       'payment_method' => array('payment_method'),
+      'uid' => array('uid'),
     ),
amateescu’s picture

Status: Needs work » Fixed

It was added by Ryan in the commit, I guess he forgot to mention it.

http://drupalcode.org/project/commerce.git/commitdiff/99252b8

rszrama’s picture

Yeah, I fixed it a bit. Sorry, should have mentioned that. ; )

serialjaywalker’s picture

Thanks for catching that, Damien and Ryan. I'll have to try not to upload patches at 4AM anymore.

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