#10993 closed defect (fixed)
MySQL: Upgrade should enforce new tables with InnoDB
Reported by: | Owned by: | Jun Omae | |
---|---|---|---|
Priority: | normal | Milestone: | 1.0.2 |
Component: | database backend | Version: | 0.12-stable |
Severity: | major | Keywords: | mysql |
Cc: | Branch: | ||
Release Notes: |
Verify expected storage engine, charset and collation when initenv and db upgrades if MySQL |
||
API Changes: | |||
Internal Changes: |
Description
During upgrade process, when Trac creates tables for MySQL, it should create a test temporary table and check if its type is MyISAM. If it is MyISAM, then Trac should enforce InnoDB engine on all newly created tables during upgrade process.
MyISAM table types breaks transactions, which becomes a source of magical bugs like #8067.
Attachments (0)
Change History (11)
comment:1 by , 12 years ago
Keywords: | mysql added |
---|---|
Milestone: | → next-major-releases |
comment:2 by , 11 years ago
Milestone: | next-major-releases → 1.0.2 |
---|---|
Owner: | set to |
Status: | new → assigned |
Proposed changes can be found in log:jomae.git:ticket10993.
The following are verified on init_db
and environment_needs_upgrade
of MySQLConnector
:
MyISAM
engine of table isn't usedutf8_bin
orutf8mb4_bin
is used as collation of tabledefault_storage_engine
variable isn'tMyISAM
character_set_database
andcollation_database
variables are one of(('utf8', 'utf8_bin'), ('utf8mb4', 'utf8mb4_bin'))
follow-up: 4 comment:3 by , 11 years ago
Looks good to me.
Are you concerned about temporary tables? Since 5.6.3 someone could sneakily set default_tmp_storage_engine. (Or other weird things, like use a CSV engine?)
comment:8:ticket:10661 proposed to:
to do a "safety check" (creating a (temp) table, verify the encoding; try to insert a few rows in a transaction (with upper/lower case conflicts); rollback and check if all rows are gone).
But I think your proposed changes are simpler and sufficient for the most likely problem of old MySQL with MyISAM default.
comment:4 by , 11 years ago
Replying to psuter:
Are you concerned about temporary tables? Since 5.6.3 someone could sneakily set default_tmp_storage_engine. (Or other weird things, like use a CSV engine?)
Okay. Updated log:jomae.git:ticket10993 branch.
I've added the verifications for default_tmp_storage_engine
and the engine is neither EXAMPLE
, ARCHIVE
, CSV
nor ISAM
that they don't support transactions in obvious.
comment:5 by , 11 years ago
Release Notes: | modified (diff) |
---|
comment:6 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:8 by , 6 years ago
Edited MySqlDb@77.
This issue might be revisited in the future, for now I'm just summarizing the discussion in gdiscussion:trac-users:MO_KSSQMdnU.
The requirement to set
default_storage_engine
anddefault_tmp_storage_engine
inmy.cnf
could be removed if:
- The storage engine was specified in a trac.ini variable such as
[trac]
mysql_storage_engine
or[mysql]
storage_engine
MySQLConnector.to_sql
was modified to include theENGINE
table option inTABLE CREATE
statements- The modules in the
upgrade
package were modified to use theENGINE
table option either directly or indirectly by use of a helper function (such asMySQLConnector.to_sql
)
comment:9 by , 6 years ago
I noticed we could use default_storage_engine
and default_tmp_storage_engine
in session variables, rather than my.cnf
.
SET @@session.default_storage_engine = 'InnoDB';
PoC: [f877c7e1d/jomae.git] (jomae.git@t10993_storage_option)
comment:10 by , 6 years ago
Setting storage engine for session looks like a good idea. Should we make a new ticket and target to a 1.3.x milestone?
Good idea, PatchWelcome (make that an
IEnvironmentSetupParticipant.environment_needs_upgrade
check, in theMySQLConnector
).