870369 - An error about longdescs.comment_id is thrown by MySQL 5.1 and older when upgrading to 4.4 if sql_auto_is_null = 1

Status: RESOLVED FIXED

(Bugzilla :: Installation & Upgrading, defect)

Description

[Frédéric Buclin]

Not sure what happened while upgrading GCC Bugzilla from 4.0.10 to 4.4, but checksetup.pl failed with:

Updating column comment_id in table bugs_activity ...
Old: mediumint
New: integer
You cannot alter the longdescs.comment_id column to be NOT NULL without
specifying a default or something for $set_nulls_to, because there are
NULL values currently in it.

I reran checksetup.pl and it completed with any errors, and Bugzilla seems to work fine. The DB schema for longdescs also looks correct.

I heard the same problem happened with RedHat and Mozilla Bugzillas, so we must do something wrong.

Comment 1

[:glob ✱]

when we hit this i checked the database for nulls:

mysql> select count(*) from longdescs where comment_id is null;
+----------+
| count(*) |
+----------+
|        0 |
+----------+
1 row in set (0.00 sec)

so wrote a small script which executed the exact same check which checksetup just ran:

# cat test-col 
#!/usr/bin/perl
use strict;
use warnings;
use lib '.';
use lib 'lib';
use Bugzilla;
my $dbh = Bugzilla->dbh;
my $table = 'longdescs';
my $name = 'comment_id';
my $any_nulls = $dbh->selectrow_array("SELECT 1 FROM $table WHERE $name IS NULL");
print $any_nulls ? "yup\n" : "nope\n";
# perl test-col 
nope

i have no idea what's going on here :(

Comment 2

[Frédéric Buclin]

Before the upgrade, the schema in 4.0 is:

MariaDB [bugs_gcc]> desc longdescs;
+-----------------+--------------+------+-----+---------------------+----------------+
| Field           | Type         | Null | Key | Default             | Extra          |
+-----------------+--------------+------+-----+---------------------+----------------+
| comment_id      | mediumint(9) | NO   | PRI | NULL                | auto_increment |

So comment_id is already NOT NULL. The error doesn't make sense at all.

The code throwing the error is in Bugzilla::DB::bz_alter_column():

            # Check for NULLs
            my $any_nulls = $self->selectrow_array(
                "SELECT 1 FROM $table WHERE $name IS NULL");
            ThrowCodeError('column_not_null_no_default_alter', 
                           { name => "$table.$name" }) if ($any_nulls);
        }

But I checked, and there is no NULL values.

Comment 3

[Frédéric Buclin]

Interesting: I tried to reproduce the error with a backup of the GCC Bugzilla DB on my local machine, which is using MariaDB 5.5 (vs MySQL 5.1 on the GCC server) and it's working fine:

Updating column comment_id in table bugs_activity ...
Old: mediumint
New: integer
Updating column comment_id in table longdescs ...
Old: mediumint auto_increment PRIMARY KEY NOT NULL
New: integer auto_increment PRIMARY KEY NOT NULL

Could that be a bug in MySQL 5.1?

Comment 4

[Frédéric Buclin]

Or maybe that when importing the backup into my local DB, the comment_id column has been automatically populated as it's of type auto-increment?

Comment 5

[Frédéric Buclin]

(In reply to Frédéric Buclin from comment #3)
> Could that be a bug in MySQL 5.1?

Or in MySQL 4.1, which was the version used before migrating to the new server. In that case, there is nothing we can do about it, I guess.

Comment 6

[Frédéric Buclin]

Attachment: patch, v1

The reason I cannot reproduce with MariaDB 5.5 is because MySQL 5.5.3 and newer has sql_auto_is_null = 0 while older versions have sql_auto_is_null = 1, which causes the problem, see:

http://dev.mysql.com/doc/refman/5.5/en/server-system-variables.html#sysvar_sql_auto_is_null

Comment 7

[Frédéric Buclin]

(In reply to Frédéric Buclin from comment #6)
> The reason I cannot reproduce with MariaDB 5.5 is because MySQL 5.5.3 and
> newer has sql_auto_is_null = 0 while older versions have sql_auto_is_null =
> 1, which causes the problem

I just tested with sql_auto_is_null = 1, and this indeed causes the crash. So setting sql_auto_is_null = 0 is the right way to go, else the SQL query from comment 2 is seen as SELECT LAST_INSERT_ID() which indeed exists.

Comment 8

[Frédéric Buclin]

This is a regression due to bug 776982, so Bugzilla 4.2 and older are not affected.

Comment 9

[Teemu Mannermaa (:wicked)]

Comment on attachment 8351040 [details] [diff] [review]
patch, v1

So basicly we are asking MySQL to not support using a "SELECT * FROM tbl_name WHERE auto_col IS NULL" statement right after an INSERT statement to determine the auto increment value used in insertion. That syntax is not conformant with SQL Standard but it seems there's no standard way defined anyway.

There doesn't seem to be any code in Bugzilla that requires this syntax (and it would fail on MySQL v5.5.3) but there might be some MySQL specific extensions that use this syntax. Nevertheless, it would be better to use Bugzilla::DB bz_last_key function to determine the last key as that's implemented for all supported databases (all have different way).

Briefly tested both trunk and 4.4 after this patch and couldn't find anything failing, including checksetup.pl runs. Also, this directly fixes current redness in our checksetup upgrade test making the tests pass (for this part, there are some other problems).

Comment 10

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/DB/Mysql.pm
Committed revision 8840.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.4/
modified Bugzilla/DB/Mysql.pm
Committed revision 8644.

Comment 11

[Frédéric Buclin]

Added to relnotes for 4.4.2.