Edgewall Software
Modify

#13128 closed defect (fixed)

Timeline broken with a MySQL 8.0 backend

Reported by: anonymous Owned by: Jun Omae
Priority: normal Milestone: 1.0.18
Component: database backend Version: 1.2.2
Severity: major Keywords: mysql
Cc: Branch: log:jomae.git@t13128+1.0-stable log:jomae.git@t13128+1.2-stable log:jomae.git@t13128+trunk
Release Notes:

Compatibility fix with MySQL 8.0.

API Changes:
Internal Changes:

Description (last modified by Ryan J Ollos)

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

Attachments (2)

Trac - MySQL 8.0 - Timeline.png (157.8 KB ) - added by anonymous 21 months ago.
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
mysql-8-system.diff (7.5 KB ) - added by anonymous 21 months ago.
Patch

Download all attachments as: .zip

Change History (20)

by anonymous, 21 months ago

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

by anonymous, 21 months ago

Attachment: mysql-8-system.diff added

Patch

comment:1 by anonymous, 21 months ago

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

comment:2 by Jun Omae, 21 months ago

Keywords: mysql added
Milestone: 1.2.41.0.18
Owner: set to Jun Omae
Status: newassigned
Version: 1.2.3

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

I'll fix it based the patch.

comment:3 by Jun Omae, 21 months ago

Version: 1.2.31.2.2

comment:4 by Ryan J Ollos, 21 months ago

Description: modified (diff)

comment:5 by Jun Omae, 21 months ago

Branch: log:jomae.git@t13128+1.0-stable log:jomae.git@t13128+1.2-stable log:jomae.git@t13128+trunk

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:

comment:6 by Christian Boos, 21 months ago

What about an upgrade step for renaming system to trac_system?

comment:7 by Ryan J Ollos, 21 months ago

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.

comment:8 by Jun Omae, 21 months ago

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
Last edited 21 months ago by Jun Omae (previous) (diff)

in reply to:  8 comment:9 by Ryan J Ollos, 21 months ago

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

comment:10 by Jun Omae, 21 months ago

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

comment:11 by Ryan J Ollos, 21 months ago

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

in reply to:  11 comment:12 by Christian Boos, 21 months ago

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 systemtrac_system substitution is then much easier for them than adding the db.quote().

comment:13 by Jun Omae, 21 months ago

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.

comment:14 by Ryan J Ollos, 21 months ago

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 systemtrac_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 ;)

Version 1, edited 21 months ago by Ryan J Ollos (previous) (next) (diff)

in reply to:  13 comment:15 by Peter Suter, 21 months ago

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.

in reply to:  13 comment:16 by Jun Omae, 21 months ago

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

comment:17 by Jun Omae, 20 months ago

Release Notes: modified (diff)

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

comment:18 by Ryan J Ollos, 20 months ago

Resolution: fixed
Status: assignedclosed

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.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jun Omae.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Jun Omae to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.