Problem/Motivation

      }
      else {
        new \Exception("Unable to parse the column type " . $row->type);
      }

in \Drupal\Core\Database\Driver\sqlite\Schema should throw the exception.

Proposed resolution

Throw the exception and test it.

Remaining tasks

User interface changes

API changes

Data model changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner created an issue. See original summary.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dagmar’s picture

Here is the patch. Do we have a place to put tests for sqlite?

dawehner’s picture

Nice catch! I guess there is no real way to test that?

dagmar’s picture

Well, there are no tests for sqlite on core/tests/Drupal/Tests/Core/Database/Driver... An also test this implies create a broken schema, right? So I don't know.

Status: Needs review » Needs work

The last submitted patch, 3: sqlite-throw-exception-2667574-3.patch, failed testing.

dagmar’s picture

Status: Needs work » Needs review
daffie’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

If there is no way to test the patch from this issue, as @dawehner is suggesting, then the patch is for me RTBC.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 74a80df and pushed to 8.1.x and 8.2.x. Thanks!

@dagmar it is not that there are no tests. We run ALL the database tests against all databases. However given that this exception would occur when we can't map back from an sqlite field this is going to be very hard to test given all affinities are covered... https://www.sqlite.org/datatype3.html.

diff --git a/core/lib/Drupal/Core/Database/Driver/sqlite/Schema.php b/core/lib/Drupal/Core/Database/Driver/sqlite/Schema.php
index b536635..a3c001c 100644
--- a/core/lib/Drupal/Core/Database/Driver/sqlite/Schema.php
+++ b/core/lib/Drupal/Core/Database/Driver/sqlite/Schema.php
@@ -420,10 +420,12 @@ protected function alterTable($table, $old_schema, $new_schema, array $mapping =
    *
    * @param $table
    *   Name of the table.
-   * @throws \Exception
-   *   If a column of the table could not be parsed.
+   *
    * @return
    *   An array representing the schema, from drupal_get_schema().
+   *
+   * @throws \Exception
+   *   If a column of the table could not be parsed.
    */
   protected function introspectSchema($table) {
     $mapped_fields = array_flip($this->getFieldTypeMap());

Fixed this coding standard on commit.

  • alexpott committed 472f3a5 on 8.2.x
    Issue #2667574 by dagmar: sqlite/Schema::introspectSchema should throw...

  • alexpott committed 74a80df on 8.1.x
    Issue #2667574 by dagmar: sqlite/Schema::introspectSchema should throw...

Status: Fixed » Closed (fixed)

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