Re: Thinking about WITH CHECK OPTION for views

On 20 January 2013 16:30, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> Dean,
>
> * Dean Rasheed (dean(dot)a(dot)rasheed(at)gmail(dot)com) wrote:
>> I've been thinking about WITH CHECK OPTION for auto-updatable views.
>> Given the timing I doubt if this will be ready for 9.3, since I only
>> get occasional evenings and weekends to hack on postgres, but I think
>> it's probably worth kicking off a discussion, starting with a
>> description of what the feature actually is.
>
> If this isn't intended for 9.3, I think it'd be good to move it to the
> post-9.3 commitfest.

Yes, I was thinking that myself. It doesn't look like I will get much
time to work on postgresql during this commitfest, so I think it would
be better spent reviewing other patches rather than trying to squeeze
this in so late.

That said, it actually looks pretty decent to me
> on first blush. I did have a couple of comments though:
>
> Why have validateWithCheckOption instead of handling that in gram.y..?
>

Yes gram.y does validate it if you use the standard SQL syntax. But,
since I've used a new rel_option to store the constraint, it is also
possible to specify it using the non-standard syntax "WITH
(check_option=local|cascaded)", so validateWithCheckOption is needed
to validate that.

> If the SQL spec says we should be disallowing WITH CHECK when there are
> INSTEAD rules or triggers, could we actually do that..? This kind of an
> error:
>
> ERROR: attribute number 2 exceeds number of columns 1
>
> is really pretty bad, any chance we could improve that or disallow the
> possibility of getting there by erroring earlier on?
>

Yes, I think that's the biggest missing piece from the patch as it stands.

I put the tests in so that I could see how it goes wrong if we don't
disallow triggers or rules on the underlying relations. AFAICT
triggers don't actually lead to any errors, but the check will be
silently ignored which is not good. Rules are worse, not only because
they lead to errors like the above, but also because it looks like
they need to also be disallowed on all underlying relations even if
the view only has the LOCAL CHECK OPTION. Triggers are allowed
provided they are not defined on the view with the check option, or
one for which a check option cascades down from another view.

Thanks for looking at it. I'll move it 9.4 CF-1.

Regards,
Dean