Re: [v9.2] Fix leaky-view problem, part 1

Noah Misch <noah@2ndquadrant.com>

From: Noah Misch <noah@2ndQuadrant.com>
To: Kohei KaiGai <kaigai@kaigai.gr.jp>
Cc: Robert Haas <robertmhaas@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Kohei Kaigai <Kohei.Kaigai@emea.nec.com>, Stephen Frost <sfrost@snowman.net>, Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2011-07-06T23:40:08Z
Lists: 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.