On 4 August 2011 09:23, Nikhil Sontakke <nikkhils@gmail.com> wrote:
>> So after writing the code to handle named NOT NULL constraints for
>> tables, I'm thinking that dumpConstraints needs to be fixed thusly:
>>
>> @@ -12888,6 +12968,27 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
>> NULL, NULL);
>> }
>> }
>> + else if (coninfo->contype == 'n' && tbinfo)
>> + {
>> + /* NOT NULL constraint on a table */
>> + if (coninfo->separate)
>> + {
>> + write_msg(NULL, "NOT NULL constraints cannot be dumped separately from their owning table\n");
>> + exit_nicely();
>> + }
>> + }
>> + else if (coninfo->contype == 'n' && tbinfo == NULL)
>> + {
>> + /* NOT NULL constraint on a domain */
>> + TypeInfo *tyinfo = coninfo->condomain;
>> +
>> + /* Ignore if not to be dumped separately */
>> + if (coninfo->separate)
>> + {
>> + write_msg(NULL, "NOT NULL constraints cannot be dumped separately from their owning domain\n");
>> + exit_nicely();
>> + }
>> + }
>> else
>> {
>> write_msg(NULL, "unrecognized constraint type: %c\n", coninfo->contype);
>>
>
> Some nit-picking.
>
> AFAICS above, we seem to be only using 'tbinfo' to identify the object
> type here - 'table' visavis 'domain'. We could probably reduce the
> above two elses to a single one and use the check of tbinfo being not
> null to decide which object type name to spit out..
>
> Although, it's difficult to see how we could end up marking NOT NULL
> constraints as 'separate' ever. So this code will be rarely exercised,
> if ever IMO.
>
There's a related issue that might affect how this code ends up. I'm
not sure if this has been discussed before, but it seems to be a
problem for CHECK constraints currently, and will affect NOT NULL in
the same way - if the constraint is NOT VALID, and some of the
existing data violates the constraint, then pg_dump needs to dump the
constraint separately, after the table's data, otherwise the restore
will fail.
So it looks like this code will have to support dumping NOT NULLs
ultimately anyway.
BTW, this happens automatically for FK constraints, so I don't think
this is a problem for 9.1.
Regards,
Dean
