Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check for dynamic row format more consistently #628

Merged
merged 1 commit into from Aug 12, 2019

Conversation

@fritzmg
Copy link
Contributor

commented Aug 9, 2019

Currently, Contao retrieves the ROW_FORMAT of a table via the Row_format field of SHOW TABLE STATUS in order to decide whether or not to include a

ALTER TABLE ... ENGINE = ... ROW_FORMAT = DYNAMIC

statement in the Install Tool.

However, Row_format includes the information of innodb_default_row_format and thus this might be skipped.

Why is this a problem? Consider the following:

  • A Contao 3 installation is updated to Contao 4.
  • The environment during the update is using MariaDB 10.3.
  • After the update, the Contao installation plus its database is deployed on another environment running MariaDB 10.1.

This causes a problem during the import of the SQL dump because the MariaDB 10.1 version of the target environment does not use a default row format of DYNAMIC. In MariaDB 10.1 the default row format is still COMPACT. Thus the SQL export of the update environment will be incompatible with the target environment, because the ROW_FORMAT=DYNAMIC create options of all the tables will be missing - which causes problems with index key lengths, since the limit will be 767 when using ROW_FORMAT=COMPACT.

This is not a problem when doing a fresh installation, as Contao 4 will always add ROW_FORMAT=DYNAMIC to the create options of the table then. It is however a problem, if you update the database from Contao 3 to 4.

Instead of checking the Row_format of a table, this PR instead checks the Create_options to see wether they already contain ROW_FORMAT=DYNAMIC or not.

@fritzmg fritzmg changed the title check for dynamic row format more consistently Check for dynamic row format more consistently Aug 9, 2019

@leofeyer

This comment has been minimized.

Copy link
Member

commented Aug 9, 2019

However, Row_format includes the information of innodb_default_row_format and thus this might be skipped.

Where did you find this?

@fritzmg

This comment has been minimized.

Copy link
Contributor Author

commented Aug 10, 2019

Just through observation/debugging. It makes sense too - when you query the row_format for a table, something must be returned, since the table obviously has a row_format, even when not defined in the create_options. It will be the server's default row_format then.

@leofeyer

This comment has been minimized.

Copy link
Member

commented Aug 12, 2019

I'm not sure about that. Running SELECT DISTINCT ROW_FORMAT FROM INNODB_SYS_TABLES on my local system shows that every table has a row format – even the ones that were not explicitly created with one. Also, the manual says The row format of a table determines how its rows are physically stored, so there cannot be no row format.

@fritzmg

This comment has been minimized.

Copy link
Contributor Author

commented Aug 12, 2019

Yes, exactly. But during a MySQL dump, the ROW_FORMAT create option is only exported for tables where a ROW_FORMAT has explicitly been set in the create options. Since Contao currently only checks the current row format of a table and not their create options, the ROW_FORMAT=DYNAMIC create option will not be explicitly set by Contao. Leading to problems when deploying the database on to another environment where the default row format is not DYNAMIC.

@leofeyer

This comment has been minimized.

Copy link
Member

commented Aug 12, 2019

How exactly do I reproduce this case?

@fritzmg

This comment has been minimized.

Copy link
Contributor Author

commented Aug 12, 2019

  1. Use a database server in your local environment with a innodb_default_row_format value of DYNAMIC (default for MariaDB 10.2.2+ I think).
  2. In your local environment, update a Contao 3.5.40 database to Contao 4.x (with utf8mb4).
  3. Create a mysql dump of that database.
  4. Import that dump on a MySQL server where the innodb_default_row_format is COMPACT (e.g. MariaDB 10.1)

The mysql dump will contain the following definition for tl_files for example:

CREATE TABLE `tl_files` (
  `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
  `pid` binary(16) DEFAULT NULL,
  `tstamp` int(10) unsigned NOT NULL DEFAULT 0,
  `uuid` binary(16) DEFAULT NULL,
  `type` varchar(16) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '',
  `path` varchar(1022) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL DEFAULT '',
  `extension` varchar(16) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL DEFAULT '',
  `hash` varchar(32) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '',
  `found` char(1) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '1',
  `name` varchar(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL DEFAULT '',
  `importantPartX` int(10) NOT NULL DEFAULT 0,
  `importantPartY` int(10) NOT NULL DEFAULT 0,
  `importantPartWidth` int(10) NOT NULL DEFAULT 0,
  `importantPartHeight` int(10) NOT NULL DEFAULT 0,
  `meta` blob DEFAULT NULL,
  PRIMARY KEY (`id`),
  UNIQUE KEY `uuid` (`uuid`),
  KEY `pid` (`pid`),
  KEY `extension` (`extension`),
  KEY `path` (`path`(768))
) ENGINE=InnoDB AUTO_INCREMENT=6772 DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci

Note the missing ROW_FORMAT=DYNAMIC create option.

When importing this on a database of Hostingwerk.de, which runs MariaDB 10.1, you will get the following error:

Index column size too large. The maximum column size is 767 bytes.

This is because under utf8mb4, the maximum index length is 767 bytes, when using a ROW_FORMAT of COMPACT for example.

@leofeyer leofeyer added the defect label Aug 12, 2019

@leofeyer leofeyer added this to the 4.7 milestone Aug 12, 2019

@leofeyer leofeyer force-pushed the fritzmg:fix-dynamic-format branch from dd42239 to 716b520 Aug 12, 2019

@leofeyer leofeyer changed the base branch from 4.7 to 4.8 Aug 12, 2019

@leofeyer leofeyer modified the milestones: 4.7, 4.8 Aug 12, 2019

@leofeyer leofeyer merged commit cd624df into contao:4.8 Aug 12, 2019

0 of 2 checks passed

Travis CI - Pull Request Build Errored
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
@leofeyer

This comment has been minimized.

Copy link
Member

commented Aug 12, 2019

Thank you @fritzmg.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.