Problem/Motivation

The createFieldSql() function is present in all the Schema() child class (mysql, pgsql and sqlite) and have the same doc comment, but there is not a function definition in the parent class like the dropTable(), renameTable() and other functions.

Proposed resolution

Create an abstract function definition in the parent class and update the doc comment.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#2 add_the_createfieldsql_function_definition_in_the_schema_parent_class-2864197-2.patch3.42 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
add_the_createfieldsql_function_definition_in_the_schema_parent_class-2864197-2.patch3.42 KB
markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Status: Needs review » Reviewed & tested by the community

This change makes sense in the context that you have provided as there are no other core extensions of the DatabaseSchema parent class where createFieldSql() is not present. It seems like there are three possibilities:

  1. There used to be another child class of DatabaseSchema in core that did not need to implement createFieldSql().
  2. This was a simple mistake and it should have always been defined in the parent class.
  3. This method was left out of the parent class definition intentionally so as to not hinder it's extension by code outside of core.

Assuming the answer is not option #3, then I think this change is good and the patch is RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott as a volunteer and at Chapter Three commented
Status: Reviewed & tested by the community » Closed (works as designed)

I don't think this change is correct. This method just happens to be called the same in all three concrete implementations. If we implement on the abstract class all custom/drivers db drivers have to implement this method even if their implementations don't need it. Basically nothing protected needs to be on the abstract class if it is not called from the abstract class itself.