Re: BUG #16038: Alter table - SegFault

Hi,

On 2019-10-04 08:26:24 -0700, Andres Freund wrote:
> On 2019-10-04 11:43:52 -0300, Alvaro Herrera wrote:
> > On 2019-Oct-04, PG Bug reporting form wrote:
> >
> > > alter table test.testa
> > > add column idb numeric(10,0) NOT NULL DEFAULT nextval('test.sq_testb'),
> > > add column fk_tmpb varchar(20);
> > >
> > > server process (PID 21884) was terminated by signal 11: Segmentation fault
> > > - empty table - alter goes ok , split alter into 2 add col - works too

Thanks for the bug report! I've pushed a fix, which will be included in
the next minor release. If you need a workaround until then, you can
avoid the crash by only adding one column add a time.

> > Hmm, confirmed, here's the stack trace:
>
> The trailing attributes in the new slot in AtRewriteTable() aren't
> necessarily set to isnull=true. So to trigger the problem, one needs a
> rewrite triggered by *another* column (otherwise we'll not hit this
> path), combined with a new column that doesn't have a default value. I
> think that's probably my bug.
>
> This seems to fix the problem:
>
> diff --git i/src/backend/commands/tablecmds.c w/src/backend/commands/tablecmds.c
> index 05593f33162..6f72b08a87d 100644
> --- i/src/backend/commands/tablecmds.c
> +++ w/src/backend/commands/tablecmds.c
> @@ -4890,6 +4890,14 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
> table_slot_callbacks(oldrel));
> newslot = MakeSingleTupleTableSlot(newTupDesc,
> table_slot_callbacks(newrel));
> +
> + /*
> + * Set all columns in the new slot to NULL initially, to ensure
> + * columns added as part of the rewrite are initialized to
> + * NULL. That's necessary as tab->newvals will not contain an
> + * expression for columns with a NULL default.
> + */
> + ExecStoreAllNullTuple(newslot);
> }

Pushed, after adding some tests.

Greetings,

Andres Freund