Unverified Commit 2c71d8ec authored by jibran's avatar jibran

Issue #2927218 by jhedstrom, nick.james, jibran: No index is added for the target_id_int column

parent e18bdbf2
......@@ -123,3 +123,68 @@ function dynamic_entity_reference_update_8201() {
}
}
/**
* Create index on all target_id_int fields.
*
* By creating the index, joins on this column will be speed up significantly.
*
* @return string
*/
function dynamic_entity_reference_update_8202() {
$schema = \Drupal::database()->schema();
$entity_field_manager = \Drupal::service('entity_field.manager');
$entity_type_manager = \Drupal::entityTypeManager();
// The _int column spec needed for creating the index.
$column_spec = [
'type' => 'int',
'unsigned' => TRUE,
'not null' => FALSE,
];
// Only update dynamic_entity_reference fields.
$return = [];
foreach ($entity_field_manager->getFieldMapByFieldType('dynamic_entity_reference') as $entity_type_id => $map) {
$entity_storage = $entity_type_manager->getStorage($entity_type_id);
// Only SQL storage based entities are supported.
if ($entity_storage instanceof SqlEntityStorageInterface) {
$field_storage_definitions = $entity_field_manager->getFieldStorageDefinitions($entity_type_id);
/** @var \Drupal\Core\Entity\Sql\DefaultTableMapping $table_mapping */
$table_mapping = $entity_storage->getTableMapping($field_storage_definitions);
// Only need field storage definitions of dynamic_entity_reference fields.
/** @var \Drupal\Core\Field\FieldStorageDefinitionInterface $field_storage_definition */
foreach (array_intersect_key($field_storage_definitions, $map) as $field_storage_definition) {
$field_name = $field_storage_definition->getName();
try {
$table = $table_mapping->getFieldTableName($field_name);
$column = $table_mapping->getFieldColumnName($field_storage_definition, 'target_id_int');
$index_column = $table_mapping->getFieldColumnName($field_storage_definition, 'target_type');
}
catch (SqlContentEntityStorageException $e) {
// Custom storage? Broken site? No matter what, if there is no table
// or column, there's little we can do.
continue;
}
$schema_info = $field_storage_definition->getSchema();
if ($schema->fieldExists($table, $column)) {
$spec = [
'fields' => [
$column => $column_spec,
$index_column => $schema_info['columns']['target_type'],
],
];
if (!$schema->indexExists($table, $column)) {
$schema->addIndex($table, $column, [$column, $index_column], $spec);
$args = [
':fields' => implode(', ', [$column, $index_column]),
':table' => $table,
];
$return[] = t('Added index on ":fields" fields from ":table" table.', $args);
}
}
}
}
}
return implode("\n", $return);
}
......@@ -128,6 +128,7 @@ class FieldStorageSubscriber implements EventSubscriberInterface {
}
$entity_type = $this->entityTypeManager->getDefinition($entity_type_id);
$tables = [];
$index_columns = [];
// If we know which field is being created / updated check whether it is
// DER.
if ($storage instanceof SqlEntityStorageInterface && !empty($der_fields[$entity_type_id])) {
......@@ -141,6 +142,7 @@ class FieldStorageSubscriber implements EventSubscriberInterface {
try {
$table = $mapping->getFieldTableName($field_name);
$column = $mapping->getFieldColumnName($storage_definitions[$field_name], 'target_id');
$index_column = $mapping->getFieldColumnName($storage_definitions[$field_name], 'target_type');
}
catch (SqlContentEntityStorageException $e) {
// Custom storage? Broken site? No matter what, if there is no table
......@@ -148,15 +150,26 @@ class FieldStorageSubscriber implements EventSubscriberInterface {
continue;
}
$tables[$table][] = $column;
$schema_info = $storage_definitions[$field_name]->getSchema();
$index_columns[$table] = [
$index_column => $schema_info['columns']['target_type'],
];
if ($entity_type->isRevisionable() && ($storage_definitions[$field_name]->isRevisionable())) {
try {
if ($mapping->requiresDedicatedTableStorage($storage_definitions[$field_name])) {
$tables[$mapping->getDedicatedRevisionTableName($storage_definitions[$field_name])][] = $column;
$index_columns[$mapping->getDedicatedRevisionTableName($storage_definitions[$field_name])] = [
$index_column => $schema_info['columns']['target_type'],
];
}
elseif ($mapping->allowsSharedTableStorage($storage_definitions[$field_name])) {
$revision_table = $entity_type->getRevisionDataTable() ?: $entity_type->getRevisionTable();
$tables[$revision_table][] = $column;
$tables[$revision_table] = array_unique($tables[$revision_table]);
$index_columns[$revision_table] = [
$index_column => $schema_info['columns']['target_type'],
];
}
}
catch (SqlContentEntityStorageException $e) {
......@@ -166,7 +179,7 @@ class FieldStorageSubscriber implements EventSubscriberInterface {
}
$new = [];
foreach ($tables as $table => $columns) {
$new[$table] = $this->intColumnHandler->create($table, $columns);
$new[$table] = $this->intColumnHandler->create($table, $columns, $index_columns[$table]);
}
foreach (array_filter($new) as $table => $columns) {
// reset($columns) is one of the new int columns. The trigger will fill
......
......@@ -56,7 +56,7 @@ abstract class IntColumnHandler implements IntColumnHandlerInterface {
/**
* {@inheritdoc}
*/
public function create($table, array $columns) {
public function create($table, array $columns, array $index_columns = []) {
$schema = $this->connection->schema();
// The integer column specification.
$spec = [
......@@ -78,8 +78,21 @@ abstract class IntColumnHandler implements IntColumnHandlerInterface {
$column_int = $column . '_int';
// Make sure the integer columns exist.
if (!$schema->fieldExists($table, $column_int)) {
$index_fields = [$column_int];
$full_spec = [
'fields' => [
$column_int => $spec,
],
];
if (!empty($index_columns)) {
$full_spec['fields'] = array_merge($full_spec['fields'], $index_columns);
$index_fields = array_merge($index_fields, array_keys($index_columns));
}
$new[] = $column_int;
$schema->addField($table, $column_int, $spec);
$schema->addIndex($table, $column_int, $index_fields, $full_spec);
}
// This is the heart of this function: before an UPDATE/INSERT, set the
// value of the integer column to the integer value of the string column.
......
......@@ -14,10 +14,13 @@ interface IntColumnHandlerInterface {
* The non-prefix table to operate on.
* @param array $columns
* The DER target_id columns.
* @param array $index_columns
* Table columns that should be added to the index that is created for the
* new _int column.
*
* @return array
* The list of new target_id_int columns.
*/
public function create($table, array $columns);
public function create($table, array $columns, array $index_columns = []);
}
......@@ -29,7 +29,7 @@ class IntColumnHandlerPostgreSQL implements IntColumnHandlerInterface {
/**
* {@inheritdoc}
*/
public function create($table, array $columns) {
public function create($table, array $columns, array $index_columns = []) {
$schema = $this->connection->schema();
if (!IntColumnHandler::allColumnsExist($schema, $table, $columns)) {
return [];
......@@ -47,7 +47,19 @@ class IntColumnHandlerPostgreSQL implements IntColumnHandlerInterface {
if (!$schema->fieldExists($table, $column_int)) {
$this->createTriggerFunction($table, $column, $column_int);
$this->createTrigger($table, $column, $column_int);
$index_fields = [$column_int];
$full_spec = [
'fields' => [
$column_int => $spec,
],
];
if (!empty($index_columns)) {
$full_spec['fields'] = array_merge($full_spec['fields'], $index_columns);
$index_fields = array_merge($index_fields, array_keys($index_columns));
}
$schema->addField($table, $column_int, $spec);
$schema->addIndex($table, $column_int, $index_fields, $full_spec);
$new[] = $column_int;
}
}
......
<?php
namespace Drupal\Tests\dynamic_entity_reference\Functional\Update;
use Drupal\FunctionalTests\Update\UpdatePathTestBase;
/**
* Tests DER update path from 8201 and on.
*
* @group dynamic_entity_reference
*/
class DerUpdate8202Test extends UpdatePathTestBase {
/**
* {@inheritdoc}
*/
protected $installProfile = 'testing';
/**
* {@inheritdoc}
*/
protected function setDatabaseDumpFiles() {
$this->databaseDumpFiles = [
__DIR__ . '/../../../fixtures/update/update_test_8202.php.gz',
];
}
/**
* Test that the _int column indexes are properly created.
*
* @see \dynamic_entity_reference_update_8202()
*/
public function testUpdate8202() {
// The index should not exist initially.
$schema = \Drupal::database()->schema();
$index_mapping = [
// Table => index name.
'entity_test__field_test' => 'field_test_target_id_int',
'entity_test_mul__field_test_mul' => 'field_test_mul_target_id_int',
];
foreach ($index_mapping as $table => $index_name) {
$this->assertFalse($schema->indexExists($table, $index_name));
}
// Run updates and verify the indexes have been created.
$this->runUpdates();
foreach ($index_mapping as $table => $index_name) {
$this->assertTrue($schema->indexExists($table, $index_name));
}
}
}
......@@ -111,6 +111,13 @@ class DerUpdateTest extends UpdatePathTestBase {
// Check the both columns of configurable fields values.
$this->assertEquals([1, 1, 1, 1, 0, 1, 1], $connection->query('SELECT field_test_mul_target_id_int FROM {entity_test_mul__field_test_mul} ORDER BY entity_id, delta')->fetchCol());
$this->assertEquals([1, 1, 1, 1, 'test', 1, 1], $connection->query('SELECT field_test_mul_target_id FROM {entity_test_mul__field_test_mul} ORDER BY entity_id, delta')->fetchCol());
// Even though this was fixed after this update hook, since this one adds
// the _int column, the index is created at that time.
// @see \Drupal\Tests\dynamic_entity_reference\Functional\Update\DerUpdate8202
$this->assertTrue(\Drupal::database()->schema()->indexExists('entity_test', 'dynamic_references__target_id_int'));
$this->assertTrue(\Drupal::database()->schema()->indexExists('entity_test__field_test', 'field_test_target_id_int'));
$this->assertTrue(\Drupal::database()->schema()->indexExists('entity_test_mul__field_test_mul', 'field_test_mul_target_id_int'));
}
}
......@@ -295,6 +295,9 @@ class DynamicEntityReferenceBaseFieldTest extends EntityKernelTestBase {
$int_column = $database->query('SELECT der__target_id_int FROM {entity_test_mul_property_data}')->fetchCol();
$str_column = $database->query('SELECT der__target_id FROM {entity_test_mul_property_data}')->fetchCol();
$this->assertSame($int_column, $str_column);
// Verify an index is created on the _int columns.
$this->assertTrue(\Drupal::database()->schema()->indexExists('entity_test', 'dynamic_references__target_id_int'));
}
/**
......
......@@ -206,6 +206,9 @@ class DynamicEntityReferenceFieldTest extends EntityKernelTestBase {
* Tests the multiple target entities loader.
*/
public function testReferencedEntitiesMultipleLoad() {
// Verify an index is created on the _int column.
$this->assertTrue(\Drupal::database()->schema()->indexExists('entity_test__field_test', 'field_test_target_id_int'));
$entity_type_manager = \Drupal::entityTypeManager();
// Create the parent entity.
$entity = $entity_type_manager
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment