Problem/Motivation

If you see the first lines of the Schema::dropTable() method you will see that if there is not table no exception is thrown.

    if (!$this->tableExists($table)) {
      return FALSE;
    }

Proposed resolution

Throw an exception like in the Schema::addField() method

    if (!$this->tableExists($table)) {
      throw new SchemaObjectDoesNotExistException(t("Cannot add field @table.@field: table doesn't exist.", ['@field' => $field, '@table' => $table]));
    }

But with the correct message.

Remaining tasks

Test the patch.

User interface changes

None.

API changes

Update the doc with the throw exception info.

Data model changes

None.

CommentFileSizeAuthor
#8 throw_an_exception_if_the_table_doesnt_exist_in_schema_droptable-2862340-8.patch2.81 KBadriancid
#2 throw_an_exception_if_the_table_doesnt_exist_in_schema_droptable-2862340-2.patch2.75 KBadriancid
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

adriancid created an issue. See original summary.

adriancid’s picture

adriancid
Primary language Spanish
Location Montreal, Canada
CreditAttribution: adriancid as a volunteer and commented
Status: Active » Needs review
FileSize
throw_an_exception_if_the_table_doesnt_exist_in_schema_droptable-2862340-2.patch2.75 KB

Status: Needs review » Needs work

The last submitted patch, 2: throw_an_exception_if_the_table_doesnt_exist_in_schema_droptable-2862340-2.patch, failed testing.

daffie’s picture

daffie CreditAttribution: daffie commented
Version: 8.3.x-dev » 9.x-dev

I like this change. All other methods in Drupal\Core\Database\Schema that test if the table exists will throw the exception: SchemaObjectDoesNotExistException. The only exception is the method dropTable(). The problem is that such a change will be bc-break and that will be only allowed in a new major version of Drupal (Drupal 9.0). For more information see https://www.drupal.org/core/d8-allowed-changes.

adriancid’s picture

adriancid
Primary language Spanish
Location Montreal, Canada
CreditAttribution: adriancid as a volunteer and commented

@daffie why do you think that this will be bc-break? I come from read the link and this is the definition:

Backward-compatibility (BC) break (#)
A backward-compatibility break is a type of API change that requires changes in code implementing a public API. This can be anything from changing the parameters of a procedural function to renaming an interface or removing a hook. See [policy, then meta] Document @internal APIs both explicitly in phpdoc and implicitly in d.o documentation for ongoing discussion of what is a public API vs. internal API.

Maybe I can't get the point, can you help me to understand please?

adriancid’s picture

adriancid
Primary language Spanish
Location Montreal, Canada
CreditAttribution: adriancid as a volunteer and commented
Issue summary: View changes
daffie’s picture

daffie CreditAttribution: daffie commented 
  $schema->dropTable('test_table');

If someone runs this code in his/her contrib module and they try to delete a table that does not exists the dropTable() method would return FALSE. The rest of their code will still run. If we make the change that you are suggesting the dropTable() method will throw an exception and if uncaught will stop the running of the code. This is a major change and not allowed.

adriancid’s picture

adriancid
Primary language Spanish
Location Montreal, Canada
CreditAttribution: adriancid as a volunteer and commented
Status: Needs work » Needs review
FileSize
throw_an_exception_if_the_table_doesnt_exist_in_schema_droptable-2862340-8.patch2.81 KB

Updated the message according to the others exception messages.

adriancid’s picture

adriancid
Primary language Spanish
Location Montreal, Canada
CreditAttribution: adriancid as a volunteer and commented

Version: 9.x-dev » 9.0.x-dev

The 9.0.x branch will open for development soon, and the placeholder 9.x branch should no longer be used. Only issues that require a new major version should be filed against 9.0.x (for example, removing deprecated code or updating dependency major versions). New developments and disruptive changes that are allowed in a minor version should be filed against 8.9.x, and significant new features will be moved to 9.1.x at committer discretion. For more information see the Allowed changes during the Drupal 8 and 9 release cycles and the Drupal 9.0.0 release plan.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Title: Throw an exception if the table doesn't exist in Schema::dropTable » Throw an exception if the table doesn't exist in Schema::dropTable()
Version: 9.0.x-dev » 9.1.x-dev
daffie’s picture

daffie CreditAttribution: daffie commented
Status: Needs review » Needs work
Issue tags: +Needs tests
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented

See also #2862667-22: Throw an exception if the field doesn't exist in Schema::dropField() comments 22 and 23; we need to find a BC way to do this in the continuous upgrade path, because calling code could be correctly handling the scenario where the return value is FALSE.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented

Also, the exception messages should not be translated.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.