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

2011/7/7 Noah Misch <noah(at)2ndquadrant(dot)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.
How about an idea to add AT_ResetAllRelOptions for internal use only?

I'll fix up the regression test also.

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>