Re: cataloguing NOT NULL constraints

On 3 August 2011 04:40, Alvaro Herrera <alvherre(at)commandprompt(dot)com> wrote:
> Excerpts from Dean Rasheed's message of sáb jul 30 18:40:46 -0400 2011:
>
>> Looks pretty good to me (not too dirty). I suppose given that the
>> parser transforms AT_ColumnConstraint into one of the existing command
>> subtypes, you could just have gram.y emit an AT_AddConstraint with the
>> ColumnDef attached, to save adding a new subtype, but there's probably
>> not much difference.
>
> Thanks.  I've done the other changes you suggested, but I don't see that
> it's desirable to have gram.y emit AT_AddConstraint directly.  It seems
> cleaner to be able to turn a NOT NULL constraint into AT_SetNotNull
> in parse_utilcmd instead.

I wasn't proposing getting rid of that bit in parse_utilcmd. I just
meant move the block that calls transformColumnConstraints() to the
AT_AddConstraint case in transformAlterTableStmt(). So it would test
if it has a ColumnDef attached, and either process a table constraint
or a set of column constraints. That would avoid the need for a new
command subtype, and so the changes to tablecmds.c would not be
needed. I think it would also avoid the need to add colname to the
Constraint struct, so none of the parsenodes.h changes would be needed
either.

 (Maybe I'll have to bite the bullet and make
> AT_AddConstraint work for not null constraints as well, as part of the
> larger patch.  Not sure.)  Currently, the table constraint syntax only
> lets you do a single constraint at a time, but you can do multiple
> constraints with the column constraint syntax.  I am not sure how hard
> it is to rework the grammar so that only a single constraint is
> allowed, but I'm not sure that it's worth the trouble either.
>
> Attached is an updated version, touching the docs and adding a new
> simple regression test.
>
> But ... I just noticed that I need to touch ALTER DOMAIN in a similar
> way as well.

Oh I keep forgetting about domains. But after a quick glance at the
docs, doesn't it already have syntax for this?

Regards,
Dean