Avoid joins when reading MCR revisions.

When loading revision slots from the new MCR schema, slot role names
and content model names need to be resolved. Doing so programmatically
based on an in-memory cache avoids having to join against two additional
table every time we load slot meta-data.

This relies on NameTableStore, which was designed for exactly this use
case.

Bug: T198561
Change-Id: Ic005931b669f9d0173ef47ac17331d44fb1141ca

diff --git a/includes/Storage/RevisionStore.php b/includes/Storage/RevisionStore.php
index 88d520c..d219267 100644 (file)
--- a/includes/Storage/RevisionStore.php
+++ b/includes/Storage/RevisionStore.php
@@ -1547,6 +1547,10 @@ class RevisionStore
                $slots = [];
 
                foreach ( $res as $row ) {
+                       // resolve role names and model names from in-memory cache, instead of joining.
+                       $row->role_name = $this->slotRoleStore->getName( (int)$row->slot_role_id );
+                       $row->model_name = $this->contentModelStore->getName( (int)$row->content_model );
+
                        $contentCallback = function ( SlotRecord $slot ) use ( $queryFlags, $row ) {
                                return $this->loadSlotContent( $slot, null, null, null, $queryFlags );
                        };
@@ -2268,6 +2272,9 @@ class RevisionStore
         *
         * @param array $options Any combination of the following strings
         *  - 'content': Join with the content table, and select content meta-data fields
+        *  - 'model': Join with the content_models table, and select the model_name field.
+        *             Only applicable if 'content' is also set.
+        *  - 'role': Join with the slot_roles table, and select the role_name field
         *
         * @return array With three keys:
         *  - tables: (string[]) to include in the `$table` to `IDatabase->select()`
@@ -2304,26 +2311,39 @@ class RevisionStore
                        }
                } else {
                        $ret['tables'][] = 'slots';
-                       $ret['tables'][] = 'slot_roles';
                        $ret['fields'] = array_merge( $ret['fields'], [
                                'slot_revision_id',
                                'slot_content_id',
                                'slot_origin',
-                               'role_name'
+                               'slot_role_id',
                        ] );
-                       $ret['joins']['slot_roles'] = [ 'INNER JOIN', [ 'slot_role_id = role_id' ] ];
+
+                       if ( in_array( 'role', $options, true ) ) {
+                               // Use left join to attach role name, so we still find the revision row even
+                               // if the role name is missing. This triggers a more obvious failure mode.
+                               $ret['tables'][] = 'slot_roles';
+                               $ret['joins']['slot_roles'] = [ 'LEFT JOIN', [ 'slot_role_id = role_id' ] ];
+                               $ret['fields'][] = 'role_name';
+                       }
 
                        if ( in_array( 'content', $options, true ) ) {
                                $ret['tables'][] = 'content';
-                               $ret['tables'][] = 'content_models';
                                $ret['fields'] = array_merge( $ret['fields'], [
                                        'content_size',
                                        'content_sha1',
                                        'content_address',
-                                       'model_name'
+                                       'content_model',
                                ] );
                                $ret['joins']['content'] = [ 'INNER JOIN', [ 'slot_content_id = content_id' ] ];
-                               $ret['joins']['content_models'] = [ 'INNER JOIN', [ 'content_model = model_id' ] ];
+
+                               if ( in_array( 'model', $options, true ) ) {
+                                       // Use left join to attach model name, so we still find the revision row even
+                                       // if the model name is missing. This triggers a more obvious failure mode.
+                                       $ret['tables'][] = 'content_models';
+                                       $ret['joins']['content_models'] = [ 'LEFT JOIN', [ 'content_model = model_id' ] ];
+                                       $ret['fields'][] = 'model_name';
+                               }
+
                        }
                }
 

diff --git a/tests/phpunit/includes/Storage/McrReadNewRevisionStoreDbTest.php b/tests/phpunit/includes/Storage/McrReadNewRevisionStoreDbTest.php
index cbae4c7..492d00c 100644 (file)
--- a/tests/phpunit/includes/Storage/McrReadNewRevisionStoreDbTest.php
+++ b/tests/phpunit/includes/Storage/McrReadNewRevisionStoreDbTest.php
@@ -173,51 +173,84 @@ class McrReadNewRevisionStoreDbTest extends RevisionStoreDbTestBase {
        }
 
        public function provideGetSlotsQueryInfo() {
-               yield [
+               yield 'no options' => [
                        [],
+                       [
+                               'tables' => [
+                                       'slots'
+                               ],
+                               'fields' => [
+                                       'slot_revision_id',
+                                       'slot_content_id',
+                                       'slot_origin',
+                                       'slot_role_id',
+                               ],
+                               'joins' => [],
+                       ]
+               ];
+               yield 'role option' => [
+                       [ 'role' ],
                        [
                                'tables' => [
                                        'slots',
                                        'slot_roles',
                                ],
-                               'fields' => array_merge(
-                                       [
-                                               'slot_revision_id',
-                                               'slot_content_id',
-                                               'slot_origin',
-                                               'role_name',
-                                       ]
-                               ),
+                               'fields' => [
+                                       'slot_revision_id',
+                                       'slot_content_id',
+                                       'slot_origin',
+                                       'slot_role_id',
+                                       'role_name',
+                               ],
                                'joins' => [
-                                       'slot_roles' => [ 'INNER JOIN', [ 'slot_role_id = role_id' ] ],
+                                       'slot_roles' => [ 'LEFT JOIN', [ 'slot_role_id = role_id' ] ],
                                ],
                        ]
                ];
-               yield [
+               yield 'content option' => [
                        [ 'content' ],
                        [
                                'tables' => [
                                        'slots',
-                                       'slot_roles',
+                                       'content',
+                               ],
+                               'fields' => [
+                                       'slot_revision_id',
+                                       'slot_content_id',
+                                       'slot_origin',
+                                       'slot_role_id',
+                                       'content_size',
+                                       'content_sha1',
+                                       'content_address',
+                                       'content_model',
+                               ],
+                               'joins' => [
+                                       'content' => [ 'INNER JOIN', [ 'slot_content_id = content_id' ] ],
+                               ],
+                       ]
+               ];
+               yield 'content and model options' => [
+                       [ 'content', 'model' ],
+                       [
+                               'tables' => [
+                                       'slots',
                                        'content',
                                        'content_models',
                                ],
-                               'fields' => array_merge(
-                                       [
-                                               'slot_revision_id',
-                                               'slot_content_id',
-                                               'slot_origin',
-                                               'role_name',
-                                               'content_size',
-                                               'content_sha1',
-                                               'content_address',
-                                               'model_name',
-                                       ]
-                               ),
+                               'fields' => [
+                                       'slot_revision_id',
+                                       'slot_content_id',
+                                       'slot_origin',
+                                       'slot_role_id',
+                                       'content_size',
+                                       'content_sha1',
+                                       'content_address',
+                                       'content_model',
+                                       'model_name',
+                               ],
                                'joins' => [
-                                       'slot_roles' => [ 'INNER JOIN', [ 'slot_role_id = role_id' ] ],
                                        'content' => [ 'INNER JOIN', [ 'slot_content_id = content_id' ] ],
-                                       'content_models' => [ 'INNER JOIN', [ 'content_model = model_id' ] ],
+                                       'content_models' => [ 'LEFT JOIN', [ 'content_model = model_id' ] ],
                                ],
                        ]
                ];

diff --git a/tests/phpunit/includes/Storage/McrRevisionStoreDbTest.php b/tests/phpunit/includes/Storage/McrRevisionStoreDbTest.php
index 649e692..af19f72 100644 (file)
--- a/tests/phpunit/includes/Storage/McrRevisionStoreDbTest.php
+++ b/tests/phpunit/includes/Storage/McrRevisionStoreDbTest.php
@@ -190,51 +190,84 @@ class McrRevisionStoreDbTest extends RevisionStoreDbTestBase {
        }
 
        public function provideGetSlotsQueryInfo() {
-               yield [
+               yield 'no options' => [
                        [],
+                       [
+                               'tables' => [
+                                       'slots'
+                               ],
+                               'fields' => [
+                                       'slot_revision_id',
+                                       'slot_content_id',
+                                       'slot_origin',
+                                       'slot_role_id',
+                               ],
+                               'joins' => [],
+                       ]
+               ];
+               yield 'role option' => [
+                       [ 'role' ],
                        [
                                'tables' => [
                                        'slots',
                                        'slot_roles',
                                ],
-                               'fields' => array_merge(
-                                       [
-                                               'slot_revision_id',
-                                               'slot_content_id',
-                                               'slot_origin',
-                                               'role_name',
-                                       ]
-                               ),
+                               'fields' => [
+                                       'slot_revision_id',
+                                       'slot_content_id',
+                                       'slot_origin',
+                                       'slot_role_id',
+                                       'role_name',
+                               ],
                                'joins' => [
-                                       'slot_roles' => [ 'INNER JOIN', [ 'slot_role_id = role_id' ] ],
+                                       'slot_roles' => [ 'LEFT JOIN', [ 'slot_role_id = role_id' ] ],
                                ],
                        ]
                ];
-               yield [
+               yield 'content option' => [
                        [ 'content' ],
                        [
                                'tables' => [
                                        'slots',
-                                       'slot_roles',
+                                       'content',
+                               ],
+                               'fields' => [
+                                       'slot_revision_id',
+                                       'slot_content_id',
+                                       'slot_origin',
+                                       'slot_role_id',
+                                       'content_size',
+                                       'content_sha1',
+                                       'content_address',
+                                       'content_model',
+                               ],
+                               'joins' => [
+                                       'content' => [ 'INNER JOIN', [ 'slot_content_id = content_id' ] ],
+                               ],
+                       ]
+               ];
+               yield 'content and model options' => [
+                       [ 'content', 'model' ],
+                       [
+                               'tables' => [
+                                       'slots',
                                        'content',
                                        'content_models',
                                ],
-                               'fields' => array_merge(
-                                       [
-                                               'slot_revision_id',
-                                               'slot_content_id',
-                                               'slot_origin',
-                                               'role_name',
-                                               'content_size',
-                                               'content_sha1',
-                                               'content_address',
-                                               'model_name',
-                                       ]
-                               ),
+                               'fields' => [
+                                       'slot_revision_id',
+                                       'slot_content_id',
+                                       'slot_origin',
+                                       'slot_role_id',
+                                       'content_size',
+                                       'content_sha1',
+                                       'content_address',
+                                       'content_model',
+                                       'model_name',
+                               ],
                                'joins' => [
-                                       'slot_roles' => [ 'INNER JOIN', [ 'slot_role_id = role_id' ] ],
                                        'content' => [ 'INNER JOIN', [ 'slot_content_id = content_id' ] ],
-                                       'content_models' => [ 'INNER JOIN', [ 'content_model = model_id' ] ],
+                                       'content_models' => [ 'LEFT JOIN', [ 'content_model = model_id' ] ],
                                ],
                        ]
                ];