Make ROW_FORMAT configurable

Could you please describe what the problem really is and how to easily trigger it?

I think Dynamic is already the default row format for InnoDB: https://dev.mysql.com/doc/refman/8.0/en/innodb-row-format-specification....

I don't know when it was introduced as default though so there might be a need for specifying it explicitly with older MySQLs.

Hard coding dynamic doesn't seem very flexible though.

As an example I have some sites where we need to cut down on disk usage and therefor use row format compressed (depends on file format Barracuda).

I suggest making it an option in the database settings similar to collation:

  $databases['default']['default'] = array (
    'database' => 'databasename',
    'username' => 'sqlusername',
    'password' => 'sqlpassword',
    'host' => 'localhost',
    'port' => '3306',
    'driver' => 'mysql',
    'prefix' => '',
    'collation' => 'utf8mb4_general_ci',
    'row_format' => 'compressed',
  );

I have made a patch doing that.

The patch lacks a test case. I'm still researching how to test this (there is no similar test for collation unfortunately) -- ideas and pointers would be welcome.

I have looked into writing a test for this and I can't figure out how to do that. I haven't found a way to manipulate database settings for a test (it is possible to I haven't looked hard enough). The feature is very similar to the collation feature and that feature isn't covered by a test either.

Updated patch where the new option is documented in default.settings.php.

For those using composer-patches, patch in #7 is the one to include...

Re-rolled patch to 8.8.x.

We're doing the equivalent in D7 because our DBAs insisted on using the compressed row format. Our db servers are I/O bound (as they usually are) so the smaller files are well worth the extra processing time. The patch looks about the same but I can backport it once this is accepted in D8 (or D9?). For now I've submitted a patch to utf8mb4-convert to handle this case: #3088343: Multiple patches: include additional tables, use compressed row format, use utf8mb4_bin only where needed

Rerolled for D9.4

Just fixing the fail test and add some more documentation here, it would be nice to have other thoughts regarding the tests and the solution.

Uploading a version of #22 that can be applied via composer patches.

At least until #3075954: Remove duplicate scaffold files is not applied. If that happens, then this is the patch to use.

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Rerolled for 10.1.x and improved comments a bit.

The one error is because of the change to default.settings.-php which makes CI fail on comparing it to duplicated default.settings.-php in Scaffold. See #3075954.

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

This needs an issue summary update as we are adding something to the database layer. What's the proposed solution to tackle this issue? Remaining tasks?

This will need test coverage as well

Once everything is agreed a change record will be needed to announce this change.

• Added an issue branch / merge request.
• Updated the issue summary.

Updated issue summary.

Fixed failing test case.

Posted in #testing for testing and @FeyP made the suggestion

Not sure about the settings file, maybe there are some tests in core to check that too. But even if not, I don't think it's untestable. Just from the top of my head, absolutely untested: You could start by getting the currently active connection info with Drupal\Core\Database\Database::getConnectionInfo(). Then check, if you're testing on mysql by checking the driver (maybe there are other ways for limiting database specific tests to the correct database, could check core for possible examples?). Then you could modify the row format and create a new database connection using Drupal\Core\Database\Database::addConnectionInfo(), then set this as the active connection, create a table (or install a test module that defines a table) and then query the information schema for the row format in use and check that it matches. Something like
SELECT row_format FROM information_schema.tables`WHERE table_schema=DATABASE() and table_name='test_table';
should do the trick.

Think that's a good start for the tests.

Thank you for the hints, @smustgrave and @FeyP!

I managed to create a working test case for this now. I couldn't have done it without you!

Ran the test without the fix and the first fails

Expected :'Compact'
Actual :'Dynamic'

Which is expected.

Good job on the test!

I do not see, why we should do this issue. In the issue summary is the reason given that we should do it, because it will reduce disk use. As disk are super cheap, this is not a good enough reason to do this issue. Cahnging the status to will not fix. If somebody has a better reason for why we should do this issue, than please add it and change to status to needs work.