Re: [v9.2] Fix leaky-view problem, part 1 - Mailing list pgsql-hackers
From | Kohei KaiGai |
---|---|
Subject | Re: [v9.2] Fix leaky-view problem, part 1 |
Date | |
Msg-id | CADyhKSX25qLXB+yDKqe_J=ao7LUHPYsxHYUVvxtnQdZZ0bsQcQ@mail.gmail.com Whole thread Raw |
In response to | Re: [v9.2] Fix leaky-view problem, part 1 (Noah Misch <noah@2ndQuadrant.com>) |
List | pgsql-hackers |
The attached patch is a revised one; that utilizes untransformRelOptions() to construct a list of DefElem to be supplied into AT_ResetRelOptions commands. It enabled me to implement more compact as I expected. How about this approach to reset existing reloptions? I'll consolidate part-0, 1 and 2 patches after we make fix the direction to distinguish leaky qualifiers from others, in the thread of part-2. Right now, I'm considering the right way to choose qualifiers to be transformed into index scans. Thanks, 2011/7/7 Noah Misch <noah@2ndquadrant.com>: > On Thu, Jul 07, 2011 at 03:56:26PM +0100, Kohei KaiGai wrote: >> 2011/7/7 Noah Misch <noah@2ndquadrant.com>: >> > 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. >> > >> My opinion is ALTER TABLE SET/RESET code should be enhanced to accept >> an operation to reset all the existing options, rather than tricky >> updates of pg_class. > > The pg_class update has ~20 lines of idiomatic code; see tablecmds.c:7931-7951. > >> How about an idea to add AT_ResetAllRelOptions for internal use only? > > If some operation is purely internal and does not otherwise benefit from the > ALTER TABLE infrastructure, there's no benefit in involving ALTER TABLE. > DefineVirtualRelation() uses ALTER TABLE to add columns because all that code > needs to exist anyway. You could make a plain function to do the update that > gets called from both ATExecSetRelOptions() and DefineVirtualRelation(). > > Thanks, > nm > -- KaiGai Kohei <kaigai@kaigai.gr.jp>
Attachment
pgsql-hackers by date: