Make ROW_FORMAT configurable

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 last submitted patch, 28: drupal-row_format-2857359-28.patch, failed testing. View results →

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 📌 Remove duplicate scaffold files Needs work .

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.

arnested → closed merge request !3759

• 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.

I remember the reason why I started having this problem was that the index key prefix length limit is 767 bytes for InnoDB tables using the default (COMPACT) row format. This 767 bytes quickly reduces to 191 UTF-8 characters. On the site where this occurred, I use views to quickly display several custom entities that have large text fields, and needed to create custom keys to speed up those queries, at the expense of a larger index file.

People running into similar problems that require the ability to change the row format should not have their hands tied by Drupal's inability to pass this parameter to the MySQL connector.

See:
* MySQL: https://dev.mysql.com/doc/refman/8.4/en/innodb-limits.html
* MariaDB: https://mariadb.com/kb/en/innodb-dynamic-row-format/

Setting to "Needs work" as per the MR comments in #42.

This needs to be committed to 11.x (which is main) first and then applied to branches.

arnested → changed the visibility of the branch 10.1.x to hidden.

arnested → changed the visibility of the branch 10.1.x to active.

I have added an 11.x merge request for this. All merge request comments have been resolved.

I was the one citing disk space as a reasoning in the first place. I can elaborate a bit on why this was important for us.

We run a single server with currently 450 small, low traffic sites. The overhead of the database tables, even without any real content, was huge before we added row format compressed.

6 years ago, we started out with 401 sites. After doing site install of 300~ sites, we had filled up the 100 GiB disk. After shifting to row format compressed we had plenty of available disk space.

The change is small, it only affects the MySQL driver, and the change is inline with similar features for the MySQL driver, e.g. the "collate" option.

arnested → changed the visibility of the branch 10.1.x to hidden.

With a new settings variable going to need a change record for what it does

If possible examples are also useful.

Thanks for keeping this one going.

(Attempt at) Change Record added.

CR is straight forward.

Ran test-only feature here https://git.drupalcode.org/issue/drupal-2857359/-/jobs/2264362 and shows multiple failures so coverage is good.

Believe this one is good. Would suggest maybe a 11.1 highlight?

Let's get a subsystem maintainer review here, since they asked a question in #42. I left a suggestion in the MR as well, so setting to needs work for that doc change.

And a title change to indicate that this is MySQL only would help.

I made the doc change and changed the title. Leaving at RTBC

It is not clear to me which row format options Drupal is going to support. Are we are going to support any other one than the default one. Could we update the CR with this information. Including which is the default row format option? With these kind of changes the CR becomes very important to me. There should be information on which row format options there are and what they do. If you are a site owner, it should be clear what the options are, what they do and when to use them. Now the CR is missing that information.

It would be great if we could add an installer test to see if we can install Drupal with another row format option other then the default one.

The PR should pass the testbot for MySQL and MariaDB.

The change request now contains information on the different row formats and which is the default one.

I don't think that this should really need a test to check if Drupal works with other row formats. This is simply passed on to the database server when creating a table, and we would only be testing database engine functionality and not Drupal. It is the responsibility of the database engine to support the different row formats and make that transparent when that table is used.

Probably CI images needs update #3468905: Update default system encoding setting for Mysql images → before merging it as all Mysql 8.0+ images complain in logs about default encoding

Updated the remaining tasks regarding the question of tests and about testing images. All from the last 4 comments.

I think this is more complex than we have here. If we just set it in Drupal settings then if someone runs optimize on the db server it will change back again... from the MySQL docs:

mysql> SELECT @@innodb_default_row_format;
+-----------------------------+
| @@innodb_default_row_format |
+-----------------------------+
| dynamic                     |
+-----------------------------+

mysql> CREATE TABLE t1 (c1 INT);

mysql> SELECT * FROM INFORMATION_SCHEMA.INNODB_SYS_TABLES WHERE NAME LIKE 'test/t1' \G
*************************** 1. row ***************************
     TABLE_ID: 54
         NAME: test/t1
         FLAG: 33
       N_COLS: 4
        SPACE: 35
  FILE_FORMAT: Barracuda
   ROW_FORMAT: Dynamic
ZIP_PAGE_SIZE: 0
   SPACE_TYPE: Single

mysql> SET GLOBAL innodb_default_row_format=COMPACT;

mysql> ALTER TABLE t1 ADD COLUMN (c2 INT);

mysql> SELECT * FROM INFORMATION_SCHEMA.INNODB_SYS_TABLES WHERE NAME LIKE 'test/t1' \G
*************************** 1. row ***************************
     TABLE_ID: 55
         NAME: test/t1
         FLAG: 1
       N_COLS: 5
        SPACE: 36
  FILE_FORMAT: Antelope
   ROW_FORMAT: Compact
ZIP_PAGE_SIZE: 0
   SPACE_TYPE: Single

In my opinion, somethings are just best left as things to do on the database and not for Drupal to do.

But the change here is going to add row format to every create table so now things that used to work to change for all tables is no longer going to work. You'll have to change the global and then set the row format on every table.

I'd be tempted to make this a documentation task and to provide good docs pointing MySQL and MariaDB docs on the subject and to detail tables where you might want to change to COMPACT if available. But also given that the default is DYNAMIC on the versions of DB drivers supported by Drupal 11 I'm not sure that this change is that necessary anymore.

I think this is more complex than we have here. If we just set it in Drupal settings then if someone runs optimize on the db server it will change back again...

Not really.

Setting the row format in Drupal doesn't alter the database's default row format. It sets the configured row format on each of the tables Drupal creates.

Running mysqloptimize doesn't change it back. The table keeps its row format.

I'd be tempted to make this a documentation task and to provide good docs pointing MySQL and MariaDB docs on the subject and to detail tables where you might want to change to COMPACT if available. But also given that the default is DYNAMIC on the versions of DB drivers supported by Drupal 11 I'm not sure that this change is that necessary anymore.

This is the problem. It can't be a documentation task.. Drupal has no way to support this. None. You can't document what Drupal blocks you from doing. What we want here is precisely the option to be able to pass this so that it can be documented.

Drupal doesn't stop you from changing your databases default row format. And setting it on every table makes it harder to manage on the database end.

Re documenting this... we could display the default row format using system_requirements_runtime() and link to good documentation about what the choices are and why you might make them.

I see... you can't set compressed as the default format...

mysql> SET GLOBAL innodb_default_row_format=COMPRESSED;
ERROR 1231 (42000): Variable 'innodb_default_row_format'
can't be set to the value of 'COMPRESSED'

So some general thoughts:

• Drupal does not stop you changing existing table to COMPRESSED but how would you get new tables to use it.
• It feels unlikely that you would want to store all tables as COMPRESSED - there must be some cost as you have to compress the data on write and uncompress on read
• With the current implementation changing this setting during the lifecycle of a site is interesting - your live environment would have tables with the old row format unless you changed them all and tests that did their own install would have the new format
• Perhaps we need some way to configure a list of tables and row formats and we should support some form of name matching so you could compress all paragraph_revision__* tables for example. And to report if tables list in this setting match the expectation

@alexpott, the same reasoning applies to the "collation" setting of the current database configuration. No one should be changing it in runtime but if you do, it only applies to newly created tables. At least that is my understanding that if you set it to "utf8mb4_general_ci", you won't start to have problems with tables created with other collations. It's just that all newly created tables will store text as UTF-8.

@jcnventura but the point is you are not likely to want to set all tables to COMPRESSED

Also \Drupal\Core\Command\DbDumpCommand::getTableSchema() needs updating to support this I guess. And we need to work out how. I think mysqldump will only add ROW_FORMAT to create table when it has been explicitly set for a table.

Also I think databases have moved on from COMPRESSED row format - see https://mariadb.com/kb/en/innodb-page-compression/#comparison-with-the-c... and https://dev.mysql.com/doc/refman/8.4/en/innodb-page-compression.html#:~:....

As per the first link:

In general, InnoDB page compression is superior to the COMPRESSED row format.

Perhaps we should look to implement this for all revision tables...

I do remember when I started this that my problem was that Drupal created all tables as COMPACT, and I needed them to be DYNAMIC. I believe this was because I was using MariaDB 10.1 before this was changed to DYNAMIC in MariaDB 10.2.2. This is according to my Lando configuration at the time (DDEV was still v0.17 back then...).
I'm now using MariaDB 10.11, so I should probably remove this patch from my composer.json, and stop trying to chase this cart.

Even though it is no longer useful for my original purposes, I think the fact remains that this is a parameter that can be nice to have in the Drupal configuration to provide access to this setting of MySQL/MariaDB. And yes, one might argue if it makes sense to be able to do this. I'm not suggesting that anyone should change the defaults.. I'm only advocating for having the option to provide this setting.

However, maybe the best would be to have a generic way to provide ANY of the options supported by MySQL (https://dev.mysql.com/doc/refman/8.4/en/create-table.html#create-table-o...), and then have a way to expand the key/values provided in that setting into key=value parameters added to the create table command.

@jcnventura it sounds like option for contrib module a-la Schema →