ALTER TABLE with multiple SET NOT NULL

Started by Allison Kapturabout 7 years ago3 messages
#1Allison Kaptur
allison.kaptur@gmail.com

Hi folks,

I encountered a surprising error when writing a migration that both added a
primary key to a table and added a new NOT NULL column. It threw the error "
column "col_d" contains null values", even though I supplied a default. The
migration looks like this:
CREATE TABLE new_table AS SELECT col_a, col_b, col_c from existing_table;
ALTER TABLE new_table
ADD COLUMN col_d UUID UNIQUE NOT NULL DEFAULT uuid_generate_v4(),
ADD PRIMARY KEY (col_a, col_b, col_c);

Because of the `DEFAULT uuid_generate_v4()`, I wouldn't expect it to be
possible for the new column to have null values, so I was surprised to get
an integrity error with the message "column "col_d" contains null values".

I found two workarounds that don't produce the error. First, if I instead
set the NOT NULL last, I get no error:
ALTER TABLE new_table
ADD COLUMN col_d UUID UNIQUE DEFAULT uuid_generate_v4(),
ADD PRIMARY KEY (col_a, col_b, col_c),
ALTER COLUMN col_d SET NOT NULL;

Second, if I do the two steps in two ALTER TABLE statements, I also get no
error.
ALTER TABLE new_table
ADD COLUMN col_d UUID UNIQUE NOT NULL DEFAULT uuid_generate_v4();
ALTER TABLE new_table
ADD PRIMARY KEY (col_a, col_b, col_c);

I'm running postgres 9.6.2.

I know that adding a column with a default requires the table & its indexes
to be rewritten, and I know that adding a primary key on a column that
doesn't have an existing NOT NULL constraint does ALTER COLUMN SET NOT NULL
on each column in the primary key. So I'm wondering if Postgres is
reordering the SET NOT NULL operations in a way that causes it to attempt
setting col_d to NOT NULL before the default values are supplied.

My understanding from the docs is that I should be able to combine any
ALTER TABLE statements into one if they don't involve RENAME or SET SCHEMA
(or a few other things in v10, which I'm not using).

So my questions are:
- Is there a way I can see what Postgres is doing under the hood? I wanted
to use EXPLAIN ANALYZE but it doesn't appear to work on alter table
statements.
- Am I missing something about my original migration, or is there a reason
I shouldn't expect it to work?

Thanks,
Allison Kaptur

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Allison Kaptur (#1)
Re: ALTER TABLE with multiple SET NOT NULL

Allison Kaptur <allison.kaptur@gmail.com> writes:

I encountered a surprising error when writing a migration that both added a
primary key to a table and added a new NOT NULL column. It threw the error "
column "col_d" contains null values", even though I supplied a default. The
migration looks like this:
CREATE TABLE new_table AS SELECT col_a, col_b, col_c from existing_table;
ALTER TABLE new_table
ADD COLUMN col_d UUID UNIQUE NOT NULL DEFAULT uuid_generate_v4(),
ADD PRIMARY KEY (col_a, col_b, col_c);

Hm, this can be made a good deal more self-contained:

regression=# create table t1 (a int);
CREATE TABLE
regression=# insert into t1 values(1);
INSERT 0 1
regression=# alter table t1 add column b float8 not null default random(),
add primary key(a);
ERROR: column "b" contains null values

It fails like that as far back as I tried (8.4). I'm guessing that we're
doing the ALTER steps in the wrong order, but haven't looked closer than
that.

Interestingly, in v11 and HEAD it works if you use a constant default,
suggesting that the fast-default feature is at least adjacent to the
problem.

regards, tom lane

In reply to: Tom Lane (#2)
Re: ALTER TABLE with multiple SET NOT NULL

Hello

I investigate this bug and found reason:

alter table t1 add column b float8 not null default random(), add primary key(a);

Here we call ATController (src/backend/commands/tablecmds.c) with two cmds: AT_AddColumn and AT_AddIndex
Then we go to phase 2 in ATRewriteCatalogs:
- succesful add new attribute, but without table rewrite - it will be later in phase 3
- call ATExecAddIndex, we want add primary key, so we call index_check_primary_key.
index_check_primary_key call AlterTableInternal and therefore another ATController with independent one AT_SetNotNull command.
ATController will call phase 2, and then its own phase 3 with validation all constraints. But at this nested level we have no AlteredTableInfo->newvals and we do not proper transform tuple.

not sure how we can proper rewrite this case.

regards, Sergei