On Sun, Apr 10, 2011 at 11:19:26AM -0400, Robert Haas wrote:
> On Sun, Apr 10, 2011 at 6:36 AM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> > On Sun, Apr 03, 2011 at 09:53:57PM -0400, Robert Haas wrote:
> >> On Fri, Apr 1, 2011 at 12:56 AM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> >> > On Thu, Mar 31, 2011 at 11:11:49AM -0400, Robert Haas wrote:
> >> >> On Thu, Mar 31, 2011 at 6:06 AM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> >> >> > The best way I can see is to make ATExecAddColumn more like ATExecDropColumn,
> >> >> > ATAddCheckConstraint, and ATExecDropConstraint. ?Namely, recurse at Exec-time
> >> >> > rather than Prep-time, and cease recursing when we satisfy the ADD COLUMN with a
> >> >> > merge. ?Did you have something else in mind?
> >> >>
> >> >> I had exactly what you just said in mind.
> >> >
> >> > Patch attached, then.
> >>
> >> Committed.
> >
> > Thanks. ?This turns out to have caused that TOAST creation regression:
> >
> > On Fri, Apr 08, 2011 at 08:12:19PM -0400, Noah Misch wrote:
> > [pg_upgrade/typed table business]
> >> I also tested a regular dump+reload of the regression database, and a pg_upgrade
> >> of the same. ?The latter failed further along, due (indirectly) to this failure
> >> to create a TOAST table:
> >>
> >> ? create table p ();
> >> ? create table ch () inherits (p);
> >> ? alter table p add column a text;
> >> ? select oid::regclass,reltoastrelid from pg_class where oid::regclass IN ('p','ch');
> >> ? insert into ch values (repeat('x', 1000000));
> >>
> >> If I "drop table a_star cascade" in the regression database before attempting
> >> pg_upgrade, it completes cleanly.
> >
> > Since ATExecAddColumn now handles the recursion, child table work queue entries
> > no longer have AT_PASS_ADD_COL subcommands. ?Consequently, this heuristic in
> > ATRewriteCatalogs skips over them:
> >
> > ? ? ? ? ? ? ? ?if (tab->relkind == RELKIND_RELATION &&
> > ? ? ? ? ? ? ? ? ? ? ? ?(tab->subcmds[AT_PASS_ADD_COL] ||
> > ? ? ? ? ? ? ? ? ? ? ? ? tab->subcmds[AT_PASS_ALTER_TYPE] ||
> > ? ? ? ? ? ? ? ? ? ? ? ? tab->subcmds[AT_PASS_COL_ATTRS]))
> > ? ? ? ? ? ? ? ? ? ? ? ?AlterTableCreateToastTable(tab->relid, (Datum) 0);
> >
> > SET STORAGE uses AT_PASS_MISC, so this test case also omits a TOAST table:
> >
> > ?set client_min_messages = debug1; -- display toast creation
> > ?create table t (a text); -- makes toast
> > ?alter table t alter a set storage plain;
> > ?alter table t add b int default 0; -- rewrite the table - no toast
> > ?alter table t alter a set storage extended; -- no toast added?
> > ?insert into t (a) values (repeat('x', 1000000)); -- fails
>
> Checking my understanding here, the first of these is a regression
> introduced by the patch for $SUBJECT, but the second one is a
> pre-existing bug. Is that right?
The patch for $SUBJECT just introduced the first regression; commit
04e17bae50a73af524731fa11210d5c3f7d8e1f9 introduced the second regression near
the beginning of the 9.1 development cycle.