328063 - [PostgreSQL] Bugzilla::DB::Pg does not support adding a new SERIAL-type column to a table

Status: RESOLVED FIXED

(Bugzilla :: Installation & Upgrading, defect)

Description

[Frédéric Buclin]

Here is what I get, on PostgreSQL only:

checksetup.pl: NOT NULL columns must have a DEFAULT or an init_value. at Bugzilla/DB/Schema/Pg.pm line 122.


This is probably a regression due to bug 225221.

Comment 1

[Max Kanat-Alexander]

This *should* be happening on all DBs. That code is supposed to be present in the base DB.pm.

Comment 2

[Max Kanat-Alexander]

Okay, here's the problem:

$dbh->bz_add_column('longdescs', 'comment_id',
                    {TYPE => 'MEDIUMSERIAL', NOTNULL => 1, PRIMARYKEY => 1});

And in Pg.pm:

if ($definition->{NOTNULL}) {
        # Handle rows that were NULL when we added the column.
        # We *must* have a DEFAULT. This check is usually handled
        # at a higher level than this code, but I figure it can't
        # hurt to have it here.
        die "NOT NULL columns must have a DEFAULT or an init_value."
            unless (exists $definition->{DEFAULT} || defined $init_value);
        push(@statements, "UPDATE $table SET $column = $default");
        push(@statements, "ALTER TABLE $table ALTER COLUMN $column "
                         . " SET NOT NULL");
    }

  What the code doesn't realize is that these SERIAL fields should be somehow automatically adding themselves and numbering things. How that happens in Pg, I'm really not sure.

Comment 3

[Max Kanat-Alexander]

Okay, even fixing that, the problem is:

[Tue Feb 21 14:17:35 2006] checksetup.pl: DBD::Pg::db do failed: ERROR:  adding columns with defaults is not implemented [for Statement "ALTER TABLE longdescs ADD COLUMN comment_id serial unique"] at Bugzilla/DB.pm line 374
[Tue Feb 21 14:17:35 2006] checksetup.pl:       Bugzilla::DB::bz_add_column('Bugzilla::DB::Pg=HASH(0xa23dae4)', 'longdescs', 'comment_id', 'HASH(0xa3699f8)') called at ./checksetup.pl line 4274

Basically, Pg 7.x doesn't support what we're trying to do. It can be done, though, I just have to work around it slightly.

Comment 4

[Max Kanat-Alexander]

Attachment: v1 (Fix Pg Backend to be able to add SERIAL types)

Okay, so here's what the problem was:

In PostgreSQL, a SERIAL type is actually translated to: "DEFAULT nextval('$table_$column_seq')" -- but Pg 7.x can't add a column that has a DEFAULT. You have to ALTER TABLE the column to add the default later.

So now for SERIAL types we fix this.

I also noticed that get_add_column_ddl for PostgreSQL didn't support adding a PRIMARY KEY, so I fixed that, too (it was just a small syntax error).

Finally, I dclone($definition), because otherwise the changes in $definition would have been passed back on to the caller and stored in bz_schema (which we don't want).

Comment 5

[Frédéric Buclin]

Comment on attachment 212653 [details] [diff] [review]
v1 (Fix Pg Backend to be able to add SERIAL types)

Tested on both an existing and a new installation. Works fine! r=LpSolit

Comment 6

[Max Kanat-Alexander]

Checking in Bugzilla/DB/Schema/Pg.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Schema/Pg.pm,v  <--  Pg.pm
new revision: 1.10; previous revision: 1.9
done