diff --git a/core/lib/Drupal/Core/Database/Driver/pgsql/Install/Tasks.php b/core/lib/Drupal/Core/Database/Driver/pgsql/Install/Tasks.php index 1557d41..1750343 100644 --- a/core/lib/Drupal/Core/Database/Driver/pgsql/Install/Tasks.php +++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Install/Tasks.php @@ -35,6 +35,10 @@ public function __construct() { 'arguments' => array(), ); $this->tasks[] = array( + 'function' => 'checkStandardConformingStrings', + 'arguments' => array(), + ); + $this->tasks[] = array( 'function' => 'initializeDatabase', 'arguments' => array(), ); @@ -185,6 +189,56 @@ protected function checkBinaryOutputSuccess() { } /** + * Check standard_conforming_strings setting. + * + * @todo explain why we want this on. + */ + public function checkStandardConformingStrings() { + $database_connection = Database::getConnection(); + if (!$this->checkStandardConformingStringsSuccess()) { + // First try to alter the database. If it fails, raise an error telling + // the user to do it themselves. + $connection_options = $database_connection->getConnectionOptions(); + // It is safe to include the database name directly here, because this + // code is only called when a connection to the database is already + // established, thus the database name is guaranteed to be a correct + // value. + $query = "ALTER DATABASE \"" . $connection_options['database'] . "\" SET standard_conforming_strings = 'on';"; + try { + $database_connection->query($query); + } + catch (\Exception $e) { + // Ignore possible errors when the user doesn't have the necessary + // privileges to ALTER the database. + } + + // Close the database connection so that the configuration parameter + // is applied to the current connection. + Database::closeConnection(); + + // Recheck, if it fails, finally just rely on the end user to do the + // right thing. + if (!$this->checkStandardConformingStringsSuccess()) { + $replacements = array( + '%setting' => 'standard_conforming_strings', + '%current_value' => 'off', + '%needed_value' => 'on', + '!query' => "" . $query . "", + ); + $this->fail(t("The %setting setting is currently set to '%current_value', but needs to be '%needed_value'. Change this by running the following query: !query", $replacements)); + } + } + } + + /** + * Verifies the standard_conforming_strings setting. + */ + protected function checkStandardConformingStringsSuccess() { + $standard_conforming_strings = Database::getConnection()->query("SHOW standard_conforming_strings")->fetchField(); + return ($standard_conforming_strings == 'on'); + } + + /** * Make PostgreSQL Drupal friendly. */ function initializeDatabase() { diff --git a/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php b/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php index 7e1e0b7..0828d59 100644 --- a/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php +++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php @@ -726,17 +726,23 @@ public function changeField($table, $field, $field_new, $spec, $new_keys = array // Usually, we do this via a simple typecast 'USING fieldname::type'. But // the typecast does not work for conversions to bytea. // @see http://www.postgresql.org/docs/current/static/datatype-binary.html + $table_information = $this->queryTableInformation($table); + $is_bytea = !empty($table_information->blob_fields[$field]); if ($spec['pgsql_type'] != 'bytea') { - $this->connection->query('ALTER TABLE {' . $table . '} ALTER "' . $field . '" TYPE ' . $field_def . ' USING "' . $field . '"::' . $field_def); + if ($is_bytea) { + $this->connection->query('ALTER TABLE {' . $table . '} ALTER "' . $field . '" TYPE ' . $field_def . ' USING convert_from("' . $field . '"' . ", 'UTF8')"); + } + else { + $this->connection->query('ALTER TABLE {' . $table . '} ALTER "' . $field . '" TYPE ' . $field_def . ' USING "' . $field . '"::' . $field_def); + } } else { // Do not attempt to convert a field that is bytea already. - $table_information = $this->queryTableInformation($table); - if (!in_array($field, $table_information->blob_fields)) { + if (!$is_bytea) { // Convert to a bytea type by using the SQL replace() function to // convert any single backslashes in the field content to double // backslashes ('\' to '\\'). - $this->connection->query('ALTER TABLE {' . $table . '} ALTER "' . $field . '" TYPE ' . $field_def . ' USING decode(replace("' . $field . '"' . ", '\\', '\\\\'), 'escape');"); + $this->connection->query('ALTER TABLE {' . $table . '} ALTER "' . $field . '" TYPE ' . $field_def . ' USING decode(replace("' . $field . '"' . ", E'\\\\', E'\\\\\\\\'), 'escape');"); } } diff --git a/core/modules/simpletest/src/KernelTestBase.php b/core/modules/simpletest/src/KernelTestBase.php index e536ec3..eaa545c 100644 --- a/core/modules/simpletest/src/KernelTestBase.php +++ b/core/modules/simpletest/src/KernelTestBase.php @@ -225,6 +225,14 @@ protected function setUp() { // the event dispatcher which can prevent modules from registering events. \Drupal::service('config.storage')->write('core.extension', array('module' => array(), 'theme' => array())); + // Ensure database tasks have been run. + require_once __DIR__ . '/../../../includes/install.inc'; + $connection = Database::getConnection(); + $errors = db_installer_object($connection->driver())->runTasks(); + if (!empty($errors)) { + $this->fail('Failed to run installer database tasks: ' . implode(', ', $errors)); + } + // Collect and set a fixed module list. $class = get_class($this); $modules = array(); diff --git a/core/modules/system/src/Tests/Database/SchemaTest.php b/core/modules/system/src/Tests/Database/SchemaTest.php index fb5a1c2..5f60bf6 100644 --- a/core/modules/system/src/Tests/Database/SchemaTest.php +++ b/core/modules/system/src/Tests/Database/SchemaTest.php @@ -640,7 +640,7 @@ protected function assertFieldCharacteristics($table_name, $field_name, $field_s } /** - * Tests changing columns between numeric types. + * Tests changing columns between types. */ function testSchemaChangeField() { $field_specs = array( @@ -661,6 +661,27 @@ function testSchemaChangeField() { $this->assertFieldChange($old_spec, $new_spec); } } + + $field_specs = array( + array('type' => 'varchar_ascii', 'length' => '255'), + array('type' => 'varchar', 'length' => '255'), + array('type' => 'text'), + array('type' => 'blob', 'size' => 'big'), + ); + + foreach ($field_specs as $i => $old_spec) { + foreach ($field_specs as $j => $new_spec) { + if ($i === $j) { + // Do not change a field into itself. + continue; + } + // Note if the serialized data contained an object this would fail on + // Postgres. + // @see https://www.drupal.org/node/1031122 + $this->assertFieldChange($old_spec, $new_spec, serialize(['string' => "This \n has \\\\ some backslash \"*string action.\\n"])); + } + } + } /** @@ -671,7 +692,7 @@ function testSchemaChangeField() { * @param $new_spec * The ending field specification. */ - protected function assertFieldChange($old_spec, $new_spec) { + protected function assertFieldChange($old_spec, $new_spec, $test_data = NULL) { $table_name = 'test_table_' . ($this->counter++); $table_spec = array( 'fields' => array( @@ -689,9 +710,25 @@ protected function assertFieldChange($old_spec, $new_spec) { // Remove inserted rows. db_truncate($table_name)->execute(); + if ($test_data) { + $id = db_insert($table_name) + ->fields(['test_field'], [$test_data]) + ->execute(); + } + // Change the field. db_change_field($table_name, 'test_field', 'test_field', $new_spec); + if ($test_data) { + $field_value = db_select($table_name) + ->fields($table_name, ['test_field']) + ->condition('serial_column', $id) + ->execute() + ->fetchField(); + $this->pass($old_spec['type'] .' to '. $new_spec['type']); + $this->assertIdentical($field_value, $test_data); + } + // Check the field was changed. $this->assertFieldCharacteristics($table_name, 'test_field', $new_spec); diff --git a/core/modules/system/system.install b/core/modules/system/system.install index 86365a9..0170487 100644 --- a/core/modules/system/system.install +++ b/core/modules/system/system.install @@ -1183,26 +1183,6 @@ function system_update_8001(&$sandbox = NULL) { if ($schema->tableExists('menu_tree')) { if (!isset($sandbox['current'])) { - // Converting directly to blob can cause problems with reading out and - // serializing the string data later on postgres, so rename the existing - // columns and create replacement ones to hold the serialized objects. - $old_fields = array( - 'title' => array( - 'description' => 'The text displayed for the link.', - 'type' => 'varchar', - 'length' => 255, - 'not null' => TRUE, - 'default' => '', - ), - 'description' => array( - 'description' => 'The description of this link - used for admin pages and title attribute.', - 'type' => 'text', - 'not null' => FALSE, - ), - ); - foreach ($old_fields as $name => $spec) { - $schema->changeField('menu_tree', $name, 'system_update_8001_' . $name, $spec); - } $spec = array( 'description' => 'The title for the link. May be a serialized TranslationWrapper.', 'type' => 'blob', @@ -1210,7 +1190,7 @@ function system_update_8001(&$sandbox = NULL) { 'not null' => FALSE, 'serialize' => TRUE, ); - $schema->addField('menu_tree', 'title', $spec); + $schema->changeField('menu_tree', 'title', 'title', $spec); $spec = array( 'description' => 'The description of this link - used for admin pages and title attribute.', 'type' => 'blob', @@ -1218,14 +1198,14 @@ function system_update_8001(&$sandbox = NULL) { 'not null' => FALSE, 'serialize' => TRUE, ); - $schema->addField('menu_tree', 'description', $spec); + $schema->changeField('menu_tree', 'description', 'description', $spec); $sandbox['current'] = 0; $sandbox['max'] = $database->query('SELECT COUNT(mlid) FROM {menu_tree}') ->fetchField(); } - $menu_links = $database->queryRange('SELECT mlid, system_update_8001_title AS title, system_update_8001_description AS description FROM {menu_tree} ORDER BY mlid ASC', $sandbox['current'], $sandbox['current'] + 50) + $menu_links = $database->queryRange('SELECT mlid, title, description FROM {menu_tree} ORDER BY mlid ASC', $sandbox['current'], $sandbox['current'] + 50) ->fetchAllAssoc('mlid'); foreach ($menu_links as $menu_link) { @@ -1246,10 +1226,8 @@ function system_update_8001(&$sandbox = NULL) { if ($sandbox['#finished'] >= 1) { // Drop unnecessary fields from {menu_tree}. - $schema->dropField('menu_tree', 'system_update_8001_title'); $schema->dropField('menu_tree', 'title_arguments'); $schema->dropField('menu_tree', 'title_context'); - $schema->dropField('menu_tree', 'system_update_8001_description'); } return t('Menu links converted'); } diff --git a/core/tests/Drupal/KernelTests/KernelTestBase.php b/core/tests/Drupal/KernelTests/KernelTestBase.php index c560e25..a57a247 100644 --- a/core/tests/Drupal/KernelTests/KernelTestBase.php +++ b/core/tests/Drupal/KernelTests/KernelTestBase.php @@ -343,6 +343,14 @@ private function bootKernel() { // register() is only called if a new container was built/compiled. $this->container = $kernel->getContainer(); + // Ensure database tasks have been run. + require_once __DIR__ . '/../../../includes/install.inc'; + $connection = Database::getConnection(); + $errors = db_installer_object($connection->driver())->runTasks(); + if (!empty($errors)) { + $this->fail('Failed to run installer database tasks: ' . implode(', ', $errors)); + } + if ($modules) { $this->container->get('module_handler')->loadAll(); }