1592129 - Bugzilla failed after mysql upgrade to 8.0.17 due to "groups" keyword in mysql

Status: RESOLVED FIXED

(Bugzilla :: Database, defect)

Description

[[e-mail]]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:70.0) Gecko/20100101 Firefox/70.0

Steps to reproduce:

run ./checksetup.pl

Actual results:

error:

Fixing file permissions...
DBD::mysql::db selectrow_array failed: 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 'groups where name = ''' at line 1 [for Statement "SELECT id FROM groups where name = ''"] at Bugzilla/Install/DB.pm line 2497.
Bugzilla::Install::DB::_fix_group_with_empty_name() called at Bugzilla/Install/DB.pm line 358
Bugzilla::Install::DB::update_table_definitions(HASH(0x562aaa321460)) called at ./checksetup.pl line 175

Expected results:

checksetup should finish without error.

(groups is new keyword in mysql, so table 'groups' should be renamed)

Comment 1

[Dylan Hardison [:dylan] (he/him)]

Attachment: GitHub Pull Request

So the obvious fix here is to use quote_identifier around all tables and columns.
This is pretty easy in Bugzilla::Object or Bugzilla::DB or Bugzilla::Install::DB as we have a reference to the database connection. However, in Bugzilla::DB::Schema we don't.

So the first step would be make it so the Schema has access to the database connection.
The way Schema is structured is... let's say it is weird. So here I want to make it less weird.

Comment 2

[Dylan Hardison [:dylan] (he/him)]

Attachment: GitHub Pull Request: use Moo for the Schema classes

This is the next step, making the classes use Moo. The rationale for this is that I need a weak_ref attribute to for referencing the DB connection.

Comment 3

[Dylan Hardison [:dylan] (he/him)]

Attachment: GitHub Pull Request: Pass in Bugzilla::DB to Bugzilla::DB::Schema

Comment 4

[Dylan Hardison [:dylan] (he/him)]

Attachment: GitHub Pull Request: Shortcut for calling quote_identifier in strings

Since we have a lot of strings that need to have identifiers quoted, this is a bit of a hack.

Bugzilla->dbh->qi returns a hash that turns its keys into quote_identifier-quoted strings.
For instance, in mysql Bugzilla->dbh->qi->{pants} will return `pants`

Comment 5

[Dylan Hardison [:dylan] (he/him)]

Attachment: WIP: GitHub Pull Request: Begin using the identifier quoting shortcut

This begins using the identifier quoting shortcut. it's still a work in progress because the weekend is drawing to an end.

Comment 6

[Dylan Hardison [:dylan] (he/him)]

Attachment: GitHub Pull Request: Fix regexp compatibility error

MySQL 8 also updated regexp syntax (possibly this is pcre?), so that an unmatched { is now an error. This patch fixes that.

Comment 7

[Dylan Hardison [:dylan] (he/him)]

Attachment: GitHub Pull Request: Quote column and table names in most cases

Ok, this one is just about 100%.

Note most of the other PRs are merged.

Comment 8

[Bill Barry]

Trying to upgrade an existing install on a new machine:

DBD::mysql::db selectrow_array failed: 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 'groups where name = ''' at line 1 [for Statement "SELECT id FROM groups where name = ''"] at Bugzilla/DB.pm line 63.
Bugzilla::DB::ANON(Bugzilla::DB::Mysql=HASH(0xb978668), "SELECT id FROM groups where name = ''") called at Bugzilla/Install/DB.pm line 2497
Bugzilla::Install::DB::_fix_group_with_empty_name() called at Bugzilla/Install/DB.pm line 358
Bugzilla::Install::DB::update_table_definitions(HASH(0x329ffc8)) called at .\checksetup.pl line 175

Comment 9

[Bill Barry]

(not used to modern bugzilla, sorry about the previous comment trying again with md syntax to make it nicer)

DBD::mysql::db selectrow_array failed: 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 'groups where name = ''' at line 1 [for Statement "SELECT id FROM groups where name = ''"] at 
Bugzilla/DB.pm line 63.
        Bugzilla::DB::__ANON__(Bugzilla::DB::Mysql=HASH(0xb978668), "SELECT id FROM groups where name = ''") called at Bugzilla/Install/DB.pm line 2497
        Bugzilla::Install::DB::_fix_group_with_empty_name() called at Bugzilla/Install/DB.pm line 358
        Bugzilla::Install::DB::update_table_definitions(HASH(0x329ffc8)) called at .\checksetup.pl line 175

Comment 10

[Bill Barry]

Attachment: quote identifier groups during install/update in checksetup.pl

Comment 11

[Rogers.Doug]

I am getting this error for a new install. Is there a fix for this?

Comment 12

[bjoern]

oh, for a new install, you don't need this bug to be fixed. Just read

https://medium.com/@adamhooper/in-mysql-never-use-utf8-use-utf8mb4-11761243e434

Best hint is 2. of the take-away lessons - If you need a database, don’t use MySQL or MariaDB. Use PostgreSQL.

Comment 13

[Jacob Lear]

If anyone is trying to figure out how to fix the code to get around this bug on the current stable release, 5.0.6, you need to fully qualify the groups table name in FROM and JOIN queries and INSERT, DELETE, ALTER, and UPDATE statements.

I found that the following files needed modified to successfully get my old Bugzilla site upgraded and working with MySQL 8:
Bugzilla/Bug.pm
Bugzilla/Group.pm
Bugzilla/Search.pm
Bugzilla/User.pm
Bugzilla/Product.pm
Bugzilla/Object.pm
Bugzilla/DB.pm
Bugzilla/DB/Schema.pm
Bugzilla/Install/DB.pm
editgroups.cgi
editproducts.cgi
editusers.cgi
sanitycheck.cgi
userprefs.cgi
whine.pl

In Bugzilla/Object.pm I fully qualified the $table variable on lines 131, 162, 389, 568, 648, and 747.
In Bugzilla/DB.pm I fully qualified the $table variable on lines 1523 (two places), 1533, 1541, 1554, 1563, and 1579.
In Bugzilla/DB/Schema.pm I fully qualified the $table variables on lines 2000, 2046, 2041, 2062.
In Bugzilla/Install/DB.pm in addition to fully qualifying groups table references, you need to fix the regular expression on line 4118 to be "'^[^,]+\\{'"

I hope these notes are helpful to someone facing similar challenges.

Comment 14

[[e-mail]]

Thanks Jacob Lear. I followed similar edits (with success) to update a running system (so we can at least migrate away and export data easily).

Comment 15

[Bill Mills-Curran]

Is this bug fixed in any version of Bugzilla?

Comment 17

[Kathy Ellison]

(In reply to Jacob Lear from comment #13)

If anyone is trying to figure out how to fix the code to get around this bug on the current stable release, 5.0.6, you need to fully qualify the groups table name in FROM and JOIN queries and INSERT, DELETE, ALTER, and UPDATE statements.

I found that the following files needed modified to successfully get my old Bugzilla site upgraded and working with MySQL 8:
Bugzilla/Bug.pm
Bugzilla/Group.pm
Bugzilla/Search.pm
Bugzilla/User.pm
Bugzilla/Product.pm
Bugzilla/Object.pm
Bugzilla/DB.pm
Bugzilla/DB/Schema.pm
Bugzilla/Install/DB.pm
editgroups.cgi
editproducts.cgi
editusers.cgi
sanitycheck.cgi
userprefs.cgi
whine.pl

In Bugzilla/Object.pm I fully qualified the $table variable on lines 131, 162, 389, 568, 648, and 747.
In Bugzilla/DB.pm I fully qualified the $table variable on lines 1523 (two places), 1533, 1541, 1554, 1563, and 1579.
In Bugzilla/DB/Schema.pm I fully qualified the $table variables on lines 2000, 2046, 2041, 2062.
In Bugzilla/Install/DB.pm in addition to fully qualifying groups table references, you need to fix the regular expression on line 4118 to be "'^[^,]+\\{'"

I hope these notes are helpful to someone facing similar challenges.

Jason, do you have an example? I'm beginning an effort to do as you suggested on my teams version of Bugzilla. We are being forced to upgrade to Mysql 8.

Comment 18

[Jeff Lawson]

This PR uses $dbh->quote_identifier as recommended in bug 1604051, and is derived from the equivalent fix from the BMO repo:
https://github.com/bugzilla/bugzilla/pull/153

Comment 20

[Dave Miller [:justdave]]

Attachment: GitHub Pull Request for 5.2 by Jeff Lawson

Dylan opted to go with Jeff Lawson's PR for this

Comment 21

[Dave Miller [:justdave]]

Attachment: GitHub Pull Request for Harmony - cherry-pick from BMO

Comment 22

[Dave Miller [:justdave]]

5.2 patch has landed:
https://github.com/bugzilla/bugzilla/commit/08679016bd83d2b10bfe47f9e9d45bcae1e9981a

harmony still pending.

Comment 23

[Dave Miller [:justdave]]

Comment on attachment 9116010 [details] [review]
GitHub Pull Request

unobsoleting patches that actually landed.

Comment 24

[Dave Miller [:justdave]]

Comment on attachment 9116014 [details] [review]
GitHub Pull Request: use Moo for the Schema classes

unobsoleting patches that actually landed.

Comment 25

[Dave Miller [:justdave]]

Comment on attachment 9116026 [details] [review]
GitHub Pull Request: Pass in Bugzilla::DB to Bugzilla::DB::Schema

unobsoleting patches that actually landed.

Comment 26

[Dave Miller [:justdave]]

Comment on attachment 9116028 [details] [review]
GitHub Pull Request: Shortcut for calling quote_identifier in strings

unobsoleting patches that actually landed.

Comment 27

[Dave Miller [:justdave]]

Comment on attachment 9124072 [details] [review]
GitHub Pull Request: Fix regexp compatibility error

unobsoleting patches that actually landed.

Comment 28

[Dave Miller [:justdave]]

The Harmony version of this patch is failing tests with the following message, trying to run tests against MySQL 8.0.22:

unable to connect to dbi:mysql:database=bugs;host=bmo.db as bugs: Plugin caching_sha2_password could not be loaded: /usr/lib/x86_64-linux-gnu/mariadb19/plugin/caching_sha2_password.so: cannot open shared object file: No such file or directory at /app/scripts/entrypoint.pl line 268.

This seems to be reported on bug 1463023, though there's no fix present.

Comment 29

[Dave Miller [:justdave]]

ok, looks like this is actually a build issue with the Docker image it's using to run the tests. The MySQL client libraries are too old to talk to MySQL 8.
Making the Docker image use a newer version of bmo-perl-slim seems to fix it, but it requires installing a couple modules via CPAN which takes forever. Working on refreshing the bugzilla-perl-slim image we have on dockerhub so we can switch to that.

Comment 30

[Dave Miller [:justdave]]

Landed on Harmony
https://github.com/bugzilla/harmony/commit/62493442cf45cd2122a6565a46de9365a5becfa5