Timeline broken with a MySQL 8.0 backend

[anonymous]

After upgrading from MySQL 5.7 to MySQL 8.0, I'm getting the following error message when accessing the "Timeline" in my Trac instance:

Trac detected an internal error:

ProgrammingError: (1064, "You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'system WHERE name='initial_database_version'' at line 1")


Python Traceback
Most recent call last:

File "C:/server/trac/.egg-cache/trac-1.2.2-py2.7-win32.egg-tmp/trac/timeline/templates/timeline.html", line 63, in <Expression u"event.render('description', context)">
  ${event.render('description', context)}
File "c:/users/trac/appdata/local/temp/easy_install-ni58x7/Trac-1.2.2-py2.7-win32.egg.tmp/trac/timeline/web_ui.py", line 374, in <lambda>
  provider.render_timeline_event(context, field, event)
File "c:/users/trac/appdata/local/temp/easy_install-ni58x7/Trac-1.2.2-py2.7-win32.egg.tmp/trac/ticket/web_ui.py", line 411, in render_timeline_event
  t_context.set_hints(preserve_newlines=self.must_preserve_newlines)
File "c:/users/trac/appdata/local/temp/easy_install-ni58x7/Trac-1.2.2-py2.7-win32.egg.tmp/trac/util/__init__.py", line 1202, in __get__
  result = self.fn(instance)
File "c:/users/trac/appdata/local/temp/easy_install-ni58x7/Trac-1.2.2-py2.7-win32.egg.tmp/trac/ticket/web_ui.py", line 125, in must_preserve_newlines
  preserve_newlines = self.env.database_initial_version >= 21 # 0.11
File "c:/users/trac/appdata/local/temp/easy_install-ni58x7/Trac-1.2.2-py2.7-win32.egg.tmp/trac/util/__init__.py", line 1202, in __get__
  result = self.fn(instance)
File "c:/users/trac/appdata/local/temp/easy_install-ni58x7/Trac-1.2.2-py2.7-win32.egg.tmp/trac/env.py", line 613, in database_initial_version
  .get_database_version('initial_database_version')
File "c:/users/trac/appdata/local/temp/easy_install-ni58x7/Trac-1.2.2-py2.7-win32.egg.tmp/trac/db/api.py", line 480, in get_database_version
  """, (name,))
File "c:/users/trac/appdata/local/temp/easy_install-ni58x7/Trac-1.2.2-py2.7-win32.egg.tmp/trac/db/api.py", line 122, in execute
  return db.execute(query, params)
File "c:/users/trac/appdata/local/temp/easy_install-ni58x7/Trac-1.2.2-py2.7-win32.egg.tmp/trac/db/util.py", line 128, in execute
  cursor.execute(query, params if params is not None else [])
File "c:/users/trac/appdata/local/temp/easy_install-ni58x7/Trac-1.2.2-py2.7-win32.egg.tmp/trac/db/util.py", line 72, in execute
  return self.cursor.execute(sql_escape_percent(sql), args)
File "C:/server/Python27/lib/site-packages/MySQLdb/cursors.py", line 205, in execute
  self.errorhandler(self, exc, value)
File "C:/server/Python27/lib/site-packages/MySQLdb/connections.py", line 36, in defaulterrorhandler
  raise errorclass, errorvalue

Reason: system has become a ​reserved keyword in MySQL 8.0.

Location: tags/trac-1.2.3/trac/db/api.py#L479​

[anonymous]

ou have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'system WHERE name='initial_database_version at line 1

[anonymous]

Patch

[anonymous]

I have added a patch that fixed the problem for me (I checked out the 1.2-branch, applied the changes and python ./setup.py installed it).

[Jun Omae]

Thanks for the reporting and patch, however your patch doesn't work with PostgreSQL.

I'll fix it based the patch.

[Jun Omae]

I noticed another issue on unit tests.

In reset_mysql_db, use DELETE FROM or TRUNCATE TABLE based on auto_increment column of information_schema.tables. However, with MySQL 8.0, the auto_increment is 1 even if the next value is not 1.

Resetting database doesn't work due to the behavior and unit tests fail.

mysql> SELECT table_schema, table_name, auto_increment FROM information_schema.tables WHERE table_name='ticket';
+--------------+------------+----------------+
| TABLE_SCHEMA | TABLE_NAME | AUTO_INCREMENT |
+--------------+------------+----------------+
| tractest     | ticket     |              1 |
+--------------+------------+----------------+
1 row in set (0.01 sec)

mysql> SHOW CREATE TABLE ticket\G
*************************** 1. row ***************************
       Table: ticket
Create Table: CREATE TABLE `ticket` (
  `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
  `type` text COLLATE utf8mb4_bin,
  `time` bigint(20) DEFAULT NULL,
  `changetime` bigint(20) DEFAULT NULL,
  `component` text COLLATE utf8mb4_bin,
  `severity` text COLLATE utf8mb4_bin,
  `priority` text COLLATE utf8mb4_bin,
  `owner` text COLLATE utf8mb4_bin,
  `reporter` text COLLATE utf8mb4_bin,
  `cc` text COLLATE utf8mb4_bin,
  `version` text COLLATE utf8mb4_bin,
  `milestone` text COLLATE utf8mb4_bin,
  `status` text COLLATE utf8mb4_bin,
  `resolution` text COLLATE utf8mb4_bin,
  `summary` text COLLATE utf8mb4_bin,
  `description` text COLLATE utf8mb4_bin,
  `keywords` text COLLATE utf8mb4_bin,
  PRIMARY KEY (`id`),
  KEY `ticket_time_idx` (`time`),
  KEY `ticket_status_idx` (`status`(191))
) ENGINE=InnoDB AUTO_INCREMENT=103 DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin
1 row in set (0.01 sec)

Instead, we could use extra column of information_schema.columns to detect whether auto_increment column is used in the table.

mysql> SELECT table_schema, table_name, column_name, extra
    -> FROM information_schema.columns
    -> WHERE extra='auto_increment'
    -> AND table_schema='tractest'
    -> ORDER BY 1, 2, 3;
+--------------+------------+-------------+----------------+
| TABLE_SCHEMA | TABLE_NAME | COLUMN_NAME | EXTRA          |
+--------------+------------+-------------+----------------+
| tractest     | report     | id          | auto_increment |
| tractest     | ticket     | id          | auto_increment |
+--------------+------------+-------------+----------------+
2 rows in set (0.00 sec)

Proposed changes:

• jomae.git@t13128+1.0-stable (​tested with MySQL 8 on circleci)
• jomae.git@t13128+1.2-stable (​tested with MySQL 8 on circleci)
• jomae.git@t13128+trunk (​tested with MySQL 8 on circleci)

[Christian Boos]

What about an upgrade step for renaming system to trac_system?

[Ryan J Ollos]

All tests pass for me on OSX 10.14.2 and MySQL 8.0.13 with the comment:5 changes.

I favor committing those now so that we restore compatibility with the latest MySQL, and then possibly renaming the table (comment:6) on trunk.

[Jun Omae]

List of plugins on trac-hacks using system table:

$ git grep -i '\<\(FROM\|INSERT\s*INTO\|UPDATE\)\s\+system\>' -- '*.py' | sed -e '\!/tests/!d; s#/.*$##' | sort | uniq -c
      8 announcerplugin
      4 backlogplugin
      4 bittenforgitplugin
      3 bookmarkplugin
      3 boxdbplugin
      4 calendarplugin
      4 cardsplugin
     12 clientsplugin
      6 codereviewerplugin
      2 componenthierarchyplugin
      3 contactsplugin
     12 directoryauthplugin
     21 discussionplugin
     20 downloadsplugin
      9 estimatorplugin
      2 exowebcodereviewplugin
      4 extendedversionplugin
      6 fullblogplugin
      4 fulltextsearchplugin
      2 geoticketplugin
      9 gringottsplugin
      4 guestbookplugin
      6 hidevalsplugin
      3 jobcontrolplugin
      3 mailarchiveplugin
     12 masterticketsplugin
      8 mathcaptchaplugin
      3 milestoneteamsplugin
      2 mindmapmacro
      3 multiprojectbacklogplugin
      9 narcissusplugin
     97 peerreviewplugin
      2 perforceplugin
      3 remoteticketplugin
     24 screenshotsplugin
      3 serverdownmacro
     45 simplemultiprojectplugin
     24 tagsplugin
      3 teamcalendarplugin
      4 timetrackingplugin
     36 timingandestimationplugin
      3 tracblplugin
     15 tracforgeplugin
     48 tracformsplugin
      6 trachacksplugin
      9 trachoursplugin
      6 tracjsganttplugin
      4 tracmilemixviewplugin
      8 tracpasteplugin
      4 tracreleaseplugin
      8 tracreportmanagerplugin
      5 tracsecdlplugin
      4 tracticketchainedfieldsplugin
      4 tracticketchangesetsplugin
     12 tractickettemplateplugin
      4 tractweakuiplugin
      8 tracunreadplugin
      4 visitcountermacro
     29 voteplugin
     16 watchlistplugin
      4 weekplanplugin
      6 wikiformsplugin
      9 worklogplugin

[Ryan J Ollos]

Replying to Jun Omae:

List of plugins on trac-hacks using system table:

Eventually, plugins should be modified to use DatabaseManager.{set_database_version/get_database_version} (added in Trac 1.2). Some of those listed have already been modified in their latest release (e.g. ​CardsPlugin in ​r16098).

[Jun Omae]

Yeah, the following plugins already use DatabaseManager.[gs]et_database_version methods.

$ git grep -w '[gs]et_database_version' -- '*.py' | sed -e 's#^\([^/]*/[^/]*\).*#\1#' | sort | uniq -c
      9 bookmarkplugin/trunk
      3 cardsplugin/trunk
     17 codereviewerplugin/1.0
      1 componenthierarchyplugin/1.2
      1 contactsplugin/1.2
      1 hidevalsplugin/1.2
      3 mailarchiveplugin/trunk
     12 masterticketsplugin/tags
     12 masterticketsplugin/trunk
      2 mathcaptchaplugin/1.2
      2 mindmapmacro/1.2
      3 onsitenotificationsplugin/trunk
      3 packagerepositoryplugin/trunk
      1 privatereportsplugin/trunk
      3 pullrequestsplugin/trunk
      6 screenshotsplugin/1.2
     18 simplemultiprojectplugin/trunk
     11 teamcalendarplugin/1.0
      3 timetrackingplugin/trunk
      8 tracpasteplugin/trunk
      1 tracticketchainedfieldsplugin/1.2
      1 tracticketchangelogplugin/1.2
      1 tractweakuiplugin/1.2
     13 voteplugin/trunk
      3 weekplanplugin/trunk

[Ryan J Ollos]

Given that information, maybe defer the table rename to 1.5.x, or later?

[Christian Boos]

Replying to Ryan J Ollos:

Given that information, maybe defer the table rename to 1.5.x, or later?

Yes, why not.

All those plugins using directly system would need to be adapted as well if they were to support MySQL 8. And to that end, doing a system → trac_system substitution is then much easier for them than adding the db.quote().

[Jun Omae]

I don't think the renaming is good idea.

The renaming would be performed by upgrade/dbN.py. The database version would be to retrieve from system or trac_system table. However, the database version is needed to determine whether system or trac_system table. That is cyclic.

[Ryan J Ollos]

More consideration of table rename: to support Trac < 1.2 with MySQL 8.0, plugins will need to db.quote the system table. There's no way around that since we wouldn't add a DB upgrade step in a minor release, and even if we did the problem would exist for plugins supporting Trac < 1.0.18.

For Trac ≥ 1.2, plugins can use the DatabaseManager API and avoid direct interactions with the system table, thereby eliminating any concern over db.quote vs system rename. Given that we'd only rename the table on trunk, it probably makes more sense just to push all plugins supporting Trac > 1.2 to use the DatabaseManager API, rather than db.quote or system → trac_system.

Keeping in mind that even using the DatabaseManager API with Trac < 1.2.4, the plugin won't be compatible with MySQL 8.0. The easy solution there is to tell the user to upgrade their Trac instance; it's not the plugin's problem.

Or plugins can just stop supporting Trac < 1.2, which is definitely my preference ;)

[Peter Suter]

Replying to Jun Omae:

I don't think the renaming is good idea.

The renaming would be performed by upgrade/dbN.py. The database version would be to retrieve from system or trac_system table. However, the database version is needed to determine whether system or trac_system table. That is cyclic.

This is a good point. It seems this could lead to many problems. (Even without plugins.) Also to support upgrading from an old environment, a new Trac would have to support system indefinitely anyway to know which upgrades to apply.

[Jun Omae]

Replying to Jun Omae:

The renaming would be performed by upgrade/dbN.py. The database version would be to retrieve from system or trac_system table. However, the database version is needed to determine whether system or trac_system table. That is cyclic.

Another idea, we could create system view to trac_system table after the renaming, to determine the database version. However, insert/update statements should be executed to trac_system table because a view in SQLite is not writable (writable in PostgreSQL and MySQL).

[Jun Omae]

The changes using db.quote() have been committed in [16881-16883].

[Ryan J Ollos]

I'm going to close this one under assumption we can make another issue if more work is to be done after 1.0.18.