Re: [v9.2] Fix leaky-view problem, part 1 - Mailing list pgsql-hackers

From Noah Misch
Subject Re: [v9.2] Fix leaky-view problem, part 1
Date
Msg-id 20110706234007.GB1840@tornado.leadboat.com
Whole thread Raw
In response to Re: [v9.2] Fix leaky-view problem, part 1  (Kohei KaiGai <kaigai@kaigai.gr.jp>)
Responses Re: [v9.2] Fix leaky-view problem, part 1
List pgsql-hackers
On Wed, Jul 06, 2011 at 10:25:12PM +0200, Kohei KaiGai wrote:
> *** a/src/backend/commands/view.c
> --- b/src/backend/commands/view.c

> --- 227,257 ----
>                   atcmd->def = (Node *) lfirst(c);
>                   atcmds = lappend(atcmds, atcmd);
>               }
>           }
>   
>           /*
> +          * If optional parameters are specified, we must set options
> +          * using ALTER TABLE SET OPTION internally.
> +          */
> +         if (list_length(options) > 0)
> +         {
> +             atcmd = makeNode(AlterTableCmd);
> +             atcmd->subtype = AT_SetRelOptions;
> +             atcmd->def = (List *)options;
> + 
> +             atcmds = lappend(atcmds, atcmd);
> +         }
> +         else
> +         {
> +             atcmd = makeNode(AlterTableCmd);
> +             atcmd->subtype = AT_ResetRelOptions;
> +             atcmd->def = (Node *) list_make1(makeDefElem("security_barrier",
> +                                                          NULL));
> +         }
> +         if (atcmds != NIL)
> +             AlterTableInternal(viewOid, atcmds, true);
> + 
> +         /*
>            * Seems okay, so return the OID of the pre-existing view.
>            */
>           relation_close(rel, NoLock);    /* keep the lock! */

That gets the job done for today, but DefineVirtualRelation() should not need
to know all view options by name to simply replace the existing list with a
new one.  I don't think you can cleanly use the ALTER TABLE SET/RESET code for
this.  Instead, compute an option list similar to how DefineRelation() does so
at tablecmds.c:491, then update pg_class.

> 2011/7/5 Noah Misch <noah@2ndquadrant.com>:
> > On Sun, Jul 03, 2011 at 11:33:38AM +0200, Kohei KaiGai wrote:
> >> --- a/src/test/regress/sql/select_views.sql
> >> +++ b/src/test/regress/sql/select_views.sql

> >> +-- cleanups
> >> +DROP ROLE IF EXISTS alice;
> >> +DROP FUNCTION IF EXISTS f_leak(text);
> >> +DROP TABLE IF EXISTS credit_cards CASCADE;
> >
> > Keep the view around.  That way, pg_dump tests of the regression database will
> > test the dumping of this view option.  (Your pg_dump support for this feature
> > does work fine, though.)

The latest version of part 1 still drops everything here.


pgsql-hackers by date:

Previous
From: Craig Ringer
Date:
Subject: Re: Crash dumps
Next
From: Brar Piening
Date:
Subject: Re: Review of VS 2010 support patches