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

RESOLVED FIXED in Bugzilla 4.4

Status

()

Bugzilla
Installation & Upgrading
--
major
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Frédéric Buclin, Assigned: Frédéric Buclin)

Tracking

({regression})

Bugzilla 4.4
regression
Bug Flags:
approval +
approval4.4 +
blocking4.4.2 +

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
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.
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 :(
(Assignee)

Comment 2

4 years ago
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.
(Assignee)

Comment 3

4 years ago
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?
(Assignee)

Comment 4

4 years ago
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?
(Assignee)

Comment 5

4 years ago
(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.
(Assignee)

Comment 6

4 years ago
Created attachment 8351040 [details] [diff] [review]
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
Assignee: installation → LpSolit
Status: NEW → ASSIGNED
Attachment #8351040 - Flags: review?(wicked)
(Assignee)

Updated

4 years ago
Target Milestone: --- → Bugzilla 4.4
(Assignee)

Comment 7

4 years ago
(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.
Flags: blocking4.4.2+
(Assignee)

Updated

4 years ago
Summary: An error about longdescs.comment_id is thrown when upgrading from 4.0 to 4.4 → An error about longdescs.comment_id is thrown by MySQL 5.1 and older when upgrading from 4.0 to 4.4 if sql_auto_is_null = 1
(Assignee)

Comment 8

4 years ago
This is a regression due to bug 776982, so Bugzilla 4.2 and older are not affected.
Severity: normal → major
Depends on: 776982
Keywords: regression
Summary: An error about longdescs.comment_id is thrown by MySQL 5.1 and older when upgrading from 4.0 to 4.4 if sql_auto_is_null = 1 → 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
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).
Attachment #8351040 - Flags: review?(wicked) → review+
Flags: approval?
Flags: approval4.4?

Updated

4 years ago
Flags: approval?
Flags: approval4.4?
Flags: approval4.4+
Flags: approval+
(Assignee)

Comment 10

4 years ago
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/DB/Mysql.pm
Committed revision 8840.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.4/
modified Bugzilla/DB/Mysql.pm
Committed revision 8644.
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Assignee)

Comment 11

4 years ago
Added to relnotes for 4.4.2.
You need to log in before you can comment on or make changes to this bug.