Re: WITH CHECK OPTION for auto-updatable views

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: WITH CHECK OPTION for auto-updatable views
Date: 2013-06-22 06:24:01
Message-ID: 20130622062401.GD7093@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Dean,

* Dean Rasheed (dean(dot)a(dot)rasheed(at)gmail(dot)com) wrote:
> Here's an updated version --- I missed the necessary update to the
> check_option column of information_schema.views.

Thanks! This is really looking quite good, but it's a bit late and I'm
going on vacation tomorrow, so I didn't quite want to commit it yet. :)
Instead, here are a few things that I'd like to see fixed up:

I could word-smith the docs all day, most likely, but at least the
following would be nice to have cleaned up:

- 'This is parameter may be either'

- I don't like "This allows an existing view's ...". The option can be
used on CREATE VIEW as well as ALTER VIEW. I'd say something like:

This parameter may be either <literal>local</> or
<literal>cascaded</>, and is equivalent to specifying <literal>WITH [
CASCADED | LOCAL ] CHECK OPTION</> (see below). This option can be
changed on existing views using <xref linkend="sql-alterview">.

- wrt what shows up in '\h create view' and '\h alter view', I think we
should go ahead and add in with the options are, ala EXPLAIN. That
avoids having to guess at it (I was trying 'with_check_option'
initially :).

- Supposedly, this option isn't available for RECURSIVE views, but it's
happily accepted:

=*# create recursive view qq (a) with (check_option = local) as select z from q;
CREATE VIEW

(same is true of ALTER VIEW on a RECURSIVE view)

- pg_dump support is there, but it outputs the definition using the PG
syntax instead of the SQL syntax; is there any particular reason for
this..? imv, we should be dumping SQL spec where we can trivially
do so.

- Why check_option_offset instead of simply check_option..? We don't
have security_barrier_offset and it seems like we should be
consistent there.

The rest looks pretty good to me. If you can fix the above, I'll review
again and would be happy to commit it. :)

Thanks!

Stephen

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Stephen Frost 2013-06-22 07:46:49 Re: MemoryContextAllocHuge(): selectively bypassing MaxAllocSize
Previous Message Amit kapila 2013-06-22 04:39:22 Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]