Problem/Motivation
This code in Sql::ensureTables()
try {
$this->getDatabase()
->schema()
->createTable($this->mapTableName, $schema);
break;
}
catch (DatabaseException $e) {
$pdo_exception = $e->getPrevious();
$mysql_index_error = $pdo_exception instanceof \PDOException && $pdo_exception->getCode() === '42000' && $pdo_exception->errorInfo[1] === 1071;
$chunk_size--;
// Rethrow the exception if the source IDs can not be in smaller
// groups.
if (!$mysql_index_error || $chunk_size <= 0) {
throw $e;
}
}
assumes the DB driver is one implementing a PDO connection.
This blocks #3259709: Create the database driver for MySQLi that would be a non-PDO implementation.
Proposed resolution
Make that code db-driver abstract.
Implement two specific exceptions inheriting from SchemaException to cover the case of column too large and key too large, and throw them from mysql driver's ExceptionHandler. Change migrate's Sql to catch the new exceptions accordingly.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comments
Comment #2
andypostComment #4
mondrakeAdded a new exception (database independent), and changed Sql to use Connection::exceptionHandler that throws the new exception.
Comment #5
mondrakeI'd like to add
@group Databasetests for the new exceptions.Comment #6
mondrakeAdded tests to mysql's SchemaTest
Comment #7
daffie commentedWe need a change record for the new exceptions. Also the IS needs to be updated for the new exceptions.
Comment #8
mondrakeUpdated IS
Comment #9
mondrakeCR drafted, https://www.drupal.org/node/3389316
Comment #10
daffie commentedCan we add testing for PostgreSQL and SQLite.
The maximum character length for a text field in PostgreSQL is 10485760. See: https://www.postgresql.org/docs/current/datatype-character.html
The maximum character length for a text field in SQLite is 1 billion. See: https://www.sqlite.org/limits.html
The maximum number of columns in an index in PostgreSQL is 32. See: https://www.postgresql.org/docs/current/limits.html
The maximum number of columns in an index in SQLite is 2000. See: https://www.sqlite.org/limits.html
Comment #11
mondrakeI suggest to do #10 in a follow up.
Comment #12
andypostI bet it just needs to extend existing tests for pgsql and sqlite like it was done for mysql
Comment #13
mondrakeWell not extend, they have to be written specifically for each database since each database has different limitations, see #10.
One may argue that a test creating a table with over 2000 columns (sqlite) to trigger the failure is overkill. That’s why I suggest to do that in separate issues where each database tests can be discussed separately.
The blocker tag is only relevant for mysqli implementation.
Comment #14
andypost@mondrake Thanks for explanation, it makes sense!
Moreover there's existing follow-up #3082239: Forbid limited length primary and unique keys, allow only in indexes
Comment #15
daffie commentedAll code changes look good.
The IS and the CR are in order.
The testing for PostgreSQL and SQLite will be done in a followup.
For me it is RTBC.
Comment #16
mondrakeFiled followups for PostgreSQL and SQLite.
Comment #17
catchVery tidy, just adding a more explicit exception instead of relying on the exception message.
Committed df59482 and pushed to 11.x. Thanks!
Comment #19
catch