There is still one problem left preventing entry of a new bug on Postgres: the resolution column does not have any default value, is declared as NOT NULL, and at bug creation time, we are not passing any value. I think that there is no need to specify the value at creation time (the bug should never have any resolution when created), so default value '' (empty string) in the DB is probably appropriate.

Comment on attachment 178309 [details] [diff] [review] V1 Due to a bug in bz_change_field_type, I think that change will run every time you run checksetup. Try it out, though, and see.

(In reply to comment #2) > (From update of attachment 178309 [details] [diff] [review] [edit]) > Due to a bug in bz_change_field_type, I think that change will run every time > you run checksetup. Try it out, though, and see. > Hmmm, what is the bug in bz_change_field_type about? Wouldn't it be easier to fix that bug first (if possible), than working around it?

You can try. I think the bug is that it doesn't take defaults into account. I didn't fix the bug because I'm going to eliminate that function very soon. If you'd like, we can just hold off on this bug until bug 285111 is resolved, maybe. Or you can check this in, and then we can live with it for a few days until we have a later change to wrap it in (like how I did it for the other default changes).

Comment on attachment 178309 [details] [diff] [review] V1 Ok, I'll wait.

"If it's not a regression from 2.18 and it's not a critical problem with something that's already landed, let's push it off." - Dave

Actually, the dependecy is on 285748, not on 285111.

Well, here is updated version which seems to work correctly, as far as I was able to test.

Comment on attachment 181739 [details] [diff] [review] V2 Yeah, that will work, but I'd be more comfortable doing it in chronological sequence at the bottom, instead of up there.

(In reply to comment #9) > (From update of attachment 181739 [details] [diff] [review] [edit]) > Yeah, that will work, but I'd be more comfortable doing it in chronological > sequence at the bottom, instead of up there. > Well, that's not so easy. If you just add the line changing the definition at the end, you end up with changing the type back and forth on every execution of checksetup (been there, done that). Or, you can remove the line I modified and add the new one at the end - then it's bit confusing why all the conversions from enum to tables are not at the same place. So the approach I took seems to be most sensible to me. What do you recommend, Max?

(In reply to comment #10) > What do you recommend, Max? Put it at the bottom, but wrap it in an "if !exists $dbh->bz_column_info(blah)->{DEFAULT}"

Comment on attachment 181739 [details] [diff] [review] V2 OK, I looked at checksetup a bit more, and I've decided that this is actually OK. It would be a big pain to go back and wrap stuff in the if statements, and there are no bugs.resolution schema changes between this one and now. The column never even gets touched. So this is fine.

Checking in checksetup.pl; /cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v <-- checksetup.pl new revision: 1.399; previous revision: 1.398 done Checking in Bugzilla/DB/Schema.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Schema.pm,v <-- Schema.pm new revision: 1.27; previous revision: 1.26 done