Thread: Altering a table with a rowtype column

On Wed, Mar 7, 2012 at 11:22 AM, Mike Blackwell <mike.blackwell@rrd.com> wrote:
> Given a pair of tables:
>
> create table a (
>   id serial,
>   stuff text,
>   more_stuff text
> );
>
> create table a_audit (
>   id serial,
>   old_record a,
>   new_record a
> );
>
> How can one alter the structure of table a?  Attempting
>
> ALTER TABLE a ADD COLUMN even_more_stuff text;
>
> results in the message:
>
>  ERROR:  cannot alter table "a" because column "a_audit.new_record" uses its
> row type
>
> A solution that doesn't lose the existing data is preferable.

works for me -- what version are you on?

merlin

On Wed, Mar 7, 2012 at 11:45 AM, Mike Blackwell <mike.blackwell@rrd.com> wrote:
>
> works for me -- what version are you on?
>
> merlin
>
> ------
>
> [wcs1459@aclnx-cisp01 ~]$ psql --version
> psql (PostgreSQL) 9.1.1
> contains support for command-line editing
>
>
> [wcs1459@aclnx-cisp01 ~]$ cat x
> create table a (
>   id serial,
>   stuff text,
>   more_stuff text
> );
>
> create table a_audit (
>   id serial,
>   a_old a,
>   a_new a
> );
>
> alter table a add column even_more_stuff boolean not null default false;
>
>
> [wcs1459@aclnx-cisp01 ~]$ psql -f x
> psql:x:5: NOTICE:  CREATE TABLE will create implicit sequence "a_id_seq" for
> serial column "a.id"
> CREATE TABLE
> psql:x:11: NOTICE:  CREATE TABLE will create implicit sequence
> "a_audit_id_seq" for serial column "a_audit.id"
> CREATE TABLE
> psql:x:13: ERROR:  cannot alter table "a" because column "a_audit.a_new"
> uses its row type

aha! that's not what you posted last time.  you appended 'not null
default false'; which inexplicably breaks the ALTER.

try this:
ALTER TABLE a ADD COLUMN even_more_stuff text not null;
ALTER TABLE a ALTER even_more_stuff set default false;
ALTER TABLE a DROP COLUMN even_more_stuff;
ALTER TABLE a ADD COLUMN even_more_stuff boolean not null default false;

(this really looks like a bug in postgres, cc-ing to bugs)

merlin

On Wed, Mar 7, 2012 at 1:17 PM, Mike Blackwell <mike.blackwell@rrd.com> wrote:
> As a followup, the workaround fails if there is data in the source table due
> to the initial null value placed in the existing data rows.
>
> [wcs1459@aclnx-cisp01 ~]$ psql --port=5433 -e -f x
> begin;
> BEGIN
> create table a (
>   id serial,
>   stuff text,
>   more_stuff text
> );
> psql:x:6: NOTICE:  CREATE TABLE will create implicit sequence "a_id_seq" for
> ser
>     ial column "a.id"
> CREATE TABLE
> create table a_audit (
>   id serial,
>   a_old a,
>   a_new a
> );
> psql:x:12: NOTICE:  CREATE TABLE will create implicit sequence
> "a_audit_id_seq"
>                  for serial column "a_audit.id"
> CREATE TABLE
> insert into a (stuff, more_stuff) values ('some', 'thing');
> INSERT 0 1
> ALTER TABLE a ADD COLUMN even_more_stuff boolean not null;
> psql:x:17: ERROR:  column "even_more_stuff" contains null values
> ALTER TABLE a ALTER even_more_stuff set default false;
> psql:x:18: ERROR:  current transaction is aborted, commands ignored until
> end of
>        transaction block
> ALTER TABLE a DROP COLUMN even_more_stuff;
> psql:x:19: ERROR:  current transaction is aborted, commands ignored until
> end of
>        transaction block
> ALTER TABLE a ADD COLUMN even_more_stuff boolean not null default false;
> psql:x:20: ERROR:  current transaction is aborted, commands ignored until
> end of
>        transaction block
> rollback;
> ROLLBACK

yup (please respond to the list) -- you can workaround the workaround
by UPDATEing the table to set the field before applying the not null
bit.  Note that if you did this, the foreign table containing the type
would have the new column all as null.

IMO, the server is being too strict on the dependency check.  Perhaps
there are some defenses here that are an early form of trying to get
field constraints to pass through to the foreign column, or it's just
a plain old bug.  I took a quick look at tablecmds.c to see if I could
find an easy fix, but it wasn't clear why the default was forcing an
dependency error and I punted.

merlin

Merlin Moncure <mmoncure@gmail.com> writes:
> On Wed, Mar 7, 2012 at 11:45 AM, Mike Blackwell <mike.blackwell@rrd.com> wrote:
>> alter table a add column even_more_stuff boolean not null default false;

> aha! that's not what you posted last time.  you appended 'not null
> default false'; which inexplicably breaks the ALTER.

> try this:
> ALTER TABLE a ADD COLUMN even_more_stuff text not null;
> ALTER TABLE a ALTER even_more_stuff set default false;
> ALTER TABLE a DROP COLUMN even_more_stuff;
> ALTER TABLE a ADD COLUMN even_more_stuff boolean not null default false;

> (this really looks like a bug in postgres, cc-ing to bugs)

It is not a bug.  The ALTER ADD ... DEFAULT ... form implies rewriting
every existing tuple of the rowtype to insert a non-null value in the
added column, and we don't have support for doing that to rowtype
columns, only to the target table and descendants.  Without a default,
it's just a catalog adjustment and doesn't involve rewriting any data.
(This stems from the fact that columns beyond a tuple's natts value are
presumed null, so we can let ADD COLUMN without a default just change
the catalogs and a null column effectively springs into existence for
every existing tuple.  ALTER ADD ... DEFAULT is specified to have a
different result, and it's not free.)

This probably could be done for rowtype columns as well, but nobody has
collected the necessary round tuits.  I think there was some fear of
locking/deadlock issues, too.

            regards, tom lane

Excerpts from Tom Lane's message of mié mar 07 17:31:32 -0300 2012:

> This probably could be done for rowtype columns as well, but nobody has
> collected the necessary round tuits.  I think there was some fear of
> locking/deadlock issues, too.

It's probably easy to do if you require it to be marked INVALID
initially and then validate the tables using it one by one.

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

On Wed, Mar 7, 2012 at 2:31 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Merlin Moncure <mmoncure@gmail.com> writes:
>> On Wed, Mar 7, 2012 at 11:45 AM, Mike Blackwell <mike.blackwell@rrd.com> wrote:
>>> alter table a add column even_more_stuff boolean not null default false;
>
>> aha! that's not what you posted last time.  you appended 'not null
>> default false'; which inexplicably breaks the ALTER.
>
>> try this:
>> ALTER TABLE a ADD COLUMN even_more_stuff text not null;
>> ALTER TABLE a ALTER even_more_stuff set default false;
>> ALTER TABLE a DROP COLUMN even_more_stuff;
>> ALTER TABLE a ADD COLUMN even_more_stuff boolean not null default false;
>
>> (this really looks like a bug in postgres, cc-ing to bugs)
>
> It is not a bug.  The ALTER ADD ... DEFAULT ... form implies rewriting
> every existing tuple of the rowtype to insert a non-null value in the
> added column, and we don't have support for doing that to rowtype
> columns, only to the target table and descendants.

I'm not buying that..it implies no such thing.  In particular, for
table-as-rowtype columns, there's no way that I can see to have
default values be generated.  So why does it follow that the dependent
table has to be rewritten?  Column constraints are not enforced on the
rowtype, so it follows that default shouldn't be either considering
there's no way to get the default to fire.  Composite type (or table
based composite) defaults are applied to the composite as a whole, not
to specific fields.

On a practical level, the error blocks nothing -- you can bypass it
trivially.   It's just an annoyance that prevents things that users
would like to be able to do with table row types.  So I'd argue to
remove the check, although I can kinda see the argument that it's not
a bug unless the check was recently introduced so that it broke older
code.

merlin

On Wed, Mar 7, 2012 at 2:49 PM, Merlin Moncure <mmoncure@gmail.com> wrote:
> On a practical level, the error blocks nothing -- you can bypass it
> trivially.   It's just an annoyance that prevents things that users
> would like to be able to do with table row types.  So I'd argue to
> remove the check, although I can kinda see the argument that it's not
> a bug unless the check was recently introduced so that it broke older
> code.

The behavior hasn't changed since at least as far back as 8.1, so
you're correct (once again) -- not a bug.  I'm really surprised I
haven't already bumped into this.  I usually don't mix
tables-as-storage with tables-as-composites though.

Mike, on 9.1, you'll probably get more mileage out of using the hstore
type for row storage if you want to do auditing in that style.

merlin

Mike

On Wed, Mar 7, 2012 at 3:49 PM, Merlin Moncure <mmoncure@gmail.com> wrote:
> On Wed, Mar 7, 2012 at 2:31 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Merlin Moncure <mmoncure@gmail.com> writes:
>>> On Wed, Mar 7, 2012 at 11:45 AM, Mike Blackwell <mike.blackwell@rrd.com> wrote:
>>>> alter table a add column even_more_stuff boolean not null default false;
>>
>>> aha! that's not what you posted last time.  you appended 'not null
>>> default false'; which inexplicably breaks the ALTER.
>>
>>> try this:
>>> ALTER TABLE a ADD COLUMN even_more_stuff text not null;
>>> ALTER TABLE a ALTER even_more_stuff set default false;
>>> ALTER TABLE a DROP COLUMN even_more_stuff;
>>> ALTER TABLE a ADD COLUMN even_more_stuff boolean not null default false;
>>
>>> (this really looks like a bug in postgres, cc-ing to bugs)
>>
>> It is not a bug.  The ALTER ADD ... DEFAULT ... form implies rewriting
>> every existing tuple of the rowtype to insert a non-null value in the
>> added column, and we don't have support for doing that to rowtype
>> columns, only to the target table and descendants.
>
> I'm not buying that..it implies no such thing.  In particular, for
> table-as-rowtype columns, there's no way that I can see to have
> default values be generated.  So why does it follow that the dependent
> table has to be rewritten?  Column constraints are not enforced on the
> rowtype, so it follows that default shouldn't be either considering
> there's no way to get the default to fire.  Composite type (or table
> based composite) defaults are applied to the composite as a whole, not
> to specific fields.

I think Tom's correct about what the right behavior would be if
composite types supported defaults, but they don't, never have, and
maybe never will.  I had a previous argument about this with Tom, and
lost, though I am not sure that anyone other than Tom thinks that the
current behavior is for the best.  But see commits
a06e41deebdf74b8b5109329dc75b2e9d9057962 and
a40b1e0bf32b1da46c1baa9bc7da87f207cd37d8.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Mar 7, 2012 at 3:49 PM, Merlin Moncure <mmoncure@gmail.com> wrote:
>> On Wed, Mar 7, 2012 at 2:31 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> It is not a bug.  The ALTER ADD ... DEFAULT ... form implies rewriting
>>> every existing tuple of the rowtype to insert a non-null value in the
>>> added column, and we don't have support for doing that to rowtype
>>> columns, only to the target table and descendants.

>> I'm not buying that..it implies no such thing.  In particular, for
>> table-as-rowtype columns, there's no way that I can see to have
>> default values be generated.  So why does it follow that the dependent
>> table has to be rewritten?  Column constraints are not enforced on the
>> rowtype, so it follows that default shouldn't be either considering
>> there's no way to get the default to fire.  Composite type (or table
>> based composite) defaults are applied to the composite as a whole, not
>> to specific fields.

> I think Tom's correct about what the right behavior would be if
> composite types supported defaults, but they don't, never have, and
> maybe never will.  I had a previous argument about this with Tom, and
> lost, though I am not sure that anyone other than Tom thinks that the
> current behavior is for the best.

Um, did I say I thought it was for the best?  I thought I said we don't
have support for doing better.

If we are willing to legislate that column defaults are not and never
will be applied to composite types, then I think Merlin might be right
that we could just let an ALTER ADD with DEFAULT ignore the existence of
composite columns.  I'd always figured that we'd want to try to fix that
omission eventually, though.

> But see commits
> a06e41deebdf74b8b5109329dc75b2e9d9057962 and
> a40b1e0bf32b1da46c1baa9bc7da87f207cd37d8.

Note that the actual problem with the original commit was that it
depended on a misreading of the SQL standard.  Per spec, ALTER ADD with
DEFAULT is *not* the same thing as ALTER ADD followed by ALTER SET
DEFAULT; the contents of the table end up different.

            regards, tom lane

On Tue, Mar 20, 2012 at 12:16 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> I think Tom's correct about what the right behavior would be if
> composite types supported defaults, but they don't, never have, and
> maybe never will.  I had a previous argument about this with Tom, and
> lost, though I am not sure that anyone other than Tom thinks that the
> current behavior is for the best.  But see commits
> a06e41deebdf74b8b5109329dc75b2e9d9057962 and
> a40b1e0bf32b1da46c1baa9bc7da87f207cd37d8.

I'll go further than that -- given the current infrastructure I'd say
that composite type defaults are not very well defined or useful
besides not being implemented. The way things work now:

create type foo as(a int, b int);
create table bar(f foo default row(1,2));

works perfectly ok.  how would you proxy the default from one of those
two columns?  does it make sense to do so?  defaults are applied to
table columns, not to types (you could argue that domains violate that
rule but IMO it's not the same thing).

type constraints are another matter.  this would be useful and
valuable but may end up being impossible to add for a lot of reasons
such as backwards compatibility and dealing with the standard's lack
(implemented nowhere in postgres except for the very special case of
IS NULL) of distinguishing between the type itself being null and it's
fields being null (making type constraints smack into plpgsql variable
declarations).

merlin

On Tue, Mar 20, 2012 at 2:48 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I think Tom's correct about what the right behavior would be if
>> composite types supported defaults, but they don't, never have, and
>> maybe never will.  I had a previous argument about this with Tom, and
>> lost, though I am not sure that anyone other than Tom thinks that the
>> current behavior is for the best.
>
> Um, did I say I thought it was for the best?  I thought I said we don't
> have support for doing better.
>
> If we are willing to legislate that column defaults are not and never
> will be applied to composite types, then I think Merlin might be right
> that we could just let an ALTER ADD with DEFAULT ignore the existence of
> composite columns.

I tend to think that's exactly what we should do, and it's what that
patch did, although as you point out my commit message was the product
of confused thinking.

> I'd always figured that we'd want to try to fix that
> omission eventually, though.

It's mildly tempting, but as Merlin points out, it's hard to know
exactly when you'd apply those rules.  We talked a while back about
domains with NOT NULL constraints; if someone does a left join with a
domain-typed column on the outer side, what are you going to put there
if you don't put NULL?  This case seems somewhat similar.  Defaults
make sense when applied to table columns, because the semantics are
clear: columns not explicitly mentioned get their default value if
any, else NULL.  But if we rule that a composite type with no default
gets the composite type's default values for each column, then we're
overriding the general SQL presumption that unspecified columns are
NULL.  And similarly for temps created by uninitialized variables or,
worse, LEFT JOINs.  In languages like C++ or even Perl, there's always
a very clear notion of when an object gets created, and constructors
and so on run at that time.  Defaults logically should run at the same
time that a constructor would, but that concept doesn't really exist
in SQL, which is seemingly deliberately quite murky about when values
spring into existence.

Does the SQL standard say anything on this topic?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company