Re: Passing initially_valid values instead of !skip_validation to StoreRelCheck() in AddRelationNewConstraints()

On 2015/12/09 5:50, Robert Haas wrote:
> On Thu, Dec 3, 2015 at 3:52 AM, amul sul <sul_amul(at)yahoo(dot)co(dot)in> wrote:
>> Can we pass initially_valid instead of !skip_validation to StoreRelCheck() in AddRelationNewConstraints(), as shown below?
>
> The comments say this:
>
> * If skip_validation is true then we skip checking that the existing rows
> * in the table satisfy the constraint, and just install the catalog entries
> * for the constraint. A new FK constraint is marked as valid iff
> * initially_valid is true. (Usually skip_validation and initially_valid
> * are inverses, but we can set both true if the table is known empty.)
>
> These comments are accurate. If you create a FK constraint as not
> valid, the system decides that it's really valid after all, but if you
> add a CHECK constraint as not valid, the system just believes you:
>
> rhaas=# create table foo (a serial primary key);
> CREATE TABLE
> rhaas=# create table bar (a int, foreign key (a) references foo (a)
> not valid, check (a != 0) not valid);
> CREATE TABLE
> rhaas=# \d bar
> Table "public.bar"
> Column | Type | Modifiers
> --------+---------+-----------
> a | integer |
> Check constraints:
> "bar_a_check" CHECK (a <> 0) NOT VALID
> Foreign-key constraints:
> "bar_a_fkey" FOREIGN KEY (a) REFERENCES foo(a)

Didn't realize that marking constraints NOT VALID during table creation
was syntactically possible. Now I see that the same grammar rule for table
constraints is used for both create table and alter table add constraint.

> I suspect this is an oversight. We allowed FOREIGN KEY constraints to
> be not valid in 722bf7017bbe796decc79c1fde03e7a83dae9ada by Simon
> Riggs, but didn't add allow it for CHECK constraints until Alvaro's
> commit of 897795240cfaaed724af2f53ed2c50c9862f951f a few months later.
> My guess is that there's no reason for these not to behave in the same
> way, but they don't. Amul's proposed one-liner might be one part of
> actually fixing that, but it wouldn't be enough by itself: you'd also
> need to teach transformCreateStmt to set the initially_valid flag to
> true, maybe by adding a new function transformCheckConstraints or so.

So, any NOT VALID specification for a FK constraint is effectively
overridden in transformFKConstraints() at table creation time but the same
doesn't happen for CHECK constraints. I agree that that could be fixed,
then as you say, Amul's one-liner would make sense.

Thanks,
Amit